Re: [Qemu-devel] nbd structured reply

[Vladimir Sementsov-Ogievskiy]

05.10.2017 16:37, Eric Blake wrote:

On 10/05/2017 06:30 AM, Vladimir Sementsov-Ogievskiy wrote:

21.09.2017 15:18, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

I'm about this:

"A server SHOULD try to minimize the number of chunks sent in a reply,
but MUST NOT mark a chunk as final if there is still a possibility of
detecting an error before transmission of that chunk completes"

What do we mean by "possibility"? Formally, such possibility exists
always, so, we'll never mark a chunk as final.


One more question:

for |NBD_REPLY_TYPE_ERROR and ||NBD_REPLY_TYPE_ERROR_OFFSET, why do we
need message_length field? why not to calc it as chunk.lenght - 4 for
||NBD_REPLY_TYPE_ERROR and chunk.lenght - 12 for
||NBD_REPLY_TYPE_ERROR_OFFSET?

For consistency.  If _all_ NBD_REPLY_TYPE_ERROR* message have a
message_length field, then it is easier to write a generic handler that


Oh, I've missed this in the spec, thanks. Of course it make sense.


knows how to deal with an unknown error, no matter what command the
error is sent in response to.  Ideally, a server should never send an
error message that the client is not expecting, but having a robust
protocol that lets clients deal with bad servers is worth the redundancy
caused by being consistent, and we are more likely to add additional
error modes to existing commands than we are to add more success modes.


For example, with NBD_REPLY_TYPE_OFFSET_DATA variable data length is
calculated, not specified separately.

That's because non-error types don't have the same consistency concerns;
if we want to introduce a new success response, we'll probably introduce
it via a new command, rather than as a reply to an existing command.
Furthermore, while error replies are likely to have a free-form text
error description, success replies tend to not need it.  The layout of
the error types is designed to make it easy to grab the free-form error
message from a known location for display to the user, even if the
client has no idea what the rest of the error means, as that may be a
useful debugging aid.


What is the reason for server to send NBD_REPLY_TYPE_ERROR with
message_lenght < chunk.lenght - 4?

In all likelihood, all well-written servers will never send garbage
bytes (possible only when setting chunk.length larger than
message_length + sizeof(documented fields)).  But we wrote the spec to
be conservative, in case we want to add a later defined field that
earlier clients will still gracefully ignore, rather than strict
(allowing inequality, instead of requiring exact lengths, lets a client
skip over what it considers garbage bytes rather than dropping the
connection because a too-new server tried to send useful information in
those bytes).




--
Best regards,
Vladimir

Re: [Qemu-devel] [Qemu-ppc] [PATCH 22/23] ppc: pnv: drop PnvChipClass::cpu_model field

[Cédric Le Goater]

On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> deduce core type directly from chip type instead of
> maintaining type mapping in PnvChipClass::cpu_model.

nice one again.
 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Cédric Le Goater 

Thanks,

C.


> ---
>  include/hw/ppc/pnv.h  |  1 -
>  include/hw/ppc/pnv_core.h |  1 -
>  hw/ppc/pnv.c  | 25 +
>  hw/ppc/pnv_core.c |  5 -
>  4 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index d82eee1..20244da 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -69,7 +69,6 @@ typedef struct PnvChipClass {
>  SysBusDeviceClass parent_class;
>  
>  /*< public >*/
> -const char *cpu_model;
>  PnvChipType  chip_type;
>  uint64_t chip_cfam_id;
>  uint64_t cores_mask;
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index a336a1f..e337af7 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -46,6 +46,5 @@ typedef struct PnvCoreClass {
>  
>  #define PNV_CORE_TYPE_SUFFIX "-" TYPE_PNV_CORE
>  #define PNV_CORE_TYPE_NAME(cpu_model) cpu_model PNV_CORE_TYPE_SUFFIX
> -extern char *pnv_core_typename(const char *model);
>  
>  #endif /* _PPC_PNV_CORE_H */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 9c5eb7c..ab7083b 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -55,6 +55,16 @@
>  #define KERNEL_LOAD_ADDR0x2000
>  #define INITRD_LOAD_ADDR0x4000
>  
> +static const char *pvn_chip_core_typename(const PnvChip *o)
> +{
> +const char *chip_type = 
> object_class_get_name(object_get_class(OBJECT(o)));
> +int len = strlen(chip_type) - strlen(PNV_CHIP_TYPE_SUFFIX);
> +char *s = g_strdup_printf(PNV_CORE_TYPE_NAME("%.*s"), len, chip_type);
> +const char *core_type = object_class_get_name(object_class_by_name(s));
> +g_free(s);
> +return core_type;
> +}
> +
>  /*
>   * On Power Systems E880 (POWER8), the max cpus (threads) should be :
>   * 4 * 4 sockets * 12 cores * 8 threads = 1536
> @@ -270,8 +280,7 @@ static int pnv_chip_lpc_offset(PnvChip *chip, void *fdt)
>  
>  static void powernv_populate_chip(PnvChip *chip, void *fdt)
>  {
> -PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> -char *typename = pnv_core_typename(pcc->cpu_model);
> +const char *typename = pvn_chip_core_typename(chip);
>  size_t typesize = object_type_get_instance_size(typename);
>  int i;
>  
> @@ -301,7 +310,6 @@ static void powernv_populate_chip(PnvChip *chip, void 
> *fdt)
>  powernv_populate_memory_node(fdt, chip->chip_id, chip->ram_start,
>   chip->ram_size);
>  }
> -g_free(typename);
>  }
>  
>  static void powernv_populate_rtc(ISADevice *d, void *fdt, int lpc_off)
> @@ -713,7 +721,6 @@ static void pnv_chip_power8e_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "power8e_v2.1";
>  k->chip_type = PNV_CHIP_POWER8E;
>  k->chip_cfam_id = 0x221ef0498000ull;  /* P8 Murano DD2.1 */
>  k->cores_mask = POWER8E_CORE_MASK;
> @@ -735,7 +742,6 @@ static void pnv_chip_power8_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "power8_v2.0";
>  k->chip_type = PNV_CHIP_POWER8;
>  k->chip_cfam_id = 0x220ea0498000ull; /* P8 Venice DD2.0 */
>  k->cores_mask = POWER8_CORE_MASK;
> @@ -757,7 +763,6 @@ static void pnv_chip_power8nvl_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "power8nvl_v1.0";
>  k->chip_type = PNV_CHIP_POWER8NVL;
>  k->chip_cfam_id = 0x120d30498000ull;  /* P8 Naples DD1.0 */
>  k->cores_mask = POWER8_CORE_MASK;
> @@ -779,7 +784,6 @@ static void pnv_chip_power9_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "power9_v1.0";
>  k->chip_type = PNV_CHIP_POWER9;
>  k->chip_cfam_id = 0x100d10498000ull; /* P9 Nimbus DD1.0 */
>  k->cores_mask = POWER9_CORE_MASK;
> @@ -854,7 +858,7 @@ static void pnv_chip_init(Object *obj)
>  static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
>  {
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> -char *typename = pnv_core_typename(pcc->cpu_model);
> +const char *typename = pvn_chip_core_typename(chip);
>  size_t typesize = object_type_get_instance_size(typename);
>  int i, j;
>  char *name;
> @@ -879,8 +883,6 @@ static void pnv_chip_icp_realize(PnvChip *chip, Error 
> **errp)
>  memory_region_add_subregion(&chip->icp_mmio, pir << 12, 
> &icp->mmio);
>  }
>  }
> -
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 23/23] ppc: pnv: consolidate type definitions and batch register them

[Cédric Le Goater]

On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> Use a new DEFINE_TYPES() helper to simplify type registration
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Cédric Le Goater 

Thanks,

C.

> ---
>  hw/ppc/pnv.c | 92 
> ++--
>  1 file changed, 34 insertions(+), 58 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index ab7083b..e23dc3c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -730,13 +730,6 @@ static void pnv_chip_power8e_class_init(ObjectClass 
> *klass, void *data)
>  dc->desc = "PowerNV Chip POWER8E";
>  }
>  
> -static const TypeInfo pnv_chip_power8e_info = {
> -.name  = TYPE_PNV_CHIP_POWER8E,
> -.parent= TYPE_PNV_CHIP,
> -.instance_size = sizeof(PnvChip),
> -.class_init= pnv_chip_power8e_class_init,
> -};
> -
>  static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -751,13 +744,6 @@ static void pnv_chip_power8_class_init(ObjectClass 
> *klass, void *data)
>  dc->desc = "PowerNV Chip POWER8";
>  }
>  
> -static const TypeInfo pnv_chip_power8_info = {
> -.name  = TYPE_PNV_CHIP_POWER8,
> -.parent= TYPE_PNV_CHIP,
> -.instance_size = sizeof(PnvChip),
> -.class_init= pnv_chip_power8_class_init,
> -};
> -
>  static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -772,13 +758,6 @@ static void pnv_chip_power8nvl_class_init(ObjectClass 
> *klass, void *data)
>  dc->desc = "PowerNV Chip POWER8NVL";
>  }
>  
> -static const TypeInfo pnv_chip_power8nvl_info = {
> -.name  = TYPE_PNV_CHIP_POWER8NVL,
> -.parent= TYPE_PNV_CHIP,
> -.instance_size = sizeof(PnvChip),
> -.class_init= pnv_chip_power8nvl_class_init,
> -};
> -
>  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -793,13 +772,6 @@ static void pnv_chip_power9_class_init(ObjectClass 
> *klass, void *data)
>  dc->desc = "PowerNV Chip POWER9";
>  }
>  
> -static const TypeInfo pnv_chip_power9_info = {
> -.name  = TYPE_PNV_CHIP_POWER9,
> -.parent= TYPE_PNV_CHIP,
> -.instance_size = sizeof(PnvChip),
> -.class_init= pnv_chip_power9_class_init,
> -};
> -
>  static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
>  {
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> @@ -1001,15 +973,6 @@ static void pnv_chip_class_init(ObjectClass *klass, 
> void *data)
>  dc->desc = "PowerNV Chip";
>  }
>  
> -static const TypeInfo pnv_chip_info = {
> -.name  = TYPE_PNV_CHIP,
> -.parent= TYPE_SYS_BUS_DEVICE,
> -.class_init= pnv_chip_class_init,
> -.instance_init = pnv_chip_init,
> -.class_size= sizeof(PnvChipClass),
> -.abstract  = true,
> -};
> -
>  static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
>  {
>  PnvMachineState *pnv = POWERNV_MACHINE(xi);
> @@ -1145,27 +1108,40 @@ static void powernv_machine_class_init(ObjectClass 
> *oc, void *data)
>  powernv_machine_class_props_init(oc);
>  }
>  
> -static const TypeInfo powernv_machine_info = {
> -.name  = TYPE_POWERNV_MACHINE,
> -.parent= TYPE_MACHINE,
> -.instance_size = sizeof(PnvMachineState),
> -.instance_init = powernv_machine_initfn,
> -.class_init= powernv_machine_class_init,
> -.interfaces = (InterfaceInfo[]) {
> -{ TYPE_XICS_FABRIC },
> -{ TYPE_INTERRUPT_STATS_PROVIDER },
> -{ },
> +#define DEFINE_PNV_CHIP_TYPE(type, class_initfn) \
> +{\
> +.name  = type,   \
> +.class_init= class_initfn,   \
> +.parent= TYPE_PNV_CHIP,  \
> +}
> +
> +static const TypeInfo types[] = {
> +{
> +.name  = TYPE_POWERNV_MACHINE,
> +.parent= TYPE_MACHINE,
> +.instance_size = sizeof(PnvMachineState),
> +.instance_init = powernv_machine_initfn,
> +.class_init= powernv_machine_class_init,
> +.interfaces = (InterfaceInfo[]) {
> +{ TYPE_XICS_FABRIC },
> +{ TYPE_INTERRUPT_STATS_PROVIDER },
> +{ },
> +},
>  },
> +{
> +.name  = TYPE_PNV_CHIP,
> +.parent= TYPE_SYS_BUS_DEVICE,
> +.class_init= pnv_chip_class_init,
> +.instance_init = pnv_chip_init,
> +.instance_size = sizeof(PnvChip),
> +.class_size= sizeof(PnvChipClass),
> +.abstract  = true,
> +},
> +DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER9, pnv_chip_power9_class_init),
> +DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
> +DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
> +DEFINE_PNV_CHIP_TYPE(TYPE_PNV_CH

Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/23] ppc: pnv: drop PnvCoreClass::cpu_oc field

[Cédric Le Goater]

On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> deduce cpu type directly from core type instead of
> maintaining type mapping in PnvCoreClass::cpu_oc and doing
> extra cpu_model parsing in pnv_core_class_init()
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Cédric Le Goater 

Thanks,

C.

> ---
>  include/hw/ppc/pnv_core.h |  1 -
>  hw/ppc/pnv_core.c | 18 --
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 3360c4b..a336a1f 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -42,7 +42,6 @@ typedef struct PnvCore {
>  
>  typedef struct PnvCoreClass {
>  DeviceClass parent_class;
> -ObjectClass *cpu_oc;
>  } PnvCoreClass;
>  
>  #define PNV_CORE_TYPE_SUFFIX "-" TYPE_PNV_CORE
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index b3e3f23..acdfa17 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -27,6 +27,16 @@
>  #include "hw/ppc/pnv_xscom.h"
>  #include "hw/ppc/xics.h"
>  
> +static const char *pvn_core_cpu_typename(PnvCore *pc)
> +{
> +const char *core_type = 
> object_class_get_name(object_get_class(OBJECT(pc)));
> +int len = strlen(core_type) - strlen(PNV_CORE_TYPE_SUFFIX);
> +char *s = g_strdup_printf(POWERPC_CPU_TYPE_NAME("%.*s"), len, core_type);
> +const char *cpu_type = object_class_get_name(object_class_by_name(s));
> +g_free(s);
> +return cpu_type;
> +}
> +
>  static void powernv_cpu_reset(void *opaque)
>  {
>  PowerPCCPU *cpu = opaque;
> @@ -148,8 +158,7 @@ static void pnv_core_realize(DeviceState *dev, Error 
> **errp)
>  {
>  PnvCore *pc = PNV_CORE(OBJECT(dev));
>  CPUCore *cc = CPU_CORE(OBJECT(dev));
> -PnvCoreClass *pcc = PNV_CORE_GET_CLASS(OBJECT(dev));
> -const char *typename = object_class_get_name(pcc->cpu_oc);
> +const char *typename = pvn_core_cpu_typename(pc);
>  size_t size = object_type_get_instance_size(typename);
>  Error *local_err = NULL;
>  void *obj;
> @@ -211,11 +220,9 @@ static Property pnv_core_properties[] = {
>  static void pnv_core_class_init(ObjectClass *oc, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(oc);
> -PnvCoreClass *pcc = PNV_CORE_CLASS(oc);
>  
>  dc->realize = pnv_core_realize;
>  dc->props = pnv_core_properties;
> -pcc->cpu_oc = cpu_class_by_name(TYPE_POWERPC_CPU, data);
>  }
>  
>  static const TypeInfo pnv_core_info = {
> @@ -223,6 +230,7 @@ static const TypeInfo pnv_core_info = {
>  .parent = TYPE_CPU_CORE,
>  .instance_size  = sizeof(PnvCore),
>  .class_size = sizeof(PnvCoreClass),
> +.class_init = pnv_core_class_init,
>  .abstract   = true,
>  };
>  
> @@ -239,8 +247,6 @@ static void pnv_core_register_types(void)
>  TypeInfo ti = {
>  .parent = TYPE_PNV_CORE,
>  .instance_size = sizeof(PnvCore),
> -.class_init = pnv_core_class_init,
> -.class_data = (void *) pnv_core_models[i],
>  };
>  ti.name = pnv_core_typename(pnv_core_models[i]);
>  type_register(&ti);
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 21/23] ppc: pnv: define core types statically

[Cédric Le Goater]

On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> pnv core type definition doesn't have any fields that
> require it to be defined at runtime. So replace code
> that fills in TypeInfo at runtime with static TypeInfo
> array that does the same at complie time.

This is much better.

> Signed-off-by: Igor Mammedov 

Reviewed-by: Cédric Le Goater 

Thanks,

C.

> ---
>  hw/ppc/pnv_core.c | 48 
>  1 file changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index acdfa17..000c87e 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -225,38 +225,30 @@ static void pnv_core_class_init(ObjectClass *oc, void 
> *data)
>  dc->props = pnv_core_properties;
>  }
>  
> -static const TypeInfo pnv_core_info = {
> -.name   = TYPE_PNV_CORE,
> -.parent = TYPE_CPU_CORE,
> -.instance_size  = sizeof(PnvCore),
> -.class_size = sizeof(PnvCoreClass),
> -.class_init = pnv_core_class_init,
> -.abstract   = true,
> -};
> -
> -static const char *pnv_core_models[] = {
> -"power8e_v2.1", "power8_v2.0", "power8nvl_v1.0", "power9_v1.0"
> -};
> -
> -static void pnv_core_register_types(void)
> -{
> -int i ;
> -
> -type_register_static(&pnv_core_info);
> -for (i = 0; i < ARRAY_SIZE(pnv_core_models); ++i) {
> -TypeInfo ti = {
> -.parent = TYPE_PNV_CORE,
> -.instance_size = sizeof(PnvCore),
> -};
> -ti.name = pnv_core_typename(pnv_core_models[i]);
> -type_register(&ti);
> -g_free((void *)ti.name);
> +#define DEFINE_PNV_CORE_TYPE(cpu_model) \
> +{   \
> +.parent = TYPE_PNV_CORE,\
> +.name = PNV_CORE_TYPE_NAME(cpu_model),  \
>  }
> -}
>  
> -type_init(pnv_core_register_types)
> +static const TypeInfo pnv_core_infos[] = {
> +{
> +.name   = TYPE_PNV_CORE,
> +.parent = TYPE_CPU_CORE,
> +.instance_size  = sizeof(PnvCore),
> +.class_size = sizeof(PnvCoreClass),
> +.class_init = pnv_core_class_init,
> +.abstract   = true,
> +},
> +DEFINE_PNV_CORE_TYPE("power8e_v2.1"),
> +DEFINE_PNV_CORE_TYPE("power8_v2.0"),
> +DEFINE_PNV_CORE_TYPE("power8nvl_v1.0"),
> +DEFINE_PNV_CORE_TYPE("power9_v1.0"),
> +};
>  
>  char *pnv_core_typename(const char *model)
>  {
>  return g_strdup_printf(PNV_CORE_TYPE_NAME("%s"), model);
>  }
> +
> +DEFINE_TYPES(pnv_core_infos)
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 19/23] ppc: pnv: normalize core/chip type names

[Cédric Le Goater]

On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> typically for cpus/core type names following convention is used
> 
>new_type_prefix-superclass_typename
> 
> make PNV core/chip to follow common convention.
> 
> Signed-off-by: Igor Mammedov 


Reviewed-by: Cédric Le Goater 

Thanks,

C.

> ---
>  include/hw/ppc/pnv.h  | 11 +++
>  include/hw/ppc/pnv_core.h |  2 ++
>  hw/ppc/pnv.c  |  2 +-
>  hw/ppc/pnv_core.c |  2 +-
>  4 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 2525f7f..d82eee1 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -80,19 +80,22 @@ typedef struct PnvChipClass {
>  uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
> -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
> +#define PNV_CHIP_TYPE_SUFFIX "-" TYPE_PNV_CHIP
> +#define PNV_CHIP_TYPE_NAME(cpu_model) cpu_model PNV_CHIP_TYPE_SUFFIX
> +
> +#define TYPE_PNV_CHIP_POWER8E PNV_CHIP_TYPE_NAME("power8e_v2.1")
>  #define PNV_CHIP_POWER8E(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
>  
> -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
> +#define TYPE_PNV_CHIP_POWER8 PNV_CHIP_TYPE_NAME("power8_v2.0")
>  #define PNV_CHIP_POWER8(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
>  
> -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
> +#define TYPE_PNV_CHIP_POWER8NVL PNV_CHIP_TYPE_NAME("power8nvl_v1.0")
>  #define PNV_CHIP_POWER8NVL(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
>  
> -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"
> +#define TYPE_PNV_CHIP_POWER9 PNV_CHIP_TYPE_NAME("power9_v1.0")
>  #define PNV_CHIP_POWER9(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
>  
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 2955a41..3360c4b 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -45,6 +45,8 @@ typedef struct PnvCoreClass {
>  ObjectClass *cpu_oc;
>  } PnvCoreClass;
>  
> +#define PNV_CORE_TYPE_SUFFIX "-" TYPE_PNV_CORE
> +#define PNV_CORE_TYPE_NAME(cpu_model) cpu_model PNV_CORE_TYPE_SUFFIX
>  extern char *pnv_core_typename(const char *model);
>  
>  #endif /* _PPC_PNV_CORE_H */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 4169837..9c5eb7c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -609,7 +609,7 @@ static void ppc_powernv_init(MachineState *machine)
>  
>  /* Create the processor chips */
>  i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> -chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%.*s",
> +chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
>  i, machine->cpu_type);
>  if (!object_class_by_name(chip_typename)) {
>  error_report("invalid CPU model '%.*s' for %s machine",
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 44b0b24..b3e3f23 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -252,5 +252,5 @@ type_init(pnv_core_register_types)
>  
>  char *pnv_core_typename(const char *model)
>  {
> -return g_strdup_printf(TYPE_PNV_CORE "-%s", model);
> +return g_strdup_printf(PNV_CORE_TYPE_NAME("%s"), model);
>  }
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 18/23] ppc: pnv: use generic cpu_model parsing

[Cédric Le Goater]

On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> use common cpu_model prasing in vl.c and set default cpu_model
> using generic MachineClass::default_cpu_type.
> 
> Beside of switching to generic infrastructure it solves several
> issues.
> 
>  * ppc_cpu_class_by_name() is used to deal with lower/upper case
>and alias translations into actual cpu type, which fixes
> '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
>usecases which error out with:
> 'invalid CPU model 'FOO' for powernv machine'
>  * allows to switch to lower-case typenames in pnv chip/core name
>(by convention typnames should be lower-case)
>  * replace aliased names /power8, power9, .../ with exact cpu model
>names (i.e. typenames should be stable but aliases might decide to
>point to other cpu model withi family or changed by kvm). It will
>also help to simplify pnv_chip/core code and get rid of dependency
>on cpu_model parsing.
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: Cédric Le Goater 

Thanks,

C.


> ---
>  include/hw/ppc/pnv.h |  8 
>  hw/ppc/pnv.c | 22 ++
>  hw/ppc/pnv_core.c|  2 +-
>  3 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 9c5437d..2525f7f 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
>  uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
> -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
>  #define PNV_CHIP_POWER8E(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
>  
> -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
>  #define PNV_CHIP_POWER8(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
>  
> -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
>  #define PNV_CHIP_POWER8NVL(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
>  
> -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"
>  #define PNV_CHIP_POWER9(obj) \
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index d46d91c..4169837 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -607,16 +607,13 @@ static void ppc_powernv_init(MachineState *machine)
>  }
>  }
>  
> -/* We need some cpu model to instantiate the PnvChip class */
> -if (machine->cpu_model == NULL) {
> -machine->cpu_model = "POWER8";
> -}
> -
>  /* Create the processor chips */
> -chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_model);
> +i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> +chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%.*s",
> +i, machine->cpu_type);
>  if (!object_class_by_name(chip_typename)) {
> -error_report("invalid CPU model '%s' for %s machine",
> - machine->cpu_model, MACHINE_GET_CLASS(machine)->name);
> +error_report("invalid CPU model '%.*s' for %s machine",
> + i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
>  exit(1);
>  }
>  
> @@ -716,7 +713,7 @@ static void pnv_chip_power8e_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "POWER8E";
> +k->cpu_model = "power8e_v2.1";
>  k->chip_type = PNV_CHIP_POWER8E;
>  k->chip_cfam_id = 0x221ef0498000ull;  /* P8 Murano DD2.1 */
>  k->cores_mask = POWER8E_CORE_MASK;
> @@ -738,7 +735,7 @@ static void pnv_chip_power8_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "POWER8";
> +k->cpu_model = "power8_v2.0";
>  k->chip_type = PNV_CHIP_POWER8;
>  k->chip_cfam_id = 0x220ea0498000ull; /* P8 Venice DD2.0 */
>  k->cores_mask = POWER8_CORE_MASK;
> @@ -760,7 +757,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "POWER8NVL";
> +k->cpu_model = "power8nvl_v1.0";
>  k->chip_type = PNV_CHIP_POWER8NVL;
>  k->chip_cfam_id = 0x120d30498000ull;  /* P8 Naples DD1.0 */
>  k->cores_mask = POWER8_CORE_MASK;
> @@ -782,7 +779,7 @@ static void pnv_chip_power9_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -k->cpu_model = "POWER9";
> +k->cpu_model = "power9_v1.0";
>  k->chip_type

Re: [Qemu-devel] [PATCH 0/2] disable the decrementer interrupt when a CPU is unplugged

[Cédric Le Goater]

On 10/06/2017 08:10 AM, Nikunj A Dadhania wrote:
> Cédric Le Goater  writes:
> 
>> Hello,
>>
>> When a CPU is stopped with the 'stop-self' RTAS call, its state
>> 'halted' is switched to 1 and, in this case, the MSR is not taken into
>> account anymore in the cpu_has_work() routine. Only the pending
>> hardware interrupts are checked with their LPCR:PECE* enablement bit.
>>
>> If the DECR timer fires after 'stop-self' is called and before the CPU
>> 'stop' state is reached, the nearly-dead CPU will have some work to do
>> and the guest will crash. This case happens very frequently with the
>> not yet upstream P9 XIVE exploitation mode. In XICS mode, the DECR is
>> occasionally fired but after 'stop' state, so no work is to be done
>> and the guest survives.
>>
>> I suspect there is a race between the QEMU mainloop triggering the
>> timers and the TCG CPU thread but I could not quite identify the root
>> cause. To be safe, let's disable the decrementer interrupt in the LPCR
>> when the CPU is halted and reenable it when the CPU is restarted.
> 
> Moreover, disabling the DECR in the reset path solves the TCG multi cpu
> reboot case, as reboot path does not call stop-cpu rtas call.

yes. I was going to restart the thread on the topic. 

Let's how these two little patches are discussed. Then we/you can 
resend the missing hunk in reset which is needed to perform a TCG 
reboot.

Thanks,  

C.


> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 3e20b1d886..c5150ee590 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -86,6 +86,15 @@ static void spapr_cpu_reset(void *opaque)
>  cs->halted = 1;
>  
>  env->spr[SPR_HIOR] = 0;
> +/* Disable DECR for secondary cpus */
> +if (cs != first_cpu) {
> +if (env->mmu_model == POWERPC_MMU_3_00) {
> +env->spr[SPR_LPCR] &= ~LPCR_DEE;
> +} else {
> +/* P7 and P8 both have same bit for DECR */
> +env->spr[SPR_LPCR] &= ~LPCR_P8_PECE3;
> +}
> +}
>  }
>  
>  static void spapr_cpu_destroy(PowerPCCPU *cpu)
> 
> 
> Regards
> Nikunj
> 

Re: [Qemu-devel] [PATCH 0/2] disable the decrementer interrupt when a CPU is unplugged

[Nikunj A Dadhania]

Cédric Le Goater  writes:

> Hello,
>
> When a CPU is stopped with the 'stop-self' RTAS call, its state
> 'halted' is switched to 1 and, in this case, the MSR is not taken into
> account anymore in the cpu_has_work() routine. Only the pending
> hardware interrupts are checked with their LPCR:PECE* enablement bit.
>
> If the DECR timer fires after 'stop-self' is called and before the CPU
> 'stop' state is reached, the nearly-dead CPU will have some work to do
> and the guest will crash. This case happens very frequently with the
> not yet upstream P9 XIVE exploitation mode. In XICS mode, the DECR is
> occasionally fired but after 'stop' state, so no work is to be done
> and the guest survives.
>
> I suspect there is a race between the QEMU mainloop triggering the
> timers and the TCG CPU thread but I could not quite identify the root
> cause. To be safe, let's disable the decrementer interrupt in the LPCR
> when the CPU is halted and reenable it when the CPU is restarted.

Moreover, disabling the DECR in the reset path solves the TCG multi cpu
reboot case, as reboot path does not call stop-cpu rtas call.

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 3e20b1d886..c5150ee590 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -86,6 +86,15 @@ static void spapr_cpu_reset(void *opaque)
 cs->halted = 1;
 
 env->spr[SPR_HIOR] = 0;
+/* Disable DECR for secondary cpus */
+if (cs != first_cpu) {
+if (env->mmu_model == POWERPC_MMU_3_00) {
+env->spr[SPR_LPCR] &= ~LPCR_DEE;
+} else {
+/* P7 and P8 both have same bit for DECR */
+env->spr[SPR_LPCR] &= ~LPCR_P8_PECE3;
+}
+}
 }
 
 static void spapr_cpu_destroy(PowerPCCPU *cpu)


Regards
Nikunj

Re: [Qemu-devel] [PATCH v2 0/4] blockjobs: add explicit job reaping

[Markus Armbruster]

Quick drive-by comment:

Kevin Wolf  writes:

[...]
> Let me try to just consolidate all of the above into a single state
> machine:
>
> 1.  CREATED --> RUNNING
> driver callback: .start
> 2a. RUNNING --> READY | CANCELLED
> via: auto transition (when bulk copy is finished) / block-job-cancel
> event: BLOCK_JOB_READY
> 2b. READY --> READY (COMPLETING) | READY (CANCELLING)
> via: block-job-complete / block-job-cancel
> event: none
> driver callback: .complete / none
> 3.  READY (CANCELLING | COMPLETING) --> DONE
> via: auto transition
>  (CANCELLING: after draining in-flight mirror requests;
>   COMPLETING: when images are in sync)
> event: BLOCK_JOB_DONE
> 4.  DONE --> PENDING
> via: auto transition (all jobs in the transaction are DONE)
> event: BLOCK_JOB_PENDING
> 5.  PENDING --> FINISHED
> via: block-job-finalize
> event: COMPLETED | CANCELLED
> driver callback: .prepare_finalize / .commit / .abort
> 6.  FINISHED --> NULL
> via: block-job-reap
> event: none
> driver callback: .clean
>
> I removed COMPLETED/CANCELLED states because they are never externally
> visible. You proposed an "auto transition" there, but the transition
> would be immediately after the previous one, so clients always see
> PENDING --> NULL | FINISHED.
>
> We would have two booleans to make explicit transition automatically:
>
> auto-finalize for block-job-finalize (default: true)
> auto-reap for block-job-reap (default: true)

Are we *sure* we need to quadruple the test matrix?

What exactly is the use case for either of these two flags?

> Both of them would be executed automatically as soon as the respective
> commands would be available.
>
> We could add more auto-* options for the remaining explicit transition

*groan*

> (block-job-complete/cancel in READY), but these are not important for
> the problems we're trying to solve here. They might become interesting
> if we do decide that we want a single copy block job instead of doing
> similar things in mirror, commit and backup.
> The naming needs some improvements (done -> pending -> finished looks
> really odd), but does this make sense otherwise?
>
> Kevin

Re: [Qemu-devel] [PULL 3/5] hmp-commands-info: Fix "info rocker-FOO" misspellings

[Markus Armbruster]

Paolo Bonzini  writes:

> On 05/10/2017 12:51, Dr. David Alan Gilbert (git) wrote:
>> From: Markus Armbruster 
>> 
>> Screwed up in commit da76ee7.
>
> Let me introduce you to these two aliases:
>
> whatis = "show -s --pretty='tformat:%h (\"%s\", %cd)' --date=short"
> pwhatis = "show -s --pretty='tformat:%h, \"%s\", %cd' --date=short"
>
> $ git whatis da76ee7
> da76ee76f7 ("hmp-commands-info: move info_cmds content out of monitor.c", 
> 2015-09-16)
>
> pwhatis is more appropriate if you are including the commit at the end
> of a parenthetical remark, like
>
>... (see commit message for 53ec73e, "block: Use bdrv_drain to
>replace uncessary bdrv_drain_all", 2015-07-07).

Heh, nice way to call out my laziness ;)  Thanks!

[Qemu-devel] [PATCH v4] target/ppc: Fix carry flag setting for shift algebraic instructions

[Sandipan Das]

For POWER ISA v3.0, the XER bit CA32 needs to be set by the shift
right algebraic instructions whenever the CA bit is to be set. This
change affects the following instructions:
  * Shift Right Algebraic Word (sraw[.])
  * Shift Right Algebraic Word Immediate (srawi[.])
  * Shift Right Algebraic Doubleword (srad[.])
  * Shift Right Algebraic Doubleword Immediate (sradi[.])

Signed-off-by: Sandipan Das 
---
v2: Add tcg_temp_free() required in gen_sraw() and gen_srad()

v3: Remove explicit checking for ISA v3.0 when setting CA32

v4: Set CA32 only when CA is being modified (as Richard suggested)
Set CA32 after checking for ISA300 in gen_* functions (as David suggested)
---
 target/ppc/int_helper.c | 16 
 target/ppc/translate.c  | 12 
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index da4e1a62c9..1c013a0ee3 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -219,17 +219,17 @@ target_ulong helper_sraw(CPUPPCState *env, target_ulong 
value,
 shift &= 0x1f;
 ret = (int32_t)value >> shift;
 if (likely(ret >= 0 || (value & ((1 << shift) - 1)) == 0)) {
-env->ca = 0;
+env->ca32 = env->ca = 0;
 } else {
-env->ca = 1;
+env->ca32 = env->ca = 1;
 }
 } else {
 ret = (int32_t)value;
-env->ca = 0;
+env->ca32 = env->ca = 0;
 }
 } else {
 ret = (int32_t)value >> 31;
-env->ca = (ret != 0);
+env->ca32 = env->ca = (ret != 0);
 }
 return (target_long)ret;
 }
