[edk2] Accessing RT services from OS

[Ankit.Singh3]

Hi All,

Wanted to access RT services from OS.
1.) Are there any already such exposed OS function or utilities ?
2.) Can we plugin our own service/function to RT at run-time. ?

Regards,
Ankit Singh

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

Re: [edk2] [patch 1/6] BaseTool/VfrCompile: Support Union type in VFR

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: Bi, Dandan 
Sent: Monday, September 4, 2017 3:16 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Gao, Liming 
Subject: [patch 1/6] BaseTool/VfrCompile: Support Union type in VFR

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

Update VfrCompiler to parse the UNION type in vfr file

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

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 406dbc5..24e0538 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,140 +320,155 @@ vfrPragmaPackDefinition :
 | pragmaPackNumber
   }
   "\)"
   ;
 
+  vfrDataUnionDefinition :
+  { TypeDef } Union<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
+  { NonNvDataMap }
+  {
+N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
+  }
+  OpenBrace
+vfrDataStructFields[TRUE]
+  CloseBrace
+  {
+N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
+  }
+  ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd ();>>
+  ;
+
 vfrDataStructDefinition :
   { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
   { NonNvDataMap }
   {
 N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
   }
   OpenBrace
-vfrDataStructFields
+vfrDataStructFields[FALSE]
   CloseBrace
   {
 N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
   }
   ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd (); >>
   ;
 
-vfrDataStructFields :
+vfrDataStructFields [BOOLEAN  FieldInUnion]:
   (
- dataStructField64 |
- dataStructField32 |
- dataStructField16 |
- dataStructField8  |
- dataStructFieldBool   |
- dataStructFieldString |
- dataStructFieldDate   |
- dataStructFieldTime   |
- dataStructFieldRef|
- dataStructFieldUser
+ dataStructField64 [FieldInUnion]|
+ dataStructField32 [FieldInUnion]|
+ dataStructField16 [FieldInUnion]|
+ dataStructField8  [FieldInUnion]|
+ dataStructFieldBool [FieldInUnion]  |
+ dataStructFieldString [FieldInUnion]|
+ dataStructFieldDate  [FieldInUnion] |
+ dataStructFieldTime  [FieldInUnion] |
+ dataStructFieldRef   [FieldInUnion] |
+ dataStructFieldUser [FieldInUnion]
   )*
   ;
 
-dataStructField64 :
+dataStructField64 [BOOLEAN  FieldInUnion]:
   << UINT32 ArrayNum = 0; >>
   D:"UINT64"
   N:StringIdentifier
   {
 OpenBracket I:Number CloseBracket   << ArrayNum = 
_STOU32(I->getText(), I->getLine()); >>
   }
-  ";"   << 
_PCATCH(gCVfrVarDataTypeDB.DataTypeAddField (N->getText(), D->getText(), 
ArrayNum), N); >>
+  ";"   << 
_PCATCH(gCVfrVarDataTypeDB.DataTypeAddField (N->getText(), D->getText(), 
ArrayNum, FieldInUnion), N); >>
   ;
 
-dataStructField32 :
+dataStructField32 [BOOLEAN  FieldInUnion]:
   << UINT32 ArrayNum = 0; >>
   D:"UINT32"
   N:StringIdentifier
   {
 OpenBracket I:Number CloseBracket   << ArrayNum = 
_STOU32(I->getText(), I->getLine()); >>
   }
-  ";" 

Re: [edk2] [patch 0/6] Support bitfield and union type in vfr

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: Bi, Dandan 
Sent: Monday, September 4, 2017 3:16 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Gao, Liming 
Subject: [patch 0/6] Support bitfield and union type in vfr

This patch series is to:
(1) Update VfrCompiler to parse UNION type, bit field in data structure.
(2) Update HiiDatabase and SetupBrowser to handle questions with Bit VarStore.
(3) Add sample questions with bit/union VarStore in DriverSample.

Cc: Eric Dong 
Cc: Liming Gao 

Dandan Bi (6):
  BaseTool/VfrCompile: Support Union type in VFR
  BaseTool/VfrCompiler: Support Bit fields in EFI/Buffer VarStore
  MdeModulePkg: Add GUID/flags to implement BitFiled support
  MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore
  MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore
  MdeModulePkg/DriverSample: Add sample questions with bit/union
VarStore

 BaseTools/Source/C/Include/Common/MdeModuleHii.h   |   12 +-
 BaseTools/Source/C/VfrCompile/VfrError.cpp |3 +-
 BaseTools/Source/C/VfrCompile/VfrError.h   |3 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp   |4 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h |   52 +-
 BaseTools/Source/C/VfrCompile/VfrSyntax.g  | 1240 
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp|  258 +++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h  |   28 +-
 MdeModulePkg/Include/Guid/MdeModuleHii.h   |   19 +-
 MdeModulePkg/MdeModulePkg.dec  |4 +
 .../Universal/DriverSampleDxe/DriverSample.c   |  120 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |2 +
 .../Universal/DriverSampleDxe/NVDataStruc.h|   51 +-
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr |  187 +++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |   61 +
 .../Universal/HiiDatabaseDxe/ConfigRouting.c   |  190 ++-
 .../Universal/HiiDatabaseDxe/HiiDatabase.h |6 +-
 .../Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf|3 +-
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c  |  142 ++-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c |  120 +-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.h |3 +
 .../Universal/SetupBrowserDxe/SetupBrowserDxe.inf  |3 +-
 22 files changed, 1961 insertions(+), 550 deletions(-)

-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 0/2] Enable IOMMU for CSMM

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiewen 
Yao
Sent: Tuesday, September 5, 2017 11:30 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 0/2] Enable IOMMU for CSMM

This serial patch is to enable IOMMU support for CSM.

Jiewen Yao (2):
  IntelSiliconPkg/Vtd: Support CSM usage.
  IntelFramdworkModulePkg/LegacyBios: Add IoMmu Support.

 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf |  1 +
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h |  1 +
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c   | 72 
+++-
 IntelSiliconPkg/IntelVTdDxe/BmDma.c |  8 +--
 IntelSiliconPkg/IntelVTdDxe/TranslationTable.c  |  2 +-
 5 files changed, 78 insertions(+), 6 deletions(-)

-- 
2.7.4.windows.1

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

[edk2] [PATCH 1/2] IntelSiliconPkg/Vtd: Support CSM usage.

[Jiewen Yao]

Remove zero address check in IoMmuMap.
The reason is that a CSM legacy driver may use legacy memory for DMA.
As such, the legacyBios need allow below 1M to the legacy device.

This patch also fixed some typo.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 IntelSiliconPkg/IntelVTdDxe/BmDma.c| 8 
 IntelSiliconPkg/IntelVTdDxe/TranslationTable.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelSiliconPkg/IntelVTdDxe/BmDma.c 
