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.7828.1606316369719877158 for ; Wed, 25 Nov 2020 06:59:30 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@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 6401C106F; Wed, 25 Nov 2020 06:59:29 -0800 (PST) Received: from [192.168.1.81] (unknown [10.37.8.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5FBF13F70D; Wed, 25 Nov 2020 06:59:28 -0800 (PST) Subject: Re: [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space To: Vijayenthiran Subramaniam , devel@edk2.groups.io, leif@nuviainc.com Cc: thomas.abraham@arm.com, Aditya.Angadi@arm.com References: <1606310988-10772-1-git-send-email-vijayenthiran.subramaniam@arm.com> From: "Ard Biesheuvel" Message-ID: Date: Wed, 25 Nov 2020 15:59:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <1606310988-10772-1-git-send-email-vijayenthiran.subramaniam@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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? > 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/ArmPlatformPkg/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 = 0; Index < mNorFlashDeviceCount; Index++) { > // Check if this NOR Flash device contain the variable storage region > - ContainVariableStorage = > - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) && > - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size); > + > + if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) { > + ContainVariableStorage = > + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) && > + (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size); > + } else { > + ContainVariableStorage = > + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) && > + (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size); > + } > > Status = NorFlashCreateInstance ( > NorFlashDevices[Index].DeviceBaseAddress, > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/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/ArmPlatformPkg/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 = 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(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase)); > - ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase)); > + if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) { > + ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet64(PcdFlashNvStorageFtwWorkingBase64)); > + } else { > + ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase)); > + } > + > + if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) { > + ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet64(PcdFlashNvStorageFtwSpareBase64)); > + } else { > + ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase)); > + } > > // Check if the size of the area is at least one block size > ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0)); > @@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders ( > ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0)); > > // Ensure the Variable area Base Addresses are aligned on a block size boundaries > - ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0); > - ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0); > - ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0); > + if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) { > + ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media.BlockSize == 0); > + ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Media.BlockSize == 0); > + ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media.BlockSize == 0); > + } > + else { > + ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0); > + ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0); > + ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0); > + } > > // > // EFI_FIRMWARE_VOLUME_HEADER > @@ -736,10 +752,11 @@ NorFlashFvbInitialize ( > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > ASSERT_EFI_ERROR (Status); > > - mFlashNvStorageVariableBase = PcdGet32 (PcdFlashNvStorageVariableBase); > + mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ? > + FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase); > > // Set the index of the first LBA for the FVB > - Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize; > + Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize; > > BootMode = GetBootModeHob (); > if (BootMode == BOOT_WITH_DEFAULT_SETTINGS) { >