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

["Roger Pau Monné" <roger@xenproject.org>]

On Thu, Aug 06, 2026 at 02:15:02PM +0200, Jan Beulich wrote:
> On 06.08.2026 13:48, Andrew Cooper wrote:
> > On 06/08/2026 12:04 pm, Roger Pau Monne wrote:
> >> --- 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.
> 
> While I agree, that's a bigger change which likely is going to be unsuitable
> for (immediate) backporting. Nevertheless the cross-device access that the
> changes as presented could cause needs preventing, by altering the checks at
> the start of pci_mmcfg_{read,write}().

Hm, OK. I was planning to be a bit mire strict in
vpci_access_allowed(), but I can also adjust pci_mmcfg_{read,write}().

Thanks, Roger.