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

[Laszlo Ersek]

On 01/17/17 10:06, Ard Biesheuvel wrote:
> 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)
> 

Fair enough, on both counts :)