Re: [Qemu-devel] [PATCH RFC v3 13/14] intel_iommu: allow dynamic switch of IOMMU region

[Peter Xu]

On Tue, Jan 17, 2017 at 08:46:04AM -0700, Alex Williamson wrote:
> On Tue, 17 Jan 2017 22:00:00 +0800
> Peter Xu  wrote:
> 
> > On Mon, Jan 16, 2017 at 12:53:57PM -0700, Alex Williamson wrote:
> > > On Fri, 13 Jan 2017 11:06:39 +0800
> > > Peter Xu  wrote:
> > >   
> > > > This is preparation work to finally enabled dynamic switching ON/OFF for
> > > > VT-d protection. The old VT-d codes is using static IOMMU address space,
> > > > and that won't satisfy vfio-pci device listeners.
> > > > 
> > > > Let me explain.
> > > > 
> > > > vfio-pci devices depend on the memory region listener and IOMMU replay
> > > > mechanism to make sure the device mapping is coherent with the guest
> > > > even if there are domain switches. And there are two kinds of domain
> > > > switches:
> > > > 
> > > >   (1) switch from domain A -> B
> > > >   (2) switch from domain A -> no domain (e.g., turn DMAR off)
> > > > 
> > > > Case (1) is handled by the context entry invalidation handling by the
> > > > VT-d replay logic. What the replay function should do here is to replay
> > > > the existing page mappings in domain B.  
> > > 
> > > There's really 2 steps here, right?  Invalidate A, replay B.  I think
> > > the code handles this, but I want to make sure.  We don't want to end
> > > up with a superset of both A & B.  
> > 
> > First of all, this discussion should be beyond this patch's scope,
> > since this patch is currently only handling the case when guest
> > disables DMAR in general.
> > 
> > Then, my understanding for above question: when we do A -> B domain
> > switch, guest will not send specific context entry invalidations for
> > A, but will for sure send one when context entry is ready for B. In
> > that sense, IMO we don't have a clear "two steps", only one, which is
> > the latter "replay B". We do correct unmap based on the PSIs
> > (page-selective invalidations) of A when guest unmaps the pages in A.
> > 
> > So, for the use case of nested device assignment (which is the goal of
> > this series for now):
> > 
> > - L1 guest put device D1,D2,... of L2 guest into domain A
> > - L1 guest map the L2 memory into L1 address space (L2GPA -> L1GPA)
> > - ... (L2 guest runs, until it stops running)
> > - L1 guest unmap all the pages in domain A
> > - L1 guest move device D1,D2,... of L2 guest outside domain A
> > 
> > This series should work for above, since before any device leaves its
> > domain, the domain will be clean and without unmapped pages.
> > 
> > However, if we have the following scenario (which I don't know whether
> > this's achievable):
> > 
> > - guest iommu domain A has device D1, D2
> > - guest iommu domain B has device D3
> > - move device D2 from domain A into B
> > 
> > Here when D2 move from A to B, IIUC our current Linux IOMMU driver
> > code will not send any PSI (page-selected invalidations) for D2 or
> > domain A because domain A still has device in it, guest should only
> > send a context entry invalidation for device D2, telling that D2 has
> > switched to domain B. In that case, I am not sure whether current
> > series can work properly, and IMHO we may need to have the domain
> > knowledge in VT-d emulation code (while we don't have it yet) in the
> > future to further support this kind of domain switches.
> 
> This is a serious issue that needs to be resolved.  The context entry
> invalidation when D2 is switched from A->B must unmap anything from
> domain A before the replay of domain B.  Your example is easily
> achieved, for instance what if domain A is the SI (static identity)
> domain for the L1 guest, domain B is the device assignment domain for
> the L2 guest with current device D3.  The user hot adds device D2 into
> the L2 guest moving it from the L1 SI domain to the device assignment
> domain.  vfio will not override existing mappings on replay, it will
> error, giving the L2 guest a device with access to the static identity
> mappings of the L1 host.  This isn't acceptable.
>  
> > > On the invalidation, a future optimization when disabling an entire
> > > memory region might also be to invalidate the entire range at once
> > > rather than each individual mapping within the range, which I think is
> > > what happens now, right?  
> > 
> > Right. IIUC this can be an enhancement to current page walk logic - we
> > can coalesce continuous IOTLB with same property and notify only once
> > for these coalesced entries.
> > 
> > Noted in my todo list.
> 
> A context entry invalidation as in the example above might make use of
> this to skip any sort of page walk logic, simply invalidate the entire
> address space.

Alex, I got one more thing to ask:

I was trying to invalidate the entire address space by sending a big
IOTLB notification to vfio-pci, which looks like:

  IOMMUTLBEntry entry = {
  .target_as = _space_memory,
  .iova = 0,
  .translated_addr = 0,
  .addr_mask = (1 << 63) - 1,
  .perm = IOMMU_NONE, /* UNMAP */
  };

Re: [Qemu-devel] [PATCH v4 1/2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Yang Ziyue]

Got it. Thanks!

2017-01-18 15:48 GMT+08:00 Thomas Huth :
>  Hi,
>
> On 18.01.2017 08:43, Yang Ziyue wrote:
>> Hi Thomas,
>>
>> I'm not quite sure about that, but did you mean that I should mention
>> the second patch before the Sign-off line instead of in the commit
>> message?
>
> No, I meant: You still got the sentence "Also some updates from
> fprintf(stderr, ...) to error_report." in the description of your first
> patch. But indeed this is now done in the second patch instead. So
> simply remove that sentence from the description of your first patch,
> and everything is fine :-)
>
>  Thomas
>

Re: [Qemu-devel] [PATCH v4 1/2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Thomas Huth]

 Hi,

On 18.01.2017 08:43, Yang Ziyue wrote:
> Hi Thomas,
> 
> I'm not quite sure about that, but did you mean that I should mention
> the second patch before the Sign-off line instead of in the commit
> message?

No, I meant: You still got the sentence "Also some updates from
fprintf(stderr, ...) to error_report." in the description of your first
patch. But indeed this is now done in the second patch instead. So
simply remove that sentence from the description of your first patch,
and everything is fine :-)

 Thomas

Re: [Qemu-devel] [PATCH v4 1/2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Yang Ziyue]

Hi Thomas,

I'm not quite sure about that, but did you mean that I should mention
the second patch before the Sign-off line instead of in the commit
message?
Thanks!

Ziyue Yang

Re: [Qemu-devel] [PATCH RFC v3 00/14] VT-d: vfio enablement and misc enhances

[Peter Xu]

On Tue, Jan 17, 2017 at 05:07:27PM +0200, Michael S. Tsirkin wrote:
> On Sat, Jan 14, 2017 at 10:59:58AM +0800, Peter Xu wrote:
> > On Fri, Jan 13, 2017 at 05:58:02PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Jan 13, 2017 at 11:06:26AM +0800, Peter Xu wrote:
> > > > v3:
> > > > - fix style error reported by patchew
> > > > - fix comment in domain switch patch: use "IOMMU address space" rather
> > > >   than "IOMMU region" [Kevin]
> > > > - add ack-by for Paolo in patch:
> > > >   "memory: add section range info for IOMMU notifier"
> > > >   (this is seperately collected besides this thread)
> > > > - remove 3 patches which are merged already (from Jason)
> > > > - rebase to master b6c0897
> > > 
> > > So 1-6 look like nice cleanups to me. Should I merge them now?
> > 
> > That'll be great if you'd like to merge them. Then I can further
> > shorten this series for the next post.
> > 
> > Regarding to the error_report() issue that Jason has mentioned, I can
> > touch them up in the future when needed - after all, most of the patch
> > content are about converting DPRINT()s into traces.
> > 
> > Thanks!
> > 
> > -- peterx
> 
> I think I agree with Jason, it's best not to have guest behaviour
> trigger error_report. So pls address and I'll merge.

Will fix. Thanks,

-- peterx

Re: [Qemu-devel] commit : "virtio: set ISR on dataplane notifications" breaks virtio-pci network on linux guest.

[Brad Campbell]

On 18/01/17 10:21, Brad Campbell wrote:

G'day all,

Upgrading from 2.7.1 to 2.8 on my machines here leaves the guests
without networking. All guests are using a vanilla self-compiled 4.8.12
kernel. The host is using a 4.9.3 kernel.

Guests are using virtio networking into a bridge on the host.



Further testing indicates this only breaks when CONFIG_GENERIC_MSI_IRQ 
is not set in the guest kernel. My guest kernel didn't have this 
enabled. Once I recompiled and re-deployed the kernels, all linux VM's 
networked fine.


I can however confirm it breaks Windows XP 32 bit guests. Upgrading to 
the latest driver (51.74.104.13000) does not solve the problem either, 
so I've had to go back to 2.7.1 to be able to use these Windows VM's.


When I get some time I'll try and set up a test environment outside of 
libvirt and run further tests.


Regards,
Brad

Re: [Qemu-devel] [PATCH RFC v2 2/6] replication: add shared-disk and shared-disk-id options

[Hailiang Zhang]

On 2017/1/17 19:25, Stefan Hajnoczi wrote:

On Mon, Dec 05, 2016 at 04:35:00PM +0800, zhanghailiang wrote:

@@ -85,6 +99,9 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
  QemuOpts *opts = NULL;
  const char *mode;
  const char *top_id;
+const char *shared_disk_id;
+BlockBackend *blk;
+BlockDriverState *tmp_bs;

  ret = -EINVAL;
  opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
@@ -119,6 +136,25 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
 "The option mode's value should be primary or secondary");
  goto fail;
  }
+s->is_shared_disk = qemu_opt_get_bool(opts, REPLICATION_SHARED_DISK,
+  false);
+if (s->is_shared_disk && (s->mode == REPLICATION_MODE_PRIMARY)) {
+shared_disk_id = qemu_opt_get(opts, REPLICATION_SHARED_DISK_ID);
+if (!shared_disk_id) {
+error_setg(_err, "Missing shared disk blk");
+goto fail;
+}
+s->shared_disk_id = g_strdup(shared_disk_id);
+blk = blk_by_name(s->shared_disk_id);
+if (!blk) {
+g_free(s->shared_disk_id);
+error_setg(_err, "There is no %s block", s->shared_disk_id);
+goto fail;


Please move the g_free() to the fail label to prevent future code
changes from introducing a memory leak.



OK, I will fix it in next version, thanks very much.

Re: [Qemu-devel] [PATCH RFC v2 2/6] replication: add shared-disk and shared-disk-id options

[Hailiang Zhang]

On 2016/12/6 0:22, Eric Blake wrote:

On 12/05/2016 02:35 AM, zhanghailiang wrote:

We use these two options to identify which disk is
shared

Signed-off-by: zhanghailiang 
Signed-off-by: Wen Congyang 
Signed-off-by: Zhang Chen 
---
v2:
- add these two options for BlockdevOptionsReplication to support
   qmp blockdev-add command.
- fix a memory leak found by Changlong
---



+++ b/qapi/block-core.json
@@ -2232,12 +2232,19 @@
  #  node who owns the replication node chain. Must not be given in
  #  primary mode.
  #
+# @shared-disk-id: #optional The id of shared disk while in replication mode.
+#
+# @shared-disk: #optional To indicate whether or not a disk is shared by
+#   primary VM and secondary VM.


Both of these need a '(since 2.9)' designation since they've missed 2.8,
and could probably also use documentation on the default value assumed
when the parameter is omitted.



OK, i will add it in next version, thanks.


+#
  # Since: 2.8
  ##
  { 'struct': 'BlockdevOptionsReplication',
'base': 'BlockdevOptionsGenericFormat',
'data': { 'mode': 'ReplicationMode',
-'*top-id': 'str' } }
+'*top-id': 'str',
+'*shared-disk-id': 'str',
+'*shared-disk': 'bool' } }

  ##
  # @NFSTransport

Re: [Qemu-devel] [PATCH RFC v2 4/6] replication: fix code logic with the new shared_disk option

[Hailiang Zhang]

On 2016/12/20 20:42, Changlong Xie wrote:

On 12/05/2016 04:35 PM, zhanghailiang wrote:

Some code logic only be needed in non-shared disk, here
we adjust these codes to prepare for shared disk scenario.

Signed-off-by: zhanghailiang 
---
   block/replication.c | 47 ---
   1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/block/replication.c b/block/replication.c
index 35e9ab3..6574cc2 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -531,21 +531,28 @@ static void replication_start(ReplicationState *rs, 
ReplicationMode mode,
   aio_context_release(aio_context);
   return;
   }
-bdrv_op_block_all(top_bs, s->blocker);
-bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);

-job = backup_job_create(NULL, s->secondary_disk->bs, 
s->hidden_disk->bs,
-0, MIRROR_SYNC_MODE_NONE, NULL, false,
+/*
+ * Only in the case of non-shared disk,
+ * the backup job is in the secondary side
+ */
+if (!s->is_shared_disk) {
+bdrv_op_block_all(top_bs, s->blocker);
+bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
+job = backup_job_create(NULL, s->secondary_disk->bs,
+s->hidden_disk->bs, 0,
+MIRROR_SYNC_MODE_NONE, NULL, false,
   BLOCKDEV_ON_ERROR_REPORT,
   BLOCKDEV_ON_ERROR_REPORT, BLOCK_JOB_INTERNAL,
   backup_job_completed, bs, NULL, _err);


Coding style here.



Will fix it in next version, thanks.


-if (local_err) {
-error_propagate(errp, local_err);
-backup_job_cleanup(bs);
-aio_context_release(aio_context);
-return;
+if (local_err) {
+error_propagate(errp, local_err);
+backup_job_cleanup(bs);
+aio_context_release(aio_context);
+return;
+}
+block_job_start(job);
   }
-block_job_start(job);

   secondary_do_checkpoint(s, errp);
   break;
@@ -575,14 +582,16 @@ static void replication_do_checkpoint(ReplicationState 
*rs, Error **errp)
   case REPLICATION_MODE_PRIMARY:
   break;
   case REPLICATION_MODE_SECONDARY:
-if (!s->secondary_disk->bs->job) {
-error_setg(errp, "Backup job was cancelled unexpectedly");
-break;
-}
-backup_do_checkpoint(s->secondary_disk->bs->job, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-break;
+if (!s->is_shared_disk) {
+if (!s->secondary_disk->bs->job) {
+error_setg(errp, "Backup job was cancelled unexpectedly");
+break;
+}
+backup_do_checkpoint(s->secondary_disk->bs->job, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+break;
+}
   }
   secondary_do_checkpoint(s, errp);
   break;
@@ -663,7 +672,7 @@ static void replication_stop(ReplicationState *rs, bool 
failover, Error **errp)
* before the BDS is closed, because we will access hidden
* disk, secondary disk in backup_job_completed().
*/
-if (s->secondary_disk->bs->job) {
+if (!s->is_shared_disk && s->secondary_disk->bs->job) {
   block_job_cancel_sync(s->secondary_disk->bs->job);
   }






.

Re: [Qemu-devel] [PATCH RFC v2 5/6] replication: Implement block replication for shared disk case

[Hailiang Zhang]

Hi Stefan,

On 2017/1/17 21:19, Stefan Hajnoczi wrote:

On Mon, Dec 05, 2016 at 04:35:03PM +0800, zhanghailiang wrote:

@@ -663,8 +695,12 @@ static void replication_stop(ReplicationState *rs, bool 
failover, Error **errp)

  switch (s->mode) {
  case REPLICATION_MODE_PRIMARY:
-s->replication_state = BLOCK_REPLICATION_DONE;
-s->error = 0;
+if (s->is_shared_disk && s->primary_disk->bs->job) {
+block_job_cancel(s->primary_disk->bs->job);


Should this be block_job_cancel_sync()?



No, here it is different from the secondary side which needs to wait
until backup job been canceled before resumes to run (Or there will be
an error, https://patchwork.kernel.org/patch/9128841/).

For primary VM, Just as you can see the design scenario in patch 1,
It accesses the shared disk directly, the backup job whose source side
is just the shared disk does not influence primary VM's running,
So IMHO, it is safe to call block_job_cancel here.

Thanks,
Hailiang



+} else {
+s->replication_state = BLOCK_REPLICATION_DONE;
+s->error = 0;
+}
  break;
  case REPLICATION_MODE_SECONDARY:
  /*
--
1.8.3.1

Re: [Qemu-devel] [PATCH v9 04/11] msix: check msix_init's return value

[Cao jin]

On 01/18/2017 12:01 AM, Michael S. Tsirkin wrote:
> On Tue, Jan 17, 2017 at 02:50:38PM +0800, Cao jin wrote:
>> forget to cc maintainers in this new patch
>>
>> On 01/17/2017 02:18 PM, Cao jin wrote:
>>> Doesn't do it for megasas & hcd-xhci, later patches will fix them.
>>>
>>> Signed-off-by: Cao jin 
> 
> I don't like this one, frankly. That's a bunch of code duplication.

Yes, code duplication, seems inevitable if move the asserts into a
separate patch.

> I suspect vfio is the only one who might reasonably get EINVAL here.
> So how about e.g. msix_validate_and_init that doesn't assert and use that
> from vfio, then switch msix_init to assert instead?
> 

Not sure if I get your idea. Do you mean: do param check via assert in
msix_init(), so that no need check its returned error outside, and
introduce new api msix_validate_and_init(same content as msix_init,
except param check) dedicated to vfio?

If I understand you right, the way we do param check for msi_init[*] &
msix_init will be inconsistent.

[*] patch: msi_init: convert assert to return -errno

-- 
Sincerely,
Cao jin

Re: [Qemu-devel] [PATCH v4 2/2] gdbstub.c: update old error report statements

[Thomas Huth]

On 18.01.2017 04:22, Ziyue Yang wrote:
> From: Ziyue Yang 
> 
> Some updates from fprintf(stderr, ...) to error_report.
> 
> Signed-off-by: Ziyue Yang 
> ---
>  gdbstub.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 426d55e..fe1d0f8 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -637,8 +637,8 @@ void gdb_register_coprocessor(CPUState *cpu,
>  *p = s;
>  if (g_pos) {
>  if (g_pos != s->base_reg) {
> -fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
> -"Expected %d got %d\n", xml, g_pos, s->base_reg);
> +error_report("Error: Bad gdb register numbering for '%s', "
> + "Expected %d got %d", xml, g_pos, s->base_reg);

Since the message is now printed in one line with a comma in between,
I'd maybe rather write "expected" without capital letter.

 Thomas

Re: [Qemu-devel] [PATCH v4 1/2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Thomas Huth]

On 18.01.2017 04:21, Ziyue Yang wrote:
> From: Ziyue Yang 
> 
> This patch is to fix the segmentation fault caused by attaching
> GDB to a QEMU instance initialized with "-M none" option.
> 
> The bug can be reproduced by
> 
>> ./qemu-system-x86_64 -M none -nographic -S -s
> 
> and attach a GDB to it by
> 
>> gdb -ex 'target remote :1234
> 
> The segmentation fault was originally caused by trying to read
> the information about CPU when communicating with GDB. However,
> it's impossible for any control flow to exist on an empty machine,
> nor can CPU's be hot plugged to an empty machine later by QOM
> commands. So I think simply disabling GDB connections on empty
> machines makes sense.
> 
> Also some updates from fprintf(stderr, ...) to error_report.

Seems like that last sentence is a left-over from your other patch and
should be removed from this patch description here.

> Signed-off-by: Ziyue Yang 
> ---
>  gdbstub.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index de62d26..426d55e 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -18,6 +18,7 @@
>   */
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "qemu/error-report.h"
>  #include "qemu/cutils.h"
>  #include "cpu.h"
>  #ifdef CONFIG_USER_ONLY
> @@ -1731,6 +1732,12 @@ int gdbserver_start(const char *device)
>  CharDriverState *mon_chr;
>  ChardevCommon common = { 0 };
> 
> +if (!first_cpu) {
> +error_report("gdbstub: meaningless to attach gdb to a "
> + "machine without any CPU.");
> +return -1;
> +}
> +
>  if (!device)
>  return -1;
>  if (strcmp(device, "none") != 0) {
> --
> 2.7.4
> 

With the "fprintf" sentence removed:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH RFC v2 09/12] vfio/ccw: get irqs info and set the eventfd fd

[Dong Jia Shi]

* Alex Williamson  [2017-01-17 15:53:13 -0700]:

> On Thu, 12 Jan 2017 08:25:10 +0100
> Dong Jia Shi  wrote:
> 
> > From: Xiao Feng Ren 
> > 
> > vfio-ccw resorts to the eventfd mechanism to communicate with userspace.
> > We fetch the irqs info via the ioctl VFIO_DEVICE_GET_IRQ_INFO,
> > register a event notifier to get the eventfd fd which is sent
> > to kernel via the ioctl VFIO_DEVICE_SET_IRQS, then we can implement
> > read operation once kernel sends the signal.
> > 
> > Signed-off-by: Xiao Feng Ren 
> > ---
> >  hw/vfio/ccw.c | 102 
> > ++
> >  1 file changed, 102 insertions(+)
> > 
> > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> > index 93394c2..c6bfce7 100644
> > --- a/hw/vfio/ccw.c
> > +++ b/hw/vfio/ccw.c
> > @@ -21,6 +21,7 @@
> >  #include "hw/vfio/vfio-common.h"
> >  #include "hw/s390x/s390-ccw.h"
> >  #include "hw/s390x/ccw-device.h"
> > +#include "qemu/error-report.h"
> >  #include "standard-headers/asm-s390/vfio_ccw.h"
> >  
> >  #define TYPE_VFIO_CCW "vfio-ccw"
> > @@ -30,6 +31,7 @@ typedef struct VFIOCCWDevice {
> >  uint64_t io_region_size;
> >  uint64_t io_region_offset;
> >  struct ccw_io_region *io_region;
> > +EventNotifier io_notifier;
> >  } VFIOCCWDevice;
> >  
> >  static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> > @@ -54,6 +56,98 @@ static void vfio_ccw_reset(DeviceState *dev)
> >  ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
> >  }
> >  
> > +static void vfio_ccw_io_notifier_handler(void *opaque)
> > +{
> > +VFIOCCWDevice *vcdev = opaque;
> > +
> > +if (!event_notifier_test_and_clear(>io_notifier)) {
> > +return;
> > +}
> > +}
> > +
> > +static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error 
> > **errp)
> > +{
> > +VFIODevice *vdev = >vdev;
> > +struct vfio_irq_info *irq_info;
> > +struct vfio_irq_set *irq_set;
> > +size_t argsz;
> > +int32_t *pfd;
> > +
> > +if (vdev->num_irqs != VFIO_CCW_NUM_IRQS) {
> > +error_setg(errp, "vfio: unexpected number of io irqs %u",
> > +   vdev->num_irqs);
> > +return;
> > +}
> 
> Same issue here as with region info, we may find reasons to add new
> interrupts in the kernel, it's happened for PCI support with things
> like device error recovery and requests to release the device.  Don't
> lock us into never expanding these, just make sure that the index that
> this implementation depends on is here.
> 
Ok. Will change it to:
if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) {
... ...
}

> > +
> > +argsz = sizeof(*irq_set);
> > +irq_info = g_malloc0(argsz);
> > +irq_info->index = VFIO_CCW_IO_IRQ_INDEX;
> > +irq_info->argsz = argsz;
> > +if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
> > +  irq_info) < 0 || irq_info->count < 1) {
> > +error_setg(errp, "vfio: Error getting irq info");
> > +goto get_error;
> > +}
> > +
> > +if (event_notifier_init(>io_notifier, 0)) {
> > +error_setg(errp, "vfio: Unable to init event notifier for IO");
> > +goto get_error;
> > +}
> > +
> > +argsz = sizeof(*irq_set) + sizeof(*pfd);
> > +irq_set = g_malloc0(argsz);
> > +irq_set->argsz = argsz;
> > +irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> > + VFIO_IRQ_SET_ACTION_TRIGGER;
> > +irq_set->index = VFIO_CCW_IO_IRQ_INDEX;
> > +irq_set->start = 0;
> > +irq_set->count = 1;
> > +pfd = (int32_t *) _set->data;
> > +
> > +*pfd = event_notifier_get_fd(>io_notifier);
> > +qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev);
> > +if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> > +error_setg(errp, "vfio: Failed to set up io notification");
> > +qemu_set_fd_handler(*pfd, NULL, NULL, vcdev);
> > +event_notifier_cleanup(>io_notifier);
> > +goto set_error;
> > +}
> > +
> > +set_error:
> > +g_free(irq_set);
> > +
> > +get_error:
> > +g_free(irq_info);
> > +}
> > +
[...]

-- 
Dong Jia

Re: [Qemu-devel] [PATCH RFC v2 08/12] vfio/ccw: get io region info

[Dong Jia Shi]

* Dong Jia Shi  [2017-01-18 13:22:56 +0800]:

> * Alex Williamson  [2017-01-17 15:49:34 -0700]:
> 
> > On Thu, 12 Jan 2017 08:25:09 +0100
> > Dong Jia Shi  wrote:
> > 
> > > vfio-ccw provides an MMIO region for I/O operations. We fetch its
> > > information via ioctls here, then we can use it performing I/O
> > > instructions and retrieving I/O results later on.
> > > 
> > > Signed-off-by: Xiao Feng Ren 
> > > ---
> > >  hw/vfio/ccw.c | 52 
> > >  1 file changed, 52 insertions(+)
> > > 
> > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> > > index 881010b..93394c2 100644
> > > --- a/hw/vfio/ccw.c
> > > +++ b/hw/vfio/ccw.c
> > > @@ -21,11 +21,15 @@
> > >  #include "hw/vfio/vfio-common.h"
> > >  #include "hw/s390x/s390-ccw.h"
> > >  #include "hw/s390x/ccw-device.h"
> > > +#include "standard-headers/asm-s390/vfio_ccw.h"
> > >  
> > >  #define TYPE_VFIO_CCW "vfio-ccw"
> > >  typedef struct VFIOCCWDevice {
> > >  S390CCWDevice cdev;
> > >  VFIODevice vdev;
> > > +uint64_t io_region_size;
> > > +uint64_t io_region_offset;
> > > +struct ccw_io_region *io_region;
> > >  } VFIOCCWDevice;
> > >  
> > >  static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> > > @@ -50,6 +54,46 @@ static void vfio_ccw_reset(DeviceState *dev)
> > >  ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
> > >  }
> > >  
> > > +static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
> > > +{
> > > +VFIODevice *vdev = >vdev;
> > > +struct vfio_region_info *info;
> > > +int ret;
> > > +
> > > +/* Sanity check device */
> > > +if (!(vdev->flags & VFIO_DEVICE_FLAGS_CCW)) {
> > > +error_setg(errp, "vfio: Um, this isn't a vfio-ccw device");
> > > +return;
> > > +}
> > > +
> > > +if (vdev->num_regions != VFIO_CCW_NUM_REGIONS) {
> > > +error_setg(errp, "vfio: Unexpected number of the I/O region %u",
> > > +   vdev->num_regions);
> > > +return;
> > > +}
> > 
> > I think you want < here, not !=, otherwise you we can never add another
> > region to describe something new on a vfio-ccw device without breaking
> > this code.  You've defined in the uapi that region index zero is always
> > VFIO_CCW_CONFIG_REGION_INDEX and this code only depends on that, no
> > matter what additional regions might be added.  That's done below.
> > 
> Right! Thanks for pointing out. I will fix.
> 
Let me make it clear. I will change it to:
if (vdev->num_regions < VFIO_CCW_CONFIG_REGION_INDEX + 1) {
... ...
}

> > > +
> > > +ret = vfio_get_region_info(vdev, VFIO_CCW_CONFIG_REGION_INDEX, 
> > > );
> > > +if (ret) {
> > > +error_setg(errp, "vfio: Error getting config info: %d", ret);
> > > +return;
> > > +}
> > > +
> > > +vcdev->io_region_size = info->size;
> > > +if (sizeof(*vcdev->io_region) != vcdev->io_region_size) {
> > > +error_setg(errp, "vfio: Unexpected size of the I/O region");
> > > +return;
> > > +}
> > > +vcdev->io_region_offset = info->offset;
> > > +vcdev->io_region = g_malloc0(info->size);
> > > +
> > > +g_free(info);
> > > +}
> > > +
> > > +static void vfio_ccw_put_region(VFIOCCWDevice *vcdev)
> > > +{
> > > +g_free(vcdev->io_region);
> > > +}
> > > +
> > >  static void vfio_put_device(VFIOCCWDevice *vcdev)
> > >  {
> > >  g_free(vcdev->vdev.name);
> > > @@ -144,8 +188,15 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
> > > **errp)
> > >  goto out_device_err;
> > >  }
> > >  
> > > +vfio_ccw_get_region(vcdev, errp);
> > > +if (*errp) {
> > > +goto out_region_err;
> > > +}
> > > +
> > >  return;
> > >  
> > > +out_region_err:
> > > +vfio_put_device(vcdev);
> > >  out_device_err:
> > >  vfio_ccw_put_group(group, path);
> > >  out_group_err:
> > > @@ -166,6 +217,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, 
> > > Error **errp)
> > >  cdc->unrealize(cdev, errp);
> > >  }
> > >  
> > > +vfio_ccw_put_region(vcdev);
> > >  vfio_put_device(vcdev);
> > >  vfio_put_group(group);
> > >  }
> > 
> 
> -- 
> Dong Jia

-- 
Dong Jia

Re: [Qemu-devel] [PATCH v2] vfio/pci: Support error recovery

[Cao jin]

Alex,
Do you have any comments on this version & and the qemu parts?

-- 
Sincerely,
Cao jin

