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

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

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

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.

And of course the device must not be passed through to domUs, as
disabling memory decoding on it can lead to a host DoS.  Not that it
makes much sense to pass devices like the one above to guests.

It also makes me wonder whether this playing with the decoding bit
that we do in Xen is safe.  It might be more resilient to only disable
memory decoding when the BARs overlap a RAM region, as that would
indeed cause issues.

We should also consider moving away from the e820 and instead using
the EFI memory map for things like is_memory_hole(), but that would
involve translating e820 memory maps into EFI format, which might be
easier and more reliable than the other way around which we currently
do.
---
 xen/arch/x86/efi/efi-boot.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 836e8c2ba1..12ad94cd71 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -169,6 +169,14 @@ static void __init 
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
 
         switch ( desc->Type )
         {
+        case EfiMemoryMappedIO:
+        case EfiMemoryMappedIOPortSpace:
+            /*
+             * There no suitable e820 memory type to represent a MMIO area
+             * except a hole.
+             */
+            continue;
+
         case EfiBootServicesCode:
         case EfiBootServicesData:
             if ( map_bs )
-- 
2.37.3