From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web08.60085.1606744104390126937 for ; Mon, 30 Nov 2020 05:48:24 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: vijayenthiran.subramaniam@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 13FC6106F for ; Mon, 30 Nov 2020 05:48:24 -0800 (PST) Received: from mail-wm1-f52.google.com (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E2CBB3F918 for ; Mon, 30 Nov 2020 05:48:23 -0800 (PST) Received: by mail-wm1-f52.google.com with SMTP id v14so12946829wml.1 for ; Mon, 30 Nov 2020 05:48:23 -0800 (PST) X-Gm-Message-State: AOAM533FnSc5BmNOt3UA4SLlog+mo1bf4hYrQ1+OuP57/QwuFgAriLVt i/ZQFor+yytot+LM2Cg4kEXpmckP9O0hvLwj6sY= X-Google-Smtp-Source: ABdhPJyzU6Ivljo7Wcg6gLcoqG4UOpitVobU8v/9M6pPrs1TKJGbwTlFIAmkOrgfsK4A3ZPuSG1jOfxOvLr6mYCWDCM= X-Received: by 2002:a1c:f017:: with SMTP id a23mr23124437wmb.56.1606744102685; Mon, 30 Nov 2020 05:48:22 -0800 (PST) MIME-Version: 1.0 References: <1606310988-10772-1-git-send-email-vijayenthiran.subramaniam@arm.com> In-Reply-To: From: "Vijayenthiran Subramaniam" Date: Mon, 30 Nov 2020 13:48:10 +0000 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space To: devel@edk2.groups.io, ard.biesheuvel@arm.com Cc: Vijayenthiran Subramaniam , leif@nuviainc.com, thomas.abraham@arm.com, Aditya.Angadi@arm.com, sami.mujawar@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ard, On Wed, Nov 25, 2020 at 2:59 PM Ard Biesheuvel wro= te: > > On 11/25/20 2:29 PM, Vijayenthiran Subramaniam wrote: > > The existing NOR flash Dxe driver supports NOR flash devices connected > > in the 32-bit address space. Extend this driver to allow NOR flash > > devices connected to 64-bit address space to be usable as well. > > > > Why? There=E2=80=99s a derivative SGI/RD platform which has NOR flash connected = to the 64-bit MMIO space. We are planning to upstream support for the platform to edk2-platform=E2=80=99s SgiPkg in the upcoming days. As a preparation, this= patch updates the NorFlashDxe to support 64-bit addresses. Regards, Vijayenthiran > > > Signed-off-by: Vijayenthiran Subramaniam > > --- > > .../Drivers/NorFlashDxe/NorFlashDxe.c | 13 ++++++-- > > .../Drivers/NorFlashDxe/NorFlashDxe.inf | 3 ++ > > .../Drivers/NorFlashDxe/NorFlashFvbDxe.c | 31 ++++++++++++++----= - > > 3 files changed, 37 insertions(+), 10 deletions(-) > > > > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlat= formPkg/Drivers/NorFlashDxe/NorFlashDxe.c > > index d9e196cbf1..f3fbbafb7d 100644 > > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c > > @@ -1298,9 +1298,16 @@ NorFlashInitialise ( > > > > for (Index =3D 0; Index < mNorFlashDeviceCount; Index++) { > > // Check if this NOR Flash device contain the variable storage re= gion > > - ContainVariableStorage =3D > > - (NorFlashDevices[Index].RegionBaseAddress <=3D PcdGet32 (PcdFl= ashNvStorageVariableBase)) && > > - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlash= NvStorageVariableSize) <=3D NorFlashDevices[Index].RegionBaseAddress + NorF= lashDevices[Index].Size); > > + > > + if (PcdGet64 (PcdFlashNvStorageVariableBase64) !=3D 0) { > > + ContainVariableStorage =3D > > + (NorFlashDevices[Index].RegionBaseAddress <=3D PcdGet64 (PcdFla= shNvStorageVariableBase64)) && > > + (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlas= hNvStorageVariableSize) <=3D NorFlashDevices[Index].RegionBaseAddress + Nor= FlashDevices[Index].Size); > > + } else { > > + ContainVariableStorage =3D > > + (NorFlashDevices[Index].RegionBaseAddress <=3D PcdGet32 (PcdFla= shNvStorageVariableBase)) && > > + (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashN= vStorageVariableSize) <=3D NorFlashDevices[Index].RegionBaseAddress + NorFl= ashDevices[Index].Size); > > + } > > > > Status =3D NorFlashCreateInstance ( > > NorFlashDevices[Index].DeviceBaseAddress, > > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPl= atformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > > index a647c01687..b2a941d672 100644 > > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf > > @@ -54,10 +54,13 @@ > > gEfiDiskIoProtocolGuid > > > > [Pcd.common] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize > > + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64 > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase > > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize > > > > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmP= latformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > > index 9cdd85096a..ecbe009495 100644 > > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c > > @@ -58,8 +58,17 @@ InitializeFvAndVariableStoreHeaders ( > > Headers =3D AllocateZeroPool(HeadersLength); > > > > // FirmwareVolumeHeader->FvLength is declared to have the Variable = area AND the FTW working area AND the FTW Spare contiguous. > > - ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNv= StorageVariableSize) =3D=3D PcdGet32(PcdFlashNvStorageFtwWorkingBase)); > > - ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlash= NvStorageFtwWorkingSize) =3D=3D PcdGet32(PcdFlashNvStorageFtwSpareBase)); > > + if (PcdGet64 (PcdFlashNvStorageVariableBase64) !=3D 0) { > > + ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFla= shNvStorageVariableSize) =3D=3D PcdGet64(PcdFlashNvStorageFtwWorkingBase64)= ); > > + } else { > > + ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlash= NvStorageVariableSize) =3D=3D PcdGet32(PcdFlashNvStorageFtwWorkingBase)); > > + } > > + > > + if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) !=3D 0) { > > + ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdF= lashNvStorageFtwWorkingSize) =3D=3D PcdGet64(PcdFlashNvStorageFtwSpareBase6= 4)); > > + } else { > > + ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFla= shNvStorageFtwWorkingSize) =3D=3D PcdGet32(PcdFlashNvStorageFtwSpareBase)); > > + } > > > > // Check if the size of the area is at least one block size > > ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(P= cdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0)); > > @@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders ( > > ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(P= cdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0)); > > > > // Ensure the Variable area Base Addresses are aligned on a block s= ize boundaries > > - ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.Blo= ckSize =3D=3D 0); > > - ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.B= lockSize =3D=3D 0); > > - ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.Blo= ckSize =3D=3D 0); > > + if (PcdGet64 (PcdFlashNvStorageVariableBase64) !=3D 0) { > > + ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media= .BlockSize =3D=3D 0); > > + ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Med= ia.BlockSize =3D=3D 0); > > + ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media= .BlockSize =3D=3D 0); > > + } > > + else { > > + ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.B= lockSize =3D=3D 0); > > + ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media= .BlockSize =3D=3D 0); > > + ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.B= lockSize =3D=3D 0); > > + } > > > > // > > // EFI_FIRMWARE_VOLUME_HEADER > > @@ -736,10 +752,11 @@ NorFlashFvbInitialize ( > > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > > ASSERT_EFI_ERROR (Status); > > > > - mFlashNvStorageVariableBase =3D PcdGet32 (PcdFlashNvStorageVariableB= ase); > > + mFlashNvStorageVariableBase =3D (FixedPcdGet64 (PcdFlashNvStorageVar= iableBase64) !=3D 0) ? > > + FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (P= cdFlashNvStorageVariableBase); > > > > // Set the index of the first LBA for the FVB > > - Instance->StartLba =3D (PcdGet32 (PcdFlashNvStorageVariableBase) - I= nstance->RegionBaseAddress) / Instance->Media.BlockSize; > > + Instance->StartLba =3D (mFlashNvStorageVariableBase - Instance->Regi= onBaseAddress) / Instance->Media.BlockSize; > > > > BootMode =3D GetBootModeHob (); > > if (BootMode =3D=3D BOOT_WITH_DEFAULT_SETTINGS) { > >