On 12/31/2016 05:15 PM, Cao jin wrote:
> Support serious device error recovery
> 
> Signed-off-by: Cao jin 
> ---
>  drivers/vfio/pci/vfio_pci.c | 70 
> +++--
>  drivers/vfio/pci/vfio_pci_private.h |  2 ++
>  2 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 712a849..752af20 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -534,6 +534,15 @@ static long vfio_pci_ioctl(void *device_data,
>  {
>   struct vfio_pci_device *vdev = device_data;
>   unsigned long minsz;
> + int ret;
> +
> + if (vdev->aer_recovering && (cmd == VFIO_DEVICE_SET_IRQS ||
> + cmd == VFIO_DEVICE_RESET || cmd == VFIO_DEVICE_PCI_HOT_RESET)) {
> + ret = wait_for_completion_interruptible(
> + >aer_completion);
> + if (ret)
> + return ret;
> + }
>  
>   if (cmd == VFIO_DEVICE_GET_INFO) {
>   struct vfio_device_info info;
> @@ -953,6 +962,15 @@ static ssize_t vfio_pci_rw(void *device_data, char 
> __user *buf,
>  {
>   unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
>   struct vfio_pci_device *vdev = device_data;
> + int ret;
> +
> + /* block all kinds of access during host recovery */
> + if (vdev->aer_recovering) {
> + ret = wait_for_completion_interruptible(
> + >aer_completion);
> + if (ret)
> + return ret;
> + }
>  
>   if (index >= VFIO_PCI_NUM_REGIONS + vdev->num_regions)
>   return -EINVAL;
> @@ -1117,6 +1135,7 @@ static int vfio_pci_probe(struct pci_dev *pdev, const 
> struct pci_device_id *id)
>   vdev->irq_type = VFIO_PCI_NUM_IRQS;
>   mutex_init(>igate);
>   spin_lock_init(>irqlock);
> + init_completion(>aer_completion);
>  
>   ret = vfio_add_group_dev(>dev, _pci_ops, vdev);
>   if (ret) {
> @@ -1176,6 +1195,9 @@ static pci_ers_result_t 
> vfio_pci_aer_err_detected(struct pci_dev *pdev,
>  {
>   struct vfio_pci_device *vdev;
>   struct vfio_device *device;
> + u32 uncor_status;
> + unsigned int aer_cap_offset;
> + int ret;
>  
>   device = vfio_device_get_from_dev(>dev);
>   if (device == NULL)
> @@ -1187,10 +1209,29 @@ static pci_ers_result_t 
> vfio_pci_aer_err_detected(struct pci_dev *pdev,
>   return PCI_ERS_RESULT_DISCONNECT;
>   }
>  
> + /*
> +  * get device's uncorrectable error status as soon as possible,
> +  * and signal it to user space. The later we read it, the possibility
> +  * the register value is mangled grows.
> +  */
> + aer_cap_offset = pci_find_ext_capability(vdev->pdev, 
> PCI_EXT_CAP_ID_ERR);
> + ret = pci_read_config_dword(vdev->pdev, aer_cap_offset +
> +PCI_ERR_UNCOR_STATUS, _status);
> +if (ret)
> +return PCI_ERS_RESULT_DISCONNECT;
> +
> + pr_info("device %d got AER detect notification. uncorrectable error 
> status = 0x%x\n", pdev->devfn, uncor_status);//to be removed
>   mutex_lock(>igate);
>  
> - if (vdev->err_trigger)
> - eventfd_signal(vdev->err_trigger, 1);
> + vdev->aer_recovering = true;
> + reinit_completion(>aer_completion);
> +
> + if (vdev->err_trigger && uncor_status) {
> + pr_info("device %d signal uncor status 0x%x to user",
> + pdev->devfn, uncor_status);
> + /* signal uncorrectable error status to user space */
> + eventfd_signal(vdev->err_trigger, uncor_status);
> +}
>  
>   mutex_unlock(>igate);
>  
> @@ -1199,8 +1240,33 @@ static pci_ers_result_t 
> vfio_pci_aer_err_detected(struct pci_dev *pdev,
>   return PCI_ERS_RESULT_CAN_RECOVER;
>  }
>  
> +static void vfio_pci_aer_resume(struct pci_dev *pdev)
> +{
> + struct vfio_pci_device *vdev;
> + struct vfio_device *device;
> +
> + device = vfio_device_get_from_dev(>dev);
> + if (device == NULL)
> + return;
> +
> + vdev = vfio_device_data(device);
> + if (vdev == NULL) {
> + vfio_device_put(device);
> + return;
> + }
> +
> + mutex_lock(>igate);
> + vdev->aer_recovering = false;
> + mutex_unlock(>igate);
> +
> + complete_all(>aer_completion);
> +
> + vfio_device_put(device);
> +}
> +
>  static const struct pci_error_handlers vfio_err_handlers = {
>   .error_detected = vfio_pci_aer_err_detected,
> + .resume = vfio_pci_aer_resume,
>  };
>  
>  static struct pci_driver vfio_pci_driver = {
> diff --git a/drivers/vfio/pci/vfio_pci_private.h 
> b/drivers/vfio/pci/vfio_pci_private.h
> index 8a7d546..ba8471f 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h

Re: [Qemu-devel] [PATCH RFC v2 08/12] vfio/ccw: get io region info

[Dong Jia Shi]

* Alex Williamson  [2017-01-17 15:49:34 -0700]:

> On Thu, 12 Jan 2017 08:25:09 +0100
> Dong Jia Shi  wrote:
> 
> > vfio-ccw provides an MMIO region for I/O operations. We fetch its
> > information via ioctls here, then we can use it performing I/O
> > instructions and retrieving I/O results later on.
> > 
> > Signed-off-by: Xiao Feng Ren 
> > ---
> >  hw/vfio/ccw.c | 52 
> >  1 file changed, 52 insertions(+)
> > 
> > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> > index 881010b..93394c2 100644
> > --- a/hw/vfio/ccw.c
> > +++ b/hw/vfio/ccw.c
> > @@ -21,11 +21,15 @@
> >  #include "hw/vfio/vfio-common.h"
> >  #include "hw/s390x/s390-ccw.h"
> >  #include "hw/s390x/ccw-device.h"
> > +#include "standard-headers/asm-s390/vfio_ccw.h"
> >  
> >  #define TYPE_VFIO_CCW "vfio-ccw"
> >  typedef struct VFIOCCWDevice {
> >  S390CCWDevice cdev;
> >  VFIODevice vdev;
> > +uint64_t io_region_size;
> > +uint64_t io_region_offset;
> > +struct ccw_io_region *io_region;
> >  } VFIOCCWDevice;
> >  
> >  static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> > @@ -50,6 +54,46 @@ static void vfio_ccw_reset(DeviceState *dev)
> >  ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
> >  }
> >  
> > +static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
> > +{
> > +VFIODevice *vdev = >vdev;
> > +struct vfio_region_info *info;
> > +int ret;
> > +
> > +/* Sanity check device */
> > +if (!(vdev->flags & VFIO_DEVICE_FLAGS_CCW)) {
> > +error_setg(errp, "vfio: Um, this isn't a vfio-ccw device");
> > +return;
> > +}
> > +
> > +if (vdev->num_regions != VFIO_CCW_NUM_REGIONS) {
> > +error_setg(errp, "vfio: Unexpected number of the I/O region %u",
> > +   vdev->num_regions);
> > +return;
> > +}
> 
> I think you want < here, not !=, otherwise you we can never add another
> region to describe something new on a vfio-ccw device without breaking
> this code.  You've defined in the uapi that region index zero is always
> VFIO_CCW_CONFIG_REGION_INDEX and this code only depends on that, no
> matter what additional regions might be added.  That's done below.
> 
Right! Thanks for pointing out. I will fix.

> > +
> > +ret = vfio_get_region_info(vdev, VFIO_CCW_CONFIG_REGION_INDEX, );
> > +if (ret) {
> > +error_setg(errp, "vfio: Error getting config info: %d", ret);
> > +return;
> > +}
> > +
> > +vcdev->io_region_size = info->size;
> > +if (sizeof(*vcdev->io_region) != vcdev->io_region_size) {
> > +error_setg(errp, "vfio: Unexpected size of the I/O region");
> > +return;
> > +}
> > +vcdev->io_region_offset = info->offset;
> > +vcdev->io_region = g_malloc0(info->size);
> > +
> > +g_free(info);
> > +}
> > +
> > +static void vfio_ccw_put_region(VFIOCCWDevice *vcdev)
> > +{
> > +g_free(vcdev->io_region);
> > +}
> > +
> >  static void vfio_put_device(VFIOCCWDevice *vcdev)
> >  {
> >  g_free(vcdev->vdev.name);
> > @@ -144,8 +188,15 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
> > **errp)
> >  goto out_device_err;
> >  }
> >  
> > +vfio_ccw_get_region(vcdev, errp);
> > +if (*errp) {
> > +goto out_region_err;
> > +}
> > +
> >  return;
> >  
> > +out_region_err:
> > +vfio_put_device(vcdev);
> >  out_device_err:
> >  vfio_ccw_put_group(group, path);
> >  out_group_err:
> > @@ -166,6 +217,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error 
> > **errp)
> >  cdc->unrealize(cdev, errp);
> >  }
> >  
> > +vfio_ccw_put_region(vcdev);
> >  vfio_put_device(vcdev);
> >  vfio_put_group(group);
> >  }
> 

-- 
Dong Jia

Re: [Qemu-devel] [PATCH RFC] migration: set cpu throttle value by workload

[Chao Fan]

Hi all,

This is a test for this RFC patch.

Start vm as following:
cmdline="./x86_64-softmmu/qemu-system-x86_64 -m 2560 \
-drive if=none,file=/nfs/img/fedora.qcow2,format=qcow2,id=foo \
-netdev tap,id=hn0,queues=1 \
-device virtio-net-pci,id=net-pci0,netdev=hn0 \
-device virtio-blk,drive=foo \
-enable-kvm -M pc -cpu host \
-vnc :3 \
-monitor stdio"

Continue running benchmark program named himeno[*](modified base on
original source). The code is in the attach file, make it in MIDDLE.
It costs much cpu calculation and memory. Then migrate the guest.
The source host and target host are in one switch.

"before" means the upstream version, "after" means applying this patch.
"idpr" means "inst_dirty_pages_rate", a new variable in this RFC PATCH.
"count" is "dirty sync count" in "info migrate".
"time" is "total time" in "info migrate".
"ct pct" is "cpu throttle percentage" in "info migrate".

 
| |before|after| 
|-|--|-| 
|count|time(s)|ct pct|time(s)| idpr |ct pct| 
|-|---|--|---|--|--| 
|  1  |3  |   0  |4  |   x  |   0  | 
|  2  |   53  |   0  |   53  | 14237|   0  | 
|  3  |   97  |   0  |   95  |  3142|   0  | 
|  4  |  109  |   0  |  105  | 11085|   0  | 
|  5  |  117  |   0  |  113  | 12894|   0  | 
|  6  |  125  |  20  |  121  | 13549|  67  | 
|  7  |  133  |  20  |  130  | 13550|  67  | 
|  8  |  141  |  20  |  136  | 13587|  67  | 
|  9  |  149  |  30  |  144  | 13553|  99  | 
| 10  |  156  |  30  |  152  |  1474|  99  |  
| 11  |  164  |  30  |  152  |  1706|  99  |  
| 12  |  172  |  40  |  153  |   0  |  99  |  
| 13  |  180  |  40  |  153  |   0  |   x  |  
| 14  |  188  |  40  |-|
| 15  |  195  |  50  |  completed  |  
| 16  |  203  |  50  | |  
| 17  |  211  |  50  | |  
| 18  |  219  |  60  | |  
| 19  |  227  |  60  | |  
| 20  |  235  |  60  | |  
| 21  |  242  |  70  | |  
| 22  |  250  |  70  | |  
| 23  |  258  |  70  | |  
| 24  |  266  |  80  | |  
| 25  |  274  |  80  | |  
| 26  |  281  |  80  | |  
| 27  |  289  |  90  | |  
| 28  |  297  |  90  | |  
| 29  |  305  |  90  | |  
| 30  |  315  |  99  | |  
| 31  |  320  |  99  | |  
| 32  |  320  |  99  | |  
| 33  |  321  |  99  | |  
| 34  |  321  |  99  | |  
|| |
|completed   | |


And the "info migrate" when completed:

before:
capabilities: xbzrle: off rdma-pin-all: off auto-converge: on
zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off 
Migration status: completed
total time: 321091 milliseconds
downtime: 573 milliseconds
setup: 40 milliseconds
transferred ram: 10509346 kbytes
throughput: 268.13 mbps
remaining ram: 0 kbytes
total ram: 2638664 kbytes
duplicate: 362439 pages
skipped: 0 pages
normal: 2621414 pages
normal bytes: 10485656 kbytes
dirty sync count: 34

after:
capabilities: xbzrle: off rdma-pin-all: off auto-converge: on
zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off 
Migration status: completed
total time: 152652 milliseconds
downtime: 290 milliseconds
setup: 47 milliseconds
transferred ram: 4997452 kbytes
throughput: 268.20 mbps
remaining ram: 0 kbytes
total ram: 2638664 kbytes
duplicate: 359598 pages
skipped: 0 pages
normal: 1246136 pages
normal bytes: 4984544 kbytes
dirty sync count: 13

It's clear that the total time is much better(321s VS 153s).
The guest began cpu throttle in the 6th dirty sync. But at this time,
the dirty pages born too much in this guest. So the default
cpu throttle percentage(20 and 10) is too small for this condition. I
just use (inst_dirty_pages_rate / 200) to calculate the cpu throttle
value. This is just an adhoc algorithm, not supported by any theories. 

Of course on the other hand, the cpu throttle percentage is higher, the
guest runs more slowly. But in the result, after applying this patch,
the guest spend 23s with the cpu throttle percentage is 67 (total time
from 121 to 144), and 9s with cpu throttle percentage is 99 (total time
from 144 to completed). But in the upstream version, the guest spend
73s with the cpu throttle percentage is 70.80.90 (total time from 21 to
30), 6s with the cpu throttle percentage is 99 (total time from 30 to
completed). So I think the influence to the guest performance after my
patch is fewer than the upstream version.

Any comments will be welcome.

[*]http://accc.riken.jp/en/supercom/himenobmt/

Thanks,

Chao FanOn Thu, Dec 29, 2016 at 05:16:19PM +0800, Chao 

Re: [Qemu-devel] [PATCH v6 kernel 3/5] virtio-balloon: speed up inflate/deflate process

[Li, Liang Z]

> > -   virtqueue_add_outbuf(vq, , 1, vb, GFP_KERNEL);
> > -   virtqueue_kick(vq);
> > +static void do_set_resp_bitmap(struct virtio_balloon *vb,
> > +   unsigned long base_pfn, int pages)
> >
> > -   /* When host has read buffer, this completes via balloon_ack */
> > -   wait_event(vb->acked, virtqueue_get_buf(vq, ));
> > +{
> > +   __le64 *range = vb->resp_data + vb->resp_pos;
> >
> > +   if (pages > (1 << VIRTIO_BALLOON_NR_PFN_BITS)) {
> > +   /* when the length field can't contain pages, set it to 0 to
> 
> /*
>  * Multi-line
>  * comments
>  * should look like this.
>  */
> 
> Also, pls start sentences with an upper-case letter.
> 

Sorry for that.

> > +* indicate the actual length is in the next __le64;
> > +*/
> 
> This is part of the interface so should be documented as such.
> 
> > +   *range = cpu_to_le64((base_pfn <<
> > +   VIRTIO_BALLOON_NR_PFN_BITS) | 0);
> > +   *(range + 1) = cpu_to_le64(pages);
> > +   vb->resp_pos += 2;
> 
> Pls use structs for this kind of stuff.

I am not sure if you mean to use 

struct  range {
__le64 pfn: 52;
__le64 nr_page: 12
}
Instead of the shift operation?

I didn't use this way because I don't want to include 'virtio-balloon.h' in 
page_alloc.c,
or copy the define of this struct in page_alloc.c

Thanks!
Liang

Re: [Qemu-devel] [PATCH 1/2] vl: Ensure the numa_post_machine_init func in the appropriate location

[Dou Liyang]

Hi, Eduardo

At 01/18/2017 04:09 AM, Eduardo Habkost wrote:

On Tue, Jan 17, 2017 at 10:42:31PM +0800, Dou Liyang wrote:

In the numa_post_machine_init(), we use CPU_FOREACH macro to set all
CPUs' namu_node. So, we should make sure that we call it after Qemu
has already initialied all the CPUs.

As we all know, the CPUs can be created by "-smp"(pc_new_cpu) or
"-device"(qdev_device_add) command. But, before the device init,
Qemu execute the numa_post_machine_init earlier. It makes the mapping
of NUMA nodes and CPUs incorrect.

The patch move the numa_post_machine_init func in the appropriate
location.

Signed-off-by: Dou Liyang 


I would like to move cpu_index initialization to
qom/cpu.c:cpu_common_realizefn(), and remove
numa_post_machine_init() completely.


Thanks, it is a good idea. I will try it later.

But, I hope to know that:

If you may want to say the cpu->**numa_node** initialization?
Because the **cpu_index** initialization is in pc_cpu_pre_plug()
currently, like that: cs->cpu_index = idx;

OR

You want to move both of them in cpu_common_realizefn() ?

Thanks,
Liyang.

 But this fixes the bug while

we don't do that.

Reviewed-by: Eduardo Habkost 

Queued on numa-next. Thanks!


---
 vl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index c643d3f..f38b0e2 100644
--- a/vl.c
+++ b/vl.c
@@ -4549,8 +4549,6 @@ int main(int argc, char **argv, char **envp)

 cpu_synchronize_all_post_init();

-numa_post_machine_init();
-
 if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
   parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
 exit(1);
@@ -4571,6 +4569,9 @@ int main(int argc, char **argv, char **envp)
   device_init_func, NULL, NULL)) {
 exit(1);
 }
+
+numa_post_machine_init();
+
 rom_reset_order_override();

 /* Did we create any drives that we failed to create a device for? */
--
2.5.5

Re: [Qemu-devel] [PATCH v3] gdbstub.c: fix GDB connection segfault caused by empty machines

[Yang Ziyue]

Hi Fam,

I've split the patch into 2 smaller ones and submitted them tagged with "v4".
Thanks!

Ziyue Yang

2017-01-18 10:15 GMT+08:00 Fam Zheng :
> On Wed, 01/18 09:28, Ziyue Yang wrote:
>> Also some updates from fprintf(stderr, ...) to error_report.
>
> Looks like they belong to a separate patch.
>
> Fam

[Qemu-devel] [PATCH v4 2/2] gdbstub.c: update old error report statements

[Ziyue Yang]

From: Ziyue Yang 

Some updates from fprintf(stderr, ...) to error_report.

Signed-off-by: Ziyue Yang 
---
 gdbstub.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 426d55e..fe1d0f8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -637,8 +637,8 @@ void gdb_register_coprocessor(CPUState *cpu,
 *p = s;
 if (g_pos) {
 if (g_pos != s->base_reg) {
-fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
-"Expected %d got %d\n", xml, g_pos, s->base_reg);
+error_report("Error: Bad gdb register numbering for '%s', "
+ "Expected %d got %d", xml, g_pos, s->base_reg);
 } else {
 cpu->gdb_num_g_regs = cpu->gdb_num_regs;
 }
@@ -890,7 +890,7 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 case 'k':
 /* Kill the target */
-fprintf(stderr, "\nQEMU: Terminated via GDBstub\n");
+error_report("QEMU: Terminated via GDBstub");
 exit(0);
 case 'D':
 /* Detach packet */
@@ -1358,8 +1358,8 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const 
char *fmt, va_list va)
 break;
 default:
 bad_format:
-fprintf(stderr, "gdbstub: Bad syscall format string '%s'\n",
-fmt - 1);
+error_report("gdbstub: Bad syscall format string '%s'",
+ fmt - 1);
 break;
 }
 } else {
--
2.7.4

Re: [Qemu-devel] [PATCH RFC v2 07/12] vfio/ccw: vfio based subchannel passthrough driver

[Dong Jia Shi]

* Alex Williamson  [2017-01-17 15:49:28 -0700]:

> On Thu, 12 Jan 2017 08:25:08 +0100
> Dong Jia Shi  wrote:
> 
> > From: Xiao Feng Ren 
> > 
> > We use the IOMMU_TYPE1 of VFIO to realize the subchannels
> > passthrough, implement a vfio based subchannels passthrough
> > driver called "vfio-ccw".
> > 
> > Support qemu parameters in the style of:
> > "-device vfio-ccw,id=xx,hostid=xx(,guestid=xx),mdevid=xx"
> 
> Why not adopt the same syntax as vfio-pci with mdev devices,
> sysfsdev=%s where %s is a path to the mdev device in sysfs, which can
> be /sys/bus/mdev/devices/$UUID or as you create from the hostid
> below /sys/bus/css/devices/%x.%x.%04x/$UUID.
Ok. This is a good point.

I will change the cmdline interface to:
-device vfio-ccw,id=xx,sysfsdev=xx(,devno=xx)

Since "hostid" is removed, I will also rename "guestid" to "devno" to
make it looking consistent with the virtio-ccw devices.

> It seems if we know where
> to find the mdev device then we can determine the parent information on
> our own through sysfs.
> 
This is true.

> > Signed-off-by: Xiao Feng Ren 
[...]

-- 
Dong Jia

[Qemu-devel] [PATCH v4 1/2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Ziyue Yang]

From: Ziyue Yang 

This patch is to fix the segmentation fault caused by attaching
GDB to a QEMU instance initialized with "-M none" option.

The bug can be reproduced by

> ./qemu-system-x86_64 -M none -nographic -S -s

and attach a GDB to it by

> gdb -ex 'target remote :1234

The segmentation fault was originally caused by trying to read
the information about CPU when communicating with GDB. However,
it's impossible for any control flow to exist on an empty machine,
nor can CPU's be hot plugged to an empty machine later by QOM
commands. So I think simply disabling GDB connections on empty
machines makes sense.

Also some updates from fprintf(stderr, ...) to error_report.

Signed-off-by: Ziyue Yang 
---
 gdbstub.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index de62d26..426d55e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -18,6 +18,7 @@
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qemu/cutils.h"
 #include "cpu.h"
 #ifdef CONFIG_USER_ONLY
@@ -1731,6 +1732,12 @@ int gdbserver_start(const char *device)
 CharDriverState *mon_chr;
 ChardevCommon common = { 0 };

+if (!first_cpu) {
+error_report("gdbstub: meaningless to attach gdb to a "
+ "machine without any CPU.");
+return -1;
+}
+
 if (!device)
 return -1;
 if (strcmp(device, "none") != 0) {
--
2.7.4

Re: [Qemu-devel] [PATCH RFC v3 14/14] intel_iommu: enable vfio devices

[Jason Wang]

On 2017年01月17日 22:45, Peter Xu wrote:

On Mon, Jan 16, 2017 at 05:54:55PM +0800, Jason Wang wrote:


On 2017年01月16日 17:18, Peter Xu wrote:

  static void vtd_iotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
hwaddr addr, uint8_t am)
  {
@@ -1222,6 +1251,7 @@ static void vtd_iotlb_page_invalidate(IntelIOMMUState *s, 
uint16_t domain_id,
  info.addr = addr;
  info.mask = ~((1 << am) - 1);
  g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_page, );
+vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am);

Is the case of GLOBAL or DSI flush missed, or we don't care it at all?

IMHO we don't. For device assignment, since we are having CM=1 here,
we should have explicit page invalidations even if guest sends
global/domain invalidations.

Thanks,

-- peterx

Is this spec required?

I think not. IMO the spec is very coarse grained on describing cache
mode...


Btw, it looks to me that both DSI and GLOBAL are
indeed explicit flush.

Actually when cache mode is on, it is unclear to me on how we should
treat domain/global invalidations, at least from the spec (as
mentioned earlier). My understanding is that they are not "explicit
flushes", which IMHO should only mean page selective IOTLB
invalidations.


Probably not, at least from the view of performance. DSI and global 
should be more efficient in some cases.





Just have a quick go through on driver codes and find this something
interesting in intel_iommu_flush_iotlb_psi():

...
 /*
  * Fallback to domain selective flush if no PSI support or the size is
  * too big.
  * PSI requires page size to be 2 ^ x, and the base address is naturally
  * aligned to the size
  */
 if (!cap_pgsel_inv(iommu->cap) || mask > cap_max_amask_val(iommu->cap))
 iommu->flush.flush_iotlb(iommu, did, 0, 0,
 DMA_TLB_DSI_FLUSH);
 else
 iommu->flush.flush_iotlb(iommu, did, addr | ih, mask,
 DMA_TLB_PSI_FLUSH);
...

I think this is interesting... and I doubt its correctness while with
cache mode enabled.

If so (sending domain invalidation instead of a big range of page
invalidations), how should we capture which pages are unmapped in
emulated IOMMU?


We don't need to track individual pages here, since all pages for a 
specific domain were unmapped I believe?





It looks like DSI_FLUSH is possible even for CM on.

And in flush_unmaps():

...
 /* In caching mode, global flushes turn emulation expensive */
 if (!cap_caching_mode(iommu->cap))
 iommu->flush.flush_iotlb(iommu, 0, 0, 0,
  DMA_TLB_GLOBAL_FLUSH);
...

If I understand the comments correctly, GLOBAL is ok for CM too (though the
code did not do it for performance reason).

I think it should be okay to send global flush for CM, but not sure
whether we should notify anything when we receive it. Hmm, anyway, I
think I need some more reading to make sure I understand the whole
thing correctly. :)

For example, when I see this commit:

commit 78d5f0f500e6ba8f6cfd0673475ff4d941d705a2
Author: Nadav Amit 
Date:   Thu Apr 8 23:00:41 2010 +0300

 intel-iommu: Avoid global flushes with caching mode.
 
 While it may be efficient on real hardware, emulation of global

 invalidations is very expensive as all shadow entries must be examined.
 This patch changes the behaviour when caching mode is enabled (which is
 the case when IOMMU emulation takes place). In this case, page specific
 invalidation is used instead.

Before I ask someone else besides qemu-devel, I am curious about
whether there is existing VT-d emulation code (outside QEMU, of
course) so that I can have a reference?


Yes, it has. The author of this patch - Nadav has done lots of research 
on emulated IOMMU. See following papers:


https://hal.inria.fr/inria-00493752/document
http://www.cse.iitd.ac.in/~sbansal/csl862-virt/readings/vIOMMU.pdf

Thanks


Quick search didn't answer me.

Thanks,

-- peterx

Re: [Qemu-devel] [PATCH RFC v2 02/12] vfio: linux-headers update for vfio-ccw

[Dong Jia Shi]

* Alex Williamson  [2017-01-17 14:51:42 -0700]:

> On Thu, 12 Jan 2017 08:25:03 +0100
> Dong Jia Shi  wrote:
> 
> > From: Xiao Feng Ren 
> > 
> > This is a placeholder for a linux-headers update.
> > 
> > Signed-off-by: Xiao Feng Ren 
> > ---
> >  include/standard-headers/asm-s390/vfio_ccw.h | 28 
> > 
> >  linux-headers/linux/vfio.h   | 17 +
> >  2 files changed, 45 insertions(+)
> >  create mode 100644 include/standard-headers/asm-s390/vfio_ccw.h
> > 
> > diff --git a/include/standard-headers/asm-s390/vfio_ccw.h 
> > b/include/standard-headers/asm-s390/vfio_ccw.h
> > new file mode 100644
> > index 000..cddc09b
> > --- /dev/null
> > +++ b/include/standard-headers/asm-s390/vfio_ccw.h
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Interfaces for vfio-ccw
> > + *
> > + * Copyright IBM Corp. 2017
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License (version 2 only)
> > + * as published by the Free Software Foundation.
> > + *
> > + * Author(s): Dong Jia Shi 
> > + */
> > +
> > +#ifndef _VFIO_CCW_H_
> > +#define _VFIO_CCW_H_
> > +
> > +#include "standard-headers/linux/types.h"
> > +
> > +struct ccw_io_region {
> > +#define ORB_AREA_SIZE 12
> > +   uint8_t  orb_area[ORB_AREA_SIZE];
> > +#define SCSW_AREA_SIZE 12
> > +   uint8_t  scsw_area[SCSW_AREA_SIZE];
> > +#define IRB_AREA_SIZE 96
> > +   uint8_t  irb_area[IRB_AREA_SIZE];
> > +   uint32_t ret_code;
> > +} QEMU_PACKED;
> > +
> > +#endif
> 
> This is really part of the uapi for the vfio-ccw mdev device, isn't it?
Yes, it is.

> Should it really be buried in asm-s390 in the kernel?
> 
We had an internal discussion on this question before, since we think
this interface is strongly s390 dependent, we put it here. What do you
suggest? Thanks,

[...]

-- 
Dong Jia

Re: [Qemu-devel] [PATCH] virtio: force VIRTIO_F_IOMMU_PLATFORM

[Jason Wang]

On 2017年01月17日 22:44, Michael S. Tsirkin wrote:

On Tue, Jan 17, 2017 at 12:01:00PM +0800, Jason Wang wrote:

We allow vhost to clear VIRITO_F_IOMMU_PLATFORM which is wrong since
VIRTIO_F_IOMMU_PLATFORM is mandatory for security. Fixing this by
enforce it after vdc->get_features().

Signed-off-by: Jason Wang 
---
  hw/virtio/virtio-bus.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index d31cc00..a886011 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -47,6 +47,7 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
**errp)
  VirtioBusState *bus = VIRTIO_BUS(qbus);
  VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
  VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
  
  DPRINTF("%s: plug device.\n", qbus->name);
  
@@ -63,8 +64,8 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp)

  klass->device_plugged(qbus->parent, errp);
  }
  
