Re: [edk2] [Patch] BaseTools: add the support for --pcd feature to patch the binary efi

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Tuesday, April 12, 2016 11:10 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: add the support for --pcd feature to patch
> the binary efi
> 
> the original --pcd feature can override the Pcd value when build the
> source driver, while it missed the binary driver. this patch add the
> support to patch the binary efi for --pcd feature.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/GenMake.py |  5 ++
>  BaseTools/Source/Python/GenFds/FfsInfStatement.py  | 14 +++-
>  BaseTools/Source/Python/GenFds/GenFds.py   | 85
> +-
>  .../Source/Python/GenFds/GenFdsGlobalVariable.py   |  2 -
>  4 files changed, 101 insertions(+), 5 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> b/BaseTools/Source/Python/AutoGen/GenMake.py
> index a4844be..afe82c8 100644
> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> @@ -1422,10 +1422,15 @@ class TopLevelMakefile(BuildFile):
>  ExtraOption += " -c"
> 
>  if GlobalData.gIgnoreSource:
>  ExtraOption += " --ignore-sources"
> 
> +if GlobalData.BuildOptionPcd:
> +for index, option in enumerate(GlobalData.gCommand):
> +if "--pcd" == option and GlobalData.gCommand[index+1]:
> +ExtraOption += " --pcd " + GlobalData.gCommand[index+1]
> +
>  MakefileName = self._FILE_NAME_[self._FileType]
>  SubBuildCommandList = []
>  for A in PlatformInfo.ArchList:
>  Command = self._MAKE_TEMPLATE_[self._FileType] %
> {"file":os.path.join("$(BUILD_DIR)", A, MakefileName)}
>  SubBuildCommandList.append(Command)
> diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> index 864e5be..a048949 100644
> --- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> +++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # process FFS generation from INF statement
>  #
> -#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
>  #  Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
>  #
>  #  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
> @@ -41,10 +41,11 @@ from FvImageSection import FvImageSection
>  from Common.Misc import PeImageClass
>  from AutoGen.GenDepex import DependencyExpression
>  from PatchPcdValue.PatchPcdValue import PatchBinaryFile
>  from Common.LongFilePathSupport import CopyLongFilePath
>  from Common.LongFilePathSupport import OpenLongFilePath as open
> +import Common.GlobalData as GlobalData
> 
>  ## generate FFS from INF
>  #
>  #
>  class FfsInfStatement(FfsInfStatementClassObject):
> @@ -258,11 +259,20 @@ class FfsInfStatement(FfsInfStatementClassObject):
>  FdfOverride = False
>  if PcdKey in FdfPcdDict:
>  DefaultValue = FdfPcdDict[PcdKey]
>  FdfOverride = True
> 
> -if not DscOverride and not FdfOverride:
> +# Override Patchable PCD value by the value from Build Option
> +BuildOptionOverride = False
> +if GlobalData.BuildOptionPcd:
> +for pcd in GlobalData.BuildOptionPcd:
> +if PcdKey == (pcd[1], pcd[0]):
> +DefaultValue = pcd[2]
> +BuildOptionOverride = True
> +break
> +
> +if not DscOverride and not FdfOverride and not 
> BuildOptionOverride:
>  continue
>  # Check value, if value are equal, no need to patch
>  if Pcd.DatumType == "VOID*":
>  if Pcd.DefaultValue == DefaultValue or DefaultValue in 
> [None, '']:
>  continue
> diff --git a/BaseTools/Source/Python/GenFds/GenFds.py
> b/BaseTools/Source/Python/GenFds/GenFds.py
> index d97fc28..ec9ff7a 100644
> --- a/BaseTools/Source/Python/GenFds/GenFds.py
> +++ b/BaseTools/Source/Python/GenFds/GenFds.py
> @@ -36,17 +36,18 @@ from Common import EdkLogger
>  from Common.String import *
>  from Common.Misc import DirCache, PathClass
>  from Common.Misc import SaveFileOnChange
>  from Common.Misc import ClearDuplicatedInf
>  from Common.Misc import GuidStructureStringToGuidString
> +from Common.Misc import CheckPcdDatum
>  from Common.BuildVersion import gBUILD_VERSION
>  from Common.MultipleWorkspace import MultipleWorkspace as mws
> 
>  

Re: [edk2] [Patch 0/2] Tool generate hash value for each output EFI image

[Gao, Liming]

Reviewed-by: Liming Gao  for this serial. 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Thursday, April 07, 2016 2:01 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/2] Tool generate hash value for each output EFI
> image
> 
> Build Tool add new report type 'HASH' to generate hash value in the build
> report.
> VolInfo Tool add new option --hash to use openssl to generate hash value
> for each PE image.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> 
> Yonghong Zhu (2):
>   BaseTools/VolInfo: generate HASH value for each PE image
>   BaseTools: generate hash value in build report for each output EFI
> image
> 
>  BaseTools/Source/C/VolInfo/VolInfo.c | 375
> ++-
>  BaseTools/Source/Python/build/BuildReport.py |  53 +++-
>  BaseTools/Source/Python/build/build.py   |   4 +-
>  3 files changed, 428 insertions(+), 4 deletions(-)
> 
> --
> 2.6.1.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] BaseTools: add the support for --pcd feature to patch the binary efi

[Yonghong Zhu]

the original --pcd feature can override the Pcd value when build the
source driver, while it missed the binary driver. this patch add the
support to patch the binary efi for --pcd feature.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/GenMake.py |  5 ++
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  | 14 +++-
 BaseTools/Source/Python/GenFds/GenFds.py   | 85 +-
 .../Source/Python/GenFds/GenFdsGlobalVariable.py   |  2 -
 4 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index a4844be..afe82c8 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1422,10 +1422,15 @@ class TopLevelMakefile(BuildFile):
 ExtraOption += " -c"
 
 if GlobalData.gIgnoreSource:
 ExtraOption += " --ignore-sources"
 
+if GlobalData.BuildOptionPcd:
+for index, option in enumerate(GlobalData.gCommand):
+if "--pcd" == option and GlobalData.gCommand[index+1]:
+ExtraOption += " --pcd " + GlobalData.gCommand[index+1]
+
 MakefileName = self._FILE_NAME_[self._FileType]
 SubBuildCommandList = []
 for A in PlatformInfo.ArchList:
 Command = self._MAKE_TEMPLATE_[self._FileType] % 
{"file":os.path.join("$(BUILD_DIR)", A, MakefileName)}
 SubBuildCommandList.append(Command)
diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py 
b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
index 864e5be..a048949 100644
--- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
+++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
@@ -1,9 +1,9 @@
 ## @file
 # process FFS generation from INF statement
 #
-#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
 #  Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
 #
 #  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
@@ -41,10 +41,11 @@ from FvImageSection import FvImageSection
 from Common.Misc import PeImageClass
 from AutoGen.GenDepex import DependencyExpression
 from PatchPcdValue.PatchPcdValue import PatchBinaryFile
 from Common.LongFilePathSupport import CopyLongFilePath
 from Common.LongFilePathSupport import OpenLongFilePath as open
+import Common.GlobalData as GlobalData
 
 ## generate FFS from INF
 #
 #
 class FfsInfStatement(FfsInfStatementClassObject):
@@ -258,11 +259,20 @@ class FfsInfStatement(FfsInfStatementClassObject):
 FdfOverride = False
 if PcdKey in FdfPcdDict:
 DefaultValue = FdfPcdDict[PcdKey]
 FdfOverride = True
 
-if not DscOverride and not FdfOverride:
+# Override Patchable PCD value by the value from Build Option
+BuildOptionOverride = False
+if GlobalData.BuildOptionPcd:
+for pcd in GlobalData.BuildOptionPcd:
+if PcdKey == (pcd[1], pcd[0]):
+DefaultValue = pcd[2]
+BuildOptionOverride = True
+break
+
+if not DscOverride and not FdfOverride and not BuildOptionOverride:
 continue
 # Check value, if value are equal, no need to patch
 if Pcd.DatumType == "VOID*":
 if Pcd.DefaultValue == DefaultValue or DefaultValue in [None, 
'']:
 continue
diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
index d97fc28..ec9ff7a 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -36,17 +36,18 @@ from Common import EdkLogger
 from Common.String import *
 from Common.Misc import DirCache, PathClass
 from Common.Misc import SaveFileOnChange
 from Common.Misc import ClearDuplicatedInf
 from Common.Misc import GuidStructureStringToGuidString
+from Common.Misc import CheckPcdDatum
 from Common.BuildVersion import gBUILD_VERSION
 from Common.MultipleWorkspace import MultipleWorkspace as mws
 
 ## Version and Copyright
 versionNumber = "1.0" + ' ' + gBUILD_VERSION
 __version__ = "%prog Version " + versionNumber
-__copyright__ = "Copyright (c) 2007 - 2014, Intel Corporation  All rights 
reserved."
+__copyright__ = "Copyright (c) 2007 - 2016, Intel Corporation  All rights 
reserved."
 
 ## Tool entrance method
 #
 # This method mainly dispatch specific methods per the command line options.
 # If no error found, return zero value so the caller of this tool can know
@@ -272,10 +273,18 @@ def main():
 

[edk2] [Patch] BaseTools: Add mixed PCD support feature

[Yonghong Zhu]

Problem statement:
The current build system requires that a PCD must use the same access
method for all modules. A Binary Module may use a different PCD access
method than: 1.A source tree build it is integrated into. 2.Other Binary
Modules in platform build that use the same PCD.

Solution:
1. Source build:
No change. PCDs must use the same access method for building all Source
Modules.
2. Mixed Source & Binary Builds or Binary Only Builds:
1) Source Modules - No changes
2) Module that is interpreted as a Binary Module
a.DSC file may optionally override default value of PatchableInModule
PCDs in scope of Binary Module.
b.DSC file must declare DynamicEx PCD subtype for all DynamicEx PCDs
from Binary Modules.
c.FDF file must list Binary Module INF

Build update:
1. PCDs in a binary module are permitted to use the PatchableInModule
or DynamicEx access methods (the Binary INF clearly identifies the PCD
access method for each PCD). The build must support binary modules that
use the same or different PCD access method than the Source INFs or
other Binary INFs.
2. Build report list PCDs that have mixed PCD access methods.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 205 +++--
 BaseTools/Source/Python/AutoGen/GenC.py| 122 ++--
 BaseTools/Source/Python/AutoGen/GenPcdDb.py|  22 ++-
 BaseTools/Source/Python/Common/GlobalData.py   |   5 +
 BaseTools/Source/Python/Common/VpdInfoFile.py  |  15 +-
 .../Source/Python/Workspace/WorkspaceCommon.py |  13 +-
 .../Source/Python/Workspace/WorkspaceDatabase.py   |  57 +-
 BaseTools/Source/Python/build/BuildReport.py   |   8 +
 8 files changed, 360 insertions(+), 87 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index f29d368..70e2720 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -411,10 +411,133 @@ class WorkspaceAutoGen(AutoGen):
 PcdItem.DefaultValue = NewValue
 
 if (TokenCName, TokenSpaceGuidCName) in PcdSet:
 PcdSet[(TokenCName, TokenSpaceGuidCName)] = NewValue
 
