Re: [edk2] [PATCH] MdePkg SmmMemLib: Remove ASSERT in SmmIsBufferOutsideSmmValid

2017-06-04 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Monday, June 5, 2017 12:51 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ; Bret
> Barkelew 
> Subject: [PATCH] MdePkg SmmMemLib: Remove ASSERT in
> SmmIsBufferOutsideSmmValid
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=577
> 
> Currently the SmmIsBufferOutsideSmmValid() function in SmmMemLib.c will
> ASSERT in certain conditions. Since this function is a "test" function,
> it should not be making decisions on how to handle a failure.
> Handling a failure should be left to the caller.
> 
> This patch is to remove ASSERT(FALSE) at line 178 of SmmMemLib.c.
> 
> Cc: Jiewen Yao 
> Cc: Bret Barkelew 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Library/SmmMemLib/SmmMemLib.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/SmmMemLib/SmmMemLib.c
> b/MdePkg/Library/SmmMemLib/SmmMemLib.c
> index b4e3156cb42a..db55a1a6c150 100644
> --- a/MdePkg/Library/SmmMemLib/SmmMemLib.c
> +++ b/MdePkg/Library/SmmMemLib/SmmMemLib.c
> @@ -6,7 +6,7 @@
>all SMRAM range via SMM_ACCESS2_PROTOCOL, including the range for
> firmware (like SMM Core
>and SMM driver) and/or specific dedicated hardware.
> 
> -  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>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
> @@ -175,7 +175,6 @@ SmmIsBufferOutsideSmmValid (
>  Buffer,
>  Length
>  ));
> -  ASSERT (FALSE);
>return FALSE;
>  }
>}
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] MdePkg SmmMemLib: Remove ASSERT in SmmIsBufferOutsideSmmValid

2017-06-04 Thread Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=577

Currently the SmmIsBufferOutsideSmmValid() function in SmmMemLib.c will
ASSERT in certain conditions. Since this function is a "test" function,
it should not be making decisions on how to handle a failure.
Handling a failure should be left to the caller.

This patch is to remove ASSERT(FALSE) at line 178 of SmmMemLib.c.

Cc: Jiewen Yao 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdePkg/Library/SmmMemLib/SmmMemLib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdePkg/Library/SmmMemLib/SmmMemLib.c 
b/MdePkg/Library/SmmMemLib/SmmMemLib.c
index b4e3156cb42a..db55a1a6c150 100644
--- a/MdePkg/Library/SmmMemLib/SmmMemLib.c
+++ b/MdePkg/Library/SmmMemLib/SmmMemLib.c
@@ -6,7 +6,7 @@
   all SMRAM range via SMM_ACCESS2_PROTOCOL, including the range for firmware 
(like SMM Core
   and SMM driver) and/or specific dedicated hardware.
 
-  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
   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
@@ -175,7 +175,6 @@ SmmIsBufferOutsideSmmValid (
 Buffer,
 Length
 ));
-  ASSERT (FALSE);
   return FALSE;
 }
   }
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-04 Thread Dandan Bi
V2: Update VfrCompiler to use member variable instead of global varable
to indicate whether current date type is Union.

Cc: Eric Dong 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 19 ++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 16 ++--
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  3 ++-
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 406dbc5..9e1212a 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -155,10 +155,11 @@ VfrParserStart (
 #token Label("label")   "label"
 #token Timeout("timeout")   "timeout"
 #token Inventory("inventory")   "inventory"
 #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
 #token Struct("struct") "struct"
+#token Union("union")   "union"
 #token Boolean("BOOLEAN")   "BOOLEAN"
 #token Uint64("UINT64") "UINT64"
 #token Uint32("UINT32") "UINT32"
 #token Uint16("UINT16") "UINT16"
 #token Char16("CHAR16") "CHAR16"
@@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
  mConstantOnlyInExpression = FALSE;
   >>
   (
   vfrPragmaPackDefinition
 | vfrDataStructDefinition
+| vfrDataUnionDefinition
   )*
   vfrFormSetDefinition
   << $Return = mParserStatus; >>
   ;
 
@@ -318,12 +320,27 @@ vfrPragmaPackDefinition :
 | pragmaPackNumber
   }
   "\)"
   ;
 
