Re: [Qemu-devel] [RFC PATCH v4 09/11] spapr: Support topologies with unfilled cores

[Bharata B Rao]

On Fri, Sep 04, 2015 at 10:44:57AM +0200, Thomas Huth wrote:
> On 04/09/15 09:01, David Gibson wrote:
> > On Thu, Aug 06, 2015 at 10:57:15AM +0530, Bharata B Rao wrote:
> >> QEMU currently supports CPU topologies where there can be cores
> >> which are not completely filled with all the threads as per the
> >> specifed SMT mode.
> >>
> >> Restore support for such topologies (Eg -smp 15,cores=4,threads=4)
> >> The last core will always have the deficit even when -device options are
> >> used to cold-plug the cores.
> >>
> >> Signed-off-by: Bharata B Rao 
> > 
> > Is there a reason to support these silly toplogies, or should we just
> > error out if this is specified?

Only reason was to ensure that existing guest with such topologies
continue to boot like before.

> 
> FYI, I've recently submitted a patch that tries to catch such illegal
> SMP configurations and simply errors out in that case:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg04549.html
> 
> It's not upstream yet, but already in Eduardo's x86 branch. I think this
> will reject the bad topology from your example, too.

It does reject -smp 15,cores=4,threads=4, but with

-smp 15,cores=4,threads=4,maxcpus=16, the guest still boots with weird
topology.

[root@localhost ~]# lscpu
Architecture:  ppc64
CPU op-mode(s):32-bit, 64-bit
Byte Order:Big Endian
CPU(s):16
On-line CPU(s) list:   0-14
Off-line CPU(s) list:  15
Thread(s) per core:3
Core(s) per socket:1
Socket(s): 4
NUMA node(s):  1
Model: IBM pSeries (emulated by qemu)
L1d cache: 64K
L1i cache: 32K
NUMA node0 CPU(s): 0-14

[root@localhost ~]# ppc64_cpu --info
Core   0:0*1*2*3* 
Core   1:4*5*6*7* 
Core   2:8*9*   10*   11* 
Core   3:   12*   13*   14*   15

Should such topologies also be prevented from booting ?

Regards,
Bharata.

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[David kiarie]

On Wed, Sep 9, 2015 at 9:47 AM, Valentine Sinitsyn
 wrote:
> Hi all,
>
>
> On 09.09.2015 09:23, David kiarie wrote:
>>
>> On Wed, Sep 9, 2015 at 12:35 AM, Jan Kiszka  wrote:
>>>
>>> [thanks for forwarding, Peter]
>>>
>>> Hi Rita,
>>>
>>> On 2015-09-08 10:11, Peter Maydell wrote:

 On 7 September 2015 at 22:31, Rita Sinha  wrote:
>
> Hi Jan,
>
> I am interested in participating in next round of Outreachy program
> with AMD IOMMU emulation project.
>
>
> I have worked on BIOS projects which includes coreboot SeaBios etc and
> bootloaders like u-boot and grub. I have experience of working with
> qemu and feel that this project is the right match for my skillset.
> Kindly guide me how to go ahead with this.
>>>
>>>
>>> The particular AMD IOMMU project moved on since we listed it. I'm CC'ing
>>> David, who is currently working on it and just recently posted related
>>> patches, and Valentine who probably oversees the status better than I
>>> (due to my lacking involvement recently). David, maybe you can briefly
>>> comment on status and plans of your work.
>>
>>
>> Hi all,
>>
>> Most recent work is here
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg02759.html
>> . Most the code is Qemu device boilerplate(so there are a ton of
>> things to add but I wanted to have the existing work merged first).
>> The IOMMU just offers basic translation. From Valentine's previous
>> comments, I only have a few minor issues to fix in the code.
>
> This is a bit of off-topic here, but I'd argue they are minor. There are
> some inaccuracies in emulation, and IOMMU itself is rather feature-limited,
> just as you said. I doubt I'd be able to run current Jailhouse
> implementation on it, for instance, albeit I haven't tried. So, your patches
> are good start, but I feel there's a somewhat long way before they actually
> get merged.

Yep, that is possible. For instance, you were insisting on 'cache'
implemention ;) which IMHO, is optional but could be a good project
start.

>
> So, in short: there are still tasks to be done in AMD IOMMU emulation
> project. I also feel it's possible to parallelize them so David and Rita can
> continue without stepping at each other toe, if the program permits it.
>
>> I obviously do other things alongside :-D this project but given some
>> time I could get the code merged and continue to add other features.
>>
>>>
>>> For the Outreachy program, just like for GSoC, we need to find a good
>>> topic that is sufficiently clear defined on program start and not worked
>>> on in parallel during the runtime. There are still a number of open
>>> topics in this area, e.g. around the older Intel IOMMU model (error
>>> handling and reporting, interrupt remapping), or maybe we find something
>>> different - depends on your interests and experiences. Do you have any
>>> public references to your previous work?
>>>
>>> Then I'd suggest to schedule an irc meeting to discuss your interests
>>> and background a bit further and consider available options.
>>>
>>> Jan
>>>
>
> Regards,
> Valentine

Re: [Qemu-devel] [RFC PATCH v4 08/11] spapr: CPU hotplug support

[Bharata B Rao]

On Fri, Sep 04, 2015 at 04:58:38PM +1000, David Gibson wrote:
> On Thu, Aug 06, 2015 at 10:57:14AM +0530, Bharata B Rao wrote:
> > Support CPU hotplug via device-add command. Set up device tree
> > entries for the hotplugged CPU core and use the exising EPOW event
> > infrastructure to send CPU hotplug notification to the guest.
> > 
> > Create only cores explicitly from boot path as well as hotplug path
> > and let the ->plug() handler of the core create the threads of the core.
> > 
> > Also support cold plugged CPUs that are specified by -device option
> > on cmdline.
> > 
> > Signed-off-by: Bharata B Rao 
> > ---
> >  hw/ppc/spapr.c  | 166 
> > +++-
> >  hw/ppc/spapr_events.c   |   3 +
> >  hw/ppc/spapr_rtas.c |  11 +++
> >  target-ppc/translate_init.c |   8 +++
> >  4 files changed, 186 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index a106980..74637b3 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -599,6 +599,18 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> > *fdt, int offset,
> >  unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
> >  uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
> >  uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
> > +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
> > +sPAPRDRConnector *drc;
> > +sPAPRDRConnectorClass *drck;
> > +int drc_index;
> > +
> > +if (smc->dr_cpu_enabled) {
> > +drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index);
> > +g_assert(drc);
> > +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +drc_index = drck->get_index(drc);
> > +_FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", 
> > drc_index)));
> > +}
> >  
> >  _FDT((fdt_setprop_cell(fdt, offset, "reg", index)));
> >  _FDT((fdt_setprop_string(fdt, offset, "device_type", "cpu")));
> > @@ -1686,6 +1698,7 @@ static void ppc_spapr_init(MachineState *machine)
> >  char *filename;
> >  int smt = kvmppc_smt_threads();
> >  int smp_max_cores = DIV_ROUND_UP(max_cpus, smp_threads);
> > +int smp_cores = DIV_ROUND_UP(smp_cpus, smp_threads);
> 
> This shadows the global variable 'smp_cores' which has a different
> meaning, so this is a very bad idea.

Oh ok, will fix this.

> >  static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >DeviceState *dev, Error **errp)
> >  {
> >  sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
> > +sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
> >  
> >  if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >  int node;
> > @@ -2192,6 +2330,29 @@ static void spapr_machine_device_plug(HotplugHandler 
> > *hotplug_dev,
> >  }
> >  
> >  spapr_memory_plug(hotplug_dev, dev, node, errp);
> > +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> > +CPUState *cs = CPU(dev);
> > +PowerPCCPU *cpu = POWERPC_CPU(cs);
> > +
> > +if (!smc->dr_cpu_enabled && dev->hotplugged) {
> > +error_setg(errp, "CPU hotplug not supported for this machine");
> > +cpu_remove_sync(cs);
> > +return;
> > +}
> > +
> > +if (((smp_cpus % smp_threads) || (max_cpus % smp_threads)) &&
> > +dev->hotplugged) {
> > +error_setg(errp, "CPU hotplug not supported for the topology "
> > +   "with %d threads %d cpus and %d maxcpus since "
> > +   "CPUs can't be fit fully into cores",
> > +   smp_threads, smp_cpus, max_cpus);
> > +cpu_remove_sync(cs);
> 
> I'd kind of prefer to reject partial cores at initial startup, rather
> than only when we actually attempt to hotplug.

I am enforcing correct topologies only while hotplugging to ensure that
existing guests with such topologies continue to work. If that is not
required then this explicit check for only hotplugged CPUs won't be needed.

If Thomas' patch ensures that we never end up in topologies with partially
filled cores, then this check isn't required.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v13 5/5] hw/arm/virt: Add gic-version option to virt machine

[Pavel Fedin]

 Hello!

> This patch doesn't apply on latest git commit i.e. 298fae389 from Sep 7.
> Can you please rebase?

 Strange, i have checked my history, and my patch should be on top of that. 
But, well, i will post
rebased version today because anyway today it conflicted with trustzone 
support. But need to test it
first.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[Valentine Sinitsyn]

Hi all,

On 09.09.2015 09:23, David kiarie wrote:

On Wed, Sep 9, 2015 at 12:35 AM, Jan Kiszka  wrote:

[thanks for forwarding, Peter]

Hi Rita,

On 2015-09-08 10:11, Peter Maydell wrote:

On 7 September 2015 at 22:31, Rita Sinha  wrote:

Hi Jan,

I am interested in participating in next round of Outreachy program
with AMD IOMMU emulation project.


I have worked on BIOS projects which includes coreboot SeaBios etc and
bootloaders like u-boot and grub. I have experience of working with
qemu and feel that this project is the right match for my skillset.
Kindly guide me how to go ahead with this.


The particular AMD IOMMU project moved on since we listed it. I'm CC'ing
David, who is currently working on it and just recently posted related
patches, and Valentine who probably oversees the status better than I
(due to my lacking involvement recently). David, maybe you can briefly
comment on status and plans of your work.


Hi all,

Most recent work is here
http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg02759.html
. Most the code is Qemu device boilerplate(so there are a ton of
things to add but I wanted to have the existing work merged first).
The IOMMU just offers basic translation. From Valentine's previous
comments, I only have a few minor issues to fix in the code.
This is a bit of off-topic here, but I'd argue they are minor. There are 
some inaccuracies in emulation, and IOMMU itself is rather 
feature-limited, just as you said. I doubt I'd be able to run current 
Jailhouse implementation on it, for instance, albeit I haven't tried. 
So, your patches are good start, but I feel there's a somewhat long way 
before they actually get merged.


So, in short: there are still tasks to be done in AMD IOMMU emulation 
project. I also feel it's possible to parallelize them so David and Rita 
can continue without stepping at each other toe, if the program permits it.



I obviously do other things alongside :-D this project but given some
time I could get the code merged and continue to add other features.



For the Outreachy program, just like for GSoC, we need to find a good
topic that is sufficiently clear defined on program start and not worked
on in parallel during the runtime. There are still a number of open
topics in this area, e.g. around the older Intel IOMMU model (error
handling and reporting, interrupt remapping), or maybe we find something
different - depends on your interests and experiences. Do you have any
public references to your previous work?

Then I'd suggest to schedule an irc meeting to discuss your interests
and background a bit further and consider available options.

Jan



Regards,
Valentine

Re: [Qemu-devel] [PATCH] linux-headers: Fix type cast warning for 64 bit MinGW-w64

[Michael S. Tsirkin]

On Tue, Sep 08, 2015 at 08:15:50PM +0100, Peter Maydell wrote:
> On 8 September 2015 at 20:11, Stefan Weil  wrote:
> > Type casts from pointers to unsigned long don't work on 64 bit Windows
> > because both types have different size.
> >
> > Compiler warning:
> >
> > include/standard-headers/linux/virtio_ring.h:146:23:
> >  warning: cast from pointer to integer of different size
> >  [-Wpointer-to-int-cast]
> >   vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + ...
> >^
> >
> > Signed-off-by: Stefan Weil 
> > ---
> >
> > This is a modification of a Linux header file,
> > something we usually try to avoid.
> >
> > I did not succeed in removing this header from compilations
> > for Windows (which would have been the best solution).
> 
> No, this header is supposed to work everywhere (that's why
> it's in standard-headers/ rather than linux-headers),
> because virtio devices must work everywhere. We need to
> improve the process which creates it from the kernel header
> by automatedly fixing up non-portable constructs
> (which is handled by scripts/update-linux-headers.sh).
> Some extra seddery in cp_virtio() is probably required.
> 
> thanks
> -- PMM

commit d768f32aec8c0ebb8499ffca89cfed8f5f1a4432
Author: Thomas Huth 
Date:   Thu Jul 2 09:21:22 2015 +0200

virtio: Fix typecast of pointer in vring_init()

The virtio_ring.h header is used in userspace programs (ie. QEMU),
too. Here we can not assume that sizeof(pointer) is the same as
sizeof(long), e.g. when compiling for Windows, so the typecast in
vring_init() should be done with (uintptr_t) instead of (unsigned long).

Signed-off-by: Thomas Huth 
Signed-off-by: Michael S. Tsirkin 


Fixed this, so it's just a question of updating from latest linux.

-- 
MST

Re: [Qemu-devel] [opnfv-tech-discuss] rfc: vhost user enhancements for vm2vm communication

[Zhang, Yang Z]

Claudio Fontana wrote on 2015-09-07:
> Coming late to the party,
> 
> On 31.08.2015 16:11, Michael S. Tsirkin wrote:
>> Hello!
>> During the KVM forum, we discussed supporting virtio on top
>> of ivshmem. I have considered it, and came up with an alternative
>> that has several advantages over that - please see below.
>> Comments welcome.
> 
> as Jan mentioned we actually discussed a virtio-shmem device which would
> incorporate the advantages of ivshmem (so no need for a separate ivshmem
> device), which would use the well known virtio interface, taking advantage of
> the new virtio-1 virtqueue layout to split r/w and read-only rings as seen 
> from
> the two sides, and make use also of BAR0 which has been freed up for use by
> the device.

Interesting! Can you elaborate it? 

> 
> This way it would be possible to share the rings and the actual memory
> for the buffers in the PCI bars. The guest VMs could decide to use the
> shared memory regions directly as prepared by the hypervisor (in the

"the shared memory regions" here means share another VM's memory or like 
ivshmem?

> jailhouse case) or QEMU/KVM, or perform their own validation on the
> input depending on the use case.
> 
> Of course the communication between VMs needs in this case to be
> pre-configured and is quite static (which is actually beneficial in our use 
> case).

pre-configured means user knows which VMs will talk to each other and configure 
it when booting guest(i.e. in Qemu command line)?

> 
> But still in your proposed solution, each VM needs to be pre-configured to
> communicate with a specific other VM using a separate device right?
> 
> But I wonder if we are addressing the same problem.. in your case you are
> looking at having a shared memory pool for all VMs potentially visible to all 
> VMs
> (the vhost-user case), while in the virtio-shmem proposal we discussed we
> were assuming specific different regions for every channel.
> 
> Ciao,
> 
> Claudio
> 
> 
>


Best regards,
Yang

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[David kiarie]

On Wed, Sep 9, 2015 at 8:01 AM, Rita Sinha  wrote:
> Hi David,
>
> Please find my response inline.
>
>
>>
>> Hi all,
>>
>> Most recent work is here
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg02759.html
>> . Most the code is Qemu device boilerplate(so there are a ton of
>> things to add but I wanted to have the existing work merged first).
>> The IOMMU just offers basic translation. From Valentine's previous
>> comments, I only have a few minor issues to fix in the code.
>>
>> I obviously do other things alongside :-D this project but given some
>> time I could get the code merged and continue to add other features.
>>
>
> Let me know if I can help in this effort which will help me get
> started on this project.

Hi Rita,

Most likely than not you'll work on the Intel IOMMU and I would
suggest, if you wish to get your feet dirty, just start right away
with the Intel IOMMU (In which case Jan could direct you better)
instead of the AMD one.

>
> Regards,
> Rita Sinha

Re: [Qemu-devel] [PATCH RFC v5 30/32] qapi: New QMP command query-qmp-schema for QMP introspection

[Markus Armbruster]

Eric Blake  writes:

> On 09/07/2015 04:16 AM, Markus Armbruster wrote:
>> qapi/introspect.json defines the introspection schema.  It's designed
>> for QMP introspection, but should do for similar uses, such as QGA.
>> 
>> The introspection schema does not reflect all the rules and
>> restrictions that apply to QAPI schemata.  A valid QAPI schema has an
>> introspection value conforming to the introspection schema, but the
>> converse is not true.
>> 
>> Introspection lowers away a number of schema details, and makes
>> implicit things explicit:
>> 
>
>> +##
>> +# @SchemaInfoObjectMember
>> +#
>> +# An object member.
>> +#
>> +# @name: the member's name, as defined in the QAPI schema.
>> +#
>> +# @type: the name of the member's type.
>> +#
>> +# @default: #optional default when used as command parameter.
>> +#   If absent, the parameter is mandatory.
>> +#   If present, the value must be null.  The parameter is
>> +#   optional, and behavior when it's missing is not specified
>> +#   here.
>> +#   Future extension: if present and non-null, the parameter
>> +#   is optional, and defaults to this value.
>> +#
>
>> +##
>> +# @SchemaInfoObjectVariant
>> +#
>> +# The variant members for a value of the type tag.
>> +#
>> +# @case: a value of the type tag.
>> +#
>> +# @type: the name of the object type that provides the variant members
>> +# when the type tag has value @case.
>
> You aren't consistent on whether secondary lines describing the same
> @variable are indented or flush left.  I don't care enough to hold up
> review, but just pointing it out in case you want to reflow some text.

I'm happy to do small, local, obviously safe changes.

> I've finished re-reading 31 and 32, and double-checking that the
> combined text of all three patches together makes sense as a whole.
> Looks like we're ready for this series to come out of RFC soon :)
>
> And I'll start rebasing and posting my followup patches that have
> already been on list...

Thanks!

Re: [Qemu-devel] [PATCH] spapr: Reduce advertised max LUNs for spapr_vscsi

[Thomas Huth]

On 09/09/15 03:22, David Gibson wrote:
> The implementation of the PAPR paravirtual SCSI adapter currently
> allows up to 32 LUNs (max_lun == 31).  However the adapter isn't really
> designed to support lots of devices - the PowerVM implementation only
> ever puts one disk per vSCSI controller.

Do you know how many LUNs are advertised by PowerVM?

> More specifically, the Linux guest side vscsi driver (the only one we
> really care about) is hardcoded to allow a maximum of 8 LUNs.

So what about changing the vscsi driver in Linux instead to support more
LUNs?

 Thomas

Re: [Qemu-devel] [Patch for-2.5 v2 3/6] Add new block driver interface to add/delete a BDS's child

[Wen Congyang]

On 09/08/2015 11:52 PM, Eric Blake wrote:
> On 09/08/2015 03:10 AM, Wen Congyang wrote:
> 
>>> Design-wise, I think we really want to have the add-child operation be
>>> handed a pre-opened BDS, rather than the options dictionary to open the
>>> BDS itself.  That is, we should use the existing blockdev-add (and
>>> enhance it to support everything) to open the BDS, and then this command
>>> should just attach that BDS as the new child (which is why it IS
>>> important that we validate that the new BDS being added doesn't create
>>> an invalid loop).
>>>
>>
>> How to check it? The parent BDS can get all children. But the child doesn't
>> know if it is some BDS's child.
> 
> If I'm not mistaken, a child DOES know what its parent(s) are, once we
> have Max's series for NULL BDS representing a BB without media.
> 

Which patch? I don't find it.

Thanks
Wen Congyang

Re: [Qemu-devel] [PATCH 02/18] aio: rename bh_lock to list_lock

[Fam Zheng]

On Thu, 08/06 15:36, Paolo Bonzini wrote:
> This will be used for AioHandlers too.  There is going to be little
> or no contention, so it is better to reuse the same lock.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Fam Zheng 

> ---
>  async.c | 16 
>  include/block/aio.h |  2 +-
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/async.c b/async.c
> index f992914..7e97614 100644
> --- a/async.c
> +++ b/async.c
> @@ -50,12 +50,12 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void 
> *opaque)
>  .cb = cb,
>  .opaque = opaque,
>  };
> -qemu_mutex_lock(&ctx->bh_lock);
> +qemu_mutex_lock(&ctx->list_lock);
>  bh->next = ctx->first_bh;
>  /* Make sure that the members are ready before putting bh into list */
>  smp_wmb();
>  ctx->first_bh = bh;
> -qemu_mutex_unlock(&ctx->bh_lock);
> +qemu_mutex_unlock(&ctx->list_lock);
>  return bh;
>  }
>  
> @@ -92,7 +92,7 @@ int aio_bh_poll(AioContext *ctx)
>  
>  /* remove deleted bhs */
>  if (!ctx->walking_bh) {
> -qemu_mutex_lock(&ctx->bh_lock);
> +qemu_mutex_lock(&ctx->list_lock);
>  bhp = &ctx->first_bh;
>  while (*bhp) {
>  bh = *bhp;
> @@ -103,7 +103,7 @@ int aio_bh_poll(AioContext *ctx)
>  bhp = &bh->next;
>  }
>  }
> -qemu_mutex_unlock(&ctx->bh_lock);
> +qemu_mutex_unlock(&ctx->list_lock);
>  }
>  
>  return ret;
> @@ -234,7 +234,7 @@ aio_ctx_finalize(GSource *source)
>  
>  thread_pool_free(ctx->thread_pool);
>  
> -qemu_mutex_lock(&ctx->bh_lock);
> +qemu_mutex_lock(&ctx->list_lock);
>  while (ctx->first_bh) {
>  QEMUBH *next = ctx->first_bh->next;
>  
> @@ -244,12 +244,12 @@ aio_ctx_finalize(GSource *source)
>  g_free(ctx->first_bh);
>  ctx->first_bh = next;
>  }
> -qemu_mutex_unlock(&ctx->bh_lock);
> +qemu_mutex_unlock(&ctx->list_lock);
>  
>  aio_set_event_notifier(ctx, &ctx->notifier, NULL);
>  event_notifier_cleanup(&ctx->notifier);
>  rfifolock_destroy(&ctx->lock);
> -qemu_mutex_destroy(&ctx->bh_lock);
> +qemu_mutex_destroy(&ctx->list_lock);
>  timerlistgroup_deinit(&ctx->tlg);
>  }
>  
> @@ -318,7 +318,7 @@ AioContext *aio_context_new(Error **errp)
> (EventNotifierHandler *)
> event_notifier_dummy_cb);
>  ctx->thread_pool = NULL;
> -qemu_mutex_init(&ctx->bh_lock);
> +qemu_mutex_init(&ctx->list_lock);
>  rfifolock_init(&ctx->lock, NULL, NULL);
>  timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
>  
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 9dd32e0..026f6c1 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -89,7 +89,7 @@ struct AioContext {
>  uint32_t notify_me;
>  
>  /* lock to protect between bh's adders and deleter */
> -QemuMutex bh_lock;
> +QemuMutex list_lock;
>  
>  /* Anchor of the list of Bottom Halves belonging to the context */
>  struct QEMUBH *first_bh;
> -- 
> 2.4.3
> 
> 
> 

Re: [Qemu-devel] [PATCH 01/18] iothread: release iothread around aio_poll

[Fam Zheng]

On Thu, 08/06 15:35, Paolo Bonzini wrote:
> This is the first step towards having fine-grained critical sections in
> dataplane threads, which resolves lock ordering problems between
> address_space_* functions (which need the BQL when doing MMIO, even
> after we complete RCU-based dispatch) and the AioContext.
> 
> Because AioContext does not use contention callbacks anymore, the
> unit test has to be changed.
> 
> Previously applied as a0710f7995f914e3044e5899bd8ff6c43c62f916 and
> then reverted.
> 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Paolo Bonzini 
> ---
>  async.c | 22 +++---
>  docs/multiple-iothreads.txt | 25 +++--
>  include/block/aio.h |  3 ---
>  iothread.c  | 11 ++-
>  tests/test-aio.c| 19 +++
>  5 files changed, 31 insertions(+), 49 deletions(-)
> 
> diff --git a/async.c b/async.c
> index efce14b..f992914 100644
> --- a/async.c
> +++ b/async.c
> @@ -79,8 +79,8 @@ int aio_bh_poll(AioContext *ctx)
>   * aio_notify again if necessary.
>   */
>  if (!bh->deleted && atomic_xchg(&bh->scheduled, 0)) {
> -/* Idle BHs and the notify BH don't count as progress */
> -if (!bh->idle && bh != ctx->notify_dummy_bh) {
> +/* Idle BHs don't count as progress */
> +if (!bh->idle) {
>  ret = 1;
>  }
>  bh->idle = 0;
> @@ -232,7 +232,6 @@ aio_ctx_finalize(GSource *source)
>  {
>  AioContext *ctx = (AioContext *) source;
>  
> -qemu_bh_delete(ctx->notify_dummy_bh);
>  thread_pool_free(ctx->thread_pool);
>  
>  qemu_mutex_lock(&ctx->bh_lock);
> @@ -299,19 +298,6 @@ static void aio_timerlist_notify(void *opaque)
>  aio_notify(opaque);
>  }
>  
> -static void aio_rfifolock_cb(void *opaque)
> -{
> -AioContext *ctx = opaque;
> -
> -/* Kick owner thread in case they are blocked in aio_poll() */
> -qemu_bh_schedule(ctx->notify_dummy_bh);
> -}
> -
> -static void notify_dummy_bh(void *opaque)
> -{
> -/* Do nothing, we were invoked just to force the event loop to iterate */
> -}
> -
>  static void event_notifier_dummy_cb(EventNotifier *e)
>  {
>  }
> @@ -333,11 +319,9 @@ AioContext *aio_context_new(Error **errp)
> event_notifier_dummy_cb);
>  ctx->thread_pool = NULL;
>  qemu_mutex_init(&ctx->bh_lock);
> -rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
> +rfifolock_init(&ctx->lock, NULL, NULL);
>  timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
>  
> -ctx->notify_dummy_bh = aio_bh_new(ctx, notify_dummy_bh, NULL);
> -
>  return ctx;
>  }
>  
> diff --git a/docs/multiple-iothreads.txt b/docs/multiple-iothreads.txt
> index 40b8419..723cc7e 100644
> --- a/docs/multiple-iothreads.txt
> +++ b/docs/multiple-iothreads.txt
> @@ -105,13 +105,10 @@ a BH in the target AioContext beforehand and then call 
> qemu_bh_schedule().  No
>  acquire/release or locking is needed for the qemu_bh_schedule() call.  But be
>  sure to acquire the AioContext for aio_bh_new() if necessary.
>  
> -The relationship between AioContext and the block layer
> 
> -The AioContext originates from the QEMU block layer because it provides a
> -scoped way of running event loop iterations until all work is done.  This
> -feature is used to complete all in-flight block I/O requests (see
> -bdrv_drain_all()).  Nowadays AioContext is a generic event loop that can be
> -used by any QEMU subsystem.
> +AioContext and the block layer
> +--
> +The AioContext originates from the QEMU block layer, even though nowadays
> +AioContext is a generic event loop that can be used by any QEMU subsystem.
>  
>  The block layer has support for AioContext integrated.  Each BlockDriverState
>  is associated with an AioContext using bdrv_set_aio_context() and
> @@ -122,9 +119,17 @@ Block layer code must therefore expect to run in an 
> IOThread and avoid using
>  old APIs that implicitly use the main loop.  See the "How to program for
>  IOThreads" above for information on how to do that.
>  
> -If main loop code such as a QMP function wishes to access a BlockDriverState 
> it
> -must first call aio_context_acquire(bdrv_get_aio_context(bs)) to ensure the
> -IOThread does not run in parallel.
> +If main loop code such as a QMP function wishes to access a BlockDriverState
> +it must first call aio_context_acquire(bdrv_get_aio_context(bs)) to ensure
> +that callbacks in the IOThread do not run in parallel.
> +
> +Code running in the monitor typically uses bdrv_drain() to ensure that
> +past requests from the guest are completed.  When a block device is
> +running in an IOThread, the IOThread can also process requests from
> +the guest (via ioeventfd).  These requests have to be blocked with
> +aio_disable_clients() before calling bdrv_drain().  You can then reenable
> 

Re: [Qemu-devel] [PATCH v2] spapr_drc: don't allow 'empty' DRCs to be unisolated

[David Gibson]

On Tue, Sep 08, 2015 at 06:44:55PM -0500, Michael Roth wrote:
> Logical resources start with allocation-state:UNUSABLE /
> isolation-state:ISOLATED. During hotplug, guests will transition
> them to allocate-state:USABLE, and then to isolate-state:UNISOLATED.
> The former transition does not seem to have any failure path for
> cases where a DRC does not have any resources associated with it to
> allocate for guest, but instead relies on the subsequent
> isolation-state:UNISOLATED transition to indicate failure in this
> situation.
> 
> Currently DRC code does not implement this logic, but instead
> tries to indicate failure by refusing the allocation-state:USABLE
> transition. Unfortunately, since that's not a documented failure
> path, guests continue undeterred, causing undefined behavior in
> QEMU and guest code.
> 
> Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).
> 
> Cc: qemu-...@nongnu.org
> Cc: David Gibson 
> Cc: Bharata B Rao 
> Signed-off-by: Michael Roth 
> ---
> v2:
>  - actually include the full changeset in the patch

Several queries for clarification:

 * Is this intended to replace Bharata's patch "spapr_drc:
   Return correct state for logical DR in entity_sense()" or to apply
   on top of it?

 * If I'm understanding correctly, the problem here is that although
   the guest is supposed to check for failures to set the allocation
   state, it's actually not?  This patch is to make qemu gracefully
   handle the guest's failure to do this?  Is that right?
   
