* [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
@ 2024-10-09 6:20 Jiqian Chen
2024-10-09 7:05 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: Jiqian Chen @ 2024-10-09 6:20 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko
Cc: xen-devel, linux-kernel, Jiqian Chen
Commit 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
adds a weak reverse dependency to the config XEN_PRIVCMD definition, its
purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
find the implementation of pcistub_get_gsi_from_sbdf.
But 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 and cover original commit's requirement, just remove
that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
of original commit is enough to meet the requirement.
Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
drivers/xen/Kconfig | 1 -
1 file changed, 1 deletion(-)
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
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-09 6:20 [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition Jiqian Chen
@ 2024-10-09 7:05 ` Jan Beulich
2024-10-09 22:46 ` Stefano Stabellini
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2024-10-09 7:05 UTC (permalink / raw)
To: Jiqian Chen
Cc: xen-devel, linux-kernel, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Marek Marczykowski
On 09.10.2024 08:20, 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, its
> purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
> and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
> is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
> find the implementation of pcistub_get_gsi_from_sbdf.
>
> But 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 and cover original commit's requirement, just remove
> that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
> of original commit is enough to meet the requirement.
>
> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
This lacks a Reported-by:.
> --- 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
The report wasn't about a build problem, but a runtime one. Removing the
dependency here doesn't change anything in the dependency of xen-privcmd
on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
exist. Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
I guess is what Marek is using in his config. Both drivers are available
in such a configuration, yet loading of xen-privcmd then requires to
load xen-pciback first. And that latter load attempt will fail in a DomU.
The two drivers simply may not have any dependency in either direction.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-09 7:05 ` Jan Beulich
@ 2024-10-09 22:46 ` Stefano Stabellini
2024-10-10 5:39 ` Jürgen Groß
0 siblings, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2024-10-09 22:46 UTC (permalink / raw)
To: Jan Beulich
Cc: Jiqian Chen, xen-devel, linux-kernel, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Marek Marczykowski
On Wed, 9 Oct 2024, Jan Beulich wrote:
> On 09.10.2024 08:20, 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, its
> > purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
> > and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
> > is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
> > find the implementation of pcistub_get_gsi_from_sbdf.
> >
> > But 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 and cover original commit's requirement, just remove
> > that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
> > of original commit is enough to meet the requirement.
> >
> > Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
> > Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>
> This lacks a Reported-by:.
>
> > --- 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
>
> The report wasn't about a build problem, but a runtime one. Removing the
> dependency here doesn't change anything in the dependency of xen-privcmd
> on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
> exist.
>
> Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
> I guess is what Marek is using in his config. Both drivers are available
> in such a configuration, yet loading of xen-privcmd then requires to
> load xen-pciback first. And that latter load attempt will fail in a DomU.
> The two drivers simply may not have any dependency in either direction.
The idea is that there should be no hard dependency on
pcistub_get_gsi_from_sbdf(). If it is available, the service will be
used, otherwise an error will be reported.
The problem is that IS_REACHABLE is a compile-time check. What we need
is a runtime check instead. Maybe symbol_get or try_module_get ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-09 22:46 ` Stefano Stabellini
@ 2024-10-10 5:39 ` Jürgen Groß
2024-10-10 7:01 ` Jan Beulich
2024-10-10 7:07 ` Chen, Jiqian
0 siblings, 2 replies; 8+ messages in thread
From: Jürgen Groß @ 2024-10-10 5:39 UTC (permalink / raw)
To: Stefano Stabellini, Jan Beulich
Cc: Jiqian Chen, xen-devel, linux-kernel, Oleksandr Tyshchenko,
Marek Marczykowski
[-- Attachment #1.1.1: Type: text/plain, Size: 3131 bytes --]
On 10.10.24 00:46, Stefano Stabellini wrote:
> On Wed, 9 Oct 2024, Jan Beulich wrote:
>> On 09.10.2024 08:20, 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, its
>>> purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
>>> and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
>>> is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
>>> find the implementation of pcistub_get_gsi_from_sbdf.
>>>
>>> But 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 and cover original commit's requirement, just remove
>>> that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
>>> of original commit is enough to meet the requirement.
>>>
>>> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
>>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>>
>> This lacks a Reported-by:.
>>
>>> --- 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
>>
>> The report wasn't about a build problem, but a runtime one. Removing the
>> dependency here doesn't change anything in the dependency of xen-privcmd
>> on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
>> exist.
>>
>> Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
>> I guess is what Marek is using in his config. Both drivers are available
>> in such a configuration, yet loading of xen-privcmd then requires to
>> load xen-pciback first. And that latter load attempt will fail in a DomU.
>> The two drivers simply may not have any dependency in either direction.
>
> The idea is that there should be no hard dependency on
> pcistub_get_gsi_from_sbdf(). If it is available, the service will be
> used, otherwise an error will be reported.
>
> The problem is that IS_REACHABLE is a compile-time check. What we need
> is a runtime check instead. Maybe symbol_get or try_module_get ?
This is a rather clumsy solution IMO.
I'm seeing the following solutions:
1. Don't fail to to load the pciback driver in a DomU, but only disable
any functionality.
2. Move the drivers/xen/xen-pciback/pci_stub.c functionality in a module
of its own, allowing the privcmd driver to be loaded without needing
the rest of pciback.
3. Add a hook to e.g. drivers/xen/pci.c instead for calling of
pcistub_get_gsi_from_sbdf() directly. pciback could register the real
call address. If pciback isn't loaded, the hook can return an error.
This would remove the explicit dependency of privcmd on pciback.
I'd prefer the 3rd variant.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-10 5:39 ` Jürgen Groß
@ 2024-10-10 7:01 ` Jan Beulich
2024-10-10 7:21 ` Jürgen Groß
2024-10-10 7:07 ` Chen, Jiqian
1 sibling, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2024-10-10 7:01 UTC (permalink / raw)
To: Jürgen Groß, Stefano Stabellini
Cc: Jiqian Chen, xen-devel, linux-kernel, Oleksandr Tyshchenko,
Marek Marczykowski
On 10.10.2024 07:39, Jürgen Groß wrote:
> On 10.10.24 00:46, Stefano Stabellini wrote:
>> On Wed, 9 Oct 2024, Jan Beulich wrote:
>>> On 09.10.2024 08:20, 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, its
>>>> purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
>>>> and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
>>>> is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
>>>> find the implementation of pcistub_get_gsi_from_sbdf.
>>>>
>>>> But 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 and cover original commit's requirement, just remove
>>>> that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
>>>> of original commit is enough to meet the requirement.
>>>>
>>>> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
>>>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>>>
>>> This lacks a Reported-by:.
>>>
>>>> --- 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
>>>
>>> The report wasn't about a build problem, but a runtime one. Removing the
>>> dependency here doesn't change anything in the dependency of xen-privcmd
>>> on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
>>> exist.
>>>
>>> Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
>>> I guess is what Marek is using in his config. Both drivers are available
>>> in such a configuration, yet loading of xen-privcmd then requires to
>>> load xen-pciback first. And that latter load attempt will fail in a DomU.
>>> The two drivers simply may not have any dependency in either direction.
>>
>> The idea is that there should be no hard dependency on
>> pcistub_get_gsi_from_sbdf(). If it is available, the service will be
>> used, otherwise an error will be reported.
>>
>> The problem is that IS_REACHABLE is a compile-time check. What we need
>> is a runtime check instead. Maybe symbol_get or try_module_get ?
>
> This is a rather clumsy solution IMO.
>
> I'm seeing the following solutions:
>
> 1. Don't fail to to load the pciback driver in a DomU, but only disable
> any functionality.
>
> 2. Move the drivers/xen/xen-pciback/pci_stub.c functionality in a module
> of its own, allowing the privcmd driver to be loaded without needing
> the rest of pciback.
>
> 3. Add a hook to e.g. drivers/xen/pci.c instead for calling of
> pcistub_get_gsi_from_sbdf() directly. pciback could register the real
> call address. If pciback isn't loaded, the hook can return an error.
> This would remove the explicit dependency of privcmd on pciback.
>
> I'd prefer the 3rd variant.
Same here - order of preference backwards in the set presented above.
In the meantime the original change may simply need reverting?
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-10 5:39 ` Jürgen Groß
2024-10-10 7:01 ` Jan Beulich
@ 2024-10-10 7:07 ` Chen, Jiqian
2024-10-10 7:18 ` Jürgen Groß
1 sibling, 1 reply; 8+ messages in thread
From: Chen, Jiqian @ 2024-10-10 7:07 UTC (permalink / raw)
To: Jürgen Groß
Cc: Stefano Stabellini, Jan Beulich, Chen, Jiqian, xen-devel,
linux-kernel, Oleksandr Tyshchenko, Marek Marczykowski
On 2024/10/10 13:39, Jürgen Groß wrote:
> On 10.10.24 00:46, Stefano Stabellini wrote:
>> On Wed, 9 Oct 2024, Jan Beulich wrote:
>>> On 09.10.2024 08:20, 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, its
>>>> purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
>>>> and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
>>>> is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
>>>> find the implementation of pcistub_get_gsi_from_sbdf.
>>>>
>>>> But 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 and cover original commit's requirement, just remove
>>>> that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
>>>> of original commit is enough to meet the requirement.
>>>>
>>>> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
>>>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>>>
>>> This lacks a Reported-by:.
>>>
>>>> --- 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
>>>
>>> The report wasn't about a build problem, but a runtime one. Removing the
>>> dependency here doesn't change anything in the dependency of xen-privcmd
>>> on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
>>> exist.
>>>
>>> Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
>>> I guess is what Marek is using in his config. Both drivers are available
>>> in such a configuration, yet loading of xen-privcmd then requires to
>>> load xen-pciback first. And that latter load attempt will fail in a DomU.
>>> The two drivers simply may not have any dependency in either direction.
>>
>> The idea is that there should be no hard dependency on
>> pcistub_get_gsi_from_sbdf(). If it is available, the service will be
>> used, otherwise an error will be reported.
>>
>> The problem is that IS_REACHABLE is a compile-time check. What we need
>> is a runtime check instead. Maybe symbol_get or try_module_get ?
>
> This is a rather clumsy solution IMO.
>
> I'm seeing the following solutions:
>
> 1. Don't fail to to load the pciback driver in a DomU, but only disable
> any functionality.
>
> 2. Move the drivers/xen/xen-pciback/pci_stub.c functionality in a module
> of its own, allowing the privcmd driver to be loaded without needing
> the rest of pciback.
>
> 3. Add a hook to e.g. drivers/xen/pci.c instead for calling of
> pcistub_get_gsi_from_sbdf() directly. pciback could register the real
> call address. If pciback isn't loaded, the hook can return an error.
> This would remove the explicit dependency of privcmd on pciback.
>
> I'd prefer the 3rd variant.
Thanks, I will send changes in v2 soon.
And I prefer to add the hook in drivers/xen/acpi.c
>
>
> Juergen
>
--
Best regards,
Jiqian Chen.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-10 7:07 ` Chen, Jiqian
@ 2024-10-10 7:18 ` Jürgen Groß
0 siblings, 0 replies; 8+ messages in thread
From: Jürgen Groß @ 2024-10-10 7:18 UTC (permalink / raw)
To: Chen, Jiqian
Cc: Stefano Stabellini, Jan Beulich, xen-devel, linux-kernel,
Oleksandr Tyshchenko, Marek Marczykowski
[-- Attachment #1.1.1: Type: text/plain, Size: 3538 bytes --]
On 10.10.24 09:07, Chen, Jiqian wrote:
> On 2024/10/10 13:39, Jürgen Groß wrote:
>> On 10.10.24 00:46, Stefano Stabellini wrote:
>>> On Wed, 9 Oct 2024, Jan Beulich wrote:
>>>> On 09.10.2024 08:20, 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, its
>>>>> purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
>>>>> and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
>>>>> is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
>>>>> find the implementation of pcistub_get_gsi_from_sbdf.
>>>>>
>>>>> But 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 and cover original commit's requirement, just remove
>>>>> that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
>>>>> of original commit is enough to meet the requirement.
>>>>>
>>>>> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
>>>>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>>>>
>>>> This lacks a Reported-by:.
>>>>
>>>>> --- 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
>>>>
>>>> The report wasn't about a build problem, but a runtime one. Removing the
>>>> dependency here doesn't change anything in the dependency of xen-privcmd
>>>> on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
>>>> exist.
>>>>
>>>> Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
>>>> I guess is what Marek is using in his config. Both drivers are available
>>>> in such a configuration, yet loading of xen-privcmd then requires to
>>>> load xen-pciback first. And that latter load attempt will fail in a DomU.
>>>> The two drivers simply may not have any dependency in either direction.
>>>
>>> The idea is that there should be no hard dependency on
>>> pcistub_get_gsi_from_sbdf(). If it is available, the service will be
>>> used, otherwise an error will be reported.
>>>
>>> The problem is that IS_REACHABLE is a compile-time check. What we need
>>> is a runtime check instead. Maybe symbol_get or try_module_get ?
>>
>> This is a rather clumsy solution IMO.
>>
>> I'm seeing the following solutions:
>>
>> 1. Don't fail to to load the pciback driver in a DomU, but only disable
>> any functionality.
>>
>> 2. Move the drivers/xen/xen-pciback/pci_stub.c functionality in a module
>> of its own, allowing the privcmd driver to be loaded without needing
>> the rest of pciback.
>>
>> 3. Add a hook to e.g. drivers/xen/pci.c instead for calling of
>> pcistub_get_gsi_from_sbdf() directly. pciback could register the real
>> call address. If pciback isn't loaded, the hook can return an error.
>> This would remove the explicit dependency of privcmd on pciback.
>>
>> I'd prefer the 3rd variant.
> Thanks, I will send changes in v2 soon.
> And I prefer to add the hook in drivers/xen/acpi.c
Agreed.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition
2024-10-10 7:01 ` Jan Beulich
@ 2024-10-10 7:21 ` Jürgen Groß
0 siblings, 0 replies; 8+ messages in thread
From: Jürgen Groß @ 2024-10-10 7:21 UTC (permalink / raw)
To: Jan Beulich, Stefano Stabellini
Cc: Jiqian Chen, xen-devel, linux-kernel, Oleksandr Tyshchenko,
Marek Marczykowski
[-- Attachment #1.1.1: Type: text/plain, Size: 3747 bytes --]
On 10.10.24 09:01, Jan Beulich wrote:
> On 10.10.2024 07:39, Jürgen Groß wrote:
>> On 10.10.24 00:46, Stefano Stabellini wrote:
>>> On Wed, 9 Oct 2024, Jan Beulich wrote:
>>>> On 09.10.2024 08:20, 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, its
>>>>> purpose is to pass the combination of compilation that CONFIG_XEN_PRIVCMD=y
>>>>> and CONFIG_XEN_PCIDEV_BACKEND=m, because in that combination, xen-pciback
>>>>> is compiled as a module but xen-privcmd is built-in, so xen-privcmd can't
>>>>> find the implementation of pcistub_get_gsi_from_sbdf.
>>>>>
>>>>> But 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 and cover original commit's requirement, just remove
>>>>> that dependency, because the code "IS_REACHABLE(CONFIG_XEN_PCIDEV_BACKEND)"
>>>>> of original commit is enough to meet the requirement.
>>>>>
>>>>> Fixes: 2fae6bb7be32 ("xen/privcmd: Add new syscall to get gsi from dev")
>>>>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>>>>
>>>> This lacks a Reported-by:.
>>>>
>>>>> --- 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
>>>>
>>>> The report wasn't about a build problem, but a runtime one. Removing the
>>>> dependency here doesn't change anything in the dependency of xen-privcmd
>>>> on xen-pciback, as the use of pcistub_get_gsi_from_sbdf() continues to
>>>> exist.
>>>>
>>>> Consider the case of XEN_PCIDEV_BACKEND=m and XEN_PRIVCMD=m, which
>>>> I guess is what Marek is using in his config. Both drivers are available
>>>> in such a configuration, yet loading of xen-privcmd then requires to
>>>> load xen-pciback first. And that latter load attempt will fail in a DomU.
>>>> The two drivers simply may not have any dependency in either direction.
>>>
>>> The idea is that there should be no hard dependency on
>>> pcistub_get_gsi_from_sbdf(). If it is available, the service will be
>>> used, otherwise an error will be reported.
>>>
>>> The problem is that IS_REACHABLE is a compile-time check. What we need
>>> is a runtime check instead. Maybe symbol_get or try_module_get ?
>>
>> This is a rather clumsy solution IMO.
>>
>> I'm seeing the following solutions:
>>
>> 1. Don't fail to to load the pciback driver in a DomU, but only disable
>> any functionality.
>>
>> 2. Move the drivers/xen/xen-pciback/pci_stub.c functionality in a module
>> of its own, allowing the privcmd driver to be loaded without needing
>> the rest of pciback.
>>
>> 3. Add a hook to e.g. drivers/xen/pci.c instead for calling of
>> pcistub_get_gsi_from_sbdf() directly. pciback could register the real
>> call address. If pciback isn't loaded, the hook can return an error.
>> This would remove the explicit dependency of privcmd on pciback.
>>
>> I'd prefer the 3rd variant.
>
> Same here - order of preference backwards in the set presented above.
> In the meantime the original change may simply need reverting?
No, I don't think so. The breakage was introduced in 6.12 and we
are still in the 6.12 rc phase.
Standard Xen setups are running just fine, its just the Qubes use
case which is broken. Having a fix in 6.12 should be enough.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-10-10 7:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-09 6:20 [PATCH] xen: Remove config dependency in XEN_PRIVCMD definition Jiqian Chen
2024-10-09 7:05 ` Jan Beulich
2024-10-09 22:46 ` Stefano Stabellini
2024-10-10 5:39 ` Jürgen Groß
2024-10-10 7:01 ` Jan Beulich
2024-10-10 7:21 ` Jürgen Groß
2024-10-10 7:07 ` Chen, Jiqian
2024-10-10 7:18 ` Jürgen Groß
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®