On 23.09.21 22:47, Stefano Stabellini wrote:
> On Thu, 23 Sep 2021, Oleksandr Andrushchenko wrote:
>> 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_andrushche...@epam.com>
>>
>> ---
>> Changes since v2:
>> - swap the patch order
>> New in v2
>> ---
>> drivers/xen/Kconfig | 24 ++++++++++++++++++++++++
>> 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, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>> index a37eb52fb401..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.
>> +
> I get this build error on ARM:
>
> drivers/xen/xen-pciback/pci_stub.c:22:10: fatal error: asm/xen/pci.h: No such
> file or directory
> #include <asm/xen/pci.h>
> ^~~~~~~~~~~~~~~
> compilation terminated.
> scripts/Makefile.build:277: recipe for target
> 'drivers/xen/xen-pciback/pci_stub.o' failed
>
>
> I imagine that the second patch is required for the build on ARM.
Ah, yes. That is why it was the first
> I
> suggest to move the definition of XEN_PCIDEV_STUB to the second patch.
> (keep XEN_PCI_STUB and xen_pcibk_pv_support here.)
I'll go with this
>
> Alternatively, just remove "tristate "Xen PCI-device stub driver"" from
> here so that XEN_PCIDEV_STUB becomes non-selectable. You can add the
> tristate property in patch #2.
It looks like half-baked...
>
>
>
>> config XEN_PCIDEV_BACKEND
>> tristate "Xen PCI-device backend driver"
>> depends on PCI && X86 && XEN
>> depends on XEN_BACKEND
>> + 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 c09c7ebd6968..f8ba2903a3ff 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;
>> +
>> 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
>>
