On Mon, Nov 19, 2018 at 11:16:09AM -0800, Ard Biesheuvel wrote:
> On Mon, 19 Nov 2018 at 11:10, Leif Lindholm <leif.lindh...@linaro.org> wrote:
> >
> > On Fri, Nov 16, 2018 at 05:29:05PM -0800, Ard Biesheuvel wrote:
> > > On Fri, 16 Nov 2018 at 16:45, Ard Biesheuvel <ard.biesheu...@linaro.org>
> > > wrote:
> > > >
> > > > NorFlashQemuLib is one of the last remaining drivers in ArmVirtPkg
> > > > that are not based on the device tree received from QEMU.
> > > >
> > > > For ArmVirtQemu, this does not really matter, given that the NOR
> > > > flash banks are always the same: the PEI code is linked to execute
> > > > in place from flash bank #0, and the fixed varstore PCDs refer to
> > > > flash bank #1 directly.
> > > >
> > > > However, ArmVirtQemuKernel can execute at any offset, and flash bank
> > >
> > > #0 is configured as secure-only when running with support for EL3 enabled.
> >
> > Never gets old :)
>
> No this is entirely reasonable: it makes perfect sense for a NOR flash
> at address 0x0 to be secure only on a system that implements EL3,
> since mach-virt's default reset vector is 0x0.
*cough* sorry, I was referring to the leading #.
> > > > In this case, NorFlashQemuLib should not expose the first flash bank
> > > > at all.
> > > >
> > > > To prevent introducing too much internal knowledge about which flash
> > > > bank is accessible under which circumstances, let's switch to using
> > > > the DTB to decide which flash banks to expose to the NOR flash driver.
> >
> > Does this mean we as a side effect get rid of the limitation that the
> > pflash image needs to be exactly 64MB. Or does that require further
> > changes on the QEMU side?
>
> No that is a QEMU thing.
OK, thanks for confirming.
But this should mean that we don't need any changes on the guest sides
if/when we do fix this in QEMU?
/
Leif
> > If it does, please add a comment to the commit message.
> > Either way:
> > Reviewed-by: Leif Lindholm <leif.lindh...@linaro.org>
> >
>
> Thanks
>
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> > > > ---
> > > > ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c | 84
> > > > +++++++++++++++-----
> > > > ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf | 12 +++
> > > > 2 files changed, 75 insertions(+), 21 deletions(-)
> > > >
> > > > diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> > > > b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> > > > index e3bbae5b06c5..dc0a15e77170 100644
> > > > --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> > > > +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> > > > @@ -1,6 +1,6 @@
> > > > /** @file
> > > >
> > > > - Copyright (c) 2014, Linaro Ltd. All rights reserved.<BR>
> > > > + Copyright (c) 2014-2018, Linaro Ltd. All rights reserved.<BR>
> > > >
> > > > This program and the accompanying materials
> > > > are licensed and made available under the terms and conditions of the
> > > > BSD License
> > > > @@ -12,13 +12,16 @@
> > > >
> > > > **/
> > > >
> > > > +#include <Library/BaseLib.h>
> > > > +#include <Library/DebugLib.h>
> > > > #include <Library/NorFlashPlatformLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +
> > > > +#include <Protocol/FdtClient.h>
> > > >
> > > > #define QEMU_NOR_BLOCK_SIZE SIZE_256KB
> > > > -#define QEMU_NOR0_BASE 0x0
> > > > -#define QEMU_NOR0_SIZE SIZE_64MB
> > > > -#define QEMU_NOR1_BASE 0x04000000
> > > > -#define QEMU_NOR1_SIZE SIZE_64MB
> > > > +
> > > > +#define MAX_FLASH_BANKS 4
> > > >
> > > > EFI_STATUS
> > > > NorFlashPlatformInitialization (
> > > > @@ -28,21 +31,7 @@ NorFlashPlatformInitialization (
> > > > return EFI_SUCCESS;
> > > > }
> > > >
> > > > -NOR_FLASH_DESCRIPTION mNorFlashDevices[] = {
> > > > - {
> > > > - QEMU_NOR0_BASE,
> > > > - QEMU_NOR0_BASE,
> > > > - QEMU_NOR0_SIZE,
> > > > - QEMU_NOR_BLOCK_SIZE,
> > > > - {0xF9B94AE2, 0x8BA6, 0x409B, {0x9D, 0x56, 0xB9, 0xB4, 0x17, 0xF5,
> > > > 0x3C, 0xB3}}
> > > > - }, {
> > > > - QEMU_NOR1_BASE,
> > > > - QEMU_NOR1_BASE,
> > > > - QEMU_NOR1_SIZE,
> > > > - QEMU_NOR_BLOCK_SIZE,
> > > > - {0x8047DB4B, 0x7E9C, 0x4C0C, {0x8E, 0xBC, 0xDF, 0xBB, 0xAA, 0xCA,
> > > > 0xCE, 0x8F}}
> > > > - }
> > > > -};
> > > > +NOR_FLASH_DESCRIPTION mNorFlashDevices[MAX_FLASH_BANKS];
> > > >
> > > > EFI_STATUS
> > > > NorFlashPlatformGetDevices (
> > > > @@ -50,7 +39,60 @@ NorFlashPlatformGetDevices (
> > > > OUT UINT32 *Count
> > > > )
> > > > {
> > > > + FDT_CLIENT_PROTOCOL *FdtClient;
> > > > + INT32 Node;
> > > > + EFI_STATUS Status;
> > > > + EFI_STATUS FindNodeStatus;
> > > > + CONST UINT64 *Reg;
> > > > + UINT32 RegSize;
> > > > + CONST CHAR8 *NodeStatus;
> > > > + UINTN Num;
> > > > +
> > > > + Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
> > > > + (VOID **)&FdtClient);
> > > > + ASSERT_EFI_ERROR (Status);
> > > > +
> > > > + Num = 0;
> > > > + for (FindNodeStatus = FdtClient->FindCompatibleNode (FdtClient,
> > > > + "cfi-flash", &Node);
> > > > + !EFI_ERROR (FindNodeStatus);
> > > > + FindNodeStatus = FdtClient->FindNextCompatibleNode (FdtClient,
> > > > + "cfi-flash", Node, &Node)) {
> > > > +
> > > > + Status = FdtClient->GetNodeProperty (FdtClient, Node, "status",
> > > > + (CONST VOID **)&NodeStatus, NULL);
> > > > + if (!EFI_ERROR (Status) && AsciiStrnCmp (NodeStatus, "ok", 2) !=
> > > > 0) {
> > > > + continue;
> > > > + }
> > > > +
> > > > + Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg",
> > > > + (CONST VOID **)&Reg, &RegSize);
> > > > + if (EFI_ERROR (Status)) {
> > > > + DEBUG ((EFI_D_ERROR, "%a: GetNodeProperty () failed (Status ==
> > > > %r)\n",
> > > > + __FUNCTION__, Status));
> > > > + continue;
> > > > + }
> > > > +
> > > > + ASSERT ((RegSize % (2 * sizeof(UINT64))) == 0);
> > > > +
> > > > + while (RegSize > 0) {
> > > > + mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)SwapBytes64
> > > > (Reg[0]);
> > > > + mNorFlashDevices[Num].RegionBaseAddress = (UINTN)SwapBytes64
> > > > (Reg[0]);
> > > > + mNorFlashDevices[Num].Size = (UINTN)SwapBytes64
> > > > (Reg[1]);
> > > > + mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
> > > > +
> > > > + Num++;
> > > > + Reg += 2;
> > > > + RegSize -= 2 * sizeof(UINT64);
> > > > +
> > > > + if (Num >= MAX_FLASH_BANKS) {
> > > > + goto Finished;
> > > > + }
> > > > + }
> > > > + }
> > > > +
> > > > +Finished:
> > > > *NorFlashDescriptions = mNorFlashDevices;
> > > > - *Count = ARRAY_SIZE (mNorFlashDevices);
> > > > + *Count = Num;
> > > > return EFI_SUCCESS;
> > > > }
> > > > diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> > > > b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> > > > index 126d1671f544..d86ff36dbd58 100644
> > > > --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> > > > +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> > > > @@ -28,3 +28,15 @@
> > > > [Packages]
> > > > MdePkg/MdePkg.dec
> > > > ArmPlatformPkg/ArmPlatformPkg.dec
> > > > + ArmVirtPkg/ArmVirtPkg.dec
> > > > +
> > > > +[LibraryClasses]
> > > > + BaseLib
> > > > + DebugLib
> > > > + UefiBootServicesTableLib
> > > > +
> > > > +[Protocols]
> > > > + gFdtClientProtocolGuid ## CONSUMES
> > > > +
> > > > +[Depex]
> > > > + gFdtClientProtocolGuid
> > > > --
> > > > 2.17.1
> > > >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