+  vfrDataUnionDefinition :
+  { TypeDef } Union<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (TRUE); >>
+  { NonNvDataMap }
+  {
+N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
+  }
+  OpenBrace
+vfrDataStructFields
+  CloseBrace
+  {
+N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
+  }
+  ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd ();>>
+  ;
+
 vfrDataStructDefinition :
-  { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
+  { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (FALSE); >>
   { NonNvDataMap }
   {
 N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
   }
   OpenBrace
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp 
b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 2f97975..186b9c9 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -776,10 +776,11 @@ CVfrVarDataTypeDB::InternalTypesListInit (
 if (New != NULL) {
   strcpy (New->mTypeName, gInternalTypesTable[Index].mTypeName);
   New->mType= gInternalTypesTable[Index].mType;
   New->mAlign   = gInternalTypesTable[Index].mAlign;
   New->mTotalSize   = gInternalTypesTable[Index].mSize;
+  New->mIsUnionType = FALSE;
   if (strcmp (gInternalTypesTable[Index].mTypeName, "EFI_HII_DATE") == 0) {
 SVfrDataField *pYearField  = new SVfrDataField;
 SVfrDataField *pMonthField = new SVfrDataField;
 SVfrDataField *pDayField   = new SVfrDataField;
 
@@ -964,11 +965,11 @@ CVfrVarDataTypeDB::Pack (
   return VFR_RETURN_SUCCESS;
 }
 
 VOID
 CVfrVarDataTypeDB::DeclareDataTypeBegin (
-  VOID
+  BOOLEAN  IsUnionType
   )
 {
   SVfrDataType *pNewType = NULL;
 
   pNewType   = new SVfrDataType;
@@ -976,10 +977,11 @@ CVfrVarDataTypeDB::DeclareDataTypeBegin (
   pNewType->mType= EFI_IFR_TYPE_OTHER;
   pNewType->mAlign   = DEFAULT_ALIGN;
   pNewType->mTotalSize   = 0;
   pNewType->mMembers = NULL;
   pNewType->mNext= NULL;
+  pNewType->mIsUnionType = IsUnionType;
 
   mNewDataType   = pNewType;
 }
 
 EFI_VFR_RETURN_CODE
@@ -1018,12 +1020,14 @@ CVfrVarDataTypeDB::DataTypeAddField (
 {
   SVfrDataField   *pNewField  = NULL;
   SVfrDataType*pFieldType = NULL;
   SVfrDataField   *pTmp;
   UINT32  Align;
+  UINT32  MaxDataTypeSize;
 
   CHECK_ERROR_RETURN (GetDataType (TypeName, ), VFR_RETURN_SUCCESS);
+  MaxDataTypeSize = mNewDataType->mTotalSize;
 
   if (strlen (FieldName) >= MAX_NAME_LEN) {
return VFR_RETURN_INVALID_PARAMETER;
   }
 
@@ -1055,11 +1059,19 @@ CVfrVarDataTypeDB::DataTypeAddField (
 pTmp->mNext= pNewField;
 pNewField->mNext   = NULL;
   }
 
   mNewDataType->mAlign = MIN 

[edk2] [RFC v2 0/2] Support Union type in VFR

2017-06-04 Thread Dandan Bi
V2: Update VfrCompiler to use member variable instead of global varable
to indicate whether current date type is Union.

This serie is the POC to update VfrCompiler to support Union type
in VarStore and construct the use cases in DriverSample.

Cc: Eric Dong 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Dandan Bi (2):
  BaseTool/VfrCompile: Support Union type in VFR
  MdeModulePkg/DriverSample: Add sample questions to refer union type

 BaseTools/Source/C/VfrCompile/VfrSyntax.g  | 19 +++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp| 16 +-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h  |  3 +-
 .../Universal/DriverSampleDxe/DriverSample.c   | 57 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |  1 +
 .../Universal/DriverSampleDxe/NVDataStruc.h| 18 +++
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 25 ++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |  4 ++
 8 files changed, 139 insertions(+), 4 deletions(-)

-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC v2 2/2] MdeModulePkg/DriverSample: Add sample questions to refer union type

2017-06-04 Thread Dandan Bi
Cc: Eric Dong 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/DriverSampleDxe/DriverSample.c   | 57 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |  1 +
 .../Universal/DriverSampleDxe/NVDataStruc.h| 18 +++
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 25 ++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |  4 ++
 5 files changed, 105 insertions(+)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
index f103b9c..c4cd7f2 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -18,10 +18,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #define DISPLAY_ONLY_MY_ITEM  0x0002
 
 CHAR16 VariableName[] = L"MyIfrNVData";
 CHAR16 MyEfiVar[] = L"MyEfiVar";
+CHAR16 MyUnionVar[] = L"MyUnionVar";
 EFI_HANDLE  DriverHandle[2] = {NULL, NULL};
 DRIVER_SAMPLE_PRIVATE_DATA  *mPrivateData = NULL;
 EFI_EVENT   mEvent;
 
 HII_VENDOR_DEVICE_PATH  mHiiVendorDevicePath0 = {
@@ -662,10 +663,14 @@ ExtractConfig (
 // through hii database, not support in this path.
 //
 if (HiiIsConfigHdrMatch(Request, , MyEfiVar)) {
   return EFI_UNSUPPORTED;
 }
+
+if (HiiIsConfigHdrMatch(Request, , MyUnionVar)) {
+  return EFI_UNSUPPORTED;
+}
 //
 // Set Request to the unified request string.
 //
 ConfigRequest = Request;
 //
@@ -884,10 +889,14 @@ RouteConfig (
   //
   if (HiiIsConfigHdrMatch(Configuration, , MyEfiVar)) 
{
 return EFI_UNSUPPORTED;
   }
 
+  if (HiiIsConfigHdrMatch(Configuration, , 
MyUnionVar)) {
+return EFI_UNSUPPORTED;
+  }
+
   //
   // Get Buffer Storage data from EFI variable
   //
   BufferSize = sizeof (DRIVER_SAMPLE_CONFIGURATION);
   Status = gRT->GetVariable (
@@ -1685,10 +1694,11 @@ DriverSampleInit (
   EFI_STRING  ConfigRequestHdr;
   EFI_STRING  NameRequestHdr;
   MY_EFI_VARSTORE_DATA*VarStoreConfig;
   EFI_INPUT_KEY   HotKey;
   EDKII_FORM_BROWSER_EXTENSION_PROTOCOL *FormBrowserEx;
+  MY_UNION_DATA   *UnionConfig;
 
   //
   // Initialize the local variables.
   //
   ConfigRequestHdr = NULL;
@@ -1962,10 +1972,57 @@ DriverSampleInit (
   return EFI_INVALID_PARAMETER;
 }
   }
   FreePool (ConfigRequestHdr);
 
+  //
+  // Initialize Union efi varstore configuration data
+  //
+  UnionConfig = >UnionConfig;
+  ZeroMem (UnionConfig, sizeof (MY_UNION_DATA));
+
+  ConfigRequestHdr = HiiConstructConfigHdr (, 
MyUnionVar, DriverHandle[0]);
+  ASSERT (ConfigRequestHdr != NULL);
+
+  BufferSize = sizeof (MY_UNION_DATA);
+  Status = gRT->GetVariable (MyUnionVar, , NULL, 
, UnionConfig);
+  if (EFI_ERROR (Status)) {
+//
+// Store zero data to EFI variable Storage.
+//
+Status = gRT->SetVariable(
+MyUnionVar,
+,
+EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS,
+sizeof (MY_UNION_DATA),
+UnionConfig
+);
+if (EFI_ERROR (Status)) {
+  DriverSampleUnload (ImageHandle);
+  return Status;
+}
+//
+// EFI variable for NV config doesn't exit, we should build this variable
+// based on default values stored in IFR
+//
+ActionFlag = HiiSetToDefaults (ConfigRequestHdr, 
EFI_HII_DEFAULT_CLASS_STANDARD);
+if (!ActionFlag) {
+  DriverSampleUnload (ImageHandle);
+  return EFI_INVALID_PARAMETER;
+}
+  } else {
+//
+// EFI variable does exist and Validate Current Setting
+//
+ActionFlag = HiiValidateSettings (ConfigRequestHdr);
+if (!ActionFlag) {
+  DriverSampleUnload (ImageHandle);
+  return EFI_INVALID_PARAMETER;
+}
+  }
+  FreePool (ConfigRequestHdr);
+
   Status = gBS->CreateEventEx (
 EVT_NOTIFY_SIGNAL, 
 TPL_NOTIFY,
 EfiEventEmptyFunction,
 NULL,
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h
index 6c97239..895547a 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h
@@ -82,10 +82,11 @@ typedef struct {
 
   EFI_HANDLE   DriverHandle[2];
   EFI_HII_HANDLE   HiiHandle[2];
   DRIVER_SAMPLE_CONFIGURATION  Configuration;
   MY_EFI_VARSTORE_DATA VarStoreConfig;
+  MY_UNION_DATAUnionConfig;
 
   //
   // Name/Value storage Name list
   //
   EFI_STRING_IDNameStringId[NAME_VALUE_NAME_NUMBER];
diff --git 

Re: [edk2] [RFC 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-04 Thread Bi, Dandan
Hi Ray,

I can update the logic in VfrCompiler to avoid using gUnionTypeStructure. New 
patches will send out.
Thanks for your comments.

Regards,
Dandan


-Original Message-
From: Ni, Ruiyu 
Sent: Monday, June 5, 2017 10:35 AM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Gao, Liming 
Subject: RE: [edk2] [RFC 1/2] BaseTool/VfrCompile: Support Union type in VFR

Dandan,
Can you avoid using gUnionTypeStructure?

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Dandan Bi
> Sent: Monday, June 5, 2017 10:13 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Gao, Liming 
> 
> Subject: [edk2] [RFC 1/2] BaseTool/VfrCompile: Support Union type in 
> VFR
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 17 +
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 13 -
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  1 +
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 406dbc5..bd8457d 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -155,10 +155,11 @@ VfrParserStart (
>  #token Label("label")   "label"
>  #token Timeout("timeout")   "timeout"
>  #token Inventory("inventory")   "inventory"
>  #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
>  #token Struct("struct") "struct"
> +#token Union("union")   "union"
>  #token Boolean("BOOLEAN")   "BOOLEAN"
>  #token Uint64("UINT64") "UINT64"
>  #token Uint32("UINT32") "UINT32"
>  #token Uint16("UINT16") "UINT16"
>  #token Char16("CHAR16") "CHAR16"
> @@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
>   mConstantOnlyInExpression = FALSE;
>>>
>(
>vfrPragmaPackDefinition
>  | vfrDataStructDefinition
> +| vfrDataUnionDefinition
>)*
>vfrFormSetDefinition
><< $Return = mParserStatus; >>
>;
> 
> @@ -318,10 +320,25 @@ vfrPragmaPackDefinition :
>  | pragmaPackNumber
>}
>"\)"
>;
> 
> +  vfrDataUnionDefinition :
> +  { TypeDef } Union<< gUnionTypeStructure = 
> TRUE;
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
> +  { NonNvDataMap }
> +  {
> +N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
> +  }
> +  OpenBrace
> +vfrDataStructFields
> +  CloseBrace
> +  {
> +N2:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
> +  }
> +  ";"   << 
> gCVfrVarDataTypeDB.DeclareDataTypeEnd ();
> gUnionTypeStructure = FALSE;>>
> +  ;
> +
>  vfrDataStructDefinition :
>{ TypeDef } Struct<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
>{ NonNvDataMap }
>{
>  N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> index 2f97975..b392476 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> @@ -1018,12 +1018,14 @@ CVfrVarDataTypeDB::DataTypeAddField (  {
>SVfrDataField   *pNewField  = NULL;
>SVfrDataType*pFieldType = NULL;
>SVfrDataField   *pTmp;
>UINT32  Align;
> +  UINT32  MaxDataTypeSize;
> 
>CHECK_ERROR_RETURN (GetDataType (TypeName, ), 
> VFR_RETURN_SUCCESS);
> +  MaxDataTypeSize = mNewDataType->mTotalSize;
> 
>if (strlen (FieldName) >= MAX_NAME_LEN) {
> return VFR_RETURN_INVALID_PARAMETER;
>}
> 
> @@ -1055,11 +1057,19 @@ CVfrVarDataTypeDB::DataTypeAddField (
>  pTmp->mNext= pNewField;
>  pNewField->mNext   = NULL;
>}
> 
>mNewDataType->mAlign = MIN (mPackAlign, MAX (pFieldType->mAlign,
> mNewDataType->mAlign));
> -  mNewDataType->mTotalSize = pNewField->mOffset + (pNewField-
> >mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 : ArrayNum);
> +
> +  if (gUnionTypeStructure) {
> +if (MaxDataTypeSize < pNewField->mFieldType->mTotalSize) {
> +  mNewDataType->mTotalSize = pNewField->mFieldType->mTotalSize;
> +}
> +pNewField->mOffset = 0;
> +  } else {
> +

Re: [edk2] [RFC 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-04 Thread Ni, Ruiyu
Dandan,
Can you avoid using gUnionTypeStructure?

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Monday, June 5, 2017 10:13 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Gao, Liming 
> Subject: [edk2] [RFC 1/2] BaseTool/VfrCompile: Support Union type in VFR
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 17 +
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 13 -
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  1 +
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 406dbc5..bd8457d 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -155,10 +155,11 @@ VfrParserStart (
>  #token Label("label")   "label"
>  #token Timeout("timeout")   "timeout"
>  #token Inventory("inventory")   "inventory"
>  #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
>  #token Struct("struct") "struct"
> +#token Union("union")   "union"
>  #token Boolean("BOOLEAN")   "BOOLEAN"
>  #token Uint64("UINT64") "UINT64"
>  #token Uint32("UINT32") "UINT32"
>  #token Uint16("UINT16") "UINT16"
>  #token Char16("CHAR16") "CHAR16"
> @@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
>   mConstantOnlyInExpression = FALSE;
>>>
>(
>vfrPragmaPackDefinition
>  | vfrDataStructDefinition
> +| vfrDataUnionDefinition
>)*
>vfrFormSetDefinition
><< $Return = mParserStatus; >>
>;
> 
> @@ -318,10 +320,25 @@ vfrPragmaPackDefinition :
>  | pragmaPackNumber
>}
>"\)"
>;
> 
> +  vfrDataUnionDefinition :
> +  { TypeDef } Union<< gUnionTypeStructure = 
> TRUE;
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
> +  { NonNvDataMap }
> +  {
> +N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
> +  }
> +  OpenBrace
> +vfrDataStructFields
> +  CloseBrace
> +  {
> +N2:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
> +  }
> +  ";"   << 
> gCVfrVarDataTypeDB.DeclareDataTypeEnd ();
> gUnionTypeStructure = FALSE;>>
> +  ;
> +
>  vfrDataStructDefinition :
>{ TypeDef } Struct<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
>{ NonNvDataMap }
>{
>  N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
> diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> index 2f97975..b392476 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> @@ -1018,12 +1018,14 @@ CVfrVarDataTypeDB::DataTypeAddField (  {
>SVfrDataField   *pNewField  = NULL;
>SVfrDataType*pFieldType = NULL;
>SVfrDataField   *pTmp;
>UINT32  Align;
> +  UINT32  MaxDataTypeSize;
> 
>CHECK_ERROR_RETURN (GetDataType (TypeName, ),
> VFR_RETURN_SUCCESS);
> +  MaxDataTypeSize = mNewDataType->mTotalSize;
> 
>if (strlen (FieldName) >= MAX_NAME_LEN) {
> return VFR_RETURN_INVALID_PARAMETER;
>}
> 
> @@ -1055,11 +1057,19 @@ CVfrVarDataTypeDB::DataTypeAddField (
>  pTmp->mNext= pNewField;
>  pNewField->mNext   = NULL;
>}
> 
>mNewDataType->mAlign = MIN (mPackAlign, MAX (pFieldType->mAlign,
> mNewDataType->mAlign));
> -  mNewDataType->mTotalSize = pNewField->mOffset + (pNewField-
> >mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 : ArrayNum);
> +
> +  if (gUnionTypeStructure) {
> +if (MaxDataTypeSize < pNewField->mFieldType->mTotalSize) {
> +  mNewDataType->mTotalSize = pNewField->mFieldType->mTotalSize;
> +}
> +pNewField->mOffset = 0;
> +  } else {
> +mNewDataType->mTotalSize = pNewField->mOffset +
> + (pNewField->mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 :
> + ArrayNum);  }
> 
>return VFR_RETURN_SUCCESS;
>  }
> 
>  VOID
> @@ -3715,10 +3725,11 @@ CVfrStringDB::GetUnicodeStringTextSize (
>}
> 
>return StringSize;
>  }
> 
> +BOOLEAN  gUnionTypeStructure = FALSE;
>  BOOLEAN  VfrCompatibleMode = FALSE;
> 
>  CVfrVarDataTypeDB gCVfrVarDataTypeDB;
>  CVfrDefaultStore  gCVfrDefaultStore;
>  CVfrDataStorage  

[edk2] [RFC 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-04 Thread Dandan Bi
Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 17 +
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 13 -
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  1 +
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 406dbc5..bd8457d 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -155,10 +155,11 @@ VfrParserStart (
 #token Label("label")   "label"
 #token Timeout("timeout")   "timeout"
 #token Inventory("inventory")   "inventory"
 #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
 #token Struct("struct") "struct"
+#token Union("union")   "union"
 #token Boolean("BOOLEAN")   "BOOLEAN"
 #token Uint64("UINT64") "UINT64"
 #token Uint32("UINT32") "UINT32"
 #token Uint16("UINT16") "UINT16"
 #token Char16("CHAR16") "CHAR16"
@@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
  mConstantOnlyInExpression = FALSE;
   >>
   (
   vfrPragmaPackDefinition
 | vfrDataStructDefinition
+| vfrDataUnionDefinition
   )*
   vfrFormSetDefinition
   << $Return = mParserStatus; >>
   ;
 
@@ -318,10 +320,25 @@ vfrPragmaPackDefinition :
 | pragmaPackNumber
   }
   "\)"
   ;
 
+  vfrDataUnionDefinition :
+  { TypeDef } Union<< gUnionTypeStructure = 
TRUE; gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
+  { NonNvDataMap }
+  {
+N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
+  }
+  OpenBrace
+vfrDataStructFields
+  CloseBrace
+  {
+N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
+  }
+  ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd (); gUnionTypeStructure = FALSE;>>
+  ;
+
 vfrDataStructDefinition :
   { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
   { NonNvDataMap }
   {
 N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp 
b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 2f97975..b392476 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -1018,12 +1018,14 @@ CVfrVarDataTypeDB::DataTypeAddField (
 {
   SVfrDataField   *pNewField  = NULL;
   SVfrDataType*pFieldType = NULL;
   SVfrDataField   *pTmp;
   UINT32  Align;
+  UINT32  MaxDataTypeSize;
 
   CHECK_ERROR_RETURN (GetDataType (TypeName, ), VFR_RETURN_SUCCESS);
+  MaxDataTypeSize = mNewDataType->mTotalSize;
 
   if (strlen (FieldName) >= MAX_NAME_LEN) {
return VFR_RETURN_INVALID_PARAMETER;
   }
 
@@ -1055,11 +1057,19 @@ CVfrVarDataTypeDB::DataTypeAddField (
 pTmp->mNext= pNewField;
 pNewField->mNext   = NULL;
   }
 
   mNewDataType->mAlign = MIN (mPackAlign, MAX (pFieldType->mAlign, 
mNewDataType->mAlign));
-  mNewDataType->mTotalSize = pNewField->mOffset + 
(pNewField->mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 : ArrayNum);
+
+  if (gUnionTypeStructure) {
+if (MaxDataTypeSize < pNewField->mFieldType->mTotalSize) {
+  mNewDataType->mTotalSize = pNewField->mFieldType->mTotalSize;
+}
+pNewField->mOffset = 0;
+  } else {
+mNewDataType->mTotalSize = pNewField->mOffset + 
(pNewField->mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 : ArrayNum);
+  }
 
   return VFR_RETURN_SUCCESS;
 }
 
 VOID
@@ -3715,10 +3725,11 @@ CVfrStringDB::GetUnicodeStringTextSize (
   }
 
   return StringSize;
 }
 
+BOOLEAN  gUnionTypeStructure = FALSE;
 BOOLEAN  VfrCompatibleMode = FALSE;
 
 CVfrVarDataTypeDB gCVfrVarDataTypeDB;
 CVfrDefaultStore  gCVfrDefaultStore;
 CVfrDataStorage  gCVfrDataStorage;
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h 
b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
index 59509c3..b6791ec 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
@@ -19,10 +19,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include "string.h"
 #include "Common/UefiBaseTypes.h"
 #include "EfiVfr.h"
 #include "VfrError.h"
 
+extern BOOLEAN  gUnionTypeStructure;
 extern BOOLEAN  VfrCompatibleMode;
 
 #define MAX_NAME_LEN   64
 #define MAX_STRING_LEN 0x100
 

[edk2] [RFC 2/2] MdeModulePkg/DriverSample: Add sample questions to refer union type

2017-06-04 Thread Dandan Bi
Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/DriverSampleDxe/DriverSample.c   | 57 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |  1 +
 .../Universal/DriverSampleDxe/NVDataStruc.h| 18 +++
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 25 ++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |  4 ++
 5 files changed, 105 insertions(+)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
index f103b9c..c4cd7f2 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -18,10 +18,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #define DISPLAY_ONLY_MY_ITEM  0x0002
 
 CHAR16 VariableName[] = L"MyIfrNVData";
 CHAR16 MyEfiVar[] = L"MyEfiVar";
+CHAR16 MyUnionVar[] = L"MyUnionVar";
 EFI_HANDLE  DriverHandle[2] = {NULL, NULL};
 DRIVER_SAMPLE_PRIVATE_DATA  *mPrivateData = NULL;
 EFI_EVENT   mEvent;
 
 HII_VENDOR_DEVICE_PATH  mHiiVendorDevicePath0 = {
@@ -662,10 +663,14 @@ ExtractConfig (
 // through hii database, not support in this path.
 //
 if (HiiIsConfigHdrMatch(Request, , MyEfiVar)) {
   return EFI_UNSUPPORTED;
 }
+
+if (HiiIsConfigHdrMatch(Request, , MyUnionVar)) {
+  return EFI_UNSUPPORTED;
+}
 //
 // Set Request to the unified request string.
 //
 ConfigRequest = Request;
 //
@@ -884,10 +889,14 @@ RouteConfig (
   //
   if (HiiIsConfigHdrMatch(Configuration, , MyEfiVar)) 
{
 return EFI_UNSUPPORTED;
   }
 
+  if (HiiIsConfigHdrMatch(Configuration, , 
MyUnionVar)) {
+return EFI_UNSUPPORTED;
+  }
+
   //
   // Get Buffer Storage data from EFI variable
   //
   BufferSize = sizeof (DRIVER_SAMPLE_CONFIGURATION);
   Status = gRT->GetVariable (
@@ -1685,10 +1694,11 @@ DriverSampleInit (
   EFI_STRING  ConfigRequestHdr;
   EFI_STRING  NameRequestHdr;
   MY_EFI_VARSTORE_DATA*VarStoreConfig;
   EFI_INPUT_KEY   HotKey;
   EDKII_FORM_BROWSER_EXTENSION_PROTOCOL *FormBrowserEx;
+  MY_UNION_DATA   *UnionConfig;
 
   //
   // Initialize the local variables.
   //
   ConfigRequestHdr = NULL;
@@ -1962,10 +1972,57 @@ DriverSampleInit (
   return EFI_INVALID_PARAMETER;
 }
   }
   FreePool (ConfigRequestHdr);
 
+  //
+  // Initialize Union efi varstore configuration data
+  //
+  UnionConfig = >UnionConfig;
+  ZeroMem (UnionConfig, sizeof (MY_UNION_DATA));
+
+  ConfigRequestHdr = HiiConstructConfigHdr (, 
MyUnionVar, DriverHandle[0]);
+  ASSERT (ConfigRequestHdr != NULL);
+
+  BufferSize = sizeof (MY_UNION_DATA);
+  Status = gRT->GetVariable (MyUnionVar, , NULL, 
, UnionConfig);
+  if (EFI_ERROR (Status)) {
+//
+// Store zero data to EFI variable Storage.
+//
+Status = gRT->SetVariable(
+MyUnionVar,
+,
+EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS,
+sizeof (MY_UNION_DATA),
+UnionConfig
+);
+if (EFI_ERROR (Status)) {
+  DriverSampleUnload (ImageHandle);
+  return Status;
+}
+//
+// EFI variable for NV config doesn't exit, we should build this variable
+// based on default values stored in IFR
+//
+ActionFlag = HiiSetToDefaults (ConfigRequestHdr, 
EFI_HII_DEFAULT_CLASS_STANDARD);
+if (!ActionFlag) {
+  DriverSampleUnload (ImageHandle);
+  return EFI_INVALID_PARAMETER;
+}
+  } else {
+//
+// EFI variable does exist and Validate Current Setting
+//
+ActionFlag = HiiValidateSettings (ConfigRequestHdr);
+if (!ActionFlag) {
+  DriverSampleUnload (ImageHandle);
+  return EFI_INVALID_PARAMETER;
+}
+  }
+  FreePool (ConfigRequestHdr);
+
   Status = gBS->CreateEventEx (
 EVT_NOTIFY_SIGNAL, 
 TPL_NOTIFY,
 EfiEventEmptyFunction,
 NULL,
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h
index 6c97239..895547a 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.h
@@ -82,10 +82,11 @@ typedef struct {
 
   EFI_HANDLE   DriverHandle[2];
   EFI_HII_HANDLE   HiiHandle[2];
   DRIVER_SAMPLE_CONFIGURATION  Configuration;
   MY_EFI_VARSTORE_DATA VarStoreConfig;
+  MY_UNION_DATAUnionConfig;
 
   //
   // Name/Value storage Name list
   //
   EFI_STRING_IDNameStringId[NAME_VALUE_NAME_NUMBER];
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h 

[edk2] [RFC 0/2] Support Union type in EFI/Buffer VarStore

2017-06-04 Thread Dandan Bi
This serie is the POC to update VfrCompiler to support Union type
in VarStore and construct the use cases in DriverSample.

Cc: Eric Dong 
Cc: Liming Gao 
Dandan Bi (2):
  BaseTool/VfrCompile: Support Union type in VFR
  MdeModulePkg/DriverSample: Add sample questions to refer union type

 BaseTools/Source/C/VfrCompile/VfrSyntax.g  | 17 +++
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp| 13 -
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h  |  1 +
 .../Universal/DriverSampleDxe/DriverSample.c   | 57 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |  1 +
 .../Universal/DriverSampleDxe/NVDataStruc.h| 18 +++
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 25 ++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |  4 ++
 8 files changed, 135 insertions(+), 1 deletion(-)

-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel