Re: [Qemu-devel] [RFC PATCH qemu 3/4] memory: Share flat views and dispatch trees between address spaces

[Alexey Kardashevskiy]

On 12/09/17 01:30, Paolo Bonzini wrote:
> On 11/09/2017 14:08, Alexey Kardashevskiy wrote:
>>> Ok, this makes sense.  Maybe it should be a flatview rather than an
>>> AddressSpaceDispatch (a FlatView is essentially a list of
>>> MemoryRegionSections; attaching the ASD to the FlatView is more or less
>>> an implementation detail).
>> The helpers I converted from AddressSpace to AddressSpaceDispatch do use
>> dispatch structure only and do not use FlatView so it seemed logical.
> 
> Understood, but from a design POV FlatView makes more sense.
> 
>> btw this address_space in MemoryRegionSection - it does not seem to make
>> much sense in the PhysPageMap::sections array, it only makes sense when
>> MemoryRegionSection uses as a temporary object when calling listeners. Will
>> it make sense if we enforce MemoryRegionSection::address_space to be NULL
>> in the array and not NULL when used temporary?
> 
> memory_region_section_get_iotlb needs to access the AddressSpaceDispatch
> for sections stored in the PhysPageMap array, because
> memory_region_section_get_iotlb uses the ASD to compute the section index.

Ohhh, not extremely trivial, out of curiosity - is that iotlb encoding
described anywhere?

Anyway, this can be simplified (or rather made more straightforward?) -
tlb_set_page_with_attrs() can calculate the section index and pass it to
memory_region_section_get_iotlb(). Still does not make much sense? It just
looks quite useless to keep that address_space pointer alive just for one
case which can easily avoid using this pointer.


-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2 04/21] ppc/xive: provide a link to the sPAPR ICS object under XIVE

[Cédric Le Goater]

On 09/12/2017 12:04 AM, Greg Kurz wrote:
> On Mon, 11 Sep 2017 19:12:18 +0200
> Cédric Le Goater  wrote:
> 
>> The sPAPR machine first starts with a XICS interrupt model and
>> depending on the guest capabilities, the XIVE exploitation mode is
>> negotiated during CAS. A reset should then be performed to rebuild the
>> device tree but the same IRQ numbers which were allocated by the
>> devices prior to reset, when the XICS model was operating, are still
>> in use.
>>
>> For this purpose, we need a common IRQ number allocator for both the
>> interrupt models: XICS legacy or XIVE exploitation. This is what the
>> ICSIRQState array of the XICS interrupt source is used for. It also
>> contains the LSI/MSI flag of an interrupt which will we need later on.
>>
>> So, let's provide a link to the sPAPR ICS object under XIVE to make
>> use of it.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive.c| 12 
>>  include/hw/ppc/spapr_xive.h |  4 
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 6d98528fae68..1681affb0848 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -56,6 +56,8 @@ void spapr_xive_reset(void *dev)
>>  static void spapr_xive_realize(DeviceState *dev, Error **errp)
>>  {
>>  sPAPRXive *xive = SPAPR_XIVE(dev);
>> +Object *obj;
>> +Error *err = NULL;
>>  
>>  if (!xive->nr_targets) {
>>  error_setg(errp, "Number of interrupt targets needs to be greater 
>> 0");
>> @@ -68,6 +70,16 @@ static void spapr_xive_realize(DeviceState *dev, Error 
>> **errp)
>>  return;
>>  }
>>  
>> +/* Retrieve SPAPR ICS source to share the IRQ number allocator */
>> +obj = object_property_get_link(OBJECT(dev), "ics", );
>> +if (!obj) {
>> +error_setg(errp, "%s: required link 'ics' not found: %s",
>> +   __func__, error_get_pretty(err));
>> +return;
> 
> err is leaked if you do this way. Please do this instead:
> 
> error_propagate(errp, err);
> error_prepend(errp, "required link 'ics' not found: ");

ok. I will fix. 

C.

> Note: I've just sent a patch to fix the same error in XICS :)
>
>> +}
>> +
>> +xive->ics = ICS_BASE(obj);
>> +
>>  /* Allocate SBEs (State Bit Entry). 2 bits, so 4 entries per byte */
>>  xive->sbe_size = DIV_ROUND_UP(xive->nr_irqs, 4);
>>  xive->sbe = g_malloc0(xive->sbe_size);
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index b17dd4f17b0b..29112589b37f 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -24,6 +24,7 @@
>>  typedef struct sPAPRXive sPAPRXive;
>>  typedef struct XiveIVE XiveIVE;
>>  typedef struct XiveEQ XiveEQ;
>> +typedef struct ICSState ICSState;
>>  
>>  #define TYPE_SPAPR_XIVE "spapr-xive"
>>  #define SPAPR_XIVE(obj) OBJECT_CHECK(sPAPRXive, (obj), TYPE_SPAPR_XIVE)
>> @@ -35,6 +36,9 @@ struct sPAPRXive {
>>  uint32_t nr_targets;
>>  uint32_t nr_irqs;
>>  
>> +/* IRQ */
>> +ICSState *ics;  /* XICS source inherited from the SPAPR machine */
>> +
>>  /* XIVE internal tables */
>>  uint8_t  *sbe;
>>  uint32_t sbe_size;
> 

Re: [Qemu-devel] [PATCH] hmp: fix "dump-quest-memory" segfault (ppc)

[Thomas Huth]

On 12.09.2017 06:52, Miroslav Rezanina wrote:
> 
> 
> - Original Message -
>> From: "Thomas Huth" 
>> To: "Laurent Vivier" , qemu-devel@nongnu.org
>> Cc: "David Gibson" , qemu-...@nongnu.org, "Dr . 
>> David Alan Gilbert"
>> , "Miroslav Rezanina" 
>> Sent: Monday, September 11, 2017 4:36:01 PM
>> Subject: Re: [PATCH] hmp: fix "dump-quest-memory" segfault (ppc)
>>
>> On 11.09.2017 13:00, Laurent Vivier wrote:
>>> Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault)
>>> fixes the problem for i386, do the same for ppc.
>>>
>>> Running QEMU with
>>> qemu-system-ppc64 -M none -nographic -m 256
>>> and executing
>>> dump-guest-memory /dev/null 0 8192
>>> results in segfault
>>>
>>> Fix by checking if we have CPU.
[...]
> 
> We need similar fix for aarch64 too.

Yes, Laurent already posted a v2 which includes a patch for ARM, too:

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02586.html

 Thomas

Re: [Qemu-devel] [PATCH] hmp: fix "dump-quest-memory" segfault (ppc)

[Miroslav Rezanina]

- Original Message -
> From: "Thomas Huth" 
> To: "Laurent Vivier" , qemu-devel@nongnu.org
> Cc: "David Gibson" , qemu-...@nongnu.org, "Dr . 
> David Alan Gilbert"
> , "Miroslav Rezanina" 
> Sent: Monday, September 11, 2017 4:36:01 PM
> Subject: Re: [PATCH] hmp: fix "dump-quest-memory" segfault (ppc)
> 
> On 11.09.2017 13:00, Laurent Vivier wrote:
> > Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault)
> > fixes the problem for i386, do the same for ppc.
> > 
> > Running QEMU with
> > qemu-system-ppc64 -M none -nographic -m 256
> > and executing
> > dump-guest-memory /dev/null 0 8192
> > results in segfault
> > 
> > Fix by checking if we have CPU.
> > 
> > Signed-off-by: Laurent Vivier 
> > ---
> >  target/ppc/arch_dump.c | 17 +++--
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> > index 8e9397aa58..dcb7b19950 100644
> > --- a/target/ppc/arch_dump.c
> > +++ b/target/ppc/arch_dump.c
> > @@ -224,17 +224,22 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
> >  int cpu_get_dump_info(ArchDumpInfo *info,
> >const struct GuestPhysBlockList *guest_phys_blocks)
> >  {
> > -PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > -PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > -
> >  info->d_machine = PPC_ELF_MACHINE;
> >  info->d_class = ELFCLASS;
> >  
> > -if ((*pcc->interrupts_big_endian)(cpu)) {
> > -info->d_endian = ELFDATA2MSB;
> > +if (first_cpu) {
> > +PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > +PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > +
> > +if ((*pcc->interrupts_big_endian)(cpu)) {
> > +info->d_endian = ELFDATA2MSB;
> > +} else {
> > +info->d_endian = ELFDATA2LSB;
> > +}
> >  } else {
> > -info->d_endian = ELFDATA2LSB;
> > +info->d_endian = ELFDATA2MSB;
> >  }
> > +
> >  /* 64KB is the max page size for pseries kernel */
> >  if (strncmp(object_get_typename(qdev_get_machine()),
> >  "pseries-", 8) == 0) {
> > 
> 
> Reviewed-by: Thomas Huth 
> 

We need similar fix for aarch64 too.

Mirek
-- 
Miroslav Rezanina
Software Engineer - Virtualization Team

Re: [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls

[David Gibson]

On Tue, Sep 12, 2017 at 01:27:53PM +1000, David Gibson wrote:
> On Mon, Sep 11, 2017 at 11:10:57PM +0200, Greg Kurz wrote:
> > This patch removes the qdev_get_machine() calls that are made
> > in spapr_cpu_core.c in situations where we can get an existing
> > pointer for the MachineState by either passing it as an argument
> > to the function or by using other already available pointers.
> > 
> > Credits to Daniel Henrique Barboza for the idea and the changelog
> > text.
> > 
> > Signed-off-by: Greg Kurz 
> > ---
> > v2: - fixed typo in spapr_cpu_reset()
> 
> Applied to ppc-for-2.11.

Wait... no.  The second hunk is fine, but the first isn't.

> 
> > ---
> >  hw/ppc/spapr_cpu_core.c |8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index dc9df0d393d1..0f32532abe99 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> >  
> >  static void spapr_cpu_reset(void *opaque)
> >  {
> > -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >  PowerPCCPU *cpu = opaque;
> > +sPAPRMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);

Although we prefer to avoid the global when we can, it's preferable to
breaking abstraction by maching the machine reach into the cpu
internals here.

Arguably, instead of having this special machine-registered cpu reset
function we should instead have a hook in vhyp which is called by the
cpu core's reset function, but that would be a different change.

> >  CPUState *cs = CPU(cpu);
> >  CPUPPCState *env = >env;
> >  
> > @@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState 
> > *dev, Error **errp)
> >  g_free(sc->threads);
> >  }
> >  
> > -static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> > +static void spapr_cpu_core_realize_child(Object *child,
> > + sPAPRMachineState *spapr, Error 
> > **errp)
> >  {
> >  Error *local_err = NULL;
> > -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >  CPUState *cs = CPU(child);
> >  PowerPCCPU *cpu = POWERPC_CPU(cs);
> >  Object *obj;
> > @@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> > Error **errp)
> >  for (j = 0; j < cc->nr_threads; j++) {
> >  obj = sc->threads + j * size;
> >  
> > -spapr_cpu_core_realize_child(obj, _err);
> > +spapr_cpu_core_realize_child(obj, spapr, _err);
> >  if (local_err) {
> >  goto err;
> >  }
> > 
> 



-- 
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] xics: fix several error leaks

[David Gibson]

On Tue, Sep 12, 2017 at 12:04:40AM +0200, Greg Kurz wrote:
> If object_property_get_link() fails then it allocates an error, which
> must be freed before returning. The error_get_pretty() function is
> merely an accessor to the error message and doesn't free anything.
> 
> The error.h header indicates how to do it right:
> 
>  * Pass an existing error to the caller with the message modified:
>  * error_propagate(errp, err);
>  * error_prepend(errp, "Could not frobnicate '%s': ", name);
> 
> Signed-off-by: Greg Kurz 

Applied to ppc-for-2.11.

> ---
>  hw/intc/xics.c |   12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index a84ba51ad8ff..80c33be02e5e 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -306,8 +306,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
>  
>  obj = object_property_get_link(OBJECT(dev), ICP_PROP_XICS, );
>  if (!obj) {
> -error_setg(errp, "%s: required link '" ICP_PROP_XICS "' not found: 
> %s",
> -   __func__, error_get_pretty(err));
> +error_propagate(errp, err);
> +error_prepend(errp, "required link '" ICP_PROP_XICS "' not found: ");
>  return;
>  }
>  
> @@ -315,8 +315,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
>  
>  obj = object_property_get_link(OBJECT(dev), ICP_PROP_CPU, );
>  if (!obj) {
> -error_setg(errp, "%s: required link '" ICP_PROP_CPU "' not found: 
> %s",
> -   __func__, error_get_pretty(err));
> +error_propagate(errp, err);
> +error_prepend(errp, "required link '" ICP_PROP_CPU "' not found: ");
>  return;
>  }
>  
> @@ -641,8 +641,8 @@ static void ics_base_realize(DeviceState *dev, Error 
> **errp)
>  
>  obj = object_property_get_link(OBJECT(dev), ICS_PROP_XICS, );
>  if (!obj) {
> -error_setg(errp, "%s: required link '" ICS_PROP_XICS "' not found: 
> %s",
> -   __func__, error_get_pretty(err));
> +error_propagate(errp, err);
> +error_prepend(errp, "required link '" ICS_PROP_XICS "' not found: ");
>  return;
>  }
>  ics->xics = XICS_FABRIC(obj);
> 

-- 
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] vfio, spapr: Fix levels calculation

[David Gibson]

On Tue, Sep 12, 2017 at 12:53:07PM +1000, Alexey Kardashevskiy wrote:
> The existing tries to round up the number of pages but @pages is always
> calculated as the rounded up value minus one  which makes ctz64() always
> return 0 and have create.levels always set 1.
> 
> This removes wrong "-1" and allows having more than 1 levels. This becomes
> handy for >128GB guests with standard 64K pages as this requires blocks
> with zone order 9 and the popular limit of CONFIG_FORCE_MAX_ZONEORDER=9
> means that only blocks up to order 8 are allowed.
> 
> Signed-off-by: Alexey Kardashevskiy 

Applied to ppc-for-2.11.

> ---
>  hw/vfio/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 32fd6a9b54..259397c002 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -163,7 +163,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
>   */
>  entries = create.window_size >> create.page_shift;
>  pages = MAX((entries * sizeof(uint64_t)) / getpagesize(), 1);
> -pages = MAX(pow2ceil(pages) - 1, 1); /* Round up */
> +pages = MAX(pow2ceil(pages), 1); /* Round up */
>  create.levels = ctz64(pages) / 6 + 1;
>  
>  ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, );

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] spapr_cpu_core: fail gracefully with non-pseries machine types

[David Gibson]

On Mon, Sep 11, 2017 at 10:52:06PM +0200, Greg Kurz wrote:
> Since commit 7cca3e466eb0 ("ppc: spapr: Move VCPU ID calculation into
> sPAPR"), QEMU aborts when started with a *-spapr-cpu-core device and
> a non-pseries machine.
> 
> Let's rely on the already existing call to object_dynamic_cast() instead
> of using the SPAPR_MACHINE() macro.
> 
> Signed-off-by: Greg Kurz 

Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_cpu_core.c |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index d04d1e0d8164..dc9df0d393d1 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -201,7 +201,7 @@ error:
>  
>  static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
>  {
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +sPAPRMachineState *spapr;
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
>  CPUCore *cc = CPU_CORE(OBJECT(dev));
> @@ -211,7 +211,8 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  void *obj;
>  int i, j;
>  
> -if (!object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE)) {
> +spapr = (sPAPRMachineState *) qdev_get_machine();
> +if (!object_dynamic_cast((Object *) spapr, TYPE_SPAPR_MACHINE)) {
>  error_setg(errp, "spapr-cpu-core needs a pseries machine");
>  return;
>  }
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls

[David Gibson]

On Mon, Sep 11, 2017 at 11:10:57PM +0200, Greg Kurz wrote:
> This patch removes the qdev_get_machine() calls that are made
> in spapr_cpu_core.c in situations where we can get an existing
> pointer for the MachineState by either passing it as an argument
> to the function or by using other already available pointers.
> 
> Credits to Daniel Henrique Barboza for the idea and the changelog
> text.
> 
> Signed-off-by: Greg Kurz 
> ---
> v2: - fixed typo in spapr_cpu_reset()

Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_cpu_core.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index dc9df0d393d1..0f32532abe99 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>  
>  static void spapr_cpu_reset(void *opaque)
>  {
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  PowerPCCPU *cpu = opaque;
> +sPAPRMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);
>  CPUState *cs = CPU(cpu);
>  CPUPPCState *env = >env;
>  
> @@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState 
> *dev, Error **errp)
>  g_free(sc->threads);
>  }
>  
> -static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> +static void spapr_cpu_core_realize_child(Object *child,
> + sPAPRMachineState *spapr, Error 
> **errp)
>  {
>  Error *local_err = NULL;
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  CPUState *cs = CPU(child);
>  PowerPCCPU *cpu = POWERPC_CPU(cs);
>  Object *obj;
> @@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  for (j = 0; j < cc->nr_threads; j++) {
>  obj = sc->threads + j * size;
>  
> -spapr_cpu_core_realize_child(obj, _err);
> +spapr_cpu_core_realize_child(obj, spapr, _err);
>  if (local_err) {
>  goto err;
>  }
> 

-- 
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] tcg/ppc: disable atomic write check on ppc32

[Richard Henderson]

On 09/11/2017 02:37 PM, Peter Maydell wrote:
> On 11 September 2017 at 21:49, Philippe Mathieu-Daudé  wrote:
>> this fixes building for ppc64 on ppc32 (changed in 5964fca8a12c):
>>
>>   qemu/tcg/ppc/tcg-target.inc.c: In function 'tb_target_set_jmp_target':
>>   qemu/include/qemu/compiler.h:86:30: error: static assertion failed: "not 
>> expecting: sizeof(*(uint64_t *)jmp_addr) > ATOMIC_REG_SIZE"
>>QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \
>>^
>>   qemu/tcg/ppc/tcg-target.inc.c:1377:9: note: in expansion of macro 
>> 'atomic_set'
>>atomic_set((uint64_t *)jmp_addr, pair);
>>^
>>
>> Suggested-by: Richard Henderson 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> This fixes Shippable builds, see:
>> https://app.shippable.com/github/qemu/qemu/runs/434/10/console
>>
>>  tcg/ppc/tcg-target.inc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
>> index 21d764c102..0417901289 100644
>> --- a/tcg/ppc/tcg-target.inc.c
>> +++ b/tcg/ppc/tcg-target.inc.c
>> @@ -1374,7 +1374,7 @@ void tb_target_set_jmp_target(uintptr_t tc_ptr, 
>> uintptr_t jmp_addr,
>>  pair = (uint64_t)i2 << 32 | i1;
>>  #endif
>>
>> -atomic_set((uint64_t *)jmp_addr, pair);
>> +atomic_set__nocheck((uint64_t *)jmp_addr, pair);
>>  flush_icache_range(jmp_addr, jmp_addr + 8);
>>  } else {
>>  intptr_t diff = addr - jmp_addr;
> 
> Can you explain why this is the right thing? On the
> face of it it looks correct to insist that we don't
> try to do an atomic set of something that's bigger
> than the host can actually handle...

It is the correct thing because ppc32 is handled earlier in the function; only
ppc64 can reach here, therefore a 64-bit atomic_set is always available.

However, I wrote the function intending to minimize the ifdefs so that we can
be sure that it all compiles -- especially the ppc32 bits which I cannot test
on gcc cfarm machines.  I didn't think about the fact that ppc32 could not
compile the _Static_assert within the 64-bit atomic_set here in the ppc64 
section.


r~

Re: [Qemu-devel] [PATCH] tcg/ppc: disable atomic write check on ppc32

[Richard Henderson]

On 09/11/2017 01:49 PM, Philippe Mathieu-Daudé wrote:
> this fixes building for ppc64 on ppc32 (changed in 5964fca8a12c):
> 
>   qemu/tcg/ppc/tcg-target.inc.c: In function 'tb_target_set_jmp_target':
>   qemu/include/qemu/compiler.h:86:30: error: static assertion failed: "not 
> expecting: sizeof(*(uint64_t *)jmp_addr) > ATOMIC_REG_SIZE"
>QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \
>^
>   qemu/tcg/ppc/tcg-target.inc.c:1377:9: note: in expansion of macro 
> 'atomic_set'
>atomic_set((uint64_t *)jmp_addr, pair);
>^
> 
> Suggested-by: Richard Henderson 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> This fixes Shippable builds, see:
> https://app.shippable.com/github/qemu/qemu/runs/434/10/console

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH] dump: do not dump non-existent guest memory

[Peter Xu]

On Mon, Sep 11, 2017 at 03:26:27PM +0200, Cornelia Huck wrote:
> It does not really make sense to dump memory that is not there.
> 
> Moreover, that fixes a segmentation fault when calling dump-guest-memory
> with no filter for a machine with no memory defined.
> 
> New behaviour is:
> 
> (qemu) dump-guest-memory /dev/null
> dump: no guest memory to dump
> (qemu) dump-guest-memory /dev/null 0 4096
> dump: no guest memory to dump
> 
> Signed-off-by: Cornelia Huck 
> ---
> 
> Another unmaintained file. Joy. cc:ing some more-or-less random folks.

I thought Marc-André had proposed to be the maintainer? But indeed I
didn't see the line in maintainer file.

Anyway, if anyone think I am ok to maintain this single file
(considering that I haven't posted patches on it for 2 years, and I
haven't been working on any kind of maintainer job), please let me
know. I'm glad to start with it (or with Marc-André).

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 09/11/17 11:52 -0700, Stefano Stabellini wrote:
> CC'ing xen-devel, and the Xen tools and x86 maintainers.
> 
> On Mon, 11 Sep 2017, Igor Mammedov wrote:
> > On Mon, 11 Sep 2017 12:41:47 +0800
> > Haozhong Zhang  wrote:
> > 
> > > This is the QEMU part patches that works with the associated Xen
> > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > > guest address space for vNVDIMM devices.
> > > 
> > > All patches can be found at
> > >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> > >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > > 
> > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > > label data, as the Xen side support for labels is not implemented yet.
> > > 
> > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > > memory region for Xen guest, in order to make the existing nvdimm
> > > device plugging path work on Xen.
> > > 
> > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > > used as the Xen device model.
> > 
> > I've skimmed over patch-set and can't say that I'm happy with
> > number of xen_enabled() invariants it introduced as well as
> > with partial blobs it creates.
> 
> I have not read the series (Haozhong, please CC me, Anthony and
> xen-devel to the whole series next time), but yes, indeed. Let's not add
> more xen_enabled() if possible.
> 
> Haozhong, was there a design document thread on xen-devel about this? If
> so, did it reach a conclusion? Was the design accepted? If so, please
> add a link to the design doc in the introductory email, so that
> everybody can read it and be on the same page.

Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed
the guest ACPI.

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html

> 
> 
> > I'd like to reduce above and a way to do this might be making xen 
> >  1. use fw_cfg
> >  2. fetch QEMU build acpi tables from fw_cfg
> >  3. extract nvdim tables (which is trivial) and use them
> > 
> > looking at xen_load_linux(), it seems possible to use fw_cfg.
> > 
> > So what's stopping xen from using it elsewhere?,
> > instead of adding more xen specific code to do 'the same'
> > job and not reusing/sharing common code with tcg/kvm.
> 
> So far, ACPI tables have not been generated by QEMU. Xen HVM machines
> rely on a firmware-like application called "hvmloader" that runs in
> guest context and generates the ACPI tables. I have no opinions on
> hvmloader and I'll let the Xen maintainers talk about it. However, keep
> in mind that with an HVM guest some devices are emulated by Xen and/or
> by other device emulators that can run alongside QEMU. QEMU doesn't have
> a full few of the system.
> 
> Here the question is: does it have to be QEMU the one to generate the
> ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
> like the rest, instead of introducing this split-brain design about
> ACPI. We need to see a design doc to fully understand this.
>

hvmloader runs in the guest and is responsible to build/load guest
ACPI. However, it's not capable to build AML at runtime (for the lack
of AML builder). If any guest ACPI object is needed (e.g. by guest
DSDT), it has to be generated from ASL by iasl at Xen compile time and
then be loaded by hvmloader at runtime.

Xen includes an OperationRegion "BIOS" in the static generated guest
DSDT, whose address is hardcoded and which contains a list of values
filled by hvmloader at runtime. Other ACPI objects can refer to those
values (e.g., the number of vCPUs). But it's not enough for generating
guest NVDIMM ACPI objects at compile time and then being customized
and loaded by hvmload, because its structure (i.e., the number of
namespace devices) cannot be decided util the guest config is known.

Alternatively, we may introduce an AML builder in hvmloader and build
all guest ACPI completely in hvmloader. Looking at the similar
implementation in QEMU, it would not be small, compared to the current
size of hvmloader. Besides, I'm still going to let QEMU handle guest
NVDIMM _DSM and _FIT calls, which is another reason I use QEMU to
build NVDIMM ACPI.

> If the design doc thread led into thinking that it has to be QEMU to
> generate them, then would it make the code nicer if we used fw_cfg to
> get the (full or partial) tables from QEMU, as Igor suggested?

I'll have a look at the code (which I didn't notice) pointed by Igor.

One possible issue to use fw_cfg is how to avoid the conflict between
ACPI built by QEMU and ACPI built by hvmloader (e.g., both may use the
same table signature / device names / ...). In my current design, QEMU
will pass the table signatures and device names used in its ACPI to
Xen, and Xen can check the conflict with its own ACPI. Perhaps we can
add necessary functions in fw_cfg as well. Anyway, let me 

Re: [Qemu-devel] [PATCH] dump: do not dump non-existent guest memory

[Peter Xu]

