Re: [PATCH for-4.17] x86/pci: allow BARs to be positioned on e820 reserved regions

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

On Wed, Oct 05, 2022 at 05:42:08PM +0200, Jan Beulich wrote:
> On 05.10.2022 16:12, Roger Pau Monné wrote:
> > Hm, I have the following diff attached below which is more limited,
> > and not so bad I think.  Initially I wanted to introduce an
> > efi_all_mapped() helper, but that would require exposing EFI_MEMORY_TYPE
> > which is quite intrusive.
> > 
> > Let me know if you think the proposal below is better and I will
> > formally send a patch with it.
> 
> Hmm, personally I like this slightly better for, as you say, its more
> limited effect. Objectively, however, I'm still unconvinced of making
> this an EFI special case. How would non-EFI firmware go about
> communicating that it is going to access a device at runtime (which,
> as said before, I consider a no-go in the first place)? Likely by
> putting its BAR range(s) in an E820_RESERVED region.

I've never encountered a non-EFI firmware that attempts to access the
BAR of a device so far (or at least none that caused problems with
Xen I should say), so I would worry about such use-case when we find
one.

> Plus the MMIO range covered on the system in question is pretty large.
> That way we're still allowing pretty wide an abuse by the firmware.
> Furthermore the MCFG range would also be covered by an
> EfiMemoryMappedIO descriptor (in fact that's the only use of the type
> I had been aware of so far). IOW the change specifically permits an
> overlap of a BAR with an MCFG range.

Additionally I could check if the device overlaps with any known MCFG
regions in pci_check_bar(), again not sure if that's more fine grained
than needed.

> 
> Who's the manufacturer of the system?

It's from Supermicro: "Supermicro X11DPU BIOS"

> Or put in different words - how
> likely is it that we could first gain understanding on their
> intentions with this region? You did say the system hangs hard without
> some kind of workaround, but that doesn't clarify to me in how far a
> use of the device by the firmware was actually involved there.

It's a black box to me, so I have no idea what the firmware attempts
to do.  It looks like that BAR is used to store some information
related to the boot sequence, the hang happened when trying to create
a new variable bootnum using efibootmgr (that's just a guess
really).

> Have you considered other routes towards dealing with the issue? One
> approach coming to mind would build on top of what you've been doing
> so far (either variant): Besides avoiding the turning off of memory
> decode, also invoke pci_ro_device(), thus protecting it from having
> its BARs relocated, and also preventing any driver in Dom0 to gain
> control of it, thus avoiding two parties competing for the device.

IMO using pci_ro_device() would be going too far here - it's not Xen
the entity using the device, so Xen doesn't really know whether
there's already some interface between the firmware and the OS driver
(or just OS) in order to signal that the device is being used by the
firmware.

> Relocating the BAR outside of the reserved region would be nice, but
> will likely not resolve the hang.

Not an option for Xen due to not having enough information about the
memory layout, and it's risky IMO anyway.

> In any event I'm still hoping to have a 3rd view on the situation as a
> whole, irrespective of specific ideas towards possible workarounds ...

Thanks for the detailed feedback.

Roger.