[PATCH 1/2] hw/xen: detect when running inside stubdomain

[PATCH 1/2] hw/xen: detect when running inside stubdomain

[Marek Marczykowski-Górecki]

Introduce global xen_is_stubdomain variable when qemu is running inside
a stubdomain instead of dom0. This will be relevant for subsequent
patches, as few things like accessing PCI config space need to be done
differently.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 hw/xen/xen-legacy-backend.c | 15 +++++++++++++++
 include/hw/xen/xen.h        |  1 +
 system/globals.c            |  1 +
 3 files changed, 17 insertions(+)

diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 124dd5f3d6..4a09ea2561 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -603,6 +603,19 @@ static void xen_set_dynamic_sysbus(void)
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_XENSYSDEV);
 }
 
+static bool xen_check_stubdomain(void)
+{
+    char *dm_path = g_strdup_printf("/local/domain/%d/image", xen_domid);
+    uint32_t dm_domid;
+    bool is_stubdom = false;
+
+    if (!xenstore_read_int(dm_path, "device-model-domid", &dm_domid))
+        is_stubdom = dm_domid != 0;
+
+    g_free(dm_path);
+    return is_stubdom;
+}
+
 void xen_be_init(void)
 {
     xenstore = qemu_xen_xs_open();
@@ -616,6 +629,8 @@ void xen_be_init(void)
         exit(1);
     }
 
+    xen_is_stubdomain = xen_check_stubdomain();
+
     xen_sysdev = qdev_new(TYPE_XENSYSDEV);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(xen_sysdev), &error_fatal);
     xen_sysbus = qbus_new(TYPE_XENSYSBUS, xen_sysdev, "xen-sysbus");
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 37ecc91fc3..ecb89ecfc1 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -36,6 +36,7 @@ enum xen_mode {
 extern uint32_t xen_domid;
 extern enum xen_mode xen_mode;
 extern bool xen_domid_restrict;
+extern bool xen_is_stubdomain;
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 int xen_set_pci_link_route(uint8_t link, uint8_t irq);
diff --git a/system/globals.c b/system/globals.c
index b6d4e72530..ac27d88bd4 100644
--- a/system/globals.c
+++ b/system/globals.c
@@ -62,6 +62,7 @@ bool qemu_uuid_set;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_DISABLED;
 bool xen_domid_restrict;
+bool xen_is_stubdomain;
 struct evtchn_backend_ops *xen_evtchn_ops;
 struct gnttab_backend_ops *xen_gnttab_ops;
 struct foreignmem_backend_ops *xen_foreignmem_ops;
-- 
2.43.0

[PATCH 2/2] xen: fix stubdom PCI addr

[Marek Marczykowski-Górecki]

From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>

When running in a stubdomain, the config space access via sysfs needs to
use BDF as seen inside stubdomain (connected via xen-pcifront), which is
different from the real BDF. For other purposes (hypercall parameters
etc), the real BDF needs to be used.
Get the in-stubdomain BDF by looking up relevant PV PCI xenstore
entries.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 hw/xen/xen-host-pci-device.c | 77 +++++++++++++++++++++++++++++++++++-
 hw/xen/xen-host-pci-device.h |  6 +++
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 8c6e9a1716..3f8a6f84a8 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -9,6 +9,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
+#include "hw/xen/xen-legacy-backend.h"
 #include "xen-host-pci-device.h"
 
 #define XEN_HOST_PCI_MAX_EXT_CAP \
@@ -33,13 +34,76 @@
 #define IORESOURCE_PREFETCH     0x00001000      /* No side effects */
 #define IORESOURCE_MEM_64       0x00100000
 
+static void xen_host_pci_fill_local_addr(XenHostPCIDevice *d, Error **errp)
+{
+    unsigned int num_devs, len, i;
+    unsigned int domain, bus, dev, func;
+    char *be_path = NULL;
+    char path[80];
+    char *msg = NULL;
+
+    be_path = qemu_xen_xs_read(xenstore, 0, "device/pci/0/backend", &len);
+    if (!be_path)
+        goto err_out;
+    snprintf(path, sizeof(path), "%s/num_devs", be_path);
+    msg = qemu_xen_xs_read(xenstore, 0, path, &len);
+    if (!msg)
+        goto err_out;
+
+    if (sscanf(msg, "%u", &num_devs) != 1) {
+        error_setg(errp, "Failed to parse %s (%s)\n", msg, path);
+        goto err_out;
+    }
+    free(msg);
+
+    for (i = 0; i < num_devs; i++) {
+        snprintf(path, sizeof(path), "%s/dev-%u", be_path, i);
+        msg = qemu_xen_xs_read(xenstore, 0, path, &len);
+        if (!msg) {
+            error_setg(errp, "Failed to read %s\n", path);
+            goto err_out;
+        }
+        if (sscanf(msg, "%x:%x:%x.%x", &domain, &bus, &dev, &func) != 4) {
+            error_setg(errp, "Failed to parse %s (%s)\n", msg, path);
+            goto err_out;
+        }
+        free(msg);
+        if (domain != d->domain ||
+                bus != d->bus ||
+                dev != d->dev ||
+                func!= d->func)
+            continue;
+        snprintf(path, sizeof(path), "%s/vdev-%u", be_path, i);
+        msg = qemu_xen_xs_read(xenstore, 0, path, &len);
+        if (!msg) {
+            error_setg(errp, "Failed to read %s\n", path);
+            goto out;
+        }
+        if (sscanf(msg, "%x:%x:%x.%x", &domain, &bus, &dev, &func) != 4) {
+            error_setg(errp, "Failed to parse %s (%s)\n", msg, path);
+            goto err_out;
+        }
+        free(msg);
+        d->local_domain = domain;
+        d->local_bus = bus;
+        d->local_dev = dev;
+        d->local_func = func;
+        goto out;
+    }
+
+err_out:
+    free(msg);
+out:
+    free(be_path);
+}
+
 static void xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
                                     const char *name, char *buf, ssize_t size)
 {
     int rc;
 
     rc = snprintf(buf, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
-                  d->domain, d->bus, d->dev, d->func, name);
+                  d->local_domain, d->local_bus, d->local_dev, d->local_func, name);
     assert(rc >= 0 && rc < size);
 }
 