On Mon, Sep 11, 2017 at 03:26:27PM +0200, Cornelia Huck wrote:
> It does not really make sense to dump memory that is not there.
> 
> Moreover, that fixes a segmentation fault when calling dump-guest-memory
> with no filter for a machine with no memory defined.
> 
> New behaviour is:
> 
> (qemu) dump-guest-memory /dev/null
> dump: no guest memory to dump
> (qemu) dump-guest-memory /dev/null 0 4096
> dump: no guest memory to dump
> 
> Signed-off-by: Cornelia Huck 

Reviewed-by: Peter Xu 

> ---
> 
> Another unmaintained file. Joy. cc:ing some more-or-less random folks.
> 
> ---
>  dump.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/dump.c b/dump.c
> index a79773d0f7..d2093e141b 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -1536,6 +1536,12 @@ static void dump_init(DumpState *s, int fd, bool 
> has_format,
>  fprintf(stderr, "DUMP: total memory to dump: %lu\n", s->total_size);
>  #endif
>  
> +/* it does not make sense to dump non-existent memory */
> +if (!s->total_size) {
> +error_setg(errp, "dump: no guest memory to dump");
> +goto cleanup;
> +}
> +
>  s->start = get_start_block(s);
>  if (s->start == -1) {
>  error_setg(errp, QERR_INVALID_PARAMETER, "begin");
> -- 
> 2.13.5
> 

-- 
Peter Xu

[Qemu-devel] [PATCH qemu] vfio, spapr: Fix levels calculation

[Alexey Kardashevskiy]

The existing tries to round up the number of pages but @pages is always
calculated as the rounded up value minus one  which makes ctz64() always
return 0 and have create.levels always set 1.

This removes wrong "-1" and allows having more than 1 levels. This becomes
handy for >128GB guests with standard 64K pages as this requires blocks
with zone order 9 and the popular limit of CONFIG_FORCE_MAX_ZONEORDER=9
means that only blocks up to order 8 are allowed.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/vfio/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 32fd6a9b54..259397c002 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -163,7 +163,7 @@ int vfio_spapr_create_window(VFIOContainer *container,
  */
 entries = create.window_size >> create.page_shift;
 pages = MAX((entries * sizeof(uint64_t)) / getpagesize(), 1);
-pages = MAX(pow2ceil(pages) - 1, 1); /* Round up */
+pages = MAX(pow2ceil(pages), 1); /* Round up */
 create.levels = ctz64(pages) / 6 + 1;
 
 ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, );
-- 
2.11.0

[Qemu-devel] [Bug 1716028] Re: qemu 2.10 locks images with no feature flag

[Scott Moser]

Kevin,
Thanks for the suggestion of share-rw=on.  I figured I'd try to change our 
'xkvm' wrapper around qemu to use that.

Unfortunately, it looks like , at least in our version of qemu (QEMU emulator
version 2.10.0(Debian 1:2.10+dfsg-0ubuntu1)), that this does not work
with the -drive path.

$ qemu-img create -f qcow2 disk1.img 1G
$ qemu-system-x86_64 \
   -device virtio-net-pci,netdev=net00 \
   -netdev type=user,id=net00 \
   -drive id=drive01,file=disk1.img,format=qcow2,share-rw=on \
   -device drive=drive01,serial=sn-drive01,driver=virtio-blk,index=1 \
   -device drive=drive01,serial=sn-drive01,driver=virtio-blk,index=2
qemu-system-x86_64: -drive id=drive01,file=disk1.img,format=qcow2,share-rw=on: 
Block format 'qcow2' does not support the option 'share-rw'

I had thought you were suggesting the above, right?

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

Title:
  qemu 2.10 locks images with no feature flag

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

Bug description:
  1) % lsb_release -rd
  Description:  Ubuntu Artful Aardvark (development branch)
  Release:  17.10

  2) % apt-cache policy qemu-system-x86
  qemu-system-x86:
Installed: 1:2.10~rc3+dfsg-0ubuntu1
Candidate: 1:2.10+dfsg-0ubuntu1
Version table:
   1:2.10+dfsg-0ubuntu1 500
  500 http://archive.ubuntu.com//ubuntu devel/main amd64 Packages
   *** 1:2.10~rc3+dfsg-0ubuntu1 100
  100 /var/lib/dpkg/status

  3) qemu locks image files with no way to discover this feature nor how
  to disable it

  4) qemu provides a way to query if it supports image locking, and what
  the default value is, and how to disable the locking via cli

  qemu 2.10 now will lock image files and warn if an image is currently
  locked.  This prevent qemu from running (and possibly corrupting said
  image).

  However, qemu does not provide any way to determine if a qemu binary
  actually has this capability.  Normally behavior changing features are
  exposed via some change to the qemu help menu or QMP/QAPI output of
  capabilities.

  I believe this slipped through since libvirt already does image
  locking, but direct cli users will be caught by this change.

  In particular, we have a use-case where we simulate multipath disks by
  creating to disks which point to the same file which now breaks
  without adding the 'file.locking=off' to the -drive parameters;  which
  is also completely undocumented and unexposed.

  Some parts of the cli like -device allow querying of settable options
  (qemu-system-x86 -device scsi_hd,?)  but nothing equivalent exists for
  -drive parameters.

  ProblemType: Bug
  DistroRelease: Ubuntu 17.10
  Package: qemu-system-x86 1:2.10~rc3+dfsg-0ubuntu1
  ProcVersionSignature: Ubuntu 4.12.0-11.12-generic 4.12.5
  Uname: Linux 4.12.0-11-generic x86_64
  NonfreeKernelModules: zfs zunicode zavl zcommon znvpair
  ApportVersion: 2.20.6-0ubuntu7
  Architecture: amd64
  Date: Fri Sep  8 12:56:53 2017
  JournalErrors:
   Hint: You are currently not seeing messages from other users and the system.
 Users in groups 'adm', 'systemd-journal' can see all messages.
 Pass -q to turn off this notice.
   -- Logs begin at Mon 2017-01-30 11:56:02 CST, end at Fri 2017-09-08 12:56:46 
CDT. --
   -- No entries --
  KvmCmdLine: COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
  MachineType: HP ProLiant DL360 Gen9
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   XDG_RUNTIME_DIR=
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/vmlinuz-4.12.0-11-generic 
root=UUID=45354276-e0c0-4bf6-9083-f130b89411cc ro --- console=ttyS1,115200
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)
  dmi.bios.date: 03/05/2015
  dmi.bios.vendor: HP
  dmi.bios.version: P89
  dmi.chassis.type: 23
  dmi.chassis.vendor: HP
  dmi.modalias: 
dmi:bvnHP:bvrP89:bd03/05/2015:svnHP:pnProLiantDL360Gen9:pvr:cvnHP:ct23:cvr:
  dmi.product.family: ProLiant
  dmi.product.name: ProLiant DL360 Gen9
  dmi.sys.vendor: HP

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

Re: [Qemu-devel] [PATCH] tcg: Fix tci build

[Jincheng Miao]

Good to know, and thanks for your review and remind.

Regards,
Jincheng Miao

On Tue, Sep 12, 2017 at 1:29 AM, Alistair Francis  wrote:
> On Mon, Sep 11, 2017 at 5:17 PM,   wrote:
>> From: Jincheng Miao 
>>
>> The previous commit 659ef5cbb8 enable LDST_LABELS in tci target,
>> but which causes tci build error like:
>> tcg/tcg.c:116:13: error: ‘tcg_out_ldst_finalize’ used but never defined 
>> [-Werror]
>>  static bool tcg_out_ldst_finalize(TCGContext *s);
>>  ^
>> cc1: all warnings being treated as errors
>> make[1]: *** [tcg/tcg.o] Error 1
>> make: *** [subdir-x86_64-softmmu] Error 2
>>
>> If this macro is not used in tci, we could just delete it.
>>
>> Signed-off-by: Jincheng Miao 
>
> Thank you for the patch!
>
> This looks good, unfortunately someone else had already sent the same
> fix. You can review their patch and add your Reviewed by line to that
> patch, that will help get the fix merged.
>
> You can see their patch here: https://patchwork.kernel.org/patch/9946503/
>
> Thanks,
> Alistair
>
>> ---
>>  tcg/tci/tcg-target.h | 4 
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/tcg/tci/tcg-target.h b/tcg/tci/tcg-target.h
>> index 5d692e1..26140d7 100644
>> --- a/tcg/tci/tcg-target.h
>> +++ b/tcg/tci/tcg-target.h
>> @@ -206,8 +206,4 @@ static inline void tb_target_set_jmp_target(uintptr_t 
>> tc_ptr,
>>  /* no need to flush icache explicitly */
>>  }
>>
>> -#ifdef CONFIG_SOFTMMU
>> -#define TCG_TARGET_NEED_LDST_LABELS
>> -#endif
>> -
>>  #endif /* TCG_TARGET_H */
>> --
>> 1.8.3.1
>>
>>

Re: [Qemu-devel] [PATCH] i386/cpu/hyperv: support over 64 vcpus for windows guests

[Gonglei (Arei)]

> -Original Message-
> From: Eduardo Habkost [mailto:ehabk...@redhat.com]
> Sent: Tuesday, September 12, 2017 2:38 AM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; m...@redhat.com; pbonz...@redhat.com;
> r...@twiddle.net; mtosa...@redhat.com; vroze...@redhat.com;
> Huangweidong (C)
> Subject: Re: [PATCH] i386/cpu/hyperv: support over 64 vcpus for windows
> guests
> 
> On Mon, Sep 11, 2017 at 11:20:27PM +0800, Gonglei wrote:
> > Starting with Windows Server 2012 and Windows 8, if
> > CPUID.4005.EAX contains a value of -1, Windows assumes specific
> > limit to the number of VPs. In this case, Windows Server 2012
> > guest VMs may use more than 64 VPs, up to the maximum supported
> > number of processors applicable to the specific Windows
> > version being used.
> >
> >
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/referenc
> e/tlfs
> >
> > For compatibility, Let's introduce a new property for X86CPU,
> > named "x-hv-max-vps" as Eduardo's suggestion, and set it
> > to 0x40 before machine 2.10.
> >
> > (The "x-" prefix indicates that the property is not supposed to
> > be a stable user interface.)
> >
> > Signed-off-by: Gonglei 
> > ---
> >  include/hw/i386/pc.h |  5 +
> >  target/i386/cpu.c|  1 +
> >  target/i386/cpu.h|  2 ++
> >  target/i386/kvm.c| 15 ++-
> >  4 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 8226904..087d184 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -371,6 +371,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> uint64_t *);
> >
> >  #define PC_COMPAT_2_10 \
> >  HW_COMPAT_2_10 \
> > +{\
> > +.driver   = TYPE_X86_CPU,\
> > +.property = "x-hv-max-vps",\
> > +.value= "0x40",\
> > +},
> >
> >  #define PC_COMPAT_2_9 \
> >  HW_COMPAT_2_9 \
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 69676e1..2702485 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -4145,6 +4145,7 @@ static Property x86_cpu_properties[] = {
> >   false),
> >  DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU,
> vmware_cpuid_freq, true),
> >  DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> > +DEFINE_PROP_INT32("x-hv-max-vps", X86CPU, hv_max_vps, -1),
> >  DEFINE_PROP_END_OF_LIST()
> >  };
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 525d35d..5c726f3 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1282,6 +1282,8 @@ struct X86CPU {
> >  int32_t socket_id;
> >  int32_t core_id;
> >  int32_t thread_id;
> > +
> > +int32_t hv_max_vps;
> >  };
> >
> >  static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 6db7783..a898bef 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -751,7 +751,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >
> >  c = _data.entries[cpuid_i++];
> >  c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
> > -c->eax = 0x40;
> > +
> > +/*
> > + * From "Requirements for Implementing the Microsoft
> > + * Hypervisor Interface":
> > + *
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/referenc
> e/tlfs
> > + *
> > + * "Starting with Windows Server 2012 and Windows 8, if
> > + * CPUID.4005.EAX contains a value of -1, Windows assumes
> > + * specific limit to the number of VPs. In this case, Windows
> > + * Server 2012 guest VMs may use more than 64 VPs, up to the
> > + * maximum supported number of processors applicable to the
> > + * specific Windows version being used."
> > + */
> 
> I would place this comment above the DEFINE_PROP_INT32
> declaration, as its purpose is to explain the -1 default.
> 
Fine, do I need to send v2? Or you adjust it directly?

Thanks,
-Gonglei
> 
> > +c->eax = cpu->hv_max_vps;
> >  c->ebx = 0x40;
> >
> >  kvm_base = KVM_CPUID_SIGNATURE_NEXT;
> > --
> > 1.8.3.1
> >
> >
> 
> --
> Eduardo

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

[Liu, Changpeng]

Hi Marcandre,

Of course, I agree your cleanup patch set should go first, just re-post the 
patch set
after the new Qemu release, I think the first 3 patches don't have dependency on
your  patch set.

From: Marc-André Lureau [mailto:marcandre.lur...@gmail.com] 
Sent: Monday, September 11, 2017 9:18 PM
To: Liu, Changpeng ; qemu-devel@nongnu.org
Cc: fel...@nutanix.com; Philippe Mathieu-Daudé 
Subject: Re: [Qemu-devel] [PATCH 00/27] vhost-user-scsi: code clean-up

Hi

On Thu, Aug 24, 2017 at 2:41 AM Liu, Changpeng  wrote:
Thanks for helping the example code clean-up. I can rebase the vhost-user-blk 
patch
set after your commits. For the issue you mentioned below, I think the 
vhost-user-scsi example
can only support 1 LUN, so LUN0 is exist, LUN1 reported error should be okay.

I think this series should go first before your vhost-user-blk, to avoid code 
churn. Please help review. 

Patches missing review: 1, 2, 3, 5, 8, 9, 10, 11, 13, 17, 20, 22, 24, 25, 26

Thanks


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

Re: [Qemu-devel] [PATCH] x86/acpi: build SRAT when memory hotplug is enabled

[Dou Liyang]

Hi Igor,

At 09/11/2017 06:58 PM, Igor Mammedov wrote:

> >
> > Igor's suggestion to enable NUMA implicitly sounds safer to me.
> >

>
> I agree with Igor too.
>
> Is anybody doing this? If not, may I make a patch to enable adding NUMA
> node implicitly first. let's see what it looks like.

As far as I'm aware nobody is doing it, so fill free to look into it.



Got it!  I will do it right now.

Thanks,
dou.


>
> Thanks,
>dou.
>
>

Re: [Qemu-devel] [PATCH v7 26/38] libqtest: Merge qtest_end() into qtest_quit()

[John Snow]

On 09/11/2017 01:20 PM, Eric Blake wrote:
> Rather than have two similar shutdown functions, where one requires
> the use of global_qtest in the header, it is better to have a single
> shutdown function that still takes care of cleaning up global_qtest
> if it is set.  All callers are updated.
> 
> Signed-off-by: Eric Blake 
> ---
>  tests/libqtest.h   | 11 ---
>  tests/libqtest.c   |  6 +-
[...]
>  tests/fdc-test.c   |  2 +-
[...]
>  tests/ide-test.c   |  2 +-

Reviewed-by: John Snow 

The rest: "ACK." Change looks sane and I assume the rest of the patch is
fairly straightforward mechanical changes. Everything still compiles and
runs successfully.

Re: [Qemu-devel] [PATCH v7 18/38] ahci-test: Drop dependence on global_qtest

[John Snow]

On 09/11/2017 08:20 PM, John Snow wrote:
> 
> 
> On 09/11/2017 01:20 PM, Eric Blake wrote:
>> Managing parallel connections to two different monitors via
>> the implicit global_qtest makes it hard to copy-and-paste code
>> to tests that are not aware of the implicit state; the
>> management of global_qtest is even harder to follow because
>> it was masked behind set_context().
>>
>> Instead, explicitly pass QTestState* around (generally, by
>> reusing the member already present in ahci->parent QOSState),
>> and call explicit qtest_* functions on all places that
>> interact with a monitor.
>>
>> We can assert that the conversion is correct by checking that
>> global_qtest remains NULL throughout the test (a later patch
>> that changes global_qtest to not be a public global variable
>> will drop the assertions).
>>
>> Bonus: there was one spots that was creating a needless temporary
>> variable to execute the 'cont' command, rather than just directly
>> passing the literal command through qtest_qmp().  Fixing that
>> gets us one step closer to enabling -Wformat checking on
>> constructed JSON.
>>
>> Signed-off-by: Eric Blake 
> 
> still LGTM, you can probably still see why I was a little iffy about
> making the global qstate ubiquitous instead of doing it this way.
> 
> (Though it does make the calls a little more verbose, they are IMO a lot
> easier to reason about in tests that deal with mixed-state and
> migrations and so on, which -- most of my qtest experience was from AHCI
> -- there is a lot of here. (Sorry Markus, I'm a
lol! I hit send too soon, I tabbed away to look for Markus' previous
email which used some German term to refer to people who preferred
explicit state.

I wound up not finding it and then getting distracted.

Enjoy.

Re: [Qemu-devel] [PATCH v7 18/38] ahci-test: Drop dependence on global_qtest

[John Snow]

On 09/11/2017 01:20 PM, Eric Blake wrote:
> Managing parallel connections to two different monitors via
> the implicit global_qtest makes it hard to copy-and-paste code
> to tests that are not aware of the implicit state; the
> management of global_qtest is even harder to follow because
> it was masked behind set_context().
> 
> Instead, explicitly pass QTestState* around (generally, by
> reusing the member already present in ahci->parent QOSState),
> and call explicit qtest_* functions on all places that
> interact with a monitor.
> 
> We can assert that the conversion is correct by checking that
> global_qtest remains NULL throughout the test (a later patch
> that changes global_qtest to not be a public global variable
> will drop the assertions).
> 
> Bonus: there was one spots that was creating a needless temporary
> variable to execute the 'cont' command, rather than just directly
> passing the literal command through qtest_qmp().  Fixing that
> gets us one step closer to enabling -Wformat checking on
> constructed JSON.
> 
> Signed-off-by: Eric Blake 

still LGTM, you can probably still see why I was a little iffy about
making the global qstate ubiquitous instead of doing it this way.

(Though it does make the calls a little more verbose, they are IMO a lot
easier to reason about in tests that deal with mixed-state and
migrations and so on, which -- most of my qtest experience was from AHCI
-- there is a lot of here. (Sorry Markus, I'm a

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v6 04/12] tests: Add vm test lib

[Fam Zheng]

On Mon, 09/11 11:44, Alex Bennée wrote:
> 
> Fam Zheng  writes:
> 
> > On Fri, 09/08 16:22, Alex Bennée wrote:
> >>
> >> Fam Zheng  writes:
> >>
> >> > This is the common code to implement a "VM test" to
> >> >
> >> >   1) Download and initialize a pre-defined VM that has necessary
> >> >   dependencies to build QEMU and SSH access.
> >> >
> >> >   2) Archive $SRC_PATH to a .tar file.
> >> >
> >> >   3) Boot the VM, and pass the source tar file to the guest.
> >> >
> >> >   4) SSH into the VM, untar the source tarball, build from the source.
> >> >
> >> > Signed-off-by: Fam Zheng 
> >> > ---
> >> >  tests/vm/basevm.py | 276 
> >> > +
> >> >  1 file changed, 276 insertions(+)
> >> >  create mode 100755 tests/vm/basevm.py
> >> >
> >> > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> >> > new file mode 100755
> >> > index 00..9db91d61fa
> >> > --- /dev/null
> >> > +++ b/tests/vm/basevm.py
> >> > @@ -0,0 +1,276 @@
> >> > +#!/usr/bin/env python
> >> > +#
> >> > +# VM testing base class
> >> > +#
> >> > +# Copyright (C) 2017 Red Hat Inc.
> >> > +#
> >> > +# Authors:
> >> > +#  Fam Zheng 
> >> > +#
> >> > +# This work is licensed under the terms of the GNU GPL, version 2.  See
> >> > +# the COPYING file in the top-level directory.
> >> > +#
> >> > +
> >> > +import os
> >> > +import sys
> >> > +import logging
> >> > +import time
> >> > +import datetime
> >> > +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", 
> >> > "scripts"))
> >> > +from qemu import QEMUMachine
> >> > +import subprocess
> >> > +import hashlib
> >> > +import optparse
> >> > +import atexit
> >> > +import tempfile
> >> > +import shutil
> >> > +import multiprocessing
> >> > +import traceback
> >> > +
> >> > +SSH_KEY = """\
> >> > +-BEGIN RSA PRIVATE KEY-
> >> > +MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
> >> > +coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
> >> > ++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
> >> > +RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
> >> > +7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
> >> > +Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
> >> > +cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
> >> > +7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
> >> > +R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
> >> > +rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
> >> > +cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
> >> > +vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
> >> > +XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
> >> > +j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
> >> > +T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
> >> > +XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
> >> > +JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
> >> > +CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
> >> > +bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
> >> > +eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
> >> > +BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
> >> > +CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
> >> > +D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
> >> > +VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
> >> > +5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
> >> > +-END RSA PRIVATE KEY-
> >> > +"""
> >> > +SSH_PUB_KEY = """\
> >> > +ssh-rsa 
> >> > B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
> >> >  qemu-vm-key
> >> > +"""
> >>
> >> I'm not sure we should be embedding the keys in the script. I understand
> >> we need a common key for downloaded images (although it would be better
> >> to post-customise the image after download with the local developer
> >> keys). Perhaps ./tests/testing-keys/id_rsa[.pub]?
> >
> > We cannot generate keys or start from local keys because it's hard to 
> > inject it
> > into the *BSD images without ssh access (chicken-and-egg problem). Adding 
> > the
> > local keys might be a good feature, indeed.
> 
> Can't libguestfs be used to manipulate the image without booting BSD
> itself?

No, not working for BSD.

> 
> Regardless I still think having the key embedded in the script rather
> than in files in the source tree is overly ugly.

OK, I can move it 

Re: [Qemu-devel] [PATCH v6 03/12] scripts: Add archive-source.sh

[Fam Zheng]

On Mon, 09/11 14:43, Alex Bennée wrote:
> 
> Fam Zheng  writes:
> 
> > On Fri, 09/08 15:42, Alex Bennée wrote:
> >>
> >> Fam Zheng  writes:
> >>
> >> > Signed-off-by: Fam Zheng 
> >> > ---
> >> >  scripts/archive-source.sh | 31 +++
> >> >  1 file changed, 31 insertions(+)
> >> >  create mode 100755 scripts/archive-source.sh
> >> >
> >> > diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
> >> > new file mode 100755
> >> > index 00..3cae7f34d3
> >> > --- /dev/null
> >> > +++ b/scripts/archive-source.sh
> >> > @@ -0,0 +1,31 @@
> >> > +#!/bin/sh
> >> > +#
> >> > +# Author: Fam Zheng 
> >> > +#
> >> > +# Create archive of source tree, including submodules
> >> > +#
> >> > +# This work is licensed under the terms of the GNU GPL, version 2.
> >> > +# See the COPYING file in the top-level directory.
> >> > +
> >> > +set -e
> >> > +
> >> > +if test $# -lt 1; then
> >> > +echo "Usage: $0 "
> >>
> >> Maybe  to make it clear what it creates?
> >
> > OK.
> >
> >>
> >> > +exit 1
> >> > +fi
> >> > +
> >> > +submodules=$(git submodule foreach --recursive --quiet 'echo $name')
> >> > +
> >> > +if test -n "$submodules"; then
> >> > +{
> >> > +git ls-files
> >>
> >> Couldn't we do the main git ls-files first and then append the data for
> >> any submodules?
> >
> > Isn't that exactly what we are doing now?
> 
> I mean hoist the git ls-files out of the if so we can avoid repeating
> with an else leg. e.g.
> 
>   git ls-files > $1.list

The output of top "git ls-files" has to be filtered by grep if the submodules
list is non-empty, so we cannot save LoC by hoisting.

Fam

>   if test -n "$submodules"; then
>   {
> .. the rest..
>   } | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) >> $1.list
> 
> >
> > Fam
> >
> >>
> >> > +for sm in $submodules; do
> >> > +(cd $sm; git ls-files) | sed "s:^:$sm/:"
> >> > +done
> >> > +} | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > 
> >> > $1.list
> >> > +else
> >> > +git ls-files > $1.list
> >> > +fi
> >> > +
> >> > +tar -cf $1 -T $1.list
> >> > +rm $1.list
> >>
> >>
> >> --
> >> Alex Bennée
> 
> 
> --
> Alex Bennée

Re: [Qemu-devel] [PATCH v7 17/38] libqos: Use explicit QTestState for remaining libqos operations

[John Snow]

On 09/11/2017 01:20 PM, Eric Blake wrote:
> Drop one more client of global_qtest by teaching all remaining
> libqos stragglers to pass in an explicit QTestState.  Change the
> setting of global_qtest from being implicit in libqos' call to
> qtest_start() to instead be explicit in all clients that are
> still relying on global_qtest.
> 
> Note that qmp_execute() can be greatly simplified in the process,
> and that we also get rid of interpolation of a JSON string into a
> temporary variable when qtest_qmp() can do it more reliably.
> 
> Signed-off-by: Eric Blake 
> 

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v7 16/38] libqos: Use explicit QTestState for ahci operations

[John Snow]

On 09/11/2017 01:20 PM, Eric Blake wrote:
> Drop one more client of global_qtest by teaching all ahci test
> functionality to pass in an explicit QTestState.  The state was
> already available, so no callers had to be adjusted.
> 
> Signed-off-by: Eric Blake 

I should admit that this makes my urge to write a wrapper/macro increase
just a little bit, but ... admitting you have a problem is the first
step to recovery.

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v7 13/38] libqos: Use explicit QTestState for fw_cfg operations

[John Snow]

On 09/11/2017 01:19 PM, Eric Blake wrote:
> Drop one more client of global_qtest by teaching all fw_cfg test
> functionality (invoked through alloc-pc) to pass in an explicit
> QTestState, adjusting all callers.  In particular, fw_cfg-test
> had to reorder things to create the test state prior to creating
> the fw_cfg (and drop a pointless strdup in the meantime), but that
> test now no longer depends on global_qtest.
> 
> Signed-off-by: Eric Blake 
> 
Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v7 08/38] libqos: Track QTestState with QPCIBus

