date:20160613

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-13 Thread Evgeny Yakovlev

Hi Feng. 

Sorry, i was AFK for the past week.

We have encountered this with Platronix USB headset. This device was reporting 
*USB Interface* descriptor sizes 1 byte larger than what the spec says. Headset 
was simply plugged in when we booted our firmware and UsbCreateDesc failed to 
parse that and returned NULL which eventually crashed the firmware later (i 
will try and send a separate patch to fix that). We fixed UsbCreateDesc and 
device enumeration went fine, all descriptors it sends are correct except for 
this odd size field.

Evgeny

> On 13 Jun 2016, at 05:32, Tian, Feng  wrote:
> 
> Hi, Yakovlev
> 
> Any response on this? 
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Tian, Feng 
> Sent: Monday, June 6, 2016 9:27 AM
> To: Evgeny Yakovlev ; edk2-devel@lists.01.org
> Cc: Tian, Feng 
> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi, Evgenv
> 
> Thanks for your contribution, Evgenv
> 
> Could you let me know what error case you met? Is the Device Desc or Config 
> Desc larger than expected or other descs?
> 
> Why I ask this question is because it would impact Xhci driver's 
> implementation in which we store some desc contents at internal. So looks 
> like XHCI driver also needs to be updated.
> 
> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evgeny 
> Yakovlev
> Sent: Sunday, June 5, 2016 10:29 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> check
> 
> According to spec if the length of a descriptor is smaller than what the 
> specification defines, then the host shall ignore it.
> However if the size is greater than expected the host will ignore the extra 
> bytes and start looking for the next descriptor at the end of actual length 
> returned. Original check did not handle the latter case correctly and only 
> allowed descriptors with lengths exactly as defined in specification.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evgeny Yakovlev 
> ---
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> index 5b8b1aa..fba60da 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> @@ -199,8 +199,8 @@ UsbCreateDesc (
> }
>   }
> 
> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> -  (Head->Type != Type) || (Head->Len != DescLen)) {
> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> +  (Head->Type != Type) || (Head->Len < DescLen)) {
> DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> return NULL;
>   }
> --
> 2.7.4 (Apple Git-66)
> 
> ___
> 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: ignore the binary LIB file in gen_libs

2016-06-13 Thread Yonghong Zhu

For single module build, it would call gen_libs target. then if it use
binary LIB file, it cause build failure.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index be07e46..c8c5fc5 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -865,11 +865,12 @@ cleanlib:
 
self.BuildTargetList.append(self._BUILD_TARGET_TEMPLATE.Replace(TargetDict))
 
 ## For creating makefile targets for dependent libraries
 def ProcessDependentLibrary(self):
 for LibraryAutoGen in self._AutoGenObject.LibraryAutoGenList:
-
self.LibraryBuildDirectoryList.append(self.PlaceMacro(LibraryAutoGen.BuildDir, 
self.Macros))
+if not LibraryAutoGen.IsBinaryModule:
+
self.LibraryBuildDirectoryList.append(self.PlaceMacro(LibraryAutoGen.BuildDir, 
self.Macros))
 
 ## Return a list containing source file's dependencies
 #
 #   @param  FileListThe list of source files
 #   @param  ForceInculeList The list of files which will be included 
forcely
-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH V2 0/3] Remove TimerLib dependency from DP

2016-06-13 Thread Carsey, Jaben

Also for the series. Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Shia, Cinnamon
> Sent: Sunday, June 12, 2016 7:44 PM
> To: Yao, Jiewen ; Zeng, Star ;
> edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Gao, Liming
> 
> Subject: Re: [edk2] [PATCH V2 0/3] Remove TimerLib dependency from DP
> Importance: High
> 
> Series reviewed by: cinnamon.s...@hpe.com
> 
> 
> -Original Message-
> From: Yao, Jiewen [mailto:jiewen@intel.com]
> Sent: Sunday, June 12, 2016 3:34 PM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Gao, Liming
> ; Shia, Cinnamon 
> Subject: RE: [edk2] [PATCH V2 0/3] Remove TimerLib dependency from DP
> 
> Yes, that is a better idea to maintain the dependency.
> Reviewed-by: jiewen@intel.com
> 
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Sunday, June 12, 2016 3:28 PM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Yao, Jiewen
> > ; Gao, Liming ; Cinnamon
> > Shia 
> > Subject: RE: [edk2] [PATCH V2 0/3] Remove TimerLib dependency from DP
> >
> > Thanks all for the R-b. Sorry to disturb you again.
> > Just considered more about the implementation to use PEI performance
> > log HOB, that will make DP hardly depends on *PeiPerformanceLib*.
> > As user may want to only dump DXE or SMM performance data, then
> > PeiPerformanceLib will be not linked and PEI performance log HOB will
> > be not built.
> >
> > DP needs performance protocol or smm performance handler to get
> > performance data, the performance protocol or smm performance handler
> > is installed by DxeCorePerformanceLib or SmmCorePerformanceLib.
> > So it means DP requires SmmCorePerformanceLib or
> > SmmCorePerformanceLib, and we can define PERFORMANCE_PROPERTY
> and
> > install performance property configuration table in
> > DxeCorePerformanceLib and SmmCorePerformanceLib.
> >
> > Please review the updated patch series V3 that will define, install
> > and use performance property configuration table.
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Star Zeng
> > Sent: Wednesday, June 8, 2016 6:24 PM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Yao, Jiewen
> > ; Gao, Liming 
> > Subject: [edk2] [PATCH V2 0/3] Remove TimerLib dependency from DP
> >
> > Current DP implementation depends on TimerLib, as different platforms
> > may implement and use their own TimerLib, it makes the dp needs to be
> > built by platform. The TimerLib dependency can be removed by using PEI
> > performance log HOB to make DP to be generic.
> >
> > Cc: Liming Gao 
> > Cc: Jiewen Yao 
> > Cc: Cinnamon Shia 
> > Cc: Jaben Carsey  Star Zeng (3):
> >   MdeModulePkg: Extend PEI_PERFORMANCE_LOG_HEADER
> >   PerformancePkg Dp_App: Remove TimerLib dependency
> >   ShellPkg UefiDpLib: Remove TimerLib dependency
> >
> >  MdeModulePkg/Include/Guid/Performance.h|  9 +++--
> >  .../Library/PeiPerformanceLib/PeiPerformanceLib.c  |  9 +
> >  PerformancePkg/Dp_App/Dp.c | 42
> > +-
> >  PerformancePkg/Dp_App/Dp.inf   |  5 ++-
> >  PerformancePkg/Dp_App/DpInternal.h |  6 ++--
> >  PerformancePkg/Dp_App/DpProfile.c  |  3 +-
> >  PerformancePkg/Dp_App/DpStrings.uni| 10 --
> >  PerformancePkg/Dp_App/DpTrace.c| 25
> > ++---
> >  PerformancePkg/Dp_App/DpUtilities.c|  1 -
> >  PerformancePkg/Dp_App/Literals.c   |  3 +-
> >  ShellPkg/Library/UefiDpLib/Dp.c| 40
> > +++--
> >  ShellPkg/Library/UefiDpLib/DpInternal.h|  6 ++--
> >  ShellPkg/Library/UefiDpLib/DpProfile.c |  3 +-
> >  ShellPkg/Library/UefiDpLib/DpTrace.c   | 23 +---
> >  ShellPkg/Library/UefiDpLib/DpUtilities.c   |  1 -
> >  ShellPkg/Library/UefiDpLib/Literals.c  |  3 +-
> >  ShellPkg/Library/UefiDpLib/UefiDpLib.inf   |  7 ++--
> >  ShellPkg/Library/UefiDpLib/UefiDpLib.uni   |  4 ++-
> >  ShellPkg/ShellPkg.dsc  |  2 +-
> >  19 files changed, 93 insertions(+), 109 deletions(-)
> >
> > --
> > 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

2016-06-13 Thread Ard Biesheuvel

On some platforms, performing cache maintenance on regions that are backed
by NOR flash result in SErrors. Since cache maintenance is unnecessary in
that case, create a PEIM specific version that only performs said cache
maintenance in its constructor if the module is shadowed in RAM. To avoid
performing the cache maintenance if the MMU code is not used to begin with,
check that explicitly in the constructor.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---

As discussed in the thread dedicated to this subject, the preferred way of
addressing this to split off the MMU manipulation code from ArmLib into a
separate ArmMmuLib, but this affects other packages and platforms. So in the
mean time, let's merge this patch so that D02 can use the upstream ArmLib
unmodified.


 ArmPkg/Library/ArmLib/AArch64/{AArch64LibConstructor.c => 
AArch64BaseLibConstructor.c} |  0
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf   
|  2 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf
| 43 +++
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
|  2 +
 ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c   
| 75 
 5 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64BaseLibConstructor.c
similarity index 100%
rename from ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
rename to ArmPkg/Library/ArmLib/AArch64/AArch64BaseLibConstructor.c
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
index 58684e8492f2..ef9d261b910d 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
@@ -32,7 +32,7 @@ [Sources.AARCH64]
 
   ../Common/AArch64/ArmLibSupport.S
   ../Common/ArmLib.c
-  AArch64LibConstructor.c
+  AArch64BaseLibConstructor.c
 
 [Packages]
   ArmPkg/ArmPkg.dec
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf 
b/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf
new file mode 100644
index ..c8f0b97750d4
--- /dev/null
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf
@@ -0,0 +1,43 @@
+#/** @file
+#
+# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
+# Portions copyright (c) 2011 - 2014, ARM 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.
+#
+#
+#**/
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = AArch64Lib
+  FILE_GUID  = ef20ddf5-b334-47b3-94cf-52ff44c29138
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmLib|PEIM PEI_CORE
+  CONSTRUCTOR= AArch64LibConstructor
+
+[Sources.AARCH64]
+  AArch64Lib.c
+  AArch64Mmu.c
+  AArch64ArchTimer.c
+  ArmLibSupportV8.S
+  AArch64Support.S
+  AArch64ArchTimerSupport.S
+
+  ../Common/AArch64/ArmLibSupport.S
+  ../Common/ArmLib.c
+  AArch64PeiLibConstructor.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  MemoryAllocationLib
+  CacheMaintenanceLib
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index cf9b7222b47b..07864bac28e6 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -26,6 +26,8 @@
 // We use this index definition to define an invalid block entry
 #define TT_ATTR_INDX_INVALID((UINT32)~0)
 