-if (klass->get_dma_as != NULL &&

-virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
+if (klass->get_dma_as != NULL && has_iommu) {
+virtio_add_feature(>host_features, VIRTIO_F_IOMMU_PLATFORM);
  vdev->dma_as = klass->get_dma_as(qbus->parent);
  } else {
  vdev->dma_as = _space_memory;

I suspect that's not enough, we must also fail or disable vhost
(depending on the options), otherwise things won't work.


Looks like with the patch, VIRTIO_F_IOMMU_PLATFORM will be passed to 
vhost_set_features(). So if vhost backend does not support it, it will 
fall back to userspace (This may not work for vhost-user, but it's a bug 
existed even before this patch).


Thanks

Re: [Qemu-devel] [PATCH RFC v2 11/15] vfio: ccw: introduce ioctls to get/set VFIO_CCW_IO_IRQ

[Dong Jia Shi]

* Alex Williamson  [2017-01-17 14:02:40 -0700]:

> On Thu, 12 Jan 2017 08:19:43 +0100
> Dong Jia Shi  wrote:
> 
> > Realize VFIO_DEVICE_GET_IRQ_INFO ioctl to retrieve
> > VFIO_CCW_IO_IRQ information.
> > 
> > Realize VFIO_DEVICE_SET_IRQS ioctl to set an eventfd fd for
> > VFIO_CCW_IO_IRQ. Once a write operation to the ccw_io_region
> > was performed, trigger a signal on this fd.
> > 
> > Signed-off-by: Dong Jia Shi 
> > Reviewed-by: Pierre Morel 
> > ---
> >  drivers/s390/cio/vfio_ccw_ops.c | 125 
> > +++-
> >  drivers/s390/cio/vfio_ccw_private.h |   4 ++
> >  include/uapi/linux/vfio.h   |  10 ++-
> >  3 files changed, 136 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/s390/cio/vfio_ccw_ops.c 
> > b/drivers/s390/cio/vfio_ccw_ops.c
> > index b702735..3c47eb6 100644
> > --- a/drivers/s390/cio/vfio_ccw_ops.c
> > +++ b/drivers/s390/cio/vfio_ccw_ops.c
> > @@ -203,6 +203,9 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device 
> > *mdev,
> > if (region->ret_code != 0)
> > return region->ret_code;
> >  
> > +   if (private->io_trigger)
> > +   eventfd_signal(private->io_trigger, 1);
> > +
> > return count;
> >  }
> >  
> > @@ -211,7 +214,7 @@ static int vfio_ccw_mdev_get_device_info(struct 
> > mdev_device *mdev,
> >  {
> > info->flags = VFIO_DEVICE_FLAGS_CCW | VFIO_DEVICE_FLAGS_RESET;
> > info->num_regions = VFIO_CCW_NUM_REGIONS;
> > -   info->num_irqs = 0;
> > +   info->num_irqs = VFIO_CCW_NUM_IRQS;
> >  
> > return 0;
> >  }
> > @@ -233,6 +236,84 @@ static int vfio_ccw_mdev_get_region_info(struct 
> > mdev_device *mdev,
> > }
> >  }
> >  
> > +int vfio_ccw_mdev_get_irq_info(struct mdev_device *mdev,
> > +  struct vfio_irq_info *info)
> > +{
> > +   if (info->index != VFIO_CCW_IO_IRQ_INDEX)
> > +   return -EINVAL;
> > +
> > +   info->count = VFIO_CCW_NUM_IRQS;
> > +   info->flags = VFIO_IRQ_INFO_EVENTFD | VFIO_IRQ_INFO_NORESIZE;
> 
> 
> VFIO_CCW_NUM_IRQS is not being used correctly here, info->count is the
> number of interrupts within this index, VFIO_CCW_NUM_IRQS is the number
> of indexes.  This is meant to handle things like PCI where we have 3
> different interrupts types (INTx, MSI, MSI-X) and some of those (MSI/X)
> support multiple vectors.  In this case I think you want info->count =
> 1 and you don't need the NORESIZE flag since that only makes sense for
> describing indexes where a subset of the available vectors may be
> enabled.  So info->count comes out to the same thing, but should not
> use the same macro to get there.
> 
Hi Alex,

I indeed use VFIO_CCW_NUM_IRQS here in a wrong way. Thanks for your
explanation. I will change it to:
info->count = 1;

> > +
> > +   return 0;
> > +}
> > +
[...]

> > @@ -281,6 +362,48 @@ static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device 
> > *mdev,
> >  
> > return copy_to_user((void __user *)arg, , minsz);
> > }
> > +   case VFIO_DEVICE_GET_IRQ_INFO:
> > +   {
> > +   struct vfio_irq_info info;
> > +
> > +   minsz = offsetofend(struct vfio_irq_info, count);
> > +
> > +   if (copy_from_user(, (void __user *)arg, minsz))
> > +   return -EFAULT;
> > +
> > +   if (info.argsz < minsz || info.index >= VFIO_CCW_NUM_IRQS)
> > +   return -EINVAL;
> > +
> > +   ret = vfio_ccw_mdev_get_irq_info(mdev, );
> > +   if (ret)
> > +   return ret;
> > +
> > +   if (info.count == -1)
> > +   return -EINVAL;
> > +
> > +   return copy_to_user((void __user *)arg, , minsz);
> > +   }
> > +   case VFIO_DEVICE_SET_IRQS:
> > +   {
> > +   struct vfio_irq_set hdr;
> > +   size_t data_size;
> > +   void __user *data;
> > +
> > +   minsz = offsetofend(struct vfio_irq_set, count);
> > +
> > +   if (copy_from_user(, (void __user *)arg, minsz))
> > +   return -EFAULT;
> > +
> > +   ret = vfio_set_irqs_validate_and_prepare(,
> > +VFIO_CCW_NUM_IRQS,
> > +VFIO_CCW_NUM_IRQS,
> > +_size);
> 
> 
> This is another instance, max_irq_type is referring to the index while
> num_irqs is referring to the count of vectors within this index.
> VFIO_CCW_NUM_IRQS should only be used for max_irq_type.

Ok. Will use 1 instead of VFIO_CCW_NUM_IRQS for @num_irqs.

> 
> > +   if (ret)
> > +   return ret;
> > +
> > +   data = (void __user *)(arg + minsz);
> > +   return vfio_ccw_mdev_set_irqs(mdev, hdr.flags, data);
> > +   }
> > case VFIO_DEVICE_RESET:
> > return vfio_ccw_mdev_reset(mdev);
> > default:
[...]

-- 
Dong Jia

Re: [Qemu-devel] [PATCH RFC v2 06/15] vfio: ccw: register vfio_ccw to the mediated device framework

[Dong Jia Shi]

* Alex Williamson  [2017-01-17 14:02:33 -0700]:

[...]

> > diff --git a/drivers/s390/cio/vfio_ccw_ops.c 
> > b/drivers/s390/cio/vfio_ccw_ops.c
[...]

> > +static int vfio_ccw_mdev_create(struct kobject *kobj, struct mdev_device 
> > *mdev)
> > +{
> > +   struct vfio_ccw_private *private = dev_get_drvdata(mdev->parent->dev);
> > +
> > +   /* Only support one mediated device for each physical subchannel. */
> > +   if (private->mdev)
> > +   return -EPERM;
> > +
> > +   private->mdev = mdev;
> > +   available_instances--;
> 
> 
> This looks racy and doesn't enforce the available instances.  Should
> this maybe be an atomic_t and use atomic_dec_if_positive() to return an
> error if no instances are available?
Hi Alex,

You are right. I will fix this according to your comment.

> 
> 
> > +
> > +   return 0;
> > +}
> > +

[...]

-- 
Dong Jia

[Qemu-devel] commit : "virtio: set ISR on dataplane notifications" breaks virtio-pci network on linux guest.

[Brad Campbell]

G'day all,

Upgrading from 2.7.1 to 2.8 on my machines here leaves the guests 
without networking. All guests are using a vanilla self-compiled 4.8.12 
kernel. The host is using a 4.9.3 kernel.


Guests are using virtio networking into a bridge on the host.

tcpdump on the bridge shows the guests sending dhcp requests and the 
host sending replies, but the replies never make it into the guest.


A bisect between 2.7.1 & 2.8 shows the following commit at fault.

This would appear to be the same fault as mentioned here : 
https://bbs.archlinux.org/viewtopic.php?id=221434


It's no issue for me to leave my hosts at 2.7, but in case it hadn't 
been raised I thought I'd track it down and report it.


brad@test:~/qemu$ git bisect good
83d768b5640946b7da55ce8335509df297e2c7cd is the first bad commit
commit 83d768b5640946b7da55ce8335509df297e2c7cd
Author: Paolo Bonzini 
Date:   Fri Nov 18 16:07:02 2016 +0100

virtio: set ISR on dataplane notifications

Dataplane has been omitting forever the step of setting ISR when
an interrupt is raised.  This caused little breakage, because the
specification actually says that ISR may not be updated in MSI mode.

Some versions of the Windows drivers however didn't clear MSI mode
correctly, and proceeded using polling mode (using ISR, not the used
ring index!) for crashdump and hibernation.  If it were just crashdump
and hibernation it would not be a big deal, but recent releases of
Windows do not really shut down, but rather log out and hibernate to
make the next startup faster.  Hence, this manifested as a more serious
hang during shutdown with e.g. Windows 8.1 and virtio-win 1.8.0 RPMs.
Newer versions fixed this, while older versions do not use MSI at all.

The failure has always been there for virtio dataplane, but it became
visible after commits 9ffe337 ("virtio-blk: always use dataplane path
if ioeventfd is active", 2016-10-30) and ad07cd6 ("virtio-scsi: always
use dataplane path if ioeventfd is active", 2016-10-30) made virtio-blk
and virtio-scsi always use the dataplane code under KVM.  The good news
therefore is that it was not a bug in the patches---they were doing
exactly what they were meant for, i.e. shake out remaining 
dataplane bugs.


The fix is not hard, so it's worth arranging for the broken drivers.
The virtio_should_notify+event_notifier_set pair that is common to
virtio-blk and virtio-scsi dataplane is replaced with a new public
function virtio_notify_irqfd that also sets ISR.  The irqfd emulation
code now need not set ISR anymore, so virtio_irq is removed.

Reviewed-by: Stefan Hajnoczi 
Tested-by: Farhan Ali 
Tested-by: Alex Williamson 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 

:04 04 fa35e9e7e6b49f32f4c5b24586f65ca7f2e6fc73 
282ecd3bb6c512e135b3e10d70de6c9bc306203e M	hw
:04 04 6ded15de7a064a8c00ac60d3f61e09e5d66a5cdd 
7dc07e32776505c037da7bd9e95a78c2518fa458 M	include


Regards,
Brad

Re: [Qemu-devel] [PATCH 2/2] vl: Ensure the cpu_synchronize_all_post_init func in the appropriate location

[Dou Liyang]

Hi, Eduardo

To clarify:

This patch is no related to the bug about NUMA.

This patch makes sure that all CPUs can run cpu state synchronization
on target vcpu thread, not just the CPUs which created by "-smp 2..."
at the beginning.

It is also caused by the execution sequence, So I put it here.

At 01/18/2017 04:21 AM, Eduardo Habkost wrote:

On Tue, Jan 17, 2017 at 10:42:32PM +0800, Dou Liyang wrote:

As the commit a4088c3eecb5f said, In the cpu_synchronize_all_post_init(),
we also use CPU_FOREACH macro to set all CPUs' namu_node.


I can't find commit a4088c3eecb5f, is it the commit ID of your
previous patch on your local tree? I don't see any NUMA-related
code triggered cpu_synchronize_all_post_init().



Oops, yes. It is the previous patch 1 in my local tree. I will modify
it.


  So, we should
make sure that we call it after Qemu has already initialied all the CPUs.




The patch move the numa_post_machine_init func in the appropriate
location.


This patch moves cpu_synchronize_all_post_init(), not
numa_post_machine_init(). Could you describe which bug you are
fixing, exactly? It doesn't seem to be related to NUMA.



Yes, I will describe it exactly in the next version.

Thanks,
Liyang


Signed-off-by: Dou Liyang 
---
 vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index f38b0e2..75adc2a 100644
--- a/vl.c
+++ b/vl.c
@@ -4547,8 +4547,6 @@ int main(int argc, char **argv, char **envp)

 audio_init();

-cpu_synchronize_all_post_init();
-
 if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
   parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
 exit(1);
@@ -4570,6 +4568,8 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }

+cpu_synchronize_all_post_init();
+
 numa_post_machine_init();

 rom_reset_order_override();
--
2.5.5

Re: [Qemu-devel] [PATCH v3] gdbstub.c: fix GDB connection segfault caused by empty machines

[Fam Zheng]

On Wed, 01/18 09:28, Ziyue Yang wrote:
> Also some updates from fprintf(stderr, ...) to error_report.

Looks like they belong to a separate patch.

Fam

Re: [Qemu-devel] [PATCH v2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Fam Zheng]

On Wed, 01/18 09:34, Yang Ziyue wrote:
> Oops seems I forgot to check the patch before submitting. Sorry for that.
> The new patch is tagged as v3 now, but I'm not sure whether it should
> be tagged as v2 since the origin v2 failed. Is there any specification
> about this? Thanks!

Hi Ziyue, thanks for submitting the patch. When you repost, use v3 please, a
(even failed or mistaken) version shouldn't be reused, so that people looking at
the subjects will know which one is newer.

Fam

Re: [Qemu-devel] [RFC PATCH v3 1/1] virtio crypto device specification: asymmetric crypto service

[Gonglei (Arei)]

Hi Xin,

>
> Subject: RE: [RFC PATCH v3 1/1] virtio crypto device specification: asymmetric
> crypto service
> 
> Ping...
> Any comments on this part of virtio crypto spec?
> 
I update the virtio crypto spec to support non-session based crypto
Operations and your asym service also needs change struct 
virtio_crypto_op_data_req
which maybe cause the compatibility complaint with the pre-existing code.

So I strongly suggest you rebase my v16 (will be submitted today), and put
asym service to struct virtio_crypto_op_data_req_mixed.

struct virtio_crypto_op_data_req_mixed {
struct virtio_crypto_op_header header;

union {
struct virtio_crypto_sym_data_req   sym_req;
struct virtio_crypto_hash_data_req  hash_req;
struct virtio_crypto_mac_data_req   mac_req;
struct virtio_crypto_aead_data_req  aead_req;
struct virtio_crypto_sym_data_req_non_sess   sym_non_sess_req;
struct virtio_crypto_hash_data_req_non_sess  hash_non_sess_req;
struct virtio_crypto_mac_data_req_non_sess   mac_non_sess_req;
struct virtio_crypto_aead_data_req_non_sess  aead_non_sess_req;

/* asymmetric crypto requests here */
...
} u;
};

> We also plan to add asymmetric crypto service part support in LKCF based 
> virtio
> crypto frontend driver, and upstream DPDK vhost library based vhost user for
> cryptodev which also focuses on asymmetric crypto service , any comments?
> 
Sounds great. It's very helpful to accelerate the progress of reviewing.
After all they are kinds of special knowledge.

Meanwhile, I can submit the cryptodev-vhost-user/cryptodev-vhost-kernel
backends in QEMU if needs.

Thanks,
-Gonglei

Re: [Qemu-devel] [virtio-dev] Re: [PATCH v6 kernel 2/5] virtio-balloon: define new feature bit and head struct

[Li, Liang Z]

> Sent: Wednesday, January 18, 2017 3:11 AM
> To: Li, Liang Z
> Cc: k...@vger.kernel.org; virtio-...@lists.oasis-open.org; qemu-
> de...@nongnu.org; linux...@kvack.org; linux-ker...@vger.kernel.org;
> virtualizat...@lists.linux-foundation.org; amit.s...@redhat.com; Hansen,
> Dave; cornelia.h...@de.ibm.com; pbonz...@redhat.com;
> da...@redhat.com; aarca...@redhat.com; dgilb...@redhat.com;
> quint...@redhat.com
> Subject: Re: [virtio-dev] Re: [PATCH v6 kernel 2/5] virtio-balloon: define new
> feature bit and head struct
> 
> On Fri, Jan 13, 2017 at 09:24:22AM +, Li, Liang Z wrote:
> > > On Wed, Dec 21, 2016 at 02:52:25PM +0800, Liang Li wrote:
> > > > Add a new feature which supports sending the page information with
> > > > range array. The current implementation uses PFNs array, which is
> > > > not very efficient. Using ranges can improve the performance of
> > > > inflating/deflating significantly.
> > > >
> > > > Signed-off-by: Liang Li 
> > > > Cc: Michael S. Tsirkin 
> > > > Cc: Paolo Bonzini 
> > > > Cc: Cornelia Huck 
> > > > Cc: Amit Shah 
> > > > Cc: Dave Hansen 
> > > > Cc: Andrea Arcangeli 
> > > > Cc: David Hildenbrand 
> > > > ---
> > > >  include/uapi/linux/virtio_balloon.h | 12 
> > > >  1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/include/uapi/linux/virtio_balloon.h
> > > > b/include/uapi/linux/virtio_balloon.h
> > > > index 343d7dd..2f850bf 100644
> > > > --- a/include/uapi/linux/virtio_balloon.h
> > > > +++ b/include/uapi/linux/virtio_balloon.h
> > > > @@ -34,10 +34,14 @@
> > > >  #define VIRTIO_BALLOON_F_MUST_TELL_HOST0 /* Tell before
> > > reclaiming pages */
> > > >  #define VIRTIO_BALLOON_F_STATS_VQ  1 /* Memory Stats virtqueue
> > > */
> > > >  #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM2 /* Deflate
> balloon
> > > on OOM */
> > > > +#define VIRTIO_BALLOON_F_PAGE_RANGE3 /* Send page info
> > > with ranges */
> > > >
> > > >  /* Size of a PFN in the balloon interface. */  #define
> > > > VIRTIO_BALLOON_PFN_SHIFT 12
> > > >
> > > > +/* Bits width for the length of the pfn range */
> > >
> > > What does this mean? Couldn't figure it out.
> > >
> > > > +#define VIRTIO_BALLOON_NR_PFN_BITS 12
> > > > +
> > > >  struct virtio_balloon_config {
> > > > /* Number of pages host wants Guest to give up. */
> > > > __u32 num_pages;
> > > > @@ -82,4 +86,12 @@ struct virtio_balloon_stat {
> > > > __virtio64 val;
> > > >  } __attribute__((packed));
> > > >
> > > > +/* Response header structure */
> > > > +struct virtio_balloon_resp_hdr {
> > > > +   __le64 cmd : 8; /* Distinguish different requests type */
> > > > +   __le64 flag: 8; /* Mark status for a specific request type */
> > > > +   __le64 id : 16; /* Distinguish requests of a specific type */
> > > > +   __le64 data_len: 32; /* Length of the following data, in bytes
> > > > +*/
> > >
> > > This use of __le64 makes no sense.  Just use u8/le16/le32 pls.
> > >
> >
> > Got it, will change in the next version.
> >
> > And could help take a look at other parts? as well as the QEMU part.
> >
> > Thanks!
> > Liang
> 
> Yes but first I would like to understand how come no fields in this new
> structure come up if I search for them in the following patch. I don't see why

It's not true, all of the field will be referenced in the following patches 
except 
the 'reserved' filed.

> should I waste time on reviewing the implementation if the interface isn't
> reasonable. You don't have to waste it too - just send RFC patches with the
> header until we can agree on it.

OK. I will post the header part separately.

Thanks!
Liang
> 
> --
> MST
> 
> -
> To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [Qemu-devel] [PATCH v2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Yang Ziyue]

Oops seems I forgot to check the patch before submitting. Sorry for that.
The new patch is tagged as v3 now, but I'm not sure whether it should
be tagged as v2 since the origin v2 failed. Is there any specification
about this? Thanks!

2017-01-18 9:19 GMT+08:00  :
> Hi,
>
> Your series seems to have some coding style problems. See output below for
> more information:
>
> Message-id: 1484702052-7900-1-git-send-email-skiver.cloud@gmail.com
> Type: series
> Subject: [Qemu-devel] [PATCH v2] gdbstub.c: fix GDB connection segfault 
> caused by empty machines
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
>
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
>
> # Useful git options
> git config --local diff.renamelimit 0
> git config --local diff.renames True
>
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
> then
> failed=1
> echo
> fi
> n=$((n+1))
> done
>
> exit $failed
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag] 
> patchew/1484702052-7900-1-git-send-email-skiver.cloud@gmail.com -> 
> patchew/1484702052-7900-1-git-send-email-skiver.cloud@gmail.com
> Switched to a new branch 'test'
> 4daee16 gdbstub.c: fix GDB connection segfault caused by empty machines
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/1: gdbstub.c: fix GDB connection segfault caused by empty 
> machines...
> ERROR: Error messages should not contain newlines
> #48: FILE: gdbstub.c:640:
> +error_report("Error: Bad gdb register numbering for '%s'\n"
>
> ERROR: Error messages should not contain newlines
> #49: FILE: gdbstub.c:641:
> + "Expected %d got %d\n", xml, g_pos, s->base_reg);
>
> ERROR: Error messages should not contain newlines
> #58: FILE: gdbstub.c:893:
> +error_report("\nQEMU: Terminated via GDBstub\n");
>
> ERROR: Error messages should not contain newlines
> #68: FILE: gdbstub.c:1361:
> +error_report("gdbstub: Bad syscall format string '%s'\n",
>
> ERROR: Error messages should not contain newlines
> #79: FILE: gdbstub.c:1737:
> + "machine without any CPU.\n");
>
> total: 5 errors, 0 warnings, 47 lines checked
>
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PATCH v3] gdbstub.c: fix GDB connection segfault caused by empty machines

[Ziyue Yang]

From: Ziyue Yang 

This patch is to fix the segmentation fault caused by attaching
GDB to a QEMU instance initialized with "-M none" option.

The bug can be reproduced by

> ./qemu-system-x86_64 -M none -nographic -S -s

and attach a GDB to it by

> gdb -ex 'target remote :1234

The segmentation fault was originally caused by trying to read
the information about CPU when communicating with GDB. However,
it's impossible for any control flow to exist on an empty machine,
nor can CPU's be hot plugged to an empty machine later by QOM
commands. So I think simply disabling GDB connections on empty
machines makes sense.

Also some updates from fprintf(stderr, ...) to error_report.

Signed-off-by: Ziyue Yang 
---
 gdbstub.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index de62d26..fe1d0f8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -18,6 +18,7 @@
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qemu/cutils.h"
 #include "cpu.h"
 #ifdef CONFIG_USER_ONLY
@@ -636,8 +637,8 @@ void gdb_register_coprocessor(CPUState *cpu,
 *p = s;
 if (g_pos) {
 if (g_pos != s->base_reg) {
-fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
-"Expected %d got %d\n", xml, g_pos, s->base_reg);
+error_report("Error: Bad gdb register numbering for '%s', "
+ "Expected %d got %d", xml, g_pos, s->base_reg);
 } else {
 cpu->gdb_num_g_regs = cpu->gdb_num_regs;
 }
@@ -889,7 +890,7 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 case 'k':
 /* Kill the target */
-fprintf(stderr, "\nQEMU: Terminated via GDBstub\n");
+error_report("QEMU: Terminated via GDBstub");
 exit(0);
 case 'D':
 /* Detach packet */
@@ -1357,8 +1358,8 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const 
char *fmt, va_list va)
 break;
 default:
 bad_format:
-fprintf(stderr, "gdbstub: Bad syscall format string '%s'\n",
-fmt - 1);
+error_report("gdbstub: Bad syscall format string '%s'",
+ fmt - 1);
 break;
 }
 } else {
@@ -1731,6 +1732,12 @@ int gdbserver_start(const char *device)
 CharDriverState *mon_chr;
 ChardevCommon common = { 0 };

+if (!first_cpu) {
+error_report("gdbstub: meaningless to attach gdb to a "
+ "machine without any CPU.");
+return -1;
+}
+
 if (!device)
 return -1;
 if (strcmp(device, "none") != 0) {
--
2.7.4

Re: [Qemu-devel] [PATCH v2] gdbstub.c: fix GDB connection segfault caused by empty machines

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Message-id: 1484702052-7900-1-git-send-email-skiver.cloud@gmail.com
Type: series
Subject: [Qemu-devel] [PATCH v2] gdbstub.c: fix GDB connection segfault caused 
by empty machines

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/1484702052-7900-1-git-send-email-skiver.cloud@gmail.com -> 
patchew/1484702052-7900-1-git-send-email-skiver.cloud@gmail.com
Switched to a new branch 'test'
4daee16 gdbstub.c: fix GDB connection segfault caused by empty machines

=== OUTPUT BEGIN ===
Checking PATCH 1/1: gdbstub.c: fix GDB connection segfault caused by empty 
machines...
ERROR: Error messages should not contain newlines
#48: FILE: gdbstub.c:640:
+error_report("Error: Bad gdb register numbering for '%s'\n"

ERROR: Error messages should not contain newlines
#49: FILE: gdbstub.c:641:
+ "Expected %d got %d\n", xml, g_pos, s->base_reg);

ERROR: Error messages should not contain newlines
#58: FILE: gdbstub.c:893:
+error_report("\nQEMU: Terminated via GDBstub\n");

ERROR: Error messages should not contain newlines
#68: FILE: gdbstub.c:1361:
+error_report("gdbstub: Bad syscall format string '%s'\n",

ERROR: Error messages should not contain newlines
#79: FILE: gdbstub.c:1737:
+ "machine without any CPU.\n");

total: 5 errors, 0 warnings, 47 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH] gdbstub.c: fix GDB connection segfault caused by empty machines

[Yang Ziyue]

2017-01-18 1:27 GMT+08:00 Thomas Huth :
>
> On 17.01.2017 16:19, Ziyue Yang wrote:
> > From: Ziyue Yang 
> >
> > This patch is to fix the segmentation fault caused by attaching
> > GDB to a QEMU instance initialized with "-M none" option.
> >
> > The bug can be reproduced by
> >
> >> ./qemu-system-x86_64 -M none -nographic -S -s
> >
> > and attach a GDB to it by
> >
> >> gdb -ex 'target remote :1234
> >
> > The segmentation fault was originally caused by trying to read
> > the information about CPU when communicating with GDB. However,
> > it's impossible for any control flow to exist on an empty machine,
> > nor can CPU's be hot plugged to an empty machine later by QOM
> > commands. So I think simply disabling GDB connections on empty
> > machines makes sense.
>
> Yes, this sounds like a proper and easy fix for the problem.
>
> > Signed-off-by: Ziyue Yang 
> > ---
> >  gdbstub.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/gdbstub.c b/gdbstub.c
> > index de62d26..413e817 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -1731,6 +1731,12 @@ int gdbserver_start(const char *device)
> >  CharDriverState *mon_chr;
> >  ChardevCommon common = { 0 };
> >
> > +if (!first_cpu) {
> > +fprintf(stderr, "gdbstub: meaningless to attach gdb to a "
> > +"machine without any CPU.\n");
> > +return -1;
> > +}
>
> Could you maybe rather use error_report() instead of fprintf()? I think
> that's the preferred way to print out an error in QEMU nowadays.

Sure. I've sent a v2 version patch fixing that.

>
>
>  Thomas
>

[Qemu-devel] [PATCH v2] gdbstub.c: fix GDB connection segfault caused by empty machines

[Ziyue Yang]

From: Ziyue Yang 

This patch is to fix the segmentation fault caused by attaching
GDB to a QEMU instance initialized with "-M none" option.

The bug can be reproduced by

> ./qemu-system-x86_64 -M none -nographic -S -s

and attach a GDB to it by

> gdb -ex 'target remote :1234

The segmentation fault was originally caused by trying to read
the information about CPU when communicating with GDB. However,
it's impossible for any control flow to exist on an empty machine,
nor can CPU's be hot plugged to an empty machine later by QOM
commands. So I think simply disabling GDB connections on empty
machines makes sense.

Also some updates from fprintf(stderr, ...) to error_report.

Signed-off-by: Ziyue Yang 
---
 gdbstub.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index de62d26..3a22ce3 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -18,6 +18,7 @@
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qemu/cutils.h"
 #include "cpu.h"
 #ifdef CONFIG_USER_ONLY
@@ -636,8 +637,8 @@ void gdb_register_coprocessor(CPUState *cpu,
 *p = s;
 if (g_pos) {
 if (g_pos != s->base_reg) {
-fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
-"Expected %d got %d\n", xml, g_pos, s->base_reg);
+error_report("Error: Bad gdb register numbering for '%s'\n"
+ "Expected %d got %d\n", xml, g_pos, s->base_reg);
 } else {
 cpu->gdb_num_g_regs = cpu->gdb_num_regs;
 }
@@ -889,7 +890,7 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 case 'k':
 /* Kill the target */
-fprintf(stderr, "\nQEMU: Terminated via GDBstub\n");
+error_report("\nQEMU: Terminated via GDBstub\n");
 exit(0);
 case 'D':
 /* Detach packet */
@@ -1357,8 +1358,8 @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const 
char *fmt, va_list va)
 break;
 default:
 bad_format:
-fprintf(stderr, "gdbstub: Bad syscall format string '%s'\n",
-fmt - 1);
+error_report("gdbstub: Bad syscall format string '%s'\n",
+ fmt - 1);
 break;
 }
 } else {
@@ -1731,6 +1732,12 @@ int gdbserver_start(const char *device)
 CharDriverState *mon_chr;
 ChardevCommon common = { 0 };

+if (!first_cpu) {
+error_report("gdbstub: meaningless to attach gdb to a "
+ "machine without any CPU.\n");
+return -1;
+}
+
 if (!device)
 return -1;
 if (strcmp(device, "none") != 0) {
--
2.7.4

Re: [Qemu-devel] [PATCH v2 3/6] ACPI: Add Virtual Machine Generation ID support

[Ben Warren]

Hi Igor,
> On Jan 17, 2017, at 5:00 AM, Igor Mammedov  wrote:
> 
> On Mon, 16 Jan 2017 11:20:55 -0800
> b...@skyportsystems.com wrote:
> 
>> From: Ben Warren 
>> 
>> This implements the VM Generation ID feature by passing a 128-bit
>> GUID to the guest via a fw_cfg blob.
>> Any time the GUID changes, and ACPI notify event is sent to the guest
>> 
>> The user interface is a simple device with one parameters:
>> - guid (string, must be in UUID format
>>   ----)
>> 
>> Signed-off-by: Ben Warren 
>> ---
>> default-configs/i386-softmmu.mak   |   1 +
>> default-configs/x86_64-softmmu.mak |   1 +
>> hw/acpi/Makefile.objs  |   1 +
>> hw/acpi/vmgenid.c  | 207 
>> +
>> hw/i386/acpi-build.c   |   5 +
>> hw/misc/Makefile.objs  |   1 +
>> include/hw/acpi/vmgenid.h  |  24 +
>> 7 files changed, 240 insertions(+)
>> create mode 100644 hw/acpi/vmgenid.c
>> create mode 100644 include/hw/acpi/vmgenid.h
>> 
>> diff --git a/default-configs/i386-softmmu.mak 
>> b/default-configs/i386-softmmu.mak
>> index 0b51360..b2bccf6 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> CONFIG_SMBIOS=y
>> CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>> +CONFIG_ACPI_VMGENID=y
>> diff --git a/default-configs/x86_64-softmmu.mak 
>> b/default-configs/x86_64-softmmu.mak
>> index 7f89503..c6bd310 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -56,3 +56,4 @@ CONFIG_IOH3420=y
>> CONFIG_I82801B11=y
>> CONFIG_SMBIOS=y
>> CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
>> +CONFIG_ACPI_VMGENID=y
>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>> index 489e63b..7dc95cd 100644
>> --- a/hw/acpi/Makefile.objs
>> +++ b/hw/acpi/Makefile.objs
>> @@ -4,6 +4,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>> common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o 
>> memory_hotplug_acpi_table.o
>> common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o
>> common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
>> +common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o
>> common-obj-$(CONFIG_ACPI) += acpi_interface.o
>> common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>> common-obj-$(CONFIG_ACPI) += aml-build.o
>> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
>> new file mode 100644
>> index 000..596c8b7
>> --- /dev/null
>> +++ b/hw/acpi/vmgenid.c
>> @@ -0,0 +1,207 @@
>> +/*
>> + *  Virtual Machine Generation ID Device
>> + *
>> + *  Copyright (C) 2016 Skyport Systems.
>> + *
>> + *  Authors: Ben Warren 
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qmp-commands.h"
>> +#include "hw/acpi/acpi.h"
>> +#include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/vmgenid.h"
>> +#include "hw/nvram/fw_cfg.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +Object *find_vmgenid_dev(Error **errp)
>> +{
>> +Object *obj = object_resolve_path_type("", VMGENID_DEVICE, NULL);
>> +if (!obj) {
>> +error_setg(errp, VMGENID_DEVICE " is not found");
>> +}
>> +return obj;
>> +}
>> +
>> +void vmgenid_build_acpi(GArray *table_offsets, GArray *table_data,
>> +BIOSLinker *linker)
> wrong alignment, should be
> 
>   f12345(arg1,
>  arg2);
> 
OK
>> +{
>> +Object *obj = find_vmgenid_dev(NULL);
>> +if (!obj) {
>> +return;
>> +}
>> +VmGenIdState *s = VMGENID(obj);
>> +
>> +acpi_add_table(table_offsets, table_data);
>> +
>> +GArray *guid = g_array_new(false, true, sizeof(s->guid.data));
>> +g_array_append_val(guid, s->guid.data);
>> +
>> +Aml *ssdt, *dev, *scope, *pkg, *method;
> Pls read CODING_STYLE and amend patch accordingly.
> 
I assume you’re concerned about mixing variable declarations and code.  I’ll 
fix.
>> +
>> +/* Put this in a separate SSDT table */
>> +ssdt = init_aml_allocator();
>> +
>> +/* Reserve space for header */
>> +acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>> +
>> +/* Storage for the GUID address */
>> +uint32_t vgia_offset = table_data->len +
>> +build_append_named_qword(ssdt->buf, "VGIA");
>> +dev = aml_device("VGEN");
>> +scope = aml_scope("\\_SB");
> swap scope and dev
> 
OK
>> +aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID")));
>> +aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter")));
>> +aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter")));
>> +
>> +/* Simple status method to check that address is linked and non-zero */
>> +method = aml_method("_STA", 0, AML_NOTSERIALIZED);
>> +Aml *if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0)));
>> +

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi: Implement fc_host feature