[John Snow]

On 09/11/2017 01:19 PM, Eric Blake wrote:
> When initializing a QPCIBus, track which QTestState the bus is
> associated with (so that a later patch can then explicitly use
> that test state for all communication on the bus, rather than
> blindly relying on global_qtest).  Update the initialization
> functions to take another parameter, and update all callers to
> pass in state (for now, most callers get away with passing the
> current global_qtest as the current state, although this required
> fixing the order of initialization to ensure qtest_start() is
> called before qpci_init*() in rtl8139-test, and provided an
> opportunity to pass in the allocator in e1000e-test).
> 
> Touch up some allocations to use g_new0() rather than g_malloc()
> while in the area, and simplify some code (all implementations
> of QOSOps provide a .init_allocator() that never fails).
> 
> Signed-off-by: Eric Blake 

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v2 5/5] tcg: restrict i386 regs definitions

[Kamil Rytarowski]

On 12.09.2017 00:13, Philippe Mathieu-Daudé wrote:
> On 09/11/2017 06:44 PM, Kamil Rytarowski wrote:
>> On 11.09.2017 23:33, Philippe Mathieu-Daudé wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>> ---
>>> cleaning while here :)
>>>
>>>   accel/tcg/user-exec.c | 18 +-
>>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
>>> index 2a975eaf69..484a3f5f8f 100644
>>> --- a/accel/tcg/user-exec.c
>>> +++ b/accel/tcg/user-exec.c
>>> @@ -25,15 +25,6 @@
>>>   #include "exec/cpu_ldst.h"
>>>   #include "translate-all.h"
>>>   -#undef EAX
>>> -#undef ECX
>>> -#undef EDX
>>> -#undef EBX
>>> -#undef ESP
>>> -#undef EBP
>>> -#undef ESI
>>> -#undef EDI
>>> -#undef EIP
>>>   #ifdef __linux__
>>>   #include 
>>>   #endif
>>> @@ -131,6 +122,15 @@ static inline int handle_cpu_signal(uintptr_t
>>> pc, unsigned long address,
>>>   }
>>>     #if defined(__i386__)
>>> +#undef EAX
>>> +#undef ECX
>>> +#undef EDX
>>> +#undef EBX
>>> +#undef ESP
>>> +#undef EBP
>>> +#undef ESI
>>> +#undef EDI
>>> +#undef EIP
>>>     #if defined(__NetBSD__)
>>>   #include 
>>>
>>
>> Why to move under i386?
> 
> I tracked the origin of these #defines in op-i386.c (7bfdb6d18c7b) and
> thought the Exx naming was for i386 while the x86_64 uses the Rxx naming
> (RAX .. RIP) so you'd only have them on i386 arch.
> However it seems I didn't realize you can access x86_64 registers in
> 32-bit mode via the EAX .. EIP naming, as you see I'm not confident with
> this CISC arch :S
> 
> So I guess it's best to ignore this patch?
> 

A typical 64-bit x86_64 OS can run 32-bit programs and reuse x86 32-bit
headers.

>>
>> SmartOS pollutes namespace with these symbols on x86_64.
>>
> 
> you should provide some VM :P
> 

Hope to see it too.

> I plan to test this project soon:
> https://www.packer.io/docs/builders/qemu.html
> 
> Regards,
> 
> Phil.




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 00/12] cleanup qemu-iotests

[Paolo Bonzini]

- Original Message -
> From: "John Snow" 
> To: "Paolo Bonzini" , qemu-devel@nongnu.org
> Cc: kw...@redhat.com, qemu-bl...@nongnu.org
> Sent: Tuesday, September 12, 2017 12:22:26 AM
> Subject: Re: [Qemu-block] [PATCH 00/12] cleanup qemu-iotests
> 
> 
> 
> On 08/09/2017 05:54 PM, Paolo Bonzini wrote:
> > The purpose of this series is to separate the "check" sources from
> > the tests.  After these patches, common.config is reduced to simple
> > shell initialization, and common.rc is only included by the tests.
> > 
> > Along the way, a lot of dead code is removed too.
> > 
> > Paolo
> > 
> > Paolo Bonzini (12):
> >   qemu-iotests: remove dead code
> >   qemu-iotests: get rid of AWK_PROG
> >   qemu-iotests: move "check" code out of common.rc
> >   qemu-iotests: limit non-_PROG-suffixed variables to common.rc
> >   qemu-iotests: do not include common.rc in "check"
> >   qemu-iotests: do not do useless search for QEMU_*_PROG
> >   qemu-iotests: disintegrate more parts of common.config
> >   qemu-iotests: fix uninitialized variable
> >   qemu-iotests: do not search for binaries in the current directory
> >   qemu-iotests: get rid of $iam
> >   qemu-iotests: include common.env and common.config early
> >   qemu-iotests: merge "check" and "common"
> > 
> >  tests/qemu-iotests/039.out   |  10 +-
> >  tests/qemu-iotests/061.out   |   4 +-
> >  tests/qemu-iotests/137.out   |   2 +-
> >  tests/qemu-iotests/check | 551
> >  ++-
> >  tests/qemu-iotests/common| 459 
> >  tests/qemu-iotests/common.config | 205 +--
> >  tests/qemu-iotests/common.qemu   |   1 +
> >  tests/qemu-iotests/common.rc | 225 
> >  8 files changed, 615 insertions(+), 842 deletions(-)
> >  delete mode 100644 tests/qemu-iotests/common
> > 
> 
> Stale? Patchset is >1 month old now, doesn't look like it got merged.

Yup, didn't get reviewed by Kevin either...  I was waiting for that
before sending v2.

Paolo

Re: [Qemu-devel] [Qemu-block] [PATCH 00/12] cleanup qemu-iotests

[John Snow]

On 08/09/2017 05:54 PM, Paolo Bonzini wrote:
> The purpose of this series is to separate the "check" sources from
> the tests.  After these patches, common.config is reduced to simple
> shell initialization, and common.rc is only included by the tests.
> 
> Along the way, a lot of dead code is removed too.
> 
> Paolo
> 
> Paolo Bonzini (12):
>   qemu-iotests: remove dead code
>   qemu-iotests: get rid of AWK_PROG
>   qemu-iotests: move "check" code out of common.rc
>   qemu-iotests: limit non-_PROG-suffixed variables to common.rc
>   qemu-iotests: do not include common.rc in "check"
>   qemu-iotests: do not do useless search for QEMU_*_PROG
>   qemu-iotests: disintegrate more parts of common.config
>   qemu-iotests: fix uninitialized variable
>   qemu-iotests: do not search for binaries in the current directory
>   qemu-iotests: get rid of $iam
>   qemu-iotests: include common.env and common.config early
>   qemu-iotests: merge "check" and "common"
> 
>  tests/qemu-iotests/039.out   |  10 +-
>  tests/qemu-iotests/061.out   |   4 +-
>  tests/qemu-iotests/137.out   |   2 +-
>  tests/qemu-iotests/check | 551 
> ++-
>  tests/qemu-iotests/common| 459 
>  tests/qemu-iotests/common.config | 205 +--
>  tests/qemu-iotests/common.qemu   |   1 +
>  tests/qemu-iotests/common.rc | 225 
>  8 files changed, 615 insertions(+), 842 deletions(-)
>  delete mode 100644 tests/qemu-iotests/common
> 

Stale? Patchset is >1 month old now, doesn't look like it got merged.

Re: [Qemu-devel] [PATCH v2 5/5] tcg: restrict i386 regs definitions

[Philippe Mathieu-Daudé]

On 09/11/2017 06:44 PM, Kamil Rytarowski wrote:

On 11.09.2017 23:33, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
cleaning while here :)

  accel/tcg/user-exec.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 2a975eaf69..484a3f5f8f 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -25,15 +25,6 @@
  #include "exec/cpu_ldst.h"
  #include "translate-all.h"
  
-#undef EAX

-#undef ECX
-#undef EDX
-#undef EBX
-#undef ESP
-#undef EBP
-#undef ESI
-#undef EDI
-#undef EIP
  #ifdef __linux__
  #include 
  #endif
@@ -131,6 +122,15 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned 
long address,
  }
  
  #if defined(__i386__)

+#undef EAX
+#undef ECX
+#undef EDX
+#undef EBX
+#undef ESP
+#undef EBP
+#undef ESI
+#undef EDI
+#undef EIP
  
  #if defined(__NetBSD__)

  #include 



Why to move under i386?


I tracked the origin of these #defines in op-i386.c (7bfdb6d18c7b) and 
thought the Exx naming was for i386 while the x86_64 uses the Rxx naming 
(RAX .. RIP) so you'd only have them on i386 arch.
However it seems I didn't realize you can access x86_64 registers in 
32-bit mode via the EAX .. EIP naming, as you see I'm not confident with 
this CISC arch :S


So I guess it's best to ignore this patch?



SmartOS pollutes namespace with these symbols on x86_64.



you should provide some VM :P

I plan to test this project soon:
https://www.packer.io/docs/builders/qemu.html

Regards,

Phil.

[Qemu-devel] [Bug 1716510] [NEW] qemu 2.10.0 cannot boot Windows 10 familly

[Maciej Piechotka]

Public bug reported:

On qemu 2.10.0 Windows 10 and Windows Server 2016 hangs during boot.
Below is setup of Windows Server 2016. Downgrading to 2.9 fixes the
problem.

/usr/bin/qemu-system-x86_64 -name guest=,debug-threads=on -S
-object
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-2-/master-key.aes -machine pc-q35-2.8,accel=kvm,usb=off,dump-guest-
core=off -cpu
host,nx=on,hv_relaxed,hv_vapic,hv_spinlocks=0x1000,hv_vpindex,hv_runtime,hv_synic,hv_reset,kvm=off
-drive file=/usr/local/share/edk2.git/ovmf-x64/OVMF-pure-
efi.fd,if=pflash,format=raw,unit=0 -drive
file=/var/lib/libvirt/qemu/nvram/_VARS.fd,if=pflash,format=raw,unit=1
-m 4096 -realtime mlock=off -smp 12,sockets=1,cores=6,threads=2 -object
iothread,id=iothread1 -object iothread,id=iothread2 -object
iothread,id=iothread3 -object iothread,id=iothread4 -object
iothread,id=iothread5 -object iothread,id=iothread6 -object
iothread,id=iothread7 -object iothread,id=iothread8 -object
iothread,id=iothread9 -object iothread,id=iothread10 -object
iothread,id=iothread11 -object iothread,id=iothread12 -uuid  -no-
user-config -nodefaults -chardev
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-2-/monitor.sock,server,nowait
-mon chardev=charmonitor,id=monitor,mode=control -rtc
base=localtime,clock=vm,driftfix=slew -no-shutdown -boot strict=on
-device
ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
-device ioh3420,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1
-device ioh3420,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2
-device ioh3420,port=0x13,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x3
-device ioh3420,port=0x14,chassis=5,id=pci.5,bus=pcie.0,addr=0x2.0x4
-device ioh3420,port=0x15,chassis=6,id=pci.6,bus=pcie.0,addr=0x2.0x5
-device nec-usb-xhci,id=usb,bus=pci.3,addr=0x0 -drive
if=none,media=cdrom,id=drive-sata0-0-0,readonly=on -device ide-
cd,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0,bootindex=2 -drive
if=none,media=cdrom,id=drive-sata0-0-1,readonly=on -device ide-
cd,bus=ide.1,drive=drive-sata0-0-1,id=sata0-0-1,bootindex=1 -drive
file=/dev/mapper/,format=raw,if=none,id=drive-sata0-0-2
-device ide-hd,bus=ide.2,drive=drive-sata0-0-2,id=sata0-0-2,bootindex=3
-netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=23 -device virtio-net-
pci,netdev=hostnet0,id=net0,mac=,bus=pci.1,addr=0x0 -netdev
tap,fd=24,id=hostnet1,vhost=on,vhostfd=25 -device virtio-net-
pci,netdev=hostnet1,id=net1,mac=,bus=pci.2,addr=0x0 -device usb-
tablet,id=input0,bus=usb.0,port=1 -spice
unix,addr=/var/lib/libvirt/qemu/domain-2-/spice.sock,disable-
ticketing,image-compression=auto_glz,seamless-migration=on -vnc
127.0.0.1:0 -device qxl-
vga,id=video0,ram_size=67108864,vram_size=16777216,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pcie.0,addr=0x1
-device vhost-scsi-
pci,wwpn=,vhostfd=26,id=hostdev0,bus=pcie.0,addr=0x9 -device
virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 -object rng-
random,id=objrng0,filename=/dev/random -device virtio-rng-
pci,rng=objrng0,id=rng0,max-bytes=1024,period=1000,bus=pci.5,addr=0x0
-msg timestamp=o

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu 2.10.0 cannot boot Windows 10 familly

Status in QEMU:
  New

Bug description:
  On qemu 2.10.0 Windows 10 and Windows Server 2016 hangs during boot.
  Below is setup of Windows Server 2016. Downgrading to 2.9 fixes the
  problem.

  /usr/bin/qemu-system-x86_64 -name guest=,debug-threads=on -S
  -object
  secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-2-/master-key.aes -machine pc-q35-2.8,accel=kvm,usb=off,dump-guest-
  core=off -cpu
  
host,nx=on,hv_relaxed,hv_vapic,hv_spinlocks=0x1000,hv_vpindex,hv_runtime,hv_synic,hv_reset,kvm=off
  -drive file=/usr/local/share/edk2.git/ovmf-x64/OVMF-pure-
  efi.fd,if=pflash,format=raw,unit=0 -drive
  file=/var/lib/libvirt/qemu/nvram/_VARS.fd,if=pflash,format=raw,unit=1
  -m 4096 -realtime mlock=off -smp 12,sockets=1,cores=6,threads=2
  -object iothread,id=iothread1 -object iothread,id=iothread2 -object
  iothread,id=iothread3 -object iothread,id=iothread4 -object
  iothread,id=iothread5 -object iothread,id=iothread6 -object
  iothread,id=iothread7 -object iothread,id=iothread8 -object
  iothread,id=iothread9 -object iothread,id=iothread10 -object
  iothread,id=iothread11 -object iothread,id=iothread12 -uuid 
  -no-user-config -nodefaults -chardev
  
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-2-/monitor.sock,server,nowait
  -mon chardev=charmonitor,id=monitor,mode=control -rtc
  base=localtime,clock=vm,driftfix=slew -no-shutdown -boot strict=on
  -device
  ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
  -device ioh3420,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1
  -device ioh3420,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2
  -device 

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2 04/21] ppc/xive: provide a link to the sPAPR ICS object under XIVE

[Greg Kurz]

On Mon, 11 Sep 2017 19:12:18 +0200
Cédric Le Goater  wrote:

> The sPAPR machine first starts with a XICS interrupt model and
> depending on the guest capabilities, the XIVE exploitation mode is
> negotiated during CAS. A reset should then be performed to rebuild the
> device tree but the same IRQ numbers which were allocated by the
> devices prior to reset, when the XICS model was operating, are still
> in use.
> 
> For this purpose, we need a common IRQ number allocator for both the
> interrupt models: XICS legacy or XIVE exploitation. This is what the
> ICSIRQState array of the XICS interrupt source is used for. It also
> contains the LSI/MSI flag of an interrupt which will we need later on.
> 
> So, let's provide a link to the sPAPR ICS object under XIVE to make
> use of it.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/spapr_xive.c| 12 
>  include/hw/ppc/spapr_xive.h |  4 
>  2 files changed, 16 insertions(+)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 6d98528fae68..1681affb0848 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -56,6 +56,8 @@ void spapr_xive_reset(void *dev)
>  static void spapr_xive_realize(DeviceState *dev, Error **errp)
>  {
>  sPAPRXive *xive = SPAPR_XIVE(dev);
> +Object *obj;
> +Error *err = NULL;
>  
>  if (!xive->nr_targets) {
>  error_setg(errp, "Number of interrupt targets needs to be greater 
> 0");
> @@ -68,6 +70,16 @@ static void spapr_xive_realize(DeviceState *dev, Error 
> **errp)
>  return;
>  }
>  
> +/* Retrieve SPAPR ICS source to share the IRQ number allocator */
> +obj = object_property_get_link(OBJECT(dev), "ics", );
> +if (!obj) {
> +error_setg(errp, "%s: required link 'ics' not found: %s",
> +   __func__, error_get_pretty(err));
> +return;

err is leaked if you do this way. Please do this instead:

error_propagate(errp, err);
error_prepend(errp, "required link 'ics' not found: ");

Note: I've just sent a patch to fix the same error in XICS :)

> +}
> +
> +xive->ics = ICS_BASE(obj);
> +
>  /* Allocate SBEs (State Bit Entry). 2 bits, so 4 entries per byte */
>  xive->sbe_size = DIV_ROUND_UP(xive->nr_irqs, 4);
>  xive->sbe = g_malloc0(xive->sbe_size);
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index b17dd4f17b0b..29112589b37f 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -24,6 +24,7 @@
>  typedef struct sPAPRXive sPAPRXive;
>  typedef struct XiveIVE XiveIVE;
>  typedef struct XiveEQ XiveEQ;
> +typedef struct ICSState ICSState;
>  
>  #define TYPE_SPAPR_XIVE "spapr-xive"
>  #define SPAPR_XIVE(obj) OBJECT_CHECK(sPAPRXive, (obj), TYPE_SPAPR_XIVE)
> @@ -35,6 +36,9 @@ struct sPAPRXive {
>  uint32_t nr_targets;
>  uint32_t nr_irqs;
>  
> +/* IRQ */
> +ICSState *ics;  /* XICS source inherited from the SPAPR machine */
> +
>  /* XIVE internal tables */
>  uint8_t  *sbe;
>  uint32_t sbe_size;



pgpQFXYGVYRH6.pgp
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] xics: fix several error leaks

[Greg Kurz]

If object_property_get_link() fails then it allocates an error, which
must be freed before returning. The error_get_pretty() function is
merely an accessor to the error message and doesn't free anything.

The error.h header indicates how to do it right:

 * Pass an existing error to the caller with the message modified:
 * error_propagate(errp, err);
 * error_prepend(errp, "Could not frobnicate '%s': ", name);

Signed-off-by: Greg Kurz 
---
 hw/intc/xics.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index a84ba51ad8ff..80c33be02e5e 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -306,8 +306,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
 
 obj = object_property_get_link(OBJECT(dev), ICP_PROP_XICS, );
 if (!obj) {
-error_setg(errp, "%s: required link '" ICP_PROP_XICS "' not found: %s",
-   __func__, error_get_pretty(err));
+error_propagate(errp, err);
+error_prepend(errp, "required link '" ICP_PROP_XICS "' not found: ");
 return;
 }
 
@@ -315,8 +315,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
 
 obj = object_property_get_link(OBJECT(dev), ICP_PROP_CPU, );
 if (!obj) {
-error_setg(errp, "%s: required link '" ICP_PROP_CPU "' not found: %s",
-   __func__, error_get_pretty(err));
+error_propagate(errp, err);
+error_prepend(errp, "required link '" ICP_PROP_CPU "' not found: ");
 return;
 }
 
@@ -641,8 +641,8 @@ static void ics_base_realize(DeviceState *dev, Error **errp)
 
 obj = object_property_get_link(OBJECT(dev), ICS_PROP_XICS, );
 if (!obj) {
-error_setg(errp, "%s: required link '" ICS_PROP_XICS "' not found: %s",
-   __func__, error_get_pretty(err));
+error_propagate(errp, err);
+error_prepend(errp, "required link '" ICS_PROP_XICS "' not found: ");
 return;
 }
 ics->xics = XICS_FABRIC(obj);

Re: [Qemu-devel] [PATCH] test-qga: add missing qemu-ga tool dependency

[Marc-André Lureau]

On Mon, Sep 11, 2017 at 11:02 PM Philippe Mathieu-Daudé 
wrote:

> this fixes running 'make check-unit' without running 'make all' beforehand:
>
> $ make check-unit
>   ...
>   GTESTER tests/test-qga
> **
> ERROR:tests/test-qga.c:73:fixture_setup: assertion failed (error == NULL):
> Failed to execute child process "/build/qemu/qemu-ga" (No such file or
> directory) (g-exec-error-quark, 8)
> make: *** [check-tests/test-qga] Error 1
>
> Signed-off-by: Philippe Mathieu-Daudé 
>

Reviewed-by: Marc-André Lureau 



> ---
> occured trying to split Travis jobs to run more/faster
>
>  tests/Makefile.include | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index fae5715e9c..59e027f6ea 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -832,7 +832,8 @@ endif
>  qtest-obj-y = tests/libqtest.o $(test-util-obj-y)
>  $(check-qtest-y): $(qtest-obj-y)
>
> -tests/test-qga: tests/test-qga.o $(qtest-obj-y)
> +tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
> +tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
>
>  SPEED = quick
>  GTESTER_OPTIONS = -k $(if $(V),--verbose,-q)
> --
> 2.14.1
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 05/22] qobject: Simplify qobject_from_jsonv()

[Eric Blake]

On 08/03/2017 08:25 PM, Eric Blake wrote:
> qobject_from_jsonv() was unusual in that it took a va_list*, instead
> of the more typical va_list; this was so that callers could pass
> NULL to avoid % interpolation.  While this works under the hood, it
> is awkward for callers, so move the magic into qjson.c rather than
> in the public interface, and finally improve the documentation of
> qobject_from_jsonf().
> 

> +/*
> + * va_list form of qobject_from_jsonf().
> + *
> + * IMPORTANT: This function aborts on error, thus it must not
> + * be used with untrusted arguments.
> + */
> +QObject *qobject_from_jsonv(const char *string, va_list ap)
> +{

Given your comments on vararg naming elsewhere in the series, I'm also
thinking this is a good chance to fix this to be named
qobject_from_vjsonf() (making it obvious it is the va_list form of
formatted json, and similar to printf/vprintf or my
qtest_startf/qtest_vstartf in my v7 preliminary cleanup series).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 3/5] tcg: move tcg-runtime to accel/tcg/

[Paolo Bonzini]

- Original Message -
> From: "Philippe Mathieu-Daudé" 
> To: "Paolo Bonzini" , "Richard Henderson" 
> , "Peter Crosthwaite"
> , "Thomas Huth" 
> Cc: "Philippe Mathieu-Daudé" , qemu-devel@nongnu.org
> Sent: Monday, September 11, 2017 11:33:26 PM
> Subject: [PATCH v2 3/5] tcg: move tcg-runtime to accel/tcg/
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Philippe Mathieu-Daudé 

To make this better, the "#ifndef CONFIG_SOFTMMU" part at the
end of this should be moved to user-exec.c.  Feel free to just
post a sixth patch for it.

Paolo

> ---
>  Makefile.target  | 2 +-
>  {tcg => accel/tcg}/tcg-runtime.h | 0
>  {tcg => accel/tcg}/tcg-runtime.c | 0
>  accel/tcg/Makefile.objs  | 1 +
>  4 files changed, 2 insertions(+), 1 deletion(-)
>  rename {tcg => accel/tcg}/tcg-runtime.h (100%)
>  rename {tcg => accel/tcg}/tcg-runtime.c (100%)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 520305b025..6361f957fb 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -94,7 +94,7 @@ all: $(PROGS) stap
>  obj-y += exec.o
>  obj-y += accel/
>  obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
> -obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/tcg-runtime.o
> +obj-$(CONFIG_TCG) += tcg/tcg-common.o
>  obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o
>  obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
>  obj-y += fpu/softfloat.o
> diff --git a/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
> similarity index 100%
> rename from tcg/tcg-runtime.h
> rename to accel/tcg/tcg-runtime.h
> diff --git a/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c
> similarity index 100%
> rename from tcg/tcg-runtime.c
> rename to accel/tcg/tcg-runtime.c
> diff --git a/accel/tcg/Makefile.objs b/accel/tcg/Makefile.objs
> index f2422d0fb3..228cd84fa4 100644
> --- a/accel/tcg/Makefile.objs
> +++ b/accel/tcg/Makefile.objs
> @@ -1,5 +1,6 @@
>  obj-$(CONFIG_SOFTMMU) += tcg-all.o
>  obj-$(CONFIG_SOFTMMU) += cputlb.o
> +obj-y += tcg-runtime.o
>  obj-y += cpu-exec.o cpu-exec-common.o translate-all.o
>  obj-y += translator.o
>  
> --
> 2.14.1
> 
> 

Re: [Qemu-devel] [PATCH v4 6/8] target/mips: Convert VM clock update prints to warn_report

[Philippe Mathieu-Daudé]

(Cc'ing Yongbok and Aurelien from get_maintainer.pl)

On 09/11/2017 04:52 PM, Alistair Francis wrote:

Convert the fprintf() messages in kvm_mips_update_state() to use
warn_report() as they aren't errors, but are just warnings.

Signed-off-by: Alistair Francis 


Reviewed-by: Philippe Mathieu-Daudé 


Cc: James Hogan 
---

  target/mips/kvm.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index a23aa438d2..3b7b1d962a 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -526,7 +526,7 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
  if (!cs->vcpu_dirty) {
  ret = kvm_mips_save_count(cs);
  if (ret < 0) {
-fprintf(stderr, "Failed saving count\n");
+warn_report("Failed saving count");
  }
  }
  } else {
@@ -535,14 +535,14 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
  ret = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_RESUME,
_resume);
  if (ret < 0) {
-fprintf(stderr, "Failed setting COUNT_RESUME\n");
+warn_report("Failed setting COUNT_RESUME");
  return;
  }
  
  if (!cs->vcpu_dirty) {

  ret = kvm_mips_restore_count(cs);
  if (ret < 0) {
-fprintf(stderr, "Failed restoring count\n");
+warn_report("Failed restoring count");
  }
  }
  }

Re: [Qemu-devel] [PATCH v4 6/8] target/mips: Convert VM clock update prints to warn_report

