From: Oleksandr Andrushchenko <oleksandr_andrushchenko@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_andrushchenko@epam.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@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;
+ }
+ }
+
pci_add_host_bridge(bridge);
return 0;
+err_priv:
+ xfree(bridge->priv);
+
err_exit:
xfree(bridge);
diff --git a/xen/arch/arm/pci/pci-host-generic.c b/xen/arch/arm/pci/pci-host-generic.c
index 46de6e43cc..cc4bc70684 100644
--- a/xen/arch/arm/pci/pci-host-generic.c
+++ b/xen/arch/arm/pci/pci-host-generic.c
@@ -29,7 +29,7 @@ static const struct dt_device_match __initconstrel gen_pci_dt_match[] =
static int __init pci_host_generic_probe(struct dt_device_node *dev,
const void *data)
{
- return pci_host_common_probe(dev, &pci_generic_ecam_ops);
+ return pci_host_common_probe(dev, &pci_generic_ecam_ops, 0);
}
DT_DEVICE_START(pci_gen, "PCI HOST GENERIC", DEVICE_PCI_HOSTBRIDGE)
diff --git a/xen/arch/arm/pci/pci-host-zynqmp.c b/xen/arch/arm/pci/pci-host-zynqmp.c
index 101edb8593..985a43c516 100644
--- a/xen/arch/arm/pci/pci-host-zynqmp.c
+++ b/xen/arch/arm/pci/pci-host-zynqmp.c
@@ -47,7 +47,7 @@ static const struct dt_device_match __initconstrel nwl_pcie_dt_match[] =
static int __init pci_host_generic_probe(struct dt_device_node *dev,
const void *data)
{
- return pci_host_common_probe(dev, &nwl_pcie_ops);
+ return pci_host_common_probe(dev, &nwl_pcie_ops, 0);
}
DT_DEVICE_START(pci_gen, "PCI HOST ZYNQMP", DEVICE_PCI_HOSTBRIDGE)
--
2.34.1
On 11.03.25 12:24, Mykyta Poturai wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@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_andrushchenko@epam.com>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@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.
Also the pci_alloc_host_bridge() could be made static, seems.
> pci_add_host_bridge(bridge);
>
> return 0;
>
> +err_priv:
> + xfree(bridge->priv);
> +
> err_exit:
> xfree(bridge);
>
[...]
-grygorii
Hi,
On 11/03/2025 15:05, Grygorii Strashko wrote:
>
>
> On 11.03.25 12:24, Mykyta Poturai wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@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_andrushchenko@epam.com>
>> Signed-off-by: Mykyta Poturai <mykyta_poturai@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
© 2016 - 2025 Red Hat, Inc.