[Fam Zheng]

On Tue, 01/17 10:54, Stefan Hajnoczi wrote:
> On Tue, Jan 17, 2017 at 12:07:30AM +0800, Fam Zheng wrote:
> >  static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
> >  {
> >  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >  VirtIOSCSI *s = VIRTIO_SCSI(dev);
> > +VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
> >  Error *err = NULL;
> >  
> > +if (vs->conf.fc_host) {
> > +if (!strcmp(vs->conf.fc_host, "off")) {
> > +vs->conf.primary_wwpn = 0;
> > +vs->conf.primary_wwnn = 0;
> > +vs->conf.secondary_wwpn = 0;
> > +vs->conf.secondary_wwnn = 0;
> > +} else if (!strcmp(vs->conf.fc_host, "primary") ||
> > +   !strcmp(vs->conf.fc_host, "secondary")) {
> > +virtio_add_feature(>host_features, 
> > VIRTIO_SCSI_F_FC_HOST);
> > +vs->conf.primary_active = !strcmp(vs->conf.fc_host, "primary");
> > +if (!vs->conf.primary_wwpn) {
> > +error_setg(errp, "fc_host enabled but primary_wwpn not 
> > set");
> > +return;
> > +}
> > +if (!vs->conf.primary_wwnn) {
> > +error_setg(errp, "fc_host enabled but primary_wwnn not 
> > set");
> > +return;
> > +}
> > +if (!vs->conf.secondary_wwpn) {
> > +error_setg(errp, "fc_host enabled but secondary_wwpn not 
> > set");
> > +return;
> > +}
> > +if (!vs->conf.secondary_wwnn) {
> > +error_setg(errp, "fc_host enabled but secondary_wwnn not 
> > set");
> > +return;
> > +}
> > +s->vm_state_change =
> > +
> > qemu_add_vm_change_state_handler(virtio_scsi_vm_state_change, s);
> 
> Missing qemu_del_vm_change_state_handler() in .unrealize() and in error
> code paths in this function.

Will fix it. Thanks!

Fam

Re: [Qemu-devel] [PATCH] ppc: Remove unused function cpu_ppc601_rtc_init()

[David Gibson]

On Tue, Jan 17, 2017 at 02:37:49PM +0100, Thomas Huth wrote:
> It is completely unused, thus it can be removed without problems.
> 
> Signed-off-by: Thomas Huth 

Applied to ppc-for-2.9, thanks.

> ---
>  hw/ppc/ppc.c | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 8945869..f9a4b51 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -950,13 +950,6 @@ clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t 
> freq)
>  }
>  
>  /* Specific helpers for POWER & PowerPC 601 RTC */
> -#if 0
> -static clk_setup_cb cpu_ppc601_rtc_init (CPUPPCState *env)
> -{
> -return cpu_ppc_tb_init(env, 7812500);
> -}
> -#endif
> -
>  void cpu_ppc601_store_rtcu (CPUPPCState *env, uint32_t value)
>  {
>  _cpu_ppc_store_tbu(env, value);

-- 
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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v4 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Mon, Jan 16, 2017 at 06:05:26PM +0100, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>> > On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
[...]
>> >> +
>> >> +} else {
>> >> +/* proxy to next handler */
>> >> +if (segv_next.sa_sigaction != NULL) {
>> >> +segv_next.sa_sigaction(signum, siginfo, sigctxt);
>> >> +} else if (segv_next.sa_handler != NULL) {
>> >> +segv_next.sa_handler(signum);
>> >> +}
>> 
>> > Is there a case when no signal handler was installed (i.e. default
>> > action)?
>> 
>> Yes, before calling hypertrace_init() or if it is called without a
>> "hypertrace_base" argument set (i.e., the user has not enabled hypertrace in 
>> the
>> command line).

> I meant "what happens if !segv_next.sa_action &&
> !segv_next.sa_handler?".  The default signal disposition should take
> effect.  This code is ignoring that case, turning everything into
> SIG_IGN but there is also SIG_DFL.

I see, I didn't take SIG_DFL and SIG_IGN into account, and if both are null (no
handler was installed by the user), I should cleanup my handler and raise() the
signal again to let it go through its default system action.

Sorry for the overall messy patch.


Thanks,
  Lluis

[Qemu-devel] [PULL 1/2] Revert "tcg/i386: Rely on undefined/undocumented behaviour of BSF/BSR"

[Richard Henderson]

This reverts commit 4ac76910734209dab83ddd3795f08fc7889ef463.

This fixes
  http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg03062.html

While I think we could get away with relying on the undocumented
behaviour, the tcg constraint system isn't powerful enough to
properly describe the required (non-)overlap conditions.

Reported-by: Eduardo Habkost 
Signed-off-by: Richard Henderson 
---
 tcg/i386/tcg-target.inc.c | 35 +--
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 01177a9..6489b73 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -1148,12 +1148,9 @@ static void tcg_out_ctz(TCGContext *s, int rexw, TCGReg 
dest, TCGReg arg1,
 tcg_debug_assert(arg2 == (rexw ? 64 : 32));
 tcg_out_modrm(s, OPC_TZCNT + rexw, dest, arg1);
 } else {
-/* ??? The manual says that the output is undefined when the
-   input is zero, but real hardware leaves it unchanged.  As
-   noted in target-i386/translate.c, real programs depend on
-   this -- now we are one more of those.  */
-tcg_debug_assert(dest == arg2);
+tcg_debug_assert(dest != arg2);
 tcg_out_modrm(s, OPC_BSF + rexw, dest, arg1);
+tcg_out_cmov(s, TCG_COND_EQ, rexw, dest, arg2);
 }
 }
 
@@ -1166,26 +1163,20 @@ static void tcg_out_clz(TCGContext *s, int rexw, TCGReg 
dest, TCGReg arg1,
 tcg_debug_assert(arg2 == (rexw ? 64 : 32));
 } else {
 tcg_debug_assert(dest != arg2);
-/* LZCNT sets C if the input was zero.  */
 tcg_out_cmov(s, TCG_COND_LTU, rexw, dest, arg2);
 }
 } else {
-TCGType type = rexw ? TCG_TYPE_I64: TCG_TYPE_I32;
-TCGArg rev = rexw ? 63 : 31;
+tcg_debug_assert(!const_a2);
+tcg_debug_assert(dest != arg1);
+tcg_debug_assert(dest != arg2);
 
-/* Recall that the output of BSR is the index not the count.
-   Therefore we must adjust the result by ^ (SIZE-1).  In some
-   cases below, we prefer an extra XOR to a JMP.  */
-/* ??? See the comment in tcg_out_ctz re BSF.  */
-if (const_a2) {
-tcg_debug_assert(dest != arg1);
-tcg_out_movi(s, type, dest, arg2 ^ rev);
-} else {
-tcg_debug_assert(dest == arg2);
-tgen_arithi(s, ARITH_XOR + rexw, dest, rev, 0);
-}
+/* Recall that the output of BSR is the index not the count.  */
 tcg_out_modrm(s, OPC_BSR + rexw, dest, arg1);
-tgen_arithi(s, ARITH_XOR + rexw, dest, rev, 0);
+tgen_arithi(s, ARITH_XOR + rexw, dest, rexw ? 63 : 31, 0);
+
+/* Since we have destroyed the flags from BSR, we have to re-test.  */
+tcg_out_cmp(s, arg1, 0, 1, rexw);
+tcg_out_cmov(s, TCG_COND_EQ, rexw, dest, arg2);
 }
 }
 
@@ -2459,7 +2450,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode 
op)
 case INDEX_op_ctz_i64:
 {
 static const TCGTargetOpDef ctz[2] = {
-{ .args_ct_str = { "r", "r", "0" } },
+{ .args_ct_str = { "", "r", "r" } },
 { .args_ct_str = { "", "r", "rW" } },
 };
 return [have_bmi1];
@@ -2468,7 +2459,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode 
op)
 case INDEX_op_clz_i64:
 {
 static const TCGTargetOpDef clz[2] = {
-{ .args_ct_str = { "", "r", "0i" } },
+{ .args_ct_str = { "", "r", "r" } },
 { .args_ct_str = { "", "r", "rW" } },
 };
 return [have_lzcnt];
-- 
2.9.3

[Qemu-devel] [PULL 0/2] Fixes for x86 host

[Richard Henderson]

Fixing a regression reported by Eduardo.


r~


The following changes since commit 23eb9e6b6d5315171cc15969bbc755f258004df0:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-01-16' into 
staging (2017-01-17 13:53:50 +)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20170117

for you to fetch changes up to 39f099ec9d6d420b6fe6f7f4f8ed80ae29c65ff2:

  tcg/i386: Always use TZCNT when available (2017-01-17 12:02:08 -0800)


tcg/i386 fixes


Richard Henderson (2):
  Revert "tcg/i386: Rely on undefined/undocumented behaviour of BSF/BSR"
  tcg/i386: Always use TZCNT when available

 tcg/i386/tcg-target.inc.c | 45 -
 1 file changed, 20 insertions(+), 25 deletions(-)

[Qemu-devel] [PULL 2/2] tcg/i386: Always use TZCNT when available

[Richard Henderson]

I think this is cleaner than sometimes using BSF.

Signed-off-by: Richard Henderson 
---
 tcg/i386/tcg-target.inc.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 6489b73..5918008 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -1143,10 +1143,14 @@ static void tcg_out_movcond64(TCGContext *s, TCGCond 
cond, TCGReg dest,
 static void tcg_out_ctz(TCGContext *s, int rexw, TCGReg dest, TCGReg arg1,
 TCGArg arg2, bool const_a2)
 {
-if (const_a2) {
-tcg_debug_assert(have_bmi1);
-tcg_debug_assert(arg2 == (rexw ? 64 : 32));
+if (have_bmi1) {
 tcg_out_modrm(s, OPC_TZCNT + rexw, dest, arg1);
+if (const_a2) {
+tcg_debug_assert(arg2 == (rexw ? 64 : 32));
+} else {
+tcg_debug_assert(dest != arg2);
+tcg_out_cmov(s, TCG_COND_LTU, rexw, dest, arg2);
+}
 } else {
 tcg_debug_assert(dest != arg2);
 tcg_out_modrm(s, OPC_BSF + rexw, dest, arg1);
-- 
2.9.3

Re: [Qemu-devel] [PATCH RFC v2 09/12] vfio/ccw: get irqs info and set the eventfd fd

[Alex Williamson]

On Thu, 12 Jan 2017 08:25:10 +0100
Dong Jia Shi  wrote:

> From: Xiao Feng Ren 
> 
> vfio-ccw resorts to the eventfd mechanism to communicate with userspace.
> We fetch the irqs info via the ioctl VFIO_DEVICE_GET_IRQ_INFO,
> register a event notifier to get the eventfd fd which is sent
> to kernel via the ioctl VFIO_DEVICE_SET_IRQS, then we can implement
> read operation once kernel sends the signal.
> 
> Signed-off-by: Xiao Feng Ren 
> ---
>  hw/vfio/ccw.c | 102 
> ++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 93394c2..c6bfce7 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -21,6 +21,7 @@
>  #include "hw/vfio/vfio-common.h"
>  #include "hw/s390x/s390-ccw.h"
>  #include "hw/s390x/ccw-device.h"
> +#include "qemu/error-report.h"
>  #include "standard-headers/asm-s390/vfio_ccw.h"
>  
>  #define TYPE_VFIO_CCW "vfio-ccw"
> @@ -30,6 +31,7 @@ typedef struct VFIOCCWDevice {
>  uint64_t io_region_size;
>  uint64_t io_region_offset;
>  struct ccw_io_region *io_region;
> +EventNotifier io_notifier;
>  } VFIOCCWDevice;
>  
>  static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> @@ -54,6 +56,98 @@ static void vfio_ccw_reset(DeviceState *dev)
>  ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
>  }
>  
> +static void vfio_ccw_io_notifier_handler(void *opaque)
> +{
> +VFIOCCWDevice *vcdev = opaque;
> +
> +if (!event_notifier_test_and_clear(>io_notifier)) {
> +return;
> +}
> +}
> +
> +static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp)
> +{
> +VFIODevice *vdev = >vdev;
> +struct vfio_irq_info *irq_info;
> +struct vfio_irq_set *irq_set;
> +size_t argsz;
> +int32_t *pfd;
> +
> +if (vdev->num_irqs != VFIO_CCW_NUM_IRQS) {
> +error_setg(errp, "vfio: unexpected number of io irqs %u",
> +   vdev->num_irqs);
> +return;
> +}

Same issue here as with region info, we may find reasons to add new
interrupts in the kernel, it's happened for PCI support with things
like device error recovery and requests to release the device.  Don't
lock us into never expanding these, just make sure that the index that
this implementation depends on is here.

> +
> +argsz = sizeof(*irq_set);
> +irq_info = g_malloc0(argsz);
> +irq_info->index = VFIO_CCW_IO_IRQ_INDEX;
> +irq_info->argsz = argsz;
> +if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
> +  irq_info) < 0 || irq_info->count < 1) {
> +error_setg(errp, "vfio: Error getting irq info");
> +goto get_error;
> +}
> +
> +if (event_notifier_init(>io_notifier, 0)) {
> +error_setg(errp, "vfio: Unable to init event notifier for IO");
> +goto get_error;
> +}
> +
> +argsz = sizeof(*irq_set) + sizeof(*pfd);
> +irq_set = g_malloc0(argsz);
> +irq_set->argsz = argsz;
> +irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> +irq_set->index = VFIO_CCW_IO_IRQ_INDEX;
> +irq_set->start = 0;
> +irq_set->count = 1;
> +pfd = (int32_t *) _set->data;
> +
> +*pfd = event_notifier_get_fd(>io_notifier);
> +qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev);
> +if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> +error_setg(errp, "vfio: Failed to set up io notification");
> +qemu_set_fd_handler(*pfd, NULL, NULL, vcdev);
> +event_notifier_cleanup(>io_notifier);
> +goto set_error;
> +}
> +
> +set_error:
> +g_free(irq_set);
> +
> +get_error:
> +g_free(irq_info);
> +}
> +
> +static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev)
> +{
> +struct vfio_irq_set *irq_set;
> +size_t argsz;
> +int32_t *pfd;
> +
> +argsz = sizeof(*irq_set) + sizeof(*pfd);
> +irq_set = g_malloc0(argsz);
> +irq_set->argsz = argsz;
> +irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> +irq_set->index = VFIO_CCW_IO_IRQ_INDEX;
> +irq_set->start = 0;
> +irq_set->count = 1;
> +pfd = (int32_t *) _set->data;
> +*pfd = -1;
> +
> +if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> +error_report("vfio: Failed to de-assign device io fd");
> +}
> +
> +qemu_set_fd_handler(event_notifier_get_fd(>io_notifier),
> +NULL, NULL, vcdev);
> +event_notifier_cleanup(>io_notifier);
> +
> +g_free(irq_set);
> +}
> +
>  static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
>  {
>  VFIODevice *vdev = >vdev;
> @@ -193,8 +287,15 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
> **errp)
>  goto out_region_err;
>  }
>  
> +vfio_ccw_register_io_notifier(vcdev, errp);
> +if (*errp) {
> +goto out_notifier_err;
> +}
> +

Re: [Qemu-devel] [PATCH RFC v2 08/12] vfio/ccw: get io region info

[Alex Williamson]

On Thu, 12 Jan 2017 08:25:09 +0100
Dong Jia Shi  wrote:

> vfio-ccw provides an MMIO region for I/O operations. We fetch its
> information via ioctls here, then we can use it performing I/O
> instructions and retrieving I/O results later on.
> 
> Signed-off-by: Xiao Feng Ren 
> ---
>  hw/vfio/ccw.c | 52 
>  1 file changed, 52 insertions(+)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 881010b..93394c2 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -21,11 +21,15 @@
>  #include "hw/vfio/vfio-common.h"
>  #include "hw/s390x/s390-ccw.h"
>  #include "hw/s390x/ccw-device.h"
> +#include "standard-headers/asm-s390/vfio_ccw.h"
>  
>  #define TYPE_VFIO_CCW "vfio-ccw"
>  typedef struct VFIOCCWDevice {
>  S390CCWDevice cdev;
>  VFIODevice vdev;
> +uint64_t io_region_size;
> +uint64_t io_region_offset;
> +struct ccw_io_region *io_region;
>  } VFIOCCWDevice;
>  
>  static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> @@ -50,6 +54,46 @@ static void vfio_ccw_reset(DeviceState *dev)
>  ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
>  }
>  
> +static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
> +{
> +VFIODevice *vdev = >vdev;
> +struct vfio_region_info *info;
> +int ret;
> +
> +/* Sanity check device */
> +if (!(vdev->flags & VFIO_DEVICE_FLAGS_CCW)) {
> +error_setg(errp, "vfio: Um, this isn't a vfio-ccw device");
> +return;
> +}
> +
> +if (vdev->num_regions != VFIO_CCW_NUM_REGIONS) {
> +error_setg(errp, "vfio: Unexpected number of the I/O region %u",
> +   vdev->num_regions);
> +return;
> +}

I think you want < here, not !=, otherwise you we can never add another
region to describe something new on a vfio-ccw device without breaking
this code.  You've defined in the uapi that region index zero is always
VFIO_CCW_CONFIG_REGION_INDEX and this code only depends on that, no
matter what additional regions might be added.  That's done below.

> +
> +ret = vfio_get_region_info(vdev, VFIO_CCW_CONFIG_REGION_INDEX, );
> +if (ret) {
> +error_setg(errp, "vfio: Error getting config info: %d", ret);
> +return;
> +}
> +
> +vcdev->io_region_size = info->size;
> +if (sizeof(*vcdev->io_region) != vcdev->io_region_size) {
> +error_setg(errp, "vfio: Unexpected size of the I/O region");
> +return;
> +}
> +vcdev->io_region_offset = info->offset;
> +vcdev->io_region = g_malloc0(info->size);
> +
> +g_free(info);
> +}
> +
> +static void vfio_ccw_put_region(VFIOCCWDevice *vcdev)
> +{
> +g_free(vcdev->io_region);
> +}
> +
>  static void vfio_put_device(VFIOCCWDevice *vcdev)
>  {
>  g_free(vcdev->vdev.name);
> @@ -144,8 +188,15 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
> **errp)
>  goto out_device_err;
>  }
>  
> +vfio_ccw_get_region(vcdev, errp);
> +if (*errp) {
> +goto out_region_err;
> +}
> +
>  return;
>  
> +out_region_err:
> +vfio_put_device(vcdev);
>  out_device_err:
>  vfio_ccw_put_group(group, path);
>  out_group_err:
> @@ -166,6 +217,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error 
> **errp)
>  cdc->unrealize(cdev, errp);
>  }
>  
> +vfio_ccw_put_region(vcdev);
>  vfio_put_device(vcdev);
>  vfio_put_group(group);
>  }

Re: [Qemu-devel] [PATCH RFC v2 07/12] vfio/ccw: vfio based subchannel passthrough driver

[Alex Williamson]

On Thu, 12 Jan 2017 08:25:08 +0100
Dong Jia Shi  wrote:

> From: Xiao Feng Ren 
> 
> We use the IOMMU_TYPE1 of VFIO to realize the subchannels
> passthrough, implement a vfio based subchannels passthrough
> driver called "vfio-ccw".
> 
> Support qemu parameters in the style of:
> "-device vfio-ccw,id=xx,hostid=xx(,guestid=xx),mdevid=xx"

Why not adopt the same syntax as vfio-pci with mdev devices,
sysfsdev=%s where %s is a path to the mdev device in sysfs, which can
be /sys/bus/mdev/devices/$UUID or as you create from the hostid
below /sys/bus/css/devices/%x.%x.%04x/$UUID.  It seems if we know where
to find the mdev device then we can determine the parent information on
our own through sysfs.

> Signed-off-by: Xiao Feng Ren 
> ---
>  default-configs/s390x-softmmu.mak |   1 +
>  hw/s390x/s390-ccw.h   |   1 +
>  hw/vfio/Makefile.objs |   1 +
>  hw/vfio/ccw.c | 209 
> ++
>  include/hw/vfio/vfio-common.h |   1 +
>  5 files changed, 213 insertions(+)
>  create mode 100644 hw/vfio/ccw.c
> 
> diff --git a/default-configs/s390x-softmmu.mak 
> b/default-configs/s390x-softmmu.mak
> index 36e15de..5576b0a 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -4,4 +4,5 @@ CONFIG_VIRTIO=y
>  CONFIG_SCLPCONSOLE=y
>  CONFIG_S390_FLIC=y
>  CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
> +CONFIG_VFIO_CCW=$(CONFIG_LINUX)
>  CONFIG_WDT_DIAG288=y
> diff --git a/hw/s390x/s390-ccw.h b/hw/s390x/s390-ccw.h
> index 9ced8cb..1fa4a97 100644
> --- a/hw/s390x/s390-ccw.h
> +++ b/hw/s390x/s390-ccw.h
> @@ -26,6 +26,7 @@
>  typedef struct S390CCWDevice {
>  CcwDevice parent_obj;
>  CssDevId hostid;
> +char *mdevid;
>  } S390CCWDevice;
>  
>  typedef struct S390CCWDeviceClass {
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index c25e32b..4c3a462 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -1,6 +1,7 @@
>  ifeq ($(CONFIG_LINUX), y)
>  obj-$(CONFIG_SOFTMMU) += common.o
>  obj-$(CONFIG_PCI) += pci.o pci-quirks.o
> +obj-$(CONFIG_VFIO_CCW) += ccw.o
>  obj-$(CONFIG_SOFTMMU) += platform.o
>  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
>  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> new file mode 100644
> index 000..881010b
> --- /dev/null
> +++ b/hw/vfio/ccw.c
> @@ -0,0 +1,209 @@
> +/*
> + * vfio based subchannel assignment support
> + *
> + * Copyright 2017 IBM Corp.
> + * Author(s): Dong Jia Shi 
> + *Xiao Feng Ren 
> + *Pierre Morel 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or(at
> + * your option) any version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include 
> +#include 
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/vfio/vfio.h"
> +#include "hw/vfio/vfio-common.h"
> +#include "hw/s390x/s390-ccw.h"
> +#include "hw/s390x/ccw-device.h"
> +
> +#define TYPE_VFIO_CCW "vfio-ccw"
> +typedef struct VFIOCCWDevice {
> +S390CCWDevice cdev;
> +VFIODevice vdev;
> +} VFIOCCWDevice;
> +
> +static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> +{
> +vdev->needs_reset = false;
> +}
> +
> +/*
> + * We don't need vfio_hot_reset_multi and vfio_eoi operationis for
> + * vfio_ccw device now.
> + */
> +struct VFIODeviceOps vfio_ccw_ops = {
> +.vfio_compute_needs_reset = vfio_ccw_compute_needs_reset,
> +};
> +
> +static void vfio_ccw_reset(DeviceState *dev)
> +{
> +CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
> +S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev);
> +VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
> +
> +ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
> +}
> +
> +static void vfio_put_device(VFIOCCWDevice *vcdev)
> +{
> +g_free(vcdev->vdev.name);
> +vfio_put_base_device(>vdev);
> +}
> +
> +static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, char **path,
> + Error **errp)
> +{
> +struct stat st;
> +int groupid;
> +GError *gerror = NULL;
> +
> +/* Check that host subchannel exists. */
> +path[0] = g_strdup_printf("/sys/bus/css/devices/%x.%x.%04x",
> +  cdev->hostid.cssid,
> +  cdev->hostid.ssid,
> +  cdev->hostid.devid);
> +if (stat(path[0], ) < 0) {
> +error_setg(errp, "vfio: no such host subchannel %s", path[0]);
> +return NULL;
> +}
> +
> +/* Check that mediated device exists. */
> +path[1] = g_strdup_printf("%s/%s", path[0], cdev->mdevid);
> +if (stat(path[0], ) < 0) {
> +error_setg(errp, "vfio: no such mediated device %s", path[1]);
> +return NULL;
> +

Re: [Qemu-devel] [PATCH v6] qqq: module for synchronizing with a simulation

[Nutaro, James J.]

Paolo,

There are several switches that need to be set for this module to work, and so 
- inspired by your comment - I decided to add a check for the necessary 
supporting switches before enabling the module. An error message is printed if 
everything is not as it should. I submitted the patch as v7.

Thanks for taking the time to review this.

Jim

-Original Message-
From: Paolo Bonzini [mailto:pbonz...@redhat.com] 
Sent: Monday, January 09, 2017 12:10 PM
To: Nutaro, James J.; qemu-devel@nongnu.org
Subject: Re: [PATCH v6] qqq: module for synchronizing with a simulation



On 09/01/2017 18:04, Nutaro, James J. wrote:
> Thanks again Paolo. When I change the command line switches, is it best to 
> submit a whole new version of the patch? Or is there another method for 
> managing patch revisions?

Yes, submit a new version, then put

v6->v7: did this and that [Paolo]

at the end of the commit message, where the square brackets mean
"suggested by...".

Thanks,

Paolo

> 
> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com] 
> Sent: Thursday, January 05, 2017 10:27 AM
> To: Nutaro, James J.; qemu-devel@nongnu.org
> Subject: Re: [PATCH v6] qqq: module for synchronizing with a simulation
> 
> 
> 
> On 05/01/2017 16:07, James J. Nutaro wrote:
>> This patch adds an interface for pacing the execution of QEMU to match an 
>> external
>> simulation clock. Its aim is to permit QEMU to be used as a module within a
>> larger simulation system.
>>
>> Signed-off-by: James J. Nutaro 
> 
> What's the difference between v5 and v6?
> 
>> +(2) Fork QEMU with the appropriate command line arguments.
>> +The -qqq part of the argument will look something like
>> +
>> +   -qqq sock=socks[1]
> 
> No need for -qqq, just make it "-icount extclock=FD".
> 
> Paolo
> 

[Qemu-devel] [PATCH v7] qqq: module for synchronizing with a simulation

[James J. Nutaro]

This patch adds an interface for pacing the execution of QEMU to match an 
external
simulation clock. Its aim is to permit QEMU to be used as a module within a
larger simulation system.

v6 -> v7 Added a check for command line arguments that are consistent
with -qqq and print an error message if incompatible arguments are
provide. [Paolo]

Signed-off-by: James J. Nutaro 
---
 Makefile.target  |   3 +
 cpus.c   |   8 +++
 docs/simulation-sync.txt |  59 
 include/sysemu/cpus.h|   1 +
 kvm-all.c|  10 +++
 qemu-options.hx  |  16 +
 qqq.c| 177 +++
 qqq.h|  37 ++
 vl.c |  36 ++
 9 files changed, 347 insertions(+)
 create mode 100644 docs/simulation-sync.txt
 create mode 100644 qqq.c
 create mode 100644 qqq.h

diff --git a/Makefile.target b/Makefile.target
index 8ae82cb..0a08fd3 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -145,6 +145,9 @@ obj-y += dump.o
 obj-y += migration/ram.o migration/savevm.o
 LIBS := $(libs_softmmu) $(LIBS)
 
+# qqq support
+obj-y += qqq.o
+
 # xen support
 obj-$(CONFIG_XEN) += xen-common.o
 obj-$(CONFIG_XEN_I386) += xen-hvm.o xen-mapcache.o
diff --git a/cpus.c b/cpus.c
index 5213351..8a98d7f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1688,3 +1688,11 @@ void dump_drift_info(FILE *f, fprintf_function 
cpu_fprintf)
 cpu_fprintf(f, "Max guest advance   NA\n");
 }
 }
