Re: [Qemu-devel] [PATCH v9 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap

[Paolo Bonzini]

On 12/02/2018 18:30, Vladimir Sementsov-Ogievskiy wrote:
> 18.01.2018 13:09, Paolo Bonzini wrote:>> We have three cases:
>>
>> 1) monitor creates and destroy bitmaps.
>>
>> 2) monitor also has to read the list.  We know it operates with BQL.
>>
>> 3) users such as mirror.c create a dirty bitmap in the monitor command
>> (under BQL), but they can operate without BQL in a separate iothread so
>> we create a separate lock (bitmap->mutex).
>>
>> While in the second and third case, bitmaps cannot disappear.  So in the
>> first case you operate with BQL+dirty bitmap mutex.  The result is that
>> you lock out both the second and the third case while creating and
>> destroying bitmaps.
>>
>>> Why do we do not need them
>>> on read from the bitmap, only on write?
>>
>> Indeed, reading the bitmap also requires taking the lock.  So
>> s/Modifying/Accessing/ in that comment.
> 
> So, finally, the whole thing is:
> 
> 1. any access to dirty_bitmaps list needs BQL or dirty_bitmap_mutex
> 2. bitmap creation or removing needs both BQL and dirty_bitmap_mutex

3. any access to a dirty bitmap needs dirty_bitmap_mutex

Paolo

> yes?
> 
> and one more question:
> Do we really have users, which accesses dirty bitmaps with only BQL?
> query-block uses dirty_bitmap_mutex..
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 02/19] spapr: introduce a skeleton for the XIVE interrupt controller

[Cédric Le Goater]

On 02/12/2018 03:40 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2018-02-12 at 13:20 +0100, Andrea Bolognani wrote:
>> On Mon, 2018-02-12 at 13:02 +1100, Alexey Kardashevskiy wrote:
>>> On 12/02/18 09:55, Benjamin Herrenschmidt wrote:
 Well, we have a problem then. It looks like Qemu broken migration is
 fundamentally incompatible with PAPR and CAS design...

 I know we don't migrate the configuration, that's not exactly what I
 had in mind tho... Can we have some piece of *data* from the machine be
 migrated first, and use it on the target to reconfigure the interrupt
 controller before the stream arrives ?
>>>
>>> These days this is done via libvirt - it reads properties it needs via QMP,
>>> then sends an XML with everything (the interrupt controller type may be one
>>> of such properties), and starts the destination QEMU with the explicit
>>> interrupt controller (like -machine pseries,intrc=xive).
>>
>> Clarification: libvirt will use the user-defined XML configuration
>> to generate the QEMU command line both for the source and the target
>> of the migration, but it will not automagically figure out properties
>> through QMP. So if you want the controller to explicitly show up on
>> the QEMU command line, libvirt should be taught about it.
> 
> Which can't work because the guest pretty much decides what it will be
> early on during the boot process.
> 
> So we're back to square 1 having to instanciate both objects in qemu
> with some kind of "activation" flag.

yes and the activation flag is the associated bit in CAS OV5 :

spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)

if a new interrupt mode is negotiated, a machine reset is required,
a new device tree is populated, new ICPs are installed, etc. There
is a little more to do with KVM and we need to find the right model 
abstraction for it. Anyhow, it is not a big problem to switch from 
one mode to another when both objects are around. It is even easier
to keep the allocated IRQs in sync in fact. 

What problem do you foresee with KVM ? this is already solved for 
irqchip=off.

Cheers, 

C.

[Qemu-devel] [PATCH v3] scripts: Add decodetree.py

[Richard Henderson]

To be used to decode ARM SVE, but could be used for any fixed-width ISA.

Signed-off-by: Richard Henderson 

---

Changes since v2:
  * Fix tests/decode/err_init3.def.
  * Mark main decoder static by default.
  * Properly diagnose unspecified bits.
  * Remove output file on error.
- I had been doing this in the Makefile, but all told
  it's cleaner to do it in the script.

Changes since v1:
  * Pass pycodestyle-{2,3}.
  * Support 16-bit and 32-bit insns (I have a def file for thumb1).
  * Testsuite (only negative tests so far).
  * Called translate functions default to static.
  * Notice duplicate assignments and missing assignments to fields.
  * Use '-' to indicate a non-decoded bit, as opposed to '.' which
must be filled in elsewhere by a format or a field.
---
 scripts/decodetree.py | 1044 +
 tests/Makefile.include|9 +-
 tests/decode/check.sh |   16 +
 tests/decode/err_argset1.def  |1 +
 tests/decode/err_argset2.def  |1 +
 tests/decode/err_field1.def   |1 +
 tests/decode/err_field2.def   |1 +
 tests/decode/err_field3.def   |1 +
 tests/decode/err_field4.def   |2 +
 tests/decode/err_field5.def   |1 +
 tests/decode/err_init1.def|2 +
 tests/decode/err_init2.def|2 +
 tests/decode/err_init3.def|3 +
 tests/decode/err_init4.def|3 +
 tests/decode/err_overlap1.def |2 +
 tests/decode/err_overlap2.def |2 +
 tests/decode/err_overlap3.def |2 +
 tests/decode/err_overlap4.def |2 +
 tests/decode/err_overlap5.def |1 +
 tests/decode/err_overlap6.def |2 +
 tests/decode/err_overlap7.def |2 +
 tests/decode/err_overlap8.def |1 +
 tests/decode/err_overlap9.def |2 +
 23 files changed, 1102 insertions(+), 1 deletion(-)
 create mode 100755 scripts/decodetree.py
 create mode 100755 tests/decode/check.sh
 create mode 100644 tests/decode/err_argset1.def
 create mode 100644 tests/decode/err_argset2.def
 create mode 100644 tests/decode/err_field1.def
 create mode 100644 tests/decode/err_field2.def
 create mode 100644 tests/decode/err_field3.def
 create mode 100644 tests/decode/err_field4.def
 create mode 100644 tests/decode/err_field5.def
 create mode 100644 tests/decode/err_init1.def
 create mode 100644 tests/decode/err_init2.def
 create mode 100644 tests/decode/err_init3.def
 create mode 100644 tests/decode/err_init4.def
 create mode 100644 tests/decode/err_overlap1.def
 create mode 100644 tests/decode/err_overlap2.def
 create mode 100644 tests/decode/err_overlap3.def
 create mode 100644 tests/decode/err_overlap4.def
 create mode 100644 tests/decode/err_overlap5.def
 create mode 100644 tests/decode/err_overlap6.def
 create mode 100644 tests/decode/err_overlap7.def
 create mode 100644 tests/decode/err_overlap8.def
 create mode 100644 tests/decode/err_overlap9.def

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
new file mode 100755
index 00..f0b33a937e
--- /dev/null
+++ b/scripts/decodetree.py
@@ -0,0 +1,1044 @@
+#!/usr/bin/env python
+#
+# Generate a decoding tree from a specification file.
+#
+# The tree is built from instruction "patterns".  A pattern may represent
+# a single architectural instruction or a group of same, depending on what
+# is convenient for further processing.
+#
+# Each pattern has "fixedbits" & "fixedmask", the combination of which
+# describes the condition under which the pattern is matched:
+#
+#   (insn & fixedmask) == fixedbits
+#
+# Each pattern may have "fields", which are extracted from the insn and
+# passed along to the translator.  Examples of such are registers,
+# immediates, and sub-opcodes.
+#
+# In support of patterns, one may declare fields, argument sets, and
+# formats, each of which may be re-used to simplify further definitions.
+#
+# *** Field syntax:
+#
+# field_def := '%' identifier ( unnamed_field )+ ( !function=identifier )?
+# unnamed_field := number ':' ( 's' ) number
+#
+# For unnamed_field, the first number is the least-significant bit position of
+# the field and the second number is the length of the field.  If the 's' is
+# present, the field is considered signed.  If multiple unnamed_fields are
+# present, they are concatenated.  In this way one can define disjoint fields.
+#
+# If !function is specified, the concatenated result is passed through the
+# named function, taking and returning an integral value.
+#
+# FIXME: the fields of the structure into which this result will be stored
+# is restricted to "int".  Which means that we cannot expand 64-bit items.
+#
+# Field examples:
+#
+#   %disp   0:s16  -- sextract(i, 0, 16)
+#   %imm9   16:6 10:3  -- extract(i, 16, 6) << 3 | extract(i, 10, 3)
+#   %disp12 0:s1 1:1 2:10  -- sextract(i, 0, 1) << 11
+# | extract(i, 1, 1) << 10
+# | extract(i, 2, 10)
+#   %shimm8 5:s8 13:1 !function=expand_shimm8
+#  -- 

[Qemu-devel] [PATCH] vnc: add qapi/error.h include to stubs

[Gerd Hoffmann]

Fixes --disable-vnc build failure.

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-stubs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc-stubs.c b/ui/vnc-stubs.c
index f51280549a..06c4ac6296 100644
--- a/ui/vnc-stubs.c
+++ b/ui/vnc-stubs.c
@@ -1,5 +1,6 @@
 #include "qemu/osdep.h"
 #include "ui/console.h"
+#include "qapi/error.h"
 
 int vnc_display_password(const char *id, const char *password)
 {
-- 
2.9.3

Re: [Qemu-devel] [RFC PATCH v6 00/20] replay additions

[Pavel Dovgalyuk]

Your command line looks wrong, because you forgot –icount, but specified other 
replay options.

I tried recording and replaying with your command line and the execution hangs 
at some moment of replay.

The problem may be hidden in –dtb option, because it may add the devices 
without configuring replay for them.

Can you specify the whole hardware configuration in the command line?

 

Pavel Dovgalyuk

 

From: Ciro Santilli [mailto:ciro.santi...@gmail.com] 
Sent: Tuesday, February 13, 2018 8:58 AM
To: Pavel Dovgalyuk
Cc: Pavel Dovgalyuk; qemu-devel@nongnu.org; kw...@redhat.com; Peter Maydell; 
war2jor...@live.com; Igor R; quint...@redhat.com; jasow...@redhat.com; 
m...@redhat.com; Aleksandr Bezzubikov; maria.klimushenk...@ispras.ru; 
kra...@redhat.com; Thomas Dullien; pbonz...@redhat.com; Alex Bennée
Subject: Re: [RFC PATCH v6 00/20] replay additions

 

 

 

On Mon, Feb 12, 2018 at 5:47 AM, Pavel Dovgalyuk  wrote:

I tested ARM only with –kernel and –initrd.

Can you provide the full command line and the disk image?

 

 

The command I tried was:

 

time ./buildroot/output.arm~/host/usr/bin/qemu-system-arm -M versatilepb 
-append 'root=/dev/sda nokaslr norandmaps printk.devkmsg=on printk.time=y - 
lkmc_eval="/rand_check.out;wget -S google.com;/poweroff.out;"' -kernel 
./buildroot/output.arm~/images/zImage -dtb 
./buildroot/output.arm~/images/versatile-pb.dtb -nographic -drive 
file=./buildroot/output.arm~/images/rootfs.ext2,if=scsi,id=img-direct,format=raw
 -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay -device 
scsi-hd,drive=img-blkreplay -netdev user,id=net1 -device rtl8139,netdev=net1 
-object filter-replay,id=replay,netdev=net1 

 

and the required files can be downloaded from:

 

https://github.com/cirosantilli/linux-kernel-module-cheat/releases/download/test-replay-arm/images.zip

 

They were generated with:

 

./build -a arm

 

on that repo.

 

 

Pavel Dovgalyuk

 

From: Ciro Santilli [mailto:ciro.santi...@gmail.com] 
Sent: Saturday, February 10, 2018 3:09 AM
To: Pavel Dovgalyuk
Cc: Pavel Dovgalyuk; qemu-devel@nongnu.org; kw...@redhat.com; Peter Maydell; 
war2jor...@live.com; Igor R; quint...@redhat.com; jasow...@redhat.com; 
m...@redhat.com; Aleksandr Bezzubikov; maria.klimushenk...@ispras.ru; 
kra...@redhat.com; Thomas Dullien; pbonz...@redhat.com; Alex Bennée
Subject: Re: [RFC PATCH v6 00/20] replay additions

 

Also, what command do you use to test on ARM? I'm a bit stuck to get the drive 
part right, e.g.:

 

-drive 
file=./buildroot/output.arm~/images/rootfs.ext2,if=scsi,id=img-direct,format=raw
 \

-drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \

-device scsi-hd,drive=img-blkreplay \

 

fails with: qemu-system-arm: -device scsi-hd,drive=img-blkreplay: Conflicts 
with use by img-direct as 'root', which does not allow 'write' on #block968

 

 

Re: [Qemu-devel] [RFC PATCH v6 00/20] replay additions

[Ciro Santilli]

On Mon, Feb 12, 2018 at 5:47 AM, Pavel Dovgalyuk  wrote:

> I tested ARM only with –kernel and –initrd.
>
> Can you provide the full command line and the disk image?
>
>
>

The command I tried was:

time ./buildroot/output.arm~/host/usr/bin/qemu-system-arm -M versatilepb
-append 'root=/dev/sda nokaslr norandmaps printk.devkmsg=on printk.time=y -
lkmc_eval="/rand_check.out;wget -S google.com;/poweroff.out;"' -kernel
./buildroot/output.arm~/images/zImage -dtb
./buildroot/output.arm~/images/versatile-pb.dtb -nographic -drive
file=./buildroot/output.arm~/images/rootfs.ext2,if=scsi,id=img-direct,format=raw
-drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay -device
scsi-hd,drive=img-blkreplay -netdev user,id=net1 -device
rtl8139,netdev=net1 -object filter-replay,id=replay,netdev=net1

and the required files can be downloaded from:

https://github.com/cirosantilli/linux-kernel-module-cheat/releases/download/test-replay-arm/images.zip

They were generated with:

./build -a arm

on that repo.



> Pavel Dovgalyuk
>
>
>
> *From:* Ciro Santilli [mailto:ciro.santi...@gmail.com]
> *Sent:* Saturday, February 10, 2018 3:09 AM
> *To:* Pavel Dovgalyuk
> *Cc:* Pavel Dovgalyuk; qemu-devel@nongnu.org; kw...@redhat.com; Peter
> Maydell; war2jor...@live.com; Igor R; quint...@redhat.com;
> jasow...@redhat.com; m...@redhat.com; Aleksandr Bezzubikov;
> maria.klimushenk...@ispras.ru; kra...@redhat.com; Thomas Dullien;
> pbonz...@redhat.com; Alex Bennée
> *Subject:* Re: [RFC PATCH v6 00/20] replay additions
>
>
>
> Also, what command do you use to test on ARM? I'm a bit stuck to get the
> drive part right, e.g.:
>
>
>
> -drive 
> file=./buildroot/output.arm~/images/rootfs.ext2,if=scsi,id=img-direct,format=raw
> \
>
> -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
>
> -device scsi-hd,drive=img-blkreplay \
>
>
>
> fails with: qemu-system-arm: -device scsi-hd,drive=img-blkreplay:
> Conflicts with use by img-direct as 'root', which does not allow 'write' on
> #block968
>
>
>

Re: [Qemu-devel] [PATCH qemu v7 4/4] ppc/spapr, vfio: Turn off MSIX emulation for VFIO devices

[David Gibson]

On Fri, Feb 09, 2018 at 06:55:03PM +1100, Alexey Kardashevskiy wrote:
> This adds a possibility for the platform to tell VFIO not to emulate MSIX
> so MMIO memory regions do not get split into chunks in flatview and
> the entire page can be registered as a KVM memory slot and make direct
> MMIO access possible for the guest.
> 
> This enables the entire MSIX BAR mapping to the guest for the pseries
> platform in order to achieve the maximum MMIO preformance for certain
> devices.
> 
> Tested on:
> LSI Logic / Symbios Logic SAS3008 PCI-Express Fusion-MPT SAS-3 (rev 02)
> 
> Signed-off-by: Alexey Kardashevskiy 

I still think having the property on the machine is an uglier option
than having it on the device, but Alex didn't like that, so I'll roll
with it.

Reivewed-by: David Gibson 

> ---
> 
> Need to split this in two?
> ---
>  hw/ppc/spapr.c |  7 +++
>  hw/vfio/pci.c  | 13 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index eb3e1a7..14d8ecb 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2855,6 +2855,11 @@ static void spapr_set_modern_hotplug_events(Object 
> *obj, bool value,
>  spapr->use_hotplug_event_source = value;
>  }
>  
> +static bool spapr_get_msix_emulation(Object *obj, Error **errp)
> +{
> +return true;
> +}
> +
>  static char *spapr_get_resize_hpt(Object *obj, Error **errp)
>  {
>  sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> @@ -2936,6 +2941,8 @@ static void spapr_instance_init(Object *obj)
>  object_property_set_description(obj, "vsmt",
>  "Virtual SMT: KVM behaves as if this 
> were"
>  " the host's SMT mode", _abort);
> +object_property_add_bool(obj, "vfio-no-msix-emulation",
> + spapr_get_msix_emulation, NULL, NULL);
>  }
>  
>  static void spapr_machine_finalizefn(Object *obj)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ae9098d..4a03085 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1580,6 +1580,19 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int 
> pos, Error **errp)
>   */
>  memory_region_set_enabled(>pdev.msix_pba_mmio, false);
>  
> +/*
> + * The emulated machine may provide a paravirt interface for MSIX setup
> + * so it is not strictly necessary to emulate MSIX here. This becomes
> + * helpful when frequently accessed MMIO registers are located in
> + * subpages adjacent to the MSIX table but the MSIX data containing page
> + * cannot be mapped because of a host page size bigger than the MSIX 
> table
> + * alignment.
> + */
> +if (object_property_get_bool(OBJECT(qdev_get_machine()),
> + "vfio-no-msix-emulation", NULL)) {
> +memory_region_set_enabled(>pdev.msix_table_mmio, false);
> +}
> +
>  return 0;
>  }
>  

-- 
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 qemu v7 3/4] vfio-pci: Allow mmap of MSIX BAR

[David Gibson]

On Fri, Feb 09, 2018 at 06:55:02PM +1100, Alexey Kardashevskiy wrote:
> At the moment we unconditionally avoid mapping MSIX data of a BAR and
> emulate MSIX table in QEMU. However it is 1) not always necessary as
> a platform may prodive a paravirt interface for MSIX configuration;
> 2) can affect the speed of MMIO access by emulating them in QEMU when
> frequently accessed registers share same system page with MSIX data,
> this is particularly a problem for systems with the page size bigger
> than 4KB.
> 
> A new capability - VFIO_REGION_INFO_CAP_MSIX_MAPPABLE - has been added
> to the kernel [1] which tells the userspace that mapping of the MSIX data
> is possible now. This makes use of it so from now on QEMU tries mapping
> the entire BAR as a whole and emulate MSIX on top of that.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a32295c612c57990d17fb0f41e7134394b2f35f6
> 
> Signed-off-by: Alexey Kardashevskiy 

Reviewed-by: David Gibson 

> ---
> Changes:
> v7:
> * test iova/llsize against pgmask in vfio_listener_region_add/del
> * s/vfio_is_cap_present/vfio_has_region_cap/
> * added comments here and there
> * s/vdev->msix->table_bar/region-nr/
> ---
>  include/hw/vfio/vfio-common.h |  1 +
>  hw/vfio/common.c  | 15 +++
>  hw/vfio/pci.c |  9 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index f3a2ac9..42dd2b0 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -171,6 +171,7 @@ int vfio_get_region_info(VFIODevice *vbasedev, int index,
>   struct vfio_region_info **info);
>  int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t type,
>   uint32_t subtype, struct vfio_region_info 
> **info);
> +bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t 
> cap_type);
>  #endif
>  extern const MemoryListener vfio_prereg_listener;
>  
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 736f271..b99ae77 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1464,6 +1464,21 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, 
> uint32_t type,
>  return -ENODEV;
>  }
>  
> +bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type)
> +{
> +struct vfio_region_info *info = NULL;
> +bool ret = false;
> +
> +if (!vfio_get_region_info(vbasedev, region, )) {
> +if (vfio_get_region_info_cap(info, cap_type)) {
> +ret = true;
> +}
> +g_free(info);
> +}
> +
> +return ret;
> +}
> +
>  /*
>   * Interfaces for IBM EEH (Enhanced Error Handling)
>   */
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 879510c..ae9098d 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1294,6 +1294,15 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice 
> *vdev)
>  VFIORegion *region = >bars[vdev->msix->table_bar].region;
>  
>  /*
> + * If the host driver allows mapping of a MSIX data, we are going to
> + * do map the entire BAR and emulate MSIX table on top of that.
> + */
> +if (vfio_has_region_cap(>vbasedev, region->nr,
> +VFIO_REGION_INFO_CAP_MSIX_MAPPABLE)) {
> +return;
> +}
> +
> +/*
>   * We expect to find a single mmap covering the whole BAR, anything else
>   * means it's either unsupported or already setup.
>   */

-- 
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 qemu v7 2/4] vfio/pci: Relax DMA map errors for MMIO regions

[David Gibson]

On Tue, Feb 13, 2018 at 04:36:30PM +1100, David Gibson wrote:
> On Tue, Feb 13, 2018 at 12:15:52PM +1100, Alexey Kardashevskiy wrote:
> > On 13/02/18 03:06, Alex Williamson wrote:
> > > On Mon, 12 Feb 2018 18:05:54 +1100
> > > Alexey Kardashevskiy  wrote:
> > > 
> > >> On 12/02/18 16:19, David Gibson wrote:
> > >>> On Fri, Feb 09, 2018 at 06:55:01PM +1100, Alexey Kardashevskiy wrote:  
> >  At the moment if vfio_memory_listener is registered in the system 
> >  memory
> >  address space, it maps/unmaps every RAM memory region for DMA.
> >  It expects system page size aligned memory sections so vfio_dma_map
> >  would not fail and so far this has been the case. A mapping failure
> >  would be fatal. A side effect of such behavior is that some MMIO pages
> >  would not be mapped silently.
> > 
> >  However we are going to change MSIX BAR handling so we will end having
> >  non-aligned sections in vfio_memory_listener (more details is in
> >  the next patch) and vfio_dma_map will exit QEMU.
> > 
> >  In order to avoid fatal failures on what previously was not a failure 
> >  and
> >  was just silently ignored, this checks the section alignment to
> >  the smallest supported IOMMU page size and prints an error if not 
> >  aligned;
> >  it also prints an error if vfio_dma_map failed despite the page size 
> >  check.
> >  Both errors are not fatal; only MMIO RAM regions are checked
> >  (aka "RAM device" regions).
> > 
> >  If the amount of errors printed is overwhelming, the MSIX relocation
> >  could be used to avoid excessive error output.
> > 
> >  This is unlikely to cause any behavioral change.
> > 
> >  Signed-off-by: Alexey Kardashevskiy   
> > >>>
> > >>> There are some relatively superficial problems noted below.
> > >>>
> > >>> But more fundamentally, this feels like it's extending an existing
> > >>> hack past the point of usefulness.
> > >>>
> > >>> The explicit check for is_ram_device() here has always bothered me -
> > >>> it's not like a real bus bridge magically knows whether a target
> > >>> address maps to RAM or not.
> > >>>
> > >>> What I think is really going on is that even for systems without an
> > >>> IOMMU, it's not really true to say that the PCI address space maps
> > >>> directly onto address_space_memory.  Instead, there's a large, but
> > >>> much less than 2^64 sized, "upstream window" at address 0 on the PCI
> > >>> bus, which is identity mapped to the system bus.  Details will vary
> > >>> with the system, but in practice we expect nothing but RAM to be in
> > >>> that window.  Addresses not within that window won't be mapped to the
> > >>> system bus but will just be broadcast on the PCI bus and might be
> > >>> picked up as a p2p transaction.  
> > >>
> > >> Currently this p2p works only via the IOMMU, direct p2p is not possible 
> > >> as
> > >> the guest needs to know physical MMIO addresses to make p2p work and it
> > >> does not.
> > > 
> > > /me points to the Direct Translated P2P section of the ACS spec, though
> > > it's as prone to spoofing by the device as ATS.  In any case, p2p
> > > reflected from the IOMMU is still p2p and offloads the CPU even if
> > > bandwidth suffers vs bare metal depending on if the data doubles back
> > > over any links.  Thanks,
> > 
> > Sure, I was just saying that p2p via IOMMU won't be as simple as broadcast
> > on the PCI bus, IOMMU needs to be programmed in advance to make this work,
> > and current that broadcast won't work for the passed through devices.
> 
> Well, sure, p2p in a guest with passthrough devices clearly needs to
> be translated through the IOMMU (and p2p from a passthrough to an
> emulated device is essentially impossible).
> 
> But.. what does that have to do with this code.  This is the memory
> area watcher, looking for memory regions being mapped directly into
> the PCI space.  NOT IOMMU regions, since those are handled separately
> by wiring up the IOMMU notifier.  This will only trigger if RAM-like,
> non-RAM regions are put into PCI space *not* behind an IOMMMU.

Duh, sorry, realised I was mixing up host and guest IOMMU.  I guess
the point here is that this will map RAM-like devices into the host
IOMMU when there is no guest IOMMU, allowing p2p transactions between
passthrough devices (though not from passthrough to emulated devices).

The conditions still seem kind of awkward to me, but I guess it makes
sense.

-- 
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 qemu v7 2/4] vfio/pci: Relax DMA map errors for MMIO regions

[David Gibson]

On Tue, Feb 13, 2018 at 12:15:52PM +1100, Alexey Kardashevskiy wrote:
> On 13/02/18 03:06, Alex Williamson wrote:
> > On Mon, 12 Feb 2018 18:05:54 +1100
> > Alexey Kardashevskiy  wrote:
> > 
> >> On 12/02/18 16:19, David Gibson wrote:
> >>> On Fri, Feb 09, 2018 at 06:55:01PM +1100, Alexey Kardashevskiy wrote:  
>  At the moment if vfio_memory_listener is registered in the system memory
>  address space, it maps/unmaps every RAM memory region for DMA.
>  It expects system page size aligned memory sections so vfio_dma_map
>  would not fail and so far this has been the case. A mapping failure
>  would be fatal. A side effect of such behavior is that some MMIO pages
>  would not be mapped silently.
> 
>  However we are going to change MSIX BAR handling so we will end having
>  non-aligned sections in vfio_memory_listener (more details is in
>  the next patch) and vfio_dma_map will exit QEMU.
> 
>  In order to avoid fatal failures on what previously was not a failure and
>  was just silently ignored, this checks the section alignment to
>  the smallest supported IOMMU page size and prints an error if not 
>  aligned;
>  it also prints an error if vfio_dma_map failed despite the page size 
>  check.
>  Both errors are not fatal; only MMIO RAM regions are checked
>  (aka "RAM device" regions).
> 
>  If the amount of errors printed is overwhelming, the MSIX relocation
>  could be used to avoid excessive error output.
> 
>  This is unlikely to cause any behavioral change.
> 
>  Signed-off-by: Alexey Kardashevskiy   
> >>>
> >>> There are some relatively superficial problems noted below.
> >>>
> >>> But more fundamentally, this feels like it's extending an existing
> >>> hack past the point of usefulness.
> >>>
> >>> The explicit check for is_ram_device() here has always bothered me -
> >>> it's not like a real bus bridge magically knows whether a target
> >>> address maps to RAM or not.
> >>>
> >>> What I think is really going on is that even for systems without an
> >>> IOMMU, it's not really true to say that the PCI address space maps
> >>> directly onto address_space_memory.  Instead, there's a large, but
> >>> much less than 2^64 sized, "upstream window" at address 0 on the PCI
> >>> bus, which is identity mapped to the system bus.  Details will vary
> >>> with the system, but in practice we expect nothing but RAM to be in
> >>> that window.  Addresses not within that window won't be mapped to the
> >>> system bus but will just be broadcast on the PCI bus and might be
> >>> picked up as a p2p transaction.  
> >>
> >> Currently this p2p works only via the IOMMU, direct p2p is not possible as
> >> the guest needs to know physical MMIO addresses to make p2p work and it
> >> does not.
> > 
> > /me points to the Direct Translated P2P section of the ACS spec, though
> > it's as prone to spoofing by the device as ATS.  In any case, p2p
> > reflected from the IOMMU is still p2p and offloads the CPU even if
> > bandwidth suffers vs bare metal depending on if the data doubles back
> > over any links.  Thanks,
> 
> Sure, I was just saying that p2p via IOMMU won't be as simple as broadcast
> on the PCI bus, IOMMU needs to be programmed in advance to make this work,
> and current that broadcast won't work for the passed through devices.

Well, sure, p2p in a guest with passthrough devices clearly needs to
be translated through the IOMMU (and p2p from a passthrough to an
emulated device is essentially impossible).

But.. what does that have to do with this code.  This is the memory
area watcher, looking for memory regions being mapped directly into
the PCI space.  NOT IOMMU regions, since those are handled separately
by wiring up the IOMMU notifier.  This will only trigger if RAM-like,
non-RAM regions are put into PCI space *not* behind an IOMMMU.

-- 
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 v3] spapr: set vsmt to MAX(8, smp_threads)

[David Gibson]

