Re: [Qemu-devel] [PATCH] microbit: make -kernel optional

[Stefan Hajnoczi]

On Fri, Dec 14, 2018 at 02:51:52PM +, Stefan Hajnoczi wrote:
> ARMv7M machine types support -kernel for ELF and raw image files.
> Microbit programs are typically in Intel HEX (.hex) format.  The generic
> loader supports .hex files but it doesn't work as expected:
> 
>   $ qemu-system-arm -M microbit -device loader,file=microbit.hex
>   Guest image must be specified (using -kernel)
> 
> This error comes from armv7m_load_kernel() but we don't have -kernel in
> this case.
> 
> This patch makes -kernel optional since most of the time we'll want to
> use -device loader instead.
> 
> Note that we need to register the reset handler that
> armv7m_load_kernel() used to register for us.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/arm/microbit.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Ping

> diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
> index a734e7f650..638f638792 100644
> --- a/hw/arm/microbit.c
> +++ b/hw/arm/microbit.c
> @@ -14,6 +14,7 @@
>  #include "hw/arm/arm.h"
>  #include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
> +#include "qemu/error-report.h"
>  
>  #include "hw/arm/nrf51_soc.h"
>  
> @@ -28,6 +29,13 @@ typedef struct {
>  #define MICROBIT_MACHINE(obj) \
>  OBJECT_CHECK(MicrobitMachineState, obj, TYPE_MICROBIT_MACHINE)
>  
> +static void microbit_cpu_reset(void *opaque)
> +{
> +ARMCPU *cpu = opaque;
> +
> +cpu_reset(CPU(cpu));
> +}
> +
>  static void microbit_init(MachineState *machine)
>  {
>  MicrobitMachineState *s = MICROBIT_MACHINE(machine);
> @@ -41,8 +49,13 @@ static void microbit_init(MachineState *machine)
>   &error_fatal);
>  object_property_set_bool(soc, true, "realized", &error_fatal);
>  
> -armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> -   NRF51_SOC(soc)->flash_size);
> +if (machine->kernel_filename) {
> +armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
> +   NRF51_SOC(soc)->flash_size);
> +} else {
> +/* armv7m_load_kernel() does this, we need to do it manually here */
> +qemu_register_reset(microbit_cpu_reset, ARM_CPU(first_cpu));
> +}
>  }
>  
>  static void microbit_machine_class_init(ObjectClass *oc, void *data)
> -- 
> 2.19.2
> 
> 


signature.asc
Description: PGP signature

[Qemu-devel] Q: nested virtualization and VMX live migration

[Syrone Wong]

Hi all,

The VMX live migration blocker is added via
https://github.com/qemu/qemu/commit/d98f26073bebddcd3da0ba1b86c3a34e840c0fb8,
it claims
"Nested VMX does not support live migration yet".

While a commit from Linux 4.20 states
> With live migration support and finally a good solution for exception event 
> injection,
> nested VMX should be ready for having a stable userspace ABI.

and the nested virtualization is enabled by default. Should we remove
the blocker for Linux 4.20 and upper?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1e58e5e59148916fa43444a406335a990783fb78

Best Regards,
Syrone Wong

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add some missing SPARC-related files

[Thomas Huth]

On 2019-01-02 20:34, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 1/2/19 4:46 PM, Thomas Huth wrote:
>> These files / devices are only used by SPARC machines, so we can sort
>> them into the corresponding categories in the MAINTAINERS file.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  MAINTAINERS | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index b6c2645..e897963 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1069,9 +1069,11 @@ M: Mark Cave-Ayland 
>>  S: Maintained
>>  F: hw/sparc/sun4m.c
>>  F: hw/sparc/sun4m_iommu.c
>> +F: hw/display/cg3.c
>> +F: hw/display/tcx.c
>>  F: hw/dma/sparc32_dma.c
>>  F: hw/misc/eccmemctl.c
>> -F: hw/misc/slavio_misc.c
>> +F: hw/*/slavio_*.c
>>  F: include/hw/sparc/sparc32_dma.h
>>  F: pc-bios/openbios-sparc32
>>  F: include/hw/sparc/sun4m_iommu.h
>> @@ -1083,6 +1085,8 @@ F: hw/sparc64/sun4u.c
>>  F: pc-bios/openbios-sparc64
>>  F: hw/pci-host/sabre.c
>>  F: include/hw/pci-host/sabre.h
>> +F: include/hw/sparc/sun4u_iommu.h
> 
> I sent a patch to trivial@ with other sun4u changes, reviewed by Mark:
> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04467.html

Oh, right, sorry!

> Neither mine nor your are complete :)
> Can I include your patch when respining the series? (Not sure I need to
> respin although).

Sure!

>> +F: include/hw/nvram/sun_nvram.h
> 
> sun_nvram.h is also used by sun4m (the previous section this patch updates).

Right. So it should maybe be listed twice... (but since the maintainer
of sun4u and sun4m is the same, it likely does not really matter)

 Thomas

Re: [Qemu-devel] [PATCH v2 0/1] drive-mirror: add incremental mode

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190102100415.24680-1-mahaocong_w...@163.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

libpmem support   no
libudev   no

WARNING: Use of SDL 1.2 is deprecated and will be removed in
WARNING: future releases. Please switch to using SDL 2.0

NOTE: cross-compilers enabled:  'cc'
  GEN x86_64-softmmu/config-devices.mak.tmp
---
  CC  block/io.o
  CC  block/create.o
/tmp/qemu-test/src/block/mirror.c: In function 'mirror_dirty_init_incremental':
/tmp/qemu-test/src/block/mirror.c:832:5: error: implicit declaration of 
function 'bdrv_copy_dirty_bitmap' [-Werror=implicit-function-declaration]
 if (!bdrv_copy_dirty_bitmap(src, dest, NULL)) {
 ^
/tmp/qemu-test/src/block/mirror.c:832:5: error: nested extern declaration of 
'bdrv_copy_dirty_bitmap' [-Werror=nested-externs]
/tmp/qemu-test/src/block/mirror.c: At top level:
cc1: error: unrecognized command line option "-Wno-format-truncation" [-Werror]
cc1: all warnings being treated as errors
make: *** [block/mirror.o] Error 1
make: *** Waiting for unfinished jobs


The full log is available at
http://patchew.org/logs/20190102100415.24680-1-mahaocong_w...@163.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH v2 0/1] drive-mirror: add incremental mode

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190102100415.24680-1-mahaocong_w...@163.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

  CC  block/sheepdog.o
  CC  block/accounting.o
/tmp/qemu-test/src/block/mirror.c: In function 'mirror_dirty_init_incremental':
/tmp/qemu-test/src/block/mirror.c:832:10: error: implicit declaration of 
function 'bdrv_copy_dirty_bitmap'; did you mean 'bdrv_clear_dirty_bitmap'? 
[-Werror=implicit-function-declaration]
 if (!bdrv_copy_dirty_bitmap(src, dest, NULL)) {
  ^~
  bdrv_clear_dirty_bitmap
/tmp/qemu-test/src/block/mirror.c:832:10: error: nested extern declaration of 
'bdrv_copy_dirty_bitmap' [-Werror=nested-externs]
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/mirror.o] Error 1
make: *** Waiting for unfinished jobs


The full log is available at
http://patchew.org/logs/20190102100415.24680-1-mahaocong_w...@163.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS

[David Gibson]

On Wed, Jan 02, 2019 at 06:57:42AM +0100, Cédric Le Goater wrote:
> The 'dual' sPAPR IRQ backend supports both interrupt mode, XIVE
> exploitation mode and the legacy compatibility mode (XICS). both modes
> are not supported at the same time.
> 
> The machine starts with the legacy mode and a new interrupt mode can
> then be negotiated by the CAS process. In this case, the new mode is
> activated after a reset to take into account the required changes in
> the machine. These impact the device tree layout, the interrupt
> presenter object and the exposed MMIO regions in the case of XIVE.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/ppc/spapr_irq.h |   1 +
>  hw/ppc/spapr.c |  10 ++-
>  hw/ppc/spapr_hcall.c   |  11 +++
>  hw/ppc/spapr_irq.c | 179 +
>  4 files changed, 198 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 283bb5002c16..14b02c3aca33 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -52,6 +52,7 @@ typedef struct sPAPRIrq {
>  extern sPAPRIrq spapr_irq_xics;
>  extern sPAPRIrq spapr_irq_xics_legacy;
>  extern sPAPRIrq spapr_irq_xive;
> +extern sPAPRIrq spapr_irq_dual;
>  
>  void spapr_irq_init(sPAPRMachineState *spapr, Error **errp);
>  int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error 
> **errp);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5e8ffda47372..eb8ef741d860 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2633,11 +2633,11 @@ static void spapr_machine_init(MachineState *machine)
>  spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>  
>  /* advertise XIVE on POWER9 machines */
> -if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> +if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
>  if (ppc_type_check_compat(machine->cpu_type, 
> CPU_POWERPC_LOGICAL_3_00,
>0, spapr->max_compat_pvr)) {
>  spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> -} else {
> +} else if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
>  error_report("XIVE-only machines require a POWER9 CPU");
>  exit(1);
>  }
> @@ -3063,6 +3063,8 @@ static char *spapr_get_ic_mode(Object *obj, Error 
> **errp)
>  return g_strdup("xics");
>  } else if (spapr->irq == &spapr_irq_xive) {
>  return g_strdup("xive");
> +} else if (spapr->irq == &spapr_irq_dual) {
> +return g_strdup("dual");
>  }
>  g_assert_not_reached();
>  }
> @@ -3076,6 +3078,8 @@ static void spapr_set_ic_mode(Object *obj, const char 
> *value, Error **errp)
>  spapr->irq = &spapr_irq_xics;
>  } else if (strcmp(value, "xive") == 0) {
>  spapr->irq = &spapr_irq_xive;
> +} else if (strcmp(value, "dual") == 0) {
> +spapr->irq = &spapr_irq_dual;
>  } else {
>  error_setg(errp, "Bad value for \"ic-mode\" property");
>  }
> @@ -3124,7 +3128,7 @@ static void spapr_instance_init(Object *obj)
>  object_property_add_str(obj, "ic-mode", spapr_get_ic_mode,
>  spapr_set_ic_mode, NULL);
>  object_property_set_description(obj, "ic-mode",
> - "Specifies the interrupt controller mode (xics, xive)",
> + "Specifies the interrupt controller mode (xics, xive, 
> dual)",
>   NULL);
>  }
>  
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ae913d070f50..45c35d41fac6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1654,6 +1654,17 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  (spapr_h_cas_compose_response(spapr, args[1], args[2],
>ov5_updates) != 0);
>  }
> +
> +/*
> + * Generate a machine reset when we have an update of the
> + * interrupt mode. Only required when the machine supports both
> + * modes.
> + */
> +if (!spapr->cas_reboot) {
> +spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> +&& spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH;
> +}
> +
>  spapr_ovec_cleanup(ov5_updates);
>  
>  if (spapr->cas_reboot) {
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d23914887ac0..d110b8cdeec7 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -230,6 +230,11 @@ static void spapr_irq_set_irq_xics(void *opaque, int 
> srcno, int val)
>  }
>  }
>  
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> +/* TODO: create the KVM XICS device */
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS \
>  (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -247,6 +252,7 @@ sPAPRIrq spapr_irq_xics = {
>  .dt_populate = spapr_dt_xics,
>  .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>

Re: [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0

[David Gibson]

On Wed, Jan 02, 2019 at 06:57:41AM +0100, Cédric Le Goater wrote:
> commit 15ed653fa49a ("ppc/xics: An ICS with offset 0 is assumed to be
> uninitialized") introduced an extra check on the ICS offset which is
> not strictly necessary.

The commit message for that suggests it was added to make pnv easier.
I take it you no longer need this for the current or expected pnv
code?

> Revert the change to be able to map the XICS IRQ number space on the
> XIVE IRQ number space.



> 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/ppc/xics.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 7668c381a887..07508cbd217e 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -139,8 +139,7 @@ struct ICSState {
>  
>  static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
>  {
> -return (ics->offset != 0) && (nr >= ics->offset)
> -&& (nr < (ics->offset + ics->nr_irqs));
> +return (nr >= ics->offset) && (nr < (ics->offset + ics->nr_irqs));
>  }
>  
>  struct ICSIRQState {

-- 
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 03/15] pci: allow cleanup/unregistration of PCI root buses

[David Gibson]

On Wed, Jan 02, 2019 at 10:27:14PM -0500, Michael S. Tsirkin wrote:
> On Thu, Jan 03, 2019 at 11:36:33AM +1100, David Gibson wrote:
> > On Fri, Dec 21, 2018 at 11:19:18AM -0500, Michael S. Tsirkin wrote:
> > > On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> > > > From: Michael Roth 
> > > > 
> > > > This adds cleanup counterparts to pci_register_root_bus(),
> > > > pci_root_bus_new(), and pci_bus_irqs().
> > > > 
> > > > These cleanup routines are needed in the case of hotpluggable
> > > > PCIHostBridge implementations. Currently we can rely on the
> > > > object_unparent()'ing of the PCIHostState recursively unparenting
> > > > and cleaning up it's child buses, but we need explicit calls
> > > > to also:
> > > > 
> > > >   1) remove the PCIHostState from pci_host_bridges global list.
> > > >  otherwise, we risk accessing freed memory when we access
> > > >  the list later
> > > >   2) clean up memory allocated in pci_bus_irqs()
> > > > 
> > > > Both are handled outside the context of any particular bus or
> > > > host bridge's init/realize functions, making it difficult to
> > > > avoid the need for explicit cleanup functions without remodeling
> > > > how PCIHostBridges are created. So keep it simple and just add
> > > > them for now.
> > > > 
> > > > Cc: Michael S. Tsirkin 
> > > > Cc: Paolo Bonzini 
> > > > Signed-off-by: Michael Roth 
> > > > Reviewed-by: David Gibson 
> > > > Signed-off-by: Greg Kurz 
> > > 
> > > Reviewed-by: Michael S. Tsirkin 
> > 
> > I've applied this tentatively to ppc-for-4.0.  Let me know, Michael,
> > if you'd prefer to take it through your tree.
> 
> 
> I think your tree makes sense for this.

Great, will do.

> 
> > > 
> > > > ---
> > > >  hw/pci/pci.c |   33 +
> > > >  include/hw/pci/pci.h |3 +++
> > > >  2 files changed, 36 insertions(+)
> > > > 
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index efb5ce196ffb..16354f91206c 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState 
> > > > *host)
> > > >  QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> > > >  }
> > > >  
> > > > +static void pci_host_bus_unregister(DeviceState *host)
> > > > +{
> > > > +PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> > > > +
> > > > +QLIST_REMOVE(host_bridge, next);
> > > > +}
> > > > +
> > > >  PCIBus *pci_device_root_bus(const PCIDevice *d)
> > > >  {
> > > >  PCIBus *bus = pci_get_bus(d);
> > > > @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, 
> > > > DeviceState *parent,
> > > >  pci_host_bus_register(parent);
> > > >  }
> > > >  
> > > > +static void pci_bus_uninit(PCIBus *bus)
> > > > +{
> > > > +pci_host_bus_unregister(BUS(bus)->parent);
> > > > +}
> > > > +
> > > >  bool pci_bus_is_express(PCIBus *bus)
> > > >  {
> > > >  return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > > > @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, 
> > > > const char *name,
> > > >  return bus;
> > > >  }
> > > >  
> > > > +void pci_root_bus_cleanup(PCIBus *bus)
> > > > +{
> > > > +pci_bus_uninit(bus);
> > > > +object_unparent(OBJECT(bus));
> > > > +}
> > > > +
> > > >  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn 
> > > > map_irq,
> > > >void *irq_opaque, int nirq)
> > > >  {
> > > > @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn 
> > > > set_irq, pci_map_irq_fn map_irq,
> > > >  bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> > > >  }
> > > >  
> > > > +void pci_bus_irqs_cleanup(PCIBus *bus)
> > > > +{
> > > > +bus->set_irq = NULL;
> > > > +bus->map_irq = NULL;
> > > > +bus->irq_opaque = NULL;
> > > > +bus->nirq = 0;
> > > > +g_free(bus->irq_count);
> > > > +}
> > > > +
> > > >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > > >pci_set_irq_fn set_irq, pci_map_irq_fn 
> > > > map_irq,
> > > >void *irq_opaque,
> > > > @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, 
> > > > const char *name,
> > > >  return bus;
> > > >  }
> > > >  
> > > > +void pci_unregister_root_bus(PCIBus *bus)
> > > > +{
> > > > +pci_bus_irqs_cleanup(bus);
> > > > +pci_root_bus_cleanup(bus);
> > > > +}
> > > > +
> > > >  int pci_bus_num(PCIBus *s)
> > > >  {
> > > >  return PCI_BUS_GET_CLASS(s)->bus_num(s);
> > > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > > index e6514bba23aa..8998e3be3390 100644
> > > > --- a/include/hw/pci/pci.h
> > > > +++ b/include/hw/pci/pci.h
> > > > @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, 
> > > > const char *name,
> > > >   MemoryRegion *address_space_mem,
> > > >   MemoryRegion *address_space_io,
> > > >   

Re: [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU

[David Gibson]

On Wed, Jan 02, 2019 at 06:57:35AM +0100, Cédric Le Goater wrote:
> which will be used by the machine only when the XIVE interrupt mode is
> in use.

I don't love the idea of putting a hook this specific into the
PowerPCCPU structure, though it might be the easiest path in the short
term.

A couple of approaches: 1) revisit my changes to allow for a pointer
to machine-defined per-cpu data.  or 2) do we actually need a cpu to
tctx pointer.

Expanding on (2) - here you use the pointer to find the right TIMA
state to access, but that could also be handled by having different
TIMA IO instances and mapping those individually to cpu_as.  On the
interrupt delivery side I think a tctx to cpu link will suffice.  For
sPAPR there might be complications with translating cpu numbers in
hcalls to the right tctx.

> 
> Signed-off-by: Cédric Le Goater 
> ---
>  target/ppc/cpu.h| 2 ++
>  hw/intc/xive.c  | 6 +++---
>  hw/ppc/spapr_cpu_core.c | 7 ++-
>  hw/ppc/spapr_irq.c  | 8 
>  4 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index d5f99f1fc7b9..c76036985623 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1177,6 +1177,7 @@ do {\
>  
>  typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
>  typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
> +typedef struct XiveTCTX XiveTCTX;
>  
>  /**
>   * PowerPCCPU:
> @@ -1196,6 +1197,7 @@ struct PowerPCCPU {
>  uint32_t compat_pvr;
>  PPCVirtualHypervisor *vhyp;
>  Object *intc;
> +XiveTCTX *tctx;
>  void *machine_data;
>  int32_t node_id; /* NUMA node this CPU belongs to */
>  PPCHash64Options *hash64_opts;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index ea33494338dc..410c53278a11 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -321,7 +321,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>uint64_t value, unsigned size)
>  {
>  PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> -XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
> +XiveTCTX *tctx = cpu->tctx;
>  const XiveTmOp *xto;
>  
>  /*
> @@ -360,7 +360,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>  static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>  {
>  PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> -XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
> +XiveTCTX *tctx = cpu->tctx;
>  const XiveTmOp *xto;
>  
>  /*
> @@ -1186,7 +1186,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, 
> uint8_t format,
>  
>  CPU_FOREACH(cs) {
>  PowerPCCPU *cpu = POWERPC_CPU(cs);
> -XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
> +XiveTCTX *tctx = cpu->tctx;
>  int ring;
>  
>  /*
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 2739b2a4b818..1473ef853336 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -194,7 +194,12 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, 
> sPAPRCPUCore *sc)
>  vmstate_unregister(NULL, &vmstate_spapr_cpu_state, 
> cpu->machine_data);
>  }
>  qemu_unregister_reset(spapr_cpu_reset, cpu);
> -object_unparent(cpu->intc);
> +if (cpu->intc) {
> +object_unparent(cpu->intc);
> +}
> +if (cpu->tctx) {
> +object_unparent(OBJECT(cpu->tctx));
> +}
>  cpu_remove_sync(CPU(cpu));
>  object_unparent(OBJECT(cpu));
>  }
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 2d2e17b66533..8d028db44ff4 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -305,7 +305,7 @@ static void spapr_irq_print_info_xive(sPAPRMachineState 
> *spapr,
>  CPU_FOREACH(cs) {
>  PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
> -xive_tctx_pic_print_info(XIVE_TCTX(cpu->intc), mon);
> +xive_tctx_pic_print_info(cpu->tctx, mon);
>  }
>  
>  spapr_xive_pic_print_info(spapr->xive, mon);
> @@ -323,13 +323,13 @@ static void 
> spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
>  return;
>  }
>  
> -cpu->intc = obj;
> +cpu->tctx = XIVE_TCTX(obj);
>  
>  /*
>   * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
>   * don't beneficiate from the reset of the XIVE IRQ backend
>   */
> -spapr_xive_set_tctx_os_cam(XIVE_TCTX(obj));
> +spapr_xive_set_tctx_os_cam(cpu->tctx);
>  }
>  
>  static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
> @@ -345,7 +345,7 @@ static void spapr_irq_reset_xive(sPAPRMachineState 
> *spapr, Error **errp)
>  PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
>  /* (TCG) Set the OS CAM line of the thread interrupt context. */
> -spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
> +spapr_xive_set_tctx_os_cam(cpu->tctx);
>  }
>  }
>  

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dro

Re: [Qemu-devel] [PATCH for-4.0 v9 10/16] qemu_thread: supplement error handling for h_resize_hpt_prepare

[David Gibson]

On Wed, Jan 02, 2019 at 02:44:17PM +0800, 李菲 wrote:
> 
> 在 2019/1/2 上午10:36, David Gibson 写道:
> > On Tue, Dec 25, 2018 at 10:04:43PM +0800, Fei Li wrote:
> > > Add a local_err to hold the error, and return the corresponding
> > > error code to replace the temporary &error_abort.
> > > 
> > > Cc: Markus Armbruster 
> > > Cc: David Gibson 
> > > Signed-off-by: Fei Li 
> > This looks like a good change, but it no longer applies due to a
> > change in the qemu_thread_create() signature.
> Sorry that I am not sure whether I understand. Do you mean using
> &error_abort is more suitable for this handling, rather than report
> the &local_err & return a failure reason?

No, I just mean that context has been altered by a global change and
the patch will need to be fixed up to cope with that.

> > 
> > > ---
> > >   hw/ppc/spapr_hcall.c | 12 
> > >   1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > > index 5bc2cf4540..7c16ade04a 100644
> > > --- a/hw/ppc/spapr_hcall.c
> > > +++ b/hw/ppc/spapr_hcall.c
> > > @@ -478,6 +478,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU 
> > > *cpu,
> > >   sPAPRPendingHPT *pending = spapr->pending_hpt;
> > >   uint64_t current_ram_size;
> > >   int rc;
> > > +Error *local_err = NULL;
> > >   if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
> > >   return H_AUTHORITY;
> > > @@ -538,10 +539,13 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU 
> > > *cpu,
> > >   pending->shift = shift;
> > >   pending->ret = H_HARDWARE;
> > > -/* TODO: let the further caller handle the error instead of abort() 
> > > here */
> > > -qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
> > > -   hpt_prepare_thread, pending,
> > > -   QEMU_THREAD_DETACHED, &error_abort);
> > > +if (!qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
> > > +hpt_prepare_thread, pending,
> > > +QEMU_THREAD_DETACHED, &local_err)) {
> > > +error_reportf_err(local_err, "failed to create 
> > > hpt_prepare_thread: ");
> > > +g_free(pending);
> > > +return H_RESOURCE;
> > I also think H_HARDWARE would be a better choice here.  Although the
> > failure is due to a resource constraint, it's not because the guest
> > asked for too much, just because the host is in dire straits.  From
> > the guest's point of view it's basically a hardware failure.
> 
> Ok, thanks. Will use H_HARDWARE instead.
> 
> Have a nice day, thanks for the review. :)
> Fei
> > 
> > > +}
> > >   spapr->pending_hpt = pending;
> 

-- 
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 04/10] spapr/xive: simplify the sPAPR IRQ qirq method for XIVE

[David Gibson]

On Wed, Jan 02, 2019 at 06:57:37AM +0100, Cédric Le Goater wrote:
> The qirq routines of the XiveSource and the sPAPRXive model are only
> used under the sPAPR IRQ backend. Simplify the overall call stack and
> gather all the code under spapr_qirq_xive(). It will ease future
> changes.
> 
> Signed-off-by: Cédric Le Goater 

Seems a good clean up in its own right, so I've applied.

> ---
>  include/hw/ppc/spapr_xive.h |  1 -
>  include/hw/ppc/xive.h   |  6 --
>  hw/intc/spapr_xive.c| 14 --
>  hw/ppc/spapr_irq.c  | 12 +++-
>  4 files changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 728735dbcfbe..9ee524fdb218 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -40,7 +40,6 @@ typedef struct sPAPRXive {
>  bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
>  bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
>  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
> -qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn);
>  
>  typedef struct sPAPRMachineState sPAPRMachineState;
>  
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 18cd114eb244..b05fe88b5b82 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -278,12 +278,6 @@ uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t 
> srcno, uint8_t pq);
>  void xive_source_pic_print_info(XiveSource *xsrc, uint32_t offset,
>  Monitor *mon);
>  
> -static inline qemu_irq xive_source_qirq(XiveSource *xsrc, uint32_t srcno)
> -{
> -assert(srcno < xsrc->nr_irqs);
> -return xsrc->qirqs[srcno];
> -}
> -
>  static inline bool xive_source_irq_is_lsi(XiveSource *xsrc, uint32_t srcno)
>  {
>  assert(srcno < xsrc->nr_irqs);
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 0e39c90cbd07..eea28337e807 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -488,20 +488,6 @@ bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn)
>  return true;
>  }
>  
> -qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn)
> -{
> -XiveSource *xsrc = &xive->source;
> -
> -if (lisn >= xive->nr_irqs) {
> -return NULL;
> -}
> -
> -/* The sPAPR machine/device should have claimed the IRQ before */
> -assert(xive_eas_is_valid(&xive->eat[lisn]));
> -
> -return xive_source_qirq(xsrc, lisn);
> -}
> -
>  /*
>   * XIVE hcalls
>   *
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 50e767120d21..b875065ef86b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -294,7 +294,17 @@ static void spapr_irq_free_xive(sPAPRMachineState 
> *spapr, int irq, int num)
>  
>  static qemu_irq spapr_qirq_xive(sPAPRMachineState *spapr, int irq)
>  {
> -return spapr_xive_qirq(spapr->xive, irq);
> +sPAPRXive *xive = spapr->xive;
> +XiveSource *xsrc = &xive->source;
> +
> +if (irq >= xive->nr_irqs) {
> +return NULL;
> +}
> +
> +/* The sPAPR machine/device should have claimed the IRQ before */
> +assert(xive_eas_is_valid(&xive->eat[irq]));
> +
> +return xsrc->qirqs[irq];
>  }
>  
>  static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,

-- 
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 1/3] tests: vhost-user-test: initialize 'fd' in chr_read

[Li Qiang]

Thomas Huth  于2019年1月2日周三 下午9:50写道：

> On 2018-12-15 13:03, Li Qiang wrote:
> > Currently when processing VHOST_USER_SET_VRING_CALL
> > if 'qemu_chr_fe_get_msgfds' get no fd, the 'fd' will
> > be a stack uninitialized value.
> >
> > Signed-off-by: Li Qiang 
> > ---
> >  tests/vhost-user-test.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> > index 45d58d8ea2..86039e61e0 100644
> > --- a/tests/vhost-user-test.c
> > +++ b/tests/vhost-user-test.c
> > @@ -309,7 +309,7 @@ static void chr_read(void *opaque, const uint8_t
> *buf, int size)
> >  CharBackend *chr = &s->chr;
> >  VhostUserMsg msg;
> >  uint8_t *p = (uint8_t *) &msg;
> > -int fd;
> > +int fd = -1;
> >
> >  if (s->test_fail) {
> >  qemu_chr_fe_disconnect(chr);
> >
>
> Shouldn't we also rather check the return code of
> qemu_chr_fe_get_msgfds() ?


Agree, there are several places need to do this. I will send out a patch
later.

Thanks,
Li Qiang


> Anyway, initializing fd to -1 here sounds
> like a good idea, so:
>
> Reviewed-by: Thomas Huth 
>

Re: [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses

[Michael S. Tsirkin]

On Thu, Jan 03, 2019 at 11:36:33AM +1100, David Gibson wrote:
> On Fri, Dec 21, 2018 at 11:19:18AM -0500, Michael S. Tsirkin wrote:
> > On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> > > From: Michael Roth 
> > > 
> > > This adds cleanup counterparts to pci_register_root_bus(),
> > > pci_root_bus_new(), and pci_bus_irqs().
> > > 
> > > These cleanup routines are needed in the case of hotpluggable
> > > PCIHostBridge implementations. Currently we can rely on the
> > > object_unparent()'ing of the PCIHostState recursively unparenting
> > > and cleaning up it's child buses, but we need explicit calls
> > > to also:
> > > 
> > >   1) remove the PCIHostState from pci_host_bridges global list.
> > >  otherwise, we risk accessing freed memory when we access
> > >  the list later
> > >   2) clean up memory allocated in pci_bus_irqs()
> > > 
> > > Both are handled outside the context of any particular bus or
> > > host bridge's init/realize functions, making it difficult to
> > > avoid the need for explicit cleanup functions without remodeling
> > > how PCIHostBridges are created. So keep it simple and just add
> > > them for now.
> > > 
> > > Cc: Michael S. Tsirkin 
> > > Cc: Paolo Bonzini 
> > > Signed-off-by: Michael Roth 
> > > Reviewed-by: David Gibson 
> > > Signed-off-by: Greg Kurz 
> > 
> > Reviewed-by: Michael S. Tsirkin 
> 
> I've applied this tentatively to ppc-for-4.0.  Let me know, Michael,
> if you'd prefer to take it through your tree.


