Re: [PATCH v1 2/2] xilink-zynq-devcfg: Fix up for memory address range size not set correctly

[Edgar E. Iglesias]

On Sun, Sep 22, 2024 at 09:24:33PM +0800, Chao Liu wrote:
> Signed-off-by: Chao Liu 


Reviewed-by: Edgar E. Iglesias 


> ---
>  hw/dma/xlnx-zynq-devcfg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/dma/xlnx-zynq-devcfg.c b/hw/dma/xlnx-zynq-devcfg.c
> index b8544d0731..7170353a62 100644
> --- a/hw/dma/xlnx-zynq-devcfg.c
> +++ b/hw/dma/xlnx-zynq-devcfg.c
> @@ -372,7 +372,7 @@ static void xlnx_zynq_devcfg_init(Object *obj)
>s->regs_info, s->regs,
>&xlnx_zynq_devcfg_reg_ops,
>XLNX_ZYNQ_DEVCFG_ERR_DEBUG,
> -  XLNX_ZYNQ_DEVCFG_R_MAX);
> +  XLNX_ZYNQ_DEVCFG_R_MAX * 4);
>  memory_region_add_subregion(&s->iomem,
>  A_CTRL,
>  ®_array->mem);
> -- 
> 2.46.1
> 

Re: [PATCH v1 1/1] block/file-posix: Avoid maybe-uninitialized warning

[Edgar E. Iglesias]

Ping!

On Wed, Aug 14, 2024 at 01:15:55PM -0500, Eric Blake wrote:
> On Mon, Aug 12, 2024 at 04:43:23PM GMT, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > Avoid a maybe-uninitialized warning in raw_refresh_zoned_limits()
> > by initializing zoned.
> > 
> > With GCC 14.1.0:
> > In function ‘raw_refresh_zoned_limits’,
> > inlined from ‘raw_refresh_limits’ at ../qemu/block/file-posix.c:1522:5:
> > ../qemu/block/file-posix.c:1405:17: error: ‘zoned’ may be used 
> > uninitialized [-Werror=maybe-uninitialized]
> >  1405 | if (ret < 0 || zoned == BLK_Z_NONE) {
> >   | ^~
> > ../qemu/block/file-posix.c:1401:20: note: ‘zoned’ was declared here
> >  1401 | BlockZoneModel zoned;
> >       |    ^
> > cc1: all warnings being treated as errors
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  block/file-posix.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Eric Blake 
> 
> > 
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index ff928b5e85..90fa54352c 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -1398,7 +1398,7 @@ static void raw_refresh_zoned_limits(BlockDriverState 
> > *bs, struct stat *st,
> >   Error **errp)
> >  {
> >  BDRVRawState *s = bs->opaque;
> > -BlockZoneModel zoned;
> > +BlockZoneModel zoned = BLK_Z_NONE;
> >  int ret;
> >  
> >  ret = get_sysfs_zoned_model(st, &zoned);
> > -- 
> > 2.43.0
> > 
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.
> Virtualization:  qemu.org | libguestfs.org
> 

Re: [PATCH] hw/xen: Remove deadcode

[Edgar E. Iglesias]

On Tue, Sep 17, 2024 at 09:03:09AM +, Anthony PERARD wrote:
> On Tue, Sep 17, 2024 at 01:22:12AM +0100, d...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > xen_be_copy_grant_refs is unused since 2019's
> >   19f87870ba ("xen: remove the legacy 'xen_disk' backend")
> > 
> > xen_config_dev_console is unused since 2018's
> >   6d7c06c213 ("Remove broken Xen PV domain builder")
> > 
> > Remove them.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> 
> Acked-by: Anthony PERARD 
> 
> Thanks,

Reviewed-by: Edgar E. Iglesias 

Cheers,
Edgar

[PATCH v2 3/4] hw/xen: xenpvh: Add pci-intx-irq-base property

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Acked-by: Stefano Stabellini 
Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-pvh-common.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 76a9b2b945..218ac851cf 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -218,6 +218,11 @@ static void xen_pvh_init(MachineState *ms)
 error_report("pci-ecam-size only supports values 0 or 0x1000");
 exit(EXIT_FAILURE);
 }
+if (!s->cfg.pci_intx_irq_base) {
+error_report("PCI enabled but pci-intx-irq-base not set");
+exit(EXIT_FAILURE);
+}
+
 xenpvh_gpex_init(s, xpc, sysmem);
 }
 
@@ -273,6 +278,30 @@ XEN_PVH_PROP_MEMMAP(pci_ecam)
 XEN_PVH_PROP_MEMMAP(pci_mmio)
 XEN_PVH_PROP_MEMMAP(pci_mmio_high)
 
+static void xen_pvh_set_pci_intx_irq_base(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+XenPVHMachineState *xp = XEN_PVH_MACHINE(obj);
+uint32_t value;
+
+if (!visit_type_uint32(v, name, &value, errp)) {
+return;
+}
+
+xp->cfg.pci_intx_irq_base = value;
+}
+
+static void xen_pvh_get_pci_intx_irq_base(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+XenPVHMachineState *xp = XEN_PVH_MACHINE(obj);
+uint32_t value = xp->cfg.pci_intx_irq_base;
+
+visit_type_uint32(v, name, &value, errp);
+}
+
 void xen_pvh_class_setup_common_props(XenPVHMachineClass *xpc)
 {
 ObjectClass *oc = OBJECT_CLASS(xpc);
@@ -318,6 +347,13 @@ do {   
   \
 OC_MEMMAP_PROP(oc, "pci-ecam", pci_ecam);
 OC_MEMMAP_PROP(oc, "pci-mmio", pci_mmio);
 OC_MEMMAP_PROP(oc, "pci-mmio-high", pci_mmio_high);
+
+object_class_property_add(oc, "pci-intx-irq-base", "uint32_t",
+  xen_pvh_get_pci_intx_irq_base,
+  xen_pvh_set_pci_intx_irq_base,
+  NULL, NULL);
+object_class_property_set_description(oc, "pci-intx-irq-base",
+  "Set PCI INTX interrupt base line.");
 }
 
 #ifdef CONFIG_TPM
-- 
2.43.0

[PATCH v2 2/4] hw/xen: xenpvh: Disable buffered IOREQs for ARM

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add a way to enable/disable buffered IOREQs for PVH machines
and disable them for ARM. ARM does not support buffered
IOREQ's nor the legacy way to map IOREQ info pages.

See the following for more details:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=2fbd7e609e1803ac5e5c26e22aa8e4b5a6cddbb1
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/ioreq.c;h=2e829d2e7f3760401b96fa7c930e2015fb1cf463;hb=HEAD#l138

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen-pvh.c| 3 +++
 hw/i386/xen/xen-pvh.c   | 3 +++
 hw/xen/xen-pvh-common.c | 2 +-
 include/hw/xen/xen-pvh-common.h | 3 +++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
index 04cb9855af..28af3910ea 100644
--- a/hw/arm/xen-pvh.c
+++ b/hw/arm/xen-pvh.c
@@ -66,6 +66,9 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void 
*data)
  */
 mc->max_cpus = GUEST_MAX_VCPUS;
 
+/* Xen/ARM does not use buffered IOREQs.  */
+xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
+
 /* List of supported features known to work on PVH ARM.  */
 xpc->has_tpm = true;
 xpc->has_virtio_mmio = true;
diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
index 45645667e9..f1f02d3311 100644
--- a/hw/i386/xen/xen-pvh.c
+++ b/hw/i386/xen/xen-pvh.c
@@ -89,6 +89,9 @@ static void xen_pvh_machine_class_init(ObjectClass *oc, void 
*data)
 /* We have an implementation specific init to create CPU objects.  */
 xpc->init = xen_pvh_init;
 
+/* Enable buffered IOREQs.  */
+xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_ATOMIC;
+
 /*
  * PCI INTX routing.
  *
diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 08641fdcec..76a9b2b945 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -195,7 +195,7 @@ static void xen_pvh_init(MachineState *ms)
 
 xen_pvh_init_ram(s, sysmem);
 xen_register_ioreq(&s->ioreq, ms->smp.max_cpus,
-   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
+   xpc->handle_bufioreq,
&xen_memory_listener);
 
 if (s->cfg.virtio_mmio_num) {
diff --git a/include/hw/xen/xen-pvh-common.h b/include/hw/xen/xen-pvh-common.h
index bc09eea936..5cdd23c2f4 100644
--- a/include/hw/xen/xen-pvh-common.h
+++ b/include/hw/xen/xen-pvh-common.h
@@ -43,6 +43,9 @@ struct XenPVHMachineClass {
  */
 int (*set_pci_link_route)(uint8_t line, uint8_t irq);
 
+/* Allow implementations to optionally enable buffered ioreqs.  */
+uint8_t handle_bufioreq;
+
 /*
  * Each implementation can optionally enable features that it
  * supports and are known to work.
-- 
2.43.0

[PATCH v2 4/4] hw/arm: xenpvh: Enable PCI for ARM PVH

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable PCI support for the ARM Xen PVH machine.

Reviewed-by: Stefano Stabellini 
Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen-pvh.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
index 28af3910ea..33f0dd5982 100644
--- a/hw/arm/xen-pvh.c
+++ b/hw/arm/xen-pvh.c
@@ -39,6 +39,16 @@ static void xen_arm_instance_init(Object *obj)
  VIRTIO_MMIO_DEV_SIZE };
 }
 
+static void xen_pvh_set_pci_intx_irq(void *opaque, int intx_irq, int level)
+{
+XenPVHMachineState *s = XEN_PVH_MACHINE(opaque);
+int irq = s->cfg.pci_intx_irq_base + intx_irq;
+
+if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
+error_report("xendevicemodel_set_pci_intx_level failed");
+}
+}
+
 static void xen_arm_machine_class_init(ObjectClass *oc, void *data)
 {
 XenPVHMachineClass *xpc = XEN_PVH_MACHINE_CLASS(oc);
@@ -69,7 +79,11 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void 
*data)
 /* Xen/ARM does not use buffered IOREQs.  */
 xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
 
+/* PCI INTX delivery.  */
+xpc->set_pci_intx_irq = xen_pvh_set_pci_intx_irq;
+
 /* List of supported features known to work on PVH ARM.  */
+xpc->has_pci = true;
 xpc->has_tpm = true;
 xpc->has_virtio_mmio = true;
 
-- 
2.43.0

[PATCH v2 0/4] hw/arm: xenpvh: Enable PCI for ARM PVH

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable PCI on the ARM PVH machine. First we add a way to control the use
of buffered IOREQ's since those are not supported on Xen/ARM.
Finally we enable the PCI support.

I've published some instructions on how to try this including the work in
progress Xen side of the PVH PCI support:
https://github.com/edgarigl/docs/blob/master/xen/pvh/virtio-pci-dom0less.md

Cheers,
Edgar

ChangeLog:

v1 -> v2:
* Change handle_ioreq from int to uint8_t.
* Fallback to legacy API if buffered ioreqs are enabled and also if
  the new API is not supported. Clarify with comments.


Edgar E. Iglesias (4):
  hw/xen: Expose handle_bufioreq in xen_register_ioreq
  hw/xen: xenpvh: Disable buffered IOREQs for ARM
  hw/xen: xenpvh: Add pci-intx-irq-base property
  hw/arm: xenpvh: Enable PCI for ARM PVH

 hw/arm/xen-pvh.c|  17 ++
 hw/i386/xen/xen-hvm.c   |   4 +-
 hw/i386/xen/xen-pvh.c   |   3 +
 hw/xen/xen-hvm-common.c | 101 
 hw/xen/xen-pvh-common.c |  40 -
 include/hw/xen/xen-hvm-common.h |   3 +
 include/hw/xen/xen-pvh-common.h |   3 +
 include/hw/xen/xen_native.h |   3 +-
 8 files changed, 133 insertions(+), 41 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] hw/xen: Expose handle_bufioreq in xen_register_ioreq

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Expose handle_bufioreq in xen_register_ioreq().
This is to allow machines to enable or disable buffered ioreqs.

No functional change since all callers still set it to
HVM_IOREQSRV_BUFIOREQ_ATOMIC.

Signed-off-by: Edgar E. Iglesias 
---
 hw/i386/xen/xen-hvm.c   |   4 +-
 hw/xen/xen-hvm-common.c | 101 
 hw/xen/xen-pvh-common.c |   4 +-
 include/hw/xen/xen-hvm-common.h |   3 +
 include/hw/xen/xen_native.h |   3 +-
 5 files changed, 74 insertions(+), 41 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 4f6446600c..d3df488c48 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -614,7 +614,9 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
**ram_memory)
 
 state = g_new0(XenIOState, 1);
 
-xen_register_ioreq(state, max_cpus, &xen_memory_listener);
+xen_register_ioreq(state, max_cpus,
+   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
+   &xen_memory_listener);
 
 xen_is_stubdomain = xen_check_stubdomain(state->xenstore);
 
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 3a9d6f981b..3ce994fc3a 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -667,6 +667,8 @@ static int xen_map_ioreq_server(XenIOState *state)
 xen_pfn_t ioreq_pfn;
 xen_pfn_t bufioreq_pfn;
 evtchn_port_t bufioreq_evtchn;
+unsigned long num_frames = 1;
+unsigned long frame = 1;
 int rc;
 
 /*
@@ -675,59 +677,79 @@ static int xen_map_ioreq_server(XenIOState *state)
  */
 QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_bufioreq != 0);
 QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_ioreq(0) != 1);
+
+if (state->has_bufioreq) {
+frame = 0;
+num_frames = 2;
+}
 state->fres = xenforeignmemory_map_resource(xen_fmem, xen_domid,
  XENMEM_resource_ioreq_server,
- state->ioservid, 0, 2,
+ state->ioservid,
+ frame, num_frames,
  &addr,
  PROT_READ | PROT_WRITE, 0);
 if (state->fres != NULL) {
 trace_xen_map_resource_ioreq(state->ioservid, addr);
-state->buffered_io_page = addr;
-state->shared_page = addr + XC_PAGE_SIZE;
+state->shared_page = addr;
+if (state->has_bufioreq) {
+state->buffered_io_page = addr;
+state->shared_page = addr + XC_PAGE_SIZE;
+}
 } else if (errno != EOPNOTSUPP) {
 error_report("failed to map ioreq server resources: error %d 
handle=%p",
  errno, xen_xc);
 return -1;
 }
 
-rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
-   (state->shared_page == NULL) ?
-   &ioreq_pfn : NULL,
-   (state->buffered_io_page == NULL) ?
-   &bufioreq_pfn : NULL,
-   &bufioreq_evtchn);
-if (rc < 0) {
-error_report("failed to get ioreq server info: error %d handle=%p",
- errno, xen_xc);
-return rc;
-}
-
-if (state->shared_page == NULL) {
+/*
+ * If we fail to map the shared page with xenforeignmemory_map_resource()
+ * or if we're using buffered ioreqs, we need xen_get_ioreq_server_info()
+ * to provide the the addresses to map the shared page and/or to get the
+ * event-channel port for buffered ioreqs.
+ */
+if (state->shared_page == NULL || state->has_bufioreq) {
 trace_xen_map_ioreq_server_shared_page(ioreq_pfn);
+rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
+   (state->shared_page == NULL) ?
+   &ioreq_pfn : NULL,
+   (state->has_bufioreq &&
+state->buffered_io_page == NULL) ?
+   &bufioreq_pfn : NULL,
+   &bufioreq_evtchn);
+if (rc < 0) {
+error_report("failed to get ioreq server info: error %d handle=%p",
+ errno, xen_xc);
+return rc;
+}
 
-state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
-  PROT_READ | PROT_WRITE,
-  1, &ioreq_pfn, NULL);
+if (state->shared_page == NULL) {
+trace_xen_map_ioreq_server_share

Re: [PATCH v1 1/4] xen: Expose handle_bufioreq in xen_register_ioreq

[Edgar E. Iglesias]

On Mon, Sep 16, 2024 at 04:45:34PM -0700, Stefano Stabellini wrote:
> On Mon, 16 Sep 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > Expose handle_bufioreq in xen_register_ioreq().
> > This is to allow machines to enable or disable buffered ioreqs.
> > 
> > No functional change since all callers still set it to
> > HVM_IOREQSRV_BUFIOREQ_ATOMIC.
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  hw/i386/xen/xen-hvm.c   |   4 +-
> >  hw/xen/xen-hvm-common.c | 100 +++-
> >  hw/xen/xen-pvh-common.c |   4 +-
> >  include/hw/xen/xen-hvm-common.h |   3 +
> >  include/hw/xen/xen_native.h |   3 +-
> >  5 files changed, 70 insertions(+), 44 deletions(-)
> > 
> > diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> > index 4f6446600c..d3df488c48 100644
> > --- a/hw/i386/xen/xen-hvm.c
> > +++ b/hw/i386/xen/xen-hvm.c
> > @@ -614,7 +614,9 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
> > **ram_memory)
> >  
> >  state = g_new0(XenIOState, 1);
> >  
> > -xen_register_ioreq(state, max_cpus, &xen_memory_listener);
> > +xen_register_ioreq(state, max_cpus,
> > +   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
> > +   &xen_memory_listener);
> >  
> >  xen_is_stubdomain = xen_check_stubdomain(state->xenstore);
> >  
> > diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
> > index 3a9d6f981b..d8824cccf1 100644
> > --- a/hw/xen/xen-hvm-common.c
> > +++ b/hw/xen/xen-hvm-common.c
> > @@ -667,6 +667,8 @@ static int xen_map_ioreq_server(XenIOState *state)
> >  xen_pfn_t ioreq_pfn;
> >  xen_pfn_t bufioreq_pfn;
> >  evtchn_port_t bufioreq_evtchn;
> > +unsigned long num_frames = 1;
> > +unsigned long frame = 1;
> >  int rc;
> >  
> >  /*
> > @@ -675,59 +677,72 @@ static int xen_map_ioreq_server(XenIOState *state)
> >   */
> >  QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_bufioreq != 0);
> >  QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_ioreq(0) != 1);
> > +
> > +if (state->has_bufioreq) {
> > +frame = 0;
> > +num_frames = 2;
> > +}
> >  state->fres = xenforeignmemory_map_resource(xen_fmem, xen_domid,
> >   XENMEM_resource_ioreq_server,
> > - state->ioservid, 0, 2,
> > + state->ioservid,
> > + frame, num_frames,
> >   &addr,
> >   PROT_READ | PROT_WRITE, 0);
> >  if (state->fres != NULL) {
> >  trace_xen_map_resource_ioreq(state->ioservid, addr);
> > -state->buffered_io_page = addr;
> > -state->shared_page = addr + XC_PAGE_SIZE;
> > +state->shared_page = addr;
> > +if (state->has_bufioreq) {
> > +state->buffered_io_page = addr;
> > +state->shared_page = addr + XC_PAGE_SIZE;
> > +}
> >  } else if (errno != EOPNOTSUPP) {
> >  error_report("failed to map ioreq server resources: error %d 
> > handle=%p",
> >   errno, xen_xc);
> >  return -1;
> >  }
> >  
> > -rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
> > -   (state->shared_page == NULL) ?
> > -   &ioreq_pfn : NULL,
> > -   (state->buffered_io_page == NULL) ?
> > -   &bufioreq_pfn : NULL,
> > -   &bufioreq_evtchn);
> > -if (rc < 0) {
> > -error_report("failed to get ioreq server info: error %d handle=%p",
> > - errno, xen_xc);
> > -return rc;
> > -}
> > -
> > -if (state->shared_page == NULL) {
> > -trace_xen_map_ioreq_server_shared_page(ioreq_pfn);
> > +if (state->has_bufioreq) {
> > +rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
> > +   (state->shared_page == NULL) ?
> > +   &ioreq_pfn : NULL,
> > +   (state->buffered_io

Re: [PATCH v1 2/4] hw/xen: xenpvh: Disable buffered IOREQs for ARM

[Edgar E. Iglesias]

On Mon, Sep 16, 2024 at 04:47:43PM -0700, Stefano Stabellini wrote:
> On Mon, 16 Sep 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > Add a way to enable/disable buffered IOREQs for PVH machines
> > and disable them for ARM. ARM does not support buffered
> > IOREQ's nor the legacy way to map IOREQ info pages.
> > 
> > See the following for more details:
> > https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=2fbd7e609e1803ac5e5c26e22aa8e4b5a6cddbb1
> > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/ioreq.c;h=2e829d2e7f3760401b96fa7c930e2015fb1cf463;hb=HEAD#l138
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  hw/arm/xen-pvh.c| 3 +++
> >  hw/i386/xen/xen-pvh.c   | 3 +++
> >  hw/xen/xen-pvh-common.c | 2 +-
> >  include/hw/xen/xen-pvh-common.h | 3 +++
> >  4 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
> > index 04cb9855af..28af3910ea 100644
> > --- a/hw/arm/xen-pvh.c
> > +++ b/hw/arm/xen-pvh.c
> > @@ -66,6 +66,9 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
> > void *data)
> >   */
> >  mc->max_cpus = GUEST_MAX_VCPUS;
> >  
> > +/* Xen/ARM does not use buffered IOREQs.  */
> > +xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
> > +
> >  /* List of supported features known to work on PVH ARM.  */
> >  xpc->has_tpm = true;
> >  xpc->has_virtio_mmio = true;
> > diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
> > index 45645667e9..f1f02d3311 100644
> > --- a/hw/i386/xen/xen-pvh.c
> > +++ b/hw/i386/xen/xen-pvh.c
> > @@ -89,6 +89,9 @@ static void xen_pvh_machine_class_init(ObjectClass *oc, 
> > void *data)
> >  /* We have an implementation specific init to create CPU objects.  */
> >  xpc->init = xen_pvh_init;
> >  
> > +/* Enable buffered IOREQs.  */
> > +xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_ATOMIC;
> > +
> >  /*
> >   * PCI INTX routing.
> >   *
> > diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
> > index 08641fdcec..76a9b2b945 100644
> > --- a/hw/xen/xen-pvh-common.c
> > +++ b/hw/xen/xen-pvh-common.c
> > @@ -195,7 +195,7 @@ static void xen_pvh_init(MachineState *ms)
> >  
> >  xen_pvh_init_ram(s, sysmem);
> >  xen_register_ioreq(&s->ioreq, ms->smp.max_cpus,
> > -   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
> > +   xpc->handle_bufioreq,
> > &xen_memory_listener);
> >  
> >  if (s->cfg.virtio_mmio_num) {
> > diff --git a/include/hw/xen/xen-pvh-common.h 
> > b/include/hw/xen/xen-pvh-common.h
> > index bc09eea936..62c44a1ce7 100644
> > --- a/include/hw/xen/xen-pvh-common.h
> > +++ b/include/hw/xen/xen-pvh-common.h
> > @@ -43,6 +43,9 @@ struct XenPVHMachineClass {
> >   */
> >  int (*set_pci_link_route)(uint8_t line, uint8_t irq);
> >  
> > +/* Allow implementations to optionally enable buffered ioreqs.  */
> > +int handle_bufioreq;
> 
> Looking at the corresponding Xen interface this field is uint8_t. I
> think it would be better to use the same type here and also as a
> parameter to xen_register_ioreq in QEMU
>

Thanks Stefano,

I chose int because the interface to the Xen libs uses int:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/include/xendevicemodel.h;h=797e0c6b2961926a68cd96b8ff5e1627067903ac;hb=HEAD#l40

But yes, the hypercall interface uses uint8_t, I'm happy to change it to
uint8_t in the next version!

Cheers,
Edgar

[PATCH v1 3/4] hw/xen: xenpvh: Add pci-intx-irq-base property

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-pvh-common.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 76a9b2b945..218ac851cf 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -218,6 +218,11 @@ static void xen_pvh_init(MachineState *ms)
 error_report("pci-ecam-size only supports values 0 or 0x1000");
 exit(EXIT_FAILURE);
 }
+if (!s->cfg.pci_intx_irq_base) {
+error_report("PCI enabled but pci-intx-irq-base not set");
+exit(EXIT_FAILURE);
+}
+
 xenpvh_gpex_init(s, xpc, sysmem);
 }
 
@@ -273,6 +278,30 @@ XEN_PVH_PROP_MEMMAP(pci_ecam)
 XEN_PVH_PROP_MEMMAP(pci_mmio)
 XEN_PVH_PROP_MEMMAP(pci_mmio_high)
 
+static void xen_pvh_set_pci_intx_irq_base(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+XenPVHMachineState *xp = XEN_PVH_MACHINE(obj);
+uint32_t value;
+
+if (!visit_type_uint32(v, name, &value, errp)) {
+return;
+}
+
+xp->cfg.pci_intx_irq_base = value;
+}
+
+static void xen_pvh_get_pci_intx_irq_base(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+XenPVHMachineState *xp = XEN_PVH_MACHINE(obj);
+uint32_t value = xp->cfg.pci_intx_irq_base;
+
+visit_type_uint32(v, name, &value, errp);
+}
+
 void xen_pvh_class_setup_common_props(XenPVHMachineClass *xpc)
 {
 ObjectClass *oc = OBJECT_CLASS(xpc);
@@ -318,6 +347,13 @@ do {   
   \
 OC_MEMMAP_PROP(oc, "pci-ecam", pci_ecam);
 OC_MEMMAP_PROP(oc, "pci-mmio", pci_mmio);
 OC_MEMMAP_PROP(oc, "pci-mmio-high", pci_mmio_high);
+
+object_class_property_add(oc, "pci-intx-irq-base", "uint32_t",
+  xen_pvh_get_pci_intx_irq_base,
+  xen_pvh_set_pci_intx_irq_base,
+  NULL, NULL);
+object_class_property_set_description(oc, "pci-intx-irq-base",
+  "Set PCI INTX interrupt base line.");
 }
 
 #ifdef CONFIG_TPM
-- 
2.43.0

[PATCH v1 4/4] hw/arm: xenpvh: Enable PCI for ARM PVH

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable PCI support for the ARM Xen PVH machine.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen-pvh.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
index 28af3910ea..33f0dd5982 100644
--- a/hw/arm/xen-pvh.c
+++ b/hw/arm/xen-pvh.c
@@ -39,6 +39,16 @@ static void xen_arm_instance_init(Object *obj)
  VIRTIO_MMIO_DEV_SIZE };
 }
 
+static void xen_pvh_set_pci_intx_irq(void *opaque, int intx_irq, int level)
+{
+XenPVHMachineState *s = XEN_PVH_MACHINE(opaque);
+int irq = s->cfg.pci_intx_irq_base + intx_irq;
+
+if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
+error_report("xendevicemodel_set_pci_intx_level failed");
+}
+}
+
 static void xen_arm_machine_class_init(ObjectClass *oc, void *data)
 {
 XenPVHMachineClass *xpc = XEN_PVH_MACHINE_CLASS(oc);
@@ -69,7 +79,11 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void 
*data)
 /* Xen/ARM does not use buffered IOREQs.  */
 xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
 
+/* PCI INTX delivery.  */
+xpc->set_pci_intx_irq = xen_pvh_set_pci_intx_irq;
+
 /* List of supported features known to work on PVH ARM.  */
+xpc->has_pci = true;
 xpc->has_tpm = true;
 xpc->has_virtio_mmio = true;
 
-- 
2.43.0

[PATCH v1 0/4] hw/arm: xenpvh: Enable PCI for ARM PVH

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Enable PCI on the ARM PVH machine. First we add a way to control the use
of buffered IOREQ's since those are not supported on Xen/ARM.
Finally we enable the PCI support.

I've published some instructions on how to try this including the work in
progress Xen side of the PVH PCI support:
https://github.com/edgarigl/docs/blob/master/xen/pvh/virtio-pci-dom0less.md

Cheers,
Edgar

Edgar E. Iglesias (4):
  xen: Expose handle_bufioreq in xen_register_ioreq
  hw/xen: xenpvh: Disable buffered IOREQs for ARM
  hw/xen: xenpvh: Add pci-intx-irq-base property
  hw/arm: xenpvh: Enable PCI for ARM PVH

 hw/arm/xen-pvh.c|  17 ++
 hw/i386/xen/xen-hvm.c   |   4 +-
 hw/i386/xen/xen-pvh.c   |   3 +
 hw/xen/xen-hvm-common.c | 100 +++-
 hw/xen/xen-pvh-common.c |  40 -
 include/hw/xen/xen-hvm-common.h |   3 +
 include/hw/xen/xen-pvh-common.h |   3 +
 include/hw/xen/xen_native.h |   3 +-
 8 files changed, 129 insertions(+), 44 deletions(-)

-- 
2.43.0

[PATCH v1 1/4] xen: Expose handle_bufioreq in xen_register_ioreq

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Expose handle_bufioreq in xen_register_ioreq().
This is to allow machines to enable or disable buffered ioreqs.

No functional change since all callers still set it to
HVM_IOREQSRV_BUFIOREQ_ATOMIC.

Signed-off-by: Edgar E. Iglesias 
---
 hw/i386/xen/xen-hvm.c   |   4 +-
 hw/xen/xen-hvm-common.c | 100 +++-
 hw/xen/xen-pvh-common.c |   4 +-
 include/hw/xen/xen-hvm-common.h |   3 +
 include/hw/xen/xen_native.h |   3 +-
 5 files changed, 70 insertions(+), 44 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 4f6446600c..d3df488c48 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -614,7 +614,9 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
**ram_memory)
 
 state = g_new0(XenIOState, 1);
 
-xen_register_ioreq(state, max_cpus, &xen_memory_listener);
+xen_register_ioreq(state, max_cpus,
+   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
+   &xen_memory_listener);
 
 xen_is_stubdomain = xen_check_stubdomain(state->xenstore);
 
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 3a9d6f981b..d8824cccf1 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -667,6 +667,8 @@ static int xen_map_ioreq_server(XenIOState *state)
 xen_pfn_t ioreq_pfn;
 xen_pfn_t bufioreq_pfn;
 evtchn_port_t bufioreq_evtchn;