+SourcePcdDict = {'DynamicEx':[], 
'PatchableInModule':[],'Dynamic':[],'FixedAtBuild':[]}
+BinaryPcdDict = {'DynamicEx':[], 'PatchableInModule':[]}
+SourcePcdDict_Keys = SourcePcdDict.keys()
+BinaryPcdDict_Keys = BinaryPcdDict.keys()
+
+# generate the SourcePcdDict and BinaryPcdDict
+for BuildData in PGen.BuildDatabase._CACHE_.values():
+if BuildData.Arch != Arch:
+continue
+if BuildData.MetaFile.Ext == '.inf':
+for key in BuildData.Pcds:
+if BuildData.Pcds[key].Pending:
+if key in Platform.Pcds:
+PcdInPlatform = Platform.Pcds[key]
+if PcdInPlatform.Type not in [None, '']:
+BuildData.Pcds[key].Type = 
PcdInPlatform.Type
+
+if BuildData.MetaFile in Platform.Modules:
+PlatformModule = 
Platform.Modules[str(BuildData.MetaFile)]
+if key in PlatformModule.Pcds:
+PcdInPlatform = PlatformModule.Pcds[key]
+if PcdInPlatform.Type not in [None, '']:
+BuildData.Pcds[key].Type = 
PcdInPlatform.Type
+
+if 'DynamicEx' in BuildData.Pcds[key].Type:
+if BuildData.IsBinaryModule:
+if (BuildData.Pcds[key].TokenCName, 
BuildData.Pcds[key].TokenSpaceGuidCName) not in BinaryPcdDict['DynamicEx']:
+
BinaryPcdDict['DynamicEx'].append((BuildData.Pcds[key].TokenCName, 
BuildData.Pcds[key].TokenSpaceGuidCName))
+else:
+if (BuildData.Pcds[key].TokenCName, 
BuildData.Pcds[key].TokenSpaceGuidCName) not in SourcePcdDict['DynamicEx']:
+
SourcePcdDict['DynamicEx'].append((BuildData.Pcds[key].TokenCName, 
BuildData.Pcds[key].TokenSpaceGuidCName))
+
+elif 'PatchableInModule' in BuildData.Pcds[key].Type:
+if BuildData.MetaFile.Ext == '.inf':
+if BuildData.IsBinaryModule:
+if (BuildData.Pcds[key].TokenCName, 
BuildData.Pcds[key].TokenSpaceGuidCName) not in 
BinaryPcdDict['PatchableInModule']:
+
BinaryPcdDict['PatchableInModule'].append((BuildData.Pcds[key].TokenCName, 

[edk2] [Patch] QuarkSocPkg: Add /Oi option to let MemoryInit pass build.

[Liming Gao]

MemoryInit uses the intrinsic memset function. To keep it pass build in VS
tool chain without source code change, /Oi option will be added.

Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf | 4 
 1 file changed, 4 insertions(+)

diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf 
b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
index e327684..78821f5 100644
--- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
+++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
@@ -74,3 +74,7 @@
 
 [Depex]
   TRUE
+
+[BuildOptions]
+  # /Oi option to use the intrinsic memset function in source code.
+  MSFT:*_*_*_CC_FLAGS = /Oi
-- 
2.8.0.windows.1

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

Re: [edk2] [Patch 0/6] Reference PCDs/Protocols defined in MdeModulePkg

[Tian, Feng]

Sorry for missing #4 patch. It's ok to me as well.

Reviewed-by: Feng Tian 

Thanks
Feng


-Original Message-
From: Tian, Feng 
Sent: Monday, April 11, 2016 4:07 PM
To: Ni, Ruiyu ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Tian, Feng 
Subject: RE: [edk2] [Patch 0/6] Reference PCDs/Protocols defined in MdeModulePkg

#1,2,3 &6 patch looks good to me

Reviewed-by: Feng Tian 

Thanks
Feng


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, April 11, 2016 3:47 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: [edk2] [Patch 0/6] Reference PCDs/Protocols defined in MdeModulePkg

The PCD/Protocol used by ISA device drivers are defined in MdeModulePkg.
Change all modules/platforms to reference the new PCDs/Protocols and remove the 
old PCDs/Protocols.

Ruiyu Ni (6):
  IntelFrameworkModulePkg/Ps2Kbd: use PCD/Protocol in MdeModulePkg
  IntelFrameworkModulePkg/Ps2AbsPointer: Use PCD defined in MdeModulePkg
  IntelFrameworkModulePkg/Ps2Mouse: Use PCD defined in MdeModulePkg
  IntelFrameworkModulePkg/KeyboardDxe: Use PCD defined in MdeModulePkg
  Vlv2TbltDevicePkg: Reference the PCD defined in MdeModulePkg
  IntelFrameworkModulePkg: Remove unused PCD/Protocol

 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   |  4 +--
 .../Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf  |  8 ++---
 .../Ps2MouseAbsolutePointer.h  |  4 +--
 .../Ps2MouseAbsolutePointerDxe.inf |  5 ++-
 .../Bus/Isa/Ps2MouseDxe/Ps2Mouse.h |  4 +--
 .../Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf|  5 ++-
 .../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf  |  7 ++--
 .../Include/Protocol/Ps2Policy.h   | 41 --
 .../IntelFrameworkModulePkg.dec| 26 --
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|  4 +--
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |  4 +--
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |  4 +--
 12 files changed, 24 insertions(+), 92 deletions(-)  delete mode 100644 
IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h

--
2.7.0.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

Re: [edk2] [Patch] QuarkSocPkg: Remove intrinsic memset function usages in MemoryInit

[Kinney, Michael D]

Liming,

Please add /Oi to MemoryInitPei.inf for now.

Thanks,

Mike

From: Gao, Liming
Sent: Monday, April 11, 2016 5:26 PM
To: Kinney, Michael D ; edk2-devel@lists.01.org
Subject: RE: [Patch] QuarkSocPkg: Remove intrinsic memset function usages in 
MemoryInit

This way still requires to include Library\BaseMemoryLib.h.

Another way is to append /Oi option in [BuildOptions] section for VS tool 
chain. It has no change to memory init code.

Thanks
Liming
From: Kinney, Michael D
Sent: Tuesday, April 12, 2016 7:54 AM
To: Gao, Liming >; 
edk2-devel@lists.01.org; Kinney, Michael D 
>
Subject: RE: [Patch] QuarkSocPkg: Remove intrinsic memset function usages in 
MemoryInit

Liming,

I think I would prefer we minimize changes to the memory init C sources.

Maybe it would be better if we added #define memset() to SetMem() in
meminit_utils.h based on a define name set in a [BuildOptions] section of
MemoryInitPei.inf

Thanks,

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Sunday, April 10, 2016 8:55 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D
> Subject: [Patch] QuarkSocPkg: Remove intrinsic memset function usages in 
> MemoryInit
>
> Use BaseMemoryLib ZeroMem replace memset function.
>
> Cc: Michael Kinney
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao
> ---
> QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> index 50692fe..321163a 100644
> --- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> +++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> @@ -1,6 +1,6 @@
> /
> *
> - * Copyright (c) 2013-2015 Intel Corporation.
> + * Copyright (c) 2013-2016 Intel Corporation.
> *
> * This program and the accompanying materials
> * are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -40,6 +40,7 @@
> #include "meminit_utils.h"
> #include "hte.h"
> #include "io.h"
> +#include "MemoryInit.h"
>
> // Override ODT to off state if requested
> #define DRMC_DEFAULT (mrc_params->rd_odt_value==0?BIT12:0)
> @@ -1108,7 +1109,7 @@ static void rcvn_cal(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
>
> // loop through each enabled channel
> @@ -1393,7 +1394,7 @@ static void wr_level(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
> // loop through each enabled channel
> for (channel_i = 0; channel_i < NUM_CHANNELS; channel_i++)
> @@ -1647,7 +1648,7 @@ static void rd_train(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
>
> // look for passing coordinates
> @@ -1969,7 +1970,7 @@ static void wr_train(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
>
> // start algorithm on the LEFT side and train each channel/bl until no 
> failures are
> observed, then repeat for the RIGHT side.
> --
> 2.8.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] QuarkSocPkg: Remove intrinsic memset function usages in MemoryInit

[Gao, Liming]

This way still requires to include Library\BaseMemoryLib.h.

Another way is to append /Oi option in [BuildOptions] section for VS tool 
chain. It has no change to memory init code.

Thanks
Liming
From: Kinney, Michael D
Sent: Tuesday, April 12, 2016 7:54 AM
To: Gao, Liming ; edk2-devel@lists.01.org; Kinney, 
Michael D 
Subject: RE: [Patch] QuarkSocPkg: Remove intrinsic memset function usages in 
MemoryInit

Liming,

I think I would prefer we minimize changes to the memory init C sources.

Maybe it would be better if we added #define memset() to SetMem() in
meminit_utils.h based on a define name set in a [BuildOptions] section of
MemoryInitPei.inf

Thanks,

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Sunday, April 10, 2016 8:55 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D
> Subject: [Patch] QuarkSocPkg: Remove intrinsic memset function usages in 
> MemoryInit
>
> Use BaseMemoryLib ZeroMem replace memset function.
>
> Cc: Michael Kinney
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao
> ---
> QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> index 50692fe..321163a 100644
> --- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> +++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> @@ -1,6 +1,6 @@
> /
> *
> - * Copyright (c) 2013-2015 Intel Corporation.
> + * Copyright (c) 2013-2016 Intel Corporation.
> *
> * This program and the accompanying materials
> * are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -40,6 +40,7 @@
> #include "meminit_utils.h"
> #include "hte.h"
> #include "io.h"
> +#include "MemoryInit.h"
>
> // Override ODT to off state if requested
> #define DRMC_DEFAULT (mrc_params->rd_odt_value==0?BIT12:0)
> @@ -1108,7 +1109,7 @@ static void rcvn_cal(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
>
> // loop through each enabled channel
> @@ -1393,7 +1394,7 @@ static void wr_level(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
> // loop through each enabled channel
> for (channel_i = 0; channel_i < NUM_CHANNELS; channel_i++)
> @@ -1647,7 +1648,7 @@ static void rd_train(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
>
> // look for passing coordinates
> @@ -1969,7 +1970,7 @@ static void wr_train(
>
> #ifdef R2R_SHARING
> // need to set "final_delay[][]" elements to "0"
> - memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> + ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
> #endif // R2R_SHARING
>
> // start algorithm on the LEFT side and train each channel/bl until no 
> failures are
> observed, then repeat for the RIGHT side.
> --
> 2.8.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] QuarkSocPkg: Remove intrinsic memset function usages in MemoryInit

[Kinney, Michael D]

Liming,

I think I would prefer we minimize changes to the memory init C sources.

Maybe it would be better if we added #define memset() to SetMem() in 
meminit_utils.h based on a define name set in a [BuildOptions] section of 
MemoryInitPei.inf

Thanks,

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Sunday, April 10, 2016 8:55 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D 
> Subject: [Patch] QuarkSocPkg: Remove intrinsic memset function usages in 
> MemoryInit
> 
> Use BaseMemoryLib ZeroMem replace memset function.
> 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> index 50692fe..321163a 100644
> --- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> +++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> @@ -1,6 +1,6 @@
>  /
>   *
> - * Copyright (c) 2013-2015 Intel Corporation.
> + * Copyright (c) 2013-2016 Intel Corporation.
>   *
>  * This program and the accompanying materials
>  * are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -40,6 +40,7 @@
>  #include "meminit_utils.h"
>  #include "hte.h"
>  #include "io.h"
> +#include "MemoryInit.h"
> 
>  // Override ODT to off state if requested
>  #define DRMC_DEFAULT(mrc_params->rd_odt_value==0?BIT12:0)
> @@ -1108,7 +1109,7 @@ static void rcvn_cal(
> 
>  #ifdef R2R_SHARING
>// need to set "final_delay[][]" elements to "0"
> -  memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> +  ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
>  #endif // R2R_SHARING
> 
>// loop through each enabled channel
> @@ -1393,7 +1394,7 @@ static void wr_level(
> 
>  #ifdef R2R_SHARING
>// need to set "final_delay[][]" elements to "0"
> -  memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> +  ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
>  #endif // R2R_SHARING
>// loop through each enabled channel
>for (channel_i = 0; channel_i < NUM_CHANNELS; channel_i++)
> @@ -1647,7 +1648,7 @@ static void rd_train(
> 
>  #ifdef R2R_SHARING
>// need to set "final_delay[][]" elements to "0"
> -  memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> +  ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
>  #endif // R2R_SHARING
> 
>// look for passing coordinates
> @@ -1969,7 +1970,7 @@ static void wr_train(
> 
>  #ifdef R2R_SHARING
>// need to set "final_delay[][]" elements to "0"
> -  memset((void *) (final_delay), 0x00, (size_t) sizeof(final_delay));
> +  ZeroMem((void *) (final_delay), (size_t) sizeof(final_delay));
>  #endif // R2R_SHARING
> 
>// start algorithm on the LEFT side and train each channel/bl until no 
> failures are
> observed, then repeat for the RIGHT side.
> --
> 2.8.0.windows.1

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

Re: [edk2] TCPIP Client for UEFI

[Andrew Fish]

> On Apr 11, 2016, at 4:29 PM, Jim Slaughter  wrote:
> 
> Hello Patrick,Thank you.I do not have a setup screen with this BIOS/uEFI. Its 
> n SOC on a board but not really a consumer PC type.I need to figure out a way 
> to setup the NIC, find something already written or write it myself. I am not 
> sure if a setup can be done after the actual uefi boot. think Setup on a 
> computer is really before the boot process.All the ifconfig commands give the 
> same protocol error.Any help?Jim S.
> 

Jim,

Not sure about your config setup issues but UEFI boots over the network via 
PXE. That is all documented in the UEFI Spec. It is how PCs have always done 
network booting. 

The 100,000 foot view is the client (UEFI ROM) does a DHCP request and sends 
the CPU architecture and UUID for the platform. The PXE Server in the network 
sends back information about what server and what image to boot. This process 
only happens if you try to network boot from UEFI. What ever got booted (OS 
loader etc.) has access to the location of the boot image so it can continue 
booting. Thus the network boot model is generally configuration less. 

Thanks,

Andrew Fish

> 
>On Saturday, April 9, 2016 11:25 PM, "Mahan, Patrick" 
>  wrote:
> 
> 
> Jim,
> 
> Okay, that’s better.
> 
> So now I would suggest looking at the BIOS/UEFI setup screens for any hints 
> about enabling the network or
> enabling network booting.
> 
> For example, I have a DELL 5810 Precision Tower that I am currently using for 
> UEFI testing.  To enable the
> network stack and PXE booting, I have to go to the config screen that shows 
> the onboard NIC config and
> enable the Network stack and PXE booting.  Not sure what the Insyde screens 
> show, but a quick google-foo
> seems to show that PXE is enabled on the ‘Boot’ config.  I would look for any 
> manual you might have for 
> configuring the BIOS/UEFI.
> 
> Now here is the problem.  You can load the UNDI driver by copying it to your 
> EFI directory on boot device.
> For most Linux systems, this is /boot/efi/EFI/ (e.g. 
> /boot/efi/EFI/ubuntu or /boot/efi/EFI/centos).
> 
> Then start the EFI shell, and in that shell use the ‘load’ command to load 
> the UNDI driver, probably from
> fs0:\EFI\\.efi, then exit the shell and back in the boot menu 
> you should be able to add
> the new NIC.  The assumption here is that when you exit the EFI shell you 
> fall back into the boot menu,
> which is not the case on the DELL I am using.
> 
> If this doesn’t work, then I suggest first contacting the AMD SOC support or 
> the Insyde folks.
> 
> Good luck,
> 
> Patrick
> 
>> On Apr 9, 2016, at 8:00 PM, Jim Slaughter  wrote:
>> 
>> I downloaded there binary as they do not have the source listed,
>> UEFI UNDI Driver
>> from  
>> http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1=13=5=5=4=3=false
>> We are using there chip on out board.
>> If I do an "ifconfig" command (no arguments) I get the following:ifconfig: 
>> "Locate protocol error - ip4Cofi Protocol"
>> Something is missing I think? No ifconfig's work.
>> Jim Slaughter
>> 
>> 
>> On Saturday, April 9, 2016 9:29 AM, "Mahan, Patrick" 
>>  wrote:
>> 
>> 
>> Jim,
>> 
>> What exactly did you download?  If I google ‘realtek refi’ I get the realtek 
>> refi undo driver as a UEFI binary (*.efi) but 
>> no source code.
>> 
>> So I am not sure what you have exactly.  If it is source code then it should 
>> be a UEFI package which should contain
>> all the files to build the NIC.
>> 
>> If you haven’t already, you can take a look at how to get started developing 
>> in UEFI at the following URL -
>> 
>> https://github.com/tianocore/tianocore.github.io/wiki/Getting%20Started%20with%20EDK%20II
>> 
>> (UEFI is based on EDK II)
>> 
>> I personally have never gotten ‘ipconfig’ to work, I instead use ‘ifconfig’. 
>>  Do a ‘ifconfig -?’ for help.
>> 
>> Patrick
>> 
>>> On Apr 8, 2016, at 3:57 PM, Jim Slaughter  wrote:
>>> 
>>> Hello,
>>> The platform is an AMD based SOC. The uEFI is from Insyde. Yes I do have a 
>>> uEFI shell.
>>> When I downloaded the C driver there were no instructions. I have not yet 
>>> configured the card. I have not seen any instructions on how to configure. 
>>> ipconfig does not work.
>>> Jim S.
>>> 
>>> 
>>> On Friday, April 8, 2016 3:40 PM, "Mahan, Patrick" 
>>>  wrote:
>>> 
>>> 
>>> 
>>> Jim,
>>> 
>>> This depends on a number of factors:
>>> 
>>> 1. What platform?  Who wrote the UEFI code for it?  Do you have access to a 
>>> EFI or UEFI shell?
>>> 
>>> 2. I'm not sure about the Realtek NIC, but did they have any instructions 
>>> for using the driver?  UEFI supports
>>> having device specific directories that a Vendor may populate with 
>>> their driver.  That might be the case for
>>> Realtek.
>>> 
>>> 3. Go into your UEFI config (again this depends 

Re: [edk2] TCPIP Client for UEFI

[Jim Slaughter]

Hello Patrick,Thank you.I do not have a setup screen with this BIOS/uEFI. Its n 
SOC on a board but not really a consumer PC type.I need to figure out a way to 
setup the NIC, find something already written or write it myself. I am not sure 
if a setup can be done after the actual uefi boot. think Setup on a computer is 
really before the boot process.All the ifconfig commands give the same protocol 
error.Any help?Jim S.
 

On Saturday, April 9, 2016 11:25 PM, "Mahan, Patrick" 
 wrote:
 

 Jim,

Okay, that’s better.

So now I would suggest looking at the BIOS/UEFI setup screens for any hints 
about enabling the network or
enabling network booting.

For example, I have a DELL 5810 Precision Tower that I am currently using for 
UEFI testing.  To enable the
network stack and PXE booting, I have to go to the config screen that shows the 
onboard NIC config and
enable the Network stack and PXE booting.  Not sure what the Insyde screens 
show, but a quick google-foo
seems to show that PXE is enabled on the ‘Boot’ config.  I would look for any 
manual you might have for 
configuring the BIOS/UEFI.

Now here is the problem.  You can load the UNDI driver by copying it to your 
EFI directory on boot device.
For most Linux systems, this is /boot/efi/EFI/ (e.g. 
/boot/efi/EFI/ubuntu or /boot/efi/EFI/centos).

Then start the EFI shell, and in that shell use the ‘load’ command to load the 
UNDI driver, probably from
fs0:\EFI\\.efi, then exit the shell and back in the boot menu 
you should be able to add
the new NIC.  The assumption here is that when you exit the EFI shell you fall 
back into the boot menu,
which is not the case on the DELL I am using.

If this doesn’t work, then I suggest first contacting the AMD SOC support or 
the Insyde folks.

Good luck,

Patrick

> On Apr 9, 2016, at 8:00 PM, Jim Slaughter  wrote:
> 
> I downloaded there binary as they do not have the source listed,
> UEFI UNDI Driver    
> from  
> http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1=13=5=5=4=3=false
> We are using there chip on out board.
> If I do an "ifconfig" command (no arguments) I get the following:ifconfig: 
> "Locate protocol error - ip4Cofi Protocol"
> Something is missing I think? No ifconfig's work.
> Jim Slaughter
> 
> 
>    On Saturday, April 9, 2016 9:29 AM, "Mahan, Patrick" 
> wrote:
> 
> 
> Jim,
> 
> What exactly did you download?  If I google ‘realtek refi’ I get the realtek 
> refi undo driver as a UEFI binary (*.efi) but 
> no source code.
> 
> So I am not sure what you have exactly.  If it is source code then it should 
> be a UEFI package which should contain
> all the files to build the NIC.
> 
> If you haven’t already, you can take a look at how to get started developing 
> in UEFI at the following URL -
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Getting%20Started%20with%20EDK%20II
> 
> (UEFI is based on EDK II)
> 
> I personally have never gotten ‘ipconfig’ to work, I instead use ‘ifconfig’.  
> Do a ‘ifconfig -?’ for help.
> 
> Patrick
> 
>> On Apr 8, 2016, at 3:57 PM, Jim Slaughter  wrote:
>> 
>> Hello,
>> The platform is an AMD based SOC. The uEFI is from Insyde. Yes I do have a 
>> uEFI shell.
>> When I downloaded the C driver there were no instructions. I have not yet 
>> configured the card. I have not seen any instructions on how to configure. 
>> ipconfig does not work.
>> Jim S.
>> 
>> 
>>    On Friday, April 8, 2016 3:40 PM, "Mahan, Patrick" 
>> wrote:
>> 
>> 
>> 
>> Jim,
>> 
>> This depends on a number of factors:
>> 
>> 1. What platform?  Who wrote the UEFI code for it?  Do you have access to a 
>> EFI or UEFI shell?
>> 
>> 2. I'm not sure about the Realtek NIC, but did they have any instructions 
>> for using the driver?  UEFI supports
>>    having device specific directories that a Vendor may populate with their 
>>driver.  That might be the case for
>>    Realtek.
>> 
>> 3. Go into your UEFI config (again this depends upon whose UEFI is running) 
>> and make sure you have enabled
>>    the network stack.
>> 
>> Always be specific in details, it makes it easier to provide you help,
>> 
>> Thanks,
>> 
>> Patrick
>> 
>> 
>> From: edk2-devel  on behalf of Jim 
>> Slaughter 
>> Sent: Friday, April 8, 2016 3:27 PM
>> To: Edk2-devel
>> Subject: [edk2] TCPIP Client for UEFI
>> 
>> Hello,I downloaded and installed a NIC driver from Realtek for uEFI.I need a 
>> TCPIP client so I can do a dhcp.Where do I find this client?Do I need any 
>> other softwareJim Slaughter
>> 
>> ___
>> 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

Re: [edk2] [PATCH] ShellPkg : Cache the environment variable into memory to enhance the performance.

[Carsey, Jaben]

2 comments below.

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Qiu, Shumin
> Sent: Sunday, April 10, 2016 5:55 AM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; Carsey, Jaben
> ; Ni, Ruiyu 
> Subject: [PATCH] ShellPkg : Cache the environment variable into memory to
> enhance the performance.
> Importance: High
> 
> Currently UEFI Shell reads variable storage to get the environment
> variables every time running a new command. And reading(writing)
> UEFI variables is a high cost operation on most platforms. In order
> to enhance the performance this patch read the variable storage once
> and cache the environment variables in memory. Every further 'set'
> command will save the variable not only to Shell cache, but also the
> flash variable storage.
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin 
> ---
>  ShellPkg/Application/Shell/Shell.c |   4 +
>  ShellPkg/Application/Shell/ShellEnvVar.c   | 175
> -
>  ShellPkg/Application/Shell/ShellEnvVar.h   |  82 +-
>  ShellPkg/Application/Shell/ShellProtocol.c | 101 ++---
>  4 files changed, 317 insertions(+), 45 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index bd695a4..12daff9 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -445,6 +445,8 @@ UefiMain (
>  Status = CommandInit();
>  ASSERT_EFI_ERROR(Status);
> 
> +Status = ShellInitEnvVarList ();
> +
>  //
>  // Check the command line
>  //
> @@ -702,6 +704,8 @@ FreeResources:
>  DEBUG_CODE(ShellInfoObject.ConsoleInfo = NULL;);
>}
> 
> +  ShellFreeEnvVarList ();
> +
>if (ShellCommandGetExit()) {
>  return ((EFI_STATUS)ShellCommandGetExitCode());
>}
> diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c
> b/ShellPkg/Application/Shell/ShellEnvVar.c
> index a8f177e..d2c721d 100644
> --- a/ShellPkg/Application/Shell/ShellEnvVar.c
> +++ b/ShellPkg/Application/Shell/ShellEnvVar.c
> @@ -1,7 +1,7 @@
>  /** @file
>function declarations for shell environment functions.
> 
> -  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2016, 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
> @@ -17,6 +17,11 @@
>  #define INIT_NAME_BUFFER_SIZE  128
>  #define INIT_DATA_BUFFER_SIZE  1024
> 
> +//
> +// The list is used to cache the environment variables.
> +//
> +ENV_VAR_LIST   gShellEnvVarList;
> +
>  /**
>Reports whether an environment variable is Volatile or Non-Volatile.
> 
> @@ -379,3 +384,171 @@ SetEnvironmentVariables(
>//
>return (SetEnvironmentVariableList(>Link));
>  }
> +
> +/**
> +  Find an environment variable in the gShellEnvVarList.
> +
> +  @param KeyThe name of the environment variable.
> +  @param Value  The value of the environment variable, the buffer
> +shoule be freed by the caller.
> +  @param ValueSize  The size in bytes of the environment variable
> +including the tailing CHAR_NELL.
> +  @param Atts   The attributes of the variable.
> +
> +  @retval EFI_SUCCESS   The command executed successfully.
> +  @retval EFI_NOT_FOUND The environment variable is not found in
> +gShellEnvVarList.
> +
> +**/
> +EFI_STATUS
> +ShellFindEnvVarInList (
> +  IN  CONST CHAR16*Key,
> +  OUT CHAR16  **Value,
> +  OUT UINTN   *ValueSize,
> +  OUT UINT32  *Atts OPTIONAL
> +  )
> +{
> +  ENV_VAR_LIST  *Node;
> +
> +  if (Key == NULL || Value == NULL || ValueSize == NULL) {
> +return SHELL_INVALID_PARAMETER;
> +  }
> +
> +  for ( Node = (ENV_VAR_LIST*)GetFirstNode()
> +  ; !IsNull(, >Link)
> +  ; Node = (ENV_VAR_LIST*)GetNextNode(, 
> >Link)
> + ){
> +if (Node->Key != NULL && StrCmp(Key, Node->Key) == 0) {
> +  *Value  = AllocateCopyPool(StrSize(Node->Val), Node->Val);
> +  *ValueSize  = StrSize(Node->Val);
> +  if (Atts != NULL) {
> +*Atts = Node->Atts;
> +  }
> +  return EFI_SUCCESS;
> +}
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
> +/**
> +  Add an environment variable into gShellEnvVarList.
> +
> +  @param KeyThe name of the environment variable.
> +  @param Value  The value of environment variable.
> +  @param ValueSize  The size in bytes of the environment variable
> +including the tailing CHAR_NULL
> +  @param Atts   The attributes of the variable.
> +
> +**/

Re: [edk2] [RFC] EDK2 Staging Proposal 4th Draft

[Laszlo Ersek]

On 04/11/16 20:23, Kinney, Michael D wrote:
> Laszlo,
> 
> I agree branch maintainer should be able to choose to rebase the branch as 
> needed.
> Branch maintainers can also create V1, V2, ..., Vn versions of the feature 
> branch
> as needed.
> 
> Advertising features under development in either personal github branches or 
> the 
> Staging branch is important.  Wiki is one option.  Tony is working on 
> Bugzilla for
> Tianocore with the ability to add a new feature in Bugzilla.  A field in
> Bugzilla with a pointer to the development branch for a new feature would be 
> another option for everyone to find the active branches for features under 
> development.  Davis also suggested use of pull-requests.  That is another way
> a set of features under development can be advertised.
> 
> I personally would prefer using Bugzilla for bugs and feature tracking with 
> maybe a Wiki page that is auto generated from Bugzilla with the features that
> are under active development.

All of these should work fine, as long as we have a static "root
pointer" where a user can start looking, and ultimately end up
- with a repo + branch pair that corresponds to a specific subject
  prefix he has seen on the list,
- with a short description of the feature.

Thanks!
Laszlo

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/11/16 18:22, Ard Biesheuvel wrote:
> On 11 April 2016 at 16:50, Laszlo Ersek  wrote:
>> On 04/11/16 16:34, Ard Biesheuvel wrote:
>>> On 11 April 2016 at 16:12, Laszlo Ersek  wrote:
 On 04/11/16 15:43, Ard Biesheuvel wrote:
>>
>> [snip]
>>
>>> I simply want to check that the 'pci-ecam-generic' DT node we end up
>>> consuming is the only one that exists in the device tree. In fact, I
>>> think it makes sense to assert that PcdPciExpressBaseAddress equals
>>> the DT node reg property when we encounter it.
>>
>> Yes, that definitely makes sense (at least if I understand it
>> correctly). That is, the library constructor would do the "reg" parsing
>> / PCD setting / caching etc (for any DXE_DRIVER that uses this instance,
>> really), and *specifically* PciHostBridgeDxe, when it parses the same
>> node (for those other two properties), could also look at "reg", and
>> assert that PcdPciExpressBaseAddress is *already set* the way it expects
>> it to be. I think that's a good idea.
>>
 Do you have a fresh v3-wip branch somewhere? I'd like to try expressing
 my proposal for patches #13 and #14 in code. (If you are okay with that.)

>>>
>>> Sure, give me 30 mins.
>>
>> Let's do the following instead (as we discussed off-list):
>>
>> Please commit patches v2 1-10 (with the fixes I requested) to the master
>> branch, and then we can collaborate, on the list, with patches for
>> fw_cfg and PciExpressLib / PciHostBridgeDxe, directly against the master
>> branch.
>>
> 
> OK, wip branch is here
> git://git.linaro.org/people/ard.biesheuvel/uefi-next.git virt-fdt-refactor-v3

Great, thank you.

* Patch

  f3089d080cd0 ArmVirtPkg/QemuFwCfgLib: move to FDT client protocol

  is exactly what I had in mind.

  Please modify the commit message (as I pointed out previously, in the
  v2 11/24 sub-thread here) to include "SmbiosPlatformDxe" in the list
  of client drivers, plus update the count "three" to "four". With that,
  you can add my

  Reviewed-by: Laszlo Ersek 

  to the patch.

  I think it's also fine to push the first two patches to master
  afterwards ("ArmVirtPkg/QemuFwCfgLib: move to FDT client protocol"
  and "ArmVirtPkg/VirtFdtDxe: remove handling of fw_cfg DT node"
  (currently a889387fee53)).

After pushing these two patches above (with my requested updates in the
first one), can you please submit a smaller, separate series that
focuses on the conversion of the PCI host node exclusively? Please see
my comments below:

* Patch

  60471256eb10 ArmVirtPkg/BaseCachingPciExpressLib: convert to FDT
   client protocol

  is also precisely what I had in mind.

  - There is one typo in the commit message (with two instances):

s/PciExpressBaseAddress/PcdPciExpressBaseAddress/

  - Also, please add a minimal comment above the 0x
default values in the two DSC files, saying that the MAX_UINT64
value means "undiscovered".

  - Furthermore, please modify the comment on PcdPciExpressBaseAddress
in the INF file like this:

## CONSUMES ## SOMETIMES_PRODUCES

  With those tweaked:

  Reviewed-by: Laszlo Ersek 

  (However, this shouldn't be pushed without the rest below.)

* Can you please submit a separate patch that removes
  PcdPciExpressBaseAddress from "ArmVirtPkg/ArmVirtXen.dsc"?

* Patch

  8528ac9292e4 ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol

  also looks mostly good. Some comments:

  - Please fix a typo in the commit message:

s/host provided/host-provided/

  - Also, the paragraph "This means some PCI related PCDs...", in the
commit message, belongs to the next patch
("ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling"). Please
move it over.

  - The ProcessPciHost() function's leading comment has become stale.
Please drop it.

  - Please do not introduce those global variables just for
returning data from ProcessPciHost() to InitializePciHostBridge().
Use locals and an appropriate signature for ProcessPciHost(). (You
can collect those values in a new structure type too.)

This might require you to bring back those explicit
zero-assignments, before the "ranges" loop.

  - Since the MmioBase and MmioSize values are no longer kept in UINT32
PCDs (but UINT64 variables), the explicit (UINT64) cast is
unnecessary, in InitializePciHostBridge().

  - The nullity checks in (EFI_ERROR (Status) || Prop == NULL) are
redundant, I think, after all of the GetNodeProperty() calls, in
ProcessPciHost().

  - Can you factor out the setting of the /chosen node to a separate
function? (Together with the Tmp variable.)

  - In this (v3-wip) iteration, you also removed the

PcdSetBool (PcdPciDisableBusEnumeration, FALSE);

call. I almost suggested to reinstate it, but doing so wouldn't be
safe actually.

Namely, according to the build report, two drivers consume
PcdPciDisableBusEnumeration: 

Re: [edk2] [RFC] EDK2 Staging Proposal 4th Draft

[Kinney, Michael D]

Laszlo,

I agree branch maintainer should be able to choose to rebase the branch as 
needed.
Branch maintainers can also create V1, V2, ..., Vn versions of the feature 
branch
as needed.

Advertising features under development in either personal github branches or 
the 
Staging branch is important.  Wiki is one option.  Tony is working on Bugzilla 
for
Tianocore with the ability to add a new feature in Bugzilla.  A field in
Bugzilla with a pointer to the development branch for a new feature would be 
another option for everyone to find the active branches for features under 
development.  Davis also suggested use of pull-requests.  That is another way
a set of features under development can be advertised.

I personally would prefer using Bugzilla for bugs and feature tracking with 
maybe a Wiki page that is auto generated from Bugzilla with the features that
are under active development.

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, April 11, 2016 7:42 AM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org ; Mangefeste, Tony
> 
> Subject: Re: [edk2] [RFC] EDK2 Staging Proposal 4th Draft
> 
> On 04/07/16 19:27, Kinney, Michael D wrote:
> > Hello,
> >
> > We left off with the 3rd draft of the edk2-staging proposal with some really
> > good feedback.  The consistent feedback was to keep any process here simple
> > and to use features already available from github where it makes sense to 
> > do so.
> >
> > The main purpose of the edk2-staging repo is to provide an area where 
> > multiple
> > developers can collaborate on a complex feature that is not yet ready for
> > integration into edk2/master.  There is no requirement to use edk2-staging.
> > Developers are welcome us use their own github forks of edk2 to develop a 
> > new
> > feature and discuss it with the edk2 community.  If using one specific 
> > developer's
> > github fork of edk2 becomes cumbersome to manage when multiple developers 
> > are
> > involved, or the ownership of a feature is moving between developers, then
> > a repo such as edk2-staging that is accessible by all edk2 maintainers may 
> > be a
> > viable option to consider.
> >
> > With this in mind, I would like to propose that edk2-staging simply be a 
> > fork
> > of edk2 and any maintainer can choose to create a feature branch in 
> > edk2-staging
> > for the purposes of collaboration on that feature.
> >
> > The active feature branches in edk2-staging may be periodically reviewed to 
> > make
> > sure the feature branches there are under active development and to remove 
> > feature
> > branches that have been abandoned.
> >
> > Mixed in the feedback on edk2-staging were requests to support use of pull 
> > requests.
> > This is an important topic that is also being evaluated, and will be 
> > discussed in
> > separate RFC email discussions.
> >
> > Please review the revised proposal below that removes requirements for 
> > approvals to
> > create/update/remove feature branches in edk2-staging.
> >
> > 
> >
> > Problem statement
> > =
> > Need place on tianocore.org where new features that are not ready for 
> > product
> > integration can be checked in for evaluation by the EDK II community prior 
> > to
> > adding to the edk2 trunk.  This serves several purposes:
> >
> > * Encourage source code to be shared earlier in the development process
> > * Allow source code to be shared that does not yet meet all edk2 required 
> > quality
> criteria
> > * Allow source code to be shared so the EDK II community can help finish 
> > and validate
> new features
> > * Provide a location to hold new features until they are deemed ready for 
> > product
> integration
> > * Provide a location to hold new features until there is a natural point in 
> > edk2
> release cycle to fully validate the new feature.
> > * Not intended to be used for bug fixes.
> > * Not intended to be used for small, simple, or low risk features.
> >
> > Proposal
> > 
> > 1) Create a new repo called edk2-staging
> > a) edk2-staging is a fork of edk2
> > b) edk2-staging/master tracks edk2/master
> >
> > 2) edk2-staging discussions can use the existing edk2-devel mailing list for
> design/patch/test.
> > Use the following style for discussion of a specific feature branch in 
> > edk2-
> staging repo.
> >
> > [staging/branch]: Subject
> >
> > 3) All commits to edk2-staging must follow same edk2 rules (e.g. Tiano 
> > Contributor's
> Agreement)
> >
> > 4) Process to add a new feature to edk2-staging
> > a) Maintainer sends patch email to edk2-devel mailing list announcing 
> > the creation
> of a new
> > feature branch in edk2-staging with Readme.MD.  Readme.MD is in 
> > root of
> feature branch
> > with summary, owners, timeline, links to related materials.
> > b) Maintainer creates feature branch in edk2-staging
> > c) 

Re: [edk2] [PATCH] EmbeddedPkg: Add GICD table init macro for ACPI 6.0

[Ard Biesheuvel]

On 11 April 2016 at 14:31, Heyi Guo  wrote:
> Hello,
>
> Any comments on this patch?
>

I think it looks fine. Leif?

> On 04/07/2016 09:32 AM, Heyi Guo wrote:
>>
>> Add macro to help initialize GICD structure in MADT table according to
>> ACPI 6.0.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Heyi Guo 
>> Cc: Leif Lindholm 
>> Cc: Ard Biesheuvel 
>> ---
>>   EmbeddedPkg/Include/Library/AcpiLib.h | 7 +++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h
>> b/EmbeddedPkg/Include/Library/AcpiLib.h
>> index 74a929c..e5bcf56 100644
>> --- a/EmbeddedPkg/Include/Library/AcpiLib.h
>> +++ b/EmbeddedPkg/Include/Library/AcpiLib.h
>> @@ -38,6 +38,13 @@
>>   GicDistHwId, GicDistBase, GicDistVector, EFI_ACPI_RESERVED_DWORD \
>> }
>>   +#define EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT(GicDistHwId, GicDistBase,
>> GicDistVector, GicVersion) \
>> +  { \
>> +EFI_ACPI_6_0_GICD, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE),
>> EFI_ACPI_RESERVED_WORD, \
>> +GicDistHwId, GicDistBase, GicDistVector, GicVersion, \
>> +{EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE,
>> EFI_ACPI_RESERVED_BYTE} \
>> +  }
>> +
>>   // Note the parking protocol is configured by UEFI if required
>>   #define EFI_ACPI_5_0_GIC_STRUCTURE_INIT(GicId, AcpiCpuId, Flags, PmuIrq,
>> GicBase) \
>> { \
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Ard Biesheuvel]

On 11 April 2016 at 18:47, Mark Rutland  wrote:
> On Mon, Apr 11, 2016 at 06:08:29PM +0200, Ard Biesheuvel wrote:
>> +//VOID
>> +//ArmReplaceLiveTranslationEntry (
>> +//  IN  UINT64  *Entry,
>> +//  IN  UINT64  Value
>> +//  )
>> +ASM_PFX(ArmReplaceLiveTranslationEntry):
>> +  .macro __replace_entry, el
>> +  mrs   x8, sctlr_el\el
>> +  and   x9, x8, #~CTRL_M_BIT  // Clear MMU enable bit
>> +  msr   sctlr_el\el, x9
>> +  isb
>> +
>> +  // write an invalid entry and invalidate it in the caches
>> +  str   xzr, [x0]
>> +  dmb   sy
>> +  dcivac, x0
>
> Is it definitely the case that the line is not potentially dirty?
>
> If it is a possiblity, you need to invalidate first.
>

Yes, that happens before this macro is invoked

>> +  .if   \el == 1
>> +  tlbi  vmalle1
>> +  .else
>> +  tlbi  alle\el
>> +  .endif
>> +  dsb   sy
>> +  msr   sctlr_el\el, x8
>> +  isb
>
> We never did str x1, [x0], so we re-enable the MMU with the entry
> invalid. If that's safe, then there's no point turning the MMU off in
> the first place; this is just a convoluted BBM sequence
>
> I thought the problem was that the entry might be in active use by this
> code (either mapping code or data). For that, you also need to create
> the new entry before re-enabling the MMU.
>

Indeed. I had spotted that myself, and mentioned it in my reply to self

> So long as speculation isn't a problem, you only need the prior
> invalidate. Though for KVM you're at the mercy of the host's cacheable
> alias regardless...
>

Could you elaborate?

>> +  .endm
>> +
>> +  // disable interrupts
>> +  mrs   x2, daif
>> +  msr   daifset, #0xf
>> +  isb
>> +
>> +  // clean and invalidate first so that we don't clobber adjacent entries
>> +  // that are dirty in the caches
>> +  dccivac, x0
>> +  dsb   ish
>> +
>> +  EL1_OR_EL2_OR_EL3(x3)
>> +1:__replace_entry 1
>> +  b 4f
>> +2:__replace_entry 2
>> +  b 4f
>> +3:__replace_entry 3
>> +4:msr   daif, x2
>> +  str   x1, [x0]
>
> As above, this is too late if the entry is in active use.
>

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

Re: [edk2] [PATCH v2] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Mark Rutland]

On Mon, Apr 11, 2016 at 06:08:29PM +0200, Ard Biesheuvel wrote:
> +//VOID
> +//ArmReplaceLiveTranslationEntry (
> +//  IN  UINT64  *Entry,
> +//  IN  UINT64  Value
> +//  )
> +ASM_PFX(ArmReplaceLiveTranslationEntry):
> +  .macro __replace_entry, el
> +  mrs   x8, sctlr_el\el
> +  and   x9, x8, #~CTRL_M_BIT  // Clear MMU enable bit
> +  msr   sctlr_el\el, x9
> +  isb
> +
> +  // write an invalid entry and invalidate it in the caches
> +  str   xzr, [x0]
> +  dmb   sy
> +  dcivac, x0

Is it definitely the case that the line is not potentially dirty?

If it is a possiblity, you need to invalidate first.

> +  .if   \el == 1
> +  tlbi  vmalle1
> +  .else
> +  tlbi  alle\el
> +  .endif
> +  dsb   sy
> +  msr   sctlr_el\el, x8
> +  isb

We never did str x1, [x0], so we re-enable the MMU with the entry
invalid. If that's safe, then there's no point turning the MMU off in
the first place; this is just a convoluted BBM sequence

I thought the problem was that the entry might be in active use by this
code (either mapping code or data). For that, you also need to create
the new entry before re-enabling the MMU.

So long as speculation isn't a problem, you only need the prior
invalidate. Though for KVM you're at the mercy of the host's cacheable
alias regardless...

> +  .endm
> +
> +  // disable interrupts
> +  mrs   x2, daif
> +  msr   daifset, #0xf
> +  isb
> +
> +  // clean and invalidate first so that we don't clobber adjacent entries
> +  // that are dirty in the caches
> +  dccivac, x0
> +  dsb   ish
> +
> +  EL1_OR_EL2_OR_EL3(x3)
> +1:__replace_entry 1
> +  b 4f
> +2:__replace_entry 2
> +  b 4f
> +3:__replace_entry 3
> +4:msr   daif, x2
> +  str   x1, [x0]

As above, this is too late if the entry is in active use.

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

Re: [edk2] [PATCH v2] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Ard Biesheuvel]

On 11 April 2016 at 18:08, Ard Biesheuvel  wrote:
> On ARM, manipulating live page tables is cumbersome since the architecture
> mandates the use of break-before-make, i.e., replacing a block entry with
> a table entry requires an intermediate step via an invalid entry, or TLB
> conflicts may occur.
>
> Since it is not generally feasible to decide in the page table manipulation
> routines whether such an invalid entry will result in those routines
> themselves to become unavailable, use a function that is callable with
> the MMU off (i.e., a leaf function that does not access the stack) to
> perform the change of a block entry into a table entry.
>
> Note that the opposite should never occur, i.e., table entries are never
> coalesced into block entries.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Include/Library/ArmLib.h   |  6 +++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf  |  5 +-
>  ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c | 36 ++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c| 17 ++-
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S| 51 
> 
>  5 files changed, 113 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index 15f610d82e1d..1689f0072db6 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -613,4 +613,10 @@ ArmClearMemoryRegionReadOnly (
>IN  UINT64Length
>);
>
> +VOID
> +ArmReplaceLiveTranslationEntry (
> +  IN  UINT64  *Entry,
> +  IN  UINT64  Value
> +  );
> +
>  #endif // __ARM_LIB__
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> index dd585dea91fb..58684e8492f2 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> @@ -17,9 +17,10 @@ [Defines]
>INF_VERSION= 0x00010005
>BASE_NAME  = AArch64Lib
>FILE_GUID  = ef20ddf5-b334-47b3-94cf-52ff44c29138
> -  MODULE_TYPE= DXE_DRIVER
> +  MODULE_TYPE= BASE
>VERSION_STRING = 1.0
>LIBRARY_CLASS  = ArmLib
> +  CONSTRUCTOR= AArch64LibConstructor
>
>  [Sources.AARCH64]
>AArch64Lib.c
> @@ -31,6 +32,7 @@ [Sources.AARCH64]
>
>../Common/AArch64/ArmLibSupport.S
>../Common/ArmLib.c
> +  AArch64LibConstructor.c
>
>  [Packages]
>ArmPkg/ArmPkg.dec
> @@ -38,6 +40,7 @@ [Packages]
>
>  [LibraryClasses]
>MemoryAllocationLib
> +  CacheMaintenanceLib
>
>  [Protocols]
>gEfiCpuArchProtocolGuid
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
> new file mode 100644
> index ..d2d0d3c15ee3
> --- /dev/null
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
> @@ -0,0 +1,36 @@
> +#/* @file
> +#
> +#  Copyright (c) 2016, Linaro Limited. 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 
> +
> +#include 
> +#include 
> +
> +RETURN_STATUS
> +EFIAPI
> +AArch64LibConstructor (
> +  VOID
> +  )
> +{
> +  extern UINT32 ArmReplaceLiveTranslationEntrySize;
> +
> +  //
> +  // The ArmReplaceLiveTranslationEntry () helper function may be invoked
> +  // with the MMU off so we have to ensure that it gets cleaned to the PoC
> +  //
> +  WriteBackDataCacheRange (ArmReplaceLiveTranslationEntry,
> +ArmReplaceLiveTranslationEntrySize);
> +
> +  return RETURN_SUCCESS;
> +}
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index b7d23c6f3286..2cc6fc45aecf 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -169,6 +169,20 @@ GetRootTranslationTableInfo (
>
>  STATIC
>  VOID
> +ReplaceLiveEntry (
> +  IN  UINT64  *Entry,
> +  IN  UINT64  Value
> +  )
> +{
> +  if (!ArmMmuEnabled ()) {
> +*Entry = Value;
> +  } else {
> +ArmReplaceLiveTranslationEntry (Entry, Value);
> +  }
> +}
> +
> +STATIC
> +VOID
>  LookupAddresstoRootTable (
>IN  UINT64  MaxAddress,
>OUT UINTN  *T0SZ,
> @@ -330,7 +344,8 @@ GetBlockEntryListFromAddress (
>  }
>
>  // Fill the BlockEntry with the new TranslationTable
> -*BlockEntry = ((UINTN)TranslationTable & 

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Ard Biesheuvel]

On 11 April 2016 at 16:50, Laszlo Ersek  wrote:
> On 04/11/16 16:34, Ard Biesheuvel wrote:
>> On 11 April 2016 at 16:12, Laszlo Ersek  wrote:
>>> On 04/11/16 15:43, Ard Biesheuvel wrote:
>
> [snip]
>
>> I simply want to check that the 'pci-ecam-generic' DT node we end up
>> consuming is the only one that exists in the device tree. In fact, I
>> think it makes sense to assert that PcdPciExpressBaseAddress equals
>> the DT node reg property when we encounter it.
>
> Yes, that definitely makes sense (at least if I understand it
> correctly). That is, the library constructor would do the "reg" parsing
> / PCD setting / caching etc (for any DXE_DRIVER that uses this instance,
> really), and *specifically* PciHostBridgeDxe, when it parses the same
> node (for those other two properties), could also look at "reg", and
> assert that PcdPciExpressBaseAddress is *already set* the way it expects
> it to be. I think that's a good idea.
>
>>> Do you have a fresh v3-wip branch somewhere? I'd like to try expressing
>>> my proposal for patches #13 and #14 in code. (If you are okay with that.)
>>>
>>
>> Sure, give me 30 mins.
>
> Let's do the following instead (as we discussed off-list):
>
> Please commit patches v2 1-10 (with the fixes I requested) to the master
> branch, and then we can collaborate, on the list, with patches for
> fw_cfg and PciExpressLib / PciHostBridgeDxe, directly against the master
> branch.
>

OK, wip branch is here
git://git.linaro.org/people/ard.biesheuvel/uefi-next.git virt-fdt-refactor-v3

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

[edk2] [PATCH v2] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Ard Biesheuvel]

On ARM, manipulating live page tables is cumbersome since the architecture
mandates the use of break-before-make, i.e., replacing a block entry with
a table entry requires an intermediate step via an invalid entry, or TLB
conflicts may occur.

Since it is not generally feasible to decide in the page table manipulation
routines whether such an invalid entry will result in those routines
themselves to become unavailable, use a function that is callable with
the MMU off (i.e., a leaf function that does not access the stack) to
perform the change of a block entry into a table entry.

Note that the opposite should never occur, i.e., table entries are never
coalesced into block entries.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Include/Library/ArmLib.h   |  6 +++
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf  |  5 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c | 36 ++
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c| 17 ++-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S| 51 
 5 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 15f610d82e1d..1689f0072db6 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -613,4 +613,10 @@ ArmClearMemoryRegionReadOnly (
   IN  UINT64Length
   );
 
+VOID
+ArmReplaceLiveTranslationEntry (
+  IN  UINT64  *Entry,
+  IN  UINT64  Value
+  );
+
 #endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
index dd585dea91fb..58684e8492f2 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
@@ -17,9 +17,10 @@ [Defines]
   INF_VERSION= 0x00010005
   BASE_NAME  = AArch64Lib
   FILE_GUID  = ef20ddf5-b334-47b3-94cf-52ff44c29138
-  MODULE_TYPE= DXE_DRIVER
+  MODULE_TYPE= BASE
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = ArmLib
+  CONSTRUCTOR= AArch64LibConstructor
 
 [Sources.AARCH64]
   AArch64Lib.c
@@ -31,6 +32,7 @@ [Sources.AARCH64]
 
   ../Common/AArch64/ArmLibSupport.S
   ../Common/ArmLib.c
+  AArch64LibConstructor.c
 
 [Packages]
   ArmPkg/ArmPkg.dec
@@ -38,6 +40,7 @@ [Packages]
 
 [LibraryClasses]
   MemoryAllocationLib
+  CacheMaintenanceLib
 
 [Protocols]
   gEfiCpuArchProtocolGuid
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
new file mode 100644
index ..d2d0d3c15ee3
--- /dev/null
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
@@ -0,0 +1,36 @@
+#/* @file
+#
+#  Copyright (c) 2016, Linaro Limited. 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 
+
+#include 
+#include 
+
+RETURN_STATUS
+EFIAPI
+AArch64LibConstructor (
+  VOID
+  )
+{
+  extern UINT32 ArmReplaceLiveTranslationEntrySize;
+
+  //
+  // The ArmReplaceLiveTranslationEntry () helper function may be invoked
+  // with the MMU off so we have to ensure that it gets cleaned to the PoC
+  //
+  WriteBackDataCacheRange (ArmReplaceLiveTranslationEntry,
+ArmReplaceLiveTranslationEntrySize);
+
+  return RETURN_SUCCESS;
+}
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index b7d23c6f3286..2cc6fc45aecf 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -169,6 +169,20 @@ GetRootTranslationTableInfo (
 
 STATIC
 VOID
+ReplaceLiveEntry (
+  IN  UINT64  *Entry,
+  IN  UINT64  Value
+  )
+{
+  if (!ArmMmuEnabled ()) {
+*Entry = Value;
+  } else {
+ArmReplaceLiveTranslationEntry (Entry, Value);
+  }
+}
+
+STATIC
+VOID
 LookupAddresstoRootTable (
   IN  UINT64  MaxAddress,
   OUT UINTN  *T0SZ,
@@ -330,7 +344,8 @@ GetBlockEntryListFromAddress (
 }
 
 // Fill the BlockEntry with the new TranslationTable
-*BlockEntry = ((UINTN)TranslationTable & 
TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TableAttributes | TT_TYPE_TABLE_ENTRY;
+ReplaceLiveEntry (BlockEntry,
+  ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) | 
TableAttributes | TT_TYPE_TABLE_ENTRY);
   }
 } else {
   if (IndexLevel != PageLevel) {
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 

Re: [edk2] BaseTools: Refinded Multiple Workspaces support.

[Marvin Häuser]

Hello Liming,

Yeah, I didn't quite think the MODULE_DIR comment through, you are definitely 
right about that. Though it would be nice to fix PLATFORM_DIR nevertheless (it 
still is $(WORKSPACE)\AnyPkg, even though it is in a PACKAGES_PATH folder), so 
Makefiles etc. can use it to build their own paths relative to the platform 
directory. I just checked a new Makefile and all other paths seem to be correct 
by now.

Regards,
Marvin.

Von: Gao, Liming 
Gesendet: Montag, 11. April 2016 16:05
An: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
Betreff: RE: [edk2] BaseTools: Refinded Multiple Workspaces support.

Marvin:
 Sorry. I miss this mail. I agree to fix PLATFORM_DIR variable value. But, 
PLATFORM_DIR variable points to the package that places platform DSC file. The 
built module may be from the different packages. So, PLATFORM_DIR can't be used 
to construct MODULE_DIR. After multiple workspaces are supported, there will 
not be root directory to include all built module INF files. So, there is no 
such central variable.

Thanks
Liming
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Marvin 
H?user
Sent: Saturday, April 9, 2016 3:42 AM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] BaseTools: Refinded Multiple Workspaces support.

Dear developers,

I do not want to clutter your inboxes, but there has not been any reply to this 
idea from Februrary. Even though I lack any kind of Python skill and BaseTools 
knowledge, as mentioned in the original E-Mail, I would try to work on it when 
I can (please do not rely on me though, in case you see such a change as 
necessary). Using PLATFORM_DIR in Makefiles would, in my opinion, make the 
Multiple Workspaces concept way better, as currently the project I am 
contributing to uses the WORKSPACE variable, as no other variable provided by 
EDK2 can be relied on when MWS is in use.

I appreciate any kind of feedback.

Best regards,
Marvin.


Von: edk2-devel  im Auftrag von Marvin Häuser 

Gesendet: Sonntag, 21. Februar 2016 02:00
An: edk2-devel@lists.01.org
Betreff: [edk2] BaseTools: Refinded Multiple Workspaces support.

Dear edk2-devel subscribers,

A few moments ago, I have published a patch to the EDK2 BaseTools with the 
subject "BaseTools: Add Multiple Workspaces support  for custom Makefiles.", 
which is aimed at providing Custom Makefile modules support for Multiple 
Workspaces. Writing the patch, I noticed that in quite a few areas WORKSPACE is 
still used (especially in MODULE_DIR, which is the reason custom Makefile 
modules would not be buildable in a different workspace from the primary). In 
the places in which it isn't, mws.join() is called to construct a path by also 
checking the PACKAGES_PATH list.

To simplify the process and ending with cleaner code, I propose to have the 
PLATFORM_DIR variable fixed to point to the correct, absolute path and 
construct other variables (such as MODULE_DIR) from that value, so PLATFORM_DIR 
serves as a central variable (changes to it affect all paths = easier to 
maintain).

Sadly, I do not think I either have the Python skills, or the insight in the 
inner workings of BaseTools to implement such a patch. I would be grateful if 
someone would look into implementing this, or a similar behavior to fully 
support Multiple Workspaces and have an easier to maintain codebase.

Regards,
Marvin.

___
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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/11/16 16:34, Ard Biesheuvel wrote:
> On 11 April 2016 at 16:12, Laszlo Ersek  wrote:
>> On 04/11/16 15:43, Ard Biesheuvel wrote:

[snip]

> I simply want to check that the 'pci-ecam-generic' DT node we end up
> consuming is the only one that exists in the device tree. In fact, I
> think it makes sense to assert that PcdPciExpressBaseAddress equals
> the DT node reg property when we encounter it.

Yes, that definitely makes sense (at least if I understand it
correctly). That is, the library constructor would do the "reg" parsing
/ PCD setting / caching etc (for any DXE_DRIVER that uses this instance,
really), and *specifically* PciHostBridgeDxe, when it parses the same
node (for those other two properties), could also look at "reg", and
assert that PcdPciExpressBaseAddress is *already set* the way it expects
it to be. I think that's a good idea.

>> Do you have a fresh v3-wip branch somewhere? I'd like to try expressing
>> my proposal for patches #13 and #14 in code. (If you are okay with that.)
>>
> 
> Sure, give me 30 mins.

Let's do the following instead (as we discussed off-list):

Please commit patches v2 1-10 (with the fixes I requested) to the master
branch, and then we can collaborate, on the list, with patches for
fw_cfg and PciExpressLib / PciHostBridgeDxe, directly against the master
branch.

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

Re: [edk2] [RFC] EDK2 Staging Proposal 4th Draft

[Laszlo Ersek]

On 04/07/16 19:27, Kinney, Michael D wrote:
> Hello,
> 
> We left off with the 3rd draft of the edk2-staging proposal with some really 
> good feedback.  The consistent feedback was to keep any process here simple 
> and to use features already available from github where it makes sense to do 
> so.
> 
> The main purpose of the edk2-staging repo is to provide an area where multiple
> developers can collaborate on a complex feature that is not yet ready for 
> integration into edk2/master.  There is no requirement to use edk2-staging.
> Developers are welcome us use their own github forks of edk2 to develop a new
> feature and discuss it with the edk2 community.  If using one specific 
> developer's 
> github fork of edk2 becomes cumbersome to manage when multiple developers are
> involved, or the ownership of a feature is moving between developers, then 
> a repo such as edk2-staging that is accessible by all edk2 maintainers may be 
> a 
> viable option to consider.
> 
> With this in mind, I would like to propose that edk2-staging simply be a fork
> of edk2 and any maintainer can choose to create a feature branch in 
> edk2-staging
> for the purposes of collaboration on that feature.
> 
> The active feature branches in edk2-staging may be periodically reviewed to 
> make
> sure the feature branches there are under active development and to remove 
> feature
> branches that have been abandoned.
> 
> Mixed in the feedback on edk2-staging were requests to support use of pull 
> requests.
> This is an important topic that is also being evaluated, and will be 
> discussed in
> separate RFC email discussions.
> 
> Please review the revised proposal below that removes requirements for 
> approvals to
> create/update/remove feature branches in edk2-staging.
> 
> 
> 
> Problem statement
> =
> Need place on tianocore.org where new features that are not ready for product 
> integration can be checked in for evaluation by the EDK II community prior to 
> adding to the edk2 trunk.  This serves several purposes:
> 
> * Encourage source code to be shared earlier in the development process
> * Allow source code to be shared that does not yet meet all edk2 required 
> quality criteria
> * Allow source code to be shared so the EDK II community can help finish and 
> validate new features
> * Provide a location to hold new features until they are deemed ready for 
> product integration
> * Provide a location to hold new features until there is a natural point in 
> edk2 release cycle to fully validate the new feature.
> * Not intended to be used for bug fixes.
> * Not intended to be used for small, simple, or low risk features.
> 
> Proposal
> 
> 1) Create a new repo called edk2-staging
>   a) edk2-staging is a fork of edk2
>   b) edk2-staging/master tracks edk2/master
> 
> 2) edk2-staging discussions can use the existing edk2-devel mailing list for 
> design/patch/test.
>   Use the following style for discussion of a specific feature branch in 
> edk2-staging repo.
> 
>   [staging/branch]: Subject
> 
> 3) All commits to edk2-staging must follow same edk2 rules (e.g. Tiano 
> Contributor's Agreement)
> 
> 4) Process to add a new feature to edk2-staging
>   a) Maintainer sends patch email to edk2-devel mailing list announcing 
> the creation of a new 
> feature branch in edk2-staging with Readme.MD.  Readme.MD is in root 
> of feature branch 
> with summary, owners, timeline, links to related materials.
>   b) Maintainer creates feature branch in edk2-staging
>   c) Maintainer is responsible for making sure feature is frequently 
> synced to edk2/master
> 
>  NOTE: Feature branch may initially use a stable edk2 tag.  As feature 
> stabilizes, 
>syncs to edk2/master can begin.
> 
> 5) Process to update sources in feature branch
>   a) Commit message subject format:
> 
>   [staging/branch PATCH]: Package/Module: Subject
> 
>   b) Directly commit changes to feature branch or if community review is 
> desired,
> then use edk2-devel review process.
> 
>   NOTE: win32 binaries are not automatically generated if a feature 
> branch includes BaseTools source changes.
> 
> 6) Process to promote an edk2-staging branch to edk2 trunk
>   a) Use edk2 patch review/commit process on edk2-devel mailing list.
> The specific git actions used to add the feature to edk2 is at the 
> discretion 
> of the maintainer(s) doing the merge.  A merge commit must always 
> contain the final
> 'Reviewed-by:' tags.
>   b) Remove feature branch from edk2-staging and archive at 
> https://github.com/tianocore/edk2-archive.
> 
> 7) Process to remove an edk2-staging branch
>   a) Stewards may periodically review of feature branches in edk2-staging 
> (once a quarter?)
>   b) If no activity for extended period of time and feature is no longer 
> deemed a candidate 
> for edk2 then stewards send 

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Ard Biesheuvel]

