[edk2] [Patch 2/3] SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.

[Eric Dong]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 SecurityPkg/Include/Library/TcgStorageOpalLib.h|  41 ++
 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426 ++---
 .../TcgStorageOpalLib/TcgStorageOpalLib.inf|   1 +
 .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +
 .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++-
 5 files changed, 731 insertions(+), 49 deletions(-)
 create mode 100644 
SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h

diff --git a/SecurityPkg/Include/Library/TcgStorageOpalLib.h 
b/SecurityPkg/Include/Library/TcgStorageOpalLib.h
index 9b64a8e5cd..2947c0eaf1 100644
--- a/SecurityPkg/Include/Library/TcgStorageOpalLib.h
+++ b/SecurityPkg/Include/Library/TcgStorageOpalLib.h
@@ -82,6 +82,15 @@ typedef struct {
 //
 UINT32 BlockSid : 1;
 
+//
+// Pyrite SSC V2 support  (0 - not supported, 1 - supported)
+//
+UINT32 PyriteSscV2 : 1;
+
+//
+// Supported Data Removal Mechanism support  (0 - not supported, 1 - 
supported)
+//
+UINT32 DataRemoval : 1;
 } OPAL_DISK_SUPPORT_ATTRIBUTE;
 
 //
@@ -834,4 +843,36 @@ OpalUtilAdminPasswordExists(
   IN  TCG_LOCKING_FEATURE_DESCRIPTOR   *LockingFeature
   );
 
+/**
+  Get Active Data Removal Mechanism Value.
+
+  @param[in]  Session,   The session info for one opal 
device.
+  @param[in]  GeneratedSid   Generated SID of disk
+  @param[in]  SidLength  Length of generatedSid in 
bytes
+  @param[out] ActiveDataRemovalMechanism Return the active data 
removal mechanism.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalUtilGetActiveDataRemovalMechanism (
+  OPAL_SESSION  *Session,
+  const VOID*GeneratedSid,
+  UINT32SidLength,
+  UINT8 *ActiveDataRemovalMechanism
+  );
+
+/**
+  Get the supported Data Removal Mechanism list.
+
+  @param[in]  Session,   The session info for one opal 
device.
+  @param[out] RemovalMechanismLists  Return the supported data 
removal mechanism lists.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalUtilGetDataRemovalMechanismLists (
+  IN  OPAL_SESSION  *Session,
+  OUT UINT32*RemovalMechanismLists
+  );
+
 #endif // _OPAL_CORE_H_
diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
index 90cc51a24c..d031ebe798 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
@@ -1,7 +1,7 @@
 /** @file
   Public API for Opal Core library.
 
-Copyright (c) 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2016 - 2018, 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
@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 
+#include "TcgStorageOpalLibInternal.h"
+
 #pragma pack(1)
 typedef struct {
 UINT8 HardwareReset : 1;
@@ -89,6 +91,7 @@ OpalTrustedSend(
   @param[in]  SpSpecificSecurity Protocol Specific
   @param[in]  BufferAddress of Data to transfer
   @param[in]  BufferSizeFull Size of Buffer, including space 
that may be used for padding.
+  @param[in]  EstimateTimeCost  Estimate the time needed.
 
 **/
 TCG_RESULT
