Re: [Qemu-devel] [PATCH v3 0/2] virtio-iommu: VFIO integration

[Bharat Bhushan]

Hi Eric,

> -Original Message-
> From: Auger Eric [mailto:eric.au...@redhat.com]
> Sent: Wednesday, August 23, 2017 10:12 PM
> To: Bharat Bhushan ;
> eric.auger@gmail.com; peter.mayd...@linaro.org;
> alex.william...@redhat.com; m...@redhat.com; qemu-...@nongnu.org;
> qemu-devel@nongnu.org
> Cc: w...@redhat.com; kevin.t...@intel.com; marc.zyng...@arm.com;
> t...@semihalf.com; will.dea...@arm.com; drjo...@redhat.com;
> robin.mur...@arm.com; christoffer.d...@linaro.org
> Subject: Re: [PATCH v3 0/2] virtio-iommu: VFIO integration
> 
> Hi Bharat,
> 
> On 21/08/2017 12:48, Bharat Bhushan wrote:
> > This V3 version is mainly about rebasing on v3 version on Virtio-iommu
> > device framework from Eric Augur and addresing review comments.
> s/Augur/Auger ;-)
I am sorry,

> >
> > This patch series allows PCI pass-through using virtio-iommu.
> >
> > This series is based on:
> >  - virtio-iommu specification written by Jean-Philippe Brucker
> >[RFC 0/3] virtio-iommu: a paravirtualized IOMMU,
> >
> >  - virtio-iommu driver by Jean-Philippe Brucker
> >[RFC PATCH linux] iommu: Add virtio-iommu driver
> >
> >  - virtio-iommu device emulation by Eric Augur.
> >[RFC v3 0/8] VIRTIO-IOMMU device
> >
> > PCI device pass-through and virtio-net-pci is tested with these
> > changes using dma-ops
> 
> I confirm it works fine now with 2 assigned VFs.
> 
> However at the moment DPDK testpmd using those 2 VFs does not work for
> me:
> 1:
> [/home/augere/UPSTREAM/dpdk/install/bin/testpmd(rte_dump_stack+0x2
> 4)
> [0x4a8a78]]
> 
> I haven't investigated yet...

I have not run DPDK before, I am compiling right now and run.

Thanks
-Bharat

> 
> Thanks
> 
> Eric
> >
> > This patch series does not implement RESV_MEM changes proposal by
> Jean-Philippe "https://lists.gnu.org/archive/html/qemu-devel/2017-
> 07/msg01796.html"
> >
> > v2->v3:
> >  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >Which is based on top of v2.10-rc0 that
> >  - Fixed issue with two PCI devices
> >  - Addressed review comments
> >
> > v1->v2:
> >   - Added trace events
> >   - removed vSMMU3 link in patch description
> >
> > Bharat Bhushan (2):
> >   target/arm/kvm: Translate the MSI doorbell in
> kvm_arch_fixup_msi_route
> >   virtio-iommu: vfio integration with virtio-iommu
> >
> >  hw/virtio/trace-events   |   5 ++
> >  hw/virtio/virtio-iommu.c | 163
> +++
> >  include/hw/virtio/virtio-iommu.h |   6 ++
> >  target/arm/kvm.c |  27 +++
> >  target/arm/trace-events  |   3 +
> >  5 files changed, 204 insertions(+)
> >

Re: [Qemu-devel] [PATCH for-2.11] hw/ide/microdrive: Mark the dscm1xxxx device with user_creatable = false

[Paolo Bonzini]

On 24/08/2017 05:03, Thomas Huth wrote:
> QEMU currently aborts with an assertion message when the user is trying
> to remove a dscm1 again:
> 
> $ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
> QEMU 2.9.93 monitor - type 'help' for more information
> (qemu) device_add dscm1,id=xyz
> (qemu) device_del xyz
> **
> ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
> Aborted (core dumped)
> 
> Looks like this device has to be wired up in code and is not meant
> to be hot-pluggable, so let's mark it with user_creatable = false.

The hotpluggable flag should be just a hint from the device, independent
of any knowledge of the board's behavior.  So a better question is why
qbus_is_hotpluggable was true at device_add time (I suppose
qdev_get_hotplug_handler was NULL at device_add time).

BTW, maybe we can get rid of qbus_is_hotpluggable by doing something
like this:

1) This code in device_set_realized:

hotplug_ctrl = qdev_get_hotplug_handler(dev);
if (hotplug_ctrl) {
hotplug_handler_pre_plug(hotplug_ctrl, dev, _err);
if (local_err != NULL) {
goto fail;
}
}

can fail with an error (QERR_DEVICE_NO_HOTPLUG is already there) if
dev->hotplugged && !hotplug_ctrl.

2) In device_add, this code is now superfluous:

if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) {
error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
return NULL;
}

3) in device_get_hotpluggable, the right side of the && can be replaced
with "qdev_get_hotplug_handler(dev) != NULL", getting rid of
qbus_is_hotpluggable.

Paolo

Re: [Qemu-devel] make check speed

[Thomas Huth]

On 23.08.2017 14:20, Cornelia Huck wrote:
> On Wed, 23 Aug 2017 10:35:43 +0200
> Thomas Huth  wrote:
> 
>> True. And I just learned that you can also already set the SPEED
>> variable to either "quick" or "slow" and that we're already using
>> g_test_quick() and g_test_slow() in a couple of places to check this. So
>> the framework for running quick vs. thorough tests is already there ...
>> we just might want to add this to some more tests, I guess...
>>
>> Question for the maintainers and the test automation folks: Is anybody
>> already running "make check SPEED=slow" or is this just rather an
>> unheard-of way of running the tests?
> 
> So I tried this on master just for fun, and 'make V=1 SPEED=slow
> check-qtest-x86_64' promptly failed for some ivshmem test.
> 
> On x86_86:
> TEST: tests/ivshmem-test... (pid=3672)
>   /x86_64/ivshmem/single:  OK
>   /x86_64/ivshmem/hotplug: OK
>   /x86_64/ivshmem/memdev:  OK
>   /x86_64/ivshmem/pair:OK
>   /x86_64/ivshmem/server-msi:  **
> ERROR:/home/cohuck/git/qemu/tests/ivshmem-test.c:367:test_ivshmem_server: 
> assertion failed (ret == 0): (1 == 0)
> FAIL
> GTester: last random seed: R02Scde8fd6835fdf17450c73e2f74f25007
> (pid=3697)
>  /x86_64/ivshmem/server-irq:  OK
> FAIL: tests/ivshmem-test
> 
> On s390x:
> TEST: tests/ivshmem-test... (pid=63617)
>   /x86_64/ivshmem/single:  OK
>   /x86_64/ivshmem/hotplug: OK
>   /x86_64/ivshmem/memdev:  OK
>   /x86_64/ivshmem/pair:OK
>   /x86_64/ivshmem/server-msi:  
> qemu-system-x86_64: -device ivshmem-doorbell,chardev=chr0,vectors=2: server 
> sent invalid ID message
> Broken pipe
> FAIL
> GTester: last random seed: R02Sda000f7be5ce27b3dfbb03d12f297b69
> (pid=63640)
>   /x86_64/ivshmem/server-irq:  
> qemu-system-x86_64: -device ivshmem,size=1M,msi=off,chardev=chr0,vectors=2: 
> server sent invalid ID message
> Broken pipe
> FAIL
> GTester: last random seed: R02S5a236dbcac35545cc34c0131fbc06162
> (pid=63648)
> FAIL: tests/ivshmem-test
> 
> Both machines are on Fedora 26.

The ivshmem test fails for me with SPEED=slow, too (on a x86 RHEL7
machine). Looks like it is definitely broken. Could anybody with some
ivshmem knowledge please have a look at this?

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH 0/4] four zpci patches

[Yi Min Zhao]

Why can't I receive [Qemu-devel] prefixed patches?


在 2017/8/23 下午3:26, Yi Min Zhao 写道:

This patch set contains four small zpci patches to fixup different issues.
1) fixup calculation of msix boundary
2) remove zpci idx from msix message, instead we could use PCIDevice's id to
find zpci device in kvm_arch_fixup_msi_route()
3) fixup ind_offset calculation for adapter interrupt routing entry
4) introduce our own iommu_replay callback

Yi Min Zhao (4):
   s390x/pci: fixup trap_msix()
   s390x/pci: remove idx from msix msg data
   s390x/pci: fixup ind_offset of msix routing entry
   s390x/pci: add iommu replay callback

  hw/s390x/s390-pci-bus.c  | 24 +---
  hw/s390x/s390-pci-bus.h  |  2 ++
  hw/s390x/s390-pci-inst.c | 28 ++--
  target/s390x/kvm.c   | 11 ++-
  4 files changed, 23 insertions(+), 42 deletions(-)

[Qemu-devel] [PATCH for-2.11] tests/test-hmp: Remove puv3 and tricore_testboard from the blacklist

[Thomas Huth]

The problem with puv3 has been fixed with 0ac241bcf9f9d99a252a352a162f
('unicore32: abort when entering "x 0" on the monitor') and the problem
with tricore_testboard has been fixed with b190f477e29c7cd03a8fee49c96d
('qemu-system-tricore: segfault when entering "x 0" on the monitor').

Signed-off-by: Thomas Huth 
---
 tests/test-hmp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index 4382df0..9f286bf 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -192,8 +192,7 @@ static void add_machine_test_case(const char *mname)
 char *path;
 
 /* Ignore blacklisted machines that have known problems */
-if (!strcmp("puv3", mname) || !strcmp("tricore_testboard", mname) ||
-!strcmp("xenfv", mname) || !strcmp("xenpv", mname)) {
+if (!strcmp("xenfv", mname) || !strcmp("xenpv", mname)) {
 return;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[David Gibson]

On Thu, Aug 24, 2017 at 02:15:01PM +1000, Paul Mackerras wrote:
> On Thu, Aug 24, 2017 at 02:02:43PM +1000, David Gibson wrote:
> > On Thu, Aug 24, 2017 at 12:54:48PM +1000, Paul Mackerras wrote:
> > > On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> > > > LoPAPR says:
> > > > 
> > > > “ibm,processor-storage-keys”
> > > > 
> > > > property name indicating the number of virtual storage keys 
> > > > supported
> > > > by the processor described by this node.
> > > > 
> > > > prop-encoded-array: Consists of two cells encoded as with 
> > > > encode-int.
> > > > The first cell represents the number of virtual storage keys 
> > > > supported
> > > > for data accesses while the second cell represents the number of
> > > > virtual storage keys supported for instruction accesses. The cell 
> > > > value
> > > > of zero indicates that no storage keys are supported for the access
> > > > type.
> > > > 
> > > > pHyp provides the property above but there's a bug in P8 firmware where 
> > > > the
> > > > second cell is zero even though POWER8 supports instruction access keys.
> > > > This bug will be fixed for P9.
> > > > 
> > > > Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 
> > > > machine.
> > > > 
> > > > Signed-off-by: Thiago Jung Bauermann 
> > > > ---
> > > > 
> > > > The sysfs files are provided by this patch for Linux:
> > > 
> > > Those sysfs files relate to the kernel's support for userspace
> > > processes using storage keys.  That is quite distinct from KVM's
> > > support for guests using storage keys, so I think that using those
> > > sysfs files to indicate what the guest can do is wrong.
> > 
> > Ah!  Glad someone pointed that out.
> > 
> > > In fact KVM allows guests to specify storage keys in the HPTE values
> > > that they set, except that there is a bug (for which Ram Pai has
> > > posted a patch) that means that KVM loses the top two bits of the key
> > > number.
> > > 
> > > What I would suggest is that we use the 'pad' field in the struct
> > > kvm_ppc_smmu_info to report the number of keys supported by KVM for
> > > guest use.
> > 
> > Is there a particular reason to put it there rather than a new KVM 
> > capability?
> 
> Just that storage keys are an aspect of the "server" (i.e. HPT) MMU,
> so it seemed to make sense to put it together with the other
> information about the HPT MMU (segment sizes, page sizes, etc.).

Ok, works for me.

> 
> > > That will be 0 in all current kernels, indicating that
> > > keys are not supported, which is reasonable because of the bug.  I
> > > will make sure the patch fixing the bug goes in first.
> > 
> > Note that the same migration concerns still apply, so we'll still want
> > machine properties to control this in qemu, which are validated
> > against what the host can do.  Since current kernels are buggy, I
> > suggest we have these properties default to 0 - i.e. you need to
> > explicitly request storage keys on the command line if you want your
> > guest to use them.  Once fixed kernels are rolled out we can look at
> > changing that in a future machine type.
> > 
> > > We could either have two u16 fields for the number of keys for data
> > > and instruction, or we could have a u32 field for the number of keys
> > > and a separate bit in the flags field to indicate that instruction
> > > keys are supported.  Which would be preferable?
> > 
> > I'd prefer the separate numbers for I and D.  It's more flexible and
> > no harder to implement AFAICT.
> 
> OK.
> 
> 
> Paul.
> 

-- 
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] spapr: Add ibm, processor-storage-keys property to CPU DT node

[Paul Mackerras]

On Thu, Aug 24, 2017 at 02:02:43PM +1000, David Gibson wrote:
> On Thu, Aug 24, 2017 at 12:54:48PM +1000, Paul Mackerras wrote:
> > On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> > > LoPAPR says:
> > > 
> > > “ibm,processor-storage-keys”
> > > 
> > > property name indicating the number of virtual storage keys supported
> > > by the processor described by this node.
> > > 
> > > prop-encoded-array: Consists of two cells encoded as with encode-int.
> > > The first cell represents the number of virtual storage keys supported
> > > for data accesses while the second cell represents the number of
> > > virtual storage keys supported for instruction accesses. The cell 
> > > value
> > > of zero indicates that no storage keys are supported for the access
> > > type.
> > > 
> > > pHyp provides the property above but there's a bug in P8 firmware where 
> > > the
> > > second cell is zero even though POWER8 supports instruction access keys.
> > > This bug will be fixed for P9.
> > > 
> > > Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
> > > 
> > > Signed-off-by: Thiago Jung Bauermann 
> > > ---
> > > 
> > > The sysfs files are provided by this patch for Linux:
> > 
> > Those sysfs files relate to the kernel's support for userspace
> > processes using storage keys.  That is quite distinct from KVM's
> > support for guests using storage keys, so I think that using those
> > sysfs files to indicate what the guest can do is wrong.
> 
> Ah!  Glad someone pointed that out.
> 
> > In fact KVM allows guests to specify storage keys in the HPTE values
> > that they set, except that there is a bug (for which Ram Pai has
> > posted a patch) that means that KVM loses the top two bits of the key
> > number.
> > 
> > What I would suggest is that we use the 'pad' field in the struct
> > kvm_ppc_smmu_info to report the number of keys supported by KVM for
> > guest use.
> 
> Is there a particular reason to put it there rather than a new KVM capability?

Just that storage keys are an aspect of the "server" (i.e. HPT) MMU,
so it seemed to make sense to put it together with the other
information about the HPT MMU (segment sizes, page sizes, etc.).

> > That will be 0 in all current kernels, indicating that
> > keys are not supported, which is reasonable because of the bug.  I
> > will make sure the patch fixing the bug goes in first.
> 
> Note that the same migration concerns still apply, so we'll still want
> machine properties to control this in qemu, which are validated
> against what the host can do.  Since current kernels are buggy, I
> suggest we have these properties default to 0 - i.e. you need to
> explicitly request storage keys on the command line if you want your
> guest to use them.  Once fixed kernels are rolled out we can look at
> changing that in a future machine type.
> 
> > We could either have two u16 fields for the number of keys for data
> > and instruction, or we could have a u32 field for the number of keys
> > and a separate bit in the flags field to indicate that instruction
> > keys are supported.  Which would be preferable?
> 
> I'd prefer the separate numbers for I and D.  It's more flexible and
> no harder to implement AFAICT.

OK.

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

Paul.

Re: [Qemu-devel] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[David Gibson]

On Thu, Aug 24, 2017 at 12:54:48PM +1000, Paul Mackerras wrote:
> On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> > LoPAPR says:
> > 
> > “ibm,processor-storage-keys”
> > 
> > property name indicating the number of virtual storage keys supported
> > by the processor described by this node.
> > 
> > prop-encoded-array: Consists of two cells encoded as with encode-int.
> > The first cell represents the number of virtual storage keys supported
> > for data accesses while the second cell represents the number of
> > virtual storage keys supported for instruction accesses. The cell value
> > of zero indicates that no storage keys are supported for the access
> > type.
> > 
> > pHyp provides the property above but there's a bug in P8 firmware where the
> > second cell is zero even though POWER8 supports instruction access keys.
> > This bug will be fixed for P9.
> > 
> > Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
> > 
> > Signed-off-by: Thiago Jung Bauermann 
> > ---
> > 
> > The sysfs files are provided by this patch for Linux:
> 
> Those sysfs files relate to the kernel's support for userspace
> processes using storage keys.  That is quite distinct from KVM's
> support for guests using storage keys, so I think that using those
> sysfs files to indicate what the guest can do is wrong.

Ah!  Glad someone pointed that out.

> In fact KVM allows guests to specify storage keys in the HPTE values
> that they set, except that there is a bug (for which Ram Pai has
> posted a patch) that means that KVM loses the top two bits of the key
> number.
> 
> What I would suggest is that we use the 'pad' field in the struct
> kvm_ppc_smmu_info to report the number of keys supported by KVM for
> guest use.

Is there a particular reason to put it there rather than a new KVM capability?

> That will be 0 in all current kernels, indicating that
> keys are not supported, which is reasonable because of the bug.  I
> will make sure the patch fixing the bug goes in first.

Note that the same migration concerns still apply, so we'll still want
machine properties to control this in qemu, which are validated
against what the host can do.  Since current kernels are buggy, I
suggest we have these properties default to 0 - i.e. you need to
explicitly request storage keys on the command line if you want your
guest to use them.  Once fixed kernels are rolled out we can look at
changing that in a future machine type.

> We could either have two u16 fields for the number of keys for data
> and instruction, or we could have a u32 field for the number of keys
> and a separate bit in the flags field to indicate that instruction
> keys are supported.  Which would be preferable?

I'd prefer the separate numbers for I and D.  It's more flexible and
no harder to implement AFAICT.

-- 
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 v2 for-2.11] hw/ppc/spapr_cpu_core: Add a proper check for spapr machine

[David Gibson]

On Thu, Aug 24, 2017 at 05:52:32AM +0200, Thomas Huth wrote:
> QEMU currently crashes when the user tries to add a spapr-cpu-core
> on a non-pseries machine:
> 
> $ qemu-system-ppc64 -S -machine ppce500,accel=tcg \
> -device POWER5+_v2.1-spapr-cpu-core
> hw/ppc/spapr_cpu_core.c:178:spapr_cpu_core_realize_child:
> Object 0x55cee1f55160 is not an instance of type spapr-machine
> Aborted (core dumped)
> 
> So let's add a proper check for the correct machine time with
> a more friendly error message here.
> 
> Reported-by: Eduardo Habkost 
> Signed-off-by: Thomas Huth 

Applied to ppc-for-2.11, thanks.

> ---
>  v2:
>  - Add the check to spapr_cpu_core_realize() instead of adding it to
>spapr_cpu_core_realize_child()
>  - Use device/machine whitelist for the device-crash-test instead of
>adding a log entry there
> 
>  hw/ppc/spapr_cpu_core.c   | 5 +
>  scripts/device-crash-test | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ea278ce..dd92326 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -220,6 +220,11 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  void *obj;
>  int i, j;
>  
> +if (!object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE)) {
> +error_setg(errp, "spapr-cpu-core needs a pseries machine");
> +return;
> +}
> +
>  sc->threads = g_malloc0(size * cc->nr_threads);
>  for (i = 0; i < cc->nr_threads; i++) {
>  char id[32];
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index 81d65b9..043b24a 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -160,6 +160,9 @@ ERROR_WHITELIST = [
>  {'machine':'q35|pc.*', 'device':'kvm-ioapic', 'expected':True}, # Only 1 
> ioapics allowed
>  {'machine':'q35|pc.*', 'device':'ioapic', 'expected':True}, # Only 1 
> ioapics allowed
>  
> +# "spapr-cpu-core needs a pseries machine"
> +{'machine':'(?!pseries).*', 'device':'.*-spapr-cpu-core', 
> 'expected':True},
> +
>  # KVM-specific devices shouldn't be tried without accel=kvm:
>  {'accel':'(?!kvm).*', 'device':'kvmclock', 'expected':True},
>  {'accel':'(?!kvm).*', 'device':'kvm-pci-assign', 'expected':True},

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

[Qemu-devel] [PATCH v2 for-2.11] hw/ppc/spapr_cpu_core: Add a proper check for spapr machine

[Thomas Huth]

QEMU currently crashes when the user tries to add a spapr-cpu-core
on a non-pseries machine:

$ qemu-system-ppc64 -S -machine ppce500,accel=tcg \
-device POWER5+_v2.1-spapr-cpu-core
hw/ppc/spapr_cpu_core.c:178:spapr_cpu_core_realize_child:
Object 0x55cee1f55160 is not an instance of type spapr-machine
Aborted (core dumped)

So let's add a proper check for the correct machine time with
a more friendly error message here.

Reported-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 v2:
 - Add the check to spapr_cpu_core_realize() instead of adding it to
   spapr_cpu_core_realize_child()
 - Use device/machine whitelist for the device-crash-test instead of
   adding a log entry there

 hw/ppc/spapr_cpu_core.c   | 5 +
 scripts/device-crash-test | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ea278ce..dd92326 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -220,6 +220,11 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error 
**errp)
 void *obj;
 int i, j;
 
+if (!object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE)) {
+error_setg(errp, "spapr-cpu-core needs a pseries machine");
+return;
+}
+
 sc->threads = g_malloc0(size * cc->nr_threads);
 for (i = 0; i < cc->nr_threads; i++) {
 char id[32];
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 81d65b9..043b24a 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -160,6 +160,9 @@ ERROR_WHITELIST = [
 {'machine':'q35|pc.*', 'device':'kvm-ioapic', 'expected':True}, # Only 1 
ioapics allowed
 {'machine':'q35|pc.*', 'device':'ioapic', 'expected':True}, # Only 1 
ioapics allowed
 
+# "spapr-cpu-core needs a pseries machine"
+{'machine':'(?!pseries).*', 'device':'.*-spapr-cpu-core', 'expected':True},
+
 # KVM-specific devices shouldn't be tried without accel=kvm:
 {'accel':'(?!kvm).*', 'device':'kvmclock', 'expected':True},
 {'accel':'(?!kvm).*', 'device':'kvm-pci-assign', 'expected':True},
-- 
1.8.3.1

Re: [Qemu-devel] make check speed

[David Gibson]

On Wed, Aug 23, 2017 at 11:33:55AM +0200, Paolo Bonzini wrote:
> On 23/08/2017 10:35, Thomas Huth wrote:
> >> If you have a many-core machine, of course, there's no simpler solution
> >> than throwing more CPUs at it. :)
> > Is it safe nowadays to run "make check -j4" for example? Last time I
> > tried (maybe 1 or 2 years ago), there were still issues since some tests
> > were using hard-coded temporary file names, so the parallel tests were
> > disturbing each other, for example...
> 
> I run -j18 all the time. :)

Nice for those that have 18-way machines at their regular disposal...

-- 
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] [Bug 1711828] Re: lock mov non generated #UD

[li]

sorry i english poor：


intel manual say：
The LOCK prefix can be prepended only to the following instructions and only to 
those forms of the instructions
where the destination operand is a memory operand: ADD, ADC, AND, BTC, BTR, 
BTS, CMPXCHG, CMPXCH8B,
CMPXCHG16B, DEC, INC, NEG, NOT, OR, SBB, SUB, XOR, XADD, and XCHG. If the LOCK 
prefix is used with one of
these instructions and the source operand is a memory operand, an undefined 
opcode exception (#UD) may be
generated. An undefined opcode exception will also be generated if the LOCK 
prefix is used with any instruction not
in the above list. The XCHG instruction always asserts the LOCK# signal 
regardless of the presence or absence of
the LOCK prefix.


but qemu NO!
lock mov   forms of the instructions non generated #UD exception!
my OS : debian 9.1 
QEMU: qemu 2.8.1




At 2017-08-21 12:54:44, "Thomas Huth" <1711...@bugs.launchpad.net> wrote:
>Could you please add a proper description to this bug? Please also try
>first whether your problem also occurs with the latest released version
>of QEMU (version 2.9 or the 2.10 release candidate), to see whether it
>has been fixed there already.
>
>** Changed in: qemu
>   Status: New => Incomplete
>
>-- 
>You received this bug notification because you are subscribed to the bug
>report.
>https://bugs.launchpad.net/bugs/1711828
>
>Title:
>  lock mov non generated #UD
>
>Status in QEMU:
>  Incomplete
>
>Bug description:
>  qemu 2.8.1 debian 9.1
>
>To manage notifications about this bug go to:
>https://bugs.launchpad.net/qemu/+bug/1711828/+subscriptions

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

Title:
  lock mov non generated #UD

Status in QEMU:
  Incomplete

Bug description:
  qemu 2.8.1 debian 9.1

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

[Qemu-devel] anyone seen or heard of large delays/stalls running qemu with kvm support?

[Chris Friesen]

Hi all,

I need to apologize up front, this is going to be a long email.  Basically the 
executive summary is that we're seeing issues where a VM is apparently not 
making forward progress in the guest, while at the same time spinning in a busy 
loop on the host.  I'm looking for any pointers/suggestions on what might be 
going on or how to narrow it down.


We're using qemu-kvm-ev-2.6.0 in the context of an OpenStack compute node 
running CentOS 7.


While running a test involving a cold migration of a VM from one host to another 
(basically shutting down the guest, moving all the files over to another host, 
then starting the guest back up) we have on a couple of cases had a VM get into 
a state where it was displaying "Guest has not initialized the display (yet)." 
on the console.  Running strace or gdb on the host against the guest qemu 
process seems to allow it to make forward progress.


Here's what I've got:

1) The root disk seems to be fine.  The root disk for the VM is iSCSI, and is 
accessible from the host.  Starting up a new VM with the same root disk and 
mostly the same commandline works fine.  (The serial port of the new VM was 
directed to stdio and the vnc option was removed.)


2) Investigating the apparently-hung instance it seems 2 of its 3 vCPUs are 
running a busy-loop:


   PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
192577 root 20 0 3816972 38360 10916 R 99.9 0.0 75:15.17 CPU 1/KVM
192578 root -2 0 3816972 38360 10916 R 99.9 0.0 75:15.36 CPU 2/KVM

(Each of the vCPU threads is affined to a separate physical CPU.)


3) Trying to retrieve the cpu info via libvirt didn't return within 10 seconds:

compute-0:/home/wrsroot# start=$SECONDS; timeout 10 virsh qemu-monitor-command 
--hmp instance-006e info cpus || echo "timeout after $((SECONDS - start)) 
seconds"

timeout after 10 seconds


4) Running gdb on 192577 shows the following backtrace:
Thread 1 (process 192577):
#0 0x7f8c596e4537 in ioctl () from /lib64/libc.so.6
#1 0x7f8c70fe0234 in kvm_vcpu_ioctl ()
#2 0x7f8c70fe02ed in kvm_cpu_exec ()
#3 0x7f8c70fcee76 in qemu_kvm_cpu_thread_fn ()
#4 0x7f8c599bedc5 in start_thread () from /lib64/libpthread.so.0
#5 0x7f8c596ed76d in clone () from /lib64/libc.so.6

5) Running gdb on 192578 gives the same backtrace.

6) Strace shows most of the time spent in ioctl...with 39K syscalls in 30 
seconds across those two PIDs:


compute-0:/home/wrsroot# timeout 30 strace -c -q -T -p 192577 -p 192578
% time seconds usecs/call calls errors syscall
-- --- --- - - 
 99.86 0.297753 9 32325 ioctl
  0.12 0.000354 0 6730 writev
  0.02 0.59 0 384 60 futex
  0.00 0.00 0 10 write
  0.00 0.00 0 2 rt_sigpending
  0.00 0.00 0 2 2 rt_sigtimedwait
-- --- --- - - 
100.00 0.298166 39453 62 total


7) Running strace is enough to allow the VM to make forward progress, here are 
kernel logs from within the guest:


[ 2540.964492] INFO: rcu_preempt self-detected stall on CPU { 0} (t=2202996 
jiffies g=-108 c=-109 q=1676)

[ 2540.984656] Task dump for CPU 0:
[ 2540.985911] systemd R running task 0 1 0 0x0008
[ 2540.988028] 88003b66 84d8f1f4 88003e003d90 
810a8676
[ 2540.990869]  81a44d40 88003e003da8 
810abed9
[ 2540.993975] 0001 88003e003dd8 8110e1a0 
88003e00ec40
[ 2540.996815] Call Trace:
[ 2540.997995]  [] sched_show_task+0xb6/0x120
[ 2540.58] [] dump_cpu_task+0x39/0x70
[ 2541.001559] [] rcu_dump_cpu_stacks+0x90/0xd0
[ 2541.002425] [] rcu_check_callbacks+0x434/0x7f0
[ 2541.003154] [] ? tick_sched_handle.isra.13+0x60/0x60
[ 2541.003981] [] update_process_times+0x47/0x80
[ 2541.004755] [] tick_sched_handle.isra.13+0x25/0x60
[ 2541.005528] [] tick_sched_timer+0x41/0x70
[ 2541.006225] [] __hrtimer_run_queues+0xfc/0x320
[ 2541.006976] [] hrtimer_interrupt+0xb0/0x1e0
[ 2541.007751] [] local_apic_timer_interrupt+0x37/0x60
[ 2541.008541] [] smp_apic_timer_interrupt+0x3f/0x60
[ 2541.009290] [] apic_timer_interrupt+0x6d/0x80
[ 2541.009996]  [] ? get_monotonic_boottime+0xb9/0x100
[ 2541.010976] [] posix_get_boottime+0x11/0x20
[ 2541.011740] [] SyS_clock_gettime+0x54/0xc0
[ 2541.012437] [] system_call_fastpath+0x16/0x1b

[ 2602.140152] NMI watchdog: BUG: soft lockup - CPU#2 stuck for 22s! 
[systemd-udevd:232]

