Re: [Qemu-devel] [PATCH v2] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device

[Ard Biesheuvel]

On 17 January 2017 at 08:50, Laszlo Ersek <address@hidden> wrote:
> (my reply is no longer related to the patch, so maybe I shouldn't send
> it... I can't resist, sorry :))
>
> On 01/17/17 08:47, Ard Biesheuvel wrote:
>> On 16 January 2017 at 22:35, Laszlo Ersek <address@hidden> wrote:
>
>>> The UEFI memory map will reflect allocations from the GCD memory space,
>>> for the Reserved and MMIO types. See "Figure 2. GCD Memory State
>>> Transitions" in "7.2.2 GCD Memory Resources", Vol2 of the PI spec.
>>>
>>> See also "9.7.1 UEFI Boot Services Dependencies" in the same,
>>>
>>>   9.7.1.8 GetMemoryMap()
>>>
>>>   The GetMemoryMap() implementation must include into the UEFI memory
>>>   map all GCD map entries of types EfiGcdMemoryTypeReserved and
>>>   EfiPersistentMemory, and all GCD map entries of type
>>>   EfiGcdMemoryTypeMemoryMappedIo that have EFI_MEMORY_RUNTIME attribute
>>>   set.
>>>
>>> (Note that I wrote Reserved earlier, not MMIO.)
>>>
>>
>> What the PI spec stipulates is irrelevant: the contract between the
>> firmware and the OS is in the UEFI and ACPI specifications, not in the
>> PI spec.
>
> I disagree that what the PI spec stipulates is irrelevant. For platforms
> that implement both PI and UEFI, the PI spec expresses additional
> requirements for the UEFI implementation (in PI terminology). So what it
> says certainly matters for the ArmVirtQemu firmware specifically.
>
> End-to-end, if we want to achieve a particular result in a UEFI OS, we
> can certainly work towards that end in the PEI phase (or in the DXE
> phase, using the DXE services) in a specific firmware that aims to
> conform to both PI and UEFI. Because, the effects that those low-level
> operations will have on the UEFI level (and consequently, on the OS) are
> well defined in the PI spec.
>

PI spec should drive the implementation choices we make at the
ArmVirtQemu end, and the ACPI generation is tightly coupled with that,
so in that sense, I agree that the PI spec *is* relevant.

However, the purpose of the patch (which we are no longer discussing
:-)), is to ensure that QEMU + ArmVirtQemu adheres to the pertinent
contracts with the OS, and PI is not one of them.

>>
>>> However, you are right that *just* the UEFI memmap entry is not
>>> sufficient, according to the PCI firmware spec. (Regardless of the fact
>>> that in practice, just the memmap entry does keep Linux happy. Or is it
>>> about to change?)
>>>
>>
>> The kernel uses the UEFI memory map for two purposes:
>> - finding out where memory is, and which parts are usable (i.e., 
>> non-reserved)
>> - setting up page tables to allow UEFI runtime services calls, which
>> may include MMIO mappings
>>
>> This means that MMIO regions in the UEFI memory map are *not*
>> considered reservations. [...]
>
> Yes, I understand that. Now please understand that my suggestion was
> never to cover the MMCONFIG area with MMIO type memory; all along I've
> been saying "reserved memory".
>
> (Again, this is now independent of the patch.)
>

I know the various specs are vague and slightly contradictory, but I
would oppose to using EfiReservedMemory to describe an MMIO region,
given that the wording of the UEFI spec (which is authoritative imo)
suggests that the memory map should only describe memory (unless we
are dealing with MMIO regions that require a runtime mapping so that
the firmware can use the device while running under the OS)