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....@citrix.com>
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
