|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH 5/5] RFC: pci: Migrate pci_mmcfg_{read,write} to pci.c
Key parts of MMCFG access bits are in mmconfig_64.c (in particular
pci_mmcfg_{read,write}()) while PCI configuration primitives (used accross the
codebase) are in pci.c.
This leads to situations where the compiler cannot optimize the `switch (len)`
for MMCFG access for all pci_conf_{read,write}N(), because they are not from
the same file.
Move the pci_mmcfg_{read,write} in pci.c and hint the compiler to inline these
functions such that it's more likely that the compiler eliminates the
`switch (len)``.
Also take the opportunity to migrate to pci_sbdf_t to reduce the parameter count
and drop many parameter domains checks.
On GCC 16.1, this leads to codegen where pci_conf_{read,write}N() doesn't call
pci_mmcfg_{read,write}() anymore and directly perform the MMIO RW.
<pci_conf_read32>:
55 push %rbp
48 89 e5 mov %rsp,%rbp
53 push %rbx
89 f8 mov %edi,%eax
89 f3 mov %esi,%ebx
c1 ef 10 shr $0x10,%edi
81 fe ff 00 00 00 cmp $0xff,%esi
77 26 ja ffff82d040301fab <pci_conf_read32+0x3a>
85 ff test %edi,%edi
75 22 jne ffff82d040301fab <pci_conf_read32+0x3a>
0f b7 f8 movzwl %ax,%edi
c1 e7 08 shl $0x8,%edi
83 e3 fc and $0xfffffffc,%ebx
09 df or %ebx,%edi
81 cf 00 00 00 80 or $0x80000000,%edi
ba 04 00 00 00 mov $0x4,%edx
be 00 00 00 00 mov $0x0,%esi
e8 2a 1c 03 00 call ffff82d040333bd3 <pci_conf_read>
eb 22 jmp ffff82d040301fcd <pci_conf_read32+0x5c>
81 fb ff 0f 00 00 cmp $0xfff,%ebx
77 24 ja ffff82d040301fd7 <pci_conf_read32+0x66>
0f b6 d0 movzbl %al,%edx
0f b6 f4 movzbl %ah,%esi
0f b7 ff movzwl %di,%edi
e8 f5 fd ff ff call ffff82d040301db6 <pci_dev_base>
48 85 c0 test %rax,%rax
74 18 je ffff82d040301fde <pci_conf_read32+0x6d>
89 db mov %ebx,%ebx
48 01 d8 add %rbx,%rax
8b 00 mov (%rax),%eax
48 8b 5d f8 mov -0x8(%rbp),%rbx
c9 leave
e9 89 12 f0 ff jmp ffff82d040203260 <__x86_return_thunk>
b8 ff ff ff ff mov $0xffffffff,%eax
eb ef jmp ffff82d040301fcd <pci_conf_read32+0x5c>
b8 ff ff ff ff mov $0xffffffff,%eax
eb e8 jmp ffff82d040301fcd <pci_conf_read32+0x5c>
Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
---
xen/arch/x86/pv/ro-page-fault.c | 3 +-
xen/arch/x86/x86_64/mmconfig.h | 43 -----------
xen/arch/x86/x86_64/mmconfig_64.c | 106 +++++---------------------
xen/arch/x86/x86_64/pci.c | 122 ++++++++++++++++++++++++++++--
xen/include/xen/pci.h | 6 +-
5 files changed, 138 insertions(+), 142 deletions(-)
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index d89306d34f..647041560f 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -331,8 +331,7 @@ static int cf_check mmcfg_intercept_write(
offset &= 0xfff;
if ( pci_conf_write_intercept(mmio_ctxt->seg, mmio_ctxt->bdf,
offset, bytes, p_data) >= 0 )
- pci_mmcfg_write(mmio_ctxt->seg, PCI_BUS(mmio_ctxt->bdf),
- PCI_DEVFN(mmio_ctxt->bdf), offset, bytes,
+ pci_mmcfg_write(PCI_SBDF(mmio_ctxt->seg, mmio_ctxt->bdf), offset,
bytes,
*(uint32_t *)p_data);
return X86EMUL_OKAY;
diff --git a/xen/arch/x86/x86_64/mmconfig.h b/xen/arch/x86/x86_64/mmconfig.h
index 27c0ae5cb1..c1786a3ceb 100644
--- a/xen/arch/x86/x86_64/mmconfig.h
+++ b/xen/arch/x86/x86_64/mmconfig.h
@@ -23,49 +23,6 @@
extern unsigned int pci_probe;
-/*
- * AMD Fam10h CPUs are buggy, and cannot access MMIO config space
- * on their northbrige except through the * %eax register. As such, you MUST
- * NOT use normal IOMEM accesses, you need to only use the magic mmio-config
- * accessor functions.
- * In fact just use pci_config_*, nothing else please.
- */
-static inline unsigned char mmio_config_readb(void __iomem *pos)
-{
- u8 val;
- asm volatile("movb (%1),%%al" : "=a" (val) : "r" (pos));
- return val;
-}
-
-static inline unsigned short mmio_config_readw(void __iomem *pos)
-{
- u16 val;
- asm volatile("movw (%1),%%ax" : "=a" (val) : "r" (pos));
- return val;
-}
-
-static inline unsigned int mmio_config_readl(void __iomem *pos)
-{
- u32 val;
- asm volatile("movl (%1),%%eax" : "=a" (val) : "r" (pos));
- return val;
-}
-
-static inline void mmio_config_writeb(void __iomem *pos, u8 val)
-{
- asm volatile("movb %%al,(%1)" :: "a" (val), "r" (pos) : "memory");
-}
-
-static inline void mmio_config_writew(void __iomem *pos, u16 val)
-{
- asm volatile("movw %%ax,(%1)" :: "a" (val), "r" (pos) : "memory");
-}
-
-static inline void mmio_config_writel(void __iomem *pos, u32 val)
-{
- asm volatile("movl %%eax,(%1)" :: "a" (val), "r" (pos) : "memory");
-}
-
/* function prototypes */
struct acpi_table_header;
int cf_check acpi_parse_mcfg(struct acpi_table_header *header);
diff --git a/xen/arch/x86/x86_64/mmconfig_64.c
b/xen/arch/x86/x86_64/mmconfig_64.c
index 940cf6d747..483dff9c2c 100644
--- a/xen/arch/x86/x86_64/mmconfig_64.c
+++ b/xen/arch/x86/x86_64/mmconfig_64.c
@@ -26,93 +26,6 @@ struct mmcfg_virt {
static struct mmcfg_virt *pci_mmcfg_virt;
static unsigned int mmcfg_pci_segment_shift;
-static char __iomem *get_virt(unsigned int seg, unsigned int *bus)
-{
- struct acpi_mcfg_allocation *cfg;
- int cfg_num;
-
- for (cfg_num = 0; cfg_num < pci_mmcfg_config_num; cfg_num++) {
- cfg = pci_mmcfg_virt[cfg_num].cfg;
- if (cfg->pci_segment == seg &&
- (cfg->start_bus_number <= *bus) &&
- (cfg->end_bus_number >= *bus)) {
- *bus -= cfg->start_bus_number;
- return pci_mmcfg_virt[cfg_num].virt;
- }
- }
-
- /* Fall back to type 0 */
- return NULL;
-}
-
-static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned
int devfn)
-{
- char __iomem *addr;
-
- addr = get_virt(seg, &bus);
- if (!addr)
- return NULL;
- return addr + ((bus << 20) | (devfn << 12));
-}
-
-int pci_mmcfg_read(unsigned int seg, unsigned int bus,
- unsigned int devfn, int reg, int len, u32 *value)
-{
- char __iomem *addr;
-
- /* Why do we have this when nobody checks it. How about a BUG()!? -AK */
- if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095))) {
-err: *value = -1;
- return -EINVAL;
- }
-
- addr = pci_dev_base(seg, bus, devfn);
- if (!addr)
- goto err;
-
- switch (len) {
- case 1:
- *value = mmio_config_readb(addr + reg);
- break;
- case 2:
- *value = mmio_config_readw(addr + reg);
- break;
- case 4:
- *value = mmio_config_readl(addr + reg);
- break;
- }
-
- return 0;
-}
-
-int pci_mmcfg_write(unsigned int seg, unsigned int bus,
- unsigned int devfn, int reg, int len, u32 value)
-{
- char __iomem *addr;
-
- /* Why do we have this when nobody checks it. How about a BUG()!? -AK */
- if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
- return -EINVAL;
-
- addr = pci_dev_base(seg, bus, devfn);
- if (!addr)
- return -EINVAL;
-
- switch (len) {
- case 1:
- mmio_config_writeb(addr + reg, value);
- break;
- case 2:
- mmio_config_writew(addr + reg, value);
- break;
- case 4:
- mmio_config_writel(addr + reg, value);
- break;
- }
-
- return 0;
-}
-
static void __iomem *mcfg_ioremap(const struct acpi_mcfg_allocation *cfg,
unsigned long idx, unsigned int prot)
{
@@ -133,6 +46,25 @@ static void __iomem *mcfg_ioremap(const struct
acpi_mcfg_allocation *cfg,
return (void __iomem *) virt;
}
+char __iomem *pci_mmcfg_base(unsigned int seg, unsigned int *bus)
+{
+ struct acpi_mcfg_allocation *cfg;
+ int cfg_num;
+
+ for (cfg_num = 0; cfg_num < pci_mmcfg_config_num; cfg_num++) {
+ cfg = pci_mmcfg_virt[cfg_num].cfg;
+ if (cfg->pci_segment == seg &&
+ (cfg->start_bus_number <= *bus) &&
+ (cfg->end_bus_number >= *bus)) {
+ *bus -= cfg->start_bus_number;
+ return pci_mmcfg_virt[cfg_num].virt;
+ }
+ }
+
+ /* Fall back to type 0 */
+ return NULL;
+}
+
int pci_mmcfg_arch_enable(unsigned int idx)
{
const typeof(pci_mmcfg_config[0]) *cfg = pci_mmcfg_virt[idx].cfg;
diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index 8d33429103..c37e3edade 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -11,13 +11,123 @@
#define PCI_CONF_ADDRESS(sbdf, reg) \
(0x80000000U | ((sbdf).bdf << 8) | ((reg) & ~3))
+/*
+ * AMD Fam10h CPUs are buggy, and cannot access MMIO config space
+ * on their northbrige except through the * %eax register. As such, you MUST
+ * NOT use normal IOMEM accesses, you need to only use the magic mmio-config
+ * accessor functions.
+ * In fact just use pci_config_*, nothing else please.
+ */
+static inline unsigned char mmio_config_readb(void __iomem *pos)
+{
+ u8 val;
+ asm volatile("movb (%1),%%al" : "=a" (val) : "r" (pos));
+ return val;
+}
+
+static inline unsigned short mmio_config_readw(void __iomem *pos)
+{
+ u16 val;
+ asm volatile("movw (%1),%%ax" : "=a" (val) : "r" (pos));
+ return val;
+}
+
+static inline unsigned int mmio_config_readl(void __iomem *pos)
+{
+ u32 val;
+ asm volatile("movl (%1),%%eax" : "=a" (val) : "r" (pos));
+ return val;
+}
+
+static inline void mmio_config_writeb(void __iomem *pos, u8 val)
+{
+ asm volatile("movb %%al,(%1)" :: "a" (val), "r" (pos) : "memory");
+}
+
+static inline void mmio_config_writew(void __iomem *pos, u16 val)
+{
+ asm volatile("movw %%ax,(%1)" :: "a" (val), "r" (pos) : "memory");
+}
+
+static inline void mmio_config_writel(void __iomem *pos, u32 val)
+{
+ asm volatile("movl %%eax,(%1)" :: "a" (val), "r" (pos) : "memory");
+}
+
+static char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned
int devfn)
+{
+ char __iomem *addr;
+
+ addr = pci_mmcfg_base(seg, &bus);
+ if (!addr)
+ return NULL;
+ return addr + ((bus << 20) | (devfn << 12));
+}
+
+static inline
+int pci_mmcfg_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int len, u32
*value)
+{
+ char __iomem *addr;
+
+ /* Why do we have this when nobody checks it. How about a BUG()!? -AK */
+ if (unlikely(reg > 4095)) {
+err: *value = -1;
+ return -EINVAL;
+ }
+
+ addr = pci_dev_base(sbdf.seg, sbdf.bus, sbdf.devfn);
+ if (!addr)
+ goto err;
+
+ switch (len) {
+ case 1:
+ *value = mmio_config_readb(addr + reg);
+ break;
+ case 2:
+ *value = mmio_config_readw(addr + reg);
+ break;
+ case 4:
+ *value = mmio_config_readl(addr + reg);
+ break;
+ }
+
+ return 0;
+}
+
+inline int pci_mmcfg_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int
len, u32 value)
+{
+ char __iomem *addr;
+
+ /* Why do we have this when nobody checks it. How about a BUG()!? -AK */
+ if (unlikely(reg > 4095))
+ return -EINVAL;
+
+ addr = pci_dev_base(sbdf.seg, sbdf.bus, sbdf.devfn);
+ if (!addr)
+ return -EINVAL;
+
+ switch (len) {
+ case 1:
+ mmio_config_writeb(addr + reg, value);
+ break;
+ case 2:
+ mmio_config_writew(addr + reg, value);
+ break;
+ case 4:
+ mmio_config_writel(addr + reg, value);
+ break;
+ }
+
+ return 0;
+}
+
uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
{
uint32_t value;
if ( sbdf.seg || reg > 255 )
{
- pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value);
+ pci_mmcfg_read(sbdf, reg, 1, &value);
return value;
}
@@ -30,7 +140,7 @@ uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg)
{
uint32_t value;
- pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, &value);
+ pci_mmcfg_read(sbdf, reg, 2, &value);
return value;
}
@@ -43,7 +153,7 @@ uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
{
uint32_t value;
- pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, &value);
+ pci_mmcfg_read(sbdf, reg, 4, &value);
return value;
}
@@ -53,7 +163,7 @@ uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg, uint8_t data)
{
if ( sbdf.seg || reg > 255 )
- pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, data);
+ pci_mmcfg_write(sbdf, reg, 1, data);
else
pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1, data);
}
@@ -61,7 +171,7 @@ void pci_conf_write8(pci_sbdf_t sbdf, unsigned int reg,
uint8_t data)
void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg, uint16_t data)
{
if ( sbdf.seg || reg > 255 )
- pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, data);
+ pci_mmcfg_write(sbdf, reg, 2, data);
else
pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2, data);
}
@@ -69,7 +179,7 @@ void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg,
uint16_t data)
void pci_conf_write32(pci_sbdf_t sbdf, unsigned int reg, uint32_t data)
{
if ( sbdf.seg || reg > 255 )
- pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, data);
+ pci_mmcfg_write(sbdf, reg, 4, data);
else
pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), 0, 4, data);
}
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index d816dcad05..b3c91fea9c 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -258,10 +258,8 @@ void pci_conf_write16(pci_sbdf_t sbdf, unsigned int reg,
uint16_t data);
void pci_conf_write32(pci_sbdf_t sbdf, unsigned int reg, uint32_t data);
uint32_t pci_conf_read(uint32_t cf8, uint8_t offset, uint8_t bytes);
void pci_conf_write(uint32_t cf8, uint8_t offset, uint8_t bytes, uint32_t
data);
-int pci_mmcfg_read(unsigned int seg, unsigned int bus,
- unsigned int devfn, int reg, int len, u32 *value);
-int pci_mmcfg_write(unsigned int seg, unsigned int bus,
- unsigned int devfn, int reg, int len, u32 value);
+char *pci_mmcfg_base(unsigned int seg, unsigned int *bus);
+int pci_mmcfg_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int len, u32
value);
unsigned int pci_find_cap_offset(pci_sbdf_t sbdf, unsigned int cap);
unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, unsigned int pos,
const unsigned int caps[], unsigned int n,
--
2.52.0
--
Teddy Astie | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |