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

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Mon, Mar 20, 2023 at 2:41 PM Bernhard Beschow <shentey@xxxxxxxxx> wrote:
>
>
>
> Am 20. März 2023 00:05:54 UTC schrieb Jason Andryuk <jandryuk@xxxxxxxxx>:
> >PCI passthrough for an HVM with a stubdom is PV PCI passthrough from
> >dom0 to the stubdom, and then QEMU passthrough of the PCI device inside
> >the stubdom.  xen-pciback has boolean module param passthrough which
> >controls "how to export PCI topology to guest".  If passthrough=1, the
> >frontend shows a PCI SBDF matching the backend host device.  When
> >passthough=0, the frontend will get a sequentially allocated SBDF.
> >
> >libxl passes the host SBDF over QMP to QEMU.  For non-stubdom or stubdom
> >with passthrough=1, this works fine.  However, it fails for
> >passthrough=0 when QEMU can't find the sysfs node for the host SBDF.
> >
> >Handle all these cases.  Look for the xenstore frontend nodes.  If they
> >are missing, then default to using the QMP command provided SBDF.  This
> >is the non-stubdom case.  If xenstore nodes are found, then read the
> >local SBDF from the xenstore nodes.  This will handle either
> >passthrough=0/1 case.
> >
> >Based on a stubdom-specific patch originally by Marek
> >Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, which is based
> >on earlier work by HW42 <hw42@xxxxxxxxx>
> >
> >Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> >---
> > hw/xen/xen-host-pci-device.c | 96 +++++++++++++++++++++++++++++++++++-
> > hw/xen/xen-host-pci-device.h |  6 +++
> > 2 files changed, 101 insertions(+), 1 deletion(-)
> >
> >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
> >@@ -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,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
>
> I'm unable to parse this sentence, which may be due to my unfamiliarity with 
> Xen terminology.

It's two sentences, but it's missing a period.
"Non-passthrough (dom0) accesses are local PCI devices and use the
given BDF."  and "Passthough (stubdom) accesses are through PV
frontend PCI device."

> There is also an extra space before "Those".

It's two spaces between sentences, which visually separates the
sentences.  It's a common formatting, so I think it's okay.

Thanks for taking a look.

> >+ * either have a BDF identical to the backend's BFD 
> >(xen-backend.passthrough=1)

(And a typo here: s/BFD/BDF/)

> >+ * 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.
> >+ */

Regards,
Jason