Re: [PATCH edk2-platforms v2 18/18] ARM/JunoPkg: Add HDLCD platform library

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 10 January 2018 at 11:45, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
> Hi Ard,
>
>
> You wrote
>
> "Well, the fact that you need to override the BaseMemoryLib
> implementation is telling. This means the region is treated as memory
> in the code, but mapped with device semantics. So this is not about
> performance, but about the face that the UEFI spec stipulates that
> unaligned access to memory are allowed on AArch64, and so if the
> contents of these regions are dereferenced as memory, they should be
> mapped as memory. If they are mapped as device, you should only use
> MmioRead32/MmioWrite32 accessors."
>
>
> In that case could you comment on the following commits:
>
> "The BaseMemoryLib has switch to use BaseMemoryLibOptDxe at OPP, but the
> flash module is device attributes and have to be alignment accessed. so we
> change the flash related drivers to use generic BaseMemoryLib which is
> alignment access. "
>
>
> Hisilicon/D02: flash related drivers switch to use generic BaseMemoryLib:
> https://git.linaro.org/uefi/OpenPlatformPkg.git/commit/Platforms/Hisilicon/D02/Pv660D02.dsc?id=e3c520596d9ebdc525f284a9da95f080af815ec9
>
>
> Hisilicon/D03: flash related drivers switch to use generic BaseMemoryLib:
> https://git.linaro.org/uefi/OpenPlatformPkg.git/commit/Platforms/Hisilicon/D03/D03.dsc?id=337713801cceb684326bfde22975310ca1bd0cc0
>
>
> Hisilicon/D05: flash related drivers switch to use generic BaseMemoryLib:
> https://git.linaro.org/uefi/OpenPlatformPkg.git/commit/Platforms/Hisilicon/D05/D05.dsc?id=0749464022407f11c0c6a46cb8eb293fc74ef236
>

NOR flash can only be mapped as device memory, because it switches
between array mode and command mode, and in the latter mode, we
require device semantics.

Ideally, we should be able to switch between cacheable and device
mappings in this case (because we also switch between modes), but this
is not possible when running under the OS, and VariableRuntimeDxe is a
DXE_RUNTIME_DRIVER type module.

In general, memory should not be mapped with device semantics and vice
versa. NOR flash is arguably a corner case, especially because the
generic variable driver is shared with x86 which doesn't care about
any of this, and so the code is quite sloppy when it comes to
dereferencing pointers that may point outside of memory.