+
+void kick_all_vcpus(void)
+{
+CPUState *cpu;
+CPU_FOREACH(cpu) {
+qemu_cpu_kick(cpu);
+}
+}
diff --git a/docs/simulation-sync.txt b/docs/simulation-sync.txt
new file mode 100644
index 000..de4dd34
--- /dev/null
+++ b/docs/simulation-sync.txt
@@ -0,0 +1,59 @@
+= Synchronizing the virtual clock with an external source =
+
+QEMU has a protocol for synchronizing its virtual clock
+with the clock of a simulator in which QEMU is embedded
+as a component. This options is enabled with the -qqq
+argument, and it should generally be accompanied by the
+following additional command line arguments:
+
+-icount 1,sleep=off -rtc clock=vm
+  or
+-enable-kvm -rtc clock=vm
+
+The -qqq argument is used to supply a file descriptor
+for a Unix socket, which is used for synchronization.
+The procedure for launching QEMU in is synchronization
+mode has three steps:
+
+(1) Create a socket pair with the Linux socketpair function.
+The code segment that does this might look like
+
+   int socks[2];
+   socketpair(AF_UNIX,SOCK_STREAM,0,socks);
+
+(2) Fork QEMU with the appropriate command line arguments.
+The -qqq part of the argument will look something like
+
+   -qqq sock=socks[1]
+
+(3) After forking QEMU, close sock[1] and retain the
+sock[0] for communicating with QEMU.
+
+The synchronization protocol is very simple. To start, the
+external simulator writes an integer to its socket with
+the amount of time in microseconds that QEMU is allowed to
+advance. The code segment that does this might look like:
+
+uint32_t ta = htonl(1000); // Advance by 1 millisecond
+write(sock[0],,sizeof(uint32_t));
+
+The external simulator can then advance its clock by this
+same amount. During this time, QEMU and the external simulator
+will be executing in parallel. When the external simulator
+completes its time advance, it waits for QEMU by reading from
+its socket. The value read will be the actual number of
+virtual microseconds by which QEMU has advanced its virtual clock.
+This will be greater than or equal to the requested advance.
+The code that does this might look like:
+
+   uint32_t ta;
+   read(fd,,sizeof(uint32_t));
+   ta = ntohl(ta);
+
+These steps are repeated until either (1) the external simulator
+closes its socket thereby causing QEMU to terminate or (2) QEMU
+stops executing (e.g., if the emulated computer is shutdown) and
+causes a read or write error on the simulator's socket.
+
+You can find an example of a simulator using this protocol in
+the adevs simulation package at http://sourceforge.net/projects/adevs/
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 3728a1e..bdf22c9 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -2,6 +2,7 @@
 #define QEMU_CPUS_H
 
 /* cpus.c */
+void kick_all_vcpus(void);
 bool qemu_in_vcpu_thread(void);
 void qemu_init_cpu_loop(void);
 void resume_all_vcpus(void);
diff --git a/kvm-all.c b/kvm-all.c
index 330219e..b66b07f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -18,6 +18,7 @@
 
 #include 
 
+#include "qqq.h"
 #include "qemu-common.h"
 #include "qemu/atomic.h"
 #include "qemu/option.h"
@@ -1926,6 +1927,15 @@ int kvm_cpu_exec(CPUState *cpu)
 qemu_cpu_kick_self();
 }
 
+if (qqq_enabled()) {
+/* Pause here while qqq is synchronizing with a simulation clock.
+ * We do not want to execute instructions past the synchronization
+ * deadline, but 

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/18] arm: Add virtualization to GICv3, and enable EL2 on 64-bit CPUs

[Alistair Francis]

On Tue, Jan 17, 2017 at 6:49 AM, Andrew Jones  wrote:
> On Tue, Jan 17, 2017 at 02:07:31PM +, Peter Maydell wrote:
>> On 9 January 2017 at 16:05, Peter Maydell  wrote:
>> > This patchset adds support for the Virtualization extensions to QEMU's
>> > GICv3 emulation. This was the last missing piece that was stopping
>> > us from turning on the EL2 support in the CPU model, so the patchset
>> > also adds support for enabling it all on the virt board via the
>> > '-machine virtualization=on' option.
>>
>> > Patches 1, 4-13, 15, 16 still need review.
>>
>> So is anybody interested in reviewing the complicated
>> bits in the middle of this patchset (5-13) or shall I
>> just throw it into target-arm.next? :-)

I have Acked and Reviewed what I can. I don't know much about GICv3 so
I can't really judge those patches. Overall they look good to me
though.

Thanks,

Alistair

>
> Unfortunately I don't have time right now to give them the justice
> they deserve. FWIW, I've done light testing and light reviewing and
> I don't have any complaints.
>
> Thanks,
> drew
>
>>
>> thanks
>> -- PMM
>>
>

Re: [Qemu-devel] [PATCH 23/23] hw/arm/virt: Add board property to enable EL2

[Alistair Francis]

On Wed, Dec 28, 2016 at 5:14 AM, Andrew Jones  wrote:
> On Tue, Dec 13, 2016 at 10:36:24AM +, Peter Maydell wrote:
>> Add a board level property to the virt board which will
>> enable EL2 on the CPU if the user asks for it. The
>> default is not to provide EL2. If EL2 is enabled then
>> we will use SMC as our PSCI conduit, and report the
>> virtualization support in the GICv3 device tree node.
>>
>> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Thanks,

Alistair

>> ---
>>  hw/arm/virt.c | 45 +++--
>>  1 file changed, 43 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: Andrew Jones 
>

Re: [Qemu-devel] [PATCH v2 15/18] hw/arm/virt-acpi-build: use SMC if booting in EL2

[Alistair Francis]

On Mon, Jan 9, 2017 at 8:05 AM, Peter Maydell  wrote:
> From: Andrew Jones 
>
> Signed-off-by: Andrew Jones 
> [PMM: look at vms->psci_conduit rather than vms->virt
>  to decide whether to use HVC or SMC, and report no
>  PSCI support at all for the 'PSCI disabled' case]
> Signed-off-by: Peter Maydell 

Acked-by: Alistair Francis 

Thanks,

Alistair

> ---
>  hw/arm/virt-acpi-build.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 085a611..ec7f83b 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -643,16 +643,30 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  }
>
>  /* FADT */
> -static void
> -build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt_tbl_offset)
> +static void build_fadt(GArray *table_data, BIOSLinker *linker,
> +   VirtMachineState *vms, unsigned dsdt_tbl_offset)
>  {
>  AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, 
> sizeof(*fadt));
>  unsigned dsdt_entry_offset = (char *)>dsdt - table_data->data;
> +uint16_t bootflags;
> +
> +switch (vms->psci_conduit) {
> +case QEMU_PSCI_CONDUIT_DISABLED:
> +bootflags = 0;
> +break;
> +case QEMU_PSCI_CONDUIT_HVC:
> +bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT | 
> ACPI_FADT_ARM_PSCI_USE_HVC;
> +break;
> +case QEMU_PSCI_CONDUIT_SMC:
> +bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT;
> +break;
> +default:
> +g_assert_not_reached();
> +}
>
> -/* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
> +/* Hardware Reduced = 1 and use PSCI 0.2+ */
>  fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI);
> -fadt->arm_boot_flags = cpu_to_le16(ACPI_FADT_ARM_PSCI_COMPLIANT |
> -   ACPI_FADT_ARM_PSCI_USE_HVC);
> +fadt->arm_boot_flags = cpu_to_le16(bootflags);
>
>  /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
>  fadt->minor_revision = 0x1;
> @@ -738,7 +752,7 @@ void virt_acpi_build(VirtMachineState *vms, 
> AcpiBuildTables *tables)
>
>  /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
>  acpi_add_table(table_offsets, tables_blob);
> -build_fadt(tables_blob, tables->linker, dsdt);
> +build_fadt(tables_blob, tables->linker, vms, dsdt);
>
>  acpi_add_table(table_offsets, tables_blob);
>  build_madt(tables_blob, tables->linker, vms);
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v2 07/18] hw/intc/gicv3: Add data fields for virtualization support

[Alistair Francis]

On Mon, Jan 9, 2017 at 8:05 AM, Peter Maydell  wrote:
> As the first step in adding support for the virtualization
> extensions to the GICv3 emulation:
>  * add the necessary data fields to the state structures
>  * add the fields to the migration state, as a subsection
>which is only present if virtualization is enabled
>
> The use of a subsection means we retain migration
> compatibility as EL2 is not enabled on any CPUs currently.
>
> Signed-off-by: Peter Maydell 

Acked-by: Alistair Francis 

Thanks,

Alistair

> ---
>  include/hw/intc/arm_gicv3_common.h | 18 ++
>  hw/intc/arm_gicv3_common.c | 25 +
>  hw/intc/arm_gicv3_cpuif.c  | 13 +
>  3 files changed, 56 insertions(+)
>
> diff --git a/include/hw/intc/arm_gicv3_common.h 
> b/include/hw/intc/arm_gicv3_common.h
> index beb2c77..665d3f8 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -38,6 +38,9 @@
>  /* Number of SGI target-list bits */
>  #define GICV3_TARGETLIST_BITS 16
>
> +/* Maximum number of list registers (architectural limit) */
> +#define GICV3_LR_MAX 16
> +
>  /* Minimum BPR for Secure, or when security not enabled */
>  #define GIC_MIN_BPR 0
>  /* Minimum BPR for Nonsecure when security is enabled */
> @@ -175,6 +178,21 @@ struct GICv3CPUState {
>  uint64_t icc_igrpen[3];
>  uint64_t icc_ctlr_el3;
>
> +/* Virtualization control interface */
> +uint64_t ich_apr[3][4]; /* ich_apr[GICV3_G1][x] never used */
> +uint64_t ich_hcr_el2;
> +uint64_t ich_lr_el2[GICV3_LR_MAX];
> +uint64_t ich_vmcr_el2;
> +
> +/* Properties of the CPU interface. These are initialized from
> + * the settings in the CPU proper.
> + * If the number of implemented list registers is 0 then the
> + * virtualization support is not implemented.
> + */
> +int num_list_regs;
> +int vpribits; /* number of virtual priority bits */
> +int vprebits; /* number of virtual preemption bits */
> +
>  /* Current highest priority pending interrupt for this CPU.
>   * This is cached information that can be recalculated from the
>   * real state above; it doesn't need to be migrated.
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 0ee67a4..16b9b0f 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -49,6 +49,27 @@ static int gicv3_post_load(void *opaque, int version_id)
>  return 0;
>  }
>
> +static bool virt_state_needed(void *opaque)
> +{
> +GICv3CPUState *cs = opaque;
> +
> +return cs->num_list_regs != 0;
> +}
> +
> +static const VMStateDescription vmstate_gicv3_cpu_virt = {
> +.name = "arm_gicv3_cpu/virt",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = virt_state_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT64_2DARRAY(ich_apr, GICv3CPUState, 3, 4),
> +VMSTATE_UINT64(ich_hcr_el2, GICv3CPUState),
> +VMSTATE_UINT64_ARRAY(ich_lr_el2, GICv3CPUState, GICV3_LR_MAX),
> +VMSTATE_UINT64(ich_vmcr_el2, GICv3CPUState),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
>  static const VMStateDescription vmstate_gicv3_cpu = {
>  .name = "arm_gicv3_cpu",
>  .version_id = 1,
> @@ -75,6 +96,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>  VMSTATE_UINT64_ARRAY(icc_igrpen, GICv3CPUState, 3),
>  VMSTATE_UINT64(icc_ctlr_el3, GICv3CPUState),
>  VMSTATE_END_OF_LIST()
> +},
> +.subsections = (const VMStateDescription * []) {
> +_gicv3_cpu_virt,
> +NULL
>  }
>  };
>
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index 35e8eb3..d2f859c 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -36,6 +36,12 @@ static bool gicv3_use_ns_bank(CPUARMState *env)
>  return !arm_is_secure_below_el3(env);
>  }
>
> +/* The minimum BPR for the virtual interface is a configurable property */
> +static inline int icv_min_vbpr(GICv3CPUState *cs)
> +{
> +return 7 - cs->vprebits;
> +}
> +
>  static int icc_highest_active_prio(GICv3CPUState *cs)
>  {
>  /* Calculate the current running priority based on the set bits
> @@ -1081,6 +1087,13 @@ static void icc_reset(CPUARMState *env, const 
> ARMCPRegInfo *ri)
>  cs->icc_ctlr_el3 = ICC_CTLR_EL3_NDS | ICC_CTLR_EL3_A3V |
>  (1 << ICC_CTLR_EL3_IDBITS_SHIFT) |
>  (7 << ICC_CTLR_EL3_PRIBITS_SHIFT);
> +
> +memset(cs->ich_apr, 0, sizeof(cs->ich_apr));
> +cs->ich_hcr_el2 = 0;
> +memset(cs->ich_lr_el2, 0, sizeof(cs->ich_lr_el2));
> +cs->ich_vmcr_el2 = ICH_VMCR_EL2_VFIQEN |
> +(icv_min_vbpr(cs) << ICH_VMCR_EL2_VBPR1_SHIFT) |
> +(icv_min_vbpr(cs) << ICH_VMCR_EL2_VBPR0_SHIFT);
>  }
>
>  static const ARMCPRegInfo gicv3_cpuif_reginfo[] = {
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v2 05/18] target-arm: Add ARMCPU fields for GIC CPU i/f config

[Alistair Francis]

On Mon, Jan 9, 2017 at 8:05 AM, Peter Maydell  wrote:
> Add fields to the ARMCPU structure to allow CPU classes to
> specify the configurable aspects of their GIC CPU interface.
> In particular, the virtualization support allows different
> values for number of list registers, priority bits and
> preemption bits.
>
> Signed-off-by: Peter Maydell 

Acked-by: Alistair Francis 

Thanks,

Alistair

> ---
>  target/arm/cpu.h   | 5 +
>  target/arm/cpu64.c | 6 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 764b511..28c5d8f 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -659,6 +659,11 @@ struct ARMCPU {
>  uint32_t dcz_blocksize;
>  uint64_t rvbar;
>
> +/* Configurable aspects of GIC cpu interface (which is part of the CPU) 
> */
> +int gic_num_lrs; /* number of list registers */
> +int gic_vpribits; /* number of virtual priority bits */
> +int gic_vprebits; /* number of virtual preemption bits */
> +
>  ARMELChangeHook *el_change_hook;
>  void *el_change_hook_opaque;
>  };
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 549cb1e..73c7f31 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -147,6 +147,9 @@ static void aarch64_a57_initfn(Object *obj)
>  cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
>  cpu->ccsidr[2] = 0x70ffe07a; /* 2048KB L2 cache */
>  cpu->dcz_blocksize = 4; /* 64 bytes */
> +cpu->gic_num_lrs = 4;
> +cpu->gic_vpribits = 5;
> +cpu->gic_vprebits = 5;
>  define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
>  }
>
> @@ -201,6 +204,9 @@ static void aarch64_a53_initfn(Object *obj)
>  cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */
>  cpu->ccsidr[2] = 0x707fe07a; /* 1024KB L2 cache */
>  cpu->dcz_blocksize = 4; /* 64 bytes */
> +cpu->gic_num_lrs = 4;
> +cpu->gic_vpribits = 5;
> +cpu->gic_vprebits = 5;
>  define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo);
>  }
>
> --
> 2.7.4
>
>

[Qemu-devel] [PATCH] xhci: bump the link TRB cycle detection limit.

[anonym]

Hi!

[Please keep me Cc:ed as I am not subscribed to this list!]

It seems the fix of CVE-2016-8576 (commit 05f43d4) introduced a regression in 
QEMU 2.8.

Cheers!

Re: [Qemu-devel] [PATCH 07/18] tcg: add vector addition operations

[Richard Henderson]

On 01/17/2017 01:07 AM, Kirill Batuzov wrote:

+/***/
+/* 64-bit and 128-bit vector arithmetic.  */
+
+static inline void *tcg_v128_swap_slot(int n)
+{
+return _ctx.v128_swap[n * 16];
+}
+
+/* Find a memory location for 128-bit TCG variable. */
+static inline void tcg_v128_to_ptr(TCGv_v128 tmp, TCGv_ptr base, int slot,
+   TCGv_ptr *real_base, intptr_t *real_offset,
+   int is_read)


None of this needs to be inline in tcg-op.h.  All of it should be out-of-line 
in tcg-op.c.




@@ -750,6 +778,7 @@ struct TCGContext {
 void *code_gen_buffer;
 size_t code_gen_buffer_size;
 void *code_gen_ptr;
+uint8_t v128_swap[16 * 3];


This is not thread-safe.
Shouldn't use space in TCGContext; should use space on stack.

Since there is no function call that is live, you can re-use the space for 
on-stack arguments.  There is TCG_STATIC_CALL_ARGS_SIZE (128) bytes allocated 
for that.  Which should be more than enough.



r~

Re: [Qemu-devel] [PATCH RFC v2 02/12] vfio: linux-headers update for vfio-ccw

[Alex Williamson]

On Thu, 12 Jan 2017 08:25:03 +0100
Dong Jia Shi  wrote:

> From: Xiao Feng Ren 
> 
> This is a placeholder for a linux-headers update.
> 
> Signed-off-by: Xiao Feng Ren 
> ---
>  include/standard-headers/asm-s390/vfio_ccw.h | 28 
> 
>  linux-headers/linux/vfio.h   | 17 +
>  2 files changed, 45 insertions(+)
>  create mode 100644 include/standard-headers/asm-s390/vfio_ccw.h
> 
> diff --git a/include/standard-headers/asm-s390/vfio_ccw.h 
> b/include/standard-headers/asm-s390/vfio_ccw.h
> new file mode 100644
> index 000..cddc09b
> --- /dev/null
> +++ b/include/standard-headers/asm-s390/vfio_ccw.h
> @@ -0,0 +1,28 @@
> +/*
> + * Interfaces for vfio-ccw
> + *
> + * Copyright IBM Corp. 2017
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License (version 2 only)
> + * as published by the Free Software Foundation.
> + *
> + * Author(s): Dong Jia Shi 
> + */
> +
> +#ifndef _VFIO_CCW_H_
> +#define _VFIO_CCW_H_
> +
> +#include "standard-headers/linux/types.h"
> +
> +struct ccw_io_region {
> +#define ORB_AREA_SIZE 12
> + uint8_t  orb_area[ORB_AREA_SIZE];
> +#define SCSW_AREA_SIZE 12
> + uint8_t  scsw_area[SCSW_AREA_SIZE];
> +#define IRB_AREA_SIZE 96
> + uint8_t  irb_area[IRB_AREA_SIZE];
> + uint32_t ret_code;
> +} QEMU_PACKED;
> +
> +#endif

This is really part of the uapi for the vfio-ccw mdev device, isn't it?
Should it really be buried in asm-s390 in the kernel?

> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index 759b850..b09d247 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -198,6 +198,7 @@ struct vfio_device_info {
>  #define VFIO_DEVICE_FLAGS_PCI(1 << 1)/* vfio-pci device */
>  #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)  /* vfio-platform device */
>  #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3) /* vfio-amba device */
> +#define VFIO_DEVICE_FLAGS_CCW   (1 << 4) /* vfio-ccw device */
>   __u32   num_regions;/* Max region index + 1 */
>   __u32   num_irqs;   /* Max IRQ index + 1 */
>  };
> @@ -436,6 +437,22 @@ enum {
>   VFIO_PCI_NUM_IRQS
>  };
>  
> +/*
> + * The VFIO-CCW bus driver makes use of the following fixed region and
> + * IRQ index mapping.  Unimplemented regions return a size of zero.
> + * Unimplemented IRQ types return a count of zero.
> + */
> +
> +enum {
> +VFIO_CCW_CONFIG_REGION_INDEX,
> +VFIO_CCW_NUM_REGIONS
> +};
> +
> +enum {
> +VFIO_CCW_IO_IRQ_INDEX,
> +VFIO_CCW_NUM_IRQS
> +};
> +
>  /**
>   * VFIO_DEVICE_GET_PCI_HOT_RESET_INFO - _IORW(VFIO_TYPE, VFIO_BASE + 12,
>   * struct vfio_pci_hot_reset_info)

Re: [Qemu-devel] [PATCH v2 03/18] target-arm: Expose output GPIO line for VCPU maintenance interrupt

[Alistair Francis]

On Mon, Jan 9, 2017 at 8:05 AM, Peter Maydell  wrote:
> The GICv3 support for virtualization includes an outbound
> maintenance interrupt signal which is asserted when the
> CPU interface wants to signal to the hypervisor that it
> needs attention. Expose this as an outbound GPIO line from
> the CPU object which can be wired up as a physical interrupt
> line by the board code (as we do already for the CPU timers).
>
> Signed-off-by: Peter Maydell 
> Reviewed-by: Edgar E. Iglesias 

Reviewed-by: Alistair Francis 

Thanks,

Alistair

> ---
>  target/arm/cpu.h | 2 ++
>  target/arm/cpu.c | 3 +++
>  2 files changed, 5 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index ab119e6..764b511 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -555,6 +555,8 @@ struct ARMCPU {
>  QEMUTimer *gt_timer[NUM_GTIMERS];
>  /* GPIO outputs for generic timer */
>  qemu_irq gt_timer_outputs[NUM_GTIMERS];
> +/* GPIO output for GICv3 maintenance interrupt signal */
> +qemu_irq gicv3_maintenance_interrupt;
>
>  /* MemoryRegion to use for secure physical accesses */
>  MemoryRegion *secure_memory;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index f5cb30a..8932086 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -466,6 +466,9 @@ static void arm_cpu_initfn(Object *obj)
>  arm_gt_stimer_cb, cpu);
>  qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
> ARRAY_SIZE(cpu->gt_timer_outputs));
> +
> +qdev_init_gpio_out_named(DEVICE(cpu), >gicv3_maintenance_interrupt,
> + "gicv3-maintenance-interrupt", 1);
>  #endif
>
>  /* DTB consumers generally don't in fact care what the 'compatible'
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v2 01/18] hw/intc/arm_gicv3: Add external IRQ lines for VIRQ and VFIQ

[Alistair Francis]

On Mon, Jan 9, 2017 at 8:05 AM, Peter Maydell  wrote:
> Augment the GICv3's QOM device interface by adding two
> new sets of sysbus IRQ lines, to signal VIRQ and VFIQ to
> each CPU.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Thanks,

Alistair

> ---
>  include/hw/intc/arm_gicv3_common.h | 2 ++
>  hw/intc/arm_gicv3_common.c | 6 ++
>  2 files changed, 8 insertions(+)
>
> diff --git a/include/hw/intc/arm_gicv3_common.h 
> b/include/hw/intc/arm_gicv3_common.h
> index 341a311..beb2c77 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -145,6 +145,8 @@ struct GICv3CPUState {
>  CPUState *cpu;
>  qemu_irq parent_irq;
>  qemu_irq parent_fiq;
> +qemu_irq parent_virq;
> +qemu_irq parent_vfiq;
>
>  /* Redistributor */
>  uint32_t level;  /* Current IRQ level */
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 0aa9b9c..0ee67a4 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -126,6 +126,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, 
> qemu_irq_handler handler,
>  for (i = 0; i < s->num_cpu; i++) {
>  sysbus_init_irq(sbd, >cpu[i].parent_fiq);
>  }
> +for (i = 0; i < s->num_cpu; i++) {
> +sysbus_init_irq(sbd, >cpu[i].parent_virq);
> +}
> +for (i = 0; i < s->num_cpu; i++) {
> +sysbus_init_irq(sbd, >cpu[i].parent_vfiq);
> +}
>
>  memory_region_init_io(>iomem_dist, OBJECT(s), ops, s,
>"gicv3_dist", 0x1);
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH 12/18] tcg/i386: support remaining vector addition operations

[Richard Henderson]

On 01/17/2017 01:07 AM, Kirill Batuzov wrote:

 #ifdef TCG_TARGET_HAS_REG128
+case INDEX_op_add_i8x16:
+tcg_out_modrm(s, OPC_PADDB, args[0], args[2]);
+break;
+case INDEX_op_add_i16x8:
+tcg_out_modrm(s, OPC_PADDW, args[0], args[2]);
+break;
 case INDEX_op_add_i32x4:
 tcg_out_modrm(s, OPC_PADDD, args[0], args[2]);
 break;
+case INDEX_op_add_i64x2:
+tcg_out_modrm(s, OPC_PADDQ, args[0], args[2]);
+break;
+#endif
+
+#ifdef TCG_TARGET_HAS_REGV64
+case INDEX_op_add_i8x8:
+tcg_out_modrm(s, OPC_PADDB, args[0], args[2]);
+break;
+case INDEX_op_add_i16x4:
+tcg_out_modrm(s, OPC_PADDW, args[0], args[2]);
+break;
+case INDEX_op_add_i32x2:
+tcg_out_modrm(s, OPC_PADDD, args[0], args[2]);
+break;
+case INDEX_op_add_i64x1:
+tcg_out_modrm(s, OPC_PADDQ, args[0], args[2]);
+break;
 #endif


Once you drop the ifdefs, combine the cases.  Also: avx1 vpadd*.


r~

[Qemu-devel] [Bug 638806] Re: Crashes with kfreebsd guest when using KVM

[Thomas Huth]

Triaging old bug tickets ... can you still reproduce this problem with
the latest version of QEMU?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Crashes with kfreebsd guest when using KVM

Status in QEMU:
  Incomplete

Bug description:
  commit 46411f863c26ff85c48b97939502007610c95398

  Linux host
  Kfreebsd guest
  qemu -boot c -hda qemu_kfreebsd_i386.img -enable-kvm

  QEMU crashes with free on invalid pointer:

  *** glibc detected *** qemu: free(): invalid pointer: 0x0253cf60 ***
  === Backtrace: =
  /lib/libc.so.6(+0x71ad6)[0x7f0844fa5ad6]
  qemu[0x494283]
  qemu[0x4951ca]
  qemu[0x49aa01]
  qemu[0x495d15]
  qemu[0x5197f4]
  qemu[0x51a297]
  /lib/libc.so.6(__libc_start_main+0xfd)[0x7f0844f52c4d]
  qemu[0x408799]
  === Memory map: 
  0040-00625000 r-xp  08:06 4186599
/usr/local/bin/qemu
  00825000-00847000 rw-p 00225000 08:06 4186599
/usr/local/bin/qemu
  00847000-00fed000 rw-p  00:00 0 
  00fed000-00fee000 rwxp  00:00 0 
  00fee000-0104b000 rw-p  00:00 0 
  022fe000-023ff000 rw-p  00:00 0 
  023ff000-0240f000 rw-p  00:00 0 
  0240f000-0255d000 rw-p  00:00 0 
  41cd2000-43cd2000 rwxp  00:00 0 
  7f0835c22000-7f0835c38000 r-xp  08:06 3407959
/lib/libgcc_s.so.1
  7f0835c38000-7f0835e37000 ---p 00016000 08:06 3407959
/lib/libgcc_s.so.1
  7f0835e37000-7f0835e38000 rw-p 00015000 08:06 3407959
/lib/libgcc_s.so.1
  7f0835e38000-7f0835e3d000 r-xp  08:06 4185228
/usr/lib/libXfixes.so.3.1.0
  7f0835e3d000-7f083603c000 ---p 5000 08:06 4185228
/usr/lib/libXfixes.so.3.1.0
  7f083603c000-7f083603d000 rw-p 4000 08:06 4185228
/usr/lib/libXfixes.so.3.1.0
  7f083603d000-7f0836046000 r-xp  08:06 4178659
/usr/lib/libXcursor.so.1.0.2
  7f0836046000-7f0836246000 ---p 9000 08:06 4178659
/usr/lib/libXcursor.so.1.0.2
  7f0836246000-7f0836247000 rw-p 9000 08:06 4178659
/usr/lib/libXcursor.so.1.0.2
  7f0836247000-7f0836294000 rw-p  00:00 0 
  7f083631c000-7f0836491000 r--p  08:06 3670017
/usr/lib/locale/locale-archive
  7f0836491000-7f0836499000 r-xp  08:06 516333 
/usr/lib/libXrandr.so.2.2.0
  7f0836499000-7f0836698000 ---p 8000 08:06 516333 
/usr/lib/libXrandr.so.2.2.0
  7f0836698000-7f0836699000 rw-p 7000 08:06 516333 
/usr/lib/libXrandr.so.2.2.0
  7f0836699000-7f08366a2000 r-xp  08:06 4180666
/usr/lib/libXrender.so.1.3.0
  7f08366a2000-7f08368a2000 ---p 9000 08:06 4180666
/usr/lib/libXrender.so.1.3.0
  7f08368a2000-7f08368a3000 rw-p 9000 08:06 4180666
/usr/lib/libXrender.so.1.3.0
  7f08368a3000-7f08368b4000 r-xp  08:06 4181769
/usr/lib/libXext.so.6.4.0
  7f08368b4000-7f0836ab4000 ---p 00011000 08:06 4181769