@@ -342,6 +406,17 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
     d->dev = dev;
     d->func = func;
 
+    if (xen_is_stubdomain) {
+        xen_host_pci_fill_local_addr(d, errp);
+        if (*errp)
+            goto error;
+    } else {
+        d->local_domain = d->domain;
+        d->local_bus = d->bus;
+        d->local_dev = d->dev;
+        d->local_func = d->func;
+    }
+
     xen_host_pci_config_open(d, errp);
     if (*errp) {
         goto error;
diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
index 4d8d34ecb0..270dcb27f7 100644
--- a/hw/xen/xen-host-pci-device.h
+++ b/hw/xen/xen-host-pci-device.h
@@ -23,6 +23,12 @@ typedef struct XenHostPCIDevice {
     uint8_t dev;
     uint8_t func;
 
+    /* different from the above in case of stubdomain */
+    uint16_t local_domain;
+    uint8_t local_bus;
+    uint8_t local_dev;
+    uint8_t local_func;
+
     uint16_t vendor_id;
     uint16_t device_id;
     uint32_t class_code;
-- 
2.43.0

Re: [PATCH 2/2] xen: fix stubdom PCI addr

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]

On Mon, Feb 19, 2024 at 07:16:06PM +0100, Marek Marczykowski-Górecki wrote:
> From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>

This shouldn't be here, it's my patch.

> When running in a stubdomain, the config space access via sysfs needs to
> use BDF as seen inside stubdomain (connected via xen-pcifront), which is
> different from the real BDF. For other purposes (hypercall parameters
> etc), the real BDF needs to be used.
> Get the in-stubdomain BDF by looking up relevant PV PCI xenstore
> entries.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  hw/xen/xen-host-pci-device.c | 77 +++++++++++++++++++++++++++++++++++-
>  hw/xen/xen-host-pci-device.h |  6 +++
>  2 files changed, 82 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 8c6e9a1716..3f8a6f84a8 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -9,6 +9,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
> +#include "hw/xen/xen-legacy-backend.h"
>  #include "xen-host-pci-device.h"
>  
>  #define XEN_HOST_PCI_MAX_EXT_CAP \
> @@ -33,13 +34,76 @@
>  #define IORESOURCE_PREFETCH     0x00001000      /* No side effects */
>  #define IORESOURCE_MEM_64       0x00100000
>  
> +static void xen_host_pci_fill_local_addr(XenHostPCIDevice *d, Error **errp)
> +{
> +    unsigned int num_devs, len, i;
> +    unsigned int domain, bus, dev, func;
> +    char *be_path = NULL;
> +    char path[80];
> +    char *msg = NULL;
> +
> +    be_path = qemu_xen_xs_read(xenstore, 0, "device/pci/0/backend", &len);
> +    if (!be_path)
> +        goto err_out;
> +    snprintf(path, sizeof(path), "%s/num_devs", be_path);
> +    msg = qemu_xen_xs_read(xenstore, 0, path, &len);
> +    if (!msg)
> +        goto err_out;
> +
> +    if (sscanf(msg, "%u", &num_devs) != 1) {
> +        error_setg(errp, "Failed to parse %s (%s)\n", msg, path);
> +        goto err_out;
> +    }
> +    free(msg);
> +
> +    for (i = 0; i < num_devs; i++) {
> +        snprintf(path, sizeof(path), "%s/dev-%u", be_path, i);
> +        msg = qemu_xen_xs_read(xenstore, 0, path, &len);
> +        if (!msg) {
> +            error_setg(errp, "Failed to read %s\n", path);
> +            goto err_out;
> +        }
> +        if (sscanf(msg, "%x:%x:%x.%x", &domain, &bus, &dev, &func) != 4) {
> +            error_setg(errp, "Failed to parse %s (%s)\n", msg, path);
> +            goto err_out;
> +        }
> +        free(msg);
> +        if (domain != d->domain ||
> +                bus != d->bus ||
> +                dev != d->dev ||
> +                func!= d->func)
> +            continue;
> +        snprintf(path, sizeof(path), "%s/vdev-%u", be_path, i);
> +        msg = qemu_xen_xs_read(xenstore, 0, path, &len);
> +        if (!msg) {
> +            error_setg(errp, "Failed to read %s\n", path);
> +            goto out;
> +        }
> +        if (sscanf(msg, "%x:%x:%x.%x", &domain, &bus, &dev, &func) != 4) {
> +            error_setg(errp, "Failed to parse %s (%s)\n", msg, path);
> +            goto err_out;
> +        }
> +        free(msg);
> +        d->local_domain = domain;
> +        d->local_bus = bus;
> +        d->local_dev = dev;
> +        d->local_func = func;
> +        goto out;
> +    }
> +
> +err_out:
> +    free(msg);
> +out:
> +    free(be_path);
> +}
> +
>  static void xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
>                                      const char *name, char *buf, ssize_t size)
>  {
>      int rc;
>  
>      rc = snprintf(buf, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
> -                  d->domain, d->bus, d->dev, d->func, name);
> +                  d->local_domain, d->local_bus, d->local_dev, d->local_func, name);
>      assert(rc >= 0 && rc < size);
>  }
>  
> @@ -342,6 +406,17 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
>      d->dev = dev;
>      d->func = func;
>  
> +    if (xen_is_stubdomain) {
> +        xen_host_pci_fill_local_addr(d, errp);
> +        if (*errp)
> +            goto error;
> +    } else {
> +        d->local_domain = d->domain;
> +        d->local_bus = d->bus;
> +        d->local_dev = d->dev;
> +        d->local_func = d->func;
> +    }
> +
>      xen_host_pci_config_open(d, errp);
>      if (*errp) {
>          goto error;
> diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
> index 4d8d34ecb0..270dcb27f7 100644
> --- a/hw/xen/xen-host-pci-device.h
> +++ b/hw/xen/xen-host-pci-device.h
> @@ -23,6 +23,12 @@ typedef struct XenHostPCIDevice {
>      uint8_t dev;
>      uint8_t func;
>  
> +    /* different from the above in case of stubdomain */
> +    uint16_t local_domain;
> +    uint8_t local_bus;
> +    uint8_t local_dev;
> +    uint8_t local_func;
> +
>      uint16_t vendor_id;
>      uint16_t device_id;
>      uint32_t class_code;
> -- 
> 2.43.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] xen: fix stubdom PCI addr

[Jason Andryuk]

On Mon, Feb 19, 2024 at 1:49 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>
>
> When running in a stubdomain, the config space access via sysfs needs to
> use BDF as seen inside stubdomain (connected via xen-pcifront), which is
> different from the real BDF. For other purposes (hypercall parameters
> etc), the real BDF needs to be used.
> Get the in-stubdomain BDF by looking up relevant PV PCI xenstore
> entries.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Anthony made these comments on a different version of this patch:
https://lore.kernel.org/xen-devel/48c55d33-aa16-4867-a477-f6df45c7d9d9@perard/

(Sorry I lost track of addressing them at the time.)

Regards,
Jason

Re: [PATCH 1/2] hw/xen: detect when running inside stubdomain

[Philippe Mathieu-Daudé]

On 19/2/24 19:16, Marek Marczykowski-Górecki wrote:
> Introduce global xen_is_stubdomain variable when qemu is running inside
> a stubdomain instead of dom0. This will be relevant for subsequent
> patches, as few things like accessing PCI config space need to be done
> differently.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>   hw/xen/xen-legacy-backend.c | 15 +++++++++++++++
>   include/hw/xen/xen.h        |  1 +
>   system/globals.c            |  1 +
>   3 files changed, 17 insertions(+)


> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index 37ecc91fc3..ecb89ecfc1 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -36,6 +36,7 @@ enum xen_mode {
>   extern uint32_t xen_domid;
>   extern enum xen_mode xen_mode;
>   extern bool xen_domid_restrict;
> +extern bool xen_is_stubdomain;
>   
>   int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
>   int xen_set_pci_link_route(uint8_t link, uint8_t irq);
> diff --git a/system/globals.c b/system/globals.c
> index b6d4e72530..ac27d88bd4 100644
> --- a/system/globals.c
> +++ b/system/globals.c
> @@ -62,6 +62,7 @@ bool qemu_uuid_set;
>   uint32_t xen_domid;
>   enum xen_mode xen_mode = XEN_DISABLED;
>   bool xen_domid_restrict;
> +bool xen_is_stubdomain;

Note for myself, Paolo and Claudio, IIUC these fields belong
to TYPE_XEN_ACCEL in accel/xen/xen-all.c. Maybe resulting in
smth like:

-- >8 --
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..fc25d8c912 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -24,11 +24,31 @@
  #include "migration/global_state.h"
  #include "hw/boards.h"

-bool xen_allowed;
+struct XenAccelState
+{
+    AccelState parent_obj;
+
+    bool xen_allowed;
+
+    enum xen_mode xen_mode;
+
+    uint32_t xen_domid;
+    bool xen_domid_restrict;

  xc_interface *xen_xc;
  xenforeignmemory_handle *xen_fmem;
  xendevicemodel_handle *xen_dmod;
+};
+
+struct XenAccelOpsClass
+{
+    AccelOpsClass parent_class;
+
+    struct evtchn_backend_ops *xen_evtchn_ops;
+    struct gnttab_backend_ops *xen_gnttab_ops;
+    struct foreignmem_backend_ops *xen_foreignmem_ops;
+    struct xenstore_backend_ops *xen_xenstore_ops;
+}

  static void xenstore_record_dm_state(const char *state)
  {
@@ -114,6 +134,13 @@ static int xen_init(MachineState *ms)
      return 0;
  }

+static void xen_accel_init(Object *obj)
+{
+    XenAccelState *s = XEN_ACCEL(obj);
+
+    s->xen_mode = XEN_DISABLED;
+}
+
  static void xen_accel_class_init(ObjectClass *oc, void *data)
  {
      AccelClass *ac = ACCEL_CLASS(oc);
@@ -142,6 +169,8 @@ static void xen_accel_class_init(ObjectClass *oc, 
void *data)
  static const TypeInfo xen_accel_type = {
      .name = TYPE_XEN_ACCEL,
      .parent = TYPE_ACCEL,
+    .instance_size = sizeof(XenAccelState),
+    .instance_init = xen_accel_init,
      .class_init = xen_accel_class_init,
  };

@@ -157,6 +186,7 @@ static const TypeInfo xen_accel_ops_type = {

      .parent = TYPE_ACCEL_OPS,
      .class_init = xen_accel_ops_class_init,
+    .class_size = sizeof(XenAccelOpsClass),
      .abstract = true,
  };

---

Re: [PATCH 1/2] hw/xen: detect when running inside stubdomain

[Jason Andryuk]

On Tue, Feb 20, 2024 at 1:50 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 19/2/24 19:16, Marek Marczykowski-Górecki wrote:
> > Introduce global xen_is_stubdomain variable when qemu is running inside
> > a stubdomain instead of dom0. This will be relevant for subsequent
> > patches, as few things like accessing PCI config space need to be done
> > differently.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> >   hw/xen/xen-legacy-backend.c | 15 +++++++++++++++
> >   include/hw/xen/xen.h        |  1 +
> >   system/globals.c            |  1 +
> >   3 files changed, 17 insertions(+)
>
>
> > diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> > index 37ecc91fc3..ecb89ecfc1 100644
> > --- a/include/hw/xen/xen.h
> > +++ b/include/hw/xen/xen.h
> > @@ -36,6 +36,7 @@ enum xen_mode {
> >   extern uint32_t xen_domid;
> >   extern enum xen_mode xen_mode;
> >   extern bool xen_domid_restrict;
> > +extern bool xen_is_stubdomain;
> >
> >   int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
> >   int xen_set_pci_link_route(uint8_t link, uint8_t irq);
> > diff --git a/system/globals.c b/system/globals.c
> > index b6d4e72530..ac27d88bd4 100644
> > --- a/system/globals.c
> > +++ b/system/globals.c
> > @@ -62,6 +62,7 @@ bool qemu_uuid_set;
> >   uint32_t xen_domid;
> >   enum xen_mode xen_mode = XEN_DISABLED;
> >   bool xen_domid_restrict;
> > +bool xen_is_stubdomain;
>
> Note for myself, Paolo and Claudio, IIUC these fields belong
> to TYPE_XEN_ACCEL in accel/xen/xen-all.c. Maybe resulting in
> smth like:

I think some of these are used by the KVM Xen-emulation, so they can't
just be moved into the Xen accelerator.  David?

Regards,
Jason

Re: [PATCH 1/2] hw/xen: detect when running inside stubdomain

[Jason Andryuk]

On Mon, Feb 19, 2024 at 1:17 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> Introduce global xen_is_stubdomain variable when qemu is running inside
> a stubdomain instead of dom0. This will be relevant for subsequent
> patches, as few things like accessing PCI config space need to be done
> differently.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Thanks,
Jason

Re: [PATCH 1/2] hw/xen: detect when running inside stubdomain

[Philippe Mathieu-Daudé]

On 19/2/24 19:16, Marek Marczykowski-Górecki wrote:
> Introduce global xen_is_stubdomain variable when qemu is running inside
> a stubdomain instead of dom0. This will be relevant for subsequent
> patches, as few things like accessing PCI config space need to be done
> differently.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>   hw/xen/xen-legacy-backend.c | 15 +++++++++++++++
>   include/hw/xen/xen.h        |  1 +
>   system/globals.c            |  1 +
>   3 files changed, 17 insertions(+)


> +static bool xen_check_stubdomain(void)
> +{
> +    char *dm_path = g_strdup_printf("/local/domain/%d/image", xen_domid);
> +    uint32_t dm_domid;
> +    bool is_stubdom = false;
> +
> +    if (!xenstore_read_int(dm_path, "device-model-domid", &dm_domid))

BTW missing braces for QEMU coding style: {

> +        is_stubdom = dm_domid != 0;

}

> +
> +    g_free(dm_path);
> +    return is_stubdom;
> +}