+INT32 HaveMmuRoutines = 1;
+
 STATIC
 UINT64
 ArmMemoryAttributeToPageAttribute (
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c
new file mode 100644
index ..2de9c7c54ed9
--- /dev/null
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c
@@ -0,0 +1,75 @@
+#/* @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 
+#include 
+
+//
+// This i

Re: [edk2] edk2 llvm branch

2016-06-13 Thread Shi, Steven

Hi Andrew,
Good news! After enhance the GenFw tool, I finally enable the clang LTO build 
on edk2 and pass test on the OVMF three platforms (OvmfPkgIa32.dsc, 
OvmfPkgX64.dsc, OvmfPkgIa32X64.dsc) in my side . Below is the code size 
comparing data between VS2015x86 and CLANGLTO38. You can see they are quite 
close and their gap is almost less than 10%.


I’ve push all my enhancement in edk2 llvm branch:  
https://github.com/shijunjing/edk2/tree/llvm, and please use “git diff master 
--name-only” to see all my updated from master.

Thank you again for the valuable advices during the enabling.

Image
toolchain

RELEASE IA32 PeiCore.efi

DEBUG IA32 PeiCore.efi

RELEASE X64 PeiCore.efi

DEBUG X64 PeiCore.efi

RELEASE X64 DxeCore.efi

DEBUG X64 DxeCore.efi

RELEASE IA32 StatusCodePei.efi

DEBUG IA32 StatusCodePei.efi

RELEASE X64 Timer.efi

DEBUG X64 Timer.efi

VS2015x86

20,512

40,832

25,440

47,808

97,600

141,216

1,408

8,736

1,888

9,920

CLANGLTO38

22,656

42,176

27,200

46,912

103,616

142,080

1,344

7,872

1,664

8,576

(CLANGLTO38 - VS2015x86)/ VS2015x86

10.45%

3.29%

6.9%

-1.8%

6.1%

0.6%

-4.5%

-9.8%

-11.8%

-13.5%


Image build commands:

VS2015x86:
build -a IA32 -t VS2015x86-p OvmfPkg\OvmfPkgIa32.dsc -n 5 -b RELEASE
build -a IA32 -t VS2015x86-p OvmfPkg\OvmfPkgIa32.dsc -n 5 -b DEBUG 
-DDEBUG_ON_SERIAL_PORT
build -a X64 -t VS2015x86-p OvmfPkg\OvmfPkgX64.dsc -n 5 -b RELEASE
build -a X64 -t VS2015x86-p OvmfPkg\OvmfPkgX64.dsc -n 5 -b DEBUG 
-DDEBUG_ON_SERIAL_PORT

CLANGLTO38:
build -a IA32 -t CLANGLTO38 -p OvmfPkg/OvmfPkgIa32.dsc -n 5 -b RELEASE
build -a IA32 -t CLANGLTO38 -p OvmfPkg/OvmfPkgIa32.dsc -n 5 -b DEBUG 
-DDEBUG_ON_SERIAL_PORT
build -a X64 -t CLANGLTO38 -p OvmfPkg/OvmfPkgX64.dsc -n 5 -b RELEASE
build -a X64 -t CLANGLTO38 -p OvmfPkg/OvmfPkgX64.dsc -n 5  -b DEBUG 
-DDEBUG_ON_SERIAL_PORT


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

2016-06-13 Thread Mark Rutland

On Mon, Jun 13, 2016 at 05:26:07PM +0200, Ard Biesheuvel wrote:
> On some platforms, performing cache maintenance on regions that are backed
> by NOR flash result in SErrors. Since cache maintenance is unnecessary in
> that case, create a PEIM specific version that only performs said cache
> maintenance in its constructor if the module is shadowed in RAM. To avoid
> performing the cache maintenance if the MMU code is not used to begin with,
> check that explicitly in the constructor.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
> 
> As discussed in the thread dedicated to this subject, the preferred way of
> addressing this to split off the MMU manipulation code from ArmLib into a
> separate ArmMmuLib, but this affects other packages and platforms. So in the
> mean time, let's merge this patch so that D02 can use the upstream ArmLib
> unmodified.

I'm missing something here (and couldn't figure out which thread covered
this earlier), and this feels pretty suspect.

Why does cache maintenance to these regions result in SError in the
first place?

Is the SRAM not accepting some writeback or fetch that occurs as a
result? How are we protected against natural evictions and/or fetches?

Does the SRAM respond badly to a CMO itself for some reason?

Thanks,
Mark.

>  ArmPkg/Library/ArmLib/AArch64/{AArch64LibConstructor.c => 
> AArch64BaseLibConstructor.c} |  0
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf 
>   |  2 +-
>  ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf  
>   | 43 +++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c   
>   |  2 +
>  ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c 
>   | 75 
>  5 files changed, 121 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64BaseLibConstructor.c
> similarity index 100%
> rename from ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
> rename to ArmPkg/Library/ArmLib/AArch64/AArch64BaseLibConstructor.c
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> index 58684e8492f2..ef9d261b910d 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
> @@ -32,7 +32,7 @@ [Sources.AARCH64]
>  
>../Common/AArch64/ArmLibSupport.S
>../Common/ArmLib.c
> -  AArch64LibConstructor.c
> +  AArch64BaseLibConstructor.c
>  
>  [Packages]
>ArmPkg/ArmPkg.dec
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf
> new file mode 100644
> index ..c8f0b97750d4
> --- /dev/null
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64LibPei.inf
> @@ -0,0 +1,43 @@
> +#/** @file
> +#
> +# Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
> +# Portions copyright (c) 2011 - 2014, ARM 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.
> +#
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION= 0x00010005
> +  BASE_NAME  = AArch64Lib
> +  FILE_GUID  = ef20ddf5-b334-47b3-94cf-52ff44c29138
> +  MODULE_TYPE= PEIM
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = ArmLib|PEIM PEI_CORE
> +  CONSTRUCTOR= AArch64LibConstructor
> +
> +[Sources.AARCH64]
> +  AArch64Lib.c
> +  AArch64Mmu.c
> +  AArch64ArchTimer.c
> +  ArmLibSupportV8.S
> +  AArch64Support.S
> +  AArch64ArchTimerSupport.S
> +
> +  ../Common/AArch64/ArmLibSupport.S
> +  ../Common/ArmLib.c
> +  AArch64PeiLibConstructor.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  MemoryAllocationLib
> +  CacheMaintenanceLib
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> index cf9b7222b47b..07864bac28e6 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
> @@ -26,6 +26,8 @@
>  // We use this index definition to define an invalid block entry
>  #define TT_ATTR_INDX_INVALID((UINT32)~0)
>  
> +INT32 HaveMmuRoutines = 1;
> +
>  STATIC
>  UINT64
>  ArmMemoryAttributeToPageAttribute (
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c 
> b/ArmPkg/Library/ArmLib/AArch64/AArch64PeiLibConstructor.c
> new file mode 100

Re: [edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

2016-06-13 Thread Ard Biesheuvel

On 13 June 2016 at 17:45, Mark Rutland  wrote:
> On Mon, Jun 13, 2016 at 05:26:07PM +0200, Ard Biesheuvel wrote:
>> On some platforms, performing cache maintenance on regions that are backed
>> by NOR flash result in SErrors. Since cache maintenance is unnecessary in
>> that case, create a PEIM specific version that only performs said cache
>> maintenance in its constructor if the module is shadowed in RAM. To avoid
>> performing the cache maintenance if the MMU code is not used to begin with,
>> check that explicitly in the constructor.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>
>> As discussed in the thread dedicated to this subject, the preferred way of
>> addressing this to split off the MMU manipulation code from ArmLib into a
>> separate ArmMmuLib, but this affects other packages and platforms. So in the
>> mean time, let's merge this patch so that D02 can use the upstream ArmLib
>> unmodified.
>
> I'm missing something here (and couldn't figure out which thread covered
> this earlier), and this feels pretty suspect.
>
> Why does cache maintenance to these regions result in SError in the
> first place?
>
> Is the SRAM not accepting some writeback or fetch that occurs as a
> result? How are we protected against natural evictions and/or fetches?
>
> Does the SRAM respond badly to a CMO itself for some reason?
>

It's NOR flash, not SRAM, and it is basically a quirk of the D02
non-DRAM memory bus implementation. I will let Heyi respond with more
details, but this patch is basically a stop gap solution until we
split off the MMU code from ArmLib, which will allow us to deal with
this more elegantly (Currently, the only PEI phase module that
manipulates the page tables with the MMU on, and is likely to split
entries is DxeIpl, which maps the DXE phase stack non-executable. The
only other user is the module that creates the page tables in the
first place, and so does not require the extra caution when splitting
block entries)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] EDK2 Platforms Proposal

2016-06-13 Thread Leif Lindholm

Ye gods - I've slipped replying to this by a month now. Very sorry
about that. Well, I think I've managed to crawl out of the hole I fell
into.

So, I think the below sounds like it _could_ resolve most of my
concerns, with a few tweaks. Full disclosure: Personally, I would only
be actively interested in (3) and (4).

I personally don't have a huge interest in (2), and I'm already doing
(4) for OpenPlatformPkg. So I'm happy to use the
edk2-platforms/ approach for this. With the added
statement that (2) platform branches should always be based off a UDK
(or similar) version, and (4) platform branches should always track
edk2/master fairly closely. For this reason, it may be better to
separate them into different namespaces - maybe edk2-platforms/stable-*
and edk2-platforms/devel-*?
(4) Platforms that have not rebased to edk2/master in a few (3?)
months should be considered abandoned and purged from the tree, but
potentially archived.

For (1) and (3) I think two strict rules would help maintain code
quality:
a) (1) is always a subset of (3), but not necessarily a strict subset.
b) Patches can only go into (1) after they have gone into (3)
Does this sound reasonable?

If these two rules are strictly followed, I think the risk of
incompatible core changes would be minimised, and compatible changes
would be fairly easy for the maintainers to guide back towards
edk2/master.

Finally, and I have left this out of the discussion until now:
The problem with binary-only components.
Would it be acceptable to people to have binary-only components in
these edk2-platforms branches?
If not, would it be more acceptable to have one or more branches in a
separate repository, and hold platforms that are somewhat incomplete
in edk2-platforms - in order to get access to the parts of these
platform ports that _are_ open?

Regards,

Leif

