Re: [PATCH] xen/vpci: allow unaligned accesses by the hardware domain

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 06/08/2026 12:04 pm, Roger Pau Monne wrote:
> It's possible for domains to generate unaligned PCI config space accesses
> when using ECAM, and hence vPCI should support those at least for the
> hardware domain.  Such unaligned accesses to the PCI config space have been
> reported to come from ACPI logic.

By this, I presume you mean AML from the DSDT/SSDT ?

Misaligned ECAM accesses have undefined behaviour.  On a particular
platform this probably means implementation defined, and for an access
wholly within a single devices CFG window it might even work.  But,
misaligning allows you to have one access hitting two devices, and this
really can't be a good thing.

I presume "fix your BIOS" isn't on the cards, even if it would be for
the greater good?

>
> Relax the checking in vpci_access_allowed() to allow such accesses for the
> hardware domain, and fix the handling in pci_conf_{read,write}{16,32}() to
> fulfill them using MMCFG.
>
> MMCFG regions are identity exposed to the hardware domain, and hence such
> unaligned accesses can only come as a result of the host having MMCFG in the
> first place, as otherwise MMCFG won't be exposed to the hardware domain
> either.
>
> Reported-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Roger Pau Monné <roger@xenproject.org>
> ---
>  tools/include/xen-tools/common-macros.h | 2 ++
>  xen/arch/x86/x86_64/pci.c               | 8 ++++----
>  xen/drivers/vpci/vpci.c                 | 4 +++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
> index 8d33429103b9..6298141c3ca7 100644
> --- a/xen/arch/x86/x86_64/pci.c
> +++ b/xen/arch/x86/x86_64/pci.c
> @@ -26,7 +26,7 @@ uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
>  
>  uint16_t pci_conf_read16(pci_sbdf_t sbdf, unsigned int reg)
>  {
> -    if ( sbdf.seg || reg > 255 )
> +    if ( sbdf.seg || reg > 255 || !IS_ALIGNED(reg, 2) )
>      {
>          uint32_t value;
>  

Personally, I think this is making a bad situation worse.

Baring quirks (i.e. K10 era), there is never a case where we want to use
the IO Ports when we've got ECAM.  Furthermore, on AMD systems when
we're lacking ECAM we can still access Extended Config Space; something
which Xen currently gets wrong in several ways.

The IO ports require a global spinlock in Xen, and then a global
resource in hardware just to be able to translate the IO access back
into the ECAM access we passed on originally.  i.e. from a safety
non-interference point of view, you want to veto any use of the IO ports
by Xen.

Xen needs to use ECAM, and only fall back to IO Ports if we think there
isn't ECAM covering the target sbdf.  This will cause (mis)alignment to
get fixed automatically.  It will also be a substantial perf boost in
the general case; all the MSI/MSI-X editing we do (far too frequently)
is in Legacy Config Space just uses IO Ports.

~Andrew