On 11 April 2016 at 16:12, Laszlo Ersek  wrote:
> On 04/11/16 15:43, Ard Biesheuvel wrote:
>> On 11 April 2016 at 15:34, Laszlo Ersek  wrote:
>>> On 04/11/16 14:17, Ard Biesheuvel wrote:
 On 8 April 2016 at 21:15, Laszlo Ersek  wrote:
>>>
>>> [snip]
>>>
> I think this patch is not the right approach.
>
> I would like to preserve the ability to write a DXE_DRIVER that accesses
> PCI config space through this PciExpressLib instance, regardless of its
> own dispatch order against PciHostBridgeDxe.
>
> (Of course, given that PCI may not be available at all in the guest,
> such a DXE_DRIVER will have to check PCI presence explicitly in its
> entry point function, similarly to how PciHostBridgeDxe looks at
> PcdPciExpressBaseAddress at startup.)
>
> Therefore, I suggest the following:
>
> - In our DSC files, set PcdPciExpressBaseAddress to MAX_UINT64. This
>   value is never a valid MMCONFIG base address. It shall mean that
>   PcdPciExpressBaseAddress has not been detected yet.
>
>   We should continue to interpret the value 0 as "detected but
>   unavailable".
>
>   Any other value means "detected and available".
>
> - Please keep the structure of this library instance (constructor etc).
>   In the constructor, if the value of PcdPciExpressBaseAddress is not
>   MAX_UINT64, then assign it to mPciExpressBaseAddress, and return
>   immediately with success.
>
>   Otherwise, parse the DTB as minimally as possible, using the client
>   protocol (-> depex needed), just to determine the value of
>   PcdPciExpressBaseAddress. If PCI is present, set the PCD to
>   the (nonzero) base address. If PCI is absent (or there is some other
>   parse error), set the PCD to zero. Either way, set
>   mPciExpressBaseAddress to the same value, and then return with
>   success.
>
> For the next patch (i.e., PciHostBridgeDxe):
>
> - The PCI presence check should be preserved in PciHostBridgeDxe, in
>   the entry point function (PCD-based). This would (see above) consume
>   the product of our one library instance that is linked into
>   PciHostBridgeDxe itself, *or* into one of those future DXE_DRIVERs
>   that I mentioned above. Because PciHostBridgeDxe itself links against
>   the library, the PCD will never be MAX_UINT64, when the driver entry
>   point checks it.
>
> - If that check passes, then PciHostBridgeDxe should implement its own,
>   more complete FDT traversal (using the FDT client protocol), in order
>   to find all the other characteristics of the pci-host node that it
>   needs for operating.
>
> - Going forward, the PCDs (from ArmPkg) that are currently used for
>   communicating these "other characteristics" from VirtFdtDxe to
>   PciHostBridgeDxe should be eliminated from ArmVirtPkg.
>
> - Drop everything else too that becomes unused (to state the obvious).
>
> Do you agree?
>

 Yes, that makes sense. I coded up the first part, which is
 straightforward enough, but for the second part, I think it is better
 to deal with the MAX_UINT64 case in the code as well. This way, the
 module and the library are independent, and we don't ignore the 'reg'
 property in the module, which is a bit strange as well considering
 that we do consume the rest of the DT node,
