On 10.10.24 10:43, Chen, Jiqian wrote:
On 2024/10/10 16:39, Jürgen Groß wrote:On 10.10.24 09:58, Jiqian Chen wrote:Commit 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev") adds a weak reverse dependency to the config XEN_PRIVCMD definition, that dependency causes xen-privcmd can't be loaded on domU, because dependent xen-pciback is always not be loaded successfully on domU.To solve above problem, remove that dependency, and do not call pcistub_get_gsi_from_sbdf() directly, instead add a hook in drivers/xen/apci.c, xen-pciback register the real call function, then in privcmd_ioctl_pcidev_get_gsi call that hook. Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev") Reported-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com> --- v1->v2 changes: Added hook xen_acpi_get_gsi_from_sbdf. Changed pcistub_get_gsi_from_sbdf to static and pciback register it as the real hook function. --- drivers/xen/Kconfig | 1 - drivers/xen/acpi.c | 17 +++++++++++++++++ drivers/xen/privcmd.c | 6 ++---- drivers/xen/xen-pciback/pci_stub.c | 4 ++-- include/xen/acpi.h | 12 ++++-------- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 72ddee4c1544..f7d6f47971fd 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -261,7 +261,6 @@ config XEN_SCSI_BACKEND config XEN_PRIVCMD tristate "Xen hypercall passthrough driver" depends on XEN - imply XEN_PCIDEV_BACKEND default m help The hypercall passthrough driver allows privileged user programs to diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c index 9e2096524fbc..0bff2f3a87d2 100644 --- a/drivers/xen/acpi.c +++ b/drivers/xen/acpi.c @@ -125,3 +125,20 @@ int xen_acpi_get_gsi_info(struct pci_dev *dev, return 0; } EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_info); + +get_gsi_from_sbdf_t get_gsi_from_sbdf = NULL; + +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func) +{ + get_gsi_from_sbdf = func; +} +EXPORT_SYMBOL_GPL(xen_acpi_register_get_gsi_func); + +int xen_acpi_get_gsi_from_sbdf(u32 sbdf) +{ + if (get_gsi_from_sbdf) + return get_gsi_from_sbdf(sbdf); + + return -EINVAL; +} +EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_from_sbdf); \ No newline at end of file diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 3273cb8c2a66..4f75bc876454 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -850,15 +850,13 @@ static long privcmd_ioctl_mmap_resource(struct file *file, static long privcmd_ioctl_pcidev_get_gsi(struct file *file, void __user *udata) { #if defined(CONFIG_XEN_ACPI) - int rc = -EINVAL; + int rc; struct privcmd_pcidev_get_gsi kdata; if (copy_from_user(&kdata, udata, sizeof(kdata))) return -EFAULT; - if (IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)) - rc = pcistub_get_gsi_from_sbdf(kdata.sbdf); - + rc = xen_acpi_get_gsi_from_sbdf(kdata.sbdf); if (rc < 0) return rc; diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 2f3da5ac62cd..900e6199eec7 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -227,7 +227,7 @@ static struct pci_dev *pcistub_device_get_pci_dev(struct xen_pcibk_device *pdev, } #ifdef CONFIG_XEN_ACPI -int pcistub_get_gsi_from_sbdf(unsigned int sbdf) +static int pcistub_get_gsi_from_sbdf(unsigned int sbdf) { struct pcistub_device *psdev; int domain = (sbdf >> 16) & 0xffff; @@ -242,7 +242,6 @@ int pcistub_get_gsi_from_sbdf(unsigned int sbdf) return psdev->gsi; } -EXPORT_SYMBOL_GPL(pcistub_get_gsi_from_sbdf); #endif struct pci_dev *pcistub_get_pci_dev_by_slot(struct xen_pcibk_device *pdev, @@ -484,6 +483,7 @@ static int pcistub_init_device(struct pcistub_device *psdev) if (err) goto config_release; psdev->gsi = gsi; + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf);I don't think this is the right place for registering the function. It should be done at the end of xen_pcibk_init() (guarded with CONFIG_XEN_ACPI). Additionally you should reset the function pointer NULL in xen_pcibk_cleanup().Should I need to add a new unregister function to reset, or just pass NULL to xen_acpi_register_get_gsi_func ?
I think passing NULL is fine. But you'll need some kind of locking to avoid a race between a call of the hook and parallel unloading the pciback module. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
