Re: [PATCH] xen: avoid generation of stub <asm/pci.h> header

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-11-02 at 09:39 +0100, Jan Beulich wrote:
> On 01.11.2023 11:15, Oleksii Kurochko wrote:
> > Platforms which doesn't have HAS_PCI enabled it is needed to
> > have <asm/pci.h>, which contains only an empty definition of
> > struct arch_pci_dev ( except ARM, it introduces several
> > ARM-specific functions ).
> > 
> > Also, for architectures ( such as PPC or RISC-V ) on initial
> > stages of adding support, it is needed to generate <asm/pci.h>
> > for only define the mentioned above arch_pci_dev structure.
> > 
> > For the Arm-only stubs ( mentioned in <asm/pci.h> for disabled
> > HAS_PCI and ARM-specific) will be needed
> > to add <asm/pci.h> directly alongside <xen/pci.h>. Only to
> > <arm/domain.c> <asm/pci.h> was added.
> > 
> > Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> albeit with two remarks:

Thanks for the remarks.

> 
> > --- a/xen/include/xen/pci.h
> > +++ b/xen/include/xen/pci.h
> > @@ -68,7 +68,18 @@ typedef union {
> >      };
> >  } pci_sbdf_t;
> >  
> > +#ifdef CONFIG_HAS_PCI
> >  #include <asm/pci.h>
> > +#else
> 
> This minimal scope of the #ifdef will do for now, but will likely
> want
> extending down the road. Even what's visible in context is already an
> entity which should be entirely unused in the code base when
> !HAS_PCI.
> 
> > +struct arch_pci_dev { };
> > +
> > +static always_inline bool is_pci_passthrough_enabled(void)
> 
> Perhaps s/always_inline/inline/ as this is moved here. We really
> shouldn't
> use always_inline unless actually have a clear purpose.
Could it be fixed during the commit ( in case there won't be any other
critical comments about this patch )?

~ Oleksii