summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDandan Bi <dandan.bi@intel.com>2018-10-12 18:13:19 +0800
committerHao Wu <hao.a.wu@intel.com>2018-10-26 15:08:44 +0800
commit979b7d802c316722da3f3dee90799728c7aab38f (patch)
tree3661b38d302750487f16c3dd527e0b54741f7a83
parentadb2c050128c62ab96bdec95f648f9a18603b028 (diff)
downloadedk2-979b7d802c316722da3f3dee90799728c7aab38f.tar.gz
edk2-979b7d802c316722da3f3dee90799728c7aab38f.tar.xz
edk2-979b7d802c316722da3f3dee90799728c7aab38f.zip
MdeModulePkg/HiiDB: Make sure database update behaviors are atomic
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.gao@intel.com> Cc: Eric Dong <eric.dong@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi <dandan.bi@intel.com> Reviewed-by: Liming Gao <liming.gao@intel.com>
-rw-r--r--MdeModulePkg/Universal/HiiDatabaseDxe/Database.c53
-rw-r--r--MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h2
-rw-r--r--MdeModulePkg/Universal/HiiDatabaseDxe/Image.c12
-rw-r--r--MdeModulePkg/Universal/HiiDatabaseDxe/String.c9
4 files changed, 75 insertions, 1 deletions
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
index 655eba8cb9..b59a9b2965 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
@@ -26,6 +26,11 @@ 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.
@@ -3493,11 +3498,14 @@ HiiNewPackageList (
}
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
//
// Build a PackageList node
//
Status = GenerateHiiDatabaseRecord (Private, &DatabaseRecord);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
@@ -3507,6 +3515,7 @@ HiiNewPackageList (
//
Status = AddPackages (Private, EFI_HII_DATABASE_NOTIFY_NEW_PACK, PackageList, DatabaseRecord);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
@@ -3534,8 +3543,17 @@ HiiNewPackageList (
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.
//
@@ -3585,6 +3603,8 @@ HiiRemovePackageList (
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
Private = HII_DATABASE_DATABASE_PRIVATE_DATA_FROM_THIS (This);
//
@@ -3602,34 +3622,42 @@ HiiRemovePackageList (
//
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;
}
@@ -3655,6 +3683,16 @@ HiiRemovePackageList (
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.
@@ -3667,6 +3705,7 @@ HiiRemovePackageList (
}
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}
@@ -3719,6 +3758,7 @@ HiiUpdatePackageList (
Status = EFI_SUCCESS;
+ EfiAcquireLock (&mHiiDatabaseLock);
//
// Get original packagelist to be updated
//
@@ -3760,6 +3800,7 @@ HiiUpdatePackageList (
}
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
@@ -3779,6 +3820,16 @@ HiiUpdatePackageList (
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.
@@ -3791,7 +3842,7 @@ 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
@@ -61,6 +61,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#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
//
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
@@ -649,6 +649,8 @@ HiiNewImage (
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);
@@ -671,6 +673,7 @@ HiiNewImage (
//
ImageBlocks = AllocatePool (ImagePackage->ImageBlockSize + NewBlockSize);
if (ImageBlocks == NULL) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
//
@@ -704,6 +707,7 @@ HiiNewImage (
//
ImagePackage = (HII_IMAGE_PACKAGE_INSTANCE *) AllocateZeroPool (sizeof (HII_IMAGE_PACKAGE_INSTANCE));
if (ImagePackage == NULL) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
//
@@ -732,6 +736,7 @@ HiiNewImage (
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;
@@ -769,6 +774,8 @@ HiiNewImage (
HiiGetDatabaseInfo(&Private->HiiDatabase);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
+
return EFI_SUCCESS;
}
@@ -1064,6 +1071,8 @@ HiiSetImage (
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.
@@ -1108,6 +1117,7 @@ HiiSetImage (
);
break;
default:
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}
@@ -1121,6 +1131,7 @@ HiiSetImage (
//
ImageBlocks = AllocateZeroPool (ImagePackage->ImageBlockSize + NewBlockSize - OldBlockSize);
if (ImageBlocks == NULL) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_OUT_OF_RESOURCES;
}
@@ -1158,6 +1169,7 @@ HiiSetImage (
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
@@ -1210,6 +1210,8 @@ HiiNewString (
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
Status = EFI_SUCCESS;
NewStringPackageCreated = FALSE;
NewStringId = 0;
@@ -1573,6 +1575,8 @@ Done:
}
}
+ EfiReleaseLock (&mHiiDatabaseLock);
+
return Status;
}
@@ -1738,6 +1742,8 @@ HiiSetString (
return EFI_NOT_FOUND;
}
+ EfiAcquireLock (&mHiiDatabaseLock);
+
Private = HII_STRING_DATABASE_PRIVATE_DATA_FROM_THIS (This);
PackageListNode = NULL;
@@ -1764,6 +1770,7 @@ HiiSetString (
(EFI_FONT_INFO *) StringFontInfo
);
if (EFI_ERROR (Status)) {
+ EfiReleaseLock (&mHiiDatabaseLock);
return Status;
}
PackageListNode->PackageListHdr.PackageLength += StringPackage->StringPkgHdr->Header.Length - OldPackageLen;
@@ -1774,11 +1781,13 @@ HiiSetString (
if (gExportAfterReadyToBoot) {
HiiGetDatabaseInfo(&Private->HiiDatabase);
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_SUCCESS;
}
}
}
+ EfiReleaseLock (&mHiiDatabaseLock);
return EFI_NOT_FOUND;
}