>>>
>>> I disagree.
>>>
>>> There are several points in your one paragraph above, so I'll have to
>>> elaborate :) (I didn't want to fragment your paragraph into individual
>>> sentences.)
>>>
>>> Long version
>>> 
>>>
>>> What you are saying (if I understand it correctly) means that any
>>> DXE_DRIVER that needs PCI config space access would have to prepare
>>> itself for an as-yet uninitialized PCD, and perform the initialization
>>> (= the parsing of "reg") itself, explicitly.
>>>
>>> I think this is wrong. I think my point may not have been clear enough:
>>>
> I would like to preserve the ability to write a DXE_DRIVER that
> accesses PCI config space through this PciExpressLib instance,
> regardless of its own dispatch order against PciHostBridgeDxe.
>>>
>>> (Or maybe it was clear, and it's just me not understanding your point.)
>>>
>>
>> No, I did not get the PCI config space nuance here.
>
> Okay, understood.
>
>> I also think that
>> is highly unlikely (and a worse layering violation than the one we are
>> dealing with) to enumerate PCI host bridges dynamically via DT and
>> subsequently wire up a driver to a fixed B/D/F.
>
> The point is exactly that it's not "subsequent".
>

I meant subsequent in Git commit log time, not in boot time.

> Really, it is dictated by the PI and UEFI abstractions (and to some
> extent the edk2 code base):
>
> - The PciIo protocol interfaces come from the generic PCI bus driver,
>   which 

Re: [edk2] [PATCH] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Ard Biesheuvel]

