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

[Julien Grall <julien@xxxxxxx>]

Hi Oleksii,

On 01/11/2023 10: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
NIT: You seem to have a mix of ARM and Arm within the same commit 
message. I know that there are debate on which one to use (even though 
the latter is correct). I am ok if you want to either, but please at 
least be consistent :).
> to add <asm/pci.h> directly alongside <xen/pci.h>. Only to
> <arm/domain.c> <asm/pci.h> was added.
You are lucky here because there is only one place needed, so including 
both <xen/pci.h> and <asm/pci.h> is ok. However, I am not sure I like 
this approach as a general solution as it may require to include both 
the common and arch specific header in multiple places.
Anyway, we can discuss this in more details once we have an example.

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

> Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
>   xen/arch/arm/domain_build.c    |  1 +
>   xen/arch/arm/include/asm/pci.h |  7 -------
>   xen/arch/ppc/include/asm/pci.h |  7 -------
>   xen/include/xen/pci.h          | 11 +++++++++++
>   4 files changed, 12 insertions(+), 14 deletions(-)
>   delete mode 100644 xen/arch/ppc/include/asm/pci.h
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 49792dd590..2dd2926b41 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -23,6 +23,7 @@
>   #include <asm/kernel.h>
>   #include <asm/setup.h>
>   #include <asm/tee/tee.h>
> +#include <asm/pci.h>
>   #include <asm/platform.h>
>   #include <asm/psci.h>
>   #include <asm/setup.h>
> diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
> index 8cb46f6b71..7f77226c9b 100644
> --- a/xen/arch/arm/include/asm/pci.h
> +++ b/xen/arch/arm/include/asm/pci.h
> @@ -130,13 +130,6 @@ bool pci_check_bar(const struct pci_dev *pdev, mfn_t 
> start, mfn_t end);
>   
>   #else   /*!CONFIG_HAS_PCI*/
>   
> -struct arch_pci_dev { };
> -
> -static always_inline bool is_pci_passthrough_enabled(void)
> -{
> -    return false;
> -}
> -
>   struct pci_dev;
>   
>   static inline void arch_pci_init_pdev(struct pci_dev *pdev) {}
Some of the definion in !CONFIG_HAS_PCI seems unnecessary. This one is 
an example. I will have a look to clean them up after your patch is 
committed.
Cheers,

--
Julien Grall