/usr/lib/libXext.so.6.4.0
  7f0836ab4000-7f0836ab5000 rw-p 00011000 08:06 4181769
/usr/lib/libXext.so.6.4.0
  7f0836ad6000-7f0836ad7000 ---p  00:00 0 
  7f0836ad7000-7f0836f5b000 rw-p  00:00 0 
  7f0836f6e000-7f0837088000 rw-s  00:04 1900557
/SYSV (deleted)
  7f0837088000-7f0837089000 rw-p  00:00 0 
  7f0837089000-7f0837889000 rw-p  00:00 0 
  7f0837889000-7f083788b000 rw-p  00:00 0 
  7f083788b000-7f083f88b000 rw-p  00:00 0 
  7f083f88b000-7f083f88c000 rw-p  00:00 0 
  7f083f88c000-7f083f88d000 ---p  00:00 0 
  7f083f88d000-7f0841a8f000 rw-p  00:00 0 
  7f0841a8f000-7f0841a94000 r-xp  08:06 4183916
/usr/lib/libXdmcp.so.6.0.0
  7f0841a94000-7f0841c93000 ---p 5000 08:06 4183916
/usr/lib/libXdmcp.so.6.0.0
  7f0841c93000-7f0841c94000 rw-p 4000 08:06 4183916
/usr/lib/libXdmcp.so.6.0.0
  7f0841c94000-7f0841c96000 r-xp  08:06 4183879
/usr/lib/libXau.so.6.0.0
  7f0841c96000-7f0841e96000 ---p 2000 08:06 4183879
/usr/lib/libXau.so.6.0.0
  7f0841e96000-7f0841e97000 rw-p 2000 08:06 4183879
/usr/lib/libXau.so.6.0.0
  7f0841e97000-7f0841eb6000 r-xp  08:06 3407929
/lib/libx86.so.1
  7f0841eb6000-7f08420b6000 ---p 0001f000 08:06 3407929
/lib/libx86.so.1
  7f08420b6000-7f08420b8000 rw-p 0001f000 08:06 3407929
/lib/libx86.so.1
  7f08420b8000-7f08420b9000 rw-p  00:00 0 
  7f08420b9000-7f08420bc000 r-xp  08:06 

[Qemu-devel] [PATCH] xhci: bump the link TRB cycle detection limit.

[anonym]

From: anonym 

While formatting partitions (on virtual USB drives and the nec-xhci
virtual USB controller) to EXT4, I have observed errors like these:

kernel: sd 8:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_ABORT
driverbyte=DRIVER_OK
kernel: sd 8:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 66 49 86
00 08 00 00
kernel: blk_update_request: I/O error, dev sda, sector 6703494
kernel: Buffer I/O error on dev dm-0, logical block 1573254, lost
async page write

Raising TRB_LINK_LIMIT fixes the limit, but the new value was
admittedly arbitrarily chosen.

Regarding cycle detection in general, allowing at most 4 levels of
links seems pretty low. This bump should be safe: a high number only
means that we get a performance hit when encountering cycles but then
we should have a fatal error any way; a low number instead means that
we'll incorrectly identify cycles and abort operations that otherwise
would succeed, like in the case above.

Signed-off-by: anonym 
---
 hw/usb/hcd-xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6dd8..d14ce126a2 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -53,7 +53,7 @@
  * to the specs when it gets them */
 #define ER_FULL_HACK
 
-#define TRB_LINK_LIMIT  4
+#define TRB_LINK_LIMIT  32
 
 #define LEN_CAP 0x40
 #define LEN_OPER(0x400 + 0x10 * MAXPORTS)
-- 
2.11.0

[Qemu-devel] [Bug 747583] Re: Windows 2008 Time Zone Change Even When Using -locatime

[Thomas Huth]

QEMU 0.12 is completely outdated nowadays ... can you still reproduce
this issue with the latest version of QEMU?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Windows 2008 Time Zone Change Even When Using -locatime

Status in QEMU:
  Incomplete

Bug description:
  * What cpu model : Intel(R) Xeon(R) CPU E5620  @ 2.40GHz
  * What kvm version you are using. : qemu-kvm-0.12.3
  * The host kernel version : 2.6.32-30-server
  * What host kernel arch you are using (i386 or x86_64) : x86_64
  * What guest you are using, including OS type: Windows 2008 Enterprise x86_64
  * The qemu command line you are using to start the guest : /usr/bin/kvm -S -M 
pc-0.12 -enable-kvm -m 1024 -smp 1 -name 2-6176 -uuid 
4d1d56b1-d0b7-506b-31a5-a87c8cb0560b -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/2-6176.monitor,server,nowait 
-monitor chardev:monitor -localtime -boot c -drive 
file=/dev/disk/by-id/scsi-3600144f05c1109004d9602950073,if=virtio,index=0,boot=on,format=raw
 -drive 
file=/dev/disk/by-id/scsi-3600144f0eae881004c7bb0920037,if=ide,media=cdrom,index=2,format=raw
 -net nic,macaddr=00:00:d1:d0:3f:5e,vlan=0,name=nic.1 -net 
tap,fd=212,vlan=0,name=tap.1 -net 
nic,macaddr=00:00:0a:d0:3f:5e,vlan=1,name=nic.1 -net 
tap,fd=213,vlan=1,name=tap.1 -chardev pty,id=serial0 -serial chardev:serial0 
-parallel none -usb -usbdevice tablet -vnc 0.0.0.0:394,password -k en-us -vga 
cirrus
  * Whether the problem goes away if using the -no-kvm-irqchip or -no-kvm-pit 
switch. : Unable to test
  * Whether the problem also appears with the -no-kvm switch. : Unable to test

  Host time zone: EDT Guest time zone: PDT

  Steps to reproduce:
  1) Set time zone to (GMT-08:00) Pacific Time (US & Canada) on guest
  2) Power off Windows 2008 Enterprise x86_64 guest completely. Ensure the kvm 
process exits.
  3) Power on Windows 2008 Enterprise x86_64 guest using virsh start 
  4) Server will show EDT time but have the time zone still set to (GMT-08:00) 
Pacific Time (US & Canada).

  Syncing the time after stopping and starting the kvm process using
  Windows "Internet Time" ntp time sync will sync the time to the
  correct PDT time.

  Doing a reboot from within the guest's operating system where kvm does
  not exit will not cause the timezone shift to happen.

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

[Qemu-devel] [Bug 741887] Re: virsh snapshot-create too slow (kvm, qcow2, savevm)

[Thomas Huth]

** Changed in: qemu
   Status: New => Won't Fix

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

Title:
  virsh snapshot-create too slow (kvm, qcow2, savevm)

Status in QEMU:
  Won't Fix
Status in qemu-kvm package in Ubuntu:
  Won't Fix

Bug description:
  Action
  ==
  # time virsh snapshot-create 1

  * Taking snapshot of a running KVM virtual machine

  Result
  ==
  Domain snapshot 1300983161 created
  real4m46.994s
  user0m0.000s
  sys 0m0.010s

  Expected result
  ===
  * Snapshot taken after few seconds instead of minutes.

  Environment
  ===
  * Ubuntu Natty Narwhal upgraded from Lucid and Meerkat, fully updated.

  * Stock natty packages of libvirt and qemu installed (libvirt-bin
  0.8.8-1ubuntu5; libvirt0 0.8.8-1ubuntu5; qemu-common 0.14.0+noroms-
  0ubuntu3; qemu-kvm 0.14.0+noroms-0ubuntu3).

  * Virtual machine disk format is qcow2 (debian 5 installed)
  image: /storage/debian.qcow2
  file format: qcow2
  virtual size: 10G (10737418240 bytes)
  disk size: 1.2G
  cluster_size: 65536
  Snapshot list:
  IDTAG VM SIZEDATE   VM CLOCK
  1 snap01  48M 2011-03-24 09:46:33   00:00:58.899
  2 1300979368  58M 2011-03-24 11:09:28   00:01:03.589
  3 1300983161  57M 2011-03-24 12:12:41   00:00:51.905

  * qcow2 disk is stored on ext4 filesystem, without RAID or LVM or any
  special setup.

  * running guest VM takes about 40M RAM from inside, from outside 576M
  are given to that machine

  * host has fast dual-core pentium cpu with virtualization support,
  around 8G of RAM and 7200rpm harddrive (dd from urandom to file gives
  about 20M/s)

  * running processes: sshd, atd (empty), crond (empty), libvirtd, tmux,
  bash, rsyslogd, upstart-socket-bridge, udevd, dnsmasq, iotop (python)

  * networking is done by bridging and bonding

  
  Detail description
  ==

  * Under root, command 'virsh create-snapshot 1' is issued on booted
  and running KVM machine with debian inside.

  * After about four minutes, the process is done.

  * 'iotop' shows two 'kvm' processes reading/writing to disk. First one
  has IO around 1500 K/s, second one has around 400 K/s. That takes
  about three minutes. Then first process grabs about 3 M/s of IO and
  suddenly dissapears (1-2 sec). Then second process does about 7.5 M/s
  of IO for around a 1-2 minutes.

  * Snapshot is successfuly created and is usable for reverting or
  extracting.

  * Pretty much the same behaviour occurs when command 'savevm' is
  issued directly from qemu monitor, without using libvirt at all
  (actually, virsh snapshot-create just calls 'savevm' to the monitor
  socket).

  * This behaviour was observed on lucid, meerkat, natty and even with
  git version of libvirt (f44bfb7fb978c9313ce050a1c4149bf04aa0a670).
  Also slowsave packages from
  https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/524447 gave
  this issue.

  
  Thank you for helping to solve this issue!

  ProblemType: Bug
  DistroRelease: Ubuntu 11.04
  Package: libvirt-bin 0.8.8-1ubuntu5
  ProcVersionSignature: Ubuntu 2.6.38-7.38-server 2.6.38
  Uname: Linux 2.6.38-7-server x86_64
  Architecture: amd64
  Date: Thu Mar 24 12:19:41 2011
  InstallationMedia: Ubuntu-Server 10.04.2 LTS "Lucid Lynx" - Release amd64 
(20110211.1)
  ProcEnviron:
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: libvirt
  UpgradeStatus: No upgrade log present (probably fresh install)

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

[Qemu-devel] [Bug 784977] Re: qemu-img convert fails to convert, generates a 512byte file output

[Thomas Huth]

Can you still reproduce this problem with the latest version of QEMU?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  qemu-img convert fails to convert, generates a 512byte file output

Status in QEMU:
  Incomplete

Bug description:
  I have a Vmware image, so I have files like 'Ubuntu.vmdk', want to
  convert to VirtualBox .vdi format using qemu, the first stage of
  extracting the image with 'qemu-img convert Ubuntu.vmdk output.bin'
  just generates a 512byte file:

  {quote}
  # Disk DescriptorFile
  version=1
  CID=36be9761
  parentCID=
  createType="twoGbMaxExtentSparse"

  # Extent description
  RW 4192256 SPARSE "Ubuntu-s001.vmdk"
  RW 4192256 SPARSE "Ubuntu-s002.vmdk"
  RW 4192256 SPARSE "Ubuntu-s003.vmdk"
  RW 4192256 SPARSE "Ubuntu-s004.vmdk"
  RW 4192256 SPARSE "Ubuntu-s005.vmdk"
  RW 4192256 SPARSE "Ubuntu-s006.vmdk"
  RW 4192256 SPARSE "Ubuntu-s007.vmdk"
  RW 4192256 SPARSE "Ubuntu-s008.vmdk"
  RW 4192256 SPARSE "Ubuntu-s009.vmdk"
  RW 4192256 SPARSE "Ubuntu-s010.vmdk"
  RW 20480 SPARSE "Ubunt
  {quote}

  Here is the input Ubuntu.vmdk file:
  {quote}
  # Disk DescriptorFile
  version=1
  CID=36be9761
  parentCID=
  createType="twoGbMaxExtentSparse"

  # Extent description
  RW 4192256 SPARSE "Ubuntu-s001.vmdk"
  RW 4192256 SPARSE "Ubuntu-s002.vmdk"
  RW 4192256 SPARSE "Ubuntu-s003.vmdk"
  RW 4192256 SPARSE "Ubuntu-s004.vmdk"
  RW 4192256 SPARSE "Ubuntu-s005.vmdk"
  RW 4192256 SPARSE "Ubuntu-s006.vmdk"
  RW 4192256 SPARSE "Ubuntu-s007.vmdk"
  RW 4192256 SPARSE "Ubuntu-s008.vmdk"
  RW 4192256 SPARSE "Ubuntu-s009.vmdk"
  RW 4192256 SPARSE "Ubuntu-s010.vmdk"
  RW 20480 SPARSE "Ubuntu-s011.vmdk"

  # The Disk Data Base 
  #DDB

  ddb.toolsVersion = "7240"
  ddb.adapterType = "lsilogic"
  ddb.geometry.sectors = "63"
  ddb.geometry.heads = "255"
  ddb.geometry.cylinders = "2610"
  ddb.virtualHWVersion = "6"
  {quote}

  No stack trace or other output was found.  Anything I can add (other
  than the 20G VM image to reproduce and I'll be happy to provide)

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

Re: [Qemu-devel] [PATCH RFC v2 11/15] vfio: ccw: introduce ioctls to get/set VFIO_CCW_IO_IRQ

[Alex Williamson]

On Thu, 12 Jan 2017 08:19:43 +0100
Dong Jia Shi  wrote:

> Realize VFIO_DEVICE_GET_IRQ_INFO ioctl to retrieve
> VFIO_CCW_IO_IRQ information.
> 
> Realize VFIO_DEVICE_SET_IRQS ioctl to set an eventfd fd for
> VFIO_CCW_IO_IRQ. Once a write operation to the ccw_io_region
> was performed, trigger a signal on this fd.
> 
> Signed-off-by: Dong Jia Shi 
> Reviewed-by: Pierre Morel 
> ---
>  drivers/s390/cio/vfio_ccw_ops.c | 125 
> +++-
>  drivers/s390/cio/vfio_ccw_private.h |   4 ++
>  include/uapi/linux/vfio.h   |  10 ++-
>  3 files changed, 136 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index b702735..3c47eb6 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -203,6 +203,9 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device 
> *mdev,
>   if (region->ret_code != 0)
>   return region->ret_code;
>  
> + if (private->io_trigger)
> + eventfd_signal(private->io_trigger, 1);
> +
>   return count;
>  }
>  
> @@ -211,7 +214,7 @@ static int vfio_ccw_mdev_get_device_info(struct 
> mdev_device *mdev,
>  {
>   info->flags = VFIO_DEVICE_FLAGS_CCW | VFIO_DEVICE_FLAGS_RESET;
>   info->num_regions = VFIO_CCW_NUM_REGIONS;
> - info->num_irqs = 0;
> + info->num_irqs = VFIO_CCW_NUM_IRQS;
>  
>   return 0;
>  }
> @@ -233,6 +236,84 @@ static int vfio_ccw_mdev_get_region_info(struct 
> mdev_device *mdev,
>   }
>  }
>  
> +int vfio_ccw_mdev_get_irq_info(struct mdev_device *mdev,
> +struct vfio_irq_info *info)
> +{
> + if (info->index != VFIO_CCW_IO_IRQ_INDEX)
> + return -EINVAL;
> +
> + info->count = VFIO_CCW_NUM_IRQS;
> + info->flags = VFIO_IRQ_INFO_EVENTFD | VFIO_IRQ_INFO_NORESIZE;


VFIO_CCW_NUM_IRQS is not being used correctly here, info->count is the
number of interrupts within this index, VFIO_CCW_NUM_IRQS is the number
of indexes.  This is meant to handle things like PCI where we have 3
different interrupts types (INTx, MSI, MSI-X) and some of those (MSI/X)
support multiple vectors.  In this case I think you want info->count =
1 and you don't need the NORESIZE flag since that only makes sense for
describing indexes where a subset of the available vectors may be
enabled.  So info->count comes out to the same thing, but should not
use the same macro to get there.

> +
> + return 0;
> +}
> +
> +static int vfio_ccw_mdev_set_irqs(struct mdev_device *mdev,
> +   uint32_t flags,
> +   void __user *data)
> +{
> + struct vfio_ccw_private *private;
> + struct eventfd_ctx **ctx;
> +
> + if (!(flags & VFIO_IRQ_SET_ACTION_TRIGGER))
> + return -EINVAL;
> +
> + private = dev_get_drvdata(mdev->dev.parent);
> + if (!private)
> + return -ENODEV;
> +
> + ctx = >io_trigger;
> +
> + switch (flags & VFIO_IRQ_SET_DATA_TYPE_MASK) {
> + case VFIO_IRQ_SET_DATA_NONE:
> + {
> + if (*ctx)
> + eventfd_signal(*ctx, 1);
> + return 0;
> + }
> + case VFIO_IRQ_SET_DATA_BOOL:
> + {
> + uint8_t trigger;
> +
> + if (get_user(trigger, (uint8_t __user *)data))
> + return -EFAULT;
> +
> + if (trigger && *ctx)
> + eventfd_signal(*ctx, 1);
> + return 0;
> + }
> + case VFIO_IRQ_SET_DATA_EVENTFD:
> + {
> + int32_t fd;
> +
> + if (get_user(fd, (int32_t __user *)data))
> + return -EFAULT;
> +
> + if (fd == -1) {
> + if (*ctx)
> + eventfd_ctx_put(*ctx);
> + *ctx = NULL;
> + } else if (fd >= 0) {
> + struct eventfd_ctx *efdctx;
> +
> + efdctx = eventfd_ctx_fdget(fd);
> + if (IS_ERR(efdctx))
> + return PTR_ERR(efdctx);
> +
> + if (*ctx)
> + eventfd_ctx_put(*ctx);
> +
> + *ctx = efdctx;
> + } else
> + return -EINVAL;
> +
> + return 0;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
>  static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device *mdev,
>  unsigned int cmd,
>  unsigned long arg)
> @@ -281,6 +362,48 @@ static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device 
> *mdev,
>  
>   return copy_to_user((void __user *)arg, , minsz);
>   }
> + case VFIO_DEVICE_GET_IRQ_INFO:
> + {
> + struct vfio_irq_info info;
> +
> + minsz = offsetofend(struct vfio_irq_info, count);
> +
> + if 

Re: [Qemu-devel] [PATCH RFC v2 06/15] vfio: ccw: register vfio_ccw to the mediated device framework

[Alex Williamson]

On Thu, 12 Jan 2017 08:19:38 +0100
Dong Jia Shi  wrote:

> To make vfio support subchannel devices, we need to leverage the
> mediated device framework to create a mediated device for the
> subchannel device.
> 
> This registers the subchannel device to the mediated device
> framework during probe to enable mediated device creation.
> 
> Signed-off-by: Dong Jia Shi 
> Reviewed-by: Pierre Morel 
> ---
>  arch/s390/Kconfig   |   2 +-
>  drivers/s390/cio/Makefile   |   2 +-
>  drivers/s390/cio/vfio_ccw_drv.c |  10 ++-
>  drivers/s390/cio/vfio_ccw_ops.c | 149 
> 
>  drivers/s390/cio/vfio_ccw_private.h |   9 +++
>  5 files changed, 169 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/s390/cio/vfio_ccw_ops.c
> 
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b920df8..32008b8 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -673,7 +673,7 @@ config EADM_SCH
>  config VFIO_CCW
>   def_tristate n
>   prompt "Support for VFIO-CCW subchannels"
> - depends on S390_CCW_IOMMU && VFIO
> + depends on S390_CCW_IOMMU && VFIO_MDEV
>   help
> This driver allows usage of VFIO-CCW subchannels.
>  
> diff --git a/drivers/s390/cio/Makefile b/drivers/s390/cio/Makefile
> index 1bec279..b0586b2 100644
> --- a/drivers/s390/cio/Makefile
> +++ b/drivers/s390/cio/Makefile
> @@ -18,5 +18,5 @@ obj-$(CONFIG_CCWGROUP) += ccwgroup.o
>  qdio-objs := qdio_main.o qdio_thinint.o qdio_debug.o qdio_setup.o
>  obj-$(CONFIG_QDIO) += qdio.o
>  
> -vfio_ccw-objs += vfio_ccw_drv.o vfio_ccw_cp.o
> +vfio_ccw-objs += vfio_ccw_drv.o vfio_ccw_cp.o vfio_ccw_ops.o
>  obj-$(CONFIG_VFIO_CCW) += vfio_ccw.o
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 5759d2a..ef34b15 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -23,7 +23,7 @@
>  /*
>   * Helpers
>   */
> -static int vfio_ccw_sch_quiesce(struct subchannel *sch)
> +int vfio_ccw_sch_quiesce(struct subchannel *sch)
>  {
>   struct vfio_ccw_private *private = dev_get_drvdata(>dev);
>   DECLARE_COMPLETION_ONSTACK(completion);
> @@ -156,8 +156,14 @@ static int vfio_ccw_sch_probe(struct subchannel *sch)
>   if (ret)
>   goto out_disable;
>  
> + ret = vfio_ccw_mdev_reg(sch);
> + if (ret)
> + goto out_rm_group;
> +
>   return 0;
>  
> +out_rm_group:
> + sysfs_remove_group(>dev.kobj, _subchannel_attr_group);
>  out_disable:
>   cio_disable_subchannel(sch);
>  out_free:
> @@ -172,6 +178,8 @@ static int vfio_ccw_sch_remove(struct subchannel *sch)
>  
>   vfio_ccw_sch_quiesce(sch);
>  
> + vfio_ccw_mdev_unreg(sch);
> +
>   sysfs_remove_group(>dev.kobj, _subchannel_attr_group);
>  
>   dev_set_drvdata(>dev, NULL);
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> new file mode 100644
> index 000..6031a10
> --- /dev/null
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -0,0 +1,149 @@
> +/*
> + * Physical device callbacks for vfio_ccw
> + *
> + * Copyright IBM Corp. 2017
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License (version 2 only)
> + * as published by the Free Software Foundation.
> + *
> + * Author(s): Dong Jia Shi 
> + *Xiao Feng Ren 
> + */
> +
> +#include 
> +#include 
> +
> +#include "vfio_ccw_private.h"
> +
> +#define MAX_INSTANCES1
> +static int available_instances = MAX_INSTANCES;
> +
> +static int vfio_ccw_mdev_notifier(struct notifier_block *nb,
> +   unsigned long action,
> +   void *data)
> +{
> + struct vfio_ccw_private *private =
> + container_of(nb, struct vfio_ccw_private, nb);
> +
> + if (!private)
> + return NOTIFY_STOP;
> +
> + /*
> +  * TODO:
> +  * Vendor drivers MUST unpin pages in response to an
> +  * invalidation.
> +  */
> + if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP)
> + return NOTIFY_BAD;
> +
> + return NOTIFY_DONE;
> +}
> +
> +static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
> +{
> + return sprintf(buf, "I/O subchannel (Non-QDIO)\n");
> +}
> +MDEV_TYPE_ATTR_RO(name);
> +
> +static ssize_t device_api_show(struct kobject *kobj, struct device *dev,
> +char *buf)
> +{
> + return sprintf(buf, "%s\n", VFIO_DEVICE_API_CCW_STRING);
> +}
> +MDEV_TYPE_ATTR_RO(device_api);
> +
> +static ssize_t available_instances_show(struct kobject *kobj,
> + struct device *dev, char *buf)
> +{
> + return sprintf(buf, "%d\n", available_instances);
> +}
> +MDEV_TYPE_ATTR_RO(available_instances);
> +
> +static struct 

[Qemu-devel] [Bug 1655764] Re: qemu-img convert -c compression can't decompress

[Jignasha]

When converting qcow2 with -c option, then after not able to boot VM
with compressed qcow2 image

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

Title:
  qemu-img convert -c compression can't decompress

Status in QEMU:
  Invalid

Bug description:
  Used -c compression option of qemu-img convert to compress qcow2,
  then libvirt mount for compressed image don't work as well as decompression 
also
  not working, tried zlib-flate to decompress

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

[Qemu-devel] [Bug 1655764] Re: qemu-img convert -c compression can't decompress

[Jignasha]

Yes used qcow2 format only when compressing, I don't think due to older
version problem

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

Title:
  qemu-img convert -c compression can't decompress

Status in QEMU:
  Invalid

Bug description:
  Used -c compression option of qemu-img convert to compress qcow2,
  then libvirt mount for compressed image don't work as well as decompression 
also
  not working, tried zlib-flate to decompress

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

Re: [Qemu-devel] [PATCH 2/2] vl: Ensure the cpu_synchronize_all_post_init func in the appropriate location

[Eduardo Habkost]

On Tue, Jan 17, 2017 at 10:42:32PM +0800, Dou Liyang wrote:
> As the commit a4088c3eecb5f said, In the cpu_synchronize_all_post_init(),
> we also use CPU_FOREACH macro to set all CPUs' namu_node.

I can't find commit a4088c3eecb5f, is it the commit ID of your
previous patch on your local tree? I don't see any NUMA-related
code triggered cpu_synchronize_all_post_init().

>   So, we should
> make sure that we call it after Qemu has already initialied all the CPUs.

> 
> The patch move the numa_post_machine_init func in the appropriate
> location.

This patch moves cpu_synchronize_all_post_init(), not
numa_post_machine_init(). Could you describe which bug you are
fixing, exactly? It doesn't seem to be related to NUMA.

> 
> Signed-off-by: Dou Liyang 
> ---
>  vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index f38b0e2..75adc2a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4547,8 +4547,6 @@ int main(int argc, char **argv, char **envp)
>  
>  audio_init();
>  
> -cpu_synchronize_all_post_init();
> -
>  if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
>parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
>  exit(1);
> @@ -4570,6 +4568,8 @@ int main(int argc, char **argv, char **envp)
>  exit(1);
>  }
>  
> +cpu_synchronize_all_post_init();
> +
>  numa_post_machine_init();
>  
>  rom_reset_order_override();
> -- 
> 2.5.5
> 
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 10/18] tcg/i386: add support for vector opcodes

[Richard Henderson]

On 01/17/2017 01:07 AM, Kirill Batuzov wrote:

To be able to generate vector operations in a TCG backend we need to do
several things.

1. We need to tell the register allocator about vector target's register.
   In case of x86 we'll use xmm0..xmm7. xmm7 is designated as a scratch
   register, others can be used by the register allocator.

2. We need a new constraint to indicate where to use vector registers. In
   this commit the 'V' constraint is introduced.

3. We need to be able to generate bare minimum: load, store and reg-to-reg
   move. MOVDQU is used for loads and stores. MOVDQA is used for reg-to-reg
   moves.

4. Finally we need to support any other opcodes we want. INDEX_op_add_i32x4
   is the only one for now. The PADDD instruction handles it perfectly.

Signed-off-by: Kirill Batuzov 
---
 tcg/i386/tcg-target.h |  24 +-
 tcg/i386/tcg-target.inc.c | 109 +++---
 2 files changed, 125 insertions(+), 8 deletions(-)

diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index 524cfc6..974a58b 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -29,8 +29,14 @@
 #define TCG_TARGET_TLB_DISPLACEMENT_BITS 31

 #ifdef __x86_64__
-# define TCG_TARGET_REG_BITS  64
-# define TCG_TARGET_NB_REGS   16
+# define TCG_TARGET_HAS_REG128 1
+# ifdef TCG_TARGET_HAS_REG128
+#  define TCG_TARGET_REG_BITS  64
+#  define TCG_TARGET_NB_REGS   24
+# else
+#  define TCG_TARGET_REG_BITS  64
+#  define TCG_TARGET_NB_REGS   16
+# endif
 #else
 # define TCG_TARGET_REG_BITS  32
 # define TCG_TARGET_NB_REGS8
