Re: [PATCH v5 07/37] serial: register vmsd with DeviceClass

[xiaoqiang zhao]

在 2019/12/20 下午9:45, Marc-André Lureau 写道:

Migration from old to new code works, however the other way fails for
devices that use serial_init/serial_mm_init with "base", used as
instance_id previously.

(with qdev_set_legacy_instance_id, the alias_id is only used in
savevm.c:find_se(), and thus can only be used to match against
"legacy" instance id values. On new code, instance_id is generated
incrementally from 0 with calculate_new_instance_id(), based on
"qdev-path/vmsd-name")

Signed-off-by: Marc-André Lureau 
---
  hw/char/serial.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 233a9e2076..e926845843 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -990,8 +990,7 @@ SerialState *serial_init(int base, qemu_irq irq, int 
baudbase,
  s->baudbase = baudbase;
  qemu_chr_fe_init(&s->chr, chr, &error_abort);
  serial_realize_core(s, &error_fatal);
-
-vmstate_register(NULL, base, &vmstate_serial, s);
+qdev_set_legacy_instance_id(dev, base, 2);
  qdev_init_nofail(dev);
  
  memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);

@@ -1006,6 +1005,7 @@ static void serial_class_init(ObjectClass *klass, void 
*data)
  
  /* internal device for serialio/serialmm, not user-creatable */

  dc->user_creatable = false;
+dc->vmsd = &vmstate_serial;
  }
  
  static const TypeInfo serial_info = {

@@ -1069,7 +1069,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
  qemu_chr_fe_init(&s->chr, chr, &error_abort);
  
  serial_realize_core(s, &error_fatal);

-vmstate_register(NULL, base, &vmstate_serial, s);
+qdev_set_legacy_instance_id(dev, base, 2);
  qdev_init_nofail(dev);
  
  memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,

Resend, cc peter
Reviewed-by: xiaoqiang zhao 

Re: [PATCH v5 04/37] chardev: generate an internal id when none given

[xiaoqiang zhao]

在 2019/12/20 下午9:45, Marc-André Lureau 写道:

Internally, qemu may create chardev without ID. Those will not be
looked up with qemu_chr_find(), which prevents using qdev_prop_set_chr().

Use id_generate(), to generate an internal name (prefixed with #), so
no conflict exist with user-named chardev.

Signed-off-by: Marc-André Lureau 
---
  chardev/char.c| 32 
  include/qemu/id.h |  1 +
  util/id.c |  1 +
  3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 7b6b2cb123..e7e7492b0e 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -37,6 +37,7 @@
  #include "qemu/help_option.h"
  #include "qemu/module.h"
  #include "qemu/option.h"
+#include "qemu/id.h"
  
  #include "chardev/char-mux.h"
  
@@ -944,10 +945,10 @@ void qemu_chr_set_feature(Chardev *chr,

  return set_bit(feature, chr->features);
  }
  
-Chardev *qemu_chardev_new(const char *id, const char *typename,

-  ChardevBackend *backend,
-  GMainContext *gcontext,
-  Error **errp)
+static Chardev *chardev_new(const char *id, const char *typename,
+ChardevBackend *backend,
+GMainContext *gcontext,
+Error **errp)
  {
  Object *obj;
  Chardev *chr = NULL;
@@ -991,6 +992,21 @@ end:
  return chr;
  }
  
+Chardev *qemu_chardev_new(const char *id, const char *typename,

+  ChardevBackend *backend,
+  GMainContext *gcontext,
+  Error **errp)
+{
+g_autofree char *genid = NULL;
+
+if (!id) {
+genid = id_generate(ID_CHR);
+id = genid;
+}
+
+return chardev_new(id, typename, backend, gcontext, errp);
+}
+
  ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
 Error **errp)
  {
@@ -1003,8 +1019,8 @@ ChardevReturn *qmp_chardev_add(const char *id, 
ChardevBackend *backend,
  return NULL;
  }
  
-chr = qemu_chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),

-   backend, NULL, errp);
+chr = chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
+  backend, NULL, errp);
  if (!chr) {
  return NULL;
  }
@@ -1061,8 +1077,8 @@ ChardevReturn *qmp_chardev_change(const char *id, 
ChardevBackend *backend,
  return NULL;
  }
  
-chr_new = qemu_chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),

-   backend, chr->gcontext, errp);
+chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
+  backend, chr->gcontext, errp);
  if (!chr_new) {
  return NULL;
  }
diff --git a/include/qemu/id.h b/include/qemu/id.h
index 40c70103e4..b55c406e69 100644
--- a/include/qemu/id.h
+++ b/include/qemu/id.h
@@ -4,6 +4,7 @@
  typedef enum IdSubSystems {
  ID_QDEV,
  ID_BLOCK,
+ID_CHR,
  ID_MAX  /* last element, used as array size */
  } IdSubSystems;
  
diff --git a/util/id.c b/util/id.c

index af1c5f1b81..5addb4460e 100644
--- a/util/id.c
+++ b/util/id.c
@@ -34,6 +34,7 @@ bool id_wellformed(const char *id)
  static const char *const id_subsys_str[ID_MAX] = {
  [ID_QDEV]  = "qdev",
  [ID_BLOCK] = "block",
+[ID_CHR] = "chr",
  };
  
  /*


Looks good to me.

Reviewed-by: xiaoqiang zhao 

[PATCH] iotests: fix usage -machine accel= together with -accel option

[Vladimir Sementsov-Ogievskiy]

Commit 6f6e1698a68ceb made these options incompatible, but it breaks
iotests:
  -accel qtest comes from QEMUQtestMachine
  and
  -machine accel=qtest comes from QEMU_OPTIONS

After this patch, -accel may still be duplicated, but this is less
invasive.

Also, fix extra comma in comment, added by the same 6f6e1698a68ceb

Fixes: 6f6e1698a68ceb
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 vl.c | 2 +-
 tests/qemu-iotests/check | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 86474a55c9..9fb859969c 100644
--- a/vl.c
+++ b/vl.c
@@ -2779,7 +2779,7 @@ static void configure_accelerators(const char *progname)
 for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
 /*
  * Filter invalid accelerators here, to prevent obscenities
- * such as "-machine accel=tcg,,thread=single".
+ * such as "-machine accel=tcg,thread=single".
  */
 if (accel_find(*tmp)) {
 qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 90970b0549..2890785a10 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -587,13 +587,13 @@ export QEMU_PROG="$(type -p "$QEMU_PROG")"
 
 case "$QEMU_PROG" in
 *qemu-system-arm|*qemu-system-aarch64)
-export QEMU_OPTIONS="-nodefaults -display none -machine 
virt,accel=qtest"
+export QEMU_OPTIONS="-nodefaults -display none -machine virt -accel 
qtest"
 ;;
 *qemu-system-tricore)
-export QEMU_OPTIONS="-nodefaults -display none -machine 
tricore_testboard,accel=qtest"
+export QEMU_OPTIONS="-nodefaults -display none -machine 
tricore_testboard -accel qtest"
 ;;
 *)
-export QEMU_OPTIONS="-nodefaults -display none -machine accel=qtest"
+export QEMU_OPTIONS="-nodefaults -display none -accel qtest"
 ;;
 esac
 
-- 
2.21.0

Re: [PATCH v5 07/37] serial: register vmsd with DeviceClass

[xiaoqiang zhao]

在 2019/12/20 下午9:45, Marc-André Lureau 写道:

Migration from old to new code works, however the other way fails for
devices that use serial_init/serial_mm_init with "base", used as
instance_id previously.

(with qdev_set_legacy_instance_id, the alias_id is only used in
savevm.c:find_se(), and thus can only be used to match against
"legacy" instance id values. On new code, instance_id is generated
incrementally from 0 with calculate_new_instance_id(), based on
"qdev-path/vmsd-name")

Signed-off-by: Marc-André Lureau 
---
  hw/char/serial.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 233a9e2076..e926845843 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -990,8 +990,7 @@ SerialState *serial_init(int base, qemu_irq irq, int 
baudbase,
  s->baudbase = baudbase;
  qemu_chr_fe_init(&s->chr, chr, &error_abort);
  serial_realize_core(s, &error_fatal);
-
-vmstate_register(NULL, base, &vmstate_serial, s);
+qdev_set_legacy_instance_id(dev, base, 2);
  qdev_init_nofail(dev);
  
  memory_region_init_io(&s->io, NULL, &serial_io_ops, s, "serial", 8);

@@ -1006,6 +1005,7 @@ static void serial_class_init(ObjectClass *klass, void 
*data)
  
  /* internal device for serialio/serialmm, not user-creatable */

  dc->user_creatable = false;
+dc->vmsd = &vmstate_serial;
  }
  
  static const TypeInfo serial_info = {

@@ -1069,7 +1069,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
  qemu_chr_fe_init(&s->chr, chr, &error_abort);
  
  serial_realize_core(s, &error_fatal);

-vmstate_register(NULL, base, &vmstate_serial, s);
+qdev_set_legacy_instance_id(dev, base, 2);
  qdev_init_nofail(dev);
  
  memory_region_init_io(&s->io, NULL, &serial_mm_ops[end], s,

Reviewed-by: xiaoqiang zhao 

[PATCH v1] hw: fix using 4.2 compat in 5.0 machine types for i440fx/q35

[Denis Plotnikov]

5.0 machine type uses 4.2 compats. This seems to be incorrect, since
the latests machine type by now is 5.0 and it should use its own
compat or shouldn't use any relying on the defaults.
Seems, like this appeared because of some problems on merge/rebase.

Signed-off-by: Denis Plotnikov 
---
 hw/i386/pc_piix.c | 1 -
 hw/i386/pc_q35.c  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ffb30c32ce..846e70bc55 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -431,7 +431,6 @@ static void pc_i440fx_5_0_machine_options(MachineClass *m)
 m->alias = "pc";
 m->is_default = 1;
 pcmc->default_cpu_version = 1;
-compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
 }
 
 DEFINE_I440FX_MACHINE(v5_0, "pc-i440fx-5.0", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 7398d7baa2..ddd485d608 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -354,7 +354,6 @@ static void pc_q35_5_0_machine_options(MachineClass *m)
 pc_q35_machine_options(m);
 m->alias = "q35";
 pcmc->default_cpu_version = 1;
-compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
 }
 
 DEFINE_Q35_MACHINE(v5_0, "pc-q35-5.0", NULL,
-- 
2.17.0

Re: [PATCH] target/ppc: fix memory dump endianness in QEMU monitor

[David Gibson]

On Thu, Dec 19, 2019 at 01:38:54PM -0300, Maxiwell S. Garcia wrote:
> The env->hflags is computed in ppc_cpu_reset(), using the MSR register
> as input. But at the point ppc_disas_set_info() is called the MSR_LE bit
> in env->hflags doesn't contain the same information that env->msr.
> 
> Signed-off-by: Maxiwell S. Garcia 
> Signed-off-by: Fabiano Rosas 

I think the change is ok as far as it goes but,

a) the commit message should expand on what the practical effect of
this is.  Looking, I think the only thing this affects is DEBUG_DISAS
output (i.e. very rarely) which is worth noting.

b) AFAICT this is the *only* thing that looks for the LE bit in
hflags. Given that, and the fact that it would be wrong in most cases,
we should remove it from hflags entirely along with this change.

> ---
>  target/ppc/translate_init.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index d33d65dff7..a0b384da9e 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10830,7 +10830,7 @@ static void ppc_disas_set_info(CPUState *cs, 
> disassemble_info *info)
>  PowerPCCPU *cpu = POWERPC_CPU(cs);
>  CPUPPCState *env = &cpu->env;
>  
> -if ((env->hflags >> MSR_LE) & 1) {
> +if (msr_le) {
>  info->endian = BFD_ENDIAN_LITTLE;
>  }
>  info->mach = env->bfd_mach;

-- 
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: [PATCH v2 00/13] ppc/pnv: remove the use of qdev_get_machine() and get_system_memory()

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:42PM +0100, Cédric Le Goater wrote:
> Hello,
> 
> The PowerNV and sPAPR machine use qdev_get_machine() and
> get_system_memory() in some places. This is not a good modeling
> pratice and it should be avoided. This series replaces the uses of
> these routines with a set of QOM properties and aliases.

Remaining patches LGTM, but will need a rebase to account for the
changes suggested in some of the earlier ones.

-- 
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: [PATCH v2 08/13] xive: Use the XIVE fabric link under the XIVE router

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:50PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz 
> 
> Now that the spapr and pnv machines do set the "xive-fabric" link, the
> use of the XIVE fabric pointer becomes mandatory. This is checked with
> an assert() in a new realize hook. Since the XIVE router is realized at
> machine init for the all the machine's life time, no risk to abort an
> already running guest (ie. not a hotplug path).
> 
> This gets rid of a qdev_get_machine() call.
> 
> Signed-off-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

LGTM, but will need rebase.

> ---
>  hw/intc/xive.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 6df89b06da38..12a362b681a6 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1378,6 +1378,13 @@ static int xive_router_get_block_id(XiveRouter *xrtr)
> return xrc->get_block_id(xrtr);
>  }
>  
> +static void xive_router_realize(DeviceState *dev, Error **errp)
> +{
> +XiveRouter *xrtr = XIVE_ROUTER(dev);
> +
> +assert(xrtr->xfb);
> +}
> +
>  /*
>   * Encode the HW CAM line in the block group mode format :
>   *
> @@ -1470,12 +1477,11 @@ int xive_presenter_tctx_match(XivePresenter *xptr, 
> XiveTCTX *tctx,
>   *
>   * The parameters represent what is sent on the PowerBus
>   */
> -static bool xive_presenter_notify(uint8_t format,
> +static bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>uint8_t nvt_blk, uint32_t nvt_idx,
>bool cam_ignore, uint8_t priority,
>uint32_t logic_serv)
>  {
> -XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine());
>  XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb);
>  XiveTCTXMatch match = { .tctx = NULL, .ring = 0 };
>  int count;
> @@ -1607,7 +1613,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, 
> uint8_t end_blk,
>  return;
>  }
>  
> -found = xive_presenter_notify(format, nvt_blk, nvt_idx,
> +found = xive_presenter_notify(xrtr->xfb, format, nvt_blk, nvt_idx,
>xive_get_field32(END_W7_F0_IGNORE, end.w7),
>priority,
>xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
> @@ -1727,6 +1733,8 @@ static void xive_router_class_init(ObjectClass *klass, 
> void *data)
>  
>  dc->desc= "XIVE Router Engine";
>  dc->props   = xive_router_properties;
> +/* Parent is SysBusDeviceClass. No need to call its realize hook */
> +dc->realize = xive_router_realize;
>  xnc->notify = xive_router_notify;
>  }
>  

-- 
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: [PATCH v2 07/13] spapr, pnv, xive: Add a "xive-fabric" link to the XIVE router

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:49PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz 
> 
> In order to get rid of qdev_get_machine(), first add a pointer to the
> XIVE fabric under the XIVE router and make it configurable through a
> QOM link property.
> 
> Configure it in the spapr and pnv machine. In the case of pnv, the XIVE
> routers are under the chip, so this is done with a QOM alias property of
> the POWER9 pnv chip.
> 
> Signed-off-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

LGTM, but will need rebase.

> ---
>  include/hw/ppc/xive.h | 5 +++--
>  hw/intc/xive.c| 8 
>  hw/ppc/pnv.c  | 6 ++
>  hw/ppc/spapr_irq.c| 2 ++
>  4 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 1b7b89098f71..1ded82b1cda8 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -324,9 +324,12 @@ typedef struct XiveTCTX {
>  /*
>   * XIVE Router
>   */
> +typedef struct XiveFabric XiveFabric;
>  
>  typedef struct XiveRouter {
>  SysBusDeviceparent;
> +
> +XiveFabric *xfb;
>  } XiveRouter;
>  
>  #define TYPE_XIVE_ROUTER "xive-router"
> @@ -402,8 +405,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, 
> XiveTCTX *tctx,
>   * XIVE Fabric (Interface between Interrupt Controller and Machine)
>   */
>  
> -typedef struct XiveFabric XiveFabric;
> -
>  #define TYPE_XIVE_FABRIC "xive-fabric"
>  #define XIVE_FABRIC(obj) \
>  INTERFACE_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index d4c6e21703b3..6df89b06da38 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1714,12 +1714,19 @@ void xive_router_notify(XiveNotifier *xn, uint32_t 
> lisn)
> xive_get_field64(EAS_END_DATA,  eas.w));
>  }
>  
> +static Property xive_router_properties[] = {
> +DEFINE_PROP_LINK("xive-fabric", XiveRouter, xfb,
> + TYPE_XIVE_FABRIC, XiveFabric *),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void xive_router_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
>  
>  dc->desc= "XIVE Router Engine";
> +dc->props   = xive_router_properties;
>  xnc->notify = xive_router_notify;
>  }
>  
> @@ -1727,6 +1734,7 @@ static const TypeInfo xive_router_info = {
>  .name  = TYPE_XIVE_ROUTER,
>  .parent= TYPE_SYS_BUS_DEVICE,
>  .abstract  = true,
> +.instance_size = sizeof(XiveRouter),
>  .class_size= sizeof(XiveRouterClass),
>  .class_init= xive_router_class_init,
>  .interfaces= (InterfaceInfo[]) {
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 2a1b15a69aed..915c80a24b3e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -804,6 +804,10 @@ static void pnv_init(MachineState *machine)
>  if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
>  object_property_set_link(chip, OBJECT(pnv), "xics", 
> &error_abort);
>  }
> +if (object_dynamic_cast(OBJECT(pnv), TYPE_XIVE_FABRIC)) {
> +object_property_set_link(chip, OBJECT(pnv), "xive-fabric",
> + &error_abort);
> +}
>  object_property_set_bool(chip, true, "realized", &error_fatal);
>  }
>  g_free(chip_typename);
> @@ -1224,6 +1228,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
>  
>  object_initialize_child(obj, "xive", &chip9->xive, sizeof(chip9->xive),
>  TYPE_PNV_XIVE, &error_abort, NULL);
> +object_property_add_alias(obj, "xive-fabric", OBJECT(&chip9->xive),
> +  "xive-fabric", &error_abort);
>  
>  object_initialize_child(obj, "psi",  &chip9->psi, sizeof(chip9->psi),
>  TYPE_PNV9_PSI, &error_abort, NULL);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 07e08d6544a0..2b656649ad6a 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -340,6 +340,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error 
> **errp)
>   * priority
>   */
>  qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
> +object_property_set_link(OBJECT(dev), OBJECT(spapr), "xive-fabric",
> + &error_abort);
>  qdev_init_nofail(dev);
>  
>  spapr->xive = SPAPR_XIVE(dev);

-- 
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: [PATCH v2 09/13] ppc/pnv: Add an "nr-threads" property to the base chip class

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:51PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz 
> 
> Set it at chip creation and forward it to the cores. This allows to drop
> a call to qdev_get_machine().
> 
> Signed-off-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

But will need rebase.

> ---
>  include/hw/ppc/pnv.h | 1 +
>  hw/ppc/pnv.c | 8 +---
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 8b957dfb5736..4c13d4394a11 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -48,6 +48,7 @@ typedef struct PnvChip {
>  uint64_t ram_size;
>  
>  uint32_t nr_cores;
> +uint32_t nr_threads;
>  uint64_t cores_mask;
>  PnvCore  **cores;
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 915c80a24b3e..e638cdc93091 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -791,6 +791,8 @@ static void pnv_init(MachineState *machine)
>  &error_fatal);
>  object_property_set_int(chip, machine->smp.cores,
>  "nr-cores", &error_fatal);
> +object_property_set_int(chip, machine->smp.threads,
> +"nr-threads", &error_fatal);
>  /*
>   * TODO: Only the MMIO range should be of interest for the
>   * controllers
> @@ -1529,7 +1531,6 @@ static void pnv_chip_core_sanitize(PnvChip *chip, Error 
> **errp)
>  
>  static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>  {
> -MachineState *ms = MACHINE(qdev_get_machine());
>  Error *error = NULL;
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>  const char *typename = pnv_chip_core_typename(chip);
> @@ -1565,8 +1566,8 @@ static void pnv_chip_core_realize(PnvChip *chip, Error 
> **errp)
>  object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core),
>&error_abort);
>  chip->cores[i] = pnv_core;
> -object_property_set_int(OBJECT(pnv_core), ms->smp.threads, 
> "nr-threads",
> -&error_fatal);
> +object_property_set_int(OBJECT(pnv_core), chip->nr_threads,
> +"nr-threads", &error_fatal);
>  object_property_set_int(OBJECT(pnv_core), core_hwid,
>  CPU_CORE_PROP_CORE_ID, &error_fatal);
>  object_property_set_int(OBJECT(pnv_core),
> @@ -1609,6 +1610,7 @@ static Property pnv_chip_properties[] = {
>  DEFINE_PROP_UINT64("cores-mask", PnvChip, cores_mask, 0x0),
>  DEFINE_PROP_LINK("system-memory", PnvChip, system_memory,
>   TYPE_MEMORY_REGION, MemoryRegion *),
> +DEFINE_PROP_UINT32("nr-threads", PnvChip, nr_threads, 1),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  

-- 
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: [PATCH v4 2/4] nvdimm: add uuid property to nvdimm

[David Gibson]

On Tue, Dec 17, 2019 at 02:48:49AM -0600, Shivaprasad G Bhat wrote:
> For ppc64, PAPR requires the nvdimm device to have UUID property
> set in the device tree. Add an option to get it from the user.
> 
> Signed-off-by: Shivaprasad G Bhat 

Reviewed-by: David Gibson 

> ---
>  hw/mem/nvdimm.c |   40 
>  include/hw/mem/nvdimm.h |7 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 375f9a588a..e1238b5bed 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -69,11 +69,51 @@ out:
>  error_propagate(errp, local_err);
>  }
>  
> +static void nvdimm_get_uuid(Object *obj, Visitor *v, const char *name,
> +  void *opaque, Error **errp)
> +{
> +NVDIMMDevice *nvdimm = NVDIMM(obj);
> +char *value = NULL;
> +
> +value = qemu_uuid_unparse_strdup(&nvdimm->uuid);
> +
> +visit_type_str(v, name, &value, errp);
> +g_free(value);
> +}
> +
> +
> +static void nvdimm_set_uuid(Object *obj, Visitor *v, const char *name,
> +  void *opaque, Error **errp)
> +{
> +NVDIMMDevice *nvdimm = NVDIMM(obj);
> +Error *local_err = NULL;
> +char *value;
> +
> +visit_type_str(v, name, &value, &local_err);
> +if (local_err) {
> +goto out;
> +}
> +
> +if (qemu_uuid_parse(value, &nvdimm->uuid) != 0) {
> +error_setg(errp, "Property '%s.%s' has invalid value",
> +   object_get_typename(obj), name);
> +goto out;
> +}
> +g_free(value);
> +
> +out:
> +error_propagate(errp, local_err);
> +}
> +
> +
>  static void nvdimm_init(Object *obj)
>  {
>  object_property_add(obj, NVDIMM_LABEL_SIZE_PROP, "int",
>  nvdimm_get_label_size, nvdimm_set_label_size, NULL,
>  NULL, NULL);
> +
> +object_property_add(obj, NVDIMM_UUID_PROP, "QemuUUID", nvdimm_get_uuid,
> +nvdimm_set_uuid, NULL, NULL, NULL);
>  }
>  
>  static void nvdimm_finalize(Object *obj)
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 523a9b3d4a..4807ca615b 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -25,6 +25,7 @@
>  
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/acpi/bios-linker-loader.h"
> +#include "qemu/uuid.h"
>  
>  #define NVDIMM_DEBUG 0
>  #define nvdimm_debug(fmt, ...)\
> @@ -49,6 +50,7 @@
> TYPE_NVDIMM)
>  
>  #define NVDIMM_LABEL_SIZE_PROP "label-size"
> +#define NVDIMM_UUID_PROP   "uuid"
>  #define NVDIMM_UNARMED_PROP"unarmed"
>  
>  struct NVDIMMDevice {
> @@ -83,6 +85,11 @@ struct NVDIMMDevice {
>   * the guest write persistence.
>   */
>  bool unarmed;
> +
> +/*
> + * The PPC64 - spapr requires each nvdimm device have a uuid.
> + */
> +QemuUUID uuid;
>  };
>  typedef struct NVDIMMDevice NVDIMMDevice;
>  
> 

-- 
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: [PATCH v4 1/4] mem: move nvdimm_device_list to utilities

[David Gibson]

On Tue, Dec 17, 2019 at 02:48:20AM -0600, Shivaprasad G Bhat wrote:
> nvdimm_device_list is required for parsing the list for devices
> in subsequent patches. Move it to common utility area.
> 
> Signed-off-by: Shivaprasad G Bhat 
> Reviewed-by: Igor Mammedov 

Reviewed-by: David Gibson 

