Re: [PATCH v2] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery

["Laszlo Ersek" <lersek@redhat.com>]

On 06/24/20 13:15, Ard Biesheuvel wrote:
> Our UEFI guest firmware takes ownership of the emulated NOR flash in
> order to support the variable runtime services, and it does not expect
> the OS to interfere with the underlying storage directly. So disable
> the NOR flash DT nodes as we discover them, in a way similar to how we
> disable the PL031 RTC in the device tree when we attach our RTC runtime
> driver to it.
> 
> Note that this also hides the NOR flash bank that carries the UEFI
> executable code, but this is not intended to be updatable from inside
> the guest anyway, and if it was, we should use capsule update to do so.
> Also, the first -pflash argument that defines the backing for this flash
> bank is often issued with the 'readonly' modifier, in order to prevent
> any changes whatsoever to be made to the executable firmware image by
> the guest.
> 
> This issue has become relevant due to the following Linux changes,
> which enable the flash driver stack for default build configurations
> targetting arm64 and 32-bit ARM.
> 
> ce693fc2a877
> ("arm64: defconfig: Enable flash device drivers for QorIQ boards", 2020-03-16).
> 
> 5f068190cc10
> ("ARM: multi_v7_defconfig: Enable support for CFI NOR FLASH", 2019-04-03)
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> index 9b1d1184bdd3..425e36f2d127 100644
> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> @@ -86,6 +86,22 @@ NorFlashPlatformGetDevices (
>        mNorFlashDevices[Num].BlockSize         = QEMU_NOR_BLOCK_SIZE;
>        Num++;
>      }
> +
> +    //
> +    // UEFI takes ownership of the NOR flash, and exposes its functionality
> +    // through the UEFI Runtime Services GetVariable, SetVariable, etc. This
> +    // means we need to disable it in the device tree to prevent the OS from
> +    // attaching its device driver as well.
> +    // Note that this also hides other flash banks, but the only other flash
> +    // bank we expect to encounter is the one that carries the UEFI executable
> +    // code, which is not intended to be guest updatable, and is usually backed
> +    // in a readonly manner by QEMU anyway.
> +    //
> +    Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
> +                          "disabled", sizeof ("disabled"));
> +    if (EFI_ERROR (Status)) {
> +        DEBUG ((DEBUG_WARN, "Failed to set NOR flash status to 'disabled'\n"));
> +    }
>    }
>  
>    *NorFlashDescriptions = mNorFlashDevices;
> 

Thank you!
Laszlo