[PATCH] efi_loader: consider no-map property of reserved memory

[Heinrich Schuchardt]

On 5/14/21 9:26 PM, Atish Patra wrote:
> On Fri, May 14, 2021 at 12:59 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 5/13/21 11:53 PM, Mark Kettenis wrote:
>>>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> Date: Thu, 13 May 2021 21:41:40 +0200
>>>
>>> Hi Heinrich & Atish,
>>>
>>>> On 5/11/21 1:47 AM, Atish Patra wrote:
>>>>> On Wed, Sep 2, 2020 at 12:10 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>>
>>>>>> On 31.08.20 20:08, Atish Patra wrote:
>>>>>>> On Thu, Aug 27, 2020 at 9:16 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>>>>>
>>>>>>>> If a reserved memory node in the device tree has the property no-map,
>>>>>>>> remove it from the UEFI memory map provided by GetMemoryMap().
>>>>>>>>
>>>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>>
>>>>>> In the mail-thread starting a
>>>>>>
>>>>>> [PATCH 1/1] EBBR: GetMemoryMap(), handling of no-map DT property
>>>>>> https://lists.linaro.org/pipermail/boot-architecture/2020-September/001389.html
>>>>>>
>>>>>> the issue has been discussed. The conclusion was that we should not
>>>>>> change the current behavior.
>>>>>>
>>>>>
>>>>> Digging some old threads :)
>>>>>
>>>>> The merged version of the patch marks any reserved memory as
>>>>> EFI_BOOT_SERVICES_DATA.
>>>>> AFAIK, these memory regions will be available after ExitBootservice.
>>>>> If the operating system chooses to
>>>>> map these region and access them, it violates the purpose of the
>>>>> reserved memory region specified by the firmware.
>>>>>
>>>>> Did I miss something ?
>>>>
>>>> The no-map property is neither described in the EBBR nor in the
>>>> Devicetree specification v0.3 but only in Linux' reserved-memory.txt.
>>>
>>> It is described (quite explicitly) in the current devicetree
>>> specification draft.
>>>
>>>> In
>>>> https://lists.linaro.org/pipermail/boot-architecture/2020-September/001418.html
>>>> Ard requested that no-map memory shall be marked EfiReservedMemory
>>>> because Linux will not map EfiReservedMemory, see Linux function
>>>> is_usable_memory().
>>>>
>>>> All reserved memory that is not marked as no-map shall be mapped by
>>>> Linux. It may for instance serve as DMA pool or as video memory. Or it
>>>> may even be reusable. See reserved-memory.txt.
>>>>
>>>> Only drivers will access their own reserved memory if the region is not
>>>> marked as reusable.
>>>
>>> I suspect Atish is asking because of the issue I opened for opensbi:
>>>
>>>     https://github.com/riscv/opensbi/issues/210
>>>
>>> On many RISC-V platforms, opensbi runs in "machine mode" (somewhat
>>> equivalent to EL3 on ARMv8) and allocates some memory for itself.  It
>>> sets up protections for this memory such that "supervisor mode"
>>> (somewhat equivalent to EL1 on ARMv8) can't access this memory.
>>>
>>> Older versions of opensbi marked this memory area as "no-map",
>>> resulting in that memory area being marked as EfiReservedMemoryType,
>>> and evrything is fine.
>>>
>>> However, according to reserved-memory.txt, "no-map" means the the area
>>> isn't supposed to be mapped by the OS at all.  That is deemed
>>> undesirable since it prevents the OS from using a large 2MB or 1G page
>>> table entry to map the memory range that happens to include the memory
>>> reserved for opensbi.  That is sub-optimal as it means the OS has to
>>> allocated more levels of page tables for this memory block and has to
>>> use 4K pages which will increase TLB pressure.  So newer versions of
>>> opensbi dropped the "no-map" property resulting in the area being
>>> marked as EfiBootServicesData.  But that is obviously wrong since the
>>> OS can't actually access that memory range.
>>>
>>> Now somewhat by accident the Linux kernel didn't actually attempt to
>>> use this memory area so the issue wasn't noticed.  But we're working
>>> on OpenBSD/riscv64 and when I added code to use the EFI memory map the
>>> OpenBSD kernel tried to use that inaccessable memory, which obviously
>>> didn't end well.
>>
>> It is not by accident. The idea of reserved memory is that only a driver
>> responsible for this reserved memory is allowed to access it.
>>
>
> What is the expected behavior if the firmware is adding reserved
> memory ? In this case, OpenSBI firmware
> adds the reserved memory node for the regions protected by PMP.

The expected behavior is defined by the device-tree specification.

>
> Once the proper kernel boots, it is not aware of the reserved memory
> anymore as it follows the efi memory mappings only.

No, it still has the device-tree to know what is reserved.

> Any reserved regions without no-map property(by firmware) are marked
> as EfiBootServicesData.
> Linux kernel currently doesn't try to access those regions but can
> access anytime during the proper kernel boot. Isn't it ?

Should Linux ignore the device-tree you need to fix it there. This is
nothing OpenSBI or U-Boot should care about.

Do you have any evidence of such a bug?

Best regards

Heinrich

>
> The obvious solution is to mark those regions as no-map however that
> degrades the performance as described by Mark.
> +Alexandre Ghiti (who pointed out the huge page mapping issue)
>
> Is there a way to ensure that any reserved memory regions are mapped
> but not accessed ?
>
>> Why should OpenBSD ever try to access reserved memory if there is no
>> driver for it? Are you describing an OpenBSD bug?
>>
>> Take this example from the MacchiatoBin device tree:
>>
>>          reserved-memory {
>>                  #address-cells = <2>;
>>                  #size-cells = <2>;
>>                  ranges;
>>
>>                  psci-area at 4000000 {
>>                          reg = <0x0 0x4000000 0x0 0x200000>;
>>                          no-map;
>>                  };
>>          };
>>
>> Any access in the reserved area will lead to a Linux crash. But this
>> node is enough to keep Linux out.
>>
>>>
>>> I suspect differences between the ARMv8 and RISC-V architecture are at
>>> play here.  On ARMv8 "secure" memory areas like this are not supposed
>>> to be mapped as speculative access might trigger a fault.  But on
>>> RISC-V mapping a "protected" memory area is fine as long as you don't
>>> try to actually access it.
>>
>> It might have been wiser to use no-map in the example above to rule out
>> the possibility of speculative access.
>>
>>>
>>> So the question really is how opensbi can express that a certain
>>> memory area is reserved (and should end up as EfiReservedMemoryType in
>>> the EFI memory map) but that it is ok to map it.  Possible solution
>>> include:
>>
>> Just follow the draft device-tree spec.
>>
>> If a reserved memory area can be mapped don't use EfiReservedMemoryType.
>> Just mark the area as reserved in the device-tree and use
>> EfiBootServicesData.
>>
>> Make sure that OpenBSD follows the DT spec.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> * Change the interpretation of the "no-map" property for RISC-V such
>>>     that it is clear that the region in question can be mapped but can
>>>     not be accessed.  This only makes sense if the RISC-V architecture
>>>     guarantees that creating a mapping for physical memory will never
>>>     cause problems even if those areas can't be accessed.
>>>
>>> * Invent a new property that conveys the desired semantics.
>>>
>>> * Always flag memory ranges under /reserved-memory as
>>>     EfiReserbedMemoryType unless that memory range is marked as
>>>     "reusable".
>>>
>>> Cheers,
>>>
>>> Mark
>>>
>>
>
>