I think your tree makes sense for this.

> > 
> > > ---
> > >  hw/pci/pci.c |   33 +
> > >  include/hw/pci/pci.h |3 +++
> > >  2 files changed, 36 insertions(+)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index efb5ce196ffb..16354f91206c 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
> > >  QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> > >  }
> > >  
> > > +static void pci_host_bus_unregister(DeviceState *host)
> > > +{
> > > +PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> > > +
> > > +QLIST_REMOVE(host_bridge, next);
> > > +}
> > > +
> > >  PCIBus *pci_device_root_bus(const PCIDevice *d)
> > >  {
> > >  PCIBus *bus = pci_get_bus(d);
> > > @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, 
> > > DeviceState *parent,
> > >  pci_host_bus_register(parent);
> > >  }
> > >  
> > > +static void pci_bus_uninit(PCIBus *bus)
> > > +{
> > > +pci_host_bus_unregister(BUS(bus)->parent);
> > > +}
> > > +
> > >  bool pci_bus_is_express(PCIBus *bus)
> > >  {
> > >  return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > > @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const 
> > > char *name,
> > >  return bus;
> > >  }
> > >  
> > > +void pci_root_bus_cleanup(PCIBus *bus)
> > > +{
> > > +pci_bus_uninit(bus);
> > > +object_unparent(OBJECT(bus));
> > > +}
> > > +
> > >  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn 
> > > map_irq,
> > >void *irq_opaque, int nirq)
> > >  {
> > > @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn 
> > > set_irq, pci_map_irq_fn map_irq,
> > >  bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> > >  }
> > >  
> > > +void pci_bus_irqs_cleanup(PCIBus *bus)
> > > +{
> > > +bus->set_irq = NULL;
> > > +bus->map_irq = NULL;
> > > +bus->irq_opaque = NULL;
> > > +bus->nirq = 0;
> > > +g_free(bus->irq_count);
> > > +}
> > > +
> > >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > >pci_set_irq_fn set_irq, pci_map_irq_fn 
> > > map_irq,
> > >void *irq_opaque,
> > > @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, 
> > > const char *name,
> > >  return bus;
> > >  }
> > >  
> > > +void pci_unregister_root_bus(PCIBus *bus)
> > > +{
> > > +pci_bus_irqs_cleanup(bus);
> > > +pci_root_bus_cleanup(bus);
> > > +}
> > > +
> > >  int pci_bus_num(PCIBus *s)
> > >  {
> > >  return PCI_BUS_GET_CLASS(s)->bus_num(s);
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index e6514bba23aa..8998e3be3390 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const 
> > > char *name,
> > >   MemoryRegion *address_space_mem,
> > >   MemoryRegion *address_space_io,
> > >   uint8_t devfn_min, const char *typename);
> > > +void pci_root_bus_cleanup(PCIBus *bus);
> > >  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn 
> > > map_irq,
> > >void *irq_opaque, int nirq);
> > > +void pci_bus_irqs_cleanup(PCIBus *bus);
> > >  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > >  /* 0 <= p

Re: [Qemu-devel] [PATCH 15/15] spapr: add hotplug hooks for PHB hotplug

[David Gibson]

On Fri, Dec 21, 2018 at 07:36:12AM +0100, Greg Kurz wrote:
> From: Michael Roth 
> 
> Hotplugging PHBs is a machine-level operation, but PHBs reside on the
> main system bus, so we register spapr machine as the handler for the
> main system bus.
> 
> We re-get the phandle of the interrupt controller systematically for
> simplicity.
> 
> Signed-off-by: Michael Roth 
> Signed-off-by: Greg Kurz 
> ---
>  hw/ppc/spapr.c |  147 
> 
>  hw/ppc/spapr_drc.c |1 
>  hw/ppc/spapr_pci.c |   16 -
>  include/hw/ppc/spapr.h |1 
>  4 files changed, 149 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5c405a5fafca..065c9f19700e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2923,6 +2923,10 @@ static void spapr_machine_init(MachineState *machine)
>  register_savevm_live(NULL, "spapr/htab", -1, 1,
>   &savevm_htab_handlers, spapr);
>  
> +if (smc->dr_phb_enabled) {
> +qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine), 
> NULL);
> +}
> +
>  qemu_register_boot_set(spapr_boot_set, spapr);
>  
>  if (kvm_enabled()) {
> @@ -3716,6 +3720,135 @@ out:
>  error_propagate(errp, local_err);
>  }
>  
> +static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> +   Error **errp)
> +{
> +sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> +const unsigned windows_supported = spapr_phb_windows_supported(sphb);
> +
> +if (sphb->index == (uint32_t)-1) {
> +error_setg(errp, "\"index\" for PAPR PHB is mandatory");
> +return;
> +}
> +
> +/*
> + * This will check that sphb->index doesn't exceed the maximum number of
> + * PHBs for the current machine type.
> + */
> +smc->phb_placement(spapr, sphb->index,
> +   &sphb->buid, &sphb->io_win_addr,
> +   &sphb->mem_win_addr, &sphb->mem64_win_addr,
> +   windows_supported, sphb->dma_liobn, errp);
> +}
> +
> +static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> +   Error **errp)
> +{
> +sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> +sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
> +void *fdt = NULL;
> +int fdt_start_offset;
> +int fdt_size;
> +Error *local_err = NULL;
> +sPAPRDRConnector *drc;
> +int ret;
> +bool hotplugged = spapr_drc_hotplugged(dev);
> +int offset, phandle = 0;
> +gchar *nodename = NULL;
> +
> +if (!smc->dr_phb_enabled) {
> +return;
> +}
> +
> +drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
> +/* hotplug hooks should check it's enabled before getting this far */
> +assert(drc);
> +
> +if (hotplugged) {
> +if (spapr->fdt_blob) {
> +/*
> + * SLOF might have pushed an updated FDT with new phandle values.
> + * Re-get the one of our interrupt controller.
> + */
> +nodename = spapr->irq->get_nodename(spapr);
> +
> +offset = fdt_subnode_offset(spapr->fdt_blob, 0, nodename);
> +if (offset < 0) {
> +error_setg(errp, "Can't find node \"%s\": %s",
> +   nodename, fdt_strerror(offset));
> +goto out;
> +}
> +
> +phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> +if (phandle < 0) {
> +error_setg(errp, "Can't get phandle of node \"%s\": %s",
> +   nodename, fdt_strerror(offset));
> +goto out;
> +}
> +}
> +DEVICE_GET_CLASS(dev)->reset(dev);
> +}
> +
> +/* For cold-plugged at initial boot and fallback for hotplug */
> +if (!phandle) {
> +phandle = PHANDLE_XICP;
> +}
> +
> +fdt = create_device_tree(&fdt_size);
> +ret = spapr_populate_pci_dt(sphb, phandle, fdt, spapr->irq->nr_msis,
> +&fdt_start_offset);
> +if (ret < 0) {
> +error_setg(&local_err, "unable to create FDT for %sPHB",
> +   dev->hotplugged ? "hotplugged " : "");
> +goto out;
> +}
> +
> +if (hotplugged) {
> +/* generally SLOF creates these, for hotplug it's up to QEMU */
> +_FDT(fdt_setprop_string(fdt, fdt_start_offset, "name", "pci"));
> +}
> +
> +spapr_drc_attach(drc, DEVICE(dev), fdt, fdt_start_offset, &local_err);
> +
> +out:
> +g_free(nodename);
> +
> +if (local_err) {
> +error_propagate(errp, local_err);
> +g_free(fdt);
> +return;
> +}
> +
> +if (hotplugged) {
> +spapr_hotplug_req_add_by_index(dr

Re: [Qemu-devel] [PATCH 14/15] spapr: Expose the name of the interrupt controller node

[David Gibson]

On Fri, Dec 21, 2018 at 10:53:13AM +0100, Greg Kurz wrote:
> On Fri, 21 Dec 2018 09:12:24 +0100
> Cédric Le Goater  wrote:
> 
> > On 12/21/18 7:35 AM, Greg Kurz wrote:
> > > This will be needed by PHB hotplug in order to access the phandle 
> > > property.
> > > 
> > > Signed-off-by: Greg Kurz   
> > 
> > I would have used the prefix 'spapr_dt_', but it's minor.
> > 
> 
> I guess there might be a v2. I'll do the change.

Actually, don't.  I'm standardizing on spapr_dt_* for functions which
generate pieces of the device tree.  This helper is related, but
doesn't really fit that pattern.

> 
> > Reviewed-by: Cédric Le Goater 
> > 
> 
> Thanks!
> 
> > Thanks,
> > 
> > C.
> > 
> > > ---
> > >  hw/intc/spapr_xive.c|9 +++--
> > >  hw/intc/xics_spapr.c|9 -
> > >  hw/ppc/spapr_irq.c  |3 +++
> > >  include/hw/ppc/spapr_irq.h  |1 +
> > >  include/hw/ppc/spapr_xive.h |1 +
> > >  include/hw/ppc/xics.h   |1 +
> > >  6 files changed, 21 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> > > index 87424de26c1c..0540aac88d2a 100644
> > > --- a/hw/intc/spapr_xive.c
> > > +++ b/hw/intc/spapr_xive.c
> > > @@ -1410,6 +1410,12 @@ void spapr_xive_hcall_init(sPAPRMachineState 
> > > *spapr)
> > >  spapr_register_hypercall(H_INT_RESET, h_int_reset);
> > >  }
> > >  
> > > +gchar *spapr_xive_get_nodename(sPAPRMachineState *spapr)
> > > +{
> > > +return g_strdup_printf("interrupt-controller@%" PRIx64,
> > > +spapr->xive->tm_base + XIVE_TM_USER_PAGE * (1 << 
> > > TM_SHIFT));
> > > +}
> > > +
> > >  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void 
> > > *fdt,
> > > uint32_t phandle)
> > >  {
> > > @@ -1444,8 +1450,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, 
> > > uint32_t nr_servers, void *fdt,
> > > XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
> > >  timas[3] = cpu_to_be64(1ull << TM_SHIFT);
> > >  
> > > -nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> > > -   xive->tm_base + XIVE_TM_USER_PAGE * (1 << 
> > > TM_SHIFT));
> > > +nodename = spapr_xive_get_nodename(spapr);
> > >  _FDT(node = fdt_add_subnode(fdt, 0, nodename));
> > >  g_free(nodename);
> > >  
> > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> > > index f67d3c80bf3a..75d40daf518d 100644
> > > --- a/hw/intc/xics_spapr.c
> > > +++ b/hw/intc/xics_spapr.c
> > > @@ -244,6 +244,13 @@ void xics_spapr_init(sPAPRMachineState *spapr)
> > >  spapr_register_hypercall(H_IPOLL, h_ipoll);
> > >  }
> > >  
> > > +#define NODENAME "interrupt-controller"
> > > +
> > > +gchar *spapr_xics_get_nodename(sPAPRMachineState *spapr)
> > > +{
> > > +return g_strdup(NODENAME);
> > > +}
> > > +
> > >  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void 
> > > *fdt,
> > > uint32_t phandle)
> > >  {
> > > @@ -252,7 +259,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t 
> > > nr_servers, void *fdt,
> > >  };
> > >  int node;
> > >  
> > > -_FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> > > +_FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
> > >  
> > >  _FDT(fdt_setprop_string(fdt, node, "device_type",
> > >  "PowerPC-External-Interrupt-Presentation"));
> > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > > index 0999a2b2d69c..703c3a3c20d5 100644
> > > --- a/hw/ppc/spapr_irq.c
> > > +++ b/hw/ppc/spapr_irq.c
> > > @@ -223,6 +223,7 @@ sPAPRIrq spapr_irq_xics = {
> > >  .qirq= spapr_qirq_xics,
> > >  .print_info  = spapr_irq_print_info_xics,
> > >  .dt_populate = spapr_dt_xics,
> > > +.get_nodename = spapr_xics_get_nodename,
> > >  .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> > >  .post_load   = spapr_irq_post_load_xics,
> > >  };
> > > @@ -349,6 +350,7 @@ sPAPRIrq spapr_irq_xive = {
> > >  .qirq= spapr_qirq_xive,
> > >  .print_info  = spapr_irq_print_info_xive,
> > >  .dt_populate = spapr_dt_xive,
> > > +.get_nodename = spapr_xive_get_nodename,
> > >  .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
> > >  .post_load   = spapr_irq_post_load_xive,
> > >  .reset   = spapr_irq_reset_xive,
> > > @@ -462,6 +464,7 @@ sPAPRIrq spapr_irq_xics_legacy = {
> > >  .qirq= spapr_qirq_xics,
> > >  .print_info  = spapr_irq_print_info_xics,
> > >  .dt_populate = spapr_dt_xics,
> > > +.get_nodename = spapr_xics_get_nodename,
> > >  .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> > >  .post_load   = spapr_irq_post_load_xics,
> > >  };
> > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> > > index b34d5a00381b..59a1cf8bbc1d 100644
> > > --- a/include/hw/ppc/spapr_irq.h
> > > +++ b/include/hw/ppc/spapr_irq.h
> > > @@ -42,6 +42,7 @@ typedef struct sPAPRIrq {
> > >  void

Re: [Qemu-devel] [PATCH 03/15] pci: allow cleanup/unregistration of PCI root buses

[David Gibson]

On Fri, Dec 21, 2018 at 11:19:18AM -0500, Michael S. Tsirkin wrote:
> On Fri, Dec 21, 2018 at 01:35:30AM +0100, Greg Kurz wrote:
> > From: Michael Roth 
> > 
> > This adds cleanup counterparts to pci_register_root_bus(),
> > pci_root_bus_new(), and pci_bus_irqs().
> > 
> > These cleanup routines are needed in the case of hotpluggable
> > PCIHostBridge implementations. Currently we can rely on the
> > object_unparent()'ing of the PCIHostState recursively unparenting
> > and cleaning up it's child buses, but we need explicit calls
> > to also:
> > 
> >   1) remove the PCIHostState from pci_host_bridges global list.
> >  otherwise, we risk accessing freed memory when we access
> >  the list later
> >   2) clean up memory allocated in pci_bus_irqs()
> > 
> > Both are handled outside the context of any particular bus or
> > host bridge's init/realize functions, making it difficult to
> > avoid the need for explicit cleanup functions without remodeling
> > how PCIHostBridges are created. So keep it simple and just add
> > them for now.
> > 
> > Cc: Michael S. Tsirkin 
> > Cc: Paolo Bonzini 
> > Signed-off-by: Michael Roth 
> > Reviewed-by: David Gibson 
> > Signed-off-by: Greg Kurz 
> 
> Reviewed-by: Michael S. Tsirkin 

I've applied this tentatively to ppc-for-4.0.  Let me know, Michael,
if you'd prefer to take it through your tree.

> 
> > ---
> >  hw/pci/pci.c |   33 +
> >  include/hw/pci/pci.h |3 +++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index efb5ce196ffb..16354f91206c 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -333,6 +333,13 @@ static void pci_host_bus_register(DeviceState *host)
> >  QLIST_INSERT_HEAD(&pci_host_bridges, host_bridge, next);
> >  }
> >  
> > +static void pci_host_bus_unregister(DeviceState *host)
> > +{
> > +PCIHostState *host_bridge = PCI_HOST_BRIDGE(host);
> > +
> > +QLIST_REMOVE(host_bridge, next);
> > +}
> > +
> >  PCIBus *pci_device_root_bus(const PCIDevice *d)
> >  {
> >  PCIBus *bus = pci_get_bus(d);
> > @@ -379,6 +386,11 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState 
> > *parent,
> >  pci_host_bus_register(parent);
> >  }
> >  
> > +static void pci_bus_uninit(PCIBus *bus)
> > +{
> > +pci_host_bus_unregister(BUS(bus)->parent);
> > +}
> > +
> >  bool pci_bus_is_express(PCIBus *bus)
> >  {
> >  return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > @@ -413,6 +425,12 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const 
> > char *name,
> >  return bus;
> >  }
> >  
> > +void pci_root_bus_cleanup(PCIBus *bus)
> > +{
> > +pci_bus_uninit(bus);
> > +object_unparent(OBJECT(bus));
> > +}
> > +
> >  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn 
> > map_irq,
> >void *irq_opaque, int nirq)
> >  {
> > @@ -423,6 +441,15 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, 
> > pci_map_irq_fn map_irq,
> >  bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> >  }
> >  
> > +void pci_bus_irqs_cleanup(PCIBus *bus)
> > +{
> > +bus->set_irq = NULL;
> > +bus->map_irq = NULL;
> > +bus->irq_opaque = NULL;
> > +bus->nirq = 0;
> > +g_free(bus->irq_count);
> > +}
> > +
> >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> >pci_set_irq_fn set_irq, pci_map_irq_fn 
> > map_irq,
> >void *irq_opaque,
> > @@ -439,6 +466,12 @@ PCIBus *pci_register_root_bus(DeviceState *parent, 
> > const char *name,
> >  return bus;
> >  }
> >  
> > +void pci_unregister_root_bus(PCIBus *bus)
> > +{
> > +pci_bus_irqs_cleanup(bus);
> > +pci_root_bus_cleanup(bus);
> > +}
> > +
> >  int pci_bus_num(PCIBus *s)
> >  {
> >  return PCI_BUS_GET_CLASS(s)->bus_num(s);
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index e6514bba23aa..8998e3be3390 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -405,8 +405,10 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const 
> > char *name,
> >   MemoryRegion *address_space_mem,
> >   MemoryRegion *address_space_io,
> >   uint8_t devfn_min, const char *typename);
> > +void pci_root_bus_cleanup(PCIBus *bus);
> >  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn 
> > map_irq,
> >void *irq_opaque, int nirq);
> > +void pci_bus_irqs_cleanup(PCIBus *bus);
> >  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> >  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > @@ -417,6 +419,7 @@ PCIBus *pci_register_root_bus(DeviceState *parent, 
> > const char *name,
> >MemoryRegion *address_space_io,
> >uint8_t devfn_min, int nirq,
> >

Re: [Qemu-devel] [PATCH 2/8] sam460ex: Clean up SPD EEPROM creation

[David Gibson]

On Wed, Jan 02, 2019 at 01:49:44PM +0100, BALATON Zoltan wrote:
> On Wed, 2 Jan 2019, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
> > > +/* IIC controllers and devices */
> > >  dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> > > -i2c[0] = PPC4xx_I2C(dev);
> > > -object_property_set_bool(OBJECT(dev), true, "realized", NULL);
> > > -smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, 
> > > smbus_eeprom_size);
> > > -g_free(smbus_eeprom_buf);
> > > -i2c_create_slave(i2c[0]->bus, "m41t80", 0x68);
> > > +i2c = PPC4xx_I2C(dev)->bus;
> > > +/* SPD EEPROM on RAM module */
> > > +spd_data = spd_data_generate(DDR2, ram_sizes[0]);
> > > +if (spd_data) {
> > > +spd_data[20] = 4; /* SO-DIMM module */
> > > +smbus_eeprom_init_one(i2c, 0x50, spd_data);
> > 
> > Any specific reason this case is handled outside spd_data_generate()?
> 
> As explained in previous reply I tried to keep number of options to the
> function to minimum and not handle every board specific case in that
> function. This board has SO-DIMM instead of regular DIMM socket but most of
> the SPD data is the same so it's easy enough to patch it here to match what
> real hardware may have. Otherwise spd_data_generate should have a lot of
> options for all different possibilites different boards might have.

Ok, fair enough.

> 
> (Also this is an example that unexpected configs might still work:
> 
> $ qemu-system-ppc -M sam460ex -m 64 -serial stdio
> qemu-system-ppc: warning: Memory size is too small for SDRAM type, adjusting 
> type
> [...]
> DRAM:  64 MiB (ECC not enabled, 460 MHz, CL0)
> 
> This SoC has a DRAM controller which accepts both DDR and DDR2 and the
> firmware does not care either so it works even if this cannot happen on real
> hardware. The spd_data[20] is different for DDR2 and DDR/SDR but in the
> latter it's ~WE latency which does not matter either so it can be set
> unconditionally. If this was not working the board code could check
> spd_data[2] != DDR2 and then abort itself.)
> 
> Regards,
> BALATON Zoltan
> 

-- 
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 06/15] spapr: enable PHB hotplug for default pseries machine type

[David Gibson]

On Fri, Dec 21, 2018 at 01:36:32AM +0100, Greg Kurz wrote:
> From: Michael Roth 
> 
> The 'dr_phb_enabled' field of that class can be set as part of
> machine-specific init code. It will be used to conditionally
> enable creation of DRC objects and device-tree description to
> facilitate hotplug of PHBs.
> 
> Since we can't migrate this state to older machine types,
> default the option to true and disable it for older machine
> types.
> 
> Signed-off-by: Michael Roth 
> Signed-off-by: Greg Kurz 

Although it makes sense to have this function first while
developing, it's usually best to have it last when you push, so you
don't have a potential bisection breakage where the support is
advertised but not fully working.

> ---
>  hw/ppc/spapr.c |2 ++
>  include/hw/ppc/spapr.h |1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1f17b5d01f4f..621006eaa862 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4011,6 +4011,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>  smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
>  spapr_caps_add_properties(smc, &error_abort);
>  smc->irq = &spapr_irq_xics;
> +smc->dr_phb_enabled = true;
>  }
>  
>  static const TypeInfo spapr_machine_info = {
> @@ -4079,6 +4080,7 @@ static void 
> spapr_machine_3_1_class_options(MachineClass *mc)
>  SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
>  smc->update_dt_enabled = false;
> +smc->dr_phb_enabled = false;
>  }
>  
>  DEFINE_SPAPR_MACHINE(3_1, "3.1", false);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 36033b89d31a..e96deefa30de 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -104,6 +104,7 @@ struct sPAPRMachineClass {
>  /*< public >*/
>  bool dr_lmb_enabled;   /* enable dynamic-reconfig/hotplug of LMBs */
>  bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
> +bool dr_phb_enabled;   /* enable dynamic-reconfig/hotplug of PHBs */
>  bool use_ohci_by_default;  /* use USB-OHCI instead of XHCI */
>  bool pre_2_10_has_unused_icps;
>  bool legacy_irq_allocation;
> 

-- 
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 04/15] spapr_pci: add proper rollback on PHB realize error path

[David Gibson]

On Fri, Dec 21, 2018 at 01:35:52AM +0100, Greg Kurz wrote:
> The current realize code assumes the PHB is coldplugged, ie, QEMU will
> terminate if an error is detected, and does not bother to free anything
> it has already allocated.
> 
> In order to support PHB hotplug, let's first ensure spapr_phb_realize()
> doesn't leak anything in case of error.
> 
> Signed-off-by: Greg Kurz 

This looks ok as far as it goes.  However, it looks like there will be
a lot of fragments duplicated between the rollback paths and
unrealize() when it's added in the next patch.

A common pattern to avoid that is to make unrealize() safe to call on
a partially realized object, then call that from the realize() failure
paths.  Is it possible to do that here?  I imagine that would involve
folding this patch with the next as well.

> ---
>  hw/ppc/spapr_pci.c |   40 +++-
>  1 file changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index e59adbe706bb..46d7062dd143 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1570,6 +1570,7 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  sPAPRTCETable *tcet;
>  const unsigned windows_supported =
>  sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
> +Object *drcs[PCI_SLOT_MAX * 8];
>  
>  if (!spapr) {
>  error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries 
> machine");
> @@ -1733,7 +1734,10 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  spapr_irq_claim(spapr, irq, true, &local_err);
>  if (local_err) {
>  error_propagate_prepend(errp, local_err, "can't allocate LSIs: 
> ");
> -return;
> +while (--i >= 0) {
> +spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
> +}
> +goto fail_del_msiwindow;
>  }
>  
>  sphb->lsi_table[i].irq = irq;
> @@ -1741,9 +1745,10 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  
>  /* allocate connectors for child PCI devices */
>  if (sphb->dr_enabled) {
> -for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> -spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> -   (sphb->index << 16) | i);
> +for (i = 0; i < ARRAY_SIZE(drcs); i++) {
> +drcs[i] =
> +OBJECT(spapr_dr_connector_new(OBJECT(phb), 
> TYPE_SPAPR_DRC_PCI,
> +  (sphb->index << 16) | i));
>  }
>  }
>  
> @@ -1753,13 +1758,38 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  if (!tcet) {
>  error_setg(errp, "Creating window#%d failed for %s",
> i, sphb->dtbusname);
> -return;
> +while (--i >= 0) {
> +tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
> +assert(tcet);
> +memory_region_del_subregion(&sphb->iommu_root,
> +spapr_tce_get_iommu(tcet));
> +object_unparent(OBJECT(tcet));
> +}
> +goto fail_free_drcs;
>  }
>  memory_region_add_subregion(&sphb->iommu_root, 0,
>  spapr_tce_get_iommu(tcet));
>  }
>  
>  sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, 
> g_free);
> +return;
> +
> +fail_free_drcs:
> +if (sphb->dr_enabled) {
> +for (i = 0; i < ARRAY_SIZE(drcs); i++) {
> +object_unparent(drcs[i]);
> +}
> +}
> +fail_del_msiwindow:
> +memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
> +address_space_destroy(&sphb->iommu_as);
> +qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
> +pci_unregister_root_bus(phb->bus);
> +memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
> +if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
> +memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
> +}
> +memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
>  }
>  
>  static int spapr_phb_children_reset(Object *child, void *opaque)
> 

-- 
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 07/15] spapr_pci: Define SPAPR_MAX_PHBS in hw/pci-host/spapr.h

[David Gibson]

On Fri, Dec 21, 2018 at 01:36:53AM +0100, Greg Kurz wrote:
> PHB hotplug will bring more users for it. Let's define it along with
> the PHB defines from which it is derived for simplicity.
> 
> While here fix a misleading comment about manual placement, which was
> abandoned with 30b3bc5aa9f4.
> 
> Signed-off-by: Greg Kurz 

Applied to ppc-for-4.0, thanks.

> ---
>  hw/ppc/spapr.c  |2 --
>  include/hw/pci-host/spapr.h |6 --
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 621006eaa862..fe3f9829ae6c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3838,8 +3838,6 @@ static void spapr_phb_placement(sPAPRMachineState 
> *spapr, uint32_t index,
>   * 1TiB 64-bit MMIO windows for each PHB.
>   */
>  const uint64_t base_buid = 0x8002000ULL;
> -#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> -SPAPR_PCI_MEM64_WIN_SIZE - 1)
>  int i;
>  
>  /* Sanity check natural alignments */
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 9d2ec1a410e8..83d5075a6ef3 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -94,11 +94,13 @@ struct sPAPRPHBState {
>  ((1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET)
>  #define SPAPR_PCI_MEM64_WIN_SIZE 0x100ULL /* 1 TiB */
>  
> -/* Without manual configuration, all PCI outbound windows will be
> - * within this range */
> +/* All PCI outbound windows will be within this range */
>  #define SPAPR_PCI_BASE   (1ULL << 45) /* 32 TiB */
>  #define SPAPR_PCI_LIMIT  (1ULL << 46) /* 64 TiB */
>  
> +#define SPAPR_MAX_PHBS ((SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / \
> +SPAPR_PCI_MEM64_WIN_SIZE - 1)
> +
>  #define SPAPR_PCI_2_7_MMIO_WIN_SIZE  0xf8000
>  #define SPAPR_PCI_IO_WIN_SIZE0x1
>  
> 

-- 
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 01/15] ppc/spapr: Receive and store device tree blob from SLOF

[David Gibson]

On Fri, Dec 21, 2018 at 01:34:48AM +0100, Greg Kurz wrote:
> From: Alexey Kardashevskiy 
> 
> SLOF receives a device tree and updates it with various properties
> before switching to the guest kernel and QEMU is not aware of any changes
> made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
> sense to pass the SLOF final device tree to QEMU to let it implement
> RTAS related tasks better, such as PCI host bus adapter hotplug.
> 
> Specifially, now QEMU can find out the actual XICS phandle (for PHB
> hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
> assisted NMI - FWNMI).
> 
> This stores the initial DT blob in the sPAPR machine and replaces it
> in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
> 
> This adds an @update_dt_enabled machine property to allow backward
> migration.
> 
> SLOF already has a hypercall since
> https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
> 
> This makes use of the new fdt_check_full() helper. In order to allow
> the configure script to pick the correct DTC version, this adjusts
> the DTC presense test.
> 
> Signed-off-by: Alexey Kardashevskiy 
> Reviewed-by: Greg Kurz 
> Signed-off-by: David Gibson 
> Signed-off-by: Greg Kurz 

I've applied this again.  Last time it seemed to create a mysterious
crash in the arm target, but I can't reproduce it any more, so fingers
crossed.

> ---
>  configure  |2 +-
>  hw/ppc/spapr.c |   43 ++-
>  hw/ppc/spapr_hcall.c   |   42 ++
>  hw/ppc/trace-events|3 +++
>  include/hw/ppc/spapr.h |7 ++-
>  5 files changed, 94 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 224d3071ac61..baeeabc29f56 100755
> --- a/configure
> +++ b/configure
> @@ -3916,7 +3916,7 @@ if test "$fdt" != "no" ; then
>cat > $TMPC << EOF
>  #include 
>  #include 
> -int main(void) { fdt_first_subnode(0, 0); return 0; }
> +int main(void) { fdt_check_full(NULL, 0); return 0; }
>  EOF
>if compile_prog "" "$fdt_libs" ; then
>  # system DTC is good - use it
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 17ad84396b31..8ea680fcde1e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1668,7 +1668,10 @@ static void spapr_machine_reset(void)
>  /* Load the fdt */
>  qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
>  cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> -g_free(fdt);
> +g_free(spapr->fdt_blob);
> +spapr->fdt_size = fdt_totalsize(fdt);
> +spapr->fdt_initial_size = spapr->fdt_size;
> +spapr->fdt_blob = fdt;
>  
>  /* Set up the entry state */
>  spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
> @@ -1919,6 +1922,39 @@ static const VMStateDescription vmstate_spapr_irq_map 
> = {
>  },
>  };
>  
> +static bool spapr_dtb_needed(void *opaque)
> +{
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
> +
> +return smc->update_dt_enabled;
> +}
> +
> +static int spapr_dtb_pre_load(void *opaque)
> +{
> +sPAPRMachineState *spapr = (sPAPRMachineState *)opaque;
> +
> +g_free(spapr->fdt_blob);
> +spapr->fdt_blob = NULL;
> +spapr->fdt_size = 0;
> +
> +return 0;
> +}
> +
> +static const VMStateDescription vmstate_spapr_dtb = {
> +.name = "spapr_dtb",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = spapr_dtb_needed,
> +.pre_load = spapr_dtb_pre_load,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
> +VMSTATE_UINT32(fdt_size, sPAPRMachineState),
> +VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
> + fdt_size),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
>  static const VMStateDescription vmstate_spapr = {
>  .name = "spapr",
>  .version_id = 3,
> @@ -1948,6 +1984,7 @@ static const VMStateDescription vmstate_spapr = {
>  &vmstate_spapr_cap_ibs,
>  &vmstate_spapr_irq_map,
>  &vmstate_spapr_cap_nested_kvm_hv,
> +&vmstate_spapr_dtb,
>  NULL
>  }
>  };
> @@ -3929,6 +3966,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>  hc->unplug = spapr_machine_device_unplug;
>  
>  smc->dr_lmb_enabled = true;
> +smc->update_dt_enabled = true;
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
>  mc->has_hotpluggable_cpus = true;
>  smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
> @@ -4024,9 +4062,12 @@ DEFINE_SPAPR_MACHINE(4_0, "4.0", true);
>  
>  static void spapr_machine_3_1_class_options(MachineClass *mc)
>  {
> +sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
>  spapr_machine_4_0_class_options(mc);
>  SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_1);
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> +smc->update_dt_enabled = false;
>  }
>  
>  DEFINE_SPAP

