Re: [PATCH for-4.17] x86/efi: don't translate EFI memory map MMIO regions to e820 RESERVED

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2022 16:17, Roger Pau Monne wrote:
> The EFI memory map contains two memory types (EfiMemoryMappedIO and
> EfiMemoryMappedIOPortSpace) used to describe IO memory areas of
> devices used by EFI.
> 
> The current parsing of the EFI memory map was translating
> EfiMemoryMappedIO and EfiMemoryMappedIOPortSpace to E820_RESERVED on
> x86.  This is an issue because device MMIO regions (BARs) should not
> be positioned on reserved regions.  Any BARs positioned on non-hole
> areas of the memory map will cause is_memory_hole() to return false,
> which would then cause memory decoding to be disabled for such device.
> This leads to EFI firmware malfunctions when using runtime services.
> 
> The system under which this was observed has:
> 
> EFI memory map:
> [...]
>  00000fd000000-00000fe7fffff type=11 attr=800000000000100d
> [...]
> 0000:00:1f.5 disabled: BAR [0xfe010, 0xfe010] overlaps with memory map
> 
> The device behind this BAR is:
> 
> 00:1f.5 Serial bus controller [0c80]: Intel Corporation Lewisburg SPI 
> Controller (rev 09)
>       Subsystem: Super Micro Computer Inc Device 091c
>       Flags: fast devsel
>       Memory at fe010000 (32-bit, non-prefetchable) [size=4K]well
> 
> For the record, the symptom observed in that machine was a hard freeze
> when attempting to set an EFI variable (XEN_EFI_set_variable).
> 
> Fix by not adding regions with type EfiMemoryMappedIO or
> EfiMemoryMappedIOPortSpace to the e820 memory map, that allows BARs to
> be positioned there.
> 
> Fixes: 75cc460a1b ('xen/pci: detect when BARs are not suitably positioned')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

In the best case this is moving us from one way of being wrong to another:
So far we wrongly include BARs in E820_RESERVED (_if_ they can be
legitimately covered by a EfiMemoryMappedIO region in the first place,
which I'm not sure is actually permitted - iirc just like E820_RESERVED
may not be used for BARs, this memory type also may not be), whereas with
your change we would no longer report non-BAR MMIO space (chipset specific
ranges for example) as reserved. In fact I think the example you provide
is at least partly due to bogus firmware behavior: The BAR is put in space
normally used for firmware specific memory (MMIO) ranges. I think firmware
should either assign the BAR differently or exclude the range from the
memory map.

I guess instead we want to handle the example you give by a firmware quirk
workaround.

> ---
> I don't understand the definition of EfiMemoryMappedIOPortSpace:
> 
> "System memory-mapped IO region that is used to translate memory
> cycles to IO cycles by the processor."

That's something (only?) IA-64 used, where kind of as a "replacement" for
x86 I/O port accesses equivalents thereof were provided (iirc 4 ports
per page) via MMIO accesses. It is this compatibility MMIO space which is
marked this way. Such ranges should never be seen on (current) x86.

> But given its name I would assume it's also likely used to mark ranges
> in use by PCI device BARs.
> 
> It would also be interesting to forward this information to dom0, so
> it doesn't attempt to move the BARs of this device(s) around, or else
> issues will arise.

None of this is device specific. There's simply (typically) one MMIO
range covering the entire 64k or I/O port space.

Jan