@@ -245,17 +245,17 @@ target_ulong helper_srad(CPUPPCState *env, target_ulong 
value,
 shift &= 0x3f;
 ret = (int64_t)value >> shift;
 if (likely(ret >= 0 || (value & ((1ULL << shift) - 1)) == 0)) {
-env->ca = 0;
+env->ca32 = env->ca = 0;
 } else {
-env->ca = 1;
+env->ca32 = env->ca = 1;
 }
 } else {
 ret = (int64_t)value;
-env->ca = 0;
+env->ca32 = env->ca = 0;
 }
 } else {
 ret = (int64_t)value >> 63;
-env->ca = (ret != 0);
+env->ca32 = env->ca = (ret != 0);
 }
 return ret;
 }
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 606b605ba0..a81ff69d75 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2181,6 +2181,9 @@ static void gen_srawi(DisasContext *ctx)
 if (sh == 0) {
 tcg_gen_ext32s_tl(dst, src);
 tcg_gen_movi_tl(cpu_ca, 0);
+if (is_isa300(ctx)) {
+tcg_gen_movi_tl(cpu_ca32, 0);
+}
 } else {
 TCGv t0;
 tcg_gen_ext32s_tl(dst, src);
@@ -2190,6 +2193,9 @@ static void gen_srawi(DisasContext *ctx)
 tcg_gen_and_tl(cpu_ca, cpu_ca, t0);
 tcg_temp_free(t0);
 tcg_gen_setcondi_tl(TCG_COND_NE, cpu_ca, cpu_ca, 0);
+if (is_isa300(ctx)) {
+tcg_gen_mov_tl(cpu_ca32, cpu_ca);
+}
 tcg_gen_sari_tl(dst, dst, sh);
 }
 if (unlikely(Rc(ctx->opcode) != 0)) {
@@ -2259,6 +2265,9 @@ static inline void gen_sradi(DisasContext *ctx, int n)
 if (sh == 0) {
 tcg_gen_mov_tl(dst, src);
 tcg_gen_movi_tl(cpu_ca, 0);
+if (is_isa300(ctx)) {
+tcg_gen_movi_tl(cpu_ca32, 0);
+}
 } else {
 TCGv t0;
 tcg_gen_andi_tl(cpu_ca, src, (1ULL << sh) - 1);
@@ -2267,6 +2276,9 @@ static inline void gen_sradi(DisasContext *ctx, int n)
 tcg_gen_and_tl(cpu_ca, cpu_ca, t0);
 tcg_temp_free(t0);
 tcg_gen_setcondi_tl(TCG_COND_NE, cpu_ca, cpu_ca, 0);
+if (is_isa300(ctx)) {
+tcg_gen_mov_tl(cpu_ca32, cpu_ca);
+}
 tcg_gen_sari_tl(dst, src, sh);
 }
 if (unlikely(Rc(ctx->opcode) != 0)) {
-- 
2.13.6

Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command

[Markus Armbruster]

Jan Dakinevich  writes:

> On 10/03/2017 05:02 PM, Eric Blake wrote:
>> On 10/03/2017 07:47 AM, Jan Dakinevich wrote:
>>> The command is intended for gathering virtio information such as status,
>>> feature bits, negotiation status. It is convenient and useful for debug
>>> purpose.
>>>
>>> The commands returns generic virtio information for virtio such as
>>> common feature names and status bits names and information for all
>>> attached to current machine devices.
>>>
>>> To retrieve names of device-specific features `get_feature_name'
>>> callback in VirtioDeviceClass also was introduced.
>>>
>>> Cc: Denis V. Lunev 
>>> Signed-off-by: Jan Dakinevich 
>>> ---
>>>  hw/block/virtio-blk.c   |  21 +
>>>  hw/char/virtio-serial-bus.c |  15 +++
>>>  hw/display/virtio-gpu.c |  13 ++
>>>  hw/net/virtio-net.c |  35 +++
>>>  hw/scsi/virtio-scsi.c   |  16 +++
>>>  hw/virtio/Makefile.objs |   2 +
>>>  hw/virtio/virtio-balloon.c  |  15 +++
>>>  hw/virtio/virtio-stub.c |   9 
>>>  hw/virtio/virtio.c  | 101 
>>> 
>>>  include/hw/virtio/virtio.h  |   2 +
>>>  qapi-schema.json|   1 +
>>>  qapi/virtio.json|  94 +
>>>  12 files changed, 324 insertions(+)
>>>  create mode 100644 hw/virtio/virtio-stub.c
>>>  create mode 100644 qapi/virtio.json
>> 
>> This creates a new .json file, but does not touch MAINTAINERS.  Our idea
>> in splitting the .json files was to make it easier for each sub-file
>> that needs a specific maintainer in addition to the overall *.json line
>> for QAPI maintainers, so this may deserve a MAINTAINERS entry.
>> 
>
> Ok.
>
>>> +++ b/qapi/virtio.json
>>> @@ -0,0 +1,94 @@
>>> +# -*- Mode: Python -*-
>>> +#
>>> +
>>> +##
>>> +# = Virtio devices
>>> +##
>>> +
>>> +{ 'include': 'common.json' }
>>> +
>>> +##
>>> +# @VirtioInfoBit:
>>> +#
>>> +# Named virtio bit
>>> +#
>>> +# @bit: bit number
>>> +#
>>> +# @name: bit name
>>> +#
>>> +# Since: 2.11.0
>>> +#
>>> +##
>>> +{
>>> +'struct': 'VirtioInfoBit',
>>> +'data': {
>>> +'bit': 'uint64',
>> 
>> Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8,
>> ...?  The documentation on 'bit number' is rather sparse.
>
> I would prefer `uint' here, but I don't see generic unsigned type (may
> be, I am mistaken). I could use uint8 here, though.
>
>> 
>>> +'name': 'str'
>> 
>> Wouldn't an enum type be better than an open-ended string?
>> 
>
> Bit names are not known here, they are obtained from virtio device
> implementations.

What exactly uses these bits?

Why do these uses justify pass-through?  By pass-through, I mean the
messenger (QEMU) merely passes them along, without understanding them.
Defeats introspection.

>>> +}
>>> +}
>>> +
>>> +##
>>> +# @VirtioInfoDevice:
>>> +#
>>> +# Information about specific virtio device
>>> +#
>>> +# @qom_path: QOM path of the device
>> 
>> Please make this 'qom-path' - new interfaces should prefer '-' over '_'.
>
> Ok.
>
>>> +#
>>> +# @feature-names: names of device-specific features
>>> +#
>>> +# @host-features: bitmask of features, provided by devices
>>> +#
>>> +# @guest-features: bitmask of features, acknowledged by guest
>>> +#
>>> +# @status: virtio device status bitmask
>>> +#
>>> +# Since: 2.11.0
>>> +#
>>> +##
>>> +{
>>> +'struct': 'VirtioInfoDevice',
>>> +'data': {
>>> +'qom_path': 'str',
>>> +'feature-names': ['VirtioInfoBit'],
>>> +'host-features': 'uint64',
>>> +'guest-features': 'uint64',
>>> +'status': 'uint64'
>> 
>> I'm wondering if this is the best representation (where the caller has
>> to parse the integer and then lookup in feature-names what each bit of
>> the integer represents).  But I'm not sure I have anything better off
>> the top of my head.
>> 
>
> Consider it as way to tell caller about names of supported features.

"Unsigned integer interpreted as combination of well-known bit-valued
symbols" is a fine C interface, but a pretty horrid QMP interface.
What's wrong with doing a set the straightforward way as "array of
enum"?

>>> +}
>>> +}
>>> +
>>> +##
>>> +# @VirtioInfo:
>>> +#
>>> +# Information about virtio devices
>>> +#
>>> +# @feature-names: names of common virtio features
>>> +#
>>> +# @status-names: names of bits which represents virtio device status
>>> +#
>>> +# @devices: list of per-device virtio information
>>> +#
>>> +# Since: 2.11.0
>>> +#
>>> +##
>>> +{
>>> +'struct': 'VirtioInfo',
>>> +'data': {
>>> +'feature-names': ['VirtioInfoBit'],
>> 
>> Why is feature-names listed at two different nestings of the return value?
>> 
>
> These are different feature names. First names are common and predefined
> for all devices. Second names are device-specific.
>
>>> +'status-names': ['VirtioInfoBit'],
>>> +'devices': ['VirtioInfoDevice']
>>> +}
>>> +}
>>> +
>>> +
>>> +##
>>> +# @query-vi

Re: [Qemu-devel] [RFC PATCH 19/32] qapi: Accept double-quoted strings

[Markus Armbruster]

Marc-André Lureau  writes:

> On Thu, Oct 5, 2017 at 6:41 AM, Markus Armbruster  wrote:
>> Marc-André Lureau  writes:
>>
>>> On Mon, Oct 2, 2017 at 5:25 PM, Markus Armbruster  wrote:
 The QAPI schema parser has always accepted only single-quoted strings,
 even though JSON strings are double-quoted.  Accept double-quoted
 strings as well, so you can write strings containing single quotes
 without backslash escapes.

 Signed-off-by: Markus Armbruster 
>>>
>>> What's the motivation to allow both? If we were to switch from single
>>> to double quote only, that would make more sense.
>>
>> Abandoning single quotes now would require us to touch pretty much every
>> line of code in the schemas.  I don't think correcting quotes is worth
>> wrecking git-blame.
>>
>
> Recent (and upcoming) changes to the schema are already quite
> invasive. I think we could do it, convert all strings to double-quote,
> and it would help with getting the schema closer to a valid json.

Is the recent (and upcoming) churn *that* bad?  Got numbers?

> Fwiw, there are tools like
> https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html
> to skip commits in git-blame. It's also fairly easy to run git blame
> before the reformatting commit.

Both techniques add friction...

>> Sadly, the schema language is neither JSON, nor an established extension
>> of JSON, nor Python.  This commit brings the schema language one step
>> closer to a superset of JSON.  I feel "homegrown superset" is a slightly
>> less bad idea than "homegrown with large overlap".
>>
>> Naming the schema files .json was in bad taste.
>>
>>> otherwise, patch looks good
>>
>> Ready to upgrade to R-by now?
>>
>> Want me to work more of my rationale into the commit message?

Re: [Qemu-devel] [RFC PATCH 19/32] qapi: Accept double-quoted strings

[Markus Armbruster]

Eric Blake  writes:

> On 10/04/2017 11:41 PM, Markus Armbruster wrote:
>
>> Sadly, the schema language is neither JSON, nor an established extension
>> of JSON, nor Python.  This commit brings the schema language one step
>> closer to a superset of JSON.  I feel "homegrown superset" is a slightly
>> less bad idea than "homegrown with large overlap".
>> 
>> Naming the schema files .json was in bad taste.
>
> Would it make sense to rename all of our files from .json to .qapi?
> Then it is obvious that we are using a homegrown syntax; and it is also
> easy enough to tweak things like .dir-locals.el to recognize that suffix
> as triggering specific formatting rules.  Git rename detection means it
> is still reasonable to blame across file renames.

I don't know.  I'm always reluctant to rename files.  Probably too
reluctant.  Opinions?

Re: [Qemu-devel] [RFC PATCH 02/32] texi2pod: Support @verbatim environment

[Markus Armbruster]

Eric Blake  writes:

> On 10/02/2017 10:25 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster 
>> ---
>>  scripts/texi2pod.pl | 11 +--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> My perl is a bit rusty, but I think I can handle this one.
>
>> 
>> diff --git a/scripts/texi2pod.pl b/scripts/texi2pod.pl
>> index 39ce584a32..2171f8b819 100755
>> --- a/scripts/texi2pod.pl
>> +++ b/scripts/texi2pod.pl
>> @@ -85,6 +85,13 @@ if (defined $out) {
>>  
>>  while(defined $inf) {
>>  while(<$inf>) {
>> +# Verbatim environment
>> +if (defined $endw and $endw eq "verbatim"
>> +and not (/^\@end\s+([a-z]+)/ and $1 eq $endw)) {
>
> You anchored to the beginning, but not the end, of the line; that means
> you accept '@end verbatim garbage' as an end marker.  Worth adding $?

Cribbed from

# End-block handler goes up here because it needs to operate even
# if we are skipping.
/^\@end\s+([a-z]+)/ and do {
# Ignore @end foo, where foo is not an operation which may
# cause us to skip, if we are presently skipping.
my $ended = $1;
next if $skipping && $ended !~ 
/^(?:ifset|ifclear|ignore|menu|iftex|copying)$/;

die "\@end $ended without \@$ended at line $.\n" unless defined $endw;
die "\@$endw ended by \@end $ended at line $.\n" unless $ended eq $endw;

$endw = pop @endwstack;

I'd prefer to stick to this regexp.

texi2pod.pl parses quite sloppily in general.

> If that's the only change, I'm okay with adding:
>
> Reviewed-by: Eric Blake 

Does this apply to the unchanged patch, too?

Re: [Qemu-devel] [REBASED 0/2] exec: further refine address_space_get_iotlb_entry()

[Michael S. Tsirkin]

On Thu, Oct 05, 2017 at 07:13:07PM +0200, Maxime Coquelin wrote:
> This series is a rebase of the first two patches of Peter's series
> improving address_space_get_iotlb_entry():
> Message-Id: <1496404254-17429-1-git-send-email-pet...@redhat.com>
> 
> It is actually not only an improvement, but fixes a regression in the way
> IOTLB updates sent to the backends are generated.
> The regression is introduced by patch:
> a764040cc8 ("exec: abstract address_space_do_translate()")
> 
> Prior to this patch IOTLB entries sent to the backend were aligned on the
> guest page boundaries (both addresses and size).
> For example, with the guest using 2MB pages:
>  * Backend sends IOTLB miss request for iova = 0x112378fb4
>  * QEMU replies with an IOTLB update with iova = 0x11220, size = 0x20
>  * Bakend insert above entry in its cache and compute the translation
> In this case, if the backend needs later to translate 0x112378004, it will
> result in a cache it and no need to send another IOTLB miss.
> 
> With this patch, the addr of the IOTLB entry will be the address requested
> via the IOTLB miss, the size is computed to cover the remaining of the guest
> page.
> The same example gives:
>  * Backend sends IOTLB miss request for iova = 0x112378fb4
>  * QEMU replies with an IOTLB update with iova = 112378fb4, size = 0x8704c
>  * Bakend insert above entry in its cache and compute the translation
> In this case, if the backend needs later to translate 0x112378004, it will
> result in another cache miss:
>  * Backend sends IOTLB miss request for iova = 0x112378004
>  * QEMU replies with an IOTLB update with iova = 0x112378004, size = 0x87FFC
>  * Bakend insert above entry in its cache and compute the translation
> It results in having much more IOTLB misses, and more importantly it pollutes
> the device IOTLB cache by multiplying the number of entries that moreover
> overlap.
> 
> Note that current Kernel & User backends implementation do not merge 
> contiguous
> and overlapping IOTLB entries at device IOTLB cache insertion.
> 
> This series fixes this regression, so that IOTLB updates are aligned on
> guest's page boundaries.

Acked-by: Michael S. Tsirkin 


> Peter Xu (2):
>   exec: add page_mask for flatview_do_translate
>   exec: simplify address_space_get_iotlb_entry
> 
>  exec.c | 75 
> +++---
>  1 file changed, 49 insertions(+), 26 deletions(-)
> 
> -- 
> 2.13.6

Re: [Qemu-devel] [PATCH v2 0/4] blockjobs: add explicit job reaping

[John Snow]

On 10/05/2017 07:38 AM, Kevin Wolf wrote:
> Am 05.10.2017 um 03:46 hat John Snow geschrieben:
>> On 10/04/2017 02:27 PM, Kevin Wolf wrote:
>>> Am 04.10.2017 um 03:52 hat John Snow geschrieben:
 For jobs that complete when a monitor isn't looking, there's no way to
 tell what the job's final return code was. We need to allow jobs to
 remain in the list until queried for reliable management.
>>>
>>> Just a short summary of what I discussed with John on IRC:
>>>
>>> Another important reason why we want to have an explicit end of block
>>> jobs is that job completion often makes changes to the graph. For a
>>> management tool that manages the block graph on a node level, it is a
>>> big problem if graph changes can happen at any point that can lead to
>>> bad race conditions. Giving the management tool control over the end of
>>> the block job makes it aware that graph changes happen.
>>>
>>> This means that compared to this RFC series, we need to move the waiting
>>> earlier in the process:
>>>
>>> 1. Block job is done and calls block_job_completed()
>>> 2. Wait for other block jobs in the same job transaction to complete
>>> 3. Send a (new) QMP event to the management tool to notify it that the
>>>job is ready to be reaped
>>
>> Oh, I suppose to distinguish it from "COMPLETED" in that sense, because
>> it isn't actually COMPLETED anymore under your vision, so it requires a
>> new event in this proposal.
>>
>> This becomes a bit messy, bumping up against both "READY" and a
>> transactional pre-completed state semantically. U, for lack of a
>> better word in the timeframe I'd like to complete this email in, let's
>> call this new theoretical state "PENDING"?
>>
>> So presently, a job goes through the following life cycle:
>>
>> 1. CREATED --> RUNNING
>> 2. RUNNING <--> PAUSED
>> 3. RUNNING --> (READY | COMPLETED | CANCELED)
>> 4. READY --> (COMPLETED | CANCELED)
>> 5. (COMPLETED | CANCELED) --> NULL
>>
>> Where we emit an event upon entering "READY", "COMPLETED" or "CANCELED".
> 
> Roughly yes, but it's not quite true because you can still pause and
> unpause ready jobs. So READY and PAUSED are kind of orthogonal.
> 
>> My patchset here effectively adds a new optional terminal state:
>>
>> 5. (COMPLETED | CANCELED) --> (NULL | FINISHED)
>> 6. FINISHED --> NULL
>>
>> Where the last transition from FINISHED to NULL is performed via
>> block-job-reap, but notably we get to re-use the events for COMPLETED |
>> CANCELED to indicate the availability of this operation to be performed.
>>
>> What happens in the case of transactionally managed jobs presently is
>> that jobs get stuck as they enter the COMPLETED|CANCELED state. If you
>> were to query them they behave as if they're RUNNING. There's no
>> discrete state that exists for this presently.
>>
>> You can cancel these as normal, but I'm not sure if you can pause them,
>> actually. (Note to self, test that.) I think they have almost exactly
>> like any RUNNING job would.
> 
> Except that they don't do any work any more. This is an mportant
> difference for a mirror job which would normally keep copying new writes
> until it sends the COMPLETED event. So when libvirt restarts and it sees
> a "RUNNING" mirror job, it can't decide whether it is still copying
> things or has already completed.
> 
> Looks like this is another reason why we want a separate state here.
> 
>> What you're proposing here is the formalization of the pre-completion
>> state ("PENDING") and that in this state, a job outside of a transaction
>> can exist until it is manually told to finally, once and for all,
>> actually finish its business. We can use this as a hook to perform and
>> last graph changes so they will not come as a surprise to the management
>> application. Maybe this operation should be called "Finalize". Again,
>> for lack of a better term in the timeframe, I'll refer to it as such for
>> now.
> 
> "finalize" doesn't sound too bad.
> 
>> I think importantly this actually distinguishes it from "reap" in that
>> the commit phase can still fail, so we can't let the job follow that
>> auto transition back to the NULL state.
> 
> Let me see if I understand correctly: We want to make sure that the
> management tool sees the final return value for the job. We have already
> decided that events aren't enough for this because the management tool
> could be restarted while we send the event, so the information is lost.
> Having it as a return value of block-job-reap isn't enough either
> because it could be lost the same way. We need a separate phase where
> libvirt can query the return value and from which we don't automatically
> transition away.
> 
> I'm afraid that you are right.
> 
>> That means that we'd need both a block-job-finalize AND a
>> block-job-reap to accomplish both of the following goals:
>>
>> (1) Allow the management application to control graph changes [libvirt]
>> (2) Prevent auto transitions to NULL state for asynchronous clients [A
>> requi

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Bharat Bhushan]

> >> Thanks
> >>
> >> Eric
> >>>
> >>> However you should be allowed to map 1 sg element of 5 pages and
> >>> then notify the host about this event I think. Still looking at the 
> >>> code...
> >>>
> >>> I still can't reproduce the issue at the moment. What kind of device
> >>> are you assigning?
> >>>
> >>> Thanks
> >>>
> >>> Eric
> 
>  Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
>  expects the map size to be a power of 2.
> >
> > Actually I missed the most important here ;-)
> 
>   if (len & iotlb->addr_mask) {
> > This check looks suspiscious to me. In our case the len is not
> > modified by the previous translation and it fails, I don't see why. It
> > should be valid to be able to notify 5 granules.
> 
> So after discussion with Alex, looks the way we notify the host currently is
> wrong. we set the addr_mask to the mapping/unmapping size
> -1 whereas this should be a page mask instead (granule size or block size?).
> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
> page and not a single one. It is unclear to me if we can notify with
> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
> code I notify by granule or block size.
> 
> Bharat, please can you add this to your TODO list?
> 
> Linu, thanks a lot for the time you spent debugging this issue.
> Curiously on my side, it is really seldom hit but it is ...

Thanks Linu and Eric, I added this to my todo list.
While I am still not able to reproduce the issue.  I tried with e1000 and now 
try with ixgbe device. May I know which device can be used to reproduce this 
issue?

Thanks
-Bharat

> 
> Thanks!
> 
> Eric
> >
> > Thanks
> >
> > Eric
>  error_report("iommu has granularity incompatible with target 
>  AS");
>  return false;
>  }
> 
>  Just trying to understand how this is not hitting in your case.
> 
> 
> >>>
> >>
> >

Re: [Qemu-devel] [PATCH 06/23] ppc: mac_oldworld: use generic cpu_model parsing

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:33PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  hw/ppc/mac_oldworld.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index bc7c8b7..010ea36 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -108,11 +108,8 @@ static void ppc_heathrow_init(MachineState *machine)
>  linux_boot = (kernel_filename != NULL);
>  
>  /* init CPUs */
> -if (machine->cpu_model == NULL)
> -machine->cpu_model = "G3";
>  for (i = 0; i < smp_cpus; i++) {
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU,
> -   machine->cpu_model));
> +cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>  env = &cpu->env;
>  
>  /* Set time-base frequency to 16.6 Mhz */
> @@ -385,6 +382,7 @@ static void heathrow_class_init(ObjectClass *oc, void 
> *data)
>  /* TOFIX "cad" when Mac floppy is implemented */
>  mc->default_boot_order = "cd";
>  mc->kvm_type = heathrow_kvm_type;
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("750_v3.1");
>  }
>  
>  static const TypeInfo ppc_heathrow_machine_info = {

-- 
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 05/23] ppc: mac_newworld: use generic cpu_model parsing

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:32PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  hw/ppc/mac_newworld.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 6d0ace2..3fa7c42 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -174,16 +174,8 @@ static void ppc_core99_init(MachineState *machine)
>  linux_boot = (kernel_filename != NULL);
>  
>  /* init CPUs */
> -if (machine->cpu_model == NULL) {
> -#ifdef TARGET_PPC64
> -machine->cpu_model = "970fx";
> -#else
> -machine->cpu_model = "G4";
> -#endif
> -}
>  for (i = 0; i < smp_cpus; i++) {
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU,
> -   machine->cpu_model));
> +cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>  env = &cpu->env;
>  
>  /* Set time-base frequency to 100 Mhz */
> @@ -520,6 +512,11 @@ static void core99_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->max_cpus = MAX_CPUS;
>  mc->default_boot_order = "cd";
>  mc->kvm_type = core99_kvm_type;
> +#ifdef TARGET_PPC64
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
> +#else
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
> +#endif
>  }
>  
>  static const TypeInfo core99_machine_info = {

-- 
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/23] qom: add helper macro DEFINE_TYPES()

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:30PM +0200, Igor Mammedov wrote:
> DEFINE_TYPES() will help to simplify following routine patterns:
> 
>  static void foo_register_types(void)
>  {
> type_register_static(&foo1_type_info);
> type_register_static(&foo2_type_info);
> ...
>  }
> 
>  type_init(foo_register_types)
> 
> or
> 
>  static void foo_register_types(void)
>  {
> int i;
> 
> for (i = 0; i < ARRAY_SIZE(type_infos); i++) {
> type_register_static(&type_infos[i]);
> }
>  }
> 
>  type_init(foo_register_types)
> 
> with a single line
> 
>  DEFINE_TYPES(type_infos)
> 
> where types have static definition which could be consolidated in
> a single array of TypeInfo structures.
> It saves us ~6-10LOC per use case and would help to replace
> imperative foo_register_types() there with declarative style of
> type registration.
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Eduardo Habkost 
> Reviewed-by: Philippe Mathieu-Daudé 

Reviewed-by: David Gibson 

> ---
>  include/qom/object.h | 36 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 9a2369c..dc73d59 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -79,6 +79,28 @@ typedef struct InterfaceInfo InterfaceInfo;
>   * #TypeInfo describes information about the type including what it inherits
>   * from, the instance and class size, and constructor/destructor hooks.
>   *
> + * Alternatively several static types could be registered using helper macro
> + * DEFINE_TYPES()
> + *
> + * 
> + *   
> + * static const TypeInfo device_types_info[] = {
> + * {
> + * .name = TYPE_MY_DEVICE_A,
> + * .parent = TYPE_DEVICE,
> + * .instance_size = sizeof(MyDeviceA),
> + * },
> + * {
> + * .name = TYPE_MY_DEVICE_B,
> + * .parent = TYPE_DEVICE,
> + * .instance_size = sizeof(MyDeviceB),
> + * },
> + * };
> + *
> + * DEFINE_TYPES(device_types_info)
> + *   
> + * 
> + *
>   * Every type has an #ObjectClass associated with it.  #ObjectClass 
> derivatives
>   * are instantiated dynamically but there is only ever one instance for any
>   * given type.  The #ObjectClass typically holds a table of function pointers
> @@ -799,6 +821,20 @@ Type type_register(const TypeInfo *info);
>  void type_register_static_array(const TypeInfo *infos, int nr_infos);
>  
>  /**
> + * DEFINE_TYPES:
> + * @type_array: The array containing #TypeInfo structures to register
> + *
> + * @type_array should be static constant that exists for the life time
> + * that the type is registered.
> + */
> +#define DEFINE_TYPES(type_array)\
> +static void do_qemu_init_ ## type_array(void)   \
> +{   \
> +type_register_static_array(type_array, ARRAY_SIZE(type_array)); \
> +}   \
> +type_init(do_qemu_init_ ## type_array)
> +
> +/**
>   * object_class_dynamic_cast_assert:
>   * @klass: The #ObjectClass to attempt to cast.
>   * @typename: The QOM typename of the class to cast to.

-- 
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/23] qom: update doc comment for type_register[_static]()

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:28PM +0200, Igor Mammedov wrote:
> type_register()/type_register_static() functions in current impl.
> can't fail returning 0, also none of the users check for error
> so update doc comment to reflect current behaviour.
> 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Eduardo Habkost 

Reviewed-by: David Gibson 

> ---
>  include/qom/object.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index e0d9824..a707b67 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -773,7 +773,7 @@ const char *object_get_typename(const Object *obj);
>   * @info and all of the strings it points to should exist for the life time
>   * that the type is registered.
>   *
> - * Returns: 0 on failure, the new #Type on success.
> + * Returns: the new #Type.
>   */
>  Type type_register_static(const TypeInfo *info);
>  
> @@ -784,7 +784,7 @@ Type type_register_static(const TypeInfo *info);
>   * Unlike type_register_static(), this call does not require @info or its
>   * string members to continue to exist after the call returns.
>   *
> - * Returns: 0 on failure, the new #Type on success.
> + * Returns: the new #Type.
>   */
>  Type type_register(const TypeInfo *info);
>  

-- 
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 11/23] ppc: spapr: replace ppc_cpu_parse_features() with cpu_parse_cpu_model()

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:38PM +0200, Igor Mammedov wrote:
> ppc_cpu_parse_features() is doing practically the same thing as
> generic cpu_parse_cpu_model(). So remove duplicated impl. and
> reuse generic one.
> 
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  include/hw/ppc/ppc.h|  2 --
>  hw/ppc/ppc.c| 25 -
>  hw/ppc/spapr_cpu_core.c |  9 -
>  3 files changed, 4 insertions(+), 32 deletions(-)
> 
> diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
> index 4e7fe11..ff0ac30 100644
> --- a/include/hw/ppc/ppc.h
> +++ b/include/hw/ppc/ppc.h
> @@ -105,6 +105,4 @@ enum {
>  
>  /* ppc_booke.c */
>  void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
> -
> -void ppc_cpu_parse_features(const char *cpu_model);
>  #endif
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 05da316..7ec35de 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -1359,28 +1359,3 @@ void PPC_debug_write (void *opaque, uint32_t addr, 
> uint32_t val)
>  break;
>  }
>  }
> -
> -void ppc_cpu_parse_features(const char *cpu_model)
> -{
> -CPUClass *cc;
> -ObjectClass *oc;
> -const char *typename;
> -gchar **model_pieces;
> -
> -model_pieces = g_strsplit(cpu_model, ",", 2);
> -if (!model_pieces[0]) {
> -error_report("Invalid/empty CPU model name");
> -exit(1);
> -}
> -
> -oc = cpu_class_by_name(TYPE_POWERPC_CPU, model_pieces[0]);
> -if (oc == NULL) {
> -error_report("Unable to find CPU definition: %s", model_pieces[0]);
> -exit(1);
> -}
> -
> -typename = object_class_get_name(oc);
> -cc = CPU_CLASS(oc);
> -cc->parse_features(typename, model_pieces[1], &error_fatal);
> -g_strfreev(model_pieces);
> -}
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 3e20b1d..3dea5ff 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -34,6 +34,7 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>   *   before passing it on to the cpu level parser.
>   */
>  gchar **inpieces;
> +gchar *newprops;
>  int i, j;
>  gchar *compat_str = NULL;
>  
> @@ -58,17 +59,15 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>  
>  if (compat_str) {
>  char *val = compat_str + strlen("compat=");
> -gchar *newprops = g_strjoinv(",", inpieces);
>  
>  object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
>  &error_fatal);
>  
> -ppc_cpu_parse_features(newprops);
> -g_free(newprops);
> -} else {
> -ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
>  }
>  
> +newprops = g_strjoinv(",", inpieces);
> +cpu_parse_cpu_model(TYPE_POWERPC_CPU, newprops);
> +g_free(newprops);
>  g_strfreev(inpieces);
>  }
>  

