From dc52b0c75bd299ea3bc4c6273d3dfeb26253c001 Mon Sep 17 00:00:00 2001 From: Shubham Kumar Date: Wed, 1 Jul 2026 16:28:59 +0530 Subject: [PATCH] FmpDevicePkg/FmpDxe: Reduce NV variable reads for FMP state In PopulateDescriptor(), each FMP device previously called GetFmpControllerState() multiple times through individual getter functions (GetVersionFromVariable, GetLowestSupportedVersionFromVariable, etc.), each performing a separate GetVariable() call to read the same FmpControllerState NV variable. Rename the getter functions to *FromFmpControllerState and simplify them to pure field extractors that take a required FMP_CONTROLLER_STATE pointer. Each getter extracts the field value if the state is non-NULL and the field is valid, returning a default otherwise. No getter calls GetFmpControllerState() internally. PopulateDescriptor() now calls GetFmpControllerState() once and passes the result to all getters, then frees it after the last use. This eliminates redundant NV reads when the variable exists and avoids repeated failed reads and error messages when the variable does not exist. Signed-off-by: Shubham Kumar --- FmpDevicePkg/FmpDxe/FmpDxe.c | 27 ++-- FmpDevicePkg/FmpDxe/VariableSupport.c | 178 +++++++++++--------------- FmpDevicePkg/FmpDxe/VariableSupport.h | 81 +++++++----- 3 files changed, 137 insertions(+), 149 deletions(-) diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index 57ab2cda04..3d83affeac 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.c +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c @@ -214,15 +214,15 @@ GetImageTypeNameString ( 2. Check if we have a variable for lowest supported version (this will be updated with each capsule applied) 3. Check Fixed at build PCD - @param[in] Private Pointer to the private context structure for the - Firmware Management Protocol instance. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @retval The largest value **/ UINT32 GetLowestSupportedVersion ( - FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private + FMP_CONTROLLER_STATE *FmpControllerState ) { EFI_STATUS Status; @@ -259,7 +259,7 @@ GetLowestSupportedVersion ( // // Check the lowest supported version UEFI variable for this device // - VariableLowestSupportedVersion = GetLowestSupportedVersionFromVariable (Private); + VariableLowestSupportedVersion = GetLowestSupportedVersionFromFmpControllerState (FmpControllerState); if (VariableLowestSupportedVersion > ReturnLsv) { ReturnLsv = VariableLowestSupportedVersion; } @@ -283,8 +283,9 @@ PopulateDescriptor ( FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private ) { - EFI_STATUS Status; - UINT32 DependenciesSize; + EFI_STATUS Status; + UINT32 DependenciesSize; + FMP_CONTROLLER_STATE *FmpControllerState; if (Private == NULL) { DEBUG ((DEBUG_ERROR, "FmpDxe(%s): PopulateDescriptor() - Private is NULL.\n", mImageIdName)); @@ -314,6 +315,8 @@ PopulateDescriptor ( // GenerateFmpVariableNames (Private); + FmpControllerState = GetFmpControllerState (Private); + // // Get the version. Some devices don't support getting the firmware version // at runtime. If FmpDeviceLib does not support returning a version, then @@ -322,7 +325,7 @@ PopulateDescriptor ( Status = FmpDeviceGetVersion (&Private->Descriptor.Version); if (Status == EFI_UNSUPPORTED) { Private->RuntimeVersionSupported = FALSE; - Private->Descriptor.Version = GetVersionFromVariable (Private); + Private->Descriptor.Version = GetVersionFromFmpControllerState (FmpControllerState); } else if (EFI_ERROR (Status)) { // // Unexpected error. Use default version. @@ -358,7 +361,7 @@ PopulateDescriptor ( ); } - Private->Descriptor.LowestSupportedImageVersion = GetLowestSupportedVersion (Private); + Private->Descriptor.LowestSupportedImageVersion = GetLowestSupportedVersion (FmpControllerState); // // Get attributes from the FmpDeviceLib @@ -390,8 +393,12 @@ PopulateDescriptor ( Private->Descriptor.Size = 0; } - Private->Descriptor.LastAttemptVersion = GetLastAttemptVersionFromVariable (Private); - Private->Descriptor.LastAttemptStatus = GetLastAttemptStatusFromVariable (Private); + Private->Descriptor.LastAttemptVersion = GetLastAttemptVersionFromFmpControllerState (FmpControllerState); + Private->Descriptor.LastAttemptStatus = GetLastAttemptStatusFromFmpControllerState (FmpControllerState); + + if (FmpControllerState != NULL) { + FreePool (FmpControllerState); + } // // Get the dependency from the FmpDependencyDeviceLib. diff --git a/FmpDevicePkg/FmpDxe/VariableSupport.c b/FmpDevicePkg/FmpDxe/VariableSupport.c index 5126c5e2d5..71914f054f 100644 --- a/FmpDevicePkg/FmpDxe/VariableSupport.c +++ b/FmpDevicePkg/FmpDxe/VariableSupport.c @@ -87,14 +87,14 @@ DeleteFmpVariable ( Retrieve the FMP Controller State UEFI Variable value. Return NULL if the variable does not exist or if the size of the UEFI Variable is not the size of FMP_CONTROLLER_STATE. The buffer for the UEFI Variable value - if allocated using the UEFI Boot Service AllocatePool(). + is allocated using the UEFI Boot Service AllocatePool(). Caller must free + the returned buffer with FreePool(). @param[in] Private Private context structure for the managed controller. @return Pointer to the allocated FMP Controller State. Returns NULL if the variable does not exist or is a different size than expected. **/ -static FMP_CONTROLLER_STATE * GetFmpControllerState ( IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private @@ -323,40 +323,31 @@ GenerateFmpVariableNames ( /** Returns the value used to fill in the Version field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default version value - is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the Version + field is not valid, then a default version value is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpState" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The version of the firmware image in the firmware device. **/ UINT32 -GetVersionFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetVersionFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ) { - FMP_CONTROLLER_STATE *FmpControllerState; - UINT32 Value; + UINT32 Value; - Value = DEFAULT_VERSION; - FmpControllerState = GetFmpControllerState (Private); - if (FmpControllerState != NULL) { - if (FmpControllerState->VersionValid) { - Value = FmpControllerState->Version; - DEBUG (( - DEBUG_INFO, - "FmpDxe(%s): Get variable %g %s Version %08x\n", - mImageIdName, - &gEfiCallerIdGuid, - Private->FmpStateVariableName, - Value - )); - } - - FreePool (FmpControllerState); + Value = DEFAULT_VERSION; + if ((FmpControllerState != NULL) && FmpControllerState->VersionValid) { + Value = FmpControllerState->Version; + DEBUG (( + DEBUG_INFO, + "FmpDxe(%s): FMP Controller State Version %08x\n", + mImageIdName, + Value + )); } return Value; @@ -365,41 +356,32 @@ GetVersionFromVariable ( /** Returns the value used to fill in the LowestSupportedVersion field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default lowest - supported version value is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the Lsv field + is not valid, then a default lowest supported version value is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpState" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The lowest supported version of the firmware image in the firmware device. **/ UINT32 -GetLowestSupportedVersionFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetLowestSupportedVersionFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ) { - FMP_CONTROLLER_STATE *FmpControllerState; - UINT32 Value; + UINT32 Value; - Value = DEFAULT_LOWESTSUPPORTEDVERSION; - FmpControllerState = GetFmpControllerState (Private); - if (FmpControllerState != NULL) { - if (FmpControllerState->LsvValid) { - Value = FmpControllerState->Lsv; - DEBUG (( - DEBUG_INFO, - "FmpDxe(%s): Get variable %g %s LowestSupportedVersion %08x\n", - mImageIdName, - &gEfiCallerIdGuid, - Private->FmpStateVariableName, - Value - )); - } - - FreePool (FmpControllerState); + Value = DEFAULT_LOWESTSUPPORTEDVERSION; + if ((FmpControllerState != NULL) && FmpControllerState->LsvValid) { + Value = FmpControllerState->Lsv; + DEBUG (( + DEBUG_INFO, + "FmpDxe(%s): FMP Controller State LowestSupportedVersion %08x\n", + mImageIdName, + Value + )); } return Value; @@ -408,40 +390,32 @@ GetLowestSupportedVersionFromVariable ( /** Returns the value used to fill in the LastAttemptStatus field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default last attempt - status value is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the + LastAttemptStatus field is not valid, then a default last attempt status value + is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpState" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The last attempt status value for the most recent capsule update. **/ UINT32 -GetLastAttemptStatusFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetLastAttemptStatusFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ) { - FMP_CONTROLLER_STATE *FmpControllerState; - UINT32 Value; + UINT32 Value; - Value = DEFAULT_LASTATTEMPTSTATUS; - FmpControllerState = GetFmpControllerState (Private); - if (FmpControllerState != NULL) { - if (FmpControllerState->LastAttemptStatusValid) { - Value = FmpControllerState->LastAttemptStatus; - DEBUG (( - DEBUG_INFO, - "FmpDxe(%s): Get variable %g %s LastAttemptStatus %08x\n", - mImageIdName, - &gEfiCallerIdGuid, - Private->FmpStateVariableName, - Value - )); - } - - FreePool (FmpControllerState); + Value = DEFAULT_LASTATTEMPTSTATUS; + if ((FmpControllerState != NULL) && FmpControllerState->LastAttemptStatusValid) { + Value = FmpControllerState->LastAttemptStatus; + DEBUG (( + DEBUG_INFO, + "FmpDxe(%s): FMP Controller State LastAttemptStatus %08x\n", + mImageIdName, + Value + )); } return Value; @@ -450,40 +424,32 @@ GetLastAttemptStatusFromVariable ( /** Returns the value used to fill in the LastAttemptVersion field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default last attempt - version value is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the + LastAttemptVersion field is not valid, then a default last attempt version + value is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpState" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The last attempt version value for the most recent capsule update. **/ UINT32 -GetLastAttemptVersionFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetLastAttemptVersionFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ) { - FMP_CONTROLLER_STATE *FmpControllerState; - UINT32 Value; + UINT32 Value; - Value = DEFAULT_LASTATTEMPTVERSION; - FmpControllerState = GetFmpControllerState (Private); - if (FmpControllerState != NULL) { - if (FmpControllerState->LastAttemptVersionValid) { - Value = FmpControllerState->LastAttemptVersion; - DEBUG (( - DEBUG_INFO, - "FmpDxe(%s): Get variable %g %s LastAttemptVersion %08x\n", - mImageIdName, - &gEfiCallerIdGuid, - Private->FmpStateVariableName, - Value - )); - } - - FreePool (FmpControllerState); + Value = DEFAULT_LASTATTEMPTVERSION; + if ((FmpControllerState != NULL) && FmpControllerState->LastAttemptVersionValid) { + Value = FmpControllerState->LastAttemptVersion; + DEBUG (( + DEBUG_INFO, + "FmpDxe(%s): FMP Controller State LastAttemptVersion %08x\n", + mImageIdName, + Value + )); } return Value; diff --git a/FmpDevicePkg/FmpDxe/VariableSupport.h b/FmpDevicePkg/FmpDxe/VariableSupport.h index 516e799fca..d11044dd7f 100644 --- a/FmpDevicePkg/FmpDxe/VariableSupport.h +++ b/FmpDevicePkg/FmpDxe/VariableSupport.h @@ -81,76 +81,91 @@ GenerateFmpVariableNames ( ); /** - Returns the value used to fill in the Version field of the - EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default version value - is returned. - - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpDxe" + Retrieve the FMP Controller State UEFI Variable value. Return NULL if + the variable does not exist or if the size of the UEFI Variable is not the + size of FMP_CONTROLLER_STATE. The buffer for the UEFI Variable value + is allocated using the UEFI Boot Service AllocatePool(). Caller must free + the returned buffer with FreePool(). @param[in] Private Private context structure for the managed controller. + @return Pointer to the allocated FMP Controller State. Returns NULL + if the variable does not exist or is a different size than expected. +**/ +FMP_CONTROLLER_STATE * +GetFmpControllerState ( + IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private + ); + +/** + Returns the value used to fill in the Version field of the + EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the Version + field is not valid, then a default version value is returned. + + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. + @return The version of the firmware image in the firmware device. **/ UINT32 -GetVersionFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetVersionFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ); /** Returns the value used to fill in the LowestSupportedVersion field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default lowest - supported version value is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the Lsv field + is not valid, then a default lowest supported version value is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpDxe" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The lowest supported version of the firmware image in the firmware device. **/ UINT32 -GetLowestSupportedVersionFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetLowestSupportedVersionFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ); /** Returns the value used to fill in the LastAttemptStatus field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default last attempt - status value is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the + LastAttemptStatus field is not valid, then a default last attempt status value + is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpDxe" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The last attempt status value for the most recent capsule update. **/ UINT32 -GetLastAttemptStatusFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetLastAttemptStatusFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ); /** Returns the value used to fill in the LastAttemptVersion field of the EFI_FIRMWARE_IMAGE_DESCRIPTOR structure that is returned by the GetImageInfo() - service of the Firmware Management Protocol. The value is read from a UEFI - variable. If the UEFI variables does not exist, then a default last attempt - version value is returned. + service of the Firmware Management Protocol. The value is extracted from the + provided FmpControllerState. If FmpControllerState is NULL or the + LastAttemptVersion field is not valid, then a default last attempt version + value is returned. - UEFI Variable accessed: GUID = gEfiCallerIdGuid, Name = L"FmpDxe" - - @param[in] Private Private context structure for the managed controller. + @param[in] FmpControllerState The cached FMP Controller State, or NULL if the + state could not be retrieved. @return The last attempt version value for the most recent capsule update. **/ UINT32 -GetLastAttemptVersionFromVariable ( - IN FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private +GetLastAttemptVersionFromFmpControllerState ( + IN FMP_CONTROLLER_STATE *FmpControllerState ); /**