On Tue, May 10, 2016 at 03:52:10PM +, Kinney, Michael D wrote:
> Leif,
> 
> You raise some very good concerns.  I do not think edk2-platforms is
> intended to solve all of these, and I agree that a branch per
> platform can make some things much worse, especially if there are
> many different core overrides in platform branches.
> 
> We need to consider different types of platform ports and figure out the 
> right 
> landing zone for each.
> 
> 1) Platforms required to validate a core feature in edk2/master
> 2) Platforms that require a validated/stable release of edk2
> 3) Platforms that are always synced with edk2/master, but do not
>improve edk2 core feature coverage.
> 4) Platforms that are under active initial porting efforts, have
>stability issues, or code quality issues.
> 
> edk2/master works for (1)
> edk2-platforms/ works for (2) and (4)
> As platforms in category (4) mature they migrate into (1), (2), or (3)
> 
> We do not have a landing zone for (3).  Maybe we can have a special
> branch in edk2-platforms that contains all the platforms in this
> category, so we can make sure things like common overrides and
> features common to many different platforms can be easily
> identified.  This special branch can be auto synced to edk2/master
> so incompatibilities introduced by platform changes or edk2/master
> changes can be quickly identified.
> 
> I am looking into a different proposal to add some tree organization
> to edk2/master.  Part of that includes a landing zone for vendor
> specific UEFI/PI device drivers that can be used by many different
> platforms.
> 
> Let's continue to refine the set of proposals here to address all of
> the important issues you have raised.
> 
> Best regards,
> 
> Mike
> 
> > -Original Message-
> > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > Sent: Tuesday, May 10, 2016 3:06 AM
> > To: Kinney, Michael D 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [RFC] EDK2 Platforms Proposal
> > 
> > Hi Mike,
> > 
> > Apologies for delay in responding, this was a topic I wanted to leave
> > some time to think about.
> > 
> > On Tue, May 03, 2016 at 04:30:36PM +, Kinney, Michael D wrote:
> > > Similar to edk2-staging, we also have a need to manage platforms
> > > that have been ported to edk2.  Jordan has created a repository
> > > called edk2-platforms and has created a branch for the
> > > minnowboard-max that uses a validated release of the UDK 2015 for
> > > the dependent packages:
> > >
> > > https://github.com/tianocore/edk2-platforms
> > >
> > > https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015
> > >
> > > Instead of creating a branch per feature in edk2-staging, the
> > > proposal is to create a branch per platform in edk2-platforms.  The
> > > maintainer(s) that create and support a platform branch can decide
> > > if the platform is synced to edk2/master for dependent packages, or
> > > uses a stable release of the edk2 for dependent packages.
> > >
> > > This proposal provides an area for platform development so we can
> > > minimize the number of platforms that are included in edk2/mast

Re: [edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

2016-06-13 Thread Mark Rutland

On Mon, Jun 13, 2016 at 05:51:23PM +0200, Ard Biesheuvel wrote:
> On 13 June 2016 at 17:45, Mark Rutland  wrote:
> > On Mon, Jun 13, 2016 at 05:26:07PM +0200, Ard Biesheuvel wrote:
> >> On some platforms, performing cache maintenance on regions that are backed
> >> by NOR flash result in SErrors. Since cache maintenance is unnecessary in
> >> that case, create a PEIM specific version that only performs said cache
> >> maintenance in its constructor if the module is shadowed in RAM. To avoid
> >> performing the cache maintenance if the MMU code is not used to begin with,
> >> check that explicitly in the constructor.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>
> >> As discussed in the thread dedicated to this subject, the preferred way of
> >> addressing this to split off the MMU manipulation code from ArmLib into a
> >> separate ArmMmuLib, but this affects other packages and platforms. So in 
> >> the
> >> mean time, let's merge this patch so that D02 can use the upstream ArmLib
> >> unmodified.
> >
> > I'm missing something here (and couldn't figure out which thread covered
> > this earlier), and this feels pretty suspect.
> >
> > Why does cache maintenance to these regions result in SError in the
> > first place?
> >
> > Is the SRAM not accepting some writeback or fetch that occurs as a
> > result? How are we protected against natural evictions and/or fetches?
> >
> > Does the SRAM respond badly to a CMO itself for some reason?
> >
> 
> It's NOR flash, not SRAM, and it is basically a quirk of the D02
> non-DRAM memory bus implementation.

Ok. I take it that means the CMO itself is the issue (i.e. it gets
forwarded to the memory controller endpoint, which barfs), rather than
asynchronous writebacks or fetches being a potential issue.

If Heyi can confirm that, then this looks fine to me. For the sake of
future reference, it would be nice to note the specific issue in the
commit message.

> I will let Heyi respond with more details, but this patch is basically
> a stop gap solution until we split off the MMU code from ArmLib, which
> will allow us to deal with this more elegantly (Currently, the only
> PEI phase module that manipulates the page tables with the MMU on, and
> is likely to split entries is DxeIpl, which maps the DXE phase stack
> non-executable. The only other user is the module that creates the
> page tables in the first place, and so does not require the extra
> caution when splitting block entries)

Understood!

My only concern was whether this was masking an issue with asynchronous
behaviour.

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

Re: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and CpuBreakpoint() as ANALYZER_NORETURN.

2016-06-13 Thread Marvin Häuser

Liming,

UNREACHABLE() in fact may have an impact as it is a signal that the code flow 
cannot reach that position. Thus a compiler or analyzer may classify everything 
following as unreachable and issue warnings about it or optimize it away. For 
stack cleanups, as Andrew suggested, this is a good thing, not-so for code 
following that instruction though - hence this patch was a bad idea.

I believe the defines are better off in Base.h, as other compiler-specific 
things (UEFI type defines, GLOBAL_REMOVE_IF_UNREFERENCED, ...) are there as 
well in contrast to BaseLib.h, which is basically an unrelated library. These 
macros may not only be used for ASSERT() but, as Andrew suggested, also in the 
handoff functions of all phase modules and these would all need to include 
BaseLib.h to get the necessary defines according to your suggestion.

In the end, it is up to you whether the defines will be in Base.h or BaseLib.h 
- or if regular and ANALYZER_ macros are one or separate patches -, but for 
both cases I do not understand the intention and consider one patch adding the 
defines to Base.h (as already submitted and still backed by myself), another 
patch adding the ANALYZER_UNREACHABLE() decoration to the end of the ASSERT 
if-branch (which will arrive once my concerns regarding the first patch have 
been addressed) and finally a third patch to add the decorations to the phase 
transition functions as pointed out by Andrew as the best solution.

Thanks for your time,
Marvin.