-- 
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 10/23] ppc: 40p/prep: replace cpu_model with cpu_type

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:37PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  hw/ppc/prep.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 94138a4..6f8accc 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -517,11 +517,8 @@ static void ppc_prep_init(MachineState *machine)
>  linux_boot = (kernel_filename != NULL);
>  
>  /* init CPUs */
> -if (machine->cpu_model == NULL)
> -machine->cpu_model = "602";
>  for (i = 0; i < smp_cpus; i++) {
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU,
> -   machine->cpu_model));
> +cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>  env = &cpu->env;
>  
>  if (env->flags & POWERPC_FLAG_RTC_CLK) {
> @@ -684,6 +681,7 @@ static void prep_machine_init(MachineClass *mc)
>  mc->block_default_type = IF_IDE;
>  mc->max_cpus = MAX_CPUS;
>  mc->default_boot_order = "cad";
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("602");
>  }
>  
>  static int prep_set_cmos_checksum(DeviceState *dev, void *opaque)
> @@ -718,10 +716,7 @@ static void ibm_40p_init(MachineState *machine)
>  char boot_device;
>  
>  /* init CPU */
> -if (!machine->cpu_model) {
> -machine->cpu_model = "604";
> -}
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, 
> machine->cpu_model));
> +cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>  env = &cpu->env;
>  if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
>  error_report("only 6xx bus is supported on this machine");
> @@ -894,6 +889,7 @@ static void ibm_40p_machine_init(MachineClass *mc)
>  mc->default_ram_size = 128 * M_BYTE;
>  mc->block_default_type = IF_SCSI;
>  mc->default_boot_order = "c";
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("604");
>  }
>  
>  DEFINE_MACHINE("40p", ibm_40p_machine_init)

-- 
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 08/23] ppc: replace cpu_model with cpu_type on ref405ep, taihu boards

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:35PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  hw/ppc/ppc405_uc.c   | 6 --
>  hw/ppc/ppc4xx_devs.c | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index 8e58065..205ebce 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -1629,7 +1629,8 @@ CPUPPCState *ppc405cr_init(MemoryRegion 
> *address_space_mem,
>  qemu_irq *pic, *irqs;
>  
>  memset(clk_setup, 0, sizeof(clk_setup));
> -cpu = ppc4xx_init("405cr", &clk_setup[PPC405CR_CPU_CLK],
> +cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405crc"),
> +  &clk_setup[PPC405CR_CPU_CLK],
>&clk_setup[PPC405CR_TMR_CLK], sysclk);
>  env = &cpu->env;
>  /* Memory mapped devices registers */
> @@ -1981,7 +1982,8 @@ CPUPPCState *ppc405ep_init(MemoryRegion 
> *address_space_mem,
>  
>  memset(clk_setup, 0, sizeof(clk_setup));
>  /* init CPUs */
> -cpu = ppc4xx_init("405ep", &clk_setup[PPC405EP_CPU_CLK],
> +cpu = ppc4xx_init(POWERPC_CPU_TYPE_NAME("405ep"),
> +  &clk_setup[PPC405EP_CPU_CLK],
>&tlb_clk_setup, sysclk);
>  env = &cpu->env;
>  clk_setup[PPC405EP_CPU_CLK].cb = tlb_clk_setup.cb;
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index 6d7f785..2e96389 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -48,7 +48,7 @@ static void ppc4xx_reset(void *opaque)
>  
>  
> /*/
>  /* Generic PowerPC 4xx processor instantiation */
> -PowerPCCPU *ppc4xx_init(const char *cpu_model,
> +PowerPCCPU *ppc4xx_init(const char *cpu_type,
>  clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
>  uint32_t sysclk)
>  {
> @@ -56,7 +56,7 @@ PowerPCCPU *ppc4xx_init(const char *cpu_model,
>  CPUPPCState *env;
>  
>  /* init CPUs */
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, cpu_model));
> +cpu = POWERPC_CPU(cpu_create(cpu_type));
>  env = &cpu->env;
>  
>  cpu_clk->cb = NULL; /* We don't care about CPU clock frequency changes */

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3] target/ppc: Fix carry flag setting for shift algebraic instructions

[David Gibson]

On Thu, Oct 05, 2017 at 08:42:56AM -0400, Richard Henderson wrote:
> On 10/03/2017 02:23 AM, Sandipan Das wrote:
> > @@ -231,6 +231,10 @@ target_ulong helper_sraw(CPUPPCState *env, 
> > target_ulong value,
> >  ret = (int32_t)value >> 31;
> >  env->ca = (ret != 0);
> >  }
> > +
> > +/* update CA32 for ISA v3.0 */
> > +env->ca32 = env->ca;
> 
> As I said before, modify ca32 only when ca is modified.
> E.g.
> 
>   env->ca32 = env->ca = (ret != 0);
> 
> > @@ -257,6 +261,10 @@ target_ulong helper_srad(CPUPPCState *env, 
> > target_ulong value,
> >  ret = (int64_t)value >> 63;
> >  env->ca = (ret != 0);
> >  }
> > +
> > +/* update CA32 for ISA v3.0 */
> > +env->ca32 = env->ca;
> 
> Likewise.
> 
> > @@ -2192,6 +2192,10 @@ static void gen_srawi(DisasContext *ctx)
> >  tcg_gen_setcondi_tl(TCG_COND_NE, cpu_ca, cpu_ca, 0);
> >  tcg_gen_sari_tl(dst, dst, sh);
> >  }
> > +
> > +/* update CA32 for ISA v3.0 */
> > +tcg_gen_mov_tl(cpu_ca32, cpu_ca);
> 
> Likewise.

Also, for the helper functions it definitely makes sense to always set
CA32 when CA is set, regardless of CPU model, it's close enough to
free.  When we're generating code, however, the trade-off is
different, we only need to test the CPU model at translate time, but
we need to execute the generated instrucitons potentially more times.

So I'm wondering if for the gen_* functions we _should_ be checking
for ISA300 before generating the CA32 update instructions.

> 
> > @@ -2269,6 +2273,10 @@ static inline void gen_sradi(DisasContext *ctx, int 
> > n)
> >  tcg_gen_setcondi_tl(TCG_COND_NE, cpu_ca, cpu_ca, 0);
> >  tcg_gen_sari_tl(dst, src, sh);
> >  }
> > +
> > +/* update CA32 for ISA v3.0 */
> > +tcg_gen_mov_tl(cpu_ca32, cpu_ca);
> 
> Likewise.
> 
> 
> r~
> 

-- 
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/23] ppc: bamboo: use generic cpu_model parsing

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:34PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  hw/ppc/ppc440_bamboo.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index f92d47f..693c215 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -182,11 +182,7 @@ static void bamboo_init(MachineState *machine)
>  int success;
>  int i;
>  
> -/* Setup CPU. */
> -if (machine->cpu_model == NULL) {
> -machine->cpu_model = "440EP";
> -}
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, 
> machine->cpu_model));
> +cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>  env = &cpu->env;
>  
>  if (env->mmu_model != POWERPC_MMU_BOOKE) {
> @@ -297,6 +293,7 @@ static void bamboo_machine_init(MachineClass *mc)
>  {
>  mc->desc = "bamboo";
>  mc->init = bamboo_init;
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("440epb");
>  }
>  
>  DEFINE_MACHINE("bamboo", bamboo_machine_init)

-- 
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 09/23] ppc: virtex-ml507: replace cpu_model with cpu_type

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:36PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

> ---
>  hw/ppc/virtex_ml507.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index ed9b406..5ac4f76 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -89,14 +89,14 @@ static void mmubooke_create_initial_mapping(CPUPPCState 
> *env,
>  
>  static PowerPCCPU *ppc440_init_xilinx(ram_addr_t *ram_size,
>int do_init,
> -  const char *cpu_model,
> +  const char *cpu_type,
>uint32_t sysclk)
>  {
>  PowerPCCPU *cpu;
>  CPUPPCState *env;
>  qemu_irq *irqs;
>  
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, cpu_model));
> +cpu = POWERPC_CPU(cpu_create(cpu_type));
>  env = &cpu->env;
>  
>  ppc_booke_timers_init(cpu, sysclk, 0/* no flags */);
> @@ -211,11 +211,7 @@ static void virtex_init(MachineState *machine)
>  int i;
>  
>  /* init CPUs */
> -if (machine->cpu_model == NULL) {
> -machine->cpu_model = "440-Xilinx";
> -}
> -
> -cpu = ppc440_init_xilinx(&ram_size, 1, machine->cpu_model, 4);
> +cpu = ppc440_init_xilinx(&ram_size, 1, machine->cpu_type, 4);
>  env = &cpu->env;
>  
>  if (env->mmu_model != POWERPC_MMU_BOOKE) {
> @@ -307,6 +303,7 @@ static void virtex_machine_init(MachineClass *mc)
>  {
>  mc->desc = "Xilinx Virtex ML507 reference design";
>  mc->init = virtex_init;
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("440-xilinx");
>  }
>  
>  DEFINE_MACHINE("virtex-ml507", virtex_machine_init)

-- 
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 02/23] qom: introduce type_register_static_array()

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:29PM +0200, Igor Mammedov wrote:
> it will help to remove code duplication of registration
> static types in places that have open coded loop to
> perform batch type registering.
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Eduardo Habkost 
> Reviewed-by: Philippe Mathieu-Daudé 

Reviewed-by: David Gibson 

> ---
>  include/qom/object.h | 10 ++
>  qom/object.c |  9 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index a707b67..9a2369c 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -789,6 +789,16 @@ Type type_register_static(const TypeInfo *info);
>  Type type_register(const TypeInfo *info);
>  
>  /**
> + * type_register_static_array:
> + * @infos: The array of the new type #TypeInfo structures.
> + * @nr_infos: number of entries in @infos
> + *
> + * @infos and all of the strings it points to should exist for the life time
> + * that the type is registered.
> + */
> +void type_register_static_array(const TypeInfo *infos, int nr_infos);
> +
> +/**
>   * object_class_dynamic_cast_assert:
>   * @klass: The #ObjectClass to attempt to cast.
>   * @typename: The QOM typename of the class to cast to.
> diff --git a/qom/object.c b/qom/object.c
> index 6a7bd92..c58c52d 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -151,6 +151,15 @@ TypeImpl *type_register_static(const TypeInfo *info)
>  return type_register(info);
>  }
>  
> +void type_register_static_array(const TypeInfo *infos, int nr_infos)
> +{
> +int i;
> +
> +for (i = 0; i < nr_infos; i++) {
> +type_register_static(&infos[i]);
> +}
> +}
> +
>  static TypeImpl *type_get_by_name(const char *name)
>  {
>  if (name == NULL) {

-- 
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/23] ppc: mpc8544ds/e500plat: use generic cpu_model parsing

[David Gibson]

On Thu, Oct 05, 2017 at 06:24:31PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov 

Acked-by: David Gibson 

Do you want me to queue the ppc patches here, or do you already have a
plan for that?

> ---
>  hw/ppc/e500.c  | 8 +---
>  hw/ppc/e500plat.c  | 1 +
>  hw/ppc/mpc8544ds.c | 2 ++
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index db0e49a..9178e70 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -803,11 +803,6 @@ void ppce500_init(MachineState *machine, PPCE500Params 
> *params)
>  SysBusDevice *s;
>  PPCE500CCSRState *ccsr;
>  
> -/* Setup CPUs */
> -if (machine->cpu_model == NULL) {
> -machine->cpu_model = "e500v2_v30";
> -}
> -
>  irqs = g_malloc0(smp_cpus * sizeof(qemu_irq *));
>  irqs[0] = g_malloc0(smp_cpus * sizeof(qemu_irq) * OPENPIC_OUTPUT_NB);
>  for (i = 0; i < smp_cpus; i++) {
> @@ -815,8 +810,7 @@ void ppce500_init(MachineState *machine, PPCE500Params 
> *params)
>  CPUState *cs;
>  qemu_irq *input;
>  
> -cpu = POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU,
> -   machine->cpu_model));
> +cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>  env = &cpu->env;
>  cs = CPU(cpu);
>  
> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
> index 94b4545..e59e80f 100644
> --- a/hw/ppc/e500plat.c
> +++ b/hw/ppc/e500plat.c
> @@ -64,6 +64,7 @@ static void e500plat_machine_init(MachineClass *mc)
>  mc->init = e500plat_init;
>  mc->max_cpus = 32;
>  mc->has_dynamic_sysbus = true;
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
>  }
>  
>  DEFINE_MACHINE("ppce500", e500plat_machine_init)
> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
> index 27b8289..1717953 100644
> --- a/hw/ppc/mpc8544ds.c
> +++ b/hw/ppc/mpc8544ds.c
> @@ -16,6 +16,7 @@
>  #include "sysemu/device_tree.h"
>  #include "hw/ppc/openpic.h"
>  #include "qemu/error-report.h"
> +#include "cpu.h"
>  
>  static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
>  {
> @@ -55,6 +56,7 @@ static void ppce500_machine_init(MachineClass *mc)
>  mc->desc = "mpc8544ds";
>  mc->init = mpc8544ds_init;
>  mc->max_cpus = 15;
> +mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
>  }
>  
>  DEFINE_MACHINE("mpc8544ds", ppce500_machine_init)

-- 
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 00/22] tcg: tb_lock removal

[Emilio G. Cota]

On Mon, Aug 07, 2017 at 19:52:16 -0400, Emilio G. Cota wrote:
> This series applies on top of the "multiple TCG contexts" series, v4:
>   https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06769.html
(snip)
> Please review!

Turns out this patchset breaks icount, even after fixing the patchset
it is based on (see [1]).

The last good patch in the series is patch 10:
"translate-all: work page-by-page in tb_invalidate_phys_range_1".
I have no time to look into fixing that, so if you end up reviewing
this set, please review only patches 1-10.

The good news is that patches > 10 get really hairy, so this should
reduce the review burden substantially.

Thanks,

Emilio

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg01198.html

Re: [Qemu-devel] [PATCH v4 11/43] tcg: define CF_PARALLEL and use it for TB hashing along with CF_COUNT_MASK

[Emilio G. Cota]

On Mon, Sep 25, 2017 at 10:01:15 -0700, Richard Henderson wrote:
> On 09/22/2017 01:40 PM, Emilio G. Cota wrote:
> > Hi Richard,
> > 
> > Are you planning to get this patchset merged in this window? If so, I can
> > give it a respin on top of the current master.
> 
> Yes, I do.  I've been intending to look at ...
> 
> > Anyway, before doing so we should fix the issue around CF_COUNT_MASK that
> > Pranith reported:
> 
> ... this one and figure out why my intuition is wrong.
> I'm at Linaro Connect this week, so it may have to wait til next.

I just tested this with icount. Turns out two fixups to this patchset are
necessary to not break icount. The first one is (again) to just
include CF_PARALLEL in the hash mask. The second is a fix for the comparison
function of tb_tc, without which removals don't work.

I'm including the fixups below.

Thanks,

Emilio

commit 7a899a8df2d769dd80ba1a7f559cb4ddbb0c568b
Author: Emilio G. Cota 
Date:   Thu Oct 5 18:40:30 2017 -0400

FIXUP for "tcg: define CF_PARALLEL ..."

