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 <jbeul...@suse.com>
> > Signed-off-by: Oleksii Kurochko <oleksii.kuroc...@gmail.com>
>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
> 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