[James Hogan]

On Mon, Sep 11, 2017 at 12:52:59PM -0700, Alistair Francis wrote:
> Convert the fprintf() messages in kvm_mips_update_state() to use
> warn_report() as they aren't errors, but are just warnings.
> 
> Signed-off-by: Alistair Francis 
> Cc: James Hogan 

Acked-by: James Hogan 

Thanks
James

> ---
> 
>  target/mips/kvm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/mips/kvm.c b/target/mips/kvm.c
> index a23aa438d2..3b7b1d962a 100644
> --- a/target/mips/kvm.c
> +++ b/target/mips/kvm.c
> @@ -526,7 +526,7 @@ static void kvm_mips_update_state(void *opaque, int 
> running, RunState state)
>  if (!cs->vcpu_dirty) {
>  ret = kvm_mips_save_count(cs);
>  if (ret < 0) {
> -fprintf(stderr, "Failed saving count\n");
> +warn_report("Failed saving count");
>  }
>  }
>  } else {
> @@ -535,14 +535,14 @@ static void kvm_mips_update_state(void *opaque, int 
> running, RunState state)
>  ret = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_RESUME,
>_resume);
>  if (ret < 0) {
> -fprintf(stderr, "Failed setting COUNT_RESUME\n");
> +warn_report("Failed setting COUNT_RESUME");
>  return;
>  }
>  
>  if (!cs->vcpu_dirty) {
>  ret = kvm_mips_restore_count(cs);
>  if (ret < 0) {
> -fprintf(stderr, "Failed restoring count\n");
> +warn_report("Failed restoring count");
>  }
>  }
>  }
> -- 
> 2.11.0
> 


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH v2 5/5] tcg: restrict i386 regs definitions

[Kamil Rytarowski]

On 11.09.2017 23:33, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> cleaning while here :)
> 
>  accel/tcg/user-exec.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 2a975eaf69..484a3f5f8f 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -25,15 +25,6 @@
>  #include "exec/cpu_ldst.h"
>  #include "translate-all.h"
>  
> -#undef EAX
> -#undef ECX
> -#undef EDX
> -#undef EBX
> -#undef ESP
> -#undef EBP
> -#undef ESI
> -#undef EDI
> -#undef EIP
>  #ifdef __linux__
>  #include 
>  #endif
> @@ -131,6 +122,15 @@ static inline int handle_cpu_signal(uintptr_t pc, 
> unsigned long address,
>  }
>  
>  #if defined(__i386__)
> +#undef EAX
> +#undef ECX
> +#undef EDX
> +#undef EBX
> +#undef ESP
> +#undef EBP
> +#undef ESI
> +#undef EDI
> +#undef EIP
>  
>  #if defined(__NetBSD__)
>  #include 
> 

Why to move under i386?

SmartOS pollutes namespace with these symbols on x86_64.



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] tcg/ppc: disable atomic write check on ppc32

[Peter Maydell]

On 11 September 2017 at 21:49, Philippe Mathieu-Daudé  wrote:
> this fixes building for ppc64 on ppc32 (changed in 5964fca8a12c):
>
>   qemu/tcg/ppc/tcg-target.inc.c: In function 'tb_target_set_jmp_target':
>   qemu/include/qemu/compiler.h:86:30: error: static assertion failed: "not 
> expecting: sizeof(*(uint64_t *)jmp_addr) > ATOMIC_REG_SIZE"
>QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \
>^
>   qemu/tcg/ppc/tcg-target.inc.c:1377:9: note: in expansion of macro 
> 'atomic_set'
>atomic_set((uint64_t *)jmp_addr, pair);
>^
>
> Suggested-by: Richard Henderson 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> This fixes Shippable builds, see:
> https://app.shippable.com/github/qemu/qemu/runs/434/10/console
>
>  tcg/ppc/tcg-target.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
> index 21d764c102..0417901289 100644
> --- a/tcg/ppc/tcg-target.inc.c
> +++ b/tcg/ppc/tcg-target.inc.c
> @@ -1374,7 +1374,7 @@ void tb_target_set_jmp_target(uintptr_t tc_ptr, 
> uintptr_t jmp_addr,
>  pair = (uint64_t)i2 << 32 | i1;
>  #endif
>
> -atomic_set((uint64_t *)jmp_addr, pair);
> +atomic_set__nocheck((uint64_t *)jmp_addr, pair);
>  flush_icache_range(jmp_addr, jmp_addr + 8);
>  } else {
>  intptr_t diff = addr - jmp_addr;

Can you explain why this is the right thing? On the
face of it it looks correct to insist that we don't
try to do an atomic set of something that's bigger
than the host can actually handle...

thanks
-- PMM

[Qemu-devel] [PATCH v2 5/5] tcg: restrict i386 regs definitions

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
cleaning while here :)

 accel/tcg/user-exec.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 2a975eaf69..484a3f5f8f 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -25,15 +25,6 @@
 #include "exec/cpu_ldst.h"
 #include "translate-all.h"
 
-#undef EAX
-#undef ECX
-#undef EDX
-#undef EBX
-#undef ESP
-#undef EBP
-#undef ESI
-#undef EDI
-#undef EIP
 #ifdef __linux__
 #include 
 #endif
@@ -131,6 +122,15 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned 
long address,
 }
 
 #if defined(__i386__)
+#undef EAX
+#undef ECX
+#undef EDX
+#undef EBX
+#undef ESP
+#undef EBP
+#undef ESI
+#undef EDI
+#undef EIP
 
 #if defined(__NetBSD__)
 #include 
-- 
2.14.1

[Qemu-devel] [PATCH v2 4/5] tcg: move atomic_template.h to accel/tcg/

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Tested-by: Thomas Huth 
---
 atomic_template.h => accel/tcg/atomic_template.h | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename atomic_template.h => accel/tcg/atomic_template.h (100%)

diff --git a/atomic_template.h b/accel/tcg/atomic_template.h
similarity index 100%
rename from atomic_template.h
rename to accel/tcg/atomic_template.h
-- 
2.14.1

[Qemu-devel] [PATCH v2 3/5] tcg: move tcg-runtime to accel/tcg/

[Philippe Mathieu-Daudé]

Suggested-by: Paolo Bonzini 
Signed-off-by: Philippe Mathieu-Daudé 
---
 Makefile.target  | 2 +-
 {tcg => accel/tcg}/tcg-runtime.h | 0
 {tcg => accel/tcg}/tcg-runtime.c | 0
 accel/tcg/Makefile.objs  | 1 +
 4 files changed, 2 insertions(+), 1 deletion(-)
 rename {tcg => accel/tcg}/tcg-runtime.h (100%)
 rename {tcg => accel/tcg}/tcg-runtime.c (100%)

diff --git a/Makefile.target b/Makefile.target
index 520305b025..6361f957fb 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -94,7 +94,7 @@ all: $(PROGS) stap
 obj-y += exec.o
 obj-y += accel/
 obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
-obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/tcg-runtime.o
+obj-$(CONFIG_TCG) += tcg/tcg-common.o
 obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o
 obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
 obj-y += fpu/softfloat.o
diff --git a/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
similarity index 100%
rename from tcg/tcg-runtime.h
rename to accel/tcg/tcg-runtime.h
diff --git a/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c
similarity index 100%
rename from tcg/tcg-runtime.c
rename to accel/tcg/tcg-runtime.c
diff --git a/accel/tcg/Makefile.objs b/accel/tcg/Makefile.objs
index f2422d0fb3..228cd84fa4 100644
--- a/accel/tcg/Makefile.objs
+++ b/accel/tcg/Makefile.objs
@@ -1,5 +1,6 @@
 obj-$(CONFIG_SOFTMMU) += tcg-all.o
 obj-$(CONFIG_SOFTMMU) += cputlb.o
+obj-y += tcg-runtime.o
 obj-y += cpu-exec.o cpu-exec-common.o translate-all.o
 obj-y += translator.o
 
-- 
2.14.1

Re: [Qemu-devel] [PATCH v7 35/38] libqtest: Merge qtest_{mem, buf}{read, write}() with {mem, buf}{read, write}()

[Greg Kurz]

On Mon, 11 Sep 2017 12:20:19 -0500
Eric Blake  wrote:

> Maintaining two layers of libqtest APIs, one that takes an explicit
> QTestState object, and the other that uses the implicit global_qtest,
> is annoying.  In the interest of getting rid of global implicit
> state and having less code to maintain, merge:
>  qtest_memread()
>  qtest_bufread()
>  qtest_memwrite()
>  qtest_bufwrite()
> with their short counterparts.  All callers that previously
> used the short form now make it explicit that they are relying on
> global_qtest, and later patches can then clean things up to remove
> the global variable.
> 
> Signed-off-by: Eric Blake 
> ---
>  tests/libqtest.h | 69 
> ++--
>  tests/libqtest.c |  8 +++---
>  tests/ahci-test.c| 16 +--
>  tests/e1000e-test.c  | 16 ++-
>  tests/i440fx-test.c  |  8 +++---
>  tests/ide-test.c | 14 +-
>  tests/libqos/ahci.c  | 22 +++
>  tests/libqos/pci-pc.c|  4 +--
>  tests/libqos/pci-spapr.c |  4 +--
>  tests/megasas-test.c |  2 +-
>  tests/postcopy-test.c| 12 -
>  tests/virtio-9p-test.c   |  4 +--
>  tests/virtio-blk-test.c  | 16 +--
>  tests/virtio-net-test.c  |  6 ++---
>  tests/virtio-scsi-test.c |  4 +--
>  15 files changed, 78 insertions(+), 127 deletions(-)
> 
[...]
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index 07cbb7f37c..e2a14bc2ff 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -116,7 +116,7 @@ typedef struct {
> 
>  static void v9fs_memwrite(P9Req *req, const void *addr, size_t len)
>  {
> -memwrite(req->t_msg + req->t_off, addr, len);
> +memwrite(global_qtest, req->t_msg + req->t_off, addr, len);

Maybe use req->v9p->qs->qts instead of global_qtest ?

>  req->t_off += len;
>  }
> 
> @@ -132,7 +132,7 @@ static void v9fs_memrewind(P9Req *req, size_t len)
> 
>  static void v9fs_memread(P9Req *req, void *addr, size_t len)
>  {
> -memread(req->r_msg + req->r_off, addr, len);
> +memread(global_qtest, req->r_msg + req->r_off, addr, len);

Same here.

But this can be done in a follow-up patch. In case you don't respin, for
tests/virtio-9p-test.c:

Acked-by: Greg Kurz 


pgpRcIrgsXRnz.pgp
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 0/5] move user-exec, tcg-runtime, atomic_template.h to accel/tcg/

[Philippe Mathieu-Daudé]

As suggested by Paolo:

  That part of tcg/tcg-runtime.c probably should be moved to user-exec.c,
  and user-exec.c should in turn be in accel/tcg.

in http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03657.html

also update MAINTAINERS accordingly.

Regards,

Phil.

Philippe Mathieu-Daudé (4):
  tcg: move user-exec
  tcg: move tcg-runtime to accel/tcg/
  tcg: move atomic_template.h to accel/tcg/
  tcg: restrict i386 regs definitions

Thomas Huth (1):
  tcg: Move softmmu_template.h to the accel/tcg/ folder

 Makefile.target|  6 +++---
 atomic_template.h => accel/tcg/atomic_template.h   |  0
 softmmu_template.h => accel/tcg/softmmu_template.h |  0
 {tcg => accel/tcg}/tcg-runtime.h   |  0
 {tcg => accel/tcg}/tcg-runtime.c   |  0
 user-exec-stub.c => accel/tcg/user-exec-stub.c |  0
 user-exec.c => accel/tcg/user-exec.c   | 18 +-
 MAINTAINERS|  4 +---
 accel/tcg/Makefile.objs|  4 
 9 files changed, 17 insertions(+), 15 deletions(-)
 rename atomic_template.h => accel/tcg/atomic_template.h (100%)
 rename softmmu_template.h => accel/tcg/softmmu_template.h (100%)
 rename {tcg => accel/tcg}/tcg-runtime.h (100%)
 rename {tcg => accel/tcg}/tcg-runtime.c (100%)
 rename user-exec-stub.c => accel/tcg/user-exec-stub.c (100%)
 rename user-exec.c => accel/tcg/user-exec.c (100%)

Based-on: 20170911204936.5020-1-f4...@amsat.org

-- 
2.14.1

[Qemu-devel] [PATCH v2 2/5] tcg: move user-exec to accel/tcg/

[Philippe Mathieu-Daudé]

Suggested-by: Paolo Bonzini 
Signed-off-by: Philippe Mathieu-Daudé 
---
 Makefile.target| 4 ++--
 user-exec-stub.c => accel/tcg/user-exec-stub.c | 0
 user-exec.c => accel/tcg/user-exec.c   | 0
 MAINTAINERS| 3 +--
 accel/tcg/Makefile.objs| 3 +++
 5 files changed, 6 insertions(+), 4 deletions(-)
 rename user-exec-stub.c => accel/tcg/user-exec-stub.c (100%)
 rename user-exec.c => accel/tcg/user-exec.c (100%)

diff --git a/Makefile.target b/Makefile.target
index 7f42c45db8..520305b025 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -119,7 +119,7 @@ QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR) \
  -I$(SRC_PATH)/linux-user
 
 obj-y += linux-user/
-obj-y += gdbstub.o thunk.o user-exec.o user-exec-stub.o
+obj-y += gdbstub.o thunk.o
 
 endif #CONFIG_LINUX_USER
 
@@ -132,7 +132,7 @@ QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user 
-I$(SRC_PATH)/bsd-user/$(TARGET_ABI_DIR) \
 -I$(SRC_PATH)/bsd-user/$(HOST_VARIANT_DIR)
 
 obj-y += bsd-user/
-obj-y += gdbstub.o user-exec.o user-exec-stub.o
+obj-y += gdbstub.o
 
 endif #CONFIG_BSD_USER
 
diff --git a/user-exec-stub.c b/accel/tcg/user-exec-stub.c
similarity index 100%
rename from user-exec-stub.c
rename to accel/tcg/user-exec-stub.c
diff --git a/user-exec.c b/accel/tcg/user-exec.c
similarity index 100%
rename from user-exec.c
rename to accel/tcg/user-exec.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a6b7e1c19f..b2d4a4711f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1602,8 +1602,7 @@ Overall
 M: Riku Voipio 
 S: Maintained
 F: thunk.c
-F: user-exec.c
-F: user-exec-stub.c
+F: accel/tcg/user-exec*.c
 
 BSD user
 S: Orphan
diff --git a/accel/tcg/Makefile.objs b/accel/tcg/Makefile.objs
index 22642e6f75..f2422d0fb3 100644
--- a/accel/tcg/Makefile.objs
+++ b/accel/tcg/Makefile.objs
@@ -2,3 +2,6 @@ obj-$(CONFIG_SOFTMMU) += tcg-all.o
 obj-$(CONFIG_SOFTMMU) += cputlb.o
 obj-y += cpu-exec.o cpu-exec-common.o translate-all.o
 obj-y += translator.o
+
+obj-$(CONFIG_USER_ONLY) += user-exec.o
+obj-$(call lnot,$(CONFIG_SOFTMMU)) += user-exec-stub.o
-- 
2.14.1

[Qemu-devel] [PATCH v2 1/5] tcg: Move softmmu_template.h to the accel/tcg/ folder

[Philippe Mathieu-Daudé]

From: Thomas Huth 

The header is only used by accel/tcg/cputlb.c so we can
move it to the accel/tcg/ folder, too.

Signed-off-by: Thomas Huth 
[PMD: reword commit title to match series]
Signed-off-by: Philippe Mathieu-Daudé 
---
 softmmu_template.h => accel/tcg/softmmu_template.h | 0
 MAINTAINERS| 1 -
 2 files changed, 1 deletion(-)
 rename softmmu_template.h => accel/tcg/softmmu_template.h (100%)

diff --git a/softmmu_template.h b/accel/tcg/softmmu_template.h
similarity index 100%
rename from softmmu_template.h
rename to accel/tcg/softmmu_template.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 36eeb42d19..a6b7e1c19f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -86,7 +86,6 @@ M: Richard Henderson 
 S: Maintained
 F: cpus.c
 F: exec.c
-F: softmmu_template.h
 F: accel/tcg/
 F: include/exec/cpu*.h
 F: include/exec/exec-all.h
-- 
2.14.1

Re: [Qemu-devel] [PATCH v7 17/38] libqos: Use explicit QTestState for remaining libqos operations

[Greg Kurz]

On Mon, 11 Sep 2017 12:20:01 -0500
Eric Blake  wrote:

> Drop one more client of global_qtest by teaching all remaining
> libqos stragglers to pass in an explicit QTestState.  Change the
> setting of global_qtest from being implicit in libqos' call to
> qtest_start() to instead be explicit in all clients that are
> still relying on global_qtest.
> 
> Note that qmp_execute() can be greatly simplified in the process,
> and that we also get rid of interpolation of a JSON string into a
> temporary variable when qtest_qmp() can do it more reliably.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v7: split libqos changes from test-ahci, catch more spots
> ---

For tests/virtio-9p-test.c.

Acked-by: Greg Kurz 

