| Message ID | 20241010075848.1002891-1-Jiqian.Chen@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen: Remove dependency between pciback and privcmd | expand |
On 10.10.2024 09:58, Jiqian Chen wrote: > --- 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; Perhaps better -EOPNOTSUPP? > +} > +EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_from_sbdf); > \ No newline at end of file Can you please take care of this as well while touching the file? Or maybe you are doing so, but the diff doesn't show it properly? > @@ -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); > } > #endif Why here rather than directly in xen_pcibk_init()? And why no change to xen_pcibk_cleanup() to remove the hook again on unload? Which will then raise the question of possible race conditions. Jan
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 <marmarek@invisiblethingslab.com> > Signed-off-by: Jiqian Chen <Jiqian.Chen@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(). Juergen
On 2024/10/10 16:17, Jan Beulich wrote: > On 10.10.2024 09:58, Jiqian Chen wrote: >> --- 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; > > Perhaps better -EOPNOTSUPP? OK, will change. > >> +} >> +EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_from_sbdf); >> \ No newline at end of file > > Can you please take care of this as well while touching the file? Or > maybe you are doing so, but the diff doesn't show it properly? Yes, will change. > >> @@ -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); >> } >> #endif > > Why here rather than directly in xen_pcibk_init()? And why no change to > xen_pcibk_cleanup() to remove the hook again on unload? Which will then > raise the question of possible race conditions. You are right, will change in next version. > > Jan
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 <marmarek@invisiblethingslab.com> >> Signed-off-by: Jiqian Chen <Jiqian.Chen@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 ? > > > Juergen
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 <marmarek@invisiblethingslab.com> >>> Signed-off-by: Jiqian Chen <Jiqian.Chen@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
On Thu, 10 Oct 2024, 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 <marmarek@invisiblethingslab.com> > Signed-off-by: Jiqian Chen <Jiqian.Chen@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; This can be static > +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); > } > #endif > > diff --git a/include/xen/acpi.h b/include/xen/acpi.h > index daa96a22d257..ef4f70e4a557 100644 > --- a/include/xen/acpi.h > +++ b/include/xen/acpi.h > @@ -91,13 +91,9 @@ static inline int xen_acpi_get_gsi_info(struct pci_dev *dev, > } > #endif > > -#ifdef CONFIG_XEN_PCI_STUB > -int pcistub_get_gsi_from_sbdf(unsigned int sbdf); > -#else > -static inline int pcistub_get_gsi_from_sbdf(unsigned int sbdf) > -{ > - return -1; > -} > -#endif > +typedef int (*get_gsi_from_sbdf_t)(u32 sbdf); > + > +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func); > +int xen_acpi_get_gsi_from_sbdf(unsigned int sbdf); > > #endif /* _XEN_ACPI_H */ > -- > 2.34.1 >
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); } #endif diff --git a/include/xen/acpi.h b/include/xen/acpi.h index daa96a22d257..ef4f70e4a557 100644 --- a/include/xen/acpi.h +++ b/include/xen/acpi.h @@ -91,13 +91,9 @@ static inline int xen_acpi_get_gsi_info(struct pci_dev *dev, } #endif -#ifdef CONFIG_XEN_PCI_STUB -int pcistub_get_gsi_from_sbdf(unsigned int sbdf); -#else -static inline int pcistub_get_gsi_from_sbdf(unsigned int sbdf) -{ - return -1; -} -#endif +typedef int (*get_gsi_from_sbdf_t)(u32 sbdf); + +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func); +int xen_acpi_get_gsi_from_sbdf(unsigned int sbdf); #endif /* _XEN_ACPI_H */
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 <marmarek@invisiblethingslab.com> Signed-off-by: Jiqian Chen <Jiqian.Chen@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(-)