b/IntelSiliconPkg/IntelVTdDxe/BmDma.c
index 5dcee00..7a5f361 100644
--- a/IntelSiliconPkg/IntelVTdDxe/BmDma.c
+++ b/IntelSiliconPkg/IntelVTdDxe/BmDma.c
@@ -77,14 +77,14 @@ IoMmuMap (
   EFI_PHYSICAL_ADDRESS  DmaMemoryTop;
   BOOLEAN   NeedRemap;
 
-  DEBUG ((DEBUG_VERBOSE, "IoMmuMap: ==> 0x%08x - 0x%08x (%x)\n", HostAddress, 
NumberOfBytes, Operation));
-
-  if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
+  if (NumberOfBytes == NULL || DeviceAddress == NULL ||
   Mapping == NULL) {
 DEBUG ((DEBUG_ERROR, "IoMmuMap: %r\n", EFI_INVALID_PARAMETER));
 return EFI_INVALID_PARAMETER;
   }
 
+  DEBUG ((DEBUG_VERBOSE, "IoMmuMap: ==> 0x%08x - 0x%08x (%x)\n", HostAddress, 
*NumberOfBytes, Operation));
+
   //
   // Make sure that Operation is valid
   //
@@ -135,7 +135,7 @@ IoMmuMap (
 if (NeedRemap) {
   //
   // Common Buffer operations can not be remapped.  If the common buffer
-  // if above 4GB, then it is not possible to generate a mapping, so return
+  // is above 4GB, then it is not possible to generate a mapping, so return
   // an error.
   //
   DEBUG ((DEBUG_ERROR, "IoMmuMap: %r\n", EFI_UNSUPPORTED));
diff --git a/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c 
b/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c
index cd3111c..ccecc95 100644
--- a/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c
+++ b/IntelSiliconPkg/IntelVTdDxe/TranslationTable.c
@@ -891,7 +891,7 @@ SetAccessAttribute (
 
   SecondLevelPagingEntry = NULL;
 
-  DEBUG ((DEBUG_INFO,"SetAccessAttribute (S%04x B%02x D%02x F%02x) (0x%016lx - 
0x%08x, %x)\n", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, 
SourceId.Bits.Function, BaseAddress, (UINTN)Length, IoMmuAccess));
+  DEBUG ((DEBUG_VERBOSE,"SetAccessAttribute (S%04x B%02x D%02x F%02x) 
(0x%016lx - 0x%08x, %x)\n", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, 
SourceId.Bits.Function, BaseAddress, (UINTN)Length, IoMmuAccess));
 
   VtdIndex = FindVtdIndexByPciDevice (Segment, SourceId, , 
);
   if (VtdIndex == (UINTN)-1) {
-- 
2.7.4.windows.1

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

[edk2] [PATCH 2/2] IntelFramdworkModulePkg/LegacyBios: Add IoMmu Support.

[Jiewen Yao]

If IOMMU is enabled, the legacy BIOS need allow the legacy memory
access by the legacy device.
The legacy memory is below 1M memory and HighPmm memory.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf |  1 +
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h |  1 +
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c   | 72 
+++-
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
index 4ca412a..48473a0 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
@@ -137,6 +137,7 @@
   gEfiLegacyBiosProtocolGuid## PRODUCES
   gEfiSerialIoProtocolGuid  ## CONSUMES
   gEfiSioProtocolGuid   ## CONSUMES
+  gEdkiiIoMmuProtocolGuid   ## CONSUMES
 
 [Pcd]
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLegacyBiosCacheLegacyRegion  ## 
CONSUMES
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
index 069646b..fe9dd74 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
@@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c 
b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
index c4c77ec..8ffdf0c 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
@@ -41,7 +41,7 @@ BOOLEAN mIgnoreBbsUpdateFlag;
 BOOLEAN mVgaInstallationInProgress  = FALSE;
 UINT32  mRomCount   = 0x00;
 ROM_INSTANCE_ENTRY  mRomEntry[ROM_MAX_ENTRIES];
-
+EDKII_IOMMU_PROTOCOL*mIoMmu;
 
 /**
   Query shadowed legacy ROM parameters registered by RomShadow() previously.
@@ -2697,6 +2697,61 @@ Done:
 }
 
 /**
+  Let IOMMU grant DMA access for the PCI device.
+
+  @param  PciHandle The EFI handle for the PCI device.
+  @param  HostAddress   The system memory address to map to the PCI 
controller.
+  @param  NumberOfBytes The number of bytes to map.
+
+  @retval EFI_SUCCESS  The DMA access is granted.
+**/
+EFI_STATUS
+IoMmuGrantAccess (
+  IN  EFI_HANDLEPciHandle,
+  IN  EFI_PHYSICAL_ADDRESS  HostAddress,
+  IN  UINTN NumberOfBytes
+  )
+{
+  EFI_PHYSICAL_ADDRESSDeviceAddress;
+  VOID*Mapping;
+  EFI_STATUS  Status;
+
+  if (PciHandle == NULL) {
+return EFI_UNSUPPORTED;
+  }
+
+  Status = EFI_SUCCESS;
+  if (mIoMmu == NULL) {
+gBS->LocateProtocol (, NULL, (VOID **));
+  }
+  if (mIoMmu != NULL) {
+Status = mIoMmu->Map (
+   mIoMmu,
+   EdkiiIoMmuOperationBusMasterCommonBuffer,
+   (VOID *)(UINTN)HostAddress,
+   ,
+   ,
+   
+   );
+if (EFI_ERROR(Status)) {
+  DEBUG ((DEBUG_ERROR, "LegacyPci - IoMmuMap - %r\n", Status));
+} else {
+  ASSERT (DeviceAddress == HostAddress);
+  Status = mIoMmu->SetAttribute (
+ mIoMmu,
+ PciHandle,
+ Mapping,
+ EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE
+ );
+  if (EFI_ERROR(Status)) {
+DEBUG ((DEBUG_ERROR, "LegacyPci - IoMmuSetAttribute - %r\n", Status));
+  }
+}
+  }
+  return Status;
+}
+
+/**
   Load a legacy PC-AT OPROM on the PciHandle device. Return information
   about how many disks were added by the OPROM and the shadow address and
   size. DiskStart & DiskEnd are INT 13h drive letters. Thus 0x80 is C:
@@ -2978,6 +3033,21 @@ LegacyBiosInstallPciRom (
   RuntimeImageLength = Pcir->MaxRuntimeImageLength * 512;
 }
   }
+
+  //
+  // Grant access for below 1M
+  // BDA/EBDA/LowPMM and scratch memory for OPROM.
+  //
+  IoMmuGrantAccess (PciHandle, 0, SIZE_1MB);
+  //
+  // Grant access for HiPmm
+  //
+  IoMmuGrantAccess (
+PciHandle,
+Private->IntThunk->EfiToLegacy16InitTable.HiPmmMemory,
+Private->IntThunk->EfiToLegacy16InitTable.HiPmmMemorySizeInBytes
+);
+
   //
   // Shadow and initialize the OpROM.
   //
-- 
2.7.4.windows.1

___

[edk2] [PATCH 0/2] Enable IOMMU for CSMM

[Jiewen Yao]

This serial patch is to enable IOMMU support for CSM.

Jiewen Yao (2):
  IntelSiliconPkg/Vtd: Support CSM usage.
  IntelFramdworkModulePkg/LegacyBios: Add IoMmu Support.

 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf |  1 +
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h |  1 +
 IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c   | 72 
+++-
 IntelSiliconPkg/IntelVTdDxe/BmDma.c |  8 +--
 IntelSiliconPkg/IntelVTdDxe/TranslationTable.c  |  2 +-
 5 files changed, 78 insertions(+), 6 deletions(-)

-- 
2.7.4.windows.1

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

Re: [edk2] [Patch] UefiCpuPkg/Lmce.c Remove useless file.

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Dong, Eric
> Sent: Friday, September 1, 2017 1:59 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: [Patch] UefiCpuPkg/Lmce.c Remove useless file.
> 
> Merge the code to MachineCheck.c file, remove this file.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/CpuCommonFeaturesLib/Lmce.c | 97 --
> 
>  1 file changed, 97 deletions(-)
>  delete mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/Lmce.c
> 
> diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Lmce.c
> b/UefiCpuPkg/Library/CpuCommonFeaturesLib/Lmce.c
> deleted file mode 100644
> index bf7e26b..000
> --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/Lmce.c
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -/** @file
> -  Local machine check exception feature.
> -
> -  Copyright (c) 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
> -  http://opensource.org/licenses/bsd-license.php
> -
> -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#include "CpuCommonFeatures.h"
> -
> -/**
> -  Detects if Local machine check exception feature supported on current
> -  processor.
> -
> -  @param[in]  ProcessorNumber  The index of the CPU executing this
> function.
> -  @param[in]  CpuInfo  A pointer to the
> REGISTER_CPU_FEATURE_INFORMATION
> -   structure for the CPU executing this function.
> -  @param[in]  ConfigData   A pointer to the configuration buffer returned
> -   by CPU_FEATURE_GET_CONFIG_DATA.  NULL if
> -   CPU_FEATURE_GET_CONFIG_DATA was not provided 
> in
> -   RegisterCpuFeature().
> -
> -  @retval TRUE Local machine check exception feature is supported.
> -  @retval FALSELocal machine check exception feature is not supported.
> -
> -  @note This service could be called by BSP/APs.
> -**/
> -BOOLEAN
> -EFIAPI
> -LmceSupport (
> -  IN UINTN ProcessorNumber,
> -  IN REGISTER_CPU_FEATURE_INFORMATION  *CpuInfo,
> -  IN VOID  *ConfigData  OPTIONAL
> -  )
> -{
> -  MSR_IA32_MCG_CAP_REGISTERMcgCap;
> -
> -  if (!McaSupport (ProcessorNumber, CpuInfo, ConfigData)) {
> -return FALSE;
> -  }
> -
> -  McgCap.Uint64 = AsmReadMsr64 (MSR_IA32_MCG_CAP);
> -  if (ProcessorNumber == 0) {
> -DEBUG ((EFI_D_INFO, "LMCE eanble = %x\n", (BOOLEAN)
> (McgCap.Bits.MCG_LMCE_P != 0)));
> -  }
> -  return (BOOLEAN) (McgCap.Bits.MCG_LMCE_P != 0); -}
> -
> -/**
> -  Initializes Local machine check exception feature to specific state.
> -
> -  @param[in]  ProcessorNumber  The index of the CPU executing this
> function.
> -  @param[in]  CpuInfo  A pointer to the
> REGISTER_CPU_FEATURE_INFORMATION
> -   structure for the CPU executing this function.
> -  @param[in]  ConfigData   A pointer to the configuration buffer returned
> -   by CPU_FEATURE_GET_CONFIG_DATA.  NULL if
> -   CPU_FEATURE_GET_CONFIG_DATA was not provided 
> in
> -   RegisterCpuFeature().
> -  @param[in]  StateIf TRUE, then the Local machine check 
> exception
> -   feature must be enabled.
> -   If FALSE, then the Local machine check 
> exception
> -   feature must be disabled.
> -
> -  @retval RETURN_SUCCESS   Local machine check exception feature is
> initialized.
> -
> -**/
> -RETURN_STATUS
> -EFIAPI
> -LmceInitialize (
> -  IN UINTN ProcessorNumber,
> -  IN REGISTER_CPU_FEATURE_INFORMATION  *CpuInfo,
> -  IN VOID  *ConfigData,  OPTIONAL
> -  IN BOOLEAN   State
> -  )
> -{
> -  MSR_IA32_FEATURE_CONTROL_REGISTER*MsrRegister;
> -
> -  ASSERT (ConfigData != NULL);
> -  MsrRegister = (MSR_IA32_FEATURE_CONTROL_REGISTER *) ConfigData;
> -  if (MsrRegister[ProcessorNumber].Bits.Lock == 0) {
> -CPU_REGISTER_TABLE_WRITE_FIELD (
> -  ProcessorNumber,
> -  Msr,
> -  MSR_IA32_FEATURE_CONTROL,
> -  MSR_IA32_FEATURE_CONTROL_REGISTER,
> -  Bits.LmceOn,
> -  (State) ? 1 : 0
> -  );
> -  }
> -  return RETURN_SUCCESS;
> -}
> --
> 2.7.0.windows.1

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

Re: [edk2] [Patch] DSC Spec: Hii type Pcd VariableName is NOT allowed to be empty string

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Zhu, Yonghong
> Sent: Tuesday, September 5, 2017 11:09 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Kinney, Michael D 
> ; Shaw, Kevin W 
> Subject: [Patch] DSC Spec: Hii type Pcd VariableName is NOT allowed to be 
> empty string
> 
> fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=597
> Cc: Liming Gao 
> Cc: Michael Kinney 
> Cc: Kevin W Shaw 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  2_dsc_overview/29_pcd_sections.md| 4 
>  3_edk_ii_dsc_file_format/310_pcd_sections.md | 6 ++
>  README.md| 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/2_dsc_overview/29_pcd_sections.md 
> b/2_dsc_overview/29_pcd_sections.md
> index 83689a9..6547656 100644
> --- a/2_dsc_overview/29_pcd_sections.md
> +++ b/2_dsc_overview/29_pcd_sections.md
> @@ -250,10 +250,12 @@ described in the following table.
>  | `NV`| `EFI_VARIABLE_NON_VOLATILE` | `0x0001` |
>  | `BS`| `EFI_VARIABLE_BOOTSERVICE_ACCESS`   | `0x0002` |
>  | `RT`| `EFI_VARIABLE_RUNTIME_ACCESS`   | `0x0004` |
>  | `RO`| `VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY` | `BIT0`   |
> 
> +**Note:** The VariableName field in the HII format PCD entry must not be an 
> empty string.
> +
>   Examples
> 
>  ```ini
>  [PcdsDynamicHii.common.Sku_Two]
>NoSuchTokenSpaceGuid.PcdPreAllocatedMem| 0x0053 0x0065 0x0074 0x0075 
> 0x0070|gSysconfigGuid|0x00A9|0x3
> @@ -345,10 +347,12 @@ backslash character is used here to indicate the 
> continuation of the line:
>  
> `PcdTokenSpaceGuidCName.PcdCName|VariableName|VariableGuid|VariableOffset[|HiiDefaultValue]`
> 
>  The optional HII Attribute entry is a comma separated list of attributes as
>  described in Table 9 HII Attributes.
> 
> +**Note:** The VariableName field in the HII format PCD entry must not be an 
> empty string.
> +
>   Examples
> 
>  ```ini
>  [PcdsDynamicExHii.IA32.Sku_Two]
>gNoSuchTokenSpaceGuid.PcdPreAllocatedMem|0x0053 0x0065 0x0074 0x0075 
> 0x0070|gSysconfigGuid|0x00A9|0x11
> diff --git a/3_edk_ii_dsc_file_format/310_pcd_sections.md 
> b/3_edk_ii_dsc_file_format/310_pcd_sections.md
> index c061e5b..2af42cc 100644
> --- a/3_edk_ii_dsc_file_format/310_pcd_sections.md
> +++ b/3_edk_ii_dsc_file_format/310_pcd_sections.md
> @@ -514,10 +514,13 @@ of the offset.
> 
>  A non-negative numeric value that is the number of bytes from the start to 
> the
>  start of this variable. The offset value must not exceed the maximum value of
>  0x (`UINT16`).
> 
> +**_HiiString_**
> +The HiiString field in the HII format PCD entry must not be an empty string.
> +
>  **_SkuIds_**
> 
>  `SkuId`s in the DSC file can be used in two different ways. They can be used 
> to
>  as conditional modifiers to exclude some content from a build, or they can be
>  used to identify and group content during a build. If no `SkuId` option 
> (`-x`)
> @@ -671,10 +674,13 @@ of the offset.
> 
>  A non-negative numeric value that is the number of bytes from the start to 
> the
>  start of this variable. The offset value must not exceed the maximum value of
>  0x (`UINT16`).
> 
> +**_HiiString_**
> +The HiiString field in the HII format PCD entry must not be an empty string.
> +
>  **_SkuIds_**
> 
>  `SkuId`s in the DSC file can be used in two different ways. They can be used 
> to
>  as conditional modifiers to exclude some content from a build, or they can be
>  used to identify and group content during a build. If no `SkuId` option 
> (`-x`)
> diff --git a/README.md b/README.md
> index 270857e..dd3419a 100644
> --- a/README.md
> +++ b/README.md
> @@ -183,5 +183,6 @@ Copyright (c) 2006-2017, Intel Corporation. All rights 
> reserved.
>  || [#521](https://bugzilla.tianocore.org/show_bug.cgi?id=521) 
> DSC spec: add clarification for mixed PCD usage in the
> DSC spec
> ||
>  || [#519](https://bugzilla.tianocore.org/show_bug.cgi?id=519) 
> DSC Spec: update Precedence of PCD Values
> ||
>  || [#584](https://bugzilla.tianocore.org/show_bug.cgi?id=584) 
> DSC Spec: Update the DSC_SPECIFICATION version to
> 0x0001001B or 1.27
> ||
>  || [#645](https://bugzilla.tianocore.org/show_bug.cgi?id=645) 
> DSC Spec: Remove Restriction on Using NULL in
> [LibraryClasses] Section
> ||
>  || [#669](https://bugzilla.tianocore.org/show_bug.cgi?id=669) 
> DSC Spec: Add multi-arg support to
> PREBUILD/POSTBUILD
> ||
> +|| [#597](https://bugzilla.tianocore.org/show_bug.cgi?id=597) 
> DSC Spec: Hii type Pcd VariableName is NOT allowed to
> be empty string
> | 

[edk2] [Patch] DSC Spec: Hii type Pcd VariableName is NOT allowed to be empty string

[Yonghong Zhu]

fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=597
Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 2_dsc_overview/29_pcd_sections.md| 4 
 3_edk_ii_dsc_file_format/310_pcd_sections.md | 6 ++
 README.md| 1 +
 3 files changed, 11 insertions(+)

diff --git a/2_dsc_overview/29_pcd_sections.md 
b/2_dsc_overview/29_pcd_sections.md
index 83689a9..6547656 100644
--- a/2_dsc_overview/29_pcd_sections.md
+++ b/2_dsc_overview/29_pcd_sections.md
@@ -250,10 +250,12 @@ described in the following table.
 | `NV`| `EFI_VARIABLE_NON_VOLATILE` | `0x0001` |
 | `BS`| `EFI_VARIABLE_BOOTSERVICE_ACCESS`   | `0x0002` |
 | `RT`| `EFI_VARIABLE_RUNTIME_ACCESS`   | `0x0004` |
 | `RO`| `VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY` | `BIT0`   |
 
+**Note:** The VariableName field in the HII format PCD entry must not be an 
empty string.
+
  Examples
 
 ```ini
 [PcdsDynamicHii.common.Sku_Two]
   NoSuchTokenSpaceGuid.PcdPreAllocatedMem| 0x0053 0x0065 0x0074 0x0075 
0x0070|gSysconfigGuid|0x00A9|0x3
@@ -345,10 +347,12 @@ backslash character is used here to indicate the 
continuation of the line:
 
`PcdTokenSpaceGuidCName.PcdCName|VariableName|VariableGuid|VariableOffset[|HiiDefaultValue]`
 
 The optional HII Attribute entry is a comma separated list of attributes as
 described in Table 9 HII Attributes.
 
+**Note:** The VariableName field in the HII format PCD entry must not be an 
empty string.
+
  Examples
 
 ```ini
 [PcdsDynamicExHii.IA32.Sku_Two]
   gNoSuchTokenSpaceGuid.PcdPreAllocatedMem|0x0053 0x0065 0x0074 0x0075 
0x0070|gSysconfigGuid|0x00A9|0x11
diff --git a/3_edk_ii_dsc_file_format/310_pcd_sections.md 
b/3_edk_ii_dsc_file_format/310_pcd_sections.md
index c061e5b..2af42cc 100644
--- a/3_edk_ii_dsc_file_format/310_pcd_sections.md
+++ b/3_edk_ii_dsc_file_format/310_pcd_sections.md
@@ -514,10 +514,13 @@ of the offset.
 
 A non-negative numeric value that is the number of bytes from the start to the
 start of this variable. The offset value must not exceed the maximum value of
 0x (`UINT16`).
 
+**_HiiString_**
+The HiiString field in the HII format PCD entry must not be an empty string.
+
 **_SkuIds_**
 
 `SkuId`s in the DSC file can be used in two different ways. They can be used to
 as conditional modifiers to exclude some content from a build, or they can be
 used to identify and group content during a build. If no `SkuId` option (`-x`)
@@ -671,10 +674,13 @@ of the offset.
 
 A non-negative numeric value that is the number of bytes from the start to the
 start of this variable. The offset value must not exceed the maximum value of
 0x (`UINT16`).
 
+**_HiiString_**
+The HiiString field in the HII format PCD entry must not be an empty string.
+
 **_SkuIds_**
 
 `SkuId`s in the DSC file can be used in two different ways. They can be used to
 as conditional modifiers to exclude some content from a build, or they can be
 used to identify and group content during a build. If no `SkuId` option (`-x`)
diff --git a/README.md b/README.md
index 270857e..dd3419a 100644
--- a/README.md
+++ b/README.md
@@ -183,5 +183,6 @@ Copyright (c) 2006-2017, Intel Corporation. All rights 
reserved.
 || [#521](https://bugzilla.tianocore.org/show_bug.cgi?id=521) DSC 
spec: add clarification for mixed PCD usage in the DSC spec 

  ||
 || [#519](https://bugzilla.tianocore.org/show_bug.cgi?id=519) DSC 
Spec: update Precedence of PCD Values   

  ||
 || [#584](https://bugzilla.tianocore.org/show_bug.cgi?id=584) DSC 
Spec: Update the DSC_SPECIFICATION version to 0x0001001B or 1.27

  ||
 || [#645](https://bugzilla.tianocore.org/show_bug.cgi?id=645) DSC 
Spec: Remove Restriction on Using NULL in [LibraryClasses] Section  

  ||
 || [#669](https://bugzilla.tianocore.org/show_bug.cgi?id=669) DSC 
Spec: Add multi-arg support to PREBUILD/POSTBUILD   

  

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Yao, Jiewen]

HI Laszlo
Thank you very much to raise this DMA topic.

I have a chat with Star. We have couples of question, from high level to low 
level.
Hope you can clarify:


1)   About disable DMA

As you mentioned, in UEFI_Driver_Writer_Guide_V1.0.1_120308.pdf, 7.7 Adding the 
Exit Boot Services feature
=
Examples from the EDK II that use this feature are the PCI device drivers for 
USB Host
Controllers. Some USB Host Controllers are PCI Bus Masters that continuously 
access a
memory buffer to poll for operation requests. Access to this memory buffer by a 
USB
Host Controller may be required to boot an operation system, but this activity 
must be
terminated when the OS calls ExitBootServices(). The typical action in the Exit 
Boot
Services Event for these types of drivers is to disable the PCI bus master and 
place the
USB Host Controller into a halted state
=

I believe the fundamental requirement is to "disable DMA".
As such, the simplest action is to clear PCI Bus Master Enable bit in the 
command register.

Do you think clearing BME is enough?
Is there any special reason that we must unmap the DMA buffer?


2)   About common buffer
If there is a special requirement that we must unmap the DMA buffer, why we 
only unmap the common buffer?

I understand your statement that unmapping a read buffer or write buffer may 
cause free memory action. But that is implementation choice.

If we have strong requirement to unmap the DMA buffer, we should achieve that.


3)   About interaction with IOMMU
I am not worried about IOMMU interaction.
I believe an proper IOMMU implementation should not block any action taken by a 
PCI device driver.
Current IntelVTdDxe driver disables VTd engine in ExitBootService, which means 
the DMA access is no longer managed by VTd engine. It is NOT related to disable 
DMA buffer.


4)   About the driver you modified
I believe we have some other PCI device driver (NVMe/SDMMC/UFS), besides the 
ones you modified in this patch (ATA/UCHI/EHCI/XHCI).
If we need take this approach, I assume they also need update, right?

Thank you
Yao Jiewen


From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Tuesday, September 5, 2017 5:20 AM
To: Zeng, Star ; edk2-devel-01 
Cc: Dong, Eric 
Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common 
buffers at ExitBootServices()

On 09/04/17 12:36, Zeng, Star wrote:
> Curious about one question when I am reviewing this patch series.
>
> Does/Should the IOMMU component disable the address translation at 
> ExitBootServices?

It is a very good and interesting question.

My answer is, I don't know. The IOMMU protocol abstraction is specific
to edk2, and its behavior is not documented in the PI or UEFI specs (as
far as I know).

For one example, the VT-d IOMMU implementation in

  IntelSiliconPkg/IntelVTdDxe

zaps all translations at ExitBootServices():

VOID
EFIAPI
OnExitBootServices (
  IN EFI_EVENT   Event,
  IN VOID*Context
  )
{
  DEBUG ((DEBUG_INFO, "Vtd OnExitBootServices\n"));
  DumpVtdRegsAll ();
  DisableDmar ();
  DumpVtdRegsAll ();
}

Now, in case someone suggested that the same approach should be followed
by all IOMMU implementations, I'd have three counter-arguments:

(1) My series states the problem (and seeks to solve it) on the PciIo
protocol level, which is standardized. The IOMMU protocol is edk2-only.
I feel that this kind of "unmap on ExitBootServices" should be done by
all UEFI drivers that map DMA common buffers, regardless of
platform-specific PciIo implementations.

(2) ExitBootServices() notify functions are called in unspecified order
(there's no way to express dependencies between them). This means that,
following the above pattern, an IO address translation could be torn
down before a reliant PCI device is de-configured in its own UEFI_DRIVER
ExitBootServices() handler. The result can be a series of IOMMU faults.
I'm not sure if that's bad in practice, but it does look like a layering
violation. (We shouldn't tear down resources from the bottom up.)

(3) The DisableDmar() function is relatively simple. It uses VT-d
registers that exist independently of any PCI devices, and of any "live"
mappings.

Some IOMMU protocol implementations might be different -- for them the
live configuration might only be known by constantly tracking the full
set of Map() and Unmap() operations. That's a hurdle for the implementation.

Furthermore, what is supposed to happen if both the IOMMU driver and the
individual PCI driver destroy the mapping at ExitBootServices()? If the
PCI driver's notification function runs first, then the IOMMU driver can
mark the mapping as "unmapped", and ignore it in its own notification
function. If the IOMMU driver's notification function runs first, then
it can again only mark 

[edk2] [Patch][edk2-platforms 2/2] Change reset type.

[zwei4]

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: zwei4 
---
 Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Reset/RuntimeDxe/ScReset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Reset/RuntimeDxe/ScReset.c 
b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Reset/RuntimeDxe/ScReset.c
index 36ac0e3d0..2bac0a5e3 100644
--- a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Reset/RuntimeDxe/ScReset.c
+++ b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Reset/RuntimeDxe/ScReset.c
@@ -296,7 +296,7 @@ IntelScResetSystem (
 
 case EfiResetCold:
   InitialData = V_RST_CNT_HARDSTARTSTATE;
-  OutputData  = V_RST_CNT_FULLRESET;
+  OutputData  = V_RST_CNT_HARDRESET;
   break;
 
 case EfiResetShutdown:
-- 
2.14.1.windows.1

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

[edk2] [Patch][edk2-platforms 1/2] Disable PCIe De-emphasis.

[zwei4]

Some PCIe device, such as Intel8265NGW/8260NGW WiFi device, disappears after 
reboot. PCIe root port De-emphasis has to be disabled to fix this issue.

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: zwei4 
---
 .../Common/PlatformSettings/PlatformSetupDxe/SouthClusterConfig.vfi   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/SouthClusterConfig.vfi
 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/SouthClusterConfig.vfi
index 35aeb34bf..1b0e93dd9 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/SouthClusterConfig.vfi
+++ 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformSetupDxe/SouthClusterConfig.vfi
@@ -1236,8 +1236,8 @@ form formid   = PCIERP2_OPTIONS_FORM_ID,
 oneof varid   = Setup.PcieRootPortSelectableDeemphasis[OFFSET_1],
   prompt   = STRING_TOKEN (STR_PCH_PCIE_SD_PROMPT),
   help = STRING_TOKEN (STR_PCH_PCIE_SD_HELP),
-  option text = STRING_TOKEN(STR_DISABLE),  value = 0, flags = 
RESET_REQUIRED;
-  option text = STRING_TOKEN(STR_ENABLE),  value = 1, flags = DEFAULT | 
MANUFACTURING | RESET_REQUIRED;
+  option text = STRING_TOKEN(STR_DISABLE),  value = 0, flags = DEFAULT | 
MANUFACTURING | RESET_REQUIRED;
+  option text = STRING_TOKEN(STR_ENABLE),  value = 1, flags = 
RESET_REQUIRED;
 endoneof;
   endif;
 endform; //End of PCIERP2_OPTIONS_FORM_ID
-- 
2.14.1.windows.1

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

Re: [edk2] [PATCH 1/5] MdeModulePkg/UsbBusDxe: log warning message at DEBUG_WARN level

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, September 5, 2017 4:14 AM
To: edk2-devel-01 
Cc: Zeng, Star ; Dong, Eric 
Subject: [PATCH 1/5] MdeModulePkg/UsbBusDxe: log warning message at DEBUG_WARN 
level

"UsbSelectConfig: failed to connect driver %r, ignored" is an error message, 
but it states at once that the error condition will not affect the control 
flow. Degrade the report to DEBUG_WARN.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c 
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
index b0e6b835ac24..62645d9bdedb 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
@@ -440,7 +440,11 @@ UsbSelectConfig (
 Status = UsbConnectDriver (UsbIf);
 
 if (EFI_ERROR (Status)) {
-  DEBUG ((EFI_D_ERROR, "UsbSelectConfig: failed to connect driver %r, 
ignored\n", Status));
+  DEBUG ((
+DEBUG_WARN,
+"UsbSelectConfig: failed to connect driver %r, ignored\n",
+Status
+));
 }
   }
 
--
2.14.1.3.gb7cf6e02401b


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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for SD/eMMC

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, September 5, 2017 8:33 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ruiyu 
> Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot
> description for SD/eMMC
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=620
> 
> Adds the support for SD/eMMC device path to show as a boot option.
> 
> The CID register content (returned from DiskInfo->Inquiry) seems do not
> provide very useful/readable 'OEM/Application ID' and 'Product name'
> field.
> 
> For SD devices, the OID is a 2-character ASCII string and the Product name is 
> a
> 5-character ASCII string.
> 
> For eMMC devices, the OID is an 8-bit binary number and the Product name
> is a 6-character ASCII string.
> 
> These strings are relatively short and do not provide a very readable
> description. Hence, this commit uses general 'SD (eMMC) Device' for the
> boot option description.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 23
> ++
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> index 7647bac2d0..d56c3e7e2c 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> @@ -155,6 +155,7 @@ BmGetDescriptionFromDiskInfo (
>CONST UINTN  SerialNumberLength = 20;
>CHAR8*StrPtr;
>UINT8Temp;
> +  EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> 
>Description  = NULL;
> 
> @@ -229,6 +230,28 @@ BmGetDescriptionFromDiskInfo (
> 
>BmEliminateExtraSpaces (Description);
>  }
> +  } else if (CompareGuid (>Interface,
> )) {
> +DevicePath = DevicePathFromHandle (Handle);
> +if (DevicePath == NULL) {
> +  return NULL;
> +}
> +
> +while (!IsDevicePathEnd (DevicePath) && (DevicePathType
> (DevicePath) != MESSAGING_DEVICE_PATH)) {
> +  DevicePath = NextDevicePathNode (DevicePath);
> +}
> +if (IsDevicePathEnd (DevicePath)) {
> +  return NULL;
> +}
> +
> +if (DevicePathSubType (DevicePath) == MSG_SD_DP) {
> +  Description = L"SD Device";
> +} else if (DevicePathSubType (DevicePath) == MSG_EMMC_DP) {
> +  Description = L"eMMC Device";
> +} else {
> +  return NULL;
> +}
> +
> +Description = AllocateCopyPool (StrSize (Description),
> + Description);
>}
> 
>return Description;
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index 264d726c26..ad4901db57 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -91,6 +91,7 @@
>gEfiDiskInfoAhciInterfaceGuid ## SOMETIMES_CONSUMES ##
> GUID
>gEfiDiskInfoIdeInterfaceGuid  ## SOMETIMES_CONSUMES ## GUID
>gEfiDiskInfoScsiInterfaceGuid ## SOMETIMES_CONSUMES ## GUID
> +  gEfiDiskInfoSdMmcInterfaceGuid## SOMETIMES_CONSUMES ##
> GUID
> 
>  [Protocols]
>gEfiPciRootBridgeIoProtocolGuid   ## CONSUMES
> --
> 2.12.0.windows.1

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

Re: [edk2] [PATCH 1/2] MdePkg/BaseIoLibIntrinsic: fix SEV (=unrolled) variants of IoWriteFifoXX()

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Monday, September 04, 2017 11:57 PM
>To: edk2-devel-01 
>Cc: Kinney, Michael D ; Justen, Jordan L
>; Gao, Liming 
>Subject: [edk2] [PATCH 1/2] MdePkg/BaseIoLibIntrinsic: fix SEV (=unrolled)
>variants of IoWriteFifoXX()
>
>In commit b6d11d7c4678 ("MdePkg: BaseIoLibIntrinsic (IoLib class)
>library", 2017-04-12), the MOV instructions in the write loops were
>probably copied from the read loops. However, the operand order was not
>adjusted.
>
>As a result, the IoWriteFifoXX() routines, when invoked in SEV guests, now
>overwrite the source buffer with value 0x01 / 0x0001 / 0x0001 -- the
>SevNoRepIo() function returns value 1 in EAX, in SEV guests --, and write
>the same value to the target IO port.
>
>Fix this by putting the target operand (AL / AX / EAX) first, and the
>source operand (BYTE / WORD / DWORD [ESI/RSI]) second.
>
>Cc: Brijesh Singh 
>Cc: Jordan Justen 
>Cc: Liming Gao 
>Cc: Michael D Kinney 
>Fixes: b6d11d7c467810ea7f2e2eda46ef0bdc57bf1475
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Laszlo Ersek 
>---
> MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 6 +++---
> MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>index 3e80c17d04a3..4b2af807cff8 100644
>--- a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>+++ b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
>@@ -212,7 +212,7 @@ ASM_PFX(IoWriteFifo8):
> jecxz   @IoWriteFifo8_Done
>
> @IoWriteFifo8_Loop:
>-mov byte [esi], al
>+mov al, byte [esi]
> out dx, al
> inc esi
> loop@IoWriteFifo8_Loop
>@@ -250,7 +250,7 @@ ASM_PFX(IoWriteFifo16):
> jecxz   @IoWriteFifo16_Done
>
> @IoWriteFifo16_Loop:
>-mov word [esi], ax
>+mov ax, word [esi]
> out dx, ax
> add esi, 2
> loop@IoWriteFifo16_Loop
>@@ -288,7 +288,7 @@ ASM_PFX(IoWriteFifo32):
> jecxz   @IoWriteFifo32_Done
>
> @IoWriteFifo32_Loop:
>-mov dword [esi], eax
>+mov eax, dword [esi]
> out dx, eax
> add esi, 4
> loop@IoWriteFifo32_Loop
>diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>index 26e016625b72..4d86a6cd5330 100644
>--- a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>+++ b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>@@ -205,7 +205,7 @@ ASM_PFX(IoWriteFifo8):
> jrcxz   @IoWriteFifo8_Done
>
> @IoWriteFifo8_Loop:
>-mov byte [rsi], al
>+mov al, byte [rsi]
> out dx, al
> inc rsi
> loop@IoWriteFifo8_Loop
>@@ -241,7 +241,7 @@ ASM_PFX(IoWriteFifo16):
> jrcxz   @IoWriteFifo16_Done
>
> @IoWriteFifo16_Loop:
>-mov word [rsi], ax
>+mov ax, word [rsi]
> out dx, ax
> add rsi, 2
> loop@IoWriteFifo16_Loop
>@@ -277,7 +277,7 @@ ASM_PFX(IoWriteFifo32):
> jrcxz   @IoWriteFifo32_Done
>
> @IoWriteFifo32_Loop:
>-mov dword [rsi], eax
>+mov eax, dword [rsi]
> out dx, eax
> add rsi, 4
> loop@IoWriteFifo32_Loop
>--
>2.14.1.3.gb7cf6e02401b
>
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for SD/eMMC

[Hao Wu]

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

Adds the support for SD/eMMC device path to show as a boot option.

The CID register content (returned from DiskInfo->Inquiry) seems do not
provide very useful/readable 'OEM/Application ID' and 'Product name'
field.

For SD devices, the OID is a 2-character ASCII string and the Product name
is a 5-character ASCII string.

For eMMC devices, the OID is an 8-bit binary number and the Product name
is a 6-character ASCII string.

These strings are relatively short and do not provide a very readable
description. Hence, this commit uses general 'SD (eMMC) Device' for the
boot option description.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 .../Library/UefiBootManagerLib/BmBootDescription.c | 23 ++
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |  1 +
 2 files changed, 24 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
index 7647bac2d0..d56c3e7e2c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
@@ -155,6 +155,7 @@ BmGetDescriptionFromDiskInfo (
   CONST UINTN  SerialNumberLength = 20;
   CHAR8*StrPtr;
   UINT8Temp;
+  EFI_DEVICE_PATH_PROTOCOL *DevicePath;
 
   Description  = NULL;
 
@@ -229,6 +230,28 @@ BmGetDescriptionFromDiskInfo (
 
   BmEliminateExtraSpaces (Description);
 }
+  } else if (CompareGuid (>Interface, 
)) {
+DevicePath = DevicePathFromHandle (Handle);
+if (DevicePath == NULL) {
+  return NULL;
+}
+
+while (!IsDevicePathEnd (DevicePath) && (DevicePathType (DevicePath) != 
MESSAGING_DEVICE_PATH)) {
+  DevicePath = NextDevicePathNode (DevicePath);
+}
+if (IsDevicePathEnd (DevicePath)) {
+  return NULL;
+}
+
+if (DevicePathSubType (DevicePath) == MSG_SD_DP) {
+  Description = L"SD Device";
+} else if (DevicePathSubType (DevicePath) == MSG_EMMC_DP) {
+  Description = L"eMMC Device";
+} else {
+  return NULL;
+}
+
+Description = AllocateCopyPool (StrSize (Description), Description);
   }
 
   return Description;
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf 
b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index 264d726c26..ad4901db57 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -91,6 +91,7 @@
   gEfiDiskInfoAhciInterfaceGuid ## SOMETIMES_CONSUMES ## GUID
   gEfiDiskInfoIdeInterfaceGuid  ## SOMETIMES_CONSUMES ## GUID
   gEfiDiskInfoScsiInterfaceGuid ## SOMETIMES_CONSUMES ## GUID
+  gEfiDiskInfoSdMmcInterfaceGuid## SOMETIMES_CONSUMES ## GUID
 
 [Protocols]
   gEfiPciRootBridgeIoProtocolGuid   ## CONSUMES
-- 
2.12.0.windows.1

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

Re: [edk2] [PATCH 2/5] UefiCpuPkg/CpuDxe: log informative message at DEBUG_INFO level

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Tuesday, September 5, 2017 4:14 AM
To: edk2-devel-01 
Cc: Dong, Eric 
Subject: [edk2] [PATCH 2/5] UefiCpuPkg/CpuDxe: log informative message at 
DEBUG_INFO level

"Detect CPU count: %d\n" is an informative message, not an error report.
Set its debug mask to DEBUG_INFO.

Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 UefiCpuPkg/CpuDxe/CpuMp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c index 
372c1e3ce4de..b3c0178d0708 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -622,7 +622,7 @@ InitializeMpSupport (
 
   MpInitLibGetNumberOfProcessors (, 
);
   mNumberOfProcessors = NumberOfProcessors;
-  DEBUG ((DEBUG_ERROR, "Detect CPU count: %d\n", mNumberOfProcessors));
+  DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors));
 
   //
   // Update CPU healthy information from Guided HOB
--
2.14.1.3.gb7cf6e02401b


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

Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

[Laszlo Ersek]

On 09/03/17 22:49, Laszlo Ersek wrote:

> I'll do some testing in the next few days.

Attempted to do it now, but the patch doesn't apply. I tried both

  git am --keep-cr ...

and

  git am --no-keep-cr ...

However, I managed to fetch the patch from the branch noted in the blurb.

I tested the patch with normal boot and S3 resume:
- IA32 build, Q35, SMM
- IA32X64 build, Q35, SMM
- X64 build, i440fx, no SMM

Experienced no regressions. Please add the following to your v2:

Tested-by: Laszlo Ersek 

Also, contingent upon the requested updates to the commit message:

Reviewed-by: Laszlo Ersek 


Jordan, could you please push Ge Song's upcoming v2, if you are
satisfied with it?

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

Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

[Laszlo Ersek]

On 09/04/17 17:11, Ge Song wrote:
> On 09/04/2017 09:34 PM, Jordan Justen wrote:
> 
>> On 2017-09-03 16:55:36, Laszlo Ersek wrote:
>>> On 09/03/17 04:12, Ge Song wrote:
 In earlier PEI stage, temporary memory(Cache As Ram) is employed as
 stack
 and heap. We move them to the new room and do some relocation fixup
 when
 permanent memory becomes available. TemporaryRamMigration()
 is responsible for switching the stack.
>>> [...]
>>>
 Cc: Jordan Justen 
 Cc: Laszlo Ersek 
 Cc: Ard Biesheuvel 
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Ge Song 
 ---
   OvmfPkg/Sec/SecMain.c | 2 ++
   1 file changed, 2 insertions(+)
>>> (No more separate emails, I promise.)
>>>
>>> Before you send v2, can you please amend the patch like this:
>>>
>>>    git commit --amend --author='Ge Song '
>>>
>>> It's OK if you mail it out from another email address, but IMO the git
>>> authorship should match the first Signed-off-by.
>> I disagree with this. Unless the individual is well known to the EDK
>> II community as the owner of both email addresses, I think the author
>> needs to send it from their author email.
>>
>> I suppose they can send it from another email, and then reply from
>> their other email saying they contributed it under the Tianocore
>> Contribution Agreement, but that sounds like a stretch...

I'm fine either way, as long as we can establish some kind of agreement
between (two of?) the three email addresses (sender, git author, S-o-b).

Thanks,
Laszlo

> OK, I'll try to send it from the same email address next time, the reason
> that using two different mail addresses is because the author email is an
> exchange account and it's hard to find the mail server preferences for
> git-send-email...
> 
>>
>>> The commit message body currently does not start with "From: Ge Song
>>> ", which means that the git authorship and the
>>> first S-o-b disagree at the moment.
>>>
>>> ... If I'm wrong about this, I can be convinced, of course.
>>>
>>> Thanks
>>> Laszlo
> 

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

Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

[Laszlo Ersek]

On 09/04/17 12:26, Zeng, Star wrote:
> Trusted Ray to help review this series. :)

Thank you all!
Laszlo


> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Monday, September 4, 2017 6:04 PM
> To: Zeng, Star ; Dong, Eric 
> Cc: Ni, Ruiyu ; Wu, Hao A ; Justen, 
> Jordan L ; edk2-devel@lists.01.org; Andrew Fish 
> ; Gao, Liming ; Kinney, Michael D 
> 
> Subject: Re: [edk2] [PATCH v4 0/6] read-only UDF file system support
> 
> On 08/24/17 19:56, Paulo Alcantara wrote:
>> Hi,
>>
>> This series introduces read-only UDF file system support in EDK2. As 
>> Laszlo (or Red Hat) seemed to be interested in such support, I'm 
>> posting it again after ~3 years.
>>
>> The idea is not replacing the default FAT file system, nor breaking 
>> any existing file system support, but extending EDK2 with a new file 
>> system that might be useful for some people who are looking for 
>> specific file system features that current FAT doesn't support.
>>
>> Originally the driver was written to support UDF file systems as 
>> specified by OSTA Universal Disk Format Specification 2.60. However, 
>> some Windows 10 Enterprise ISO (UDF bridge) images that I tested 
>> supported a revision of 1.02 thus I had to rework the driver a little 
>> bit to support such revision as well.
>>
>> v2:
>>  - Rework to _partially_ support UDF revisions <2.60.
>>  - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h
>>instead of creating another one (UDF_VOLUME_DESCRIPTOR).
>>  - Fixed UdfDxe to correctly follow UEFI driver model.
>>  - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one.
>>  - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver
>>check for specific UDF device path to decide whether or not install
>>SimpleFs protocol.
>>  - Place MdePkg changes in a separate patch.
>> v3:
>>  - Install UDF partition child handles with a Vendor-Defined Media
>>Device Path.
>>  - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a
>>specific UDF file system GUID when determining to whether or not
>>start the driver.
>>  - Removed leading TAB chars in some source files identified by
>>PatchCheck.py tool.
> 
> Ping -- can we please get feedback on this? (Patches #1 through #3.)
> 
> Thanks
> Laszlo
> 
>> v4:
>>  - Added missing R-b's.
>>
>> Repo:   https://github.com/pcacjr/edk2.git
>> Branch: udf-fs-v4
>>
>> Cc: Laszlo Ersek 
>> Cc: Jordan Justen 
>> Cc: Andrew Fish 
>> Cc: Michael D Kinney 
>> Cc: Liming Gao 
>> Cc: Star Zeng 
>> Cc: Eric Dong 
>> Cc: Mark Doran 
>> Cc: Ruiyu Ni 
>> Cc: hao.a...@intel.com
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara 
>> ---
>>
>> Paulo Alcantara (6):
>>   MdePkg: Add UDF volume structure definitions
>>   MdeModulePkg/PartitionDxe: Add UDF file system support
>>   MdeModulePkg: Initial UDF/ECMA-167 file system support
>>   OvmfPkg: Enable UDF file system support
>>   ArmVirtPkg: Enable UDF file system support
>>   Nt32Pkg: Enable UDF file system support
>>
>>  ArmVirtPkg/ArmVirtQemu.dsc |3 +-
>>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc   |3 +-
>>  ArmVirtPkg/ArmVirtQemuKernel.dsc   |3 +-
>>  ArmVirtPkg/ArmVirtXen.dsc  |3 +-
>>  ArmVirtPkg/ArmVirtXen.fdf  |1 +
>>  .../Universal/Disk/PartitionDxe/Partition.c|9 +-
>>  .../Universal/Disk/PartitionDxe/Partition.h|   32 +-
>>  .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |3 +-
>>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c |  318 +++
>>  MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c |  185 ++
>>  MdeModulePkg/Universal/Disk/UdfDxe/File.c  |  903 
>>  MdeModulePkg/Universal/Disk/UdfDxe/FileName.c  |  195 ++
>>  .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 2447 
>> 
>>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c   |  344 +++
>>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h   | 1244 ++
>>  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf  |   66 +
>>  MdePkg/Include/IndustryStandard/Udf.h  |   60 +
>>  Nt32Pkg/Nt32Pkg.dsc|1 +
>>  Nt32Pkg/Nt32Pkg.fdf|1 +
>>  OvmfPkg/OvmfPkgIa32.dsc|1 +
>>  OvmfPkg/OvmfPkgIa32.fdf|1 +
>>  OvmfPkg/OvmfPkgIa32X64.dsc |1 +
>>  

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Laszlo Ersek]

On 09/04/17 12:36, Zeng, Star wrote:
> Curious about one question when I am reviewing this patch series.
> 
> Does/Should the IOMMU component disable the address translation at 
> ExitBootServices?

It is a very good and interesting question.

My answer is, I don't know. The IOMMU protocol abstraction is specific
to edk2, and its behavior is not documented in the PI or UEFI specs (as
far as I know).

For one example, the VT-d IOMMU implementation in

  IntelSiliconPkg/IntelVTdDxe

zaps all translations at ExitBootServices():

VOID
EFIAPI
OnExitBootServices (
  IN EFI_EVENT   Event,
  IN VOID*Context
  )
{
  DEBUG ((DEBUG_INFO, "Vtd OnExitBootServices\n"));
  DumpVtdRegsAll ();
  DisableDmar ();
  DumpVtdRegsAll ();
}

Now, in case someone suggested that the same approach should be followed
by all IOMMU implementations, I'd have three counter-arguments:

(1) My series states the problem (and seeks to solve it) on the PciIo
protocol level, which is standardized. The IOMMU protocol is edk2-only.
I feel that this kind of "unmap on ExitBootServices" should be done by
all UEFI drivers that map DMA common buffers, regardless of
platform-specific PciIo implementations.

(2) ExitBootServices() notify functions are called in unspecified order
(there's no way to express dependencies between them). This means that,
following the above pattern, an IO address translation could be torn
down before a reliant PCI device is de-configured in its own UEFI_DRIVER
ExitBootServices() handler. The result can be a series of IOMMU faults.
I'm not sure if that's bad in practice, but it does look like a layering
violation. (We shouldn't tear down resources from the bottom up.)

(3) The DisableDmar() function is relatively simple. It uses VT-d
registers that exist independently of any PCI devices, and of any "live"
mappings.

Some IOMMU protocol implementations might be different -- for them the
live configuration might only be known by constantly tracking the full
set of Map() and Unmap() operations. That's a hurdle for the implementation.

Furthermore, what is supposed to happen if both the IOMMU driver and the
individual PCI driver destroy the mapping at ExitBootServices()? If the
PCI driver's notification function runs first, then the IOMMU driver can
mark the mapping as "unmapped", and ignore it in its own notification
function. If the IOMMU driver's notification function runs first, then
it can again only mark the mapping as "unmapped", so that when the PCI
driver tries to unmap it as well, things don't blow up. This seems to
imply that an unmapped mapping data structure can never be recycled,
because we never know what action might follow. This sounds very
confusing to me.


Now, there are two arguments in favor of the IOMMU ExitBootServices()
callback as well:

- Security. It doesn't matter if some driver forgets to de-configure a
translation, the IOMMU driver won't. Layering violations be damned.

- In ExitBootServices() notification functions, the UEFI memory map must
not be altered (memory cannot be released). This means that Bus Master
Write and Bus Master Read operations, for pending transfers, cannot be
unmapped (only CommonBuffer operations can be), because unmapping
Write/Read might free memory (bounce buffers) *implicitly*. For
CommonBuffer operations, separate PciIo.FreeBuffer() calls are necessary
for releasing memory, thus the notification functions can simply *not*
call FreeBuffer(), to stay safe. But this is not possible for pending
Read/Write operations.


My current thinking is that (a) PCI drivers should unmap pending Common
Buffer operations at ExitBootServices; (b) PCI drivers should never
return from their protocol member functions with pending Write/Read
operations, only CommonBuffer operations (put differently, if the
operation is asynchronous on the higher protocol level, then make the
underlying BMDMA operation CommonBuffer); (c) IOMMU protocol
implementations should not be required to clean up translations, only
allowed to, if they can do it without interfering with (a).

Basically, the question is, who *owns* the translations. Based on the
PciIo interfaces, and on the requirement that PCI drivers Map() and
Unmap() buffers, my opinion is that the PCI drivers own the translations.

If you think that we should take this question to the USWG, I'm OK with
that.

Thanks,
Laszlo



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Monday, September 4, 2017 3:55 AM
> To: edk2-devel-01 
> Cc: Dong, Eric ; Zeng, Star 
> Subject: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common 
> buffers at ExitBootServices()
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: pci_host_controllers_unmap
> 
> This series updates four PCI Host Controller drivers so that they don't leave 
> CommonBuffer 

[edk2] [PATCH 2/5] UefiCpuPkg/CpuDxe: log informative message at DEBUG_INFO level

[Laszlo Ersek]

"Detect CPU count: %d\n" is an informative message, not an error report.
Set its debug mask to DEBUG_INFO.

Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 UefiCpuPkg/CpuDxe/CpuMp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index 372c1e3ce4de..b3c0178d0708 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -622,7 +622,7 @@ InitializeMpSupport (
 
   MpInitLibGetNumberOfProcessors (, 
);
   mNumberOfProcessors = NumberOfProcessors;
-  DEBUG ((DEBUG_ERROR, "Detect CPU count: %d\n", mNumberOfProcessors));
+  DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors));
 
   //
   // Update CPU healthy information from Guided HOB
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 3/5] OvmfPkg/PlatformPei: log informative message at DEBUG_INFO level

[Laszlo Ersek]

"Platform PEIM Loaded" is an informative message, not an error report. Set
its debug mask to DEBUG_INFO.

Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/PlatformPei/Platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 3ccb7d0fbfaa..5a78668126b4 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -627,7 +627,7 @@ InitializePlatform (
 {
   EFI_STATUSStatus;
 
-  DEBUG ((EFI_D_ERROR, "Platform PEIM Loaded\n"));
+  DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));
 
   DebugDumpCmos ();
 
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 5/5] OvmfPkg/SataControllerDxe: log informative message at DEBUG_INFO level

[Laszlo Ersek]

When a UEFI_DRIVER attempts to open a protocol interface with BY_DRIVER
attribute that it already has open with BY_DRIVER attribute,
OpenProtocol() returns EFI_ALREADY_STARTED. This is not an error. The
UEFI-2.7 spec currently says,

> EFI_ALREADY_STARTED -- Attributes is BY_DRIVER and there is an item on
>the open list with an attribute of BY_DRIVER
>whose agent handle is the same as AgentHandle.

(In fact it is so much an expected condition that recent USWG Mantis
ticket  will codify its
additional edk2-specific behavior, namely to output the protocol interface
at once.)

Downgrade the log mask for this one condition to DEBUG_INFO, in
SataControllerStart(). This will match the log mask of the other two
informative messages in this function, "SataControllerStart START", and
"SataControllerStart END status = %r" (at which point Status can only be
EFI_SUCCESS).

Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/SataControllerDxe/SataController.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/SataControllerDxe/SataController.c 
b/OvmfPkg/SataControllerDxe/SataController.c
index 1f84ad034e5b..2a5c3ba9f1b2 100644
--- a/OvmfPkg/SataControllerDxe/SataController.c
+++ b/OvmfPkg/SataControllerDxe/SataController.c
@@ -388,6 +388,7 @@ SataControllerStart (
   IN EFI_DEVICE_PATH_PROTOCOL   *RemainingDevicePath
   )
 {
+  UINTN BailLogMask;
   EFI_STATUSStatus;
   EFI_PCI_IO_PROTOCOL   *PciIo;
   UINT64OriginalPciAttributes;
@@ -398,6 +399,7 @@ SataControllerStart (
 
   DEBUG ((EFI_D_INFO, "SataControllerStart START\n"));
 
+  BailLogMask = DEBUG_ERROR;
   SataPrivateData = NULL;
 
   //
@@ -412,6 +414,14 @@ SataControllerStart (
   EFI_OPEN_PROTOCOL_BY_DRIVER
   );
   if (EFI_ERROR (Status)) {
+if (Status == EFI_ALREADY_STARTED) {
+  //
+  // This is an expected condition for OpenProtocol() / BY_DRIVER, in a
+  // DriverBindingStart() member function; degrade the log mask to
+  // DEBUG_INFO.
+  //
+  BailLogMask = DEBUG_INFO;
+}
 goto Bail;
   }
 
@@ -542,7 +552,8 @@ ClosePciIo:
  );
 
 Bail:
-  DEBUG ((EFI_D_ERROR, "SataControllerStart error return status = %r\n", 
Status));
+  DEBUG ((BailLogMask, "SataControllerStart error return status = %r\n",
+Status));
   return Status;
 }
 
-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH 1/5] MdeModulePkg/UsbBusDxe: log warning message at DEBUG_WARN level

[Laszlo Ersek]

"UsbSelectConfig: failed to connect driver %r, ignored" is an error
message, but it states at once that the error condition will not affect
the control flow. Degrade the report to DEBUG_WARN.

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c 
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
index b0e6b835ac24..62645d9bdedb 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
@@ -440,7 +440,11 @@ UsbSelectConfig (
 Status = UsbConnectDriver (UsbIf);
 
 if (EFI_ERROR (Status)) {
-  DEBUG ((EFI_D_ERROR, "UsbSelectConfig: failed to connect driver %r, 
ignored\n", Status));
+  DEBUG ((
+DEBUG_WARN,
+"UsbSelectConfig: failed to connect driver %r, ignored\n",
+Status
+));
 }
   }
 
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 0/5] MdeModulePkg, UefiCpuPkg, OvmfPkg: lower some DEBUGs from ERROR level

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: degrade_non_errors

Building OVMF with

  --pcd=gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel=0x8000

(i.e., DEBUG_ERROR only) exposes a smattering of DEBUG messages that are
logged with masks that contain DEBUG_ERROR, even though they don't
actually report errors. Downgrade them appropriately -- most of them to
DEBUG_INFO, one to DEBUG_WARN.

Cc: Eric Dong 
Cc: Jordan Justen 
Cc: Star Zeng 

Thanks,
Laszlo

Laszlo Ersek (5):
  MdeModulePkg/UsbBusDxe: log warning message at DEBUG_WARN level
  UefiCpuPkg/CpuDxe: log informative message at DEBUG_INFO level
  OvmfPkg/PlatformPei: log informative message at DEBUG_INFO level
  OvmfPkg/PlatformBootManagerLib: log informative message at DEBUG_INFO
lvl
  OvmfPkg/SataControllerDxe: log informative message at DEBUG_INFO level

 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c   |  6 +-
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c |  2 +-
 OvmfPkg/PlatformPei/Platform.c   |  2 +-
 OvmfPkg/SataControllerDxe/SataController.c   | 13 -
 UefiCpuPkg/CpuDxe/CpuMp.c|  2 +-
 5 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH 0/2] MdePkg, OvmfPkg: fix IoWriteFifo8(), and print debug messages with it

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: debugprint_iofifo

The first patch (for MdePkg) fixes an assembly language operand ordering
oversight in the unrolled / SEV implementations of IoWriteFifoXX().

The second patch makes OvmfPkg use IoWriteFifo8() for printing DEBUG
messages and ASSERT() failures.

Thanks,
Laszlo

Cc: Brijesh Singh 
Cc: Jordan Justen 
Cc: Liming Gao 
Cc: Michael D Kinney 

Laszlo Ersek (2):
  MdePkg/BaseIoLibIntrinsic: fix SEV (=unrolled) variants of
IoWriteFifoXX()
  OvmfPkg/PlatformDebugLibIoPort: write messages with IoWriteFifo8()

 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c | 20 
 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm |  6 +++---
 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  |  6 +++---
 3 files changed, 14 insertions(+), 18 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH 1/2] MdePkg/BaseIoLibIntrinsic: fix SEV (=unrolled) variants of IoWriteFifoXX()

[Laszlo Ersek]

In commit b6d11d7c4678 ("MdePkg: BaseIoLibIntrinsic (IoLib class)
library", 2017-04-12), the MOV instructions in the write loops were
probably copied from the read loops. However, the operand order was not
adjusted.

As a result, the IoWriteFifoXX() routines, when invoked in SEV guests, now
overwrite the source buffer with value 0x01 / 0x0001 / 0x0001 -- the
SevNoRepIo() function returns value 1 in EAX, in SEV guests --, and write
the same value to the target IO port.

Fix this by putting the target operand (AL / AX / EAX) first, and the
source operand (BYTE / WORD / DWORD [ESI/RSI]) second.

Cc: Brijesh Singh 
Cc: Jordan Justen 
Cc: Liming Gao 
Cc: Michael D Kinney 
Fixes: b6d11d7c467810ea7f2e2eda46ef0bdc57bf1475
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 6 +++---
 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm 
b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
index 3e80c17d04a3..4b2af807cff8 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
+++ b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
@@ -212,7 +212,7 @@ ASM_PFX(IoWriteFifo8):
 jecxz   @IoWriteFifo8_Done
 
 @IoWriteFifo8_Loop:
-mov byte [esi], al
+mov al, byte [esi]
 out dx, al
 inc esi
 loop@IoWriteFifo8_Loop
@@ -250,7 +250,7 @@ ASM_PFX(IoWriteFifo16):
 jecxz   @IoWriteFifo16_Done
 
 @IoWriteFifo16_Loop:
-mov word [esi], ax
+mov ax, word [esi]
 out dx, ax
 add esi, 2
 loop@IoWriteFifo16_Loop
@@ -288,7 +288,7 @@ ASM_PFX(IoWriteFifo32):
 jecxz   @IoWriteFifo32_Done
 
 @IoWriteFifo32_Loop:
-mov dword [esi], eax
+mov eax, dword [esi]
 out dx, eax
 add esi, 4
 loop@IoWriteFifo32_Loop
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm 
b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
index 26e016625b72..4d86a6cd5330 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
+++ b/MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
@@ -205,7 +205,7 @@ ASM_PFX(IoWriteFifo8):
 jrcxz   @IoWriteFifo8_Done
 
 @IoWriteFifo8_Loop:
-mov byte [rsi], al
+mov al, byte [rsi]
 out dx, al
 inc rsi
 loop@IoWriteFifo8_Loop
@@ -241,7 +241,7 @@ ASM_PFX(IoWriteFifo16):
 jrcxz   @IoWriteFifo16_Done
 
 @IoWriteFifo16_Loop:
-mov word [rsi], ax
+mov ax, word [rsi]
 out dx, ax
 add rsi, 2
 loop@IoWriteFifo16_Loop
@@ -277,7 +277,7 @@ ASM_PFX(IoWriteFifo32):
 jrcxz   @IoWriteFifo32_Done
 
 @IoWriteFifo32_Loop:
-mov dword [rsi], eax
+mov eax, dword [rsi]
 out dx, eax
 add rsi, 4
 loop@IoWriteFifo32_Loop
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 2/2] OvmfPkg/PlatformDebugLibIoPort: write messages with IoWriteFifo8()

[Laszlo Ersek]

Since commit 19c6d9feaaf8 ("MdePkg: Expand BaseIoLibIntrinsic (IoLib
class) library", 2017-01-14), IoWriteFifo8() has been widely available to
modules. Use it to print debug messages and assertion failures to the QEMU
debug port, rather than open-coded loops.

In the general case this speeds up logging, because debug messages will
now trap to QEMU once per message (as opposed to once per character), due
to "REP OUTSB" in "MdePkg/Library/BaseIoLibIntrinsic/*/IoFifoSev.nasm".

In SEV guests, there is no speedup (SEV doesn't support the REP prefix).
SEV is detected internally to BaseIoLibIntrinsic.

Cc: Brijesh Singh 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c 
b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
index 44850a9dbad0..5435767c1c69 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c
@@ -69,7 +69,7 @@ DebugPrint (
 {
   CHAR8Buffer[MAX_DEBUG_MESSAGE_LENGTH];
   VA_LIST  Marker;
-  UINT8*Ptr;
+  UINTNLength;
 
   //
   // If Format is NULL, then ASSERT().
@@ -87,15 +87,13 @@ DebugPrint (
   // Convert the DEBUG() message to an ASCII String
   //
   VA_START (Marker, Format);
-  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
+  Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
   VA_END (Marker);
 
   //
   // Send the print string to the debug I/O port
   //
-  for (Ptr = (UINT8 *) Buffer; *Ptr; Ptr++) {
-IoWrite8 (PcdGet16(PcdDebugIoPort), *Ptr);
-  }
+  IoWriteFifo8 (PcdGet16 (PcdDebugIoPort), Length, Buffer);
 }
 
 
@@ -129,20 +127,18 @@ DebugAssert (
   )
 {
   CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
-  UINT8 *Ptr;
+  UINTN  Length;
 
   //
   // Generate the ASSERT() message in Ascii format
   //
-  AsciiSPrint (Buffer, sizeof Buffer, "ASSERT %a(%Lu): %a\n", FileName,
-(UINT64)LineNumber, Description);
+  Length = AsciiSPrint (Buffer, sizeof Buffer, "ASSERT %a(%Lu): %a\n",
+ FileName, (UINT64)LineNumber, Description);
 
   //
-  // Send the print string to the Console Output device
+  // Send the print string to the debug I/O port
   //
-  for (Ptr = (UINT8 *) Buffer; *Ptr; Ptr++) {
-IoWrite8 (PcdGet16(PcdDebugIoPort), *Ptr);
-  }
+  IoWriteFifo8 (PcdGet16 (PcdDebugIoPort), Length, Buffer);
 
   //
   // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
-- 
2.14.1.3.gb7cf6e02401b

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

Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

[Ge Song]

On 09/04/2017 09:34 PM, Jordan Justen wrote:


On 2017-09-03 16:55:36, Laszlo Ersek wrote:

On 09/03/17 04:12, Ge Song wrote:

In earlier PEI stage, temporary memory(Cache As Ram) is employed as stack
and heap. We move them to the new room and do some relocation fixup when
permanent memory becomes available. TemporaryRamMigration()
is responsible for switching the stack.

[...]


Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ge Song 
---
  OvmfPkg/Sec/SecMain.c | 2 ++
  1 file changed, 2 insertions(+)

(No more separate emails, I promise.)

Before you send v2, can you please amend the patch like this:

   git commit --amend --author='Ge Song '

It's OK if you mail it out from another email address, but IMO the git
authorship should match the first Signed-off-by.

I disagree with this. Unless the individual is well known to the EDK
II community as the owner of both email addresses, I think the author
needs to send it from their author email.

I suppose they can send it from another email, and then reply from
their other email saying they contributed it under the Tianocore
Contribution Agreement, but that sounds like a stretch...

-Jordan


OK, I'll try to send it from the same email address next time, the reason
that using two different mail addresses is because the author email is an
exchange account and it's hard to find the mail server preferences for
git-send-email...




The commit message body currently does not start with "From: Ge Song
", which means that the git authorship and the
first S-o-b disagree at the moment.

... If I'm wrong about this, I can be convinced, of course.

Thanks
Laszlo


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

Re: [edk2] [PATCH] Platform/ARM/VExpressPkg:Fix Pcd broken in edk2

[Evan Lloyd]

Thanks, Leif.

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: 01 September 2017 22:48
> To: Evan Lloyd 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Matteo Carlini ; ryan.har...@linaro.org
> Subject: Re: [PATCH] Platform/ARM/VExpressPkg:Fix Pcd broken in edk2
>
> On Fri, Sep 01, 2017 at 06:27:28PM +0100, evan.ll...@arm.com wrote:
> > From: EvanLloyd 
> >
> > A recent change in edk2:
> > EmbeddedPkg/EmbeddedPkg.dsc: fix build for non-ARM architectures
> > actually broke the build for AARCH64 architectures (well Juno, really).
> > The PCD moved, PcdIsp1761BaseAddress, is used by
> > edk2\EmbeddedPkg\Drivers\Isp1761UsbDxe\Isp1761UsbDxe.inf, which
> was
> > included by Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> >
> > This patch moves the Isp1761UsbDxe.inf to [Components.ARM] so that it
> > doesn't invoke the PCD for Juno.
>
> Err, oops.
>
> The only explanation I can think of is that in a moment of genius I decided
> that since TC2 was the only platform using the driver, that was the only one I
> had to build test after the change...

[[Evan Lloyd]] Big of you to own up.  We've all been there.

>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Evan Lloyd 
>
> Reviewed-by: Leif Lindholm 
>
> However, would you be able to submit this under Contribution Agreement
> 1.1? (a reply stating so would be sufficient, I can fold it in before
> pushing)

[[Evan Lloyd]]
[[Evan Lloyd]] "Err, oops."  Please be so good as to accept this assurance that 
I intended to use Contribution Agreement 1.1.  I'll change our template today, 
to prevent this happening again.

>
> /
> Leif
>
> > ---
> >
> > Code may be examined at:
> > https://github.com/EvanLloyd/edk2-
> platforms/blob/148_fix_juno_build_v1
> > /Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> >
> >  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > index
> >
> 770007d1c89300f11c130515244686db6f97c96b..f82df48a00b22967f9c974152
> 6fc
> > 0c1b75073fa0 100644
> > --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
> > @@ -471,9 +471,6 @@ [Components.common]
> >EmbeddedPkg/Drivers/Lan91xDxe/Lan91xDxe.inf
> >  !endif
> >
> > -  # ISP1761 USB OTG Controller
> > -  EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf
> > -
> >#
> ># Android Fastboot
> >#
> > @@ -505,3 +502,9 @@ [Components.common]
> >PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> >
> BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgC
> ommandLib.inf
> >}
> > +
> > +[Components.ARM]
> > +
> > +  # ISP1761 USB OTG Controller
> > +  EmbeddedPkg/Drivers/Isp1761UsbDxe/Isp1761UsbDxe.inf
> > +
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com] 
Sent: Friday, August 25, 2017 1:56 AM
To: edk2-devel@lists.01.org
Cc: Paulo Alcantara ; Laszlo Ersek ; 
Justen, Jordan L ; Andrew Fish ; 
Kinney, Michael D ; Gao, Liming 
; Zeng, Star ; Dong, Eric 
; Doran, Mark ; Ni, Ruiyu 
; Wu, Hao A 
Subject: [PATCH v4 0/6] read-only UDF file system support

Hi,

This series introduces read-only UDF file system support in EDK2. As Laszlo (or 
Red Hat) seemed to be interested in such support, I'm posting it again after ~3 
years.

The idea is not replacing the default FAT file system, nor breaking any 
existing file system support, but extending EDK2 with a new file system that 
might be useful for some people who are looking for specific file system 
features that current FAT doesn't support.

Originally the driver was written to support UDF file systems as specified by 
OSTA Universal Disk Format Specification 2.60. However, some Windows 10 
Enterprise ISO (UDF bridge) images that I tested supported a revision of 1.02 
thus I had to rework the driver a little bit to support such revision as well.

v2:
 - Rework to _partially_ support UDF revisions <2.60.
 - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h
   instead of creating another one (UDF_VOLUME_DESCRIPTOR).
 - Fixed UdfDxe to correctly follow UEFI driver model.
 - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one.
 - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver
   check for specific UDF device path to decide whether or not install
   SimpleFs protocol.
 - Place MdePkg changes in a separate patch.
v3:
 - Install UDF partition child handles with a Vendor-Defined Media
   Device Path.
 - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a
   specific UDF file system GUID when determining to whether or not
   start the driver.
 - Removed leading TAB chars in some source files identified by
   PatchCheck.py tool.
v4:
 - Added missing R-b's.

Repo:   https://github.com/pcacjr/edk2.git
Branch: udf-fs-v4

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Mark Doran 
Cc: Ruiyu Ni 
Cc: hao.a...@intel.com
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (6):
  MdePkg: Add UDF volume structure definitions
  MdeModulePkg/PartitionDxe: Add UDF file system support
  MdeModulePkg: Initial UDF/ECMA-167 file system support
  OvmfPkg: Enable UDF file system support
  ArmVirtPkg: Enable UDF file system support
  Nt32Pkg: Enable UDF file system support

 ArmVirtPkg/ArmVirtQemu.dsc |3 +-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc   |3 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc   |3 +-
 ArmVirtPkg/ArmVirtXen.dsc  |3 +-
 ArmVirtPkg/ArmVirtXen.fdf  |1 +
 .../Universal/Disk/PartitionDxe/Partition.c|9 +-
 .../Universal/Disk/PartitionDxe/Partition.h|   32 +-
 .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |3 +-
 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c |  318 +++
 MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c |  185 ++
 MdeModulePkg/Universal/Disk/UdfDxe/File.c  |  903 
 MdeModulePkg/Universal/Disk/UdfDxe/FileName.c  |  195 ++
 .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 2447 
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c   |  344 +++
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h   | 1244 ++
 MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf  |   66 +
 MdePkg/Include/IndustryStandard/Udf.h  |   60 +
 Nt32Pkg/Nt32Pkg.dsc|1 +
 Nt32Pkg/Nt32Pkg.fdf|1 +
 OvmfPkg/OvmfPkgIa32.dsc|1 +
 OvmfPkg/OvmfPkgIa32.fdf|1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |1 +
 OvmfPkg/OvmfPkgIa32X64.fdf |1 +
 OvmfPkg/OvmfPkgX64.dsc |1 +
 OvmfPkg/OvmfPkgX64.fdf |1 +
 25 files changed, 5816 insertions(+), 11 deletions(-)  create mode 100644 
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/File.c
 create mode 100644 

Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

[Jordan Justen]

On 2017-09-03 16:55:36, Laszlo Ersek wrote:
> On 09/03/17 04:12, Ge Song wrote:
> > In earlier PEI stage, temporary memory(Cache As Ram) is employed as stack
> > and heap. We move them to the new room and do some relocation fixup when
> > permanent memory becomes available. TemporaryRamMigration()
> > is responsible for switching the stack.
> 
> [...]
> 
> > Cc: Jordan Justen 
> > Cc: Laszlo Ersek 
> > Cc: Ard Biesheuvel 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ge Song 
> > ---
> >  OvmfPkg/Sec/SecMain.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> (No more separate emails, I promise.)
> 
> Before you send v2, can you please amend the patch like this:
> 
>   git commit --amend --author='Ge Song '
> 
> It's OK if you mail it out from another email address, but IMO the git
> authorship should match the first Signed-off-by.

I disagree with this. Unless the individual is well known to the EDK
II community as the owner of both email addresses, I think the author
needs to send it from their author email.

I suppose they can send it from another email, and then reply from
their other email saying they contributed it under the Tianocore
Contribution Agreement, but that sounds like a stretch...

-Jordan

> The commit message body currently does not start with "From: Ge Song
> ", which means that the git authorship and the
> first S-o-b disagree at the moment.
> 
> ... If I'm wrong about this, I can be convinced, of course.
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

[Zeng, Star]

Trusted Ray to help review this series. :)


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Monday, September 4, 2017 6:04 PM
To: Zeng, Star ; Dong, Eric 
Cc: Ni, Ruiyu ; Wu, Hao A ; Justen, 
Jordan L ; edk2-devel@lists.01.org; Andrew Fish 
; Gao, Liming ; Kinney, Michael D 

Subject: Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

On 08/24/17 19:56, Paulo Alcantara wrote:
> Hi,
> 
> This series introduces read-only UDF file system support in EDK2. As 
> Laszlo (or Red Hat) seemed to be interested in such support, I'm 
> posting it again after ~3 years.
> 
> The idea is not replacing the default FAT file system, nor breaking 
> any existing file system support, but extending EDK2 with a new file 
> system that might be useful for some people who are looking for 
> specific file system features that current FAT doesn't support.
> 
> Originally the driver was written to support UDF file systems as 
> specified by OSTA Universal Disk Format Specification 2.60. However, 
> some Windows 10 Enterprise ISO (UDF bridge) images that I tested 
> supported a revision of 1.02 thus I had to rework the driver a little 
> bit to support such revision as well.
> 
> v2:
>  - Rework to _partially_ support UDF revisions <2.60.
>  - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h
>instead of creating another one (UDF_VOLUME_DESCRIPTOR).
>  - Fixed UdfDxe to correctly follow UEFI driver model.
>  - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one.
>  - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver
>check for specific UDF device path to decide whether or not install
>SimpleFs protocol.
>  - Place MdePkg changes in a separate patch.
> v3:
>  - Install UDF partition child handles with a Vendor-Defined Media
>Device Path.
>  - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a
>specific UDF file system GUID when determining to whether or not
>start the driver.
>  - Removed leading TAB chars in some source files identified by
>PatchCheck.py tool.

Ping -- can we please get feedback on this? (Patches #1 through #3.)

Thanks
Laszlo

> v4:
>  - Added missing R-b's.
> 
> Repo:   https://github.com/pcacjr/edk2.git
> Branch: udf-fs-v4
> 
> Cc: Laszlo Ersek 
> Cc: Jordan Justen 
> Cc: Andrew Fish 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Mark Doran 
> Cc: Ruiyu Ni 
> Cc: hao.a...@intel.com
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara 
> ---
> 
> Paulo Alcantara (6):
>   MdePkg: Add UDF volume structure definitions
>   MdeModulePkg/PartitionDxe: Add UDF file system support
>   MdeModulePkg: Initial UDF/ECMA-167 file system support
>   OvmfPkg: Enable UDF file system support
>   ArmVirtPkg: Enable UDF file system support
>   Nt32Pkg: Enable UDF file system support
> 
>  ArmVirtPkg/ArmVirtQemu.dsc |3 +-
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc   |3 +-
>  ArmVirtPkg/ArmVirtQemuKernel.dsc   |3 +-
>  ArmVirtPkg/ArmVirtXen.dsc  |3 +-
>  ArmVirtPkg/ArmVirtXen.fdf  |1 +
>  .../Universal/Disk/PartitionDxe/Partition.c|9 +-
>  .../Universal/Disk/PartitionDxe/Partition.h|   32 +-
>  .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |3 +-
>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c |  318 +++
>  MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c |  185 ++
>  MdeModulePkg/Universal/Disk/UdfDxe/File.c  |  903 
>  MdeModulePkg/Universal/Disk/UdfDxe/FileName.c  |  195 ++
>  .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 2447 
> 
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c   |  344 +++
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h   | 1244 ++
>  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf  |   66 +
>  MdePkg/Include/IndustryStandard/Udf.h  |   60 +
>  Nt32Pkg/Nt32Pkg.dsc|1 +
>  Nt32Pkg/Nt32Pkg.fdf|1 +
>  OvmfPkg/OvmfPkgIa32.dsc|1 +
>  OvmfPkg/OvmfPkgIa32.fdf|1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf |1 +
>  OvmfPkg/OvmfPkgX64.dsc |1 +
>  OvmfPkg/OvmfPkgX64.fdf |1 +
>  25 files 

[edk2] Accessing RT services from OS

[Ankit.Singh3]

Dell - Internal Use - Confidential

Hi All,

Wanted to access RT services from OS, as there any already any such exposed OS 
function or utilities.

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

Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

[Laszlo Ersek]

On 08/24/17 19:56, Paulo Alcantara wrote:
> Hi,
> 
> This series introduces read-only UDF file system support in EDK2. As
> Laszlo (or Red Hat) seemed to be interested in such support, I'm posting
> it again after ~3 years.
> 
> The idea is not replacing the default FAT file system, nor breaking any
> existing file system support, but extending EDK2 with a new file system
> that might be useful for some people who are looking for specific file
> system features that current FAT doesn't support.
> 
> Originally the driver was written to support UDF file systems as
> specified by OSTA Universal Disk Format Specification 2.60. However,
> some Windows 10 Enterprise ISO (UDF bridge) images that I tested
> supported a revision of 1.02 thus I had to rework the driver a little
> bit to support such revision as well.
> 
> v2:
>  - Rework to _partially_ support UDF revisions <2.60.
>  - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h
>instead of creating another one (UDF_VOLUME_DESCRIPTOR).
>  - Fixed UdfDxe to correctly follow UEFI driver model.
>  - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one.
>  - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver
>check for specific UDF device path to decide whether or not install
>SimpleFs protocol.
>  - Place MdePkg changes in a separate patch.
> v3:
>  - Install UDF partition child handles with a Vendor-Defined Media
>Device Path.
>  - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a
>specific UDF file system GUID when determining to whether or not
>start the driver.
>  - Removed leading TAB chars in some source files identified by
>PatchCheck.py tool.

Ping -- can we please get feedback on this? (Patches #1 through #3.)

Thanks
Laszlo

> v4:
>  - Added missing R-b's.
> 
> Repo:   https://github.com/pcacjr/edk2.git
> Branch: udf-fs-v4
> 
> Cc: Laszlo Ersek 
> Cc: Jordan Justen 
> Cc: Andrew Fish 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Mark Doran 
> Cc: Ruiyu Ni 
> Cc: hao.a...@intel.com
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara 
> ---
> 
> Paulo Alcantara (6):
>   MdePkg: Add UDF volume structure definitions
>   MdeModulePkg/PartitionDxe: Add UDF file system support
>   MdeModulePkg: Initial UDF/ECMA-167 file system support
>   OvmfPkg: Enable UDF file system support
>   ArmVirtPkg: Enable UDF file system support
>   Nt32Pkg: Enable UDF file system support
> 
>  ArmVirtPkg/ArmVirtQemu.dsc |3 +-
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc   |3 +-
>  ArmVirtPkg/ArmVirtQemuKernel.dsc   |3 +-
>  ArmVirtPkg/ArmVirtXen.dsc  |3 +-
>  ArmVirtPkg/ArmVirtXen.fdf  |1 +
>  .../Universal/Disk/PartitionDxe/Partition.c|9 +-
>  .../Universal/Disk/PartitionDxe/Partition.h|   32 +-
>  .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |3 +-
>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c |  318 +++
>  MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c |  185 ++
>  MdeModulePkg/Universal/Disk/UdfDxe/File.c  |  903 
>  MdeModulePkg/Universal/Disk/UdfDxe/FileName.c  |  195 ++
>  .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 2447 
> 
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.c   |  344 +++
>  MdeModulePkg/Universal/Disk/UdfDxe/Udf.h   | 1244 ++
>  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf  |   66 +
>  MdePkg/Include/IndustryStandard/Udf.h  |   60 +
>  Nt32Pkg/Nt32Pkg.dsc|1 +
>  Nt32Pkg/Nt32Pkg.fdf|1 +
>  OvmfPkg/OvmfPkgIa32.dsc|1 +
>  OvmfPkg/OvmfPkgIa32.fdf|1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf |1 +
>  OvmfPkg/OvmfPkgX64.dsc |1 +
>  OvmfPkg/OvmfPkgX64.fdf |1 +
>  25 files changed, 5816 insertions(+), 11 deletions(-)
>  create mode 100644 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/File.c
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/FileName.c
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>  create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  create mode 100644 

Re: [edk2] [PATCH v1 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory fail

[Ge Song]

On 09/04/2017 04:49 AM, Laszlo Ersek wrote:

On 09/03/17 04:12, Ge Song wrote:

In earlier PEI stage, temporary memory(Cache As Ram) is employed as stack

Please remove the "Cache As Ram" reference; it doesn't apply to OVMF.

I suggest "temporary memory at PcdOvmfSecPeiTempRamBase"


and heap. We move them to the new room and do some relocation fixup when
permanent memory becomes available. TemporaryRamMigration()
is responsible for switching the stack.

In the begining of the TemporaryRamMigration(),

I suggest rewording this as "before entering TemporaryRamMigration()" --
when I read the above, I went looking for Ebp/Rbp manipulation within
TemporaryRamMigration().


Ebp/Rbp is populated with content of Esp/Rsp and used as frame pointer,
after the execution of SetJump/LongJump, stack migrates to new position

Please start a new sentence with "after the execution of ...".


while the context keeps unchanged. But when TemporaryRamMigration() exits,
Esp/Rsp is filled with the content of Ebp/Rbp to destroy this stack frame.
The result is, stack switches back to previous temporary momery.

s/momery/memory/

Thanks for your detailed guidance, I'll revise the things you mentioned in
V2 patch.

And I must express my thanks for your article
"Laszlo's unkempt git guide for edk2 contributors and maintainers".
It helps me a lot!

But, more importantly, what are the practical consequences of this bug?

Does it mean that the temporary SEC/PEI stack is used until the end of
PEI, even after permanent RAM is installed, during both normal boot and
S3 resume?

Yes, I think that's exactly true.

If that's the case, I wonder how it was caught.

Once I was tracing a variable and found the abnormal value in SP.

If it was caught during S3 resume, then the only symptom could have been
stack overflow. Because during normal boot, both temporary and permanent
PEI RAM are reserved, so even if we stay on the temporary stack during
S3, if we don't overflow it, we cannot corrupt data:


/**
   Publish system RAM and reserve memory regions

**/
VOID
InitializeRamRegions (
   VOID
   )
{
   if (!mXen) {
 QemuInitializeRam ();
   } else {
 XenPublishRamRegions ();
   }

   if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
 //
 // This is the memory range that will be used for PEI on S3 resume
 //
 BuildMemoryAllocationHob (
   mS3AcpiReservedMemoryBase,
   mS3AcpiReservedMemorySize,
   EfiACPIMemoryNVS
   );

 //
 // Cover the initial RAM area used as stack and temporary PEI heap.
 //
 // This is reserved as ACPI NVS so it can be used on S3 resume.
 //
 BuildMemoryAllocationHob (
   PcdGet32 (PcdOvmfSecPeiTempRamBase),
   PcdGet32 (PcdOvmfSecPeiTempRamSize),
   EfiACPIMemoryNVS
   );

With stack overflow during S3 resume, we may have corrupted OS data
residing very low.

Yes, I think this is the real problem.

In current implementation, stack located in temporary memory will not be
exhausted during the whole PEI phase so there won't be an system crash
during S3 resume caused by this.

The size of stack in temporary memory is 32K, when in permanent memory it
is 128K. This approximately simulate the scene in real machine. 
Logically the

stack should be switched to permanent memory when the latter becomes
available.

Since hardware protection mechanism for stack is disabled, currently using
larger stack can reduced the possibility of stack overflow, thus, the 
possibility

of crash during S3 resume

Check the GDT set up by reset vector module:
GDT_BASE:
; null descriptor
NULL_SELequ $-GDT_BASE
DW  0; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB  0; sys flag, dpl, type
DB  0; limit 19:16, flags
DB  0; base 31:24

; linear data segment descriptor
LINEAR_SEL  equ $-GDT_BASE
DW  0x   ; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(DATA32_TYPE)
DB 
GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf)

DB  0; base 31:24

; linear code segment descriptor
LINEAR_CODE_SEL equ $-GDT_BASE
DW  0x   ; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(CODE32_TYPE)
DB 
GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf)

DB  0; base 31:24

%ifdef ARCH_X64
; linear code (64-bit) segment descriptor
LINEAR_CODE64_SEL   equ $-GDT_BASE
DW  0x   ; limit 15:0
DW  0; base 15:0
DB  0; base 23:16
DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(CODE64_TYPE)
DB 
GRANULARITY_FLAG(1)|DEFAULT_SIZE32(0)|CODE64_FLAG(1)|UPPER_LIMIT(0xf)

DB  0; base 31:24
%endif

[edk2] [Patch] BaseTools: add support for BIOS build with binary cache

[Yonghong Zhu]

Add three new options:
--hash enables hash-based caching during build process. when --hash is
enabled, build tool will base on the module hash value to do the
incremental build, without --hash, build tool will base on the
timestamp to do the incremental build. --hash option use md5 method to
get every hash value, DSC/FDF, tools_def.txt, build_rule.txt and build
command are calculated as global hash value, Package DEC and its
include header files are calculated as package hash value, Module
source files and its INF file are calculated as module hash value.
Library hash value will combine the global hash value and its dependent
package hash value. Driver hash value will combine the global hash
value, its dependent package hash value and its linked library hash
value.
When --hash and --bindest are specified, build tool will copy the
generated binary files for each module into the directory specified by
bindest at the build phase. Bindest caches all generated binary files.
When --hash and --binsource are specified, build tool will try to get
the binary files from the binary source directory at the build phase.
If the cached binary has the same hash value, it will be directly used.
Otherwise, build tool will compile the source files and generate the
binary files.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py   | 161 +--
 BaseTools/Source/Python/Common/GlobalData.py |   7 ++
 BaseTools/Source/Python/build/build.py   |  30 +
 3 files changed, 189 insertions(+), 9 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 70e2e62..246bfec 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -41,10 +41,11 @@ import Common.VpdInfoFile as VpdInfoFile
 from GenPcdDb import CreatePcdDatabaseCode
 from Workspace.MetaFileCommentParser import UsageList
 from Common.MultipleWorkspace import MultipleWorkspace as mws
 import InfSectionParser
 import datetime
+import hashlib
 
 ## Regular expression for splitting Dependency Expression string into tokens
 gDepexTokenPattern = re.compile("(\(|\)|\w+| \S+\.inf)")
 
 #
@@ -263,10 +264,14 @@ class WorkspaceAutoGen(AutoGen):
 self.FdfFile= FlashDefinitionFile
 self.FdTargetList   = Fds
 self.FvTargetList   = Fvs
 self.CapTargetList  = Caps
 self.AutoGenObjectList = []
+self._BuildDir  = None
+self._FvDir = None
+self._MakeFileDir   = None
+self._BuildCommand  = None
 
 # there's many relative directory operations, so ...
 os.chdir(self.WorkspaceDir)
 
 #
@@ -642,10 +647,18 @@ class WorkspaceAutoGen(AutoGen):
 #
 Pa.CollectPlatformDynamicPcds()
 Pa.CollectFixedAtBuildPcds()
 self.AutoGenObjectList.append(Pa)
 
+#
+# Generate Package level hash value
+#
+GlobalData.gPackageHash[Arch] = {}
+if GlobalData.gUseHashCache:
+for Pkg in Pkgs:
+self._GenPkgLevelHash(Pkg)
+
 #
 # Check PCDs token value conflict in each DEC file.
 #
 self._CheckAllPcdsTokenValueConflict()
 
@@ -655,15 +668,10 @@ class WorkspaceAutoGen(AutoGen):
 self._CheckPcdDefineAndType()
 
 # if self.FdfFile:
 # self._CheckDuplicateInFV(Fdf)
 
-self._BuildDir = None
-self._FvDir = None
-self._MakeFileDir = None
-self._BuildCommand = None
-
 #
 # Create BuildOptions Macro & PCD metafile, also add the Active 
Platform and FDF file.
 #
 content = 'gCommandLineDefines: '
 content += str(GlobalData.gCommandLineDefines)
@@ -675,10 +683,14 @@ class WorkspaceAutoGen(AutoGen):
 content += str(self.Platform)
 content += os.linesep
 if self.FdfFile:
 content += 'Flash Image Definition: '
 content += str(self.FdfFile)
+content += os.linesep
+if GlobalData.gBinCacheDest:
+content += 'Cache of .efi location: '
+content += str(GlobalData.gBinCacheDest)
 SaveFileOnChange(os.path.join(self.BuildDir, 'BuildOptions'), content, 
False)
 
 #
 # Create PcdToken Number file for Dynamic/DynamicEx Pcd.
 #
@@ -704,10 +716,22 @@ class WorkspaceAutoGen(AutoGen):
 for f in AllWorkSpaceMetaFiles:
 if os.stat(f)[8] > SrcTimeStamp:
 SrcTimeStamp = os.stat(f)[8]
 self._SrcTimeStamp = SrcTimeStamp
 
+if GlobalData.gUseHashCache:
+m = hashlib.md5()
+for files in AllWorkSpaceMetaFiles:
+if files.endswith('.dec'):
+continue
+f = 

[edk2] [Patch] Build spec: add description for build with binary cache

[Yonghong Zhu]

fixes:https://bugzilla.tianocore.org/show_bug.cgi?id=689
Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 .../82_auto-generation_process.md| 20 
 README.md|  1 +
 appendix_d_buildexe_command/d4_usage.md  | 19 +++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/8_pre-build_autogen_stage/82_auto-generation_process.md 
b/8_pre-build_autogen_stage/82_auto-generation_process.md
index 671a7d5..f1d7158 100644
--- a/8_pre-build_autogen_stage/82_auto-generation_process.md
+++ b/8_pre-build_autogen_stage/82_auto-generation_process.md
@@ -1031,10 +1031,30 @@ maximum command line length.  The default value is 4096.
 **Note:** The following `FLAGS` options are included in the response file:
 `PP_FLAGS`, `CC_FLAGS`, `VFRPP_FLAGS`, `APP_FLAGS`, `ASLPP_FLAGS`, 
`ASLCC_FLAGS`,
 and `ASM_FLAGS`.
 **
 
+ 8.2.4.15 Build with Binary Cache
+
+**build** tool provides three new options for binary cache feature.
+--hash enables hash-based caching during build process. when --hash is enabled,
+build tool will base on the module hash value to do the incremental build, 
without
+--hash, build tool will base on the timestamp to do the incremental build. 
--hash
+option use md5 method to get every hash value, DSC/FDF, tools_def.txt, 
build_rule.txt
+and build command are calculated as global hash value, Package DEC and its 
include
+header files are calculated as package hash value, Module source files and its 
INF
+file are calculated as module hash value. Library hash value will combine the 
global
+hash value and its dependent package hash value. Driver hash value will 
combine the
+global hash value, its dependent package hash value and its linked library 
hash value.
+When --hash and --bindest are specified, build tool will copy the generated 
binary 
+files for each module into the directory specified by bindest at the build 
phase.
+Bindest caches all generated binary files.
+When --hash and --binsource are specified, build tool will try to get the 
binary
+files from the binary source directory at the build phase. If the cached 
binary has
+the same hash value, it will be directly used. Otherwise, build tool will 
compile the
+source files and generate the binary files.
+
 ### 8.2.5 Post processing
 
 Once all files are parsed, the build tools will do following work for each EDK
 II module:
 
diff --git a/README.md b/README.md
index 52abb6a..ca59a35 100644
--- a/README.md
+++ b/README.md
@@ -215,5 +215,6 @@ Copyright (c) 2008-2017, Intel Corporation. All rights 
reserved.
 || [#523](https://bugzilla.tianocore.org/show_bug.cgi?id=523) 
Build spec: add EBNF for the --pcd syntax in the Section D.4


 |   |
 || [#517](https://bugzilla.tianocore.org/show_bug.cgi?id=517) 
Build spec: chapter 5.2.2 Guided Tools add description for Pkcs7Sign tool and 
BrotliCompress tool 

   |   |
 || [#481](https://bugzilla.tianocore.org/show_bug.cgi?id=481) 
Build Spec: add clarification for not used Pcd that build tool will not do 
additional checks on its value  

  |   |
 || [#518](https://bugzilla.tianocore.org/show_bug.cgi?id=518) 
Build Spec: Update Precedence of PCD Values 


 |   |
 || [#669](https://bugzilla.tianocore.org/show_bug.cgi?id=669) 
Build Spec: Add multi-arg support to PREBUILD/POSTBUILD 


 |   |
+|| [#689](https://bugzilla.tianocore.org/show_bug.cgi?id=689) 
Build spec: add description for build with binary cache 
   

[edk2] [Patch] BaseTools: Not show *B when Pcd value in build option same with DEC

[Yonghong Zhu]

Per build spec, If the value obtained from either a build option, the
DSC or FDF is the same as the value in the DEC, then *B , *P or *F
will not be shown in the report.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/build/BuildReport.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/build/BuildReport.py 
b/BaseTools/Source/Python/build/BuildReport.py
index a7cbb6a..38ee26d 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -969,14 +969,14 @@ class PcdReport(object):
 DscMatch = (DscDefaultValue.strip() == 
PcdValue.strip())
 
 #
 # Report PCD item according to their override relationship
 #
-if BuildOptionMatch:
-FileWrite(File, ' *B %-*s: %6s %10s = %-22s' % 
(self.MaxLen, PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', 
PcdValue.strip()))
-elif DecMatch and InfMatch:
+if DecMatch and InfMatch:
 FileWrite(File, '%-*s: %6s %10s = %-22s' % 
(self.MaxLen, PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', 
PcdValue.strip()))
+elif BuildOptionMatch:
+FileWrite(File, ' *B %-*s: %6s %10s = %-22s' % 
(self.MaxLen, PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', 
PcdValue.strip()))
 else:
 if DscMatch:
 if (Pcd.TokenCName, Key) in self.FdfPcdSet:
 FileWrite(File, ' *F %-*s: %6s %10s = %-22s' % 
(self.MaxLen, PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', 
PcdValue.strip()))
 else:
-- 
2.6.1.windows.1

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

[edk2] [edk2-VfrSpecification PATCH] Add union data type and bit fields in VFR Data Struct Definition

[Dandan Bi]

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

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../23_vfr_data_struct_definition.md   | 39 ++
 README.md  |  1 +
 2 files changed, 40 insertions(+)

diff --git a/2_vfr_description_in_bnf/23_vfr_data_struct_definition.md 
b/2_vfr_description_in_bnf/23_vfr_data_struct_definition.md
index 6d6730d..4937e6a 100644
--- a/2_vfr_description_in_bnf/23_vfr_data_struct_definition.md
+++ b/2_vfr_description_in_bnf/23_vfr_data_struct_definition.md
@@ -36,10 +36,16 @@ vfrDataStructDefinition ::=
  { "typedef" } "struct"
  { StringIdentifier }
  "{" vfrDataStructFields "}"
  { StringIdentifier } ";"
 
+vfrDataStructDefinition ::=
+ { "typedef" } "union"
+ { StringIdentifier }
+ "{" vfrDataStructFields "}"
+ { StringIdentifier } ";"
+
 vfrDataStructFields ::=
   (
   dataStructField64
 | dataStructField32
 | dataStructField16
@@ -48,10 +54,14 @@ vfrDataStructFields ::=
 | dataStructFieldString
 | dataStructFieldDate
 | dataStructFieldTime
 | dataStructFieldRef
 | dataStructFieldUser
+| dataStructBitField64
+| dataStructBitField32
+| dataStructBitField16
+| dataStructBitField8
   )*
 
 dataStructField64 ::=
   "UINT64"
   StringIdentifier { "[" Number "]" } ";"
@@ -89,10 +99,26 @@ dataStructFieldRef ::=
   StringIdentifier { "[" Number "]" } ";"
 
 dataStructFieldUser ::=
   StringIdentifier
   StringIdentifier { "[" Number "]" } ";"
+
+dataStructBitField64 ::=
+  "UINT64"
+  { StringIdentifier } ":" Number ";"
+
+dataStructBitField32 ::=
+  "UINT32"
+  { StringIdentifier } ":" Number ";"
+
+dataStructBitField16 ::=
+  "UINT16"
+  { StringIdentifier } ":" Number ";"
+
+dataStructBitField8 ::=
+  "UINT8"
+  { StringIdentifier } ":" Number ";"
 ```
 
  BEHAVIORS AND RESTRICTIONS
 
 The data structure definition is in C-style language. `enum` type is not
@@ -108,10 +134,23 @@ typedef struct {
   UINT8 mU8;
   UINT16 mU16;
   UINT32 mU32[10];
   UINT64 mU64;
 } MyData;
+
+typedef union {
+  UINT16   Field16;
+  UINT8Field8;
+} MyUnionData;
+
+typedef struct {
+  UINT16   Field16;
+  UINT8MyBits1 : 1;
+  UINT8MyBits2 : 3;
+  UINT8MyBits3 : 3;
+  UINT16   MyBits4 : 4;
+} MyBitsData;
 ```
 
 **Unsupported Example of enum type:**
 
 ```c
diff --git a/README.md b/README.md
index 888eb81..0a596eb 100644
--- a/README.md
+++ b/README.md
@@ -89,5 +89,6 @@ Copyright (c) 2007-2017, Intel Corporation. All rights 
reserved.
 | 1.60 | Update syntax for goto, image, questionref and constant value 
opcodes, correct CALLBACK flag to INTEREACTIVE, correct help string for old 
syntax date/time example, and add examples for expression opcodes.   | December 
1, 2011  |
 | 1.70 | Clarify restriction that enum type and struct data filed with 
more than one dimensions array are not supported.   
 | May 18, 2012 
 |
 | 1.80 | Add syntax for warningif opcode, update definition for name/value 
varstore and subtitle opcode, update referenced UEFI spec version info. 
 | Jan 14, 2014 
 |
 | 1.90 | Correct sample code for catenate/match/cond opcode. Add syntax 
for match2 opcode. Add sample code to show the buffer type constant value for 
orderedlist opcode and default opcode.| July 2, 
2015  |
 | 1.91 | Convert to Gitbook

 | April 2017   
 |
+|  | [#683](https://bugzilla.tianocore.org/show_bug.cgi?id=683) VFR 
Spec: Add union data type and bit fields in VFR Data Struct Definition  
|   
|
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 2/2] SD : Updated CMD 6 implememtation.

[Meenakshi Aggarwal]

Hi Jun,


I checked my host controller driver and i am taking care of its endianness in 
host controller driver itself.


I researched further on the bit number I am checking for High speed in CMD6 
data. 

I tried to find out in SD specs if response for CMD 6 comes it LE or BE format.

I didn't find out anything mentioned directly for CMD6 but yes for ACMD51 (SCR 
command), it follows BE.

We are reading only 8 bytes in SCR register to get SD version, while SD version 
comes in bit 59:56:


SD Memory Card - Spec. Version   SD_SPEC [59:56]


I think similar stands true for CMD6 as well. Bit 512 is coming first on DATA 
line.

So I am checking correct bits in patch.

I have refer linux code also, there also they are considering bit 512 is coming 
first.

Please comment.


Thanks,
Meenakshi


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Friday, September 01, 2017 3:03 PM
> To: Jun Nie ; Leif Lindholm 
> Cc: edk2-devel@lists.01.org; Haojian Zhuang 
> Subject: Re: [edk2] [PATCH 2/2] SD : Updated CMD 6 implememtation.
> 
> [This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
> 
> Hi Leif and Jun,
> 
> 
> Thanks for your review.
> 
> 
> My comments are inlined.
> 
> Regards,
> Meenakshi
> 
> > -Original Message-
> > From: Jun Nie [mailto:jun@linaro.org]
> > Sent: Thursday, August 31, 2017 8:13 PM
> > To: Leif Lindholm ; Meenakshi Aggarwal
> > 
> > Cc: edk2-devel@lists.01.org; Haojian Zhuang
> > 
> > Subject: Re: [edk2] [PATCH 2/2] SD : Updated CMD 6 implememtation.
> >
> > On 2017年08月31日 20:06, Leif Lindholm wrote:
> > > On Wed, Aug 30, 2017 at 07:50:59PM +0530, Meenakshi Aggarwal wrote:
> > >> For setting high speed in SD card,
> > >> First CMD 6 (Switch) is send to check if card supports High Speed
> > >> and Second command is send to switch card to high speed mode.
> > >>
> > >> In current inplementation, CMD 6 was sent only once to switch the
> > >> card into HS mode without checking if card supports HS or not,
> > >> which is not as per specification and also we are not setting the HS i.e.
> > >> 5000 but directly asking the card to switch to 2600 which
> > >> is incorrect as SD card supports either 2500 or 5000.
> >
> > Good catch, check should be done before setting function. And the
> > setting result should be checked before return. Logic is correct in this 
> > patch.
> >
> > >
> > > Same as previous one: Jun, Haojian?
> > >
> > > I do have a couple of style comments below.
> > >
> > >> Signed-off-by: Meenakshi Aggarwal 
> > >> ---
> > >>   EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 64
> > 
> > >>   1 file changed, 55 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > >> b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > >> index 7f74c54..3071b3b 100644
> > >> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > >> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > >> @@ -317,6 +317,24 @@ InitializeEmmcDevice (
> > >> return Status;
> > >>   }
> > >>
> > >> +
> > >> +STATIC
> > >> +UINT32
> > >> +CreateSwitchCmdArgument (
> > >
> > > This helper function is a good addition, thanks.
> > >
> > >> +  IN  UINT8  Mode,
> > >> +  IN  UINT8  Group,
> > >> +  IN  UINT8  Value
> > >> +  )
> > >> +{
> > >> +  UINT32 Argument;
> > >> +
> > >> +  Argument = Mode << 31 | 0x00FF;
> > >
> > > Just because I hate implicit type promotion, could you make Mode
> > > UINT32 in the input, please?
> > >
> I will surely do this.
> 
> > >> +  Argument &= ~(0xF << (Group * 4));  Argument |= Value << (Group
> > >> + * 4);
> > >> +
> > >> +  return Argument;
> > >> +}
> > >> +
> > >>   STATIC
> > >>   EFI_STATUS
> > >>   InitializeSdMmcDevice (
> > >> @@ -326,6 +344,7 @@ InitializeSdMmcDevice (
> > >> UINT32CmdArg;
> > >> UINT32Response[4];
> > >> UINT32Buffer[128];
> > >> +  UINT32Speed;
> > >> UINTN BlockSize;
> > >> UINTN CardSize;
> > >> UINTN NumBlocks;
> > >> @@ -334,6 +353,7 @@ InitializeSdMmcDevice (
> > >> EFI_STATUSStatus;
> > >> EFI_MMC_HOST_PROTOCOL *MmcHost;
> > >>
> > >> +  Speed = 2500;
> > >
> > > Could this be given a #define with a descriptive name, in Mmc.h?
> > >
> ok
> > >> MmcHost = MmcHostInstance->MmcHost;
> > >>
> > >> // Send a command to get Card specific data @@ -439,43 +459,69
> > >> @@ InitializeSdMmcDevice (
> > >>   }
> > >> }
> > >> if (CccSwitch) {
> > >> +/* SD Switch, Mode:0, Group:0, Value:0 */
> > >> +CmdArg = CreateSwitchCmdArgument(0, 0, 0);
> >
> > A 

[edk2] [Patch 0/2] Fix bug in IP driver for IpSec protocol notify

[Fu Siyuan]

Fu Siyuan (2):
  MdeModulePkg/Ip4Dxe: fix a bug in IP4 driver for IpSec protocol
notify.
  NetworkPkg/Ip6Dxe: fix a bug in IP6 driver for IpSec protocol notify.

 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c | 14 +++---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c  | 10 ++
 NetworkPkg/Ip6Dxe/Ip6Driver.c | 18 --
 NetworkPkg/Ip6Dxe/Ip6Input.c  | 14 ++
 4 files changed, 27 insertions(+), 29 deletions(-)

-- 
1.9.5.msysgit.1

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

[edk2] [Patch 2/2] NetworkPkg/Ip6Dxe: fix a bug in IP6 driver for IpSec protocol notify.

[Fu Siyuan]

The IP driver uses EfiCreateProtocolNotifyEvent() to register notify callback
function for IpSec protocol, but it didn't notice that the callback will always
be executed at least once, even the protocol wasn't in handle database.
As a result, the Ip6IpSecProcessPacket() will still always call LocateProtocol()
even the IpSec protocol is not installed, which will impact the network
performance.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/Ip6Dxe/Ip6Driver.c | 18 --
 NetworkPkg/Ip6Dxe/Ip6Input.c  | 14 ++
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6Driver.c b/NetworkPkg/Ip6Dxe/Ip6Driver.c
index 8a8cc89..43af838 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Driver.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Driver.c
@@ -1,7 +1,7 @@
 /** @file
   The driver binding and service binding protocol for IP6 driver.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
 
   This program and the accompanying materials
@@ -41,14 +41,20 @@ IpSec2InstalledCallback (
   IN VOID   *Context
   )
 {
+  EFI_STATUSStatus;
   //
-  // Close the event so it does not get called again.
+  // Test if protocol was even found.
+  // Notification function will be called at least once.
   //
-  gBS->CloseEvent (Event);
-
-  mIpSec2Installed = TRUE;
+  Status = gBS->LocateProtocol (, NULL, );
+  if (Status == EFI_SUCCESS && mIpSec != NULL) {
+//
+// Close the event so it does not get called again.
+//
+gBS->CloseEvent (Event);
 
-  return;
+mIpSec2Installed = TRUE;
+  }
 }
 
 /**
diff --git a/NetworkPkg/Ip6Dxe/Ip6Input.c b/NetworkPkg/Ip6Dxe/Ip6Input.c
index e53e087..6aa 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Input.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Input.c
@@ -1,7 +1,7 @@
 /** @file
   IP6 internal functions to process the incoming packets.
 
-  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
 
   This program and the accompanying materials
@@ -530,6 +530,7 @@ Ip6IpSecProcessPacket (
   if (!mIpSec2Installed) {
 goto ON_EXIT;
   }
+  ASSERT (mIpSec != NULL);
   
   Packet= *Netbuf;
   RecycleEvent  = NULL;
@@ -541,17 +542,6 @@ Ip6IpSecProcessPacket (
   FragmentCount = Packet->BlockOpNum;
   ZeroMem (, sizeof (EFI_IP6_HEADER));
 
-  if (mIpSec == NULL) {
-gBS->LocateProtocol (, NULL, (VOID **) );
-
-//
-// Check whether the ipsec protocol is available.
-//
-if (mIpSec == NULL) {
-  goto ON_EXIT;
-}
-  }
-
   //
   // Check whether the ipsec enable variable is set.
   //
-- 
1.9.5.msysgit.1

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

[edk2] [Patch 1/2] MdeModulePkg/Ip4Dxe: fix a bug in IP4 driver for IpSec protocol notify.

[Fu Siyuan]

The IP driver uses EfiCreateProtocolNotifyEvent() to register notify callback
function for IpSec protocol, but it didn't notice that the callback will always
be executed at least once, even the protocol wasn't in handle database.
As a result, the Ip4IpSecProcessPacket() will still always call LocateProtocol()
even the IpSec protocol is not installed, which will impact the network
performance.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c | 14 +++---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c  | 10 ++
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
index 792db5c..03ba458 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
@@ -41,12 +41,20 @@ IpSec2InstalledCallback (
   IN VOID   *Context
   )
 {
+  EFI_STATUSStatus;
   //
-  // Close the event so it does not get called again.
+  // Test if protocol was even found.
+  // Notification function will be called at least once.
   //
-  gBS->CloseEvent (Event);
+  Status = gBS->LocateProtocol (, NULL, );
+  if (Status == EFI_SUCCESS && mIpSec != NULL) {
+//
+// Close the event so it does not get called again.
+//
+gBS->CloseEvent (Event);
 
-  mIpSec2Installed = TRUE;
+mIpSec2Installed = TRUE;
+  }
 }
 
 /**
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c
index 09b8f2b..e694323 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c
@@ -1,7 +1,7 @@
 /** @file
   IP4 input process.
   
-Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
 
 This program and the accompanying materials
@@ -518,6 +518,7 @@ Ip4IpSecProcessPacket (
   if (!mIpSec2Installed) {
 goto ON_EXIT;
   }
+  ASSERT (mIpSec != NULL);
   
   Packet= *Netbuf;
   RecycleEvent  = NULL;
@@ -527,13 +528,6 @@ Ip4IpSecProcessPacket (
   FragmentCount = Packet->BlockOpNum;
 
   ZeroMem (, sizeof (IP4_HEAD));
-  
-  if (mIpSec == NULL) {
-gBS->LocateProtocol (, NULL, (VOID **) );
-if (mIpSec == NULL) {
-  goto ON_EXIT;
-}
-  }
 
   //
   // Check whether the IPsec enable variable is set.
-- 
1.9.5.msysgit.1

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

[edk2] [patch 3/6] MdeModulePkg: Add GUID/flags to implement BitField support

[Dandan Bi]

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

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Include/Guid/MdeModuleHii.h | 19 ++-
 MdeModulePkg/MdeModulePkg.dec|  4 
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Include/Guid/MdeModuleHii.h 
b/MdeModulePkg/Include/Guid/MdeModuleHii.h
index 81821da..cfce1ea 100644
--- a/MdeModulePkg/Include/Guid/MdeModuleHii.h
+++ b/MdeModulePkg/Include/Guid/MdeModuleHii.h
@@ -1,9 +1,9 @@
 /** @file
   EDKII extented HII IFR guid opcodes.
 
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 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 that accompanies this 
distribution.  
 The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php.

 
@@ -209,12 +209,29 @@ typedef struct _EFI_IFR_GUID_VAREQNAME {
   /// The the Unicode String will be used as a EFI Variable Name.
   ///
   UINT16  NameId;
 } EFI_IFR_GUID_VAREQNAME;
 
+///
+/// EDKII implementation extension GUID, used to indaicate there are bit 
fields in the varstore.
+///
+#define EFI_IFR_BITVARSTORE_GUID \
+  {0x82DDD68B, 0x9163, 0x4187, {0x9B, 0x27, 0x20, 0xA8, 0xFD, 0x60,0xA7, 0x1D}}
+
+///
+/// EDKII implementation extension flags, used to indaicate the disply style 
and bit width for bit filed storage.
+///
+#define EFI_IFR_DISPLAY_BIT0xC0
+#define EFI_IFR_DISPLAY_INT_DEC_BIT0x00
+#define EFI_IFR_DISPLAY_UINT_DEC_BIT   0x40
+#define EFI_IFR_DISPLAY_UINT_HEX_BIT   0x80
+
+#define EFI_IFR_NUMERIC_SIZE_BIT   0x3F
+
 #pragma pack()
 
 extern EFI_GUID gEfiIfrTianoGuid;
 extern EFI_GUID gEfiIfrFrameworkGuid;
+extern EFI_GUID gEfiIfrBitvarstoreGuid;
 
 #endif
 
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 593bff3..8c8d76e 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -182,10 +182,14 @@
 
   ## Guid for EDKII implementation GUIDed opcodes
   #  Include/Guid/MdeModuleHii.h
   gEfiIfrTianoGuid  = { 0xf0b1735, 0x87a0, 0x4193, {0xb2, 0x66, 0x53, 
0x8c, 0x38, 0xaf, 0x48, 0xce }}
 
+  ## Guid for EDKII implementation extension, used to indaicate there are bit 
fields in the varstore.
+  #  Include/Guid/MdeModuleHii.h
+  gEfiIfrBitvarstoreGuid  = {0x82DDD68B, 0x9163, 0x4187, {0x9B, 0x27, 0x20, 
0xA8, 0xFD, 0x60,0xA7, 0x1D}}
+
   ## Guid for Framework vfr GUIDed opcodes.
   #  Include/Guid/MdeModuleHii.h
   gEfiIfrFrameworkGuid  = { 0x31ca5d1a, 0xd511, 0x4931, { 0xb7, 0x82, 0xae, 
0x6b, 0x2b, 0x17, 0x8c, 0xd7 }}
 
   ## Guid to specify the System Non Volatile FV
-- 
1.9.5.msysgit.1

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

[edk2] [patch 4/6] MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore

[Dandan Bi]

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

For oneof/numeric/checkbox, their storage may be bit field.
When generating  string to get default value
for these questions, we need to parse the Ifr data to get
the bit Varstore info,and then generating the correct
 string.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/HiiDatabaseDxe/ConfigRouting.c   | 190 +++--
 .../Universal/HiiDatabaseDxe/HiiDatabase.h |   6 +-
 .../Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf|   3 +-
 3 files changed, 184 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index c9ff1cf..19cb382 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -13,10 +13,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 **/
 
 
 #include "HiiDatabase.h"
 extern HII_DATABASE_PRIVATE_DATA mPrivate;
+BOOLEAN mQuestionReferBitVar = FALSE;
 
 /**
   Calculate the number of Unicode characters of the incoming Configuration 
string,
   not including NULL terminator.
 
@@ -1223,19 +1224,19 @@ InsertBlockData (
   // Insert block data in its Offset and Width order.
   //
   for (Link = BlockLink->ForwardLink; Link != BlockLink; Link = 
Link->ForwardLink) {
 BlockArray = BASE_CR (Link, IFR_BLOCK_DATA, Entry);
 if (BlockArray->Offset == BlockSingleData->Offset) {
-  if (BlockArray->Width > BlockSingleData->Width) {
+  if ((BlockArray->Width > BlockSingleData->Width) || 
(BlockSingleData->IsBitVar && BlockArray->Width == BlockSingleData->Width)) {
 //
 // Insert this block data in the front of block array
 //
 InsertTailList (Link, >Entry);
 return;
   }
 
-  if (BlockArray->Width == BlockSingleData->Width) {
+  if ((!BlockSingleData->IsBitVar) && BlockArray->Width == 
BlockSingleData->Width) {
 //
 // The same block array has been added.
 //
 if (BlockSingleData != BlockArray) {
   FreePool (BlockSingleData);
@@ -1978,10 +1979,11 @@ Done:
   @param  HiiHandle  The hii handle for this form package.
   @param  VarStorageData The varstore data structure.
   @param  IfrOpHdr   Ifr opcode header for this opcode.
   @param  VarWidth   The buffer width for this opcode.
   @param  ReturnData The data block added for this opcode.
+  @param  IsBitVar   Whether the the opcode refers to bit storage.
 
   @retval  EFI_SUCCESS   This opcode is required.
   @retval  EFI_NOT_FOUND This opcode is not required.
   @retval  OthersContain some error.
  
@@ -1991,20 +1993,26 @@ IsThisOpcodeRequired (
   IN IFR_BLOCK_DATA   *RequestBlockArray,
   IN EFI_HII_HANDLE   HiiHandle,
   IN OUT IFR_VARSTORAGE_DATA  *VarStorageData,
   IN EFI_IFR_OP_HEADER*IfrOpHdr,
   IN UINT16   VarWidth,
-  OUTIFR_BLOCK_DATA   **ReturnData
+  OUTIFR_BLOCK_DATA   **ReturnData,
+  IN BOOLEAN  IsBitVar
   )
 {
   IFR_BLOCK_DATA   *BlockData;
   UINT16   VarOffset;
   EFI_STRING_IDNameId;
   EFI_IFR_QUESTION_HEADER  *IfrQuestionHdr;
+  UINT16   BitOffset;
+  UINT16   BitWidth;
+  UINT16   BitVarWidth;
 
   NameId= 0;
   VarOffset = 0;
+  BitOffset = 0;
+  BitVarWidth = 0;
   IfrQuestionHdr = (EFI_IFR_QUESTION_HEADER  *)((CHAR8 *) IfrOpHdr + sizeof 
(EFI_IFR_OP_HEADER));
 
   if (VarStorageData->Type == EFI_HII_VARSTORE_NAME_VALUE) {
 NameId = IfrQuestionHdr->VarStoreInfo.VarName;
 
@@ -2016,11 +2024,21 @@ IsThisOpcodeRequired (
   // This question is not in the requested string. Skip it.
   //
   return EFI_NOT_FOUND;
 }
   } else {
-VarOffset = IfrQuestionHdr->VarStoreInfo.VarOffset;
+if (IsBitVar) {
+  BitOffset = IfrQuestionHdr->VarStoreInfo.VarOffset;
+  VarOffset = BitOffset / 8;
+  BitVarWidth = VarWidth;
+  BitWidth  = BitOffset - (VarOffset * 8) + BitVarWidth;
+  VarWidth  = (BitWidth % 8 == 0 ? BitWidth / 8: BitWidth / 8 + 1);
+} else {
+  VarOffset = IfrQuestionHdr->VarStoreInfo.VarOffset;
+  BitVarWidth = VarWidth;
+  BitOffset = VarOffset * 8;
+}
 
 //
 // Check whether this question is in requested block array.
 //
 if (!BlockArrayCheck (RequestBlockArray, VarOffset, VarWidth, FALSE, 
HiiHandle)) {
@@ -2051,10 +2069,13 @@ IsThisOpcodeRequired (
 
   BlockData->Width  = VarWidth;
   BlockData->QuestionId = IfrQuestionHdr->QuestionId;
   BlockData->OpCode = IfrOpHdr->OpCode;
   

[edk2] [patch 1/6] BaseTool/VfrCompile: Support Union type in VFR

[Dandan Bi]

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

Update VfrCompiler to parse the UNION type in vfr file

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

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 406dbc5..24e0538 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,140 +320,155 @@ vfrPragmaPackDefinition :
 | pragmaPackNumber
   }
   "\)"
   ;
 
+  vfrDataUnionDefinition :
+  { TypeDef } Union<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
+  { NonNvDataMap }
+  {
+N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
+  }
+  OpenBrace
+vfrDataStructFields[TRUE]
+  CloseBrace
+  {
+N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
+  }
+  ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd ();>>
+  ;
+
 vfrDataStructDefinition :
   { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
   { NonNvDataMap }
   {
 N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
   }
   OpenBrace
-vfrDataStructFields
+vfrDataStructFields[FALSE]
   CloseBrace
   {
 N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
   }
   ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd (); >>
   ;
 
-vfrDataStructFields :
+vfrDataStructFields [BOOLEAN  FieldInUnion]:
   (
- dataStructField64 |
- dataStructField32 |
- dataStructField16 |
- dataStructField8  |
- dataStructFieldBool   |
- dataStructFieldString |
- dataStructFieldDate   |
- dataStructFieldTime   |
- dataStructFieldRef|
- dataStructFieldUser
+ dataStructField64 [FieldInUnion]|
+ dataStructField32 [FieldInUnion]|
+ dataStructField16 [FieldInUnion]|
+ dataStructField8  [FieldInUnion]|
+ dataStructFieldBool [FieldInUnion]  |
+ dataStructFieldString [FieldInUnion]|
+ dataStructFieldDate  [FieldInUnion] |
+ dataStructFieldTime  [FieldInUnion] |
+ dataStructFieldRef   [FieldInUnion] |
+ dataStructFieldUser [FieldInUnion]
   )*
   ;
 
-dataStructField64 :
+dataStructField64 [BOOLEAN  FieldInUnion]:
   << UINT32 ArrayNum = 0; >>
   D:"UINT64"
   N:StringIdentifier
   {
 OpenBracket I:Number CloseBracket   << ArrayNum = 
_STOU32(I->getText(), I->getLine()); >>
   }
-  ";"   << 
_PCATCH(gCVfrVarDataTypeDB.DataTypeAddField (N->getText(), D->getText(), 
ArrayNum), N); >>
+  ";"   << 
_PCATCH(gCVfrVarDataTypeDB.DataTypeAddField (N->getText(), D->getText(), 
ArrayNum, FieldInUnion), N); >>
   ;
 
-dataStructField32 :
+dataStructField32 [BOOLEAN  FieldInUnion]:
   << UINT32 ArrayNum = 0; >>
   D:"UINT32"
   N:StringIdentifier
   {
 OpenBracket I:Number CloseBracket   << ArrayNum = 
_STOU32(I->getText(), I->getLine()); >>
   }
-  ";"   << 
_PCATCH(gCVfrVarDataTypeDB.DataTypeAddField (N->getText(), D->getText(), 
ArrayNum), N); >>
+  ";"   << 
_PCATCH(gCVfrVarDataTypeDB.DataTypeAddField (N->getText(), D->getText(), 
ArrayNum, FieldInUnion), N); >>
   ;
 
-dataStructField16 :

[edk2] [patch 6/6] MdeModulePkg/DriverSample: Add sample questions with bit/union VarStore

[Dandan Bi]

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

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/DriverSampleDxe/DriverSample.c   | 120 +
 .../Universal/DriverSampleDxe/DriverSample.h   |   2 +
 .../Universal/DriverSampleDxe/NVDataStruc.h|  51 +-
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 187 +
 .../Universal/DriverSampleDxe/VfrStrings.uni   |  61 +++
 5 files changed, 420 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
index bbd9713..af31615 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -18,10 +18,13 @@ 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 MyEfiBitVar[] = L"MyEfiBitVar";
+CHAR16 MyEfiUnionVar[] = L"MyEfiUnionVar";
+
 EFI_HANDLE  DriverHandle[2] = {NULL, NULL};
 DRIVER_SAMPLE_PRIVATE_DATA  *mPrivateData = NULL;
 EFI_EVENT   mEvent;
 
 HII_VENDOR_DEVICE_PATH  mHiiVendorDevicePath0 = {
@@ -662,10 +665,17 @@ ExtractConfig (
 // through hii database, not support in this path.
 //
 if (HiiIsConfigHdrMatch(Request, , MyEfiVar)) {
   return EFI_UNSUPPORTED;
 }
+if (HiiIsConfigHdrMatch(Request, , MyEfiBitVar)) {
+  return EFI_UNSUPPORTED;
+}
+if (HiiIsConfigHdrMatch(Request, , 
MyEfiUnionVar)) {
+  return EFI_UNSUPPORTED;
+}
+
 //
 // Set Request to the unified request string.
 //
 ConfigRequest = Request;
 //
@@ -883,10 +893,16 @@ RouteConfig (
   // through hii database, not support in this path.
   //
   if (HiiIsConfigHdrMatch(Configuration, , MyEfiVar)) 
{
 return EFI_UNSUPPORTED;
   }
+  if (HiiIsConfigHdrMatch(Configuration, , 
MyEfiBitVar)) {
+return EFI_UNSUPPORTED;
+  }
+  if (HiiIsConfigHdrMatch(Configuration, , 
MyEfiUnionVar)) {
+return EFI_UNSUPPORTED;
+  }
 
   //
   // Get Buffer Storage data from EFI variable
   //
   BufferSize = sizeof (DRIVER_SAMPLE_CONFIGURATION);
@@ -1295,10 +1311,14 @@ DriverCallback (
 for (Index = 0; Index < 3; Index ++) {
   SetArrayData (Value, EFI_IFR_TYPE_NUM_SIZE_8, Index, BufferValue--);
 }
   break;
 
+  case 0x:
+Value->u8 = 12;
+break;
+
   default:
 Status = EFI_UNSUPPORTED;
   break;
   }
 }
@@ -1309,10 +1329,14 @@ DriverCallback (
   switch (QuestionId) {
   case 0x1240:
 Value->u8 = DEFAULT_CLASS_MANUFACTURING_VALUE;
   break;
 
+ case 0x:
+Value->u8 = 13;
+break;
+
   default:
 Status = EFI_UNSUPPORTED;  
   break;
   }
 }
@@ -1703,10 +1727,12 @@ DriverSampleInit (
   DRIVER_SAMPLE_CONFIGURATION *Configuration;
   BOOLEAN ActionFlag;
   EFI_STRING  ConfigRequestHdr;
   EFI_STRING  NameRequestHdr;
   MY_EFI_VARSTORE_DATA*VarStoreConfig;
+  MY_EFI_BITS_VARSTORE_DATA   *BitsVarStoreConfig;
+  MY_EFI_UNION_DATA   *UnionConfig;
   EFI_INPUT_KEY   HotKey;
   EDKII_FORM_BROWSER_EXTENSION_PROTOCOL *FormBrowserEx;
 
   //
   // Initialize the local variables.
@@ -1991,10 +2017,104 @@ DriverSampleInit (
   return EFI_INVALID_PARAMETER;
 }
   }
   FreePool (ConfigRequestHdr);
 
+  //
+  // Initialize Bits efi varstore configuration data
+  //
+  BitsVarStoreConfig = >BitsVarStoreConfig;
+  ZeroMem (BitsVarStoreConfig, sizeof (MY_EFI_BITS_VARSTORE_DATA));
+
+  ConfigRequestHdr = HiiConstructConfigHdr (, 
MyEfiBitVar, DriverHandle[0]);
+  ASSERT (ConfigRequestHdr != NULL);
+
+  BufferSize = sizeof (MY_EFI_BITS_VARSTORE_DATA);
+  Status = gRT->GetVariable (MyEfiBitVar, , NULL, 
, BitsVarStoreConfig);
+  if (EFI_ERROR (Status)) {
+//
+// Store zero data to EFI variable Storage.
+//
+Status = gRT->SetVariable(
+MyEfiBitVar,
+,
+EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS,
+sizeof (MY_EFI_BITS_VARSTORE_DATA),
+BitsVarStoreConfig
+);
+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;
+

Re: [edk2] [Patch] MdePkg: Add UEFI 2.7 defined GUID and structure for AIP network media type.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of fanwang2
Sent: Thursday, August 24, 2017 5:27 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Wang, Fan ; Fu, Siyuan 

Subject: [edk2] [Patch] MdePkg: Add UEFI 2.7 defined GUID and structure for AIP 
network media type.

From: Wang Fan 

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 MdePkg/Include/Protocol/AdapterInformation.h | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Protocol/AdapterInformation.h 
b/MdePkg/Include/Protocol/AdapterInformation.h
index 275e1b6..6421e56 100644
--- a/MdePkg/Include/Protocol/AdapterInformation.h
+++ b/MdePkg/Include/Protocol/AdapterInformation.h
@@ -1,11 +1,11 @@
 /** @file
   EFI Adapter Information Protocol definition.
   The EFI Adapter Information Protocol is used to dynamically and quickly 
discover
   or set device information for an adapter.
 
-  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2014 - 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
   http://opensource.org/licenses/bsd-license.php
 
@@ -44,10 +44,16 @@
 #define EFI_ADAPTER_INFO_UNDI_IPV6_SUPPORT_GUID \
   { \
 0x4bd56be3, 0x4975, 0x4d8a, {0xa0, 0xad, 0xc4, 0x91, 0x20, 0x4b, 0x5d, 
0x4d} \
   }
 
+#define EFI_ADAPTER_INFO_MEDIA_TYPE_GUID \
+  { \
+0x8484472f, 0x71ec, 0x411a, { 0xb3, 0x9c, 0x62, 0xcd, 0x94, 0xd9, 0x91, 
0x6e } \
+  }
+
+
 typedef struct _EFI_ADAPTER_INFORMATION_PROTOCOL 
EFI_ADAPTER_INFORMATION_PROTOCOL;
 
 ///
 /// EFI_ADAPTER_INFO_MEDIA_STATE
 ///
@@ -60,10 +66,23 @@ typedef struct {
   ///
   EFI_STATUSMediaState;
 } EFI_ADAPTER_INFO_MEDIA_STATE;
 
 ///
+/// EFI_ADAPTER_INFO_MEDIA_TYPE
+///
+typedef struct {
+  ///
+  /// Indicates the current media type. MediaType can have any of the 
following values:
+  /// 1: Ethernet Network Adapter
+  /// 2: Ethernet Wireless Network Adapter
+  /// 3~255: Reserved
+  ///
+  UINT8 MediaType;
+} EFI_ADAPTER_INFO_MEDIA_TYPE;
+
+///
 /// EFI_ADAPTER_INFO_NETWORK_BOOT
 ///
 typedef struct {
   ///
   /// TRUE if the adapter supports booting from iSCSI IPv4 targets.
-- 
1.9.5.msysgit.1

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

[edk2] [patch 0/6] Support bitfield and union type in vfr

[Dandan Bi]

This patch series is to:
(1) Update VfrCompiler to parse UNION type, bit field in data structure.
(2) Update HiiDatabase and SetupBrowser to handle questions with Bit VarStore.
(3) Add sample questions with bit/union VarStore in DriverSample.

Cc: Eric Dong 
Cc: Liming Gao 

Dandan Bi (6):
  BaseTool/VfrCompile: Support Union type in VFR
  BaseTool/VfrCompiler: Support Bit fields in EFI/Buffer VarStore
  MdeModulePkg: Add GUID/flags to implement BitFiled support
  MdeModulePkg/HiiDatabase: Handle questions with Bit VarStore
  MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore
  MdeModulePkg/DriverSample: Add sample questions with bit/union
VarStore

 BaseTools/Source/C/Include/Common/MdeModuleHii.h   |   12 +-
 BaseTools/Source/C/VfrCompile/VfrError.cpp |3 +-
 BaseTools/Source/C/VfrCompile/VfrError.h   |3 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp   |4 +-
 BaseTools/Source/C/VfrCompile/VfrFormPkg.h |   52 +-
 BaseTools/Source/C/VfrCompile/VfrSyntax.g  | 1240 
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp|  258 +++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h  |   28 +-
 MdeModulePkg/Include/Guid/MdeModuleHii.h   |   19 +-
 MdeModulePkg/MdeModulePkg.dec  |4 +
 .../Universal/DriverSampleDxe/DriverSample.c   |  120 ++
 .../Universal/DriverSampleDxe/DriverSample.h   |2 +
 .../Universal/DriverSampleDxe/NVDataStruc.h|   51 +-
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr |  187 +++
 .../Universal/DriverSampleDxe/VfrStrings.uni   |   61 +
 .../Universal/HiiDatabaseDxe/ConfigRouting.c   |  190 ++-
 .../Universal/HiiDatabaseDxe/HiiDatabase.h |6 +-
 .../Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf|3 +-
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c  |  142 ++-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c |  120 +-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.h |3 +
 .../Universal/SetupBrowserDxe/SetupBrowserDxe.inf  |3 +-
 22 files changed, 1961 insertions(+), 550 deletions(-)

-- 
1.9.5.msysgit.1

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

[edk2] [patch 5/6] MdeModulePkg/SetupBrowser: Handle questions with Bit VarStore

[Dandan Bi]

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 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c  | 142 -
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 120 +++--
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.h |   3 +
 .../Universal/SetupBrowserDxe/SetupBrowserDxe.inf  |   3 +-
 4 files changed, 226 insertions(+), 42 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
index 6b3e5e0..147ae9d 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/IfrParse.c
@@ -15,10 +15,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include "Setup.h"
 
 UINT16   mStatementIndex;
 UINT16   mExpressionOpCodeIndex;
 EFI_QUESTION_ID  mUsedQuestionId;
+BOOLEAN  mQuestionReferBitVar = FALSE;
+
 extern LIST_ENTRY  gBrowserStorageList;
 /**
   Initialize Statement header members.
 
   @param  OpCodeData Pointer of the raw OpCode data.
@@ -57,10 +59,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 (>Prompt, >Prompt, sizeof (EFI_STRING_ID));
   CopyMem (>Help, >Help, sizeof (EFI_STRING_ID));
 
@@ -1312,10 +1315,12 @@ ParseOpCodes (
   BOOLEAN InUnknownScope;
   UINT8   UnknownDepth;
   FORMSET_DEFAULTSTORE*PreDefaultStore;
   LIST_ENTRY  *DefaultLink;
   BOOLEAN HaveInserted;
+  UINT16  ByteWidth;
+  UINT16  BitWidth;
 
   SuppressForQuestion  = FALSE;
   SuppressForOption= FALSE;
   InScopeDisable   = FALSE;
   DepthOfDisable   = 0;
@@ -1978,47 +1983,100 @@ ParseOpCodes (
   ASSERT(CurrentStatement != NULL);
 
   CurrentStatement->Flags = ((EFI_IFR_ONE_OF *) OpCodeData)->Flags;
   Value = >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 (mQuestionReferBitVar) {
+mQuestionReferBitVar = FALSE;
+//
+// Get the bit var store info (bit/byte offset, bit/byte offset)
+//
+CurrentStatement->BitStorageWidth = CurrentStatement->Flags & 
EFI_IFR_NUMERIC_SIZE_BIT;
+CurrentStatement->QuestionReferToBitField = TRUE;
+CurrentStatement->BitVarOffset = 
CurrentStatement->VarStoreInfo.VarOffset;
+CurrentStatement->VarStoreInfo.VarOffset = 
CurrentStatement->BitVarOffset / 8;
+BitWidth = CurrentStatement->BitVarOffset - 
(CurrentStatement->VarStoreInfo.VarOffset * 8) +  
CurrentStatement->BitStorageWidth;
+ByteWidth = BitWidth / 8;
+CurrentStatement->StorageWidth = (BitWidth % 8 == 0? ByteWidth: 
ByteWidth + 1);
 
-  case EFI_IFR_NUMERIC_SIZE_2:
-CopyMem (>Minimum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u16.MinValue, sizeof (UINT16));
-CopyMem (>Maximum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u16.MaxValue, sizeof (UINT16));
-CopyMem (>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 (>Minimum, &((EFI_IFR_NUMERIC *) 
OpCodeData)->data.u32.MinValue, sizeof (UINT32));
-CopyMem (>Maximum, &((EFI_IFR_NUMERIC *)