> ---
>  hw/acpi/nvdimm.c|   28 +---
>  include/qemu/nvdimm-utils.h |7 +++
>  util/Makefile.objs  |1 +
>  util/nvdimm-utils.c |   29 +
>  4 files changed, 38 insertions(+), 27 deletions(-)
>  create mode 100644 include/qemu/nvdimm-utils.h
>  create mode 100644 util/nvdimm-utils.c
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 9fdad6dc3f..5219dd0e2e 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -32,33 +32,7 @@
>  #include "hw/acpi/bios-linker-loader.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/mem/nvdimm.h"
> -
> -static int nvdimm_device_list(Object *obj, void *opaque)
> -{
> -GSList **list = opaque;
> -
> -if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
> -*list = g_slist_append(*list, DEVICE(obj));
> -}
> -
> -object_child_foreach(obj, nvdimm_device_list, opaque);
> -return 0;
> -}
> -
> -/*
> - * inquire NVDIMM devices and link them into the list which is
> - * returned to the caller.
> - *
> - * Note: it is the caller's responsibility to free the list to avoid
> - * memory leak.
> - */
> -static GSList *nvdimm_get_device_list(void)
> -{
> -GSList *list = NULL;
> -
> -object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
> -return list;
> -}
> +#include "qemu/nvdimm-utils.h"
>  
>  #define NVDIMM_UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
> { (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
> diff --git a/include/qemu/nvdimm-utils.h b/include/qemu/nvdimm-utils.h
> new file mode 100644
> index 00..4b8b198ba7
> --- /dev/null
> +++ b/include/qemu/nvdimm-utils.h
> @@ -0,0 +1,7 @@
> +#ifndef NVDIMM_UTILS_H
> +#define NVDIMM_UTILS_H
> +
> +#include "qemu/osdep.h"
> +
> +GSList *nvdimm_get_device_list(void);
> +#endif
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index df124af1c5..2a096fe190 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -20,6 +20,7 @@ util-obj-y += envlist.o path.o module.o
>  util-obj-y += host-utils.o
>  util-obj-y += bitmap.o bitops.o hbitmap.o
>  util-obj-y += fifo8.o
> +util-obj-y += nvdimm-utils.o
>  util-obj-y += cacheinfo.o
>  util-obj-y += error.o qemu-error.o
>  util-obj-y += qemu-print.o
> diff --git a/util/nvdimm-utils.c b/util/nvdimm-utils.c
> new file mode 100644
> index 00..5cc768ca47
> --- /dev/null
> +++ b/util/nvdimm-utils.c
> @@ -0,0 +1,29 @@
> +#include "qemu/nvdimm-utils.h"
> +#include "hw/mem/nvdimm.h"
> +
> +static int nvdimm_device_list(Object *obj, void *opaque)
> +{
> +GSList **list = opaque;
> +
> +if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
> +*list = g_slist_append(*list, DEVICE(obj));
> +}
> +
> +object_child_foreach(obj, nvdimm_device_list, opaque);
> +return 0;
> +}
> +
> +/*
> + * inquire NVDIMM devices and link them into the list which is
> + * returned to the caller.
> + *
> + * Note: it is the caller's responsibility to free the list to avoid
> + * memory leak.
> + */
> +GSList *nvdimm_get_device_list(void)
> +{
> +GSList *list = NULL;
> +
> +object_child_foreach(qdev_get_machine(), nvdimm_device_list, &list);
> +return list;
> +}
> 

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

[Bug 1575561] Re: config qemu virtio_queue_size to 1024，create vm boot from network failed

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  config qemu virtio_queue_size to 1024，create vm boot from network
  failed

Status in QEMU:
  Expired

Bug description:
  config qemu virtio_queue_size to 1024，create vm boot from network failed。
  in the vm vncviewer，see the error log：
  “ERROR queue size 1024 > 256
  could  not open net0: no such file or directory"

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

[Bug 1605506] Re: qemu driver_mirror error "Operation not permitted"

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  qemu driver_mirror error "Operation not permitted"

Status in QEMU:
  Expired

Bug description:
  I use libvirtd to call qemu drive_mirror return error message "Operation not 
permitted", But directly run qemu and call drive_mirror is OK;
  when drive_mirror target is logic device return error message "Operation not 
permitted",But the file is OK;

  Operating Environment：
  OS:ubuntu 14.04
  kernel:3.16.0-28-generic
  libvirt-bin version: 1.2.2-0ubuntu13.1.17
  qemu:2.5.0 or 2.6.0
  run vm user: root
  （retry in redhat7.2 have the same problem！）

  Here is my running process:
  libvirtd call
  prepare libvirt xml   libvirt.xml
  
  i-745F35DC
  65536
  1
  
  hvm
  
  destroy
  restart
  destroy
  
  
/usr/local/qemu-2.5.0-20160720/bin/qemu-system-x86_64
  
  
  
  
  
  bc-system
  
  
  
  

  virsh create libvirt.xml
  root@test:/opt/run/instance/i-745F35DC# virsh list
   IdName   State
  
   2 i-745F35DC running
  call drive_mirror:
  virsh qemu-monitor-command --hmp i-745F35DC 'drive_mirror -n -f 
drive-ide0-0-0 /dev/vg_bc_local/test raw'
  Could not open '/dev/vg_bc_local/test': Operation not permitted

  directly run qemu and call drive_mirror:
  /usr/local/qemu-2.5.0-20160720/bin/qemu-system-x86_64 -hda /tmp/image.raw -m 
64 --enable-kvm -vnc :51 -monitor stdio
  QEMU 2.5.0 monitor - type 'help' for more information
  (qemu) info block
  ide0-hd0 (#block135): /tmp/image.raw (raw)
  Cache mode:   writeback
  (qemu) drive_mirror -n -f ide0-hd0 /dev/vg_bc_local/test raw
  (qemu) info block-jobs
  Type mirror, device ide0-hd0: Completed 41126400 of 41126400 bytes, speed 
limit 0 bytes/s
  (qemu) block_job_cancel ide0-hd0
  (qemu) info block-jobs
  No active jobs

  It is OK!!!

  
  Here is my debugging process:
  Recompile qemu-2.5.0 to  enable debug
  ../configure --prefix=/usr/local/qemu-2.5.0-20160720 
--enable-trace-backend=simple --enable-werror --disable-xen --disable-virtfs 
--enable-kvm --enable-seccomp --enable-docs --enable-debug-tcg 
--enable-vnc-sasl --enable-linux-aio --enable-lzo --enable-snappy 
--enable-usb-redir --enable-vnc-png --disable-vnc-jpeg --enable-uuid 
--disable-vhost-scsi --enable-rbd 
--block-drv-rw-whitelist=qcow2,raw,file,host_device,blkdebug,nbd,iscsi,rbd,cdp 
--block-drv-ro-whitelist=vmdk,vhdx,vpc --target-list=x86_64-softmmu CFLAGS=-O0

  Use libvirtd to Re-run VM and debug by gdb
  VM process info:
  root  7804 1  0 10:45 ?00:00:10 
/usr/local/qemu-2.5.0-20160720/bin/qemu-system-x86_64 -name i-745F35DC -S 
-machine pc-i440fx-2.5,accel=kvm,usb=off -cpu 
Westmere,+invpcid,+erms,+bmi2,+smep,+avx2,+bmi1,+fsgsbase,+abm,+rdtscp,+pdpe1gb,+rdrand,+f16c,+avx,+osxsave,+xsave,+tsc-deadline,+movbe,+pcid,+pdcm,+xtpr,+fma,+tm2,+est,+vmx,+ds_cpl,+monitor,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+ds,+vme
 -m 64 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid 
ef55dfa6-b82e-488d-a7fc-4c882f8091ab -no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/i-745F35DC.monitor,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime 
-no-shutdown -boot strict=on -device 
piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive 
file=/tmp/image.raw,if=none,id=drive-ide0-0-0,format=raw,serial=bc-system,cache=none
 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=2 
-vnc 0.0.0.0:0,password -k en-us -device 
cirrus-vga,id=video0,bus=pci.0,addr=0x2 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3

  gdb -p  7804
  set breakpoint
  (gdb) i b
  Num Type   Disp Enb AddressWhat
  1   breakpoint keep y   0x7f51d9e92cb1 in qmp_drive_mirror at 
/opt/qemu/qemu-2.5.0/blockdev.c:3310
  2   breakpoint keep y   0x7f51da1252f2 in raw_open_common at 
/opt/qemu/qemu-2.5.0/block/raw-posix.c:457
  3   breakpoint keep y   0x7f51da12500f in raw_parse_flags at 
/opt/qemu/qemu-2.5.0/block/raw-posix.c:358
  (gdb)

  call drive_mirror and debug:

  (gdb)
  raw_open_common (bs=0x7f4b27259ab0, options=0x7f4b27480290, bdrv_flags=24674, 
open_flags=0, errp=0x7fff4a19f548)
  at /opt/qemu/qemu-2.5.0/block/raw-posix.c:484
  484s->fd = -1;
  (gdb) n
  485fd = qemu_open(filename, s->open_flags, 0644);
  (gdb) s
  qemu_open (name=0x7f4b2642b5c0 "/dev/vg_bc_local/test", flags=2) at 
/opt/qemu/qemu-2.5.0/util/osdep.c:177
  177int mode = 0;
  (gdb) n
  183if (strstart(name, "/dev/fdset/", &fdset_id_str

Re: [PATCH v2 05/13] spapr/xive: Use device_class_set_parent_realize()

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:47PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz 
> 
> The XIVE router base class currently inherits an empty realize hook
> from the sysbus device base class, but it will soon implement one
> of its own to perform some sanity checks. Do the preliminary plumbing
> to have it called.
> 
> Signed-off-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

Applied to ppc-for-5.0, thanks.

> ---
>  include/hw/ppc/spapr_xive.h | 10 ++
>  hw/intc/spapr_xive.c| 12 +++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 3a103c224d44..93d09d68deb7 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -15,6 +15,10 @@
>  
>  #define TYPE_SPAPR_XIVE "spapr-xive"
>  #define SPAPR_XIVE(obj) OBJECT_CHECK(SpaprXive, (obj), TYPE_SPAPR_XIVE)
> +#define SPAPR_XIVE_CLASS(klass) \
> +OBJECT_CLASS_CHECK(SpaprXiveClass, (klass), TYPE_SPAPR_XIVE)
> +#define SPAPR_XIVE_GET_CLASS(obj)   \
> +OBJECT_GET_CLASS(SpaprXiveClass, (obj), TYPE_SPAPR_XIVE)
>  
>  typedef struct SpaprXive {
>  XiveRouterparent;
> @@ -47,6 +51,12 @@ typedef struct SpaprXive {
>  VMChangeStateEntry *change;
>  } SpaprXive;
>  
> +typedef struct SpaprXiveClass {
> +XiveRouterClass parent;
> +
> +DeviceRealize parent_realize;
> +} SpaprXiveClass;
> +
>  /*
>   * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
>   * to the controller block id value. It can nevertheless be changed
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 57305c56d707..32322470a8b8 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -286,10 +286,17 @@ static void spapr_xive_instance_init(Object *obj)
>  static void spapr_xive_realize(DeviceState *dev, Error **errp)
>  {
>  SpaprXive *xive = SPAPR_XIVE(dev);
> +SpaprXiveClass *sxc = SPAPR_XIVE_GET_CLASS(xive);
>  XiveSource *xsrc = &xive->source;
>  XiveENDSource *end_xsrc = &xive->end_source;
>  Error *local_err = NULL;
>  
> +sxc->parent_realize(dev, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
>  if (!xive->nr_irqs) {
>  error_setg(errp, "Number of interrupt needs to be greater 0");
>  return;
> @@ -760,10 +767,12 @@ static void spapr_xive_class_init(ObjectClass *klass, 
> void *data)
>  XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
>  SpaprInterruptControllerClass *sicc = SPAPR_INTC_CLASS(klass);
>  XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
> +SpaprXiveClass *sxc = SPAPR_XIVE_CLASS(klass);
>  
>  dc->desc= "sPAPR XIVE Interrupt Controller";
>  dc->props   = spapr_xive_properties;
> -dc->realize = spapr_xive_realize;
> +device_class_set_parent_realize(dc, spapr_xive_realize,
> +&sxc->parent_realize);
>  dc->vmsd= &vmstate_spapr_xive;
>  
>  xrc->get_eas = spapr_xive_get_eas;
> @@ -794,6 +803,7 @@ static const TypeInfo spapr_xive_info = {
>  .instance_init = spapr_xive_instance_init,
>  .instance_size = sizeof(SpaprXive),
>  .class_init = spapr_xive_class_init,
> +.class_size = sizeof(SpaprXiveClass),
>  .interfaces = (InterfaceInfo[]) {
>  { TYPE_SPAPR_INTC },
>  { }

-- 
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: [PATCH v2 06/13] pnv/xive: Use device_class_set_parent_realize()

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:48PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz 
> 
> The XIVE router base class currently inherits an empty realize hook
> from the sysbus device base class, but it will soon implement one
> of its own to perform some sanity checks. Do the preliminary plumbing
> to have it called.
> 
> Signed-off-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

But will need a rebase due to changes earlier in the series.

> ---
>  include/hw/ppc/pnv_xive.h | 10 ++
>  hw/intc/pnv_xive.c| 10 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
> index 4d641db691c8..ba9bbeab88c3 100644
> --- a/include/hw/ppc/pnv_xive.h
> +++ b/include/hw/ppc/pnv_xive.h
> @@ -16,6 +16,10 @@ struct PnvChip;
>  
>  #define TYPE_PNV_XIVE "pnv-xive"
>  #define PNV_XIVE(obj) OBJECT_CHECK(PnvXive, (obj), TYPE_PNV_XIVE)
> +#define PNV_XIVE_CLASS(klass)   \
> +OBJECT_CLASS_CHECK(PnvXiveClass, (klass), TYPE_PNV_XIVE)
> +#define PNV_XIVE_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(PnvXiveClass, (obj), TYPE_PNV_XIVE)
>  
>  #define XIVE_BLOCK_MAX  16
>  
> @@ -87,6 +91,12 @@ typedef struct PnvXive {
>  uint64_t  edt[XIVE_TABLE_EDT_MAX];
>  } PnvXive;
>  
> +typedef struct PnvXiveClass {
> +XiveRouterClass parent_class;
> +
> +DeviceRealize parent_realize;
> +} PnvXiveClass;
> +
>  void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon);
>  
>  #endif /* PPC_PNV_XIVE_H */
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 66970a60733b..1962f884d6de 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1816,10 +1816,17 @@ static void pnv_xive_init(Object *obj)
>  static void pnv_xive_realize(DeviceState *dev, Error **errp)
>  {
>  PnvXive *xive = PNV_XIVE(dev);
> +PnvXiveClass *pxc = PNV_XIVE_GET_CLASS(dev);
>  XiveSource *xsrc = &xive->ipi_source;
>  XiveENDSource *end_xsrc = &xive->end_source;
>  Error *local_err = NULL;
>  
> +pxc->parent_realize(dev, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
>  assert(xive->chip);
>  assert(xive->system_memory);
>  
> @@ -1950,10 +1957,12 @@ static void pnv_xive_class_init(ObjectClass *klass, 
> void *data)
>  XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
>  XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
>  XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
> +PnvXiveClass *pxc = PNV_XIVE_CLASS(klass);
>  
>  xdc->dt_xscom = pnv_xive_dt_xscom;
>  
>  dc->desc = "PowerNV XIVE Interrupt Controller";
> +device_class_set_parent_realize(dc, pnv_xive_realize, 
> &pxc->parent_realize);
>  dc->realize = pnv_xive_realize;
>  dc->props = pnv_xive_properties;
>  
> @@ -1974,6 +1983,7 @@ static const TypeInfo pnv_xive_info = {
>  .instance_init = pnv_xive_init,
>  .instance_size = sizeof(PnvXive),
>  .class_init= pnv_xive_class_init,
> +.class_size= sizeof(PnvXiveClass),
>  .interfaces= (InterfaceInfo[]) {
>  { TYPE_PNV_XSCOM_INTERFACE },
>  { }

-- 
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: [PATCH v2 04/13] ppc/pnv: Introduce a "xics" property under the POWER8 chip

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:46PM +0100, Cédric Le Goater wrote:
> POWER8 is the only chip using the XICS interface. Add a "xics" link
> and a XICSFabric attribute under this chip to remove the use of
> qdev_get_machine()
> 
> Signed-off-by: Cédric Le Goater 
> Reviewed-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

but will need a rebase.

> ---
>  include/hw/ppc/pnv.h |  2 ++
>  hw/ppc/pnv.c | 26 --
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index f31180618672..8b957dfb5736 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -74,6 +74,8 @@ typedef struct Pnv8Chip {
>  Pnv8Psi  psi;
>  PnvOCC   occ;
>  PnvHomer homer;
> +
> +XICSFabric*xics;
>  } Pnv8Chip;
>  
>  #define TYPE_PNV9_CHIP "pnv9-chip"
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 163a658806e2..2a1b15a69aed 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -797,6 +797,13 @@ static void pnv_init(MachineState *machine)
>   */
>  object_property_set_link(chip, OBJECT(sysmem), "system-memory",
>   &error_abort);
> +/*
> + * The POWER8 machine use the XICS interrupt interface.
> + * Propagate the XICS fabric to the chip and its controllers.
> + */
> +if (object_dynamic_cast(OBJECT(pnv), TYPE_XICS_FABRIC)) {
> +object_property_set_link(chip, OBJECT(pnv), "xics", 
> &error_abort);
> +}
>  object_property_set_bool(chip, true, "realized", &error_fatal);
>  }
>  g_free(chip_typename);
> @@ -838,12 +845,12 @@ static uint32_t pnv_chip_core_pir_p8(PnvChip *chip, 
> uint32_t core_id)
>  static void pnv_chip_power8_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>  Error **errp)
>  {
> +Pnv8Chip *chip8 = PNV8_CHIP(chip);
>  Error *local_err = NULL;
>  Object *obj;
>  PnvCPUState *pnv_cpu = pnv_cpu_state(cpu);
>  
> -obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, 
> XICS_FABRIC(qdev_get_machine()),
> - &local_err);
> +obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, chip8->xics, &local_err);
>  if (local_err) {
>  error_propagate(errp, local_err);
>  return;
> @@ -997,6 +1004,12 @@ static void pnv_chip_power8_instance_init(Object *obj)
>  {
>  Pnv8Chip *chip8 = PNV8_CHIP(obj);
>  
> +object_property_add_link(obj, "xics", TYPE_XICS_FABRIC,
> + (Object **)&chip8->xics,
> + object_property_allow_set_link,
> + OBJ_PROP_LINK_STRONG,
> + &error_abort);
> +
>  object_initialize_child(obj, "psi",  &chip8->psi, sizeof(chip8->psi),
>  TYPE_PNV8_PSI, &error_abort, NULL);
>  
> @@ -1016,7 +1029,6 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error 
> **errp)
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>  int i, j;
>  char *name;
> -XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
>  
>  name = g_strdup_printf("icp-%x", chip->chip_id);
>  memory_region_init(&chip8->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
> @@ -1032,7 +1044,7 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error 
> **errp)
>  
>  for (j = 0; j < CPU_CORE(pnv_core)->nr_threads; j++) {
>  uint32_t pir = pcc->core_pir(chip, core_hwid) + j;
> -PnvICPState *icp = PNV_ICP(xics_icp_get(xi, pir));
> +PnvICPState *icp = PNV_ICP(xics_icp_get(chip8->xics, pir));
>  
>  memory_region_add_subregion(&chip8->icp_mmio, pir << 12,
>  &icp->mmio);
> @@ -1048,6 +1060,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, 
> Error **errp)
>  Pnv8Psi *psi8 = &chip8->psi;
>  Error *local_err = NULL;
>  
> +assert(chip8->xics);
> +
>  /* XSCOM bridge is first */
>  pnv_xscom_realize(chip, PNV_XSCOM_SIZE, &local_err);
>  if (local_err) {
> @@ -1067,8 +1081,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, 
> Error **errp)
>  "bar", &error_fatal);
>  object_property_set_link(OBJECT(&chip8->psi), 
> OBJECT(chip->system_memory),
>   "system-memory", &error_abort);
> -object_property_set_link(OBJECT(&chip8->psi), OBJECT(qdev_get_machine()),
> - ICS_PROP_XICS, &error_abort);
> +object_property_set_link(OBJECT(&chip8->psi), OBJECT(chip8->xics),
> +  ICS_PROP_XICS, &error_abort);
>  object_property_set_bool(OBJECT(&chip8->psi), true, "realized", 
> &local_err);
>  if (local_err) {
>  error_propagate(errp, local_err);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other

[Bug 1857269] [NEW] Spanish keyboard from Spain (Europe) not found

[José Antonio López Cano]

Public bug reported:

Hello,

I am working with windows qemu version:

qemu-w64-setup-20190815

I have installed a msdos virtual machine on qemu with sp keyboard layout
(Spain at Europe). I have found that some keys do not work in the way
they should. I believe that the problem is that es qemu spanish keyboard
layout is the latin one, la in msdos sysytem.

I ask you to create the Spain layout.


Thanks in advance.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Spanish keyboard from Spain (Europe) not found

Status in QEMU:
  New

Bug description:
  Hello,

  I am working with windows qemu version:

  qemu-w64-setup-20190815

  I have installed a msdos virtual machine on qemu with sp keyboard
  layout (Spain at Europe). I have found that some keys do not work in
  the way they should. I believe that the problem is that es qemu
  spanish keyboard layout is the latin one, la in msdos sysytem.

  I ask you to create the Spain layout.


  Thanks in advance.

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

[PATCH] dma/rc4030: correctly reset DMA translation table at reset

[Hervé Poussineau]

This fixes a freeze at reboot, introduced in 
c627e7526a902dd5bb1907dbbd5cf961679dfa68

Signed-off-by: Hervé Poussineau 
---
 hw/dma/rc4030.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index c4cf8236f4..76302fe431 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -534,6 +534,7 @@ static void rc4030_reset(DeviceState *dev)
 
 memset(s->dma_regs, 0, sizeof(s->dma_regs));
 
+s->dma_tl_base = s->dma_tl_limit = 0;
 s->remote_failed_address = s->memory_failed_address = 0;
 s->cache_maint = 0;
 s->cache_ptag = s->cache_ltag = 0;
-- 
2.19.2

Re: [PATCH v39 04/22] target/avr: Add instruction translation - Registers definition

[Aleksandar Markovic]

On Wednesday, December 18, 2019, Michael Rolnik  wrote:

> Signed-off-by: Michael Rolnik 
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
> ---
>  target/avr/translate.c | 143 +
>  1 file changed, 143 insertions(+)
>  create mode 100644 target/avr/translate.c
>
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> new file mode 100644
> index 00..0139bcabb1
> --- /dev/null
> +++ b/target/avr/translate.c
> @@ -0,0 +1,143 @@
> +/*
> + * QEMU AVR CPU
> + *
> + * Copyright (c) 2019 Michael Rolnik
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> + * 
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/qemu-print.h"
> +#include "tcg/tcg.h"
> +#include "cpu.h"
> +#include "exec/exec-all.h"
> +#include "tcg-op.h"
> +#include "exec/cpu_ldst.h"
> +#include "exec/helper-proto.h"
> +#include "exec/helper-gen.h"
> +#include "exec/log.h"
> +#include "exec/translator.h"
> +#include "exec/gen-icount.h"
> +
> +/*
> + *  Define if you want a BREAK instruction translated to a breakpoint
> + *  Active debugging connection is assumed
> + *  This is for
> + *  https://github.com/seharris/qemu-avr-tests/tree/master/
> instruction-tests
> + *  tests
> + */
> +#undef BREAKPOINT_ON_BREAK
> +
> +static TCGv cpu_pc;
> +
> +static TCGv cpu_Cf;
> +static TCGv cpu_Zf;
> +static TCGv cpu_Nf;
> +static TCGv cpu_Vf;
> +static TCGv cpu_Sf;
> +static TCGv cpu_Hf;
> +static TCGv cpu_Tf;
> +static TCGv cpu_If;
> +
> +static TCGv cpu_rampD;
> +static TCGv cpu_rampX;
> +static TCGv cpu_rampY;
> +static TCGv cpu_rampZ;
> +
> +static TCGv cpu_r[NUMBER_OF_CPU_REGISTERS];
> +static TCGv cpu_eind;
> +static TCGv cpu_sp;
> +
> +static TCGv cpu_skip;
> +
> +static const char reg_names[NUMBER_OF_CPU_REGISTERS][8] = {
> +"r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
> +"r8",  "r9",  "r10", "r11", "r12", "r13", "r14", "r15",
> +"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> +"r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
> +};
> +#define REG(x) (cpu_r[x])
> +
> +enum {
> +DISAS_EXIT   = DISAS_TARGET_0,  /* We want return to the cpu main
> loop.  */
> +DISAS_LOOKUP = DISAS_TARGET_1,  /* We have a variable condition
> exit.  */
> +DISAS_CHAIN  = DISAS_TARGET_2,  /* We have a single condition exit.
> */
> +};
> +
> +typedef struct DisasContext DisasContext;
> +
> +/* This is the state at translation time. */
> +struct DisasContext {
> +TranslationBlock *tb;
> +
> +CPUAVRState *env;
> +CPUState *cs;
> +
> +target_long npc;
> +uint32_t opcode;
> +
> +/* Routine used to access memory */
> +int memidx;
> +int bstate;
> +int singlestep;
> +
> +TCGv skip_var0;
> +TCGv skip_var1;
> +TCGCond skip_cond;
> +bool free_skip_var0;




[PATCH v36 04/17] target/avr: Add instruction translation - Registers
definition
Inbox
M
Michael Rolnik
Signed-off-by: Michael Rolnik  ---
target/avr/translate.c | 132 + 1
file changed, 132 in...
A
Aleksandar Markovic
to Michael, QEMU, Richard, +5
Nov 26
Details
This set of four lines are by far the hardest to connect to the
documentation.

Please add before them a sizable comment with explanations for:

- the reason these variables are introduced
- why are they here (part of DisasContext)
- what other variables elsewhere in your solutiin are closely related to
these four variables
- what they affect
- summary of the way they work

Perhaps add comments to other places where "skip"-related data fields
are introduced too.

(I believe the implementation is correct, but it is extremely hard to the
reader
to reverse-engineer the intentions)

Yours, Aleksandar
Aleksa

> +};
> +
> +static int to_regs_16_31_by_one(DisasContext *ctx, int indx)
> +{
> +return 16 + (indx % 16);
> +}
> +
> +static int to_regs_16_23_by_one(DisasContext *ctx, int indx)
> +{
> +return 16 + (indx % 8);
> +}
> +static int to_regs_24_30_by_two(DisasContext *ctx, int indx)
> +{
> +return 24 + (indx % 4) * 2;
> +}
> +static int to_regs_00_30_by_two(DisasContext *ctx, int indx)
> +{
> +return (indx % 16) * 2;
> +}
> +
> +static uint16_t next_word(DisasContext *ctx)
> +{
> +return cpu_lduw_code(ctx->env, ctx->npc++ * 2);
> +}
> +
> +static int append_16(DisasCont

Re: [PATCH v39 05/22] target/avr: Add instruction translation - Arithmetic and Logic Instructions

[Aleksandar Markovic]

On Wednesday, December 18, 2019, Michael Rolnik  wrote:

> This includes:
> - ADD, ADC, ADIW
> - SBIW, SUB, SUBI, SBC, SBCI
> - AND, ANDI
> - OR, ORI, EOR
> - COM, NEG
> - INC, DEC
> - MUL, MULS, MULSU
> - FMUL, FMULS, FMULSU
> - DES
>
>
>
...

+
> +/*
> + *  Performs the logical AND between the contents of register Rd and
> register
> + *  Rr and places the result in the destination register Rd.
> + */
> +static bool trans_AND(DisasContext *ctx, arg_AND *a)
> +{
> +TCGv Rd = cpu_r[a->rd];
> +TCGv Rr = cpu_r[a->rr];
> +TCGv R = tcg_temp_new_i32();
> +
> +tcg_gen_and_tl(R, Rd, Rr); /* Rd = Rd and Rr */
> +tcg_gen_movi_tl(cpu_Vf, 0); /* Vf = 0 */


Hi, Michael.

Please add before this line a blank line and a comment:

/* update status register */

This is needed to visually separate core functionality and updating status
register in trans_AND() function.

And please repeat that for all instructions that update status register.

Regards,
Aleksandar

Re: [PATCH v27 06/21] target/rx: CPU definition

[Yoshinori Sato]

On Sun, 22 Dec 2019 01:03:04 +0900,
Aleksandar Markovic wrote:
> 
> [1  ]
> [2  ]
> On Saturday, December 21, 2019, Yoshinori Sato 
> wrote:
> 
> Signed-off-by: Yoshinori Sato 
>
> Message-Id: <20190616142836.10614-4-ys...@users.sourceforge.jp>
> Reviewed-by: Richard Henderson 
> Message-Id: <20190607091116.49044-4-ys...@users.sourceforge.jp>
> 
> Hi, Yoshinori,
> 
> I noticed you have a lot of line like this
> 
> Message-Id: <20190607091116.49044-9-ys...@users.sourceforge.jp>
> 
> ... sometimes even several in the same message.
> 
> May I ask you to delete *all* such lines from commit messages, and resend the
> series?
> 
> Also, I don't see  the documentation update for the new target. Please provide
> dome appropriate content in qemu-doc.texi (
> https://github.com/qemu/qemu/blob/master/qemu-doc.texi ), and include it in
> the next version.
> 
> Best regards,
> 
> Aleksandar

OK. I will update.
Thanks.
 
> 
> Signed-off-by: Richard Henderson 
> [PMD: Use newer QOM style, split cpu-qom.h, restrict access to
>  extable array, use rx_cpu_tlb_fill() extracted from patch of
>  Yoshinori Sato 'Convert to CPUClass::tlb_fill']
> Signed-off-by: Philippe Mathieu-Daudé 
> Acked-by: Igor Mammedov 
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/cpu-param.h   |  31 ++
>  target/rx/cpu-qom.h     |  42 
>  target/rx/cpu.h         | 181 +
>  target/rx/cpu.c         | 217 
>  target/rx/gdbstub.c     | 112 +
>  target/rx/Makefile.objs |   1 -
>  6 files changed, 583 insertions(+), 1 deletion(-)
>  create mode 100644 target/rx/cpu-param.h
>  create mode 100644 target/rx/cpu-qom.h
>  create mode 100644 target/rx/cpu.h
>  create mode 100644 target/rx/cpu.c
>  create mode 100644 target/rx/gdbstub.c
>
> diff --git a/target/rx/cpu-param.h b/target/rx/cpu-param.h
> new file mode 100644
> index 00..5da87fbebe
> --- /dev/null
> +++ b/target/rx/cpu-param.h
> @@ -0,0 +1,31 @@
> +/*
> + *  RX cpu parameters
> + *
> + *  Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify
> it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#ifndef RX_CPU_PARAM_H
> +#define RX_CPU_PARAM_H
> +
> +#define TARGET_LONG_BITS 32
> +#define TARGET_PAGE_BITS 12
> +
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
> +#define NB_MMU_MODES 1
> +#define MMU_MODE0_SUFFIX _all
> +
> +#endif
> diff --git a/target/rx/cpu-qom.h b/target/rx/cpu-qom.h
> new file mode 100644
> index 00..8328900f3f
> --- /dev/null
> +++ b/target/rx/cpu-qom.h
> @@ -0,0 +1,42 @@
> +#ifndef QEMU_RX_CPU_QOM_H
> +#define QEMU_RX_CPU_QOM_H
> +
> +#include "hw/core/cpu.h"
> +/*
> + * RX CPU
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + * SPDX-License-Identifier: LGPL-2.0+
> + */
> +
> +#define TYPE_RX_CPU "rx-cpu"
> +
> +#define TYPE_RX62N_CPU RX_CPU_TYPE_NAME("rx62n")
> +
> +#define RXCPU_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(RXCPUClass, (klass), TYPE_RX_CPU)
> +#define RXCPU(obj) \
> +    OBJECT_CHECK(RXCPU, (obj), TYPE_RX_CPU)
> +#define RXCPU_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(RXCPUClass, (obj), TYPE_RX_CPU)
> +
> +/*
> + * RXCPUClass:
> + * @parent_realize: The parent class' realize handler.
> + * @parent_reset: The parent class' reset handler.
> + *
> + * A RX CPU model.
> + */
> +typedef struct RXCPUClass {
> +    /*< private >*/
> +    CPUClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    void (*parent_reset)(CPUState *cpu);
> +
> +} RXCPUClass;
> +
> +#define CPUArchState struct CPURXState
> +
> +#endif
> diff --git a/target/rx/cpu.h b/target/rx/cpu.h
> new file mode 100644
> index 00..2d1eb7665c
> --- /dev/null
> +++ b/target/rx/cpu.h
> @@ -0,0 +1,181 @@
> +/*
> + *  RX emulation definition
> + *
> + *  Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redist

[Bug 1857226] [NEW] 'set_link net0 off' not working with e1000e driver

[Jannik]

Public bug reported:

I'm encountering a bug with the e1000e network driver, that appears to
got previously reported at rhbz. Steps to reproduce are provided in
detail there:

https://bugzilla.redhat.com/show_bug.cgi?id=1707646

It is about switching off network link state (set_link net0 off) having
no effect when using the e1000e driver.

Version details:
QEMU emulator version 4.1.1 (qemu-4.1.1-1.fc31)
Fedora 31

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  'set_link net0 off' not working with e1000e driver

Status in QEMU:
  New

Bug description:
  I'm encountering a bug with the e1000e network driver, that appears to
  got previously reported at rhbz. Steps to reproduce are provided in
  detail there:

  https://bugzilla.redhat.com/show_bug.cgi?id=1707646

  It is about switching off network link state (set_link net0 off)
  having no effect when using the e1000e driver.

  Version details:
  QEMU emulator version 4.1.1 (qemu-4.1.1-1.fc31)
  Fedora 31

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

Re: [PATCH v5 0/2] virtio: make seg_max virtqueue size dependent

[Michael S. Tsirkin]

On Fri, Dec 20, 2019 at 05:09:03PM +0300, Denis Plotnikov wrote:
> v5:
>   * rebased on the recent master [MST]
>   * NOTE: the test doesn't pass because 5.0 machine type use 4.2 compat
>   instead of it's own or no compat at all. The test will pass
>   once the new 5.0 compat is used. 


So please fix that up first (in a separate patch).

> v4:
>   * rebased on 4.2 [MST]
> 
> v3:
>   * add property to set in machine type [MST]
>   * add min queue size check [Stefan]
>   * add avocado based test [Max, Stefan, Eduardo, Cleber]
> 
> v2:
>   * the standalone patch to make seg_max virtqueue size dependent
>   * other patches are postponed
> 
> v1:
>   the initial series
> 
> Denis Plotnikov (2):
>   virtio: make seg_max virtqueue size dependent
>   tests: add virtio-scsi and virtio-blk seg_max_adjust test
> 
>  hw/block/virtio-blk.c |   9 +-
>  hw/core/machine.c |   3 +
>  hw/scsi/vhost-scsi.c  |   2 +
>  hw/scsi/virtio-scsi.c |  10 +-
>  include/hw/virtio/virtio-blk.h|   1 +
>  include/hw/virtio/virtio-scsi.h   |   1 +
>  tests/acceptance/virtio_seg_max_adjust.py | 134 ++
>  7 files changed, 158 insertions(+), 2 deletions(-)
>  create mode 100755 tests/acceptance/virtio_seg_max_adjust.py
> 
> -- 
> 2.17.0

Re: [PULL 00/24] virtio, pci, pc: fixes, features

[Michael S. Tsirkin]

On Fri, Dec 20, 2019 at 06:24:43PM +, Peter Maydell wrote:
> On Thu, 19 Dec 2019 at 13:27, Michael S. Tsirkin  wrote:
> >
> > The following changes since commit b0ca999a43a22b38158a33d3f5881648bb4f:
> >
> >   Update version for v4.2.0 release (2019-12-12 16:45:57 +)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to d4fbea918a37c0586f1a0e15ac6ef04c9fc7b96b:
> >
> >   vhost-user-scsi: reset the device if supported (2019-12-19 08:25:35 -0500)
> >
> > 
> > virtio, pci, pc: fixes, features
> >
> > Bugfixes all over the place.
> > HMAT support.
> > New flags for vhost-user-blk utility.
> >
> > Signed-off-by: Michael S. Tsirkin 
> >
> 
> Compile failure on OSX:
> /Users/pm215/src/qemu-for-merges/hw/core/numa.c:427:20: error: format
> specifies type 'unsigned char' but the argument has type 'int'
> [-Werror,-Wformat]
>node->level - 1);
> ~~~^~~
> /Users/pm215/src/qemu-for-merges/include/qapi/error.h:166:35: note:
> expanded from macro 'error_setg'
> (fmt), ## __VA_ARGS__)
>   ^~~
> /Users/pm215/src/qemu-for-merges/hw/core/numa.c:440:20: error: format
> specifies type 'unsigned char' but the argument has type 'int'
> [-Werror,-Wformat]
>node->level + 1);
> ~~~^~~
> /Users/pm215/src/qemu-for-merges/include/qapi/error.h:166:35: note:
> expanded from macro 'error_setg'
> (fmt), ## __VA_ARGS__)
>   ^~~
> 2 errors generated.
> 
> thanks
> -- PMM


I think I fixed this up, can you try again with
new for_upstream at cd8b62554728373e3dcdbc450a3d76a9ce4e7beb please?

Re: [PULL 00/24] virtio, pci, pc: fixes, features

[Michael S. Tsirkin]

On Fri, Dec 20, 2019 at 06:24:43PM +, Peter Maydell wrote:
> On Thu, 19 Dec 2019 at 13:27, Michael S. Tsirkin  wrote:
> >
> > The following changes since commit b0ca999a43a22b38158a33d3f5881648bb4f:
> >
> >   Update version for v4.2.0 release (2019-12-12 16:45:57 +)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to d4fbea918a37c0586f1a0e15ac6ef04c9fc7b96b:
> >
> >   vhost-user-scsi: reset the device if supported (2019-12-19 08:25:35 -0500)
> >
> > 
> > virtio, pci, pc: fixes, features
> >
> > Bugfixes all over the place.
> > HMAT support.
> > New flags for vhost-user-blk utility.
> >
> > Signed-off-by: Michael S. Tsirkin 
> >
> 
> Compile failure on OSX:
> /Users/pm215/src/qemu-for-merges/hw/core/numa.c:427:20: error: format
> specifies type 'unsigned char' but the argument has type 'int'
> [-Werror,-Wformat]
>node->level - 1);
> ~~~^~~
> /Users/pm215/src/qemu-for-merges/include/qapi/error.h:166:35: note:
> expanded from macro 'error_setg'
> (fmt), ## __VA_ARGS__)
>   ^~~
> /Users/pm215/src/qemu-for-merges/hw/core/numa.c:440:20: error: format
> specifies type 'unsigned char' but the argument has type 'int'
> [-Werror,-Wformat]
>node->level + 1);
> ~~~^~~
> /Users/pm215/src/qemu-for-merges/include/qapi/error.h:166:35: note:
> expanded from macro 'error_setg'
> (fmt), ## __VA_ARGS__)
>   ^~~
> 2 errors generated.
> 
> thanks
> -- PMM


Could you pls check whether the following fixes it?
Thanks!

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 33fda31a4c..747c9680b0 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -421,7 +421,7 @@ void parse_numa_hmat_cache(MachineState *ms, 
NumaHmatCacheOptions *node,
 ms->numa_state->hmat_cache[node->node_id][node->level - 1]->size)) 
{
 error_setg(errp, "Invalid size=%" PRIu64 ", the size of level=%" PRIu8
" should be less than the size(%" PRIu64 ") of "
-   "level=%" PRIu8, node->size, node->level,
+   "level=%u", node->size, node->level,
ms->numa_state->hmat_cache[node->node_id]
  [node->level - 1]->size,
node->level - 1);
@@ -434,7 +434,7 @@ void parse_numa_hmat_cache(MachineState *ms, 
NumaHmatCacheOptions *node,
 ms->numa_state->hmat_cache[node->node_id][node->level + 1]->size)) 
{
 error_setg(errp, "Invalid size=%" PRIu64 ", the size of level=%" PRIu8
" should be larger than the size(%" PRIu64 ") of "
-   "level=%" PRIu8, node->size, node->level,
+   "level=%u", node->size, node->level,
ms->numa_state->hmat_cache[node->node_id]
  [node->level + 1]->size,
node->level + 1);

Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

[Philippe Mathieu-Daudé]

On 12/20/19 10:15 PM, Sven Schnelle wrote:
> This adds emulation of Artist graphics good enough
> to get a Text console on both Linux and HP-UX. The
> X11 server from HP-UX also works.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  hw/display/Kconfig   |4 +
>  hw/display/Makefile.objs |1 +
>  hw/display/artist.c  | 1450 ++
>  hw/display/trace-events  |9 +
>  hw/hppa/Kconfig  |1 +
>  hw/hppa/hppa_hardware.h  |1 +
>  hw/hppa/machine.c|9 +
>  7 files changed, 1475 insertions(+)
>  create mode 100644 hw/display/artist.c
> 
[...]
> diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h
> index 507f91e05d..4a2fe2df60 100644
> --- a/hw/hppa/hppa_hardware.h
> +++ b/hw/hppa/hppa_hardware.h
> @@ -22,6 +22,7 @@
>  #define LASI_PS2KBD_HPA 0xffd08000
>  #define LASI_PS2MOU_HPA 0xffd08100
>  #define LASI_GFX_HPA0xf800
> +#define ARTIST_FB_ADDR  0xf900
>  #define CPU_HPA 0xfffb
>  #define MEMORY_HPA  0xfffbf000
>  
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index 33e3769d0b..6c67399054 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -75,6 +75,7 @@ static void machine_hppa_init(MachineState *machine)
>  MemoryRegion *cpu_region;
>  long i;
>  unsigned int smp_cpus = machine->smp.cpus;
> +SysBusDevice *s;
>  
>  ram_size = machine->ram_size;
>  
> @@ -127,6 +128,14 @@ static void machine_hppa_init(MachineState *machine)
>  dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
>  lsi53c8xx_handle_legacy_cmdline(dev);
>  
> +if (vga_interface_type != VGA_NONE) {
> +dev = qdev_create(NULL, "artist");
> +qdev_init_nofail(dev);
> +s = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(s, 0, LASI_GFX_HPA);
> +sysbus_mmio_map(s, 1, ARTIST_FB_ADDR);

How is this chipset connected on the board?
If it is a card you can plug on a bus, you can use a condition.
If it is soldered or part of another chipset, then it has to be mapped
unconditionally.
IOW I think you should drop the 'if (vga_interface_type != VGA_NONE)' check.

> +}
> +
>  /* Network setup.  e1000 is good enough, failing Tulip support.  */
>  for (i = 0; i < nb_nics; i++) {
>  if (!enable_lasi_lan()) {
> 

Re: [PATCH v5 6/6] seabios-hppa: update to latest version

[Philippe Mathieu-Daudé]

On 12/20/19 10:15 PM, Sven Schnelle wrote:
> Helge Deller (13):
>   Add PDC_MEM_MAP and ENTRY_INIT_SRCH_FRST for OSF/MkLinux
>   Return non-existant BTLB for PDC_BLOCK_TLB
>   Add serial, parallel and LAN port support of  LASI chip
>   Implement ENTRY_IO_BBLOCK_IN IODC function
>   Do not print \r on parisc SeaBIOS
>   Fix serial ports and add PDC_MODEL functions for special instructions 
> enablement
>   Implement SeaBIOS returning additional addresses. Fixes HP-UX boot.
>   Fix mod_pgs (number of pages) for graphic cards
>   Merge pull request #3 from svenschnelle/sti
>   Merge pull request #4 from svenschnelle/parisc-qemu-4.1.0
>   parisc: Implement PDC rendenzvous
>   parisc: Improve soft power button emulation
>   parisc: Fix line wrapping in STI console code
> 
> Sven Schnelle (7):
>   parisc: fix PDC info for graphics adapter
>   parisc: add missing header guard to hppa.h
>   parisc: add LASI PS/2 emulation.
>   parisc: Add STI support
>   parisc: wire up graphics console
>   parisc: Add support for setting STI screen resolution
>   parisc: support LASI RTC register
> 
> Required for STI and LASI support. Also adds a few Bugfixes.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  pc-bios/hppa-firmware.img | Bin 783724 -> 766136 bytes
>  roms/seabios-hppa |   2 +-
>  2 files changed, 1 insertion(+), 1 deletion(-)

Odd, MTA from work refused to deliver this patch because it "might
contains virus" =)

Since you share url of repository with content, you can send your series
with `git-format-patch --no-binary`.

Re: [PATCH] hw/i386/pc: fix regression in parsing vga cmdline parameter

[Paolo Bonzini]

On 21/12/19 17:21, Peter Wu wrote:
> When the 'vga=' parameter is succeeded by another parameter, QEMU 4.2.0
> would refuse to start with a rather cryptic message:
> 
> $ qemu-system-x86_64 -kernel /boot/vmlinuz-linux -append 'vga=792 quiet'
> qemu: can't parse 'vga' parameter: Invalid argument
> 
> It was not clear whether this applied to the '-vga std' parameter or the
> '-append' one. Fix the parsing regression and clarify the error.
> 
> Fixes: 133ef074bd ("hw/i386/pc: replace use of strtol with qemu_strtoui in 
> x86_load_linux()")
> Cc: Sergio Lopez 
> Signed-off-by: Peter Wu 
> ---
> Hi,
> 
> This fixes a regression in QEMU 4.2.0 where my existing scripts would
> fail to boot while it worked fine with QEMU 4.1.1.
> 
> I do wonder whether QEMU has any business in strictly enforcing the
> contents of the kernel command line. Perhaps it should only warn about
> the issue, and not exit? Previously it would silently ignore bad values.
> 
> Kind regards,
> Peter
> ---
>  hw/i386/x86.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 394edc2f72..121650ae51 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -508,6 +508,7 @@ void x86_load_linux(X86MachineState *x86ms,
>  vmode = strstr(kernel_cmdline, "vga=");
>  if (vmode) {
>  unsigned int video_mode;
> +const char *end;
>  int ret;
>  /* skip "vga=" */
>  vmode += 4;
> @@ -518,10 +519,9 @@ void x86_load_linux(X86MachineState *x86ms,
>  } else if (!strncmp(vmode, "ask", 3)) {
>  video_mode = 0xfffd;
>  } else {
> -ret = qemu_strtoui(vmode, NULL, 0, &video_mode);
> -if (ret != 0) {
> -fprintf(stderr, "qemu: can't parse 'vga' parameter: %s\n",
> -strerror(-ret));
> +ret = qemu_strtoui(vmode, &end, 0, &video_mode);
> +if (ret != 0 || (*end && *end != ' ')) {
> +fprintf(stderr, "qemu: invalid 'vga=' kernel parameter.\n");
>  exit(1);
>  }
>  }
> 

Cc: qemu-sta...@nongnu.org

Queued, thanks.

Re: [Qemu-devel] Maintainers, please tell us how to boot your machines!

[Philippe Mathieu-Daudé]

On 5/17/19 7:42 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> How do you want to proceed with all the information provided in this
>> thread? I think a big table in the wiki collecting the answers is ideal.
>> What do you think?
> 
> Yes, please!  I haven't been able to find the time...

I gathered all the information from this thread here:
https://wiki.qemu.org/Testing/Acceptance#Machines
(with a link in https://wiki.qemu.org/Testing#System_emulation).

I also added other info I collected during 4.2 merge window.

Should we suggest a new policy that new machines must have a test?
I'll later purpose some idea to deal with machines only running non
opensource code.

I think most of the data from the acceptance tests we have could be
generated (json?) and we could concat with another manual maintained
json (or yaml to json?) to have this table easily updatable on the wiki.
Now I remember why I had forgotten about HTML, it is painful to edit.

Thanks all for sharing this information.

Regards,

Phil.

[RFC PATCH v3 25/27] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit

[Alberto Garcia]

Now that the implementation of subclusters is complete we can finally
add the necessary options to create and read images with this feature,
which we call "extended L2 entries".

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c|  65 ++--
 block/qcow2.h|   8 ++-
 include/block/block_int.h|   1 +
 qapi/block-core.json |   7 +++
 tests/qemu-iotests/031.out   |   8 +--
 tests/qemu-iotests/036.out   |   4 +-
 tests/qemu-iotests/049.out   | 102 +++
 tests/qemu-iotests/060.out   |   1 +
 tests/qemu-iotests/061.out   |  20 +++---
 tests/qemu-iotests/065   |  18 --
 tests/qemu-iotests/082.out   |  48 ---
 tests/qemu-iotests/085.out   |  38 ++--
 tests/qemu-iotests/144.out   |   4 +-
 tests/qemu-iotests/182.out   |   2 +-
 tests/qemu-iotests/185.out   |   8 +--
 tests/qemu-iotests/198.out   |   2 +
 tests/qemu-iotests/206.out   |   4 ++
 tests/qemu-iotests/242.out   |   5 ++
 tests/qemu-iotests/255.out   |   8 +--
 tests/qemu-iotests/273.out   |   9 ++-
 tests/qemu-iotests/common.filter |   1 +
 21 files changed, 245 insertions(+), 118 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 0267722065..4f26953b1e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1383,6 +1383,12 @@ static int coroutine_fn qcow2_do_open(BlockDriverState 
*bs, QDict *options,
 s->subcluster_size = s->cluster_size / s->subclusters_per_cluster;
 s->subcluster_bits = ctz32(s->subcluster_size);
 
+if (s->subcluster_size < (1 << MIN_CLUSTER_BITS)) {
+error_setg(errp, "Unsupported subcluster size: %d", 
s->subcluster_size);
+ret = -EINVAL;
+goto fail;
+}
+
 /* Check support for various header values */
 if (header.refcount_order > 6) {
 error_setg(errp, "Reference count entry width too large; may not "
@@ -2856,6 +2862,11 @@ int qcow2_update_header(BlockDriverState *bs)
 .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
 .name = "lazy refcounts",
 },
+{
+.type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
+.bit  = QCOW2_INCOMPAT_EXTL2_BITNR,
+.name = "extended L2 entries",
+},
 };
 
 ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE,
@@ -3184,7 +3195,8 @@ static int64_t qcow2_calc_prealloc_size(int64_t 
total_size,
 return meta_size + aligned_total_size;
 }
 
-static bool validate_cluster_size(size_t cluster_size, Error **errp)
+static bool validate_cluster_size(size_t cluster_size, bool extended_l2,
+  Error **errp)
 {
 int cluster_bits = ctz32(cluster_size);
 if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS ||
@@ -3194,16 +3206,28 @@ static bool validate_cluster_size(size_t cluster_size, 
Error **errp)
"%dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
 return false;
 }
+
+if (extended_l2) {
+unsigned min_cluster_size =
+(1 << MIN_CLUSTER_BITS) * QCOW_MAX_SUBCLUSTERS_PER_CLUSTER;
+if (cluster_size < min_cluster_size) {
+error_setg(errp, "Extended L2 entries are only supported with "
+   "cluster sizes of at least %u bytes", min_cluster_size);
+return false;
+}
+}
+
 return true;
 }
 
-static size_t qcow2_opt_get_cluster_size_del(QemuOpts *opts, Error **errp)
+static size_t qcow2_opt_get_cluster_size_del(QemuOpts *opts, bool extended_l2,
+ Error **errp)
 {
 size_t cluster_size;
 
 cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
  DEFAULT_CLUSTER_SIZE);
-if (!validate_cluster_size(cluster_size, errp)) {
+if (!validate_cluster_size(cluster_size, extended_l2, errp)) {
 return 0;
 }
 return cluster_size;
@@ -3316,7 +3340,20 @@ qcow2_co_create(BlockdevCreateOptions *create_options, 
Error **errp)
 cluster_size = DEFAULT_CLUSTER_SIZE;
 }
 
-if (!validate_cluster_size(cluster_size, errp)) {
+if (!qcow2_opts->has_extended_l2) {
+qcow2_opts->extended_l2 = false;
+}
+if (qcow2_opts->extended_l2) {
+if (version < 3) {
+error_setg(errp, "Extended L2 entries are only supported with "
+   "compatibility level 1.1 and above (use version=v3 or "
+   "greater)");
+ret = -EINVAL;
+goto out;
+}
+}
+
+if (!validate_cluster_size(cluster_size, qcow2_opts->extended_l2, errp)) {
 ret = -EINVAL;
 goto out;
 }
@@ -3436,6 +3473,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, 
Error **errp)
 cpu_to_be64(QCOW2_AUTOCLEAR_DATA_FILE_RAW);
 }
 
+if (qcow2_opts->extended_l2) {
+hea

[RFC PATCH v3 12/27] qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*

[Alberto Garcia]

In order to support extended L2 entries some functions of the qcow2
driver need to start dealing with subclusters instead of clusters.

qcow2_get_cluster_offset() is modified to return the subcluster
type instead of the cluster type, and all callers are updated to
replace all values of QCow2ClusterType with their QCow2SubclusterType
equivalents (as returned by qcow2_cluster_to_subcluster_type()).

This patch only changes the data types, there are no semantic changes.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 19 +-
 block/qcow2.c | 82 +--
 block/qcow2.h |  3 +-
 3 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 851c7e6165..40c2e34a2a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -497,21 +497,22 @@ static int coroutine_fn 
do_perform_cow_write(BlockDriverState *bs,
 /*
  * get_cluster_offset
  *
- * For a given offset of the virtual disk, find the cluster type and offset in
- * the qcow2 file. The offset is stored in *cluster_offset.
+ * For a given offset of the virtual disk, find the cluster offset in
+ * the qcow2 file and store it in *cluster_offset.
  *
  * On entry, *bytes is the maximum number of contiguous bytes starting at
  * offset that we are interested in.
  *
  * On exit, *bytes is the number of bytes starting at offset that have the same
- * cluster type and (if applicable) are stored contiguously in the image file.
- * Compressed clusters are always returned one by one.
+ * subcluster type and (if applicable) are stored contiguously in the image
+ * file. The subcluster type is stored in *subcluster_type. Compressed clusters
+ * are always processed one by one.
  *
- * Returns the cluster type (QCOW2_CLUSTER_*) on success, -errno in error
- * cases.
+ * Returns 0 on success, -errno in error cases.
  */
 int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
- unsigned int *bytes, uint64_t *cluster_offset)
+ unsigned int *bytes, uint64_t *cluster_offset,
+ QCow2SubclusterType *subcluster_type)
 {
 BDRVQcow2State *s = bs->opaque;
 unsigned int l2_index;
@@ -653,7 +654,9 @@ out:
 assert(bytes_available - offset_in_cluster <= UINT_MAX);
 *bytes = bytes_available - offset_in_cluster;
 
-return type;
+*subcluster_type = qcow2_cluster_to_subcluster_type(type);
+
+return 0;
 
 fail:
 qcow2_cache_put(s->l2_table_cache, (void **)&l2_slice);
diff --git a/block/qcow2.c b/block/qcow2.c
index e7607d90d4..9277d680ef 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1964,6 +1964,7 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 BDRVQcow2State *s = bs->opaque;
 uint64_t cluster_offset;
 unsigned int bytes;
+QCow2SubclusterType type;
 int ret, status = 0;
 
 qemu_co_mutex_lock(&s->lock);
@@ -1975,7 +1976,7 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 }
 
 bytes = MIN(INT_MAX, count);
-ret = qcow2_get_cluster_offset(bs, offset, &bytes, &cluster_offset);
+ret = qcow2_get_cluster_offset(bs, offset, &bytes, &cluster_offset, &type);
 qemu_co_mutex_unlock(&s->lock);
 if (ret < 0) {
 return ret;
@@ -1983,15 +1984,16 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 
 *pnum = bytes;
 
-if ((ret == QCOW2_CLUSTER_NORMAL || ret == QCOW2_CLUSTER_ZERO_ALLOC) &&
-!s->crypto) {
+if ((type == QCOW2_SUBCLUSTER_NORMAL ||
+ type == QCOW2_SUBCLUSTER_ZERO_ALLOC) && !s->crypto) {
 *map = cluster_offset | offset_into_cluster(s, offset);
 *file = s->data_file->bs;
 status |= BDRV_BLOCK_OFFSET_VALID;
 }
-if (ret == QCOW2_CLUSTER_ZERO_PLAIN || ret == QCOW2_CLUSTER_ZERO_ALLOC) {
+if (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
+type == QCOW2_SUBCLUSTER_ZERO_ALLOC) {
 status |= BDRV_BLOCK_ZERO;
-} else if (ret != QCOW2_CLUSTER_UNALLOCATED) {
+} else if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN) {
 status |= BDRV_BLOCK_DATA;
 }
 if (s->metadata_preallocation && (status & BDRV_BLOCK_DATA) &&
@@ -2094,7 +2096,7 @@ typedef struct Qcow2AioTask {
 AioTask task;
 
 BlockDriverState *bs;
-QCow2ClusterType cluster_type; /* only for read */
+QCow2SubclusterType subcluster_type; /* only for read */
 uint64_t file_cluster_offset;
 uint64_t offset;
 uint64_t bytes;
@@ -2107,7 +2109,7 @@ static coroutine_fn int 
qcow2_co_preadv_task_entry(AioTask *task);
 static coroutine_fn int qcow2_add_task(BlockDriverState *bs,
AioTaskPool *pool,
AioTaskFunc func,
-   QCow2ClusterType cluster_type,
+   QCow2SubclusterType subcluster_type,
  

[RFC PATCH v3 09/27] qcow2: Add l2_entry_size()

[Alberto Garcia]

qcow2 images with subclusters have 128-bit L2 entries. The first 64
bits contain the same information as traditional images and the last
64 bits form a bitmap with the status of each individual subcluster.

Because of that we cannot assume that L2 entries are sizeof(uint64_t)
anymore. This function returns the proper value for the image.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c  | 12 ++--
 block/qcow2-refcount.c | 14 --
 block/qcow2.c  |  8 
 block/qcow2.h  |  9 +
 4 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 683c9569ad..851c7e6165 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -209,7 +209,7 @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
uint64_t l2_offset, uint64_t **l2_slice)
 {
 BDRVQcow2State *s = bs->opaque;
-int start_of_slice = sizeof(uint64_t) *
+int start_of_slice = l2_entry_size(s) *
 (offset_to_l2_index(s, offset) - offset_to_l2_slice_index(s, offset));
 
 return qcow2_cache_get(bs, s->l2_table_cache, l2_offset + start_of_slice,
@@ -277,7 +277,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index)
 
 /* allocate a new l2 entry */
 
-l2_offset = qcow2_alloc_clusters(bs, s->l2_size * sizeof(uint64_t));
+l2_offset = qcow2_alloc_clusters(bs, s->l2_size * l2_entry_size(s));
 if (l2_offset < 0) {
 ret = l2_offset;
 goto fail;
@@ -301,7 +301,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index)
 
 /* allocate a new entry in the l2 cache */
 
-slice_size2 = s->l2_slice_size * sizeof(uint64_t);
+slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
 trace_qcow2_l2_allocate_get_empty(bs, l1_index);
@@ -365,7 +365,7 @@ fail:
 }
 s->l1_table[l1_index] = old_l2_offset;
 if (l2_offset > 0) {
-qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t),
+qcow2_free_clusters(bs, l2_offset, s->l2_size * l2_entry_size(s),
 QCOW2_DISCARD_ALWAYS);
 }
 return ret;
@@ -708,7 +708,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t 
offset,
 
 /* Then decrease the refcount of the old table */
 if (l2_offset) {
-qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t),
+qcow2_free_clusters(bs, l2_offset, s->l2_size * l2_entry_size(s),
 QCOW2_DISCARD_OTHER);
 }
 
@@ -1895,7 +1895,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 int ret;
 int i, j;
 
-slice_size2 = s->l2_slice_size * sizeof(uint64_t);
+slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
 if (!is_active_l1) {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 223048569e..de85ed29a4 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1253,7 +1253,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 l2_slice = NULL;
 l1_table = NULL;
 l1_size2 = l1_size * sizeof(uint64_t);
-slice_size2 = s->l2_slice_size * sizeof(uint64_t);
+slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
 s->cache_discards = true;
@@ -1604,7 +1604,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 int i, l2_size, nb_csectors, ret;
 
 /* Read L2 table from disk */
-l2_size = s->l2_size * sizeof(uint64_t);
+l2_size = s->l2_size * l2_entry_size(s);
 l2_table = g_malloc(l2_size);
 
 ret = bdrv_pread(bs->file, l2_offset, l2_table, l2_size);
@@ -1679,15 +1679,16 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
 offset);
 if (fix & BDRV_FIX_ERRORS) {
+int idx = i * (l2_entry_size(s) / sizeof(uint64_t));
 uint64_t l2e_offset =
-l2_offset + (uint64_t)i * sizeof(uint64_t);
+l2_offset + (uint64_t)i * l2_entry_size(s);
 int ign = active ? QCOW2_OL_ACTIVE_L2 :
QCOW2_OL_INACTIVE_L2;
 
 l2_entry = QCOW_OFLAG_ZERO;
 set_l2_entry(s, l2_table, i, l2_entry);
 ret = qcow2_pre_write_overlap_check(bs, ign,
-l2e_offset, sizeof(uint64_t), false);
+l2e_offset, l2_entry_size(s), false);
 if (ret < 0) {
 fprintf(stderr, "ERROR: Overlap check failed\n");
 res->check_errors++;
@@ -1697,7 +1698,8 @@ static int check_refcou

[RFC PATCH v3 26/27] qcow2: Add subcluster support to qcow2_measure()

[Alberto Garcia]

Extended L2 entries are bigger than normal L2 entries so this has an
impact on the amount of metadata needed for a qcow2 file.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 4f26953b1e..62093de1c6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3162,28 +3162,31 @@ int64_t qcow2_refcount_metadata_size(int64_t clusters, 
size_t cluster_size,
  * @total_size: virtual disk size in bytes
  * @cluster_size: cluster size in bytes
  * @refcount_order: refcount bits power-of-2 exponent
+ * @extended_l2: true if the image has extended L2 entries
  *
  * Returns: Total number of bytes required for the fully allocated image
  * (including metadata).
  */
 static int64_t qcow2_calc_prealloc_size(int64_t total_size,
 size_t cluster_size,
-int refcount_order)
+int refcount_order,
+bool extended_l2)
 {
 int64_t meta_size = 0;
 uint64_t nl1e, nl2e;
 int64_t aligned_total_size = ROUND_UP(total_size, cluster_size);
+size_t l2e_size = extended_l2 ? L2E_SIZE_EXTENDED : L2E_SIZE_NORMAL;
 
 /* header: 1 cluster */
 meta_size += cluster_size;
 
 /* total size of L2 tables */
 nl2e = aligned_total_size / cluster_size;
-nl2e = ROUND_UP(nl2e, cluster_size / sizeof(uint64_t));
-meta_size += nl2e * sizeof(uint64_t);
+nl2e = ROUND_UP(nl2e, cluster_size / l2e_size);
+meta_size += nl2e * l2e_size;
 
 /* total size of L1 tables */
-nl1e = nl2e * sizeof(uint64_t) / cluster_size;
+nl1e = nl2e * l2e_size / cluster_size;
 nl1e = ROUND_UP(nl1e, cluster_size / sizeof(uint64_t));
 meta_size += nl1e * sizeof(uint64_t);
 
@@ -4704,6 +4707,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 bool has_backing_file;
 bool has_luks;
 bool extended_l2;
+size_t l2e_size;
 
 /* Parse image creation options */
 extended_l2 = qemu_opt_get_bool_del(opts, BLOCK_OPT_EXTL2, false);
@@ -4754,8 +4758,9 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 virtual_size = ROUND_UP(virtual_size, cluster_size);
 
 /* Check that virtual disk size is valid */
+l2e_size = extended_l2 ? L2E_SIZE_EXTENDED : L2E_SIZE_NORMAL;
 l2_tables = DIV_ROUND_UP(virtual_size / cluster_size,
- cluster_size / sizeof(uint64_t));
+ cluster_size / l2e_size);
 if (l2_tables * sizeof(uint64_t) > QCOW_MAX_L1_SIZE) {
 error_setg(&local_err, "The image size is too large "
"(try using a larger cluster size)");
@@ -4818,9 +4823,9 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 }
 
 info = g_new(BlockMeasureInfo, 1);
-info->fully_allocated =
+info->fully_allocated = luks_payload_size +
 qcow2_calc_prealloc_size(virtual_size, cluster_size,
- ctz32(refcount_bits)) + luks_payload_size;
+ ctz32(refcount_bits), extended_l2);
 
 /* Remove data clusters that are not required.  This overestimates the
  * required size because metadata needed for the fully allocated file is
-- 
2.20.1

[RFC PATCH v3 02/27] qcow2: Split cluster_needs_cow() out of count_cow_clusters()

[Alberto Garcia]

We are going to need it in other places.

Signed-off-by: Alberto Garcia 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2-cluster.c | 34 +++---
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 617618dc54..e078bddcc2 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1069,6 +1069,24 @@ static void calculate_l2_meta(BlockDriverState *bs,
 QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
 }
 
+/* Returns true if writing to a cluster requires COW */
+static bool cluster_needs_cow(BlockDriverState *bs, uint64_t l2_entry)
+{
+switch (qcow2_get_cluster_type(bs, l2_entry)) {
+case QCOW2_CLUSTER_NORMAL:
+if (l2_entry & QCOW_OFLAG_COPIED) {
+return false;
+}
+case QCOW2_CLUSTER_UNALLOCATED:
+case QCOW2_CLUSTER_COMPRESSED:
+case QCOW2_CLUSTER_ZERO_PLAIN:
+case QCOW2_CLUSTER_ZERO_ALLOC:
+return true;
+default:
+abort();
+}
+}
+
 /*
  * Returns the number of contiguous clusters that can be used for an allocating
  * write, but require COW to be performed (this includes yet unallocated space,
@@ -1081,25 +1099,11 @@ static int count_cow_clusters(BlockDriverState *bs, int 
nb_clusters,
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_slice[l2_index + i]);
-QCow2ClusterType cluster_type = qcow2_get_cluster_type(bs, l2_entry);
-
-switch(cluster_type) {
-case QCOW2_CLUSTER_NORMAL:
-if (l2_entry & QCOW_OFLAG_COPIED) {
-goto out;
-}
+if (!cluster_needs_cow(bs, l2_entry)) {
 break;
-case QCOW2_CLUSTER_UNALLOCATED:
-case QCOW2_CLUSTER_COMPRESSED:
-case QCOW2_CLUSTER_ZERO_PLAIN:
-case QCOW2_CLUSTER_ZERO_ALLOC:
-break;
-default:
-abort();
 }
 }
 
-out:
 assert(i <= nb_clusters);
 return i;
 }
-- 
2.20.1

[RFC PATCH v3 05/27] qcow2: Document the Extended L2 Entries feature

[Alberto Garcia]

Subcluster allocation in qcow2 is implemented by extending the
existing L2 table entries and adding additional information to
indicate the allocation status of each subcluster.

This patch documents the changes to the qcow2 format and how they
affect the calculation of the L2 cache size.

Signed-off-by: Alberto Garcia 
---
 docs/interop/qcow2.txt | 68 --
 docs/qcow2-cache.txt   | 19 +++-
 2 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index af5711e533..d34261f955 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -39,6 +39,9 @@ The first cluster of a qcow2 image contains the file header:
 as the maximum cluster size and won't be able to open 
images
 with larger cluster sizes.
 
+Note: if the image has Extended L2 Entries then 
cluster_bits
+must be at least 14 (i.e. 16384 byte clusters).
+
  24 - 31:   size
 Virtual disk size in bytes.
 
@@ -109,7 +112,12 @@ in the description of a field.
 An External Data File Name header extension may
 be present if this bit is set.
 
-Bits 3-63:  Reserved (set to 0)
+Bit 3:  Extended L2 Entries.  If this bit is set then
+L2 table entries use an extended format that
+allows subcluster-based allocation. See the
+Extended L2 Entries section for more details.
+
+Bits 4-63:  Reserved (set to 0)
 
  80 -  87:  compatible_features
 Bitmask of compatible features. An implementation can
@@ -437,7 +445,7 @@ cannot be relaxed without an incompatible layout change).
 Given an offset into the virtual disk, the offset into the image file can be
 obtained as follows:
 
-l2_entries = (cluster_size / sizeof(uint64_t))
+l2_entries = (cluster_size / sizeof(uint64_t))[*]
 
 l2_index = (offset / cluster_size) % l2_entries
 l1_index = (offset / cluster_size) / l2_entries
@@ -447,6 +455,8 @@ obtained as follows:
 
 return cluster_offset + (offset % cluster_size)
 
+[*] this changes if Extended L2 Entries are enabled, see next section
+
 L1 table entry:
 
 Bit  0 -  8:Reserved (set to 0)
@@ -487,7 +497,8 @@ Standard Cluster Descriptor:
 nor is data read from the backing file if the cluster is
 unallocated.
 
-With version 2, this is always 0.
+With version 2 or with extended L2 entries (see the next
+section), this is always 0.
 
  1 -  8:Reserved (set to 0)
 
@@ -524,6 +535,57 @@ file (except if bit 0 in the Standard Cluster Descriptor 
is set). If there is
 no backing file or the backing file is smaller than the image, they shall read
 zeros for all parts that are not covered by the backing file.
 
+== Extended L2 Entries ==
+
+An image uses Extended L2 Entries if bit 3 is set on the incompatible_features
+field of the header.
+
+In these images standard data clusters are divided into 32 subclusters of the
+same size. They are contiguous and start from the beginning of the cluster.
+Subclusters can be allocated independently and the L2 entry contains 
information
+indicating the status of each one of them. Compressed data clusters don't have
+subclusters so they are treated like in images without this feature.
+
+The size of an extended L2 entry is 128 bits so the number of entries per table
+is calculated using this formula:
+
+l2_entries = (cluster_size / (2 * sizeof(uint64_t)))
+
+The first 64 bits have the same format as the standard L2 table entry described
+in the previous section, with the exception of bit 0 of the standard cluster
+descriptor.
+
+The last 64 bits contain a subcluster allocation bitmap with this format:
+
+Subcluster Allocation Bitmap (for standard clusters):
+
+Bit  0 -  31:   Allocation status (one bit per subcluster)
+
+1: the subcluster is allocated. In this case the
+   host cluster offset field must contain a valid
+   offset.
+0: the subcluster is not allocated. In this case
+   read requests shall go to the backing file or
+   return zeros if there is no backing file data.
+
+Bits are assigned starting from the most significant one.
+(i.e. bit x is used for subcluster 31 - x)
+
+32 -  63Subcluster reads as zeros (one bit per subcluster)
+
+1: the subcluster reads as zeros. In this case the
+   allocation status bit must be unset. The host
+   cluster offset field may or may not be set.
+   

[RFC PATCH v3 15/27] qcow2: Add subcluster support to qcow2_get_cluster_offset()

[Alberto Garcia]

The logic of this function remains pretty much the same, except that
it uses count_contiguous_subclusters(), which combines the logic of
count_contiguous_clusters() / count_contiguous_clusters_unallocated()
and checks individual subclusters.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 136 --
 block/qcow2.h |  36 +--
 2 files changed, 80 insertions(+), 92 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index c6eb480ee8..c10601a828 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -372,66 +372,55 @@ fail:
 }
 
 /*
- * Checks how many clusters in a given L2 slice are contiguous in the image
- * file. As soon as one of the flags in the bitmask stop_flags changes compared
- * to the first cluster, the search is stopped and the cluster is not counted
- * as contiguous. (This allows it, for example, to stop at the first compressed
- * cluster which may require a different handling)
+ * Return the number of contiguous subclusters of the exact same type
+ * in a given L2 slice, starting from cluster @l2_index, subcluster
+ * @sc_index. Allocated subclusters are required to be contiguous in
+ * the image file.
+ * At most @nb_clusters are checked (note that this means clusters,
+ * not subclusters).
  */
-static int count_contiguous_clusters(BlockDriverState *bs, int nb_clusters,
-int cluster_size, uint64_t *l2_slice, int l2_index, uint64_t 
stop_flags)
+static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
+unsigned sc_index, uint64_t *l2_slice,
+int l2_index)
 {
 BDRVQcow2State *s = bs->opaque;
-int i;
-QCow2ClusterType first_cluster_type;
-uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW_OFLAG_COMPRESSED;
-uint64_t first_entry = get_l2_entry(s, l2_slice, l2_index);
-uint64_t offset = first_entry & mask;
-
-first_cluster_type = qcow2_get_cluster_type(bs, first_entry);
-if (first_cluster_type == QCOW2_CLUSTER_UNALLOCATED) {
-return 0;
+int i, j, count = 0;
+uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index);
+uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
+uint64_t expected_offset = l2_entry & L2E_OFFSET_MASK;
+bool check_offset = true;
+QCow2SubclusterType type =
+qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc_index);
+
+assert(type != QCOW2_SUBCLUSTER_INVALID); /* The caller should check this 
*/
+
+if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
+/* Compressed clusters are always processed one by one */
+return s->subclusters_per_cluster - sc_index;
 }
 
-/* must be allocated */
-assert(first_cluster_type == QCOW2_CLUSTER_NORMAL ||
-   first_cluster_type == QCOW2_CLUSTER_ZERO_ALLOC);
-
-for (i = 0; i < nb_clusters; i++) {
-uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index + i) & mask;
-if (offset + (uint64_t) i * cluster_size != l2_entry) {
-break;
-}
+if (type == QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN ||
+type == QCOW2_SUBCLUSTER_ZERO_PLAIN) {
+check_offset = false;
 }
 
-return i;
-}
-
-/*
- * Checks how many consecutive unallocated clusters in a given L2
- * slice have the same cluster type.
- */
-static int count_contiguous_clusters_unallocated(BlockDriverState *bs,
- int nb_clusters,
- uint64_t *l2_slice,
- int l2_index,
- QCow2ClusterType wanted_type)
-{
-BDRVQcow2State *s = bs->opaque;
-int i;
-
-assert(wanted_type == QCOW2_CLUSTER_ZERO_PLAIN ||
-   wanted_type == QCOW2_CLUSTER_UNALLOCATED);
 for (i = 0; i < nb_clusters; i++) {
-uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
-QCow2ClusterType type = qcow2_get_cluster_type(bs, entry);
-
-if (type != wanted_type) {
-break;
+l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
+l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+if (check_offset && expected_offset != (l2_entry & L2E_OFFSET_MASK)) {
+goto out;
+}
+for (j = (i == 0) ? sc_index : 0; j < s->subclusters_per_cluster; j++) 
{
+if (qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, j) != type) 
{
+goto out;
+}
+count++;
 }
+expected_offset += s->cluster_size;
 }
 
-return i;
+out:
+return count;
 }
 
 static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
@@ -515,12 +504,12 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
  QCow2SubclusterType *subcluster_type)
 {
 BDRVQcow2State *s = bs->opaque;
-unsigned int l

[RFC PATCH v3 06/27] qcow2: Add dummy has_subclusters() function

[Alberto Garcia]

This function will be used by the qcow2 code to check if an image has
subclusters or not.

At the moment this simply returns false. Once all patches needed for
subcluster support are ready then QEMU will be able to create and
read images with subclusters and this function will return the actual
value.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 6823d3f68f..1db3fc5dbc 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -495,6 +495,12 @@ typedef enum QCow2MetadataOverlap {
 
 #define INV_OFFSET (-1ULL)
 
+static inline bool has_subclusters(BDRVQcow2State *s)
+{
+/* FIXME: Return false until this feature is complete */
+return false;
+}
+
 static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx)
 {
-- 
2.20.1

[RFC PATCH v3 19/27] qcow2: Add subcluster support to expand_zero_clusters_in_l1()

[Alberto Garcia]

Two changes are needed in order to add subcluster support to this
function: deallocated clusters must have their bitmaps cleared, and
expanded clusters must have all the "subcluster allocated" bits set.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 207f670c94..ede75138d2 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2054,6 +2054,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 /* not backed; therefore we can simply deallocate the
  * cluster */
 set_l2_entry(s, l2_slice, j, 0);
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, j, 0);
+}
 l2_dirty = true;
 continue;
 }
@@ -2120,6 +2123,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 } else {
 set_l2_entry(s, l2_slice, j, offset);
 }
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, j, QCOW_L2_BITMAP_ALL_ALLOC);
+}
 l2_dirty = true;
 }
 
-- 
2.20.1

[RFC PATCH v3 00/27] Add subcluster allocation to qcow2

[Alberto Garcia]

Hi,

here's the new version of the patches to add subcluster allocation
support to qcow2.

Please refer to the cover letter of the first version for a full
description of the patches:

   https://lists.gnu.org/archive/html/qemu-block/2019-10/msg00983.html

This version fixes many of the problems highlighted by Max. I decided
not to replace completely the cluster logic with subcluster logic in
all cases because I felt that sometimes it only complicated the code.
Let's see what you think :-)

Berto

v3:
- Patch 01: Rename host_offset to host_cluster_offset and make 'bytes'
an unsigned int [Max]
- Patch 03: Rename cluster_needs_cow to cluster_needs_new_alloc and
count_cow_clusters to count_single_write_clusters. Update
documentation and add more assertions and checks [Max]
- Patch 09: Update qcow2_co_truncate() to properly support extended L2
entries [Max]
- Patch 10: Forbid calling set_l2_bitmap() if the image does not have
extended L2 entries [Max]
- Patch 11 (new): Add QCow2SubclusterType [Max]
- Patch 12 (new): Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*
- Patch 13 (new): Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC
- Patch 14: Use QCow2SubclusterType instead of QCow2ClusterType [Max]
- Patch 15: Use QCow2SubclusterType instead of QCow2ClusterType [Max]
- Patch 19: Don't call set_l2_bitmap() if the image does not have
extended L2 entries [Max]
- Patch 21: Use smaller data types.
- Patch 22: Don't call set_l2_bitmap() if the image does not have
extended L2 entries [Max]
- Patch 23: Use smaller data types.
- Patch 25: Update test results and documentation. Move the check for
the minimum subcluster size to validate_cluster_size().
- Patch 26 (new): Add subcluster support to qcow2_measure()
- Patch 27: Add more tests

v2: https://lists.gnu.org/archive/html/qemu-block/2019-10/msg01642.html
- Patch 12: Update after the changes in 88f468e546.
- Patch 21 (new): Clear the L2 bitmap when allocating a compressed
  cluster. Compressed clusters should have the bitmap all set to 0.
- Patch 24: Document the new fields in the QAPI documentation [Eric].
- Patch 25: Allow qcow2 preallocation with backing files.
- Patch 26: Add some tests for qcow2 images with extended L2 entries.

v1: https://lists.gnu.org/archive/html/qemu-block/2019-10/msg00983.html

Output of git backport-diff against v2:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/27:[0013] [FC] 'qcow2: Add calculate_l2_meta()'
002/27:[] [-C] 'qcow2: Split cluster_needs_cow() out of 
count_cow_clusters()'
003/27:[0083] [FC] 'qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in 
handle_copied()'
004/27:[] [-C] 'qcow2: Add get_l2_entry() and set_l2_entry()'
005/27:[] [--] 'qcow2: Document the Extended L2 Entries feature'
006/27:[] [--] 'qcow2: Add dummy has_subclusters() function'
007/27:[] [--] 'qcow2: Add subcluster-related fields to BDRVQcow2State'
008/27:[] [--] 'qcow2: Add offset_to_sc_index()'
009/27:[0008] [FC] 'qcow2: Add l2_entry_size()'
010/27:[0008] [FC] 'qcow2: Update get/set_l2_entry() and add 
get/set_l2_bitmap()'
011/27:[down] 'qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()'
012/27:[down] 'qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*'
013/27:[down] 'qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC'
014/27:[0060] [FC] 'qcow2: Add subcluster support to calculate_l2_meta()'
015/27:[0091] [FC] 'qcow2: Add subcluster support to qcow2_get_cluster_offset()'
016/27:[] [--] 'qcow2: Add subcluster support to zero_in_l2_slice()'
017/27:[] [--] 'qcow2: Add subcluster support to discard_in_l2_slice()'
018/27:[] [--] 'qcow2: Add subcluster support to check_refcounts_l2()'
019/27:[0008] [FC] 'qcow2: Add subcluster support to 
expand_zero_clusters_in_l1()'
020/27:[] [--] 'qcow2: Fix offset calculation in handle_dependencies()'
021/27:[0007] [FC] 'qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()'
022/27:[0004] [FC] 'qcow2: Clear the L2 bitmap when allocating a compressed 
cluster'
023/27:[0002] [FC] 'qcow2: Add subcluster support to handle_alloc_space()'
024/27:[] [-C] 'qcow2: Restrict qcow2_co_pwrite_zeroes() to full clusters 
only'
025/27:[0049] [FC] 'qcow2: Add the 'extended_l2' option and the 
QCOW2_INCOMPAT_EXTL2 bit'
026/27:[down] 'qcow2: Add subcluster support to qcow2_measure()'
027/27:[0046] [FC] 'iotests: Add tests for qcow2 images with extended L2 
entries'

Alberto Garcia (27):
  qcow2: Add calculate_l2_meta()
  qcow2: Split cluster_needs_cow() out of count_cow_clusters()
  qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()
  qcow2: Add get_l2_entry() and set_l2_entry()
  qcow2: Document the Extended L2 Entries feature
  qcow2: Add dummy has_subclusters() function
  qcow2: Add subcluster-r

[RFC PATCH v3 04/27] qcow2: Add get_l2_entry() and set_l2_entry()

[Alberto Garcia]

The size of an L2 entry is 64 bits, but if we want to have subclusters
we need extended L2 entries. This means that we have to access L2
tables and slices differently depending on whether an image has
extended L2 entries or not.

This patch replaces all l2_slice[] accesses with calls to
get_l2_entry() and set_l2_entry().

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c  | 65 ++
 block/qcow2-refcount.c | 17 +--
 block/qcow2.h  | 12 
 3 files changed, 55 insertions(+), 39 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9387f15866..683c9569ad 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -379,12 +379,13 @@ fail:
  * cluster which may require a different handling)
  */
 static int count_contiguous_clusters(BlockDriverState *bs, int nb_clusters,
-int cluster_size, uint64_t *l2_slice, uint64_t stop_flags)
+int cluster_size, uint64_t *l2_slice, int l2_index, uint64_t 
stop_flags)
 {
+BDRVQcow2State *s = bs->opaque;
 int i;
 QCow2ClusterType first_cluster_type;
 uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW_OFLAG_COMPRESSED;
-uint64_t first_entry = be64_to_cpu(l2_slice[0]);
+uint64_t first_entry = get_l2_entry(s, l2_slice, l2_index);
 uint64_t offset = first_entry & mask;
 
 first_cluster_type = qcow2_get_cluster_type(bs, first_entry);
@@ -397,7 +398,7 @@ static int count_contiguous_clusters(BlockDriverState *bs, 
int nb_clusters,
first_cluster_type == QCOW2_CLUSTER_ZERO_ALLOC);
 
 for (i = 0; i < nb_clusters; i++) {
-uint64_t l2_entry = be64_to_cpu(l2_slice[i]) & mask;
+uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index + i) & mask;
 if (offset + (uint64_t) i * cluster_size != l2_entry) {
 break;
 }
@@ -413,14 +414,16 @@ static int count_contiguous_clusters(BlockDriverState 
*bs, int nb_clusters,
 static int count_contiguous_clusters_unallocated(BlockDriverState *bs,
  int nb_clusters,
  uint64_t *l2_slice,
+ int l2_index,
  QCow2ClusterType wanted_type)
 {
+BDRVQcow2State *s = bs->opaque;
 int i;
 
 assert(wanted_type == QCOW2_CLUSTER_ZERO_PLAIN ||
wanted_type == QCOW2_CLUSTER_UNALLOCATED);
 for (i = 0; i < nb_clusters; i++) {
-uint64_t entry = be64_to_cpu(l2_slice[i]);
+uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
 QCow2ClusterType type = qcow2_get_cluster_type(bs, entry);
 
 if (type != wanted_type) {
@@ -566,7 +569,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 /* find the cluster offset for the given disk offset */
 
 l2_index = offset_to_l2_slice_index(s, offset);
-*cluster_offset = be64_to_cpu(l2_slice[l2_index]);
+*cluster_offset = get_l2_entry(s, l2_slice, l2_index);
 
 nb_clusters = size_to_clusters(s, bytes_needed);
 /* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
@@ -601,14 +604,14 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 case QCOW2_CLUSTER_UNALLOCATED:
 /* how many empty clusters ? */
 c = count_contiguous_clusters_unallocated(bs, nb_clusters,
-  &l2_slice[l2_index], type);
+  l2_slice, l2_index, type);
 *cluster_offset = 0;
 break;
 case QCOW2_CLUSTER_ZERO_ALLOC:
 case QCOW2_CLUSTER_NORMAL:
 /* how many allocated clusters ? */
 c = count_contiguous_clusters(bs, nb_clusters, s->cluster_size,
-  &l2_slice[l2_index], QCOW_OFLAG_ZERO);
+  l2_slice, l2_index, QCOW_OFLAG_ZERO);
 *cluster_offset &= L2E_OFFSET_MASK;
 if (offset_into_cluster(s, *cluster_offset)) {
 qcow2_signal_corruption(bs, true, -1, -1,
@@ -761,7 +764,7 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState 
*bs,
 
 /* Compression can't overwrite anything. Fail if the cluster was already
  * allocated. */
-cluster_offset = be64_to_cpu(l2_slice[l2_index]);
+cluster_offset = get_l2_entry(s, l2_slice, l2_index);
 if (cluster_offset & L2E_OFFSET_MASK) {
 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
 return -EIO;
@@ -786,7 +789,7 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState 
*bs,
 
 BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE_COMPRESSED);
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
-l2_slice[l2_index] = cpu_to_be64(cluster_offset);
+set_l2_entry(s, l2_slice, l2_index, cluster_offset);
 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
 
 *host_offset = cluster_offset & s->c

[RFC PATCH v3 13/27] qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC

[Alberto Garcia]

When dealing with subcluster types there is a new value called
QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC that has no equivalent in
QCow2ClusterType.

This patch handles that value in all places where subcluster types
are processed.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 9277d680ef..1d3da0ccf6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1985,7 +1985,8 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 *pnum = bytes;
 
 if ((type == QCOW2_SUBCLUSTER_NORMAL ||
- type == QCOW2_SUBCLUSTER_ZERO_ALLOC) && !s->crypto) {
+ type == QCOW2_SUBCLUSTER_ZERO_ALLOC ||
+ type == QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC) && !s->crypto) {
 *map = cluster_offset | offset_into_cluster(s, offset);
 *file = s->data_file->bs;
 status |= BDRV_BLOCK_OFFSET_VALID;
@@ -1993,7 +1994,8 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 if (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
 type == QCOW2_SUBCLUSTER_ZERO_ALLOC) {
 status |= BDRV_BLOCK_ZERO;
-} else if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN) {
+} else if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN &&
+   type != QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC) {
 status |= BDRV_BLOCK_DATA;
 }
 if (s->metadata_preallocation && (status & BDRV_BLOCK_DATA) &&
@@ -2163,6 +2165,7 @@ static coroutine_fn int 
qcow2_co_preadv_task(BlockDriverState *bs,
 g_assert_not_reached();
 
 case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
+case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
 assert(bs->backing); /* otherwise handled in qcow2_co_preadv_part */
 
 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
@@ -2236,7 +2239,8 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 
 if (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
 type == QCOW2_SUBCLUSTER_ZERO_ALLOC ||
-(type == QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN && !bs->backing))
+(type == QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN && !bs->backing) ||
+(type == QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC && !bs->backing))
 {
 qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
 } else {
@@ -3750,6 +3754,7 @@ static coroutine_fn int 
qcow2_co_pwrite_zeroes(BlockDriverState *bs,
 return -ENOTSUP;
 }
 if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN &&
+type != QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC &&
 type != QCOW2_SUBCLUSTER_ZERO_PLAIN &&
 type != QCOW2_SUBCLUSTER_ZERO_ALLOC) {
 qemu_co_mutex_unlock(&s->lock);
@@ -3822,6 +3827,7 @@ qcow2_co_copy_range_from(BlockDriverState *bs,
 
 switch (type) {
 case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
+case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
 if (bs->backing && bs->backing->bs) {
 int64_t backing_length = bdrv_getlength(bs->backing->bs);
 if (src_offset >= backing_length) {
-- 
2.20.1

[RFC PATCH v3 18/27] qcow2: Add subcluster support to check_refcounts_l2()

[Alberto Garcia]

Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
image has subclusters. Instead, the individual 'all zeroes' bits must
be used.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-refcount.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index de85ed29a4..65f4fc27c3 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1685,8 +1685,13 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 int ign = active ? QCOW2_OL_ACTIVE_L2 :
QCOW2_OL_INACTIVE_L2;
 
-l2_entry = QCOW_OFLAG_ZERO;
-set_l2_entry(s, l2_table, i, l2_entry);
+if (has_subclusters(s)) {
+set_l2_entry(s, l2_table, i, 0);
+set_l2_bitmap(s, l2_table, i,
+  QCOW_L2_BITMAP_ALL_ZEROES);
+} else {
+set_l2_entry(s, l2_table, i, QCOW_OFLAG_ZERO);
+}
 ret = qcow2_pre_write_overlap_check(bs, ign,
 l2e_offset, l2_entry_size(s), false);
 if (ret < 0) {
-- 
2.20.1

[RFC PATCH v3 14/27] qcow2: Add subcluster support to calculate_l2_meta()

[Alberto Garcia]

If an image has subclusters then there are more copy-on-write
scenarios that we need to consider. Let's say we have a write request
from the middle of subcluster #3 until the end of the cluster:

   - If the cluster is new, then subclusters #0 to #3 from the old
 cluster must be copied into the new one.

   - If the cluster is new but the old cluster was unallocated, then
 only subcluster #3 needs copy-on-write. #0 to #2 are marked as
 unallocated in the bitmap of the new L2 entry.

   - If we are overwriting an old cluster and subcluster #3 is
 unallocated or has the all-zeroes bit set then we need
 copy-on-write on subcluster #3.

   - If we are overwriting an old cluster and subcluster #3 was
 allocated then there is no need to copy-on-write.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 140 +-
 1 file changed, 110 insertions(+), 30 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 40c2e34a2a..c6eb480ee8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1041,56 +1041,128 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
  * If @keep_old is true it means that the clusters were already
  * allocated and will be overwritten. If false then the clusters are
  * new and we have to decrease the reference count of the old ones.
+ *
+ * Returns 1 on success, -errno on failure (in order to match the
+ * return value of handle_copied() and handle_alloc()).
  */
-static void calculate_l2_meta(BlockDriverState *bs,
-  uint64_t host_cluster_offset,
-  uint64_t guest_offset, unsigned bytes,
-  uint64_t *l2_slice, QCowL2Meta **m, bool 
keep_old)
+static int calculate_l2_meta(BlockDriverState *bs, uint64_t 
host_cluster_offset,
+ uint64_t guest_offset, unsigned bytes,
+ uint64_t *l2_slice, QCowL2Meta **m, bool keep_old)
 {
 BDRVQcow2State *s = bs->opaque;
-int l2_index = offset_to_l2_slice_index(s, guest_offset);
-uint64_t l2_entry;
+int sc_index, l2_index = offset_to_l2_slice_index(s, guest_offset);
+uint64_t l2_entry, l2_bitmap;
 unsigned cow_start_from, cow_end_to;
 unsigned cow_start_to = offset_into_cluster(s, guest_offset);
 unsigned cow_end_from = cow_start_to + bytes;
 unsigned nb_clusters = size_to_clusters(s, cow_end_from);
 QCowL2Meta *old_m = *m;
-QCow2ClusterType type;
+QCow2SubclusterType type;
 
 assert(nb_clusters <= s->l2_slice_size - l2_index);
 
-/* Return if there's no COW (all clusters are normal and we keep them) */
+/* Return if there's no COW (all subclusters are normal and we are
+ * keeping the clusters) */
 if (keep_old) {
+unsigned first_sc = cow_start_to / s->subcluster_size;
+unsigned last_sc = (cow_end_from - 1) / s->subcluster_size;
 int i;
-for (i = 0; i < nb_clusters; i++) {
-l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
-if (qcow2_get_cluster_type(bs, l2_entry) != QCOW2_CLUSTER_NORMAL) {
+for (i = first_sc; i <= last_sc; i++) {
+unsigned c = i / s->subclusters_per_cluster;
+unsigned sc = i % s->subclusters_per_cluster;
+l2_entry = get_l2_entry(s, l2_slice, l2_index + c);
+l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + c);
+type = qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc);
+if (type == QCOW2_SUBCLUSTER_INVALID) {
+l2_index += c; /* Point to the invalid entry */
+goto fail;
+}
+if (type != QCOW2_SUBCLUSTER_NORMAL) {
 break;
 }
 }
-if (i == nb_clusters) {
-return;
+if (i == last_sc + 1) {
+return 1;
 }
 }
 
 /* Get the L2 entry from the first cluster */
 l2_entry = get_l2_entry(s, l2_slice, l2_index);
-type = qcow2_get_cluster_type(bs, l2_entry);
+l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
+sc_index = offset_to_sc_index(s, guest_offset);
+type = qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc_index);
 
-if (type == QCOW2_CLUSTER_NORMAL && keep_old) {
-cow_start_from = cow_start_to;
+if (type == QCOW2_SUBCLUSTER_INVALID) {
+goto fail;
+}
+
+if (!keep_old) {
+switch (type) {
+case QCOW2_SUBCLUSTER_NORMAL:
+case QCOW2_SUBCLUSTER_COMPRESSED:
+case QCOW2_SUBCLUSTER_ZERO_ALLOC:
+case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
+cow_start_from = 0;
+break;
+case QCOW2_SUBCLUSTER_ZERO_PLAIN:
+case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
+cow_start_from = sc_index << s->subcluster_bits;
+break;
+default:
+g_assert_not_reached();
+}
 } else {
-cow_start_fro

[RFC PATCH v3 22/27] qcow2: Clear the L2 bitmap when allocating a compressed cluster

[Alberto Garcia]

Compressed clusters always have the bitmap part of the extended L2
entry set to 0.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ed291a4042..af0f01621c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -789,6 +789,9 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState 
*bs,
 BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE_COMPRESSED);
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
 set_l2_entry(s, l2_slice, l2_index, cluster_offset);
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, l2_index, 0);
+}
 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
 
 *host_offset = cluster_offset & s->cluster_offset_mask;
-- 
2.20.1

[RFC PATCH v3 17/27] qcow2: Add subcluster support to discard_in_l2_slice()

[Alberto Garcia]

Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
image has subclusters. Instead, the individual 'all zeroes' bits must
be used.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 70b0aaa00a..207f670c94 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1790,7 +1790,11 @@ static int discard_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 
 /* First remove L2 entries */
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
-if (!full_discard && s->qcow_version >= 3) {
+if (has_subclusters(s)) {
+set_l2_entry(s, l2_slice, l2_index + i, 0);
+set_l2_bitmap(s, l2_slice, l2_index + i,
+  full_discard ? 0 : QCOW_L2_BITMAP_ALL_ZEROES);
+} else if (!full_discard && s->qcow_version >= 3) {
 set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
 } else {
 set_l2_entry(s, l2_slice, l2_index + i, 0);
-- 
2.20.1

[RFC PATCH v3 23/27] qcow2: Add subcluster support to handle_alloc_space()

[Alberto Garcia]

The bdrv_co_pwrite_zeroes() call here fills complete clusters with
zeroes, but it can happen that some subclusters are not part of the
write request or the copy-on-write. This patch makes sure that only
the affected subclusters are overwritten.

A potential improvement would be to also fill with zeroes the other
subclusters if we can guarantee that we are not overwriting existing
data. However this would waste more disk space, so we should first
evaluate if it's really worth doing.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 1d3da0ccf6..242001afa2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2354,6 +2354,9 @@ static int handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
 
 for (m = l2meta; m != NULL; m = m->next) {
 int ret;
+uint64_t start_offset = m->alloc_offset + m->cow_start.offset;
+unsigned nb_bytes = m->cow_end.offset + m->cow_end.nb_bytes -
+m->cow_start.offset;
 
 if (!m->cow_start.nb_bytes && !m->cow_end.nb_bytes) {
 continue;
@@ -2368,16 +2371,14 @@ static int handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
  * efficiently zero out the whole clusters
  */
 
-ret = qcow2_pre_write_overlap_check(bs, 0, m->alloc_offset,
-m->nb_clusters * s->cluster_size,
+ret = qcow2_pre_write_overlap_check(bs, 0, start_offset, nb_bytes,
 true);
 if (ret < 0) {
 return ret;
 }
 
 BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_SPACE);
-ret = bdrv_co_pwrite_zeroes(s->data_file, m->alloc_offset,
-m->nb_clusters * s->cluster_size,
+ret = bdrv_co_pwrite_zeroes(s->data_file, start_offset, nb_bytes,
 BDRV_REQ_NO_FALLBACK);
 if (ret < 0) {
 if (ret != -ENOTSUP && ret != -EAGAIN) {
-- 
2.20.1

[RFC PATCH v3 07/27] qcow2: Add subcluster-related fields to BDRVQcow2State

[Alberto Garcia]

This patch adds the following new fields to BDRVQcow2State:

- subclusters_per_cluster: Number of subclusters in a cluster
- subcluster_size: The size of each subcluster, in bytes
- subcluster_bits: No. of bits so 1 << subcluster_bits = subcluster_size

Images without subclusters are treated as if they had exactly one,
with subcluster_size = cluster_size.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c | 5 +
 block/qcow2.h | 5 +
 2 files changed, 10 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 3866b47946..cbd857e9c7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1378,6 +1378,11 @@ static int coroutine_fn qcow2_do_open(BlockDriverState 
*bs, QDict *options,
 }
 }
 
+s->subclusters_per_cluster =
+has_subclusters(s) ? QCOW_MAX_SUBCLUSTERS_PER_CLUSTER : 1;
+s->subcluster_size = s->cluster_size / s->subclusters_per_cluster;
+s->subcluster_bits = ctz32(s->subcluster_size);
+
 /* Check support for various header values */
 if (header.refcount_order > 6) {
 error_setg(errp, "Reference count entry width too large; may not "
diff --git a/block/qcow2.h b/block/qcow2.h
index 1db3fc5dbc..941330cfc9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -78,6 +78,8 @@
 /* The cluster reads as all zeros */
 #define QCOW_OFLAG_ZERO (1ULL << 0)
 
+#define QCOW_MAX_SUBCLUSTERS_PER_CLUSTER 32
+
 #define MIN_CLUSTER_BITS 9
 #define MAX_CLUSTER_BITS 21
 
@@ -284,6 +286,9 @@ typedef struct BDRVQcow2State {
 int cluster_bits;
 int cluster_size;
 int l2_slice_size;
+int subcluster_bits;
+int subcluster_size;
+int subclusters_per_cluster;
 int l2_bits;
 int l2_size;
 int l1_size;
-- 
2.20.1

[RFC PATCH v3 27/27] iotests: Add tests for qcow2 images with extended L2 entries

[Alberto Garcia]

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/271 | 256 +
 tests/qemu-iotests/271.out | 208 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 465 insertions(+)
 create mode 100755 tests/qemu-iotests/271
 create mode 100644 tests/qemu-iotests/271.out

diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
new file mode 100755
index 00..73cdc37bf0
--- /dev/null
+++ b/tests/qemu-iotests/271
@@ -0,0 +1,256 @@
+#!/bin/bash
+#
+# Test qcow2 images with extended L2 entries
+#
+# Copyright (C) 2019 Igalia, S.L.
+# Author: Alberto Garcia 
+#
+# 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=be...@igalia.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+rm -f "$TEST_IMG.raw"
+rm -f "$TEST_IMG.backing"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file nfs
+_supported_os Linux
+
+IMGOPTS="extended_l2=on"
+l2_offset=262144 # 0x4
+
+_verify_img()
+{
+$QEMU_IMG compare "$TEST_IMG" "$TEST_IMG.raw" | grep -v 'Images are 
identical'
+$QEMU_IMG check "$TEST_IMG" | _filter_qemu_img_check | \
+grep -v 'No errors were found on the image'
+}
+
+_read_l2_entry()
+{
+entry_no=$1
+nentries=$2
+offset=$(($l2_offset + $entry_no * 16))
+length=$((nentries * 16))
+$QEMU_IO -f raw -c "read -v $offset $length" "$TEST_IMG" | _filter_qemu_io 
| head -n -2
+}
+
+_test_write()
+{
+cmd="$1"
+l2_entry_idx="$2"
+l2_entry_num="$3"
+raw_cmd=`echo $1 | sed s/-c//` # Raw images don't support -c
+echo "$cmd"
+$QEMU_IO -c "$cmd" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "$raw_cmd" -f raw "$TEST_IMG.raw" | _filter_qemu_io
+_verify_img
+if [ -n "$l2_entry_idx" ]; then
+_read_l2_entry "$l2_entry_idx" "$l2_entry_num"
+fi
+}
+
+_reset_img()
+{
+$QEMU_IMG create -f raw "$TEST_IMG.raw" 1M | _filter_img_create
+if [ "$use_backing_file" = "yes" ]; then
+$QEMU_IMG create -f raw "$TEST_IMG.backing" 1M | _filter_img_create
+$QEMU_IO -c 'write -q -P 0xFF 0 1M' -f raw "$TEST_IMG.backing" | 
_filter_qemu_io
+$QEMU_IO -c 'write -q -P 0xFF 0 1M' -f raw "$TEST_IMG.raw" | 
_filter_qemu_io
+_make_test_img -b "$TEST_IMG.backing" 1M
+else
+_make_test_img 1M
+fi
+}
+
+# Test that writing to an image with subclusters produces the expected
+# results, in images with and without backing files
+for use_backing_file in yes no; do
+echo
+echo "### Standard write tests (backing file: $use_backing_file) ###"
+echo
+_reset_img
+### Write subcluster #0 (beginning of subcluster) ###
+_test_write 'write -q -P 1 0 1k' 0 1
+
+### Write subcluster #1 (middle of subcluster) ###
+_test_write 'write -q -P 2 3k 512' 0 1
+
+### Write subcluster #2 (end of subcluster) ###
+_test_write 'write -q -P 3 5k 1k' 0 1
+
+### Write subcluster #3 (full subcluster) ###
+_test_write 'write -q -P 4 6k 2k' 0 1
+
+### Write subclusters #4-6 (full subclusters) ###
+_test_write 'write -q -P 5 8k 6k' 0 1
+
+### Write subclusters #7-9 (partial subclusters) ###
+_test_write 'write -q -P 6 15k 4k' 0 1
+
+### Write subcluster #16 (partial subcluster) ###
+_test_write 'write -q -P 7 32k 1k' 0 2
+
+### Write subcluster #31-#34 (cluster overlap) ###
+_test_write 'write -q -P 8 63k 4k' 0 2
+
+### Zero subcluster #1 (TODO: use the "all zeros" bit)
+_test_write 'write -q -z 2k 2k' 0 1
+
+### Zero cluster #0
+_test_write 'write -q -z 0 64k' 0 1
+
+### Fill cluster #0 with data
+_test_write 'write -q -P 9 0 64k' 0 1
+
+### Zero and unmap half of cluster #0 (this won't unmap it)
+_test_write 'write -q -z -u 0 32k' 0 1
+
+### Zero and unmap cluster #0
+_test_write 'write -q -z -u 0 64k' 0 1
+
+### Write subcluster #2 (middle of subcluster)
+_test_write 'write -q -P 10 3k 512' 0 1
+
+### Fill cluster #0 with data
+_test_write 'write -q -P 11 0 64k' 0 1
+
+### Discard cluster #0
+_test_write 'discard -q 0 64k' 0 1
+
+### Write compressed data to cluster #0
+_test_write 'write -q -c

[RFC PATCH v3 20/27] qcow2: Fix offset calculation in handle_dependencies()

[Alberto Garcia]

l2meta_cow_start() and l2meta_cow_end() are not necessarily
cluster-aligned if the image has subclusters, so update the
calculation of old_start and old_end to guarantee that no two requests
try to write on the same cluster.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ede75138d2..0a40944667 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1279,8 +1279,8 @@ static int handle_dependencies(BlockDriverState *bs, 
uint64_t guest_offset,
 
 uint64_t start = guest_offset;
 uint64_t end = start + bytes;
-uint64_t old_start = l2meta_cow_start(old_alloc);
-uint64_t old_end = l2meta_cow_end(old_alloc);
+uint64_t old_start = start_of_cluster(s, l2meta_cow_start(old_alloc));
+uint64_t old_end = ROUND_UP(l2meta_cow_end(old_alloc), 
s->cluster_size);
 
 if (end <= old_start || start >= old_end) {
 /* No intersection */
-- 
2.20.1

[RFC PATCH v3 21/27] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

[Alberto Garcia]

The L2 bitmap needs to be updated after each write to indicate what
new subclusters are now allocated.

This needs to happen even if the cluster was already allocated and the
L2 entry was otherwise valid.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0a40944667..ed291a4042 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -986,6 +986,23 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
QCowL2Meta *m)
 
 set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_COPIED |
  (cluster_offset + (i << s->cluster_bits)));
+
+/* Update bitmap with the subclusters that were just written */
+if (has_subclusters(s)) {
+unsigned written_from = m->cow_start.offset;
+unsigned written_to = m->cow_end.offset + m->cow_end.nb_bytes ?:
+m->nb_clusters << s->cluster_bits;
+uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+int sc;
+for (sc = 0; sc < s->subclusters_per_cluster; sc++) {
+int sc_off = i * s->cluster_size + sc * s->subcluster_size;
+if (sc_off >= written_from && sc_off < written_to) {
+l2_bitmap |= QCOW_OFLAG_SUB_ALLOC(sc);
+l2_bitmap &= ~QCOW_OFLAG_SUB_ZERO(sc);
+}
+}
+set_l2_bitmap(s, l2_slice, l2_index + i, l2_bitmap);
+}
  }
 
 
-- 
2.20.1

[RFC PATCH v3 08/27] qcow2: Add offset_to_sc_index()

[Alberto Garcia]

For a given offset, return the subcluster number within its cluster
(i.e. with 32 subclusters per cluster it returns a number between 0
and 31).

Signed-off-by: Alberto Garcia 
---
 block/qcow2.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 941330cfc9..523bc489a5 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -566,6 +566,11 @@ static inline int offset_to_l2_slice_index(BDRVQcow2State 
*s, int64_t offset)
 return (offset >> s->cluster_bits) & (s->l2_slice_size - 1);
 }
 
+static inline int offset_to_sc_index(BDRVQcow2State *s, int64_t offset)
+{
+return (offset >> s->subcluster_bits) & (s->subclusters_per_cluster - 1);
+}
+
 static inline int64_t qcow2_vm_state_offset(BDRVQcow2State *s)
 {
 return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
-- 
2.20.1

[RFC PATCH v3 10/27] qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap()

[Alberto Garcia]

Extended L2 entries are 128-bit wide: 64 bits for the entry itself and
64 bits for the subcluster allocation bitmap.

In order to support them correctly get/set_l2_entry() need to be
updated so they take the entry width into account in order to
calculate the correct offset.

This patch also adds the get/set_l2_bitmap() functions that are
used to access the bitmaps. For convenience we allow calling
get_l2_bitmap() on images without subclusters, although the caller
does not need and should ignore the returned value.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.h | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 8be020bb76..64b0a814f4 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -518,15 +518,37 @@ static inline size_t l2_entry_size(BDRVQcow2State *s)
 static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx)
 {
+idx *= l2_entry_size(s) / sizeof(uint64_t);
 return be64_to_cpu(l2_slice[idx]);
 }
 
+static inline uint64_t get_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice,
+ int idx)
+{
+if (has_subclusters(s)) {
+idx *= l2_entry_size(s) / sizeof(uint64_t);
+return be64_to_cpu(l2_slice[idx + 1]);
+} else {
+/* For convenience only; the caller should ignore this value. */
+return 0;
+}
+}
+
 static inline void set_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx, uint64_t entry)
 {
+idx *= l2_entry_size(s) / sizeof(uint64_t);
 l2_slice[idx] = cpu_to_be64(entry);
 }
 
+static inline void set_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice,
+ int idx, uint64_t bitmap)
+{
+assert(has_subclusters(s));
+idx *= l2_entry_size(s) / sizeof(uint64_t);
+l2_slice[idx + 1] = cpu_to_be64(bitmap);
+}
+
 static inline bool has_data_file(BlockDriverState *bs)
 {
 BDRVQcow2State *s = bs->opaque;
-- 
2.20.1

[RFC PATCH v3 01/27] qcow2: Add calculate_l2_meta()

[Alberto Garcia]

handle_alloc() creates a QCowL2Meta structure in order to update the
image metadata and perform the necessary copy-on-write operations.

This patch moves that code to a separate function so it can be used
from other places.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 77 +--
 1 file changed, 53 insertions(+), 24 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 8982b7b762..617618dc54 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1019,6 +1019,56 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
 QCOW2_DISCARD_NEVER);
 }
 
+/*
+ * For a given write request, create a new QCowL2Meta structure, add
+ * it to @m and the BDRVQcow2State.cluster_allocs list.
+ *
+ * @host_cluster_offset points to the beginning of the first cluster.
+ *
+ * @guest_offset and @bytes indicate the offset and length of the
+ * request.
+ *
+ * If @keep_old is true it means that the clusters were already
+ * allocated and will be overwritten. If false then the clusters are
+ * new and we have to decrease the reference count of the old ones.
+ */
+static void calculate_l2_meta(BlockDriverState *bs,
+  uint64_t host_cluster_offset,
+  uint64_t guest_offset, unsigned bytes,
+  QCowL2Meta **m, bool keep_old)
+{
+BDRVQcow2State *s = bs->opaque;
+unsigned cow_start_from = 0;
+unsigned cow_start_to = offset_into_cluster(s, guest_offset);
+unsigned cow_end_from = cow_start_to + bytes;
+unsigned cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
+unsigned nb_clusters = size_to_clusters(s, cow_end_from);
+QCowL2Meta *old_m = *m;
+
+*m = g_malloc0(sizeof(**m));
+**m = (QCowL2Meta) {
+.next   = old_m,
+
+.alloc_offset   = host_cluster_offset,
+.offset = start_of_cluster(s, guest_offset),
+.nb_clusters= nb_clusters,
+
+.keep_old_clusters = keep_old,
+
+.cow_start = {
+.offset = cow_start_from,
+.nb_bytes   = cow_start_to - cow_start_from,
+},
+.cow_end = {
+.offset = cow_end_from,
+.nb_bytes   = cow_end_to - cow_end_from,
+},
+};
+
+qemu_co_queue_init(&(*m)->dependent_requests);
+QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
+}
+
 /*
  * Returns the number of contiguous clusters that can be used for an allocating
  * write, but require COW to be performed (this includes yet unallocated space,
@@ -1417,35 +1467,14 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 uint64_t requested_bytes = *bytes + offset_into_cluster(s, guest_offset);
 int avail_bytes = nb_clusters << s->cluster_bits;
 int nb_bytes = MIN(requested_bytes, avail_bytes);
-QCowL2Meta *old_m = *m;
-
-*m = g_malloc0(sizeof(**m));
-
-**m = (QCowL2Meta) {
-.next   = old_m,
-
-.alloc_offset   = alloc_cluster_offset,
-.offset = start_of_cluster(s, guest_offset),
-.nb_clusters= nb_clusters,
-
-.keep_old_clusters  = keep_old_clusters,
-
-.cow_start = {
-.offset = 0,
-.nb_bytes   = offset_into_cluster(s, guest_offset),
-},
-.cow_end = {
-.offset = nb_bytes,
-.nb_bytes   = avail_bytes - nb_bytes,
-},
-};
-qemu_co_queue_init(&(*m)->dependent_requests);
-QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
 
 *host_offset = alloc_cluster_offset + offset_into_cluster(s, guest_offset);
 *bytes = MIN(*bytes, nb_bytes - offset_into_cluster(s, guest_offset));
 assert(*bytes != 0);
 
+calculate_l2_meta(bs, alloc_cluster_offset, guest_offset, *bytes,
+  m, keep_old_clusters);
+
 return 1;
 
 fail:
-- 
2.20.1

[RFC PATCH v3 03/27] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()

[Alberto Garcia]

When writing to a qcow2 file there are two functions that take a
virtual offset and return a host offset, possibly allocating new
clusters if necessary:

   - handle_copied() looks for normal data clusters that are already
 allocated and have a reference count of 1. In those clusters we
 can simply write the data and there is no need to perform any
 copy-on-write.

   - handle_alloc() looks for clusters that do need copy-on-write,
 either because they haven't been allocated yet, because their
 reference count is != 1 or because they are ZERO_ALLOC clusters.

The ZERO_ALLOC case is a bit special because those are clusters that
are already allocated and they could perfectly be dealt with in
handle_copied() (as long as copy-on-write is performed when required).

In fact, there is extra code specifically for them in handle_alloc()
that tries to reuse the existing allocation if possible and frees them
otherwise.

This patch changes the handling of ZERO_ALLOC clusters so the
semantics of these two functions are now like this:

   - handle_copied() looks for clusters that are already allocated and
 which we can overwrite (NORMAL and ZERO_ALLOC clusters with a
 reference count of 1).

   - handle_alloc() looks for clusters for which we need a new
 allocation (all other cases).

One importante difference after this change is that clusters found in
handle_copied() may now require copy-on-write, but this will be anyway
necessary once we add support for subclusters.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 226 +++---
 1 file changed, 126 insertions(+), 100 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index e078bddcc2..9387f15866 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1021,13 +1021,18 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
 
 /*
  * For a given write request, create a new QCowL2Meta structure, add
- * it to @m and the BDRVQcow2State.cluster_allocs list.
+ * it to @m and the BDRVQcow2State.cluster_allocs list. If the write
+ * request does not need copy-on-write or changes to the L2 metadata
+ * then this function does nothing.
  *
  * @host_cluster_offset points to the beginning of the first cluster.
  *
  * @guest_offset and @bytes indicate the offset and length of the
  * request.
  *
+ * @l2_slice contains the L2 entries of all clusters involved in this
+ * write request.
+ *
  * If @keep_old is true it means that the clusters were already
  * allocated and will be overwritten. If false then the clusters are
  * new and we have to decrease the reference count of the old ones.
@@ -1035,15 +1040,53 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
 static void calculate_l2_meta(BlockDriverState *bs,
   uint64_t host_cluster_offset,
   uint64_t guest_offset, unsigned bytes,
-  QCowL2Meta **m, bool keep_old)
+  uint64_t *l2_slice, QCowL2Meta **m, bool 
keep_old)
 {
 BDRVQcow2State *s = bs->opaque;
-unsigned cow_start_from = 0;
+int l2_index = offset_to_l2_slice_index(s, guest_offset);
+uint64_t l2_entry;
+unsigned cow_start_from, cow_end_to;
 unsigned cow_start_to = offset_into_cluster(s, guest_offset);
 unsigned cow_end_from = cow_start_to + bytes;
-unsigned cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
 unsigned nb_clusters = size_to_clusters(s, cow_end_from);
 QCowL2Meta *old_m = *m;
+QCow2ClusterType type;
+
+assert(nb_clusters <= s->l2_slice_size - l2_index);
+
+/* Return if there's no COW (all clusters are normal and we keep them) */
+if (keep_old) {
+int i;
+for (i = 0; i < nb_clusters; i++) {
+l2_entry = be64_to_cpu(l2_slice[l2_index + i]);
+if (qcow2_get_cluster_type(bs, l2_entry) != QCOW2_CLUSTER_NORMAL) {
+break;
+}
+}
+if (i == nb_clusters) {
+return;
+}
+}
+
+/* Get the L2 entry from the first cluster */
+l2_entry = be64_to_cpu(l2_slice[l2_index]);
+type = qcow2_get_cluster_type(bs, l2_entry);
+
+if (type == QCOW2_CLUSTER_NORMAL && keep_old) {
+cow_start_from = cow_start_to;
+} else {
+cow_start_from = 0;
+}
+
+/* Get the L2 entry from the last cluster */
+l2_entry = be64_to_cpu(l2_slice[l2_index + nb_clusters - 1]);
+type = qcow2_get_cluster_type(bs, l2_entry);
+
+if (type == QCOW2_CLUSTER_NORMAL && keep_old) {
+cow_end_to = cow_end_from;
+} else {
+cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
+}
 
 *m = g_malloc0(sizeof(**m));
 **m = (QCowL2Meta) {
@@ -1069,18 +1112,20 @@ static void calculate_l2_meta(BlockDriverState *bs,
 QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
 }
 
-/* Returns true if writing to a cluster requi

[RFC PATCH v3 16/27] qcow2: Add subcluster support to zero_in_l2_slice()

[Alberto Garcia]

Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
image has subclusters. Instead, the individual 'all zeroes' bits must
be used.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index c10601a828..70b0aaa00a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1870,7 +1870,7 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 assert(nb_clusters <= INT_MAX);
 
 for (i = 0; i < nb_clusters; i++) {
-uint64_t old_offset;
+uint64_t old_offset, l2_entry = 0;
 QCow2ClusterType cluster_type;
 
 old_offset = get_l2_entry(s, l2_slice, l2_index + i);
@@ -1887,12 +1887,18 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
 if (cluster_type == QCOW2_CLUSTER_COMPRESSED || unmap) {
-set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
 qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
 } else {
-uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
-set_l2_entry(s, l2_slice, l2_index + i, entry | QCOW_OFLAG_ZERO);
+l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
 }
+
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, l2_index + i, 
QCOW_L2_BITMAP_ALL_ZEROES);
+} else {
+l2_entry |= QCOW_OFLAG_ZERO;
+}
+
+set_l2_entry(s, l2_slice, l2_index + i, l2_entry);
 }
 
 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
-- 
2.20.1

[RFC PATCH v3 11/27] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()

[Alberto Garcia]

This patch adds QCow2SubclusterType, which is the subcluster-level
version of QCow2ClusterType. All QCOW2_SUBCLUSTER_* values have the
the same meaning as their QCOW2_CLUSTER_* equivalents (when they
exist). See below for details and caveats.

In images without extended L2 entries clusters are treated as having
exactly one subcluster so it is possible to replace one data type with
the other while keeping the exact same semantics.

With extended L2 entries there are new possible values, and every
subcluster in the same cluster can obviously have a different
QCow2SubclusterType so functions need to be adapted to work on the
subcluster level.

There are several things that have to be taken into account:

  a) QCOW2_SUBCLUSTER_COMPRESSED means that the whole cluster is
 compressed. We do not support compression at the subcluster
 level.

  b) There are two different values for unallocated subclusters:
 QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN which means that the whole
 cluster is unallocated, and QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC
 which means that the cluster is allocated but the subcluster is
 not. The latter can only happen in images with extended L2
 entries.

  c) QCOW2_SUBCLUSTER_INVALID is used to detect the cases where an L2
 entry has a value that violates the specification. The caller is
 responsible for handling these situations.

 To prevent compatibility problems with images that have invalid
 values but are currently being read by QEMU without causing side
 effects, QCOW2_SUBCLUSTER_INVALID is only returned for images
 with extended L2 entries.

qcow2_cluster_to_subcluster_type() is added as a separate function
from qcow2_get_subcluster_type(), but this is only temporary and both
will be merged in a subsequent patch.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.h | 92 +++
 1 file changed, 92 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 64b0a814f4..321ba9550f 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -80,6 +80,15 @@
 
 #define QCOW_MAX_SUBCLUSTERS_PER_CLUSTER 32
 
+/* The subcluster X [0..31] reads as zeroes */
+#define QCOW_OFLAG_SUB_ZERO(X)((1ULL << 63) >> (X))
+/* The subcluster X [0..31] is allocated */
+#define QCOW_OFLAG_SUB_ALLOC(X)   ((1ULL << 31) >> (X))
+/* L2 entry bitmap with all "read as zeroes" bits set */
+#define QCOW_L2_BITMAP_ALL_ZEROES 0xULL
+/* L2 entry bitmap with all allocation bits set */
+#define QCOW_L2_BITMAP_ALL_ALLOC  0xULL
+
 /* Size of normal and extended L2 entries */
 #define L2E_SIZE_NORMAL   (sizeof(uint64_t))
 #define L2E_SIZE_EXTENDED (sizeof(uint64_t) * 2)
@@ -455,6 +464,16 @@ typedef enum QCow2ClusterType {
 QCOW2_CLUSTER_COMPRESSED,
 } QCow2ClusterType;
 
+typedef enum QCow2SubclusterType {
+QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN,
+QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC,
+QCOW2_SUBCLUSTER_ZERO_PLAIN,
+QCOW2_SUBCLUSTER_ZERO_ALLOC,
+QCOW2_SUBCLUSTER_NORMAL,
+QCOW2_SUBCLUSTER_COMPRESSED,
+QCOW2_SUBCLUSTER_INVALID,
+} QCow2SubclusterType;
+
 typedef enum QCow2MetadataOverlap {
 QCOW2_OL_MAIN_HEADER_BITNR  = 0,
 QCOW2_OL_ACTIVE_L1_BITNR= 1,
@@ -632,6 +651,79 @@ static inline QCow2ClusterType 
qcow2_get_cluster_type(BlockDriverState *bs,
 }
 }
 
+/* For an image without extended L2 entries, return the
+ * QCow2SubclusterType equivalent of a given QCow2ClusterType */
+static inline
+QCow2SubclusterType qcow2_cluster_to_subcluster_type(QCow2ClusterType type)
+{
+switch (type) {
+case QCOW2_CLUSTER_COMPRESSED:
+return QCOW2_SUBCLUSTER_COMPRESSED;
+case QCOW2_CLUSTER_ZERO_PLAIN:
+return QCOW2_SUBCLUSTER_ZERO_PLAIN;
+case QCOW2_CLUSTER_ZERO_ALLOC:
+return QCOW2_SUBCLUSTER_ZERO_ALLOC;
+case QCOW2_CLUSTER_NORMAL:
+return QCOW2_SUBCLUSTER_NORMAL;
+case QCOW2_CLUSTER_UNALLOCATED:
+return QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
+default:
+g_assert_not_reached();
+}
+}
+
+/* In an image without subsclusters @l2_bitmap is ignored and
+ * @sc_index must be 0. */
+static inline
+QCow2SubclusterType qcow2_get_subcluster_type(BlockDriverState *bs,
+  uint64_t l2_entry,
+  uint64_t l2_bitmap,
+  unsigned sc_index)
+{
+BDRVQcow2State *s = bs->opaque;
+QCow2ClusterType type = qcow2_get_cluster_type(bs, l2_entry);
+assert(sc_index < s->subclusters_per_cluster);
+
+if (has_subclusters(s)) {
+bool sc_zero  = l2_bitmap & QCOW_OFLAG_SUB_ZERO(sc_index);
+bool sc_alloc = l2_bitmap & QCOW_OFLAG_SUB_ALLOC(sc_index);
+switch (type) {
+case QCOW2_CLUSTER_COMPRESSED:
+if (l2_bitmap != 0) {
+return QCOW2_SUBCLUSTER_INVALID;
+}
+return QCOW2_SUBCLUSTER_COMPRESSED;
+case QCOW2_CLUST

[RFC PATCH v3 24/27] qcow2: Restrict qcow2_co_pwrite_zeroes() to full clusters only

[Alberto Garcia]

Ideally it should be possible to zero individual subclusters using
this function, but this is currently not implemented.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 242001afa2..0267722065 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3754,6 +3754,12 @@ static coroutine_fn int 
qcow2_co_pwrite_zeroes(BlockDriverState *bs,
 qemu_co_mutex_unlock(&s->lock);
 return -ENOTSUP;
 }
+/* TODO: allow zeroing separate subclusters, we only allow
+ * zeroing full clusters at the moment. */
+if (nr != bytes) {
+qemu_co_mutex_unlock(&s->lock);
+return -ENOTSUP;
+}
 if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN &&
 type != QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC &&
 type != QCOW2_SUBCLUSTER_ZERO_PLAIN &&
-- 
2.20.1

Re: [PATCH v2 2/6] hw/display/tcx: Add missing fall through comments

[Mark Cave-Ayland]

On 18/12/2019 19:25, Philippe Mathieu-Daudé wrote:

> When building with GCC9 using CFLAG -Wimplicit-fallthrough=2 we get:
> 
>   hw/display/tcx.c: In function ‘tcx_dac_writel’:
>   hw/display/tcx.c:453:26: error: this statement may fall through 
> [-Werror=implicit-fallthrough=]
> 453 | s->dac_index = (s->dac_index + 1) & 0xff; /* Index 
> autoincrement */
> | ~^~~
>   hw/display/tcx.c:454:9: note: here
> 454 | default:
> | ^~~
>   hw/display/tcx.c: In function ‘tcx_dac_readl’:
>   hw/display/tcx.c:412:22: error: this statement may fall through 
> [-Werror=implicit-fallthrough=]
> 412 | s->dac_index = (s->dac_index + 1) & 0xff; /* Index 
> autoincrement */
> | ~^~~
>   hw/display/tcx.c:413:5: note: here
> 413 | default:
> | ^~~
>   cc1: all warnings being treated as errors
> 
> Give a hint to GCC by adding the missing fall through comments.
> 
> Fixes: 55d7bfe22
> Reviewed-by: Richard Henderson 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> v2: Reword the description (Aleksandar)
> 
> Cc: Aleksandar Markovic 
> Cc: Olivier Danet 
> Cc: Mark Cave-Ayland 
> ---
>  hw/display/tcx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 14e829d3fa..abbeb30284 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -410,6 +410,7 @@ static uint64_t tcx_dac_readl(void *opaque, hwaddr addr,
>  case 2:
>  val = s->b[s->dac_index] << 24;
>  s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement */
> +/* fall through */
>  default:
>  s->dac_state = 0;
>  break;
> @@ -451,6 +452,7 @@ static void tcx_dac_writel(void *opaque, hwaddr addr, 
> uint64_t val,
>  s->b[index] = val >> 24;
>  update_palette_entries(s, index, index + 1);
>  s->dac_index = (s->dac_index + 1) & 0xff; /* Index autoincrement 
> */
> +/* fall through */
>  default:
>  s->dac_state = 0;
>  break;

Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 1/2] q800: fix mac_via RTC PRAM commands

[Mark Cave-Ayland]

On 19/12/2019 20:14, Laurent Vivier wrote:

> The command byte is not decoded correctly.
> 
> This patch reworks the RTC/PRAM interface and fixes the problem.
> It adds a comment before the function to explain how are encoded commands
> and some trace-events to ease debugging.
> 
> Bug: https://bugs.launchpad.net/qemu/+bug/1856549
> Fixes: 6dca62a000 ("hw/m68k: add VIA support")
> Signed-off-by: Laurent Vivier 
> ---
>  hw/misc/mac_via.c| 274 ++-
>  hw/misc/trace-events |  19 +++
>  2 files changed, 210 insertions(+), 83 deletions(-)
> 
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index f3f130ad96..e5658af922 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -27,7 +27,7 @@
>  #include "sysemu/runstate.h"
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
> -
> +#include "trace.h"
>  
>  /*
>   * VIAs: There are two in every machine,
> @@ -278,6 +278,21 @@
>  /* VIA returns time offset from Jan 1, 1904, not 1970 */
>  #define RTC_OFFSET 2082844800
>  
> +enum {
> +REG_0,
> +REG_1,
> +REG_2,
> +REG_3,
> +REG_TEST,
> +REG_WPROTECT,
> +REG_PRAM_ADDR,
> +REG_PRAM_ADDR_LAST = REG_PRAM_ADDR + 19,
> +REG_PRAM_SECT,
> +REG_PRAM_SECT_LAST = REG_PRAM_SECT + 7,
> +REG_INVALID,
> +REG_EMPTY = 0xff,
> +};
> +
>  static void via1_VBL_update(MOS6522Q800VIA1State *v1s)
>  {
>  MOS6522State *s = MOS6522(v1s);
> @@ -360,10 +375,62 @@ static void via2_irq_request(void *opaque, int irq, int 
> level)
>  mdc->update_irq(s);
>  }
>  
> +/*
> + * RTC Commands
> + *
> + * Command byteRegister addressed by the command
> + *
> + * z001Seconds register 0 (lowest-order byte)
> + * z101Seconds register 1
> + * z0001001Seconds register 2
> + * z0001101Seconds register 3 (highest-order byte)
> + * 00110001Test register (write-only)
> + * 00110101Write-Protect Register (write-only)
> + * z010aa01RAM address 100aa ($10-$13) (first 20 bytes only)
> + * z101RAM address 0 ($00-$0F) (first 20 bytes only)
> + * z0111aaaExtended memory designator and sector number
> + *
> + * For a read request, z=1, for a write z=0
> + * The letter a indicates bits whose value depend on what parameter
> + * RAM byte you want to address
> + */
> +static int via1_rtc_compact_cmd(uint8_t value)
> +{
> +uint8_t read = value & 0x80;
> +
> +value &= 0x7f;
> +
> +/* the last 2 bits of a command byte must always be 0b01 ... */
> +if ((value & 0x78) == 0x38) {
> +/* except for the extended memory designator */
> +return read | (REG_PRAM_SECT + (value & 0x07));
> +}
> +if ((value & 0x03) == 0x01) {
> +value >>= 2;
> +if ((value & 0x1c) == 0) {
> +/* seconds registers */
> +return read | (REG_0 + (value & 0x03));
> +} else if ((value == 0x0c) && !read) {
> +return REG_TEST;
> +} else if ((value == 0x0d) && !read) {
> +return REG_WPROTECT;
> +} else if ((value & 0x1c) == 0x08) {
> +/* RAM address 0x10 to 0x13 */
> +return read | (REG_PRAM_ADDR + 0x10 + (value & 0x03));
> +} else if ((value & 0x43) == 0x41) {
> +/* RAM address 0x00 to 0x0f */
> +return read | (REG_PRAM_ADDR + (value & 0x0f));
> +}
> +}
> +return REG_INVALID;
> +}
> +
>  static void via1_rtc_update(MacVIAState *m)
>  {
>  MOS6522Q800VIA1State *v1s = &m->mos6522_via1;
>  MOS6522State *s = MOS6522(v1s);
> +int cmd, sector, addr;
> +uint32_t time;
>  
>  if (s->b & VIA1B_vRTCEnb) {
>  return;
> @@ -376,7 +443,9 @@ static void via1_rtc_update(MacVIAState *m)
>  m->data_out |= s->b & VIA1B_vRTCData;
>  m->data_out_cnt++;
>  }
> +trace_via1_rtc_update_data_out(m->data_out_cnt, m->data_out);
>  } else {
> +trace_via1_rtc_update_data_in(m->data_in_cnt, m->data_in);
>  /* receive bits from the RTC */
>  if ((v1s->last_b & VIA1B_vRTCClk) &&
>  !(s->b & VIA1B_vRTCClk) &&
> @@ -386,96 +455,132 @@ static void via1_rtc_update(MacVIAState *m)
>  m->data_in <<= 1;
>  m->data_in_cnt--;
>  }
> +return;
>  }
>  
> -if (m->data_out_cnt == 8) {
> -m->data_out_cnt = 0;
> -
> -if (m->cmd == 0) {
> -if (m->data_out & 0x80) {
> -/* this is a read command */
> -uint32_t time = m->tick_offset +
> -   (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
> -   NANOSECONDS_PER_SECOND);
> -if (m->data_out == 0x81) {/* seconds register 0 */
> -m->data_in = time & 0xff;
> -m->data_in_cnt = 8;
> -} else if (m->data_out == 0x85) { /* seconds register 1 */
> -m->data_in = (time 

Re: [PATCH] hw/i386/x86-iommu: Add missing stubs

[Paolo Bonzini]

On 20/12/19 16:42, Philippe Mathieu-Daudé wrote:
> In commit 6c730e4af9 we introduced a stub to build the MicroVM
> machine without Intel IOMMU. This stub is incomplete for the
> other PC machines. Add the missing stubs.

In other words, without this patch you cannot build without Q35 (which
brings in the IOMMU, at least unless building
--without-default-devices).  Is this correct?

Paolo

> 
> Fixes: 6c730e4af9
> Reported-by: Travis-CI
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/i386/x86-iommu-stub.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/i386/x86-iommu-stub.c b/hw/i386/x86-iommu-stub.c
> index 03576cdccb..c5ba077f9d 100644
> --- a/hw/i386/x86-iommu-stub.c
> +++ b/hw/i386/x86-iommu-stub.c
> @@ -32,3 +32,12 @@ X86IOMMUState *x86_iommu_get_default(void)
>  return NULL;
>  }
>  
> +bool x86_iommu_ir_supported(X86IOMMUState *s)
> +{
> +return false;
> +}
> +
> +IommuType x86_iommu_get_type(void)
> +{
> +abort();
> +}
> 

Re: [PATCH 1/2] hppa: Do not enable artist graphics with -nographic option

[Helge Deller]

Hi Sven,

On 22.12.19 09:39, Sven Schnelle wrote:
> On Sat, Dec 21, 2019 at 11:24:02PM +0100, Helge Deller wrote:
>> When qemu was started with the -nographic option, do not enable the
>> artist graphic card emulation.
>
> Hmm, isn't '-nographic -vnc' a valid option that wouldn't work anymore in 
> that case?

You don't need the "-nographic" then, just use the "-vnc" parameter.

> I used '-nographic -vga none' to disable Artist.

With my patch "-nographic" is sufficient and behaves as the help says:
-nographicdisable graphical output and redirect serial I/Os to console

Helge

[Bug 1856335] Re: Cache Layout wrong on many Zen Arch CPUs

[Damir]

This is the commit I am referencing:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=8f4202fb1080f86958782b1fca0bf0279f67d136

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

Title:
  Cache Layout wrong on many Zen Arch CPUs

Status in QEMU:
  New

Bug description:
  AMD CPUs have L3 cache per 2, 3 or 4 cores. Currently, TOPOEXT seems
  to always map Cache ass if it was an 4-Core per CCX CPU, which is
  incorrect, and costs upwards 30% performance (more realistically 10%)
  in L3 Cache Layout aware applications.

  Example on a 4-CCX CPU (1950X /w 8 Cores and no SMT):

    
  EPYC-IBPB
  AMD
  

  In windows, coreinfo reports correctly:

    Unified Cache 1, Level 3,8 MB, Assoc  16, LineSize  64
    Unified Cache 6, Level 3,8 MB, Assoc  16, LineSize  64

  On a 3-CCX CPU (3960X /w 6 cores and no SMT):

   
  EPYC-IBPB
  AMD
  

  in windows, coreinfo reports incorrectly:

  --  Unified Cache  1, Level 3,8 MB, Assoc  16, LineSize  64
  **  Unified Cache  6, Level 3,8 MB, Assoc  16, LineSize  64

  Validated against 3.0, 3.1, 4.1 and 4.2 versions of qemu-kvm.

  With newer Qemu there is a fix (that does behave correctly) in using the dies 
parameter:
   

  The problem is that the dies are exposed differently than how AMD does
  it natively, they are exposed to Windows as sockets, which means, that
  if you are nto a business user, you can't ever have a machine with
  more than two CCX (6 cores) as consumer versions of Windows only
  supports two sockets. (Should this be reported as a separate bug?)

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

[Bug 1856335] Re: Cache Layout wrong on many Zen Arch CPUs

[Damir]

Hi,

I've since confirmed that this bug also exist (as expected) on Linux
guests, as well as Zen1 EPYC 7401 CPUs, to make sure this wasn't a
problem with the detection of the newer consumer platform.

Basically it seems (looking at the code with layman eyes) that as long
as you have a topology that is dividable by 4 or 8, it will always
result in the wrong topology being exposed to the guest, even when the
correct option can be built (12, 24 core CPUs, although, it would be
great if we could support 9 Core VM CPus as that is a reasonable use
case for VMs (3 CCXs of 3 Cores for a total of 9 (or 18 SMT threads)).

Pinging the author and committer of the TopoEXT feature / EPYC cpu model
as they should probably know best how to solve this issue.

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

Title:
  Cache Layout wrong on many Zen Arch CPUs

Status in QEMU:
  New

Bug description:
  AMD CPUs have L3 cache per 2, 3 or 4 cores. Currently, TOPOEXT seems
  to always map Cache ass if it was an 4-Core per CCX CPU, which is
  incorrect, and costs upwards 30% performance (more realistically 10%)
  in L3 Cache Layout aware applications.

  Example on a 4-CCX CPU (1950X /w 8 Cores and no SMT):

    
  EPYC-IBPB
  AMD
  

  In windows, coreinfo reports correctly:

    Unified Cache 1, Level 3,8 MB, Assoc  16, LineSize  64
    Unified Cache 6, Level 3,8 MB, Assoc  16, LineSize  64

  On a 3-CCX CPU (3960X /w 6 cores and no SMT):

   
  EPYC-IBPB
  AMD
  

  in windows, coreinfo reports incorrectly:

  --  Unified Cache  1, Level 3,8 MB, Assoc  16, LineSize  64
  **  Unified Cache  6, Level 3,8 MB, Assoc  16, LineSize  64

  Validated against 3.0, 3.1, 4.1 and 4.2 versions of qemu-kvm.

  With newer Qemu there is a fix (that does behave correctly) in using the dies 
parameter:
   

  The problem is that the dies are exposed differently than how AMD does
  it natively, they are exposed to Windows as sockets, which means, that
  if you are nto a business user, you can't ever have a machine with
  more than two CCX (6 cores) as consumer versions of Windows only
  supports two sockets. (Should this be reported as a separate bug?)

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

Re: [PATCH v2 03/13] ppc/pnv: Introduce a "xics" property alias under the PSI model

[David Gibson]

On Thu, Dec 19, 2019 at 07:11:45PM +0100, Cédric Le Goater wrote:
> This removes the need of the intermediate link under PSI to pass the
> XICS link to the underlying ICSState object.
> 
> Signed-off-by: Cédric Le Goater 
> Reviewed-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

LGTM, but will need a rebase if we're dropping the previous patch.

> ---
>  hw/ppc/pnv.c |  4 ++--
>  hw/ppc/pnv_psi.c | 11 ++-
>  2 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 1d8bfb164a32..163a658806e2 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -999,8 +999,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
>  
>  object_initialize_child(obj, "psi",  &chip8->psi, sizeof(chip8->psi),
>  TYPE_PNV8_PSI, &error_abort, NULL);
> -object_property_add_const_link(OBJECT(&chip8->psi), "xics",
> -   OBJECT(qdev_get_machine()), &error_abort);
>  
>  object_initialize_child(obj, "lpc",  &chip8->lpc, sizeof(chip8->lpc),
>  TYPE_PNV8_LPC, &error_abort, NULL);
> @@ -1069,6 +1067,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, 
> Error **errp)
>  "bar", &error_fatal);
>  object_property_set_link(OBJECT(&chip8->psi), 
> OBJECT(chip->system_memory),
>   "system-memory", &error_abort);
> +object_property_set_link(OBJECT(&chip8->psi), OBJECT(qdev_get_machine()),
> + ICS_PROP_XICS, &error_abort);
>  object_property_set_bool(OBJECT(&chip8->psi), true, "realized", 
> &local_err);
>  if (local_err) {
>  error_propagate(errp, local_err);
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index 28d34e5c193a..d3124f673571 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -470,6 +470,8 @@ static void pnv_psi_power8_instance_init(Object *obj)
>  
>  object_initialize_child(obj, "ics-psi",  &psi8->ics, sizeof(psi8->ics),
>  TYPE_ICS, &error_abort, NULL);
> +object_property_add_alias(obj, ICS_PROP_XICS, OBJECT(&psi8->ics),
> +  ICS_PROP_XICS, &error_abort);
>  }
>  
>  static const uint8_t irq_to_xivr[] = {
> @@ -485,21 +487,12 @@ static void pnv_psi_power8_realize(DeviceState *dev, 
> Error **errp)
>  {
>  PnvPsi *psi = PNV_PSI(dev);
>  ICSState *ics = &PNV8_PSI(psi)->ics;
> -Object *obj;
>  Error *err = NULL;
>  unsigned int i;
>  
>  assert(psi->system_memory);
>  
> -obj = object_property_get_link(OBJECT(dev), "xics", &err);
> -if (!obj) {
> -error_setg(errp, "%s: required link 'xics' not found: %s",
> -   __func__, error_get_pretty(err));
> -return;
> -}
> -
>  /* Create PSI interrupt control source */
> -object_property_set_link(OBJECT(ics), obj, ICS_PROP_XICS, &error_abort);
>  object_property_set_int(OBJECT(ics), PSI_NUM_INTERRUPTS, "nr-irqs", 
> &err);
>  if (err) {
>  error_propagate(errp, err);

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


signature.asc
Description: PGP signature

Re: [PATCH 1/2] hppa: Do not enable artist graphics with -nographic option

[Sven Schnelle]

Hi Helge,

On Sat, Dec 21, 2019 at 11:24:02PM +0100, Helge Deller wrote:
> When qemu was started with the -nographic option, do not enable the
> artist graphic card emulation.

Hmm, isn't '-nographic -vnc' a valid option that wouldn't work anymore in that
case? I used '-nographic -vga none' to disable Artist.

Regards
Sven

Re: [PATCH] i386: pass CLZERO to guests with EPYC CPU model on AMD ZEN platform

[Paolo Bonzini]

Il dom 22 dic 2019, 08:49 Ani Sinha  ha scritto:

>
> Ping …
>

Why ping? You got questions from Eduardo, so you need to answer them and/or
send a fixed version of the patch.

Thanks,

Paolo


> > On Dec 18, 2019, at 5:23 PM, Paolo Bonzini  wrote:
> >
> > On 18/12/19 10:05, Ani Sinha wrote:
> >> CLZERO CPUID should be passed on to the guests that use EPYC or
> EPYC-IBPB CPU
> >> model when the AMD ZEN based host supports it. This change makes it
> recognize
> >> this CPUID for guests which use EPYC or EPYC-IBPB CPU model.
> >>
> >> Signed-off-by: Ani Sinha 
> >> ---
> >> target/i386/cpu.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> >> index 69f518a..55f0691 100644
> >> --- a/target/i386/cpu.c
> >> +++ b/target/i386/cpu.c
> >> @@ -3813,6 +3813,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >> CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
> >> CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
> >> CPUID_EXT3_TOPOEXT,
> >> +.features[FEAT_8000_0008_EBX] =
> >> +CPUID_8000_0008_EBX_CLZERO,
> >> .features[FEAT_7_0_EBX] =
> >> CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
> CPUID_7_0_EBX_AVX2 |
> >> CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 |
> CPUID_7_0_EBX_RDSEED |
> >>
> >
> > This needs to be done only for newer machine type (or is it CPU model
> > versions now? need Eduardo to respond).
> >
> > Paolo
>
>