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

[Atish Patra]

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.

Once the proper kernel boots, it is not aware of the reserved memory
anymore as it follows the efi memory mappings only.
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 ?

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


-- 
Regards,
Atish