Re: [PATCH v6 3/4] xen/arm: add support for PCI child bus

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 12 Jun 2025, Mykyta Poturai wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> PCI host bridges often have different ways to access the root and child
> bus configuration spaces. One of the examples is Designware's host bridge
> and its multiple clones [1].
> 
> Linux kernel implements this by instantiating a child bus when device
> drivers provide not only the usual pci_ops to access ECAM space (this is
> the case for the generic host bridge), but also means to access the child
> bus which has a dedicated configuration space and own implementation for
> accessing the bus, e.g. child_ops.
> 
> For Xen it is not feasible to fully implement PCI bus infrastructure as
> Linux kernel does, but still child bus can be supported.
> 
> Add support for the PCI child bus which includes the following changes:
> - introduce bus mapping functions depending on SBDF
> - assign bus start and end for the child bus and re-configure the same for
>   the parent (root) bus
> - make pci_find_host_bridge be aware of multiple busses behind the same bridge
> - update pci_host_bridge_mappings, so it also doesn't map to guest the memory
>   spaces belonging to the child bus
> - make pci_host_common_probe accept one more pci_ops structure for the child 
> bus
> - install MMIO handlers for the child bus for hardware domain
> - re-work vpci_mmio_{write|read} with parent and child approach in mind
> 
> [1] https://elixir.bootlin.com/linux/v5.15/source/drivers/pci/controller/dwc
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>

[...]


> diff --git a/xen/arch/arm/pci/pci-host-common.c 
> b/xen/arch/arm/pci/pci-host-common.c
> index 53953d4895..487c545f3a 100644
> --- a/xen/arch/arm/pci/pci-host-common.c
> +++ b/xen/arch/arm/pci/pci-host-common.c
> @@ -57,17 +57,12 @@ static void pci_ecam_free(struct pci_config_window *cfg)
>      xfree(cfg);
>  }
>  
> -static struct pci_config_window * __init
> -gen_pci_init(struct dt_device_node *dev, const struct pci_ecam_ops *ops)
> +void __init pci_generic_init_bus_range(struct dt_device_node *dev,
> +                                       struct pci_host_bridge *bridge,
> +                                       struct pci_config_window *cfg)
>  {
> -    int err, cfg_reg_idx;
>      u32 bus_range[2];
> -    paddr_t addr, size;
> -    struct pci_config_window *cfg;
> -
> -    cfg = xzalloc(struct pci_config_window);
> -    if ( !cfg )
> -        return NULL;
> +    int err;
>  
>      err = dt_property_read_u32_array(dev, "bus-range", bus_range,
>                                       ARRAY_SIZE(bus_range));
> @@ -82,6 +77,35 @@ gen_pci_init(struct dt_device_node *dev, const struct 
> pci_ecam_ops *ops)
>          if ( cfg->busn_end > cfg->busn_start + 0xff )
>              cfg->busn_end = cfg->busn_start + 0xff;
>      }
> +}
> +
> +void __init pci_generic_init_bus_range_child(struct dt_device_node *dev,
> +                                             struct pci_host_bridge *bridge,
> +                                             struct pci_config_window *cfg)
> +{
> +    cfg->busn_start = bridge->cfg->busn_start + 1;
> +    cfg->busn_end = bridge->cfg->busn_end;
> +    bridge->cfg->busn_end = bridge->cfg->busn_start;

I think Stewart meant that pci_generic_init_bus_range_child should be
renamed pci_designware_init_bus_range_child and moved to
pci-designware.c.

However, given that we don't have a second user of this and I am not
certain how generic (or not) this bus_range_child function is, also
considering the other comments have been addressed:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

It can be moved later as appropriate if this turns out not to be generic
once we have a second user.


> +    printk(XENLOG_INFO "Root bus end updated: [bus %x-%x]\n",
> +           bridge->cfg->busn_start, bridge->cfg->busn_end);
> +}