Hi
> On 24 Nov 2021, at 7:59 am, Oleksandr Andrushchenko <andr2...@gmail.com>
> wrote:
>
> From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
>
> If a PCI host bridge device is present in the device tree, but is
> disabled, then its PCI host bridge driver was not instantiated.
> This results in the failure of the pci_get_host_bridge_segment()
> and the following panic during Xen start:
>
> (XEN) Device tree generation failed (-22).
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Could not set up DOM0 guest OS
> (XEN) ****************************************
>
> Fix this by adding "linux,pci-domain" property for all device tree nodes
> which have "pci" device type, so we know which segments will be used by
> the guest for which bridges.
>
> Fixes: 4cfab4425d39 ("xen/arm: Add linux,pci-domain property for hwdom if not
> available.")
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
Reviewed-by: Rahul Singh <rahul.si...@arm.com>
Tested-by: Rahul Singh <rahul.si...@arm.com>
Regards,
Rahul
>
> ---
> Since v6:
> - use use_dt_domains in pci_get_new_domain_nr and return -1 if set
> - do not set "linux,pci-domain" if parent device is "pci"
> - move the code to a new helper handle_linux_pci_domain (Julien)
> New in v6
> ---
> xen/arch/arm/domain_build.c | 66 +++++++++++++++++++++++-------
> xen/arch/arm/pci/pci-host-common.c | 8 +++-
> xen/include/asm-arm/pci.h | 8 ++++
> 3 files changed, 66 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e76cee3caccf..c83c02ab8ac6 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -654,6 +654,55 @@ static void __init allocate_static_memory(struct domain
> *d,
> }
> #endif
>
> +/*
> + * When PCI passthrough is available 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
> + * to write an heuristic to detect whether a device node describes
> + * a host bridge.
> + *
> + * The current heuristic assumes that a device is a host bridge
> + * if the type is "pci" and then parent type is not "pci".
> + */
> +static int handle_linux_pci_domain(struct kernel_info *kinfo,
> + const struct dt_device_node *node)
> +{
> + uint16_t segment;
> + int res;
> +
> + if ( !is_pci_passthrough_enabled() )
> + return 0;
> +
> + if ( !dt_device_type_is_equal(node, "pci") )
> + return 0;
> +
> + if ( node->parent && dt_device_type_is_equal(node->parent, "pci") )
> + return 0;
> +
> + if ( dt_find_property(node, "linux,pci-domain", NULL) )
> + return 0;
> +
> + /* Allocate and create the linux,pci-domain */
> + res = pci_get_host_bridge_segment(node, &segment);
> + if ( res < 0 )
> + {
> + res = pci_get_new_domain_nr();
> + if ( res < 0 )
> + {
> + printk(XENLOG_DEBUG "Can't assign PCI segment to %s\n",
> + node->full_name);
> + return -FDT_ERR_NOTFOUND;
> + }
> +
> + segment = res;
> + printk(XENLOG_DEBUG "Assigned segment %d to %s\n",
> + segment, node->full_name);
> + }
> +
> + return fdt_property_cell(kinfo->fdt, "linux,pci-domain", segment);
> +}
> +
> static int __init write_properties(struct domain *d, struct kernel_info
> *kinfo,
> const struct dt_device_node *node)
> {
> @@ -755,21 +804,10 @@ static int __init write_properties(struct domain *d,
> struct kernel_info *kinfo,
> return res;
> }
>
> - if ( is_pci_passthrough_enabled() && dt_device_type_is_equal(node,
> "pci") )
> - {
> - if ( !dt_find_property(node, "linux,pci-domain", NULL) )
> - {
> - uint16_t segment;
> -
> - res = pci_get_host_bridge_segment(node, &segment);
> - if ( res < 0 )
> - return res;
> + res = handle_linux_pci_domain(kinfo, node);
>
> - res = fdt_property_cell(kinfo->fdt, "linux,pci-domain", segment);
> - if ( res )
> - return res;
> - }
> - }
> + if ( res )
> + return res;
>
> /*
> * Override the property "status" to disable the device when it's
> diff --git a/xen/arch/arm/pci/pci-host-common.c
> b/xen/arch/arm/pci/pci-host-common.c
> index cdeb3a283a77..9653b92b5b2e 100644
> --- a/xen/arch/arm/pci/pci-host-common.c
> +++ b/xen/arch/arm/pci/pci-host-common.c
> @@ -30,6 +30,8 @@ static LIST_HEAD(pci_host_bridges);
>
> static atomic_t domain_nr = ATOMIC_INIT(-1);
>
> +static int use_dt_domains = -1;
> +
> static inline void __iomem *pci_remap_cfgspace(paddr_t start, size_t len)
> {
> return ioremap_nocache(start, len);
> @@ -137,14 +139,16 @@ void pci_add_host_bridge(struct pci_host_bridge *bridge)
> list_add_tail(&bridge->node, &pci_host_bridges);
> }
>
> -static int pci_get_new_domain_nr(void)
> +int pci_get_new_domain_nr(void)
> {
> + if ( use_dt_domains )
> + return -1;
> +
> return atomic_inc_return(&domain_nr);
> }
>
> static int pci_bus_find_domain_nr(struct dt_device_node *dev)
> {
> - static int use_dt_domains = -1;
> int domain;
>
> domain = dt_get_pci_domain_nr(dev);
> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> index 81273e0d87ac..c20eba643d86 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -108,6 +108,8 @@ static always_inline bool is_pci_passthrough_enabled(void)
>
> void arch_pci_init_pdev(struct pci_dev *pdev);
>
> +int pci_get_new_domain_nr(void);
> +
> #else /*!CONFIG_HAS_PCI*/
>
> struct arch_pci_dev { };
> @@ -128,5 +130,11 @@ static inline int pci_get_host_bridge_segment(const
> struct dt_device_node *node,
> return -EINVAL;
> }
>
> +static inline int pci_get_new_domain_nr(void)
> +{
> + ASSERT_UNREACHABLE();
> + return -1;
> +}
> +
> #endif /*!CONFIG_HAS_PCI*/
> #endif /* __ARM_PCI_H__ */
> --
> 2.25.1
>
