public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Dandan Bi <dandan.bi@intel.com>
To: edk2-devel@lists.01.org
Cc: Eric Dong <eric.dong@intel.com>, Liming Gao <liming.gao@intel.com>
Subject: [PATCH v2 3/6] MdeModulePkg/UefiHiiLib: Add codes to validate question with bit fields
Date: Tue, 12 Sep 2017 11:44:05 +0800	[thread overview]
Message-ID: <1505187848-389908-4-git-send-email-dandan.bi@intel.com> (raw)
In-Reply-To: <1505187848-389908-1-git-send-email-dandan.bi@intel.com>

V2: Add previous missing logic to handle question with Bit VarStore
in UefiHiiLib.

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

In UefiHiiLib, there are codes to validate the current setting
of questions, now update the logic to handle question with bit
storage.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Library/UefiHiiLib/HiiLib.c         | 270 ++++++++++++++++-------
 MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h |   4 +-
 MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf   |   5 +-
 3 files changed, 202 insertions(+), 77 deletions(-)

diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
index cd0cd35..9df42d1 100644
--- a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
+++ b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
@@ -49,10 +49,11 @@ EFI_FORM_BROWSER2_PROTOCOL  *mUefiFormBrowser2 = NULL;
 GLOBAL_REMOVE_IF_UNREFERENCED CONST EFI_HII_PACKAGE_HEADER  mEndOfPakageList = {
   sizeof (EFI_HII_PACKAGE_HEADER),
   EFI_HII_PACKAGE_END
 };
 
