MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=545

For oneof/numeric/CheckBox(storage can be Bit VarStore)
If the question value can be updated and shown correctly
in UI page, we need do enhancements in following cases:
1. Parse the Ifr data to get the bit VarStore info correctly.
2. Set/get value to/from bit VarStore correctly.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>

4 files changed, 219 insertions, 42 deletions

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 6b3e5e098e..9bda482a4a 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -59,6 +59,7 @@ CreateStatement (
 

   Statement->Operand = ((EFI_IFR_OP_HEADER *) OpCodeData)->OpCode;

   Statement->OpCode  = (EFI_IFR_OP_HEADER *) OpCodeData;

+  Statement->QuestionReferToBitField = FALSE;

 

   StatementHdr = (EFI_IFR_STATEMENT_HEADER *) (OpCodeData + sizeof (EFI_IFR_OP_HEADER));

   CopyMem (&Statement->Prompt, &StatementHdr->Prompt, sizeof (EFI_STRING_ID));

@@ -1314,6 +1315,8 @@ ParseOpCodes (
   FORMSET_DEFAULTSTORE    *PreDefaultStore;

   LIST_ENTRY              *DefaultLink;

   BOOLEAN                 HaveInserted;

+  UINT16                  TotalBits;

+  BOOLEAN                 QuestionReferBitField;

 

   SuppressForQuestion      = FALSE;

   SuppressForOption        = FALSE;

@@ -1335,6 +1338,7 @@ ParseOpCodes (
   ConditionalExprCount     = 0;

   InUnknownScope           = FALSE;

   UnknownDepth             = 0;

+  QuestionReferBitField    = FALSE;

 

   //

   // Get the number of Statements and Expressions

@@ -1980,43 +1984,94 @@ ParseOpCodes (
       CurrentStatement->Flags = ((EFI_IFR_ONE_OF *) OpCodeData)->Flags;

       Value = &CurrentStatement->HiiValue;

 

-      switch (CurrentStatement->Flags & EFI_IFR_NUMERIC_SIZE) {

-      case EFI_IFR_NUMERIC_SIZE_1:

-        CurrentStatement->Minimum = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.MinValue;

-        CurrentStatement->Maximum = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.MaxValue;

-        CurrentStatement->Step    = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.Step;

-        CurrentStatement->StorageWidth = (UINT16) sizeof (UINT8);

-        Value->Type = EFI_IFR_TYPE_NUM_SIZE_8;

-        break;

+      if (QuestionReferBitField) {

+        //

+        // Get the bit var store info (bit/byte offset, bit/byte offset)

+        //

+        CurrentStatement->QuestionReferToBitField = TRUE;

+        CurrentStatement->BitStorageWidth = CurrentStatement->Flags & EDKII_IFR_NUMERIC_SIZE_BIT;

+        CurrentStatement->BitVarOffset = CurrentStatement->VarStoreInfo.VarOffset;

+        CurrentStatement->VarStoreInfo.VarOffset = CurrentStatement->BitVarOffset / 8;

+        TotalBits = CurrentStatement->BitVarOffset % 8 + CurrentStatement->BitStorageWidth;

+        CurrentStatement->StorageWidth = (TotalBits % 8 == 0? TotalBits / 8: TotalBits / 8 + 1);

 

-      case EFI_IFR_NUMERIC_SIZE_2:

-        CopyMem (&CurrentStatement->Minimum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.MinValue, sizeof (UINT16));

-        CopyMem (&CurrentStatement->Maximum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.MaxValue, sizeof (UINT16));

-        CopyMem (&CurrentStatement->Step,    &((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.Step,     sizeof (UINT16));

-        CurrentStatement->StorageWidth = (UINT16) sizeof (UINT16);

-        Value->Type = EFI_IFR_TYPE_NUM_SIZE_16;

-        break;

+        //

+        // Get the Minimum/Maximum/Step value(Note: bit field type has been stored as UINT32 type)

+        //

+        CurrentStatement->Minimum = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MinValue;

+        CurrentStatement->Maximum = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MaxValue;

+        CurrentStatement->Step    = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.Step;

 

-      case EFI_IFR_NUMERIC_SIZE_4:

-        CopyMem (&CurrentStatement->Minimum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MinValue, sizeof (UINT32));

-        CopyMem (&CurrentStatement->Maximum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MaxValue, sizeof (UINT32));

-        CopyMem (&CurrentStatement->Step,    &((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.Step,     sizeof (UINT32));

-        CurrentStatement->StorageWidth = (UINT16) sizeof (UINT32);

-        Value->Type = EFI_IFR_TYPE_NUM_SIZE_32;

-        break;

+        //

+        // Update the Flag and type of Minimum/Maximum/Step according to the actual width of bit field,

+        // in order to make Browser handle these question with bit varstore correctly.

+        //

+        ((EFI_IFR_NUMERIC *) OpCodeData)->Flags &=  EDKII_IFR_DISPLAY_BIT;

+        ((EFI_IFR_NUMERIC *) OpCodeData)->Flags >>= 2;

+        switch (CurrentStatement->StorageWidth) {

+        case 1:

+          ((EFI_IFR_NUMERIC *) OpCodeData)->Flags |= EFI_IFR_TYPE_NUM_SIZE_8;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.MinValue = (UINT8)CurrentStatement->Minimum;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.MaxValue = (UINT8)CurrentStatement->Maximum;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.Step = (UINT8)CurrentStatement->Step;

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_8;

+          break;

+        case 2:

+          ((EFI_IFR_NUMERIC *) OpCodeData)->Flags |= EFI_IFR_TYPE_NUM_SIZE_16;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.MinValue = (UINT16)CurrentStatement->Minimum;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.MaxValue = (UINT16)CurrentStatement->Maximum;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.Step = (UINT16)CurrentStatement->Step;

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_16;

+          break;

+        case 3:

+        case 4:

+          ((EFI_IFR_NUMERIC *) OpCodeData)->Flags |= EFI_IFR_TYPE_NUM_SIZE_32;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MinValue = (UINT32)CurrentStatement->Minimum;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MaxValue = (UINT32)CurrentStatement->Maximum;

+          ((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.Step = (UINT32)CurrentStatement->Step;

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_32;

+          break;

+        default:

+          break;

+        }

+      } else {

+        switch (CurrentStatement->Flags & EFI_IFR_NUMERIC_SIZE) {

+        case EFI_IFR_NUMERIC_SIZE_1:

+          CurrentStatement->Minimum = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.MinValue;

+          CurrentStatement->Maximum = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.MaxValue;

+          CurrentStatement->Step    = ((EFI_IFR_NUMERIC *) OpCodeData)->data.u8.Step;

+          CurrentStatement->StorageWidth = (UINT16) sizeof (UINT8);

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_8;

+          break;

 

-      case EFI_IFR_NUMERIC_SIZE_8:

-        CopyMem (&CurrentStatement->Minimum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u64.MinValue, sizeof (UINT64));

-        CopyMem (&CurrentStatement->Maximum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u64.MaxValue, sizeof (UINT64));

-        CopyMem (&CurrentStatement->Step,    &((EFI_IFR_NUMERIC *) OpCodeData)->data.u64.Step,     sizeof (UINT64));

-        CurrentStatement->StorageWidth = (UINT16) sizeof (UINT64);

-        Value->Type = EFI_IFR_TYPE_NUM_SIZE_64;

-        break;

+        case EFI_IFR_NUMERIC_SIZE_2:

+          CopyMem (&CurrentStatement->Minimum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.MinValue, sizeof (UINT16));

+          CopyMem (&CurrentStatement->Maximum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.MaxValue, sizeof (UINT16));

+          CopyMem (&CurrentStatement->Step,    &((EFI_IFR_NUMERIC *) OpCodeData)->data.u16.Step,     sizeof (UINT16));

+          CurrentStatement->StorageWidth = (UINT16) sizeof (UINT16);

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_16;

+          break;

 

-      default:

-        break;

-      }

+        case EFI_IFR_NUMERIC_SIZE_4:

+          CopyMem (&CurrentStatement->Minimum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MinValue, sizeof (UINT32));

+          CopyMem (&CurrentStatement->Maximum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.MaxValue, sizeof (UINT32));

+          CopyMem (&CurrentStatement->Step,    &((EFI_IFR_NUMERIC *) OpCodeData)->data.u32.Step,     sizeof (UINT32));

+          CurrentStatement->StorageWidth = (UINT16) sizeof (UINT32);

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_32;

+          break;

+

+        case EFI_IFR_NUMERIC_SIZE_8:

+          CopyMem (&CurrentStatement->Minimum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u64.MinValue, sizeof (UINT64));

+          CopyMem (&CurrentStatement->Maximum, &((EFI_IFR_NUMERIC *) OpCodeData)->data.u64.MaxValue, sizeof (UINT64));

+          CopyMem (&CurrentStatement->Step,    &((EFI_IFR_NUMERIC *) OpCodeData)->data.u64.Step,     sizeof (UINT64));

+          CurrentStatement->StorageWidth = (UINT16) sizeof (UINT64);

+          Value->Type = EFI_IFR_TYPE_NUM_SIZE_64;

+          break;

 

+        default:

+          break;

+        }

+      }

       InitializeRequestElement (FormSet, CurrentStatement, CurrentForm);

 

       if ((Operand == EFI_IFR_ONE_OF_OP) && Scope != 0) {

@@ -2047,6 +2102,18 @@ ParseOpCodes (
       CurrentStatement->StorageWidth = (UINT16) sizeof (BOOLEAN);

       CurrentStatement->HiiValue.Type = EFI_IFR_TYPE_BOOLEAN;

 

+      if (QuestionReferBitField) {

+        //

+        // Get the bit var store info (bit/byte offset, bit/byte offset)

+        //

+        CurrentStatement->QuestionReferToBitField = TRUE;

+        CurrentStatement->BitStorageWidth = 1;

+        CurrentStatement->BitVarOffset = CurrentStatement->VarStoreInfo.VarOffset;

+        CurrentStatement->VarStoreInfo.VarOffset = CurrentStatement->BitVarOffset / 8;

+        TotalBits = CurrentStatement->BitVarOffset % 8 + CurrentStatement->BitStorageWidth;

+        CurrentStatement->StorageWidth = (TotalBits % 8 == 0? TotalBits / 8: TotalBits / 8 + 1);

+      }

+

       InitializeRequestElement (FormSet, CurrentStatement, CurrentForm);

 

       break;

@@ -2594,14 +2661,19 @@ ParseOpCodes (
     //

     // Vendor specific

     //

-    case EFI_IFR_GUID_OP:     

+    case EFI_IFR_GUID_OP:

       CurrentStatement = CreateStatement (OpCodeData, FormSet, CurrentForm);

+      if (CompareGuid ((EFI_GUID *)(OpCodeData + sizeof (EFI_IFR_OP_HEADER)), &gEdkiiIfrBitVarstoreGuid)) {

+        Scope = 0;

+        QuestionReferBitField = TRUE;

+      }

       break;

 

     //

     // Scope End

     //

     case EFI_IFR_END_OP:

+      QuestionReferBitField = FALSE;

       Status = PopScope (&ScopeOpCode);

       if (EFI_ERROR (Status)) {

         ResetScopeStack ();

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index 89e06deae8..48beeb6539 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -1369,6 +1369,71 @@ ConfigRespToStorage (
 }

 

 /**

+  Get bit field value from the buffer and then set the value for the question.

+  Note: Data type UINT32 can cover all the bit field value.

+

+  @param  Question        The question refer to bit field.

+  @param  Buffer          Point to the buffer which the question value get from.

+

+**/

+VOID

+GetBitsQuestionValue (

+  IN  FORM_BROWSER_STATEMENT *Question,

+  IN  UINT8                  *Buffer

+  )

+{

+  UINTN    StartBit;

+  UINTN    EndBit;

+  UINT32   RetVal;

+  UINT32   BufferValue;

+

+  StartBit = Question->BitVarOffset % 8;

+  EndBit = StartBit + Question->BitStorageWidth - 1;

+

+  CopyMem ((UINT8 *) &BufferValue, Buffer, Question->StorageWidth);

+

+  RetVal = BitFieldRead32 (BufferValue, StartBit, EndBit);

+

+  //

+  // Set question value.

+  // Note: Since Question with BufferValue (orderedlist, password, string)are not supported to refer bit field.

+  // Only oneof/checkbox/oneof can support bit field.So we can copy the value to the Hiivalue of Question directly.

+  //

+  CopyMem ((UINT8 *) &Question->HiiValue.Value, (UINT8 *) &RetVal, Question->StorageWidth);

+}

+

+/**

+  Set bit field value to the buffer.

+  Note: Data type UINT32 can cover all the bit field value.

+

+  @param  Question        The question refer to bit field.

+  @param  Buffer          Point to the buffer which the question value set to.

+  @param  Value           The bit field value need to set.

+

+**/

+VOID

+SetBitsQuestionValue (

+  IN     FORM_BROWSER_STATEMENT *Question,

+  IN OUT UINT8                  *Buffer,

+  IN     UINT32                 Value

+  )

+{

+  UINT32   Operand;

+  UINTN    StartBit;

+  UINTN    EndBit;

+  UINT32   RetVal;

+

+  StartBit = Question->BitVarOffset % 8;

+  EndBit = StartBit + Question->BitStorageWidth - 1;

+

+  CopyMem ((UINT8*) &Operand, Buffer, Question->StorageWidth);

+

+  RetVal = BitFieldWrite32 (Operand, StartBit, EndBit, Value);

+

+  CopyMem (Buffer, (UINT8*) &RetVal, Question->StorageWidth);

+}

+

+/**

   Convert the buffer value to HiiValue.

 

   @param  Question               The question.

@@ -1395,6 +1460,9 @@ BufferToValue (
   BOOLEAN                      IsString;

   UINTN                        Length;

   EFI_STATUS                   Status;

+  UINT8                        *Buffer;

+

+  Buffer = NULL;

 

   IsString = (BOOLEAN) ((Question->HiiValue.Type == EFI_IFR_TYPE_STRING) ?  TRUE : FALSE);

   if (Question->Storage->Type == EFI_HII_VARSTORE_BUFFER || 

@@ -1416,7 +1484,13 @@ BufferToValue (
     //

     // Other type of Questions

     //

-    Dst = (UINT8 *) &Question->HiiValue.Value;

+    if (Question->QuestionReferToBitField) {

+      Buffer = (UINT8 *)AllocateZeroPool (Question->StorageWidth);

+      ASSERT (Buffer != NULL);

+      Dst = Buffer;

+    } else {

+      Dst = (UINT8 *) &Question->HiiValue.Value;

+    }

   }

 

   //

@@ -1474,6 +1548,13 @@ BufferToValue (
 

   *StringPtr = TempChar;

 

+  if (Question->QuestionReferToBitField) {

+    GetBitsQuestionValue (Question, Buffer);

+    if (Buffer != NULL) {

+      FreePool (Buffer);

+    }

+  }

+

   return Status;

 }

 

@@ -1678,13 +1759,23 @@ GetQuestionValue (
       if (GetValueFrom == GetSetValueWithEditBuffer) {

         //

         // Copy from storage Edit buffer

+        // If the Question refer to bit filed, get the value in the related bit filed.

         //

-        CopyMem (Dst, Storage->EditBuffer + Question->VarStoreInfo.VarOffset, StorageWidth);

+        if (Question->QuestionReferToBitField) {

+          GetBitsQuestionValue (Question, Storage->EditBuffer + Question->VarStoreInfo.VarOffset);

+        } else {

+          CopyMem (Dst, Storage->EditBuffer + Question->VarStoreInfo.VarOffset, StorageWidth);

+        }

       } else {

         //

         // Copy from storage Edit buffer

+        // If the Question refer to bit filed, get the value in the related bit filed.

         //

-        CopyMem (Dst, Storage->Buffer + Question->VarStoreInfo.VarOffset, StorageWidth);

+        if (Question->QuestionReferToBitField) {

+          GetBitsQuestionValue (Question, Storage->Buffer + Question->VarStoreInfo.VarOffset);

+        } else {

+          CopyMem (Dst, Storage->Buffer + Question->VarStoreInfo.VarOffset, StorageWidth);

+        }

       }

     } else {

       Value = NULL;

@@ -1950,13 +2041,23 @@ SetQuestionValue (
       if (SetValueTo == GetSetValueWithEditBuffer) {

         //

         // Copy to storage edit buffer

-        //      

-        CopyMem (Storage->EditBuffer + Question->VarStoreInfo.VarOffset, Src, StorageWidth);

+        // If the Question refer to bit filed, copy the value in related bit filed to storage edit buffer.

+        //

+        if (Question->QuestionReferToBitField) {

+          SetBitsQuestionValue (Question, Storage->EditBuffer + Question->VarStoreInfo.VarOffset, (UINT32)(*Src));

+        } else {

+          CopyMem (Storage->EditBuffer + Question->VarStoreInfo.VarOffset, Src, StorageWidth);

+        }

       } else if (SetValueTo == GetSetValueWithBuffer) {

         //

-        // Copy to storage edit buffer

-        //     

-        CopyMem (Storage->Buffer + Question->VarStoreInfo.VarOffset, Src, StorageWidth);

+        // Copy to storage buffer

+        // If the Question refer to bit filed, copy the value in related bit filed to storage buffer.

+        //

+        if (Question->QuestionReferToBitField) {

+          SetBitsQuestionValue (Question, Storage->Buffer + Question->VarStoreInfo.VarOffset, (UINT32)(*Src));

+        } else {

+          CopyMem (Storage->Buffer + Question->VarStoreInfo.VarOffset, Src, StorageWidth);

+        }

       }

     } else {

       if (IsString) {

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h
index de140e9cb2..09e0be7494 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h
@@ -329,7 +329,10 @@ struct _FORM_BROWSER_STATEMENT{
   BROWSER_STORAGE       *Storage;

   VAR_STORE_INFO        VarStoreInfo;

   UINT16                StorageWidth;

+  UINT16                BitStorageWidth;

+  UINT16                BitVarOffset;

   UINT8                 QuestionFlags;

+  BOOLEAN               QuestionReferToBitField;// Whether the question is stored in a bit field.

   CHAR16                *VariableName;    // Name/Value or EFI Variable name

   CHAR16                *BlockName;       // Buffer storage block name: "OFFSET=...WIDTH=..."

 

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf b/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
index 012a39bee5..fefaefe59c 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
@@ -3,7 +3,7 @@
 #

 # It also produces FormBrowserEx(2) protocol to let user register the different Hot key service. 

 #

-# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>

 #

 #  This program and the accompanying materials

 #  are licensed and made available under the terms and conditions of the BSD License

@@ -62,6 +62,7 @@
   gEfiHiiPlatformSetupFormsetGuid               ## SOMETIMES_CONSUMES  ## GUID

   gEfiHiiStandardFormGuid                       ## SOMETIMES_CONSUMES  ## GUID

   gZeroGuid                                     ## SOMETIMES_CONSUMES  ## GUID

+  gEdkiiIfrBitVarstoreGuid                      ## SOMETIMES_CONSUMES  ## GUID

 

 [Protocols]

   gEfiHiiConfigAccessProtocolGuid               ## SOMETIMES_CONSUMES