>  tests/ahci-test.c |  1 +
>  tests/ivshmem-test.c  |  1 +
>  tests/libqos/libqos-pc.c  |  2 +-
>  tests/libqos/libqos.c | 30 ++
>  tests/megasas-test.c  |  5 -
>  tests/rtas-test.c |  1 +
>  tests/usb-hcd-uhci-test.c |  1 +
>  C|  1 +
>  tests/virtio-blk-test.c   |  1 +
>  tests/virtio-net-test.c   | 15 +--
>  tests/virtio-scsi-test.c  | 16 +---
>  11 files changed, 39 insertions(+), 35 deletions(-)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index c94d1bd712..d6696cc370 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -157,6 +157,7 @@ static AHCIQState *ahci_vboot(const char *cli, va_list ap)
> 
>  s = g_malloc0(sizeof(AHCIQState));
>  s->parent = qtest_pc_vboot(cli, ap);
> +global_qtest = s->parent->qts;
>  alloc_set_flags(s->parent->alloc, ALLOC_LEAK_ASSERT);
> 
>  /* Verify that we have an AHCI device present. */
> diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> index d35c922254..d8f8530a4d 100644
> --- a/tests/ivshmem-test.c
> +++ b/tests/ivshmem-test.c
> @@ -131,6 +131,7 @@ static void setup_vm_cmd(IVState *s, const char *cmd, 
> bool msix)
>  g_printerr("ivshmem-test tests are only available on x86 or 
> ppc64\n");
>  exit(EXIT_FAILURE);
>  }
> +global_qtest = s->qs->qts;
>  s->dev = get_device(s->qs->pcibus);
> 
>  s->reg_bar = qpci_iomap(s->dev, 0, );
> diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
> index b554758802..a9c1aceaa7 100644
> --- a/tests/libqos/libqos-pc.c
> +++ b/tests/libqos/libqos-pc.c
> @@ -25,7 +25,7 @@ QOSState *qtest_pc_boot(const char *cmdline_fmt, ...)
>  qs = qtest_vboot(_ops, cmdline_fmt, ap);
>  va_end(ap);
> 
> -qtest_irq_intercept_in(global_qtest, "ioapic");
> +qtest_irq_intercept_in(qs->qts, "ioapic");
> 
>  return qs;
>  }
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index 40018b9c07..9798e1d027 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -20,7 +20,7 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, 
> va_list ap)
>  QOSState *qs = g_new0(QOSState, 1);
> 
>  cmdline = g_strdup_vprintf(cmdline_fmt, ap);
> -qs->qts = qtest_start(cmdline);
> +qs->qts = qtest_init(cmdline);
>  qs->ops = ops;
>  if (ops) {
>  qs->alloc = ops->init_allocator(qs->qts, ALLOC_NO_FLAGS);
> @@ -80,29 +80,21 @@ void set_context(QOSState *s)
>  global_qtest = s->qts;
>  }
> 
> -static QDict *qmp_execute(const char *command)
> +static QDict *qmp_execute(QTestState *qts, const char *command)
>  {
> -char *fmt;
> -QDict *rsp;
> -
> -fmt = g_strdup_printf("{ 'execute': '%s' }", command);
> -rsp = qmp(fmt);
> -g_free(fmt);
> -
> -return rsp;
> +return qtest_qmp(qts, "{ 'execute': %s }", command);
>  }
> 
>  void migrate(QOSState *from, QOSState *to, const char *uri)
>  {
>  const char *st;
> -char *s;
>  QDict *rsp, *sub;
>  bool running;
> 
>  set_context(from);
> 
>  /* Is the machine currently running? */
> -rsp = qmp_execute("query-status");
> +rsp = qmp_execute(from->qts, "query-status");
>  g_assert(qdict_haskey(rsp, "return"));
>  sub = qdict_get_qdict(rsp, "return");
>  g_assert(qdict_haskey(sub, "running"));
> @@ -110,30 +102,28 @@ void migrate(QOSState *from, QOSState *to, const char 
> *uri)
>  QDECREF(rsp);
> 
>  /* Issue the migrate command. */
> -s = g_strdup_printf("{ 'execute': 'migrate',"
> -"'arguments': { 'uri': '%s' } }",
> -uri);
> -rsp = qmp(s);
> -g_free(s);
> +rsp = qtest_qmp(from->qts,
> +"{ 'execute': 'migrate', 'arguments': { 'uri': %s }}",
> +uri);
>  g_assert(qdict_haskey(rsp, "return"));
>  QDECREF(rsp);
> 
>  /* Wait for STOP event, but only if we were running: */
>  if (running) {
> -qmp_eventwait("STOP");
> +qtest_qmp_eventwait(from->qts, "STOP");
>  }
> 
>  /* If we were running, we can wait for an event. */
>  if (running) {
>  migrate_allocator(from->alloc, to->alloc);
> 

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

[Eric Blake]

We already have several files that knowingly require assert()
to work, sometimes because refactoring the code for proper
error handling has not been tackled yet; there are probably
other files that have a similar situation but with no comments
documenting the same.  In fact, we have places in migration
that handle untrusted input with assertions, where disabling
the assertions risks a worse security hole than the current
behavior of losing the guest to SIGABRT when migration fails
because of the assertion.  Promote our current per-file
safety-valve to instead be project-wide, and expand it to also
cover glib's g_assert().

Note that we do NOT want to encourage 'assert(side-effects);'
(that is a bad practice that prevents copy-and-paste of code to
other projects that CAN disable assertions; plus it costs
unnecessary reviewer mental cycles to remember whether a project
special-cases the crippling of asserts); and we would LIKE to
fix migration to not rely on asserts (but that takes a big code
audit).  But in the meantime, we DO want to send a message
that anyone that disables assertions has to tweak code in order
to compile, making it obvious that they are taking on additional
risk that we are not going to support.  At the same time, leave
comments mentioning NDEBUG in files that we know still need to
be scrubbed, so there is at least something to grep for.

It would be possible to come up with some other mechanism for
doing runtime checking by default, but which does not abort
the program on failure, while leaving side effects in place
(unlike how crippling assert() avoids even the side effects),
perhaps under the name q_verify(); but it was not deemed worth
the effort (developers should not have to learn a replacement
when the standard C macro works just fine, and it would be a lot
of churn for little gain).  The patch specifically uses #error
rather than #warn so that a user is forced to tweak the header
to acknowledge the issue, even when not using a -Werror
compilation.

Signed-off-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 

---
v2: promote from RFC, leave NDEBUG mentioned in comments, beef up
commit message

I'm not sure if this would fit best as a top-level patch applied
directly by Peter, or if it would fit through qemu-trivial, or if
it belongs to Paolo's misc queue.  But consensus seemed to be that
we are okay with having it.

---
 include/qemu/osdep.h | 16 
 hw/scsi/mptsas.c |  6 ++
 hw/virtio/virtio.c   |  6 ++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 6855b94bbf..99666383b2 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -107,6 +107,22 @@ extern int daemon(int, int);
 #include "glib-compat.h"
 #include "qemu/typedefs.h"

+/*
+ * We have a lot of unaudited code that may fail in strange ways, or
+ * even be a security risk during migration, if you disable assertions
+ * at compile-time.  You may comment out these safety checks if you
+ * absolutely want to disable assertion overhead, but it is not
+ * supported upstream so the risk is all yours.  Meanwhile, please
+ * submit patches to remove any side-effects inside an assertion, or
+ * fixing error handling that should use Error instead of assert.
+ */
+#ifdef NDEBUG
+#error building with NDEBUG is not supported
+#endif
+#ifdef G_DISABLE_ASSERT
+#error building with G_DISABLE_ASSERT is not supported
+#endif
+
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
 #endif
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 765ab53c34..9962286bd6 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -1236,11 +1236,9 @@ static void *mptsas_load_request(QEMUFile *f, 
SCSIRequest *sreq)
 n = qemu_get_be32(f);
 /* TODO: add a way for SCSIBusInfo's load_request to fail,
  * and fail migration instead of asserting here.
- * When we do, we might be able to re-enable NDEBUG below.
+ * This is just one thing (there are probably more) that must be
+ * fixed before we can allow NDEBUG compilation.
  */
-#ifdef NDEBUG
-#error building with NDEBUG is not supported
-#endif
 assert(n >= 0);

 pci_dma_sglist_init(>qsg, pci, n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 464947f76d..3129d25c00 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1025,11 +1025,9 @@ void *qemu_get_virtqueue_element(VirtIODevice *vdev, 
QEMUFile *f, size_t sz)

 /* TODO: teach all callers that this can fail, and return failure instead
  * of asserting here.
- * When we do, we might be able to re-enable NDEBUG below.
+ * This is just one thing (there are probably more) that must be
+ * fixed before we can allow NDEBUG compilation.
  */
-#ifdef NDEBUG
-#error building with NDEBUG is not supported
-#endif
 assert(ARRAY_SIZE(data.in_addr) >= data.in_num);
 assert(ARRAY_SIZE(data.out_addr) >= 

[Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls

[Greg Kurz]

This patch removes the qdev_get_machine() calls that are made
in spapr_cpu_core.c in situations where we can get an existing
pointer for the MachineState by either passing it as an argument
to the function or by using other already available pointers.

Credits to Daniel Henrique Barboza for the idea and the changelog
text.

Signed-off-by: Greg Kurz 
---
v2: - fixed typo in spapr_cpu_reset()
---
 hw/ppc/spapr_cpu_core.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index dc9df0d393d1..0f32532abe99 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
 
 static void spapr_cpu_reset(void *opaque)
 {
-sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 PowerPCCPU *cpu = opaque;
+sPAPRMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);
 CPUState *cs = CPU(cpu);
 CPUPPCState *env = >env;
 
@@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, 
Error **errp)
 g_free(sc->threads);
 }
 
-static void spapr_cpu_core_realize_child(Object *child, Error **errp)
+static void spapr_cpu_core_realize_child(Object *child,
+ sPAPRMachineState *spapr, Error 
**errp)
 {
 Error *local_err = NULL;
-sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 CPUState *cs = CPU(child);
 PowerPCCPU *cpu = POWERPC_CPU(cs);
 Object *obj;
@@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error 
**errp)
 for (j = 0; j < cc->nr_threads; j++) {
 obj = sc->threads + j * size;
 
-spapr_cpu_core_realize_child(obj, _err);
+spapr_cpu_core_realize_child(obj, spapr, _err);
 if (local_err) {
 goto err;
 }

Re: [Qemu-devel] [PATCH] spapr_cpu_core: cleaning up qdev_get_machine() calls

[Greg Kurz]

On Mon, 11 Sep 2017 22:52:52 +0200
Greg Kurz  wrote:

> This patch removes the qdev_get_machine() calls that are made
> in spapr_cpu_core.c in situations where we can get an existing
> pointer for the MachineState by either passing it as an argument
> to the function or by using other already available pointers.
> 
> Credits to Daniel Henrique Barboza for the idea and the changelog
> text.
> 
> Signed-off-by: Greg Kurz 
> ---
>  hw/ppc/spapr_cpu_core.c |8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index dc9df0d393d1..e26279116736 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>  
>  static void spapr_cpu_reset(void *opaque)
>  {
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  PowerPCCPU *cpu = opaque;
> +sPAPRMachineState *spapr = SPAPR_MACHINE(>vhyp);
^
  Ouch! :-\

I'll send a v2.

>  CPUState *cs = CPU(cpu);
>  CPUPPCState *env = >env;
>  
> @@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState 
> *dev, Error **errp)
>  g_free(sc->threads);
>  }
>  
> -static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> +static void spapr_cpu_core_realize_child(Object *child,
> + sPAPRMachineState *spapr, Error 
> **errp)
>  {
>  Error *local_err = NULL;
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  CPUState *cs = CPU(child);
>  PowerPCCPU *cpu = POWERPC_CPU(cs);
>  Object *obj;
> @@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  for (j = 0; j < cc->nr_threads; j++) {
>  obj = sc->threads + j * size;
>  
> -spapr_cpu_core_realize_child(obj, _err);
> +spapr_cpu_core_realize_child(obj, spapr, _err);
>  if (local_err) {
>  goto err;
>  }
> 
> 



pgpgVuNtKS41I.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Philippe Mathieu-Daudé]

Hi Stefan,

On 09/11/2017 05:26 PM, Stefan Weil wrote:> I had an assertion because 
of a missing qemu-ga

when I‌ only had run "make check", so there is a
dependency (perhaps check: tools) missing.

Now "make check" is running fine with all targets
(at least until check-qtest-ppc64). Sorry, I have
to finish for today.


I had this patch in my travis-ci queue, here extracted alone:
https://patchwork.kernel.org/patch/9948117/

Regards,

Phil.

[Qemu-devel] [PATCH] Drop gld linker usage on SunOS

[Kamil Rytarowski]

This is required to be removed on SmartOS (Illumos).
As of now there are no alternative supported SunOS distributions.

Signed-off-by: Kamil Rytarowski 
---
 configure | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure b/configure
index fd7e3a5e81..3dc0a71984 100755
--- a/configure
+++ b/configure
@@ -746,7 +746,6 @@ SunOS)
   solaris="yes"
   make="${MAKE-gmake}"
   install="${INSTALL-ginstall}"
-  ld="gld"
   smbd="${SMBD-/usr/sfw/sbin/smbd}"
   if test -f /usr/include/sys/soundcard.h ; then
 audio_drv_list="oss"
-- 
2.14.1

[Qemu-devel] [PATCH] test-qga: add missing qemu-ga tool dependency

[Philippe Mathieu-Daudé]

this fixes running 'make check-unit' without running 'make all' beforehand:

$ make check-unit
  ...
  GTESTER tests/test-qga
**
ERROR:tests/test-qga.c:73:fixture_setup: assertion failed (error == NULL): 
Failed to execute child process "/build/qemu/qemu-ga" (No such file or 
directory) (g-exec-error-quark, 8)
make: *** [check-tests/test-qga] Error 1

Signed-off-by: Philippe Mathieu-Daudé 
---
occured trying to split Travis jobs to run more/faster

 tests/Makefile.include | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index fae5715e9c..59e027f6ea 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -832,7 +832,8 @@ endif
 qtest-obj-y = tests/libqtest.o $(test-util-obj-y)
 $(check-qtest-y): $(qtest-obj-y)
 
-tests/test-qga: tests/test-qga.o $(qtest-obj-y)
+tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
+tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
 
 SPEED = quick
 GTESTER_OPTIONS = -k $(if $(V),--verbose,-q)
-- 
2.14.1

[Qemu-devel] [PATCH] spapr_cpu_core: fail gracefully with non-pseries machine types

[Greg Kurz]

Since commit 7cca3e466eb0 ("ppc: spapr: Move VCPU ID calculation into
sPAPR"), QEMU aborts when started with a *-spapr-cpu-core device and
a non-pseries machine.

Let's rely on the already existing call to object_dynamic_cast() instead
of using the SPAPR_MACHINE() macro.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_cpu_core.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index d04d1e0d8164..dc9df0d393d1 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -201,7 +201,7 @@ error:
 
 static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
 {
-sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+sPAPRMachineState *spapr;
 sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
 sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
 CPUCore *cc = CPU_CORE(OBJECT(dev));
@@ -211,7 +211,8 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error 
**errp)
 void *obj;
 int i, j;
 
-if (!object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE)) {
+spapr = (sPAPRMachineState *) qdev_get_machine();
+if (!object_dynamic_cast((Object *) spapr, TYPE_SPAPR_MACHINE)) {
 error_setg(errp, "spapr-cpu-core needs a pseries machine");
 return;
 }

[Qemu-devel] [PATCH] spapr_cpu_core: cleaning up qdev_get_machine() calls

[Greg Kurz]

This patch removes the qdev_get_machine() calls that are made
in spapr_cpu_core.c in situations where we can get an existing
pointer for the MachineState by either passing it as an argument
to the function or by using other already available pointers.

Credits to Daniel Henrique Barboza for the idea and the changelog
text.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_cpu_core.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index dc9df0d393d1..e26279116736 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
 
 static void spapr_cpu_reset(void *opaque)
 {
-sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 PowerPCCPU *cpu = opaque;
+sPAPRMachineState *spapr = SPAPR_MACHINE(>vhyp);
 CPUState *cs = CPU(cpu);
 CPUPPCState *env = >env;
 
@@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, 
Error **errp)
 g_free(sc->threads);
 }
 
-static void spapr_cpu_core_realize_child(Object *child, Error **errp)
+static void spapr_cpu_core_realize_child(Object *child,
+ sPAPRMachineState *spapr, Error 
**errp)
 {
 Error *local_err = NULL;
-sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 CPUState *cs = CPU(child);
 PowerPCCPU *cpu = POWERPC_CPU(cs);
 Object *obj;
@@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error 
**errp)
 for (j = 0; j < cc->nr_threads; j++) {
 obj = sc->threads + j * size;
 
-spapr_cpu_core_realize_child(obj, _err);
+spapr_cpu_core_realize_child(obj, spapr, _err);
 if (local_err) {
 goto err;
 }

[Qemu-devel] [Bug 1715700] Re: Windows 7 guest won't boot on qemu 2.10 (works on 2.9)

[Aleksei Kovura]

Bisected. Good thing Igor is already here :)

208fa0e43645edd0b0d8f838857dfc79daff40a8 is the first bad commit
commit 208fa0e43645edd0b0d8f838857dfc79daff40a8
Author: Igor Mammedov 
Date:   Fri Jul 28 11:09:05 2017 +0200

pc: make 'pc.rom' readonly when machine has PCI enabled

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

Title:
  Windows 7 guest won't boot on qemu 2.10 (works on 2.9)

Status in QEMU:
  New

Bug description:
  Qemu version: 2.10 stable.
  Guest: Windows 7 SP1 x64, virtio drivers are already installed in the guest.
  Command line:
  qemu-system-x86_64 \
  -nodefaults \
  -nodefconfig \
  -machine type=q35,accel=kvm \
  -enable-kvm \
  -cpu host \
  -m 2048 \
  -vga virtio \
  -boot menu=on \
  -smbios file=/path/dmidecode_BIOS.bin \
  -acpitable file=/path/acpi_slic.bin \
  -bios /path/OVMF_CODE.fd \
  -net none \
  -drive if=virtio,media=disk,file=/media/win7.qcow2 \
  -device pcie-root-port \
  -device ich9-usb-ehci1 \
  -device ich9-usb-uhci1 \
  -device ich9-usb-uhci2 \
  -device ich9-usb-uhci3

  Windows hangs at boot with waving flag screen (flag doesn't freeze,
  keeps waving indefinitely). Same command line boots fine with Qemu
  2.9. I tried changing machine type to pc-q35-2.9 - same result.

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

[Qemu-devel] [PATCH] tcg/ppc: disable atomic write check on ppc32

[Philippe Mathieu-Daudé]

this fixes building for ppc64 on ppc32 (changed in 5964fca8a12c):

  qemu/tcg/ppc/tcg-target.inc.c: In function 'tb_target_set_jmp_target':
  qemu/include/qemu/compiler.h:86:30: error: static assertion failed: "not 
expecting: sizeof(*(uint64_t *)jmp_addr) > ATOMIC_REG_SIZE"
   QEMU_BUILD_BUG_ON(sizeof(*ptr) > ATOMIC_REG_SIZE); \
   ^
  qemu/tcg/ppc/tcg-target.inc.c:1377:9: note: in expansion of macro 'atomic_set'
   atomic_set((uint64_t *)jmp_addr, pair);
   ^

Suggested-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
This fixes Shippable builds, see:
https://app.shippable.com/github/qemu/qemu/runs/434/10/console

 tcg/ppc/tcg-target.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 21d764c102..0417901289 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -1374,7 +1374,7 @@ void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t 
jmp_addr,
 pair = (uint64_t)i2 << 32 | i1;
 #endif
 
-atomic_set((uint64_t *)jmp_addr, pair);
+atomic_set__nocheck((uint64_t *)jmp_addr, pair);
 flush_icache_range(jmp_addr, jmp_addr + 8);
 } else {
 intptr_t diff = addr - jmp_addr;
-- 
2.14.1

[Qemu-devel] [Bug 1615823] Re: Windows 10 reports no compatible TPM found yet device manager shows it?

[Nelson Chan via Qemu-devel]

My laptop has a TPM 1.2 chip made by IFX (dmesg: tpm_tis 00:07: 1.2 TPM
(device-id 0x1B, rev-id 16))

I couldn't get it to work in libvirt (I am running ubuntu 17.04) until I
upgraded my Windows 10 to version 1607. I needed to change the CPU to
"core2duo" first before I could apply the version 1607 patch (if you
don't do that, you can never apply the patch). After applying this
Windows patch, Windows can detect and use the TPM device assigned to it
successfully.

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

Title:
  Windows 10 reports no compatible TPM found yet device manager shows
  it?

Status in QEMU:
  New

Bug description:
  Ubuntu 16.04 with stock kvm, libvirt, ovmf
  Qemu 2.5 installed from stock ubuntu ppa
  Qemu 2.6.1 built from tarball.
  Qemu 2.7.0-rc4 built from tarball.

  Windows 10 guest reports a TPM device is installed and the driver
  functional under Device Manager-->Security Devices.  TPM Administrator
  however advises no compatible TPM chip can be found.

  Qemu 2.5 is buggy and prevents the guest loading the TPM driver, this
  was addressed by
  
http://git.qemu.org/?p=qemu.git;a=commit;h=2b1c2e8e5f1990f0a201a8cbf9d366fca60f4aa8

  Have tested the below cmd out on both qemu-2.6.1 and qemu-2.7.0-rc4,
  both suffer the same problem.  My TPM is most certainly compatible as
  installing Win10Pro onto the same host as bare metal provides me the
  desired and expected functionality aka Bitlocker and TPM Administrator
  work.

  sudo ./qemu-system-x86_64 \
  -enable-kvm \
  -machine q35 \
  -cpu host \
  -m 4096 \
  -smp 4,sockets=1,cores=2,threads=2 \
  -device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
  -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
  -device 
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vgamem_mb=16,bus=pcie.0,addr=0x2
 \
  -drive file=/usr/share/qemu/OVMF.fd,if=pflash,format=raw,unit=0,readonly=on \
  -drive file=/mnt/120GB_SSD/wintpm_VARS.fd,if=pflash,format=raw,unit=1 \
  -drive 
file=/mnt/120GB_SSD/wintpm.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \
  -device 
virtio-blk-pci,scsi=off,bus=pci.2,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=2
 \
  -drive file="/mnt/share/Filestorage/Images/Microsoft Windows 10 Pro 
x64.iso",format=raw,if=none,media=cdrom,id=drive-sata0-0-0,readonly=on \
  -device ide-cd,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
  -drive 
file=/mnt/share/Filestorage/Images/virtio-win-0.1.117.iso,format=raw,if=none,media=cdrom,id=drive-sata0-0-1,readonly=on
 \
  -device ide-cd,bus=ide.1,drive=drive-sata0-0-1,id=sata0-0-1 \
  -tpmdev 
passthrough,id=tpm-tpm0,path=/dev/tpm0,cancel-path=/sys/class/tpm/tpm0/device/cancel
 \
  -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0

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

[Qemu-devel] [PATCH v2] Replace round_page() with TARGET_PAGE_ALIGN()

[Kamil Rytarowski]

This change fixes conflict with the DragonFly BSD headers.

Signed-off-by: Kamil Rytarowski 
---
 hw/ppc/mac_newworld.c | 11 +++
 hw/ppc/mac_oldworld.c | 11 +++
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index d466634997..51bf83019b 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -124,11 +124,6 @@ static uint64_t translate_kernel_address(void *opaque, 
uint64_t addr)
 return (addr & 0x0fff) + KERNEL_LOAD_ADDR;
 }
 
-static hwaddr round_page(hwaddr addr)
-{
-return (addr + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
-}
-
 static void ppc_core99_reset(void *opaque)
 {
 PowerPCCPU *cpu = opaque;
@@ -256,7 +251,7 @@ static void ppc_core99_init(MachineState *machine)
 }
 /* load initrd */
 if (initrd_filename) {
-initrd_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + 
KERNEL_GAP);
 initrd_size = load_image_targphys(initrd_filename, initrd_base,
   ram_size - initrd_base);
 if (initrd_size < 0) {
@@ -264,11 +259,11 @@ static void ppc_core99_init(MachineState *machine)
  initrd_filename);
 exit(1);
 }
-cmdline_base = round_page(initrd_base + initrd_size);
+cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
 } else {
 initrd_base = 0;
 initrd_size = 0;
-cmdline_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + 
KERNEL_GAP);
 }
 ppc_boot_device = 'm';
 } else {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index fcac399562..feb31911ca 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -66,11 +66,6 @@ static uint64_t translate_kernel_address(void *opaque, 
uint64_t addr)
 return (addr & 0x0fff) + KERNEL_LOAD_ADDR;
 }
 
-static hwaddr round_page(hwaddr addr)
-{
-return (addr + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
-}
-
 static void ppc_heathrow_reset(void *opaque)
 {
 PowerPCCPU *cpu = opaque;
@@ -191,7 +186,7 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 /* load initrd */
 if (initrd_filename) {
-initrd_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + 
KERNEL_GAP);
 initrd_size = load_image_targphys(initrd_filename, initrd_base,
   ram_size - initrd_base);
 if (initrd_size < 0) {
@@ -199,11 +194,11 @@ static void ppc_heathrow_init(MachineState *machine)
  initrd_filename);
 exit(1);
 }
-cmdline_base = round_page(initrd_base + initrd_size);
+cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
 } else {
 initrd_base = 0;
 initrd_size = 0;
-cmdline_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + 
KERNEL_GAP);
 }
 ppc_boot_device = 'm';
 } else {
-- 
2.14.1

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Stefan Weil]

Am 11.09.2017 um 20:24 schrieb Peter Maydell:
> I've also turned on a tci compile check on my pre-merge tests.
> (It doesn't pass "make check" for me, though...)

Did you run "make" before running "make check"?

I had an assertion because of a missing qemu-ga
when I‌ only had run "make check", so there is a
dependency (perhaps check: tools) missing.

Now "make check" is running fine with all targets
(at least until check-qtest-ppc64). Sorry, I have
to finish for today.

Stefan

Re: [Qemu-devel] [PATCH] Rename round_page() to round_pageq()

[Kamil Rytarowski]

On 11.09.2017 22:16, Peter Maydell wrote:
> On 11 September 2017 at 21:00, Kamil Rytarowski  wrote:
>> This change fixes conflict with the DragonFly BSD headers.
>>
>> Signed-off-by: Kamil Rytarowski 
> 
> What's the rationale for calling it round_pageq()
> in particular?
> 
> thanks
> -- PMM
> 

It might be a bad taste, but q stands for qemu.. I'm going to send a
better patch soon with a switch to TARGET_PAGE_ALIGN().



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Rename round_page() to round_pageq()

[Peter Maydell]

On 11 September 2017 at 21:00, Kamil Rytarowski  wrote:
> This change fixes conflict with the DragonFly BSD headers.
>
> Signed-off-by: Kamil Rytarowski 

What's the rationale for calling it round_pageq()
in particular?

thanks
-- PMM

[Qemu-devel] [PATCH] Rename round_page() to round_pageq()

[Kamil Rytarowski]

This change fixes conflict with the DragonFly BSD headers.

Signed-off-by: Kamil Rytarowski 
---
 hw/ppc/mac_newworld.c | 8 
 hw/ppc/mac_oldworld.c | 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index d466634997..b9d4bca74f 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -124,7 +124,7 @@ static uint64_t translate_kernel_address(void *opaque, 
uint64_t addr)
 return (addr & 0x0fff) + KERNEL_LOAD_ADDR;
 }
 
-static hwaddr round_page(hwaddr addr)
+static hwaddr round_pageq(hwaddr addr)
 {
 return (addr + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
 }
@@ -256,7 +256,7 @@ static void ppc_core99_init(MachineState *machine)
 }
 /* load initrd */
 if (initrd_filename) {
-initrd_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+initrd_base = round_pageq(kernel_base + kernel_size + KERNEL_GAP);
 initrd_size = load_image_targphys(initrd_filename, initrd_base,
   ram_size - initrd_base);
 if (initrd_size < 0) {
@@ -264,11 +264,11 @@ static void ppc_core99_init(MachineState *machine)
  initrd_filename);
 exit(1);
 }
-cmdline_base = round_page(initrd_base + initrd_size);
+cmdline_base = round_pageq(initrd_base + initrd_size);
 } else {
 initrd_base = 0;
 initrd_size = 0;
-cmdline_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+cmdline_base = round_pageq(kernel_base + kernel_size + KERNEL_GAP);
 }
 ppc_boot_device = 'm';
 } else {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index fcac399562..50fefb339b 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -66,7 +66,7 @@ static uint64_t translate_kernel_address(void *opaque, 
uint64_t addr)
 return (addr & 0x0fff) + KERNEL_LOAD_ADDR;
 }
 
