REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1235
When update contents in HiiDatabase, like:
1. Add/update/remove package list
2. Add/update string
3. Add/update image
We should make these operations atomic to prevent the
potential issue that the one update operation with
higher TPL may interrupt another.
This commit is to make the HiiDatabase update behaviors
atomic by adding EfiAcquireLock/EfiReleaseLock function.
Cc: Liming Gao <liming....@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan...@intel.com>
---
.../Universal/HiiDatabaseDxe/Database.c | 53 ++++++++++++++++++-
.../Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +
MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 12 +++++
.../Universal/HiiDatabaseDxe/String.c | 9 ++++
4 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
index 1e2724f126..85fd10b63b 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
@@ -24,10 +24,15 @@ UINTN gConfigRespSize = 0;
BOOLEAN gExportConfigResp = FALSE;
UINTN gNvDefaultStoreSize = 0;
SKU_ID gSkuId = 0xFFFFFFFFFFFFFFFF;
LIST_ENTRY gVarStorageList =
INITIALIZE_LIST_HEAD_VARIABLE (gVarStorageList);
+//
+// HII database lock.
+//
+EFI_LOCK mHiiDatabaseLock = EFI_INITIALIZE_LOCK_VARIABLE(TPL_NOTIFY);
+
/**
This function generates a HII_DATABASE_RECORD node and adds into hii
database.
This is a internal function.
@param Private hii database private structure
@@ -3491,24 +3496,28 @@ HiiNewPackageList (
DatabaseRecord->DriverHandle == DriverHandle) {
return EFI_INVALID_PARAMETER;
}
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
//
// Build a PackageList node
//
Status = GenerateHiiDatabaseRecord (Private, &DatabaseRecord);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
//
// Fill in information of the created Package List node
// according to incoming package list.
//
Status = AddPackages (Private, EFI_HII_DATABASE_NOTIFY_NEW_PACK,
PackageList, DatabaseRecord);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
DatabaseRecord->DriverHandle = DriverHandle;
@@ -3532,12 +3541,21 @@ HiiNewPackageList (
// Only after ReadyToBoot, need to do the export.
//
if (gExportAfterReadyToBoot) {
HiiGetDatabaseInfo (This);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
//
+ // Notes:
+ // HiiGetDatabaseInfo () will get the contents of HII data base,
+ // belong to the atomic behavior of Hii Database update.
+ // And since HiiGetConfigRespInfo () will get the configuration setting info
from HII drivers
+ // we can not think it belong to the atomic behavior of Hii Database update.
+ // That's why EfiReleaseLock (&mHiiDatabaseLock) is callled before
HiiGetConfigRespInfo ().
+ //
+
// Check whether need to get the configuration setting info from HII drivers.
// When after ReadyToBoot and need to do the export for form package add.
//
if (gExportAfterReadyToBoot && gExportConfigResp) {
HiiGetConfigRespInfo (This);
@@ -3583,10 +3601,12 @@ HiiRemovePackageList (
if (!IsHiiHandleValid (Handle)) {
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
Private = HII_DATABASE_DATABASE_PRIVATE_DATA_FROM_THIS (This);
//
// Get the packagelist to be removed.
//
@@ -3600,38 +3620,46 @@ HiiRemovePackageList (
// Call registered functions with REMOVE_PACK before removing packages
// then remove them.
//
Status = RemoveGuidPackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveFormPackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveKeyboardLayoutPackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveStringPackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveFontPackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveImagePackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveSimpleFontPackages (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
Status = RemoveDevicePathPackage (Private, Handle, PackageList);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
//
// Free resources of the package list
@@ -3653,10 +3681,20 @@ HiiRemovePackageList (
// Only after ReadyToBoot, need to do the export.
//
if (gExportAfterReadyToBoot) {
HiiGetDatabaseInfo (This);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
+
+ //
+ // Notes:
+ // HiiGetDatabaseInfo () will get the contents of HII data base,
+ // belong to the atomic behavior of Hii Database update.
+ // And since HiiGetConfigRespInfo () will get the configuration setting
info from HII drivers
+ // we can not think it belong to the atomic behavior of Hii Database
update.
+ // That's why EfiReleaseLock (&mHiiDatabaseLock) is callled before
HiiGetConfigRespInfo ().
+ //
//
// Check whether need to get the configuration setting info from HII
drivers.
// When after ReadyToBoot and need to do the export for form package
remove.
//
@@ -3665,10 +3703,11 @@ HiiRemovePackageList (
}
return EFI_SUCCESS;
}
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}
/**
@@ -3717,10 +3756,11 @@ HiiUpdatePackageList (
PackageHdrPtr = (EFI_HII_PACKAGE_HEADER *) ((UINT8 *) PackageList + sizeof
(EFI_HII_PACKAGE_LIST_HEADER));
Status = EFI_SUCCESS;
+ EfiAcquireLock (&mHiiDatabaseLock);
//
// Get original packagelist to be updated
//
for (Link = Private->DatabaseList.ForwardLink; Link !=
&Private->DatabaseList; Link = Link->ForwardLink) {
Node = CR (Link, HII_DATABASE_RECORD, DatabaseEntry,
HII_DATABASE_RECORD_SIGNATURE);
@@ -3758,10 +3798,11 @@ HiiUpdatePackageList (
Status = RemoveDevicePathPackage (Private, Handle, OldPackageList);
break;
}
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
PackageHdrPtr = (EFI_HII_PACKAGE_HEADER *) ((UINT8 *) PackageHdrPtr +
PackageHeader.Length);
CopyMem (&PackageHeader, PackageHdrPtr, sizeof
(EFI_HII_PACKAGE_HEADER));
@@ -3777,10 +3818,20 @@ HiiUpdatePackageList (
// Only after ReadyToBoot, need to do the export.
//
if (gExportAfterReadyToBoot && Status == EFI_SUCCESS) {
HiiGetDatabaseInfo (This);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
+
+ //
+ // Notes:
+ // HiiGetDatabaseInfo () will get the contents of HII data base,
+ // belong to the atomic behavior of Hii Database update.
+ // And since HiiGetConfigRespInfo () will get the configuration setting
info from HII drivers
+ // we can not think it belong to the atomic behavior of Hii Database
update.
+ // That's why EfiReleaseLock (&mHiiDatabaseLock) is callled before
HiiGetConfigRespInfo ().
+ //
//
// Check whether need to get the configuration setting info from HII
drivers.
// When after ReadyToBoot and need to do the export for form package
update.
//
@@ -3789,11 +3840,11 @@ HiiUpdatePackageList (
}
return Status;
}
}
-
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}
/**
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
index 3a6eb8a55c..154e36e88f 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
@@ -59,10 +59,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#define BITMAP_LEN_1_BIT(Width, Height) (((Width) + 7) / 8 * (Height))
#define BITMAP_LEN_4_BIT(Width, Height) (((Width) + 1) / 2 * (Height))
#define BITMAP_LEN_8_BIT(Width, Height) ((Width) * (Height))
#define BITMAP_LEN_24_BIT(Width, Height) ((Width) * (Height) * 3)
+extern EFI_LOCK mHiiDatabaseLock;
+
//
// IFR data structure
//
// BASE_CR (a, IFR_DEFAULT_VALUE_DATA, Entry) to get the whole structure.
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
index d1010820e0..71ebc559c0 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
@@ -647,10 +647,12 @@ HiiNewImage (
PackageListNode = LocatePackageList (&Private->DatabaseList, PackageList);
if (PackageListNode == NULL) {
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof
(EFI_HII_RGB_PIXEL) +
BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);
//
// Get the image package in the package list,
@@ -669,10 +671,11 @@ HiiNewImage (
//
// Update the package's image block by appending the new block to the end.
//
ImageBlocks = AllocatePool (ImagePackage->ImageBlockSize + NewBlockSize);
if (ImageBlocks == NULL) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
//
// Copy the original content.
//
@@ -702,10 +705,11 @@ HiiNewImage (
// The specified package list does not contain image package.
// Create one to add this image block.
//
ImagePackage = (HII_IMAGE_PACKAGE_INSTANCE *) AllocateZeroPool (sizeof
(HII_IMAGE_PACKAGE_INSTANCE));
if (ImagePackage == NULL) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
//
// Output the image id of the incoming image being inserted, which is the
// first image block so that id is initially to one.
@@ -730,10 +734,11 @@ HiiNewImage (
//
ImagePackage->ImageBlockSize = NewBlockSize + sizeof
(EFI_HII_IIBT_END_BLOCK);
ImagePackage->ImageBlock = AllocateZeroPool (NewBlockSize + sizeof
(EFI_HII_IIBT_END_BLOCK));
if (ImagePackage->ImageBlock == NULL) {
FreePool (ImagePackage);
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
ImageBlocks = ImagePackage->ImageBlock;
//
@@ -767,10 +772,12 @@ HiiNewImage (
//
if (gExportAfterReadyToBoot) {
HiiGetDatabaseInfo(&Private->HiiDatabase);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
+
return EFI_SUCCESS;
}
/**
@@ -1062,10 +1069,12 @@ HiiSetImage (
CurrentImageBlock = GetImageIdOrAddress (ImagePackage->ImageBlock, &ImageId);
if (CurrentImageBlock == NULL) {
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
//
// Get the size of original image block. Use some common block code here
// since the definition of some structures is the same.
//
switch (CurrentImageBlock->BlockType) {
@@ -1106,10 +1115,11 @@ HiiSetImage (
(UINT32) ReadUnaligned16 ((VOID *)
&((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width),
ReadUnaligned16 ((VOID *)
&((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height)
);
break;
default:
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}
//
// Create the new image block according to input image.
@@ -1119,10 +1129,11 @@ HiiSetImage (
//
// Adjust the image package to remove the original block firstly then add
the new block.
//
ImageBlocks = AllocateZeroPool (ImagePackage->ImageBlockSize + NewBlockSize
- OldBlockSize);
if (ImageBlocks == NULL) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
Part1Size = (UINT32) ((UINTN) CurrentImageBlock - (UINTN)
ImagePackage->ImageBlock);
Part2Size = ImagePackage->ImageBlockSize - Part1Size - OldBlockSize;
@@ -1156,10 +1167,11 @@ HiiSetImage (
//
if (gExportAfterReadyToBoot) {
HiiGetDatabaseInfo(&Private->HiiDatabase);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
index aeda47430f..a8178bdca2 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
@@ -1208,10 +1208,12 @@ HiiNewString (
}
if (PackageListNode == NULL) {
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
Status = EFI_SUCCESS;
NewStringPackageCreated = FALSE;
NewStringId = 0;
NextStringId = 0;
StringPackage = NULL;
@@ -1571,10 +1573,12 @@ Done:
if (!EFI_ERROR (Status)) {
HiiGetDatabaseInfo(&Private->HiiDatabase);
}
}
+ EfiReleaseLock (&mHiiDatabaseLock);
+
return Status;
}
/**
@@ -1736,10 +1740,12 @@ HiiSetString (
if (!IsHiiHandleValid (PackageList)) {
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
Private = HII_STRING_DATABASE_PRIVATE_DATA_FROM_THIS (This);
PackageListNode = NULL;
for (Link = Private->DatabaseList.ForwardLink; Link !=
&Private->DatabaseList; Link = Link->ForwardLink) {
DatabaseRecord = CR (Link, HII_DATABASE_RECORD, DatabaseEntry,
HII_DATABASE_RECORD_SIGNATURE);
@@ -1762,25 +1768,28 @@ HiiSetString (
StringId,
(EFI_STRING) String,
(EFI_FONT_INFO *) StringFontInfo
);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
PackageListNode->PackageListHdr.PackageLength +=
StringPackage->StringPkgHdr->Header.Length - OldPackageLen;
//
// Check whether need to get the contents of HiiDataBase.
// Only after ReadyToBoot to do the export.
//
if (gExportAfterReadyToBoot) {
HiiGetDatabaseInfo(&Private->HiiDatabase);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_SUCCESS;
}
}
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}
--
2.18.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
