Re: [PATCH v2 1/2] x86/pci: prevent cross-device accesses in pci_mmcfg_{read,write}()

On 06.08.2026 17:26, Roger Pau Monne wrote:
> Introduce a specific check that prevents an accesses from spilling across
> two devices.
> 
> Signed-off-by: Roger Pau Monné <[email protected]>

Reviewed-by: Jan Beulich <[email protected]>
albeit with a remark:

> --- a/xen/arch/x86/x86_64/mmconfig_64.c
> +++ b/xen/arch/x86/x86_64/mmconfig_64.c
> @@ -61,7 +61,8 @@ int pci_mmcfg_read(unsigned int seg, unsigned int bus,
>      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))) {
> +    if (unlikely((bus > 255) || (devfn > 255) ||
> +                 (reg + len > PCI_CFG_SPACE_EXP_SIZE))) {
>  err:        *value = -1;
>          return -EINVAL;
>      }
> @@ -91,7 +92,8 @@ int pci_mmcfg_write(unsigned int seg, unsigned int bus,
>      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)))
> +    if (unlikely((bus > 255) || (devfn > 255) ||
> +                 (reg + len > PCI_CFG_SPACE_EXP_SIZE)))
>          return -EINVAL;
>  
>      addr = pci_dev_base(seg, bus, devfn);

In both cases the unlikely() uses won't have the intended effect, from all I
know. They would help as used here only if the compiler managed to fold all
three parts of the ||-expression into a single conditional branch, which I
don't think it would end up doing.

Jan