Signed-off-by: Emilio G. Cota 

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 025fae0..8b2f233 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -326,7 +326,7 @@ struct TranslationBlock {
 #define CF_INVALID 0x8 /* TB is stale. Setters must acquire tb_lock */
 #define CF_PARALLEL0x10 /* Generate code for a parallel context */
 /* cflags' mask for hashing/comparison */
-#define CF_HASH_MASK (CF_COUNT_MASK | CF_PARALLEL)
+#define CF_HASH_MASK (CF_PARALLEL)
 
 /* Per-vCPU dynamic tracing state used to generate this TB */
 uint32_t trace_vcpu_dstate;

commit c102c2409a5a134fd7f9ba61f69a5ae29df99c89
Author: Emilio G. Cota 
Date:   Thu Oct 5 18:51:24 2017 -0400

FIXUP for "translate-all: use a binary search tree to ..."

Signed-off-by: Emilio G. Cota 

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9f1faff..fe607ca 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -793,19 +793,19 @@ static gint tb_tc_cmp(gconstpointer ap, gconstpointer bp)
 const struct tb_tc *b = bp;
 
 /*
- * When both sizes are set, we know this isn't a lookup and therefore
- * the two buffers are non-overlapping.
+ * When both sizes are set, we know this isn't a lookup.
  * This is the most likely case: every TB must be inserted; lookups
  * are a lot less frequent.
  */
 if (likely(a->size && b->size)) {
-/* a->ptr == b->ptr would mean the buffers overlap */
-g_assert(a->ptr != b->ptr);
-
 if (a->ptr > b->ptr) {
 return 1;
+} else if (a->ptr < b->ptr) {
+return -1;
 }
-return -1;
+/* a->ptr == b->ptr should happen only on deletions */
+g_assert(a->size == b->size);
+return 0;
 }
 /*
  * All lookups have either .size field set to 0.

[Qemu-devel] Patch to add helpful tracing output for driver authors in NVMe emulation

[Doug Gale]

I added the tracing output in this patch to assist me in implementing
an NVMe driver. It helped tremendously.

>From 1d19086cdef8d492929852d582cb41dcc5026f71 Mon Sep 17 00:00:00 2001
From: Doug Gale 
Date: Thu, 5 Oct 2017 19:02:03 -0400
Subject: [PATCH] Add tracing output to NVMe emulation to help driver authors.

It is off by default, enable it by uncommenting #define DEBUG_NVME
or through CFLAGS

Signed-off-by: Doug Gale 
---
 hw/block/nvme.c | 191 +++-
 1 file changed, 177 insertions(+), 14 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9aa32692a3..74220c0171 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -36,6 +36,14 @@

 #include "nvme.h"

+//#define DEBUG_NVME
+
+#ifdef DEBUG_NVME
+#define DPRINTF(fmt, ...) fprintf(stderr, "nvme: " fmt "\n", ## __VA_ARGS__)
+#else
+#define DPRINTF(fmt, ...) ((void)0)
+#endif
+
 static void nvme_process_sq(void *opaque);

 static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
@@ -86,10 +94,14 @@ static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
 {
 if (cq->irq_enabled) {
 if (msix_enabled(&(n->parent_obj))) {
+DPRINTF("raising MSI-X IRQ vector %u", cq->vector);
 msix_notify(&(n->parent_obj), cq->vector);
 } else {
+DPRINTF("pulsing IRQ pin");
 pci_irq_pulse(&n->parent_obj);
 }
+} else {
+DPRINTF("IRQ is masked");
 }
 }

@@ -101,9 +113,11 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
QEMUIOVector *iov, uint64_t prp1,
 int num_prps = (len >> n->page_bits) + 1;

 if (!prp1) {
+DPRINTF("Invalid PRP!");
 return NVME_INVALID_FIELD | NVME_DNR;
 } else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
+DPRINTF("PRP in controller memory");
 qsg->nsg = 0;
 qemu_iovec_init(iov, num_prps);
 qemu_iovec_add(iov, (void *)&n->cmbuf[prp1 -
n->ctrl_mem.addr], trans_len);
@@ -168,6 +182,7 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
QEMUIOVector *iov, uint64_t prp1,

  unmap:
 qemu_sglist_destroy(qsg);
+DPRINTF("invalid SGL!");
 return NVME_INVALID_FIELD | NVME_DNR;
 }

@@ -178,16 +193,22 @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n,
uint8_t *ptr, uint32_t len,
 QEMUIOVector iov;
 uint16_t status = NVME_SUCCESS;

+DPRINTF("DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64,
+prp1, prp2);
+
 if (nvme_map_prp(&qsg, &iov, prp1, prp2, len, n)) {
+DPRINTF("DMA read invalid PRP field!");
 return NVME_INVALID_FIELD | NVME_DNR;
 }
 if (qsg.nsg > 0) {
 if (dma_buf_read(ptr, len, &qsg)) {
+DPRINTF("DMA read invalid SGL field!");
 status = NVME_INVALID_FIELD | NVME_DNR;
 }
 qemu_sglist_destroy(&qsg);
 } else {
 if (qemu_iovec_to_buf(&iov, 0, ptr, len) != len) {
+DPRINTF("invalid field!");
 status = NVME_INVALID_FIELD | NVME_DNR;
 }
 qemu_iovec_destroy(&iov);
@@ -274,6 +295,7 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n,
NvmeNamespace *ns, NvmeCmd *cmd,
 uint32_t aio_nlb = nlb << (data_shift - BDRV_SECTOR_BITS);

 if (slba + nlb > ns->id_ns.nsze) {
+DPRINTF("Invalid LBA!");
 return NVME_LBA_RANGE | NVME_DNR;
 }

@@ -301,13 +323,19 @@ static uint16_t nvme_rw(NvmeCtrl *n,
NvmeNamespace *ns, NvmeCmd *cmd,
 int is_write = rw->opcode == NVME_CMD_WRITE ? 1 : 0;
 enum BlockAcctType acct = is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ;

+DPRINTF("%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64,
+is_write ? "write" : "read",
+nlb, data_size, slba);
+
 if ((slba + nlb) > ns->id_ns.nsze) {
 block_acct_invalid(blk_get_stats(n->conf.blk), acct);
+DPRINTF("Invalid LBA!");
 return NVME_LBA_RANGE | NVME_DNR;
 }

 if (nvme_map_prp(&req->qsg, &req->iov, prp1, prp2, data_size, n)) {
 block_acct_invalid(blk_get_stats(n->conf.blk), acct);
+DPRINTF("Invalid PRP!");
 return NVME_INVALID_FIELD | NVME_DNR;
 }

@@ -337,6 +365,7 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd
*cmd, NvmeRequest *req)
 uint32_t nsid = le32_to_cpu(cmd->nsid);

 if (nsid == 0 || nsid > n->num_namespaces) {
+DPRINTF("Invalid namespace!");
 return NVME_INVALID_NSID | NVME_DNR;
 }

@@ -350,6 +379,7 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd
*cmd, NvmeRequest *req)
 case NVME_CMD_READ:
 return nvme_rw(n, ns, cmd, req);
 default:
+DPRINTF("Invalid opcode!");
 return NVME_INVALID_OPCODE | NVME_DNR;
 }
 }
@@ -374,9 +404,12 @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeCmd *cmd)
 uint16_t qid = le16_to_cpu(c->qid);

 if (!qid || nvme_check_sqid(n, qid)) {
+DPRINTF("invalid submission queue deletion! qid=%u", qid);
 return NVME_INV

[Qemu-devel] [Bug 1719196] Re: [arm64 ocata] newly created instances are unable to raise network interfaces

[dann frazier]

Thanks so much for doing that Sean.

Omitting expected changes (uuid, mac address, etc), here's are the
significant changes I see:

1) N uses the QEMU 'virt' model, O uses 'virt-2.8'
2) N and O both expose a pci root, but N also exposed 2 PCI bridges that O does 
not.
3) N exposes an additional serial device.
4) N and O both use an apparmor seclabel. However, O also has a DAC model.

#4 is the most interesting to me. Is there a way to configure ocata nova
to not enable DAC?

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

Title:
  [arm64 ocata] newly created instances are unable to raise network
  interfaces

Status in libvirt:
  New
Status in QEMU:
  New

Bug description:
  arm64 Ocata ,

  I'm testing to see I can get Ocata running on arm64 and using the
  openstack-base bundle to deploy it.  I have added the bundle to the
  log file attached to this bug.

  When I create a new instance via nova, the VM comes up and runs,
  however fails to raise its eth0 interface. This occurs on both
  internal and external networks.

  ubuntu@openstackaw:~$ nova list
  
+--+-+++-++
  | ID   | Name| Status | Task State | 
Power State | Networks   |
  
+--+-+++-++
  | dcaf6d51-f81e-4cbd-ac77-0c5d21bde57c | sfeole1 | ACTIVE | -  | 
Running | internal=10.5.5.3  |
  | aa0b8aee-5650-41f4-8fa0-aeccdc763425 | sfeole2 | ACTIVE | -  | 
Running | internal=10.5.5.13 |
  
+--+-+++-++
  ubuntu@openstackaw:~$ nova show aa0b8aee-5650-41f4-8fa0-aeccdc763425
  
+--+--+
  | Property | Value
|
  
+--+--+
  | OS-DCF:diskConfig| MANUAL   
|
  | OS-EXT-AZ:availability_zone  | nova 
|
  | OS-EXT-SRV-ATTR:host | awrep3   
|
  | OS-EXT-SRV-ATTR:hypervisor_hostname  | awrep3.maas  
|
  | OS-EXT-SRV-ATTR:instance_name| instance-0003
|
  | OS-EXT-STS:power_state   | 1
|
  | OS-EXT-STS:task_state| -
|
  | OS-EXT-STS:vm_state  | active   
|
  | OS-SRV-USG:launched_at   | 2017-09-24T14:23:08.00   
|
  | OS-SRV-USG:terminated_at | -
|
  | accessIPv4   |  
|
  | accessIPv6   |  
|
  | config_drive |  
|
  | created  | 2017-09-24T14:22:41Z 
|
  | flavor   | m1.small 
(717660ae-0440-4b19-a762-ffeb32a0575c)  |
  | hostId   | 
5612a00671c47255d2ebd6737a64ec9bd3a5866d1233ecf3e988b025 |
  | id   | aa0b8aee-5650-41f4-8fa0-aeccdc763425 
|
  | image| zestynosplash 
(e88fd1bd-f040-44d8-9e7c-c462ccf4b945) |
  | internal network | 10.5.5.13
|
  | key_name | mykey
|
  | metadata | {}   
|
  | name | sfeole2  
|
  | os-extended-volumes:volumes_attached | []   
|
  | progress | 0
|
  | security_groups  | default  
|
  | status   | ACTIVE   
|
  | tenant_id| 9f7a21c1ad264fec81abc09f3960ad1d 
 

Re: [Qemu-devel] [Qemu-block] [PATCH 8/8] nbd: Minimal structured read for client

[Eric Blake]

On 10/05/2017 05:36 AM, Paolo Bonzini wrote:
> On 05/10/2017 12:02, Vladimir Sementsov-Ogievskiy wrote:
>> 03.10.2017 17:06, Paolo Bonzini wrote:
>>> On 03/10/2017 15:35, Vladimir Sementsov-Ogievskiy wrote:
>> In the end this probably means that you have a read_chunk_header
>> function and a read_chunk function.  READ has a loop that calls
>> read_chunk_header followed by direct reading into the QEMUIOVector,
>> while everyone else calls read_chunk.
> accordingly to spec, we can receive several error reply chunks to any
> request,
> so loop, receiving them should be common for all requests I think
 as well as handling error chunks should be common..
>>> Yes, reading error chunks should be part of read_chunk_header.
>>>
>>> Paolo
>>
>> So, you want a loop in READ, and separate loop for other commands? Then
>> we will have separate loop for BLOCK_STATUS and for all future commands
>> with specific replies?
> 
> There should be a separate loop for each command.
> 
> The only difference between READ and other commands is that READ
> receives directly in QEMUIOVector, while other commands can use a common
> function to to receive each structured reply chunk into malloc-ed memory.

To make sure we're on the same page, here's how I see it.  At a high
level, we have:

Each command calls nbd_co_send_request once, then calls
nbd_co_receive_reply in a loop until there is an indication of the last
packet.  nbd_co_receive_reply waits for data to come from the server,
and parses the header.

If the packet is unrecognized, report failure and request a quit
(negative return value)

If it is old-style:
- if the command is read, and we did not negotiate structured read, then
we also read the payload into qiov
- if the command is read, but we negotiated structured read, the server
is buggy, so report the bug and request a quit
- for all other commands, there is no payload, return success or failure
to the caller based on the header payload
- at any rate, the reply to the caller is that this is the final packet,
and there is no payload returned (so we return negative or 1, but never 0)

Otherwise, it is new-style:
- if we did not negotiate structured reply, the server is buggy, so
report the bug and request a quit (negative return)
- if the chunk is an error, we process the entire packet and report the
error; if we have any commands that care about the error details, we
could return the error in a malloc'd discriminated union, but we can
probably get by without that. If callers don't care about details, but
the error chunk is not final, it may be easier to just store the fact
that an error occurred and return 0 to tell the caller to keep looping,
and return the negative value later when the final chunk is finally received
- if the chunk is NBD_REPLY_TYPE_NONE, there is no payload, and this
should be the final chunk, so the return to the caller can be the same
as for old-style (return 1 if we had no earlier error packets, or the
saved negative value corresponding to the first error received)
- if the command is read, we can read the payload into qiov (saves
malloc'ing space for the reply only to copy it into the qiov), so we
don't have to return any data
- for any other command, we malloc space for the non-error payload, and
then it is up to the command's loop to process the payload

so the signature can be something like:

int nbd_co_receive_reply(NBDClientSession *s, QEMUIOVector *qiov,
 void **payload)

where it returns -errno on failure, 0 if the command is not complete,
and 1 if the command is done.  READ passes qiov, which is fully
populated when the function returns 1; all other commands pass NULL.
Commands pass NULL for payload if they don't expect a payload return
(this includes READ); but a command that expects a payload
(BLOCK_STATUS) passes a pointer in payload and gets malloc'd space
stored there if return is 0 or 1.

Does that sound like we're on the right design track?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 15/23] ppc: spapr: register 'host' core type along with the rest of core types

[Greg Kurz]

On Thu,  5 Oct 2017 18:24:42 +0200
Igor Mammedov  wrote:

> consolidate 'host' core type registration by moving it from
> KVM specific code into spapr_cpu_core.c, similar like it's
> done in x86 target.
> 
> Signed-off-by: Igor Mammedov 
> ---

On the way you could have dropped this line in target/ppc/kvm.c:

#include "hw/ppc/spapr_cpu_core.h"

Note, there's also:

#if defined(TARGET_PPC64)
#include "hw/ppc/spapr_cpu_core.h"
#endif

but Philippe (on Cc) has already sent a patch to drop this one:

https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg06499.html

Anyway,

Reviewed-by: Greg Kurz 

>  include/hw/ppc/spapr_cpu_core.h |  1 -
>  hw/ppc/spapr_cpu_core.c |  5 -
>  target/ppc/kvm.c| 11 ---
>  3 files changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 264ce68..42765de 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -38,5 +38,4 @@ typedef struct sPAPRCPUCoreClass {
>  } sPAPRCPUCoreClass;
>  
>  char *spapr_get_cpu_core_type(const char *model);
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
>  #endif
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 8e13e52..f2da4be 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -220,7 +220,7 @@ static Property spapr_cpu_core_properties[] = {
>  DEFINE_PROP_END_OF_LIST()
>  };
>  
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(oc);
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
> @@ -257,6 +257,9 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>  DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"),
>  DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
>  DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
> +#ifdef CONFIG_KVM
> +DEFINE_SPAPR_CPU_CORE_TYPE("host"),
> +#endif
>  };
>  
>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index c2152ed..cb5777a 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2508,17 +2508,6 @@ static int kvm_ppc_register_host_cpu_type(void)
>  oc = object_class_by_name(type_info.name);
>  g_assert(oc);
>  
> -#if defined(TARGET_PPC64)
> -type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host");
> -type_info.parent = TYPE_SPAPR_CPU_CORE,
> -type_info.instance_size = sizeof(sPAPRCPUCore);
> -type_info.instance_init = NULL;
> -type_info.class_init = spapr_cpu_core_class_init;
> -type_info.class_data = (void *) POWERPC_CPU_TYPE_NAME("host");
> -type_register(&type_info);
> -g_free((void *)type_info.name);
> -#endif
> -
>  /*
>   * Update generic CPU family class alias (e.g. on a POWER8NVL host,
>   * we want "POWER8" to be a "family" alias that points to the current

Re: [Qemu-devel] Qemu Documentation

[Swetheendra Tallamraju]

I want yo add extra functionality of booting from virtual USB through qemu.
I need to write a new block of code in /hw/USB to emulate virtual USB. I
don't have any clue of how to procced. Code documentation of other devices
like hub network would help us understanding and implementing usb

On Thu, Oct 5, 2017 at 12:03 AM, Swetheendra Tallamraju <
swetheendr...@gmail.com> wrote:

> I am working on qemu source code to provide extra functionality of
> emulating virtual usb. Can I get any  documentation for the qemu source
> code that helps me in implementing this?
>

Re: [Qemu-devel] [PATCH 14/23] ppc: spapr: use cpu type name directly

[Greg Kurz]

On Thu,  5 Oct 2017 18:24:41 +0200
Igor Mammedov  wrote:

> replace sPAPRCPUCoreClass::cpu_class with cpu type name
> since it were needed just to get that at points it were
> accessed.
> 
> Signed-off-by: Igor Mammedov 
> ---

Reviewed-by: Greg Kurz 

>  include/hw/ppc/spapr_cpu_core.h |  2 +-
>  hw/ppc/spapr.c  |  6 ++
>  hw/ppc/spapr_cpu_core.c | 13 +
>  target/ppc/kvm.c|  2 +-
>  4 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 66dcf52..264ce68 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -34,7 +34,7 @@ typedef struct sPAPRCPUCore {
>  
>  typedef struct sPAPRCPUCoreClass {
>  DeviceClass parent_class;
> -ObjectClass *cpu_class;
> +const char *cpu_type;
>  } sPAPRCPUCoreClass;
>  
>  char *spapr_get_cpu_core_type(const char *model);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 01b3012..ad7afd6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3142,8 +3142,7 @@ void spapr_core_release(DeviceState *dev)
>  if (smc->pre_2_10_has_unused_icps) {
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  int i;
>  
>  for (i = 0; i < cc->nr_threads; i++) {
> @@ -3239,8 +3238,7 @@ static void spapr_core_plug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  
>  if (smc->pre_2_10_has_unused_icps) {
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  int i;
>  
>  for (i = 0; i < cc->nr_threads; i++) {
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 01f4ec8..8e13e52 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -90,8 +90,7 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, 
> Error **errp)
>  {
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  CPUCore *cc = CPU_CORE(dev);
>  int i;
>  
> @@ -152,8 +151,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
>  CPUCore *cc = CPU_CORE(OBJECT(dev));
> -const char *typename = object_class_get_name(scc->cpu_class);
> -size_t size = object_type_get_instance_size(typename);
> +size_t size = object_type_get_instance_size(scc->cpu_type);
>  Error *local_err = NULL;
>  void *obj;
>  int i, j;
> @@ -172,7 +170,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>  
>  obj = sc->threads + i * size;
>  
> -object_initialize(obj, size, typename);
> +object_initialize(obj, size, scc->cpu_type);
>  cs = CPU(obj);
>  cpu = POWERPC_CPU(cs);
>  cs->cpu_index = cc->core_id + i;
> @@ -230,14 +228,13 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void 
> *data)
>  dc->realize = spapr_cpu_core_realize;
>  dc->unrealize = spapr_cpu_core_unrealizefn;
>  dc->props = spapr_cpu_core_properties;
> -scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data);
> -g_assert(scc->cpu_class);
> +scc->cpu_type = data;
>  }
>  
>  #define DEFINE_SPAPR_CPU_CORE_TYPE(cpu_model) \
>  {   \
>  .parent = TYPE_SPAPR_CPU_CORE,  \
> -.class_data = (void *) cpu_model,   \
> +.class_data = (void *) POWERPC_CPU_TYPE_NAME(cpu_model), \
>  .class_init = spapr_cpu_core_class_init,\
>  .name = SPAPR_CPU_CORE_TYPE_NAME(cpu_model),\
>  }
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 171d3d8..c2152ed 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2514,7 +2514,7 @@ static int kvm_ppc_register_host_cpu_type(void)
>  type_info.instance_size = sizeof(sPAPRCPUCore);
>  type_info.instance_init = NULL;
>  type_info.class_init = spapr_cpu_core_class_init;
> -type_info.class_data = (void *) "host";
> +type_info.class_data = (void *) POWERPC_CPU_TYPE_NAME("host");
>  type_register(&type_info);
>  g_free((void *)type_info.name);
>  #endif

[Qemu-devel] [PULL 7/9] config: qemu_config_parse() return number of config groups

[Eduardo Habkost]

Change qemu_config_parse() to return the number of config groups
in success and -EINVAL on error. This will allow callers of
qemu_config_parse() to check if something was really loaded from
the config file.

All existing callers of qemu_config_parse() and
qemu_read_config_file() only check if the return value was
negative, so the change shouldn't affect them.

Reviewed-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Eduardo Habkost 
Message-Id: <20171004025043.3788-2-ehabk...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 block/blkdebug.c   |  1 -
 util/qemu-config.c | 15 +++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 46e53f2f09..dfdf9b91aa 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -244,7 +244,6 @@ static int read_config(BDRVBlkdebugState *s, const char 
*filename,
 ret = qemu_config_parse(f, config_groups, filename);
 if (ret < 0) {
 error_setg(errp, "Could not parse blkdebug config file");
-ret = -EINVAL;
 goto fail;
 }
 }
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 405dd1a1d7..99b0e46fa3 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -385,6 +385,7 @@ void qemu_config_write(FILE *fp)
 }
 }
 
+/* Returns number of config groups on success, -errno on error */
 int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
 {
 char line[1024], group[64], id[64], arg[64], value[1024];
@@ -392,7 +393,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 QemuOptsList *list = NULL;
 Error *local_err = NULL;
 QemuOpts *opts = NULL;
-int res = -1, lno = 0;
+int res = -EINVAL, lno = 0;
+int count = 0;
 
 loc_push_none(&loc);
 while (fgets(line, sizeof(line), fp) != NULL) {
@@ -413,6 +415,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 goto out;
 }
 opts = qemu_opts_create(list, id, 1, NULL);
+count++;
 continue;
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
@@ -423,6 +426,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 goto out;
 }
 opts = qemu_opts_create(list, NULL, 0, &error_abort);
+count++;
 continue;
 }
 value[0] = '\0';
@@ -447,7 +451,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 error_report("error reading file");
 goto out;
 }
-res = 0;
+res = count;
 out:
 loc_pop(&loc);
 return res;
@@ -464,12 +468,7 @@ int qemu_read_config_file(const char *filename)
 
 ret = qemu_config_parse(f, vm_config_groups, filename);
 fclose(f);
-
-if (ret == 0) {
-return 0;
-} else {
-return -EINVAL;
-}
+return ret;
 }
 
 static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
-- 
2.13.6

[Qemu-devel] [PULL 9/9] x86: Correct translation of some rdgsbase and wrgsbase encodings

[Eduardo Habkost]

From: Todd Eisenberger 

It looks like there was a transcription error when writing this code
initially.  The code previously only decoded src or dst of rax.  This
resolves
https://bugs.launchpad.net/qemu/+bug/1719984.

Signed-off-by: Todd Eisenberger 
Message-Id: 
Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
 target/i386/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index a8986f4c1a..7b920115f9 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8155,9 +8155,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState 
*cpu)
 break;
 
 case 0xc0 ... 0xc7: /* rdfsbase (f3 0f ae /0) */
-case 0xc8 ... 0xc8: /* rdgsbase (f3 0f ae /1) */
+case 0xc8 ... 0xcf: /* rdgsbase (f3 0f ae /1) */
 case 0xd0 ... 0xd7: /* wrfsbase (f3 0f ae /2) */
-case 0xd8 ... 0xd8: /* wrgsbase (f3 0f ae /3) */
+case 0xd8 ... 0xdf: /* wrgsbase (f3 0f ae /3) */
 if (CODE64(s)
 && (prefixes & PREFIX_REPZ)
 && !(prefixes & PREFIX_LOCK)
-- 
2.13.6

[Qemu-devel] [PULL 5/9] vl: Eliminate defconfig variable

[Eduardo Habkost]

Both -nodefconfig and -no-user-config options do the same thing
today, we only need one variable to keep track of them.

Suggested-by: Markus Armbruster 
Acked-by: Alistair Francis 
Reviewed-by: Markus Armbruster 
Signed-off-by: Eduardo Habkost 
Message-Id: <20171004030025.7866-2-ehabk...@redhat.com>
Reviewed-by: Daniel P. Berrange 
Signed-off-by: Eduardo Habkost 
---
 vl.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 3fed457921..ebea42e0ea 100644
--- a/vl.c
+++ b/vl.c
@@ -3111,7 +3111,6 @@ int main(int argc, char **argv, char **envp)
 const char *qtest_log = NULL;
 const char *pid_file = NULL;
 const char *incoming = NULL;
-bool defconfig = true;
 bool userconfig = true;
 bool nographic = false;
 DisplayType display_type = DT_DEFAULT;
@@ -3213,8 +3212,6 @@ int main(int argc, char **argv, char **envp)
 popt = lookup_opt(argc, argv, &optarg, &optind);
 switch (popt->index) {
 case QEMU_OPTION_nodefconfig:
-defconfig = false;
-break;
 case QEMU_OPTION_nouserconfig:
 userconfig = false;
 break;
@@ -3222,7 +3219,7 @@ int main(int argc, char **argv, char **envp)
 }
 }
 
-if (defconfig && userconfig) {
+if (userconfig) {
 if (qemu_read_default_config_file() < 0) {
 exit(1);
 }
-- 
2.13.6

[Qemu-devel] [PULL 6/9] qemu-options: Deprecate -nodefconfig

[Eduardo Habkost]

Since 2012 (commit ba6212d8 "Eliminate cpus-x86_64.conf file") we
have no default config files that would be disabled using
-nodefconfig.  Update documentation and document -nodefconfig as
deprecated.

Cc: Markus Armbruster 
Acked-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
Message-Id: <20171004030025.7866-3-ehabk...@redhat.com>
Reviewed-by: Markus Armbruster 
Reviewed-by: Daniel P. Berrange 
Signed-off-by: Eduardo Habkost 
---
 qemu-doc.texi   |  4 
 qemu-options.hx | 17 -
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index ecd186a159..d8bb2c664f 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2496,6 +2496,10 @@ would automatically enable USB support on the machine 
type.
 If using the new syntax, USB support must be explicitly
 enabled via the ``-machine usb=on'' argument.
 
+@subsection -nodefconfig (since 2.11.0)
+
+The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
+
 @section qemu-img command line arguments
 
 @subsection convert -s (since 2.0.0)
diff --git a/qemu-options.hx b/qemu-options.hx
index 39225ae6c3..981742d191 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4067,26 +4067,17 @@ Write device configuration to @var{file}. The 
@var{file} can be either filename
 command line and device configuration into file or dash @code{-}) character to 
print the
 output to stdout. This can be later used as input file for @code{-readconfig} 
option.
 ETEXI
-DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig,
-"-nodefconfig\n"
-"do not load default config files at startup\n",
-QEMU_ARCH_ALL)
-STEXI
-@item -nodefconfig
-@findex -nodefconfig
-Normally QEMU loads configuration files from @var{sysconfdir} and 
@var{datadir} at startup.
-The @code{-nodefconfig} option will prevent QEMU from loading any of those 
config files.
-ETEXI
+HXCOMM Deprecated, same as -no-user-config
+DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig, "", QEMU_ARCH_ALL)
 DEF("no-user-config", 0, QEMU_OPTION_nouserconfig,
 "-no-user-config\n"
-"do not load user-provided config files at startup\n",
+"do not load default user-provided config files at 
startup\n",
 QEMU_ARCH_ALL)
 STEXI
 @item -no-user-config
 @findex -no-user-config
 The @code{-no-user-config} option makes QEMU not load any of the user-provided
-config files on @var{sysconfdir}, but won't make it skip the QEMU-provided 
config
-files from @var{datadir}.
+config files on @var{sysconfdir}.
 ETEXI
 DEF("trace", HAS_ARG, QEMU_OPTION_trace,
 "-trace [[enable=]][,events=][,file=]\n"
-- 
2.13.6

[Qemu-devel] [PULL 8/9] qom: update doc comment for type_register[_static]()

[Eduardo Habkost]

From: Igor Mammedov 

type_register()/type_register_static() functions in current impl.
can't fail returning 0, also none of the users check for error
so update doc comment to reflect current behaviour.

Suggested-by: Eduardo Habkost 
Signed-off-by: Igor Mammedov 
Message-Id: <1507111682-66171-2-git-send-email-imamm...@redhat.com>
Reviewed-by: Eduardo Habkost 
Signed-off-by: Eduardo Habkost 
---
 include/qom/object.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index e0d9824415..a707b67781 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -773,7 +773,7 @@ const char *object_get_typename(const Object *obj);
  * @info and all of the strings it points to should exist for the life time
  * that the type is registered.
  *
- * Returns: 0 on failure, the new #Type on success.
+ * Returns: the new #Type.
  */
 Type type_register_static(const TypeInfo *info);
 
@@ -784,7 +784,7 @@ Type type_register_static(const TypeInfo *info);
  * Unlike type_register_static(), this call does not require @info or its
  * string members to continue to exist after the call returns.
  *
- * Returns: 0 on failure, the new #Type on success.
+ * Returns: the new #Type.
  */
 Type type_register(const TypeInfo *info);
 
-- 
2.13.6

[Qemu-devel] [PULL 4/9] machine: Add a valid_cpu_types property

[Eduardo Habkost]

From: Alistair Francis 

This patch add a MachineClass element that can be set in the machine C
code to specify a list of supported CPU types. If the supported CPU
types are specified the user enter CPU (by -cpu at runtime) is checked
against the supported types and QEMU exits if they aren't supported.

Signed-off-by: Alistair Francis 
Message-Id: 

[ehabkost: removed assert(), rewrote comment]
Signed-off-by: Eduardo Habkost 
---
 include/hw/boards.h |  1 +
 hw/core/machine.c   | 32 
 2 files changed, 33 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 156e0a5701..191a5b3cd8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -191,6 +191,7 @@ struct MachineClass {
 bool has_hotpluggable_cpus;
 bool ignore_memory_transaction_failures;
 int numa_mem_align_shift;
+const char **valid_cpu_types;
 void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
  int nb_nodes, ram_addr_t size);
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 80647edc2a..36c2fb069c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -758,6 +758,38 @@ void machine_run_board_init(MachineState *machine)
 if (nb_numa_nodes) {
 machine_numa_finish_init(machine);
 }
+
+/* If the machine supports the valid_cpu_types check and the user
+ * specified a CPU with -cpu check here that the user CPU is supported.
+ */
+if (machine_class->valid_cpu_types && machine->cpu_type) {
+ObjectClass *class = object_class_by_name(machine->cpu_type);
+int i;
+
+for (i = 0; machine_class->valid_cpu_types[i]; i++) {
+if (object_class_dynamic_cast(class,
+  machine_class->valid_cpu_types[i])) {
+/* The user specificed CPU is in the valid field, we are
+ * good to go.
+ */
+break;
+}
+}
+
+if (!machine_class->valid_cpu_types[i]) {
+/* The user specified CPU is not valid */
+error_report("Invalid CPU type: %s", machine->cpu_type);
+error_printf("The valid types are: %s",
+ machine_class->valid_cpu_types[0]);
+for (i = 1; machine_class->valid_cpu_types[i]; i++) {
+error_printf(", %s", machine_class->valid_cpu_types[i]);
+}
+error_printf("\n");
+
+exit(1);
+}
+}
+
 machine_class->init(machine);
 }
 
-- 
2.13.6

[Qemu-devel] [PULL 1/9] hw/acpi-build: Make assignment statement of next_base easy to read

[Eduardo Habkost]

From: Dou Liyang 

It may be hard to read the assignment statement of "next_base", so

S/next_base += (1ULL << 32) - pcms->below_4g_mem_size;
 /next_base = mem_base + mem_len;

... for readability.

No functionality change.

Signed-off-by: Dou Liyang 
Message-Id: <1504231805-30957-3-git-send-email-douly.f...@cn.fujitsu.com>
Signed-off-by: Eduardo Habkost 
---
 hw/i386/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2af37a9129..73e3443bce 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2381,7 +2381,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 }
 mem_base = 1ULL << 32;
 mem_len = next_base - pcms->below_4g_mem_size;
-next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+next_base = mem_base + mem_len;
 }
 numamem = acpi_data_push(table_data, sizeof *numamem);
 build_srat_memory(numamem, mem_base, mem_len, i - 1,
-- 
2.13.6

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

[Eduardo Habkost]

From: Dou Liyang 

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

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

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

Suggested-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
Message-Id: <1504231805-30957-4-git-send-email-douly.f...@cn.fujitsu.com>
Signed-off-by: Eduardo Habkost 
---
 tests/bios-tables-test.c  |  24 
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 5104 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 224 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 7788 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 224 bytes
 5 files changed, 24 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 564da45f65..f0923152ff 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -808,6 +808,28 @@ static void test_acpi_piix4_tcg_memhp(void)
 free_test_data(&data);
 }
 
+static void test_acpi_q35_tcg_numamem(void)
+{
+test_data data;
+
+memset(&data, 0, sizeof(data));
+data.machine = MACHINE_Q35;
+data.variant = ".numamem";
+test_acpi_one(" -numa node -numa node,mem=128", &data);
+free_test_data(&data);
+}
+
+static void test_acpi_piix4_tcg_numamem(void)
+{
+test_data data;
+
+memset(&data, 0, sizeof(data));
+data.machine = MACHINE_PC;
+data.variant = ".numamem";
+test_acpi_one(" -numa node -numa node,mem=128", &data);
+free_test_data(&data);
+}
+
 int main(int argc, char *argv[])
 {
 const char *arch = qtest_get_arch();
@@ -830,6 +852,8 @@ int main(int argc, char *argv[])
 qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
 qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
 qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
+qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
+qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
 }
 ret = g_test_run();
 boot_sector_cleanup(disk);
diff --git a/tests/acpi-test-data/pc/DSDT.numamem 
b/tests/acpi-test-data/pc/DSDT.numamem
new file mode 100644
index 
..bc703d77ee48720af1b801c518b43511c95c852d
GIT binary patch
literal 5104
zcmb7IZExE~5}qX~n_5aXWizjC8^LKVXmL&SlDFc3gC$b76k9SQ%DEh9oG8b|DMfK1
z(gvt&6i`&ab^ERG;xE-sw(qx*XF!z}jjPX%ajXzq&jTQFq
zw)Zd3`{YZHwS3rmsXyOp`CsgNTR@obq*UN7;@fd>bkHW>7wH}lOw3;x+kz<+;?^`Xya)uvf@QYk*O7srT^929^Z
z(63x0Z^&J@d%=E?%?G|FWMJruoz0sP@PWP*8-tlj#VIsH94;*3Ze&b}dmP>U!{4qE
z3%vBYS9|OW!Z=at1=2Sc+mq77{0X)+xWdGVZ3hlYIS{M^y2LNdbhD6aE$wxA%i+Vt_OfTwcvY}08l4PJ2-Q=9}7b8sV#4=e38#LcV|M*sM#V>#9yxmCRb#$#4_CDp-{qY)9{PBnYsRh0J+mH1
zKs}S1;o4VI5D$`V2fn5`9>Zs)r#)|D%xxO?Y1-|sO=Fmt%;AAdU;&}>q~cmRsk40k
zs~L#PG0akqe;WSnfH51ML2`oJGg3{f;=t!L=AB?>mQFSF$)!L(*L3O*`??)^fz_;D
zq4}Zp;)Hd~-`{LKJ7zr_SkIz=DIGuw@R^_6V|!(JIv?C%;y60xe4>R2=lq2
zd27-a9i_3C|w1tG&gG0m!lrDl#mvgwr8
z(ulMQjkJ+yR%#X12by%d#rHOYDuup;{v`{hUCCs!8S)^!&tpc)Y%Kp(>hXg%?3tNN
z=8;jJ!WveHyO%ewE8=3K7|D04M3d8K%m=S`@nBLx-urykbFZGztgZGvqZ*@#exD&W
zNreoj@*CwD(=lsmR2a;AS0R=mO%QXqd#b^Er&k*f1@5QRp
zE1#qa_VaWqE}!H=IC7mHXftCpr`KF31=~4|IsON`COWuCFqBfleh$@dgp$z
zs!&?t8N&}|D5jR$rv$?!tQHz6jjNzi?}b{eNf}N_0me)dgVHE6Xg~T8Pway#7z>!u
zD|V?_%H(j*g0_pYn>JcsS4b6{^{o(hA
zOd*oHie0wr;nMt?1cN)JPMzd}SMZ7%*!jG(iRPVrb8bpu=roRRH0M+WyFu*pP`XJP
z3PAtU@$$LdYs-HzmqQ2cm8u(<5II6mc&x|t7*#{P4F#GT1}X
z4-mKeu9F=KxZV;N$d2or;7C)@cEqj)LA0tk`~stZF1wu*L;69tFKA
zSh;}mNT^`N#0wqJqM)|~YeGO1kx;>!6wqW8^tND~5zv`Ps9?ngF!Xgc3VKJd>H?}q
zLIrC|KvPlB_XX>mfX+oi1?#+k&PPEDf)z7A49kdw3f40MdL{~56s%_j^lT(lu$~jp
zb5T%Ju$~vt^N~=&dO<)hL_saVdQm_xMnVPaf`BeWK}&-54FP>45-M2V6wo)Lpv!{w
zEdhNi5-M0P3FxKMphkJYx?gKZy~J)C@6tW0b&mo;M**u@D5Oacnk5vlB88d`38YEx
zwl%zn_Z%!MLrEfOq*GAB;xN>at(zkq8N(w!)RDDYBORH;gF4iaz1yUl-ECua#am?0
z!2>5`yhl(z*5hrm=it#1dTbU@KGy3b;~^0GbRZw=mq^BR7Wz&gAM1mNF)@W6FOxmT
z9MeiGn0^Um-1d>aLo(`(OVoJae}w3#J#8W0bsKCru(jB1F=Cg2*ZH@DxXmUE`z~9*2k!U%mXRq
Yf~!ZCL8t>-1O^}2VG2>z!9?-X08OU~0RR91

literal 0
HcmV?d1

diff --git a/tests/acpi-test-data/q35/DSDT.numamem 
b/tests/acpi-test-data/q35/DSDT.numamem
new file mode 100644
index 
..fcb18d947b28c09c8b5a4fa65692efb41ef3a5d9
GIT binary patch
literal 7788
zcmb7JTW=f38J#7U)M~kumeR`dU4ruvv}j_=PLrZV1CzVFRiZ?Zbew0D2nJQ>iNFm$TLd{i1Cv5m>(Yk5>h?S3es3>yTVbQ<^y`ht
zD}C=ePCt}eX{TR`+QRAIY(!SDHr3zgg!YqrB8+aW4A(RJ+l6`8?=}B<{fDKCH@>;`
zu=MP=|M{Ta~UFtA!C_Y?zOFtJ@Tw5a0ip6LxRcCp`aFWj$JMwWI!W@Or<~
zEr|iO!~<%&{pZ;AB}}cr9Mv;2C#&|v>NTrlpDKVZ)XihFy-#Jsow56+7{tP
zvy>JVe#-b+YjvX(vnnZfk5~&}RYX{-tXp9jUu!L