> -Original Message-
> From: Gao, Liming [mailto:liming@intel.com]
> Sent: Monday, June 13, 2016 4:33 AM
> To: Marvin Häuser ; edk2-
> de...@lists.01.org
> Cc: Kinney, Michael D 
> Subject: RE: [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and
> CpuBreakpoint() as ANALYZER_NORETURN.
> 
> Marvin:
>   This is a better. I suggest to create two patch sets. I think you focuses 
> on the
> first. For the second, UNREACHABLE is a decorator, and has no real impact.
> 1. For static code analyzer, add ANALYZER_UNREACHABLE macro in
> BaseLib.h, and use it in _ASSERT() macro in DebugLib.h 2. For un reachable
> case, add UNREACHABLE macro in BaseLib.h, and apply it in code pointed by
> Andrew.
> 
> Thanks
> Liming
> > -Original Message-
> > From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> > Sent: Monday, June 13, 2016 2:46 AM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D ; Gao, Liming
> > 
> > Subject: RE: [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and
> > CpuBreakpoint() as ANALYZER_NORETURN.
> >
> > Dear package maintainers and reviewers,
> >
> > Please do not approve this patch yet. I have thought further about the
> > consequences of decorating CpuDeadLoop() and CpuBreakpoint() with the
> > ANALYZER_NORETURN attribute and fear that the Analyzer, against the
> > original intention, could issue incorrect warnings when they are used
> > in the middle of code. All code following could probably be classified
> > as unreachable (as the Analyzer will rightfully assume the Cpu* functions
> won't return).
> >
> > I do not know if there is a way to limit this behavior to ASSERT.
> > Maybe this patch should be gotten rid of and UNREACHABLE() should be
> > added within the ASSERT() macro itself? This would make more sense
> > than what I have done in this patch, looking back.
> >
> > If you have thoughts, please be kind enough to share them.
> >
> > Regards,
> > Marvin.
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Marvin Häuser
> > > Sent: Friday, June 10, 2016 11:02 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: michael.d.kin...@intel.com; liming@intel.com
> > > Subject: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop()
> > > and
> > > CpuBreakpoint() as ANALYZER_NORETURN.
> > >
> > > Add the ANALYZER_NORETURN attribute to CpuDeadLoop() and
> > > CpuBreakpoint() to avoid false 'possible NULL-dereference' warnings
> > > when dereferencing pointers after having validated them with
> > > ASSERT(). As the ANALYZER-prefixed versions are being used, the code
> > > following the calls
> > will
> > > not be optimized away.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Marvin Haeuser 
> > > ---
> > >  MdePkg/Library/BaseLib/CpuDeadLoop.c   | 3 +++
> > >  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 3 +++
> > >  MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.c| 3 +++
> > >  MdePkg/Library/BaseLib/Ipf/CpuBreakpoint.c | 3 +++
> > >  MdePkg/Library/BaseLib/Ipf/CpuBreakpointMsc.c  | 3 +++
> > >  MdePkg/Library/BaseLib/X64/CpuBreakpoint.c | 3 +++
> > >  MdePkg/Library/BaseLib/X64/GccInline.c | 3 +++
> > >  MdePkg/Include/Library/BaseLib.h   | 2 ++
> > >  MdePkg/Library/BaseLib/AArch64/CpuBreakpoint.S | 1 +
> > >  MdePkg/Library/BaseLib/Arm/CpuBreakpoint.S | 1 +
> > >  MdePkg/Library/BaseLib/Arm/CpuBreakpoint.asm   | 1 +
> > >  MdePkg/Library/BaseLib

Re: [edk2] [PATCH 0/2] Add IntelSiliconPkg

2016-06-13 Thread Kinney, Michael D

Hi,

I agree with Giri's comments here.  The IntelSiliconPkg can be added using
current flat directory structure.

I will update the RFC for the proposed directory structure changes to include
target location for this new package.

Thanks,

Mike

> -Original Message-
> From: Mudusuru, Giri P
> Sent: Thursday, June 2, 2016 8:00 AM
> To: Yao, Jiewen ; af...@apple.com; Justen, Jordan L
> 
> Cc: Kinney, Michael D ; edk2-devel@lists.01.org; 
> Yao,
> Jiewen ; Mudusuru, Giri P 
> Subject: RE: [edk2] [PATCH 0/2] Add IntelSiliconPkg
> 
> As Jiewen mentioned the naming of the package is already closed with Mike. 
> The tree
> organization with package path, is generic and this package will be moved 
> under
> different folder along with others.
> 
> I would suggest to submit the patch and have the organization as separate 
> task and not
> hold changes to combine with restructuring packages.
> 
> Thanks,
> -Giri
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao, 
> Jiewen
> Sent: Wednesday, June 1, 2016 6:01 PM
> To: af...@apple.com; Justen, Jordan L 
> Cc: Kinney, Michael D ; edk2-devel@lists.01.org; 
> Yao,
> Jiewen 
> Subject: Re: [edk2] [PATCH 0/2] Add IntelSiliconPkg
> 
> Intel has internal discussion on the naming. (Mike is included) We decide to 
> choose
> IntelSiliconPkg because it is for Intel silicon.
> -- IntelPkg is confusing - is that Intel platform?
> -- IntelDevicePkg is confusing - is that for intel NIC?
> 
> Yes, I know Mike is working on restructuring package directory. This 
> IntelSiliconPkg
> can be put to Silicon/Intel/... in the future. There is no conflict.
> For package internal restructure (like MdeModulePkg ==> multiple small pkg), 
> I am not
> clear when it can be finalized. And I do not believe it is a good reason to 
> hold all
> new features check in.
> 
> Anyway, I am OK to wait 2 weeks till Mike is back.
> 
> Thank you
> Yao Jiewen
> 
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Thursday, June 2, 2016 4:59 AM
> To: Justen, Jordan L 
> Cc: Yao, Jiewen ; edk2-devel@lists.01.org; Kinney, 
> Michael D
> 
> Subject: Re: [edk2] [PATCH 0/2] Add IntelSiliconPkg
> 
> 
> > On Jun 1, 2016, at 1:39 PM, Jordan Justen
> mailto:jordan.l.jus...@intel.com>> wrote:
> >
> > Mike is looking to restructure the tree soon. Should we hold off on
> > adding this, or perhaps try to add it using the new structure?
> >
> > If not, then what about IntelPkg or IntelDevicePkg instead? Since
> > nothing in the tree is using this, what external platforms will depend
> > on it? Will they be able to move with Mike's restructuring?
> >
> 
> Mike's out this week and next week in case timing is important?
> 
> Thanks,
> 
> Andrew Fish
> 
> > -Jordan
> >
> > On 2016-05-31 20:02:10, Jiewen Yao wrote:
> >> This series patch adds the initial version of IntelSiliconPkg and an
> >> include file.
> >>
> >> We will use IntelSiliconPkg for open source common Intel silicon
> >> related modules.
> >>
> >>
> >> Jiewen Yao (2):
> >>  IntelSiliconPkg: Add initial version.
> >>  IntelSiliconPkg/IgdOpRegion: Add definition for Intel IGD OpRegion.
> >>
> >> IntelSiliconPkg/Contributions.txt  | 218 
> >> 
> >> IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h | 119 +++
> >> IntelSiliconPkg/IntelSiliconPkg.dec|  24 +++
> >> IntelSiliconPkg/License.txt|  25 +++
> >> 4 files changed, 386 insertions(+)
> >> create mode 100644 IntelSiliconPkg/Contributions.txt create mode
> >> 100644 IntelSiliconPkg/Include/IndustryStandard/IgdOpRegion.h
> >> create mode 100644 IntelSiliconPkg/IntelSiliconPkg.dec
> >> create mode 100644 IntelSiliconPkg/License.txt
> >>
> >> --
> >> 2.7.4.windows.1
> >>
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-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 V3 1/3] MdeModulePkg: Define and install performance property configuration table

2016-06-13 Thread Kinney, Michael D

Star,

The PERFORMANCE_PROPERTY structure has a field called CpuFreq.

Since there are many different types of timer sources, some not 
related to  the CPU frequencies, should this field name just be 
Frequency?

Also, one of the long standing issues with the DP command is that
all the modules that record performance records must use the same
instance of the TimerLib so this same frequency counter is used 
for all performance records.

If we are going to make the types of changes you are proposing
here, can we also evaluate if we can update the performance 
records to include the counter frequency information so the DP
command can support evaluation of log entries that are generated
using different TimerLib instances?

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star 
> Zeng
> Sent: Sunday, June 12, 2016 12:27 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Yao, Jiewen 
> ; Gao,
> Liming 
> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
> property
> configuration table
> 
> Define PERFORMANCE_PROPERTY, and install performance property configuration
> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
> 
> Cc: Liming Gao 
> Cc: Jiewen Yao 
> Cc: Cinnamon Shia 
> Cc: Jaben Carsey 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Include/Guid/Performance.h| 12 ++-
>  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
> +-
>  .../DxeCorePerformanceLib.inf  |  4 +++-
>  .../DxeCorePerformanceLibInternal.h|  3 ++-
>  .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 
>  .../SmmCorePerformanceLib.inf  |  5 -
>  6 files changed, 65 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Guid/Performance.h
> b/MdeModulePkg/Include/Guid/Performance.h
> index c40046c87811..fe6972d9bf6d 100644
> --- a/MdeModulePkg/Include/Guid/Performance.h
> +++ b/MdeModulePkg/Include/Guid/Performance.h
> @@ -4,7 +4,7 @@
>* performance protocol interfaces.
>* performance variables.
> 
> -Copyright (c) 2009 - 2013, 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 that accompanies this 
> distribution.
>  The full text of the license may be found at
> @@ -18,6 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR
> IMPLIED.
>  #ifndef __PERFORMANCE_DATA_H__
>  #define __PERFORMANCE_DATA_H__
> 
> +#define PERFORMANCE_PROPERTY_REVISION 0x1
> +
> +typedef struct {
> +  UINT32Revision;
> +  UINT32Reserved;
> +  UINT64CpuFreq;
> +  UINT64TimerStartValue;
> +  UINT64TimerEndValue;
> +} PERFORMANCE_PROPERTY;
> +
>  //
>  // PEI_PERFORMANCE_STRING_SIZE must be a multiple of 8.
>  //
> diff --git 
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 4739bb842661..de7f7cbff9b1 100644
> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> @@ -61,6 +61,8 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = {
>GetGaugeEx
>};
> 
> +PERFORMANCE_PROPERTY mPerformanceProperty;
> +
>  /**
>Searches in the gauge array with keyword Handle, Token, Module and 
> Identifier.
> 
> @@ -502,6 +504,10 @@ DxeCorePerformanceLibConstructor (
>)
>  {
>EFI_STATUSStatus;
> +  UINT64Freq;
> +  UINT64StartValue;
> +  UINT64EndValue;
> +  PERFORMANCE_PROPERTY  *PerformanceProperty;
> 
>if (!PerformanceMeasurementEnabled ()) {
>  //
> @@ -531,7 +537,23 @@ DxeCorePerformanceLibConstructor (
> 
>InternalGetPeiPerformance ();
> 
> -  return Status;
> +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
> &PerformanceProperty);
> +  if (EFI_ERROR (Status)) {
> +PerformanceProperty = &mPerformanceProperty;
> +//
> +// Install configuration table for performance property.
> +//
> +PerformanceProperty->Revision = PERFORMANCE_PROPERTY_REVISION;
> +PerformanceProperty->Reserved = 0;
> +Freq = GetPerformanceCounterProperties (&StartValue, &EndValue);
> +PerformanceProperty->CpuFreq = Freq;
> +PerformanceProperty->TimerStartValue = StartValue;
> +PerformanceProperty->TimerEndValue = EndValue;
> +Status = gBS->InstallConfigurationTable (&gPerformanceProtocolGuid,
> PerformanceProperty);
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  return EFI_SUCCESS;
>  }
> 
>  /**
> diff --git 
> a/MdeModulePkg/L

Re: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and CpuBreakpoint() as ANALYZER_NORETURN.

2016-06-13 Thread Kinney, Michael D

Marvin,

I agree that Base.h is the right place for these new macros.

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Marvin 
> Häuser
> Sent: Monday, June 13, 2016 11:05 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and
> CpuBreakpoint() as ANALYZER_NORETURN.
> 
> Liming,
> 
> UNREACHABLE() in fact may have an impact as it is a signal that the code flow 
> cannot
> reach that position. Thus a compiler or analyzer may classify everything 
> following as
> unreachable and issue warnings about it or optimize it away. For stack 
> cleanups, as
> Andrew suggested, this is a good thing, not-so for code following that 
> instruction
> though - hence this patch was a bad idea.
> 
> I believe the defines are better off in Base.h, as other compiler-specific 
> things (UEFI
> type defines, GLOBAL_REMOVE_IF_UNREFERENCED, ...) are there as well in 
> contrast to
> BaseLib.h, which is basically an unrelated library. These macros may not only 
> be used
> for ASSERT() but, as Andrew suggested, also in the handoff functions of all 
> phase
> modules and these would all need to include BaseLib.h to get the necessary 
> defines
> according to your suggestion.
> 
> In the end, it is up to you whether the defines will be in Base.h or 
> BaseLib.h - or if
> regular and ANALYZER_ macros are one or separate patches -, but for both 
> cases I do not
> understand the intention and consider one patch adding the defines to Base.h 
> (as
> already submitted and still backed by myself), another patch adding the
> ANALYZER_UNREACHABLE() decoration to the end of the ASSERT if-branch (which 
> will arrive
> once my concerns regarding the first patch have been addressed) and finally a 
> third
> patch to add the decorations to the phase transition functions as pointed out 
> by Andrew
> as the best solution.
> 
> Thanks for your time,
> Marvin.
> 
> > -Original Message-
> > From: Gao, Liming [mailto:liming@intel.com]
> > Sent: Monday, June 13, 2016 4:33 AM
> > To: Marvin Häuser ; edk2-
> > de...@lists.01.org
> > Cc: Kinney, Michael D 
> > Subject: RE: [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and
> > CpuBreakpoint() as ANALYZER_NORETURN.
> >
> > Marvin:
> >   This is a better. I suggest to create two patch sets. I think you focuses 
> > on the
> > first. For the second, UNREACHABLE is a decorator, and has no real impact.
> > 1. For static code analyzer, add ANALYZER_UNREACHABLE macro in
> > BaseLib.h, and use it in _ASSERT() macro in DebugLib.h 2. For un reachable
> > case, add UNREACHABLE macro in BaseLib.h, and apply it in code pointed by
> > Andrew.
> >
> > Thanks
> > Liming
> > > -Original Message-
> > > From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> > > Sent: Monday, June 13, 2016 2:46 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Kinney, Michael D ; Gao, Liming
> > > 
> > > Subject: RE: [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and
> > > CpuBreakpoint() as ANALYZER_NORETURN.
> > >
> > > Dear package maintainers and reviewers,
> > >
> > > Please do not approve this patch yet. I have thought further about the
> > > consequences of decorating CpuDeadLoop() and CpuBreakpoint() with the
> > > ANALYZER_NORETURN attribute and fear that the Analyzer, against the
> > > original intention, could issue incorrect warnings when they are used
> > > in the middle of code. All code following could probably be classified
> > > as unreachable (as the Analyzer will rightfully assume the Cpu* functions
> > won't return).
> > >
> > > I do not know if there is a way to limit this behavior to ASSERT.
> > > Maybe this patch should be gotten rid of and UNREACHABLE() should be
> > > added within the ASSERT() macro itself? This would make more sense
> > > than what I have done in this patch, looking back.
> > >
> > > If you have thoughts, please be kind enough to share them.
> > >
> > > Regards,
> > > Marvin.
> > >
> > > > -Original Message-
> > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > > Of Marvin Häuser
> > > > Sent: Friday, June 10, 2016 11:02 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: michael.d.kin...@intel.com; liming@intel.com
> > > > Subject: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop()
> > > > and
> > > > CpuBreakpoint() as ANALYZER_NORETURN.
> > > >
> > > > Add the ANALYZER_NORETURN attribute to CpuDeadLoop() and
> > > > CpuBreakpoint() to avoid false 'possible NULL-dereference' warnings
> > > > when dereferencing pointers after having validated them with
> > > > ASSERT(). As the ANALYZER-prefixed versions are being used, the code
> > > > following the calls
> > > will
> > > > not be optimized away.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Marvin Haeuser 
> > > > ---
> > > >  MdePkg/Library/BaseLib/CpuDeadLoop.c   | 3 +++
> > > >  MdePkg/

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

2016-06-13 Thread Andrew Fish


> On Jun 13, 2016, at 4:14 PM, Kinney, Michael D  
> wrote:
> 
> Star,
> 
> The PERFORMANCE_PROPERTY structure has a field called CpuFreq.
> 
> Since there are many different types of timer sources, some not 
> related to  the CPU frequencies, should this field name just be 
> Frequency?
> 
> Also, one of the long standing issues with the DP command is that
> all the modules that record performance records must use the same
> instance of the TimerLib so this same frequency counter is used 
> for all performance records.
> 
> If we are going to make the types of changes you are proposing
> here, can we also evaluate if we can update the performance 
> records to include the counter frequency information so the DP
> command can support evaluation of log entries that are generated
> using different TimerLib instances?
> 

Mike,

Maybe thee should just be multiple instances of the performance logs, one per 
time source vs tagging every entry? 

It can get really complicated as I seem to remember updating a timer lib to use 
TSC + timer XYZ. Depending on how the Freq is calculated you could get 
different frequencies for the TSC. For example a PCD value vs. calibration with 
APIC could make the time sources look not the same for the TSC. Thus maybe some 
kind of indication of the the TSC was used could be useful? 

Thanks,

Andrew Fish

> Thanks,
> 
> Mike
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star 
>> Zeng
>> Sent: Sunday, June 12, 2016 12:27 AM
>> To: edk2-devel@lists.01.org
>> Cc: Carsey, Jaben ; Yao, Jiewen 
>> ; Gao,
>> Liming 
>> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
>> property
>> configuration table
>> 
>> Define PERFORMANCE_PROPERTY, and install performance property configuration
>> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
>> 
>> Cc: Liming Gao 
>> Cc: Jiewen Yao 
>> Cc: Cinnamon Shia 
>> Cc: Jaben Carsey 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Star Zeng 
>> ---
>> MdeModulePkg/Include/Guid/Performance.h| 12 ++-
>> .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
>> +-
>> .../DxeCorePerformanceLib.inf  |  4 +++-
>> .../DxeCorePerformanceLibInternal.h|  3 ++-
>> .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 
>> .../SmmCorePerformanceLib.inf  |  5 -
>> 6 files changed, 65 insertions(+), 5 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Include/Guid/Performance.h
>> b/MdeModulePkg/Include/Guid/Performance.h
>> index c40046c87811..fe6972d9bf6d 100644
>> --- a/MdeModulePkg/Include/Guid/Performance.h
>> +++ b/MdeModulePkg/Include/Guid/Performance.h
>> @@ -4,7 +4,7 @@
>>   * performance protocol interfaces.
>>   * performance variables.
>> 
>> -Copyright (c) 2009 - 2013, 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 that accompanies this 
>> distribution.
>> The full text of the license may be found at
>> @@ -18,6 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR
>> IMPLIED.
>> #ifndef __PERFORMANCE_DATA_H__
>> #define __PERFORMANCE_DATA_H__
>> 
>> +#define PERFORMANCE_PROPERTY_REVISION 0x1
>> +
>> +typedef struct {
>> +  UINT32Revision;
>> +  UINT32Reserved;
>> +  UINT64CpuFreq;
>> +  UINT64TimerStartValue;
>> +  UINT64TimerEndValue;
>> +} PERFORMANCE_PROPERTY;
>> +
>> //
>> // PEI_PERFORMANCE_STRING_SIZE must be a multiple of 8.
>> //
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 4739bb842661..de7f7cbff9b1 100644
>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> @@ -61,6 +61,8 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = {
>>   GetGaugeEx
>>   };
>> 
>> +PERFORMANCE_PROPERTY mPerformanceProperty;
>> +
>> /**
>>   Searches in the gauge array with keyword Handle, Token, Module and 
>> Identifier.
>> 
>> @@ -502,6 +504,10 @@ DxeCorePerformanceLibConstructor (
>>   )
>> {
>>   EFI_STATUSStatus;
>> +  UINT64Freq;
>> +  UINT64StartValue;
>> +  UINT64EndValue;
>> +  PERFORMANCE_PROPERTY  *PerformanceProperty;
>> 
>>   if (!PerformanceMeasurementEnabled ()) {
>> //
>> @@ -531,7 +537,23 @@ DxeCorePerformanceLibConstructor (
>> 
>>   InternalGetPeiPerformance ();
>> 
>> -  return Status;
>> +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid,
>> &PerformanceProperty);
>> +  if (EFI_ER

Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: CryptoPkg/TlsLib: Remove NULL cipher

2016-06-13 Thread Wu, Jiaxin

Reviewed-By: Wu Jiaxin 

Best Regards!
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Thomas Palmer
> Sent: Tuesday, June 7, 2016 10:47 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin 
> Subject: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: CryptoPkg/TlsLib:
> Remove NULL cipher
> 
> The term "NULL" refers to NULL-MD5, NULL-SHA and NULL-SHA256 when
> used to set the SSL cipher list.  As both MD5 and SHA variants are explicitly
> listed in our code, I surmise enabling all three by setting the cipher list 
> to just
> NULL was not the intended behavior.
> This patch will remove NULL as an option for the cipher list and allow NULL-
> SHA256 instead.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  CryptoPkg/Library/TlsLib/TlsLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/CryptoPkg/Library/TlsLib/TlsLib.c
> b/CryptoPkg/Library/TlsLib/TlsLib.c
> index 9f56b7a..b76dd20 100644
> --- a/CryptoPkg/Library/TlsLib/TlsLib.c
> +++ b/CryptoPkg/Library/TlsLib/TlsLib.c
> @@ -53,7 +53,6 @@ typedef struct {
>  // OpenSSL-used Cipher Suite name.
>  //
>  STATIC CONST TLS_CIPHER_PAIR TlsCipherMappingTable[] = {
> -  { 0x, "NULL" }, /// TLS_NULL_WITH_NULL_NULL
>{ 0x0001, "NULL-MD5" }, /// TLS_RSA_WITH_NULL_MD5
>{ 0x0002, "NULL-SHA" }, /// TLS_RSA_WITH_NULL_SHA
>{ 0x0004, "RC4-MD5" },  /// TLS_RSA_WITH_RC4_128_MD5
> @@ -62,6 +61,7 @@ STATIC CONST TLS_CIPHER_PAIR
> TlsCipherMappingTable[] = {
>{ 0x000A, "DES-CBC3-SHA" }, /// TLS_RSA_WITH_3DES_EDE_CBC_SHA
>{ 0x002F, "AES128-SHA" },   /// TLS_RSA_WITH_AES_128_CBC_SHA
>{ 0x0035, "AES256-SHA" },   /// TLS_RSA_WITH_AES_256_CBC_SHA
> +  { 0x003B, "NULL-SHA256" },  /// TLS_RSA_WITH_NULL_SHA256
>{ 0x003C, "AES128-SHA256" },/// TLS_RSA_WITH_AES_128_CBC_SHA256
>{ 0x003D, "AES256-SHA256" } /// TLS_RSA_WITH_AES_256_CBC_SHA256
>  };
> --
> 1.9.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] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-13 Thread Tian, Feng

Thanks for your info, Yakovlev.

Did you see problem with XHCI? If yes, I can provide a patch and could you help 
verify it?

Thanks
Feng

-Original Message-
From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
Sent: Monday, June 13, 2016 5:29 PM
To: Tian, Feng 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

Hi Feng. 

Sorry, i was AFK for the past week.

We have encountered this with Platronix USB headset. This device was reporting 
*USB Interface* descriptor sizes 1 byte larger than what the spec says. Headset 
was simply plugged in when we booted our firmware and UsbCreateDesc failed to 
parse that and returned NULL which eventually crashed the firmware later (i 
will try and send a separate patch to fix that). We fixed UsbCreateDesc and 
device enumeration went fine, all descriptors it sends are correct except for 
this odd size field.

Evgeny

> On 13 Jun 2016, at 05:32, Tian, Feng  wrote:
> 
> Hi, Yakovlev
> 
> Any response on this? 
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Tian, Feng 
> Sent: Monday, June 6, 2016 9:27 AM
> To: Evgeny Yakovlev ; edk2-devel@lists.01.org
> Cc: Tian, Feng 
> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi, Evgenv
> 
> Thanks for your contribution, Evgenv
> 
> Could you let me know what error case you met? Is the Device Desc or Config 
> Desc larger than expected or other descs?
> 
> Why I ask this question is because it would impact Xhci driver's 
> implementation in which we store some desc contents at internal. So looks 
> like XHCI driver also needs to be updated.
> 
> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evgeny 
> Yakovlev
> Sent: Sunday, June 5, 2016 10:29 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> check
> 
> According to spec if the length of a descriptor is smaller than what the 
> specification defines, then the host shall ignore it.
> However if the size is greater than expected the host will ignore the extra 
> bytes and start looking for the next descriptor at the end of actual length 
> returned. Original check did not handle the latter case correctly and only 
> allowed descriptors with lengths exactly as defined in specification.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evgeny Yakovlev 
> ---
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> index 5b8b1aa..fba60da 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> @@ -199,8 +199,8 @@ UsbCreateDesc (
> }
>   }
> 
> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> -  (Head->Type != Type) || (Head->Len != DescLen)) {
> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> +  (Head->Type != Type) || (Head->Len < DescLen)) {
> DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> return NULL;
>   }
> --
> 2.7.4 (Apple Git-66)
> 
> ___
> 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 V3 1/3] MdeModulePkg: Define and install performance property configuration table

2016-06-13 Thread Yao, Jiewen

Thanks Mike.

Another possible way I am thinking is that: Can we just record *time* instead 
of *tick* in the perf record?
Then there will be no worry on which timer lib the module is using.

Thank you
Yao Jiewen

From: Kinney, Michael D
Sent: Tuesday, June 14, 2016 7:15 AM
To: Zeng, Star ; edk2-devel@lists.01.org; Kinney, Michael 
D 
Cc: Carsey, Jaben ; Yao, Jiewen ; 
Gao, Liming 
Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
property configuration table

Star,

The PERFORMANCE_PROPERTY structure has a field called CpuFreq.

Since there are many different types of timer sources, some not
related to  the CPU frequencies, should this field name just be
Frequency?

Also, one of the long standing issues with the DP command is that
all the modules that record performance records must use the same
instance of the TimerLib so this same frequency counter is used
for all performance records.

If we are going to make the types of changes you are proposing
here, can we also evaluate if we can update the performance
records to include the counter frequency information so the DP
command can support evaluation of log entries that are generated
using different TimerLib instances?

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star 
> Zeng
> Sent: Sunday, June 12, 2016 12:27 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben mailto:jaben.car...@intel.com>>; 
> Yao, Jiewen mailto:jiewen@intel.com>>; Gao,
> Liming mailto:liming@intel.com>>
> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
> property
> configuration table
>
> Define PERFORMANCE_PROPERTY, and install performance property configuration
> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
>
> Cc: Liming Gao mailto:liming@intel.com>>
> Cc: Jiewen Yao mailto:jiewen@intel.com>>
> Cc: Cinnamon Shia mailto:cinnamon.s...@hpe.com>>
> Cc: Jaben Carsey mailto:jaben.car...@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng mailto:star.z...@intel.com>>
> ---
>  MdeModulePkg/Include/Guid/Performance.h| 12 ++-
>  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
> +-
>  .../DxeCorePerformanceLib.inf  |  4 +++-
>  .../DxeCorePerformanceLibInternal.h|  3 ++-
>  .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 
>  .../SmmCorePerformanceLib.inf  |  5 -
>  6 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Guid/Performance.h
> b/MdeModulePkg/Include/Guid/Performance.h
> index c40046c87811..fe6972d9bf6d 100644
> --- a/MdeModulePkg/Include/Guid/Performance.h
> +++ b/MdeModulePkg/Include/Guid/Performance.h
> @@ -4,7 +4,7 @@
>* performance protocol interfaces.
>* performance variables.
>
> -Copyright (c) 2009 - 2013, 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 that accompanies this 
> distribution.
>  The full text of the license may be found at
> @@ -18,6 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR
> IMPLIED.
>  #ifndef __PERFORMANCE_DATA_H__
>  #define __PERFORMANCE_DATA_H__
>
> +#define PERFORMANCE_PROPERTY_REVISION 0x1
> +
> +typedef struct {
> +  UINT32Revision;
> +  UINT32Reserved;
> +  UINT64CpuFreq;
> +  UINT64TimerStartValue;
> +  UINT64TimerEndValue;
> +} PERFORMANCE_PROPERTY;
> +
>  //
>  // PEI_PERFORMANCE_STRING_SIZE must be a multiple of 8.
>  //
> diff --git 
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 4739bb842661..de7f7cbff9b1 100644
> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> @@ -61,6 +61,8 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = {
>GetGaugeEx
>};
>
> +PERFORMANCE_PROPERTY mPerformanceProperty;
> +
>  /**
>Searches in the gauge array with keyword Handle, Token, Module and 
> Identifier.
>
> @@ -502,6 +504,10 @@ DxeCorePerformanceLibConstructor (
>)
>  {
>EFI_STATUSStatus;
> +  UINT64Freq;
> +  UINT64StartValue;
> +  UINT64EndValue;
> +  PERFORMANCE_PROPERTY  *PerformanceProperty;
>
>if (!PerformanceMeasurementEnabled ()) {
>  //
> @@ -531,7 +537,23 @@ DxeCorePerformanceLibConstructor (
>
>InternalGetPeiPerformance ();
>
> -  return Status;
> +  Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid

Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: CryptoPkg/TlsLib: Handshake failure

2016-06-13 Thread Wu, Jiaxin

Reviewed-By: Wu Jiaxin 

I will commit the patch soon. Thanks for the contribution.

Best Regards!
Jiaxin

> -Original Message-
> From: Thomas Palmer [mailto:thomas.pal...@hpe.com]
> Sent: Wednesday, June 8, 2016 2:36 AM
> To: edk2-devel@lists.01.org
> Cc: samer.el-haj-mahm...@hpe.com; Wu, Jiaxin ;
> Thomas Palmer 
> Subject: [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: CryptoPkg/TlsLib:
> Handshake failure
> 
> TlsLib should inspect the return from the SSL_do_handshake and return
> EFI_PROTOCOL_ERROR on certain conditions that are not recoverable.
> 
> For example, if a client is configured with a certain set of ciphers that the 
> TLS
> server does not support, the server will send a fatal alert before the
> handshake finishes.  Our TLS protocol only expects an alert to come after the
> handshake, so we would have continued TLS operations.
> 
> Please note I am using types int and unsigned long to match the OpenSSL api.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  CryptoPkg/Library/TlsLib/TlsLib.c | 29 +++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/CryptoPkg/Library/TlsLib/TlsLib.c
> b/CryptoPkg/Library/TlsLib/TlsLib.c
> index b76dd20..8b441a5 100644
> --- a/CryptoPkg/Library/TlsLib/TlsLib.c
> +++ b/CryptoPkg/Library/TlsLib/TlsLib.c
> @@ -616,6 +616,8 @@ TlsDoHandshake (
>  {
>TLS_CONNECTION  *TlsConn;
>UINTN   PendingBufferSize;
> +  int ret;
> +  unsigned long   e;
> 
>TlsConn   = (TLS_CONNECTION *) Tls;
>PendingBufferSize = 0;
> @@ -638,18 +640,41 @@ TlsDoHandshake (
>  PendingBufferSize = (UINTN) BIO_ctrl_pending (TlsConn->OutBio);
>  if (PendingBufferSize == 0) {
>SSL_set_connect_state (TlsConn->Ssl);
> -  SSL_do_handshake (TlsConn->Ssl);
> +  ret = SSL_do_handshake (TlsConn->Ssl);
>PendingBufferSize = (UINTN) BIO_ctrl_pending (TlsConn->OutBio);
>  }
>} else {
>  PendingBufferSize = (UINTN) BIO_ctrl_pending (TlsConn->OutBio);
>  if (PendingBufferSize == 0) {
>BIO_write (TlsConn->InBio, BufferIn, (UINT32) BufferInSize);
> -  SSL_do_handshake (TlsConn->Ssl);
> +  ret = SSL_do_handshake (TlsConn->Ssl);
>PendingBufferSize = (UINTN) BIO_ctrl_pending (TlsConn->OutBio);
>  }
>}
> 
> +  if (ret < 1) {
> +ret = SSL_get_error (TlsConn->Ssl, ret);
> +if (ret == SSL_ERROR_SSL ||
> +ret == SSL_ERROR_SYSCALL ||
> +ret == SSL_ERROR_ZERO_RETURN) {
> +  DEBUG ((DEBUG_ERROR, "%a SSL_HANDSHAKE_ERROR State=0x%x
> SSL_ERROR_%a\n", __FUNCTION__, SSL_state (TlsConn->Ssl),
> +ret == SSL_ERROR_SSL ? "SSL":
> +ret == SSL_ERROR_SYSCALL ? "SYSCALL":
> +"ZERO_RETURN"
> +));
> +  DEBUG_CODE_BEGIN ();
> +  while (1) {
> +e = ERR_get_error ();
> +if (e == 0) {
> +  break;
> +}
> +DEBUG ((DEBUG_ERROR, "%a ERROR 0x%x=L%x:F%x:R%x\n",
> __FUNCTION__, e, ERR_GET_LIB (e), ERR_GET_FUNC (e), ERR_GET_REASON
> (e)));
> +  }
> +  DEBUG_CODE_END ();
> +  return EFI_PROTOCOL_ERROR;
> +}
> +  }
> +
>if (PendingBufferSize > *BufferOutSize) {
>  *BufferOutSize = PendingBufferSize;
>  return EFI_BUFFER_TOO_SMALL;
> --
> 1.9.1

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

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

2016-06-13 Thread Kinney, Michael D

Jiewen,

The reason the original design does not record time is to minimize the time 
required to add a performance record to reduce the overhead of doing 
performance measurements.

Reading a counter is much simpler and takes less time than converting to a time 
value that usually requires multiple/divide operations.  Delaying this time 
conversion to the DP command resolves this issue.

Mike

From: Yao, Jiewen
Sent: Monday, June 13, 2016 5:42 PM
To: Kinney, Michael D ; Zeng, Star 
; edk2-devel@lists.01.org
Cc: Carsey, Jaben ; Gao, Liming ; 
Yao, Jiewen 
Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
property configuration table

Thanks Mike.

Another possible way I am thinking is that: Can we just record *time* instead 
of *tick* in the perf record?
Then there will be no worry on which timer lib the module is using.

Thank you
Yao Jiewen

From: Kinney, Michael D
Sent: Tuesday, June 14, 2016 7:15 AM
To: Zeng, Star mailto:star.z...@intel.com>>; 
edk2-devel@lists.01.org; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Carsey, Jaben mailto:jaben.car...@intel.com>>; Yao, 
Jiewen mailto:jiewen@intel.com>>; Gao, Liming 
mailto:liming@intel.com>>
Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
property configuration table

Star,

The PERFORMANCE_PROPERTY structure has a field called CpuFreq.

Since there are many different types of timer sources, some not
related to  the CPU frequencies, should this field name just be
Frequency?

Also, one of the long standing issues with the DP command is that
all the modules that record performance records must use the same
instance of the TimerLib so this same frequency counter is used
for all performance records.

If we are going to make the types of changes you are proposing
here, can we also evaluate if we can update the performance
records to include the counter frequency information so the DP
command can support evaluation of log entries that are generated
using different TimerLib instances?

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star 
> Zeng
> Sent: Sunday, June 12, 2016 12:27 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben mailto:jaben.car...@intel.com>>; 
> Yao, Jiewen mailto:jiewen@intel.com>>; Gao,
> Liming mailto:liming@intel.com>>
> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
> property
> configuration table
>
> Define PERFORMANCE_PROPERTY, and install performance property configuration
> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
>
> Cc: Liming Gao mailto:liming@intel.com>>
> Cc: Jiewen Yao mailto:jiewen@intel.com>>
> Cc: Cinnamon Shia mailto:cinnamon.s...@hpe.com>>
> Cc: Jaben Carsey mailto:jaben.car...@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng mailto:star.z...@intel.com>>
> ---
>  MdeModulePkg/Include/Guid/Performance.h| 12 ++-
>  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
> +-
>  .../DxeCorePerformanceLib.inf  |  4 +++-
>  .../DxeCorePerformanceLibInternal.h|  3 ++-
>  .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 
>  .../SmmCorePerformanceLib.inf  |  5 -
>  6 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Guid/Performance.h
> b/MdeModulePkg/Include/Guid/Performance.h
> index c40046c87811..fe6972d9bf6d 100644
> --- a/MdeModulePkg/Include/Guid/Performance.h
> +++ b/MdeModulePkg/Include/Guid/Performance.h
> @@ -4,7 +4,7 @@
>* performance protocol interfaces.
>* performance variables.
>
> -Copyright (c) 2009 - 2013, 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 that accompanies this 
> distribution.
>  The full text of the license may be found at
> @@ -18,6 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR
> IMPLIED.
>  #ifndef __PERFORMANCE_DATA_H__
>  #define __PERFORMANCE_DATA_H__
>
> +#define PERFORMANCE_PROPERTY_REVISION 0x1
> +
> +typedef struct {
> +  UINT32Revision;
> +  UINT32Reserved;
> +  UINT64CpuFreq;
> +  UINT64TimerStartValue;
> +  UINT64TimerEndValue;
> +} PERFORMANCE_PROPERTY;
> +
>  //
>  // PEI_PERFORMANCE_STRING_SIZE must be a multiple of 8.
>  //
> diff --git 
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 4739bb842661..de7f7cbff9b1 100644
> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerfor

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

2016-06-13 Thread Kinney, Michael D

Andrew,

I like the idea of a record type for a time source.  A PerformanceLib 
constructor could add that record,
and performance log entries could be tagged with a time source record indicator.

I agree that any timer source that requires an algorithm to measure the 
frequency could get slightly
different frequency values which would increase the number of time source 
records for the same timer.

The other even more complicated issue is a performance record whose start value 
is set in one module
and stop value is set in a different module.  If those two modules use 
different TimerLib instances,
then it won't work.  Which is one of the reasons the single TimerLib instance 
solution is used.

Mike

> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Monday, June 13, 2016 4:34 PM
> To: Kinney, Michael D 
> Cc: Zeng, Star ; edk2-devel@lists.01.org; Carsey, Jaben
> ; Yao, Jiewen ; Gao, Liming
> 
> Subject: Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install 
> performance
> property configuration table
> 
> 
> > On Jun 13, 2016, at 4:14 PM, Kinney, Michael D  
> > wrote:
> >
> > Star,
> >
> > The PERFORMANCE_PROPERTY structure has a field called CpuFreq.
> >
> > Since there are many different types of timer sources, some not
> > related to  the CPU frequencies, should this field name just be
> > Frequency?
> >
> > Also, one of the long standing issues with the DP command is that
> > all the modules that record performance records must use the same
> > instance of the TimerLib so this same frequency counter is used
> > for all performance records.
> >
> > If we are going to make the types of changes you are proposing
> > here, can we also evaluate if we can update the performance
> > records to include the counter frequency information so the DP
> > command can support evaluation of log entries that are generated
> > using different TimerLib instances?
> >
> 
> Mike,
> 
> Maybe thee should just be multiple instances of the performance logs, one per 
> time
> source vs tagging every entry?
> 
> It can get really complicated as I seem to remember updating a timer lib to 
> use TSC +
> timer XYZ. Depending on how the Freq is calculated you could get different 
> frequencies
> for the TSC. For example a PCD value vs. calibration with APIC could make the 
> time
> sources look not the same for the TSC. Thus maybe some kind of indication of 
> the the
> TSC was used could be useful?
> 
> Thanks,
> 
> Andrew Fish
> 
> > Thanks,
> >
> > Mike
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> >> Star Zeng
> >> Sent: Sunday, June 12, 2016 12:27 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: Carsey, Jaben ; Yao, Jiewen 
> >> ; Gao,
> >> Liming 
> >> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install 
> >> performance property
> >> configuration table
> >>
> >> Define PERFORMANCE_PROPERTY, and install performance property configuration
> >> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
> >>
> >> Cc: Liming Gao 
> >> Cc: Jiewen Yao 
> >> Cc: Cinnamon Shia 
> >> Cc: Jaben Carsey 
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Star Zeng 
> >> ---
> >> MdeModulePkg/Include/Guid/Performance.h| 12 ++-
> >> .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
> >> +-
> >> .../DxeCorePerformanceLib.inf  |  4 +++-
> >> .../DxeCorePerformanceLibInternal.h|  3 ++-
> >> .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 
> >> 
> >> .../SmmCorePerformanceLib.inf  |  5 -
> >> 6 files changed, 65 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Include/Guid/Performance.h
> >> b/MdeModulePkg/Include/Guid/Performance.h
> >> index c40046c87811..fe6972d9bf6d 100644
> >> --- a/MdeModulePkg/Include/Guid/Performance.h
> >> +++ b/MdeModulePkg/Include/Guid/Performance.h
> >> @@ -4,7 +4,7 @@
> >>   * performance protocol interfaces.
> >>   * performance variables.
> >>
> >> -Copyright (c) 2009 - 2013, 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 that accompanies this 
> >> distribution.
> >> The full text of the license may be found at
> >> @@ -18,6 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
> >> EITHER EXPRESS
> OR
> >> IMPLIED.
> >> #ifndef __PERFORMANCE_DATA_H__
> >> #define __PERFORMANCE_DATA_H__
> >>
> >> +#define PERFORMANCE_PROPERTY_REVISION 0x1
> >> +
> >> +typedef struct {
> >> +  UINT32Revision;
> >> +  UINT32Reserved;
> >> +  UINT64CpuFreq;
> >> +  UINT64TimerStartValue;
> >> +  UINT64TimerEndValue;
> >> +} PERFORMANCE_PROPERTY

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

2016-06-13 Thread Zeng, Star

Mike,

It is make sense to change CpuFreq field name to Frequency.

We ever thought about to catch the TimerLib mismatch case, but the frequency 
returned from TimerLib may have very very small deviation, for example the 
AcpiTimerLib instances in PcAtChipsetPkg that are using ACPI timer to calibrate 
TSC frequency.
Although the very very small deviation does not impact the final duration 
calculation, but that makes PerformanceLib can't compare the frequencies.
Another, if we want PerformanceLib to record frequency for every entry, the 
interface of PerformanceLib and performance protocol also needs to be changed 
to accept frequency from caller.


Thanks,
Star
From: Kinney, Michael D
Sent: Tuesday, June 14, 2016 8:50 AM
To: Yao, Jiewen ; Zeng, Star ; 
edk2-devel@lists.01.org; Kinney, Michael D 
Cc: Carsey, Jaben ; Gao, Liming 
Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
property configuration table

Jiewen,

The reason the original design does not record time is to minimize the time 
required to add a performance record to reduce the overhead of doing 
performance measurements.

Reading a counter is much simpler and takes less time than converting to a time 
value that usually requires multiple/divide operations.  Delaying this time 
conversion to the DP command resolves this issue.

Mike

From: Yao, Jiewen
Sent: Monday, June 13, 2016 5:42 PM
To: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Zeng, Star 
mailto:star.z...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Carsey, Jaben mailto:jaben.car...@intel.com>>; Gao, 
Liming mailto:liming@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>
Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
property configuration table

Thanks Mike.

Another possible way I am thinking is that: Can we just record *time* instead 
of *tick* in the perf record?
Then there will be no worry on which timer lib the module is using.

Thank you
Yao Jiewen

From: Kinney, Michael D
Sent: Tuesday, June 14, 2016 7:15 AM
To: Zeng, Star mailto:star.z...@intel.com>>; 
edk2-devel@lists.01.org; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Carsey, Jaben mailto:jaben.car...@intel.com>>; Yao, 
Jiewen mailto:jiewen@intel.com>>; Gao, Liming 
mailto:liming@intel.com>>
Subject: RE: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
property configuration table

Star,

The PERFORMANCE_PROPERTY structure has a field called CpuFreq.

Since there are many different types of timer sources, some not
related to  the CPU frequencies, should this field name just be
Frequency?

Also, one of the long standing issues with the DP command is that
all the modules that record performance records must use the same
instance of the TimerLib so this same frequency counter is used
for all performance records.

If we are going to make the types of changes you are proposing
here, can we also evaluate if we can update the performance
records to include the counter frequency information so the DP
command can support evaluation of log entries that are generated
using different TimerLib instances?

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star 
> Zeng
> Sent: Sunday, June 12, 2016 12:27 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben mailto:jaben.car...@intel.com>>; 
> Yao, Jiewen mailto:jiewen@intel.com>>; Gao,
> Liming mailto:liming@intel.com>>
> Subject: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance 
> property
> configuration table
>
> Define PERFORMANCE_PROPERTY, and install performance property configuration
> table in DxeCorePerformanceLib and SmmCorePerformanceLib.
>
> Cc: Liming Gao mailto:liming@intel.com>>
> Cc: Jiewen Yao mailto:jiewen@intel.com>>
> Cc: Cinnamon Shia mailto:cinnamon.s...@hpe.com>>
> Cc: Jaben Carsey mailto:jaben.car...@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng mailto:star.z...@intel.com>>
> ---
>  MdeModulePkg/Include/Guid/Performance.h| 12 ++-
>  .../DxeCorePerformanceLib/DxeCorePerformanceLib.c  | 24 
> +-
>  .../DxeCorePerformanceLib.inf  |  4 +++-
>  .../DxeCorePerformanceLibInternal.h|  3 ++-
>  .../SmmCorePerformanceLib/SmmCorePerformanceLib.c  | 22 
>  .../SmmCorePerformanceLib.inf  |  5 -
>  6 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Guid/Performance.h
> b/MdeModulePkg/Include/Guid/Performance.h
> index c40046c87811..fe6972d9bf6d 100644
> --- a/MdeModulePkg/Include/Guid/Performance.h
> +++ b/MdeModulePkg/Include/Guid/Performance.h
> @@ -4,7 +4,7 @@
>* performance protocol interfaces.
>* performance variables.
>
> -Copyright (c) 2009 - 20

[edk2] [Patch] NetworkPkg/TcpDxe: Fix GCC build failure

2016-06-13 Thread Jiaxin Wu

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/TcpDxe/SockImpl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockImpl.c b/NetworkPkg/TcpDxe/SockImpl.c
index 35e0f6a..5addbd1 100644
--- a/NetworkPkg/TcpDxe/SockImpl.c
+++ b/NetworkPkg/TcpDxe/SockImpl.c
@@ -591,16 +591,14 @@ SockCancelToken (
   IN OUT LIST_ENTRY *SpecifiedTokenList
   )
 {
   EFI_STATUS Status;
   LIST_ENTRY *Entry;
-  LIST_ENTRY *Next;
   SOCK_TOKEN *SockToken;
 
   Status= EFI_SUCCESS;
   Entry = NULL;
-  Next  = NULL;
   SockToken = NULL;
 
   if (IsListEmpty (SpecifiedTokenList) && Token != NULL) {
 return EFI_NOT_FOUND;
   }
-- 
1.9.5.msysgit.1

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

Re: [edk2] [RFC] Add more flexible PCD value formats in DEC/DSC files

2016-06-13 Thread Zhu, Yonghong

Hi Mike,

Sorry for the delay response. I have some comment:
1. Is this proposal only for DEC and DSC file ? Do we need cover the Pcd value 
in the INF file and FDF file ?
2. From build spec, it require build tools support C format GUIDs as Void * PCD 
value, current this feature is not implemented. So whether this new proposal 
need to support the  C format GUIDs as Void * PCD value ?

Best Regards,
Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Kinney, 
Michael D
Sent: Thursday, May 19, 2016 3:22 AM
To: edk2-devel@lists.01.org; Kinney, Michael D 
Subject: [edk2] [RFC] Add more flexible PCD value formats in DEC/DSC files

Hello,

Today, the DEC/DSC specifications limit the PCD value formats to the following:

  BOOLEAN  TRUE or FALSE
  UINT88-bit decimal or hexadecimal value
  UINT16   16-bit decimal or hexadecimal value
  UINT32   32-bit decimal or hexadecimal value
  UINT64   64-bit decimal or hexadecimal value
  VOID*ASCII string(e.g. "Hello")
  VOID*Unicode string(e.g. L"Hello")
  VOID*Array of hexadecimal byte values(e.g. {0x01, 0x02})

I would like to propose more flexible value formats for PCDs.
In DEC and DSC files.  This would include the following:

* Character values using single quotes(e.g. 'A').
* Multi-Character values using single quotes(e.g. 'ABCD').
* Able to use TRUE/FALSE for UINT8/16/32/64 values
* Able to use TRUE/FALSE in VOID* byte array
* Able to use decimal values in VOID* byte array
* Able to use single quote character values in VOID* byte arrays
* Able to use ASCII string, Unicode String, Multi-Character
  strings for UINT8/16/32/64 values as long as they fit in the
  target type.

Some examples of this additional flexibility are:

  UINT8   TRUE
  UINT8   FALSE
  UINT8   0x12
  UINT8   12
  UINT8   'A'
  UINT16   TRUE
  UINT16  0x1234
  UINT16  1234
  UINT16  'AB'
  UINT16  "A"
  UINT32  0x12345678
  UINT32  12345678
  UINT32  "ABC"
  UINT32  L"A"
  UINT32  'ABCD'
  UINT64  0x1234567812345678
  UINT64  1234567812345678
  UINT64  "ABCDEFG"
  UINT64  L"ABC"
  UINT64  'ABCDEFGH'
  VOID*   {0x1, 0x2, 0x3}
  VOID*   {10, 11, 12, 13, 14}
  VOID*   {'X', 'Y', 'Z'}
  VOID*   {TRUE, FALSE, FALSE, TRUE}
  VOID*   {0x41, 0x42, 67, 68, 'E', 'F', TRUE, FALSE}

Here is a python function that can parse these formats and return an python 
integer value that does not have any size limits.  Based on the data type, the 
size can be checked to see the value return fits in the target type.  All 
strings and byte arrays are reversed for little-endian byte ordering.

  NOTE: Need to add explicit null-terminator for ASCII/Unicode
  strings.  Multi-character constants are not null-terminated.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 

def ParseValue (Value):
  if type(Value) == type(0):
return Value
  if type(Value) <> type(''):
raise ValueError
  Value = Value.strip()
  if Value.startswith('L"') and Value.endswith('"'):
# Unicode String
List = list(Value[2:-1])
List.reverse()
Value = 0
for Char in List:
  Value = (Value << 16) | ord(Char)
return Value
  if Value[0] == '"' and Value[-1] == '"':
# ASCII String
List = list(Value[1:-1])
List.reverse()
Value = 0
for Char in List:
  Value = (Value << 8) | ord(Char)
return Value
  if Value[0] == "'" and Value[-1] == "'":
# Character constant
List = list(Value[1:-1])
List.reverse()
Value = 0
for Char in List:
  Value = (Value << 8) | ord(Char)
return Value
  if Value[0] == '{' and Value[-1] == '}':
# Byte array
Value = Value[1:-1]
List = [Item.strip() for Item in Value.split(',')]
List.reverse()
Value = 0
for Item in List:
  ItemValue = ParseValue(Item)
  if ItemValue > 255:
raise ValueError
  Value = (Value << 8) | ItemValue
return Value
  if Value.lower().startswith('0x'):
return int(Value, 16)
  if Value[0].isdigit():
return int(Value, 10)
  if Value.lower() == 'true':
return 1
  if Value.lower() == 'false':
return 0
  raise ValueError

Best regards,

Mike

___
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] NetworkPkg/TcpDxe: Fix GCC build failure

2016-06-13 Thread Ye, Ting

Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Tuesday, June 14, 2016 10:44 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Zhang, Lubo 

Subject: [Patch] NetworkPkg/TcpDxe: Fix GCC build failure

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/TcpDxe/SockImpl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockImpl.c b/NetworkPkg/TcpDxe/SockImpl.c index 
35e0f6a..5addbd1 100644
--- a/NetworkPkg/TcpDxe/SockImpl.c
+++ b/NetworkPkg/TcpDxe/SockImpl.c
@@ -591,16 +591,14 @@ SockCancelToken (
   IN OUT LIST_ENTRY *SpecifiedTokenList
   )
 {
   EFI_STATUS Status;
   LIST_ENTRY *Entry;
-  LIST_ENTRY *Next;
   SOCK_TOKEN *SockToken;
 
   Status= EFI_SUCCESS;
   Entry = NULL;
-  Next  = NULL;
   SockToken = NULL;
 
   if (IsListEmpty (SpecifiedTokenList) && Token != NULL) {
 return EFI_NOT_FOUND;
   }
--
1.9.5.msysgit.1

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

Re: [edk2] [Patch] NetworkPkg/TcpDxe: Fix GCC build failure

2016-06-13 Thread Fu, Siyuan

Reviewed-by: Fu Siyuan 



> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, June 14, 2016 10:44 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Zhang,
> Lubo 
> Subject: [Patch] NetworkPkg/TcpDxe: Fix GCC build failure
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Zhang Lubo 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/TcpDxe/SockImpl.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/NetworkPkg/TcpDxe/SockImpl.c b/NetworkPkg/TcpDxe/SockImpl.c
> index 35e0f6a..5addbd1 100644
> --- a/NetworkPkg/TcpDxe/SockImpl.c
> +++ b/NetworkPkg/TcpDxe/SockImpl.c
> @@ -591,16 +591,14 @@ SockCancelToken (
>IN OUT LIST_ENTRY *SpecifiedTokenList
>)
>  {
>EFI_STATUS Status;
>LIST_ENTRY *Entry;
> -  LIST_ENTRY *Next;
>SOCK_TOKEN *SockToken;
> 
>Status= EFI_SUCCESS;
>Entry = NULL;
> -  Next  = NULL;
>SockToken = NULL;
> 
>if (IsListEmpty (SpecifiedTokenList) && Token != NULL) {
>  return EFI_NOT_FOUND;
>}
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

[edk2] [Patch] BaseTools: ignore the binary LIB file in gen_libs

Re: [edk2] [PATCH V2 0/3] Remove TimerLib dependency from DP

[edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

Re: [edk2] edk2 llvm branch

Re: [edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

Re: [edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

Re: [edk2] [RFC] EDK2 Platforms Proposal

Re: [edk2] [PATCH v2] ArmPkg/ArmLib: avoid cache maintenance in PEIMs when executing in place

Re: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and CpuBreakpoint() as ANALYZER_NORETURN.

Re: [edk2] [PATCH 0/2] Add IntelSiliconPkg

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

Re: [edk2] [PATCH v1 2/2] MdePkg/BaseLib: Flag CpuDeadLoop() and CpuBreakpoint() as ANALYZER_NORETURN.

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: CryptoPkg/TlsLib: Remove NULL cipher

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

Re: [edk2] [PATCH] [edk2-staging/HTTPS-TLS][PATCH]: CryptoPkg/TlsLib: Handshake failure

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

Re: [edk2] [PATCH V3 1/3] MdeModulePkg: Define and install performance property configuration table

[edk2] [Patch] NetworkPkg/TcpDxe: Fix GCC build failure

Re: [edk2] [RFC] Add more flexible PCD value formats in DEC/DSC files

Re: [edk2] [Patch] NetworkPkg/TcpDxe: Fix GCC build failure

Re: [edk2] [Patch] NetworkPkg/TcpDxe: Fix GCC build failure

25 matches

Site Navigation

Mail list logo

Footer information