Hi Stefano,
> On 23 Sep 2021, at 3:23 am, Stefano Stabellini <sstabell...@kernel.org> wrote:
>
> Subject should have xen/arm
>
>
> On Wed, 22 Sep 2021, Rahul Singh wrote:
>> Implement generic pci access functions to read/write the configuration
>> space.
>>
>> Signed-off-by: Rahul Singh <rahul.si...@arm.com>
>> ---
>> Change in v2: Fixed comments
>> ---
>> xen/arch/arm/pci/pci-access.c | 58 ++++++++++++++++++++++++++++++
>> xen/arch/arm/pci/pci-host-common.c | 19 ++++++++++
>> xen/include/asm-arm/pci.h | 2 ++
>> 3 files changed, 79 insertions(+)
>>
>> diff --git a/xen/arch/arm/pci/pci-access.c b/xen/arch/arm/pci/pci-access.c
>> index 04fe9fbf92..45500cec2a 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, uint32_t sbdf,
>> uint32_t reg, uint32_t len, uint32_t *value)
>> @@ -72,6 +73,63 @@ int pci_generic_config_write(struct pci_host_bridge
>> *bridge, uint32_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);
>> +
>
> No blank line
Ack.
>
>
>> + 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, (uint32_t) sbdf.sbdf, reg, len, &val);
>
> The more I look at these casts the less I like them :-D
>
> One idea is to move the definition of pci_sbdf_t somewhere else
> entirely, for instance xen/include/xen/types.h, then we can use
> pci_sbdf_t everywhere
>
Ok. Let me try to get rid of this in the next version.
>
>> + 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, (uint32_t) sbdf.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 4beec14f2f..3bdc336268 100644
>> --- a/xen/arch/arm/pci/pci-host-common.c
>> +++ b/xen/arch/arm/pci/pci-host-common.c
>> @@ -243,6 +243,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->bus_start) || (bus > bridge->bus_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 4b32c7088a..5406daecda 100644
>> --- a/xen/include/asm-arm/pci.h
>> +++ b/xen/include/asm-arm/pci.h
>> @@ -18,6 +18,7 @@
>> #ifdef CONFIG_HAS_PCI
>>
>> #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
>> +#define PRI_pci "%04x:%02x:%02x.%u"
>
> This is added in this patch but it is unused here
Ack. I will remove this and will use %pp to print SBDF.
Regards,
Rahul