> ---
>  hw/ppc/spapr_drc.c | 12 
>  hw/ppc/spapr_rtas.c|  9 +++--
>  include/hw/ppc/spapr.h |  1 +
>  include/hw/ppc/spapr_drc.h |  2 ++
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 9ce844a..c1f664f 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -66,6 +66,18 @@ static int set_isolation_state(sPAPRDRConnector *drc,
>  
>  DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
>  
> +if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> +/* cannot unisolate a non-existant resource. this generally
> + * happens for logical resources where transitions from
> + * allocation-state:UNUSABLE to allocation-state:USABLE are
> + * unguarded, but instead rely on a subsequent
> + * isolation-state:UNISOLATED transition to indicate failure
> + */
> +if (!drc->dev) {
> +return -1;
> +}
> +}
> +
>  drc->isolation_state = state;
>  
>  if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 3b7b20b..0ddedca 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -372,6 +372,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  uint32_t sensor_type;
>  uint32_t sensor_index;
>  uint32_t sensor_state;
> +int drc_ret, ret = RTAS_OUT_SUCCESS;
>  sPAPRDRConnector *drc;
>  sPAPRDRConnectorClass *drck;
>  
> @@ -413,7 +414,11 @@ static void rtas_set_indicator(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  spapr_ccs_remove(spapr, ccs);
>  }
>  }
> -drck->set_isolation_state(drc, sensor_state);
> +drc_ret = drck->set_isolation_state(drc, sensor_state);
> +if (drc_ret != 0) {
> +ret = (drc_ret == -1) ? RTAS_OUT_NO_SUCH_INDICATOR
> +  : RTAS_OUT_HW_ERROR;
> +}
>  break;
>  case RTAS_SENSOR_TYPE_DR:
>  drck->set_indicator_state(drc, sensor_state);
> @@ -425,7 +430,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  goto out_unimplemented;
>  }
>  
> -rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +rtas_st(rets, 0, ret);
>  return;
>  
>  out_unimplemented:
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c75cc5e..ffb108d 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -412,6 +412,7 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
>  #define RTAS_OUT_BUSY   -2
>  #define RTAS_OUT_PARAM_ERROR-3
>  #define RTAS_OUT_NOT_SUPPORTED  -3
> +#define RTAS_OUT_NO_SUCH_INDICATOR  -3
>  #define RTAS_OUT_NOT_AUTHORIZED -9002
>  
>  /* RTAS tokens */
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 28ffeae..b2c1209 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -165,6 +165,8 @@ typedef struct sPAPRDRConnectorClass {
>  /*< public >*/
>  
>  /* accessors for guest-visible (generally via RTAS) DR state */
> +
> +/* returns -1 if DRC cannot be set to requested isolation state */
>  int (*set_isolation_state)(sPAPRDRConnector *drc,
> sPAPRDRIsolationState state);
>  int (*set_indicator_state)(sPAPRDRConnector *dr

Re: [Qemu-devel] [FIX PATCH] spapr_drc: Return correct state for logical DR in entity_sense()

[David Gibson]

On Tue, Sep 08, 2015 at 05:03:25PM -0500, Michael Roth wrote:
> Quoting Michael Roth (2015-09-08 16:03:56)
> > Quoting David Gibson (2015-09-07 20:22:50)
> > > On Mon, Sep 07, 2015 at 11:37:04AM +0530, Bharata B Rao wrote:
> > > > When drmgr is run in the guest to add a device for which device_add
> > > > hasn't been issued in QEMU, configure-connector call fails.
> > > > When configure-connector call fails, the guest would release (*)
> > > > the previously acquired DRC by setting back the DRC isolation state
> > > > to ISOLATED and allocation state to UNUSABLE. These calls will be issued
> > > > only if get-sensor-state call returns PRESENT state. However currently 
> > > > for
> > > > a logical DR, entity_sense() would unconditinally return UNUSABLE
> > > > state only. This prevents any subsequent hotplug of the device with
> > > > that DRC.
> > 
> > This seems a little odd. I think we default to ALLOCATION_STATE_UNUSABLE
> > for logical DR, and it's up the guest to transition to USABLE, which
> > probably happens prior to the configure-connector calls. So I think the
> > net effect of this fix is that guest will see these unallocated/unattached
> > resources the same way they would a resource that was actually attached
> > via device_add, and all we're really doing is working around the
> > eventual configuration failure that that will lead to by pretending a
> > resource was actually there.
> > 
> > According to PAPR+ 2.7:
> > 
> > 13.7.3.1 Acquire Logical Resource from Resource Pool:
> > 
> >   If the state is “unusable” the OS issues set-indicator (allocation-state, 
> > usable) to attempt to allocate the re-
> >   source. Similarly, if the state is “available for exchange” the OS issues 
> > set-indicator (allocation-state, ex-
> >   change) to attempt to allocate the resource, and if the state is 
> > “available for recovery” the OS issues
> >   set-indicator (allocation-state, recover) to attempt to allocate the 
> > resource.
> > 
> > and
> > 
> > 13.7 Logical Resource Dynamic Reconfiguration (LRDR):
> > 
> >   The OS may use the get-sensor-state RTAS call with the dr-entity-sense 
> > token to deter-
> > mine if a given drc-index refers to a connector that is currently usable 
> > for DR operations. If the connector is not
> > currently usable the return state is “DR entity unusable” (2). A 
> > set-indicator (isolation state) RTAS call to an unusable
> > connector or (dr-indicator) to any logical resource connector results in a 
> > “No such indicator implemented” return sta-
> > tus.  
> > 
> > So I think maybe the proper fix is to make sure that
> > drc->set_indicator_state() fails with an error that indicates to RTAS to
> > return NO_SENSOR (-3) for cases where we haven't attached a resource
> > to the DRC via device_add.
> 
> Patch incoming:
> 
>   spapr_drc: don't allow 'empty' DRCs to be unisolated
> 
> applies to spapr-next but requires revert of this patch.
> 
> Bharata, can you give it a spin with CPU hotplug and see if it fixes the
> issue you hit?
> 
> > 
> > Which also kind of re-opens the discussion of whether or not
> > drc->set_indicator_state() should return RTAS errors directly. I'd
> > still stray away from that for now but maybe if we get more cases
> > like this it'll start becoming more practical.
> 
> I'm starting to second-guess myself on this. I'm trying to maintain
> separation between RTAS/DRC but result is a bit pathological. Feel free
> to comment in the above patch.

To my mind the set_indicator path is pretty much inherently specific
to PAPR's weird way of doing things.  For that reason I think it's
just going to be simplest for it to return PAPR defined errors -
i.e. RTAS errors.

The whole thing is so tied to PAPR, I don't think it makes sense to
try to use generic error codes for it.  Trying to separate "DRC" error
codes from the RTAS codes they result in sounds like a translation
layer for no real gain.

> Just FYI: original author names appear to have gotten lost in recent
> spapr-next rebase.

Ah, thanks for catching that.  Should be fixed now.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


pgp2bkB9GxkXK.pgp
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH v4 04/11] cpus: Add a sync version of cpu_remove()

[Bharata B Rao]

On Fri, Sep 04, 2015 at 04:11:38PM +1000, David Gibson wrote:
> On Thu, Aug 06, 2015 at 10:57:10AM +0530, Bharata B Rao wrote:
> > This sync API will be used by the CPU hotplug code to wait for the CPU to
> > completely get removed before flagging the failure to the device_add
> > command.
> > 
> > Sync version of this call is needed to correctly recover from CPU
> > realization failures when ->plug() handler fails.
> > 
> > Signed-off-by: Bharata B Rao 
> > ---
> >  cpus.c| 14 ++
> >  include/qom/cpu.h |  8 
> >  2 files changed, 22 insertions(+)
> > 
> > diff --git a/cpus.c b/cpus.c
> > index 73ae2e7..9d9644e 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -999,6 +999,8 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
> >  qemu_kvm_wait_io_event(cpu);
> >  if (cpu->exit && !cpu_can_run(cpu)) {
> >  qemu_kvm_destroy_vcpu(cpu);
> > +cpu->created = false;
> > +qemu_cond_signal(&qemu_cpu_cond);
> >  qemu_mutex_unlock(&qemu_global_mutex);
> >  return NULL;
> >  }
> > @@ -1104,6 +1106,8 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> >  }
> >  if (remove_cpu) {
> >  qemu_tcg_destroy_vcpu(remove_cpu);
> > +cpu->created = false;
> > +qemu_cond_signal(&qemu_cpu_cond);
> >  remove_cpu = NULL;
> >  }
> >  }
> > @@ -1283,6 +1287,16 @@ void cpu_remove(CPUState *cpu)
> >  qemu_cpu_kick(cpu);
> >  }
> >  
> > +void cpu_remove_sync(CPUState *cpu)
> > +{
> > +cpu->stop = true;
> > +cpu->exit = true;
> > +qemu_cpu_kick(cpu);
> 
> It would be nicer for this to call the async cpu_remove() above, to
> ensure they stay in sync.

Makes sense, will incorporate this in the next iteration.

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH v4 02/11] exec: Do vmstate unregistration from cpu_exec_exit()

[Bharata B Rao]

On Fri, Sep 04, 2015 at 04:03:43PM +1000, David Gibson wrote:
> On Thu, Aug 06, 2015 at 10:57:08AM +0530, Bharata B Rao wrote:
> > cpu_exec_init() does vmstate_register and register_savevm for the CPU 
> > device.
> > These need to be undone from cpu_exec_exit(). These changes are needed to
> > support CPU hot removal and also to correctly fail hotplug attempts
> > beyond max_cpus.
> > 
> > Signed-off-by: Bharata B Rao 
> 
> As with 1/2, looks reasonable, but I'm wondering how x86 hotplug is
> getting away without this.

x86 does this explicitly in CPU's unrealizefn. Since registration is
done from cpu_exec_init(), I though un-registration could be done
in cpu_exec_init() instead of each arch doing it separately.

As said earlier, I will probably pursue these changes separately from
this series.

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH v4 01/11] exec: Remove cpu from cpus list during cpu_exec_exit()

[Bharata B Rao]

On Fri, Sep 04, 2015 at 03:31:24PM +1000, David Gibson wrote:
> On Thu, Aug 06, 2015 at 10:57:07AM +0530, Bharata B Rao wrote:
> > CPUState *cpu gets added to the cpus list during cpu_exec_init(). It
> > should be removed from cpu_exec_exit().
> > 
> > cpu_exec_init() is called from generic CPU::instance_finalize and some
> > archs like PowerPC call it from CPU unrealizefn. So ensure that we
> > dequeue the cpu only once.
> > 
> > Instead of introducing a new field CPUState.queued, I could have used
> > CPUState.cpu_index to check if the cpu is already dequeued from the list.
> > Since that doesn't work for CONFIG_USER_ONLY, I had to add a new field.
> > 
> > Signed-off-by: Bharata B Rao 
> 
> This seems reasonable to me, but I'm wondering how x86 cpu hotplug /
> unplug is working without it.

x86 hotplug/unplug code currently resides in Zhu's git tree
(git://github.com/zhugh/qemu). They are removing the CPU from the list
explicitly in x86 CPU's instance_finalize routine.

Since we add CPU to the list in cpu_exec_init(), I thought it makes
sense to remove it in cpu_exec_exit().

May be it makes sense to separately purse this patch and the next one
so that other archs are also taken into account correctly.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH 0/2] ACPI/arm-virt: add DBG2

[Andrew Jones]

On Wed, Sep 09, 2015 at 12:25:45PM +0800, Shannon Zhao wrote:
> 
> 
> On 2015/9/8 21:04, Leif Lindholm wrote:
> > On Tue, Sep 08, 2015 at 11:18:27AM +0800, Shannon Zhao wrote:
> >> On 2015/9/7 22:23, Leif Lindholm wrote:
> >>> The Debug Port Table 2 (DBG2) is mandated by the ARM Server Base Boot
> >>> Requirements specification. Add the DBG2 table definitions, and set up
> >>> an entry in the ARM virt machine for the pl011 UART.
> >>
> >> Looking at Documentation/arm64/acpi_object_usage.txt in Linux kernel, it
> >> says
> >> "
> >> DBG2   Signature Reserved (signature == "DBG2")
> >>== DeBuG port table 2 ==
> >>Microsoft only table, will not be supported.
> >> "
> >> It seems that Linux kernel doesn't support or need it, but Windows
> >> requires it. So does it need to test this on Windows?
> > 
> > No, it can be tested under Linux with the set I just sent out:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/368614.html
> 
> So you need to change Documentation/arm64/acpi_object_usage.txt as well.
> But that is kernel side.

I also wonder if Documentation/arm64/arm-acpi.txt should have its
"Booting using ACPI tables" section updated to reference (describe even?)
the use of SPCR. Maybe Leif's already doing that with the
this-time-really-for-upstream kernel spcr patches.

drew

Re: [Qemu-devel] [PATCH] typofixes - v4 - https://github.com/vlajos/misspell_fixer

[Stefan Weil]

Am 08.09.2015 um 23:48 schrieb Peter Maydell:
> On 8 September 2015 at 22:45, Veres Lajos  wrote:
>> Signed-off-by: Veres Lajos 
>> ---
>>  disas/i386.c|2 +-
>>  disas/s390.c|4 ++--
>>  docs/specs/ppc-spapr-hcalls.txt |4 ++--
>>  docs/writing-qmp-commands.txt   |2 +-
>>  hw/audio/fmopl.c|2 +-
>>  hw/core/qdev.c  |2 +-
>>  hw/cris/axis_dev88.c|2 +-
>>  hw/display/qxl-render.c |2 +-
>>  hw/dma/xilinx_axidma.c  |2 +-
>>  hw/input/stellaris_input.c  |2 +-
>>  hw/intc/xics.c  |2 +-
>>  hw/net/fsl_etsec/etsec.c|2 +-
>>  hw/sd/pl181.c   |2 +-
>>  hw/vfio/pci.c   |2 +-
>>  hw/xen/xen-host-pci-device.c|2 +-
>>  include/block/scsi.h|2 +-
>>  include/exec/memory.h   |2 +-
>>  libcacard/card_7816t.h  |2 +-
>>  libdecnumber/decContext.c   |2 +-
>>  libdecnumber/decNumber.c|8 
>>  linux-user/syscall.c|1 -
>>  linux-user/syscall_defs.h   |4 ++--
>>  qga/commands-posix.c|2 +-
>>  target-alpha/mem_helper.c   |2 +-
>>  target-arm/cpu.h|4 ++--
>>  target-cris/cpu.h   |2 +-
>>  target-cris/translate.c |6 +++---
>>  target-mips/helper.c|2 +-
>>  target-s390x/kvm.c  |4 ++--
>>  target-sh4/helper.c |6 +++---
>>  tcg/ia64/tcg-target.c   |2 +-
>>  tcg/tcg.c   |4 ++--
>>  tests/image-fuzzer/runner.py|2 +-
>>  tests/qemu-iotests/041  |2 +-
>>  util/qemu-thread-posix.c|2 +-
>>  35 files changed, 47 insertions(+), 48 deletions(-)
> Reviewed-by: Peter Maydell 
>
> thanks
> -- PMM
>

The subject of this patch can be fixed when it is applied to trivial.

Regards
Stefan

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[Rita Sinha]

Hi David,

Please find my response inline.


>
> Hi all,
>
> Most recent work is here
> http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg02759.html
> . Most the code is Qemu device boilerplate(so there are a ton of
> things to add but I wanted to have the existing work merged first).
> The IOMMU just offers basic translation. From Valentine's previous
> comments, I only have a few minor issues to fix in the code.
>
> I obviously do other things alongside :-D this project but given some
> time I could get the code merged and continue to add other features.
>

Let me know if I can help in this effort which will help me get
started on this project.

Regards,
Rita Sinha

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[Rita Sinha]

Hi Jan,

Please find my response inline.

>
> For the Outreachy program, just like for GSoC, we need to find a good
> topic that is sufficiently clear defined on program start and not worked
> on in parallel during the runtime. There are still a number of open
> topics in this area, e.g. around the older Intel IOMMU model (error
> handling and reporting, interrupt remapping), or maybe we find something
> different - depends on your interests and experiences. Do you have any
> public references to your previous work?
>

Most of my work has been for proprietary code repository and for
making contributions to open source projects is the reason why I am
applying to Outreachy.

I have worked on ACPI, qemu, coreboot, SeaBIOS, u-boot and linux
kernel and would love to work related areas.
Could you list down the open items for the same where I can select the
one which matches my skillset so that I can contribute more
efficiently to  this community.

> Then I'd suggest to schedule an irc meeting to discuss your interests
> and background a bit further and consider available options.
>

I guess that would be wonderful. Let me know when you are available on the IRC.


Regards,
Rita Sinha

Re: [Qemu-devel] [PATCH 0/2] ACPI/arm-virt: add DBG2

[Shannon Zhao]

On 2015/9/8 21:04, Leif Lindholm wrote:
> On Tue, Sep 08, 2015 at 11:18:27AM +0800, Shannon Zhao wrote:
>> On 2015/9/7 22:23, Leif Lindholm wrote:
>>> The Debug Port Table 2 (DBG2) is mandated by the ARM Server Base Boot
>>> Requirements specification. Add the DBG2 table definitions, and set up
>>> an entry in the ARM virt machine for the pl011 UART.
>>
>> Looking at Documentation/arm64/acpi_object_usage.txt in Linux kernel, it
>> says
>> "
>> DBG2   Signature Reserved (signature == "DBG2")
>>== DeBuG port table 2 ==
>>Microsoft only table, will not be supported.
>> "
>> It seems that Linux kernel doesn't support or need it, but Windows
>> requires it. So does it need to test this on Windows?
> 
> No, it can be tested under Linux with the set I just sent out:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/368614.html

So you need to change Documentation/arm64/acpi_object_usage.txt as well.
But that is kernel side.

> But I needed this set for QEMU for the kernel people to have something
> to test :)
> 
> That said, I would be overjoyed if someone _could_ test it on Windows.
> 
> /
> Leif
> 
>>> Leif Lindholm (2):
>>>   ACPI: Add definitions for the DBG2 table
>>>   hw/arm/virt-acpi-build: Add DBG2 table
>>>
>>>  hw/arm/virt-acpi-build.c| 37 -
>>>  include/hw/acpi/acpi-defs.h | 37 +++--
>>>  2 files changed, 71 insertions(+), 3 deletions(-)
>>>
>>
>> -- 
>> Shannon
>>
> 
> 

-- 
Shannon

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[David kiarie]

On Wed, Sep 9, 2015 at 12:35 AM, Jan Kiszka  wrote:
> [thanks for forwarding, Peter]
>
> Hi Rita,
>
> On 2015-09-08 10:11, Peter Maydell wrote:
>> On 7 September 2015 at 22:31, Rita Sinha  wrote:
>>> Hi Jan,
>>>
>>> I am interested in participating in next round of Outreachy program
>>> with AMD IOMMU emulation project.
>>>
>>>
>>> I have worked on BIOS projects which includes coreboot SeaBios etc and
>>> bootloaders like u-boot and grub. I have experience of working with
>>> qemu and feel that this project is the right match for my skillset.
>>> Kindly guide me how to go ahead with this.
>
> The particular AMD IOMMU project moved on since we listed it. I'm CC'ing
> David, who is currently working on it and just recently posted related
> patches, and Valentine who probably oversees the status better than I
> (due to my lacking involvement recently). David, maybe you can briefly
> comment on status and plans of your work.

Hi all,

Most recent work is here
http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg02759.html
. Most the code is Qemu device boilerplate(so there are a ton of
things to add but I wanted to have the existing work merged first).
The IOMMU just offers basic translation. From Valentine's previous
comments, I only have a few minor issues to fix in the code.

I obviously do other things alongside :-D this project but given some
time I could get the code merged and continue to add other features.

>
> For the Outreachy program, just like for GSoC, we need to find a good
> topic that is sufficiently clear defined on program start and not worked
> on in parallel during the runtime. There are still a number of open
> topics in this area, e.g. around the older Intel IOMMU model (error
> handling and reporting, interrupt remapping), or maybe we find something
> different - depends on your interests and experiences. Do you have any
> public references to your previous work?
>
> Then I'd suggest to schedule an irc meeting to discuss your interests
> and background a bit further and consider available options.
>
> Jan
>

Re: [Qemu-devel] [PATCH v2] spapr_drc: don't allow 'empty' DRCs to be unisolated

[Bharata B Rao]

On Tue, Sep 08, 2015 at 06:44:55PM -0500, Michael Roth wrote:
> Logical resources start with allocation-state:UNUSABLE /
> isolation-state:ISOLATED. During hotplug, guests will transition
> them to allocate-state:USABLE, and then to isolate-state:UNISOLATED.
> The former transition does not seem to have any failure path for
> cases where a DRC does not have any resources associated with it to
> allocate for guest, but instead relies on the subsequent
> isolation-state:UNISOLATED transition to indicate failure in this
> situation.
> 
> Currently DRC code does not implement this logic, but instead
> tries to indicate failure by refusing the allocation-state:USABLE
> transition. Unfortunately, since that's not a documented failure
> path, guests continue undeterred, causing undefined behavior in
> QEMU and guest code.
> 
> Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).
> 
> Cc: qemu-...@nongnu.org
> Cc: David Gibson 
> Cc: Bharata B Rao 
> Signed-off-by: Michael Roth 

Tested-by: Bharata B Rao 

Re: [Qemu-devel] [FIX PATCH] spapr_drc: Return correct state for logical DR in entity_sense()

[Bharata B Rao]

On Tue, Sep 08, 2015 at 05:03:25PM -0500, Michael Roth wrote:
> Quoting Michael Roth (2015-09-08 16:03:56)
> > Quoting David Gibson (2015-09-07 20:22:50)
> > > On Mon, Sep 07, 2015 at 11:37:04AM +0530, Bharata B Rao wrote:
> > > > When drmgr is run in the guest to add a device for which device_add
> > > > hasn't been issued in QEMU, configure-connector call fails.
> > > > When configure-connector call fails, the guest would release (*)
> > > > the previously acquired DRC by setting back the DRC isolation state
> > > > to ISOLATED and allocation state to UNUSABLE. These calls will be issued
> > > > only if get-sensor-state call returns PRESENT state. However currently 
> > > > for
> > > > a logical DR, entity_sense() would unconditinally return UNUSABLE
> > > > state only. This prevents any subsequent hotplug of the device with
> > > > that DRC.
> > 
> > This seems a little odd. I think we default to ALLOCATION_STATE_UNUSABLE
> > for logical DR, and it's up the guest to transition to USABLE, which
> > probably happens prior to the configure-connector calls. So I think the
> > net effect of this fix is that guest will see these unallocated/unattached
> > resources the same way they would a resource that was actually attached
> > via device_add, and all we're really doing is working around the
> > eventual configuration failure that that will lead to by pretending a
> > resource was actually there.
> > 
> > According to PAPR+ 2.7:
> > 
> > 13.7.3.1 Acquire Logical Resource from Resource Pool:
> > 
> >   If the state is “unusable” the OS issues set-indicator (allocation-state, 
> > usable) to attempt to allocate the re-
> >   source. Similarly, if the state is “available for exchange” the OS issues 
> > set-indicator (allocation-state, ex-
> >   change) to attempt to allocate the resource, and if the state is 
> > “available for recovery” the OS issues
> >   set-indicator (allocation-state, recover) to attempt to allocate the 
> > resource.
> > 
> > and
> > 
> > 13.7 Logical Resource Dynamic Reconfiguration (LRDR):
> > 
> >   The OS may use the get-sensor-state RTAS call with the dr-entity-sense 
> > token to deter-
> > mine if a given drc-index refers to a connector that is currently usable 
> > for DR operations. If the connector is not
> > currently usable the return state is “DR entity unusable” (2). A 
> > set-indicator (isolation state) RTAS call to an unusable
> > connector or (dr-indicator) to any logical resource connector results in a 
> > “No such indicator implemented” return sta-
> > tus.  
> > 
> > So I think maybe the proper fix is to make sure that
> > drc->set_indicator_state() fails with an error that indicates to RTAS to
> > return NO_SENSOR (-3) for cases where we haven't attached a resource
> > to the DRC via device_add.
> 
> Patch incoming:
> 
>   spapr_drc: don't allow 'empty' DRCs to be unisolated
> 
> applies to spapr-next but requires revert of this patch.

Yes, please revert this patch in favour of Michael's.

> 
> Bharata, can you give it a spin with CPU hotplug and see if it fixes the
> issue you hit?

Yes it does, thanks.

Regards,
Bharata.

Re: [Qemu-devel] [PATCHEW] Series failed testing

[Fam Zheng]

On Wed, 09/09 11:39, Patchew Jenkins wrote:
> This series failed automatic testing, see attachment for the full log and 
> below for the tail.
> (If you see a false alarm or anything unclear, please reply and help us 
> improve!)
> 
> Started by an SCM change
> Building in workspace /var/lib/jenkins/jobs/qemu/workspace
>  > git rev-parse --is-inside-work-tree # timeout=10
> Fetching changes from the remote Git repository
>  > git config remote.origin.url https://github.com/famz/qemu-patchew # 
> timeout=10
> Fetching upstream changes from https://github.com/famz/qemu-patchew
>  > git --version # timeout=10
>  > git -c core.askpass=true fetch --tags --progress 
> https://github.com/famz/qemu-patchew +refs/heads/*:refs/remotes/origin/*
> Seen 442 remote branches
> Checking out Revision 6f7c15d485f7f8daf1a3b39df03415b676cd721e 
> (origin/<1441761736-32030-1-git-send-email-da...@gibson.dropbear.id.au>)
>  > git config core.sparsecheckout # timeout=10
>  > git checkout -f 6f7c15d485f7f8daf1a3b39df03415b676cd721e
> Using 'Changelog to branch' strategy.
> No emails were triggered.
> [workspace] $ /bin/sh -xe /tmp/hudson5690229581019177586.sh
> + git log --reverse --oneline origin/master..
> 6f7c15d spapr: Reduce advertised max LUNs for spapr_vscsi
> + sudo docker run -t --net=none --rm -v 
> /var/lib/jenkins/jobs/qemu/workspace:/tmp/qemu-build --name patchew-tester 
> patchew-tester /usr/local/bin/qemu-build.sh /tmp/qemu-build
> Error response from daemon: Conflict. The name "patchew-tester" is already in 
> use by container 4f39610cd778. You have to delete (or rename) that container 
> to be able to reuse that name.

Never mind, the build machine has a problem spawning tester instance. It's
being fixed.

Fam

Re: [Qemu-devel] [PATCH COLO-Frame v9 00/32] COarse-grain LOck-stepping(COLO) Virtual Machines for Non-stop Service (FT)

[zhanghailiang]

Ping...

Hi Juan & Amit,

Could you please help review this series ?
Since it is already comes v9, i really hope to get your feedback on this series 
:)

Thanks,
zhanghailiang

On 2015/9/2 16:22, zhanghailiang wrote:

This is the 9th version of COLO.

Please Note that, this version is very different from the previous versions.
since we have decided to realize proxy in qemu, which based on slirp in qemu.
We dropped all the original colo proxy related part.

It will be a long time for proxy to be ready for merging, so here we extract
the basic periodic checkpoint part that not depend on proxy into this series.
Actually, the 'periodic' mode is also what we want to support in COLO, it is
based on Yang Hongyang's netfilter series. and this mode is very like
MicroCheckpointing and Remus.

You can find the discussion about why & how to realize the colo proxy in qemu
from the follow link:
http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg04069.html

As usual, here is only COLO frame part, you can get the whole codes from github:
https://github.com/coloft/qemu/commits/colo-v2.0-periodic-mode

Compared with previous versions, this version is more easy to test.

Test procedure:
1. Startup qemu
Primary side:
# x86_64-softmmu/qemu-system-x86_64 -enable-kvm -netdev tap,id=bn0 -netfilter 
buffer,id=f0,netdev=bn0,chain=in -device virtio-net-pci,id=net-pci0,netdev=bn0 
-boot c -drive 
if=virtio,id=disk1,driver=quorum,read-pattern=fifo,cache=none,aio=native,children.0.file.filename=/mnt/sdd/pure_IMG/linux/redhat/rhel_6.5_64_2U_ide,children.0.driver=raw
 -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-tablet -monitor 
stdio -S

Secondary side:
# x86_64-softmmu/qemu-system-x86_64 -enable-kvm -netdev tap,id=bn0 -device 
virtio-net-pci,id=net-pci0,netdev=bn0 -drive 
if=none,driver=raw,file=/mnt/sdd/pure_IMG/linux/redhat/rhel_6.5_64_2U_ide,id=colo1,cache=none,aio=native
 -drive 
if=virtio,driver=replication,mode=secondary,throttling.bps-total=7000,file.file.filename=/mnt/ramfs/active_disk.img,file.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.driver=qcow2,file.backing.backing.backing_reference=colo1,file.backing.allow-write-backing-file=on
 -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-table -monitor stdio 
-incoming tcp:0:

2. On Secondary VM's QEMU monitor, issue command
(qemu) nbd_server_start 192.168.2.88:8889
(qemu) nbd_server_add -w colo1

3. On Primary VM's QEMU monitor, issue command:
(qemu) child_add disk1 
child.driver=replication,child.mode=primary,child.file.host=192.168.2.88,child.file.port=8889,child.file.export=colo1,child.file.driver=nbd,child.ignore-errors=on
(qemu) migrate_set_capability colo on
(qemu) migrate tcp:192.168.2.88:

4. After the above steps, you will see, whenever you make changes to PVM, SVM 
will be synced.
You can by issue command "migrate_set_parameter checkpoint-delay 2000"
to change the checkpoint period time.

5. Failover test
You can kill PVM  and run 'colo_lost_heartbeat' in SVM's
monitor at the same time, then SVM will failover and client will not feel this 
change.

COLO is a totally new feature which is still in early stage,
your comments and feedback are warmly welcomed.

TODO:
1. checkpoint based on proxy in qemu
2. The capability of continuous FT

v9:
- Drop colo proxy related part (colo-nic.c file)
- Convert COLO protocol name definition to QAPI
- Smash failover related patch (patch 19/20/23)
- Fix colo exit event according Eric's comments.
- Fix some typos from Eric's comments
- Fix bug 'invalid runstate transition: 'colo' -> 'prelaunch' reported
   by Dave (patch 27)
- Use migrate_set_parameter intead of ecolo-set-checkpoint-period to set
   checkpoint delay time (patch 25)
- Add new patch (patch 29/30) to seperate the process of saving/loading
   device and state during checkpoint. which will reduce the data size
   for sending and also reduce the qsb size used in checkpoint.

Wen Congyang (1):
   COLO: Add block replication into colo process

zhanghailiang (31):
   configure: Add parameter for configure to enable/disable COLO support
   migration: Introduce capability 'colo' to migration
   COLO: migrate colo related info to slave
   migration: Add state records for migration incoming
   migration: Integrate COLO checkpoint process into migration
   migration: Integrate COLO checkpoint process into loadvm
   migration: Rename the'file' member of MigrationState and
 MigrationIncomingState
   COLO/migration: establish a new communication path from destination to
 source
   COLO: Implement colo checkpoint protocol
   COLO: Add a new RunState RUN_STATE_COLO
   QEMUSizedBuffer: Introduce two help functions for qsb
   COLO: Save PVM state to secondary side when do checkpoint
   COLO: Load PVM's dirty pages into SVM's RAM cache temporarily
   COLO: Load VMState into qsb before restore it
   COLO: Flush PVM's cached RAM into SVM's memory
   COLO: synchronize PVM's state to SVM periodically
   COLO fail

Re: [Qemu-devel] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Fam Zheng]