@@ -98,10 +101,10 @@ OpalTrustedRecv(
   UINT8  SecurityProtocol,
   UINT16 SpSpecific,
   VOID   *Buffer,
-  UINTN  BufferSize
+  UINTN  BufferSize,
+  UINT32 EstimateTimeCost
   )
 {
-
   UINTN   TransferLength512;
   UINT32  Tries;
   TCG_COM_PACKET  *ComPacket;
@@ -129,9 +132,15 @@ OpalTrustedRecv(
   // so we need to retry the IF-RECV to get the actual Data.
   // See TCG Core Spec v2 Table 45 IF-RECV ComPacket Field Values Summary
   // This is an arbitrary number of retries, not from the spec.
-  // have a max timeout of 10 seconds, 5000 tries * 2ms = 10s
   //
-  Tries = 5000;
+  // if user input estimate time cost(second level) value bigger than 10s, 
base on user input value to wait.
+  // Else, Use a max timeout of 10 seconds to wait, 5000 tries * 2ms = 10s
+  //
+  if (EstimateTimeCost > 10) {
+Tries = EstimateTimeCost * 500; // 500 = 1000 * 1000 / 2000;
+  } else {
+Tries = 5000;
+  }
   while ((Tries--) > 0) {
 ZeroMem( Buffer, BufferSize );
 TransferSize = 0;
@@ -146,7 +155,6 @@ OpalTrustedRecv(
  Buffer,
  
  );
-
 if (EFI_ERROR (Status)) {

[edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.

[Eric Dong]

Eanble the pyrite 2.0 devices for opal driver.

Eric Dong (3):
  MdePkg: Add Feature definitions add in pyrite 2.0 spec.
  SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.
  SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

 MdePkg/Include/IndustryStandard/TcgStorageCore.h   |   2 +
 MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  54 +++
 SecurityPkg/Include/Library/TcgStorageOpalLib.h|  41 ++
 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426 ++---
 .../TcgStorageOpalLib/TcgStorageOpalLib.inf|   1 +
 .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +
 .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++-
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c |  60 ++-
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h |   9 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c|  84 
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.c|   1 +
 11 files changed, 934 insertions(+), 56 deletions(-)
 create mode 100644 
SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h

-- 
2.15.0.windows.1

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

[edk2] [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

[Eric Dong]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 60 ++--
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h |  9 +++
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c| 84 ++
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.c|  1 +
 4 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 4133e503e2..91ab372f0c 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -105,10 +105,9 @@ OpalSupportGetAvailableActions(
   }
 
   //
-  // Psid revert is available for any device with media encryption support
-  // Revert is allowed for any device with media encryption support, however 
it requires
+  // Psid revert is available for any device with media encryption support or 
pyrite 2.0 type support.
   //
-  if (SupportedAttributes->MediaEncryption) {
+  if (SupportedAttributes->PyriteSscV2 || 
SupportedAttributes->MediaEncryption) {
 
 //
 // Only allow psid revert if media encryption is enabled.
@@ -657,6 +656,8 @@ OpalEndOfDxeEventNotify (
   @param[in]  Dev   The device which need Psid to process Psid Revert
 OPAL request.
   @param[in]  PopUpString   Pop up string.
+  @param[in]  PopUpString2  Pop up string in line 2.
+
   @param[out] PressEsc  Whether user escape function through Press ESC.
 
   @retval Psid string if success. NULL if failed.
@@ -666,6 +667,7 @@ CHAR8 *
 OpalDriverPopUpPsidInput (
   IN OPAL_DRIVER_DEVICE *Dev,
   IN CHAR16 *PopUpString,
+  IN CHAR16 *PopUpString2,
   OUT BOOLEAN   *PressEsc
   )
 {
@@ -689,6 +691,7 @@ OpalDriverPopUpPsidInput (
   EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
   ,
   PopUpString,
+  PopUpString2,
   L"-",
   Mask,
   NULL
@@ -1369,6 +1372,8 @@ ProcessOpalRequestPsidRevert (
   EFI_INPUT_KEY Key;
   TCG_RESULTRet;
   CHAR16*PopUpString;
+  CHAR16*PopUpString2;
+  UINTN BufferSize;
 
   if (Dev == NULL) {
 return;
@@ -1378,6 +1383,20 @@ ProcessOpalRequestPsidRevert (
 
   PopUpString = OpalGetPopUpString (Dev, RequestString);
 
+  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME) {
+BufferSize = StrSize (L"Warning: Revert action will take about ## 
seconds, DO NOT power off system during the revert action!");
+PopUpString2 = AllocateZeroPool (BufferSize);
+ASSERT (PopUpString2 != NULL);
+UnicodeSPrint (
+PopUpString2,
+BufferSize,
+L"WARNING: Revert action will take about %d seconds, DO NOT power off 
system during the revert action!",
+Dev->OpalDisk.EstimateTimeCost
+  );
+  } else {
+PopUpString2 = NULL;
+  }
+
   Count = 0;
 
   ZeroMem(, sizeof(Session));
@@ -1386,7 +1405,7 @@ ProcessOpalRequestPsidRevert (
   Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
 
   while (Count < MAX_PSID_TRY_COUNT) {
-Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, );
+Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2, 
);
 if (PressEsc) {
 do {
   CreatePopUp (
@@ -1400,7 +1419,7 @@ ProcessOpalRequestPsidRevert (
 
 if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
   gST->ConOut->ClearScreen(gST->ConOut);
-  return;
+  goto Done;
 } else {
   //
   // Let user input Psid again.
@@ -1456,6 +1475,11 @@ ProcessOpalRequestPsidRevert (
 } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
 gST->ConOut->ClearScreen(gST->ConOut);
   }
+
+Done:
+  if (PopUpString2 != NULL) {
+FreePool (PopUpString2);
+  }
 }
 
 /**
@@ -1482,6 +1506,8 @@ ProcessOpalRequestRevert (
   TCG_RESULTRet;
   BOOLEAN   PasswordFailed;
   CHAR16*PopUpString;
+  CHAR16*PopUpString2;
+  UINTN BufferSize;
 
   if (Dev == NULL) {
 return;
@@ -1491,6 +1517,20 @@ ProcessOpalRequestRevert (
 
   PopUpString = OpalGetPopUpString (Dev, RequestString);
 
+  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME) {
+BufferSize = StrSize (L"Warning: Revert action will take about ## 
seconds, DO NOT power off system during the revert action!");
+PopUpString2 = AllocateZeroPool (BufferSize);
+ASSERT (PopUpString2 != NULL);
+UnicodeSPrint (
+PopUpString2,
+BufferSize,
+L"WARNING: Revert action will take about %d seconds, DO NOT power off 
system during the revert action!",
+Dev->OpalDisk.EstimateTimeCost
+  );
+  } else {
+PopUpString2 = NULL;
+  }
+
   Count = 0;
 
   ZeroMem(, sizeof(Session));
@@ -1499,7 +1539,7 @@ ProcessOpalRequestRevert (
   

[edk2] [Patch 1/3] MdePkg: Add Feature definitions add in pyrite 2.0 spec.

[Eric Dong]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 MdePkg/Include/IndustryStandard/TcgStorageCore.h |  2 +
 MdePkg/Include/IndustryStandard/TcgStorageOpal.h | 54 
 2 files changed, 56 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/TcgStorageCore.h 
b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
index 56ea92f2eb..6d80da2401 100644
--- a/MdePkg/Include/IndustryStandard/TcgStorageCore.h
+++ b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
@@ -228,7 +228,9 @@ typedef enum {
 #define TCG_FEATURE_OPAL_SSC_V2_0_0 (UINT16)0x0203
 #define TCG_FEATURE_OPAL_SSC_LITE   (UINT16)0x0301
 #define TCG_FEATURE_PYRITE_SSC  (UINT16)0x0302
+#define TCG_FEATURE_PYRITE_SSC_V2_0_0   (UINT16)0x0303
 #define TCG_FEATURE_BLOCK_SID   (UINT16)0x0402
+#define TCG_FEATURE_DATA_REMOVAL(UINT16)0x0404
 
 // ACE Expression values
 #define TCG_ACE_EXPRESSION_AND 0x0
diff --git a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h 
b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
index 91d5008c05..8ff36efe50 100644
--- a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
+++ b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
@@ -34,6 +34,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define OPAL_ADMIN_SP_ACTIVATE_METHOD   TCG_TO_UID(0x00, 0x00, 0x00, 0x06, 
0x00, 0x00, 0x02, 0x03)
 #define OPAL_ADMIN_SP_REVERT_METHOD TCG_TO_UID(0x00, 0x00, 0x00, 0x06, 
0x00, 0x00, 0x02, 0x02)
 
+// ADMIN_SP
+// Data Removal mechanism
+#define OPAL_UID_ADMIN_SP_DATA_REMOVAL_MECHANISM  TCG_TO_UID(0x00, 0x00, 0x11, 
0x01, 0x00, 0x00, 0x00, 0x01)
 
 // LOCKING SP
 // Authorities
@@ -93,6 +96,23 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define OPAL_LOCKING_SP_C_PIN_TRYLIMIT_COL 5
 #define OPAL_RANDOM_METHOD_MAX_COUNT_SIZE 32
 
+// Data Removal Mechanism column.
+#define OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL  1
+
+//
+// Supported Data Removal Mechanism.
+// Detail see Pyrite SSC v2 spec.
+//
+typedef enum {
+  OverwriteDataErase = 0,
+  BlockErase,
+  CryptoErase,
+  Unmap,
+  ResetWritePointers,
+  VendorSpecificErase,
+  ResearvedMechanism
+} SUPPORTED_DATA_REMOVAL_MECHANISM;
+
 #pragma pack(1)
 
 typedef struct _OPAL_GEOMETRY_REPORTING_FEATURE {
@@ -162,6 +182,38 @@ typedef struct _PYRITE_SSC_FEATURE_DESCRIPTOR {
   UINT8Future[5];
 } PYRITE_SSC_FEATURE_DESCRIPTOR;
 
+typedef struct _PYRITE_SSCV2_FEATURE_DESCRIPTOR {
+  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
+  UINT16   BaseComdIdBE;
+  UINT16   NumComIdsBE;
+  UINT8Reserved[5];
+  UINT8InitialCPINSIDPIN;
+  UINT8CPINSIDPINRevertBehavior;
+  UINT8Future[5];
+} PYRITE_SSCV2_FEATURE_DESCRIPTOR;
+
+typedef struct _DATA_REMOVAL_FEATURE_DESCRIPTOR {
+  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
+  UINT8Reserved;
+  UINT8OperationProcessing : 1;
+  UINT8Reserved2 : 7;
+  UINT8RemovalMechanism;
+  UINT8FormatBit0 : 1;   // Data Removal Time 
Format for Bit 0
+  UINT8FormatBit1 : 1;   // Data Removal Time 
Format for Bit 1
+  UINT8FormatBit2 : 1;   // Data Removal Time 
Format for Bit 2
+  UINT8FormatBit3 : 1;   // Data Removal Time 
Format for Bit 3
+  UINT8FormatBit4 : 1;   // Data Removal Time 
Format for Bit 4
+  UINT8FormatBit5 : 1;   // Data Removal Time 
Format for Bit 5
+  UINT8Reserved3 : 2;
+  UINT16   TimeBit0; // Data Removal Time 
for Supported Data Removal Mechanism Bit 0
+  UINT16   TimeBit1; // Data Removal Time 
for Supported Data Removal Mechanism Bit 1
+  UINT16   TimeBit2; // Data Removal Time 
for Supported Data Removal Mechanism Bit 2
+  UINT16   TimeBit3; // Data Removal Time 
for Supported Data Removal Mechanism Bit 3
+  UINT16   TimeBit4; // Data Removal Time 
for Supported Data Removal Mechanism Bit 4
+  UINT16   TimeBit5; // Data Removal Time 
for Supported Data Removal Mechanism Bit 5
+  UINT8Future[16];
+} DATA_REMOVAL_FEATURE_DESCRIPTOR;
+
 typedef union {
   TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER CommonHeader;
   TCG_TPER_FEATURE_DESCRIPTOR  Tper;
@@ -173,7 +225,9 @@ typedef union {
   OPAL_SSCV2_FEATURE_DESCRIPTOROpalSscV2;
  

Re: [edk2] Set "db" variable in secure boot setup mode still requires generating PKCS#7?

[Long, Qin]

Hi, David,

Yes, in Setup / Custom mode, no need to generate the AuthData for verification. 
It's good enough to create the AUTH_2 descriptor / headers without CertData as 
the parameter for SetVariable() call.

Do you mean this code snippet can succeed to enroll KEK, but fail to enroll DB 
data?
The data initialization from code snippet looks good. What's the returned 
errcode value? (And one reminder is that KEK and DB are binding with different 
vendor GUID: gEfiGlobalVariableGuid, and gEfiImageSecurityDatabaseGuid).


Best Regards & Thanks,
LONG, Qin

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of David F.
Sent: Thursday, May 3, 2018 12:26 AM
To: Laszlo Ersek 
Cc: edk2 developers list 
Subject: Re: [edk2] Set "db" variable in secure boot setup mode still requires 
generating PKCS#7?

This Intel mobo didn't like?  This is the code snippet that builds it:

// calc size of header (with no certdata) and crt file data to add
size_t authhdrsize;
size_t siglisthdrsize;

if (applyrawdata) {
  authhdrsize=0;
  siglisthdrsize=0;
}
else {
  authhdrsize=offsetof(EFI_VARIABLE_AUTHENTICATION_2,
AuthInfo)+offsetof(WIN_CERTIFICATE_UEFI_GUID, CertData);
  siglisthdrsize=sizeof(EFI_SIGNATURE_LIST)+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
}
size_t tempbufsize=ffinfo.FileSize+authhdrsize+siglisthdrsize;

BYTE *tempbuf;
if ((tempbuf=new BYTE [tempbufsize])!=NULL) {
  // variable to determine where to read file
  BYTE *certdata=tempbuf;
  // determine if need to prefix .crt for kek/db entries
  if (!applyrawdata) {
// zero header part of buffer so all are init to zero
memset(tempbuf, 0, authhdrsize+siglisthdrsize);
//
// setup EFI_VARIABLE_AUTHENTICATION_2  header
//
EFI_VARIABLE_AUTHENTICATION_2
*efivarauth2=(EFI_VARIABLE_AUTHENTICATION_2 *) tempbuf;
// setup time
TimeTToUEFITimeGMT(time(NULL), >TimeStamp);
efivarauth2->TimeStamp.Nanosecond=0;
// setup authinfo (without any CertData)
efivarauth2->AuthInfo.Hdr.dwLength=offsetof(WIN_CERTIFICATE_UEFI_GUID,
CertData);
efivarauth2->AuthInfo.Hdr.wRevision=0x200;
efivarauth2->AuthInfo.Hdr.wCertificateType=WIN_CERT_TYPE_EFI_GUID;
efivarauth2->AuthInfo.CertType=gEfiCertPkcs7Guid;
//
// setup EFI_SIGNATURE_LIST
//
EFI_SIGNATURE_LIST *efisiglist=(EFI_SIGNATURE_LIST *)
(tempbuf+authhdrsize);
efisiglist->SignatureType=gEfiCertX509Guid;

efisiglist->SignatureListSize=(uint32_t)(ffinfo.FileSize+siglisthdrsize);
efisiglist->SignatureHeaderSize=0;
efisiglist->SignatureSize=ffinfo.FileSize+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
//
// setup EFI_SIGNATURE_DATA  (no owner)
//
EFI_SIGNATURE_DATA *efisigdata=(EFI_SIGNATURE_DATA *)
((BYTE*)efisiglist+sizeof(EFI_SIGNATURE_LIST)+efisiglist->SignatureHeaderSize);
certdata=efisigdata->SignatureData;
  }
  // Read file to buffer
  if ((errcode=FSOpenReadCloseFile(openpath, certdata, 0, ffinfo.FileSize,
NULL, filesys))==ERROR_NONE) {
// have the data, now write it to the correct variable
uint32_t varattr=EFI_VARIABLE_NON_VOLATILE|
 EFI_VARIABLE_BOOTSERVICE_ACCESS|
 EFI_VARIABLE_RUNTIME_ACCESS|
 EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
if (!rparam) {
  varattr|=EFI_VARIABLE_APPEND_WRITE;
}

// update variable
errcode=UEFISetVariable(varname, guidstr, tempbuf, tempbufsize,
varattr);
  }
  // clean up
  delete[] tempbuf;
}


On Wed, May 2, 2018 at 3:21 AM, Laszlo Ersek 
> wrote:

> On 05/01/18 23:13, David F. wrote:
> > Hi,
> >
> > Had a fairly simple task of wanting to install the latest MS .crt
> > files for KEK, and their two files for the "db" (the Windows CA and
> > UEFI CA) in a system placed in setup/custom mode.  However, even
> > though it seemed to take the KEK, it never took the "db", always had a
> > problem on a DH77KC mobo (dumped data headers looked as expected). Now
> > when I constructed it, I thought I could leave out any PKCS#7 data
> > (set the expected CertType but in the Hdr dwLength only included
> > CertType and not any CertData),
>
> Right, I've stumbled upon that too. According to the UEFI spec, dwLength
> should include CertData too, but edk2 does *not* accept that. This can
> be seen e.g. in
> "SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigImpl.c",
> function CreateTimeBasedPayload():
>
> >   //
> >   // In Setup mode or Custom mode, the variable does not need to be
> signed but the
> >   // parameters to the SetVariable() call still need to be prepared as
> authenticated
> >   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> without certificate
> >   // data in it.
> >   //
> > ...
> >   DescriptorData->AuthInfo.Hdr.dwLength = OFFSET_OF
> (WIN_CERTIFICATE_UEFI_GUID, CertData);
>
> Back to your email:
>
> On 05/01/18 23:13, David F. wrote:
> > but looking 

[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Fix format of ReadMe.MD

[Guo, Mang]

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Guo Mang 
---
 Platform/ReadMe.MD | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Platform/ReadMe.MD b/Platform/ReadMe.MD
index b0341a5..814eaa2 100644
--- a/Platform/ReadMe.MD
+++ b/Platform/ReadMe.MD
@@ -92,14 +92,14 @@ Intel(R) Firmware Support Package(Intel(R) FSP)
 1. Open command window, goto the workspace dir, e.g. c:\MyWorkspace.
 2. Type "cd edk2-platforms".
 3. Build
-
+   ```
   Examples:  
To build release version 32-bit BIOS for Minnowboard 3 FAB B with 
VS2013,
  BuildBIOS.bat /vs13 /B /IA32 Broxton Release
 
To build release version 64-bit BIOS for Minnowboard 3 FAB B with 
VS2013,
  BuildBIOS.bat /vs13 /B /x64 Broxton Release
-
+   ```
 ## **Linux Build Instructions**
 
 ### Pre-requisites
@@ -120,8 +120,10 @@ Intel(R) Firmware Support Package(Intel(R) FSP)
 
 1. Open a command prompt, goto the platform package 
"MyWorkspace/edk2-platforms".
 2. Build
+   ```
- Type "./BuildBIOS.sh /B Broxton Release" to build a release version.
- Type "./BuildBIOS.sh /B Broxton Debug" to build a debug version.
+   ```

 ## **Related Materials**
 
-- 
2.10.1.windows.1

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

Re: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

[Dong, Eric]

Hi Thomas,

Thanks for your patches. We just finished internal verification of them.

Reviewed-by: Eric Dong  and pushed them.

Thanks,
Eric

-Original Message-
From: Palmer, Thomas [mailto:thomas.pal...@hpe.com] 
Sent: Saturday, April 21, 2018 12:00 AM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Wang, Nickle (HPS SW) ; Gao, Liming 
; Dong, Eric ; Zeng, Star 
; Zhang, Chao B 
Subject: RE: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: 
Update RouteConfig function

I have no opinion / please the 5th.  I defer to the experts.


Regards,

Thomas Palmer

"I have only made this letter longer because I have not had the time to make it 
shorter" - Blaise Pascal


-Original Message-
From: Bi, Dandan [mailto:dandan...@intel.com] 
Sent: Friday, April 20, 2018 2:34 AM
To: Palmer, Thomas ; edk2-devel@lists.01.org
Cc: Wang, Nickle (HPS SW) ; Gao, Liming 
; Dong, Eric ; Zeng, Star 
; Zhang, Chao B 
Subject: RE: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: 
Update RouteConfig function

Thanks for the updating. These changes make sense.
Reviewed-by: Dandan Bi   for this patch series.

But the Spec seems not to be clear enough.
When looking into details about the "progress" parameter in EFI HII 
Configuration Routing Protocol and  EFI_HII_CONFIG_ACCESS_PROTOCOL.

Description of "progress" parameter in ExtractConfig() in UEFI 2.7 Spec:
Progress
On return, points to a character in the Request string. Points to the string's 
null terminator if request was successful. Points to the most recent '&' before 
the first failing name / value pair (or the beginning of the string if the 
failure is in the first name / value pair) if the request was not successful

EFI_NOT_FOUND
A configuration element matching the routing data is not found. Progress set to 
the first character in the routing header.


Description of "progress" parameter in RouteConfig () in UEFI 2.7 Spec:
Progress
a pointer to a string filled in with the offset of the most recent '&' before 
the first failing name / value pair (or the beginning of the string if the 
failure is in the first name / value pair) or the terminating NULL if all was 
successful.

EFI_NOT_FOUND
Target for the specified routing data was not found.

Compared with ExtractConfig(), the description of "Progress" parameter in 
RouteConfig()  is not very clear. 
We think an ECR is nice to have to clarify them. What do you think?


Thanks,
Dandan

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Thomas 
Palmer
Sent: Thursday, April 19, 2018 4:32 AM
To: edk2-devel@lists.01.org
Cc: nickle.w...@hpe.com; Gao, Liming 
Subject: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: 
Update RouteConfig function

According to UEFI spec, the RouteConfig protocol function should populate the 
Progress pointer with an address inside Configuration.  This patch ensures that 
these functions are compliant when EFI_NOT_FOUND is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer 
---
 .../Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c   | 3 +++
 1 file changed, 3 insertions(+)

diff --git 
a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c 
b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
index a4828b7130c7..3092184ab760 100644
--- a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
+++ b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMai
+++ ntUi.c
@@ -2,6 +2,7 @@
   Legacy Boot Maintainence UI implementation.
 
 Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+(C) Copyright 2018 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -563,6 +564,8 @@ 
LegacyBootOptionRouteConfig (
 return EFI_INVALID_PARAMETER;
   }
 
+  *Progress = Configuration;
+
   //
   // Check routing data in .
   // Note: there is no name for Name/Value storage, only GUID will be checked
--
2.7.4

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

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

Re: [edk2] [PATCH 1/1] MdeModulePkg/Sd: append CMD12 for multiple blocks

[Wu, Hao A]

Hi Haojian,

Sorry for the delayed response.

As far as I know, we enabled the auto CMD12 feature within:
SdMmcExecTrb(), MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c

//
// Only SD memory card needs to use AUTO CMD12 feature.
//
if (Private->Slot[Trb->Slot].CardType == SdCardType) {
  if (BlkCount > 1) {
TransMode |= BIT2;
  }
}

So I think the explicitly sending CMD12 in SdRwMultiBlocks() is not needed.

Have you met problems when using SdRwMultiBlocks()? Could you help to
provide detailed device information when you meet the problem?

Thanks in advance.

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Haojian Zhuang
> Sent: Saturday, April 28, 2018 1:24 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Dong, Eric; Ard Biesheuvel; Leif Lindholm; Haojian Zhuang; 
> Zeng,
> Star
> Subject: [edk2] [PATCH 1/1] MdeModulePkg/Sd: append CMD12 for multiple
> blocks
> 
> Send CMD12 to stop transimission for accessing multiple blocks. It's
> required by SD Card protocol.
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Ruiyu Ni 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c | 35
> +++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> index 516c3e704288..64259f99f9bc 100644
> --- a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> @@ -426,6 +426,36 @@ Error:
>return Status;
>  }
> 
> +EFI_STATUS
> +SdStopTrans (
> +  IN  SD_DEVICE *Device
> +  )
> +{
> +  EFI_STATUS   Status;
> +  EFI_SD_MMC_PASS_THRU_PROTOCOL*PassThru;
> +  EFI_SD_MMC_COMMAND_BLOCK SdMmcCmdBlk;
> +  EFI_SD_MMC_STATUS_BLOCK  SdMmcStatusBlk;
> +  EFI_SD_MMC_PASS_THRU_COMMAND_PACKET  Packet;
> +
> +  PassThru = Device->Private->PassThru;
> +
> +  ZeroMem (, sizeof (SdMmcCmdBlk));
> +  ZeroMem (, sizeof (SdMmcStatusBlk));
> +  ZeroMem (, sizeof (Packet));
> +
> +  Packet.SdMmcCmdBlk= 
> +  Packet.SdMmcStatusBlk = 
> +  Packet.Timeout= SD_GENERIC_TIMEOUT;
> +
> +  SdMmcCmdBlk.CommandIndex = SD_STOP_TRANSMISSION;
> +  SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
> +  SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
> +  SdMmcCmdBlk.CommandArgument = 0;
> +
> +  Status = PassThru->PassThru (PassThru, Device->Slot, , NULL);
> +  return Status;
> +}
> +
>  /**
>Read/write multiple blocks through sync or async I/O request.
> 
> @@ -555,6 +585,11 @@ Error:
>  }
>}
> 
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  Status = SdStopTrans (Device);
>return Status;
>  }
> 
> --
> 2.7.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] MdeModulePkg/Core: Fix MaxAddress in Gcd

[Zeng, Star]

The GcdAllocateType are EfiGcdAllocateMaxAddress*, the MaxAddress comes from 
input BaseAddress parameter.

The original code logic is correct according to PI spec.

PI Spec:
If GcdAllocateType is EfiGcdAllocateMaxAddressSearchBottomUp, then the GCD
memory space map is searched from the lowest address up to BaseAddress looking 
for
unallocated memory ranges of Length bytes beginning on a boundary specified by 
Alignment
that matches GcdMemoryType.
If GcdAllocateType is EfiGcdAllocateMaxAddressSearchTopDown, then the GCD
memory space map is searched from BaseAddress down to the lowest address 
looking for
unallocated memory ranges of Length bytes beginning on a boundary specified by 
Alignment
that matches GcdMemoryType.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Roman 
Bacik
Sent: Thursday, May 3, 2018 2:01 AM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Vladimir Olovyannikov 

Subject: [edk2] [PATCH v2] MdeModulePkg/Core: Fix MaxAddress in Gcd

When BottomUp search is used the MaxAddress is incorrectly chosen to
be the BaseAddress instead of the EndAddress.

Cc: Ruiyu Ni 
Cc: Vladimir Olovyannikov 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Roman Bacik 
---
MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index e17e98230b79..9eeb2bd74599 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -1170,8 +1170,8 @@ CoreAllocateSpace (
//
// Compute the maximum address to use in the search algorithm
//
- if (GcdAllocateType == EfiGcdAllocateMaxAddressSearchBottomUp ||
- GcdAllocateType == EfiGcdAllocateMaxAddressSearchTopDown ) {
+ if (GcdAllocateType == EfiGcdAllocateMaxAddressSearchTopDown ||
+ GcdAllocateType == EfiGcdAllocateAnySearchTopDown ) {
MaxAddress = *BaseAddress;
} else {
MaxAddress = Entry->EndAddress;
-- 
1.9.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 edk-platforms 4/4] Platform/Hisilicon/HiKey: enable virtual keyboard

[Leif Lindholm]

On Thu, Mar 08, 2018 at 09:30:56PM +0800, Haojian Zhuang wrote:
> Enable virtual keyboard driver on HiKey platform. The platform
> driver reads pattern from memory or GPIO pin. When the value
> is matched, it simulates a key value that is used to adjust
> the sequence of boot options.

The addition of HiKeyDxe and the enabling of virtual keyboard support
should be separate patches, just like for 2/4.


> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  Platform/Hisilicon/HiKey/HiKey.dsc|   8 +
>  Platform/Hisilicon/HiKey/HiKey.fdf|   8 +
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf|  57 +
>  Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h |  48 
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c  | 229 
>  5 files changed, 350 insertions(+)
> 
> diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc 
> b/Platform/Hisilicon/HiKey/HiKey.dsc
> index 5c1604d7f689..83dd68a820b1 100644
> --- a/Platform/Hisilicon/HiKey/HiKey.dsc
> +++ b/Platform/Hisilicon/HiKey/HiKey.dsc
> @@ -189,9 +189,17 @@ [Components.common]
>#
># GPIO
>#
> +  Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
>ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
>  
>#
> +  # Virtual Keyboard
> +  #
> +  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
> +
> +  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
> +
> +  #
># MMC/SD
>#
>EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
> diff --git a/Platform/Hisilicon/HiKey/HiKey.fdf 
> b/Platform/Hisilicon/HiKey/HiKey.fdf
> index 2a5c5a4d6e79..2bca7232b6e5 100644
> --- a/Platform/Hisilicon/HiKey/HiKey.fdf
> +++ b/Platform/Hisilicon/HiKey/HiKey.fdf
> @@ -120,9 +120,17 @@ [FV.FvMain]
>#
># GPIO
>#
> +  INF Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
>INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
>  
>#
> +  # Virtual Keyboard
> +  #
> +  INF EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
> +
> +  INF Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
> +
> +  #
># Multimedia Card Interface
>#
>INF EmbeddedPkg/Universal/MmcDxe/MmcDxe.inf
> diff --git a/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf 
> b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
> new file mode 100644
> index ..702fdb1eebf0
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
> @@ -0,0 +1,57 @@
> +#
> +#  Copyright (c) 2013 - 2014, ARM Ltd. All rights reserved.
> +#  Copyright (c) 2018, Linaro Ltd. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  INF_VERSION= 0x00010005

0x0001001a

> +  BASE_NAME  = HiKeyDxe
> +  FILE_GUID  = f567684b-1089-4214-8881-d64b20cbda2f
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 1.0
> +  ENTRY_POINT= HiKeyEntryPoint
> +
> +[Sources.common]
> +  HiKeyDxe.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

MdeM... before MdeP...

> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  CacheMaintenanceLib
> +  DebugLib
> +  DxeServicesLib
> +  DxeServicesTableLib
> +  FdtLib
> +  IoLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiRuntimeServicesTableLib
> +  UefiLib
> +  UefiDriverEntryPoint

Are all of the above used by this driver?

> +
> +[Protocols]
> +  gEmbeddedGpioProtocolGuid
> +  gPlatformVirtualKeyboardProtocolGuid
> +
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiFileInfoGuid

I can't find any references to gEfiFileInfoGuid in this patch.

> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h 
> b/Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h
> new file mode 100644
> index ..8419685611bf
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi6220/Include/Hi6220RegsPeri.h
> @@ -0,0 +1,48 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Linaro Ltd. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS 

Re: [edk2] [PATCH v2 edk-platforms 2/4] Platform/Hisilicon/HiKey960: enable virtual keyboard

[Leif Lindholm]

On Thu, Mar 08, 2018 at 09:30:04PM +0800, Haojian Zhuang wrote:
> Enable virtual keyboard on HiKey960 platform. The platform
> driver read pattern from memory or GPIO pin. When the value
> is matched, it simulates a hotkey that is used to adjust
> sequence of boot options.

This patch looks like it contains an awful lot more than described
here. Can it be split up?

> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  Platform/Hisilicon/HiKey960/HiKey960.dsc|  10 +
>  Platform/Hisilicon/HiKey960/HiKey960.fdf|   7 +
>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  54 +++
>  Silicon/Hisilicon/Hi3660/Include/Hi3660.h   | 147 ++
>  Silicon/Hisilicon/Hi3660/Include/Hkadc.h|  66 +++
>  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 491 
> 
>  6 files changed, 775 insertions(+)
> 
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
> b/Platform/Hisilicon/HiKey960/HiKey960.dsc
> index 3da1b8556321..859ab84f8415 100644
> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
> @@ -68,6 +68,9 @@ [LibraryClasses.common.SEC]
>PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>
> PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>  
> +[BuildOptions]
> +  GCC:*_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Silicon/Hisilicon/Hi3660/Include

Please add a Hi3660/Hi3660.dec instead, like for Hi6220.

> +
>  
> 
>  #
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> @@ -183,6 +186,13 @@ [Components.common]
>ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
>  
>#
> +  # Virtual Keyboard
> +  #
> +  EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
> +
> +  Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
> +
> +  #
># USB Host Support
>#
>MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf 
> b/Platform/Hisilicon/HiKey960/HiKey960.fdf
> index 162dbaaf2646..d65f77878575 100644
> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
> @@ -124,6 +124,13 @@ [FV.FvMain]
>INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
>  
>#
> +  # Virtual Keyboard
> +  #
> +  INF EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboardDxe.inf
> +
> +  INF Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
> +
> +  #
># USB Host Support
>#
>INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf 
> b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
> new file mode 100644
> index ..cc517656b340
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
> @@ -0,0 +1,54 @@
> +#
> +#  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  INF_VERSION= 0x00010019

0x0001001a

> +  BASE_NAME  = HiKey960Dxe
> +  FILE_GUID  = 6d824b2c-640e-4643-b9f2-9c09e8bff429
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 1.0
> +  ENTRY_POINT= HiKey960EntryPoint
> +
> +[Sources.common]
> +  HiKey960Dxe.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

MdeM... before MdeP...

> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  CacheMaintenanceLib
> +  DebugLib
> +  DxeServicesTableLib
> +  FdtLib
> +  IoLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +  UefiRuntimeServicesTableLib

Are all of these really used?

> +
> +[Protocols]
> +  gEmbeddedGpioProtocolGuid
> +  gPlatformVirtualKeyboardProtocolGuid
> +
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiFileInfoGuid

I don't see gEfiFileInfoGuid referenced anywhere else in this patch?

> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/Hisilicon/Hi3660/Include/Hi3660.h 
> b/Silicon/Hisilicon/Hi3660/Include/Hi3660.h
> new file mode 100644
> index ..f3ce12f64ed5
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi3660/Include/Hi3660.h
> 

Re: [edk2] Incorrect Author on patch

[Rebecca Cran]

On 5/2/2018 7:40 AM, Laszlo Ersek wrote:


On 05/02/18 14:46, Evan Lloyd wrote:

  I can well understand why it would be useful to use Gerrit as a means
  of reviewing a patch - actually a brilliant idea,

(actually, *not* a brilliant idea, but that's just my opinion :) )


*Please* not Gerrit. If we're going to use a code review system, please 
let us choose Phabricator. It has a much nicer user interface.
You can see an example of the review requests it creates at 
https://reviews.freebsd.org/differential/ .


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

[edk2] [PATCH v2] MdeModulePkg/Bus: Enable ascending resource list

[Roman Bacik]

Some processors require resource list sorted in ascending order.

Cc: Ruiyu Ni 
Cc: Vladimir Olovyannikov 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Roman Bacik 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf   |  1 +
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 43 ++
 MdeModulePkg/MdeModulePkg.dec  |  3 ++
 MdeModulePkg/MdeModulePkg.dsc  |  1 +
 4 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index 97608bf..5cb3761 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -110,6 +110,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdAriSupport  ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration##
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdListAscending   ## CONSUMES

 [UserExtensions.TianoCore."ExtraFiles"]
   PciBusDxeExtra.uni
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index 2f713fc..45575fa 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -106,19 +106,30 @@ InsertResourceNode (
   PCI_RESOURCE_NODE *Temp;
   UINT64ResNodeAlignRest;
   UINT64TempAlignRest;
+  BOOLEAN   Ascending;

   ASSERT (Bridge  != NULL);
   ASSERT (ResNode != NULL);

-  InsertHeadList (>ChildList, >Link);
+  Ascending = FeaturePcdGet (PcdListAscending);
+
+  if (!Ascending) {
+InsertHeadList (>ChildList, >Link);
+CurrentLink = Bridge->ChildList.ForwardLink->ForwardLink;
+  } else {
+CurrentLink = Bridge->ChildList.BackLink;
+InsertTailList (>ChildList, >Link);
+  }

-  CurrentLink = Bridge->ChildList.ForwardLink->ForwardLink;
   while (CurrentLink != >ChildList) {
 Temp = RESOURCE_NODE_FROM_LINK (CurrentLink);

-if (ResNode->Alignment > Temp->Alignment) {
+if ((Ascending && Temp->Alignment >= ResNode->Alignment) ||
+   (!Ascending && ResNode->Alignment > Temp->Alignment)) {
   break;
-} else if (ResNode->Alignment == Temp->Alignment) {
+}
+
+if (!Ascending && ResNode->Alignment == Temp->Alignment) {
   ResNodeAlignRest  = ResNode->Length & ResNode->Alignment;
   TempAlignRest = Temp->Length & Temp->Alignment;
   if ((ResNodeAlignRest == 0) || (ResNodeAlignRest >= TempAlignRest)) {
@@ -128,7 +139,11 @@ InsertResourceNode (

 SwapListEntries (>Link, CurrentLink);

-CurrentLink = ResNode->Link.ForwardLink;
+if (Ascending) {
+  CurrentLink = ResNode->Link.BackLink;
+} else {
+  CurrentLink = ResNode->Link.ForwardLink;
+}
   }
 }

@@ -1269,6 +1284,7 @@ ProgramBar (
   EFI_PCI_IO_PROTOCOL *PciIo;
   UINT64  Address;
   UINT32  Address32;
+  BOOLEAN   Ascending;

   ASSERT (Node->Bar < PCI_MAX_BAR);

@@ -1282,6 +1298,7 @@ ProgramBar (

   Address = 0;
   PciIo   = &(Node->PciDev->PciIo);
+  Ascending = FeaturePcdGet (PcdListAscending);

   Address = Base + Node->Offset;

@@ -1300,6 +1317,10 @@ ProgramBar (
   case PciBarTypeMem32:
   case PciBarTypePMem32:

+if (Ascending) {
+  Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
+  Address %= Base;
+}
 PciIo->Pci.Write (
  PciIo,
  EfiPciIoWidthUint32,
@@ -1308,13 +1329,19 @@ ProgramBar (
  
  );

-Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
+if (!Ascending) {
+  Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
+}

 break;

   case PciBarTypeMem64:
   case PciBarTypePMem64:

+if (Ascending) {
+  Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
+  Address %= Base;
+}
 Address32 = (UINT32) (Address & 0x);

 PciIo->Pci.Write (
@@ -1335,7 +1362,9 @@ ProgramBar (
  
  );

-Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
+if (!Ascending) {
+  Node->PciDev->PciBar[Node->Bar].BaseAddress = Address;
+}

 break;

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index cc39718..911f33a 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1005,6 +1005,9 @@
   # @Prompt Enable UEFI Stack Guard.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055

+  ## Indicates if the resource list is sorted in ascending order
+  gEfiMdeModulePkgTokenSpaceGuid.PcdListAscending|FALSE|BOOLEAN|0x30001056
+
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Dynamic type PCD can be registered callback function for Pcd
setting action.
   #  

[edk2] [PATCH v2] MdeModulePkg/Core: Fix MaxAddress in Gcd

[Roman Bacik]

When BottomUp search is used the MaxAddress is incorrectly chosen to
be the BaseAddress instead of the EndAddress.

Cc: Ruiyu Ni 
Cc: Vladimir Olovyannikov 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Roman Bacik 
---
MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index e17e98230b79..9eeb2bd74599 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -1170,8 +1170,8 @@ CoreAllocateSpace (
//
// Compute the maximum address to use in the search algorithm
//
- if (GcdAllocateType == EfiGcdAllocateMaxAddressSearchBottomUp ||
- GcdAllocateType == EfiGcdAllocateMaxAddressSearchTopDown ) {
+ if (GcdAllocateType == EfiGcdAllocateMaxAddressSearchTopDown ||
+ GcdAllocateType == EfiGcdAllocateAnySearchTopDown ) {
MaxAddress = *BaseAddress;
} else {
MaxAddress = Entry->EndAddress;
-- 
1.9.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH edk2-platforms v1] Platform/ARM: Correct GIC naming

[Alexei Fedorov]

From: Alexei Fedorov 

ARM Generic Interrupt Controller is incorrectly named as
"ARM General Interrupt Controller" in ArmJuno.dsc,
ArmVExpress-CTA15-A7.dsc and ArmVExpress-FVP-AArch64.dsc.

This patch corrects the comment by changing "General" to "Generic".

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Alexei Fedorov 
Reviewed-by:   Evan Lloyd 
---
All the changes can be reviewed at:
https://github.com/AlexeiFedorov/edk2-platforms/tree/261_correct_gic_naming_v1

Notes:
v1:
- Change ARM "General" Interrupt Controller to "Generic" [Alexei]

 Platform/ARM/JunoPkg/ArmJuno.dsc | 2 +-
 Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc| 4 ++--
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
index 
5ec563cefbcd9d97614dc9944bfaeb3822c4634d..8d6dd0207db5f16d7fcc0c5fd59fea38498f3034
 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dsc
+++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
@@ -150,7 +150,7 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout|4
 
   #
-  # ARM General Interrupt Controller
+  # ARM Generic Interrupt Controller
   #
   gArmTokenSpaceGuid.PcdGicDistributorBase|0x2C01
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x2C02F000
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc 
b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc
index 
ef737670126db980abcc0e0ac9b84321bfa97584..6405d955f15a87c8d7b379f8c05b43f3e0830c54
 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2012-2017, ARM Limited. All rights reserved.
+#  Copyright (c) 2012-2018, ARM Limited. All rights reserved.
 #  Copyright (c) 2015, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
@@ -156,7 +156,7 @@ [PcdsFixedAtBuild.common]
 
 
   #
-  # ARM General Interrupt Controller
+  # ARM Generic Interrupt Controller
   #
   gArmTokenSpaceGuid.PcdGicDistributorBase|0x2C001000
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x2C002000
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
index 
600bffb66703305f4a6baec930cd044998744c24..ca19fd65265a531e8c0869460aa9b6358b70bf91
 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
@@ -147,7 +147,7 @@ [PcdsFixedAtBuild.common]
   gArmPlatformTokenSpaceGuid.PcdPL180MciBaseAddress|0x1C05
 
   #
-  # ARM General Interrupt Controller
+  # ARM Generic Interrupt Controller
   #
   gArmTokenSpaceGuid.PcdGicDistributorBase|0x2f00
   gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x2f10
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

Re: [edk2] Set "db" variable in secure boot setup mode still requires generating PKCS#7?

[David F.]

This Intel mobo didn't like?  This is the code snippet that builds it:

// calc size of header (with no certdata) and crt file data to add
size_t authhdrsize;
size_t siglisthdrsize;

if (applyrawdata) {
  authhdrsize=0;
  siglisthdrsize=0;
}
else {
  authhdrsize=offsetof(EFI_VARIABLE_AUTHENTICATION_2,
AuthInfo)+offsetof(WIN_CERTIFICATE_UEFI_GUID, CertData);
  siglisthdrsize=sizeof(EFI_SIGNATURE_LIST)+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
}
size_t tempbufsize=ffinfo.FileSize+authhdrsize+siglisthdrsize;

BYTE *tempbuf;
if ((tempbuf=new BYTE [tempbufsize])!=NULL) {
  // variable to determine where to read file
  BYTE *certdata=tempbuf;
  // determine if need to prefix .crt for kek/db entries
  if (!applyrawdata) {
// zero header part of buffer so all are init to zero
memset(tempbuf, 0, authhdrsize+siglisthdrsize);
//
// setup EFI_VARIABLE_AUTHENTICATION_2  header
//
EFI_VARIABLE_AUTHENTICATION_2
*efivarauth2=(EFI_VARIABLE_AUTHENTICATION_2 *) tempbuf;
// setup time
TimeTToUEFITimeGMT(time(NULL), >TimeStamp);
efivarauth2->TimeStamp.Nanosecond=0;
// setup authinfo (without any CertData)
efivarauth2->AuthInfo.Hdr.dwLength=offsetof(WIN_CERTIFICATE_UEFI_GUID,
CertData);
efivarauth2->AuthInfo.Hdr.wRevision=0x200;
efivarauth2->AuthInfo.Hdr.wCertificateType=WIN_CERT_TYPE_EFI_GUID;
efivarauth2->AuthInfo.CertType=gEfiCertPkcs7Guid;
//
// setup EFI_SIGNATURE_LIST
//
EFI_SIGNATURE_LIST *efisiglist=(EFI_SIGNATURE_LIST *)
(tempbuf+authhdrsize);
efisiglist->SignatureType=gEfiCertX509Guid;

efisiglist->SignatureListSize=(uint32_t)(ffinfo.FileSize+siglisthdrsize);
efisiglist->SignatureHeaderSize=0;
efisiglist->SignatureSize=ffinfo.FileSize+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
//
// setup EFI_SIGNATURE_DATA  (no owner)
//
EFI_SIGNATURE_DATA *efisigdata=(EFI_SIGNATURE_DATA *)
((BYTE*)efisiglist+sizeof(EFI_SIGNATURE_LIST)+efisiglist->SignatureHeaderSize);
certdata=efisigdata->SignatureData;
  }
  // Read file to buffer
  if ((errcode=FSOpenReadCloseFile(openpath, certdata, 0, ffinfo.FileSize,
NULL, filesys))==ERROR_NONE) {
// have the data, now write it to the correct variable
uint32_t varattr=EFI_VARIABLE_NON_VOLATILE|
 EFI_VARIABLE_BOOTSERVICE_ACCESS|
 EFI_VARIABLE_RUNTIME_ACCESS|
 EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
if (!rparam) {
  varattr|=EFI_VARIABLE_APPEND_WRITE;
}

// update variable
errcode=UEFISetVariable(varname, guidstr, tempbuf, tempbufsize,
varattr);
  }
  // clean up
  delete[] tempbuf;
}


On Wed, May 2, 2018 at 3:21 AM, Laszlo Ersek  wrote:

> On 05/01/18 23:13, David F. wrote:
> > Hi,
> >
> > Had a fairly simple task of wanting to install the latest MS .crt
> > files for KEK, and their two files for the "db" (the Windows CA and
> > UEFI CA) in a system placed in setup/custom mode.  However, even
> > though it seemed to take the KEK, it never took the "db", always had a
> > problem on a DH77KC mobo (dumped data headers looked as expected). Now
> > when I constructed it, I thought I could leave out any PKCS#7 data
> > (set the expected CertType but in the Hdr dwLength only included
> > CertType and not any CertData),
>
> Right, I've stumbled upon that too. According to the UEFI spec, dwLength
> should include CertData too, but edk2 does *not* accept that. This can
> be seen e.g. in
> "SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigImpl.c",
> function CreateTimeBasedPayload():
>
> >   //
> >   // In Setup mode or Custom mode, the variable does not need to be
> signed but the
> >   // parameters to the SetVariable() call still need to be prepared as
> authenticated
> >   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> without certificate
> >   // data in it.
> >   //
> > ...
> >   DescriptorData->AuthInfo.Hdr.dwLength = OFFSET_OF
> (WIN_CERTIFICATE_UEFI_GUID, CertData);
>
> Back to your email:
>
> On 05/01/18 23:13, David F. wrote:
> > but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> > always have to generate the hash, sign it, create all the PKCS stuff
> > even in setup mode?That would surely unnecessarily bloat any apps
> > that really only need to update things in setup mode wouldn't it?   So
> > to confirm, that is a requirement even in setup mode?If so, why?
>
> It's not a requirement; see the code comment I quoted above.
>
> Thanks,
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 edk-platforms 3/4] Platform/Hisilicon/HiKey: add gpio platform driver

[Leif Lindholm]

(Reviewing near-identical patches out of order.)

On Thu, Mar 08, 2018 at 09:30:28PM +0800, Haojian Zhuang wrote:
> Add gpio platform driver to enable GPIO in HiKey platform.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf | 36 +++
>  Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c   | 68 
> 
>  2 files changed, 104 insertions(+)
> 
> diff --git a/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf 
> b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
> new file mode 100644
> index ..272ed1c0cea2
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.inf
> @@ -0,0 +1,36 @@
> +#
> +#  Copyright (c) 2018, Linaro. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  INF_VERSION= 0x00010019

0x0001001a?

> +  BASE_NAME  = HiKeyGpio
> +  FILE_GUID  = b51a851c-7bf7-463f-b261-cfb158b7f699
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 1.0
> +  ENTRY_POINT= HiKeyGpioEntryPoint
> +
> +[Sources.common]
> +  HiKeyGpioDxe.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gPlatformGpioProtocolGuid
> +
> +[Depex]
> +  TRUE
> diff --git a/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c 
> b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c
> new file mode 100644
> index ..543f65d7b12d
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey/HiKeyGpioDxe/HiKeyGpioDxe.c
> @@ -0,0 +1,68 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Linaro. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +
> +#include 
> +

Add similar comment as I asked for in 1/4?

> +GPIO_CONTROLLER gGpioDevice[]= {

Missing space before '='.

> +  { 0xf8011000, 0, 8 },// GPIO0
> +  { 0xf8012000, 8, 8 },// GPIO1
> +  { 0xf8013000, 16, 8 },   // GPIO2
> +  { 0xf8014000, 24, 8 },   // GPIO3
> +  { 0xf702, 32, 8 },   // GPIO4
> +  { 0xf7021000, 40, 8 },   // GPIO5
> +  { 0xf7022000, 48, 8 },   // GPIO6
> +  { 0xf7023000, 56, 8 },   // GPIO7
> +  { 0xf7024000, 64, 8 },   // GPIO8
> +  { 0xf7025000, 72, 8 },   // GPIO9
> +  { 0xf7026000, 80, 8 },   // GPIO10
> +  { 0xf7027000, 88, 8 },   // GPIO11
> +  { 0xf7028000, 96, 8 },   // GPIO12
> +  { 0xf7029000, 104, 8 },  // GPIO13
> +  { 0xf702a000, 112, 8 },  // GPIO14
> +  { 0xf702b000, 120, 8 },  // GPIO15
> +  { 0xf702c000, 128, 8 },  // GPIO16
> +  { 0xf702d000, 136, 8 },  // GPIO17
> +  { 0xf702e000, 144, 8 },  // GPIO18
> +  { 0xf702f000, 152, 8 }   // GPIO19
> +};
> +

Add similar comment as I asked for in 1/4?

> +PLATFORM_GPIO_CONTROLLER gPlatformGpioDevice = {
> +  160, 20, gGpioDevice
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +HiKeyGpioEntryPoint (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_HANDLE  Handle;
> +
> +  // Install the Embedded Platform GPIO Protocol onto a new handle
> +  Handle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces(
> +  ,
> +  , ,
> +  NULL
> + );
> +  if (EFI_ERROR(Status)) {
> +Status = EFI_OUT_OF_RESOURCES;

So, I didn't comment on this for 1/4, but ...
Why modify Status?

/
Leif

> +  }
> +
> +  return Status;
> +}
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Incorrect Author on patch

[Gao, Liming]

Evan:
  I agree this is a mistake in edk2 project. We mix the operation in edk2 
project and our internal project. We will double check the patch and avoid such 
case happen again.

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evan 
> Lloyd
> Sent: Wednesday, May 2, 2018 8:46 PM
> To: Laszlo Ersek ; Ni, Ruiyu 
> Cc: edk2-devel (edk2-devel@lists.01.org) 
> Subject: Re: [edk2] Incorrect Author on patch
> 
> One obvious way of precluding this sort of problem would be to move to using 
> the pull request mechanism on GitHub, rather than
> requiring maintainers to play with upstreaming local patches.  I can well 
> understand why it would be useful to use Gerrit as a means of
> reviewing a patch - actually a brilliant idea, but it does introduce 
> problems. If, post review, the merge were  to be from a pull request
> then there would be no risk of "meta-data corruption".
> 
> Regards
> Evan
> 
> > -Original Message-
> > From: Laszlo Ersek 
> > Sent: 25 April 2018 22:15
> > To: Evan Lloyd ; ruiyu...@intel.com
> > Cc: edk2-devel (edk2-devel@lists.01.org) 
> > Subject: Re: [edk2] Incorrect Author on patch
> >
> > On 04/25/18 17:02, Evan Lloyd wrote:
> > > Hi Ruiyu.
> > > When we look at the edk2 git log, or GitHub
> > >
> > https://github.com/tianocore/edk2/commit/ee4dc24f57c32a445e7c747396c
> > 9b
> > > fbd8b221568 we see that Sami's AcpiView patch shows you as the Author.
> > > I'm not sure what might have caused that (and it is obviously 
> > > accidental), but
> > it is a little unfortunate in that the commit doesn't show up on Sami's 
> > GitHub
> > stats.
> > > Fortunately, this is not currently significant for our organisation, 
> > > although I'm
> > sure Sami would prefer to have the credit.
> > > This is a trivial matter, but you may wish to understand what caused it 
> > > so that
> > you don't accidentally upset someone for whom the stats are significant.
> >
> > Right, this occurred at least one other time as well: see commit
> > 8b5c80e0296c ("MdeModulePkg/UefiBootManagerLib: fix
> > AddLoadOptionVariable docs/prototype", 2018-04-23). Ray pushed the patch
> > (so the Committer field is certainly right), but Ray's name+email replaced 
> > mine
> > in the Author field. I had noticed that earlier, but now I'm seeing it as a 
> > pattern.
> >
> > I believe this is a tooling issue. I notice the following bit on the commit
> > message:
> >
> > Change-Id: I8a609d6502b6f8929b2f568acaa147065003b6f4
> >
> > I certainly didn't post the patch with that, and I doubt Ray added it 
> > manually.
> > So, whatever tool Ray used to handle the patch lost the authorship
> > information.
> >
> > And, I see the exact same kind of tag, namely
> >
> > Change-Id: Ifa23dc80ab8ab042c56e88424847e796a8122a7c
> >
> > on commit ee4dc24f57c3 ("ShellPkg: Add acpiview tool to dump ACPI tables",
> > 2018-04-23), which you mention.
> >
> > ... In fact, my patch happens to be the direct ancestor of Sami's, in the 
> > git
> > history, and their commit times are just ~3 minutes apart. I'm quite 
> > certain Ray
> > has started using a new tool.
> >
> > For example, commit bc2288f59ba2 ("UefiCpuPkg/MpInitLib: put
> > mReservedApLoopFunc in executable memory", 2018-03-08) was also
> > committed by Ray, *but* Jian's authorship was preserved fine. (No "Change-
> > Id" either.)
> >
> > ... Oh... it looks like those Change-Id's were added by Gerrit:
> >
> > https://git.eclipse.org/r/Documentation/user-changeid.html
> >
> > And then, please look at this:
> >
> > https://gerrit-review.googlesource.com/Documentation/error-invalid-
> > author.html
> >
> > For every pushed commit Gerrit verifies that the e-mail address of
> > the author matches one of the registered e-mail addresses of the
> > pushing user. If this is not the case pushing the commit fails with
> > the error message "invalid author". This policy can be bypassed by
> > having the access right 'Forge Author'.
> >
> > Uh, what?... Gerrit says "invalid author" if Ray pushes a patch that wasn't
> > authored by himself? And the option to override that is called "forge" 
> > author?
> > O_o
> >
> > Anyway, the page continues,
> >
> > If pushing to Gerrit fails with the error message "invalid author"
> > and somebody else is author of the commit for which the push fails,
> > then you have no permissions to forge the author identity. In this
> > case you may contact the project owner to request the access right
> > '+1 Forge Author Identity' in the 'Forge Identity' category or ask
> > the maintainer to commit this change on the author’s behalf.
> >
> > Ray, if you absolutely must use Gerrit, please make sure that you have the 
> > '+1
> > Forge Author Identity' access right. IMO, we maintainers are responsible for
> > preserving git 

Re: [edk2] [PATCH v2 edk-platforms 1/4] Platform/Hisilicon/HiKey960: add gpio platform driver

[Leif Lindholm]

On Thu, Mar 08, 2018 at 09:29:50PM +0800, Haojian Zhuang wrote:
> Add gpio platform driver to enable GPIO in HiKey960 platform.
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang 
> ---
>  Platform/Hisilicon/HiKey960/HiKey960.dsc|  1 +
>  Platform/Hisilicon/HiKey960/HiKey960.fdf|  1 +
>  Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf | 35 
> +
>  Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c   | 77 
> 
>  4 files changed, 114 insertions(+)
> 
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc 
> b/Platform/Hisilicon/HiKey960/HiKey960.dsc
> index 36f43956ab40..3da1b8556321 100644
> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
> @@ -179,6 +179,7 @@ [Components.common]
>#
># GPIO
>#
> +  Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
>ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
>  
>#
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf 
> b/Platform/Hisilicon/HiKey960/HiKey960.fdf
> index 655032a36c53..162dbaaf2646 100644
> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
> @@ -120,6 +120,7 @@ [FV.FvMain]
>#
># GPIO
>#
> +  INF Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
>INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf
>  
>#
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf 
> b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
> new file mode 100644
> index ..a16213f02520
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf
> @@ -0,0 +1,35 @@
> +#
> +#  Copyright (c) 2018, Linaro. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +
> +[Defines]
> +  INF_VERSION= 0x00010019

Probably bump this to 0x0001001a by now.

> +  BASE_NAME  = HiKey960GpioDxe
> +  FILE_GUID  = 6aa12592-7e36-4aec-acf8-2ac2fd13815c
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 1.0
> +  ENTRY_POINT= HiKey960GpioEntryPoint
> +
> +[Sources]
> +  HiKey960GpioDxe.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gPlatformGpioProtocolGuid
> +
> +[Depex]
> +  TRUE
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c 
> b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c
> new file mode 100644
> index ..986feceea564
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c
> @@ -0,0 +1,77 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Linaro. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +
> +#include 
> +
> +GPIO_CONTROLLER gGpioDevice[]= {

Missing space before '='.

> +  { 0xe8a0b000, 0, 8 },// GPIO0

It would not improve readability to request all of these live-coded
values to be replaced by #defines, but barring that, could you add a
comment header before the definition?:

//  { base address, gpio index, gpio count }

> +  { 0xe8a0c000, 8, 8 },// GPIO1
> +  { 0xe8a0d000, 16, 8 },   // GPIO2
> +  { 0xe8a0e000, 24, 8 },   // GPIO3
> +  { 0xe8a0f000, 32, 8 },   // GPIO4
> +  { 0xe8a1, 40, 8 },   // GPIO5
> +  { 0xe8a11000, 48, 8 },   // GPIO6
> +  { 0xe8a12000, 56, 8 },   // GPIO7
> +  { 0xe8a13000, 64, 8 },   // GPIO8
> +  { 0xe8a14000, 72, 8 },   // GPIO9
> +  { 0xe8a15000, 80, 8 },   // GPIO10
> +  { 0xe8a16000, 88, 8 },   // GPIO11
> +  { 0xe8a17000, 96, 8 },   // GPIO12
> +  { 0xe8a18000, 104, 8 },  // GPIO13
> +  { 0xe8a19000, 112, 8 },  // GPIO14
> +  { 0xe8a1a000, 120, 8 },  // GPIO15
> +  { 0xe8a1b000, 128, 8 },  // GPIO16
> +  { 0xe8a1c000, 136, 8 },  // GPIO17
> +  { 0xff3b4000, 144, 8 }, 

Re: [edk2] Incorrect Author on patch

[Laszlo Ersek]

On 05/02/18 14:46, Evan Lloyd wrote:
> One obvious way of precluding this sort of problem would be to move to
> using the pull request mechanism on GitHub, rather than requiring
> maintainers to play with upstreaming local patches.

In my opinion:
- *git* pull requests: yes,
- *github* pull requests: please no.

We discussed this a few years (?) ago, and I believe the outcome was
that pulling remote branches was fine, as long as:

(a) the location to pull from was noted in the cover letter of a patch
series, or in a separate pull request emailed to the list, and

(b) the maintainer pulling the remote branch verified that the patches
reviewed on the list were formatted from the commits on the branch
being pulled.

The remote branch being pulled may well reside on github, but the review
must occur on the list (not on the GitHub web UI), and the actual fetch
& merge actions must be carried out by a maintainer locally in their
work environment, from where they are supposed to push the new commits
to the central repo. In other words, we must not start relying on GitHub
tools for either patch reviews or for generating merge commits (or any
other kinds of commits).

>  I can well understand why it would be useful to use Gerrit as a means
>  of reviewing a patch - actually a brilliant idea,

(actually, *not* a brilliant idea, but that's just my opinion :) )

> but it does introduce problems. If, post review, the merge were  to be
> from a pull request then there would be no risk of "meta-data
> corruption".

Agreed 100% about "no risk of meta-data corruption".

However there is a new risk, namely that of getting unreviewed code into
the tree. See requirement (b). And I can imagine that such verification
is more trouble for some maintainers than simply applying the patches
from the list.

Basically, the way I would do it, is:
- identify the branch fork-off point from the pull request,
- create a local branch at that point,
- apply the patches from the mailing list to the local branch,
- fetch the remote branch,
- format the patches from the local branch,
- format the patches from the remote branch,
- compare the two sets of locally-formatted patches,
- eyeball the results.

Local reformatting of the patches applied from the mailing list is
necessary because formatting options (such as context size, file order,
diff algorithm, subject prefix) affect the comparison. And, simply
git-diffing the commits on the parallel branches isn't sufficient,
because it doesn't compare authorship or even commit messages.

In other projects, pulling from someone basically means letting that
someone commit to the project unsupervised. So it's a position of trust.
Hence signed pull requests. And those people that submit the pull
requests, they certainly need to apply the "leaf" patches from mailing
lists, from other contributors, after review.

All in all, I don't think there's a way around applying patches intact
from the mailing list. Whatever workflow we choose, that action will be
part of that workflow.

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

Re: [edk2] Incorrect Author on patch

[Evan Lloyd]

One obvious way of precluding this sort of problem would be to move to using 
the pull request mechanism on GitHub, rather than requiring maintainers to play 
with upstreaming local patches.  I can well understand why it would be useful 
to use Gerrit as a means of reviewing a patch - actually a brilliant idea, but 
it does introduce problems. If, post review, the merge were  to be from a pull 
request then there would be no risk of "meta-data corruption".

Regards
Evan

> -Original Message-
> From: Laszlo Ersek 
> Sent: 25 April 2018 22:15
> To: Evan Lloyd ; ruiyu...@intel.com
> Cc: edk2-devel (edk2-devel@lists.01.org) 
> Subject: Re: [edk2] Incorrect Author on patch
>
> On 04/25/18 17:02, Evan Lloyd wrote:
> > Hi Ruiyu.
> > When we look at the edk2 git log, or GitHub
> >
> https://github.com/tianocore/edk2/commit/ee4dc24f57c32a445e7c747396c
> 9b
> > fbd8b221568 we see that Sami's AcpiView patch shows you as the Author.
> > I'm not sure what might have caused that (and it is obviously accidental), 
> > but
> it is a little unfortunate in that the commit doesn't show up on Sami's GitHub
> stats.
> > Fortunately, this is not currently significant for our organisation, 
> > although I'm
> sure Sami would prefer to have the credit.
> > This is a trivial matter, but you may wish to understand what caused it so 
> > that
> you don't accidentally upset someone for whom the stats are significant.
>
> Right, this occurred at least one other time as well: see commit
> 8b5c80e0296c ("MdeModulePkg/UefiBootManagerLib: fix
> AddLoadOptionVariable docs/prototype", 2018-04-23). Ray pushed the patch
> (so the Committer field is certainly right), but Ray's name+email replaced 
> mine
> in the Author field. I had noticed that earlier, but now I'm seeing it as a 
> pattern.
>
> I believe this is a tooling issue. I notice the following bit on the commit
> message:
>
> Change-Id: I8a609d6502b6f8929b2f568acaa147065003b6f4
>
> I certainly didn't post the patch with that, and I doubt Ray added it 
> manually.
> So, whatever tool Ray used to handle the patch lost the authorship
> information.
>
> And, I see the exact same kind of tag, namely
>
> Change-Id: Ifa23dc80ab8ab042c56e88424847e796a8122a7c
>
> on commit ee4dc24f57c3 ("ShellPkg: Add acpiview tool to dump ACPI tables",
> 2018-04-23), which you mention.
>
> ... In fact, my patch happens to be the direct ancestor of Sami's, in the git
> history, and their commit times are just ~3 minutes apart. I'm quite certain 
> Ray
> has started using a new tool.
>
> For example, commit bc2288f59ba2 ("UefiCpuPkg/MpInitLib: put
> mReservedApLoopFunc in executable memory", 2018-03-08) was also
> committed by Ray, *but* Jian's authorship was preserved fine. (No "Change-
> Id" either.)
>
> ... Oh... it looks like those Change-Id's were added by Gerrit:
>
> https://git.eclipse.org/r/Documentation/user-changeid.html
>
> And then, please look at this:
>
> https://gerrit-review.googlesource.com/Documentation/error-invalid-
> author.html
>
> For every pushed commit Gerrit verifies that the e-mail address of
> the author matches one of the registered e-mail addresses of the
> pushing user. If this is not the case pushing the commit fails with
> the error message "invalid author". This policy can be bypassed by
> having the access right 'Forge Author'.
>
> Uh, what?... Gerrit says "invalid author" if Ray pushes a patch that wasn't
> authored by himself? And the option to override that is called "forge" author?
> O_o
>
> Anyway, the page continues,
>
> If pushing to Gerrit fails with the error message "invalid author"
> and somebody else is author of the commit for which the push fails,
> then you have no permissions to forge the author identity. In this
> case you may contact the project owner to request the access right
> '+1 Forge Author Identity' in the 'Forge Identity' category or ask
> the maintainer to commit this change on the author’s behalf.
>
> Ray, if you absolutely must use Gerrit, please make sure that you have the '+1
> Forge Author Identity' access right. IMO, we maintainers are responsible for
> preserving git metadata the best we can.
>
> (For example, if a patch is applied from the mailing list with "git am", it
> preserves the authorship information -- the documentation says, 'The commit
> author name is taken from the "From: " line of the message, and commit
> author date is taken from the "Date: " line of the message.')
>
> Thank you!
> Laszlo
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list

Re: [edk2] Set "db" variable in secure boot setup mode still requires generating PKCS#7?

[Laszlo Ersek]

On 05/01/18 23:13, David F. wrote:
> Hi,
>
> Had a fairly simple task of wanting to install the latest MS .crt
> files for KEK, and their two files for the "db" (the Windows CA and
> UEFI CA) in a system placed in setup/custom mode.  However, even
> though it seemed to take the KEK, it never took the "db", always had a
> problem on a DH77KC mobo (dumped data headers looked as expected). Now
> when I constructed it, I thought I could leave out any PKCS#7 data
> (set the expected CertType but in the Hdr dwLength only included
> CertType and not any CertData),

Right, I've stumbled upon that too. According to the UEFI spec, dwLength
should include CertData too, but edk2 does *not* accept that. This can
be seen e.g. in
"SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c",
function CreateTimeBasedPayload():

>   //
>   // In Setup mode or Custom mode, the variable does not need to be signed 
> but the
>   // parameters to the SetVariable() call still need to be prepared as 
> authenticated
>   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor without 
> certificate
>   // data in it.
>   //
> ...
>   DescriptorData->AuthInfo.Hdr.dwLength = OFFSET_OF 
> (WIN_CERTIFICATE_UEFI_GUID, CertData);

Back to your email:

On 05/01/18 23:13, David F. wrote:
> but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> always have to generate the hash, sign it, create all the PKCS stuff
> even in setup mode?That would surely unnecessarily bloat any apps
> that really only need to update things in setup mode wouldn't it?   So
> to confirm, that is a requirement even in setup mode?If so, why?

It's not a requirement; see the code comment I quoted above.

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

Re: [edk2] [PATCH edk2-platforms] Platform/Socionext/DeveloperBox: reduce default timeout to 5 seconds

[Ard Biesheuvel]

On 1 May 2018 at 14:28, Leif Lindholm  wrote:
> On Tue, May 01, 2018 at 01:35:40PM +0200, Ard Biesheuvel wrote:
>> The default timeout value for the delay during which the splash screen
>> is shown and the BDS menu can be entered is still at 30 seconds, which
>> is rather long. Reduce it to 5 seconds instead.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>
> Reviewed-by: Leif Lindholm 
>

Thanks. Pushed as bf8a7048c80d8fb814279fa13000ab3f80ef61fc


>> ---
>>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
>> b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> index 1731843e825c..676f3483bfe9 100644
>> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> @@ -436,7 +436,7 @@ [PcdsDynamicExDefault.common.DEFAULT]
>>gEfiMdeModulePkgTokenSpaceGuid.PcdSystemFmpCapsuleImageTypeIdGuid|{0xe5, 
>> 0x4c, 0xb9, 0x50, 0x63, 0x8b, 0x49, 0x48, 0x8a, 0xf4, 0xea, 0x47, 0x93, 
>> 0x56, 0xf0, 0xe3}
>>
>>  [PcdsDynamicHii]
>> -  
>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|30
>> +  
>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
>>
>>
>> gSynQuacerTokenSpaceGuid.PcdPlatformSettings|L"SynQuacerPlatformSettings"|gSynQuacerPlatformFormSetGuid|0x0|0x0|NV,BS
>>
>> --
>> 2.17.0
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms] Silicon/AMD/Styx/AcpiTables: fix wrongly copy/pasted variable name

[Ard Biesheuvel]

On 1 May 2018 at 19:55, Leif Lindholm  wrote:
> On Tue, May 01, 2018 at 06:32:44PM +0200, Ard Biesheuvel wrote:
>> Rename the variable holding the Styx PPTT table to mStyxPpttTable.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>
> Reviewed-by: Leif Lindholm 
>

Thanks. Pushed as 0db6171fb5a63e8d58ee7cfdecf36f5c57ce9315

>> ---
>>  Silicon/AMD/Styx/AcpiTables/Pptt.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Silicon/AMD/Styx/AcpiTables/Pptt.c 
>> b/Silicon/AMD/Styx/AcpiTables/Pptt.c
>> index d9d7c494d86f..1746bbe8b596 100644
>> --- a/Silicon/AMD/Styx/AcpiTables/Pptt.c
>> +++ b/Silicon/AMD/Styx/AcpiTables/Pptt.c
>> @@ -156,7 +156,7 @@ typedef struct {
>>} 
>>   \
>>  }
>>
>> -STATIC STYX_PPTT_TABLE mSynQuacerPpttTable = {
>> +STATIC STYX_PPTT_TABLE mStyxPpttTable = {
>>{
>>  
>> AMD_ACPI_HEADER(EFI_ACPI_6_2_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGNATURE,
>>  STYX_PPTT_TABLE,
>> @@ -221,5 +221,5 @@ PpttHeader (
>>VOID
>>)
>>  {
>> -  return (EFI_ACPI_DESCRIPTION_HEADER *)
>> +  return (EFI_ACPI_DESCRIPTION_HEADER *)
>>  }
>> --
>> 2.17.0
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] MdeModulePkg/ConPlatform: Support short-form USB device path

[Zeng, Star]

Since the checking of MSG_USB_DP and HW_PCCARD_DP is removed on purpose, we'd 
better to add commit message for that. :)

With that, Reviewed-by: Star Zeng .


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Wednesday, May 2, 2018 3:52 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Kinney, Michael D 
; Zeng, Star 
Subject: [edk2] [PATCH v2] MdeModulePkg/ConPlatform: Support short-form USB 
device path

Today's implementation does an exact device path match to check whether the 
device path of a console is in ConIn/ConOut/ErrOut.
But that doesn't work for the USB keyboard.
Because when a platform have multiple USB port, ConIn needs to carry all device 
paths corresponding to each port.
Even worse, today's BDS core logic removes the device path from 
ConIn/ConOut/ErrOut when the connection to that device path fails.
So if user switches the USB keyboard from one port to another across boot, the 
USB keyboard doesn't work in the second boot.

ConPlatform driver solved this problem by adding the
IsHotPlugDevice() function. So that for USB keyboard, ConPlatform doesn't care 
whether its device path is in ConIn or not.
But the rule is too loose, and now causes platform BDS cannot control whether 
to enable USB keyboard as an active console.

The patch changes ConPlatform to support USB short-form device path when 
checking whether the device path of a console is in ConIn/ConOut/ErrOut.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Hao A Wu 
Cc: Michael D Kinney 
Cc: Star Zeng 
---
 .../Universal/Console/ConPlatformDxe/ConPlatform.c | 534 ++--- 
 .../Universal/Console/ConPlatformDxe/ConPlatform.h |  30 +-
 .../Console/ConPlatformDxe/ConPlatformDxe.inf  |   3 +-
 3 files changed, 361 insertions(+), 206 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c 
b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
index 6b53e8ac74..e12876a34b 100644
--- a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
+++ b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
@@ -2,7 +2,7 @@
   Console Platform DXE Driver, install Console Device Guids and update Console
   Environment Variables.
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, 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 @@ -202,9 +202,8 @@ 
ConPlatformDriverBindingSupported (
   Start this driver on ControllerHandle by opening Simple Text Input Protocol,
   reading Device Path, and installing Console In Devcice GUID on 
ControllerHandle.
 
-  If this devcie is not one hot-plug devce, append its device path into the
-  console environment variables ConInDev.
-  
+  Append its device path into the console environment variables ConInDev.
+
   @param  This Protocol instance pointer.
   @param  ControllerHandle Handle of device to bind driver to
   @param  RemainingDevicePath  Optional parameter use to pick a specific child 
@@ -270,58 +269,32 @@ ConPlatformTextInDriverBindingStart (
   }
 
   //
-  // Check the device path, if it is a hot plug device,
-  // do not put the device path into ConInDev, and install
-  // gEfiConsoleInDeviceGuid to the device handle directly.
-  // The policy is, make hot plug device plug in and play immediately.
+  // Append the device path to the ConInDev environment variable  //  
+ ConPlatformUpdateDeviceVariable (
+L"ConInDev",
+DevicePath,
+Append
+);
+
+  //
+  // If the device path is an instance in the ConIn environment 
+ variable,  // then install EfiConsoleInDeviceGuid onto 
+ ControllerHandle
   //
-  if (IsHotPlugDevice (DevicePath)) {
+  if (IsInConInVariable) {
 gBS->InstallMultipleProtocolInterfaces (
,
,
NULL,
NULL
);
-//
-// Append the device path to ConInDev only if it is in ConIn variable.
-//
-if (IsInConInVariable) {
-  ConPlatformUpdateDeviceVariable (
-L"ConInDev",
-DevicePath,
-Append
-);
-}
   } else {
-//
-// If it is not a hot-plug device, append the device path to the
-// ConInDev environment variable
-//
-ConPlatformUpdateDeviceVariable (
-  L"ConInDev",
-  DevicePath,
-  Append
-  );
-
-//
-// If the device path is an instance in the ConIn environment variable,
-// then install EfiConsoleInDeviceGuid onto ControllerHandle
-//
-if (IsInConInVariable) {
-  

Re: [edk2] [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when polling

[Zeng, Star]

Another minor comment below.

There are three lines added with no code, I guess they are added accidently. It 
is better to remove them.

+
+
+


Thanks,
Star
-Original Message-
From: Zeng, Star 
Sent: Wednesday, May 2, 2018 2:39 PM
To: Ni, Ruiyu ; 'edk2-devel@lists.01.org' 

Cc: Chiu, Chasel ; Zeng, Star 
Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when 
polling

Reviewed-by: Star Zeng  if it is updated. :)


Thanks,
Star
-Original Message-
From: Ni, Ruiyu
Sent: Wednesday, May 2, 2018 2:37 PM
To: Zeng, Star ; 'edk2-devel@lists.01.org' 

Cc: Chiu, Chasel 
Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when 
polling

Star,
You are correct.
 ">" is enough here. I will change it when committing the patch.

Thanks/Ray

> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, May 2, 2018 1:12 PM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Chiu, Chasel 
> Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io 
> overhead when polling
> 
> If Multiplicand * Multiplier + Remainder = MAX_UINT64, Even 
> Multiplicand =
> MAX_UINT64 / Multiplier, Overflow still happens.
> 
> So ">=" is used here.
> 
> 
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Wednesday, May 2, 2018 11:44 AM
> > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > Cc: Chiu, Chasel ; Zeng, Star 
> > 
> > Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io 
> > overhead when polling
> >
> > Is it more accurate
> > if (Multiplicand >= DivU64x64Remainder (MAX_UINT64, Multiplier,
> > NULL)) {
> > ->
> > if (Multiplicand > DivU64x64Remainder (MAX_UINT64, Multiplier,
> > NULL)) {
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Thursday, April 26, 2018 10:24 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Chiu, Chasel 
> > 
> > Subject: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io
> overhead
> > when polling
> >
> > RootBridgeIo.PollMem()/PollIo() originally don't count the IO/MMIO 
> > access overhead when delaying.
> > The patch changes the implementation to count the access overhead so 
> > that the actually delay equals to user required delay.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ruiyu Ni 
> > Cc: Star Zeng 
> > Cc: Chasel Chiu 
> > ---
> >  .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf  |   3 +-
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   3 +-
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 151
> +++-
> > -
> >  3 files changed, 115 insertions(+), 42 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > index 42bd8a23cb..2e56959a8f 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #   Generic PCI Host Bridge driver.
> >  #
> > -#  Copyright (c) 2009 - 2016, Intel Corporation. All rights 
> > reserved.
> > +#  Copyright (c) 2009 - 2018, Intel Corporation. All rights 
> > +reserved.
> >  #
> >  #  This program and the accompanying materials  #  are licensed and 
> > made available under the terms and conditions of the BSD License @@
> > -43,6 +43,7 @@ [LibraryClasses]
> >PciSegmentLib
> >UefiLib
> >PciHostBridgeLib
> > +  TimerLib
> >
> >  [Protocols]
> >gEfiMetronomeArchProtocolGuid   ## CONSUMES
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > index d3dfb57fc6..e2f651aee4 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > @@ -2,7 +2,7 @@
> >
> >The PCI Root Bridge header file.
> >
> > -Copyright (c) 1999 - 2017, Intel Corporation. All rights 
> > reserved.
> > +Copyright (c) 1999 - 2018, 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 @@ -36,6 +36,7 @@ WITHOUT WARRANTIES OR
> REPRESENTATIONS OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include "PciHostResource.h"
> >
> >
> > diff --git 

[edk2] [PATCH v2] MdeModulePkg/ConPlatform: Support short-form USB device path

[Ruiyu Ni]

Today's implementation does an exact device path match to check
whether the device path of a console is in ConIn/ConOut/ErrOut.
But that doesn't work for the USB keyboard.
Because when a platform have multiple USB port, ConIn needs to
carry all device paths corresponding to each port.
Even worse, today's BDS core logic removes the device path from
ConIn/ConOut/ErrOut when the connection to that device path fails.
So if user switches the USB keyboard from one port to another across
boot, the USB keyboard doesn't work in the second boot.

ConPlatform driver solved this problem by adding the
IsHotPlugDevice() function. So that for USB keyboard, ConPlatform
doesn't care whether its device path is in ConIn or not.
But the rule is too loose, and now causes platform BDS cannot control
whether to enable USB keyboard as an active console.

The patch changes ConPlatform to support USB short-form device path
when checking whether the device path of a console is in
ConIn/ConOut/ErrOut.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Hao A Wu 
Cc: Michael D Kinney 
Cc: Star Zeng 
---
 .../Universal/Console/ConPlatformDxe/ConPlatform.c | 534 ++---
 .../Universal/Console/ConPlatformDxe/ConPlatform.h |  30 +-
 .../Console/ConPlatformDxe/ConPlatformDxe.inf  |   3 +-
 3 files changed, 361 insertions(+), 206 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c 
b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
index 6b53e8ac74..e12876a34b 100644
--- a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
+++ b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
@@ -2,7 +2,7 @@
   Console Platform DXE Driver, install Console Device Guids and update Console
   Environment Variables.
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, 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
@@ -202,9 +202,8 @@ ConPlatformDriverBindingSupported (
   Start this driver on ControllerHandle by opening Simple Text Input Protocol,
   reading Device Path, and installing Console In Devcice GUID on 
ControllerHandle.
 
-  If this devcie is not one hot-plug devce, append its device path into the 
-  console environment variables ConInDev.
-  
+  Append its device path into the console environment variables ConInDev.
+
   @param  This Protocol instance pointer.
   @param  ControllerHandle Handle of device to bind driver to
   @param  RemainingDevicePath  Optional parameter use to pick a specific child
@@ -270,58 +269,32 @@ ConPlatformTextInDriverBindingStart (
   }
 
   //
-  // Check the device path, if it is a hot plug device,
-  // do not put the device path into ConInDev, and install
-  // gEfiConsoleInDeviceGuid to the device handle directly.
-  // The policy is, make hot plug device plug in and play immediately.
+  // Append the device path to the ConInDev environment variable
+  //
+  ConPlatformUpdateDeviceVariable (
+L"ConInDev",
+DevicePath,
+Append
+);
+
+  //
+  // If the device path is an instance in the ConIn environment variable,
+  // then install EfiConsoleInDeviceGuid onto ControllerHandle
   //
-  if (IsHotPlugDevice (DevicePath)) {
+  if (IsInConInVariable) {
 gBS->InstallMultipleProtocolInterfaces (
,
,
NULL,
NULL
);
-//
-// Append the device path to ConInDev only if it is in ConIn variable.
-//
-if (IsInConInVariable) {
-  ConPlatformUpdateDeviceVariable (
-L"ConInDev",
-DevicePath,
-Append
-);
-}
   } else {
-//
-// If it is not a hot-plug device, append the device path to the
-// ConInDev environment variable
-//
-ConPlatformUpdateDeviceVariable (
-  L"ConInDev",
-  DevicePath,
-  Append
-  );
-
-//
-// If the device path is an instance in the ConIn environment variable,
-// then install EfiConsoleInDeviceGuid onto ControllerHandle
-//
-if (IsInConInVariable) {
-  gBS->InstallMultipleProtocolInterfaces (
- ,
- ,
- NULL,
- NULL
- );
-} else {
-  gBS->CloseProtocol (
- ControllerHandle,
- ,
- This->DriverBindingHandle,
- ControllerHandle
- );
-}
+gBS->CloseProtocol (
+   ControllerHandle,
+   ,
+   This->DriverBindingHandle,
+   ControllerHandle
+   );
   }
 
   return EFI_SUCCESS;
@@ -334,9 +307,8 @@ ConPlatformTextInDriverBindingStart (
   reading Device Path, and installing Console Out 

Re: [edk2] [PATCH] MdeModulePkg/ConPlatform: Support short-form USB device path

[Ni, Ruiyu]

Thanks/Ray

> -Original Message-
> From: Zeng, Star
> Sent: Thursday, April 26, 2018 3:05 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Kinney, Michael D
> ; Zeng, Star 
> Subject: RE: [PATCH] MdeModulePkg/ConPlatform: Support short-form USB
> device path
> 
> Ray,
> 
> Some minor comments below.
> 
> 1. How MSG_USB_DP and HW_PCCARD_DP be handled? We see they are
> checked in original IsHotPlugDevice()?
Original implementation to always accept USB/PCCARD device as active console is 
wrong.
So these code is just removed.

> 
> 2. gEfiUsbIoProtocolGuid needs be stated in ConPlatformDxe.inf?
Thanks. I will fix it.

> 
> 3. The comment " If it is not a hot-plug device, append the device path to " 
> in
> ConPlatformTextInDriverBindingStart()/ConPlatformTextOutDriverBindingSt
> art() needs be updated accordingly since IsHotPlugDevice() will be removed?
Thanks. I will fix it in v2 patch.

> 
> 
> Thanks,
> Star
> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, April 25, 2018 1:44 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Kinney, Michael D
> ; Zeng, Star 
> Subject: [PATCH] MdeModulePkg/ConPlatform: Support short-form USB
> device path
> 
> Today's implementation does an exact device path match to check
> whether the device path of a console is in ConIn/ConOut/ErrOut.
> But that doesn't work for the USB keyboard.
> Because when a platform have multiple USB port, ConIn needs to
> carry all device paths corresponding to each port.
> Even worse, today's BDS core logic removes the device path from
> ConIn/ConOut/ErrOut when the connection to that device path fails.
> So if user switches the USB keyboard from one port to another across
> boot, the USB keyboard doesn't work in the second boot.
> 
> ConPlatform driver solved this problem by adding the
> IsHotPlugDevice() function. So that for USB keyboard, ConPlatform
> doesn't care whether its device path is in ConIn or not.
> But the rule is too loose, and now causes platform BDS cannot control
> whether to enable USB keyboard as an active console.
> 
> The patch changes ConPlatform to support USB short-form device path
> when checking whether the device path of a console is in
> ConIn/ConOut/ErrOut.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Hao A Wu 
> Cc: Michael D Kinney 
> Cc: Star Zeng 
> ---
>  .../Universal/Console/ConPlatformDxe/ConPlatform.c | 526
> ++---
>  .../Universal/Console/ConPlatformDxe/ConPlatform.h |  20 +-
>  2 files changed, 353 insertions(+), 193 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
> b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
> index 6b53e8ac74..a509d0e3a5 100644
> --- a/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
> +++ b/MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatform.c
> @@ -2,7 +2,7 @@
>Console Platform DXE Driver, install Console Device Guids and update
> Console
>Environment Variables.
> 
> -Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2018, 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
> @@ -270,58 +270,33 @@ ConPlatformTextInDriverBindingStart (
>}
> 
>//
> -  // Check the device path, if it is a hot plug device,
> -  // do not put the device path into ConInDev, and install
> -  // gEfiConsoleInDeviceGuid to the device handle directly.
> -  // The policy is, make hot plug device plug in and play immediately.
> +  // If it is not a hot-plug device, append the device path to the
> +  // ConInDev environment variable
>//
> -  if (IsHotPlugDevice (DevicePath)) {
> +  ConPlatformUpdateDeviceVariable (
> +L"ConInDev",
> +DevicePath,
> +Append
> +);
> +
> +  //
> +  // If the device path is an instance in the ConIn environment variable,
> +  // then install EfiConsoleInDeviceGuid onto ControllerHandle
> +  //
> +  if (IsInConInVariable) {
>  gBS->InstallMultipleProtocolInterfaces (
> ,
> ,
> NULL,
> NULL
> );
> -//
> -// Append the device path to ConInDev only if it is in ConIn variable.
> -//
> -if (IsInConInVariable) {
> -  ConPlatformUpdateDeviceVariable (
> -L"ConInDev",
> -DevicePath,
> -Append
> -);
> -}
>} else {
> -//
> -// If it is not a hot-plug device, append the device path to the
> -// ConInDev environment variable
> -//
> -

Re: [edk2] [PATCH v2 19/27] BaseTools: Replace Binary File type strings with predefined constant

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu 

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Friday, April 27, 2018 12:58 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v2 19/27] BaseTools: Replace Binary File type strings with 
predefined constant

BINARY_FILE_TYPE_FW was 'FW'
BINARY_FILE_TYPE_GUID was 'GUID'
BINARY_FILE_TYPE_PREEFORM was 'PREEFORM'
BINARY_FILE_TYPE_UEFI_APP was 'UEFI_APP'
BINARY_FILE_TYPE_UNI_UI was 'UNI_UI'
BINARY_FILE_TYPE_UNI_VER was 'UNI_VER'
BINARY_FILE_TYPE_LIB was 'LIB'
BINARY_FILE_TYPE_PE32 was 'PE32'
BINARY_FILE_TYPE_PIC was 'PIC'
BINARY_FILE_TYPE_PEI_DEPEX was 'PEI_DEPEX'
BINARY_FILE_TYPE_DXE_DEPEX was 'DXE_DEPEX'
BINARY_FILE_TYPE_SMM_DEPEX was 'SMM_DEPEX'
BINARY_FILE_TYPE_TE was 'TE'
BINARY_FILE_TYPE_VER was 'VER'
BINARY_FILE_TYPE_UI was 'UI'
BINARY_FILE_TYPE_BIN was 'BIN'
BINARY_FILE_TYPE_FV was 'FV'

v2 - split apart FV and GUID types with different meanings.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 12 +--
 BaseTools/Source/Python/AutoGen/GenC.py|  8 +-
 BaseTools/Source/Python/AutoGen/GenPcdDb.py|  4 +-
 BaseTools/Source/Python/Common/DataType.py |  2 +
 BaseTools/Source/Python/Common/Expression.py   |  2 +-
 BaseTools/Source/Python/Common/Misc.py |  4 +-
 BaseTools/Source/Python/GenFds/DataSection.py  |  6 +-
 BaseTools/Source/Python/GenFds/DepexSection.py |  6 +-
 BaseTools/Source/Python/GenFds/EfiSection.py   | 10 +--
 BaseTools/Source/Python/GenFds/Fd.py   |  3 +-
 BaseTools/Source/Python/GenFds/FdfParser.py| 92 
++--
 BaseTools/Source/Python/GenFds/Ffs.py  | 14 +--
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  | 22 ++---
 BaseTools/Source/Python/GenFds/Fv.py   |  4 +-
 BaseTools/Source/Python/GenFds/GenFds.py   | 10 +--
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py |  4 +-
 BaseTools/Source/Python/GenFds/OptRomInfStatement.py   |  2 +-
 BaseTools/Source/Python/GenFds/Region.py   |  5 +-
 BaseTools/Source/Python/GenFds/Section.py  | 46 +-
 BaseTools/Source/Python/GenFds/UiSection.py|  2 +-
 BaseTools/Source/Python/Trim/Trim.py   |  2 +-
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  2 +-
 BaseTools/Source/Python/Workspace/InfBuildData.py  |  2 +-
 BaseTools/Source/Python/build/BuildReport.py   | 12 +--
 24 files changed, 140 insertions(+), 136 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 2a6fa05ab819..671f02133aca 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -922,7 +922,7 @@ class WorkspaceAutoGen(AutoGen):
 ## Return the directory to store FV files
 def _GetFvDir(self):
 if self._FvDir is None:
-self._FvDir = path.join(self.BuildDir, 'FV')
+self._FvDir = path.join(self.BuildDir, TAB_FV_DIRECTORY)
 return self._FvDir
 
 ## Return the directory to store all intermediate and final files built
@@ -1325,7 +1325,7 @@ class PlatformAutoGen(AutoGen):
 
 def UpdateNVStoreMaxSize(self,OrgVpdFile):
 if self.VariableInfo:
-VpdMapFilePath = os.path.join(self.BuildDir, "FV", "%s.map" % 
self.Platform.VpdToolGuid)
+VpdMapFilePath = os.path.join(self.BuildDir, TAB_FV_DIRECTORY, 
"%s.map" % self.Platform.VpdToolGuid)
 PcdNvStoreDfBuffer = [item for item in self._DynamicPcdList if 
item.TokenCName == "PcdNvStoreDefaultValueBuffer" and item.TokenSpaceGuidCName 
== "gEfiMdeModulePkgTokenSpaceGuid"]
 
 if PcdNvStoreDfBuffer:
@@ -1718,7 +1718,7 @@ class PlatformAutoGen(AutoGen):
 
 # Process VPD map file generated by third party BPDG tool
 if NeedProcessVpdMapFile:
-VpdMapFilePath = os.path.join(self.BuildDir, "FV", 
"%s.map" % self.Platform.VpdToolGuid)
+VpdMapFilePath = os.path.join(self.BuildDir, 
TAB_FV_DIRECTORY, "%s.map" % self.Platform.VpdToolGuid)
 if os.path.exists(VpdMapFilePath):
 VpdFile.Read(VpdMapFilePath)
 
@@ -1769,7 +1769,7 @@ class PlatformAutoGen(AutoGen):
 self.AllPcdList = self._NonDynamicPcdList + self._DynamicPcdList
 
 def FixVpdOffset(self,VpdFile ):
-FvPath = os.path.join(self.BuildDir, "FV")
+FvPath = os.path.join(self.BuildDir, TAB_FV_DIRECTORY)
 if not os.path.exists(FvPath):
 try:
 os.makedirs(FvPath)
@@ -1782,7 +1782,7 @@ class 

Re: [edk2] [PATCH v1 26/27] BaseTools: remove unused MigrationUtilities.py

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Friday, April 20, 2018 11:52 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v1 26/27] BaseTools: remove unused MigrationUtilities.py

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/MigrationUtilities.py | 568 
 BaseTools/Source/Python/Makefile |   1 -
 2 files changed, 569 deletions(-)

diff --git a/BaseTools/Source/Python/Common/MigrationUtilities.py 
b/BaseTools/Source/Python/Common/MigrationUtilities.py
deleted file mode 100644
index 0c93c72a60f6..
--- a/BaseTools/Source/Python/Common/MigrationUtilities.py
+++ /dev/null
@@ -1,568 +0,0 @@
-## @file
-# Contains several utilitities shared by migration tools.
-#
-# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved. -# 
This program and the accompanying materials -# are licensed and made available 
under the terms and conditions of the BSD License -# which accompanies this 
distribution.  The full text of the license may be found at -# 
http://opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -# 
WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-
-##
-# Import Modules
-#
-import Common.LongFilePathOs as os
-import re
-import EdkLogger
-from optparse import OptionParser
-from Common.BuildToolError import *
-from XmlRoutines import *
-from CommonDataClass.CommonClass import * -from Common.LongFilePathSupport 
import OpenLongFilePath as open
-
-## Set all fields of CommonClass object.
-#
-# Set all attributes of CommonClass object from XML Dom object of XmlCommon.
-#
-# @param  Common The destine CommonClass object.
-# @param  XmlCommon  The source XML Dom object.
-#
-def SetCommon(Common, XmlCommon):
-XmlTag = "Usage"
-Common.Usage = XmlAttribute(XmlCommon, XmlTag).split()
-
-XmlTag = TAB_PCDS_FEATURE_FLAG
-Common.FeatureFlag = XmlAttribute(XmlCommon, XmlTag)
-
-XmlTag = "SupArchList"
-Common.SupArchList = XmlAttribute(XmlCommon, XmlTag).split()
-
-XmlTag = XmlNodeName(XmlCommon) + "/" + "HelpText"
-Common.HelpText = XmlElement(XmlCommon, XmlTag)
-
-
-## Set some fields of CommonHeaderClass object.
-#
-# Set Name, Guid, FileName and FullPath fields of CommonHeaderClass object 
from -# XML Dom object of XmlCommonHeader, NameTag and FileName.
-#
-# @param  CommonHeader   The destine CommonClass object.
-# @param  XmlCommonHeaderThe source XML Dom object.
-# @param  NameTagThe name tag in XML Dom object.
-# @param  FileName   The file name of the XML file.
-#
-def SetIdentification(CommonHeader, XmlCommonHeader, NameTag, FileName):
-XmlParentTag = XmlNodeName(XmlCommonHeader)
-
-XmlTag = XmlParentTag + "/" + NameTag
-CommonHeader.Name = XmlElement(XmlCommonHeader, XmlTag)
-
-XmlTag = XmlParentTag + "/" + "GuidValue"
-CommonHeader.Guid = XmlElement(XmlCommonHeader, XmlTag)
-
-XmlTag = XmlParentTag + "/" + "Version"
-CommonHeader.Version = XmlElement(XmlCommonHeader, XmlTag)
-
-CommonHeader.FileName = os.path.basename(FileName)
-CommonHeader.FullPath = os.path.abspath(FileName)
-
-
-## Regular expression to match specification and value.
-mReSpecification = re.compile(r"(?P\w+)\s+(?P\w*)")
-
-## Add specification to specification dictionary.
-#
-# Abstract specification name, value pair from Specification String and add 
them -# to specification dictionary.
-#
-# @param  SpecificationDict   The destine Specification dictionary.
-# @param  SpecificationString The source Specification String from which the
-# specification name and value pair is abstracted.
-#
-def AddToSpecificationDict(SpecificationDict, SpecificationString):
-"""Abstract specification name, value pair from Specification String"""
-for SpecificationMatch in mReSpecification.finditer(SpecificationString):
-Specification = SpecificationMatch.group("Specification")
-Value = SpecificationMatch.group("Value")
-SpecificationDict[Specification] = Value
-
-## Set all fields of CommonHeaderClass object.
-#
-# Set all attributes of CommonHeaderClass object from XML Dom object of -# 
XmlCommonHeader, NameTag and FileName.
-#
-# @param  CommonHeader   The destine CommonClass object.
-# @param  XmlCommonHeaderThe source XML Dom object.
-# @param  NameTagThe name tag in XML Dom object.
-# @param  FileName   The file name of the XML file.
-#
-def SetCommonHeader(CommonHeader, XmlCommonHeader):
-"""Set all attributes of CommonHeaderClass object from 

Re: [edk2] [PATCH v1 22/27] BaseTools: remove redundant if comparison

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Friday, April 20, 2018 11:52 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v1 22/27] BaseTools: remove redundant if comparison

inherently python will check string and list for None and having data

if  in [None, ''] and similar are superflous.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 26 
 BaseTools/Source/Python/AutoGen/GenC.py|  2 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |  4 +--
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py | 32 
++--
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  8 ++---
 BaseTools/Source/Python/Workspace/InfBuildData.py  |  2 +-
 6 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 534fbe79fad9..871afedcde4b 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -420,14 +420,14 @@ class WorkspaceAutoGen(AutoGen):
 if BuildData.Pcds[key].Pending:
 if key in Platform.Pcds:
 PcdInPlatform = Platform.Pcds[key]
-if PcdInPlatform.Type not in [None, '']:
+if PcdInPlatform.Type:
 BuildData.Pcds[key].Type = 
PcdInPlatform.Type
 
 if BuildData.MetaFile in Platform.Modules:
 PlatformModule = 
Platform.Modules[str(BuildData.MetaFile)]
 if key in PlatformModule.Pcds:
 PcdInPlatform = PlatformModule.Pcds[key]
-if PcdInPlatform.Type not in [None, '']:
+if PcdInPlatform.Type:
 BuildData.Pcds[key].Type = 
PcdInPlatform.Type
 
 if TAB_PCDS_DYNAMIC_EX in BuildData.Pcds[key].Type:
@@ -1394,7 +1394,7 @@ class PlatformAutoGen(AutoGen):
 
 for PcdFromModule in M.ModulePcdList + M.LibraryPcdList:
 # make sure that the "VOID*" kind of datum has MaxDatumSize set
-if PcdFromModule.DatumType == "VOID*" and 
PcdFromModule.MaxDatumSize in [None, '']:
+if PcdFromModule.DatumType == "VOID*" and not 
PcdFromModule.MaxDatumSize:
 NoDatumTypePcdList.add("%s.%s [%s]" % 
(PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, F))
 
 # Check the PCD from Binary INF or Source INF @@ -1471,7 
+1471,7 @@ class PlatformAutoGen(AutoGen):
 ExtraData="\n\tExisted %s PCD %s 
in:\n\t\t%s\n"
 % (PcdFromModule.Type, 
PcdFromModule.TokenCName, InfName))
 # make sure that the "VOID*" kind of datum has 
MaxDatumSize set
-if PcdFromModule.DatumType == "VOID*" and 
PcdFromModule.MaxDatumSize in [None, '']:
+if PcdFromModule.DatumType == "VOID*" and not 
PcdFromModule.MaxDatumSize:
 NoDatumTypePcdList.add("%s.%s [%s]" % 
(PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, InfName))
 if M.ModuleType in SUP_MODULE_SET_PEI:
 PcdFromModule.Phase = "PEI"
@@ -1999,7 +1999,7 @@ class PlatformAutoGen(AutoGen):
 BuildRuleFile = None
 if TAB_TAT_DEFINES_BUILD_RULE_CONF in 
self.Workspace.TargetTxt.TargetTxtDictionary:
 BuildRuleFile = 
self.Workspace.TargetTxt.TargetTxtDictionary[TAB_TAT_DEFINES_BUILD_RULE_CONF]
-if BuildRuleFile in [None, '']:
+if not BuildRuleFile:
 BuildRuleFile = gDefaultBuildRuleFile
 self._BuildRule = BuildRule(BuildRuleFile)
 if self._BuildRule._FileVersion == "":
@@ -2327,13 +2327,13 @@ class PlatformAutoGen(AutoGen):
 TokenCName = PcdItem[0]
 break
 if FromPcd is not None:
-if ToPcd.Pending and FromPcd.Type not in [None, '']:
+if ToPcd.Pending and FromPcd.Type:
 ToPcd.Type = FromPcd.Type
-elif (ToPcd.Type not in [None, '']) and (FromPcd.Type not in 
[None, ''])\
+elif (ToPcd.Type) and (FromPcd.Type)\
 and (ToPcd.Type != FromPcd.Type) and (ToPcd.Type in 
FromPcd.Type):
 if ToPcd.Type.strip() == TAB_PCDS_DYNAMIC_EX:
 ToPcd.Type = FromPcd.Type
-   

Re: [edk2] [PATCH v1 14/27] BaseTools: Define and use a set for common list

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Friday, April 20, 2018 11:52 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v1 14/27] BaseTools: Define and use a set for common list

share a set for both PEI module types

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 4 ++--
 BaseTools/Source/Python/AutoGen/GenC.py| 8 
 BaseTools/Source/Python/Common/DataType.py | 1 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 9c3bf864e360..60088c87a03b 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1431,7 +1431,7 @@ class PlatformAutoGen(AutoGen):
 # used by DXE module, it should be stored in DXE PCD 
database.
 # The default Phase is DXE
 #
-if M.ModuleType in [SUP_MODULE_PEIM, SUP_MODULE_PEI_CORE]:
+if M.ModuleType in SUP_MODULE_SET_PEI:
 PcdFromModule.Phase = "PEI"
 if PcdFromModule not in self._DynaPcdList_:
 self._DynaPcdList_.append(PcdFromModule)
@@ -1473,7 +1473,7 @@ class PlatformAutoGen(AutoGen):
 # make sure that the "VOID*" kind of datum has 
MaxDatumSize set
 if PcdFromModule.DatumType == "VOID*" and 
PcdFromModule.MaxDatumSize in [None, '']:
 NoDatumTypePcdList.add("%s.%s [%s]" % 
(PcdFromModule.TokenSpaceGuidCName, PcdFromModule.TokenCName, InfName))
-if M.ModuleType in [SUP_MODULE_PEIM, SUP_MODULE_PEI_CORE]:
+if M.ModuleType in SUP_MODULE_SET_PEI:
 PcdFromModule.Phase = "PEI"
 if PcdFromModule not in self._DynaPcdList_ and 
PcdFromModule.Type in GenC.gDynamicExPcd:
 self._DynaPcdList_.append(PcdFromModule)
diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 7bc352274e66..57ff699a968a 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -1411,7 +1411,7 @@ def CreateLibraryConstructorCode(Info, AutoGenC, 
AutoGenH):
 if Lib.ModuleType in [SUP_MODULE_BASE, SUP_MODULE_SEC]:
 
ConstructorPrototypeString.Append(gLibraryStructorPrototype[SUP_MODULE_BASE].Replace(Dict))
 
ConstructorCallingString.Append(gLibraryStructorCall[SUP_MODULE_BASE].Replace(Dict))
-elif Lib.ModuleType in [SUP_MODULE_PEI_CORE,SUP_MODULE_PEIM]:
+elif Lib.ModuleType in SUP_MODULE_SET_PEI:
 
ConstructorPrototypeString.Append(gLibraryStructorPrototype['PEI'].Replace(Dict))
 
ConstructorCallingString.Append(gLibraryStructorCall['PEI'].Replace(Dict))
 elif Lib.ModuleType in 
[SUP_MODULE_DXE_CORE,SUP_MODULE_DXE_DRIVER,SUP_MODULE_DXE_SMM_DRIVER,SUP_MODULE_DXE_RUNTIME_DRIVER,
@@ -1441,7 +1441,7 @@ def CreateLibraryConstructorCode(Info, AutoGenC, 
AutoGenH):
 else:
 if Info.ModuleType in [SUP_MODULE_BASE, SUP_MODULE_SEC]:
 AutoGenC.Append(gLibraryString[SUP_MODULE_BASE].Replace(Dict))
-elif Info.ModuleType in [SUP_MODULE_PEI_CORE,SUP_MODULE_PEIM]:
+elif Info.ModuleType in SUP_MODULE_SET_PEI:
 AutoGenC.Append(gLibraryString['PEI'].Replace(Dict))
 elif Info.ModuleType in 
[SUP_MODULE_DXE_CORE,SUP_MODULE_DXE_DRIVER,SUP_MODULE_DXE_SMM_DRIVER,SUP_MODULE_DXE_RUNTIME_DRIVER,
  
SUP_MODULE_DXE_SAL_DRIVER,SUP_MODULE_UEFI_DRIVER,SUP_MODULE_UEFI_APPLICATION,SUP_MODULE_SMM_CORE]:
@@ -1473,7 +1473,7 @@ def CreateLibraryDestructorCode(Info, AutoGenC, AutoGenH):
 if Lib.ModuleType in [SUP_MODULE_BASE, SUP_MODULE_SEC]:
 
DestructorPrototypeString.Append(gLibraryStructorPrototype[SUP_MODULE_BASE].Replace(Dict))
 
DestructorCallingString.Append(gLibraryStructorCall[SUP_MODULE_BASE].Replace(Dict))
-elif Lib.ModuleType in [SUP_MODULE_PEI_CORE,SUP_MODULE_PEIM]:
+elif Lib.ModuleType in SUP_MODULE_SET_PEI:
 
DestructorPrototypeString.Append(gLibraryStructorPrototype['PEI'].Replace(Dict))
 
DestructorCallingString.Append(gLibraryStructorCall['PEI'].Replace(Dict))
 elif Lib.ModuleType in 
[SUP_MODULE_DXE_CORE,SUP_MODULE_DXE_DRIVER,SUP_MODULE_DXE_SMM_DRIVER,SUP_MODULE_DXE_RUNTIME_DRIVER,
@@ -1503,7 +1503,7 @@ def CreateLibraryDestructorCode(Info, AutoGenC, AutoGenH):
 else:
 if Info.ModuleType in [SUP_MODULE_BASE, SUP_MODULE_SEC]:
 

Re: [edk2] [PATCH v3 16/27] BaseTools: Replace EDK Component strings with predefined constant

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Friday, April 27, 2018 10:04 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v3 16/27] BaseTools: Replace EDK Component strings with 
predefined constant

EDK_COMPONENT_TYPE_LIBRARY was 'LIBRARY'
EDK_COMPONENT_TYPE_SECURITY_CORE was 'SECURITY_CORE'
EDK_COMPONENT_TYPE_COMBINED_PEIM_DRIVER was 'COMBINED_PEIM_DRIVER'
EDK_COMPONENT_TYPE_PIC_PEIM was 'PIC_PEIM'
EDK_COMPONENT_TYPE_RELOCATABLE_PEIM was 'RELOCATABLE_PEIM'
EDK_COMPONENT_TYPE_BS_DRIVER was 'BS_DRIVER'
EDK_COMPONENT_TYPE_RT_DRIVER was 'RT_DRIVER'
EDK_COMPONENT_TYPE_SAL_RT_DRIVER was 'SAL_RT_DRIVER'
EDK_COMPONENT_TYPE_APPLICATION was 'APPLICATION'

v2 - revert 2 files.  will update later in own patches.
v3 - fix v2

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Common/DataType.py| 28 ++--
 BaseTools/Source/Python/GenFds/FdfParser.py   |  6 ++---
 BaseTools/Source/Python/Workspace/InfBuildData.py |  2 +-
 BaseTools/Source/Python/build/build.py|  8 +++---
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/BaseTools/Source/Python/Common/DataType.py 
b/BaseTools/Source/Python/Common/DataType.py
index cac455f9f7cf..ffd73d6852d1 100644
--- a/BaseTools/Source/Python/Common/DataType.py
+++ b/BaseTools/Source/Python/Common/DataType.py
@@ -84,7 +84,7 @@ SUP_MODULE_LIST_STRING = TAB_VALUE_SPLIT.join(SUP_MODULE_LIST)
 SUP_MODULE_SET_PEI = {SUP_MODULE_PEIM, SUP_MODULE_PEI_CORE}
 
 EDK_COMPONENT_TYPE_LIBRARY = 'LIBRARY'
-EDK_COMPONENT_TYPE_SECUARITY_CORE = 'SECUARITY_CORE'
+EDK_COMPONENT_TYPE_SECURITY_CORE = 'SECURITY_CORE'
 EDK_COMPONENT_TYPE_PEI_CORE = SUP_MODULE_PEI_CORE  
EDK_COMPONENT_TYPE_COMBINED_PEIM_DRIVER = 'COMBINED_PEIM_DRIVER'
 EDK_COMPONENT_TYPE_PIC_PEIM = 'PIC_PEIM'
@@ -97,18 +97,18 @@ EDK_NAME = 'EDK'
 EDKII_NAME = 'EDKII'
 
 COMPONENT_TO_MODULE_MAP_DICT = {
-"LIBRARY"   :   SUP_MODULE_BASE,
-"SECURITY_CORE" :   SUP_MODULE_SEC,
-SUP_MODULE_PEI_CORE :   SUP_MODULE_PEI_CORE,
-"COMBINED_PEIM_DRIVER"  :   SUP_MODULE_PEIM,
-"PIC_PEIM"  :   SUP_MODULE_PEIM,
-"RELOCATABLE_PEIM"  :   SUP_MODULE_PEIM,
-"PE32_PEIM" :   SUP_MODULE_PEIM,
-"BS_DRIVER" :   SUP_MODULE_DXE_DRIVER,
-"RT_DRIVER" :   SUP_MODULE_DXE_RUNTIME_DRIVER,
-"SAL_RT_DRIVER" :   SUP_MODULE_DXE_SAL_DRIVER,
-"APPLICATION"   :   SUP_MODULE_UEFI_APPLICATION,
-"LOGO"  :   SUP_MODULE_BASE,
+EDK_COMPONENT_TYPE_LIBRARY   :   SUP_MODULE_BASE,
+EDK_COMPONENT_TYPE_SECURITY_CORE :   SUP_MODULE_SEC,
+EDK_COMPONENT_TYPE_PEI_CORE  :   SUP_MODULE_PEI_CORE,
+EDK_COMPONENT_TYPE_COMBINED_PEIM_DRIVER  :   SUP_MODULE_PEIM,
+EDK_COMPONENT_TYPE_PIC_PEIM  :   SUP_MODULE_PEIM,
+EDK_COMPONENT_TYPE_RELOCATABLE_PEIM  :   SUP_MODULE_PEIM,
+"PE32_PEIM"  :   SUP_MODULE_PEIM,
+EDK_COMPONENT_TYPE_BS_DRIVER :   SUP_MODULE_DXE_DRIVER,
+EDK_COMPONENT_TYPE_RT_DRIVER :   SUP_MODULE_DXE_RUNTIME_DRIVER,
+EDK_COMPONENT_TYPE_SAL_RT_DRIVER :   SUP_MODULE_DXE_SAL_DRIVER,
+EDK_COMPONENT_TYPE_APPLICATION   :   SUP_MODULE_UEFI_APPLICATION,
+"LOGO"   :   SUP_MODULE_BASE,
 }
 
 BINARY_FILE_TYPE_FW = 'FW'
@@ -129,7 +129,7 @@ BINARY_FILE_TYPE_UI = 'UI'
 BINARY_FILE_TYPE_BIN = 'BIN'
 BINARY_FILE_TYPE_FV = 'FV'
 
-PLATFORM_COMPONENT_TYPE_LIBRARY = 'LIBRARY'
+PLATFORM_COMPONENT_TYPE_LIBRARY = EDK_COMPONENT_TYPE_LIBRARY
 PLATFORM_COMPONENT_TYPE_LIBRARY_CLASS = 'LIBRARY_CLASS'
 PLATFORM_COMPONENT_TYPE_MODULE = 'MODULE'
 
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index db8947e4b453..d15656c5fc6c 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -3630,8 +3630,8 @@ class FdfParser:
  SUP_MODULE_DXE_DRIVER, SUP_MODULE_DXE_SAL_DRIVER, 
\
  SUP_MODULE_DXE_SMM_DRIVER, 
SUP_MODULE_DXE_RUNTIME_DRIVER, \
  SUP_MODULE_UEFI_DRIVER, 
SUP_MODULE_UEFI_APPLICATION, SUP_MODULE_USER_DEFINED, "DEFAULT", 
SUP_MODULE_BASE, \
- "SECURITY_CORE", "COMBINED_PEIM_DRIVER", 
"PIC_PEIM", "RELOCATABLE_PEIM", \
-"PE32_PEIM", "BS_DRIVER", "RT_DRIVER", 
"SAL_RT_DRIVER", "APPLICATION", "ACPITABLE", SUP_MODULE_SMM_CORE, 
SUP_MODULE_MM_STANDALONE, SUP_MODULE_MM_CORE_STANDALONE):
+ 

Re: [edk2] [PATCH v2 11/27] BaseTools: Workspace/MetaFileParser - refactor dicts

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Friday, April 27, 2018 10:04 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH v2 11/27] BaseTools: Workspace/MetaFileParser - refactor dicts

make defaultdict to avoid initialize inner items to empty the dict, call clear 
instead of making a new object

v2 - to empty the dict, dont re-run constructor, just call .clear() in post 
process API also.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/Workspace/MetaFileParser.py | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
b/BaseTools/Source/Python/Workspace/MetaFileParser.py
index 550359f9abb2..51126d710b2b 100644
--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
@@ -31,7 +31,7 @@ from Common.Misc import GuidStructureStringToGuidString, 
CheckPcdDatum, PathClas  from Common.Expression import *  from 
CommonDataClass.Exceptions import *  from Common.LongFilePathSupport import 
OpenLongFilePath as open
-
+from collections import defaultdict
 from MetaFileTable import MetaFileStorage  from MetaFileCommentParser import 
CheckInfComment
 
@@ -163,7 +163,7 @@ class MetaFileParser(object):
 self._FileDir = self.MetaFile.Dir
 self._Defines = {}
 self._FileLocalMacros = {}
-self._SectionsMacroDict = {}
+self._SectionsMacroDict = defaultdict(dict)
 
 # for recursive parsing
 self._Owner = [Owner]
@@ -421,17 +421,16 @@ class MetaFileParser(object):
 def _ConstructSectionMacroDict(self, Name, Value):
 ScopeKey = [(Scope[0], Scope[1],Scope[2]) for Scope in self._Scope]
 ScopeKey = tuple(ScopeKey)
-SectionDictKey = self._SectionType, ScopeKey
 #
 # DecParser SectionType is a list, will contain more than one item 
only in Pcd Section
 # As Pcd section macro usage is not alllowed, so here it is safe
 #
 if type(self) == DecParser:
 SectionDictKey = self._SectionType[0], ScopeKey
-if SectionDictKey not in self._SectionsMacroDict:
-self._SectionsMacroDict[SectionDictKey] = {}
-SectionLocalMacros = self._SectionsMacroDict[SectionDictKey]
-SectionLocalMacros[Name] = Value
+else:
+SectionDictKey = self._SectionType, ScopeKey
+
+self._SectionsMacroDict[SectionDictKey][Name] = Value
 
 ## Get section Macros that are applicable to current line, which may come 
from other sections 
 ## that share the same name while scope is wider @@ -936,7 +935,7 @@ class 
DscParser(MetaFileParser):
 self._SubsectionType = MODEL_UNKNOWN
 self._SubsectionName = ''
 self._Owner[-1] = -1
-OwnerId = {}
+OwnerId.clear()
 continue
 # subsection header
 elif Line[0] == TAB_OPTION_START and Line[-1] == TAB_OPTION_END:
@@ -1296,7 +1295,7 @@ class DscParser(MetaFileParser):
 self._DirectiveEvalStack = []
 self._FileWithError = self.MetaFile
 self._FileLocalMacros = {}
-self._SectionsMacroDict = {}
+self._SectionsMacroDict.clear()
 GlobalData.gPlatformDefines = {}
 
 # Get all macro and PCD which has straitforward value
--
2.16.2.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when polling

[Zeng, Star]

Reviewed-by: Star Zeng  if it is updated. :)


Thanks,
Star
-Original Message-
From: Ni, Ruiyu 
Sent: Wednesday, May 2, 2018 2:37 PM
To: Zeng, Star ; 'edk2-devel@lists.01.org' 

Cc: Chiu, Chasel 
Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io overhead when 
polling

Star,
You are correct.
 ">" is enough here. I will change it when committing the patch.

Thanks/Ray

> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, May 2, 2018 1:12 PM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Chiu, Chasel 
> Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io 
> overhead when polling
> 
> If Multiplicand * Multiplier + Remainder = MAX_UINT64, Even 
> Multiplicand =
> MAX_UINT64 / Multiplier, Overflow still happens.
> 
> So ">=" is used here.
> 
> 
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Wednesday, May 2, 2018 11:44 AM
> > To: Ni, Ruiyu ; edk2-devel@lists.01.org
> > Cc: Chiu, Chasel ; Zeng, Star 
> > 
> > Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io 
> > overhead when polling
> >
> > Is it more accurate
> > if (Multiplicand >= DivU64x64Remainder (MAX_UINT64, Multiplier, 
> > NULL)) {
> > ->
> > if (Multiplicand > DivU64x64Remainder (MAX_UINT64, Multiplier, 
> > NULL)) {
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Thursday, April 26, 2018 10:24 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Chiu, Chasel 
> > 
> > Subject: [PATCH] MdeModulePkg/PciHostBridge: Count the (mm)io
> overhead
> > when polling
> >
> > RootBridgeIo.PollMem()/PollIo() originally don't count the IO/MMIO 
> > access overhead when delaying.
> > The patch changes the implementation to count the access overhead so 
> > that the actually delay equals to user required delay.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ruiyu Ni 
> > Cc: Star Zeng 
> > Cc: Chasel Chiu 
> > ---
> >  .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf  |   3 +-
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   3 +-
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 151
> +++-
> > -
> >  3 files changed, 115 insertions(+), 42 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > index 42bd8a23cb..2e56959a8f 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #   Generic PCI Host Bridge driver.
> >  #
> > -#  Copyright (c) 2009 - 2016, Intel Corporation. All rights 
> > reserved.
> > +#  Copyright (c) 2009 - 2018, Intel Corporation. All rights 
> > +reserved.
> >  #
> >  #  This program and the accompanying materials  #  are licensed and 
> > made available under the terms and conditions of the BSD License @@
> > -43,6 +43,7 @@ [LibraryClasses]
> >PciSegmentLib
> >UefiLib
> >PciHostBridgeLib
> > +  TimerLib
> >
> >  [Protocols]
> >gEfiMetronomeArchProtocolGuid   ## CONSUMES
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > index d3dfb57fc6..e2f651aee4 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> > @@ -2,7 +2,7 @@
> >
> >The PCI Root Bridge header file.
> >
> > -Copyright (c) 1999 - 2017, Intel Corporation. All rights 
> > reserved.
> > +Copyright (c) 1999 - 2018, 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 @@ -36,6 +36,7 @@ WITHOUT WARRANTIES OR
> REPRESENTATIONS OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include "PciHostResource.h"
> >
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > index 5764c2f49f..459e962fe7 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > @@ -2,7 +2,7 @@
> >
> >PCI Root Bridge Io Protocol code.
> >
> > -Copyright (c) 1999 - 2017, Intel Corporation. All rights 
> > reserved.
> > +Copyright (c) 1999 - 2018, Intel Corporation. All rights 
> > +reserved.
> >  This program and the