Re: [PATCH v2 2/4] xen/pci: Introduce new CONFIG_PCI_ATS flag for PCI ATS functionality.

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 4 Nov 2020, Jan Beulich wrote:
> On 04.11.2020 16:43, Jan Beulich wrote:
> > On 03.11.2020 16:59, Rahul Singh wrote:
> >> --- a/xen/drivers/pci/Kconfig
> >> +++ b/xen/drivers/pci/Kconfig
> >> @@ -1,3 +1,12 @@
> >>  
> >>  config HAS_PCI
> >>    bool
> >> +
> >> +config PCI_ATS
> >> +  bool "PCI ATS support"
> >> +  default y
> >> +  depends on X86 && HAS_PCI
> >> +  ---help---
> >> +   Enable PCI Address Translation Services.
> >> +
> >> +   If unsure, say Y.
> > 
> > Support for "---help---" having gone away in Linux, I think we'd
> > better not add new instances. Also indentation of help content
> > typically is by a tab and two spaces. With these two adjusted
> > 
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Initially I wanted to merely reply indicating I'd be fine making
> these changes while committing, but there are two more things
> (and I withdraw my R-b): For one, isn't strict pci_dev's ats
> field now unused when !PCI_ATS? If so, if should get an #ifdef
> added. And then, what exactly is it in ats.c that's x86-specific?
> Shouldn't the whole file instead be moved one level up, and be
> usable by Arm right away?

If the issue is that ATS wouldn't work on ARM straight away, then I
think it would be best to make this a silent option like we did in patch
#1: if x86 && HAS_PCI -> automatically enable, otherwise disable. I
wouldn't move the code just yet, that's better done when we can actually
test it on ARM.