On Wed, 07/29 14:03, Paolo Bonzini wrote:
> 
> 
> On 29/07/2015 13:53, Fam Zheng wrote:
> >> > Yes, though I think you'd end up reverting patches 10 and 11 in the end.
> > We will add outer disable/enable pairs to prevent another threads's aio_poll
> > from sneaking in between bdrv_aio_poll calls, but we needn't obsolete
> > bdrv_aio_poll() because of that - it can be useful by itself. For example
> > bdrv_aio_cancel shouldn't look at ioeventfd, otherwise it could spin for too
> > long on high load.  Does that make sense?
> 
> Did you mean bdrv_drain() (when it is not already surrounded by
> disable/enable pairs in the caller)?  But yes, that makes sense.
> 
> I'm not sure that it makes sense to disable/enable in places such as
> bdrv_pread.  The caller's block, if any, should suffice.  In this sense
> you'd end up reverting large parts of patch 10.
> 
> Then you would have to see how many calls to bdrv_aio_poll are still
> there, and how many can be converted with no semantic change to aio_poll
> (e.g. there's no difference in qemu-img.c), and you'd end up reverting
> patches 9 and 11 too.  But we can look at that later.

Another advantage for bdrv_aio_poll() is, in main loop we will not need
a separate AioContext in changes like:

http://patchwork.ozlabs.org/patch/514968/

Because nested aio_poll will automatically be limited to only process block
layer events. My idea is to eventually let main loop use aio_poll, which means
we would also move chardev onto it. It would be neat to put all fds of the main
thread into a single AioContext.

Fam

Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item

[Programmingkid]

On Sep 8, 2015, at 2:46 PM, Markus Armbruster wrote:

> Programmingkid  writes:
> 
>> On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:
>> 
>>> On 2 September 2015 at 01:56, Programmingkid
>>>  wrote:
 Add "Mount Image File..." and a "Eject Image File" menu items to
 cocoa interface. This patch makes sharing files between the
 host and the guest user-friendly.
 
 The "Mount Image File..." menu item displays a dialog box having the
 user pick an image file to use in QEMU. The image file is setup as
 a USB flash drive. The user can do the equivalent of removing the
 flash drive by selecting the file in the "Eject Image File" submenu.
 
 Signed-off-by: John Arbuckle 
 
 ---
 ui/cocoa.m |  212
 +++-
 1 files changed, 210 insertions(+), 1 deletions(-)
 
 diff --git a/ui/cocoa.m b/ui/cocoa.m
 index 334e6f6..6c0ec18 100644
 --- a/ui/cocoa.m
 +++ b/ui/cocoa.m
 @@ -52,6 +52,9 @@
 #endif
 
 
 
 #define cgrect(nsrect) (*(CGRect *)&(nsrect))
 +#define USB_DISK_ID "USB_DISK"
 +#define EJECT_IMAGE_FILE_TAG 2099
 
 
 
 typedef struct {
int width;
 @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
}
 }
 
 
 
 +/* Sends a command to the monitor console */
 +static void sendMonitorCommand(const char * commandString)
 +{
 +int index;
 +char * consoleName;
 +static QemuConsole *monitor;
 +
 +/* If the monitor console hasn't been found yet */
 +if(!monitor) {
 +index = 0;
 +/* Find the monitor console */
 +while (qemu_console_lookup_by_index(index) != NULL) {
 +consoleName =
 qemu_console_get_label(qemu_console_lookup_by_index(index));
 +if(strstr(consoleName, "monitor")) {
 +monitor = qemu_console_lookup_by_index(index);
 +break;
 +}
 +index++;
 +}
 +}
 +
 +/* If the monitor console was not found */
 +if(!monitor) {
 +NSBeep();
 +QEMU_Alert(@"Failed to find the monitor console!");
 +return;
 +}
 +
 +/* send each letter in the commandString to the monitor */
 +for (index = 0; index < strlen(commandString); index++) {
 +kbd_put_keysym_console(monitor, commandString[index]);
 +}
>>> 
>>> We're doing this by sending a string to the human monitor?
> 
> No way :)
> 
> You should not send a string to a monitor (QMP or HMP) just because you
> can't be bothered to look up the proper C interfaces.

I wouldn't say it like that. I did try to find the functions I needed, but 
didn't succeed. 
Didn't you say yourself you don't want to see gui patches that change other 
code?
My research indicated there would have to be changes to other files if I did 
try to use
the C interface functions.  

For example, the function add_init_drive() in device-hotplug.c looked really 
good. The
problem was that it is a static function. Since you don't want changes made to 
other
files, I decided not to use it. 

> 
>>> That definitely doesn't seem like the right way to do this
>>> (and there might not even be a human monitor to talk to)...
>> 
>> Under what situation is the human monitor not available? 
>> 
>> Would you know what function I should use in place of the commands the
>> patch uses?
> 
> I explained that already for QMP:
> http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg8.html
> 
> The mapping from HMP to the C interfaces can be more complex.  Going
> from QMP to C is easier.

The problem with QMP is that it is so difficult to use. It could be made to be 
more
user-friendly. I will try to use it anyways in my next patch. Thank you.

Re: [Qemu-devel] [PATCH v2 2/2] Add dynamic generation of module_block.h

[Fam Zheng]

On Tue, 09/08 15:53, Marc Marí wrote:
> diff --git a/include/qemu/module_block.h b/include/qemu/module_block.h
> deleted file mode 100644
> index d725db8..000
> --- a/include/qemu/module_block.h
> +++ /dev/null
> @@ -1,90 +0,0 @@
> -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> -/*
> - * QEMU Block Module Infrastructure
> - *
> - * Copyright Red Hat, Inc. 2015
> - *
> - * Authors:
> - *  Marc Mari   
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2.  See
> - * the COPYING file in the top-level directory.
> - *
> - */
> -

Could you reorder the patches so you don't need to add them remove the
generated header?

> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
> new file mode 100755
> index 000..0846362
> --- /dev/null
> +++ b/scripts/modules/module_block.py
> @@ -0,0 +1,134 @@
> +#!/usr/bin/python
> +#
> +# Module information generator
> +#
> +# Copyright Red Hat, Inc. 2015
> +#
> +# Authors:
> +#  Marc Mari 
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
> +
> +from __future__ import print_function
> +import sys
> +import os
> +
> +def get_string_struct(line):
> +data = line.split()
> +
> +# data[0] -> struct element name
> +# data[1] -> =
> +# data[2] -> value
> +
> +return data[2].replace('"', '')[:-1]
> +
> +def add_module(fhader, library, format_name, protocol_name,
> +probe, probe_device):
> +lines = []
> +lines.append('.library_name = "' + library + '",')
> +if format_name != "":
> +lines.append('.format_name = "' + format_name + '",')
> +if protocol_name != "":
> +lines.append('.protocol_name = "' + protocol_name + '",')
> +if probe:
> +lines.append('.has_probe = true,')
> +if probe_device:
> +lines.append('.has_probe_device = true,')
> +
> +text = '\n\t'.join(lines)
> +fheader.write('\n\t{\n\t' + text + '\n\t},')
> +
> +def process_file(fheader, filename):
> +# This parser assumes the coding style rules are being followed
> +with open(filename, "r") as cfile:
> +found_something = False
> +found_start = False
> +library, _ = os.path.splitext(os.path.basename(filename))
> +for line in cfile:
> +if found_start:
> +line = line.replace('\n', '')
> +if line.find(".format_name") != -1:
> +format_name = get_string_struct(line)
> +elif line.find(".protocol_name") != -1:
> +protocol_name = get_string_struct(line)
> +elif line.find(".bdrv_probe") != -1:
> +probe = True
> +elif line.find(".bdrv_probe_device") != -1:
> +probe_device = True
> +elif line == "};":
> +add_module(fheader, library, format_name, protocol_name,
> +probe, probe_device)
> +found_start = False
> +elif line.find("static BlockDriver") != -1:
> +found_something = True
> +found_start = True
> +format_name = ""
> +protocol_name = ""
> +probe = False
> +probe_device = False
> +
> +if not found_something:
> +print("No BlockDriver struct found in " + filename + ". \
> +Is this really a module?", file=sys.stderr)
> +sys.exit(1)
> +
> +def print_top(fheader):
> +fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +/*
> + * QEMU Block Module Infrastructure
> + *
> + * Copyright Red Hat, Inc. 2015
> + *
> + * Authors:
> + *  Marc Mari   
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +''')
> +
> +fheader.write('''#ifndef QEMU_MODULE_BLOCK_H
> +#define QEMU_MODULE_BLOCK_H
> +
> +#include "qemu-common.h"
> +
> +static const struct {
> +const char *format_name;
> +const char *protocol_name;
> +const char *library_name;
> +bool has_probe;
> +bool has_probe_device;
> +} block_driver_modules[] = {''')
> +
> +def print_bottom(fheader):
> +fheader.write('''
> +};
> +
> +#endif
> +''')
> +
> +# First argument: output folder
> +# All other arguments: modules source files (.c)
> +output_dir = sys.argv[1]
> +if not os.path.isdir(output_dir):
> +print("Folder " + output_dir + " does not exist", file=sys.stderr)
> +sys.exit(1)
> +
> +path = output_dir + 'module_block.h'
> +
> +with open(path, 'w') as fheader:
> +print_top(fheader)
> +
> +for filename in sys.argv[2:]:
> +if os.path.isfile(filename):
> +process_file(fheader, filename)
> +else:
> +print("File " + filename + " does not exist.", file=sys.stderr)
> +sys.exit(1)
> +
> +print_bottom(f

Re: [Qemu-devel] [PATCH 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

On Tue, Sep 08, 2015 at 03:22:30PM -0600, Eric Blake wrote:
> On 09/08/2015 01:38 AM, Yuanhan Liu wrote:
> > From: Ouyang Changchun 
> > 
> > This patch is initially based a patch from Nikolay Nikolaev.
> > 
> > Here is the latest version for adding vhost-user multiple queue support,
> > by creating a nc and vhost_net pair for each queue.
> > 
> 
> Reviewing grammar and interface only:

Thanks, and will fix them in next patch.

--yliu
> 
> > +++ b/docs/specs/vhost-user.txt
> > @@ -135,6 +135,19 @@ As older slaves don't support negotiating protocol 
> > features,
> >  a feature bit was dedicated for this purpose:
> >  #define VHOST_USER_F_PROTOCOL_FEATURES 30
> >  
> > +Multiple queue support
> > +---
> > +Multiple queue is treated as a protocal extension, hence the slave has to
> 
> s/protocal/protocol/
> 
> > +implement protocol features first. Multiple queues is supported only when
> > +the protocol feature VHOST_USER_PROTOCOL_F_MQ(bit 0) is set.
> > +
> > +The max # of queues the slave support can be queried with message
> 
> s/#/number/
> s/support/supports/
> 
> > +VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the # of 
> > requested
> 
> s/#/number/
> 
> > +queues is bigger than that.
> > +
> > +As all queues share one connection, the master use a unique index for each
> 
> s/use/uses/
> 
> > +queue in the sent message to identify one specified queue.
> > +
> 
> > +++ b/qapi-schema.json
> > @@ -2480,12 +2480,16 @@
> >  #
> >  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: 
> > false).
> >  #
> > +# @queues: #optional number of queues to be created for multiqueue 
> > vhost-user
> > +#  (default: 1) (Since 2.5)
> > +#
> >  # Since 2.1
> >  ##
> >  { 'struct': 'NetdevVhostUserOptions',
> >'data': {
> >  'chardev':'str',
> > -'*vhostforce':'bool' } }
> > +'*vhostforce':'bool',
> > +'*queues':'int' } }
> 
> Looks okay.
> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

[Qemu-devel] [Bug 1422307] Re: qemu-nbd corrupts files

[Launchpad Bug Tracker]

This bug was fixed in the package qemu - 2.0.0+dfsg-2ubuntu1.18

---
qemu (2.0.0+dfsg-2ubuntu1.18) trusty-proposed; urgency=medium

  * qemu-nbd-fix-vdi-corruption.patch:
qemu-nbd: fix corruption while writing VDI volumes (LP: #1422307)

 -- Pierre Schweitzer   Mon, 17 Aug 2015 11:43:39
+0200

** Changed in: qemu (Ubuntu Trusty)
   Status: Fix Committed => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1422307

Title:
  qemu-nbd corrupts files

Status in QEMU:
  Fix Released
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Trusty:
  Fix Released

Bug description:
  [Impact]
  A race condition in the VDI block driver of Qemu leads to image (and thus 
file system) corruption under certain circumstances.
  This makes Qemu tools usage for VDI formatted images particularly dangerous 
(qemu-img, qemu-nbd).
  The bug fix introduces locks to prevent such race condition.

  
  [Test Case]
  A simple test case was provided in comment #5 
(https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1422307/comments/5):

  $ ./qemu-img create -f vdi test.vdi 2G
  Formatting 'test.vdi', fmt=vdi size=2147483648 static=off
  $ ./qemu-img create -f raw test.raw 2G
  Formatting 'test.raw', fmt=raw size=2147483648
  $ x86_64-softmmu/qemu-system-x86_64 -enable-kvm -drive 
if=virtio,file=blkverify:test.raw:test.vdi,format=raw -drive 
if=virtio,file=data.img,format=raw,format=raw -cdrom ~/tmp/arch.iso -m 512 
-boot d
  blkverify: read sector_num=810976 nb_sectors=256 contents mismatch in sector 
811008

  Operations in the guest:
  $ dd if=/dev/vdb of=/dev/vda
  $ dd if=/dev/vda of=/dev/null

  [Regression Potential]
  In case of bugs affecting the way locks are used, deadlocks could be a 
regression, but they would only affect VDI images.

  
  Original bug report:
  Dear all,

  On Trusty, in certain situations, try to copy files over a qemu-nbd
  mounted file system leads to write errors (and thus, file corruption).

  Here is the last example I tried:
  -> virtual disk is a VDI disk
  -> It has only one partition, in FAT

  Here is my mount process:
  # modprobe nbd max_part=63
  # qemu-nbd -c /dev/nbd0 "virtual_disk.vdi"
  # partprobe /dev/nbd0
  # mount /dev/nbd0p1 /tmp/mnt/

  Partition is properly mounted at that point:
  /dev/nbd0p1 on /tmp/mnt type vfat (rw)

  Now, when I copy a file (rather big, ~28MB):
  # cp file_to_copy /tmp/mnt/ ; sync
  # md5sum /tmp/mnt/file_to_copy
  2efc9f32e4267782b11d63d2f128a363  /tmp/mnt/file_to_copy
  # umount /tmp/mnt
  # mount /dev/nbd0p1 /tmp/mnt/
  # md5sum /tmp/mnt/file_to_copy
  42b0a3bf73f704d03ce301716d7654de  /tmp/mnt/file_to_copy

  The first hash was obviously the right one.

  On a previous attempt I did, I spotted thanks to vbindiff that parts of the 
file were just filed with 0s instead of actual data.
  It will randomly work after several attempts to write.

  Version information:
  # qemu-nbd --version
  qemu-nbd version 0.0.1
  Written by Anthony Liguori.

  Cheers,

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1422307/+subscriptions

[Qemu-devel] [PATCH] spapr: Reduce advertised max LUNs for spapr_vscsi

[David Gibson]

The implementation of the PAPR paravirtual SCSI adapter currently
allows up to 32 LUNs (max_lun == 31).  However the adapter isn't really
designed to support lots of devices - the PowerVM implementation only
ever puts one disk per vSCSI controller.

More specifically, the Linux guest side vscsi driver (the only one we
really care about) is hardcoded to allow a maximum of 8 LUNs.

So, reduce the number of LUNs on the qemu side to 8, so we don't
falsely advertise that more LUNs can work.

Signed-off-by: David Gibson 
---
 hw/scsi/spapr_vscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 891424f..bb1dd6f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -1192,7 +1192,7 @@ static const struct SCSIBusInfo vscsi_scsi_info = {
 .tcq = true,
 .max_channel = 7, /* logical unit addressing format */
 .max_target = 63,
-.max_lun = 31,
+.max_lun = 7,
 
 .transfer_data = vscsi_transfer_data,
 .complete = vscsi_command_complete,
-- 
2.4.3

Re: [Qemu-devel] [PATCH v1 1/1] xlnx-zynqmp: Remove unnecessary brackets around error messages

[Peter Crosthwaite]

On Tue, Sep 8, 2015 at 5:32 PM, Alistair Francis
 wrote:
> The errp and err variable have unnecessary brackets around them,
> so remove the brackets.
>
> Signed-off-by: Alistair Francis 

Reviewed-by: Peter Crosthwaite 

> ---
>
>  hw/arm/xlnx-zynqmp.c |   10 +-
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 2955f3b..2185542 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -128,7 +128,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>  qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", 
> XLNX_ZYNQMP_NUM_APU_CPUS);
>  object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
>  if (err) {
> -error_propagate((errp), (err));
> +error_propagate(errp, err);
>  return;
>  }
>  assert(ARRAY_SIZE(xlnx_zynqmp_gic_regions) == XLNX_ZYNQMP_GIC_REGIONS);
> @@ -173,7 +173,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>  object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
>   &err);
>  if (err) {
> -error_propagate((errp), (err));
> +error_propagate(errp, err);
>  return;
>  }
>
> @@ -206,7 +206,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>  object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "realized",
>   &err);
>  if (err) {
> -error_propagate((errp), (err));
> +error_propagate(errp, err);
>  return;
>  }
>  }
> @@ -229,7 +229,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>  }
>  object_property_set_bool(OBJECT(&s->gem[i]), true, "realized", &err);
>  if (err) {
> -error_propagate((errp), (err));
> +error_propagate(errp, err);
>  return;
>  }
>  sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem[i]), 0, gem_addr[i]);
> @@ -240,7 +240,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>  for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) {
>  object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", 
> &err);
>  if (err) {
> -error_propagate((errp), (err));
> +error_propagate(errp, err);
>  return;
>  }
>  sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, uart_addr[i]);
> --
> 1.7.1
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/2] spapr: Add support for hwrng when available

[Sam Bobroff]

On Tue, Sep 08, 2015 at 07:38:12AM +0200, Thomas Huth wrote:
> On 08/09/15 07:03, Sam Bobroff wrote:
> > On Tue, Sep 01, 2015 at 12:53:26PM +0200, Thomas Huth wrote:
> >> On 01/09/15 02:38, David Gibson wrote:
> >>> On Mon, Aug 31, 2015 at 08:46:01PM +0200, Thomas Huth wrote:
>  From: Michael Ellerman 
> 
>  Some powerpc systems have support for a hardware random number generator
>  (hwrng). If such a hwrng is present the host kernel can provide access
>  to it via the H_RANDOM hcall.
> 
>  The kernel advertises the presence of a hwrng with the KVM_CAP_PPC_HWRNG
>  capability. If this is detected we add the appropriate device tree bits
>  to advertise the presence of the hwrng to the guest kernel.
> 
>  Signed-off-by: Michael Ellerman 
>  [thuth: Refreshed patch so it applies to QEMU master branch]
>  Signed-off-by: Thomas Huth 
> >>>
> >>> So, I'm confused by one thing.
> >>>
> >>> I thought new kernel handled hcalls were supposed to be disabled by
> >>> default, but I don't see any calls to kvmppc_enable_hcall() to turn on
> >>> H_RANDOM.
> >>
> >> Michael's patch was from 2013, the kvmppc_enable_hcall() stuff seems to
> >> be from 2014 ... so the enablement is likely missing in this patch,
> >> indeed. I didn't test the in-kernel hypercall yet, just my QEMU
> >> implementation so far, that's why I did not notice this yet.
> >>
> >> Michael, do you want to rework your patch? Or shall I add an additional
> >> enablement patch to my queue?
> > 
> > If I recall correctly, it's specifically not enabled: there was quite a lot 
> > of
> > discussion about it when Michael posted the patches and I think the 
> > consensus
> > was that it should only be enabled by QEMU, and only if the user could 
> > decide
> > if it was used or not.
> 
> Can you find this discussion in a mailing list archive somewhere? The
> only discussions I've found are basically these:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg04233.html
> https://lkml.org/lkml/fancy/2013/10/1/49
> 
> ... and there it was only discussed that the call should be implemented
> in QEMU, too. I did not spot any discussion about making it switchable
> for the user?

Sorry that part wasn't very well worded: I was trying to say that QEMU's
configuration should be able to choose how H_RANDOM handled, rather than having
it automatically choose based on what was available in KVM and that's what
we're considering now anyway. (See David's comment elsewhere in this thread.)

Sam.

[Qemu-devel] [PATCH v3 3/3] i.MX: Add GPIO devices to i.MX25 SOC

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois 
Reviewed-by: Peter Crosthwaite 
---

Changes since v1:
  * various coding style cleanup
  
Changes since v2:
  * no change.

 hw/arm/fsl-imx25.c | 29 +
 include/hw/arm/fsl-imx25.h | 15 +++
 2 files changed, 44 insertions(+)

diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 6d157c9..86fde42 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -63,6 +63,11 @@ static void fsl_imx25_init(Object *obj)
 object_initialize(&s->i2c[i], sizeof(s->i2c[i]), TYPE_IMX_I2C);
 qdev_set_parent_bus(DEVICE(&s->i2c[i]), sysbus_get_default());
 }
+
+for (i = 0; i < FSL_IMX25_NUM_GPIOS; i++) {
+object_initialize(&s->gpio[i], sizeof(s->gpio[i]), TYPE_IMX_GPIO);
+qdev_set_parent_bus(DEVICE(&s->gpio[i]), sysbus_get_default());
+}
 }
 
 static void fsl_imx25_realize(DeviceState *dev, Error **errp)
@@ -214,6 +219,30 @@ static void fsl_imx25_realize(DeviceState *dev, Error 
**errp)
 i2c_table[i].irq));
 }
 
+/* Initialize all GPIOs */
+for (i = 0; i < FSL_IMX25_NUM_GPIOS; i++) {
+static const struct {
+hwaddr addr;
+unsigned int irq;
+} gpio_table[FSL_IMX25_NUM_GPIOS] = {
+{ FSL_IMX25_GPIO1_ADDR, FSL_IMX25_GPIO1_IRQ },
+{ FSL_IMX25_GPIO2_ADDR, FSL_IMX25_GPIO2_IRQ },
+{ FSL_IMX25_GPIO3_ADDR, FSL_IMX25_GPIO3_IRQ },
+{ FSL_IMX25_GPIO4_ADDR, FSL_IMX25_GPIO4_IRQ }
+};
+
+object_property_set_bool(OBJECT(&s->gpio[i]), true, "realized", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0, gpio_table[i].addr);
+/* Connect GPIO IRQ to PIC */
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+   qdev_get_gpio_in(DEVICE(&s->avic),
+gpio_table[i].irq));
+}
+
 /* initialize 2 x 16 KB ROM */
 memory_region_init_rom_device(&s->rom[0], NULL, NULL, NULL,
   "imx25.rom0", FSL_IMX25_ROM0_SIZE, &err);
diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h
index 7f6bb64..73f50c6 100644
--- a/include/hw/arm/fsl-imx25.h
+++ b/include/hw/arm/fsl-imx25.h
@@ -25,6 +25,7 @@
 #include "hw/timer/imx_epit.h"
 #include "hw/net/imx_fec.h"
 #include "hw/i2c/imx_i2c.h"
+#include "hw/gpio/imx_gpio.h"
 #include "exec/memory.h"
 
 #define TYPE_FSL_IMX25 "fsl,imx25"
@@ -34,6 +35,7 @@
 #define FSL_IMX25_NUM_GPTS 4
 #define FSL_IMX25_NUM_EPITS 2
 #define FSL_IMX25_NUM_I2CS 3