[ 2602.140438] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 23s! 
[ksoftirqd/0:3]
[[ 2602.140455] Modules linked in: ata_piix(+) serio_raw virtio_blk(+) 
virtio_net floppy(+) libata sunrpc

[32m OK [0m] Started udev Coldplug all Devices.
[ 2602.140457] CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 
3.10.0-514.16.1.el7.24.tis.x86_64 #1
[ 2602.140458] Hardware name: Fedora Project OpenStack Nova, BIOS 
1.9.1-5.el7_3.2.tis.1 04/01/2014

[ 2602.140472] task: 88003b661620 ti: 88003b6a task.ti: 
88003b6a
[ 2602.140478] RIP: 0010:[] [] 

[Qemu-devel] [PATCH for-2.11] hw/ide/microdrive: Mark the dscm1xxxx device with user_creatable = false

[Thomas Huth]

QEMU currently aborts with an assertion message when the user is trying
to remove a dscm1 again:

$ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
QEMU 2.9.93 monitor - type 'help' for more information
(qemu) device_add dscm1,id=xyz
(qemu) device_del xyz
**
ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
Aborted (core dumped)

Looks like this device has to be wired up in code and is not meant
to be hot-pluggable, so let's mark it with user_creatable = false.

Signed-off-by: Thomas Huth 
---
 hw/ide/microdrive.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index e3fd30e..17917c0 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -575,12 +575,15 @@ PCMCIACardState *dscm1_init(DriveInfo *dinfo)
 static void dscm1_class_init(ObjectClass *oc, void *data)
 {
 PCMCIACardClass *pcc = PCMCIA_CARD_CLASS(oc);
+DeviceClass *dc = DEVICE_CLASS(oc);
 
 pcc->cis = dscm1_cis;
 pcc->cis_len = sizeof(dscm1_cis);
 
 pcc->attach = dscm1_attach;
 pcc->detach = dscm1_detach;
+/* Reason: Needs to be wired-up in code, see dscm1_init() */
+dc->user_creatable = false;
 }
 
 static const TypeInfo dscm1_type_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 15/15] ppc: Add aCube Sam460ex board

[David Gibson]

On Wed, Aug 23, 2017 at 01:12:06PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Aug 2017, David Gibson wrote:
> > On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
> > > Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
> > > This is not a full implementation yet with a lot of components still
> > > missing but enough to start a Linux kernel and the U-Boot firmware.
> > > 
> > > Signed-off-by: François Revol 
> > > Signed-off-by: BALATON Zoltan 
> > 
> > As usual, only fairly superficial review here.
> > 
> > > ---
> > >  default-configs/ppcemb-softmmu.mak |   3 +
> > >  hw/ppc/Makefile.objs   |   2 +
> > >  hw/ppc/sam460ex.c  | 611 
> > > +
> > >  3 files changed, 616 insertions(+)
> > >  create mode 100644 hw/ppc/sam460ex.c
> > > 
> > > diff --git a/default-configs/ppcemb-softmmu.mak 
> > > b/default-configs/ppcemb-softmmu.mak
> > > index 635923a..90b42f0 100644
> > > --- a/default-configs/ppcemb-softmmu.mak
> > > +++ b/default-configs/ppcemb-softmmu.mak
> [...]
> > > +/*/
> > > +/* SPD eeprom content from mips_malta.c */
> > 
> > What's the connection with mips_malta?
> 
> The board's firmware wants to see SPD EEPROMs of the connected memory while
> initialising the memory controller. This is why we need to implement SDRAM
> controller, I2C and SPD EEPROMs. MIPS malta board had already SPD EEPROM
> implementation so this is based on that. The comment just indicates where
> this code comes from.

Ok.

[snip]
> > > +env->nip = bi->entry;
> > > +
> > > +/* Create a mapping for the kernel.  */
> > > +mmubooke_create_initial_mapping(env, 0, 0);
> > > +env->gpr[6] = tswap32(EPAPR_MAGIC);
> > 
> > I'm pretty sure the tswap can't be right here.  env->gpr is in host
> > native order and I'd expect the constant to be as well.
> 
> I know nothing about this, maybe Francois remembers why it's there. But
> booting linux with -kernel works so it's probably either correct or does not
> matter.

Have you attempted it on both BE and LE hosts though?

> 
> > > +env->gpr[7] = (16 << 20) - 8; /*bi->ima_size;*/
> > 
> > So, entering the kernel directly can be useful, particularly during
> > early development.  However, having both firmware and non-firmware
> > entry paths can lead to confusing bugs if there's a subtle difference
> > between the initial (to the kernel) states between the two paths.  For
> > that reason, the usual preferred way to implement -kernel is to still
> > run the usual firmware, but use some way of telling it to boot
> > immediately into the supplied kernel.
> > 
> > I won't object to merging it this way - just a wanrning that this may
> > bite you in the future if you're not careful.
> 
> Warning taken, at this point until firmware cannot reliably boot things
> having another way to test is useful to have. In the future when booting via
> firmware works well we can figure out what to do with this.

Fair enough.

[snip]
> > > +generate_eeprom_spd(_eeprom_buf[i * 256], ram_sizes[i]);
> > > +}
> > > +generate_eeprom_serial(_eeprom_buf[4 * 256]);
> > > +generate_eeprom_serial(_eeprom_buf[6 * 256]);
> > > +
> > > +/* IIC controllers */
> > > +dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> > > +i2c[0] = PPC4xx_I2C(dev);
> > > +object_property_set_bool(OBJECT(dev), true, "realized", NULL);
> > > +smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, 
> > > smbus_eeprom_size);
> > > +g_free(smbus_eeprom_buf);
> > > +
> > > +dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
> > > +i2c[1] = PPC4xx_I2C(dev);
> > > +
> > > +/* External bus controller */
> > > +ppc405_ebc_init(env);
> > > +
> > > +/* CPR */
> > > +ppc4xx_cpr_init(env);
> > > +
> > > +/* PLB to AHB bridge */
> > > +ppc4xx_ahb_init(env);
> > > +
> > > +/* System DCRs */
> > > +ppc4xx_sdr_init(env);
> > > +
> > > +/* MAL */
> > > +ppc4xx_mal_init(env, 4, 16, [2][3]);
> > > +
> > > +/* 256K of L2 cache as memory */
> > > +ppc4xx_l2sram_init(env);
> > 
> > Seems like a lot of this peripheral setup should be handled by a SoC
> > helper function, since it's not really board specific (I'm guessing).
> > I'm ok with that being a later clean up though.
> 
> I know, the ppc405 has an init function and I considered moving some of this
> to a similar function but I've left it here for now for simplicity until
> it's decided what's the best way to clean this up.

Ok, seems reasonable.

> > > +/* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
> > > +memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 
> > > << 10,
> > > +   _abort);
> > > +memory_region_add_subregion(address_space_mem, 0x4LL, 
> > > 

Re: [Qemu-devel] [PATCH 15/15] ppc: Add aCube Sam460ex board

[David Gibson]

On Wed, Aug 23, 2017 at 02:47:42PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Aug 2017, François Revol wrote:
> > Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
> > > > What's the connection with mips_malta?
> > > 
> > > The board's firmware wants to see SPD EEPROMs of the connected memory
> > > while initialising the memory controller. This is why we need to
> > > implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> > > already SPD EEPROM implementation so this is based on that. The comment
> > > just indicates where this code comes from.
> > 
> > Indeed, and I copy-pasted from elsewhere for this.
> > 
> > > > > +fprintf(stderr, "qemu: Error registering flash memory.\n");
> > > > 
> > > > Use error_report() instead, please.
> > 
> > I guess this didn't exist back when I started writing it...
> 
> No problem, I can take care of these.
> 
> > > > > +/* Create reset TLB entries for BookE, mapping only the flash
> > > > > memory.  */
> > > > > +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> > > > > +{
> > > > > +ppcemb_tlb_t *tlb = >tlb.tlbe[0];
> > > > > +
> > > > > +/* on reset the flash is mapped by a shadow TLB,
> > > > > + * but since we don't implement them we need to use
> > > > > + * the same values U-Boot will use to avoid a fault.
> > > > > + */
> > > > 
> > > > Usually the reset state of the MMU is handled in the cpu code rather
> > > > than the board code.  Is there a specific reason you need it in the
> > > > board code here?
> > > 
> > > I'm not sure, probably lack of a better place. The ppc440_bamboo board
> > > this is based on has it the same way in the board code. Maybe this could
> > > be cleaned up when someone wants to QOMify the SoC models sometimes.
> > 
> > Thing is, the code allows both booting with U-Boot and with a kernel
> > directly, and the MMU mapping differ in those cases.
> > 
> > Maybe the CPU reset should use the U-Boot setup and the kernel boot
> > would just overwrite it?
> > 
> > > > > +env->nip = bi->entry;
> > > > > +
> > > > > +/* Create a mapping for the kernel.  */
> > > > > +mmubooke_create_initial_mapping(env, 0, 0);
> > > > > +env->gpr[6] = tswap32(EPAPR_MAGIC);
> > > > 
> > > > I'm pretty sure the tswap can't be right here.  env->gpr is in host
> > > > native order and I'd expect the constant to be as well.
> > > 
> > > I know nothing about this, maybe Francois remembers why it's there. But
> > > booting linux with -kernel works so it's probably either correct or does
> > > not matter.
> > 
> > Absolutely no idea. It seems to be there from the first commit in my own
> > history here.
> > 
> > I don't recall testing booting linux at all though.
> > Linux does check the magic, so it'd be weird if it booted:
> > 
> > https://github.com/torvalds/linux/blob/master/arch/powerpc/boot/epapr.c
> 
> Is this code used on Sam460 at all? Is U-Boot ePAPR compliant
> firmware?

It can be, depends on the build, I think.

> Isn't that only needed on OpenFirmware?

No, not at all.  True OF will generally *not* be ePAPR compliant -
ePAPR was explicitly built as a much simpler interface that doesn't
require an OF implementation.  It uses some concepts from OF,
specifically the contents of the device tree, but other than that it's
not related.

> 
> > But maybe it got added later than the version you tested?
> 
> I've tried some of these:
> http://www.supertuxkart-amiga.de/amiga/sam.html#downloads
> which also have kernel 4.5 so that's fairly recent. These kernels are
> "u-boot legacy uImage" so maybe they don't need ePAPR magic? Are there some
> docs on what the kernel expects on this board or it has to be dug out from
> U-Boot?

If it's a legacy uImage I suspect it's not using ePAPR.

-- 
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 13/15] ppc4xx: Add more PLB registers

[David Gibson]

On Wed, Aug 23, 2017 at 12:16:24PM +0200, BALATON Zoltan wrote:
> On Wed, 23 Aug 2017, David Gibson wrote:
> > On Sun, Aug 20, 2017 at 07:23:05PM +0200, BALATON Zoltan wrote:
> > > These registers are present in 440 SoCs (and maybe in others too) and
> > > U-Boot accesses them when printing register info. We don't emulate
> > > these but add them to avoid crashing when they are read or written.
> > > 
> > > Signed-off-by: BALATON Zoltan 
> > 
> > I'm ok with stub implementation, but I'm a bit uncomfortable with
> > registering these DCRs unconditionally rather than just on the chips
> > that actually implement them.
> 
> Problem is that I don't know which chips have these. I can only try to find
> out from the U-Boot sources where a comment says these are common registers
> for all SoCs (in u-boot/arch/powerpc/include/asm/ppc4xx.h:
> 
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/powerpc/include/asm/ppc4xx.h;h=45ff5dbacd9243e83bb2f6551e2dd64a7e544bf5;hb=e2351d5cf1e97408b4c52bafeaa85e0ca85c920c
> 
> while looking for this I've just noticed that u-boot has removed ppc440
> support just before 2017.07-rc3 so this is the last version that still has
> these files). So if that's true it should be OK for 405 too.

Ok, just to make sure I'm understanding correctly are you saying:

1) You suspect these registers were actually on all versions of the
   device, they just weren't implemented until now.

or

2) The registers are definitely on only some versions of the device,
   but you're not sure which ones

> 
> > > ---
> > >  hw/ppc/ppc405_uc.c | 12 +---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> > > index e621d0a..8e58065 100644
> > > --- a/hw/ppc/ppc405_uc.c
> > > +++ b/hw/ppc/ppc405_uc.c
> > > @@ -105,9 +105,12 @@ ram_addr_t ppc405_set_bootinfo (CPUPPCState *env, 
> > > ppc4xx_bd_info_t *bd,
> > >  
> > > /*/
> > >  /* Peripheral local bus arbitrer */
> > >  enum {
> > > -PLB0_BESR = 0x084,
> > > -PLB0_BEAR = 0x086,
> > > -PLB0_ACR  = 0x087,
> > > +PLB3A0_ACR = 0x077,
> > > +PLB4A0_ACR = 0x081,
> > > +PLB0_BESR  = 0x084,
> > > +PLB0_BEAR  = 0x086,
> > > +PLB0_ACR   = 0x087,
> > > +PLB4A1_ACR = 0x089,
> > >  };
> > > 
> > >  typedef struct ppc4xx_plb_t ppc4xx_plb_t;
> > > @@ -179,9 +182,12 @@ void ppc4xx_plb_init(CPUPPCState *env)
> > >  ppc4xx_plb_t *plb;
> > > 
> > >  plb = g_malloc0(sizeof(ppc4xx_plb_t));
> > > +ppc_dcr_register(env, PLB3A0_ACR, plb, _read_plb, 
> > > _write_plb);
> > > +ppc_dcr_register(env, PLB4A0_ACR, plb, _read_plb, 
> > > _write_plb);
> > >  ppc_dcr_register(env, PLB0_ACR, plb, _read_plb, _write_plb);
> > >  ppc_dcr_register(env, PLB0_BEAR, plb, _read_plb, _write_plb);
> > >  ppc_dcr_register(env, PLB0_BESR, plb, _read_plb, _write_plb);
> > > +ppc_dcr_register(env, PLB4A1_ACR, plb, _read_plb, 
> > > _write_plb);
> > >  qemu_register_reset(ppc4xx_plb_reset, plb);
> > >  }
> > > 
> > 
> > 
> 

-- 
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] spapr: Add ibm, processor-storage-keys property to CPU DT node

[Paul Mackerras]

On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> LoPAPR says:
> 
> “ibm,processor-storage-keys”
> 
> property name indicating the number of virtual storage keys supported
> by the processor described by this node.
> 
> prop-encoded-array: Consists of two cells encoded as with encode-int.
> The first cell represents the number of virtual storage keys supported
> for data accesses while the second cell represents the number of
> virtual storage keys supported for instruction accesses. The cell value
> of zero indicates that no storage keys are supported for the access
> type.
> 
> pHyp provides the property above but there's a bug in P8 firmware where the
> second cell is zero even though POWER8 supports instruction access keys.
> This bug will be fixed for P9.
> 
> Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
> 
> Signed-off-by: Thiago Jung Bauermann 
> ---
> 
> The sysfs files are provided by this patch for Linux:

Those sysfs files relate to the kernel's support for userspace
processes using storage keys.  That is quite distinct from KVM's
support for guests using storage keys, so I think that using those
sysfs files to indicate what the guest can do is wrong.

In fact KVM allows guests to specify storage keys in the HPTE values
that they set, except that there is a bug (for which Ram Pai has
posted a patch) that means that KVM loses the top two bits of the key
number.

What I would suggest is that we use the 'pad' field in the struct
kvm_ppc_smmu_info to report the number of keys supported by KVM for
guest use.  That will be 0 in all current kernels, indicating that
keys are not supported, which is reasonable because of the bug.  I
will make sure the patch fixing the bug goes in first.

We could either have two u16 fields for the number of keys for data
and instruction, or we could have a u32 field for the number of keys
and a separate bit in the flags field to indicate that instruction
keys are supported.  Which would be preferable?

Paul.

