[edk2] [PATCH] MdeModulePkg PcdDxe: TokenNumber should start from 1.

2015-06-09 Thread Star Zeng
Because 0 is reserved as invalid token number.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 Universal/PCD/Dxe/Service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Universal/PCD/Dxe/Service.c b/Universal/PCD/Dxe/Service.c
index 9b4701b..cf5f244 100644
--- a/Universal/PCD/Dxe/Service.c
+++ b/Universal/PCD/Dxe/Service.c
@@ -1864,7 +1864,7 @@ VariableLockDynamicHiiPcd (
   //
   // Go through PCD database to find out DynamicHii PCDs.
   //
-  for (TokenNumber = 0; TokenNumber < LocalTokenCount; TokenNumber++) {
+  for (TokenNumber = 1; TokenNumber <= LocalTokenCount; TokenNumber++) {
 if (IsPeiDb) {
   LocalTokenNumber = GetLocalTokenNumber (TRUE, TokenNumber);
 } else {
-- 
1.9.5.msysgit.0


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 2/2] SecurityPkg Variable: Support the new introduced PcdMaxAuthVariableSize.

2015-06-09 Thread Yao, Jiewen
Looks good. Thanks!

Reviewed-by: Yao, Jiewen 

-Original Message-
From: Star Zeng [mailto:star.z...@intel.com] 
Sent: Tuesday, June 09, 2015 6:03 PM
To: edk2-devel@lists.sourceforge.net
Subject: [edk2] [PATCH 2/2] SecurityPkg Variable: Support the new introduced 
PcdMaxAuthVariableSize.

1. If PcdMaxAuthVariableSize is set to 0, keep current behavior as is and 
PcdMaxVariableSize used.
2. If PcdMaxAuthVariableSize is set to non 0, it will work on authenticated 
variables.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 .../VariableAuthenticated/RuntimeDxe/AuthService.c | 11 ++--  
.../VariableAuthenticated/RuntimeDxe/AuthService.h |  9 +++-
 .../VariableAuthenticated/RuntimeDxe/Variable.c| 60 ++
 .../VariableAuthenticated/RuntimeDxe/Variable.h|  8 +++
 .../RuntimeDxe/VariableRuntimeDxe.inf  |  1 +
 .../VariableAuthenticated/RuntimeDxe/VariableSmm.c |  2 +-
 .../RuntimeDxe/VariableSmm.inf |  3 +-
 .../RuntimeDxe/VariableSmmRuntimeDxe.c |  6 ++-
 .../RuntimeDxe/VariableSmmRuntimeDxe.inf   |  1 +
 9 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c 
b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 36d4470..9599c8a 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -124,13 +124,18 @@ InCustomMode (
 /**
   Initializes for authenticated varibale service.
 
+  @param[in] MaxAuthVariableSizeReflect the overhead associated with the 
saving
+of a single EFI authenticated variable 
with the exception
+of the overhead associated with the length
+of the string name of the EFI variable.
+
   @retval EFI_SUCCESS   Function successfully executed.
   @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resources.
 
 **/
 EFI_STATUS
 AutenticatedVariableServiceInitialize (
-  VOID
+  IN UINTN  MaxAuthVariableSize
   )
 {
   EFI_STATUS  Status;
@@ -158,7 +163,7 @@ AutenticatedVariableServiceInitialize (
   //
   // Reserve runtime buffer for public key database. The size excludes 
variable header and name size.
   //
-  mMaxKeyDbSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - 
sizeof (AUTHVAR_KEYDB_NAME);
+  mMaxKeyDbSize = (UINT32) (MaxAuthVariableSize - sizeof 
+ (AUTHVAR_KEYDB_NAME));
   mMaxKeyNumber = mMaxKeyDbSize / EFI_CERT_TYPE_RSA2048_SIZE;
   mPubKeyStore  = AllocateRuntimePool (mMaxKeyDbSize);
   if (mPubKeyStore == NULL) {
@@ -168,7 +173,7 @@ AutenticatedVariableServiceInitialize (
   //
   // Reserve runtime buffer for certificate database. The size excludes 
variable header and name size.
   //
-  mMaxCertDbSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - 
sizeof (EFI_CERT_DB_NAME);
+  mMaxCertDbSize = (UINT32) (MaxAuthVariableSize - sizeof 
+ (EFI_CERT_DB_NAME));
   mCertDbStore   = AllocateRuntimePool (mMaxCertDbSize);
   if (mCertDbStore == NULL) {
 return EFI_OUT_OF_RESOURCES;
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h 
b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
index f28c825..56def50 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
@@ -139,13 +139,18 @@ UpdatePlatformMode (
 /**
   Initializes for authenticated varibale service.
 
+  @param[in] MaxAuthVariableSizeReflect the overhead associated with the 
saving
+of a single EFI authenticated variable 
with the exception
+of the overhead associated with the length
+of the string name of the EFI variable.
+
   @retval EFI_SUCCESS   Function successfully executed.
-  @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resource.
+  @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resources.
 
 **/
 EFI_STATUS
 AutenticatedVariableServiceInitialize (
-  VOID
+  IN UINTN  MaxAuthVariableSize
   );
 
 /**
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c 
b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 7ecc29b..15d0531 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -2153,7 +2153,7 @@ UpdateVariable (
   // Trying to update NV variable prior to the installation of 
EFI_VARIABLE_WRITE_ARCH_PROTOCOL
   //
   return EFI_NOT_AVAILABLE_YET;
-} else if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) {
+} else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
   //
   // Trying to update volatile authenticated variable prior to the 
installation of EF

Re: [edk2] [PATCH] MdePkg: Make the implementation of UefiFileHandleLib instance consistent with comments in header file.

2015-06-09 Thread Carsey, Jaben
Can you say return "EFI_INVALID_PARAMETER" instead of "error"

Reviewed-by: Jaben Carsey 

>-Original Message-
>From: Qiu Shumin [mailto:shumin@intel.com]
>Sent: Tuesday, June 09, 2015 8:04 PM
>To: edk2-devel@lists.sourceforge.net
>Subject: [edk2] [PATCH] MdePkg: Make the implementation of
>UefiFileHandleLib instance consistent with comments in header file.
>
>The comment for FileHandleIsDirectory in FileHandleLib.h states that "If
>DirHandle is NULL, then ASSERT()." But the instance of UefiFileHandleLib
>returns EFI_INVALID_PARAMETER. The patch makes the comments and the
>implementation consistent.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Qiu Shumin 
>---
> MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>index be66c57..724532a 100644
>--- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>+++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>@@ -385,7 +385,7 @@ FileHandleFlush (
> /**
>   function to determine if a given handle is a directory handle
>
>-  if DirHandle is NULL then return error
>+  if DirHandle is NULL, then ASSERT().
>
>   open the file information on the DirHandle and verify that the Attribute
>   includes EFI_FILE_DIRECTORY bit set.
>@@ -404,9 +404,7 @@ FileHandleIsDirectory (
> {
>   EFI_FILE_INFO *DirInfo;
>
>-  if (DirHandle == NULL) {
>-return (EFI_INVALID_PARAMETER);
>-  }
>+  ASSERT (DirHandle != NULL);
>
>   //
>   // get the file information for DirHandle
>--
>1.9.5.msysgit.1
>
>
>
>--
>___
>edk2-devel mailing list
>edk2-devel@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/edk2-devel

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [Patch 2/2] IntelFrameworkModulePkg/IsaSerialDxe: Use PcdSerialClockRate instead of hard code value

2015-06-09 Thread Tian, Feng
Good to me.

Reviewed-by: Feng Tian 

-Original Message-
From: Ni, Ruiyu 
Sent: Wednesday, June 10, 2015 11:22
To: edk2-devel@lists.sourceforge.net
Cc: Ni, Ruiyu; Tian, Feng
Subject: [Patch 2/2] IntelFrameworkModulePkg/IsaSerialDxe: Use 
PcdSerialClockRate instead of hard code value

So that the driver can work on a certain hardware when a platform module 
dynamically changes the PCD value.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf| 12 +++-
 IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c|  6 +++---
 IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h|  8 +---
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf 
b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
index 064d4a0..4abaac9 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
@@ -4,7 +4,7 @@
 # Produces the Serial I/O protocol for standard UARTS using ISA I/O. This 
driver  # supports the 8250, 16450, 16550 and 16550A UART types.
 #
-# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2015, Intel Corporation. All rights 
+reserved.
 #
 # This program and the accompanying materials  # are licensed and made 
available under the terms and conditions of the BSD License @@ -42,6 +42,7 @@
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   IntelFrameworkPkg/IntelFrameworkPkg.dec
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
@@ -69,10 +70,11 @@
   
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdIsaBusSerialUseHalfHandshake|FALSE 
  ## CONSUMES
 
 [Pcd]
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200  ## CONSUMES
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|8   ## CONSUMES
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|1 ## CONSUMES
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|1   ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|8 ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|1   ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|1 ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200 ## CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   IsaSerialDxeExtra.uni
diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c 
b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
index 15d2bab..57ee669 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
@@ -1,7 +1,7 @@
 /** @file
   Serial driver for standard UARTS on an ISA bus.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -1393,7 +1393,7 
@@ IsaSerialSetAttributes (
   // Compute divisor use to program the baud rate using a round determination
   //
   Divisor = (UINT32) DivU64x32Remainder (
-   SERIAL_PORT_INPUT_CLOCK,
+   PcdGet32 (PcdSerialClockRate),
((UINT32) BaudRate * 16),
&Remained
);
@@ -1410,7 +1410,7 @@ IsaSerialSetAttributes (
   //
   // Compute the actual baud rate that the serial port will be programmed for.
   //
-  BaudRate = SERIAL_PORT_INPUT_CLOCK / Divisor / 16;
+  BaudRate = PcdGet32 (PcdSerialClockRate) / Divisor / 16;
 
   //
   // Put serial port on Divisor Latch Mode diff --git 
a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h 
b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h
index c8d9cb1..9d50ca9 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h
@@ -1,7 +1,7 @@
 /** @file
   Include for Serial Driver
   
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -128,12 +128,6 
@@ typedef struct {
  
EFI_SERIAL_OUTPUT_BUFFER_EMPTY  | \
  EFI_SERIAL_INPUT_BUFFER_EMPTY)
 
-
-//
-// (2400/13)MHz input clock
-//
-#define SERIAL_PORT_INPUT_CLOCK 1843200
-
 //
 // 115200 baud with rounding errors
 //
--
1.

[edk2] [Patch] PcAtChipsetPkg/PcRtc: Fix a Y2K bug

2015-06-09 Thread Ruiyu Ni
The original driver cannot handle the case when system time runs from 
1999/12/31 23:59:59
to 2000/1/1 0:0:0.
A simple test to set system time to 1999/12/31 23:59:59 can expose this bug.
The patch limits the driver to only support year in 100 range and decide the 
century value based
on the supporting range: Century either equals to PcdMinimalYear / 100 or 
equals to PcdMinimalYear / 100 + 1.

The patch passed the Y2K test.
However with year range [1998, 2097], when system time is 2097/12/31 23:59:59,
the next second system time will become 1998/1/1 0:0:0. I think it's a 
acceptable limitation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
Cc: Liming Gao 
Cc: Michael D Kinney 
---
 PcAtChipsetPkg/PcAtChipsetPkg.dec  |  5 +-
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 55 +-
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h | 11 +
 3 files changed, 27 insertions(+), 44 deletions(-)

diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec 
b/PcAtChipsetPkg/PcAtChipsetPkg.dec
index fb6fb8b..6a1415c 100644
--- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
+++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
@@ -123,7 +123,8 @@
 
   ## This PCD specifies the maximal valid year in RTC.
   # @Prompt Maximal valid year in RTC.
-  gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2099|UINT16|0x000E
+  # @Expression gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < 
gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100
+  gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000E
   
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Defines the ACPI register set base address.
@@ -168,4 +169,4 @@
   gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPortBaseAddressMask 
|0xFFFE|UINT16|0x0018
 
 [UserExtensions.TianoCore."ExtraFiles"]
-  PcAtChipsetPkgExtra.uni
\ No newline at end of file
+  PcAtChipsetPkgExtra.uni
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c 
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 23f8d3b..9ec309c 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -100,7 +100,6 @@ PcRtcInit (
   RTC_REGISTER_A  RegisterA;
   RTC_REGISTER_B  RegisterB;
   RTC_REGISTER_D  RegisterD;
-  UINT8   Century;
   EFI_TIMETime;
   UINTN   DataSize;
   UINT32  TimerVar;
@@ -163,8 +162,6 @@ PcRtcInit (
   Time.Month  = RtcRead (RTC_ADDRESS_MONTH);
   Time.Year   = RtcRead (RTC_ADDRESS_YEAR);
 
-  Century = RtcRead (RTC_ADDRESS_CENTURY);
-  
   //
   // Set RTC configuration after get original time
   // The value of bit AIE should be reserved.
@@ -201,7 +198,7 @@ PcRtcInit (
   //
   // Validate time fields
   //
-  Status = ConvertRtcTimeToEfiTime (&Time, Century, RegisterB);
+  Status = ConvertRtcTimeToEfiTime (&Time, RegisterB);
   if (!EFI_ERROR (Status)) {
 Status = RtcTimeFieldsValid (&Time);
   }
@@ -218,7 +215,7 @@ PcRtcInit (
 Time.Hour   = RTC_INIT_HOUR;
 Time.Day= RTC_INIT_DAY;
 Time.Month  = RTC_INIT_MONTH;
-Time.Year   = RTC_INIT_YEAR;
+Time.Year   = PcdGet16 (PcdMinimalValidYear);
 Time.Nanosecond  = 0;
 Time.TimeZone = EFI_UNSPECIFIED_TIMEZONE;
 Time.Daylight = 0;
@@ -251,7 +248,7 @@ PcRtcInit (
   Time.Hour   = RTC_INIT_HOUR;
   Time.Day= RTC_INIT_DAY;
   Time.Month  = RTC_INIT_MONTH;
-  Time.Year   = RTC_INIT_YEAR;
+  Time.Year   = PcdGet16 (PcdMinimalValidYear);
   Time.Nanosecond  = 0;
   Time.TimeZone = Global->SavedTimeZone;
   Time.Daylight = Global->Daylight;;
@@ -272,8 +269,8 @@ PcRtcInit (
 }
 return EFI_DEVICE_ERROR;
   }
-  
-  ConvertEfiTimeToRtcTime (&Time, RegisterB, &Century);
+
+  ConvertEfiTimeToRtcTime (&Time, RegisterB);
 
   //
   // Set the Y/M/D info to variable as it has no corresponding hw registers.
@@ -343,7 +340,6 @@ PcRtcGetTime (
 {
   EFI_STATUS  Status;
   RTC_REGISTER_B  RegisterB;
-  UINT8   Century;
 
   //
   // Check parameters for null pointer
@@ -383,8 +379,6 @@ PcRtcGetTime (
   Time->Month   = RtcRead (RTC_ADDRESS_MONTH);
   Time->Year= RtcRead (RTC_ADDRESS_YEAR);
 
-  Century = RtcRead (RTC_ADDRESS_CENTURY);
-  
   //
   // Release RTC Lock.
   //
@@ -401,7 +395,7 @@ PcRtcGetTime (
   //
   // Make sure all field values are in correct range
   //
-  Status = ConvertRtcTimeToEfiTime (Time, Century, RegisterB);
+  Status = ConvertRtcTimeToEfiTime (Time, RegisterB);
   if (!EFI_ERROR (Status)) {
 Status = RtcTimeFieldsValid (Time);
   }
@@ -447,7 +441,6 @@ PcRtcSetTime (
   EFI_STATUS  Status;
   EFI_TIMERtcTime;
   RTC_REGISTER_B  RegisterB;
-  UINT8   Century;
   UINT32  TimerVar;
 
   if (Time == NULL) {
@@ -506,7 +499,7 @@ PcRtcSetTime (
   RegisterB.Bits.Set  = 1;
   RtcWrite (RTC_ADDRESS_REGISTER_B, RegisterB.Data);
 
-  ConvertEfiTimeToRtcTime (&RtcTime, RegisterB, &Century);
+  ConvertEfiTimeToRtcTime (&RtcTime, RegisterB);
 
   RtcWrite (RTC_A

Re: [edk2] [Patch 1/2] MdeModulePkg: Extend PcdSerialClockRate to dynamic type

2015-06-09 Thread Tian, Feng
Reviewed-by: Feng Tian 

-Original Message-
From: Ni, Ruiyu 
Sent: Wednesday, June 10, 2015 11:22
To: edk2-devel@lists.sourceforge.net
Cc: Ni, Ruiyu; Tian, Feng
Subject: [Patch 1/2] MdeModulePkg: Extend PcdSerialClockRate to dynamic type

So it could allow platform module to change the serial clock rate per different 
hardware.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 MdeModulePkg/MdeModulePkg.dec | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index 39793b8..15d351d 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -890,10 +890,6 @@
   # @Prompt Memory profile memory type.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileMemoryType|0x0|UINT64|0x30001042
 
-  ## UART clock frequency is for the baud rate configuration.
-  # @Prompt Serial Port Clock Rate.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200|UINT32|0x00010066
-  
   ## PCI Serial Device Info. It is an array of Device, Function, and Power 
Management
   #  information that describes the path that contains zero or more PCI to PCI 
briges 
   #  followed by a PCI serial device.  Each array entry is 4-bytes in length.  
The @@ -968,6 +964,10 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x400e
 
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
+  ## UART clock frequency is for the baud rate configuration.
+  # @Prompt Serial Port Clock Rate.
+  
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200|UINT32|0x000
+ 10066
+
   ## This PCD points to the front page formset GUID
   #  Compare the FormsetGuid or ClassGuid with this PCD value can detect 
whether in front page
   gEfiMdeModulePkgTokenSpaceGuid.PcdFrontPageFormSetGuid|{ 0xbc, 0x30, 0x0c, 
0x9e,0x06, 0x3f, 0xa6, 0x4b, 0x82, 0x88, 0x9, 0x17, 0x9b, 0x85, 0x5d, 0xbe 
}|VOID*|0x0001006e
--
1.9.5.msysgit.1


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [patch 0/3] *** Update OpenSSL support to 1.0.2a release ***

2015-06-09 Thread Ye, Ting
Looks good to me.

Reviewed-by: Ye Ting  

-Original Message-
From: Long, Qin 
Sent: Wednesday, June 10, 2015 10:21 AM
To: Ye, Ting; ard.biesheu...@linaro.org; edk2-devel@lists.sourceforge.net
Subject: [patch 0/3] *** Update OpenSSL support to 1.0.2a release ***

Update the EDKII crypto provider from openssl 0.9.8zf to 1.0.2a.
The OpenSSL Project announced that the support for version 0.9.8 will cease
on 31st December 2015. This patch updates the EDKII openssl support to the
latest 1.0.2 branch.
[V2]
Re-send the patch with few build issue fixes.

qlong (3):
  [CryptoPkg]  Remove the old patch file for openssl-0.9.8zf build, and
add the patch file for openssl-1.0.2a.
  [CryptoPkg] Update INF file, installation scripts and HOWTO for
openssl-1.0.2a support.
  [CryptoPkg] Updates some support header files and wrapper files to
support openssl-1.0.2a build, and correct some openssl API usages
when handling ASN.1 en/decoding.

 CryptoPkg/Include/OpenSslSupport.h |   8 +-
 .../Library/BaseCryptLib/Pk/CryptAuthenticode.c|   6 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c |  10 +-
 .../Library/BaseCryptLib/Pk/CryptPkcs7Verify.c |  13 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c|  12 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  |   8 +-
 .../Library/OpensslLib/EDKII_openssl-0.9.8zf.patch | 279 --
 .../Library/OpensslLib/EDKII_openssl-1.0.2a.patch  | 358 
 CryptoPkg/Library/OpensslLib/Install.cmd   | 146 ++---
 CryptoPkg/Library/OpensslLib/Install.sh| 146 ++---
 CryptoPkg/Library/OpensslLib/OpensslLib.inf| 620 ++---
 CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt   |  46 +-
 12 files changed, 1002 insertions(+), 650 deletions(-)
 delete mode 100644 CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch
 create mode 100644 CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2a.patch

-- 
1.9.5.msysgit.1


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [Patch 1/2] MdeModulePkg: Extend PcdSerialClockRate to dynamic type

2015-06-09 Thread Ruiyu Ni
So it could allow platform module to change the serial clock rate per different 
hardware.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 MdeModulePkg/MdeModulePkg.dec | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 39793b8..15d351d 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -890,10 +890,6 @@
   # @Prompt Memory profile memory type.
   
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileMemoryType|0x0|UINT64|0x30001042
 
-  ## UART clock frequency is for the baud rate configuration.
-  # @Prompt Serial Port Clock Rate.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200|UINT32|0x00010066
-  
   ## PCI Serial Device Info. It is an array of Device, Function, and Power 
Management
   #  information that describes the path that contains zero or more PCI to PCI 
briges 
   #  followed by a PCI serial device.  Each array entry is 4-bytes in length.  
The 
@@ -968,6 +964,10 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x400e
 
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
+  ## UART clock frequency is for the baud rate configuration.
+  # @Prompt Serial Port Clock Rate.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200|UINT32|0x00010066
+
   ## This PCD points to the front page formset GUID
   #  Compare the FormsetGuid or ClassGuid with this PCD value can detect 
whether in front page
   gEfiMdeModulePkgTokenSpaceGuid.PcdFrontPageFormSetGuid|{ 0xbc, 0x30, 0x0c, 
0x9e,0x06, 0x3f, 0xa6, 0x4b, 0x82, 0x88, 0x9, 0x17, 0x9b, 0x85, 0x5d, 0xbe 
}|VOID*|0x0001006e
-- 
1.9.5.msysgit.1


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [Patch 2/2] IntelFrameworkModulePkg/IsaSerialDxe: Use PcdSerialClockRate instead of hard code value

2015-06-09 Thread Ruiyu Ni
So that the driver can work on a certain hardware when a platform module
dynamically changes the PCD value.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Feng Tian 
---
 .../Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf| 12 +++-
 IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c|  6 +++---
 IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h|  8 +---
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf 
b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
index 064d4a0..4abaac9 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
@@ -4,7 +4,7 @@
 # Produces the Serial I/O protocol for standard UARTS using ISA I/O. This 
driver
 # supports the 8250, 16450, 16550 and 16550A UART types.
 #
-# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
 #
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
@@ -42,6 +42,7 @@
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   IntelFrameworkPkg/IntelFrameworkPkg.dec
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
 
@@ -69,10 +70,11 @@
   
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdIsaBusSerialUseHalfHandshake|FALSE 
  ## CONSUMES
 
 [Pcd]
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200  ## CONSUMES
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|8   ## CONSUMES
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|1 ## CONSUMES
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|1   ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits|8 ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity|1   ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits|1 ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1843200 ## CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   IsaSerialDxeExtra.uni
diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c 
b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
index 15d2bab..57ee669 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
@@ -1,7 +1,7 @@
 /** @file
   Serial driver for standard UARTS on an ISA bus.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -1393,7 +1393,7 @@ IsaSerialSetAttributes (
   // Compute divisor use to program the baud rate using a round determination
   //
   Divisor = (UINT32) DivU64x32Remainder (
-   SERIAL_PORT_INPUT_CLOCK,
+   PcdGet32 (PcdSerialClockRate),
((UINT32) BaudRate * 16),
&Remained
);
@@ -1410,7 +1410,7 @@ IsaSerialSetAttributes (
   //
   // Compute the actual baud rate that the serial port will be programmed for.
   //
-  BaudRate = SERIAL_PORT_INPUT_CLOCK / Divisor / 16;
+  BaudRate = PcdGet32 (PcdSerialClockRate) / Divisor / 16;
 
   //
   // Put serial port on Divisor Latch Mode
diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h 
b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h
index c8d9cb1..9d50ca9 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h
@@ -1,7 +1,7 @@
 /** @file
   Include for Serial Driver
   
-Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -128,12 +128,6 @@ typedef struct {
  
EFI_SERIAL_OUTPUT_BUFFER_EMPTY  | \
  EFI_SERIAL_INPUT_BUFFER_EMPTY)
 
-
-//
-// (2400/13)MHz input clock
-//
-#define SERIAL_PORT_INPUT_CLOCK 1843200
-
 //
 // 115200 baud with rounding errors
 //
-- 
1.9.5.msysgit.1


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [Patch 0/2] Let IsaSerialDxe use dynamic PCD PcdSerialClockRate

2015-06-09 Thread Ruiyu Ni
So that for certain hardware with different serial clock rate, IsaSerialDxe can 
still manage it.

Ruiyu Ni (2):
  MdeModulePkg: Extend PcdSerialClockRate to dynamic type
  IntelFrameworkModulePkg/IsaSerialDxe: Use PcdSerialClockRate instead
of hard code value

 .../Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf| 12 +++-
 IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c|  6 +++---
 IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.h|  8 +---
 MdeModulePkg/MdeModulePkg.dec|  8 
 4 files changed, 15 insertions(+), 19 deletions(-)

-- 
1.9.5.msysgit.1


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [PATCH] MdePkg: Make the implementation of UefiFileHandleLib instance consistent with comments in header file.

2015-06-09 Thread Qiu Shumin
The comment for FileHandleIsDirectory in FileHandleLib.h states that "If 
DirHandle is NULL, then ASSERT()." But the instance of UefiFileHandleLib 
returns EFI_INVALID_PARAMETER. The patch makes the comments and the 
implementation consistent.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c 
b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
index be66c57..724532a 100644
--- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
+++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
@@ -385,7 +385,7 @@ FileHandleFlush (
 /**
   function to determine if a given handle is a directory handle
 
-  if DirHandle is NULL then return error
+  if DirHandle is NULL, then ASSERT().
 
   open the file information on the DirHandle and verify that the Attribute
   includes EFI_FILE_DIRECTORY bit set.
@@ -404,9 +404,7 @@ FileHandleIsDirectory (
 {
   EFI_FILE_INFO *DirInfo;
 
-  if (DirHandle == NULL) {
-return (EFI_INVALID_PARAMETER);
-  }
+  ASSERT (DirHandle != NULL);
 
   //
   // get the file information for DirHandle
-- 
1.9.5.msysgit.1



--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] "cond" vs. ?

2015-06-09 Thread Tim Lewis
Folks -

It appears that the VFR spec is not clear about how the 2nd and 3rd parameters 
of the "cond" operator are handled.

cond(expr1, expr2, expr3)

Is it: if (expr1) then x = expr2 else expr3

Or

If (expr1) then x = expr3 else expr2

We have seen alternate implementations in the wild.

Thanks,

Tim

The VFR spec says:


conditionalExp ::=

"cond"

"("

vfrStatementExpression

"?"

vfrStatementExpression

":"

vfrStatementExpression

")"

BEHAVIORS AND RESTRICTIONS:

Generates EFI_IFR_CONDITIONAL op-codes.VFR Description in BNF 52

Example:

numeric varid = MyData.Data,

prompt = STRING_TOKEN(STR_PROMPT),

help = STRING_TOKEN(STR_HELP),

minimum = 0,

maximum = 255,

default value = cond(2 == 1 ? 5 : 10),
endnumeric;



--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [patch 2/3] [CryptoPkg] Update INF file, installation scripts and HOWTO for openssl-1.0.2a support.

2015-06-09 Thread qlong
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long, Qin 
Signed-off-by: qlong 
---
 CryptoPkg/Library/OpensslLib/Install.cmd | 146 ---
 CryptoPkg/Library/OpensslLib/Install.sh  | 146 ---
 CryptoPkg/Library/OpensslLib/OpensslLib.inf  | 620 +++
 CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt |  46 +-
 4 files changed, 608 insertions(+), 350 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/Install.cmd 
b/CryptoPkg/Library/OpensslLib/Install.cmd
index 8f1d016..a633f57 100755
--- a/CryptoPkg/Library/OpensslLib/Install.cmd
+++ b/CryptoPkg/Library/OpensslLib/Install.cmd
@@ -1,71 +1,77 @@
-cd openssl-0.9.8zf
-copy e_os2.h  ..\..\..\Include\openssl
-copy crypto\crypto.h  ..\..\..\Include\openssl
-copy crypto\tmdiff.h  ..\..\..\Include\openssl
-copy crypto\opensslv.h ..\..\..\Include\openssl
-copy crypto\opensslconf.h ..\..\..\Include\openssl
-copy crypto\ebcdic.h ..\..\..\Include\openssl
-copy crypto\symhacks.h ..\..\..\Include\openssl
-copy crypto\ossl_typ.h ..\..\..\Include\openssl
-copy crypto\md2\md2.h ..\..\..\Include\openssl
-copy crypto\md4\md4.h ..\..\..\Include\openssl
-copy crypto\md5\md5.h ..\..\..\Include\openssl
-copy crypto\sha\sha.h ..\..\..\Include\openssl
-copy crypto\hmac\hmac.h ..\..\..\Include\openssl
-copy crypto\ripemd\ripemd.h ..\..\..\Include\openssl
-copy crypto\des\des.h ..\..\..\Include\openssl
-copy crypto\des\des_old.h ..\..\..\Include\openssl
-copy crypto\rc2\rc2.h ..\..\..\Include\openssl
-copy crypto\rc4\rc4.h ..\..\..\Include\openssl
-copy crypto\idea\idea.h ..\..\..\Include\openssl
-copy crypto\bf\blowfish.h ..\..\..\Include\openssl
-copy crypto\cast\cast.h ..\..\..\Include\openssl
-copy crypto\aes\aes.h ..\..\..\Include\openssl
-copy crypto\bn\bn.h ..\..\..\Include\openssl
-copy crypto\rsa\rsa.h ..\..\..\Include\openssl
-copy crypto\dsa\dsa.h ..\..\..\Include\openssl
-copy crypto\dso\dso.h ..\..\..\Include\openssl
-copy crypto\dh\dh.h ..\..\..\Include\openssl
-copy crypto\ec\ec.h ..\..\..\Include\openssl
-copy crypto\ecdh\ecdh.h ..\..\..\Include\openssl
-copy crypto\ecdsa\ecdsa.h ..\..\..\Include\openssl
-copy crypto\buffer\buffer.h ..\..\..\Include\openssl
-copy crypto\bio\bio.h ..\..\..\Include\openssl
-copy crypto\stack\stack.h ..\..\..\Include\openssl
-copy crypto\stack\safestack.h ..\..\..\Include\openssl
-copy crypto\lhash\lhash.h ..\..\..\Include\openssl
-copy crypto\rand\rand.h ..\..\..\Include\openssl
-copy crypto\err\err.h ..\..\..\Include\openssl
-copy crypto\objects\objects.h ..\..\..\Include\openssl
-copy crypto\objects\obj_mac.h ..\..\..\Include\openssl
-copy crypto\evp\evp.h ..\..\..\Include\openssl
-copy crypto\asn1\asn1.h ..\..\..\Include\openssl
-copy crypto\asn1\asn1_mac.h ..\..\..\Include\openssl
-copy crypto\asn1\asn1t.h ..\..\..\Include\openssl
-copy crypto\pem\pem.h ..\..\..\Include\openssl
-copy crypto\pem\pem2.h ..\..\..\Include\openssl
-copy crypto\x509\x509.h ..\..\..\Include\openssl
-copy crypto\x509\x509_vfy.h ..\..\..\Include\openssl
-copy crypto\x509v3\x509v3.h ..\..\..\Include\openssl
-copy crypto\conf\conf.h ..\..\..\Include\openssl
-copy crypto\conf\conf_api.h ..\..\..\Include\openssl
-copy crypto\txt_db\txt_db.h ..\..\..\Include\openssl
-copy crypto\pkcs7\pkcs7.h ..\..\..\Include\openssl
-copy crypto\pkcs12\pkcs12.h ..\..\..\Include\openssl
-copy crypto\comp\comp.h ..\..\..\Include\openssl
-copy crypto\engine\engine.h ..\..\..\Include\openssl
-copy crypto\ocsp\ocsp.h ..\..\..\Include\openssl
-copy crypto\ui\ui.h ..\..\..\Include\openssl
-copy crypto\ui\ui_compat.h ..\..\..\Include\openssl
-copy crypto\krb5\krb5_asn.h ..\..\..\Include\openssl
-copy crypto\store\store.h ..\..\..\Include\openssl
-copy crypto\pqueue\pqueue.h ..\..\..\Include\openssl
-copy crypto\pqueue\pq_compat.h ..\..\..\Include\openssl
-copy ssl\ssl.h ..\..\..\Include\openssl
-copy ssl\ssl2.h ..\..\..\Include\openssl
-copy ssl\ssl3.h ..\..\..\Include\openssl
-copy ssl\ssl23.h ..\..\..\Include\openssl
-copy ssl\tls1.h ..\..\..\Include\openssl
-copy ssl\dtls1.h ..\..\..\Include\openssl
-copy ssl\kssl.h ..\..\..\Include\openssl
+cd openssl-1.0.2a
+copy e_os2.h..\..\..\Include\openssl
+copy crypto\crypto.h..\..\..\Include\openssl
+copy crypto\opensslv.h  ..\..\..\Include\openssl
+copy crypto\opensslconf.h   ..\..\..\Include\openssl
+copy crypto\ebcdic.h..\..\..\Include\openssl
+copy crypto\symhacks.h  ..\..\..\Include\openssl
+copy crypto\ossl_typ.h  ..\..\..\Include\openssl
+copy crypto\objects\objects.h   ..\..\..\Include\openssl
+copy crypto\objects\obj_mac.h   ..\..\..\Include\openssl
+copy crypto\md4\md4.h   ..\..\..\Include\openssl
+copy crypto\md5\md5.h   ..\..\..\Include\openssl
+copy crypto\sha\sha.h   ..\..\..\Include\openssl
+copy crypto\mdc2\mdc2.h ..\..\..\Include\openssl
+copy crypto\hmac\hmac.h ..\..\..\Include\openssl
+copy crypto\ripemd\ripemd.h ..\..\..\Include\openssl
+co

[edk2] [patch 3/3] [CryptoPkg] Updates some support header files and wrapper files to support openssl-1.0.2a build, and correct some openssl API usages when handling ASN.1 en/decoding.

2015-06-09 Thread qlong
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long, Qin 
Signed-off-by: qlong 
---
 CryptoPkg/Include/OpenSslSupport.h|  8 +++-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c |  6 --
 CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c| 10 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c  | 13 +++--
 CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c   | 12 
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c |  8 +---
 6 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/CryptoPkg/Include/OpenSslSupport.h 
b/CryptoPkg/Include/OpenSslSupport.h
index ed889e9..b5a8b58 100644
--- a/CryptoPkg/Include/OpenSslSupport.h
+++ b/CryptoPkg/Include/OpenSslSupport.h
@@ -1,7 +1,7 @@
 /** @file
   Root include file to support building OpenSSL Crypto Library.
 
-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -109,6 +109,11 @@ struct tm {
   char  *tm_zone;   /* timezone abbreviation */
 };
 
+struct timeval {
+  long tv_sec;  /* time value, in seconds */
+  long tv_usec; /* time value, in microseconds */
+} timeval;
+
 struct dirent {
   UINT32  d_fileno; /* file number of entry */
   UINT16  d_reclen; /* length of this record */
@@ -240,5 +245,6 @@ extern FILE  *stdout;
 #define assert(expression)
 #define localtime(timer)  NULL
 #define gmtime_r(timer,result)(result = NULL)
+#define atoi(nptr)AsciiStrDecimalToUintn(nptr)
 
 #endif
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
index 4ce2b06..9e93355 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
@@ -9,7 +9,7 @@
   AuthenticodeVerify() will get PE/COFF Authenticode and will do basic check 
for
   data structure.
 
-Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -72,6 +72,7 @@ AuthenticodeVerify (
 {
   BOOLEAN  Status;
   PKCS7*Pkcs7;
+  CONST UINT8  *Temp;
   CONST UINT8  *OrigAuthData;
   UINT8*SpcIndirectDataContent;
   UINT8Asn1Byte;
@@ -96,7 +97,8 @@ AuthenticodeVerify (
   //
   // Retrieve & Parse PKCS#7 Data (DER encoding) from Authenticode Signature
   //
-  Pkcs7 = d2i_PKCS7 (NULL, &AuthData, (int)DataSize);
+  Temp  = AuthData;
+  Pkcs7 = d2i_PKCS7 (NULL, &Temp, (int)DataSize);
   if (Pkcs7 == NULL) {
 goto _Exit;
   }
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c
index 63fe78f..704eb4e 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c
@@ -1,7 +1,7 @@
 /** @file
   PKCS#7 SignedData Sign Wrapper Implementation over OpenSSL.
 
-Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -116,9 +116,9 @@ Pkcs7Sign (
   if (Key == NULL) {
 goto _Exit;
   }
-  Key->save_type = EVP_PKEY_RSA;
-  Key->type  = EVP_PKEY_type (EVP_PKEY_RSA);
-  Key->pkey.rsa  = (RSA *) RsaContext;
+  if (EVP_PKEY_assign_RSA (Key, (RSA *) RsaContext) == 0) {
+goto _Exit;
+  }
 
   //
   // Convert the data to be signed to BIO format. 
@@ -175,7 +175,7 @@ Pkcs7Sign (
   }
 
   CopyMem (*SignedData, P7Data + 19, *SignedDataSize);
-  
+
   OPENSSL_free (P7Data);
 
   Status = TRUE;
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
index a9665d5..863e4b7 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
@@ -10,7 +10,7 @@
   WrapPkcs7Data(), Pkcs7GetSigners(), Pkcs7Verify() will get UEFI Authenticated
   Variable and will do basic check for data structure.
 
-Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompani

[edk2] [patch 1/3] [CryptoPkg] Remove the old patch file for openssl-0.9.8zf build, and add the patch file for openssl-1.0.2a.

2015-06-09 Thread qlong
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long, Qin 
Signed-off-by: qlong 
---
 .../Library/OpensslLib/EDKII_openssl-0.9.8zf.patch | 279 
 .../Library/OpensslLib/EDKII_openssl-1.0.2a.patch  | 358 +
 2 files changed, 358 insertions(+), 279 deletions(-)
 delete mode 100644 CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch
 create mode 100644 CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2a.patch

diff --git a/CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch 
b/CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch
deleted file mode 100644
index 4abe62c..000
--- a/CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch
+++ /dev/null
@@ -1,279 +0,0 @@
-Index: crypto/bio/bss_file.c
-===
 crypto/bio/bss_file.c  (revision 1)
-+++ crypto/bio/bss_file.c  (working copy)
-@@ -418,6 +418,23 @@
- return (ret);
- }
- 
-+#else
-+
-+BIO_METHOD *BIO_s_file(void)
-+{
-+return NULL;
-+}
-+
-+BIO *BIO_new_file(const char *filename, const char *mode)
-+{
-+return NULL;
-+}
-+
-+BIO *BIO_new_fp(FILE *stream, int close_flag)
-+{
-+return NULL;
-+}
-+
- # endif /* OPENSSL_NO_STDIO */
- 
- #endif  /* HEADER_BSS_FILE_C */
-Index: crypto/crypto.h
-===
 crypto/crypto.h(revision 1)
-+++ crypto/crypto.h(working copy)
-@@ -239,15 +239,15 @@
- # ifndef OPENSSL_NO_LOCKING
- #  ifndef CRYPTO_w_lock
- #   define CRYPTO_w_lock(type) \
--CRYPTO_lock(CRYPTO_LOCK|CRYPTO_WRITE,type,__FILE__,__LINE__)
-+CRYPTO_lock(CRYPTO_LOCK|CRYPTO_WRITE,type,NULL,0)
- #   define CRYPTO_w_unlock(type)   \
--CRYPTO_lock(CRYPTO_UNLOCK|CRYPTO_WRITE,type,__FILE__,__LINE__)
-+CRYPTO_lock(CRYPTO_UNLOCK|CRYPTO_WRITE,type,NULL,0)
- #   define CRYPTO_r_lock(type) \
--CRYPTO_lock(CRYPTO_LOCK|CRYPTO_READ,type,__FILE__,__LINE__)
-+CRYPTO_lock(CRYPTO_LOCK|CRYPTO_READ,type,NULL,0)
- #   define CRYPTO_r_unlock(type)   \
--CRYPTO_lock(CRYPTO_UNLOCK|CRYPTO_READ,type,__FILE__,__LINE__)
-+CRYPTO_lock(CRYPTO_UNLOCK|CRYPTO_READ,type,NULL,0)
- #   define CRYPTO_add(addr,amount,type)\
--CRYPTO_add_lock(addr,amount,type,__FILE__,__LINE__)
-+CRYPTO_add_lock(addr,amount,type,NULL,0)
- #  endif
- # else
- #  define CRYPTO_w_lock(a)
-@@ -374,19 +374,19 @@
- # define MemCheck_off()  CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE)
- # define is_MemCheck_on() CRYPTO_is_mem_check_on()
- 
--# define OPENSSL_malloc(num) CRYPTO_malloc((int)num,__FILE__,__LINE__)
--# define OPENSSL_strdup(str) CRYPTO_strdup((str),__FILE__,__LINE__)
-+# define OPENSSL_malloc(num) CRYPTO_malloc((int)num,NULL,0)
-+# define OPENSSL_strdup(str) CRYPTO_strdup((str),NULL,0)
- # define OPENSSL_realloc(addr,num) \
--CRYPTO_realloc((char *)addr,(int)num,__FILE__,__LINE__)
-+CRYPTO_realloc((char *)addr,(int)num,NULL,0)
- # define OPENSSL_realloc_clean(addr,old_num,num) \
--CRYPTO_realloc_clean(addr,old_num,num,__FILE__,__LINE__)
-+CRYPTO_realloc_clean(addr,old_num,num,NULL,0)
- # define OPENSSL_remalloc(addr,num) \
--CRYPTO_remalloc((char **)addr,(int)num,__FILE__,__LINE__)
-+CRYPTO_remalloc((char **)addr,(int)num,NULL,0)
- # define OPENSSL_freeFuncCRYPTO_free
- # define OPENSSL_free(addr)  CRYPTO_free(addr)
- 
- # define OPENSSL_malloc_locked(num) \
--CRYPTO_malloc_locked((int)num,__FILE__,__LINE__)
-+CRYPTO_malloc_locked((int)num,NULL,0)
- # define OPENSSL_free_locked(addr) CRYPTO_free_locked(addr)
- 
- const char *SSLeay_version(int type);
-@@ -531,7 +531,7 @@
- long CRYPTO_get_mem_debug_options(void);
- 
- # define CRYPTO_push_info(info) \
--CRYPTO_push_info_(info, __FILE__, __LINE__);
-+CRYPTO_push_info_(info, NULL, 0);
- int CRYPTO_push_info_(const char *info, const char *file, int line);
- int CRYPTO_pop_info(void);
- int CRYPTO_remove_all_info(void);
-@@ -578,7 +578,7 @@
- 
- /* die if we have to */
- void OpenSSLDie(const char *file, int line, const char *assertion);
--# define OPENSSL_assert(e)   (void)((e) ? 0 : (OpenSSLDie(__FILE__, 
__LINE__, #e),1))
-+# define OPENSSL_assert(e)   (void)((e) ? 0 : (OpenSSLDie(NULL, 0, #e),1))
- 
- unsigned long *OPENSSL_ia32cap_loc(void);
- # define OPENSSL_ia32cap (*(OPENSSL_ia32cap_loc()))
-@@ -585,10 +585,10 @@
- int OPENSSL_isservice(void);
- 
- # ifdef OPENSSL_FIPS
--#  define FIPS_ERROR_IGNORED(alg) OpenSSLDie(__FILE__, __LINE__, \
-+#  define FIPS_ERROR_IGNORED(alg) OpenSSLDie(NULL, 0, \
- alg " previous FIPS forbidden algorithm error ignored");
- 
--#  define FIPS_BAD_ABORT(alg) OpenSSLDie(__FILE__, __LINE__, \
-+#  define FIPS_BAD_ABORT(alg) OpenSSLDie(NULL, 0, \
- #alg " Algorithm forbidden in FIPS mode");
- 
- #  ifdef OPENSSL_FIPS_STRICT

[edk2] [patch 0/3] *** Update OpenSSL support to 1.0.2a release ***

2015-06-09 Thread qlong
Update the EDKII crypto provider from openssl 0.9.8zf to 1.0.2a.
The OpenSSL Project announced that the support for version 0.9.8 will cease
on 31st December 2015. This patch updates the EDKII openssl support to the
latest 1.0.2 branch.
[V2]
Re-send the patch with few build issue fixes.

qlong (3):
  [CryptoPkg]  Remove the old patch file for openssl-0.9.8zf build, and
add the patch file for openssl-1.0.2a.
  [CryptoPkg] Update INF file, installation scripts and HOWTO for
openssl-1.0.2a support.
  [CryptoPkg] Updates some support header files and wrapper files to
support openssl-1.0.2a build, and correct some openssl API usages
when handling ASN.1 en/decoding.

 CryptoPkg/Include/OpenSslSupport.h |   8 +-
 .../Library/BaseCryptLib/Pk/CryptAuthenticode.c|   6 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Sign.c |  10 +-
 .../Library/BaseCryptLib/Pk/CryptPkcs7Verify.c |  13 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c|  12 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c  |   8 +-
 .../Library/OpensslLib/EDKII_openssl-0.9.8zf.patch | 279 --
 .../Library/OpensslLib/EDKII_openssl-1.0.2a.patch  | 358 
 CryptoPkg/Library/OpensslLib/Install.cmd   | 146 ++---
 CryptoPkg/Library/OpensslLib/Install.sh| 146 ++---
 CryptoPkg/Library/OpensslLib/OpensslLib.inf| 620 ++---
 CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt   |  46 +-
 12 files changed, 1002 insertions(+), 650 deletions(-)
 delete mode 100644 CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch
 create mode 100644 CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2a.patch

-- 
1.9.5.msysgit.1


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [Patch] BaseTools: Support build options for specific module type in DSC.

2015-06-09 Thread Yingke Liu
This patch extended BuildOptions section in DSC to support 
[BuildOptions.$(arch).CodeBase.$(MODULE_TYPE)]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yingke Liu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 11 ---
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 14 ++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index d1ed0a6..a2a8e7e 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2074,8 +2074,11 @@ class PlatformAutoGen(AutoGen):
 # Get the different options for the different style module
 if Module.AutoGenVersion < 0x00010005:
 PlatformOptions = self.EdkBuildOption
+ModuleTypeOptions = 
self.Platform.GetBuildOptionsByModuleType(EDK_NAME, Module.ModuleType)
 else:
 PlatformOptions = self.EdkIIBuildOption
+ModuleTypeOptions = 
self.Platform.GetBuildOptionsByModuleType(EDKII_NAME, Module.ModuleType)
+ModuleTypeOptions = self._ExpandBuildOption(ModuleTypeOptions)
 ModuleOptions = self._ExpandBuildOption(Module.BuildOptions)
 if Module in self.Platform.Modules:
 PlatformModule = self.Platform.Modules[str(Module)]
@@ -2084,19 +2087,21 @@ class PlatformAutoGen(AutoGen):
 PlatformModuleOptions = {}
 
 BuildRuleOrder = None
-for Options in [self.ToolDefinition, ModuleOptions, PlatformOptions, 
PlatformModuleOptions]:
+for Options in [self.ToolDefinition, ModuleOptions, PlatformOptions, 
ModuleTypeOptions, PlatformModuleOptions]:
 for Tool in Options:
 for Attr in Options[Tool]:
 if Attr == TAB_TOD_DEFINES_BUILDRULEORDER:
 BuildRuleOrder = Options[Tool][Attr]
 
-AllTools = set(ModuleOptions.keys() + PlatformOptions.keys() + 
PlatformModuleOptions.keys() + self.ToolDefinition.keys())
+AllTools = set(ModuleOptions.keys() + PlatformOptions.keys() +
+   PlatformModuleOptions.keys() + ModuleTypeOptions.keys() 
+
+   self.ToolDefinition.keys())
 BuildOptions = {}
 for Tool in AllTools:
 if Tool not in BuildOptions:
 BuildOptions[Tool] = {}
 
-for Options in [self.ToolDefinition, ModuleOptions, 
PlatformOptions, PlatformModuleOptions]:
+for Options in [self.ToolDefinition, ModuleOptions, 
PlatformOptions, ModuleTypeOptions, PlatformModuleOptions]:
 if Tool not in Options:
 continue
 for Attr in Options[Tool]:
diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
index 14cd22d..1371bb0 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
@@ -147,6 +147,7 @@ class DscBuildData(PlatformBuildClassObject):
 self._Pcds  = None
 self._DecPcds   = None
 self._BuildOptions  = None
+self._ModuleTypeOptions = None
 self._LoadFixAddress= None
 self._RFCLanguages  = None
 self._ISOLanguages  = None
@@ -767,6 +768,19 @@ class DscBuildData(PlatformBuildClassObject):
 self._BuildOptions[ToolChainFamily, ToolChain, EDK_NAME] = 
Option
 return self._BuildOptions
 
+def GetBuildOptionsByModuleType(self, Edk, ModuleType):
+if self._ModuleTypeOptions == None:
+self._ModuleTypeOptions = sdict()
+if (Edk, ModuleType) not in self._ModuleTypeOptions:
+options = sdict()
+self._ModuleTypeOptions[Edk, ModuleType] = options
+DriverType = '%s.%s' % (Edk, ModuleType)
+RecordList = self._RawData[MODEL_META_DATA_BUILD_OPTION, 
self._Arch, DriverType]
+for ToolChainFamily, ToolChain, Option, Arch, Type, Dummy3, Dummy4 
in RecordList:
+if Arch == self._Arch and Type == DriverType:
+options[ToolChainFamily, ToolChain, Edk] = Option
+return self._ModuleTypeOptions[Edk, ModuleType]
+
 ## Retrieve non-dynamic PCD settings
 #
 #   @param  TypePCD type
-- 
1.9.5.msysgit.0


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] Question about WinDbg Support--

2015-06-09 Thread Saiprasad Chavali
Thanks Ruiyu, will do so.

Sai

From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Tuesday, June 09, 2015 7:13 PM
To: Tian, Feng; Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Please also add /Gy- which can let WinDbg recognize the calling stack correctly.

From: Tian, Feng
Sent: Wednesday, June 10, 2015 9:40 AM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Conclusion sharing after offline discussion with Sai:

After adding /GL- besides /Od, the local variables/function parameters could be 
resolved in Windbg.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 08:35
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Subject: RE: [edk2] Question about WinDbg Support--

HI,

I see the warning when it is compiling the module, overwriting the default /O1 
with /Od and from the "makefile", here is the macro:

CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1ib2 /GL /FIAutoGen.h 
/EHs-c- /GR- /GF /Gy /Zi /Gm /D XA_USB_TRANS /Od

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Tuesday, June 09, 2015 5:21 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

I think it should be ok if /Od takes effect. Could you double check those 
modules' Makefile files in build directory? Is the /Od compiler option really 
added in CC_FLAGS macro?

Ray,

Do you have any comment on this issue?

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 05:47
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi,

Even after turning on the
 "Build option: /Od"
for the Dxe module/driver, local variables/function parameters are not getting 
resolved, wondering any missing steps, while compiling.

The symbol path looks fine to me as I am able to step through the code.

I have set
"*_*_*_GENFW_FLAGS = --keepexceptiontable"  as well.

Thanks
Sai

From: Saiprasad Chavali
Sent: Tuesday, May 12, 2015 9:20 AM
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Yes the Host connects to the Target with Windbg. And I can see Initial serial 
dump on the Terminal redirect port.
We don't have any standard platform yet, so using the DELL laptop to test USB/ 
Bluetooth Functionality.

And I believe the exception happens after couple of protocol interfaces are 
loaded, I think we entered into the DXE phase (Not sure though).

If it is not a supported scenario, due think it make sense to chase it or try 
to load it standalone if we have to debug the Dxe Driver.

Thanks
Sai


From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Monday, May 11, 2015 11:33 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

What do you do on DuetPkg to enable SourceLevelDebug? We didn't enable and test 
such use case. Do you see the connection of Windbg with target?

The exception is not printed by EDKII Duet, it's more like legacy bios.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Tuesday, May 12, 2015 07:38
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Need some help, as USB->SerialAdapter is not an option, I replaced with a 
proper serial port, now I am having issues with Windbg on "DuetpkgIa32.dsc" FV 
image?

Looks like I am getting a CPU exception? Not sure what is wrong?

Your help is so much appreciated

Thanks
Sai

Access violation - code c005 (first/second chance not available)

* Target encountered an exception: Vector = 8, Error Code = 
be801426*

eax=be7cdf68 ebx=be815368 ecx= edx= esi=b97ce000 edi=
eip=0010 esp=be7cdf54 ebp=8000 iopl=0 nv up di pl nz na po nc
cs=0010  ss=0008  ds=0008  es=0008  fs=0008  gs=0008 efl=2ee0
0010 f1  ???


From: Saiprasad Chavali
Sent: Wednesday, May 06, 2015 6:52 PM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Thanks Feng, for the clarification.

Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 6:20 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Yes, the driver you saw is a UEFI driver and only for DXE phase. To enable this 
feature, you need develop a new DebugCommunicationLibFtdi library instance for 
SEC/PEI/DXE phase.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sen

Re: [edk2] Question about WinDbg Support--

2015-06-09 Thread Ni, Ruiyu
Please also add /Gy- which can let WinDbg recognize the calling stack correctly.

From: Tian, Feng
Sent: Wednesday, June 10, 2015 9:40 AM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Conclusion sharing after offline discussion with Sai:

After adding /GL- besides /Od, the local variables/function parameters could be 
resolved in Windbg.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 08:35
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Subject: RE: [edk2] Question about WinDbg Support--

HI,

I see the warning when it is compiling the module, overwriting the default /O1 
with /Od and from the "makefile", here is the macro:

CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1ib2 /GL /FIAutoGen.h 
/EHs-c- /GR- /GF /Gy /Zi /Gm /D XA_USB_TRANS /Od

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Tuesday, June 09, 2015 5:21 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

I think it should be ok if /Od takes effect. Could you double check those 
modules' Makefile files in build directory? Is the /Od compiler option really 
added in CC_FLAGS macro?

Ray,

Do you have any comment on this issue?

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 05:47
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi,

Even after turning on the
 "Build option: /Od"
for the Dxe module/driver, local variables/function parameters are not getting 
resolved, wondering any missing steps, while compiling.

The symbol path looks fine to me as I am able to step through the code.

I have set
"*_*_*_GENFW_FLAGS = --keepexceptiontable"  as well.

Thanks
Sai

From: Saiprasad Chavali
Sent: Tuesday, May 12, 2015 9:20 AM
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Yes the Host connects to the Target with Windbg. And I can see Initial serial 
dump on the Terminal redirect port.
We don't have any standard platform yet, so using the DELL laptop to test USB/ 
Bluetooth Functionality.

And I believe the exception happens after couple of protocol interfaces are 
loaded, I think we entered into the DXE phase (Not sure though).

If it is not a supported scenario, due think it make sense to chase it or try 
to load it standalone if we have to debug the Dxe Driver.

Thanks
Sai


From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Monday, May 11, 2015 11:33 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

What do you do on DuetPkg to enable SourceLevelDebug? We didn't enable and test 
such use case. Do you see the connection of Windbg with target?

The exception is not printed by EDKII Duet, it's more like legacy bios.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Tuesday, May 12, 2015 07:38
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Need some help, as USB->SerialAdapter is not an option, I replaced with a 
proper serial port, now I am having issues with Windbg on "DuetpkgIa32.dsc" FV 
image?

Looks like I am getting a CPU exception? Not sure what is wrong?

Your help is so much appreciated

Thanks
Sai

Access violation - code c005 (first/second chance not available)

* Target encountered an exception: Vector = 8, Error Code = 
be801426*

eax=be7cdf68 ebx=be815368 ecx= edx= esi=b97ce000 edi=
eip=0010 esp=be7cdf54 ebp=8000 iopl=0 nv up di pl nz na po nc
cs=0010  ss=0008  ds=0008  es=0008  fs=0008  gs=0008 efl=2ee0
0010 f1  ???


From: Saiprasad Chavali
Sent: Wednesday, May 06, 2015 6:52 PM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Thanks Feng, for the clarification.

Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 6:20 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Yes, the driver you saw is a UEFI driver and only for DXE phase. To enable this 
feature, you need develop a new DebugCommunicationLibFtdi library instance for 
SEC/PEI/DXE phase.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 8:56 AM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Feng,

I have the Ftdi

Re: [edk2] [PATCH] BaseTools\Source\Python\Makefile

2015-06-09 Thread Gao, Liming
Larry:
  Please replace TAB with space.

Thanks
Liming
From: Hauch, Larry [mailto:larry.ha...@intel.com]
Sent: Wednesday, June 10, 2015 2:50 AM
To: edk2-devel@lists.sourceforge.net
Subject: [edk2] [PATCH] BaseTools\Source\Python\Makefile

The attached patch adds new files to the Makefile for testing changed sources.
The files were added April 9th, revision 17158, but the Makefile was not 
updated.


Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: lhauch mailto:larry.ha...@intel.com>>

Reviewed-by: Joe Peterson 
mailto:joe.peter...@intel.com>>

Cheers,
Larry Hauch
Intel Corporation
SSG, STO, Platform Software Infrastructure
705 5th Ave S. Suite 500
Seattle, WA 98104
Work: (206) 701-8842

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] Question about WinDbg Support--

2015-06-09 Thread Tian, Feng
Conclusion sharing after offline discussion with Sai:

After adding /GL- besides /Od, the local variables/function parameters could be 
resolved in Windbg.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 08:35
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Subject: RE: [edk2] Question about WinDbg Support--

HI,

I see the warning when it is compiling the module, overwriting the default /O1 
with /Od and from the "makefile", here is the macro:

CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1ib2 /GL /FIAutoGen.h 
/EHs-c- /GR- /GF /Gy /Zi /Gm /D XA_USB_TRANS /Od

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Tuesday, June 09, 2015 5:21 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

I think it should be ok if /Od takes effect. Could you double check those 
modules' Makefile files in build directory? Is the /Od compiler option really 
added in CC_FLAGS macro?

Ray,

Do you have any comment on this issue?

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 05:47
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi,

Even after turning on the
 "Build option: /Od"
for the Dxe module/driver, local variables/function parameters are not getting 
resolved, wondering any missing steps, while compiling.

The symbol path looks fine to me as I am able to step through the code.

I have set
"*_*_*_GENFW_FLAGS = --keepexceptiontable"  as well.

Thanks
Sai

From: Saiprasad Chavali
Sent: Tuesday, May 12, 2015 9:20 AM
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Yes the Host connects to the Target with Windbg. And I can see Initial serial 
dump on the Terminal redirect port.
We don't have any standard platform yet, so using the DELL laptop to test USB/ 
Bluetooth Functionality.

And I believe the exception happens after couple of protocol interfaces are 
loaded, I think we entered into the DXE phase (Not sure though).

If it is not a supported scenario, due think it make sense to chase it or try 
to load it standalone if we have to debug the Dxe Driver.

Thanks
Sai


From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Monday, May 11, 2015 11:33 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

What do you do on DuetPkg to enable SourceLevelDebug? We didn't enable and test 
such use case. Do you see the connection of Windbg with target?

The exception is not printed by EDKII Duet, it's more like legacy bios.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Tuesday, May 12, 2015 07:38
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Need some help, as USB->SerialAdapter is not an option, I replaced with a 
proper serial port, now I am having issues with Windbg on "DuetpkgIa32.dsc" FV 
image?

Looks like I am getting a CPU exception? Not sure what is wrong?

Your help is so much appreciated

Thanks
Sai

Access violation - code c005 (first/second chance not available)

* Target encountered an exception: Vector = 8, Error Code = 
be801426*

eax=be7cdf68 ebx=be815368 ecx= edx= esi=b97ce000 edi=
eip=0010 esp=be7cdf54 ebp=8000 iopl=0 nv up di pl nz na po nc
cs=0010  ss=0008  ds=0008  es=0008  fs=0008  gs=0008 efl=2ee0
0010 f1  ???


From: Saiprasad Chavali
Sent: Wednesday, May 06, 2015 6:52 PM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Thanks Feng, for the clarification.

Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 6:20 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Yes, the driver you saw is a UEFI driver and only for DXE phase. To enable this 
feature, you need develop a new DebugCommunicationLibFtdi library instance for 
SEC/PEI/DXE phase.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 8:56 AM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Feng,

I have the Ftdi driver (optionrompackage) part of the image, unfortunately this 
will not get loaded during early DXE phase (OR) supports the debug agent lib 
right?

I believe the debug agent works with "debug communication lib usb" (or) "debug 
communication serial port", if not mistaken

Re: [edk2] attention git users: a productivity boost for edk2

2015-06-09 Thread Gao, Liming
Good BKM sharing. 

EDKII meta data file has .inf, .dec, *.dsc and *.fdf. What's *.dsc.inc and 
*.fdf.inc? They are the part of DSC and FDF files that included in the full DSC 
and FDF files?

Thanks
Liming
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, June 10, 2015 1:38 AM
To: edk2-devel list
Subject: [edk2] attention git users: a productivity boost for edk2

(Sorry about the sensationalist subject line, I don't have a degree in 
marketing :))

Edk2 uses a large number of text files with *sections*:

[Section.LOL]

I'm sure you've been annoyed quite a few times, while reviewing patches, that 
you couldn't immediately see the section that a hunk modified. (I know I have.) 
We used to have two solutions for this:

- On the reviewer side, apply the patchset and review it patch-wise,
  against the full source code as context.

- On the sender side, generate the patches with a larger context.
  Options are -U, --inter-hunk-context=, and even
  --function-context. Unfortunately, these don't resolve the question of
  sections reliably (or they produce overkill output).

Turns out git has a trick for this up its sleeve: see gitattributes(5).

(1) Edit your .git/info/attributes file, adding the following lines:

*.dec diff=ini
*.dsc diff=ini
*.dsc.inc diff=ini
*.fdf diff=ini
*.fdf.inc diff=ini
*.inf diff=ini

(2) in your .git/config, add

[diff "ini"]
xfuncname = "^\\[[A-Za-z0-9_., ]+]"

I just set these in my edk2 clone, and tested them on my local branch where I 
had applied Ard's series

  [PATCH 0/7] ArmPkg/ArmVirtPkg: GIC revision detection

for review. A good example is patch #4, "ArmPkg: copy ArmGicArchLib to 
ArmGicArchSecLib". Now I can immediately see, in the hunk headers (@@), what 
section each change belongs to.

Here it is (git show output trimmed to relevant files):

> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> index 0859bc3..6e6687c 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> @@ -138,6 +138,7 @@ [LibraryClasses.common.SEC]
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg.dsc
> index be31025..14d82f6 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
> @@ -134,6 +134,8 @@ [LibraryClasses.common.SEC]
>
> DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
>
> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/D
> efaultExceptionHandlerLibBase.inf
>
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc 
> b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> index 19de381..4b4867f 100644
> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> @@ -128,6 +128,8 @@ [LibraryClasses.common.SEC]
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/P
> rePiHobListPointerLib.inf
>  !endif
>
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc 
> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> index 84e2a99..8f7b5f1 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> @@ -144,6 +144,8 @@ [LibraryClasses.common.SEC]
># Trustzone Support
>
> ArmTrustedMonitorLib|ArmPlatformPkg/Library/ArmTrustedMonitorLibNull/A
> rmTrustedMonitorLibNull.inf
>
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.PEI_CORE]
>HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf

This git-diff output shows quickly that Ard resolved the library class for SEC 
modules, a fact that was not visible otherwise from the patch itself.

I recommend that all git users working with edk2 apply these settings -- they 
are *very* helpful for reviewers. Personally, I will make this a requirement 
for patches that I'm expected (or asked) to review.

Re: [edk2] [PATCH 3/3] EdkCompatibilityPkg SmmBaseHelper: Unregister profile image correctly.

2015-06-09 Thread Zeng, Star
It is because the PCD PcdMemoryProfilePropertyMask is only essentially used in 
SmramProfileRecord.c.
So move the include PcdLib.h from SmmBaseHelper.c to SmramProfileRecord.c.

Thanks,
Star
-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Wednesday, June 10, 2015 2:02 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [PATCH 3/3] EdkCompatibilityPkg SmmBaseHelper: Unregister 
profile image correctly.

On 9 June 2015 at 04:10, Star Zeng  wrote:
> Call UnregisterSmramProfileImage() before image buffer freed.
>
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c  | 5 
> ++---
>  EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c 
> | 3 ++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git 
> a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c 
> b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> index 2b94e4d..1d16449 100644
> --- a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> +++ b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> @@ -11,7 +11,7 @@
>
>SmmHandlerEntry() will receive untrusted input and do validation.
>
> -  Copyright (c) 2009 - 2013, Intel Corporation. All rights 
> reserved.
> +  Copyright (c) 2009 - 2015, Intel Corporation. All rights 
> + reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license 
> may be found at @@ -34,7 +34,6 @@  #include 
>   #include 
>   #include  -#include 
> 

What is this change for? It is not mentioned in the commit log.

>  #include 
>  #include   #include 
>  @@ -734,10 +733,10 @@ LoadImage (
>  RegisterSmramProfileImage (FilePath, DstBuffer, PageCount);
>  Status = gBS->StartImage (*ImageHandle, NULL, NULL);
>  if (EFI_ERROR (Status)) {
> +  UnregisterSmramProfileImage (FilePath, DstBuffer, PageCount);
>mLoadPe32Image->UnLoadPeImage (mLoadPe32Image, *ImageHandle);
>*ImageHandle = NULL;
>FreePages ((VOID *)(UINTN)DstBuffer, PageCount);
> -  UnregisterSmramProfileImage (FilePath, DstBuffer, PageCount);
>  }
>}
>
> diff --git 
> a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c 
> b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c
> index 84eba48..a1797ea 100644
> --- 
> a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c
> +++ b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileReco
> +++ rd.c
> @@ -1,6 +1,6 @@
>  /** @file
>
> -  Copyright (c) 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2014 - 2015, Intel Corporation. All rights 
> + reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license 
> may be found at @@ -20,6 +20,7 @@  #include   
> #include   #include 
> +#include 

And this one?

>  #include 
>
>  #include 
> --
> 1.9.5.msysgit.0
>
>
> --
>  ___
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] Question about WinDbg Support--

2015-06-09 Thread Saiprasad Chavali
HI,

I see the warning when it is compiling the module, overwriting the default /O1 
with /Od and from the "makefile", here is the macro:

CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1ib2 /GL /FIAutoGen.h 
/EHs-c- /GR- /GF /Gy /Zi /Gm /D XA_USB_TRANS /Od

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Tuesday, June 09, 2015 5:21 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'; Ni, Ruiyu
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

I think it should be ok if /Od takes effect. Could you double check those 
modules' Makefile files in build directory? Is the /Od compiler option really 
added in CC_FLAGS macro?

Ray,

Do you have any comment on this issue?

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 05:47
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi,

Even after turning on the
 "Build option: /Od"
for the Dxe module/driver, local variables/function parameters are not getting 
resolved, wondering any missing steps, while compiling.

The symbol path looks fine to me as I am able to step through the code.

I have set
"*_*_*_GENFW_FLAGS = --keepexceptiontable"  as well.

Thanks
Sai

From: Saiprasad Chavali
Sent: Tuesday, May 12, 2015 9:20 AM
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Yes the Host connects to the Target with Windbg. And I can see Initial serial 
dump on the Terminal redirect port.
We don't have any standard platform yet, so using the DELL laptop to test USB/ 
Bluetooth Functionality.

And I believe the exception happens after couple of protocol interfaces are 
loaded, I think we entered into the DXE phase (Not sure though).

If it is not a supported scenario, due think it make sense to chase it or try 
to load it standalone if we have to debug the Dxe Driver.

Thanks
Sai


From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Monday, May 11, 2015 11:33 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

What do you do on DuetPkg to enable SourceLevelDebug? We didn't enable and test 
such use case. Do you see the connection of Windbg with target?

The exception is not printed by EDKII Duet, it's more like legacy bios.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Tuesday, May 12, 2015 07:38
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Need some help, as USB->SerialAdapter is not an option, I replaced with a 
proper serial port, now I am having issues with Windbg on "DuetpkgIa32.dsc" FV 
image?

Looks like I am getting a CPU exception? Not sure what is wrong?

Your help is so much appreciated

Thanks
Sai

Access violation - code c005 (first/second chance not available)

* Target encountered an exception: Vector = 8, Error Code = 
be801426*

eax=be7cdf68 ebx=be815368 ecx= edx= esi=b97ce000 edi=
eip=0010 esp=be7cdf54 ebp=8000 iopl=0 nv up di pl nz na po nc
cs=0010  ss=0008  ds=0008  es=0008  fs=0008  gs=0008 efl=2ee0
0010 f1  ???


From: Saiprasad Chavali
Sent: Wednesday, May 06, 2015 6:52 PM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Thanks Feng, for the clarification.

Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 6:20 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Yes, the driver you saw is a UEFI driver and only for DXE phase. To enable this 
feature, you need develop a new DebugCommunicationLibFtdi library instance for 
SEC/PEI/DXE phase.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 8:56 AM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Feng,

I have the Ftdi driver (optionrompackage) part of the image, unfortunately this 
will not get loaded during early DXE phase (OR) supports the debug agent lib 
right?

I believe the debug agent works with "debug communication lib usb" (or) "debug 
communication serial port", if not mistaken.

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 5:42 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi, Sai

We don't support this usage. Such usage model needs a Ftdi drive

Re: [edk2] Question about WinDbg Support--

2015-06-09 Thread Tian, Feng
Sai,

I think it should be ok if /Od takes effect. Could you double check those 
modules' Makefile files in build directory? Is the /Od compiler option really 
added in CC_FLAGS macro?

Ray,

Do you have any comment on this issue?

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Wednesday, June 10, 2015 05:47
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi,

Even after turning on the
 "Build option: /Od"
for the Dxe module/driver, local variables/function parameters are not getting 
resolved, wondering any missing steps, while compiling.

The symbol path looks fine to me as I am able to step through the code.

I have set
"*_*_*_GENFW_FLAGS = --keepexceptiontable"  as well.

Thanks
Sai

From: Saiprasad Chavali
Sent: Tuesday, May 12, 2015 9:20 AM
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Yes the Host connects to the Target with Windbg. And I can see Initial serial 
dump on the Terminal redirect port.
We don't have any standard platform yet, so using the DELL laptop to test USB/ 
Bluetooth Functionality.

And I believe the exception happens after couple of protocol interfaces are 
loaded, I think we entered into the DXE phase (Not sure though).

If it is not a supported scenario, due think it make sense to chase it or try 
to load it standalone if we have to debug the Dxe Driver.

Thanks
Sai


From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Monday, May 11, 2015 11:33 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

What do you do on DuetPkg to enable SourceLevelDebug? We didn't enable and test 
such use case. Do you see the connection of Windbg with target?

The exception is not printed by EDKII Duet, it's more like legacy bios.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Tuesday, May 12, 2015 07:38
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Need some help, as USB->SerialAdapter is not an option, I replaced with a 
proper serial port, now I am having issues with Windbg on "DuetpkgIa32.dsc" FV 
image?

Looks like I am getting a CPU exception? Not sure what is wrong?

Your help is so much appreciated

Thanks
Sai

Access violation - code c005 (first/second chance not available)

* Target encountered an exception: Vector = 8, Error Code = 
be801426*

eax=be7cdf68 ebx=be815368 ecx= edx= esi=b97ce000 edi=
eip=0010 esp=be7cdf54 ebp=8000 iopl=0 nv up di pl nz na po nc
cs=0010  ss=0008  ds=0008  es=0008  fs=0008  gs=0008 efl=2ee0
0010 f1  ???


From: Saiprasad Chavali
Sent: Wednesday, May 06, 2015 6:52 PM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Thanks Feng, for the clarification.

Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 6:20 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Yes, the driver you saw is a UEFI driver and only for DXE phase. To enable this 
feature, you need develop a new DebugCommunicationLibFtdi library instance for 
SEC/PEI/DXE phase.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 8:56 AM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Feng,

I have the Ftdi driver (optionrompackage) part of the image, unfortunately this 
will not get loaded during early DXE phase (OR) supports the debug agent lib 
right?

I believe the debug agent works with "debug communication lib usb" (or) "debug 
communication serial port", if not mistaken.

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 5:42 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi, Sai

We don't support this usage. Such usage model needs a Ftdi driver in Target 
BIOS.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 1:18 AM
To: edk2-devel@lists.sourceforge.net; 
Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Tian,

The host is fine, how about the Target? The problem is with the Target the 
Laptops/pc no more serial ports, so using the USB adapter?


Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]

Re: [edk2] Question about WinDbg Support--

2015-06-09 Thread Saiprasad Chavali
Hi,

Even after turning on the
 "Build option: /Od"
for the Dxe module/driver, local variables/function parameters are not getting 
resolved, wondering any missing steps, while compiling.

The symbol path looks fine to me as I am able to step through the code.

I have set
"*_*_*_GENFW_FLAGS = --keepexceptiontable"  as well.

Thanks
Sai

From: Saiprasad Chavali
Sent: Tuesday, May 12, 2015 9:20 AM
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Yes the Host connects to the Target with Windbg. And I can see Initial serial 
dump on the Terminal redirect port.
We don't have any standard platform yet, so using the DELL laptop to test USB/ 
Bluetooth Functionality.

And I believe the exception happens after couple of protocol interfaces are 
loaded, I think we entered into the DXE phase (Not sure though).

If it is not a supported scenario, due think it make sense to chase it or try 
to load it standalone if we have to debug the Dxe Driver.

Thanks
Sai


From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Monday, May 11, 2015 11:33 PM
To: Saiprasad Chavali; 'edk2-devel@lists.sourceforge.net'
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support--

Sai,

What do you do on DuetPkg to enable SourceLevelDebug? We didn't enable and test 
such use case. Do you see the connection of Windbg with target?

The exception is not printed by EDKII Duet, it's more like legacy bios.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Tuesday, May 12, 2015 07:38
To: Tian, Feng; 'edk2-devel@lists.sourceforge.net'
Subject: RE: [edk2] Question about WinDbg Support--

Hi Feng,

Need some help, as USB->SerialAdapter is not an option, I replaced with a 
proper serial port, now I am having issues with Windbg on "DuetpkgIa32.dsc" FV 
image?

Looks like I am getting a CPU exception? Not sure what is wrong?

Your help is so much appreciated

Thanks
Sai

Access violation - code c005 (first/second chance not available)

* Target encountered an exception: Vector = 8, Error Code = 
be801426*

eax=be7cdf68 ebx=be815368 ecx= edx= esi=b97ce000 edi=
eip=0010 esp=be7cdf54 ebp=8000 iopl=0 nv up di pl nz na po nc
cs=0010  ss=0008  ds=0008  es=0008  fs=0008  gs=0008 efl=2ee0
0010 f1  ???


From: Saiprasad Chavali
Sent: Wednesday, May 06, 2015 6:52 PM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Thanks Feng, for the clarification.

Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 6:20 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Yes, the driver you saw is a UEFI driver and only for DXE phase. To enable this 
feature, you need develop a new DebugCommunicationLibFtdi library instance for 
SEC/PEI/DXE phase.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 8:56 AM
To: Tian, Feng; 
edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Feng,

I have the Ftdi driver (optionrompackage) part of the image, unfortunately this 
will not get loaded during early DXE phase (OR) supports the debug agent lib 
right?

I believe the debug agent works with "debug communication lib usb" (or) "debug 
communication serial port", if not mistaken.

Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 5:42 PM
To: Saiprasad Chavali; 
edk2-devel@lists.sourceforge.net
Cc: Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi, Sai

We don't support this usage. Such usage model needs a Ftdi driver in Target 
BIOS.

Thanks
Feng

From: Saiprasad Chavali [mailto:s...@marvell.com]
Sent: Thursday, May 7, 2015 1:18 AM
To: edk2-devel@lists.sourceforge.net; 
Tian, Feng
Subject: RE: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

Hi Tian,

The host is fine, how about the Target? The problem is with the Target the 
Laptops/pc no more serial ports, so using the USB adapter?


Thanks
Sai

From: Tian, Feng [mailto:feng.t...@intel.com]
Sent: Wednesday, May 06, 2015 1:33 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] Question about WinDbg Support-- Target with USB->Serial 
Adapter.

For Usb2Serial cable, we support the usage that the cable's usb end is 
connected with Host and the cable's serial end is connected with Target.

By this way, Host will identify a new COM port when connecting this cable like 
above.

Re: [edk2] [PATCH 4/7] ArmPkg: copy ArmGicArchLib to ArmGicArchSecLib

2015-06-09 Thread Laszlo Ersek
On 06/09/15 22:42, Ard Biesheuvel wrote:
> On 9 June 2015 at 18:33, Laszlo Ersek  wrote:
>> On 05/29/15 14:33, Ard Biesheuvel wrote:
>>> Clone ArmGicArchLib into a SEC phase specific ArmGicArchSecLib
>>> so that we can modify the former in a subsequent patch to cache
>>> the GIC revision in a global variable.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/AArch64/ArmGicArchLib.c 
>>>| 0
>>>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/Arm/ArmGicArchLib.c 
>>>| 0
>>>  ArmPkg/Library/{ArmGicArchLib/ArmGicArchLib.inf => 
>>> ArmGicArchSecLib/ArmGicArchSecLib.inf} | 6 +++---
>>>  ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
>>>| 1 +
>>>  ArmPlatformPkg/ArmPlatformPkg.dsc  
>>>| 2 ++
>>>  ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc  
>>>| 2 ++
>>>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc  
>>>| 2 ++
>>>  7 files changed, 10 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c 
>>> b/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
>>> similarity index 100%
>>> copy from ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
>>> copy to ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
>>> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c 
>>> b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
>>> similarity index 100%
>>> copy from ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
>>> copy to ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
>>> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf 
>>> b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>> similarity index 79%
>>> copy from ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>>> copy to ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>> index d71b2adc3027..a2fb623a8537 100644
>>> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>>> +++ b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>> @@ -13,11 +13,11 @@
>>>
>>>  [Defines]
>>>INF_VERSION= 0x00010005
>>> -  BASE_NAME  = ArmGicArchLib
>>> -  FILE_GUID  = cd67f41a-26e9-4482-90c9-a9aff803382a
>>> +  BASE_NAME  = ArmGicArchSecLib
>>> +  FILE_GUID  = c1dd9745-9459-4e9a-9f5b-99cbd233c27d
>>>MODULE_TYPE= BASE
>>>VERSION_STRING = 1.0
>>> -  LIBRARY_CLASS  = ArmGicArchLib
>>> +  LIBRARY_CLASS  = ArmGicArchLib|SEC
>>
>> * Okay, so this patch does the clone for SEC modules and points the SEC
>> library class resolutions in the DSC files to the clone, overriding the
>> default resolutions in them.
>>
>> ArmPkg/ArmPkg.dsc and ArmVirtPkg/ArmVirt.dsc.inc are not updated. For
>> ArmVirtPkg, the default resolution stays intact in this patch, which
>> makes sense, because patch #7 handles it separately.
>>
>> But, is there any particular reason to leave ArmPkg/ArmPkg.dsc unchanged
>> in this patch?
> 
> Actually, that is an oversight. I should probably add it there for
> completeness, although the exact purpose of these package .DSCs
> escapes me, to be honest.

Me too. :)

> Anyway, ArmPkg.dsc still builds ok with these changes, so I assume
> there is no SEC module being built that [transitively] relies on
> ArmGicArchLib

Right.

> 
>> Hm... I guess it should be safe, because patch #5 is
>> explicit about DXE_DRIVER, UEFI_DRIVER, UEFI_APPLICATION; so if there's
>> a problem later, it will show up as a build time failure. Okay.
>>
>> * Anyway, my main concern was PEIMs. Especially because in patch #3 you
>> wrote,
>>
>>> this statelessness is only needed for SEC type modules
>>
>> This is in general not true, because PEIMs also may execute-in-place
>> from flash. However, from patch #5 I can see that the caching version
>> will be available only to DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION,
>> leaving PEIMs uncovered -- which is safe, and also good enough since
>> (apparently) no PEIMs depend on this library class.
>>
> 
> i have you on record (and I can dig up the email, if you like) saying
> that you prefer the LIBRARY_CLASS phase modifiers to be minimal.

Do you also have me on record for never contradicting myself? ;)

So, don't bother.

Cheers
Laszlo

>> If you have to respin the series for any reason, please consider
>> mentioning PEIMs in the commit message of patch #3 and patch #4, and
>> making ArmGicArchSecLib available to PEIMs too. However, as I said
>> above, that would be quite speculative, and this could be addressed any
>> time later, if a new PEIM actually needed the library. So feel free to
>> ignore this note.
>>
>> Reviewed-by: Laszlo Ersek 
>>
> 
> Thanks,
> Ard,
>

Re: [edk2] [PATCH 4/7] ArmPkg: copy ArmGicArchLib to ArmGicArchSecLib

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 18:33, Laszlo Ersek  wrote:
> On 05/29/15 14:33, Ard Biesheuvel wrote:
>> Clone ArmGicArchLib into a SEC phase specific ArmGicArchSecLib
>> so that we can modify the former in a subsequent patch to cache
>> the GIC revision in a global variable.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/AArch64/ArmGicArchLib.c  
>>   | 0
>>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/Arm/ArmGicArchLib.c  
>>   | 0
>>  ArmPkg/Library/{ArmGicArchLib/ArmGicArchLib.inf => 
>> ArmGicArchSecLib/ArmGicArchSecLib.inf} | 6 +++---
>>  ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc  
>>   | 1 +
>>  ArmPlatformPkg/ArmPlatformPkg.dsc   
>>   | 2 ++
>>  ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc   
>>   | 2 ++
>>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc   
>>   | 2 ++
>>  7 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c 
>> b/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
>> similarity index 100%
>> copy from ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
>> copy to ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
>> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c 
>> b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
>> similarity index 100%
>> copy from ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
>> copy to ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
>> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf 
>> b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> similarity index 79%
>> copy from ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>> copy to ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> index d71b2adc3027..a2fb623a8537 100644
>> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>> +++ b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> @@ -13,11 +13,11 @@
>>
>>  [Defines]
>>INF_VERSION= 0x00010005
>> -  BASE_NAME  = ArmGicArchLib
>> -  FILE_GUID  = cd67f41a-26e9-4482-90c9-a9aff803382a
>> +  BASE_NAME  = ArmGicArchSecLib
>> +  FILE_GUID  = c1dd9745-9459-4e9a-9f5b-99cbd233c27d
>>MODULE_TYPE= BASE
>>VERSION_STRING = 1.0
>> -  LIBRARY_CLASS  = ArmGicArchLib
>> +  LIBRARY_CLASS  = ArmGicArchLib|SEC
>
> * Okay, so this patch does the clone for SEC modules and points the SEC
> library class resolutions in the DSC files to the clone, overriding the
> default resolutions in them.
>
> ArmPkg/ArmPkg.dsc and ArmVirtPkg/ArmVirt.dsc.inc are not updated. For
> ArmVirtPkg, the default resolution stays intact in this patch, which
> makes sense, because patch #7 handles it separately.
>
> But, is there any particular reason to leave ArmPkg/ArmPkg.dsc unchanged
> in this patch?

Actually, that is an oversight. I should probably add it there for
completeness, although the exact purpose of these package .DSCs
escapes me, to be honest.
Anyway, ArmPkg.dsc still builds ok with these changes, so I assume
there is no SEC module being built that [transitively] relies on
ArmGicArchLib

> Hm... I guess it should be safe, because patch #5 is
> explicit about DXE_DRIVER, UEFI_DRIVER, UEFI_APPLICATION; so if there's
> a problem later, it will show up as a build time failure. Okay.
>
> * Anyway, my main concern was PEIMs. Especially because in patch #3 you
> wrote,
>
>> this statelessness is only needed for SEC type modules
>
> This is in general not true, because PEIMs also may execute-in-place
> from flash. However, from patch #5 I can see that the caching version
> will be available only to DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION,
> leaving PEIMs uncovered -- which is safe, and also good enough since
> (apparently) no PEIMs depend on this library class.
>

i have you on record (and I can dig up the email, if you like) saying
that you prefer the LIBRARY_CLASS phase modifiers to be minimal.

> If you have to respin the series for any reason, please consider
> mentioning PEIMs in the commit message of patch #3 and patch #4, and
> making ArmGicArchSecLib available to PEIMs too. However, as I said
> above, that would be quite speculative, and this could be addressed any
> time later, if a new PEIM actually needed the library. So feel free to
> ignore this note.
>
> Reviewed-by: Laszlo Ersek 
>

Thanks,
Ard,


>>
>>  [Sources.ARM]
>>Arm/ArmGicArchLib.c
>> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
>> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
>> index 0859bc31ff00..6e6687c8a735 100644
>> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
>> +++ b/ArmPlatformPkg/ArmPlat

Re: [edk2] attention git users: a productivity boost for edk2

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 20:09, Andrew Fish  wrote:
>
> On Jun 9, 2015, at 10:50 AM, Ard Biesheuvel 
> wrote:
>
> Turns out git has a trick for this up its sleeve: see gitattributes(5).
>
> (1) Edit your .git/info/attributes file, adding the following lines:
>
> *.dec diff=ini
> *.dsc diff=ini
> *.dsc.inc diff=ini
> *.fdf diff=ini
> *.fdf.inc diff=ini
> *.inf diff=ini
>
> (2) in your .git/config, add
>
> [diff "ini"]
>xfuncname = "^\\[[A-Za-z0-9_., ]+]"
>
>
> Nice find! Very useful indeed.
>
>
> Can we check this into git to make it the default for the project?
>

The attributes file could also live in $WORKSPACE/.gitattributes, so
that part can easily be checked in.

The config snippet could be added to $WORKSPACE/.gitconfig (or any
other file), and included by the repo config by doing something like

git config --add include.path ../.gitconfig

Something like this could perhaps be added to edksetup.___ or some
other script that is involved in setting up the workspace

-- 
Ard.

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [PATCH] BaseTools\Source\Python\Makefile

2015-06-09 Thread Hauch, Larry
The attached patch adds new files to the Makefile for testing changed sources.
The files were added April 9th, revision 17158, but the Makefile was not 
updated.


Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: lhauch mailto:larry.ha...@intel.com>>

Reviewed-by: Joe Peterson 
mailto:joe.peter...@intel.com>>

Cheers,
Larry Hauch
Intel Corporation
SSG, STO, Platform Software Infrastructure
705 5th Ave S. Suite 500
Seattle, WA 98104
Work: (206) 701-8842



Makefile.patch
Description: Makefile.patch
--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] attention git users: a productivity boost for edk2

2015-06-09 Thread Andrew Fish

> On Jun 9, 2015, at 10:50 AM, Ard Biesheuvel  wrote:
> 
>> Turns out git has a trick for this up its sleeve: see gitattributes(5).
>> 
>> (1) Edit your .git/info/attributes file, adding the following lines:
>> 
>> *.dec diff=ini
>> *.dsc diff=ini
>> *.dsc.inc diff=ini
>> *.fdf diff=ini
>> *.fdf.inc diff=ini
>> *.inf diff=ini
>> 
>> (2) in your .git/config, add
>> 
>> [diff "ini"]
>>xfuncname = "^\\[[A-Za-z0-9_., ]+]"
>> 
> 
> Nice find! Very useful indeed.

Can we check this into git to make it the default for the project?

Thanks,

Andrew Fish--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 3/3] EdkCompatibilityPkg SmmBaseHelper: Unregister profile image correctly.

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 04:10, Star Zeng  wrote:
> Call UnregisterSmramProfileImage() before image buffer freed.
>
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c  | 5 
> ++---
>  EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c | 3 ++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c 
> b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> index 2b94e4d..1d16449 100644
> --- a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> +++ b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> @@ -11,7 +11,7 @@
>
>SmmHandlerEntry() will receive untrusted input and do validation.
>
> -  Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -34,7 +34,6 @@
>  #include 
>  #include 
>  #include 
> -#include 

What is this change for? It is not mentioned in the commit log.

>  #include 
>  #include 
>  #include 
> @@ -734,10 +733,10 @@ LoadImage (
>  RegisterSmramProfileImage (FilePath, DstBuffer, PageCount);
>  Status = gBS->StartImage (*ImageHandle, NULL, NULL);
>  if (EFI_ERROR (Status)) {
> +  UnregisterSmramProfileImage (FilePath, DstBuffer, PageCount);
>mLoadPe32Image->UnLoadPeImage (mLoadPe32Image, *ImageHandle);
>*ImageHandle = NULL;
>FreePages ((VOID *)(UINTN)DstBuffer, PageCount);
> -  UnregisterSmramProfileImage (FilePath, DstBuffer, PageCount);
>  }
>}
>
> diff --git 
> a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c 
> b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c
> index 84eba48..a1797ea 100644
> --- a/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c
> +++ b/EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmramProfileRecord.c
> @@ -1,6 +1,6 @@
>  /** @file
>
> -  Copyright (c) 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2014 - 2015, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

And this one?

>  #include 
>
>  #include 
> --
> 1.9.5.msysgit.0
>
>
> --
> ___
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] attention git users: a productivity boost for edk2

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 19:37, Laszlo Ersek  wrote:
> (Sorry about the sensationalist subject line, I don't have a degree in
> marketing :))
>
> Edk2 uses a large number of text files with *sections*:
>
> [Section.LOL]
>
> I'm sure you've been annoyed quite a few times, while reviewing patches,
> that you couldn't immediately see the section that a hunk modified. (I
> know I have.) We used to have two solutions for this:
>
> - On the reviewer side, apply the patchset and review it patch-wise,
>   against the full source code as context.
>
> - On the sender side, generate the patches with a larger context.
>   Options are -U, --inter-hunk-context=, and even
>   --function-context. Unfortunately, these don't resolve the question of
>   sections reliably (or they produce overkill output).
>
> Turns out git has a trick for this up its sleeve: see gitattributes(5).
>
> (1) Edit your .git/info/attributes file, adding the following lines:
>
> *.dec diff=ini
> *.dsc diff=ini
> *.dsc.inc diff=ini
> *.fdf diff=ini
> *.fdf.inc diff=ini
> *.inf diff=ini
>
> (2) in your .git/config, add
>
> [diff "ini"]
> xfuncname = "^\\[[A-Za-z0-9_., ]+]"
>

Nice find! Very useful indeed.

-- 
Ard.

> I just set these in my edk2 clone, and tested them on my local branch
> where I had applied Ard's series
>
>   [PATCH 0/7] ArmPkg/ArmVirtPkg: GIC revision detection
>
> for review. A good example is patch #4, "ArmPkg: copy ArmGicArchLib to
> ArmGicArchSecLib". Now I can immediately see, in the hunk headers (@@),
> what section each change belongs to.
>
> Here it is (git show output trimmed to relevant files):
>
>> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
>> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
>> index 0859bc3..6e6687c 100644
>> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
>> +++ b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
>> @@ -138,6 +138,7 @@ [LibraryClasses.common.SEC]
>>
>> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>>
>> PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>>PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>
>>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc 
>> b/ArmPlatformPkg/ArmPlatformPkg.dsc
>> index be31025..14d82f6 100644
>> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
>> @@ -134,6 +134,8 @@ [LibraryClasses.common.SEC]
>>
>> DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
>>
>> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
>>
>> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> +
>>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>
>> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc 
>> b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
>> index 19de381..4b4867f 100644
>> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
>> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
>> @@ -128,6 +128,8 @@ [LibraryClasses.common.SEC]
>>
>> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>>  !endif
>>
>> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> +
>>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>>
>> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc 
>> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
>> index 84e2a99..8f7b5f1 100644
>> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
>> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
>> @@ -144,6 +144,8 @@ [LibraryClasses.common.SEC]
>># Trustzone Support
>>
>> ArmTrustedMonitorLib|ArmPlatformPkg/Library/ArmTrustedMonitorLibNull/ArmTrustedMonitorLibNull.inf
>>
>> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>> +
>>  [LibraryClasses.common.PEI_CORE]
>>HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>>PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
>
> This git-diff output shows quickly that Ard resolved the library class
> for SEC modules, a fact that was not visible otherwise from the patch
> itself.
>
> I recommend that all git users working with edk2 apply these settings --
> they are *very* helpful for reviewers. Personally, I will make this a
> requirement for patches that I'm expected (or asked) to review.
>
> Thanks
> Laszlo
>
> --
> ___
> edk2-devel mailing list
> 

[edk2] attention git users: a productivity boost for edk2

2015-06-09 Thread Laszlo Ersek
(Sorry about the sensationalist subject line, I don't have a degree in
marketing :))

Edk2 uses a large number of text files with *sections*:

[Section.LOL]

I'm sure you've been annoyed quite a few times, while reviewing patches,
that you couldn't immediately see the section that a hunk modified. (I
know I have.) We used to have two solutions for this:

- On the reviewer side, apply the patchset and review it patch-wise,
  against the full source code as context.

- On the sender side, generate the patches with a larger context.
  Options are -U, --inter-hunk-context=, and even
  --function-context. Unfortunately, these don't resolve the question of
  sections reliably (or they produce overkill output).

Turns out git has a trick for this up its sleeve: see gitattributes(5).

(1) Edit your .git/info/attributes file, adding the following lines:

*.dec diff=ini
*.dsc diff=ini
*.dsc.inc diff=ini
*.fdf diff=ini
*.fdf.inc diff=ini
*.inf diff=ini

(2) in your .git/config, add

[diff "ini"]
xfuncname = "^\\[[A-Za-z0-9_., ]+]"

I just set these in my edk2 clone, and tested them on my local branch
where I had applied Ard's series

  [PATCH 0/7] ArmPkg/ArmVirtPkg: GIC revision detection

for review. A good example is patch #4, "ArmPkg: copy ArmGicArchLib to
ArmGicArchSecLib". Now I can immediately see, in the hunk headers (@@),
what section each change belongs to.

Here it is (git show output trimmed to relevant files):

> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> index 0859bc3..6e6687c 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> @@ -138,6 +138,7 @@ [LibraryClasses.common.SEC]
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg.dsc
> index be31025..14d82f6 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc
> @@ -134,6 +134,8 @@ [LibraryClasses.common.SEC]
>
> DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
>
> DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLibBase.inf
>
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> diff --git a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc 
> b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> index 19de381..4b4867f 100644
> --- a/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> +++ b/ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> @@ -128,6 +128,8 @@ [LibraryClasses.common.SEC]
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>  !endif
>
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
>
> diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc 
> b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> index 84e2a99..8f7b5f1 100644
> --- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> +++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
> @@ -144,6 +144,8 @@ [LibraryClasses.common.SEC]
># Trustzone Support
>
> ArmTrustedMonitorLib|ArmPlatformPkg/Library/ArmTrustedMonitorLibNull/ArmTrustedMonitorLibNull.inf
>
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> +
>  [LibraryClasses.common.PEI_CORE]
>HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf

This git-diff output shows quickly that Ard resolved the library class
for SEC modules, a fact that was not visible otherwise from the patch
itself.

I recommend that all git users working with edk2 apply these settings --
they are *very* helpful for reviewers. Personally, I will make this a
requirement for patches that I'm expected (or asked) to review.

Thanks
Laszlo

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Update nasmb build rule

2015-06-09 Thread Andrew Fish

> On Jun 9, 2015, at 10:00 AM, Jordan Justen  wrote:
> 
> On 2015-06-09 02:38:02, Gao, Liming wrote:
>> Jordan:
>>  Yes. I agree .bin is better postfix for .nasmb. After we fully
>>  migrate .asm16 to .nasmb, I will update our reference platform to
>>  use .bin in their FDF Ffs Rules.
> 
> This sounds like an ok plan.
> 
> But, then we will have to leave the build-rule copy to a .com file,
> right? Because now we will have to be backward-compatible with the
> fact that .nasmb was making .com files for a few months. Sigh... :)
> 

Who is consuming the .com file? 

The [Masm16-Code-File] rule in 
https://svn.code.sf.net/p/edk2/code/trunk/edk2/BaseTools/Conf/build_rule.template
 

 does an ASMLINK/DLINK/otool depending on the tool chain. 
While the [Nasm-to-Binary-Code-File] just makes a binary. I assume this means 
the nasmb binary is the correct form. 

Ah so now I see 
https://svn.code.sf.net/p/edk2/code/trunk/edk2/Vlv2TbltDevicePkg/ 

[Rule.Common.SEC]
  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
PE32  PE32Align = 8   $(INF_OUTPUT)/$(MODULE_NAME).efi
RAW BIN   Align = 16  |.com
  }

[Rule.Common.SEC.BINARY]
  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
PE32  PE32Align = 8   |.efi
RAW BIN   Align = 16  |.com
  }

So the build_rule.template file can be per compiler but the rules in the FDF 
file are generic. So this is the reason that is hard to change. If we changed 
the build_rule.template file then the rules in platform FDF files would all 
break. So this ends up being a compatibility issue with trees based on the 
edk2. 

Thanks,

Andrew Fish

> Anyway, my suggestion was to make new .inf files for the nasm
> versions, and convert the platforms to the new .inf and update their
> .fdf files when changing to the nasm .inf.
> 
> Then, in the end you can delete the masm .inf files and .asm16 files.
> 
> -Jordan
> 
>> -Original Message-
>> From: Justen, Jordan L 
>> Sent: Tuesday, June 09, 2015 3:12 PM
>> To: Gao, Liming; edk2-devel@lists.sourceforge.net
>> Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule
>> 
>> On 2015-06-08 18:31:30, Gao, Liming wrote:
>>> Jordan:
>>>  I don't get any feedback from others. You also say I can go ahead.
>>>  So, I add this change.
>> 
>> Ok. You are right. It is true that I said: "I don't like the idea of 
>> creating the .com for .nasmb sources, but if you insist that it is required, 
>> then you can go ahead."
>> 
>> So, I guess you went ahead.
>> 
>> Can you at least agree that the use of the .com extension is confusing in 
>> this case, since .com files are 16-bit DOS executables? I think the only 
>> reason they appeared in EDK/EDK II is because masm16 would only output a 
>> .com file.
>> 
>> I don't think we need to make everything backward compatible. This is a new 
>> tool without the legacy masm baggage, so do we really need to go out of our 
>> way to bring the masm baggage along?
>> 
>> Really, wouldn't it be reasonable for a platform to make a 1-line change to 
>> their .fdf file if for no other reason than to make it obvious that a big 
>> change just happened to their SEC phase?
>> 
>> This project goes *way* out of it's way to try to be backward compatible. In 
>> my opinion, often too far.
>> 
>> Ok. Rant complete. Thanks for your time, :)
>> 
>> -Jordan
>> 
>>>  I add this support for compatibility. I want to update our core
>>>  module to use .nasmb, and platform can directly use the updated
>>>  core module without changes. Now, our most platforms use .com
>>>  postfix in their platform FDF Ffs Rule. With this patch, after we
>>>  update more core modules to use .nasmb instead .asm16, it will not
>>>  impact platforms.
>>> 
>>> Thanks
>>> Liming
>>> -Original Message-
>>> From: Justen, Jordan L
>>> Sent: Tuesday, June 9, 2015 12:56 AM
>>> To: Gao, Liming; edk2-devel@lists.sourceforge.net
>>> Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule
>>> 
>>> On 2014-11-21 10:16:47, Jordan Justen wrote:
 On 2014-11-20 22:12:42, Gao, Liming wrote:
> Jordan:
>  NASMB is replacement of ASM16.
 
 I see .nasmb as an alternative, but not a replacement. (.asm16 is 
 still supported by EDK II.)
 
 I do hope that long term it replaces all uses of .asm16 though. :)
 
>  Now, ASM16 has been used in the
>  different modules. When the module owners do this conversion, they
>  expect it is compatible and doesn't impact platform, because
>  module owner can't predict how many platform will be impacted. So,
>  I think the compatibility is the important point to persuade the
>  module owners to do this conversion.
 
 I would rather see .bin be used in this case, because it makes more 
 sense than .com. I guess .com was used because it was the output 
 from link16? But, .c

Re: [edk2] [Xen-devel] The size of memory is wrong inside of virtual machine(VM) when using OVMF

2015-06-09 Thread Wei Liu
On Mon, Jun 08, 2015 at 10:57:30PM +0200, Laszlo Ersek wrote:
[...]
> 
> The issue is real. I've been working on some patches today for this.
> I'll soon send an RFC series.
> 
> It's actually easy to *test* this scenario: just create a 128GB or so
> file, with "fallocate", on a sufficiently big filesystem. Then format it
> with mkswap and add it with swapon. I have strict memory overcommit
> settings on my laptop (/proc/sys/vm/overcommit_memory = 2), but with
> such a swap file, a >= 64GB guest can easily be started.
> 
> (I used my old spindle disk for the swap file. Since the guest is doing
> practically nothing, it doesn't cause the disk to thrash.)
> 

That is really neat trick. :-)

Only that Xen requires real ram to start a guest so I can't use it. :-/


Wei.

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 5/7] ArmPkg: cache detected revision in ArmGicArchLib

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 18:48, Laszlo Ersek  wrote:
> On 05/29/15 14:33, Ard Biesheuvel wrote:
>> Instead of inferring the GIC revision from the CPU id registers
>> and the presence/availability of the system register interface
>> upon each invocation, move the logic to a constructor and cache
>> the result.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 23 ++--
>>  ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 23 ++--
>>  ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf   |  3 +-
>>  3 files changed, 40 insertions(+), 9 deletions(-)
>>
>> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c 
>> b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
>> index 0e0fa3b9f33e..9853c7ba8566 100644
>> --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
>> +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
>> @@ -15,9 +15,11 @@
>>  #include 
>>  #include 
>>
>> -ARM_GIC_ARCH_REVISION
>> +STATIC ARM_GIC_ARCH_REVISIONmGicArchRevision;
>> +
>> +RETURN_STATUS
>>  EFIAPI
>> -ArmGicGetSupportedArchRevision (
>> +ArmGicArchLibInitialize (
>>VOID
>>)
>>  {
>> @@ -43,9 +45,22 @@ ArmGicGetSupportedArchRevision (
>>IccSre = ArmGicV3GetControlSystemRegisterEnable ();
>>  }
>>  if (IccSre & ICC_SRE_EL2_SRE) {
>> -  return ARM_GIC_ARCH_REVISION_3;
>> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
>> +  goto Done;
>>  }
>>}
>>
>> -  return ARM_GIC_ARCH_REVISION_2;
>> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
>> +
>> +Done:
>> +  return RETURN_SUCCESS;
>> +}
>> +
>> +ARM_GIC_ARCH_REVISION
>> +EFIAPI
>> +ArmGicGetSupportedArchRevision (
>> +  VOID
>> +  )
>> +{
>> +  return mGicArchRevision;
>>  }
>> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c 
>> b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
>> index f256de704631..f8822a224580 100644
>> --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
>> +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
>> @@ -15,9 +15,11 @@
>>  #include 
>>  #include 
>>
>> -ARM_GIC_ARCH_REVISION
>> +STATIC ARM_GIC_ARCH_REVISIONmGicArchRevision;
>> +
>> +RETURN_STATUS
>>  EFIAPI
>> -ArmGicGetSupportedArchRevision (
>> +ArmGicArchLibInitialize (
>>VOID
>>)
>>  {
>> @@ -43,9 +45,22 @@ ArmGicGetSupportedArchRevision (
>>IccSre = ArmGicV3GetControlSystemRegisterEnable ();
>>  }
>>  if (IccSre & ICC_SRE_EL2_SRE) {
>> -  return ARM_GIC_ARCH_REVISION_3;
>> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
>> +  goto Done;
>>  }
>>}
>>
>> -  return ARM_GIC_ARCH_REVISION_2;
>> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
>> +
>> +Done:
>> +  return RETURN_SUCCESS;
>> +}
>> +
>> +ARM_GIC_ARCH_REVISION
>> +EFIAPI
>> +ArmGicGetSupportedArchRevision (
>> +  VOID
>> +  )
>> +{
>> +  return mGicArchRevision;
>>  }
>> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf 
>> b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>> index d71b2adc3027..7dbcb08f50d6 100644
>> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>> +++ b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
>> @@ -17,7 +17,8 @@
>>FILE_GUID  = cd67f41a-26e9-4482-90c9-a9aff803382a
>>MODULE_TYPE= BASE
>>VERSION_STRING = 1.0
>> -  LIBRARY_CLASS  = ArmGicArchLib
>> +  LIBRARY_CLASS  = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER 
>> UEFI_APPLICATION
>> +  CONSTRUCTOR= ArmGicArchLibInitialize
>>
>>  [Sources.ARM]
>>Arm/ArmGicArchLib.c
>>
>
> I trust that you rebuilt all of the DSCs that use this (now restricted)
> library instance via their default library class resolutions, and there
> were no errors (ie. no other referring module types than spelled out
> here) :)
>
> Code-wise, it looks fine.
>
> Reviewed-by: Laszlo Ersek 
>
> I think I'm finished reviewing the still pending patches. At the end of
> this series, we have three instances for the library class:
>
> ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf:
> LIBRARY_CLASS = ArmGicArchLib|SEC
>
> ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf:
> LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION
>
> ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf:
> LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION
>
> The first one (without caching) is used for SEC modules in:
> - ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> - ArmPlatformPkg/ArmPlatformPkg.dsc
> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> - ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
>
> The second one (with caching in a static variable) is used as the
> default resolution in:
> - ArmPkg/ArmPkg.dsc
> - ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> - ArmPlatformPkg/ArmPlatformPkg.dsc
> - ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
> - ArmPlatformPkg/ArmVExpre

Re: [edk2] [Patch] BaseTools: Update nasmb build rule

2015-06-09 Thread Jordan Justen
On 2015-06-09 02:38:02, Gao, Liming wrote:
> Jordan:
>   Yes. I agree .bin is better postfix for .nasmb. After we fully
>   migrate .asm16 to .nasmb, I will update our reference platform to
>   use .bin in their FDF Ffs Rules.

This sounds like an ok plan.

But, then we will have to leave the build-rule copy to a .com file,
right? Because now we will have to be backward-compatible with the
fact that .nasmb was making .com files for a few months. Sigh... :)

Anyway, my suggestion was to make new .inf files for the nasm
versions, and convert the platforms to the new .inf and update their
.fdf files when changing to the nasm .inf.

Then, in the end you can delete the masm .inf files and .asm16 files.

-Jordan

> -Original Message-
> From: Justen, Jordan L 
> Sent: Tuesday, June 09, 2015 3:12 PM
> To: Gao, Liming; edk2-devel@lists.sourceforge.net
> Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule
> 
> On 2015-06-08 18:31:30, Gao, Liming wrote:
> > Jordan:
> >   I don't get any feedback from others. You also say I can go ahead.
> >   So, I add this change.
> 
> Ok. You are right. It is true that I said: "I don't like the idea of creating 
> the .com for .nasmb sources, but if you insist that it is required, then you 
> can go ahead."
> 
> So, I guess you went ahead.
> 
> Can you at least agree that the use of the .com extension is confusing in 
> this case, since .com files are 16-bit DOS executables? I think the only 
> reason they appeared in EDK/EDK II is because masm16 would only output a .com 
> file.
> 
> I don't think we need to make everything backward compatible. This is a new 
> tool without the legacy masm baggage, so do we really need to go out of our 
> way to bring the masm baggage along?
> 
> Really, wouldn't it be reasonable for a platform to make a 1-line change to 
> their .fdf file if for no other reason than to make it obvious that a big 
> change just happened to their SEC phase?
> 
> This project goes *way* out of it's way to try to be backward compatible. In 
> my opinion, often too far.
> 
> Ok. Rant complete. Thanks for your time, :)
> 
> -Jordan
> 
> >   I add this support for compatibility. I want to update our core
> >   module to use .nasmb, and platform can directly use the updated
> >   core module without changes. Now, our most platforms use .com
> >   postfix in their platform FDF Ffs Rule. With this patch, after we
> >   update more core modules to use .nasmb instead .asm16, it will not
> >   impact platforms.
> > 
> > Thanks
> > Liming
> > -Original Message-
> > From: Justen, Jordan L
> > Sent: Tuesday, June 9, 2015 12:56 AM
> > To: Gao, Liming; edk2-devel@lists.sourceforge.net
> > Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule
> > 
> > On 2014-11-21 10:16:47, Jordan Justen wrote:
> > > On 2014-11-20 22:12:42, Gao, Liming wrote:
> > > > Jordan:
> > > >   NASMB is replacement of ASM16.
> > > 
> > > I see .nasmb as an alternative, but not a replacement. (.asm16 is 
> > > still supported by EDK II.)
> > > 
> > > I do hope that long term it replaces all uses of .asm16 though. :)
> > > 
> > > >   Now, ASM16 has been used in the
> > > >   different modules. When the module owners do this conversion, they
> > > >   expect it is compatible and doesn't impact platform, because
> > > >   module owner can't predict how many platform will be impacted. So,
> > > >   I think the compatibility is the important point to persuade the
> > > >   module owners to do this conversion.
> > > 
> > > I would rather see .bin be used in this case, because it makes more 
> > > sense than .com. I guess .com was used because it was the output 
> > > from link16? But, .com files are known as old dos based executables. 
> > > It is a confusing use of the extension.
> > 
> > I see you went ahead and committed this without further discussion.
> > 
> > I also see that you converted an internal platform's fdf from being 
> > able to accept either .bin or .com to only accept .com. So, apparently 
> > you have real preference for using .com rather than .bin.
> > 
> > Can you explain why you think .com makes better sense than .bin in 
> > this context?
> > 
> > http://en.wikipedia.org/wiki/COM_file
> > 
> > -Jordan
> > 
> > > If we copy the file like in your patch, then few platforms will 
> > > actually change from using .com to .bin, but I think it would be an 
> > > improvement for them to convert to using the .bin extension.
> > > 
> > > Maybe another idea is to leave the .asm16 .inf in place, and create 
> > > a new .inf with the .nasmb source. Then the platform will have to 
> > > change both the .dsc and .fdf to make use of NASM. This will also 
> > > leave the platform with the option to continue using MASM16/LINK16.
> > > 
> > > I don't like the idea of creating the .com for .nasmb sources, but 
> > > if you insist that it is required, then you can go ahead. I will not 
> > > give my r-b for the change though. :)
> > > 
> > > -Jordan
> > > 
> > > > -Origina

Re: [edk2] [PATCH 5/7] ArmPkg: cache detected revision in ArmGicArchLib

2015-06-09 Thread Laszlo Ersek
On 05/29/15 14:33, Ard Biesheuvel wrote:
> Instead of inferring the GIC revision from the CPU id registers
> and the presence/availability of the system register interface
> upon each invocation, move the logic to a constructor and cache
> the result.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c | 23 ++--
>  ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c | 23 ++--
>  ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf   |  3 +-
>  3 files changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c 
> b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> index 0e0fa3b9f33e..9853c7ba8566 100644
> --- a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> +++ b/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> @@ -15,9 +15,11 @@
>  #include 
>  #include 
>  
> -ARM_GIC_ARCH_REVISION
> +STATIC ARM_GIC_ARCH_REVISIONmGicArchRevision;
> +
> +RETURN_STATUS
>  EFIAPI
> -ArmGicGetSupportedArchRevision (
> +ArmGicArchLibInitialize (
>VOID
>)
>  {
> @@ -43,9 +45,22 @@ ArmGicGetSupportedArchRevision (
>IccSre = ArmGicV3GetControlSystemRegisterEnable ();
>  }
>  if (IccSre & ICC_SRE_EL2_SRE) {
> -  return ARM_GIC_ARCH_REVISION_3;
> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
> +  goto Done;
>  }
>}
>  
> -  return ARM_GIC_ARCH_REVISION_2;
> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
> +
> +Done:
> +  return RETURN_SUCCESS;
> +}
> +
> +ARM_GIC_ARCH_REVISION
> +EFIAPI
> +ArmGicGetSupportedArchRevision (
> +  VOID
> +  )
> +{
> +  return mGicArchRevision;
>  }
> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c 
> b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> index f256de704631..f8822a224580 100644
> --- a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> +++ b/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> @@ -15,9 +15,11 @@
>  #include 
>  #include 
>  
> -ARM_GIC_ARCH_REVISION
> +STATIC ARM_GIC_ARCH_REVISIONmGicArchRevision;
> +
> +RETURN_STATUS
>  EFIAPI
> -ArmGicGetSupportedArchRevision (
> +ArmGicArchLibInitialize (
>VOID
>)
>  {
> @@ -43,9 +45,22 @@ ArmGicGetSupportedArchRevision (
>IccSre = ArmGicV3GetControlSystemRegisterEnable ();
>  }
>  if (IccSre & ICC_SRE_EL2_SRE) {
> -  return ARM_GIC_ARCH_REVISION_3;
> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_3;
> +  goto Done;
>  }
>}
>  
> -  return ARM_GIC_ARCH_REVISION_2;
> +  mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
> +
> +Done:
> +  return RETURN_SUCCESS;
> +}
> +
> +ARM_GIC_ARCH_REVISION
> +EFIAPI
> +ArmGicGetSupportedArchRevision (
> +  VOID
> +  )
> +{
> +  return mGicArchRevision;
>  }
> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf 
> b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> index d71b2adc3027..7dbcb08f50d6 100644
> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +++ b/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> @@ -17,7 +17,8 @@
>FILE_GUID  = cd67f41a-26e9-4482-90c9-a9aff803382a
>MODULE_TYPE= BASE
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = ArmGicArchLib
> +  LIBRARY_CLASS  = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER 
> UEFI_APPLICATION
> +  CONSTRUCTOR= ArmGicArchLibInitialize
>  
>  [Sources.ARM]
>Arm/ArmGicArchLib.c
> 

I trust that you rebuilt all of the DSCs that use this (now restricted)
library instance via their default library class resolutions, and there
were no errors (ie. no other referring module types than spelled out
here) :)

Code-wise, it looks fine.

Reviewed-by: Laszlo Ersek 

I think I'm finished reviewing the still pending patches. At the end of
this series, we have three instances for the library class:

ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf:
LIBRARY_CLASS = ArmGicArchLib|SEC

ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf:
LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION

ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf:
LIBRARY_CLASS = ArmGicArchLib|DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION

The first one (without caching) is used for SEC modules in:
- ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
- ArmPlatformPkg/ArmPlatformPkg.dsc
- ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
- ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc

The second one (with caching in a static variable) is used as the
default resolution in:
- ArmPkg/ArmPkg.dsc
- ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
- ArmPlatformPkg/ArmPlatformPkg.dsc
- ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
- ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc

The third one (also with caching in a static variable, but taken from a
PCD, not probed from hardware) is used as the default resolution in:
- ArmVirtPkg/ArmVirt.dsc.inc

Looks good!

Tha

Re: [edk2] [PATCH 4/7] ArmPkg: copy ArmGicArchLib to ArmGicArchSecLib

2015-06-09 Thread Laszlo Ersek
On 05/29/15 14:33, Ard Biesheuvel wrote:
> Clone ArmGicArchLib into a SEC phase specific ArmGicArchSecLib
> so that we can modify the former in a subsequent patch to cache
> the GIC revision in a global variable.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/AArch64/ArmGicArchLib.c   
>  | 0
>  ArmPkg/Library/{ArmGicArchLib => ArmGicArchSecLib}/Arm/ArmGicArchLib.c   
>  | 0
>  ArmPkg/Library/{ArmGicArchLib/ArmGicArchLib.inf => 
> ArmGicArchSecLib/ArmGicArchSecLib.inf} | 6 +++---
>  ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc   
>  | 1 +
>  ArmPlatformPkg/ArmPlatformPkg.dsc
>  | 2 ++
>  ArmPlatformPkg/ArmRealViewEbPkg/ArmRealViewEb.dsc.inc
>  | 2 ++
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress.dsc.inc
>  | 2 ++
>  7 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c 
> b/ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
> similarity index 100%
> copy from ArmPkg/Library/ArmGicArchLib/AArch64/ArmGicArchLib.c
> copy to ArmPkg/Library/ArmGicArchSecLib/AArch64/ArmGicArchLib.c
> diff --git a/ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c 
> b/ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> similarity index 100%
> copy from ArmPkg/Library/ArmGicArchLib/Arm/ArmGicArchLib.c
> copy to ArmPkg/Library/ArmGicArchSecLib/Arm/ArmGicArchLib.c
> diff --git a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf 
> b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> similarity index 79%
> copy from ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> copy to ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> index d71b2adc3027..a2fb623a8537 100644
> --- a/ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
> +++ b/ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
> @@ -13,11 +13,11 @@
>  
>  [Defines]
>INF_VERSION= 0x00010005
> -  BASE_NAME  = ArmGicArchLib
> -  FILE_GUID  = cd67f41a-26e9-4482-90c9-a9aff803382a
> +  BASE_NAME  = ArmGicArchSecLib
> +  FILE_GUID  = c1dd9745-9459-4e9a-9f5b-99cbd233c27d
>MODULE_TYPE= BASE
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = ArmGicArchLib
> +  LIBRARY_CLASS  = ArmGicArchLib|SEC

* Okay, so this patch does the clone for SEC modules and points the SEC
library class resolutions in the DSC files to the clone, overriding the
default resolutions in them.

ArmPkg/ArmPkg.dsc and ArmVirtPkg/ArmVirt.dsc.inc are not updated. For
ArmVirtPkg, the default resolution stays intact in this patch, which
makes sense, because patch #7 handles it separately.

But, is there any particular reason to leave ArmPkg/ArmPkg.dsc unchanged
in this patch? Hm... I guess it should be safe, because patch #5 is
explicit about DXE_DRIVER, UEFI_DRIVER, UEFI_APPLICATION; so if there's
a problem later, it will show up as a build time failure. Okay.

* Anyway, my main concern was PEIMs. Especially because in patch #3 you
wrote,

> this statelessness is only needed for SEC type modules

This is in general not true, because PEIMs also may execute-in-place
from flash. However, from patch #5 I can see that the caching version
will be available only to DXE_DRIVER UEFI_DRIVER UEFI_APPLICATION,
leaving PEIMs uncovered -- which is safe, and also good enough since
(apparently) no PEIMs depend on this library class.

If you have to respin the series for any reason, please consider
mentioning PEIMs in the commit message of patch #3 and patch #4, and
making ArmGicArchSecLib available to PEIMs too. However, as I said
above, that would be quite speculative, and this could be addressed any
time later, if a new PEIM actually needed the library. So feel free to
ignore this note.

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

>  
>  [Sources.ARM]
>Arm/ArmGicArchLib.c
> diff --git a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> index 0859bc31ff00..6e6687c8a735 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> +++ b/ArmPlatformPkg/ArmPlatformPkg-2ndstage.dsc
> @@ -138,6 +138,7 @@
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
>PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +  ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>  
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>MemoryInitPeiLib|ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.inf
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc 
> b/ArmPlatformPkg/ArmPlatformPkg.dsc
> index be31025d9

Re: [edk2] [PATCH v5 2/2] OvmfPkg/PlatformPei: Initialise RCBA (B0:D31:F0 0xf0) register

2015-06-09 Thread Jordan Justen
Series Reviewed-by: Jordan Justen 
and committed. Thanks for the contribution!

On 2015-06-08 17:16:14, Paulo Alcantara wrote:
> This patch initialises root complex register block BAR in order to
> support TCO watchdog emulation features (e.g. reboot upon NO_REBOOT bit
> not set) on QEMU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Paulo Alcantara 
> ---
>  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h |  5 +
>  OvmfPkg/PlatformPei/Platform.c| 17 -
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h 
> b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> index 4f59a7c..18b34a3 100644
> --- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> +++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> @@ -77,6 +77,9 @@
>  #define ICH9_GEN_PMCON_1  0xA0
>  #define ICH9_GEN_PMCON_1_SMI_LOCK   BIT4
>  
> +#define ICH9_RCBA 0xF0
> +#define ICH9_RCBA_ENBIT0
> +
>  //
>  // IO ports
>  //
> @@ -90,4 +93,6 @@
>  #define ICH9_SMI_EN_APMC_EN  BIT5
>  #define ICH9_SMI_EN_GBL_SMI_EN   BIT0
>  
> +#define ICH9_ROOT_COMPLEX_BASE 0xFED1C000
> +
>  #endif
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 0e41d30..1ad5bfc 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -214,13 +214,18 @@ MemMapInitialization (
>  // 0xFEC0IO-APIC4 KB
>  // 0xFEC01000gap 1020 KB
>  // 0xFED0HPET   1 KB
> -// 0xFED00400gap 1023 KB
> +// 0xFED00400gap  111 KB
> +// 0xFED1C000gap (PIIX4) / RCRB (ICH9) 16 KB
> +// 0xFED2gap  896 KB
>  // 0xFEE0LAPIC  1 MB
>  //
>  AddIoMemoryRangeHob (TopOfLowRam < BASE_2GB ?
>   BASE_2GB : TopOfLowRam, 0xFC00);
>  AddIoMemoryBaseSizeHob (0xFEC0, SIZE_4KB);
>  AddIoMemoryBaseSizeHob (0xFED0, SIZE_1KB);
> +if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> +  AddIoMemoryBaseSizeHob (ICH9_ROOT_COMPLEX_BASE, SIZE_16KB);
> +}
>  AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);
>}
>  }
> @@ -292,6 +297,16 @@ MiscInitialization (
>  //
>  PciOr8 (AcpiCtlReg, AcpiEnBit);
>}
> +
> +  if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> +//
> +// Set Root Complex Register Block BAR
> +//
> +PciWrite32 (
> +  POWER_MGMT_REGISTER_Q35 (ICH9_RCBA),
> +  ICH9_ROOT_COMPLEX_BASE | ICH9_RCBA_EN
> +  );
> +  }
>  }
>  
>  
> -- 
> 2.1.0
> 

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [PATCH 2/7] ArmPkg: merge ArmGicV[23]Lib.h into ArmGicLib.h

2015-06-09 Thread Laszlo Ersek
On 05/29/15 14:33, Ard Biesheuvel wrote:
> Before splitting off ArmGicArchLib and moving it out of
> ArmPkg/Drivers/ArmGic into ArmPkg/Library, make sure that the
> GIC specific declarations it depends on are not hidden away in
> local headers "GicV2/GicV2Lib.h" and "GicV3/GicV3Lib.h".
> 
> So merge them with . This is entirely
> appropriate, since this is not a header that declares a public
> interface into ArmGicLib, but defines implementation internals.

The code movements in this patch seem okay to me, but I'm unsure about
your claim "Library/ArmGicLib.h [...] is not a header that declares a
public interface into ArmGicLib".

For example, take the function ArmGicSetupNonSecure(). This function is
declared in "ArmPkg/Include/Library/ArmGicLib.h", which one could say is
the library class itself (and your claim is the opposite).

The function is called in "ArmPlatformPkg/Sec/Sec.c" (the library client).

The funcion is implemented in "ArmPkg/Drivers/ArmGic/ArmGicSecLib.c",
which is built as part of "ArmPkg/Drivers/ArmGic/ArmGicSecLib.inf". That
INF file has LIBRARY_CLASS = ArmGicLib.

This kinda looks to counter your claim.

... Hm... on the other hand. Functions in a library class header must be
implemented by all library instances. And, we have another ArmGicLib
instance under

  ArmPkg/Drivers/ArmGic/ArmGicLib.inf

that does not build "ArmGicSecLib.c", therefore it will lack an
implementation for ArmGicSetupNonSecure().

In addition, [LibraryClasses.common] in "ArmPkg/ArmPkg.dec" does not
list the header file.

So I guess you're right, "ArmPkg/Include/Library/ArmGicLib.h" is already
a "dumping ground" for declarations of internal-use functions.

Reviewed-by: Laszlo Ersek 

Laszlo

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c |  2 -
>  ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c |  2 -
>  ArmPkg/Drivers/ArmGic/ArmGicLib.c |  3 -
>  ArmPkg/Drivers/ArmGic/ArmGicSecLib.c  |  2 -
>  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c |  3 +-
>  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.h | 54 ---
>  ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c |  3 +-
>  ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Lib.h | 68 
>  ArmPkg/Include/Library/ArmGicLib.h| 94 
>  9 files changed, 98 insertions(+), 133 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c 
> b/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c
> index 88fa4621e613..0e0fa3b9f33e 100644
> --- a/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c
> +++ b/ArmPkg/Drivers/ArmGic/AArch64/ArmGicArchLib.c
> @@ -15,8 +15,6 @@
>  #include 
>  #include 
>  
> -#include "GicV3/ArmGicV3Lib.h"
> -
>  ARM_GIC_ARCH_REVISION
>  EFIAPI
>  ArmGicGetSupportedArchRevision (
> diff --git a/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c 
> b/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c
> index 9ef56efeaa7b..f256de704631 100644
> --- a/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c
> +++ b/ArmPkg/Drivers/ArmGic/Arm/ArmGicArchLib.c
> @@ -15,8 +15,6 @@
>  #include 
>  #include 
>  
> -#include "GicV3/ArmGicV3Lib.h"
> -
>  ARM_GIC_ARCH_REVISION
>  EFIAPI
>  ArmGicGetSupportedArchRevision (
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c 
> b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> index 48708e3812b4..248e896c4b94 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> @@ -19,9 +19,6 @@
>  #include 
>  #include 
>  
> -#include "GicV2/ArmGicV2Lib.h"
> -#include "GicV3/ArmGicV3Lib.h"
> -
>  /**
>   * Return the base address of the GIC redistributor for the current CPU
>   *
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicSecLib.c 
> b/ArmPkg/Drivers/ArmGic/ArmGicSecLib.c
> index 1fdd4d73bd7d..d64806d2f16b 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicSecLib.c
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicSecLib.c
> @@ -17,8 +17,6 @@
>  #include 
>  #include 
>  
> -#include "GicV2/ArmGicV2Lib.h"
> -
>  /*
>   * This function configures the interrupts set by the mask to be secure.
>   *
> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c 
> b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> index 743c534e04d9..e649ac1bc6af 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> @@ -22,8 +22,9 @@ Abstract:
>  
>  --*/
>  
> +#include 
> +
>  #include "ArmGicDxe.h"
> -#include "GicV2/ArmGicV2Lib.h"
>  
>  #define ARM_GIC_DEFAULT_PRIORITY  0x80
>  
> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.h 
> b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.h
> deleted file mode 100644
> index 6803467346a3..
> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.h
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -/** @file
> -*
> -*  Copyright (c) 2013-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 f

Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 15:55, Ard Biesheuvel  wrote:
> On 9 June 2015 at 15:34, Yao, Jiewen  wrote:
>> Thank you! Agree, I have similar concern on 
>> PeCoffLoaderGetPeHeaderMagicValue().
>>
>> I have seen duplicated code in below modules:
>> 1) 
>> SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(363):  
>> if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
>> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> 2) 
>> SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(1690): 
>>  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
>> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> 3) SecurityPkg\Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c(419):  if 
>> (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
>> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> 4) SecurityPkg\Tcg\TrEEDxe\MeasureBootPeCoff.c(114):  if 
>> (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
>> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> 5) 
>> SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigImpl.c(1734):
>>   if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
>> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>
>> How I wish it is exposed!
>>
>
> Actually, I do think this is somewhat of a concern, considering that
> is the SecurityPkg. Having these kinds of exceptions on all
> architectures, and in various places in the code, just because one
> architecture needs it is not very elegant, especially since only IA64
> builds are ever expected to try and load IA64 images in the first
> place.
>
> Is there any way we could export PeCoffLoaderGetPeHeaderMagicValue ()
> from BasePecoffLib, and special case the implementation so the hack
> only gets included on IPF builds? Or will we violate some spec by
> doing that?
>

... or perhaps get rid of the special case entirely? ELILO is dead,
and I doubt anyone using IA64 in production using an affected version
of ELILO would be interested in running the latest Tianocore.

-- 
Ard.


>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Tuesday, June 09, 2015 9:22 PM
>> To: Yao, Jiewen
>> Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>
>> On 9 June 2015 at 14:42, Yao, Jiewen  wrote:
>>> Hi Ard
>>> You are right. I happen to find a logic error, too.
>>>
>>> Attach patch for your review.
>>>
>>
>> Yes, that looks fine to me, although it is unfortunate that can't just use 
>> PeCoffLoaderGetPeHeaderMagicValue () directly.
>> It builds fine on GCC/AArch64
>>
>> Reviewed-by: Ard Biesheuvel 
>>
>> Thanks,
>> Ard.
>>
>>
>>
>>>
>>>
>>> -Original Message-
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: Tuesday, June 09, 2015 8:28 PM
>>> To: Yao, Jiewen
>>> Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
>>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support,
>>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>>
>>> On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
 On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
> Hi Ard
> Thanks to report this. I have fixed this in 17565.

 Wow, that was quick! Thanks!

>>>
>>> You have fixed the build, but the copy-pasted comments are still there, 
>>> which make no sense at all anymore, since they refer to the Magic variable, 
>>> which has been removed.
>>>
>>> Next time, could you please send your patches (including fixes like
>>> these) to the mailing list first? That way, people have a chance to see 
>>> what's going in before its gets merged.
>>>
>>> Thanks,
>>> Ard.
>>>
>>>

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Friday, June 05, 2015 9:12 PM
> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao,
> Liming
> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support,
> MdeModulePkg/DxeCore support UEFI2.5 properties table
>
> On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
>> Hi
>>
>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties
>> Table feature.
>>
>>
>>
>> MdePkg – add Properties table definition.
>>
>> MdeModulePkg – add properties table support in DXE core.
>>
>> MdeModulePkg – add PropertiesTableAttributesDxe driver to set
>> ACPINvs/Reserved memory type to be XP.
>>
>>
>>
>> Signed-off-by: Yao, Jiewen jiewen@intel.com
>>
>> Reviewed-by: Zeng, Star star.z...@intel.com
>>
>> Reviewed-by: Gao, Liming liming@intel.com
>>
>
> This patch breaks the build for GCC on 64-bit ARM
>
> 13:03:43

Re: [edk2] [PATCH v5 2/2] OvmfPkg/PlatformPei: Initialise RCBA (B0:D31:F0 0xf0) register

2015-06-09 Thread Laszlo Ersek
On 06/09/15 02:16, Paulo Alcantara wrote:
> This patch initialises root complex register block BAR in order to
> support TCO watchdog emulation features (e.g. reboot upon NO_REBOOT bit
> not set) on QEMU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Paulo Alcantara 
> ---
>  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h |  5 +
>  OvmfPkg/PlatformPei/Platform.c| 17 -
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h 
> b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> index 4f59a7c..18b34a3 100644
> --- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> +++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> @@ -77,6 +77,9 @@
>  #define ICH9_GEN_PMCON_1  0xA0
>  #define ICH9_GEN_PMCON_1_SMI_LOCK   BIT4
>  
> +#define ICH9_RCBA 0xF0
> +#define ICH9_RCBA_ENBIT0
> +
>  //
>  // IO ports
>  //
> @@ -90,4 +93,6 @@
>  #define ICH9_SMI_EN_APMC_EN  BIT5
>  #define ICH9_SMI_EN_GBL_SMI_EN   BIT0
>  
> +#define ICH9_ROOT_COMPLEX_BASE 0xFED1C000
> +
>  #endif
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 0e41d30..1ad5bfc 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -214,13 +214,18 @@ MemMapInitialization (
>  // 0xFEC0IO-APIC4 KB
>  // 0xFEC01000gap 1020 KB
>  // 0xFED0HPET   1 KB
> -// 0xFED00400gap 1023 KB
> +// 0xFED00400gap  111 KB
> +// 0xFED1C000gap (PIIX4) / RCRB (ICH9) 16 KB
> +// 0xFED2gap  896 KB
>  // 0xFEE0LAPIC  1 MB
>  //
>  AddIoMemoryRangeHob (TopOfLowRam < BASE_2GB ?
>   BASE_2GB : TopOfLowRam, 0xFC00);
>  AddIoMemoryBaseSizeHob (0xFEC0, SIZE_4KB);
>  AddIoMemoryBaseSizeHob (0xFED0, SIZE_1KB);
> +if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> +  AddIoMemoryBaseSizeHob (ICH9_ROOT_COMPLEX_BASE, SIZE_16KB);
> +}
>  AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);
>}
>  }
> @@ -292,6 +297,16 @@ MiscInitialization (
>  //
>  PciOr8 (AcpiCtlReg, AcpiEnBit);
>}
> +
> +  if (mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> +//
> +// Set Root Complex Register Block BAR
> +//
> +PciWrite32 (
> +  POWER_MGMT_REGISTER_Q35 (ICH9_RCBA),
> +  ICH9_ROOT_COMPLEX_BASE | ICH9_RCBA_EN
> +  );
> +  }
>  }
>  
>  
> 

series
Reviewed-by: Laszlo Ersek 

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 15:34, Yao, Jiewen  wrote:
> Thank you! Agree, I have similar concern on 
> PeCoffLoaderGetPeHeaderMagicValue().
>
> I have seen duplicated code in below modules:
> 1) 
> SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(363):  
> if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> 2) 
> SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(1690):  
> if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> 3) SecurityPkg\Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c(419):  if 
> (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> 4) SecurityPkg\Tcg\TrEEDxe\MeasureBootPeCoff.c(114):  if 
> (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> 5) 
> SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigImpl.c(1734):
>   if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>
> How I wish it is exposed!
>

Actually, I do think this is somewhat of a concern, considering that
is the SecurityPkg. Having these kinds of exceptions on all
architectures, and in various places in the code, just because one
architecture needs it is not very elegant, especially since only IA64
builds are ever expected to try and load IA64 images in the first
place.

Is there any way we could export PeCoffLoaderGetPeHeaderMagicValue ()
from BasePecoffLib, and special case the implementation so the hack
only gets included on IPF builds? Or will we violate some spec by
doing that?

Regards,
Ard.




> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, June 09, 2015 9:22 PM
> To: Yao, Jiewen
> Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
> MdeModulePkg/DxeCore support UEFI2.5 properties table
>
> On 9 June 2015 at 14:42, Yao, Jiewen  wrote:
>> Hi Ard
>> You are right. I happen to find a logic error, too.
>>
>> Attach patch for your review.
>>
>
> Yes, that looks fine to me, although it is unfortunate that can't just use 
> PeCoffLoaderGetPeHeaderMagicValue () directly.
> It builds fine on GCC/AArch64
>
> Reviewed-by: Ard Biesheuvel 
>
> Thanks,
> Ard.
>
>
>
>>
>>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Tuesday, June 09, 2015 8:28 PM
>> To: Yao, Jiewen
>> Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support,
>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>
>> On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
>>> On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
 Hi Ard
 Thanks to report this. I have fixed this in 17565.
>>>
>>> Wow, that was quick! Thanks!
>>>
>>
>> You have fixed the build, but the copy-pasted comments are still there, 
>> which make no sense at all anymore, since they refer to the Magic variable, 
>> which has been removed.
>>
>> Next time, could you please send your patches (including fixes like
>> these) to the mailing list first? That way, people have a chance to see 
>> what's going in before its gets merged.
>>
>> Thanks,
>> Ard.
>>
>>
>>>
 -Original Message-
 From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
 Sent: Friday, June 05, 2015 9:12 PM
 To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao,
 Liming
 Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support,
 MdeModulePkg/DxeCore support UEFI2.5 properties table

 On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
> Hi
>
> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties
> Table feature.
>
>
>
> MdePkg – add Properties table definition.
>
> MdeModulePkg – add properties table support in DXE core.
>
> MdeModulePkg – add PropertiesTableAttributesDxe driver to set
> ACPINvs/Reserved memory type to be XP.
>
>
>
> Signed-off-by: Yao, Jiewen jiewen@intel.com
>
> Reviewed-by: Zeng, Star star.z...@intel.com
>
> Reviewed-by: Gao, Liming liming@intel.com
>

 This patch breaks the build for GCC on 64-bit ARM

 13:03:43 
 /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:
 In function ‘InsertImageRecord’:
 13:03:43 
 /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10:
 error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable]
 13:03

Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Yao, Jiewen
Thank you! Agree, I have similar concern on PeCoffLoaderGetPeHeaderMagicValue().

I have seen duplicated code in below modules:
1) SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(363):  
if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
2) SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(1690): 
 if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
3) SecurityPkg\Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c(419):  if 
(Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
4) SecurityPkg\Tcg\TrEEDxe\MeasureBootPeCoff.c(114):  if 
(Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
5) 
SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigImpl.c(1734):
  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {

How I wish it is exposed!

Thank you
Yao Jiewen

-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Tuesday, June 09, 2015 9:22 PM
To: Yao, Jiewen
Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
MdeModulePkg/DxeCore support UEFI2.5 properties table

On 9 June 2015 at 14:42, Yao, Jiewen  wrote:
> Hi Ard
> You are right. I happen to find a logic error, too.
>
> Attach patch for your review.
>

Yes, that looks fine to me, although it is unfortunate that can't just use 
PeCoffLoaderGetPeHeaderMagicValue () directly.
It builds fine on GCC/AArch64

Reviewed-by: Ard Biesheuvel 

Thanks,
Ard.



>
>
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, June 09, 2015 8:28 PM
> To: Yao, Jiewen
> Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
> MdeModulePkg/DxeCore support UEFI2.5 properties table
>
> On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
>> On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
>>> Hi Ard
>>> Thanks to report this. I have fixed this in 17565.
>>
>> Wow, that was quick! Thanks!
>>
>
> You have fixed the build, but the copy-pasted comments are still there, which 
> make no sense at all anymore, since they refer to the Magic variable, which 
> has been removed.
>
> Next time, could you please send your patches (including fixes like
> these) to the mailing list first? That way, people have a chance to see 
> what's going in before its gets merged.
>
> Thanks,
> Ard.
>
>
>>
>>> -Original Message-
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: Friday, June 05, 2015 9:12 PM
>>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao, 
>>> Liming
>>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
>>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>>
>>> On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
 Hi

 Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties 
 Table feature.



 MdePkg – add Properties table definition.

 MdeModulePkg – add properties table support in DXE core.

 MdeModulePkg – add PropertiesTableAttributesDxe driver to set 
 ACPINvs/Reserved memory type to be XP.



 Signed-off-by: Yao, Jiewen jiewen@intel.com

 Reviewed-by: Zeng, Star star.z...@intel.com

 Reviewed-by: Gao, Liming liming@intel.com

>>>
>>> This patch breaks the build for GCC on 64-bit ARM
>>>
>>> 13:03:43 
>>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:
>>> In function ‘InsertImageRecord’:
>>> 13:03:43 
>>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10:
>>> error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable]
>>> 13:03:43UINT16   Magic;
>>> 13:03:43   ^
>>> 13:03:43 
>>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1110:10:
>>> error: variable ‘ImageType’ set but not used 
>>> [-Werror=unused-but-set-variable]
>>> 13:03:43UINT16   ImageType;
>>> 13:03:43   ^
>>>
>>>
>>> and honestly, i can't be bothered to look at the code myself, considering 
>>> the way it was dumped into the mailing list and the repository.
>>>
>>> Please get this fixed
>>>
>>> Regards,
>>> Ard.
>>>
>>>

 ---
 -
 --
 

 _

Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Ard Biesheuvel
On 9 June 2015 at 14:42, Yao, Jiewen  wrote:
> Hi Ard
> You are right. I happen to find a logic error, too.
>
> Attach patch for your review.
>

Yes, that looks fine to me, although it is unfortunate that can't just
use PeCoffLoaderGetPeHeaderMagicValue () directly.
It builds fine on GCC/AArch64

Reviewed-by: Ard Biesheuvel 

Thanks,
Ard.



>
>
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, June 09, 2015 8:28 PM
> To: Yao, Jiewen
> Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
> MdeModulePkg/DxeCore support UEFI2.5 properties table
>
> On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
>> On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
>>> Hi Ard
>>> Thanks to report this. I have fixed this in 17565.
>>
>> Wow, that was quick! Thanks!
>>
>
> You have fixed the build, but the copy-pasted comments are still there, which 
> make no sense at all anymore, since they refer to the Magic variable, which 
> has been removed.
>
> Next time, could you please send your patches (including fixes like
> these) to the mailing list first? That way, people have a chance to see 
> what's going in before its gets merged.
>
> Thanks,
> Ard.
>
>
>>
>>> -Original Message-
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: Friday, June 05, 2015 9:12 PM
>>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao,
>>> Liming
>>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support,
>>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>>
>>> On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
 Hi

 Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties
 Table feature.



 MdePkg – add Properties table definition.

 MdeModulePkg – add properties table support in DXE core.

 MdeModulePkg – add PropertiesTableAttributesDxe driver to set
 ACPINvs/Reserved memory type to be XP.



 Signed-off-by: Yao, Jiewen jiewen@intel.com

 Reviewed-by: Zeng, Star star.z...@intel.com

 Reviewed-by: Gao, Liming liming@intel.com

>>>
>>> This patch breaks the build for GCC on 64-bit ARM
>>>
>>> 13:03:43 
>>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:
>>> In function ‘InsertImageRecord’:
>>> 13:03:43 
>>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10:
>>> error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable]
>>> 13:03:43UINT16   Magic;
>>> 13:03:43   ^
>>> 13:03:43 
>>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1110:10:
>>> error: variable ‘ImageType’ set but not used 
>>> [-Werror=unused-but-set-variable]
>>> 13:03:43UINT16   ImageType;
>>> 13:03:43   ^
>>>
>>>
>>> and honestly, i can't be bothered to look at the code myself, considering 
>>> the way it was dumped into the mailing list and the repository.
>>>
>>> Please get this fixed
>>>
>>> Regards,
>>> Ard.
>>>
>>>

 
 --
 

 ___
 edk2-devel mailing list
 edk2-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/edk2-devel


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Yao, Jiewen
The correct log should be:
  - The SectionAlignment is got from Magic number.
  - The Magic number is got from PE header and machine type.

The original code mix them.

Thank you
Yao Jiewen

-Original Message-
From: Yao, Jiewen 
Sent: Tuesday, June 09, 2015 8:42 PM
To: 'Ard Biesheuvel'
Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
Subject: RE: [edk2] [patch] MdePkg/PropertiesTable support, 
MdeModulePkg/DxeCore support UEFI2.5 properties table

Hi Ard
You are right. I happen to find a logic error, too.

Attach patch for your review.



-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Tuesday, June 09, 2015 8:28 PM
To: Yao, Jiewen
Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
MdeModulePkg/DxeCore support UEFI2.5 properties table

On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
> On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
>> Hi Ard
>> Thanks to report this. I have fixed this in 17565.
>
> Wow, that was quick! Thanks!
>

You have fixed the build, but the copy-pasted comments are still there, which 
make no sense at all anymore, since they refer to the Magic variable, which has 
been removed.

Next time, could you please send your patches (including fixes like
these) to the mailing list first? That way, people have a chance to see what's 
going in before its gets merged.

Thanks,
Ard.


>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Friday, June 05, 2015 9:12 PM
>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao, 
>> Liming
>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>
>> On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
>>> Hi
>>>
>>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties 
>>> Table feature.
>>>
>>>
>>>
>>> MdePkg – add Properties table definition.
>>>
>>> MdeModulePkg – add properties table support in DXE core.
>>>
>>> MdeModulePkg – add PropertiesTableAttributesDxe driver to set 
>>> ACPINvs/Reserved memory type to be XP.
>>>
>>>
>>>
>>> Signed-off-by: Yao, Jiewen jiewen@intel.com
>>>
>>> Reviewed-by: Zeng, Star star.z...@intel.com
>>>
>>> Reviewed-by: Gao, Liming liming@intel.com
>>>
>>
>> This patch breaks the build for GCC on 64-bit ARM
>>
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:
>> In function ‘InsertImageRecord’:
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10:
>> error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable]
>> 13:03:43UINT16   Magic;
>> 13:03:43   ^
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1110:10:
>> error: variable ‘ImageType’ set but not used 
>> [-Werror=unused-but-set-variable]
>> 13:03:43UINT16   ImageType;
>> 13:03:43   ^
>>
>>
>> and honestly, i can't be bothered to look at the code myself, considering 
>> the way it was dumped into the mailing list and the repository.
>>
>> Please get this fixed
>>
>> Regards,
>> Ard.
>>
>>
>>>
>>> 
>>> --
>>> 
>>>
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>
--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Yao, Jiewen
Hi Ard
You are right. I happen to find a logic error, too.

Attach patch for your review.



-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Tuesday, June 09, 2015 8:28 PM
To: Yao, Jiewen
Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming
Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
MdeModulePkg/DxeCore support UEFI2.5 properties table

On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
> On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
>> Hi Ard
>> Thanks to report this. I have fixed this in 17565.
>
> Wow, that was quick! Thanks!
>

You have fixed the build, but the copy-pasted comments are still there, which 
make no sense at all anymore, since they refer to the Magic variable, which has 
been removed.

Next time, could you please send your patches (including fixes like
these) to the mailing list first? That way, people have a chance to see what's 
going in before its gets merged.

Thanks,
Ard.


>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Friday, June 05, 2015 9:12 PM
>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao, 
>> Liming
>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>
>> On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
>>> Hi
>>>
>>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties 
>>> Table feature.
>>>
>>>
>>>
>>> MdePkg – add Properties table definition.
>>>
>>> MdeModulePkg – add properties table support in DXE core.
>>>
>>> MdeModulePkg – add PropertiesTableAttributesDxe driver to set 
>>> ACPINvs/Reserved memory type to be XP.
>>>
>>>
>>>
>>> Signed-off-by: Yao, Jiewen jiewen@intel.com
>>>
>>> Reviewed-by: Zeng, Star star.z...@intel.com
>>>
>>> Reviewed-by: Gao, Liming liming@intel.com
>>>
>>
>> This patch breaks the build for GCC on 64-bit ARM
>>
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:
>> In function ‘InsertImageRecord’:
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10:
>> error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable]
>> 13:03:43UINT16   Magic;
>> 13:03:43   ^
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1110:10:
>> error: variable ‘ImageType’ set but not used 
>> [-Werror=unused-but-set-variable]
>> 13:03:43UINT16   ImageType;
>> 13:03:43   ^
>>
>>
>> and honestly, i can't be bothered to look at the code myself, considering 
>> the way it was dumped into the mailing list and the repository.
>>
>> Please get this fixed
>>
>> Regards,
>> Ard.
>>
>>
>>>
>>> 
>>> --
>>> 
>>>
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>


PropertiesTable.c.patch
Description: PropertiesTable.c.patch
--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Ard Biesheuvel
On 5 June 2015 at 15:22, Ard Biesheuvel  wrote:
> On 5 June 2015 at 15:21, Yao, Jiewen  wrote:
>> Hi Ard
>> Thanks to report this. I have fixed this in 17565.
>
> Wow, that was quick! Thanks!
>

You have fixed the build, but the copy-pasted comments are still
there, which make no sense at all anymore, since they refer to the
Magic variable, which has been removed.

Next time, could you please send your patches (including fixes like
these) to the mailing list first? That way, people have a chance to
see what's going in before its gets merged.

Thanks,
Ard.


>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Friday, June 05, 2015 9:12 PM
>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao, Liming
>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
>> MdeModulePkg/DxeCore support UEFI2.5 properties table
>>
>> On 4 June 2015 at 16:34, Yao, Jiewen  wrote:
>>> Hi
>>>
>>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties
>>> Table feature.
>>>
>>>
>>>
>>> MdePkg – add Properties table definition.
>>>
>>> MdeModulePkg – add properties table support in DXE core.
>>>
>>> MdeModulePkg – add PropertiesTableAttributesDxe driver to set
>>> ACPINvs/Reserved memory type to be XP.
>>>
>>>
>>>
>>> Signed-off-by: Yao, Jiewen jiewen@intel.com
>>>
>>> Reviewed-by: Zeng, Star star.z...@intel.com
>>>
>>> Reviewed-by: Gao, Liming liming@intel.com
>>>
>>
>> This patch breaks the build for GCC on 64-bit ARM
>>
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:
>> In function ‘InsertImageRecord’:
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10:
>> error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable]
>> 13:03:43UINT16   Magic;
>> 13:03:43   ^
>> 13:03:43 
>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1110:10:
>> error: variable ‘ImageType’ set but not used 
>> [-Werror=unused-but-set-variable]
>> 13:03:43UINT16   ImageType;
>> 13:03:43   ^
>>
>>
>> and honestly, i can't be bothered to look at the code myself, considering 
>> the way it was dumped into the mailing list and the repository.
>>
>> Please get this fixed
>>
>> Regards,
>> Ard.
>>
>>
>>>
>>> --
>>> 
>>>
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] Question about PEX boot on Xen with OVMF as bios

2015-06-09 Thread Anthony PERARD
On Sun, Jun 07, 2015 at 01:22:46PM +0100, Wei Liu wrote:
> On Tue, May 26, 2015 at 12:02:22PM +0200, Laszlo Ersek wrote:
> > (3) Spelling out a special case of (2) -- there's no xen-netfront driver
> > in OVMF.
> > 
> 
> This is also correct. IIRC there is only Xen PV disk driver in OVMF.
> I think the infrastructure needed to port netfront driver is already
> there. Just that the driver is not yet there.
> 
> Anthony, am I right?

That correct. But the infrastructure might need some refactoring to handle
a netfront.

-- 
Anthony PERARD

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [PATCH 1/2] MdeModulePkg: Add PcdMaxAuthVariableSize declaration.

2015-06-09 Thread Star Zeng
Currently, Max Authenticated variable size = Max Regular Variable Size,
and both are controlled via a single PCD:
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize.

This PCD is introduced to control the Max Size of Authenticated Variable.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdeModulePkg/MdeModulePkg.dec |   5 +
 MdeModulePkg/MdeModulePkg.uni | Bin 165808 -> 166786 bytes
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index e3f4600..97237ca 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -717,6 +717,11 @@
   # @Prompt Maximum variable size.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400|UINT32|0x3003
 
+  ## The maximum size of a single authenticated variable.
+  # The value is 0 as default for compatibility that maximum authenticated 
variable size is specified by PcdMaxVariableSize.
+  # @Prompt Maximum authenticated variable size.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x00|UINT32|0x3009
+
   ## The maximum size of single hardware error record variable.
   # In IA32/X64 platforms, this value should be larger than 1KB.
   # In IA64 platforms, this value should be larger than 128KB.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 
238474def78b262d32cd92983f065d97c7050f88..b33060e79bf6ae54c18afb4feffdcf252583f5b5
 100644
GIT binary patch
delta 227
zcmdnc&($=YYr_>Yc1MO%h7yL1$&9b0CkwnZnyg?dAesmfV@PGl1IlGGBr_y3luUN?
z5ufZ}T85;BXm$7EtE@RD<761UoO$$u`

-- 
1.9.5.msysgit.0


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


[edk2] [PATCH 2/2] SecurityPkg Variable: Support the new introduced PcdMaxAuthVariableSize.

2015-06-09 Thread Star Zeng
1. If PcdMaxAuthVariableSize is set to 0, keep current behavior as is and
PcdMaxVariableSize used.
2. If PcdMaxAuthVariableSize is set to non 0, it will work on
authenticated variables.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 .../VariableAuthenticated/RuntimeDxe/AuthService.c | 11 ++--
 .../VariableAuthenticated/RuntimeDxe/AuthService.h |  9 +++-
 .../VariableAuthenticated/RuntimeDxe/Variable.c| 60 ++
 .../VariableAuthenticated/RuntimeDxe/Variable.h|  8 +++
 .../RuntimeDxe/VariableRuntimeDxe.inf  |  1 +
 .../VariableAuthenticated/RuntimeDxe/VariableSmm.c |  2 +-
 .../RuntimeDxe/VariableSmm.inf |  3 +-
 .../RuntimeDxe/VariableSmmRuntimeDxe.c |  6 ++-
 .../RuntimeDxe/VariableSmmRuntimeDxe.inf   |  1 +
 9 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c 
b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 36d4470..9599c8a 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -124,13 +124,18 @@ InCustomMode (
 /**
   Initializes for authenticated varibale service.
 
+  @param[in] MaxAuthVariableSizeReflect the overhead associated with the 
saving
+of a single EFI authenticated variable 
with the exception
+of the overhead associated with the length
+of the string name of the EFI variable.
+
   @retval EFI_SUCCESS   Function successfully executed.
   @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resources.
 
 **/
 EFI_STATUS
 AutenticatedVariableServiceInitialize (
-  VOID
+  IN UINTN  MaxAuthVariableSize
   )
 {
   EFI_STATUS  Status;
@@ -158,7 +163,7 @@ AutenticatedVariableServiceInitialize (
   //
   // Reserve runtime buffer for public key database. The size excludes 
variable header and name size.
   //
-  mMaxKeyDbSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - 
sizeof (AUTHVAR_KEYDB_NAME);
+  mMaxKeyDbSize = (UINT32) (MaxAuthVariableSize - sizeof (AUTHVAR_KEYDB_NAME));
   mMaxKeyNumber = mMaxKeyDbSize / EFI_CERT_TYPE_RSA2048_SIZE;
   mPubKeyStore  = AllocateRuntimePool (mMaxKeyDbSize);
   if (mPubKeyStore == NULL) {
@@ -168,7 +173,7 @@ AutenticatedVariableServiceInitialize (
   //
   // Reserve runtime buffer for certificate database. The size excludes 
variable header and name size.
   //
-  mMaxCertDbSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - 
sizeof (EFI_CERT_DB_NAME);
+  mMaxCertDbSize = (UINT32) (MaxAuthVariableSize - sizeof (EFI_CERT_DB_NAME));
   mCertDbStore   = AllocateRuntimePool (mMaxCertDbSize);
   if (mCertDbStore == NULL) {
 return EFI_OUT_OF_RESOURCES;
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h 
b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
index f28c825..56def50 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
@@ -139,13 +139,18 @@ UpdatePlatformMode (
 /**
   Initializes for authenticated varibale service.
 
+  @param[in] MaxAuthVariableSizeReflect the overhead associated with the 
saving
+of a single EFI authenticated variable 
with the exception
+of the overhead associated with the length
+of the string name of the EFI variable.
+
   @retval EFI_SUCCESS   Function successfully executed.
-  @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resource.
+  @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resources.
 
 **/
 EFI_STATUS
 AutenticatedVariableServiceInitialize (
-  VOID
+  IN UINTN  MaxAuthVariableSize
   );
 
 /**
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c 
b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 7ecc29b..15d0531 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -2153,7 +2153,7 @@ UpdateVariable (
   // Trying to update NV variable prior to the installation of 
EFI_VARIABLE_WRITE_ARCH_PROTOCOL
   //
   return EFI_NOT_AVAILABLE_YET;
-} else if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0) {
+} else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
   //
   // Trying to update volatile authenticated variable prior to the 
installation of EFI_VARIABLE_WRITE_ARCH_PROTOCOL
   // The authenticated variable perhaps is not initialized, just return 
here.
@@ -2190,7 +2190,7 @@ UpdateVariable (
   // as a temporary storage.
   //
   NextVariable = GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN) 
mVariableModuleGlobal->VariableGlobal.VolatileVa

[edk2] [PATCH 0/2] Introduce new PcdMaxAuthVariableSize PCD.

2015-06-09 Thread Star Zeng
Star Zeng (2):
  MdeModulePkg: Add PcdMaxAuthVariableSize declaration.
  SecurityPkg Variable: Support the new introduced
PcdMaxAuthVariableSize.

 MdeModulePkg/MdeModulePkg.dec  |   5 ++
 MdeModulePkg/MdeModulePkg.uni  | Bin 165808 -> 166786 bytes
 .../VariableAuthenticated/RuntimeDxe/AuthService.c |  11 ++--
 .../VariableAuthenticated/RuntimeDxe/AuthService.h |   9 +++-
 .../VariableAuthenticated/RuntimeDxe/Variable.c|  60 ++---
 .../VariableAuthenticated/RuntimeDxe/Variable.h|   8 +++
 .../RuntimeDxe/VariableRuntimeDxe.inf  |   1 +
 .../VariableAuthenticated/RuntimeDxe/VariableSmm.c |   2 +-
 .../RuntimeDxe/VariableSmm.inf |   3 +-
 .../RuntimeDxe/VariableSmmRuntimeDxe.c |   6 ++-
 .../RuntimeDxe/VariableSmmRuntimeDxe.inf   |   1 +
 11 files changed, 78 insertions(+), 28 deletions(-)

-- 
1.9.5.msysgit.0


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Update nasmb build rule

2015-06-09 Thread Gao, Liming
Jordan:
  Yes. I agree .bin is better postfix for .nasmb. After we fully migrate .asm16 
to .nasmb, I will update our reference platform to use .bin in their FDF Ffs 
Rules. 

Thanks
Liming
-Original Message-
From: Justen, Jordan L 
Sent: Tuesday, June 09, 2015 3:12 PM
To: Gao, Liming; edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule

On 2015-06-08 18:31:30, Gao, Liming wrote:
> Jordan:
>   I don't get any feedback from others. You also say I can go ahead.
>   So, I add this change.

Ok. You are right. It is true that I said: "I don't like the idea of creating 
the .com for .nasmb sources, but if you insist that it is required, then you 
can go ahead."

So, I guess you went ahead.

Can you at least agree that the use of the .com extension is confusing in this 
case, since .com files are 16-bit DOS executables? I think the only reason they 
appeared in EDK/EDK II is because masm16 would only output a .com file.

I don't think we need to make everything backward compatible. This is a new 
tool without the legacy masm baggage, so do we really need to go out of our way 
to bring the masm baggage along?

Really, wouldn't it be reasonable for a platform to make a 1-line change to 
their .fdf file if for no other reason than to make it obvious that a big 
change just happened to their SEC phase?

This project goes *way* out of it's way to try to be backward compatible. In my 
opinion, often too far.

Ok. Rant complete. Thanks for your time, :)

-Jordan

>   I add this support for compatibility. I want to update our core
>   module to use .nasmb, and platform can directly use the updated
>   core module without changes. Now, our most platforms use .com
>   postfix in their platform FDF Ffs Rule. With this patch, after we
>   update more core modules to use .nasmb instead .asm16, it will not
>   impact platforms.
> 
> Thanks
> Liming
> -Original Message-
> From: Justen, Jordan L
> Sent: Tuesday, June 9, 2015 12:56 AM
> To: Gao, Liming; edk2-devel@lists.sourceforge.net
> Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule
> 
> On 2014-11-21 10:16:47, Jordan Justen wrote:
> > On 2014-11-20 22:12:42, Gao, Liming wrote:
> > > Jordan:
> > >   NASMB is replacement of ASM16.
> > 
> > I see .nasmb as an alternative, but not a replacement. (.asm16 is 
> > still supported by EDK II.)
> > 
> > I do hope that long term it replaces all uses of .asm16 though. :)
> > 
> > >   Now, ASM16 has been used in the
> > >   different modules. When the module owners do this conversion, they
> > >   expect it is compatible and doesn't impact platform, because
> > >   module owner can't predict how many platform will be impacted. So,
> > >   I think the compatibility is the important point to persuade the
> > >   module owners to do this conversion.
> > 
> > I would rather see .bin be used in this case, because it makes more 
> > sense than .com. I guess .com was used because it was the output 
> > from link16? But, .com files are known as old dos based executables. 
> > It is a confusing use of the extension.
> 
> I see you went ahead and committed this without further discussion.
> 
> I also see that you converted an internal platform's fdf from being 
> able to accept either .bin or .com to only accept .com. So, apparently 
> you have real preference for using .com rather than .bin.
> 
> Can you explain why you think .com makes better sense than .bin in 
> this context?
> 
> http://en.wikipedia.org/wiki/COM_file
> 
> -Jordan
> 
> > If we copy the file like in your patch, then few platforms will 
> > actually change from using .com to .bin, but I think it would be an 
> > improvement for them to convert to using the .bin extension.
> > 
> > Maybe another idea is to leave the .asm16 .inf in place, and create 
> > a new .inf with the .nasmb source. Then the platform will have to 
> > change both the .dsc and .fdf to make use of NASM. This will also 
> > leave the platform with the option to continue using MASM16/LINK16.
> > 
> > I don't like the idea of creating the .com for .nasmb sources, but 
> > if you insist that it is required, then you can go ahead. I will not 
> > give my r-b for the change though. :)
> > 
> > -Jordan
> > 
> > > -Original Message-
> > > From: Justen, Jordan L
> > > Sent: Friday, November 21, 2014 12:58 PM
> > > To: edk2-devel@lists.sourceforge.net; Gao, Liming
> > > Subject: Re: [edk2] [Patch] BaseTools: Update nasmb build rule
> > > 
> > > On 2014-11-18 23:49:45, Gao, Liming wrote:
> > > > Nasmb can replace ASM16. Now, ASM16 output file is *.com. To be 
> > > > compatible with it, update nasmb build rule to output *.com file also.
> > > > 
> > > > ASM16 output file is used by Platform FDF Rule section. With 
> > > > this patch, Platform FDF file will not be impacted after asm16 
> > > > file is converted to nasmb file.
> > > 
> > > Liming,
> > > 
> > > I don't think we need this change. With a small change to the .fdf, .bin 
> > > can 

Re: [edk2] [PATCH 0/7] ArmPkg/ArmVirtPkg: GIC revision detection

2015-06-09 Thread Ard Biesheuvel
On 5 June 2015 at 09:46, Ard Biesheuvel  wrote:
> On 29 May 2015 at 14:33, Ard Biesheuvel  wrote:
>> The current GIC revision detection code infers the GIC revision from
>> the AA64PFR0_EL1.GIC feature bit that tells us whether the GIC system
>> register interface is implemented in the hardware, and then proceeds
>> to attempt and enable it.
>>
>> The library containing this code deliberately does not cache the
>> detected revision since it may execute in the SEC phase and may
>> be running from NOR flash and not RAM.
>>
>> However, since the detection code runs very often, and is quite
>> heavy-weight when running under virtualization (especially KVM),
>> this series refactors the GIC revision detection to:
>> - use fewer system register accesses if possible
>> - provide an alternative that does cache the detected revision
>> - use the DT supplied revision when executing on a DT based platform
>>
>>
>> Ard Biesheuvel (7):
>>   ArmPkg: reduce sysreg access count in GIC revision probe
>>   ArmPkg: merge ArmGicV[23]Lib.h into ArmGicLib.h
>>   ArmPkg: split off ArmGicArchLib from ArmGicLib
>>   ArmPkg: copy ArmGicArchLib to ArmGicArchSecLib
>>   ArmPkg: cache detected revision in ArmGicArchLib
>>   ArmVirtPkg: record GIC revision in dynamic PCD
>>   ArmVirtPkg: implement DT-based ArmGicArchLib
>>
>
> Hello Olivier,
>
> May we please have your input regarding patches #2 and #3 in this
> series? Those are prerequisites for patch #7, which is essentially
> both a performance and a correctness fix for ArmVirtPkg.
> (Patches #4 and #5 only affect bare-metal platforms, and are not
> depended upon by subsequent patches, but I'd still like your opinion
> on those as well)
>

OK, I will go ahead and submit patch #1, which Olivier has already
reviewed, and should reduce the number of sysreg accesses (and thus
the number of world switches under KVM) by two thirds. But I would
still like some feedbacks (and acks/r-b) on patches #2 - #5 please?

Regards,
Ard.

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [Patch] SourceLevelDebugPkg/DxeDebugAgent: Initialize Local APIC Timer

2015-06-09 Thread Ni, Ruiyu
Reviewed-by: Ruiyu Ni 

> -Original Message-
> From: Fan, Jeff
> Sent: Tuesday, June 9, 2015 10:21 AM
> To: edk2-devel@lists.sourceforge.net
> Cc: Ni, Ruiyu
> Subject: [Patch] SourceLevelDebugPkg/DxeDebugAgent: Initialize Local APIC
> Timer
> 
> Now Debug Agent library uses Local APIC Timer to implement time-out
> mechanism.
> For AP, its local APIC timer may not work. This fix is to initialize Local
> APIC timer if it doesn't work as expected when debugging AP function.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> CC: Ruiyu Ni 
> ---
>  .../Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c|
> 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git
> a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAg
> entLib.c
> b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAg
> entLib.c
> index 56a5e1a..f7fbb3c 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAg
> entLib.c
> +++
> b/SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAg
> entLib.c
> @@ -365,9 +365,19 @@ InitializeDebugAgent (
>IA32_DESCRIPTOR  IdtDescriptor;
>IA32_DESCRIPTOR  *Ia32Idtr;
>IA32_IDT_ENTRY   *Ia32IdtEntry;
> +  BOOLEAN  PeriodicMode;
> +  UINTNTimerCycle;
> 
>if (InitFlag == DEBUG_AGENT_INIT_DXE_AP) {
>  //
> +// Check if CPU APIC Timer is working, otherwise initialize it.
> +//
> +GetApicTimerState (NULL, &PeriodicMode, NULL);
> +TimerCycle = GetApicTimerInitCount ();
> +if (!PeriodicMode || TimerCycle == 0) {
> +  InitializeDebugTimer (NULL, FALSE);
> +}
> +//
>  // Invoked by AP, enable interrupt to let AP could receive IPI from
> other processors
>  //
>  EnableInterrupts ();
> --
> 1.9.5.msysgit.0


--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [patch] MdePkg/PropertiesTable support, MdeModulePkg/DxeCore support UEFI2.5 properties table

2015-06-09 Thread Laszlo Ersek
On 06/09/15 03:20, Yao, Jiewen wrote:
> HI Laszlo
> Thanks for your feedback. It reminds us to use GIT.
> I am sorry that I missed your feedback before. I search it in my mailbox 
> again today, but I fail to find it.
> Not sure why. :-(
> 
> Currently we are still using SVN on some machines. Sorry to bring trouble for 
> GIT user.
> We are moving towards GIT, and we are involving community for more discussion.

Works for me, thank you. I just hope that eventually everyone on the
Intel firmware team will move to git. (I do realize there's the other
discussion about submodules / subtrees, which could be a blocker for you
guys.)

Thanks
Laszlo

> 
> Thank you
> Yao Jiewen
> 
> -Original Message-
> From: Justen, Jordan L 
> Sent: Tuesday, June 09, 2015 8:14 AM
> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Laszlo Ersek; Kinney, 
> Michael D
> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, 
> MdeModulePkg/DxeCore support UEFI2.5 properties table
> 
> On 2015-06-04 10:47:37, Laszlo Ersek wrote:
>> Jiewen,
>>
>> On 06/04/15 16:34, Yao, Jiewen wrote:> Hi
>>>
>>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties 
>>> Table feature.
>>>
>>> MdePkg \u2013 add Properties table definition.
>>> MdeModulePkg \u2013 add properties table support in DXE core.
>>> MdeModulePkg \u2013 add PropertiesTableAttributesDxe driver to set 
>>> ACPINvs/Reserved memory type to be XP.
>>>
>>> Signed-off-by: Yao, Jiewen jiewen@intel.com 
>>> 
>>> Reviewed-by: Zeng, Star star.z...@intel.com 
>>> 
>>> Reviewed-by: Gao, Liming liming@intel.com 
>>> 
>>
>> Honest question: do you ever intend to abandon the following bad
>> practices:
>> - extremely long lines,
>> - huge code bombs in a few patches,
>> - posting patches as attachments?
>>
>> The diffstat for the second attachment is:
>>
>>  /PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.uni  
>>  |1
>>  /PropertiesTableAttributesDxe/PropertiesTableAttributesDxeExtra.uni 
>>  |1
>>  Core/Dxe/DxeMain.h  
>>  |   29
>>  Core/Dxe/DxeMain.inf
>>  |4
>>  Core/Dxe/DxeMain/DxeMain.c  
>>  |2
>>  Core/Dxe/Image/Image.c  
>>  |2
>>  Core/Dxe/Misc/PropertiesTable.c 
>>  | 1418 ++
>>  MdeModulePkg.dec
>>  |   22
>>  MdeModulePkg.dsc
>>  |2
>>  Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.c   
>>  |  412 ++
>>  Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.inf 
>>  |  114
>>  Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.uni 
>>  |1
>>  
>> Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxeExtra.uni 
>> |1
>>  13 files changed, 2009 insertions(+)
>>
>> Also, the longest line in the patch is 214 characters.
>>
>> Do you ever intend to adopt inline patches, to break up features into 
>> series of patches, and to follow the line length hints of the edk2 
>> coding style?
>>
>> Honestly, I have the impression about these postings that they are an 
>> after-the-fact (non-)courtesy to the mailing list. "We wrote this at 
>> Intel, it has been reviewed, it's going in, we'll ignore your 
>> formatting requests that you've repeated many times; if you are unable 
>> to review it as-is, your loss".
>>
>> If you are committed to ignore these bugs in your patches in the long 
>> term, I'd like to know that.
> 
> Jiewen, it seems like Laszlo expressed some (valid) concerns with your patch, 
> but you didn't respond, and checked them in anyway.
> 
> Considering how much effort it is for people to code review patches, it is 
> not very friendly to ignore their feedback.
> 
> Occasionally, in urgent cases, we can disagree and still move forward even 
> when there is disagreement, but I don't think it is good to ignore the 
> discussion entirely and move forward with no regard for the concerns.
> 
> Did you intend this edk2-devel "code-review" to actually be "We wrote this at 
> Intel, it has been reviewed, it's going in" as was Laszlo's concern?
> 
> I think the real start of the issue is that this should have been posted to 
> edk2-devel before there was any Reviewed-by. And, following the posting, all 
> feedback from the community should be considered.
> 
> Mike, do you have any thoughts on how we might be able to address these 
> issues? Maybe we could consider removing commit privileges for developers 
> that don't follow the process? Is there someone else on the team besides you 
> that I should ask this question to?
> 
> Thanks,
> 
> -Jordan
> 

Re: [edk2] [Patch]Vlv2TbltDevicePkg: make sure system could boot properly when "Setup" variable is corrupted

2015-06-09 Thread He, Tim
Ok, I will describe it in the check-in log. thank you David..

Best Regards,
Tim

From: Wei, David
Sent: Tuesday, June 09, 2015 3:39 PM
To: He, Tim; edk2-devel@lists.sourceforge.net
Cc: Wu, Mike; Lu, ShifeiX A; Wei, David
Subject: RE: [edk2][Patch]Vlv2TbltDevicePkg: make sure system could boot 
properly when "Setup" variable is corrupted

Hi Tim,

Looks like this solution could not catch "Setup" variable corruption when its 
size is not changed?  If this limit is true, could you describe it in the 
check-in log?

Thanks,
David | SSG BIOS

From: He, Tim
Sent: Tuesday, June 09, 2015 10:22 AM
To: edk2-devel@lists.sourceforge.net
Cc: Wei, David; Wu, Mike; Lu, ShifeiX A
Subject: [edk2][Patch]Vlv2TbltDevicePkg: make sure system could boot properly 
when "Setup" variable is corrupted

Recovery "Setup" variable for Minnowboard Max to make sure system could boot 
properly in case of "Setup" variable is corrupted somehow.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tim He mailto:tim...@intel.com>>

Best Regards,
Tim

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [Patch]Vlv2TbltDevicePkg: make sure system could boot properly when "Setup" variable is corrupted

2015-06-09 Thread Wei, David
Hi Tim,

Looks like this solution could not catch "Setup" variable corruption when its 
size is not changed?  If this limit is true, could you describe it in the 
check-in log?

Thanks,
David | SSG BIOS

From: He, Tim
Sent: Tuesday, June 09, 2015 10:22 AM
To: edk2-devel@lists.sourceforge.net
Cc: Wei, David; Wu, Mike; Lu, ShifeiX A
Subject: [edk2][Patch]Vlv2TbltDevicePkg: make sure system could boot properly 
when "Setup" variable is corrupted

Recovery "Setup" variable for Minnowboard Max to make sure system could boot 
properly in case of "Setup" variable is corrupted somehow.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tim He mailto:tim...@intel.com>>

Best Regards,
Tim

--
___
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Update nasmb build rule

2015-06-09 Thread Jordan Justen
On 2015-06-08 18:31:30, Gao, Liming wrote:
> Jordan:
>   I don't get any feedback from others. You also say I can go ahead.
>   So, I add this change.

Ok. You are right. It is true that I said: "I don't like the idea of
creating the .com for .nasmb sources, but if you insist that it is
required, then you can go ahead."

So, I guess you went ahead.

Can you at least agree that the use of the .com extension is confusing
in this case, since .com files are 16-bit DOS executables? I think the
only reason they appeared in EDK/EDK II is because masm16 would only
output a .com file.

I don't think we need to make everything backward compatible. This is
a new tool without the legacy masm baggage, so do we really need to go
out of our way to bring the masm baggage along?

Really, wouldn't it be reasonable for a platform to make a 1-line
change to their .fdf file if for no other reason than to make it
obvious that a big change just happened to their SEC phase?

This project goes *way* out of it's way to try to be backward
compatible. In my opinion, often too far.

Ok. Rant complete. Thanks for your time, :)

-Jordan

>   I add this support for compatibility. I want to update our core
>   module to use .nasmb, and platform can directly use the updated
>   core module without changes. Now, our most platforms use .com
>   postfix in their platform FDF Ffs Rule. With this patch, after we
>   update more core modules to use .nasmb instead .asm16, it will not
>   impact platforms.
> 
> Thanks
> Liming
> -Original Message-
> From: Justen, Jordan L 
> Sent: Tuesday, June 9, 2015 12:56 AM
> To: Gao, Liming; edk2-devel@lists.sourceforge.net
> Subject: RE: [edk2] [Patch] BaseTools: Update nasmb build rule
> 
> On 2014-11-21 10:16:47, Jordan Justen wrote:
> > On 2014-11-20 22:12:42, Gao, Liming wrote:
> > > Jordan:
> > >   NASMB is replacement of ASM16.
> > 
> > I see .nasmb as an alternative, but not a replacement. (.asm16 is 
> > still supported by EDK II.)
> > 
> > I do hope that long term it replaces all uses of .asm16 though. :)
> > 
> > >   Now, ASM16 has been used in the
> > >   different modules. When the module owners do this conversion, they
> > >   expect it is compatible and doesn't impact platform, because
> > >   module owner can't predict how many platform will be impacted. So,
> > >   I think the compatibility is the important point to persuade the
> > >   module owners to do this conversion.
> > 
> > I would rather see .bin be used in this case, because it makes more 
> > sense than .com. I guess .com was used because it was the output from 
> > link16? But, .com files are known as old dos based executables. It is 
> > a confusing use of the extension.
> 
> I see you went ahead and committed this without further discussion.
> 
> I also see that you converted an internal platform's fdf from being
> able to accept either .bin or .com to only accept .com. So,
> apparently you have real preference for using .com rather than .bin.
> 
> Can you explain why you think .com makes better sense than .bin in
> this context?
> 
> http://en.wikipedia.org/wiki/COM_file
> 
> -Jordan
> 
> > If we copy the file like in your patch, then few platforms will 
> > actually change from using .com to .bin, but I think it would be an 
> > improvement for them to convert to using the .bin extension.
> > 
> > Maybe another idea is to leave the .asm16 .inf in place, and create a 
> > new .inf with the .nasmb source. Then the platform will have to change 
> > both the .dsc and .fdf to make use of NASM. This will also leave the 
> > platform with the option to continue using MASM16/LINK16.
> > 
> > I don't like the idea of creating the .com for .nasmb sources, but if 
> > you insist that it is required, then you can go ahead. I will not give 
> > my r-b for the change though. :)
> > 
> > -Jordan
> > 
> > > -Original Message-
> > > From: Justen, Jordan L
> > > Sent: Friday, November 21, 2014 12:58 PM
> > > To: edk2-devel@lists.sourceforge.net; Gao, Liming
> > > Subject: Re: [edk2] [Patch] BaseTools: Update nasmb build rule
> > > 
> > > On 2014-11-18 23:49:45, Gao, Liming wrote:
> > > > Nasmb can replace ASM16. Now, ASM16 output file is *.com. To be 
> > > > compatible with it, update nasmb build rule to output *.com file also.
> > > > 
> > > > ASM16 output file is used by Platform FDF Rule section. With this 
> > > > patch, Platform FDF file will not be impacted after asm16 file is 
> > > > converted to nasmb file.
> > > 
> > > Liming,
> > > 
> > > I don't think we need this change. With a small change to the .fdf, .bin 
> > > can work as easily as .com.
> > > 
> > > I think .com was a bad extension for this file, even when
> > > masm16/link16 was being used. Maybe we should have copied from .com to 
> > > .bin before.
> > > 
> > > -Jordan
> > > 
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Liming Gao 
> > > > 
> > > > ---
> > > > 
> > > > Index: build_rule.template
> > > > 
> > > > ==