Re: [PATCH RFC] xen/pci: detect when BARs overlap RAM

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jan 26, 2022 at 04:27:04PM +0100, Jan Beulich wrote:
> On 26.01.2022 15:45, Andrew Cooper wrote:
> > On 26/01/2022 12:26, Roger Pau Monne wrote:
> >> One of the boxes where I was attempting to boot Xen in PVH dom0 mode
> >> has quirky firmware, as it will handover with a device with memory
> >> decoding enabled and a BAR of size 4K at address 0. Such BAR overlaps
> >> with a RAM range on the e820.
> >>
> >> This interacts badly with the dom0 PVH build, as BARs will be setup on
> >> the p2m before RAM, so if there's a BAR positioned over a RAM region
> >> it will trigger a domain crash when the dom0 builder attempts to
> >> populate that region with a regular RAM page.
> >>
> >> It's in general a very bad idea to have a BAR overlapping with a RAM
> >> region, so add some sanity checks for devices that are added with
> >> memory decoding enabled in order to assure that BARs are not placed on
> >> top of memory regions. If overlaps are detected just disable the
> >> memory decoding bit for the device and expect the hardware domain to
> >> properly position the BAR.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > 
> > I'm not sure this is a sensible approach.
> > 
> > A bar at 0 is utterly unusable, because it is outside of the host bridge
> > window.
> > 
> > The absence of any coherent GPA map for guests (needs fixing for oh so
> > many reasons) is a primary contributor to the problem, because Xen
> > *should* know where the guest's low and high MMIO windows are before
> > attempting to map BARs into the guest.
> 
> But this is all about Dom0, ...
> 
> > The proper fix is to teach Xen about GPA maps, and reject BAR insertion
> > outside of the respective bridge windows.
> 
> ... and hence this isn't about "insertion", but about what we find
> upon booting.
> 
> > An fix in the short term would be to disable the problematic BAR when
> > scanning the PCI bus to being with.
> 
> You can't disable an individual BAR (except for the ROM one), you need
> to disable all memory ones collectively (by disabling memory decode).
> Which is what Roger does.

I've thought about just disabling the ROM BAR if that's the only
conflicting one, but it was easier given the context to just disable
memory decoding globally like it's done for plain BARs. Didn't seem
worth the extra code to special case the ROM BAR disabling.

Thanks, Roger.