V5: Update the EDKII extension GUID name with EDKII/Edkii prefix. Refine the interface and implemetation of function GetBitsQuestionValue and SetBitsQuestionValue.
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.d...@intel.com> Cc: Liming Gao <liming....@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi <dandan...@intel.com> --- MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c | 138 ++++++++++++++++----- MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 117 +++++++++++++++-- MdeModulePkg/Universal/SetupBrowserDxe/Setup.h | 3 + .../Universal/SetupBrowserDxe/SetupBrowserDxe.inf | 3 +- 4 files changed, 219 insertions(+), 42 deletions(-) diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c index 6b3e5e0..3081eb8 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c +++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c @@ -57,10 +57,11 @@ CreateStatement ( Statement->Signature = FORM_BROWSER_STATEMENT_SIGNATURE; 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)); CopyMem (&Statement->Help, &StatementHdr->Help, sizeof (EFI_STRING_ID)); @@ -1312,10 +1313,12 @@ ParseOpCodes ( BOOLEAN InUnknownScope; UINT8 UnknownDepth; FORMSET_DEFAULTSTORE *PreDefaultStore; LIST_ENTRY *DefaultLink; BOOLEAN HaveInserted; + UINT16 TotalBits; + BOOLEAN QuestionReferBitField; SuppressForQuestion = FALSE; SuppressForOption = FALSE; InScopeDisable = FALSE; DepthOfDisable = 0; @@ -1333,10 +1336,11 @@ ParseOpCodes ( MapExpressionList = NULL; TempVarstoreId = 0; ConditionalExprCount = 0; InUnknownScope = FALSE; UnknownDepth = 0; + QuestionReferBitField = FALSE; // // Get the number of Statements and Expressions // CountOpCodes (FormSet, &NumberOfStatement, &NumberOfExpression); @@ -1978,47 +1982,98 @@ ParseOpCodes ( ASSERT(CurrentStatement != NULL); 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 & EFI_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 &= EFI_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) { SuppressForOption = TRUE; } @@ -2045,10 +2100,22 @@ ParseOpCodes ( CurrentStatement->Flags = ((EFI_IFR_CHECKBOX *) OpCodeData)->Flags; 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; case EFI_IFR_STRING_OP: @@ -2592,18 +2659,23 @@ ParseOpCodes ( break; // // 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 (); return Status; } diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c index 89e06de..c079974 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c @@ -1367,10 +1367,75 @@ ConfigRespToStorage ( return Status; } /** + 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. @param Value Unicode buffer save the question value. @@ -1393,10 +1458,13 @@ BufferToValue ( UINTN Index; UINT8 DigitUint8; 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 || Question->Storage->Type == EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER) { IsBufferStorage = TRUE; @@ -1414,11 +1482,17 @@ BufferToValue ( Dst = Question->BufferValue; } else { // // 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; + } } // // Temp cut at the end of this section, end with '\0' or '&'. // @@ -1472,10 +1546,17 @@ BufferToValue ( } } *StringPtr = TempChar; + if (Question->QuestionReferToBitField) { + GetBitsQuestionValue (Question, Buffer); + if (Buffer != NULL) { + FreePool (Buffer); + } + } + return Status; } /** Get Question's current Value. @@ -1676,17 +1757,27 @@ GetQuestionValue ( if (GetValueFrom == GetSetValueWithEditBuffer || GetValueFrom == GetSetValueWithBuffer ) { if (IsBufferStorage) { 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; Status = GetValueByName (Storage, Question->VariableName, &Value, GetValueFrom); if (EFI_ERROR (Status)) { @@ -1948,17 +2039,27 @@ SetQuestionValue ( if (SetValueTo == GetSetValueWithEditBuffer || SetValueTo == GetSetValueWithBuffer) { if (IsBufferStorage) { 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) { // // Allocate enough string buffer. diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h index de140e9..09e0be7 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.h @@ -327,11 +327,14 @@ struct _FORM_BROWSER_STATEMENT{ EFI_QUESTION_ID QuestionId; // The value of zero is reserved EFI_VARSTORE_ID VarStoreId; // A value of zero indicates no variable storage 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=..." EFI_HII_VALUE HiiValue; // Edit copy for checkbox, numberic, oneof UINT8 *BufferValue; // Edit copy for string, password, orderedlist diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf b/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf index 012a39b..fefaefe 100644 --- a/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf +++ b/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf @@ -1,11 +1,11 @@ ## @file # The DXE driver produces FORM BROWSER2 protocol defined in UEFI specification. # # 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 # which accompanies this distribution. The full text of the license may be found at # http://opensource.org/licenses/bsd-license.php @@ -60,10 +60,11 @@ [Guids] gEfiIfrFrameworkGuid ## SOMETIMES_CONSUMES ## GUID gEfiHiiPlatformSetupFormsetGuid ## SOMETIMES_CONSUMES ## GUID gEfiHiiStandardFormGuid ## SOMETIMES_CONSUMES ## GUID gZeroGuid ## SOMETIMES_CONSUMES ## GUID + gEdkiiIfrBitVarstoreGuid ## SOMETIMES_CONSUMES ## GUID [Protocols] gEfiHiiConfigAccessProtocolGuid ## SOMETIMES_CONSUMES gEfiFormBrowser2ProtocolGuid ## PRODUCES gEdkiiFormBrowserEx2ProtocolGuid ## PRODUCES -- 1.9.5.msysgit.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