On 11 April 2016 at 16:10, Mark Rutland  wrote:
> Hi Ard,
>
> On Mon, Apr 11, 2016 at 03:57:15PM +0200, Ard Biesheuvel wrote:
>> On ARM, manipulating live page tables is cumbersome since the architecture
>> mandates the use of break-before-make, i.e., replacing a block entry with
>> a table entry requires an intermediate step via an invalid entry, or TLB
>> conflicts may occur.
>>
>> Since it is not generally feasible to decide in the page table manipulation
>> routines whether such an invalid entry will result in those routines
>> themselves to become unavailable, and since UEFI uses an identity mapping
>> anyway, it is far simpler to just disable the MMU, perform the page table
>> manipulations, flush the TLBs and re-enable the MMU.
>
> Perhaps I am missing something, but I suspect that this isn't so simple.

I am afraid it was I who was missing something. Not having a good day ... :-)

> More on that below.
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 39 
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
>> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
>> index b7d23c6f3286..2f8f05df8aec 100644
>> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
>> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
>> @@ -286,6 +286,11 @@ GetBlockEntryListFromAddress (
>>}
>>  } else if ((*BlockEntry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
>>// If we are not at the last level then we need to split this 
>> BlockEntry
>> +  // Since splitting live block entries may cause TLB conflicts, we 
>> need to
>> +  // enter this function with the MMU disabled and flush the TLBs before
>> +  // re-enabling it. This is the responsibility of the caller.
>> +  ASSERT (!ArmMmuEnabled ());
>> +
>>if (IndexLevel != PageLevel) {
>>  // Retrieve the attributes from the block entry
>>  Attributes = *BlockEntry & TT_ATTRIBUTES_MASK;
>> @@ -453,6 +458,8 @@ SetMemoryAttributes (
>>RETURN_STATUSStatus;
>>ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;
>>UINT64  *TranslationTable;
>> +  UINTNSavedInterruptState;
>> +  BOOLEAN  SavedMmuState;
>>
>>MemoryRegion.PhysicalBase = BaseAddress;
>>MemoryRegion.VirtualBase = BaseAddress;
>> @@ -461,6 +468,15 @@ SetMemoryAttributes (
>>
>>TranslationTable = ArmGetTTBR0BaseAddress ();
>>
>> +  SavedMmuState = ArmMmuEnabled ();
>> +  if (SavedMmuState) {
>> +SavedInterruptState = ArmGetInterruptState ();
>> +if (SavedInterruptState) {
>> +  ArmDisableInterrupts();
>> +}
>> +ArmDisableMmu();
>> +  }
>
> Unless ArmDisableMmu() performs a Clean+Invalidate of all memory in use
> by EDK2 (after the MMU is disabled), this is not safe.
>
> For example, the latest version of your stack (which may be dirty in
> cache) is not guaranteed to be visible after the MMU is disabled. If any
> of that is dirty it may be naturally evicted at any time,
> masking/corrupting data written with the MMU off. Any implicit memory
> accesses generated by the compiler may go wrong.
>
> A similar problem applies to anything previously mapped with cacheable
> attributes.
>

Indeed. I can't believe I didn't spot that. I did have the clarity of
mind to put you on cc though :-)


>> +
>>Status = FillTranslationTable (TranslationTable, );
>>if (RETURN_ERROR (Status)) {
>>  return Status;
>> @@ -468,6 +484,12 @@ SetMemoryAttributes (
>>
>>// Invalidate all TLB entries so changes are synced
>>ArmInvalidateTlb ();
>> +  if (SavedMmuState) {
>> +ArmEnableMmu();
>> +if (SavedInterruptState) {
>> +  ArmEnableInterrupts ();
>> +}
>> +  }
>
> Likewise, now everything written with the MMU off is not guaranteed to
> be visible to subsequent cacheable accesses, due to stale lines
> allocated before the MMU was disabled. That includes the modifications
> to the page tables, which may become eventually become visible to
> cacheable accesses in an unpredictable fashion (e.g. you might still end
> up with the TLBs filling with conflicting entries).
>
>>return RETURN_SUCCESS;
>>  }
>> @@ -483,9 +505,20 @@ SetMemoryRegionAttribute (
>>  {
>>RETURN_STATUSStatus;
>>UINT64   *RootTable;
>> +  UINTNSavedInterruptState;
>> +  BOOLEAN  SavedMmuState;
>>
>>RootTable = ArmGetTTBR0BaseAddress ();
>>
>> +  SavedMmuState = ArmMmuEnabled ();
>> +  if (SavedMmuState) {
>> +SavedInterruptState = ArmGetInterruptState ();
>> +if (SavedInterruptState) {
>> +  ArmDisableInterrupts();
>> +}
>> +ArmDisableMmu();
>> +  }
>> +
>>Status = UpdateRegionMapping (RootTable, BaseAddress, Length, Attributes, 
>> BlockEntryMask);
>>if 

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/11/16 15:43, Ard Biesheuvel wrote:
> On 11 April 2016 at 15:34, Laszlo Ersek  wrote:
>> On 04/11/16 14:17, Ard Biesheuvel wrote:
>>> On 8 April 2016 at 21:15, Laszlo Ersek  wrote:
>>
>> [snip]
>>
 I think this patch is not the right approach.

 I would like to preserve the ability to write a DXE_DRIVER that accesses
 PCI config space through this PciExpressLib instance, regardless of its
 own dispatch order against PciHostBridgeDxe.

 (Of course, given that PCI may not be available at all in the guest,
 such a DXE_DRIVER will have to check PCI presence explicitly in its
 entry point function, similarly to how PciHostBridgeDxe looks at
 PcdPciExpressBaseAddress at startup.)

 Therefore, I suggest the following:

 - In our DSC files, set PcdPciExpressBaseAddress to MAX_UINT64. This
   value is never a valid MMCONFIG base address. It shall mean that
   PcdPciExpressBaseAddress has not been detected yet.

   We should continue to interpret the value 0 as "detected but
   unavailable".

   Any other value means "detected and available".

 - Please keep the structure of this library instance (constructor etc).
   In the constructor, if the value of PcdPciExpressBaseAddress is not
   MAX_UINT64, then assign it to mPciExpressBaseAddress, and return
   immediately with success.

   Otherwise, parse the DTB as minimally as possible, using the client
   protocol (-> depex needed), just to determine the value of
   PcdPciExpressBaseAddress. If PCI is present, set the PCD to
   the (nonzero) base address. If PCI is absent (or there is some other
   parse error), set the PCD to zero. Either way, set
   mPciExpressBaseAddress to the same value, and then return with
   success.

 For the next patch (i.e., PciHostBridgeDxe):

 - The PCI presence check should be preserved in PciHostBridgeDxe, in
   the entry point function (PCD-based). This would (see above) consume
   the product of our one library instance that is linked into
   PciHostBridgeDxe itself, *or* into one of those future DXE_DRIVERs
   that I mentioned above. Because PciHostBridgeDxe itself links against
   the library, the PCD will never be MAX_UINT64, when the driver entry
   point checks it.

 - If that check passes, then PciHostBridgeDxe should implement its own,
   more complete FDT traversal (using the FDT client protocol), in order
   to find all the other characteristics of the pci-host node that it
   needs for operating.

 - Going forward, the PCDs (from ArmPkg) that are currently used for
   communicating these "other characteristics" from VirtFdtDxe to
   PciHostBridgeDxe should be eliminated from ArmVirtPkg.

 - Drop everything else too that becomes unused (to state the obvious).

 Do you agree?

>>>
>>> Yes, that makes sense. I coded up the first part, which is
>>> straightforward enough, but for the second part, I think it is better
>>> to deal with the MAX_UINT64 case in the code as well. This way, the
>>> module and the library are independent, and we don't ignore the 'reg'
>>> property in the module, which is a bit strange as well considering
>>> that we do consume the rest of the DT node,
>>
>> I disagree.
>>
>> There are several points in your one paragraph above, so I'll have to
>> elaborate :) (I didn't want to fragment your paragraph into individual
>> sentences.)
>>
>> Long version
>> 
>>
>> What you are saying (if I understand it correctly) means that any
>> DXE_DRIVER that needs PCI config space access would have to prepare
>> itself for an as-yet uninitialized PCD, and perform the initialization
>> (= the parsing of "reg") itself, explicitly.
>>
>> I think this is wrong. I think my point may not have been clear enough:
>>
 I would like to preserve the ability to write a DXE_DRIVER that
 accesses PCI config space through this PciExpressLib instance,
 regardless of its own dispatch order against PciHostBridgeDxe.
>>
>> (Or maybe it was clear, and it's just me not understanding your point.)
>>
> 
> No, I did not get the PCI config space nuance here.

Okay, understood.

> I also think that
> is highly unlikely (and a worse layering violation than the one we are
> dealing with) to enumerate PCI host bridges dynamically via DT and
> subsequently wire up a driver to a fixed B/D/F.

The point is exactly that it's not "subsequent".

Really, it is dictated by the PI and UEFI abstractions (and to some
extent the edk2 code base):

- The PciIo protocol interfaces come from the generic PCI bus driver,
  which is a UEFI_DRIVER, conforming to the UEFI driver model. Which
  implies that drivers that consume PciIo protocol interfaces:

  - have to be UEFI drivers themselves (conforming to the UEFI driver
model),

  - or must register protocol notify 

Re: [edk2] [PATCH] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Mark Rutland]

Hi Ard,

On Mon, Apr 11, 2016 at 03:57:15PM +0200, Ard Biesheuvel wrote:
> On ARM, manipulating live page tables is cumbersome since the architecture
> mandates the use of break-before-make, i.e., replacing a block entry with
> a table entry requires an intermediate step via an invalid entry, or TLB
> conflicts may occur.
> 
> Since it is not generally feasible to decide in the page table manipulation
> routines whether such an invalid entry will result in those routines
> themselves to become unavailable, and since UEFI uses an identity mapping
> anyway, it is far simpler to just disable the MMU, perform the page table
> manipulations, flush the TLBs and re-enable the MMU.

Perhaps I am missing something, but I suspect that this isn't so simple.
More on that below.

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 39 
>  1 file changed, 39 insertions(+)
> 
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index b7d23c6f3286..2f8f05df8aec 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -286,6 +286,11 @@ GetBlockEntryListFromAddress (
>}
>  } else if ((*BlockEntry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
>// If we are not at the last level then we need to split this 
> BlockEntry
> +  // Since splitting live block entries may cause TLB conflicts, we need 
> to
> +  // enter this function with the MMU disabled and flush the TLBs before
> +  // re-enabling it. This is the responsibility of the caller.
> +  ASSERT (!ArmMmuEnabled ());
> +
>if (IndexLevel != PageLevel) {
>  // Retrieve the attributes from the block entry
>  Attributes = *BlockEntry & TT_ATTRIBUTES_MASK;
> @@ -453,6 +458,8 @@ SetMemoryAttributes (
>RETURN_STATUSStatus;
>ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;
>UINT64  *TranslationTable;
> +  UINTNSavedInterruptState;
> +  BOOLEAN  SavedMmuState;
>  
>MemoryRegion.PhysicalBase = BaseAddress;
>MemoryRegion.VirtualBase = BaseAddress;
> @@ -461,6 +468,15 @@ SetMemoryAttributes (
>  
>TranslationTable = ArmGetTTBR0BaseAddress ();
>  
> +  SavedMmuState = ArmMmuEnabled ();
> +  if (SavedMmuState) {
> +SavedInterruptState = ArmGetInterruptState ();
> +if (SavedInterruptState) {
> +  ArmDisableInterrupts();
> +}
> +ArmDisableMmu();
> +  }

Unless ArmDisableMmu() performs a Clean+Invalidate of all memory in use
by EDK2 (after the MMU is disabled), this is not safe.

For example, the latest version of your stack (which may be dirty in
cache) is not guaranteed to be visible after the MMU is disabled. If any
of that is dirty it may be naturally evicted at any time,
masking/corrupting data written with the MMU off. Any implicit memory
accesses generated by the compiler may go wrong.

A similar problem applies to anything previously mapped with cacheable
attributes.

> +
>Status = FillTranslationTable (TranslationTable, );
>if (RETURN_ERROR (Status)) {
>  return Status;
> @@ -468,6 +484,12 @@ SetMemoryAttributes (
>  
>// Invalidate all TLB entries so changes are synced
>ArmInvalidateTlb ();
> +  if (SavedMmuState) {
> +ArmEnableMmu();
> +if (SavedInterruptState) {
> +  ArmEnableInterrupts ();
> +}
> +  }

Likewise, now everything written with the MMU off is not guaranteed to
be visible to subsequent cacheable accesses, due to stale lines
allocated before the MMU was disabled. That includes the modifications
to the page tables, which may become eventually become visible to
cacheable accesses in an unpredictable fashion (e.g. you might still end
up with the TLBs filling with conflicting entries).

>return RETURN_SUCCESS;
>  }
> @@ -483,9 +505,20 @@ SetMemoryRegionAttribute (
>  {
>RETURN_STATUSStatus;
>UINT64   *RootTable;
> +  UINTNSavedInterruptState;
> +  BOOLEAN  SavedMmuState;
>  
>RootTable = ArmGetTTBR0BaseAddress ();
>  
> +  SavedMmuState = ArmMmuEnabled ();
> +  if (SavedMmuState) {
> +SavedInterruptState = ArmGetInterruptState ();
> +if (SavedInterruptState) {
> +  ArmDisableInterrupts();
> +}
> +ArmDisableMmu();
> +  }
> +
>Status = UpdateRegionMapping (RootTable, BaseAddress, Length, Attributes, 
> BlockEntryMask);
>if (RETURN_ERROR (Status)) {
>  return Status;
> @@ -493,6 +526,12 @@ SetMemoryRegionAttribute (
>  
>// Invalidate all TLB entries so changes are synced
>ArmInvalidateTlb ();
> +  if (SavedMmuState) {
> +ArmEnableMmu();
> +if (SavedInterruptState) {
> +  ArmEnableInterrupts ();
> +}
> +  }

The same problems apply to both of these.

Thanks,
Mark.

Re: [edk2] BaseTools: Refinded Multiple Workspaces support.

[Gao, Liming]

Marvin:
 Sorry. I miss this mail. I agree to fix PLATFORM_DIR variable value. But, 
PLATFORM_DIR variable points to the package that places platform DSC file. The 
built module may be from the different packages. So, PLATFORM_DIR can't be used 
to construct MODULE_DIR. After multiple workspaces are supported, there will 
not be root directory to include all built module INF files. So, there is no 
such central variable. 

Thanks
Liming
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Marvin 
H?user
Sent: Saturday, April 9, 2016 3:42 AM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] BaseTools: Refinded Multiple Workspaces support.

Dear developers,

I do not want to clutter your inboxes, but there has not been any reply to this 
idea from Februrary. Even though I lack any kind of Python skill and BaseTools 
knowledge, as mentioned in the original E-Mail, I would try to work on it when 
I can (please do not rely on me though, in case you see such a change as 
necessary). Using PLATFORM_DIR in Makefiles would, in my opinion, make the 
Multiple Workspaces concept way better, as currently the project I am 
contributing to uses the WORKSPACE variable, as no other variable provided by 
EDK2 can be relied on when MWS is in use.

I appreciate any kind of feedback.

Best regards,
Marvin.


Von: edk2-devel  im Auftrag von Marvin Häuser 

Gesendet: Sonntag, 21. Februar 2016 02:00
An: edk2-devel@lists.01.org
Betreff: [edk2] BaseTools: Refinded Multiple Workspaces support.

Dear edk2-devel subscribers,

A few moments ago, I have published a patch to the EDK2 BaseTools with the 
subject "BaseTools: Add Multiple Workspaces support  for custom Makefiles.", 
which is aimed at providing Custom Makefile modules support for Multiple 
Workspaces. Writing the patch, I noticed that in quite a few areas WORKSPACE is 
still used (especially in MODULE_DIR, which is the reason custom Makefile 
modules would not be buildable in a different workspace from the primary). In 
the places in which it isn't, mws.join() is called to construct a path by also 
checking the PACKAGES_PATH list.

To simplify the process and ending with cleaner code, I propose to have the 
PLATFORM_DIR variable fixed to point to the correct, absolute path and 
construct other variables (such as MODULE_DIR) from that value, so PLATFORM_DIR 
serves as a central variable (changes to it affect all paths = easier to 
maintain).

Sadly, I do not think I either have the Python skills, or the insight in the 
inner workings of BaseTools to implement such a patch. I would be grateful if 
someone would look into implementing this, or a similar behavior to fully 
support Multiple Workspaces and have an easier to maintain codebase.

Regards,
Marvin.

___
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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Ard Biesheuvel]

On ARM, manipulating live page tables is cumbersome since the architecture
mandates the use of break-before-make, i.e., replacing a block entry with
a table entry requires an intermediate step via an invalid entry, or TLB
conflicts may occur.

Since it is not generally feasible to decide in the page table manipulation
routines whether such an invalid entry will result in those routines
themselves to become unavailable, and since UEFI uses an identity mapping
anyway, it is far simpler to just disable the MMU, perform the page table
manipulations, flush the TLBs and re-enable the MMU.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 39 
 1 file changed, 39 insertions(+)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index b7d23c6f3286..2f8f05df8aec 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -286,6 +286,11 @@ GetBlockEntryListFromAddress (
   }
 } else if ((*BlockEntry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
   // If we are not at the last level then we need to split this BlockEntry
+  // Since splitting live block entries may cause TLB conflicts, we need to
+  // enter this function with the MMU disabled and flush the TLBs before
+  // re-enabling it. This is the responsibility of the caller.
+  ASSERT (!ArmMmuEnabled ());
+
   if (IndexLevel != PageLevel) {
 // Retrieve the attributes from the block entry
 Attributes = *BlockEntry & TT_ATTRIBUTES_MASK;
@@ -453,6 +458,8 @@ SetMemoryAttributes (
   RETURN_STATUSStatus;
   ARM_MEMORY_REGION_DESCRIPTOR MemoryRegion;
   UINT64  *TranslationTable;
+  UINTNSavedInterruptState;
+  BOOLEAN  SavedMmuState;
 
   MemoryRegion.PhysicalBase = BaseAddress;
   MemoryRegion.VirtualBase = BaseAddress;
@@ -461,6 +468,15 @@ SetMemoryAttributes (
 
   TranslationTable = ArmGetTTBR0BaseAddress ();
 
+  SavedMmuState = ArmMmuEnabled ();
+  if (SavedMmuState) {
+SavedInterruptState = ArmGetInterruptState ();
+if (SavedInterruptState) {
+  ArmDisableInterrupts();
+}
+ArmDisableMmu();
+  }
+
   Status = FillTranslationTable (TranslationTable, );
   if (RETURN_ERROR (Status)) {
 return Status;
@@ -468,6 +484,12 @@ SetMemoryAttributes (
 
   // Invalidate all TLB entries so changes are synced
   ArmInvalidateTlb ();
+  if (SavedMmuState) {
+ArmEnableMmu();
+if (SavedInterruptState) {
+  ArmEnableInterrupts ();
+}
+  }
 
   return RETURN_SUCCESS;
 }
@@ -483,9 +505,20 @@ SetMemoryRegionAttribute (
 {
   RETURN_STATUSStatus;
   UINT64   *RootTable;
+  UINTNSavedInterruptState;
+  BOOLEAN  SavedMmuState;
 
   RootTable = ArmGetTTBR0BaseAddress ();
 
+  SavedMmuState = ArmMmuEnabled ();
+  if (SavedMmuState) {
+SavedInterruptState = ArmGetInterruptState ();
+if (SavedInterruptState) {
+  ArmDisableInterrupts();
+}
+ArmDisableMmu();
+  }
+
   Status = UpdateRegionMapping (RootTable, BaseAddress, Length, Attributes, 
BlockEntryMask);
   if (RETURN_ERROR (Status)) {
 return Status;
@@ -493,6 +526,12 @@ SetMemoryRegionAttribute (
 
   // Invalidate all TLB entries so changes are synced
   ArmInvalidateTlb ();
+  if (SavedMmuState) {
+ArmEnableMmu();
+if (SavedInterruptState) {
+  ArmEnableInterrupts ();
+}
+  }
 
   return RETURN_SUCCESS;
 }
-- 
2.5.0

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Ard Biesheuvel]

On 11 April 2016 at 15:34, Laszlo Ersek  wrote:
> On 04/11/16 14:17, Ard Biesheuvel wrote:
>> On 8 April 2016 at 21:15, Laszlo Ersek  wrote:
>
> [snip]
>
>>> I think this patch is not the right approach.
>>>
>>> I would like to preserve the ability to write a DXE_DRIVER that accesses
>>> PCI config space through this PciExpressLib instance, regardless of its
>>> own dispatch order against PciHostBridgeDxe.
>>>
>>> (Of course, given that PCI may not be available at all in the guest,
>>> such a DXE_DRIVER will have to check PCI presence explicitly in its
>>> entry point function, similarly to how PciHostBridgeDxe looks at
>>> PcdPciExpressBaseAddress at startup.)
>>>
>>> Therefore, I suggest the following:
>>>
>>> - In our DSC files, set PcdPciExpressBaseAddress to MAX_UINT64. This
>>>   value is never a valid MMCONFIG base address. It shall mean that
>>>   PcdPciExpressBaseAddress has not been detected yet.
>>>
>>>   We should continue to interpret the value 0 as "detected but
>>>   unavailable".
>>>
>>>   Any other value means "detected and available".
>>>
>>> - Please keep the structure of this library instance (constructor etc).
>>>   In the constructor, if the value of PcdPciExpressBaseAddress is not
>>>   MAX_UINT64, then assign it to mPciExpressBaseAddress, and return
>>>   immediately with success.
>>>
>>>   Otherwise, parse the DTB as minimally as possible, using the client
>>>   protocol (-> depex needed), just to determine the value of
>>>   PcdPciExpressBaseAddress. If PCI is present, set the PCD to
>>>   the (nonzero) base address. If PCI is absent (or there is some other
>>>   parse error), set the PCD to zero. Either way, set
>>>   mPciExpressBaseAddress to the same value, and then return with
>>>   success.
>>>
>>> For the next patch (i.e., PciHostBridgeDxe):
>>>
>>> - The PCI presence check should be preserved in PciHostBridgeDxe, in
>>>   the entry point function (PCD-based). This would (see above) consume
>>>   the product of our one library instance that is linked into
>>>   PciHostBridgeDxe itself, *or* into one of those future DXE_DRIVERs
>>>   that I mentioned above. Because PciHostBridgeDxe itself links against
>>>   the library, the PCD will never be MAX_UINT64, when the driver entry
>>>   point checks it.
>>>
>>> - If that check passes, then PciHostBridgeDxe should implement its own,
>>>   more complete FDT traversal (using the FDT client protocol), in order
>>>   to find all the other characteristics of the pci-host node that it
>>>   needs for operating.
>>>
>>> - Going forward, the PCDs (from ArmPkg) that are currently used for
>>>   communicating these "other characteristics" from VirtFdtDxe to
>>>   PciHostBridgeDxe should be eliminated from ArmVirtPkg.
>>>
>>> - Drop everything else too that becomes unused (to state the obvious).
>>>
>>> Do you agree?
>>>
>>
>> Yes, that makes sense. I coded up the first part, which is
>> straightforward enough, but for the second part, I think it is better
>> to deal with the MAX_UINT64 case in the code as well. This way, the
>> module and the library are independent, and we don't ignore the 'reg'
>> property in the module, which is a bit strange as well considering
>> that we do consume the rest of the DT node,
>
> I disagree.
>
> There are several points in your one paragraph above, so I'll have to
> elaborate :) (I didn't want to fragment your paragraph into individual
> sentences.)
>
> Long version
> 
>
> What you are saying (if I understand it correctly) means that any
> DXE_DRIVER that needs PCI config space access would have to prepare
> itself for an as-yet uninitialized PCD, and perform the initialization
> (= the parsing of "reg") itself, explicitly.
>
> I think this is wrong. I think my point may not have been clear enough:
>
>>> I would like to preserve the ability to write a DXE_DRIVER that
>>> accesses PCI config space through this PciExpressLib instance,
>>> regardless of its own dispatch order against PciHostBridgeDxe.
>
> (Or maybe it was clear, and it's just me not understanding your point.)
>

No, I did not get the PCI config space nuance here. I also think that
is highly unlikely (and a worse layering violation than the one we are
dealing with) to enumerate PCI host bridges dynamically via DT and
subsequently wire up a driver to a fixed B/D/F.

But I don't feel strongly about this at all, to be honest. The only
thing I will do is add an assertion that there is only a single
'pci-ecam-generic' DT node, since having multiple will break lots of
other stuff anyway, and it also validates the assumption that the
contents of PcdPciExpressBaseAddress correspond with the DT node we
are dealing with.

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/11/16 14:17, Ard Biesheuvel wrote:
> On 8 April 2016 at 21:15, Laszlo Ersek  wrote:

[snip]

>> I think this patch is not the right approach.
>>
>> I would like to preserve the ability to write a DXE_DRIVER that accesses
>> PCI config space through this PciExpressLib instance, regardless of its
>> own dispatch order against PciHostBridgeDxe.
>>
>> (Of course, given that PCI may not be available at all in the guest,
>> such a DXE_DRIVER will have to check PCI presence explicitly in its
>> entry point function, similarly to how PciHostBridgeDxe looks at
>> PcdPciExpressBaseAddress at startup.)
>>
>> Therefore, I suggest the following:
>>
>> - In our DSC files, set PcdPciExpressBaseAddress to MAX_UINT64. This
>>   value is never a valid MMCONFIG base address. It shall mean that
>>   PcdPciExpressBaseAddress has not been detected yet.
>>
>>   We should continue to interpret the value 0 as "detected but
>>   unavailable".
>>
>>   Any other value means "detected and available".
>>
>> - Please keep the structure of this library instance (constructor etc).
>>   In the constructor, if the value of PcdPciExpressBaseAddress is not
>>   MAX_UINT64, then assign it to mPciExpressBaseAddress, and return
>>   immediately with success.
>>
>>   Otherwise, parse the DTB as minimally as possible, using the client
>>   protocol (-> depex needed), just to determine the value of
>>   PcdPciExpressBaseAddress. If PCI is present, set the PCD to
>>   the (nonzero) base address. If PCI is absent (or there is some other
>>   parse error), set the PCD to zero. Either way, set
>>   mPciExpressBaseAddress to the same value, and then return with
>>   success.
>>
>> For the next patch (i.e., PciHostBridgeDxe):
>>
>> - The PCI presence check should be preserved in PciHostBridgeDxe, in
>>   the entry point function (PCD-based). This would (see above) consume
>>   the product of our one library instance that is linked into
>>   PciHostBridgeDxe itself, *or* into one of those future DXE_DRIVERs
>>   that I mentioned above. Because PciHostBridgeDxe itself links against
>>   the library, the PCD will never be MAX_UINT64, when the driver entry
>>   point checks it.
>>
>> - If that check passes, then PciHostBridgeDxe should implement its own,
>>   more complete FDT traversal (using the FDT client protocol), in order
>>   to find all the other characteristics of the pci-host node that it
>>   needs for operating.
>>
>> - Going forward, the PCDs (from ArmPkg) that are currently used for
>>   communicating these "other characteristics" from VirtFdtDxe to
>>   PciHostBridgeDxe should be eliminated from ArmVirtPkg.
>>
>> - Drop everything else too that becomes unused (to state the obvious).
>>
>> Do you agree?
>>
> 
> Yes, that makes sense. I coded up the first part, which is
> straightforward enough, but for the second part, I think it is better
> to deal with the MAX_UINT64 case in the code as well. This way, the
> module and the library are independent, and we don't ignore the 'reg'
> property in the module, which is a bit strange as well considering
> that we do consume the rest of the DT node,

I disagree.

There are several points in your one paragraph above, so I'll have to
elaborate :) (I didn't want to fragment your paragraph into individual
sentences.)

Long version


What you are saying (if I understand it correctly) means that any
DXE_DRIVER that needs PCI config space access would have to prepare
itself for an as-yet uninitialized PCD, and perform the initialization
(= the parsing of "reg") itself, explicitly.

I think this is wrong. I think my point may not have been clear enough:

>> I would like to preserve the ability to write a DXE_DRIVER that
>> accesses PCI config space through this PciExpressLib instance,
>> regardless of its own dispatch order against PciHostBridgeDxe.

(Or maybe it was clear, and it's just me not understanding your point.)

The idea is this: assume for the sake of argument that the "virt"
machine type grows a special (board-specific) PCI B/D/F that is
necessary for implementing a given PI or UEFI protocol, in a DXE_DRIVER
module, without relying on the PCI bus driver and/or PciHostBridgeDxe.

A good example for this, on x86 / OVMF, is the SmmControl2Dxe driver:

- It implements EFI_SMM_CONTROL2_PROTOCOL, in a DXE_RUNTIME_DRIVER.

- It accesses the config space registers of some fixed location PCI
  devices directly with PciLib. (Those B/D/F constants come from Intel's
  ICH9 / Q35 specs.)

- Its entry point function can run before or after the entry point
  function of PciHostBridgeDxe.

This kind of activity is valid for a DXE_(RUNTIME_)DRIVER, and I would
like to keep it available to ArmVirtPkg.

Now, the problem we have here is that PCI(e) is entirely optional in the
"virt" machtype of today, whereas the PciLib / PciExpressLib /
PciSegmentLib classes all assume that PCI(e) is available
unconditionally (and the only variable is "where"). So, these library
classes do not 

Re: [edk2] [PATCH] EmbeddedPkg: Add GICD table init macro for ACPI 6.0

[Heyi Guo]

Hello,

Any comments on this patch?

Regards.

Heyi

On 04/07/2016 09:32 AM, Heyi Guo wrote:

Add macro to help initialize GICD structure in MADT table according to
ACPI 6.0.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
---
  EmbeddedPkg/Include/Library/AcpiLib.h | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h 
b/EmbeddedPkg/Include/Library/AcpiLib.h
index 74a929c..e5bcf56 100644
--- a/EmbeddedPkg/Include/Library/AcpiLib.h
+++ b/EmbeddedPkg/Include/Library/AcpiLib.h
@@ -38,6 +38,13 @@
  GicDistHwId, GicDistBase, GicDistVector, EFI_ACPI_RESERVED_DWORD \
}
  
+#define EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT(GicDistHwId, GicDistBase, GicDistVector, GicVersion) \

+  { \
+EFI_ACPI_6_0_GICD, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE), 
EFI_ACPI_RESERVED_WORD, \
+GicDistHwId, GicDistBase, GicDistVector, GicVersion, \
+{EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE} \
+  }
+
  // Note the parking protocol is configured by UEFI if required
  #define EFI_ACPI_5_0_GIC_STRUCTURE_INIT(GicId, AcpiCpuId, Flags, PmuIrq, 
GicBase) \
{ \


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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Ard Biesheuvel]

On 8 April 2016 at 21:15, Laszlo Ersek  wrote:
> On 04/08/16 11:45, Ard Biesheuvel wrote:
>> Instead of using a constructor, which may reference a dynamic PCD which is
>> set by the DXE entry point of its user, defer the assignment of the global
>> mPciExpressBaseAddress until the first the library is actually invoked.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf |  
>> 1 -
>>  ArmVirtPkg/Library/BaseCachingPciExpressLib/PciExpressLib.c  | 
>> 16 
>>  2 files changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git 
>> a/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf 
>> b/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
>> index f6a346d49f22..5374f1b9a369 100644
>> --- 
>> a/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
>> +++ 
>> b/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
>> @@ -23,7 +23,6 @@ [Defines]
>>MODULE_TYPE= BASE
>>VERSION_STRING = 1.0
>>LIBRARY_CLASS  = PciExpressLib|DXE_DRIVER UEFI_DRIVER 
>> UEFI_APPLICATION
>> -  CONSTRUCTOR= PciExpressLibInitialize
>>
>>  #
>>  #  VALID_ARCHITECTURES   = ARM AARCH64
>> diff --git a/ArmVirtPkg/Library/BaseCachingPciExpressLib/PciExpressLib.c 
>> b/ArmVirtPkg/Library/BaseCachingPciExpressLib/PciExpressLib.c
>> index 6479f53b3714..0ce6d118483b 100644
>> --- a/ArmVirtPkg/Library/BaseCachingPciExpressLib/PciExpressLib.c
>> +++ b/ArmVirtPkg/Library/BaseCachingPciExpressLib/PciExpressLib.c
>> @@ -68,18 +68,7 @@ PciExpressRegisterForRuntimeAccess (
>>return RETURN_UNSUPPORTED;
>>  }
>>
>> -STATIC UINT64 mPciExpressBaseAddress;
>> -
>> -RETURN_STATUS
>> -EFIAPI
>> -PciExpressLibInitialize (
>> -  VOID
>> -  )
>> -{
>> -  mPciExpressBaseAddress = PcdGet64 (PcdPciExpressBaseAddress);
>> -  return RETURN_SUCCESS;
>> -}
>> -
>> +STATIC UINT64 mPciExpressBaseAddress = MAX_UINT64;
>>
>>  /**
>>Gets the base address of PCI Express.
>> @@ -92,6 +81,9 @@ GetPciExpressBaseAddress (
>>VOID
>>)
>>  {
>> +  if (mPciExpressBaseAddress == MAX_UINT64) {
>> +mPciExpressBaseAddress = PcdGet64 (PcdPciExpressBaseAddress);
>> +  }
>>return (VOID*)(UINTN) mPciExpressBaseAddress;
>>  }
>>
>>
>
> I think this patch is not the right approach.
>
> I would like to preserve the ability to write a DXE_DRIVER that accesses
> PCI config space through this PciExpressLib instance, regardless of its
> own dispatch order against PciHostBridgeDxe.
>
> (Of course, given that PCI may not be available at all in the guest,
> such a DXE_DRIVER will have to check PCI presence explicitly in its
> entry point function, similarly to how PciHostBridgeDxe looks at
> PcdPciExpressBaseAddress at startup.)
>
> Therefore, I suggest the following:
>
> - In our DSC files, set PcdPciExpressBaseAddress to MAX_UINT64. This
>   value is never a valid MMCONFIG base address. It shall mean that
>   PcdPciExpressBaseAddress has not been detected yet.
>
>   We should continue to interpret the value 0 as "detected but
>   unavailable".
>
>   Any other value means "detected and available".
>
> - Please keep the structure of this library instance (constructor etc).
>   In the constructor, if the value of PcdPciExpressBaseAddress is not
>   MAX_UINT64, then assign it to mPciExpressBaseAddress, and return
>   immediately with success.
>
>   Otherwise, parse the DTB as minimally as possible, using the client
>   protocol (-> depex needed), just to determine the value of
>   PcdPciExpressBaseAddress. If PCI is present, set the PCD to
>   the (nonzero) base address. If PCI is absent (or there is some other
>   parse error), set the PCD to zero. Either way, set
>   mPciExpressBaseAddress to the same value, and then return with
>   success.
>
> For the next patch (i.e., PciHostBridgeDxe):
>
> - The PCI presence check should be preserved in PciHostBridgeDxe, in
>   the entry point function (PCD-based). This would (see above) consume
>   the product of our one library instance that is linked into
>   PciHostBridgeDxe itself, *or* into one of those future DXE_DRIVERs
>   that I mentioned above. Because PciHostBridgeDxe itself links against
>   the library, the PCD will never be MAX_UINT64, when the driver entry
>   point checks it.
>
> - If that check passes, then PciHostBridgeDxe should implement its own,
>   more complete FDT traversal (using the FDT client protocol), in order
>   to find all the other characteristics of the pci-host node that it
>   needs for operating.
>
> - Going forward, the PCDs (from ArmPkg) that are currently used for
>   communicating these "other characteristics" from VirtFdtDxe to
>   PciHostBridgeDxe should be eliminated from ArmVirtPkg.
>
> - Drop everything else too that becomes 

Re: [edk2] [PATCH 1/1] UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of IO ports

[Laszlo Ersek]

On 04/11/16 13:50, Fan, Jeff wrote:
> Reviewed-by: Jeff Fan 
> 
> VS2013 build pass.

Thanks a lot, Jeff. Commit fb8b54694c53.

Cheers
Laszlo

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

Re: [edk2] [PATCH 1/1] UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of IO ports

[Fan, Jeff]

Reviewed-by: Jeff Fan 

VS2013 build pass.

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Friday, April 08, 2016 6:22 PM
To: edk2-devel-01
Cc: Justen, Jordan L; Ni, Ruiyu; Fan, Jeff; Mark
Subject: [PATCH 1/1] UefiCpuPkg: CpuIo2Dxe: optimize FIFO reads and writes of 
IO ports

* Short description:

  The CpuIoServiceRead() and CpuIoServiceWrite() functions transfer data
  between memory and IO ports with individual Io(Read|Write)(8|16|32)
  function calls, each in an appropriately set up loop.

  On the Ia32 and X64 platforms however, FIFO reads and writes can be
  optimized, by coding them in assembly, and delegating the loop to the
  CPU, with the REP prefix.

  On KVM virtualization hosts, this difference has a huge performance
  impact: if the loop is open-coded, then the virtual machine traps to the
  hypervisor on every single UINT8 / UINT16 / UINT32 transfer, whereas
  with the REP prefix, KVM can transfer up to a page of data per VM trap.
  This is especially noticeable with IDE PIO transfers, where all the data
  are squeezed through IO ports.

* Long description:

  The RootBridgeIoIoRW() function in

PcAtChipsetPkg/PciHostBridgeDxe/PciRootBridgeIo.c

  used to have the exact same IO port acces optimization, dating back
  verbatim to commit 1fd376d9792:

PcAtChipsetPkg/PciHostBridgeDxe: Improve KVM FIFO I/O read/write
  performance

  OvmfPkg cloned the "PcAtChipsetPkg/PciHostBridgeDxe" driver (for
  unrelated reasons), and inherited the optimization from PcAtChipsetPkg.

  The "PcAtChipsetPkg/PciHostBridgeDxe" driver was ultimately removed in
  commit 111d79db47:

PcAtChipsetPkg/PciHostBridge: Remove PciHostBridge driver

  and OvmfPkg too was rebased to the new core Pci Host Bridge Driver, in
  commit 4014885ffd:

OvmfPkg: switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe

  This caused the optimization to go lost. Namely, the
  RootBridgeIoIoRead() and RootBridgeIoIoWrite() functions in the new core
  Pci Host Bridge Driver delegate IO port accesses to
  EFI_CPU_IO2_PROTOCOL. And, in OvmfPkg (and likely most other Ia32 / X64
  edk2 platforms), this protocol is provided by "UefiCpuPkg/CpuIo2Dxe",
  which lacks the optimization.

  Therefore, this patch ports the C source code logic from commit
  1fd376d9792 (see above) to "UefiCpuPkg/CpuIo2Dxe", plus it ports the
  NASM-converted assembly helper functions from OvmfPkg commits
  6026bf460037 and ace1d0517b65:

OvmfPkg PciHostBridgeDxe: Convert Ia32/IoFifo.asm to NASM

OvmfPkg PciHostBridgeDxe: Convert X64/IoFifo.asm to NASM

  In order to support the MSFT and INTEL toolchains as well, the *.asm
  files are ported from OvmfPkg as well, immediately from before the above
  conversion (that is, at 6026bf460037^).

* Notes about the port:

  - The write and read branches from commit 1fd376d9792 are split to the
separate functions CpuIoServiceWrite() and CpuIoServiceRead().

  - The EfiPciWidthUintXX constants are replaced with EfiCpuIoWidthUintXX.

  - The cast expression "(UINTN) Address" is replaced with
"(UINTN)Address" (i.e., no space), because that's how the receiving
functions spell it as well.

  - The labels in the switch statements are unindented by one level, to
match the edk2 coding style (and the rest of UefiCpuPkg) better.

* The first signoff belongs to Jordan, because he authored all of
  1fd376d9792, 6026bf460037 and ace1d0517b65.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
Ref: https://www.redhat.com/archives/vfio-users/2016-April/msg00029.html
Reported-by: Mark 
Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/10424/focus=10432
Reported-by: Jordan Justen 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Cc: Jeff Fan 
Cc: Mark 
---

Notes:
v2:
- port *.asm files too, for INTEL and MSFT [Jordan, Jeff]

 UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf|  11 ++
 UefiCpuPkg/CpuIo2Dxe/IoFifo.h | 176 
 UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c  |  49 ++
 UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.asm  | 140   
UefiCpuPkg/CpuIo2Dxe/Ia32/IoFifo.nasm | 136 +++
 UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.asm   | 126 ++
 UefiCpuPkg/CpuIo2Dxe/X64/IoFifo.nasm  | 125 ++
 7 files changed, 763 insertions(+)

diff --git a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf 
b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
index 8ef8b3d31cff..8af39ff250c0 100644
--- a/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
+++ b/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
@@ -30,7 +30,18 @@ [Defines]
 [Sources]
   CpuIo2Dxe.c
   CpuIo2Dxe.h
+  IoFifo.h
   
+[Sources.IA32]
+  Ia32/IoFifo.nasm | GCC
+  Ia32/IoFifo.asm  | MSFT
+  Ia32/IoFifo.asm  | INTEL
+
+[Sources.X64]

Re: [edk2] [Patch 0/6] Reference PCDs/Protocols defined in MdeModulePkg

[Tian, Feng]

#1,2,3 &6 patch looks good to me

Reviewed-by: Feng Tian 

Thanks
Feng


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, April 11, 2016 3:47 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: [edk2] [Patch 0/6] Reference PCDs/Protocols defined in MdeModulePkg

The PCD/Protocol used by ISA device drivers are defined in MdeModulePkg.
Change all modules/platforms to reference the new PCDs/Protocols and remove the 
old PCDs/Protocols.

Ruiyu Ni (6):
  IntelFrameworkModulePkg/Ps2Kbd: use PCD/Protocol in MdeModulePkg
  IntelFrameworkModulePkg/Ps2AbsPointer: Use PCD defined in MdeModulePkg
  IntelFrameworkModulePkg/Ps2Mouse: Use PCD defined in MdeModulePkg
  IntelFrameworkModulePkg/KeyboardDxe: Use PCD defined in MdeModulePkg
  Vlv2TbltDevicePkg: Reference the PCD defined in MdeModulePkg
  IntelFrameworkModulePkg: Remove unused PCD/Protocol

 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   |  4 +--
 .../Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf  |  8 ++---
 .../Ps2MouseAbsolutePointer.h  |  4 +--
 .../Ps2MouseAbsolutePointerDxe.inf |  5 ++-
 .../Bus/Isa/Ps2MouseDxe/Ps2Mouse.h |  4 +--
 .../Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf|  5 ++-
 .../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf  |  7 ++--
 .../Include/Protocol/Ps2Policy.h   | 41 --
 .../IntelFrameworkModulePkg.dec| 26 --
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|  4 +--
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |  4 +--
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |  4 +--
 12 files changed, 24 insertions(+), 92 deletions(-)  delete mode 100644 
IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h

--
2.7.0.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 v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

[Jiaxin Wu]

v2:
To support the multiple certificate configuration,
EFI_SIGNATURE_LIST format is used for the variable
'TlsCaCertificate'.

This patch is used to enable HTTPS feature. HttpDxe driver
will consume TlsDxe driver. It can both support
http and https feature, it’s depended on the information in URL,
the HTTP instance can be able to determine whether to use http
or https.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Long Qin 
Cc: El-Haj-Mahmoud Samer 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpDriver.h   |8 +-
 NetworkPkg/HttpDxe/HttpDxe.inf|8 +-
 NetworkPkg/HttpDxe/HttpImpl.c |  188 +++-
 NetworkPkg/HttpDxe/HttpProto.c|  395 ++---
 NetworkPkg/HttpDxe/HttpProto.h|   65 +-
 NetworkPkg/HttpDxe/HttpsSupport.c | 1701 +
 NetworkPkg/HttpDxe/HttpsSupport.h |  314 +++
 7 files changed, 2542 insertions(+), 137 deletions(-)
 create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.c
 create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.h

diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 9c0002a..3c30c12 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -1,9 +1,9 @@
 /** @file
   The header files of the driver binding and service binding protocol for 
HttpDxe driver.
 
-  Copyright (c) 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 
   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
@@ -22,10 +22,11 @@
 
 //
 // Libraries
 //
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
@@ -48,12 +49,14 @@
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 
-
+#include 
 //
 // Produced Protocols
 //
 #include 
 
@@ -77,10 +80,11 @@ extern EFI_HTTP_UTILITIES_PROTOCOL  *mHttpUtilities;
 // Include files with function prototypes
 //
 #include "ComponentName.h"
 #include "HttpImpl.h"
 #include "HttpProto.h"
+#include "HttpsSupport.h"
 #include "HttpDns.h"
 
 typedef struct {
   EFI_SERVICE_BINDING_PROTOCOL  *ServiceBinding;
   UINTN NumberOfChildren;
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index bf2cbee..a228c3d 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,9 +1,9 @@
 ## @file
 #  Implementation of EFI HTTP protocol interfaces.
 #
-#  Copyright (c) 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2015 - 2016, 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.
@@ -36,14 +36,17 @@
   HttpDriver.c
   HttpImpl.h
   HttpImpl.c
   HttpProto.h
   HttpProto.c
+  HttpsSupport.h
+  HttpsSupport.c
 
 [LibraryClasses]
   UefiDriverEntryPoint
   UefiBootServicesTableLib
+  UefiRuntimeServicesTableLib
   MemoryAllocationLib
   BaseLib
   UefiLib
   DebugLib
   NetLib
@@ -62,8 +65,11 @@
   gEfiDns4ProtocolGuid ## SOMETIMES_CONSUMES
   gEfiDns6ServiceBindingProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiDns6ProtocolGuid ## SOMETIMES_CONSUMES
   gEfiIp4Config2ProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiIp6ConfigProtocolGuid## SOMETIMES_CONSUMES
+  gEfiTlsServiceBindingProtocolGuid## SOMETIMES_CONSUMES
+  gEfiTlsProtocolGuid  ## SOMETIMES_CONSUMES
+  gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   HttpDxeExtra.uni
\ No newline at end of file
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 63b683e..8d81a90 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -238,10 +238,11 @@ EfiHttpRequest (
   CHAR8 *HostName;
   UINT16RemotePort;
   HTTP_PROTOCOL *HttpInstance;
   BOOLEAN   Configure;
   BOOLEAN   ReConfigure;
+  BOOLEAN   TlsConfigure;
   CHAR8 *RequestStr;
   CHAR8 *Url;
   UINTN UrlLen;
   CHAR16*HostNameStr;
   HTTP_TOKEN_WRAP   *Wrap;
@@ -306,10 +307,38 @@ EfiHttpRequest (
 HttpInstance->Url = Url;
   } 
 
 
   

[edk2] [PATCH v2 4/6] NetworkPkg: TlsDxe driver implementation over OpenSSL

[Jiaxin Wu]

v2:
Refine the MAX_BUFFER_SIZE

This patch is the implementation of EFI TLS Protocol
and EFI TLS Configuration Protocol Interfaces.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Long Qin 
Cc: El-Haj-Mahmoud Samer 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/NetworkPkg.dsc |   3 +
 NetworkPkg/TlsDxe/TlsConfigProtocol.c | 152 +
 NetworkPkg/TlsDxe/TlsDriver.c | 499 +++
 NetworkPkg/TlsDxe/TlsDriver.h | 237 +
 NetworkPkg/TlsDxe/TlsDxe.inf  |  67 
 NetworkPkg/TlsDxe/TlsDxe.uni  |  25 ++
 NetworkPkg/TlsDxe/TlsDxeExtra.uni |  20 ++
 NetworkPkg/TlsDxe/TlsImpl.c   | 280 +++
 NetworkPkg/TlsDxe/TlsImpl.h   | 342 +++
 NetworkPkg/TlsDxe/TlsProtocol.c   | 627 ++
 10 files changed, 2252 insertions(+)
 create mode 100644 NetworkPkg/TlsDxe/TlsConfigProtocol.c
 create mode 100644 NetworkPkg/TlsDxe/TlsDriver.c
 create mode 100644 NetworkPkg/TlsDxe/TlsDriver.h
 create mode 100644 NetworkPkg/TlsDxe/TlsDxe.inf
 create mode 100644 NetworkPkg/TlsDxe/TlsDxe.uni
 create mode 100644 NetworkPkg/TlsDxe/TlsDxeExtra.uni
 create mode 100644 NetworkPkg/TlsDxe/TlsImpl.c
 create mode 100644 NetworkPkg/TlsDxe/TlsImpl.h
 create mode 100644 NetworkPkg/TlsDxe/TlsProtocol.c

diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
index 0695dc1..2712a6a 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -47,10 +47,12 @@
   TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
   HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  OpensslTlsLib|CryptoPkg/Library/OpensslLib/OpensslTlsLib.inf
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
   
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
  
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
@@ -103,10 +105,11 @@
   NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
   NetworkPkg/DnsDxe/DnsDxe.inf
   NetworkPkg/HttpDxe/HttpDxe.inf
   NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
   NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+  NetworkPkg/TlsDxe/TlsDxe.inf
 
   NetworkPkg/Application/IfConfig6/IfConfig6.inf
   NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
   NetworkPkg/Application/VConfig/VConfig.inf
 
diff --git a/NetworkPkg/TlsDxe/TlsConfigProtocol.c 
b/NetworkPkg/TlsDxe/TlsConfigProtocol.c
new file mode 100644
index 000..2855be1
--- /dev/null
+++ b/NetworkPkg/TlsDxe/TlsConfigProtocol.c
@@ -0,0 +1,152 @@
+/** @file
+  Implementation of EFI TLS Configuration Protocol Interfaces.
+
+  Copyright (c) 2016, 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 "TlsImpl.h"
+
+EFI_TLS_CONFIGURATION_PROTOCOL  mTlsConfigurationProtocol = {
+  TlsConfigurationSetData,
+  TlsConfigurationGetData
+};
+
+/**
+  Set TLS configuration data.
+
+  The SetData() function sets TLS configuration to non-volatile storage or 
volatile
+  storage.
+
+  @param[in]  ThisPointer to the 
EFI_TLS_CONFIGURATION_PROTOCOL instance.
+  @param[in]  DataTypeConfiguration data type.
+  @param[in]  DataPointer to configuration data.
+  @param[in]  DataSizeTotal size of configuration data.
+
+  @retval EFI_SUCCESS The TLS configuration data is set 
successfully.
+  @retval EFI_INVALID_PARAMETER   One or more of the following conditions is 
TRUE:
+  This is NULL.
+  Data is NULL.
+  DataSize is 0.
+  @retval EFI_UNSUPPORTED The DataType is unsupported.
+  @retval EFI_OUT_OF_RESOURCESRequired system resources could not be 
allocated.
+
+**/
+EFI_STATUS
+EFIAPI
+TlsConfigurationSetData (
+  IN EFI_TLS_CONFIGURATION_PROTOCOL  *This,
+  IN EFI_TLS_CONFIG_DATA_TYPEDataType,
+  IN VOID*Data,
+  IN UINTN   DataSize
+  )
+{
+  EFI_STATUSStatus;
+  TLS_INSTANCE  *Instance;
+  EFI_TPL   OldTpl;
+
+  Status = EFI_SUCCESS;
+

[edk2] [PATCH v2 3/6] CryptoPkg: Add new TlsLib library

[Jiaxin Wu]

v2:
Refine the MAX_BUFFER_SIZE

This patch is used to add new TlsLib library, which is wrapped
over OpenSSL. The implementation provides TLS library functions
for EFI TLS protocol.

Cc: Long Qin 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: El-Haj-Mahmoud Samer 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 CryptoPkg/CryptoPkg.dec |4 +
 CryptoPkg/CryptoPkg.dsc |1 +
 CryptoPkg/Include/Library/TlsLib.h  |  802 
 CryptoPkg/Library/TlsLib/TlsLib.c   | 1772 +++
 CryptoPkg/Library/TlsLib/TlsLib.inf |   46 +
 CryptoPkg/Library/TlsLib/TlsLib.uni |   19 +
 6 files changed, 2644 insertions(+)
 create mode 100644 CryptoPkg/Include/Library/TlsLib.h
 create mode 100644 CryptoPkg/Library/TlsLib/TlsLib.c
 create mode 100644 CryptoPkg/Library/TlsLib/TlsLib.inf
 create mode 100644 CryptoPkg/Library/TlsLib/TlsLib.uni

diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index e1cdb8e..ea02ad7 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -29,10 +29,14 @@
 [LibraryClasses]
   ##  @libraryclass  Provides basic library functions for cryptographic 
primitives.
   ##
   BaseCryptLib|Include/Library/BaseCryptLib.h
 
+  ##  @libraryclass  Provides TLS library functions for EFI TLS protocol.
+  ##
+  TlsLib|Include/Library/TlsLib.h
+
 [Protocols]
   ## Include/Protocol/RuntimeCrypt.h
   gEfiRuntimeCryptProtocolGuid = { 0xe1475e0c, 0x1746, 0x4802, {0x86, 0x2e, 
0x1, 0x1c, 0x2c, 0x2d, 0x9d, 0x86 }}
 
 [UserExtensions.TianoCore."ExtraFiles"]
diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index bb7f082..c81d349 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -122,10 +122,11 @@
 
###
 [Components]
   CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
   CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+  CryptoPkg/Library/TlsLib/TlsLib.inf
 
   CryptoPkg/Application/Cryptest/Cryptest.inf
 
   CryptoPkg/CryptRuntimeDxe/CryptRuntimeDxe.inf
 
diff --git a/CryptoPkg/Include/Library/TlsLib.h 
b/CryptoPkg/Include/Library/TlsLib.h
new file mode 100644
index 000..d62375b
--- /dev/null
+++ b/CryptoPkg/Include/Library/TlsLib.h
@@ -0,0 +1,802 @@
+/** @file
+  Defines TLS Library APIs.
+
+Copyright (c) 2016, 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.
+
+**/
+
+#ifndef __TLS_LIB_H__
+#define __TLS_LIB_H__
+
+/**
+  Initializes the OpenSSL library.
+
+  This function registers ciphers and digests used directly and indirectly
+  by SSL/TLS, and initializes the readable error messages.
+  This function must be called before any other action takes places.
+
+**/
+VOID
+EFIAPI
+TlsInitialize (
+  VOID
+  );
+
+/**
+  Free an allocated SSL_CTX object.
+
+  @param[in]  TlsCtxPointer to the SSL_CTX object to be released.
+  
+**/
+VOID
+EFIAPI
+TlsCtxFree (
+  IN   VOID  *TlsCtx
+  );
+
+/**
+  Creates a new SSL_CTX object as framework to establish TLS/SSL enabled
+  connections.
+
+  @param[in]  MajorVerMajor Version of TLS/SSL Protocol.
+  @param[in]  MinorVerMinor Version of TLS/SSL Protocol.
+
+  @return  Pointer to an allocated SSL_CTX object.
+   If the creation failed, TlsCtxNew() returns NULL.
+
+**/
+VOID *
+EFIAPI
+TlsCtxNew (
+  IN UINT8MajorVer,
+  IN UINT8MinorVer
+  );
+
+/**
+  Free an allocated TLS object.
+
+  This function removes the TLS object pointed to by Tls and frees up the
+  allocated memory. If Tls is NULL, nothing is done.
+
+  @param[in]  TlsPointer to the TLS object to be freed.
+
+**/
+VOID
+EFIAPI
+TlsFree (
+  IN VOID *Tls
+  );
+
+/**
+  Create a new TLS object for a connection.
+
+  This function creates a new TLS object for a connection. The new object
+  inherits the setting of the underlying context TlsCtx: connection method,
+  options, verification setting.
+
+  @param[in]  TlsCtxPointer to the SSL_CTX object.
+
+  @return  Pointer to an allocated SSL object.
+   If the creation failed, TlsNew() returns NULL.
+
+**/
+VOID *
+EFIAPI
+TlsNew (
+  IN VOID *TlsCtx
+  );
+
+/**
+  Checks if the TLS handshake was done.
+
+  This function will check if the specified TLS handshake was done.
+
+  @param[in]  TlsPointer to the TLS object for 

[edk2] [Patch 5/6] Vlv2TbltDevicePkg: Reference the PCD defined in MdeModulePkg

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: David Wei 
---
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 4 ++--
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 4 ++--
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index c78ff87..d8e48c0 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -589,10 +589,10 @@
 
 
   ## This PCD specifies whether PS2 keyboard does a extended verification 
during start.
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
 
   ## This PCD specifies whether PS2 mouse does a extended verification during 
start.
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
 
 !if $(VARIABLE_INFO_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index dd5f383..0eae7b5 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -589,10 +589,10 @@
 
 
   ## This PCD specifies whether PS2 keyboard does a extended verification 
during start.
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
 
   ## This PCD specifies whether PS2 mouse does a extended verification during 
start.
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
 
 !if $(VARIABLE_INFO_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 7b04efa..4205201 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -589,10 +589,10 @@
 
 
   ## This PCD specifies whether PS2 keyboard does a extended verification 
during start.
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
 
   ## This PCD specifies whether PS2 mouse does a extended verification during 
start.
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
 
 !if $(VARIABLE_INFO_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE
-- 
2.7.0.windows.1

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

[edk2] [Patch 6/6] IntelFrameworkModulePkg: Remove unused PCD/Protocol

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Include/Protocol/Ps2Policy.h   | 41 --
 .../IntelFrameworkModulePkg.dec| 26 --
 2 files changed, 67 deletions(-)
 delete mode 100644 IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h

diff --git a/IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h 
b/IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h
deleted file mode 100644
index 8e915ed..000
--- a/IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/** @file
-  PS/2 policy protocol abstracts the specific platform initialization and 
settings.
-
-Copyright (c) 2006 - 2010, 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.
  
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,  
   
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.  
   
-
-**/
-
-
-#ifndef _PS2_POLICY_PROTOCOL_H_
-#define _PS2_POLICY_PROTOCOL_H_
-
-#define EFI_PS2_POLICY_PROTOCOL_GUID \
-  { \
-0x4df19259, 0xdc71, 0x4d46, {0xbe, 0xf1, 0x35, 0x7b, 0xb5, 0x78, 0xc4, 
0x18 } \
-  }
-
-#define EFI_KEYBOARD_CAPSLOCK   0x0004
-#define EFI_KEYBOARD_NUMLOCK0x0002
-#define EFI_KEYBOARD_SCROLLLOCK 0x0001
-
-typedef
-EFI_STATUS
-(EFIAPI *EFI_PS2_INIT_HARDWARE) (
-  IN  EFI_HANDLE  Handle
-  );
-
-typedef struct {
-  UINT8 KeyboardLight;
-  EFI_PS2_INIT_HARDWARE Ps2InitHardware;
-} EFI_PS2_POLICY_PROTOCOL;
-
-extern EFI_GUID gEfiPs2PolicyProtocolGuid;
-
-#endif
diff --git a/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec 
b/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
index a609f1b..5327242 100644
--- a/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
+++ b/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
@@ -91,10 +91,6 @@
   #  Include/Protocol/IsaAcpi.h
   gEfiIsaAcpiProtocolGuid= { 0x64a892dc, 0x5561, 0x4536, { 0x92, 0xc7, 
0x79, 0x9b, 0xfc, 0x18, 0x33, 0x55 }}
 
-  ## PS/2 policy protocol abstracts the specific platform initialization and 
setting.
-  #  Include/Protocol/Ps2Policy.h
-  gEfiPs2PolicyProtocolGuid  = { 0x4DF19259, 0xDC71, 0x4D46, { 0xBE, 0xF1, 
0x35, 0x7B, 0xB5, 0x78, 0xC4, 0x18 }}
-
   ## OEM Badging Protocol defines the interface to get the OEM badging image 
with the dispaly attribute.
   #  Include/Protocol/OEMBadging.h
   gEfiOEMBadgingProtocolGuid = { 0x170E13C0, 0xBF1B, 0x4218, { 0x87, 0x1D, 
0x2A, 0xBD, 0xC6, 0xF8, 0x87, 0xBC }}
@@ -134,27 +130,12 @@
   # @Prompt Turn on Legacy Support in S3 Boot
   
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPlatformCsmSupport|TRUE|BOOLEAN|0x00010044
 
-  ## Indicates if PS2 keyboard does a extended verification during start.
-  #  Extended verification will take some performance. It can be set to FALSE 
for boot performance.
-  #   TRUE  - Turn on PS2 keyboard extended verification.
-  #   FALSE - Turn off PS2 keyboard extended verification.
-  # @Prompt Turn on PS2 Keyboard Extended Verification
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|TRUE|BOOLEAN|0x00010045
-
   ## Indicates if Framework Acpi Support protocol is installed.  
   #   TRUE  - Install Framework Acpi Support protocol.
   #   FALSE - Doesn't install Framework Acpi Support protocol.
   # @Prompt Enable Framework Acpi Support
   
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdInstallAcpiSupportProtocol|TRUE|BOOLEAN|0x00010046
 
-
-  ## Indicates if PS2 mouse does a extended verification during start.
-  #  Extended verification will take some performance. It can be set to FALSE 
for boot performance.
-  #   TRUE  - Turn on PS2 mouse extended verification. 
-  #   FALSE - Turn off PS2 mouse extended verification. 
-  # @Prompt Turn on PS2 Mouse Extended Verification
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|TRUE|BOOLEAN|0x00010047
-
   ## Indicates if only Boot logo is showed and all message output is disabled 
in BDS.
   #   TRUE  - Only Boot Logo is showed in boot.
   #   FALSE - All messages and Boot Logo are showed in boot.
@@ -266,12 +247,5 @@
   # @Expression 0x8001 | 
(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdHighPmmMemorySize & 0x1000) == 0
   
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdHighPmmMemorySize|0x40|UINT32|0x300a
 
-  ## Indicates if to use the optimized timing for best PS2 detection 
performance.
-  #  Note this PCD could be set to TRUE for best boot performance and set to 
FALSE for best device compatibility.
-  #   TRUE  - Use the optimized timing for best PS2 detection performance.

[edk2] [Patch 2/6] IntelFrameworkModulePkg/Ps2AbsPointer: Use PCD defined in MdeModulePkg

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointer.h | 4 ++--
 .../Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointerDxe.inf| 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git 
a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointer.h
 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointer.h
index a4e2174..17d1847 100644
--- 
a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointer.h
+++ 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointer.h
@@ -1,7 +1,7 @@
 /** @file
   A Ps2MouseAbsolutePointer driver header file
   
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, 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
@@ -15,7 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef __PS2MOUSEABSOLUTEPOINTER_H__
 #define __PS2MOUSEABSOLUTEPOINTER_H__
 
-#include 
+#include 
 
 #include 
 #include 
diff --git 
a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointerDxe.inf
 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointerDxe.inf
index be16671..48adde6 100644
--- 
a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointerDxe.inf
+++ 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseAbsolutePointerDxe/Ps2MouseAbsolutePointerDxe.inf
@@ -4,7 +4,7 @@
 # This driver simulates a touch pad absolute pointing device using a standard
 # PS2 mouse as the input hardware.
 #
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2006 - 2016, 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
@@ -43,7 +43,6 @@
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
-  IntelFrameworkPkg/IntelFrameworkPkg.dec
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
 [LibraryClasses]
@@ -62,7 +61,7 @@
   gEfiDevicePathProtocolGuid## TO_START
 
 [FeaturePcd]
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification   
## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification   ## CONSUMES
 
 #
 # [Event]
-- 
2.7.0.windows.1

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

[edk2] [Patch 3/6] IntelFrameworkModulePkg/Ps2Mouse: Use PCD defined in MdeModulePkg

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2Mouse.h  | 4 ++--
 IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2Mouse.h 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2Mouse.h
index 604fb47..1d9f139 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2Mouse.h
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2Mouse.h
@@ -1,7 +1,7 @@
 /** @file
   PS/2 Mouse driver header file.
   
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, 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
@@ -15,7 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef _PS2MOUSE_H_
 #define _PS2MOUSE_H_
 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf
index ea7af5b..876b09f 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf
@@ -3,7 +3,7 @@
 #
 # This dirver provides support for PS2 based mice.
 #
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2006 - 2016, 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
@@ -42,7 +42,6 @@
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
-  IntelFrameworkPkg/IntelFrameworkPkg.dec
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
 [LibraryClasses]
@@ -61,7 +60,7 @@
   gEfiDevicePathProtocolGuid## TO_START
 
 [FeaturePcd]
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification   
## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification   ## CONSUMES
 
 #
 # [Event]
-- 
2.7.0.windows.1

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

[edk2] [Patch 4/6] IntelFrameworkModulePkg/KeyboardDxe: Use PCD defined in MdeModulePkg

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf 
b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
index a453480..4d45364 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
@@ -4,7 +4,7 @@
 # Ps2 Keyboard driver by using Legacy Bios protocol service and IsaIo protocol 
 # service. This dirver uses legacy INT16 to get the key stroke status.
 #
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions
@@ -45,6 +45,7 @@
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   IntelFrameworkPkg/IntelFrameworkPkg.dec
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
@@ -68,10 +69,10 @@
   gEfiPs2PolicyProtocolGuid ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE  
## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE  ## 
CONSUMES
 
 [Pcd]
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdFastPS2Detection
  ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection  ## 
SOMETIMES_CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   KeyboardDxeExtra.uni
-- 
2.7.0.windows.1

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

[edk2] [Patch 0/6] Reference PCDs/Protocols defined in MdeModulePkg

[Ruiyu Ni]

The PCD/Protocol used by ISA device drivers are defined in MdeModulePkg.
Change all modules/platforms to reference the new PCDs/Protocols and
remove the old PCDs/Protocols.

Ruiyu Ni (6):
  IntelFrameworkModulePkg/Ps2Kbd: use PCD/Protocol in MdeModulePkg
  IntelFrameworkModulePkg/Ps2AbsPointer: Use PCD defined in MdeModulePkg
  IntelFrameworkModulePkg/Ps2Mouse: Use PCD defined in MdeModulePkg
  IntelFrameworkModulePkg/KeyboardDxe: Use PCD defined in MdeModulePkg
  Vlv2TbltDevicePkg: Reference the PCD defined in MdeModulePkg
  IntelFrameworkModulePkg: Remove unused PCD/Protocol

 .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   |  4 +--
 .../Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf  |  8 ++---
 .../Ps2MouseAbsolutePointer.h  |  4 +--
 .../Ps2MouseAbsolutePointerDxe.inf |  5 ++-
 .../Bus/Isa/Ps2MouseDxe/Ps2Mouse.h |  4 +--
 .../Bus/Isa/Ps2MouseDxe/Ps2MouseDxe.inf|  5 ++-
 .../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf  |  7 ++--
 .../Include/Protocol/Ps2Policy.h   | 41 --
 .../IntelFrameworkModulePkg.dec| 26 --
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc|  4 +--
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc  |  4 +--
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc   |  4 +--
 12 files changed, 24 insertions(+), 92 deletions(-)
 delete mode 100644 IntelFrameworkModulePkg/Include/Protocol/Ps2Policy.h

-- 
2.7.0.windows.1

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

[edk2] [Patch 1/6] IntelFrameworkModulePkg/Ps2Kbd: use PCD/Protocol in MdeModulePkg

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  | 4 ++--
 IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
index 2023905..62d0e78 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
@@ -1,7 +1,7 @@
 /** @file
   PS/2 keyboard driver header file
 
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, 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
@@ -15,7 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef _PS2KEYBOARD_H_
 #define _PS2KEYBOARD_H_
 
-#include 
+#include 
 
 #include 
 #include 
diff --git a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf 
b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
index 13c1ea9..9d72ceb 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+++ b/IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
@@ -4,7 +4,7 @@
 # Ps2 Keyboard Driver for UEFI. The keyboard type implemented follows IBM
 # compatible PS2 protocol using Scan Code Set 1.
 #
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2006 - 2016, 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
@@ -43,7 +43,7 @@
 
 [Packages]
   MdePkg/MdePkg.dec
-  IntelFrameworkPkg/IntelFrameworkPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
 [LibraryClasses]
@@ -67,10 +67,10 @@
   gEfiDevicePathProtocolGuid## TO_START
 
 [FeaturePcd]
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification   ## 
CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification   ## CONSUMES
 
 [Pcd]
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdFastPS2Detection ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection ## 
SOMETIMES_CONSUMES
 
 #
 # [Event]
-- 
2.7.0.windows.1

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