Re: [Qemu-devel] [PATCH 02/15] spapr: move spapr_create_phb() to core machine code

[David Gibson]

On Fri, Dec 21, 2018 at 01:35:09AM +0100, Greg Kurz wrote:
> This function is only used when creating the default PHB. Let's rename
> it and move it to the core machine code for clarity.
> 
> Signed-off-by: Greg Kurz 
> Reviewed-by: Alexey Kardashevskiy 
> Reviewed-by: David Gibson 

Applied to ppc-for-4.0, thanks.

> ---
>  hw/ppc/spapr.c  |   13 -
>  hw/ppc/spapr_pci.c  |   11 ---
>  include/hw/pci-host/spapr.h |2 --
>  3 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8ea680fcde1e..1f17b5d01f4f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2551,6 +2551,17 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
>  }
>  }
>  
> +static PCIHostState *spapr_create_default_phb(void)
> +{
> +DeviceState *dev;
> +
> +dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
> +qdev_prop_set_uint32(dev, "index", 0);
> +qdev_init_nofail(dev);
> +
> +return PCI_HOST_BRIDGE(dev);
> +}
> +
>  /* pSeries LPAR / sPAPR hardware init */
>  static void spapr_machine_init(MachineState *machine)
>  {
> @@ -2782,7 +2793,7 @@ static void spapr_machine_init(MachineState *machine)
>  /* Set up PCI */
>  spapr_pci_rtas_init();
>  
> -phb = spapr_create_phb(spapr, 0);
> +phb = spapr_create_default_phb();
>  
>  for (i = 0; i < nb_nics; i++) {
>  NICInfo *nd = &nd_table[i];
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2374d55fc112..e59adbe706bb 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1979,17 +1979,6 @@ static const TypeInfo spapr_phb_info = {
>  }
>  };
>  
> -PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index)
> -{
> -DeviceState *dev;
> -
> -dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
> -qdev_prop_set_uint32(dev, "index", index);
> -qdev_init_nofail(dev);
> -
> -return PCI_HOST_BRIDGE(dev);
> -}
> -
>  typedef struct sPAPRFDT {
>  void *fdt;
>  int node_off;
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 7c66c3872f96..a65cfef16945 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -111,8 +111,6 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct 
> sPAPRPHBState *phb, int pin)
>  return spapr_qirq(spapr, phb->lsi_table[pin].irq);
>  }
>  
> -PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index);
> -
>  int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void 
> *fdt,
>uint32_t nr_msis);
>  
> 

-- 
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 1/8] smbus: Add a helper to generate SPD EEPROM data

[David Gibson]

