From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EF6B5226CD7AD for ; Wed, 11 Apr 2018 17:55:50 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3AB8281A88BF; Thu, 12 Apr 2018 00:55:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-97.rdu2.redhat.com [10.10.120.97]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3E3E21208F7B; Thu, 12 Apr 2018 00:55:49 +0000 (UTC) From: Laszlo Ersek To: edk2-devel@lists.01.org Cc: Ard Biesheuvel , Leif Lindholm , Steve Capper , Supreeth Venkatesh Date: Thu, 12 Apr 2018 02:55:35 +0200 Message-Id: <20180412005540.26651-6-lersek@redhat.com> In-Reply-To: <20180412005540.26651-1-lersek@redhat.com> References: <20180412005540.26651-1-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Thu, 12 Apr 2018 00:55:50 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Thu, 12 Apr 2018 00:55:50 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: [PATCH 05/10] ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Apr 2018 00:55:51 -0000 The lazy initialization of the varstore FVB makes no longer sense at this point: - "mNorFlashInstanceTemplate.Initialize" is NULL; - in NorFlashCreateInstance(), we only set Instance->Initialize to non-NULL -- namely NorFlashFvbInitialize() -- if the FVB stands for the variable store (see "ContainVariableStorage" / "SupportFvb"); - we call Instance->Initialize() from three places: - from NorFlashWriteSingleBlock(), which is too late for the variable read service ("variable write" depends on "variable read"); - from InitializeFvAndVariableStoreHeaders(), but that is only reachable from NorFlashFvbInitialize(), i.e. recursively from Instance->Initialize() itself; - and from FvbRead(), which is never called by the variable driver, only by the FTW driver. However, the variable driver may read (not write) the memory-mapped varstore flash chip before the FTW driver is dispatched. Therefore the lazy initialization is both superfluous and insufficient. Initialize the varstore headers eagerly, before we install the FVB protocol interface. Cc: Ard Biesheuvel Cc: Leif Lindholm Cc: Steve Capper Cc: Supreeth Venkatesh Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h | 6 ------ ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 13 +------------ ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 9 --------- 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h index c24680098f62..5c07694fbfaa 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h @@ -119,36 +119,30 @@ #define INSTANCE_FROM_FVB_THIS(a) CR(a, NOR_FLASH_INSTANCE, FvbProtocol, NOR_FLASH_SIGNATURE) #define INSTANCE_FROM_BLKIO_THIS(a) CR(a, NOR_FLASH_INSTANCE, BlockIoProtocol, NOR_FLASH_SIGNATURE) #define INSTANCE_FROM_DISKIO_THIS(a) CR(a, NOR_FLASH_INSTANCE, DiskIoProtocol, NOR_FLASH_SIGNATURE) typedef struct _NOR_FLASH_INSTANCE NOR_FLASH_INSTANCE; -typedef EFI_STATUS (*NOR_FLASH_INITIALIZE) (NOR_FLASH_INSTANCE* Instance); - typedef struct { VENDOR_DEVICE_PATH Vendor; EFI_DEVICE_PATH_PROTOCOL End; } NOR_FLASH_DEVICE_PATH; struct _NOR_FLASH_INSTANCE { UINT32 Signature; EFI_HANDLE Handle; - BOOLEAN Initialized; - NOR_FLASH_INITIALIZE Initialize; - UINTN DeviceBaseAddress; UINTN RegionBaseAddress; UINTN Size; EFI_LBA StartLba; EFI_BLOCK_IO_PROTOCOL BlockIoProtocol; EFI_BLOCK_IO_MEDIA Media; EFI_DISK_IO_PROTOCOL DiskIoProtocol; - BOOLEAN SupportFvb; EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol; VOID* ShadowBuffer; NOR_FLASH_DEVICE_PATH DevicePath; }; diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c index 1098d9501cc7..46e815beb343 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c @@ -29,15 +29,12 @@ NOR_FLASH_INSTANCE **mNorFlashInstances; UINT32 mNorFlashDeviceCount; NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = { NOR_FLASH_SIGNATURE, // Signature NULL, // Handle ... NEED TO BE FILLED - FALSE, // Initialized - NULL, // Initialize - 0, // DeviceBaseAddress ... NEED TO BE FILLED 0, // RegionBaseAddress ... NEED TO BE FILLED 0, // Size ... NEED TO BE FILLED 0, // StartLba { @@ -66,13 +63,12 @@ NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = { { EFI_DISK_IO_PROTOCOL_REVISION, // Revision NorFlashDiskIoReadDisk, // ReadDisk NorFlashDiskIoWriteDisk // WriteDisk }, - FALSE, // SupportFvb ... NEED TO BE FILLED { FvbGetAttributes, // GetAttributes FvbSetAttributes, // SetAttributes FvbGetPhysicalAddress, // GetPhysicalAddress FvbGetBlockSize, // GetBlockSize FvbRead, // Read @@ -134,14 +130,13 @@ NorFlashCreateInstance ( Instance->ShadowBuffer = AllocateRuntimePool (BlockSize);; if (Instance->ShadowBuffer == NULL) { return EFI_OUT_OF_RESOURCES; } if (SupportFvb) { - Instance->SupportFvb = TRUE; - Instance->Initialize = NorFlashFvbInitialize; + NorFlashFvbInitialize (Instance); Status = gBS->InstallMultipleProtocolInterfaces ( &Instance->Handle, &gEfiDevicePathProtocolGuid, &Instance->DevicePath, &gEfiBlockIoProtocolGuid, &Instance->BlockIoProtocol, &gEfiFirmwareVolumeBlockProtocolGuid, &Instance->FvbProtocol, @@ -149,14 +144,12 @@ NorFlashCreateInstance ( ); if (EFI_ERROR(Status)) { FreePool (Instance); return Status; } } else { - Instance->Initialized = TRUE; - Status = gBS->InstallMultipleProtocolInterfaces ( &Instance->Handle, &gEfiDevicePathProtocolGuid, &Instance->DevicePath, &gEfiBlockIoProtocolGuid, &Instance->BlockIoProtocol, &gEfiDiskIoProtocolGuid, &Instance->DiskIoProtocol, NULL @@ -921,16 +914,12 @@ NorFlashWriteSingleBlock ( UINTN BlockSize; UINTN BlockAddress; UINTN PrevBlockAddress; PrevBlockAddress = 0; - if (!Instance->Initialized && Instance->Initialize) { - Instance->Initialize(Instance); - } - DEBUG ((DEBUG_BLKIO, "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Lba, Offset, *NumBytes, Buffer)); // Detect WriteDisabled state if (Instance->Media.ReadOnly == TRUE) { DEBUG ((EFI_D_ERROR, "NorFlashWriteSingleBlock: ERROR - Can not write: Device is in WriteDisabled state.\n")); // It is in WriteDisabled state, return an error right away diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c index 3ea858f46ffb..c3e6489f398f 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c @@ -56,16 +56,12 @@ InitializeFvAndVariableStoreHeaders ( EFI_STATUS Status; VOID* Headers; UINTN HeadersLength; EFI_FIRMWARE_VOLUME_HEADER *FirmwareVolumeHeader; VARIABLE_STORE_HEADER *VariableStoreHeader; - if (!Instance->Initialized && Instance->Initialize) { - Instance->Initialize (Instance); - } - HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER); 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)); @@ -428,16 +424,12 @@ FvbRead ( NOR_FLASH_INSTANCE *Instance; Instance = INSTANCE_FROM_FVB_THIS(This); DEBUG ((DEBUG_BLKIO, "FvbRead(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Instance->StartLba + Lba, Offset, *NumBytes, Buffer)); - if (!Instance->Initialized && Instance->Initialize) { - Instance->Initialize(Instance); - } - TempStatus = EFI_SUCCESS; // Cache the block size to avoid de-referencing pointers all the time BlockSize = Instance->Media.BlockSize; DEBUG ((DEBUG_BLKIO, "FvbRead: Check if (Offset=0x%x + NumBytes=0x%x) <= BlockSize=0x%x\n", Offset, *NumBytes, BlockSize )); @@ -746,13 +738,12 @@ NorFlashFvbInitialize ( Status = gDS->SetMemorySpaceAttributes ( Instance->DeviceBaseAddress, RuntimeMmioRegionSize, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); ASSERT_EFI_ERROR (Status); - Instance->Initialized = TRUE; mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase); // Set the index of the first LBA for the FVB Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize; BootMode = GetBootModeHob (); -- 2.14.1.3.gb7cf6e02401b