-static hwaddr round_page(hwaddr addr)
+static hwaddr round_pageq(hwaddr addr)
 {
 return (addr + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
 }
@@ -191,7 +191,7 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 /* load initrd */
 if (initrd_filename) {
-initrd_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+initrd_base = round_pageq(kernel_base + kernel_size + KERNEL_GAP);
 initrd_size = load_image_targphys(initrd_filename, initrd_base,
   ram_size - initrd_base);
 if (initrd_size < 0) {
@@ -199,11 +199,11 @@ static void ppc_heathrow_init(MachineState *machine)
  initrd_filename);
 exit(1);
 }
-cmdline_base = round_page(initrd_base + initrd_size);
+cmdline_base = round_pageq(initrd_base + initrd_size);
 } else {
 initrd_base = 0;
 initrd_size = 0;
-cmdline_base = round_page(kernel_base + kernel_size + KERNEL_GAP);
+cmdline_base = round_pageq(kernel_base + kernel_size + KERNEL_GAP);
 }
 ppc_boot_device = 'm';
 } else {
-- 
2.14.1

Re: [Qemu-devel] [PATCH v1 0/2] Remove libqemustub.a in order to improve error

[Alistair Francis]

On Mon, Sep 11, 2017 at 9:36 AM, Alistair Francis
 wrote:
> On Mon, Sep 11, 2017 at 2:57 AM, Paolo Bonzini  wrote:
>> On 18/08/2017 20:47, Alistair Francis wrote:
>>> As discussed with Paolo and Markus let's combine libqemustub.a into
>>> libqemuutil.a to avoid circular dependencies.
>>>
>>> Alistair Francis (2):
>>>   Makefile: Remove libqemustub.a
>>>   Convert remaining single line fprintf() to warn_report()
>>>
>>>  Makefile|  7 +++
>>>  Makefile.target |  2 +-
>>>  docs/devel/build-system.txt | 14 +-
>>>  tests/Makefile.include  |  8 
>>>  util/cutils.c   |  3 ++-
>>>  5 files changed, 15 insertions(+), 19 deletions(-)
>>>
>>
>> Queued, thanks (with Philippe's adjustments to the comments).
>
> Thanks Paolo.
>
> I actually sent a new version with Philippe's comments, it's included
> as part of the "More warning reporting fixed" series.
>
> Version 5 of that has been reviewed by Markus, I will send it out
> today. Can you take that instead?

Ah, I meant version 4.

The series should be very close to being merged. If you want that
whole series can go in or you can take this and I can rebase the
larger series. Whatever is easiest for you.

Thanks,
Alistair

>
> Thanks,
> Alistair
>
>>
>> Paolo

Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] Discover openpty(3) dynamically in configure

[Kamil Rytarowski]

On 11.09.2017 19:16, Kamil Rytarowski wrote:
> openpty(3) might:
>  - exists in libc (OSX)
>  - exists in libutil (GNU, BSD)
>  - does not exist (SmartOS)
> 
> Add a function to discover this function in the ./configure script.
> Add new config types: CONFIG_OPENPTY_LIBC and CONFIG_OPENPTY_LIBUTIL,
> respectively defined when openpts(3) links with -lc or -lutil.
> 
> Replace the condition adding -lutil in tests (for openpty(3)) from
> CONFIG_POSIX to CONFIG_OPENPTY_LIBUTIL.
> 
> Replace the fallback openpty(3) impelementation comment from Solaris
> to SmartOS. Solaris is EOL'ed and it's confirmed that it does not work
> on Oracle Solaris.

Signed-off-by: Kamil Rytarowski 

> ---
>  configure  | 25 +
>  tests/Makefile.include |  2 +-
>  util/qemu-openpty.c|  4 ++--
>  3 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index fd7e3a5e81..a614adcd29 100755
> --- a/configure
> +++ b/configure
> @@ -3819,6 +3819,25 @@ EOF
>fi
>  fi
>  
> +##
> +# openpty probe
> +openpty_libc=no
> +openpty_libutil=no
> +cat > $TMPC << EOF
> +extern int openpty(int *amaster, int *aslave, char *name, void *termp, void 
> *winp);
> +
> +int main(void)
> +{
> +int master_fd, slave_fd;
> +return openpty(_fd, _fd, 0, 0, 0) == 0;
> +}
> +EOF
> +if compile_prog "" "" ; then
> +  openpty_libc=yes
> +elif compile_prog "" "-lutil" ; then
> +  openpty_libutil=yes
> +fi
> +
>  ##
>  # signalfd probe
>  signalfd="no"
> @@ -5788,6 +5807,12 @@ fi
>  if test "$fdt" = "yes" ; then
>echo "CONFIG_FDT=y" >> $config_host_mak
>  fi
> +if test "$openpty_libc" = "yes" ; then
> +  echo "CONFIG_OPENPTY_LIBC=y" >> $config_host_mak
> +fi
> +if test "$openpty_libutil" = "yes" ; then
> +  echo "CONFIG_OPENPTY_LIBUTIL=y" >> $config_host_mak
> +fi
>  if test "$signalfd" = "yes" ; then
>echo "CONFIG_SIGNALFD=y" >> $config_host_mak
>  fi
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index fae5715e9c..e7e0bc2724 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -814,7 +814,7 @@ tests/migration/initrd-stress.img: 
> tests/migration/stress$(EXESUF)
>   rm $(INITRD_WORK_DIR)/init
>   rmdir $(INITRD_WORK_DIR)
>  
> -ifeq ($(CONFIG_POSIX),y)
> +ifeq ($(CONFIG_OPENPTY_LIBUTIL),y)
>  LIBS += -lutil
>  endif
>  
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index 2e8b43bdf5..62c87e5563 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -51,8 +51,8 @@
>  # include 
>  #endif
>  
> -#ifdef __sun__
> -/* Once Solaris has openpty(), this is going to be removed. */
> +/* The fallback implementation is needed at least on SmartOS. */
> +#if !defined(CONFIG_OPENPTY_LIBC) && !defined(CONFIG_OPENPTY_LIBUTIL)
>  static int openpty(int *amaster, int *aslave, char *name,
> struct termios *termp, struct winsize *winp)
>  {
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v4 7/8] Makefile: Remove libqemustub.a

[Alistair Francis]

Using two libraries (libqemuutil.a and libqemustub.a) would sometimes
result in circular dependencies. To avoid these issues let's just
combine both into a single library that functions as both.

Signed-off-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
---

V3:
 - Fix up the docs
 - Rebase into this series
V2:
 - Skipped

 Makefile|  7 +++
 Makefile.target |  2 +-
 docs/devel/build-system.txt | 16 +++-
 tests/Makefile.include  |  6 +++---
 4 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 337a1f6f9b..8f1273a159 100644
--- a/Makefile
+++ b/Makefile
@@ -344,7 +344,7 @@ subdir-dtc:dtc/libfdt dtc/tests
 dtc/%:
mkdir -p $@
 
-$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(chardev-obj-y) \
+$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \
$(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
@@ -364,12 +364,11 @@ Makefile: $(version-obj-y)
 ##
 # Build libraries
 
-libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y) $(trace-obj-y)
+libqemuutil.a: $(util-obj-y) $(trace-obj-y) $(stub-obj-y)
 
 ##
 
-COMMON_LDADDS = libqemuutil.a libqemustub.a
+COMMON_LDADDS = libqemuutil.a
 
 qemu-img.o: qemu-img-cmds.h
 
diff --git a/Makefile.target b/Makefile.target
index 7f42c45db8..0a80caf79c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -193,7 +193,7 @@ all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
-COMMON_LDADDS = ../libqemuutil.a ../libqemustub.a
+COMMON_LDADDS = ../libqemuutil.a
 
 # build either PROG or PROGW
 $(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS)
diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt
index 2af1e668c5..0d54294cb1 100644
--- a/docs/devel/build-system.txt
+++ b/docs/devel/build-system.txt
@@ -232,15 +232,13 @@ The utility code that is used by all binaries is built 
into a
 static archive called libqemuutil.a, which is then linked to all the
 binaries. In order to provide hooks that are only needed by some of the
 binaries, code in libqemuutil.a may depend on other functions that are
-not fully implemented by all QEMU binaries. To deal with this there is a
-second library called libqemustub.a which provides dummy stubs for all
-these functions. These will get lazy linked into the binary if the real
-implementation is not present. In this way, the libqemustub.a static
-library can be thought of as a portable implementation of the weak
-symbols concept. All binaries should link to both libqemuutil.a and
-libqemustub.a. e.g.
-
- qemu-img$(EXESUF): qemu-img.o ..snip.. libqemuutil.a libqemustub.a
+not fully implemented by all QEMU binaries. Dummy stubs for all  these
+functions are also provided by this library, and will get lazy linked
+into the binary if the real implementation is not present. In this way,
+this static library can be thought of as a portable implementation of
+the weak symbols concept. All binaries should link to libqemuutil.a. e.g.
+
+ qemu-img$(EXESUF): qemu-img.o ..snip.. libqemuutil.a
 
 
 Windows platform portability
diff --git a/tests/Makefile.include b/tests/Makefile.include
index fae5715e9c..3d6726e95c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -561,7 +561,7 @@ QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
 
 # Deps that are common to various different sets of tests below
-test-util-obj-y = libqemuutil.a libqemustub.a
+test-util-obj-y = libqemuutil.a
 test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
tests/test-qapi-event.o tests/test-qmp-introspect.o \
@@ -617,8 +617,8 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
$(test-io-obj-y)
 tests/test-timed-average$(EXESUF): tests/test-timed-average.o 
$(test-util-obj-y)
 tests/test-base64$(EXESUF): tests/test-base64.o \
-   libqemuutil.a libqemustub.a
-tests/ptimer-test$(EXESUF): tests/ptimer-test.o tests/ptimer-test-stubs.o 
hw/core/ptimer.o libqemustub.a
+   libqemuutil.a
+tests/ptimer-test$(EXESUF): tests/ptimer-test.o tests/ptimer-test-stubs.o 
hw/core/ptimer.o $(test-util-obj-y)
 
 tests/test-logging$(EXESUF): tests/test-logging.o $(test-util-obj-y)
 
-- 
2.11.0

[Qemu-devel] [PATCH v4 4/8] Convert multi-line fprintf() to warn_report()

[Alistair Francis]

Convert all the multi-line uses of fprintf(stderr, "warning:"..."\n"...
to use warn_report() instead. This helps standardise on a single
method of printing warnings to the user.

All of the warnings were changed using these commands:
  find ./* -type f -exec sed -i \
'N; {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +
  find ./* -type f -exec sed -i \
'N;N; {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +
  find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +
  find ./* -type f -exec sed -i \
'N;N;N;N {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +
  find ./* -type f -exec sed -i \
'N;N;N;N;N {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +
  find ./* -type f -exec sed -i \
'N;N;N;N;N;N {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +
  find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(.*".*warning[,:] 
\(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig}' \
{} +

Indentation fixed up manually afterwards.

Some of the lines were manually edited to reduce the line length to below
80 charecters. Some of the lines with newlines in the middle of the
string were also manually edit to avoid checkpatch errrors.

The #include lines were manually updated to allow the code to compile.

Several of the warning messages can be improved after this patch, to
keep this patch mechanical this has been moved into a later patch.

Signed-off-by: Alistair Francis 
Cc: Paolo Bonzini 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Peter Maydell 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Cornelia Huck 
Cc: Christian Borntraeger 
Cc: Alexander Graf 
Cc: Jason Wang 
Cc: David Gibson 
Cc: Gerd Hoffmann 
Acked-by: Cornelia Huck 
Reviewed-by: Markus Armbruster 
---
I couldn't figure out any nice way (it is possible with some more logic
inside the sed apparently) to do this is one command, so I had to use
all of the commands above.

 accel/kvm/kvm-all.c |  7 +++
 block/vvfat.c   |  4 ++--
 hw/acpi/core.c  |  7 +++
 hw/arm/vexpress.c   |  4 ++--
 hw/i386/xen/xen-mapcache.c  |  5 +++--
 hw/mips/mips_malta.c|  4 ++--
 hw/mips/mips_r4k.c  |  6 +++---
 hw/s390x/s390-virtio.c  | 16 
 net/hub.c   |  9 -
 net/net.c   | 14 +++---
 target/i386/cpu.c   | 12 ++--
 target/i386/hax-mem.c   |  6 +++---
 target/ppc/translate_init.c | 18 +-
 ui/keymaps.c|  9 +
 util/main-loop.c|  6 +++---
 15 files changed, 63 insertions(+), 64 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f85553a851..c9a9b75fa1 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1629,10 +1629,9 @@ static int kvm_init(MachineState *ms)
 
 while (nc->name) {
 if (nc->num > soft_vcpus_limit) {
-fprintf(stderr,
-"Warning: Number of %s cpus requested (%d) exceeds "
-"the recommended cpus supported by KVM (%d)\n",
-nc->name, nc->num, soft_vcpus_limit);
+warn_report("Number of %s cpus requested (%d) exceeds "
+"the recommended cpus supported by KVM (%d)",
+nc->name, nc->num, soft_vcpus_limit);
 
 if (nc->num > hard_vcpus_limit) {
 fprintf(stderr, "Number of %s cpus requested (%d) exceeds "
diff --git a/block/vvfat.c b/block/vvfat.c
index efad5750ba..6659a4a96a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1227,8 +1227,8 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 switch (s->fat_type) {
 case 32:
-fprintf(stderr, "Big fat greek warning: FAT32 has not been tested. 
"
-"You are welcome to do so!\n");
+warn_report("FAT32 has not been tested. "
+"You are welcome to do so!");
 break;
 case 16:
 case 12:
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 2a1b79c838..cd0a1d357b 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -184,10 +184,9 @@ static void acpi_table_install(const char unsigned *blob, 
size_t bloblen,
 }
 
 

[Qemu-devel] [PATCH v4 6/8] target/mips: Convert VM clock update prints to warn_report

[Alistair Francis]

Convert the fprintf() messages in kvm_mips_update_state() to use
warn_report() as they aren't errors, but are just warnings.

Signed-off-by: Alistair Francis 
Cc: James Hogan 
---

 target/mips/kvm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index a23aa438d2..3b7b1d962a 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -526,7 +526,7 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
 if (!cs->vcpu_dirty) {
 ret = kvm_mips_save_count(cs);
 if (ret < 0) {
-fprintf(stderr, "Failed saving count\n");
+warn_report("Failed saving count");
 }
 }
 } else {
@@ -535,14 +535,14 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
 ret = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_RESUME,
   _resume);
 if (ret < 0) {
-fprintf(stderr, "Failed setting COUNT_RESUME\n");
+warn_report("Failed setting COUNT_RESUME");
 return;
 }
 
 if (!cs->vcpu_dirty) {
 ret = kvm_mips_restore_count(cs);
 if (ret < 0) {
-fprintf(stderr, "Failed restoring count\n");
+warn_report("Failed restoring count");
 }
 }
 }
-- 
2.11.0

[Qemu-devel] [PATCH v4 3/8] Convert single line fprintf(.../n) to warn_report()

[Alistair Francis]

Convert all the single line uses of fprintf(stderr, "warning:"..."\n"...
to use warn_report() instead. This helps standardise on a single
method of printing warnings to the user.

All of the warnings were changed using this command:
  find ./* -type f -exec sed -i \
's|fprintf(.*".*warning[,:] \(.*\)\\n"\(.*\));|warn_report("\1"\2);|Ig' \
{} +

Some of the lines were manually edited to reduce the line length to below
80 charecters.

The #include lines were manually updated to allow the code to compile.

Signed-off-by: Alistair Francis 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Gerd Hoffmann 
Cc: Jason Wang 
Cc: Michael Roth 
Cc: James Hogan 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Stefan Hajnoczi 
Reviewed-by: Markus Armbruster 
Reviewed-by: James Hogan  [mips]
---
V4:
 - Rebase

 block/vvfat.c  | 4 +++-
 hw/acpi/core.c | 3 ++-
 hw/i386/pc.c   | 2 +-
 hw/misc/applesmc.c | 2 +-
 hw/usb/hcd-ehci.c  | 5 +++--
 hw/virtio/virtio-balloon.c | 3 ++-
 net/hub.c  | 3 ++-
 qga/vss-win32.c| 2 +-
 target/mips/kvm.c  | 4 ++--
 trace/simple.c | 3 ++-
 ui/keymaps.c   | 2 +-
 ui/spice-display.c | 2 +-
 12 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index c54fa94651..efad5750ba 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -32,6 +32,7 @@
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 
 #ifndef S_IWGRP
 #define S_IWGRP 0
@@ -3028,7 +3029,8 @@ DLOG(checkpoint());
 if (memcmp(direntries + k,
 array_get(&(s->directory), dir_index + k),
 sizeof(direntry_t))) {
-fprintf(stderr, "Warning: tried to write to 
write-protected file\n");
+warn_report("tried to write to write-protected "
+"file");
 return -1;
 }
 }
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 95fcac95a2..2a1b79c838 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -28,6 +28,7 @@
 #include "qapi/opts-visitor.h"
 #include "qapi-visit.h"
 #include "qapi-event.h"
+#include "qemu/error-report.h"
 
 struct acpi_table_header {
 uint16_t _length; /* our length, not actual part of the hdr */
@@ -221,7 +222,7 @@ static void acpi_table_install(const char unsigned *blob, 
size_t bloblen,
 }
 
 if (!has_header && changed_fields == 0) {
-fprintf(stderr, "warning: ACPI table: no headers are specified\n");
+warn_report("ACPI table: no headers are specified");
 }
 
 /* recalculate checksum */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c882f8c2ea..ef5f30e644 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1310,7 +1310,7 @@ void pc_acpi_init(const char *default_dsdt)
 
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, default_dsdt);
 if (filename == NULL) {
-fprintf(stderr, "WARNING: failed to find %s\n", default_dsdt);
+warn_report("failed to find %s", default_dsdt);
 } else {
 QemuOpts *opts = qemu_opts_create(qemu_find_opts("acpi"), NULL, 0,
   _abort);
diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 7896812304..7be8b5f13c 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -331,7 +331,7 @@ static void applesmc_isa_realize(DeviceState *dev, Error 
**errp)
 s->iobase + APPLESMC_ERR_PORT);
 
 if (!s->osk || (strlen(s->osk) != 64)) {
-fprintf(stderr, "WARNING: Using AppleSMC with invalid key\n");
+warn_report("Using AppleSMC with invalid key");
 s->osk = default_osk;
 }
 
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 604912cb3e..46fd30b075 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -32,6 +32,7 @@
 #include "hw/usb/ehci-regs.h"
 #include "hw/usb/hcd-ehci.h"
 #include "trace.h"
+#include "qemu/error-report.h"
 
 #define FRAME_TIMER_FREQ 1000
 #define FRAME_TIMER_NS   (NANOSECONDS_PER_SECOND / FRAME_TIMER_FREQ)
@@ -348,7 +349,7 @@ static void ehci_trace_sitd(EHCIState *s, hwaddr addr,
 static void ehci_trace_guest_bug(EHCIState *s, const char *message)
 {
 trace_usb_ehci_guest_bug(message);
-fprintf(stderr, "ehci warning: %s\n", message);
+warn_report("%s", message);
 }
 
 static inline 

[Qemu-devel] [PATCH v4 5/8] General warn report fixups

[Alistair Francis]

Tidy up some of the warn_report() messages after having converted them
to use warn_report().

Signed-off-by: Alistair Francis 
Reviewed-by: Markus Armbruster 
---
V4:
 - Small fixes

 block/vvfat.c   |  3 +--
 hw/arm/vexpress.c   |  2 +-
 hw/i386/xen/xen-mapcache.c  |  2 +-
 hw/mips/mips_malta.c|  4 ++--
 hw/mips/mips_r4k.c  |  3 +--
 hw/s390x/s390-virtio.c  | 14 --
 net/hub.c   |  6 ++
 net/net.c   |  5 +++--
 target/i386/hax-mem.c   |  6 +++---
 target/ppc/translate_init.c |  3 +--
 ui/keymaps.c|  3 +--
 11 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 6659a4a96a..cbabb36f62 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1227,8 +1227,7 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 switch (s->fat_type) {
 case 32:
-warn_report("FAT32 has not been tested. "
-"You are welcome to do so!");
+warn_report("FAT32 has not been tested. You are welcome to do so!");
 break;
 case 16:
 case 12:
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index f72ee6658b..96c5eebeaf 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -494,7 +494,7 @@ static void vexpress_modify_dtb(const struct arm_boot_info 
*info, void *fdt)
  * happen with older device tree blobs.
  */
 warn_report("couldn't find interrupt controller in "
-"dtb; will not include virtio-mmio devices in the dtb.");
+"dtb; will not include virtio-mmio devices in the dtb");
 } else {
 int i;
 const hwaddr *map = daughterboard->motherboard_map;
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index 91a4fd6984..baab93b614 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -127,7 +127,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void 
*opaque)
 
 if (rlimit_as.rlim_max != RLIM_INFINITY) {
 warn_report("QEMU's maximum size of virtual"
-" memory is not infinity.");
+" memory is not infinity");
 }
 if (rlimit_as.rlim_max < MCACHE_MAX_SIZE + NON_MCACHE_MEMORY_SIZE) {
 mapcache->max_mcache_size = rlimit_as.rlim_max -
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 233e2ee802..7d6e58348e 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -216,8 +216,8 @@ static void generate_eeprom_spd(uint8_t *eeprom, ram_addr_t 
ram_size)
 }
 
 if (ram_size) {
-warn_report("SPD cannot represent final %dMB"
-" of SDRAM", (int)ram_size);
+warn_report("SPD cannot represent final " RAM_ADDR_FMT "MB"
+" of SDRAM", ram_size);
 }
 
 /* fill in SPD memory information */
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 6ffb88fd70..b48a4d72ac 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -254,8 +254,7 @@ void mips_r4k_init(MachineState *machine)
}
 } else if (!qtest_enabled()) {
 /* not fatal */
-warn_report("could not load MIPS bios '%s'",
-bios_name);
+warn_report("could not load MIPS bios '%s'", bios_name);
 }
 g_free(filename);
 
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 25781f04d8..0e91c465f2 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -141,9 +141,10 @@ void gtod_save(QEMUFile *f, void *opaque)
 
 r = s390_get_clock(_high, _low);
 if (r) {
-warn_report("Unable to get guest clock for migration. "
-"Error code %d. Guest clock will not be migrated "
-"which could cause the guest to hang.", r);
+warn_report("Unable to get guest clock for migration: %s",
+strerror(-r));
+error_printf("Guest clock will not be migrated "
+ "which could cause the guest to hang.");
 qemu_put_byte(f, S390_TOD_CLOCK_VALUE_MISSING);
 return;
 }
@@ -170,9 +171,10 @@ int gtod_load(QEMUFile *f, void *opaque, int version_id)
 
 r = s390_set_clock(_high, _low);
 if (r) {
-warn_report("Unable to set guest clock value. "
-"s390_get_clock returned error %d. This could cause "
-"the guest to hang.", r);
+warn_report("Unable to set guest clock for migration: %s",
+strerror(-r));
+error_printf("Guest clock will not be restored "
+ "which could cause the guest to hang.");
 }
 
 return 0;
diff --git a/net/hub.c b/net/hub.c
index 745a2168a1..14b4eec68f 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -310,8 +310,7 @@ void net_hub_check_clients(void)
 QLIST_FOREACH(port, >ports, next) {
 peer = 

[Qemu-devel] [PATCH v4 8/8] Convert remaining single line fprintf() to warn_report()

[Alistair Francis]

Convert any remaining uses of fprintf(stderr, "warning:"...
to use warn_report() instead. This helps standardise on a single
method of printing warnings to the user.

All of the warnings were changed using this command:
  find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' 
{} +

The #include line and the change to the test Makefile were manually
updated to allow the code to compile.

Signed-off-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Markus Armbruster 
---

This pattern matches any case of fprintf(stderr, "warning:"... and is
the most open pattern match in the series.


 tests/Makefile.include | 2 +-
 util/cutils.c  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3d6726e95c..264765 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -593,7 +593,7 @@ tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
 tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
$(test-crypto-obj-y)
 tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
 tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
migration/page_cache.o $(test-util-obj-y)
-tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
+tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o 
$(test-util-obj-y)
 tests/test-int128$(EXESUF): tests/test-int128.o
 tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
 tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
diff --git a/util/cutils.c b/util/cutils.c
index 1534682083..b33ede83d1 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -30,6 +30,7 @@
 #include "qemu/iov.h"
 #include "net/net.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 
 void strpadcpy(char *buf, int buf_size, const char *str, char pad)
 {
@@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial)
 return initial;
 }
 if (debug < 0 || debug > max || errno != 0) {
-fprintf(stderr, "warning: %s not in [0, %d]", name, max);
+warn_report("%s not in [0, %d]", name, max);
 return initial;
 }
 return debug;
-- 
2.11.0

[Qemu-devel] [PATCH v4 2/8] Convert remaining error_report() to warn_report()

[Alistair Francis]

In a previous patch (3dc6f8693694a649a9c83f1e2746565b47683923) we
converted uses of error_report("warning:"... to use warn_report()
instead. This was to help standardise on a single method of printing
warnings to the user.

There appears to have been some cases that slipped through in patch sets
applied around the same time, this patch catches the few remaining
cases.

All of the warnings were changed using this command:
  find ./* -type f -exec sed -i \
's|error_report(".*warning[,:] |warn_report("|Ig' {} +

Indentation fixed up manually afterwards.

Two messages were manually fixed up as well.

Signed-off-by: Alistair Francis 
Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Alexander Graf 
Cc: Richard Henderson 
Cc: Stefan Hajnoczi 
Acked-by: Cornelia Huck 
Reviewed-by: Markus Armbruster 
---

 block/qcow2.c  | 9 +
 target/s390x/kvm.c | 4 ++--
 trace/control.c| 4 ++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index bae5893327..d33fb3ecdd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -301,10 +301,11 @@ static int qcow2_read_extensions(BlockDriverState *bs, 
uint64_t start_offset,
 }
 
 if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
-error_report("WARNING: a program lacking bitmap support "
- "modified this file, so all bitmaps are now "
- "considered inconsistent. Some clusters may be "
- "leaked, run 'qemu-img check -r' on the image "
+warn_report("a program lacking bitmap support "
+"modified this file, so all bitmaps are now "
+"considered inconsistent");
+error_printf("Some clusters may be leaked, "
+ "run 'qemu-img check -r' on the image "
  "file to fix.");
 if (need_update_header != NULL) {
 /* Updating is needed to drop invalid bitmap extension. */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index d07763ff2c..ad7ce9fc70 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -224,8 +224,8 @@ static void kvm_s390_enable_cmma(void)
 };
 
 if (mem_path) {
-error_report("Warning: CMM will not be enabled because it is not "
- "compatible to hugetlbfs.");
+warn_report("CMM will not be enabled because it is not "
+"compatible with hugetlbfs.");
 return;
 }
 rc = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, );
diff --git a/trace/control.c b/trace/control.c
index 82d8989c4d..2769934bec 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -72,8 +72,8 @@ void trace_event_register_group(TraceEvent **events)
 if (likely(next_vcpu_id < CPU_TRACE_DSTATE_MAX_EVENTS)) {
 events[i]->vcpu_id = next_vcpu_id++;
 } else {
-error_report("WARNING: too many vcpu trace events; dropping '%s'",
- events[i]->name);
+warn_report("too many vcpu trace events; dropping '%s'",
+events[i]->name);
 }
 }
 event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 1);
-- 
2.11.0

[Qemu-devel] [PATCH v4 1/8] hw/i386: Improve some of the warning messages

[Alistair Francis]

Signed-off-by: Alistair Francis 
Suggested-by: Eduardo Habkost 
Cc: Eduardo Habkost 
---
V4:
 - Fixup pc_q35.c message
V3:
 - Improve the messages

 hw/i386/acpi-build.c | 15 ++-
 hw/i386/pc.c |  7 +++
 hw/i386/pc_q35.c |  8 +---
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4d19d91e1b..9776812588 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2750,17 +2750,22 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
  ACPI_BUILD_ALIGN_SIZE);
 if (tables_blob->len > legacy_table_size) {
 /* Should happen only with PCI bridges and -M pc-i440fx-2.0.  */
-warn_report("migration may not work.");
+warn_report("ACPI table size %u exceeds %d bytes,"
+" migration may not work",
+tables_blob->len, legacy_table_size);
+error_printf("Try removing CPUs, NUMA nodes, memory slots"
+ " or PCI bridges.");
 }
 g_array_set_size(tables_blob, legacy_table_size);
 } else {
 /* Make sure we have a buffer in case we need to resize the tables. */
 if (tables_blob->len > ACPI_BUILD_TABLE_SIZE / 2) {
 /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. 
 */
-warn_report("ACPI tables are larger than 64k.");
-warn_report("migration may not work.");
-warn_report("please remove CPUs, NUMA nodes, "
-"memory slots or PCI bridges.");
+warn_report("ACPI table size %u exceeds %d bytes,"
+" migration may not work",
+tables_blob->len, ACPI_BUILD_TABLE_SIZE / 2);
+error_printf("Try removing CPUs, NUMA nodes, memory slots"
+ " or PCI bridges.");
 }
 acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 21081041d5..c882f8c2ea 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -384,7 +384,7 @@ ISADevice *pc_find_fdc0(void)
 warn_report("multiple floppy disk controllers with "
 "iobase=0x3f0 have been found");
 error_printf("the one being picked for CMOS setup might not reflect "
- "your intent\n");
+ "your intent");
 }
 
 return state.floppy;
@@ -2098,9 +2098,8 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, 
Visitor *v,
 }
 
 if (value < (1ULL << 20)) {
-warn_report("small max_ram_below_4g(%"PRIu64
-") less than 1M.  BIOS may not work..",
-value);
+warn_report("Only %" PRIu64 " bytes of RAM below the 4GiB boundary,"
+"BIOS may not work with less than 1MiB", value);
 }
 
 pcms->max_ram_below_4g = value;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c1cba584d1..7947efb680 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -101,9 +101,11 @@ static void pc_q35_init(MachineState *machine)
 lowmem = pcms->max_ram_below_4g;
 if (machine->ram_size - lowmem > lowmem &&
 lowmem & ((1ULL << 30) - 1)) {
-warn_report("Large machine and max_ram_below_4g(%"PRIu64
-") not a multiple of 1G; possible bad performance.",
-pcms->max_ram_below_4g);
+warn_report("There is possibly poor performance as the ram size "
+" (0x%" PRIx64 ") is more then twice the size of"
+" max-ram-below-4g (%"PRIu64") and"
+" max-ram-below-4g is not a multiple of 1G.",
+machine->ram_size, pcms->max_ram_below_4g);
 }
 }
 
-- 
2.11.0

[Qemu-devel] [PATCH v4 0/8] More warning reporting fixed

[Alistair Francis]

This series expands on my previous series by converting more existing
prints to use warn_report() instead of error_report() or fprintf().

As discussed with Paolo and Markus this series combines libqemustub.a into
libqemuutil.a to avoid circular dependencies.

V4:
 - Improve some extra MIPs messages
 - Fix build issues
 - Fix i386 print message
V3:
 - Small corrections as reported by Markus
 - Rename patch 3 and 5 so they don't have the same name
 - Combine libqemustub.a into libqemuutil.a
 - Add an extra patch with general cleanups

V2:
 - Fixup auto CC logic so everyone is CCed



