Hi Stefano,
On 23/09/2021 07:23, Stefano Stabellini 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
+ 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
I really dislike them. This is kind of defeating the purpose of trying
to be more typesafe.
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
AFAIU, the problem is the prototype helpers are defined in asm/pci.h
which is included by xen/pci.h before defining sbdf_t. Is it correct?
If so there are two options:
1) define sbdf_t and then include asm/pci.h.
2) Name the union and then pre-declare it.
Option 1 is probably nicer is we have more types in the future that are
used by arch specific but defined in the common headers. We have a few
places that uses this approach.
Cheers,
--
Julien Grall