[PATCH v1 2/2] x86/pci: Prefer using mmcfg for accessing configuration space

Teddy Astie posted 2 patches 1 month ago
[PATCH v1 2/2] x86/pci: Prefer using mmcfg for accessing configuration space
Posted by Teddy Astie 1 month ago
Current logic prefer using CFC/CF8 and fallbacks on mmcfg when accessing
>255 registers or a non-zero segment. Change the logic to always rely
on mmcfg unless it is not available to avoid locking on pci_config_lock
if possible.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
Are there x86 platforms where MMCFG is the only way to access PCI configuration space ?

 xen/arch/x86/x86_64/pci.c | 52 +++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index 8d33429103..3b3df8014d 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -14,62 +14,56 @@
 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
 {
     uint32_t value;
+    int ret = pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value);
 
-    if ( sbdf.seg || reg > 255 )
-    {
-        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value);
-        return value;
-    }
+    if ( unlikely(ret == -ENODEV) && !sbdf.seg && reg <= 255 )
+        return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1);
 
-    return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1);
+    return value;
 }
 
 uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg)
 {
-    if ( sbdf.seg || reg > 255 )
-    {
-        uint32_t value;
+    uint32_t value;
+    int ret = pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, &value);
 
-        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, &value);
-        return value;
-    }
+    if ( unlikely(ret == -ENODEV) && !sbdf.seg && reg <= 255 )
+        return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2);
 
-    return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2);
+    return value;
 }
 
 uint32_t pci_conf_read32(pci_sbdf_t sbdf, unsigned int reg)
 {
-    if ( sbdf.seg || reg > 255 )
-    {
-        uint32_t value;
+    uint32_t value;
+    int ret = pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, &value);
 
-        pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, &value);
-        return value;
-    }
+    if ( unlikely(ret == -ENODEV) && !sbdf.seg && reg <= 255 )
+        return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), 0, 4);
 
-    return pci_conf_read(PCI_CONF_ADDRESS(sbdf, reg), 0, 4);
+    return value;
 }
 
 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);
-    else
+    int ret = pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, data);
+
+    if ( unlikely(ret == -ENODEV) && !sbdf.seg && reg <= 255 )
         pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 3, 1, 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);
-    else
+    int ret = pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 2, data);
+
+    if ( unlikely(ret == -ENODEV) && !sbdf.seg && reg <= 255 )
         pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), reg & 2, 2, 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);
-    else
+    int ret = pci_mmcfg_write(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 4, data);
+
+    if ( unlikely(ret == -ENODEV) && !sbdf.seg && reg <= 255 )
         pci_conf_write(PCI_CONF_ADDRESS(sbdf, reg), 0, 4, data);
 }
-- 
2.52.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH v1 2/2] x86/pci: Prefer using mmcfg for accessing configuration space
Posted by Jan Beulich 1 month ago
On 07.01.2026 17:54, Teddy Astie wrote:
> Current logic prefer using CFC/CF8 and fallbacks on mmcfg when accessing
>> 255 registers or a non-zero segment. Change the logic to always rely

(Minor: Many mail programs, like mine, will mistake a > in the first column
as being reply quoting.)

> on mmcfg unless it is not available to avoid locking on pci_config_lock
> if possible.
> 
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> Are there x86 platforms where MMCFG is the only way to access PCI configuration space ?

If there were, how would that fact be communicated?

> --- a/xen/arch/x86/x86_64/pci.c
> +++ b/xen/arch/x86/x86_64/pci.c
> @@ -14,62 +14,56 @@
>  uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
>  {
>      uint32_t value;
> +    int ret = pci_mmcfg_read(sbdf.seg, sbdf.bus, sbdf.devfn, reg, 1, &value);

Along the lines of what in particular Roger said in reply to the cover letter,
I'm unconvinced we want to slow down (even if just minimally) things by
unconditionally making this call (and similar ones below).

Jan