Re: [PATCH v3 13/17] xen/arm: Implement pci access functions

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Rahul,

> On 28 Sep 2021, at 19:18, Rahul Singh <rahul.singh@xxxxxxx> wrote:
> 
> Implement generic pci access functions to read/write the configuration
> space.
> 
> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> Change in v3:
> - Remove PRI_pci as not used.
> - Replace uint32_t sbdf to pci_sbdf_t sbdf to avoid typecast
> Change in v2: Fixed comments
> ---
> xen/arch/arm/pci/pci-access.c      | 57 ++++++++++++++++++++++++++++++
> xen/arch/arm/pci/pci-host-common.c | 19 ++++++++++
> xen/include/asm-arm/pci.h          |  1 +
> 3 files changed, 77 insertions(+)
> 
> diff --git a/xen/arch/arm/pci/pci-access.c b/xen/arch/arm/pci/pci-access.c
> index 3cd14a4b87..9f9aac43d7 100644
> --- a/xen/arch/arm/pci/pci-access.c
> +++ b/xen/arch/arm/pci/pci-access.c
> @@ -16,6 +16,7 @@
> #include <asm/io.h>
> 
> #define INVALID_VALUE (~0U)
> +#define PCI_ERR_VALUE(len) GENMASK(0, len * 8)
> 
> int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
>                             uint32_t reg, uint32_t len, uint32_t *value)
> @@ -72,6 +73,62 @@ int pci_generic_config_write(struct pci_host_bridge 
> *bridge, pci_sbdf_t sbdf,
>     return 0;
> }
> 
> +static uint32_t pci_config_read(pci_sbdf_t sbdf, unsigned int reg,
> +                                unsigned int len)
> +{
> +    uint32_t val = PCI_ERR_VALUE(len);
> +    struct pci_host_bridge *bridge = pci_find_host_bridge(sbdf.seg, 
> sbdf.bus);
> +
> +    if ( unlikely(!bridge) )
> +        return val;
> +
> +    if ( unlikely(!bridge->ops->read) )
> +        return val;
> +
> +    bridge->ops->read(bridge, sbdf, reg, len, &val);
> +
> +    return val;
> +}
> +
> +static void pci_config_write(pci_sbdf_t sbdf, unsigned int reg,
> +                             unsigned int len, uint32_t val)
> +{
> +    struct pci_host_bridge *bridge = pci_find_host_bridge(sbdf.seg, 
> sbdf.bus);
> +
> +    if ( unlikely(!bridge) )
> +        return;
> +
> +    if ( unlikely(!bridge->ops->write) )
> +        return;
> +
> +    bridge->ops->write(bridge, sbdf, reg, len, val);
> +}
> +
> +/*
> + * Wrappers for all PCI configuration access functions.
> + */
> +
> +#define PCI_OP_WRITE(size, type)                            \
> +    void pci_conf_write##size(pci_sbdf_t sbdf,              \
> +                              unsigned int reg, type val)   \
> +{                                                           \
> +    pci_config_write(sbdf, reg, size / 8, val);             \
> +}
> +
> +#define PCI_OP_READ(size, type)                             \
> +    type pci_conf_read##size(pci_sbdf_t sbdf,               \
> +                              unsigned int reg)             \
> +{                                                           \
> +    return pci_config_read(sbdf, reg, size / 8);            \
> +}
> +
> +PCI_OP_READ(8, uint8_t)
> +PCI_OP_READ(16, uint16_t)
> +PCI_OP_READ(32, uint32_t)
> +PCI_OP_WRITE(8, uint8_t)
> +PCI_OP_WRITE(16, uint16_t)
> +PCI_OP_WRITE(32, uint32_t)
> +
> /*
>  * Local variables:
>  * mode: C
> diff --git a/xen/arch/arm/pci/pci-host-common.c 
> b/xen/arch/arm/pci/pci-host-common.c
> index a08e06cea1..c5941b10e9 100644
> --- a/xen/arch/arm/pci/pci-host-common.c
> +++ b/xen/arch/arm/pci/pci-host-common.c
> @@ -236,6 +236,25 @@ err_exit:
>     return err;
> }
> 
> +/*
> + * This function will lookup an hostbridge based on the segment and bus
> + * number.
> + */
> +struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus)
> +{
> +    struct pci_host_bridge *bridge;
> +
> +    list_for_each_entry( bridge, &pci_host_bridges, node )
> +    {
> +        if ( bridge->segment != segment )
> +            continue;
> +        if ( (bus < bridge->cfg->busn_start) || (bus > 
> bridge->cfg->busn_end) )
> +            continue;
> +        return bridge;
> +    }
> +
> +    return NULL;
> +}
> /*
>  * Local variables:
>  * mode: C
> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> index bb7eda6705..49c9622902 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -81,6 +81,7 @@ int pci_generic_config_write(struct pci_host_bridge 
> *bridge, pci_sbdf_t sbdf,
>                              uint32_t reg, uint32_t len, uint32_t value);
> void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
>                                pci_sbdf_t sbdf, uint32_t where);
> +struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus);
> 
> static always_inline bool is_pci_passthrough_enabled(void)
> {
> -- 
> 2.17.1
>