Re: [PATCH] xen: Fix host pci for stubdom

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Mon, May 15, 2023 at 11:04 AM Anthony PERARD
<anthony.perard@xxxxxxxxxx> wrote:
>
> On Sun, Mar 19, 2023 at 08:05:54PM -0400, Jason Andryuk wrote:
> > diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> > index 8c6e9a1716..51a72b432d 100644
> > --- a/hw/xen/xen-host-pci-device.c
> > +++ b/hw/xen/xen-host-pci-device.c
> > @@ -33,13 +34,101 @@
> >  #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 BFD 
> > (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;
> > +    char path[80];
> > +    char *msg;
> > +
> > +    be_path = qemu_xen_xs_read(xenstore, 0, "device/pci/0/backend", &len);
> > +    if (!be_path) {
> > +        /*
> > +         * be_path doesn't exist, so we are dealing with a local
> > +         * (non-passthough) device.
> > +         */
> > +        d->local_domain = d->domain;
> > +        d->local_bus = d->bus;
> > +        d->local_dev = d->dev;
> > +        d->local_func = d->func;
> > +
> > +        return;
> > +    }
> > +
> > +    snprintf(path, sizeof(path), "%s/num_devs", be_path);
>
> Is 80 bytes for `path` enough?
> What if the path is truncated due to the limit?
>
>
> There's xs_node_scanf() which might be useful. It does the error
> handling and call scanf(). But I'm not sure if it can be used here, in
> this file.

Thanks for the suggestion - I'll take a look.  Your other comments
sound good, too.

Also, for the next version, I plan to change the From: to Marek. I was
thinking of doing it earlier, but failed to do so when it was time to
send out the patch.  Most of the code is Marek's from his Qubes
stubdom repo.  My modifications were to make it work with non-stubdom
as well.

Thanks,
Jason