+unsigned long num_frames = 1;
+unsigned long frame = 1;
 int rc;
 
 /*
@@ -675,59 +677,72 @@ static int xen_map_ioreq_server(XenIOState *state)
  */
 QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_bufioreq != 0);
 QEMU_BUILD_BUG_ON(XENMEM_resource_ioreq_server_frame_ioreq(0) != 1);
+
+if (state->has_bufioreq) {
+frame = 0;
+num_frames = 2;
+}
 state->fres = xenforeignmemory_map_resource(xen_fmem, xen_domid,
  XENMEM_resource_ioreq_server,
- state->ioservid, 0, 2,
+ state->ioservid,
+ frame, num_frames,
  &addr,
  PROT_READ | PROT_WRITE, 0);
 if (state->fres != NULL) {
 trace_xen_map_resource_ioreq(state->ioservid, addr);
-state->buffered_io_page = addr;
-state->shared_page = addr + XC_PAGE_SIZE;
+state->shared_page = addr;
+if (state->has_bufioreq) {
+state->buffered_io_page = addr;
+state->shared_page = addr + XC_PAGE_SIZE;
+}
 } else if (errno != EOPNOTSUPP) {
 error_report("failed to map ioreq server resources: error %d 
handle=%p",
  errno, xen_xc);
 return -1;
 }
 
-rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
-   (state->shared_page == NULL) ?
-   &ioreq_pfn : NULL,
-   (state->buffered_io_page == NULL) ?
-   &bufioreq_pfn : NULL,
-   &bufioreq_evtchn);
-if (rc < 0) {
-error_report("failed to get ioreq server info: error %d handle=%p",
- errno, xen_xc);
-return rc;
-}
-
-if (state->shared_page == NULL) {
-trace_xen_map_ioreq_server_shared_page(ioreq_pfn);
+if (state->has_bufioreq) {
+rc = xen_get_ioreq_server_info(xen_domid, state->ioservid,
+   (state->shared_page == NULL) ?
+   &ioreq_pfn : NULL,
+   (state->buffered_io_page == NULL) ?
+   &bufioreq_pfn : NULL,
+   &bufioreq_evtchn);
+if (rc < 0) {
+error_report("failed to get ioreq server info: error %d handle=%p",
+errno, xen_xc);
+return rc;
+}
 
-state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
-  PROT_READ | PROT_WRITE,
-  1, &ioreq_pfn, NULL);
 if (state->shared_page == NULL) {
-error_report("map shared IO page returned error %d handle=%p",
- errno, xen_xc);
+trace_xen_map_ioreq_server_shared_page(ioreq_pfn);
+
+state->shared_page = xenforeignmemory_map(xen_fmem, xen_domid,
+  PROT_READ | PROT_WRITE,
+  1, &ioreq_pfn, NULL);
+if (state->shared_page == NULL) {
+

[PATCH v1 2/4] hw/xen: xenpvh: Disable buffered IOREQs for ARM

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add a way to enable/disable buffered IOREQs for PVH machines
and disable them for ARM. ARM does not support buffered
IOREQ's nor the legacy way to map IOREQ info pages.

See the following for more details:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=2fbd7e609e1803ac5e5c26e22aa8e4b5a6cddbb1
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/ioreq.c;h=2e829d2e7f3760401b96fa7c930e2015fb1cf463;hb=HEAD#l138

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen-pvh.c| 3 +++
 hw/i386/xen/xen-pvh.c   | 3 +++
 hw/xen/xen-pvh-common.c | 2 +-
 include/hw/xen/xen-pvh-common.h | 3 +++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/arm/xen-pvh.c b/hw/arm/xen-pvh.c
index 04cb9855af..28af3910ea 100644
--- a/hw/arm/xen-pvh.c
+++ b/hw/arm/xen-pvh.c
@@ -66,6 +66,9 @@ static void xen_arm_machine_class_init(ObjectClass *oc, void 
*data)
  */
 mc->max_cpus = GUEST_MAX_VCPUS;
 
+/* Xen/ARM does not use buffered IOREQs.  */
+xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_OFF;
+
 /* List of supported features known to work on PVH ARM.  */
 xpc->has_tpm = true;
 xpc->has_virtio_mmio = true;
diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
index 45645667e9..f1f02d3311 100644
--- a/hw/i386/xen/xen-pvh.c
+++ b/hw/i386/xen/xen-pvh.c
@@ -89,6 +89,9 @@ static void xen_pvh_machine_class_init(ObjectClass *oc, void 
*data)
 /* We have an implementation specific init to create CPU objects.  */
 xpc->init = xen_pvh_init;
 
+/* Enable buffered IOREQs.  */
+xpc->handle_bufioreq = HVM_IOREQSRV_BUFIOREQ_ATOMIC;
+
 /*
  * PCI INTX routing.
  *
diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 08641fdcec..76a9b2b945 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -195,7 +195,7 @@ static void xen_pvh_init(MachineState *ms)
 
 xen_pvh_init_ram(s, sysmem);
 xen_register_ioreq(&s->ioreq, ms->smp.max_cpus,
-   HVM_IOREQSRV_BUFIOREQ_ATOMIC,
+   xpc->handle_bufioreq,
&xen_memory_listener);
 
 if (s->cfg.virtio_mmio_num) {
diff --git a/include/hw/xen/xen-pvh-common.h b/include/hw/xen/xen-pvh-common.h
index bc09eea936..62c44a1ce7 100644
--- a/include/hw/xen/xen-pvh-common.h
+++ b/include/hw/xen/xen-pvh-common.h
@@ -43,6 +43,9 @@ struct XenPVHMachineClass {
  */
 int (*set_pci_link_route)(uint8_t line, uint8_t irq);
 
+/* Allow implementations to optionally enable buffered ioreqs.  */
+int handle_bufioreq;
+
 /*
  * Each implementation can optionally enable features that it
  * supports and are known to work.
-- 
2.43.0

Re: [PATCH v2 00/15] target/cris: Remove the deprecated CRIS target

[Edgar E. Iglesias]

On Mon, Sep 9, 2024 at 7:25 AM Philippe Mathieu-Daudé 
wrote:

> Hi Edgar,
>
> On 4/9/24 16:35, Philippe Mathieu-Daudé wrote:
> > Since v1:
> > - Split in smaller patches (pm215)
> >
> > The CRIS target is deprecated since v9.0 (commit
> > c7bbef40234 "docs: mark CRIS support as deprecated").
> >
> > Remove:
> > - Buildsys / CI infra
> > - User emulation
> > - System emulation (axis-dev88 machine and ETRAX devices)
> > - Tests
>
> You acked the deprecation commit (c7bbef4023).
> No objection for the removal? I'd rather have your
> explicit Acked-by before merging this.
>
>
Hi Phil,

Yes, sorry, I haven't had time to review each patch but:
Acked-by: Edgar E. Iglesias 

Cheers,
Edgar




> Thanks,
>
> Phil.
>
> > Philippe Mathieu-Daudé (15):
> >tests/tcg: Remove CRIS libc test files
> >tests/tcg: Remove CRIS bare test files
> >buildsys: Remove CRIS cross container
> >linux-user: Remove support for CRIS target
> >hw/cris: Remove the axis-dev88 machine
> >hw/cris: Remove image loader helper
> >hw/intc: Remove TYPE_ETRAX_FS_PIC device
> >hw/char: Remove TYPE_ETRAX_FS_SERIAL device
> >hw/net: Remove TYPE_ETRAX_FS_ETH device
> >hw/dma: Remove ETRAX_FS DMA device
> >hw/timer: Remove TYPE_ETRAX_FS_TIMER device
> >system: Remove support for CRIS target
> >target/cris: Remove the deprecated CRIS target
> >disas: Remove CRIS disassembler
> >seccomp: Remove check for CRIS host
>
>

[PULL v1 03/12] hw/arm: xenpvh: Tweak machine description

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Tweak machine description to better express that this is
a Xen PVH machine for ARM.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/xen_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 766a194fa1..5f75cc3779 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -216,7 +216,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 {
 
 MachineClass *mc = MACHINE_CLASS(oc);
-mc->desc = "Xen Para-virtualized PC";
+mc->desc = "Xen PVH ARM machine";
 mc->init = xen_arm_init;
 mc->max_cpus = 1;
 mc->default_machine_opts = "accel=xen";
-- 
2.43.0

[PULL v1 06/12] hw/arm: xenpvh: Move stubbed functions to xen-stubs.c

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/meson.build |  5 -
 hw/arm/xen-stubs.c | 32 
 hw/arm/xen_arm.c   | 20 
 3 files changed, 36 insertions(+), 21 deletions(-)
 create mode 100644 hw/arm/xen-stubs.c

diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 0c07ab522f..074612b40c 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -59,7 +59,10 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true: 
files('fsl-imx7.c', 'mcimx7d-sabre.
 arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
 arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 
'mcimx6ul-evk.c'))
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
-arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
+arm_ss.add(when: 'CONFIG_XEN', if_true: files(
+  'xen-stubs.c',
+  'xen_arm.c',
+))
 
 system_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
 system_ss.add(when: 'CONFIG_CHEETAH', if_true: files('palm.c'))
diff --git a/hw/arm/xen-stubs.c b/hw/arm/xen-stubs.c
new file mode 100644
index 00..4ac6a56a96
--- /dev/null
+++ b/hw/arm/xen-stubs.c
@@ -0,0 +1,32 @@
+/*
+ * Stubs for unimplemented Xen functions for ARM.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/qapi-commands-migration.h"
+#include "hw/boards.h"
+#include "sysemu/sysemu.h"
+#include "hw/xen/xen-hvm-common.h"
+#include "hw/xen/arch_hvm.h"
+
+void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+{
+hw_error("Invalid ioreq type 0x%x\n", req->type);
+return;
+}
+
+void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
+ bool add)
+{
+}
+
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+}
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+}
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 16b3f00992..f0868e7be5 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -115,26 +115,6 @@ static void xen_init_ram(MachineState *machine)
 memory_region_add_subregion(sysmem, XEN_GRANT_ADDR_OFF, &xen_grants);
 }
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
-{
-hw_error("Invalid ioreq type 0x%x\n", req->type);
-
-return;
-}
-
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
- bool add)
-{
-}
-
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
-{
-}
-
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-}
-
 #ifdef CONFIG_TPM
 static void xen_enable_tpm(XenArmState *xam)
 {
-- 
2.43.0

[PULL v1 10/12] hw/xen: pvh-common: Add support for creating PCIe/GPEX

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add support for optionally creating a PCIe/GPEX controller.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/xen/xen-pvh-common.c | 76 +
 include/hw/xen/xen-pvh-common.h | 29 +
 2 files changed, 105 insertions(+)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 295f920442..28d7168446 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -122,6 +122,64 @@ static void xen_enable_tpm(XenPVHMachineState *s)
 }
 #endif
 
+/*
+ * We use the GPEX PCIe controller with its internal INTX PCI interrupt
+ * swizzling. This swizzling is emulated in QEMU and routes all INTX
+ * interrupts from endpoints down to only 4 INTX interrupts.
+ * See include/hw/pci/pci.h : pci_swizzle()
+ */
+static inline void xenpvh_gpex_init(XenPVHMachineState *s,
+XenPVHMachineClass *xpc,
+MemoryRegion *sysmem)
+{
+MemoryRegion *ecam_reg;
+MemoryRegion *mmio_reg;
+DeviceState *dev;
+int i;
+
+object_initialize_child(OBJECT(s), "gpex", &s->pci.gpex,
+TYPE_GPEX_HOST);
+dev = DEVICE(&s->pci.gpex);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_add_subregion(sysmem, s->cfg.pci_ecam.base, ecam_reg);
+
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+
+if (s->cfg.pci_mmio.size) {
+memory_region_init_alias(&s->pci.mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg,
+ s->cfg.pci_mmio.base, s->cfg.pci_mmio.size);
+memory_region_add_subregion(sysmem, s->cfg.pci_mmio.base,
+&s->pci.mmio_alias);
+}
+
+if (s->cfg.pci_mmio_high.size) {
+memory_region_init_alias(&s->pci.mmio_high_alias, OBJECT(dev),
+"pcie-mmio-high",
+mmio_reg, s->cfg.pci_mmio_high.base, 
s->cfg.pci_mmio_high.size);
+memory_region_add_subregion(sysmem, s->cfg.pci_mmio_high.base,
+&s->pci.mmio_high_alias);
+}
+
+/*
+ * PVH implementations with PCI enabled must provide set_pci_intx_irq()
+ * and optionally an implementation of set_pci_link_route().
+ */
+assert(xpc->set_pci_intx_irq);
+
+for (i = 0; i < GPEX_NUM_IRQS; i++) {
+qemu_irq irq = qemu_allocate_irq(xpc->set_pci_intx_irq, s, i);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
+gpex_set_irq_num(GPEX_HOST(dev), i, s->cfg.pci_intx_irq_base + i);
+if (xpc->set_pci_link_route) {
+xpc->set_pci_link_route(i, s->cfg.pci_intx_irq_base + i);
+}
+}
+}
+
 static void xen_pvh_init(MachineState *ms)
 {
 XenPVHMachineState *s = XEN_PVH_MACHINE(ms);
@@ -152,6 +210,15 @@ static void xen_pvh_init(MachineState *ms)
 }
 #endif
 
+/* Non-zero pci-ecam-size enables PCI.  */
+if (s->cfg.pci_ecam.size) {
+if (s->cfg.pci_ecam.size != 256 * MiB) {
+error_report("pci-ecam-size only supports values 0 or 0x1000");
+exit(EXIT_FAILURE);
+}
+xenpvh_gpex_init(s, xpc, sysmem);
+}
+
 /* Call the implementation specific init.  */
 if (xpc->init) {
 xpc->init(ms);
@@ -200,6 +267,9 @@ XEN_PVH_PROP_MEMMAP(ram_high)
 /* TPM only has a base-addr option.  */
 XEN_PVH_PROP_MEMMAP_BASE(tpm)
 XEN_PVH_PROP_MEMMAP(virtio_mmio)
+XEN_PVH_PROP_MEMMAP(pci_ecam)
+XEN_PVH_PROP_MEMMAP(pci_mmio)
+XEN_PVH_PROP_MEMMAP(pci_mmio_high)
 
 void xen_pvh_class_setup_common_props(XenPVHMachineClass *xpc)
 {
@@ -242,6 +312,12 @@ do {   
   \
 OC_MEMMAP_PROP(oc, "virtio-mmio", virtio_mmio);
 }
 
+if (xpc->has_pci) {
+OC_MEMMAP_PROP(oc, "pci-ecam", pci_ecam);
+OC_MEMMAP_PROP(oc, "pci-mmio", pci_mmio);
+OC_MEMMAP_PROP(oc, "pci-mmio-high", pci_mmio_high);
+}
+
 #ifdef CONFIG_TPM
 if (xpc->has_tpm) {
 object_class_property_add(oc, "tpm-base-addr", "uint64_t",
diff --git a/include/hw/xen/xen-pvh-common.h b/include/hw/xen/xen-pvh-common.h
index 77fd98b9fe..bc09eea936 100644
--- a/include/hw/xen/xen-pvh-common.h
+++ b/include/hw/xen/xen-pvh-common.h
@@ -25,10 +25,29 @@ struct XenPVHMachineClass {
 /* PVH implementation specific init.  */
 void (*init)(MachineState *state);
 
+/*
+ * set_pci_intx_irq - Deliver INTX irqs to the guest.
+ *
+ * @opaque: pointer to XenPVHMachineState.
+ * @irq: IRQ after swizzling, between 0-3.
+ * @level: IRQ level.
+ */
+void (*set_pci_intx_irq)(vo

[PULL v1 05/12] hw/arm: xenpvh: Remove double-negation in warning

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/xen_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index fda65d0d8d..16b3f00992 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -165,7 +165,7 @@ static void xen_arm_init(MachineState *machine)
 xam->state =  g_new0(XenIOState, 1);
 
 if (machine->ram_size == 0) {
-warn_report("%s non-zero ram size not specified. QEMU machine started"
+warn_report("%s: ram size not specified. QEMU machine started"
 " without IOREQ (no emulated devices including virtio)",
 MACHINE_CLASS(object_get_class(OBJECT(machine)))->desc);
 return;
-- 
2.43.0

[PULL v1 09/12] hw/arm: xenpvh: Reverse virtio-mmio creation order

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

We've been creating the virtio-mmio devices in forwards order
but since the qbus lists prepend (rather than append) entries,
the virtio busses end up with decreasing base address order.

Xen enables virtio-mmio nodes in forwards order so there's been
a missmatch. So far, we've been working around this with an
out-of-tree patch to Xen.

This reverses the order making sure the virtio busses end up
ordered with increasing base addresses avoiding the need to
patch Xen.

Signed-off-by: Edgar E. Iglesias 
Acked-by: Stefano Stabellini 
---
 hw/xen/xen-pvh-common.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 880e8143d7..295f920442 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -75,7 +75,18 @@ static void 
xen_create_virtio_mmio_devices(XenPVHMachineState *s)
 {
 int i;
 
-for (i = 0; i < s->cfg.virtio_mmio_num; i++) {
+/*
+ * We create the transports in reverse order. Since qbus_realize()
+ * prepends (not appends) new child buses, the decrementing loop below will
+ * create a list of virtio-mmio buses with increasing base addresses.
+ *
+ * When a -device option is processed from the command line,
+ * qbus_find_recursive() picks the next free virtio-mmio bus in forwards
+ * order.
+ *
+ * This is what the Xen tools expect.
+ */
+for (i = s->cfg.virtio_mmio_num - 1; i >= 0; i--) {
 hwaddr base = s->cfg.virtio_mmio.base + i * s->cfg.virtio_mmio.size;
 qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
  s->cfg.virtio_mmio_irq_base + i);
-- 
2.43.0

[PULL v1 07/12] hw/arm: xenpvh: Break out a common PVH machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Break out a common Xen PVH machine in preparation for
adding a x86 Xen PVH machine.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/trace-events |   5 -
 hw/arm/xen_arm.c| 198 +++
 hw/xen/meson.build  |   1 +
 hw/xen/trace-events |   4 +
 hw/xen/xen-pvh-common.c | 275 
 include/hw/xen/xen-pvh-common.h |  59 +++
 6 files changed, 358 insertions(+), 184 deletions(-)
 create mode 100644 hw/xen/xen-pvh-common.c
 create mode 100644 include/hw/xen/xen-pvh-common.h

diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index be6c8f720b..c64ad344bd 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -68,10 +68,5 @@ z2_aer915_send_too_long(int8_t msg) "message too long (%i 
bytes)"
 z2_aer915_send(uint8_t reg, uint8_t value) "reg %d value 0x%02x"
 z2_aer915_event(int8_t event, int8_t len) "i2c event =0x%x len=%d bytes"
 
-# xen_arm.c
-xen_create_virtio_mmio_devices(int i, int irq, uint64_t base) "Created 
virtio-mmio device %d: irq %d base 0x%"PRIx64
-xen_init_ram(uint64_t machine_ram_size) "Initialized xen ram with size 
0x%"PRIx64
-xen_enable_tpm(uint64_t addr) "Connected tpmdev at address 0x%"PRIx64
-
 # bcm2838.c
 bcm2838_gic_set_irq(int irq, int level) "gic irq:%d lvl:%d"
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index f0868e7be5..04cb9855af 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -7,44 +7,12 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qapi/qapi-commands-migration.h"
-#include "qapi/visitor.h"
 #include "hw/boards.h"
-#include "hw/irq.h"
-#include "hw/sysbus.h"
-#include "sysemu/block-backend.h"
-#include "sysemu/tpm_backend.h"
 #include "sysemu/sysemu.h"
-#include "hw/xen/xen-hvm-common.h"
-#include "sysemu/tpm.h"
+#include "hw/xen/xen-pvh-common.h"
 #include "hw/xen/arch_hvm.h"
-#include "trace.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
-OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
-
-static const MemoryListener xen_memory_listener = {
-.region_add = xen_region_add,
-.region_del = xen_region_del,
-.log_start = NULL,
-.log_stop = NULL,
-.log_sync = NULL,
-.log_global_start = NULL,
-.log_global_stop = NULL,
-.priority = MEMORY_LISTENER_PRIORITY_ACCEL,
-};
-
-struct XenArmState {
-/*< private >*/
-MachineState parent;
-
-XenIOState *state;
-
-struct {
-uint64_t tpm_base_addr;
-} cfg;
-};
-
-static MemoryRegion ram_lo, ram_hi;
 
 /*
  * VIRTIO_MMIO_DEV_SIZE is imported from tools/libs/light/libxl_arm.c under Xen
@@ -57,147 +25,26 @@ static MemoryRegion ram_lo, ram_hi;
 #define NR_VIRTIO_MMIO_DEVICES   \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
 
-static void xen_set_irq(void *opaque, int irq, int level)
-{
-if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
-error_report("xendevicemodel_set_irq_level failed");
-}
-}
-
-static void xen_create_virtio_mmio_devices(XenArmState *xam)
-{
-int i;
-
-for (i = 0; i < NR_VIRTIO_MMIO_DEVICES; i++) {
-hwaddr base = GUEST_VIRTIO_MMIO_BASE + i * VIRTIO_MMIO_DEV_SIZE;
-qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
- GUEST_VIRTIO_MMIO_SPI_FIRST + i);
-
-sysbus_create_simple("virtio-mmio", base, irq);
-
-trace_xen_create_virtio_mmio_devices(i,
- GUEST_VIRTIO_MMIO_SPI_FIRST + i,
- base);
-}
-}
-
-static void xen_init_ram(MachineState *machine)
+static void xen_arm_instance_init(Object *obj)
 {
-MemoryRegion *sysmem = get_system_memory();
-ram_addr_t block_len, ram_size[GUEST_RAM_BANKS];
-
-trace_xen_init_ram(machine->ram_size);
-if (machine->ram_size <= GUEST_RAM0_SIZE) {
-ram_size[0] = machine->ram_size;
-ram_size[1] = 0;
-block_len = GUEST_RAM0_BASE + ram_size[0];
-} else {
-ram_size[0] = GUEST_RAM0_SIZE;
-ram_size[1] = machine->ram_size - GUEST_RAM0_SIZE;
-block_len = GUEST_RAM1_BASE + ram_size[1];
-}
+XenPVHMachineState *s = XEN_PVH_MACHINE(obj);
 
-memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
-   &error_fatal);
+/* Default values.  */
+s->cfg.ram_low = (MemMapEntry) { GUEST_RAM0_BASE, GUEST_RAM0_SIZE };
+s->cfg.ram_high = (MemMapEntry) { GUEST_RAM1_BASE, GUEST_RAM1_SIZE };
 
-memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &xen_memory,
- GU

[PULL v1 04/12] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add SMP support for Xen PVH ARM guests.
Create ms->smp.max_cpus ioreq servers to handle hotplug.

Note that ms->smp.max_cpus will be passed to us by the
user (Xen tools) set to the guests maxvcpus.

The value in mc->max_cpus is an absolute maximum for the
-smp option and won't be used to setup ioreq servers unless
the user explicitly specifies it with -smp.

If the user doesn't pass -smp on the command-line, smp.cpus
and smp.max_cpus will default to 1.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/xen_arm.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 5f75cc3779..fda65d0d8d 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -173,7 +173,7 @@ static void xen_arm_init(MachineState *machine)
 
 xen_init_ram(machine);
 
-xen_register_ioreq(xam->state, machine->smp.cpus, &xen_memory_listener);
+xen_register_ioreq(xam->state, machine->smp.max_cpus, 
&xen_memory_listener);
 
 xen_create_virtio_mmio_devices(xam);
 
@@ -218,7 +218,26 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 MachineClass *mc = MACHINE_CLASS(oc);
 mc->desc = "Xen PVH ARM machine";
 mc->init = xen_arm_init;
-mc->max_cpus = 1;
+
+/*
+ * mc->max_cpus holds the MAX value allowed in the -smp command-line opts.
+ *
+ * 1. If users don't pass any -smp option:
+ *   ms->smp.cpus will default to 1.
+ *   ms->smp.max_cpus will default to 1.
+ *
+ * 2. If users pass -smp X:
+ *   ms->smp.cpus will be set to X.
+ *   ms->smp.max_cpus will also be set to X.
+ *
+ * 3. If users pass -smp X,maxcpus=Y:
+ *   ms->smp.cpus will be set to X.
+ *   ms->smp.max_cpus will be set to Y.
+ *
+ * In scenarios 2 and 3, if X or Y are set to something larger than
+ * mc->max_cpus, QEMU will bail out with an error message.
+ */
+mc->max_cpus = GUEST_MAX_VCPUS;
 mc->default_machine_opts = "accel=xen";
 /* Set explicitly here to make sure that real ram_size is passed */
 mc->default_ram_size = 0;
-- 
2.43.0

[PULL v1 08/12] hw/arm: xenpvh: Rename xen_arm.c -> xen-pvh.c

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Rename xen_arm.c -> xen-pvh.c to better express that this
is a PVH machine and to align with x86 HVM and future PVH
machine filenames:
hw/i386/xen/xen-hvm.c
hw/i386/xen/xen-pvh.c (in preparation)

No functional changes.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/meson.build  | 2 +-
 hw/arm/{xen_arm.c => xen-pvh.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename hw/arm/{xen_arm.c => xen-pvh.c} (100%)

diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 074612b40c..4059d0be2e 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -61,7 +61,7 @@ arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: 
files('fsl-imx6ul.c', 'mcimx6ul-e
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
 arm_ss.add(when: 'CONFIG_XEN', if_true: files(
   'xen-stubs.c',
-  'xen_arm.c',
+  'xen-pvh.c',
 ))
 
 system_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen-pvh.c
similarity index 100%
rename from hw/arm/xen_arm.c
rename to hw/arm/xen-pvh.c
-- 
2.43.0

[PULL v1 02/12] hw/arm: xenpvh: Update file header to use SPDX

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Update file header to use SPDX and remove stray empty
comment line.

No functional changes.

Signed-off-by: Edgar E. Iglesias 
Acked-by: Stefano Stabellini 
---
 hw/arm/xen_arm.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 6fad829ede..766a194fa1 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -1,24 +1,7 @@
 /*
  * QEMU ARM Xen PVH Machine
  *
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
+ * SPDX-License-Identifier: MIT
  */
 
 #include "qemu/osdep.h"
-- 
2.43.0

[PULL v1 11/12] hw/i386/xen: Add a Xen PVH x86 machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add a Xen PVH x86 machine based on the abstract PVH Machine.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/i386/xen/meson.build |   1 +
 hw/i386/xen/xen-pvh.c   | 121 
 2 files changed, 122 insertions(+)
 create mode 100644 hw/i386/xen/xen-pvh.c

diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 3f0df8bc07..c73c62b8e3 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -4,6 +4,7 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files(
 ))
 i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-hvm.c',
+  'xen-pvh.c',
 ))
 
 i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files(
diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
new file mode 100644
index 00..45645667e9
--- /dev/null
+++ b/hw/i386/xen/xen-pvh.c
@@ -0,0 +1,121 @@
+/*
+ * QEMU Xen PVH x86 Machine
+ *
+ * Copyright (c) 2024 Advanced Micro Devices, Inc.
+ * Written by Edgar E. Iglesias 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "hw/boards.h"
+#include "sysemu/sysemu.h"
+#include "hw/xen/arch_hvm.h"
+#include 
+#include "hw/xen/xen-pvh-common.h"
+
+#define TYPE_XEN_PVH_X86  MACHINE_TYPE_NAME("xenpvh")
+OBJECT_DECLARE_SIMPLE_TYPE(XenPVHx86State, XEN_PVH_X86)
+
+struct XenPVHx86State {
+/*< private >*/
+XenPVHMachineState parent;
+
+DeviceState **cpu;
+};
+
+static DeviceState *xen_pvh_cpu_new(MachineState *ms,
+int64_t apic_id)
+{
+Object *cpu = object_new(ms->cpu_type);
+
+object_property_add_child(OBJECT(ms), "cpu[*]", cpu);
+object_property_set_uint(cpu, "apic-id", apic_id, &error_fatal);
+qdev_realize(DEVICE(cpu), NULL, &error_fatal);
+object_unref(cpu);
+
+return DEVICE(cpu);
+}
+
+static void xen_pvh_init(MachineState *ms)
+{
+XenPVHx86State *xp = XEN_PVH_X86(ms);
+int i;
+
+/* Create dummy cores. This will indirectly create the APIC MSI window.  */
+xp->cpu = g_malloc(sizeof xp->cpu[0] * ms->smp.max_cpus);
+for (i = 0; i < ms->smp.max_cpus; i++) {
+xp->cpu[i] = xen_pvh_cpu_new(ms, i);
+}
+}
+
+static void xen_pvh_instance_init(Object *obj)
+{
+XenPVHMachineState *s = XEN_PVH_MACHINE(obj);
+
+/* Default values.  */
+s->cfg.ram_low = (MemMapEntry) { 0x0, 0x8000U };
+s->cfg.ram_high = (MemMapEntry) { 0xC0ULL, 0x40ULL };
+s->cfg.pci_intx_irq_base = 16;
+}
+
+/*
+ * Deliver INTX interrupts to Xen guest.
+ */
+static void xen_pvh_set_pci_intx_irq(void *opaque, int irq, int level)
+{
+/*
+ * Since QEMU emulates all of the swizziling
+ * We don't want Xen to do any additional swizzling in
+ * xen_set_pci_intx_level() so we always set device to 0.
+ */
+if (xen_set_pci_intx_level(xen_domid, 0, 0, 0, irq, level)) {
+error_report("xendevicemodel_set_pci_intx_level failed");
+}
+}
+
+static void xen_pvh_machine_class_init(ObjectClass *oc, void *data)
+{
+XenPVHMachineClass *xpc = XEN_PVH_MACHINE_CLASS(oc);
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc = "Xen PVH x86 machine";
+mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
+
+/* mc->max_cpus holds the MAX value allowed in the -smp cmd-line opts. */
+mc->max_cpus = HVM_MAX_VCPUS;
+
+/* We have an implementation specific init to create CPU objects.  */
+xpc->init = xen_pvh_init;
+
+/*
+ * PCI INTX routing.
+ *
+ * We describe the mapping between the 4 INTX interrupt and GSIs
+ * using xen_set_pci_link_route(). xen_pvh_set_pci_intx_irq is
+ * used to deliver the interrupt.
+ */
+xpc->set_pci_intx_irq = xen_pvh_set_pci_intx_irq;
+xpc->set_pci_link_route = xen_set_pci_link_route;
+
+/* List of supported features known to work on PVH x86.  */
+xpc->has_pci = true;
+
+xen_pvh_class_setup_common_props(xpc);
+}
+
+static const TypeInfo xen_pvh_x86_machine_type = {
+.name = TYPE_XEN_PVH_X86,
+.parent = TYPE_XEN_PVH_MACHINE,
+.class_init = xen_pvh_machine_class_init,
+.instance_init = xen_pvh_instance_init,
+.instance_size = sizeof(XenPVHx86State),
+};
+
+static void xen_pvh_machine_register_types(void)
+{
+type_register_static(&xen_pvh_x86_machine_type);
+}
+
+type_init(xen_pvh_machine_register_types)
-- 
2.43.0

[PULL v1 12/12] docs/system/i386: xenpvh: Add a basic description

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 MAINTAINERS |  1 +
 docs/system/i386/xenpvh.rst | 49 +
 docs/system/target-i386.rst |  1 +
 3 files changed, 51 insertions(+)
 create mode 100644 docs/system/i386/xenpvh.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index c2fb0c2f42..c14ac014e2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -560,6 +560,7 @@ F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
 F: docs/system/arm/xenpvh.rst
+F: docs/system/i386/xenpvh.rst
 
 Guest CPU Cores (NVMM)
 --
diff --git a/docs/system/i386/xenpvh.rst b/docs/system/i386/xenpvh.rst
new file mode 100644
index 00..354250f073
--- /dev/null
+++ b/docs/system/i386/xenpvh.rst
@@ -0,0 +1,49 @@
+Xen PVH machine (``xenpvh``)
+=
+
+Xen supports a spectrum of types of guests that vary in how they depend
+on HW virtualization features, emulation models and paravirtualization.
+PVH is a mode that uses HW virtualization features (like HVM) but tries
+to avoid emulation models and instead use passthrough or
+paravirtualized devices.
+
+QEMU can be used to provide PV virtio devices on an emulated PCIe controller.
+That is the purpose of this minimal machine.
+
+Supported devices
+-
+
+The x86 Xen PVH QEMU machine provide the following devices:
+
+- RAM
+- GPEX host bridge
+- virtio-pci devices
+
+The idea is to only connect virtio-pci devices but in theory any compatible
+PCI device model will work depending on Xen and guest support.
+
+Running
+---
+
+The Xen tools will typically construct a command-line and launch QEMU
+for you when needed. But here's an example of what it can look like in
+case you need to construct one manually:
+
+.. code-block:: console
+
+qemu-system-i386 -xen-domid 3 -no-shutdown\
+  -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-3,server=on,wait=off \
+  -mon chardev=libxl-cmd,mode=control \
+  -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-3,server=on,wait=off \
+  -mon chardev=libxenstat-cmd,mode=control\
+  -nodefaults \
+  -no-user-config \
+  -xen-attach -name g0\
+  -vnc none   \
+  -display none   \
+  -device virtio-net-pci,id=nic0,netdev=net0,mac=00:16:3e:5c:81:78 \
+  -netdev 
type=tap,id=net0,ifname=vif3.0-emu,br=xenbr0,script=no,downscript=no \
+  -smp 4,maxcpus=4\
+  -nographic  \
+  -machine 
xenpvh,ram-low-base=0,ram-low-size=2147483648,ram-high-base=4294967296,ram-high-size=2147483648,pci-ecam-base=824633720832,pci-ecam-size=268435456,pci-mmio-base=4026531840,pci-mmio-size=33554432,pci-mmio-high-base=824902156288,pci-mmio-high-size=68719476736
 \
+  -m 4096
diff --git a/docs/system/target-i386.rst b/docs/system/target-i386.rst
index 1b8a1f248a..23e84e3ba7 100644
--- a/docs/system/target-i386.rst
+++ b/docs/system/target-i386.rst
@@ -26,6 +26,7 @@ Architectural features
i386/cpu
i386/hyperv
i386/xen
+   i386/xenpvh
i386/kvm-pv
i386/sgx
i386/amd-memory-encryption
-- 
2.43.0

[PULL v1 01/12] MAINTAINERS: Add docs/system/arm/xenpvh.rst

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Acked-by: Stefano Stabellini 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3584d6a6c6..c2fb0c2f42 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -559,6 +559,7 @@ F: include/hw/xen/
 F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
+F: docs/system/arm/xenpvh.rst
 
 Guest CPU Cores (NVMM)
 --
-- 
2.43.0

[PULL v1 00/12] Xen queue

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

The following changes since commit e638d685ec2a0700fb9529cbd1b2823ac4120c53:

  Open 9.2 development tree (2024-09-03 09:18:43 -0700)

are available in the Git repository at:

  https://gitlab.com/edgar.iglesias/qemu.git 
tags/edgar/xen-queue-2024-09-04.for-upstream

for you to fetch changes up to 0b57c8160a2a6c833cfb1d958f08385c4391ab70:

  docs/system/i386: xenpvh: Add a basic description (2024-09-04 16:50:43 +0200)


Edgars Xen queue.

--------
Edgar E. Iglesias (12):
  MAINTAINERS: Add docs/system/arm/xenpvh.rst
  hw/arm: xenpvh: Update file header to use SPDX
  hw/arm: xenpvh: Tweak machine description
  hw/arm: xenpvh: Add support for SMP guests
  hw/arm: xenpvh: Remove double-negation in warning
  hw/arm: xenpvh: Move stubbed functions to xen-stubs.c
  hw/arm: xenpvh: Break out a common PVH machine
  hw/arm: xenpvh: Rename xen_arm.c -> xen-pvh.c
  hw/arm: xenpvh: Reverse virtio-mmio creation order
  hw/xen: pvh-common: Add support for creating PCIe/GPEX
  hw/i386/xen: Add a Xen PVH x86 machine
  docs/system/i386: xenpvh: Add a basic description

 MAINTAINERS |   2 +
 docs/system/i386/xenpvh.rst |  49 ++
 docs/system/target-i386.rst |   1 +
 hw/arm/meson.build  |   5 +-
 hw/arm/trace-events |   5 -
 hw/arm/xen-pvh.c|  89 ++
 hw/arm/xen-stubs.c  |  32 
 hw/arm/xen_arm.c| 267 -
 hw/i386/xen/meson.build |   1 +
 hw/i386/xen/xen-pvh.c   | 121 ++
 hw/xen/meson.build  |   1 +
 hw/xen/trace-events |   4 +
 hw/xen/xen-pvh-common.c | 362 
 include/hw/xen/xen-pvh-common.h |  88 ++
 14 files changed, 754 insertions(+), 273 deletions(-)
 create mode 100644 docs/system/i386/xenpvh.rst
 create mode 100644 hw/arm/xen-pvh.c
 create mode 100644 hw/arm/xen-stubs.c
 delete mode 100644 hw/arm/xen_arm.c
 create mode 100644 hw/i386/xen/xen-pvh.c
 create mode 100644 hw/xen/xen-pvh-common.c
 create mode 100644 include/hw/xen/xen-pvh-common.h

-- 
2.43.0

Re: [PATCH] hw/arm/xilinx_zynq: Enable Security Extensions

[Edgar E. Iglesias]

On Thu, Aug 29, 2024 at 01:50:02PM +0100, Peter Maydell wrote:
> On Wed, 28 Aug 2024 at 01:51, Sebastian Huber
>  wrote:
> >
> > The system supports the Security Extensions (core and GIC).  This change is
> > necessary to run tests which pass on the real hardware.
> >
> > Signed-off-by: Sebastian Huber 
> 
> (Added the maintainers to cc.)
> 
> Does the system have any secure-only devices, RAM, etc?

Yes, on real HW there are but I don't think we've modelled any of it yet.
There's TZ both on the SoC and also ability to create FPGA logic that
can issue secure/non-secure transactions. Here's an overview:
https://docs.amd.com/v/u/en-US/ug1019-zynq-trustzone

The primary use-case for the upstream Zynq-7000 QEMU models has historically
been to run the Open Source SW stack from Linux (some times from u-boot)
and up. It's important that we don't break that.

So as long as we add additional support without breaking direct Linux
boots, I think it's OK to incrementally enable missing pieces even
if there's not yet coherent support for firmware boot.

In this case, IIUC, when doing direct Linux boot, TYPE_ARM_LINUX_BOOT_IF
will take care of the GIC setup for us.

> 
> How much testing have you done with this change? (The main
> reason we disabled has-el3 on this board back in 2014 was
> as a backwards-compatibility thing when we added EL3 support
> to the CPU model -- we didn't have a ton of images for the
> board so we erred on the safe side of not changing the
> behaviour to avoid potentially breaking existing guest code.)


I tried this patch on a couple of my images and it works fine for me!

Reviewed-by: Edgar E. Iglesias 
Tested-by: Edgar E. Iglesias 



> 
> > ---
> >  hw/arm/xilinx_zynq.c | 8 
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> > index 3c56b9abe1..37c234f5ab 100644
> > --- a/hw/arm/xilinx_zynq.c
> > +++ b/hw/arm/xilinx_zynq.c
> > @@ -219,14 +219,6 @@ static void zynq_init(MachineState *machine)
> >  for (n = 0; n < smp_cpus; n++) {
> >  Object *cpuobj = object_new(machine->cpu_type);
> >
> > -/*
> > - * By default A9 CPUs have EL3 enabled.  This board does not 
> > currently
> > - * support EL3 so the CPU EL3 property is disabled before 
> > realization.
> > - */
> > -if (object_property_find(cpuobj, "has_el3")) {
> > -object_property_set_bool(cpuobj, "has_el3", false, 
> > &error_fatal);
> > -}
> > -
> >  object_property_set_int(cpuobj, "midr", ZYNQ_BOARD_MIDR,
> >  &error_fatal);
> >  object_property_set_int(cpuobj, "reset-cbar", MPCORE_PERIPHBASE,
> 
> thanks
> -- PMM

Re: [PATCH for-9.2 0/6] arm: xlnx: fix minor memory leaks

[Edgar E. Iglesias]

On Thu, Aug 22, 2024 at 6:21 PM Peter Maydell 
wrote:

> This patchset fixes a collection of minor memory leaks in
> various xlnx devices, all detected by clang LeakSanitizer
> when running 'make check'. Since these are longstanding
> and not very important leaks, this is 9.2 material.
>
>
All of it looks good to me:
Reviewed-by: Edgar E. Iglesias 



> thanks
> -- PMM
>
> Peter Maydell (6):
>   hw/misc/xlnx-versal-cfu: destroy fifo in finalize
>   hw/misc/xlnx-versal-trng: Free s->prng in finalize, not unrealize
>   hw/nvram/xlnx-bbram: Call register_finalize_block
>   hw/nvram/xlnx-zynqmp-efuse: Call register_finalize_block
>   hw/misc/xlnx-versal-trng: Call register_finalize_block
>   hm/nvram/xlnx-versal-efuse-ctrl: Call register_finalize_block
>
>  include/hw/misc/xlnx-versal-trng.h   |  1 +
>  include/hw/nvram/xlnx-bbram.h|  1 +
>  include/hw/nvram/xlnx-versal-efuse.h |  1 +
>  include/hw/nvram/xlnx-zynqmp-efuse.h |  1 +
>  hw/misc/xlnx-versal-cfu.c|  8 
>  hw/misc/xlnx-versal-trng.c   | 12 ++--
>  hw/nvram/xlnx-bbram.c| 13 ++---
>  hw/nvram/xlnx-versal-efuse-ctrl.c|  6 +++---
>  hw/nvram/xlnx-zynqmp-efuse.c | 13 ++---
>  9 files changed, 41 insertions(+), 15 deletions(-)
>
> --
> 2.34.1
>
>

[PATCH v2 11/12] hw/i386/xen: Add a Xen PVH x86 machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add a Xen PVH x86 machine based on the abstract PVH Machine.

Signed-off-by: Edgar E. Iglesias 
---
 hw/i386/xen/meson.build |   1 +
 hw/i386/xen/xen-pvh.c   | 121 
 2 files changed, 122 insertions(+)
 create mode 100644 hw/i386/xen/xen-pvh.c

diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 3f0df8bc07..c73c62b8e3 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -4,6 +4,7 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files(
 ))
 i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-hvm.c',
+  'xen-pvh.c',
 ))
 
 i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files(
diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
new file mode 100644
index 00..45645667e9
--- /dev/null
+++ b/hw/i386/xen/xen-pvh.c
@@ -0,0 +1,121 @@
+/*
+ * QEMU Xen PVH x86 Machine
+ *
+ * Copyright (c) 2024 Advanced Micro Devices, Inc.
+ * Written by Edgar E. Iglesias 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "hw/boards.h"
+#include "sysemu/sysemu.h"
+#include "hw/xen/arch_hvm.h"
+#include 
+#include "hw/xen/xen-pvh-common.h"
+
+#define TYPE_XEN_PVH_X86  MACHINE_TYPE_NAME("xenpvh")
+OBJECT_DECLARE_SIMPLE_TYPE(XenPVHx86State, XEN_PVH_X86)
+
+struct XenPVHx86State {
+/*< private >*/
+XenPVHMachineState parent;
+
+DeviceState **cpu;
+};
+
+static DeviceState *xen_pvh_cpu_new(MachineState *ms,
+int64_t apic_id)
+{
+Object *cpu = object_new(ms->cpu_type);
+
+object_property_add_child(OBJECT(ms), "cpu[*]", cpu);
+object_property_set_uint(cpu, "apic-id", apic_id, &error_fatal);
+qdev_realize(DEVICE(cpu), NULL, &error_fatal);
+object_unref(cpu);
+
+return DEVICE(cpu);
+}
+
+static void xen_pvh_init(MachineState *ms)
+{
+XenPVHx86State *xp = XEN_PVH_X86(ms);
+int i;
+
+/* Create dummy cores. This will indirectly create the APIC MSI window.  */
+xp->cpu = g_malloc(sizeof xp->cpu[0] * ms->smp.max_cpus);
+for (i = 0; i < ms->smp.max_cpus; i++) {
+xp->cpu[i] = xen_pvh_cpu_new(ms, i);
+}
+}
+
+static void xen_pvh_instance_init(Object *obj)
+{
+XenPVHMachineState *s = XEN_PVH_MACHINE(obj);
+
+/* Default values.  */
+s->cfg.ram_low = (MemMapEntry) { 0x0, 0x8000U };
+s->cfg.ram_high = (MemMapEntry) { 0xC0ULL, 0x40ULL };
+s->cfg.pci_intx_irq_base = 16;
+}
+
+/*
+ * Deliver INTX interrupts to Xen guest.
+ */
+static void xen_pvh_set_pci_intx_irq(void *opaque, int irq, int level)
+{
+/*
+ * Since QEMU emulates all of the swizziling
+ * We don't want Xen to do any additional swizzling in
+ * xen_set_pci_intx_level() so we always set device to 0.
+ */
+if (xen_set_pci_intx_level(xen_domid, 0, 0, 0, irq, level)) {
+error_report("xendevicemodel_set_pci_intx_level failed");
+}
+}
+
+static void xen_pvh_machine_class_init(ObjectClass *oc, void *data)
+{
+XenPVHMachineClass *xpc = XEN_PVH_MACHINE_CLASS(oc);
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc = "Xen PVH x86 machine";
+mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
+
+/* mc->max_cpus holds the MAX value allowed in the -smp cmd-line opts. */
+mc->max_cpus = HVM_MAX_VCPUS;
+
+/* We have an implementation specific init to create CPU objects.  */
+xpc->init = xen_pvh_init;
+
+/*
+ * PCI INTX routing.
+ *
+ * We describe the mapping between the 4 INTX interrupt and GSIs
+ * using xen_set_pci_link_route(). xen_pvh_set_pci_intx_irq is
+ * used to deliver the interrupt.
+ */
+xpc->set_pci_intx_irq = xen_pvh_set_pci_intx_irq;
+xpc->set_pci_link_route = xen_set_pci_link_route;
+
+/* List of supported features known to work on PVH x86.  */
+xpc->has_pci = true;
+
+xen_pvh_class_setup_common_props(xpc);
+}
+
+static const TypeInfo xen_pvh_x86_machine_type = {
+.name = TYPE_XEN_PVH_X86,
+.parent = TYPE_XEN_PVH_MACHINE,
+.class_init = xen_pvh_machine_class_init,
+.instance_init = xen_pvh_instance_init,
+.instance_size = sizeof(XenPVHx86State),
+};
+
+static void xen_pvh_machine_register_types(void)
+{
+type_register_static(&xen_pvh_x86_machine_type);
+}
+
+type_init(xen_pvh_machine_register_types)
-- 
2.43.0

[PATCH v2 03/12] hw/arm: xenpvh: Tweak machine description

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Tweak machine description to better express that this is
a Xen PVH machine for ARM.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/xen_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 766a194fa1..5f75cc3779 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -216,7 +216,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 {
 
 MachineClass *mc = MACHINE_CLASS(oc);
-mc->desc = "Xen Para-virtualized PC";
+mc->desc = "Xen PVH ARM machine";
 mc->init = xen_arm_init;
 mc->max_cpus = 1;
 mc->default_machine_opts = "accel=xen";
-- 
2.43.0

[PATCH v2 02/12] hw/arm: xenpvh: Update file header to use SPDX

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Update file header to use SPDX and remove stray empty
comment line.

No functional changes.

Signed-off-by: Edgar E. Iglesias 
Acked-by: Stefano Stabellini 
---
 hw/arm/xen_arm.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 6fad829ede..766a194fa1 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -1,24 +1,7 @@
 /*
  * QEMU ARM Xen PVH Machine
  *
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
+ * SPDX-License-Identifier: MIT
  */
 
 #include "qemu/osdep.h"
-- 
2.43.0

[PATCH v2 08/12] hw/arm: xenpvh: Rename xen_arm.c -> xen-pvh.c

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Rename xen_arm.c -> xen-pvh.c to better express that this
is a PVH machine and to align with x86 HVM and future PVH
machine filenames:
hw/i386/xen/xen-hvm.c
hw/i386/xen/xen-pvh.c (in preparation)

No functional changes.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 hw/arm/meson.build  | 2 +-
 hw/arm/{xen_arm.c => xen-pvh.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename hw/arm/{xen_arm.c => xen-pvh.c} (100%)

diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 074612b40c..4059d0be2e 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -61,7 +61,7 @@ arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: 
files('fsl-imx6ul.c', 'mcimx6ul-e
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
 arm_ss.add(when: 'CONFIG_XEN', if_true: files(
   'xen-stubs.c',
-  'xen_arm.c',
+  'xen-pvh.c',
 ))
 
 system_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen-pvh.c
similarity index 100%
rename from hw/arm/xen_arm.c
rename to hw/arm/xen-pvh.c
-- 
2.43.0

[PATCH v2 06/12] hw/arm: xenpvh: Move stubbed functions to xen-stubs.c

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/meson.build |  5 -
 hw/arm/xen-stubs.c | 32 
 hw/arm/xen_arm.c   | 20 
 3 files changed, 36 insertions(+), 21 deletions(-)
 create mode 100644 hw/arm/xen-stubs.c

diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 0c07ab522f..074612b40c 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -59,7 +59,10 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true: 
files('fsl-imx7.c', 'mcimx7d-sabre.
 arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
 arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 
'mcimx6ul-evk.c'))
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
-arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
+arm_ss.add(when: 'CONFIG_XEN', if_true: files(
+  'xen-stubs.c',
+  'xen_arm.c',
+))
 
 system_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
 system_ss.add(when: 'CONFIG_CHEETAH', if_true: files('palm.c'))
diff --git a/hw/arm/xen-stubs.c b/hw/arm/xen-stubs.c
new file mode 100644
index 00..4ac6a56a96
--- /dev/null
+++ b/hw/arm/xen-stubs.c
@@ -0,0 +1,32 @@
+/*
+ * Stubs for unimplemented Xen functions for ARM.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/qapi-commands-migration.h"
+#include "hw/boards.h"
+#include "sysemu/sysemu.h"
+#include "hw/xen/xen-hvm-common.h"
+#include "hw/xen/arch_hvm.h"
+
+void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
+{
+hw_error("Invalid ioreq type 0x%x\n", req->type);
+return;
+}
+
+void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
+ bool add)
+{
+}
+
+void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+}
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+}
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 16b3f00992..f0868e7be5 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -115,26 +115,6 @@ static void xen_init_ram(MachineState *machine)
 memory_region_add_subregion(sysmem, XEN_GRANT_ADDR_OFF, &xen_grants);
 }
 
-void arch_handle_ioreq(XenIOState *state, ioreq_t *req)
-{
-hw_error("Invalid ioreq type 0x%x\n", req->type);
-
-return;
-}
-
-void arch_xen_set_memory(XenIOState *state, MemoryRegionSection *section,
- bool add)
-{
-}
-
-void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
-{
-}
-
-void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
-{
-}
-
 #ifdef CONFIG_TPM
 static void xen_enable_tpm(XenArmState *xam)
 {
-- 
2.43.0

[PATCH v2 10/12] hw/xen: pvh-common: Add support for creating PCIe/GPEX

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add support for optionally creating a PCIe/GPEX controller.

Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-pvh-common.c | 76 +
 include/hw/xen/xen-pvh-common.h | 29 +
 2 files changed, 105 insertions(+)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 295f920442..28d7168446 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -122,6 +122,64 @@ static void xen_enable_tpm(XenPVHMachineState *s)
 }
 #endif
 
+/*
+ * We use the GPEX PCIe controller with its internal INTX PCI interrupt
+ * swizzling. This swizzling is emulated in QEMU and routes all INTX
+ * interrupts from endpoints down to only 4 INTX interrupts.
+ * See include/hw/pci/pci.h : pci_swizzle()
+ */
+static inline void xenpvh_gpex_init(XenPVHMachineState *s,
+XenPVHMachineClass *xpc,
+MemoryRegion *sysmem)
+{
+MemoryRegion *ecam_reg;
+MemoryRegion *mmio_reg;
+DeviceState *dev;
+int i;
+
+object_initialize_child(OBJECT(s), "gpex", &s->pci.gpex,
+TYPE_GPEX_HOST);
+dev = DEVICE(&s->pci.gpex);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_add_subregion(sysmem, s->cfg.pci_ecam.base, ecam_reg);
+
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+
+if (s->cfg.pci_mmio.size) {
+memory_region_init_alias(&s->pci.mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg,
+ s->cfg.pci_mmio.base, s->cfg.pci_mmio.size);
+memory_region_add_subregion(sysmem, s->cfg.pci_mmio.base,
+&s->pci.mmio_alias);
+}
+
+if (s->cfg.pci_mmio_high.size) {
+memory_region_init_alias(&s->pci.mmio_high_alias, OBJECT(dev),
+"pcie-mmio-high",
+mmio_reg, s->cfg.pci_mmio_high.base, 
s->cfg.pci_mmio_high.size);
+memory_region_add_subregion(sysmem, s->cfg.pci_mmio_high.base,
+&s->pci.mmio_high_alias);
+}
+
+/*
+ * PVH implementations with PCI enabled must provide set_pci_intx_irq()
+ * and optionally an implementation of set_pci_link_route().
+ */
+assert(xpc->set_pci_intx_irq);
+
+for (i = 0; i < GPEX_NUM_IRQS; i++) {
+qemu_irq irq = qemu_allocate_irq(xpc->set_pci_intx_irq, s, i);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
+gpex_set_irq_num(GPEX_HOST(dev), i, s->cfg.pci_intx_irq_base + i);
+if (xpc->set_pci_link_route) {
+xpc->set_pci_link_route(i, s->cfg.pci_intx_irq_base + i);
+}
+}
+}
+
 static void xen_pvh_init(MachineState *ms)
 {
 XenPVHMachineState *s = XEN_PVH_MACHINE(ms);
@@ -152,6 +210,15 @@ static void xen_pvh_init(MachineState *ms)
 }
 #endif
 
+/* Non-zero pci-ecam-size enables PCI.  */
+if (s->cfg.pci_ecam.size) {
+if (s->cfg.pci_ecam.size != 256 * MiB) {
+error_report("pci-ecam-size only supports values 0 or 0x1000");
+exit(EXIT_FAILURE);
+}
+xenpvh_gpex_init(s, xpc, sysmem);
+}
+
 /* Call the implementation specific init.  */
 if (xpc->init) {
 xpc->init(ms);
@@ -200,6 +267,9 @@ XEN_PVH_PROP_MEMMAP(ram_high)
 /* TPM only has a base-addr option.  */
 XEN_PVH_PROP_MEMMAP_BASE(tpm)
 XEN_PVH_PROP_MEMMAP(virtio_mmio)
+XEN_PVH_PROP_MEMMAP(pci_ecam)
+XEN_PVH_PROP_MEMMAP(pci_mmio)
+XEN_PVH_PROP_MEMMAP(pci_mmio_high)
 
 void xen_pvh_class_setup_common_props(XenPVHMachineClass *xpc)
 {
@@ -242,6 +312,12 @@ do {   
   \
 OC_MEMMAP_PROP(oc, "virtio-mmio", virtio_mmio);
 }
 
+if (xpc->has_pci) {
+OC_MEMMAP_PROP(oc, "pci-ecam", pci_ecam);
+OC_MEMMAP_PROP(oc, "pci-mmio", pci_mmio);
+OC_MEMMAP_PROP(oc, "pci-mmio-high", pci_mmio_high);
+}
+
 #ifdef CONFIG_TPM
 if (xpc->has_tpm) {
 object_class_property_add(oc, "tpm-base-addr", "uint64_t",
diff --git a/include/hw/xen/xen-pvh-common.h b/include/hw/xen/xen-pvh-common.h
index 77fd98b9fe..bc09eea936 100644
--- a/include/hw/xen/xen-pvh-common.h
+++ b/include/hw/xen/xen-pvh-common.h
@@ -25,10 +25,29 @@ struct XenPVHMachineClass {
 /* PVH implementation specific init.  */
 void (*init)(MachineState *state);
 
+/*
+ * set_pci_intx_irq - Deliver INTX irqs to the guest.
+ *
+ * @opaque: pointer to XenPVHMachineState.
+ * @irq: IRQ after swizzling, between 0-3.
+ * @level: IRQ level.
+ */
+void (*set_pci_intx_irq)(void *opaque, int irq, int level);
+
+ 

[PATCH v2 07/12] hw/arm: xenpvh: Break out a common PVH machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Break out a common Xen PVH machine in preparation for
adding a x86 Xen PVH machine.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/trace-events |   5 -
 hw/arm/xen_arm.c| 198 +++
 hw/xen/meson.build  |   1 +
 hw/xen/trace-events |   4 +
 hw/xen/xen-pvh-common.c | 275 
 include/hw/xen/xen-pvh-common.h |  59 +++
 6 files changed, 358 insertions(+), 184 deletions(-)
 create mode 100644 hw/xen/xen-pvh-common.c
 create mode 100644 include/hw/xen/xen-pvh-common.h

diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index be6c8f720b..c64ad344bd 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -68,10 +68,5 @@ z2_aer915_send_too_long(int8_t msg) "message too long (%i 
bytes)"
 z2_aer915_send(uint8_t reg, uint8_t value) "reg %d value 0x%02x"
 z2_aer915_event(int8_t event, int8_t len) "i2c event =0x%x len=%d bytes"
 
-# xen_arm.c
-xen_create_virtio_mmio_devices(int i, int irq, uint64_t base) "Created 
virtio-mmio device %d: irq %d base 0x%"PRIx64
-xen_init_ram(uint64_t machine_ram_size) "Initialized xen ram with size 
0x%"PRIx64
-xen_enable_tpm(uint64_t addr) "Connected tpmdev at address 0x%"PRIx64
-
 # bcm2838.c
 bcm2838_gic_set_irq(int irq, int level) "gic irq:%d lvl:%d"
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index f0868e7be5..04cb9855af 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -7,44 +7,12 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qapi/qapi-commands-migration.h"
-#include "qapi/visitor.h"
 #include "hw/boards.h"
-#include "hw/irq.h"
-#include "hw/sysbus.h"
-#include "sysemu/block-backend.h"
-#include "sysemu/tpm_backend.h"
 #include "sysemu/sysemu.h"
-#include "hw/xen/xen-hvm-common.h"
-#include "sysemu/tpm.h"
+#include "hw/xen/xen-pvh-common.h"
 #include "hw/xen/arch_hvm.h"
-#include "trace.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
-OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
-
-static const MemoryListener xen_memory_listener = {
-.region_add = xen_region_add,
-.region_del = xen_region_del,
-.log_start = NULL,
-.log_stop = NULL,
-.log_sync = NULL,
-.log_global_start = NULL,
-.log_global_stop = NULL,
-.priority = MEMORY_LISTENER_PRIORITY_ACCEL,
-};
-
-struct XenArmState {
-/*< private >*/
-MachineState parent;
-
-XenIOState *state;
-
-struct {
-uint64_t tpm_base_addr;
-} cfg;
-};
-
-static MemoryRegion ram_lo, ram_hi;
 
 /*
  * VIRTIO_MMIO_DEV_SIZE is imported from tools/libs/light/libxl_arm.c under Xen
@@ -57,147 +25,26 @@ static MemoryRegion ram_lo, ram_hi;
 #define NR_VIRTIO_MMIO_DEVICES   \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
 
-static void xen_set_irq(void *opaque, int irq, int level)
-{
-if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
-error_report("xendevicemodel_set_irq_level failed");
-}
-}
-
-static void xen_create_virtio_mmio_devices(XenArmState *xam)
-{
-int i;
-
-for (i = 0; i < NR_VIRTIO_MMIO_DEVICES; i++) {
-hwaddr base = GUEST_VIRTIO_MMIO_BASE + i * VIRTIO_MMIO_DEV_SIZE;
-qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
- GUEST_VIRTIO_MMIO_SPI_FIRST + i);
-
-sysbus_create_simple("virtio-mmio", base, irq);
-
-trace_xen_create_virtio_mmio_devices(i,
- GUEST_VIRTIO_MMIO_SPI_FIRST + i,
- base);
-}
-}
-
-static void xen_init_ram(MachineState *machine)
+static void xen_arm_instance_init(Object *obj)
 {
-MemoryRegion *sysmem = get_system_memory();
-ram_addr_t block_len, ram_size[GUEST_RAM_BANKS];
-
-trace_xen_init_ram(machine->ram_size);
-if (machine->ram_size <= GUEST_RAM0_SIZE) {
-ram_size[0] = machine->ram_size;
-ram_size[1] = 0;
-block_len = GUEST_RAM0_BASE + ram_size[0];
-} else {
-ram_size[0] = GUEST_RAM0_SIZE;
-ram_size[1] = machine->ram_size - GUEST_RAM0_SIZE;
-block_len = GUEST_RAM1_BASE + ram_size[1];
-}
+XenPVHMachineState *s = XEN_PVH_MACHINE(obj);
 
-memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
-   &error_fatal);
+/* Default values.  */
+s->cfg.ram_low = (MemMapEntry) { GUEST_RAM0_BASE, GUEST_RAM0_SIZE };
+s->cfg.ram_high = (MemMapEntry) { GUEST_RAM1_BASE, GUEST_RAM1_SIZE };
 
-memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &xen_memory,
- GUEST_RAM0_BASE, ram_size[0]);
-

[PATCH v2 01/12] MAINTAINERS: Add docs/system/arm/xenpvh.rst

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Acked-by: Stefano Stabellini 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3584d6a6c6..c2fb0c2f42 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -559,6 +559,7 @@ F: include/hw/xen/
 F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
+F: docs/system/arm/xenpvh.rst
 
 Guest CPU Cores (NVMM)
 --
-- 
2.43.0

[PATCH v2 12/12] docs/system/i386: xenpvh: Add a basic description

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Stefano Stabellini 
---
 MAINTAINERS |  1 +
 docs/system/i386/xenpvh.rst | 49 +
 docs/system/target-i386.rst |  1 +
 3 files changed, 51 insertions(+)
 create mode 100644 docs/system/i386/xenpvh.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index c2fb0c2f42..c14ac014e2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -560,6 +560,7 @@ F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
 F: docs/system/arm/xenpvh.rst
+F: docs/system/i386/xenpvh.rst
 
 Guest CPU Cores (NVMM)
 --
diff --git a/docs/system/i386/xenpvh.rst b/docs/system/i386/xenpvh.rst
new file mode 100644
index 00..354250f073
--- /dev/null
+++ b/docs/system/i386/xenpvh.rst
@@ -0,0 +1,49 @@
+Xen PVH machine (``xenpvh``)
+=
+
+Xen supports a spectrum of types of guests that vary in how they depend
+on HW virtualization features, emulation models and paravirtualization.
+PVH is a mode that uses HW virtualization features (like HVM) but tries
+to avoid emulation models and instead use passthrough or
+paravirtualized devices.
+
+QEMU can be used to provide PV virtio devices on an emulated PCIe controller.
+That is the purpose of this minimal machine.
+
+Supported devices
+-
+
+The x86 Xen PVH QEMU machine provide the following devices:
+
+- RAM
+- GPEX host bridge
+- virtio-pci devices
+
+The idea is to only connect virtio-pci devices but in theory any compatible
+PCI device model will work depending on Xen and guest support.
+
+Running
+---
+
+The Xen tools will typically construct a command-line and launch QEMU
+for you when needed. But here's an example of what it can look like in
+case you need to construct one manually:
+
+.. code-block:: console
+
+qemu-system-i386 -xen-domid 3 -no-shutdown\
+  -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-3,server=on,wait=off \
+  -mon chardev=libxl-cmd,mode=control \
+  -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-3,server=on,wait=off \
+  -mon chardev=libxenstat-cmd,mode=control\
+  -nodefaults \
+  -no-user-config \
+  -xen-attach -name g0\
+  -vnc none   \
+  -display none   \
+  -device virtio-net-pci,id=nic0,netdev=net0,mac=00:16:3e:5c:81:78 \
+  -netdev 
type=tap,id=net0,ifname=vif3.0-emu,br=xenbr0,script=no,downscript=no \
+  -smp 4,maxcpus=4\
+  -nographic  \
+  -machine 
xenpvh,ram-low-base=0,ram-low-size=2147483648,ram-high-base=4294967296,ram-high-size=2147483648,pci-ecam-base=824633720832,pci-ecam-size=268435456,pci-mmio-base=4026531840,pci-mmio-size=33554432,pci-mmio-high-base=824902156288,pci-mmio-high-size=68719476736
 \
+  -m 4096
diff --git a/docs/system/target-i386.rst b/docs/system/target-i386.rst
index 1b8a1f248a..23e84e3ba7 100644
--- a/docs/system/target-i386.rst
+++ b/docs/system/target-i386.rst
@@ -26,6 +26,7 @@ Architectural features
i386/cpu
i386/hyperv
i386/xen
+   i386/xenpvh
i386/kvm-pv
i386/sgx
i386/amd-memory-encryption
-- 
2.43.0

[PATCH v2 09/12] hw/arm: xenpvh: Reverse virtio-mmio creation order

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

We've been creating the virtio-mmio devices in forwards order
but since the qbus lists prepend (rather than append) entries,
the virtio busses end up with decreasing base address order.

Xen enables virtio-mmio nodes in forwards order so there's been
a missmatch. So far, we've been working around this with an
out-of-tree patch to Xen.

This reverses the order making sure the virtio busses end up
ordered with increasing base addresses avoiding the need to
patch Xen.

Signed-off-by: Edgar E. Iglesias 
Acked-by: Stefano Stabellini 
---
 hw/xen/xen-pvh-common.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 880e8143d7..295f920442 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -75,7 +75,18 @@ static void 
xen_create_virtio_mmio_devices(XenPVHMachineState *s)
 {
 int i;
 
-for (i = 0; i < s->cfg.virtio_mmio_num; i++) {
+/*
+ * We create the transports in reverse order. Since qbus_realize()
+ * prepends (not appends) new child buses, the decrementing loop below will
+ * create a list of virtio-mmio buses with increasing base addresses.
+ *
+ * When a -device option is processed from the command line,
+ * qbus_find_recursive() picks the next free virtio-mmio bus in forwards
+ * order.
+ *
+ * This is what the Xen tools expect.
+ */
+for (i = s->cfg.virtio_mmio_num - 1; i >= 0; i--) {
 hwaddr base = s->cfg.virtio_mmio.base + i * s->cfg.virtio_mmio.size;
 qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
  s->cfg.virtio_mmio_irq_base + i);
-- 
2.43.0

[PATCH v2 04/12] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add SMP support for Xen PVH ARM guests.
Create ms->smp.max_cpus ioreq servers to handle hotplug.

Note that ms->smp.max_cpus will be passed to us by the
user (Xen tools) set to the guests maxvcpus.

The value in mc->max_cpus is an absolute maximum for the
-smp option and won't be used to setup ioreq servers unless
the user explicitly specifies it with -smp.

If the user doesn't pass -smp on the command-line, smp.cpus
and smp.max_cpus will default to 1.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen_arm.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 5f75cc3779..fda65d0d8d 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -173,7 +173,7 @@ static void xen_arm_init(MachineState *machine)
 
 xen_init_ram(machine);
 
-xen_register_ioreq(xam->state, machine->smp.cpus, &xen_memory_listener);
+xen_register_ioreq(xam->state, machine->smp.max_cpus, 
&xen_memory_listener);
 
 xen_create_virtio_mmio_devices(xam);
 
@@ -218,7 +218,26 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 MachineClass *mc = MACHINE_CLASS(oc);
 mc->desc = "Xen PVH ARM machine";
 mc->init = xen_arm_init;
-mc->max_cpus = 1;
+
+/*
+ * mc->max_cpus holds the MAX value allowed in the -smp command-line opts.
+ *
+ * 1. If users don't pass any -smp option:
+ *   ms->smp.cpus will default to 1.
+ *   ms->smp.max_cpus will default to 1.
+ *
+ * 2. If users pass -smp X:
+ *   ms->smp.cpus will be set to X.
+ *   ms->smp.max_cpus will also be set to X.
+ *
+ * 3. If users pass -smp X,maxcpus=Y:
+ *   ms->smp.cpus will be set to X.
+ *   ms->smp.max_cpus will be set to Y.
+ *
+ * In scenarios 2 and 3, if X or Y are set to something larger than
+ * mc->max_cpus, QEMU will bail out with an error message.
+ */
+mc->max_cpus = GUEST_MAX_VCPUS;
 mc->default_machine_opts = "accel=xen";
 /* Set explicitly here to make sure that real ram_size is passed */
 mc->default_ram_size = 0;
-- 
2.43.0

[PATCH v2 05/12] hw/arm: xenpvh: Remove double-negation in warning

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index fda65d0d8d..16b3f00992 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -165,7 +165,7 @@ static void xen_arm_init(MachineState *machine)
 xam->state =  g_new0(XenIOState, 1);
 
 if (machine->ram_size == 0) {
-warn_report("%s non-zero ram size not specified. QEMU machine started"
+warn_report("%s: ram size not specified. QEMU machine started"
 " without IOREQ (no emulated devices including virtio)",
 MACHINE_CLASS(object_get_class(OBJECT(machine)))->desc);
 return;
-- 
2.43.0

[PATCH v2 00/12] xen: pvh: Partial QOM:fication with new x86 PVH machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This series breaks out parts of the ARM PVH support into an abstract
machine that other targets can reuse.. There's a bit of refactoring
and some bug-fixes along the way.

Finally we add a new x86 xen-pvh machine.

The corresponding changes in Xen for PVH x86 are work in progress
(by Xenia Ragiadakou). You can find a current version here:
https://github.com/edgarigl/xen/tree/edgar/virtio-pvh-upstream 

I've briefly described the steps to run Xen PVH x86 guests here
(including example guest kernel config, xl.cfg and xl command-lines):
https://github.com/edgarigl/docs/blob/master/xen/pvh/xenpvh-x86.md

Cheers,
Edgar

ChangeLog:

v1 -> v2:
* Use an abstract PVH Machine to share code between arch specific
  PVH machines. This allows more sharing than a using a device component.
* Add comments describing our PCI INTX support.
* Allow the PVH target specific implementation to provide PCI
  INTX delivery via class methods.
* Clarify use of mc->max_cpus in comments.
* Use GUEST_MAX_VCPUS mc->max_cpus on ARM.
* Use HVM_MAX_VCPUS mc->max_cpus on x86.
* pvh-common: Directly use s->cfg. members when creating gpex.
* pvh-common: Clarify use of pci-ecam-size in comments.
* x86/xen-pvh: Create smp.max_cpus nr of dummy cores (not smp.cpus).
* x86/xen-pvh: Replace static array holding dummy cores with a dynamic one.
* Move stubbed Xen/ARM functions from hw/arm/xen_arm.c into xen-stubs.c

Edgar E. Iglesias (12):
  MAINTAINERS: Add docs/system/arm/xenpvh.rst
  hw/arm: xenpvh: Update file header to use SPDX
  hw/arm: xenpvh: Tweak machine description
  hw/arm: xenpvh: Add support for SMP guests
  hw/arm: xenpvh: Remove double-negation in warning
  hw/arm: xenpvh: Move stubbed functions to xen-stubs.c
  hw/arm: xenpvh: Break out a common PVH machine
  hw/arm: xenpvh: Rename xen_arm.c -> xen-pvh.c
  hw/arm: xenpvh: Reverse virtio-mmio creation order
  hw/xen: pvh-common: Add support for creating PCIe/GPEX
  hw/i386/xen: Add a Xen PVH x86 machine
  docs/system/i386: xenpvh: Add a basic description

 MAINTAINERS |   2 +
 docs/system/i386/xenpvh.rst |  49 +
 docs/system/target-i386.rst |   1 +
 hw/arm/meson.build  |   5 +-
 hw/arm/trace-events |   5 -
 hw/arm/xen-pvh.c|  89 
 hw/arm/xen-stubs.c  |  32 +++
 hw/arm/xen_arm.c| 267 ---
 hw/i386/xen/meson.build |   1 +
 hw/i386/xen/xen-pvh.c   | 121 +++
 hw/xen/meson.build  |   1 +
 hw/xen/trace-events |   4 +
 hw/xen/xen-pvh-common.c | 362 
 include/hw/xen/xen-pvh-common.h |  88 
 14 files changed, 754 insertions(+), 273 deletions(-)
 create mode 100644 docs/system/i386/xenpvh.rst
 create mode 100644 hw/arm/xen-pvh.c
 create mode 100644 hw/arm/xen-stubs.c
 delete mode 100644 hw/arm/xen_arm.c
 create mode 100644 hw/i386/xen/xen-pvh.c
 create mode 100644 hw/xen/xen-pvh-common.c
 create mode 100644 include/hw/xen/xen-pvh-common.h

-- 
2.43.0

Re: [PATCH v1 04/10] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

On Sat, Aug 17, 2024 at 2:45 AM Jason Andryuk  wrote:

> On 2024-08-16 12:53, Stefano Stabellini wrote:
> > On Fri, 16 Aug 2024, Edgar E. Iglesias wrote:
> >> On Thu, Aug 15, 2024 at 2:30 AM Stefano Stabellini <
> sstabell...@kernel.org> wrote:
> >>    On Wed, 14 Aug 2024, Edgar E. Iglesias wrote:
> >>> On Tue, Aug 13, 2024 at 03:52:32PM -0700, Stefano Stabellini
> wrote:
> >>> > On Tue, 13 Aug 2024, Edgar E. Iglesias wrote:
> >>> > > On Mon, Aug 12, 2024 at 06:47:17PM -0700, Stefano
> Stabellini wrote:
> >>    > > > > On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> >>> > > > > From: "Edgar E. Iglesias" 
> >>> > > > >
> >>> > > > > Add SMP support for Xen PVH ARM guests. Create
> max_cpus ioreq
> >>> > > > > servers to handle hotplug.
> >>> > > > >
> >>> > > > > Signed-off-by: Edgar E. Iglesias <
> edgar.igles...@amd.com>
> >>> > > > > ---
> >>> > > > >  hw/arm/xen_arm.c | 5 +++--
> >>> > > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> >>> > > > >
> >>> > > > > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> >>> > > > > index 5f75cc3779..ef8315969c 100644
> >>> > > > > --- a/hw/arm/xen_arm.c
> >>> > > > > +++ b/hw/arm/xen_arm.c
> >>> > > > > @@ -173,7 +173,7 @@ static void
> xen_arm_init(MachineState *machine)
> >>> > > > >
> >>> > > > >  xen_init_ram(machine);
> >>> > > > >
> >>> > > > > -xen_register_ioreq(xam->state, machine->smp.cpus,
> &xen_memory_listener);
> >>> > > > > +xen_register_ioreq(xam->state,
> machine->smp.max_cpus, &xen_memory_listener);
> >>> > > > >
> >>> > > > >  xen_create_virtio_mmio_devices(xam);
> >>> > > > >
> >>> > > > > @@ -218,7 +218,8 @@ static void
> xen_arm_machine_class_init(ObjectClass *oc, void *data)
> >>> > > > >  MachineClass *mc = MACHINE_CLASS(oc);
> >>> > > > >  mc->desc = "Xen PVH ARM machine";
> >>> > > > >  mc->init = xen_arm_init;
> >>> > > > > -mc->max_cpus = 1;
> >>> > > > > +/* MAX number of vcpus supported by Xen.  */
> >>> > > > > +mc->max_cpus = GUEST_MAX_VCPUS;
> >>> > > >
> >>> > > > Will this cause allocations of data structures with 128
> elements?
> >>> > > > Looking at hw/xen/xen-hvm-common.c:xen_do_ioreq_register
> it seems
> >>> > > > possible? Or
> hw/xen/xen-hvm-common.c:xen_do_ioreq_register is called
> >>> > >
> >>> > > Yes, in theory there's probably overhead with this but as
> you correctly
> >>> > > noted below, a PVH aware xl will set the max_cpus option
> to a lower value.
> >>> > >
> >>> > > With a non-pvh aware xl, I was a little worried about the
> overhead
> >>> > > but I couldn't see any visible slow-down on ARM neither in
> boot or in network
> >>> > > performance (I didn't run very sophisticated benchmarks).
> >>> >
> >>> > What do you mean by "non-pvh aware xl"? All useful versions
> of xl
> >>> > support pvh?
> >>>
> >>>
> >>> I mean an xl without our PVH patches merged.
> >>> xl in upstream doesn't know much about PVH yet.
> >>> Even for ARM, we're still carrying significant patches in our
> tree.
> >>
> >>Oh I see. In that case, I don't think we need to support
> "non-pvh aware xl".
> >>
> >>
> >>> > > > later on with the precise vCPU value which should be
> provided to QEMU
> >>> > > &g

Re: [PATCH v1 04/10] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

On Thu, Aug 15, 2024 at 2:30 AM Stefano Stabellini 
wrote:

> On Wed, 14 Aug 2024, Edgar E. Iglesias wrote:
> > On Tue, Aug 13, 2024 at 03:52:32PM -0700, Stefano Stabellini wrote:
> > > On Tue, 13 Aug 2024, Edgar E. Iglesias wrote:
> > > > On Mon, Aug 12, 2024 at 06:47:17PM -0700, Stefano Stabellini wrote:
> > > > > On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > > > > > From: "Edgar E. Iglesias" 
> > > > > >
> > > > > > Add SMP support for Xen PVH ARM guests. Create max_cpus ioreq
> > > > > > servers to handle hotplug.
> > > > > >
> > > > > > Signed-off-by: Edgar E. Iglesias 
> > > > > > ---
> > > > > >  hw/arm/xen_arm.c | 5 +++--
> > > > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> > > > > > index 5f75cc3779..ef8315969c 100644
> > > > > > --- a/hw/arm/xen_arm.c
> > > > > > +++ b/hw/arm/xen_arm.c
> > > > > > @@ -173,7 +173,7 @@ static void xen_arm_init(MachineState
> *machine)
> > > > > >
> > > > > >  xen_init_ram(machine);
> > > > > >
> > > > > > -xen_register_ioreq(xam->state, machine->smp.cpus,
> &xen_memory_listener);
> > > > > > +xen_register_ioreq(xam->state, machine->smp.max_cpus,
> &xen_memory_listener);
> > > > > >
> > > > > >  xen_create_virtio_mmio_devices(xam);
> > > > > >
> > > > > > @@ -218,7 +218,8 @@ static void
> xen_arm_machine_class_init(ObjectClass *oc, void *data)
> > > > > >  MachineClass *mc = MACHINE_CLASS(oc);
> > > > > >  mc->desc = "Xen PVH ARM machine";
> > > > > >  mc->init = xen_arm_init;
> > > > > > -mc->max_cpus = 1;
> > > > > > +/* MAX number of vcpus supported by Xen.  */
> > > > > > +mc->max_cpus = GUEST_MAX_VCPUS;
> > > > >
> > > > > Will this cause allocations of data structures with 128 elements?
> > > > > Looking at hw/xen/xen-hvm-common.c:xen_do_ioreq_register it seems
> > > > > possible? Or hw/xen/xen-hvm-common.c:xen_do_ioreq_register is
> called
> > > >
> > > > Yes, in theory there's probably overhead with this but as you
> correctly
> > > > noted below, a PVH aware xl will set the max_cpus option to a lower
> value.
> > > >
> > > > With a non-pvh aware xl, I was a little worried about the overhead
> > > > but I couldn't see any visible slow-down on ARM neither in boot or
> in network
> > > > performance (I didn't run very sophisticated benchmarks).
> > >
> > > What do you mean by "non-pvh aware xl"? All useful versions of xl
> > > support pvh?
> >
> >
> > I mean an xl without our PVH patches merged.
> > xl in upstream doesn't know much about PVH yet.
> > Even for ARM, we're still carrying significant patches in our tree.
>
> Oh I see. In that case, I don't think we need to support "non-pvh aware
> xl".
>
>
> > > > > later on with the precise vCPU value which should be provided to
> QEMU
> > > > > via the -smp command line option
> > > > > (tools/libs/light/libxl_dm.c:libxl__build_device_model_args_new)?
> > > >
> > > > Yes, a pvh aware xl will for example pass -smp 2,maxcpus=4 based on
> > > > values from the xl.cfg. If the user doesn't set maxvcpus in xl.cfg,
> xl
> > > > will set maxvcpus to the same value as vcpus.
> > >
> > > OK good. In that case if this is just an initial value meant to be
> > > overwritten, I think it is best to keep it as 1.
> >
> > Sorry but that won't work. I think the confusion here may be that
> > it's easy to mix up mc->max_cpus and machine->smp.max_cpus, these are
> > not the same. They have different purposes.
> >
> > I'll try to clarify the 3 values in play.
> >
> > machine-smp.cpus:
> > Number of guest vcpus active at boot.
> > Passed to QEMU via the -smp command-line option.
> > We don't use this value in QEMU's ARM PVH machines.
> >
> > machine->smp.max_cpus:
> > Max number of vcpus that the guest can use (equal or larger than
> machine-smp.cpus

Re: [PATCH v1 09/10] hw/i386/xen: Add a Xen PVH x86 machine

[Edgar E. Iglesias]

On Mon, Aug 12, 2024 at 06:48:52PM -0700, Stefano Stabellini wrote:
> On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > This adds a Xen PVH x86 machine based on the PVH Common
> > module used by the ARM PVH machine.
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  hw/i386/xen/meson.build |   1 +
> >  hw/i386/xen/xen-pvh.c   | 196 
> >  2 files changed, 197 insertions(+)
> >  create mode 100644 hw/i386/xen/xen-pvh.c
> > 
> > diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
> > index 3f0df8bc07..c73c62b8e3 100644
> > --- a/hw/i386/xen/meson.build
> > +++ b/hw/i386/xen/meson.build
> > @@ -4,6 +4,7 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files(
> >  ))
> >  i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
> >'xen-hvm.c',
> > +  'xen-pvh.c',
> >  ))
> >  
> >  i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files(
> > diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
> > new file mode 100644
> > index 000000..9c3d3fc58d
> > --- /dev/null
> > +++ b/hw/i386/xen/xen-pvh.c
> > @@ -0,0 +1,196 @@
> > +/*
> > + * QEMU Xen PVH x86 Machine
> > + *
> > + * Copyright (c) 2024 Advanced Micro Devices, Inc.
> > + * Written by Edgar E. Iglesias 
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "qapi/visitor.h"
> > +#include "qemu/error-report.h"
> > +#include "hw/boards.h"
> > +#include "sysemu/sysemu.h"
> > +#include "hw/xen/arch_hvm.h"
> > +#include "hw/xen/xen.h"
> > +#include "hw/xen/xen-pvh-common.h"
> > +
> > +#define TYPE_XEN_PVH_X86  MACHINE_TYPE_NAME("xenpvh")
> > +OBJECT_DECLARE_SIMPLE_TYPE(XenPVHx86State, XEN_PVH_X86)
> > +
> > +#define PVH_MAX_CPUS 128
> > +
> > +struct XenPVHx86State {
> > +/*< private >*/
> > +MachineState parent;
> > +
> > +DeviceState *cpu[PVH_MAX_CPUS];
> > +XenPVHCommonState pvh;
> > +
> > +/*
> > + * We provide these properties to allow Xen to move things to other
> > + * addresses for example when users need to accomodate the memory-map
> > + * for 1:1 mapped devices/memory.
> > + */
> > +struct {
> > +MemMapEntry ram_low, ram_high;
> > +MemMapEntry pci_ecam, pci_mmio, pci_mmio_high;
> 
> Can we use the same properties already present under XenPVHCommonState?
> 
> 
> 
> > +} cfg;
> > +};
> > +
> > +static void xenpvh_cpu_new(MachineState *ms,
> > +   XenPVHx86State *xp,
> > +   int cpu_idx,
> > +   int64_t apic_id)
> > +{
> > +Object *cpu = object_new(ms->cpu_type);
> > +
> > +object_property_add_child(OBJECT(ms), "cpu[*]", cpu);
> > +object_property_set_uint(cpu, "apic-id", apic_id, &error_fatal);
> > +qdev_realize(DEVICE(cpu), NULL, &error_fatal);
> > +object_unref(cpu);
> > +
> > +xp->cpu[cpu_idx] = DEVICE(cpu);
> 
> 
> Why do we need to create CPU devices in QEMU given that we are only
> doing device emulation? I guess it is because 
> 
> 
> 
> > +}
> > +
> > +static void xenpvh_init(MachineState *ms)
> > +{
> > +XenPVHx86State *xp = XEN_PVH_X86(ms);
> > +const struct {
> > +const char *name;
> > +MemMapEntry *map;
> > +} map[] = {
> > +{ "ram-low", &xp->cfg.ram_low },
> > +{ "ram-high", &xp->cfg.ram_high },
> > +{ "pci-ecam", &xp->cfg.pci_ecam },
> > +{ "pci-mmio", &xp->cfg.pci_mmio },
> > +{ "pci-mmio-high", &xp->cfg.pci_mmio_high },
> > +};
> > +int i;
> > +
> > +object_initialize_child(OBJECT(ms), "pvh", &xp->pvh, 
> > TYPE_XEN_PVH_COMMON);
> > +object_property_set_int(OBJECT(&xp->pvh), "max-cpus", ms->smp.max_cpus,
> > +&error_abort);
> > +object_property_set_int(OBJECT(&xp->pvh), "ram-size", ms->ram_size,
> > +&error_abort);
> > +
> > +   

Re: [PATCH v1 08/10] hw/xen: pvh-common: Add support for creating PCIe/GPEX

[Edgar E. Iglesias]

On Mon, Aug 12, 2024 at 06:48:37PM -0700, Stefano Stabellini wrote:
> On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > Add support for optionally creating a PCIe/GPEX controller.
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  hw/xen/xen-pvh-common.c | 66 +
> >  include/hw/xen/xen-pvh-common.h | 10 -
> >  2 files changed, 75 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
> > index 69a2dbdb6d..b1432e4bd9 100644
> > --- a/hw/xen/xen-pvh-common.c
> > +++ b/hw/xen/xen-pvh-common.c
> > @@ -120,6 +120,59 @@ static void xen_enable_tpm(XenPVHCommonState *s)
> >  }
> >  #endif
> >  
> > +static void xen_set_pci_intx_irq(void *opaque, int irq, int level)
> > +{
> > +if (xen_set_pci_intx_level(xen_domid, 0, 0, 0, irq, level)) {
> 
> Looking at the implementation of XEN_DMOP_set_pci_intx_level in
> xen/arch/x86/hvm/dm.c, it looks like the device parameter of
> xen_set_pci_intx_level is required?

Yes, by setting device = 0, we're bypassing the irq swizzling in Xen.
I'll try to clarify below.


> 
> 
> > +error_report("xendevicemodel_set_pci_intx_level failed");
> > +}
> > +}
> > +
> > +static inline void xenpvh_gpex_init(XenPVHCommonState *s,
> > +MemoryRegion *sysmem,
> > +hwaddr ecam_base, hwaddr ecam_size,
> > +hwaddr mmio_base, hwaddr mmio_size,
> > +hwaddr mmio_high_base,
> > +hwaddr mmio_high_size,
> > +int intx_irq_base)
> > +{
> > +MemoryRegion *ecam_reg;
> > +MemoryRegion *mmio_reg;
> > +DeviceState *dev;
> > +int i;
> > +
> > +object_initialize_child(OBJECT(s), "gpex", &s->pci.gpex,
> > +TYPE_GPEX_HOST);
> > +dev = DEVICE(&s->pci.gpex);
> > +sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> > +
> > +ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > +memory_region_add_subregion(sysmem, ecam_base, ecam_reg);
> 
> I notice we don't use ecam_size anywhere? Is that because the size is
> standard?

Yes. we could remove the size property, having it slightly simplifies the
prop setting code (keeping these memmap prop-pairs alike) but it's not a big 
deal.

> 
> 
> > +mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> > +
> > +if (mmio_size) {
> > +memory_region_init_alias(&s->pci.mmio_alias, OBJECT(dev), 
> > "pcie-mmio",
> > + mmio_reg, mmio_base, mmio_size);
> > +memory_region_add_subregion(sysmem, mmio_base, &s->pci.mmio_alias);
> > +}
> > +
> > +if (mmio_high_size) {
> > +memory_region_init_alias(&s->pci.mmio_high_alias, OBJECT(dev),
> > +"pcie-mmio-high",
> > +mmio_reg, mmio_high_base, mmio_high_size);
> > +memory_region_add_subregion(sysmem, mmio_high_base,
> > +&s->pci.mmio_high_alias);
> > +}
> > +
> > +for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +qemu_irq irq = qemu_allocate_irq(xen_set_pci_intx_irq, s, i);
> > +
> > +sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
> > +gpex_set_irq_num(GPEX_HOST(dev), i, intx_irq_base + i);
> > +xen_set_pci_link_route(i, intx_irq_base + i);
> 
> xen_set_pci_link_route is not currently implemented on ARM?
> 
> Looking at hw/i386/pc_piix.c:piix_intx_routing_notifier_xen it seems
> that the routing is much more complex over there. But looking at other
> machines that use GPEX such as hw/arm/virt.c it looks like the routing
> is straightforward the same way as in this patch.
> 
> I thought that PCI interrupt pin swizzling was required, but maybe not ?
> 
> It is totally fine if we do something different, simpler, than
> hw/i386/pc_piix.c:piix_intx_routing_notifier_xen. I just want to make
> sure that things remain consistent between ARM and x86, and also between
> Xen and QEMU view of virtual PCI interrupt routing.
>

Good questions. The following is the way I understand things but I may
ofcourse be wrong.

Yes, we're doing things differently than hw/i386/pc_piix.c mainly
because we're using the GPEX PCIe host bridge with it's inte

Re: [PATCH v1 05/10] hw/arm: xenpvh: Break out a common PVH module

[Edgar E. Iglesias]

On Mon, Aug 12, 2024 at 06:47:51PM -0700, Stefano Stabellini wrote:
> On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > Break out a common Xen PVH module in preparation for
> > adding a x86 Xen PVH Machine.
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  hw/arm/trace-events |   5 -
> >  hw/arm/xen_arm.c| 154 ++
> >  hw/xen/meson.build  |   1 +
> >  hw/xen/trace-events |   4 +
> >  hw/xen/xen-pvh-common.c | 185 
> >  include/hw/xen/xen-pvh-common.h |  45 
> >  6 files changed, 269 insertions(+), 125 deletions(-)
> >  create mode 100644 hw/xen/xen-pvh-common.c
> >  create mode 100644 include/hw/xen/xen-pvh-common.h
> > 
> > diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> > index be6c8f720b..c64ad344bd 100644
> > --- a/hw/arm/trace-events
> > +++ b/hw/arm/trace-events
> > @@ -68,10 +68,5 @@ z2_aer915_send_too_long(int8_t msg) "message too long 
> > (%i bytes)"
> >  z2_aer915_send(uint8_t reg, uint8_t value) "reg %d value 0x%02x"
> >  z2_aer915_event(int8_t event, int8_t len) "i2c event =0x%x len=%d bytes"
> >  
> > -# xen_arm.c
> > -xen_create_virtio_mmio_devices(int i, int irq, uint64_t base) "Created 
> > virtio-mmio device %d: irq %d base 0x%"PRIx64
> > -xen_init_ram(uint64_t machine_ram_size) "Initialized xen ram with size 
> > 0x%"PRIx64
> > -xen_enable_tpm(uint64_t addr) "Connected tpmdev at address 0x%"PRIx64
> > -
> >  # bcm2838.c
> >  bcm2838_gic_set_irq(int irq, int level) "gic irq:%d lvl:%d"
> > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> > index ef8315969c..b8a5c09bdf 100644
> > --- a/hw/arm/xen_arm.c
> > +++ b/hw/arm/xen_arm.c
> > @@ -12,40 +12,25 @@
> >  #include "hw/irq.h"
> >  #include "hw/sysbus.h"
> >  #include "sysemu/block-backend.h"
> > -#include "sysemu/tpm_backend.h"
> >  #include "sysemu/sysemu.h"
> > -#include "hw/xen/xen-hvm-common.h"
> > +#include "hw/xen/xen-pvh-common.h"
> >  #include "sysemu/tpm.h"
> >  #include "hw/xen/arch_hvm.h"
> > -#include "trace.h"
> >  
> >  #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
> >  OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
> >  
> > -static const MemoryListener xen_memory_listener = {
> > -.region_add = xen_region_add,
> > -.region_del = xen_region_del,
> > -.log_start = NULL,
> > -.log_stop = NULL,
> > -.log_sync = NULL,
> > -.log_global_start = NULL,
> > -.log_global_stop = NULL,
> > -.priority = MEMORY_LISTENER_PRIORITY_ACCEL,
> > -};
> > -
> >  struct XenArmState {
> >  /*< private >*/
> >  MachineState parent;
> >  
> > -XenIOState *state;
> > +XenPVHCommonState pvh;
> >  
> >  struct {
> >  uint64_t tpm_base_addr;
> >  } cfg;
> >  };
> >  
> > -static MemoryRegion ram_lo, ram_hi;
> > -
> >  /*
> >   * VIRTIO_MMIO_DEV_SIZE is imported from tools/libs/light/libxl_arm.c 
> > under Xen
> >   * repository.
> > @@ -57,64 +42,6 @@ static MemoryRegion ram_lo, ram_hi;
> >  #define NR_VIRTIO_MMIO_DEVICES   \
> > (GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
> >  
> > -static void xen_set_irq(void *opaque, int irq, int level)
> > -{
> > -if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
> > -error_report("xendevicemodel_set_irq_level failed");
> > -}
> > -}
> > -
> > -static void xen_create_virtio_mmio_devices(XenArmState *xam)
> > -{
> > -int i;
> > -
> > -for (i = 0; i < NR_VIRTIO_MMIO_DEVICES; i++) {
> > -hwaddr base = GUEST_VIRTIO_MMIO_BASE + i * VIRTIO_MMIO_DEV_SIZE;
> > -qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
> > - GUEST_VIRTIO_MMIO_SPI_FIRST + i);
> > -
> > -sysbus_create_simple("virtio-mmio", base, irq);
> > -
> > -trace_xen_create_virtio_mmio_devices(i,
> > - GUEST_VIRTIO_MMIO_SPI_FIRST + 
> > i,
> > - base);
> > -}
> > -}
> 
> There are 4 trivial functions in this file:
> 

Re: [PATCH v1 04/10] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

On Tue, Aug 13, 2024 at 03:52:32PM -0700, Stefano Stabellini wrote:
> On Tue, 13 Aug 2024, Edgar E. Iglesias wrote:
> > On Mon, Aug 12, 2024 at 06:47:17PM -0700, Stefano Stabellini wrote:
> > > On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > > > From: "Edgar E. Iglesias" 
> > > > 
> > > > Add SMP support for Xen PVH ARM guests. Create max_cpus ioreq
> > > > servers to handle hotplug.
> > > > 
> > > > Signed-off-by: Edgar E. Iglesias 
> > > > ---
> > > >  hw/arm/xen_arm.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> > > > index 5f75cc3779..ef8315969c 100644
> > > > --- a/hw/arm/xen_arm.c
> > > > +++ b/hw/arm/xen_arm.c
> > > > @@ -173,7 +173,7 @@ static void xen_arm_init(MachineState *machine)
> > > >  
> > > >  xen_init_ram(machine);
> > > >  
> > > > -xen_register_ioreq(xam->state, machine->smp.cpus, 
> > > > &xen_memory_listener);
> > > > +xen_register_ioreq(xam->state, machine->smp.max_cpus, 
> > > > &xen_memory_listener);
> > > >  
> > > >  xen_create_virtio_mmio_devices(xam);
> > > >  
> > > > @@ -218,7 +218,8 @@ static void xen_arm_machine_class_init(ObjectClass 
> > > > *oc, void *data)
> > > >  MachineClass *mc = MACHINE_CLASS(oc);
> > > >  mc->desc = "Xen PVH ARM machine";
> > > >  mc->init = xen_arm_init;
> > > > -mc->max_cpus = 1;
> > > > +/* MAX number of vcpus supported by Xen.  */
> > > > +mc->max_cpus = GUEST_MAX_VCPUS;
> > > 
> > > Will this cause allocations of data structures with 128 elements?
> > > Looking at hw/xen/xen-hvm-common.c:xen_do_ioreq_register it seems
> > > possible? Or hw/xen/xen-hvm-common.c:xen_do_ioreq_register is called
> > 
> > Yes, in theory there's probably overhead with this but as you correctly
> > noted below, a PVH aware xl will set the max_cpus option to a lower value.
> > 
> > With a non-pvh aware xl, I was a little worried about the overhead
> > but I couldn't see any visible slow-down on ARM neither in boot or in 
> > network
> > performance (I didn't run very sophisticated benchmarks).
>  
> What do you mean by "non-pvh aware xl"? All useful versions of xl
> support pvh?


I mean an xl without our PVH patches merged.
xl in upstream doesn't know much about PVH yet.
Even for ARM, we're still carrying significant patches in our tree.


> > > later on with the precise vCPU value which should be provided to QEMU
> > > via the -smp command line option
> > > (tools/libs/light/libxl_dm.c:libxl__build_device_model_args_new)?
> > 
> > Yes, a pvh aware xl will for example pass -smp 2,maxcpus=4 based on
> > values from the xl.cfg. If the user doesn't set maxvcpus in xl.cfg, xl
> > will set maxvcpus to the same value as vcpus.
> 
> OK good. In that case if this is just an initial value meant to be
> overwritten, I think it is best to keep it as 1.

Sorry but that won't work. I think the confusion here may be that
it's easy to mix up mc->max_cpus and machine->smp.max_cpus, these are
not the same. They have different purposes.

I'll try to clarify the 3 values in play.

machine-smp.cpus:
Number of guest vcpus active at boot.
Passed to QEMU via the -smp command-line option.
We don't use this value in QEMU's ARM PVH machines.

machine->smp.max_cpus:
Max number of vcpus that the guest can use (equal or larger than 
machine-smp.cpus).
Will be set by xl via the "-smp X,maxcpus=Y" command-line option to QEMU.
Taken from maxvcpus from xl.cfg, same as XEN_DMOP_nr_vcpus.
This is what we use for xen_register_ioreq().

mc->max_cpus:
Absolute MAX in QEMU used to cap the -smp command-line options.
If xl tries to set -smp (machine->smp.max_cpus) larger than this, QEMU will 
bail out.
Used to setup xen_register_ioreq() ONLY if -smp maxcpus was NOT set (i.e by a 
non PVH aware xl).
Cannot be 1 because that would limit QEMU to MAX 1 vcpu.

I guess we could set mc->max_cpus to what XEN_DMOP_nr_vcpus returns but I'll
have to check if we can even issue that hypercall this early in QEMU since
mc->max_cpus is setup before we even parse the machine options. We may
not yet know what domid we're attaching to yet.

Cheers,
Edgar

Re: [PATCH v1 04/10] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

On Mon, Aug 12, 2024 at 06:47:17PM -0700, Stefano Stabellini wrote:
> On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> > 
> > Add SMP support for Xen PVH ARM guests. Create max_cpus ioreq
> > servers to handle hotplug.
> > 
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  hw/arm/xen_arm.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> > index 5f75cc3779..ef8315969c 100644
> > --- a/hw/arm/xen_arm.c
> > +++ b/hw/arm/xen_arm.c
> > @@ -173,7 +173,7 @@ static void xen_arm_init(MachineState *machine)
> >  
> >  xen_init_ram(machine);
> >  
> > -xen_register_ioreq(xam->state, machine->smp.cpus, 
> > &xen_memory_listener);
> > +xen_register_ioreq(xam->state, machine->smp.max_cpus, 
> > &xen_memory_listener);
> >  
> >  xen_create_virtio_mmio_devices(xam);
> >  
> > @@ -218,7 +218,8 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
> > void *data)
> >  MachineClass *mc = MACHINE_CLASS(oc);
> >  mc->desc = "Xen PVH ARM machine";
> >  mc->init = xen_arm_init;
> > -mc->max_cpus = 1;
> > +/* MAX number of vcpus supported by Xen.  */
> > +mc->max_cpus = GUEST_MAX_VCPUS;
> 
> Will this cause allocations of data structures with 128 elements?
> Looking at hw/xen/xen-hvm-common.c:xen_do_ioreq_register it seems
> possible? Or hw/xen/xen-hvm-common.c:xen_do_ioreq_register is called

Yes, in theory there's probably overhead with this but as you correctly
noted below, a PVH aware xl will set the max_cpus option to a lower value.

With a non-pvh aware xl, I was a little worried about the overhead
but I couldn't see any visible slow-down on ARM neither in boot or in network
performance (I didn't run very sophisticated benchmarks).


> later on with the precise vCPU value which should be provided to QEMU
> via the -smp command line option
> (tools/libs/light/libxl_dm.c:libxl__build_device_model_args_new)?

Yes, a pvh aware xl will for example pass -smp 2,maxcpus=4 based on
values from the xl.cfg. If the user doesn't set maxvcpus in xl.cfg, xl
will set maxvcpus to the same value as vcpus.

Best regards,
Edgar

[PATCH v1 0/1] block/file-posix: Avoid maybe-uninitialized warning

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Hi,

I ran into the following build-warning when building QEMU
with GCC 14.1.0:
[925/1857] Compiling C object libblock.a.p/block_file-posix.c.o
FAILED: libblock.a.p/block_file-posix.c.o 
aarch64-poky-linux-gcc -mcpu=cortex-a57+crc -mbranch-protection=standard 
-fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security 
-Werror=format-security 
--sysroot=/opt/yoxen/arm64/sysroots/cortexa57-poky-linux -Ilibblock.a.p -I. 
-I../qemu -Iqapi -Itrace -Iui -Iui/shader -Iblock 
-I/opt/yoxen/arm64/sysroots/cortexa57-poky-linux/usr/include/glib-2.0 
-I/opt/yoxen/arm64/sysroots/cortexa57-poky-linux/usr/lib/glib-2.0/include 
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g 
-fstack-protector-strong -Wempty-body -Wendif-labels -Wexpansion-to-defined 
-Wformat-security -Wformat-y2k -Wignored-qualifiers -Wimplicit-fallthrough=2 
-Winit-self -Wmissing-format-attribute -Wmissing-prototypes -Wnested-externs 
-Wold-style-declaration -Wold-style-definition -Wredundant-decls -Wshadow=local 
-Wstrict-prototypes -Wtype-limits -Wundef -Wvla -Wwrite-strings 
-Wno-missing-include-dirs -Wno-psabi -Wno-shift-negative-value -isystem 
/home/edgar/src/c/qemu/qemu/linux-headers -isystem linux-headers -iquote . 
-iquote /home/edgar/src/c/qemu/qemu -iquote /home/edgar/src/c/qemu/qemu/include 
-iquote /home/edgar/src/c/qemu/qemu/host/include/aarch64 -iquote 
/home/edgar/src/c/qemu/qemu/host/include/generic -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common 
-fwrapv -ftrivial-auto-var-init=zero -fzero-call-used-regs=used-gpr -O2 -pipe 
-g -feliminate-unused-debug-types -Os -fPIE -MD -MQ 
libblock.a.p/block_file-posix.c.o -MF libblock.a.p/block_file-posix.c.o.d -o 
libblock.a.p/block_file-posix.c.o -c ../qemu/block/file-posix.c
In function ‘raw_refresh_zoned_limits’,
inlined from ‘raw_refresh_limits’ at ../qemu/block/file-posix.c:1522:5:
../qemu/block/file-posix.c:1405:17: error: ‘zoned’ may be used uninitialized 
[-Werror=maybe-uninitialized]
 1405 | if (ret < 0 || zoned == BLK_Z_NONE) {
  | ^~
../qemu/block/file-posix.c:1401:20: note: ‘zoned’ was declared here
 1401 | BlockZoneModel zoned;
  |^
cc1: all warnings being treated as errors

It looks like a false positive and this fix avoids the warning.

Cheers,
Edgar

Edgar E. Iglesias (1):
  block/file-posix: Avoid maybe-uninitialized warning

 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.43.0

[PATCH v1 1/1] block/file-posix: Avoid maybe-uninitialized warning

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Avoid a maybe-uninitialized warning in raw_refresh_zoned_limits()
by initializing zoned.

With GCC 14.1.0:
In function ‘raw_refresh_zoned_limits’,
inlined from ‘raw_refresh_limits’ at ../qemu/block/file-posix.c:1522:5:
../qemu/block/file-posix.c:1405:17: error: ‘zoned’ may be used uninitialized 
[-Werror=maybe-uninitialized]
 1405 | if (ret < 0 || zoned == BLK_Z_NONE) {
  | ^~
../qemu/block/file-posix.c:1401:20: note: ‘zoned’ was declared here
 1401 | BlockZoneModel zoned;
  |^
cc1: all warnings being treated as errors

Signed-off-by: Edgar E. Iglesias 
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index ff928b5e85..90fa54352c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1398,7 +1398,7 @@ static void raw_refresh_zoned_limits(BlockDriverState 
*bs, struct stat *st,
  Error **errp)
 {
 BDRVRawState *s = bs->opaque;
-BlockZoneModel zoned;
+BlockZoneModel zoned = BLK_Z_NONE;
 int ret;
 
 ret = get_sysfs_zoned_model(st, &zoned);
-- 
2.43.0

[PATCH v1 03/10] hw/arm: xenpvh: Tweak machine description

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Tweak machine description to better express that this is
a Xen PVH machine for ARM.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 766a194fa1..5f75cc3779 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -216,7 +216,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 {
 
 MachineClass *mc = MACHINE_CLASS(oc);
-mc->desc = "Xen Para-virtualized PC";
+mc->desc = "Xen PVH ARM machine";
 mc->init = xen_arm_init;
 mc->max_cpus = 1;
 mc->default_machine_opts = "accel=xen";
-- 
2.43.0

[PATCH v1 05/10] hw/arm: xenpvh: Break out a common PVH module

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Break out a common Xen PVH module in preparation for
adding a x86 Xen PVH Machine.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/trace-events |   5 -
 hw/arm/xen_arm.c| 154 ++
 hw/xen/meson.build  |   1 +
 hw/xen/trace-events |   4 +
 hw/xen/xen-pvh-common.c | 185 
 include/hw/xen/xen-pvh-common.h |  45 
 6 files changed, 269 insertions(+), 125 deletions(-)
 create mode 100644 hw/xen/xen-pvh-common.c
 create mode 100644 include/hw/xen/xen-pvh-common.h

diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index be6c8f720b..c64ad344bd 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -68,10 +68,5 @@ z2_aer915_send_too_long(int8_t msg) "message too long (%i 
bytes)"
 z2_aer915_send(uint8_t reg, uint8_t value) "reg %d value 0x%02x"
 z2_aer915_event(int8_t event, int8_t len) "i2c event =0x%x len=%d bytes"
 
-# xen_arm.c
-xen_create_virtio_mmio_devices(int i, int irq, uint64_t base) "Created 
virtio-mmio device %d: irq %d base 0x%"PRIx64
-xen_init_ram(uint64_t machine_ram_size) "Initialized xen ram with size 
0x%"PRIx64
-xen_enable_tpm(uint64_t addr) "Connected tpmdev at address 0x%"PRIx64
-
 # bcm2838.c
 bcm2838_gic_set_irq(int irq, int level) "gic irq:%d lvl:%d"
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index ef8315969c..b8a5c09bdf 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -12,40 +12,25 @@
 #include "hw/irq.h"
 #include "hw/sysbus.h"
 #include "sysemu/block-backend.h"
-#include "sysemu/tpm_backend.h"
 #include "sysemu/sysemu.h"
-#include "hw/xen/xen-hvm-common.h"
+#include "hw/xen/xen-pvh-common.h"
 #include "sysemu/tpm.h"
 #include "hw/xen/arch_hvm.h"
-#include "trace.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
 
-static const MemoryListener xen_memory_listener = {
-.region_add = xen_region_add,
-.region_del = xen_region_del,
-.log_start = NULL,
-.log_stop = NULL,
-.log_sync = NULL,
-.log_global_start = NULL,
-.log_global_stop = NULL,
-.priority = MEMORY_LISTENER_PRIORITY_ACCEL,
-};
-
 struct XenArmState {
 /*< private >*/
 MachineState parent;
 
-XenIOState *state;
+XenPVHCommonState pvh;
 
 struct {
 uint64_t tpm_base_addr;
 } cfg;
 };
 
-static MemoryRegion ram_lo, ram_hi;
-
 /*
  * VIRTIO_MMIO_DEV_SIZE is imported from tools/libs/light/libxl_arm.c under Xen
  * repository.
@@ -57,64 +42,6 @@ static MemoryRegion ram_lo, ram_hi;
 #define NR_VIRTIO_MMIO_DEVICES   \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
 
-static void xen_set_irq(void *opaque, int irq, int level)
-{
-if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
-error_report("xendevicemodel_set_irq_level failed");
-}
-}
-
-static void xen_create_virtio_mmio_devices(XenArmState *xam)
-{
-int i;
-
-for (i = 0; i < NR_VIRTIO_MMIO_DEVICES; i++) {
-hwaddr base = GUEST_VIRTIO_MMIO_BASE + i * VIRTIO_MMIO_DEV_SIZE;
-qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
- GUEST_VIRTIO_MMIO_SPI_FIRST + i);
-
-sysbus_create_simple("virtio-mmio", base, irq);
-
-trace_xen_create_virtio_mmio_devices(i,
- GUEST_VIRTIO_MMIO_SPI_FIRST + i,
- base);
-}
-}
-
-static void xen_init_ram(MachineState *machine)
-{
-MemoryRegion *sysmem = get_system_memory();
-ram_addr_t block_len, ram_size[GUEST_RAM_BANKS];
-
-trace_xen_init_ram(machine->ram_size);
-if (machine->ram_size <= GUEST_RAM0_SIZE) {
-ram_size[0] = machine->ram_size;
-ram_size[1] = 0;
-block_len = GUEST_RAM0_BASE + ram_size[0];
-} else {
-ram_size[0] = GUEST_RAM0_SIZE;
-ram_size[1] = machine->ram_size - GUEST_RAM0_SIZE;
-block_len = GUEST_RAM1_BASE + ram_size[1];
-}
-
-memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
-   &error_fatal);
-
-memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &xen_memory,
- GUEST_RAM0_BASE, ram_size[0]);
-memory_region_add_subregion(sysmem, GUEST_RAM0_BASE, &ram_lo);
-if (ram_size[1] > 0) {
-memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &xen_memory,
- GUEST_RAM1_BASE, ram_size[1]);
-memory_region_add_subregion(sysmem, GUEST_RAM1_BASE, &ram_hi);
-}
-
-/* Setup support for grants.  */
-memory_region_init_ram(&xen

[PATCH v1 09/10] hw/i386/xen: Add a Xen PVH x86 machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This adds a Xen PVH x86 machine based on the PVH Common
module used by the ARM PVH machine.

Signed-off-by: Edgar E. Iglesias 
---
 hw/i386/xen/meson.build |   1 +
 hw/i386/xen/xen-pvh.c   | 196 
 2 files changed, 197 insertions(+)
 create mode 100644 hw/i386/xen/xen-pvh.c

diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 3f0df8bc07..c73c62b8e3 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -4,6 +4,7 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files(
 ))
 i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
   'xen-hvm.c',
+  'xen-pvh.c',
 ))
 
 i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files(
diff --git a/hw/i386/xen/xen-pvh.c b/hw/i386/xen/xen-pvh.c
new file mode 100644
index 00..9c3d3fc58d
--- /dev/null
+++ b/hw/i386/xen/xen-pvh.c
@@ -0,0 +1,196 @@
+/*
+ * QEMU Xen PVH x86 Machine
+ *
+ * Copyright (c) 2024 Advanced Micro Devices, Inc.
+ * Written by Edgar E. Iglesias 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/error-report.h"
+#include "hw/boards.h"
+#include "sysemu/sysemu.h"
+#include "hw/xen/arch_hvm.h"
+#include "hw/xen/xen.h"
+#include "hw/xen/xen-pvh-common.h"
+
+#define TYPE_XEN_PVH_X86  MACHINE_TYPE_NAME("xenpvh")
+OBJECT_DECLARE_SIMPLE_TYPE(XenPVHx86State, XEN_PVH_X86)
+
+#define PVH_MAX_CPUS 128
+
+struct XenPVHx86State {
+/*< private >*/
+MachineState parent;
+
+DeviceState *cpu[PVH_MAX_CPUS];
+XenPVHCommonState pvh;
+
+/*
+ * We provide these properties to allow Xen to move things to other
+ * addresses for example when users need to accomodate the memory-map
+ * for 1:1 mapped devices/memory.
+ */
+struct {
+MemMapEntry ram_low, ram_high;
+MemMapEntry pci_ecam, pci_mmio, pci_mmio_high;
+} cfg;
+};
+
+static void xenpvh_cpu_new(MachineState *ms,
+   XenPVHx86State *xp,
+   int cpu_idx,
+   int64_t apic_id)
+{
+Object *cpu = object_new(ms->cpu_type);
+
+object_property_add_child(OBJECT(ms), "cpu[*]", cpu);
+object_property_set_uint(cpu, "apic-id", apic_id, &error_fatal);
+qdev_realize(DEVICE(cpu), NULL, &error_fatal);
+object_unref(cpu);
+
+xp->cpu[cpu_idx] = DEVICE(cpu);
+}
+
+static void xenpvh_init(MachineState *ms)
+{
+XenPVHx86State *xp = XEN_PVH_X86(ms);
+const struct {
+const char *name;
+MemMapEntry *map;
+} map[] = {
+{ "ram-low", &xp->cfg.ram_low },
+{ "ram-high", &xp->cfg.ram_high },
+{ "pci-ecam", &xp->cfg.pci_ecam },
+{ "pci-mmio", &xp->cfg.pci_mmio },
+{ "pci-mmio-high", &xp->cfg.pci_mmio_high },
+};
+int i;
+
+object_initialize_child(OBJECT(ms), "pvh", &xp->pvh, TYPE_XEN_PVH_COMMON);
+object_property_set_int(OBJECT(&xp->pvh), "max-cpus", ms->smp.max_cpus,
+&error_abort);
+object_property_set_int(OBJECT(&xp->pvh), "ram-size", ms->ram_size,
+&error_abort);
+
+for (i = 0; i < ARRAY_SIZE(map); i++) {
+g_autofree char *base_name = g_strdup_printf("%s-base", map[i].name);
+g_autofree char *size_name = g_strdup_printf("%s-size", map[i].name);
+
+object_property_set_int(OBJECT(&xp->pvh), base_name, map[i].map->base,
+ &error_abort);
+object_property_set_int(OBJECT(&xp->pvh), size_name, map[i].map->size,
+ &error_abort);
+}
+
+/* GSI's 16 - 20 are used for legacy PCIe INTX IRQs.  */
+object_property_set_int(OBJECT(&xp->pvh), "pci-intx-irq-base", 16,
+&error_abort);
+
+sysbus_realize(SYS_BUS_DEVICE(&xp->pvh), &error_abort);
+
+/* Create dummy cores. This will indirectly create the APIC MSI window.  */
+for (i = 0; i < ms->smp.cpus; i++) {
+xenpvh_cpu_new(ms, xp, i, i);
+}
+}
+
+#define XENPVH_PROP_MEMMAP_SETTER(n, f)\
+static void xenpvh_set_ ## n ## _ ## f(Object *obj, Visitor *v,\
+   const char *name, void *opaque, \
+   Error **errp)   \
+{  \
+XenPVHx86State *xp = XEN_PVH_X86(obj); 

[PATCH v1 10/10] docs/system/i386: xenpvh: Add a basic description

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 MAINTAINERS |  1 +
 docs/system/i386/xenpvh.rst | 49 +
 docs/system/target-i386.rst |  1 +
 3 files changed, 51 insertions(+)
 create mode 100644 docs/system/i386/xenpvh.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index a24c2e14d9..da4c9d4d46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -560,6 +560,7 @@ F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
 F: docs/system/arm/xenpvh.rst
+F: docs/system/i386/xenpvh.rst
 
 Guest CPU Cores (NVMM)
 --
diff --git a/docs/system/i386/xenpvh.rst b/docs/system/i386/xenpvh.rst
new file mode 100644
index 00..354250f073
--- /dev/null
+++ b/docs/system/i386/xenpvh.rst
@@ -0,0 +1,49 @@
+Xen PVH machine (``xenpvh``)
+=
+
+Xen supports a spectrum of types of guests that vary in how they depend
+on HW virtualization features, emulation models and paravirtualization.
+PVH is a mode that uses HW virtualization features (like HVM) but tries
+to avoid emulation models and instead use passthrough or
+paravirtualized devices.
+
+QEMU can be used to provide PV virtio devices on an emulated PCIe controller.
+That is the purpose of this minimal machine.
+
+Supported devices
+-
+
+The x86 Xen PVH QEMU machine provide the following devices:
+
+- RAM
+- GPEX host bridge
+- virtio-pci devices
+
+The idea is to only connect virtio-pci devices but in theory any compatible
+PCI device model will work depending on Xen and guest support.
+
+Running
+---
+
+The Xen tools will typically construct a command-line and launch QEMU
+for you when needed. But here's an example of what it can look like in
+case you need to construct one manually:
+
+.. code-block:: console
+
+qemu-system-i386 -xen-domid 3 -no-shutdown\
+  -chardev 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-3,server=on,wait=off \
+  -mon chardev=libxl-cmd,mode=control \
+  -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-3,server=on,wait=off \
+  -mon chardev=libxenstat-cmd,mode=control\
+  -nodefaults \
+  -no-user-config \
+  -xen-attach -name g0\
+  -vnc none   \
+  -display none   \
+  -device virtio-net-pci,id=nic0,netdev=net0,mac=00:16:3e:5c:81:78 \
+  -netdev 
type=tap,id=net0,ifname=vif3.0-emu,br=xenbr0,script=no,downscript=no \
+  -smp 4,maxcpus=4\
+  -nographic  \
+  -machine 
xenpvh,ram-low-base=0,ram-low-size=2147483648,ram-high-base=4294967296,ram-high-size=2147483648,pci-ecam-base=824633720832,pci-ecam-size=268435456,pci-mmio-base=4026531840,pci-mmio-size=33554432,pci-mmio-high-base=824902156288,pci-mmio-high-size=68719476736
 \
+  -m 4096
diff --git a/docs/system/target-i386.rst b/docs/system/target-i386.rst
index 1b8a1f248a..23e84e3ba7 100644
--- a/docs/system/target-i386.rst
+++ b/docs/system/target-i386.rst
@@ -26,6 +26,7 @@ Architectural features
i386/cpu
i386/hyperv
i386/xen
+   i386/xenpvh
i386/kvm-pv
i386/sgx
i386/amd-memory-encryption
-- 
2.43.0

[PATCH v1 04/10] hw/arm: xenpvh: Add support for SMP guests

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add SMP support for Xen PVH ARM guests. Create max_cpus ioreq
servers to handle hotplug.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen_arm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 5f75cc3779..ef8315969c 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -173,7 +173,7 @@ static void xen_arm_init(MachineState *machine)
 
 xen_init_ram(machine);
 
-xen_register_ioreq(xam->state, machine->smp.cpus, &xen_memory_listener);
+xen_register_ioreq(xam->state, machine->smp.max_cpus, 
&xen_memory_listener);
 
 xen_create_virtio_mmio_devices(xam);
 
@@ -218,7 +218,8 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 MachineClass *mc = MACHINE_CLASS(oc);
 mc->desc = "Xen PVH ARM machine";
 mc->init = xen_arm_init;
-mc->max_cpus = 1;
+/* MAX number of vcpus supported by Xen.  */
+mc->max_cpus = GUEST_MAX_VCPUS;
 mc->default_machine_opts = "accel=xen";
 /* Set explicitly here to make sure that real ram_size is passed */
 mc->default_ram_size = 0;
-- 
2.43.0

[PATCH v1 02/10] hw/arm: xenpvh: Update file header to use SPDX

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Update file header to use SPDX and remove stray empty
comment line.

No functional changes.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xen_arm.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 6fad829ede..766a194fa1 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -1,24 +1,7 @@
 /*
  * QEMU ARM Xen PVH Machine
  *
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
+ * SPDX-License-Identifier: MIT
  */
 
 #include "qemu/osdep.h"
-- 
2.43.0

[PATCH v1 07/10] hw/arm: xenpvh: Reverse virtio-mmio creation order

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

We've been creating the virtio-mmio devices in forwards order
but since the qbus lists prepend (rather than append) entries,
the virtio busses end up with decreasing base address order.

Xen enables virtio-mmio nodes in forwards order so there's been
a missmatch. So far, we've been working around this with an
out-of-tree patch to Xen.

This reverses the order making sure the virtio busses end up
ordered with increasing base addresses avoiding the need to
patch Xen.

Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-pvh-common.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 0d368398d0..69a2dbdb6d 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -73,7 +73,18 @@ static void xen_create_virtio_mmio_devices(XenPVHCommonState 
*s)
 {
 int i;
 
-for (i = 0; i < s->cfg.virtio_mmio_num; i++) {
+/*
+ * We create the transports in reverse order. Since qbus_realize()
+ * prepends (not appends) new child buses, the decrementing loop below will
+ * create a list of virtio-mmio buses with increasing base addresses.
+ *
+ * When a -device option is processed from the command line,
+ * qbus_find_recursive() picks the next free virtio-mmio bus in forwards
+ * order.
+ *
+ * This is what the Xen tools expect.
+ */
+for (i = s->cfg.virtio_mmio_num - 1; i >= 0; i--) {
 hwaddr base = s->cfg.virtio_mmio.base + i * s->cfg.virtio_mmio.size;
 qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
  s->cfg.virtio_mmio_irq_base + i);
-- 
2.43.0

[PATCH v1 08/10] hw/xen: pvh-common: Add support for creating PCIe/GPEX

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Add support for optionally creating a PCIe/GPEX controller.

Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-pvh-common.c | 66 +
 include/hw/xen/xen-pvh-common.h | 10 -
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-pvh-common.c b/hw/xen/xen-pvh-common.c
index 69a2dbdb6d..b1432e4bd9 100644
--- a/hw/xen/xen-pvh-common.c
+++ b/hw/xen/xen-pvh-common.c
@@ -120,6 +120,59 @@ static void xen_enable_tpm(XenPVHCommonState *s)
 }
 #endif
 
+static void xen_set_pci_intx_irq(void *opaque, int irq, int level)
+{
+if (xen_set_pci_intx_level(xen_domid, 0, 0, 0, irq, level)) {
+error_report("xendevicemodel_set_pci_intx_level failed");
+}
+}
+
+static inline void xenpvh_gpex_init(XenPVHCommonState *s,
+MemoryRegion *sysmem,
+hwaddr ecam_base, hwaddr ecam_size,
+hwaddr mmio_base, hwaddr mmio_size,
+hwaddr mmio_high_base,
+hwaddr mmio_high_size,
+int intx_irq_base)
+{
+MemoryRegion *ecam_reg;
+MemoryRegion *mmio_reg;
+DeviceState *dev;
+int i;
+
+object_initialize_child(OBJECT(s), "gpex", &s->pci.gpex,
+TYPE_GPEX_HOST);
+dev = DEVICE(&s->pci.gpex);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_add_subregion(sysmem, ecam_base, ecam_reg);
+
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+
+if (mmio_size) {
+memory_region_init_alias(&s->pci.mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg, mmio_base, mmio_size);
+memory_region_add_subregion(sysmem, mmio_base, &s->pci.mmio_alias);
+}
+
+if (mmio_high_size) {
+memory_region_init_alias(&s->pci.mmio_high_alias, OBJECT(dev),
+"pcie-mmio-high",
+mmio_reg, mmio_high_base, mmio_high_size);
+memory_region_add_subregion(sysmem, mmio_high_base,
+&s->pci.mmio_high_alias);
+}
+
+for (i = 0; i < GPEX_NUM_IRQS; i++) {
+qemu_irq irq = qemu_allocate_irq(xen_set_pci_intx_irq, s, i);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
+gpex_set_irq_num(GPEX_HOST(dev), i, intx_irq_base + i);
+xen_set_pci_link_route(i, intx_irq_base + i);
+}
+}
+
 static void xen_pvh_realize(DeviceState *dev, Error **errp)
 {
 XenPVHCommonState *s = XEN_PVH_COMMON(dev);
@@ -152,6 +205,14 @@ static void xen_pvh_realize(DeviceState *dev, Error **errp)
 warn_report("tpm-base-addr is not provided. TPM will not be enabled");
 }
 #endif
+
+if (s->cfg.ecam.size) {
+xenpvh_gpex_init(s, sysmem,
+ s->cfg.ecam.base, s->cfg.ecam.size,
+ s->cfg.mmio.base, s->cfg.mmio.size,
+ s->cfg.mmio_high.base, s->cfg.mmio_high.size,
+ s->cfg.pci_intx_irq_base);
+}
 }
 
 #define DEFINE_PROP_MEMMAP(n, f) \
@@ -165,10 +226,15 @@ static Property xen_pvh_properties[] = {
 DEFINE_PROP_MEMMAP("ram-high", ram_high),
 DEFINE_PROP_MEMMAP("virtio-mmio", virtio_mmio),
 DEFINE_PROP_MEMMAP("tpm", tpm),
+DEFINE_PROP_MEMMAP("pci-ecam", ecam),
+DEFINE_PROP_MEMMAP("pci-mmio", mmio),
+DEFINE_PROP_MEMMAP("pci-mmio-high", mmio_high),
 DEFINE_PROP_UINT32("virtio-mmio-num", XenPVHCommonState,
cfg.virtio_mmio_num, 0),
 DEFINE_PROP_UINT32("virtio-mmio-irq-base", XenPVHCommonState,
cfg.virtio_mmio_irq_base, 0),
+DEFINE_PROP_UINT32("pci-intx-irq-base", XenPVHCommonState,
+   cfg.pci_intx_irq_base, 0),
 DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/include/hw/xen/xen-pvh-common.h b/include/hw/xen/xen-pvh-common.h
index e958b441fd..faacfca70e 100644
--- a/include/hw/xen/xen-pvh-common.h
+++ b/include/hw/xen/xen-pvh-common.h
@@ -29,17 +29,25 @@ typedef struct XenPVHCommonState {
 MemoryRegion high;
 } ram;
 
+struct {
+GPEXHost gpex;
+MemoryRegion mmio_alias;
+MemoryRegion mmio_high_alias;
+} pci;
+
 struct {
 uint64_t ram_size;
 uint32_t max_cpus;
 uint32_t virtio_mmio_num;
 uint32_t virtio_mmio_irq_base;
+uint32_t pci_intx_irq_base;
 struct {
 uint64_t base;
 uint64_t size;
 } ram_low, ram_high,
   virtio_mmio,
-  tpm;
+  tpm,
+  ecam, mmio, mmio_high;
 } cfg;
 } XenPVHCommonState;
 #endif
-- 
2.43.0

[PATCH v1 06/10] hw/arm: xenpvh: Rename xen_arm.c -> xen-pvh.c

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Rename xen_arm.c -> xen-pvh.c to better express that this
is a PVH machine and to align with x86 HVM and future PVH
machine filenames:
hw/i386/xen/xen-hvm.c
hw/i386/xen/xen-pvh.c (in preparation)

No functional changes.

Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/meson.build  | 2 +-
 hw/arm/{xen_arm.c => xen-pvh.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename hw/arm/{xen_arm.c => xen-pvh.c} (100%)

diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 0c07ab522f..769fe9ec1a 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -59,7 +59,7 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true: 
files('fsl-imx7.c', 'mcimx7d-sabre.
 arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
 arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 
'mcimx6ul-evk.c'))
 arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
-arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
+arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen-pvh.c'))
 
 system_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
 system_ss.add(when: 'CONFIG_CHEETAH', if_true: files('palm.c'))
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen-pvh.c
similarity index 100%
rename from hw/arm/xen_arm.c
rename to hw/arm/xen-pvh.c
-- 
2.43.0

[PATCH v1 01/10] MAINTAINERS: Add docs/system/arm/xenpvh.rst

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Signed-off-by: Edgar E. Iglesias 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 10af212632..a24c2e14d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -559,6 +559,7 @@ F: include/hw/xen/
 F: include/sysemu/xen.h
 F: include/sysemu/xen-mapcache.h
 F: stubs/xen-hw-stub.c
+F: docs/system/arm/xenpvh.rst
 
 Guest CPU Cores (NVMM)
 --
-- 
2.43.0

[PATCH v1 00/10] xen: pvh: Partial QOM:fication with new x86 PVH machine

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This series breaks-out parts of the ARM PVH support into a reusable
QOM module. There's a bit of refactoring and some bug-fixes along
the way.

Finally we add a new x86 xen-pvh machine using the new xen-pvh-common
module.

The corresponding changes Xen for PVH x86 are work in progress
(by Xenia Ragiadakou). You can find a current version here:
https://github.com/edgarigl/xen/tree/edgar/virtio-pvh-upstream 

I've briefly described the steps to run Xen PVH x86 guests here
(including example guest kernel config, xl.cfg and xl command-lines):
https://github.com/edgarigl/docs/blob/master/xen/pvh/xenpvh-x86.md

Cheers,
Edgar

Edgar E. Iglesias (10):
  MAINTAINERS: Add docs/system/arm/xenpvh.rst
  hw/arm: xenpvh: Update file header to use SPDX
  hw/arm: xenpvh: Tweak machine description
  hw/arm: xenpvh: Add support for SMP guests
  hw/arm: xenpvh: Break out a common PVH module
  hw/arm: xenpvh: Rename xen_arm.c -> xen-pvh.c
  hw/arm: xenpvh: Reverse virtio-mmio creation order
  hw/xen: pvh-common: Add support for creating PCIe/GPEX
  hw/i386/xen: Add a Xen PVH x86 machine
  docs/system/i386: xenpvh: Add a basic description

 MAINTAINERS |   2 +
 docs/system/i386/xenpvh.rst |  49 ++
 docs/system/target-i386.rst |   1 +
 hw/arm/meson.build  |   2 +-
 hw/arm/trace-events |   5 -
 hw/arm/xen-pvh.c| 165 
 hw/arm/xen_arm.c| 267 
 hw/i386/xen/meson.build |   1 +
 hw/i386/xen/xen-pvh.c   | 196 +++
 hw/xen/meson.build  |   1 +
 hw/xen/trace-events |   4 +
 hw/xen/xen-pvh-common.c | 262 +++
 include/hw/xen/xen-pvh-common.h |  53 +++
 13 files changed, 735 insertions(+), 273 deletions(-)
 create mode 100644 docs/system/i386/xenpvh.rst
 create mode 100644 hw/arm/xen-pvh.c
 delete mode 100644 hw/arm/xen_arm.c
 create mode 100644 hw/i386/xen/xen-pvh.c
 create mode 100644 hw/xen/xen-pvh-common.c
 create mode 100644 include/hw/xen/xen-pvh-common.h

-- 
2.43.0

Re: [PATCH 0/2] Consolidate embedded PPC initial mappung functions

[Edgar E. Iglesias]

On Tue, Jul 16, 2024 at 02:07:56PM +0200, BALATON Zoltan wrote:
> Embedded PPC has always enabled MMU so it needs initial mappings to
> start. This code is duplicated within machines which this small series
> aims to consolidate into helper functions to reduce duplicated code
> and make the board code simpler.
> 
> Regards,
> BALATON Zoltan

I tested this with my virtex-ml507 images:

Reviewed-by: Edgar E. Iglesias 
Tested-by: Edgar E. Iglesias 

Cheers,
Edgar


> 
> BALATON Zoltan (2):
>   hw/ppc: Consolidate e500 initial mapping creation functions
>   hw/ppc: Consolidate ppc440 initial mapping creation functions
> 
>  hw/ppc/e500.c  | 41 ++
>  hw/ppc/e500.h  |  2 --
>  hw/ppc/ppc440_bamboo.c | 28 +++---
>  hw/ppc/ppc_booke.c | 10 ++
>  hw/ppc/ppce500_spin.c  | 30 +---
>  hw/ppc/sam460ex.c  | 45 ++
>  hw/ppc/virtex_ml507.c  | 28 +++---
>  include/hw/ppc/ppc.h   |  7 +++
>  8 files changed, 61 insertions(+), 130 deletions(-)
> 
> -- 
> 2.30.9
> 

[PULL v1 2/3] physmem: Bail out qemu_ram_block_from_host() for invalid ram addrs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Bail out in qemu_ram_block_from_host() when
xen_ram_addr_from_mapcache() does not find an existing
mapping.

Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Alex Bennée 
Reviewed-by: Stefano Stabellini 
---
 system/physmem.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/system/physmem.c b/system/physmem.c
index 14aa025d41..2154432cb6 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2277,6 +2277,10 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool 
round_offset,
 ram_addr_t ram_addr;
 RCU_READ_LOCK_GUARD();
 ram_addr = xen_ram_addr_from_mapcache(ptr);
+if (ram_addr == RAM_ADDR_INVALID) {
+return NULL;
+}
+
 block = qemu_get_ram_block(ram_addr);
 if (block) {
 *offset = ram_addr - block->offset;
-- 
2.43.0

[PULL v1 3/3] xen: mapcache: Fix unmapping of first entries in buckets

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This fixes the clobbering of the entry->next pointer when
unmapping the first entry in a bucket of a mapcache.

Fixes: 123acd816d ("xen: mapcache: Unmap first entries in buckets")
Reported-by: Anthony PERARD 
Signed-off-by: Edgar E. Iglesias 
Reviewed-by: Anthony PERARD 
Reviewed-by: Stefano Stabellini 
---
 hw/xen/xen-mapcache.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 5f23b0adbe..18ba7b1d8f 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -597,7 +597,17 @@ static void 
xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
 pentry->next = entry->next;
 g_free(entry);
 } else {
-memset(entry, 0, sizeof *entry);
+/*
+ * Invalidate mapping but keep entry->next pointing to the rest
+ * of the list.
+ *
+ * Note that lock is already zero here, otherwise we don't unmap.
+ */
+entry->paddr_index = 0;
+entry->vaddr_base = NULL;
+entry->valid_mapping = NULL;
+entry->flags = 0;
+entry->size = 0;
 }
 }
 
-- 
2.43.0

[PULL v1 0/3] Xen queue

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

The following changes since commit 23901b2b721c0576007ab7580da8aa855d6042a9:

  Merge tag 'pull-target-arm-20240711' of 
https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-07-11 
12:00:00 -0700)

are available in the Git repository at:

  https://gitlab.com/edgar.iglesias/qemu.git 
tags/edgar/xen-queue-2024-07-12.for-upstream

for you to fetch changes up to 872cb9cced796e75d4f719c31d70ed5fd629efca:

  xen: mapcache: Fix unmapping of first entries in buckets (2024-07-12 00:17:36 
+0200)


Edgars Xen queue.

--------
Edgar E. Iglesias (2):
  physmem: Bail out qemu_ram_block_from_host() for invalid ram addrs
  xen: mapcache: Fix unmapping of first entries in buckets

Stefano Stabellini (1):
  MAINTAINERS: add Edgar as Xen maintainer

 MAINTAINERS   |  1 +
 hw/xen/xen-mapcache.c | 12 +++-
 system/physmem.c  |  4 
 3 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.43.0

[PULL v1 1/3] MAINTAINERS: add Edgar as Xen maintainer

[Edgar E. Iglesias]

From: Stefano Stabellini 

Add Edgar as Xen subsystem maintainer in QEMU. Edgar has been a QEMU
maintainer for years, and has already made key changes to one of the
most difficult areas of the Xen subsystem (the mapcache).

Edgar volunteered helping us maintain the Xen subsystem in QEMU and we
are very happy to welcome him to the team. His knowledge and expertise
with QEMU internals will be of great help.

Signed-off-by: Stefano Stabellini 
Reviewed-by: Paul Durrant 
Acked-by: Anthony PERARD 
Reviewed-by: Alex Bennée 
Signed-off-by: Edgar E. Iglesias 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6725913c8b..63e11095a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -536,6 +536,7 @@ X86 Xen CPUs
 M: Stefano Stabellini 
 M: Anthony PERARD 
 M: Paul Durrant 
+M: Edgar E. Iglesias 
 L: xen-de...@lists.xenproject.org
 S: Supported
 F: */xen*
-- 
2.43.0

Re: [PATCH] MAINTAINERS: add Edgar as Xen maintainer

[Edgar E. Iglesias]

On Thu, Jul 11, 2024 at 12:09 PM Alex Bennée  wrote:

> Stefano Stabellini  writes:
>
> > Add Edgar as Xen subsystem maintainer in QEMU. Edgar has been a QEMU
> > maintainer for years, and has already made key changes to one of the
> > most difficult areas of the Xen subsystem (the mapcache).
> >
> > Edgar volunteered helping us maintain the Xen subsystem in QEMU and we
> > are very happy to welcome him to the team. His knowledge and expertise
> > with QEMU internals will be of great help.
> >
> > Signed-off-by: Stefano Stabellini 
>
> Reviewed-by: Alex Bennée 
>
>
Thanks all!

I'll put this together with the reviewed mapcache fixes into a pull-request!

Best regards,
Edgar

Re: [PATCH] MAINTAINERS: add Edgar as Xen maintainer

[Edgar E. Iglesias]

On Wed, Jul 10, 2024 at 10:28 PM Stefano Stabellini 
wrote:

> Add Edgar as Xen subsystem maintainer in QEMU. Edgar has been a QEMU
> maintainer for years, and has already made key changes to one of the
> most difficult areas of the Xen subsystem (the mapcache).
>
> Edgar volunteered helping us maintain the Xen subsystem in QEMU and we
> are very happy to welcome him to the team. His knowledge and expertise
> with QEMU internals will be of great help.
>
> Signed-off-by: Stefano Stabellini 
>
>
Thanks Stefano!

Reviewed-by: Edgar E. Iglesias 




> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6725913c8b..63e11095a2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -536,6 +536,7 @@ X86 Xen CPUs
>  M: Stefano Stabellini 
>  M: Anthony PERARD 
>  M: Paul Durrant 
> +M: Edgar E. Iglesias 
>  L: xen-de...@lists.xenproject.org
>  S: Supported
>  F: */xen*
>

Re: [PATCH v1 2/2] xen: mapcache: Fix unmapping of first entries in buckets

[Edgar E. Iglesias]

On Thu, Jul 4, 2024 at 9:48 PM Edgar E. Iglesias 
wrote:

> On Thu, Jul 04, 2024 at 05:44:52PM +0100, Alex Bennée wrote:
> > Anthony PERARD  writes:
> >
> > > On Tue, Jul 02, 2024 at 12:44:21AM +0200, Edgar E. Iglesias wrote:
> > >> From: "Edgar E. Iglesias" 
> > >>
> > >> This fixes the clobbering of the entry->next pointer when
> > >> unmapping the first entry in a bucket of a mapcache.
> > >>
> > >> Fixes: 123acd816d ("xen: mapcache: Unmap first entries in buckets")
> > >> Reported-by: Anthony PERARD 
> > >> Signed-off-by: Edgar E. Iglesias 
> > >> ---
> > >>  hw/xen/xen-mapcache.c | 12 +++-
> > >>  1 file changed, 11 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> > >> index 5f23b0adbe..18ba7b1d8f 100644
> > >> --- a/hw/xen/xen-mapcache.c
> > >> +++ b/hw/xen/xen-mapcache.c
> > >> @@ -597,7 +597,17 @@ static void
> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
> > >>  pentry->next = entry->next;
> > >>  g_free(entry);
> > >>  } else {
> > >> -memset(entry, 0, sizeof *entry);
> > >> +/*
> > >> + * Invalidate mapping but keep entry->next pointing to the
> rest
> > >> + * of the list.
> > >> + *
> > >> + * Note that lock is already zero here, otherwise we don't
> unmap.
> > >> + */
> > >> +entry->paddr_index = 0;
> > >> +entry->vaddr_base = NULL;
> > >> +entry->valid_mapping = NULL;
> > >> +entry->flags = 0;
> > >> +entry->size = 0;
> > >
> > > This kind of feels like mc->entry should be an array of pointer rather
> > > than an array of MapCacheEntry but that seems to work well enough and
> > > not the first time entries are been cleared like that.
> >
> > The use of a hand rolled list is a bit of a concern considering QEMU and
> > Glib both provide various abstractions used around the rest of the code
> > base. The original patch that introduces the mapcache doesn't tell me
> > much about access patterns for the cache, just that it is trying to
> > solve memory exhaustion issues with lots of dynamic small mappings.
> >
> > Maybe a simpler structure is desirable?
> >
> > We also have an interval tree implementation ("qemu/interval-tree.h") if
> > what we really want is a sorted tree of memory that can be iterated
> > locklessly.
> >
>
> Yes, it would be interesting to benchmark other options.
> I agree that we should at minimum reuse existing lists/hash tables.
>
> We've also had some discussions around removing it partially or
> alltogether but
> there are some concerns around that. We're going to need something to
> keep track of grants. For 32-bit hosts, it's a problem to exhaust virtual
> address-space if mapping all of the guest (are folks still using 32-bit
> hosts?).
> There may be other issues aswell.
>
> Some benefits are that we'll remove some of the complexity and latency for
> mapping
> and unmapping stuff continously.
>
>
One more thing I forgot to add is that IMO, these larger longer term
changes should not block this tiny bugfix...

Cheers,
Edgar

Re: [PATCH v1 2/2] xen: mapcache: Fix unmapping of first entries in buckets

[Edgar E. Iglesias]

On Thu, Jul 04, 2024 at 05:44:52PM +0100, Alex Bennée wrote:
> Anthony PERARD  writes:
> 
> > On Tue, Jul 02, 2024 at 12:44:21AM +0200, Edgar E. Iglesias wrote:
> >> From: "Edgar E. Iglesias" 
> >> 
> >> This fixes the clobbering of the entry->next pointer when
> >> unmapping the first entry in a bucket of a mapcache.
> >> 
> >> Fixes: 123acd816d ("xen: mapcache: Unmap first entries in buckets")
> >> Reported-by: Anthony PERARD 
> >> Signed-off-by: Edgar E. Iglesias 
> >> ---
> >>  hw/xen/xen-mapcache.c | 12 +++-
> >>  1 file changed, 11 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> >> index 5f23b0adbe..18ba7b1d8f 100644
> >> --- a/hw/xen/xen-mapcache.c
> >> +++ b/hw/xen/xen-mapcache.c
> >> @@ -597,7 +597,17 @@ static void 
> >> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
> >>  pentry->next = entry->next;
> >>  g_free(entry);
> >>  } else {
> >> -memset(entry, 0, sizeof *entry);
> >> +/*
> >> + * Invalidate mapping but keep entry->next pointing to the rest
> >> + * of the list.
> >> + *
> >> + * Note that lock is already zero here, otherwise we don't unmap.
> >> + */
> >> +entry->paddr_index = 0;
> >> +entry->vaddr_base = NULL;
> >> +entry->valid_mapping = NULL;
> >> +entry->flags = 0;
> >> +entry->size = 0;
> >
> > This kind of feels like mc->entry should be an array of pointer rather
> > than an array of MapCacheEntry but that seems to work well enough and
> > not the first time entries are been cleared like that.
> 
> The use of a hand rolled list is a bit of a concern considering QEMU and
> Glib both provide various abstractions used around the rest of the code
> base. The original patch that introduces the mapcache doesn't tell me
> much about access patterns for the cache, just that it is trying to
> solve memory exhaustion issues with lots of dynamic small mappings.
> 
> Maybe a simpler structure is desirable?
> 
> We also have an interval tree implementation ("qemu/interval-tree.h") if
> what we really want is a sorted tree of memory that can be iterated
> locklessly.
> 

Yes, it would be interesting to benchmark other options.
I agree that we should at minimum reuse existing lists/hash tables.

We've also had some discussions around removing it partially or alltogether but
there are some concerns around that. We're going to need something to
keep track of grants. For 32-bit hosts, it's a problem to exhaust virtual
address-space if mapping all of the guest (are folks still using 32-bit hosts?).
There may be other issues aswell.

Some benefits are that we'll remove some of the complexity and latency for 
mapping
and unmapping stuff continously.

Cheers,
Edgar

Re: [PATCH v1 1/2] physmem: Bail out qemu_ram_block_from_host() for invalid ram addrs

[Edgar E. Iglesias]

On Thu, Jul 4, 2024 at 1:26 PM Alex Bennée  wrote:

> "Edgar E. Iglesias"  writes:
>
> > From: "Edgar E. Iglesias" 
> >
> > Bail out in qemu_ram_block_from_host() when
> > xen_ram_addr_from_mapcache() does not find an existing
> > mapping.
> >
> > Signed-off-by: Edgar E. Iglesias 
> > ---
> >  system/physmem.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/system/physmem.c b/system/physmem.c
> > index 33d09f7571..59d1576c2b 100644
> > --- a/system/physmem.c
> > +++ b/system/physmem.c
> > @@ -2277,6 +2277,10 @@ RAMBlock *qemu_ram_block_from_host(void *ptr,
> bool round_offset,
> >  ram_addr_t ram_addr;
> >  RCU_READ_LOCK_GUARD();
> >  ram_addr = xen_ram_addr_from_mapcache(ptr);
> > +if (ram_addr == RAM_ADDR_INVALID) {
> > +return NULL;
> > +}
> > +
>
> Isn't this indicative of a failure? Should there at least be a trace
> point for failed mappings?
>
>
Yes but there are already trace points for the failure cases inside
xen_ram_addr_from_mapcache().
Do those address your concerns or do you think we need additional trace
points?

Cheers,
Edgar


> >  block = qemu_get_ram_block(ram_addr);
> >  if (block) {
> >  *offset = ram_addr - block->offset;
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>

[PATCH v1 1/2] physmem: Bail out qemu_ram_block_from_host() for invalid ram addrs

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Bail out in qemu_ram_block_from_host() when
xen_ram_addr_from_mapcache() does not find an existing
mapping.

Signed-off-by: Edgar E. Iglesias 
---
 system/physmem.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/system/physmem.c b/system/physmem.c
index 33d09f7571..59d1576c2b 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2277,6 +2277,10 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool 
round_offset,
 ram_addr_t ram_addr;
 RCU_READ_LOCK_GUARD();
 ram_addr = xen_ram_addr_from_mapcache(ptr);
+if (ram_addr == RAM_ADDR_INVALID) {
+return NULL;
+}
+
 block = qemu_get_ram_block(ram_addr);
 if (block) {
 *offset = ram_addr - block->offset;
-- 
2.43.0

[PATCH v1 0/2] xen: mapcache: Fix unmapping of first the entry in a bucket

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This fixes the unmapping of the first mapping in a bucket of a mapcache.

We also add error handling to qemu_ram_block_from_host() to bail out when
xen_ram_addr_from_mapcache() doesn't find an existing mapping.

Cheers,
Edgar

Edgar E. Iglesias (2):
  physmem: Bail out qemu_ram_block_from_host() for invalid ram addrs
  xen: mapcache: Fix unmapping of first entries in buckets

 hw/xen/xen-mapcache.c | 12 +++-
 system/physmem.c  |  4 
 2 files changed, 15 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v1 2/2] xen: mapcache: Fix unmapping of first entries in buckets

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

This fixes the clobbering of the entry->next pointer when
unmapping the first entry in a bucket of a mapcache.

Fixes: 123acd816d ("xen: mapcache: Unmap first entries in buckets")
Reported-by: Anthony PERARD 
Signed-off-by: Edgar E. Iglesias 
---
 hw/xen/xen-mapcache.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 5f23b0adbe..18ba7b1d8f 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -597,7 +597,17 @@ static void 
xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
 pentry->next = entry->next;
 g_free(entry);
 } else {
-memset(entry, 0, sizeof *entry);
+/*
+ * Invalidate mapping but keep entry->next pointing to the rest
+ * of the list.
+ *
+ * Note that lock is already zero here, otherwise we don't unmap.
+ */
+entry->paddr_index = 0;
+entry->vaddr_base = NULL;
+entry->valid_mapping = NULL;
+entry->flags = 0;
+entry->size = 0;
 }
 }
 
-- 
2.43.0

Re: [PATCH v8 2/8] xen: mapcache: Unmap first entries in buckets

[Edgar E. Iglesias]

On Mon, Jul 1, 2024 at 6:21 PM Anthony PERARD 
wrote:

> On Mon, Jul 01, 2024 at 04:34:53PM +0200, Edgar E. Iglesias wrote:
> > On Mon, Jul 1, 2024 at 4:30 PM Edgar E. Iglesias <
> edgar.igles...@gmail.com>
> > wrote:
> > > On Mon, Jul 1, 2024 at 3:58 PM Edgar E. Iglesias <
> edgar.igles...@gmail.com>
> > > wrote:
> > >> Any chance you could try to get a backtrace from QEMU when it failed?
>
> Here it is:
>
>
> #3  0x7fa8762f4472 in __GI_abort () at ./stdlib/abort.c:79
> save_stage = 1
> act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction =
> 0x20}, sa_mask = {__val = {94603440166168, 18446744073709551615,
> 94603406369640, 0, 0, 94603406346720, 94603440166168, 140361486774256, 0,
> 140361486773376, 94603401285536, 140361496232688, 94603440166096,
> 140361486773456, 94603401289576, 140360849280256}}, sa_flags = -1804462896,
> sa_restorer = 0x748f2d40}
> #4  0x560a92230f0d in qemu_get_ram_block (addr=18446744073709551615)
> at ../system/physmem.c:801
> block = 0x0
> #5  0x560a922350ab in qemu_ram_block_from_host (ptr=0x7fa84e8fcd00,
> round_offset=false, offset=0x7fa8748f2de8) at ../system/physmem.c:2280
> ram_addr = 18446744073709551615
> _rcu_read_auto = 0x1
> block = 0x0
> host = 0x7fa84e8fcd00 ""
> _rcu_read_auto = 0x7fa8751f8288
> #6  0x560a92229669 in memory_region_from_host (ptr=0x7fa84e8fcd00,
> offset=0x7fa8748f2de8) at ../system/memory.c:2440
> block = 0x0
> #7  0x560a92237418 in address_space_unmap (as=0x560a94b05a20,
> buffer=0x7fa84e8fcd00, len=32768, is_write=true, access_len=32768) at
> ../system/physmem.c:3246
> mr = 0x0
> addr1 = 0
> __PRETTY_FUNCTION__ = "address_space_unmap"
> #8  0x560a91fd6cd3 in dma_memory_unmap (as=0x560a94b05a20,
> buffer=0x7fa84e8fcd00, len=32768, dir=DMA_DIRECTION_FROM_DEVICE,
> access_len=32768) at /root/build/qemu/include/sysemu/dma.h:236
> #9  0x560a91fd763d in dma_blk_unmap (dbs=0x560a94d87400) at
> ../system/dma-helpers.c:93
> i = 1
> #10 0x560a91fd76e6 in dma_complete (dbs=0x560a94d87400, ret=0) at
> ../system/dma-helpers.c:105
> __PRETTY_FUNCTION__ = "dma_complete"
> #11 0x560a91fd781c in dma_blk_cb (opaque=0x560a94d87400, ret=0) at
> ../system/dma-helpers.c:129
> dbs = 0x560a94d87400
> ctx = 0x560a9448da90
> cur_addr = 0
> cur_len = 0
> mem = 0x0
> __PRETTY_FUNCTION__ = "dma_blk_cb"
> #12 0x560a9232e974 in blk_aio_complete (acb=0x560a9448d5f0) at
> ../block/block-backend.c:1555
> #13 0x560a9232ebd1 in blk_aio_read_entry (opaque=0x560a9448d5f0) at
> ../block/block-backend.c:1610
> acb = 0x560a9448d5f0
> rwco = 0x560a9448d618
> qiov = 0x560a94d87460
> __PRETTY_FUNCTION__ = "blk_aio_read_entry"
>
> > > One more thing, regarding this specific patch. I don't think we should
> > > clear the
> > > entire entry, the next field should be kept, otherwise we'll disconnect
> > > following
> > > mappings that will never be found again. IIUC, this could very well be
> > > causing the problem you see.
> > >
> > > Does the following make sense?
> > >
> > And here without double-freeing entry->valid_mapping:
> >
> > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> > index 5f23b0adbe..667807b3b6 100644
> > --- a/hw/xen/xen-mapcache.c
> > +++ b/hw/xen/xen-mapcache.c
> > @@ -597,7 +597,13 @@ static void
> > xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
> >  pentry->next = entry->next;
> >  g_free(entry);
> >  } else {
> > -memset(entry, 0, sizeof *entry);
> > +/* Invalidate mapping.  */
> > +entry->paddr_index = 0;
> > +entry->vaddr_base = NULL;
> > +entry->size = 0;
> > +entry->valid_mapping = NULL;
> > +entry->flags = 0;
> > +/* Keep entry->next pointing to the rest of the list.  */
> >  }
> >  }
>
> I've tried this patch, and that fix the issue I've seen. I'll run more
> tests on it, just in case, but there's no reason that would break
> something else.
>
> Cheers,
>

Thanks Anthony, I'll send out a proper patch tomorrow.

Cheers,
Edgar

Re: [PATCH v8 2/8] xen: mapcache: Unmap first entries in buckets

[Edgar E. Iglesias]

On Mon, Jul 1, 2024 at 4:30 PM Edgar E. Iglesias 
wrote:

>
>
> On Mon, Jul 1, 2024 at 3:58 PM Edgar E. Iglesias 
> wrote:
>
>> On Mon, Jul 1, 2024 at 2:55 PM Anthony PERARD 
>> wrote:
>>
>>> Hi all,
>>>
>>> Following this commit, a test which install Debian in a guest with OVMF
>>> as firmware started to fail. QEMU exit with an error when GRUB is
>>> running on the freshly installed Debian (I don't know if GRUB is
>>> starting Linux or not).
>>> The error is:
>>> Bad ram offset 
>>>
>>> Some logs:
>>>
>>> http://logs.test-lab.xenproject.org/osstest/logs/186611/test-amd64-amd64-xl-qemuu-ovmf-amd64/info.html
>>>
>>> Any idea? Something is trying to do something with the address "-1" when
>>> it shouldn't?
>>>
>>>
>> Hi Anothny,
>>
>> Yes, it looks like something is calling qemu_get_ram_block() on something
>> that isn't mapped.
>> One possible path is in qemu_ram_block_from_host() but there may be
>> others.
>>
>> The following patch may help.
>> Any chance you could try to get a backtrace from QEMU when it failed?
>>
>> diff --git a/system/physmem.c b/system/physmem.c
>> index 33d09f7571..2669c4dbbb 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -2277,6 +2277,9 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool
>> round_offset,
>>  ram_addr_t ram_addr;
>>  RCU_READ_LOCK_GUARD();
>>  ram_addr = xen_ram_addr_from_mapcache(ptr);
>> +if (ram_addr == RAM_ADDR_INVALID) {
>> +return NULL;
>> +}
>>  block = qemu_get_ram_block(ram_addr);
>>  if (block) {
>>  *offset = ram_addr - block->offset;
>>
>>
>>
> One more thing, regarding this specific patch. I don't think we should
> clear the
> entire entry, the next field should be kept, otherwise we'll disconnect
> following
> mappings that will never be found again. IIUC, this could very well be
> causing the problem you see.
>
> Does the following make sense?
>
> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> index 5f23b0adbe..e9df53c19d 100644
> --- a/hw/xen/xen-mapcache.c
> +++ b/hw/xen/xen-mapcache.c
> @@ -597,7 +597,14 @@ static void
> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
>  pentry->next = entry->next;
>  g_free(entry);
>  } else {
> -memset(entry, 0, sizeof *entry);
> +/* Invalidate mapping.  */
> +entry->paddr_index = 0;
> +entry->vaddr_base = NULL;
> +entry->size = 0;
> +g_free(entry->valid_mapping);
> +entry->valid_mapping = NULL;
> +entry->flags = 0;
> +/* Keep entry->next pointing to the rest of the list.  */
>  }
>  }
>
>
>

And here without double-freeing entry->valid_mapping:

diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 5f23b0adbe..667807b3b6 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -597,7 +597,13 @@ static void
xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
 pentry->next = entry->next;
 g_free(entry);
 } else {
-memset(entry, 0, sizeof *entry);
+/* Invalidate mapping.  */
+entry->paddr_index = 0;
+entry->vaddr_base = NULL;
+entry->size = 0;
+entry->valid_mapping = NULL;
+entry->flags = 0;
+/* Keep entry->next pointing to the rest of the list.  */
 }
 }

Re: [PATCH v8 2/8] xen: mapcache: Unmap first entries in buckets

[Edgar E. Iglesias]

On Mon, Jul 1, 2024 at 3:58 PM Edgar E. Iglesias 
wrote:

> On Mon, Jul 1, 2024 at 2:55 PM Anthony PERARD 
> wrote:
>
>> Hi all,
>>
>> Following this commit, a test which install Debian in a guest with OVMF
>> as firmware started to fail. QEMU exit with an error when GRUB is
>> running on the freshly installed Debian (I don't know if GRUB is
>> starting Linux or not).
>> The error is:
>> Bad ram offset 
>>
>> Some logs:
>>
>> http://logs.test-lab.xenproject.org/osstest/logs/186611/test-amd64-amd64-xl-qemuu-ovmf-amd64/info.html
>>
>> Any idea? Something is trying to do something with the address "-1" when
>> it shouldn't?
>>
>>
> Hi Anothny,
>
> Yes, it looks like something is calling qemu_get_ram_block() on something
> that isn't mapped.
> One possible path is in qemu_ram_block_from_host() but there may be others.
>
> The following patch may help.
> Any chance you could try to get a backtrace from QEMU when it failed?
>
> diff --git a/system/physmem.c b/system/physmem.c
> index 33d09f7571..2669c4dbbb 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -2277,6 +2277,9 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool
> round_offset,
>  ram_addr_t ram_addr;
>  RCU_READ_LOCK_GUARD();
>  ram_addr = xen_ram_addr_from_mapcache(ptr);
> +if (ram_addr == RAM_ADDR_INVALID) {
> +return NULL;
> +}
>  block = qemu_get_ram_block(ram_addr);
>  if (block) {
>  *offset = ram_addr - block->offset;
>
>
>
One more thing, regarding this specific patch. I don't think we should
clear the
entire entry, the next field should be kept, otherwise we'll disconnect
following
mappings that will never be found again. IIUC, this could very well be
causing the problem you see.

Does the following make sense?

diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
index 5f23b0adbe..e9df53c19d 100644
--- a/hw/xen/xen-mapcache.c
+++ b/hw/xen/xen-mapcache.c
@@ -597,7 +597,14 @@ static void
xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
 pentry->next = entry->next;
 g_free(entry);
 } else {
-memset(entry, 0, sizeof *entry);
+/* Invalidate mapping.  */
+entry->paddr_index = 0;
+entry->vaddr_base = NULL;
+entry->size = 0;
+g_free(entry->valid_mapping);
+entry->valid_mapping = NULL;
+entry->flags = 0;
+/* Keep entry->next pointing to the rest of the list.  */
 }
 }









>
>
>> Cheers,
>>
>> Anthony
>>
>> On Wed, May 29, 2024 at 04:07:33PM +0200, Edgar E. Iglesias wrote:
>> > From: "Edgar E. Iglesias" 
>> >
>> > When invalidating memory ranges, if we happen to hit the first
>> > entry in a bucket we were never unmapping it. This was harmless
>> > for foreign mappings but now that we're looking to reuse the
>> > mapcache for transient grant mappings, we must unmap entries
>> > when invalidated.
>> >
>> > Signed-off-by: Edgar E. Iglesias 
>> > Reviewed-by: Stefano Stabellini 
>> > ---
>> >  hw/xen/xen-mapcache.c | 11 ---
>> >  1 file changed, 8 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
>> > index bc860f4373..ec95445696 100644
>> > --- a/hw/xen/xen-mapcache.c
>> > +++ b/hw/xen/xen-mapcache.c
>> > @@ -491,18 +491,23 @@ static void
>> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
>> >  return;
>> >  }
>> >  entry->lock--;
>> > -if (entry->lock > 0 || pentry == NULL) {
>> > +if (entry->lock > 0) {
>> >  return;
>> >  }
>> >
>> > -pentry->next = entry->next;
>> >  ram_block_notify_remove(entry->vaddr_base, entry->size,
>> entry->size);
>> >  if (munmap(entry->vaddr_base, entry->size) != 0) {
>> >  perror("unmap fails");
>> >  exit(-1);
>> >  }
>> > +
>> >  g_free(entry->valid_mapping);
>> > -g_free(entry);
>> > +if (pentry) {
>> > +pentry->next = entry->next;
>> > +g_free(entry);
>> > +} else {
>> > +memset(entry, 0, sizeof *entry);
>> > +}
>> >  }
>> >
>> >  typedef struct XenMapCacheData {
>> > --
>> > 2.40.1
>> >
>> >
>> --
>>
>> Anthony Perard | Vates XCP-ng Developer
>>
>> XCP-ng & Xen Orchestra - Vates solutions
>>
>> web: https://vates.tech
>>
>

Re: [PATCH v8 2/8] xen: mapcache: Unmap first entries in buckets

[Edgar E. Iglesias]

On Mon, Jul 1, 2024 at 2:55 PM Anthony PERARD 
wrote:

> Hi all,
>
> Following this commit, a test which install Debian in a guest with OVMF
> as firmware started to fail. QEMU exit with an error when GRUB is
> running on the freshly installed Debian (I don't know if GRUB is
> starting Linux or not).
> The error is:
> Bad ram offset 
>
> Some logs:
>
> http://logs.test-lab.xenproject.org/osstest/logs/186611/test-amd64-amd64-xl-qemuu-ovmf-amd64/info.html
>
> Any idea? Something is trying to do something with the address "-1" when
> it shouldn't?
>
>
Hi Anothny,

Yes, it looks like something is calling qemu_get_ram_block() on something
that isn't mapped.
One possible path is in qemu_ram_block_from_host() but there may be others.

The following patch may help.
Any chance you could try to get a backtrace from QEMU when it failed?

diff --git a/system/physmem.c b/system/physmem.c
index 33d09f7571..2669c4dbbb 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2277,6 +2277,9 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool
round_offset,
 ram_addr_t ram_addr;
 RCU_READ_LOCK_GUARD();
 ram_addr = xen_ram_addr_from_mapcache(ptr);
+if (ram_addr == RAM_ADDR_INVALID) {
+return NULL;
+}
 block = qemu_get_ram_block(ram_addr);
 if (block) {
 *offset = ram_addr - block->offset;





> Cheers,
>
> Anthony
>
> On Wed, May 29, 2024 at 04:07:33PM +0200, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" 
> >
> > When invalidating memory ranges, if we happen to hit the first
> > entry in a bucket we were never unmapping it. This was harmless
> > for foreign mappings but now that we're looking to reuse the
> > mapcache for transient grant mappings, we must unmap entries
> > when invalidated.
> >
> > Signed-off-by: Edgar E. Iglesias 
> > Reviewed-by: Stefano Stabellini 
> > ---
> >  hw/xen/xen-mapcache.c | 11 ---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> > index bc860f4373..ec95445696 100644
> > --- a/hw/xen/xen-mapcache.c
> > +++ b/hw/xen/xen-mapcache.c
> > @@ -491,18 +491,23 @@ static void
> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
> >  return;
> >  }
> >  entry->lock--;
> > -if (entry->lock > 0 || pentry == NULL) {
> > +if (entry->lock > 0) {
> >  return;
> >  }
> >
> > -pentry->next = entry->next;
> >  ram_block_notify_remove(entry->vaddr_base, entry->size,
> entry->size);
> >  if (munmap(entry->vaddr_base, entry->size) != 0) {
> >  perror("unmap fails");
> >  exit(-1);
> >  }
> > +
> >  g_free(entry->valid_mapping);
> > -g_free(entry);
> > +if (pentry) {
> > +pentry->next = entry->next;
> > +g_free(entry);
> > +} else {
> > +memset(entry, 0, sizeof *entry);
> > +}
> >  }
> >
> >  typedef struct XenMapCacheData {
> > --
> > 2.40.1
> >
> >
> --
>
> Anthony Perard | Vates XCP-ng Developer
>
> XCP-ng & Xen Orchestra - Vates solutions
>
> web: https://vates.tech
>

Re: [PATCH] hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu

[Edgar E. Iglesias]

On Thu, Jun 20, 2024 at 12:25:51PM +0200, Philippe Mathieu-Daudé wrote:
> On 20/6/24 12:10, Peter Maydell wrote:
> > On Tue, 18 Jun 2024 at 15:51, Philippe Mathieu-Daudé  
> > wrote:
> > > 
> > > On 18/6/24 16:40, Zheyu Ma wrote:
> > > > This commit updates the a9_gtimer_get_current_cpu() function to handle
> > > > cases where QTest is enabled. When QTest is used, it returns 0 instead
> > > > of dereferencing the current_cpu, which can be NULL. This prevents the
> > > > program from crashing during QTest runs.
> > > > 
> > > > Reproducer:
> > > > cat << EOF | qemu-system-aarch64 -display \
> > > > none -machine accel=qtest, -m 512M -machine npcm750-evb -qtest stdio
> > > > writel 0xf03fe20c 0x26d7468c
> > > > EOF
> > > > 
> > > > Signed-off-by: Zheyu Ma 
> > > > ---
> > > >hw/timer/a9gtimer.c | 5 +
> > > >1 file changed, 5 insertions(+)
> 
> 
> > > >if (current_cpu->cpu_index >= s->num_cpu) {
> > > 
> > > That said, such accesses of @current_cpu from hw/ are dubious.
> > 
> > True, but I'm not sure we ever settled on the right way to avoid
> > them, did we?
> 
> No we didn't, it is still in my TODO list; we might discuss it
> when I post my RFC.
>

Yeah, this way of getting the core id is a problem when having multiple
ARM CPU subsystems (and for heterogenous cores).

IIRC, when I looked at what the GIC v2 HW does, the GIC exposes an AMBA
port for each CPU. In my mental model that would translate to exposing
multiple Memory Reginos (sysbus_init_mmio) and mapping the appropriate
device MR to each CPU AddressSpace.

We could also do it with memory attributes but I don't think the
master Ids are standardised enough to extract a core-index from
with out having SoC specific code,, at least not accross Xilinx devices.

I never looked at newer GIC versions nor the mmio mapped timers
though...

Cheers,
Edgar

Re: [PATCH v2 2/3] hw/arm/xilinx_zynq: Add boot-mode property

[Edgar E. Iglesias]

On Wed, Jun 19, 2024 at 11:16 AM Sai Pavan Boddu 
wrote:

> Read boot-mode value as machine property and propagate that to
> SLCR.BOOT_MODE register.
>
>
Hi Sai,

It seems a little odd to have -machine boot-mode and -boot. Perhaps someone
else has a better idea how this could be done?

Anyway, I'm OK with your approach:
Acked-by: Edgar E. Iglesias 



> Signed-off-by: Sai Pavan Boddu 
> ---
>  hw/arm/xilinx_zynq.c | 31 +++
>  1 file changed, 31 insertions(+)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 7f7a3d23fbe..39f07e6dfd8 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -38,6 +38,7 @@
>  #include "qom/object.h"
>  #include "exec/tswap.h"
>  #include "target/arm/cpu-qom.h"
> +#include "qapi/visitor.h"
>
>  #define TYPE_ZYNQ_MACHINE MACHINE_TYPE_NAME("xilinx-zynq-a9")
>  OBJECT_DECLARE_SIMPLE_TYPE(ZynqMachineState, ZYNQ_MACHINE)
> @@ -90,6 +91,7 @@ struct ZynqMachineState {
>  MachineState parent;
>  Clock *ps_clk;
>  ARMCPU *cpu[ZYNQ_MAX_CPUS];
> +uint8_t boot_mode;
>  };
>
>  static void zynq_write_board_setup(ARMCPU *cpu,
> @@ -176,6 +178,27 @@ static inline int zynq_init_spi_flashes(uint32_t
> base_addr, qemu_irq irq,
>  return unit;
>  }
>
> +static void zynq_set_boot_mode(Object *obj, const char *str,
> +   Error **errp)
> +{
> +ZynqMachineState *m = ZYNQ_MACHINE(obj);
> +uint8_t mode = 0;
> +
> +if (!strcasecmp(str, "QSPI")) {
> +mode = 1;
> +} else if (!strcasecmp(str, "SD")) {
> +mode = 5;
> +} else if (!strcasecmp(str, "NOR")) {
> +mode = 2;
> +} else if (!strcasecmp(str, "JTAG")) {
> +mode = 0;
> +} else {
> +error_setg(errp, "%s bootmode is not supported", str);
> +return;
> +}
> +m->boot_mode = mode;
> +}
> +
>  static void zynq_init(MachineState *machine)
>  {
>  ZynqMachineState *zynq_machine = ZYNQ_MACHINE(machine);
> @@ -241,6 +264,7 @@ static void zynq_init(MachineState *machine)
>  /* Create slcr, keep a pointer to connect clocks */
>  slcr = qdev_new("xilinx-zynq_slcr");
>  qdev_connect_clock_in(slcr, "ps_clk", zynq_machine->ps_clk);
> +qdev_prop_set_uint8(slcr, "boot-mode", zynq_machine->boot_mode);
>  sysbus_realize_and_unref(SYS_BUS_DEVICE(slcr), &error_fatal);
>  sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF800);
>
> @@ -372,6 +396,7 @@ static void zynq_machine_class_init(ObjectClass *oc,
> void *data)
>  NULL
>  };
>  MachineClass *mc = MACHINE_CLASS(oc);
> +ObjectProperty *prop;
>  mc->desc = "Xilinx Zynq Platform Baseboard for Cortex-A9";
>  mc->init = zynq_init;
>  mc->max_cpus = ZYNQ_MAX_CPUS;
> @@ -379,6 +404,12 @@ static void zynq_machine_class_init(ObjectClass *oc,
> void *data)
>  mc->ignore_memory_transaction_failures = true;
>  mc->valid_cpu_types = valid_cpu_types;
>  mc->default_ram_id = "zynq.ext_ram";
> +prop = object_class_property_add_str(oc, "boot-mode", NULL,
> +  zynq_set_boot_mode);
> +object_class_property_set_description(oc, "boot-mode",
> +  "Supported boot modes:"
> +  " JTAG QSPI SD NOR");
> +object_property_set_default_str(prop, "QSPI");
>  }
>
>  static const TypeInfo zynq_machine_type = {
> --
> 2.34.1
>
>

Re: [PATCH v2 1/3] hw/misc/zynq_slcr: Add BootMode property

[Edgar E. Iglesias]

On Wed, Jun 19, 2024 at 11:16 AM Sai Pavan Boddu 
wrote:

> BootMode property sets user values into BOOT_MODE register, on hardware
> these are derived from board switches.
>
>

Reviewed-by: Edgar E. Iglesias 



> Signed-off-by: Sai Pavan Boddu 
> ---
>  hw/misc/zynq_slcr.c | 22 +-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index 3412ff099ea..ad814c3a79b 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -24,6 +24,8 @@
>  #include "hw/registerfields.h"
>  #include "hw/qdev-clock.h"
>  #include "qom/object.h"
> +#include "hw/qdev-properties.h"
> +#include "qapi/error.h"
>
>  #ifndef ZYNQ_SLCR_ERR_DEBUG
>  #define ZYNQ_SLCR_ERR_DEBUG 0
> @@ -121,6 +123,7 @@ REG32(RST_REASON, 0x250)
>
>  REG32(REBOOT_STATUS, 0x258)
>  REG32(BOOT_MODE, 0x25c)
> +FIELD(BOOT_MODE, BOOT_MODE, 0, 4)
>
>  REG32(APU_CTRL, 0x300)
>  REG32(WDT_CLK_SEL, 0x304)
> @@ -195,6 +198,7 @@ struct ZynqSLCRState {
>  Clock *ps_clk;
>  Clock *uart0_ref_clk;
>  Clock *uart1_ref_clk;
> +uint8_t boot_mode;
>  };
>
>  /*
> @@ -371,7 +375,7 @@ static void zynq_slcr_reset_init(Object *obj,
> ResetType type)
>  s->regs[R_FPGA_RST_CTRL]  = 0x01F33F0F;
>  s->regs[R_RST_REASON] = 0x0040;
>
> -s->regs[R_BOOT_MODE]  = 0x0001;
> +s->regs[R_BOOT_MODE]  = s->boot_mode & R_BOOT_MODE_BOOT_MODE_MASK;
>
>  /* 0x700 - 0x7D4 */
>  for (i = 0; i < 54; i++) {
> @@ -588,6 +592,15 @@ static const ClockPortInitArray zynq_slcr_clocks = {
>  QDEV_CLOCK_END
>  };
>
> +static void zynq_slcr_realize(DeviceState *dev, Error **errp)
> +{
> +ZynqSLCRState *s = ZYNQ_SLCR(dev);
> +
> +if (s->boot_mode > 0xF) {
> +error_setg(errp, "Invalid boot mode %d specified", s->boot_mode);
> +}
> +}
> +
>  static void zynq_slcr_init(Object *obj)
>  {
>  ZynqSLCRState *s = ZYNQ_SLCR(obj);
> @@ -610,15 +623,22 @@ static const VMStateDescription vmstate_zynq_slcr = {
>  }
>  };
>
> +static Property zynq_slcr_props[] = {
> +DEFINE_PROP_UINT8("boot-mode", ZynqSLCRState, boot_mode, 1),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void zynq_slcr_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  ResettableClass *rc = RESETTABLE_CLASS(klass);
>
>  dc->vmsd = &vmstate_zynq_slcr;
> +dc->realize = zynq_slcr_realize;
>  rc->phases.enter = zynq_slcr_reset_init;
>  rc->phases.hold  = zynq_slcr_reset_hold;
>  rc->phases.exit  = zynq_slcr_reset_exit;
> +device_class_set_props(dc, zynq_slcr_props);
>  }
>
>  static const TypeInfo zynq_slcr_info = {
> --
> 2.34.1
>
>

Re: [PATCH v2 3/3] docs/system/arm: Add a doc for zynq board

[Edgar E. Iglesias]

On Wed, Jun 19, 2024 at 11:16 AM Sai Pavan Boddu 
wrote:

> Added the supported device list and an example command.
>
>
Thanks Sai!

You need to add an entry in the Xilinx Zynq section of the MAINTAINERS
file, e.g:
F: docs/system/arm/xlnx-zynq.rst

I would also list the supported boot-mode values in lower-case or the
boot-mode example in upper-case.
I know case doesn't matter but it would be nice to have the example
consistent with the list of supported values.

With those changes feel free to add:
Reviewed-by: Edgar E. Iglesias 

Cheers,
Edgar



> Signed-off-by: Sai Pavan Boddu 
> ---
>  docs/system/arm/xlnx-zynq.rst | 47 +++
>  docs/system/target-arm.rst|  1 +
>  2 files changed, 48 insertions(+)
>  create mode 100644 docs/system/arm/xlnx-zynq.rst
>
> diff --git a/docs/system/arm/xlnx-zynq.rst b/docs/system/arm/xlnx-zynq.rst
> new file mode 100644
> index 000..419cc1aec8b
> --- /dev/null
> +++ b/docs/system/arm/xlnx-zynq.rst
> @@ -0,0 +1,47 @@
> +Xilinx Zynq board (``xilinx-zynq-a9``)
> +==
> +The Zynq 7000 family is based on the AMD SoC architecture. These products
> +integrate a feature-rich dual or single-core Arm Cortex-A9 MPCore based
> +processing system (PS) and AMD programmable logic (PL) in a single device.
> +
> +More details here:
> +
> https://docs.amd.com/r/en-US/ug585-zynq-7000-SoC-TRM/Zynq-7000-SoC-Technical-Reference-Manual
> +
> +QEMU xilinx-zynq-a9 board supports following devices:
> +- A9 MPCORE
> +- cortex-a9
> +- GIC v1
> +- Generic timer
> +- wdt
> +- OCM 256KB
> +- SMC SRAM@0xe200 64MB
> +- Zynq SLCR
> +- SPI x2
> +- QSPI
> +- UART
> +- TTC x2
> +- Gigabit Ethernet Controller x2
> +- SD Controller x2
> +- XADC
> +- Arm PrimeCell DMA Controller
> +- DDR Memory
> +- USB 2.0 x2
> +
> +Running
> +"""""""
> +Direct Linux boot of a generic ARM upstream Linux kernel:
> +
> +.. code-block:: bash
> +
> +  $ qemu-system-aarch64 -M xilinx-zynq-a9 \
> +-dtb zynq-zc702.dtb  -serial null -serial mon:stdio \
> +-display none  -m 1024 \
> +-initrd rootfs.cpio.gz -kernel zImage
> +
> +For configuring the boot-mode provide the following on the command line:
> +
> +.. code-block:: bash
> +
> +   -machine boot-mode=qspi
> +
> +Supported values are JTAG, SD, QSPI, NOR.
> diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst
> index 870d30e3502..7b992722846 100644
> --- a/docs/system/target-arm.rst
> +++ b/docs/system/target-arm.rst
> @@ -109,6 +109,7 @@ undocumented; you can get a complete list by running
> arm/virt
> arm/xenpvh
> arm/xlnx-versal-virt
> +   arm/xlnx-zynq
>
>  Emulated CPU architecture support
>  =
> --
> 2.34.1
>
>

[PULL v1 1/3] hw/dma: Enhance error handling in loading description

[Edgar E. Iglesias]

From: "Fea.Wang" 

Loading a description from memory may cause a bus-error. In this
case, the DMA should stop working, set the error flag, and return
the failure value.

When calling the loading a description function, it should be noticed
that the function may return a failure value. Breaking the loop in this
case is one of the possible ways to handle it.

Signed-off-by: Fea.Wang 
Reviewed-by: Frank Chang 
Reviewed-by: Edgar E. Iglesias 
Signed-off-by: Edgar E. Iglesias 
---
 hw/dma/xilinx_axidma.c | 30 ++
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 0ae056ed06..ad307994c2 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -71,8 +71,11 @@ enum {
 enum {
 DMASR_HALTED = 1,
 DMASR_IDLE  = 2,
+DMASR_SLVERR = 1 << 5,
+DMASR_DECERR = 1 << 6,
 DMASR_IOC_IRQ  = 1 << 12,
 DMASR_DLY_IRQ  = 1 << 13,
+DMASR_ERR_IRQ  = 1 << 14,
 
 DMASR_IRQ_MASK = 7 << 12
 };
@@ -190,17 +193,32 @@ static inline int streamid_from_addr(hwaddr addr)
 return sid;
 }
 
-static void stream_desc_load(struct Stream *s, hwaddr addr)
+static MemTxResult stream_desc_load(struct Stream *s, hwaddr addr)
 {
 struct SDesc *d = &s->desc;
 
-address_space_read(&s->dma->as, addr, MEMTXATTRS_UNSPECIFIED, d, sizeof 
*d);
+MemTxResult result = address_space_read(&s->dma->as,
+addr, MEMTXATTRS_UNSPECIFIED,
+d, sizeof *d);
+if (result != MEMTX_OK) {
+if (result == MEMTX_DECODE_ERROR) {
+s->regs[R_DMASR] |= DMASR_DECERR;
+} else {
+s->regs[R_DMASR] |= DMASR_SLVERR;
+}
+
+s->regs[R_DMACR] &= ~DMACR_RUNSTOP;
+s->regs[R_DMASR] |= DMASR_HALTED;
+s->regs[R_DMASR] |= DMASR_ERR_IRQ;
+return result;
+}
 
 /* Convert from LE into host endianness.  */
 d->buffer_address = le64_to_cpu(d->buffer_address);
 d->nxtdesc = le64_to_cpu(d->nxtdesc);
 d->control = le32_to_cpu(d->control);
 d->status = le32_to_cpu(d->status);
+return result;
 }
 
 static void stream_desc_store(struct Stream *s, hwaddr addr)
@@ -279,7 +297,9 @@ static void stream_process_mem2s(struct Stream *s, 
StreamSink *tx_data_dev,
 }
 
 while (1) {
-stream_desc_load(s, s->regs[R_CURDESC]);
+if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) {
+break;
+}
 
 if (s->desc.status & SDESC_STATUS_COMPLETE) {
 s->regs[R_DMASR] |= DMASR_HALTED;
@@ -336,7 +356,9 @@ static size_t stream_process_s2mem(struct Stream *s, 
unsigned char *buf,
 }
 
 while (len) {
-stream_desc_load(s, s->regs[R_CURDESC]);
+if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) {
+break;
+}
 
 if (s->desc.status & SDESC_STATUS_COMPLETE) {
 s->regs[R_DMASR] |= DMASR_HALTED;
-- 
2.43.0

[PULL v1 2/3] hw/dma: Add a trace log for a description loading failure

[Edgar E. Iglesias]

From: "Fea.Wang" 

Due to a description loading failure, adding a trace log makes observing
the DMA behavior easy.

Signed-off-by: Fea.Wang 
Reviewed-by: Edgar E. Iglesias 
Reviewed-by: Frank Chang 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Edgar E. Iglesias 
---
 hw/dma/trace-events| 3 +++
 hw/dma/xilinx_axidma.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/hw/dma/trace-events b/hw/dma/trace-events
index 3c47df54e4..4c09790f9a 100644
--- a/hw/dma/trace-events
+++ b/hw/dma/trace-events
@@ -44,3 +44,6 @@ pl330_debug_exec_stall(void) "stall of debug instruction not 
implemented"
 pl330_iomem_write(uint32_t offset, uint32_t value) "addr: 0x%08"PRIx32" data: 
0x%08"PRIx32
 pl330_iomem_write_clr(int i) "event interrupt lowered %d"
 pl330_iomem_read(uint32_t addr, uint32_t data) "addr: 0x%08"PRIx32" data: 
0x%08"PRIx32
+
+# xilinx_axidma.c
+xilinx_axidma_loading_desc_fail(uint32_t res) "error:%u"
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index ad307994c2..c9cfc3169b 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -36,6 +36,7 @@
 #include "sysemu/dma.h"
 #include "hw/stream.h"
 #include "qom/object.h"
+#include "trace.h"
 
 #define D(x)
 
@@ -201,6 +202,8 @@ static MemTxResult stream_desc_load(struct Stream *s, 
hwaddr addr)
 addr, MEMTXATTRS_UNSPECIFIED,
 d, sizeof *d);
 if (result != MEMTX_OK) {
+trace_xilinx_axidma_loading_desc_fail(result);
+
 if (result == MEMTX_DECODE_ERROR) {
 s->regs[R_DMASR] |= DMASR_DECERR;
 } else {
-- 
2.43.0

[PULL v1 0/3] Xilinx DMA/Ethernet updates

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

The following changes since commit 900536d3e97aed7fdd9cb4dadd3bf7023360e819:

  Merge tag 'dirtylimit-dirtyrate-pull-request-20240617' of 
https://github.com/newfriday/qemu into staging (2024-06-17 11:40:24 -0700)

are available in the Git repository at:

  https://gitlab.com/edgar.iglesias/qemu.git 
tags/edgar/xilinx-queue-2024-06-17.for-upstream

for you to fetch changes up to 3a6d374b754b4b345195ff6846eeaffedc96a7c5:

  hw/net: Fix the transmission return size (2024-06-18 14:52:05 +0200)


Xilinx queue:

hw/dma: Add error handling for loading descriptions failing (Fea Wang)


Fea.Wang (3):
  hw/dma: Enhance error handling in loading description
  hw/dma: Add a trace log for a description loading failure
  hw/net: Fix the transmission return size

 hw/dma/trace-events |  3 +++
 hw/dma/xilinx_axidma.c  | 33 +
 hw/net/xilinx_axienet.c |  2 +-
 3 files changed, 33 insertions(+), 5 deletions(-)

-- 
2.43.0

[PULL v1 3/3] hw/net: Fix the transmission return size

[Edgar E. Iglesias]

From: "Fea.Wang" 

Fix the transmission return size because not all bytes could be
transmitted successfully. So, return a successful length instead of a
constant value.

Signed-off-by: Fea.Wang 
Reviewed-by: Edgar E. Iglesias 
Reviewed-by: Frank Chang 
Signed-off-by: Edgar E. Iglesias 
---
 hw/net/xilinx_axienet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 7d1fd37b4a..05d41bd548 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -847,7 +847,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t 
*buf, size_t size)
 axienet_eth_rx_notify(s);
 
 enet_update_irq(s);
-return size;
+return s->rxpos;
 }
 
 static size_t
-- 
2.43.0

Re: [PATCH 2/2] hw/arm/xilinx_zynq: Add boot-mode property

[Edgar E. Iglesias]

On Thu, Jun 13, 2024 at 5:36 PM Sai Pavan Boddu 
wrote:

> Read boot-mode value as machine property and propagate that to
> SLCR.BOOT_MODE register.
>
>
Hi Sai,

Directly exposing the register field to the user to set on the command-line
probably makes usability a little too rough (user has to check the register
specs in the TRM to change boot-mode).
We could perhaps add friendly names that we internally map to the register
field values.

Another question, can we use the existing -boot command-line arg for this?
Something along the lines of what x86 PC does:
https://github.com/qemu/qemu/blob/master/hw/i386/pc.c#L395

I don't know if the framework allows for long names but something like the
following would be nice:
qemu -boot spi,ethernet,jtag,uart,etc

Would also be great to document a small example, perhaps in
https://github.com/qemu/qemu/tree/master/docs/system/arm

Best regards,
Edgar



> Signed-off-by: Sai Pavan Boddu 
> ---
>  hw/arm/xilinx_zynq.c | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 7f7a3d23fb..4dfa9184ac 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -38,6 +38,7 @@
>  #include "qom/object.h"
>  #include "exec/tswap.h"
>  #include "target/arm/cpu-qom.h"
> +#include "qapi/visitor.h"
>
>  #define TYPE_ZYNQ_MACHINE MACHINE_TYPE_NAME("xilinx-zynq-a9")
>  OBJECT_DECLARE_SIMPLE_TYPE(ZynqMachineState, ZYNQ_MACHINE)
> @@ -90,6 +91,7 @@ struct ZynqMachineState {
>  MachineState parent;
>  Clock *ps_clk;
>  ARMCPU *cpu[ZYNQ_MAX_CPUS];
> +uint8_t BootMode;
>  };
>
>  static void zynq_write_board_setup(ARMCPU *cpu,
> @@ -176,6 +178,19 @@ static inline int zynq_init_spi_flashes(uint32_t
> base_addr, qemu_irq irq,
>  return unit;
>  }
>
> +static void zynq_set_boot_mode(Object *obj, Visitor *v,
> +   const char *name, void *opaque,
> +   Error **errp)
> +{
> +ZynqMachineState *m = ZYNQ_MACHINE(obj);
> +uint8_t val;
> +
> +if (!visit_type_uint8(v, name, &val, errp)) {
> +return;
> +}
> +m->BootMode = val;
> +}
> +
>  static void zynq_init(MachineState *machine)
>  {
>  ZynqMachineState *zynq_machine = ZYNQ_MACHINE(machine);
> @@ -241,6 +256,7 @@ static void zynq_init(MachineState *machine)
>  /* Create slcr, keep a pointer to connect clocks */
>  slcr = qdev_new("xilinx-zynq_slcr");
>  qdev_connect_clock_in(slcr, "ps_clk", zynq_machine->ps_clk);
> +qdev_prop_set_uint8(slcr, "boot-mode", zynq_machine->BootMode);
>  sysbus_realize_and_unref(SYS_BUS_DEVICE(slcr), &error_fatal);
>  sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF800);
>
> @@ -372,6 +388,7 @@ static void zynq_machine_class_init(ObjectClass *oc,
> void *data)
>  NULL
>  };
>  MachineClass *mc = MACHINE_CLASS(oc);
> +ObjectProperty *prop;
>  mc->desc = "Xilinx Zynq Platform Baseboard for Cortex-A9";
>  mc->init = zynq_init;
>  mc->max_cpus = ZYNQ_MAX_CPUS;
> @@ -379,6 +396,11 @@ static void zynq_machine_class_init(ObjectClass *oc,
> void *data)
>  mc->ignore_memory_transaction_failures = true;
>  mc->valid_cpu_types = valid_cpu_types;
>  mc->default_ram_id = "zynq.ext_ram";
> +prop = object_class_property_add(oc, "boot-mode", "uint8_t", NULL,
> +  zynq_set_boot_mode, NULL, NULL);
> +object_class_property_set_description(oc, "boot-mode",
> +  "Update SLCR.BOOT_MODE
> register");
> +object_property_set_default_uint(prop, 1);
>  }
>
>  static const TypeInfo zynq_machine_type = {
> --
> 2.34.1
>
>

Re: [PATCH] hw/net: cadence_gem: fix: type2_compare_x_word_0 error

[Edgar E. Iglesias]

On Thu, Jun 6, 2024 at 2:06 PM Peter Maydell 
wrote:

> On Thu, 6 Jun 2024 at 12:04, Edgar E. Iglesias 
> wrote:
> >
> > On Thu, Jun 6, 2024 at 12:00 PM Andrew.Yuan 
> wrote:
> >>
> >> In the Cadence IP for Gigabit Ethernet MAC Part Number: IP7014
> IP Rev: R1p12 - Doc Rev: 1.3 User Guide, the specification for the
> type2_compare_x_word_0 register is as follows:
> >> The byte stored in bits [23:16] is compared against the byte in
> the received frame from the selected offset+0, and the byte stored in bits
> [31:24] is compared against the byte in
> >> the received frame from the selected offset+1.
> >>
> >> However, there is an implementation error in the cadence_gem
> model in qemu：
> >> the byte stored in bits [31:24] is compared against the byte in
> the received frame from the selected offset+0
> >>
> >> Now, the error code is as follows:
> >> rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
> >>
> >> and needs to be corrected to：
> >> rx_cmp = rxbuf_ptr[offset + 1] << 8 | rxbuf_ptr[offset];
> >>
> >> Signed-off-by: Andrew.Yuan 
> >
> >
> >
> > LGTM:
> > Reviewed-by: Edgar E. Iglesias 
> >
> > At some point it would be nice to add the missing logic for the
> DISABLE_MASK bit that
> > extends the compare range from 16 to 32-bits.
>
> I had a look at this device's code, and I'm trying to
> figure out how we know at this point that there really are
> two bytes pointed to by rxbuf_ptr.
>  * The get_queue_from_screen() function takes a rxbufsize
>argument, but it never uses it...
>  * the callsite in gem_receive() will (in the "strip FCS" case)
>pass its buf argument as rxbuf_ptr, but it will use a
>rxbufsize argument which has been raised to at least
>GEM_DMACFG_RBUFSZ_MUL, even if the input size argument
>is smaller, so even if get_queue_from_screen() honoured
>its rxbufsize argument it wouldn't help
>
> Would somebody who understands the device like to have a look ?
>
>
Yes, I agree that it looks strange. The padding to minimum 60B seems wrong
since we're blindly extending buf from something less than 60B to 60B
and then potentially copying from it...

Cheers,
Edgar


> This is a separate issue from the incorrect array offset
> argument this patch fixes, though.
>
> thanks
> -- PMM
>

Re: [PATCH v2 1/3] hw/dma: Enhance error handling in loading description

[Edgar E. Iglesias]

On Tue, Jun 4, 2024 at 9:10 AM Fea.Wang  wrote:

> Loading a description from memory may cause a bus-error. In this
> case, the DMA should stop working, set the error flag, and return
> the failure value.
>
> When calling the loading a description function, it should be noticed
> that the function may return a failure value. Breaking the loop in this
> case is one of the possible ways to handle it.
>
> Signed-off-by: Fea.Wang 
> Reviewed-by: Frank Chang 
>

Thanks!
Reviewed-by: Edgar E. Iglesias 



---
>  hw/dma/xilinx_axidma.c | 30 ++
>  1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index 0ae056ed06..ad307994c2 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -71,8 +71,11 @@ enum {
>  enum {
>  DMASR_HALTED = 1,
>  DMASR_IDLE  = 2,
> +DMASR_SLVERR = 1 << 5,
> +DMASR_DECERR = 1 << 6,
>  DMASR_IOC_IRQ  = 1 << 12,
>  DMASR_DLY_IRQ  = 1 << 13,
> +DMASR_ERR_IRQ  = 1 << 14,
>
>  DMASR_IRQ_MASK = 7 << 12
>  };
> @@ -190,17 +193,32 @@ static inline int streamid_from_addr(hwaddr addr)
>  return sid;
>  }
>
> -static void stream_desc_load(struct Stream *s, hwaddr addr)
> +static MemTxResult stream_desc_load(struct Stream *s, hwaddr addr)
>  {
>  struct SDesc *d = &s->desc;
>
> -address_space_read(&s->dma->as, addr, MEMTXATTRS_UNSPECIFIED, d,
> sizeof *d);
> +MemTxResult result = address_space_read(&s->dma->as,
> +addr, MEMTXATTRS_UNSPECIFIED,
> +d, sizeof *d);
> +if (result != MEMTX_OK) {
> +if (result == MEMTX_DECODE_ERROR) {
> +s->regs[R_DMASR] |= DMASR_DECERR;
> +} else {
> +s->regs[R_DMASR] |= DMASR_SLVERR;
> +}
> +
> +s->regs[R_DMACR] &= ~DMACR_RUNSTOP;
> +s->regs[R_DMASR] |= DMASR_HALTED;
> +s->regs[R_DMASR] |= DMASR_ERR_IRQ;
> +return result;
> +}
>
>  /* Convert from LE into host endianness.  */
>  d->buffer_address = le64_to_cpu(d->buffer_address);
>  d->nxtdesc = le64_to_cpu(d->nxtdesc);
>  d->control = le32_to_cpu(d->control);
>  d->status = le32_to_cpu(d->status);
> +return result;
>  }
>
>  static void stream_desc_store(struct Stream *s, hwaddr addr)
> @@ -279,7 +297,9 @@ static void stream_process_mem2s(struct Stream *s,
> StreamSink *tx_data_dev,
>  }
>
>  while (1) {
> -stream_desc_load(s, s->regs[R_CURDESC]);
> +if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) {
> +break;
> +}
>
>  if (s->desc.status & SDESC_STATUS_COMPLETE) {
>  s->regs[R_DMASR] |= DMASR_HALTED;
> @@ -336,7 +356,9 @@ static size_t stream_process_s2mem(struct Stream *s,
> unsigned char *buf,
>  }
>
>  while (len) {
> -stream_desc_load(s, s->regs[R_CURDESC]);
> +if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) {
> +break;
> +}
>
>  if (s->desc.status & SDESC_STATUS_COMPLETE) {
>  s->regs[R_DMASR] |= DMASR_HALTED;
> --
> 2.34.1
>
>

Re: [PATCH] hw/net: cadence_gem: fix: type2_compare_x_word_0 error

[Edgar E. Iglesias]

On Thu, Jun 6, 2024 at 12:00 PM Andrew.Yuan 
wrote:

> In the Cadence IP for Gigabit Ethernet MAC Part Number: IP7014 IP
> Rev: R1p12 - Doc Rev: 1.3 User Guide, the specification for the
> type2_compare_x_word_0 register is as follows:
> The byte stored in bits [23:16] is compared against the byte in
> the received frame from the selected offset+0, and the byte stored in bits
> [31:24] is compared against the byte in
> the received frame from the selected offset+1.
>
> However, there is an implementation error in the cadence_gem model
> in qemu：
> the byte stored in bits [31:24] is compared against the byte in
> the received frame from the selected offset+0
>
> Now, the error code is as follows:
> rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
>
> and needs to be corrected to：
> rx_cmp = rxbuf_ptr[offset + 1] << 8 | rxbuf_ptr[offset];
>
> Signed-off-by: Andrew.Yuan 
>


LGTM:
Reviewed-by: Edgar E. Iglesias 

At some point it would be nice to add the missing logic for the
DISABLE_MASK bit that
extends the compare range from 16 to 32-bits.

Cheers,
Edgar



> ---
>  hw/net/cadence_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index ec7bf562e5..9c73ded0d3 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -946,7 +946,7 @@ static int get_queue_from_screen(CadenceGEMState *s,
> uint8_t *rxbuf_ptr,
>  break;
>  }
>
> -rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
> +rx_cmp = rxbuf_ptr[offset + 1] << 8 | rxbuf_ptr[offset];
>  mask = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, MASK_VALUE);
>  compare = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0,
> COMPARE_VALUE);
>
> --
> 2.37.0.windows.1
>
>

[PATCH v1 0/1] hw/intc/arm_gic: Fix deactivation of SPI lines

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Julien reported that he has seen strange behaviour when running
Xen on QEMU using GICv2. When Xen migrates a guest's vCPU to
another pCPU while the vCPU is handling an interrupt the guest
is unable to properly deactivate interrupts.

It sounds like something rare but in some setups it actually
happens all the time.

Looking at it a little closer, our GICv2 model treats
deactivation of SPI lines as if they were PPI's, i.e banked per
CPU core. The state for active interrupts should only be banked
for PPI lines, not for SPI lines.

When deactivating SPI lines, I think we need to handle the state
as unbanked, similar to how we handle writes to GICD_ICACTIVER.

This fixes the problem on my side.

Cheers,
Edgar


Edgar E. Iglesias (1):
  hw/intc/arm_gic: Fix deactivation of SPI lines

 hw/intc/gic_internal.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)


base-commit: d16cab541ab9217977e2a39abf3d79f914146741
-- 
2.40.1

[PATCH v1 1/1] hw/intc/arm_gic: Fix deactivation of SPI lines

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Julien reported that he has seen strange behaviour when running
Xen on QEMU using GICv2. When Xen migrates a guest's vCPU from
one pCPU to another while the vCPU is handling an interrupt, the
guest is unable to properly deactivate interrupts.

Looking at it a little closer, our GICv2 model treats
deactivation of SPI lines as if they were PPI's, i.e banked per
CPU core. The state for active interrupts should only be banked
for PPI lines, not for SPI lines.

Make deactivation of SPI lines unbanked, similar to how we
handle writes to GICD_ICACTIVER.

Reported-by: Julien Grall 
Signed-off-by: Edgar E. Iglesias 
---
 hw/intc/gic_internal.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 8d29b40ca1..8ddbf554c6 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -280,6 +280,8 @@ static inline void gic_set_active(GICState *s, int irq, int 
cpu)
 
 static inline void gic_clear_active(GICState *s, int irq, int cpu)
 {
+unsigned int cm;
+
 if (gic_is_vcpu(cpu)) {
 uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
 GICH_LR_CLEAR_ACTIVE(*entry);
@@ -301,11 +303,13 @@ static inline void gic_clear_active(GICState *s, int irq, 
int cpu)
  * the GIC is secure.
  */
 if (!s->security_extn || GIC_DIST_TEST_GROUP(phys_irq, 1 << rcpu)) 
{
-GIC_DIST_CLEAR_ACTIVE(phys_irq, 1 << rcpu);
+cm = phys_irq < GIC_INTERNAL ? 1 << rcpu : ALL_CPU_MASK;
+GIC_DIST_CLEAR_ACTIVE(phys_irq, cm);
 }
 }
 } else {
-GIC_DIST_CLEAR_ACTIVE(irq, 1 << cpu);
+cm = irq < GIC_INTERNAL ? 1 << cpu : ALL_CPU_MASK;
+GIC_DIST_CLEAR_ACTIVE(irq, cm);
 }
 }
 
-- 
2.40.1

Re: [PATCH 4/4] hw/net: Fix the transmission return size

[Edgar E. Iglesias]

On Mon, Jun 3, 2024 at 7:48 AM Fea.Wang  wrote:

> Fix the transmission return size because not all bytes could be
> transmitted successfully. So, return a successful length instead of a
> constant value.
>
>
How did you test this patch, on Linux or something else? I have some
memory that we had some trouble with similar patches before.

Anyway, the change looks good to me:
Reviewed-by: Edgar E. Iglesias 



> Signed-off-by: Fea.Wang 
> ---
>  hw/net/xilinx_axienet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 7d1fd37b4a..05d41bd548 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -847,7 +847,7 @@ static ssize_t eth_rx(NetClientState *nc, const
> uint8_t *buf, size_t size)
>  axienet_eth_rx_notify(s);
>
>  enet_update_irq(s);
> -return size;
> +return s->rxpos;
>  }
>
>  static size_t
> --
> 2.34.1
>
>

Re: [PATCH 3/4] hw/dma: Add a trace log for a description loading failure

[Edgar E. Iglesias]

On Mon, Jun 3, 2024 at 7:48 AM Fea.Wang  wrote:

> Due to a description loading failure, adding a trace log makes observing
> the DMA behavior easy.
>
>
Reviewed-by: Edgar E. Iglesias 



> Signed-off-by: Fea.Wang 
> ---
>  hw/dma/trace-events| 3 +++
>  hw/dma/xilinx_axidma.c | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/hw/dma/trace-events b/hw/dma/trace-events
> index 3c47df54e4..95db083be4 100644
> --- a/hw/dma/trace-events
> +++ b/hw/dma/trace-events
> @@ -44,3 +44,6 @@ pl330_debug_exec_stall(void) "stall of debug instruction
> not implemented"
>  pl330_iomem_write(uint32_t offset, uint32_t value) "addr: 0x%08"PRIx32"
> data: 0x%08"PRIx32
>  pl330_iomem_write_clr(int i) "event interrupt lowered %d"
>  pl330_iomem_read(uint32_t addr, uint32_t data) "addr: 0x%08"PRIx32" data:
> 0x%08"PRIx32
> +
> +# xilinx_axidma.c
> +xilinx_axidma_loading_desc_fail(uint32_t res) "error:%d"
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index b8ab5a423d..1bbb9d6c4c 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -36,6 +36,7 @@
>  #include "sysemu/dma.h"
>  #include "hw/stream.h"
>  #include "qom/object.h"
> +#include "trace.h"
>
>  #define D(x)
>
> @@ -200,6 +201,8 @@ static MemTxResult stream_desc_load(struct Stream *s,
> hwaddr addr)
>  addr, MEMTXATTRS_UNSPECIFIED,
>  d, sizeof *d);
>  if (result != MEMTX_OK) {
> +trace_xilinx_axidma_loading_desc_fail(result);
> +
>  s->regs[R_DMACR] &= ~DMACR_RUNSTOP;
>  s->regs[R_DMASR] |= DMASR_HALTED;
>  s->regs[R_DMASR] |= DMASR_SLVERR;
> --
> 2.34.1
>
>

Re: [PATCH 2/4] hw/dma: Break the loop when loading descriptions fails

[Edgar E. Iglesias]

On Mon, Jun 3, 2024 at 7:48 AM Fea.Wang  wrote:

> When calling the loading a description function, it should be noticed
> that the function may return a failure value. Breaking the loop is one
> of the possible ways to handle it.
>
> Signed-off-by: Fea.Wang 
>


Looks good, a nitpick comment, I would either squash this with patch #1
or move the change to return of error code in stream_desc_load() to this
patch.




> ---
>  hw/dma/xilinx_axidma.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index 4b475e5484..b8ab5a423d 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -291,7 +291,9 @@ static void stream_process_mem2s(struct Stream *s,
> StreamSink *tx_data_dev,
>  }
>
>  while (1) {
> -stream_desc_load(s, s->regs[R_CURDESC]);
> +if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) {
> +break;
> +}
>
>  if (s->desc.status & SDESC_STATUS_COMPLETE) {
>  s->regs[R_DMASR] |= DMASR_HALTED;
> @@ -348,7 +350,9 @@ static size_t stream_process_s2mem(struct Stream *s,
> unsigned char *buf,
>  }
>
>  while (len) {
> -stream_desc_load(s, s->regs[R_CURDESC]);
> +if (MEMTX_OK != stream_desc_load(s, s->regs[R_CURDESC])) {
> +break;
> +}
>
>  if (s->desc.status & SDESC_STATUS_COMPLETE) {
>  s->regs[R_DMASR] |= DMASR_HALTED;
> --
> 2.34.1
>
>

Re: [PATCH 1/4] hw/dma: Enhance error handling in loading description

[Edgar E. Iglesias]

On Mon, Jun 3, 2024 at 7:47 AM Fea.Wang  wrote:

> Loading a description from memory may cause a bus-error. In this
> case, the DMA should stop working, set the error flag, and return
> the error value.
>
> Signed-off-by: Fea.Wang 
>


Hi Fea,

I've got a couple of small comments:


---
>  hw/dma/xilinx_axidma.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index 0ae056ed06..4b475e5484 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -71,8 +71,10 @@ enum {
>  enum {
>  DMASR_HALTED = 1,
>  DMASR_IDLE  = 2,
> +DMASR_SLVERR = 1 << 5,
>

We should also add DMASR_DECERR = 1 << 6


>  DMASR_IOC_IRQ  = 1 << 12,
>  DMASR_DLY_IRQ  = 1 << 13,
> +DMASR_ERR_IRQ  = 1 << 14,
>
>  DMASR_IRQ_MASK = 7 << 12
>  };
> @@ -190,17 +192,27 @@ static inline int streamid_from_addr(hwaddr addr)
>  return sid;
>  }
>
> -static void stream_desc_load(struct Stream *s, hwaddr addr)
> +static MemTxResult stream_desc_load(struct Stream *s, hwaddr addr)
>  {
>  struct SDesc *d = &s->desc;
>
> -address_space_read(&s->dma->as, addr, MEMTXATTRS_UNSPECIFIED, d,
> sizeof *d);
> +MemTxResult result = address_space_read(&s->dma->as,
> +addr, MEMTXATTRS_UNSPECIFIED,
> +d, sizeof *d);
> +if (result != MEMTX_OK) {
> +s->regs[R_DMACR] &= ~DMACR_RUNSTOP;
> +s->regs[R_DMASR] |= DMASR_HALTED;
> +s->regs[R_DMASR] |= DMASR_SLVERR;
>

... and map MEMTX_DECODE_ERROR to DMASR_DECERR and everything else to
SLVERR, for example:
if (result == MEMTX_DECODE_ERROR) {
s->regs[R_DMASR] |= DMASR_DECERR;
} else {
s->regs[R_DMASR] |= DMASR_SLVERR;
}


> +s->regs[R_DMASR] |= DMASR_ERR_IRQ;
> +return result;
> +}
>
>  /* Convert from LE into host endianness.  */
>  d->buffer_address = le64_to_cpu(d->buffer_address);
>  d->nxtdesc = le64_to_cpu(d->nxtdesc);
>  d->control = le32_to_cpu(d->control);
>  d->status = le32_to_cpu(d->status);
> +return result;
>  }
>
>  static void stream_desc_store(struct Stream *s, hwaddr addr)
> --
> 2.34.1
>
>

Re: [PATCH] hw/dma/xlnx_dpdma: Read descriptor into buffer, not into pointer-to-buffer

[Edgar E. Iglesias]

On Fri, May 31, 2024 at 2:46 PM Peter Maydell 
wrote:

> In fdf029762f501 we factored out the handling of reading and writing
> DMA descriptors from guest memory.  Unfortunately we accidentally
> made the descriptor-read read the descriptor into the address of the
> buffer rather than into the buffer, because we didn't notice we
> needed to update the arguments to the dma_memory_read() call. Before
> the refactoring, "&desc" is the address of a local struct DPDMADescriptor
> variable in xlnx_dpdma_start_operation(), which is the correct target
> for the guest-memory-read. But after the refactoring 'desc' is the
> "DPDMADescriptor *desc" argument to the new function, and so it is
> already an address.
>
> This bug is an overrun of a stack variable, since a pointer is at
> most 8 bytes long and we try to read 64 bytes, as well as being
> incorrect behaviour.
>
> Pass 'desc' rather than '&desc' as the dma_memory_read() argument
> to fix this.
>
> (The same bug is not present in xlnx_dpdma_write_descriptor(),
> because there we are writing the descriptor from a local struct
> variable "DPDMADescriptor tmp_desc" and so passing &tmp_desc to
> dma_memory_write() is correct.)
>
> Spotted by Coverity: CID 1546649
>
>
+ CC Fred.

Reviewed-by: Edgar E. Iglesias 




> Fixes: fdf029762f50101 ("xlnx_dpdma: fix descriptor endianness bug")
> Signed-off-by: Peter Maydell 
> ---
>  hw/dma/xlnx_dpdma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index dde4aeca401..a685bd28bb8 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -619,7 +619,7 @@ static MemTxResult
> xlnx_dpdma_read_descriptor(XlnxDPDMAState *s,
>DPDMADescriptor *desc)
>  {
>  MemTxResult res = dma_memory_read(&address_space_memory, desc_addr,
> -  &desc, sizeof(DPDMADescriptor),
> +  desc, sizeof(DPDMADescriptor),
>MEMTXATTRS_UNSPECIFIED);
>  if (res) {
>  return res;
> --
> 2.34.1
>
>