On Wed, Jan 02, 2019 at 01:36:04PM +0100, BALATON Zoltan wrote:
> On Wed, 2 Jan 2019, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
> > > There are several boards with SPD EEPROMs that are now using
> > > duplicated or slightly different hard coded data. Add a helper to
> > > generate SPD data for a memory module of given type and size that
> > > could be used by these boards (either as is or with further changes if
> > > needed) which should help cleaning this up and avoid further duplication.
> > > 
> > > Signed-off-by: BALATON Zoltan 
> > > ---
> > >  hw/i2c/smbus_eeprom.c  | 128 
> > > +
> > >  include/hw/i2c/smbus.h |   3 ++
> > >  2 files changed, 131 insertions(+)
> > > 
> > > diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
> > > index f18aa3de35..a1f51eb921 100644
> > > --- a/hw/i2c/smbus_eeprom.c
> > > +++ b/hw/i2c/smbus_eeprom.c
> > > @@ -23,6 +23,8 @@
> > >   */
> > > 
> > >  #include "qemu/osdep.h"
> > > +#include "qemu/error-report.h"
> > > +#include "qemu/units.h"
> > >  #include "hw/hw.h"
> > >  #include "hw/i2c/i2c.h"
> > >  #include "hw/i2c/smbus.h"
> > > @@ -162,3 +164,129 @@ void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom,
> > >  smbus_eeprom_init_one(smbus, 0x50 + i, eeprom_buf + (i * 256));
> > >  }
> > >  }
> > > +
> > > +/* Generate SDRAM SPD EEPROM data describing a module of type and size */
> > > +uint8_t *spd_data_generate(enum sdram_type type, ram_addr_t ram_size)
> > > +{
> > > +uint8_t *spd;
> > > +uint8_t nbanks;
> > > +uint16_t density;
> > > +uint32_t size;
> > > +int min_log2, max_log2, sz_log2;
> > > +int i;
> > > +
> > > +switch (type) {
> > > +case SDR:
> > > +min_log2 = 2;
> > > +max_log2 = 9;
> > > +break;
> > > +case DDR:
> > > +min_log2 = 5;
> > > +max_log2 = 12;
> > > +break;
> > > +case DDR2:
> > > +min_log2 = 7;
> > > +max_log2 = 14;
> > > +break;
> > > +default:
> > > +error_report("Unknown SDRAM type");
> > > +abort();
> > 
> > The error handling might work a little cleaner if you give this
> > function an Error ** parameter, then just pass in &error_abort from
> > the callers.
> 
> Good idea but I'm not sure it's needed. This is the only fatal error here
> (apart from g_malloc0 failing which should also abort) and in practice this
> could only happen if a caller specifies wrong type which is quite unlikely
> given that it's an enum so value outside of that would fail to compile with
> a warning (promoted to error by default). So this default case is only
> really here to please the compiler.

Ok.  If the only reason you'd hit the default case is a bug in the
caller, then just use a g_assert_not_reached() rather than
error_report().  As a rule of thumb use asserts or aborts for things
that have to be bugs in the code, error_report() for things that
indicate a user or configuration error.

> > > +}
> > > +size = ram_size >> 20; /* work in terms of megabytes */
> > > +if (size < 4) {
> > > +error_report("SDRAM size is too small");
> > > +return NULL;
> > > +}
> > > +sz_log2 = 31 - clz32(size);
> > > +size = 1U << sz_log2;
> > > +if (ram_size > size * MiB) {
> > > +warn_report("SDRAM size 0x"RAM_ADDR_FMT" is not a power of 2, "
> > > +"truncating to %u MB", ram_size, size);
> > > +}
> > > +if (sz_log2 < min_log2) {
> > > +warn_report("Memory size is too small for SDRAM type, adjusting 
> > > type");
> > > +if (size >= 32) {
> > > +type = DDR;
> > > +min_log2 = 5;
> > > +max_log2 = 12;
> > > +} else {
> > > +type = SDR;
> > > +min_log2 = 2;
> > > +max_log2 = 9;
> > > +}
> > 
> > What do these various fall back cases represent?  Are they bugs in the
> > callers, or a user configuration error?
> 
> The memory size is given by the user so it can be a config error (like when
> board has DDR2 but user sets memory size to 64MB).
> 
> > If the first, we should just assert or abort.  If the second I think
> > we should still die with a fatal error - allowing broken
> > configurations to work with just a warning is kind of begging to
> > maintain nasty compatiliby cruft in the future.
> 
> I'd leave that to the caller to decide and not abort in this
> function.

Right.  The obvious way to do that is to have this function take an
Error *, and use error_setg() where you have explicit warns now.  The
caller can pass &error_fatal if it just wants to treat that as a user
error, and do something else if it wants to implement a fallback.

> It
> will warn user that config is unexpected for the board but does not prevent
> it and try to give something that might still work (e.g. DDR instead of
> DDR2). Then the caller can check the returned data and abort i

Re: [Qemu-devel] About live migration rollback

[Gonglei (Arei)]

Hi,

> 
> * Gonglei (Arei) (arei.gong...@huawei.com) wrote:
> > Hi Dave,
> >
> > We discussed some live migration fallback scenarios in this year's KVM 
> > forum,
> > and now I can provide another scenario, perhaps the upstream should
> consider rolling
> > back for this situation.
> >
> > Environments information:
> >
> > host A: cpu E5620(model WestmereEP without flag xsave)
> > host B: cpu E5-2643(model SandyBridgeEP with flag xsave)
> >
> > The reproduce steps is :
> > 1. Start a windows 2008 vm with -cpu host(which means host-passthrough).
> 
> Well we don't guarantee migration across -cpu host - does this problem
> go away if both qemu's are started with matching CPU flags
> (corresponding to the Westmere) ?
> 
Sorry, we didn't test other cpu model scenarios since we should assure
that the live migration support from lower generation CPUs to higher
generation CPUs. :(


> > 2. Migrate the vm to host B when cr4.OSXSAVE=0.
> > 3. Vm runs on host B for a while so that cr4.OSXSAVE changes to 1.
> > 4. Then migrate the vm to host A successfully, but vm was paused, and qemu
> printed log as followed:
> >
> > KVM: entry failed, hardware error 0x8021
> >
> > If you're running a guest on an Intel machine without unrestricted mode
> > support, the failure can be most likely due to the guest entering an invalid
> > state for Intel VT. For example, the guest maybe running in big real mode
> > which is not supported on less recent Intel processors.
> >
> > EAX=019b3bb0 EBX=01a3ae80 ECX=01a61ce8 EDX=
> > ESI=01a62000 EDI= EBP= ESP=01718b20
> > EIP=0185d982 EFL=0286 [--S--P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> > ES =   9300
> > CS =f000   9b00
> > SS =   9300
> > DS =   9300
> > FS =   9300
> > GS =   9300
> > LDT=   8200
> > TR =   8b00
> > GDT=  
> > IDT=  
> > CR0=6010 CR2= CR3= CR4=
> > DR0= DR1= DR2=
> DR3=
> > DR6=0ff0 DR7=0400
> > EFER=
> > Code=00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <00> 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00
> >
> > Problem happened when kvm_put_sregs returns err -22(called by
> kvm_arch_put_registers(qemu)).
> >
> > Because kvm_arch_vcpu_ioctl_set_sregs(kvm module) checked that
> > guest_cpuid_has no X86_FEATURE_XSAVE but cr4.OSXSAVE=1.
> > We should cancel migration if kvm_arch_put_registers returns error.
> 
> Do you have a backtrace of when the kvm_arch_put_registers is called
> when it fails?

The main backtrace is below:

 qemu_loadvm_state
 cpu_synchronize_all_post_init--> w/o return value
 cpu_synchronize_post_init   --> w/o return value
 kvm_cpu_synchronize_post_init  --> w/o return value
run_on_cpu  ---> w/o return value
   do_kvm_cpu_synchronize_post_init  --> w/o 
return value
  kvm_arch_put_registers  --> w/ return value

Root cause is some functions don't have return values, the migration thread
can't detect those failures. Paolo?

> If it's called during the loading of the device state then we should be
> able to detect it and fail the migration; however if it's only failing
> after the CPU is restarted after the migration then it's a bit too late.
> 
Actually the CPUs haven't started in this scenario.

Thanks,
-Gonglei

Re: [Qemu-devel] [PATCH] MAINTAINERS: add qemu_vga.ndrv file entry for Mac machines

[David Gibson]

On Wed, Jan 02, 2019 at 10:22:43AM +, Mark Cave-Ayland wrote:
> The VGA driver built from Ben's QemuMacDrivers repository is used exclusively
> by the Mac machines.
> 
> Signed-off-by: Mark Cave-Ayland 

Applied, thanks.

> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 344cb2d7fd..3da3d103b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -965,6 +965,7 @@ F: include/hw/misc/macio/
>  F: include/hw/misc/mos6522.h
>  F: include/hw/ppc/mac_dbdma.h
>  F: include/hw/pci-host/uninorth.h
> +F: pc-bios/qemu_vga.ndrv
>  
>  Old World
>  M: Mark Cave-Ayland 
> @@ -976,6 +977,7 @@ F: hw/pci-host/grackle.c
>  F: hw/misc/macio/
>  F: hw/intc/heathrow_pic.c
>  F: include/hw/intc/heathrow_pic.h
> +F: pc-bios/qemu_vga.ndrv
>  
>  PReP
>  M: Hervé Poussineau 

-- 
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 v5 0/9] target/ppc: prepare for conversion to TCG vector operations

[David Gibson]

On Wed, Jan 02, 2019 at 09:14:14AM +, Mark Cave-Ayland wrote:
> This patchset is an attempt at trying to improve the VMX (Altivec) instruction
> performance by laying the groundwork for use of the new TCG vector operations.
> 
> Patches 1 and 2 fix a sign-extension error discovered in EXTRACT_SHELPER and 
> an
> associated typo in the SIMM5 macro which were discovered whilst testing 
> Richard's
> follow-on TCG vector improvements patchset.
> 
> In order to use TCG vector operations, the registers must be accessible from 
> cpu_env
> whilst currently they are accessed via arrays of static TCG globals. Patches 
> 3-5
> are therefore mechanical patches which introduce access helpers for FPR, AVR 
> and VSR
> registers using the supplied TCGv_i64 parameter.
> 
> Once this is done, patch 6 enables us to remove the static TCG global arrays 
> and updates
> the access helpers to read/write to the relevant fields in cpu_env directly.
> 
> Patches 7 and 8 perform the legwork required to enable VSX instructions to be 
> converted
> to use TCG vector operations in future by rearranging the FP, VMX and VSX 
> registers into
> a single aligned VSR register array (the scope of this patchset is VMX only).
> 
> Patch 9 removes the AVR* macros and replaces them with the corresponding Vsr* 
> macros
> since they are equivalent.
> 
> Finally thanks to Richard for taking the time to answer some of my (mostly 
> beginner)
> questions related to TCG.
> 
> Signed-off-by: Mark Cave-Ayland 

Applied to ppc-for-4.0, thanks.

> 
> v5:
> - Fix up KVM-enabled builds on PPC host due to missing conversion of 
> target/ppc/kvm.c
> 
> v4:
> - Rebase onto master
> - Add extra R-B tags from Richard
> - Leave HI_IDX/LO_IDX in int_helper.c in patch 9 (similarly named macros are 
> also
>   used in other files so let's ensure there is no confusion)
> - Add cpu_fpr_ptr(), cpu_vsrl_ptr() and cpu_avr_ptr() as suggested by Richard 
> in
>   patch 8
> 
> v3:
> - Rebase onto master, drop RFC prefix, alter subject line
> - Add A-B tags from David
> - Add SIMM5/EXTRACT_HELPER macro fix patches to the start of the series
> - Drop patch 4 from previous patchset (delay AVR register writeback) as it 
> should
>   not be required.
> - Remove extra get_fpr() accidentally added to GEN_FLOAT macros in patch 3
> - Fix temporary leak when VMX/VSX not enabled in patches 4 and 5
> - Add patch to remove AVR* macros, replacing them with Vsr* macros
> - Drop patches converting logical, add and sub instructions to TCG vector ops 
> (let
>   Richard incorporate this into his TCG vector improvements patchset)
> 
> v2:
> - Rebase onto master
> - Add comment explaining rationale for FPR helpers in description for patch 1
> - Add R-B tags from Richard
> - Add patch 3 to delay AVR register writeback as spotted by Richard
> - Add patches 6 and 7 to merge FPR, VMX and VSX registers into the vsr array
>   to facilitate conversion of VSX instructions to vector operations later
> - Fix accidental bug whereby the conversion of get_vsr()/set_vsr() to access
>   data from cpu_env was incorrectly squashed into patch 3
> - Move set_fpr() further down in gen_fsqrts() and gen_frsqrtes() in patch 1
> 
> Mark Cave-Ayland (9):
>   target/ppc: fix typo in SIMM5 extraction helper
>   target/ppc: switch EXTRACT_HELPER macros over to use
> sextract32/extract32
>   target/ppc: introduce get_fpr() and set_fpr() helpers for FP register
> access
>   target/ppc: introduce get_avr64() and set_avr64() helpers for VMX
> register access
>   target/ppc: introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}()
> helpers for VSR register access
>   target/ppc: switch FPR, VMX and VSX helpers to access data directly
> from cpu_env
>   target/ppc: merge ppc_vsr_t and ppc_avr_t union types
>   target/ppc: move FP and VMX registers into aligned vsr register array
>   target/ppc: replace AVR* macros with Vsr* macros
> 
>  linux-user/ppc/signal.c |  28 +-
>  target/ppc/arch_dump.c  |  15 +-
>  target/ppc/cpu.h|  42 +-
>  target/ppc/gdbstub.c|   8 +-
>  target/ppc/int_helper.c |  86 ++--
>  target/ppc/internal.h   |  39 +-
>  target/ppc/kvm.c|  24 +-
>  target/ppc/machine.c|  72 ++-
>  target/ppc/monitor.c|   4 +-
>  target/ppc/translate.c  |  73 ++-
>  target/ppc/translate/dfp-impl.inc.c |   2 +-
>  target/ppc/translate/fp-impl.inc.c  | 486 +++-
>  target/ppc/translate/vmx-impl.inc.c | 154 +--
>  target/ppc/translate/vsx-impl.inc.c | 862 
> ++--
>  target/ppc/translate_init.inc.c |  26 +-
>  15 files changed, 1374 insertions(+), 547 deletions(-)
> 

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


signatu

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add some missing ppc-related files

[David Gibson]

On Wed, Jan 02, 2019 at 04:57:56PM +0100, Thomas Huth wrote:
> hw/gpio/mpc8xxx.c is only used by the e500 machine, so add it there.
> And the hw/input/adb* files are specific to the Mac machines, so
> they should be assigned to these categories.
> 
> Signed-off-by: Thomas Huth 

Applied to ppc-for-4.0, thanks.

> ---
>  MAINTAINERS | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ef6b7f6..eb037df 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -951,6 +951,7 @@ L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/ppc/e500.[hc]
>  F: hw/ppc/e500plat.c
> +F: hw/gpio/mpc8xxx.c
>  F: include/hw/ppc/ppc_e500.h
>  F: include/hw/pci-host/ppce500.h
>  F: pc-bios/u-boot.e500
> @@ -962,7 +963,7 @@ S: Odd Fixes
>  F: hw/ppc/mpc8544ds.c
>  F: hw/ppc/mpc8544_guts.c
>  
> -New World
> +New World (mac99)
>  M: David Gibson 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
> @@ -972,12 +973,14 @@ F: hw/pci-bridge/dec.[hc]
>  F: hw/misc/macio/
>  F: hw/misc/mos6522.c
>  F: hw/nvram/mac_nvram.c
> +F: hw/input/adb*
>  F: include/hw/misc/macio/
>  F: include/hw/misc/mos6522.h
>  F: include/hw/ppc/mac_dbdma.h
>  F: include/hw/pci-host/uninorth.h
> +F: include/hw/input/adb*
>  
> -Old World
> +Old World (g3beige)
>  M: David Gibson 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
> @@ -985,7 +988,9 @@ F: hw/ppc/mac_oldworld.c
>  F: hw/pci-host/grackle.c
>  F: hw/misc/macio/
>  F: hw/intc/heathrow_pic.c
> +F: hw/input/adb*
>  F: include/hw/intc/heathrow_pic.h
> +F: include/hw/input/adb*
>  
>  PReP
>  M: Hervé Poussineau 

-- 
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-io: Reinitialize optind correctly before parsing inner command.

[Eric Blake]

On 12/30/18 12:09 PM, Richard W.M. Jones wrote:
> On FreeBSD 11.2:
> 
>   $ ./qemu-io -f raw -c "aio_write 0 512" "nbd:localhost:10809"
>   Parsing error: non-numeric argument, or extraneous/unrecognized suffix -- 
> aio_write
> 
> After main option parsing, we reinitialize optind so we can parse each
> command.  The error happens when parsing the aio_write command.  After
> the aio_write getopt loop, optind == 0 and argv[optind] points to the
> command name ("aio_write" in this case).  The code fails because it
> tries to parse argv[optind] (which it thinks is the first argument) as
> an integer.
> 
> In fact optind _starts_ the loop as 0, because we set it to 0.
> 
> The FreeBSD manual page says:
> 
>   In order to use getopt() to evaluate multiple sets of arguments, or to
>   evaluate a single set of arguments multiple times, the variable optreset
>   must be set to 1 before the second and each additional set of calls to
>   getopt(), and the variable optind must be reinitialized.
> 
> (From the rest of the man page it is clear that optind must be
> reinitialized to 1).
> 
> Unfortunately this conflicts with the glibc man page which says:
> 
>   A program that scans multiple argument vectors,  or  rescans  the  same
>   vector  more than once, and wants to make use of GNU extensions such as
>   '+' and '-' at  the  start  of  optstring,  or  changes  the  value  of
>   POSIXLY_CORRECT  between scans, must reinitialize getopt() by resetting
>   optind to 0, rather than the traditional value of 1.  (Resetting  to  0
>   forces  the  invocation  of  an  internal  initialization  routine that
>   rechecks POSIXLY_CORRECT and checks for GNU extensions in optstring.)

The glibc manual implies that setting optind = 1 is a weak reset
(sufficient if you are NOT using either leading '+' or '-', and if you
do NOT expect POSIXLY_CORRECT to have changed value since the last
time), and that optind = 0 is a hard reset needed only when the weak
reset is insufficient.

> 
> Reinitialize optind to either 0 or 1 depending on whether we're using
> glibc or not.
> 
> I didn't set optreset - it's not present in glibc and it doesn't seem
> to make any difference on FreeBSD.

BSD has optreset as its way of forcing hard reset (instead of optind=0),
but both platforms allow optind=1 for soft reset.

> 
> Signed-off-by: Richard W.M. Jones 
> ---
>  qemu-io-cmds.c | 4 
>  1 file changed, 4 insertions(+)
> 

Given that none of our uses of getopt() in qemu-io-cmds.c rely on
leading '+' or '-', and that we don't call setenv() to change
POSIXLY_CORRECT on the fly, I think the simpler patch is to just blindly
set optind = 1 when we want a soft reset, as a hard reset is overkill
for our needs.

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 2c39124036..ca4e258579 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -114,7 +114,11 @@ static int command(BlockBackend *blk, const cmdinfo_t 
> *ct, int argc,
>  }
>  }
>  
> +#ifdef __GNU_LIBRARY__
>  optind = 0;
> +#else
> +optind = 1;
> +#endif

and thus we don't need the #ifdef __GNU_LIBRARY__ code, nor the syntax
check violation.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1810343] [NEW] qemu-nbd -l and -s options don't work together

[Michael]

Public bug reported:

When using qemu-nbd with -l to load a snapshot along with -s to create
new active layer the tool fails to find the snapshot specified on the
command line:

For example the following does not work:
  sudo qemu-nbd -s --load-snapshot=files  --connect /dev/nbd0 rootfs.qcow2  
 
  Failed to load snapshot: Can't find snapshot

However, the following option works
  sudo qemu-nbd -s --connect /dev/nbd0 rootfs.qcow2
and so does
  sudo qemu-nbd --load-snapshot=files  --connect /dev/nbd0 rootfs.qcow2

** 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/1810343

Title:
  qemu-nbd -l and -s options don't work together

Status in QEMU:
  New

Bug description:
  When using qemu-nbd with -l to load a snapshot along with -s to create
  new active layer the tool fails to find the snapshot specified on the
  command line:

  For example the following does not work:
sudo qemu-nbd -s --load-snapshot=files  --connect /dev/nbd0 rootfs.qcow2
   
Failed to load snapshot: Can't find snapshot

  However, the following option works
sudo qemu-nbd -s --connect /dev/nbd0 rootfs.qcow2
  and so does
sudo qemu-nbd --load-snapshot=files  --connect /dev/nbd0 rootfs.qcow2

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

Re: [Qemu-devel] [PULL 00/42] tcg queued patches

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20181225205529.10874-1-richard.hender...@linaro.org/



Hi,

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

Type: series
Message-id: 20181225205529.10874-1-richard.hender...@linaro.org
Subject: [Qemu-devel] [PULL 00/42] tcg queued patches

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
0f7294b tcg: Improve call argument loading
6aa2ac3 tcg: Record register preferences during liveness
982118d tcg: Add TCG_OPF_BB_EXIT
0dc2410 tcg: Split out more subroutines from liveness_pass_1
8c8f69a tcg: Rename and adjust liveness_pass_1 helpers
07ab04a tcg: Reindent parts of liveness_pass_1
44c70fc tcg: Dump register preference info with liveness
97ca81f tcg: Improve register allocation for matching constraints
b89850f tcg: Add output_pref to TCGOp
fc00884 tcg: Add preferred_reg argument to tcg_reg_alloc_do_movi
bec5e0c tcg: Add preferred_reg argument to temp_sync
3981b34 tcg: Add preferred_reg argument to temp_load
61a9975 tcg: Add preferred_reg argument to tcg_reg_alloc
7091b2d tcg: Add reachable_code_pass
8797c51 tcg: Reference count labels
b02acc5 tcg: Add TCG_CALL_NO_RETURN
c5ac9a3 tcg: Renumber TCG_CALL_* flags
12f1b06 linux-user: Add safe_syscall for riscv64 host
1bd00fc disas/microblaze: Remove unused REG_SP macro
d27ea8d configure: Add support for building RISC-V host
33c4186 disas: Add RISC-V support
88b5d5d tcg: Add RISC-V cpu signal handler
3eb2e05 tcg/riscv: Add the target init code
2880f15 tcg/riscv: Add the prologue generation and register the JIT
b6cf6f4 tcg/riscv: Add the out op decoder
c20edc5 tcg/riscv: Add direct load and store instructions
671a9f9 tcg/riscv: Add slowpath load and store instructions
68b9b41 tcg/riscv: Add branch and jump instructions
65276e1 tcg/riscv: Add the add2 and sub2 instructions
584bb6e tcg/riscv: Add the out load and store instructions
0b72e86 tcg/riscv: Add the extract instructions
23558fd tcg/riscv: Add the mov and movi instruction
30ffb28 tcg/riscv: Add the relocation functions
5bfaa0b tcg/riscv: Add the instruction emitters
0beb56b tcg/riscv: Add the immediate encoders
5ce042f tcg/riscv: Add support for the constraints
0b60272 tcg/riscv: Add the tcg target registers
f2633ad tcg/riscv: Add the tcg-target.h file
1a6fb6f exec: Add RISC-V GCC poison macro
5edbfa3 linux-user: Add host dependency for RISC-V 64-bit
147bba0 linux-user: Add host dependency for RISC-V 32-bit
6971b73 elf.h: Add the RISCV ELF magic numbers

=== OUTPUT BEGIN ===
Checking PATCH 1/42: elf.h: Add the RISCV ELF magic numbers...
Checking PATCH 2/42: linux-user: Add host dependency for RISC-V 32-bit...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

total: 0 errors, 1 warnings, 18 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/42: linux-user: Add host dependency for RISC-V 64-bit...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

total: 0 errors, 1 warnings, 18 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/42: exec: Add RISC-V GCC poison macro...
Checking PATCH 5/42: tcg/riscv: Add the tcg-target.h file...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

WARNING: architecture specific defines should be avoided
#74: FILE: tcg/riscv/tcg-target.h:28:
+#if __riscv_xlen == 32

total: 0 errors, 2 warnings, 199 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 6/42: tcg/riscv: Add the tcg target registers...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
new file mode 100644

total: 0 errors, 1 warnings, 118 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 7/42: tcg/riscv: Add support for the constraints...
Checking PATCH 8/42: tcg/riscv: Add the immediate encoders...
Checking PATCH 9/42: tcg/riscv: Add the instruction emitters...
Checking

Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating

[Eric Blake]

On 1/2/19 12:01 PM, Christophe Fergeau wrote:
> Adding Markus to cc: list, I forgot to do it when sending the patch.

Also worth backporting via qemu-stable, now in cc.

> 
> Christophe
> 
> On Wed, Jan 02, 2019 at 03:05:35PM +0100, Christophe Fergeau wrote:
>> commit 8bca4613 added support for %% in json strings when interpolating,
>> but in doing so, this broke handling of % when not interpolating as the
>> '%' is skipped in both cases.
>> This commit ensures we only try to handle %% when interpolating.
>>
>> Signed-off-by: Christophe Fergeau 
>> ---
>>  qobject/json-parser.c | 10 ++
>>  tests/check-qjson.c   |  5 +
>>  2 files changed, 11 insertions(+), 4 deletions(-)
>>

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 09/21] configure: use pkg-config for obtaining xen version

[Stefano Stabellini]

On Tue, 11 Dec 2018, Daniel wrote:
> On Tue, Dec 11, 2018 at 10:34:52AM +, Peter Maydell wrote:
> > On Tue, 25 Apr 2017 at 19:35, Stefano Stabellini  
> > wrote:
> > >
> > > From: Juergen Gross 
> > >
> > > Instead of trying to guess the Xen version to use by compiling various
> > > test programs first just ask the system via pkg-config. Only if it
> > > can't return the version fall back to the test program scheme.
> > >
> > > If configure is being called with dedicated flags for the Xen libraries
> > > use those instead of the pkg-config output. This will avoid breaking
> > > an in-tree Xen build of an old Xen version while a new Xen version is
> > > installed on the build machine: pkg-config would pick up the installed
> > > Xen config files as the Xen tree wouldn't contain any of them.
> > >
> > > Signed-off-by: Juergen Gross 
> > > Signed-off-by: Stefano Stabellini 
> > > Tested-by: Paul Durrant 
> > > Reviewed-by: Stefano Stabellini 
> > > ---
> > >  configure | 159 
> > > ++
> > >  1 file changed, 88 insertions(+), 71 deletions(-)
> > >
> > > diff --git a/configure b/configure
> > > index 271bea8..3133ef8 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -1975,30 +1975,46 @@ fi
> > >  # xen probe
> > >
> > >  if test "$xen" != "no" ; then
> > > -  xen_libs="-lxenstore -lxenctrl -lxenguest"
> > > -  xen_stable_libs="-lxencall -lxenforeignmemory -lxengnttab -lxenevtchn"
> > > +  # Check whether Xen library path is specified via --extra-ldflags to 
> > > avoid
> > > +  # overriding this setting with pkg-config output. If not, try 
> > > pkg-config
> > > +  # to obtain all needed flags.
> > > +
> > > +  if ! echo $EXTRA_LDFLAGS | grep tools/libxc > /dev/null && \
> > > + $pkg_config --exists xencontrol ; then
> > > +xen_ctrl_version="$(printf '%d%02d%02d' \
> > > +  $($pkg_config --modversion xencontrol | sed 's/\./ /g') )"
> > > +xen=yes
> > > +xen_pc="xencontrol xenstore xenguest xenforeignmemory xengnttab"
> > > +xen_pc="$xen_pc xenevtchn xendevicemodel"
> > > +QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags $xen_pc)"
> > > +libs_softmmu="$($pkg_config --libs $xen_pc) $libs_softmmu"
> > > +LDFLAGS="$($pkg_config --libs $xen_pc) $LDFLAGS"
> > > +  else
> > 
> > Hi -- this is an old patch, but MJT has just noticed that
> > it means that (assuming configure takes the "we have a pkg-config
> > for Xen" path) the Xen libraries get added to both libs_softmmu
> > and LDFLAGS, which means that everything, including the linux-user
> > binaries, gets linked against them. The old fallback path
> > only adds them to libs_softmmu.
> > 
> > Juergen: is there a reason why you added the libs to both
> > libs_softmmu and LDFLAGS here? Can we just delete the line
> > that alters LDFLAGS?
> 
> In the 'else' block that takes the non-pkg-config path, the libs are
> only added to "libs_softmmu". So I think removing LDFLAGS is right.

Yes, I think you are correct.

Re: [Qemu-devel] [PATCH v15 23/26] sched: early boot clock

[Salvatore Bonaccorso]

Hi,

On Tue, Nov 06, 2018 at 06:35:36AM -0500, Steven Sistare wrote:
> Pavel has a new email address, cc'd - steve
> 
> On 11/6/2018 12:42 AM, Dominique Martinet wrote:
> > (added various kvm/virtualization lists in Cc as well as qemu as I don't
> > know who's "wrong" here)
> > 
> > Pavel Tatashin wrote on Thu, Jul 19, 2018:
> >> Allow sched_clock() to be used before schec_clock_init() is called.
> >> This provides with a way to get early boot timestamps on machines with
> >> unstable clocks.
> > 
> > This isn't something I understand, but bisect tells me this patch
> > (landed as 857baa87b64 ("sched/clock: Enable sched clock early")) makes
> > a VM running with kvmclock take a step in uptime/printk timer early in
> > boot sequence as illustrated below. The step seems to be related to the
> > amount of time the host was suspended while qemu was running before the
> > reboot.
> > 
> > $ dmesg
> > ...
> > [0.00] SMBIOS 2.8 present.
> > [0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> > ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 04/01/2014
> > [0.00] Hypervisor detected: KVM
> > [0.00] kvm-clock: Using msrs 4b564d01 and 4b564d00
> > [283120.529821] kvm-clock: cpu 0, msr 321a8001, primary cpu clock
> > [283120.529822] clocksource: kvm-clock: mask: 0x 
> > max_cycles: 0x1cd42e4dffb, max_idle_ns: 881590591483 ns
> > [283120.529824] tsc: Detected 2592.000 MHz processor
> > ...
> > 
> > (The VM is x86_64 on x86_64, I can provide my .config on request but
> > don't think it's related)
> > 
> > 
> > It's rather annoying for me as I often reboot VMs and rely on the
> > 'uptime' command to check if I did just reboot or not as I have the
> > attention span of a goldfish; I'd rather not have to find something else
> > to check if I did just reboot or not.
> > 
> > Note that if the qemu process is restarted, there is no offset anymore.
> > 
> > I unfortunately just did that so cannot say with confidence (putting my
> > laptop to sleep for 30s only led to a 2s offset and I do not want to
> > wait longer right now), but it looks like the clock is still mostly
> > correct after reboot after disabling my VM's ntp client. Will infirm
> > that tomorrow if I was wrong.
> > 
> > 
> > Happy to try to help fixing this in any way, as written above the quote
> > I'm not even actually sure who is wrong here.

A user in Debian reported the same/similar issue (with 4.19.13):

https://bugs.debian.org/918036

Regards,
Salvatore

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add some missing SPARC-related files

[Philippe Mathieu-Daudé]

Hi Thomas,

On 1/2/19 4:46 PM, Thomas Huth wrote:
> These files / devices are only used by SPARC machines, so we can sort
> them into the corresponding categories in the MAINTAINERS file.
> 
> Signed-off-by: Thomas Huth 
> ---
>  MAINTAINERS | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b6c2645..e897963 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1069,9 +1069,11 @@ M: Mark Cave-Ayland 
>  S: Maintained
>  F: hw/sparc/sun4m.c
>  F: hw/sparc/sun4m_iommu.c
> +F: hw/display/cg3.c
> +F: hw/display/tcx.c
>  F: hw/dma/sparc32_dma.c
>  F: hw/misc/eccmemctl.c
> -F: hw/misc/slavio_misc.c
> +F: hw/*/slavio_*.c
>  F: include/hw/sparc/sparc32_dma.h
>  F: pc-bios/openbios-sparc32
>  F: include/hw/sparc/sun4m_iommu.h
> @@ -1083,6 +1085,8 @@ F: hw/sparc64/sun4u.c
>  F: pc-bios/openbios-sparc64
>  F: hw/pci-host/sabre.c
>  F: include/hw/pci-host/sabre.h
> +F: include/hw/sparc/sun4u_iommu.h

I sent a patch to trivial@ with other sun4u changes, reviewed by Mark:
https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04467.html

Neither mine nor your are complete :)
Can I include your patch when respining the series? (Not sure I need to
respin although).

> +F: include/hw/nvram/sun_nvram.h

sun_nvram.h is also used by sun4m (the previous section this patch updates).

>  
>  Sun4v
>  M: Artyom Tarasenko 
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH] json: Fix % handling when not interpolating

[Christophe Fergeau]

Adding Markus to cc: list, I forgot to do it when sending the patch.

Christophe

On Wed, Jan 02, 2019 at 03:05:35PM +0100, Christophe Fergeau wrote:
> commit 8bca4613 added support for %% in json strings when interpolating,
> but in doing so, this broke handling of % when not interpolating as the
> '%' is skipped in both cases.
> This commit ensures we only try to handle %% when interpolating.
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  qobject/json-parser.c | 10 ++
>  tests/check-qjson.c   |  5 +
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 7a7ae9e8d1..d8eb210c0c 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -208,11 +208,13 @@ static QString *parse_string(JSONParserContext *ctxt, 
> JSONToken *token)
>  }
>  break;
>  case '%':
> -if (ctxt->ap && ptr[1] != '%') {
> -parse_error(ctxt, token, "can't interpolate into string");
> -goto out;
> +if (ctxt->ap) {
> +if (ptr[1] != '%') {
> +parse_error(ctxt, token, "can't interpolate into 
> string");
> +goto out;
> +}
> +ptr++;
>  }
> -ptr++;
>  /* fall through */
>  default:
>  cp = mod_utf8_codepoint(ptr, 6, &end);
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index d876a7a96e..fa2afccb0a 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -175,6 +175,11 @@ static void utf8_string(void)
>  "\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
>  "\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
>  "\\u03BA\\u1F79\\u03C3\\u03BC\\u03B5",
> +},
> +/* '%' character when not interpolating */
> +{
> +"100%",
> +"100%",
>  },
>  /* 2  Boundary condition test cases */
>  /* 2.1  First possible sequence of a certain length */
> -- 
> 2.20.1
> 
> 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-phi...@redhat.com/



Hi,

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

Type: series
Message-id: 20181226171538.21984-1-phi...@redhat.com
Subject: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5ac365d util/cutils: Move function documentations to the header
1d4b496 util/cutils: Move ctype macros to "cutils.h"
70db713 util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"

=== OUTPUT BEGIN ===
Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to 
"cutils.h"...
WARNING: Block comments use a leading /* on a separate line
#42: FILE: include/qemu/cutils.h:160:
+/**

total: 0 errors, 1 warnings, 48 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/3: util/cutils: Move ctype macros to "cutils.h"...
WARNING: Block comments use a leading /* on a separate line
#100: FILE: include/qemu/cutils.h:6:
+/** unsigned ctype macros:

ERROR: code indent should never use tabs
#108: FILE: include/qemu/cutils.h:14:
+#define qemu_isalnum(c)^I^Iisalnum((unsigned char)(c))$

ERROR: code indent should never use tabs
#109: FILE: include/qemu/cutils.h:15:
+#define qemu_isalpha(c)^I^Iisalpha((unsigned char)(c))$

ERROR: code indent should never use tabs
#110: FILE: include/qemu/cutils.h:16:
+#define qemu_iscntrl(c)^I^Iiscntrl((unsigned char)(c))$

ERROR: code indent should never use tabs
#111: FILE: include/qemu/cutils.h:17:
+#define qemu_isdigit(c)^I^Iisdigit((unsigned char)(c))$

ERROR: code indent should never use tabs
#112: FILE: include/qemu/cutils.h:18:
+#define qemu_isgraph(c)^I^Iisgraph((unsigned char)(c))$

ERROR: code indent should never use tabs
#113: FILE: include/qemu/cutils.h:19:
+#define qemu_islower(c)^I^Iislower((unsigned char)(c))$

ERROR: code indent should never use tabs
#114: FILE: include/qemu/cutils.h:20:
+#define qemu_isprint(c)^I^Iisprint((unsigned char)(c))$

ERROR: code indent should never use tabs
#115: FILE: include/qemu/cutils.h:21:
+#define qemu_ispunct(c)^I^Iispunct((unsigned char)(c))$

ERROR: code indent should never use tabs
#116: FILE: include/qemu/cutils.h:22:
+#define qemu_isspace(c)^I^Iisspace((unsigned char)(c))$

ERROR: code indent should never use tabs
#117: FILE: include/qemu/cutils.h:23:
+#define qemu_isupper(c)^I^Iisupper((unsigned char)(c))$

ERROR: code indent should never use tabs
#118: FILE: include/qemu/cutils.h:24:
+#define qemu_isxdigit(c)^Iisxdigit((unsigned char)(c))$

ERROR: code indent should never use tabs
#119: FILE: include/qemu/cutils.h:25:
+#define qemu_tolower(c)^I^Itolower((unsigned char)(c))$

ERROR: code indent should never use tabs
#120: FILE: include/qemu/cutils.h:26:
+#define qemu_toupper(c)^I^Itoupper((unsigned char)(c))$

ERROR: code indent should never use tabs
#121: FILE: include/qemu/cutils.h:27:
+#define qemu_isascii(c)^I^Iisascii((unsigned char)(c))$

ERROR: code indent should never use tabs
#122: FILE: include/qemu/cutils.h:28:
+#define qemu_toascii(c)^I^Itoascii((unsigned char)(c))$

total: 15 errors, 1 warnings, 126 lines checked

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

Checking PATCH 3/3: util/cutils: Move function documentations to the header...
WARNING: Block comments use a leading /* on a separate line
#73: FILE: include/qemu/cutils.h:156:
+/**

WARNING: Block comments use a leading /* on a separate line
#95: FILE: include/qemu/cutils.h:179:
+/**

WARNING: Block comments use a leading /* on a separate line
#124: FILE: include/qemu/cutils.h:208:
+/**

WARNING: Block comments use a leading /* on a separate line
#154: FILE: include/qemu/cutils.h:238:
+/**

WARNING: Block comments use a leading /* on a separate line
#183: FILE: include/qemu/cutils.h:267:
+/**

WARNING: Block comments use a leading /* on a separate line
#214: FILE: include/qemu/cutils.h:298:
+/**

WARNING: Block comments use a leading /* on a separate line
#225: FILE: include/qemu/cutils.h:309:
+/**

WARNING: Block comments use a leading /* on a separate line
#235: FILE: include/qemu/cutils.h:319:
+/**

WARNING: Block comments use a leading /* on a separate line
#263: FILE: 

[Qemu-devel] [PATCH] MAINTAINERS: Add some missing ppc-related files

[Thomas Huth]

hw/gpio/mpc8xxx.c is only used by the e500 machine, so add it there.
And the hw/input/adb* files are specific to the Mac machines, so
they should be assigned to these categories.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index ef6b7f6..eb037df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -951,6 +951,7 @@ L: qemu-...@nongnu.org
 S: Odd Fixes
 F: hw/ppc/e500.[hc]
 F: hw/ppc/e500plat.c
+F: hw/gpio/mpc8xxx.c
 F: include/hw/ppc/ppc_e500.h
 F: include/hw/pci-host/ppce500.h
 F: pc-bios/u-boot.e500
@@ -962,7 +963,7 @@ S: Odd Fixes
 F: hw/ppc/mpc8544ds.c
 F: hw/ppc/mpc8544_guts.c
 
-New World
+New World (mac99)
 M: David Gibson 
 L: qemu-...@nongnu.org
 S: Odd Fixes
@@ -972,12 +973,14 @@ F: hw/pci-bridge/dec.[hc]
 F: hw/misc/macio/
 F: hw/misc/mos6522.c
 F: hw/nvram/mac_nvram.c
+F: hw/input/adb*
 F: include/hw/misc/macio/
 F: include/hw/misc/mos6522.h
 F: include/hw/ppc/mac_dbdma.h
 F: include/hw/pci-host/uninorth.h
+F: include/hw/input/adb*
 
-Old World
+Old World (g3beige)
 M: David Gibson 
 L: qemu-...@nongnu.org
 S: Odd Fixes
@@ -985,7 +988,9 @@ F: hw/ppc/mac_oldworld.c
 F: hw/pci-host/grackle.c
 F: hw/misc/macio/
 F: hw/intc/heathrow_pic.c
+F: hw/input/adb*
 F: include/hw/intc/heathrow_pic.h
+F: include/hw/input/adb*
 
 PReP
 M: Hervé Poussineau 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/1] drive-mirror: add incremental mode

[Eric Blake]

On 1/2/19 4:04 AM, mahaocong wrote:
> From: mahaocong 
> 
> This patch adds possibility to start mirroring with user-created-bitmap.
> Compare with v1, this patch fix some shortcomings.
> 1.separate feature about copy dirty-bitmap to an individual patch.
> 2.remove checking for cancelled after mirror_dirty_init_incremental for bitmap
>   copyimg don't have yield point.
> 3.adjuest input parameters on mirror_start_job and mirror_start, and so It is
>   no need to find bitmap on mirror_dirty_init_incremental again.
> 4.assert the bitmap name is NULL on blockdev_mirror_common.
> 5.change the parameter's name in qmp command 'drive-mirror' from 'bitmap_name'
>   to 'bitmap'.
> 
> As for the doubt about the new incremental mode, I think it is means that the
> initial bitmap is appointed by user, compare with the full mode and top mode.
> There is no different in other behaves, such as in iteration and complete
> action.
> 
> mahaocong (1):
>   drive-mirror: add incremental mode
>   mirror: add incremental mode. we must add a user-named-bitmap first, then 
> set
>   this bitmap as the initial bitmap on incremental mode drive-mirror.
>   in details, I first create unnamed-bitmap with the same granularity 
> of
>   user-bitmap. Next, copy It's hbitmap to unnamed-bitmap. Then, start
>   mirror with this unname-bitmap.
>   dirty-bitmap: add new API to copy dirty-bitmap. This feature is add in a
>   separate patch called "add new function to copy dirty-bitmap"

This is confusing. The first line 'mahaocong (1):' looks like you are
sending a patch series with only one patch (by the way, you can use 'git
commit format.coverletter auto' to only send a 0/N cover letter for N >
1, since a single patch is the only case where you don't technically
need a cover letter).  But the remaining three entries make it look like
you have three patches intended to be sent (and where the patch titled
"mirror: add incremental mode. we must..." is an awfully long subject line).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/1] drive-mirror: add incremental mode

[Eric Blake]

On 1/2/19 4:04 AM, mahaocong wrote:
> From: mahaocong 
> 
> Signed-off-by: mahaocong 

The subject line says "what" changed, but you are missing a commit body
that says "why" the change is useful.  Much of the content in your cover
letter should be copied into this commit body, as the cover letter does
not get stored into git.

> ---
>  block/mirror.c| 55 
> ---
>  blockdev.c| 37 +--
>  include/block/block_int.h |  3 ++-
>  qapi/block-core.json  |  9 +++-
>  4 files changed, 88 insertions(+), 16 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -1727,6 +1727,13 @@
>  #(all the disk, only the sectors allocated in the topmost image, or
>  #only new I/O).
>  #
> +# @bitmap: the name of user-created-bitmap which is used to initialize the
> +#  unnamed-bitmap used on incremental drive-mirror. It's hbitmap

"It's" should only be used when you could also have said "It is" or "It
has" - but in this sentence, you are using a possessive which is spelled
"Its".  However, the entire concept of an hbitmap is an internal
implementation detail, so I don't know if it makes sense to be
mentioning an hbitmap in this location.

> +#  will be copy to mirror bitmap. If user select incremental mode,
> +#  bitmap-name should not be null, and can not set granularity for
> +#  the mirror bitmap should have the same granularity with
> +#  user-created-bitmap.

Missing a '(since 4.0)' tag.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] MAINTAINERS: Add some missing SPARC-related files

[Thomas Huth]

These files / devices are only used by SPARC machines, so we can sort
them into the corresponding categories in the MAINTAINERS file.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b6c2645..e897963 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1069,9 +1069,11 @@ M: Mark Cave-Ayland 
 S: Maintained
 F: hw/sparc/sun4m.c
 F: hw/sparc/sun4m_iommu.c
+F: hw/display/cg3.c
+F: hw/display/tcx.c
 F: hw/dma/sparc32_dma.c
 F: hw/misc/eccmemctl.c
-F: hw/misc/slavio_misc.c
+F: hw/*/slavio_*.c
 F: include/hw/sparc/sparc32_dma.h
 F: pc-bios/openbios-sparc32
 F: include/hw/sparc/sun4m_iommu.h
@@ -1083,6 +1085,8 @@ F: hw/sparc64/sun4u.c
 F: pc-bios/openbios-sparc64
 F: hw/pci-host/sabre.c
 F: include/hw/pci-host/sabre.h
+F: include/hw/sparc/sun4u_iommu.h
+F: include/hw/nvram/sun_nvram.h
 
 Sun4v
 M: Artyom Tarasenko 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 0/5] Fix strncpy() warnings for GCC8 new -Wstringop-truncation

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181228173356.15359-1-phi...@redhat.com/



Hi,

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

Type: series
Message-id: 20181228173356.15359-1-phi...@redhat.com
Subject: [Qemu-devel] [PATCH v4 0/5] Fix strncpy() warnings for GCC8 new 
-Wstringop-truncation

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7afec15 migration: Use strnlen() for fixed-size string
3f0d878 migration: Fix stringop-truncation warning
f6b1b52 hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays
16e5ce8 block/sheepdog: Use QEMU_NONSTRING for non NUL-terminated arrays
f4ac58a qemu/compiler: Define QEMU_NONSTRING

=== OUTPUT BEGIN ===
Checking PATCH 1/5: qemu/compiler: Define QEMU_NONSTRING...
WARNING: architecture specific defines should be avoided
#53: FILE: include/qemu/compiler.h:163:
+#if __has_attribute(nonstring)

total: 0 errors, 1 warnings, 21 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/5: block/sheepdog: Use QEMU_NONSTRING for non NUL-terminated 
arrays...
Checking PATCH 3/5: hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays...
ERROR: space prohibited before open square bracket '['
#82: FILE: include/hw/acpi/acpi-defs.h:60:
+uint8_t  oem_id [6] \

WARNING: Block comments use a leading /* on a separate line
#83: FILE: include/hw/acpi/acpi-defs.h:61:
+ QEMU_NONSTRING; /* OEM identification */ \

ERROR: space prohibited before open square bracket '['
#84: FILE: include/hw/acpi/acpi-defs.h:62:
+uint8_t  oem_table_id [8] \

WARNING: Block comments use a leading /* on a separate line
#85: FILE: include/hw/acpi/acpi-defs.h:63:
+ QEMU_NONSTRING; /* OEM table identification */ \

ERROR: space prohibited before open square bracket '['
#88: FILE: include/hw/acpi/acpi-defs.h:65:
+uint8_t  asl_compiler_id [4] \

WARNING: Block comments use a leading /* on a separate line
#89: FILE: include/hw/acpi/acpi-defs.h:66:
+ QEMU_NONSTRING; /* ASL compiler vendor ID */ \

total: 3 errors, 3 warnings, 48 lines checked

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

Checking PATCH 4/5: migration: Fix stringop-truncation warning...
Checking PATCH 5/5: migration: Use strnlen() for fixed-size string...
WARNING: Block comments use a leading /* on a separate line
#49: FILE: migration/global_state.c:94:
+/* This condition should never happen during migration, because

total: 0 errors, 1 warnings, 25 lines checked

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

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181228173356.15359-1-phi...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [Bug 1809665] Re: Xbox One controller USB passthrough disconnections and stops

[Saverio Miroddi]

So! These are the options and respective logs; they still don't make the
controller work - it doesn't work at all.

# option 1

  -device nec-usb-xhci \
  -device usb-tablet \
  -device 
usb-host,vendorid=0x$VGAPT_XBOX_PAD_VEND_ID,productid=0x$VGAPT_XBOX_PAD_PROD_ID 
\

# log

libusb: error [udev_hotplug_event] ignoring udev action bind
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
libusb: error [_open_sysfs_attr] open 
/sys/bus/usb/devices/1-10/bConfigurationValue failed ret=-1 errno=2
libusb: error [_get_usbfs_fd] File doesn't exist, wait 10 ms and try again
libusb: error [_get_usbfs_fd] libusb couldn't open USB device 
/dev/bus/usb/001/019: No such file or directory
libusb: error [udev_hotplug_event] ignoring udev action bind
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
libusb: error [_open_sysfs_attr] open 
/sys/bus/usb/devices/1-10/bConfigurationValue failed ret=-1 errno=2
libusb: error [_get_usbfs_fd] File doesn't exist, wait 10 ms and try again
libusb: error [_get_usbfs_fd] libusb couldn't open USB device 
/dev/bus/usb/001/020: No such file or directory
libusb: error [udev_hotplug_event] ignoring udev action bind
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
libusb: error [_open_sysfs_attr] open 
/sys/bus/usb/devices/1-10/bConfigurationValue failed ret=-1 errno=2
libusb: error [_get_usbfs_fd] File doesn't exist, wait 10 ms and try again
libusb: error [_get_usbfs_fd] libusb couldn't open USB device 
/dev/bus/usb/001/021: No such file or directory
libusb: error [udev_hotplug_event] ignoring udev action bind

# option 2

-device usb-ehci \
-device usb-tablet \
-device 
usb-host,vendorid=0x$VGAPT_XBOX_PAD_VEND_ID,productid=0x$VGAPT_XBOX_PAD_PROD_ID 
\

# log

qemu-system-x86_64: Warning: speed mismatch trying to attach usb device 
"Controller" (full speed) to bus "usb-bus.0", port "2" (high speed)
libusb: error [udev_hotplug_event] ignoring udev action bind
qemu-system-x86_64: Warning: speed mismatch trying to attach usb device 
"Controller" (full speed) to bus "usb-bus.0", port "2" (high speed)
libusb: error [udev_hotplug_event] ignoring udev action bind
qemu-system-x86_64: Warning: speed mismatch trying to attach usb device 
"Controller" (full speed) to bus "usb-bus.0", port "2" (high speed)
libusb: error [udev_hotplug_event] ignoring udev action bind

I've tried another game controller with the second option:

# option 2b

-device usb-ehci \
-device usb-tablet \
-device 
usb-host,vendorid=0x$VGAPT_ARCADE_STICK_VEND_ID,productid=0x$VGAPT_ARCADE_STICK_PROD_ID
 \

# log

libusb: error [udev_hotplug_event] ignoring udev action bind
qemu-system-x86_64: Warning: speed mismatch trying to attach usb device 
"Arcade Fight Stick" (full speed) to bus "usb-bus.0", port "3" (high speed)
qemu-system-x86_64: Warning: speed mismatch trying to attach usb device 
"Arcade Fight Stick" (full speed) to bus "usb-bus.0", port "3" (high speed)
qemu-system-x86_64: Warning: speed mismatch trying to attach usb device 
"Arcade Fight Stick" (full speed) to bus "usb-bus.0", port "3" (high speed)

and in this configuration, it doesn't work (it does with `-usb`).

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

Title:
  Xbox One controller USB passthrough disconnections and stops

Status in QEMU:
  New

Bug description:
  I can't properly passthrough my Xbox One controller to a virtual
  machine; it causes USB disconnections on the host, ultimately
  preventing it to work (at all) on the guest

  I've seen a few other cases reported in other websites, which show the
  same symptoms:

  - https://www.reddit.com/r/VFIO/comments/97dhbw/qemu_w10_xbox_one_controller
  - 
https://unix.stackexchange.com/questions/452751/how-can-i-pass-through-an-xbox-one-controller-to-a-windows-vm-on-ubuntu

  This is sample:

  libusb: error [udev_hotplug_event] ignoring udev action bind
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  libusb: error [_get_usbfs_fd] File doesn't exist, wait 10 ms and try again
  libusb: error [_get_usbfs_fd] lib

Re: [Qemu-devel] [PATCH v2 3/3] util: check the return value of fcntl in qemu_set_{block, nonblock}

[Michael S. Tsirkin]

On Wed, Jan 02, 2019 at 03:07:24PM +0100, Thomas Huth wrote:
> On 2018-12-15 13:03, Li Qiang wrote:
> > Assert that the return value is not an error. This is like commit
> > 7e6478e7d4f for qemu_set_cloexec.
> > 
> > Signed-off-by: Li Qiang 
> > ---
> >  util/oslib-posix.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> > index c1bee2a581..4ce1ba9ca4 100644
> > --- a/util/oslib-posix.c
> > +++ b/util/oslib-posix.c
> > @@ -233,14 +233,18 @@ void qemu_set_block(int fd)
> >  {
> >  int f;
> >  f = fcntl(fd, F_GETFL);
> > -fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
> > +assert(f != -1);
> > +f = fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
> > +assert(f != -1);
> >  }
> >  
> >  void qemu_set_nonblock(int fd)
> >  {
> >  int f;
> >  f = fcntl(fd, F_GETFL);
> > -fcntl(fd, F_SETFL, f | O_NONBLOCK);
> > +assert(f != -1);
> > +f = fcntl(fd, F_SETFL, f | O_NONBLOCK);
> > +assert(f != -1);
> >  }
> 
> Reviewed-by: Thomas Huth 
> 
> Michael, could you take this patch series through your vhost tree? Or
> shall I pick them up for the qtests tree? In the latter case, please
> provide an ACK for the second patch.

Pls go ahead and merge it.

-- 
MST

Re: [Qemu-devel] [PATCH v2 1/3] tests: vhost-user-test: initialize 'fd' in chr_read

[Michael S. Tsirkin]

On Wed, Jan 02, 2019 at 02:50:50PM +0100, Thomas Huth wrote:
> On 2018-12-15 13:03, Li Qiang wrote:
> > Currently when processing VHOST_USER_SET_VRING_CALL
> > if 'qemu_chr_fe_get_msgfds' get no fd, the 'fd' will
> > be a stack uninitialized value.
> > 
> > Signed-off-by: Li Qiang 
> > ---
> >  tests/vhost-user-test.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> > index 45d58d8ea2..86039e61e0 100644
> > --- a/tests/vhost-user-test.c
> > +++ b/tests/vhost-user-test.c
> > @@ -309,7 +309,7 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> > int size)
> >  CharBackend *chr = &s->chr;
> >  VhostUserMsg msg;
> >  uint8_t *p = (uint8_t *) &msg;
> > -int fd;
> > +int fd = -1;
> >  
> >  if (s->test_fail) {
> >  qemu_chr_fe_disconnect(chr);
> > 
> 
> Shouldn't we also rather check the return code of
> qemu_chr_fe_get_msgfds() ? Anyway, initializing fd to -1 here sounds
> like a good idea, so:
> 
> Reviewed-by: Thomas Huth 

Reviewed-by: Michael S. Tsirkin 

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add ARM-related files for hw/[misc|input|timer]/

[Thomas Huth]

On 2019-01-02 15:42, Philippe Mathieu-Daudé wrote:
> On 1/2/19 1:53 PM, Thomas Huth wrote:
>> Some of the files in hw/input/, hw/misc/ and hw/timer/ are only
>> used by one of the ARM machines, so we can assign these files to
>> the corresponding boards.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  MAINTAINERS | 16 ++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 180695f..693abdf 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
[...]
>> @@ -688,10 +698,10 @@ L: qemu-...@nongnu.org
>>  S: Odd Fixes
>>  F: hw/arm/sabrelite.c
>>  F: hw/arm/fsl-imx6.c
>> -F: hw/misc/imx6_src.c
>> +F: hw/misc/imx6_*.c
>>  F: hw/ssi/imx_spi.c
>>  F: include/hw/arm/fsl-imx6.h
>> -F: include/hw/misc/imx6_src.h
>> +F: include/hw/misc/imx6_*.h
>>  F: include/hw/ssi/imx_spi.h
> 
> New models for the MCIMX6UL EVK might eventually match here.

The current files use "*imx6ul*" which do not match here due to the
underscore after the "6". So I hope future files will use this naming
scheme, too, then we should be fine.

>>  
>>  Sharp SL-5500 (Collie) PDA
>> @@ -802,7 +812,9 @@ R: Joel Stanley 
>>  L: qemu-...@nongnu.org
>>  S: Maintained
>>  F: hw/*/*aspeed*
>> +F: hw/misc/pca9552.c
>>  F: include/hw/*/*aspeed*
>> +F: include/hw/misc/pca9552*.h
>>  F: hw/net/ftgmac100.c
>>  F: include/hw/net/ftgmac100.h
> 
> OK
> 
> Reviewed-by: Philippe Mathieu-Daudé 

Thanks!

 Thomas

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add ARM-related files for hw/[misc|input|timer]/

[Philippe Mathieu-Daudé]

On 1/2/19 1:53 PM, Thomas Huth wrote:
> Some of the files in hw/input/, hw/misc/ and hw/timer/ are only
> used by one of the ARM machines, so we can assign these files to
> the corresponding boards.
> 
> Signed-off-by: Thomas Huth 
> ---
>  MAINTAINERS | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 180695f..693abdf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -510,6 +510,7 @@ F: hw/intc/arm*
>  F: hw/intc/gic_internal.h
>  F: hw/misc/a9scu.c
>  F: hw/misc/arm11scu.c
> +F: hw/misc/arm_l2x0.c
>  F: hw/timer/a9gtimer*
>  F: hw/timer/arm*
>  F: include/hw/arm/arm*.h
> @@ -582,6 +583,7 @@ L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/integratorcp.c
>  F: hw/misc/arm_integrator_debug.c
> +F: include/hw/misc/arm_integrator_debug.h

OK

>  
>  MCIMX6UL EVK / i.MX6ul
>  M: Peter Maydell 
> @@ -601,7 +603,9 @@ L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/arm/mcimx7d-sabre.c
>  F: hw/arm/fsl-imx7.c
> +F: hw/misc/imx7_*.c
>  F: include/hw/arm/fsl-imx7.h
> +F: include/hw/misc/imx7_*.h
>  F: hw/pci-host/designware.c
>  F: include/hw/pci-host/designware.h

OK

>  
> @@ -635,6 +639,10 @@ M: Peter Maydell 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/arm/nseries.c
> +F: hw/input/lm832x.c
> +F: hw/input/tsc2005.c
> +F: hw/misc/cbus.c
> +F: hw/timer/twl92230.c

OK

>  
>  Palm
>  M: Andrzej Zaborowski 
> @@ -642,6 +650,7 @@ M: Peter Maydell 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/arm/palm.c
> +F: hw/input/tsc210x.c

OK

>  
>  Raspberry Pi
>  M: Peter Maydell 
> @@ -678,6 +687,7 @@ F: hw/display/tc6393xb.c
>  F: hw/gpio/max7310.c
>  F: hw/gpio/zaurus.c
>  F: hw/misc/mst_fpga.c
> +F: hw/misc/max111x.c
>  F: include/hw/arm/pxa.h
>  F: include/hw/arm/sharpsl.h

OK

>  
> @@ -688,10 +698,10 @@ L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/arm/sabrelite.c
>  F: hw/arm/fsl-imx6.c
> -F: hw/misc/imx6_src.c
> +F: hw/misc/imx6_*.c
>  F: hw/ssi/imx_spi.c
>  F: include/hw/arm/fsl-imx6.h
> -F: include/hw/misc/imx6_src.h
> +F: include/hw/misc/imx6_*.h
>  F: include/hw/ssi/imx_spi.h

New models for the MCIMX6UL EVK might eventually match here.

>  
>  Sharp SL-5500 (Collie) PDA
> @@ -802,7 +812,9 @@ R: Joel Stanley 
>  L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/*/*aspeed*
> +F: hw/misc/pca9552.c
>  F: include/hw/*/*aspeed*
> +F: include/hw/misc/pca9552*.h
>  F: hw/net/ftgmac100.c
>  F: include/hw/net/ftgmac100.h

OK

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH] cpus: ignore ESRCH in qemu_cpu_kick_thread()

[Philippe Mathieu-Daudé]

On 1/2/19 3:16 PM, Laurent Vivier wrote:
> We can have a race condition between qemu_cpu_kick_thread() and
> qemu_kvm_cpu_thread_fn() when we hotunplug a CPU. In this case,
> qemu_cpu_kick_thread() can try to kick a thread that is exiting.
> pthread_kill() returns an error and qemu is stopped by an exit(1).
> 
>qemu:qemu_cpu_kick_thread: No such process
> 
> We can ignore safely this error.
> 
> Signed-off-by: Laurent Vivier 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  cpus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 0ddeeefc14..4717490bd0 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1778,7 +1778,7 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
>  }
>  cpu->thread_kicked = true;
>  err = pthread_kill(cpu->thread->thread, SIG_IPI);
> -if (err) {
> +if (err && err != ESRCH) {
>  fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
>  exit(1);
>  }
> 

[Qemu-devel] [Bug 1795100] Re: VNC unix-domain socket unlink()ed prematurely

[Dr. David Alan Gilbert]

Added Daniel to the bug.

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

Title:
  VNC unix-domain socket unlink()ed prematurely

Status in QEMU:
  New

Bug description:
  With qemu 3.0.0 (I don't believe this happened with previous
  versions), if I tell it `-vnc unix:/path/to/vnc.sock`, qemu will
  unlink() that file when the first client disconnects, meaning that
  once I disconnect, I can't ever reconnect without restarting the VM.

  A stupid testcase demonstrating the issue:

  In terminal A:

  $ qemu-system-x86_64 -vnc unix:$PWD/vnc.sock

  In terminal B:

  $ ls vnc.sock
  vnc.sock
  $ socat STDIO UNIX-CONNECT:vnc.sock <<<''
  RFB 003.008
  $ ls vnc.sock
  ls: cannot access 'vnc.sock': No such file or directory

  I have determined that the offending unlink() call is the one in
  io/channel-socket.c:qio_channel_socket_close().  That call was first
  introduced in commit d66f78e1eaa832f73c771d9df1b606fe75d52a50, which
  first appeared in version 3.0.0.

  This type of premature unlink() does not happen on monitor.sock with
  `-monitor unix:/path/to/monitor.sock,server,nowait`.

  I am not familiar enough with the QIO subsystem to suggest a fix that
  fixes VNC, but preserves the QMP fix targeted in the offending commit.

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

Re: [Qemu-devel] [Qemu-block] [PATCH] dmg: fix binary search

[Julio Faracco]

Hi Stefan,

I created a simple DMG file from MacOS to reproduce the problem:
https://bugs.launchpad.net/qemu/+bug/1809304

Em qua, 2 de jan de 2019 às 08:47, Stefan Hajnoczi 
escreveu:

> On Fri, Dec 21, 2018 at 09:58:03PM +0800, yuchenlin wrote:
> > There is a possible hang in original binary searsh implemtation. That is
> > if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
> >
> > The chunk1 will be still 4, and so on.
> >
> > Signed-off-by: yuchenlin 
> > ---
> >  block/dmg.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Do you have an example dmg file that triggers this bug?
>
> > diff --git a/block/dmg.c b/block/dmg.c
> > index 50e91aef6d..0e05702f5d 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -572,14 +572,14 @@ static inline uint32_t search_chunk(BDRVDMGState
> *s, uint64_t sector_num)
> >  {
> >  /* binary search */
> >  uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
> > -while (chunk1 != chunk2) {
> > +while (chunk1 <= chunk2) {
> >  chunk3 = (chunk1 + chunk2) / 2;
> >  if (s->sectors[chunk3] > sector_num) {
> > -chunk2 = chunk3;
> > +chunk2 = chunk3 - 1;
>
> What if chunk1 = 0 and chunk2 = 1?  chunk3 = 0 and the new chunk2 value
> would be 0x so the loop continues with out-of-bounds sectors[]
> accesses!
>

[Qemu-devel] [PATCH] cpus: ignore ESRCH in qemu_cpu_kick_thread()

[Laurent Vivier]

We can have a race condition between qemu_cpu_kick_thread() and
qemu_kvm_cpu_thread_fn() when we hotunplug a CPU. In this case,
qemu_cpu_kick_thread() can try to kick a thread that is exiting.
pthread_kill() returns an error and qemu is stopped by an exit(1).

   qemu:qemu_cpu_kick_thread: No such process

We can ignore safely this error.

Signed-off-by: Laurent Vivier 
---
 cpus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 0ddeeefc14..4717490bd0 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1778,7 +1778,7 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
 }
 cpu->thread_kicked = true;
 err = pthread_kill(cpu->thread->thread, SIG_IPI);
-if (err) {
+if (err && err != ESRCH) {
 fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
 exit(1);
 }
-- 
2.20.1

Re: [Qemu-devel] [PATCH] dmg: Fixing wrong dmg block type value for block terminator.

[Stefan Hajnoczi]

On Fri, Dec 28, 2018 at 12:50:55PM -0200, Julio Faracco wrote:
> This is a trivial patch to fix a wrong value for block terminator.
> The old value was 0x7fff which is wrong. It was not affecting the
> code because QEMU dmg block is not handling block terminator right now.
> Neverthless, it should be fixed.
> 
> Signed-off-by: Julio Faracco 
> ---
>  block/dmg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 3/3] util: check the return value of fcntl in qemu_set_{block, nonblock}

[Thomas Huth]

On 2018-12-15 13:03, Li Qiang wrote:
> Assert that the return value is not an error. This is like commit
> 7e6478e7d4f for qemu_set_cloexec.
> 
> Signed-off-by: Li Qiang 
> ---
>  util/oslib-posix.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index c1bee2a581..4ce1ba9ca4 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -233,14 +233,18 @@ void qemu_set_block(int fd)
>  {
>  int f;
>  f = fcntl(fd, F_GETFL);
> -fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
> +assert(f != -1);
> +f = fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
> +assert(f != -1);
>  }
>  
>  void qemu_set_nonblock(int fd)
>  {
>  int f;
>  f = fcntl(fd, F_GETFL);
> -fcntl(fd, F_SETFL, f | O_NONBLOCK);
> +assert(f != -1);
> +f = fcntl(fd, F_SETFL, f | O_NONBLOCK);
> +assert(f != -1);
>  }

Reviewed-by: Thomas Huth 

Michael, could you take this patch series through your vhost tree? Or
shall I pick them up for the qtests tree? In the latter case, please
provide an ACK for the second patch.

[Qemu-devel] [PATCH] json: Fix % handling when not interpolating

[Christophe Fergeau]

commit 8bca4613 added support for %% in json strings when interpolating,
but in doing so, this broke handling of % when not interpolating as the
'%' is skipped in both cases.
This commit ensures we only try to handle %% when interpolating.

Signed-off-by: Christophe Fergeau 
---
 qobject/json-parser.c | 10 ++
 tests/check-qjson.c   |  5 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 7a7ae9e8d1..d8eb210c0c 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -208,11 +208,13 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
 }
 break;
 case '%':
-if (ctxt->ap && ptr[1] != '%') {
-parse_error(ctxt, token, "can't interpolate into string");
-goto out;
+if (ctxt->ap) {
+if (ptr[1] != '%') {
+parse_error(ctxt, token, "can't interpolate into string");
+goto out;
+}
+ptr++;
 }
-ptr++;
 /* fall through */
 default:
 cp = mod_utf8_codepoint(ptr, 6, &end);
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index d876a7a96e..fa2afccb0a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -175,6 +175,11 @@ static void utf8_string(void)
 "\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
 "\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
 "\\u03BA\\u1F79\\u03C3\\u03BC\\u03B5",
+},
+/* '%' character when not interpolating */
+{
+"100%",
+"100%",
 },
 /* 2  Boundary condition test cases */
 /* 2.1  First possible sequence of a certain length */
-- 
2.20.1

Re: [Qemu-devel] [PATCH] scripts/qemugdb: support coroutine backtrace in coredumps

[Stefan Hajnoczi]

On Thu, Dec 27, 2018 at 05:36:04PM +, Vladimir Sementsov-Ogievskiy wrote:
> 23.04.2018 16:28, Pedro Alves wrote:
> > On 04/23/2018 02:37 AM, Simon Marchi wrote:
> >> On 2018-04-09 10:08 PM, Stefan Hajnoczi wrote:
> >>> I wonder what the point of select-frame is then...
> >>>
> >>> I have CCed the GDB mailing list.  Maybe someone can help us.  Context:
> >>>
> >>> QEMU implements coroutines using jmpbuf.  We'd like to print coroutine
> >>> call stacks in GDB and have a script that works when a process is being
> >>> debugged (it sets the registers).
> >>>
> >>> Now we'd like to extend the script to work on core dumps where it's not
> >>> possible to set registers (since there is no process being debugged).
> >>>
> >>> Is there a way to backtrace an arbitrary call stack in a core dump?
> >>
> >> Not that I know of.  The "frame  " form of the frame
> >> command sounds like it should be usable to achieve that, but it doesn't
> >> seem to work in that way.  I really wonder if it's working as it was
> >> intended initially.  I guess using that form of the frame command should
> >> override/mask the real current values of $sp and $pc?
> > 
> > Yeah, "frame " has a lot of problems.
> > 
> > This series was working toward sorting out the "frame" command:
> > 
> >https://sourceware.org/ml/gdb-patches/2015-09/msg00248.html
> > 
> > Follow the urls there for more background.
> > 
> > To me, the important questions to answer are here:
> >   https://sourceware.org/ml/gdb-patches/2015-09/msg00658.html
> > 
> > Unfortunately, I don't think the series moved past that point.
> > 
> > Thanks,
> > Pedro Alves
> > 
> 
> 
> Hi Pedro!
> 
> Hmm, returned to this topic. I've spent this day digging in gdb code, and 
> found it much
> more difficult than qemu)..
> 
> I've failed to find something like
> 
> create_frame_with_registers, or create_thread_with_registers.. Looks like 
> registers comes
> from some register caches, backed by different sources of registers or 
> something like this.
> 
> So, I'd like to ask several questions:
> 
> 1. Any news on the topic since April?

Not on my side, sorry.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-4.0 v9 00/16] qemu_thread_create: propagate the error to callers to handle

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181225140449.15786-1-...@suse.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