[Qemu-devel] [PULL 0/9] x86 and machine queue, 2017-10-05

[Eduardo Habkost]

The following changes since commit d8f932cc696250cb740240d668b39df5fbb2d5a0:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' 
into staging (2017-10-05 16:54:29 +0100)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/x86-and-machine-pull-request

for you to fetch changes up to 7d7e9c4fd6873d9624956cbe0a5cbfae59b7f8af:

  x86: Correct translation of some rdgsbase and wrgsbase encodings (2017-10-05 
17:30:06 -0300)


x86 and machine queue, 2017-10-05

Includes x86, NUMA, ACPI, QOM, CPU, and option/config parsing
patches.

Highlights:
* Deprecation of -nodefconfig option;
* MachineClass::valid_cpu_types field.



Alistair Francis (1):
  machine: Add a valid_cpu_types property

Dou Liyang (2):
  hw/acpi-build: Make assignment statement of next_base easy to read
  ACPI/unit-test: Add a new testcase for RAM allocation in numa node

Eduardo Habkost (3):
  vl: Eliminate defconfig variable
  qemu-options: Deprecate -nodefconfig
  config: qemu_config_parse() return number of config groups

Igor Mammedov (1):
  qom: update doc comment for type_register[_static]()

Philippe Mathieu-Daudé (1):
  qom/cpu: move cpu_model null check to cpu_class_by_name()

Todd Eisenberger (1):
  x86: Correct translation of some rdgsbase and wrgsbase encodings

 include/hw/boards.h   |   1 +
 include/qom/object.h  |   4 ++--
 block/blkdebug.c  |   1 -
 hw/core/machine.c |  32 
 hw/i386/acpi-build.c  |   2 +-
 qom/cpu.c |   7 ++-
 target/alpha/cpu.c|   6 +-
 target/arm/cpu.c  |   4 
 target/cris/cpu.c |   4 
 target/i386/translate.c   |   4 ++--
 target/lm32/cpu.c |   4 
 target/m68k/cpu.c |   4 
 target/mips/cpu.c |   4 
 target/moxie/cpu.c|   8 +---
 target/openrisc/cpu.c |   4 
 target/sh4/cpu.c  |   3 ---
 target/sparc/cpu.c|   4 
 target/tricore/cpu.c  |   4 
 target/unicore32/cpu.c|   4 
 target/xtensa/cpu.c   |   4 
 tests/bios-tables-test.c  |  24 
 util/qemu-config.c|  15 +++
 vl.c  |   5 +
 qemu-doc.texi |   4 
 qemu-options.hx   |  17 -
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 5104 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 224 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 7788 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 224 bytes
 29 files changed, 86 insertions(+), 87 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

-- 
2.13.6

[Qemu-devel] [PULL 3/9] qom/cpu: move cpu_model null check to cpu_class_by_name()

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

and clean every implementation.

Suggested-by: Eduardo Habkost 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20170917232842.14544-1-f4...@amsat.org>
Reviewed-by: Igor Mammedov 
Reviewed-by: Laurent Vivier 
Reviewed-by: Artyom Tarasenko 
Signed-off-by: Eduardo Habkost 
---
 qom/cpu.c  | 7 ++-
 target/alpha/cpu.c | 6 +-
 target/arm/cpu.c   | 4 
 target/cris/cpu.c  | 4 
 target/lm32/cpu.c  | 4 
 target/m68k/cpu.c  | 4 
 target/mips/cpu.c  | 4 
 target/moxie/cpu.c | 8 +---
 target/openrisc/cpu.c  | 4 
 target/sh4/cpu.c   | 3 ---
 target/sparc/cpu.c | 4 
 target/tricore/cpu.c   | 4 
 target/unicore32/cpu.c | 4 
 target/xtensa/cpu.c| 4 
 14 files changed, 8 insertions(+), 56 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index 94fa8fe005..54c9452b1c 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -316,7 +316,12 @@ static bool cpu_common_has_work(CPUState *cs)
 
 ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model)
 {
-CPUClass *cc = CPU_CLASS(object_class_by_name(typename));
+CPUClass *cc;
+
+if (!cpu_model) {
+return NULL;
+}
+cc = CPU_CLASS(object_class_by_name(typename));
 
 return cc->class_by_name(cpu_model);
 }
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index e6c6aabdf0..b8a21f4e01 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -127,14 +127,10 @@ static const AlphaCPUAlias alpha_cpu_aliases[] = {
 
 static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
 {
-ObjectClass *oc = NULL;
+ObjectClass *oc;
 char *typename;
 int i;
 
-if (cpu_model == NULL) {
-return NULL;
-}
-
 oc = object_class_by_name(cpu_model);
 if (oc != NULL && object_class_dynamic_cast(oc, TYPE_ALPHA_CPU) != NULL &&
 !object_class_is_abstract(oc)) {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 4300de66e2..4b81d07b9d 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -913,10 +913,6 @@ static ObjectClass *arm_cpu_class_by_name(const char 
*cpu_model)
 char *typename;
 char **cpuname;
 
-if (!cpu_model) {
-return NULL;
-}
-
 cpuname = g_strsplit(cpu_model, ",", 1);
 typename = g_strdup_printf(ARM_CPU_TYPE_NAME("%s"), cpuname[0]);
 oc = object_class_by_name(typename);
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index ceebfed79b..88d93f2d11 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -69,10 +69,6 @@ static ObjectClass *cris_cpu_class_by_name(const char 
*cpu_model)
 ObjectClass *oc;
 char *typename;
 
-if (cpu_model == NULL) {
-return NULL;
-}
-
 #if defined(CONFIG_USER_ONLY)
 if (strcasecmp(cpu_model, "any") == 0) {
 return object_class_by_name("crisv32-" TYPE_CRIS_CPU);
diff --git a/target/lm32/cpu.c b/target/lm32/cpu.c
index 2b8c36b6d0..bf081f56d2 100644
--- a/target/lm32/cpu.c
+++ b/target/lm32/cpu.c
@@ -246,10 +246,6 @@ static ObjectClass *lm32_cpu_class_by_name(const char 
*cpu_model)
 ObjectClass *oc;
 char *typename;
 
-if (cpu_model == NULL) {
-return NULL;
-}
-
 typename = g_strdup_printf("%s-" TYPE_LM32_CPU, cpu_model);
 oc = object_class_by_name(typename);
 g_free(typename);
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 55bf24bae6..8c70e0805c 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -87,10 +87,6 @@ static ObjectClass *m68k_cpu_class_by_name(const char 
*cpu_model)
 ObjectClass *oc;
 char *typename;
 
-if (cpu_model == NULL) {
-return NULL;
-}
-
 typename = g_strdup_printf("%s-" TYPE_M68K_CPU, cpu_model);
 oc = object_class_by_name(typename);
 g_free(typename);
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 1a9a3ed94d..c15b894362 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -166,10 +166,6 @@ static ObjectClass *mips_cpu_class_by_name(const char 
*cpu_model)
 ObjectClass *oc;
 char *typename;
 
-if (cpu_model == NULL) {
-return NULL;
-}
-
 typename = mips_cpu_type_name(cpu_model);
 oc = object_class_by_name(typename);
 g_free(typename);
diff --git a/target/moxie/cpu.c b/target/moxie/cpu.c
index 748d02f29e..30bd44fcad 100644
--- a/target/moxie/cpu.c
+++ b/target/moxie/cpu.c
@@ -89,13 +89,7 @@ static void moxie_cpu_initfn(Object *obj)
 
 static ObjectClass *moxie_cpu_class_by_name(const char *cpu_model)
 {
-ObjectClass *oc;
-
-if (cpu_model == NULL) {
-return NULL;
-}
-
-oc = object_class_by_name(cpu_model);
+ObjectClass *oc = object_class_by_name(cpu_model);
 if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_MOXIE_CPU) ||
object_class_is_abstract(oc))) {
 return NULL;
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index a979f0bf8b..af9cdcc102 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -108,10 +108,6 @@

Re: [Qemu-devel] [PATCH 13/23] ppc: spapr: define core types statically

[Greg Kurz]

On Thu,  5 Oct 2017 18:24:40 +0200
Igor Mammedov  wrote:

> spapr core type definition doesn't have any fields that
> require it to be defined at runtime. So replace code
> that fills in TypeInfo at runtime with static TypeInfo
> array that does the same at complie time.
> 
> Signed-off-by: Igor Mammedov 
> ---

Reviewed-by: Greg Kurz 

>  include/hw/ppc/spapr_cpu_core.h |  2 +
>  hw/ppc/spapr_cpu_core.c | 85 
> +
>  2 files changed, 29 insertions(+), 58 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 93051e9..66dcf52 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -21,6 +21,8 @@
>  #define SPAPR_CPU_CORE_GET_CLASS(obj) \
>   OBJECT_GET_CLASS(sPAPRCPUCoreClass, (obj), TYPE_SPAPR_CPU_CORE)
>  
> +#define SPAPR_CPU_CORE_TYPE_NAME(model) model "-" TYPE_SPAPR_CPU_CORE
> +
>  typedef struct sPAPRCPUCore {
>  /*< private >*/
>  CPUCore parent_obj;
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 427d47f..01f4ec8 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -217,36 +217,6 @@ err:
>  error_propagate(errp, local_err);
>  }
>  
> -static const char *spapr_core_models[] = {
> -/* 970 */
> -"970_v2.2",
> -
> -/* 970MP variants */
> -"970mp_v1.0",
> -"970mp_v1.1",
> -
> -/* POWER5+ */
> -"power5+_v2.1",
> -
> -/* POWER7 */
> -"power7_v2.3",
> -
> -/* POWER7+ */
> -"power7+_v2.1",
> -
> -/* POWER8 */
> -"power8_v2.0",
> -
> -/* POWER8E */
> -"power8e_v2.1",
> -
> -/* POWER8NVL */
> -"power8nvl_v1.0",
> -
> -/* POWER9 */
> -"power9_v1.0",
> -};
> -
>  static Property spapr_cpu_core_properties[] = {
>  DEFINE_PROP_INT32("node-id", sPAPRCPUCore, node_id, 
> CPU_UNSET_NUMA_NODE_ID),
>  DEFINE_PROP_END_OF_LIST()
> @@ -264,33 +234,32 @@ void spapr_cpu_core_class_init(ObjectClass *oc, void 
> *data)
>  g_assert(scc->cpu_class);
>  }
>  
> -static const TypeInfo spapr_cpu_core_type_info = {
> -.name = TYPE_SPAPR_CPU_CORE,
> -.parent = TYPE_CPU_CORE,
> -.abstract = true,
> -.instance_size = sizeof(sPAPRCPUCore),
> -.class_size = sizeof(sPAPRCPUCoreClass),
> -};
> -
> -static void spapr_cpu_core_register_types(void)
> -{
> -int i;
> -
> -type_register_static(&spapr_cpu_core_type_info);
> -
> -for (i = 0; i < ARRAY_SIZE(spapr_core_models); i++) {
> -TypeInfo type_info = {
> -.parent = TYPE_SPAPR_CPU_CORE,
> -.instance_size = sizeof(sPAPRCPUCore),
> -.class_init = spapr_cpu_core_class_init,
> -.class_data = (void *) spapr_core_models[i],
> -};
> -
> -type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE,
> - spapr_core_models[i]);
> -type_register(&type_info);
> -g_free((void *)type_info.name);
> +#define DEFINE_SPAPR_CPU_CORE_TYPE(cpu_model) \
> +{   \
> +.parent = TYPE_SPAPR_CPU_CORE,  \
> +.class_data = (void *) cpu_model,   \
> +.class_init = spapr_cpu_core_class_init,\
> +.name = SPAPR_CPU_CORE_TYPE_NAME(cpu_model),\
>  }
> -}
>  
> -type_init(spapr_cpu_core_register_types)
> +static const TypeInfo spapr_cpu_core_type_infos[] = {
> +{
> +.name = TYPE_SPAPR_CPU_CORE,
> +.parent = TYPE_CPU_CORE,
> +.abstract = true,
> +.instance_size = sizeof(sPAPRCPUCore),
> +.class_size = sizeof(sPAPRCPUCoreClass),
> +},
> +DEFINE_SPAPR_CPU_CORE_TYPE("970_v2.2"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.0"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.1"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power5+_v2.1"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power7_v2.3"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power7+_v2.1"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power8_v2.0"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power8e_v2.1"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
> +DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
> +};
> +
> +DEFINE_TYPES(spapr_cpu_core_type_infos)

Re: [Qemu-devel] [PATCH 18/20] target/arm: Implement BLXNS

[Peter Maydell]

On 5 October 2017 at 19:56, Richard Henderson
 wrote:
> On 09/22/2017 11:00 AM, Peter Maydell wrote:
>> +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest)
>> +{
> ...
>> +if (dest & 1) {
>> +/* target is Secure, so this is just a normal BLX,
>> + * except that the low bit doesn't indicate Thumb/not.
>> + */
>> +env->regs[14] = nextinst;
>> +env->thumb = 1;
>> +env->regs[15] = dest & ~1;
>> +return;
>> +}
> ...
>> +switch_v7m_security_state(env, dest & 1);
>> +env->thumb = 1;
>> +env->regs[15] = dest & ~1;
>
> dest & 1 is known to be 0.

Yes. I liked the symmetry with the tail end of the v7m_bxns helper,
which is conceptually doing the same thing, and assumed the
compiler would be smart enough not to generate unnecessary code.

>> +static inline void gen_blxns(DisasContext *s, int rm)
>> +{
>> +TCGv_i32 var = load_reg(s, rm);
>> +
>> +/* We don't need to sync condexec state, for the same reason as blxns.
>
> s/blxns/bxns/ ?

Yes.

thanks
-- PMM

[Qemu-devel] [PATCH 1/1] hw/ppc/spapr_drc.c: adding drc->dev into detach quiesce condition

[Daniel Henrique Barboza]

In cases where a device is hotplugged and hot-unplugged shortly after,
there is a chance of QEMU breaking with the following message:

hw/ppc/spapr_drc.c:417:spapr_drc_detach: assertion failed: (drc->dev)
Aborted

spapr_drc_detach makes a g_assert(drc->dev) to ensure that the following
spapr_drc_release call is able to execute the appropriate callback
using drc->dev as a parameter. However, in a scenario where a hotplug
is quickly followed by a hot-unplug, this g_assert can be reached before
the hotplug operation sets drc->dev in spapr_drc_attach.

This patch makes use of the awaiting quiesce mechanism inside
spapr_drc_detach to fix this scenario. Inside spapr_drc_detach there is a
quiesce condition that relies on drc->state being equal to drck->empty_state.
If this doesn't happen, it is considered that the drc is not ready to be
detached. By extending this condition to include drc->dev being non-null
we cover this situation where the drc is still being attached and drc->dev
isn't set yet during the detach.

Fixes: https://bugs.launchpad.net/qemu/+bug/1718118
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/spapr_drc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 915e9b51c4..6ad8190360 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -414,11 +414,9 @@ void spapr_drc_detach(sPAPRDRConnector *drc)
 
 trace_spapr_drc_detach(spapr_drc_index(drc));
 
-g_assert(drc->dev);
-
 drc->unplug_requested = true;
 
-if (drc->state != drck->empty_state) {
+if (!drc->dev || (drc->state != drck->empty_state)) {
 trace_spapr_drc_awaiting_quiesce(spapr_drc_index(drc));
 return;
 }
-- 
2.13.6

Re: [Qemu-devel] [PULL v1 10/10] specs: Describe the TPM support in QEMU

[Stefan Berger]

On 10/05/2017 12:48 PM, Stefan Berger wrote:

This patch adds a description of the current TPM support in QEMU
to the specs.

Several public specs are referenced via their landing page on the
trustedcomputinggroup.org website.

Signed-off-by: Stefan Berger 
Reviewed-by: Laszlo Ersek 


This is obviously a mess-up. I'll fixed that on my github account, which 
now puts the last change at 3bfcc21bf9e437734bb3e90db7e5b5f6b8bf.


   Stefan

Re: [Qemu-devel] [Qemu-ppc] [PATCH 12/23] ppc: move '-cpu foo, compat=xxx' parsing into ppc_cpu_parse_featurestr()

[Greg Kurz]

On Thu,  5 Oct 2017 18:24:39 +0200
Igor Mammedov  wrote:

> there is a dedicated callback CPUClass::parse_features
> which purpose is to convert -cpu features into a set of
> global properties AND deal with compat/legacy features
> that couldn't be directly translated into CPU's properties.
> 
> Create ppc variant of it (ppc_cpu_parse_featurestr) and
> move 'compat=val' handling from spapr_cpu_core.c into it.
> That removes a dependency of board/core code on cpu_model
> parsing and would let to reuse common -cpu parsing
> introduced by 6063d4c0
> 
> Signed-off-by: Igor Mammedov 
> ---
>  include/hw/ppc/spapr.h  |  1 -
>  target/ppc/cpu-qom.h|  1 +
>  hw/ppc/spapr.c  |  2 +-
>  hw/ppc/spapr_cpu_core.c | 50 --
>  target/ppc/translate_init.c | 58 
> +
>  5 files changed, 60 insertions(+), 52 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c1b365f..8ca4f94 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -659,7 +659,6 @@ void 
> spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
>  uint32_t count, uint32_t index);
>  void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
> uint32_t count, uint32_t 
> index);
> -void spapr_cpu_parse_features(sPAPRMachineState *spapr);
>  int spapr_hpt_shift_for_ramsize(uint64_t ramsize);
>  void spapr_reallocate_hpt(sPAPRMachineState *spapr, int shift,
>Error **errp);
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index d0cf6ca..429b47f 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -181,6 +181,7 @@ typedef struct PowerPCCPUClass {
>  DeviceRealize parent_realize;
>  DeviceUnrealize parent_unrealize;
>  void (*parent_reset)(CPUState *cpu);
> +void (*parent_parse_features)(const char *type, char *str, Error **errp);
>  
>  uint32_t pvr;
>  bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ff87f15..01b3012 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2366,7 +2366,7 @@ static void ppc_spapr_init(MachineState *machine)
>  machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
>  }
>  
> -spapr_cpu_parse_features(spapr);
> +cpu_parse_cpu_model(TYPE_POWERPC_CPU, machine->cpu_model);
>  
>  spapr_set_vsmt_mode(spapr, &error_fatal);
>  
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 3dea5ff..427d47f 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -21,56 +21,6 @@
>  #include "sysemu/hw_accel.h"
>  #include "qemu/error-report.h"
>  
> -void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> -{
> -/*
> - * Backwards compatibility hack:
> - *
> - *   CPUs had a "compat=" property which didn't make sense for
> - *   anything except pseries.  It was replaced by "max-cpu-compat"
> - *   machine option.  This supports old command lines like
> - *   -cpu POWER8,compat=power7
> - *   By stripping the compat option and applying it to the machine
> - *   before passing it on to the cpu level parser.
> - */
> -gchar **inpieces;
> -gchar *newprops;
> -int i, j;
> -gchar *compat_str = NULL;
> -
> -inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
> -
> -/* inpieces[0] is the actual model string */
> -i = 1;
> -j = 1;
> -while (inpieces[i]) {
> -if (g_str_has_prefix(inpieces[i], "compat=")) {
> -/* in case of multiple compat= options */
> -g_free(compat_str);
> -compat_str = inpieces[i];
> -} else {
> -j++;
> -}
> -
> -i++;
> -/* Excise compat options from list */
> -inpieces[j] = inpieces[i];
> -}
> -
> -if (compat_str) {
> -char *val = compat_str + strlen("compat=");
> -
> -object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
> -&error_fatal);
> -
> -}
> -
> -newprops = g_strjoinv(",", inpieces);
> -cpu_parse_cpu_model(TYPE_POWERPC_CPU, newprops);
> -g_free(newprops);
> -g_strfreev(inpieces);
> -}
> -
>  static void spapr_cpu_reset(void *opaque)
>  {
>  PowerPCCPU *cpu = opaque;
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index c6399a3..5ee91e8 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -10313,6 +10313,62 @@ static ObjectClass *ppc_cpu_class_by_name(const char 
> *name)
>  
>  return NULL;
>  }

Maybe add en empty line here ?

> +static void ppc_cpu_parse_featurestr(const char *typename, char *features,
> + Error **errp)
> +{
> +const PowerPCCPUClass *pcc;
> +

Re: [Qemu-devel] [PATCH] qemu_opt_print: Remove shadowing opt decl

[Eric Blake]

On 10/05/2017 02:07 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> opt was declared as a separate local inside the last loop,
> shadowing the local at the top of the function.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  util/qemu-option.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 9b1dc8093b..877c5b4d67 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -766,7 +766,7 @@ void qemu_opts_print(QemuOpts *opts, const char 
> *separator)
>  }
>  for (; desc && desc->name; desc++) {
>  const char *value;
> -QemuOpt *opt = qemu_opt_find(opts, desc->name);
> +opt = qemu_opt_find(opts, desc->name);
>  
>  value = opt ? opt->str : desc->def_value_str;
>  if (!value) {
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 5/6] block: Perform copy-on-read in loop

[Eric Blake]

Improve our braindead copy-on-read implementation.  Pre-patch,
we have multiple issues:
- we create a bounce buffer and perform a write for the entire
request, even if the active image already has 99% of the
clusters occupied, and really only needs to copy-on-read the
remaining 1% of the clusters
- our bounce buffer was as large as the read request, and can
needlessly exhaust our memory by using double the memory of
the request size (the original request plus our bounce buffer),
rather than a capped maximum overhead beyond the original
- if a driver has a max_transfer limit, we are bypassing the
normal code in bdrv_aligned_preadv() that fragments to that
limit, and instead attempt to read the entire buffer from the
driver in one go, which some drivers may assert on
- a client can request a large request of nearly 2G such that
rounding the request out to cluster boundaries results in a
byte count larger than 2G.  While this cannot exceed 32 bits,
it DOES have some follow-on problems:
-- the call to bdrv_driver_pread() can assert for exceeding
BDRV_REQUEST_MAX_BYTES, if the driver is old and lacks
.bdrv_co_preadv
-- if the buffer is all zeroes, the subsequent call to
bdrv_co_do_pwrite_zeroes is a no-op due to a negative size,
which means we did not actually copy on read

Fix all of these issues by breaking up the action into a loop,
where each iteration is capped to sane limits.  Also, querying
the allocation status allows us to optimize: when data is
already present in the active layer, we don't need to bounce.

Note that the code has a telling comment that copy-on-read
should probably be a filter driver rather than a bolt-on hack
in io.c; but that remains a task for another day.

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Reviewed-by: Kevin Wolf 
Reviewed-by: Stefan Hajnoczi 

---
v2: avoid uninit ret on 0-length op [patchew, Kevin]
---
 block/io.c | 120 +
 1 file changed, 82 insertions(+), 38 deletions(-)

diff --git a/block/io.c b/block/io.c
index a5598ed869..8e419070b5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -34,6 +34,9 @@

 #define NOT_DONE 0x7fff /* used while emulated sync operation in progress 
*/

+/* Maximum bounce buffer for copy-on-read and write zeroes, in bytes */
+#define MAX_BOUNCE_BUFFER (32768 << BDRV_SECTOR_BITS)
+
 static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
 int64_t offset, int bytes, BdrvRequestFlags flags);

@@ -945,11 +948,14 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,

 BlockDriver *drv = bs->drv;
 struct iovec iov;
-QEMUIOVector bounce_qiov;
+QEMUIOVector local_qiov;
 int64_t cluster_offset;
 unsigned int cluster_bytes;
 size_t skip_bytes;
 int ret;
+int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
+BDRV_REQUEST_MAX_BYTES);
+unsigned int progress = 0;

 /* FIXME We cannot require callers to have write permissions when all they
  * are doing is a read request. If we did things right, write permissions
@@ -961,53 +967,95 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 // assert(child->perm & (BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE));

 /* Cover entire cluster so no additional backing file I/O is required when
- * allocating cluster in the image file.
+ * allocating cluster in the image file.  Note that this value may exceed
+ * BDRV_REQUEST_MAX_BYTES (even when the original read did not), which
+ * is one reason we loop rather than doing it all at once.
  */
 bdrv_round_to_clusters(bs, offset, bytes, &cluster_offset, &cluster_bytes);
+skip_bytes = offset - cluster_offset;

 trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
cluster_offset, cluster_bytes);

-iov.iov_len = cluster_bytes;
-iov.iov_base = bounce_buffer = qemu_try_blockalign(bs, iov.iov_len);
+bounce_buffer = qemu_try_blockalign(bs,
+MIN(MIN(max_transfer, cluster_bytes),
+MAX_BOUNCE_BUFFER));
 if (bounce_buffer == NULL) {
 ret = -ENOMEM;
 goto err;
 }

-qemu_iovec_init_external(&bounce_qiov, &iov, 1);
+while (cluster_bytes) {
+int64_t pnum;

-ret = bdrv_driver_preadv(bs, cluster_offset, cluster_bytes,
- &bounce_qiov, 0);
-if (ret < 0) {
-goto err;
-}
+ret = bdrv_is_allocated(bs, cluster_offset,
+MIN(cluster_bytes, max_transfer), &pnum);
+if (ret < 0) {
+/* Safe to treat errors in querying allocation as if
+ * unallocated; we'll probably fail again soon on the
+ * read, but at least that will set a decent errno.
+ */
+pnum = MIN(cluster_bytes, max_transfer);
+}

-bdrv_debug_event(bs, BLKDBG_COR_WRITE);
-

Re: [Qemu-devel] [PATCH 1/1] virtio/pci/migration: Convert to VMState

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (git) (dgilb...@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Convert the 'modern_state' part of virtio-pci to modern migration
> macros.

Ping.

Dave

> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  hw/virtio/virtio-pci.c | 108 
> +
>  1 file changed, 27 insertions(+), 81 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 8b0d6b69cd..f825a68a84 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -88,77 +88,19 @@ static void virtio_pci_save_config(DeviceState *d, 
> QEMUFile *f)
>  qemu_put_be16(f, vdev->config_vector);
>  }
>  
> -static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq,
> -   QEMUFile *f)
> -{
> -vq->num = qemu_get_be16(f);
> -vq->enabled = qemu_get_be16(f);
> -vq->desc[0] = qemu_get_be32(f);
> -vq->desc[1] = qemu_get_be32(f);
> -vq->avail[0] = qemu_get_be32(f);
> -vq->avail[1] = qemu_get_be32(f);
> -vq->used[0] = qemu_get_be32(f);
> -vq->used[1] = qemu_get_be32(f);
> -}
> -
> -static bool virtio_pci_has_extra_state(DeviceState *d)
> -{
> -VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> -
> -return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
> -}
> -
> -static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
> -   VMStateField *field)
> -{
> -VirtIOPCIProxy *proxy = pv;
> -int i;
> -
> -proxy->dfselect = qemu_get_be32(f);
> -proxy->gfselect = qemu_get_be32(f);
> -proxy->guest_features[0] = qemu_get_be32(f);
> -proxy->guest_features[1] = qemu_get_be32(f);
> -for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> -virtio_pci_load_modern_queue_state(&proxy->vqs[i], f);
> -}
> -
> -return 0;
> -}
> -
> -static void virtio_pci_save_modern_queue_state(VirtIOPCIQueue *vq,
> -   QEMUFile *f)
> -{
> -qemu_put_be16(f, vq->num);
> -qemu_put_be16(f, vq->enabled);
> -qemu_put_be32(f, vq->desc[0]);
> -qemu_put_be32(f, vq->desc[1]);
> -qemu_put_be32(f, vq->avail[0]);
> -qemu_put_be32(f, vq->avail[1]);
> -qemu_put_be32(f, vq->used[0]);
> -qemu_put_be32(f, vq->used[1]);
> -}
> -
> -static int put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
> -   VMStateField *field, QJSON *vmdesc)
> -{
> -VirtIOPCIProxy *proxy = pv;
> -int i;
> -
> -qemu_put_be32(f, proxy->dfselect);
> -qemu_put_be32(f, proxy->gfselect);
> -qemu_put_be32(f, proxy->guest_features[0]);
> -qemu_put_be32(f, proxy->guest_features[1]);
> -for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> -virtio_pci_save_modern_queue_state(&proxy->vqs[i], f);
> +static const VMStateDescription vmstate_virtio_pci_modern_queue_state = {
> +.name = "virtio_pci/modern_queue_state",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT16(num, VirtIOPCIQueue),
> +VMSTATE_UNUSED(1), /* enabled was stored as be16 */
> +VMSTATE_BOOL(enabled, VirtIOPCIQueue),
> +VMSTATE_UINT32_ARRAY(desc, VirtIOPCIQueue, 2),
> +VMSTATE_UINT32_ARRAY(avail, VirtIOPCIQueue, 2),
> +VMSTATE_UINT32_ARRAY(used, VirtIOPCIQueue, 2),
> +VMSTATE_END_OF_LIST()
>  }
> -
> -return 0;
> -}
> -
> -static const VMStateInfo vmstate_info_virtio_pci_modern_state = {
> -.name = "virtqueue_state",
> -.get = get_virtio_pci_modern_state,
> -.put = put_virtio_pci_modern_state,
>  };
>  
>  static bool virtio_pci_modern_state_needed(void *opaque)
> @@ -168,21 +110,18 @@ static bool virtio_pci_modern_state_needed(void *opaque)
>  return virtio_pci_modern(proxy);
>  }
>  
> -static const VMStateDescription vmstate_virtio_pci_modern_state = {
> +static const VMStateDescription vmstate_virtio_pci_modern_state_sub = {
>  .name = "virtio_pci/modern_state",
>  .version_id = 1,
>  .minimum_version_id = 1,
>  .needed = &virtio_pci_modern_state_needed,
>  .fields = (VMStateField[]) {
> -{
> -.name = "modern_state",
> -.version_id   = 0,
> -.field_exists = NULL,
> -.size = 0,
> -.info = &vmstate_info_virtio_pci_modern_state,
> -.flags= VMS_SINGLE,
> -.offset   = 0,
> -},
> +VMSTATE_UINT32(dfselect, VirtIOPCIProxy),
> +VMSTATE_UINT32(gfselect, VirtIOPCIProxy),
> +VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy, 2),
> +VMSTATE_STRUCT_ARRAY(vqs, VirtIOPCIProxy, VIRTIO_QUEUE_MAX, 0,
> + vmstate_virtio_pci_modern_queue_state,
> + VirtIOPCIQueue),
>  VMSTATE_END_OF_LIST()
>  }
>  };
> @@ -196,11 +135,18 @@ static const VMStateDescription vmstate_virtio_p

