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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.10.2022 10:37, Roger Pau Monné wrote:
> On Tue, Oct 04, 2022 at 06:11:50PM +0200, Jan Beulich wrote:
>> On 04.10.2022 17:36, Roger Pau Monne wrote:
>>> The EFI memory map contains two memory types (EfiMemoryMappedIO and
>>> EfiMemoryMappedIOPortSpace) used to describe IO memory areas used by
>>> EFI firmware.
>>>
>>> The current parsing of the EFI memory map is translating
>>> EfiMemoryMappedIO to E820_RESERVED on x86.  This causes issues on some
>>> boxes as the firmware is relying on using those regions to position
>>> the BARs of devices being used (possibly during runtime) for the
>>> firmware.
>>>
>>> Xen will disable memory decoding on any device that has BARs
>>> positioned over any regions on the e820 memory map, hence the firmware
>>> will malfunction after Xen turning off memory decoding for the
>>> device(s) that have BARs mapped in EfiMemoryMappedIO regions.
>>>
>>> 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 allowing BARs of PCI devices to be positioned over reserved
>>> memory regions, but print a warning message about such overlap.
>>
>> Somewhat hesitantly I might ack this change, but I'd like to give
>> others (Andrew in particular) some time to voice their views. As
>> said during the earlier discussion - I think we're relaxing things
>> too much by going this route.
> 
> Another option would be to explicitly check in efi_memmap for
> EfiMemoryMappedIO regions and BAR overlap and only allow those.  That
> would be more cumbersome code wise AFAICT.

Indeed there's a question of balancing well here, between two outcomes
neither of which is really desirable.

Jan