Re: [Qemu-devel] [PATCH 06/27] vhost-user-scsi: code style fixes

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 20 
  1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index cddaaf8995..8d6ff18cc4 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -305,7 +305,8 @@ fail:
  }
  
  static struct scsi_task *scsi_task_new(int cdb_len, uint8_t *cdb, int dir,

-   int xfer_len) {
+   int xfer_len)
+{
  struct scsi_task *task;
  
  assert(cdb_len > 0);

@@ -344,7 +345,8 @@ static int handle_cmd_sync(struct iscsi_context *ctx,
 VirtIOSCSICmdReq *req,
 struct iovec *out, unsigned int out_len,
 VirtIOSCSICmdResp *rsp,
-   struct iovec *in, unsigned int in_len) {
+   struct iovec *in, unsigned int in_len)
+{
  struct scsi_task *task;
  uint32_t dir;
  uint32_t len;
@@ -454,7 +456,8 @@ static void vus_panic_cb(VuDev *vu_dev, const char *buf)
  }
  
  static void vus_add_watch_cb(VuDev *vu_dev, int fd, int vu_evt, vu_watch_cb cb,

- void *pvt) {
+ void *pvt)
+{
  vhost_scsi_dev_t *vdev_scsi;
  guint id;
  
@@ -529,7 +532,7 @@ static void vus_proc_req(VuDev *vu_dev, int idx)

  return;
  }
  
-if ((idx < 0) || (idx >= VHOST_MAX_NR_VIRTQUEUE)) {

+if (idx < 0 || idx >= VHOST_MAX_NR_VIRTQUEUE) {
  PERR("VQ Index out of range: %d", idx);
  vus_panic_cb(vu_dev, NULL);
  return;
@@ -556,8 +559,8 @@ static void vus_proc_req(VuDev *vu_dev, int idx)
  }
  PDBG("Popped elem@%p", elem);
  
-assert(!((elem->out_num > 1) && (elem->in_num > 1)));

-assert((elem->out_num > 0) && (elem->in_num > 0));
+assert(!(elem->out_num > 1 && elem->in_num > 1));
+assert(elem->out_num > 0 && elem->in_num > 0);
  
  if (elem->out_sg[0].iov_len < sizeof(VirtIOSCSICmdReq)) {

  PERR("Invalid virtio-scsi req header");
@@ -593,7 +596,7 @@ static void vus_queue_set_started(VuDev *vu_dev, int idx, 
bool started)
  
  assert(vu_dev);
  
-if ((idx < 0) || (idx >= VHOST_MAX_NR_VIRTQUEUE)) {

+if (idx < 0 || idx >= VHOST_MAX_NR_VIRTQUEUE) {
  PERR("VQ Index out of range: %d", idx);
  vus_panic_cb(vu_dev, NULL);
  return;
@@ -750,7 +753,8 @@ err:
  }
  
  static int vdev_scsi_add_iscsi_lun(vhost_scsi_dev_t *vdev_scsi,

-   char *iscsi_uri, uint32_t lun) {
+   char *iscsi_uri, uint32_t lun)
+{
  assert(vdev_scsi);
  assert(iscsi_uri);
  assert(lun < VUS_MAX_LUNS);

Re: [Qemu-devel] [PATCH 15/15] ppc: Add aCube Sam460ex board

[David Gibson]

On Wed, Aug 23, 2017 at 01:43:56PM +0200, François Revol wrote:
> Le 23/08/2017 à 13:12, BALATON Zoltan a écrit :
> >> What's the connection with mips_malta?
> > 
> > The board's firmware wants to see SPD EEPROMs of the connected memory
> > while initialising the memory controller. This is why we need to
> > implement SDRAM controller, I2C and SPD EEPROMs. MIPS malta board had
> > already SPD EEPROM implementation so this is based on that. The comment
> > just indicates where this code comes from.
> 
> Indeed, and I copy-pasted from elsewhere for this.
> 
> >>> +fprintf(stderr, "qemu: Error registering flash memory.\n");
> >>
> >> Use error_report() instead, please.
> 
> I guess this didn't exist back when I started writing it...

Probably not.

> >>> +/* Create reset TLB entries for BookE, mapping only the flash
> >>> memory.  */
> >>> +static void mmubooke_create_initial_mapping_uboot(CPUPPCState *env)
> >>> +{
> >>> +ppcemb_tlb_t *tlb = >tlb.tlbe[0];
> >>> +
> >>> +/* on reset the flash is mapped by a shadow TLB,
> >>> + * but since we don't implement them we need to use
> >>> + * the same values U-Boot will use to avoid a fault.
> >>> + */
> >>
> >> Usually the reset state of the MMU is handled in the cpu code rather
> >> than the board code.  Is there a specific reason you need it in the
> >> board code here?
> > 
> > I'm not sure, probably lack of a better place. The ppc440_bamboo board
> > this is based on has it the same way in the board code. Maybe this could
> > be cleaned up when someone wants to QOMify the SoC models sometimes.
> 
> Thing is, the code allows both booting with U-Boot and with a kernel
> directly, and the MMU mapping differ in those cases.
> 
> Maybe the CPU reset should use the U-Boot setup and the kernel boot
> would just overwrite it?

Yes.  Basically the CPU reset should do what real hardware does - and
I'd expect u-boot to be written to work with that.  The kernel, on the
other hand will expect whatever mappings come out of u-boot (or
another bootloader).  Long term, I think you want to always use the
hardware setup and actually run the guest firmware to set up the
mappings the kernel expects.  For early development where it's useful
to be able to boot into a kernel without firmware, faking the right
mappings in the board code is perfectly reasonable.

-- 
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 for-2.11 08/27] sparc: replace cpu_sparc_init() with cpu_generic_init()

[Philippe Mathieu-Daudé]

On 08/21/2017 08:11 AM, Igor Mammedov wrote:

Philippe Mathieu-Daudé  wrote:


I succeed booting a SPARC image once applying 1 to 8 (this commit),
so not being able to boot the image previously is probably due to patch
order, which mean current order is not bisect-able.


I repeated patch by patch (1-8) build/test steps and it works for me
(i.e. compiles and both sparc/sparc64 target machines start as expected)

Could you re-check with clean build and if failure still persist
pin-point exact cause?


Sorry Igor, I guess it was some issue on my side (dirty working space 
probably).


Regards,

Phil.

Re: [Qemu-devel] [PATCH for-2.11 08/27] sparc: replace cpu_sparc_init() with cpu_generic_init()

[Philippe Mathieu-Daudé]

On 08/18/2017 07:08 AM, Igor Mammedov wrote:

it's just a wrapper, drop it and use cpu_generic_init() directly

Signed-off-by: Igor Mammedov 


Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


---
CC: Fabien Chouteau 
CC: Mark Cave-Ayland 
CC: Artyom Tarasenko 
---
  target/sparc/cpu.h   | 3 +--
  hw/sparc/leon3.c | 2 +-
  hw/sparc/sun4m.c | 2 +-
  hw/sparc64/sparc64.c | 2 +-
  target/sparc/cpu.c   | 5 -
  5 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 0e41916..b45cfb4 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -579,7 +579,6 @@ void cpu_raise_exception_ra(CPUSPARCState *, int, 
uintptr_t) QEMU_NORETURN;
  
  #ifndef NO_CPU_IO_DEFS

  /* cpu_init.c */
-SPARCCPU *cpu_sparc_init(const char *cpu_model);
  void cpu_sparc_set_id(CPUSPARCState *env, unsigned int cpu);
  void sparc_cpu_list(FILE *f, fprintf_function cpu_fprintf);
  /* mmu_helper.c */
@@ -656,7 +655,7 @@ hwaddr cpu_get_phys_page_nofault(CPUSPARCState *env, 
target_ulong addr,
  int cpu_sparc_signal_handler(int host_signum, void *pinfo, void *puc);
  
  #ifndef NO_CPU_IO_DEFS

-#define cpu_init(cpu_model) CPU(cpu_sparc_init(cpu_model))
+#define cpu_init(cpu_model) cpu_generic_init(TYPE_SPARC_CPU, cpu_model)
  #endif
  
  #define cpu_signal_handler cpu_sparc_signal_handler

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index d5ff188..56512ec 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -126,7 +126,7 @@ static void leon3_generic_hw_init(MachineState *machine)
  cpu_model = "LEON3";
  }
  
-cpu = cpu_sparc_init(cpu_model);

+cpu = SPARC_CPU(cpu_generic_init(TYPE_SPARC_CPU, cpu_model));
  if (cpu == NULL) {
  fprintf(stderr, "qemu: Unable to find Sparc CPU definition\n");
  exit(1);
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 89dd8a9..cf47dca 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -797,7 +797,7 @@ static void cpu_devinit(const char *cpu_model, unsigned int 
id,
  SPARCCPU *cpu;
  CPUSPARCState *env;
  
-cpu = cpu_sparc_init(cpu_model);

+cpu = SPARC_CPU(cpu_generic_init(TYPE_SPARC_CPU, cpu_model));
  if (cpu == NULL) {
  fprintf(stderr, "qemu: Unable to find Sparc CPU definition\n");
  exit(1);
diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
index 4e4fdab..ecf38a4 100644
--- a/hw/sparc64/sparc64.c
+++ b/hw/sparc64/sparc64.c
@@ -353,7 +353,7 @@ SPARCCPU *sparc64_cpu_devinit(const char *cpu_model,
  if (cpu_model == NULL) {
  cpu_model = default_cpu_model;
  }
-cpu = cpu_sparc_init(cpu_model);
+cpu = SPARC_CPU(cpu_generic_init(TYPE_SPARC_CPU, cpu_model));
  if (cpu == NULL) {
  fprintf(stderr, "Unable to find Sparc CPU definition\n");
  exit(1);
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index fd01cbf..2917021 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -110,11 +110,6 @@ static void sparc_cpu_parse_features(const char *typename, 
char *features,
  cpu_legacy_parse_featurestr(typename, features, errp);
  }
  
-SPARCCPU *cpu_sparc_init(const char *cpu_model)

-{
-return SPARC_CPU(cpu_generic_init(TYPE_SPARC_CPU, cpu_model));
-}
-
  void cpu_sparc_set_id(CPUSPARCState *env, unsigned int cpu)
  {
  #if !defined(TARGET_SPARC64)

Re: [Qemu-devel] [PATCH for-2.11 04/27] sparc: move adhoc CPUSPARCState initialization to realize time

[Philippe Mathieu-Daudé]

On 08/18/2017 07:08 AM, Igor Mammedov wrote:

SPARCCPU::env was initialized from previosuly set properties


"previously"


(with help of sparc_cpu_parse_features) in cpu_sparc_register().
However there is not reason to keep it there as this task is
typically done at realize time. So move post properties
initialization into sparc_cpu_realizefn, which brings
cpu_sparc_init() closer to cpu_generic_init().

Signed-off-by: Igor Mammedov 


Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


---
CC: Mark Cave-Ayland 
CC: Artyom Tarasenko 
---
  target/sparc/cpu.c | 30 +++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index e735d73..7f74b1c 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -109,7 +109,6 @@ static void sparc_cpu_parse_features(CPUState *cs, char 
*features,
  
  static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)

  {
-CPUSPARCState *env = >env;
  char *s = g_strdup(cpu_model);
  char *featurestr = strtok(s, ",");
  Error *err = NULL;
@@ -122,19 +121,6 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
*cpu_model)
  return -1;
  }
  
-env->version = env->def.iu_version;

-env->fsr = env->def.fpu_version;
-env->nwindows = env->def.nwindows;
-#if !defined(TARGET_SPARC64)
-env->mmuregs[0] |= env->def.mmu_version;
-cpu_sparc_set_id(env, 0);
-env->mxccregs[7] |= env->def.mxcc_version;
-#else
-env->mmu_version = env->def.mmu_version;
-env->maxtl = env->def.maxtl;
-env->version |= env->def.maxtl << 8;
-env->version |= env->def.nwindows - 1;
-#endif
  return 0;
  }
  
@@ -817,15 +803,29 @@ static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)

  CPUState *cs = CPU(dev);
  SPARCCPUClass *scc = SPARC_CPU_GET_CLASS(dev);
  Error *local_err = NULL;
-#if defined(CONFIG_USER_ONLY)
  SPARCCPU *cpu = SPARC_CPU(dev);
  CPUSPARCState *env = >env;
  
+#if defined(CONFIG_USER_ONLY)

  if ((env->def.features & CPU_FEATURE_FLOAT)) {
  env->def.features |= CPU_FEATURE_FLOAT128;
  }
  #endif
  
+env->version = env->def.iu_version;

+env->fsr = env->def.fpu_version;
+env->nwindows = env->def.nwindows;
+#if !defined(TARGET_SPARC64)
+env->mmuregs[0] |= env->def.mmu_version;
+cpu_sparc_set_id(env, 0);
+env->mxccregs[7] |= env->def.mxcc_version;
+#else
+env->mmu_version = env->def.mmu_version;
+env->maxtl = env->def.maxtl;
+env->version |= env->def.maxtl << 8;
+env->version |= env->def.nwindows - 1;
+#endif
+
  cpu_exec_realizefn(cs, _err);
  if (local_err != NULL) {
  error_propagate(errp, local_err);

Re: [Qemu-devel] [PATCH for-2.11 02/27] sparc: embed sparc_def_t into CPUSPARCState

[Philippe Mathieu-Daudé]

On 08/18/2017 07:08 AM, Igor Mammedov wrote:

Make CPUSPARCState::def embedded so it would be allocated as part
of cpu instance and we won't have to worry about cleaning def pointer
up mannualy on cpu destruction.

Signed-off-by: Igor Mammedov 


Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


---
Embedded CPUSPARCState::def also needed for follow up patch
to use its fields with static qdev properties.

CC: Riku Voipio 
CC: Laurent Vivier 
CC: Mark Cave-Ayland 
CC: Artyom Tarasenko 
---
  linux-user/sparc/target_syscall.h |  2 +-
  target/sparc/cpu.h|  8 
  target/sparc/cpu.c| 39 ---
  target/sparc/int32_helper.c   |  2 +-
  target/sparc/int64_helper.c   |  2 +-
  target/sparc/ldst_helper.c| 14 +++---
  target/sparc/mmu_helper.c |  2 +-
  target/sparc/translate.c  |  2 +-
  target/sparc/win_helper.c |  4 ++--
  9 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/linux-user/sparc/target_syscall.h 
b/linux-user/sparc/target_syscall.h
index f97aa6b..5f09abf 100644
--- a/linux-user/sparc/target_syscall.h
+++ b/linux-user/sparc/target_syscall.h
@@ -31,7 +31,7 @@ struct target_pt_regs {
  
  static inline abi_ulong target_shmlba(CPUSPARCState *env)

  {
-if (!(env->def->features & CPU_FEATURE_FLUSH)) {
+if (!(env->def.features & CPU_FEATURE_FLUSH)) {
  return 64 * 1024;
  } else {
  return 256 * 1024;
diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 95a36a4..0e41916 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -529,7 +529,7 @@ struct CPUSPARCState {
  #define SOFTINT_INTRMASK (0xFFFE)
  #define SOFTINT_REG_MASK (SOFTINT_STIMER|SOFTINT_INTRMASK|SOFTINT_TIMER)
  #endif
-sparc_def_t *def;
+sparc_def_t def;
  
  void *irq_manager;

  void (*qemu_irq_ack)(CPUSPARCState *env, void *irq_manager, int intno);
@@ -679,7 +679,7 @@ int cpu_sparc_signal_handler(int host_signum, void *pinfo, 
void *puc);
  #if defined (TARGET_SPARC64)
  static inline int cpu_has_hypervisor(CPUSPARCState *env1)
  {
-return env1->def->features & CPU_FEATURE_HYPV;
+return env1->def.features & CPU_FEATURE_HYPV;
  }
  
  static inline int cpu_hypervisor_mode(CPUSPARCState *env1)

@@ -788,14 +788,14 @@ static inline void cpu_get_tb_cpu_state(CPUSPARCState 
*env, target_ulong *pc,
  if (env->pstate & PS_AM) {
  flags |= TB_FLAG_AM_ENABLED;
  }
-if ((env->def->features & CPU_FEATURE_FLOAT)
+if ((env->def.features & CPU_FEATURE_FLOAT)
  && (env->pstate & PS_PEF)
  && (env->fprs & FPRS_FEF)) {
  flags |= TB_FLAG_FPU_ENABLED;
  }
  flags |= env->asi << TB_FLAG_ASI_SHIFT;
  #else
-if ((env->def->features & CPU_FEATURE_FLOAT) && env->psref) {
+if ((env->def.features & CPU_FEATURE_FLOAT) && env->psref) {
  flags |= TB_FLAG_FPU_ENABLED;
  }
  #endif
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 2994c09..f4e7343 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -66,7 +66,7 @@ static void sparc_cpu_reset(CPUState *s)
  env->lsu = 0;
  #else
  env->mmuregs[0] &= ~(MMU_E | MMU_NF);
-env->mmuregs[0] |= env->def->mmu_bm;
+env->mmuregs[0] |= env->def.mmu_bm;
  #endif
  env->pc = 0;
  env->npc = env->pc + 4;
@@ -120,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
*cpu_model)
  return -1;
  }
  
-env->version = env->def->iu_version;

-env->fsr = env->def->fpu_version;
-env->nwindows = env->def->nwindows;
+env->version = env->def.iu_version;
+env->fsr = env->def.fpu_version;
+env->nwindows = env->def.nwindows;
  #if !defined(TARGET_SPARC64)
-env->mmuregs[0] |= env->def->mmu_version;
+env->mmuregs[0] |= env->def.mmu_version;
  cpu_sparc_set_id(env, 0);
-env->mxccregs[7] |= env->def->mxcc_version;
+env->mxccregs[7] |= env->def.mxcc_version;
  #else
-env->mmu_version = env->def->mmu_version;
-env->maxtl = env->def->maxtl;
-env->version |= env->def->maxtl << 8;
-env->version |= env->def->nwindows - 1;
+env->mmu_version = env->def.mmu_version;
+env->maxtl = env->def.maxtl;
+env->version |= env->def.maxtl << 8;
+env->version |= env->def.nwindows - 1;
  #endif
  return 0;
  }
@@ -558,7 +558,7 @@ static void sparc_cpu_parse_features(CPUState *cs, char 
*features,
   Error **errp)
  {
  SPARCCPU *cpu = SPARC_CPU(cs);
-sparc_def_t *cpu_def = cpu->env.def;
+sparc_def_t *cpu_def = >env.def;
  char *featurestr;
  uint32_t plus_features = 0;
  uint32_t minus_features = 0;
@@ -819,8 +819,8 @@ static void sparc_cpu_realizefn(DeviceState *dev, Error 
**errp)
  SPARCCPU *cpu = SPARC_CPU(dev);
  

Re: [Qemu-devel] [PATCH for-2.11 01/27] sparc: convert cpu models to SPARC cpu subclasses

[Philippe Mathieu-Daudé]

On 08/22/2017 10:12 PM, Philippe Mathieu-Daudé wrote:

On 08/18/2017 07:08 AM, Igor Mammedov wrote:

QOMfy cpu models handling introducing propper cpu types
for each cpu model.

Signed-off-by: Igor Mammedov 
---
with this and conversion of features to properties,
it would be possible to replace cpu_sparc_init() with
cpu_generic_init() and reuse common -cpu handling
infrastructure.

CC: Mark Cave-Ayland 
CC: Artyom Tarasenko 
CC: Philippe Mathieu-Daudé 

v2:
    * make base class abstract (Philippe Mathieu-Daudé 
)

---
   target/sparc/cpu-qom.h |   2 +
   target/sparc/cpu.c | 121 
+

   2 files changed, 84 insertions(+), 39 deletions(-)

diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
index f63af72..af6d57a 100644
--- a/target/sparc/cpu-qom.h
+++ b/target/sparc/cpu-qom.h
@@ -35,6 +35,7 @@
   #define SPARC_CPU_GET_CLASS(obj) \
   OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)
+typedef struct sparc_def_t sparc_def_t;
   /**
    * SPARCCPUClass:
    * @parent_realize: The parent class' realize handler.
@@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {
   DeviceRealize parent_realize;
   void (*parent_reset)(CPUState *cpu);
+    sparc_def_t *cpu_def;
   } SPARCCPUClass;
   typedef struct SPARCCPU SPARCCPU;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index d606eb5..2994c09 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -25,8 +25,6 @@
   //#define DEBUG_FEATURES
-static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char 
*cpu_model);

-
   /* CPUClass::reset() */
   static void sparc_cpu_reset(CPUState *s)
   {
@@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu, 
const char *cpu_model)

   {
   CPUSPARCState *env = >env;
   char *s = g_strdup(cpu_model);
-    char *featurestr, *name = strtok(s, ",");
-    sparc_def_t def1, *def = 
+    char *featurestr = strtok(s, ",");
   Error *err = NULL;
-    if (cpu_sparc_find_by_name(def, name) < 0) {
-    g_free(s);
-    return -1;
-    }
-
-    env->def = g_memdup(def, sizeof(*def));
-
   featurestr = strtok(NULL, ",");
   sparc_cpu_parse_features(CPU(cpu), featurestr, );
   g_free(s);
@@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, 
const char *cpu_model)

   return -1;
   }
-    env->version = def->iu_version;
-    env->fsr = def->fpu_version;
-    env->nwindows = def->nwindows;
+    env->version = env->def->iu_version;
+    env->fsr = env->def->fpu_version;
+    env->nwindows = env->def->nwindows;
   #if !defined(TARGET_SPARC64)
-    env->mmuregs[0] |= def->mmu_version;
+    env->mmuregs[0] |= env->def->mmu_version;
   cpu_sparc_set_id(env, 0);
-    env->mxccregs[7] |= def->mxcc_version;
+    env->mxccregs[7] |= env->def->mxcc_version;
   #else
-    env->mmu_version = def->mmu_version;
-    env->maxtl = def->maxtl;
-    env->version |= def->maxtl << 8;
-    env->version |= def->nwindows - 1;
+    env->mmu_version = env->def->mmu_version;
+    env->maxtl = env->def->maxtl;
+    env->version |= env->def->maxtl << 8;
+    env->version |= env->def->nwindows - 1;
   #endif
   return 0;
   }
@@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu, 
const char *cpu_model)

   SPARCCPU *cpu_sparc_init(const char *cpu_model)
   {
   SPARCCPU *cpu;
+    ObjectClass *oc;
+    char *str, *name;
+
+    str = g_strdup(cpu_model);
+    name = strtok(str, ",");


you can free 'str' once here:

 g_free(str);


+    oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
+    if (oc == NULL) {
+    g_free(str);


drop


+    return NULL;
+    }
+    g_free(str);


drop


I'll fix it up if I have to respin series (note this function is 
removed within series)


Indeed I didn't notice



[...]


Anyway I can't start Aurelien's image [1] with this commit:

Can't reproduce it locally (either dirty/clean tree rebuild)
with ./configure --enable-debug


$ sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
qemu: Unable to find Sparc CPU definition
for this kind of error guest image doesn't matter as error happens 
during machine_init()



[1] https://people.debian.org/~aurel32/qemu/sparc/

It boots just fine with default CPU model:

./sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2


Yes, my bad!

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 



Sorry I'v been distracted by something else :/
I might have done something wrong, I'll test it again tomorrow.



and with explicitly specified one:

sparc-softmmu/qemu-system-sparc -cpu "Fujitsu MB86904"

I get from "info qom-tree"
...
  /device[0] (Fujitsu-MB86904-sparc-cpu)
...

Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

[Dou Liyang]

Hi Igor,

At 08/24/2017 01:47 AM, Igor Mammedov wrote:

On Wed, 23 Aug 2017 21:35:29 +0800
Dou Liyang  wrote:


Hi Igor,

At 08/23/2017 08:45 PM, Igor Mammedov wrote:

On Wed, 23 Aug 2017 20:12:51 +0800
Dou Liyang  wrote:


Hi Igor,

At 08/23/2017 04:40 PM, Igor Mammedov wrote:

On Tue, 22 Aug 2017 11:24:10 +0800
Dou Liyang  wrote:


As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
  ... \
  -m 128,slots=3,maxmem=1G \
  -numa node -numa node,mem=128M \

But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.

Add a testcase for this situation to make sure the ACPI table is
correct for guest.

Suggested-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
---
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
 tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
 tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
 tests/bios-tables-test.c  |  30 ++
 7 files changed, 30 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem



considering only SRAT table has been changed and the other
tables match with default blobs, I'd suggest to keep only



Our testcase is：

+test_acpi_one(" -m 128,slots=3,maxmem=1G"
+  " -numa node -numa node,mem=128"
+  " -numa dist,src=0,dst=1,val=21",
+  );

The DSDT and SLIT don't match with default blobs.

do you actually need SLIT table /i.e. -numa dist/ for test at all?
it looks not relevant for the test case at the hand,
I'd suggest to drop '-numa dist' option for the test.



OK, Got it, will drop '-numa dist' option in next version.



So, they can't be dropped.


I wonder what's changed, could you post DSDT diff here?



Just like memory hot-plug cases, when we use the '-m 128
128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
Memory Device in the DSDT table.

for your case '-numa node -numa node,mem=128', there is no need in enabling 
memory hotplug.


Thank you very much for your kind explanation.

Yes, I understood.


If I recall it correctly the default memory for x86 is 128Mb,
hence removing "-m" would probably make DSDT match default one.


Yes, I removed the "-m":

test_acpi_one(" -numa node -numa node,mem=128", );

but, the DSDT didn't match the default one. because, if we support
NUMA, the DSDT will give us "_PXM" to map the CPU to node.


--- a/tmp/asl-7QO54Y.dsl
+++ b/tmp/asl-EWM54Y.dsl
@@ -5,13 +5,13 @@
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/acpi-test-data/pc/DSDT, Thu Aug 24 09:20:29 2017
+ * Disassembly of /tmp/aml-1YM54Y, Thu Aug 24 09:20:29 2017
  *
  * Original Table Header:
  * Signature"DSDT"
- * Length   0x13EA (5098)
+ * Length   0x13F0 (5104)
  * Revision 0x01  32-bit table (V1), no 64-bit math 
support

- * Checksum 0x78
+ * Checksum 0x13
  * OEM ID   "BOCHS "
  * OEM Table ID "BXPCDSDT"
  * OEM Revision 0x0001 (1)
@@ -783,6 +783,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", 
"BXPCDSDT", 0x0001)

 {
 COST (Zero, Arg0, Arg1, Arg2)
 }
+
+Name (_PXM, Zero)  // _PXM: Device Proximity
 }
 }
 }


Thanks,
dou.





[...]

Re: [Qemu-devel] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[David Gibson]

On Wed, Aug 23, 2017 at 08:14:32PM -0300, Thiago Jung Bauermann wrote:
> 
> Hello David,
> 
> Thank you for your input.
> 
> David Gibson  writes:
> 
> > On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> >> LoPAPR says:
> >> 
> >> “ibm,processor-storage-keys”
> >> 
> >> property name indicating the number of virtual storage keys supported
> >> by the processor described by this node.
> >> 
> >> prop-encoded-array: Consists of two cells encoded as with encode-int.
> >> The first cell represents the number of virtual storage keys supported
> >> for data accesses while the second cell represents the number of
> >> virtual storage keys supported for instruction accesses. The cell value
> >> of zero indicates that no storage keys are supported for the access
> >> type.
> >> 
> >> pHyp provides the property above but there's a bug in P8 firmware where the
> >> second cell is zero even though POWER8 supports instruction access keys.
> >> This bug will be fixed for P9.
> >> 
> >> Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
> >> 
> >> Signed-off-by: Thiago Jung Bauermann 
> >
> > Regardless of anything else, this clearly won't go in until 2.11 opens.
> 
> Ok, not a problem.
> 
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -605,6 +605,80 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> >> *fdt, int offset,
> >>pcc->radix_page_info->count *
> >>sizeof(radix_AP_encodings[0];
> >>  }
> >> +
> >> +if (spapr->storage_keys) {
> >> +uint32_t val[2];
> >> +
> >> +val[0] = cpu_to_be32(spapr->storage_keys);
> >> +val[1] = spapr->insn_keys ? val[0] : 0;
> >> +
> >> +_FDT(fdt_setprop(fdt, offset, "ibm,processor-storage-keys",
> >> + val, sizeof(val)));
> >> +}
> >> +}
> >> +
> >> +#define SYSFS_PROT_KEYS_PATH "/sys/kernel/mm/protection_keys/"
> >> +#define SYSFS_USABLE_STORAGE_KEYS SYSFS_PROT_KEYS_PATH "usable_keys"
> >> +#define SYSFS_DISABLE_EXEC_KEYS SYSFS_PROT_KEYS_PATH 
> >> "disable_execute_supported"
> >> +
> >> +static void setup_storage_keys(CPUPPCState *env, sPAPRMachineState *spapr)
> >> +{
> >> +if (!(env->mmu_model & POWERPC_MMU_AMR))
> >> +return;
> >> +
> >> +if (kvm_enabled()) {
> >> +char buf[sizeof("false\n")];
> >> +uint32_t keys;
> >> +FILE *fd;
> >
> > For starters, reasonably complex kvm-only code like this should go
> > into target/ppc/kvm.c.
> 
> Ok, will move the code there.
> 
> > But, there's a more fundamental problem.  Automatically adjusting
> > guest visible parameters based on the host's capabilities is really
> > problematic: if you migrate to a host with different capabilities
> > what's usable suddenly changes, but the guest can't know.
> >
> > The usual way to deal with this is instead to have explicit machine
> > parameters to control the value, and check if those are possible on
> > the host.  It's then up to the user and/or management layer to set the
> > parameters suitable to allow migration between all machines that they
> > care about.
> 
> Good point, I hadn't thought of it. I will add the machine parameter
> then and send a v2. Can the default value of the parameter be what is
> supported by the host? 

That's probably ok, although it's not entirely unproblematic.  For
safety I think we'd also need to migrate the value and on the
destination check that the migrated value is supportable.  Presumably
it's ok if the new host supports more storage keys than the former
one, just not the other way around.

-- 
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 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

[Dou Liyang]

Hi Eduardo,

At 08/24/2017 01:25 AM, Eduardo Habkost wrote:

On Wed, Aug 23, 2017 at 09:35:29PM +0800, Dou Liyang wrote:

Hi Igor,

At 08/23/2017 08:45 PM, Igor Mammedov wrote:

On Wed, 23 Aug 2017 20:12:51 +0800
Dou Liyang  wrote:


Hi Igor,

At 08/23/2017 04:40 PM, Igor Mammedov wrote:

On Tue, 22 Aug 2017 11:24:10 +0800
Dou Liyang  wrote:


As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
  ... \
  -m 128,slots=3,maxmem=1G \
  -numa node -numa node,mem=128M \

But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.

Add a testcase for this situation to make sure the ACPI table is
correct for guest.

Suggested-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
---
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
 tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
 tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
 tests/bios-tables-test.c  |  30 ++
 7 files changed, 30 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem



considering only SRAT table has been changed and the other
tables match with default blobs, I'd suggest to keep only



Our testcase is：

+test_acpi_one(" -m 128,slots=3,maxmem=1G"
+  " -numa node -numa node,mem=128"
+  " -numa dist,src=0,dst=1,val=21",
+  );

The DSDT and SLIT don't match with default blobs.

do you actually need SLIT table /i.e. -numa dist/ for test at all?
it looks not relevant for the test case at the hand,
I'd suggest to drop '-numa dist' option for the test.



OK, Got it, will drop '-numa dist' option in next version.



So, they can't be dropped.


I wonder what's changed, could you post DSDT diff here?



Just like memory hot-plug cases, when we use the '-m
128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
Memory Device in the DSDT table.


Do you really need to use -m 128,slots=3,maxmem=1G to test your
bug fix?



I was wrong, As the default memory for x86 is 128Mb, I will remove this 
option to make one case just do one thing.


Thanks,
dou.

Re: [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci

[Thomas Huth]

On 23.08.2017 17:54, Cornelia Huck wrote:
> The msi routing code in kvm calls some pci functions: provide
> some stubs to enable builds without pci.
> 
> Also, to make this more obvious, guard them via a pci_available boolean
> (which also can be reused in other places).
> 
> Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn")
> Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions")
> Reviewed-by: Pierre Morel 
> Signed-off-by: Cornelia Huck 
> ---
>  accel/kvm/kvm-all.c  |  6 +++---
>  hw/pci/pci-stub.c| 16 +++-
>  hw/pci/pci.c |  2 ++
>  include/hw/pci/pci.h |  2 ++
>  4 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 46ce479dc3..f85553a851 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, 
> PCIDevice *dev)
>  int virq;
>  MSIMessage msg = {0, 0};
>  
> -if (dev) {
> +if (pci_available && dev) {
>  msg = pci_get_msi_message(dev, vector);
>  }
>  
> @@ -1271,7 +1271,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, 
> PCIDevice *dev)
>  kroute.u.msi.address_lo = (uint32_t)msg.address;
>  kroute.u.msi.address_hi = msg.address >> 32;
>  kroute.u.msi.data = le32_to_cpu(msg.data);
> -if (kvm_msi_devid_required()) {
> +if (pci_available && kvm_msi_devid_required()) {
>  kroute.flags = KVM_MSI_VALID_DEVID;
>  kroute.u.msi.devid = pci_requester_id(dev);
>  }
> @@ -1309,7 +1309,7 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, 
> MSIMessage msg,
>  kroute.u.msi.address_lo = (uint32_t)msg.address;
>  kroute.u.msi.address_hi = msg.address >> 32;
>  kroute.u.msi.data = le32_to_cpu(msg.data);
> -if (kvm_msi_devid_required()) {
> +if (pci_available && kvm_msi_devid_required()) {
>  kroute.flags = KVM_MSI_VALID_DEVID;
>  kroute.u.msi.devid = pci_requester_id(dev);
>  }
> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> index ecad664946..2d3b32c6ab 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -23,10 +23,11 @@
>  #include "monitor/monitor.h"
>  #include "qapi/qmp/qerror.h"
>  #include "hw/pci/pci.h"
> -#include "qmp-commands.h"
>  #include "hw/pci/msi.h"
> +#include "qmp-commands.h"

Nit: Unnecessary movement of the #include statement.

If you fix that:

Reviewed-by: Thomas Huth 

[Qemu-devel] qemu wowlan support

[Shahar Matityahu]

hello,
I am using qemu and I want to know if it is possible to to use wowlan under
qemu
I assigned my NIC to the qemu with pci-pass-through and I couldn't get it
to work with wowlan




Thanks in advance,
Shahar S Matityahu

[Qemu-devel] [PATCH] qemu-doc: Add UUID support in initiator name

[Fred Rolland]

Update doc with the usage of UUID for initiator name.

Related-To: https://bugzilla.redhat.com/1006468
Signed-off-by: Fred Rolland 
---
 qemu-doc.texi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 9811476..4076226 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1066,10 +1066,11 @@ in a configuration file provided via '-readconfig' or 
directly on the
 command line.
 
 If the initiator-name is not specified qemu will use a default name
-of 'iqn.2008-11.org.linux-kvm[:'] where  is the name of the
+of 'iqn.2008-11.org.linux-kvm[:'] where  is the UUID of the
+virtual machine. If the UUID is not specified qemu will use
+'iqn.2008-11.org.linux-kvm[:'] where  is the name of the
 virtual machine.
 
-
 @example
 Setting a specific initiator name to use when logging in to the target
 -iscsi initiator-name=iqn.qemu.test:my-initiator
-- 
2.9.5

Re: [Qemu-devel] [PATCH 00/27] vhost-user-scsi: code clean-up

[Liu, Changpeng]

Thanks for helping the example code clean-up. I can rebase the vhost-user-blk 
patch
set after your commits. For the issue you mentioned below, I think the 
vhost-user-scsi example
can only support 1 LUN, so LUN0 is exist, LUN1 reported error should be okay.

> -Original Message-
> From: Marc-André Lureau [mailto:marcandre.lur...@redhat.com]
> Sent: Thursday, August 24, 2017 12:20 AM
> To: qemu-devel@nongnu.org
> Cc: Liu, Changpeng ; fel...@nutanix.com; Marc-André
> Lureau 
> Subject: [PATCH 00/27] vhost-user-scsi: code clean-up
> 
> Hi,
> 
> While reviewing vhost-user-blk, I realized a lot of code was based on
> vhost-user-scsi, and I found a number of improvements could be
> made. As a result in this series, I tried to move common glib code in
> libvhost-user-glib. (I originally made libvhost-user glib-free, so if
> external projects want to play with it, they don't have to depend on
> glib, for ex vhost-user-bridge doesn't use glib).
> 
> I haven't done extensive testing, I tried to setup a LUN with help
> from https://fedoraproject.org/wiki/Scsi-target-utils_Quickstart_Guide, but
> the guest says "Unexpected response from lun 1 while scanning, scan
> aborted" (before or after the series). Help welcome!
> 
> Thanks
> 
> Marc-André Lureau (27):
>   glib-compat: move G_SOURCE_CONTINUE/REMOVE there
>   libvhost-user: drop dependency on glib
>   libvhost-user: improve vu_queue_pop() doc
>   vhost-user-scsi: use g_strdup()
>   vhost-user-scsi: connect unix socket before allocating
>   vhost-user-scsi: code style fixes
>   vhost-user-scsi: use glib allocation
>   vhost-user-scsi: glib calls that allocate don't return NULL
>   vhost-user-scsi: also free the gtree
>   vhost-user-scsi: remove vdev_scsi_find_by_vu()
>   vhost-user-scsi: simplify unix path cleanup
>   vhost-user-scsi: use NULL pointer
>   vhost-user-scsi: use glib watch directly
>   vhost-user-scsi: assert() in iscsi_add_lun()
>   vhost-user-scsi: remove vdev_scsi_add_iscsi_lun()
>   vhost-user-scsi: remove VUS_MAX_LUNS
>   vhost-user-scsi: remove unimplemented functions
>   vhost-user-scsi: rename VUS types
>   vhost-user-scsi: avoid use of iscsi_ namespace
>   vhost-user-scsi: don't copy iscsi/scsi-lowlevel.h
>   vhost-user-scsi: drop extra callback pointer
>   vhost-user-scsi: simplify source handling
>   vhost-user-scsi: use glib logging
>   libvhost-user: add glib source helper
>   build-sys: fix libvhost-user.a build
>   vhost-user-scsi: use libvhost-user glib helper
>   vhost-user-scsi: remove server_sock from VusDev
> 
>  contrib/libvhost-user/libvhost-user-glib.h |  32 ++
>  contrib/libvhost-user/libvhost-user.h  |   3 +-
>  include/glib-compat.h  |   7 +
>  contrib/libvhost-user/libvhost-user-glib.c | 145 +++
>  contrib/libvhost-user/libvhost-user.c  |  25 +-
>  contrib/vhost-user-scsi/vhost-user-scsi.c  | 619 
> +
>  Makefile   |   3 +-
>  Makefile.objs  |   3 +-
>  contrib/libvhost-user/Makefile.objs|   2 +-
>  tests/Makefile.include |   2 +-
>  10 files changed, 320 insertions(+), 521 deletions(-)
>  create mode 100644 contrib/libvhost-user/libvhost-user-glib.h
>  create mode 100644 contrib/libvhost-user/libvhost-user-glib.c
> 
> --
> 2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang

[Philippe Mathieu-Daudé]

Hi Anatol,

Next time use 'v2' for an updated patch (for the automated tracking 
tools), also no need to link it to the previous Message-Id it looks more 
messy and can get lost.


On 08/23/2017 09:01 PM, Anatol Pomozov wrote:

  * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
CPU to make sure no new instructions are used.
  * add memset() implementation. Clang implements array zeroing in
print_num() via memset() function call.
---
  tests/multiboot/Makefile| 2 +-
  tests/multiboot/libc.c  | 9 +
  tests/multiboot/libc.h  | 2 ++
  tests/multiboot/run_test.sh | 1 +
  4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..2e015409dc 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
  CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-march=pentium


Now that I remark it, I think -ffreestanding is a better flag to 
multiboot than -fno-builtin.



  ASFLAGS=-m32
  
  LD=ld

diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
  
  return dest;

  }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
  
  static void print_char(char c)

  {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
  typedef signed char int8_t;
  
  typedef uint32_t uintptr_t;

+typedef uint32_t size_t;
  


I'd also add here:

_Static_assert(sizeof(void *) == sizeof(uint32_t), "Expecting a 32-bit 
arch");


Anyway:
Reviewed-by: Philippe Mathieu-Daudé 

  
  /* stdarg.h */

@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
  
  void printf(const char *fmt, ...);

  void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
  
  #endif

diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index f04e35cbf0..38dcfef42c 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -29,6 +29,7 @@ run_qemu() {
  printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
  
  $QEMU \

+-cpu pentium \
  -kernel $kernel \
  -display none \
  -device isa-debugcon,chardev=stdio \

Re: [Qemu-devel] [PATCH 16/27] vhost-user-scsi: remove VUS_MAX_LUNS

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

There is no code to support more than 1 yet, no need for that today.

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index f6f9e7b192..429e3065ea 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -53,8 +53,6 @@
  
  /** vhost-user-scsi specific definitions **/
  
- /* Only 1 LUN and device supported today */

-#define VUS_MAX_LUNS 1
  #define VUS_ISCSI_INITIATOR "iqn.2016-11.com.nutanix:vhost-user-scsi"
  
  typedef struct iscsi_lun {

@@ -67,7 +65,7 @@ typedef struct vhost_scsi_dev {
  int server_sock;
  GMainLoop *loop;
  GTree *fdmap;   /* fd -> gsource context id */
-iscsi_lun_t luns[VUS_MAX_LUNS];
+iscsi_lun_t lun;
  } vhost_scsi_dev_t;
  
  /** glib event loop integration for libvhost-user and misc callbacks **/

@@ -533,7 +531,7 @@ static void vus_proc_req(VuDev *vu_dev, int idx)
  }
  rsp = (VirtIOSCSICmdResp *)elem->in_sg[0].iov_base;
  
-if (handle_cmd_sync(vdev_scsi->luns[0].iscsi_ctx,

+if (handle_cmd_sync(vdev_scsi->lun.iscsi_ctx,
  req, >out_sg[1], elem->out_num - 1,
  rsp, >in_sg[1], elem->in_num - 1) != 0) {
  vus_panic_cb(vu_dev, NULL);
@@ -722,7 +720,7 @@ int main(int argc, char **argv)
  }
  vdev_scsi = vdev_scsi_new(sock);
  
-if (iscsi_add_lun(_scsi->luns[0], iscsi_uri) != 0) {

+if (iscsi_add_lun(_scsi->lun, iscsi_uri) != 0) {
  goto err;
  }
  

Re: [Qemu-devel] [PATCH 19/27] vhost-user-scsi: avoid use of iscsi_ namespace

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

It is confusing and could easily conflict with future versions.

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 2ceca964f4..2ab14f1a83 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -245,7 +245,7 @@ struct scsi_task {
  
  /** libiscsi integration **/
  
-static int iscsi_add_lun(VusIscsiLun *lun, char *iscsi_uri)

+static int vus_iscsi_add_lun(VusIscsiLun *lun, char *iscsi_uri)
  {
  struct iscsi_url *iscsi_url;
  struct iscsi_context *iscsi_ctx;
@@ -705,7 +705,7 @@ int main(int argc, char **argv)
  }
  vdev_scsi = vdev_scsi_new(sock);
  
-if (iscsi_add_lun(_scsi->lun, iscsi_uri) != 0) {

+if (vus_iscsi_add_lun(_scsi->lun, iscsi_uri) != 0) {
  goto err;
  }
  

Re: [Qemu-devel] [PATCH 27/27] vhost-user-scsi: remove server_sock from VusDev

[Philippe Mathieu-Daudé]

Hi Marc-André,

On 08/23/2017 01:20 PM, Marc-André Lureau wrote:

It is unneeded in the VusDev device structure, and also simplify a bit
the code.

Signed-off-by: Marc-André Lureau 
---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 52 ++-
  1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index cfd62b46ce..3166331856 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -28,7 +28,6 @@ typedef struct VusIscsiLun {
  typedef struct VusDev {
  VugDev parent;
  
-int server_sock;

  VusIscsiLun lun;
  } VusDev;
  
@@ -371,48 +370,30 @@ fail:
  
  static void vdev_scsi_free(VusDev *vdev_scsi)


let's kill vdev_scsi_free(),


  {
-if (vdev_scsi->server_sock >= 0) {
-close(vdev_scsi->server_sock);
-}
  g_free(vdev_scsi);
  }
  
-static VusDev *vdev_scsi_new(int server_sock)

+static VusDev *vdev_scsi_new(void)
  {
-VusDev *vdev_scsi;
-
-assert(server_sock >= 0);
-
-vdev_scsi = g_new0(VusDev, 1);
-vdev_scsi->server_sock = server_sock;
-
-return vdev_scsi;
+return g_new0(VusDev, 1);


also kill vdev_scsi_new() ...


  }
  
-static int vdev_scsi_run(VusDev *vdev_scsi)

+static int vdev_scsi_run(VusDev *vdev_scsi, int sock)
  {
  GMainLoop *loop;
  GIOChannel *chan;
-int cli_sock;
  int ret = 0;
  
  assert(vdev_scsi);

-assert(vdev_scsi->server_sock >= 0);
-
-cli_sock = accept(vdev_scsi->server_sock, NULL, NULL);
-if (cli_sock < 0) {
-perror("accept");
-return -1;
-}
  
  loop = g_main_loop_new(NULL, FALSE);

  vug_init(_scsi->parent,
- cli_sock,
+ sock,
   loop,
   vus_panic_cb,
   _iface);
  
-chan = g_io_channel_unix_new(cli_sock);

+chan = g_io_channel_unix_new(sock);
  g_io_add_watch(chan, G_IO_IN, vus_vhost_cb, vdev_scsi);
  g_main_loop_run(loop);
  g_io_channel_unref(chan);
@@ -428,7 +409,7 @@ int main(int argc, char **argv)
  VusDev *vdev_scsi = NULL;
  char *unix_fn = NULL;
  char *iscsi_uri = NULL;
-int sock, opt, err = EXIT_SUCCESS;
+int lsock = -1, csock = -1, opt, err = EXIT_SUCCESS;
  
  while ((opt = getopt(argc, argv, "u:i:")) != -1) {

  switch (opt) {
@@ -448,17 +429,24 @@ int main(int argc, char **argv)
  goto help;
  }
  
-sock = unix_sock_new(unix_fn);

-if (sock < 0) {
+lsock = unix_sock_new(unix_fn);
+if (lsock < 0) {
+goto err;
+}
+
+csock = accept(lsock, NULL, NULL);
+if (csock < 0) {
+perror("accept");
  goto err;
  }
-vdev_scsi = vdev_scsi_new(sock);
+
+vdev_scsi = vdev_scsi_new();


... and use g_new0() directly:

   vdev_scsi = g_new0(VusDev, 1);

  
  if (vus_iscsi_add_lun(_scsi->lun, iscsi_uri) != 0) {

  goto err;
  }
  
-if (vdev_scsi_run(vdev_scsi) != 0) {

+if (vdev_scsi_run(vdev_scsi, csock) != 0) {
  goto err;
  }
  
@@ -467,6 +455,12 @@ out:

  vdev_scsi_free(vdev_scsi);


and g_free() here:

g_free(vdev_scsi);

If you agree:
Reviewed-by: Philippe Mathieu-Daudé 


  unlink(unix_fn);
  }
+if (csock >= 0) {
+close(csock);
+}
+if (lsock >= 0) {
+close(lsock);
+}
  g_free(unix_fn);
  g_free(iscsi_uri);
  

Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang

[Anatol Pomozov]

On Wed, Aug 23, 2017 at 4:44 PM, Philippe Mathieu-Daudé  wrote:
> Hi Anatol,
>
> On 08/23/2017 04:22 PM, Anatol Pomozov wrote:
>>
>>   * explicitly disable SSE as clang enables it by default for 32bit code
>>   * add memset() implementation. Clang complains that the function is
>> absent, gcc seems provides default built-in.
>> ---
>>   tests/multiboot/Makefile | 2 +-
>>   tests/multiboot/libc.c   | 9 +
>>   tests/multiboot/libc.h   | 2 ++
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
>> index 856e76682a..7fadbca33a 100644
>> --- a/tests/multiboot/Makefile
>> +++ b/tests/multiboot/Makefile
>> @@ -1,5 +1,5 @@
>>   CC=gcc
>> -CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc
>> -fno-builtin
>> +CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc
>> -fno-builtin -mno-sse
>
>
> What about using -march=i586 instead? so no need to disable each next
> features clang decide to default enable.
>
> Even cleaner IMHO, use -march=pentium in CFLAGS and add "-cpu pentium" in
> run_test.sh so we are sure the generated code matches, what do you think?

It sounds reasonable. Just sent an updated patch. Works with gcc 6.3
and clang 3.8.

[Qemu-devel] [PATCH] multiboot: make tests work with clang

[Anatol Pomozov]

 * clang 3.8 enables SSE even for 32bit code. Generate code for pentium
   CPU to make sure no new instructions are used.
 * add memset() implementation. Clang implements array zeroing in
   print_num() via memset() function call.
---
 tests/multiboot/Makefile| 2 +-
 tests/multiboot/libc.c  | 9 +
 tests/multiboot/libc.h  | 2 ++
 tests/multiboot/run_test.sh | 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..2e015409dc 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-march=pentium
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
 return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
 
 #endif
diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
index f04e35cbf0..38dcfef42c 100755
--- a/tests/multiboot/run_test.sh
+++ b/tests/multiboot/run_test.sh
@@ -29,6 +29,7 @@ run_qemu() {
 printf %b "\n\n=== Running test case: $kernel $* ===\n\n" >> test.log
 
 $QEMU \
+-cpu pentium \
 -kernel $kernel \
 -display none \
 -device isa-debugcon,chardev=stdio \
-- 
2.14.1.342.g6490525c54-goog

Re: [Qemu-devel] [PATCH 23/27] vhost-user-scsi: use glib logging

[Philippe Mathieu-Daudé]

On 08/23/2017 01:20 PM, Marc-André Lureau wrote:

- PLOG is unused
- code is compiled out unless debug is enabled
- logging is too verbose
- you can pipe to ts to have timestamp if needed, or use structured
   logging with more recent glib

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 77 +--
  1 file changed, 21 insertions(+), 56 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 516a9d3966..e3ec8b9b2c 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -18,42 +18,6 @@
  
  #include 
  
-/* #define VUS_DEBUG 1 */

-
-/** Log helpers **/
-
-#define PPRE  \
-struct timespec ts;   \
-char   timebuf[64];   \
-struct tm tm; \
-(void)clock_gettime(CLOCK_REALTIME, ); \
-(void)strftime(timebuf, 64, "%Y%m%d %T", gmtime_r(_sec, ))
-
-#define PEXT(lvl, msg, ...) do {  \
-PPRE; \
-fprintf(stderr, "%s.%06ld " lvl ": %s:%s():%d: " msg "\n",\
-timebuf, ts.tv_nsec / 1000,   \
-__FILE__, __func__, __LINE__, ## __VA_ARGS__);\
-} while (0)
-
-#define PNOR(lvl, msg, ...) do {  \
-PPRE; \
-fprintf(stderr, "%s.%06ld " lvl ": " msg "\n",\
-timebuf, ts.tv_nsec / 1000, ## __VA_ARGS__);  \
-} while (0)
-
-#ifdef VUS_DEBUG
-#define PDBG(msg, ...) PEXT("DBG", msg, ## __VA_ARGS__)
-#define PERR(msg, ...) PEXT("ERR", msg, ## __VA_ARGS__)
-#define PLOG(msg, ...) PEXT("LOG", msg, ## __VA_ARGS__)
-#else
-#define PDBG(msg, ...) { }
-#define PERR(msg, ...) PNOR("ERR", msg, ## __VA_ARGS__)
-#define PLOG(msg, ...) PNOR("LOG", msg, ## __VA_ARGS__)
-#endif
-
-/** vhost-user-scsi specific definitions **/
-
  #define VUS_ISCSI_INITIATOR "iqn.2016-11.com.nutanix:vhost-user-scsi"
  
  typedef struct VusIscsiLun {

@@ -166,27 +130,28 @@ static int vus_iscsi_add_lun(VusIscsiLun *lun, char 
*iscsi_uri)
  
  iscsi_ctx = iscsi_create_context(VUS_ISCSI_INITIATOR);

  if (!iscsi_ctx) {
-PERR("Unable to create iSCSI context");
+g_warning("Unable to create iSCSI context");
  return -1;
  }
  
  iscsi_url = iscsi_parse_full_url(iscsi_ctx, iscsi_uri);

  if (!iscsi_url) {
-PERR("Unable to parse iSCSI URL: %s", iscsi_get_error(iscsi_ctx));
+g_warning("Unable to parse iSCSI URL: %s", iscsi_get_error(iscsi_ctx));
  goto fail;
  }
  
  iscsi_set_session_type(iscsi_ctx, ISCSI_SESSION_NORMAL);

  iscsi_set_header_digest(iscsi_ctx, ISCSI_HEADER_DIGEST_NONE_CRC32C);
  if (iscsi_full_connect_sync(iscsi_ctx, iscsi_url->portal, 
iscsi_url->lun)) {
-PERR("Unable to login to iSCSI portal: %s", 
iscsi_get_error(iscsi_ctx));
+g_warning("Unable to login to iSCSI portal: %s",
+  iscsi_get_error(iscsi_ctx));
  goto fail;
  }
  
  lun->iscsi_ctx = iscsi_ctx;

  lun->iscsi_lun = iscsi_url->lun;
  
-PDBG("Context %p created for lun 0: %s", iscsi_ctx, iscsi_uri);

+g_debug("Context %p created for lun 0: %s", iscsi_ctx, iscsi_uri);
  
  out:

  if (iscsi_url) {
@@ -228,7 +193,7 @@ static int get_cdb_len(uint8_t *cdb)
  case 4: return 16;
  case 5: return 12;
  }
-PERR("Unable to determine cdb len (0x%02hhX)", cdb[0] >> 5);
+g_warning("Unable to determine cdb len (0x%02hhX)", cdb[0] >> 5);
  return -1;
  }
  
@@ -250,7 +215,7 @@ static int handle_cmd_sync(struct iscsi_context *ctx,
  
  if (!(!req->lun[1] && req->lun[2] == 0x40 && !req->lun[3])) {

  /* Ignore anything different than target=0, lun=0 */
-PDBG("Ignoring unconnected lun (0x%hhX, 0x%hhX)",
+g_debug("Ignoring unconnected lun (0x%hhX, 0x%hhX)",
   req->lun[1], req->lun[3]);
  rsp->status = SCSI_STATUS_CHECK_CONDITION;
  memset(rsp->sense, 0, sizeof(rsp->sense));
@@ -293,10 +258,10 @@ static int handle_cmd_sync(struct iscsi_context *ctx,
  task->iovector_in.niov = in_len;
  }
  
-PDBG("Sending iscsi cmd (cdb_len=%d, dir=%d, task=%p)",

+g_debug("Sending iscsi cmd (cdb_len=%d, dir=%d, task=%p)",
   cdb_len, dir, task);
  if (!iscsi_scsi_command_sync(ctx, 0, task, NULL)) {
-PERR("Error serving SCSI command");
+g_warning("Error serving SCSI command");
  g_free(task);
  return -1;
  }
@@ -314,7 +279,7 @@ 

Re: [Qemu-devel] [PATCH 21/27] vhost-user-scsi: drop extra callback pointer

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

Use the one from the source with casting, like any other glib source.

Signed-off-by: Marc-André Lureau 
---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 795799ec14..102d910e8f 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -81,7 +81,6 @@ typedef struct vus_gsrc {
  GSource parent;
  VusDev *vdev_scsi;
  GPollFD gfd;
-vu_watch_cb vu_cb;
  } vus_gsrc_t;
  
  static gint vus_fdmap_compare(gconstpointer a, gconstpointer b)

@@ -112,18 +111,13 @@ static gboolean vus_gsrc_dispatch(GSource *src, 
GSourceFunc cb, gpointer data)
  vus_gsrc_t *vus_src = (vus_gsrc_t *)src;
  
  assert(vus_src);

-assert(!(vus_src->vu_cb && cb));
  
  vdev_scsi = vus_src->vdev_scsi;
  
  assert(vdev_scsi);
  
-if (cb) {

-return cb(data);
-}
-if (vus_src->vu_cb) {
-vus_src->vu_cb(_scsi->vu_dev, vus_src->gfd.revents, data);
-}
+((vu_watch_cb)cb) (_scsi->vu_dev, vus_src->gfd.revents, data);


dropping the space between func/args:
Reviewed-by: Philippe Mathieu-Daudé 


+
  return G_SOURCE_CONTINUE;
  }
  
@@ -146,12 +140,12 @@ static void vus_gsrc_new(VusDev *vdev_scsi, int fd, GIOCondition cond,

  assert(vu_cb);
  
  vus_gsrc = g_source_new(_gsrc_funcs, sizeof(vus_gsrc_t));

+g_source_set_callback(vus_gsrc, (GSourceFunc) vu_cb, data, NULL);
  vus_src = (vus_gsrc_t *)vus_gsrc;
  
  vus_src->vdev_scsi = vdev_scsi;

  vus_src->gfd.fd = fd;
  vus_src->gfd.events = cond;
-vus_src->vu_cb = vu_cb;
  
  g_source_add_poll(vus_gsrc, _src->gfd);

  id = g_source_attach(vus_gsrc, NULL);

Re: [Qemu-devel] [PATCH] multiboot: make tests work with clang

[Philippe Mathieu-Daudé]

Hi Anatol,

On 08/23/2017 04:22 PM, Anatol Pomozov wrote:

  * explicitly disable SSE as clang enables it by default for 32bit code
  * add memset() implementation. Clang complains that the function is
absent, gcc seems provides default built-in.
---
  tests/multiboot/Makefile | 2 +-
  tests/multiboot/libc.c   | 9 +
  tests/multiboot/libc.h   | 2 ++
  3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..7fadbca33a 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
  CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-mno-sse


What about using -march=i586 instead? so no need to disable each next 
features clang decide to default enable.


Even cleaner IMHO, use -march=pentium in CFLAGS and add "-cpu pentium" 
in run_test.sh so we are sure the generated code matches, what do you think?


Regards,

Phil.


  ASFLAGS=-m32
  
  LD=ld

diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
  
  return dest;

  }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
  
  static void print_char(char c)

  {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
  typedef signed char int8_t;
  
  typedef uint32_t uintptr_t;

+typedef uint32_t size_t;
  
  
  /* stdarg.h */

@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
  
  void printf(const char *fmt, ...);

  void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
  
  #endif

Re: [Qemu-devel] [RFC v2 4/8] QAPI: new QMP command option "without-bql"

[Fam Zheng]

On Wed, 08/23 18:44, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > Introducing this new parameter for QMP commands in general to mark out
> > when the command does not need BQL.  Normally QMP command executions are
> > done with the protection of BQL in QEMU.  However the truth is that not
> > all the QMP commands require the BQL.
> > 
> > This new parameter provides a way to allow QMP commands to run in
> > parallel when possible, without the contention on the BQL.
> > 
> > Since the default value of "without-bql" is still false, so now all QMP
> > commands are still protected by BQL still.
> > 
> > Signed-off-by: Peter Xu 
> 
> We should define what a 'without-bql' command is allowed to do:
>'Commands that have without-bql set _may_ be called without the bql
>being taken.  They must not take the bql or any other lock that may
>become dependent on the bql.'
>(Do we need to say anything about RCU?)
> 
> Also, 'no-bql' is shorter :-)

Or rather "need-bql" that defaults to true to avoid double negative (TM) with
"no-bql = false"?

Fam

Re: [Qemu-devel] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[Thiago Jung Bauermann]

Hello David,

Thank you for your input.

David Gibson  writes:

> On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
>> LoPAPR says:
>> 
>> “ibm,processor-storage-keys”
>> 
>> property name indicating the number of virtual storage keys supported
>> by the processor described by this node.
>> 
>> prop-encoded-array: Consists of two cells encoded as with encode-int.
>> The first cell represents the number of virtual storage keys supported
>> for data accesses while the second cell represents the number of
>> virtual storage keys supported for instruction accesses. The cell value
>> of zero indicates that no storage keys are supported for the access
>> type.
>> 
>> pHyp provides the property above but there's a bug in P8 firmware where the
>> second cell is zero even though POWER8 supports instruction access keys.
>> This bug will be fixed for P9.
>> 
>> Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
>> 
>> Signed-off-by: Thiago Jung Bauermann 
>
> Regardless of anything else, this clearly won't go in until 2.11 opens.

Ok, not a problem.

>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -605,6 +605,80 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
>> *fdt, int offset,
>>pcc->radix_page_info->count *
>>sizeof(radix_AP_encodings[0];
>>  }
>> +
>> +if (spapr->storage_keys) {
>> +uint32_t val[2];
>> +
>> +val[0] = cpu_to_be32(spapr->storage_keys);
>> +val[1] = spapr->insn_keys ? val[0] : 0;
>> +
>> +_FDT(fdt_setprop(fdt, offset, "ibm,processor-storage-keys",
>> + val, sizeof(val)));
>> +}
>> +}
>> +
>> +#define SYSFS_PROT_KEYS_PATH "/sys/kernel/mm/protection_keys/"
>> +#define SYSFS_USABLE_STORAGE_KEYS SYSFS_PROT_KEYS_PATH "usable_keys"
>> +#define SYSFS_DISABLE_EXEC_KEYS SYSFS_PROT_KEYS_PATH 
>> "disable_execute_supported"
>> +
>> +static void setup_storage_keys(CPUPPCState *env, sPAPRMachineState *spapr)
>> +{
>> +if (!(env->mmu_model & POWERPC_MMU_AMR))
>> +return;
>> +
>> +if (kvm_enabled()) {
>> +char buf[sizeof("false\n")];
>> +uint32_t keys;
>> +FILE *fd;
>
> For starters, reasonably complex kvm-only code like this should go
> into target/ppc/kvm.c.

Ok, will move the code there.

> But, there's a more fundamental problem.  Automatically adjusting
> guest visible parameters based on the host's capabilities is really
> problematic: if you migrate to a host with different capabilities
> what's usable suddenly changes, but the guest can't know.
>
> The usual way to deal with this is instead to have explicit machine
> parameters to control the value, and check if those are possible on
> the host.  It's then up to the user and/or management layer to set the
> parameters suitable to allow migration between all machines that they
> care about.

Good point, I hadn't thought of it. I will add the machine parameter
then and send a v2. Can the default value of the parameter be what is
supported by the host? 

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

[Qemu-devel] [PATCH 5/5] pci: Validate interfaces on base_class_init

[Eduardo Habkost]

Make sure we don't forget to add the legacy-PCI or PCIe interface
names on PCI device classes in the future.

Signed-off-by: Eduardo Habkost 
---
 hw/pci/pci.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index baa3429..7ac5cc6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2525,6 +2525,17 @@ static void pci_device_class_init(ObjectClass *klass, 
void *data)
 pc->realize = pci_default_realize;
 }
 
+static void pci_device_class_base_init(ObjectClass *klass, void *data)
+{
+if (!object_class_is_abstract(klass)) {
+ObjectClass *legacy =
+object_class_dynamic_cast(klass, INTERFACE_LEGACY_PCI_DEVICE);
+ObjectClass *pcie =
+object_class_dynamic_cast(klass, INTERFACE_PCIE_DEVICE);
+assert(legacy || pcie);
+}
+}
+
 AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
 {
 PCIBus *bus = PCI_BUS(dev->bus);
@@ -2649,6 +2660,7 @@ static const TypeInfo pci_device_type_info = {
 .abstract = true,
 .class_size = sizeof(PCIDeviceClass),
 .class_init = pci_device_class_init,
+.class_base_init = pci_device_class_base_init,
 };
 
 static void pci_register_types(void)
-- 
2.9.4

Re: [Qemu-devel] Number of usable slots in PCIe Root Port / PCIe Switch Downstream Port

[Eduardo Habkost]

On Wed, Aug 23, 2017 at 11:16:27PM +0300, Marcel Apfelbaum wrote:
> On 23/08/2017 19:46, Andrea Bolognani wrote:
> > AFAIK some PCI controllers, particularly:
> > 
> >* PCIe Root Port (pcie-root-port, ioh3420)
> >* PCIe Switch Downstream Port (xio3130-downstream)
> > 
> > only have a single usable slot. libvirt knows about this
> > fact, and will prevent you from adding more than one
> > device to the respective bus.
> > 
> > However, as Lukáš recently noticed, QEMU won't complain
> > if you add more devices:
> > 
> >$ qemu-system-x86_64 \
> >  -nodefaults -nographic \
> >  -M q35 -monitor stdio \
> >  -device pcie-root-port,id=pci.1 \
> >  -device virtio-scsi-pci,bus=pci.1 \
> >  -device virtio-scsi-pci,bus=pci.1
> >QEMU 2.9.0 monitor - type 'help' for more information
> >(qemu) info qtree
> >bus: main-system-bus
> >  type System
> >  [...]
> >  dev: q35-pcihost, id ""
> >[...]
> >bus: pcie.0
> >  type PCIE
> >  dev: pcie-root-port, id "pci.1"
> >[...]
> >bus: pci.1
> >  type PCIE
> >  dev: virtio-scsi-pci, id ""
> >[...]
> >addr = 01.0
> >bus: virtio-bus
> >  type virtio-pci-bus
> >  dev: virtio-scsi-device, id ""
> >[...]
> >bus: scsi.1
> >  type SCSI
> >  dev: virtio-scsi-pci, id ""
> >[...]
> >addr = 00.0
> >bus: virtio-bus
> >  type virtio-pci-bus
> >  dev: virtio-scsi-device, id ""
> >[...]
> >bus: scsi.0
> >  type SCSI
> >(qemu)
> > 
> 
> Hi Andrea,
> 
> > As you can see, all devices will show up in the qtree;
> > only the one with addr=00.0, however, will actually be
> > visible to the guest OS according to my tests.
> > 
> > Is such a configuration considered valid?
> 
> Definitely no.
> 
> > Should QEMU
> > complain loudly about it and refuse to start?
> 
> Yes, but we need a clean way to do it. Eduardo is working
> on a series that would expose to libvirt this info,
> Eduardo can you please confirm?

The work has two parts: 1) actually preventing invalid
configurations; 2) providing a query interface to let libvirt
know which configurations are valid.

My focus is on (2) (adding a query-device-slots interface), but
we surely need to address bugs related to (1) first, so both the
device-plugging code and slot-querying code are correct and agree
with each other.

-- 
Eduardo

[Qemu-devel] [PATCH 4/5] pci: Add INTERFACE_LEGACY_PCI_DEVICE to legacy PCI devices

[Eduardo Habkost]

Add INTERFACE_LEGACY_PCI_DEVICE to all direct subtypes of
TYPE_PCI_DEVICE, except:

1) The ones that already have INTERFACE_PCIE_DEVICE set:

* base-xhci
* e1000e
* nvme
* pvscsi
* vfio-pci
* virtio-pci
* vmxnet3

2) base-pci-bridge

Not all PCI bridges are legacy PCI devices, so
INTERFACE_LEGACY_PCI_DEVICE is added only to the subtypes that
are actually legacy PCI devices:

* dec-21154-p2p-bridge
* i82801b11-bridge
* pbm-bridge
* pci-bridge

The direct subtypes of base-pci-bridge not touched by this patch
are:

* xilinx-pcie-root: Already marked as PCIe-only device.
* pcie-port: all non-abstract subtypes of pcie-port are already
  marked as PCIe-only devices.

3) megasas-base

Not all megasas devices are legacy PCI devices, so the interface
names are added to the subclasses registered by
megasas_register_types(), according to information in the
megasas_devices[] array.

"megasas-gen2" already implements INTERFACE_PCIE_DEVICE, so add
INTERFACE_LEGACY_PCI_DEVICE only to "megasas".

Signed-off-by: Eduardo Habkost 
---
 hw/acpi/piix4.c |  1 +
 hw/audio/ac97.c |  4 
 hw/audio/es1370.c   |  4 
 hw/audio/intel-hda.c|  4 
 hw/char/serial-pci.c| 12 
 hw/display/cirrus_vga.c |  4 
 hw/display/qxl.c|  4 
 hw/display/sm501.c  |  4 
 hw/display/vga-pci.c|  4 
 hw/display/vmware_vga.c |  4 
 hw/i2c/smbus_ich9.c |  4 
 hw/i386/amd_iommu.c |  4 
 hw/i386/kvm/pci-assign.c|  4 
 hw/i386/pc_piix.c   |  4 
 hw/i386/xen/xen_platform.c  |  4 
 hw/i386/xen/xen_pvdevice.c  |  4 
 hw/ide/ich.c|  4 
 hw/ide/pci.c|  4 
 hw/ipack/tpci200.c  |  4 
 hw/isa/i82378.c |  4 
 hw/isa/lpc_ich9.c   |  1 +
 hw/isa/piix4.c  |  4 
 hw/isa/vt82c686.c   | 16 
 hw/mips/gt64xxx_pci.c   |  4 
 hw/misc/edu.c   |  5 +
 hw/misc/ivshmem.c   |  4 
 hw/misc/macio/macio.c   |  4 
 hw/misc/pci-testdev.c   |  4 
 hw/net/e1000.c  |  4 
 hw/net/eepro100.c   |  4 
 hw/net/ne2000.c |  4 
 hw/net/pcnet-pci.c  |  4 
 hw/net/rocker/rocker.c  |  4 
 hw/net/rtl8139.c|  4 
 hw/pci-bridge/dec.c |  8 
 hw/pci-bridge/i82801b11.c   |  4 
 hw/pci-bridge/pci_bridge_dev.c  |  1 +
 hw/pci-bridge/pci_expander_bridge.c |  8 
 hw/pci-host/apb.c   |  8 
 hw/pci-host/bonito.c|  4 
 hw/pci-host/gpex.c  |  4 
 hw/pci-host/grackle.c   |  4 
 hw/pci-host/piix.c  |  8 
 hw/pci-host/ppce500.c   |  4 
 hw/pci-host/prep.c  |  4 
 hw/pci-host/q35.c   |  4 
 hw/pci-host/uninorth.c  | 16 
 hw/pci-host/versatile.c |  4 
 hw/ppc/ppc4xx_pci.c |  4 
 hw/scsi/esp-pci.c   |  4 
 hw/scsi/lsi53c895a.c|  4 
 hw/scsi/megasas.c   |  4 
 hw/scsi/mptsas.c|  4 
 hw/sd/sdhci.c   |  4 
 hw/sh4/sh_pci.c |  4 
 hw/sparc64/sun4u.c  |  4 
 hw/usb/hcd-ehci-pci.c   |  4 
 hw/usb/hcd-ohci.c   |  4 
 hw/usb/hcd-uhci.c   |  4 
 hw/vfio/pci-quirks.c|  4 
 hw/watchdog/wdt_i6300esb.c  |  4 
 hw/xen/xen_pt.c |  4 
 62 files changed, 288 insertions(+)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index f276967..defe98a 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -723,6 +723,7 @@ static const TypeInfo piix4_pm_info = {
 .interfaces = (InterfaceInfo[]) {
 { TYPE_HOTPLUG_HANDLER },
 { TYPE_ACPI_DEVICE_IF },
+{ INTERFACE_LEGACY_PCI_DEVICE },
 { }
 }
 };
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 959c786..6027e15 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1431,6 +1431,10 @@ static const TypeInfo ac97_info = {
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof (AC97LinkState),
 .class_init= ac97_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_LEGACY_PCI_DEVICE },
+{ },
+},
 };
 
 static void ac97_register_types (void)
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index dd7c23d..ed14ec3 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -1082,6 

[Qemu-devel] [PATCH 3/5] pci: Add INTERFACE_PCIE_DEVICE to all PCIe devices

[Eduardo Habkost]

Change all devices that set is_express=1 to implement
INTERFACE_PCIE_DEVICE.

Signed-off-by: Eduardo Habkost 
---
 hw/block/nvme.c| 4 
 hw/net/e1000e.c| 4 
 hw/pci-bridge/pcie_root_port.c | 4 
 hw/pci-bridge/xio3130_downstream.c | 4 
 hw/pci-bridge/xio3130_upstream.c   | 4 
 hw/pci-host/xilinx-pcie.c  | 4 
 hw/scsi/megasas.c  | 6 ++
 hw/usb/hcd-xhci.c  | 4 
 8 files changed, 34 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 6071dc1..26d58b6 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1110,6 +1110,10 @@ static const TypeInfo nvme_info = {
 .instance_size = sizeof(NvmeCtrl),
 .class_init= nvme_class_init,
 .instance_init = nvme_instance_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void nvme_register_types(void)
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 6c42b44..81f7934 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -708,6 +708,10 @@ static const TypeInfo e1000e_info = {
 .instance_size = sizeof(E1000EState),
 .class_init = e1000e_class_init,
 .instance_init = e1000e_instance_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void e1000e_register_types(void)
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 4d588cb..9b6e4ce 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -161,6 +161,10 @@ static const TypeInfo rp_info = {
 .class_init= rp_class_init,
 .abstract  = true,
 .class_size = sizeof(PCIERootPortClass),
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void rp_register_types(void)
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index e706f36..7d2f762 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -195,6 +195,10 @@ static const TypeInfo xio3130_downstream_info = {
 .name  = "xio3130-downstream",
 .parent= TYPE_PCIE_SLOT,
 .class_init= xio3130_downstream_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void xio3130_downstream_register_types(void)
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index a052224..227997c 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -166,6 +166,10 @@ static const TypeInfo xio3130_upstream_info = {
 .name  = "x3130-upstream",
 .parent= TYPE_PCIE_PORT,
 .class_init= xio3130_upstream_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void xio3130_upstream_register_types(void)
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 4613dda..7659253 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -317,6 +317,10 @@ static const TypeInfo xilinx_pcie_root_info = {
 .parent = TYPE_PCI_BRIDGE,
 .instance_size = sizeof(XilinxPCIERoot),
 .class_init = xilinx_pcie_root_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void xilinx_pcie_register(void)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 734fdae..3641c30 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2451,6 +2451,7 @@ typedef struct MegasasInfo {
 int osts;
 const VMStateDescription *vmsd;
 Property *props;
+InterfaceInfo *interfaces;
 } MegasasInfo;
 
 static struct MegasasInfo megasas_devices[] = {
@@ -2480,6 +2481,10 @@ static struct MegasasInfo megasas_devices[] = {
 .is_express = true,
 .vmsd = _megasas_gen2,
 .props = megasas_properties_gen2,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 }
 };
 
@@ -2531,6 +2536,7 @@ static void megasas_register_types(void)
 type_info.parent = TYPE_MEGASAS_BASE;
 type_info.class_data = (void *)info;
 type_info.class_init = megasas_class_init;
+type_info.interfaces = info->interfaces;
 
 type_register(_info);
 }
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 204ea69..d95ed4f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3670,6 +3670,10 @@ static const TypeInfo xhci_info = {
 .instance_size = sizeof(XHCIState),
 .class_init= xhci_class_init,
 .abstract  = true,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ }
+},
 };
 
 static void qemu_xhci_class_init(ObjectClass *klass, void *data)
-- 
2.9.4

[Qemu-devel] [PATCH 1/5] pci: INTERFACE_LEGACY_PCI_DEVICE and INTERFACE_PCIE_DEVICE interfaces

[Eduardo Habkost]

Those two interfaces will be used to indicate which device types
support legacy PCI or PCI-express buses.  Management software
will be able to use the qom-list-types QMP command to query that
information.

Signed-off-by: Eduardo Habkost 
---
 include/hw/pci/pci.h |  6 ++
 hw/pci/pci.c | 12 
 2 files changed, 18 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e598b09..f5e8ab9 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -195,6 +195,12 @@ enum {
 #define PCI_DEVICE_GET_CLASS(obj) \
  OBJECT_GET_CLASS(PCIDeviceClass, (obj), TYPE_PCI_DEVICE)
 
+/* Interface implemented by devices that can be plugged on PCIe buses */
+#define INTERFACE_PCIE_DEVICE "pci-express-device"
+
+/* Interface implemented by devices that can be plugged on legacy PCI buses */
+#define INTERFACE_LEGACY_PCI_DEVICE "legacy-pci-device"
+
 typedef struct PCIINTxRoute {
 enum {
 PCI_INTX_ENABLED,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 258fbe5..baa3429 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -168,6 +168,16 @@ static const TypeInfo pci_bus_info = {
 .class_init = pci_bus_class_init,
 };
 
+static const TypeInfo pcie_interface_info = {
+.name  = INTERFACE_PCIE_DEVICE,
+.parent= TYPE_INTERFACE,
+};
+
+static const TypeInfo legacy_pci_interface_info = {
+.name  = INTERFACE_LEGACY_PCI_DEVICE,
+.parent= TYPE_INTERFACE,
+};
+
 static const TypeInfo pcie_bus_info = {
 .name = TYPE_PCIE_BUS,
 .parent = TYPE_PCI_BUS,
@@ -2645,6 +2655,8 @@ static void pci_register_types(void)
 {
 type_register_static(_bus_info);
 type_register_static(_bus_info);
+type_register_static(_pci_interface_info);
+type_register_static(_interface_info);
 type_register_static(_device_type_info);
 }
 
-- 
2.9.4

[Qemu-devel] [PATCH 2/5] pci: Add interface names to hybrid PCI devices

[Eduardo Habkost]

The following devices support both PCIe and legacy PCI, by
including special code to handle the QEMU_PCI_CAP_EXPRESS flag:

* vfio-pci (is_express=1, but legacy PCI handled by
  vfio_populate_device())
* vmxnet3 (is_express=0, but PCIe handled by vmxnet3_realize())
* pvscsi (is_express=0, but PCIe handled by pvscsi_realize())
* virtio-pci (is_express=0, but PCIe handled by
  virtio_pci_dc_realize(), and additional legacy PCI code at
  virtio_pci_realize())

Signed-off-by: Eduardo Habkost 
---
 hw/net/vmxnet3.c   | 5 +
 hw/scsi/vmw_pvscsi.c   | 2 ++
 hw/vfio/pci.c  | 5 +
 hw/virtio/virtio-pci.c | 5 +
 4 files changed, 17 insertions(+)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index a19a7a3..61feacf 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2651,6 +2651,11 @@ static const TypeInfo vmxnet3_info = {
 .instance_size = sizeof(VMXNET3State),
 .class_init= vmxnet3_class_init,
 .instance_init = vmxnet3_instance_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_LEGACY_PCI_DEVICE },
+{ }
+},
 };
 
 static void vmxnet3_register_types(void)
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 77d8b6f..40dfffa 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1300,6 +1300,8 @@ static const TypeInfo pvscsi_info = {
 .class_init= pvscsi_class_init,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_HOTPLUG_HANDLER },
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_LEGACY_PCI_DEVICE },
 { }
 }
 };
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 31e1edf..2b21391 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3023,6 +3023,11 @@ static const TypeInfo vfio_pci_dev_info = {
 .class_init = vfio_pci_dev_class_init,
 .instance_init = vfio_instance_init,
 .instance_finalize = vfio_instance_finalize,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_LEGACY_PCI_DEVICE },
+{ }
+},
 };
 
 static void register_vfio_pci_dev_type(void)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 8b0d6b6..8c0b6bf 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1958,6 +1958,11 @@ static const TypeInfo virtio_pci_info = {
 .class_init= virtio_pci_class_init,
 .class_size= sizeof(VirtioPCIClass),
 .abstract  = true,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_PCIE_DEVICE },
+{ INTERFACE_LEGACY_PCI_DEVICE },
+{ }
+},
 };
 
 /* virtio-blk-pci */
-- 
2.9.4

[Qemu-devel] [PATCH 0/5] Mark legacy/PCIe/hybrid PCI devices using interface names

[Eduardo Habkost]

This series is a reimplementation of one portion of an old RFC:
  Subject: [RFC v2 00/20] qmp: Report bus information on
   'query-machines'

Specifically, this reimplements the portion that marks each PCI
device type as legacy/PCIe/hybrid.  It uses two new QOM interface
names to do that: INTERFACE_LEGACY_PCI_DEVICE
("legacy-pci-device") and INTERFACE_PCIE_DEVICE
("pci-express-device").  Legacy PCI devices will implement only
the former; PCIe-only devices will implement only the latter;
hybrid devices will implement both interfaces.

With this, management software will then be able to use
qom-list-types to find out which PCI devices are
legacy/PCIe/hybrid.

In the future, the new interface names can be used in the
bus/slot querying commands, to indicate which types of devices
are accepted on each slot.

The last patch in the series adds an assertion to the PCI device
class code, to ensure we won't forget to add the corresponding
interface names to new PCI device classes.

Eduardo Habkost (5):
  pci: INTERFACE_LEGACY_PCI_DEVICE and INTERFACE_PCIE_DEVICE interfaces
  pci: Add interface names to hybrid PCI devices
  pci: Add INTERFACE_PCIE_DEVICE to all PCIe devices
  pci: Add INTERFACE_LEGACY_PCI_DEVICE to legacy PCI devices
  pci: Validate interfaces on base_class_init

 include/hw/pci/pci.h|  6 ++
 hw/acpi/piix4.c |  1 +
 hw/audio/ac97.c |  4 
 hw/audio/es1370.c   |  4 
 hw/audio/intel-hda.c|  4 
 hw/block/nvme.c |  4 
 hw/char/serial-pci.c| 12 
 hw/display/cirrus_vga.c |  4 
 hw/display/qxl.c|  4 
 hw/display/sm501.c  |  4 
 hw/display/vga-pci.c|  4 
 hw/display/vmware_vga.c |  4 
 hw/i2c/smbus_ich9.c |  4 
 hw/i386/amd_iommu.c |  4 
 hw/i386/kvm/pci-assign.c|  4 
 hw/i386/pc_piix.c   |  4 
 hw/i386/xen/xen_platform.c  |  4 
 hw/i386/xen/xen_pvdevice.c  |  4 
 hw/ide/ich.c|  4 
 hw/ide/pci.c|  4 
 hw/ipack/tpci200.c  |  4 
 hw/isa/i82378.c |  4 
 hw/isa/lpc_ich9.c   |  1 +
 hw/isa/piix4.c  |  4 
 hw/isa/vt82c686.c   | 16 
 hw/mips/gt64xxx_pci.c   |  4 
 hw/misc/edu.c   |  5 +
 hw/misc/ivshmem.c   |  4 
 hw/misc/macio/macio.c   |  4 
 hw/misc/pci-testdev.c   |  4 
 hw/net/e1000.c  |  4 
 hw/net/e1000e.c |  4 
 hw/net/eepro100.c   |  4 
 hw/net/ne2000.c |  4 
 hw/net/pcnet-pci.c  |  4 
 hw/net/rocker/rocker.c  |  4 
 hw/net/rtl8139.c|  4 
 hw/net/vmxnet3.c|  5 +
 hw/pci-bridge/dec.c |  8 
 hw/pci-bridge/i82801b11.c   |  4 
 hw/pci-bridge/pci_bridge_dev.c  |  1 +
 hw/pci-bridge/pci_expander_bridge.c |  8 
 hw/pci-bridge/pcie_root_port.c  |  4 
 hw/pci-bridge/xio3130_downstream.c  |  4 
 hw/pci-bridge/xio3130_upstream.c|  4 
 hw/pci-host/apb.c   |  8 
 hw/pci-host/bonito.c|  4 
 hw/pci-host/gpex.c  |  4 
 hw/pci-host/grackle.c   |  4 
 hw/pci-host/piix.c  |  8 
 hw/pci-host/ppce500.c   |  4 
 hw/pci-host/prep.c  |  4 
 hw/pci-host/q35.c   |  4 
 hw/pci-host/uninorth.c  | 16 
 hw/pci-host/versatile.c |  4 
 hw/pci-host/xilinx-pcie.c   |  4 
 hw/pci/pci.c| 24 
 hw/ppc/ppc4xx_pci.c |  4 
 hw/scsi/esp-pci.c   |  4 
 hw/scsi/lsi53c895a.c|  4 
 hw/scsi/megasas.c   | 10 ++
 hw/scsi/mptsas.c|  4 
 hw/scsi/vmw_pvscsi.c|  2 ++
 hw/sd/sdhci.c   |  4 
 hw/sh4/sh_pci.c |  4 
 hw/sparc64/sun4u.c  |  4 
 hw/usb/hcd-ehci-pci.c   |  4 
 hw/usb/hcd-ohci.c   |  4 
 hw/usb/hcd-uhci.c   |  4 
 hw/usb/hcd-xhci.c   |  4 
 hw/vfio/pci-quirks.c|  4 
 hw/vfio/pci.c   |  5 +
 hw/virtio/virtio-pci.c  |  5 +
 hw/watchdog/wdt_i6300esb.c  |  4 
 hw/xen/xen_pt.c |  4 
 75 files changed, 369 insertions(+)

-- 
2.9.4

Re: [Qemu-devel] [Qemu-block] [PATCH v5 08/13] tests: Rely more on global_qtest

[Paolo Bonzini]

On 23/08/2017 23:30, Eric Blake wrote:
>> Well, whatever is assigning to global_qtest should be using the long form.
> Question - what about going the other way, and switching ALL callers to
> always use the explicit form?  I'd really like to maintain only one
> form, but if we think maintaining the explicit form is better, then I'd
> rather see:
> 
> s = qtest_init(...);
> qmp(s, "{'execute':'foo'}");

I think the short form is not problematic per se, but when e.g.
migration is involved, then the tests are abusing it...

Paolo



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH v5 08/13] tests: Rely more on global_qtest

[Eric Blake]

On 08/23/2017 03:02 PM, Paolo Bonzini wrote:
> On 23/08/2017 21:26, Eric Blake wrote:
>>> I agree with using the short form where possible, but I disagree on
>>> removing the long forms.  Rather, global_qtest in my opinion should have
>>> been static (though I'm not proposing that you do this); inlining the
>>> wrappers is not needed for performance.
>> But no one outside of libqtest.c is using the long form.  Maintaining a
>> long form that isn't used is counter-productive, if the short form is
>> good enough for everything we need.
> 
> Well, whatever is assigning to global_qtest should be using the long form.

Question - what about going the other way, and switching ALL callers to
always use the explicit form?  I'd really like to maintain only one
form, but if we think maintaining the explicit form is better, then I'd
rather see:

s = qtest_init(...);
qmp(s, "{'execute':'foo'}");

than:

qtest_init(...); // implicitly sets global_test
qmp("{'execute':'foo'}"); // implicitly uses global_test

and not the existing:

qtest_init(...);
qtest_qmp(global_test, "{'execute':'foo'}");

Then, in combination with my other series to make qmp() more friendly,
it would look like:

s = qtest_init(...);
qmp(s, "foo");

Or, put another way, I'd rather have the short name irq_intercept_in()
than the long name qtest_irq_intercept_in(); but as long as we maintain
only ONE set of names, then I'm okay having that one set always be
explicit about passing QTestState *s as a parameter and completely
getting rid of the global_test variable (rather than making it static).

But whatever we decide, we'll want to stick to it, and it may require a
respin - so let's get a consensus before I do any more churn on this series.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[ChristianEhrhardt]

Yes, with all the series of [1] on top it finally works.
Saw it already being merged on master.
Expecting a late rc4 or early release tag and then wrap all it up.

Thanks everybody involved!

[1]: http://lists.nongnu.org/archive/html/qemu-
devel/2017-08/msg04513.html

** Changed in: qemu
   Status: New => Fix Committed

** No longer affects: libvirt (Ubuntu)

** Changed in: qemu (Ubuntu)
   Status: Confirmed => In Progress

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

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  Fix Committed
Status in qemu package in Ubuntu:
  In Progress

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

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

Re: [Qemu-devel] [PULL 0/6] NBD pull request for 2.10-rc4

[Christian Ehrhardt]

On Wed, Aug 23, 2017 at 6:33 PM, Eric Blake  wrote:
[...]
>
> 
> nbd patches for 2017-08-23
>
> - Fam Zheng: 0/4 block: Fix non-shared storage migration
> - Stefan Hajnoczi: qemu-iotests: add 194 non-shared storage migration test
> - Stefan Hajnoczi: nbd-client: avoid spurious qio_channel_yield() re-entry
>
> 
> Fam Zheng (3):
>   block-backend: Refactor inactivate check
>   block-backend: Allow more "can inactivate" cases
>   mirror: Mark target BB as "force allow inactivate"
>
> Stefan Hajnoczi (3):
>   block: Update open_flags after ->inactivate() callback
>   qemu-iotests: add 194 non-shared storage migration test
>   nbd-client: avoid spurious qio_channel_yield() re-entry
>

... no need for tested-by after being applied already, but I wanted to
thank Fam, Stefan, David and everybody else involved.
With all six on top of rc3 the copy-storage migrations are working on 2.10
now - thanks!

[Qemu-devel] [PATCH] hw/ppc: CAS reset on early device hotplug

[Daniel Henrique Barboza]

This patch is a follow up on the discussions made in patch
"hw/ppc: disable hotplug before CAS is completed" that can be
found at [1].

At this moment, we do not support CPU/memory hotplug in early
boot stages, before CAS. When a hotplug occurs, the event is logged
in an internal rtas event log queue and an IRQ pulse is fired. In
regular conditions, the guest handles the interrupt by executing
check_exception, fetching the generated hotplug event and enabling
the device for use.

In early boot, this IRQ isn't caught (SLOF does not handle hotplug
events), leaving the event in the rtas event log queue. If the guest
executes check_exception due to another hotplug event, the re-assertion
of the IRQ ends up de-queuing the first hotplug event as well. In short,
a device hotplugged before CAS is considered coldplugged by SLOF.
This leads to device misbehavior and, in some cases, guest kernel
Ooops when trying to unplug the device.

A proper fix would be to turn every device hotplugged before CAS
as a colplugged device. This is not trivial to do with the current
code base though - the FDT is written in the guest memory at
ppc_spapr_reset and can't be retrieved without adding extra state
(fdt_size for example) that will need to managed and migrated. Adding
the hotplugged DT in the middle of CAS negotiation via the updated DT
tree works with CPU devs, but panics the guest kernel at boot. Additional
analysis would be necessary for LMBs and PCI devices. There are]
questions to be made in QEMU/SLOF/kernel level about how we can make
this change in a sustainable way.

Until we go all the way with the proper fix, this patch works around
the situation by issuing a CAS reset if a hotplugged device is detected
during CAS negotiations:

- 'spapr_cas_completed' is a helper function that is used in the 'plug'
functions of CPU, LMB and PCI devices to inform if we are past CAS. If a
device is hotplugged after CAS, no changes are made. Otherwise, we
do not fire any hotplug event (it will be lost anyway) and no DRC changes
are made, leaving the DRC in empty state.

- In the middle of CAS negotiations, the function
'spapr_hotplugged_dev_before_cas' goes through all the DRCs to see
if there are any DRC that belongs to a hotplugged dev and it has
empty state. This matches the condition of a hotplugged device before
CAS, returning '1' in 'spapr_h_cas_compose_response' which will set
spapr->cas_reboot to true in 'h_client_architecture_support', causing
the machine to reboot.

- after reboot, the hotplugged devs DTs will be added in the base FDT
tree by ppc_spapr_reset and will behave as expected.

- no changes are made for coldplug devices. A colplug device is either
a device that has dev->hotplugged = false or any device that was added
in the INMIGRATE runstate.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02855.html

Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/spapr.c  | 72 +
 hw/ppc/spapr_ovec.c |  7 +
 hw/ppc/spapr_pci.c  | 10 ++-
 include/hw/ppc/spapr.h  |  2 ++
 include/hw/ppc/spapr_ovec.h |  1 +
 5 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cec441c..10faae3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -790,6 +790,31 @@ out:
 return ret;
 }
 
+static bool spapr_hotplugged_dev_before_cas(void)
+{
+Object *drc_container, *obj;
+ObjectProperty *prop;
+ObjectPropertyIterator iter;
+sPAPRDRConnector *drc;
+sPAPRDRConnectorClass *drck;
+
+drc_container = container_get(object_get_root(), "/dr-connector");
+object_property_iter_init(, drc_container);
+while ((prop = object_property_iter_next())) {
+if (!strstart(prop->type, "link<", NULL)) {
+continue;
+}
+obj = object_property_get_link(drc_container, prop->name, NULL);
+drc = SPAPR_DR_CONNECTOR(obj);
+drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+if (drc->dev && drc->dev->hotplugged &&
+(drc->state == drck->empty_state)) {
+return true;
+}
+}
+return false;
+}
+
 int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
  target_ulong addr, target_ulong size,
  sPAPROptionVector *ov5_updates)
@@ -797,6 +822,10 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
 void *fdt, *fdt_skel;
 sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
 
+if (spapr_hotplugged_dev_before_cas()) {
+return 1;
+}
+
 size -= sizeof(hdr);
 
 /* Create sceleton */
@@ -2710,9 +2739,22 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error 
**errp)
 }
 }
 
+/*
+ * 'h_client_architecture_support' will set at least OV5_FORM1_AFFINITY
+ * in ov5_cas when intersecting it with spapr->ov5 and ov5_guest. It's safe
+ * then to assume that CAS ov5_cas will have 

Re: [Qemu-devel] [PATCH] audio: intel-hda: do not use old_mmio accesses

[Matt Parker]

On 22 August 2017 at 09:44, Peter Maydell  wrote:
> On 21 August 2017 at 22:13, Matt Parker  wrote:
>> intel-hda is still using the old_mmio accessors for io.
>> This updates the device to use .read and .write accessors instead.
>
> Hi; thanks for this patch.
>
> It looks like you forgot to provide your Signed-off-by: line;
> we can't accept patches without one, I'm afraid.
>
>> ---
>>  hw/audio/intel-hda.c | 56 
>> +---
>>  1 file changed, 9 insertions(+), 47 deletions(-)
>>
>> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
>> index 06acc98f7b..c0f002f744 100644
>> --- a/hw/audio/intel-hda.c
>> +++ b/hw/audio/intel-hda.c
>> @@ -1043,66 +1043,28 @@ static void intel_hda_regs_reset(IntelHDAState *d)
>>
>>  /* - */
>>
>> -static void intel_hda_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
>> +static void intel_hda_mmio_write(void *opaque, hwaddr addr, uint64_t val, 
>> unsigned size)
>>  {
>>  IntelHDAState *d = opaque;
>>  const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
>>
>> -intel_hda_reg_write(d, reg, val, 0xff);
>> +intel_hda_reg_write(d, reg, val, (1UL << (size * 8)) - 1);
>
> If size is 4 and 'unsigned long' is 32 bits (which it usually
> is) then this will shift off the end of the value, which is
> undefined behaviour.
>
> You could fix that by using '1ULL' instead of '1UL', but
> I'd suggest using MAKE_64BIT_MASK(0, size * 8) for this.
> (that macro is in "qemu/bitops.h", which you'll probably need
> to add a #include for.) Using the macro makes the intent a bit
> clearer and means nobody has to think about whether the bit
> shifting is doing what it's supposed to.
>
> I recommend putting a newline in the prototype to keep
> it below 80 lines and please the checkpatch script, too.
>
Thanks for the suggestions. I'll have a look into bitops.h and make
sure to run the checkpatch script before submitting any future
patches.

Matt

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

[Edgar E. Iglesias]

On Tue, Aug 22, 2017 at 10:04:25PM +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 22, 2017 at 03:13:57PM +, Diana Madalina Craciun wrote:
> > On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
> > > On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
> > >> Hi Edgar,
> > >>
> > >> On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> > >>> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
> >  Hi Diana,
> >  On 23/05/2017 13:12, Diana Craciun wrote:
> > > Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> > > Currently, for PCI devices, the requester ID was used as device
> > > ID in the virt machine. If the system has multiple masters that
> >  if the system has multiple root complex?
> > > use MSIs a unique ID accross the platform is needed.
> >  across
> > > A static scheme is used and each master is allocated a range of IDs
> > > with the formula:
> > > DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> > > recommended by SBSA).
> > >
> > > This ID will be configured in the machine creation and if not 
> > > configured
> > > the PCI requester ID will be used insteead.
> >  instead
> > > Signed-off-by: Diana Craciun 
> > > ---
> > >  hw/arm/virt.c  | 26 ++
> > >  hw/pci-host/gpex.c |  6 ++
> > >  hw/pci/msi.c   |  2 +-
> > >  hw/pci/pci.c   | 25 +
> > >  include/hw/arm/virt.h  |  1 +
> > >  include/hw/pci-host/gpex.h |  2 ++
> > >  include/hw/pci/pci.h   |  8 
> > >  kvm-all.c  |  4 ++--
> > >  8 files changed, 71 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 5f62a03..a969694 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams 
> > > platform_bus_params;
> > >  #define RAMLIMIT_GB 255
> > >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> > >  
> > > +#define STREAM_ID_RANGE_SIZE 0x1
> > > +
> > >  /* Addresses and sizes of our components.
> > >   * 0..128MB is space for a flash device so we can run bootrom code 
> > > such as UEFI.
> > >   * 128MB..256MB is used for miscellaneous device I/O.
> > > @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
> > >  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS 
> > > -1 */
> > >  };
> > >  
> > > +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> > > Currently
> > > + * for PCI devices the requester ID was used as device ID. But if 
> > > the system has
> > > + * multiple masters that use MSIs, the requester ID may cause 
> > > deviceID clashes.
> > > + * So a unique number is  needed accross the system.
> > > + * We are using the following formula:
> > > + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> > > + * (as recommanded by SBSA). Currently we do not have an SMMU 
> > > emulation, but the
> > > + * same formula can be used for the generation of the streamID as 
> > > well.
> > > + * For each master the device ID will be derrived from the requester 
> > > ID using
> > > + * the abovemntione formula.
> > > + */
> >  I think most of this comment should only be in the commit message. 
> >  typos
> >  in derived and above mentioned.
> > 
> >  stream id is the terminology for the id space at the input of the smmu.
> >  device id is the terminology for the id space at the input of the msi
> >  controller I think.
> > 
> >  RID -> deviceID (no IOMMU)
> >  RID -> streamID -> deviceID (IOMMU)
> > 
> >  I would personally get rid of all streamid uses as the smmu is not yet
> >  supported and stick to the
> >  Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> > 
> > > +
> > > +static const uint32_t streamidmap[] = {
> > > +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> > > +};
> > > +
> > >  static const char *valid_cpus[] = {
> > >  "cortex-a15",
> > >  "cortex-a53",
> > > @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState 
> > > *vms, qemu_irq *pic)
> > >  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
> > >  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
> > >  hwaddr base = base_mmio;
> > > +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> > > STREAM_ID_RANGE_SIZE;
> >  msi-base?
> >  STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
> > >  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> > >  int irq = vms->irqmap[VIRT_PCIE];
> > >  MemoryRegion *mmio_alias;

Re: [Qemu-devel] RFC: qemu RISC-V

[Paolo Bonzini]

On 23/08/2017 21:25, KONRAD Frederic wrote:
> Hi all,
> 
> I found somes slides about RISC-V at the KVM forum 2016.
> 
> Seems the upstreaming process should have started in september 2016 but
> I didn't see anything.
> 
> Is that still planed?

We're waiting from the privileged interface specification to become stable.

The linux-user emulation could go in now (for some value of now), but
the RISC-V people wanted to have the system emulation first.

Paolo

Re: [Qemu-devel] A use-after-free in slirp

[Samuel Thibault]

Hello,

P J P, on jeu. 03 août 2017 17:45:06 +0530, wrote:
> ==31922==ERROR: AddressSanitizer: heap-use-after-free on address 
> 0x6141ff8c at pc 0x56485de28ea0 bp 0x7f00f44fc950 sp 0x7f00f44fc940
> READ of size 4 at 0x6141ff8c thread T2
> #0 0x56485de28e9f in if_start slirp/if.c:230
> #1 0x56485de28a58 in if_output slirp/if.c:141
> #2 0x56485de35173 in ip_output slirp/ip_output.c:85
> #3 0x56485de57c48 in tcp_respond slirp/tcp_subr.c:218
> #4 0x56485de52440 in tcp_input slirp/tcp_input.c:1392
> #5 0x56485de329ef in ip_input slirp/ip_input.c:206
> #6 0x56485de3cf93 in slirp_input slirp/slirp.c:872
> #7 0x56485de0726d in net_slirp_receive net/slirp.c:119
> #8 0x56485ddee24d in nc_sendv_compat net/net.c:707
> #9 0x56485ddee3dd in qemu_deliver_packet_iov net/net.c:734
> #10 0x56485ddf422c in qemu_net_queue_deliver_iov net/queue.c:179
> ...

Please don't strip the output :)
The most interesting part is what exactly freed this.

> A full trace output can be seen
> 
>   here  -> https://paste.fedoraproject.org/paste/gh~hDctqUQ8uVt6UdG~zbg

The paste is not available any more. Is it really very large? It's
usually really better to just send it by mail, so it's archived in the
mailing list etc.

Samuel

Re: [Qemu-devel] Number of usable slots in PCIe Root Port / PCIe Switch Downstream Port

[Marcel Apfelbaum]

On 23/08/2017 19:46, Andrea Bolognani wrote:

AFAIK some PCI controllers, particularly:

   * PCIe Root Port (pcie-root-port, ioh3420)
   * PCIe Switch Downstream Port (xio3130-downstream)

only have a single usable slot. libvirt knows about this
fact, and will prevent you from adding more than one
device to the respective bus.

However, as Lukáš recently noticed, QEMU won't complain
if you add more devices:

   $ qemu-system-x86_64 \
 -nodefaults -nographic \
 -M q35 -monitor stdio \
 -device pcie-root-port,id=pci.1 \
 -device virtio-scsi-pci,bus=pci.1 \
 -device virtio-scsi-pci,bus=pci.1
   QEMU 2.9.0 monitor - type 'help' for more information
   (qemu) info qtree
   bus: main-system-bus
 type System
 [...]
 dev: q35-pcihost, id ""
   [...]
   bus: pcie.0
 type PCIE
 dev: pcie-root-port, id "pci.1"
   [...]
   bus: pci.1
 type PCIE
 dev: virtio-scsi-pci, id ""
   [...]
   addr = 01.0
   bus: virtio-bus
 type virtio-pci-bus
 dev: virtio-scsi-device, id ""
   [...]
   bus: scsi.1
 type SCSI
 dev: virtio-scsi-pci, id ""
   [...]
   addr = 00.0
   bus: virtio-bus
 type virtio-pci-bus
 dev: virtio-scsi-device, id ""
   [...]
   bus: scsi.0
 type SCSI
   (qemu)



Hi Andrea,


As you can see, all devices will show up in the qtree;
only the one with addr=00.0, however, will actually be
visible to the guest OS according to my tests.

Is such a configuration considered valid? 


Definitely no.


Should QEMU
complain loudly about it and refuse to start? 


Yes, but we need a clean way to do it. Eduardo is working
on a series that would expose to libvirt this info,
Eduardo can you please confirm?

Or should

libvirt and the guest OS / firmware start allowing it?



I thought libvirt does not allow it anyway and yes, is a bug,
but considered low priority.
I personally hoped Eduardo's work can also help in this direction
(classifying PCI Buses, we can add a 'rule' to allow only one
PCIe device per bus.)

But maybe a simple patch preventing QEMU to start would be enough.

Thanks for bringing the issue to my attention,
Marcel

Re: [Qemu-devel] [PATCH v2] audio: intel-hda: do not use old_mmio accesses

[Matt Parker]

I've just noticed the suggested changes from the v1 patch regarding
using MAKE_64BIT_MASK.
This patch is can be ignored for now.

On 23 August 2017 at 20:37, Matt Parker  wrote:
> intel-hda is currently using the old_mmio accessors for io.
> This updates the device to use .read and .write accessors instead.
>
> Signed-off-by: Matt Parker 
> ---
>  hw/audio/intel-hda.c | 57 
> +---
>  1 file changed, 10 insertions(+), 47 deletions(-)
>
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index 06acc98f7b..95e3e460fb 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -1043,66 +1043,29 @@ static void intel_hda_regs_reset(IntelHDAState *d)
>
>  /* - */
>
> -static void intel_hda_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
> +static void intel_hda_mmio_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned size)
>  {
>  IntelHDAState *d = opaque;
>  const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
>
> -intel_hda_reg_write(d, reg, val, 0xff);
> +intel_hda_reg_write(d, reg, val, (1UL << (size * 8)) - 1);
>  }
>
> -static void intel_hda_mmio_writew(void *opaque, hwaddr addr, uint32_t val)
> +static uint64_t intel_hda_mmio_read(void *opaque, hwaddr addr, unsigned size)
>  {
>  IntelHDAState *d = opaque;
>  const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
>
> -intel_hda_reg_write(d, reg, val, 0x);
> -}
> -
> -static void intel_hda_mmio_writel(void *opaque, hwaddr addr, uint32_t val)
> -{
> -IntelHDAState *d = opaque;
> -const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
> -
> -intel_hda_reg_write(d, reg, val, 0x);
> -}
> -
> -static uint32_t intel_hda_mmio_readb(void *opaque, hwaddr addr)
> -{
> -IntelHDAState *d = opaque;
> -const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
> -
> -return intel_hda_reg_read(d, reg, 0xff);
> -}
> -
> -static uint32_t intel_hda_mmio_readw(void *opaque, hwaddr addr)
> -{
> -IntelHDAState *d = opaque;
> -const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
> -
> -return intel_hda_reg_read(d, reg, 0x);
> -}
> -
> -static uint32_t intel_hda_mmio_readl(void *opaque, hwaddr addr)
> -{
> -IntelHDAState *d = opaque;
> -const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
> -
> -return intel_hda_reg_read(d, reg, 0x);
> +return intel_hda_reg_read(d, reg, (1UL << (size * 8)) - 1);
>  }
>
>  static const MemoryRegionOps intel_hda_mmio_ops = {
> -.old_mmio = {
> -.read = {
> -intel_hda_mmio_readb,
> -intel_hda_mmio_readw,
> -intel_hda_mmio_readl,
> -},
> -.write = {
> -intel_hda_mmio_writeb,
> -intel_hda_mmio_writew,
> -intel_hda_mmio_writel,
> -},
> +.read = intel_hda_mmio_read,
> +.write = intel_hda_mmio_write,
> +.impl = {
> +.min_access_size = 1,
> +.max_access_size = 4,
>  },
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  };
> --
> 2.13.2
>

Re: [Qemu-devel] [Qemu-block] [PATCH v5 08/13] tests: Rely more on global_qtest

[Paolo Bonzini]

On 23/08/2017 21:26, Eric Blake wrote:
>> I agree with using the short form where possible, but I disagree on
>> removing the long forms.  Rather, global_qtest in my opinion should have
>> been static (though I'm not proposing that you do this); inlining the
>> wrappers is not needed for performance.
> But no one outside of libqtest.c is using the long form.  Maintaining a
> long form that isn't used is counter-productive, if the short form is
> good enough for everything we need.

Well, whatever is assigning to global_qtest should be using the long form.

Paolo



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2] audio: intel-hda: do not use old_mmio accesses

[Matt Parker]

intel-hda is currently using the old_mmio accessors for io.
This updates the device to use .read and .write accessors instead.

Signed-off-by: Matt Parker 
---
 hw/audio/intel-hda.c | 57 +---
 1 file changed, 10 insertions(+), 47 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 06acc98f7b..95e3e460fb 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1043,66 +1043,29 @@ static void intel_hda_regs_reset(IntelHDAState *d)
 
 /* - */
 
-static void intel_hda_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
+static void intel_hda_mmio_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size)
 {
 IntelHDAState *d = opaque;
 const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
 
-intel_hda_reg_write(d, reg, val, 0xff);
+intel_hda_reg_write(d, reg, val, (1UL << (size * 8)) - 1);
 }
 
-static void intel_hda_mmio_writew(void *opaque, hwaddr addr, uint32_t val)
+static uint64_t intel_hda_mmio_read(void *opaque, hwaddr addr, unsigned size)
 {
 IntelHDAState *d = opaque;
 const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
 
-intel_hda_reg_write(d, reg, val, 0x);
-}
-
-static void intel_hda_mmio_writel(void *opaque, hwaddr addr, uint32_t val)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-intel_hda_reg_write(d, reg, val, 0x);
-}
-
-static uint32_t intel_hda_mmio_readb(void *opaque, hwaddr addr)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-return intel_hda_reg_read(d, reg, 0xff);
-}
-
-static uint32_t intel_hda_mmio_readw(void *opaque, hwaddr addr)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-return intel_hda_reg_read(d, reg, 0x);
-}
-
-static uint32_t intel_hda_mmio_readl(void *opaque, hwaddr addr)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-return intel_hda_reg_read(d, reg, 0x);
+return intel_hda_reg_read(d, reg, (1UL << (size * 8)) - 1);
 }
 
 static const MemoryRegionOps intel_hda_mmio_ops = {
-.old_mmio = {
-.read = {
-intel_hda_mmio_readb,
-intel_hda_mmio_readw,
-intel_hda_mmio_readl,
-},
-.write = {
-intel_hda_mmio_writeb,
-intel_hda_mmio_writew,
-intel_hda_mmio_writel,
-},
+.read = intel_hda_mmio_read,
+.write = intel_hda_mmio_write,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 4,
 },
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
-- 
2.13.2

Re: [Qemu-devel] [PATCH v2] qcow2: allocate cluster_cache/cluster_data on demand

[Eric Blake]

On 08/21/2017 08:55 AM, Stefan Hajnoczi wrote:
> Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
> * cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
> s->cluster_data when the first read operation is performance on a
> compressed cluster.
> 
> The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
> code paths that can allocate these buffers, so remove the free functions
> in the error code path.
> 
> This patch can result in significant memory savings when many qcow2
> disks are attached or backing file chains are long:
> 
> Before 12.81% (1,023,193,088B)
> After   5.36% (393,893,888B)
> 
> Reported-by: Alexey Kardashevskiy 
> Tested-by: Alexey Kardashevskiy 
> Cc: Kevin Wolf 
> Signed-off-by: Stefan Hajnoczi 
> ---

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] hw/misc/auxbus.c: Mark the aux-to-i2c-bridge device as non-hotpluggable

[KONRAD Frederic]

On 08/23/2017 07:22 AM, Thomas Huth wrote:

On 22.08.2017 20:15, KONRAD Frederic wrote:

Hi Thomas,

Looking to this seems there is a second issue:
The aux-to-i2c-bridge device should connect on a TYPE_AUX_BUS.

I don't think there isn't any on integratorcp..

Anyway the patch you sent fix this issue indirectly and as far as
I remember I wasn't able to make this I2C bridge an
TYPE_AUX_DEVICE as it's a special device and it is internal (only
instantiated at the bus creation).


If it is intended to be completely internal only (i.e. also not for use
with the "-device" parameter), then it should even be marked with
"user_creatable = false"! ... if you got some spare minutes, could you
maybe sent a patch for that? I'm not familiar with that code, so I'm
always having trouble to give the reasoning for setting a
"user_creatable = false" somewhere...

  Thanks,
   Thomas



Ok will do!

Thanks,
Fred

Re: [Qemu-devel] [Qemu-block] [PATCH v5 08/13] tests: Rely more on global_qtest

[Eric Blake]

On 08/19/2017 11:34 AM, Paolo Bonzini wrote:
> On 18/08/2017 23:33, John Snow wrote:
>>> Before the next few patches
>>> get rid of the qtest_* layer, we first need to update the remaining
>>> few spots that were using the long form where we can instead rely
>>> on the short form.
>>>
>> Not a big fan of globals and implicit state, but I do at least agree
>> that we don't need two sets of functions.
> 
> I agree with using the short form where possible, but I disagree on
> removing the long forms.  Rather, global_qtest in my opinion should have
> been static (though I'm not proposing that you do this); inlining the
> wrappers is not needed for performance.

But no one outside of libqtest.c is using the long form.  Maintaining a
long form that isn't used is counter-productive, if the short form is
good enough for everything we need.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] RFC: qemu RISC-V

[KONRAD Frederic]

Hi all,

I found somes slides about RISC-V at the KVM forum 2016.

Seems the upstreaming process should have started in september 
2016 but I didn't see anything.


Is that still planed?

Thanks,
Fred

[Qemu-devel] [PATCH] multiboot: make tests work with clang

[Anatol Pomozov]

 * explicitly disable SSE as clang enables it by default for 32bit code
 * add memset() implementation. Clang complains that the function is
   absent, gcc seems provides default built-in.
---
 tests/multiboot/Makefile | 2 +-
 tests/multiboot/libc.c   | 9 +
 tests/multiboot/libc.h   | 2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile
index 856e76682a..7fadbca33a 100644
--- a/tests/multiboot/Makefile
+++ b/tests/multiboot/Makefile
@@ -1,5 +1,5 @@
 CC=gcc
-CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin
+CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc -fno-builtin 
-mno-sse
 ASFLAGS=-m32
 
 LD=ld
diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c
index 6df9bda96d..512fccd7fa 100644
--- a/tests/multiboot/libc.c
+++ b/tests/multiboot/libc.c
@@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n)
 
 return dest;
 }
