[PATCH v1 1/2] x86/pci: Improve pci_mmcfg_{read,write} error handling

Teddy Astie posted 2 patches 1 month ago
[PATCH v1 1/2] x86/pci: Improve pci_mmcfg_{read,write} error handling
Posted by Teddy Astie 1 month ago
Return -ENODEV in case no mmcfg information is available instead of -EINVAL,
that is also returned when the parameters are incorrect.

It helps us distinguish between incorrect usage and no MMCFG support.

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
 xen/arch/x86/x86_64/mmconfig_64.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/x86_64/mmconfig_64.c b/xen/arch/x86/x86_64/mmconfig_64.c
index ffdc62700d..1a2803d2a3 100644
--- a/xen/arch/x86/x86_64/mmconfig_64.c
+++ b/xen/arch/x86/x86_64/mmconfig_64.c
@@ -60,15 +60,15 @@ int pci_mmcfg_read(unsigned int seg, unsigned int bus,
 {
     char __iomem *addr;
 
+    *value = -1;
+
     /* 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;
+    if ( unlikely((bus > 255) || (devfn > 255) || (reg > 4095)) )
         return -EINVAL;
-    }
 
     addr = pci_dev_base(seg, bus, devfn);
     if (!addr)
-        goto err;
+        return -ENODEV;
 
     switch (len) {
     case 1:
@@ -96,7 +96,7 @@ int pci_mmcfg_write(unsigned int seg, unsigned int bus,
 
     addr = pci_dev_base(seg, bus, devfn);
     if (!addr)
-        return -EINVAL;
+        return -ENODEV;
 
     switch (len) {
     case 1:
-- 
2.52.0



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH v1 1/2] x86/pci: Improve pci_mmcfg_{read,write} error handling
Posted by Jan Beulich 1 month ago
On 07.01.2026 17:54, Teddy Astie wrote:
> --- a/xen/arch/x86/x86_64/mmconfig_64.c
> +++ b/xen/arch/x86/x86_64/mmconfig_64.c
> @@ -60,15 +60,15 @@ int pci_mmcfg_read(unsigned int seg, unsigned int bus,
>  {
>      char __iomem *addr;
>  
> +    *value = -1;
> +
>      /* Why do we have this when nobody checks it. How about a BUG()!? -AK */

While it may be okay to retain this comment right here, the next patch the
latest should drop it (and its counterpart below). Question though is whether
earlier replies to the cover letter won't result in there not actually
appearing any users of the return values, in which case the change here
would be meaningless. Hence while in principle I'm okay with it, I won't ack
it for the time being.

Jan