Re: [PATCH v2 1/7] xen/arm: allow PCI host bridge to have private data

[Julien Grall]

Hi,

On 11/03/2025 15:05, Grygorii Strashko wrote:


On 11.03.25 12:24, Mykyta Poturai wrote:
From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>

Some of the PCI host bridges require private data. Create a generic
approach for that, so such bridges may request the private data to
be allocated during initialization.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
Signed-off-by: Mykyta Poturai <mykyta_potu...@epam.com>
---
v1->v2:
* no change
---
  xen/arch/arm/include/asm/pci.h      |  4 +++-
  xen/arch/arm/pci/pci-host-common.c  | 18 +++++++++++++++++-
  xen/arch/arm/pci/pci-host-generic.c |  2 +-
  xen/arch/arm/pci/pci-host-zynqmp.c  |  2 +-
  4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/ 
asm/pci.h
index 7f77226c9b..44344ac8c1 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -66,6 +66,7 @@ struct pci_host_bridge {
      uint16_t segment;                /* Segment number */
      struct pci_config_window* cfg;   /* Pointer to the bridge config 
window */
      const struct pci_ops *ops;
+    void *priv;                      /* Private data of the bridge. */
  };
  struct pci_ops {
@@ -95,7 +96,8 @@ struct pci_ecam_ops {
  extern const struct pci_ecam_ops pci_generic_ecam_ops;
  int pci_host_common_probe(struct dt_device_node *dev,
-                          const struct pci_ecam_ops *ops);
+                          const struct pci_ecam_ops *ops,
+                          size_t priv_sz);
  int pci_generic_config_read(struct pci_host_bridge *bridge, 
pci_sbdf_t sbdf,
                              uint32_t reg, uint32_t len, uint32_t 
*value);
  int pci_generic_config_write(struct pci_host_bridge *bridge, 
pci_sbdf_t sbdf,
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/ 
pci-host-common.c
index c0faf0f436..be7e6c3510 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -209,7 +209,8 @@ static int pci_bus_find_domain_nr(struct 
dt_device_node *dev)
  }
  int pci_host_common_probe(struct dt_device_node *dev,
-                          const struct pci_ecam_ops *ops)
+                          const struct pci_ecam_ops *ops,
+                          size_t priv_sz)
  {
      struct pci_host_bridge *bridge;
      struct pci_config_window *cfg;
@@ -241,11 +242,26 @@ int pci_host_common_probe(struct dt_device_node 
*dev,
          printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" 
property in DT\n");
          BUG();
      }
+
      bridge->segment = domain;
+
+    if ( priv_sz )
+    {
+        bridge->priv = xzalloc_bytes(priv_sz);
+        if ( !bridge->priv )
+        {
+            err = -ENOMEM;
+            goto err_priv;
+        }
+    }
+

I'd like to propose to move allocation into pci_alloc_host_bridge()
to keep mallocs in one place and do it earlier, before proceeding
with other initialization steps.

In both cases, we need to pass the size of the structure in parameter 
and the structure is not initialized until later. I would rather prefer
if the allocation is done by each PCI controller that needs it close to 
the initialization of "priv".

Cheers,

--
Julien Grall