Alistair Francis (8):
  hw/i386: Improve some of the warning messages
  Convert remaining error_report() to warn_report()
  Convert single line fprintf(.../n) to warn_report()
  Convert multi-line fprintf() to warn_report()
  General warn report fixups
  target/mips: Convert VM clock update prints to warn_report
  Makefile: Remove libqemustub.a
  Convert remaining single line fprintf() to warn_report()

 Makefile|  7 +++
 Makefile.target |  2 +-
 accel/kvm/kvm-all.c |  7 +++
 block/qcow2.c   |  9 +
 block/vvfat.c   |  7 ---
 docs/devel/build-system.txt | 16 +++-
 hw/acpi/core.c  | 10 +-
 hw/arm/vexpress.c   |  4 ++--
 hw/i386/acpi-build.c| 15 ++-
 hw/i386/pc.c|  9 -
 hw/i386/pc_q35.c|  8 +---
 hw/i386/xen/xen-mapcache.c  |  5 +++--
 hw/mips/mips_malta.c|  4 ++--
 hw/mips/mips_r4k.c  |  5 ++---
 hw/misc/applesmc.c  |  2 +-
 hw/s390x/s390-virtio.c  | 18 ++
 hw/usb/hcd-ehci.c   |  5 +++--
 hw/virtio/virtio-balloon.c  |  3 ++-
 net/hub.c   | 10 --
 net/net.c   | 15 ---
 qga/vss-win32.c |  2 +-
 target/i386/cpu.c   | 12 ++--
 target/i386/hax-mem.c   |  6 +++---
 target/mips/kvm.c   | 10 +-
 target/ppc/translate_init.c | 17 -
 target/s390x/kvm.c  |  4 ++--
 tests/Makefile.include  |  8 
 trace/control.c |  4 ++--
 trace/simple.c  |  3 ++-
 ui/keymaps.c| 10 +-
 ui/spice-display.c  |  2 +-
 util/cutils.c   |  3 ++-
 util/main-loop.c|  6 +++---
 33 files changed, 128 insertions(+), 120 deletions(-)

-- 
2.11.0

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Stefan Weil]

Am 11.09.2017 um 21:47 schrieb Philippe Mathieu-Daudé:
[...]> Stefan are you testing no-X86 targets?

I only tested x86_64:

./configure --enable-debug --enable-tcg-interpreter \
  --target-list=x86_64-linux-user,x86_64-softmmu

If other targets fail, I'll have a look to find the cause.

Stefan

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Philippe Mathieu-Daudé]

Hi Peter, Stefan,


Am 11.09.2017 um 20:24 schrieb Peter Maydell:

I've also turned on a tci compile check on my pre-merge tests.
(It doesn't pass "make check" for me, though...)


Peter you might want to restrict it to X86...

--target-list=subdir-i386-softmmu,x86_64-softmmu,x86_64-linux-user



I just tested it myself. "make check" passed, but printed some
KVM related messages which could be suppressed by fixing the
test code:

[...]
   GTESTER tests/test-qht-par
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
[...]
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
make: Verzeichnis „/qemu/bin/“ wird verlassen


Stefan are you testing no-X86 targets?

Regards,

Phil.

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Stefan Weil]

Am 11.09.2017 um 20:24 schrieb Peter Maydell:
> On 11 September 2017 at 19:01, Richard Henderson
>  wrote:
>> On 09/10/2017 07:28 PM, Philippe Mathieu-Daudé wrote:
>>> changed in 659ef5cbb893, this fixes building with --enable-tcg-interpreter:
>>>
>>> /home/travis/build/qemu/qemu/tcg/tcg.c:116:14: error: 
>>> ‘tcg_out_ldst_finalize’ used but never defined [-Werror]
>>>  static bool tcg_out_ldst_finalize(TCGContext *s);
>>>   ^
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>
>> Oops.
>>
>> Reviewed-by: Richard Henderson 
> 
> Applied to master as a buildfix, thanks.

Thank you, Peter.

> I've also turned on a tci compile check on my pre-merge tests.
> (It doesn't pass "make check" for me, though...)

I just tested it myself. "make check" passed, but printed some
KVM related messages which could be suppressed by fixing the
test code:

[...]
  GTESTER tests/test-qht-par
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
[...]
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
make: Verzeichnis „/qemu/bin/“ wird verlassen

What failures do you get?

Regards,
Stefan

Re: [Qemu-devel] [Patch 0/3] vfio: reusing address space for the same iommu group devices

[Alex Williamson]

On Tue, 12 Sep 2017 02:56:29 +0800
w...@redhat.com wrote:

> From: Wei Xu 
> 
> Recently I have been testing passing through 2 ixgbe(82599ES) nics which
> belong to the same iommu group to a guest with virtual iommu(vIOMMU) on
> my desktop, while vfio failed to realize the second device and prompted
> error message as 'group xxx used in multiple address spaces'.
> 
> It turned out to be that vtd doesn't know any group info while choosing
> an address space for the two devices, therefore it creates two separate
> address space for each which breaks granularity isolation.
> 
> This patch fixes this by looking up if there is any exist device within
> the same iommu group and shares the address space before creating a new
> one.
> 
> I am not sure if this fixes the problem in a correct way due to my limited
> knowledge about vfio, please come back to me for any feedback & comments,
> Thanks.

Hi Wei,

Are the devices in the same IOMMU group on the host or guest or both?
My impression is that host IOMMU groups with more than a single
endpoint are fundamentally incompatible with vIOMMU.  Even if we could
enforce a VM topology which logically should place them into the same
IOMMU group, IOMMU groups themselves are a Linux concept and another
guest OS could rightfully attempt to use them in separate VT-d contexts.
The only way we could ensure a shared context is if they were in a
conventional PCI hierarchy within the VM.  I'd suggest that vIOMMU
should be reserved for hosts and devices which have full isolation and
for which the VM and guest can create separate mappings per device.

I'm not sure if what I describe is the actual problem you're facing as I
thought we had quirks to expose the per-port isolation on 82599
devices, but perhaps the lack of isolation is coming from an upstream
device.  If I'm off, please provide further details of exactly the host
and guest scenarios.  Thanks,

Alex

> Wei Xu (3):
>   vfio: reusing address space for the same iommu group devices
>   vfio: invoke looking up address space.
>   vfio: remove checking duplicated vfio device
> 
>  hw/vfio/common.c  | 28 
>  hw/vfio/pci.c | 15 ++-
>  include/hw/vfio/vfio-common.h |  1 +
>  3 files changed, 35 insertions(+), 9 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 6/6] io: Reply to ping frames

[Brandon Carpenter]

On Mon, Sep 11, 2017 at 10:10 AM, Daniel P. Berrange 
 wrote:
It feels like this is still dangerous - the client simply has to 
interleave each "ping" with a 1 byte binary frame to get around this 
limit. We need to make sure we have an absolute cap on the output 
buffer size.


Okay. I see that now that I look at it more closely. This breed of 
asynchronous I/O is tricky because the conditions for reading/writing 
are all over the place. There's a lot of context to keep in your head.


I have a fix. And I realized that I was missing a patch in the series 
for RFC-compliant closing of websocket connections, which I must have 
lost during a rebase. Should I submit v3 of the patch series or just 
add those patches to this thread?


Thank you,
--
Brandon Carpenter | Software Engineer
Cypherpath, Inc.
400 Columbia Point Drive Ste 101 | Richland, Washington USA
Office: (650) 713-3060


--


CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
for the sole use of the intended recipient(s) and may contain proprietary, 
confidential or privileged information or otherwise be protected by law. 
Any unauthorized review, use, disclosure or distribution is prohibited. If 
you are not the intended recipient, please notify the sender and destroy 
all copies and the original message.

Re: [Qemu-devel] [PATCH v7 07/22] migration: Make migrate_fd_error() the owner of the Error

[Dr. David Alan Gilbert]

* Eric Blake (ebl...@redhat.com) wrote:
> On 09/06/2017 06:51 AM, Juan Quintela wrote:
> > So far, we had to free the error after each caller, so just do it
> > here.  Once there, tls.c was leaking the error.
> 
> You mention tls.c,
> 
> > 
> > Signed-off-by: Juan Quintela 
> > ---
> >  migration/channel.c   |  1 -
> >  migration/migration.c | 10 --
> >  migration/migration.h |  4 ++--
> >  migration/socket.c|  1 -
> >  4 files changed, 6 insertions(+), 10 deletions(-)
> 
> but don't touch it.  Am I missing something?


hmm well I see in migration/tls.c:

if (qio_task_propagate_error(task, )) {
trace_migration_tls_outgoing_handshake_error(error_get_pretty(err));
migrate_fd_error(s, err);
error_free(err);
} else {
trace_migration_tls_outgoing_handshake_complete();
migration_channel_connect(s, ioc, NULL);
}

so I think that error_free has to go?

Dave

> >  
> > -void migrate_fd_error(MigrationState *s, const Error *error)
> > +void migrate_fd_error(MigrationState *s, Error *error)
> >  {
> 
> No comments at definition,
> 
> > +++ b/migration/migration.h
> > @@ -163,8 +163,8 @@ bool  migration_has_all_channels(void);
> >  
> >  uint64_t migrate_max_downtime(void);
> >  
> > -void migrate_set_error(MigrationState *s, const Error *error);
> > -void migrate_fd_error(MigrationState *s, const Error *error);
> > +void migrate_set_error(MigrationState *s, Error *error);
> > +void migrate_fd_error(MigrationState *s, Error *error);
> 
> or at declaration. That would be worth adding at some point, but this
> patch isn't making it worse.
> 
> The code looks okay in isolation, so if it is only the commit message
> that needs fixing,
> Reviewed-by: Eric Blake 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Stefano Stabellini]

CC'ing xen-devel, and the Xen tools and x86 maintainers.

On Mon, 11 Sep 2017, Igor Mammedov wrote:
> On Mon, 11 Sep 2017 12:41:47 +0800
> Haozhong Zhang  wrote:
> 
> > This is the QEMU part patches that works with the associated Xen
> > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > guest address space for vNVDIMM devices.
> > 
> > All patches can be found at
> >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > 
> > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > label data, as the Xen side support for labels is not implemented yet.
> > 
> > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > memory region for Xen guest, in order to make the existing nvdimm
> > device plugging path work on Xen.
> > 
> > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > used as the Xen device model.
> 
> I've skimmed over patch-set and can't say that I'm happy with
> number of xen_enabled() invariants it introduced as well as
> with partial blobs it creates.

I have not read the series (Haozhong, please CC me, Anthony and
xen-devel to the whole series next time), but yes, indeed. Let's not add
more xen_enabled() if possible.

Haozhong, was there a design document thread on xen-devel about this? If
so, did it reach a conclusion? Was the design accepted? If so, please
add a link to the design doc in the introductory email, so that
everybody can read it and be on the same page.


> I'd like to reduce above and a way to do this might be making xen 
>  1. use fw_cfg
>  2. fetch QEMU build acpi tables from fw_cfg
>  3. extract nvdim tables (which is trivial) and use them
> 
> looking at xen_load_linux(), it seems possible to use fw_cfg.
> 
> So what's stopping xen from using it elsewhere?,
> instead of adding more xen specific code to do 'the same'
> job and not reusing/sharing common code with tcg/kvm.

So far, ACPI tables have not been generated by QEMU. Xen HVM machines
rely on a firmware-like application called "hvmloader" that runs in
guest context and generates the ACPI tables. I have no opinions on
hvmloader and I'll let the Xen maintainers talk about it. However, keep
in mind that with an HVM guest some devices are emulated by Xen and/or
by other device emulators that can run alongside QEMU. QEMU doesn't have
a full few of the system.

Here the question is: does it have to be QEMU the one to generate the
ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
like the rest, instead of introducing this split-brain design about
ACPI. We need to see a design doc to fully understand this.

If the design doc thread led into thinking that it has to be QEMU to
generate them, then would it make the code nicer if we used fw_cfg to
get the (full or partial) tables from QEMU, as Igor suggested?


> > Haozhong Zhang (10):
> >   nvdimm: do not intiailize nvdimm->label_data if label size is zero
> >   hw/xen-hvm: create the hotplug memory region on Xen
> >   hostmem-xen: add a host memory backend for Xen
> >   nvdimm acpi: do not use fw_cfg on Xen
> >   hw/xen-hvm: initialize DM ACPI
> >   hw/xen-hvm: add function to copy ACPI into guest memory
> >   nvdimm acpi: copy NFIT to Xen guest
> >   nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest
> >   nvdimm acpi: do not build _FIT method on Xen
> >   hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled
> > 
> >  backends/Makefile.objs |   1 +
> >  backends/hostmem-xen.c | 108 ++
> >  backends/hostmem.c |   9 +++
> >  hw/acpi/aml-build.c|  10 ++-
> >  hw/acpi/nvdimm.c   |  79 ++-
> >  hw/i386/pc.c   | 102 ++---
> >  hw/i386/xen/xen-hvm.c  | 204 
> > -
> >  hw/mem/nvdimm.c|  10 ++-
> >  hw/mem/pc-dimm.c   |   6 +-
> >  include/hw/i386/pc.h   |   1 +
> >  include/hw/xen/xen.h   |  25 ++
> >  stubs/xen-hvm.c|  10 +++
> >  12 files changed, 495 insertions(+), 70 deletions(-)
> >  create mode 100644 backends/hostmem-xen.c
> > 
> 

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hello

On Thu, Aug 17, 2017 at 1:54 PM, Anatol Pomozov
 wrote:
> Hi
>
> On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf  wrote:
>> Am 04.08.2017 um 06:53 hat Anatol Pomozov geschrieben:
>>> Hi Kevin
>>>
>>> Thanks for the information.
>>>
>>> So I sounds like we do want multiboot to load all sections regardless
>>> of its segments info. To achieve it we need to read sections headers
>>> and load all section that were not loaded yet.
>>>
>>> I have a working implementation here
>>> https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5
>>> Tested it with my multiboot OS image. The target iterates over all
>>> sections and prints their names/address. The output result is the same
>>> for QEMU and for VmWare+GRUB.
>>>
>>> Let me know if this idea looks good so I can send this patch to qemu 
>>> maillist.
>>
>> I'm not sure if I'll find the time to review this in detail, but I'll
>> just give it a quick look.
>>
>> You seem to attempt to add support for 64 bit ELF binaries. The
>> Multiboot standard doesn't explicitly say that this is forbidden, but
>> everything in it expects 32 bit binaries and I don't think GRUB (as the
>> reference implementation for Multiboot) accepts 64 bit ELFs either. The
>> boot state is 32 bit protected mode in any case. So unless I'm mistaken
>> on GRUB's behaviour, I'd rather not support 64 bit ELFs.
>
> Grub actually does support ELF64 loading with multiboot. Here is the
> GRUB code that implements it
> https://github.com/coreos/grub/blob/master/grub-core/loader/multiboot.c#L210
>
> It would be great if QEMU behaved similar way.
>
> Actually I've been using qemu with ELF64 multiboot for a while and it
> works great.
>
>>
>> You shouldn't look at ELF headers at all if MULTIBOOT_AOUT_KLUDGE is
>> given. In this case, the kernel image is to be treated as a flat binary
>> and section headers can't be expected. I think your code breaks non-ELF
>> kernels.


I have these changes ready now
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03265.html
It added ELF parser functionality as discussed above. Are there any
open questions? What is the best way to proceed forward with these
patches?

I also have interest in adding Multiboot2 support to qemu. But before
going with it I would like to finish working on my current patchset.

Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hello

I have these changes ready now
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03265.html
It added ELF parser functionality as discussed above. Are there any
open questions? What is the best way to proceed forward with these
patches?

I also have interest in adding Multiboot2 support to qemu. But before
going with it I would like to finish working on my current patchset.

On Thu, Aug 17, 2017 at 1:54 PM, Anatol Pomozov
 wrote:
> Hi
>
> On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf  wrote:
>> Am 04.08.2017 um 06:53 hat Anatol Pomozov geschrieben:
>>> Hi Kevin
>>>
>>> Thanks for the information.
>>>
>>> So I sounds like we do want multiboot to load all sections regardless
>>> of its segments info. To achieve it we need to read sections headers
>>> and load all section that were not loaded yet.
>>>
>>> I have a working implementation here
>>> https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5
>>> Tested it with my multiboot OS image. The target iterates over all
>>> sections and prints their names/address. The output result is the same
>>> for QEMU and for VmWare+GRUB.
>>>
>>> Let me know if this idea looks good so I can send this patch to qemu 
>>> maillist.
>>
>> I'm not sure if I'll find the time to review this in detail, but I'll
>> just give it a quick look.
>>
>> You seem to attempt to add support for 64 bit ELF binaries. The
>> Multiboot standard doesn't explicitly say that this is forbidden, but
>> everything in it expects 32 bit binaries and I don't think GRUB (as the
>> reference implementation for Multiboot) accepts 64 bit ELFs either. The
>> boot state is 32 bit protected mode in any case. So unless I'm mistaken
>> on GRUB's behaviour, I'd rather not support 64 bit ELFs.
>
> Grub actually does support ELF64 loading with multiboot. Here is the
> GRUB code that implements it
> https://github.com/coreos/grub/blob/master/grub-core/loader/multiboot.c#L210
>
> It would be great if QEMU behaved similar way.
>
> Actually I've been using qemu with ELF64 multiboot for a while and it
> works great.
>
>>
>> You shouldn't look at ELF headers at all if MULTIBOOT_AOUT_KLUDGE is
>> given. In this case, the kernel image is to be treated as a flat binary
>> and section headers can't be expected. I think your code breaks non-ELF
>> kernels.
>
> Ok, will revert this part back.
>
>>
>> As for loading the section headers, duplicating ELF parser code may be
>> functionally correct, but probably not the best way to implement things.
>> As I wrote in an earlier email, load_elf() already parses all the
>> information that you need. You really just need to store it somewhere,
>> which would probably just mean adding a new parameter to load_elf() and
>> the parser could then store a copy of the data there if it's non-NULL.
>
> Loading section headers is actually a small part of my change. It also:
>   - calculates memory needed to load all sections into memory
>   - allocates memory
>   - loads sections
>
> This "load all sections into memory" functionality is very multiboot
> specific. But if you think if load_elf() is better place for it then I
> can look at moving it to load_elf().

Re: [Qemu-devel] [PATCH] i386/cpu/hyperv: support over 64 vcpus for windows guests

[Eduardo Habkost]

On Mon, Sep 11, 2017 at 11:20:27PM +0800, Gonglei wrote:
> Starting with Windows Server 2012 and Windows 8, if
> CPUID.4005.EAX contains a value of -1, Windows assumes specific
> limit to the number of VPs. In this case, Windows Server 2012
> guest VMs may use more than 64 VPs, up to the maximum supported
> number of processors applicable to the specific Windows
> version being used.
> 
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
> 
> For compatibility, Let's introduce a new property for X86CPU,
> named "x-hv-max-vps" as Eduardo's suggestion, and set it
> to 0x40 before machine 2.10.
> 
> (The "x-" prefix indicates that the property is not supposed to
> be a stable user interface.)
> 
> Signed-off-by: Gonglei 
> ---
>  include/hw/i386/pc.h |  5 +
>  target/i386/cpu.c|  1 +
>  target/i386/cpu.h|  2 ++
>  target/i386/kvm.c| 15 ++-
>  4 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8226904..087d184 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -371,6 +371,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
> *);
>  
>  #define PC_COMPAT_2_10 \
>  HW_COMPAT_2_10 \
> +{\
> +.driver   = TYPE_X86_CPU,\
> +.property = "x-hv-max-vps",\
> +.value= "0x40",\
> +},
>  
>  #define PC_COMPAT_2_9 \
>  HW_COMPAT_2_9 \
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 69676e1..2702485 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4145,6 +4145,7 @@ static Property x86_cpu_properties[] = {
>   false),
>  DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true),
>  DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true),
> +DEFINE_PROP_INT32("x-hv-max-vps", X86CPU, hv_max_vps, -1),
>  DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 525d35d..5c726f3 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1282,6 +1282,8 @@ struct X86CPU {
>  int32_t socket_id;
>  int32_t core_id;
>  int32_t thread_id;
> +
> +int32_t hv_max_vps;
>  };
>  
>  static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6db7783..a898bef 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -751,7 +751,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  
>  c = _data.entries[cpuid_i++];
>  c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
> -c->eax = 0x40;
> +
> +/*
> + * From "Requirements for Implementing the Microsoft
> + * Hypervisor Interface":
> + * 
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
> + *
> + * "Starting with Windows Server 2012 and Windows 8, if
> + * CPUID.4005.EAX contains a value of -1, Windows assumes
> + * specific limit to the number of VPs. In this case, Windows
> + * Server 2012 guest VMs may use more than 64 VPs, up to the
> + * maximum supported number of processors applicable to the
> + * specific Windows version being used."
> + */

I would place this comment above the DEFINE_PROP_INT32
declaration, as its purpose is to explain the -1 default.


> +c->eax = cpu->hv_max_vps;
>  c->ebx = 0x40;
>  
>  kvm_base = KVM_CPUID_SIGNATURE_NEXT;
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo

[Qemu-devel] [Patch 3/3] vfio: remove checking duplicated vfio device

[wexu]

From: Wei Xu 

This has been done when introducing 'vfio_lookup_as()'
patch as a side work to reuse the loop.

Signed-off-by: Wei Xu 
---
 hw/vfio/pci.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 856cefd..d78f756 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2632,7 +2632,6 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice 
*vdev)
 static void vfio_realize(PCIDevice *pdev, Error **errp)
 {
 VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
-VFIODevice *vbasedev_iter;
 VFIOGroup *group;
 char *tmp, group_path[PATH_MAX], *group_name;
 Error *err = NULL;
@@ -2697,14 +2696,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 goto error;
 }
 
-QLIST_FOREACH(vbasedev_iter, >device_list, next) {
-if (strcmp(vbasedev_iter->name, vdev->vbasedev.name) == 0) {
-error_setg(errp, "device is already attached");
-vfio_put_group(group);
-goto error;
-}
-}
-
 ret = vfio_get_device(group, vdev->vbasedev.name, >vbasedev, errp);
 if (ret) {
 vfio_put_group(group);
-- 
1.8.3.1

[Qemu-devel] [Patch 2/3] vfio: invoke looking up address space.

[wexu]

From: Wei Xu 

Invoke looking up correct address space before getting an
IOMMU group.

Signed-off-by: Wei Xu 
---
 hw/vfio/pci.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 31e1edf..856cefd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2640,6 +2640,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 struct stat st;
 int groupid;
 int i, ret;
+AddressSpace *as;
 
 if (!vdev->vbasedev.sysfsdev) {
 if (!(~vdev->host.domain || ~vdev->host.bus ||
@@ -2686,7 +2687,12 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 
 trace_vfio_realize(vdev->vbasedev.name, groupid);
 
-group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), 
errp);
+as = vfio_lookup_as(groupid, pdev, errp);
+if (!as) {
+goto error;
+}
+
+group = vfio_get_group(groupid, as, errp);
 if (!group) {
 goto error;
 }
-- 
1.8.3.1

[Qemu-devel] [Patch 0/3] vfio: reusing address space for the same iommu group devices

[wexu]

From: Wei Xu 

Recently I have been testing passing through 2 ixgbe(82599ES) nics which
belong to the same iommu group to a guest with virtual iommu(vIOMMU) on
my desktop, while vfio failed to realize the second device and prompted
error message as 'group xxx used in multiple address spaces'.

It turned out to be that vtd doesn't know any group info while choosing
an address space for the two devices, therefore it creates two separate
address space for each which breaks granularity isolation.

This patch fixes this by looking up if there is any exist device within
the same iommu group and shares the address space before creating a new
one.

I am not sure if this fixes the problem in a correct way due to my limited
knowledge about vfio, please come back to me for any feedback & comments,
Thanks.

Wei Xu (3):
  vfio: reusing address space for the same iommu group devices
  vfio: invoke looking up address space.
  vfio: remove checking duplicated vfio device

 hw/vfio/common.c  | 28 
 hw/vfio/pci.c | 15 ++-
 include/hw/vfio/vfio-common.h |  1 +
 3 files changed, 35 insertions(+), 9 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [Patch 1/3] vfio: reusing address space for the same IOMMU group devices

[wexu]

From: Wei Xu 

Currently address space of a vfio device is selected by directly
looking up pci device IOMMU address space during realizing, this
usually works for most none separate address space targeted cases
since they are using the system address space, i.e. a q35 machine
without virtual IOMMU. Unfortunately, when it comes down to the case
having a virtual IOMMU(x86 vtd in this case) and two vfio devices in
the same IOMMU group, the virtual IOMMU(vtd) creates two separate
address space for each device, this breaks the minimum granularity for
vfio, and the device fails realizing by prompting 'group xxx used in
multiple address spaces'.

This patch is a helper looking up the same IOMMU device before
invoking creating an new address space for a device, thus fixes the issue.

As a side work for the all groups/devices loop, also it checks if the device
has been assigned to the guest twice before creating an extra group and
removing it later which is not necessary.

Signed-off-by: Wei Xu 
---
 hw/vfio/common.c  | 28 
 include/hw/vfio/vfio-common.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7b2924c..63c3609 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -35,6 +35,7 @@
 #include "sysemu/kvm.h"
 #include "trace.h"
 #include "qapi/error.h"
+#include "hw/vfio/pci.h"
 
 struct vfio_group_head vfio_group_list =
 QLIST_HEAD_INITIALIZER(vfio_group_list);
@@ -1183,6 +1184,33 @@ static void vfio_disconnect_container(VFIOGroup *group)
 }
 }
 
+AddressSpace *vfio_lookup_as(int groupid, PCIDevice *pdev, Error **errp)
+{
+VFIOGroup *group;
+VFIODevice *vbasedev_iter;
+VFIOPCIDevice *vdev, *vd;
+
+vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+QLIST_FOREACH(group, _group_list, next) {
+QLIST_FOREACH(vbasedev_iter, >device_list, next) {
+if (strcmp(vbasedev_iter->name, vdev->vbasedev.name) == 0) {
+error_setg(errp, "device is already attached");
+return 0;
+}
+
+if (vbasedev_iter->group->groupid == groupid) {
+vd = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
+
+if (vd->pdev.bus == pdev->bus) {
+return vbasedev_iter->group->container->space->as;
+}
+}
+}
+}
+
+return pci_device_iommu_address_space(pdev);
+}
+
 VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
 {
 VFIOGroup *group;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index f3a2ac9..5b4827b 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -157,6 +157,7 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool 
enabled);
 void vfio_region_exit(VFIORegion *region);
 void vfio_region_finalize(VFIORegion *region);
 void vfio_reset_handler(void *opaque);
+AddressSpace *vfio_lookup_as(int groupid, PCIDevice *pdev, Error **errp);
 VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
 void vfio_put_group(VFIOGroup *group);
 int vfio_get_device(VFIOGroup *group, const char *name,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 4/5] s390x/css: support ccw IDA

[Halil Pasic]

On 09/06/2017 03:10 PM, Cornelia Huck wrote:
> On Tue,  5 Sep 2017 13:16:44 +0200
> Halil Pasic  wrote:
> 
>> Let's add indirect data addressing support for our virtual channel
>> subsystem. This implementation does no prefetching, but steps
>> trough the idal as we go.
>>
>> Signed-off-by: Halil Pasic 
>> ---
>>  hw/s390x/css.c | 110 
>> -
>>  1 file changed, 109 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index c1bc9944e6..9d8f9fd7ea 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -819,6 +819,114 @@ incr:
>>  return 0;
>>  }
>>  
>> +/* retuns values between 1 and bs, where bs is a power of 2 */
> 
> 'bs' is 'block size'?

Yes.

> 
>> +static inline uint16_t ida_continuous_left(hwaddr cda, uint64_t bs)
>> +{
>> +return bs - (cda & (bs - 1));
>> +}
>> +
>> +static inline uint64_t ccw_ida_block_size(uint8_t flags)
>> +{
>> +return 1ULL << (((flags ^ CDS_F_C64) & (CDS_F_C64 | CDS_F_I2K)) ? 11 : 
>> 12);
>> +}
>> +
>> +static inline int ida_read_next_idaw(CcwDataStream *cds)
>> +{
>> +union {uint64_t fmt2; uint32_t fmt1; } idaw;
>> +bool is_fmt2 = cds->flags & CDS_F_C64;
>> +int ret;
>> +hwaddr idaw_addr;
>> +
>> +if (is_fmt2) {
>> +idaw_addr = cds->cda_orig + sizeof(idaw.fmt2) * cds->at_idaw;
>> +if (idaw_addr & 0x07) {
>> +return -EINVAL; /* chanel program check */
> 
> How fashionable ;)
> 

Learned something new.

> [s/chanel/channel/ :D]
> 
>> +}
>> +ret = address_space_rw(_space_memory, idaw_addr,
>> +   MEMTXATTRS_UNSPECIFIED, (void *) ,
>> +   sizeof(idaw.fmt2), false);
>> +cds->cda = be64_to_cpu(idaw.fmt2);
>> +} else {
>> +idaw_addr = cds->cda_orig + sizeof(idaw.fmt1) * cds->at_idaw;
>> +if (idaw_addr & 0x03) {
>> +return -EINVAL; /* chanel program check */
>> +
>> +}
>> +ret = address_space_rw(_space_memory, idaw_addr,
>> +   MEMTXATTRS_UNSPECIFIED, (void *) ,
>> +   sizeof(idaw.fmt1), false);
>> +cds->cda = be64_to_cpu(idaw.fmt1);
>> +}
>> +++(cds->at_idaw);
>> +if (ret != MEMTX_OK) {
>> +/* assume inaccessible address */
>> +return -EINVAL; /* chanel program check */
>> +
>> +}
>> +return 0;
>> +}
>> +
>> +static int ccw_dstream_rw_ida(CcwDataStream *cds, void *buff, int len,
>> +  CcwDataStreamOp op)
>> +{
>> +uint64_t bs = ccw_ida_block_size(cds->flags);
>> +int ret = 0;
>> +uint16_t cont_left, iter_len;
>> +
>> +ret = cds_check_len(cds, len);
>> +if (ret <= 0) {
>> +return ret;
>> +}
>> +if (!cds->at_idaw) {
>> +/* read first idaw */
>> +ret = ida_read_next_idaw(cds);
>> +if (ret) {
>> +goto err;
>> +}
>> +cont_left = ida_continuous_left(cds->cda, bs);
>> +} else {
>> +cont_left = ida_continuous_left(cds->cda, bs);
>> +if (cont_left == bs) {
>> +ret = ida_read_next_idaw(cds);
>> +if (ret) {
>> +goto err;
>> +}
>> +if (cds->cda & (bs - 1)) {
>> +ret = -EINVAL; /* chanel program check */
>> +goto err;
>> +}
>> +}
>> +}
>> +do_iter_len:
>> +iter_len = MIN(len, cont_left);
>> +if (op == CDS_OP_A) {
>> +goto incr;
>> +}
>> +ret = address_space_rw(_space_memory, cds->cda,
>> +   MEMTXATTRS_UNSPECIFIED, buff, iter_len, op);
>> +if (ret != MEMTX_OK) {
>> +/* assume inaccessible address */
>> +ret = -EINVAL; /* chanel program check */
>> +goto err;
>> +}
>> +incr:
>> +cds->at_byte += iter_len;
>> +cds->cda += iter_len;
>> +len -= iter_len;
>> +if (len) {
>> +ret = ida_read_next_idaw(cds);
>> +if (ret) {
>> +goto err;
>> +}
>> +cont_left = bs;
>> +goto do_iter_len;
>> +}
>> +return ret;
>> +err:
>> +cds->flags |= CDS_F_STREAM_BROKEN;
>> +return ret;
>> +}
> 
> I'm not so happy about the many gotos (excepting the err ones). Any
> chance to get around these?
> 
Certainly possible:

static int ccw_dstream_rw_ida(CcwDataStream *cds, void *buff, int len,  
  CcwDataStreamOp op)   
{   
uint64_t bs = ccw_ida_block_size(cds->flags);   
int ret = 0;
uint16_t cont_left, iter_len;   

ret = 

Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h

[Eduardo Habkost]

On Mon, Sep 11, 2017 at 07:56:19PM +0200, David Hildenbrand wrote:
> 
> 
>   #endif
> >>>
> >>> Why not use typedefs.h?
> >>
> >> See Markus's reply.  But, maybe it's even better to use S390CPU* and
> >> include target/s390x/cpu-qom.h, which by design provides as little
> >> definitions as needed.
> > 
> > I don't see an argument against moving typedef CPUS390XState to
> > typedefs.h in Markus' reply.  I see one argument for it (reducing
> > the need for non-cyclic includes).
> > 
> > cpu-qom.h includes cpu.h, so I don't know why using S390CPU*
> > would solve any problem.  I don't disagree about changing the
> > function to use S390CPU* eventually, but it would still require
> > us make a choice between: a) including the header where the
> > typedef name is declared (cpu.h or cpu-qom.h); or b) moving the
> > typedef name declaration to typedefs.h.
> 
> It includes qom/cpu.h, not cpu.h. That's why using cpu-qom.h for such
> typedefs works (see v4).
> 

Oops, I was looking at an older tree (before commit 347b1a5c).
You're right, sorry for the noise.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Peter Maydell]

On 11 September 2017 at 19:01, Richard Henderson
 wrote:
> On 09/10/2017 07:28 PM, Philippe Mathieu-Daudé wrote:
>> changed in 659ef5cbb893, this fixes building with --enable-tcg-interpreter:
>>
>> /home/travis/build/qemu/qemu/tcg/tcg.c:116:14: error: 
>> ‘tcg_out_ldst_finalize’ used but never defined [-Werror]
>>  static bool tcg_out_ldst_finalize(TCGContext *s);
>>   ^
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>
> Oops.
>
> Reviewed-by: Richard Henderson 

Applied to master as a buildfix, thanks.

I've also turned on a tci compile check on my pre-merge tests.
(It doesn't pass "make check" for me, though...)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h

[Eduardo Habkost]

On Mon, Sep 11, 2017 at 12:23:10PM +0200, Paolo Bonzini wrote:
> On 10/09/2017 00:07, Eduardo Habkost wrote:
> > On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:
> >> On 08.09.2017 06:21, Thomas Huth wrote:
> >>> On 07.09.2017 22:13, David Hildenbrand wrote:
>  Implemented in sclp.c, so let's move it to the right include file.
>  Fix up one include. Do a forward declaration of CPUS390XState to fix the
>  two sclp consoles complaining.
> 
>  Signed-off-by: David Hildenbrand 
>  ---
>   include/hw/s390x/sclp.h| 2 ++
>   target/s390x/cpu.h | 1 -
>   target/s390x/misc_helper.c | 1 +
>   3 files changed, 3 insertions(+), 1 deletion(-)
> 
>  diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>  index a72d096081..4b86a8a293 100644
>  --- a/include/hw/s390x/sclp.h
>  +++ b/include/hw/s390x/sclp.h
>  @@ -242,5 +242,7 @@ sclpMemoryHotplugDev 
>  *init_sclp_memory_hotplug_dev(void);
>   sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
>   void sclp_service_interrupt(uint32_t sccb);
>   void raise_irq_cpu_hotplug(void);
>  +typedef struct CPUS390XState CPUS390XState;
>  +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
> >>>
> >>> That's dangerous and likely does not work with certain versions of GCC.
> >>> You can't do a "forward declaration" with typedef in C, I'm afraid. See
> >>> for example:
> >>>
> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html
> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html
> >>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef
> >>>
> >>> All this typedef'ing in QEMU is pretty bad ... we run into this problem
> >>> again and again. include/qemu/typedefs.h is just a work-around for this.
> >>> I know people like typedefs for some reasons (I used to do that, too,
> >>> before I realized the trouble with them), but IMHO we should rather
> >>> adopt the typedef-related rules from the kernel coding conventions 
> >>> instead:
> >>>
> >>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs
> >>>
> >>>   Thomas
> >>>
> >>
> >> Yes, this is really nasty. And I wasn't aware of the involved issues.
> >>
> >> This seems to be the only feasible solution (including cpu.h sounds
> >> wrong and will require a bunch of other includes):
> >>
> >>
> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> >> index a72d096081..ce80915a02 100644
> >> --- a/include/hw/s390x/sclp.h
> >> +++ b/include/hw/s390x/sclp.h
> >> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev
> >> *init_sclp_memory_hotplug_dev(void);
> >>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
> >>  void sclp_service_interrupt(uint32_t sccb);
> >>  void raise_irq_cpu_hotplug(void);
> >> +struct CPUS390XState;
> >> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,
> >> uint32_t code);
> >>
> >>  #endif
> > 
> > Why not use typedefs.h?
> 
> See Markus's reply.  But, maybe it's even better to use S390CPU* and
> include target/s390x/cpu-qom.h, which by design provides as little
> definitions as needed.

I don't see an argument against moving typedef CPUS390XState to
typedefs.h in Markus' reply.  I see one argument for it (reducing
the need for non-cyclic includes).

cpu-qom.h includes cpu.h, so I don't know why using S390CPU*
would solve any problem.  I don't disagree about changing the
function to use S390CPU* eventually, but it would still require
us make a choice between: a) including the header where the
typedef name is declared (cpu.h or cpu-qom.h); or b) moving the
typedef name declaration to typedefs.h.

> 
> > Signed-off-by: Eduardo Habkost 
> > ---
> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> > index 4b86a8a293..3512bf8283 100644
> > --- a/include/hw/s390x/sclp.h
> > +++ b/include/hw/s390x/sclp.h
> > @@ -242,7 +242,6 @@ sclpMemoryHotplugDev 
> > *init_sclp_memory_hotplug_dev(void);
> >  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
> >  void sclp_service_interrupt(uint32_t sccb);
> >  void raise_irq_cpu_hotplug(void);
> > -typedef struct CPUS390XState CPUS390XState;
> >  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
> >  
> >  #endif
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index 39bc8351a3..9c97bffa92 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -21,6 +21,7 @@ typedef struct Chardev Chardev;
> >  typedef struct CompatProperty CompatProperty;
> >  typedef struct CPUAddressSpace CPUAddressSpace;
> >  typedef struct CPUState CPUState;
> > +typedef struct CPUS390XState CPUS390XState;
> >  typedef struct DeviceListener DeviceListener;
> >  typedef struct DeviceState DeviceState;
> >  typedef struct DirtyBitmapSnapshot DirtyBitmapSnapshot;
> > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> > 

Re: [Qemu-devel] [PATCH v3 08/21] s390x: move sclp_service_call() to sclp.h

[Eduardo Habkost]

On Mon, Sep 11, 2017 at 04:23:09AM +0200, Thomas Huth wrote:
> On 10.09.2017 00:07, Eduardo Habkost wrote:
> > On Fri, Sep 08, 2017 at 02:46:36PM +0200, David Hildenbrand wrote:
> >> On 08.09.2017 06:21, Thomas Huth wrote:
> >>> On 07.09.2017 22:13, David Hildenbrand wrote:
>  Implemented in sclp.c, so let's move it to the right include file.
>  Fix up one include. Do a forward declaration of CPUS390XState to fix the
>  two sclp consoles complaining.
> 
>  Signed-off-by: David Hildenbrand 
>  ---
>   include/hw/s390x/sclp.h| 2 ++
>   target/s390x/cpu.h | 1 -
>   target/s390x/misc_helper.c | 1 +
>   3 files changed, 3 insertions(+), 1 deletion(-)
> 
>  diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>  index a72d096081..4b86a8a293 100644
>  --- a/include/hw/s390x/sclp.h
>  +++ b/include/hw/s390x/sclp.h
>  @@ -242,5 +242,7 @@ sclpMemoryHotplugDev 
>  *init_sclp_memory_hotplug_dev(void);
>   sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
>   void sclp_service_interrupt(uint32_t sccb);
>   void raise_irq_cpu_hotplug(void);
>  +typedef struct CPUS390XState CPUS390XState;
>  +int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
> >>>
> >>> That's dangerous and likely does not work with certain versions of GCC.
> >>> You can't do a "forward declaration" with typedef in C, I'm afraid. See
> >>> for example:
> >>>
> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01454.html
> >>>  https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg03337.html
> >>>  https://stackoverflow.com/questions/8367646/redefinition-of-typedef
> >>>
> >>> All this typedef'ing in QEMU is pretty bad ... we run into this problem
> >>> again and again. include/qemu/typedefs.h is just a work-around for this.
> >>> I know people like typedefs for some reasons (I used to do that, too,
> >>> before I realized the trouble with them), but IMHO we should rather
> >>> adopt the typedef-related rules from the kernel coding conventions 
> >>> instead:
> >>>
> >>>  https://www.kernel.org/doc/html/v4.13/process/coding-style.html#typedefs
> >>>
> >>>   Thomas
> >>>
> >>
> >> Yes, this is really nasty. And I wasn't aware of the involved issues.
> >>
> >> This seems to be the only feasible solution (including cpu.h sounds
> >> wrong and will require a bunch of other includes):
> >>
> >>
> >> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> >> index a72d096081..ce80915a02 100644
> >> --- a/include/hw/s390x/sclp.h
> >> +++ b/include/hw/s390x/sclp.h
> >> @@ -242,5 +242,7 @@ sclpMemoryHotplugDev
> >> *init_sclp_memory_hotplug_dev(void);
> >>  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
> >>  void sclp_service_interrupt(uint32_t sccb);
> >>  void raise_irq_cpu_hotplug(void);
> >> +struct CPUS390XState;
> >> +int sclp_service_call(struct CPUS390XState *env, uint64_t sccb,
> >> uint32_t code);
> >>
> >>  #endif
> > 
> > Why not use typedefs.h?
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> > index 4b86a8a293..3512bf8283 100644
> > --- a/include/hw/s390x/sclp.h
> > +++ b/include/hw/s390x/sclp.h
> > @@ -242,7 +242,6 @@ sclpMemoryHotplugDev 
> > *init_sclp_memory_hotplug_dev(void);
> >  sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
> >  void sclp_service_interrupt(uint32_t sccb);
> >  void raise_irq_cpu_hotplug(void);
> > -typedef struct CPUS390XState CPUS390XState;
> >  int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
> >  
> >  #endif
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index 39bc8351a3..9c97bffa92 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> 
> Using include/qeemu/typedefs.h here is IMHO really ugly. Do we really
> want to pollute a common include file with target specific code? My
> preferences are first to avoid typdefs, but if we really need/want them
> (do we? There is no comment about this in our coding styles), I think we
> should rather introduce target-specific typedefs.h headers, too, for
> everything that is not part of the common code.

I don't see any advantage in splitting typedefs.h into
arch-specific files.  We don't split typedefs.h into
subsystem-specific or device-specific headers, so I don't see we
would need a per-architecture split either.  typedefs.h is just a
global collection of type identifiers that helps us reduce header
dependency hell.

(Anyway, the current problem is now going solved by using
S390CPU* instead of CPUS390XState*, so there's no need to touch
typedefs.h this time.)

About keeping using typedefs: I don't have an strong opinion
for/against them[1], but I would prefer to keep style consistent
even if it's not explicitly documented.

[1] The fact that it would make typedefs.h completely unnecessary
makes me inclined towards the 

[Qemu-devel] [PATCH v1 4/6] kvm: we never have overlapping slots in kvm_set_phys_mem()

[David Hildenbrand]

The way flatview handles memory sections, we will never have overlapping
memory sections in kvm.

address_space_update_topology_pass() will make sure that we will only
get called for

a) an existing memory section for which we only update parameters
(log_start, log_stop).
b) an existing memory section we want to delete (region_del)
c) a brand new memory section we want to add (region_add)

We cannot have overlapping memory sections in kvm as we will first remove
the overlapping sections and then add the ones without conflicts.

Therefore we can remove the complexity for handling prefix and suffix
slots.

Signed-off-by: David Hildenbrand 
---
 accel/kvm/kvm-all.c | 68 +
 1 file changed, 11 insertions(+), 57 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 88b0e631bd..b677d1b13e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -725,7 +725,7 @@ kvm_check_extension_list(KVMState *s, const 
KVMCapabilityInfo *list)
 static void kvm_set_phys_mem(KVMMemoryListener *kml,
  MemoryRegionSection *section, bool add)
 {
-KVMSlot *mem, old;
+KVMSlot *mem;
 int err;
 MemoryRegion *mr = section->mr;
 bool writeable = !mr->readonly && !mr->rom_device;
@@ -750,28 +750,17 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
 ram = memory_region_get_ram_ptr(mr) + section->offset_within_region +
   (section->offset_within_address_space - start_addr);
 
-while (1) {
-mem = kvm_lookup_overlapping_slot(kml, start_addr, start_addr + size);
+mem = kvm_lookup_matching_slot(kml, start_addr, size);
+if (!add) {
 if (!mem) {
-break;
-}
-
-if (add && start_addr >= mem->start_addr &&
-(start_addr + size <= mem->start_addr + mem->memory_size) &&
-(ram - start_addr == mem->ram - mem->start_addr)) {
-/* The new slot fits into the existing one and comes with
- * identical parameters - update flags and done. */
-kvm_slot_update_flags(kml, mem, mr);
+g_assert(!memory_region_is_ram(mr) && !writeable && 
!mr->romd_mode);
 return;
 }
-
-old = *mem;
-
 if (mem->flags & KVM_MEM_LOG_DIRTY_PAGES) {
 kvm_physical_sync_dirty_bitmap(kml, section);
 }
 
-/* unregister the overlapping slot */
+/* unregister the slot */
 mem->memory_size = 0;
 err = kvm_set_user_memory_region(kml, mem);
 if (err) {
@@ -779,51 +768,16 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
 __func__, strerror(-err));
 abort();
 }
-
-/* register prefix slot */
-if (old.start_addr < start_addr) {
-mem = kvm_alloc_slot(kml);
-mem->memory_size = start_addr - old.start_addr;
-mem->start_addr = old.start_addr;
-mem->ram = old.ram;
-mem->flags =  kvm_mem_flags(mr);
-
-err = kvm_set_user_memory_region(kml, mem);
-if (err) {
-fprintf(stderr, "%s: error registering prefix slot: %s\n",
-__func__, strerror(-err));
-#ifdef TARGET_PPC
-fprintf(stderr, "%s: This is probably because your kernel's " \
-"PAGE_SIZE is too big. Please try to use 4k " \
-"PAGE_SIZE!\n", __func__);
-#endif
-abort();
-}
-}
-
-/* register suffix slot */
-if (old.start_addr + old.memory_size > start_addr + size) {
-ram_addr_t size_delta;
-
-mem = kvm_alloc_slot(kml);
-mem->start_addr = start_addr + size;
-size_delta = mem->start_addr - old.start_addr;
-mem->memory_size = old.memory_size - size_delta;
-mem->ram = old.ram + size_delta;
-mem->flags = kvm_mem_flags(mr);
-
-err = kvm_set_user_memory_region(kml, mem);
-if (err) {
-fprintf(stderr, "%s: error registering suffix slot: %s\n",
-__func__, strerror(-err));
-abort();
-}
-}
+return;
 }
 
-if (!add) {
+if (mem) {
+/* update the slot */
+kvm_slot_update_flags(kml, mem, mr);
 return;
 }
+
+/* register the new slot */
 mem = kvm_alloc_slot(kml);
 mem->memory_size = size;
 mem->start_addr = start_addr;
-- 
2.13.5

Re: [Qemu-devel] [PATCH] tcg/tci: do not use ldst label (never implemented)

[Peter Maydell]

On 11 September 2017 at 19:13, Philippe Mathieu-Daudé  wrote:
> On 09/10/2017 11:28 PM, Philippe Mathieu-Daudé wrote:
>>
>> changed in 659ef5cbb893, this fixes building with
>> --enable-tcg-interpreter:
>>
>> /home/travis/build/qemu/qemu/tcg/tcg.c:116:14: error:
>> ‘tcg_out_ldst_finalize’ used but never defined [-Werror]
>>   static bool tcg_out_ldst_finalize(TCGContext *s);
>>^
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>
>
> I'm not sure I can sign for Jincheng, but since his patch has the same
> content you could add:
>
> Signed-off-by: Jincheng Miao 

In that sort of case we generally just say "first person's
patch won". Extra signed-off-by tags are for where there's
really multiple peoples' work in the patch.

thanks
-- PMM

[Qemu-devel] [PATCH v1 1/6] kvm: require JOIN_MEMORY_REGIONS_WORKS

[David Hildenbrand]

We already require DESTROY_MEMORY_REGION_WORKS, JOIN_MEMORY_REGIONS_WORKS
was added just half a year later.

In addition, with flatview overlapping memory regions are first
removed before adding the changed one. So we can't really detect joining
memory regions this way.

Let's just get rid of this special handling.

Signed-off-by: David Hildenbrand 
---
 accel/kvm/kvm-all.c | 42 +-
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f85553a851..985b179ab6 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -79,7 +79,6 @@ struct KVMState
 int coalesced_mmio;
 struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
 bool coalesced_flush_in_progress;
-int broken_set_mem_region;
 int vcpu_events;
 int robust_singlestep;
 int debugregs;
@@ -127,6 +126,7 @@ static bool kvm_immediate_exit;
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
 KVM_CAP_INFO(USER_MEMORY),
 KVM_CAP_INFO(DESTROY_MEMORY_REGION_WORKS),
+KVM_CAP_INFO(JOIN_MEMORY_REGIONS_WORKS),
 KVM_CAP_LAST_INFO
 };
 
@@ -696,7 +696,6 @@ kvm_check_extension_list(KVMState *s, const 
KVMCapabilityInfo *list)
 static void kvm_set_phys_mem(KVMMemoryListener *kml,
  MemoryRegionSection *section, bool add)
 {
-KVMState *s = kvm_state;
 KVMSlot *mem, old;
 int err;
 MemoryRegion *mr = section->mr;
@@ -763,35 +762,6 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
 abort();
 }
 
-/* Workaround for older KVM versions: we can't join slots, even not by
- * unregistering the previous ones and then registering the larger
- * slot. We have to maintain the existing fragmentation. Sigh.
- *
- * This workaround assumes that the new slot starts at the same
- * address as the first existing one. If not or if some overlapping
- * slot comes around later, we will fail (not seen in practice so far)
- * - and actually require a recent KVM version. */
-if (s->broken_set_mem_region &&
-old.start_addr == start_addr && old.memory_size < size && add) {
-mem = kvm_alloc_slot(kml);
-mem->memory_size = old.memory_size;
-mem->start_addr = old.start_addr;
-mem->ram = old.ram;
-mem->flags = kvm_mem_flags(mr);
-
-err = kvm_set_user_memory_region(kml, mem);
-if (err) {
-fprintf(stderr, "%s: error updating slot: %s\n", __func__,
-strerror(-err));
-abort();
-}
-
-start_addr += old.memory_size;
-ram += old.memory_size;
-size -= old.memory_size;
-continue;
-}
-
 /* register prefix slot */
 if (old.start_addr < start_addr) {
 mem = kvm_alloc_slot(kml);
@@ -833,10 +803,6 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
 }
 }
 
-/* in case the KVM bug workaround already "consumed" the new slot */
-if (!size) {
-return;
-}
 if (!add) {
 return;
 }
@@ -1692,12 +1658,6 @@ static int kvm_init(MachineState *ms)
 
 s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
 
-s->broken_set_mem_region = 1;
-ret = kvm_check_extension(s, KVM_CAP_JOIN_MEMORY_REGIONS_WORKS);
-if (ret > 0) {
-s->broken_set_mem_region = 0;
-}
-
 #ifdef KVM_CAP_VCPU_EVENTS
 s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
 #endif
-- 
2.13.5