Re: [PATCH v5 7/9] xen/arm: Fix mapping for PCI bridge mmio region

[Julien Grall <julien@xxxxxxx>]

Hi Stewart,

On 04/10/2023 15:55, Stewart Hildebrand wrote:
> From: Rahul Singh <rahul.singh@xxxxxxx>
>
> Current code skip the mapping for PCI bridge MMIO region to dom0 when
> pci_passthrough_enabled flag is set. Mapping should be skip when
> has_vpci(d) is enabled for the domain, as we need to skip the mapping
> only when VPCI handler are registered for ECAM.
>
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
> ---
> v4->v5:
> * new patch
I am a bit lost. How is this a new patch but...

> * rebase on top of "dynamic node programming using overlay dtbo" series
> * replace !is_pci_passthrough_enabled() check with !IS_ENABLED(CONFIG_HAS_PCI)
>    instead of removing
... there is a changelog. Did you get the patch from somewhere?

> ---
>   xen/arch/arm/device.c       | 2 +-
>   xen/arch/arm/domain_build.c | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
> index 1f631d327441..4d69c298858d 100644
> --- a/xen/arch/arm/device.c
> +++ b/xen/arch/arm/device.c
> @@ -330,7 +330,7 @@ int handle_device(struct domain *d, struct dt_device_node 
> *dev, p2m_type_t p2mt,
>           .d = d,
>           .p2mt = p2mt,
>           .skip_mapping = !own_device ||
> -                        (is_pci_passthrough_enabled() &&
> +                        (has_vpci(d) &&
>                           (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
>           .iomem_ranges = iomem_ranges,
>           .irq_ranges = irq_ranges
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7da254709d17..2c55528a62d4 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1064,7 +1064,7 @@ static void __init assign_static_memory_11(struct domain 
> *d,
>   #endif
>   
>   /*
> - * When PCI passthrough is available we want to keep the
> + * When HAS_PCI is enabled we want to keep the
>    * "linux,pci-domain" in sync for every host bridge.
>    *
>    * Xen may not have a driver for all the host bridges. So we have
> @@ -1080,7 +1080,7 @@ static int __init handle_linux_pci_domain(struct 
> kernel_info *kinfo,
>       uint16_t segment;
>       int res;
>   
> -    if ( !is_pci_passthrough_enabled() )
> +    if ( !IS_ENABLED(CONFIG_HAS_PCI) )
I don't understand why this wants to be HAS_PCI rather than has_vcpi()? 
This also doesn't seem to be mentioned in the commit message.
>           return 0;
>   
>       if ( !dt_device_type_is_equal(node, "pci") )
Cheers,

--
Julien Grall