[Qemu-devel] [PATCH v3 6/6] iotests: Add test 197 for covering copy-on-read

[Eric Blake]

Add a test for qcow2 copy-on-read behavior, including exposure
for the just-fixed bugs.

The copy-on-read behavior is always to a qcow2 image, but the
test is careful to allow running with most image protocol/format
combos as the backing file being copied from (luks being the
exception, as it is harder to pass the right secret to all the
right places).  In fact, for './check nbd', this appears to be
the first time we've had a qcow2 image wrapping NBD, requiring
an additional line in _filter_img_create to match the similar
line in _filter_img_info.

Invoking blkdebug to prove we don't write too much took some
effort to get working; and it requires that $TEST_WRAP (based
on $TEST_DIR) not be subject to word splitting.  We may decide
later to have the entire iotests suite use relative rather than
absolute names, to avoid problems inherited by the absolute
name of $PWD or $TEST_DIR, at which point the sanity check in
this commit could be simplified.

This test requires at least 2G of consecutive memory to succeed;
as such, it is prone to spurious failures, particularly on
32-bit machines under load.  This situation is detected and
triggers an early exit to skip the test, rather than a failure.
To manually provoke this setup on a beefier machine, I used:
  $ (ulimit -S -v 100; ./check -qcow2 197)

Signed-off-by: Eric Blake 

---
v3: add out-of-memory detection [patchew]
v2: test 0-length query [Kevin], sanity check TEST_DIR [Jeff]

I only tested with -raw, -qcow2, -qed, and -nbd. I won't be
surprised if the test fails in some other setup...
---
 tests/qemu-iotests/common.filter |   1 +
 tests/qemu-iotests/197   | 109 +++
 tests/qemu-iotests/197.out   |  26 ++
 tests/qemu-iotests/group |   1 +
 4 files changed, 137 insertions(+)
 create mode 100755 tests/qemu-iotests/197
 create mode 100644 tests/qemu-iotests/197.out

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 9d5442ecd9..227b37e941 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -111,6 +111,7 @@ _filter_img_create()
 sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
 -e "s#$TEST_DIR#TEST_DIR#g" \
 -e "s#$IMGFMT#IMGFMT#g" \
+-e 's#nbd:127.0.0.1:10810#TEST_DIR/t.IMGFMT#g' \
 -e "s# encryption=off##g" \
 -e "s# cluster_size=[0-9]\\+##g" \
 -e "s# table_size=[0-9]\\+##g" \
diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
new file mode 100755
index 00..887eb4f496
--- /dev/null
+++ b/tests/qemu-iotests/197
@@ -0,0 +1,109 @@
+#!/bin/bash
+#
+# Test case for copy-on-read into qcow2
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=ebl...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1 # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+TEST_WRAP="$TEST_DIR/t.wrap.qcow2"
+BLKDBG_CONF="$TEST_DIR/blkdebug.conf"
+
+# Sanity check: our use of blkdebug fails if $TEST_DIR contains spaces
+# or other problems
+case "$TEST_DIR" in
+*[^-_a-zA-Z0-9/]*)
+_notrun "Suspicious TEST_DIR='$TEST_DIR', cowardly refusing to run" ;;
+esac
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$BLKDBG_CONF"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# Test is supported for any backing file; but we force qcow2 for our wrapper.
+_supported_fmt generic
+_supported_proto generic
+_supported_os Linux
+# LUKS support may be possible, but it complicates things.
+_unsupported_fmt luks
+
+echo
+echo '=== Copy-on-read ==='
+echo
+
+# Prep the images
+_make_test_img 4G
+$QEMU_IO -c "write -P 55 3G 1k" "$TEST_IMG" | _filter_qemu_io
+IMGPROTO=file IMGFMT=qcow2 IMGOPTS= TEST_IMG_FILE="$TEST_WRAP" \
+_make_test_img -F "$IMGFMT" -b "$TEST_IMG" | _filter_img_create
+$QEMU_IO -f qcow2 -c "write -z -u 1M 64k" "$TEST_WRAP" | _filter_qemu_io
+
+# Ensure that a read of two clusters, but where one is already allocated,
+# does not re-write the allocated cluster
+cat > "$BLKDBG_CONF" <&1 | _filter_qemu_io)
+case $output in
+*allocate*)
+_notrun "Insufficent memory to run test" ;;
+*) printf '%s\n' "$output" ;;
+esac
+$QEMU_IO -f qcow2 -C -c "read -P 0 $((3*1024*1024*1024 + 1024)) 1k" \
+"$TEST_WRAP" | _filter

[Qemu-devel] [PATCH] ui/cocoa.m: Fix console selection keys

[John Arbuckle]

Fix console selection keys so that the right console is selected. 

Signed-off-by: John Arbuckle 
---
 ui/cocoa.m | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 93e56d0518..2794f60b27 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -631,7 +631,7 @@ - (void) handleEvent:(NSEvent *)event
 
 // enable graphic console
 case Q_KEY_CODE_1 ... Q_KEY_CODE_9: // '1' to '9' keys
-console_select(keycode - 11);
+console_select(keycode - Q_KEY_CODE_1);
 break;
 }
 
-- 
2.13.5 (Apple Git-94)

[Qemu-devel] [PATCH v3 4/6] block: Add blkdebug hook for copy-on-read

[Eric Blake]

Make it possible to inject errors on writes performed during a
read operation due to copy-on-read semantics.

Signed-off-by: Eric Blake 
Reviewed-by: Jeff Cody 
Reviewed-by: Kevin Wolf 
Reviewed-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
---
 qapi/block-core.json | 5 -
 block/io.c   | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 750bb0c77c..ab96e348e6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2538,6 +2538,8 @@
 #
 # @l1_shrink_free_l2_clusters: discard the l2 tables. (since 2.11)
 #
+# @cor_write: a write due to copy-on-read (since 2.11)
+#
 # Since: 2.9
 ##
 { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
@@ -2555,7 +2557,8 @@
 'flush_to_disk', 'pwritev_rmw_head', 'pwritev_rmw_after_head',
 'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
 'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
-'l1_shrink_write_table', 'l1_shrink_free_l2_clusters' ] }
+'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
+'cor_write'] }

 ##
 # @BlkdebugInjectErrorOptions:
diff --git a/block/io.c b/block/io.c
index 94f74703b7..a5598ed869 100644
--- a/block/io.c
+++ b/block/io.c
@@ -983,6 +983,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild 
*child,
 goto err;
 }

+bdrv_debug_event(bs, BLKDBG_COR_WRITE);
 if (drv->bdrv_co_pwrite_zeroes &&
 buffer_is_zero(bounce_buffer, iov.iov_len)) {
 /* FIXME: Should we (perhaps conditionally) be setting
-- 
2.13.6

[Qemu-devel] [PATCH v3 2/6] block: Uniform handling of 0-length bdrv_get_block_status()

[Eric Blake]

Handle a 0-length block status request up front, with a uniform
return value claiming the area is not allocated.

Most callers don't pass a length of 0 to bdrv_get_block_status()
and friends; but it definitely happens with a 0-length read when
copy-on-read is enabled.  While we could audit all callers to
ensure that they never make a 0-length request, and then assert
that fact, it was just as easy to fix things to always report
success (as long as the callers are careful to not go into an
infinite loop).  However, we had inconsistent behavior on whether
the status is reported as allocated or defers to the backing
layer, depending on what callbacks the driver implements, and
possibly wasting quite a few CPU cycles to get to that answer.
Consistently reporting unallocated up front doesn't really hurt
anything, and makes it easier both for callers (0-length requests
now have well-defined behavior) and for drivers (drivers don't
have to deal with 0-length requests).

Signed-off-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 

---
v3: split into two conditionals [Stefan], simple enough to keep R-b
v2: new patch
---
 block/io.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/io.c b/block/io.c
index e0f904583f..94f74703b7 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1777,6 +1777,10 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status(BlockDriverState *bs,
 *pnum = 0;
 return BDRV_BLOCK_EOF;
 }
+if (!nb_sectors) {
+*pnum = 0;
+return 0;
+}

 n = total_sectors - sector_num;
 if (n < nb_sectors) {
-- 
2.13.6

[Qemu-devel] [PATCH] qemu_opt_print: Remove shadowing opt decl

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

opt was declared as a separate local inside the last loop,
shadowing the local at the top of the function.

Signed-off-by: Dr. David Alan Gilbert 
---
 util/qemu-option.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 9b1dc8093b..877c5b4d67 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -766,7 +766,7 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
 }
 for (; desc && desc->name; desc++) {
 const char *value;
-QemuOpt *opt = qemu_opt_find(opts, desc->name);
+opt = qemu_opt_find(opts, desc->name);
 
 value = opt ? opt->str : desc->def_value_str;
 if (!value) {
-- 
2.13.6

[Qemu-devel] [PATCH v3 0/6] block: Avoid copy-on-read assertions

[Eric Blake]

During my quest to switch block status to be byte-based, John
forced me to evaluate whether we have a situation during
copy-on-read where we could exceed BDRV_REQUEST_MAX_BYTES [1].
Sure enough, we have a number of pre-existing bugs in the
copy-on-read code.  Fix those, along with adding a test.

Available as a tag at:
git fetch git://repo.or.cz/qemu/ericb.git nbd-copy-on-read-v3

Since v2 (available at [2]):
- add a new patch to fix an iotests wart
- tweak patch 5 (now 6) to skip rather than fail on limited memory [patchew]
- tweak patch 2 condition for legibility [Stefan]
- add R-b

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07286.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00524.html

001/6:[] [--] 'qemu-io: Add -C for opening with copy-on-read'
002/6:[0008] [FC] 'block: Uniform handling of 0-length bdrv_get_block_status()'
003/6:[down] 'iotests: Restore stty settings on completion'
004/6:[] [--] 'block: Add blkdebug hook for copy-on-read'
005/6:[] [--] 'block: Perform copy-on-read in loop'
006/6:[0013] [FC] 'iotests: Add test 197 for covering copy-on-read'

Eric Blake (6):
  qemu-io: Add -C for opening with copy-on-read
  block: Uniform handling of 0-length bdrv_get_block_status()
  iotests: Restore stty settings on completion
  block: Add blkdebug hook for copy-on-read
  block: Perform copy-on-read in loop
  iotests: Add test 197 for covering copy-on-read

 qapi/block-core.json |   5 +-
 block/io.c   | 123 +++
 qemu-io.c|  15 -
 tests/qemu-iotests/common.filter |   1 +
 tests/qemu-iotests/197   | 109 ++
 tests/qemu-iotests/197.out   |  26 +
 tests/qemu-iotests/check |  10 
 tests/qemu-iotests/group |   1 +
 8 files changed, 249 insertions(+), 41 deletions(-)
 create mode 100755 tests/qemu-iotests/197
 create mode 100644 tests/qemu-iotests/197.out

-- 
2.13.6

[Qemu-devel] [PATCH v3 3/6] iotests: Restore stty settings on completion

[Eric Blake]

Executing qemu with a terminal as stdin will temporarily alter stty
settings on that terminal (for example, disabling echo), because of
how we run both the monitor and any multiplexing with guest input.
Normally, qemu restores the original settings on exit; but if an
iotest triggers qemu to abort in the middle, we can be left with
the altered terminal setup.  This can make life very annoying when
debugging an iotest failure (not everyone remembers the trick of
blind-typing 'stty sane' without echo, and some people prefer
terminal settings that are slightly different than the defaults
picked by 'stty sane').

It is possible to avoid qemu corrupting the terminal by not passing
a terminal to qemu's stdin in the first place (as in, use
'./check ... 

---
v3: new patch
---
 tests/qemu-iotests/check | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 176cb8e937..e6b6ff7a04 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -134,6 +134,13 @@ export VALGRIND_QEMU=
 export IMGKEYSECRET=
 export IMGOPTSSYNTAX=false

+# Save current tty settings, since an aborting qemu call may leave things
+# screwed up
+STTY_RESTORE=
+if test -t 0; then
+STTY_RESTORE=$(stty -g)
+fi
+
 for r
 do

@@ -664,6 +671,9 @@ END{ if (NR > 0) {
 needwrap=false
 fi

+if test -n "$STTY_RESTORE"; then
+stty $STTY_RESTORE
+fi
 rm -f "${TEST_DIR}"/*.out "${TEST_DIR}"/*.err "${TEST_DIR}"/*.time
 rm -f "${TEST_DIR}"/check.pid "${TEST_DIR}"/check.sts
 rm -f $tmp.*
-- 
2.13.6

[Qemu-devel] [PATCH v3 1/6] qemu-io: Add -C for opening with copy-on-read

[Eric Blake]

Make it easier to enable copy-on-read during iotests, by
exposing a new bool option to main and open.

Signed-off-by: Eric Blake 
Reviewed-by: Jeff Cody 
Reviewed-by: Kevin Wolf 
Reviewed-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
---
 qemu-io.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index 265445ad89..c70bde3eb1 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -102,6 +102,7 @@ static void open_help(void)
 " Opens a file for subsequent use by all of the other qemu-io commands.\n"
 " -r, -- open file read-only\n"
 " -s, -- use snapshot file\n"
+" -C, -- use copy-on-read\n"
 " -n, -- disable host cache, short for -t none\n"
 " -U, -- force shared permissions\n"
 " -k, -- use kernel AIO implementation (on Linux only)\n"
@@ -120,7 +121,7 @@ static const cmdinfo_t open_cmd = {
 .argmin = 1,
 .argmax = -1,
 .flags  = CMD_NOFILE_OK,
-.args   = "[-rsnkU] [-t cache] [-d discard] [-o options] [path]",
+.args   = "[-rsCnkU] [-t cache] [-d discard] [-o options] [path]",
 .oneline= "open the file specified by path",
 .help   = open_help,
 };
@@ -145,7 +146,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
 QDict *opts;
 bool force_share = false;

-while ((c = getopt(argc, argv, "snro:kt:d:U")) != -1) {
+while ((c = getopt(argc, argv, "snCro:kt:d:U")) != -1) {
 switch (c) {
 case 's':
 flags |= BDRV_O_SNAPSHOT;
@@ -154,6 +155,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
 flags |= BDRV_O_NOCACHE;
 writethrough = false;
 break;
+case 'C':
+flags |= BDRV_O_COPY_ON_READ;
+break;
 case 'r':
 readonly = 1;
 break;
@@ -251,6 +255,7 @@ static void usage(const char *name)
 "  -r, --read-only  export read-only\n"
 "  -s, --snapshot   use snapshot file\n"
 "  -n, --nocachedisable host cache, short for -t none\n"
+"  -C, --copy-on-read   enable copy-on-read\n"
 "  -m, --misalign   misalign allocations for O_DIRECT\n"
 "  -k, --native-aio use kernel AIO implementation (on Linux only)\n"
 "  -t, --cache=MODE use the given cache mode for the image\n"
@@ -439,7 +444,7 @@ static QemuOptsList file_opts = {
 int main(int argc, char **argv)
 {
 int readonly = 0;
-const char *sopt = "hVc:d:f:rsnmkt:T:U";
+const char *sopt = "hVc:d:f:rsnCmkt:T:U";
 const struct option lopt[] = {
 { "help", no_argument, NULL, 'h' },
 { "version", no_argument, NULL, 'V' },
@@ -448,6 +453,7 @@ int main(int argc, char **argv)
 { "read-only", no_argument, NULL, 'r' },
 { "snapshot", no_argument, NULL, 's' },
 { "nocache", no_argument, NULL, 'n' },
+{ "copy-on-read", no_argument, NULL, 'C' },
 { "misalign", no_argument, NULL, 'm' },
 { "native-aio", no_argument, NULL, 'k' },
 { "discard", required_argument, NULL, 'd' },
@@ -492,6 +498,9 @@ int main(int argc, char **argv)
 flags |= BDRV_O_NOCACHE;
 writethrough = false;
 break;
+case 'C':
+flags |= BDRV_O_COPY_ON_READ;
+break;
 case 'd':
 if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
 error_report("Invalid discard option: %s", optarg);
-- 
2.13.6

Re: [Qemu-devel] [PATCH 20/20] nvic: Add missing code for writing SHCSR.HARDFAULTPENDED bit

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> When we added support for the new SHCSR bits in v8M in commit
> 437d59c17e9 the code to support writing to the new HARDFAULTPENDED
> bit was accidentally only added for non-secure writes; the
> secure banked version of the bit should also be writable.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/intc/armv7m_nvic.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 19/20] target/arm: Implement secure function return

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> Secure function return happens when a non-secure function has been
> called using BLXNS and so has a particular magic LR value (either
> 0xfefe or 0xfeff). The function return via BX behaves
> specially when the new PC value is this magic value, in the same
> way that exception returns are handled.
> 
> Adjust our BX excret guards so that they recognize the function
> return magic number as well, and perform the function-return
> unstacking in do_v7m_exception_exit().
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/internals.h |   7 +++
>  target/arm/helper.c| 115 
> +
>  target/arm/translate.c |  14 +-
>  3 files changed, 126 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 18/20] target/arm: Implement BLXNS

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest)
> +{
...
> +if (dest & 1) {
> +/* target is Secure, so this is just a normal BLX,
> + * except that the low bit doesn't indicate Thumb/not.
> + */
> +env->regs[14] = nextinst;
> +env->thumb = 1;
> +env->regs[15] = dest & ~1;
> +return;
> +}
...
> +switch_v7m_security_state(env, dest & 1);
> +env->thumb = 1;
> +env->regs[15] = dest & ~1;

dest & 1 is known to be 0.

> +static inline void gen_blxns(DisasContext *s, int rm)
> +{
> +TCGv_i32 var = load_reg(s, rm);
> +
> +/* We don't need to sync condexec state, for the same reason as blxns.

s/blxns/bxns/ ?

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 17/20] target/arm: Implement SG instruction

[Richard Henderson]

On 10/05/2017 02:55 PM, Peter Maydell wrote:
> Oops. I missed this in my testing because it happens that the
> two halves of an SG instruction are the same value :-)

Hah.  I didn't notice that either.


r~

Re: [Qemu-devel] [PATCH 17/20] target/arm: Implement SG instruction

[Peter Maydell]

On 5 October 2017 at 19:50, Richard Henderson
 wrote:
> On 09/22/2017 11:00 AM, Peter Maydell wrote:
>> Implement the SG instruction, which we emulate 'by hand' in the
>> exception handling code path.
>>
>> Signed-off-by: Peter Maydell 
>> ---
>>  target/arm/helper.c | 129 
>> ++--
>>  1 file changed, 124 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index b1ecb66..8df819d 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -41,6 +41,10 @@ typedef struct V8M_SAttributes {
>>  bool irvalid;
>>  } V8M_SAttributes;
>>
>> +static void v8m_security_lookup(CPUARMState *env, uint32_t address,
>> +MMUAccessType access_type, ARMMMUIdx 
>> mmu_idx,
>> +V8M_SAttributes *sattrs);
>> +
>>  /* Definitions for the PMCCNTR and PMCR registers */
>>  #define PMCRD   0x8
>>  #define PMCRC   0x4
>> @@ -6724,6 +6728,123 @@ static void arm_log_exception(int idx)
>>  }
>>  }
>>
>> +static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, uint16_t 
>> *insn)
>> +{
>
> This function doesn't take an address ...
>
>> +if (get_phys_addr(env, env->regs[15], MMU_INST_FETCH, mmu_idx,
>> +  &physaddr, &attrs, &prot, &page_size, &fsr, &fi)) {
>
> ... reading it directly from r15 ...
>
>> +if (insn != 0xe97f) {
>> +/* Not an SG instruction first half (we choose the IMPDEF
>> + * early-SG-check option).
>> + */
>> +goto gen_invep;
>> +}
>> +
>> +if (!v7m_read_half_insn(cpu, mmu_idx, &insn)) {
>> +return false;
>> +}
>> +
>> +if (insn != 0xe97f) {
>> +/* Not an SG instruction second half */
>> +goto gen_invep;
>> +}
>
> ... but somehow expects to get two different values read from the same 
> address?
>
> Certainly you'd get the wrong exception frame if you incremented r15 in 
> between.

Oops. I missed this in my testing because it happens that the
two halves of an SG instruction are the same value :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 17/20] target/arm: Implement SG instruction

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> Implement the SG instruction, which we emulate 'by hand' in the
> exception handling code path.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/helper.c | 129 
> ++--
>  1 file changed, 124 insertions(+), 5 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b1ecb66..8df819d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -41,6 +41,10 @@ typedef struct V8M_SAttributes {
>  bool irvalid;
>  } V8M_SAttributes;
>  
> +static void v8m_security_lookup(CPUARMState *env, uint32_t address,
> +MMUAccessType access_type, ARMMMUIdx mmu_idx,
> +V8M_SAttributes *sattrs);
> +
>  /* Definitions for the PMCCNTR and PMCR registers */
>  #define PMCRD   0x8
>  #define PMCRC   0x4
> @@ -6724,6 +6728,123 @@ static void arm_log_exception(int idx)
>  }
>  }
>  
> +static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, uint16_t 
> *insn)
> +{

This function doesn't take an address ...

> +if (get_phys_addr(env, env->regs[15], MMU_INST_FETCH, mmu_idx,
> +  &physaddr, &attrs, &prot, &page_size, &fsr, &fi)) {

... reading it directly from r15 ...

> +if (insn != 0xe97f) {
> +/* Not an SG instruction first half (we choose the IMPDEF
> + * early-SG-check option).
> + */
> +goto gen_invep;
> +}
> +
> +if (!v7m_read_half_insn(cpu, mmu_idx, &insn)) {
> +return false;
> +}
> +
> +if (insn != 0xe97f) {
> +/* Not an SG instruction second half */
> +goto gen_invep;
> +}

... but somehow expects to get two different values read from the same address?

Certainly you'd get the wrong exception frame if you incremented r15 in between.

> +env->regs[15] += 4;

... that make this right and the implicit address to the readers wrong.

I don't see anything else amiss in the patch.


r~

[Qemu-devel] [PATCH] Make scrolling work again

[John Arbuckle]

Make scrolling in the monitor work.

Signed-off-by: John Arbuckle 
---
 ui/cocoa.m | 88 +++---
 1 file changed, 56 insertions(+), 32 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 93e56d0518..5545c42b9c 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -281,6 +281,7 @@ - (void) switchSurface:(DisplaySurface *)surface;
 - (void) grabMouse;
 - (void) ungrabMouse;
 - (void) toggleFullScreen:(id)sender;
+- (void) handleMonitorInput:(NSEvent *)event;
 - (void) handleEvent:(NSEvent *)event;
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
 /* The state surrounding mouse grabbing is potentially confusing.
@@ -554,6 +555,60 @@ - (void) toggleStatefulModifier: (int)keycode {
 qemu_input_event_send_key_qcode(dcl->con, keycode, false);
 }
 
+// Does the work of sending input to the monitor
+- (void) handleMonitorInput:(NSEvent *)event
+{
+int keysym = 0;
+int control_key = 0;
+
+// if the control key is down
+if ([event modifierFlags] & NSEventModifierFlagControl) {
+control_key = 4; // shifts by one nibble (hex digit)
+}
+
+/* translates Macintosh keycodes to QEMU's keysym */
+int translation_matrix[] = {
+[0 ... 0xff] = 0,   // invalid key
+
+[kVK_UpArrow]   = QEMU_KEY_UP,
+[kVK_DownArrow] = QEMU_KEY_DOWN,
+[kVK_RightArrow]= QEMU_KEY_RIGHT,
+[kVK_LeftArrow] = QEMU_KEY_LEFT,
+[kVK_Home]  = QEMU_KEY_HOME,
+[kVK_End]   = QEMU_KEY_END,
+[kVK_PageUp]= QEMU_KEY_PAGEUP,
+[kVK_PageDown]  = QEMU_KEY_PAGEDOWN,
+[kVK_ForwardDelete] = QEMU_KEY_DELETE,
+[kVK_Delete]= QEMU_KEY_BACKSPACE,
+
+/*
+ * Shift value by one hex digit.
+ * Since no key has a 3 digit hex value there is no chance
+ * for overlap.
+ */
+[kVK_UpArrow << 4]   = QEMU_KEY_CTRL_UP,
+[kVK_DownArrow << 4] = QEMU_KEY_CTRL_DOWN,
+[kVK_RightArrow << 4]= QEMU_KEY_CTRL_RIGHT,
+[kVK_LeftArrow << 4] = QEMU_KEY_CTRL_LEFT,
+[kVK_Home << 4]  = QEMU_KEY_CTRL_HOME,
+[kVK_End << 4]   = QEMU_KEY_CTRL_END,
+[kVK_PageUp << 4]= QEMU_KEY_CTRL_PAGEUP,
+[kVK_PageDown << 4]  = QEMU_KEY_CTRL_PAGEDOWN,
+};
+
+keysym = translation_matrix[[event keyCode] << control_key];
+
+// if not a key that needs translating
+if (keysym == 0) {
+NSString *ks = [event characters];
+if ([ks length] > 0)
+keysym = [ks characterAtIndex:0];
+}
+
+if(keysym)
+kbd_put_keysym(keysym);
+}
+
 - (void) handleEvent:(NSEvent *)event
 {
 COCOA_DEBUG("QemuCocoaView: handleEvent\n");
@@ -641,38 +696,7 @@ - (void) handleEvent:(NSEvent *)event
 
 // handlekeys for Monitor
 } else {
-int keysym = 0;
-switch([event keyCode]) {
-case 115:
-keysym = QEMU_KEY_HOME;
-break;
-case 117:
-keysym = QEMU_KEY_DELETE;
-break;
-case 119:
-keysym = QEMU_KEY_END;
-break;
-case 123:
-keysym = QEMU_KEY_LEFT;
-break;
-case 124:
-keysym = QEMU_KEY_RIGHT;
-break;
-case 125:
-keysym = QEMU_KEY_DOWN;
-break;
-case 126:
-keysym = QEMU_KEY_UP;
-break;
-default:
-{
-NSString *ks = [event characters];
-if ([ks length] > 0)
-keysym = [ks characterAtIndex:0];
-}
-}
-if (keysym)
-kbd_put_keysym(keysym);
+[self handleMonitorInput: event];
 }
 break;
 case NSEventTypeKeyUp:
-- 
2.13.5 (Apple Git-94)

Re: [Qemu-devel] [PATCH 16/20] target/arm: Factor out "get mmuidx for specified security state"

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> For the SG instruction and secure function return we are going
> to want to do memory accesses using the MMU index of the CPU
> in secure state, even though the CPU is currently in non-secure
> state. Write arm_v7m_mmu_idx_for_secstate() to do this job,
> and use it in cpu_mmu_index().
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/cpu.h | 32 +---
>  1 file changed, 21 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 15/20] target/arm: Fix calculation of secure mm_idx values

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> In cpu_mmu_index() we try to do this:
> if (env->v7m.secure) {
> mmu_idx += ARMMMUIdx_MSUser;
> }
> but it will give the wrong answer, because ARMMMUIdx_MSUser
> includes the 0x40 ARM_MMU_IDX_M field, and so does the
> mmu_idx we're adding to, and we'll end up with 0x8n rather
> than 0x4n. This error is then nullified by the call to
> arm_to_core_mmu_idx() which masks out the high part, but
> we're about to factor out the code that calculates the
> ARMMMUIdx values so it can be used without passing it through
> arm_to_core_mmu_idx(), so fix this bug first.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/cpu.h | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 14/20] target/arm: Implement security attribute lookups for memory accesses

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> Implement the security attribute lookups for memory accesses
> in the get_phys_addr() functions, causing these to generate
> various kinds of SecureFault for bad accesses.
> 
> The major subtlety in this code relates to handling of the
> case when the security attributes the SAU assigns to the
> address don't match the current security state of the CPU.
> 
> In the ARM ARM pseudocode for validating instruction
> accesses, the security attributes of the address determine
> whether the Secure or NonSecure MPU state is used. At face
> value, handling this would require us to encode the relevant
> bits of state into mmu_idx for both S and NS at once, which
> would result in our needing 16 mmu indexes. Fortunately we
> don't actually need to do this because a mismatch between
> address attributes and CPU state means either:
>  * some kind of fault (usually a SecureFault, but in theory
>perhaps a UserFault for unaligned access to Device memory)
>  * execution of the SG instruction in NS state from a
>Secure & NonSecure code region
> 
> The purpose of SG is simply to flip the CPU into Secure
> state, so we can handle it by emulating execution of that
> instruction directly in arm_v7m_cpu_do_interrupt(), which
> means we can treat all the mismatch cases as "throw an
> exception" and we don't need to encode the state of the
> other MPU bank into our mmu_idx values.
> 
> This commit doesn't include the actual emulation of SG;
> it also doesn't include implementation of the IDAU, which
> is a per-board way to specify hard-coded memory attributes
> for addresses, which override the CPU-internal SAU if they
> specify a more secure setting than the SAU is programmed to.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/internals.h |  15 
>  target/arm/helper.c| 182 
> -
>  2 files changed, 195 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 11/23] ppc: spapr: replace ppc_cpu_parse_features() with cpu_parse_cpu_model()