libpmem support   no
libudev   no

WARNING: Use of SDL 1.2 is deprecated and will be removed in
WARNING: future releases. Please switch to using SDL 2.0

NOTE: cross-compilers enabled:  'cc'
  GEN x86_64-softmmu/config-devices.mak.tmp
---
  CC  hw/usb/host-stub.o
  CC  hw/virtio/virtio-bus.o
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c: In function 
'init_event_notifier':
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c:404:9: error: implicit 
declaration of function 'error_setg' [-Werror=implicit-function-declaration]
 error_setg(errp, "ccid-card-emul: event notifier creation failed");
 ^
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c:404:9: error: nested extern 
declaration of 'error_setg' [-Werror=nested-externs]
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c: In function 'emulated_realize':
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c:513:13: error: implicit 
declaration of function 'error_append_hint' 
[-Werror=implicit-function-declaration]
 error_append_hint(errp, "%s\n", ptable->name);
 ^
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c:513:13: error: nested extern 
declaration of 'error_append_hint' [-Werror=nested-externs]
/tmp/qemu-test/src/hw/usb/ccid-card-emulated.c: At top level:
cc1: error: unrecognized command line option "-Wno-format-truncation" [-Werror]
cc1: all warnings being treated as errors
make: *** [hw/usb/ccid-card-emulated.o] Error 1
make: *** Waiting for unfinished jobs


The full log is available at
http://patchew.org/logs/20181225140449.15786-1-...@suse.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH v2 1/3] tests: vhost-user-test: initialize 'fd' in chr_read

[Thomas Huth]

On 2018-12-15 13:03, Li Qiang wrote:
> Currently when processing VHOST_USER_SET_VRING_CALL
> if 'qemu_chr_fe_get_msgfds' get no fd, the 'fd' will
> be a stack uninitialized value.
> 
> Signed-off-by: Li Qiang 
> ---
>  tests/vhost-user-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 45d58d8ea2..86039e61e0 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -309,7 +309,7 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> int size)
>  CharBackend *chr = &s->chr;
>  VhostUserMsg msg;
>  uint8_t *p = (uint8_t *) &msg;
> -int fd;
> +int fd = -1;
>  
>  if (s->test_fail) {
>  qemu_chr_fe_disconnect(chr);
> 

Shouldn't we also rather check the return code of
qemu_chr_fe_get_msgfds() ? Anyway, initializing fd to -1 here sounds
like a good idea, so:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 0/8] tests: apci: consolidate and cleanup ACPI test code

[Michael S. Tsirkin]

On Wed, Jan 02, 2019 at 02:11:23PM +0100, Thomas Huth wrote:
> On 2018-12-27 15:13, Igor Mammedov wrote:
> > Changes since v1:
> >   * rebase on top of current master due to a lots of conflicts with
> > 'qtest global' removal being merged first
> 
> Sorry for that ... I hope it was not too much trouble!
> 
> >   * drop explicit cast to uint8_t* as sdt->aml is uint8_t* now
> >   * drop not comment explaining strange offsets as offsets are
> > now follow ACPI spec
> > 
> > While working on adding tests for virt/arm board (uefi/XSDT/64-bit table 
> > pointers),
> > I found it's rather difficult to deal with mixed ACPI testing code that 
> > we've
> > collected so far. So instead of just adding a pile of XSDT hacks on top, 
> > here
> > goes small refactoring series:
> >* that removes dead code
> >* replaces reading tables with a fetch per table everywhere instead of
> >  mix of field by field and whole table
> >* consolidates the way tables are read (reduces code duplication)
> >* test no longer depends on ACPI structures from QEMU (i.e. doesn't 
> > affected
> >  by mistakes there) 
> >* fixes FACS not being compared against reference tables
> > Overall test is reduced on ~160LOC and hopefully it makes easier to follow 
> > and
> > add more stuff on top.
> > 
> > PS:
> > arm/virt test patches fill follow up a separate series on top of this one
> > for not to mix things up
> > 
> > Git tree for testing:
> >   https://github.com/imammedo/qemu acpi_tests_cleanup_v2
> > 
> > CC: "Michael S. Tsirkin" 
> > CC: Thomas Huth 
> 
> Do you want me to take this through the qtests tree, or will you take it
> through your ACPI tree?
> 
> In the latter case, FWIW:
> Acked-by: Thomas Huth 

I'll merge it, thanks for the ack!

Re: [Qemu-devel] live migration

[Dr. David Alan Gilbert]

* ? ? (zach_tur...@outlook.com) wrote:
> 
> Hello, I have a question I would like to ask.
> If I add the -incoming parameter when starting the qemu virtual machine, the 
> virtual machine will block all the time, waiting for the migration connection 
> request to arrive.
> I want to modify the time of this blocking wait, how should I modify the 
> source code?

It's not a timeout; it's a permanent wait for an incoming VM image.

What are you trying to do, and what would you like it to do if
it were to time out?

Dave

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

Re: [Qemu-devel] [RFC v2 4/4] pvh: Boot uncompressed kernel using direct boot ABI

[Stefan Hajnoczi]

On Fri, Dec 21, 2018 at 08:03:52PM +, Liam Merwick wrote:
> @@ -1336,7 +1470,7 @@ void pc_memory_init(PCMachineState *pcms,
>  int linux_boot, i;
>  MemoryRegion *ram, *option_rom_mr;
>  MemoryRegion *ram_below_4g, *ram_above_4g;
> -FWCfgState *fw_cfg;
> +FWCfgState *fw_cfg = NULL;

What is the purpose of this change?


signature.asc
Description: PGP signature

Re: [Qemu-devel] d_off field in struct dirent and 32-on-64 emulation

[Adhemerval Zanella]

On 31/12/2018 15:03, Joseph Myers wrote:
> On Fri, 28 Dec 2018, Adhemerval Zanella wrote:
> 
 Currently we only have nios2 and csky (unfortunately).  But since generic 
 definition for off_t and off64_t still assumes non-LFS support, all new
 32-bits ports potentially might carry the issue.
>>>
>>> For csky, we could still change the type of the non-standard d_off
>>> field to long long int.  This way, only telldir would have to fail
>>> when truncation is necessary, as mentioned below:
>>
>> I think it makes no sense to continue making non-LFS as default for
>> newer 32 bits ports, the support will be emulated with LFS syscalls.
> 
> Any new 32-bit port that uses 64-bit time_t will also use 64-bit offsets 
> (because we don't have any glibc configurations that support the 
> combination of 64-bit time with 32-bit offsets, and don't want to add 
> them).  That should apply for RISC-V 32-bit at least.
> 
> I've filed  for 
> missing overflow checks in telldir when the default off_t is wider than 
> long int (currently just applies to x32; not sure why we don't see glibc 
> test failures on x32 resulting from the quiet truncation, as the issue is 
> certainly there in the source code).
> 

What about csky? Should we still make it use 32-bit offsets as default
configuration even when kernel does not support it natively?

Re: [Qemu-devel] [RFC v2 2/4] elf-ops.h: Add get_elf_note_type()

[Stefan Hajnoczi]

On Fri, Dec 21, 2018 at 08:03:50PM +, Liam Merwick wrote:
> +while (note_type != elf_note_type) {
> +nhdr_namesz = nhdr->n_namesz;
> +nhdr_descsz = nhdr->n_descsz;
> +
> +elf_note_entry_offset = nhdr_size +
> +QEMU_ALIGN_UP(nhdr_namesz, phdr_align) +
> +QEMU_ALIGN_UP(nhdr_descsz, phdr_align);
> +
> +/* If the offset calculated in this iteration exceeds the
> +  * supplied size, we are done and no matching note was found.
> +  */

Indentation is off here.  QEMU uses 4-space indentation.

> +if (elf_note_entry_offset > note_size) {
> +return NULL;
> +}
> +
> +/* skip to the next ELF Note entry */
> +nhdr = (void *)nhdr + elf_note_entry_offset;
> +note_type = nhdr->n_type;
> +}
> +
> +return nhdr;
> +}
> +
>  static int glue(load_elf, SZ)(const char *name, int fd,
>uint64_t (*elf_note_fn)(void *, void *, bool),
>uint64_t (*translate_fn)(void *, uint64_t),
> @@ -512,6 +555,13 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>  }
>  }
>  
> + /* Search the ELF notes to find one with a type matching the
> +  * value passed in via 'translate_opaque'
> +  */
> +nhdr = (struct elf_note *)data;

Ah, I see data gets used here!  It would be clearer to move loading of
data into this patch.

> + assert(translate_opaque != NULL);
> +nhdr = glue(get_elf_note_type, SZ)(nhdr, file_size, ph->p_align,
> +   *(uint64_t 
> *)translate_opaque);

Indentation is off in this hunk.  QEMU uses 4-space indentation.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 0/8] tests: apci: consolidate and cleanup ACPI test code

[Thomas Huth]

On 2018-12-27 15:13, Igor Mammedov wrote:
> Changes since v1:
>   * rebase on top of current master due to a lots of conflicts with
> 'qtest global' removal being merged first

Sorry for that ... I hope it was not too much trouble!

>   * drop explicit cast to uint8_t* as sdt->aml is uint8_t* now
>   * drop not comment explaining strange offsets as offsets are
> now follow ACPI spec
> 
> While working on adding tests for virt/arm board (uefi/XSDT/64-bit table 
> pointers),
> I found it's rather difficult to deal with mixed ACPI testing code that we've
> collected so far. So instead of just adding a pile of XSDT hacks on top, here
> goes small refactoring series:
>* that removes dead code
>* replaces reading tables with a fetch per table everywhere instead of
>  mix of field by field and whole table
>* consolidates the way tables are read (reduces code duplication)
>* test no longer depends on ACPI structures from QEMU (i.e. doesn't 
> affected
>  by mistakes there) 
>* fixes FACS not being compared against reference tables
> Overall test is reduced on ~160LOC and hopefully it makes easier to follow and
> add more stuff on top.
> 
> PS:
> arm/virt test patches fill follow up a separate series on top of this one
> for not to mix things up
> 
> Git tree for testing:
>   https://github.com/imammedo/qemu acpi_tests_cleanup_v2
> 
> CC: "Michael S. Tsirkin" 
> CC: Thomas Huth 

Do you want me to take this through the qtests tree, or will you take it
through your ACPI tree?

In the latter case, FWIW:
Acked-by: Thomas Huth 

Re: [Qemu-devel] [RFC v2 1/4] elf: Add optional function ptr to load_elf() to parse ELF notes

[Stefan Hajnoczi]

On Fri, Dec 21, 2018 at 08:03:49PM +, Liam Merwick wrote:
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 74679ff8da3a..37d20a3800c1 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -266,6 +266,7 @@ fail:
>  }
>  
>  static int glue(load_elf, SZ)(const char *name, int fd,
> +  uint64_t (*elf_note_fn)(void *, void *, bool),
>uint64_t (*translate_fn)(void *, uint64_t),
>void *translate_opaque,
>int must_swab, uint64_t *pentry,
> @@ -496,8 +497,30 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>  high = addr + mem_size;
>  
>  data = NULL;
> +
> +} else if (ph->p_type == PT_NOTE && elf_note_fn) {
> +struct elf_note *nhdr = NULL;
> +
> +file_size = ph->p_filesz; /* Size of the range of ELF notes */
> +data = g_malloc0(file_size);
> +if (ph->p_filesz > 0) {
> +if (lseek(fd, ph->p_offset, SEEK_SET) < 0) {
> +goto fail;
> +}
> +if (read(fd, data, file_size) != file_size) {
> +goto fail;
> +}
> +}
> +
> +if (nhdr != NULL) {
> +bool is64 =
> +sizeof(struct elf_note) == sizeof(struct elf64_note);
> +elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);

How does data get used?

> +}
> +g_free(data);

Missing data = NULL to prevent double free later?


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] MAINTAINERS: Add ARM-related files for hw/[misc|input|timer]/

[Thomas Huth]

Some of the files in hw/input/, hw/misc/ and hw/timer/ are only
used by one of the ARM machines, so we can assign these files to
the corresponding boards.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 180695f..693abdf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -510,6 +510,7 @@ F: hw/intc/arm*
 F: hw/intc/gic_internal.h
 F: hw/misc/a9scu.c
 F: hw/misc/arm11scu.c
+F: hw/misc/arm_l2x0.c
 F: hw/timer/a9gtimer*
 F: hw/timer/arm*
 F: include/hw/arm/arm*.h
@@ -582,6 +583,7 @@ L: qemu-...@nongnu.org
 S: Maintained
 F: hw/arm/integratorcp.c
 F: hw/misc/arm_integrator_debug.c
+F: include/hw/misc/arm_integrator_debug.h
 
 MCIMX6UL EVK / i.MX6ul
 M: Peter Maydell 
@@ -601,7 +603,9 @@ L: qemu-...@nongnu.org
 S: Odd Fixes
 F: hw/arm/mcimx7d-sabre.c
 F: hw/arm/fsl-imx7.c
+F: hw/misc/imx7_*.c
 F: include/hw/arm/fsl-imx7.h
+F: include/hw/misc/imx7_*.h
 F: hw/pci-host/designware.c
 F: include/hw/pci-host/designware.h
 
@@ -635,6 +639,10 @@ M: Peter Maydell 
 L: qemu-...@nongnu.org
 S: Odd Fixes
 F: hw/arm/nseries.c
+F: hw/input/lm832x.c
+F: hw/input/tsc2005.c
+F: hw/misc/cbus.c
+F: hw/timer/twl92230.c
 
 Palm
 M: Andrzej Zaborowski 
@@ -642,6 +650,7 @@ M: Peter Maydell 
 L: qemu-...@nongnu.org
 S: Odd Fixes
 F: hw/arm/palm.c
+F: hw/input/tsc210x.c
 
 Raspberry Pi
 M: Peter Maydell 
@@ -678,6 +687,7 @@ F: hw/display/tc6393xb.c
 F: hw/gpio/max7310.c
 F: hw/gpio/zaurus.c
 F: hw/misc/mst_fpga.c
+F: hw/misc/max111x.c
 F: include/hw/arm/pxa.h
 F: include/hw/arm/sharpsl.h
 
@@ -688,10 +698,10 @@ L: qemu-...@nongnu.org
 S: Odd Fixes
 F: hw/arm/sabrelite.c
 F: hw/arm/fsl-imx6.c
-F: hw/misc/imx6_src.c
+F: hw/misc/imx6_*.c
 F: hw/ssi/imx_spi.c
 F: include/hw/arm/fsl-imx6.h
-F: include/hw/misc/imx6_src.h
+F: include/hw/misc/imx6_*.h
 F: include/hw/ssi/imx_spi.h
 
 Sharp SL-5500 (Collie) PDA
@@ -802,7 +812,9 @@ R: Joel Stanley 
 L: qemu-...@nongnu.org
 S: Maintained
 F: hw/*/*aspeed*
+F: hw/misc/pca9552.c
 F: include/hw/*/*aspeed*