+#define FSL_IMX25_NUM_GPIOS 4
 
 typedef struct FslIMX25State {
 /*< private >*/
@@ -48,6 +50,7 @@ typedef struct FslIMX25State {
 IMXEPITState   epit[FSL_IMX25_NUM_EPITS];
 IMXFECStatefec;
 IMXI2CStatei2c[FSL_IMX25_NUM_I2CS];
+IMXGPIOState   gpio[FSL_IMX25_NUM_GPIOS];
 MemoryRegion   rom[2];
 MemoryRegion   iram;
 MemoryRegion   iram_alias;
@@ -204,6 +207,14 @@ typedef struct FslIMX25State {
 #define FSL_IMX25_EPIT1_SIZE0x4000
 #define FSL_IMX25_EPIT2_ADDR0x53F98000
 #define FSL_IMX25_EPIT2_SIZE0x4000
+#define FSL_IMX25_GPIO4_ADDR0x53F9C000
+#define FSL_IMX25_GPIO4_SIZE0x4000
+#define FSL_IMX25_GPIO3_ADDR0x53FA4000
+#define FSL_IMX25_GPIO3_SIZE0x4000
+#define FSL_IMX25_GPIO1_ADDR0x53FCC000
+#define FSL_IMX25_GPIO1_SIZE0x4000
+#define FSL_IMX25_GPIO2_ADDR0x53FD
+#define FSL_IMX25_GPIO2_SIZE0x4000
 #define FSL_IMX25_AVIC_ADDR 0x6800
 #define FSL_IMX25_AVIC_SIZE 0x4000
 #define FSL_IMX25_IRAM_ADDR 0x7800
@@ -230,5 +241,9 @@ typedef struct FslIMX25State {
 #define FSL_IMX25_I2C1_IRQ  3
 #define FSL_IMX25_I2C2_IRQ  4
 #define FSL_IMX25_I2C3_IRQ  10
+#define FSL_IMX25_GPIO1_IRQ 52
+#define FSL_IMX25_GPIO2_IRQ 51
+#define FSL_IMX25_GPIO3_IRQ 16
+#define FSL_IMX25_GPIO4_IRQ 23
 
 #endif /* FSL_IMX25_H */
-- 
2.1.4

[Qemu-devel] [PATCH v3 1/3] i.MX: Add GPIO device

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
  * added "has-edge-sel" property 
  * use extract64() and deposit64() in read/write icr access
  * set "number of GPIO pin" as a #define
  * reorganize functions in file to group them
  * various coding style cleanup
  * rename state handler array in output array.
 
Changes since v2: 
  * always compile DEBUG functions 
  * Fix imx_gpio_update_int to use isr, imr and gdir
  * Fix imx_gpio_set_int_line to check gdir instead of imr
  * Fix imx_gpio_set to update psr even if line is output
  * Fix PSR read to use gdir.

 hw/gpio/Makefile.objs  |   1 +
 hw/gpio/imx_gpio.c | 340 +
 include/hw/gpio/imx_gpio.h |  63 +
 3 files changed, 404 insertions(+)
 create mode 100644 hw/gpio/imx_gpio.c
 create mode 100644 include/hw/gpio/imx_gpio.h

diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs
index 1abcf17..52233f7 100644
--- a/hw/gpio/Makefile.objs
+++ b/hw/gpio/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-$(CONFIG_ZAURUS) += zaurus.o
 common-obj-$(CONFIG_E500) += mpc8xxx.o
 
 obj-$(CONFIG_OMAP) += omap_gpio.o
+obj-$(CONFIG_IMX) += imx_gpio.o
diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
new file mode 100644
index 000..f8d7ef8
--- /dev/null
+++ b/hw/gpio/imx_gpio.c
@@ -0,0 +1,340 @@
+/*
+ * i.MX processors GPIO emulation.
+ *
+ * Copyright (C) 2015 Jean-Christophe Dubois 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/gpio/imx_gpio.h"
+
+#ifndef DEBUG_IMX_GPIO
+#define DEBUG_IMX_GPIO 0
+#endif
+
+typedef enum IMXGPIOLevel {
+IMX_GPIO_LEVEL_LOW = 0,
+IMX_GPIO_LEVEL_HIGH = 1,
+} IMXGPIOLevel;
+
+#define DPRINTF(fmt, args...) \
+  do { \
+  if (DEBUG_IMX_GPIO) { \
+  fprintf(stderr, "%s: " fmt , __func__, ##args); \
+  } \
+  } while (0)
+
+static const char *imx_gpio_reg_name(uint32_t reg)
+{
+switch (reg) {
+case DR_ADDR:
+return "DR";
+case GDIR_ADDR:
+return "GDIR";
+case PSR_ADDR:
+return "PSR";
+case ICR1_ADDR:
+return "ICR1";
+case ICR2_ADDR:
+return "ICR2";
+case IMR_ADDR:
+return "IMR";
+case ISR_ADDR:
+return "ISR";
+case EDGE_SEL_ADDR:
+return "EDGE_SEL";
+default:
+return "[?]";
+}
+}
+
+static void imx_gpio_update_int(IMXGPIOState *s)
+{
+qemu_set_irq(s->irq, (s->isr & s->imr & ~s->gdir) ? 1 : 0);
+}
+
+static void imx_gpio_set_int_line(IMXGPIOState *s, int line, IMXGPIOLevel 
level)
+{
+/* if this signal isn't configured as an input signal, nothing to do */
+if (!extract32(s->gdir, line, 1)) {
+return;
+}
+
+/* When set, EDGE_SEL overrides the ICR config */
+if (extract32(s->edge_sel, line, 1)) {
+/* we detect interrupt on rising and falling edge */
+if (extract32(s->psr, line, 1) != level) {
+/* level changed */
+s->isr = deposit32(s->isr, line, 1, 1);
+}
+} else if (extract64(s->icr, 2*line + 1, 1)) {
+/* interrupt is edge sensitive */
+if (extract32(s->psr, line, 1) != level) {
+/* level changed */
+if (extract64(s->icr, 2*line, 1) != level) {
+s->isr = deposit32(s->isr, line, 1, 1);
+}
+}
+} else {
+/* interrupt is level sensitive */
+if (extract64(s->icr, 2*line, 1) == level) {
+s->isr = deposit32(s->isr, line, 1, 1);
+}
+}
+}
+
+static void imx_gpio_set(void *opaque, int line, int level)
+{
+IMXGPIOState *s = IMX_GPIO(opaque);
+IMXGPIOLevel imx_level = level ? IMX_GPIO_LEVEL_HIGH : IMX_GPIO_LEVEL_LOW;
+
+imx_gpio_set_int_line(s, line, imx_level);
+
+/* this is an input signal, so set PSR */
+s->psr = deposit32(s->psr, line, 1, imx_level);
+
+imx_gpio_update_int(s);
+}
+
+static void imx_gpio_set_all_int_lines(IMXGPIOState *s)
+{
+int i;
+
+for (i = 0; i < IMX_GPIO_PIN_COUNT; i++) {
+IMXGPIOLevel imx_level = extract32(s->psr, i, 1);
+imx_gpio_set_int_line(s, i, imx_level);
+}
+
+imx_gpio_update_int(s);
+}
+
+static inline void imx_gpio_set_all_output_lines(IMXGPIOState *s)
+{
+int i;
+
+for (i = 0; i < IMX_GPIO_PIN_COUNT; i++) {
+/*
+ * if the line is set as output, then forward the line
+   

[Qemu-devel] [PATCH v3 2/3] i.MX: Add GPIO devices to i.MX31 SOC

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois 
Reviewed-by: Peter Crosthwaite 
---

Changes since v1:
  * Use "has-edge-sel" property for i.MX31
  * various coding style cleanup

Changes since v2:
  * failing to set "has-edge-sel" is fatal.

 hw/arm/fsl-imx31.c | 30 ++
 include/hw/arm/fsl-imx31.h | 12 
 2 files changed, 42 insertions(+)

diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 87548c8..8e1ed48 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -55,6 +55,11 @@ static void fsl_imx31_init(Object *obj)
 object_initialize(&s->i2c[i], sizeof(s->i2c[i]), TYPE_IMX_I2C);
 qdev_set_parent_bus(DEVICE(&s->i2c[i]), sysbus_get_default());
 }
+
+for (i = 0; i < FSL_IMX31_NUM_GPIOS; i++) {
+object_initialize(&s->gpio[i], sizeof(s->gpio[i]), TYPE_IMX_GPIO);
+qdev_set_parent_bus(DEVICE(&s->gpio[i]), sysbus_get_default());
+}
 }
 
 static void fsl_imx31_realize(DeviceState *dev, Error **errp)
@@ -184,6 +189,31 @@ static void fsl_imx31_realize(DeviceState *dev, Error 
**errp)
 i2c_table[i].irq));
 }
 
+/* Initialize all GPIOs */
+for (i = 0; i < FSL_IMX31_NUM_GPIOS; i++) {
+static const struct {
+hwaddr addr;
+unsigned int irq;
+} gpio_table[FSL_IMX31_NUM_GPIOS] = {
+{ FSL_IMX31_GPIO1_ADDR, FSL_IMX31_GPIO1_IRQ },
+{ FSL_IMX31_GPIO2_ADDR, FSL_IMX31_GPIO2_IRQ },
+{ FSL_IMX31_GPIO3_ADDR, FSL_IMX31_GPIO3_IRQ }
+};
+
+object_property_set_bool(OBJECT(&s->gpio[i]), false, "has-edge-sel",
+ &error_abort);
+object_property_set_bool(OBJECT(&s->gpio[i]), true, "realized", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0, gpio_table[i].addr);
+/* Connect GPIO IRQ to PIC */
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+   qdev_get_gpio_in(DEVICE(&s->avic),
+gpio_table[i].irq));
+}
+
 /* On a real system, the first 16k is a `secure boot rom' */
 memory_region_init_rom_device(&s->secure_rom, NULL, NULL, NULL,
   "imx31.secure_rom",
diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h
index 891166f..5e8f795 100644
--- a/include/hw/arm/fsl-imx31.h
+++ b/include/hw/arm/fsl-imx31.h
@@ -24,6 +24,7 @@
 #include "hw/timer/imx_gpt.h"
 #include "hw/timer/imx_epit.h"
 #include "hw/i2c/imx_i2c.h"
+#include "hw/gpio/imx_gpio.h"
 #include "exec/memory.h"
 
 #define TYPE_FSL_IMX31 "fsl,imx31"
@@ -32,6 +33,7 @@
 #define FSL_IMX31_NUM_UARTS 2
 #define FSL_IMX31_NUM_EPITS 2
 #define FSL_IMX31_NUM_I2CS 3
+#define FSL_IMX31_NUM_GPIOS 3
 
 typedef struct FslIMX31State {
 /*< private >*/
@@ -45,6 +47,7 @@ typedef struct FslIMX31State {
 IMXGPTStategpt;
 IMXEPITState   epit[FSL_IMX31_NUM_EPITS];
 IMXI2CStatei2c[FSL_IMX31_NUM_I2CS];
+IMXGPIOState   gpio[FSL_IMX31_NUM_GPIOS];
 MemoryRegion   secure_rom;
 MemoryRegion   rom;
 MemoryRegion   iram;
@@ -77,6 +80,12 @@ typedef struct FslIMX31State {
 #define FSL_IMX31_EPIT1_SIZE0x4000
 #define FSL_IMX31_EPIT2_ADDR0x53F98000
 #define FSL_IMX31_EPIT2_SIZE0x4000
+#define FSL_IMX31_GPIO3_ADDR0x53FA4000
+#define FSL_IMX31_GPIO3_SIZE0x4000
+#define FSL_IMX31_GPIO1_ADDR0x53FCC000
+#define FSL_IMX31_GPIO1_SIZE0x4000
+#define FSL_IMX31_GPIO2_ADDR0x53FD
+#define FSL_IMX31_GPIO2_SIZE0x4000
 #define FSL_IMX31_AVIC_ADDR 0x6800
 #define FSL_IMX31_AVIC_SIZE 0x100
 #define FSL_IMX31_SDRAM0_ADDR   0x8000
@@ -106,5 +115,8 @@ typedef struct FslIMX31State {
 #define FSL_IMX31_I2C1_IRQ  10
 #define FSL_IMX31_I2C2_IRQ  4
 #define FSL_IMX31_I2C3_IRQ  3
+#define FSL_IMX31_GPIO1_IRQ 52
+#define FSL_IMX31_GPIO2_IRQ 51
+#define FSL_IMX31_GPIO3_IRQ 56
 
 #endif /* FSL_IMX31_H */
-- 
2.1.4

[Qemu-devel] [PATCH v3 0/3] i.MX: Add GPIO devices to i.MX SOC

[Jean-Christophe Dubois]

Add GPIO devices to i.MX31 and i.MX25 SOC

Jean-Christophe Dubois (3):
  i.MX: Add GPIO device
  i.MX: Add GPIO devices to i.MX31 SOC
  i.MX: Add GPIO devices to i.MX25 SOC

 hw/arm/fsl-imx25.c |  29 
 hw/arm/fsl-imx31.c |  30 
 hw/gpio/Makefile.objs  |   1 +
 hw/gpio/imx_gpio.c | 340 +
 include/hw/arm/fsl-imx25.h |  15 ++
 include/hw/arm/fsl-imx31.h |  12 ++
 include/hw/gpio/imx_gpio.h |  63 +
 7 files changed, 490 insertions(+)
 create mode 100644 hw/gpio/imx_gpio.c
 create mode 100644 include/hw/gpio/imx_gpio.h

-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 1/3] i.MX: Add GPIO device

[Jean-Christophe DUBOIS]

Le 07/09/2015 04:01, Peter Crosthwaite a écrit :

On Sun, Sep 6, 2015 at 1:45 PM, Jean-Christophe Dubois
 wrote:

Signed-off-by: Jean-Christophe Dubois 
---

Changes since V1:
   * added "has-edge-sel" property
   * use extract64() and deposit64() in read/write icr access
   * set "number of GPIO pin" as a #define
   * reorganize functions in file to group them
   * various coding style cleanup
   * rename state handler array in output array.

  hw/gpio/Makefile.objs  |   1 +
  hw/gpio/imx_gpio.c | 347 +
  include/hw/gpio/imx_gpio.h |  63 
  3 files changed, 411 insertions(+)
  create mode 100644 hw/gpio/imx_gpio.c
  create mode 100644 include/hw/gpio/imx_gpio.h

diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs
index 1abcf17..52233f7 100644
--- a/hw/gpio/Makefile.objs
+++ b/hw/gpio/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-$(CONFIG_ZAURUS) += zaurus.o
  common-obj-$(CONFIG_E500) += mpc8xxx.o

  obj-$(CONFIG_OMAP) += omap_gpio.o
+obj-$(CONFIG_IMX) += imx_gpio.o
diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
new file mode 100644
index 000..fc43c73
--- /dev/null
+++ b/hw/gpio/imx_gpio.c
@@ -0,0 +1,347 @@
+/*
+ * i.MX processors GPIO emulation.
+ *
+ * Copyright (C) 2015 Jean-Christophe Dubois 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/gpio/imx_gpio.h"
+

Cutting from here [1] ...


+#ifdef DEBUG_GPIO

This needs a less generic name, such as DEBUG_IMX_GPIO. This allows
defining at configure as a -D CFLAG without other consequences.

OK



+#  define DPRINTF(fmt, args...) \
+  do { fprintf(stder, "%s: " fmt , __func__, ##args); } while (0)

stderr.

OK (it is caught by the "always compile" scheme now)



+
+static char const *imx_gpio_reg_name(uint32_t reg)

const char *. Just for consistency with code base. A grep of "const
char \*" suggests this is the norm. We could add a CODING_STYLE.

OK



+{
+switch (reg) {
+case DR_ADDR:
+return "DR";
+case GDIR_ADDR:
+return "GDIR";
+case PSR_ADDR:
+return "PSR";
+case ICR1_ADDR:
+return "ICR1";
+case ICR2_ADDR:
+return "ICR2";
+case IMR_ADDR:
+return "IMR";
+case ISR_ADDR:
+return "ISR";
+case EDGE_SEL_ADDR:
+/* This register is not present in i.MX31 */
+return "EDGE_SEL";
+default:
+return "[?]";
+}
+}
+#else /* DEBUG_GPIO */
+#  define DPRINTF(fmt, args...) do {} while (0)
+#endif /* DEBUG_GPIO */

[1] ... to here. I would try something like:

#ifndef DEBUG_GPIO
#define DEBUG_GPIO 1
#endif

#define DPRINTF(fmt, args...) \
   do { fprintf(stder, "%s: " fmt , __func__, ##args); } while (0)

static char const *imx_gpio_reg_name(uint32_t reg)
{
 switch (reg) {
 case DR_ADDR:
 return "DR";
...
 return "EDGE_SEL";
 default:
 return "[?]";
 }
}

No, #else needed. As the DPRINTF bodies are always compiled you also
need (and want) always-compilation of gpio_reg_name and without the
unused static warn.

OK



+
+typedef enum IMXGPIOLevel {
+IMX_GPIO_LEVEL_LOW = 0,
+IMX_GPIO_LEVEL_HIGH = 1,
+} IMXGPIOLevel;
+
+static void imx_gpio_update_int(IMXGPIOState *s)
+{
+qemu_set_irq(s->irq, s->isr ? 1 : 0);
+}
+
+static void imx_gpio_set_int_line(IMXGPIOState *s, int line, IMXGPIOLevel 
level)
+{
+/* if this signal isn't configured as an interrupt source, nothing to do */
+if (!extract32(s->imr, line, 1)) {
+return;
+}

I had a read up of the TRM and found this:

"
Interrupt Status bits. Bit i of this register is asserted (active
high) when the active condition is detected on the GPIO
input and is waiting for service. The value of this register is
independent of the value in the IMR register. When the
active condition has been detected, the corresponding bit remains set
until cleared by software.
"

IMR does not affect ISR, IMR only bitwise gates the top level
interrupt signalling. ISR is unaffected. I think this short return
should be dropped IIUC.


OK, then I have to also use IMR when raising interrupts in 
imx_gpio_update_int()



+
+/* When set, EDGE_SEL override the ICR config */

"overrides"

OK



+if (extract32(s->edge_sel, line, 1)) {
+/* we detect interrupt on rising and falling edge */
+if (extract32(s->psr, line, 1) != level) {
+  

[Qemu-devel] [PATCH v1 1/1] xlnx-zynqmp: Remove unnecessary brackets around error messages

[Alistair Francis]

The errp and err variable have unnecessary brackets around them,
so remove the brackets.

Signed-off-by: Alistair Francis 
---

 hw/arm/xlnx-zynqmp.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 2955f3b..2185542 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -128,7 +128,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
 object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
 if (err) {
-error_propagate((errp), (err));
+error_propagate(errp, err);
 return;
 }
 assert(ARRAY_SIZE(xlnx_zynqmp_gic_regions) == XLNX_ZYNQMP_GIC_REGIONS);
@@ -173,7 +173,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
  &err);
 if (err) {
-error_propagate((errp), (err));
+error_propagate(errp, err);
 return;
 }
 
@@ -206,7 +206,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "realized",
  &err);
 if (err) {
-error_propagate((errp), (err));
+error_propagate(errp, err);
 return;
 }
 }
@@ -229,7 +229,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 }
 object_property_set_bool(OBJECT(&s->gem[i]), true, "realized", &err);
 if (err) {
-error_propagate((errp), (err));
+error_propagate(errp, err);
 return;
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem[i]), 0, gem_addr[i]);
@@ -240,7 +240,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) {
 object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
 if (err) {
-error_propagate((errp), (err));
+error_propagate(errp, err);
 return;
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, uart_addr[i]);
-- 
1.7.1

Re: [Qemu-devel] [PATCH RESEND v6 0/4] xlnx-zynqmp: Connect the AHCI SATA device

[Alistair Francis]

On Tue, Sep 8, 2015 at 8:00 AM, Peter Maydell  wrote:
> On 4 September 2015 at 16:17, Alistair Francis
>  wrote:
>> This series connects the AHCI SATA device to the ZynqMP
>> machine. It requires a restructure of the AHCI file to
>> make the AHCI state struct visible. It also requires a
>> small change to the ahci_irq_lower() and ahci_irq_raise()
>> functions to avoid assuming that the AHCIState is a child
>> of AHCIPCIState.
>
> No idea why the list doesn't like patch 4, but anyway I've
> applied the series to target-arm.next.

Thanks for that Peter.

>
> PS: You should send a patch to fix all the occurrences
> of "error_propagate((errp), (err));"  in xlnx-zynqmp.c...

Thanks for pointing that out. I'm working on a patch now.

Thanks,

Alistair

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated

[Michael Roth]

Quoting Michael Roth (2015-09-08 17:03:59)
> Logical resources start with allocation-state:UNUSABLE /
> isolation-state:ISOLATED. During hotplug, guests will transition
> them to allocate-state:USABLE, and then to isolate-state:UNISOLATED.
> The former transition does not seem to have any failure path for
> cases where a DRC does not have any resources associated with it to
> allocate for guest, but instead relies on the subsequent
> isolation-state:UNISOLATED transition to indicate failure in this
> situation.
> 
> Currently DRC code does not implement this logic, but instead
> tries to indicate failure by refusing the allocation-state:USABLE
> transition. Unfortunately, since that's not a documented failure
> path, guests continue undeterred, causing undefined behavior in
> QEMU and guest code.
> 
> Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).
> 
> Cc: qemu-...@nongnu.org
> Cc: David Gibson 
> Cc: Bharata B Rao 
> Signed-off-by: Michael Roth 

Argh, please ignore. This patch is missing the full changeset. v2 sent.

> ---
>  hw/ppc/spapr_drc.c | 12 
>  include/hw/ppc/spapr_drc.h |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 9ce844a..c1f664f 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -66,6 +66,18 @@ static int set_isolation_state(sPAPRDRConnector *drc,
> 
>  DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
> 
> +if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
> +/* cannot unisolate a non-existant resource. this generally
> + * happens for logical resources where transitions from
> + * allocation-state:UNUSABLE to allocation-state:USABLE are
> + * unguarded, but instead rely on a subsequent
> + * isolation-state:UNISOLATED transition to indicate failure
> + */
> +if (!drc->dev) {
> +return -1;
> +}
> +}
> +
>  drc->isolation_state = state;
> 
>  if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 28ffeae..3fbe9ea 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -109,6 +109,7 @@ typedef enum {
>   * unusable: device not currently available to OS
>   * exchange: (currently unused)
>   * recover: (currently unused)
> + * no_sensor: for logical DR only, returned when no resource available
>   */
>  typedef enum {
>  SPAPR_DR_ENTITY_SENSE_EMPTY = 0,
> @@ -116,6 +117,7 @@ typedef enum {
>  SPAPR_DR_ENTITY_SENSE_UNUSABLE  = 2,
>  SPAPR_DR_ENTITY_SENSE_EXCHANGE  = 3,
>  SPAPR_DR_ENTITY_SENSE_RECOVER   = 4,
> +SPAPR_DR_ENTITY_SENSE_NO_SENSOR = -3,
>  } sPAPRDREntitySense;
> 
>  typedef enum {
> -- 
> 1.9.1
> 

[Qemu-devel] [PATCH v2] spapr_drc: don't allow 'empty' DRCs to be unisolated

[Michael Roth]

Logical resources start with allocation-state:UNUSABLE /
isolation-state:ISOLATED. During hotplug, guests will transition
them to allocate-state:USABLE, and then to isolate-state:UNISOLATED.
The former transition does not seem to have any failure path for
cases where a DRC does not have any resources associated with it to
allocate for guest, but instead relies on the subsequent
isolation-state:UNISOLATED transition to indicate failure in this
situation.

Currently DRC code does not implement this logic, but instead
tries to indicate failure by refusing the allocation-state:USABLE
transition. Unfortunately, since that's not a documented failure
path, guests continue undeterred, causing undefined behavior in
QEMU and guest code.

Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).

Cc: qemu-...@nongnu.org
Cc: David Gibson 
Cc: Bharata B Rao 
Signed-off-by: Michael Roth 
---
v2:
 - actually include the full changeset in the patch
---
 hw/ppc/spapr_drc.c | 12 
 hw/ppc/spapr_rtas.c|  9 +++--
 include/hw/ppc/spapr.h |  1 +
 include/hw/ppc/spapr_drc.h |  2 ++
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 9ce844a..c1f664f 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -66,6 +66,18 @@ static int set_isolation_state(sPAPRDRConnector *drc,
 
 DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
 
+if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
+/* cannot unisolate a non-existant resource. this generally
+ * happens for logical resources where transitions from
+ * allocation-state:UNUSABLE to allocation-state:USABLE are
+ * unguarded, but instead rely on a subsequent
+ * isolation-state:UNISOLATED transition to indicate failure
+ */
+if (!drc->dev) {
+return -1;
+}
+}
+
 drc->isolation_state = state;
 
 if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 3b7b20b..0ddedca 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -372,6 +372,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 uint32_t sensor_type;
 uint32_t sensor_index;
 uint32_t sensor_state;
+int drc_ret, ret = RTAS_OUT_SUCCESS;
 sPAPRDRConnector *drc;
 sPAPRDRConnectorClass *drck;
 
@@ -413,7 +414,11 @@ static void rtas_set_indicator(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 spapr_ccs_remove(spapr, ccs);
 }
 }
-drck->set_isolation_state(drc, sensor_state);
+drc_ret = drck->set_isolation_state(drc, sensor_state);
+if (drc_ret != 0) {
+ret = (drc_ret == -1) ? RTAS_OUT_NO_SUCH_INDICATOR
+  : RTAS_OUT_HW_ERROR;
+}
 break;
 case RTAS_SENSOR_TYPE_DR:
 drck->set_indicator_state(drc, sensor_state);
@@ -425,7 +430,7 @@ static void rtas_set_indicator(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 goto out_unimplemented;
 }
 
-rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+rtas_st(rets, 0, ret);
 return;
 
 out_unimplemented:
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index c75cc5e..ffb108d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -412,6 +412,7 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_OUT_BUSY   -2
 #define RTAS_OUT_PARAM_ERROR-3
 #define RTAS_OUT_NOT_SUPPORTED  -3
+#define RTAS_OUT_NO_SUCH_INDICATOR  -3
 #define RTAS_OUT_NOT_AUTHORIZED -9002
 
 /* RTAS tokens */
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 28ffeae..b2c1209 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -165,6 +165,8 @@ typedef struct sPAPRDRConnectorClass {
 /*< public >*/
 
 /* accessors for guest-visible (generally via RTAS) DR state */
+
+/* returns -1 if DRC cannot be set to requested isolation state */
 int (*set_isolation_state)(sPAPRDRConnector *drc,
sPAPRDRIsolationState state);
 int (*set_indicator_state)(sPAPRDRConnector *drc,
-- 
1.9.1

[Qemu-devel] [PATCH RFC 13/14] qemu-char: Add qemu_chr_disconnect to close a fd accepted by listen fd

[marcandre . lureau]

From: Tetsuya Mukawa 

The patch introduces qemu_chr_disconnect(). The function is used for
closing a fd accepted by listen fd. Though we already have qemu_chr_delete(),
but it closes not only accepted fd but also listen fd. This new function
is used when we still want to keep listen fd.

Signed-off-by: Tetsuya Mukawa 
Reviewed-by: Marc-André Lureau 
---
 include/sysemu/char.h | 7 +++
 qemu-char.c   | 8 
 2 files changed, 15 insertions(+)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5fd0a09..9f84f95 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -70,6 +70,7 @@ struct CharDriverState {
 IOReadHandler *chr_read;
 void *handler_opaque;
 void (*chr_close)(struct CharDriverState *chr);
+void (*chr_disconnect)(struct CharDriverState *chr);
 void (*chr_accept_input)(struct CharDriverState *chr);
 void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
 void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
@@ -124,6 +125,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
  */
 CharDriverState *qemu_chr_new(const char *label, const char *filename,
   void (*init)(struct CharDriverState *s));
+/**
+ * @qemu_chr_disconnect:
+ *
+ * Close a fd accpeted by character backend.
+ */
+void qemu_chr_disconnect(CharDriverState *chr);
 
 /**
  * @qemu_chr_delete:
diff --git a/qemu-char.c b/qemu-char.c
index a6b9bd2..67155e9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3849,6 +3849,13 @@ void qemu_chr_fe_release(CharDriverState *s)
 s->avail_connections++;
 }
 
+void qemu_chr_disconnect(CharDriverState *chr)
+{
+if (chr->chr_disconnect) {
+chr->chr_disconnect(chr);
+}
+}
+
 void qemu_chr_free(CharDriverState *chr)
 {
 if (chr->chr_close) {
@@ -4172,6 +4179,7 @@ static CharDriverState 
*qmp_chardev_open_socket(ChardevSocket *sock,
 chr->chr_write = tcp_chr_write;
 chr->chr_sync_read = tcp_chr_sync_read;
 chr->chr_close = tcp_chr_close;
+chr->chr_disconnect = tcp_chr_disconnect;
 chr->get_msgfds = tcp_get_msgfds;
 chr->set_msgfds = tcp_set_msgfds;
 chr->chr_add_client = tcp_chr_add_client;
-- 
2.4.3

[Qemu-devel] [PATCH RFC 12/14] vhost-user: add shutdown support

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 docs/specs/vhost-user.txt | 15 +++
 hw/virtio/vhost-user.c| 30 +-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 5e00bd3..854493e 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -331,6 +331,21 @@ Message types
   This message is only sent if VHOST_USER_PROTOCOL_F_SLAVE_REQ
   feature is available.
 
+Slave message types
+---
+
+ * VHOST_USER_SLAVE_SHUTDOWN:
+  Id: 1
+  Master payload: N/A
+  Slave payload: u64
+
+  Request the master to shutdown the slave. A 0 reply is for
+  success, in which case the slave may close all connections
+  immediately and quit. A non-zero reply cancels the request.
+
+  Before a reply comes, the master may make other requests in
+  order to flush or sync state.
+
 Migration
 -
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 49f566c..949382c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -55,6 +55,7 @@ typedef enum VhostUserRequest {
 
 typedef enum VhostUserSlaveRequest {
 VHOST_USER_SLAVE_NONE = 0,
+VHOST_USER_SLAVE_SHUTDOWN = 1,
 VHOST_USER_SLAVE_MAX
 }  VhostUserSlaveRequest;
 
@@ -140,7 +141,7 @@ static VhostUserRequest 
vhost_user_request_translate(unsigned long int request)
 case VHOST_SET_VRING_ERR:
 return VHOST_USER_SET_VRING_ERR;
 default:
-return VHOST_USER_MAX;
+return request;
 }
 }
 
@@ -375,6 +376,21 @@ static int slave_can_receive(void *opaque)
 return VHOST_USER_HDR_SIZE;
 }
 
+static int vhost_user_slave_write(struct vhost_dev *dev, VhostUserMsg *msg,
+  int *fds, int fd_num)
+{
+struct vhost_user *u = dev->opaque;
+CharDriverState *chr = u->slave_chr;
+int size = VHOST_USER_HDR_SIZE + msg->size;
+
+if (fd_num) {
+qemu_chr_fe_set_msgfds(chr, fds, fd_num);
+}
+
+return qemu_chr_fe_write_all(chr, (const uint8_t *) msg, size) == size ?
+0 : -1;
+}
+
 static void slave_receive(void *opaque, const uint8_t *buf, int size)
 {
 struct vhost_dev *dev = opaque;
@@ -386,6 +402,18 @@ static void slave_receive(void *opaque, const uint8_t 
*buf, int size)
 }
 
 switch (msg->request) {
+case VHOST_USER_SLAVE_SHUTDOWN: {
+uint64_t success = 1;
+
+if (dev->stop) {
+dev->stop(dev);
+success = 0;
+}
+
+msg->u64 = success;
+vhost_user_slave_write(dev, msg, NULL, 0);
+return;
+}
 default:
 error_report("Received unexpected msg type.");
 }
-- 
2.4.3

[Qemu-devel] [PATCH RFC 06/14] vhost-net: keep VIRTIO_NET_F_STATUS for vhost-user

[marcandre . lureau]

From: Marc-André Lureau 

Even if slave does not declare VIRTIO_NET_F_STATUS, we can keep
this feature enabled as it is driven by qemu.

Signed-off-by: Marc-André Lureau 
---
 hw/net/vhost_net.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9850520..ea15220 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -77,7 +77,6 @@ static const int user_feature_bits[] = {
 VIRTIO_NET_F_HOST_ECN,
 VIRTIO_NET_F_HOST_UFO,
 VIRTIO_NET_F_MRG_RXBUF,
-VIRTIO_NET_F_STATUS,
 VIRTIO_NET_F_CTRL_VQ,
 VIRTIO_NET_F_CTRL_RX,
 VIRTIO_NET_F_CTRL_VLAN,
-- 
2.4.3

[Qemu-devel] [PATCH RFC 11/14] vhost-user: add slave-fd support

[marcandre . lureau]

From: Marc-André Lureau 

Learn to give a socket to the slave to let him make requests to the
master.

Signed-off-by: Marc-André Lureau 
---
 docs/specs/vhost-user.txt | 23 +++
 hw/virtio/vhost-user.c| 71 ++-
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 1bc6adb..5e00bd3 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -136,10 +136,23 @@ As older slaves don't support negotiating protocol 
features,
 a feature bit was dedicated for this purpose:
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+Slave communication
+---
+
+Since the vhost-user protocol do not let slave make requests back to
+the master, an optional communication channel is provided if the slave
+declares VHOST_USER_PROTOCOL_F_SLAVE_REQ feature.
+
+The fd is provided via VHOST_USER_SET_SLAVE_FD ancillary data.
+
+A slave may then send VHOST_USER_SLAVE_* messages to the master by
+using this fd.
+
 Protocol features
 -
 
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_SLAVE_REQ 1
 
 Message types
 -
@@ -308,6 +321,16 @@ Message types
   invalid FD flag. This flag is set when there is no file descriptor
   in the ancillary data.
 
+ * VHOST_USER_SET_SLAVE_FD
+  Id: 17
+  Equivalent ioctl: N/A
+  Master payload: N/A
+
+  Set the file descriptor for the salve to make VHOST_USER_SLAVE_*
+  request to the master. It is passed in the ancillary data.
+  This message is only sent if VHOST_USER_PROTOCOL_F_SLAVE_REQ
+  feature is available.
+
 Migration
 -
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 2875b69..49f566c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -27,8 +27,9 @@
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_SLAVE_REQ 1
 
 typedef enum VhostUserRequest {
 VHOST_USER_NONE = 0,
@@ -48,9 +49,15 @@ typedef enum VhostUserRequest {
 VHOST_USER_SET_VRING_ERR = 14,
 VHOST_USER_GET_PROTOCOL_FEATURES = 15,
 VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+VHOST_USER_SET_SLAVE_FD = 17,
 VHOST_USER_MAX
 } VhostUserRequest;
 
+typedef enum VhostUserSlaveRequest {
+VHOST_USER_SLAVE_NONE = 0,
+VHOST_USER_SLAVE_MAX
+}  VhostUserSlaveRequest;
+
 typedef struct VhostUserMemoryRegion {
 uint64_t guest_phys_addr;
 uint64_t memory_size;
@@ -93,6 +100,7 @@ static VhostUserMsg m __attribute__ ((unused));
 
 struct vhost_user {
 CharDriverState *chr;
+CharDriverState *slave_chr;
 };
 
 static bool ioeventfd_enabled(void)
@@ -277,6 +285,7 @@ static int vhost_user_call(struct vhost_dev *dev,
 
 break;
 
+case VHOST_USER_SET_SLAVE_FD:
 case VHOST_USER_SET_LOG_FD:
 fds[fd_num++] = *((int *) arg);
 break;
@@ -361,6 +370,43 @@ end:
 return ret;
 }
 
+static int slave_can_receive(void *opaque)
+{
+return VHOST_USER_HDR_SIZE;
+}
+
+static void slave_receive(void *opaque, const uint8_t *buf, int size)
+{
+struct vhost_dev *dev = opaque;
+VhostUserMsg *msg = (VhostUserMsg *)buf;
+
+if (size != VHOST_USER_HDR_SIZE) {
+error_report("Failed to read from slave.");
+return;
+}
+
+switch (msg->request) {
+default:
+error_report("Received unexpected msg type.");
+}
+}
+
+static void slave_event(void *opaque, int event)
+{
+struct vhost_dev *dev = opaque;
+struct vhost_user *u = dev->opaque;
+CharDriverState *slave_chr = u->slave_chr;
+
+switch (event) {
+case CHR_EVENT_CLOSED:
+if (slave_chr) {
+u->slave_chr = NULL;
+qemu_chr_free(slave_chr);
+}
+break;
+}
+}
+
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
 VhostUserMsg msg = { 0 };
@@ -417,16 +463,39 @@ static int vhost_user_init(struct vhost_dev *dev, void 
*opaque)
 }
 }
 
+if (__virtio_has_feature(dev->protocol_features,
+ VHOST_USER_PROTOCOL_F_SLAVE_REQ)) {
+int sv[2];
+
+if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+error_report("socketpair() failed");
+return -1;
+}
+
+vhost_user_call(dev, VHOST_USER_SET_SLAVE_FD, &sv[1]);
+
+u->slave_chr = qemu_chr_open_eventfd(sv[0]);
+qemu_chr_add_handlers(u->slave_chr, slave_can_receive, slave_receive,
+  slave_event, dev);
+}
+
+
 return 0;
 }
 
 static int vhost_user_cleanup(struct vhost_dev *dev)
 {
 struct vhost_user *u;
+CharDriverState *slave_chr;
 
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
 u = dev->opaque;
+if (u->slave_chr) {
+slave_chr = u->slave_chr;
+  

[Qemu-devel] [PATCH RFC 14/14] test: start vhost-user reconnect test

[marcandre . lureau]

From: Marc-André Lureau 

Check that SLAVE_FD & SHUTDOWN message work and that the master
asked for the ring read status.

Signed-off-by: Marc-André Lureau 
---
 tests/Makefile  |   2 +-
 tests/vhost-user-test.c | 168 
 2 files changed, 155 insertions(+), 15 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 34c6136..740542c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -412,7 +412,7 @@ tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o 
$(libqos-usb-obj-y)
 tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y)
 tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
 tests/pc-cpu-test$(EXESUF): tests/pc-cpu-test.o
-tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o 
qemu-timer.o $(qtest-obj-y)
+tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o 
qemu-timer.o $(qtest-obj-y) $(libqos-pc-obj-y) $(libqos-virtio-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a 
libqemustub.a
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
$(block-obj-y) libqemuutil.a libqemustub.a
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index f78483d..dba50c0 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -21,6 +21,12 @@
 #include 
 #include 
 #include 
+#include "libqos/pci-pc.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+#include "libqos/malloc.h"
+#include "libqos/malloc-pc.h"
+#include "libqos/malloc-generic.h"
 
 /* GLIB version compatibility flags */
 #if !GLIB_CHECK_VERSION(2, 26, 0)
@@ -40,12 +46,12 @@
 #define QEMU_CMD_ACCEL  " -machine accel=tcg"
 #define QEMU_CMD_MEM" -m %d -object memory-backend-file,id=mem,size=%dM,"\
 "mem-path=%s,share=on -numa node,memdev=mem"
-#define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s"
+#define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
 #define QEMU_CMD_NET" -device virtio-net-pci,netdev=net0 "
 #define QEMU_CMD_ROM" -option-rom ../pc-bios/pxe-virtio.rom"
 
-#define QEMU_CMDQEMU_CMD_ACCEL QEMU_CMD_MEM QEMU_CMD_CHR \
+#define QEMU_CMDQEMU_CMD_ACCEL QEMU_CMD_MEM QEMU_CMD_CHR\
 QEMU_CMD_NETDEV QEMU_CMD_NET QEMU_CMD_ROM
 
 #define HUGETLBFS_MAGIC   0x958458f6
@@ -56,6 +62,7 @@
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_SLAVE_REQ 1
 
 #define VHOST_LOG_PAGE 0x1000
 
@@ -77,9 +84,16 @@ typedef enum VhostUserRequest {
 VHOST_USER_SET_VRING_ERR = 14,
 VHOST_USER_GET_PROTOCOL_FEATURES = 15,
 VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+VHOST_USER_SET_SLAVE_FD = 17,
 VHOST_USER_MAX
 } VhostUserRequest;
 
+typedef enum VhostUserSlaveRequest {
+VHOST_USER_SLAVE_NONE = 0,
+VHOST_USER_SLAVE_SHUTDOWN = 1,
+VHOST_USER_SLAVE_MAX
+} VhostUserSlaveRequest;
+
 typedef struct VhostUserMemoryRegion {
 uint64_t guest_phys_addr;
 uint64_t memory_size;
@@ -118,6 +132,7 @@ static VhostUserMsg m __attribute__ ((unused));
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION(0x1)
 /*/
+#define VIRTIO_QUEUE_MAX 1024
 
 typedef struct TestServer {
 gchar *socket_path;
@@ -129,6 +144,9 @@ typedef struct TestServer {
 GMutex *data_mutex;
 GCond *data_cond;
 int log_fd;
+int req_fd;
+int get_base_count;
+uint16_t set_base[VIRTIO_QUEUE_MAX];
 } TestServer;
 
 static const char *hugefs;
@@ -331,7 +349,8 @@ static void chr_read(void *opaque, const uint8_t *buf, int 
size)
 /* send back features to qemu */
 msg.flags |= VHOST_USER_REPLY_MASK;
 msg.size = sizeof(m.u64);
-msg.u64 = 1 << VHOST_USER_PROTOCOL_F_LOG_SHMFD;
+msg.u64 = 1 << VHOST_USER_PROTOCOL_F_LOG_SHMFD |
+1 << VHOST_USER_PROTOCOL_F_SLAVE_REQ;
 p = (uint8_t *) &msg;
 qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
 break;
@@ -343,6 +362,11 @@ static void chr_read(void *opaque, const uint8_t *buf, int 
size)
 msg.state.num = 0;
 p = (uint8_t *) &msg;
 qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
+s->get_base_count++;
+break;
+
+case VHOST_USER_SET_VRING_BASE:
+s->set_base[msg.state.index] = msg.state.num;
 break;
 
 case VHOST_USER_SET_MEM_TABLE:
@@ -371,6 +395,11 @@ static void chr_read(void *opaque, const uint8_t *buf, int 
size)
 g_cond_signal(s->data_cond);
 break;
 
+case VHOST_USER_SET_SLAVE_FD:
+qemu_chr_fe_get_msgfds(chr, &s->req_fd, 1);
+g_cond_signal(s->data_cond);
+break;
+
 case

[Qemu-devel] [PATCH RFC 09/14] vhost-user: add vhost_user to hold the chr

[marcandre . lureau]

From: Marc-André Lureau 

Next patches will add more fields to the structure

Signed-off-by: Marc-André Lureau 
---
 hw/virtio/vhost-user.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b2f46a9..2875b69 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -91,6 +91,10 @@ static VhostUserMsg m __attribute__ ((unused));
 /* The version of the protocol we support */
 #define VHOST_USER_VERSION(0x1)
 
+struct vhost_user {
+CharDriverState *chr;
+};
+
 static bool ioeventfd_enabled(void)
 {
 return kvm_enabled() && kvm_eventfds_enabled();
@@ -134,7 +138,8 @@ static VhostUserRequest 
vhost_user_request_translate(unsigned long int request)
 
 static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg)
 {
-CharDriverState *chr = dev->opaque;
+struct vhost_user *u = dev->opaque;
+CharDriverState *chr = u->chr;
 uint8_t *p = (uint8_t *) msg;
 int r, size = VHOST_USER_HDR_SIZE;
 
@@ -181,7 +186,8 @@ fail:
 static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
 int *fds, int fd_num)
 {
-CharDriverState *chr = dev->opaque;
+struct vhost_user *u = dev->opaque;
+CharDriverState *chr = u->chr;
 int size = VHOST_USER_HDR_SIZE + msg->size;
 
 if (fd_num) {
@@ -358,11 +364,14 @@ end:
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
 VhostUserMsg msg = { 0 };
+struct vhost_user *u;
 int err;
 
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
-dev->opaque = opaque;
+u = g_new0(struct vhost_user, 1);
+u->chr = opaque;
+dev->opaque = u;
 
 msg.request = VHOST_USER_GET_FEATURES;
 msg.flags = VHOST_USER_VERSION;
@@ -413,8 +422,12 @@ static int vhost_user_init(struct vhost_dev *dev, void 
*opaque)
 
 static int vhost_user_cleanup(struct vhost_dev *dev)
 {
+struct vhost_user *u;
+
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
+u = dev->opaque;
+g_free(u);
 dev->opaque = 0;
 
 return 0;
-- 
2.4.3

[Qemu-devel] [PATCH RFC 07/14] virtio-net: enable tx notification if up and vhost started

[marcandre . lureau]

From: Marc-André Lureau 

When vhost is restarted, make sure the tx notification is on.

Signed-off-by: Marc-André Lureau 
---
 hw/net/virtio-net.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 8d28e45..d494c45 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -195,6 +195,10 @@ static void virtio_net_set_status(struct VirtIODevice 
*vdev, uint8_t status)
 } else {
 qemu_bh_cancel(q->tx_bh);
 }
+
+if (n->vhost_started) {
+virtio_queue_set_notification(q->tx_vq, 1);
+}
 }
 }
 }
-- 
2.4.3

[Qemu-devel] [PATCH RFC 10/14] qemu-char: add qemu_chr_free()

[marcandre . lureau]

From: Marc-André Lureau 

If a chardev is allowed to be created outside of QMP, then it must be
also possible to free it. This is useful for ivshmem that creates
chardev anonymously and must be able to free them.

Signed-off-by: Marc-André Lureau 
Acked-by: Paolo Bonzini 
---
 include/sysemu/char.h | 10 +-
 qemu-char.c   |  9 +++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 832b7fe..5fd0a09 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -128,11 +128,19 @@ CharDriverState *qemu_chr_new(const char *label, const 
char *filename,
 /**
  * @qemu_chr_delete:
  *
- * Destroy a character backend.
+ * Destroy a character backend and remove it from the list of
+ * identified character backends.
  */
 void qemu_chr_delete(CharDriverState *chr);
 
 /**
+ * @qemu_chr_free:
+ *
+ * Destroy a character backend.
+ */
+void qemu_chr_free(CharDriverState *chr);
+
+/**
  * @qemu_chr_fe_set_echo:
  *
  * Ask the backend to override its normal echo setting.  This only really
diff --git a/qemu-char.c b/qemu-char.c
index c37a9f9..a6b9bd2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3849,9 +3849,8 @@ void qemu_chr_fe_release(CharDriverState *s)
 s->avail_connections++;
 }
 
-void qemu_chr_delete(CharDriverState *chr)
+void qemu_chr_free(CharDriverState *chr)
 {
-QTAILQ_REMOVE(&chardevs, chr, next);
 if (chr->chr_close) {
 chr->chr_close(chr);
 }
@@ -3861,6 +3860,12 @@ void qemu_chr_delete(CharDriverState *chr)
 g_free(chr);
 }
 
+void qemu_chr_delete(CharDriverState *chr)
+{
+QTAILQ_REMOVE(&chardevs, chr, next);
+qemu_chr_free(chr);
+}
+
 ChardevInfoList *qmp_query_chardev(Error **errp)
 {
 ChardevInfoList *chr_list = NULL;
-- 
2.4.3

[Qemu-devel] [PATCH RFC 05/14] qemu-char: make tcp_chr_disconnect() reentrant-safe

[marcandre . lureau]

From: Marc-André Lureau 

During CHR_EVENT_CLOSED, the function could be reentered, make this
case safe.

Signed-off-by: Marc-André Lureau 
---
 qemu-char.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index d34bfd1..c37a9f9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2833,6 +2833,10 @@ static void tcp_chr_disconnect(CharDriverState *chr)
 {
 TCPCharDriver *s = chr->opaque;
 
+if (!s->connected) {
+return;
+}
+
 s->connected = 0;
 if (s->listen_chan) {
 s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN,
-- 
2.4.3

[Qemu-devel] [PATCH RFC 08/14] vhost: add vhost_dev stop callback

[marcandre . lureau]

From: Marc-André Lureau 

vhost backend may want to stop the device, for example if it wants to
restart itself (translates to a link down for vhost-net).

Signed-off-by: Marc-André Lureau 
---
 hw/net/vhost_net.c| 13 +
 include/hw/virtio/vhost.h |  4 
 2 files changed, 17 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ea15220..f977e2d 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -134,6 +134,18 @@ static int vhost_net_get_fd(NetClientState *backend)
 }
 }
 
+static void vhost_net_backend_stop(struct vhost_dev *dev)
+{
+struct vhost_net *net = container_of(dev, struct vhost_net, dev);
+NetClientState *nc = net->nc;
+NetClientState *peer = nc->peer;
+
+peer->link_down = 1;
+if (peer->info->link_status_changed) {
+peer->info->link_status_changed(peer);
+}
+}
+
 struct vhost_net *vhost_net_init(VhostNetOptions *options)
 {
 int r;
@@ -163,6 +175,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 
 net->dev.nvqs = 2;
 net->dev.vqs = net->vqs;
+net->dev.stop = vhost_net_backend_stop;
 
 r = vhost_dev_init(&net->dev, options->opaque,
options->backend_type);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index ab1dcac..48efd87 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -35,6 +35,8 @@ struct vhost_log {
 vhost_log_chunk_t *log;
 };
 
+typedef void (*vhost_stop)(struct vhost_dev *dev);
+
 struct vhost_memory;
 struct vhost_dev {
 MemoryListener memory_listener;
@@ -59,6 +61,8 @@ struct vhost_dev {
 const VhostOps *vhost_ops;
 void *opaque;
 struct vhost_log *log;
+/* backend request to stop */
+vhost_stop stop;
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-- 
2.4.3

[Qemu-devel] [PATCH RFC 03/14] qemu-char: avoid potential double-free

[marcandre . lureau]

From: Marc-André Lureau 

If tcp_set_msgfds() is called several time with NULL fds, this
could lead to double-free.

Signed-off-by: Marc-André Lureau 
---
 qemu-char.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-char.c b/qemu-char.c
index d956f8d..bc37628 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2713,6 +2713,7 @@ static int tcp_set_msgfds(CharDriverState *chr, int *fds, 
int num)
 /* clear old pending fd array */
 if (s->write_msgfds) {
 g_free(s->write_msgfds);
+s->write_msgfds = NULL;
 }
 
 if (num) {
-- 
2.4.3

[Qemu-devel] [PATCH RFC 01/14] vhost-user: Add ability to know vhost-user backend disconnection

[marcandre . lureau]

From: Tetsuya Mukawa 

Current QEMU cannot detect vhost-user backend disconnection. The
patch adds ability to know it.
To know disconnection, add watcher to detect G_IO_HUP event. When
G_IO_HUP event is detected, the disconnected socket will be read
to cause a CHR_EVENT_CLOSED.

Signed-off-by: Tetsuya Mukawa 
Reviewed-by: Marc-André Lureau 
---
 net/vhost-user.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 5657992..266b54d 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -20,6 +20,7 @@ typedef struct VhostUserState {
 NetClientState nc;
 CharDriverState *chr;
 VHostNetState *vhost_net;
+int watch;
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -120,6 +121,20 @@ static void net_vhost_link_down(VhostUserState *s, bool 
link_down)
 }
 }
 
+static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition cond,
+   void *opaque)
+{
+VhostUserState *s = opaque;
+uint8_t buf[1];
+
+/* We don't actually want to read anything, but CHR_EVENT_CLOSED will be
+ * raised as a side-effect of the read.
+ */
+qemu_chr_fe_read_all(s->chr, buf, sizeof(buf));
+
+return FALSE;
+}
+
 static void net_vhost_user_event(void *opaque, int event)
 {
 VhostUserState *s = opaque;
@@ -128,12 +143,15 @@ static void net_vhost_user_event(void *opaque, int event)
 
 switch (event) {
 case CHR_EVENT_OPENED:
+s->watch = qemu_chr_fe_add_watch(s->chr, G_IO_HUP, 
net_vhost_user_watch, s);
 vhost_user_start(s);
 net_vhost_link_down(s, false);
 break;
 case CHR_EVENT_CLOSED:
 net_vhost_link_down(s, true);
 vhost_user_stop(s);
+g_source_remove(s->watch);
+s->watch = 0;
 break;
 }
 }
-- 
2.4.3

[Qemu-devel] [PATCH RFC 04/14] qemu-char: remove all msgfds on disconnect

[marcandre . lureau]

From: Marc-André Lureau 

Disconnect should reset context.

Signed-off-by: Marc-André Lureau 
---
 qemu-char.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-char.c b/qemu-char.c
index bc37628..d34bfd1 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2846,6 +2846,7 @@ static void tcp_chr_disconnect(CharDriverState *chr)
 SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE,
  "disconnected:", s->addr, s->is_listen, s->is_telnet);
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+tcp_set_msgfds(chr, NULL, 0);
 if (s->reconnect_time) {
 qemu_chr_socket_restart_timer(chr);
 }
-- 
2.4.3

[Qemu-devel] [PATCH RFC 00/14] vhost-user: shutdown and reconnection

[marcandre . lureau]

From: Marc-André Lureau 

In a previous series "Add feature to start QEMU without vhost-user
backend", Tetsuya Mukawa proposed to allow the vhost-user backend to
disconnect and reconnect. However, Michael Tsirkin pointed out that
you can't do that without extra care, because the guest and hypervisor
don't know the slave ring manipulation state, there might be pending
replies for example that could be lost, and suggested to reset the
guest queues, but this requires kernel changes, and it may have to
clear the ring and lose queued packets.

The following series starts from the idea that the slave can request a
"managed" shutdown instead and later recover (I guess the use case for
this is to allow for example to update static dispatching/filter rules
etc)

In order to do it, the slave must be in a good state, that is it
should flush all pending buffers so that resume after
VHOST_SET_VRING_BASE is enough to resume where it lefts. The guest is
made aware of virtio-net disconnection thanks to VIRTIO_NET_S_LINK_UP
status, so communication can be stopped.

Unfortunately, vhost-user protocol isn't bidirectional, so a new
optional communication channel is added for the slave to make request
to the master, such as a the new shutdown request.

I have done some testing with modified vapp and linux 4.2, it seems to
work just fine. But more intensive testing and review are required, as
I am not sure this approach can be made solid enough. Before going
further, I would welcome any comment or testing suggestions!

The series is based on top of pending vhost-user migration series, but
for easier testing you may just use the following git repo:
https://github.com/elmarco/qemu vhost-user-reconnect branch

Marc-André Lureau (12):
  vhost-user: remove useless is_server field
  qemu-char: avoid potential double-free
  qemu-char: remove all msgfds on disconnect
  qemu-char: make tcp_chr_disconnect() reentrant-safe
  vhost-net: keep VIRTIO_NET_F_STATUS for vhost-user
  virtio-net: enable tx notification if up and vhost started
  vhost: add vhost_dev stop callback
  vhost-user: add vhost_user to hold the chr
  qemu-char: add qemu_chr_free()
  vhost-user: add slave-fd support
  vhost-user: add shutdown support
  test: start vhost-user reconnect test

Tetsuya Mukawa (2):
  vhost-user: Add ability to know vhost-user backend disconnection
  qemu-char: Add qemu_chr_disconnect to close a fd accepted by listen fd

 docs/specs/vhost-user.txt |  38 +++
 hw/net/vhost_net.c|  14 +++-
 hw/net/virtio-net.c   |   4 ++
 hw/virtio/vhost-user.c| 120 +++--
 include/hw/virtio/vhost.h |   4 ++
 include/sysemu/char.h |  17 -
 net/vhost-user.c  |  20 +-
 qemu-char.c   |  23 ++-
 tests/Makefile|   2 +-
 tests/vhost-user-test.c   | 168 ++
 10 files changed, 384 insertions(+), 26 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH RFC 02/14] vhost-user: remove useless is_server field

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 net/vhost-user.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 266b54d..5d24129 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -26,7 +26,6 @@ typedef struct VhostUserState {
 typedef struct VhostUserChardevProps {
 bool is_socket;
 bool is_unix;
-bool is_server;
 } VhostUserChardevProps;
 
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
@@ -187,7 +186,6 @@ static int net_vhost_chardev_opts(void *opaque,
 } else if (strcmp(name, "path") == 0) {
 props->is_unix = true;
 } else if (strcmp(name, "server") == 0) {
-props->is_server = true;
 } else {
 error_setg(errp,
"vhost-user does not support a chardev with option %s=%s",
-- 
2.4.3

Re: [Qemu-devel] [Qemu-ppc] target-ppc: Fix SRR0 when taking unaligned exceptions

[Benjamin Herrenschmidt]

On Thu, 2015-07-02 at 14:44 +1000, Anton Blanchard wrote:
> We are setting SRR0 to the instruction before the one causing the
> unaligned exception. A quick testcase:
> 
 ../..

> p_helper.c b/target-ppc/excp_helper.c
> index b803475..4250106 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -200,7 +200,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu,
> int excp_model, int excp)
>  /* Get rS/rD and rA from faulting opcode */
>  env->spr[SPR_DSISR] |= (cpu_ldl_code(env, (env->nip - 4))
>  & 0x03FF) >> 16;.

You need to also fix the above to use env->nip instead of env->nip - 4
when generating DSISR

> -goto store_current;
> +goto store_next;
>  case POWERPC_EXCP_PROGRAM:   /* Program exception   
>  */
>  switch (env->error_code & ~0xF) {
>  case POWERPC_EXCP_FP:

[Qemu-devel] [Bug 638955] Re: emulated netcards don't work with recent sunos kernel

[Jan Vlug]

See also bug #1395217

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/638955

Title:
  emulated netcards don't work with recent sunos kernel

Status in QEMU:
  New

Bug description:
  hi there,

  i'm using qemu-kvm backend in version: # qemu-kvm -version
  QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 
Fabrice Bellard

  and there are just *not working any of model=$type with combinations
  of recent sunos (solaris, openindiana, opensolaris, ..) ..

  you can download for testing purposes iso from here: http://dlc-
  origin.openindiana.org/isos/147/ or from here:
  http://genunix.org/distributions/indiana/ << osol and oi are also
  bubuntu-like *live cds, so no need to bother with installing

  behaviour is as follows:
  e1000 - receiving doesn't work, transmitting works .. dladm (tool for handle 
ethers) shows that is all ok, correct mode is loaded up, it just seems like 
this driver works at 100% but ..

  rtl8169|pcnet - works in 10Mbit mode with several other issues like
  high cpu utilization and so .. dladm is unable to recognize options
  for this kind of -nic

  others - just don't work

  .. i experienced this issue several times in past .. woraround was,
  that rtl8169 worked so-so .. with recent sunos kernel it doesn't.

  it's easy to reproduce, this is why i'm not putting here more then
  launching script for my virtual machine:

  # cat openindiana.sh
  qemu-kvm -hda /home/kvm/openindiana/openindiana.img -m 2048 -localtime -cdrom 
/home/kvm/+images/oi-dev-147-x86.iso -boot d \
  -vga std -vnc :9 -k en-us -monitor 
unix:/home/kvm/openindiana/instance,server,nowait \
  -net nic,model=e1000,vlan=1 -net tap,ifname=oi0,script=no,vlan=1 &

  sleep 2;
  ip l set oi0 up;
  ip a a 192.168.99.9/24 dev oi0;

  regards by daniel

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/638955/+subscriptions

[Qemu-devel] [Bug 1395217] Re: Networking in qemu 2.0.0 and beyond is not compatible with Open Solaris (Illumos) 5.11

[Jan Vlug]

See also bug #638955

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1395217

Title:
  Networking in qemu 2.0.0 and beyond is not compatible with Open
  Solaris (Illumos) 5.11

Status in QEMU:
  New

Bug description:
  The networking code in qemu in versions 2.0.0 and beyond is non-
  functional with Solaris/Illumos 5.11 images.

  Building 1.7.1, 2.0.0, 2.0.2, 2.1.2,and 2.2.0rc1with the following
  standard Slackware config:

  # From Slackware build tree . . . 
  ./configure \
--prefix=/usr \
--libdir=/usr/lib64 \
--sysconfdir=/etc \
--localstatedir=/var \
--enable-gtk \
--enable-system \
--enable-kvm \
--disable-debug-info \
--enable-virtfs \
--enable-sdl \
--audio-drv-list=alsa,oss,sdl,esd \
--enable-libusb \
--disable-vnc \
--target-list=x86_64-linux-user,i386-linux-user,x86_64-softmmu,i386-softmmu 
\
--enable-spice \
--enable-usb-redir 

  
  And attempting to run the same VM image with the following command (or via 
virt-manager):

  macaddress="DE:AD:BE:EF:3F:A4"

  qemu-system-x86_64 nex4x -cdrom /dev/cdrom -name "Nex41" -cpu Westmere
  -machine accel=kvm -smp 2 -m 4000 -net nic,macaddr=$macaddress  -net 
bridge,br=b
  r0 -net dump,file=/usr1/tmp/ -drive file=nex4x_d1 -drive 
file=nex4x_d2
   -enable-kvm

  Gives success on 1.7.1, and a deaf VM on all subsequent versions.

  Notable in validating my config, is that a Windows 7 image runs
  cleanly with networking on *all* builds, so my configuration appears
  to be good - qemu just hates Solaris at this point.

  Watching with wireshark (as well as pulling network traces from qemu
  as noted above) it appears that the notable difference in the two
  configs is that for some reason, Solaris gets stuck arping for it's
  own interface on startup, and never really comes on line on the
  network.  If other hosts attempt to ping the Solaris instance, they
  can successfully arp the bad VM, but not the other way around.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1395217/+subscriptions

[Qemu-devel] [PATCH] spapr_drc: don't allow 'empty' DRCs to be unisolated

[Michael Roth]

Logical resources start with allocation-state:UNUSABLE /
isolation-state:ISOLATED. During hotplug, guests will transition
them to allocate-state:USABLE, and then to isolate-state:UNISOLATED.
The former transition does not seem to have any failure path for
cases where a DRC does not have any resources associated with it to
allocate for guest, but instead relies on the subsequent
isolation-state:UNISOLATED transition to indicate failure in this
situation.

Currently DRC code does not implement this logic, but instead
tries to indicate failure by refusing the allocation-state:USABLE
transition. Unfortunately, since that's not a documented failure
path, guests continue undeterred, causing undefined behavior in
QEMU and guest code.

Fix this by handling things as PAPR defines (13.7 and 13.7.3.1).

Cc: qemu-...@nongnu.org
Cc: David Gibson 
Cc: Bharata B Rao 
Signed-off-by: Michael Roth 
---
 hw/ppc/spapr_drc.c | 12 
 include/hw/ppc/spapr_drc.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 9ce844a..c1f664f 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -66,6 +66,18 @@ static int set_isolation_state(sPAPRDRConnector *drc,
 
 DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
 
+if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
+/* cannot unisolate a non-existant resource. this generally
+ * happens for logical resources where transitions from
+ * allocation-state:UNUSABLE to allocation-state:USABLE are
+ * unguarded, but instead rely on a subsequent
+ * isolation-state:UNISOLATED transition to indicate failure
+ */
+if (!drc->dev) {
+return -1;
+}
+}
+
 drc->isolation_state = state;
 
 if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 28ffeae..3fbe9ea 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -109,6 +109,7 @@ typedef enum {
  * unusable: device not currently available to OS
  * exchange: (currently unused)
  * recover: (currently unused)
+ * no_sensor: for logical DR only, returned when no resource available
  */
 typedef enum {
 SPAPR_DR_ENTITY_SENSE_EMPTY = 0,
@@ -116,6 +117,7 @@ typedef enum {
 SPAPR_DR_ENTITY_SENSE_UNUSABLE  = 2,
 SPAPR_DR_ENTITY_SENSE_EXCHANGE  = 3,
 SPAPR_DR_ENTITY_SENSE_RECOVER   = 4,
+SPAPR_DR_ENTITY_SENSE_NO_SENSOR = -3,
 } sPAPRDREntitySense;
 
 typedef enum {
-- 
1.9.1

Re: [Qemu-devel] [FIX PATCH] spapr_drc: Return correct state for logical DR in entity_sense()

[Michael Roth]

Quoting Michael Roth (2015-09-08 16:03:56)
> Quoting David Gibson (2015-09-07 20:22:50)
> > On Mon, Sep 07, 2015 at 11:37:04AM +0530, Bharata B Rao wrote:
> > > When drmgr is run in the guest to add a device for which device_add
> > > hasn't been issued in QEMU, configure-connector call fails.
> > > When configure-connector call fails, the guest would release (*)
> > > the previously acquired DRC by setting back the DRC isolation state
> > > to ISOLATED and allocation state to UNUSABLE. These calls will be issued
> > > only if get-sensor-state call returns PRESENT state. However currently for
> > > a logical DR, entity_sense() would unconditinally return UNUSABLE
> > > state only. This prevents any subsequent hotplug of the device with
> > > that DRC.
> 
> This seems a little odd. I think we default to ALLOCATION_STATE_UNUSABLE
> for logical DR, and it's up the guest to transition to USABLE, which
> probably happens prior to the configure-connector calls. So I think the
> net effect of this fix is that guest will see these unallocated/unattached
> resources the same way they would a resource that was actually attached
> via device_add, and all we're really doing is working around the
> eventual configuration failure that that will lead to by pretending a
> resource was actually there.
> 
> According to PAPR+ 2.7:
> 
> 13.7.3.1 Acquire Logical Resource from Resource Pool:
> 
>   If the state is “unusable” the OS issues set-indicator (allocation-state, 
> usable) to attempt to allocate the re-
>   source. Similarly, if the state is “available for exchange” the OS issues 
> set-indicator (allocation-state, ex-
>   change) to attempt to allocate the resource, and if the state is “available 
> for recovery” the OS issues
>   set-indicator (allocation-state, recover) to attempt to allocate the 
> resource.
> 
> and
> 
> 13.7 Logical Resource Dynamic Reconfiguration (LRDR):
> 
>   The OS may use the get-sensor-state RTAS call with the dr-entity-sense 
> token to deter-
> mine if a given drc-index refers to a connector that is currently usable for 
> DR operations. If the connector is not
> currently usable the return state is “DR entity unusable” (2). A 
> set-indicator (isolation state) RTAS call to an unusable
> connector or (dr-indicator) to any logical resource connector results in a 
> “No such indicator implemented” return sta-
> tus.  
> 
> So I think maybe the proper fix is to make sure that
> drc->set_indicator_state() fails with an error that indicates to RTAS to
> return NO_SENSOR (-3) for cases where we haven't attached a resource
> to the DRC via device_add.

Patch incoming:

  spapr_drc: don't allow 'empty' DRCs to be unisolated

applies to spapr-next but requires revert of this patch.

Bharata, can you give it a spin with CPU hotplug and see if it fixes the
issue you hit?

> 
> Which also kind of re-opens the discussion of whether or not
> drc->set_indicator_state() should return RTAS errors directly. I'd
> still stray away from that for now but maybe if we get more cases
> like this it'll start becoming more practical.

I'm starting to second-guess myself on this. I'm trying to maintain
separation between RTAS/DRC but result is a bit pathological. Feel free
to comment in the above patch.

Just FYI: original author names appear to have gotten lost in recent
spapr-next rebase.

> 
> > > 
> > > Fix this by returning the right state in entity_sense() by checking
> > > the allocation_state of DRC.
> > > 
> > > (*) 
> > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-September/133430.html
> > > 
> > > Signed-off-by: Bharata B Rao 
> > > Cc: Michael Roth 
> > 
> > Reviewed-by: David Gibson 
> > 
> > and applied to my tree.
> > 
> > > ---
> > >  hw/ppc/spapr_drc.c | 6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > > index 9ce844a..2586065 100644
> > > --- a/hw/ppc/spapr_drc.c
> > > +++ b/hw/ppc/spapr_drc.c
> > > @@ -186,7 +186,11 @@ static sPAPRDREntitySense 
> > > entity_sense(sPAPRDRConnector *drc)
> > >   */
> > >  state = SPAPR_DR_ENTITY_SENSE_EMPTY;
> > >  } else {
> > > -state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > > +if (drc->allocation_state == 
> > > SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> > > +state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > > +} else {
> > > +state = SPAPR_DR_ENTITY_SENSE_PRESENT;
> > > +}
> > >  }
> > >  }
> > >  
> > 
> > -- 
> > David Gibson| I'll have my music baroque, and my code
> > david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
> > | _way_ _around_!
> > http://www.ozlabs.org/~dgibson
> 
> 

Re: [Qemu-devel] [PATCH] typofixes - v4 - https://github.com/vlajos/misspell_fixer

[Peter Maydell]

On 8 September 2015 at 22:45, Veres Lajos  wrote:
> Signed-off-by: Veres Lajos 
> ---
>  disas/i386.c|2 +-
>  disas/s390.c|4 ++--
>  docs/specs/ppc-spapr-hcalls.txt |4 ++--
>  docs/writing-qmp-commands.txt   |2 +-
>  hw/audio/fmopl.c|2 +-
>  hw/core/qdev.c  |2 +-
>  hw/cris/axis_dev88.c|2 +-
>  hw/display/qxl-render.c |2 +-
>  hw/dma/xilinx_axidma.c  |2 +-
>  hw/input/stellaris_input.c  |2 +-
>  hw/intc/xics.c  |2 +-
>  hw/net/fsl_etsec/etsec.c|2 +-
>  hw/sd/pl181.c   |2 +-
>  hw/vfio/pci.c   |2 +-
>  hw/xen/xen-host-pci-device.c|2 +-
>  include/block/scsi.h|2 +-
>  include/exec/memory.h   |2 +-
>  libcacard/card_7816t.h  |2 +-
>  libdecnumber/decContext.c   |2 +-
>  libdecnumber/decNumber.c|8 
>  linux-user/syscall.c|1 -
>  linux-user/syscall_defs.h   |4 ++--
>  qga/commands-posix.c|2 +-
>  target-alpha/mem_helper.c   |2 +-
>  target-arm/cpu.h|4 ++--
>  target-cris/cpu.h   |2 +-
>  target-cris/translate.c |6 +++---
>  target-mips/helper.c|2 +-
>  target-s390x/kvm.c  |4 ++--
>  target-sh4/helper.c |6 +++---
>  tcg/ia64/tcg-target.c   |2 +-
>  tcg/tcg.c   |4 ++--
>  tests/image-fuzzer/runner.py|2 +-
>  tests/qemu-iotests/041  |2 +-
>  util/qemu-thread-posix.c|2 +-
>  35 files changed, 47 insertions(+), 48 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH] typofixes - v4 - https://github.com/vlajos/misspell_fixer

[Veres Lajos]

Signed-off-by: Veres Lajos 
---
 disas/i386.c|2 +-
 disas/s390.c|4 ++--
 docs/specs/ppc-spapr-hcalls.txt |4 ++--
 docs/writing-qmp-commands.txt   |2 +-
 hw/audio/fmopl.c|2 +-
 hw/core/qdev.c  |2 +-
 hw/cris/axis_dev88.c|2 +-
 hw/display/qxl-render.c |2 +-
 hw/dma/xilinx_axidma.c  |2 +-
 hw/input/stellaris_input.c  |2 +-
 hw/intc/xics.c  |2 +-
 hw/net/fsl_etsec/etsec.c|2 +-
 hw/sd/pl181.c   |2 +-
 hw/vfio/pci.c   |2 +-
 hw/xen/xen-host-pci-device.c|2 +-
 include/block/scsi.h|2 +-
 include/exec/memory.h   |2 +-
 libcacard/card_7816t.h  |2 +-
 libdecnumber/decContext.c   |2 +-
 libdecnumber/decNumber.c|8 
 linux-user/syscall.c|1 -
 linux-user/syscall_defs.h   |4 ++--
 qga/commands-posix.c|2 +-
 target-alpha/mem_helper.c   |2 +-
 target-arm/cpu.h|4 ++--
 target-cris/cpu.h   |2 +-
 target-cris/translate.c |6 +++---
 target-mips/helper.c|2 +-
 target-s390x/kvm.c  |4 ++--
 target-sh4/helper.c |6 +++---
 tcg/ia64/tcg-target.c   |2 +-
 tcg/tcg.c   |4 ++--
 tests/image-fuzzer/runner.py|2 +-
 tests/qemu-iotests/041  |2 +-
 util/qemu-thread-posix.c|2 +-
 35 files changed, 47 insertions(+), 48 deletions(-)

diff --git a/disas/i386.c b/disas/i386.c
index 00ceca9..c63d4a0 100644
--- a/disas/i386.c
+++ b/disas/i386.c
@@ -357,7 +357,7 @@ fetch_data(struct disassemble_info *info, bfd_byte *addr)
 #define Rd { OP_R, d_mode }
 #define Rm { OP_R, m_mode }
 #define Ib { OP_I, b_mode }
-#define sIb { OP_sI, b_mode }  /* sign extened byte */
+#define sIb { OP_sI, b_mode }  /* sign extended byte */
 #define Iv { OP_I, v_mode }
 #define Iq { OP_I, q_mode }
 #define Iv64 { OP_I64, v_mode }
diff --git a/disas/s390.c b/disas/s390.c
index 974460c..c29bc4e 100644
--- a/disas/s390.c
+++ b/disas/s390.c
@@ -613,7 +613,7 @@ static const struct s390_operand s390_operands[] =
   names of the instruction format that you can find in the principals
   of operation.
2) the last part of the definition (y in INSTR_x_y) gives you an idea
-  which operands the binary represenation of the instruction has.
+  which operands the binary representation of the instruction has.
   The meanings of the letters in y are:
   a - access register
   c - control register
@@ -627,7 +627,7 @@ static const struct s390_operand s390_operands[] =
   m - mode field, 4 bit
   0 - operand skipped.
   The order of the letters reflects the layout of the format in
-  storage and not the order of the paramaters of the instructions.
+  storage and not the order of the parameters of the instructions.
   The use of the letters is not a 100% match with the PoP but it is
   quite close.

diff --git a/docs/specs/ppc-spapr-hcalls.txt b/docs/specs/ppc-spapr-hcalls.txt
index 667b3fa..5bd8eab 100644
--- a/docs/specs/ppc-spapr-hcalls.txt
+++ b/docs/specs/ppc-spapr-hcalls.txt
@@ -41,8 +41,8 @@ When the guest runs in "real mode" (in powerpc lingua this 
means
 with MMU disabled, ie guest effective == guest physical), it only
 has access to a subset of memory and no IOs.

-PAPR provides a set of hypervisor calls to perform cachable or
-non-cachable accesses to any guest physical addresses that the
+PAPR provides a set of hypervisor calls to perform cacheable or
+non-cacheable accesses to any guest physical addresses that the
 guest can use in order to access IO devices while in real mode.

 This is typically used by the firmware running in the guest.
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index ab1fdd3..f7693ca 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -122,7 +122,7 @@ There are a few things to be noticed:
 Now a little hack is needed. As we're still using the old QMP server we need
 to add the new command to its internal dispatch table. This step won't be
 required in the near future. Open the qmp-commands.hx file and add the
-following in the botton:
+following at the bottom:

 {
 .name   = "hello-world",
diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
index adcef2d..81c0c1b 100644
--- a/hw/audio/fmopl.c
+++ b/hw/audio/fmopl.c
@@ -1177,7 +1177,7 @@ void OPLResetChip(FM_OPL *OPL)
OPLWriteReg(OPL,0x03,0); /* Timer2 */
OPLWriteReg(OPL,0x04,0); /* IRQ mask clear */
for(i = 0xff ; i >= 0x20 ; i-- ) OPLWriteReg(OPL,i,0);
-   /* reset OPerator paramater */
+   /* reset operator parameter */
for( c = 0 ; c < OPL->max_ch ; c++ )
{
OPL_CH *CH = &OPL->P_CH[c];
diff --git a/hw/core/qdev.c b/hw

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed

[John Snow]

On 09/08/2015 05:38 PM, Max Reitz wrote:
> On 08.09.2015 23:37, John Snow wrote:
>> 
>> 
>> On 09/08/2015 05:29 PM, Max Reitz wrote:
>>> On 08.09.2015 23:25, John Snow wrote:
 
 
 On 08/31/2015 03:05 PM, Max Reitz wrote:
> Currently, if a subprocess of a python test (i.e. qemu-io, 
> qemu-img, or qemu) receives a signal and is subsequently 
> aborted, this is not logged.
> 
> This patch makes python tests always check the exit code
> of these subprocesses, and emit a message if they have
> been killed.
> 
> Signed-off-by: Max Reitz  --- 
> tests/qemu-iotests/iotests.py | 26
> +- 1 file changed, 21
> insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py 
> b/tests/qemu-iotests/iotests.py index 927c74a..d082597
> 100644 --- a/tests/qemu-iotests/iotests.py +++ 
> b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@ 
> socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 
> 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img
> and return the exit code''' devnull = open('/dev/null',
> 'r+') - return subprocess.call(qemu_img_args + list(args), 
> stdin=devnull, stdout=devnull) +exitcode = 
> subprocess.call(qemu_img_args + list(args), stdin=devnull, 
> stdout=devnull) +if exitcode < 0: + 
> sys.stderr.write('qemu-img received signal %i: %s\n' % 
> (-exitcode, ' '.join(qemu_img_args + list(args +
> return exitcode
> 
 
 Why tack on the arguments after the retcode for the print?
 The format makes it look like it should be a description for
 the signal received.
>>> 
>>> qemu_img_args contains the qemu-img filename as well, so it
>>> should be obvious that that is the command line.
>>> 
>>> For the "why": I hope it will make debugging easier by
>>> providing the exact command line so you can reproduce the
>>> problem outside of the test.
>>> 
>>> Max
>>> 
>> 
>> Sorry for being captain bikeshed: can this be something like:
>> 
>> "%s received signal %i; args: %s"
>> 
>> to avoid the string looking like it was meant to answer the
>> question of what the signal was? I won't insist on it, though:
>> 
>> Reviewed-by: John Snow 
> 
> Well, feel free to fix it since it's already in master. :-)
> 
> (also, compare how bash does it:
>> $FILE: line $LINE: $PID $SIGNAL_NAME (core dumped) ($ARGS)
> )
> 
> Max
> 

ugh! pfeh! didn't see the v2, sorry. ignore this garbage.

--js

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed

[Max Reitz]

On 08.09.2015 23:37, John Snow wrote:
> 
> 
> On 09/08/2015 05:29 PM, Max Reitz wrote:
>> On 08.09.2015 23:25, John Snow wrote:
>>>
>>>
>>> On 08/31/2015 03:05 PM, Max Reitz wrote:
 Currently, if a subprocess of a python test (i.e. qemu-io,
 qemu-img, or qemu) receives a signal and is subsequently
 aborted, this is not logged.

 This patch makes python tests always check the exit code of
 these subprocesses, and emit a message if they have been
 killed.

 Signed-off-by: Max Reitz  --- 
 tests/qemu-iotests/iotests.py | 26 +- 1
 file changed, 21 insertions(+), 5 deletions(-)

 diff --git a/tests/qemu-iotests/iotests.py
 b/tests/qemu-iotests/iotests.py index 927c74a..d082597 100644 
 --- a/tests/qemu-iotests/iotests.py +++
 b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@
 socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER',
 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img and
 return the exit code''' devnull = open('/dev/null', 'r+') -
 return subprocess.call(qemu_img_args + list(args),
 stdin=devnull, stdout=devnull) +exitcode =
 subprocess.call(qemu_img_args + list(args), stdin=devnull,
 stdout=devnull) +if exitcode < 0: +
 sys.stderr.write('qemu-img received signal %i: %s\n' %
 (-exitcode, ' '.join(qemu_img_args + list(args +return
 exitcode

>>>
>>> Why tack on the arguments after the retcode for the print? The
>>> format makes it look like it should be a description for the
>>> signal received.
>>
>> qemu_img_args contains the qemu-img filename as well, so it should
>> be obvious that that is the command line.
>>
>> For the "why": I hope it will make debugging easier by providing
>> the exact command line so you can reproduce the problem outside of
>> the test.
>>
>> Max
>>
> 
> Sorry for being captain bikeshed: can this be something like:
> 
> "%s received signal %i; args: %s"
> 
> to avoid the string looking like it was meant to answer the question
> of what the signal was?
> I won't insist on it, though:
> 
> Reviewed-by: John Snow 

Well, feel free to fix it since it's already in master. :-)

(also, compare how bash does it:
> $FILE: line $LINE: $PID $SIGNAL_NAME (core dumped) ($ARGS)
)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed

[John Snow]

On 09/08/2015 05:29 PM, Max Reitz wrote:
> On 08.09.2015 23:25, John Snow wrote:
>> 
>> 
>> On 08/31/2015 03:05 PM, Max Reitz wrote:
>>> Currently, if a subprocess of a python test (i.e. qemu-io,
>>> qemu-img, or qemu) receives a signal and is subsequently
>>> aborted, this is not logged.
>>> 
>>> This patch makes python tests always check the exit code of
>>> these subprocesses, and emit a message if they have been
>>> killed.
>>> 
>>> Signed-off-by: Max Reitz  --- 
>>> tests/qemu-iotests/iotests.py | 26 +- 1
>>> file changed, 21 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/tests/qemu-iotests/iotests.py
>>> b/tests/qemu-iotests/iotests.py index 927c74a..d082597 100644 
>>> --- a/tests/qemu-iotests/iotests.py +++
>>> b/tests/qemu-iotests/iotests.py @@ -49,20 +49,34 @@
>>> socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER',
>>> 'socket_scm_helper') def qemu_img(*args): '''Run qemu-img and
>>> return the exit code''' devnull = open('/dev/null', 'r+') -
>>> return subprocess.call(qemu_img_args + list(args),
>>> stdin=devnull, stdout=devnull) +exitcode =
>>> subprocess.call(qemu_img_args + list(args), stdin=devnull,
>>> stdout=devnull) +if exitcode < 0: +
>>> sys.stderr.write('qemu-img received signal %i: %s\n' %
>>> (-exitcode, ' '.join(qemu_img_args + list(args +return
>>> exitcode
>>> 
>> 
>> Why tack on the arguments after the retcode for the print? The
>> format makes it look like it should be a description for the
>> signal received.
> 
> qemu_img_args contains the qemu-img filename as well, so it should
> be obvious that that is the command line.
> 
> For the "why": I hope it will make debugging easier by providing
> the exact command line so you can reproduce the problem outside of
> the test.
> 
> Max
> 

Sorry for being captain bikeshed: can this be something like:

"%s received signal %i; args: %s"

to avoid the string looking like it was meant to answer the question
of what the signal was?
I won't insist on it, though:

Reviewed-by: John Snow 

>> 
>>> def qemu_img_verbose(*args): '''Run qemu-img without
>>> suppressing its output and return the exit code''' -return
>>> subprocess.call(qemu_img_args + list(args)) +exitcode =
>>> subprocess.call(qemu_img_args + list(args)) +if exitcode <
>>> 0: +sys.stderr.write('qemu-img received signal %i:
>>> %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args +
>>> return exitcode
>>> 
>>> def qemu_img_pipe(*args): '''Run qemu-img and return its
>>> output''' -return subprocess.Popen(qemu_img_args +
>>> list(args), stdout=subprocess.PIPE).communicate()[0] +subp
>>> = subprocess.Popen(qemu_img_args + list(args),
>>> stdout=subprocess.PIPE) +exitcode = subp.wait() +if
>>> exitcode < 0: +sys.stderr.write('qemu-img received
>>> signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args +
>>> list(args +return subp.communicate()[0]
>>> 
>>> def qemu_io(*args): '''Run qemu-io and return the stdout
>>> data''' args = qemu_io_args + list(args) -return
>>> subprocess.Popen(args,
>>> stdout=subprocess.PIPE).communicate()[0] +subp =
>>> subprocess.Popen(args, stdout=subprocess.PIPE) +exitcode =
>>> subp.wait() +if exitcode < 0: +
>>> sys.stderr.write('qemu-io received signal %i: %s\n' %
>>> (-exitcode, ' '.join(args))) +return subp.communicate()[0]
>>> 
>>> def compare_images(img1, img2): '''Return True if two image
>>> files are identical''' @@ -197,7 +211,9 @@ class VM(object): 
>>> '''Terminate the VM and clean up''' if not self._popen is
>>> None: self._qmp.cmd('quit') -self._popen.wait() +
>>> exitcode = self._popen.wait() +if exitcode < 0: +
>>> sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,
>>> ' '.join(self._args))) os.remove(self._monitor_path) 
>>> os.remove(self._qtest_path) os.remove(self._qemu_log_path)
>>> 
> 
> 

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[Jan Kiszka]

[thanks for forwarding, Peter]

Hi Rita,

On 2015-09-08 10:11, Peter Maydell wrote:
> On 7 September 2015 at 22:31, Rita Sinha  wrote:
>> Hi Jan,
>>
>> I am interested in participating in next round of Outreachy program
>> with AMD IOMMU emulation project.
>>
>>
>> I have worked on BIOS projects which includes coreboot SeaBios etc and
>> bootloaders like u-boot and grub. I have experience of working with
>> qemu and feel that this project is the right match for my skillset.
>> Kindly guide me how to go ahead with this.

The particular AMD IOMMU project moved on since we listed it. I'm CC'ing
David, who is currently working on it and just recently posted related
patches, and Valentine who probably oversees the status better than I
(due to my lacking involvement recently). David, maybe you can briefly
comment on status and plans of your work.

For the Outreachy program, just like for GSoC, we need to find a good
topic that is sufficiently clear defined on program start and not worked
on in parallel during the runtime. There are still a number of open
topics in this area, e.g. around the older Intel IOMMU model (error
handling and reporting, interrupt remapping), or maybe we find something
different - depends on your interests and experiences. Do you have any
public references to your previous work?

Then I'd suggest to schedule an irc meeting to discuss your interests
and background a bit further and consider available options.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed

[Max Reitz]

On 08.09.2015 23:25, John Snow wrote:
> 
> 
> On 08/31/2015 03:05 PM, Max Reitz wrote:
>> Currently, if a subprocess of a python test (i.e. qemu-io, qemu-img, or
>> qemu) receives a signal and is subsequently aborted, this is not logged.
>>
>> This patch makes python tests always check the exit code of these
>> subprocesses, and emit a message if they have been killed.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  tests/qemu-iotests/iotests.py | 26 +-
>>  1 file changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 927c74a..d082597 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -49,20 +49,34 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 
>> 'socket_scm_helper')
>>  def qemu_img(*args):
>>  '''Run qemu-img and return the exit code'''
>>  devnull = open('/dev/null', 'r+')
>> -return subprocess.call(qemu_img_args + list(args), stdin=devnull, 
>> stdout=devnull)
>> +exitcode = subprocess.call(qemu_img_args + list(args), stdin=devnull, 
>> stdout=devnull)
>> +if exitcode < 0:
>> +sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, 
>> ' '.join(qemu_img_args + list(args
>> +return exitcode
>>  
> 
> Why tack on the arguments after the retcode for the print? The format
> makes it look like it should be a description for the signal received.

qemu_img_args contains the qemu-img filename as well, so it should be
obvious that that is the command line.

For the "why": I hope it will make debugging easier by providing the
exact command line so you can reproduce the problem outside of the test.

Max

> 
>>  def qemu_img_verbose(*args):
>>  '''Run qemu-img without suppressing its output and return the exit 
>> code'''
>> -return subprocess.call(qemu_img_args + list(args))
>> +exitcode = subprocess.call(qemu_img_args + list(args))
>> +if exitcode < 0:
>> +sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, 
>> ' '.join(qemu_img_args + list(args
>> +return exitcode
>>  
>>  def qemu_img_pipe(*args):
>>  '''Run qemu-img and return its output'''
>> -return subprocess.Popen(qemu_img_args + list(args), 
>> stdout=subprocess.PIPE).communicate()[0]
>> +subp = subprocess.Popen(qemu_img_args + list(args), 
>> stdout=subprocess.PIPE)
>> +exitcode = subp.wait()
>> +if exitcode < 0:
>> +sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, 
>> ' '.join(qemu_img_args + list(args
>> +return subp.communicate()[0]
>>  
>>  def qemu_io(*args):
>>  '''Run qemu-io and return the stdout data'''
>>  args = qemu_io_args + list(args)
>> -return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0]
>> +subp = subprocess.Popen(args, stdout=subprocess.PIPE)
>> +exitcode = subp.wait()
>> +if exitcode < 0:
>> +sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
>> '.join(args)))
>> +return subp.communicate()[0]
>>  
>>  def compare_images(img1, img2):
>>  '''Return True if two image files are identical'''
>> @@ -197,7 +211,9 @@ class VM(object):
>>  '''Terminate the VM and clean up'''
>>  if not self._popen is None:
>>  self._qmp.cmd('quit')
>> -self._popen.wait()
>> +exitcode = self._popen.wait()
>> +if exitcode < 0:
>> +sys.stderr.write('qemu received signal %i: %s\n' % 
>> (-exitcode, ' '.join(self._args)))
>>  os.remove(self._monitor_path)
>>  os.remove(self._qtest_path)
>>  os.remove(self._qemu_log_path)
>>




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: Warn if python subprocess is killed

[John Snow]

On 08/31/2015 03:05 PM, Max Reitz wrote:
> Currently, if a subprocess of a python test (i.e. qemu-io, qemu-img, or
> qemu) receives a signal and is subsequently aborted, this is not logged.
> 
> This patch makes python tests always check the exit code of these
> subprocesses, and emit a message if they have been killed.
> 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/iotests.py | 26 +-
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 927c74a..d082597 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -49,20 +49,34 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 
> 'socket_scm_helper')
>  def qemu_img(*args):
>  '''Run qemu-img and return the exit code'''
>  devnull = open('/dev/null', 'r+')
> -return subprocess.call(qemu_img_args + list(args), stdin=devnull, 
> stdout=devnull)
> +exitcode = subprocess.call(qemu_img_args + list(args), stdin=devnull, 
> stdout=devnull)
> +if exitcode < 0:
> +sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' 
> '.join(qemu_img_args + list(args
> +return exitcode
>  

Why tack on the arguments after the retcode for the print? The format
makes it look like it should be a description for the signal received.

>  def qemu_img_verbose(*args):
>  '''Run qemu-img without suppressing its output and return the exit 
> code'''
> -return subprocess.call(qemu_img_args + list(args))
> +exitcode = subprocess.call(qemu_img_args + list(args))
> +if exitcode < 0:
> +sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' 
> '.join(qemu_img_args + list(args
> +return exitcode
>  
>  def qemu_img_pipe(*args):
>  '''Run qemu-img and return its output'''
> -return subprocess.Popen(qemu_img_args + list(args), 
> stdout=subprocess.PIPE).communicate()[0]
> +subp = subprocess.Popen(qemu_img_args + list(args), 
> stdout=subprocess.PIPE)
> +exitcode = subp.wait()
> +if exitcode < 0:
> +sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' 
> '.join(qemu_img_args + list(args
> +return subp.communicate()[0]
>  
>  def qemu_io(*args):
>  '''Run qemu-io and return the stdout data'''
>  args = qemu_io_args + list(args)
> -return subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0]
> +subp = subprocess.Popen(args, stdout=subprocess.PIPE)
> +exitcode = subp.wait()
> +if exitcode < 0:
> +sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
> '.join(args)))
> +return subp.communicate()[0]
>  
>  def compare_images(img1, img2):
>  '''Return True if two image files are identical'''
> @@ -197,7 +211,9 @@ class VM(object):
>  '''Terminate the VM and clean up'''
>  if not self._popen is None:
>  self._qmp.cmd('quit')
> -self._popen.wait()
> +exitcode = self._popen.wait()
> +if exitcode < 0:
> +sys.stderr.write('qemu received signal %i: %s\n' % 
> (-exitcode, ' '.join(self._args)))
>  os.remove(self._monitor_path)
>  os.remove(self._qtest_path)
>  os.remove(self._qemu_log_path)
> 

Re: [Qemu-devel] [PATCH 6/7] vhost-user: add multiple queue support

[Eric Blake]

On 09/08/2015 01:38 AM, Yuanhan Liu wrote:
> From: Ouyang Changchun 
> 
> This patch is initially based a patch from Nikolay Nikolaev.
> 
> Here is the latest version for adding vhost-user multiple queue support,
> by creating a nc and vhost_net pair for each queue.
> 

Reviewing grammar and interface only:

> +++ b/docs/specs/vhost-user.txt
> @@ -135,6 +135,19 @@ As older slaves don't support negotiating protocol 
> features,
>  a feature bit was dedicated for this purpose:
>  #define VHOST_USER_F_PROTOCOL_FEATURES 30
>  
> +Multiple queue support
> +---
> +Multiple queue is treated as a protocal extension, hence the slave has to

s/protocal/protocol/

> +implement protocol features first. Multiple queues is supported only when
> +the protocol feature VHOST_USER_PROTOCOL_F_MQ(bit 0) is set.
> +
> +The max # of queues the slave support can be queried with message

s/#/number/
s/support/supports/

> +VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the # of requested

s/#/number/

> +queues is bigger than that.
> +
> +As all queues share one connection, the master use a unique index for each

s/use/uses/

> +queue in the sent message to identify one specified queue.
> +

> +++ b/qapi-schema.json
> @@ -2480,12 +2480,16 @@
>  #
>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: 
> false).
>  #
> +# @queues: #optional number of queues to be created for multiqueue vhost-user
> +#  (default: 1) (Since 2.5)
> +#
>  # Since 2.1
>  ##
>  { 'struct': 'NetdevVhostUserOptions',
>'data': {
>  'chardev':'str',
> -'*vhostforce':'bool' } }
> +'*vhostforce':'bool',
> +'*queues':'int' } }

Looks okay.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium

[Max Reitz]

On 08.09.2015 11:13, Wen Congyang wrote:
> On 07/21/2015 01:45 AM, Max Reitz wrote:
>> And a helper function for that, which directly takes a pointer to the
>> BDS to be inserted instead of its node-name (which will be used for
>> implementing 'change' using blockdev-insert-medium).
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  blockdev.c   | 48 
>>  qapi/block-core.json | 17 +
>>  qmp-commands.hx  | 37 +
>>  3 files changed, 102 insertions(+)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 481760a..a80d0e2 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, 
>> Error **errp)
>>  }
>>  }
>>  
>> +static void qmp_blockdev_insert_anon_medium(const char *device,
>> +BlockDriverState *bs, Error 
>> **errp)
>> +{
>> +BlockBackend *blk;
>> +bool has_device;
>> +
>> +blk = blk_by_name(device);
>> +if (!blk) {
>> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> +  "Device '%s' not found", device);
>> +return;
>> +}
>> +
>> +/* For BBs without a device, we can exchange the BDS tree at will */
>> +has_device = blk_get_attached_dev(blk);
>> +
>> +if (has_device && !blk_dev_has_removable_media(blk)) {
>> +error_setg(errp, "Device '%s' is not removable", device);
>> +return;
>> +}
>> +
>> +if (has_device && !blk_dev_is_tray_open(blk)) {
>> +error_setg(errp, "Tray of device '%s' is not open", device);
>> +return;
>> +}
>> +
>> +if (blk_bs(blk)) {
>> +error_setg(errp, "There already is a medium in device '%s'", 
>> device);
>> +return;
>> +}
>> +
>> +blk_insert_bs(blk, bs);
>> +}
>> +
>> +void qmp_blockdev_insert_medium(const char *device, const char *node_name,
>> +Error **errp)
>> +{
>> +BlockDriverState *bs;
>> +
>> +bs = bdrv_find_node(node_name);
>> +if (!bs) {
>> +error_setg(errp, "Node '%s' not found", node_name);
>> +return;
>> +}
> 
> Hmm, it is OK if the bs is not top BDS?

I think so, yes. Generally, there's probably no reason to do that, but I
don't know why we should not allow that case. For instance, you might
want to make a backing file available read-only somewhere.

It should be impossible to make it available writable, and it should not
be allowed to start a block-commit operation while the backing file can
be accessed by the guest, but this should be achieved using op blockers.

What we need for this to work are fine-grained op blockers, I think. But
working around that for now by only allowing to insert top BDS won't
work, since you can still start block jobs which target top BDS, too
(e.g. blockdev-backup can write to a BDS/BB that is visible to the guest).

All in all, I think it's fine to insert non-top BDS, but we should
definitely worry about which exact BDS one can insert once we have
fine-grained op blockers.

Max

> Thanks
> Wen Congyang
> 
>> +
>> +qmp_blockdev_insert_anon_medium(device, bs, errp);
>> +}
>> +
>>  /* throttling disk I/O limits */
>>  void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t 
>> bps_rd,
>> int64_t bps_wr,
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 63a83e4..84c9b23 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1925,6 +1925,23 @@
>>  { 'command': 'blockdev-remove-medium',
>>'data': { 'device': 'str' } }
>>  
>> +##
>> +# @blockdev-insert-medium:
>> +#
>> +# Inserts a medium (a block driver state tree) into a block device. That 
>> block
>> +# device's tray must currently be open and there must be no medium inserted
>> +# already.
>> +#
>> +# @device:block device name
>> +#
>> +# @node-name: name of a node in the block driver state graph
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'command': 'blockdev-insert-medium',
>> +  'data': { 'device': 'str',
>> +'node-name': 'str'} }
>> +
>>  
>>  ##
>>  # @BlockErrorAction
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index ff6c572..b4c34fe 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -3991,6 +3991,43 @@ Example:
>>  EQMP
>>  
>>  {
>> +.name   = "blockdev-insert-medium",
>> +.args_type  = "device:s,node-name:s",
>> +.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium,
>> +},
>> +
>> +SQMP
>> +blockdev-insert-medium
>> +--
>> +
>> +Inserts a medium (a block driver state tree) into a block device. That block
>> +device's tray must currently be open and there must be no medium inserted
>> +already.
>> +
>> +Arguments:
>> +
>> +- "device": block device name (json-string)
>> +- "node-name": root node of the BDS tree to insert into the block device
>> +
>> +Example:
>> +
>> +-> { "execute": "blockd

[Qemu-devel] Mac OS 9 updates

[Programmingkid]

Are we going to see any more GSoC 2015 updates? 

[Qemu-devel] [PULL 05/20] hw/intc/arm_gic: Actually set the active bits for active interrupts

[Peter Maydell]

Although we were correctly handling interrupts becoming active
and then inactive, we weren't actually exposing this to the guest
by setting the 'active' flag for the interrupt, so reads
of GICD_ICACTIVERn and GICD_ISACTIVERn would generally incorrectly
return zeroes. Correct this oversight.

Signed-off-by: Peter Maydell 
Message-id: 1438089748-5528-6-git-send-email-peter.mayd...@linaro.org
---
 hw/intc/arm_gic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 9daa8cd..2df550c 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -262,6 +262,7 @@ static void gic_activate_irq(GICState *s, int cpu, int irq)
 }
 
 s->running_priority[cpu] = prio;
+GIC_SET_ACTIVE(irq, 1 << cpu);
 }
 
 static int gic_get_prio_from_apr_bits(GICState *s, int cpu)
@@ -536,6 +537,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq, 
MemTxAttrs attrs)
  */
 
 gic_drop_prio(s, cpu, group);
+GIC_CLEAR_ACTIVE(irq, cm);
 gic_update(s);
 }
 
-- 
1.9.1

Re: [Qemu-devel] where can i customize rbd object size?

[Josh Durgin]

On 09/03/2015 09:48 AM, Stefan Hajnoczi wrote:

On Wed, Aug 19, 2015 at 03:39:20PM +0800, Jaze Lee wrote:

Hello,

 qemu-img convert -f qcow2 Trove---mysql-5.6---2015-07-16.qcow2 -O raw
rbd:openstack-00/8205d01a-874c-44c0-b114-1c03821fcc24:conf=/etc/ceph/ceph.conf

How can i specify the object size that rbd uses?  I found that the
qemu-image can only use the default object size. It is defined in
block/rbd.c

 #define OBJ_MAX_SIZE

(1UL << OBJ_DEFAULT_OBJ_ORDER

)


Those are remnants from before librbd existed, they aren't used in qemu
anymore.


 If someone know how to specify the object size, please tell me. thanks
a lot.
 By the way i do not find the define for  OBJ_DEFAULT_OBJ_ORDER
,
if someone knows , please tell me. Thanks a lot.


I have CCed the rbd.c maintainer for you:
$ scripts/get_maintainer.pl -f block/rbd.c
Josh Durgin  (supporter:RBD)



Thanks for CCing me, I missed it before.

You can specify the rbd object size in bytes via the cluster_size
option, e.g.

qemu-img convert -f qcow2 Trove---mysql-5.6---2015-07-16.qcow2 -O raw -o 
cluster_size=8388608 
rbd:openstack-00/8205d01a-874c-44c0-b114-1c03821fcc24:conf=/etc/ceph/ceph.conf


The code handling this is here:

https://lxr.missinglinkelectronics.com/#qemu+v2.1.0/block/rbd.c#L318

Josh

Re: [Qemu-devel] [PATCH v1 05/10] target-arm: Add VTTBR_EL2

[Edgar E. Iglesias]

On Tue, Sep 08, 2015 at 03:27:05PM +0100, Peter Maydell wrote:
> On 3 September 2015 at 21:14, Edgar E. Iglesias
>  wrote:
> > From: "Edgar E. Iglesias" 
> >
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  target-arm/cpu.h|  1 +
> >  target-arm/helper.c | 34 --
> >  2 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index ba22e12..0ebdaf7 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -221,6 +221,7 @@ typedef struct CPUARMState {
> >  };
> >  uint64_t ttbr1_el[4];
> >  };
> > +uint64_t vttbr_el2; /* Virtualization Translation Table Base.  */
> >  /* MMU translation table base control. */
> >  TCR tcr_el[4];
> >  TCR vtcr_el2; /* Virtualization Translation Control.  */
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index c82aa1d..ec19e68 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -2200,6 +2200,19 @@ static void vmsa_ttbr_write(CPUARMState *env, const 
> > ARMCPRegInfo *ri,
> >  raw_write(env, ri, value);
> >  }
> >
> > +static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> > +uint64_t value)
> > +{
> > +ARMCPU *cpu = arm_env_get_cpu(env);
> > +CPUState *cs = CPU(cpu);
> > +
> > +if (raw_read(env, ri) != value) {
> > +tlb_flush_by_mmuidx(cs, ARMMMUIdx_S12NSE1, ARMMMUIdx_S12NSE0,
> > +ARMMMUIdx_S2NS, -1);
> 
> We only need the TLB flush because this could change the VMID and
> our TLB doesn't handle VMIDs, right? That could use a comment
> (compare the remark about ASIDs in vmsa_ttbr_write()).

Yes, will add a comment.


> 
> > +raw_write(env, ri, value);
> > +}
> > +}
> > +
> >  static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
> >  { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
> >.access = PL1_RW, .type = ARM_CP_ALIAS,
> > @@ -3131,6 +3144,14 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
> >.opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
> >.access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> >.readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > +{ .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
> > +  .cp = 15, .opc1 = 6, .crm = 2,
> > +  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +  .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> > +{ .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
> > +  .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
> > +  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +  .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> 
> RAZ/WI registers not using ARM_CP_CONST again...

Thanks, will fix!

Cheers,
Edgar




> 
> >  { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
> >.opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
> >.access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> > @@ -3271,6 +3292,16 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
> >.writefn = vmsa_tcr_el1_write,
> >.resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
> >.fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
> > +{ .name = "VTTBR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_64BIT,
> > +  .cp = 15, .opc1 = 6, .crm = 2,
> > +  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +  .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
> > +  .writefn = vttbr_write },
> > +{ .name = "VTTBR_EL2", .state = ARM_CP_STATE_AA64,
> > +  .opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 0,
> > +  .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
> > +  .fieldoffset = offsetof(CPUARMState, cp15.vttbr_el2),
> > +  .writefn = vttbr_write },
> >  { .name = "SCTLR_EL2", .state = ARM_CP_STATE_BOTH,
> >.opc0 = 3, .opc1 = 4, .crn = 1, .crm = 0, .opc2 = 0,
> >.access = PL2_RW, .raw_writefn = raw_write, .writefn = sctlr_write,
> > @@ -5770,8 +5801,7 @@ static inline uint64_t regime_ttbr(CPUARMState *env, 
> > ARMMMUIdx mmu_idx,
> > int ttbrn)
> >  {
> >  if (mmu_idx == ARMMMUIdx_S2NS) {
> > -/* TODO: return VTTBR_EL2 */
> > -g_assert_not_reached();
> > +return env->cp15.vttbr_el2;
> >  }
> >  if (ttbrn == 0) {
> >  return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
> > --
> > 1.9.1
> >
> 
> thanks
> -- PMM

Re: [Qemu-devel] [FIX PATCH] spapr_drc: Return correct state for logical DR in entity_sense()

[Michael Roth]

Quoting Michael Roth (2015-09-08 16:03:56)
> Quoting David Gibson (2015-09-07 20:22:50)
> > On Mon, Sep 07, 2015 at 11:37:04AM +0530, Bharata B Rao wrote:
> > > When drmgr is run in the guest to add a device for which device_add
> > > hasn't been issued in QEMU, configure-connector call fails.
> > > When configure-connector call fails, the guest would release (*)
> > > the previously acquired DRC by setting back the DRC isolation state
> > > to ISOLATED and allocation state to UNUSABLE. These calls will be issued
> > > only if get-sensor-state call returns PRESENT state. However currently for
> > > a logical DR, entity_sense() would unconditinally return UNUSABLE
> > > state only. This prevents any subsequent hotplug of the device with
> > > that DRC.
> 
> This seems a little odd. I think we default to ALLOCATION_STATE_UNUSABLE
> for logical DR, and it's up the guest to transition to USABLE, which
> probably happens prior to the configure-connector calls. So I think the
> net effect of this fix is that guest will see these unallocated/unattached
> resources the same way they would a resource that was actually attached
> via device_add, and all we're really doing is working around the
> eventual configuration failure that that will lead to by pretending a
> resource was actually there.
> 
> According to PAPR+ 2.7:
> 
> 13.7.3.1 Acquire Logical Resource from Resource Pool:
> 
>   If the state is “unusable” the OS issues set-indicator (allocation-state, 
> usable) to attempt to allocate the re-
>   source. Similarly, if the state is “available for exchange” the OS issues 
> set-indicator (allocation-state, ex-
>   change) to attempt to allocate the resource, and if the state is “available 
> for recovery” the OS issues
>   set-indicator (allocation-state, recover) to attempt to allocate the 
> resource.
> 
> and
> 
> 13.7 Logical Resource Dynamic Reconfiguration (LRDR):
> 
>   The OS may use the get-sensor-state RTAS call with the dr-entity-sense 
> token to deter-
> mine if a given drc-index refers to a connector that is currently usable for 
> DR operations. If the connector is not
> currently usable the return state is “DR entity unusable” (2). A 
> set-indicator (isolation state) RTAS call to an unusable
> connector or (dr-indicator) to any logical resource connector results in a 
> “No such indicator implemented” return sta-
> tus.  
> 
> So I think maybe the proper fix is to make sure that
> drc->set_indicator_state() fails with an error that indicates to RTAS to

drc->set_isolation_state(), I mean.

> return NO_SENSOR (-3) for cases where we haven't attached a resource
> to the DRC via device_add.
> 
> Which also kind of re-opens the discussion of whether or not
> drc->set_indicator_state() should return RTAS errors directly. I'd

and again.

> still stray away from that for now but maybe if we get more cases
> like this it'll start becoming more practical.
> 
> > > 
> > > Fix this by returning the right state in entity_sense() by checking
> > > the allocation_state of DRC.
> > > 
> > > (*) 
> > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-September/133430.html
> > > 
> > > Signed-off-by: Bharata B Rao 
> > > Cc: Michael Roth 
> > 
> > Reviewed-by: David Gibson 
> > 
> > and applied to my tree.
> > 
> > > ---
> > >  hw/ppc/spapr_drc.c | 6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > > index 9ce844a..2586065 100644
> > > --- a/hw/ppc/spapr_drc.c
> > > +++ b/hw/ppc/spapr_drc.c
> > > @@ -186,7 +186,11 @@ static sPAPRDREntitySense 
> > > entity_sense(sPAPRDRConnector *drc)
> > >   */
> > >  state = SPAPR_DR_ENTITY_SENSE_EMPTY;
> > >  } else {
> > > -state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > > +if (drc->allocation_state == 
> > > SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> > > +state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > > +} else {
> > > +state = SPAPR_DR_ENTITY_SENSE_PRESENT;
> > > +}
> > >  }
> > >  }
> > >  
> > 
> > -- 
> > David Gibson| I'll have my music baroque, and my code
> > david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
> > | _way_ _around_!
> > http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [FIX PATCH] spapr_drc: Return correct state for logical DR in entity_sense()

[Michael Roth]

Quoting David Gibson (2015-09-07 20:22:50)
> On Mon, Sep 07, 2015 at 11:37:04AM +0530, Bharata B Rao wrote:
> > When drmgr is run in the guest to add a device for which device_add
> > hasn't been issued in QEMU, configure-connector call fails.
> > When configure-connector call fails, the guest would release (*)
> > the previously acquired DRC by setting back the DRC isolation state
> > to ISOLATED and allocation state to UNUSABLE. These calls will be issued
> > only if get-sensor-state call returns PRESENT state. However currently for
> > a logical DR, entity_sense() would unconditinally return UNUSABLE
> > state only. This prevents any subsequent hotplug of the device with
> > that DRC.

This seems a little odd. I think we default to ALLOCATION_STATE_UNUSABLE
for logical DR, and it's up the guest to transition to USABLE, which
probably happens prior to the configure-connector calls. So I think the
net effect of this fix is that guest will see these unallocated/unattached
resources the same way they would a resource that was actually attached
via device_add, and all we're really doing is working around the
eventual configuration failure that that will lead to by pretending a
resource was actually there.

According to PAPR+ 2.7:

13.7.3.1 Acquire Logical Resource from Resource Pool:

  If the state is “unusable” the OS issues set-indicator (allocation-state, 
usable) to attempt to allocate the re-
  source. Similarly, if the state is “available for exchange” the OS issues 
set-indicator (allocation-state, ex-
  change) to attempt to allocate the resource, and if the state is “available 
for recovery” the OS issues
  set-indicator (allocation-state, recover) to attempt to allocate the resource.

and

13.7 Logical Resource Dynamic Reconfiguration (LRDR):

  The OS may use the get-sensor-state RTAS call with the dr-entity-sense token 
to deter-
mine if a given drc-index refers to a connector that is currently usable for DR 
operations. If the connector is not
currently usable the return state is “DR entity unusable” (2). A set-indicator 
(isolation state) RTAS call to an unusable
connector or (dr-indicator) to any logical resource connector results in a “No 
such indicator implemented” return sta-
tus.  

So I think maybe the proper fix is to make sure that
drc->set_indicator_state() fails with an error that indicates to RTAS to
return NO_SENSOR (-3) for cases where we haven't attached a resource
to the DRC via device_add.

Which also kind of re-opens the discussion of whether or not
drc->set_indicator_state() should return RTAS errors directly. I'd
still stray away from that for now but maybe if we get more cases
like this it'll start becoming more practical.

> > 
> > Fix this by returning the right state in entity_sense() by checking
> > the allocation_state of DRC.
> > 
> > (*) 
> > https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-September/133430.html
> > 
> > Signed-off-by: Bharata B Rao 
> > Cc: Michael Roth 
> 
> Reviewed-by: David Gibson 
> 
> and applied to my tree.
> 
> > ---
> >  hw/ppc/spapr_drc.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index 9ce844a..2586065 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -186,7 +186,11 @@ static sPAPRDREntitySense 
> > entity_sense(sPAPRDRConnector *drc)
> >   */
> >  state = SPAPR_DR_ENTITY_SENSE_EMPTY;
> >  } else {
> > -state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > +if (drc->allocation_state == 
> > SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> > +state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > +} else {
> > +state = SPAPR_DR_ENTITY_SENSE_PRESENT;
> > +}
> >  }
> >  }
> >  
> 
> -- 
> David Gibson| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson

[Qemu-devel] [PULL 08/20] hw/intc/arm_gic_common: Configure IRQs as NS if doing direct NS kernel boot

[Peter Maydell]

If we directly boot a kernel in NonSecure on a system where the GIC
supports the security extensions then we must cause the GIC to
configure its interrupts into group 1 (NonSecure) rather than the
usual group 0, and with their initial priority set to the highest
NonSecure priority rather than the usual highest Secure priority.
Otherwise the guest kernel will be unable to use any interrupts.

Implement this behaviour, controlled by a flag which we set if
appropriate when the ARM bootloader code calls our ARMLinuxBootIf
interface callback.

Signed-off-by: Peter Maydell 
Reviewed-by: Peter Crosthwaite 
Reviewed-by: Peter Maydell 
Reviewed-by: Edgar E. Iglesias 
Tested-by: Edgar E. Iglesias 
Message-id: 1441383782-24378-4-git-send-email-peter.mayd...@linaro.org
---
 hw/intc/arm_gic_common.c | 51 +---
 include/hw/intc/arm_gic_common.h |  1 +
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index aa0e7d8..9c82b97 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -19,6 +19,7 @@
  */
 
 #include "gic_internal.h"
+#include "hw/arm/linux-boot-if.h"
 
 static void gic_pre_save(void *opaque)
 {
@@ -164,12 +165,27 @@ static void arm_gic_common_reset(DeviceState *dev)
 {
 GICState *s = ARM_GIC_COMMON(dev);
 int i, j;
+int resetprio;
+
+/* If we're resetting a TZ-aware GIC as if secure firmware
+ * had set it up ready to start a kernel in non-secure,
+ * we need to set interrupt priorities to a "zero for the
+ * NS view" value. This is particularly critical for the
+ * priority_mask[] values, because if they are zero then NS
+ * code cannot ever rewrite the priority to anything else.
+ */
+if (s->security_extn && s->irq_reset_nonsecure) {
+resetprio = 0x80;
+} else {
+resetprio = 0;
+}
+
 memset(s->irq_state, 0, GIC_MAXIRQ * sizeof(gic_irq_state));
 for (i = 0 ; i < s->num_cpu; i++) {
 if (s->revision == REV_11MPCORE) {
 s->priority_mask[i] = 0xf0;
 } else {
-s->priority_mask[i] = 0;
+s->priority_mask[i] = resetprio;
 }
 s->current_pending[i] = 1023;
 s->running_priority[i] = 0x100;
@@ -177,7 +193,7 @@ static void arm_gic_common_reset(DeviceState *dev)
 s->bpr[i] = GIC_MIN_BPR;
 s->abpr[i] = GIC_MIN_ABPR;
 for (j = 0; j < GIC_INTERNAL; j++) {
-s->priority1[j][i] = 0;
+s->priority1[j][i] = resetprio;
 }
 for (j = 0; j < GIC_NR_SGIS; j++) {
 s->sgi_pending[j][i] = 0;
@@ -189,7 +205,7 @@ static void arm_gic_common_reset(DeviceState *dev)
 }
 
 for (i = 0; i < ARRAY_SIZE(s->priority2); i++) {
-s->priority2[i] = 0;
+s->priority2[i] = resetprio;
 }
 
 for (i = 0; i < GIC_MAXIRQ; i++) {
@@ -200,9 +216,32 @@ static void arm_gic_common_reset(DeviceState *dev)
 s->irq_target[i] = 0;
 }
 }
+if (s->security_extn && s->irq_reset_nonsecure) {
+for (i = 0; i < GIC_MAXIRQ; i++) {
+GIC_SET_GROUP(i, ALL_CPU_MASK);
+}
+}
+
 s->ctlr = 0;
 }
 
+static void arm_gic_common_linux_init(ARMLinuxBootIf *obj,
+  bool secure_boot)
+{
+GICState *s = ARM_GIC_COMMON(obj);
+
+if (s->security_extn && !secure_boot) {
+/* We're directly booting a kernel into NonSecure. If this GIC
+ * implements the security extensions then we must configure it
+ * to have all the interrupts be NonSecure (this is a job that
+ * is done by the Secure boot firmware in real hardware, and in
+ * this mode QEMU is acting as a minimalist firmware-and-bootloader
+ * equivalent).
+ */
+s->irq_reset_nonsecure = true;
+}
+}
+
 static Property arm_gic_common_properties[] = {
 DEFINE_PROP_UINT32("num-cpu", GICState, num_cpu, 1),
 DEFINE_PROP_UINT32("num-irq", GICState, num_irq, 32),
@@ -219,11 +258,13 @@ static Property arm_gic_common_properties[] = {
 static void arm_gic_common_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
+ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
 
 dc->reset = arm_gic_common_reset;
 dc->realize = arm_gic_common_realize;
 dc->props = arm_gic_common_properties;
 dc->vmsd = &vmstate_gic;
+albifc->arm_linux_init = arm_gic_common_linux_init;
 }
 
 static const TypeInfo arm_gic_common_type = {
@@ -233,6 +274,10 @@ static const TypeInfo arm_gic_common_type = {
 .class_size = sizeof(ARMGICCommonClass),
 .class_init = arm_gic_common_class_init,
 .abstract = true,
+.interfaces = (InterfaceInfo []) {
+{ TYPE_ARM_LINUX_BOOT_IF },
+{ },
+},
 };
 
 static void register_types(void)
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index b9dfe05..564a72b 100644

[Qemu-devel] [PULL 0/19] xen-2015-09-08-tag

[Stefano Stabellini]

The following changes since commit 8611280505119e296757a60711a881341603fa5a:

  target-microblaze: Use setcond for pcmp* (2015-09-08 08:49:33 +0200)

are available in the git repository at:

  git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-2015-09-08-tag

for you to fetch changes up to ba2250ad148997b1352aba976aac66b55410e7e4:

  xen/pt: Use XEN_PT_LOG properly to guard against compiler warnings. 
(2015-09-08 15:21:56 +)


Xen branch xen-2015-09-08


Don Slutz (1):
  xen-hvm: Add trace to ioreq

Jan Beulich (1):
  xen/HVM: atomically access pointers in bufioreq handling

Konrad Rzeszutek Wilk (7):
  xen-hvm: When using xc_domain_add_to_physmap also include errno when 
reporting
  xen/pt: Update comments with proper function name.
  xen/pt: Make xen_pt_msi_set_enable static
  xen/pt: xen_host_pci_config_read returns -errno, not -1 on failure
  xen: use errno instead of rc for xc_domain_add_to_physmap
  xen/pt/msi: Add the register value when printing logging and error 
messages
  xen/pt: Use XEN_PT_LOG properly to guard against compiler warnings.

Michael S. Tsirkin (1):
  i440fx: make types configurable at run-time

Tiejun Chen (9):
  pc_init1: pass parameters just with types
  piix: create host bridge to passthrough
  hw/pci-assign: split pci-assign.c
  xen, gfx passthrough: basic graphics passthrough support
  xen, gfx passthrough: retrieve VGA BIOS to work
  igd gfx passthrough: create a isa bridge
  xen, gfx passthrough: register a isa bridge
  xen, gfx passthrough: register host bridge specific to passthrough
  xen, gfx passthrough: add opregion mapping

 configure |   28 +
 hw/core/machine.c |   20 +++
 hw/i386/Makefile.objs |1 +
 hw/i386/kvm/pci-assign.c  |   82 ++---
 hw/i386/pc_piix.c |  139 -
 hw/i386/pci-assign-load-rom.c |   93 ++
 hw/pci-host/piix.c|   91 +-
 hw/xen/Makefile.objs  |1 +
 hw/xen/xen-host-pci-device.c  |5 +
 hw/xen/xen-host-pci-device.h  |1 +
 hw/xen/xen_pt.c   |   42 ++-
 hw/xen/xen_pt.h   |   22 +++-
 hw/xen/xen_pt_config_init.c   |   59 -
 hw/xen/xen_pt_graphics.c  |  272 +
 hw/xen/xen_pt_msi.c   |2 +-
 include/hw/boards.h   |1 +
 include/hw/i386/pc.h  |9 +-
 include/hw/pci/pci-assign.h   |   27 
 include/hw/xen/xen_common.h   |   34 +-
 qemu-options.hx   |3 +
 trace-events  |7 ++
 vl.c  |   10 ++
 xen-hvm.c |   55 +++--
 23 files changed, 891 insertions(+), 113 deletions(-)
 create mode 100644 hw/i386/pci-assign-load-rom.c
 create mode 100644 hw/xen/xen_pt_graphics.c
 create mode 100644 include/hw/pci/pci-assign.h

[Qemu-devel] [PATCH v2 4/6] xen: use errno instead of rc for xc_domain_add_to_physmap

[Konrad Rzeszutek Wilk]

In Xen 4.6 commit cd2f100f0f61b3f333d52d1737dd73f02daee592
"libxc: Fix do_memory_op to return negative value on errors"
made the libxc API less odd-ball: On errors, return value is
-1 and error code is in errno. On success the return value
is either 0 or an positive value.

Since we could be running with an old toolstack in which the
Exx value is in rc or the newer, we add an wrapper around
the xc_domain_add_to_physmap (called xen_xc_domain_add_to_physmap)
which will always return the EXX.

Xen 4.6 did not change the libxc functions mentioned (same parameters)
so we piggyback on the fact that Xen 4.6 has a new function:
commit 504ed2053362381ac01b98db9313454488b7db40 "tools/libxc: Expose
new hypercall xc_reserved_device_memory_map" and check for that.

Suggested-by: Stefano Stabellini 
Signed-off-by: Konrad Rzeszutek Wilk 
Reviewed-by: Stefano Stabellini 
---
 configure   | 26 ++
 include/hw/xen/xen_common.h | 22 ++
 xen-hvm.c   |  4 ++--
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index d854936..f162025 100755
--- a/configure
+++ b/configure
@@ -1889,6 +1889,32 @@ int main(void) {
   xc_gnttab_open(NULL, 0);
   xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
   xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
+  xc_reserved_device_memory_map(xc, 0, 0, 0, 0, NULL, 0);
+  return 0;
+}
+EOF
+  compile_prog "" "$xen_libs"
+then
+xen_ctrl_version=460
+xen=yes
+
+  elif
+  cat > $TMPC <
+#include 
+#include 
+#include 
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xc_interface *xc;
+  xs_daemon_open();
+  xc = xc_interface_open(0, 0, 0);
+  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+  xc_gnttab_open(NULL, 0);
+  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+  xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
   xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
   return 0;
 }
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index ed5fd3e..3cd5875 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -407,4 +407,26 @@ static inline int xen_set_ioreq_server_state(XenXC xc, 
domid_t dom,
 
 #endif
 
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 460
+static inline int xen_xc_domain_add_to_physmap(XenXC xch, uint32_t domid,
+   unsigned int space,
+   unsigned long idx,
+   xen_pfn_t gpfn)
+{
+return xc_domain_add_to_physmap(xch, domid, space, idx, gpfn);
+}
+#else
+static inline int xen_xc_domain_add_to_physmap(XenXC xch, uint32_t domid,
+   unsigned int space,
+   unsigned long idx,
+   xen_pfn_t gpfn)
+{
+/* In Xen 4.6 rc is -1 and errno contains the error value. */
+int rc = xc_domain_add_to_physmap(xch, domid, space, idx, gpfn);
+if (rc == -1)
+return errno;
+return rc;
+}
+#endif
+
 #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/xen-hvm.c b/xen-hvm.c
index 0408462..fae2d22 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -345,7 +345,7 @@ go_physmap:
 unsigned long idx = pfn + i;
 xen_pfn_t gpfn = start_gpfn + i;
 
-rc = xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn, 
idx, gpfn);
+rc = xen_xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn, 
idx, gpfn);
 if (rc) {
 DPRINTF("add_to_physmap MFN %"PRI_xen_pfn" to PFN %"
 PRI_xen_pfn" failed: %d\n", idx, gpfn, rc);
@@ -422,7 +422,7 @@ static int xen_remove_from_physmap(XenIOState *state,
 xen_pfn_t idx = start_addr + i;
 xen_pfn_t gpfn = phys_offset + i;
 
-rc = xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn, 
idx, gpfn);
+rc = xen_xc_domain_add_to_physmap(xen_xc, xen_domid, XENMAPSPACE_gmfn, 
idx, gpfn);
 if (rc) {
 fprintf(stderr, "add_to_physmap MFN %"PRI_xen_pfn" to PFN %"
 PRI_xen_pfn" failed: %d\n", idx, gpfn, rc);
-- 
2.1.0

[Qemu-devel] [PATCH v2 04/10] xen/pt: Remove XenPTReg->data field.

[Konrad Rzeszutek Wilk]

We do not want to have two entries to cache the guest configuration
registers: XenPTReg->data and dev.config. Instead we want to use
only the dev.config.

To do without much complications we rip out the ->data field
and replace it with an pointer to the dev.config. This way we
have the type-checking (uint8_t, uint16_t, etc) and as well
and pre-computed location.

Alternatively we could compute the offset in dev.config by
using the XenPTRRegInfo and XenPTRegGroup every time but
this way we have the pre-computed values.

This change also exposes some mis-use:
 - In 'xen_pt_status_reg_init' we used u32 for the Capabilities Pointer
   register, but said register is an an u16.
 - In 'xen_pt_msgdata_reg_write' we used u32 but should have only use u16.

Signed-off-by: Konrad Rzeszutek Wilk 
---
 hw/xen/xen_pt.h |  6 +++-
 hw/xen/xen_pt_config_init.c | 73 +++--
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 09358b1..4bf9e80 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -134,7 +134,11 @@ struct XenPTRegInfo {
 struct XenPTReg {
 QLIST_ENTRY(XenPTReg) entries;
 XenPTRegInfo *reg;
-uint32_t data; /* emulated value */
+union {
+uint8_t *byte;
+uint16_t *half_word;
+uint32_t *word;
+} ptr; /* pointer to dev.config. */
 };
 
 typedef const struct XenPTRegGroupInfo XenPTRegGroupInfo;
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 55be4ee..727f814 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -128,10 +128,11 @@ static int xen_pt_byte_reg_read(XenPCIPassthroughState 
*s, XenPTReg *cfg_entry,
 {
 XenPTRegInfo *reg = cfg_entry->reg;
 uint8_t valid_emu_mask = 0;
+uint8_t *data = cfg_entry->ptr.byte;
 
 /* emulate byte register */
 valid_emu_mask = reg->emu_mask & valid_mask;
-*value = XEN_PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
+*value = XEN_PT_MERGE_VALUE(*value, *data, ~valid_emu_mask);
 
 return 0;
 }
@@ -140,10 +141,11 @@ static int xen_pt_word_reg_read(XenPCIPassthroughState 
*s, XenPTReg *cfg_entry,
 {
 XenPTRegInfo *reg = cfg_entry->reg;
 uint16_t valid_emu_mask = 0;
+uint16_t *data = cfg_entry->ptr.half_word;
 
 /* emulate word register */
 valid_emu_mask = reg->emu_mask & valid_mask;
-*value = XEN_PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
+*value = XEN_PT_MERGE_VALUE(*value, *data, ~valid_emu_mask);
 
 return 0;
 }
@@ -152,10 +154,11 @@ static int xen_pt_long_reg_read(XenPCIPassthroughState 
*s, XenPTReg *cfg_entry,
 {
 XenPTRegInfo *reg = cfg_entry->reg;
 uint32_t valid_emu_mask = 0;
+uint32_t *data = cfg_entry->ptr.word;
 
 /* emulate long register */
 valid_emu_mask = reg->emu_mask & valid_mask;
-*value = XEN_PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
+*value = XEN_PT_MERGE_VALUE(*value, *data, ~valid_emu_mask);
 
 return 0;
 }
@@ -169,10 +172,11 @@ static int xen_pt_byte_reg_write(XenPCIPassthroughState 
*s, XenPTReg *cfg_entry,
 XenPTRegInfo *reg = cfg_entry->reg;
 uint8_t writable_mask = 0;
 uint8_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
+uint8_t *data = cfg_entry->ptr.byte;
 
 /* modify emulate register */
 writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
-cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
+*data = XEN_PT_MERGE_VALUE(*val, *data, writable_mask);
 
 /* create value for writing to I/O device register */
 *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
@@ -186,10 +190,11 @@ static int xen_pt_word_reg_write(XenPCIPassthroughState 
*s, XenPTReg *cfg_entry,
 XenPTRegInfo *reg = cfg_entry->reg;
 uint16_t writable_mask = 0;
 uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
+uint16_t *data = cfg_entry->ptr.half_word;
 
 /* modify emulate register */
 writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
-cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
+*data = XEN_PT_MERGE_VALUE(*val, *data, writable_mask);
 
 /* create value for writing to I/O device register */
 *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
@@ -203,10 +208,11 @@ static int xen_pt_long_reg_write(XenPCIPassthroughState 
*s, XenPTReg *cfg_entry,
 XenPTRegInfo *reg = cfg_entry->reg;
 uint32_t writable_mask = 0;
 uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
+uint32_t *data = cfg_entry->ptr.word;
 
 /* modify emulate register */
 writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
-cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
+*data = XEN_PT_MERGE_VALUE(*val, *data, writable_mask);
 
 /* create value for writing to I/O device register */
 *val = XEN_PT_MER

[Qemu-devel] [PATCH v7 4/5] qmp/hmp: Add throttle ratio to query-migrate and info migrate

[Jason J. Herne]

Report throttle percentage in info migrate and query-migrate responses when
cpu throttling is active.

Signed-off-by: Jason J. Herne 
Reviewed-by: Dr. David Alan Gilbert 
---
 hmp.c | 5 +
 migration/migration.c | 5 +
 qapi-schema.json  | 7 ++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index eb65998..36bc76e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -229,6 +229,11 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->xbzrle_cache->overflow);
 }
 
+if (info->has_x_cpu_throttle_percentage) {
+monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
+   info->x_cpu_throttle_percentage);
+}
+
 qapi_free_MigrationInfo(info);
 qapi_free_MigrationCapabilityStatusList(caps);
 }
diff --git a/migration/migration.c b/migration/migration.c
index 7708c54..b29450a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -274,6 +274,11 @@ MigrationInfo *qmp_query_migrate(Error **errp)
 info->disk->total = blk_mig_bytes_total();
 }
 
+if (cpu_throttle_active()) {
+info->has_x_cpu_throttle_percentage = true;
+info->x_cpu_throttle_percentage = cpu_throttle_get_percentage();
+}
+
 get_xbzrle_cache_stats(info);
 break;
 case MIGRATION_STATUS_COMPLETED:
diff --git a/qapi-schema.json b/qapi-schema.json
index 6eba9ed..cef20c7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -474,6 +474,10 @@
 #may be expensive, but do not actually occur during the iterative
 #migration rounds themselves. (since 1.6)
 #
+# @x-cpu-throttle-percentage: #optional percentage of time guest cpus are being
+#   throttled during auto-converge. This is only present when auto-converge
+#   has started throttling guest cpus. (Since 2.5)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationInfo',
@@ -483,7 +487,8 @@
'*total-time': 'int',
'*expected-downtime': 'int',
'*downtime': 'int',
-   '*setup-time': 'int'} }
+   '*setup-time': 'int',
+   '*x-cpu-throttle-percentage': 'int'} }
 
 ##
 # @query-migrate
-- 
1.9.1

Re: [Qemu-devel] [PATCH] typofixes - v3 - https://github.com/vlajos/misspell_fixer

[Peter Maydell]

On 8 September 2015 at 21:01, Veres Lajos  wrote:
> Signed-off-by: Veres Lajos 
> ---
>  disas/i386.c|2 +-
>  disas/s390.c|4 ++--
>  docs/specs/ppc-spapr-hcalls.txt |4 ++--
>  docs/writing-qmp-commands.txt   |2 +-
>  hw/audio/fmopl.c|2 +-
>  hw/core/qdev.c  |2 +-
>  hw/cris/axis_dev88.c|2 +-
>  hw/display/qxl-render.c |2 +-
>  hw/dma/xilinx_axidma.c  |2 +-
>  hw/input/stellaris_input.c  |2 +-
>  hw/intc/xics.c  |2 +-
>  hw/net/fsl_etsec/etsec.c|2 +-
>  hw/sd/pl181.c   |2 +-
>  hw/vfio/pci.c   |2 +-
>  hw/xen/xen-host-pci-device.c|2 +-
>  include/block/scsi.h|2 +-
>  include/exec/memory.h   |2 +-
>  libcacard/card_7816t.h  |2 +-
>  libdecnumber/decContext.c   |2 +-
>  libdecnumber/decNumber.c|8 
>  linux-user/syscall.c|2 +-
>  linux-user/syscall_defs.h   |4 ++--
>  qga/commands-posix.c|2 +-
>  target-alpha/mem_helper.c   |2 +-
>  target-arm/cpu.h|4 ++--
>  target-cris/cpu.h   |2 +-
>  target-cris/translate.c |6 +++---
>  target-mips/helper.c|2 +-
>  target-s390x/kvm.c  |4 ++--
>  target-sh4/helper.c |6 +++---
>  tcg/ia64/tcg-target.c   |2 +-
>  tcg/tcg.c   |4 ++--
>  tests/image-fuzzer/runner.py|2 +-
>  tests/qemu-iotests/041  |2 +-
>  util/qemu-thread-posix.c|2 +-
>  35 files changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/disas/i386.c b/disas/i386.c
> index 00ceca9..c63d4a0 100644
> --- a/disas/i386.c
> +++ b/disas/i386.c
> @@ -357,7 +357,7 @@ fetch_data(struct disassemble_info *info, bfd_byte *addr)
>  #define Rd { OP_R, d_mode }
>  #define Rm { OP_R, m_mode }
>  #define Ib { OP_I, b_mode }
> -#define sIb { OP_sI, b_mode }  /* sign extened byte */
> +#define sIb { OP_sI, b_mode }  /* sign extended byte */
>  #define Iv { OP_I, v_mode }
>  #define Iq { OP_I, q_mode }
>  #define Iv64 { OP_I64, v_mode }
> diff --git a/disas/s390.c b/disas/s390.c
> index 974460c..c29bc4e 100644
> --- a/disas/s390.c
> +++ b/disas/s390.c
> @@ -613,7 +613,7 @@ static const struct s390_operand s390_operands[] =
>names of the instruction format that you can find in the principals
>of operation.
> 2) the last part of the definition (y in INSTR_x_y) gives you an idea
> -  which operands the binary represenation of the instruction has.
> +  which operands the binary representation of the instruction has.
>The meanings of the letters in y are:
>a - access register
>c - control register
> @@ -627,7 +627,7 @@ static const struct s390_operand s390_operands[] =
>m - mode field, 4 bit
>0 - operand skipped.
>The order of the letters reflects the layout of the format in
> -  storage and not the order of the paramaters of the instructions.
> +  storage and not the order of the parameters of the instructions.
>The use of the letters is not a 100% match with the PoP but it is
>quite close.
>
> diff --git a/docs/specs/ppc-spapr-hcalls.txt b/docs/specs/ppc-spapr-hcalls.txt
> index 667b3fa..5bd8eab 100644
> --- a/docs/specs/ppc-spapr-hcalls.txt
> +++ b/docs/specs/ppc-spapr-hcalls.txt
> @@ -41,8 +41,8 @@ When the guest runs in "real mode" (in powerpc lingua this 
> means
>  with MMU disabled, ie guest effective == guest physical), it only
>  has access to a subset of memory and no IOs.
>
> -PAPR provides a set of hypervisor calls to perform cachable or
> -non-cachable accesses to any guest physical addresses that the
> +PAPR provides a set of hypervisor calls to perform cacheable or
> +non-cacheable accesses to any guest physical addresses that the
>  guest can use in order to access IO devices while in real mode.
>
>  This is typically used by the firmware running in the guest.
> diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
> index ab1fdd3..af73bf3 100644
> --- a/docs/writing-qmp-commands.txt
> +++ b/docs/writing-qmp-commands.txt
> @@ -122,7 +122,7 @@ There are a few things to be noticed:
>  Now a little hack is needed. As we're still using the old QMP server we need
>  to add the new command to its internal dispatch table. This step won't be
>  required in the near future. Open the qmp-commands.hx file and add the
> -following in the botton:
> +following in the bottom:

"at the bottom"

>
>  {
>  .name   = "hello-world",
> diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
> index adcef2d..6f0 100644
> --- a/hw/audio/fmopl.c
> +++ b/hw/audio/fmopl.c
> @@ -1177,7 +1177,7 @@ void OPLResetChip(FM_OPL *OPL)
> OPLWriteReg(OPL,0x03,0); /* Timer2 */
> OPLWriteReg(OPL,0x04,0); /* IRQ ma

[Qemu-devel] [PATCH] specs/vhost-user.txt: fix name of messages

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 docs/specs/vhost-user.txt | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 0322bcf..1bc6adb 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -112,19 +112,19 @@ The communication consists of master sending message 
requests and slave sending
 message replies. Most of the requests don't require replies. Here is a list of
 the ones that do:
 
- * VHOST_GET_FEATURES
- * VHOST_GET_PROTOCOL_FEATURES
- * VHOST_GET_VRING_BASE
+ * VHOST_USER_GET_FEATURES
+ * VHOST_USER_GET_PROTOCOL_FEATURES
+ * VHOST_USER_GET_VRING_BASE
 
 There are several messages that the master sends with file descriptors passed
 in the ancillary data:
 
- * VHOST_SET_MEM_TABLE
- * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
- * VHOST_SET_LOG_FD
- * VHOST_SET_VRING_KICK
- * VHOST_SET_VRING_CALL
- * VHOST_SET_VRING_ERR
+ * VHOST_USER_SET_MEM_TABLE
+ * VHOST_USER_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
+ * VHOST_USER_SET_LOG_FD
+ * VHOST_USER_SET_VRING_KICK
+ * VHOST_USER_SET_VRING_CALL
+ * VHOST_USER_SET_VRING_ERR
 
 If Master is unable to send the full message or receives a wrong reply it will
 close the connection. An optional reconnection mechanism can be implemented.
-- 
2.4.3

Re: [Qemu-devel] [PATCH 1/2] qcow2: Make size_to_clusters() return int64_t

[Max Reitz]

On 08.09.2015 22:22, Eric Blake wrote:
> On 09/08/2015 02:09 PM, Max Reitz wrote:
>> Sadly, some images may have more clusters than what can be represented
>> using a plain int. We should be prepared for that case (in
>> qcow2_check_refcounts() we actually were trying to catch that case, but
>> since size_to_clusters() truncated the returned value, that check never
>> did anything useful).
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  block/qcow2-cluster.c | 20 +++-
>>  block/qcow2.h |  2 +-
>>  2 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>> index 2975b83..a34f0b1 100644
>> --- a/block/qcow2-cluster.c
>> +++ b/block/qcow2-cluster.c
>> @@ -473,8 +473,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
>> uint64_t offset,
>>  unsigned int l2_index;
>>  uint64_t l1_index, l2_offset, *l2_table;
>>  int l1_bits, c;
>> -unsigned int index_in_cluster, nb_clusters;
>> -uint64_t nb_available, nb_needed;
>> +unsigned int index_in_cluster;
>> +uint64_t nb_available, nb_needed, nb_clusters;
> 
> Most uses are storing the results unsigned...
> 
>>  
>> -static inline int size_to_clusters(BDRVQcow2State *s, int64_t size)
>> +static inline int64_t size_to_clusters(BDRVQcow2State *s, int64_t size)
>>  {
>>  return (size + (s->cluster_size - 1)) >> s->cluster_bits;
>>  }
> 
> ...and the function itself doesn't appear to intentionally return
> negative (unless size was passed in as negative, but then that may be
> accidental).  Should it just return uint64_t instead?

It won't matter in practice because we generally don't support any
offsets bigger than INT64_MAX anyway; the @size parameter has been an
int64_t all along, too.

If I have to respin for some reason (i.e. maintainer not willing to fix
up the comment in patch 2), I'll probably change the type, though.

> At any rate, I agree that 'int' is too small, so:
> Reviewed-by: Eric Blake 

Thanks!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC 00/20] Do away with TB retranslation

[Peter Maydell]

On 8 September 2015 at 20:28, Richard Henderson  wrote:
> On 09/08/2015 12:06 PM, Peter Maydell wrote:
>> On 8 September 2015 at 20:00, Richard Henderson  wrote:
>>> On 09/08/2015 11:56 AM, Peter Maydell wrote:
 My sparc test image (which is just the 32-bit debian from
 Aurelien's website) boots fine even with this patchset...
>>>
>>> Odd, it shouldn't.  ;-)
>>>
>>> Anyway, I've just fixed the sparc problem and re-pushed the tree to
>>>
>>>   git://github.com/rth7680/qemu.git tcg-search-2
>>>
>>> for anyone who wants to do any more testing.
>>
>> ...so what was the bug? (Push doesn't seem to have made it
>> to github yet.)
>
> Err.. it has.  Tip should be 98cb3e2ecffd126177f43634b643be81bdc764e7.
> So I guess you pulled it post fix?

Yep, that's what I've been reviewing...would explain why I
couldn't get it to fall over in testing :-)

> The problem was in 12/20, "target-sparc: Remove gen_opc_jump_pc".
>
> The original was slightly off in how it was computing the npc in a delay slot.
>  The replacement keeps the dc->jump_pc array, but verifies that the value of
> dc->jump_pc[1] is as expected: jump false to next insn.  It's a smaller change
> to the translator, and easier to verify correctness.

Yeah. My r-b applies to the fixed version, not the mail on the list.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] qcow2: Make size_to_clusters() return int64_t

[Eric Blake]

On 09/08/2015 02:09 PM, Max Reitz wrote:
> Sadly, some images may have more clusters than what can be represented
> using a plain int. We should be prepared for that case (in
> qcow2_check_refcounts() we actually were trying to catch that case, but
> since size_to_clusters() truncated the returned value, that check never
> did anything useful).
> 
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-cluster.c | 20 +++-
>  block/qcow2.h |  2 +-
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 2975b83..a34f0b1 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -473,8 +473,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>  unsigned int l2_index;
>  uint64_t l1_index, l2_offset, *l2_table;
>  int l1_bits, c;
> -unsigned int index_in_cluster, nb_clusters;
> -uint64_t nb_available, nb_needed;
> +unsigned int index_in_cluster;
> +uint64_t nb_available, nb_needed, nb_clusters;

Most uses are storing the results unsigned...

>  
> -static inline int size_to_clusters(BDRVQcow2State *s, int64_t size)
> +static inline int64_t size_to_clusters(BDRVQcow2State *s, int64_t size)
>  {
>  return (size + (s->cluster_size - 1)) >> s->cluster_bits;
>  }

...and the function itself doesn't appear to intentionally return
negative (unless size was passed in as negative, but then that may be
accidental).  Should it just return uint64_t instead?

At any rate, I agree that 'int' is too small, so:
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 2/2] iotests: Add test for checking large image files

[Eric Blake]

On 09/08/2015 02:09 PM, Max Reitz wrote:
> Add a test for checking a qcow2 file with a multiple of 2^32 clusters.
> 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/138 | 73 
> ++
>  tests/qemu-iotests/138.out |  9 ++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 83 insertions(+)
>  create mode 100755 tests/qemu-iotests/138
>  create mode 100644 tests/qemu-iotests/138.out
> 

> +# Put the data cluster at a multiple of 2 TB, resulting in the image 
> apparently
> +# having a multiple of 2^32 clusters
> +# (To be more specific: It is at 32 PB)
> +poke_file "$TEST_IMG" 2048 "\x80\x80\x00\x00\x00\x00\x00\x00"
> +
> +# An offset of 32 PB results in qemu-img check having to allocate an 
> in-memory
> +# refcount table of 128 TB (16 bit refcounts, 512 byte clusters).
> +# This should be generally too much for any system and thus fail.
> +# What this test is checking is that the qcow2 driver actually tries to 
> allocate
> +# such a large amount of memory (and is consequently aborting) instead of 
> having
> +# truncated the cluster count somewhere (which would result in much less 
> memory
> +# being allocated and then a segfault occuring).

s/occuring/occurring/

With the typo fixed,
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/2] qcow2: Make size_to_clusters() return int64_t

[Max Reitz]

On 08.09.2015 22:09, Max Reitz wrote:
> Sadly, some images may have more clusters than what can be represented
> using a plain int. We should be prepared for that case (in
> qcow2_check_refcounts() we actually were trying to catch that case, but
> since size_to_clusters() truncated the returned value, that check never
> did anything useful).
> 

Cc: qemu-stable 

> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-cluster.c | 20 +++-
>  block/qcow2.h |  2 +-
>  2 files changed, 12 insertions(+), 10 deletions(-)



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 2/2] iotests: Add test for checking large image files

[Max Reitz]

Add a test for checking a qcow2 file with a multiple of 2^32 clusters.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/138 | 73 ++
 tests/qemu-iotests/138.out |  9 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 83 insertions(+)
 create mode 100755 tests/qemu-iotests/138
 create mode 100644 tests/qemu-iotests/138.out

diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138
new file mode 100755
index 000..0507ccc
--- /dev/null
+++ b/tests/qemu-iotests/138
@@ -0,0 +1,73 @@
+#!/bin/bash
+#
+# General test case for qcow2's image check
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+echo
+echo '=== Check on an image with a multiple of 2^32 clusters ==='
+echo
+
+IMGOPTS=$(_optstr_add "$IMGOPTS" "cluster_size=512") \
+_make_test_img 512
+
+# Allocate L2 table
+$QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# Put the data cluster at a multiple of 2 TB, resulting in the image apparently
+# having a multiple of 2^32 clusters
+# (To be more specific: It is at 32 PB)
+poke_file "$TEST_IMG" 2048 "\x80\x80\x00\x00\x00\x00\x00\x00"
+
+# An offset of 32 PB results in qemu-img check having to allocate an in-memory
+# refcount table of 128 TB (16 bit refcounts, 512 byte clusters).
+# This should be generally too much for any system and thus fail.
+# What this test is checking is that the qcow2 driver actually tries to 
allocate
+# such a large amount of memory (and is consequently aborting) instead of 
having
+# truncated the cluster count somewhere (which would result in much less memory
+# being allocated and then a segfault occuring).
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/138.out b/tests/qemu-iotests/138.out
new file mode 100644
index 000..3fe911f
--- /dev/null
+++ b/tests/qemu-iotests/138.out
@@ -0,0 +1,9 @@
+QA output created by 138
+
+=== Check on an image with a multiple of 2^32 clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=512
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Check failed: Cannot allocate memory
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3a6a8f0..439b1d2 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -135,3 +135,4 @@
 134 rw auto quick
 135 rw auto
 137 rw auto
+138 rw auto quick
-- 
2.5.1

[Qemu-devel] [PATCH 1/2] qcow2: Make size_to_clusters() return int64_t

[Max Reitz]

Sadly, some images may have more clusters than what can be represented
using a plain int. We should be prepared for that case (in
qcow2_check_refcounts() we actually were trying to catch that case, but
since size_to_clusters() truncated the returned value, that check never
did anything useful).

Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 20 +++-
 block/qcow2.h |  2 +-
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2975b83..a34f0b1 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -473,8 +473,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 unsigned int l2_index;
 uint64_t l1_index, l2_offset, *l2_table;
 int l1_bits, c;
-unsigned int index_in_cluster, nb_clusters;
-uint64_t nb_available, nb_needed;
+unsigned int index_in_cluster;
+uint64_t nb_available, nb_needed, nb_clusters;
 int ret;
 
 index_in_cluster = (offset >> 9) & (s->cluster_sectors - 1);
@@ -837,10 +837,10 @@ err:
  * write, but require COW to be performed (this includes yet unallocated space,
  * which must copy from the backing file)
  */
-static int count_cow_clusters(BDRVQcow2State *s, int nb_clusters,
+static int count_cow_clusters(BDRVQcow2State *s, uint64_t nb_clusters,
 uint64_t *l2_table, int l2_index)
 {
-int i;
+uint64_t i;
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_table[l2_index + i]);
@@ -960,7 +960,7 @@ static int handle_copied(BlockDriverState *bs, uint64_t 
guest_offset,
 int l2_index;
 uint64_t cluster_offset;
 uint64_t *l2_table;
-unsigned int nb_clusters;
+uint64_t nb_clusters;
 unsigned int keep_clusters;
 int ret;
 
@@ -1426,7 +1426,7 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
uint64_t cluster_offset)
  * clusters.
  */
 static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
-unsigned int nb_clusters, enum qcow2_discard_type type, bool full_discard)
+uint64_t nb_clusters, enum qcow2_discard_type type, bool full_discard)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l2_table;
@@ -1441,6 +1441,7 @@ static int discard_single_l2(BlockDriverState *bs, 
uint64_t offset,
 
 /* Limit nb_clusters to one L2 table */
 nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+assert(nb_clusters <= INT_MAX);
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t old_l2_entry;
@@ -1503,7 +1504,7 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t 
offset,
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t end_offset;
-unsigned int nb_clusters;
+uint64_t nb_clusters;
 int ret;
 
 end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
@@ -1545,7 +1546,7 @@ fail:
  * clusters.
  */
 static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
-unsigned int nb_clusters)
+uint64_t nb_clusters)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l2_table;
@@ -1560,6 +1561,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t 
offset,
 
 /* Limit nb_clusters to one L2 table */
 nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+assert(nb_clusters <= INT_MAX);
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t old_offset;
@@ -1584,7 +1586,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t 
offset,
 int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors)
 {
 BDRVQcow2State *s = bs->opaque;
-unsigned int nb_clusters;
+uint64_t nb_clusters;
 int ret;
 
 /* The zero flag is only supported by version 3 and newer */
diff --git a/block/qcow2.h b/block/qcow2.h
index 61f1b57..ce292a0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -415,7 +415,7 @@ static inline int64_t offset_into_cluster(BDRVQcow2State 
*s, int64_t offset)
 return offset & (s->cluster_size - 1);
 }
 
-static inline int size_to_clusters(BDRVQcow2State *s, int64_t size)
+static inline int64_t size_to_clusters(BDRVQcow2State *s, int64_t size)
 {
 return (size + (s->cluster_size - 1)) >> s->cluster_bits;
 }
-- 
2.5.1

[Qemu-devel] [PATCH 0/2] qcow2: Make size_to_clusters() return int64_t

[Max Reitz]

Some callers actually expected that function to return int64_t. As it
turns out, it doesn't. Fix that.


Max Reitz (2):
  qcow2: Make size_to_clusters() return int64_t
  iotests: Add test for checking large image files

 block/qcow2-cluster.c  | 20 +++--
 block/qcow2.h  |  2 +-
 tests/qemu-iotests/138 | 73 ++
 tests/qemu-iotests/138.out |  9 ++
 tests/qemu-iotests/group   |  1 +
 5 files changed, 95 insertions(+), 10 deletions(-)
 create mode 100755 tests/qemu-iotests/138
 create mode 100644 tests/qemu-iotests/138.out

-- 
2.5.1

Re: [Qemu-devel] [PATCH RFC v5 30/32] qapi: New QMP command query-qmp-schema for QMP introspection

[Eric Blake]

On 09/07/2015 04:16 AM, Markus Armbruster wrote:
> qapi/introspect.json defines the introspection schema.  It's designed
> for QMP introspection, but should do for similar uses, such as QGA.
> 
> The introspection schema does not reflect all the rules and
> restrictions that apply to QAPI schemata.  A valid QAPI schema has an
> introspection value conforming to the introspection schema, but the
> converse is not true.
> 
> Introspection lowers away a number of schema details, and makes
> implicit things explicit:
> 

> +##
> +# @SchemaInfoObjectMember
> +#
> +# An object member.
> +#
> +# @name: the member's name, as defined in the QAPI schema.
> +#
> +# @type: the name of the member's type.
> +#
> +# @default: #optional default when used as command parameter.
> +#   If absent, the parameter is mandatory.
> +#   If present, the value must be null.  The parameter is
> +#   optional, and behavior when it's missing is not specified
> +#   here.
> +#   Future extension: if present and non-null, the parameter
> +#   is optional, and defaults to this value.
> +#

> +##
> +# @SchemaInfoObjectVariant
> +#
> +# The variant members for a value of the type tag.
> +#
> +# @case: a value of the type tag.
> +#
> +# @type: the name of the object type that provides the variant members
> +# when the type tag has value @case.

You aren't consistent on whether secondary lines describing the same
@variable are indented or flush left.  I don't care enough to hold up
review, but just pointing it out in case you want to reflow some text.

I've finished re-reading 31 and 32, and double-checking that the
combined text of all three patches together makes sense as a whole.
Looks like we're ready for this series to come out of RFC soon :)

And I'll start rebasing and posting my followup patches that have
already been on list...

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] typofixes - v3 - https://github.com/vlajos/misspell_fixer

[Veres Lajos]

Signed-off-by: Veres Lajos 
---
 disas/i386.c|2 +-
 disas/s390.c|4 ++--
 docs/specs/ppc-spapr-hcalls.txt |4 ++--
 docs/writing-qmp-commands.txt   |2 +-
 hw/audio/fmopl.c|2 +-
 hw/core/qdev.c  |2 +-
 hw/cris/axis_dev88.c|2 +-
 hw/display/qxl-render.c |2 +-
 hw/dma/xilinx_axidma.c  |2 +-
 hw/input/stellaris_input.c  |2 +-
 hw/intc/xics.c  |2 +-
 hw/net/fsl_etsec/etsec.c|2 +-
 hw/sd/pl181.c   |2 +-
 hw/vfio/pci.c   |2 +-
 hw/xen/xen-host-pci-device.c|2 +-
 include/block/scsi.h|2 +-
 include/exec/memory.h   |2 +-
 libcacard/card_7816t.h  |2 +-
 libdecnumber/decContext.c   |2 +-
 libdecnumber/decNumber.c|8 
 linux-user/syscall.c|2 +-
 linux-user/syscall_defs.h   |4 ++--
 qga/commands-posix.c|2 +-
 target-alpha/mem_helper.c   |2 +-
 target-arm/cpu.h|4 ++--
 target-cris/cpu.h   |2 +-
 target-cris/translate.c |6 +++---
 target-mips/helper.c|2 +-
 target-s390x/kvm.c  |4 ++--
 target-sh4/helper.c |6 +++---
 tcg/ia64/tcg-target.c   |2 +-
 tcg/tcg.c   |4 ++--
 tests/image-fuzzer/runner.py|2 +-
 tests/qemu-iotests/041  |2 +-
 util/qemu-thread-posix.c|2 +-
 35 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/disas/i386.c b/disas/i386.c
index 00ceca9..c63d4a0 100644
--- a/disas/i386.c
+++ b/disas/i386.c
@@ -357,7 +357,7 @@ fetch_data(struct disassemble_info *info, bfd_byte *addr)
 #define Rd { OP_R, d_mode }
 #define Rm { OP_R, m_mode }
 #define Ib { OP_I, b_mode }
-#define sIb { OP_sI, b_mode }  /* sign extened byte */
+#define sIb { OP_sI, b_mode }  /* sign extended byte */
 #define Iv { OP_I, v_mode }
 #define Iq { OP_I, q_mode }
 #define Iv64 { OP_I64, v_mode }
diff --git a/disas/s390.c b/disas/s390.c
index 974460c..c29bc4e 100644
--- a/disas/s390.c
+++ b/disas/s390.c
@@ -613,7 +613,7 @@ static const struct s390_operand s390_operands[] =
   names of the instruction format that you can find in the principals
   of operation.
2) the last part of the definition (y in INSTR_x_y) gives you an idea
-  which operands the binary represenation of the instruction has.
+  which operands the binary representation of the instruction has.
   The meanings of the letters in y are:
   a - access register
   c - control register
@@ -627,7 +627,7 @@ static const struct s390_operand s390_operands[] =
   m - mode field, 4 bit
   0 - operand skipped.
   The order of the letters reflects the layout of the format in
-  storage and not the order of the paramaters of the instructions.
+  storage and not the order of the parameters of the instructions.
   The use of the letters is not a 100% match with the PoP but it is
   quite close.

diff --git a/docs/specs/ppc-spapr-hcalls.txt b/docs/specs/ppc-spapr-hcalls.txt
index 667b3fa..5bd8eab 100644
--- a/docs/specs/ppc-spapr-hcalls.txt
+++ b/docs/specs/ppc-spapr-hcalls.txt
@@ -41,8 +41,8 @@ When the guest runs in "real mode" (in powerpc lingua this 
means
 with MMU disabled, ie guest effective == guest physical), it only
 has access to a subset of memory and no IOs.

-PAPR provides a set of hypervisor calls to perform cachable or
-non-cachable accesses to any guest physical addresses that the
+PAPR provides a set of hypervisor calls to perform cacheable or
+non-cacheable accesses to any guest physical addresses that the
 guest can use in order to access IO devices while in real mode.

 This is typically used by the firmware running in the guest.
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index ab1fdd3..af73bf3 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -122,7 +122,7 @@ There are a few things to be noticed:
 Now a little hack is needed. As we're still using the old QMP server we need
 to add the new command to its internal dispatch table. This step won't be
 required in the near future. Open the qmp-commands.hx file and add the
-following in the botton:
+following in the bottom:

 {
 .name   = "hello-world",
diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c
index adcef2d..6f0 100644
--- a/hw/audio/fmopl.c
+++ b/hw/audio/fmopl.c
@@ -1177,7 +1177,7 @@ void OPLResetChip(FM_OPL *OPL)
OPLWriteReg(OPL,0x03,0); /* Timer2 */
OPLWriteReg(OPL,0x04,0); /* IRQ mask clear */
for(i = 0xff ; i >= 0x20 ; i-- ) OPLWriteReg(OPL,i,0);
-   /* reset OPerator paramater */
+   /* reset OPerator parameter */
for( c = 0 ; c < OPL->max_ch ; c++ )
{
OPL_CH *CH = &OPL->P_CH[c];
diff --git a/hw/core/qdev.c b/h

[Qemu-devel] [PULL 15/20] target-arm: Add AArch64 access to PAR_EL1

[Peter Maydell]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Alistair Francis 
Message-id: 1441311266-8644-4-git-send-email-edgar.igles...@gmail.com
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 target-arm/helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 38a05e1..fc4b65f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2993,6 +2993,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
 { .name = "AT_S1E3W", .state = ARM_CP_STATE_AA64,
   .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 1,
   .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },
+{ .name = "PAR_EL1", .state = ARM_CP_STATE_AA64,
+  .type = ARM_CP_ALIAS,
+  .opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0,
+  .access = PL1_RW, .resetvalue = 0,
+  .fieldoffset = offsetof(CPUARMState, cp15.par_el[1]),
+  .writefn = par_write },
 #endif
 /* TLB invalidate last level of translation table walk */
 { .name = "TLBIMVALIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 5,
-- 
1.9.1

[Qemu-devel] [PATCH v2 05/10] xen/pt: Log xen_host_pci_get in two init functions

[Konrad Rzeszutek Wilk]

To help with troubleshooting in the field.

Acked-by: Stefano Stabellini 
Signed-off-by: Konrad Rzeszutek Wilk 
---
 hw/xen/xen_pt_config_init.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 727f814..67ecc71 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1791,6 +1791,8 @@ static int xen_pt_ptr_reg_init(XenPCIPassthroughState *s,
 rc = xen_host_pci_get_byte(&s->real_device,
reg_field + PCI_CAP_LIST_ID, &cap_id);
 if (rc) {
+XEN_PT_ERR(&s->dev, "Failed to read capability @0x%x 
(rc:%d)\n",
+   reg_field + PCI_CAP_LIST_ID, rc);
 return rc;
 }
 if (xen_pt_emu_reg_grps[i].grp_id == cap_id) {
@@ -1990,6 +1992,9 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
   reg_grp_offset,
   ®_grp_entry->size);
 if (rc < 0) {
+XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld, type=0x%x, 
rc:%d\n",
+   i, ARRAY_SIZE(xen_pt_emu_reg_grps),
+   xen_pt_emu_reg_grps[i].grp_type, rc);
 xen_pt_config_delete(s);
 return rc;
 }
@@ -2004,6 +2009,10 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
 /* initialize capability register */
 rc = xen_pt_config_reg_init(s, reg_grp_entry, regs);
 if (rc < 0) {
+XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld reg 
0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
+   j, 
ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
+   regs->offset, 
xen_pt_emu_reg_grps[i].grp_type,
+   i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
 xen_pt_config_delete(s);
 return rc;
 }
-- 
2.1.0