[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] xen-pciback: prepare for the split for stub and PV
On 23.09.21 00:10, Stefano Stabellini wrote: > On Wed, 22 Sep 2021, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> >> >> Currently PCI backend implements multiple functionalities at a time. >> To name a few: >> 1. it is used as a database for assignable PCI devices, e.g. xl >> pci-assignable-{add|remove|list} manipulates that list. So, whenever >> the toolstack needs to know which PCI devices can be passed through >> it reads that from the relevant sysfs entries of the pciback. >> 2. it is used to hold the unbound PCI devices list, e.g. when passing >> through a PCI device it needs to be unbound from the relevant device >> driver and bound to pciback (strictly speaking it is not required >> that the device is bound to pciback, but pciback is again used as a >> database of the passed through PCI devices, so we can re-bind the >> devices back to their original drivers when guest domain shuts down) >> 3. device reset for the devices being passed through >> 4. para-virtualized use-cases support >> >> The para-virtualized part of the driver is not always needed as some >> architectures, e.g. Arm or x86 PVH Dom0, are not using backend-frontend >> model for PCI device passthrough. For such use-cases make the very >> first step in splitting the xen-pciback driver into two parts: extended >> PCI stub and PCI PV backend drivers. At the moment x86 platform will >> continue using CONFIG_XEN_PCIDEV_BACKEND for the fully featured backend >> driver and new platforms may build a driver with limited functionality >> (no PV) by enabling CONFIG_XEN_PCIDEV_STUB. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> > Please make this the first patch of the series so that... > >> --- >> New in v2 >> --- >> drivers/xen/Kconfig | 26 +++++++++++++++++++++++++- >> drivers/xen/Makefile | 2 +- >> drivers/xen/xen-pciback/Makefile | 1 + >> drivers/xen/xen-pciback/pciback.h | 5 +++++ >> drivers/xen/xen-pciback/xenbus.c | 6 +++++- >> 5 files changed, 37 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig >> index 057ddf61ef61..6e92c6be19f1 100644 >> --- a/drivers/xen/Kconfig >> +++ b/drivers/xen/Kconfig >> @@ -180,10 +180,34 @@ config SWIOTLB_XEN >> select DMA_OPS >> select SWIOTLB >> >> +config XEN_PCI_STUB >> + bool >> + >> +config XEN_PCIDEV_STUB >> + tristate "Xen PCI-device stub driver" >> + depends on PCI && !X86 && XEN >> + depends on XEN_BACKEND >> + select XEN_PCI_STUB >> + default m >> + help >> + The PCI device stub driver provides limited version of the PCI >> + device backend driver without para-virtualized support for guests. >> + If you select this to be a module, you will need to make sure no >> + other driver has bound to the device(s) you want to make visible to >> + other guests. >> + >> + The "hide" parameter (only applicable if backend driver is compiled >> + into the kernel) allows you to bind the PCI devices to this module >> + from the default device drivers. The argument is the list of PCI BDFs: >> + xen-pciback.hide=(03:00.0)(04:00.0) >> + >> + If in doubt, say m. >> + >> config XEN_PCIDEV_BACKEND >> tristate "Xen PCI-device backend driver" >> - depends on PCI && XEN >> + depends on PCI && X86 && XEN >> depends on XEN_BACKEND > ...we don't need this Yes, I didn't like this change too > > >> + select XEN_PCI_STUB >> default m >> help >> The PCI device backend driver allows the kernel to export arbitrary >> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile >> index 3434593455b2..5aae66e638a7 100644 >> --- a/drivers/xen/Makefile >> +++ b/drivers/xen/Makefile >> @@ -24,7 +24,7 @@ obj-$(CONFIG_XEN_SYS_HYPERVISOR) += sys-hypervisor.o >> obj-$(CONFIG_XEN_PVHVM_GUEST) += platform-pci.o >> obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o >> obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o >> -obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/ >> +obj-$(CONFIG_XEN_PCI_STUB) += xen-pciback/ >> obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o >> obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o >> obj-$(CONFIG_XEN_EFI) += efi.o >> diff --git a/drivers/xen/xen-pciback/Makefile >> b/drivers/xen/xen-pciback/Makefile >> index e8d981d43235..e2cb376444a6 100644 >> --- a/drivers/xen/xen-pciback/Makefile >> +++ b/drivers/xen/xen-pciback/Makefile >> @@ -1,5 +1,6 @@ >> # SPDX-License-Identifier: GPL-2.0 >> obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback.o >> +obj-$(CONFIG_XEN_PCIDEV_STUB) += xen-pciback.o >> >> xen-pciback-y := pci_stub.o pciback_ops.o xenbus.o >> xen-pciback-y += conf_space.o conf_space_header.o \ >> diff --git a/drivers/xen/xen-pciback/pciback.h >> b/drivers/xen/xen-pciback/pciback.h >> index 95e28ee48d52..9a64196e831d 100644 >> --- a/drivers/xen/xen-pciback/pciback.h >> +++ b/drivers/xen/xen-pciback/pciback.h >> @@ -71,6 +71,11 @@ struct pci_dev *pcistub_get_pci_dev(struct >> xen_pcibk_device *pdev, >> struct pci_dev *dev); >> void pcistub_put_pci_dev(struct pci_dev *dev); >> >> +static inline bool xen_pcibk_pv_support(void) >> +{ >> + return IS_ENABLED(CONFIG_XEN_PCIDEV_BACKEND); >> +} >> + >> /* Ensure a device is turned off or reset */ >> void xen_pcibk_reset_device(struct pci_dev *pdev); >> >> diff --git a/drivers/xen/xen-pciback/xenbus.c >> b/drivers/xen/xen-pciback/xenbus.c >> index da34ce85dc88..bde63ef677b8 100644 >> --- a/drivers/xen/xen-pciback/xenbus.c >> +++ b/drivers/xen/xen-pciback/xenbus.c >> @@ -743,6 +743,9 @@ const struct xen_pcibk_backend *__read_mostly >> xen_pcibk_backend; >> >> int __init xen_pcibk_xenbus_register(void) >> { >> + if (!xen_pcibk_pv_support()) >> + return 0; > Is this truly enough to stop the PV backend from initializing? Have you > actually tested it to make sure? If it works, amazing! I am quite happy > about this approach :-) Well, I put some logs into the driver and saw nothing obvious pointing to any backend activities (probably this is also because I don't have any frontend). I see that the xenbus driver is not registered. In XenStore I see: root@dom0:~# xenstore-ls -f | grep pci /local/domain/0/backend/pci = "" /local/domain/0/backend/pci/2 = "" /local/domain/0/backend/pci/2/0 = "" /local/domain/0/backend/pci/2/0/frontend = "/local/domain/2/device/pci/0" /local/domain/0/backend/pci/2/0/frontend-id = "2" /local/domain/0/backend/pci/2/0/online = "1" /local/domain/0/backend/pci/2/0/state = "1" /local/domain/0/backend/pci/2/0/domain = "DomU" /local/domain/0/backend/pci/2/0/key-0 = "0000:03:00.0" /local/domain/0/backend/pci/2/0/dev-0 = "0000:03:00.0" /local/domain/0/backend/pci/2/0/opts-0 = "msitranslate=0,power_mgmt=0,permissive=0,rdm_policy=strict" /local/domain/0/backend/pci/2/0/state-0 = "1" /local/domain/0/backend/pci/2/0/num_devs = "1" /local/domain/2/device/pci = "" /local/domain/2/device/pci/0 = "" /local/domain/2/device/pci/0/backend = "/local/domain/0/backend/pci/2/0" /local/domain/2/device/pci/0/backend-id = "0" /local/domain/2/device/pci/0/state = "1" /libxl/pci = "" /libxl/pci/0000-03-00-0 = "" /libxl/pci/0000-03-00-0/domid = "2" But IIUIC these come from the toolstack @Juergen, do you know how to check if the backend is indeed not running or the above should be enough to prove? > > > > >> xen_pcibk_backend = &xen_pcibk_vpci_backend; >> if (passthrough) >> xen_pcibk_backend = &xen_pcibk_passthrough_backend; >> @@ -752,5 +755,6 @@ int __init xen_pcibk_xenbus_register(void) >> >> void __exit xen_pcibk_xenbus_unregister(void) >> { >> - xenbus_unregister_driver(&xen_pcibk_driver); >> + if (xen_pcibk_pv_support()) >> + xenbus_unregister_driver(&xen_pcibk_driver); >> } >> -- >> 2.25.1 >> Thank you, Oleksandr
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |