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

[Jason Andryuk]

On Tue, Mar 5, 2024 at 2:13 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> From: Frédéric Pierret (fepitre) <frederic.pierret@qubes-os.org>

Needs to be changed to Marek.

> 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>
> ---
> Changes in v2:
> - use xs_node_scanf
> - use %d instead of %u to read values written as %d
> - add a comment from another iteration of this patch by Jason Andryuk
> ---
>  hw/xen/xen-host-pci-device.c | 69 +++++++++++++++++++++++++++++++++++-
>  hw/xen/xen-host-pci-device.h |  6 ++++
>  2 files changed, 74 insertions(+), 1 deletion(-)
>
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 8c6e9a1716..8ea2a5a4af 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -9,6 +9,8 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
> +#include "hw/xen/xen-legacy-backend.h"
> +#include "hw/xen/xen-bus-helper.h"
>  #include "xen-host-pci-device.h"
>
>  #define XEN_HOST_PCI_MAX_EXT_CAP \
> @@ -33,13 +35,67 @@
>  #define IORESOURCE_PREFETCH     0x00001000      /* No side effects */
>  #define IORESOURCE_MEM_64       0x00100000
>
> +/*
> + * Non-passthrough (dom0) accesses are local PCI devices and use the given 
> BDF
> + * Passthough (stubdom) accesses are through PV frontend PCI device.  Those
> + * either have a BDF identical to the backend's BDF 
> (xen-backend.passthrough=1)
> + * or a local virtual BDF (xen-backend.passthrough=0)
> + *
> + * We are always given the backend's BDF and need to lookup the appropriate
> + * local BDF for sysfs access.
> + */
> +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];

path is now only used for dev/vdev-%d, so 80 could be reduced.

> +
> +    be_path = qemu_xen_xs_read(xenstore, 0, "device/pci/0/backend", &len);
> +    if (!be_path)

error_setg() here?

> +        goto out;
> +
> +    if (xs_node_scanf(xenstore, 0, be_path, "num_devs", NULL, "%d", 
> &num_devs) != 1) {
> +        error_setg(errp, "Failed to read or parse %s/num_devs\n", be_path);
> +        goto out;
> +    }
> +
> +    for (i = 0; i < num_devs; i++) {
> +        snprintf(path, sizeof(path), "dev-%d", i);
> +        if (xs_node_scanf(xenstore, 0, be_path, path, NULL,
> +                          "%x:%x:%x.%x", &domain, &bus, &dev, &func) != 4) {
> +            error_setg(errp, "Failed to read or parse %s/%s\n", be_path, 
> path);
> +            goto out;
> +        }
> +        if (domain != d->domain ||
> +                bus != d->bus ||
> +                dev != d->dev ||
> +                func!= d->func)
> +            continue;
> +        snprintf(path, sizeof(path), "vdev-%d", i);
> +        if (xs_node_scanf(xenstore, 0, be_path, path, NULL,
> +                          "%x:%x:%x.%x", &domain, &bus, &dev, &func) != 4) {
> +            error_setg(errp, "Failed to read or parse %s/%s\n", be_path, 
> path);
> +            goto out;
> +        }
> +        d->local_domain = domain;
> +        d->local_bus = bus;
> +        d->local_dev = dev;
> +        d->local_func = func;
> +        goto out;
> +    }

error_setg here in case we exited the loop without finding a match?

Thanks,
Jason

> +
> +out:
> +    free(be_path);
> +}
> +