+F: include/hw/misc/pca9552*.h
 F: hw/net/ftgmac100.c
 F: include/hw/net/ftgmac100.h
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/8] sam460ex: Clean up SPD EEPROM creation

[BALATON Zoltan]

On Wed, 2 Jan 2019, David Gibson wrote:

On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:

+/* IIC controllers and devices */
 dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
-i2c[0] = PPC4xx_I2C(dev);
-object_property_set_bool(OBJECT(dev), true, "realized", NULL);
-smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
-g_free(smbus_eeprom_buf);
-i2c_create_slave(i2c[0]->bus, "m41t80", 0x68);
+i2c = PPC4xx_I2C(dev)->bus;
+/* SPD EEPROM on RAM module */
+spd_data = spd_data_generate(DDR2, ram_sizes[0]);
+if (spd_data) {
+spd_data[20] = 4; /* SO-DIMM module */
+smbus_eeprom_init_one(i2c, 0x50, spd_data);


Any specific reason this case is handled outside spd_data_generate()?


As explained in previous reply I tried to keep number of options to the 
function to minimum and not handle every board specific case in that 
function. This board has SO-DIMM instead of regular DIMM socket but most 
of the SPD data is the same so it's easy enough to patch it here to match 
what real hardware may have. Otherwise spd_data_generate should have a lot 
of options for all different possibilites different boards might have.


(Also this is an example that unexpected configs might still work:

$ qemu-system-ppc -M sam460ex -m 64 -serial stdio
qemu-system-ppc: warning: Memory size is too small for SDRAM type, adjusting 
type
[...]
DRAM:  64 MiB (ECC not enabled, 460 MHz, CL0)

This SoC has a DRAM controller which accepts both DDR and DDR2 and the 
firmware does not care either so it works even if this cannot happen on 
real hardware. The spd_data[20] is different for DDR2 and DDR/SDR but in 
the latter it's ~WE latency which does not matter either so it can be set 
unconditionally. If this was not working the board code could check 
spd_data[2] != DDR2 and then abort itself.)


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH v2 3/3] dmg: don't skip zero chunk

[Stefan Hajnoczi]

On Sun, Dec 23, 2018 at 10:59:39AM +0800, yuchenlin wrote:
> The dmg file has many tables which describe: "start from sector XXX to
> sector XXX, the compression method is XXX and where the compressed data
> resides on".
> 
> Each sector in the expanded file should be covered by a table. The table
> will describe the offset of compressed data (or raw depends on the type)
> in the dmg.
> 
> For example:
> 
> [---The expanded file]
> [---bzip table ---]/* zeros */[---zlib---]
> ^
> | if we want to read this sector.
> 
> we will find bzip table which contains this sector, and get the
> compressed data offset, read it from dmg, uncompress it, finally write to
> expanded file.
> 
> If we skip zero chunk (table), some sector cannot find the table which
> will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
> and finally causing dmg_co_preadv() return EIO.
> 
> See:
> 
> [---The expanded file]
> [---bzip table ---]/* zeros */[---zlib---]
> ^
> | if we want to read this sector.
> 
> Oops, we cannot find the table contains it...
> 
> In the original implementation, we don't have zero table. When we try to
> read sector inside the zero chunk. We will get EIO, and skip reading.
> 
> After this patch, we treat zero chunk the same as ignore chunk, it will
> directly write zero and avoid some sector may not find the table.
> 
> After this patch:
> 
> [---The expanded file]
> [---bzip table ---][--zeros--][---zlib---]
> 
> Signed-off-by: yuchenlin 
> ---
>  block/dmg.c | 19 ---
>  1 file changed, 12 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/8] smbus: Add a helper to generate SPD EEPROM data

[BALATON Zoltan]

On Wed, 2 Jan 2019, David Gibson wrote:

On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:

There are several boards with SPD EEPROMs that are now using
duplicated or slightly different hard coded data. Add a helper to
generate SPD data for a memory module of given type and size that
could be used by these boards (either as is or with further changes if
needed) which should help cleaning this up and avoid further duplication.

Signed-off-by: BALATON Zoltan 
---
 hw/i2c/smbus_eeprom.c  | 128 +
 include/hw/i2c/smbus.h |   3 ++
 2 files changed, 131 insertions(+)

diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
index f18aa3de35..a1f51eb921 100644
--- a/hw/i2c/smbus_eeprom.c
+++ b/hw/i2c/smbus_eeprom.c
@@ -23,6 +23,8 @@
  */

 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/units.h"
 #include "hw/hw.h"
 #include "hw/i2c/i2c.h"
 #include "hw/i2c/smbus.h"
@@ -162,3 +164,129 @@ void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom,
 smbus_eeprom_init_one(smbus, 0x50 + i, eeprom_buf + (i * 256));
 }
 }
