Hi Ilias, Please find my response inline marked [SAMI].
Regards, Sami Mujawar -----Original Message----- From: Ilias Apalodimas <ilias.apalodi...@linaro.org> Sent: 29 January 2021 08:02 AM To: Sami Mujawar <sami.muja...@arm.com> Cc: Sughosh Ganu <sughosh.g...@linaro.org>; devel@edk2.groups.io; Ard Biesheuvel <ard.biesheu...@arm.com>; Leif Lindholm <l...@nuviainc.com>; Sahil Malhotra <sahil.malho...@linaro.org> Subject: Re: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver Hi Sami, Thanks for taking the time On Wed, 27 Jan 2021 at 19:10, Sami Mujawar <sami.muja...@arm.com> wrote: > > Hi Sughosh, > > There are some edk2 coding standard incompatibilities (like space between > function name and opening bracket, function parameter alignment etc.) and > documentation for some functions is missing in the patch. Can you fix these, > please? > Please also run the ECC tool in Drivers/OpTeeRpmb folder and fix any reported > issues. The ECC errors 10002, 10006 and 10022 can be ignored for now. > > Other than that, please find my response inline marked [SAMI]. > > Regards, > > Sami Mujawar > > -----Original Message----- > From: Sughosh Ganu <sughosh.g...@linaro.org> > Sent: 16 December 2020 11:09 AM > To: devel@edk2.groups.io > Cc: Sami Mujawar <sami.muja...@arm.com>; Ard Biesheuvel > <ard.biesheu...@arm.com>; Leif Lindholm <l...@nuviainc.com>; Sahil Malhotra > <sahil.malho...@linaro.org>; Ilias Apalodimas <ilias.apalodi...@linaro.org> > Subject: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE > backed RPMB driver > > From: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > A following patch is adding support for building StMM in order to run it > from OP-TEE. > OP-TEE in combination with a NS-world supplicant can use the RPMB > partition of an eMMC to store EFI variables. The supplicant > functionality is currently available in U-Boot only but can be ported > into EDK2. Assuming similar functionality is added in EDK2, this will > allow any hardware with an RPMB partition to store EFI variables > securely. > > So let's add a driver that enables access of the RPMB partition through > OP-TEE. Since the upper layers expect a byte addressable interface, > the driver allocates memory and patches the PCDs, while syncing the > memory/hardware on read/write callbacks. > > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> [...] > --- /dev/null > +++ b/Drivers/OpTeeRpmb/OpTeeRpmbFvb.h > @@ -0,0 +1,35 @@ > +/** @file > + > + Copyright (c) 2020, Linaro Ltd. All rights reserved.<BR> > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef __OPTEE_RPMB_FV_ > +#define __OPTEE_RPMB_FV_ > [SAMI] This does not follow the edk2 coding standard. See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-5-all-include-file-contents-must-be-protected-by-a-include-guard > [/SAMI] Ok > + > +/* SVC Args */ > +#define SP_SVC_RPMB_READ 0xC4000066 > +#define SP_SVC_RPMB_WRITE 0xC4000067 > [SAMI] Is there a specification reference for this? If so, please add to the > file header. > See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/52_spacing#5-2-3-1-every-new-file-shall-begin-with-a-file-header-comment-block. > [/SAMI] No there isn't. Since those are FFA calls it's something we define internally in OP-TEE. We had a discussion with Arm about standardizing this in the future, but it can wait until we get full FFA and SP support in OP-TEE. The rpmb 'driver' is essentially a bunch of OP-TEE SVC calls that end up using the OP-TEE APIs for accessing an RPMB partition. I could add a reference to the OP-TEE code if that makes sense? [SAMI] I think that may be a good reference point for now. [/SAMI] > + > +#define FILENAME "EFI_VARS" > + > +#define FLASH_SIGNATURE SIGNATURE_32('r', 'p', 'm', 'b') > +#define INSTANCE_FROM_FVB_THIS(a) CR(a, MEM_INSTANCE, FvbProtocol, \ > + FLASH_SIGNATURE) > + > +typedef struct _MEM_INSTANCE MEM_INSTANCE; > +typedef EFI_STATUS (*MEM_INITIALIZE) (MEM_INSTANCE* Instance); > +struct _MEM_INSTANCE > +{ > + UINT32 Signature; > + MEM_INITIALIZE Initialize; > + BOOLEAN Initialized; > + EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL FvbProtocol; > + EFI_HANDLE Handle; > + EFI_PHYSICAL_ADDRESS MemBaseAddress; > + UINT16 BlockSize; > + UINT16 NBlocks; > +}; > [SAMI] Please add documentation for structure. See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/56_declarations_and_types#5-6-3-2-structure-declaration-with-forward-reference-or-self-reference > [/SAMI] Ok > + > +#endif > diff --git a/Drivers/OpTeeRpmb/FixupPcd.c b/Drivers/OpTeeRpmb/FixupPcd.c > new file mode 100644 [...] > + > + Instance = INSTANCE_FROM_FVB_THIS(FvbProtocol); > + // Patch PCDs with the the correct values > + PatchPcdSet32 (PcdFlashNvStorageVariableBase, Instance->MemBaseAddress); > + PatchPcdSet32 (PcdFlashNvStorageFtwWorkingBase, Instance->MemBaseAddress + > + PcdGet32 (PcdFlashNvStorageVariableSize)); > + PatchPcdSet32 (PcdFlashNvStorageFtwSpareBase, Instance->MemBaseAddress + > + PcdGet32 (PcdFlashNvStorageVariableSize) + > + PcdGet32 (PcdFlashNvStorageFtwWorkingSize));NorFlashDxe > [SAMI] Should the 64-bit versions of the PCDs be used here. > A recent change added similar support to the > ArmPlatformPkg\Drivers\NorFlashDxe. > [/SAMI] Looking at the NorFlashDxe commit, this is needed for NOR flash devices connected in a 64-bit address space. In our case OP-TEE maps the memory StMM can use, so that depends on the platform and how OP-TEE is layed out in memory. I'll have a look on the current OP-TEE supported platforms and if that's a case I'll change it. > + > + DEBUG ((DEBUG_INFO, "%a: Fixup PcdFlashNvStorageVariableBase: 0x%lx\n", [...] > + > +#include "OpTeeRpmbFvb.h" > + > +static const UINT16 mem_mgr_id = 3U; > +static const UINT16 storage_id = 4U; > [SAMI] Please change to STATIC CONST and also update the variable names > according to the edk2 coding standard. See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/4_naming_conventions/43_identifiers#4-3-3-2-any-variable-with-file-scope-or-better-shall-be-prefixed-by-an-m-or-g > [/SAMI] Ok > + > +STATIC MEM_INSTANCE mInstance; > + > +/** > + Sends an SVC call to OP-TEE for reading/writing an RPMB partition > + > + @param SvcAct SVC ID for read/write > + @param Addr Base address of the buffer. When reading contents will be > + copied to that buffer after reading them from the device. > + When writing, the buffer holds the contents we want to > + write cwtoin the device > + @param NumBytes Number of bytes to read/write > + @param Offset Offset into the RPMB file > + > + @retval OP-TEE return code > +**/ > + > [SAMI] Remove blank line. Same at other places as well. > [/SAMI] Ok > +STATIC > +EFI_STATUS > +ReadWriteRpmb ( > + UINTN SvcAct, > + UINTN Addr, > + UINTN NumBytes, > + UINTN Offset > + ) > +{ > + ARM_SVC_ARGS SvcArgs; > + EFI_STATUS Status; > + > + ZeroMem (&SvcArgs, sizeof (SvcArgs)); > + > + SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64; > + SvcArgs.Arg1 = storage_id; > + SvcArgs.Arg2 = 0; > + SvcArgs.Arg3 = SvcAct; > + SvcArgs.Arg4 = Addr; > + SvcArgs.Arg5 = NumBytes; > + SvcArgs.Arg6 = Offset; > + > + ArmCallSvc (&SvcArgs); > + if (SvcArgs.Arg3) { > + DEBUG ((DEBUG_ERROR, "%a: Svc Call 0x%08x Addr: 0x%08x len: 0x%x Offset: > 0x%x failed with 0x%x\n", > + __func__, SvcAct, Addr, NumBytes, Offset, SvcArgs.Arg3)); > + } > + > + switch (SvcArgs.Arg3) { > + case ARM_SVC_SPM_RET_SUCCESS: > + Status = EFI_SUCCESS; > + break; > + > + case ARM_SVC_SPM_RET_NOT_SUPPORTED: > + Status = EFI_UNSUPPORTED; > + break; > + > + case ARM_SVC_SPM_RET_INVALID_PARAMS: > + Status = EFI_INVALID_PARAMETER; > + break; > + > + case ARM_SVC_SPM_RET_DENIED: > + Status = EFI_ACCESS_DENIED; > + break; > + > + case ARM_SVC_SPM_RET_NO_MEMORY: > + Status = EFI_BAD_BUFFER_SIZE; > + break; > + > + default: > + Status = EFI_ACCESS_DENIED; > + } > [SAMI] Should the error handling here be updated similar to the FF-A > StandaloneMmPkg patches? > [/SAMI] I actually picked up the error handling from the previous non-FFA code. I'll check what's on Sughosh latest patches and fix it if there are any differences. Looking at it again EFI_BAD_BUFFER_SIZE can change to indicate out of memory properly anyway. > + > + return Status; > +} [...] > +STATIC > +EFI_STATUS > +OpTeeRpmbFvbRead ( > + IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This, > + IN EFI_LBA Lba, > + IN UINTN Offset, > + IN OUT UINTN *NumBytes, > + IN OUT UINT8 *Buffer > + ) > +{ > + EFI_STATUS Status = EFI_SUCCESS; > + MEM_INSTANCE *Instance; > + VOID *Base; > + > + Instance = INSTANCE_FROM_FVB_THIS(This); > + if (Instance->Initialized == FALSE) { > [SAMI] if (!Instance->Initialized) ? > See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/57_c_programming#5-7-2-1-boolean-values-variable-type-boolean-do-not-require-explicit-comparisons-to-true-or-false > [/SAMI] Ok > + Instance->Initialize (Instance); > + } > + > + Base = (VOID *)Instance->MemBaseAddress + Lba * Instance->BlockSize + > Offset; [...] > + MEM_INSTANCE *Instance; > + EFI_STATUS Status = EFI_SUCCESS; > + VOID *Base; > + > + Instance = INSTANCE_FROM_FVB_THIS(This); > + if (Instance->Initialized == FALSE) { > [SAMI] if (!Instance->Initialized) ? > See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/57_c_programming#5-7-2-1-boolean-values-variable-type-boolean-do-not-require-explicit-comparisons-to-true-or-false > [/SAMI] sure > + Instance->Initialize (Instance); > + } > + Base = (VOID *)Instance->MemBaseAddress + Lba * Instance->BlockSize + > Offset; > + Status = ReadWriteRpmb (SP_SVC_RPMB_WRITE, (UINTN) Buffer, *NumBytes, > + Lba * Instance->BlockSize + Offset); > + if (Status != EFI_SUCCESS) { > [SAMI] if (EFI_ERROR (Status)) ? > [/SAMI] Ok > + return Status; > + } [...] > + if (!Buf) { > + return EFI_DEVICE_ERROR; > + } > + SetMem64 (Buf, NumLba * Instance->BlockSize, ~0UL); > + // Write the device > + Status = ReadWriteRpmb (SP_SVC_RPMB_WRITE, (UINTN) Buf, NumBytes, > + Start * Instance->BlockSize); > + if (Status != EFI_SUCCESS) { > [SAMI] if (EFI_ERROR (Status)) ? > [/SAMI] > + return Status; Ok > + } > + // Update the in memory copy > + SetMem64 (Base, NumLba * Instance->BlockSize, ~0UL); > + FreePool (Buf); > + } > + > + VA_END (Args); > + > + return EFI_SUCCESS; > +} > + > +/** > + Since we use a memory backed storage we need to restore the RPMB contents > + into memory before we register the Fvb protocol. > + > + @param Instace Address to copy flash contents to > + > + @retval 0 on success, OP-TEE error on failure > +**/ > +STATIC > +VOID > +ReadEntireFlash ( > + MEM_INSTANCE *Instance > + ) > +{ > + UINTN ReadAddr; > + > + UINTN StorageFtwWorkingSize = PcdGet32(PcdFlashNvStorageFtwWorkingSize); > + UINTN StorageVariableSize = PcdGet32(PcdFlashNvStorageVariableSize); > + UINTN StorageFtwSpareSize = PcdGet32(PcdFlashNvStorageFtwSpareSize); > + > + ReadAddr = Instance->MemBaseAddress; > + // There's no need to check if the read failed here. The upper EDK2 layers > + // will initialize the flash correctly if the in-memory copy is wrong > + ReadWriteRpmb(SP_SVC_RPMB_READ, ReadAddr, StorageVariableSize + > + StorageFtwWorkingSize + StorageFtwSpareSize , 0); > +} > + > + > +STATIC > +EFI_STATUS > +EFIAPI > +ValidateFvHeader ( > + IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader > + ) > +{ > + UINT16 Checksum; > + VARIABLE_STORE_HEADER *VariableStoreHeader; > + UINTN VariableStoreLength; > + UINTN FvLength; > + > + FvLength = PcdGet32(PcdFlashNvStorageVariableSize) + > + PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > + PcdGet32(PcdFlashNvStorageFtwSpareSize); > + > + // Verify the header revision, header signature, length > + // Length of FvBlock cannot be 2**64-1 > + // HeaderLength cannot be an odd number > + // > + if ( (FwVolHeader->Revision != EFI_FVH_REVISION) > + || (FwVolHeader->Signature != EFI_FVH_SIGNATURE) > + || (FwVolHeader->FvLength != FvLength) > + ) > + { > + DEBUG ((DEBUG_INFO, "%a: No Firmware Volume header present\n", > + __FUNCTION__)); > + return EFI_NOT_FOUND; > + } > + > + // Check the Firmware Volume Guid > + if (!CompareGuid (&FwVolHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid)) { > [SAMI] Not a comment for this patch but I noticed that BaseTools has an > implementation of CompareGuid() that returns INTN. > I wonder if that is intentional. Moreover, it also appears that the > CompareGuid() usage in BaseTools does not consistently follow the explicit > comparison rule > (https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/57_c_programming#5-7-2-1-boolean-values-variable-type-boolean-do-not-require-explicit-comparisons-to-true-or-false). > [/SAMI] > + DEBUG ((DEBUG_INFO, "%a: Firmware Volume Guid non-compatible\n", > + __FUNCTION__)); > + return EFI_NOT_FOUND; > + } > + > + // Verify the header checksum > + Checksum = CalculateSum16((UINT16*)FwVolHeader, FwVolHeader->HeaderLength); > + if (Checksum != 0) { > + DEBUG ((DEBUG_INFO, "%a: FV checksum is invalid (Checksum:0x%X)\n", > + __FUNCTION__, Checksum)); > + return EFI_NOT_FOUND; > [SAMI] Minor: By this point we should be fairly certain that this is a > Firmware Volume header. > So, should the error code returned here be EFI_VOLUME_CORRUPTED? Would the > same apply to the remaining checks below? > [/SAMI] I think that makes sense, but I'll have to check if there's a difference in behavior of the upper layers depending on the return value. The relevant Volume Header checks is a c/p from ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c and I kept it for consistency. Should we change the other driver as well at some point? [SAMI] Looking at the NorFlashDxe code, there may be more needed. But I think it will be a good improvement. [/SAMI] > + } > + > + VariableStoreHeader = (VOID *)((UINTN)FwVolHeader + > [SAMI] Should the typecast be (VARIABLE_STORE_HEADER*) instead of (VOID *)? > [/SAMI] Yes > + FwVolHeader->HeaderLength); > + > + // Check the Variable Store Guid [...] > + FirmwareVolumeHeader->Revision = EFI_FVH_REVISION; > + FirmwareVolumeHeader->BlockMap[0].NumBlocks = Instance->NBlocks; > + FirmwareVolumeHeader->BlockMap[0].Length = Instance->BlockSize; > + FirmwareVolumeHeader->BlockMap[1].NumBlocks = 0; > + FirmwareVolumeHeader->BlockMap[1].Length = 0; > + FirmwareVolumeHeader->Checksum = CalculateCheckSum16 ( > + (UINT16*)FirmwareVolumeHeader, > + FirmwareVolumeHeader->HeaderLength); > + > + // > + // VARIABLE_STORE_HEADER > + // > + VariableStoreHeader = (VOID *)((UINTN)Headers + > [SAMI] Should the typecase be (VARIABLE_STORE_HEADER*) instead of (VOID *)? > [/SAMI] Yes > + FirmwareVolumeHeader->HeaderLength); > + CopyGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid); > + VariableStoreHeader->Size = PcdGet32(PcdFlashNvStorageVariableSize) - > + FirmwareVolumeHeader->HeaderLength; > + VariableStoreHeader->Format = VARIABLE_STORE_FORMATTED; > + VariableStoreHeader->State = VARIABLE_STORE_HEALTHY; > + > + Status = ReadWriteRpmb(SP_SVC_RPMB_WRITE, (UINTN) Headers, HeadersLength, > 0); > + if (Status != EFI_SUCCESS) { > [SAMI] if (EFI_ERROR (Status)) ? > [/SAMI] Ok > + goto Exit; > + } > + // Install the combined header in memory > + CopyMem ((VOID*) Instance->MemBaseAddress, Headers, HeadersLength); > + > +Exit: > + FreePool (Headers); > + return Status; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +FvbInitialize ( > + MEM_INSTANCE *Instance > + ) > +{ > + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader; > + EFI_STATUS Status; > + > + if (Instance->Initialized == TRUE) { > [SAMI] if (Instance->Initialized) ? > See > https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/57_c_programming#5-7-2-1-boolean-values-variable-type-boolean-do-not-require-explicit-comparisons-to-true-or-false > [/SAMI] Sure > + return EFI_SUCCESS; > + } > + > + // 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)); > + > + // Check if the size of the area is at least one block size > + ASSERT ((PcdGet32 (PcdFlashNvStorageVariableSize) > 0) && > + (PcdGet32 (PcdFlashNvStorageVariableSize) / Instance->BlockSize > > 0)); > + ASSERT ((PcdGet32 (PcdFlashNvStorageFtwWorkingSize) > 0) && > + (PcdGet32 (PcdFlashNvStorageFtwWorkingSize) / Instance->BlockSize > > 0)); > + ASSERT ((PcdGet32 (PcdFlashNvStorageFtwSpareSize) > 0) && > + (PcdGet32 (PcdFlashNvStorageFtwSpareSize) / Instance->BlockSize > > 0)); > + > + // Ensure the Variable areas are aligned on block size boundaries > + ASSERT ((PcdGet32 (PcdFlashNvStorageVariableBase) % Instance->BlockSize) > == 0); > + ASSERT ((PcdGet32 (PcdFlashNvStorageFtwWorkingBase) % Instance->BlockSize) > == 0); > + ASSERT ((PcdGet32 (PcdFlashNvStorageFtwSpareBase) % Instance->BlockSize) > == 0); > [SAMI] These asserts may need to be adjusted if 64-bit versions of the PCDs > are used. > [/SAMI] Noted > + > + // Read the file from disk and copy it to memory > + ReadEntireFlash (Instance); > + > + FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *) Instance->MemBaseAddress; > + Status = ValidateFvHeader(FwVolHeader); > + if (EFI_ERROR (Status)) { > + // There is no valid header, so time to install one. > + DEBUG ((DEBUG_INFO, "%a: The FVB Header is not valid.\n", __FUNCTION__)); > + > + // Reset memory > + SetMem64 ((VOID *)Instance->MemBaseAddress, Instance->NBlocks * > Instance->BlockSize, ~0UL); > + DEBUG ((DEBUG_INFO, "%a: Erasing Flash.\n", __FUNCTION__)); > + Status = ReadWriteRpmb(SP_SVC_RPMB_WRITE, Instance->MemBaseAddress, > + PcdGet32(PcdFlashNvStorageVariableSize) + > + PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > + PcdGet32(PcdFlashNvStorageFtwSpareSize), 0); > + if (Status != EFI_SUCCESS) { > [SAMI] if (EFI_ERROR (Status)) ? > [/SAMI] Sure > + return Status; > + } > + // Install all appropriate headers > + DEBUG ((DEBUG_INFO, "%a: Installing a correct one for this volume.\n", > + __FUNCTION__)); > + Status = InitializeFvAndVariableStoreHeaders (Instance); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + } else { > + DEBUG ((DEBUG_INFO, "%a: Found valid FVB Header.\n", __FUNCTION__)); > + } > + Instance->Initialized = TRUE; > + > + return EFI_SUCCESS; > [SAMI] return Status; ? > [/SAMI] > +} Sure > + > +EFI_STATUS > +EFIAPI > +OpTeeRpmbFvbInit ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + VOID *Addr; > + UINTN FvLength; > + UINTN NBlocks; > + > + FvLength = PcdGet32(PcdFlashNvStorageVariableSize) + > + PcdGet32(PcdFlashNvStorageFtwWorkingSize) + > + PcdGet32(PcdFlashNvStorageFtwSpareSize); > + > + NBlocks = EFI_SIZE_TO_PAGES(ALIGN_VARIABLE(FvLength)); > + Addr = AllocatePages(NBlocks); > + ASSERT (Addr != NULL); > [SAMI] Asserts will vanish in release builds and a failure to allocate memory > would result in incorrect results. > Please handle this error and return EFI_OUT_OF_RESOURCES. > [/SAMI] Fair enough > + > + SetMem (&mInstance, sizeof (mInstance), 0); > + > + mInstance.FvbProtocol.GetPhysicalAddress = OpTeeRpmbFvbGetPhysicalAddress; > + mInstance.FvbProtocol.GetAttributes = OpTeeRpmbFvbGetAttributes; > + mInstance.FvbProtocol.SetAttributes = OpTeeRpmbFvbSetAttributes; > + mInstance.FvbProtocol.GetBlockSize = OpTeeRpmbFvbGetBlockSize; > + mInstance.FvbProtocol.EraseBlocks = OpTeeRpmbFvbErase; > + mInstance.FvbProtocol.Write = OpTeeRpmbFvbWrite; > + mInstance.FvbProtocol.Read = OpTeeRpmbFvbRead; > + > + mInstance.MemBaseAddress = (EFI_PHYSICAL_ADDRESS) Addr; > + mInstance.Signature = FLASH_SIGNATURE; > + mInstance.Initialize = FvbInitialize; > + mInstance.BlockSize = EFI_PAGE_SIZE; > + mInstance.NBlocks = NBlocks; > + > + // Update the defined PCDs related to Variable Storage > + PatchPcdSet32 (PcdFlashNvStorageVariableBase, mInstance.MemBaseAddress); > [SAMI] Should the 64-bit versions of the PCDs be used here. > A recent change added similar support to the > ArmPlatformPkg\Drivers\NorFlashDxe. > [/SAMI] Noted > + PatchPcdSet32 (PcdFlashNvStorageFtwWorkingBase, mInstance.MemBaseAddress + > + PcdGet32 (PcdFlashNvStorageVariableSize)); > + PatchPcdSet32 (PcdFlashNvStorageFtwSpareBase, mInstance.MemBaseAddress + > + PcdGet32 (PcdFlashNvStorageVariableSize) + > + PcdGet32 (PcdFlashNvStorageFtwWorkingSize)); > + > + Status = gMmst->MmInstallProtocolInterface ( > + &mInstance.Handle, > + &gEfiSmmFirmwareVolumeBlockProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &mInstance.FvbProtocol > + ); > + ASSERT_EFI_ERROR (Status); > + > + DEBUG ((DEBUG_INFO, "%a: Register OP-TEE RPMB Fvb\n", __FUNCTION__)); > + DEBUG ((DEBUG_INFO, "%a: Using NV store FV in-memory copy at 0x%lx\n", > + __FUNCTION__, PatchPcdGet32 (PcdFlashNvStorageVariableBase))); > + > + return Status; > +} > -- > 2.17.1 > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#70908): https://edk2.groups.io/g/devel/message/70908 Mute This Topic: https://groups.io/mt/78998101/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
