| Message ID | 20241011034227.1278144-1-Jiqian.Chen@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] xen: Remove dependency between pciback and privcmd | expand |
On 11.10.2024 05:42, Jiqian Chen wrote: > @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void) > bus_register_notifier(&pci_bus_type, &pci_stub_nb); > #endif > > +#ifdef CONFIG_XEN_ACPI > + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf); > +#endif > + > return err; > } > > static void __exit xen_pcibk_cleanup(void) > { > +#ifdef CONFIG_XEN_ACPI > + xen_acpi_register_get_gsi_func(NULL); > +#endif Just wondering - instead of these two #ifdef-s, ... > --- 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(u32 sbdf); ... wouldn't a static inline stub (for the !XEN_ACPI case) aid overall readability? Jan
On 2024/10/11 16:54, Jan Beulich wrote: > On 11.10.2024 05:42, Jiqian Chen wrote: >> @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void) >> bus_register_notifier(&pci_bus_type, &pci_stub_nb); >> #endif >> >> +#ifdef CONFIG_XEN_ACPI >> + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf); >> +#endif >> + >> return err; >> } >> >> static void __exit xen_pcibk_cleanup(void) >> { >> +#ifdef CONFIG_XEN_ACPI >> + xen_acpi_register_get_gsi_func(NULL); >> +#endif > > Just wondering - instead of these two #ifdef-s, ... > >> --- 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(u32 sbdf); > > ... wouldn't a static inline stub (for the !XEN_ACPI case) aid overall readability? I'm not sure if other files do this. But for me, it feels a little strange to use "#ifdef CONFIG_XEN_ACPI #else" in apci.h, like self-containment. And "#include apci.h" in pic_stub.c is also wraped with CONFIG_XEN_ACPI. > > Jan
On 2024/10/11 17:20, Chen, Jiqian wrote: > On 2024/10/11 16:54, Jan Beulich wrote: >> On 11.10.2024 05:42, Jiqian Chen wrote: >>> @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void) >>> bus_register_notifier(&pci_bus_type, &pci_stub_nb); >>> #endif >>> >>> +#ifdef CONFIG_XEN_ACPI >>> + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf); >>> +#endif >>> + >>> return err; >>> } >>> >>> static void __exit xen_pcibk_cleanup(void) >>> { >>> +#ifdef CONFIG_XEN_ACPI >>> + xen_acpi_register_get_gsi_func(NULL); >>> +#endif >> >> Just wondering - instead of these two #ifdef-s, ... >> >>> --- 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(u32 sbdf); >> >> ... wouldn't a static inline stub (for the !XEN_ACPI case) aid overall readability? > I'm not sure if other files do this. But for me, it feels a little strange to use "#ifdef CONFIG_XEN_ACPI #else" in apci.h, like self-containment. > And "#include apci.h" in pic_stub.c is also wraped with CONFIG_XEN_ACPI. OK, I saw other files also do this. If you insist, I will make modifications in the next version. Thank you! > >> >> Jan >
On 11.10.2024 11:33, Chen, Jiqian wrote: > On 2024/10/11 17:20, Chen, Jiqian wrote: >> On 2024/10/11 16:54, Jan Beulich wrote: >>> On 11.10.2024 05:42, Jiqian Chen wrote: >>>> @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void) >>>> bus_register_notifier(&pci_bus_type, &pci_stub_nb); >>>> #endif >>>> >>>> +#ifdef CONFIG_XEN_ACPI >>>> + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf); >>>> +#endif >>>> + >>>> return err; >>>> } >>>> >>>> static void __exit xen_pcibk_cleanup(void) >>>> { >>>> +#ifdef CONFIG_XEN_ACPI >>>> + xen_acpi_register_get_gsi_func(NULL); >>>> +#endif >>> >>> Just wondering - instead of these two #ifdef-s, ... >>> >>>> --- 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(u32 sbdf); >>> >>> ... wouldn't a static inline stub (for the !XEN_ACPI case) aid overall readability? >> I'm not sure if other files do this. But for me, it feels a little strange to use "#ifdef CONFIG_XEN_ACPI #else" in apci.h, like self-containment. >> And "#include apci.h" in pic_stub.c is also wraped with CONFIG_XEN_ACPI. > OK, I saw other files also do this. > If you insist, I will make modifications in the next version. Well, I'm not a maintainer of this code, so I can't very well insist. Jan
On 11.10.24 12:10, Jan Beulich wrote: > On 11.10.2024 11:33, Chen, Jiqian wrote: >> On 2024/10/11 17:20, Chen, Jiqian wrote: >>> On 2024/10/11 16:54, Jan Beulich wrote: >>>> On 11.10.2024 05:42, Jiqian Chen wrote: >>>>> @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void) >>>>> bus_register_notifier(&pci_bus_type, &pci_stub_nb); >>>>> #endif >>>>> >>>>> +#ifdef CONFIG_XEN_ACPI >>>>> + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf); >>>>> +#endif >>>>> + >>>>> return err; >>>>> } >>>>> >>>>> static void __exit xen_pcibk_cleanup(void) >>>>> { >>>>> +#ifdef CONFIG_XEN_ACPI >>>>> + xen_acpi_register_get_gsi_func(NULL); >>>>> +#endif >>>> >>>> Just wondering - instead of these two #ifdef-s, ... >>>> >>>>> --- 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(u32 sbdf); >>>> >>>> ... wouldn't a static inline stub (for the !XEN_ACPI case) aid overall readability? >>> I'm not sure if other files do this. But for me, it feels a little strange to use "#ifdef CONFIG_XEN_ACPI #else" in apci.h, like self-containment. >>> And "#include apci.h" in pic_stub.c is also wraped with CONFIG_XEN_ACPI. >> OK, I saw other files also do this. >> If you insist, I will make modifications in the next version. > > Well, I'm not a maintainer of this code, so I can't very well insist. In this case I'm in favor of having the #ifdefs at the calling site. The amount of code isn't larger this way, while it is more clear when reading the code that the calls are only done in the CONFIG_XEN_ACPI case. Juergen
On 11.10.24 05:42, 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 isn't always 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> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On 11.10.24 05:42, 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 isn't always 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> Unfortunately I'm seeing a build breakage for the 32-bit x86 build. Reason is that drivers/xen/acpi.c is being built for Dom0 only, so having CONFIG_XEN_ACPI defined is not enough for ensuring the new functions are present. I suggest to add a new CONFIG item CONFIG_XEN_DOM0_ACPI like config XEN_DOM0_ACPI def_bool y depends on XEN_ACPI && XEN_DOM0 and then replace all CONFIG_XEN_ACPI instances in your patch with CONFIG_XEN_DOM0_ACPI. This includes the use case in privcmd_ioctl_pcidev_get_gsi(). Juergen
On 2024/10/11 20:06, Juergen Gross wrote: > On 11.10.24 05:42, 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 isn't always 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> > > Unfortunately I'm seeing a build breakage for the 32-bit x86 build. > > Reason is that drivers/xen/acpi.c is being built for Dom0 only, so having > CONFIG_XEN_ACPI defined is not enough for ensuring the new functions are > present. > > I suggest to add a new CONFIG item CONFIG_XEN_DOM0_ACPI like > > config XEN_DOM0_ACPI > def_bool y > depends on XEN_ACPI && XEN_DOM0 > > and then replace all CONFIG_XEN_ACPI instances in your patch with > CONFIG_XEN_DOM0_ACPI. This includes the use case in > privcmd_ioctl_pcidev_get_gsi(). How about add stubs for xen_acpi_register_get_gsi_func and xen_acpi_get_gsi_from_sbdf when "!CONFIG_XEN_DOM0" in acpi.h, like the other functions in that file do. > > > Juergen
On 2024/10/12 10:22, Chen, Jiqian wrote: > On 2024/10/11 20:06, Juergen Gross wrote: >> On 11.10.24 05:42, 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 isn't always 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> >> >> Unfortunately I'm seeing a build breakage for the 32-bit x86 build. Could you please attach the link or steps? Then I can reproduce it, and validate it locally with new changes. >> >> Reason is that drivers/xen/acpi.c is being built for Dom0 only, so having >> CONFIG_XEN_ACPI defined is not enough for ensuring the new functions are >> present. >> >> I suggest to add a new CONFIG item CONFIG_XEN_DOM0_ACPI like >> >> config XEN_DOM0_ACPI >> def_bool y >> depends on XEN_ACPI && XEN_DOM0 >> >> and then replace all CONFIG_XEN_ACPI instances in your patch with >> CONFIG_XEN_DOM0_ACPI. This includes the use case in >> privcmd_ioctl_pcidev_get_gsi(). > > How about add stubs for xen_acpi_register_get_gsi_func and xen_acpi_get_gsi_from_sbdf when "!CONFIG_XEN_DOM0" in acpi.h, like the other functions in that file do. > >> >> >> Juergen >
On 12.10.24 04:36, Chen, Jiqian wrote: > On 2024/10/12 10:22, Chen, Jiqian wrote: >> On 2024/10/11 20:06, Juergen Gross wrote: >>> On 11.10.24 05:42, 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 isn't always 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> >>> >>> Unfortunately I'm seeing a build breakage for the 32-bit x86 build. > Could you please attach the link or steps? > Then I can reproduce it, and validate it locally with new changes. I'm using the attached kernel config. Juergen
On 2024/10/12 13:48, Jürgen Groß wrote: > On 12.10.24 04:36, Chen, Jiqian wrote: >> On 2024/10/12 10:22, Chen, Jiqian wrote: >>> On 2024/10/11 20:06, Juergen Gross wrote: >>>> On 11.10.24 05:42, 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 isn't always 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> >>>> >>>> Unfortunately I'm seeing a build breakage for the 32-bit x86 build. >> Could you please attach the link or steps? >> Then I can reproduce it, and validate it locally with new changes. > > I'm using the attached kernel config. Thanks, I can reproduce the 32-bit build error locally. And this "Add stubs for xen_acpi_register_get_gsi_func and xen_acpi_get_gsi_from_sbdf when "!CONFIG_XEN_DOM0" in acpi.h, like the other functions in that file do." can fix it. Is it okay? > > > Juergen >
On 12.10.24 08:50, Chen, Jiqian wrote: > On 2024/10/12 13:48, Jürgen Groß wrote: >> On 12.10.24 04:36, Chen, Jiqian wrote: >>> On 2024/10/12 10:22, Chen, Jiqian wrote: >>>> On 2024/10/11 20:06, Juergen Gross wrote: >>>>> On 11.10.24 05:42, 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 isn't always 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> >>>>> >>>>> Unfortunately I'm seeing a build breakage for the 32-bit x86 build. >>> Could you please attach the link or steps? >>> Then I can reproduce it, and validate it locally with new changes. >> >> I'm using the attached kernel config. > Thanks, I can reproduce the 32-bit build error locally. > And this "Add stubs for xen_acpi_register_get_gsi_func and xen_acpi_get_gsi_from_sbdf when "!CONFIG_XEN_DOM0" in acpi.h, like the other functions in that file do." can fix it. > Is it okay? Fine with me. Juergen
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..d2ee605c5ca1 100644 --- a/drivers/xen/acpi.c +++ b/drivers/xen/acpi.c @@ -125,3 +125,27 @@ int xen_acpi_get_gsi_info(struct pci_dev *dev, return 0; } EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_info); + +static get_gsi_from_sbdf_t get_gsi_from_sbdf; +static DEFINE_RWLOCK(get_gsi_from_sbdf_lock); + +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func) +{ + write_lock(&get_gsi_from_sbdf_lock); + get_gsi_from_sbdf = func; + write_unlock(&get_gsi_from_sbdf_lock); +} +EXPORT_SYMBOL_GPL(xen_acpi_register_get_gsi_func); + +int xen_acpi_get_gsi_from_sbdf(u32 sbdf) +{ + int ret = -EOPNOTSUPP; + + read_lock(&get_gsi_from_sbdf_lock); + if (get_gsi_from_sbdf) + ret = get_gsi_from_sbdf(sbdf); + read_unlock(&get_gsi_from_sbdf_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(xen_acpi_get_gsi_from_sbdf); 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..b616b7768c3b 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, @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void) bus_register_notifier(&pci_bus_type, &pci_stub_nb); #endif +#ifdef CONFIG_XEN_ACPI + xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf); +#endif + return err; } static void __exit xen_pcibk_cleanup(void) { +#ifdef CONFIG_XEN_ACPI + xen_acpi_register_get_gsi_func(NULL); +#endif + #ifdef CONFIG_PCI_IOV bus_unregister_notifier(&pci_bus_type, &pci_stub_nb); #endif diff --git a/include/xen/acpi.h b/include/xen/acpi.h index daa96a22d257..8cb081c633cc 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(u32 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 isn't always 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> --- v2->v3 changes: Added rwlock get_gsi_from_sbdf_lock to avoid races. Called xen_acpi_register_get_gsi_func to register function in xen_pcibk_init and set NULL to hook in xen_pcibk_cleanup when unloading. 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 | 24 ++++++++++++++++++++++++ drivers/xen/privcmd.c | 6 ++---- drivers/xen/xen-pciback/pci_stub.c | 11 +++++++++-- include/xen/acpi.h | 12 ++++-------- 5 files changed, 39 insertions(+), 15 deletions(-)