+void *memset(void *s, int c, size_t n)
+{
+size_t i;
+char *d = s;
+for (i = 0; i < n; i++) {
+*d++ = c;
+}
+return s;
+}
 
 static void print_char(char c)
 {
diff --git a/tests/multiboot/libc.h b/tests/multiboot/libc.h
index 04c9922c27..44b71350dd 100644
--- a/tests/multiboot/libc.h
+++ b/tests/multiboot/libc.h
@@ -36,6 +36,7 @@ typedef signed short int16_t;
 typedef signed char int8_t;
 
 typedef uint32_t uintptr_t;
+typedef uint32_t size_t;
 
 
 /* stdarg.h */
@@ -58,5 +59,6 @@ static inline void outb(uint16_t port, uint8_t data)
 
 void printf(const char *fmt, ...);
 void* memcpy(void *dest, const void *src, int n);
+void *memset(void *s, int c, size_t n);
 
 #endif
-- 
2.14.1.342.g6490525c54-goog

Re: [Qemu-devel] [RFC PATCH] osdep.h: Prohibit disabling assert() in supported builds

[Eric Blake]

On 08/22/2017 06:19 AM, Halil Pasic wrote:

> OTOH I do think this is to some degree institutionalizing a bad practice
> (you say we do not want to do that, but IMHO refusing to build with
> NDEBUG makes only sense if we want to alter the semantic of assert so
> that once bad becomes acceptable). I can live with that, but I'm not
> happy about it. Have we considered rolling our own construct which is
> designed to exhibit the properties we desire?

I've thought about it, and it may have even been discussed on the list
perhaps (although I didn't go searching to verify).

> 
> I mean, if it's about the side effects we could create something like
> q_assert(cond) and state that cond is evaluate exactly once (and
> depending on what we want to have, make the actions on !cond (calling
> abort(), producing a diagnostic message) compile time tweak-able or
> not). I assume we could then convert each usage of assert to the safe
> q_assert programmatically.

I'd prefer that if we are going to introduce our own construct that
always evaluates side effects, and only has a compile-time switch on
whether to abort() or (foolishly) plow on, that we name it something
without 'assert' in the name, so that reviewers don't have to be
confused about remembering which variant evaluates side effects.  Maybe:

q_verify(cond)

is a good name, which performs the side effects of 'cond' no matter
what, but also allows us to abort() if cond fails vs. ignore the
failure; perhaps where we make a compile-time decision between the two
behaviors as a configure --enable-FOO flag.

Converting all existing assert() and g_assert() to q_verify() should be
fairly simple, if we like the idea.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[ChristianEhrhardt]

Yeah seems to be slightly different than the former assert.

2017-08-23 18:41:54.556+: initiating migration
bdrv_inactivate_recurse: entry for drive-virtio-disk0
bdrv_inactivate_recurse: entry for #block133
bdrv_inactivate_recurse: entry for #block329
bdrv_inactivate_recurse: entry for #block202
bdrv_inactivate_recurse: exit end good for #block202
bdrv_inactivate_recurse: exit end good for #block329
bdrv_inactivate_recurse: entry for #block025
bdrv_inactivate_recurse: exit end good for #block025
bdrv_inactivate_recurse: exit end good for #block133
bdrv_inactivate_recurse: exit end good for drive-virtio-disk0
bdrv_inactivate_recurse: entry for #block799
bdrv_inactivate_recurse: entry for #block626
bdrv_inactivate_recurse: exit end good for #block626
bdrv_inactivate_recurse: exit end good for #block799
bdrv_inactivate_recurse: entry for drive-virtio-disk1
bdrv_inactivate_recurse: entry for #block570
bdrv_inactivate_recurse: entry for #block485
bdrv_inactivate_recurse: exit end good for #block485
bdrv_inactivate_recurse: exit end good for #block570
bdrv_inactivate_recurse: exit end good for drive-virtio-disk1
bdrv_inactivate_recurse: entry for #block1058
bdrv_inactivate_recurse: entry for #block920
bdrv_inactivate_recurse: exit end good for #block920
bdrv_inactivate_recurse: exit end good for #block1058
bdrv_inactivate_recurse: entry for drive-virtio-disk0
Unexpected error in bdrv_check_perm() at 
/build/qemu-0OVYHF/qemu-2.10~rc3+dfsg/block.c:1574:
2017-08-23T18:41:54.730131Z qemu-system-x86_64: Block node is read-only

Which is:
1553 /* 
  
1554  * Check whether permissions on this node can be changed in a way that 
  
1555  * @cumulative_perms and @cumulative_shared_perms are the new cumulative   
  
1556  * permissions of all its parents. This involves checking whether all 
necessary  
1557  * permission changes to child nodes can be performed. 
  
1558  * 
  
1559  * A call to this function must always be followed by a call to 
bdrv_set_perm()  
1560  * or bdrv_abort_perm_update().
  
1561  */
  
1562 static int bdrv_check_perm(BlockDriverState *bs, uint64_t 
cumulative_perms,  
1563uint64_t cumulative_shared_perms,   
  
1564GSList *ignore_children, Error **errp)  
  
1565 {  
  
1566 BlockDriver *drv = bs->drv;
  
1567 BdrvChild *c;  
  
1568 int ret;   
  
1569
  
1570 /* Write permissions never work with read-only images */   
  
1571 if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) 
&&  
1572 !bdrv_is_writable(bs)) 
  
1573 {  
  
1574 error_setg(errp, "Block node is read-only");   
  
1575 return -EPERM; 
  
1576 } 

Adding in debug symbols to see in gdb which device that actually is showed me:
I don't know what you might need so the full struct:

(gdb) p *bs
$2 = {open_flags = 2050, read_only = false, encrypted = false, sg = false, 
probed = false, force_share = false, implicit = true, 
  drv = 0x1a67219800 , opaque = 0x0, aio_context = 
0x1a684ae0d0, aio_notifiers = {lh_first = 0x1a6a4850e0}, 
  walking_aio_notifiers = false, filename = 
"/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow", '\000' , 
  backing_file = "/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow", 
'\000' , 
  backing_format = "qcow2\000\000\000\000\000\000\000\000\000\000", 
full_open_options = 0x0, 
  exact_filename = 
"/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow", '\000' , backing = 0x1a6971a4a0, 
  file = 0x0, bl = {request_alignment = 1, max_pdiscard = 0, pdiscard_alignment 
= 0, max_pwrite_zeroes = 0, pwrite_zeroes_alignment = 0, 
opt_transfer = 0, max_transfer = 0, min_mem_alignment = 512, 
opt_mem_alignment = 4096, max_iov = 1024}, supported_write_flags = 0, 
  supported_zero_flags = 0, node_name = "#block814", '\000' , 
node_list = {tqe_next = 0x1a684b44d0, tqe_prev = 0x1a6b02e0c0}, 
  bs_list = {tqe_next = 0x1a6a010030, tqe_prev = 0x1a6ab6bc50}, monitor_list = 
{tqe_next = 0x0, tqe_prev = 0x0}, refcnt = 3, op_blockers = {{
  lh_first = 0x1a69e18e80}, {lh_first = 

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[Dr. David Alan Gilbert]

just tested current head - 1eed33994e28d4a0437ba6e944bbc3ec5e4a29a0 -
seems to work for me.

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

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

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

Re: [Qemu-devel] [ANNOUNCE] QEMU 2.10.0-rc3 is now available

[Peter Maydell]

On 22 August 2017 at 17:50, Michael Roth  wrote:
> Quoting Peter Maydell (2017-08-22 11:20:18)
>> We've had a few late-breaking PPC host bugs that we need
>> to fix, so we'll tag rc3 either today or more likely tomorrow
>> and then have the final release next week.
>
> Ok, thanks for the heads up.

We also ended up with some very late fixes for bad NBD
bugs, which I've just applied to master. Since those
were diagnosed and fixed in a rather short timescale,
I've opted to not tag rc4 just yet, but instead give
it another 24 hours to make sure we're happy we really
did fix all the NBD bugs. Plan is to tag rc4 on Thursday
24th and final release next week (wednesday?)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 8/8] tcg/i386: Add vector operations

[Richard Henderson]

On 08/22/2017 06:15 AM, Alex Bennée wrote:
>> +#ifndef have_avx2
>> +/* There are a number of things we must check before we can be
>> +   sure of not hitting invalid opcode.  */
>> +if (c & bit_OSXSAVE) {
>> +unsigned xcrl, xcrh;
>> +asm ("xgetbv" : "=a" (xcrl), "=d" (xcrh) : "c" (0));
>> +if (xcrl & 6 == 6) {
> 
> My picky compiler complains:
> 
> /home/alex/lsrc/qemu/qemu.git/tcg/i386/tcg-target.inc.c: In function 
> ‘tcg_target_init’:
> /home/alex/lsrc/qemu/qemu.git/tcg/i386/tcg-target.inc.c:3053:22: error: 
> suggest parentheses around comparison in operand of ‘&’ [-Werror=parentheses]
>  if (xcrl & 6 == 6) {


Bah.  I forgot that my default build uses -march=native, and my laptop has
AVX2, so this bit wouldn't have been compile tested at all.

Fixed on the branch.


r~

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[Dr. David Alan Gilbert]

Looks good here, just retested:

here's teh top of my git:

f89f59fad5119f878aaedf711af90802ddcb99c7 nbd-client: avoid spurious 
qio_channel_yield() re-entry
cf26039a2b50f078b4ad90b88eea5bb28971c0d8 block: Update open_flags after 
->inactivate() callback
8ccc527d84ec9a5052cfae19edbc44abb5ac03ae mirror: Mark target BB as "force allow 
inactivate"
34c3f17c99a43f261560edbd3da1188dd0c398ab block-backend: Allow more "can 
inactivate" cases
952ad9fd9dd43e92016d5bfc0ff93bdeaec13bf9 block-backend: Refactor inactivate 
check
1f296733876434118fd766cfef5eb6f29ecab6a8 Update version for v2.10.0-rc3 release

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

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

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

Re: [Qemu-devel] [PULL 0/6] NBD pull request for 2.10-rc4

[Peter Maydell]

On 23 August 2017 at 17:33, Eric Blake  wrote:
> The following changes since commit 3da2bd8c4a80178fb115c70b858cecbfb1eb1067:
>
>   numa: Move numa_legacy_auto_assign_ram to pc-i440fx-2.9 (2017-08-23 
> 13:53:15 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-08-23
>
> for you to fetch changes up to 40f4a21895b5a7eae4011593837069f63460d983:
>
>   nbd-client: avoid spurious qio_channel_yield() re-entry (2017-08-23 
> 11:22:15 -0500)
>
> 
> nbd patches for 2017-08-23
>
> - Fam Zheng: 0/4 block: Fix non-shared storage migration
> - Stefan Hajnoczi: qemu-iotests: add 194 non-shared storage migration test
> - Stefan Hajnoczi: nbd-client: avoid spurious qio_channel_yield() re-entry
>
> 
> Fam Zheng (3):
>   block-backend: Refactor inactivate check
>   block-backend: Allow more "can inactivate" cases
>   mirror: Mark target BB as "force allow inactivate"
>
> Stefan Hajnoczi (3):
>   block: Update open_flags after ->inactivate() callback
>   qemu-iotests: add 194 non-shared storage migration test
>   nbd-client: avoid spurious qio_channel_yield() re-entry

Applied, thanks.

-- PMM

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[Vladimir Sementsov-Ogievskiy]

23.08.2017 21:04, Vladimir Sementsov-Ogievskiy wrote:

23.08.2017 11:59, Vladimir Sementsov-Ogievskiy wrote:

22.08.2017 22:07, John Snow wrote:

Well, we knew we'd want this sooner or later. I've got some pings
downstream over whether or not we support persistent bitmaps for
non-qcow2 formats.

Currently: no, we don't.

We tried two different ideas for storage agnostic bitmap persistence:


(1) Using qcow2 as a storage container format (similar to VM save
states) for information not associated with that particular drive.

This didn't go over so well; we decided it was too messy 
logistically to

manage data in any meaningful sense in a file that didn't share any
semantic relationship to the qcow2 in question.

Well, "We" decided is more like "Kevin and Max" decided. They are 
right,

though.


(2) Using a new proto-wrapper format that Fam Zheng designed that 
serves

as a simple top-layer redirect for metadata that allows us to associate
raw bitmap data with an arbitrary backing image.

This also didn't go over so well, the desire for a "new format" was
pretty thin, even if it was only a pseudo-format.


We'd still like to be able to use bitmaps with non-qcow2 formats
however, and people are going to keep asking. So here's a third 
thought:



(3) Add either a new flag that turns qcow2's backing file into a full
R/W backing file, or add a new extension to qcow2 entirely (bypassing
the traditional backing file mechanism to avoid confusion for older
tooling) that adds a new read-write backing file field.

This RW backing file field will be used for all reads AND writes; the
qcow2 in question becomes a metadata container on top of the BDS chain.
We can re-use Vladimir's bitmap persistence extension to save 
bitmaps in

a qcow2 shell.

The qcow2 becomes effectively a metadata cache for a new (essentially)
filter node that handles features such as bitmaps. This could also be
used to provide allocation map data for RAW files and other goodies 
down

the road.

Hopefully this achieves our desire to not create new formats AND our
desire to concentrate features (and debugging, testing, etc) into 
qcow2,

while allowing users to "have bitmaps with raw files."

Of course, in this scenario, users now have two files: a qcow2 wrapper
and the actual raw file in question; but regardless of how we were 
going

to solve this, a raw file necessitates an external file of some sort,
else we give up the idea that it was a raw file.


Sound good?


Looks interesting. It is a clean reusing of qcow2-bitmaps without any 
modifications to them.


Should there be some problems with internal snapshots and other things?



Hm. looks like that this backing file should not only receive all 
reads and writes, but almost everything ->bdrv_ handlers, except 
bitmap related of course. This doesn't seems simple to implement. 
Especially if imaging some not-raw feature-full format under this thin 
qcow2 layer.. Or we can restrict this RW backing file to be raw-only?





So, anyway, I see only two differences (from the outside) between this 
approach and just a separate bitmap-only qcow2 without a data:


1. in RW-backing approach qcow2-bitmap file has a link to data file (as 
a backing). It looks good.


2. in RW-backing approach qcow2-bitmap file is a top of the virtual 
disk, in separate-file approach it is an option of the real data drive. 
In my opinion the second is more clean for users ("to add this feature 
you should use other file as your disk" vs "to add this feature you 
should add an option to your disk description")



I think (may be I'm not right) loading bitmaps from additional 
qcow2-only-bitmaps file is simpler to implement (it will be specified in 
command line in drive options, like bitmaps_file=/path/to/it and then 
attached directly to BlockDriverState). The only drawback of simple 
qcow2-bitmap file  is that it has not a link inside it to the data file 
(like shabacking). We can ignore it, or we can implement this link as a 
separate extension to qcow2.


--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/3] multiboot: load elf sections and section headers

[Anatol Pomozov]

Hi

On Wed, Aug 23, 2017 at 10:22 AM, Anatol Pomozov
 wrote:
> Multiboot may load section headers and all sections (even those that are
> not part of any segment) to target memory.
>
> Tested with an ELF application that uses data from strings table
> section.
>
> Signed-off-by: Anatol Pomozov 
> ---
>  hw/core/loader.c |   8 ++--
>  hw/i386/multiboot.c  |  83 +
>  hw/s390x/ipl.c   |   2 +-
>  include/hw/elf_ops.h | 107 
> ++-
>  include/hw/loader.h  |  11 -
>  tests/multiboot/Makefile |   5 +-
>  tests/multiboot/modules.out  |  22 -
>  tests/multiboot/run_test.sh  |   6 ++-
>  tests/multiboot/sections.c   |  55 ++
>  tests/multiboot/sections.out |  21 +
>  tests/multiboot/start.S  |   2 +-
>  11 files changed, 261 insertions(+), 61 deletions(-)
>  create mode 100644 tests/multiboot/sections.c
>  create mode 100644 tests/multiboot/sections.out
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index ebe574c7ea..c7e3608e7a 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -439,7 +439,7 @@ int load_elf_as(const char *filename,
>  {
>  return load_elf_ram(filename, translate_fn, translate_opaque,
>  pentry, lowaddr, highaddr, big_endian, elf_machine,
> -clear_lsb, data_swab, as, true);
> +clear_lsb, data_swab, as, true, NULL);
>  }
>
>  /* return < 0 if error, otherwise the number of bytes loaded in memory */
> @@ -448,7 +448,7 @@ int load_elf_ram(const char *filename,
>   void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
>   uint64_t *highaddr, int big_endian, int elf_machine,
>   int clear_lsb, int data_swab, AddressSpace *as,
> - bool load_rom)
> + bool load_rom, struct sections_data *sections)
>  {
>  int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
>  uint8_t e_ident[EI_NIDENT];
> @@ -488,11 +488,11 @@ int load_elf_ram(const char *filename,
>  if (e_ident[EI_CLASS] == ELFCLASS64) {
>  ret = load_elf64(filename, fd, translate_fn, translate_opaque, 
> must_swab,
>   pentry, lowaddr, highaddr, elf_machine, clear_lsb,
> - data_swab, as, load_rom);
> + data_swab, as, load_rom, sections);
>  } else {
>  ret = load_elf32(filename, fd, translate_fn, translate_opaque, 
> must_swab,
>   pentry, lowaddr, highaddr, elf_machine, clear_lsb,
> - data_swab, as, load_rom);
> + data_swab, as, load_rom, sections);
>  }
>
>   fail:
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index a49217ac62..9ced0bfe30 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -125,7 +125,7 @@ int load_multiboot(FWCfgState *fw_cfg,
>  {
>  int i;
>  bool is_multiboot = false;
> -uint32_t flags = 0;
> +uint32_t flags = 0, bootinfo_flags = 0;
>  uint32_t mh_entry_addr;
>  uint32_t mh_load_addr;
>  uint32_t mb_kernel_size;
> @@ -134,6 +134,7 @@ int load_multiboot(FWCfgState *fw_cfg,
>  uint8_t *mb_bootinfo_data;
>  uint32_t cmdline_len;
>  struct multiboot_header *multiboot_header;
> +struct sections_data sections;
>
>  /* Ok, let's see if it is a multiboot image.
> The header is 12x32bit long, so the latest entry may be 8192 - 48. */
> @@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
>  if (flags & MULTIBOOT_VIDEO_MODE) {
>  fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
>  }
> -if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
> -uint64_t elf_entry;
> -uint64_t elf_low, elf_high;
> -int kernel_size;
> -fclose(f);
>
> -if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
> -fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
> -exit(1);
> -}
> -
> -kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
> -   _low, _high, 0, EM_NONE,
> -   0, 0);
> -if (kernel_size < 0) {
> -fprintf(stderr, "Error while loading elf kernel\n");
> -exit(1);
> -}
> -mh_load_addr = elf_low;
> -mb_kernel_size = elf_high - elf_low;
> -mh_entry_addr = elf_entry;
> -
> -mbs.mb_buf = g_malloc(mb_kernel_size);
> -if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != 
> mb_kernel_size) {
> -fprintf(stderr, "Error while fetching elf kernel from rom\n");
> -exit(1);
> -}
> -
> -mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
> %#zx\n",
> -  

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[Dr. David Alan Gilbert]

I need to recheck with that combo - I'd seen that error but only when
I'd commented out 'if (!blk->dev && !blk_name(blk)[0]) {'  when
debugging earlier.

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

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

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

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[Vladimir Sementsov-Ogievskiy]

23.08.2017 11:59, Vladimir Sementsov-Ogievskiy wrote:

22.08.2017 22:07, John Snow wrote:

Well, we knew we'd want this sooner or later. I've got some pings
downstream over whether or not we support persistent bitmaps for
non-qcow2 formats.

Currently: no, we don't.

We tried two different ideas for storage agnostic bitmap persistence:


(1) Using qcow2 as a storage container format (similar to VM save
states) for information not associated with that particular drive.

This didn't go over so well; we decided it was too messy logistically to
manage data in any meaningful sense in a file that didn't share any
semantic relationship to the qcow2 in question.

Well, "We" decided is more like "Kevin and Max" decided. They are right,
though.


(2) Using a new proto-wrapper format that Fam Zheng designed that serves
as a simple top-layer redirect for metadata that allows us to associate
raw bitmap data with an arbitrary backing image.

This also didn't go over so well, the desire for a "new format" was
pretty thin, even if it was only a pseudo-format.


We'd still like to be able to use bitmaps with non-qcow2 formats
however, and people are going to keep asking. So here's a third thought:


(3) Add either a new flag that turns qcow2's backing file into a full
R/W backing file, or add a new extension to qcow2 entirely (bypassing
the traditional backing file mechanism to avoid confusion for older
tooling) that adds a new read-write backing file field.

This RW backing file field will be used for all reads AND writes; the
qcow2 in question becomes a metadata container on top of the BDS chain.
We can re-use Vladimir's bitmap persistence extension to save bitmaps in
a qcow2 shell.

The qcow2 becomes effectively a metadata cache for a new (essentially)
filter node that handles features such as bitmaps. This could also be
used to provide allocation map data for RAW files and other goodies down
the road.

Hopefully this achieves our desire to not create new formats AND our
desire to concentrate features (and debugging, testing, etc) into qcow2,
while allowing users to "have bitmaps with raw files."

Of course, in this scenario, users now have two files: a qcow2 wrapper
and the actual raw file in question; but regardless of how we were going
to solve this, a raw file necessitates an external file of some sort,
else we give up the idea that it was a raw file.


Sound good?


Looks interesting. It is a clean reusing of qcow2-bitmaps without any 
modifications to them.


Should there be some problems with internal snapshots and other things?



Hm. looks like that this backing file should not only receive all reads 
and writes, but almost everything ->bdrv_ handlers, except bitmap 
related of course. This doesn't seems simple to implement. Especially if 
imaging some not-raw feature-full format under this thin qcow2 layer.. 
Or we can restrict this RW backing file to be raw-only?



--
Best regards,
Vladimir

Re: [Qemu-devel] [RFC v2 8/8] migration: add incoming mgmt lock

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Now at least migrate_incoming can be run in parallel.  Let's provide a
> migration lock to protect it.
> 
> Signed-off-by: Peter Xu 
> ---
>  migration/migration.c | 6 ++
>  migration/migration.h | 3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index c3fe0ed..32058f7 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -145,6 +145,7 @@ MigrationIncomingState 
> *migration_incoming_get_current(void)
>  mis_current.state = MIGRATION_STATUS_NONE;
>  memset(_current, 0, sizeof(MigrationIncomingState));
>  qemu_mutex_init(_current.rp_mutex);
> +qemu_mutex_init(_current.mgmt_mutex);
>  qemu_event_init(_current.main_thread_load_event, false);
>  once = true;
>  }
> @@ -1171,6 +1172,7 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
>  {
>  Error *local_err = NULL;
>  static bool once = true;
> +MigrationIncomingState *mis = migration_incoming_get_current();

migration_incoming_get_current isn't actually thread-safe itself unless
you can guarantee the initial allocation has happened - otherwise both
threads can race and do the 'once' code at the same time.

Similarly, these locks - they don't protect our 'once' - so a second
thread could come in here and both get past the !once check.

Dave

>  
>  if (!deferred_incoming) {
>  error_setg(errp, "For use with '-incoming defer'");
> @@ -1180,8 +1182,12 @@ void qmp_migrate_incoming(const char *uri, Error 
> **errp)
>  error_setg(errp, "The incoming migration has already been started");
>  }
>  
> +qemu_mutex_lock(>mgmt_mutex);
> +
>  qemu_start_incoming_migration(uri, _err);
>  
> +qemu_mutex_unlock(>mgmt_mutex);
> +
>  if (local_err) {
>  error_propagate(errp, local_err);
>  return;
> diff --git a/migration/migration.h b/migration/migration.h
> index 148c9fa..95f077b 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -58,6 +58,9 @@ struct MigrationIncomingState {
>  /* The coroutine we should enter (back) after failover */
>  Coroutine *migration_incoming_co;
>  QemuSemaphore colo_incoming_sem;
> +
> +/* Migration incoming management lock */
> +QemuMutex mgmt_mutex;
>  };
>  
>  MigrationIncomingState *migration_incoming_get_current(void);
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false

[Eduardo Habkost]

On Wed, Aug 23, 2017 at 06:51:28PM +0100, Peter Maydell wrote:
> On 23 August 2017 at 18:21, Eduardo Habkost  wrote:
> > On Tue, Aug 22, 2017 at 05:46:29PM +0200, Thomas Huth wrote:
> >> QEMU currently exits unexpectedly when the user accidentially
> >> tries to do something like this:
> >>
> >> $ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
> >> QEMU 2.9.93 monitor - type 'help' for more information
> >> (qemu) device_add allwinner-a10
> >> Unsupported NIC model: smc91c111
> >>
> >> Exiting just due to a "device_add" should not happen. Looking closer
> >> at the the realize and instance_init function of this device also
> >> reveals that it is using serial_hds and nd_table directly there, so
> >> this device is clearly not creatable by the user and should be marked
> >> accordingly.
> >>
> >> Signed-off-by: Thomas Huth 
> >> ---
> >>  hw/arm/allwinner-a10.c| 2 ++
> >>  scripts/device-crash-test | 1 -
> >>  2 files changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> >> index f62a9a3..43a3f01 100644
> >> --- a/hw/arm/allwinner-a10.c
> >> +++ b/hw/arm/allwinner-a10.c
> >> @@ -118,6 +118,8 @@ static void aw_a10_class_init(ObjectClass *oc, void 
> >> *data)
> >>  DeviceClass *dc = DEVICE_CLASS(oc);
> >>
> >>  dc->realize = aw_a10_realize;
> >> +/* Reason: Uses serial_hds in realize and nd_table in instance_init */
> >> +dc->user_creatable = false;
> >
> > I assume this patch will be replaced by one changing TYPE_DEVICE
> > to default to user_creatable=false, based on the replies to the
> > hw/misc/auxbus.c patch?
> 
> user-creatable and hotplug are different things -- most
> devices are user-creatable, many fewer are hotpluggable.
> So the tradeoff for the default doesn't *necessarily* go
> the same way.

You're right, I mixed things up when reading this patch.

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 18/27] vhost-user-scsi: rename VUS types

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

- use Vus prefix consistently
- use CamelCase, since that's glib & libvhost-user style
- avoid _t postfix, usually for system headers

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 46 +++
  1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 882d887a32..2ceca964f4 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -55,18 +55,18 @@
  
  #define VUS_ISCSI_INITIATOR "iqn.2016-11.com.nutanix:vhost-user-scsi"
  
-typedef struct iscsi_lun {

+typedef struct VusIscsiLun {
  struct iscsi_context *iscsi_ctx;
  int iscsi_lun;
-} iscsi_lun_t;
+} VusIscsiLun;
  
-typedef struct vhost_scsi_dev {

+typedef struct VusDev {
  VuDev vu_dev;
  int server_sock;
  GMainLoop *loop;
  GTree *fdmap;   /* fd -> gsource context id */
-iscsi_lun_t lun;
-} vhost_scsi_dev_t;
+VusIscsiLun lun;
+} VusDev;
  
  /** glib event loop integration for libvhost-user and misc callbacks **/
  
@@ -78,7 +78,7 @@ QEMU_BUILD_BUG_ON((int)G_IO_HUP != (int)VU_WATCH_HUP);
  
  typedef struct vus_gsrc {

  GSource parent;
-vhost_scsi_dev_t *vdev_scsi;
+VusDev *vdev_scsi;
  GPollFD gfd;
  vu_watch_cb vu_cb;
  } vus_gsrc_t;
@@ -107,7 +107,7 @@ static gboolean vus_gsrc_check(GSource *src)
  
  static gboolean vus_gsrc_dispatch(GSource *src, GSourceFunc cb, gpointer data)

  {
-vhost_scsi_dev_t *vdev_scsi;
+VusDev *vdev_scsi;
  vus_gsrc_t *vus_src = (vus_gsrc_t *)src;
  
  assert(vus_src);

@@ -133,7 +133,7 @@ static GSourceFuncs vus_gsrc_funcs = {
  NULL
  };
  
-static void vus_gsrc_new(vhost_scsi_dev_t *vdev_scsi, int fd, GIOCondition cond,

+static void vus_gsrc_new(VusDev *vdev_scsi, int fd, GIOCondition cond,
   vu_watch_cb vu_cb, gpointer data)
  {
  GSource *vus_gsrc;
@@ -245,7 +245,7 @@ struct scsi_task {
  
  /** libiscsi integration **/
  
-static int iscsi_add_lun(iscsi_lun_t *lun, char *iscsi_uri)

+static int iscsi_add_lun(VusIscsiLun *lun, char *iscsi_uri)
  {
  struct iscsi_url *iscsi_url;
  struct iscsi_context *iscsi_ctx;
@@ -415,11 +415,11 @@ static int handle_cmd_sync(struct iscsi_context *ctx,
  
  static void vus_panic_cb(VuDev *vu_dev, const char *buf)

  {
-vhost_scsi_dev_t *vdev_scsi;
+VusDev *vdev_scsi;
  
  assert(vu_dev);
  
-vdev_scsi = container_of(vu_dev, vhost_scsi_dev_t, vu_dev);

+vdev_scsi = container_of(vu_dev, VusDev, vu_dev);
  if (buf) {
  PERR("vu_panic: %s", buf);
  }
@@ -430,14 +430,14 @@ static void vus_panic_cb(VuDev *vu_dev, const char *buf)
  static void vus_add_watch_cb(VuDev *vu_dev, int fd, int vu_evt, vu_watch_cb 
cb,
   void *pvt)
  {
-vhost_scsi_dev_t *vdev_scsi;
+VusDev *vdev_scsi;
  guint id;
  
  assert(vu_dev);

  assert(fd >= 0);
  assert(cb);
  
-vdev_scsi = container_of(vu_dev, vhost_scsi_dev_t, vu_dev);

+vdev_scsi = container_of(vu_dev, VusDev, vu_dev);
  id = (guint)(uintptr_t)g_tree_lookup(vdev_scsi->fdmap,
   (gpointer)(uintptr_t)fd);
  if (id) {
@@ -452,13 +452,13 @@ static void vus_add_watch_cb(VuDev *vu_dev, int fd, int 
vu_evt, vu_watch_cb cb,
  
  static void vus_del_watch_cb(VuDev *vu_dev, int fd)

  {
-vhost_scsi_dev_t *vdev_scsi;
+VusDev *vdev_scsi;
  guint id;
  
  assert(vu_dev);

  assert(fd >= 0);
  
-vdev_scsi = container_of(vu_dev, vhost_scsi_dev_t, vu_dev);

+vdev_scsi = container_of(vu_dev, VusDev, vu_dev);
  id = (guint)(uintptr_t)g_tree_lookup(vdev_scsi->fdmap,
   (gpointer)(uintptr_t)fd);
  if (id) {
@@ -471,12 +471,12 @@ static void vus_del_watch_cb(VuDev *vu_dev, int fd)
  
  static void vus_proc_req(VuDev *vu_dev, int idx)

  {
-vhost_scsi_dev_t *vdev_scsi;
+VusDev *vdev_scsi;
  VuVirtq *vq;
  
  assert(vu_dev);
  
-vdev_scsi = container_of(vu_dev, vhost_scsi_dev_t, vu_dev);

+vdev_scsi = container_of(vu_dev, VusDev, vu_dev);
  if (idx < 0 || idx >= VHOST_MAX_NR_VIRTQUEUE) {
  PERR("VQ Index out of range: %d", idx);
  vus_panic_cb(vu_dev, NULL);
@@ -617,7 +617,7 @@ fail:
  
  /** vhost-user-scsi **/
  
-static void vdev_scsi_free(vhost_scsi_dev_t *vdev_scsi)

+static void vdev_scsi_free(VusDev *vdev_scsi)
  {
  if (vdev_scsi->server_sock >= 0) {
  close(vdev_scsi->server_sock);
@@ -627,13 +627,13 @@ static void vdev_scsi_free(vhost_scsi_dev_t *vdev_scsi)
  g_free(vdev_scsi);
  }
  
-static vhost_scsi_dev_t *vdev_scsi_new(int server_sock)

+static VusDev *vdev_scsi_new(int server_sock)
  {
-vhost_scsi_dev_t *vdev_scsi;
+VusDev 

Re: [Qemu-devel] [PATCH for-2.11 06/27] x86: extract legacy cpu features format parser

[Eduardo Habkost]

On Wed, Aug 23, 2017 at 07:37:39PM +0200, Igor Mammedov wrote:
> On Wed, 23 Aug 2017 13:46:38 -0300
> Eduardo Habkost  wrote:
> 
> > On Wed, Aug 23, 2017 at 06:29:02PM +0200, Igor Mammedov wrote:
> > > On Wed, 23 Aug 2017 11:34:14 -0300
> > > Eduardo Habkost  wrote:
> > > 
> > > > On Fri, Aug 18, 2017 at 12:08:38PM +0200, Igor Mammedov wrote:
> > > > > Move cpu_model +-feat parsing into a separate file so that it
> > > > > could be reused later for parsing similar format of sparc target
> > > > > 
> > > > > Signed-off-by: Igor Mammedov 
> > > > > ---
> > > > > CC: Richard Henderson 
> > > > > CC: Eduardo Habkost 
> > > > > CC: Mark Cave-Ayland 
> > > > > CC: Artyom Tarasenko 
> > > > > CC: Philippe Mathieu-Daudé 
> > > > > ---
> > > > >  include/qom/cpu.h |   2 +
> > > > >  default-configs/i386-bsd-user.mak |   1 +
> > > > >  default-configs/i386-linux-user.mak   |   1 +
> > > > >  default-configs/i386-softmmu.mak  |   1 +
> > > > >  default-configs/x86_64-bsd-user.mak   |   1 +
> > > > >  default-configs/x86_64-linux-user.mak |   1 +
> > > > >  default-configs/x86_64-softmmu.mak|   1 +
> > > > >  target/i386/cpu.c | 125 
> > > > > +-
> > > > >  util/Makefile.objs|   1 +
> > > > >  util/legacy_cpu_features_parser.c | 161 
> > > > > ++
> > > > >  10 files changed, 171 insertions(+), 124 deletions(-)
> > > > >  create mode 100644 util/legacy_cpu_features_parser.c
> > > > >   
> > > > [...]
> > > > > diff --git a/util/legacy_cpu_features_parser.c 
> > > > > b/util/legacy_cpu_features_parser.c
> > > > > new file mode 100644
> > > > > index 000..6b352a3
> > > > > --- /dev/null
> > > > > +++ b/util/legacy_cpu_features_parser.c
> > > > > @@ -0,0 +1,161 @@
> > > > > +/* Support for legacy -cpu cpu,features CLI option with +-feat 
> > > > > syntax,
> > > > > + * used by x86/sparc targets
> > > > > + *
> > > > > + * Author: Andreas Färber 
> > > > > + * Author: Andre Przywara 
> > > > > + * Author: Eduardo Habkost 
> > > > > + * Author: Igor Mammedov 
> > > > > + * Author: Paolo Bonzini 
> > > > > + * Author: Markus Armbruster   
> > > > 
> > > > IANAL, but I believe a
> > > >   Copyright (c)  
> > > > line is needed here.
> > > > 
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or 
> > > > > modify
> > > > > + * it under the terms of the GNU General Public License as published 
> > > > > by
> > > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > > + * (at your option) any later version.
> > > > > +
> > > > > + * This program is distributed in the hope that it will be useful,
> > > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > > + * GNU General Public License for more details.
> > > > > +
> > > > > + * You should have received a copy of the GNU General Public License 
> > > > > along
> > > > > + * with this program; if not, see .
> > > > > + */
> > > > > +
> > > > > +#include "qemu/osdep.h"
> > > > > +#include "qapi/error.h"
> > > > > +#include "qemu/cutils.h"
> > > > > +#include "qom/cpu.h"
> > > > > +#include "qemu/error-report.h"
> > > > > +#include "hw/qdev-properties.h"
> > > > > +
> > > > > +static inline void feat2prop(char *s)
> > > > > +{
> > > > > +while ((s = strchr(s, '_'))) {
> > > > > +*s = '-';
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +static gint compare_string(gconstpointer a, gconstpointer b)
> > > > > +{
> > > > > +return g_strcmp0(a, b);
> > > > > +}
> > > > > +
> > > > > +static void
> > > > > +cpu_add_feat_as_prop(const char *typename, const char *name, const 
> > > > > char *val)
> > > > > +{
> > > > > +GlobalProperty *prop = g_new0(typeof(*prop), 1);
> > > > > +prop->driver = typename;
> > > > > +prop->property = g_strdup(name);
> > > > > +prop->value = g_strdup(val);
> > > > > +prop->errp = _fatal;
> > > > > +qdev_prop_register_global(prop);
> > > > > +}
> > > > > +
> > > > > +/* DO NOT USE WITH NEW CODE
> > > > > + * Parse "+feature,-feature,feature=foo" CPU feature string
> > > > > + */
> > > > > +void cpu_legacy_parse_featurestr(const char *typename, char 
> > > > > *features,
> > > > > + Error **errp)
> > > > > +{
> > > > > +/* Compatibily hack to maintain legacy +-feat semantic,
> > > > > + * where +-feat overwrites any feature set by
> > > > > + * feat=on|feat even if the later is parsed after +-feat
> > > > > + * (i.e. "-x2apic,x2apic=on" will result in 

Re: [Qemu-devel] [PATCH 15/27] vhost-user-scsi: remove vdev_scsi_add_iscsi_lun()

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 16 +---
  1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 988417393f..f6f9e7b192 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -658,20 +658,6 @@ static vhost_scsi_dev_t *vdev_scsi_new(int server_sock)
  return vdev_scsi;
  }
  
-static int vdev_scsi_add_iscsi_lun(vhost_scsi_dev_t *vdev_scsi,

-   char *iscsi_uri, uint32_t lun)
-{
-assert(vdev_scsi);
-assert(iscsi_uri);
-assert(lun < VUS_MAX_LUNS);
-
-if (iscsi_add_lun(_scsi->luns[lun], iscsi_uri) != 0) {
-return -1;
-}
-
-return 0;
-}
-
  static int vdev_scsi_run(vhost_scsi_dev_t *vdev_scsi)
  {
  GIOChannel *chan;
@@ -736,7 +722,7 @@ int main(int argc, char **argv)
  }
  vdev_scsi = vdev_scsi_new(sock);
  
-if (vdev_scsi_add_iscsi_lun(vdev_scsi, iscsi_uri, 0) != 0) {

+if (iscsi_add_lun(_scsi->luns[0], iscsi_uri) != 0) {
  goto err;
  }
  

Re: [Qemu-devel] [PATCH 14/27] vhost-user-scsi: assert() in iscsi_add_lun()

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

Instead of a preliminary check, add an assert to the function that has
the pre-condition.

Signed-off-by: Marc-André Lureau 
---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index b40009e234..988417393f 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -255,6 +255,7 @@ static int iscsi_add_lun(iscsi_lun_t *lun, char *iscsi_uri)
  
  assert(lun);

  assert(iscsi_uri);
+assert(!lun->iscsi_ctx);
  
  iscsi_ctx = iscsi_create_context(VUS_ISCSI_INITIATOR);

  if (!iscsi_ctx) {
@@ -664,11 +665,6 @@ static int vdev_scsi_add_iscsi_lun(vhost_scsi_dev_t 
*vdev_scsi,
  assert(iscsi_uri);
  assert(lun < VUS_MAX_LUNS);
  
-if (vdev_scsi->luns[lun].iscsi_ctx) {

-PERR("Lun %d already configured", lun);
-return -1;
-}


the error message is lost :/

anyway:
Reviewed-by: Philippe Mathieu-Daudé 


-
  if (iscsi_add_lun(_scsi->luns[lun], iscsi_uri) != 0) {
  return -1;
  }

Re: [Qemu-devel] [PATCH 07/27] vhost-user-scsi: use glib allocation

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

Use g_new/g_free instead of plain malloc. This simplify a bit memory
handling since glib will abort if it cannot allocate.

Signed-off-by: Marc-André Lureau 
---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 35 ---
  1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 8d6ff18cc4..f3fc8c23c6 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -312,12 +312,7 @@ static struct scsi_task *scsi_task_new(int cdb_len, 
uint8_t *cdb, int dir,
  assert(cdb_len > 0);
  assert(cdb);
  
-task = calloc(1, sizeof(struct scsi_task));

-if (!task) {
-PERR("Error allocating task: %s", strerror(errno));
-return NULL;
-}
-
+task = g_new0(struct scsi_task, 1);
  memcpy(task->cdb, cdb, cdb_len);
  task->cdb_size = cdb_len;
  task->xfer_dir = dir;
@@ -393,10 +388,6 @@ static int handle_cmd_sync(struct iscsi_context *ctx,
  }
  
  task = scsi_task_new(cdb_len, req->cdb, dir, len);

-if (!task) {
-PERR("Unable to create iscsi task");
-return -1;
-}
  
  if (dir == SCSI_XFER_TO_DEV) {

  task->iovector_out.iov = (struct scsi_iovec *)out;
@@ -410,7 +401,7 @@ static int handle_cmd_sync(struct iscsi_context *ctx,
   cdb_len, dir, task);
  if (!iscsi_scsi_command_sync(ctx, 0, task, NULL)) {
  PERR("Error serving SCSI command");
-free(task);
+g_free(task);
  return -1;
  }
  
@@ -425,7 +416,7 @@ static int handle_cmd_sync(struct iscsi_context *ctx,

  memcpy(rsp->sense, >datain.data[2], rsp->sense_len);
  }
  
-free(task);

+g_free(task);
  
  PDBG("Filled in rsp: status=%hhX, resid=%u, response=%hhX, sense_len=%u",

   rsp->status, rsp->resid, rsp->response, rsp->sense_len);
@@ -692,7 +683,7 @@ static vhost_scsi_dev_t *vdev_scsi_find_by_vu(VuDev *vu_dev)
  return NULL;
  }
  
-static void vdev_scsi_deinit(vhost_scsi_dev_t *vdev_scsi)

+static void vdev_scsi_free(vhost_scsi_dev_t *vdev_scsi)
  {
  if (!vdev_scsi) {
  return;
@@ -716,20 +707,16 @@ static void vdev_scsi_deinit(vhost_scsi_dev_t *vdev_scsi)
  g_main_loop_unref(vdev_scsi->loop);
  vdev_scsi->loop = NULL;
  }
+g_free(vdev_scsi);


Reviewed-by: Philippe Mathieu-Daudé 


  }
  
  static vhost_scsi_dev_t *vdev_scsi_new(int server_sock)

  {
-vhost_scsi_dev_t *vdev_scsi = NULL;
+vhost_scsi_dev_t *vdev_scsi;
  
  assert(server_sock >= 0);
  
-vdev_scsi = calloc(1, sizeof(vhost_scsi_dev_t));

-if (!vdev_scsi) {
-PERR("calloc: %s", strerror(errno));
-return NULL;
-}
-
+vdev_scsi = g_new0(vhost_scsi_dev_t, 1);
  vdev_scsi->server_sock = server_sock;
  vdev_scsi->loop = g_main_loop_new(NULL, FALSE);
  if (!vdev_scsi->loop) {
@@ -746,8 +733,7 @@ static vhost_scsi_dev_t *vdev_scsi_new(int server_sock)
  return vdev_scsi;
  
  err:

-vdev_scsi_deinit(vdev_scsi);
-free(vdev_scsi);
+vdev_scsi_free(vdev_scsi);
  
  return NULL;

  }
@@ -854,10 +840,7 @@ int main(int argc, char **argv)
  }
  
  out:

-if (vdev_scsi) {
-vdev_scsi_deinit(vdev_scsi);
-free(vdev_scsi);
-}
+vdev_scsi_free(vdev_scsi);
  g_free(unix_fn);
  g_free(iscsi_uri);
  

Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable = false

[Peter Maydell]

On 23 August 2017 at 18:21, Eduardo Habkost  wrote:
> On Tue, Aug 22, 2017 at 05:46:29PM +0200, Thomas Huth wrote:
>> QEMU currently exits unexpectedly when the user accidentially
>> tries to do something like this:
>>
>> $ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
>> QEMU 2.9.93 monitor - type 'help' for more information
>> (qemu) device_add allwinner-a10
>> Unsupported NIC model: smc91c111
>>
>> Exiting just due to a "device_add" should not happen. Looking closer
>> at the the realize and instance_init function of this device also
>> reveals that it is using serial_hds and nd_table directly there, so
>> this device is clearly not creatable by the user and should be marked
>> accordingly.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  hw/arm/allwinner-a10.c| 2 ++
>>  scripts/device-crash-test | 1 -
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>> index f62a9a3..43a3f01 100644
>> --- a/hw/arm/allwinner-a10.c
>> +++ b/hw/arm/allwinner-a10.c
>> @@ -118,6 +118,8 @@ static void aw_a10_class_init(ObjectClass *oc, void 
>> *data)
>>  DeviceClass *dc = DEVICE_CLASS(oc);
>>
>>  dc->realize = aw_a10_realize;
>> +/* Reason: Uses serial_hds in realize and nd_table in instance_init */
>> +dc->user_creatable = false;
>
> I assume this patch will be replaced by one changing TYPE_DEVICE
> to default to user_creatable=false, based on the replies to the
> hw/misc/auxbus.c patch?

user-creatable and hotplug are different things -- most
devices are user-creatable, many fewer are hotpluggable.
So the tradeoff for the default doesn't *necessarily* go
the same way.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 04/27] vhost-user-scsi: use g_strdup()

[Philippe Mathieu-Daudé]

On 08/23/2017 01:19 PM, Marc-André Lureau wrote:

Since vhost-user-scsi uses glib.

Signed-off-by: Marc-André Lureau 


Reviewed-by: Philippe Mathieu-Daudé 


---
  contrib/vhost-user-scsi/vhost-user-scsi.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 78bcc65f5a..1fb57da2da 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -822,10 +822,10 @@ int main(int argc, char **argv)
  case 'h':
  goto help;
  case 'u':
-unix_fn = strdup(optarg);
+unix_fn = g_strdup(optarg);
  break;
  case 'i':
-iscsi_uri = strdup(optarg);
+iscsi_uri = g_strdup(optarg);
  break;
  default:
  goto help;
@@ -854,12 +854,8 @@ out:
  vdev_scsi_deinit(vdev_scsi);
  free(vdev_scsi);
  }
-if (unix_fn) {
-free(unix_fn);
-}
-if (iscsi_uri) {
-free(iscsi_uri);
-}
+g_free(unix_fn);
+g_free(iscsi_uri);
  
  return err;
  

Re: [Qemu-devel] [PATCH 12/27] vhost-user-scsi: use NULL pointer

[Philippe Mathieu-Daudé]

On Wed, Aug 23, 2017 at 1:19 PM, Marc-André Lureau
 wrote:
> Signed-off-by: Marc-André Lureau 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c 
> b/contrib/vhost-user-scsi/vhost-user-scsi.c
> index 624efc24ca..596c1effa1 100644
> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
> @@ -686,7 +686,7 @@ static int vdev_scsi_run(vhost_scsi_dev_t *vdev_scsi)
>  assert(vdev_scsi->server_sock >= 0);
>  assert(vdev_scsi->loop);
>
> -cli_sock = accept(vdev_scsi->server_sock, (void *)0, (void *)0);
> +cli_sock = accept(vdev_scsi->server_sock, NULL, NULL);
>  if (cli_sock < 0) {
>  perror("accept");
>  return -1;
> --
> 2.14.1.146.gd35faa819
>
>

Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

[Igor Mammedov]

On Wed, 23 Aug 2017 21:35:29 +0800
Dou Liyang  wrote:

> Hi Igor,
> 
> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 20:12:51 +0800
> > Dou Liyang  wrote:
> >
> >> Hi Igor,
> >>
> >> At 08/23/2017 04:40 PM, Igor Mammedov wrote:
> >>> On Tue, 22 Aug 2017 11:24:10 +0800
> >>> Dou Liyang  wrote:
> >>>
>  As QEMU supports the memory-less node, it is possible that there is
>  no RAM in the first numa node(also be called as node0). eg:
>    ... \
>    -m 128,slots=3,maxmem=1G \
>    -numa node -numa node,mem=128M \
> 
>  But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
>  table. Only fixing it is not enough.
> 
>  Add a testcase for this situation to make sure the ACPI table is
>  correct for guest.
> 
>  Suggested-by: Eduardo Habkost 
>  Signed-off-by: Dou Liyang 
>  ---
>   tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
>   tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
>   tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
>   tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
>   tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
>   tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
>   tests/bios-tables-test.c  |  30 
>  ++
>   7 files changed, 30 insertions(+)
>   create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
>   create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
>   create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
>   create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
>   create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
>   create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
> >>>
> >>>
> >>> considering only SRAT table has been changed and the other
> >>> tables match with default blobs, I'd suggest to keep only
> >>
> >>
> >> Our testcase is：
> >>
> >> +test_acpi_one(" -m 128,slots=3,maxmem=1G"
> >> +  " -numa node -numa node,mem=128"
> >> +  " -numa dist,src=0,dst=1,val=21",
> >> +  );
> >>
> >> The DSDT and SLIT don't match with default blobs.
> > do you actually need SLIT table /i.e. -numa dist/ for test at all?
> > it looks not relevant for the test case at the hand,
> > I'd suggest to drop '-numa dist' option for the test.
> >
> 
> OK, Got it, will drop '-numa dist' option in next version.
> 
> >>
> >> So, they can't be dropped.
> >
> > I wonder what's changed, could you post DSDT diff here?
> >
> 
> Just like memory hot-plug cases, when we use the '-m 128
> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
> Memory Device in the DSDT table.
for your case '-numa node -numa node,mem=128', there is no need in enabling 
memory hotplug.
If I recall it correctly the default memory for x86 is 128Mb,
hence removing "-m" would probably make DSDT match default one.

[...] 

Re: [Qemu-devel] [RFC v2 5/8] hmp: support "without_bql"

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Introducing new option "without_bql" for HMP commands.  It works just
> like QMP "without-bql", but for HMP commands.
> 
> Signed-off-by: Peter Xu 

It's going to be interesting when we have hmp commands that just call
their qmp equivalent that we need to check for no mistakes.

Dave

> ---
>  monitor.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/monitor.c b/monitor.c
> index 3d4ecff..c26c797 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -125,6 +125,8 @@ typedef struct mon_cmd_t {
>  const char *args_type;
>  const char *params;
>  const char *help;
> +/* Whether this command can be run without taking BQL? */
> +bool without_bql;
>  void (*cmd)(Monitor *mon, const QDict *qdict);
>  /* @sub_table is a list of 2nd level of commands. If it does not exist,
>   * cmd should be used. If it exists, sub_table[?].cmd should be
> @@ -3154,6 +3156,14 @@ static void handle_hmp_command(Monitor *mon, const 
> char *cmdline)
>  return;
>  }
>  
> +if (cmd->without_bql) {
> +/*
> + * This is similar to QMP's "without-bql".  See comments in
> + * do_qmp_dispatch().
> + */
> +take_bql = false;
> +}
> +
>  if (take_bql) {
>  qemu_mutex_lock_iothread();
>  }
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[John Snow]

On 08/23/2017 01:31 PM, Max Reitz wrote:
> On 2017-08-22 21:07, John Snow wrote:
> 
> [...]
> 
>> (3) Add either a new flag that turns qcow2's backing file into a full
>> R/W backing file, or add a new extension to qcow2 entirely (bypassing
>> the traditional backing file mechanism to avoid confusion for older
>> tooling) that adds a new read-write backing file field.
>>
>> This RW backing file field will be used for all reads AND writes; the
>> qcow2 in question becomes a metadata container on top of the BDS chain.
>> We can re-use Vladimir's bitmap persistence extension to save bitmaps in
>> a qcow2 shell.
>>
>> The qcow2 becomes effectively a metadata cache for a new (essentially)
>> filter node that handles features such as bitmaps. This could also be
>> used to provide allocation map data for RAW files and other goodies down
>> the road.
>>
>> Hopefully this achieves our desire to not create new formats AND our
>> desire to concentrate features (and debugging, testing, etc) into qcow2,
>> while allowing users to "have bitmaps with raw files."
>>
>> Of course, in this scenario, users now have two files: a qcow2 wrapper
>> and the actual raw file in question; but regardless of how we were going
>> to solve this, a raw file necessitates an external file of some sort,
>> else we give up the idea that it was a raw file.
>>
>>
>> Sound good?
> 
> While I don't quite like the idea of R/W backing files, I guess "don't
> quite like" is still rather good.
> 

Yeah, it's not necessarily my first pick, but it might be the least bad.
If you have any suggestions or alternatives for a way to accomplish the
same in a way that does not leave any, even faint, bad taste in your
mouth you are more than welcome to suggest it.

> I like how this idea would allow us to use the existing code without
> putting just arbitrary binary data into the qcow2 file; the data still
> relates to the virtual disk represented by the qcow2 image.
> 
> So design-wise, I don't think I have any complaints.
> 
> As for Kevin, he's on PTO, though. ;-)
> 
> Max
> 

Ah, right. Well... I'll just have to have something fun for him to look at.

Re: [Qemu-devel] [RFC v2 4/8] QAPI: new QMP command option "without-bql"

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Introducing this new parameter for QMP commands in general to mark out
> when the command does not need BQL.  Normally QMP command executions are
> done with the protection of BQL in QEMU.  However the truth is that not
> all the QMP commands require the BQL.
> 
> This new parameter provides a way to allow QMP commands to run in
> parallel when possible, without the contention on the BQL.
> 
> Since the default value of "without-bql" is still false, so now all QMP
> commands are still protected by BQL still.
> 
> Signed-off-by: Peter Xu 

We should define what a 'without-bql' command is allowed to do:
   'Commands that have without-bql set _may_ be called without the bql
   being taken.  They must not take the bql or any other lock that may
   become dependent on the bql.'
   (Do we need to say anything about RCU?)

Also, 'no-bql' is shorter :-)

Dave

> ---
>  docs/devel/qapi-code-gen.txt   | 10 +-
>  include/qapi/qmp/dispatch.h|  1 +
>  qapi/qmp-dispatch.c| 11 +++
>  scripts/qapi-commands.py   | 18 +-
>  scripts/qapi-introspect.py |  2 +-
>  scripts/qapi.py| 15 ++-
>  scripts/qapi2texi.py   |  2 +-
>  tests/qapi-schema/test-qapi.py |  2 +-
>  8 files changed, 47 insertions(+), 14 deletions(-)
> 
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 9903ac4..4960d00 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -556,7 +556,8 @@ following example objects:
>  
>  Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
>   '*returns': TYPE-NAME, '*boxed': true,
> - '*gen': false, '*success-response': false }
> + '*gen': false, '*success-response': false,
> + '*without-bql': false }
>  
>  Commands are defined by using a dictionary containing several members,
>  where three members are most common.  The 'command' member is a
> @@ -636,6 +637,13 @@ possible, the command expression should include the 
> optional key
>  'success-response' with boolean value false.  So far, only QGA makes
>  use of this member.
>  
> +Most of the commands require the Big QEMU Lock (BQL) be held during
> +execution.  However, there is a small subset of the commands that may
> +not really need BQL at all.  To mark out this kind of commands, we can
> +specify "without-bql" to "true".  This parameter is only a hint for
> +internal QMP implementation to provide possiblility to allow commands
> +be run in parallel, or reduce the contention of the lock.  Users of QMP
> +should not really be aware of such information.

Well, I think users of these commands might select them specifically
because they know that they won't block.  Those who care about latency might
look to use commands that don't take the lock because of a reduced
effect on the performance as well.

Dave

>  === Events ===
>  
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 20578dc..ec5c620 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -23,6 +23,7 @@ typedef enum QmpCommandOptions
>  {
>  QCO_NO_OPTIONS = 0x0,
>  QCO_NO_SUCCESS_RESP = 0x1,
> +QCO_WITHOUT_BQL = 0x2,
>  } QmpCommandOptions;
>  
>  typedef struct QmpCommand
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 3b6b224..b7fba5e 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -107,6 +107,17 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, 
> QObject *request,
>  QINCREF(args);
>  }
>  
> +if (cmd->options & QCO_WITHOUT_BQL) {
> +/*
> + * If this command can live without BQL, then we don't take
> + * it.  One thing to mention: we may have already taken the
> + * BQL before reaching here.  If so, we just keep it.  So
> + * generally speaking we are trying our best on reducing the
> + * contention of BQL.
> + */
> +take_bql = false;
> +}
> +
>  if (take_bql) {
>  qemu_mutex_lock_iothread();
>  }
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 974d0a4..155a0a4 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -192,10 +192,17 @@ out:
>  return ret
>  
>  
> -def gen_register_command(name, success_response):
> -options = 'QCO_NO_OPTIONS'
> +def gen_register_command(name, success_response, without_bql):
> +options = []
> +
>  if not success_response:
> -options = 'QCO_NO_SUCCESS_RESP'
> +options += ['QCO_NO_SUCCESS_RESP']
> +if without_bql:
> +options += ['QCO_WITHOUT_BQL']
> +
> +if not options:
> +options = ['QCO_NO_OPTIONS']
> +options = " | ".join(options)
>  
>  ret = mcgen('''
>  qmp_register_command(cmds, "%(name)s",
> @@ -241,7 +248,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
>  

Re: [Qemu-devel] [PATCH for-2.11 06/27] x86: extract legacy cpu features format parser

[Igor Mammedov]

On Wed, 23 Aug 2017 13:46:38 -0300
Eduardo Habkost  wrote:

> On Wed, Aug 23, 2017 at 06:29:02PM +0200, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 11:34:14 -0300
> > Eduardo Habkost  wrote:
> > 
> > > On Fri, Aug 18, 2017 at 12:08:38PM +0200, Igor Mammedov wrote:
> > > > Move cpu_model +-feat parsing into a separate file so that it
> > > > could be reused later for parsing similar format of sparc target
> > > > 
> > > > Signed-off-by: Igor Mammedov 
> > > > ---
> > > > CC: Richard Henderson 
> > > > CC: Eduardo Habkost 
> > > > CC: Mark Cave-Ayland 
> > > > CC: Artyom Tarasenko 
> > > > CC: Philippe Mathieu-Daudé 
> > > > ---
> > > >  include/qom/cpu.h |   2 +
> > > >  default-configs/i386-bsd-user.mak |   1 +
> > > >  default-configs/i386-linux-user.mak   |   1 +
> > > >  default-configs/i386-softmmu.mak  |   1 +
> > > >  default-configs/x86_64-bsd-user.mak   |   1 +
> > > >  default-configs/x86_64-linux-user.mak |   1 +
> > > >  default-configs/x86_64-softmmu.mak|   1 +
> > > >  target/i386/cpu.c | 125 +-
> > > >  util/Makefile.objs|   1 +
> > > >  util/legacy_cpu_features_parser.c | 161 
> > > > ++
> > > >  10 files changed, 171 insertions(+), 124 deletions(-)
> > > >  create mode 100644 util/legacy_cpu_features_parser.c
> > > >   
> > > [...]
> > > > diff --git a/util/legacy_cpu_features_parser.c 
> > > > b/util/legacy_cpu_features_parser.c
> > > > new file mode 100644
> > > > index 000..6b352a3
> > > > --- /dev/null
> > > > +++ b/util/legacy_cpu_features_parser.c
> > > > @@ -0,0 +1,161 @@
> > > > +/* Support for legacy -cpu cpu,features CLI option with +-feat syntax,
> > > > + * used by x86/sparc targets
> > > > + *
> > > > + * Author: Andreas Färber 
> > > > + * Author: Andre Przywara 
> > > > + * Author: Eduardo Habkost 
> > > > + * Author: Igor Mammedov 
> > > > + * Author: Paolo Bonzini 
> > > > + * Author: Markus Armbruster   
> > > 
> > > IANAL, but I believe a
> > >   Copyright (c)  
> > > line is needed here.
> > > 
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published by
> > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > + * (at your option) any later version.
> > > > +
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > + * GNU General Public License for more details.
> > > > +
> > > > + * You should have received a copy of the GNU General Public License 
> > > > along
> > > > + * with this program; if not, see .
> > > > + */
> > > > +
> > > > +#include "qemu/osdep.h"
> > > > +#include "qapi/error.h"
> > > > +#include "qemu/cutils.h"
> > > > +#include "qom/cpu.h"
> > > > +#include "qemu/error-report.h"
> > > > +#include "hw/qdev-properties.h"
> > > > +
> > > > +static inline void feat2prop(char *s)
> > > > +{
> > > > +while ((s = strchr(s, '_'))) {
> > > > +*s = '-';
> > > > +}
> > > > +}
> > > > +
> > > > +static gint compare_string(gconstpointer a, gconstpointer b)
> > > > +{
> > > > +return g_strcmp0(a, b);
> > > > +}
> > > > +
> > > > +static void
> > > > +cpu_add_feat_as_prop(const char *typename, const char *name, const 
> > > > char *val)
> > > > +{
> > > > +GlobalProperty *prop = g_new0(typeof(*prop), 1);
> > > > +prop->driver = typename;
> > > > +prop->property = g_strdup(name);
> > > > +prop->value = g_strdup(val);
> > > > +prop->errp = _fatal;
> > > > +qdev_prop_register_global(prop);
> > > > +}
> > > > +
> > > > +/* DO NOT USE WITH NEW CODE
> > > > + * Parse "+feature,-feature,feature=foo" CPU feature string
> > > > + */
> > > > +void cpu_legacy_parse_featurestr(const char *typename, char *features,
> > > > + Error **errp)
> > > > +{
> > > > +/* Compatibily hack to maintain legacy +-feat semantic,
> > > > + * where +-feat overwrites any feature set by
> > > > + * feat=on|feat even if the later is parsed after +-feat
> > > > + * (i.e. "-x2apic,x2apic=on" will result in x2apic disabled)
> > > > + */
> > > > +GList *l, *plus_features = NULL, *minus_features = NULL;
> > > > +char *featurestr; /* Single 'key=value" string being parsed */
> > > > +static bool cpu_globals_initialized;
> > > > +bool ambiguous = false;
> > > > +
> > > > +if (cpu_globals_initialized) {
> > > > +return;

Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

[Eduardo Habkost]

On Wed, Aug 23, 2017 at 05:07:12PM +0200, Gerd Hoffmann wrote:
> On Thu, 2017-08-17 at 10:56 +0200, Markus Armbruster wrote:
> > Gerd, can we delete the vnc_init_func() stub?  Things still compile
> > for
> > me when I do.
> > 
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 7262bef..2c3b2cd 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -484,11 +484,6 @@ static inline QemuOpts *vnc_parse(const char
> > *str, Error **errp)
> >  error_setg(errp, "VNC support is disabled");
> >  return NULL;
> >  }
> > -static inline int vnc_init_func(void *opaque, QemuOpts *opts, Error
> > **errp)
> > -{
> > -error_setg(errp, "VNC support is disabled");
> > -return -1;
> > -}
> >  #endif
> 
> Looking at a663fbd9e2f65fae81018d81f231ad79510cf9fb.  Guess we want fix
> qemu_opts_foreach to not barf on NULL, then revert that commit, so
> f8c75b2486 (adding Eduardo to Cc) works as intended.

I agree with that plan, but I wouldn't mind having the stub
removed in the meantime.

> 
> Beside that:  The inline for vnc_init_func looks pointless (it's used
> via function pointer).  Also it can be empty as it will never be
> actually called.  Which is fine, vnc_parse will throw an error if
> someone tries to use vnc with a binary without vnc support.

True.

-- 
Eduardo

Re: [Qemu-devel] [RFC v2 2/8] monitor: allow monitor to create thread to poll

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Firstly, introduce Monitor.use_thread, and set it for monitors that are
> using non-mux typed backend chardev.  We only do this for monitors, so
> mux-typed chardevs are not suitable (when it connects to, e.g., serials
> and the monitor together).
> 
> When use_thread is set, we create standalone thread to poll the monitor
> events, isolated from the main loop thread.  Here we still need to take
> the BQL before dispatching the tasks since some of the monitor commands
> are not allowed to execute without the protection of BQL.  Then this
> gives us the chance to avoid taking the BQL for some monitor commands in
> the future.
> 
> * Why this change?
> 
> We need these per-monitor threads to make sure we can have at least one
> monitor that will never stuck (that can receive further monitor
> commands).
> 
> * So when will monitors stuck?  And, how do they stuck?

(Minor: 'stuck' is past tense, 'stick' is probably the right word; however
'block' is probably what you actually want)

> After we have postcopy and remote page faults, it's simple to achieve a
> stuck in the monitor (which is also a stuck in main loop thread):
> 
> (1) Monitor deadlock on BQL
> 
> As we may know, when postcopy is running on destination VM, the vcpu
> threads can stuck merely any time as long as it tries to access an
> uncopied guest page.  Meanwhile, when the stuck happens, it is possible
> that the vcpu thread is holding the BQL.  If the page fault is not
> handled quickly, you'll find that monitors stop working, which is trying
> to take the BQL.
> 
> If the page fault cannot be handled correctly (one case is a paused
> postcopy, when network is temporarily down), monitors will hang
> forever.  Without current patch, that means the main loop hanged.  We'll
> never find a way to talk to VM again.
> 
> (2) Monitor tries to run codes page-faulted vcpus
> 
> The HMP command "info cpus" is one of the good example - it tries to
> kick all the vcpus and sync status from them.  However, if there is any
> vcpu that stuck at an unhandled page fault, it can never achieve the
> sync, then the HMP hangs.  Again, it hangs the main loop thread as well.
> 
> After either (1) or (2), we can see the deadlock problem:
> 
> - On one hand, if monitor hangs, we cannot do the postcopy recovery,
>   because postcopy recovery needs user to specify new listening port on
>   destination monitor.
> 
> - On the other hand, if we cannot recover the paused postcopy, then page
>   faults cannot be serviced, and the monitors will possibly hang
>   forever then.
> 
> * How this patch helps?
> 
> - Firstly, we'll have our own thread for each dedicated monitor (or say,
>   the backend chardev is only used for monitor), so even main loop
>   thread hangs (it is always possible), this monitor thread may still
>   survive.
> 
> - Not all monitor commands need the BQL.  We can selectively take the
>   BQL (depends on which command we are running) to avoid waiting on a
>   page-faulted vcpu thread that has taken the BQL (this will be done in
>   following up patches).
> 
> Signed-off-by: Peter Xu 

A few high level things:
  a) I think this patch probably wants to split into
 1) A patch that decides whether to create a new thread and
 initialises it
 2) One that starts to fix up the locking

  b) I think you also need to take the bql around any of the custom
 completion functions; (maybe in monitor_find_completion ?)
 since they do things like walk the lists of devices.

  c) As mentioned on irc there's fun to be had with cur_mon and error
 handling - in my local world I have cur_mon declared as __thread
 but never got around to thinking aobut what should set it up.
 There's also 'wavcapture: Convert to error_report' that I posted
 in March that got rid of some uses of cur_mon in wavcapture.c
 for error_report.  But there's some interesting stuff to be checked
 with where error_reporting goes.

  d) I wonder if it's better to have thread as a flag, so that you have
 to explicitly ask for a monitor to have it's own thread.

I'll leave it to Dan to check over the chardev mechanics in here.

Dave

> ---
>  monitor.c   | 75 
> +
>  qapi/qmp-dispatch.c | 15 +++
>  2 files changed, 85 insertions(+), 5 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 7c90df7..3d4ecff 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -36,6 +36,8 @@
>  #include "net/net.h"
>  #include "net/slirp.h"
>  #include "chardev/char-fe.h"
> +#include "chardev/char-mux.h"
> +#include "chardev/char-io.h"
>  #include "ui/qemu-spice.h"
>  #include "sysemu/numa.h"
>  #include "monitor/monitor.h"
> @@ -190,6 +192,8 @@ struct Monitor {
>  int flags;
>  int suspend_cnt;
>  bool skip_flush;
> +/* Whether the monitor wants to be polled in standalone thread */
> +bool use_thread;
>  
>  QemuMutex out_lock;
> 

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[Max Reitz]

On 2017-08-22 21:07, John Snow wrote:

[...]

> (3) Add either a new flag that turns qcow2's backing file into a full
> R/W backing file, or add a new extension to qcow2 entirely (bypassing
> the traditional backing file mechanism to avoid confusion for older
> tooling) that adds a new read-write backing file field.
> 
> This RW backing file field will be used for all reads AND writes; the
> qcow2 in question becomes a metadata container on top of the BDS chain.
> We can re-use Vladimir's bitmap persistence extension to save bitmaps in
> a qcow2 shell.
> 
> The qcow2 becomes effectively a metadata cache for a new (essentially)
> filter node that handles features such as bitmaps. This could also be
> used to provide allocation map data for RAW files and other goodies down
> the road.
> 
> Hopefully this achieves our desire to not create new formats AND our
> desire to concentrate features (and debugging, testing, etc) into qcow2,
> while allowing users to "have bitmaps with raw files."
> 
> Of course, in this scenario, users now have two files: a qcow2 wrapper
> and the actual raw file in question; but regardless of how we were going
> to solve this, a raw file necessitates an external file of some sort,
> else we give up the idea that it was a raw file.
> 
> 
> Sound good?

While I don't quite like the idea of R/W backing files, I guess "don't
quite like" is still rather good.

I like how this idea would allow us to use the existing code without
putting just arbitrary binary data into the qcow2 file; the data still
relates to the virtual disk represented by the qcow2 image.

So design-wise, I don't think I have any complaints.

As for Kevin, he's on PTO, though. ;-)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

[Eduardo Habkost]

On Wed, Aug 23, 2017 at 09:35:29PM +0800, Dou Liyang wrote:
> Hi Igor,
> 
> At 08/23/2017 08:45 PM, Igor Mammedov wrote:
> > On Wed, 23 Aug 2017 20:12:51 +0800
> > Dou Liyang  wrote:
> > 
> > > Hi Igor,
> > > 
> > > At 08/23/2017 04:40 PM, Igor Mammedov wrote:
> > > > On Tue, 22 Aug 2017 11:24:10 +0800
> > > > Dou Liyang  wrote:
> > > > 
> > > > > As QEMU supports the memory-less node, it is possible that there is
> > > > > no RAM in the first numa node(also be called as node0). eg:
> > > > >   ... \
> > > > >   -m 128,slots=3,maxmem=1G \
> > > > >   -numa node -numa node,mem=128M \
> > > > > 
> > > > > But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
> > > > > table. Only fixing it is not enough.
> > > > > 
> > > > > Add a testcase for this situation to make sure the ACPI table is
> > > > > correct for guest.
> > > > > 
> > > > > Suggested-by: Eduardo Habkost 
> > > > > Signed-off-by: Dou Liyang 
> > > > > ---
> > > > >  tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
> > > > >  tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
> > > > >  tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
> > > > >  tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
> > > > >  tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
> > > > >  tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
> > > > >  tests/bios-tables-test.c  |  30 
> > > > > ++
> > > > >  7 files changed, 30 insertions(+)
> > > > >  create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
> > > > >  create mode 100644 tests/acpi-test-data/q35/SRAT.numamem
> > > > 
> > > > 
> > > > considering only SRAT table has been changed and the other
> > > > tables match with default blobs, I'd suggest to keep only
> > > 
> > > 
> > > Our testcase is：
> > > 
> > > +test_acpi_one(" -m 128,slots=3,maxmem=1G"
> > > +  " -numa node -numa node,mem=128"
> > > +  " -numa dist,src=0,dst=1,val=21",
> > > +  );
> > > 
> > > The DSDT and SLIT don't match with default blobs.
> > do you actually need SLIT table /i.e. -numa dist/ for test at all?
> > it looks not relevant for the test case at the hand,
> > I'd suggest to drop '-numa dist' option for the test.
> > 
> 
> OK, Got it, will drop '-numa dist' option in next version.
> 
> > > 
> > > So, they can't be dropped.
> > 
> > I wonder what's changed, could you post DSDT diff here?
> > 
> 
> Just like memory hot-plug cases, when we use the '-m
> 128,slots=3,maxmem=1G' option, As the ACPI spec said, There may be some
> Memory Device in the DSDT table.

Do you really need to use -m 128,slots=3,maxmem=1G to test your
bug fix?

-- 
Eduardo

[Qemu-devel] [PATCH 1/3] multiboot: Change multiboot_info from array of bytes to a C struct

[Anatol Pomozov]

Using C structs makes the code more readable and prevents type conversion
errors.

Borrow multiboot1 header from GRUB project.

Signed-off-by: Anatol Pomozov 
---
 hw/i386/multiboot.c | 124 +
 hw/i386/multiboot_header.h  | 254 
 tests/multiboot/mmap.c  |  14 +--
 tests/multiboot/mmap.out|  10 ++
 tests/multiboot/modules.c   |  12 ++-
 tests/multiboot/modules.out |  10 ++
 tests/multiboot/multiboot.h |  66 
 7 files changed, 339 insertions(+), 151 deletions(-)
 create mode 100644 hw/i386/multiboot_header.h
 delete mode 100644 tests/multiboot/multiboot.h

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..19113c0fce 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,6 +28,7 @@
 #include "hw/hw.h"
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
+#include "multiboot_header.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
@@ -47,39 +48,9 @@
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
-enum {
-/* Multiboot info */
-MBI_FLAGS   = 0,
-MBI_MEM_LOWER   = 4,
-MBI_MEM_UPPER   = 8,
-MBI_BOOT_DEVICE = 12,
-MBI_CMDLINE = 16,
-MBI_MODS_COUNT  = 20,
-MBI_MODS_ADDR   = 24,
-MBI_MMAP_ADDR   = 48,
-MBI_BOOTLOADER  = 64,
-
-MBI_SIZE= 88,
-
-/* Multiboot modules */
-MB_MOD_START= 0,
-MB_MOD_END  = 4,
-MB_MOD_CMDLINE  = 8,
-
-MB_MOD_SIZE = 16,
-
-/* Region offsets */
-ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
-ADDR_MBI  = ADDR_E820_MAP + 0x500,
-
-/* Multiboot flags */
-MULTIBOOT_FLAGS_MEMORY  = 1 << 0,
-MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
-MULTIBOOT_FLAGS_CMDLINE = 1 << 2,
-MULTIBOOT_FLAGS_MODULES = 1 << 3,
-MULTIBOOT_FLAGS_MMAP= 1 << 6,
-MULTIBOOT_FLAGS_BOOTLOADER  = 1 << 9,
-};
+/* Region offsets */
+#define ADDR_E820_MAP MULTIBOOT_STRUCT_ADDR
+#define ADDR_MBI  (ADDR_E820_MAP + 0x500)
 
 typedef struct {
 /* buffer holding kernel, cmdlines and mb_infos */
@@ -128,14 +99,15 @@ static void mb_add_mod(MultibootState *s,
hwaddr start, hwaddr end,
hwaddr cmdline_phys)
 {
-char *p;
+multiboot_module_t *mod;
 assert(s->mb_mods_count < s->mb_mods_avail);
 
-p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+mod = s->mb_buf + s->offset_mbinfo +
+  sizeof(multiboot_module_t) * s->mb_mods_count;
 
-stl_p(p + MB_MOD_START,   start);
-stl_p(p + MB_MOD_END, end);
-stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+stl_p(>mod_start, start);
+stl_p(>mod_end,   end);
+stl_p(>cmdline,   cmdline_phys);
 
 mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
  s->mb_mods_count, start, end);
@@ -151,26 +123,29 @@ int load_multiboot(FWCfgState *fw_cfg,
int kernel_file_size,
uint8_t *header)
 {
-int i, is_multiboot = 0;
+int i;
+bool is_multiboot = false;
 uint32_t flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
 MultibootState mbs;
-uint8_t bootinfo[MBI_SIZE];
+multiboot_info_t bootinfo;
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
+struct multiboot_header *multiboot_header;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
 for (i = 0; i < (8192 - 48); i += 4) {
-if (ldl_p(header+i) == 0x1BADB002) {
-uint32_t checksum = ldl_p(header+i+8);
-flags = ldl_p(header+i+4);
+multiboot_header = (struct multiboot_header *)(header + i);
+if (ldl_p(_header->magic) == MULTIBOOT_HEADER_MAGIC) {
+uint32_t checksum = ldl_p(_header->checksum);
+flags = ldl_p(_header->flags);
 checksum += flags;
-checksum += (uint32_t)0x1BADB002;
+checksum += (uint32_t)MULTIBOOT_HEADER_MAGIC;
 if (!checksum) {
-is_multiboot = 1;
+is_multiboot = true;
 break;
 }
 }
@@ -180,13 +155,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 return 0; /* no multiboot */
 
 mb_debug("qemu: I believe we found a multiboot image!\n");
-memset(bootinfo, 0, sizeof(bootinfo));
+memset(, 0, sizeof(bootinfo));
 memset(, 0, sizeof(mbs));
 
-if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
+if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
+if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
 uint64_t elf_entry;
 uint64_t elf_low, elf_high;
 int kernel_size;
@@ -217,14 +192,14 @@ int load_multiboot(FWCfgState *fw_cfg,
 

[Qemu-devel] [PATCH 3/3] multiboot: load elf sections and section headers

[Anatol Pomozov]

Multiboot may load section headers and all sections (even those that are
not part of any segment) to target memory.

Tested with an ELF application that uses data from strings table
section.

Signed-off-by: Anatol Pomozov 
---
 hw/core/loader.c |   8 ++--
 hw/i386/multiboot.c  |  83 +
 hw/s390x/ipl.c   |   2 +-
 include/hw/elf_ops.h | 107 ++-
 include/hw/loader.h  |  11 -
 tests/multiboot/Makefile |   5 +-
 tests/multiboot/modules.out  |  22 -
 tests/multiboot/run_test.sh  |   6 ++-
 tests/multiboot/sections.c   |  55 ++
 tests/multiboot/sections.out |  21 +
 tests/multiboot/start.S  |   2 +-
 11 files changed, 261 insertions(+), 61 deletions(-)
 create mode 100644 tests/multiboot/sections.c
 create mode 100644 tests/multiboot/sections.out

diff --git a/hw/core/loader.c b/hw/core/loader.c
index ebe574c7ea..c7e3608e7a 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -439,7 +439,7 @@ int load_elf_as(const char *filename,
 {
 return load_elf_ram(filename, translate_fn, translate_opaque,
 pentry, lowaddr, highaddr, big_endian, elf_machine,
-clear_lsb, data_swab, as, true);
+clear_lsb, data_swab, as, true, NULL);
 }
 
 /* return < 0 if error, otherwise the number of bytes loaded in memory */
@@ -448,7 +448,7 @@ int load_elf_ram(const char *filename,
  void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
  uint64_t *highaddr, int big_endian, int elf_machine,
  int clear_lsb, int data_swab, AddressSpace *as,
- bool load_rom)
+ bool load_rom, struct sections_data *sections)
 {
 int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
 uint8_t e_ident[EI_NIDENT];
@@ -488,11 +488,11 @@ int load_elf_ram(const char *filename,
 if (e_ident[EI_CLASS] == ELFCLASS64) {
 ret = load_elf64(filename, fd, translate_fn, translate_opaque, 
must_swab,
  pentry, lowaddr, highaddr, elf_machine, clear_lsb,
- data_swab, as, load_rom);
+ data_swab, as, load_rom, sections);
 } else {
 ret = load_elf32(filename, fd, translate_fn, translate_opaque, 
must_swab,
  pentry, lowaddr, highaddr, elf_machine, clear_lsb,
- data_swab, as, load_rom);
+ data_swab, as, load_rom, sections);
 }
 
  fail:
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index a49217ac62..9ced0bfe30 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -125,7 +125,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 {
 int i;
 bool is_multiboot = false;
-uint32_t flags = 0;
+uint32_t flags = 0, bootinfo_flags = 0;
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
@@ -134,6 +134,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 uint8_t *mb_bootinfo_data;
 uint32_t cmdline_len;
 struct multiboot_header *multiboot_header;
+struct sections_data sections;
 
 /* Ok, let's see if it is a multiboot image.
The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -161,37 +162,8 @@ int load_multiboot(FWCfgState *fw_cfg,
 if (flags & MULTIBOOT_VIDEO_MODE) {
 fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
 }
-if (!(flags & MULTIBOOT_AOUT_KLUDGE)) {
-uint64_t elf_entry;
-uint64_t elf_low, elf_high;
-int kernel_size;
-fclose(f);
 
-if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
-exit(1);
-}
-
-kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   _low, _high, 0, EM_NONE,
-   0, 0);
-if (kernel_size < 0) {
-fprintf(stderr, "Error while loading elf kernel\n");
-exit(1);
-}
-mh_load_addr = elf_low;
-mb_kernel_size = elf_high - elf_low;
-mh_entry_addr = elf_entry;
-
-mbs.mb_buf = g_malloc(mb_kernel_size);
-if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != 
mb_kernel_size) {
-fprintf(stderr, "Error while fetching elf kernel from rom\n");
-exit(1);
-}
-
-mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
-  mb_kernel_size, (size_t)mh_entry_addr);
-} else {
+if (flags & MULTIBOOT_AOUT_KLUDGE) {
 /* Valid if mh_flags sets MULTIBOOT_AOUT_KLUDGE. */
 uint32_t mh_header_addr = ldl_p(_header->header_addr);
 uint32_t mh_load_end_addr = ldl_p(_header->load_end_addr);
@@ -209,12 +181,6 @@ int