Re: [PATCH for-4.17 5/6] pci: do not disable memory decoding for devices

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.10.2022 11:46, Roger Pau Monne wrote:
> Commit 75cc460a1b added checks to ensure the position of the BARs from
> PCI devices don't overlap with regions defined on the memory map.
> When there's a collision memory decoding is left disabled for the
> device, assuming that dom0 will reposition the BAR if necessary and
> enable memory decoding.
> 
> While this would be the case for devices being used by dom0, devices
> being used by the firmware itself that have no driver would usually be
> left with memory decoding disabled by dom0 if that's the state dom0
> found them in, and thus firmware trying to make use of them will not
> function correctly.
> 
> The initial intent of 75cc460a1b was to prevent vPCI from creating
> MMIO mappings on the dom0 p2m over regions that would otherwise
> already have mappings established.  It's my view now that we likely
> went too far with 75cc460a1b, and Xen disabling memory decoding of
> devices (as buggy as they might be) is harmful, and reduces the set of
> hardware on which Xen works.
> 
> This commits reverts most of 75cc460a1b, and instead adds checks to
> vPCI in order to prevent misplaced BARs from being added to the
> hardware domain p2m.

Which makes me wonder: How do things work then? Dom0 then still can't
access the BAR address range, can it? Plus with this adjustment, is
...

>  Signaling on whether BARs are mapped is tracked
> in the vpci structure, so that misplaced BARs are not mapped, and thus
> Xen won't attempt to unmap them when memory decoding is disabled.
> 
> This restores the behavior of Xen for PV dom0 to the state it was
> previous to 75cc460a1b, while also introducing a more contained fix
> for the vPCI BAR mapping issues.

... this (in particular "restores the behavior") a valid description
of this change?

> Fixes: 75cc460a1b ('xen/pci: detect when BARs are not suitably positioned')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> AT Citrix we have a system with a device with the following BARs:
> 
> BAR [0xfe010, 0xfe010] -> in a EfiMemoryMappedIO region
> BAR [0, 0x1fff] -> not positioned, outside host bridge window
> 
> And memory decoding enabled by the firmware.  With the current code
> (or any of the previous fix proposals), Xen would still disable memory
> decoding for the device, and the system will freeze when attempting to
> set EFI vars.

Isn't the latter (BAR at address 0) yet another problem? I have to admit
that I'm uncertain in how far it is a good idea to try to make Xen look
to work on such a system ...

Jan