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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.10.2022 10:48, Roger Pau Monné wrote:
> 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.

Right - I don't expect there's a realistic chance of getting hold of a
firmware person of theirs.

>  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.

Fair point (I did think of that, but ended up not spelling it out).

Jan