@@ -56,6 +62,16 @@ typedef enum {
 TCG_REG_R13,
 TCG_REG_R14,
 TCG_REG_R15,
+#ifdef TCG_TARGET_HAS_REG128
+TCG_REG_XMM0,
+TCG_REG_XMM1,
+TCG_REG_XMM2,
+TCG_REG_XMM3,
+TCG_REG_XMM4,
+TCG_REG_XMM5,
+TCG_REG_XMM6,
+TCG_REG_XMM7,
+#endif


There's no need to conditionalize this.  The registers can be always defined 
even if they're not used.  We really really really want to keep ifdefs to an 
absolute minimum.


Why are you not defining xmm8-15?


@@ -634,9 +662,24 @@ static inline void tgen_arithr(TCGContext *s, int subop, 
int dest, int src)
 static inline void tcg_out_mov(TCGContext *s, TCGType type,
TCGReg ret, TCGReg arg)
 {
+int opc;
 if (arg != ret) {
-int opc = OPC_MOVL_GvEv + (type == TCG_TYPE_I64 ? P_REXW : 0);
-tcg_out_modrm(s, opc, ret, arg);
+switch (type) {
+#ifdef TCG_TARGET_HAS_REG128
+case TCG_TYPE_V128:
+ret -= TCG_REG_XMM0;
+arg -= TCG_REG_XMM0;
+tcg_out_modrm(s, OPC_MOVDQA_R2R, ret, arg);
+break;
+#endif
+case TCG_TYPE_I32:
+case TCG_TYPE_I64:
+opc = OPC_MOVL_GvEv + (type == TCG_TYPE_I64 ? P_REXW : 0);
+tcg_out_modrm(s, opc, ret, arg);
+break;
+default:
+assert(0);


g_assert_not_reached().

Again, no ifdefs.

We probably want to generate avx1 code when the cpu supports it, to avoid mode 
switches in the vector registers.  In this case, simply issue the same opcode, 
vex encoded.



+#ifdef TCG_TARGET_HAS_REG128
+{ INDEX_op_add_i32x4, { "V", "0", "V" } },
+#endif


And, clearly, you need to rebase.


r~

[Qemu-devel] [Bug 1414466] Re: -net user, hostfwd=... is not working(qemu-system-aarch64)

[P.Constantine]

Redirect does happen, but no packets appear on guest interface: checked
by iptables rule for `NEW` on `tcpport 22` inside guest.

On host:

$ sudo lsof -itcp | grep 2851
packer24233  kit6u  IPv4 1532725  0t0  TCP 
localhost:52822->localhost:2851 (ESTABLISHED)
qemu-syst 24286  kit   12u  IPv4 1530169  0t0  TCP *:2851 (LISTEN)
qemu-syst 24286  kit   21u  IPv4 1575945  0t0  TCP 
localhost:2851->localhost:52820 (CLOSE_WAIT)
qemu-syst 24286  kit   22u  IPv4 1532726  0t0  TCP 
localhost:2851->localhost:52822 (ESTABLISHED)
qemu-syst 24286  kit   23u  IPv4 1532645  0t0  TCP 
localhost:2851->localhost:52812 (CLOSE_WAIT)
qemu-syst 24286  kit   24u  IPv4 1532646  0t0  TCP 
localhost:2851->localhost:52814 (CLOSE_WAIT)

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

Title:
  -net user,hostfwd=... is not working(qemu-system-aarch64)

Status in QEMU:
  Confirmed

Bug description:
  QEMU version: git a46b3aaf6bb038d4f6f192a84df204f10929e75c

   /opt/qemu.git/bin/qemu-system-aarch64 --version
  QEMU emulator version 2.2.50, Copyright (c) 2003-2008 Fabrice Bellard

  Hosts:
  ovs - host machine (Ubuntu 14.04.1, x86_64)
  debian8-arm64 - guest 

  Guest start:
  user@ovs:~$ /opt/qemu.git/bin/qemu-system-aarch64 -machine virt -cpu 
cortex-a57 -nographic -smp 1 -m 512 -kernel vmlinuz-run -initrd initrd-run.img 
-append "root=/dev/sda2 console=ttyAMA0" -global virtio-blk-device.scsi=off 
-device virtio-scsi-device,id=scsi -drive 
file=debian8-arm64.img,id=rootimg,cache=unsafe,if=none -device 
scsi-hd,drive=rootimg -netdev user,id=unet -device 
virtio-net-device,netdev=unet -net user,hostfwd=tcp:127.0.0.1:1122-:22

  root@debian8-arm64:~# netstat -ntplu | grep ssh
  tcp0  0 0.0.0.0:22  0.0.0.0:*   LISTEN
  410/sshd
  tcp6   0  0 :::22   :::*LISTEN
  410/sshd   

  (no firewall in guest vm)

  user@ovs:~$ netstat -ntplu | grep 1122
  tcp0  0 127.0.0.1:1122  0.0.0.0:*   LISTEN
  18722/qemu-system-a

  user@ovs:~$ time ssh user@127.0.0.1 -p 1122
  ssh_exchange_identification: read: Connection reset by peer

  real  1m29.341s
  user  0m0.005s
  sys   0m0.000s

  Inside guest vm sshd works fine:
  root@debian8-arm64:~# ssh user@127.0.0.1 -p 22
  user@127.0.0.1's password: 
  
  user@debian8-arm64:~$ exit
  logout
  Connection to 127.0.0.1 closed.

  root@debian8-arm64:~# ssh user@10.0.2.15 -p 22
  user@10.0.2.15's password: 
  ...
  user@debian8-arm64:~$ exit
  logout
  Connection to 10.0.2.15 closed.

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

[Qemu-devel] [Bug 1655764] Re: qemu-img convert -c compression can't decompress

[Thomas Huth]

When reporting bugs, please always use the latest version of QEMU, old
versions like 2.1 are not supported anymore. I just also noticed that
Stefan Hajnoczi replied to the bug mail on the qemu-devel mailing list
(see https://www.mail-archive.com/qemu-devel@nongnu.org/msg422186.html)
- seems like the bridge did not mirror this to the bug tracker:

"QEMU image compression uses the compression feature available in some
 disk image formats (like qcow2).  This is not the same as compressing a
 file with gzip, bzip2, or similar tools.
 Therefore this error is expected and not a bug."

** Changed in: qemu
   Status: New => Invalid

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

Title:
  qemu-img convert -c compression can't decompress

Status in QEMU:
  Invalid

Bug description:
  Used -c compression option of qemu-img convert to compress qcow2,
  then libvirt mount for compressed image don't work as well as decompression 
also
  not working, tried zlib-flate to decompress

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

Re: [Qemu-devel] [PATCH 1/2] vl: Ensure the numa_post_machine_init func in the appropriate location

[Eduardo Habkost]

On Tue, Jan 17, 2017 at 10:42:31PM +0800, Dou Liyang wrote:
> In the numa_post_machine_init(), we use CPU_FOREACH macro to set all
> CPUs' namu_node. So, we should make sure that we call it after Qemu
> has already initialied all the CPUs.
> 
> As we all know, the CPUs can be created by "-smp"(pc_new_cpu) or
> "-device"(qdev_device_add) command. But, before the device init,
> Qemu execute the numa_post_machine_init earlier. It makes the mapping
> of NUMA nodes and CPUs incorrect.
> 
> The patch move the numa_post_machine_init func in the appropriate
> location.
> 
> Signed-off-by: Dou Liyang 

I would like to move cpu_index initialization to
qom/cpu.c:cpu_common_realizefn(), and remove
numa_post_machine_init() completely. But this fixes the bug while
we don't do that.

Reviewed-by: Eduardo Habkost 

Queued on numa-next. Thanks!

> ---
>  vl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index c643d3f..f38b0e2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4549,8 +4549,6 @@ int main(int argc, char **argv, char **envp)
>  
>  cpu_synchronize_all_post_init();
>  
> -numa_post_machine_init();
> -
>  if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
>parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
>  exit(1);
> @@ -4571,6 +4569,9 @@ int main(int argc, char **argv, char **envp)
>device_init_func, NULL, NULL)) {
>  exit(1);
>  }
> +
> +numa_post_machine_init();
> +
>  rom_reset_order_override();
>  
>  /* Did we create any drives that we failed to create a device for? */
> -- 
> 2.5.5
> 
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 08/18] target/arm: support access to vector guest registers as globals

[Richard Henderson]

On 01/17/2017 01:07 AM, Kirill Batuzov wrote:

+for (i = 0; i < 16; i++) {
+overlap_temps[i][0] = GET_TCGV_V128(cpu_Q[i]);
+overlap_temps[i][1] = (TCGArg)-1;
+sub_temps[i][0] = GET_TCGV_V64(cpu_D[i * 2]);
+sub_temps[i][1] = GET_TCGV_V64(cpu_D[i * 2 + 1]);
+sub_temps[i][2] = (TCGArg)-1;
+tcg_temp_set_overlap_temps(GET_TCGV_V64(cpu_D[i * 2]),
+   overlap_temps[i]);
+tcg_temp_set_overlap_temps(GET_TCGV_V64(cpu_D[i * 2 + 1]),
+   overlap_temps[i]);
+tcg_temp_set_sub_temps(GET_TCGV_V128(cpu_Q[i]), sub_temps[i]);
 }


Should we simply detect this generically as the registers are declared?  This 
seems tedious to do for each target.



r~

[Qemu-devel] [Bug 1655764] Re: qemu-img convert -c compression can't decompress

[Jignasha]

qemu-img version 2.1.2, Copyright (c) 2004-2008 Fabrice Bellard

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

Title:
  qemu-img convert -c compression can't decompress

Status in QEMU:
  Invalid

Bug description:
  Used -c compression option of qemu-img convert to compress qcow2,
  then libvirt mount for compressed image don't work as well as decompression 
also
  not working, tried zlib-flate to decompress

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

[Qemu-devel] [Bug 1414466] Re: -net user, hostfwd=... is not working(qemu-system-aarch64)

[P.Constantine]

Doesn't work even with proper hostfwd
Doesn't work even with `-redir`

$ qemu-system-x86_64 -machine type=pc,accel=kvm -netdev
user,id=user.0,hostfwd=tcp::2851-:22 -display sdl -cpu host -smp cpus=2
-device rtl8139,netdev=user.0 -cdrom /home/kit/git/packer-
xenserver/packer_cache/57f4a00eef5b4d4157f20847e586e5ef2a503ee05c83c9296c08fd0c2f0c8e4f.iso
-boot once=d -vnc 127.0.0.1:19 -name XenServer62 -m 2048M -drive file
=output-
qemu/XenServer62,if=scsi,cache=writeback,discard=ignore,format=qcow2


** Changed in: qemu
   Status: Invalid => Confirmed

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

Title:
  -net user,hostfwd=... is not working(qemu-system-aarch64)

Status in QEMU:
  Confirmed

Bug description:
  QEMU version: git a46b3aaf6bb038d4f6f192a84df204f10929e75c

   /opt/qemu.git/bin/qemu-system-aarch64 --version
  QEMU emulator version 2.2.50, Copyright (c) 2003-2008 Fabrice Bellard

  Hosts:
  ovs - host machine (Ubuntu 14.04.1, x86_64)
  debian8-arm64 - guest 

  Guest start:
  user@ovs:~$ /opt/qemu.git/bin/qemu-system-aarch64 -machine virt -cpu 
cortex-a57 -nographic -smp 1 -m 512 -kernel vmlinuz-run -initrd initrd-run.img 
-append "root=/dev/sda2 console=ttyAMA0" -global virtio-blk-device.scsi=off 
-device virtio-scsi-device,id=scsi -drive 
file=debian8-arm64.img,id=rootimg,cache=unsafe,if=none -device 
scsi-hd,drive=rootimg -netdev user,id=unet -device 
virtio-net-device,netdev=unet -net user,hostfwd=tcp:127.0.0.1:1122-:22

  root@debian8-arm64:~# netstat -ntplu | grep ssh
  tcp0  0 0.0.0.0:22  0.0.0.0:*   LISTEN
  410/sshd
  tcp6   0  0 :::22   :::*LISTEN
  410/sshd   

  (no firewall in guest vm)

  user@ovs:~$ netstat -ntplu | grep 1122
  tcp0  0 127.0.0.1:1122  0.0.0.0:*   LISTEN
  18722/qemu-system-a

  user@ovs:~$ time ssh user@127.0.0.1 -p 1122
  ssh_exchange_identification: read: Connection reset by peer

  real  1m29.341s
  user  0m0.005s
  sys   0m0.000s

  Inside guest vm sshd works fine:
  root@debian8-arm64:~# ssh user@127.0.0.1 -p 22
  user@127.0.0.1's password: 
  
  user@debian8-arm64:~$ exit
  logout
  Connection to 127.0.0.1 closed.

  root@debian8-arm64:~# ssh user@10.0.2.15 -p 22
  user@10.0.2.15's password: 
  ...
  user@debian8-arm64:~$ exit
  logout
  Connection to 10.0.2.15 closed.

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

[Qemu-devel] [Bug 629791] Re: sysret sets invalid ss

[Thomas Huth]

** Changed in: qemu
   Status: New => Invalid

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

Title:
  sysret sets invalid ss

Status in QEMU:
  Invalid

Bug description:
  I'm developing an OS. I use only sysret to enter user space. When an
  interrupt occurred, it would GPF on iretq'ing from it. On
  investigating, the cs on the stack is 0x2b (valid and correct). The ss
  on the stack is 0x20, which has a rpl of 0 which is incorrect. iretq
  checks that and gpf's. Making the irq handler manually modify it to
  0x23 fixes it locally.

  This happens on the non-kvm'ed qemu. I haven't tried the kvm'ed one.
  Qemu version 0.12.5. I haven't tried with the current development
  version either.

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

[Qemu-devel] [Bug 1525123] Re: USB assert failure on hcd-uhci.c

[Thomas Huth]

Triaging old bug tickets ... can you still reproduce this issue with the
latest version of QEMU (version 2.8)?

** Information type changed from Private Security to Public Security

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

Title:
  USB assert failure on hcd-uhci.c

Status in QEMU:
  New

Bug description:
  When inserting the attached kernel moudle in the guest OS, QEMU quits
  with therse assert failure:

  [insert kernel module in guest root shell]
  root@qemu:~# insmod mymod.ko
  root@qemu:~#
  Connection closed by foreign host.

  [host message]
  qemu-system-x86_64: hw/usb/core.c:718: usb_ep_get: Assertion `pid == 0x69 || 
pid == 0xe1' failed.
  Aborted

  The direct cause of this bug is due to misimplementation of UHCI.
  According to Intel's UHCI design guide, packet identification in transfer 
descriptor must be one of these three values : IN (69h), OUT (E1h), and SETUP 
(2Dh). Any other value in this field must cause the HALT of only HOST 
CONTROLLER.

  However, due to misimplementation in uhci_handle_td, instead of host
  controller being halted, QEMU itself dies with assertion failure. The
  assertion code is in usb_ep_get():718, which is called during
  uhci_handle_td().

  Another issue resides in uhci_handle_td(). This function must check
  that transfer descriptor's pid is one of IN, OUT, SETUP before calling
  usb_ep_get() or other functions. If it does so, usb_ep_get() only
  needs to check if pid is not SETUP.

  This kind of assert failure can be misused by malwares to avoid being
  analyzed by terminating only in the virtual environments and still
  execute the malicious code in real machines.

  
  [How to run exploit code]
  Prepare linux kernel's source header, then type these lines in root shell.
  # make
  # insmod mymod.ko

  It needs uhci-hcd.h from linux kernel source.
  I attached linux 3.18.24's uhci-hcd.h for tempory measure; You should get 
proper version of uhci-hcd.h.
  In the following envrionment, this exploit worked, exiting whole QEMU, not 
only USB.

  QEMU was running on these environment :
  [CPU model] Intel(R) Core(TM) i5-4590 CPU @ 3.30GHz
  [qemu version] QEMU 2.5.0-rc3 (compiled from source, gcc 4.8.4)
  [host info] Ubuntu 14.04.3, x86_64, 3.19.0-32-generic
  [guest info] Ubuntu 14.04.3, x86_64, 3.19.0-28-generic
  [QEMU argument]
  x86_64-softmmu/qemu-system-x86_64 -hda /media/hdd/img/ubuntu1404.qcow2 \
   -m 512 \
   --usbdevice disk:format=qcow2:../usb.img \
   --enable-kvm

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

[Qemu-devel] [Bug 1655764] Re: qemu-img convert -c compression can't decompress

[Thomas Huth]

Which version of QEMU (or rather qemu-img) are you using?

** Information type changed from Private to Public

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

Title:
  qemu-img convert -c compression can't decompress

Status in QEMU:
  New

Bug description:
  Used -c compression option of qemu-img convert to compress qcow2,
  then libvirt mount for compressed image don't work as well as decompression 
also
  not working, tried zlib-flate to decompress

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

Re: [Qemu-devel] [PATCH 06/18] tcg: allow globals to overlap

[Richard Henderson]

On 01/17/2017 01:07 AM, Kirill Batuzov wrote:

Sometimes the target architecture may allow some parts of a register to be
accessed as a different register. If both of these registers are
implemented as globals in QEMU, then their content will overlap and the
change to one global will also change the value of the other. To handle
such situation properly, some fixes are needed in the register allocator
and liveness analysis.


You need to handle the overlap during optimization as well.  Otherwise you'll 
propagate values that you shouldn't.



r~

[Qemu-devel] [Bug 1523811] Re: USB assert failure on dev-storage.c

[Thomas Huth]

Triaging old bug tickets ... can you still reproduce this issue with the
latest version of QEMU (version 2.8)?

** Information type changed from Private Security to Public Security

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

Title:
  USB assert failure on dev-storage.c

Status in QEMU:
  New

Bug description:
  On executing the attached python script in the guest OS, QEMU dies
  with assert failure:

  [run python script in guest root shell]
  # python a.py

  [host message]
  qemu-system-x86_64: hw/usb/dev-storage.c:445: usb_msd_handle_data: Assertion 
`le32_to_cpu(s->csw.residue) == 0' failed.
  Aborted (core dumped)

  
  When I detach the kernel driver and send CBW and reattach it again, without 
conforming to the command/data/status protocol, QEMU dies.
  I think this is due to misimplementation of Command/Data/Status protocol in 
Bulk-only transfer.
  This kind of assert failure can be misused by malwares to avoid being 
analyzed by terminating only in the virtual environments and still execute the 
malicious code in real machines.
  Before running python script, make sure to change a.py that it should points 
to usb mass storage's vid and pid.

  QEMU was running on these environment : 
  [CPU model]Intel(R) Core(TM) i5-4590 CPU @ 3.30GHz
  [qemu version] QEMU 2.5.0-rc2 (compiled from source, gcc 4.8.4)
  [host info]Ubuntu 14.04.3, x86_64, 3.19.0-32-generic
  [guest info]   Ubuntu 14.04.3, x86_64, 3.19.0-28-generic
  [QEMU argument]
  x86_64-softmmu/qemu-system-x86_64 -hda /media/hdd/img/ubuntu1404.qcow2.5 \
-m 512 \
--usbdevice disk:format=qcow2:../usb.img.5 \
--enable-kvm

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

[Qemu-devel] [PATCH 2/3] stubs: acpi_table_add() stub

[Eduardo Habkost]

Create a stub to avoid an #ifdef in do_acpitable_option().

The stub should never be called because -acpitable is defined as
arch-specific in qemu-options.hx.

Signed-off-by: Eduardo Habkost 
---
 arch_init.c | 2 --
 stubs/acpi.c| 7 +++
 stubs/Makefile.objs | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)
 create mode 100644 stubs/acpi.c

diff --git a/arch_init.c b/arch_init.c
index 9647c8d337..0416d84211 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -237,7 +237,6 @@ void audio_init(void)
 
 void do_acpitable_option(const QemuOpts *opts)
 {
-#ifdef TARGET_I386
 Error *err = NULL;
 
 acpi_table_add(opts, );
@@ -245,7 +244,6 @@ void do_acpitable_option(const QemuOpts *opts)
 error_reportf_err(err, "Wrong acpi table provided: ");
 exit(1);
 }
-#endif
 }
 
 int kvm_available(void)
diff --git a/stubs/acpi.c b/stubs/acpi.c
new file mode 100644
index 00..65ef2d7086
--- /dev/null
+++ b/stubs/acpi.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi.h"
+
+void acpi_table_add(const QemuOpts *opts, Error **errp)
+{
+abort(); /* must never be called */
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 83ddcad3c3..18236935d5 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -51,3 +51,4 @@ stub-obj-y += ipmi.o
 stub-obj-y += pc_madt_cpu_entry.o
 stub-obj-y += migration-colo.o
 stub-obj-y += smbios.o
+stub-obj-y += acpi.o
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 1/3] stubs: Add smbios_entry_add() stub

[Eduardo Habkost]

Instead of using an #ifdef in arch_init.c, add a
smbios_entry_add() stub. The stub will never be called because
the -smbios option is defined as arch-specific in
qemu-options.hx.

Signed-off-by: Eduardo Habkost 
---
 include/sysemu/arch_init.h | 1 -
 arch_init.c| 7 ---
 stubs/smbios.c | 7 +++
 vl.c   | 2 +-
 stubs/Makefile.objs| 1 +
 5 files changed, 9 insertions(+), 9 deletions(-)
 create mode 100644 stubs/smbios.c

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 1c9dad1b72..88dcf77a62 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -29,7 +29,6 @@ extern const uint32_t arch_type;
 
 void select_soundhw(const char *optarg);
 void do_acpitable_option(const QemuOpts *opts);
-void do_smbios_option(QemuOpts *opts);
 void audio_init(void);
 int kvm_available(void);
 int xen_available(void);
diff --git a/arch_init.c b/arch_init.c
index 5cc58b2c35..9647c8d337 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -248,13 +248,6 @@ void do_acpitable_option(const QemuOpts *opts)
 #endif
 }
 
-void do_smbios_option(QemuOpts *opts)
-{
-#ifdef TARGET_I386
-smbios_entry_add(opts);
-#endif
-}
-
 int kvm_available(void)
 {
 #ifdef CONFIG_KVM
diff --git a/stubs/smbios.c b/stubs/smbios.c
new file mode 100644
index 00..e86e8f821c
--- /dev/null
+++ b/stubs/smbios.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/smbios/smbios.h"
+
+void smbios_entry_add(QemuOpts *opts)
+{
+abort(); /* must never be called */
+}
diff --git a/vl.c b/vl.c
index c643d3ff3a..38d812286a 100644
--- a/vl.c
+++ b/vl.c
@@ -3707,7 +3707,7 @@ int main(int argc, char **argv, char **envp)
 if (!opts) {
 exit(1);
 }
-do_smbios_option(opts);
+smbios_entry_add(opts);
 break;
 case QEMU_OPTION_fwcfg:
 opts = qemu_opts_parse_noisily(qemu_find_opts("fw_cfg"),
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 2b5bb74fce..83ddcad3c3 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -50,3 +50,4 @@ stub-obj-y += smbios_type_38.o
 stub-obj-y += ipmi.o
 stub-obj-y += pc_madt_cpu_entry.o
 stub-obj-y += migration-colo.o
+stub-obj-y += smbios.o
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 3/3] arch_init: Move acpi_table_add() call back to vl.c

[Eduardo Habkost]

Now that the code is not arch-dependent anymore, it can be moved
back to vl.c.

Signed-off-by: Eduardo Habkost 
---
 include/sysemu/arch_init.h |  1 -
 arch_init.c| 11 ---
 vl.c   |  7 ++-
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 88dcf77a62..20b01e3004 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -28,7 +28,6 @@ enum {
 extern const uint32_t arch_type;
 
 void select_soundhw(const char *optarg);
-void do_acpitable_option(const QemuOpts *opts);
 void audio_init(void);
 int kvm_available(void);
 int xen_available(void);
diff --git a/arch_init.c b/arch_init.c
index 0416d84211..591afe6e2c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -235,17 +235,6 @@ void audio_init(void)
 }
 }
 
-void do_acpitable_option(const QemuOpts *opts)
-{
-Error *err = NULL;
-
-acpi_table_add(opts, );
-if (err) {
-error_reportf_err(err, "Wrong acpi table provided: ");
-exit(1);
-}
-}
-
 int kvm_available(void)
 {
 #ifdef CONFIG_KVM
diff --git a/vl.c b/vl.c
index 38d812286a..9571b17eb6 100644
--- a/vl.c
+++ b/vl.c
@@ -65,6 +65,7 @@ int main(int argc, char **argv)
 #include "hw/bt.h"
 #include "sysemu/watchdog.h"
 #include "hw/smbios/smbios.h"
+#include "hw/acpi/acpi.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"
@@ -3699,7 +3700,11 @@ int main(int argc, char **argv, char **envp)
 if (!opts) {
 exit(1);
 }
-do_acpitable_option(opts);
+acpi_table_add(opts, );
+if (err) {
+error_reportf_err(err, "Wrong acpi table provided: ");
+exit(1);
+}
 break;
 case QEMU_OPTION_smbios:
 opts = qemu_opts_parse_noisily(qemu_find_opts("smbios"),
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 0/3] Remove arch_init SMBIOS and ACPI code

[Eduardo Habkost]

This creates stubs for SMBIOS and ACPI option parsing functions,
so that we don't need arch-specific #ifdefs in arch_init just for
calling the option parsers.

Eduardo Habkost (3):
  stubs: Add smbios_entry_add() stub
  stubs: acpi_table_add() stub
  arch_init: Move acpi_table_add() call back to vl.c

 include/sysemu/arch_init.h |  2 --
 arch_init.c| 20 
 stubs/acpi.c   |  7 +++
 stubs/smbios.c |  7 +++
 vl.c   |  9 +++--
 stubs/Makefile.objs|  2 ++
 6 files changed, 23 insertions(+), 24 deletions(-)
 create mode 100644 stubs/acpi.c
 create mode 100644 stubs/smbios.c

-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [PATCH v4 1/2] memory: tune mtree_print_mr() to dump mr type

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> We were dumping RW bits for each memory region, that might be confusing.
> It'll make more sense to dump the memory region type directly rather
> than the RW bits since that's how the bits are derived.
> 
> Meanwhile, with some slight cleanup in the function.
> 
> Signed-off-by: Peter Xu 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  memory.c | 48 +++-
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 2bfc37f..c42bde4 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2450,6 +2450,21 @@ void address_space_destroy(AddressSpace *as)
>  call_rcu(as, do_address_space_destroy, rcu);
>  }
>  
> +static const char *memory_region_type(MemoryRegion *mr)
> +{
> +if (memory_region_is_ram_device(mr)) {
> +return "ramd";
> +} else if (memory_region_is_romd(mr)) {
> +return "romd";
> +} else if (memory_region_is_rom(mr)) {
> +return "rom";
> +} else if (memory_region_is_ram(mr)) {
> +return "ram";
> +} else {
> +return "i/o";
> +}
> +}
> +
>  typedef struct MemoryRegionList MemoryRegionList;
>  
>  struct MemoryRegionList {
> @@ -2459,6 +2474,10 @@ struct MemoryRegionList {
>  
>  typedef QTAILQ_HEAD(queue, MemoryRegionList) MemoryRegionListHead;
>  
> +#define MR_SIZE(size) (int128_nz(size) ? (hwaddr)int128_get64( \
> +   int128_sub((size), int128_one())) : 0)
> +#define MTREE_INDENT "  "
> +
>  static void mtree_print_mr(fprintf_function mon_printf, void *f,
> const MemoryRegion *mr, unsigned int level,
> hwaddr base,
> @@ -2474,7 +2493,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
> void *f,
>  }
>  
>  for (i = 0; i < level; i++) {
> -mon_printf(f, "  ");
> +mon_printf(f, MTREE_INDENT);
>  }
>  
>  if (mr->alias) {
> @@ -2494,37 +2513,24 @@ static void mtree_print_mr(fprintf_function 
> mon_printf, void *f,
>  QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
>  }
>  mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
> -   " (prio %d, %c%c): alias %s @%s " TARGET_FMT_plx
> +   " (prio %d, %s): alias %s @%s " TARGET_FMT_plx
> "-" TARGET_FMT_plx "%s\n",
> base + mr->addr,
> -   base + mr->addr
> -   + (int128_nz(mr->size) ?
> -  (hwaddr)int128_get64(int128_sub(mr->size,
> -  int128_one())) : 0),
> +   base + mr->addr + MR_SIZE(mr->size),
> mr->priority,
> -   mr->romd_mode ? 'R' : '-',
> -   !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
> -   : '-',
> +   memory_region_type((MemoryRegion *)mr),
> memory_region_name(mr),
> memory_region_name(mr->alias),
> mr->alias_offset,
> -   mr->alias_offset
> -   + (int128_nz(mr->size) ?
> -  (hwaddr)int128_get64(int128_sub(mr->size,
> -  int128_one())) : 0),
> +   mr->alias_offset + MR_SIZE(mr->size),
> mr->enabled ? "" : " [disabled]");
>  } else {
>  mon_printf(f,
> -   TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): 
> %s%s\n",
> +   TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): 
> %s%s\n",
> base + mr->addr,
> -   base + mr->addr
> -   + (int128_nz(mr->size) ?
> -  (hwaddr)int128_get64(int128_sub(mr->size,
> -  int128_one())) : 0),
> +   base + mr->addr + MR_SIZE(mr->size),
> mr->priority,
> -   mr->romd_mode ? 'R' : '-',
> -   !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
> -   : '-',
> +   memory_region_type((MemoryRegion *)mr),
> memory_region_name(mr),
> mr->enabled ? "" : " [disabled]");
>  }
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v6 kernel 3/5] virtio-balloon: speed up inflate/deflate process

[Michael S. Tsirkin]

On Wed, Dec 21, 2016 at 02:52:26PM +0800, Liang Li wrote:
>  
> - /* We should always be able to add one buffer to an empty queue. */
> - virtqueue_add_outbuf(vq, , 1, vb, GFP_KERNEL);
> - virtqueue_kick(vq);
> +static void do_set_resp_bitmap(struct virtio_balloon *vb,
> + unsigned long base_pfn, int pages)
>  
> - /* When host has read buffer, this completes via balloon_ack */
> - wait_event(vb->acked, virtqueue_get_buf(vq, ));
> +{
> + __le64 *range = vb->resp_data + vb->resp_pos;
>  
> + if (pages > (1 << VIRTIO_BALLOON_NR_PFN_BITS)) {
> + /* when the length field can't contain pages, set it to 0 to

/*
 * Multi-line
 * comments
 * should look like this.
 */

Also, pls start sentences with an upper-case letter.

> +  * indicate the actual length is in the next __le64;
> +  */

This is part of the interface so should be documented as such.

> + *range = cpu_to_le64((base_pfn <<
> + VIRTIO_BALLOON_NR_PFN_BITS) | 0);
> + *(range + 1) = cpu_to_le64(pages);
> + vb->resp_pos += 2;

Pls use structs for this kind of stuff.

> + } else {
> + *range = (base_pfn << VIRTIO_BALLOON_NR_PFN_BITS) | pages;
> + vb->resp_pos++;
> + }
> +}

