Add Lock for cirtical section in PCD database processing routines as PCD database is a shared resource in the system. The lock level is defined as EFI_TPL_CALLBACK. The PCD spec should be updated accordingly.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@2264 6f19259b-4bc3-4df7-8a09-765794883524

3 files changed, 81 insertions, 33 deletions

diff --git a/EdkModulePkg/Universal/PCD/Dxe/Pcd.c b/EdkModulePkg/Universal/PCD/Dxe/Pcd.c
index c8db1b02b7..57c67d6713 100644
--- a/EdkModulePkg/Universal/PCD/Dxe/Pcd.c
+++ b/EdkModulePkg/Universal/PCD/Dxe/Pcd.c
@@ -17,6 +17,7 @@ Module Name: Pcd.c
 

 #include "Service.h"

 

+EFI_LOCK mPcdDatabaseLock = EFI_INITIALIZE_LOCK_VARIABLE(EFI_TPL_CALLBACK);

 

 PCD_PROTOCOL mPcdInstance = {

   DxePcdSetSku,

@@ -472,9 +473,20 @@ DxeRegisterCallBackOnSet (
   IN  PCD_PROTOCOL_CALLBACK   CallBackFunction

   )

 {

+  EFI_STATUS Status;

+  

   ASSERT (CallBackFunction != NULL);

   

-  return DxeRegisterCallBackWorker (TokenNumber, Guid, CallBackFunction);

+  //

+  // Aquire lock to prevent reentrance from TPL_CALLBACK level

+  //

+  EfiAcquireLock (&mPcdDatabaseLock);

+

+  Status = DxeRegisterCallBackWorker (TokenNumber, Guid, CallBackFunction);

+

+  EfiReleaseLock (&mPcdDatabaseLock);

+  

+  return Status;

 }

 

 

@@ -487,9 +499,20 @@ DxeUnRegisterCallBackOnSet (
   IN  PCD_PROTOCOL_CALLBACK   CallBackFunction

   )

 {

+  EFI_STATUS Status;

+  

   ASSERT (CallBackFunction != NULL);

+

+  //

+  // Aquire lock to prevent reentrance from TPL_CALLBACK level

+  //

+  EfiAcquireLock (&mPcdDatabaseLock);

   

-  return DxeUnRegisterCallBackWorker (TokenNumber, Guid, CallBackFunction);

+  Status = DxeUnRegisterCallBackWorker (TokenNumber, Guid, CallBackFunction);

+

+  EfiReleaseLock (&mPcdDatabaseLock);

+  

+  return Status;

 }

 

 

diff --git a/EdkModulePkg/Universal/PCD/Dxe/Service.c b/EdkModulePkg/Universal/PCD/Dxe/Service.c
index a4b19878a5..13abab001a 100644
--- a/EdkModulePkg/Universal/PCD/Dxe/Service.c
+++ b/EdkModulePkg/Universal/PCD/Dxe/Service.c
@@ -29,25 +29,32 @@ GetWorker (
 {

   UINT32              *LocalTokenNumberTable;

   UINT16              *SizeTable;

-  BOOLEAN             IsPeiDb;

-  UINT32              Offset;

   EFI_GUID            *GuidTable;

   UINT16              *StringTable;

   EFI_GUID            *Guid;

   UINT16              *Name;

   VARIABLE_HEAD       *VariableHead;

   UINT8               *VaraiableDefaultBuffer;

-  EFI_STATUS          Status;

-  UINTN               DataSize;

   UINT8               *Data;

   VPD_HEAD            *VpdHead;

   UINT8               *PcdDb;

-  UINT16              StringTableIdx;      

-  UINT32              LocalTokenNumber;

+  VOID                *RetPtr;

   UINTN               MaxSize;

   UINTN               TmpTokenNumber;

+  UINTN               DataSize;

+  EFI_STATUS          Status;

+  UINT32              LocalTokenNumber;

+  UINT32              Offset;

+  UINT16              StringTableIdx;      

+  BOOLEAN             IsPeiDb;

 

   //

+  // Aquire lock to prevent reentrance from TPL_CALLBACK level

+  //

+  EfiAcquireLock (&mPcdDatabaseLock);

+

+  RetPtr = NULL;

+  //

   // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.

   // We have to decrement TokenNumber by 1 to make it usable

   // as the array index.

@@ -100,7 +107,8 @@ GetWorker (
   switch (LocalTokenNumber & PCD_TYPE_ALL_SET) {

     case PCD_TYPE_VPD:

       VpdHead = (VPD_HEAD *) ((UINT8 *) PcdDb + Offset);

-      return (VOID *) (UINTN) (FixedPcdGet32(PcdVpdBaseAddress) + VpdHead->Offset);

+      RetPtr = (VOID *) (UINTN) (FixedPcdGet32(PcdVpdBaseAddress) + VpdHead->Offset);

+      break;

       

     case PCD_TYPE_HII:

       GuidTable   = IsPeiDb ? mPcdDatabase->PeiDb.Init.GuidTable :

@@ -134,14 +142,16 @@ GetWorker (
       // Return 1) either the default value specified by Platform Integrator 

       //        2) Or the value Set by a PCD set operation.

       //

-      return (VOID *) VaraiableDefaultBuffer;

+      RetPtr = (VOID *) VaraiableDefaultBuffer;

+      break;

 

     case PCD_TYPE_STRING:

       StringTableIdx = (UINT16) *((UINT8 *) PcdDb + Offset);

-      return (VOID *) &StringTable[StringTableIdx];

+      RetPtr = (VOID *) &StringTable[StringTableIdx];

+      break;

 

     case PCD_TYPE_DATA:

-      return (VOID *) ((UINT8 *) PcdDb + Offset);

+      RetPtr = (VOID *) ((UINT8 *) PcdDb + Offset);

       break;

 

     default:

@@ -150,9 +160,9 @@ GetWorker (
       

   }

 

-  ASSERT (FALSE);

-      

-  return NULL;

+  EfiReleaseLock (&mPcdDatabaseLock);

+  

+  return RetPtr;

   

 }

 

@@ -561,6 +571,11 @@ SetWorker (
   UINTN               TmpTokenNumber;

 

   //

+  // Aquire lock to prevent reentrance from TPL_CALLBACK level

+  //

+  EfiAcquireLock (&mPcdDatabaseLock);

+

+  //

   // TokenNumber Zero is reserved as PCD_INVALID_TOKEN_NUMBER.

   // We have to decrement TokenNumber by 1 to make it usable

   // as the array index.

@@ -621,20 +636,23 @@ SetWorker (
   switch (LocalTokenNumber & PCD_TYPE_ALL_SET) {

     case PCD_TYPE_VPD:

       ASSERT (FALSE);

-      return EFI_INVALID_PARAMETER;

+      Status = EFI_INVALID_PARAMETER;

+      break;

     

     case PCD_TYPE_STRING:

       if (SetPtrTypeSize (TmpTokenNumber, Size)) {

         CopyMem (&StringTable[*((UINT16 *)InternalData)], Data, *Size);

-        return EFI_SUCCESS;

+        Status = EFI_SUCCESS;

       } else {

-        return EFI_INVALID_PARAMETER;

+        Status = EFI_INVALID_PARAMETER;

       }

+      break;

 

     case PCD_TYPE_HII:

       if (PtrType) {

         if (!SetPtrTypeSize (TmpTokenNumber, Size)) {

-          return EFI_INVALID_PARAMETER;

+          Status = EFI_INVALID_PARAMETER;

+          break;

         }

       }

       

@@ -651,50 +669,55 @@ SetWorker (
 

       if (EFI_NOT_FOUND == Status) {

         CopyMem (PcdDb + VariableHead->DefaultValueOffset, Data, *Size);

-        return EFI_SUCCESS;

-      } else {

-        return Status;

-      }

+        Status = EFI_SUCCESS;

+      } 

+      break;

       

     case PCD_TYPE_DATA:

       if (PtrType) {

         if (SetPtrTypeSize (TmpTokenNumber, Size)) {

           CopyMem (InternalData, Data, *Size);

-          return EFI_SUCCESS;

+          Status = EFI_SUCCESS;

         } else {

-          return EFI_INVALID_PARAMETER;

+          Status = EFI_INVALID_PARAMETER;

         }

+        break;

       }

 

+      Status = EFI_SUCCESS;

       switch (*Size) {

         case sizeof(UINT8):

           *((UINT8 *) InternalData) = *((UINT8 *) Data);

-          return EFI_SUCCESS;

+          break;

 

         case sizeof(UINT16):

           *((UINT16 *) InternalData) = *((UINT16 *) Data);

-          return EFI_SUCCESS;

+          break;

 

         case sizeof(UINT32):

           *((UINT32 *) InternalData) = *((UINT32 *) Data);

-          return EFI_SUCCESS;

+          break;

 

         case sizeof(UINT64):

           *((UINT64 *) InternalData) = *((UINT64 *) Data);

-          return EFI_SUCCESS;

+          break;

 

         default:

           ASSERT (FALSE);

-          return EFI_NOT_FOUND;

+          Status = EFI_NOT_FOUND;

+          break;

       }

+      break;

 

     default:

       ASSERT (FALSE);

+      Status = EFI_NOT_FOUND;

       break;

     }

-      

-  ASSERT (FALSE);

-  return EFI_NOT_FOUND;

+

+  EfiReleaseLock (&mPcdDatabaseLock);

+  

+  return Status;

 }

 

 

diff --git a/EdkModulePkg/Universal/PCD/Dxe/Service.h b/EdkModulePkg/Universal/PCD/Dxe/Service.h
index 67a7435c23..572d172f3c 100644
--- a/EdkModulePkg/Universal/PCD/Dxe/Service.h
+++ b/EdkModulePkg/Universal/PCD/Dxe/Service.h
@@ -475,4 +475,6 @@ extern PCD_DATABASE * mPcdDatabase;
 

 extern DXE_PCD_DATABASE_INIT gDXEPcdDbInit;

 

+extern EFI_LOCK mPcdDatabaseLock;

+

 #endif