On Mon, Feb 12, 2018 at 12:11:27PM +0100, Greg Kurz wrote:
> On Sat, 10 Feb 2018 20:23:07 +1100
> David Gibson  wrote:
> 
> > On Fri, Feb 09, 2018 at 03:06:49PM +0100, Greg Kurz wrote:
> > > On Fri,  9 Feb 2018 09:18:58 +0100
> > > Laurent Vivier  wrote:
> > >   
> > > > We ignore silently the value of smp_threads when we set
> > > > the default VSMT value, and if smp_threads is greater than VSMT
> > > > kernel is going into trouble later.
> > > >   
> > > 
> > > Hi Laurent,
> > > 
> > > I've looked a bit more and I'm not sure what kernel troubles you're 
> > > referring to,
> > > but several places in QEMU where we use kvm_ppc_smt() later on do assume 
> > > that
> > > smp_threads > kvm_ppc_smt(). Basically, everywhere we compute a vCPU id:
> > > 
> > > In spapr_init_cpus() when creating DRC connectors:
> > > 
> > > int core_id = i * smp_threads;
> > > 
> > > if (mc->has_hotpluggable_cpus) {
> > > spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
> > >(core_id / smp_threads) * smt);
> > > }
> > > 
> > > or in spapr_cpu_core_realize() when creating vCPUs:
> > > 
> > > cpu->vcpu_id = (cc->core_id * spapr->vsmt / smp_threads) + i;
> > > 
> > > It is visible by adding some printfs in the current code base. This is 
> > > what
> > > happens when passing -smp cores=2,threads=16 without your patch:
> > > 
> > > DRC connector to vcpu_id 0
> > > CPU vcpu_id 0
> > > CPU vcpu_id 1
> > > CPU vcpu_id 2
> > > CPU vcpu_id 3
> > > CPU vcpu_id 4
> > > CPU vcpu_id 5
> > > CPU vcpu_id 6
> > > CPU vcpu_id 7
> > > CPU vcpu_id 8
> > > CPU vcpu_id 9
> > > CPU vcpu_id 10
> > > CPU vcpu_id 11
> > > CPU vcpu_id 12
> > > CPU vcpu_id 13
> > > CPU vcpu_id 14
> > > CPU vcpu_id 15
> > > DRC connector to vcpu_id 8
> > > ^^^
> > >  should be 16
> > > CPU vcpu_id 8
> > >^^^
> > >should start numbering at 16
> > > CPU vcpu_id 9
> > > CPU vcpu_id 10
> > > CPU vcpu_id 11
> > > CPU vcpu_id 12
> > > CPU vcpu_id 13
> > > CPU vcpu_id 14
> > > CPU vcpu_id 15
> > > CPU vcpu_id 16
> > > CPU vcpu_id 17
> > > CPU vcpu_id 18
> > > CPU vcpu_id 19
> > > CPU vcpu_id 20
> > > CPU vcpu_id 21
> > > CPU vcpu_id 22
> > > CPU vcpu_id 23
> > > qemu-system-ppc64: kvm_init_vcpu failed: File exists
> > >   
> > >  CPU 8 already created by the first core
> > > 
> > > I'm not feeling comfortable with the rest of the code silently depending 
> > > on
> > > the fact that spapr_set_vsmt_mode() terminates QEMU if it cannot enforce
> > > smp_threads <= kvm_ppc_smt().  
> > 
> > I'm not quite sure what you're suggesting as an alternative, though.
> > 
> 
> I haven't suggested anything yet :)
> 
> But I was thinking of:
> - having a single function to compute the vcpu_id, instead of open-coding
>   the formula in several places like the current code does,

That seems like a good idea.

> - this function should ensure all pre-requisites to compute the vcpu_id are
>   met (including trying to set VSMT) or return an error

I'm much more dubious about this.  Checks that can be performed
passively I'm ok with, but I think something that looks like a simple
calculation helper shouldn't be having side-effects like poking at KVM
state.

-- 
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] vhost-user: fix memory leak

[linzhecheng]

g_free() was moved from vhost_net_cleanup in commit e6bcb1b, so we should
free net after vhost_net_cleanup

Signed-off-by: linzhecheng 

diff --git a/net/vhost-user.c b/net/vhost-user.c
index cb45512506..d024573e45 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -109,6 +109,7 @@ static int vhost_user_start(int queues, NetClientState 
*ncs[], CharBackend *be)
 err:
 if (net) {
 vhost_net_cleanup(net);
+g_free(net);
 }
 vhost_user_stop(i, ncs);
 return -1;
-- 
2.12.2.windows.2

[Qemu-devel] [PATCH v13 30/30] sdhci: add Spec v4.2 register definitions

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sd/sdhci-internal.h |  9 +
 hw/sd/sdhci.c  | 14 ++
 2 files changed, 23 insertions(+)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 0092627076..e1bb733aed 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -198,6 +198,10 @@ FIELD(SDHC_HOSTCTL2, V18_ENA,  3, 1); /* UHS-I 
only */
 FIELD(SDHC_HOSTCTL2, DRIVER_STRENGTH,  4, 2); /* UHS-I only */
 FIELD(SDHC_HOSTCTL2, EXECUTE_TUNING,   6, 1); /* UHS-I only */
 FIELD(SDHC_HOSTCTL2, SAMPLING_CLKSEL,  7, 1); /* UHS-I only */
+FIELD(SDHC_HOSTCTL2, UHS_II_ENA,   8, 1); /* since v4 */
+FIELD(SDHC_HOSTCTL2, ADMA2_LENGTH,10, 1); /* since v4 */
+FIELD(SDHC_HOSTCTL2, CMD23_ENA,   11, 1); /* since v4 */
+FIELD(SDHC_HOSTCTL2, VERSION4,12, 1); /* since v4 */
 FIELD(SDHC_HOSTCTL2, ASYNC_INT,   14, 1);
 FIELD(SDHC_HOSTCTL2, PRESET_ENA,  15, 1);
 
@@ -216,10 +220,12 @@ FIELD(SDHC_CAPAB, SUSPRESUME, 23, 1);
 FIELD(SDHC_CAPAB, V33,24, 1);
 FIELD(SDHC_CAPAB, V30,25, 1);
 FIELD(SDHC_CAPAB, V18,26, 1);
+FIELD(SDHC_CAPAB, BUS64BIT_V4,27, 1); /* since v4.10 */
 FIELD(SDHC_CAPAB, BUS64BIT,   28, 1); /* since v2 */
 FIELD(SDHC_CAPAB, ASYNC_INT,  29, 1); /* since v3 */
 FIELD(SDHC_CAPAB, SLOT_TYPE,  30, 2); /* since v3 */
 FIELD(SDHC_CAPAB, BUS_SPEED,  32, 3); /* since v3 */
+FIELD(SDHC_CAPAB, UHS_II, 35, 8); /* since v4.20 */
 FIELD(SDHC_CAPAB, DRIVER_STRENGTH,36, 3); /* since v3 */
 FIELD(SDHC_CAPAB, DRIVER_TYPE_A,  36, 1); /* since v3 */
 FIELD(SDHC_CAPAB, DRIVER_TYPE_C,  37, 1); /* since v3 */
@@ -228,12 +234,15 @@ FIELD(SDHC_CAPAB, TIMER_RETUNING, 40, 4); /* since v3 
*/
 FIELD(SDHC_CAPAB, SDR50_TUNING,   45, 1); /* since v3 */
 FIELD(SDHC_CAPAB, RETUNING_MODE,  46, 2); /* since v3 */
 FIELD(SDHC_CAPAB, CLOCK_MULT, 48, 8); /* since v3 */
+FIELD(SDHC_CAPAB, ADMA3,  59, 1); /* since v4.20 */
+FIELD(SDHC_CAPAB, V18_VDD2,   60, 1); /* since v4.20 */
 
 /* HWInit Maximum Current Capabilities Register 0x0 */
 #define SDHC_MAXCURR   0x48
 FIELD(SDHC_MAXCURR, V33_VDD1,  0, 8);
 FIELD(SDHC_MAXCURR, V30_VDD1,  8, 8);
 FIELD(SDHC_MAXCURR, V18_VDD1, 16, 8);
+FIELD(SDHC_MAXCURR, V18_VDD2, 32, 8); /* since v4.20 */
 
 /* W Force Event Auto CMD12 Error Interrupt Register 0x */
 #define SDHC_FEAER 0x50
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 0cd968fc6b..74b1802503 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -91,6 +91,20 @@ static void sdhci_check_capareg(SDHCIState *s, Error **errp)
 bool unit_mhz;
 
 switch (s->sd_spec_version) {
+case 4:
+val = FIELD_EX64(s->capareg, SDHC_CAPAB, BUS64BIT_V4);
+msk = FIELD_DP64(msk, SDHC_CAPAB, BUS64BIT_V4, 0);
+trace_sdhci_capareg("64-bit system bus (v4)", val);
+
+val = FIELD_EX64(s->capareg, SDHC_CAPAB, UHS_II);
+msk = FIELD_DP64(msk, SDHC_CAPAB, UHS_II, 0);
+trace_sdhci_capareg("UHS-II", val);
+
+val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA3);
+msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA3, 0);
+trace_sdhci_capareg("ADMA3", val);
+
+/* fallback */
 case 3:
 val = FIELD_EX64(s->capareg, SDHC_CAPAB, ASYNC_INT);
 trace_sdhci_capareg("async interrupt", val);
-- 
2.16.1

Re: [Qemu-devel] 答复: [PATCH] vhost-user: fix memory leak

[Philippe Mathieu-Daudé]

On 02/13/2018 01:16 AM, linzhecheng wrote:
>> -邮件原件-
>> 发件人: Philippe Mathieu-Daudé [mailto:philippe.mathieu.da...@gmail.com]
>> 代表 Philippe Mathieu-Daudé
>> 发送时间: 2018年2月13日 11:54
>> 收件人: linzhecheng ; qemu-devel@nongnu.org
>> 抄送: pbonz...@redhat.com; wangxin (U) ;
>> lidonglin ; m...@redhat.com
>> 主题: Re: [Qemu-devel] [PATCH] vhost-user: fix memory leak
>>
>> Hi Linzhecheng,
>>
>> On 02/12/2018 11:53 PM, linzhecheng wrote:
>>> fix memory leak
>>>
>>> Signed-off-by: linzhecheng 
>>>
>>> diff --git a/net/vhost-user.c b/net/vhost-user.c index
>>> cb45512506..d024573e45 100644
>>> --- a/net/vhost-user.c
>>> +++ b/net/vhost-user.c
>>> @@ -109,6 +109,7 @@ static int vhost_user_start(int queues,
>>> NetClientState *ncs[], CharBackend *be)
>>>  err:
>>>  if (net) {
>>>  vhost_net_cleanup(net);
>>> +g_free(net);
>>
>> I think this g_free() belongs to vhost_net_cleanup() in net/vhost_net.c:
> I think your qemu version is out of date,  g_free was moved from 
> vhost_net_cleanup in commit e6bcb1b

Now reading e6bcb1b I can understand your patch.

Can you add a reference to this commit in your patch description?

"g_free was moved from vhost_net_cleanup in commit e6bcb1b" might be enough.

Adding reference:
Reviewed-by: Philippe Mathieu-Daudé 

>>
>> void vhost_net_cleanup(struct vhost_net *net) {
>> vhost_dev_cleanup(>dev);
>> g_free(net);
>> }
>>
>> Regards,
>>
>> Phil.
>>
>>>  }
>>>  vhost_user_stop(i, ncs);
>>>  return -1;
>>>

[Qemu-devel] [PATCH v13 25/30] hw/arm/fsl-imx6: implement SDHCI Spec. v3

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 hw/arm/fsl-imx6.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index e6559a8b12..b6ac72de27 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -27,6 +27,8 @@
 #include "chardev/char.h"
 #include "qemu/error-report.h"
 
+#define IMX6_ESDHC_CAPABILITIES 0x057834b4
+
 #define NAME_SIZE 20
 
 static void fsl_imx6_init(Object *obj)
@@ -348,6 +350,11 @@ static void fsl_imx6_realize(DeviceState *dev, Error 
**errp)
 { FSL_IMX6_uSDHC4_ADDR, FSL_IMX6_uSDHC4_IRQ },
 };
 
+/* UHS-I SDIO3.0 SDR104 1.8V ADMA */
+object_property_set_uint(OBJECT(>esdhc[i]), 3, "sd-spec-version",
+ );
+object_property_set_uint(OBJECT(>esdhc[i]), IMX6_ESDHC_CAPABILITIES,
+ "capareg", );
 object_property_set_bool(OBJECT(>esdhc[i]), true, "realized", );
 if (err) {
 error_propagate(errp, err);
-- 
2.16.1

[Qemu-devel] [PATCH v13 22/30] sdhci: implement CMD/DAT[] fields in the Present State register

[Philippe Mathieu-Daudé]

[based on a patch from Alistair Francis 
 from qemu/xilinx tag xilinx-v2015.2]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci-internal.h |  2 ++
 include/hw/sd/sd.h |  4 
 hw/sd/core.c   | 34 ++
 hw/sd/sd.c | 16 
 hw/sd/sdhci.c  |  4 
 hw/sd/trace-events |  2 ++
 6 files changed, 62 insertions(+)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 5c69270988..0092627076 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -82,6 +82,8 @@
 #define SDHC_CARD_PRESENT  0x0001
 #define SDHC_CARD_DETECT   0x0004
 #define SDHC_WRITE_PROTECT 0x0008
+FIELD(SDHC_PRNSTS, DAT_LVL,20, 4);
+FIELD(SDHC_PRNSTS, CMD_LVL,24, 1);
 #define TRANSFERRING_DATA(x)   \
 ((x) & (SDHC_DOING_READ | SDHC_DOING_WRITE))
 
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index f086679493..bf1eb0713c 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -103,6 +103,8 @@ typedef struct {
 uint8_t (*read_data)(SDState *sd);
 bool (*data_ready)(SDState *sd);
 void (*set_voltage)(SDState *sd, uint16_t millivolts);
+uint8_t (*get_dat_lines)(SDState *sd);
+bool (*get_cmd_line)(SDState *sd);
 void (*enable)(SDState *sd, bool enable);
 bool (*get_inserted)(SDState *sd);
 bool (*get_readonly)(SDState *sd);
@@ -150,6 +152,8 @@ void sd_enable(SDState *sd, bool enable);
  * an SDBus rather than directly with SDState)
  */
 void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts);
+uint8_t sdbus_get_dat_lines(SDBus *sdbus);
+bool sdbus_get_cmd_line(SDBus *sdbus);
 int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response);
 void sdbus_write_data(SDBus *sd, uint8_t value);
 uint8_t sdbus_read_data(SDBus *sd);
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 6d198ea775..3c6eae6c88 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -41,6 +41,40 @@ static SDState *get_card(SDBus *sdbus)
 return SD_CARD(kid->child);
 }
 
+uint8_t sdbus_get_dat_lines(SDBus *sdbus)
+{
+SDState *slave = get_card(sdbus);
+uint8_t dat_lines = 0b; /* 4 bit bus width */
+
+if (slave) {
+SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+
+if (sc->get_dat_lines) {
+dat_lines = sc->get_dat_lines(slave);
+}
+}
+trace_sdbus_get_dat_lines(sdbus_name(sdbus), dat_lines);
+
+return dat_lines;
+}
+
+bool sdbus_get_cmd_line(SDBus *sdbus)
+{
+SDState *slave = get_card(sdbus);
+bool cmd_line = true;
+
+if (slave) {
+SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+
+if (sc->get_cmd_line) {
+cmd_line = sc->get_cmd_line(slave);
+}
+}
+trace_sdbus_get_cmd_line(sdbus_name(sdbus), cmd_line);
+
+return cmd_line;
+}
+
 void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
 {
 SDState *card = get_card(sdbus);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a8d7a522c0..9ac9b63ff8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -126,8 +126,20 @@ struct SDState {
 BlockBackend *blk;
 
 bool enable;
+uint8_t dat_lines;
+bool cmd_line;
 };
 
+static uint8_t sd_get_dat_lines(SDState *sd)
+{
+return sd->enable ? sd->dat_lines : 0;
+}
+
+static bool sd_get_cmd_line(SDState *sd)
+{
+return sd->enable ? sd->cmd_line : false;
+}
+
 static void sd_set_voltage(SDState *sd, uint16_t millivolts)
 {
 switch (millivolts) {
@@ -457,6 +469,8 @@ static void sd_reset(DeviceState *dev)
 sd->blk_len = 0x200;
 sd->pwd_len = 0;
 sd->expecting_acmd = false;
+sd->dat_lines = 0xf;
+sd->cmd_line = true;
 sd->multi_blk_cnt = 0;
 }
 
@@ -1939,6 +1953,8 @@ static void sd_class_init(ObjectClass *klass, void *data)
 dc->bus_type = TYPE_SD_BUS;
 
 sc->set_voltage = sd_set_voltage;
+sc->get_dat_lines = sd_get_dat_lines;
+sc->get_cmd_line = sd_get_cmd_line;
 sc->do_command = sd_do_command;
 sc->write_data = sd_write_data;
 sc->read_data = sd_read_data;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 4e717117e1..0cd968fc6b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1003,6 +1003,10 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, 
unsigned size)
 break;
 case SDHC_PRNSTS:
 ret = s->prnsts;
+ret = FIELD_DP32(ret, SDHC_PRNSTS, DAT_LVL,
+ sdbus_get_dat_lines(>sdbus));
+ret = FIELD_DP32(ret, SDHC_PRNSTS, CMD_LVL,
+ sdbus_get_cmd_line(>sdbus));
 break;
 case SDHC_HOSTCTL:
 ret = s->hostctl1 | (s->pwrcon << 8) | (s->blkgap << 16) |
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 84d2f398b1..0f8536db32 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -5,6 +5,8 @@ sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg, 
uint8_t crc) "@%s
 

[Qemu-devel] [PATCH v13 28/30] sdhci: check Spec v3 capabilities qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 tests/sdhci-test.c | 12 
 tests/Makefile.include |  1 +
 2 files changed, 13 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 898c43ff4f..39d0f87788 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -42,10 +42,22 @@ static const struct sdhci_t {
 { "arm","smdkc210",
 {0x1251, 2, 0,  {1, 0x5e80080} } },
 
+/* i.MX 6 */
+{ "arm","sabrelite",
+{0x0219, 3, 0,  {1, 0x057834b4} } },
+
+/* BCM2835 */
+{ "arm","raspi2",
+{0x3f30, 3, 52, {0, 0x052134b4} } },
+
 /* Zynq-7000 */
 { "arm","xilinx-zynq-a9",   /* Datasheet: UG585 (v1.12.1) */
 {0xe010, 2, 0,  {1, 0x69ec0080} } },
 
+/* ZynqMP */
+{ "aarch64", "xlnx-zcu102", /* Datasheet: UG1085 (v1.7) */
+{0xff16, 3, 0,  {1, 0x280737ec6481} } },
+
 };
 
 typedef struct QSDHCI {
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 52be9b3fa5..278c13aa93 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -371,6 +371,7 @@ check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
 check-qtest-arm-y += tests/sdhci-test$(EXESUF)
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
+check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
 
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
-- 
2.16.1

Re: [Qemu-devel] [PATCH] tcg: Improve tcg_gen_muli_i32/i64

[Philippe Mathieu-Daudé]

On 02/11/2018 05:39 PM, Richard Henderson wrote:
> Convert multiplication by power of two to left shift.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  tcg/tcg-op.c | 24 ++--
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index 3467787323..34b96d68f3 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -277,9 +277,15 @@ void tcg_gen_setcondi_i32(TCGCond cond, TCGv_i32 ret,
>  
>  void tcg_gen_muli_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2)
>  {
> -TCGv_i32 t0 = tcg_const_i32(arg2);
> -tcg_gen_mul_i32(ret, arg1, t0);
> -tcg_temp_free_i32(t0);
> +if (arg2 == 0) {
> +tcg_gen_movi_i32(ret, 0);
> +} else if (is_power_of_2(arg2)) {
> +tcg_gen_shli_i32(ret, arg1, ctz32(arg2));
> +} else {
> +TCGv_i32 t0 = tcg_const_i32(arg2);
> +tcg_gen_mul_i32(ret, arg1, t0);
> +tcg_temp_free_i32(t0);
> +}
>  }
>  
>  void tcg_gen_div_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2)
> @@ -1430,9 +1436,15 @@ void tcg_gen_setcondi_i64(TCGCond cond, TCGv_i64 ret,
>  
>  void tcg_gen_muli_i64(TCGv_i64 ret, TCGv_i64 arg1, int64_t arg2)
>  {
> -TCGv_i64 t0 = tcg_const_i64(arg2);
> -tcg_gen_mul_i64(ret, arg1, t0);
> -tcg_temp_free_i64(t0);
> +if (arg2 == 0) {
> +tcg_gen_movi_i64(ret, 0);
> +} else if (is_power_of_2(arg2)) {
> +tcg_gen_shli_i64(ret, arg1, ctz64(arg2));
> +} else {
> +TCGv_i64 t0 = tcg_const_i64(arg2);
> +tcg_gen_mul_i64(ret, arg1, t0);
> +tcg_temp_free_i64(t0);
> +}
>  }
>  
>  void tcg_gen_div_i64(TCGv_i64 ret, TCGv_i64 arg1, TCGv_i64 arg2)
> 

[Qemu-devel] [PATCH v13 19/30] sdhci: implement the Host Control 2 register (tuning sequence)

[Philippe Mathieu-Daudé]

[based on a patch from Alistair Francis 
 from qemu/xilinx tag xilinx-v2015.2]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci-internal.h | 10 ++
 include/hw/sd/sdhci.h  |  1 +
 hw/sd/sdhci.c  | 22 +++---
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index bfb39d614b..5c69270988 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -189,6 +189,16 @@ FIELD(SDHC_ACMD12ERRSTS, TIMEOUT_ERR,  1, 1);
 FIELD(SDHC_ACMD12ERRSTS, CRC_ERR,  2, 1);
 FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR,4, 1);
 
+/* Host Control Register 2 (since v3) */
+#define SDHC_HOSTCTL2  0x3E
+FIELD(SDHC_HOSTCTL2, UHS_MODE_SEL, 0, 3);
+FIELD(SDHC_HOSTCTL2, V18_ENA,  3, 1); /* UHS-I only */
+FIELD(SDHC_HOSTCTL2, DRIVER_STRENGTH,  4, 2); /* UHS-I only */
+FIELD(SDHC_HOSTCTL2, EXECUTE_TUNING,   6, 1); /* UHS-I only */
+FIELD(SDHC_HOSTCTL2, SAMPLING_CLKSEL,  7, 1); /* UHS-I only */
+FIELD(SDHC_HOSTCTL2, ASYNC_INT,   14, 1);
+FIELD(SDHC_HOSTCTL2, PRESET_ENA,  15, 1);
+
 /* HWInit Capabilities Register 0x05E80080 */
 #define SDHC_CAPAB 0x40
 FIELD(SDHC_CAPAB, TOCLKFREQ,   0, 6);
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 54594845ce..fd606e9928 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -73,6 +73,7 @@ typedef struct SDHCIState {
 uint16_t norintsigen;  /* Normal Interrupt Signal Enable Register */
 uint16_t errintsigen;  /* Error Interrupt Signal Enable Register */
 uint16_t acmd12errsts; /* Auto CMD12 error status register */
+uint16_t hostctl2; /* Host Control 2 */
 uint64_t admasysaddr;  /* ADMA System Address Register */
 
 /* Read-only registers */
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 9a8cdd551c..1dbcb99f52 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -408,14 +408,29 @@ static void sdhci_end_transfer(SDHCIState *s)
 static void sdhci_read_block_from_card(SDHCIState *s)
 {
 int index = 0;
+uint8_t data;
+const uint16_t blk_size = s->blksize & BLOCK_SIZE_MASK;
 
 if ((s->trnmod & SDHC_TRNS_MULTI) &&
 (s->trnmod & SDHC_TRNS_BLK_CNT_EN) && (s->blkcnt == 0)) {
 return;
 }
 
-for (index = 0; index < (s->blksize & BLOCK_SIZE_MASK); index++) {
-s->fifo_buffer[index] = sdbus_read_data(>sdbus);
+for (index = 0; index < blk_size; index++) {
+data = sdbus_read_data(>sdbus);
+if (!FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, EXECUTE_TUNING)) {
+/* Device is not in tunning */
+s->fifo_buffer[index] = data;
+}
+}
+
+if (FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, EXECUTE_TUNING)) {
+/* Device is in tunning */
+s->hostctl2 &= ~R_SDHC_HOSTCTL2_EXECUTE_TUNING_MASK;
+s->hostctl2 |= R_SDHC_HOSTCTL2_SAMPLING_CLKSEL_MASK;
+s->prnsts &= ~(SDHC_DAT_LINE_ACTIVE | SDHC_DOING_READ |
+   SDHC_DATA_INHIBIT);
+goto read_done;
 }
 
 /* New data now available for READ through Buffer Port Register */
@@ -440,6 +455,7 @@ static void sdhci_read_block_from_card(SDHCIState *s)
 }
 }
 
+read_done:
 sdhci_update_irq(s);
 }
 
@@ -1005,7 +1021,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, 
unsigned size)
 ret = s->norintsigen | (s->errintsigen << 16);
 break;
 case SDHC_ACMD12ERRSTS:
-ret = s->acmd12errsts;
+ret = s->acmd12errsts | (s->hostctl2 << 16);
 break;
 case SDHC_CAPAB:
 ret = (uint32_t)s->capareg;
-- 
2.16.1

[Qemu-devel] [PATCH v13 27/30] hw/arm/xilinx_zynqmp: enable the UHS-I mode

[Philippe Mathieu-Daudé]

see the Xilinx datasheet "UG1085" (v1.7)

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/arm/xlnx-zynqmp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index e39ad73bec..4b93a3abd2 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -400,6 +400,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
  */
 object_property_set_uint(sdhci, 3, "sd-spec-version", );
 object_property_set_uint(sdhci, SDHCI_CAPABILITIES, "capareg", );
+object_property_set_uint(sdhci, UHS_I, "uhs", );
 object_property_set_bool(sdhci, true, "realized", );
 if (err) {
 error_propagate(errp, err);
-- 
2.16.1

[Qemu-devel] [PATCH v13 29/30] sdhci: add a check_capab_v3() qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 tests/sdhci-test.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 39d0f87788..c0b45da88a 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -16,6 +16,8 @@
 #define SDHC_CAPAB  0x40
 FIELD(SDHC_CAPAB, BASECLKFREQ,   8, 8); /* since v2 */
 FIELD(SDHC_CAPAB, SDMA, 22, 1);
+FIELD(SDHC_CAPAB, SDR,  32, 3); /* since v3 */
+FIELD(SDHC_CAPAB, DRIVER,   36, 3); /* since v3 */
 #define SDHC_HCVER  0xFE
 
 static const struct sdhci_t {
@@ -161,6 +163,20 @@ static void check_capab_sdma(QSDHCI *s, bool supported)
 g_assert_cmpuint(capab_sdma, ==, supported);
 }
 
+static void check_capab_v3(QSDHCI *s, uint8_t version)
+{
+uint64_t capab, capab_v3;
+
+if (version < 3) {
+/* before v3 those fields are RESERVED */
+capab = sdhci_readq(s, SDHC_CAPAB);
+capab_v3 = FIELD_EX64(capab, SDHC_CAPAB, SDR);
+g_assert_cmpuint(capab_v3, ==, 0);
+capab_v3 = FIELD_EX64(capab, SDHC_CAPAB, DRIVER);
+g_assert_cmpuint(capab_v3, ==, 0);
+}
+}
+
 static QSDHCI *machine_start(const struct sdhci_t *test)
 {
 QSDHCI *s = g_new0(QSDHCI, 1);
@@ -209,6 +225,7 @@ static void test_machine(const void *data)
 check_specs_version(s, test->sdhci.version);
 check_capab_capareg(s, test->sdhci.capab.reg);
 check_capab_readonly(s);
+check_capab_v3(s, test->sdhci.version);
 check_capab_sdma(s, test->sdhci.capab.sdma);
 check_capab_baseclock(s, test->sdhci.baseclock);
 
-- 
2.16.1

[Qemu-devel] [PATCH v13 24/30] hw/arm/bcm2835_peripherals: change maximum block size to 1kB

[Philippe Mathieu-Daudé]

following the datasheet.

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 hw/arm/bcm2835_peripherals.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index ca971e83e0..13b63970d7 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -19,7 +19,7 @@
 #define BCM2835_VC_PERI_BASE 0x7e00
 
 /* Capabilities for SD controller: no DMA, high-speed, default clocks etc. */
-#define BCM2835_SDHC_CAPAREG 0x52034b4
+#define BCM2835_SDHC_CAPAREG 0x52134b4
 
 static void bcm2835_peripherals_init(Object *obj)
 {
-- 
2.16.1

[Qemu-devel] [PATCH v13 16/30] hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet

[Philippe Mathieu-Daudé]

checking Xilinx datasheet "UG585" (v1.12.1)

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/arm/xilinx_zynq.c | 53 
 tests/sdhci-test.c   |  5 +
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1836a4ed45..0f76333770 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -61,6 +61,8 @@ static const int dma_irqs[8] = {
 #define SLCR_XILINX_UNLOCK_KEY  0xdf0d
 #define SLCR_XILINX_LOCK_KEY0x767b
 
+#define ZYNQ_SDHCI_CAPABILITIES 0x69ec0080  /* Datasheet: UG585 (v1.12.1) */
+
 #define ARMV7_IMM16(x) (extract32((x),  0, 12) | \
 extract32((x), 12,  4) << 16)
 
@@ -165,10 +167,8 @@ static void zynq_init(MachineState *machine)
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
 MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
-DeviceState *dev, *carddev;
+DeviceState *dev;
 SysBusDevice *busdev;
-DriveInfo *di;
-BlockBackend *blk;
 qemu_irq pic[64];
 int n;
 
@@ -247,27 +247,32 @@ static void zynq_init(MachineState *machine)
 gem_init(_table[0], 0xE000B000, pic[54-IRQ_OFFSET]);
 gem_init(_table[1], 0xE000C000, pic[77-IRQ_OFFSET]);
 
-dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE010);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
-
-di = drive_get_next(IF_SD);
-blk = di ? blk_by_legacy_dinfo(di) : NULL;
-carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
-qdev_prop_set_drive(carddev, "drive", blk, _fatal);
-object_property_set_bool(OBJECT(carddev), true, "realized", _fatal);
-
-dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
-
-di = drive_get_next(IF_SD);
-blk = di ? blk_by_legacy_dinfo(di) : NULL;
-carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
-qdev_prop_set_drive(carddev, "drive", blk, _fatal);
-object_property_set_bool(OBJECT(carddev), true, "realized", _fatal);
+for (n = 0; n < 2; n++) {
+int hci_irq = n ? 79 : 56;
+hwaddr hci_addr = n ? 0xE0101000 : 0xE010;
+DriveInfo *di;
+BlockBackend *blk;
+DeviceState *carddev;
+
+/* Compatible with:
+ * - SD Host Controller Specification Version 2.0 Part A2
+ * - SDIO Specification Version 2.0
+ * - MMC Specification Version 3.31
+ */
+dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
+qdev_prop_set_uint8(dev, "sd-spec-version", 2);
+qdev_prop_set_uint64(dev, "capareg", ZYNQ_SDHCI_CAPABILITIES);
+qdev_init_nofail(dev);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, hci_addr);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[hci_irq - IRQ_OFFSET]);
+
+di = drive_get_next(IF_SD);
+blk = di ? blk_by_legacy_dinfo(di) : NULL;
+carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
+qdev_prop_set_drive(carddev, "drive", blk, _fatal);
+object_property_set_bool(OBJECT(carddev), true, "realized",
+ _fatal);
+}
 
 dev = qdev_create(NULL, TYPE_ZYNQ_XADC);
 qdev_init_nofail(dev);
diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 24feea744a..898c43ff4f 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -41,6 +41,11 @@ static const struct sdhci_t {
 /* Exynos4210 */
 { "arm","smdkc210",
 {0x1251, 2, 0,  {1, 0x5e80080} } },
+
+/* Zynq-7000 */
+{ "arm","xilinx-zynq-a9",   /* Datasheet: UG585 (v1.12.1) */
+{0xe010, 2, 0,  {1, 0x69ec0080} } },
+
 };
 
 typedef struct QSDHCI {
-- 
2.16.1

[Qemu-devel] 答复: [PATCH] vhost-user: fix memory leak

[linzhecheng]

> -邮件原件-
> 发件人: Philippe Mathieu-Daudé [mailto:philippe.mathieu.da...@gmail.com]
> 代表 Philippe Mathieu-Daudé
> 发送时间: 2018年2月13日 11:54
> 收件人: linzhecheng ; qemu-devel@nongnu.org
> 抄送: pbonz...@redhat.com; wangxin (U) ;
> lidonglin ; m...@redhat.com
> 主题: Re: [Qemu-devel] [PATCH] vhost-user: fix memory leak
> 
> Hi Linzhecheng,
> 
> On 02/12/2018 11:53 PM, linzhecheng wrote:
> > fix memory leak
> >
> > Signed-off-by: linzhecheng 
> >
> > diff --git a/net/vhost-user.c b/net/vhost-user.c index
> > cb45512506..d024573e45 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -109,6 +109,7 @@ static int vhost_user_start(int queues,
> > NetClientState *ncs[], CharBackend *be)
> >  err:
> >  if (net) {
> >  vhost_net_cleanup(net);
> > +g_free(net);
> 
> I think this g_free() belongs to vhost_net_cleanup() in net/vhost_net.c:
I think your qemu version is out of date,  g_free was moved from 
vhost_net_cleanup in commit e6bcb1b
> 
> void vhost_net_cleanup(struct vhost_net *net) {
> vhost_dev_cleanup(>dev);
> g_free(net);
> }
> 
> Regards,
> 
> Phil.
> 
> >  }
> >  vhost_user_stop(i, ncs);
> >  return -1;
> >

[Qemu-devel] [PATCH v13 18/30] sdhci: rename the hostctl1 register

[Philippe Mathieu-Daudé]

As per the Spec v3.00

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 include/hw/sd/sdhci.h |  2 +-
 hw/sd/sdhci.c | 18 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 2a26b46f05..54594845ce 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -59,7 +59,7 @@ typedef struct SDHCIState {
 uint16_t cmdreg;   /* Command Register */
 uint32_t rspreg[4];/* Response Registers 0-3 */
 uint32_t prnsts;   /* Present State Register */
-uint8_t  hostctl;  /* Host Control Register */
+uint8_t  hostctl1; /* Host Control Register */
 uint8_t  pwrcon;   /* Power control Register */
 uint8_t  blkgap;   /* Block Gap Control Register */
 uint8_t  wakcon;   /* WakeUp Control Register */
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e7214d6f60..9a8cdd551c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -691,7 +691,7 @@ static void get_adma_description(SDHCIState *s, ADMADescr 
*dscr)
 uint32_t adma1 = 0;
 uint64_t adma2 = 0;
 hwaddr entry_addr = (hwaddr)s->admasysaddr;
-switch (SDHC_DMA_TYPE(s->hostctl)) {
+switch (SDHC_DMA_TYPE(s->hostctl1)) {
 case SDHC_CTRL_ADMA2_32:
 dma_memory_read(s->dma_as, entry_addr, (uint8_t *),
 sizeof(adma2));
@@ -880,7 +880,7 @@ static void sdhci_data_transfer(void *opaque)
 SDHCIState *s = (SDHCIState *)opaque;
 
 if (s->trnmod & SDHC_TRNS_DMA) {
-switch (SDHC_DMA_TYPE(s->hostctl)) {
+switch (SDHC_DMA_TYPE(s->hostctl1)) {
 case SDHC_CTRL_SDMA:
 if ((s->blkcnt == 1) || !(s->trnmod & SDHC_TRNS_MULTI)) {
 sdhci_sdma_transfer_single_block(s);
@@ -989,7 +989,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, 
unsigned size)
 ret = s->prnsts;
 break;
 case SDHC_HOSTCTL:
-ret = s->hostctl | (s->pwrcon << 8) | (s->blkgap << 16) |
+ret = s->hostctl1 | (s->pwrcon << 8) | (s->blkgap << 16) |
   (s->wakcon << 24);
 break;
 case SDHC_CLKCON:
@@ -1107,7 +1107,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 MASKED_WRITE(s->sdmasysad, mask, value);
 /* Writing to last byte of sdmasysad might trigger transfer */
 if (!(mask & 0xFF00) && TRANSFERRING_DATA(s->prnsts) && s->blkcnt 
&&
-s->blksize && SDHC_DMA_TYPE(s->hostctl) == SDHC_CTRL_SDMA) {
+s->blksize && SDHC_DMA_TYPE(s->hostctl1) == SDHC_CTRL_SDMA) {
 if (s->trnmod & SDHC_TRNS_MULTI) {
 sdhci_sdma_transfer_multi_blocks(s);
 } else {
@@ -1159,7 +1159,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 if (!(mask & 0xFF)) {
 sdhci_blkgap_write(s, value >> 16);
 }
-MASKED_WRITE(s->hostctl, mask, value);
+MASKED_WRITE(s->hostctl1, mask, value);
 MASKED_WRITE(s->pwrcon, mask >> 8, value >> 8);
 MASKED_WRITE(s->wakcon, mask >> 24, value >> 24);
 if (!(s->prnsts & SDHC_CARD_PRESENT) || ((s->pwrcon >> 1) & 0x7) < 5 ||
@@ -1380,7 +1380,7 @@ const VMStateDescription sdhci_vmstate = {
 VMSTATE_UINT16(cmdreg, SDHCIState),
 VMSTATE_UINT32_ARRAY(rspreg, SDHCIState, 4),
 VMSTATE_UINT32(prnsts, SDHCIState),
-VMSTATE_UINT8(hostctl, SDHCIState),
+VMSTATE_UINT8(hostctl1, SDHCIState),
 VMSTATE_UINT8(pwrcon, SDHCIState),
 VMSTATE_UINT8(blkgap, SDHCIState),
 VMSTATE_UINT8(wakcon, SDHCIState),
@@ -1598,13 +1598,13 @@ static uint64_t usdhc_read(void *opaque, hwaddr offset, 
unsigned size)
  * manipulation code see comments in a similar part of
  * usdhc_write()
  */
-hostctl = SDHC_DMA_TYPE(s->hostctl) << (8 - 3);
+hostctl = SDHC_DMA_TYPE(s->hostctl1) << (8 - 3);
 
-if (s->hostctl & SDHC_CTRL_8BITBUS) {
+if (s->hostctl1 & SDHC_CTRL_8BITBUS) {
 hostctl |= ESDHC_CTRL_8BITBUS;
 }
 
-if (s->hostctl & SDHC_CTRL_4BITBUS) {
+if (s->hostctl1 & SDHC_CTRL_4BITBUS) {
 hostctl |= ESDHC_CTRL_4BITBUS;
 }
 
-- 
2.16.1

[Qemu-devel] [PATCH v13 14/30] hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()

[Philippe Mathieu-Daudé]

We only set a 32-bit value, but this is a good practice in case this
code is used as reference.

(missed in 5efc9016e52)

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/arm/exynos4210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index e8e1d81e62..d89322c7ea 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -378,7 +378,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 DriveInfo *di;
 
 dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
-qdev_prop_set_uint32(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
+qdev_prop_set_uint64(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
 qdev_init_nofail(dev);
 
 busdev = SYS_BUS_DEVICE(dev);
-- 
2.16.1

[Qemu-devel] [PATCH v13 23/30] hw/arm/bcm2835_peripherals: implement SDHCI Spec v3

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/arm/bcm2835_peripherals.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 12e0dd11af..ca971e83e0 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -254,14 +254,19 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 memory_region_add_subregion(>peri_mr, RNG_OFFSET,
 sysbus_mmio_get_region(SYS_BUS_DEVICE(>rng), 0));
 
-/* Extended Mass Media Controller */
-object_property_set_int(OBJECT(>sdhci), BCM2835_SDHC_CAPAREG, "capareg",
-);
-if (err) {
-error_propagate(errp, err);
-return;
-}
-
+/* Extended Mass Media Controller
+ *
+ * Compatible with:
+ * - SD Host Controller Specification Version 3.0 Draft 1.0
+ * - SDIO Specification Version 3.0
+ * - MMC Specification Version 4.4
+ *
+ * For the exact details please refer to the Arasan documentation:
+ *   SD3.0_Host_AHB_eMMC4.4_Usersguide_ver5.9_jan11_10.pdf
+ */
+object_property_set_uint(OBJECT(>sdhci), 3, "sd-spec-version", );
+object_property_set_uint(OBJECT(>sdhci), BCM2835_SDHC_CAPAREG, 
"capareg",
+ );
 object_property_set_bool(OBJECT(>sdhci), true, "pending-insert-quirk",
  );
 if (err) {
-- 
2.16.1

[Qemu-devel] [PATCH v13 26/30] hw/arm/xilinx_zynqmp: fix the capabilities/spec version to match the datasheet

[Philippe Mathieu-Daudé]

checking Xilinx datasheet "UG1085" (v1.7)

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/arm/xlnx-zynqmp.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index ca398c4159..e39ad73bec 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -53,6 +53,8 @@
 #define IPI_ADDR0xFF30
 #define IPI_IRQ 64
 
+#define SDHCI_CAPABILITIES  0x280737ec6481 /* Datasheet: UG1085 (v1.7) */
+
 static const uint64_t gem_addr[XLNX_ZYNQMP_NUM_GEMS] = {
 0xFF0B, 0xFF0C, 0xFF0D, 0xFF0E,
 };
@@ -387,22 +389,27 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 sysbus_connect_irq(SYS_BUS_DEVICE(>sata), 0, gic_spi[SATA_INTR]);
 
 for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) {
-char *bus_name;
-
-object_property_set_bool(OBJECT(>sdhci[i]), true,
- "realized", );
+char *bus_name = g_strdup_printf("sd-bus%d", i);
+SysBusDevice *sbd = SYS_BUS_DEVICE(>sdhci[i]);
+Object *sdhci = OBJECT(>sdhci[i]);
+
+/* Compatible with:
+ * - SD Host Controller Specification Version 3.00
+ * - SDIO Specification Version 3.0
+ * - eMMC Specification Version 4.51
+ */
+object_property_set_uint(sdhci, 3, "sd-spec-version", );
+object_property_set_uint(sdhci, SDHCI_CAPABILITIES, "capareg", );
+object_property_set_bool(sdhci, true, "realized", );
 if (err) {
 error_propagate(errp, err);
 return;
 }
-sysbus_mmio_map(SYS_BUS_DEVICE(>sdhci[i]), 0,
-sdhci_addr[i]);
-sysbus_connect_irq(SYS_BUS_DEVICE(>sdhci[i]), 0,
-   gic_spi[sdhci_intr[i]]);
+sysbus_mmio_map(sbd, 0, sdhci_addr[i]);
+sysbus_connect_irq(sbd, 0, gic_spi[sdhci_intr[i]]);
+
 /* Alias controller SD bus to the SoC itself */
-bus_name = g_strdup_printf("sd-bus%d", i);
-object_property_add_alias(OBJECT(s), bus_name,
-  OBJECT(>sdhci[i]), "sd-bus",
+object_property_add_alias(OBJECT(s), bus_name, sdhci, "sd-bus",
   _abort);
 g_free(bus_name);
 }
-- 
2.16.1

[Qemu-devel] [PATCH v13 15/30] hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 hw/arm/exynos4210.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index d89322c7ea..06f9d1ffa4 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -377,6 +377,18 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 BlockBackend *blk;
 DriveInfo *di;
 
+/* Compatible with:
+ * - SD Host Controller Specification Version 2.0
+ * - SDIO Specification Version 2.0
+ * - MMC Specification Version 4.3
+ * - SDMA
+ * - ADMA2
+ *
+ * As this part of the Exynos4210 is not publically available,
+ * we used the "HS-MMC Controller S3C2416X RISC Microprocessor"
+ * public datasheet which is very similar (implementing
+ * MMC Specification Version 4.0 being the only difference noted)
+ */
 dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
 qdev_prop_set_uint64(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES);
 qdev_init_nofail(dev);
-- 
2.16.1

[Qemu-devel] [PATCH v13 10/30] sdhci: check the Spec v1 capabilities correctness

[Philippe Mathieu-Daudé]

Incorrect value will throw an error.

Note than Spec v2 is supported by default.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci-internal.h | 22 ++-
 hw/sd/sdhci.c  | 99 +-
 hw/sd/trace-events |  1 +
 3 files changed, 118 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index def1c7f7aa..96d7f4dde7 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -86,7 +86,9 @@
 
 /* R/W Host control Register 0x0 */
 #define SDHC_HOSTCTL   0x28
-#define SDHC_CTRL_LED  0x01
+FIELD(SDHC_HOSTCTL, LED_CTRL,  0, 1);
+FIELD(SDHC_HOSTCTL, DATATRANSFERWIDTH, 1, 1); /* SD mode only */
+FIELD(SDHC_HOSTCTL, HIGH_SPEED,2, 1);
 #define SDHC_CTRL_DMA_CHECK_MASK   0x18
 #define SDHC_CTRL_SDMA 0x00
 #define SDHC_CTRL_ADMA1_32 0x08
@@ -102,6 +104,7 @@
 /* R/W Power Control Register 0x0 */
 #define SDHC_PWRCON0x29
 #define SDHC_POWER_ON  (1 << 0)
+FIELD(SDHC_PWRCON, BUS_VOLTAGE,1, 3);
 
 /* R/W Block Gap Control Register 0x0 */
 #define SDHC_BLKGAP0x2A
@@ -124,6 +127,7 @@
 
 /* R/W Timeout Control Register 0x0 */
 #define SDHC_TIMEOUTCON0x2E
+FIELD(SDHC_TIMEOUTCON, COUNTER,0, 4);
 
 /* R/W Software Reset Register 0x0 */
 #define SDHC_SWRST 0x2F
@@ -180,17 +184,31 @@
 
 /* ROC Auto CMD12 error status register 0x0 */
 #define SDHC_ACMD12ERRSTS  0x3C
+FIELD(SDHC_ACMD12ERRSTS, TIMEOUT_ERR,  1, 1);
+FIELD(SDHC_ACMD12ERRSTS, CRC_ERR,  2, 1);
+FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR,4, 1);
 
 /* HWInit Capabilities Register 0x05E80080 */
 #define SDHC_CAPAB 0x40
-#define SDHC_CAN_DO_DMA0x0040
 #define SDHC_CAN_DO_ADMA2  0x0008
 #define SDHC_CAN_DO_ADMA1  0x0010
 #define SDHC_64_BIT_BUS_SUPPORT(1 << 28)
+FIELD(SDHC_CAPAB, TOCLKFREQ,   0, 6);
+FIELD(SDHC_CAPAB, TOUNIT,  7, 1);
+FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8);
 FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
+FIELD(SDHC_CAPAB, HIGHSPEED,  21, 1);
+FIELD(SDHC_CAPAB, SDMA,   22, 1);
+FIELD(SDHC_CAPAB, SUSPRESUME, 23, 1);
+FIELD(SDHC_CAPAB, V33,24, 1);
+FIELD(SDHC_CAPAB, V30,25, 1);
+FIELD(SDHC_CAPAB, V18,26, 1);
 
 /* HWInit Maximum Current Capabilities Register 0x0 */
 #define SDHC_MAXCURR   0x48
+FIELD(SDHC_MAXCURR, V33_VDD1,  0, 8);
+FIELD(SDHC_MAXCURR, V30_VDD1,  8, 8);
+FIELD(SDHC_MAXCURR, V18_VDD1, 16, 8);
 
 /* W Force Event Auto CMD12 Error Interrupt Register 0x */
 #define SDHC_FEAER 0x50
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index f22ed9181c..bd3581502a 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
@@ -63,6 +64,92 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
 return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
 }
 
+/* return true on error */
+static bool sdhci_check_capab_freq_range(SDHCIState *s, const char *desc,
+ uint8_t freq, Error **errp)
+{
+switch (freq) {
+case 0:
+case 10 ... 63:
+break;
+default:
+error_setg(errp, "SD %s clock frequency can have value"
+   "in range 0-63 only", desc);
+return true;
+}
+return false;
+}
+
+static void sdhci_check_capareg(SDHCIState *s, Error **errp)
+{
+uint64_t msk = s->capareg;
+uint32_t val;
+bool unit_mhz;
+
+switch (s->sd_spec_version) {
+case 2: /* default version */
+
+/* fallback */
+case 1:
+unit_mhz = FIELD_EX64(s->capareg, SDHC_CAPAB, TOUNIT);
+msk = FIELD_DP64(msk, SDHC_CAPAB, TOUNIT, 0);
+
+val = FIELD_EX64(s->capareg, SDHC_CAPAB, TOCLKFREQ);
+msk = FIELD_DP64(msk, SDHC_CAPAB, TOCLKFREQ, 0);
+trace_sdhci_capareg(unit_mhz ? "timeout (MHz)" : "timeout (KHz)", val);
+if (sdhci_check_capab_freq_range(s, "timeout", val, errp)) {
+return;
+}
+
+val = FIELD_EX64(s->capareg, SDHC_CAPAB, BASECLKFREQ);
+msk = FIELD_DP64(msk, SDHC_CAPAB, BASECLKFREQ, 0);
+trace_sdhci_capareg(unit_mhz ? "base (MHz)" : "Base (KHz)", val);
+if (sdhci_check_capab_freq_range(s, "base", val, errp)) {
+return;
+}
+
+val = FIELD_EX64(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH);
+if (val >= 0b11) {
+error_setg(errp, "block size can be 512, 1024 or 2048 only");
+return;
+}
+msk = FIELD_DP64(msk, SDHC_CAPAB, 

[Qemu-devel] [PATCH v13 21/30] sdhci: implement UHS-I voltage switch

[Philippe Mathieu-Daudé]

[based on a patch from Alistair Francis 
 from qemu/xilinx tag xilinx-v2015.2]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 include/hw/sd/sd.h| 16 
 include/hw/sd/sdhci.h |  1 +
 hw/sd/core.c  | 13 +
 hw/sd/sd.c| 13 +
 hw/sd/sdhci.c | 12 +++-
 hw/sd/trace-events|  1 +
 6 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 96caefe373..f086679493 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -55,6 +55,20 @@
 #define AKE_SEQ_ERROR  (1 << 3)
 #define OCR_CCS_BITN30
 
+typedef enum {
+SD_VOLTAGE_0_4V = 400,  /* currently not supported */
+SD_VOLTAGE_1_8V = 1800,
+SD_VOLTAGE_3_0V = 3000,
+SD_VOLTAGE_3_3V = 3300,
+} sd_voltage_mv_t;
+
+typedef enum  {
+UHS_NOT_SUPPORTED   = 0,
+UHS_I   = 1,
+UHS_II  = 2,/* currently not supported */
+UHS_III = 3,/* currently not supported */
+} sd_uhs_mode_t;
+
 typedef enum {
 sd_none = -1,
 sd_bc = 0, /* broadcast -- no response */
@@ -88,6 +102,7 @@ typedef struct {
 void (*write_data)(SDState *sd, uint8_t value);
 uint8_t (*read_data)(SDState *sd);
 bool (*data_ready)(SDState *sd);
+void (*set_voltage)(SDState *sd, uint16_t millivolts);
 void (*enable)(SDState *sd, bool enable);
 bool (*get_inserted)(SDState *sd);
 bool (*get_readonly)(SDState *sd);
@@ -134,6 +149,7 @@ void sd_enable(SDState *sd, bool enable);
 /* Functions to be used by qdevified callers (working via
  * an SDBus rather than directly with SDState)
  */
+void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts);
 int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response);
 void sdbus_write_data(SDBus *sd, uint8_t value);
 uint8_t sdbus_read_data(SDBus *sd);
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index fd606e9928..f321767c56 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -96,6 +96,7 @@ typedef struct SDHCIState {
 bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
 uint32_t quirks;
 uint8_t sd_spec_version;
+uint8_t uhs_mode;
 } SDHCIState;
 
 /*
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 498284f109..6d198ea775 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -41,6 +41,19 @@ static SDState *get_card(SDBus *sdbus)
 return SD_CARD(kid->child);
 }
 
+void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
+{
+SDState *card = get_card(sdbus);
+
+trace_sdbus_set_voltage(sdbus_name(sdbus), millivolts);
+if (card) {
+SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+assert(sc->set_voltage);
+sc->set_voltage(card, millivolts);
+}
+}
+
 int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
 {
 SDState *card = get_card(sdbus);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 73e405a04f..a8d7a522c0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -128,6 +128,18 @@ struct SDState {
 bool enable;
 };
 
+static void sd_set_voltage(SDState *sd, uint16_t millivolts)
+{
+switch (millivolts) {
+case 3001 ... 3600: /* SD_VOLTAGE_3_3V */
+case 2001 ... 3000: /* SD_VOLTAGE_3_0V */
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "SD card voltage not supported: %.3fV",
+  millivolts / 1000.f);
+}
+}
+
 static void sd_set_mode(SDState *sd)
 {
 switch (sd->state) {
@@ -1926,6 +1938,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
 dc->reset = sd_reset;
 dc->bus_type = TYPE_SD_BUS;
 
+sc->set_voltage = sd_set_voltage;
 sc->do_command = sd_do_command;
 sc->write_data = sd_write_data;
 sc->read_data = sd_read_data;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1dbcb99f52..4e717117e1 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1255,7 +1255,16 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 sdhci_update_irq(s);
 break;
 case SDHC_ACMD12ERRSTS:
-MASKED_WRITE(s->acmd12errsts, mask, value);
+MASKED_WRITE(s->acmd12errsts, mask, value & UINT16_MAX);
+if (s->uhs_mode >= UHS_I) {
+MASKED_WRITE(s->hostctl2, mask >> 16, value >> 16);
+
+if (FIELD_EX32(s->hostctl2, SDHC_HOSTCTL2, V18_ENA)) {
+sdbus_set_voltage(>sdbus, SD_VOLTAGE_1_8V);
+} else {
+sdbus_set_voltage(>sdbus, SD_VOLTAGE_3_3V);
+}
+}
 break;
 
 case SDHC_CAPAB:
@@ -1310,6 +1319,7 @@ static void sdhci_init_readonly_registers(SDHCIState *s, 
Error **errp)
 
 #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
 DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
+DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
 \
 /* Capabilities 

[Qemu-devel] [PATCH v13 20/30] sdbus: add trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/core.c   | 14 --
 hw/sd/trace-events |  5 +
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/sd/core.c b/hw/sd/core.c
index 295dc44ab7..498284f109 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -23,6 +23,12 @@
 #include "hw/qdev-core.h"
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
+#include "trace.h"
+
+static inline const char *sdbus_name(SDBus *sdbus)
+{
+return sdbus->qbus.name;
+}
 
 static SDState *get_card(SDBus *sdbus)
 {
@@ -39,6 +45,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t 
*response)
 {
 SDState *card = get_card(sdbus);
 
+trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg, req->crc);
 if (card) {
 SDCardClass *sc = SD_CARD_GET_CLASS(card);
 
@@ -52,6 +59,7 @@ void sdbus_write_data(SDBus *sdbus, uint8_t value)
 {
 SDState *card = get_card(sdbus);
 
+trace_sdbus_write(sdbus_name(sdbus), value);
 if (card) {
 SDCardClass *sc = SD_CARD_GET_CLASS(card);
 
@@ -62,14 +70,16 @@ void sdbus_write_data(SDBus *sdbus, uint8_t value)
 uint8_t sdbus_read_data(SDBus *sdbus)
 {
 SDState *card = get_card(sdbus);
+uint8_t value = 0;
 
 if (card) {
 SDCardClass *sc = SD_CARD_GET_CLASS(card);
 
-return sc->read_data(card);
+value = sc->read_data(card);
 }
+trace_sdbus_read(sdbus_name(sdbus), value);
 
-return 0;
+return value;
 }
 
 bool sdbus_data_ready(SDBus *sdbus)
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 78d8707669..ea2746c8b7 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -1,5 +1,10 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# hw/sd/core.c
+sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg, uint8_t crc) 
"@%s CMD%02d arg 0x%08x crc 0x%02x"
+sdbus_read(const char *bus_name, uint8_t value) "@%s value 0x%02x"
+sdbus_write(const char *bus_name, uint8_t value) "@%s value 0x%02x"
+
 # hw/sd/sdhci.c
 sdhci_set_inserted(const char *level) "card state changed: %s"
 sdhci_send_command(uint8_t cmd, uint32_t arg) "CMD%02u ARG[0x%08x]"
-- 
2.16.1

[Qemu-devel] [PATCH v13 04/30] sdhci: add a check_capab_baseclock() qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 tests/sdhci-test.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 51af5eac67..d6eb3c3a48 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -14,6 +14,7 @@
 #include "hw/pci/pci.h"
 
 #define SDHC_CAPAB  0x40
+FIELD(SDHC_CAPAB, BASECLKFREQ,   8, 8); /* since v2 */
 #define SDHC_HCVER  0xFE
 
 static const struct sdhci_t {
@@ -98,6 +99,18 @@ static void check_capab_readonly(QSDHCI *s)
 g_assert_cmpuint(capab1, ==, capab0);
 }
 
+static void check_capab_baseclock(QSDHCI *s, uint8_t expected_freq)
+{
+uint64_t capab, capab_freq;
+
+if (!expected_freq) {
+return;
+}
+capab = sdhci_readq(s, SDHC_CAPAB);
+capab_freq = FIELD_EX64(capab, SDHC_CAPAB, BASECLKFREQ);
+g_assert_cmpuint(capab_freq, ==, expected_freq);
+}
+
 static QSDHCI *machine_start(const struct sdhci_t *test)
 {
 QSDHCI *s = g_new0(QSDHCI, 1);
@@ -145,6 +158,7 @@ static void test_machine(const void *data)
 
 check_capab_capareg(s, test->sdhci.capab.reg);
 check_capab_readonly(s);
+check_capab_baseclock(s, test->sdhci.baseclock);
 
 machine_stop(s);
 }
-- 
2.16.1

[Qemu-devel] [PATCH v13 08/30] sdhci: use a numeric value for the default CAPAB register

[Philippe Mathieu-Daudé]

using many #defines is not portable when scaling to different HCI.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci.c | 74 +--
 1 file changed, 16 insertions(+), 58 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 17a1348f0f..491e624262 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -38,67 +38,25 @@
 #define TYPE_SDHCI_BUS "sdhci-bus"
 #define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS)
 
+#define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
+
 /* Default SD/MMC host controller features information, which will be
  * presented in CAPABILITIES register of generic SD host controller at reset.
- * If not stated otherwise:
- * 0 - not supported, 1 - supported, other - prohibited.
+ *
+ * support:
+ * - 3.3v and 1.8v voltages
+ * - SDMA/ADMA1/ADMA2
+ * - high-speed
+ * max host controller R/W buffers size: 512B
+ * max clock frequency for SDclock: 52 MHz
+ * timeout clock frequency: 52 MHz
+ *
+ * does not support:
+ * - 3.0v voltage
+ * - 64-bit system bus
+ * - suspend/resume
  */
-#define SDHC_CAPAB_64BITBUS   0ul/* 64-bit System Bus Support */
-#define SDHC_CAPAB_18V1ul/* Voltage support 1.8v */
-#define SDHC_CAPAB_30V0ul/* Voltage support 3.0v */
-#define SDHC_CAPAB_33V1ul/* Voltage support 3.3v */
-#define SDHC_CAPAB_SUSPRESUME 0ul/* Suspend/resume support */
-#define SDHC_CAPAB_SDMA   1ul/* SDMA support */
-#define SDHC_CAPAB_HIGHSPEED  1ul/* High speed support */
-#define SDHC_CAPAB_ADMA1  1ul/* ADMA1 support */
-#define SDHC_CAPAB_ADMA2  1ul/* ADMA2 support */
-/* Maximum host controller R/W buffers size
- * Possible values: 512, 1024, 2048 bytes */
-#define SDHC_CAPAB_MAXBLOCKLENGTH 512ul
-/* Maximum clock frequency for SDclock in MHz
- * value in range 10-63 MHz, 0 - not defined */
-#define SDHC_CAPAB_BASECLKFREQ52ul
-#define SDHC_CAPAB_TOUNIT 1ul  /* Timeout clock unit 0 - kHz, 1 - MHz 
*/
-/* Timeout clock frequency 1-63, 0 - not defined */
-#define SDHC_CAPAB_TOCLKFREQ  52ul
-
-/* Now check all parameters and calculate CAPABILITIES REGISTER value */
-#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_18V > 1 || SDHC_CAPAB_30V > 1 || 
\
-SDHC_CAPAB_33V > 1 || SDHC_CAPAB_SUSPRESUME > 1 || SDHC_CAPAB_SDMA > 1 ||  
\
-SDHC_CAPAB_HIGHSPEED > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 
||\
-SDHC_CAPAB_TOUNIT > 1
-#error Capabilities features can have value 0 or 1 only!
-#endif
-
-#if SDHC_CAPAB_MAXBLOCKLENGTH == 512
-#define MAX_BLOCK_LENGTH 0ul
-#elif SDHC_CAPAB_MAXBLOCKLENGTH == 1024
-#define MAX_BLOCK_LENGTH 1ul
-#elif SDHC_CAPAB_MAXBLOCKLENGTH == 2048
-#define MAX_BLOCK_LENGTH 2ul
-#else
-#error Max host controller block size can have value 512, 1024 or 2048 only!
-#endif
-
-#if (SDHC_CAPAB_BASECLKFREQ > 0 && SDHC_CAPAB_BASECLKFREQ < 10) || \
-SDHC_CAPAB_BASECLKFREQ > 63
-#error SDclock frequency can have value in range 0, 10-63 only!
-#endif
-
-#if SDHC_CAPAB_TOCLKFREQ > 63
-#error Timeout clock frequency can have value in range 0-63 only!
-#endif
-
-#define SDHC_CAPAB_REG_DEFAULT \
-   ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_18V << 26) | \
-(SDHC_CAPAB_30V << 25) | (SDHC_CAPAB_33V << 24) |  \
-(SDHC_CAPAB_SUSPRESUME << 23) | (SDHC_CAPAB_SDMA << 22) |  \
-(SDHC_CAPAB_HIGHSPEED << 21) | (SDHC_CAPAB_ADMA1 << 20) |  \
-(SDHC_CAPAB_ADMA2 << 19) | (MAX_BLOCK_LENGTH << 16) |  \
-(SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \
-(SDHC_CAPAB_TOCLKFREQ))
-
-#define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
+#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
 
 static uint8_t sdhci_slotint(SDHCIState *s)
 {
-- 
2.16.1

[Qemu-devel] [PATCH v13 12/30] sdhci: Fix 64-bit ADMA2

[Philippe Mathieu-Daudé]

From: Sai Pavan Boddu 

The 64-bit ADMA address is not converted to the cpu endianes correctly.
This patch fixes the issue and uses a valid mask for the attribute data.

Signed-off-by: Sai Pavan Boddu 
[AF: Re-write commit message]
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 650265a472..4bd35078a9 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -667,8 +667,8 @@ static void get_adma_description(SDHCIState *s, ADMADescr 
*dscr)
 dscr->length = le16_to_cpu(dscr->length);
 dma_memory_read(s->dma_as, entry_addr + 4,
 (uint8_t *)(>addr), 8);
-dscr->attr = le64_to_cpu(dscr->attr);
-dscr->attr &= 0xfff8;
+dscr->addr = le64_to_cpu(dscr->addr);
+dscr->attr &= (uint8_t) ~0xC0;
 dscr->incr = 12;
 break;
 }
-- 
2.16.1

[Qemu-devel] [PATCH v13 07/30] sdhci: add a 'spec_version property' (default to v2)

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci-internal.h |  4 ++--
 include/hw/sd/sdhci.h  |  2 ++
 hw/sd/sdhci.c  | 27 +++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 0991acd724..64556480a9 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -216,9 +216,9 @@
 /* Slot interrupt status */
 #define SDHC_SLOT_INT_STATUS0xFC
 
-/* HWInit Host Controller Version Register 0x0401 */
+/* HWInit Host Controller Version Register */
 #define SDHC_HCVER  0xFE
-#define SD_HOST_SPECv2_VERS 0x2401
+#define SDHC_HCVER_VENDOR   0x24
 
 #define SDHC_REGISTERS_MAP_SIZE 0x100
 #define SDHC_INSERTION_DELAY(NANOSECONDS_PER_SECOND)
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index f8d1ba3538..2a26b46f05 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -78,6 +78,7 @@ typedef struct SDHCIState {
 /* Read-only registers */
 uint64_t capareg;  /* Capabilities Register */
 uint64_t maxcurr;  /* Maximum Current Capabilities Register */
+uint16_t version;  /* Host Controller Version Register */
 
 uint8_t  *fifo_buffer; /* SD host i/o FIFO buffer */
 uint32_t buf_maxsz;
@@ -93,6 +94,7 @@ typedef struct SDHCIState {
 /* Configurable properties */
 bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
 uint32_t quirks;
+uint8_t sd_spec_version;
 } SDHCIState;
 
 /*
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 3602286f46..17a1348f0f 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -173,7 +173,8 @@ static void sdhci_reset(SDHCIState *s)
 
 timer_del(s->insert_timer);
 timer_del(s->transfer_timer);
-/* Set all registers to 0. Capabilities registers are not cleared
+
+/* Set all registers to 0. Capabilities/Version registers are not cleared
  * and assumed to always preserve their value, given to them during
  * initialization */
 memset(>sdmasysad, 0, (uintptr_t)>capareg - 
(uintptr_t)>sdmasysad);
@@ -918,7 +919,7 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, 
unsigned size)
 ret = (uint32_t)(s->admasysaddr >> 32);
 break;
 case SDHC_SLOT_INT_STATUS:
-ret = (SD_HOST_SPECv2_VERS << 16) | sdhci_slotint(s);
+ret = (s->version << 16) | sdhci_slotint(s);
 break;
 default:
 qemu_log_mask(LOG_UNIMP, "SDHC rd_%ub @0x%02" HWADDR_PRIx " "
@@ -1174,11 +1175,22 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState 
*s)
 }
 }
 
+static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
+{
+if (s->sd_spec_version != 2) {
+error_setg(errp, "Only Spec v2 is supported");
+return;
+}
+s->version = (SDHC_HCVER_VENDOR << 8) | (s->sd_spec_version - 1);
+}
+
 /* --- qdev common --- */
 
 #define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
-/* Capabilities registers provide information on supported features
- * of this specific host controller implementation */ \
+DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
+\
+/* Capabilities registers provide information on supported
+ * features of this specific host controller implementation */ \
 DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
 DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
 
@@ -1206,6 +1218,13 @@ static void sdhci_uninitfn(SDHCIState *s)
 
 static void sdhci_common_realize(SDHCIState *s, Error **errp)
 {
+Error *local_err = NULL;
+
+sdhci_init_readonly_registers(s, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 s->buf_maxsz = sdhci_get_fifolen(s);
 s->fifo_buffer = g_malloc0(s->buf_maxsz);
 
-- 
2.16.1

[Qemu-devel] [PATCH v13 09/30] sdhci: simplify sdhci_get_fifolen()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci-internal.h |  4 +++-
 hw/sd/sdhci.c  | 20 +---
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 64556480a9..def1c7f7aa 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -24,6 +24,8 @@
 #ifndef SDHCI_INTERNAL_H
 #define SDHCI_INTERNAL_H
 
+#include "hw/registerfields.h"
+
 /* R/W SDMA System Address register 0x0 */
 #define SDHC_SYSAD 0x00
 
@@ -185,7 +187,7 @@
 #define SDHC_CAN_DO_ADMA2  0x0008
 #define SDHC_CAN_DO_ADMA1  0x0010
 #define SDHC_64_BIT_BUS_SUPPORT(1 << 28)
-#define SDHC_CAPAB_BLOCKSIZE(x)(((x) >> 16) & 0x3)
+FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
 
 /* HWInit Maximum Current Capabilities Register 0x0 */
 #define SDHC_MAXCURR   0x48
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 491e624262..f22ed9181c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -58,6 +58,11 @@
  */
 #define SDHC_CAPAB_REG_DEFAULT 0x057834b4
 
+static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
+{
+return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
+}
+
 static uint8_t sdhci_slotint(SDHCIState *s)
 {
 return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) 
||
@@ -1118,21 +1123,6 @@ static const MemoryRegionOps sdhci_mmio_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
-{
-switch (SDHC_CAPAB_BLOCKSIZE(s->capareg)) {
-case 0:
-return 512;
-case 1:
-return 1024;
-case 2:
-return 2048;
-default:
-hw_error("SDHC: unsupported value for maximum block size\n");
-return 0;
-}
-}
-
 static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
 {
 if (s->sd_spec_version != 2) {
-- 
2.16.1

[Qemu-devel] [PATCH v13 06/30] sdhci: add qtest to check the SD Spec version

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 tests/sdhci-test.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 7c50c0482b..24feea744a 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -54,6 +54,19 @@ typedef struct QSDHCI {
 };
 } QSDHCI;
 
+static uint32_t sdhci_readl(QSDHCI *s, uint32_t reg)
+{
+uint32_t val;
+
+if (s->pci.dev) {
+qpci_memread(s->pci.dev, s->mem_bar, reg, , sizeof(val));
+} else {
+val = qtest_readl(global_qtest, s->addr + reg);
+}
+
+return val;
+}
+
 static uint64_t sdhci_readq(QSDHCI *s, uint32_t reg)
 {
 uint64_t val;
@@ -78,6 +91,16 @@ static void sdhci_writeq(QSDHCI *s, uint32_t reg, uint64_t 
val)
 
 /* tests */
 
+static void check_specs_version(QSDHCI *s, uint8_t version)
+{
+uint32_t v;
+
+v = sdhci_readl(s, SDHC_HCVER);
+v &= 0xff;
+v += 1;
+g_assert_cmpuint(v, ==, version);
+}
+
 static void check_capab_capareg(QSDHCI *s, uint64_t expected_capab)
 {
 uint64_t capab;
@@ -166,6 +189,7 @@ static void test_machine(const void *data)
 
 s = machine_start(test);
 
+check_specs_version(s, test->sdhci.version);
 check_capab_capareg(s, test->sdhci.capab.reg);
 check_capab_readonly(s);
 check_capab_sdma(s, test->sdhci.capab.sdma);
-- 
2.16.1

[Qemu-devel] [PATCH v13 03/30] sdhci: add check_capab_readonly() qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 tests/sdhci-test.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 82f3785a72..51af5eac67 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -65,6 +65,15 @@ static uint64_t sdhci_readq(QSDHCI *s, uint32_t reg)
 return val;
 }
 
+static void sdhci_writeq(QSDHCI *s, uint32_t reg, uint64_t val)
+{
+if (s->pci.dev) {
+qpci_memwrite(s->pci.dev, s->mem_bar, reg, , sizeof(val));
+} else {
+qtest_writeq(global_qtest, s->addr + reg, val);
+}
+}
+
 /* tests */
 
 static void check_capab_capareg(QSDHCI *s, uint64_t expected_capab)
@@ -75,6 +84,20 @@ static void check_capab_capareg(QSDHCI *s, uint64_t 
expected_capab)
 g_assert_cmphex(capab, ==, expected_capab);
 }
 
+static void check_capab_readonly(QSDHCI *s)
+{
+const uint64_t vrand = 0x123456789abcdef;
+uint64_t capab0, capab1;
+
+capab0 = sdhci_readq(s, SDHC_CAPAB);
+g_assert_cmpuint(capab0, !=, vrand);
+
+sdhci_writeq(s, SDHC_CAPAB, vrand);
+capab1 = sdhci_readq(s, SDHC_CAPAB);
+g_assert_cmpuint(capab1, !=, vrand);
+g_assert_cmpuint(capab1, ==, capab0);
+}
+
 static QSDHCI *machine_start(const struct sdhci_t *test)
 {
 QSDHCI *s = g_new0(QSDHCI, 1);
@@ -121,6 +144,7 @@ static void test_machine(const void *data)
 s = machine_start(test);
 
 check_capab_capareg(s, test->sdhci.capab.reg);
+check_capab_readonly(s);
 
 machine_stop(s);
 }
-- 
2.16.1

[Qemu-devel] [PATCH v13 05/30] sdhci: add a check_capab_sdma() qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 tests/sdhci-test.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index d6eb3c3a48..7c50c0482b 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -15,6 +15,7 @@
 
 #define SDHC_CAPAB  0x40
 FIELD(SDHC_CAPAB, BASECLKFREQ,   8, 8); /* since v2 */
+FIELD(SDHC_CAPAB, SDMA, 22, 1);
 #define SDHC_HCVER  0xFE
 
 static const struct sdhci_t {
@@ -111,6 +112,15 @@ static void check_capab_baseclock(QSDHCI *s, uint8_t 
expected_freq)
 g_assert_cmpuint(capab_freq, ==, expected_freq);
 }
 
+static void check_capab_sdma(QSDHCI *s, bool supported)
+{
+uint64_t capab, capab_sdma;
+
+capab = sdhci_readq(s, SDHC_CAPAB);
+capab_sdma = FIELD_EX64(capab, SDHC_CAPAB, SDMA);
+g_assert_cmpuint(capab_sdma, ==, supported);
+}
+
 static QSDHCI *machine_start(const struct sdhci_t *test)
 {
 QSDHCI *s = g_new0(QSDHCI, 1);
@@ -158,6 +168,7 @@ static void test_machine(const void *data)
 
 check_capab_capareg(s, test->sdhci.capab.reg);
 check_capab_readonly(s);
+check_capab_sdma(s, test->sdhci.capab.sdma);
 check_capab_baseclock(s, test->sdhci.baseclock);
 
 machine_stop(s);
-- 
2.16.1

[Qemu-devel] [PATCH v13 02/30] sdhci: add qtest to check the SD capabilities register

[Philippe Mathieu-Daudé]

The PCI model is tested with the pc/x86_64 machine,
the SysBus model with the smdkc210/arm machine.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Paolo Bonzini 
---
 tests/sdhci-test.c | 145 +
 tests/Makefile.include |   3 +
 2 files changed, 148 insertions(+)
 create mode 100644 tests/sdhci-test.c

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
new file mode 100644
index 00..82f3785a72
--- /dev/null
+++ b/tests/sdhci-test.c
@@ -0,0 +1,145 @@
+/*
+ * QTest testcase for SDHCI controllers
+ *
+ * Written by Philippe Mathieu-Daudé 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "hw/registerfields.h"
+#include "libqtest.h"
+#include "libqos/pci-pc.h"
+#include "hw/pci/pci.h"
+
+#define SDHC_CAPAB  0x40
+#define SDHC_HCVER  0xFE
+
+static const struct sdhci_t {
+const char *arch, *machine;
+struct {
+uintptr_t addr;
+uint8_t version;
+uint8_t baseclock;
+struct {
+bool sdma;
+uint64_t reg;
+} capab;
+} sdhci;
+struct {
+uint16_t vendor_id, device_id;
+} pci;
+} models[] = {
+/* PC via PCI */
+{ "x86_64", "pc",
+{-1, 2, 0,  {1, 0x057834b4} },
+.pci = { PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_SDHCI } },
+
+/* Exynos4210 */
+{ "arm","smdkc210",
+{0x1251, 2, 0,  {1, 0x5e80080} } },
+};
+
+typedef struct QSDHCI {
+struct {
+QPCIBus *bus;
+QPCIDevice *dev;
+} pci;
+union {
+QPCIBar mem_bar;
+uint64_t addr;
+};
+} QSDHCI;
+
+static uint64_t sdhci_readq(QSDHCI *s, uint32_t reg)
+{
+uint64_t val;
+
+if (s->pci.dev) {
+qpci_memread(s->pci.dev, s->mem_bar, reg, , sizeof(val));
+} else {
+val = qtest_readq(global_qtest, s->addr + reg);
+}
+
+return val;
+}
+
+/* tests */
+
+static void check_capab_capareg(QSDHCI *s, uint64_t expected_capab)
+{
+uint64_t capab;
+
+capab = sdhci_readq(s, SDHC_CAPAB);
+g_assert_cmphex(capab, ==, expected_capab);
+}
+
+static QSDHCI *machine_start(const struct sdhci_t *test)
+{
+QSDHCI *s = g_new0(QSDHCI, 1);
+
+if (test->pci.vendor_id) {
+/* PCI */
+uint16_t vendor_id, device_id;
+uint64_t barsize;
+
+global_qtest = qtest_startf("-machine %s -device sdhci-pci",
+test->machine);
+
+s->pci.bus = qpci_init_pc(NULL);
+
+/* Find PCI device and verify it's the right one */
+s->pci.dev = qpci_device_find(s->pci.bus, QPCI_DEVFN(4, 0));
+g_assert_nonnull(s->pci.dev);
+vendor_id = qpci_config_readw(s->pci.dev, PCI_VENDOR_ID);
+device_id = qpci_config_readw(s->pci.dev, PCI_DEVICE_ID);
+g_assert(vendor_id == test->pci.vendor_id);
+g_assert(device_id == test->pci.device_id);
+s->mem_bar = qpci_iomap(s->pci.dev, 0, );
+qpci_device_enable(s->pci.dev);
+} else {
+/* SysBus */
+global_qtest = qtest_startf("-machine %s", test->machine);
+s->addr = test->sdhci.addr;
+}
+
+return s;
+}
+
+static void machine_stop(QSDHCI *s)
+{
+g_free(s->pci.dev);
+qtest_quit(global_qtest);
+}
+
+static void test_machine(const void *data)
+{
+const struct sdhci_t *test = data;
+QSDHCI *s;
+
+s = machine_start(test);
+
+check_capab_capareg(s, test->sdhci.capab.reg);
+
+machine_stop(s);
+}
+
+int main(int argc, char *argv[])
+{
+const char *arch = qtest_get_arch();
+char *name;
+int i;
+
+g_test_init(, , NULL);
+for (i = 0; i < ARRAY_SIZE(models); i++) {
+if (strcmp(arch, models[i].arch)) {
+continue;
+}
+name = g_strdup_printf("sdhci/%s", models[i].machine);
+qtest_add_data_func(name, [i], test_machine);
+g_free(name);
+}
+
+return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f41da235ae..52be9b3fa5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -294,6 +294,7 @@ check-qtest-i386-y += tests/migration-test$(EXESUF)
 check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
 check-qtest-i386-y += tests/numa-test$(EXESUF)
 check-qtest-x86_64-y += $(check-qtest-i386-y)
+check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
 
@@ -367,6 +368,7 @@ gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
 check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 gcov-files-arm-y += hw/timer/arm_mptimer.c
 check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
+check-qtest-arm-y += 

[Qemu-devel] [PATCH v13 00/30] SDHCI: clean v1/2 Specs, UHS-I cards tuning sequence

[Philippe Mathieu-Daudé]

This series includes the last versions of both series:
- SDHCI: clean v1/v2 Specs (part 2)
- SDHCI: add tuning sequence for UHS-I cards (part 3)

Since v12:
- rebased...
- bcc'ing patchew, spaming others

Since v11:
- rebased due to conflict (IMX_USDHC fd1e5c817964)
- QSDHCI uses union (Paolo)
- do not enable UNIMP logging when running qtests

Since v10:
- rebased
- add Paolo's R-b in patch 2
- rename patch 11 subject (Alistair)
- add Alistair's R-b in "UHS-I cards tuning sequence" patches

Thanks Paolo :)

Phil.

$ git backport-diff with v11
001/30:[] [--] 'sdhci: use error_propagate(local_err) in realize()'
002/30:[0028] [FC] 'sdhci: add qtest to check the SD capabilities register'
003/30:[0016] [FC] 'sdhci: add check_capab_readonly() qtest'
004/30:[0010] [FC] 'sdhci: add a check_capab_baseclock() qtest'
005/30:[0006] [FC] 'sdhci: add a check_capab_sdma() qtest'
006/30:[0012] [FC] 'sdhci: add qtest to check the SD Spec version'
007/30:[] [-C] 'sdhci: add a 'spec_version property' (default to v2)'
008/30:[] [--] 'sdhci: use a numeric value for the default CAPAB register'
009/30:[] [--] 'sdhci: simplify sdhci_get_fifolen()'
010/30:[0029] [FC] 'sdhci: check the Spec v1 capabilities correctness'
011/30:[] [--] 'sdhci: replace DMA magic value by BLOCK_SIZE_MASK'
012/30:[] [--] 'sdhci: Fix 64-bit ADMA2'
013/30:[0006] [FC] 'sdhci: check Spec v2 capabilities (DMA and 64-bit bus)'
014/30:[] [--] 'hw/arm/exynos4210: access the 64-bit capareg with 
qdev_prop_set_uint64()'
015/30:[] [--] 'hw/arm/exynos4210: add a comment about a very similar SDHCI 
(Spec. v2)'
016/30:[] [--] 'hw/arm/xilinx_zynq: fix the capabilities register to match 
the datasheet'
017/30:[] [-C] 'sdhci: add support for v3 capabilities'
018/30:[0006] [FC] 'sdhci: rename the hostctl1 register'
019/30:[] [--] 'sdhci: implement the Host Control 2 register (tuning 
sequence)'
020/30:[] [--] 'sdbus: add trace events'
021/30:[] [-C] 'sdhci: implement UHS-I voltage switch'
022/30:[] [--] 'sdhci: implement CMD/DAT[] fields in the Present State 
register'
023/30:[] [--] 'hw/arm/bcm2835_peripherals: implement SDHCI Spec v3'
024/30:[] [--] 'hw/arm/bcm2835_peripherals: change maximum block size to 
1kB'
025/30:[] [--] 'hw/arm/fsl-imx6: implement SDHCI Spec. v3'
026/30:[] [--] 'hw/arm/xilinx_zynqmp: fix the capabilities/spec version to 
match the datasheet'
027/30:[] [--] 'hw/arm/xilinx_zynqmp: enable the UHS-I mode'
028/30:[] [--] 'sdhci: check Spec v3 capabilities qtest'
029/30:[0006] [FC] 'sdhci: add a check_capab_v3() qtest'
030/30:[0008] [FC] 'sdhci: add Spec v4.2 register definitions'

Philippe Mathieu-Daudé (29):
  sdhci: use error_propagate(local_err) in realize()
  sdhci: add qtest to check the SD capabilities register
  sdhci: add check_capab_readonly() qtest
  sdhci: add a check_capab_baseclock() qtest
  sdhci: add a check_capab_sdma() qtest
  sdhci: add qtest to check the SD Spec version
  sdhci: add a 'spec_version property' (default to v2)
  sdhci: use a numeric value for the default CAPAB register
  sdhci: simplify sdhci_get_fifolen()
  sdhci: check the Spec v1 capabilities correctness
  sdhci: replace DMA magic value by BLOCK_SIZE_MASK
  sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
  hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
  hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
  hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
  sdhci: add support for v3 capabilities
  sdhci: rename the hostctl1 register
  sdhci: implement the Host Control 2 register (tuning sequence)
  sdbus: add trace events
  sdhci: implement UHS-I voltage switch
  sdhci: implement CMD/DAT[] fields in the Present State register
  hw/arm/bcm2835_peripherals: implement SDHCI Spec v3
  hw/arm/bcm2835_peripherals: change maximum block size to 1kB
  hw/arm/fsl-imx6: implement SDHCI Spec. v3
  hw/arm/xilinx_zynqmp: fix the capabilities/spec version to match the datasheet
  hw/arm/xilinx_zynqmp: enable the UHS-I mode
  sdhci: check Spec v3 capabilities qtest
  sdhci: add a check_capab_v3() qtest
  sdhci: add Spec v4.2 register definitions

Sai Pavan Boddu (1):
  sdhci: Fix 64-bit ADMA2

 include/hw/sd/sd.h   |  20 +++
 include/hw/sd/sdhci.h|   6 +-
 hw/sd/sdhci-internal.h   |  78 +++--
 hw/arm/bcm2835_peripherals.c |  23 +--
 hw/arm/exynos4210.c  |  14 +-
 hw/arm/fsl-imx6.c|   7 +
 hw/arm/xilinx_zynq.c |  53 +++---
 hw/arm/xlnx-zynqmp.c |  30 ++--
 hw/sd/core.c |  61 ++-
 hw/sd/sd.c   |  29 
 hw/sd/sdhci.c| 375 +++
 hw/sd/trace-events   |   9 ++
 tests/sdhci-test.c   | 252 +
 tests/Makefile.include   |   4 +
 14 files changed, 800 insertions(+), 161 deletions(-)
 create mode 100644 tests/sdhci-test.c

-- 
2.16.1

[Qemu-devel] [PATCH v13 01/30] sdhci: use error_propagate(local_err) in realize()

[Philippe Mathieu-Daudé]

avoid the "errp && *errp" pattern (not recommended in "qapi/error.h" comments).

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sdhci.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ee95e78aeb..3602286f46 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1302,10 +1302,12 @@ static Property sdhci_pci_properties[] = {
 static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
 {
 SDHCIState *s = PCI_SDHCI(dev);
+Error *local_err = NULL;
 
 sdhci_initfn(s);
 sdhci_common_realize(s, errp);
-if (errp && *errp) {
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
@@ -1383,9 +1385,11 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error 
** errp)
 {
 SDHCIState *s = SYSBUS_SDHCI(dev);
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+Error *local_err = NULL;
 
 sdhci_common_realize(s, errp);
-if (errp && *errp) {
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
-- 
2.16.1

Re: [Qemu-devel] [PATCH 0/2] migration: Fix early failure crash

[Peter Xu]

On Mon, Feb 12, 2018 at 04:03:38PM +, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> This fixes a crash for the case where a migration exits with an error
> very early, this is probably due to my recent error handling change.
> 
> I also add a test to make sure this doesn't fail again, the test does
> output one line of junk, suggestions for how to clean it up are welcome:
> 
> [dgilbert@dgilbert-t530 try]$ tests/migration-test 
> /x86_64/migration/deprecated: OK
> /x86_64/migration/bad_dest: qemu-system-x86_64: Failed to connect socket: 
> Connection refused
> OK
> /x86_64/migration/postcopy/unix: OK

So we have more than one way to log things (error_report routes things
directly to stderr while we also have the qemu log stuff).

A stupid but fast way I can think of is just don't dump this in
migrate_fd_cleanup, since after all it's only for HMP and people
should also see that when query migration status.  But it'll be a bit
inconvenient for HMP users encountering failures.

Or maybe we can hack around fd 2 specifically in that test?  It's at
least ugly though...

Anyway, the patches look good to me.

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH] vhost-user: fix memory leak

[Philippe Mathieu-Daudé]

Hi Linzhecheng,

On 02/12/2018 11:53 PM, linzhecheng wrote:
> fix memory leak
> 
> Signed-off-by: linzhecheng 
> 
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index cb45512506..d024573e45 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -109,6 +109,7 @@ static int vhost_user_start(int queues, NetClientState 
> *ncs[], CharBackend *be)
>  err:
>  if (net) {
>  vhost_net_cleanup(net);
> +g_free(net);

I think this g_free() belongs to vhost_net_cleanup() in net/vhost_net.c:

void vhost_net_cleanup(struct vhost_net *net)
{
vhost_dev_cleanup(>dev);
g_free(net);
}

Regards,

Phil.

>  }
>  vhost_user_stop(i, ncs);
>  return -1;
> 

Re: [Qemu-devel] [RESEND PATCH] PPC: e500: Fix duplicate kernel load and device tree overlap

[David Gibson]

On Fri, Feb 09, 2018 at 08:49:45AM +0100, David Engraf wrote:
> Hello David,
> 
> Am 09.02.2018 um 06:33 schrieb David Gibson:
> > On Thu, Feb 08, 2018 at 09:36:21AM +0100, David Engraf wrote:
> > > This patch fixes an incorrect behavior when the -kernel argument has been
> > > specified without -bios. In this case the kernel was loaded twice. At 
> > > address
> > > 32M as a raw image and afterwards by load_elf/load_uimage at the
> > > corresponding load address. In this case the region for the device tree 
> > > and
> > > the raw kernel image may overlap.
> > > 
> > > The patch fixes the behavior by loading the kernel image once with
> > > load_elf/load_uimage and skips loading the raw image. It also ensures that
> > > the device tree is generated behind bios/kernel/initrd.
> > > 
> > > Signed-off-by: David Engraf 
> > 
> > Sorry I've taken so long to respond to this.  I've been busy, then
> > away, then busy, then recovering from surgery, then...
> > 
> > I think this looks good overall, just a couple of details I'd like to
> > check, see below.
> > 
> > > ---
> > >   hw/ppc/e500.c | 89 
> > > ---
> > >   1 file changed, 48 insertions(+), 41 deletions(-)
> > > 
> > > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> > > index c4fe06ea2a..0321bd66a8 100644
> > > --- a/hw/ppc/e500.c
> > > +++ b/hw/ppc/e500.c
> > > @@ -776,7 +776,6 @@ void ppce500_init(MachineState *machine, 
> > > PPCE500Params *params)
> > >   MemoryRegion *ram = g_new(MemoryRegion, 1);
> > >   PCIBus *pci_bus;
> > >   CPUPPCState *env = NULL;
> > > -uint64_t loadaddr;
> > >   hwaddr kernel_base = -1LL;
> > >   int kernel_size = 0;
> > >   hwaddr dt_base = 0;
> > > @@ -913,11 +912,6 @@ void ppce500_init(MachineState *machine, 
> > > PPCE500Params *params)
> > >   /* Register spinning region */
> > >   sysbus_create_simple("e500-spin", params->spin_base, NULL);
> > > -if (cur_base < (32 * 1024 * 1024)) {
> > > -/* u-boot occupies memory up to 32MB, so load blobs above */
> > > -cur_base = (32 * 1024 * 1024);
> > > -}
> > > -
> > >   if (params->has_mpc8xxx_gpio) {
> > >   qemu_irq poweroff_irq;
> > > @@ -952,36 +946,6 @@ void ppce500_init(MachineState *machine, 
> > > PPCE500Params *params)
> > >   sysbus_mmio_get_region(s, 0));
> > >   }
> > > -/* Load kernel. */
> > > -if (machine->kernel_filename) {
> > > -kernel_base = cur_base;
> > > -kernel_size = load_image_targphys(machine->kernel_filename,
> > > -  cur_base,
> > > -  ram_size - cur_base);
> > > -if (kernel_size < 0) {
> > > -fprintf(stderr, "qemu: could not load kernel '%s'\n",
> > > -machine->kernel_filename);
> > > -exit(1);
> > > -}
> > > -
> > > -cur_base += kernel_size;
> > > -}
> > > -
> > > -/* Load initrd. */
> > > -if (machine->initrd_filename) {
> > > -initrd_base = (cur_base + INITRD_LOAD_PAD) & ~INITRD_PAD_MASK;
> > > -initrd_size = load_image_targphys(machine->initrd_filename, 
> > > initrd_base,
> > > -  ram_size - initrd_base);
> > > -
> > > -if (initrd_size < 0) {
> > > -fprintf(stderr, "qemu: could not load initial ram disk 
> > > '%s'\n",
> > > -machine->initrd_filename);
> > > -exit(1);
> > > -}
> > > -
> > > -cur_base = initrd_base + initrd_size;
> > > -}
> > > -
> > >   /*
> > >* Smart firmware defaults ahead!
> > >*
> > > @@ -1006,24 +970,67 @@ void ppce500_init(MachineState *machine, 
> > > PPCE500Params *params)
> > >   }
> > >   filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> > > -bios_size = load_elf(filename, NULL, NULL, _entry, , 
> > > NULL,
> > > +bios_size = load_elf(filename, NULL, NULL, _entry, _base, 
> > > NULL,
> > >1, PPC_ELF_MACHINE, 0, 0);
> > >   if (bios_size < 0) {
> > >   /*
> > >* Hrm. No ELF image? Try a uImage, maybe someone is giving us 
> > > an
> > >* ePAPR compliant kernel
> > >*/
> > > -kernel_size = load_uimage(filename, _entry, , NULL,
> > > -  NULL, NULL);
> > > -if (kernel_size < 0) {
> > > +bios_size = load_uimage(filename, _entry, _base, NULL,
> > > +NULL, NULL);
> > > +if (bios_size < 0) {
> > >   fprintf(stderr, "qemu: could not load firmware '%s'\n", 
> > > filename);
> > >   exit(1);
> > >   }
> > >   }
> > > +cur_base += bios_size;
> > >   g_free(filename);
> > > +/* Load bare kernel only if no bios/u-boot has been provided */
> > > +if 

[Qemu-devel] [PATCH] vhost-user: fix memory leak

[linzhecheng]

fix memory leak

Signed-off-by: linzhecheng 

diff --git a/net/vhost-user.c b/net/vhost-user.c
index cb45512506..d024573e45 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -109,6 +109,7 @@ static int vhost_user_start(int queues, NetClientState 
*ncs[], CharBackend *be)
 err:
 if (net) {
 vhost_net_cleanup(net);
+g_free(net);
 }
 vhost_user_stop(i, ncs);
 return -1;
-- 
2.12.2.windows.2

[Qemu-devel] gdbstub.c gdb-get-register returns 0, breaking remote gdb call

[Dan Kaminsky]

Unsure the scope of this bug or the proper fix, so bringing it up here
first.

gdbstub.c:gdb_read_register will return 0, and thus E14, when a remote gdb
tries to call a function in the exposed linux kernel.  This appears to be
because the caller expects to be able to receive a generic register size by
calling one plus the number of known registers.  However,
x86_cpu_gdb_read_register() in target/i386/gdbstub.c will only provide
register sizes for known registers, since they're not always the same.

I'm dealing with this just by shimming 8 whenever gdb_read_register returns
0, but that's obviously not correct.  Suggestions?

--Dan

P.S.  call still won't work unless linux is launched with noexec=off
noexec32=off, due to NX.

[Qemu-devel] [PATCH v2] block/nvme: fix Coverity reports

[Fam Zheng]

From: Paolo Bonzini 

1) string not null terminated in sysfs_find_group_file

2) NULL pointer dereference and dead local variable in nvme_init.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Fam Zheng 

---

v2: Fix error path.
---
 block/nvme.c| 10 +++---
 util/vfio-helpers.c |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index e9d0e218fc..a62c92a190 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -644,7 +644,7 @@ static int nvme_init(BlockDriverState *bs, const char 
*device, int namespace,
 aio_set_event_notifier(bdrv_get_aio_context(bs), >irq_notifier,
false, nvme_handle_event, nvme_poll_cb);
 
-nvme_identify(bs, namespace, errp);
+nvme_identify(bs, namespace, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 ret = -EIO;
@@ -665,8 +665,12 @@ fail_queue:
 nvme_free_queue_pair(bs, s->queues[0]);
 fail:
 g_free(s->queues);
-qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE);
-qemu_vfio_close(s->vfio);
+if (s->regs) {
+qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE);
+}
+if (s->vfio) {
+qemu_vfio_close(s->vfio);
+}
 event_notifier_cleanup(>irq_notifier);
 return ret;
 }
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index f478b68400..006674c916 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -104,7 +104,7 @@ static char *sysfs_find_group_file(const char *device, 
Error **errp)
 char *path = NULL;
 
 sysfs_link = g_strdup_printf("/sys/bus/pci/devices/%s/iommu_group", 
device);
-sysfs_group = g_malloc(PATH_MAX);
+sysfs_group = g_malloc0(PATH_MAX);
 if (readlink(sysfs_link, sysfs_group, PATH_MAX - 1) == -1) {
 error_setg_errno(errp, errno, "Failed to find iommu group sysfs path");
 goto out;
-- 
2.14.3

Re: [Qemu-devel] [Qemu-stable] [PATCH 00/54] Patch Round-up for stable 2.11.1, freeze on 2018-02-12

[Michael Roth]

Quoting Michael Roth (2018-02-06 13:14:21)
> Hi everyone,  
> 
> 
> The following new patches are queued for QEMU stable v2.11.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-2.11-staging
> 
> The release is planned for 2017-02-14:
> 
>   https://wiki.qemu.org/Planning/2.11
> 
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.
> 
> Of particular importance would be any feedback on the various QEMU
> patches relating to Spectre/Meltdown mitigation. The current tree has
> what I understand to be the QEMU components required for x86, s390,
> and pseries, but feedback/confirmation from the various authors would
> be greatly appreciated.

Thank you for the responses/suggestions. The following additional
patches have been queued for the release and pushed to:

https://github.com/mdroth/qemu/commits/stable-2.11-staging

spapr: add missing break in h_get_cpu_characteristics() (Greg Kurz)
vga: check the validation of memory addr when draw text (linzhecheng)
input: fix memory leak (linzhecheng)
ui: correctly advance output buffer when writing SASL data (Daniel P.  Berrangé)
ui: avoid sign extension using client width/height (Daniel P. Berrange)
ui: mix misleading comments & return types of VNC I/O helper methods (Daniel P. 
Berrange)
ui: add trace events related to VNC client throttling (Daniel P.  Berrange)
ui: place a hard cap on VNC server output buffer size (Daniel P.  Berrange)
ui: fix VNC client throttling when forced update is requested (Daniel P.  
Berrange)
ui: fix VNC client throttling when audio capture is active (Daniel P.  Berrange)
ui: refactor code for determining if an update should be sent to the client 
(Daniel P. Berrange)
ui: correctly reset framebuffer update state after processing dirty regions 
(Daniel P. Berrange)
ui: introduce enum to track VNC client framebuffer update request state (Daniel 
P. Berrange)
ui: track how much decoded data we consumed when doing SASL encoding (Daniel P. 
Berrange)
ui: avoid pointless VNC updates if framebuffer isn't dirty (Daniel P.  Berrange)
ui: remove redundant indentation in vnc_client_update (Daniel P.  Berrange)
ui: remove unreachable code in vnc_update_client (Daniel P. Berrange)
ui: remove 'sync' parameter from vnc_update_client (Daniel P. Berrange)
migration: incoming postcopy advise sanity checks (Greg Kurz)
target/sh4: add missing tcg_temp_free() in _decode_opc() (Philippe 
Mathieu-Daudé)
migration/savevm.c: set MAX_VM_CMD_PACKAGED_SIZE to 1ul << 32 (Daniel Henrique 
Barboza)
migration: Recover block devices if failure in device state (Dr. David Alan 
Gilbert)
migration: Don't leak IO channels (Ross Lagerwall)
s390x/sclp: fix event mask handling (Christian Borntraeger)
memory: set ioeventfd_update_pending after address_space_update_ioeventfds 
(linzhecheng)

> 
> Thanks!
> 
> 
> 
> The following changes since commit 0a0dc59d27527b78a195c2d838d28b7b49e5a639:
> 
>   Update version for v2.11.0 release (2017-12-13 14:31:09 +)
> 
> are available in the git repository at:
> 
>   git://github.com/mdroth/qemu.git stable-2.11-staging
> 
> for you to fetch changes up to ed8b4ecc68d6bfe98000b08d649049d0c1174c11:
> 
>   target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS (2018-02-05 19:07:38 
> -0600)
> 
> 
> Alex Bennée (1):
>   target/sh4: fix TCG leak during gusa sequence
> 
> Alex Williamson (1):
>   vfio: Fix vfio-kvm group registration
> 
> Christian Borntraeger (2):
>   s390x/kvm: Handle bpb feature
>   s390x/kvm: provide stfle.81
> 
> Claudio Imbrenda (1):
>   s390x: fix storage attributes migration for non-small guests
> 
> Cornelia Huck (1):
>   linux-headers: update
> 
> Cédric Le Goater (1):
>   target/ppc: introduce the PPC_BIT() macro
> 
> David Gibson (7):
>   spapr: Add pseries-2.12 machine type
>   spapr: Capabilities infrastructure
>   spapr: Treat Hardware Transactional Memory (HTM) as an optional 
> capability
>   spapr: Validate capabilities on migration
>   target/ppc: Clean up probing of VMX, VSX and DFP availability on KVM
>   spapr: Handle VMX/VSX presence as an spapr capability flag
>   spapr: Handle Decimal Floating Point (DFP) as an optional capability
> 
> Eduardo Habkost (5):
>   i386: Change X86CPUDefinition::model_id to const char*
>   i386: Add spec-ctrl CPUID bit
>   i386: Add FEAT_8000_0008_EBX CPUID feature word
>   i386: Add new -IBRS versions of Intel CPU models
>   i386: Add EPYC-IBPB CPU model
> 
> Eric Auger (1):
>   linux-headers: update to 4.15-rc1
> 
> Fam Zheng (3):
>   block: Open backing image in force share mode for size probe
>   osdep: Retry SETLK upon EINTR
>   usb-storage: Fix share-rw option parsing
> 
> Greg Kurz (2):
>   

Re: [Qemu-devel] [PATCH RESEND v12 11/30] sdhci: replace DMA magic value by BLOCK_SIZE_MASK

[Fam Zheng]

On Mon, 02/12 15:00, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Alistair Francis 

Hmm.. Something is going weird here. It's still not in the archive

https://lists.gnu.org/archive/html/qemu-devel/2018-02/threads.html

So I manually bounced it to impor...@patchew.org and then it is seen there. But
git apply failed:

https://patchew.org/QEMU/20180209145430.26007-1-f4...@amsat.org/

> Switched to a new branch '20180209145430.26007-1-f4...@amsat.org'
> Applying: sdhci: use error_propagate(local_err) in realize()
> Applying: sdhci: add qtest to check the SD capabilities register
> Applying: sdhci: add check_capab_readonly() qtest
> Applying: sdhci: add a check_capab_baseclock() qtest
> Applying: sdhci: add a check_capab_sdma() qtest
> Applying: sdhci: add qtest to check the SD Spec version
> Applying: sdhci: add a 'spec_version property' (default to v2)
> Applying: sdhci: use a numeric value for the default CAPAB register
> Applying: sdhci: simplify sdhci_get_fifolen()
> Applying: sdhci: check the Spec v1 capabilities correctness
> Applying: sdhci: replace DMA magic value by BLOCK_SIZE_MASK
> error: sha1 information is lacking or useless (hw/sd/sdhci.c).
> error: could not build fake ancestor
> Patch failed at 0001 sdhci: replace DMA magic value by BLOCK_SIZE_MASK
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> Failed to apply patch:
> [PATCH RESEND v12 11/30] sdhci: replace DMA magic value by BLOCK_SIZE_MASK

Fam

Re: [Qemu-devel] [PATCH qemu v7 2/4] vfio/pci: Relax DMA map errors for MMIO regions

[Alexey Kardashevskiy]

On 13/02/18 03:06, Alex Williamson wrote:
> On Mon, 12 Feb 2018 18:05:54 +1100
> Alexey Kardashevskiy  wrote:
> 
>> On 12/02/18 16:19, David Gibson wrote:
>>> On Fri, Feb 09, 2018 at 06:55:01PM +1100, Alexey Kardashevskiy wrote:  
 At the moment if vfio_memory_listener is registered in the system memory
 address space, it maps/unmaps every RAM memory region for DMA.
 It expects system page size aligned memory sections so vfio_dma_map
 would not fail and so far this has been the case. A mapping failure
 would be fatal. A side effect of such behavior is that some MMIO pages
 would not be mapped silently.

 However we are going to change MSIX BAR handling so we will end having
 non-aligned sections in vfio_memory_listener (more details is in
 the next patch) and vfio_dma_map will exit QEMU.

 In order to avoid fatal failures on what previously was not a failure and
 was just silently ignored, this checks the section alignment to
 the smallest supported IOMMU page size and prints an error if not aligned;
 it also prints an error if vfio_dma_map failed despite the page size check.
 Both errors are not fatal; only MMIO RAM regions are checked
 (aka "RAM device" regions).

 If the amount of errors printed is overwhelming, the MSIX relocation
 could be used to avoid excessive error output.

 This is unlikely to cause any behavioral change.

 Signed-off-by: Alexey Kardashevskiy   
>>>
>>> There are some relatively superficial problems noted below.
>>>
>>> But more fundamentally, this feels like it's extending an existing
>>> hack past the point of usefulness.
>>>
>>> The explicit check for is_ram_device() here has always bothered me -
>>> it's not like a real bus bridge magically knows whether a target
>>> address maps to RAM or not.
>>>
>>> What I think is really going on is that even for systems without an
>>> IOMMU, it's not really true to say that the PCI address space maps
>>> directly onto address_space_memory.  Instead, there's a large, but
>>> much less than 2^64 sized, "upstream window" at address 0 on the PCI
>>> bus, which is identity mapped to the system bus.  Details will vary
>>> with the system, but in practice we expect nothing but RAM to be in
>>> that window.  Addresses not within that window won't be mapped to the
>>> system bus but will just be broadcast on the PCI bus and might be
>>> picked up as a p2p transaction.  
>>
>> Currently this p2p works only via the IOMMU, direct p2p is not possible as
>> the guest needs to know physical MMIO addresses to make p2p work and it
>> does not.
> 
> /me points to the Direct Translated P2P section of the ACS spec, though
> it's as prone to spoofing by the device as ATS.  In any case, p2p
> reflected from the IOMMU is still p2p and offloads the CPU even if
> bandwidth suffers vs bare metal depending on if the data doubles back
> over any links.  Thanks,

Sure, I was just saying that p2p via IOMMU won't be as simple as broadcast
on the PCI bus, IOMMU needs to be programmed in advance to make this work,
and current that broadcast won't work for the passed through devices.



-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 02/19] spapr: introduce a skeleton for the XIVE interrupt controller

[Alexey Kardashevskiy]

On 13/02/18 01:40, Benjamin Herrenschmidt wrote:
> On Mon, 2018-02-12 at 13:20 +0100, Andrea Bolognani wrote:
>> On Mon, 2018-02-12 at 13:02 +1100, Alexey Kardashevskiy wrote:
>>> On 12/02/18 09:55, Benjamin Herrenschmidt wrote:
 Well, we have a problem then. It looks like Qemu broken migration is
 fundamentally incompatible with PAPR and CAS design...

 I know we don't migrate the configuration, that's not exactly what I
 had in mind tho... Can we have some piece of *data* from the machine be
 migrated first, and use it on the target to reconfigure the interrupt
 controller before the stream arrives ?
>>>
>>> These days this is done via libvirt - it reads properties it needs via QMP,
>>> then sends an XML with everything (the interrupt controller type may be one
>>> of such properties), and starts the destination QEMU with the explicit
>>> interrupt controller (like -machine pseries,intrc=xive).
>>
>> Clarification: libvirt will use the user-defined XML configuration
>> to generate the QEMU command line both for the source and the target
>> of the migration, but it will not automagically figure out properties
>> through QMP. So if you want the controller to explicitly show up on
>> the QEMU command line, libvirt should be taught about it.
> 
> Which can't work because the guest pretty much decides what it will be
> early on during the boot process.


At the time of migration the guest has told QEMU what intrc it wants (via
cas?) and libvirt can ask QEMU via QMP about that when migrating.


> So we're back to square 1 having to instanciate both objects in qemu
> with some kind of "activation" flag.
> 
> Cheers,
> Ben.
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH] travis: use libgcc-4.8-dev (libgcc-6-dev is not available on Ubuntu 14.04)

[Paolo Bonzini]

On 12/02/2018 19:46, Philippe Mathieu-Daudé wrote:
> Travis image is based on Ubuntu Trusty (14.04), since d83414e1fd1 we get:
> 
>   $ sudo -E \
> apt-get -yq --no-install-suggests --no-install-recommends --force-yes \
>   install \
> libaio-dev libattr1-dev libbrlapi-dev libcap-ng-dev libgcc-6-dev \
> libgnutls-dev libgtk-3-dev libiscsi-dev liblttng-ust-dev \
> libncurses5-dev libnfs-dev libnss3-dev libpixman-1-dev libpng12-dev \
> librados-dev libsdl1.2-dev libseccomp-dev libspice-protocol-dev \
> libspice-server-dev libssh2-1-dev liburcu-dev libusb-1.0-0-dev \
> libvte-2.90-dev sparse uuid-dev
>   Reading package lists...
>   Building dependency tree...
>   Reading state information...
>   E: Unable to locate package libgcc-6-dev
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Since libgcc-dev is used, Travis jobs take much longer.
> 
>  .travis.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 0dd5020552..79377c8de0 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -13,7 +13,7 @@ addons:
>- libattr1-dev
>- libbrlapi-dev
>- libcap-ng-dev
> -  - libgcc-6-dev
> +  - libgcc-4.8-dev
>- libgnutls-dev
>- libgtk-3-dev
>- libiscsi-dev
> 

I can queue this immediately, if that's useful to get it in sooner.

Paolo

Re: [Qemu-devel] [PATCH v2 29/29] qapi: Don't create useless directory qapi-generated

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

We used to generate first test and later QGA QAPI code into
qapi-generated/.  Commit b93b63f574 moved the test code to tests/.
Commit 54c2e50205 moved the QGA code to qga/qapi-generated/.  The
directory has been unused since.

Signed-off-by: Markus Armbruster 
---
  .gitignore | 1 -
  Makefile   | 1 -
  configure  | 1 -
  3 files changed, 3 deletions(-)


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 28/29] Fix up dangling references to qmp-commands.* in comment and doc

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

Fix up the reference to qmp-commands.hx in qmp.c.  Missed in commit
5032a16d1d.

Fix up the reference to qmp-commands.txt in
docs/xen-save-devices-state.txt.  Missed in commit 4d8bb958fa.

Signed-off-by: Markus Armbruster 
---
  docs/xen-save-devices-state.txt |  3 +--
  qmp.c   | 14 +++---
  2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/docs/xen-save-devices-state.txt b/docs/xen-save-devices-state.txt
index a72ecc8081..1912ecad25 100644
--- a/docs/xen-save-devices-state.txt
+++ b/docs/xen-save-devices-state.txt
@@ -8,8 +8,7 @@ These operations are normally used with migration (see 
migration.txt),
  however it is also possible to save the state of all devices to file,
  without saving the RAM or the block devices of the VM.
  
-This operation is called "xen-save-devices-state" (see

-qmp-commands.txt)
+The save operation is available as QMP command xen-save-devices-state.


I like the fact that you made the reword more generic (in case we rename 
things again, one less place to edit).


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH] tcg: Improve tcg_gen_muli_i32/i64

[Emilio G. Cota]

On Sun, Feb 11, 2018 at 12:39:34 -0800, Richard Henderson wrote:
> Convert multiplication by power of two to left shift.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Emilio G. Cota 

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v2 27/29] qapi: Move qapi-schema.json to qapi/, rename generated files

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

Move qapi-schema.json to qapi/, so it's next to its modules, and all
files get generated to qapi/, not just the ones generated for modules.

Consistently name the generated files qapi-MODULE.EXT:
qmp-commands.[ch] become qapi-commands.[ch], qapi-event.[ch] become
qapi-events.[ch], and qmp-introspect.[ch] become qapi-introspect.[ch].
This gets rid of the temporary hacks in scripts/qapi/commands.py and
scripts/qapi/events.py.


Ah, so my parallel series that proposed naming the file 
qapi/qmp-schema.qapi gets interesting, with your patch favoring the 
qapi- naming everywhere.  I'll have to think about how much (or little) 
of my series to rebase on top of this (I like my notion of renaming to 
the .qapi suffix, though, as we really are using files that aren't JSON, 
but only resemble it).




Signed-off-by: Markus Armbruster 
---



+++ b/.gitignore
@@ -29,8 +29,8 @@
  /qga/qapi-generated
  /qapi-generated
  /qapi-gen-timestamp
-/qapi-builtin-types.[ch]
-/qapi-builtin-visit.[ch]
+/qapi/qapi-builtin-types.[ch]
+/qapi/qapi-builtin-visit.[ch]


Might be some interesting churn if you like my idea of using globs for 
easier maintenance of this file.



+++ b/tpm.c
@@ -182,7 +182,6 @@ int tpm_config_parse(QemuOptsList *opts_list, const char 
*optarg)
  
  /*

   * Walk the list of active TPM backends and collect information about them
- * following the schema description in qapi-schema.json.
   */


Should the overall comment keep the trailing '.'?

Reviewed-by: Eric Blake 

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

[Qemu-devel] [PATCH v2 4/5] usb-mtp: Introduce write support for MTP objects

[Bandan Das]

Allow write operations on behalf of the initiator. The
precursor to write is the sending of the write metadata
that consists of the ObjectInfo dataset. This patch introduces
a flag that is set when the responder is ready to receive
write data based on a previous SendObjectInfo operation by
the initiator (The SendObjectInfo implementation is in a
later patch)

Signed-off-by: Bandan Das 
---
 hw/usb/dev-mtp.c | 159 ++-
 1 file changed, 157 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 5f53f200c4..8d615cabc0 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -47,6 +47,7 @@ enum mtp_code {
 CMD_GET_OBJECT_INFO= 0x1008,
 CMD_GET_OBJECT = 0x1009,
 CMD_DELETE_OBJECT  = 0x100b,
+CMD_SEND_OBJECT= 0x100d,
 CMD_GET_PARTIAL_OBJECT = 0x101b,
 CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
 CMD_GET_OBJECT_PROP_DESC   = 0x9802,
@@ -66,7 +67,9 @@ enum mtp_code {
 RES_STORE_READ_ONLY= 0x200e,
 RES_PARTIAL_DELETE = 0x2012,
 RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
+RES_INVALID_OBJECTINFO = 0x2015,
 RES_INVALID_PARENT_OBJECT  = 0x201a,
+RES_STORE_FULL = 0x200c,
 RES_INVALID_PARAMETER  = 0x201d,
 RES_SESSION_ALREADY_OPEN   = 0x201e,
 RES_INVALID_OBJECT_PROP_CODE   = 0xA801,
@@ -182,6 +185,14 @@ struct MTPState {
 int  inotifyfd;
 QTAILQ_HEAD(events, MTPMonEntry) events;
 #endif
+/* Responder is expecting a write operation */
+bool write_pending;
+struct {
+uint32_t parent_handle;
+uint16_t format;
+uint32_t size;
+char *filename;
+} dataset;
 };
 
 #define TYPE_USB_MTP "usb-mtp"
@@ -803,6 +814,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, 
MTPControl *c)
 CMD_GET_OBJECT_HANDLES,
 CMD_GET_OBJECT_INFO,
 CMD_DELETE_OBJECT,
+CMD_SEND_OBJECT,
 CMD_GET_OBJECT,
 CMD_GET_PARTIAL_OBJECT,
 CMD_GET_OBJECT_PROPS_SUPPORTED,
@@ -1381,6 +1393,14 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 nres = 1;
 res0 = data_in->length;
 break;
+case CMD_SEND_OBJECT:
+if (!s->write_pending) {
+usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO,
+ c->trans, 0, 0, 0, 0);
+return;
+}
+s->data_out = usb_mtp_data_alloc(c);
+return;
 case CMD_GET_OBJECT_PROPS_SUPPORTED:
 if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
 c->argv[0] != FMT_ASSOCIATION) {
@@ -1475,12 +1495,133 @@ static void usb_mtp_cancel_packet(USBDevice *dev, 
USBPacket *p)
 fprintf(stderr, "%s\n", __func__);
 }
 
+mode_t getumask(void)
+{
+mode_t mask = umask(0);
+umask(mask);
+return mask;
+}
+
+static void usb_mtp_write_data(MTPState *s)
+{
+MTPData *d = s->data_out;
+MTPObject *parent =
+usb_mtp_object_lookup(s, s->dataset.parent_handle);
+char *path = NULL;
+int rc = -1;
+mode_t mask = ~getumask() & 0666;
+
+assert(d != NULL);
+
+if (parent == NULL || !s->write_pending) {
+usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO, d->trans,
+ 0, 0, 0, 0);
+return;
+}
+
+if (s->dataset.filename) {
+path = g_strdup_printf("%s/%s", parent->path, s->dataset.filename);
+if (s->dataset.format == FMT_ASSOCIATION) {
+d->fd = mkdir(path, mask);
+goto free;
+}
+if (s->dataset.size < d->length) {
+usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+goto done;
+}
+d->fd = open(path, O_CREAT | O_WRONLY, mask);
+if (d->fd == -1) {
+usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+goto done;
+}
+
+/*
+ * Return success if initiator sent 0 sized data
+ */
+if (!s->dataset.size) {
+goto success;
+}
+
+rc = write(d->fd, d->data, s->dataset.size);
+if (rc == -1) {
+usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
+ 0, 0, 0, 0);
+goto done;
+}
+if (rc != s->dataset.size) {
+usb_mtp_queue_result(s, RES_INCOMPLETE_TRANSFER, d->trans,
+ 0, 0, 0, 0);
+goto done;
+}
+}
+
+success:
+usb_mtp_queue_result(s, RES_OK, d->trans,
+ 0, 0, 0, 0);
+
+done:
+/*
+ * The write dataset is kept around and freed only
+ * on success or if another write request comes in
+ */
+if (d->fd != -1) {
+close(d->fd);
+}
+free:
+g_free(s->dataset.filename);
+g_free(path);
+   

Re: [Qemu-devel] [PATCH v2 26/29] docs: Correct outdated information on QAPI

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

* Fix guidance on error classes

* Point to generated documentation

* Drop plea for documentation, because the QAPI code generator
   enforces it since commit 3313b6124b

* Minor tweaks here and there

Signed-off-by: Markus Armbruster 
---
  docs/devel/writing-qmp-commands.txt | 25 +
  docs/interop/qmp-intro.txt  |  3 ++-
  2 files changed, 11 insertions(+), 17 deletions(-)



Reviewed-by: Eric Blake 

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

[Qemu-devel] [PATCH v2 1/5] usb-mtp: Add one more argument when building results

[Bandan Das]

The response to a SendObjectInfo consists of the storageid,
parent obejct handle and the handle reserved for the new
incoming object

Signed-off-by: Bandan Das 
---
 hw/usb/dev-mtp.c | 50 +++---
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 94c2e94f10..b55aa8205e 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -765,7 +765,8 @@ static void usb_mtp_add_time(MTPData *data, time_t time)
 /* --- */
 
 static void usb_mtp_queue_result(MTPState *s, uint16_t code, uint32_t trans,
- int argc, uint32_t arg0, uint32_t arg1)
+ int argc, uint32_t arg0, uint32_t arg1,
+ uint32_t arg2)
 {
 MTPControl *c = g_new0(MTPControl, 1);
 
@@ -778,6 +779,9 @@ static void usb_mtp_queue_result(MTPState *s, uint16_t 
code, uint32_t trans,
 if (argc > 1) {
 c->argv[1] = arg1;
 }
+if (argc > 2) {
+c->argv[2] = arg2;
+}
 
 assert(s->result == NULL);
 s->result = c;
@@ -1119,7 +1123,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 /* sanity checks */
 if (c->code >= CMD_CLOSE_SESSION && s->session == 0) {
 usb_mtp_queue_result(s, RES_SESSION_NOT_OPEN,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 
@@ -1131,12 +1135,12 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 case CMD_OPEN_SESSION:
 if (s->session) {
 usb_mtp_queue_result(s, RES_SESSION_ALREADY_OPEN,
- c->trans, 1, s->session, 0);
+ c->trans, 1, s->session, 0, 0);
 return;
 }
 if (c->argv[0] == 0) {
 usb_mtp_queue_result(s, RES_INVALID_PARAMETER,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 trace_usb_mtp_op_open_session(s->dev.addr);
@@ -1165,7 +1169,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 if (c->argv[0] != QEMU_STORAGE_ID &&
 c->argv[0] != 0x) {
 usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 data_in = usb_mtp_get_storage_info(s, c);
@@ -1175,12 +1179,12 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 if (c->argv[0] != QEMU_STORAGE_ID &&
 c->argv[0] != 0x) {
 usb_mtp_queue_result(s, RES_INVALID_STORAGE_ID,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 if (c->argv[1] != 0x) {
 usb_mtp_queue_result(s, RES_SPEC_BY_FORMAT_UNSUPPORTED,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 if (c->argv[2] == 0x ||
@@ -1191,12 +1195,12 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 }
 if (o == NULL) {
 usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 if (o->format != FMT_ASSOCIATION) {
 usb_mtp_queue_result(s, RES_INVALID_PARENT_OBJECT,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 usb_mtp_object_readdir(s, o);
@@ -1212,7 +1216,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 o = usb_mtp_object_lookup(s, c->argv[0]);
 if (o == NULL) {
 usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 data_in = usb_mtp_get_object_info(s, c, o);
@@ -1221,18 +1225,18 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 o = usb_mtp_object_lookup(s, c->argv[0]);
 if (o == NULL) {
 usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 if (o->format == FMT_ASSOCIATION) {
 usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
- c->trans, 0, 0, 0);
+ c->trans, 0, 0, 0, 0);
 return;
 }
 data_in = usb_mtp_get_object(s, c, o);
 if (data_in == NULL) {
 

Re: [Qemu-devel] [PULL 00/12] ppc-for-2.12 queue 20180212

[David Gibson]

On Mon, Feb 12, 2018 at 07:46:21PM +0100, Laurent Vivier wrote:
> On 12/02/2018 04:40, David Gibson wrote:
> > The following changes since commit c7b02d7d032d6022060e4b393827c963c93ce63f:
> > 
> >   Merge remote-tracking branch 
> > 'remotes/stsquad/tags/pull-travis-speedup-090218-1' into staging 
> > (2018-02-09 16:12:34 +)
> > 
> > are available in the Git repository at:
> > 
> >   git://github.com/dgibson/qemu.git tags/ppc-for-2.12-20180212
> > 
> > for you to fetch changes up to 51f233ec92cdab7030cb7407dd7f009911c805b0:
> > 
> >   misc: introduce new mos6522 VIA device and enable it for ppc builds 
> > (2018-02-11 10:18:52 +1100)
> > 
> > 
> > ppc patch queue 2018-02-12
> > 
> > Here's the accumulatead ppc and pseries related patches for the last
> > while.  Highlights are:
> > * A number of Macintosh / CUDA cleanups from Mark Cave-Ayland
> > * An important bug fix (missing "break;") for
> >   H_GET_CPU_CHARACTERISTICS
> > * Yet another fix for SMT mode handling
> > * Assorted other cleanups and fixes
> > 
> > 
> > Daniel Henrique Barboza (1):
> >   hw/ppc: rename functions in comments
> > 
> > Greg Kurz (1):
> >   spapr: add missing break in h_get_cpu_characteristics()
> > 
> > Laurent Vivier (1):
> >   spapr: set vsmt to MAX(8, smp_threads)
> > 
> > Mark Cave-Ayland (9):
> >   cuda: do not use old_mmio accesses
> >   cuda: don't allow writes to port output pins
> >   cuda: introduce CUDAState parameter to get_counter()
> >   cuda: rename frequency property to tb_frequency
> >   cuda: minor cosmetic tidy-ups to get_next_irq_time()
> >   cuda: don't call cuda_update() when writing to ACR register
> >   cuda: set timer 1 frequency property to CUDA_TIMER_FREQ
> >   cuda: factor out timebase-derived counter value and load time
> >   misc: introduce new mos6522 VIA device and enable it for ppc builds
> 
> David, following patches from Mark's series are missing in your pull
> request, is that normal?

I don't know about normal, but it's expected.  I didn't get around to
reviewing and merging those last few patches of his before preparing
the pull request.  They'll be in the next one.

> 
>   cuda: convert to use the shared mos6522 device
>   ppc: move CUDAState and other CUDA-related definitions into separate
>cuda.h file
>   cuda: convert to trace-events
> 
> Thanks,
> Laurent
> 

-- 
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 3/5] usb-mtp: Support delete of mtp objects

[Bandan Das]

Write of existing objects by the initiator is acheived by
making a temporary buffer with the new changes, deleting the
old file and then writing a new file with the same name.
Advertise delete support and mark the store READ/WRITE since
some initiators will fail the operation if the store is marked
read only.

Signed-off-by: Bandan Das 
---
 hw/usb/dev-mtp.c | 123 +++
 1 file changed, 123 insertions(+)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 63f8f3b90b..5f53f200c4 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -46,6 +46,7 @@ enum mtp_code {
 CMD_GET_OBJECT_HANDLES = 0x1007,
 CMD_GET_OBJECT_INFO= 0x1008,
 CMD_GET_OBJECT = 0x1009,
+CMD_DELETE_OBJECT  = 0x100b,
 CMD_GET_PARTIAL_OBJECT = 0x101b,
 CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
 CMD_GET_OBJECT_PROP_DESC   = 0x9802,
@@ -62,6 +63,8 @@ enum mtp_code {
 RES_INVALID_STORAGE_ID = 0x2008,
 RES_INVALID_OBJECT_HANDLE  = 0x2009,
 RES_INVALID_OBJECT_FORMAT_CODE = 0x200b,
+RES_STORE_READ_ONLY= 0x200e,
+RES_PARTIAL_DELETE = 0x2012,
 RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
 RES_INVALID_PARENT_OBJECT  = 0x201a,
 RES_INVALID_PARAMETER  = 0x201d,
@@ -799,6 +802,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, 
MTPControl *c)
 CMD_GET_NUM_OBJECTS,
 CMD_GET_OBJECT_HANDLES,
 CMD_GET_OBJECT_INFO,
+CMD_DELETE_OBJECT,
 CMD_GET_OBJECT,
 CMD_GET_PARTIAL_OBJECT,
 CMD_GET_OBJECT_PROPS_SUPPORTED,
@@ -1113,6 +1117,120 @@ static MTPData *usb_mtp_get_object_prop_value(MTPState 
*s, MTPControl *c,
 return d;
 }
 
+/* Return correct return code for a delete event */
+enum {
+ALL_DELETE,
+PARTIAL_DELETE,
+READ_ONLY,
+};
+
+#ifndef CONFIG_INOTIFY1
+/* Assumes that children, if any, have been already freed */
+static void usb_mtp_object_free_one(MTPState *s, MTPObject *o)
+{
+assert(o->nchildren == 0);
+QTAILQ_REMOVE(>objects, o, next);
+g_free(o->name);
+g_free(o->path);
+g_free(o);
+}
+#endif
+
+static int usb_mtp_deletefn(MTPState *s, MTPObject *o, uint32_t trans)
+{
+MTPObject *iter, *iter2;
+bool partial_delete = false;
+bool success = false;
+
+/*
+ * TODO: Add support for Protection Status
+ */
+
+QLIST_FOREACH(iter, >children, list) {
+if (iter->format == FMT_ASSOCIATION) {
+QLIST_FOREACH(iter2, >children, list) {
+usb_mtp_deletefn(s, iter2, trans);
+}
+}
+}
+
+if (o->format == FMT_UNDEFINED_OBJECT) {
+if (remove(o->path)) {
+partial_delete = true;
+} else {
+#ifndef CONFIG_INOTIFY1
+usb_mtp_object_free_one(s, o);
+#endif
+success = true;
+}
+}
+
+if (o->format == FMT_ASSOCIATION) {
+if (rmdir(o->path)) {
+partial_delete = true;
+} else {
+#ifndef CONFIG_INOTIFY1
+usb_mtp_object_free_one(s, o);
+#endif
+success = true;
+}
+}
+
+if (success && partial_delete) {
+return PARTIAL_DELETE;
+}
+if (!success && partial_delete) {
+return READ_ONLY;
+}
+return ALL_DELETE;
+}
+
+static void usb_mtp_object_delete(MTPState *s, uint32_t handle,
+  uint32_t format_code, uint32_t trans)
+{
+MTPObject *o;
+int ret;
+
+/* Return error if store is read-only */
+if (!FLAG_SET(s, MTP_FLAG_WRITABLE)) {
+usb_mtp_queue_result(s, RES_STORE_READ_ONLY,
+ trans, 0, 0, 0, 0);
+return;
+}
+
+if (format_code != 0) {
+usb_mtp_queue_result(s, RES_SPEC_BY_FORMAT_UNSUPPORTED,
+ trans, 0, 0, 0, 0);
+return;
+}
+
+if (handle == 0xFFF) {
+o = QTAILQ_FIRST(>objects);
+} else {
+o = usb_mtp_object_lookup(s, handle);
+}
+if (o == NULL) {
+usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE,
+ trans, 0, 0, 0, 0);
+return;
+}
+
+ret = usb_mtp_deletefn(s, o, trans);
+if (ret == PARTIAL_DELETE) {
+usb_mtp_queue_result(s, RES_PARTIAL_DELETE,
+ trans, 0, 0, 0, 0);
+return;
+} else if (ret == READ_ONLY) {
+usb_mtp_queue_result(s, RES_STORE_READ_ONLY, trans,
+ 0, 0, 0, 0);
+return;
+} else {
+usb_mtp_queue_result(s, RES_OK, trans,
+ 0, 0, 0, 0);
+return;
+}
+}
+
 static void usb_mtp_command(MTPState *s, MTPControl *c)
 {
 MTPData *data_in = NULL;
@@ -1239,6 +1357,9 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 return;
 }
 break;
+case CMD_DELETE_OBJECT:
+usb_mtp_object_delete(s, 

[Qemu-devel] [PATCH v2 0/5] Initial write support for MTP object

[Bandan Das]

v2:
  3/5: Set mtp store flag to read only
  4/5: Fix compiler warnings and change default file permissions
  5/5: Fix file permissions

These patches implement write support for Qemu's MTP
emulation. Simple tests such as delete/move/edit/copy work ok.
Current issues/TODO:

 - File transfers > 4GB has not been tested and will probably not work
 - Some (or most) MTP clients don't advertise hidden files and folders (names
 that start with a .) even though iiuc Qemu MTP does advertise these files.
 This can confuse certain applications such as text editors or git.
 - Also related, file editors typically run fsync when saving. Depending on
 the MTP client, it may choose not to implement it (such as simple-mtpfs that 
 runs on top of fuse).
 - Needs more testing :)

Bandan Das (5):
  usb-mtp: Add one more argument when building results
  usb-mtp: print parent path in IN_IGNORED trace fn
  usb-mtp: Support delete of mtp objects
  usb-mtp: Introduce write support for MTP objects
  usb-mtp: Advertise SendObjectInfo for write support

 hw/usb/dev-mtp.c | 455 +++
 1 file changed, 426 insertions(+), 29 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v2 2/5] usb-mtp: print parent path in IN_IGNORED trace fn

[Bandan Das]

Fix a possible null dereference when deleting a folder and
its contents. An ignored event might be received for its contents
after the parent folder is deleted which will return a null object.

Signed-off-by: Bandan Das 
---
 hw/usb/dev-mtp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index b55aa8205e..63f8f3b90b 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -540,9 +540,8 @@ static void inotify_watchfn(void *arg)
 break;
 
 case IN_IGNORED:
-o = usb_mtp_object_lookup_name(parent, event->name, 
event->len);
-trace_usb_mtp_inotify_event(s->dev.addr, o->path,
-  event->mask, "Obj ignored");
+trace_usb_mtp_inotify_event(s->dev.addr, parent->path,
+  event->mask, "Obj parent dir ignored");
 break;
 
 default:
-- 
2.14.3

[Qemu-devel] [PATCH v2 5/5] usb-mtp: Advertise SendObjectInfo for write support

[Bandan Das]

This patch implements a dummy ObjectInfo structure so that
it's easy to typecast the incoming data. If the metadata is
valid, write_pending is set. Also, the incoming filename
is utf-16, so, instead of depending on external libraries, just
implement a simple function to get the filename

Signed-off-by: Bandan Das 
---
 hw/usb/dev-mtp.c | 118 ++-
 1 file changed, 117 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 8d615cabc0..90cf54e2fe 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -47,6 +47,7 @@ enum mtp_code {
 CMD_GET_OBJECT_INFO= 0x1008,
 CMD_GET_OBJECT = 0x1009,
 CMD_DELETE_OBJECT  = 0x100b,
+CMD_SEND_OBJECT_INFO   = 0x100c,
 CMD_SEND_OBJECT= 0x100d,
 CMD_GET_PARTIAL_OBJECT = 0x101b,
 CMD_GET_OBJECT_PROPS_SUPPORTED = 0x9801,
@@ -66,8 +67,10 @@ enum mtp_code {
 RES_INVALID_OBJECT_FORMAT_CODE = 0x200b,
 RES_STORE_READ_ONLY= 0x200e,
 RES_PARTIAL_DELETE = 0x2012,
+RES_STORE_NOT_AVAILABLE= 0x2013,
 RES_SPEC_BY_FORMAT_UNSUPPORTED = 0x2014,
 RES_INVALID_OBJECTINFO = 0x2015,
+RES_DESTINATION_UNSUPPORTED= 0x2020,
 RES_INVALID_PARENT_OBJECT  = 0x201a,
 RES_STORE_FULL = 0x200c,
 RES_INVALID_PARAMETER  = 0x201d,
@@ -195,6 +198,24 @@ struct MTPState {
 } dataset;
 };
 
+/*
+ * ObjectInfo dataset received from initiator
+ * Fields we don't care about are ignored
+ */
+typedef struct {
+char __pad1[4];
+uint16_t format;
+char __pad2[2];
+uint32_t size;
+char __pad3[30];
+uint16_t assoc_type;
+uint32_t assoc_desc;
+char __pad4[4];
+uint8_t length;
+uint16_t filename[0];
+/* string and other data follows */
+} QEMU_PACKED ObjectInfo;
+
 #define TYPE_USB_MTP "usb-mtp"
 #define USB_MTP(obj) OBJECT_CHECK(MTPState, (obj), TYPE_USB_MTP)
 
@@ -814,6 +835,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, 
MTPControl *c)
 CMD_GET_OBJECT_HANDLES,
 CMD_GET_OBJECT_INFO,
 CMD_DELETE_OBJECT,
+CMD_SEND_OBJECT_INFO,
 CMD_SEND_OBJECT,
 CMD_GET_OBJECT,
 CMD_GET_PARTIAL_OBJECT,
@@ -1246,7 +1268,7 @@ static void usb_mtp_object_delete(MTPState *s, uint32_t 
handle,
 static void usb_mtp_command(MTPState *s, MTPControl *c)
 {
 MTPData *data_in = NULL;
-MTPObject *o;
+MTPObject *o = NULL;
 uint32_t nres = 0, res0 = 0;
 
 /* sanity checks */
@@ -1393,6 +1415,37 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 nres = 1;
 res0 = data_in->length;
 break;
+case CMD_SEND_OBJECT_INFO:
+/* First parameter points to storage id or is 0 */
+if (c->argv[0] && (c->argv[0] != QEMU_STORAGE_ID)) {
+usb_mtp_queue_result(s, RES_STORE_NOT_AVAILABLE, c->trans,
+ 0, 0, 0, 0);
+} else if (c->argv[1] && !c->argv[0]) {
+/* If second parameter is specified, first must also be specified 
*/
+usb_mtp_queue_result(s, RES_DESTINATION_UNSUPPORTED, c->trans,
+ 0, 0, 0, 0);
+} else {
+uint32_t handle = c->argv[1];
+if (handle == 0x || handle == 0) {
+/* root object */
+o = QTAILQ_FIRST(>objects);
+} else {
+o = usb_mtp_object_lookup(s, handle);
+}
+if (o == NULL) {
+usb_mtp_queue_result(s, RES_INVALID_OBJECT_HANDLE, c->trans,
+ 0, 0, 0, 0);
+}
+if (o->format != FMT_ASSOCIATION) {
+usb_mtp_queue_result(s, RES_INVALID_PARENT_OBJECT, c->trans,
+ 0, 0, 0, 0);
+}
+}
+if (o) {
+s->dataset.parent_handle = o->handle;
+}
+s->data_out = usb_mtp_data_alloc(c);
+return;
 case CMD_SEND_OBJECT:
 if (!s->write_pending) {
 usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO,
@@ -1502,6 +1555,17 @@ mode_t getumask(void)
 return mask;
 }
 
+static void utf16_to_str(uint8_t len, uint16_t *arr, char *name)
+{
+int count;
+
+for (count = 0; count < len; count++) {
+/* Check for valid ascii */
+assert(!(arr[count] & 0xFF80));
+name[count] = arr[count];
+}
+}
+
 static void usb_mtp_write_data(MTPState *s)
 {
 MTPData *d = s->data_out;
@@ -1575,6 +1639,45 @@ free:
 s->write_pending = false;
 }
 
+static void usb_mtp_write_metadata(MTPState *s)
+{
+MTPData *d = s->data_out;
+ObjectInfo *dataset = (ObjectInfo *)d->data;
+char *filename = g_new0(char, dataset->length);
+MTPObject *o;
+MTPObject *p = usb_mtp_object_lookup(s, s->dataset.parent_handle);
+uint32_t next_handle = 

Re: [Qemu-devel] [PATCH v2 25/29] docs/devel/writing-qmp-commands: Update for modular QAPI

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

With modular code generation, putting stuff right into
qapi-schema.json is a bad idea.  Update writing-qmp-commands.txt
accordingly.

Signed-off-by: Markus Armbruster 
---
  docs/devel/writing-qmp-commands.txt | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)



Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 24/29] qapi: Empty out qapi-schema.json

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

The previous commit improved compile time by including less of the
generated QAPI headers.  This is impossible for stuff defined directly
in qapi-schema.json, because that ends up in headers that that pull in
everything.

Move everything but include directives from qapi-schema.json to new
sub-module qapi/misc.json, then include just the "misc" shard where
possible.

It's not possible everywhere, except:


Odd wording, did you mean one of:

It's possible everywhere, except:
It's almost possible everywhere, except:



* monitor.c needs qmp-command.h to get qmp_init_marshall()


s/marshall/marshal/



* monitor.c, ui/vnc.c and the generated qapi-event-FOO.c need
   qapi-event.h to get enum QAPIEvent

Perhaps we'll get rid of those some other day.

Adding a type to qapi/migration.json now recompiles some 120 instead
of 2300 out of 5100 objects.


Getting better :)



Signed-off-by: Markus Armbruster 
---
  .gitignore |4 +
  Makefile   |9 +
  Makefile.objs  |4 +
  arch_init.c|2 +-
  balloon.c  |2 +-
  block/iscsi.c  |2 +-
  cpus.c |2 +-
  dump.c |4 +-
  hmp.c  |   10 +-
  hw/acpi/core.c |2 +-
  hw/acpi/cpu.c  |2 +-
  hw/acpi/memory_hotplug.c   |2 +-
  hw/acpi/vmgenid.c  |2 +-
  hw/core/qdev.c |2 +-
  hw/i386/xen/xen-hvm.c  |2 +-
  hw/ipmi/ipmi.c |2 +-
  hw/pci/pci-stub.c  |2 +-
  hw/pci/pci.c   |2 +-
  hw/ppc/spapr_rtc.c |2 +-
  hw/s390x/s390-skeys.c  |2 +-
  hw/timer/mc146818rtc.c |4 +-
  hw/virtio/virtio-balloon.c |2 +-
  hw/watchdog/watchdog.c |2 +-
  include/hw/qdev-properties.h   |3 +-
  include/monitor/monitor.h  |2 +-
  include/sysemu/arch_init.h |2 +-
  include/sysemu/balloon.h   |2 +-
  include/sysemu/dump.h  |2 +-
  include/sysemu/hostmem.h   |2 +-
  include/sysemu/replay.h|3 +-
  iothread.c |2 +-
  migration/savevm.c |3 +-
  numa.c |4 +-


Mostly mechanical,


  qapi-schema.json   | 3098 +---
  qapi/misc.json | 3090 +++


A huge move of content (git won't flag it as a rename, though ;(

But why is the new file smaller than what was removed from the old file? 
 Let's see...[1]



  qapi/run-state.json|   10 +
  qdev-monitor.c |2 +-
  qmp.c  |4 +-
  stubs/uuid.c   |2 +-
  stubs/vmgenid.c|2 +-
  stubs/xen-hvm.c|2 +-
  target/arm/monitor.c   |3 +-
  target/i386/cpu.c  |4 +-
  tests/qmp-test.c   |3 +-
  tests/test-qobject-input-visitor.c |2 +-
  tests/test-visitor-serialization.c |1 -
  ui/gtk.c   |2 +-
  util/qemu-config.c |2 +-
  vl.c   |4 +-
  49 files changed, 3181 insertions(+), 3144 deletions(-)
  create mode 100644 qapi/misc.json


Huge email, but reviewing is not too hard.



diff --git a/.gitignore b/.gitignore
index 42c57998fd..7f162e862f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@
  /qapi/qapi-commands-crypto.[ch]
  /qapi/qapi-commands-introspect.[ch]
  /qapi/qapi-commands-migration.[ch]
+/qapi/qapi-commands-misc.[ch]


If my comments on the earlier patch result in a glob here, you wouldn't 
need these changes.



--- a/Makefile
+++ b/Makefile
@@ -99,6 +99,7 @@ GENERATED_FILES += qapi/qapi-types-common.h 
qapi/qapi-types-common.c
  GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c
  GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c
  GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c
+GENERATED_FILES += qapi/qapi-types-misc.h qapi/qapi-types-misc.c
  GENERATED_FILES += qapi/qapi-types-net.h qapi/qapi-types-net.c
  GENERATED_FILES += qapi/qapi-types-rocker.h qapi/qapi-types-rocker.c
  GENERATED_FILES += qapi/qapi-types-run-state.h qapi/qapi-types-run-state.c
@@ -116,6 +117,7 @@ GENERATED_FILES += qapi/qapi-visit-common.h 
qapi/qapi-visit-common.c
  GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c
  GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c
  GENERATED_FILES += qapi/qapi-visit-migration.h qapi/qapi-visit-migration.c
+GENERATED_FILES 

Re: [Qemu-devel] "make check -j4" hangs

[Paolo Bonzini]

On 12/02/2018 20:30, Thomas Huth wrote:
> On 12.02.2018 15:42, klim wrote:
> [...]
>> I just have reverted my 2 commits and
>>
>> after that make check -j32 hangs
>>
>> with
>>
>> GTester: last random seed: R02Sb95a3bf6ab4c05540cec188081a7cc2a
>>
>> in vhost-user-test
>>
>> so it is not my fault
> 
> You're right. I've bisected the issue again, and this time I've applied
> 8f6d701044b ("tests/test-filter-redirector: move close()") on top of
> each step if necessary, and indeed, the "vhost-user-test" problem has
> nothing to do with your patches, Klim, but with the one from Marc-André.
> According to git bisect, this is the commit that introduced the problem:
> 
>   commit 7e49f5e8e508ed020c96798b3f7083e24e0e425b
>   tests: use memfd in vhost-user-test
> 
> Peter, since this is quite annoying if "make check" is not working
> reliably, could you please revert that commit until a proper solution is
> found?

I was about to send one when Klim answered.  I'll send it tomorrow, with
Marc-André's patch reverted.

Paolo

Re: [Qemu-devel] [PATCH v2 23/29] Include less of the generated modular QAPI headers

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

In my "build everything" tree, a change to the types in
qapi-schema.json triggers a recompile of about 4800 out of 5100
objects.

The previous commit split up qmp-commands.h, qmp-event.h, qmp-visit.h,
qapi-types.h.  Each of these headers still includes all its shards.
Reduce compile time by including just the shards we actually need.

To illustrate the benefits: adding a type to qapi/migration.json now
recompiles some 2300 instead of 4800 objects.  The next commit will
improve it further.

Signed-off-by: Markus Armbruster 
---
  backends/cryptodev.c |  1 -
  backends/hostmem.c   |  3 ++-


How did you determine which shards to include where? Remove all shards, 
try compiling, and see what fails, then add back in the right includes? 
Or was it scripted somehow?  (Trying to figure out, in case I have to 
rebase this patch)



+++ b/block/crypto.c
@@ -24,9 +24,9 @@
  #include "sysemu/block-backend.h"
  #include "crypto/block.h"
  #include "qapi/opts-visitor.h"
+#include "qapi/qapi-visit-crypto.h"
  #include "qapi/qmp/qdict.h"
  #include "qapi/qobject-input-visitor.h"
-#include "qapi-visit.h"
  #include "qapi/error.h"


Any rhyme or reason to the resulting include order that I should be 
looking for (we aren't alphabetical in general, but if your changes were 
trying to honor a particular pattern among the few affected lines, 
that's useful to know).



+++ b/scripts/qapi/commands.py


The non-mechanical portion of the patch :)


@@ -241,6 +241,9 @@ class 
QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
  
  def _begin_module(self, name):

  self._visited_ret_types[self._genc] = set()
+commands = self._module_basename('qapi-commands', name)
+types = self._module_basename('qapi-types', name)
+visit = self._module_basename('qapi-visit', name)
  self._genc.add(mcgen('''
  #include "qemu/osdep.h"
  #include "qemu-common.h"
@@ -251,18 +254,17 @@ class 
QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
  #include "qapi/qobject-input-visitor.h"
  #include "qapi/dealloc-visitor.h"
  #include "qapi/error.h"
-#include "%(prefix)sqapi-types.h"
-#include "%(prefix)sqapi-visit.h"
-#include "%(prefix)sqmp-commands.h"
+#include "%(visit)s.h"
+#include "%(commands)s.h"
  
  ''',

- prefix=self._prefix))
+ commands=commands, visit=visit))
  self._genh.add(mcgen('''
-#include "%(prefix)sqapi-types.h"
+#include "%(types)s.h"
  #include "qapi/qmp/dispatch.h"
  
  ''',

- prefix=self._prefix))
+ types=types))


Makes sense.

The compiler will catch anything we did wrong.

Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 22/29] qapi: Generate separate .h, .c for each module

[Eric Blake]

On 02/11/2018 03:36 AM, Markus Armbruster wrote:

Our qapi-schema.json is composed of modules connected by include
directives, but the generated code is monolithic all the same: one
qapi-types.h with all the types, one qapi-visit.h with all the
visitors, and so forth.  These monolithic headers get included all
over the place.  In my "build everything" tree, adding a QAPI type
recompiles about 4800 out of 5100 objects.

We wouldn't write such monolithic headers by hand.  It stands to
reason that we shouldn't generate them, either.

Split up generated qapi-types.h to mirror the schema's modular
structure: one header per module.  Name the main module's header
qapi-types.h, and sub-module D/B.json's header D/qapi-types-B.h.

Mirror the schema's includes in the headers, so that qapi-types.h gets
you everything exactly as before.  If you need less, you can include
one or more of the sub-module headers.  To be exploited shortly.

Split up qapi-types.c, qapi-visit.h, qapi-visit.c, qmp-commands.h,
qmp-commands.c, qapi-event.h, qapi-event.c the same way.
qmp-introspect.h, qmp-introspect.c and qapi.texi remain monolithic.


Make sense.



The split of qmp-commands.c duplicates static helper function
qmp_marshal_output_str() in qapi-commands-char.c and
qapi-commands-misc.c.  This happens when commands returning the same
type occur in multiple modules.  Not worth avoiding.


As long as it is static, and neither .c file includes the other, we're 
fine (gdb may have a harder time figuring out which copy to put a 
breakpoint on, but that's not too terrible).




Since I'm going to rename qapi-event.[ch] to qapi-events.[ch], and
qmp-commands.[ch] to qapi-commands.[ch], name the shards that way
already, to reduce churn.  This requires temporary hacks in
commands.py and events.py.  They'll go away with the rename.


The planned rename makes sense; prepping now is fine.



Signed-off-by: Markus Armbruster 
---
  .gitignore   |  60 
  Makefile | 120 +++
  Makefile.objs|  65 -
  scripts/qapi/commands.py |  35 +-
  scripts/qapi/common.py   |  21 +++--
  scripts/qapi/events.py   |  19 ++--
  6 files changed, 300 insertions(+), 20 deletions(-)

diff --git a/.gitignore b/.gitignore
index 9477a08b6b..42c57998fd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,7 +31,67 @@
  /qapi-gen-timestamp
  /qapi-builtin-types.[ch]
  /qapi-builtin-visit.[ch]
+/qapi/qapi-commands-block-core.[ch]
+/qapi/qapi-commands-block.[ch]


Is it worth using a glob instead of specific patterns, as in:

/qapi/qapi-commands-*.[ch]

Maybe being precise doesn't hurt, if we aren't adding sub-modules all 
that often; but being generous with a glob makes it less likely that a 
future module addition will forget to update .gitignore.




+++ b/Makefile
@@ -92,10 +92,70 @@ include $(SRC_PATH)/rules.mak
  GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
  GENERATED_FILES += qapi-builtin-types.h qapi-builtin-types.c
  GENERATED_FILES += qapi-types.h qapi-types.c
+GENERATED_FILES += qapi/qapi-types-block-core.h qapi/qapi-types-block-core.c


For the makefile, I'd suggest using an intermediate list of module 
names, and then using make string operations to expand it into larger 
lists, to cut down on the repetition.  Something like (untested):


QAPI_MODULES = block-core block char common ...
GENERATED_FILES += $(addprefix qapi/qapi-types-,$(addsuffix 
.c,$(QAPI_MODULES))
GENERATED_FILES += $(addprefix qapi/qapi-types-,$(addsuffix 
.h,$(QAPI_MODULES))




@@ -525,10 +585,70 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
  
  qapi-builtin-types.c qapi-builtin-types.h \

  qapi-types.c qapi-types.h \
+qapi/qapi-types-block-core.c qapi/qapi-types-block-core.h \
+qapi/qapi-types-block.c qapi/qapi-types-block.h \


And repeating the list of filenames; again, an intermediate variable 
would let you do:


QAPI_GENERATED_FILES = ...
GENERATED_FILES += $(QAPI_GENERATED_FILES)

$(QAPI_GENERATED_FILES): ...


  qmp-introspect.h qmp-introspect.c \
  qapi-doc.texi: \
  qapi-gen-timestamp ;


Not only does it cut down on the repetition, but it will make adding a 
future module easier.



diff --git a/Makefile.objs b/Makefile.objs
index 2813e984fd..7a55d45669 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -3,8 +3,56 @@
  stub-obj-y = stubs/ crypto/
  util-obj-y = util/ qobject/ qapi/
  util-obj-y += qapi-builtin-types.o
+util-obj-y += qapi-types.o
+util-obj-y += qapi/qapi-types-block-core.o


Perhaps this can also exploit make text manipulation?

Otherwise looks good.

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

[Qemu-devel] [Bug 1749016] [NEW] VHDX BAT and Metadata Region Header Required Bit Not Set

[Michael Fruchtman]

Public bug reported:

When converting a VMDK to VHDX the resulting VHDX's Region table has a
small error. According to the VHDX specification the BAT and Metadata
entries for the region header required bit should be set to 1.  In a
VHDX created by qemu-img, this bit is not set.

See Table 4: Known Region Properties of the VHDX specification.

The structure format is as following from Structure 4: Region Table
Entry:

struct VHDX_REGION_TABLE_ENTRY {
GUID Guid;
UINT64 FileOffset;
UINT32 Length;
UINT32 Required:1;
UINT32 Reserved:31;
}

The Required bit for VHDX specified BAT and Metadata Regions Required
bit in the entry is not set as required in the current specification.

VHDX Region Table in a valid VHDX

Offset(h)00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
0x0003   72 65 67 69 AE 8C 6B C6 02 00 00 00 00 00 00 00
0x00030010   66 77 C2 2D 23 F6 00 42 9D 64 11 5E 9B FD 4A 08
0x00030020   00 00 30 00 00 00 00 00 00 00 10 00 01 00 00 00  
0x00030030   06 A2 7C 8B 90 47 9A 4B B8 FE 57 5F 05 0F 88 6E
0x00030040   00 00 20 00 00 00 00 00 00 00 10 00 01 00 00 00

VHDX Region Table in a VHDX converted by qemu-img from VMDK

Offset(h)00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
0x0003   72 65 67 69 AE 8C 6B C6 02 00 00 00 00 00 00 00
0x00030010   66 77 C2 2D 23 F6 00 42 9D 64 11 5E 9B FD 4A 08
0x00030020   00 00 30 00 00 00 00 00 00 00 10 00 00 00 00 00  
0x00030030   06 A2 7C 8B 90 47 9A 4B B8 FE 57 5F 05 0F 88 6E
0x00030040   00 00 20 00 00 00 00 00 00 00 10 00 00 00 00 00

The fist bit at 0x0003002A and 0x0003004A should be set to 1.

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: qemu-img vhdx

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

Title:
  VHDX BAT and Metadata Region Header Required Bit Not Set

Status in QEMU:
  New

Bug description:
  When converting a VMDK to VHDX the resulting VHDX's Region table has a
  small error. According to the VHDX specification the BAT and Metadata
  entries for the region header required bit should be set to 1.  In a
  VHDX created by qemu-img, this bit is not set.

  See Table 4: Known Region Properties of the VHDX specification.

  The structure format is as following from Structure 4: Region Table
  Entry:

  struct VHDX_REGION_TABLE_ENTRY {
  GUID Guid;
  UINT64 FileOffset;
  UINT32 Length;
  UINT32 Required:1;
  UINT32 Reserved:31;
  }

  The Required bit for VHDX specified BAT and Metadata Regions Required
  bit in the entry is not set as required in the current specification.

  VHDX Region Table in a valid VHDX

  Offset(h)00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
  0x0003   72 65 67 69 AE 8C 6B C6 02 00 00 00 00 00 00 00
  0x00030010   66 77 C2 2D 23 F6 00 42 9D 64 11 5E 9B FD 4A 08
  0x00030020   00 00 30 00 00 00 00 00 00 00 10 00 01 00 00 00  
  0x00030030   06 A2 7C 8B 90 47 9A 4B B8 FE 57 5F 05 0F 88 6E
  0x00030040   00 00 20 00 00 00 00 00 00 00 10 00 01 00 00 00

  VHDX Region Table in a VHDX converted by qemu-img from VMDK

  Offset(h)00 01 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F
  0x0003   72 65 67 69 AE 8C 6B C6 02 00 00 00 00 00 00 00
  0x00030010   66 77 C2 2D 23 F6 00 42 9D 64 11 5E 9B FD 4A 08
  0x00030020   00 00 30 00 00 00 00 00 00 00 10 00 00 00 00 00  
  0x00030030   06 A2 7C 8B 90 47 9A 4B B8 FE 57 5F 05 0F 88 6E
  0x00030040   00 00 20 00 00 00 00 00 00 00 10 00 00 00 00 00

  The fist bit at 0x0003002A and 0x0003004A should be set to 1.

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

Re: [Qemu-devel] [PATCH v2 20/29] qapi/types qapi/visit: Generate built-in stuff into separate files

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

Linking code from multiple separate QAPI schemata into the same
program is possible, but involves some weirdness around built-in
types:

* We generate code for built-in types into .c only with option
   --builtins.  The user is responsible for generating code for exactly
   one QAPI schema per program with --builtins.

* We generate code for built-in types into .h regardless of
   --builtins, but guarded by #ifndef QAPI_VISIT_BUILTIN.  Because all
   copies of this code are exactly the same, including any combination
   of these headers works.

Replace this contraption by something more conventional: generate code
for built-in types into their very own files: qapi-builtin-types.c,
qapi-builtin-visit.c, qapi-builtin-types.h, qapi-builtin-visit.h, but
only with --builtins.  Obey --output-dir, but ignore --prefix for
them.

Make qapi-types.h include qapi-builtin-types.h.  With multiple
schemata you now have multiple qapi-types.[ch], but only one
qapi-builtin-types.[ch].  Same for qapi-visit.[ch] and
qapi-builtin-visit.[ch].

Bonus: if all you need is built-in stuff, you can include a much
smaller header.  To be exploited shortly.

Signed-off-by: Markus Armbruster 
---
@@ -2046,6 +2046,7 @@ class QAPIGenH(QAPIGenC):
  
  
  class QAPIGenDoc(QAPIGen):

+
  def _top(self, fname):
  return (QAPIGen._top(self, fname)
  + '@c AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')


Does this hunk belong in an earlier patch?

Otherwise,
Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v8 05/28] target/i386: add memory encryption feature cpuid support

[Borislav Petkov]

On Mon, Feb 12, 2018 at 03:07:26PM -0600, Brijesh Singh wrote:
> In current implementation, when -cpu ...,+sev is passed without
> appropriate SEV configuration then we populate the Fn8000_001F CPUID but
> VMCB will not have SEV bit set hence a guest will be launched as
> non-SEV.

I think we should fail the guest init if sev is not really supported by
the host. Otherwise people might get mislead.

> > Changing existing CPU models is possible only on very specific
> > circumstances (where VMs that are currently runnable would always
> > stay runnable), and would require compat_props entries to keep
> > compatibility on existing machine-types.
> 
> Ah I didn't consider that case. What is recommendation, should we create
> a new CPU Model (EPYC-SEV) ?

Can we please stop creating a new CPU model with every new CPUID feature
support added? This is just ridiculous.

If this is about live migration, then by all means, fail the migration
if the feature bits are not compatible. But replicating CPU models and
then adding one new differing feature doesn't make any sense.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [Qemu-devel] [PATCH v8 05/28] target/i386: add memory encryption feature cpuid support

[Brijesh Singh]

On 2/12/18 12:38 PM, Eduardo Habkost wrote:
> On Mon, Feb 12, 2018 at 09:36:52AM -0600, Brijesh Singh wrote:
>> AMD EPYC processors support memory encryption feature. The feature
>> is reported through CPUID 8000_001F[EAX].
>>
>> Fn8000_001F [EAX]:
>>  Bit 0   Secure Memory Encryption (SME) supported
>>  Bit 1   Secure Encrypted Virtualization (SEV) supported
>>  Bit 2   Page flush MSR supported
>>  Bit 3   Ecrypted State (SEV-ES) support
>>
>> when memory encryption feature is reported, CPUID 8000_001F[EBX] should
>> provide additional information regarding the feature (such as which page
>> table bit is used to mark pages as encrypted etc). The information in EBX
>> and ECX may vary from one family to another hence we use the host cpuid
>> to populate the EBX information.
>>
>> The details for memory encryption CPUID is available in AMD APM
>> (https://support.amd.com/TechDocs/24594.pdf) Section E.4.17
>>
>> Cc: Paolo Bonzini 
>> Cc: Richard Henderson 
>> Cc: Eduardo Habkost 
>> Signed-off-by: Brijesh Singh 
>> ---
>>  target/i386/cpu.c | 36 
>>  target/i386/cpu.h |  3 +++
>>  2 files changed, 39 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index b5e431e769da..475d98a44880 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -235,6 +235,7 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t 
>> vendor1,
>>  #define TCG_EXT4_FEATURES 0
>>  #define TCG_SVM_FEATURES 0
>>  #define TCG_KVM_FEATURES 0
>> +#define TCG_MEM_ENCRYPT_FEATURES 0
>>  #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
>>CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX | \
>>CPUID_7_0_EBX_PCOMMIT | CPUID_7_0_EBX_CLFLUSHOPT |\
>> @@ -546,6 +547,20 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
>> = {
>>  .cpuid_reg = R_EDX,
>>  .tcg_features = ~0U,
>>  },
>> +[FEAT_MEM_ENCRYPT] = {
>> +.feat_names = {
>> +NULL, "sev", NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +NULL, NULL, NULL, NULL,
>> +},
>> +.cpuid_eax = 0x801F, .cpuid_reg = R_EAX,
>> +.tcg_features = TCG_MEM_ENCRYPT_FEATURES,
>> +}
> What should happen if "-cpu ...,+sev" is used without the
> appropriate SEV configuration on the command-line?
>

 If host supports SEV feature then we should communicate guest that SEV
feature is available but not enabled. At least that's what I am doing
today. Having said that, I am flexible to change if you all things
otherwise. We could abort the launch if SEV configuration is missing and
feature is enabled.

In current implementation, when -cpu ...,+sev is passed without
appropriate SEV configuration then we populate the Fn8000_001F CPUID but
VMCB will not have SEV bit set hence a guest will be launched as
non-SEV. The presence of Fn8000_001F provides the hint that HW supports
the SEV feature but does not mean that it is enabled. A guest OS need to
call MSR_AMD64_SEV (0xc001_0131) to determine if SEV is enabled. The
MSR_AMD64_SEV is non interceptable read-only MSR. This MSR is set by the
HW when SEV bit is enabled in VMCB. SEV bit in VMCB is enabled only when
qemu adds those extra SEV configuration (i.e KVM_SEV_INIT is success)

The steps used by guest OS to determine SEV active is documented here [1]

[1]
https://git.kernel.org/pub/scm/virt/kvm/kvm.git/tree/Documentation/x86/amd-memory-encryption.txt?h=linux-next#n57


>
>>  };
>>  
>>  typedef struct X86RegisterInfo32 {
>> @@ -1966,6 +1981,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
>>  CPUID_XSAVE_XGETBV1,
>>  .features[FEAT_6_EAX] =
>>  CPUID_6_EAX_ARAT,
>> +/* Missing: SEV_ES */
>> +.features[FEAT_MEM_ENCRYPT] =
>> +CPUID_8000_001F_EAX_SEV,
> Changing existing CPU models is possible only on very specific
> circumstances (where VMs that are currently runnable would always
> stay runnable), and would require compat_props entries to keep
> compatibility on existing machine-types.

Ah I didn't consider that case. What is recommendation, should we create
a new CPU Model (EPYC-SEV) ?

> I don't think this is one case where adding the feature will be
> safe (even if adding compat code).  What about existing VMs that
> are running on hosts that don't return SEV on KVM_GET_SUPPORTED_CPUID?
> "-cpu EPYC" would become not runnable on those hosts.
>
> (BTW, do you have a pointer to the KVM patches that enable SEV on
> KVM_GET_SUPPORTED_CPUID?  I couldn't find them.)
>

https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=linux-next=8765d75329a386dd7742f94a1ea5fdcdea8d93d0


>>  

Re: [Qemu-devel] [PATCH v2 19/29] qapi: Make code-generating visitors use QAPIGen more

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

The use of QAPIGen is rather shallow so far: most of the output
accumulation is not converted.  Take the next step: convert output
accumulation in the code-generating visitor classes.  Helper functions
outside these classes are not converted.

Signed-off-by: Markus Armbruster 
---
  scripts/qapi/commands.py   | 71 
  scripts/qapi/common.py | 13 
  scripts/qapi/doc.py| 74 --
  scripts/qapi/events.py | 55 ---
  scripts/qapi/introspect.py | 56 +---
  scripts/qapi/types.py  | 81 +++---
  scripts/qapi/visit.py  | 80 +++--
  7 files changed, 188 insertions(+), 242 deletions(-)



Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PULL 0/7] machine/CPU queue, 2018-02-12

[Eduardo Habkost]

On Mon, Feb 12, 2018 at 04:53:40PM -0200, Eduardo Habkost wrote:
> The following changes since commit 8e3fb8029efaf220ab48290cdb5151c682227030:
> 
>   Merge remote-tracking branch 'remotes/mjt/tags/trivial-patches-fetch' into 
> staging (2018-02-12 13:00:03 +)
> 
> are available in the Git repository at:
> 
>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
> 
> for you to fetch changes up to 71479144b8c743f0f2b7cdf831f65edc6931c3cf:
> 
>   cpu: drop unnecessary NULL check and cpu_common_class_by_name() (2018-02-12 
> 15:17:05 -0200)
> 
> 
> machine/CPU queue, 2018-02-12
> 
> 

Please ignore this.  I just noticed I ran testing on the wrong
branch (before the last rebase), and this conflicts with QAPI
changes merged last Friday.  Sorry for the noise.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 18/29] qapi: Rename generated qmp-marshal.c to qmp-commands.c

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

All generated .c are named like their .h, except for qmp-marshal.c and
qmp-commands.h.  To add to the confusion, tests-qmp-commands.c falsely
matches generated test-qmp-commands.h.

Get rid of this unnecessary complication.


Yay for saner naming.



Signed-off-by: Markus Armbruster 
---
  .gitignore |  3 +--
  Makefile   |  6 +++---
  Makefile.objs  |  2 +-
  docs/devel/qapi-code-gen.txt   |  6 +++---
  qga/Makefile.objs  |  2 +-
  scripts/qapi/commands.py   |  2 +-
  tests/.gitignore   |  5 ++---
  tests/Makefile.include | 10 +-
  tests/{test-qmp-commands.c => test-qmp-cmds.c} |  0
  9 files changed, 17 insertions(+), 19 deletions(-)
  rename tests/{test-qmp-commands.c => test-qmp-cmds.c} (100%)




diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-cmds.c
similarity index 100%
rename from tests/test-qmp-commands.c
rename to tests/test-qmp-cmds.c



Noticed while renaming this file - it's non-trivial in length, but bears 
no copyright or license clause.  Perhaps we should add that (but can be 
separate patch).


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 4/4] acpi: build TPM Physical Presence interface

[Stefan Berger]

On 02/12/2018 03:46 PM, Kevin O'Connor wrote:

On Fri, Feb 09, 2018 at 03:19:31PM -0500, Stefan Berger wrote:

The PPI device in this patch series allocates 0x400 bytes. 0x200 bytes are
used by the OperationRegion() in this patch series. The rest was thought of
for future extensions.

To allow both firmwares to use PPI, we would need to be able to have the
OperationRegion() be flexible and located at 0x  for EDK2 and 0xFED4
5000 for SeaBIOS, per choice of the firmware. One way to achieve this would
be to have the firmwares choose their operation region base address by
writing into the PPI memory device at offset 0x200 (for example). A '1'
there would mean that we want the OperationRegion() at 0x , and a
'2' would mean at the base address of the PPI device (0xFED4 5000). This
could be achieved by declaring a 2nd OperationRegion() in the ACPI code that
is located at 0xFED4 5200 and we declare that 1st OperationRegion()'s
address based on findings from there. Further, a flags variable at offset
0x201 could indicate whether an SMI is needed to enter SMM or not.
Obviously, the ACPI code would become more complex to be able to support
both firmwares at the same time.

This is a lot of details but I rather post this description before posting
more patches. To make these changes and get it to work with at least SeaBIOS
is probably fairly easy. But is this acceptable?

I'm not sure I fully understand the goals of the PPI interface.
Here's what I understand so far:

The TPM specs define some actions that are considered privileged.  An
example of this would be disabling the TPM itself.  In order to
prevent an attacker from performing these actions without
authorization, the TPM specs define a mechanism to assert "physical
presence" before the privileged action can be done.  They do this by
having the firmware present a menu during early boot that permits
these privileged operations, and then the firmware locks the TPM chip
so the actions can no longer be done by any software that runs after
the firmware.  Thus "physical presence" is asserted by demonstrating
one has console access to the machine during early boot.

The PPI spec implements a work around for this - presumably some found
the enforcement mechanism too onerous.  It allows the OS to provide a
request code to the firmware, and on the next boot the firmware will
take the requested action before it locks the chip.  Thus allowing the
OS to indirectly perform the privileged action even after the chip has
been locked.  Thus, the PPI system seems to be an "elaborate hack" to
allow users to circumvent the physical presence mechanism (if they
choose to).


Correct.


Here's what I understand the proposed implementation involves:

1 - in addition to emulating the TPM device itself, QEMU will also
 introduce a virtual memory device with 0x400 bytes.

Correct.


2 - on first boot the firmware (seabios and uefi) will populate the
 memory region created in step 1.  In particular it will fill an
 array with the list of request codes it supports.  (Each request
 is an 8bit value, the array has 256 entries.)
Correct. Each firmware would fill out the 256 byte array depending on 
what it supports. The 8 bit values are basically flags and so on.

3 - QEMU will produce AML code implementing the standard PPI ACPI
 interface.  This AML code will take the request, find the table
 produced in step 1, compare it to the list of accepted requests
 produced in step 2, and then place the 8bit request in another
 qemu virtual memory device (at 0x or 0xFED45000).


Correct.

Now EDK2 wants to store the code in a UEFI variable in NVRAM. We 
therefore would need to trigger an SMI. In SeaBIOS we wouldn't have to 
do this.



4 - the OS will signal a reboot, qemu will do its normal reboot logic,
 and the firmware will be run again.

5 - the firmware will extract the code written in stage 3, and if the
 tpm device has been configured to accept PPI codes from the OS, it
 will invoke the requested action.


SeaBIOS would look into memory to find the code. EDK2 will read the code 
from a UEFI variable.



Did I understand the above correctly?

I think so. With the fine differences between SeaBIOS and EDK2 pointed out.


Separately, is there someone clamoring for PPI support today?  That
is, is the goal to implement PPI to make QEMU more spec compliant, or
is there someone struggling to perform a particular task today that
this support would improve?


We could defer the implementation of this. My main goal was to to 
support TPM migration in QEMU and the PPI device also needs to be 
migrated as part of TPM migration. So that's why I am looking at PPI now.


Stefan

Thanks,
-Kevin

Re: [Qemu-devel] [PATCH v2 4/4] acpi: build TPM Physical Presence interface

[Kevin O'Connor]

On Fri, Feb 09, 2018 at 03:19:31PM -0500, Stefan Berger wrote:
> The PPI device in this patch series allocates 0x400 bytes. 0x200 bytes are
> used by the OperationRegion() in this patch series. The rest was thought of
> for future extensions.
>
> To allow both firmwares to use PPI, we would need to be able to have the
> OperationRegion() be flexible and located at 0x  for EDK2 and 0xFED4
> 5000 for SeaBIOS, per choice of the firmware. One way to achieve this would
> be to have the firmwares choose their operation region base address by
> writing into the PPI memory device at offset 0x200 (for example). A '1'
> there would mean that we want the OperationRegion() at 0x , and a
> '2' would mean at the base address of the PPI device (0xFED4 5000). This
> could be achieved by declaring a 2nd OperationRegion() in the ACPI code that
> is located at 0xFED4 5200 and we declare that 1st OperationRegion()'s
> address based on findings from there. Further, a flags variable at offset
> 0x201 could indicate whether an SMI is needed to enter SMM or not.
> Obviously, the ACPI code would become more complex to be able to support
> both firmwares at the same time.
>
> This is a lot of details but I rather post this description before posting
> more patches. To make these changes and get it to work with at least SeaBIOS
> is probably fairly easy. But is this acceptable?

I'm not sure I fully understand the goals of the PPI interface.
Here's what I understand so far:

The TPM specs define some actions that are considered privileged.  An
example of this would be disabling the TPM itself.  In order to
prevent an attacker from performing these actions without
authorization, the TPM specs define a mechanism to assert "physical
presence" before the privileged action can be done.  They do this by
having the firmware present a menu during early boot that permits
these privileged operations, and then the firmware locks the TPM chip
so the actions can no longer be done by any software that runs after
the firmware.  Thus "physical presence" is asserted by demonstrating
one has console access to the machine during early boot.

The PPI spec implements a work around for this - presumably some found
the enforcement mechanism too onerous.  It allows the OS to provide a
request code to the firmware, and on the next boot the firmware will
take the requested action before it locks the chip.  Thus allowing the
OS to indirectly perform the privileged action even after the chip has
been locked.  Thus, the PPI system seems to be an "elaborate hack" to
allow users to circumvent the physical presence mechanism (if they
choose to).

Here's what I understand the proposed implementation involves:

1 - in addition to emulating the TPM device itself, QEMU will also
introduce a virtual memory device with 0x400 bytes.

2 - on first boot the firmware (seabios and uefi) will populate the
memory region created in step 1.  In particular it will fill an
array with the list of request codes it supports.  (Each request
is an 8bit value, the array has 256 entries.)

3 - QEMU will produce AML code implementing the standard PPI ACPI
interface.  This AML code will take the request, find the table
produced in step 1, compare it to the list of accepted requests
produced in step 2, and then place the 8bit request in another
qemu virtual memory device (at 0x or 0xFED45000).

4 - the OS will signal a reboot, qemu will do its normal reboot logic,
and the firmware will be run again.

5 - the firmware will extract the code written in stage 3, and if the
tpm device has been configured to accept PPI codes from the OS, it
will invoke the requested action.

Did I understand the above correctly?

Separately, is there someone clamoring for PPI support today?  That
is, is the goal to implement PPI to make QEMU more spec compliant, or
is there someone struggling to perform a particular task today that
this support would improve?

Thanks,
-Kevin

Re: [Qemu-devel] [PATCH 2/3] qmp: add query-cpus-fast

[David Hildenbrand]

On 12.02.2018 13:14, Viktor Mihajlovski wrote:
> From: Luiz Capitulino 
> 
> The query-cpus command has an extremely serious side effect:
> it always interrupts all running vCPUs so that they can run
> ioctl calls. This can cause a huge performance degradation for
> some workloads. And most of the information retrieved by the
> ioctl calls are not even used by query-cpus.
> 
> This commit introduces a replacement for query-cpus called
> query-cpus-fast, which has the following features:
> 
>  o Never interrupt vCPUs threads. query-cpus-fast only returns
>vCPU information maintained by QEMU itself, which should be
>sufficient for most management software needs
> 
>  o Make "halted" field optional: we only return it if the
>halted state is maintained by QEMU. But this also gives
>the option of dropping the field in the future (see below)
> 

If I'm not wrong, this comment is superseded by ...

>  o Drop irrelevant fields such as "current", "pc" and "arch"
> 
>  o Drop field "halted" since it can't be provided fast reliably
>and is too volatile on most architectures to be really useful
> 

this comment :)

>  o Rename some fields for better clarification & proper naming
>standard>
> Signed-off-by: Luiz Capitulino 
> Signed-off-by: Viktor Mihajlovski 

Wondering if we could tweak the old interface with a simple flag "fast =
true".


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v2 17/29] qapi: Record 'include' directives in intermediate representation

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

The include directive permits modular QAPI schemata, but the generated
code is monolithic all the same.  To permit generating modular code,
the front end needs to pass more information on inclusions to the back
ends.  The commit before last added the necessary information to the
parse tree.  This commit adds it to the intermediate representation
and its QAPISchemaVisitor.  A later commit will use this to to
generate modular code.

New entity QAPISchemaInclude represents inclusions.  Call new visitor
method visit_include() for it, so visitors can see the sub-modules a
module includes.

Note that unlike other entities, QAPISchemaInclude has no name, and is
therefore not added to entity_dict.

New QAPISchemaEntity attribute @module names the entity's source file.
Call new visitor method visit_module() when it changes during a visit,
so visitors can keep track of the module being visited.

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
---
@@ -1479,16 +1497,19 @@ class QAPISchema(object):
  self._entity_dict = {}
  self._predefining = True
  self._def_predefineds()
-self._predefining = False
  self._def_exprs(exprs)
  self.check()


Why does self._predfining not need to be toggled anymore?  Do we even 
need this variable any more...




 def _def_entity(self, ent):
 # Only the predefined types are allowed to not have info
 assert ent.info or self._predefining
-assert ent.name not in self._entity_dict


...and/or is this assert now worthless?



+++ b/tests/qapi-schema/comments.out
@@ -1,4 +1,5 @@
  object q_empty
  enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
  prefix QTYPE
+module comments.json
  enum Status ['good', 'bad', 'ugly']


Based on the generated output, it looks like you can tell whether you 
are in the predefining stage by not having any module at all; the first 
visit_module call is what flips the switch that everything else is 
defined by a module and must therefore have associated info.


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

Re: [Qemu-devel] [PATCH 1/3] qmp: expose s390-specific CPU info

[David Hildenbrand]

On 12.02.2018 13:14, Viktor Mihajlovski wrote:
> Presently s390x is the only architecture not exposing specific
> CPU information via QMP query-cpus. Upstream discussion has shown
> that it could make sense to report the architecture specific CPU
> state, e.g. to detect that a CPU has been stopped.
> 
> With this change the output of query-cpus will look like this on
> s390:
> 
>[
>  {"arch": "s390", "current": true,
>   "props": {"core-id": 0}, "cpu-state": "operating", "CPU": 0,
>   "qom_path": "/machine/unattached/device[0]",
>   "halted": false, "thread_id": 63115},
>  {"arch": "s390", "current": false,
>   "props": {"core-id": 1}, "cpu-state": "stopped", "CPU": 1,
>   "qom_path": "/machine/unattached/device[1]",
>   "halted": true, "thread_id": 63116}
>]
> 
> Signed-off-by: Viktor Mihajlovski 
> Acked-by: Eric Blake 
> ---
>  cpus.c |  6 ++
>  hmp.c  |  4 
>  hw/intc/s390_flic.c|  4 ++--
>  hw/s390x/s390-virtio-ccw.c |  2 +-
>  qapi-schema.json   | 28 +++-
>  target/s390x/cpu.c | 24 
>  target/s390x/cpu.h |  7 ++-
>  target/s390x/kvm.c |  8 
>  target/s390x/sigp.c| 38 +++---
>  9 files changed, 77 insertions(+), 44 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index f298b65..6006931 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2100,6 +2100,9 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>  #elif defined(TARGET_TRICORE)
>  TriCoreCPU *tricore_cpu = TRICORE_CPU(cpu);
>  CPUTriCoreState *env = _cpu->env;
> +#elif defined(TARGET_S390X)
> +S390CPU *s390_cpu = S390_CPU(cpu);
> +CPUS390XState *env = _cpu->env;
>  #endif
>  
>  cpu_synchronize_state(cpu);
> @@ -2127,6 +2130,9 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>  #elif defined(TARGET_TRICORE)
>  info->value->arch = CPU_INFO_ARCH_TRICORE;
>  info->value->u.tricore.PC = env->PC;
> +#elif defined(TARGET_S390X)
> +info->value->arch = CPU_INFO_ARCH_S390;
> +info->value->u.s390.cpu_state = env->cpu_state;
>  #else
>  info->value->arch = CPU_INFO_ARCH_OTHER;
>  #endif
> diff --git a/hmp.c b/hmp.c
> index 7870d6a..a6b94b7 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -392,6 +392,10 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>  case CPU_INFO_ARCH_TRICORE:
>  monitor_printf(mon, " PC=0x%016" PRIx64, 
> cpu->value->u.tricore.PC);
>  break;
> +case CPU_INFO_ARCH_S390:
> +monitor_printf(mon, " state=%s",
> +   CpuS390State_str(cpu->value->u.s390.cpu_state));
> +break;
>  default:
>  break;
>  }
> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index a85a149..5f8168f 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -192,8 +192,8 @@ static void qemu_s390_flic_notify(uint32_t type)
>  cs->interrupt_request |= CPU_INTERRUPT_HARD;
>  
>  /* ignore CPUs that are not sleeping */
> -if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING &&
> -s390_cpu_get_state(cpu) != CPU_STATE_LOAD) {
> +if (s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING &&
> +s390_cpu_get_state(cpu) != S390_CPU_STATE_LOAD) {
>  continue;
>  }
>  
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4abbe89..4d0c3de 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -368,7 +368,7 @@ static void s390_machine_reset(void)
>  
>  /* all cpus are stopped - configure and start the ipl cpu only */
>  s390_ipl_prepare_cpu(ipl_cpu);
> -s390_cpu_set_state(CPU_STATE_OPERATING, ipl_cpu);
> +s390_cpu_set_state(S390_CPU_STATE_OPERATING, ipl_cpu);
>  }
>  
>  static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5c06745..66e0927 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -410,10 +410,12 @@
>  # An enumeration of cpu types that enable additional information during
>  # @query-cpus.
>  #
> +# @s390: since 2.12
> +#
>  # Since: 2.6
>  ##
>  { 'enum': 'CpuInfoArch',
> -  'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 'other' ] }
> +  'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'other' ] }
>  
>  ##
>  # @CpuInfo:
> @@ -452,6 +454,7 @@
>  'ppc': 'CpuInfoPPC',
>  'mips': 'CpuInfoMIPS',
>  'tricore': 'CpuInfoTricore',
> +'s390': 'CpuInfoS390',
>  'other': 'CpuInfoOther' } }
>  
>  ##
> @@ -522,6 +525,29 @@
>  { 'struct': 'CpuInfoOther', 'data': { } }
>  
>  ##
> +# @CpuS390State:
> +#
> +# An enumeration of cpu states that can be assumed by a virtual
> +# S390 CPU
> +#
> +# Since: 2.12
> +##
> +{ 'enum': 

Re: [Qemu-devel] [PATCH v3] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code

[Eric Blake]

On 02/12/2018 12:48 PM, Daniel P. Berrangé wrote:

From: "Daniel P. Berrange" 

qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
done and thus getchar() will never return the EOF constant. Instead we have to
query the TTY attributes to determine the configured EOF character (usually
Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
regression that prevented Ctrl-D from triggering an exit of qemu-io that has
existed since readline was first added in

   commit 0cf17e181798063c3824c8200ba46f25f54faa1a
   Author: Stefan Hajnoczi 
   Date:   Thu Nov 14 11:54:17 2013 +0100

 qemu-io: use readline.c

It also ensures that a newline is printed when exiting, to complete the
line output by the "qemu-io> " prompt.

Signed-off-by: Daniel P. Berrange 
---


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v3] tests/migration: Add source to PC boot block

[Eric Blake]

On 02/12/2018 12:34 PM, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

The boot block used in the migration test is currently only
shipped as a hex (with the source in the git commit message),


Would be nice to point to commit ea0c6d62 (I assume that's the commit 
message you're referring to).



change this to actually include the source.


Yeah, GPL really wants us to ship the preferred editing form of sources ;)



A script is added to rebuild the header but the expectation is that
the generated hex is shipped as well as the .s, so that
there's no requirement to have just the right assembler etc.

Signed-off-by: Dr. David Alan Gilbert 
---



+++ b/tests/migration-test.c
@@ -80,57 +80,13 @@ static const char *tmpfs;
  /* A simple PC boot sector that modifies memory (1-100MB) quickly
   * outputing a 'B' every so often if it's still running.


Pre-existing, but while here,
s/outputing/outputting/


diff --git a/tests/migration/rebuild-x86-bootblock.sh 
b/tests/migration/rebuild-x86-bootblock.sh
new file mode 100755
index 00..ee9b53ceb4
--- /dev/null
+++ b/tests/migration/rebuild-x86-bootblock.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+# Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# Author: dgilb...@redhat.com
+
+ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
+HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
+
+if [ ! -e "$ASMFILE" ]
+then
+  echo "Couldn't find $ASMFILE" >&2
+  exit 1
+fi
+
+ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.X)

Portable use of mktemp requires at least 6 X, not 5.


+cd $ASM_WORK_DIR &&


Unsafe if $PWD contains spaces; needs to be quoted.


+as --32 -march=i486 "$ASMFILE" -o x86.o &&
+objcopy -O binary x86.o x86.boot &&
+dd if=x86.boot of=x86.bootsect \
+  bs=256 count=2 skip=124 &&
+xxd -i x86.bootsect |
+sed -e 's/.*int.*//' > x86.hex &&
+cat - x86.hex < "$HEADER"
+/* This file is automatically generated from
+ * tests/migration/x86-a-b-bootblock.s, edit that and then run
+ * tests/migration/rebuild-x86-bootblock.sh to update,
+ * and then remember to send both in your patch submission.
+ */
+HERE
+
+rm x86.hex x86.bootsect x86.boot x86.o
+cd .. && rmdir $ASM_WORK_DIR


Another place that needs quoting.


+++ b/tests/migration/x86-a-b-bootblock.s
@@ -0,0 +1,92 @@
+# x86 bootblock used in migration test
+#  repeatedly increments the first byte of each page in a 100MB
+#  range.
+#  Outputs an initial 'A' on serial followed by repeated 'B's
+#
+# run   tests/migration/rebuild-x86-bootblock.sh
+#   to regenerate the hex, and remember to include both the .h and .s
+#   in any patches.
+#
+# Copyright (c) 2016 Red Hat, Inc. and/or its affiliates


Do you want to add 2018, since you've now modified things since the 
original commit?


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

Re: [Qemu-devel] [PATCH v2 4/4] acpi: build TPM Physical Presence interface

[Stefan Berger]

On 02/12/2018 02:45 PM, Kevin O'Connor wrote:

On Fri, Feb 09, 2018 at 03:19:31PM -0500, Stefan Berger wrote:

I have played around with this patch and some modifications to EDK2. Though
for EDK2 the question is whether to try to circumvent their current
implementation that uses SMM or use SMM. With this patch so far I circumvent
it, which is maybe not a good idea.

The facts for EDK2's PPI:

- from within the OS a PPI code is submitted to ACPI and ACPI enters SMM via
an SMI and the PPI code is written into a UEFI variable. For this ACPI uses
the memory are at 0x  to pass parameters from the OS (via ACPI) to
SMM. This is declared in ACPI with an OperationRegion() at that address.
Once the machine is rebooted, UEFI reads the variable and finds the PPI code
and reacts to it.

I'm a bit confused by this.  The top 1M of the first 4G of ram is
generally mapped to the flash device on real machines.  Indeed, this
is part of the mechanism used to boot an X86 machine - it starts
execution from flash at 0xfff0.  This is true even on modern
machines.

So, it seems strange that UEFI is pushing a code through a memory
device at 0x.  I can't see how that would be portable.  Are
you sure the memory write to 0x is not just a trigger to
invoke the SMI?


I base this on the code here:

https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl#L81

OperationRegion (TNVS, SystemMemory, 0x, 0xF0)



It sounds as if the ultimate TPM interface that must be supported is
the ACPI DSDT methods.  Since you're crafting the DSDT table yourself,
why does there need to be two different backends - why can't the same
mechanism be used for both SeaBIOS and UEFI?  Is this because you are
looking to reuse TPM code already in UEFI that requires a specific
interface?


UEFI uses SMM to write the PPI code into a UEFI variable that it then 
presumably stores back to NVRAM. With SeaBIOS I would go the path of 
having the ACPI code write the code in the OperationRegion() and leave 
it at that, so not invoke SMM. EDK2 also reads the result of the 
operation from a register that SMM uses to pass a return value through. 
We wouldn't need that for SeaBIOS.


   Stefan



-Kevin

Re: [Qemu-devel] [PATCH v2 16/29] qapi: Generate in source order

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

The generators' conversion to visitors (merge commit 9e72681d16)
changed the processing order of entities from source order to
alphabetical order.  The next commit needs source order, so change it
back.

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
---


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 15/29] qapi: Record 'include' directives in parse tree

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

The parse tree is a list of expressions.  Except include expressions
currently get replaced by the included file's parse tree.

Instead of throwing away the include expression, keep it with the file
name expanded so you don't have to track the including file's
directory to make sense of it.

A future commit will put this include expression to use.

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
---
  scripts/qapi/common.py | 21 +
  1 file changed, 17 insertions(+), 4 deletions(-)



Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PULL 0/7] machine/CPU queue, 2018-02-12

[no-reply]

Hi,

This series failed docker-quick@centos6 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180212185347.8433-1-ehabk...@redhat.com
Subject: [Qemu-devel] [PULL 0/7] machine/CPU queue, 2018-02-12

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7cc38e0302 cpu: drop unnecessary NULL check and cpu_common_class_by_name()
12a50bbdc5 cpu: get rid of unused cpu_init() defines
8504aaa9e2 Use cpu_create(type) instead of cpu_init(cpu_model)
241c2eec2c cpu: add CPU_RESOLVING_TYPE macro
94262332d1 tests: add machine 'none' with -cpu test
872df060a5 nios2: 10m50_devboard: replace cpu_model with cpu_type
27d4d37cb1 pc: correct misspelled CPU model-id for pc 2.2

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-ff3p7ua7/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   centos6
  GEN 
/var/tmp/patchew-tester-tmp-ff3p7ua7/src/docker-src.2018-02-12-14.43.15.24982/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-ff3p7ua7/src/docker-src.2018-02-12-14.43.15.24982/qemu.tar.vroot'...
done.
Checking out files:  45% (2635/5810)   
Checking out files:  46% (2673/5810)   
Checking out files:  47% (2731/5810)   
Checking out files:  48% (2789/5810)   
Checking out files:  49% (2847/5810)   
Checking out files:  50% (2905/5810)   
Checking out files:  51% (2964/5810)   
Checking out files:  52% (3022/5810)   
Checking out files:  53% (3080/5810)   
Checking out files:  54% (3138/5810)   
Checking out files:  55% (3196/5810)   
Checking out files:  56% (3254/5810)   
Checking out files:  57% (3312/5810)   
Checking out files:  58% (3370/5810)   
Checking out files:  59% (3428/5810)   
Checking out files:  60% (3486/5810)   
Checking out files:  61% (3545/5810)   
Checking out files:  62% (3603/5810)   
Checking out files:  63% (3661/5810)   
Checking out files:  64% (3719/5810)   
Checking out files:  65% (3777/5810)   
Checking out files:  66% (3835/5810)   
Checking out files:  67% (3893/5810)   
Checking out files:  68% (3951/5810)   
Checking out files:  69% (4009/5810)   
Checking out files:  70% (4067/5810)   
Checking out files:  71% (4126/5810)   
Checking out files:  72% (4184/5810)   
Checking out files:  73% (4242/5810)   
Checking out files:  74% (4300/5810)   
Checking out files:  75% (4358/5810)   
Checking out files:  76% (4416/5810)   
Checking out files:  77% (4474/5810)   
Checking out files:  78% (4532/5810)   
Checking out files:  79% (4590/5810)   
Checking out files:  80% (4648/5810)   
Checking out files:  81% (4707/5810)   
Checking out files:  82% (4765/5810)   
Checking out files:  83% (4823/5810)   
Checking out files:  84% (4881/5810)   
Checking out files:  85% (4939/5810)   
Checking out files:  86% (4997/5810)   
Checking out files:  87% (5055/5810)   
Checking out files:  88% (5113/5810)   
Checking out files:  89% (5171/5810)   
Checking out files:  90% (5229/5810)   
Checking out files:  91% (5288/5810)   
Checking out files:  92% (5346/5810)   
Checking out files:  93% (5404/5810)   
Checking out files:  93% (5455/5810)   
Checking out files:  94% (5462/5810)   
Checking out files:  95% (5520/5810)   
Checking out files:  96% (5578/5810)   
Checking out files:  97% (5636/5810)   
Checking out files:  98% (5694/5810)   
Checking out files:  99% (5752/5810)   
Checking out files: 100% (5810/5810)   
Checking out files: 100% (5810/5810), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-ff3p7ua7/src/docker-src.2018-02-12-14.43.15.24982/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-ff3p7ua7/src/docker-src.2018-02-12-14.43.15.24982/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64

Re: [Qemu-devel] [PATCH v2 14/29] qapi: Concentrate QAPISchemaParser.exprs updates in .__init__()

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
---
  scripts/qapi/common.py | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)


Again, not much mention of 'why' in the commit message.  But 
centralizing the initialization operations in one place rather than 
having to chase down multiple places is good.


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 13/29] qapi: Lift error reporting from QAPISchema.__init__() to callers

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
---
  scripts/qapi-gen.py|  8 ++--
  scripts/qapi/common.py | 23 +--
  tests/qapi-schema/test-qapi.py | 10 --
  3 files changed, 23 insertions(+), 18 deletions(-)


The commit message didn't say why, but the change is reasonable.

Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 10/29] qapi: Touch generated files only when they change

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

A massive number of objects depends on QAPI-generated headers.  In my
"build everything" tree, it's roughly 4800 out of 5100.  This is
particularly annoying when only some of the generated files change,
say for a doc fix.

Improve qapi-gen.py to touch its output files only if they actually
change.  Rebuild time for a QAPI doc fix drops from many minutes to a
few seconds.  Rebuilds get faster for certain code changes, too.  For
instance, adding a simple QMP event now recompiles less than 200
instead of 4800 objects.  But adding a QAPI type is as bad as ever;
we've clearly got more work to do.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
  scripts/qapi/common.py | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 8290795dc1..2e58573a39 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1951,9 +1951,16 @@ class QAPIGen(object):
  except os.error as e:
  if e.errno != errno.EEXIST:
  raise
-f = open(os.path.join(output_dir, fname), 'w')
-f.write(self._top(fname) + self._preamble + self._body
+fd = os.open(os.path.join(output_dir, fname),
+ os.O_RDWR | os.O_CREAT, 0666)


patchew complained here for mingw; I'm not sure why.

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

Re: [Qemu-devel] [PATCH v2 4/4] acpi: build TPM Physical Presence interface

[Kevin O'Connor]

On Fri, Feb 09, 2018 at 03:19:31PM -0500, Stefan Berger wrote:
> I have played around with this patch and some modifications to EDK2. Though
> for EDK2 the question is whether to try to circumvent their current
> implementation that uses SMM or use SMM. With this patch so far I circumvent
> it, which is maybe not a good idea.
> 
> The facts for EDK2's PPI:
> 
> - from within the OS a PPI code is submitted to ACPI and ACPI enters SMM via
> an SMI and the PPI code is written into a UEFI variable. For this ACPI uses
> the memory are at 0x  to pass parameters from the OS (via ACPI) to
> SMM. This is declared in ACPI with an OperationRegion() at that address.
> Once the machine is rebooted, UEFI reads the variable and finds the PPI code
> and reacts to it.

I'm a bit confused by this.  The top 1M of the first 4G of ram is
generally mapped to the flash device on real machines.  Indeed, this
is part of the mechanism used to boot an X86 machine - it starts
execution from flash at 0xfff0.  This is true even on modern
machines.

So, it seems strange that UEFI is pushing a code through a memory
device at 0x.  I can't see how that would be portable.  Are
you sure the memory write to 0x is not just a trigger to
invoke the SMI?

It sounds as if the ultimate TPM interface that must be supported is
the ACPI DSDT methods.  Since you're crafting the DSDT table yourself,
why does there need to be two different backends - why can't the same
mechanism be used for both SeaBIOS and UEFI?  Is this because you are
looking to reuse TPM code already in UEFI that requires a specific
interface?

-Kevin

Re: [Qemu-devel] [PATCH v2 09/29] qapi-gen: Convert from getopt to argparse

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

argparse is nicer to use than getopt, and gives us --help almost for
free.

Signed-off-by: Markus Armbruster 
---
  scripts/qapi-gen.py| 48 ++--
  scripts/qapi/common.py | 43 ---
  2 files changed, 30 insertions(+), 61 deletions(-)



More functionality in fewer lines of code - always a win ;)

Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 08/29] qapi-gen: New common driver for code and doc generators

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

Whenever qapi-schema.json changes, we run six programs eleven times to
update eleven files.  Similar for qga/qapi-schema.json.  This is
silly.  Replace the six programs by a single program that spits out
all eleven files.

The programs become modules in new Python package qapi, along with the
helper library.  This requires moving them to scripts/qapi/.

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
---
  .gitignore |  2 +
  Makefile   | 86 +--
  docs/devel/qapi-code-gen.txt   | 97 ++
  monitor.c  |  2 +-
  qapi-schema.json   |  2 +-
  scripts/qapi-gen.py| 41 +
  scripts/qapi/__init__.py   |  0
  scripts/{qapi-commands.py => qapi/commands.py} | 23 ++---
  scripts/{qapi.py => qapi/common.py}| 18 +---
  scripts/{qapi2texi.py => qapi/doc.py}  | 29 ++-
  scripts/{qapi-event.py => qapi/events.py}  | 23 ++---
  scripts/{qapi-introspect.py => qapi/introspect.py} | 32 ++-
  scripts/{qapi-types.py => qapi/types.py}   | 34 ++--
  scripts/{qapi-visit.py => qapi/visit.py}   | 34 ++--
  tests/Makefile.include | 56 ++---
  tests/qapi-schema/test-qapi.py |  4 +-
  16 files changed, 193 insertions(+), 290 deletions(-)
  create mode 100755 scripts/qapi-gen.py
  create mode 100644 scripts/qapi/__init__.py
  rename scripts/{qapi-commands.py => qapi/commands.py} (94%)
  rename scripts/{qapi.py => qapi/common.py} (99%)
  rename scripts/{qapi2texi.py => qapi/doc.py} (92%)
  mode change 100755 => 100644


Still forgot mention that the mode bit change was intentional, but not 
worth a respin for just that.



  rename scripts/{qapi-event.py => qapi/events.py} (92%)
  rename scripts/{qapi-introspect.py => qapi/introspect.py} (90%)
  rename scripts/{qapi-types.py => qapi/types.py} (90%)
  rename scripts/{qapi-visit.py => qapi/visit.py} (92%)


Reviewed-by: Eric Blake 


+++ b/docs/devel/qapi-code-gen.txt



-$ python scripts/qapi-event.py --output-dir="qapi-generated"
---prefix="example-" example-schema.json
  $ cat qapi-generated/example-qapi-event.h
  [Uninteresting stuff omitted...]
  
@@ -1302,23 +1296,22 @@ Example:

  }
  
  const char *const example_QAPIEvent_lookup[] = {

-[EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
+
+[EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
  [EXAMPLE_QAPI_EVENT__MAX] = NULL,
  };


Looks like our generated code indentation has slightly regressed from 
what we would write by hand, but it's still okay for generated code (and 
the commit message in 5/29 did call that out)



+++ b/scripts/qapi-gen.py



+++ b/scripts/qapi/commands.py
@@ -13,7 +13,7 @@ This work is licensed under the terms of the GNU GPL, version 
2.
  See the COPYING file in the top-level directory.
  """
  
-from qapi import *

+from qapi.common import *
  
  
  def gen_command_decl(name, arg_type, boxed, ret_type):

@@ -255,13 +255,8 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
  self._regy += gen_register_command(name, success_response)
  
  
-def main(argv):

-(input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line()
-
-blurb = '''
- * Schema-defined QAPI/QMP commands
-'''
-
+def gen_commands(schema, output_dir, prefix):
+blurb = ' * Schema-defined QAPI/QMP commands'


We discussed whether to make the assignment to blurb be a one-liner in 
an earlier patch, but I'm also fine with the churn you have over the 
course of the series.



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

Re: [Qemu-devel] [PATCH v2 03/29] qapi: Generate up-to-date copyright notice

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

Each generator carries a copyright notice for the generator itself,
and another one for the files it generates.  Only the former have been
updated along the way, the latter have not, and are all out of date.

Fix by copying the generator's copyright notice to the generated files
instead.  Note that the fix doesn't copy the "Authors:" part; the
generated files' outdated Authors list goes away without replacement.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Marc-André Lureau 
---



+++ b/scripts/qapi-commands.py



@@ -257,16 +258,11 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
  
  blurb = '''

   * Schema-defined QAPI/QMP commands
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- *  Anthony Liguori   
  '''


This is where we could shorten blurb to a one-line assignment instead of 
a multiline '''...''' string, if desired.  R-b stands either way, though.


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

Re: [Qemu-devel] [PATCH v3 1/1] dump.c: allow fd_write_vmcore to return errno on failure

[Murilo Opsfelder Araujo]

On 02/12/2018 03:31 PM, Eric Blake wrote:
> On 02/12/2018 08:46 AM, Murilo Opsfelder Araujo wrote:
>> On 02/12/2018 12:25 PM, Daniel Henrique Barboza wrote:
>>> From: Yasmin Beatriz 
>>>
>>> fd_write_vmcore can fail to execute for a lot of reasons that can be
>>> retrieved by errno, but it only returns -1. This makes difficult for
>>> the caller to know what happened and only a generic error message is
>>> propagated back to the user. This is an example using dump-guest-memory:
>>>
> 
>>> +++ b/dump.c
>>> @@ -107,7 +107,7 @@ static int fd_write_vmcore(const void *buf,
>>> size_t size, void *opaque)
>>>
>>>   written_size = qemu_write_full(s->fd, buf, size);
>>>   if (written_size != size) {
>>> -    return -1;
>>> +    return -errno;
>>>   }
>>>
>>>   return 0;
>>> @@ -140,7 +140,7 @@ static void write_elf64_header(DumpState *s,
>>> Error **errp)
>>>
>>>   ret = fd_write_vmcore(_header, sizeof(elf_header), s);
>>>   if (ret < 0) {
>>> -    error_setg(errp, "dump: failed to write elf header");
>>> +    error_setg_errno(errp, -ret, "dump: failed to write elf
>>> header");
>>
>> Do we need -ret passed to error_setg_errno()? fd_write_vmcore() returns
>> negative errno in case of error.
> 
> Yes, this usage is correct.  error_setg_errno() takes a positive errno
> value (using strerror, which only decodes positive values into useful
> strings); but we typically return negative errno values (as was
> correctly done in fd_write_vmcore), so the extra layer of negation here
> is needed.
> 

For some reason I assumed "non-zero" in the error_setg_errno()
description as negative.

Thanks Daniel and Eric.

Re: [Qemu-devel] "make check -j4" hangs

[Thomas Huth]

On 12.02.2018 15:42, klim wrote:
[...]
> I just have reverted my 2 commits and
> 
> after that make check -j32 hangs
> 
> with
> 
> GTester: last random seed: R02Sb95a3bf6ab4c05540cec188081a7cc2a
> 
> in vhost-user-test
> 
> so it is not my fault

You're right. I've bisected the issue again, and this time I've applied
8f6d701044b ("tests/test-filter-redirector: move close()") on top of
each step if necessary, and indeed, the "vhost-user-test" problem has
nothing to do with your patches, Klim, but with the one from Marc-André.
According to git bisect, this is the commit that introduced the problem:

commit 7e49f5e8e508ed020c96798b3f7083e24e0e425b
tests: use memfd in vhost-user-test

Peter, since this is quite annoying if "make check" is not working
reliably, could you please revert that commit until a proper solution is
found?

 Thanks,
  Thomas

[Qemu-devel] [RFC v6 22/22] hw/vfio/common: Do not print error when viommu translates into an mmio region

[Eric Auger]

On ARM, the MSI doorbell is translated by the virtual IOMMU.
As such address_space_translate() returns the MSI controller
MMIO region and we get an "iommu map to non memory area"
message. Let's remove this latter.

Signed-off-by: Eric Auger 
---
 hw/vfio/common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 8f3fa0c..1fc8e28 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -326,8 +326,6 @@ static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void 
**vaddr,
  iotlb->translated_addr,
  , , writable);
 if (!memory_region_is_ram(mr)) {
-error_report("iommu map to non memory area %"HWADDR_PRIx"",
- xlat);
 return false;
 }
 
-- 
1.9.1

[Qemu-devel] [RFC v6 21/22] virtio-iommu: Implement set_page_size_mask

[Eric Auger]

We implement the set_page_size_mask callback to allow the
virtio-iommu to be aware of any restrictions on the page size
mask due to an underlying HW IOMMU.

Signed-off-by: Eric Auger 
---
 hw/virtio/trace-events   |  1 +
 hw/virtio/virtio-iommu.c | 16 
 2 files changed, 17 insertions(+)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index ce718e8..1145995 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -55,3 +55,4 @@ virtio_iommu_translate_out(uint64_t virt_addr, uint64_t 
phys_addr, uint32_t sid)
 virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t 
addr, uint64_t size, uint32_t flags, size_t filled) "dev= %d, subtype=%d 
addr=0x%"PRIx64" size=0x%"PRIx64" flags=%d filled=0x%lx"
 virtio_iommu_fill_none_property(uint32_t devid) "devid=%d"
 virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, 
uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
+virtio_iommu_set_page_size_mask(const char *iommu_mr, uint64_t mask) "mr=%s 
page_size_mask=0x%"PRIx64
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index a8fabef..1a15ccc 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -837,6 +837,21 @@ unlock:
 return entry;
 }
 
+static void virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
+uint64_t page_size_mask)
+{
+IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+VirtIOIOMMU *s = sdev->viommu;
+
+s->config.page_size_mask &= page_size_mask;
+if (!s->config.page_size_mask) {
+error_setg(_fatal,
+   "No compatible page size between guest and host iommus");
+}
+
+trace_virtio_iommu_set_page_size_mask(mr->parent_obj.name, page_size_mask);
+}
+
 static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
 VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
@@ -1027,6 +1042,7 @@ static void 
virtio_iommu_memory_region_class_init(ObjectClass *klass,
 IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
 
 imrc->translate = virtio_iommu_translate;
+imrc->set_page_size_mask = virtio_iommu_set_page_size_mask;
 }
 
 static const TypeInfo virtio_iommu_info = {
-- 
1.9.1