[Greg Kurz]

On Thu,  5 Oct 2017 18:24:38 +0200
Igor Mammedov  wrote:

> ppc_cpu_parse_features() is doing practically the same thing as
> generic cpu_parse_cpu_model(). So remove duplicated impl. and
> reuse generic one.
> 
> Signed-off-by: Igor Mammedov 
> ---

Reviewed-by: Greg Kurz 

>  include/hw/ppc/ppc.h|  2 --
>  hw/ppc/ppc.c| 25 -
>  hw/ppc/spapr_cpu_core.c |  9 -
>  3 files changed, 4 insertions(+), 32 deletions(-)
> 
> diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
> index 4e7fe11..ff0ac30 100644
> --- a/include/hw/ppc/ppc.h
> +++ b/include/hw/ppc/ppc.h
> @@ -105,6 +105,4 @@ enum {
>  
>  /* ppc_booke.c */
>  void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
> -
> -void ppc_cpu_parse_features(const char *cpu_model);
>  #endif
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 05da316..7ec35de 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -1359,28 +1359,3 @@ void PPC_debug_write (void *opaque, uint32_t addr, 
> uint32_t val)
>  break;
>  }
>  }
> -
> -void ppc_cpu_parse_features(const char *cpu_model)
> -{
> -CPUClass *cc;
> -ObjectClass *oc;
> -const char *typename;
> -gchar **model_pieces;
> -
> -model_pieces = g_strsplit(cpu_model, ",", 2);
> -if (!model_pieces[0]) {
> -error_report("Invalid/empty CPU model name");
> -exit(1);
> -}
> -
> -oc = cpu_class_by_name(TYPE_POWERPC_CPU, model_pieces[0]);
> -if (oc == NULL) {
> -error_report("Unable to find CPU definition: %s", model_pieces[0]);
> -exit(1);
> -}
> -
> -typename = object_class_get_name(oc);
> -cc = CPU_CLASS(oc);
> -cc->parse_features(typename, model_pieces[1], &error_fatal);
> -g_strfreev(model_pieces);
> -}
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 3e20b1d..3dea5ff 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -34,6 +34,7 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>   *   before passing it on to the cpu level parser.
>   */
>  gchar **inpieces;
> +gchar *newprops;
>  int i, j;
>  gchar *compat_str = NULL;
>  
> @@ -58,17 +59,15 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>  
>  if (compat_str) {
>  char *val = compat_str + strlen("compat=");
> -gchar *newprops = g_strjoinv(",", inpieces);
>  
>  object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
>  &error_fatal);
>  
> -ppc_cpu_parse_features(newprops);
> -g_free(newprops);
> -} else {
> -ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
>  }
>  
> +newprops = g_strjoinv(",", inpieces);
> +cpu_parse_cpu_model(TYPE_POWERPC_CPU, newprops);
> +g_free(newprops);
>  g_strfreev(inpieces);
>  }
>  

Re: [Qemu-devel] [PATCH 13/20] nvic: Implement Security Attribution Unit registers

[Richard Henderson]

On 09/22/2017 11:00 AM, Peter Maydell wrote:
> Implement the register interface for the SAU: SAU_CTRL,
> SAU_TYPE, SAU_RNR, SAU_RBAR and SAU_RLAR. None of the
> actual behaviour is implemented here; registers just
> read back as written.
> 
> When the CPU definition for Cortex-M33 is eventually
> added, its initfn will set cpu->sau_sregion, in the same
> way that we currently set cpu->pmsav7_dregion for the
> M3 and M4.
> 
> Number of SAU regions is typically a configurable
> CPU parameter, but this patch doesn't provide a
> QEMU CPU property for it. We can easily add one when
> we have a board that requires it.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/cpu.h  |  10 +
>  hw/intc/armv7m_nvic.c | 116 
> ++
>  target/arm/cpu.c  |  27 
>  target/arm/machine.c  |  14 ++
>  4 files changed, 167 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 12/20] target/arm: Add v8M support to exception entry code

[Richard Henderson]

On 09/22/2017 10:59 AM, Peter Maydell wrote:
> Add support for v8M and in particular the security extension
> to the exception entry code. This requires changes to:
>  * calculation of the exception-return magic LR value
>  * push the callee-saves registers in certain cases
>  * clear registers when taking non-secure exceptions to avoid
>leaking information from the interrupted secure code
>  * switch to the correct security state on entry
>  * use the vector table for the security state we're targeting
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/helper.c | 165 
> +---
>  1 file changed, 145 insertions(+), 20 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] Qemu Documentation

[John Snow]

On 10/05/2017 12:03 AM, Swetheendra Tallamraju wrote:
> I am working on qemu source code to provide extra functionality of
> emulating virtual usb. Can I get any  documentation for the qemu source
> code that helps me in implementing this?
> 


The docs in source code and in the docs/ folder are what you get, more
or less. It will be a quicker process for you if you ask specific
questions about the codebase.

What types of docs are you looking for, for instance, or what you are
trying to accomplish.

--js

Re: [Qemu-devel] [PATCH v2 0/2] ui/cocoa.m: enable guest to see control-alt key combinations

[no-reply]

Hi,

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

Type: series
Message-id: 20171005145557.5746-1-programmingk...@gmail.com
Subject: [Qemu-devel] [PATCH v2 0/2] ui/cocoa.m: enable guest to see 
control-alt key combinations

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

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1506092407-26985-1-git-send-email-peter.mayd...@linaro.org -> 
patchew/1506092407-26985-1-git-send-email-peter.mayd...@linaro.org
 t [tag update]patchew/20171005155057.7664-1-berra...@redhat.com -> 
patchew/20171005155057.7664-1-berra...@redhat.com
Switched to a new branch 'test'
16fa5de081 ui/cocoa.m: send ctrl-alt key combinations to guest if not used by 
QEMU
d71057fc7d ui/cocoa.m: move ungrab to ctrl-alt-g

=== OUTPUT BEGIN ===
Checking PATCH 1/2: ui/cocoa.m: move ungrab to ctrl-alt-g...
ERROR: The correct form is "Signed-off-by"
#8: 
signed-off-by: John Arbuckle 

total: 1 errors, 0 warnings, 40 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/2: ui/cocoa.m: send ctrl-alt key combinations to guest if not 
used by QEMU...
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH v2 0/4] blockjobs: add explicit job reaping

[John Snow]

Nikolay: You mentioned a while ago that you had issues with incremental
backup's eventual return status being unknown. Can you please elaborate
for me why this is a problem?

I assume due to the long running of a backup job it's entirely possible
to imagine losing connection to QEMU and missing the event depending on
how long the interruption is.

Backup operations are expensive, so we need some definite way to catch
this return status.

Please let me know if you have any feedback to this thread.

On 10/05/2017 07:38 AM, Kevin Wolf wrote:
> Am 05.10.2017 um 03:46 hat John Snow geschrieben:
>> On 10/04/2017 02:27 PM, Kevin Wolf wrote:
>>> Am 04.10.2017 um 03:52 hat John Snow geschrieben: For jobs that 
>>> complete when a monitor isn't looking, there's no way to
 tell what the job's final return code was. We need to allow jobs to
 remain in the list until queried for reliable management.
>>>
>>> Just a short summary of what I discussed with John on IRC:
>>>
>>> Another important reason why we want to have an explicit end of block
>>> jobs is that job completion often makes changes to the graph. For a
>>> management tool that manages the block graph on a node level, it is a
>>> big problem if graph changes can happen at any point that can lead to
>>> bad race conditions. Giving the management tool control over the end of
>>> the block job makes it aware that graph changes happen.
>>>
>>> This means that compared to this RFC series, we need to move the waiting
>>> earlier in the process:
>>>
>>> 1. Block job is done and calls block_job_completed()
>>> 2. Wait for other block jobs in the same job transaction to complete
>>> 3. Send a (new) QMP event to the management tool to notify it that the
>>>job is ready to be reaped
>>
>> Oh, I suppose to distinguish it from "COMPLETED" in that sense, because
>> it isn't actually COMPLETED anymore under your vision, so it requires a
>> new event in this proposal.
>>
>> This becomes a bit messy, bumping up against both "READY" and a
>> transactional pre-completed state semantically. U, for lack of a
>> better word in the timeframe I'd like to complete this email in, let's
>> call this new theoretical state "PENDING"?
>>
>> So presently, a job goes through the following life cycle:
>>
>> 1. CREATED --> RUNNING
>> 2. RUNNING <--> PAUSED
>> 3. RUNNING --> (READY | COMPLETED | CANCELED)
>> 4. READY --> (COMPLETED | CANCELED)
>> 5. (COMPLETED | CANCELED) --> NULL
>>
>> Where we emit an event upon entering "READY", "COMPLETED" or "CANCELED".
> 
> Roughly yes, but it's not quite true because you can still pause and
> unpause ready jobs. So READY and PAUSED are kind of orthogonal.
> 

But you cannot block-job-complete a running job, so I included it here
so we could keep the concept of the ready-to-complete state in mind.

>> My patchset here effectively adds a new optional terminal state:
>>
>> 5. (COMPLETED | CANCELED) --> (NULL | FINISHED)
>> 6. FINISHED --> NULL
>>
>> Where the last transition from FINISHED to NULL is performed via
>> block-job-reap, but notably we get to re-use the events for COMPLETED |
>> CANCELED to indicate the availability of this operation to be performed.
>>
>> What happens in the case of transactionally managed jobs presently is
>> that jobs get stuck as they enter the COMPLETED|CANCELED state. If you
>> were to query them they behave as if they're RUNNING. There's no
>> discrete state that exists for this presently.
>>
>> You can cancel these as normal, but I'm not sure if you can pause them,
>> actually. (Note to self, test that.) I think they have almost exactly
>> like any RUNNING job would.
> 
> Except that they don't do any work any more. This is an mportant
> difference for a mirror job which would normally keep copying new writes
> until it sends the COMPLETED event. So when libvirt restarts and it sees
> a "RUNNING" mirror job, it can't decide whether it is still copying
> things or has already completed.
> 
> Looks like this is another reason why we want a separate state here.

Yes, I realized as I was writing it that we have no real way to tell
that a job is simply pending completion.

> 
>> What you're proposing here is the formalization of the pre-completion
>> state ("PENDING") and that in this state, a job outside of a transaction
>> can exist until it is manually told to finally, once and for all,
>> actually finish its business. We can use this as a hook to perform and
>> last graph changes so they will not come as a surprise to the management
>> application. Maybe this operation should be called "Finalize". Again,
>> for lack of a better term in the timeframe, I'll refer to it as such for
>> now.
> 
> "finalize" doesn't sound too bad.
> 

Though taken altogether, the set of names we've accumulated is a little
ridiculous.

>> I think importantly this actually distinguishes it from "reap" in that
>> the commit phase can still fail, so we can't let the job follow that
>> auto transition back to the NULL state.
> 
> 

Re: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_pop

[Alexandre DERUMIER]

Hi,

has somebody reviewed this patch ?

I'm also able de reproduce the vm crash like the proxmox user.
This patch is fixing it for me too.

Regards,

Alexandre


- Mail original -
De: "Wolfgang Bumiller" 
À: "qemu-devel" 
Cc: "pbonzini" , "Michael S. Tsirkin" 
Envoyé: Mercredi 20 Septembre 2017 08:09:33
Objet: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_pop

While changing the s/g list allocation, commit 3b3b0628 
also changed the descriptor counting to count iovec entries 
as split by cpu_physical_memory_map(). Previously only the 
actual descriptor entries were counted and the split into 
the iovec happened afterwards in virtqueue_map(). 
Count the entries again instead to avoid erroneous 
"Looped descriptor" errors. 

Reported-by: Hans Middelhoek  
Link: https://forum.proxmox.com/threads/vm-crash-with-memory-hotplug.35904/ 
Fixes: 3b3b0628217e ("virtio: slim down allocation of VirtQueueElements") 
Signed-off-by: Wolfgang Bumiller  
--- 
hw/virtio/virtio.c | 6 +++--- 
1 file changed, 3 insertions(+), 3 deletions(-) 

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c 
index 890b4d7eb7..33bb770177 100644 
--- a/hw/virtio/virtio.c 
+++ b/hw/virtio/virtio.c 
@@ -834,7 +834,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) 
int64_t len; 
VirtIODevice *vdev = vq->vdev; 
VirtQueueElement *elem = NULL; 
- unsigned out_num, in_num; 
+ unsigned out_num, in_num, elem_entries; 
hwaddr addr[VIRTQUEUE_MAX_SIZE]; 
struct iovec iov[VIRTQUEUE_MAX_SIZE]; 
VRingDesc desc; 
@@ -852,7 +852,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) 
smp_rmb(); 

/* When we start there are none of either input nor output. */ 
- out_num = in_num = 0; 
+ out_num = in_num = elem_entries = 0; 

max = vq->vring.num; 

@@ -922,7 +922,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) 
} 

/* If we've got too many, that implies a descriptor loop. */ 
- if ((in_num + out_num) > max) { 
+ if (++elem_entries > max) { 
virtio_error(vdev, "Looped descriptor"); 
goto err_undo_map; 
} 
-- 
2.11.0 

Re: [Qemu-devel] [PATCH 11/20] target/arm: Add support for restoring v8M additional state context

[Richard Henderson]

On 09/22/2017 10:59 AM, Peter Maydell wrote:
> For v8M, exceptions from Secure to Non-Secure state will save
> callee-saved registers to the exception frame as well as the
> caller-saved registers. Add support for unstacking these
> registers in exception exit when necessary.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/helper.c | 30 ++
>  1 file changed, 30 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 33/40] sparc: sparc: use generic cpu_model parsing

[Mark Cave-Ayland]

On 05/10/17 14:51, Igor Mammedov wrote:

> Signed-off-by: Igor Mammedov 
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
> ---
> CC: mark.cave-ayl...@ilande.co.uk
> CC: atar4q...@gmail.com
> ---
>  hw/sparc/sun4m.c | 29 -
>  1 file changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index e1bdd48..68b2378 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -94,7 +94,6 @@ struct sun4m_hwdef {
>  } vsimm[MAX_VSIMMS];
>  hwaddr ecc_base;
>  uint64_t max_mem;
> -const char * const default_cpu_model;
>  uint32_t ecc_version;
>  uint32_t iommu_version;
>  uint16_t machine_id;
> @@ -790,14 +789,14 @@ static const TypeInfo ram_info = {
>  .class_init= ram_class_init,
>  };
>  
> -static void cpu_devinit(const char *cpu_model, unsigned int id,
> +static void cpu_devinit(const char *cpu_type, unsigned int id,
>  uint64_t prom_addr, qemu_irq **cpu_irqs)
>  {
>  CPUState *cs;
>  SPARCCPU *cpu;
>  CPUSPARCState *env;
>  
> -cpu = SPARC_CPU(cpu_generic_init(TYPE_SPARC_CPU, cpu_model));
> +cpu = SPARC_CPU(cpu_create(cpu_type));
>  env = &cpu->env;
>  
>  cpu_sparc_set_id(env, id);
> @@ -820,7 +819,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>MachineState *machine)
>  {
>  DeviceState *slavio_intctl;
> -const char *cpu_model = machine->cpu_model;
>  unsigned int i;
>  void *iommu, *espdma, *ledma, *nvram;
>  qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS],
> @@ -833,11 +831,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
> *hwdef,
>  unsigned int num_vsimms;
>  
>  /* init CPUs */
> -if (!cpu_model)
> -cpu_model = hwdef->default_cpu_model;
> -
>  for(i = 0; i < smp_cpus; i++) {
> -cpu_devinit(cpu_model, i, hwdef->slavio_base, &cpu_irqs[i]);
> +cpu_devinit(machine->cpu_type, i, hwdef->slavio_base, &cpu_irqs[i]);
>  }
>  
>  for (i = smp_cpus; i < MAX_CPUS; i++)
> @@ -1074,7 +1069,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = ss5_id,
>  .iommu_version = 0x0500,
>  .max_mem = 0x1000,
> -.default_cpu_model = "Fujitsu MB86904",
>  },
>  /* SS-10 */
>  {
> @@ -1100,7 +1094,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = ss10_id,
>  .iommu_version = 0x0300,
>  .max_mem = 0xfULL,
> -.default_cpu_model = "TI SuperSparc II",
>  },
>  /* SS-600MP */
>  {
> @@ -1124,7 +1117,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = ss600mp_id,
>  .iommu_version = 0x0100,
>  .max_mem = 0xfULL,
> -.default_cpu_model = "TI SuperSparc II",
>  },
>  /* SS-20 */
>  {
> @@ -1166,7 +1158,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = ss20_id,
>  .iommu_version = 0x1300,
>  .max_mem = 0xfULL,
> -.default_cpu_model = "TI SuperSparc II",
>  },
>  /* Voyager */
>  {
> @@ -1190,7 +1181,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = vger_id,
>  .iommu_version = 0x0500,
>  .max_mem = 0x1000,
> -.default_cpu_model = "Fujitsu MB86904",
>  },
>  /* LX */
>  {
> @@ -1215,7 +1205,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = lx_id,
>  .iommu_version = 0x0400,
>  .max_mem = 0x1000,
> -.default_cpu_model = "TI MicroSparc I",
>  },
>  /* SS-4 */
>  {
> @@ -1240,7 +1229,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = ss4_id,
>  .iommu_version = 0x0500,
>  .max_mem = 0x1000,
> -.default_cpu_model = "Fujitsu MB86904",
>  },
>  /* SPARCClassic */
>  {
> @@ -1264,7 +1252,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = scls_id,
>  .iommu_version = 0x0500,
>  .max_mem = 0x1000,
> -.default_cpu_model = "TI MicroSparc I",
>  },
>  /* SPARCbook */
>  {
> @@ -1288,7 +1275,6 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
>  .machine_id = sbook_id,
>  .iommu_version = 0x0500,
>  .max_mem = 0x1000,
> -.default_cpu_model = "TI MicroSparc I",
>  },
>  };
>  
> @@ -1355,6 +1341,7 @@ static void ss5_class_init(ObjectClass *oc, void *data)
>  mc->block_default_type = IF_SCSI;
>  mc->is_default = 1;
>  mc->default_boot_order = "c";
> +mc->default_cpu_type = SPARC_CPU_TYPE_NAME("Fujitsu-MB86904");
>  }
>  
>  static const TypeInfo ss5_type = {
> @@ -1372,6 +1359,7 @@ static void ss10_class_init(ObjectClass *oc, void *data)
>  mc->block_default_type = IF_

[Qemu-devel] [PATCH v2 3/3] scripts: Remove debug parameter from QEMUMachine

[Eduardo Habkost]

All scripts that use the QEMUMachine and QEMUQtestMachine classes
(device-crash-test, tests/migration/*, iotests.py, basevm.py)
already configure logging.

The basicConfig() call inside QEMUMachine.__init__() is being
kept just to make sure a script would still work if it didn't
configure logging.

Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 6 ++
 tests/migration/guestperf/engine.py | 6 ++
 tests/qemu-iotests/iotests.py   | 2 --
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f6d2e68627..9bfdf6d37d 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -54,7 +54,7 @@ class QEMUMachine(object):
 
 def __init__(self, binary, args=None, wrapper=None, name=None,
  test_dir="/var/tmp", monitor_address=None,
- socket_scm_helper=None, debug=False):
+ socket_scm_helper=None):
 '''
 Initialize a QEMUMachine
 
@@ -65,7 +65,6 @@ class QEMUMachine(object):
 @param test_dir: where to create socket and log file
 @param monitor_address: address for QMP monitor
 @param socket_scm_helper: helper program, required for send_fd_scm()"
-@param debug: enable debug mode
 @note: Qemu process is not started until launch() is used.
 '''
 if args is None:
@@ -85,12 +84,11 @@ class QEMUMachine(object):
 self._events = []
 self._iolog = None
 self._socket_scm_helper = socket_scm_helper
-self._debug = debug
 self._qmp = None
 self._qemu_full_args = None
 
 # just in case logging wasn't configured by the main script:
-logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
+logging.basicConfig()
 
 def __enter__(self):
 return self
diff --git a/tests/migration/guestperf/engine.py 
b/tests/migration/guestperf/engine.py
index 0a13050bc6..e14d4320b2 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -388,15 +388,13 @@ class Engine(object):
args=self._get_src_args(hardware),
wrapper=self._get_src_wrapper(hardware),
name="qemu-src-%d" % os.getpid(),
-   monitor_address=srcmonaddr,
-   debug=self._debug)
+   monitor_address=srcmonaddr)
 
 dst = qemu.QEMUMachine(self._binary,
args=self._get_dst_args(hardware, uri),
wrapper=self._get_dst_wrapper(hardware),
name="qemu-dst-%d" % os.getpid(),
-   monitor_address=dstmonaddr,
-   debug=self._debug)
+   monitor_address=dstmonaddr)
 
 try:
 src.launch()
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 36a7757aaf..6f057904a9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -195,8 +195,6 @@ class VM(qtest.QEMUQtestMachine):
 super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
  test_dir=test_dir,
  socket_scm_helper=socket_scm_helper)
-if debug:
-self._debug = True
 self._num_drives = 0
 
 def add_device(self, opts):
-- 
2.13.6

[Qemu-devel] [PATCH v2 1/3] guestperf: Configure logging on all shell frontends

[Eduardo Habkost]

The logging module will eventually replace the 'debug' parameter
in QEMUMachine and QEMUMonitorProtocol.

Cc: Daniel P. Berrange 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Inline init_logging() method on all callers because not all
  classes derive from BaseShell (reported by Lukáš Doktor)
---
 tests/migration/guestperf/shell.py | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tests/migration/guestperf/shell.py 
b/tests/migration/guestperf/shell.py
index 7992459a97..b272978f47 100644
--- a/tests/migration/guestperf/shell.py
+++ b/tests/migration/guestperf/shell.py
@@ -26,6 +26,7 @@ sys.path.append(os.path.join(os.path.dirname(__file__),
 import argparse
 import fnmatch
 import platform
+import logging
 
 from guestperf.hardware import Hardware
 from guestperf.engine import Engine
@@ -147,6 +148,10 @@ class Shell(BaseShell):
 
 def run(self, argv):
 args = self._parser.parse_args(argv)
+logging.basicConfig(level=(logging.DEBUG if args.debug else
+   logging.INFO if args.verbose else
+   logging.WARN))
+
 
 engine = self.get_engine(args)
 hardware = self.get_hardware(args)
@@ -179,6 +184,10 @@ class BatchShell(BaseShell):
 
 def run(self, argv):
 args = self._parser.parse_args(argv)
+logging.basicConfig(level=(logging.DEBUG if args.debug else
+   logging.INFO if args.verbose else
+   logging.WARN))
+
 
 engine = self.get_engine(args)
 hardware = self.get_hardware(args)
@@ -231,6 +240,10 @@ class PlotShell(object):
 
 def run(self, argv):
 args = self._parser.parse_args(argv)
+logging.basicConfig(level=(logging.DEBUG if args.debug else
+   logging.INFO if args.verbose else
+   logging.WARN))
+
 
 if len(args.reports) == 0:
 print >>sys.stderr, "At least one report required"
-- 
2.13.6

[Qemu-devel] [PATCH v2 2/3] scripts: Remove debug parameter from QEMUMonitorProtocol

[Eduardo Habkost]

Use logging module for the QMP debug messages.  The only scripts
that set debug=True are iotests.py and guestperf/engine.py, and
they already call logging.basicConfig() to set up logging.

Scripts that don't configure logging are safe as long as they
don't need debugging output, because debug messages don't trigger
the "No handlers could be found for logger" message from the
Python logging module.

Scripts that already configure logging but don't use debug=True
(e.g. scripts/vm/basevm.py) will get QMP debugging enabled for
free.

Cc: "Alex Bennée" 
Cc: Fam Zheng 
Cc: "Philippe Mathieu-Daudé" 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Actually remove debug parameter from method definition
  (Fam Zheng)
* Fix "<<<" vs ">>>" confusion
  (Fam Zheng)
* Remove "import sys" line
  (Lukáš Doktor)
---
 scripts/qemu.py|  3 +--
 scripts/qmp/qmp.py | 16 +++-
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index c9a106fbce..f6d2e68627 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -177,8 +177,7 @@ class QEMUMachine(object):
 
 def _pre_launch(self):
 self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
-server=True,
-debug=self._debug)
+server=True)
 
 def _post_launch(self):
 self._qmp.accept()
diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index ef12e8a1a0..07c9632e9e 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -11,7 +11,7 @@
 import json
 import errno
 import socket
-import sys
+import logging
 
 
 class QMPError(Exception):
@@ -32,12 +32,14 @@ class QMPTimeoutError(QMPError):
 
 class QEMUMonitorProtocol(object):
 
+#: Logger object for debugging messages
+logger = logging.getLogger('QMP')
 #: Socket's error class
 error = socket.error
 #: Socket's timeout
 timeout = socket.timeout
 
-def __init__(self, address, server=False, debug=False):
+def __init__(self, address, server=False):
 """
 Create a QEMUMonitorProtocol class.
 
@@ -51,7 +53,6 @@ class QEMUMonitorProtocol(object):
 """
 self.__events = []
 self.__address = address
-self._debug = debug
 self.__sock = self.__get_sock()
 self.__sockfile = None
 if server:
@@ -83,8 +84,7 @@ class QEMUMonitorProtocol(object):
 return
 resp = json.loads(data)
 if 'event' in resp:
-if self._debug:
-print >>sys.stderr, "QMP:<<< %s" % resp
+self.logger.debug("<<< %s", resp)
 self.__events.append(resp)
 if not only_event:
 continue
@@ -164,8 +164,7 @@ class QEMUMonitorProtocol(object):
 @return QMP response as a Python dict or None if the connection has
 been closed
 """
-if self._debug:
-print >>sys.stderr, "QMP:>>> %s" % qmp_cmd
+self.logger.debug(">>> %s", qmp_cmd)
 try:
 self.__sock.sendall(json.dumps(qmp_cmd))
 except socket.error as err:
@@ -173,8 +172,7 @@ class QEMUMonitorProtocol(object):
 return
 raise socket.error(err)
 resp = self.__json_read()
-if self._debug:
-print >>sys.stderr, "QMP:<<< %s" % resp
+self.logger.debug("<<< %s", resp)
 return resp
 
 def cmd(self, name, args=None, cmd_id=None):
-- 
2.13.6

[Qemu-devel] [PATCH v2 0/3] scripts: Remove 'debug' parameter from QEMUMachine & QEMUMonitorProtocol

[Eduardo Habkost]

Changes v1 -> v2:
* Rebased to python-next (some patches from v1 are already queued
  there)S
* guestperf: Inline init_logging() method on all callers because
  not all classes derive from BaseShell (reported by Lukáš
  Doktor)

This series removes the 'debug' parameter from QEMUMachine and
QEMUMonitorProtocol and lets scripts use the logging module to
enable debugging messages.

Eduardo Habkost (3):
  guestperf: Configure logging on all shell frontends
  scripts: Remove debug parameter from QEMUMonitorProtocol
  scripts: Remove debug parameter from QEMUMachine

 scripts/qemu.py |  9 +++--
 scripts/qmp/qmp.py  | 16 +++-
 tests/migration/guestperf/engine.py |  6 ++
 tests/migration/guestperf/shell.py  | 13 +
 tests/qemu-iotests/iotests.py   |  2 --
 5 files changed, 25 insertions(+), 21 deletions(-)

-- 
2.13.6

Re: [Qemu-devel] [PATCH 10/20] target/arm: Update excret sanity checks for v8M

[Richard Henderson]

On 09/22/2017 10:59 AM, Peter Maydell wrote:
> In v8M, more bits are defined in the exception-return magic
> values; update the code that checks these so we accept
> the v8M values when the CPU permits them.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/helper.c | 73 
> ++---
>  1 file changed, 58 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [REBASED 2/2] exec: simplify address_space_get_iotlb_entry

[Maxime Coquelin]

From: Peter Xu 

This patch let address_space_get_iotlb_entry() to use the newly
introduced page_mask parameter in flatview_do_translate(). Then we
will be sure the IOTLB can be aligned to page mask, also we should
nicely support huge pages now when introducing a764040.

Fixes: a764040 ("exec: abstract address_space_do_translate()")
Signed-off-by: Peter Xu 
Signed-off-by: Maxime Coquelin 
---
 exec.c | 31 ++-
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/exec.c b/exec.c
index c5f2752f7d..39fc96a19e 100644
--- a/exec.c
+++ b/exec.c
@@ -551,14 +551,14 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace 
*as, hwaddr addr,
 bool is_write)
 {
 MemoryRegionSection section;
-hwaddr xlat, plen;
+hwaddr xlat, page_mask;
 
-/* Try to get maximum page mask during translation. */
-plen = (hwaddr)-1;
-
-/* This can never be MMIO. */
-section = flatview_do_translate(address_space_to_flatview(as), addr,
-&xlat, &plen, NULL, is_write, false, &as);
+/*
+ * This can never be MMIO, and we don't really care about plen,
+ * but page mask.
+ */
+section = flatview_do_translate(address_space_to_flatview(as), addr, &xlat,
+NULL, &page_mask, is_write, false, &as);
 
 /* Illegal translation */
 if (section.mr == &io_mem_unassigned) {
@@ -569,22 +569,11 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace 
*as, hwaddr addr,
 xlat += section.offset_within_address_space -
 section.offset_within_region;
 
-if (plen == (hwaddr)-1) {
-/*
- * We use default page size here. Logically it only happens
- * for identity mappings.
- */
-plen = TARGET_PAGE_SIZE;
-}
-
-/* Convert to address mask */
-plen -= 1;
-
 return (IOMMUTLBEntry) {
 .target_as = as,
-.iova = addr & ~plen,
-.translated_addr = xlat & ~plen,
-.addr_mask = plen,
+.iova = addr & ~page_mask,
+.translated_addr = xlat & ~page_mask,
+.addr_mask = page_mask,
 /* IOTLBs are for DMAs, and DMA only allows on RAMs. */
 .perm = IOMMU_RW,
 };
-- 
2.13.6

Re: [Qemu-devel] [PATCH 1/2] qdev_monitor: Simplify error handling in qdev_device_add()

[Eduardo Habkost]

On Thu, Oct 05, 2017 at 03:59:12PM +0200, Igor Mammedov wrote:
> On Thu,  5 Oct 2017 14:32:17 +0200
> Thomas Huth  wrote:
> 
> > Instead of doing the clean-ups on errors multiple times, introduce
> > a jump label at the end of the function that can be used by all
> > error paths that need this cleanup.
> > 
> > Suggested-by: Igor Mammedov 
> > Signed-off-by: Thomas Huth 
> > ---
> >  qdev-monitor.c | 21 ++---
> >  1 file changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index 8fd6df9..cb2b109 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -620,22 +620,21 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error 
> > **errp)
> >  
> >  /* set properties */
> >  if (qemu_opt_foreach(opts, set_property, dev, &err)) {
> > -error_propagate(errp, err);
> > -object_unparent(OBJECT(dev));
> > -object_unref(OBJECT(dev));
> > -return NULL;
> > +goto err_del_dev;
> >  }
> >  
> >  dev->opts = opts;
> >  object_property_set_bool(OBJECT(dev), true, "realized", &err);
> > -if (err != NULL) {
> > -error_propagate(errp, err);
> > -dev->opts = NULL;
> > -object_unparent(OBJECT(dev));
> > -object_unref(OBJECT(dev));
> > -return NULL;
> > +if (!err) {
> > +return dev;
> >  }
> typically the same error check pattern is used through out the function
> so I'd not do inversion here. i.e. keep normal flow non-branched and jump on 
> error
> 
>  if (err != NULL) {
> goto err_del_dev;
>  }
>  return dev;
> 
>  err_del_dev:
> dev->opts = NULL;

I prefer this pattern also.  It makes the success/error paths
very easy to spot.

-- 
Eduardo

[Qemu-devel] [REBASED 1/2] exec: add page_mask for flatview_do_translate

[Maxime Coquelin]

From: Peter Xu 

The function is originally used for flatview_space_translate() and what
we care about most is (xlat, plen) range. However for iotlb requests, we
don't really care about "plen", but the size of the page that "xlat" is
located on. While, plen cannot really contain this information.

A simple example to show why "plen" is not good for IOTLB translations:

E.g., for huge pages, it is possible that guest mapped 1G huge page on
device side that used this GPA range:

  0x1 - 0x13fff

Then let's say we want to translate one IOVA that finally mapped to GPA
0x13e00 (which is located on this 1G huge page). Then here we'll
get:

  (xlat, plen) = (0x13fffe00, 0x200)

So the IOTLB would be only covering a very small range since from
"plen" (which is 0x200 bytes) we cannot tell the size of the page.

Actually we can really know that this is a huge page - we just throw the
information away in flatview_do_translate().

This patch introduced "page_mask" optional parameter to capture that
page mask info. Also, I made "plen" an optional parameter as well, with
some comments for the whole function.

No functional change yet.

Signed-off-by: Peter Xu 
Signed-off-by: Maxime Coquelin 
---
 exec.c | 46 --
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 7a80460725..c5f2752f7d 100644
--- a/exec.c
+++ b/exec.c
@@ -467,11 +467,29 @@ address_space_translate_internal(AddressSpaceDispatch *d, 
hwaddr addr, hwaddr *x
 return section;
 }
 
-/* Called from RCU critical section */
+/**
+ * flatview_do_translate - translate an address in FlatView
+ *
+ * @fv: the flat view that we want to translate on
+ * @addr: the address to be translated in above address space
+ * @xlat: the translated address offset within memory region. It
+ *cannot be @NULL.
+ * @plen_out: valid read/write length of the translated address. It
+ *can be @NULL when we don't care about it.
+ * @page_mask_out: page mask for the translated address. This
+ *should only be meaningful for IOMMU translated
+ *addresses, since there may be huge pages that this bit
+ *would tell. It can be @NULL if we don't care about it.
+ * @is_write: whether the translation operation is for write
+ * @is_mmio: whether this can be MMIO, set true if it can
+ *
+ * This function is called from RCU critical section
+ */
 static MemoryRegionSection flatview_do_translate(FlatView *fv,
  hwaddr addr,
  hwaddr *xlat,
- hwaddr *plen,
+ hwaddr *plen_out,
+ hwaddr *page_mask_out,
  bool is_write,
  bool is_mmio,
  AddressSpace **target_as)
@@ -480,11 +498,17 @@ static MemoryRegionSection flatview_do_translate(FlatView 
*fv,
 MemoryRegionSection *section;
 IOMMUMemoryRegion *iommu_mr;
 IOMMUMemoryRegionClass *imrc;
+hwaddr page_mask = TARGET_PAGE_MASK;
+hwaddr plen = (hwaddr)(-1);
+
+if (plen_out) {
+plen = *plen_out;
+}
 
 for (;;) {
 section = address_space_translate_internal(
 flatview_to_dispatch(fv), addr, &addr,
-plen, is_mmio);
+&plen, is_mmio);
 
 iommu_mr = memory_region_get_iommu(section->mr);
 if (!iommu_mr) {
@@ -496,7 +520,8 @@ static MemoryRegionSection flatview_do_translate(FlatView 
*fv,
 IOMMU_WO : IOMMU_RO);
 addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
 | (addr & iotlb.addr_mask));
-*plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
+page_mask = iotlb.addr_mask;
+plen = MIN(plen, (addr | iotlb.addr_mask) - addr + 1);
 if (!(iotlb.perm & (1 << is_write))) {
 goto translate_fail;
 }
@@ -507,6 +532,14 @@ static MemoryRegionSection flatview_do_translate(FlatView 
*fv,
 
 *xlat = addr;
 
+if (page_mask_out) {
+*page_mask_out = page_mask;
+}
+
+if (plen_out) {
+*plen_out = plen;
+}
+
 return *section;
 
 translate_fail:
@@ -525,7 +558,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace 
*as, hwaddr addr,
 
 /* This can never be MMIO. */
 section = flatview_do_translate(address_space_to_flatview(as), addr,
-&xlat, &plen, is_write, false, &as);
+&xlat, &plen, NULL, is_write, false, &as);
 
 /* Illegal translation */
 if (section.mr == &io_mem_unassigned) {
@@ -569,7 +602,8 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, 
hwaddr *xlat,
 AddressSpace *as = NULL;
 
   

[Qemu-devel] [REBASED 0/2] exec: further refine address_space_get_iotlb_entry()

[Maxime Coquelin]

This series is a rebase of the first two patches of Peter's series
improving address_space_get_iotlb_entry():
Message-Id: <1496404254-17429-1-git-send-email-pet...@redhat.com>

It is actually not only an improvement, but fixes a regression in the way
IOTLB updates sent to the backends are generated.
The regression is introduced by patch:
a764040cc8 ("exec: abstract address_space_do_translate()")

Prior to this patch IOTLB entries sent to the backend were aligned on the
guest page boundaries (both addresses and size).
For example, with the guest using 2MB pages:
 * Backend sends IOTLB miss request for iova = 0x112378fb4
 * QEMU replies with an IOTLB update with iova = 0x11220, size = 0x20
 * Bakend insert above entry in its cache and compute the translation
In this case, if the backend needs later to translate 0x112378004, it will
result in a cache it and no need to send another IOTLB miss.

With this patch, the addr of the IOTLB entry will be the address requested
via the IOTLB miss, the size is computed to cover the remaining of the guest
page.
The same example gives:
 * Backend sends IOTLB miss request for iova = 0x112378fb4
 * QEMU replies with an IOTLB update with iova = 112378fb4, size = 0x8704c
 * Bakend insert above entry in its cache and compute the translation
In this case, if the backend needs later to translate 0x112378004, it will
result in another cache miss:
 * Backend sends IOTLB miss request for iova = 0x112378004
 * QEMU replies with an IOTLB update with iova = 0x112378004, size = 0x87FFC
 * Bakend insert above entry in its cache and compute the translation
It results in having much more IOTLB misses, and more importantly it pollutes
the device IOTLB cache by multiplying the number of entries that moreover
overlap.

Note that current Kernel & User backends implementation do not merge contiguous
and overlapping IOTLB entries at device IOTLB cache insertion.

This series fixes this regression, so that IOTLB updates are aligned on
guest's page boundaries.

Peter Xu (2):
  exec: add page_mask for flatview_do_translate
  exec: simplify address_space_get_iotlb_entry

 exec.c | 75 +++---
 1 file changed, 49 insertions(+), 26 deletions(-)

-- 
2.13.6

Re: [Qemu-devel] [PATCH v1 3/5] xlnx-zcu102: Specify the valid CPUs

[Eduardo Habkost]

On Thu, Oct 05, 2017 at 11:04:27AM +0200, Igor Mammedov wrote:
> On Wed, 4 Oct 2017 14:39:20 -0700
> Alistair Francis  wrote:
> 
> > On Wed, Oct 4, 2017 at 9:34 AM, Eduardo Habkost  wrote:
> > > On Wed, Oct 04, 2017 at 03:08:16PM +0200, Igor Mammedov wrote:  
> > >> On Wed, 4 Oct 2017 09:28:51 -0300
> > >> Eduardo Habkost  wrote:
> > >>  
> > >> > On Wed, Oct 04, 2017 at 01:12:32PM +0200, Igor Mammedov wrote:  
> > >> > > On Tue, 3 Oct 2017 14:41:17 -0700
> > >> > > Alistair Francis  wrote:
> > >> > >  
> > >> > > > On Tue, Oct 3, 2017 at 1:36 PM, Eduardo Habkost 
> > >> > > >  wrote:  
> > >> > > > > On Tue, Oct 03, 2017 at 01:05:13PM -0700, Alistair Francis 
> > >> > > > > wrote:  
> > >> > > > >> List all possible valid CPU options.
> > >> > > > >>
> > >> > > > >> Signed-off-by: Alistair Francis 
> > >> > > > >> ---
> > >> > > > >>
> > >> > > > >>  hw/arm/xlnx-zcu102.c | 10 ++
> > >> > > > >>  hw/arm/xlnx-zynqmp.c | 16 +---
> > >> > > > >>  include/hw/arm/xlnx-zynqmp.h |  1 +
> > >> > > > >>  3 files changed, 20 insertions(+), 7 deletions(-)
> > >> > > > >>
> > >> > > > >> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> > >> > > > >> index 519a16ed98..039649e522 100644
> > >> > > > >> --- a/hw/arm/xlnx-zcu102.c
> > >> > > > >> +++ b/hw/arm/xlnx-zcu102.c
> > >> > > > >> @@ -98,6 +98,8 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, 
> > >> > > > >> MachineState *machine)
> > >> > > > >>  object_property_add_child(OBJECT(machine), "soc", 
> > >> > > > >> OBJECT(&s->soc),
> > >> > > > >>&error_abort);
> > >> > > > >>
> > >> > > > >> +object_property_set_str(OBJECT(&s->soc), 
> > >> > > > >> machine->cpu_type, "cpu-type",
> > >> > > > >> +&error_fatal);  
> > >> > > > >
> > >> > > > > Do you have plans to support other CPU types to xlnx_zynqmp in
> > >> > > > > the future?  If not, I wouldn't bother adding the cpu-type
> > >> > > > > property and the extra boilerplate code if it's always going to
> > >> > > > > be set to cortex-a53.  
> > >> > > >
> > >> > > > No, it'll always be A53.
> > >> > > >
> > >> > > > I did think of that, but I also wanted to use the new option! I 
> > >> > > > also
> > >> > > > think there is an advantage in sanely handling users '-cpu' option,
> > >> > > > before now we just ignored it, so I think it still does give a
> > >> > > > benefit. That'll be especially important on the Xilinx tree 
> > >> > > > (sometimes
> > >> > > > people use our machines with a different CPU to 'benchmark' or test
> > >> > > > other CPUs with our CoSimulation setup). So I think it does make 
> > >> > > > sense
> > >> > > > to keep in.  
> > >> > > if cpu isn't user settable, one could just outright die if cpu_type
> > >> > > is not NULL and say that user's CLI is wrong.
> > >> > > (i.e. don't give users illusion that they allowed to use '-cpu')  
> > >> >
> > >> > Isn't it exactly what this patch does, by setting:
> > >> > mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
> > >> > mc->valid_cpu_types = xlnx_zynqmp_valid_cpus;
> > >> > ?
> > >> >
> > >> > Except that "-cpu cortex-a53" won't die, which is a good thing.  
> > >> allowing "-cpu cortex-a53" here, would allow to use feature parsing
> > >> which weren't allowed or were ignored before if user supplied '-cpu'.
> > >> so I'd more strict and refuse any -cpu and break CLI that tries to use it
> > >> if board has non configurable cpu type. It would be easier to relax
> > >> restriction later if necessary.
> > >>
> > >> using validate_cpus here just to have users for the new code,
> > >> doesn't seem like valid justification and at that it makes board
> > >> code more complex where it's not necessary and build in cpu type
> > >> works just fine.  
> > >
> > > It's up to the board maintainer to decide what's the best option.
> > > Both features are independent from each other and can be
> > > implemented by machine core.  
> > 
> > N!
> > 
> > My hope with this series is that eventually we could hit a state where
> > every single machine acts the same way with the -cpu option.
> > 
> > I really don't like what we do now where some boards use it, some
> > boards error and some boars just ignore the option. I think we should
> > agree on something and every machine should follow the same flow so
> > that users know what to expect when they use the -cpu option.
> > 
> > If this means we allow machines to specify they don't support the
> > option or only have a single element in the list of supported options
> > doesn't really matter, but all machines should do the same thing.
> > 
> > >
> > > In either case, the valid_cpu_types feature will be still very
> > > useful for boards like pxa270 and sa1110, which support -cpu but
> > > only with specific families of CPU types (grep for
> > > "strncmp(cpu_type").
> > >  
> > >>
> > >> wrt centralized way to refuse -cpu if board doesn't support it,
> > >> (which is not really rel

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration

[Auger Eric]

Hi Linu,

On 05/10/2017 14:13, Auger Eric wrote:
> Hi Linu,
> 
> On 05/10/2017 13:54, Auger Eric wrote:
>> Hi Linu,
>> On 05/10/2017 12:46, Auger Eric wrote:
>>> Hi Linu,
>>> On 04/10/2017 13:49, Linu Cherian wrote:
 Hi Eric,


 On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
> Hi Linu,
>
> On 27/09/2017 11:21, Linu Cherian wrote:
>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>> Hi Linu,
>>>
>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
 Hi,

> -Original Message-
> From: Linu Cherian [mailto:linuc.dec...@gmail.com]
> Sent: Wednesday, September 27, 2017 1:11 PM
> To: Bharat Bhushan 
> Cc: eric.au...@redhat.com; eric.auger@gmail.com;
> peter.mayd...@linaro.org; alex.william...@redhat.com; m...@redhat.com;
> qemu-...@nongnu.org; qemu-devel@nongnu.org; kevin.t...@intel.com;
> marc.zyng...@arm.com; t...@semihalf.com; will.dea...@arm.com;
> drjo...@redhat.com; robin.mur...@arm.com; christoffer.d...@linaro.org;
> bharatb.ya...@gmail.com
> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>
> Hi,
>
> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>
>> This version is mainly about rebasing on v4 version on virtio-iommu
>> device framework from Eric Augur and addresing review comments.
>>
>> This patch series allows PCI pass-through using virtio-iommu.
>>
>> This series is based on:
>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>> [1] [RFC] virtio-iommu version 0.4
>> git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>
>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>> branch?
>>>
>>> Thanks
>>
>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>> Hope you are referring to the same.
>
> Yes that's the right one. I will also investigate on my side this 
> afternoon.
>
> Thanks
>
> Eric

 With the below workaround, atleast ping works for me.

 diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
 index 249964a..2904617 100644
 --- a/drivers/iommu/virtio-iommu.c
 +++ b/drivers/iommu/virtio-iommu.c
 .attach_dev = viommu_attach_dev,
 .map= viommu_map,
 .unmap  = viommu_unmap,
 -   .map_sg = viommu_map_sg,
 +   .map_sg = default_iommu_map_sg,
 .iova_to_phys   = viommu_iova_to_phys,
 .add_device = viommu_add_device,
 .remove_device  = viommu_remove_device,


 Looks like the qemu backend doesnt have support to handle the map requests 
 from 
 virtio_iommu_map_sg, since it  merges multiple map requests into one with 
 mapsize larger than page size(for eg. 0x5000). 
>>> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
>>> request for each sg element. The map size matches the sg element size.
>>> Then each request is sent separately in _viommu_send_reqs_sync. I don't
>>> see any concatenation. Looks Jean has a plan to check if it can
>>> concatenate anything (/* TODO: merge physically-contiguous mappings if
>>> any */) but this is not implemented yet.
>>
>> Hopefully I was just able to reproduce your issue with an igb device. I
>> keep on debugging...
>>
>> vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>
>>
>> Thanks
>>
>> Eric
>>>
>>> However you should be allowed to map 1 sg element of 5 pages and then
>>> notify the host about this event I think. Still looking at the code...
>>>
>>> I still can't reproduce the issue at the moment. What kind of device are
>>> you assigning?
>>>
>>> Thanks
>>>
>>> Eric

 Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
 the map size to be a power of 2.
> 
> Actually I missed the most important here ;-)

  if (len & iotlb->addr_mask) {
> This check looks suspiscious to me. In our case the len is not modified
> by the previous translation and it fails, I don't see why. It should be
> valid to be able to notify 5 granules.

So after discussion with Alex, looks the way we notify the host
currently is wrong. we set the addr_mask to the mapping/unmapping size
-1 whereas this should be a page mask instead (granule size or block
size?). So if the guest maps 5 x 4kB pages we should send 5
notifications for each page and not a single one. It is unclear to me if
we can notify with hugepage/block page size mask. Peter may
confirm/infirm this. in vsmmuv3 code I notif

[Qemu-devel] [PATCH 1/2] spapr/rtas: disable the decrementer interrupt when a CPU is unplugged

[Cédric Le Goater]

When a CPU is stopped with the 'stop-self' RTAS call, its state
'halted' is switched to 1 and, in this case, the MSR is not taken into
account anymore in the cpu_has_work() routine. Only the pending
hardware interrupts are checked with their LPCR:PECE* enablement bit.

If the DECR timer fires after 'stop-self' is called and before the CPU
'stop' state is reached, the nearly-dead CPU will have some work to do
and the guest will crash. This case happens very frequently with the
not yet upstream P9 XIVE exploitation mode. In XICS mode, the DECR is
occasionally fired but after 'stop' state, so no work is to be done
and the guest survives.

I suspect there is a race between the QEMU mainloop triggering the
timers and the TCG CPU thread but I could not quite identify the root
cause. To be safe, let's disable the decrementer interrupt in the LPCR
when the CPU is halted and reenable it when the CPU is restarted.

Signed-off-by: Cédric Le Goater 
---
 hw/ppc/spapr_rtas.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index cdf0b607a0a0..2389220c9738 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -174,6 +174,15 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, 
sPAPRMachineState *spapr,
 kvm_cpu_synchronize_state(cs);
 
 env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
+
+/* Enable DECR interrupt */
+if (env->mmu_model == POWERPC_MMU_3_00) {
+env->spr[SPR_LPCR] |= LPCR_DEE;
+} else {
+/* P7 and P8 both have same bit for DECR */
+env->spr[SPR_LPCR] |= LPCR_P8_PECE3;
+}
+
 env->nip = start;
 env->gpr[3] = r3;
 cs->halted = 0;
@@ -210,6 +219,13 @@ static void rtas_stop_self(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
  * no need to bother with specific bits, we just clear it.
  */
 env->msr = 0;
+
+if (env->mmu_model == POWERPC_MMU_3_00) {
+env->spr[SPR_LPCR] &= ~LPCR_DEE;
+} else {
+/* P7 and P8 both have same bit for DECR */
+env->spr[SPR_LPCR] &= ~LPCR_P8_PECE3;
+}
 }
 
 static inline int sysparm_st(target_ulong addr, target_ulong len,
-- 
2.13.6

Re: [Qemu-devel] [PULL v1 00/10] Merge tpm 2017/10/04

[Peter Maydell]

On 5 October 2017 at 17:54, Stefan Berger  wrote:
> On 10/05/2017 12:53 PM, Peter Maydell wrote:
>> Hi; this pull request appears to be signed with a gpg key that
>> isn't signed by anybody else... Are there other people you work
>> with at IBM who can verify your id and sign your key for you?
>
>
> Does the other person need to be part of the QEMU community?

Not necessarily, if they're part of the wider 'web of
trust' in gpg (eg other open source people are often
indirectly connected to QEMU people).

thanks
-- PMM

[Qemu-devel] [PULL v1 10/10] specs: Describe the TPM support in QEMU

[Stefan Berger]

This patch adds a description of the current TPM support in QEMU
to the specs.

Several public specs are referenced via their landing page on the
trustedcomputinggroup.org website.

Signed-off-by: Stefan Berger 
Reviewed-by: Laszlo Ersek 
---
 docs/specs/tpm.txt | 123 +
 1 file changed, 123 insertions(+)
 create mode 100644 docs/specs/tpm.txt

diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
new file mode 100644
index 000..914daac
--- /dev/null
+++ b/docs/specs/tpm.txt
@@ -0,0 +1,123 @@
+QEMU TPM Device
+===
+
+= Guest-side Hardware Interface =
+
+The QEMU TPM emulation implements a TPM TIS hardware interface following the
+Trusted Computing Group's specification "TCG PC Client Specific TPM Interface
+Specification (TIS)", Specification Version 1.3, 21 March 2013. This
+specification, or a later version of it, can be accessed from the following
+URL:
+
+https://trustedcomputinggroup.org/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/
+
+The TIS interface makes a memory mapped IO region in the area 0xfed4 -
+0xfed44fff available to the guest operating system.
+
+
+QEMU files related to TPM TIS interface:
+ - hw/tpm/tpm_tis.c
+ - hw/tpm/tpm_tis.h
+
+
+= ACPI Interface =
+
+The TPM device is defined with ACPI ID "PNP0C31". QEMU builds a SSDT and passes
+it into the guest through the fw_cfg device. The device description contains
+the base address of the TIS interface 0xfed4 and the size of the MMIO area
+(0x5000). In case a TPM2 is used by QEMU, a TPM2 ACPI table is also provided.
+The device is described to be used in polling mode rather than interrupt mode
+primarily because no unused IRQ could be found.
+
+To support measurement logs to be written by the firmware, e.g. SeaBIOS, a TCPA
+table is implemented. This table provides a 64kb buffer where the firmware can
+write its log into. For TPM 2 only a more recent version of the TPM2 table
+provides support for measurements logs and a TCPA table does not need to be
+created.
+
+The TCPA and TPM2 ACPI tables follow the Trusted Computing Group specification
+"TCG ACPI Specification" Family "1.2" and "2.0", Level 00 Revision 00.37. This
+specification, or a later version of it, can be accessed from the following
+URL:
+
+https://trustedcomputinggroup.org/tcg-acpi-specification/
+
+
+QEMU files related to TPM ACPI tables:
+ - hw/i386/acpi-build.c
+ - include/hw/acpi/tpm.h
+
+
+= TPM backend devices =
+
+The TPM implementation is split into two parts, frontend and backend. The
+frontend part is the hardware interface, such as the TPM TIS interface
+described earlier, and the other part is the TPM backend interface. The backend
+interfaces implement the interaction with a TPM device, which may be a physical
+or an emulated device. The split between the front- and backend devices allows
+a frontend to be connected with any available backend. This enables the TIS
+interface to be used with the passthrough backend or the (future) swtpm 
backend.
+
+
+QEMU files related to TPM backends:
+ - backends/tpm.c
+ - include/sysemu/tpm_backend.h
+ - include/sysemu/tpm_backend_int.h
+
+
+== The QEMU TPM passthrough device ==
+
+In case QEMU is run on Linux as the host operating system it is possible to
+make the hardware TPM device available to a single QEMU guest. In this case the
+user must make sure that no other program is using the device, e.g., /dev/tpm0,
+before trying to start QEMU with it.
+
+The passthrough driver uses the host's TPM device for sending TPM commands
+and receiving responses from. Besides that it accesses the TPM device's sysfs
+entry for support of command cancellation. Since none of the state of a
+hardware TPM can be migrated between hosts, virtual machine migration is
+disabled when the TPM passthrough driver is used.
+
+Since the host's TPM device will already be initialized by the host's firmware,
+certain commands, e.g. TPM_Startup(), sent by the virtual firmware for device
+initialization, will fail. In this case the firmware should not use the TPM.
+
+Sharing the device with the host is generally not a recommended usage scenario
+for a TPM device. The primary reason for this is that two operating systems can
+then access the device's single set of resources, such as platform 
configuration
+registers (PCRs). Applications or kernel security subsystems, such as the
+Linux Integrity Measurement Architecture (IMA), are not expecting to share 
PCRs.
+
+
+QEMU files related to the TPM passthrough device:
+ - hw/tpm/tpm_passthrough.c
+ - hw/tpm/tpm_util.c
+ - hw/tpm/tpm_util.h
+
+
+Command line to start QEMU with the TPM passthrough device using the host's
+hardware TPM /dev/tpm0:
+
+qemu-system-x86_64 -display sdl -enable-kvm \
+  -m 1024 -boot d -bios bios-256k.bin -boot menu=on \
+  -tpmdev passthrough,id=tpm0,path=/dev/tpm0 \
+  -device tpm-tis,tpmdev=tpm0 test.img
+
+The following commands should result in similar output inside the VM with a

Re: [Qemu-devel] [PULL v1 00/10] Merge tpm 2017/10/04

[Stefan Berger]

On 10/05/2017 12:53 PM, Peter Maydell wrote:

On 5 October 2017 at 17:48, Stefan Berger  wrote:

The following changes since commit d147f7e815f97cb477e223586bcb80c316ae10ea:

   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
staging (2017-10-03 16:27:24 +0100)

are available in the git repository at:

   git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2017-10-04-1

for you to fetch changes up to 5e64d0182fe9d1c9caa347c5bee0a0591f9be2ef:

   specs: Describe the TPM support in QEMU (2017-10-05 12:39:15 -0400)


Merge tpm 2017/10/04 v1


Hi; this pull request appears to be signed with a gpg key that
isn't signed by anybody else... Are there other people you work
with at IBM who can verify your id and sign your key for you?


Does the other person need to be part of the QEMU community?

   Stefan



thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command

[Jan Dakinevich]

On 10/04/2017 07:00 PM, Eric Blake wrote:
> On 10/04/2017 09:26 AM, Jan Dakinevich wrote:
> 
>> +{
>> +'struct': 'VirtioInfo',
>> +'data': {
>> +'feature-names': ['VirtioInfoBit'],
>
> Why is feature-names listed at two different nestings of the return value?
>

 These are different feature names. First names are common and predefined
 for all devices. Second names are device-specific.
>>>
>>> If you can turn these into enums (union'd enums?) then you might
>>> be able to get rid of a lot of your array filling/naming conversion
>>> boilerplate. (Not sure if it's worth it, but it's worth looking).
>>>
>>
>> I would be happy to drop this boilerplate, but how enum could help here?
>> To respond my requirement it should be something like set, not enum.
>> Even so, having set, I would have been needed to declare mapping between
>> names in set type and bit numbers within feature bitmask.
> 
> Instead of returning a bitmask ("mask":123) as well as an array naming
> those bits
> ([{"bit":1,"name":"bit1"},{"bit":2","name":"bit2"},{"bit":4,"name":"bit4},...]),
> you could omit the bit numbers and just return an array of named bits
> (["bit1", "bit2", "bit4"]).  An enum lets you declare up front what
> named bits are supported (and code can introspect when new named bits
> are supported in newer qemu).
>
But how can I declare in this notation that name "bit1" is owned by bit
#1, name "bit2" is owned by bit #2, name "bit4" is owned by bit #4, and
all other bits has no names.

> Perhaps it's easier to first take a step back, and show what the desired
> output might be like, and then we can figure out how to represent that
> output in QAPI.
> 

Yeah... I expect the following HMP output:

(qmue) info virtio
virtio-blk-device at :00:07.0
  status:   0x07 acknowledge,driver,driver_ok
  host features:0x000179000e54
  guest features:   0x3e54
  common features:
   notify_on_empty
any_layout
 indirect_desc   acked
 event_idx   acked
 version_1
  specific features:
   seg_max   acked
  blk_size   acked
 flush   acked
  topology   acked
virtio-serial-device at :00:08.0
  status:   0x07 acknowledge,driver,driver_ok
  host features:0x00017906
  guest features:   0x3002
  common features:
   notify_on_empty
any_layout
 indirect_desc   acked
 event_idx   acked
 version_1
  specific features:
 multiport   acked
   emerg_write

-- 
Best regards
Jan Dakinevich