+
+/* Generate SDRAM SPD EEPROM data describing a module of type and size */
+uint8_t *spd_data_generate(enum sdram_type type, ram_addr_t ram_size)
+{
+uint8_t *spd;
+uint8_t nbanks;
+uint16_t density;
+uint32_t size;
+int min_log2, max_log2, sz_log2;
+int i;
+
+switch (type) {
+case SDR:
+min_log2 = 2;
+max_log2 = 9;
+break;
+case DDR:
+min_log2 = 5;
+max_log2 = 12;
+break;
+case DDR2:
+min_log2 = 7;
+max_log2 = 14;
+break;
+default:
+error_report("Unknown SDRAM type");
+abort();


The error handling might work a little cleaner if you give this
function an Error ** parameter, then just pass in &error_abort from
the callers.


Good idea but I'm not sure it's needed. This is the only fatal error here 
(apart from g_malloc0 failing which should also abort) and in practice 
this could only happen if a caller specifies wrong type which is quite 
unlikely given that it's an enum so value outside of that would fail to 
compile with a warning (promoted to error by default). So this default 
case is only really here to please the compiler.



+}
+size = ram_size >> 20; /* work in terms of megabytes */
+if (size < 4) {
+error_report("SDRAM size is too small");
+return NULL;
+}
+sz_log2 = 31 - clz32(size);
+size = 1U << sz_log2;
+if (ram_size > size * MiB) {
+warn_report("SDRAM size 0x"RAM_ADDR_FMT" is not a power of 2, "
+"truncating to %u MB", ram_size, size);
+}
+if (sz_log2 < min_log2) {
+warn_report("Memory size is too small for SDRAM type, adjusting type");
+if (size >= 32) {
+type = DDR;
+min_log2 = 5;
+max_log2 = 12;
+} else {
+type = SDR;
+min_log2 = 2;
+max_log2 = 9;
+}


What do these various fall back cases represent?  Are they bugs in the
callers, or a user configuration error?


The memory size is given by the user so it can be a config error (like 
when board has DDR2 but user sets memory size to 64MB).



If the first, we should just assert or abort.  If the second I think
we should still die with a fatal error - allowing broken
configurations to work with just a warning is kind of begging to
maintain nasty compatiliby cruft in the future.


I'd leave that to the caller to decide and not abort in this function. It 
will warn user that config is unexpected for the board but does not 
prevent it and try to give something that might still work (e.g. DDR 
instead of DDR2). Then the caller can check the returned data and abort if 
it insists that only DDR2 will work for the machine. Otherwise we'd make 
it impossible to use non-standard memory sizes for cases when it would 
still work (like when the OS does not check SPD EEPROMs and would happily 
use whatever memory size). I think it's already possible to start machines 
with odd memory sizes so that's not new and I did not want to prevent this 
if SPD EEPROMs are added (just SPD can't describe all memory in that 
case which may only be a problem for firmware but not when directly 
booting a kernel for example).


The idea of this function is to generate some data to start from instead 
of the static data some boards now have. which is often sufficient for the 
board as is but it could be checked or modified further to fit the 
specific needs of the board. As those needs can be widely different I did 
not attempt to handle all of them in this function to keep it generic.


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH v2 1/3] dmg: fix binary search

[林育辰]

Hi, Stefan

Thank you for your reviewing.

This series is focus on fixing bug #1809304 (see:
https://bugs.launchpad.net/qemu/+bug/1809304).
There is an example dmg file in #1809304 which will trigger this bug.

About your case, I think we can simply check whether chunk3 is zero before
we decrease it.
if s->sectors[chunk3] > sector_num and chunk3 is zero (i.e. s->sectors[0] >
sector_num), it means we cannot find the table contains sector_num.
We can return s->n_chunks (error) directly.

What do you think?

Thanks,
Yu-Chen


Stefan Hajnoczi  於 2019年1月2日 週三 下午7:49寫道：

> On Sun, Dec 23, 2018 at 10:59:37AM +0800, yuchenlin wrote:
> > There is a possible hang in original binary search implementation. That
> is
> > if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
> >
> > The chunk1 will be still 4, and so on.
> >
> > Signed-off-by: yuchenlin 
> > ---
> >  block/dmg.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/block/dmg.c b/block/dmg.c
> > index 50e91aef6d..0e05702f5d 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -572,14 +572,14 @@ static inline uint32_t search_chunk(BDRVDMGState
> *s, uint64_t sector_num)
> >  {
> >  /* binary search */
> >  uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
> > -while (chunk1 != chunk2) {
> > +while (chunk1 <= chunk2) {
> >  chunk3 = (chunk1 + chunk2) / 2;
> >  if (s->sectors[chunk3] > sector_num) {
> > -chunk2 = chunk3;
> > +chunk2 = chunk3 - 1;
>
> Question from the previous email you sent:
>
> What happens when chunk1 = 0, chunk2 = 1, and chunk3 = 0?  This would
> cause out-of-bounds sectors[] accesses.
>
> Stefan
>

Re: [Qemu-devel] [PATCH v4 5/5] migration: Use strnlen() for fixed-size string

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (phi...@redhat.com) wrote:
> GCC 8 introduced the -Wstringop-overflow, which detect buffer overflow
> by string-modifying functions declared in , such strncpy(),
> used in global_state_store_running().
> 
> GCC indeed found an incorrect use of strlen(), because this array
> is loaded by VMSTATE_BUFFER(runstate, GlobalState) then parsed
> using qapi_enum_parse which does not get the buffer length.
> 
> Use strnlen() which returns sizeof(s->runstate) if the array is not
> NUL-terminated, assert the size is within range, and enforce the array
> to be NUL-terminated to avoid an overflow in qapi_enum_parse().
> 
> This fixes:
> 
> CC  migration/global_state.o
>   qemu/migration/global_state.c: In function 'global_state_pre_save':
>   qemu/migration/global_state.c:109:15: error: 'strlen' argument 1 declared 
> attribute 'nonstring' [-Werror=stringop-overflow=]
>s->size = strlen((char *)s->runstate) + 1;
>  ^~~
>   qemu/migration/global_state.c:24:13: note: argument 'runstate' declared here
>uint8_t runstate[100] QEMU_NONSTRING;
>^~~~
>   cc1: all warnings being treated as errors
>   make: *** [qemu/rules.mak:69: migration/global_state.o] Error 1
> 
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/global_state.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 01805c567a..4f060a6dbd 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -89,6 +89,16 @@ static int global_state_post_load(void *opaque, int 
> version_id)
>  s->received = true;
>  trace_migrate_global_state_post_load(runstate);
>  
> +if (strnlen((char *)s->runstate,
> +sizeof(s->runstate)) == sizeof(s->runstate)) {
> +/* This condition should never happen during migration, because
> + * all runstate names are shorter than 100 bytes (the size of
> + * s->runstate). However, a malicious stream could overflow
> + * the qapi_enum_parse() call, so we force the last character
> + * to a NUL byte.
> + */
> +s->runstate[sizeof(s->runstate) - 1] = '\0';
> +}
>  r = qapi_enum_parse(&RunState_lookup, runstate, -1, &local_err);
>  
>  if (r == -1) {
> @@ -107,7 +117,8 @@ static int global_state_pre_save(void *opaque)
>  GlobalState *s = opaque;
>  
>  trace_migrate_global_state_pre_save((char *)s->runstate);
> -s->size = strlen((char *)s->runstate) + 1;
> +s->size = strnlen((char *)s->runstate, sizeof(s->runstate)) + 1;
> +assert(s->size <= sizeof(s->runstate));
>  
>  return 0;
>  }
> -- 
> 2.17.2
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/3] dmg: use enumeration type instead of hard coding number

[Stefan Hajnoczi]

On Sun, Dec 23, 2018 at 10:59:38AM +0800, yuchenlin wrote:
> Signed-off-by: yuchenlin 
> ---
>  block/dmg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 1/3] dmg: fix binary search

[Stefan Hajnoczi]

On Sun, Dec 23, 2018 at 10:59:37AM +0800, yuchenlin wrote:
> There is a possible hang in original binary search implementation. That is
> if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
> 
> The chunk1 will be still 4, and so on.
> 
> Signed-off-by: yuchenlin 
> ---
>  block/dmg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/dmg.c b/block/dmg.c
> index 50e91aef6d..0e05702f5d 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -572,14 +572,14 @@ static inline uint32_t search_chunk(BDRVDMGState *s, 
> uint64_t sector_num)
>  {
>  /* binary search */
>  uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
> -while (chunk1 != chunk2) {
> +while (chunk1 <= chunk2) {
>  chunk3 = (chunk1 + chunk2) / 2;
>  if (s->sectors[chunk3] > sector_num) {
> -chunk2 = chunk3;
> +chunk2 = chunk3 - 1;

Question from the previous email you sent:

What happens when chunk1 = 0, chunk2 = 1, and chunk3 = 0?  This would
cause out-of-bounds sectors[] accesses.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 4/5] migration: Use QEMU_NONSTRING for non NUL-terminated arrays

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (phi...@redhat.com) wrote:
> GCC 8 added a -Wstringop-truncation warning:
> 
>   The -Wstringop-truncation warning added in GCC 8.0 via r254630 for
>   bug 81117 is specifically intended to highlight likely unintended
>   uses of the strncpy function that truncate the terminating NUL
>   character from the source string.
> 
> This new warning leads to compilation failures:
> 
> CC  migration/global_state.o
>   qemu/migration/global_state.c: In function 'global_state_store_running':
>   qemu/migration/global_state.c:45:5: error: 'strncpy' specified bound 100 
> equals destination size [-Werror=stringop-truncation]
>strncpy((char *)global_state.runstate, state, 
> sizeof(global_state.runstate));
>
> ^~~~
>   make: *** [qemu/rules.mak:69: migration/global_state.o] Error 1
> 
> Use the QEMU_NONSTRING attribute, since this array is intended to store
> character arrays that do not necessarily contain a terminating NUL.
> 
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  migration/global_state.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 8e8ab5c51e..6e19333422 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -21,7 +21,7 @@
>  
>  typedef struct {
>  uint32_t size;
> -uint8_t runstate[100];
> +uint8_t runstate[100] QEMU_NONSTRING;

Hmm; global_state_post_load needs to be fixed for this;  it
uses s->runsate and ends up passing it to both a trace
and a qapi_enum_parse - so it's really treating it as a string.
That code is unsafe anyway since it's assuming the received
runstate would be terminated.

Dave

>  RunState state;
>  bool received;
>  } GlobalState;
> -- 
> 2.17.2
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] About live migration rollback

[Dr. David Alan Gilbert]

* Gonglei (Arei) (arei.gong...@huawei.com) wrote:
> Hi Dave,
> 
> We discussed some live migration fallback scenarios in this year's KVM forum, 
> and now I can provide another scenario, perhaps the upstream should consider 
> rolling
> back for this situation.
> 
> Environments information:
> 
> host A: cpu E5620(model WestmereEP without flag xsave)
> host B: cpu E5-2643(model SandyBridgeEP with flag xsave)
> 
> The reproduce steps is :
> 1. Start a windows 2008 vm with -cpu host(which means host-passthrough).

Well we don't guarantee migration across -cpu host - does this problem
go away if both qemu's are started with matching CPU flags
(corresponding to the Westmere) ?

> 2. Migrate the vm to host B when cr4.OSXSAVE=0.
> 3. Vm runs on host B for a while so that cr4.OSXSAVE changes to 1.
> 4. Then migrate the vm to host A successfully, but vm was paused, and qemu 
> printed log as followed:
> 
> KVM: entry failed, hardware error 0x8021
> 
> If you're running a guest on an Intel machine without unrestricted mode
> support, the failure can be most likely due to the guest entering an invalid
> state for Intel VT. For example, the guest maybe running in big real mode
> which is not supported on less recent Intel processors.
> 
> EAX=019b3bb0 EBX=01a3ae80 ECX=01a61ce8 EDX=
> ESI=01a62000 EDI= EBP= ESP=01718b20
> EIP=0185d982 EFL=0286 [--S--P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =   9300
> CS =f000   9b00
> SS =   9300
> DS =   9300
> FS =   9300
> GS =   9300
> LDT=   8200
> TR =   8b00
> GDT=  
> IDT=  
> CR0=6010 CR2= CR3= CR4=
> DR0= DR1= DR2= 
> DR3=
> DR6=0ff0 DR7=0400
> EFER=
> Code=00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <00> 00 00 
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 00
> 
> Problem happened when kvm_put_sregs returns err -22(called by 
> kvm_arch_put_registers(qemu)).
> 
> Because kvm_arch_vcpu_ioctl_set_sregs(kvm module) checked that 
> guest_cpuid_has no X86_FEATURE_XSAVE but cr4.OSXSAVE=1.
> We should cancel migration if kvm_arch_put_registers returns error.

Do you have a backtrace of when the kvm_arch_put_registers is called
when it fails?
If it's called during the loading of the device state then we should be
able to detect it and fail the migration; however if it's only failing
after the CPU is restarted after the migration then it's a bit too late.

Dave

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

Re: [Qemu-devel] [PATCH] gitdm: Add other IBMers

[Andrew Jeffery]

On Wed, 2 Jan 2019, at 21:36, Joel Stanley wrote:
> Here are some IBMers who use their personal addresses when submitting
> patches.
> 
> Signed-off-by: Joel Stanley 

For the addition of my address:

Acked-by: Andrew Jeffery 

> ---
>  contrib/gitdm/group-map-ibm | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/contrib/gitdm/group-map-ibm b/contrib/gitdm/group-map-ibm
> index b66db5f4a825..6c0570107d65 100644
> --- a/contrib/gitdm/group-map-ibm
> +++ b/contrib/gitdm/group-map-ibm
> @@ -2,5 +2,10 @@
>  # Some IBM contributors submit via another domain
>  #
>  
> +a...@ozlabs.ru
> +and...@aj.id.au
> +b...@kernel.crashing.org
>  c...@kaod.org
>  gr...@kaod.org
> +j...@jms.id.au
> +sjitindarsi...@gmail.com
> -- 
> 2.19.1
> 

[Qemu-devel] [Bug 1810000] Re: qemu system emulator crashed with the attachment of usb-bt-dongle device

[Thomas Huth]

Thanks for the bug report! Unfortunately, the bluetooth code in QEMU is
completely unmaintained and quite bit-rotten already, so it is already
marked as deprecated and will be removed soon unless somebody steps up
and volunteers to maintain it:

 https://qemu.weilnetz.de/doc/qemu-doc.html#bluetooth-_0028since-
3_002e1_0029

For the time being, you likely have to use "-device pci-ohci" instead of
"-device qemu-xhci" if you want to try to use it...

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

Title:
  qemu system emulator crashed with the attachment of usb-bt-dongle
  device

Status in QEMU:
  New

Bug description:
  I am testing usb-bt-dongle device on xchi host controller, and found
  that the qemu crashed directly with an assertion failer.

  Here is the information to reproduce the crash:

  Qemu git revision: 9b2e891ec5ccdb4a7d583b77988848282606fdea
  System emulator: qemu-x86_64
  VM image: 
https://people.debian.org/~aurel32/qemu/amd64/debian_squeeze_amd64_desktop.qcow2
  CommandLine: qemu-system-x86_64 -M q35 -device qemu-xhci,id=xhci -enable-kvm 
-device usb-bt-dongle  -hda ./debian_wheezy_amd64_standard.qcow2

  Error message:

  qemu-system-x86_64: /build/qemu-
  Eap4uc/qemu-2.11+dfsg/hw/usb/core.c:592: usb_packet_copy: Assertion
  `p->actual_length + bytes <= iov->size' failed.

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

[Qemu-devel] [PATCH 1/1] include: Auto-generate the sizes lookup table

[Leonid Bloch]

The lookup table for power-of-two sizes is now auto-generated during the
build, and not hard-coded into the units.h file.

Signed-off-by: Leonid Bloch 
---
 .gitignore   |  1 +
 Makefile |  5 +++
 block/qcow2.h|  2 +-
 block/vdi.c  |  1 +
 include/qemu/units.h | 73 
 scripts/gen-sizes.sh | 66 +++
 6 files changed, 74 insertions(+), 74 deletions(-)
 create mode 100755 scripts/gen-sizes.sh

diff --git a/.gitignore b/.gitignore
index 0430257313..5945220303 100644
--- a/.gitignore
+++ b/.gitignore
@@ -59,6 +59,7 @@
 /qemu-version.h
 /qemu-version.h.tmp
 /module_block.h
+/include/qemu/sizes.h
 /scsi/qemu-pr-helper
 /vhost-user-scsi
 /vhost-user-blk
diff --git a/Makefile b/Makefile
index dd53965f77..435d92821b 100644
--- a/Makefile
+++ b/Makefile
@@ -122,6 +122,8 @@ endif
 
 GENERATED_FILES += module_block.h
 
+GENERATED_FILES += $(SRC_PATH)/include/qemu/sizes.h
+
 TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
 TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
 TRACE_DTRACE =
@@ -499,6 +501,9 @@ ifdef CONFIG_MPATH
 scsi/qemu-pr-helper$(EXESUF): LIBS += -ludev -lmultipath -lmpathpersist
 endif
 
+$(SRC_PATH)/include/qemu/sizes.h: $(SRC_PATH)/scripts/gen-sizes.sh
+   $(call quiet-command,sh $(SRC_PATH)/scripts/gen-sizes.sh $@,"GEN","$@")
+
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > 
$@,"GEN","$@")
 
diff --git a/block/qcow2.h b/block/qcow2.h
index a98d24500b..41d14baa17 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -27,7 +27,7 @@
 
 #include "crypto/block.h"
 #include "qemu/coroutine.h"
-#include "qemu/units.h"
+#include "qemu/sizes.h"
 
 //#define DEBUG_ALLOC
 //#define DEBUG_ALLOC2
diff --git a/block/vdi.c b/block/vdi.c
index 2380daa583..62dafe0cc6 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -51,6 +51,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
+#include "qemu/sizes.h"
 #include "qapi/error.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-visit-block-core.h"
diff --git a/include/qemu/units.h b/include/qemu/units.h
index 1c959d182e..692db3fbb2 100644
--- a/include/qemu/units.h
+++ b/include/qemu/units.h
@@ -17,77 +17,4 @@
 #define PiB (INT64_C(1) << 50)
 #define EiB (INT64_C(1) << 60)
 
-/*
- * The following lookup table is intended to be used when a literal string of
- * the number of bytes is required (for example if it needs to be stringified).
- * It can also be used for generic shortcuts of power-of-two sizes.
- * This table is generated using the AWK script below:
- *
- *  BEGIN {
- *  suffix="KMGTPE";
- *  for(i=10; i<64; i++) {
- *  val=2**i;
- *  s=substr(suffix, int(i/10), 1);
- *  n=2**(i%10);
- *  pad=21-int(log(n)/log(10));
- *  printf("#define S_%d%siB %*d\n", n, s, pad, val);
- *  }
- *  }
- */
-
-#define S_1KiB  1024
-#define S_2KiB  2048
-#define S_4KiB  4096
-#define S_8KiB  8192
-#define S_16KiB16384
-#define S_32KiB32768
-#define S_64KiB65536
-#define S_128KiB  131072
-#define S_256KiB  262144
-#define S_512KiB  524288
-#define S_1MiB   1048576
-#define S_2MiB   2097152
-#define S_4MiB   4194304
-#define S_8MiB   8388608
-#define S_16MiB 16777216
-#define S_32MiB 33554432
-#define S_64MiB 67108864
-#define S_128MiB   134217728
-#define S_256MiB   268435456
-#define S_512MiB   536870912
-#define S_1GiB1073741824
-#define S_2GiB2147483648
-#define S_4GiB4294967296
-#define S_8GiB8589934592
-#define S_16GiB  17179869184
-#define S_32GiB  34359738368
-#define S_64GiB  68719476736
-#define S_128GiB137438953472
-#define S_256GiB274877906944
-#define S_512GiB549755813888
-#define S_1TiB 1099511627776
-#define S_2TiB 219902322
-#define S_4TiB 4398046511104
-#define S_8TiB 8796093022208
-#define S_16TiB   17592186044416
-#define S_32TiB   35184372088832
-#define S_64TiB   70368744177664
-#define S_128TiB 140737488355328
-#define S_256TiB 281474976710656
-#define S_512TiB 562949953421312
-#define S_1PiB  1125899906842624
-#define S_2PiB  2251799813685248
-#define S_4PiB  4503599627370496
-#define S_8PiB  9007199254740992
-#define S_16PiB18014398509481984
-#define S_32PiB36028797018963968
-#define S_64PiB72057594037927936
-#define S_128PiB  144115188075855872
-#define S_256PiB  288230376151711744
-#define S_512PiB  576460752303423488
-#define S_1EiB   1152921504606846976
-#define S_2EiB   2305843009213693952
-#define S_4EiB   4611

[Qemu-devel] [PATCH 0/1] include: Auto-generate the sizes lookup table

[Leonid Bloch]

Following the conversations here:
https://patchwork.kernel.org/patch/10665157
and here:
https://patchwork.kernel.org/patch/10666975

Making the lookup table for power-of-two sizes auto-generated, instead
of being hard-coded into the units.h file.

I'm not sure if the changes I've made to Makefile here are "standard".
Please correct me if that's not the case.

Sorry it took so much time - I was busy with something completely
different.


Regards,
Leonid.

Leonid Bloch (1):
  include: Auto-generate the sizes lookup table

 .gitignore   |  1 +
 Makefile |  5 +++
 block/qcow2.h|  2 +-
 block/vdi.c  |  1 +
 include/qemu/units.h | 73 
 scripts/gen-sizes.sh | 66 +++
 6 files changed, 74 insertions(+), 74 deletions(-)
 create mode 100755 scripts/gen-sizes.sh

-- 
2.20.1

[Qemu-devel] [PATCH] gitdm: Add other IBMers

[Joel Stanley]

Here are some IBMers who use their personal addresses when submitting
patches.

Signed-off-by: Joel Stanley 
---
 contrib/gitdm/group-map-ibm | 5 +
 1 file changed, 5 insertions(+)

diff --git a/contrib/gitdm/group-map-ibm b/contrib/gitdm/group-map-ibm
index b66db5f4a825..6c0570107d65 100644
--- a/contrib/gitdm/group-map-ibm
+++ b/contrib/gitdm/group-map-ibm
@@ -2,5 +2,10 @@
 # Some IBM contributors submit via another domain
 #
 
+a...@ozlabs.ru
+and...@aj.id.au
+b...@kernel.crashing.org
 c...@kaod.org
 gr...@kaod.org
+j...@jms.id.au
+sjitindarsi...@gmail.com
-- 
2.19.1

[Qemu-devel] [Bug 1793275] Re: Hosts fail to start after update to QEMU 3.0

[Thomas Huth]

Ok, thanks for the update. Closing this bug since it seems to be fixed
in 3.1.

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

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

Title:
  Hosts fail to start after update to QEMU 3.0

Status in QEMU:
  Fix Released

Bug description:
  Host OS: Archlinux
  Host Architecture: AMD64
  Guest OS: FreeBSD-11.2 (x2) and Archlinux (x1)
  Guest Architecture: AMD64

  I have been using QEMU 2.x without issue for a number of years but
  since updating to QEMU 3.0 my guests do not complete startup.

  FreeBSD 11.2 guest failure symptom:
  The two FreeBSD-11.2 guests output repeated messages of "unexpected cache 
type 4". This appears to be an internal error message and I've not found any 
instances of it through Google search.

  Archlinux guest failure symptom:
  The single Archlinux guest gets no further than the message "uncompressing 
initial ramdisk".

  The guests are started by a qemu-kvm invokation. No virtual machine
  managers are used. The command lines used (from ps awx) to launch the
  VMs are:

  [neil@optimus ~]$ ps awx |grep qemu
   1492 ?Sl 3:19 /usr/bin/qemu-system-x86_64 -daemonize -pidfile 
/run/qemu_vps1.pid -enable-kvm -cpu host -smp 2 -k en-gb -boot order=c -drive 
file=/dev/system/vps1,cache=none,format=raw,if=virtio,index=0,media=disk -m 
1024 -name FreeBSD_1 -net nic,macaddr=52:54:AD:86:64:00,model=virtio -net 
vde,sock=/run/vde_switch-tap0.sock -monitor telnet:127.0.0.2:23,server,nowait 
-vnc 192.168.0.1:0
   1510 ?Sl 0:54 /usr/bin/qemu-system-x86_64 -daemonize -pidfile 
/run/qemu_vps2.pid -enable-kvm -cpu host -smp 2 -k en-gb -boot order=c -drive 
file=/dev/system/vps2,cache=none,format=raw,if=virtio,index=0,media=disk -m 
1024 -name Archlinux -net nic,macaddr=52:54:AD:86:64:01,model=virtio -net 
vde,sock=/run/vde_switch-tap0.sock -monitor telnet:127.0.0.3:23,server,nowait 
-vnc 192.168.0.1:1
   1529 ?Sl 3:07 /usr/bin/qemu-system-x86_64 -daemonize -pidfile 
/run/qemu_vps3.pid -enable-kvm -cpu host -smp 2 -k en-gb -boot order=c -drive 
file=/dev/system/vps3,cache=none,format=raw,if=virtio,index=0,media=disk -m 
1024 -name FreeBSD_2 -net nic,macaddr=52:54:AD:86:64:02,model=virtio -net 
vde,sock=/run/vde_switch-tap0.sock -monitor telnet:127.0.0.4:23,server,nowait 
-vnc 192.168.0.1:2

  The VMs were installed to LVM volumes on the host machine (hence the
  /dev/system/vpsN device names). Networking is over a Linux tap
  interface connected to a VDE2 virtual network switch.

  Currently working version of QEMU: qemu-headless 2.12.1-1
  Failing version of QEMU: qemu-headless-3.0.0-1

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

[Qemu-devel] [Bug 1809665] Re: Xbox One controller USB passthrough disconnections and stops

[Thomas Huth]

Do you get a different behavior if you use "-device usb-ehci" or
"-device nec-usb-xhci" instead of the "-usb" parameter?

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

Title:
  Xbox One controller USB passthrough disconnections and stops

Status in QEMU:
  New

Bug description:
  I can't properly passthrough my Xbox One controller to a virtual
  machine; it causes USB disconnections on the host, ultimately
  preventing it to work (at all) on the guest

  I've seen a few other cases reported in other websites, which show the
  same symptoms:

  - https://www.reddit.com/r/VFIO/comments/97dhbw/qemu_w10_xbox_one_controller
  - 
https://unix.stackexchange.com/questions/452751/how-can-i-pass-through-an-xbox-one-controller-to-a-windows-vm-on-ubuntu

  This is sample:

  libusb: error [udev_hotplug_event] ignoring udev action bind
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  qemu-system-x86_64: libusb_release_interface: -4 [NO_DEVICE]
  libusb: error [_get_usbfs_fd] File doesn't exist, wait 10 ms and try again
  libusb: error [_get_usbfs_fd] libusb couldn't open USB device
  /dev/bus/usb/003/016: No such file or directory

  I think this is a quite long-standing issue, as I've been experiencing
  through several versions, including the current one (3.1).

  I can reproduce this 100% of the times, on multiple host O/S
  distributions (the current one being based on Ubuntu 18.04 x86-64).

  I compile QEMU directly from source, and execute it via commandline;
  the command is very long, however, the relevant part is standard (I
  think):

  -usb \
  -device usb-tablet \
  -device 
usb-host,vendorid=0x$VGAPT_XBOX_PAD_VEND_ID,productid=0x$VGAPT_XBOX_PAD_PROD_ID 
\

  The guest is Windows 10 64bit.

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

Re: [Qemu-devel] [PATCH] MAINTAINERS: add qemu_vga.ndrv file entry for Mac machines

[Philippe Mathieu-Daudé]

On 1/2/19 11:22 AM, Mark Cave-Ayland wrote:
> The VGA driver built from Ben's QemuMacDrivers repository is used exclusively
> by the Mac machines.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 344cb2d7fd..3da3d103b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -965,6 +965,7 @@ F: include/hw/misc/macio/
>  F: include/hw/misc/mos6522.h
>  F: include/hw/ppc/mac_dbdma.h
>  F: include/hw/pci-host/uninorth.h
> +F: pc-bios/qemu_vga.ndrv
>  
>  Old World
>  M: Mark Cave-Ayland 
> @@ -976,6 +977,7 @@ F: hw/pci-host/grackle.c
>  F: hw/misc/macio/
>  F: hw/intc/heathrow_pic.c
>  F: include/hw/intc/heathrow_pic.h
> +F: pc-bios/qemu_vga.ndrv
>  
>  PReP
>  M: Hervé Poussineau 
> 

Re: [Qemu-devel] [PATCH] qom: cpu: destroy work_mutex in cpu_common_finalize

[Philippe Mathieu-Daudé]

On 1/2/19 8:41 AM, Li Qiang wrote:
> Commit 376692b9dc6(cpus: protect work list with work_mutex)
> initialize a work_mutex in cpu_common_initfn, however forget
> to destroy it. This will cause resource leak when hotunplug cpu
> or hotplug cpu fails.
> 

Long standing leak ;)

Maybe add:

Fixes: 376692b9dc6

> Signed-off-by: Li Qiang 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  qom/cpu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 9ad1372d57..367ebf9d61 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -380,6 +380,9 @@ static void cpu_common_initfn(Object *obj)
>  
>  static void cpu_common_finalize(Object *obj)
>  {
> +CPUState *cpu = CPU(obj);
> +
> +qemu_mutex_destroy(&cpu->work_mutex);
>  }
>  
>  static int64_t cpu_common_get_arch_id(CPUState *cpu)
> 

Re: [Qemu-devel] [PATCH for-4.0 v9 10/16] qemu_thread: supplement error handling for h_resize_hpt_prepare

[李菲]

在 2019/1/2 上午10:36, David Gibson 写道:

On Tue, Dec 25, 2018 at 10:04:43PM +0800, Fei Li wrote:

Add a local_err to hold the error, and return the corresponding
error code to replace the temporary &error_abort.

Cc: Markus Armbruster 
Cc: David Gibson 
Signed-off-by: Fei Li 

This looks like a good change, but it no longer applies due to a
change in the qemu_thread_create() signature.

Sorry that I am not sure whether I understand. Do you mean using
&error_abort is more suitable for this handling, rather than report
the &local_err & return a failure reason?



---
  hw/ppc/spapr_hcall.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 5bc2cf4540..7c16ade04a 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -478,6 +478,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
  sPAPRPendingHPT *pending = spapr->pending_hpt;
  uint64_t current_ram_size;
  int rc;
+Error *local_err = NULL;
  
  if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {

  return H_AUTHORITY;
@@ -538,10 +539,13 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
  pending->shift = shift;
  pending->ret = H_HARDWARE;
  
-/* TODO: let the further caller handle the error instead of abort() here */

-qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
-   hpt_prepare_thread, pending,
-   QEMU_THREAD_DETACHED, &error_abort);
+if (!qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
+hpt_prepare_thread, pending,
+QEMU_THREAD_DETACHED, &local_err)) {
+error_reportf_err(local_err, "failed to create hpt_prepare_thread: ");
+g_free(pending);
+return H_RESOURCE;

I also think H_HARDWARE would be a better choice here.  Although the
failure is due to a resource constraint, it's not because the guest
asked for too much, just because the host is in dire straits.  From
the guest's point of view it's basically a hardware failure.


Ok, thanks. Will use H_HARDWARE instead.

Have a nice day, thanks for the review. :)
Fei



+}
  
  spapr->pending_hpt = pending;
  

Re: [Qemu-devel] Patchew down?

[Philippe Mathieu-Daudé]

On 1/2/19 2:17 AM, Fam Zheng wrote:
>> On Dec 30, 2018, at 08:09, Philippe Mathieu-Daudé  wrote:
>>
>> I think patchew is having some trouble since at least 1 week:
>> new series aren't added.
> 
> Thanks for the reminder, Phil. I fixed https on patches.org but forgot to 
> switch the importer back from http. Now it’s back to work.

The patches queue isn't even lost, awesome!

Thanks :)

Phil.

Re: [Qemu-devel] [PATCH] dmg: fix binary search

[Stefan Hajnoczi]

On Fri, Dec 21, 2018 at 09:58:03PM +0800, yuchenlin wrote:
> There is a possible hang in original binary searsh implemtation. That is
> if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
> 
> The chunk1 will be still 4, and so on.
> 
> Signed-off-by: yuchenlin 
> ---
>  block/dmg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Do you have an example dmg file that triggers this bug?

> diff --git a/block/dmg.c b/block/dmg.c
> index 50e91aef6d..0e05702f5d 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -572,14 +572,14 @@ static inline uint32_t search_chunk(BDRVDMGState *s, 
> uint64_t sector_num)
>  {
>  /* binary search */
>  uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
> -while (chunk1 != chunk2) {
> +while (chunk1 <= chunk2) {
>  chunk3 = (chunk1 + chunk2) / 2;
>  if (s->sectors[chunk3] > sector_num) {
> -chunk2 = chunk3;
> +chunk2 = chunk3 - 1;

What if chunk1 = 0 and chunk2 = 1?  chunk3 = 0 and the new chunk2 value
would be 0x so the loop continues with out-of-bounds sectors[]
accesses!


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] MAINTAINERS: add qemu_vga.ndrv file entry for Mac machines

[Mark Cave-Ayland]

The VGA driver built from Ben's QemuMacDrivers repository is used exclusively
by the Mac machines.

Signed-off-by: Mark Cave-Ayland 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 344cb2d7fd..3da3d103b1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -965,6 +965,7 @@ F: include/hw/misc/macio/
 F: include/hw/misc/mos6522.h
 F: include/hw/ppc/mac_dbdma.h
 F: include/hw/pci-host/uninorth.h
+F: pc-bios/qemu_vga.ndrv
 
 Old World
 M: Mark Cave-Ayland 
@@ -976,6 +977,7 @@ F: hw/pci-host/grackle.c
 F: hw/misc/macio/
 F: hw/intc/heathrow_pic.c
 F: include/hw/intc/heathrow_pic.h
+F: pc-bios/qemu_vga.ndrv
 
 PReP
 M: Hervé Poussineau 
-- 
2.11.0

Re: [Qemu-devel] [PATCH v3 09/16] virtio: split vhost scsi bits from virtio-pci

[Stefan Hajnoczi]

On Fri, Dec 14, 2018 at 10:19:38AM +0100, Thomas Huth wrote:
> On 2018-12-13 22:00, Juan Quintela wrote:
> > Reviewed-by: Laurent Vivier 
> > Signed-off-by: Juan Quintela 
> > ---
> >  hw/virtio/Makefile.objs|  1 +
> >  hw/virtio/vhost-scsi-pci.c | 95 ++
> >  hw/virtio/virtio-pci.c | 59 ---
> >  hw/virtio/virtio-pci.h | 19 
> >  4 files changed, 96 insertions(+), 78 deletions(-)
> >  create mode 100644 hw/virtio/vhost-scsi-pci.c
> > 
> > diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> > index f851a6f2b5..0a56946c1a 100644
> > --- a/hw/virtio/Makefile.objs
> > +++ b/hw/virtio/Makefile.objs
> > @@ -15,6 +15,7 @@ ifeq ($(CONFIG_PCI),y)
> >  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock-pci.o
> >  obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk-pci.o
> >  obj-$(CONFIG_VHOST_USER_SCSI) += vhost-user-scsi-pci.o
> > +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi-pci.o
> >  obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
> >  obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
> >  obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
> > diff --git a/hw/virtio/vhost-scsi-pci.c b/hw/virtio/vhost-scsi-pci.c
> > new file mode 100644
> > index 00..2be2c567ea
> > --- /dev/null
> > +++ b/hw/virtio/vhost-scsi-pci.c
> > @@ -0,0 +1,95 @@
> > +/*
> > + * Vhost scsi PCI bindings
> > + *
> > + * Copyright IBM, Corp. 2011
> > + *
> > + * Authors:
> > + *  Stefan Hajnoczi   
> > + *
> > + * Changes for QEMU mainline + tcm_vhost kernel upstream:
> > + *  Nicholas Bellinger 
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2 or 
> > later.
> > + * See the COPYING.LIB file in the top-level directory.
> 
> Let's double-check: Stefan, Nicholas, should this file be LGPLv2.1+
> (like hw/scsi/vhost-scsi.c) or rather GPLv2+ (like hw/virtio/virtio-pci.c) ?

I had the LGPL in mind.

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v2 1/1] drive-mirror: add incremental mode

[mahaocong]

From: mahaocong 

Signed-off-by: mahaocong 
---
 block/mirror.c| 55 ---
 blockdev.c| 37 +--
 include/block/block_int.h |  3 ++-
 qapi/block-core.json  |  9 +++-
 4 files changed, 88 insertions(+), 16 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index ab59ad77e8..1a16a94bea 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -50,6 +50,7 @@ typedef struct MirrorBlockJob {
 /* Used to block operations on the drive-mirror-replace target */
 Error *replace_blocker;
 bool is_none_mode;
+BdrvDirtyBitmap *src_bitmap;
 BlockMirrorBackingMode backing_mode;
 MirrorCopyMode copy_mode;
 BlockdevOnError on_source_error, on_target_error;
@@ -814,6 +815,27 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob 
*s)
 return 0;
 }
 
+/*
+ * init dirty bitmap by using user bitmap. usr->hbitmap will be copy to
+ * mirror bitmap->hbitmap instead of reuse it.
+ */
+static int coroutine_fn mirror_dirty_init_incremental(MirrorBlockJob *s)
+{
+BdrvDirtyBitmap *src, *dest;
+
+dest = s->dirty_bitmap;
+src = s->src_bitmap;
+if (!src) {
+return -1;
+}
+
+if (!bdrv_copy_dirty_bitmap(src, dest, NULL)) {
+return -1;
+}
+
+return 0;
+}
+
 /* Called when going out of the streaming phase to flush the bulk of the
  * data to the medium, or just before completing.
  */
@@ -913,9 +935,17 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
 
 s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 if (!s->is_none_mode) {
-ret = mirror_dirty_init(s);
-if (ret < 0 || job_is_cancelled(&s->common.job)) {
-goto immediate_exit;
+/* incremental mode */
+if (s->src_bitmap) {
+ret = mirror_dirty_init_incremental(s);
+if (ret < 0) {
+goto immediate_exit;
+}
+} else {
+ret = mirror_dirty_init(s);
+if (ret < 0 || job_is_cancelled(&s->common.job)) {
+goto immediate_exit;
+}
 }
 }
 
@@ -1484,7 +1514,8 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
  BlockCompletionFunc *cb,
  void *opaque,
  const BlockJobDriver *driver,
- bool is_none_mode, BlockDriverState *base,
+ bool is_none_mode, BdrvDirtyBitmap *src_bitmap,
+ BlockDriverState *base,
  bool auto_complete, const char *filter_node_name,
  bool is_mirror, MirrorCopyMode copy_mode,
  Error **errp)
@@ -1598,6 +1629,7 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
 s->on_source_error = on_source_error;
 s->on_target_error = on_target_error;
 s->is_none_mode = is_none_mode;
+s->src_bitmap = src_bitmap;
 s->backing_mode = backing_mode;
 s->copy_mode = copy_mode;
 s->base = base;
@@ -1664,7 +1696,8 @@ void mirror_start(const char *job_id, BlockDriverState 
*bs,
   BlockDriverState *target, const char *replaces,
   int creation_flags, int64_t speed,
   uint32_t granularity, int64_t buf_size,
-  MirrorSyncMode mode, BlockMirrorBackingMode backing_mode,
+  MirrorSyncMode mode, BdrvDirtyBitmap *src_bitmap,
+  BlockMirrorBackingMode backing_mode,
   BlockdevOnError on_source_error,
   BlockdevOnError on_target_error,
   bool unmap, const char *filter_node_name,
@@ -1673,17 +1706,14 @@ void mirror_start(const char *job_id, BlockDriverState 
*bs,
 bool is_none_mode;
 BlockDriverState *base;
 
-if (mode == MIRROR_SYNC_MODE_INCREMENTAL) {
-error_setg(errp, "Sync mode 'incremental' not supported");
-return;
-}
 is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
 base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
 mirror_start_job(job_id, bs, creation_flags, target, replaces,
  speed, granularity, buf_size, backing_mode,
  on_source_error, on_target_error, unmap, NULL, NULL,
- &mirror_job_driver, is_none_mode, base, false,
- filter_node_name, true, copy_mode, errp);
+ &mirror_job_driver, is_none_mode,
+ src_bitmap, base, false, filter_node_name, true,
+ copy_mode, errp);
 }
 
 void commit_active_start(const char *job_id, BlockDriverState *bs,
@@ -1707,7 +1737,8 @@ void commit_active_start(const char *job_id, 
BlockDriverState *bs,
 mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0,
  MIRROR_LEAVE_BACKING_CHAIN,
  o

[Qemu-devel] [PATCH v2 0/1] drive-mirror: add incremental mode

[mahaocong]

From: mahaocong 

This patch adds possibility to start mirroring with user-created-bitmap.
Compare with v1, this patch fix some shortcomings.
1.separate feature about copy dirty-bitmap to an individual patch.
2.remove checking for cancelled after mirror_dirty_init_incremental for bitmap
  copyimg don't have yield point.
3.adjuest input parameters on mirror_start_job and mirror_start, and so It is
  no need to find bitmap on mirror_dirty_init_incremental again.
4.assert the bitmap name is NULL on blockdev_mirror_common.
5.change the parameter's name in qmp command 'drive-mirror' from 'bitmap_name'
  to 'bitmap'.

As for the doubt about the new incremental mode, I think it is means that the
initial bitmap is appointed by user, compare with the full mode and top mode.
There is no different in other behaves, such as in iteration and complete
action.

mahaocong (1):
  drive-mirror: add incremental mode
  mirror: add incremental mode. we must add a user-named-bitmap first, then set
  this bitmap as the initial bitmap on incremental mode drive-mirror.
  in details, I first create unnamed-bitmap with the same granularity of
  user-bitmap. Next, copy It's hbitmap to unnamed-bitmap. Then, start
  mirror with this unname-bitmap.
  dirty-bitmap: add new API to copy dirty-bitmap. This feature is add in a
  separate patch called "add new function to copy dirty-bitmap"

 block/mirror.c| 55 ---
 blockdev.c| 37 +--
 include/block/block_int.h |  3 ++-
 qapi/block-core.json  |  9 +++-
 4 files changed, 88 insertions(+), 16 deletions(-)

-- 
2.14.1

[Qemu-devel] [PATCH v1] add new function to copy dirty-bitmap

[mahaocong]

From: mahaocong 

This patch adds new function to copy the hbitmap from an exist dirty-bitmap to
another. The destination bitmap should have the same size and granularity with
the source bitmap, or the copy will fail.

Signed-off-by: mahaocong 
---
 block/dirty-bitmap.c | 14 ++
 include/block/dirty-bitmap.h |  3 +++
 include/qemu/hbitmap.h   |  2 ++
 util/hbitmap.c   | 28 
 4 files changed, 47 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 89fd1d7f8b..4445182e2a 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -348,6 +348,20 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
 return ret;
 }
 
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BdrvDirtyBitmap *src,
+BdrvDirtyBitmap *dest,
+Error **errp)
+{
+qemu_mutex_lock(src->mutex);
+if (!hbitmap_copy(dest->bitmap, src->bitmap)) {
+error_setg(errp, "Error: copy src bitmap failed");
+return NULL;
+}
+qemu_mutex_unlock(src->mutex);
+
+return dest;
+}
+
 /**
  * Truncates _all_ bitmaps attached to a BDS.
  * Called with BQL taken.
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 8f38a3dec1..ae2e82f44b 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -21,6 +21,9 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState 
*bs,
 BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BdrvDirtyBitmap *src,
+BdrvDirtyBitmap *dest,
+Error **errp);
 void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap);
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
 const char *name);
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index a7cb780592..6de3b4eb7e 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -91,6 +91,8 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, 
HBitmap *result);
  */
 bool hbitmap_can_merge(const HBitmap *a, const HBitmap *b);
 
+bool hbitmap_copy(HBitmap *dst, const HBitmap *src);
+
 /**
  * hbitmap_empty:
  * @hb: HBitmap to operate on.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 8d402c59d9..7ae2fc270c 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -765,6 +765,34 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, 
HBitmap *result)
 return true;
 }
 
+/*
+ * Copy HBitmaps form src to dst.
+ *
+ * @return true if the copy was successful,
+ * false if it was not attempted.
+ */
+bool hbitmap_copy(HBitmap *dst, const HBitmap *src)
+{
+int i;
+
+if ((dst->size != src->size) || (dst->granularity != src->granularity)) {
+return false;
+}
+
+if (hbitmap_count(src) == 0) {
+return true;
+}
+
+for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+memcpy(dst->levels[i], src->levels[i],
+   src->sizes[i] * sizeof(unsigned long));
+}
+
+dst->count = src->count;
+return true;
+}
+
+
 HBitmap *hbitmap_create_meta(HBitmap *hb, int chunk_size)
 {
 assert(!(chunk_size & (chunk_size - 1)));
-- 
2.14.1

[Qemu-devel] [PATCH v5 8/9] target/ppc: move FP and VMX registers into aligned vsr register array

[Mark Cave-Ayland]

The VSX register array is a block of 64 128-bit registers where the first 32
registers consist of the existing 64-bit FP registers extended to 128-bit
using new VSR registers, and the last 32 registers are the VMX 128-bit
registers as show below:

64-bit   64-bit
+++
|FP0 ||  VSR0
+++
|FP1 ||  VSR1
+++
|... |... |  ...
+++
|FP30||  VSR30
+++
|FP31||  VSR31
+++
|  VMX0   |  VSR32
+-+
|  VMX1   |  VSR33
+-+
|  ...|  ...
+-+
|  VMX30  |  VSR62
+-+
|  VMX31  |  VSR63
+-+

In order to allow for future conversion of VSX instructions to use TCG vector
operations, recreate the same layout using an aligned version of the existing
vsr register array.

Since the old fpr and avr register arrays are removed, the existing callers
must also be updated to use the correct offset in the vsr register array. This
also includes switching the relevant VMState fields over to using subarrays
to make sure that migration is preserved.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
 linux-user/ppc/signal.c | 28 ---
 target/ppc/arch_dump.c  | 15 
 target/ppc/cpu.h| 25 +
 target/ppc/gdbstub.c|  8 ++---
 target/ppc/internal.h   | 18 +++---
 target/ppc/kvm.c| 24 +++--
 target/ppc/machine.c| 72 ++---
 target/ppc/monitor.c|  4 +--
 target/ppc/translate.c  | 14 
 target/ppc/translate/dfp-impl.inc.c |  2 +-
 target/ppc/translate/vmx-impl.inc.c |  7 +++-
 target/ppc/translate/vsx-impl.inc.c |  4 +--
 target/ppc/translate_init.inc.c | 26 +++---
 13 files changed, 165 insertions(+), 82 deletions(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index 2ae120a2bc..619a56950d 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -258,8 +258,8 @@ static void save_user_regs(CPUPPCState *env, struct 
target_mcontext *frame)
 /* Save Altivec registers if necessary.  */
 if (env->insns_flags & PPC_ALTIVEC) {
 uint32_t *vrsave;
-for (i = 0; i < ARRAY_SIZE(env->avr); i++) {
-ppc_avr_t *avr = &env->avr[i];
+for (i = 0; i < 32; i++) {
+ppc_avr_t *avr = cpu_avr_ptr(env, i);
 ppc_avr_t *vreg = (ppc_avr_t *)&frame->mc_vregs.altivec[i];
 
 __put_user(avr->u64[PPC_VEC_HI], &vreg->u64[0]);
@@ -281,15 +281,17 @@ static void save_user_regs(CPUPPCState *env, struct 
target_mcontext *frame)
 /* Save VSX second halves */
 if (env->insns_flags2 & PPC2_VSX) {
 uint64_t *vsregs = (uint64_t *)&frame->mc_vregs.altivec[34];
-for (i = 0; i < ARRAY_SIZE(env->vsr); i++) {
-__put_user(env->vsr[i], &vsregs[i]);
+for (i = 0; i < 32; i++) {
+uint64_t *vsrl = cpu_vsrl_ptr(env, i);
+__put_user(*vsrl, &vsregs[i]);
 }
 }
 
 /* Save floating point registers.  */
 if (env->insns_flags & PPC_FLOAT) {
-for (i = 0; i < ARRAY_SIZE(env->fpr); i++) {
-__put_user(env->fpr[i], &frame->mc_fregs[i]);
+for (i = 0; i < 32; i++) {
+uint64_t *fpr = cpu_fpr_ptr(env, i);
+__put_user(*fpr, &frame->mc_fregs[i]);
 }
 __put_user((uint64_t) env->fpscr, &frame->mc_fregs[32]);
 }
@@ -373,8 +375,8 @@ static void restore_user_regs(CPUPPCState *env,
 #else
 v_regs = (ppc_avr_t *)frame->mc_vregs.altivec;
 #endif
-for (i = 0; i < ARRAY_SIZE(env->avr); i++) {
-ppc_avr_t *avr = &env->avr[i];
+for (i = 0; i < 32; i++) {
+ppc_avr_t *avr = cpu_avr_ptr(env, i);
 ppc_avr_t *vreg = &v_regs[i];
 
 __get_user(avr->u64[PPC_VEC_HI], &vreg->u64[0]);
@@ -393,16 +395,18 @@ static void restore_user_regs(CPUPPCState *env,
 /* Restore VSX second halves */
 if (env->insns_flags2 & PPC2_VSX) {
 uint64_t *vsregs = (uint64_t *)&frame->mc_vregs.altivec[34];
-for (i = 0; i < ARRAY_SIZE(env->vsr); i++) {
-__get_user(en

[Qemu-devel] [PATCH v5 9/9] target/ppc: replace AVR* macros with Vsr* macros

[Mark Cave-Ayland]

Now that the VMX and VSR register sets have been combined, the same macros can
be used to access both AVR and VSR field members.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
---
 target/ppc/int_helper.c | 30 +-
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 9d715be25c..598731d47a 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -391,13 +391,9 @@ target_ulong helper_602_mfrom(target_ulong arg)
 #if defined(HOST_WORDS_BIGENDIAN)
 #define HI_IDX 0
 #define LO_IDX 1
-#define AVRB(i) u8[i]
-#define AVRW(i) u32[i]
 #else
 #define HI_IDX 1
 #define LO_IDX 0
-#define AVRB(i) u8[15-(i)]
-#define AVRW(i) u32[3-(i)]
 #endif
 
 #if defined(HOST_WORDS_BIGENDIAN)
@@ -3277,11 +3273,11 @@ void helper_vcipher(ppc_avr_t *r, ppc_avr_t *a, 
ppc_avr_t *b)
 int i;
 
 VECTOR_FOR_INORDER_I(i, u32) {
-result.AVRW(i) = b->AVRW(i) ^
-(AES_Te0[a->AVRB(AES_shifts[4*i + 0])] ^
- AES_Te1[a->AVRB(AES_shifts[4*i + 1])] ^
- AES_Te2[a->AVRB(AES_shifts[4*i + 2])] ^
- AES_Te3[a->AVRB(AES_shifts[4*i + 3])]);
+result.VsrW(i) = b->VsrW(i) ^
+(AES_Te0[a->VsrB(AES_shifts[4 * i + 0])] ^
+ AES_Te1[a->VsrB(AES_shifts[4 * i + 1])] ^
+ AES_Te2[a->VsrB(AES_shifts[4 * i + 2])] ^
+ AES_Te3[a->VsrB(AES_shifts[4 * i + 3])]);
 }
 *r = result;
 }
@@ -3292,7 +3288,7 @@ void helper_vcipherlast(ppc_avr_t *r, ppc_avr_t *a, 
ppc_avr_t *b)
 int i;
 
 VECTOR_FOR_INORDER_I(i, u8) {
-result.AVRB(i) = b->AVRB(i) ^ (AES_sbox[a->AVRB(AES_shifts[i])]);
+result.VsrB(i) = b->VsrB(i) ^ (AES_sbox[a->VsrB(AES_shifts[i])]);
 }
 *r = result;
 }
@@ -3305,15 +3301,15 @@ void helper_vncipher(ppc_avr_t *r, ppc_avr_t *a, 
ppc_avr_t *b)
 ppc_avr_t tmp;
 
 VECTOR_FOR_INORDER_I(i, u8) {
-tmp.AVRB(i) = b->AVRB(i) ^ AES_isbox[a->AVRB(AES_ishifts[i])];
+tmp.VsrB(i) = b->VsrB(i) ^ AES_isbox[a->VsrB(AES_ishifts[i])];
 }
 
 VECTOR_FOR_INORDER_I(i, u32) {
-r->AVRW(i) =
-AES_imc[tmp.AVRB(4*i + 0)][0] ^
-AES_imc[tmp.AVRB(4*i + 1)][1] ^
-AES_imc[tmp.AVRB(4*i + 2)][2] ^
-AES_imc[tmp.AVRB(4*i + 3)][3];
+r->VsrW(i) =
+AES_imc[tmp.VsrB(4 * i + 0)][0] ^
+AES_imc[tmp.VsrB(4 * i + 1)][1] ^
+AES_imc[tmp.VsrB(4 * i + 2)][2] ^
+AES_imc[tmp.VsrB(4 * i + 3)][3];
 }
 }
 
@@ -3323,7 +3319,7 @@ void helper_vncipherlast(ppc_avr_t *r, ppc_avr_t *a, 
ppc_avr_t *b)
 int i;
 
 VECTOR_FOR_INORDER_I(i, u8) {
-result.AVRB(i) = b->AVRB(i) ^ (AES_isbox[a->AVRB(AES_ishifts[i])]);
+result.VsrB(i) = b->VsrB(i) ^ (AES_isbox[a->VsrB(AES_ishifts[i])]);
 }
 *r = result;
 }
-- 
2.11.0

[Qemu-devel] [PATCH v5 6/9] target/ppc: switch FPR, VMX and VSX helpers to access data directly from cpu_env

[Mark Cave-Ayland]

Instead of accessing the FPR, VMX and VSX registers through static arrays of
TCGv_i64 globals, remove them and change the helpers to load/store data directly
within cpu_env.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
 target/ppc/translate.c  | 59 ++---
 target/ppc/translate/vsx-impl.inc.c |  4 +--
 2 files changed, 18 insertions(+), 45 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 3bb24e7310..b18ded07b3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -55,15 +55,9 @@
 /* global register indexes */
 static char cpu_reg_names[10*3 + 22*4 /* GPR */
 + 10*4 + 22*5 /* SPE GPRh */
-+ 10*4 + 22*5 /* FPR */
-+ 2*(10*6 + 22*7) /* AVRh, AVRl */
-+ 10*5 + 22*6 /* VSR */
 + 8*5 /* CRF */];
 static TCGv cpu_gpr[32];
 static TCGv cpu_gprh[32];
-static TCGv_i64 cpu_fpr[32];
-static TCGv_i64 cpu_avrh[32], cpu_avrl[32];
-static TCGv_i64 cpu_vsr[32];
 static TCGv_i32 cpu_crf[8];
 static TCGv cpu_nip;
 static TCGv cpu_msr;
@@ -108,39 +102,6 @@ void ppc_translate_init(void)
  offsetof(CPUPPCState, gprh[i]), p);
 p += (i < 10) ? 4 : 5;
 cpu_reg_names_size -= (i < 10) ? 4 : 5;
-
-snprintf(p, cpu_reg_names_size, "fp%d", i);
-cpu_fpr[i] = tcg_global_mem_new_i64(cpu_env,
-offsetof(CPUPPCState, fpr[i]), p);
-p += (i < 10) ? 4 : 5;
-cpu_reg_names_size -= (i < 10) ? 4 : 5;
-
-snprintf(p, cpu_reg_names_size, "avr%dH", i);
-#ifdef HOST_WORDS_BIGENDIAN
-cpu_avrh[i] = tcg_global_mem_new_i64(cpu_env,
- offsetof(CPUPPCState, 
avr[i].u64[0]), p);
-#else
-cpu_avrh[i] = tcg_global_mem_new_i64(cpu_env,
- offsetof(CPUPPCState, 
avr[i].u64[1]), p);
-#endif
-p += (i < 10) ? 6 : 7;
-cpu_reg_names_size -= (i < 10) ? 6 : 7;
-
-snprintf(p, cpu_reg_names_size, "avr%dL", i);
-#ifdef HOST_WORDS_BIGENDIAN
-cpu_avrl[i] = tcg_global_mem_new_i64(cpu_env,
- offsetof(CPUPPCState, 
avr[i].u64[1]), p);
-#else
-cpu_avrl[i] = tcg_global_mem_new_i64(cpu_env,
- offsetof(CPUPPCState, 
avr[i].u64[0]), p);
-#endif
-p += (i < 10) ? 6 : 7;
-cpu_reg_names_size -= (i < 10) ? 6 : 7;
-snprintf(p, cpu_reg_names_size, "vsr%d", i);
-cpu_vsr[i] = tcg_global_mem_new_i64(cpu_env,
-offsetof(CPUPPCState, vsr[i]), p);
-p += (i < 10) ? 5 : 6;
-cpu_reg_names_size -= (i < 10) ? 5 : 6;
 }
 
 cpu_nip = tcg_global_mem_new(cpu_env,
@@ -6701,22 +6662,34 @@ GEN_TM_PRIV_NOOP(trechkpt);
 
 static inline void get_fpr(TCGv_i64 dst, int regno)
 {
-tcg_gen_mov_i64(dst, cpu_fpr[regno]);
+tcg_gen_ld_i64(dst, cpu_env, offsetof(CPUPPCState, fpr[regno]));
 }
 
 static inline void set_fpr(int regno, TCGv_i64 src)
 {
-tcg_gen_mov_i64(cpu_fpr[regno], src);
+tcg_gen_st_i64(src, cpu_env, offsetof(CPUPPCState, fpr[regno]));
 }
 
 static inline void get_avr64(TCGv_i64 dst, int regno, bool high)
 {
-tcg_gen_mov_i64(dst, (high ? cpu_avrh : cpu_avrl)[regno]);
+#ifdef HOST_WORDS_BIGENDIAN
+tcg_gen_ld_i64(dst, cpu_env, offsetof(CPUPPCState,
+  avr[regno].u64[(high ? 0 : 1)]));
+#else
+tcg_gen_ld_i64(dst, cpu_env, offsetof(CPUPPCState,
+  avr[regno].u64[(high ? 1 : 0)]));
+#endif
 }
 
 static inline void set_avr64(int regno, TCGv_i64 src, bool high)
 {
-tcg_gen_mov_i64((high ? cpu_avrh : cpu_avrl)[regno], src);
+#ifdef HOST_WORDS_BIGENDIAN
+tcg_gen_st_i64(src, cpu_env, offsetof(CPUPPCState,
+  avr[regno].u64[(high ? 0 : 1)]));
+#else
+tcg_gen_st_i64(src, cpu_env, offsetof(CPUPPCState,
+  avr[regno].u64[(high ? 1 : 0)]));
+#endif
 }
 
 #include "translate/fp-impl.inc.c"
diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index f0665df1a5..7eaa36b4d5 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -2,12 +2,12 @@
 
 static inline void get_vsr(TCGv_i64 dst, int n)
 {
-tcg_gen_mov_i64(dst, cpu_vsr[n]);
+tcg_gen_ld_i64(dst, cpu_env, offsetof(CPUPPCState, vsr[n]));
 }
 
 static inline void set_vsr(int n, TCGv_i64 src)
 {
-tcg_gen_mov_i64(cpu_vsr[n], src);
+tcg_gen_st_i64(src, cpu_env, offsetof(CPUPPCState, vsr[n]));
 }
 
 static inline void get_cpu_vsrh(TCGv_i64 dst, int n)
-- 
2.11.0

[Qemu-devel] [PATCH v5 7/9] target/ppc: merge ppc_vsr_t and ppc_avr_t union types

[Mark Cave-Ayland]

Since the VSX registers are actually a superset of the VMX registers then they
can be represented by the same type. Merge ppc_avr_t into ppc_vsr_t and change
ppc_avr_t to be a simple typedef alias.

Note that due to a difference in the naming of the float32 member between
ppc_avr_t and ppc_vsr_t, references to the ppc_avr_t f member must be replaced
with f32 instead.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
 target/ppc/cpu.h| 17 ---
 target/ppc/int_helper.c | 56 +
 target/ppc/internal.h   | 11 --
 3 files changed, 39 insertions(+), 45 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index d5f99f1fc7..578641ac20 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -218,7 +218,6 @@ typedef struct opc_handler_t opc_handler_t;
 /* Types used to describe some PowerPC registers etc. */
 typedef struct DisasContext DisasContext;
 typedef struct ppc_spr_t ppc_spr_t;
-typedef union ppc_avr_t ppc_avr_t;
 typedef union ppc_tlb_t ppc_tlb_t;
 typedef struct ppc_hash_pte64 ppc_hash_pte64_t;
 
@@ -242,22 +241,26 @@ struct ppc_spr_t {
 #endif
 };
 
-/* Altivec registers (128 bits) */
-union ppc_avr_t {
-float32 f[4];
+/* VSX/Altivec registers (128 bits) */
+typedef union _ppc_vsr_t {
 uint8_t u8[16];
 uint16_t u16[8];
 uint32_t u32[4];
+uint64_t u64[2];
 int8_t s8[16];
 int16_t s16[8];
 int32_t s32[4];
-uint64_t u64[2];
 int64_t s64[2];
+float32 f32[4];
+float64 f64[2];
+float128 f128;
 #ifdef CONFIG_INT128
 __uint128_t u128;
 #endif
-Int128 s128;
-};
+Int128  s128;
+} ppc_vsr_t;
+
+typedef ppc_vsr_t ppc_avr_t;
 
 #if !defined(CONFIG_USER_ONLY)
 /* Software TLB cache */
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index fcac90a4a9..9d715be25c 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -548,8 +548,8 @@ VARITH_DO(muluwm, *, u32)
 {   \
 int i;  \
 \
-for (i = 0; i < ARRAY_SIZE(r->f); i++) {\
-r->f[i] = func(a->f[i], b->f[i], &env->vec_status); \
+for (i = 0; i < ARRAY_SIZE(r->f32); i++) {  \
+r->f32[i] = func(a->f32[i], b->f32[i], &env->vec_status);   \
 }   \
 }
 VARITHFP(addfp, float32_add)
@@ -563,9 +563,9 @@ VARITHFP(maxfp, float32_max)
ppc_avr_t *b, ppc_avr_t *c)  \
 {   \
 int i;  \
-for (i = 0; i < ARRAY_SIZE(r->f); i++) {\
-r->f[i] = float32_muladd(a->f[i], c->f[i], b->f[i], \
- type, &env->vec_status);   \
+for (i = 0; i < ARRAY_SIZE(r->f32); i++) {  \
+r->f32[i] = float32_muladd(a->f32[i], c->f32[i], b->f32[i], \
+   type, &env->vec_status); \
 }   \
 }
 VARITHFPFMA(maddfp, 0);
@@ -670,9 +670,9 @@ VABSDU(w, u32)
 {   \
 int i;  \
 \
-for (i = 0; i < ARRAY_SIZE(r->f); i++) {\
+for (i = 0; i < ARRAY_SIZE(r->f32); i++) {  \
 float32 t = cvt(b->element[i], &env->vec_status);   \
-r->f[i] = float32_scalbn(t, -uim, &env->vec_status);\
+r->f32[i] = float32_scalbn(t, -uim, &env->vec_status);  \
 }   \
 }
 VCF(ux, uint32_to_float32, u32)
@@ -782,9 +782,9 @@ VCMPNE(w, u32, uint32_t, 0)
 uint32_t none = 0;  \
 int i;  \
 \
-for (i = 0; i < ARRAY_SIZE(r->f); i++) {\
+for (i = 0; i < ARRAY_SIZE(r->f32); i++) {  \
 uint32_t result;\
-int rel = float32_compare_quiet(a->f[i], b->f[i],   \
+int rel = float32_compare_quiet(a->f32[i], b->f32[i],   \
 &env->vec_status);  \
 if (rel == float_relation_unordered) {   

[Qemu-devel] [PATCH v5 0/9] target/ppc: prepare for conversion to TCG vector operations

[Mark Cave-Ayland]

This patchset is an attempt at trying to improve the VMX (Altivec) instruction
performance by laying the groundwork for use of the new TCG vector operations.

Patches 1 and 2 fix a sign-extension error discovered in EXTRACT_SHELPER and an
associated typo in the SIMM5 macro which were discovered whilst testing 
Richard's
follow-on TCG vector improvements patchset.

In order to use TCG vector operations, the registers must be accessible from 
cpu_env
whilst currently they are accessed via arrays of static TCG globals. Patches 3-5
are therefore mechanical patches which introduce access helpers for FPR, AVR 
and VSR
registers using the supplied TCGv_i64 parameter.

Once this is done, patch 6 enables us to remove the static TCG global arrays 
and updates
the access helpers to read/write to the relevant fields in cpu_env directly.

Patches 7 and 8 perform the legwork required to enable VSX instructions to be 
converted
to use TCG vector operations in future by rearranging the FP, VMX and VSX 
registers into
a single aligned VSR register array (the scope of this patchset is VMX only).

Patch 9 removes the AVR* macros and replaces them with the corresponding Vsr* 
macros
since they are equivalent.

Finally thanks to Richard for taking the time to answer some of my (mostly 
beginner)
questions related to TCG.

Signed-off-by: Mark Cave-Ayland 

v5:
- Fix up KVM-enabled builds on PPC host due to missing conversion of 
target/ppc/kvm.c

v4:
- Rebase onto master
- Add extra R-B tags from Richard
- Leave HI_IDX/LO_IDX in int_helper.c in patch 9 (similarly named macros are 
also
  used in other files so let's ensure there is no confusion)
- Add cpu_fpr_ptr(), cpu_vsrl_ptr() and cpu_avr_ptr() as suggested by Richard in
  patch 8

v3:
- Rebase onto master, drop RFC prefix, alter subject line
- Add A-B tags from David
- Add SIMM5/EXTRACT_HELPER macro fix patches to the start of the series
- Drop patch 4 from previous patchset (delay AVR register writeback) as it 
should
  not be required.
- Remove extra get_fpr() accidentally added to GEN_FLOAT macros in patch 3
- Fix temporary leak when VMX/VSX not enabled in patches 4 and 5
- Add patch to remove AVR* macros, replacing them with Vsr* macros
- Drop patches converting logical, add and sub instructions to TCG vector ops 
(let
  Richard incorporate this into his TCG vector improvements patchset)

v2:
- Rebase onto master
- Add comment explaining rationale for FPR helpers in description for patch 1
- Add R-B tags from Richard
- Add patch 3 to delay AVR register writeback as spotted by Richard
- Add patches 6 and 7 to merge FPR, VMX and VSX registers into the vsr array
  to facilitate conversion of VSX instructions to vector operations later
- Fix accidental bug whereby the conversion of get_vsr()/set_vsr() to access
  data from cpu_env was incorrectly squashed into patch 3
- Move set_fpr() further down in gen_fsqrts() and gen_frsqrtes() in patch 1

Mark Cave-Ayland (9):
  target/ppc: fix typo in SIMM5 extraction helper
  target/ppc: switch EXTRACT_HELPER macros over to use
sextract32/extract32
  target/ppc: introduce get_fpr() and set_fpr() helpers for FP register
access
  target/ppc: introduce get_avr64() and set_avr64() helpers for VMX
register access
  target/ppc: introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}()
helpers for VSR register access
  target/ppc: switch FPR, VMX and VSX helpers to access data directly
from cpu_env
  target/ppc: merge ppc_vsr_t and ppc_avr_t union types
  target/ppc: move FP and VMX registers into aligned vsr register array
  target/ppc: replace AVR* macros with Vsr* macros

 linux-user/ppc/signal.c |  28 +-
 target/ppc/arch_dump.c  |  15 +-
 target/ppc/cpu.h|  42 +-
 target/ppc/gdbstub.c|   8 +-
 target/ppc/int_helper.c |  86 ++--
 target/ppc/internal.h   |  39 +-
 target/ppc/kvm.c|  24 +-
 target/ppc/machine.c|  72 ++-
 target/ppc/monitor.c|   4 +-
 target/ppc/translate.c  |  73 ++-
 target/ppc/translate/dfp-impl.inc.c |   2 +-
 target/ppc/translate/fp-impl.inc.c  | 486 +++-
 target/ppc/translate/vmx-impl.inc.c | 154 +--
 target/ppc/translate/vsx-impl.inc.c | 862 ++--
 target/ppc/translate_init.inc.c |  26 +-
 15 files changed, 1374 insertions(+), 547 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH v5 5/9] target/ppc: introduce get_cpu_vsr{l, h}() and set_cpu_vsr{l, h}() helpers for VSR register access

[Mark Cave-Ayland]

These helpers allow us to move VSR register values to/from the specified 
TCGv_i64
argument.

To prevent VSX helpers accessing the cpu_vsr array directly, add extra TCG
temporaries as required.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
 target/ppc/translate/vsx-impl.inc.c | 862 ++--
 1 file changed, 638 insertions(+), 224 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index 85ed135d44..f0665df1a5 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -1,20 +1,48 @@
 /***   VSX extension   ***/
 
-static inline TCGv_i64 cpu_vsrh(int n)
+static inline void get_vsr(TCGv_i64 dst, int n)
+{
+tcg_gen_mov_i64(dst, cpu_vsr[n]);
+}
+
+static inline void set_vsr(int n, TCGv_i64 src)
+{
+tcg_gen_mov_i64(cpu_vsr[n], src);
+}
+
+static inline void get_cpu_vsrh(TCGv_i64 dst, int n)
+{
+if (n < 32) {
+get_fpr(dst, n);
+} else {
+get_avr64(dst, n - 32, true);
+}
+}
+
+static inline void get_cpu_vsrl(TCGv_i64 dst, int n)
+{
+if (n < 32) {
+get_vsr(dst, n);
+} else {
+get_avr64(dst, n - 32, false);
+}
+}
+
+static inline void set_cpu_vsrh(int n, TCGv_i64 src)
 {
 if (n < 32) {
-return cpu_fpr[n];
+set_fpr(n, src);
 } else {
-return cpu_avrh[n-32];
+set_avr64(n - 32, src, true);
 }
 }
 
-static inline TCGv_i64 cpu_vsrl(int n)
+static inline void set_cpu_vsrl(int n, TCGv_i64 src)
 {
 if (n < 32) {
-return cpu_vsr[n];
+set_vsr(n, src);
 } else {
-return cpu_avrl[n-32];
+set_avr64(n - 32, src, false);
 }
 }
 
@@ -22,16 +50,20 @@ static inline TCGv_i64 cpu_vsrl(int n)
 static void gen_##name(DisasContext *ctx) \
 { \
 TCGv EA;  \
+TCGv_i64 t0;  \
 if (unlikely(!ctx->vsx_enabled)) {\
 gen_exception(ctx, POWERPC_EXCP_VSXU);\
 return;   \
 } \
+t0 = tcg_temp_new_i64();  \
 gen_set_access_type(ctx, ACCESS_INT); \
 EA = tcg_temp_new();  \
 gen_addr_reg_index(ctx, EA);  \
-gen_qemu_##operation(ctx, cpu_vsrh(xT(ctx->opcode)), EA); \
+gen_qemu_##operation(ctx, t0, EA);\
+set_cpu_vsrh(xT(ctx->opcode), t0);\
 /* NOTE: cpu_vsrl is undefined */ \
 tcg_temp_free(EA);\
+tcg_temp_free_i64(t0);\
 }
 
 VSX_LOAD_SCALAR(lxsdx, ld64_i64)
@@ -44,43 +76,60 @@ VSX_LOAD_SCALAR(lxsspx, ld32fs)
 static void gen_lxvd2x(DisasContext *ctx)
 {
 TCGv EA;
+TCGv_i64 t0;
 if (unlikely(!ctx->vsx_enabled)) {
 gen_exception(ctx, POWERPC_EXCP_VSXU);
 return;
 }
+t0 = tcg_temp_new_i64();
 gen_set_access_type(ctx, ACCESS_INT);
 EA = tcg_temp_new();
 gen_addr_reg_index(ctx, EA);
-gen_qemu_ld64_i64(ctx, cpu_vsrh(xT(ctx->opcode)), EA);
+gen_qemu_ld64_i64(ctx, t0, EA);
+set_cpu_vsrh(xT(ctx->opcode), t0);
 tcg_gen_addi_tl(EA, EA, 8);
-gen_qemu_ld64_i64(ctx, cpu_vsrl(xT(ctx->opcode)), EA);
+gen_qemu_ld64_i64(ctx, t0, EA);
+set_cpu_vsrl(xT(ctx->opcode), t0);
 tcg_temp_free(EA);
+tcg_temp_free_i64(t0);
 }
 
 static void gen_lxvdsx(DisasContext *ctx)
 {
 TCGv EA;
+TCGv_i64 t0;
+TCGv_i64 t1;
 if (unlikely(!ctx->vsx_enabled)) {
 gen_exception(ctx, POWERPC_EXCP_VSXU);
 return;
 }
+t0 = tcg_temp_new_i64();
+t1 = tcg_temp_new_i64();
 gen_set_access_type(ctx, ACCESS_INT);
 EA = tcg_temp_new();
 gen_addr_reg_index(ctx, EA);
-gen_qemu_ld64_i64(ctx, cpu_vsrh(xT(ctx->opcode)), EA);
-tcg_gen_mov_i64(cpu_vsrl(xT(ctx->opcode)), cpu_vsrh(xT(ctx->opcode)));
+gen_qemu_ld64_i64(ctx, t0, EA);
+set_cpu_vsrh(xT(ctx->opcode), t0);
+tcg_gen_mov_i64(t1, t0);
+set_cpu_vsrl(xT(ctx->opcode), t1);
 tcg_temp_free(EA);
+tcg_temp_free_i64(t0);
+tcg_temp_free_i64(t1);
 }
 
 static void gen_lxvw4x(DisasContext *ctx)
 {
 TCGv EA;
-TCGv_i64 xth = cpu_vsrh(xT(ctx->opcode));
-TCGv_i64 xtl = cpu_vsrl(xT(ctx->opcode));
+TCGv_i64 xth;
+TCGv_i64 xtl;
 if (unlikely(!ctx->vsx_enabled)) {
 gen_exception(ctx, POWERPC_EXCP_VSXU);
 return;
 }
+xth = tcg_temp_new_i64();
+xtl = tcg_temp_new_i64();
+get_cpu_vsrh(xth, xT(ctx->opcode));
+get_cpu_vsrl(xtl, xT(ctx->opcode));
 gen_set

[Qemu-devel] [PATCH v5 2/9] target/ppc: switch EXTRACT_HELPER macros over to use sextract32/extract32

[Mark Cave-Ayland]

These ensure that we consistently handle signed and unsigned extensions 
correctly
when decoding immediates from instruction opcodes.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
---
 target/ppc/internal.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 8b35863549..5d460247e2 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -52,20 +52,20 @@ FUNC_MASK(mask_u64, uint64_t, 64, UINT64_MAX);
 #define EXTRACT_HELPER(name, shift, nb)   \
 static inline uint32_t name(uint32_t opcode)  \
 { \
-return (opcode >> (shift)) & ((1 << (nb)) - 1);   \
+return extract32(opcode, shift, nb);  \
 }
 
 #define EXTRACT_SHELPER(name, shift, nb)  \
 static inline int32_t name(uint32_t opcode)   \
 { \
-return (int16_t)((opcode >> (shift)) & ((1 << (nb)) - 1));\
+return sextract32(opcode, shift, nb); \
 }
 
 #define EXTRACT_HELPER_SPLIT(name, shift1, nb1, shift2, nb2)  \
 static inline uint32_t name(uint32_t opcode)  \
 { \
-return (((opcode >> (shift1)) & ((1 << (nb1)) - 1)) << nb2) | \
-((opcode >> (shift2)) & ((1 << (nb2)) - 1));  \
+return extract32(opcode, shift1, nb1) << nb2 |\
+   extract32(opcode, shift2, nb2);\
 }
 
 #define EXTRACT_HELPER_SPLIT_3(name,  \
-- 
2.11.0

[Qemu-devel] [PATCH v5 4/9] target/ppc: introduce get_avr64() and set_avr64() helpers for VMX register access

[Mark Cave-Ayland]

These helpers allow us to move AVR register values to/from the specified 
TCGv_i64
argument.

To prevent VMX helpers accessing the cpu_avr{l,h} arrays directly, add extra TCG
temporaries as required.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
 target/ppc/translate.c  |  10 +++
 target/ppc/translate/vmx-impl.inc.c | 147 
 2 files changed, 124 insertions(+), 33 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 9cecab42f3..3bb24e7310 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6709,6 +6709,16 @@ static inline void set_fpr(int regno, TCGv_i64 src)
 tcg_gen_mov_i64(cpu_fpr[regno], src);
 }
 
+static inline void get_avr64(TCGv_i64 dst, int regno, bool high)
+{
+tcg_gen_mov_i64(dst, (high ? cpu_avrh : cpu_avrl)[regno]);
+}
+
+static inline void set_avr64(int regno, TCGv_i64 src, bool high)
+{
+tcg_gen_mov_i64((high ? cpu_avrh : cpu_avrl)[regno], src);
+}
+
 #include "translate/fp-impl.inc.c"
 
 #include "translate/vmx-impl.inc.c"
diff --git a/target/ppc/translate/vmx-impl.inc.c 
b/target/ppc/translate/vmx-impl.inc.c
index 3cb6fc2926..5e8327e9a3 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -18,52 +18,66 @@ static inline TCGv_ptr gen_avr_ptr(int reg)
 static void glue(gen_, name)(DisasContext *ctx)
   \
 { \
 TCGv EA;  \
+TCGv_i64 avr; \
 if (unlikely(!ctx->altivec_enabled)) {\
 gen_exception(ctx, POWERPC_EXCP_VPU); \
 return;   \
 } \
 gen_set_access_type(ctx, ACCESS_INT); \
+avr = tcg_temp_new_i64(); \
 EA = tcg_temp_new();  \
 gen_addr_reg_index(ctx, EA);  \
 tcg_gen_andi_tl(EA, EA, ~0xf);\
 /* We only need to swap high and low halves. gen_qemu_ld64_i64 does   \
necessary 64-bit byteswap already. */  \
 if (ctx->le_mode) {   \
-gen_qemu_ld64_i64(ctx, cpu_avrl[rD(ctx->opcode)], EA);\
+gen_qemu_ld64_i64(ctx, avr, EA);  \
+set_avr64(rD(ctx->opcode), avr, false);   \
 tcg_gen_addi_tl(EA, EA, 8);   \
-gen_qemu_ld64_i64(ctx, cpu_avrh[rD(ctx->opcode)], EA);\
+gen_qemu_ld64_i64(ctx, avr, EA);  \
+set_avr64(rD(ctx->opcode), avr, true);\
 } else {  \
-gen_qemu_ld64_i64(ctx, cpu_avrh[rD(ctx->opcode)], EA);\
+gen_qemu_ld64_i64(ctx, avr, EA);  \
+set_avr64(rD(ctx->opcode), avr, true);\
 tcg_gen_addi_tl(EA, EA, 8);   \
-gen_qemu_ld64_i64(ctx, cpu_avrl[rD(ctx->opcode)], EA);\
+gen_qemu_ld64_i64(ctx, avr, EA);  \
+set_avr64(rD(ctx->opcode), avr, false);   \
 } \
 tcg_temp_free(EA);\
+tcg_temp_free_i64(avr);   \
 }
 
 #define GEN_VR_STX(name, opc2, opc3)  \
 static void gen_st##name(DisasContext *ctx)   \
 { \
 TCGv EA;  \
+TCGv_i64 avr; \
 if (unlikely(!ctx->altivec_enabled)) {\
 gen_exception(ctx, POWERPC_EXCP_VPU); \
 return;   \
 } \
 gen_set_access_type(ctx, ACCESS_INT); \
+avr = tcg_temp_new_i64();   

[Qemu-devel] [PATCH v5 3/9] target/ppc: introduce get_fpr() and set_fpr() helpers for FP register access

[Mark Cave-Ayland]

These helpers allow us to move FP register values to/from the specified TCGv_i64
argument in the VSR helpers to be introduced shortly.

To prevent FP helpers accessing the cpu_fpr array directly, add extra TCG
temporaries as required.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
 target/ppc/translate.c |  10 +
 target/ppc/translate/fp-impl.inc.c | 486 -
 2 files changed, 386 insertions(+), 110 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 96894ab9a8..9cecab42f3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6699,6 +6699,16 @@ static inline void gen_##name(DisasContext *ctx) 
  \
 GEN_TM_PRIV_NOOP(treclaim);
 GEN_TM_PRIV_NOOP(trechkpt);
 
+static inline void get_fpr(TCGv_i64 dst, int regno)
+{
+tcg_gen_mov_i64(dst, cpu_fpr[regno]);
+}
+
+static inline void set_fpr(int regno, TCGv_i64 src)
+{
+tcg_gen_mov_i64(cpu_fpr[regno], src);
+}
+
 #include "translate/fp-impl.inc.c"
 
 #include "translate/vmx-impl.inc.c"
diff --git a/target/ppc/translate/fp-impl.inc.c 
b/target/ppc/translate/fp-impl.inc.c
index 08770ba9f5..0f21a4e477 100644
--- a/target/ppc/translate/fp-impl.inc.c
+++ b/target/ppc/translate/fp-impl.inc.c
@@ -34,24 +34,37 @@ static void gen_set_cr1_from_fpscr(DisasContext *ctx)
 #define _GEN_FLOAT_ACB(name, op, op1, op2, isfloat, set_fprf, type)   \
 static void gen_f##name(DisasContext *ctx)\
 { \
+TCGv_i64 t0;  \
+TCGv_i64 t1;  \
+TCGv_i64 t2;  \
+TCGv_i64 t3;  \
 if (unlikely(!ctx->fpu_enabled)) {\
 gen_exception(ctx, POWERPC_EXCP_FPU); \
 return;   \
 } \
+t0 = tcg_temp_new_i64();  \
+t1 = tcg_temp_new_i64();  \
+t2 = tcg_temp_new_i64();  \
+t3 = tcg_temp_new_i64();  \
 gen_reset_fpstatus(); \
-gen_helper_f##op(cpu_fpr[rD(ctx->opcode)], cpu_env,   \
- cpu_fpr[rA(ctx->opcode)],\
- cpu_fpr[rC(ctx->opcode)], cpu_fpr[rB(ctx->opcode)]); \
+get_fpr(t0, rA(ctx->opcode)); \
+get_fpr(t1, rC(ctx->opcode)); \
+get_fpr(t2, rB(ctx->opcode)); \
+gen_helper_f##op(t3, cpu_env, t0, t1, t2);\
 if (isfloat) {\
-gen_helper_frsp(cpu_fpr[rD(ctx->opcode)], cpu_env,\
-cpu_fpr[rD(ctx->opcode)]);\
+gen_helper_frsp(t3, cpu_env, t3); \
 } \
+set_fpr(rD(ctx->opcode), t3); \
 if (set_fprf) {   \
-gen_compute_fprf_float64(cpu_fpr[rD(ctx->opcode)]);   \
+gen_compute_fprf_float64(t3); \
 } \
 if (unlikely(Rc(ctx->opcode) != 0)) { \
 gen_set_cr1_from_fpscr(ctx);  \
 } \
+tcg_temp_free_i64(t0);\
+tcg_temp_free_i64(t1);\
+tcg_temp_free_i64(t2);\
+tcg_temp_free_i64(t3);\
 }
 
 #define GEN_FLOAT_ACB(name, op2, set_fprf, type)  \
@@ -61,24 +74,33 @@ _GEN_FLOAT_ACB(name##s, name, 0x3B, op2, 1, set_fprf, type);
 #define _GEN_FLOAT_AB(name, op, op1, op2, inval, isfloat, set_fprf, type) \
 static void gen_f##name(DisasContext *ctx)\
 {   

[Qemu-devel] [PATCH v5 1/9] target/ppc: fix typo in SIMM5 extraction helper

[Mark Cave-Ayland]

As the macro name suggests, the argument should be signed and not unsigned.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Richard Henderson 
---
 target/ppc/internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index a9bcadff42..8b35863549 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -124,7 +124,7 @@ EXTRACT_SHELPER(SIMM, 0, 16);
 /* 16 bits unsigned immediate value */
 EXTRACT_HELPER(UIMM, 0, 16);
 /* 5 bits signed immediate value */
-EXTRACT_HELPER(SIMM5, 16, 5);
+EXTRACT_SHELPER(SIMM5, 16, 5);
 /* 5 bits signed immediate value */
 EXTRACT_HELPER(UIMM5, 16, 5);
 /* 4 bits unsigned immediate value */
-- 
2.11.0