[v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

[PATCH v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Posted by Stewart Hildebrand 4 months, 3 weeks ago

From: Rahul Singh <rahul.singh@arm.com>

Setting CONFIG_PCI_PASSTHROUGH=y will enable PCI passthrough on ARM,
even though the feature is not yet complete in the current upstream
codebase. The purpose of this is to make it easier to enable the
necessary configs (HAS_PCI, HAS_VPCI) for testing and development of PCI
passthrough on ARM.

Since PCI passthrough on ARM is still work in progress at this time,
make it depend on EXPERT.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
This requires ("xen/arm: fix build with HAS_PCI") [4] to be merged first

The alpine-3.18-gcc-arm64-randconfig CI job should pick up the new
config option, but if we wanted it to be built more consistently in CI
we could add CONFIG_PCI_PASSTHROUGH=y to one of the arm jobs in
automation/gitlab-ci/build.yaml, e.g. alpine-3.18-gcc-debug-arm64.

v6->v7:
* rebase
* send patch separately from series [3]
* add HAS_VPCI_GUEST_SUPPORT since it's upstream now
* drop Julien's A-b due to changes and was given several releases ago

v5->v6:
* no change

v4->v5:
* no change

v3->v4:
* no change

v2->v3:
* add Julien's A-b

v1->v2:
* drop "ARM" naming since it is already in an ARM category
* depend on EXPERT instead of UNSUPPORTED

Changes from downstream to v1:
* depends on ARM_64 (Stefano)
* Don't select HAS_VPCI_GUEST_SUPPORT since this config option is not currently
  used in the upstream codebase. This will want to be re-added here once the
  vpci series [2] is merged.
* Don't select ARM_SMMU_V3 since this option can already be selected
  independently. While PCI passthrough on ARM depends on an SMMU, it does not
  depend on a particular version or variant of an SMMU.
* Don't select HAS_ITS since this option can already be selected independently.
  HAS_ITS may want to be added here once the MSI series [1] is merged.
* Don't select LATE_HWDOM since this option is unrelated to PCI passthrough.

[1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commits/poc/pci-passthrough
[2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00660.html
[3] https://lore.kernel.org/xen-devel/20231113222118.825758-1-stewart.hildebrand@amd.com/T/#t
[4] https://lore.kernel.org/xen-devel/20250610142300.197599-1-stewart.hildebrand@amd.com/T/#t

(cherry picked from commit 9a08f1f7ce28ec619640ba9ce11018bf443e9a0e from the
 downstream branch [1])
---
 xen/arch/arm/Kconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 3f25da3ca5fd..778feff16a55 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -258,6 +258,16 @@ config PARTIAL_EMULATION
 
 source "arch/arm/firmware/Kconfig"
 
+config PCI_PASSTHROUGH
+	bool "PCI passthrough" if EXPERT
+	depends on ARM_64
+	select HAS_PCI
+	select HAS_VPCI
+	select HAS_VPCI_GUEST_SUPPORT
+	default n
+	help
+	  This option enables PCI device passthrough
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"

base-commit: 86a12671c5d33063b6f958bdcca7c9d14cd5aac8
-- 
2.49.0

Re: [PATCH v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Posted by Jan Beulich 4 months, 3 weeks ago

On 10.06.2025 19:46, Stewart Hildebrand wrote:
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -258,6 +258,16 @@ config PARTIAL_EMULATION
>  
>  source "arch/arm/firmware/Kconfig"
>  
> +config PCI_PASSTHROUGH
> +	bool "PCI passthrough" if EXPERT
> +	depends on ARM_64
> +	select HAS_PCI
> +	select HAS_VPCI
> +	select HAS_VPCI_GUEST_SUPPORT

What about HAS_PASSTHROUGH? Seeing that being selected by MMU, what about
that connection here?

Selecting HAS_VPCI shouldn't really be necessary; HAS_VPCI_GUEST_SUPPORT
does that for you already. (I further wonder whether HAS_VPCI wouldn't
in turn better select HAS_PCI.)

> +	default n

This is unnecessary (redundant) and would hence better be omitted.

Jan

Re: [PATCH v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Posted by Stewart Hildebrand 4 months, 2 weeks ago

On 6/11/25 01:23, Jan Beulich wrote:
> On 10.06.2025 19:46, Stewart Hildebrand wrote:
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -258,6 +258,16 @@ config PARTIAL_EMULATION
>>  
>>  source "arch/arm/firmware/Kconfig"
>>  
>> +config PCI_PASSTHROUGH
>> +	bool "PCI passthrough" if EXPERT
>> +	depends on ARM_64
>> +	select HAS_PCI
>> +	select HAS_VPCI
>> +	select HAS_VPCI_GUEST_SUPPORT
> 
> What about HAS_PASSTHROUGH? Seeing that being selected by MMU, what about
> that connection here?

Hm, yes. I'm not sure if depends or select would be better?

	depends on ARM_64 && HAS_PASSTHROUGH

or

	select HAS_PASSTHROUGH

> Selecting HAS_VPCI shouldn't really be necessary; HAS_VPCI_GUEST_SUPPORT
> does that for you already. (I further wonder whether HAS_VPCI wouldn't
> in turn better select HAS_PCI.)

I'd be happy to make these changes.

>> +	default n
> 
> This is unnecessary (redundant) and would hence better be omitted.

OK, I'll remove it.

Re: [PATCH v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Posted by Jan Beulich 4 months, 2 weeks ago

On 12.06.2025 15:39, Stewart Hildebrand wrote:
> On 6/11/25 01:23, Jan Beulich wrote:
>> On 10.06.2025 19:46, Stewart Hildebrand wrote:
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -258,6 +258,16 @@ config PARTIAL_EMULATION
>>>  
>>>  source "arch/arm/firmware/Kconfig"
>>>  
>>> +config PCI_PASSTHROUGH
>>> +	bool "PCI passthrough" if EXPERT
>>> +	depends on ARM_64
>>> +	select HAS_PCI
>>> +	select HAS_VPCI
>>> +	select HAS_VPCI_GUEST_SUPPORT
>>
>> What about HAS_PASSTHROUGH? Seeing that being selected by MMU, what about
>> that connection here?
> 
> Hm, yes. I'm not sure if depends or select would be better?
> 
> 	depends on ARM_64 && HAS_PASSTHROUGH
> 
> or
> 
> 	select HAS_PASSTHROUGH

Actually, thinking of it: HAS_* shouldn't be selected based on user choices,
imo. Perhaps unless the HAS_* is a sub-feature of the dependent (as in e.g.
"select HAS_VPCI if HVM" as we have it for x86). In which case all the
select-s here might be wrong to use.

Jan

Jan

Re: [PATCH v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Posted by Stewart Hildebrand 4 months, 2 weeks ago

On 6/12/25 10:58, Jan Beulich wrote:
> On 12.06.2025 15:39, Stewart Hildebrand wrote:
>> On 6/11/25 01:23, Jan Beulich wrote:
>>> On 10.06.2025 19:46, Stewart Hildebrand wrote:
>>>> --- a/xen/arch/arm/Kconfig
>>>> +++ b/xen/arch/arm/Kconfig
>>>> @@ -258,6 +258,16 @@ config PARTIAL_EMULATION
>>>>  
>>>>  source "arch/arm/firmware/Kconfig"
>>>>  
>>>> +config PCI_PASSTHROUGH
>>>> +	bool "PCI passthrough" if EXPERT
>>>> +	depends on ARM_64
>>>> +	select HAS_PCI
>>>> +	select HAS_VPCI
>>>> +	select HAS_VPCI_GUEST_SUPPORT
>>>
>>> What about HAS_PASSTHROUGH? Seeing that being selected by MMU, what about
>>> that connection here?
>>
>> Hm, yes. I'm not sure if depends or select would be better?
>>
>> 	depends on ARM_64 && HAS_PASSTHROUGH
>>
>> or
>>
>> 	select HAS_PASSTHROUGH
> 
> Actually, thinking of it: HAS_* shouldn't be selected based on user choices,
> imo. Perhaps unless the HAS_* is a sub-feature of the dependent (as in e.g.
> "select HAS_VPCI if HVM" as we have it for x86). In which case all the
> select-s here might be wrong to use.

How about something like this:

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 3f25da3ca5fd..95a2cd3d006d 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -8,6 +8,8 @@ config ARM_64
 	depends on !ARM_32
 	select 64BIT
 	select HAS_FAST_MULTIPLY
+	select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
+	select HAS_PASSTHROUGH if PCI_PASSTHROUGH
 
 config ARM
 	def_bool y
@@ -258,6 +260,12 @@ config PARTIAL_EMULATION
 
 source "arch/arm/firmware/Kconfig"
 
+config PCI_PASSTHROUGH
+	bool "PCI passthrough" if EXPERT
+	depends on ARM_64
+	help
+	  This option enables PCI device passthrough
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index 20050e9bb8b3..0c376704ddc9 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -13,6 +13,7 @@ source "drivers/pci/Kconfig"
 source "drivers/video/Kconfig"
 
 config HAS_VPCI
+	select HAS_PCI
 	bool
 
 config HAS_VPCI_GUEST_SUPPORT

Re: [PATCH v7] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Posted by Jan Beulich 4 months, 2 weeks ago

On 12.06.2025 17:18, Stewart Hildebrand wrote:
> On 6/12/25 10:58, Jan Beulich wrote:
>> On 12.06.2025 15:39, Stewart Hildebrand wrote:
>>> On 6/11/25 01:23, Jan Beulich wrote:
>>>> On 10.06.2025 19:46, Stewart Hildebrand wrote:
>>>>> --- a/xen/arch/arm/Kconfig
>>>>> +++ b/xen/arch/arm/Kconfig
>>>>> @@ -258,6 +258,16 @@ config PARTIAL_EMULATION
>>>>>  
>>>>>  source "arch/arm/firmware/Kconfig"
>>>>>  
>>>>> +config PCI_PASSTHROUGH
>>>>> +	bool "PCI passthrough" if EXPERT
>>>>> +	depends on ARM_64
>>>>> +	select HAS_PCI
>>>>> +	select HAS_VPCI
>>>>> +	select HAS_VPCI_GUEST_SUPPORT
>>>>
>>>> What about HAS_PASSTHROUGH? Seeing that being selected by MMU, what about
>>>> that connection here?
>>>
>>> Hm, yes. I'm not sure if depends or select would be better?
>>>
>>> 	depends on ARM_64 && HAS_PASSTHROUGH
>>>
>>> or
>>>
>>> 	select HAS_PASSTHROUGH
>>
>> Actually, thinking of it: HAS_* shouldn't be selected based on user choices,
>> imo. Perhaps unless the HAS_* is a sub-feature of the dependent (as in e.g.
>> "select HAS_VPCI if HVM" as we have it for x86). In which case all the
>> select-s here might be wrong to use.
> 
> How about something like this:
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 3f25da3ca5fd..95a2cd3d006d 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -8,6 +8,8 @@ config ARM_64
>  	depends on !ARM_32
>  	select 64BIT
>  	select HAS_FAST_MULTIPLY
> +	select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH

This one may be okay, subject to the Arm maintainers' views.

> +	select HAS_PASSTHROUGH if PCI_PASSTHROUGH

At the first glance this one instead should be ...

> @@ -258,6 +260,12 @@ config PARTIAL_EMULATION
>  
>  source "arch/arm/firmware/Kconfig"
>  
> +config PCI_PASSTHROUGH
> +	bool "PCI passthrough" if EXPERT
> +	depends on ARM_64
> +	help
> +	  This option enables PCI device passthrough

... a "depends on" here, imo. Yet that would go against your intentions,
I think, so doing it the way you have it may be okay, subject to how you
justify it.

> --- a/xen/drivers/Kconfig
> +++ b/xen/drivers/Kconfig
> @@ -13,6 +13,7 @@ source "drivers/pci/Kconfig"
>  source "drivers/video/Kconfig"
>  
>  config HAS_VPCI
> +	select HAS_PCI
>  	bool

Please can the "bool" remain first?

Jan