Re: [Qemu-devel] [virtio-dev] Re: [PATCH v6 kernel 2/5] virtio-balloon: define new feature bit and head struct

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 09:24:22AM +, Li, Liang Z wrote:
> > On Wed, Dec 21, 2016 at 02:52:25PM +0800, Liang Li wrote:
> > > Add a new feature which supports sending the page information with
> > > range array. The current implementation uses PFNs array, which is not
> > > very efficient. Using ranges can improve the performance of
> > > inflating/deflating significantly.
> > >
> > > Signed-off-by: Liang Li 
> > > Cc: Michael S. Tsirkin 
> > > Cc: Paolo Bonzini 
> > > Cc: Cornelia Huck 
> > > Cc: Amit Shah 
> > > Cc: Dave Hansen 
> > > Cc: Andrea Arcangeli 
> > > Cc: David Hildenbrand 
> > > ---
> > >  include/uapi/linux/virtio_balloon.h | 12 
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/virtio_balloon.h
> > > b/include/uapi/linux/virtio_balloon.h
> > > index 343d7dd..2f850bf 100644
> > > --- a/include/uapi/linux/virtio_balloon.h
> > > +++ b/include/uapi/linux/virtio_balloon.h
> > > @@ -34,10 +34,14 @@
> > >  #define VIRTIO_BALLOON_F_MUST_TELL_HOST  0 /* Tell before
> > reclaiming pages */
> > >  #define VIRTIO_BALLOON_F_STATS_VQ1 /* Memory Stats virtqueue
> > */
> > >  #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM  2 /* Deflate balloon
> > on OOM */
> > > +#define VIRTIO_BALLOON_F_PAGE_RANGE  3 /* Send page info
> > with ranges */
> > >
> > >  /* Size of a PFN in the balloon interface. */  #define
> > > VIRTIO_BALLOON_PFN_SHIFT 12
> > >
> > > +/* Bits width for the length of the pfn range */
> > 
> > What does this mean? Couldn't figure it out.
> > 
> > > +#define VIRTIO_BALLOON_NR_PFN_BITS 12
> > > +
> > >  struct virtio_balloon_config {
> > >   /* Number of pages host wants Guest to give up. */
> > >   __u32 num_pages;
> > > @@ -82,4 +86,12 @@ struct virtio_balloon_stat {
> > >   __virtio64 val;
> > >  } __attribute__((packed));
> > >
> > > +/* Response header structure */
> > > +struct virtio_balloon_resp_hdr {
> > > + __le64 cmd : 8; /* Distinguish different requests type */
> > > + __le64 flag: 8; /* Mark status for a specific request type */
> > > + __le64 id : 16; /* Distinguish requests of a specific type */
> > > + __le64 data_len: 32; /* Length of the following data, in bytes */
> > 
> > This use of __le64 makes no sense.  Just use u8/le16/le32 pls.
> > 
> 
> Got it, will change in the next version. 
> 
> And could help take a look at other parts? as well as the QEMU part.
> 
> Thanks!
> Liang

Yes but first I would like to understand how come no fields
in this new structure come up if I search for them in the
following patch. I don't see why should I waste time on
reviewing the implementation if the interface isn't
reasonable. You don't have to waste it too - just send RFC
patches with the header until we can agree on it.

-- 
MST

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Michael S. Tsirkin]

On Tue, Jan 17, 2017 at 06:53:17PM +, Felipe Franciosi wrote:
> 
> > On 17 Jan 2017, at 10:41, Michael S. Tsirkin  wrote:
> > 
> > On Fri, Jan 13, 2017 at 10:29:46PM +, Felipe Franciosi wrote:
> >> 
> >>> On 13 Jan 2017, at 10:18, Michael S. Tsirkin  wrote:
> >>> 
> >>> On Fri, Jan 13, 2017 at 05:15:22PM +, Felipe Franciosi wrote:
>  
> > On 13 Jan 2017, at 09:04, Michael S. Tsirkin  wrote:
> > 
> > On Fri, Jan 13, 2017 at 03:09:46PM +, Felipe Franciosi wrote:
> >> Hi Marc-Andre,
> >> 
> >>> On 13 Jan 2017, at 07:03, Marc-André Lureau  
> >>> wrote:
> >>> 
> >>> Hi
> >>> 
> >>> - Original Message -
>  Currently, VQs are started as soon as a SET_VRING_KICK is received. 
>  That
>  is too early in the VQ setup process, as the backend might not yet 
>  have
> >>> 
> >>> I think we may want to reconsider queue_set_started(), move it 
> >>> elsewhere, since kick/call fds aren't mandatory to process the rings.
> >> 
> >> Hmm. The fds aren't mandatory, but I imagine in that case we should 
> >> still receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
> >> VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?
> > 
> > Please look at docs/specs/vhost-user.txt, Starting and stopping rings
> > 
> > The spec says:
> > Client must start ring upon receiving a kick (that is, 
> > detecting that
> > file descriptor is readable) on the descriptor specified by
> > VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
> > VHOST_USER_GET_VRING_BASE.
>  
>  Yes I have seen the spec, but there is a race with the current 
>  libvhost-user code which needs attention. My initial proposal (which got 
>  turned down) was to send a spurious notification upon seeing a callfd. 
>  Then I came up with this proposal. See below.
>  
> > 
> > 
> >>> 
>  a callfd to notify in case it received a kick and fully processed the
>  request/command. This patch only starts a VQ when a SET_VRING_CALL is
>  received.
> >>> 
> >>> I don't like that much, as soon as the kick fd is received, it should 
> >>> start polling it imho. callfd is optional, it may have one and not 
> >>> the other.
> >> 
> >> So the question is whether we should be receiving a SET_VRING_CALL 
> >> anyway or not, regardless of an fd being sent. (I think we do, but I 
> >> haven't done extensive testing with other device types.)
> > 
> > I would say not, only KICK is mandatory and that is also not enough
> > to process ring. You must wait for it to be readable.
>  
>  The problem is that Qemu takes time between sending the kickfd and the 
>  callfd. Hence the race. Consider this scenario:
>  
>  1) Guest configures the device
>  2) Guest put a request on a virtq
>  3) Guest kicks
>  4) Qemu starts configuring the backend
>  4.a) Qemu sends the masked callfds
>  4.b) Qemu sends the virtq sizes and addresses
>  4.c) Qemu sends the kickfds
>  
>  (When using MQ, Qemu will only send the callfd once all VQs are 
>  configured)
>  
>  5) The backend starts listening on the kickfd upon receiving it
>  6) The backend picks up the guest's request
>  7) The backend processes the request
>  8) The backend puts the response on the used ring
>  9) The backend notifies the masked callfd
>  
>  4.d) Qemu sends the callfds
>  
>  At which point the guest missed the notification and gets stuck.
>  
>  Perhaps you prefer my initial proposal of sending a spurious 
>  notification when the backend sees a callfd?
>  
>  Felipe
> >>> 
> >>> I thought we read the masked callfd when we unmask it,
> >>> and forward the interrupt. See kvm_irqfd_assign:
> >>> 
> >>>   /*
> >>>* Check if there was an event already pending on the eventfd
> >>>* before we registered, and trigger it as if we didn't miss it.
> >>>*/
> >>>   events = f.file->f_op->poll(f.file, >pt);
> >>> 
> >>>   if (events & POLLIN)
> >>>   schedule_work(>inject);
> >>> 
> >>> 
> >>> 
> >>> Is this a problem you observe in practice?
> >> 
> >> Thanks for pointing out to this code; I wasn't aware of it.
> >> 
> >> Indeed I'm encountering it in practice. And I've checked that my kernel 
> >> has the code above.
> >> 
> >> Starts to sound like a race:
> >> Qemu registers the new notifier with kvm
> >> Backend kicks the (now no longer registered) maskfd
> > 
> > vhost user is not supposed to use maskfd at all.
> > 
> > We have this code:
> >if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> >dev->use_guest_notifier_mask = false;
> >}
> > 
> > isn't it 

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Felipe Franciosi]

> On 17 Jan 2017, at 10:41, Michael S. Tsirkin  wrote:
> 
> On Fri, Jan 13, 2017 at 10:29:46PM +, Felipe Franciosi wrote:
>> 
>>> On 13 Jan 2017, at 10:18, Michael S. Tsirkin  wrote:
>>> 
>>> On Fri, Jan 13, 2017 at 05:15:22PM +, Felipe Franciosi wrote:
 
> On 13 Jan 2017, at 09:04, Michael S. Tsirkin  wrote:
> 
> On Fri, Jan 13, 2017 at 03:09:46PM +, Felipe Franciosi wrote:
>> Hi Marc-Andre,
>> 
>>> On 13 Jan 2017, at 07:03, Marc-André Lureau  wrote:
>>> 
>>> Hi
>>> 
>>> - Original Message -
 Currently, VQs are started as soon as a SET_VRING_KICK is received. 
 That
 is too early in the VQ setup process, as the backend might not yet have
>>> 
>>> I think we may want to reconsider queue_set_started(), move it 
>>> elsewhere, since kick/call fds aren't mandatory to process the rings.
>> 
>> Hmm. The fds aren't mandatory, but I imagine in that case we should 
>> still receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
>> VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?
> 
> Please look at docs/specs/vhost-user.txt, Starting and stopping rings
> 
> The spec says:
>   Client must start ring upon receiving a kick (that is, detecting that
>   file descriptor is readable) on the descriptor specified by
>   VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
>   VHOST_USER_GET_VRING_BASE.
 
 Yes I have seen the spec, but there is a race with the current 
 libvhost-user code which needs attention. My initial proposal (which got 
 turned down) was to send a spurious notification upon seeing a callfd. 
 Then I came up with this proposal. See below.
 
> 
> 
>>> 
 a callfd to notify in case it received a kick and fully processed the
 request/command. This patch only starts a VQ when a SET_VRING_CALL is
 received.
>>> 
>>> I don't like that much, as soon as the kick fd is received, it should 
>>> start polling it imho. callfd is optional, it may have one and not the 
>>> other.
>> 
>> So the question is whether we should be receiving a SET_VRING_CALL 
>> anyway or not, regardless of an fd being sent. (I think we do, but I 
>> haven't done extensive testing with other device types.)
> 
> I would say not, only KICK is mandatory and that is also not enough
> to process ring. You must wait for it to be readable.
 
 The problem is that Qemu takes time between sending the kickfd and the 
 callfd. Hence the race. Consider this scenario:
 
 1) Guest configures the device
 2) Guest put a request on a virtq
 3) Guest kicks
 4) Qemu starts configuring the backend
 4.a) Qemu sends the masked callfds
 4.b) Qemu sends the virtq sizes and addresses
 4.c) Qemu sends the kickfds
 
 (When using MQ, Qemu will only send the callfd once all VQs are configured)
 
 5) The backend starts listening on the kickfd upon receiving it
 6) The backend picks up the guest's request
 7) The backend processes the request
 8) The backend puts the response on the used ring
 9) The backend notifies the masked callfd
 
 4.d) Qemu sends the callfds
 
 At which point the guest missed the notification and gets stuck.
 
 Perhaps you prefer my initial proposal of sending a spurious notification 
 when the backend sees a callfd?
 
 Felipe
>>> 
>>> I thought we read the masked callfd when we unmask it,
>>> and forward the interrupt. See kvm_irqfd_assign:
>>> 
>>>   /*
>>>* Check if there was an event already pending on the eventfd
>>>* before we registered, and trigger it as if we didn't miss it.
>>>*/
>>>   events = f.file->f_op->poll(f.file, >pt);
>>> 
>>>   if (events & POLLIN)
>>>   schedule_work(>inject);
>>> 
>>> 
>>> 
>>> Is this a problem you observe in practice?
>> 
>> Thanks for pointing out to this code; I wasn't aware of it.
>> 
>> Indeed I'm encountering it in practice. And I've checked that my kernel has 
>> the code above.
>> 
>> Starts to sound like a race:
>> Qemu registers the new notifier with kvm
>> Backend kicks the (now no longer registered) maskfd
> 
> vhost user is not supposed to use maskfd at all.
> 
> We have this code:
>if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
>dev->use_guest_notifier_mask = false;
>}
> 
> isn't it effective?

I'm observing this problem when using vhost-user-scsi, not -net. So the code 
above is not in effect. Anyway, I'd expect the race I described to also happen 
on vhost-scsi.

The problem is aggravated on storage for the following reason:
SeaBIOS configures the vhost-(user)-scsi device and finds the boot drive and 
reads the boot data.
Then the guest kernel boots, 

[Qemu-devel] [PATCH 0/3] arch_init: Move soundhw code to hw/audio/soundhw.c

[Eduardo Habkost]

This moves the arch_init.c soundhw code to its own file, renames
audio_init() to soundhw_init(), and renames hw/audio/audio.h to
hw/audio/soundhw.h.

Eduardo Habkost (3):
  audio: Move arch_init audio code to hw/audio/soundhw.c
  audio: Rename audio_init() to soundhw_init()
  audio: Rename hw/audio/audio.h to hw/audio/soundhw.h

 include/hw/audio/{audio.h => soundhw.h} |   3 +
 include/sysemu/arch_init.h  |   2 -
 arch_init.c | 126 +-
 hw/audio/ac97.c |   2 +-
 hw/audio/adlib.c|   2 +-
 hw/audio/cs4231a.c  |   2 +-
 hw/audio/es1370.c   |   2 +-
 hw/audio/gus.c  |   2 +-
 hw/audio/intel-hda.c|   2 +-
 hw/audio/pcspk.c|   2 +-
 hw/audio/sb16.c |   2 +-
 hw/audio/soundhw.c  | 156 
 vl.c|   3 +-
 hw/audio/Makefile.objs  |   2 +
 14 files changed, 172 insertions(+), 136 deletions(-)
 rename include/hw/audio/{audio.h => soundhw.h} (81%)
 create mode 100644 hw/audio/soundhw.c

-- 
2.11.0.259.g40922b1

[Qemu-devel] [Bug 1581936] Re: Frozen Windows 7 VMs with VGA CVE-2016-3712 fix (2.6.0 and 2.5.1.1)

[Thomas Huth]

Commit 94ef4f337fb614f18b7 has been released with QEMU version 2.7

** Changed in: qemu
   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/1581936

Title:
  Frozen Windows 7 VMs with VGA CVE-2016-3712 fix (2.6.0 and 2.5.1.1)

Status in QEMU:
  Fix Released

Bug description:
  Hi,

  As already posted on the QEMU devel list [1] I stumbled upon a problem
  with QEMU in version 2.5.1.1 and 2.6.0.

  the VM shows Windows loading
  files for the installation, then the "Starting Windows" screen appears
  here it hangs and never continues.

  Changing the "-vga" option to cirrus solves this, the installation can
  proceed and finish. When changing back to std (or also qxl, vmware) the
  installed VM also hangs on the "Starting Windows" screen while qemu
  showing a little but no excessive load.

  This phenomena appears also with QEMU 2.6.0 but not with 2.6.0-rc4, a
  git bisect shows fd3c136b3e1482cd0ec7285d6bc2a3e6a62c38d7 (vga: make
  sure vga register setup for vbe stays intact (CVE-2016-3712)) as the
  culprit for this regression, as its a fix for a DoS its not an option to
  just revert it, I guess.

  The bisect log is:

  git bisect start
  # bad: [bfc766d38e1fae5767d43845c15c79ac8fa6d6af] Update version for v2.6.0 
release
  git bisect bad bfc766d38e1fae5767d43845c15c79ac8fa6d6af
  # good: [975eb6a547f809608ccb08c221552f11af25] Update version for 
v2.6.0-rc4 release
  git bisect good 975eb6a547f809608ccb08c221552f11af25
  # good: [2068192dcccd8a80dddfcc8df6164cf9c26e0fc4] vga: update vga register 
setup on vbe changes
  git bisect good 2068192dcccd8a80dddfcc8df6164cf9c26e0fc4
  # bad: [53db932604dfa7bb9241d132e0173894cf54261c] Merge remote-tracking 
branch 'remotes/kraxel/tags/pull-vga-20160509-1' into staging
  git bisect bad 53db932604dfa7bb9241d132e0173894cf54261c
  # bad: [fd3c136b3e1482cd0ec7285d6bc2a3e6a62c38d7] vga: make sure vga register 
setup for vbe stays intact (CVE-2016-3712).
  git bisect bad fd3c136b3e1482cd0ec7285d6bc2a3e6a62c38d7
  # first bad commit: [fd3c136b3e1482cd0ec7285d6bc2a3e6a62c38d7] vga: make sure 
vga register setup for vbe stays intact (CVE-2016-3712).

  
  I could reproduce that with QEMU 2.5.1 and QEMU 2.6 on a Debian derivate
  (Promox VE) with 4.4 Kernel and also with QEMU 2.6 on an Arch Linux
  System with a 4.5 Kernel, so it should not be host distro depended. Both
  machines have Intel x86_64 processors.
  The problem should be reproducible with said Versions or a build from
  git including the above mentioned commit (fd3c136) by starting a VM with
  an Windows 7 ISO, e.g.:

  Freezing installation (as vga defaults to std I marked it as optional):
  ./x86_64-softmmu/qemu-system-x86_64 -boot d -cdrom win7.iso -m 1024 [-vga 
(std|qxl|vmware)]

  Working installation:
  ./x86_64-softmmu/qemu-system-x86_64 -boot d -cdrom win7.iso -m 1024 -vga 
cirrus

  If someone has already an installed Windows 7 VM this behaviour should be
  also observable when trying to start it with the new versions of QEMU.

  Noteworthy may be that Windows 10 is working, I do not had time to get
  other Windows versions and test them, I'll do that as soon as possible.
  Various Linux system also seems do work fine, at least I did not ran
  into an issue there yet.

  I also tried testing with SeaBIOS and OVMF as firmware, as initially I
  had no idea what broke, both lead to the same result - without the 
  CVE-2016-3712 fix they both work, with not.
  Further, KVM enabled and disabled does not make any difference.

  
  [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg02416.html

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

[Qemu-devel] [PATCH 3/3] audio: Rename hw/audio/audio.h to hw/audio/soundhw.h

[Eduardo Habkost]

All the functions in hw/audio/audio.h are called "soundhw_*()"
and live in hw/audio/audiohw.c. Rename the header file for
consistency.

Signed-off-by: Eduardo Habkost 
---
 include/hw/audio/{audio.h => soundhw.h} | 0
 arch_init.c | 2 +-
 hw/audio/ac97.c | 2 +-
 hw/audio/adlib.c| 2 +-
 hw/audio/cs4231a.c  | 2 +-
 hw/audio/es1370.c   | 2 +-
 hw/audio/gus.c  | 2 +-
 hw/audio/intel-hda.c| 2 +-
 hw/audio/pcspk.c| 2 +-
 hw/audio/sb16.c | 2 +-
 hw/audio/soundhw.c  | 2 +-
 vl.c| 2 +-
 12 files changed, 11 insertions(+), 11 deletions(-)
 rename include/hw/audio/{audio.h => soundhw.h} (100%)

diff --git a/include/hw/audio/audio.h b/include/hw/audio/soundhw.h
similarity index 100%
rename from include/hw/audio/audio.h
rename to include/hw/audio/soundhw.h
diff --git a/arch_init.c b/arch_init.c
index 16465c3f54..da26b40056 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -27,7 +27,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/arch_init.h"
 #include "hw/pci/pci.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "hw/smbios/smbios.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index c30657501c..959c786261 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -19,7 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/hw.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/pci/pci.h"
 #include "sysemu/dma.h"
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 7836446fc8..a9356e83b5 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -25,7 +25,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/hw.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/isa/isa.h"
 
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index 3ecd0582bf..096e8e98d7 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -23,7 +23,7 @@
  */
 #include "qemu/osdep.h"
 #include "hw/hw.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev.h"
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index fe64c1ac37..dd7c23d185 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -28,7 +28,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/hw.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/pci/pci.h"
 #include "sysemu/dma.h"
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 3d08a6576a..21861be869 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -24,7 +24,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/hw.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/isa/isa.h"
 #include "gusemu.h"
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 537face94d..8e36147f64 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -22,7 +22,7 @@
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
 #include "qemu/timer.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "intel-hda.h"
 #include "intel-hda-defs.h"
 #include "sysemu/dma.h"
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 798002277b..c815a2b9ca 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -26,7 +26,7 @@
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
 #include "hw/isa/isa.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "qemu/timer.h"
 #include "hw/timer/i8254.h"
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 6b4427f242..6ab2f6f89a 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -23,7 +23,7 @@
  */
 #include "qemu/osdep.h"
 #include "hw/hw.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev.h"
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index 2178a5c8ef..710912e715 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -28,7 +28,7 @@
 #include "qom/object.h"
 #include "hw/isa/isa.h"
 #include "hw/pci/pci.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 
 struct soundhw {
 const char *name;
diff --git a/vl.c b/vl.c
index a5bf85ebec..27213fd6af 100644
--- a/vl.c
+++ b/vl.c
@@ -87,7 +87,7 @@ int main(int argc, char **argv)
 #include "migration/block.h"
 #include "sysemu/tpm.h"
 #include "sysemu/dma.h"
-#include "hw/audio/audio.h"
+#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "migration/migration.h"
 #include "sysemu/cpus.h"
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [PATCH v6 wave 2 2/3] hw/isa/lpc_ich9: add broadcast SMI feature

[Laszlo Ersek]

On 01/17/17 15:20, Igor Mammedov wrote:
> On Tue, 17 Jan 2017 14:46:05 +0100
> Laszlo Ersek  wrote:
> 
>> On 01/17/17 14:20, Igor Mammedov wrote:
>>> On Tue, 17 Jan 2017 13:06:13 +0100
>>> Laszlo Ersek  wrote:
>>>   
 On 01/17/17 12:21, Igor Mammedov wrote:  
> On Mon, 16 Jan 2017 21:46:55 +0100
> Laszlo Ersek  wrote:
> 
>> On 01/13/17 14:09, Igor Mammedov wrote:
>>> On Thu, 12 Jan 2017 19:24:45 +0100
>>> Laszlo Ersek  wrote:
>>>   
 The generic edk2 SMM infrastructure prefers
 EFI_SMM_CONTROL2_PROTOCOL.Trigger() to inject an SMI on each 
 processor. If
 Trigger() only brings the current processor into SMM, then edk2 
 handles it
 in the following ways:

 (1) If Trigger() is executed by the BSP (which is guaranteed before
 ExitBootServices(), but is not necessarily true at runtime), then:

 (a) If edk2 has been configured for "traditional" SMM 
 synchronization,
 then the BSP sends directed SMIs to the APs with APIC delivery,
 bringing them into SMM individually. Then the BSP runs the SMI
 handler / dispatcher.

 (b) If edk2 has been configured for "relaxed" SMM synchronization,
 then the APs that are not already in SMM are not brought in, 
 and
 the BSP runs the SMI handler / dispatcher.

 (2) If Trigger() is executed by an AP (which is possible after
 ExitBootServices(), and can be forced e.g. by "taskset -c 1
 efibootmgr"), then the AP in question brings in the BSP with a
 directed SMI, and the BSP runs the SMI handler / dispatcher.

 The smaller problem with (1a) and (2) is that the BSP and AP
 synchronization is slow. For example, the "taskset -c 1 efibootmgr"
 command from (2) can take more than 3 seconds to complete, because
 efibootmgr accesses non-volatile UEFI variables intensively.

 The larger problem is that QEMU's current behavior diverges from the
 behavior usually seen on physical hardware, and that keeps exposing
 obscure corner cases, race conditions and other instabilities in edk2,
 which generally expects / prefers a software SMI to affect all CPUs at
 once.

 Therefore introduce the "broadcast SMI" feature that causes QEMU to 
 inject
 the SMI on all VCPUs.

 While the original posting of this patch
 
 only intended to speed up (2), based on our recent "stress testing" of 
 SMM
 this patch actually provides functional improvements.

 Cc: "Michael S. Tsirkin" 
 Cc: Gerd Hoffmann 
 Cc: Igor Mammedov 
 Cc: Paolo Bonzini 
 Signed-off-by: Laszlo Ersek 
 Reviewed-by: Michael S. Tsirkin 
 ---

 Notes:
 v6:
 - no changes, pick up Michael's R-b
 
 v5:
 - replace the ICH9_LPC_SMI_F_BROADCAST bit value with the
   ICH9_LPC_SMI_F_BROADCAST_BIT bit position (necessary for
   DEFINE_PROP_BIT() in the next patch)

  include/hw/i386/ich9.h |  3 +++
  hw/isa/lpc_ich9.c  | 10 +-
  2 files changed, 12 insertions(+), 1 deletion(-)

 diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
 index da1118727146..18dcca7ebcbf 100644
 --- a/include/hw/i386/ich9.h
 +++ b/include/hw/i386/ich9.h
 @@ -250,4 +250,7 @@ Object *ich9_lpc_find(void);
  #define ICH9_SMB_HST_D1 0x06
  #define ICH9_SMB_HOST_BLOCK_DB  0x07
  
 +/* bit positions used in fw_cfg SMI feature negotiation */
 +#define ICH9_LPC_SMI_F_BROADCAST_BIT0
 +
  #endif /* HW_ICH9_H */
 diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
 index 376b7801a42c..ced6f803a4f2 100644
 --- a/hw/isa/lpc_ich9.c
 +++ b/hw/isa/lpc_ich9.c
 @@ -437,7 +437,15 @@ static void ich9_apm_ctrl_changed(uint32_t val, 
 void *arg)
  
  /* SMI_EN = PMBASE + 30. SMI control and enable register */
  if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) {
 -cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
 +if (lpc->smi_negotiated_features &
 +(UINT64_C(1) << ICH9_LPC_SMI_F_BROADCAST_BIT)) {
 +CPUState *cs;
 +CPU_FOREACH(cs) {
 +   

[Qemu-devel] [Bug 1246890] Re: AC97 sound card crashes QEMU

[Thomas Huth]

** Changed in: qemu
   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/1246890

Title:
  AC97 sound card crashes QEMU

Status in QEMU:
  Fix Released

Bug description:
  The AC97 sound card does not work. It stops QEMU on startup. The cause
  appears to be some kind of deadlock.

  Steps to reproduce:
  Just add -soundhw ac97 to QEMU's arguments. Example: qemu-system-ppc -soundhw 
ac97

  The example above is all it takes to reproduce the problem.

  This problem has been observed on Mac OS X and Debian Linux.

  I question whether the ac97 support ever worked. It is a file that was
  taken from VirtualBox and added to QEMU. I do know that VirtualBox's
  support for the ac97 sound card works perfectly.

  The exact line of code that stops QEMU in its tracks is located in the
  file main-loop.c, in the function os_host_main_loop_wait(), the call
  made to qemu_mutex_lock_iothread(). The is where QEMU stops under Mac
  OS X.

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

[Qemu-devel] [PATCH 1/3] audio: Move arch_init audio code to hw/audio/soundhw.c

[Eduardo Habkost]

There's no reason to keep the soundhw table in arch_init.c. Move
that code to a new hw/audio/soundhw.c file.

While moving the code, trivial coding style issues were fixed.

Signed-off-by: Eduardo Habkost 
---
 include/hw/audio/audio.h   |   3 +
 include/sysemu/arch_init.h |   2 -
 arch_init.c| 124 ---
 hw/audio/soundhw.c | 156 +
 vl.c   |   1 +
 hw/audio/Makefile.objs |   2 +
 6 files changed, 162 insertions(+), 126 deletions(-)
 create mode 100644 hw/audio/soundhw.c

diff --git a/include/hw/audio/audio.h b/include/hw/audio/audio.h
index 55d40f71bf..259bb2cf96 100644
--- a/include/hw/audio/audio.h
+++ b/include/hw/audio/audio.h
@@ -7,4 +7,7 @@ void isa_register_soundhw(const char *name, const char *descr,
 void pci_register_soundhw(const char *name, const char *descr,
   int (*init_pci)(PCIBus *bus));
 
+void audio_init(void);
+void select_soundhw(const char *optarg);
+
 #endif
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 1c9dad1b72..608c49fcdf 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -27,10 +27,8 @@ enum {
 
 extern const uint32_t arch_type;
 
-void select_soundhw(const char *optarg);
 void do_acpitable_option(const QemuOpts *opts);
 void do_smbios_option(QemuOpts *opts);
-void audio_init(void);
 int kvm_available(void);
 int xen_available(void);
 
diff --git a/arch_init.c b/arch_init.c
index 5cc58b2c35..16465c3f54 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -111,130 +111,6 @@ int qemu_read_default_config_files(bool userconfig)
 return 0;
 }
 
-struct soundhw {
-const char *name;
-const char *descr;
-int enabled;
-int isa;
-union {
-int (*init_isa) (ISABus *bus);
-int (*init_pci) (PCIBus *bus);
-} init;
-};
-
-static struct soundhw soundhw[9];
-static int soundhw_count;
-
-void isa_register_soundhw(const char *name, const char *descr,
-  int (*init_isa)(ISABus *bus))
-{
-assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
-soundhw[soundhw_count].name = name;
-soundhw[soundhw_count].descr = descr;
-soundhw[soundhw_count].isa = 1;
-soundhw[soundhw_count].init.init_isa = init_isa;
-soundhw_count++;
-}
-
-void pci_register_soundhw(const char *name, const char *descr,
-  int (*init_pci)(PCIBus *bus))
-{
-assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
-soundhw[soundhw_count].name = name;
-soundhw[soundhw_count].descr = descr;
-soundhw[soundhw_count].isa = 0;
-soundhw[soundhw_count].init.init_pci = init_pci;
-soundhw_count++;
-}
-
-void select_soundhw(const char *optarg)
-{
-struct soundhw *c;
-
-if (is_help_option(optarg)) {
-show_valid_cards:
-
-if (soundhw_count) {
- printf("Valid sound card names (comma separated):\n");
- for (c = soundhw; c->name; ++c) {
- printf ("%-11s %s\n", c->name, c->descr);
- }
- printf("\n-soundhw all will enable all of the above\n");
-} else {
- printf("Machine has no user-selectable audio hardware "
-"(it may or may not have always-present audio 
hardware).\n");
-}
-exit(!is_help_option(optarg));
-}
-else {
-size_t l;
-const char *p;
-char *e;
-int bad_card = 0;
-
-if (!strcmp(optarg, "all")) {
-for (c = soundhw; c->name; ++c) {
-c->enabled = 1;
-}
-return;
-}
-
-p = optarg;
-while (*p) {
-e = strchr(p, ',');
-l = !e ? strlen(p) : (size_t) (e - p);
-
-for (c = soundhw; c->name; ++c) {
-if (!strncmp(c->name, p, l) && !c->name[l]) {
-c->enabled = 1;
-break;
-}
-}
-
-if (!c->name) {
-if (l > 80) {
-error_report("Unknown sound card name (too big to show)");
-}
-else {
-error_report("Unknown sound card name `%.*s'",
- (int) l, p);
-}
-bad_card = 1;
-}
-p += l + (e != NULL);
-}
-
-if (bad_card) {
-goto show_valid_cards;
-}
-}
-}
-
-void audio_init(void)
-{
-struct soundhw *c;
-ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, 
NULL);
-PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, 
NULL);
-
-for (c = soundhw; c->name; ++c) {
-if (c->enabled) {
-if (c->isa) {
-if (!isa_bus) {
-error_report("ISA bus not available for %s", c->name);
-exit(1);
-}
-