+BOOLEAN mQuestionReferBitField = FALSE;
 /**
   Extract Hii package list GUID for given HII handle.
 
   If HiiHandle could not be found in the HII database, then ASSERT.
   If Guid is NULL, then ASSERT.
@@ -1167,10 +1168,16 @@ ValidateQuestionFromVfr (
   EFI_IFR_STRING               *IfrString;
   CHAR8                        *VarStoreName;
   UINTN                        Index;
   CHAR16                       *QuestionName;
   CHAR16                       *StringPtr;
+  UINT16                       BitOffset;
+  UINT16                       BitWidth;
+  UINT16                       TotalBits;
+  UINT32                       PreBits;
+  UINT32                       Mask;
+  UINT32                       *Value;
 
   //
   // Initialize the local variables.
   //
   Index             = 0;
@@ -1180,10 +1187,12 @@ ValidateQuestionFromVfr (
   IfrVarStore       = NULL;
   IfrNameValueStore = NULL;
   IfrEfiVarStore    = NULL;
   ZeroMem (&VarStoreData, sizeof (IFR_VARSTORAGE_DATA));
   ZeroMem (&VarBlockData, sizeof (VarBlockData));
+  BitOffset = 0;
+  BitWidth = 0;
 
   //
   // Check IFR value is in block data, then Validate Value
   //
   PackageOffset = sizeof (EFI_HII_PACKAGE_LIST_HEADER);
@@ -1343,12 +1352,23 @@ ValidateQuestionFromVfr (
             }
           } else {
             //
             // Get Offset by Question header and Width by DataType Flags
             //
-            Offset = IfrOneOf->Question.VarStoreInfo.VarOffset;
-            Width  = (UINT16) (1 << (IfrOneOf->Flags & EFI_IFR_NUMERIC_SIZE));
+            if (mQuestionReferBitField) {
+              //
+              // Get the byte offset/width for bit field.
+              //
+              BitOffset = IfrOneOf->Question.VarStoreInfo.VarOffset;
+              BitWidth = IfrOneOf->Flags & EFI_IFR_NUMERIC_SIZE_BIT;
+              Offset = BitOffset / 8;
+              TotalBits  = BitOffset - (Offset * 8) + BitWidth;
+              Width  = (TotalBits % 8 == 0 ? TotalBits / 8: TotalBits / 8 + 1);
+            } else {
+              Offset = IfrOneOf->Question.VarStoreInfo.VarOffset;
+              Width  = (UINT16) (1 << (IfrOneOf->Flags & EFI_IFR_NUMERIC_SIZE));
+            }
             //
             // Check whether this question is in current block array.
             //
             if (!BlockArrayCheck (CurrentBlockArray, Offset, Width)) {
               //
@@ -1368,11 +1388,27 @@ ValidateQuestionFromVfr (
 
             //
             // Get the current value for oneof opcode
             //
             VarValue = 0;
-            CopyMem (&VarValue, VarBuffer +  Offset, Width);
+            if (mQuestionReferBitField) {
+              //
+              // Get the value in bit fields.
+              //
+              mQuestionReferBitField = FALSE;
+              Value = (UINT32*) &VarValue;
+              PreBits = BitOffset - Offset * 8;
+              Mask = (1 << BitWidth) -1;
+              *Value = *(VarBuffer + Offset);
+              *Value |= *(VarBuffer + Offset + 1) << 8;
+              *Value |= *(VarBuffer + Offset + 2) << 16;
+              *Value |= *(VarBuffer + Offset + 3) << 24;
+              *Value >>= PreBits;
+              *Value &= Mask;
+            } else {
+              CopyMem (&VarValue, VarBuffer +  Offset, Width);
+            }
           }
           //
           // Set Block Data, to be checked in the following Oneof option opcode.
           //
           VarBlockData.OpCode     = IfrOpHdr->OpCode;
@@ -1414,12 +1450,26 @@ ValidateQuestionFromVfr (
             }
           } else {
             //
             // Get Offset by Question header and Width by DataType Flags
             //
-            Offset = IfrNumeric->Question.VarStoreInfo.VarOffset;
-            Width  = (UINT16) (1 << (IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE));
+            if (mQuestionReferBitField) {
+              //
+              // Get the byte offset/width for bit field.
+              //
+              BitOffset = IfrNumeric->Question.VarStoreInfo.VarOffset;
+              BitWidth = IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE_BIT;
+              Offset = BitOffset / 8;
+              //
+              // Use current bit width and the bit width before current bit (with same byte offset) to calculate the byte width.
+              //
+              TotalBits = BitOffset - (Offset * 8) + BitWidth;
+              Width  = (TotalBits % 8 == 0 ? TotalBits / 8: TotalBits / 8 + 1);
+            } else {
+              Offset = IfrNumeric->Question.VarStoreInfo.VarOffset;
+              Width  = (UINT16) (1 << (IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE));
+            }
             //
             // Check whether this question is in current block array.
             //
             if (!BlockArrayCheck (CurrentBlockArray, Offset, Width)) {
               //
@@ -1439,81 +1489,118 @@ ValidateQuestionFromVfr (
 
             //
             // Check the current value is in the numeric range.
             //
             VarValue = 0;
-            CopyMem (&VarValue, VarBuffer +  Offset, Width);
-          }
-          if ((IfrNumeric->Flags & EFI_IFR_DISPLAY) == 0) {
-            switch (IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE) {
-            case EFI_IFR_NUMERIC_SIZE_1:
-              if ((INT8) VarValue < (INT8) IfrNumeric->data.u8.MinValue || (INT8) VarValue > (INT8) IfrNumeric->data.u8.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
-              }
-              break;
-            case EFI_IFR_NUMERIC_SIZE_2:
-              if ((INT16) VarValue < (INT16) IfrNumeric->data.u16.MinValue || (INT16) VarValue > (INT16) IfrNumeric->data.u16.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
-              }
-              break;
-            case EFI_IFR_NUMERIC_SIZE_4:
-              if ((INT32) VarValue < (INT32) IfrNumeric->data.u32.MinValue || (INT32) VarValue > (INT32) IfrNumeric->data.u32.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
-              }
-              break;
-            case EFI_IFR_NUMERIC_SIZE_8:
-              if ((INT64) VarValue < (INT64) IfrNumeric->data.u64.MinValue || (INT64) VarValue > (INT64) IfrNumeric->data.u64.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
-              }
-              break;
+            if (mQuestionReferBitField) {
+              //
+              // Get the value in the bit fields.
+              //
+              Value = (UINT32*) &VarValue;
+              PreBits = BitOffset - Offset * 8;
+              Mask = (1<< BitWidth) -1;
+              *Value = *(VarBuffer +  Offset);
+              *Value |= *(VarBuffer +  Offset + 1) << 8;
+              *Value |= *(VarBuffer +  Offset + 2) << 16;
+              *Value |= *(VarBuffer +  Offset + 3) << 24;
+              *Value >>= PreBits;
+              *Value &= Mask;
+            } else {
+              CopyMem (&VarValue, VarBuffer +  Offset, Width);
             }
+          }
+          if ( mQuestionReferBitField) {
+             //
+             // Value in bit fields was stored as UINt32 type.
+             //
+             mQuestionReferBitField = FALSE;
+             if ((IfrNumeric->Flags & EFI_IFR_DISPLAY_BIT) == 0) {
+               if ((INT32) VarValue < (INT32) IfrNumeric->data.u32.MinValue || (INT32) VarValue > (INT32) IfrNumeric->data.u32.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+             } else {
+               if (VarValue < IfrNumeric->data.u32.MinValue || VarValue > IfrNumeric->data.u32.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+             }
           } else {
-            switch (IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE) {
-            case EFI_IFR_NUMERIC_SIZE_1:
-              if ((UINT8) VarValue < IfrNumeric->data.u8.MinValue || (UINT8) VarValue > IfrNumeric->data.u8.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
-              }
-              break;
-            case EFI_IFR_NUMERIC_SIZE_2:
-              if ((UINT16) VarValue < IfrNumeric->data.u16.MinValue || (UINT16) VarValue > IfrNumeric->data.u16.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
-              }
-              break;
-            case EFI_IFR_NUMERIC_SIZE_4:
-              if ((UINT32) VarValue < IfrNumeric->data.u32.MinValue || (UINT32) VarValue > IfrNumeric->data.u32.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
+            if ((IfrNumeric->Flags & EFI_IFR_DISPLAY) == 0) {
+              switch (IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE) {
+              case EFI_IFR_NUMERIC_SIZE_1:
+                if ((INT8) VarValue < (INT8) IfrNumeric->data.u8.MinValue || (INT8) VarValue > (INT8) IfrNumeric->data.u8.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
+              case EFI_IFR_NUMERIC_SIZE_2:
+                if ((INT16) VarValue < (INT16) IfrNumeric->data.u16.MinValue || (INT16) VarValue > (INT16) IfrNumeric->data.u16.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
+              case EFI_IFR_NUMERIC_SIZE_4:
+                if ((INT32) VarValue < (INT32) IfrNumeric->data.u32.MinValue || (INT32) VarValue > (INT32) IfrNumeric->data.u32.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
+              case EFI_IFR_NUMERIC_SIZE_8:
+                if ((INT64) VarValue < (INT64) IfrNumeric->data.u64.MinValue || (INT64) VarValue > (INT64) IfrNumeric->data.u64.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
               }
-              break;
-            case EFI_IFR_NUMERIC_SIZE_8:
-              if ((UINT64) VarValue < IfrNumeric->data.u64.MinValue || (UINT64) VarValue > IfrNumeric->data.u64.MaxValue) {
-                //
-                // Not in the valid range.
-                //
-                return EFI_INVALID_PARAMETER;
+            } else {
+              switch (IfrNumeric->Flags & EFI_IFR_NUMERIC_SIZE) {
+              case EFI_IFR_NUMERIC_SIZE_1:
+                if ((UINT8) VarValue < IfrNumeric->data.u8.MinValue || (UINT8) VarValue > IfrNumeric->data.u8.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
+              case EFI_IFR_NUMERIC_SIZE_2:
+                if ((UINT16) VarValue < IfrNumeric->data.u16.MinValue || (UINT16) VarValue > IfrNumeric->data.u16.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
+              case EFI_IFR_NUMERIC_SIZE_4:
+                if ((UINT32) VarValue < IfrNumeric->data.u32.MinValue || (UINT32) VarValue > IfrNumeric->data.u32.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
+              case EFI_IFR_NUMERIC_SIZE_8:
+                if ((UINT64) VarValue < IfrNumeric->data.u64.MinValue || (UINT64) VarValue > IfrNumeric->data.u64.MaxValue) {
+                  //
+                  // Not in the valid range.
+                  //
+                  return EFI_INVALID_PARAMETER;
+                }
+                break;
               }
-              break;
             }
           }
           break;
         case EFI_IFR_CHECKBOX_OP:
           //
@@ -1552,12 +1639,23 @@ ValidateQuestionFromVfr (
             }
           } else {
             //
             // Get Offset by Question header
             //
-            Offset = IfrCheckBox->Question.VarStoreInfo.VarOffset;
-            Width  = (UINT16) sizeof (BOOLEAN);
+           if (mQuestionReferBitField) {
+              //
+              // Get the byte offset/width for bit field.
+              //
+              BitOffset = IfrCheckBox->Question.VarStoreInfo.VarOffset;
+              BitWidth = 1;
+              Offset = BitOffset / 8;
+              TotalBits  = BitOffset - (Offset * 8) + BitWidth;
+              Width  = (TotalBits % 8 == 0 ? TotalBits / 8: TotalBits / 8 + 1);
+            } else {
+              Offset = IfrCheckBox->Question.VarStoreInfo.VarOffset;
+              Width  = (UINT16) sizeof (BOOLEAN);
+            }
             //
             // Check whether this question is in current block array.
             //
             if (!BlockArrayCheck (CurrentBlockArray, Offset, Width)) {
               //
@@ -1576,11 +1674,27 @@ ValidateQuestionFromVfr (
             }
             //
             // Check the current value is in the numeric range.
             //
             VarValue = 0;
-            CopyMem (&VarValue, VarBuffer +  Offset, Width);
+            if (mQuestionReferBitField) {
+              //
+              // Get the value in bit fields.
+              //
+              mQuestionReferBitField = FALSE;
+              Value = (UINT32*) &VarValue;
+              PreBits = BitOffset - Offset* 8;
+              Mask = (1<< BitWidth) -1;
+              *Value = *(VarBuffer +  Offset);
+              *Value |= *(VarBuffer +  Offset + 1) << 8;
+              *Value |= *(VarBuffer +  Offset + 2) << 16;
+              *Value |= *(VarBuffer +  Offset + 3) << 24;
+              *Value >>= PreBits;
+              *Value &= Mask;
+            } else {
+              CopyMem (&VarValue, VarBuffer +  Offset, Width);
+            }
           }
           //
           // Boolean type, only 1 and 0 is valid.
           //
           if (VarValue > 1) {
@@ -1692,10 +1806,11 @@ ValidateQuestionFromVfr (
               VarBlockData.OpCode = 0;
             }
           }
           break;
         case EFI_IFR_END_OP:
+          mQuestionReferBitField = FALSE;
           //
           // Decrease opcode scope for the validated opcode
           //
           if (VarBlockData.Scope > 0) {
             VarBlockData.Scope --;
@@ -1706,10 +1821,15 @@ ValidateQuestionFromVfr (
           //
           if ((VarBlockData.Scope == 0) && (VarBlockData.OpCode == EFI_IFR_ONE_OF_OP)) {
             return EFI_INVALID_PARAMETER;
           }
           break;
+        case EFI_IFR_GUID_OP:
+          if (CompareGuid ((EFI_GUID *)((UINT8*)IfrOpHdr + sizeof (EFI_IFR_OP_HEADER)), &gEfiIfrBitvarstoreGuid)) {
+            mQuestionReferBitField = TRUE;
+          }
+          break;
         default:
           //
           // Increase Scope for the validated opcode
           //
           if (VarBlockData.Scope > 0) {
diff --git a/MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h b/MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h
index 9bf7696..293c226 100644
--- a/MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h
+++ b/MdeModulePkg/Library/UefiHiiLib/InternalHiiLib.h
@@ -1,9 +1,9 @@
 /** @file
   Internal include file for the HII Library instance.
 
-  Copyright (c) 2007, 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         
   which accompanies this distribution.  The full text of the license may be found at        
   http://opensource.org/licenses/bsd-license.php                                            
 
@@ -18,10 +18,12 @@
 #include <Uefi.h>
 
 #include <Protocol/DevicePath.h>
 #include <Protocol/FormBrowser2.h>
 
+#include <Guid/MdeModuleHii.h>
+
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/HiiLib.h>
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
diff --git a/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf b/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
index 62f435a..411c758 100644
--- a/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
+++ b/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
@@ -1,9 +1,9 @@
 ## @file
 #  HII Library implementation using UEFI HII protocols and services.
 #
-#  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 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
 #  which accompanies this distribution. The full text of the license may be found at
 #  http://opensource.org/licenses/bsd-license.php
@@ -49,5 +49,8 @@
   PrintLib
 
 [Protocols]
   gEfiFormBrowser2ProtocolGuid ## SOMETIMES_CONSUMES
   gEfiDevicePathProtocolGuid   ## SOMETIMES_CONSUMES
+
+[Guids]
+  gEfiIfrBitvarstoreGuid       ## SOMETIMES_CONSUMES ## GUID
-- 
1.9.5.msysgit.1



  parent reply	other threads:[~2017-09-12  3:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-12  3:44 [PATCH v2 0/6] Support bitfield in storage of vfr Dandan Bi
2017-09-12  3:44 ` [PATCH v2 1/6] BaseTool/VfrCompiler: Support Bit fields in EFI/Buffer VarStore Dandan Bi
2017-09-12  3:44 ` [PATCH v2 2/6] MdeModulePkg: Add GUID/flags to implement BitField support Dandan Bi
2017-09-12  3:44 ` Dandan Bi [this message]
2017-09-12  3:44 ` [PATCH v2 4/6] MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore Dandan Bi
2017-09-12  3:44 ` [PATCH v2 5/6] MdeModulePkg/SetupBrowser: " Dandan Bi
2017-09-12  3:44 ` [PATCH v2 6/6] MdeModulePkg/DriverSample: Add sample questions with bit/union VarStore Dandan Bi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1505187848-389908-4-git-send-email-dandan.bi@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox