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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 16 Jun 2025, Jan Beulich wrote:
> On 16.06.2025 12:37, Stewart Hildebrand wrote:
> > On 6/16/25 02:42, Jan Beulich wrote:
> >> On 13.06.2025 17:17, Stewart Hildebrand wrote:
> >>> --- 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
> >>
> >> Seeing this, I like this as little as I liked ...
> >>
> >>> @@ -258,6 +260,12 @@ config PARTIAL_EMULATION
> >>>  
> >>>  source "arch/arm/firmware/Kconfig"
> >>>  
> >>> +config PCI_PASSTHROUGH
> >>> + bool "PCI passthrough" if EXPERT
> >>> + depends on ARM_64
> >>
> >> ... the form with the select-s put here. I'll (obviously) leave it to the
> >> Arm maintainers to judge, but my recommendation would be to simply drop
> >> this patch. As per the description it's merely "make it easier ...",
> >> which imo doesn't warrant such an abuse of HAS_*.
> > 
> > "easier" was a poor choice of word. "possible" is more accurate. This
> > patch addresses a real issue: currently the PCI and vPCI bits can't be
> > built for Arm, allowing build issues to go unnoticed. E.g. see
> > 4ce671963eb1 ("xen/arm: fix build with HAS_PCI").
> 
> Which gets us back to the question of whether to use "depends on
> HAS_PASSTHROUGH" (I think yes then) and where to put the remaining select
> (might then better move back to the new option).

In my opinion, HAS_ options should not be user-configurable but rather
properties of the architecture. Therefore, I would add HAS_PASSTHROUGH
to ARM_64 unconditionally. Then I would make PASSTHROUGH
user-configurable, but dependent on HAS_PASSTHROUGH.

In the rest of the code, we would update the checks to be based on
PASSTHROUGH instead of HAS_PASSTHROUGH.

That said, this patch is simpler while my suggestion is more invasive.
Also this patch is at v8 and we shouldn't keep increasing the scope of
the work for contributors. Finally, I am not certain all maintainers
would agree with my view I just outlined.

So based on the above, and based on the fact that we certainly need this
patch or something like it:

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>