Re: [edk2-devel] [PATCH v5 0/9] Add new feature that evacuate temporary to permanent memory (CVE-2019-11098)

[Guomin Jiang]

I see it and will do it later.

I remind that everyone should pay attention to it as well.

Thanks.
> -Original Message-
> From: Laszlo Ersek 
> Sent: Friday, July 10, 2020 1:47 PM
> To: devel@edk2.groups.io; Jiang, Guomin 
> Cc: Wang, Jian J ; Wu, Hao A
> ; Bi, Dandan ; Gao, Liming
> ; De, Debkumar ; Han,
> Harry ; West, Catharine ;
> Dong, Eric ; Ni, Ray ; Justen,
> Jordan L ; Andrew Fish ; Ard
> Biesheuvel ; Anthony Perard
> ; Julien Grall ; Leif Lindholm
> ; Kumar, Rahul1 ; Yao,
> Jiewen ; Zhang, Chao B ;
> Zhang, Qi1 
> Subject: Re: [edk2-devel] [PATCH v5 0/9] Add new feature that evacuate
> temporary to permanent memory (CVE-2019-11098)
> 
> Guomin,
> 
> On 07/09/20 03:56, Guomin Jiang wrote:
> > The TOCTOU vulnerability allow that the physical present person to replace
> the code with the normal BootGuard check and PCR0 value.
> > The issue occur when BootGuard measure IBB and access flash code after
> NEM disable.
> > the reason why we access the flash code is that we have some pointer to
> flash.
> > To avoid this vulnerability, we need to convert those pointers, the patch
> series do this work and make sure that no code will access flash address.
> >
> > v2:
> > Create gEdkiiMigratedFvInfoGuid HOB and add
> PcdMigrateTemporaryRamFirmwareVolumes to control whole feature.
> >
> > v3:
> > Remove changes which is not related with the feature and disable the
> feature in virtual platform.
> >
> > v4:
> > Disable the feature as default, Copy the Tcg2Pei behavior to TcgPei
> >
> > v5:
> > Initialize local variable Shadow and return EFI_ABORTED when
> RepublishSecPpi not installed.
> 
> When you post a new version of a patch set to the list, and there is an
> associated BZ ticket, please *always* (not just for this BZ) capture the fact 
> of
> posting the next version in a new BZ comment. Please record the version of
> the patch series being posted, and also include a link to the series blurb
> (patch 0), in the mailing list archive.
> 
> I did that for you, covering the first four versions (v1 throuogh v4) of the
> series in comment 16 on TianoCore#1614:
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=1614#c16
> 
> Please do the same (in a new BZ comment) for the current version (v5), and
> please repeat the same for any further versions.
> 
> Again this applies to all BZs and all posted patches.
> 
> Thanks
> Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62334): https://edk2.groups.io/g/devel/message/62334
Mute This Topic: https://groups.io/mt/75390172/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers

[Gao, Zhichao]

It is highly unrecommended to initializes the local variable at its declaration 
like below. I didn't find the rule in the CCS 2.1 spec. But most of the edk2 
code never do like this. There are serval places like below, I just point out 
one example.
UINT16 NameSpaceStrLen = *(UINT16 *) Ptr;

For function ParseAcpiRsdp:
ProcessAcpiTable ((VOID *) *XsdtAddress);
Causing a warning and build failure with IA32 arch. I think the correct 
statement should be:
ProcessAcpiTable ((VOID *)(UINTN)*XsdtAddress);

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Tomas Pilar
> (tpilar)
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray ; Gao,
> Zhichao 
> Subject: [edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
> 
> The tests for checking specific constraints and checking
> for buffer overflows have been simplified to use a standard
> set of templates defined in the logging facility.
> 
> This regularises some of the error handling and makes
> it easier to write more tests like this in the future.
> 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Signed-off-by: Tomas Pilar 
> ---
>  .../UefiShellAcpiViewCommandLib/AcpiParser.c  |  25 ---
>  .../UefiShellAcpiViewCommandLib/AcpiParser.h  |  18 --
>  .../Arm/SbbrValidator.c   |  65 +++---
>  .../Parsers/Dbg2/Dbg2Parser.c | 118 +++---
>  .../Parsers/Fadt/FadtParser.c |  48 ++--
>  .../Parsers/Gtdt/GtdtParser.c |  84 ++-
>  .../Parsers/Iort/IortParser.c | 207 +++---
>  .../Parsers/Madt/MadtParser.c |  99 +++--
>  .../Parsers/Mcfg/McfgParser.c |  11 +-
>  .../Parsers/Pptt/PpttParser.c | 165 +++---
>  .../Parsers/Rsdp/RsdpParser.c |  42 +---
>  .../Parsers/Slit/SlitParser.c | 122 ---
>  .../Parsers/Spcr/SpcrParser.c |  31 +--
>  .../Parsers/Srat/SratParser.c | 188 +---
>  .../Parsers/Xsdt/XsdtParser.c |  92 ++--
>  15 files changed, 375 insertions(+), 940 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 9bbf724dfdfe..58599442d210 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -24,31 +24,6 @@ STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
>PARSE_ACPI_HEADER (&AcpiHdrInfo)
>  };
> 
> -/**
> -  This function increments the ACPI table error counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementErrorCount (
> -  VOID
> -  )
> -{
> -  mTableErrorCount++;
> -}
> -
> -/**
> -  This function increments the ACPI table warning counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementWarningCount (
> -  VOID
> -  )
> -{
> -  mTableWarningCount++;
> -}
> -
> -
>  /**
>This function verifies the ACPI table checksum.
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index bd3cdb774fb5..cdae433fef3b 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -18,24 +18,6 @@
>  /// that allows us to process the log options.
>  #define RSDP_TABLE_INFO  SIGNATURE_32('R', 'S', 'D', 'P')
> 
> -/**
> -  This function increments the ACPI table error counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementErrorCount (
> -  VOID
> -  );
> -
> -/**
> -  This function increments the ACPI table warning counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementWarningCount (
> -  VOID
> -  );
> -
>  /**
>This function verifies the ACPI table checksum.
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> index d3284417fa5f..ba80a5ab3b40 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> @@ -18,15 +18,16 @@
>  #include 
>  #include 
>  #include "AcpiParser.h"
> +#include "AcpiViewLog.h"
>  #include "Arm/SbbrValidator.h"
> 
>  /**
>SBBR specification version strings
>  **/
> -STATIC CONST CHAR8* ArmSbbrVersions[ArmSbbrVersionMax] = {
> -  "1.0", // ArmSbbrVersion_1_0
> -  "1.1", // ArmSbbrVersion_1_1
> -  "1.2"  // ArmSbbrVersion_1_2
> +STATIC CONST CHAR16* ArmSbbrVersions[ArmSbbrVersionMax] = {
> +  L"SBBR-v1.0", // ArmSbbrVersion_1_0
> +  L"SBBR-v1.1", // ArmSbbrVersion_1_1
> +  L"SBBR-v1.2"  // ArmSbbrVersion_1_2
>  };
> 
>  /**
> @@ -96,6 +97,16 @@ STATIC ACPI_TABLE_COUNTER ArmSbbrTableCounts[] = {
> 
> {EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGN
> ATURE, 0}
>  };
> 
> +STATIC_ASSERT (
> +  ARRAY_SIZE (ArmSbbr10Mandatory) <= ARRAY_SIZE 

Re: [edk2-devel] [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility

[Gao, Zhichao]

See below.

> -Original Message-
> From: Tomas Pilar 
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray ; Gao,
> Zhichao 
> Subject: [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility
> 
> Extract error and warning logging into separate methods. Fold highlighting and
> other output properties into the logging methods.
> 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Signed-off-by: Tomas Pilar 
> ---
>  .../UefiShellAcpiViewCommandLib/AcpiParser.c  |   5 +-
>  .../UefiShellAcpiViewCommandLib/AcpiViewLog.c | 230
> +  .../UefiShellAcpiViewCommandLib/AcpiViewLog.h | 233
> ++
>  .../UefiShellAcpiViewCommandLib.inf   |   2 +
>  4 files changed, 466 insertions(+), 4 deletions(-)  create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
>  create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 7017fa93efae..b88594cf3865 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -11,13 +11,10 @@
>  #include "AcpiParser.h"
>  #include "AcpiView.h"
>  #include "AcpiViewConfig.h"
> +#include "AcpiViewLog.h"
> 
>  STATIC UINT32   gIndent;
> 
> -// Publicly accessible error and warning counters.
> -UINT32   mTableErrorCount;
> -UINT32   mTableWarningCount;
> -
>  STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
> 
>  /**
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> new file mode 100644
> index ..9b9aaa855fdc
> --- /dev/null
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> @@ -0,0 +1,230 @@
> +/** @file
> +  'acpiview' logging and output facility
> +
> +  Copyright (c) 2020, ARM Limited. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#include "AcpiViewLog.h"
> +#include "AcpiViewConfig.h"
> +#include "AcpiParser.h"
> +#include 
> +
> +static CHAR16 mOutputBuffer [MAX_OUTPUT_SIZE] = { 0 };
> +
> +// String descriptions of error types
> +static const CHAR16* mErrorTypeDesc [ACPI_ERROR_MAX] = {
> +  L"Not an error",///< ACPI_ERROR_NONE
> +  L"Generic", ///< ACPI_ERROR_GENERIC
> +  L"Checksum",///< ACPI_ERROR_CSUM
> +  L"Parsing", ///< ACPI_ERROR_PARSE
> +  L"Length",  ///< ACPI_ERROR_LENGTH
> +  L"Value",   ///< ACPI_ERROR_VALUE
> +  L"Cross-check", ///< ACPI_ERROR_CROSS
> +};
> +
> +// Publicly accessible error and warning counters.
> +UINT32   mTableErrorCount;
> +UINT32   mTableWarningCount;
> +
> +/**
> +  Change the attributes of the standard output console
> +  to change the colour of the text according to the given
> +  severity of a log message.
> +
> +  @param[in] Severity  The severity of the log message that is being
> +   annotated with changed colour text.
> +  @param[in] OriginalAttribute The current attributes of ConOut that will be
> modified.
> +**/
> +static
> +VOID
> +EFIAPI
> +ApplyColor (
> +  IN ACPI_LOG_SEVERITY Severity,
> +  IN UINTN OriginalAttribute
> +  )
> +{
> +  if (!mConfig.ColourHighlighting) {
> +return;
> +  }
> +
> +  // Strip the foreground colour
> +  UINTN NewAttribute = OriginalAttribute & 0xF0;
> +
> +  // Add specific foreground colour based on severity  switch
> + (Severity) {  case ACPI_DEBUG:
> +NewAttribute |= EFI_DARKGRAY;
> +break;
> +  case ACPI_HIGHLIGHT:
> +NewAttribute |= EFI_LIGHTBLUE;
> +break;
> +  case ACPI_GOOD:
> +NewAttribute |= EFI_GREEN;
> +break;
> +  case ACPI_ITEM:
> +  case ACPI_WARN:
> +NewAttribute |= EFI_YELLOW;
> +break;
> +  case ACPI_BAD:
> +  case ACPI_ERROR:
> +  case ACPI_FATAL:
> +NewAttribute |= EFI_RED;
> +break;
> +  case ACPI_INFO:
> +  default:
> +NewAttribute |= OriginalAttribute;
> +break;
> +  }
> +
> +  gST->ConOut->SetAttribute (gST->ConOut, NewAttribute); }
> +
> +/**
> +  Restore ConOut text attributes.
> +
> +  @param[in] OriginalAttribute The attribute set that will be restored.
> +**/
> +static
> +VOID
> +EFIAPI
> +RestoreColor(
> +  IN UINTN OriginalAttribute
> +  )
> +{
> +  gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); }
> +
> +/**
> +  Formats and prints an ascii string to screen.
> +
> +  @param[in] Format String that will be formatted and printed.
> +  @param[in] Marker The marker for variable parameters to be formatted.
> +**/
> +static
> +VOID
> +EFIAPI
> +AcpiViewVSOutput (
> +  IN const CHAR16  *Format,
> +  IN VA_LIST Marker
> +  )
> +{
> +  UnicodeVSPrint (mOutputBuffer, sizeof(mOutputBuffer), Format,
> +Marker);
> +  gST->ConOut->OutputString(gST->ConOut, mOutputBuffer); }
> +
> +/**
> +  Formats a

Re: [edk2-devel] [PATCH 1/8] ShellPkg/AcpiView: Extract configuration struct

[Gao, Zhichao]

> -Original Message-
> From: Tomas Pilar 
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray ; Gao,
> Zhichao 
> Subject: [PATCH 1/8] ShellPkg/AcpiView: Extract configuration struct
> 
> Remove accessor method bloat by creating a configuration struct that is linked
> using an extern symbol in the config header file. Directly reference the 
> config
> struct for all read and write accesses in the entire module.
> 
> Rationalise the remaining methods in the config header and source.
> 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Signed-off-by: Tomas Pilar 
> ---
>  .../UefiShellAcpiViewCommandLib/AcpiParser.c  |  18 +-
>  .../AcpiTableParser.c |   4 +-
>  .../UefiShellAcpiViewCommandLib/AcpiView.c|  67 +++
>  .../AcpiViewConfig.c  | 180 ++
>  .../AcpiViewConfig.h  | 138 ++
>  .../UefiShellAcpiViewCommandLib.c |  18 +-
>  6 files changed, 71 insertions(+), 354 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 02f6d771c7e1..3a029b01cc20 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -137,7 +137,7 @@ VerifyChecksum (
>if (Log) {
>  OriginalAttribute = gST->ConOut->Mode->Attribute;
>  if (Checksum == 0) {
> -  if (GetColourHighlighting ()) {
> +  if (mConfig.ColourHighlighting) {
>  gST->ConOut->SetAttribute (
> gST->ConOut,
> EFI_TEXT_ATTR (EFI_GREEN, @@ -147,7 +147,7 @@
> VerifyChecksum (
>Print (L"Table Checksum : OK\n\n");
>  } else {
>IncrementErrorCount ();
> -  if (GetColourHighlighting ()) {
> +  if (mConfig.ColourHighlighting) {
>  gST->ConOut->SetAttribute (
> gST->ConOut,
> EFI_TEXT_ATTR (EFI_RED, @@ -156,7 +156,7 @@ 
> VerifyChecksum (
>}
>Print (L"Table Checksum : FAILED (0x%X)\n\n", Checksum);
>  }
> -if (GetColourHighlighting ()) {
> +if (mConfig.ColourHighlighting) {
>gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
>  }
>}
> @@ -507,7 +507,6 @@ ParseAcpi (
>  {
>UINT32  Index;
>UINT32  Offset;
> -  BOOLEAN HighLight;
>UINTN   OriginalAttribute;
> 
>//
> @@ -520,9 +519,8 @@ ParseAcpi (
>gIndent += Indent;
> 
>if (Trace && (AsciiName != NULL)){
> -HighLight = GetColourHighlighting ();
> 
> -if (HighLight) {
> +if (mConfig.ColourHighlighting) {
>OriginalAttribute = gST->ConOut->Mode->Attribute;
>gST->ConOut->SetAttribute (
>   gST->ConOut,
> @@ -537,7 +535,7 @@ ParseAcpi (
>(OUTPUT_FIELD_COLUMN_WIDTH - gIndent),
>AsciiName
>);
> -if (HighLight) {
> +if (mConfig.ColourHighlighting) {
>gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
>  }
>}
> @@ -555,8 +553,7 @@ ParseAcpi (
>continue;
>  }
> 
> -if (GetConsistencyChecking () &&
> -(Offset != Parser[Index].Offset)) {
> +if (mConfig.ConsistencyCheck && (Offset != Parser[Index].Offset)) {
>IncrementErrorCount ();
>Print (
>  L"\nERROR: %a: Offset Mismatch for %s\n"
> @@ -599,8 +596,7 @@ ParseAcpi (
> 
>  // Validating only makes sense if we are tracing
>  // the parsed table entries, to report by table name.
> -if (GetConsistencyChecking () &&
> -(Parser[Index].FieldValidator != NULL)) {
> +if (mConfig.ConsistencyCheck && (Parser[Index].FieldValidator
> + != NULL)) {
>Parser[Index].FieldValidator (Ptr, Parser[Index].Context);
>  }
>}
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> index 4b618f131eac..526cb8cb7cad 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> @@ -222,13 +222,13 @@ ProcessAcpiTable (
>return;
>  }
> 
> -if (GetConsistencyChecking ()) {
> +if (mConfig.ConsistencyCheck) {
>VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
>  }
>}
> 
>  #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> -  if (GetMandatoryTableValidate ()) {
> +  if (mConfig.MandatoryTableValidate) {
>  ArmSbbrIncrementTableCount (*AcpiTableSignature);
>}
>  #endif
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> index 9a5b013fb234..d2c14d5b8f5a 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> @@ -48,15 +48,14 @@ D

Re: [edk2-devel] [PATCH v5 0/9] Add new feature that evacuate temporary to permanent memory (CVE-2019-11098)

[Laszlo Ersek]

Guomin,

On 07/09/20 03:56, Guomin Jiang wrote:
> The TOCTOU vulnerability allow that the physical present person to replace 
> the code with the normal BootGuard check and PCR0 value.
> The issue occur when BootGuard measure IBB and access flash code after NEM 
> disable.
> the reason why we access the flash code is that we have some pointer to flash.
> To avoid this vulnerability, we need to convert those pointers, the patch 
> series do this work and make sure that no code will access flash address.
> 
> v2:
> Create gEdkiiMigratedFvInfoGuid HOB and add 
> PcdMigrateTemporaryRamFirmwareVolumes to control whole feature.
> 
> v3:
> Remove changes which is not related with the feature and disable the feature 
> in virtual platform.
> 
> v4:
> Disable the feature as default, Copy the Tcg2Pei behavior to TcgPei
> 
> v5:
> Initialize local variable Shadow and return EFI_ABORTED when RepublishSecPpi 
> not installed.

When you post a new version of a patch set to the list, and there is an
associated BZ ticket, please *always* (not just for this BZ) capture the
fact of posting the next version in a new BZ comment. Please record the
version of the patch series being posted, and also include a link to the
series blurb (patch 0), in the mailing list archive.

I did that for you, covering the first four versions (v1 throuogh v4) of
the series in comment 16 on TianoCore#1614:

  https://bugzilla.tianocore.org/show_bug.cgi?id=1614#c16

Please do the same (in a new BZ comment) for the current version (v5),
and please repeat the same for any further versions.

Again this applies to all BZs and all posted patches.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62330): https://edk2.groups.io/g/devel/message/62330
Mute This Topic: https://groups.io/mt/75390172/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision

[Laszlo Ersek]

(+Marc-André, Stefan)

On 07/10/20 02:44, Gao, Zhichao wrote:
> This bug is not obeserved by me. But I view the code. The condition is 
> incorrect and it would affect the TCG operation:
> if (!mIsTcg2PPVerLowerThan_1_3) {
> if (OperationRequest < 
> TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
>   //
>   // TCG2 PP1.3 spec defined operations that are reserved or 
> un-implemented
>   //
>   return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
> }
>   } else {
>//
>// TCG PP lower than 1.3. (1.0, 1.1, 1.2)
>//
>if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
>  RequestConfirmed = TRUE;
>} else if (OperationRequest < 
> TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
>  return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
>}
>   }
> 

I've found that code myself, but I'm not familiar enough with TPM PPI
stuff to understand immediately the effects of this change. I can see
that where we used to return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED before, we could now assign
"RequestConfirmed = TRUE", and vice versa, due to
"mIsTcg2PPVerLowerThan_1_3" being potentially inverted.

But what does that *mean*? What is the behavioral change that human
end-users, or software components, will experience?

Thanks
Laszlo

> So I think it should be fixed.
> 
> Thanks,
> Zhichao
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
>> Sent: Thursday, July 9, 2020 6:02 PM
>> To: devel@edk2.groups.io; Gao, Zhichao 
>> Cc: Terry Lee ; Yao, Jiewen ; Wang,
>> Jian J ; Zhang, Chao B 
>> Subject: Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
>> incorrect TCG VER comparision
>>
>> On 07/09/20 04:46, Gao, Zhichao wrote:
>>> From: Terry Lee 
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2697
>>>
>>> Tcg2PhysicalPresenceLibConstructor set the module variable
>>> mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version comparision.
>>>
>>> Cc: Jiewen Yao 
>>> Cc: Jian J Wang 
>>> Cc: Chao Zhang 
>>> Signed-off-by: Zhichao Gao 
>>> ---
>>>  .../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
>>> ceLib.c
>>> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
>>> ceLib.c
>>> index 1c46d5e69d..8afaa0a785 100644
>>> ---
>>> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
>>> ceLib.c
>>> +++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
>>> +++ esenceLib.c
>>> @@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor (  {
>>>EFI_STATUS  Status;
>>>
>>> -  if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
>>> *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
>>> sizeof(PP_INF_VERSION_1_2) - 1) <= 0) {
>>> +  if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
>>> + *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
>>> + sizeof(PP_INF_VERSION_1_2) - 1) >= 0) {
>>>  mIsTcg2PPVerLowerThan_1_3 = TRUE;
>>>}
>>>
>>>
>>
>> What is the practical impact of this bug / fix?
>>
>> Thanks
>> Laszlo
>>
>>
>> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62329): https://edk2.groups.io/g/devel/message/62329
Mute This Topic: https://groups.io/mt/75390754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2: PATCH] UefiCpuPkg: To enable X2Apic by default

[Laszlo Ersek]

On 07/10/20 05:31, Ji-yunX Lu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2845
> Platform shall enable X2APIC by default to meet requirements for interrupt 
> steering policy on Windows OS.
> 
> Change-Id: Ia9e24bce79c91762c560fa3de6260716939f0b1b
> Signed-off-by: Ji-yunX Lu 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 21 -
>  1 file changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ab7a8ed663..70bc5da195 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -488,8 +488,8 @@ CollectProcessorCount (
>)
>  {
>UINTN  Index;
> -  CPU_INFO_IN_HOB*CpuInfoInHob;
>BOOLEANX2Apic;
> +  CPUID_VERSION_INFO_ECX VersionInfoEcx;
>  
>//
>// Send 1st broadcast IPI to APs to wakeup APs
> @@ -505,26 +505,13 @@ CollectProcessorCount (
>  CpuPause ();
>}
>  
> -
> -  //
> -  // Enable x2APIC mode if
> -  //  1. Number of CPU is greater than 255; or
> -  //  2. There are any logical processors reporting an Initial APIC ID of 
> 255 or greater.
> -  //
>X2Apic = FALSE;
> -  if (CpuMpData->CpuCount > 255) {
> +  AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, &VersionInfoEcx.Uint32, NULL);
> +  if (VersionInfoEcx.Bits.x2APIC == 1) {
>  //
> -// If there are more than 255 processor found, force to enable X2APIC
> +// Enable x2APIC mode if capable
>  //
>  X2Apic = TRUE;
> -  } else {
> -CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
> -for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> -  if (CpuInfoInHob[Index].InitialApicId >= 0xFF) {
> -X2Apic = TRUE;
> -break;
> -  }
> -}
>}
>  
>if (X2Apic) {
> 

(1) I think this would break platforms that resolve the LocalApicLib
class to the "BaseXApicLib.inf" instance.

Based on the message of my earlier commit decb365b0016 ("OvmfPkg: select
LocalApicLib instance with x2apic support", 2015-11-30), it seems like
the BaseXApicLib instance notices and trips an assert when the LAPIC is
in X2APIC mode, at the next time a LocalApicLib API is used.

The BaseXApicLib instance contains many ASSERTs like this:

  ASSERT (GetApicMode () == LOCAL_APIC_MODE_XAPIC);

and the GetApicMode() function itself has the following ASSERT:

  ASSERT (ApicBaseMsr.Bits.EXTD == 0);

So, the change proposed in this patch needs to be gated by a boolean or
Feature PCD, and the PCD should default to FALSE. If the platform uses
the BaseXApicX2ApicLib instance, then it can set the PCD to TRUE.

In turn, for such platform DSCs that already use BaseXApicX2ApicLib
exclusively, in edk2 and in edk2-platforms, please post patches that set
the PCD to TRUE. This includes the OvmfPkg platform DSC files, for example.

(2) Please drop the "Change-Id" tag from the commit message.

(3) I think this patch would not pass PatchCheck.py; the commit message
is not wrapped correctly. The first line is 105 characters long.

(4) Please elaborate on the "requirements for interrupt steering policy
on Windows OS", in the commit message. I don't understand what that
means. If you have a link to MSDN or similar, please include that.

(5) Please take better care of the associated BZ
. Given that you
posted a patch, (a) the BZ should have status IN_PROGRESS, (b) you
should be set as the assignee, and (c) a BZ comment should link your
patch from the mailing list archive.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62328): https://edk2.groups.io/g/devel/message/62328
Mute This Topic: https://groups.io/mt/75413450/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch] MdeModulePkg/Variable/RuntimeDxe: Fix return status from Reclaim()

[Michael D Kinney]

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

Update Reclaim() to return the error status from the reclaim
operation and not the status of SynchronizeRuntimeVariableCache()
that can be EFI_SUCCESS even through the status from reclaim
is an error.  Without this change, the return status from
SetVariable() can be EFI_SUCCESS even though the variable was
not actually set.  This occurs if the variable store is full
and a Reclaim() is invoked to free up space and even after all
possible space is freed, there is still not enough room for
the variable being set.  This condition should return
EFI_OUT_OF_RESOURCES.

Cc: Hao A Wu 
Cc: Liming Gao 
Signed-off-by: Michael D Kinney 
---
 .../Universal/Variable/RuntimeDxe/Variable.c  | 30 +++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 1e71fc642c..41f8ff4ceb 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -531,6 +531,7 @@ Reclaim (
   VOID  *Point1;
   BOOLEAN   FoundAdded;
   EFI_STATUSStatus;
+  EFI_STATUSDoneStatus;
   UINTN CommonVariableTotalSize;
   UINTN CommonUserVariableTotalSize;
   UINTN HwErrVariableTotalSize;
@@ -774,25 +775,30 @@ Reclaim (
   }
 
 Done:
+  DoneStatus = EFI_SUCCESS;
   if (IsVolatile || mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
-Status =  SynchronizeRuntimeVariableCache (
-
&mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCache,
-0,
-VariableStoreHeader->Size
-);
-ASSERT_EFI_ERROR (Status);
+DoneStatus = SynchronizeRuntimeVariableCache (
+   
&mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCache,
+   0,
+   VariableStoreHeader->Size
+   );
+ASSERT_EFI_ERROR (DoneStatus);
 FreePool (ValidBuffer);
   } else {
 //
 // For NV variable reclaim, we use mNvVariableCache as the buffer, so copy 
the data back.
 //
 CopyMem (mNvVariableCache, (UINT8 *) (UINTN) VariableBase, 
VariableStoreHeader->Size);
-Status =  SynchronizeRuntimeVariableCache (
-
&mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache,
-0,
-VariableStoreHeader->Size
-);
-ASSERT_EFI_ERROR (Status);
+DoneStatus = SynchronizeRuntimeVariableCache (
+   
&mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache,
+   0,
+   VariableStoreHeader->Size
+   );
+ASSERT_EFI_ERROR (DoneStatus);
+  }
+
+  if (!EFI_ERROR (Status) && EFI_ERROR (DoneStatus)) {
+Status = DoneStatus;
   }
 
   return Status;
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62327): https://edk2.groups.io/g/devel/message/62327
Mute This Topic: https://groups.io/mt/75412007/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/1] CryptoPkg/OpensslLib: Upgrade OpenSSL to 1.1.1g

[Guomin Jiang]

Upgrade openssl to 1.1.1g. the directory have been reorganized,
openssl moved crypto/include/internal to include/crypto folder.
So we change directory to match the re-organization.

Cc: Jian J Wang 
Cc: Xiaoyu Lu 
Signed-off-by: GuoMinJ 
---
 CryptoPkg/CryptoPkg.dec   |  1 -
 CryptoPkg/Library/OpensslLib/OpensslLib.inf   | 58 +--
 .../Library/OpensslLib/OpensslLibCrypto.inf   | 50 
 .../Include/{internal => crypto}/dso_conf.h   | 32 +-
 .../Library/Include/openssl/opensslconf.h |  3 -
 .../Library/BaseCryptLib/Hash/CryptSm3.c  |  2 +-
 .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c |  4 +-
 CryptoPkg/Library/OpensslLib/rand_pool.c  |  2 +-
 CryptoPkg/Library/OpensslLib/openssl  |  2 +-
 CryptoPkg/Library/OpensslLib/process_files.pl | 10 ++--
 10 files changed, 80 insertions(+), 84 deletions(-)
 rename CryptoPkg/Library/Include/{internal => crypto}/dso_conf.h (76%)

diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index 4d1a1368a8d4..5888941bab4c 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -23,7 +23,6 @@ [Includes.Common.Private]
   Private
   Library/Include
   Library/OpensslLib/openssl/include
-  Library/OpensslLib/openssl/crypto/include
 
 [LibraryClasses]
   ##  @libraryclass  Provides basic library functions for cryptographic 
primitives.
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index c8ec9454bd90..dbbe5386a10c 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -477,45 +477,45 @@ [Sources]
   $(OPENSSL_PATH)/crypto/s390x_arch.h
   $(OPENSSL_PATH)/crypto/sparc_arch.h
   $(OPENSSL_PATH)/crypto/vms_rms.h
-  $(OPENSSL_PATH)/crypto/aes/aes_locl.h
+  $(OPENSSL_PATH)/crypto/aes/aes_local.h
   $(OPENSSL_PATH)/crypto/asn1/asn1_item_list.h
-  $(OPENSSL_PATH)/crypto/asn1/asn1_locl.h
+  $(OPENSSL_PATH)/crypto/asn1/asn1_local.h
   $(OPENSSL_PATH)/crypto/asn1/charmap.h
   $(OPENSSL_PATH)/crypto/asn1/standard_methods.h
   $(OPENSSL_PATH)/crypto/asn1/tbl_standard.h
-  $(OPENSSL_PATH)/crypto/async/async_locl.h
+  $(OPENSSL_PATH)/crypto/async/async_local.h
   $(OPENSSL_PATH)/crypto/async/arch/async_null.h
   $(OPENSSL_PATH)/crypto/async/arch/async_posix.h
   $(OPENSSL_PATH)/crypto/async/arch/async_win.h
-  $(OPENSSL_PATH)/crypto/bio/bio_lcl.h
-  $(OPENSSL_PATH)/crypto/bn/bn_lcl.h
+  $(OPENSSL_PATH)/crypto/bio/bio_local.h
+  $(OPENSSL_PATH)/crypto/bn/bn_local.h
   $(OPENSSL_PATH)/crypto/bn/bn_prime.h
   $(OPENSSL_PATH)/crypto/bn/rsaz_exp.h
-  $(OPENSSL_PATH)/crypto/comp/comp_lcl.h
+  $(OPENSSL_PATH)/crypto/comp/comp_local.h
   $(OPENSSL_PATH)/crypto/conf/conf_def.h
-  $(OPENSSL_PATH)/crypto/conf/conf_lcl.h
-  $(OPENSSL_PATH)/crypto/dh/dh_locl.h
-  $(OPENSSL_PATH)/crypto/dso/dso_locl.h
-  $(OPENSSL_PATH)/crypto/evp/evp_locl.h
-  $(OPENSSL_PATH)/crypto/hmac/hmac_lcl.h
-  $(OPENSSL_PATH)/crypto/lhash/lhash_lcl.h
-  $(OPENSSL_PATH)/crypto/md5/md5_locl.h
-  $(OPENSSL_PATH)/crypto/modes/modes_lcl.h
+  $(OPENSSL_PATH)/crypto/conf/conf_local.h
+  $(OPENSSL_PATH)/crypto/dh/dh_local.h
+  $(OPENSSL_PATH)/crypto/dso/dso_local.h
+  $(OPENSSL_PATH)/crypto/evp/evp_local.h
+  $(OPENSSL_PATH)/crypto/hmac/hmac_local.h
+  $(OPENSSL_PATH)/crypto/lhash/lhash_local.h
+  $(OPENSSL_PATH)/crypto/md5/md5_local.h
+  $(OPENSSL_PATH)/crypto/modes/modes_local.h
   $(OPENSSL_PATH)/crypto/objects/obj_dat.h
-  $(OPENSSL_PATH)/crypto/objects/obj_lcl.h
+  $(OPENSSL_PATH)/crypto/objects/obj_local.h
   $(OPENSSL_PATH)/crypto/objects/obj_xref.h
-  $(OPENSSL_PATH)/crypto/ocsp/ocsp_lcl.h
-  $(OPENSSL_PATH)/crypto/pkcs12/p12_lcl.h
-  $(OPENSSL_PATH)/crypto/rand/rand_lcl.h
-  $(OPENSSL_PATH)/crypto/rsa/rsa_locl.h
-  $(OPENSSL_PATH)/crypto/sha/sha_locl.h
+  $(OPENSSL_PATH)/crypto/ocsp/ocsp_local.h
+  $(OPENSSL_PATH)/crypto/pkcs12/p12_local.h
+  $(OPENSSL_PATH)/crypto/rand/rand_local.h
+  $(OPENSSL_PATH)/crypto/rsa/rsa_local.h
+  $(OPENSSL_PATH)/crypto/sha/sha_local.h
   $(OPENSSL_PATH)/crypto/siphash/siphash_local.h
-  $(OPENSSL_PATH)/crypto/sm3/sm3_locl.h
-  $(OPENSSL_PATH)/crypto/store/store_locl.h
-  $(OPENSSL_PATH)/crypto/ui/ui_locl.h
-  $(OPENSSL_PATH)/crypto/x509/x509_lcl.h
+  $(OPENSSL_PATH)/crypto/sm3/sm3_local.h
+  $(OPENSSL_PATH)/crypto/store/store_local.h
+  $(OPENSSL_PATH)/crypto/ui/ui_local.h
+  $(OPENSSL_PATH)/crypto/x509/x509_local.h
   $(OPENSSL_PATH)/crypto/x509v3/ext_dat.h
-  $(OPENSSL_PATH)/crypto/x509v3/pcy_int.h
+  $(OPENSSL_PATH)/crypto/x509v3/pcy_local.h
   $(OPENSSL_PATH)/crypto/x509v3/standard_exts.h
   $(OPENSSL_PATH)/crypto/x509v3/v3_admis.h
   $(OPENSSL_PATH)/ssl/bio_ssl.c
@@ -562,13 +562,13 @@ [Sources]
   $(OPENSSL_PATH)/ssl/t1_trce.c
   $(OPENSSL_PATH)/ssl/tls13_enc.c
   $(OPENSSL_PATH)/ssl/tls_srp.c
-  $(OPENSSL_PATH)/ssl/packet_locl.h
+  $(OPENSSL_PATH)/ssl/packet_local.h
   $(OPENSSL_PATH)/ssl/ssl_cert_table.h
-  $(OPENSSL_PATH)/ssl/ssl_locl.h
+  $(OPENSSL_PATH)/ssl/ssl_local.h
   $(OPENSSL_P

Re: [edk2-devel] [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the partition check issue

[Wu, Hao A]

> -Original Message-
> From: Gao, Zhichao 
> Sent: Friday, July 10, 2020 8:41 AM
> To: Wu, Hao A ; devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: RE: [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the partition
> check issue
> 
> I have tested on the OVMF with linux iso image like Ubuntu, Fedora and
> RedHat.
> 
> It can enumerate under the shell and load the grub efi application.


Thanks Zhichao,

For the series,
Reviewed-by: Hao A Wu 

Best Regards,
Hao Wu


> 
> Thanks,
> Zhichao
> 
> > -Original Message-
> > From: Wu, Hao A 
> > Sent: Thursday, July 9, 2020 1:04 PM
> > To: Gao, Zhichao ; devel@edk2.groups.io
> > Cc: Ni, Ray ; Laszlo Ersek 
> > Subject: RE: [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the
> > partition check issue
> >
> > > -Original Message-
> > > From: Gao, Zhichao 
> > > Sent: Wednesday, July 8, 2020 10:27 AM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A ; Ni, Ray ;
> > > Laszlo Ersek 
> > > Subject: [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the partition
> > > check issue
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> > >
> > > V1:
> > > Separate the UDF from the partition rountine array and do the check
> > > for every media.
> > >
> > > V2:
> > > Drop V1 because it is a bug: there should not be two partition types
> > > in one media.
> > > 1. Correct the LastBlock value in MBR handler. It should be the
> > > number of sectors (512 bytes).
> > > 2. Skip the MBR check if the MBR is added for the Windows
> > > comaptiblity. We treat such media as ElTorito and do the check.
> > > 3. Fix the partition check bug: One partition type returns already
> > > started should be treated as success to avoid multi partition type
> > > be installed into same media.
> >
> >
> > Hello Zhichao,
> >
> > The changes look good to me.
> > May I know what tests have been done for the series? Thanks.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Cc: Hao A Wu 
> > > Cc: Ray Ni 
> > > Cc: Laszlo Ersek 
> > > Signed-off-by: Zhichao Gao 
> > >
> > > Zhichao Gao (3):
> > >   MdeModulePkg/PartitionDxe: Correct the MBR last block value
> > >   MdeModulePkg/PartitionDxe: Skip the MBR that add for CD-ROM
> > >   MdeModulePkg/PartitionDxe: Add already start check for child
> > > hanldes
> > >
> > >  .../Universal/Disk/PartitionDxe/Mbr.c | 42 +++
> > >  .../Universal/Disk/PartitionDxe/Partition.c   |  9 
> > >  2 files changed, 44 insertions(+), 7 deletions(-)
> > >
> > > --
> > > 2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62325): https://edk2.groups.io/g/devel/message/62325
Mute This Topic: https://groups.io/mt/75369396/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision

[Gao, Zhichao]

This bug is not obeserved by me. But I view the code. The condition is 
incorrect and it would affect the TCG operation:
if (!mIsTcg2PPVerLowerThan_1_3) {
if (OperationRequest < 
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
  //
  // TCG2 PP1.3 spec defined operations that are reserved or 
un-implemented
  //
  return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
  } else {
   //
   // TCG PP lower than 1.3. (1.0, 1.1, 1.2)
   //
   if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
 RequestConfirmed = TRUE;
   } else if (OperationRequest < 
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
 return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
   }
  }

So I think it should be fixed.

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
> Sent: Thursday, July 9, 2020 6:02 PM
> To: devel@edk2.groups.io; Gao, Zhichao 
> Cc: Terry Lee ; Yao, Jiewen ; Wang,
> Jian J ; Zhang, Chao B 
> Subject: Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
> incorrect TCG VER comparision
> 
> On 07/09/20 04:46, Gao, Zhichao wrote:
> > From: Terry Lee 
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2697
> >
> > Tcg2PhysicalPresenceLibConstructor set the module variable
> > mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version comparision.
> >
> > Cc: Jiewen Yao 
> > Cc: Jian J Wang 
> > Cc: Chao Zhang 
> > Signed-off-by: Zhichao Gao 
> > ---
> >  .../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
> > ceLib.c
> > b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
> > ceLib.c
> > index 1c46d5e69d..8afaa0a785 100644
> > ---
> > a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
> > ceLib.c
> > +++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
> > +++ esenceLib.c
> > @@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor (  {
> >EFI_STATUS  Status;
> >
> > -  if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
> > *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
> > sizeof(PP_INF_VERSION_1_2) - 1) <= 0) {
> > +  if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
> > + *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
> > + sizeof(PP_INF_VERSION_1_2) - 1) >= 0) {
> >  mIsTcg2PPVerLowerThan_1_3 = TRUE;
> >}
> >
> >
> 
> What is the practical impact of this bug / fix?
> 
> Thanks
> Laszlo
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62324): https://edk2.groups.io/g/devel/message/62324
Mute This Topic: https://groups.io/mt/75390754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the partition check issue

[Gao, Zhichao]

I have tested on the OVMF with linux iso image like Ubuntu, Fedora and RedHat.

It can enumerate under the shell and load the grub efi application.

Thanks,
Zhichao

> -Original Message-
> From: Wu, Hao A 
> Sent: Thursday, July 9, 2020 1:04 PM
> To: Gao, Zhichao ; devel@edk2.groups.io
> Cc: Ni, Ray ; Laszlo Ersek 
> Subject: RE: [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the partition check
> issue
> 
> > -Original Message-
> > From: Gao, Zhichao 
> > Sent: Wednesday, July 8, 2020 10:27 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A ; Ni, Ray ; Laszlo
> > Ersek 
> > Subject: [PATCH V2 0/3] MdeModulePkg/PartitionDxe: Fix the partition
> > check issue
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> >
> > V1:
> > Separate the UDF from the partition rountine array and do the check
> > for every media.
> >
> > V2:
> > Drop V1 because it is a bug: there should not be two partition types
> > in one media.
> > 1. Correct the LastBlock value in MBR handler. It should be the number
> > of sectors (512 bytes).
> > 2. Skip the MBR check if the MBR is added for the Windows
> > comaptiblity. We treat such media as ElTorito and do the check.
> > 3. Fix the partition check bug: One partition type returns already
> > started should be treated as success to avoid multi partition type be
> > installed into same media.
> 
> 
> Hello Zhichao,
> 
> The changes look good to me.
> May I know what tests have been done for the series? Thanks.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Cc: Hao A Wu 
> > Cc: Ray Ni 
> > Cc: Laszlo Ersek 
> > Signed-off-by: Zhichao Gao 
> >
> > Zhichao Gao (3):
> >   MdeModulePkg/PartitionDxe: Correct the MBR last block value
> >   MdeModulePkg/PartitionDxe: Skip the MBR that add for CD-ROM
> >   MdeModulePkg/PartitionDxe: Add already start check for child hanldes
> >
> >  .../Universal/Disk/PartitionDxe/Mbr.c | 42 +++
> >  .../Universal/Disk/PartitionDxe/Partition.c   |  9 
> >  2 files changed, 44 insertions(+), 7 deletions(-)
> >
> > --
> > 2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62323): https://edk2.groups.io/g/devel/message/62323
Mute This Topic: https://groups.io/mt/75369396/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v2 02/16] MdePkg/BaseCpuLibNull: Add Null version of CpuLib for host testing

[Sean]

Reviewed-by: Sean Brogan 

On 7/8/2020 9:05 PM, Michael D Kinney wrote:

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

The services in CpuLib usually generate exceptions in a unit test
host application.  Provide a Null instance that can be safely used.

This Null instance can also be used as a template for implementing
new instances of CpuLib.

Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Jiewen Yao 
Signed-off-by: Michael D Kinney 
---
  .../Library/BaseCpuLibNull/BaseCpuLibNull.c   | 37 +++
  .../Library/BaseCpuLibNull/BaseCpuLibNull.inf | 26 +
  .../Library/BaseCpuLibNull/BaseCpuLibNull.uni | 11 ++
  MdePkg/MdePkg.dsc |  3 +-
  4 files changed, 76 insertions(+), 1 deletion(-)
  create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
  create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
  create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni

diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c 
b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
new file mode 100644
index 00..3ba7a35096
--- /dev/null
+++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
@@ -0,0 +1,37 @@
+/** @file
+  Null instance of CPU Library.
+
+  Copyright (c) 2020, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+/**
+  Places the CPU in a sleep state until an interrupt is received.
+
+  Places the CPU in a sleep state until an interrupt is received. If interrupts
+  are disabled prior to calling this function, then the CPU will be placed in a
+  sleep state indefinitely.
+
+**/
+VOID
+EFIAPI
+CpuSleep (
+  VOID
+  )
+{
+}
+
+/**
+  Flushes all the Translation Lookaside Buffers(TLB) entries in a CPU.
+
+  Flushes all the Translation Lookaside Buffers(TLB) entries in a CPU.
+
+**/
+VOID
+EFIAPI
+CpuFlushTlb (
+  VOID
+  )
+{
+}
diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf 
b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
new file mode 100644
index 00..a9e8399038
--- /dev/null
+++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
@@ -0,0 +1,26 @@
+## @file
+#  Null instance of CPU Library.
+#
+#  Copyright (c) 2020, Intel Corporation. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = BaseCpuLibNull
+  MODULE_UNI_FILE= BaseCpuLibNull.uni
+  FILE_GUID  = 8A29AAA5-0FB7-44CC-8709-1344FE89B878
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = CpuLib
+
+#
+#  VALID_ARCHITECTURES   = IA32 X64 EBC ARM AARCH64 RISCV64
+#
+
+[Sources]
+  BaseCpuLibNull.c
+
+[Packages]
+  MdePkg/MdePkg.dec
diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni 
b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
new file mode 100644
index 00..1030221d5c
--- /dev/null
+++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
@@ -0,0 +1,11 @@
+// /** @file
+// Null instance of CPU Library.
+//
+// Copyright (c) 2020, Intel Corporation. All rights reserved.
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_MODULE_ABSTRACT #language en-US "Null Instance of CPU 
Library"
+
+#string STR_MODULE_DESCRIPTION  #language en-US "Null instance of CPU 
Library."
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 6cd38e7ec3..3abe65ec7f 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -1,7 +1,7 @@
  ## @file
  # EFI/PI MdePkg Package
  #
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
  # (C) Copyright 2020 Hewlett Packard Enterprise Development LP
  #
@@ -36,6 +36,7 @@ [Components]
MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+  MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62322): https://edk2.groups.io/g/devel/message/62322
Mute This Topic: https://groups.io/mt/75391609/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch v2 01/16] BaseTools/Python: Allow HOST_APPLICATION to use NULL libraries

[Sean]

Reviewed-by: Sean Brogan 

On 7/8/2020 9:05 PM, Michael D Kinney wrote:

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

Update HOST_APPLICATION module type to use NULL library instances.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Jiewen Yao 
Signed-off-by: Michael D Kinney 
---
  BaseTools/Source/Python/Workspace/WorkspaceCommon.py | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py 
b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
index 913e710fd9..53027a0e30 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
@@ -1,7 +1,7 @@
  ## @file
  # Common routines used by workspace
  #
-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved.
  # SPDX-License-Identifier: BSD-2-Clause-Patent
  #
  
@@ -100,7 +100,7 @@ def GetModuleLibInstances(Module, Platform, BuildDatabase, Arch, Target, Toolcha

  # If a module has a MODULE_TYPE of USER_DEFINED,
  # do not link in NULL library class instances from the global 
[LibraryClasses.*] sections.
  #
-if Module.ModuleType != SUP_MODULE_USER_DEFINED and Module.ModuleType != 
SUP_MODULE_HOST_APPLICATION:
+if Module.ModuleType != SUP_MODULE_USER_DEFINED:
  for LibraryClass in Platform.LibraryClasses.GetKeys():
  if LibraryClass.startswith("NULL") and 
Platform.LibraryClasses[LibraryClass, Module.ModuleType]:
  Module.LibraryClasses[LibraryClass] = 
Platform.LibraryClasses[LibraryClass, Module.ModuleType]



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62321): https://edk2.groups.io/g/devel/message/62321
Mute This Topic: https://groups.io/mt/75391608/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 0/3] Add Host Based Unit Tests for BaseCryptLib to CryptoPkg

[matthewfcarlson]

From: Matthew Carlson 

This turns on Host Based Unit Tests for CryptoPkg, adds the unit test itself, 
and 
adds a POSIX BaseTimerLib for unit tests.

Matthew Carlson (3):
  UnitTestFrameworkPkg : BaseTimerLibPosix: Adds a host-based timer Lib
  CryptoPkg: BaseCryptLib: Add unit tests (Host and Shell based)
  AzurePipelines : Pr Gate: Turn on HBUT for CryptoPkg

 CryptoPkg/Library/BaseCryptLib/SysCall/UnitTestHostCrtWrapper.c
|   93 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/AuthenticodeTests.c   
| 1002 
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c   
|   66 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BlockCipherTests.c
|  293 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/DhTests.c 
|  106 +++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/HashTests.c   
|  197 
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/HmacTests.c   
|  184 
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/OaepEncryptTests.c
|  308 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs5Pbkdf2Tests.c
|   71 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs7EkuTests.c   
|  524 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RandTests.c   
|   51 +
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPkcs7Tests.c   
|  415 
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c
|  310 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TSTests.c 
|  335 +++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/UnitTestMain.c
|   81 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/UnitTestMainBCOP.c
|   58 ++
 UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/TimerLibPosix.c   
|  125 +++
 .azurepipelines/templates/pr-gate-build-job.yml
|2 +-
 CryptoPkg/CryptoPkg.ci.yaml
|6 +-
 CryptoPkg/CryptoPkg.dsc
|   26 +
 CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf
|   90 ++
 CryptoPkg/Test/UnitTest/CryptoPkgHostUnitTest.dsc  
|   35 +
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs7EkuTestSignatures.h  
|  789 +++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h
|  121 +++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf  
|   46 +
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibUefiShell.inf 
|   49 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/ChainCreationInstructions.txt
|   92 ++
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/CreateTestCerts.cmd  
|   11 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/SignFirmwareWithEKUs.cmd
 |   76 ++
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingIssuingCA.ini
  |   45 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingLeafSigner.ini
 |   25 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingLeafSignerPid1.ini
 |   24 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingLeafSignerPid12345.ini
 |   27 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingNoEKUsInSigner.ini
 |   16 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingPolicyCA.ini
   |   28 +
 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsingRoot.ini
   |   28 +
 UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.inf 
|   38 +
 UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.uni 
|   15 +
 UnitTestFrameworkPkg/Test/UnitTestFrameworkPkgHostTest.dsc 
|1 +
 UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc  
|4 +-
 40 files changed, 5808 insertions(+), 5 deletions(-)
 create mode 100644 
CryptoPkg/Library/BaseCryptLib/SysCall/UnitTestHostCrtWrapper.c
 create mode 100644 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/AuthenticodeTests.c
 create mode 100644 
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/B

[edk2-devel] [PATCH v2 3/3] AzurePipelines : Pr Gate: Turn on HBUT for CryptoPkg

[matthewfcarlson]

From: Matthew Carlson 

Turns on Host Based Unit Tests for CryptoPkg by enabling the target
NOOPT in the CI pipeline.

Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Michael D Kinney 
Cc: Liming Gao 

Signed-off-by: Matthew Carlson 
---
 .azurepipelines/templates/pr-gate-build-job.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.azurepipelines/templates/pr-gate-build-job.yml 
b/.azurepipelines/templates/pr-gate-build-job.yml
index a9f89aa68451..e84ba80030b1 100644
--- a/.azurepipelines/templates/pr-gate-build-job.yml
+++ b/.azurepipelines/templates/pr-gate-build-job.yml
@@ -40,7 +40,7 @@ jobs:
 Build.Targets: 'DEBUG,RELEASE,NO-TARGET,NOOPT'
   TARGET_CRYPTO:
 Build.Pkgs: 'CryptoPkg'
-Build.Targets: 'DEBUG,RELEASE,NO-TARGET'
+Build.Targets: 'DEBUG,RELEASE,NO-TARGET,NOOPT'
   TARGET_SECURITY:
 Build.Pkgs: 'SecurityPkg'
 Build.Targets: 'DEBUG,RELEASE,NO-TARGET'
-- 
2.27.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62319): https://edk2.groups.io/g/devel/message/62319
Mute This Topic: https://groups.io/mt/75405699/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/3] UnitTestFrameworkPkg : BaseTimerLibPosix: Adds a host-based timer Lib

[matthewfcarlson]

From: Matthew Carlson 

This adds a host based BaseTimerLib that won't assert.

Cc: Michael D Kinney 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Signed-off-by: Matthew Carlson 
---
 UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/TimerLibPosix.c   | 
125 
 UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.inf |  
38 ++
 UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.uni |  
15 +++
 UnitTestFrameworkPkg/Test/UnitTestFrameworkPkgHostTest.dsc |   
1 +
 UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc  |   
4 +-
 5 files changed, 180 insertions(+), 3 deletions(-)

diff --git 
a/UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/TimerLibPosix.c 
b/UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/TimerLibPosix.c
new file mode 100644
index ..cd12dde7bd81
--- /dev/null
+++ b/UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/TimerLibPosix.c
@@ -0,0 +1,125 @@
+/** @file
+  A semi-functional instance of the Timer Library.
+
+  Copyright (c) 2007 - 2011, Intel Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Stalls the CPU for at least the given number of microseconds.
+
+  @param  MicroSeconds  The minimum number of microseconds to delay.
+  @return The value of MicroSeconds inputted.
+
+**/
+UINTN
+EFIAPI
+MicroSecondDelay (
+  IN  UINTN MicroSeconds
+  )
+{
+  return NanoSecondDelay(MicroSeconds * 100) / 100;
+}
+
+/**
+  Stalls the CPU for at least the given number of nanoseconds.
+
+  @param  NanoSeconds The minimum number of nanoseconds to delay.
+  @return The value of NanoSeconds inputted.
+
+**/
+UINTN
+EFIAPI
+NanoSecondDelay (
+  IN  UINTN NanoSeconds
+  )
+{
+  // Since this is a host based test, we don't actually want to stall
+  return 0;
+}
+
+/**
+  Retrieves the current value of a 64-bit free running performance counter.
+
+  The counter can either count up by 1 or count down by 1. If the physical
+  performance counter counts by a larger increment, then the counter values
+  must be translated. The properties of the counter can be retrieved from
+  GetPerformanceCounterProperties().
+
+  @return The current value of the free running performance counter.
+
+**/
+UINT64
+EFIAPI
+GetPerformanceCounter (
+  VOID
+  )
+{
+  // Return the current number of nanoseconds on the clock
+  struct timespec ts;
+  timespec_get(&ts, TIME_UTC);
+  return ts.tv_nsec;
+}
+
+/**
+  Retrieves the 64-bit frequency in Hz and the range of performance counter
+  values.
+
+  If StartValue is not NULL, then the value that the performance counter starts
+  with immediately after is it rolls over is returned in StartValue. If
+  EndValue is not NULL, then the value that the performance counter end with
+  immediately before it rolls over is returned in EndValue. The 64-bit
+  frequency of the performance counter in Hz is always returned. If StartValue
+  is less than EndValue, then the performance counter counts up. If StartValue
+  is greater than EndValue, then the performance counter counts down. For
+  example, a 64-bit free running counter that counts up would have a StartValue
+  of 0 and an EndValue of 0x. A 24-bit free running counter
+  that counts down would have a StartValue of 0xFF and an EndValue of 0.
+
+  @param  StartValue  The value the performance counter starts with when it
+  rolls over.
+  @param  EndValueThe value that the performance counter ends with before
+  it rolls over.
+
+  @return The frequency in Hz.
+
+**/
+UINT64
+EFIAPI
+GetPerformanceCounterProperties (
+  OUT  UINT64*StartValue,  OPTIONAL
+  OUT  UINT64*EndValue OPTIONAL
+  )
+{
+  return (UINT64)(-1);
+}
+
+/**
+  Converts elapsed ticks of performance counter to time in nanoseconds.
+
+  This function converts the elapsed ticks of running performance counter to
+  time value in unit of nanoseconds.
+
+  @param  Ticks The number of elapsed ticks of running performance counter.
+
+  @return The elapsed time in nanoseconds.
+
+**/
+UINT64
+EFIAPI
+GetTimeInNanoSecond (
+  IN  UINT64 Ticks
+  )
+{
+  return Ticks;
+}
diff --git 
a/UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.inf 
b/UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.inf
new file mode 100644
index ..aed6f15e1828
--- /dev/null
+++ b/UnitTestFrameworkPkg/Library/Posix/BaseTimerLibPosix/BaseTimerLibPosix.inf
@@ -0,0 +1,38 @@
+## @file
+#  An instance of Timer Library for posix compliant hosts.
+#
+#  A semi-functional instance of the Timer Library that can be used for host 
based testing
+#  as a functional timer library instance.
+
+#
+#  C

Re: [edk2-devel] [PATCH v1 1/3] UnitTestFrameworkPkg : BaseTimerLib: Adds a host-based timer Lib

[Matthew Carlson via groups.io]

I sent out a new patch series. Not sure, it might be waiting in the
--
- Matthew Carlson

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62316): https://edk2.groups.io/g/devel/message/62316
Mute This Topic: https://groups.io/mt/75379678/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platforms] [PATCH v3 0/2] Add USB driver support

[Meenakshi Aggarwal]

This patchset implement USB driver for DWC3 controller and enable
USB for LX2160A Platform.

Changes in v2:
- Indentation changes
- Incorporated review comments
- create EndOfDxe event and initialize USB in EndOfDxe
  callback function.

Changes in v3:
- Passing Usb initialization function as init function
  of RegisterNonDiscoverableMmioDevice()

Meenakshi Aggarwal (2):
  Silicon/NXP: Add DWC3 USB controller initialization driver
  Platform/NXP:LX2160: Enable support of USB controller

 Silicon/NXP/NxpQoriqLs.dec   |   5 +
 Silicon/NXP/LX2160A/LX2160A.dsc.inc  |   4 +
 Silicon/NXP/NxpQoriqLs.dsc.inc   |  12 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc |   1 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf |  18 +-
 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf |  45 
 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h   | 138 ++
 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c   | 275 
 8 files changed, 495 insertions(+), 3 deletions(-)
 create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
 create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
 create mode 100644 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c

-- 
1.9.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62313): https://edk2.groups.io/g/devel/message/62313
Mute This Topic: https://groups.io/mt/75403794/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platforms] [PATCH v3 2/2] Platform/NXP:LX2160: Enable support of USB controller

[Meenakshi Aggarwal]

From: Meenakshi Aggarwal 

- Enable support of USB drives on lx2160 RDB board.
- LX2160 has DWC3 controller
- Increase FD size to accomodate USB driver.

Signed-off-by: Meenakshi Aggarwal 
---
 Silicon/NXP/LX2160A/LX2160A.dsc.inc  |  4 
 Silicon/NXP/NxpQoriqLs.dsc.inc   | 12 
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc |  1 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf | 18 +++---
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc 
b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
index af22b4dd973c..55dd3b5442eb 100644
--- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
+++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
@@ -36,6 +36,10 @@ [PcdsFixedAtBuild.common]
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x21C
 
+  gNxpQoriqLsTokenSpaceGuid.PcdUsbBaseAddr|0x310
+  gNxpQoriqLsTokenSpaceGuid.PcdUsbSize|0x1
+  gNxpQoriqLsTokenSpaceGuid.PcdNumUsbController|2
+
 [PcdsFeatureFlag]
   gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|TRUE
 
diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
index ee639d552483..1f0f272da6c7 100644
--- a/Silicon/NXP/NxpQoriqLs.dsc.inc
+++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
@@ -366,6 +366,18 @@ [Components.common]
   FatPkg/EnhancedFatDxe/Fat.inf
 
   #
+  # USB
+  #
+
+  MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
+  MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
+  MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
+  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
+  MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+
+  
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+
+  #
   # Bds
   #
   MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc 
b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
index 9b3e0386c13e..ec27a1a219a5 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc
@@ -43,4 +43,5 @@ [Components.common]
 gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
   }
 
+  Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
  ##
diff --git a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf 
b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
index eec1c0774a86..5cb809e8b3a0 100644
--- a/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
+++ b/Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf
@@ -23,10 +23,10 @@
 
 [FD.LX2160ARDB_EFI]
 BaseAddress   = 0x8200|gArmTokenSpaceGuid.PcdFdBaseAddress  #The base 
address of the FLASH Device.
-Size  = 0x0014|gArmTokenSpaceGuid.PcdFdSize #The size in 
bytes of the FLASH Device
+Size  = 0x0016|gArmTokenSpaceGuid.PcdFdSize #The size in 
bytes of the FLASH Device
 ErasePolarity = 1
 BlockSize = 0x1
-NumBlocks = 0x14
+NumBlocks = 0x16
 
 

 #
@@ -43,7 +43,7 @@ [FD.LX2160ARDB_EFI]
 # RegionType 
 #
 

-0x|0x0014
+0x|0x0016
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
@@ -120,6 +120,18 @@ [FV.FvMain]
   INF FatPkg/EnhancedFatDxe/Fat.inf
   INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
 
+  INF 
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+
+  #
+  # USB Support
+  #
+  INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
+  INF MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
+  INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
+  INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
+  INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+
+  INF Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
   #
   # UEFI application (Shell Embedded Boot Loader)
   #
-- 
1.9.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62314): https://edk2.groups.io/g/devel/message/62314
Mute This Topic: https://groups.io/mt/75403795/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platforms] [PATCH v3 1/2] Silicon/NXP: Add DWC3 USB controller initialization driver

[Meenakshi Aggarwal]

From: Meenakshi Aggarwal 

Add support of DWC3 controller driver which performs
DWC3 controller initialization and register itself as
NonDiscoverableMmioDevice

Signed-off-by: Meenakshi Aggarwal 
---
 Silicon/NXP/NxpQoriqLs.dec   |   5 +
 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf |  45 
 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h   | 138 ++
 Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c   | 275 
 4 files changed, 463 insertions(+)

diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
index d4d3057af509..72c1744fc934 100644
--- a/Silicon/NXP/NxpQoriqLs.dec
+++ b/Silicon/NXP/NxpQoriqLs.dec
@@ -36,6 +36,11 @@ [PcdsFixedAtBuild.common]
   gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x0|UINT32|0x0502
   gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x0|UINT32|0x0503
 
+  # Pcds for USB
+  gNxpQoriqLsTokenSpaceGuid.PcdUsbBaseAddr|0x0|UINT64|0x0510
+  gNxpQoriqLsTokenSpaceGuid.PcdUsbSize|0x0|UINT32|0x0511
+  gNxpQoriqLsTokenSpaceGuid.PcdNumUsbController|0|UINT32|0x0512
+
 [PcdsDynamic.common]
   gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x0600
   gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl|FALSE|BOOLEAN|0x0601
diff --git a/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf 
b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
new file mode 100644
index ..ea7532b4324b
--- /dev/null
+++ b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
@@ -0,0 +1,45 @@
+#  UsbHcd.inf
+#
+#  Copyright 2017, 2020 NXP
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+
+[Defines]
+  INF_VERSION = 0x0001001A
+  BASE_NAME   = UsbHcdDxe
+  FILE_GUID   = 196e7c2a-37b2-4b85-8683-718588952449
+  MODULE_TYPE = DXE_DRIVER
+  VERSION_STRING  = 1.0
+  ENTRY_POINT = InitializeUsbHcd
+
+[Sources.common]
+  UsbHcd.c
+  UsbHcd.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  Silicon/NXP/NxpQoriqLs.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  IoLib
+  MemoryAllocationLib
+  NonDiscoverableDeviceRegistrationLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+
+[FixedPcd]
+  gNxpQoriqLsTokenSpaceGuid.PcdNumUsbController
+  gNxpQoriqLsTokenSpaceGuid.PcdUsbBaseAddr
+  gNxpQoriqLsTokenSpaceGuid.PcdUsbSize
+
+[Guids]
+  gEfiEndOfDxeEventGroupGuid
+
+[Depex]
+  TRUE
diff --git a/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h 
b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
new file mode 100644
index ..cd9f9ad80125
--- /dev/null
+++ b/Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
@@ -0,0 +1,138 @@
+/** @file
+
+  Copyright 2017, 2020 NXP
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef USB_HCD_H_
+#define USB_HCD_H_
+
+#include 
+
+/* Global constants */
+#define DWC3_GSNPSID_MASK  0x
+#define DWC3_SYNOPSYS_ID   0x5533
+#define DWC3_RELEASE_MASK  0x
+#define DWC3_REG_OFFSET0xC100
+#define DWC3_RELEASE_190a  0x190a
+
+/* Global Configuration Register */
+#define DWC3_GCTL_U2RSTECN BIT16
+#define DWC3_GCTL_PRTCAPDIR(N) ((N) << 12)
+#define DWC3_GCTL_PRTCAP_HOST  1
+#define DWC3_GCTL_PRTCAP_OTG   3
+#define DWC3_GCTL_CORESOFTRESETBIT11
+#define DWC3_GCTL_SCALEDOWN(N) ((N) << 4)
+#define DWC3_GCTL_SCALEDOWN_MASK   DWC3_GCTL_SCALEDOWN(3)
+#define DWC3_GCTL_DISSCRAMBLE  BIT3
+#define DWC3_GCTL_DSBLCLKGTNG  BIT0
+
+/* Global HWPARAMS1 Register */
+#define DWC3_GHWPARAMS1_EN_PWROPT(N)   (((N) & (3 << 24)) >> 24)
+#define DWC3_GHWPARAMS1_EN_PWROPT_CLK  1
+
+/* Global USB2 PHY Configuration Register */
+#define DWC3_GUSB2PHYCFG_PHYSOFTRSTBIT31
+
+/* Global USB3 PIPE Control Register */
+#define DWC3_GUSB3PIPECTL_PHYSOFTRST   BIT31
+
+/* Global Frame Length Adjustment Register */
+#define GFLADJ_30MHZ_REG_SEL   BIT7
+#define GFLADJ_30MHZ(N)((N) & 0x3f)
+#define GFLADJ_30MHZ_DEFAULT   0x20
+
+/* Default to the FSL XHCI defines */
+#define USB3_ENABLE_BEAT_BURST 0xF
+#define USB3_ENABLE_BEAT_BURST_MASK0xFF
+#define USB3_SET_BEAT_BURST_LIMIT  0xF00
+
+typedef struct {
+  UINT32 GEvntAdrLo;
+  UINT32 GEvntAdrHi;
+  UINT32 GEvntSiz;
+  UINT32 GEvntCount;
+} G_EVENT_BUFFER;
+
+typedef struct {
+  UINT32 DDepCmdPar2;
+  UINT32 DDepCmdPar1;
+  UINT32 DDepCmdPar0;
+  UINT32 DDepCmd;
+} D_PHYSICAL_EP;
+
+typedef struct {
+  UINT32 GSBusCfg0;
+  UINT32 GSBusCfg1;
+  UINT32 GTxThrCfg;
+  UINT32 GRxThrCfg;
+  UINT32 GCtl;
+  UINT32 Res1;
+  UINT32 GSts;
+  UINT32 Res2;
+  UINT32 GSnpsId;
+  UINT32 GGpio;
+  UINT32 GUid;
+  UINT32 GUctl;
+  UINT64 GBusErrAddr;
+  UINT64 GPrtbImap;
+  UINT32 GHwParams0;
+ 

Re: [edk2-devel] [Patch 04/15] MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions

[Michael D Kinney]

Liming,

In order to support host based unit tests, all the code
has to be compatible with a user mode process.  This means
any functions that execute instructions that are not allowed
in a user mode process need to be separated out.

For the IA32/X64 specific GccInline.c files, the analysis
was done for APIs that use instructions that would 
generate an exception in a user mode process.

In addition, the BaseLib functions that return system
state (e.g. CS/DS/ES/SS/FS/GS) are also split out.  It
does not make sense to return system state information
from the currently executing user mode process.  Instead,
the FW code under test would expect system state that
matches a FW execution environment.  Each unit test
implementation has the option of providing emulation
of this system state if that is required to exercise the
code under test.

No changes to the MSFT specific BaseLib implementation
were required because there those are already split out
to one function per file.

Best regards,

Mike

> -Original Message-
> From: Gao, Liming 
> Sent: Thursday, July 9, 2020 7:07 AM
> To: Kinney, Michael D ;
> devel@edk2.groups.io
> Cc: Sean Brogan ; Bret
> Barkelew ; Yao, Jiewen
> 
> Subject: RE: [Patch 04/15] MdePkg/BaseLib: Break out
> IA32/X64 GCC inline privileged functions
> 
> Mike:
>   Is there the rule to know which function can't be in
> unit test? And, those functions will be in GccInline.c
> or GccInlinePriv.c? There is no change in MSFT C source
> file, because MSFT C source file has been separated as
> the function level. Right?
> 
> Thanks
> Liming
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Monday, June 15, 2020 8:19 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming ; Sean Brogan
> ; Bret Barkelew
> ;
> > Yao, Jiewen 
> > Subject: [Patch 04/15] MdePkg/BaseLib: Break out
> IA32/X64 GCC inline privileged functions
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2800
> >
> > Break out the IA32/X64 GCC inline functions that can
> not be used
> > in a unit test host application into their own source
> file.  This
> > does not make any changes to the BaseLib library
> instance.  This
> > is in preparation for a new BaseLib instances that is
> safe to use
> > with host-based unit test applications.
> >
> > Cc: Liming Gao 
> > Cc: Sean Brogan 
> > Cc: Bret Barkelew 
> > Cc: Jiewen Yao 
> > Signed-off-by: Michael D Kinney
> 
> > ---
> >  MdePkg/Library/BaseLib/BaseLib.inf|4
> +-
> >  MdePkg/Library/BaseLib/Ia32/GccInline.c   | 1181
> +---
> >  .../Ia32/{GccInline.c => GccInlinePriv.c} |  601
> +---
> >  MdePkg/Library/BaseLib/X64/GccInline.c| 1240
> +
> >  .../X64/{GccInline.c => GccInlinePriv.c}  |  572
> +---
> >  5 files changed, 11 insertions(+), 3587 deletions(-)
> >  copy MdePkg/Library/BaseLib/Ia32/{GccInline.c =>
> GccInlinePriv.c} (62%)
> >  copy MdePkg/Library/BaseLib/X64/{GccInline.c =>
> GccInlinePriv.c} (65%)
> >
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> > index a57ae2da31..c740a819ca 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -1,7 +1,7 @@
> >  ## @file
> >  #  Base Library implementation.
> >  #
> > -#  Copyright (c) 2007 - 2019, Intel Corporation. All
> rights reserved.
> > +#  Copyright (c) 2007 - 2020, Intel Corporation. All
> rights reserved.
> >  #  Portions copyright (c) 2008 - 2009, Apple Inc. All
> rights reserved.
> >  #  Portions copyright (c) 2011 - 2013, ARM Ltd. All
> rights reserved.
> >  #  Copyright (c) 2020, Hewlett Packard Enterprise
> Development LP. All rights reserved.
> > @@ -156,6 +156,7 @@ [Sources.Ia32]
> >
> >
> >Ia32/GccInline.c | GCC
> > +  Ia32/GccInlinePriv.c | GCC
> >Ia32/Thunk16.nasm
> >Ia32/EnableDisableInterrupts.nasm| GCC
> >Ia32/EnablePaging64.nasm
> > @@ -310,6 +311,7 @@ [Sources.X64]
> >X86PatchInstruction.c
> >X86SpeculationBarrier.c
> >X64/GccInline.c | GCC
> > +  X64/GccInlinePriv.c | GCC
> >X64/EnableDisableInterrupts.nasm
> >X64/DisablePaging64.nasm
> >X64/RdRand.nasm
> > diff --git a/MdePkg/Library/BaseLib/Ia32/GccInline.c
> b/MdePkg/Library/BaseLib/Ia32/GccInline.c
> > index 5287200f87..6ed938187a 100644
> > --- a/MdePkg/Library/BaseLib/Ia32/GccInline.c
> > +++ b/MdePkg/Library/BaseLib/Ia32/GccInline.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >GCC inline implementation of BaseLib processor
> specific functions.
> >
> > -  Copyright (c) 2006 - 2018, Intel Corporation. All
> rights reserved.
> > +  Copyright (c) 2006 - 2020, Intel Corporation. All
> rights reserved.
> >Portions copyright (c) 2008 - 2009, Apple Inc. All
> rights reserved.
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -10,8 +10,6 @@
> >
> >  #include "BaseLibInternals.h"
> >
> > -
> > -
> >  /**
> >Used to serialize load and store operations.
> >
> > @@ -31,41 +29,6 @@ MemoryFence (
> >__asm__ __vola

Re: [edk2-devel] [Patch v2 05/16] MdePkg/Library/BaseLib: Add BaseLib instance for host based unit tests

[Liming Gao]

Mike:
  New library instance library class should be UnitTestHostBaseLib instead of 
BaseLib. 

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, July 9, 2020 12:05 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Sean Brogan 
> ; Bret Barkelew ;
> Yao, Jiewen 
> Subject: [Patch v2 05/16] MdePkg/Library/BaseLib: Add BaseLib instance for 
> host based unit tests
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2800
> 
> Add a new version of BaseLib that is safe for use from host based
> unit test applications.  Host based unit test applications may need
> to provide implementations of some BaseLib functions that provide
> simple emulation to exercise the code under test.  The structure
> UNIT_TEST_HOST_BASE_LIB is filled in with services that provide
> default emulation for BaseLib APIs that would normally generate
> exceptions in a host based unit test application.  This structure
> allows an individual unit test to replace the default emulation of
> a BaseLib service with an alternate version that is required by a
> specific unit test.
> 
> Normally cmocka would be used to mock services the code under
> test calls.  However, the BaseLib is used by the Unit Test
> Framework itself, so using a mocked interface is not possible.
> The use of a structure to provide hooks for unit test is not
> expected to be a common feature.  It should only be required
> for libraries that are used by both the Unit Test Framework and
> the code under test where the code under test requires a
> different behavior than the Unit Test Framework.
> 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> Signed-off-by: Michael D Kinney 
> ---
>  MdePkg/Library/BaseLib/UnitTestHost.c |  140 +
>  MdePkg/Library/BaseLib/UnitTestHost.h |   66 +
>  .../Library/BaseLib/UnitTestHostBaseLib.inf   |  216 ++
>  .../Library/BaseLib/UnitTestHostBaseLib.uni   |   11 +
>  MdePkg/Library/BaseLib/X86UnitTestHost.c  | 2977 +
>  MdePkg/MdePkg.dec |3 +-
>  MdePkg/Test/MdePkgHostTest.dsc|5 +
>  .../Include/HostTest/UnitTestHostBaseLib.h|  582 
>  8 files changed, 3999 insertions(+), 1 deletion(-)
>  create mode 100644 MdePkg/Library/BaseLib/UnitTestHost.c
>  create mode 100644 MdePkg/Library/BaseLib/UnitTestHost.h
>  create mode 100644 MdePkg/Library/BaseLib/UnitTestHostBaseLib.inf
>  create mode 100644 MdePkg/Library/BaseLib/UnitTestHostBaseLib.uni
>  create mode 100644 MdePkg/Library/BaseLib/X86UnitTestHost.c
>  create mode 100644 
> MdePkg/Test/UnitTest/Include/HostTest/UnitTestHostBaseLib.h
> 
> diff --git a/MdePkg/Library/BaseLib/UnitTestHost.c 
> b/MdePkg/Library/BaseLib/UnitTestHost.c
> new file mode 100644
> index 00..79eec7caca
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/UnitTestHost.c
> @@ -0,0 +1,140 @@
> +/** @file
> +  Common Unit Test Host functions.
> +
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "UnitTestHost.h"
> +
> +///
> +/// Module global variable for simple system emulation of interrupt state
> +///
> +STATIC BOOLEAN  mUnitTestHostBaseLibInterruptState;
> +
> +/**
> +  Enables CPU interrupts.
> +
> +**/
> +VOID
> +EFIAPI
> +UnitTestHostBaseLibEnableInterrupts (
> +  VOID
> +  )
> +{
> +  mUnitTestHostBaseLibInterruptState = TRUE;
> +}
> +
> +/**
> +  Disables CPU interrupts.
> +
> +**/
> +VOID
> +EFIAPI
> +UnitTestHostBaseLibDisableInterrupts (
> +  VOID
> +  )
> +{
> +  mUnitTestHostBaseLibInterruptState = FALSE;
> +}
> +
> +/**
> +  Enables CPU interrupts for the smallest window required to capture any
> +  pending interrupts.
> +
> +**/
> +VOID
> +EFIAPI
> +UnitTestHostBaseLibEnableDisableInterrupts (
> +  VOID
> +  )
> +{
> +  mUnitTestHostBaseLibInterruptState = FALSE;
> +}
> +
> +/**
> +  Set the current CPU interrupt state.
> +
> +  Sets the current CPU interrupt state to the state specified by
> +  InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
> +  InterruptState is FALSE, then interrupts are disabled. InterruptState is
> +  returned.
> +
> +  @param  InterruptState  TRUE if interrupts should enabled. FALSE if
> +  interrupts should be disabled.
> +
> +  @return InterruptState
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +UnitTestHostBaseLibGetInterruptState (
> +  VOID
> +  )
> +{
> +  return mUnitTestHostBaseLibInterruptState;
> +}
> +
> +/**
> +  Enables CPU interrupts.
> +
> +**/
> +VOID
> +EFIAPI
> +EnableInterrupts (
> +  VOID
> +  )
> +{
> +  gUnitTestHostBaseLib.Common->EnableInterrupts ();
> +}
> +
> +/**
> +  Disables CPU interrupts.
> +
> +**/
> +VOID
> +EFIAPI
> +DisableInterrupts (
> +  VOID
> +  )
> +{
> +  gUnitTestHostBaseLib.Common->DisableInterrupts ();
> +}
> +
> +/**
> +  Enables CPU interrupts for the smallest window required to capture any
> +  pending interrupts.
> +
> +**/
> +VOI

Re: [edk2-devel] [Patch 04/15] MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions

[Liming Gao]

Mike:
  Is there the rule to know which function can't be in unit test? And, those 
functions will be in GccInline.c or GccInlinePriv.c? There is no change in MSFT 
C source file, because MSFT C source file has been separated as the function 
level. Right?

Thanks
Liming
> -Original Message-
> From: Kinney, Michael D 
> Sent: Monday, June 15, 2020 8:19 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Sean Brogan 
> ; Bret Barkelew ;
> Yao, Jiewen 
> Subject: [Patch 04/15] MdePkg/BaseLib: Break out IA32/X64 GCC inline 
> privileged functions
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2800
> 
> Break out the IA32/X64 GCC inline functions that can not be used
> in a unit test host application into their own source file.  This
> does not make any changes to the BaseLib library instance.  This
> is in preparation for a new BaseLib instances that is safe to use
> with host-based unit test applications.
> 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> Signed-off-by: Michael D Kinney 
> ---
>  MdePkg/Library/BaseLib/BaseLib.inf|4 +-
>  MdePkg/Library/BaseLib/Ia32/GccInline.c   | 1181 +---
>  .../Ia32/{GccInline.c => GccInlinePriv.c} |  601 +---
>  MdePkg/Library/BaseLib/X64/GccInline.c| 1240 +
>  .../X64/{GccInline.c => GccInlinePriv.c}  |  572 +---
>  5 files changed, 11 insertions(+), 3587 deletions(-)
>  copy MdePkg/Library/BaseLib/Ia32/{GccInline.c => GccInlinePriv.c} (62%)
>  copy MdePkg/Library/BaseLib/X64/{GccInline.c => GccInlinePriv.c} (65%)
> 
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
> b/MdePkg/Library/BaseLib/BaseLib.inf
> index a57ae2da31..c740a819ca 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Base Library implementation.
>  #
> -#  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
>  #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>  #  Portions copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
>  #  Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
> reserved.
> @@ -156,6 +156,7 @@ [Sources.Ia32]
> 
> 
>Ia32/GccInline.c | GCC
> +  Ia32/GccInlinePriv.c | GCC
>Ia32/Thunk16.nasm
>Ia32/EnableDisableInterrupts.nasm| GCC
>Ia32/EnablePaging64.nasm
> @@ -310,6 +311,7 @@ [Sources.X64]
>X86PatchInstruction.c
>X86SpeculationBarrier.c
>X64/GccInline.c | GCC
> +  X64/GccInlinePriv.c | GCC
>X64/EnableDisableInterrupts.nasm
>X64/DisablePaging64.nasm
>X64/RdRand.nasm
> diff --git a/MdePkg/Library/BaseLib/Ia32/GccInline.c 
> b/MdePkg/Library/BaseLib/Ia32/GccInline.c
> index 5287200f87..6ed938187a 100644
> --- a/MdePkg/Library/BaseLib/Ia32/GccInline.c
> +++ b/MdePkg/Library/BaseLib/Ia32/GccInline.c
> @@ -1,7 +1,7 @@
>  /** @file
>GCC inline implementation of BaseLib processor specific functions.
> 
> -  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> +  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
>Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -10,8 +10,6 @@
> 
>  #include "BaseLibInternals.h"
> 
> -
> -
>  /**
>Used to serialize load and store operations.
> 
> @@ -31,41 +29,6 @@ MemoryFence (
>__asm__ __volatile__ ("":::"memory");
>  }
> 
> -
> -/**
> -  Enables CPU interrupts.
> -
> -  Enables CPU interrupts.
> -
> -**/
> -VOID
> -EFIAPI
> -EnableInterrupts (
> -  VOID
> -  )
> -{
> -  __asm__ __volatile__ ("sti"::: "memory");
> -}
> -
> -
> -/**
> -  Disables CPU interrupts.
> -
> -  Disables CPU interrupts.
> -
> -**/
> -VOID
> -EFIAPI
> -DisableInterrupts (
> -  VOID
> -  )
> -{
> -  __asm__ __volatile__ ("cli"::: "memory");
> -}
> -
> -
> -
> -
>  /**
>Requests CPU to pause for a short period of time.
> 
> @@ -82,7 +45,6 @@ CpuPause (
>__asm__ __volatile__ ("pause");
>  }
> 
> -
>  /**
>Generates a breakpoint on the CPU.
> 
> @@ -99,75 +61,6 @@ CpuBreakpoint (
>__asm__ __volatile__ ("int $3");
>  }
> 
> -
> -
> -/**
> -  Returns a 64-bit Machine Specific Register(MSR).
> -
> -  Reads and returns the 64-bit MSR specified by Index. No parameter checking 
> is
> -  performed on Index, and some Index values may cause CPU exceptions. The
> -  caller must either guarantee that Index is valid, or the caller must set up
> -  exception handlers to catch the exceptions. This function is only available
> -  on IA-32 and X64.
> -
> -  @param  Index The 32-bit MSR index to read.
> -
> -  @return The value of the MSR identified by Index.
> -
> -**/
> -UINT64
> -EFIAPI
> -AsmReadMsr64 (
> -  IN  UINT32Index
> -  )
> -{
> -  UINT64 Data;
> -
> -  __asm__ __volatile__ (
> -"rdmsr"
> -: "=A" (Data)   // %0
> -: "c"  (Index)  // %1
> -);
> -
> -

Re: [edk2-devel] [Patch 02/15] MdePkg/BaseCpuLibNull: Add Null version of CpuLib for host testing

[Liming Gao]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Kinney, Michael D 
> Sent: Monday, June 15, 2020 8:19 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Sean Brogan 
> ; Bret Barkelew ;
> Yao, Jiewen 
> Subject: [Patch 02/15] MdePkg/BaseCpuLibNull: Add Null version of CpuLib for 
> host testing
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2798
> 
> The services in CpuLib usually generate exceptions in a unit test
> host application.  Provide a Null instance that can be safely used.
> 
> This Null instance can also be used as a template for implementing
> new instances of CpuLib.
> 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> Signed-off-by: Michael D Kinney 
> ---
>  .../Library/BaseCpuLibNull/BaseCpuLibNull.c   | 37 +++
>  .../Library/BaseCpuLibNull/BaseCpuLibNull.inf | 26 +
>  .../Library/BaseCpuLibNull/BaseCpuLibNull.uni | 11 ++
>  MdePkg/MdePkg.dsc |  3 +-
>  4 files changed, 76 insertions(+), 1 deletion(-)
>  create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
>  create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
>  create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
> 
> diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c 
> b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
> new file mode 100644
> index 00..3ba7a35096
> --- /dev/null
> +++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
> @@ -0,0 +1,37 @@
> +/** @file
> +  Null instance of CPU Library.
> +
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +/**
> +  Places the CPU in a sleep state until an interrupt is received.
> +
> +  Places the CPU in a sleep state until an interrupt is received. If 
> interrupts
> +  are disabled prior to calling this function, then the CPU will be placed 
> in a
> +  sleep state indefinitely.
> +
> +**/
> +VOID
> +EFIAPI
> +CpuSleep (
> +  VOID
> +  )
> +{
> +}
> +
> +/**
> +  Flushes all the Translation Lookaside Buffers(TLB) entries in a CPU.
> +
> +  Flushes all the Translation Lookaside Buffers(TLB) entries in a CPU.
> +
> +**/
> +VOID
> +EFIAPI
> +CpuFlushTlb (
> +  VOID
> +  )
> +{
> +}
> diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf 
> b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
> new file mode 100644
> index 00..a9e8399038
> --- /dev/null
> +++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
> @@ -0,0 +1,26 @@
> +## @file
> +#  Null instance of CPU Library.
> +#
> +#  Copyright (c) 2020, Intel Corporation. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION= 0x00010005
> +  BASE_NAME  = BaseCpuLibNull
> +  MODULE_UNI_FILE= BaseCpuLibNull.uni
> +  FILE_GUID  = 8A29AAA5-0FB7-44CC-8709-1344FE89B878
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = CpuLib
> +
> +#
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC ARM AARCH64 RISCV64
> +#
> +
> +[Sources]
> +  BaseCpuLibNull.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni 
> b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
> new file mode 100644
> index 00..1030221d5c
> --- /dev/null
> +++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
> @@ -0,0 +1,11 @@
> +// /** @file
> +// Null instance of CPU Library.
> +//
> +// Copyright (c) 2020, Intel Corporation. All rights reserved.
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +// **/
> +
> +#string STR_MODULE_ABSTRACT #language en-US "Null Instance of 
> CPU Library"
> +
> +#string STR_MODULE_DESCRIPTION  #language en-US "Null instance of 
> CPU Library."
> diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
> index 6cd38e7ec3..3abe65ec7f 100644
> --- a/MdePkg/MdePkg.dsc
> +++ b/MdePkg/MdePkg.dsc
> @@ -1,7 +1,7 @@
>  ## @file
>  # EFI/PI MdePkg Package
>  #
> -# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>  # (C) Copyright 2020 Hewlett Packard Enterprise Development LP
>  #
> @@ -36,6 +36,7 @@ [Components]
>MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
>MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
>MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
> +  MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
>MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
>MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> --
> 2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group

Re: [edk2-devel] [Patch 03/15] MdePkg/BaseCacheMaintenanceLibNull: Add Null instance for host testing

[Liming Gao]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Kinney, Michael D 
> Sent: Monday, June 15, 2020 8:19 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Sean Brogan 
> ; Bret Barkelew ;
> Yao, Jiewen 
> Subject: [Patch 03/15] MdePkg/BaseCacheMaintenanceLibNull: Add Null instance 
> for host testing
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2799
> 
> The services in CacheMaintenanceLib usually generate exceptions in a
> unit test host application.  Provide a Null instance that can be safely
> used.
> 
> This Null instance can also be used as a template for implementing
> new instances of CacheMaintenanceLib.
> 
> Cc: Liming Gao 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Jiewen Yao 
> Signed-off-by: Michael D Kinney 
> ---
>  .../BaseCacheMaintenanceLibNull.c | 225 ++
>  .../BaseCacheMaintenanceLibNull.inf   |  29 +++
>  .../BaseCacheMaintenanceLibNull.uni   |  12 +
>  MdePkg/MdePkg.dsc |   1 +
>  4 files changed, 267 insertions(+)
>  create mode 100644 
> MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
>  create mode 100644 
> MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.inf
>  create mode 100644 
> MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.uni
> 
> diff --git 
> a/MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
> b/MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
> new file mode 100644
> index 00..fd5b9d4710
> --- /dev/null
> +++ b/MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
> @@ -0,0 +1,225 @@
> +/** @file
> +  Null Cache Maintenance Librfary implementation.
> +
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +#include 
> +
> +/**
> +  Invalidates the entire instruction cache in cache coherency domain of the
> +  calling CPU.
> +
> +**/
> +VOID
> +EFIAPI
> +InvalidateInstructionCache (
> +  VOID
> +  )
> +{
> +}
> +
> +/**
> +  Invalidates a range of instruction cache lines in the cache coherency 
> domain
> +  of the calling CPU.
> +
> +  Invalidates the instruction cache lines specified by Address and Length. If
> +  Address is not aligned on a cache line boundary, then entire instruction
> +  cache line containing Address is invalidated. If Address + Length is not
> +  aligned on a cache line boundary, then the entire instruction cache line
> +  containing Address + Length -1 is invalidated. This function may choose to
> +  invalidate the entire instruction cache if that is more efficient than
> +  invalidating the specified range. If Length is 0, then no instruction cache
> +  lines are invalidated. Address is returned.
> +
> +  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
> +
> +  @param  Address The base address of the instruction cache lines to
> +  invalidate. If the CPU is in a physical addressing mode, 
> then
> +  Address is a physical address. If the CPU is in a virtual
> +  addressing mode, then Address is a virtual address.
> +
> +  @param  Length  The number of bytes to invalidate from the instruction 
> cache.
> +
> +  @return Address.
> +
> +**/
> +VOID *
> +EFIAPI
> +InvalidateInstructionCacheRange (
> +  IN  VOID  *Address,
> +  IN  UINTN Length
> +  )
> +{
> +  ASSERT (Length <= MAX_ADDRESS - (UINTN)Address + 1);
> +  return Address;
> +}
> +
> +/**
> +  Writes back and invalidates the entire data cache in cache coherency domain
> +  of the calling CPU.
> +
> +  Writes back and invalidates the entire data cache in cache coherency domain
> +  of the calling CPU. This function guarantees that all dirty cache lines are
> +  written back to system memory, and also invalidates all the data cache 
> lines
> +  in the cache coherency domain of the calling CPU.
> +
> +**/
> +VOID
> +EFIAPI
> +WriteBackInvalidateDataCache (
> +  VOID
> +  )
> +{
> +}
> +
> +/**
> +  Writes back and invalidates a range of data cache lines in the cache
> +  coherency domain of the calling CPU.
> +
> +  Writes Back and Invalidate the data cache lines specified by Address and
> +  Length. If Address is not aligned on a cache line boundary, then entire 
> data
> +  cache line containing Address is written back and invalidated. If Address +
> +  Length is not aligned on a cache line boundary, then the entire data cache
> +  line containing Address + Length -1 is written back and invalidated. This
> +  function may choose to write back and invalidate the entire data cache if
> +  that is more efficient than writing back and invalidating the specified
> +  range. If Length is 0, then no data cache lines are written back and
> +  invalidated. Address is returned.
> +
> +  If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
> +

Re: [edk2-devel] [Patch v2 15/16] MdePkg/Library/BaseStackCheckLib: Fix PCD type in INF

[Liming Gao]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Kinney, Michael D 
> Sent: Thursday, July 9, 2020 12:05 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming 
> Subject: [Patch v2 15/16] MdePkg/Library/BaseStackCheckLib: Fix PCD type in 
> INF
> 
> Update INF file to use a [Pcd] section instead of a
> [FixedPcd] section.  [FixedPcd] should only be used in an
> INF file if the source code looks up the PCD value using
> the PcdLib FixedPcdGetxx() services.  Using [FixedPcd]
> forces a platform to configure the PCD to type FixedAtBuild.
> In this case, PcdDebugPropertyMask supports PCD types
> FixedAtBuild and PatchableInModule.  Without this change
> any platform that wants to use PcdDebugPropertyMask as
> type PatchableInModule breaks the build.
> 
> Cc: Liming Gao 
> Signed-off-by: Michael D Kinney 
> ---
>  MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf 
> b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> index 4ae3bd1a82..0dc3c4a83a 100644
> --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> @@ -36,5 +36,5 @@ [LibraryClasses]
>BaseLib
>DebugLib
> 
> -[FixedPcd]
> +[Pcd]
>gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask  ## CONSUMES
> --
> 2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62306): https://edk2.groups.io/g/devel/message/62306
Mute This Topic: https://groups.io/mt/75391622/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms] [PATCH] IntelSiliconPkg: IOMMU generic bug fix

[Liming Gao]

Lorena:
  I have one minor comment on this patch. The error return status should be 
EFI_UNSUPPORTED.

+  return error;
==>
return EFI_UNSUPPORTED;

Thanks
Liming
From: De Leon Vazquez, Lorena R 
Sent: Tuesday, July 7, 2020 1:24 AM
To: Gao, Liming ; devel@edk2.groups.io; Lohr, Paul A 
; Yao, Jiewen 
Cc: Kinney, Michael D 
Subject: RE: [edk2-devel] [edk2-platforms] [PATCH] IntelSiliconPkg: IOMMU 
generic bug fix

Hi Liming,
I've attached the patch

Thanks,
Lorena

From: Gao, Liming mailto:liming@intel.com>>
Sent: Thursday, July 2, 2020 8:54 PM
To: devel@edk2.groups.io; Lohr, Paul A 
mailto:paul.a.l...@intel.com>>; Yao, Jiewen 
mailto:jiewen@intel.com>>; De Leon Vazquez, Lorena R 
mailto:lorena.r.de.leon.vazq...@intel.com>>
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: [edk2-devel] [edk2-platforms] [PATCH] IntelSiliconPkg: IOMMU 
generic bug fix

Paul:
  This patch is missing to be merged.

Lorena:
  I can't extract the patch from the mail. Can you send the patch to me? I can 
help merge it.

Thanks
Liming
From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Lohr, Paul A
Sent: Thursday, July 2, 2020 9:56 PM
To: devel@edk2.groups.io; Yao, Jiewen 
mailto:jiewen@intel.com>>; De Leon Vazquez, Lorena R 
mailto:lorena.r.de.leon.vazq...@intel.com>>
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH] IntelSiliconPkg: IOMMU 
generic bug fix

Hello,

It seems this did not get checked in.  Is there something wrong with the patch 
itself?  Or was this simply submitted incorrectly?  I don't see a Bugzilla 
associated with it is why I ask.

Paul A. Lohr - Server Firmware Enabling
512.239.9073 (cell)
512.794.5044 (work)

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Yao, Jiewen
Sent: Monday, March 2, 2020 5:46 PM
To: De Leon Vazquez, Lorena R 
mailto:lorena.r.de.leon.vazq...@intel.com>>;
 devel@edk2.groups.io
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH] IntelSiliconPkg: IOMMU 
generic bug fix

Reviewed-by: jiewen@intel.com

From: De Leon Vazquez, Lorena R 
mailto:lorena.r.de.leon.vazq...@intel.com>>
Sent: Tuesday, March 3, 2020 7:04 AM
To: devel@edk2.groups.io
Cc: Yao, Jiewen mailto:jiewen@intel.com>>; Kinney, 
Michael D mailto:michael.d.kin...@intel.com>>
Subject: [edk2-platforms] [PATCH] IntelSiliconPkg: IOMMU generic bug fix

Looks like Addresswidth is BIT wise values. Right now these values are not used 
any

Suggested-by: Star Zeng star.z...@intel.com
Signed-off-by: 
lorena.r.de.leon.vazq...@intel.com

--
.../Feature/VTd/IntelVTdDxe/TranslationTable.c| 11 ---
.../Feature/VTd/IntelVTdDxe/TranslationTableEx.c  | 11 ---
2 files changed, 8 insertions(+), 14 deletions(-)

diff --git 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
index cc970c0..61fbb4a 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
@@ -128,14 +128,11 @@ CreateContextEntry (

 DEBUG ((DEBUG_INFO,"Source: S%04x B%02x D%02x F%02x\n", 
mVtdUnitInformation[VtdIndex].Segment, SourceId.Bits.Bus, SourceId.Bits.Device, 
SourceId.Bits.Function));

-switch (mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW) {
-case BIT1:
-  ContextEntry->Bits.AddressWidth = 0x1;
-  break;
-case BIT2:
-  ContextEntry->Bits.AddressWidth = 0x2;
-  break;
+if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) == 0) {
+  DEBUG((DEBUG_ERROR, " 4-level page-table is not supported on VTD %d 
\n", VtdIndex));
+  return error;
 }
+ContextEntry->Bits.AddressWidth = 0x2;
   }

   FlushPageTableMemory (VtdIndex, 
(UINTN)mVtdUnitInformation[VtdIndex].RootEntryTable, 
EFI_PAGES_TO_SIZE(EntryTablePages));
diff --git 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c
index 0da1611..6bd31b7 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.c
@@ -78,14 +78,11 @@ CreateExtContextEntry (

 DEBUG ((DEBUG_INFO,"DOMAIN: S%04x, B%02x D%02x F%02x\n", 
mVtdUnitInformation[VtdIndex].Segment, SourceId.Bits.Bus, SourceId.Bits.Device, 
SourceId.Bits.Function));

-switch (mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW) {
-case BIT1:
-  ExtContextEntry->Bits.AddressWidth = 0x1;
-  break;

Re: [edk2-devel] [Patch v2 01/16] BaseTools/Python: Allow HOST_APPLICATION to use NULL libraries

[Bob Feng]

Reviewed-by: Bob Feng  

-Original Message-
From: Kinney, Michael D  
Sent: Thursday, July 9, 2020 12:05 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Gao, Liming ; 
Sean Brogan ; Bret Barkelew 
; Yao, Jiewen 
Subject: [Patch v2 01/16] BaseTools/Python: Allow HOST_APPLICATION to use NULL 
libraries

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

Update HOST_APPLICATION module type to use NULL library instances.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Jiewen Yao 
Signed-off-by: Michael D Kinney 
---
 BaseTools/Source/Python/Workspace/WorkspaceCommon.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py 
b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
index 913e710fd9..53027a0e30 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
@@ -1,7 +1,7 @@
 ## @file
 # Common routines used by workspace
 #
-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2012 - 2020, Intel Corporation. All rights 
+reserved.
 # SPDX-License-Identifier: BSD-2-Clause-Patent  #
 
@@ -100,7 +100,7 @@ def GetModuleLibInstances(Module, Platform, BuildDatabase, 
Arch, Target, Toolcha
 # If a module has a MODULE_TYPE of USER_DEFINED,
 # do not link in NULL library class instances from the global 
[LibraryClasses.*] sections.
 #
-if Module.ModuleType != SUP_MODULE_USER_DEFINED and Module.ModuleType != 
SUP_MODULE_HOST_APPLICATION:
+if Module.ModuleType != SUP_MODULE_USER_DEFINED:
 for LibraryClass in Platform.LibraryClasses.GetKeys():
 if LibraryClass.startswith("NULL") and 
Platform.LibraryClasses[LibraryClass, Module.ModuleType]:
 Module.LibraryClasses[LibraryClass] = 
Platform.LibraryClasses[LibraryClass, Module.ModuleType]
--
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62304): https://edk2.groups.io/g/devel/message/62304
Mute This Topic: https://groups.io/mt/75391608/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Cancelled Event: TianoCore Design Meeting - APAC/NAMO - Friday, 10 July 2020 #cal-cancelled

[devel@edk2.groups.io Calendar]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:g7lq.1578029159272351097.u...@groups.io
DTSTAMP:20200709T114247Z
ORGANIZER;CN=Ray Ni:mailto:ray...@intel.com
DTSTART:20200710T013000Z
DTEND:20200710T023000Z
SUMMARY:TianoCore Design Meeting - APAC/NAMO
DESCRIPTION:For more info\, see here \nhttps //www.tianocore.org/design-m
 eeting/\n\n## Join Zoom Meeting\n\nhttps //zoom.us/j/299494771\n\n \n\nMe
 eting ID  299 494 771\n\n \n\nOne tap mobile\n\n+16699009128\,\,299494771
 # US (San Jose)\n\n+13462487799\,\,299494771# US (Houston)\n\n \n\nDial b
 y your location\n\nâ+1 669 900 9128 US (San Jose)\n\nâ+1 346 24
 8 7799 US (Houston)\n\nâ+1 301 715 8592 US\n\nâ+1 312 626 6799 
 US (Chicago)\n\nâ+1 646 558 8656 US (New York)\n\nâ+1 253 215 8
 782 US\n\nMeeting ID  299 494 771\n\nFind your local number  https //zoom
 .us/u/ajd9Bs4kZ
LOCATION:https://zoom.us/j/299494771
SEQUENCE:999
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

Re: [edk2-devel] [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver

[Sami Mujawar]

Hi Ard,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

-Original Message-
From: Ard Biesheuvel  
Sent: 07 July 2020 02:17 PM
To: Sami Mujawar ; devel@edk2.groups.io
Cc: l...@nuviainc.com; ray...@intel.com; Alexandru Elisei 
; Andre Przywara ; Matteo 
Carlini ; Laura Moretta ; nd 

Subject: Re: [PATCH v4 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Some virtual machine managers like Kvmtool emulate the MC146818 RTC 
> controller in the MMIO space so that architectures that do not support 
> I/O Mapped I/O can use the RTC. This patch adds MMIO support to the 
> RTC controller driver.
> 
> The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
>If PcdRtcUseMmio is:
>  TRUE  - Indicates the RTC port registers are in MMIO space.
>  FALSE - Indicates the RTC port registers are in I/O space.
>  Default is I/O space.
> 
> Additionally two new PCDs PcdRtcIndexRegister64 and
> PcdRtcTargetRegister64 have been introduced to provide the base 
> address for the RTC registers in the MMIO space.
> 
> When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver 
> converts the pointers to the RTC MMIO registers so that the RTC 
> registers are accessible post ExitBootServices.
> 
> Signed-off-by: Sami Mujawar 

This code looks good to me now, but please drop the unnecessary whitespace and 
header changes to PcRtc.h, and the whitespace changes to PcRtcEntry.c

[SAMI] I will fix this and repost the patch.

With that,

Reviewed-by: Ard Biesheuvel 


> ---
> 
> Notes:
>  v4:
>- Updated based on review comments. [Sami]
>- Use static helper functions instead of function pointers. [Ard]
>  Ref: https://edk2.groups.io/g/devel/topic/75081468
>  
>  v3:
>- Make PcdRtcUseMmio a feature PCD. [Sami]
>- Read the RTC MMIO base address from the DT.   [Andre]
>- Introduce PCDs for RTC Index and Target register base [Sami]
>  address in the MMIO space.
>- Move RTC MMIO region mapping code to a separate platform  [Sami]
>  specific library. This library also reads the base addresses
>  for the RTC from DT and configures the RTC Index and Target
>  register PCDs.
>  Ref: https://edk2.groups.io/g/devel/topic/74200905#60307
>  
>  v2:
>- Code review comments incorporated.[Sami]
>  
>  v1:
>- Add support to read/write from RTC registers using[Sami]
>  MMIO access
>- Use wrapper functions for RtcRead/Write accessors [Leif]
>  Ref: https://edk2.groups.io/g/devel/topic/30915281#30695
> 
>   PcAtChipsetPkg/PcAtChipsetPkg.dec  
> |  16 +++
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c 
> | 120 ++--
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h 
> |   2 +
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> |  58 +-
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf 
> |   8 ++
>   5 files changed, 190 insertions(+), 14 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec 
> b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> index 
> 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc290c
> f3bb905e211b 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> @@ -6,6 +6,7 @@
>   #
>   # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
>   # Copyright (c) 2017, AMD Inc. All rights reserved.
> +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
>   #
>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>   #
> @@ -41,6 +42,13 @@ [PcdsFeatureFlag]
> # @Prompt Configure HPET to use MSI.
> 
> gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x1000
>   
> +  ## Indicates the RTC port registers are in MMIO space, or in I/O space.
> +  #  Default is I/O space.
> +  #   TRUE  - RTC port registers are in MMIO space.
> +  #   FALSE - RTC port registers are in I/O space.
> +  # @Prompt RTC port registers use MMIO.
> +  
> + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x0021
> +
>   [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, PcdsPatchableInModule]
> ## This PCD specifies the base address of the HPET timer.
> # @Prompt HPET base address.
> @@ -68,6 +76,14 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, 
> PcdsPatchableInModule]
> # @Expression 0x8001 | 
> gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < 
> gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100
> 
> gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x00
> 0E
>   
> +  ## Specifies RTC Index Register address in MMIO space.
> +  # @Pr

Re: [edk2-devel] [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library

[Sami Mujawar]

Hi Ard,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Ard Biesheuvel 
via groups.io
Sent: 07 July 2020 02:20 PM
To: Sami Mujawar ; devel@edk2.groups.io
Cc: l...@nuviainc.com; ler...@redhat.com; Alexandru Elisei 
; Andre Przywara ; Matteo 
Carlini ; Laura Moretta ; nd 

Subject: Re: [edk2-devel] [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt 
Client Library

On 7/7/20 3:47 PM, Sami Mujawar wrote:
> Add library that parses the Kvmtool device tree and updates the 
> dynamic PCDs describing the RTC Memory map.
> 
> It also maps the MMIO region used by the RTC as runtime memory so that 
> the RTC registers are accessible post ExitBootServices.
> 
> Since UEFI takes ownership of the RTC hardware disable the RTC node in 
> the DT to prevent the OS from attaching its device driver as well.
> 
> Signed-off-by: Sami Mujawar 
> ---
> 
> Notes:
>  v4:
>- Updated based on review comments. [Sami]
>- Cleanup include file list, make local functions static,   [Ard]
>  drop EFIAPI, use EFI_PAGE_MASK and use ReadUnaligned64.
>  Ref: https://edk2.groups.io/g/devel/topic/75081472
>  

You haven't answered my question about adding and removing the GCD memory space.
[SAMI] The call to gDS->AddMemorySpace () was failing on my experimental branch 
to enable PCI support for the kvmtool firmware.
With the current kvmtool implementation, when PCI is enabled, some part of the 
PCI IO region is overlapping with the region used by the RTC controller.
Since this patch series is not adding PCI support for kvmtool firmware, this 
issue cannot be seen.
[/SAMI]

>  v3:
>- Introduce library to read and map the MMIO base addresses [Sami]
>  for the RTC registers from the DT and configure the Index
>  and Target register PCDs.
> 
>   ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c   | 230 
> 
>   ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf |  42 
> 
>   2 files changed, 272 insertions(+)
> 
> diff --git 
> a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c 
> b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
> new file mode 100644
> index 
> ..93715958952f933007ff143a22cbe81606cc539e
> --- /dev/null
> +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c
> @@ -0,0 +1,230 @@
> +/** @file
> +  FDT client library for motorola,mc146818 RTC driver
> +
> +  Copyright (c) 2020, ARM Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/** RTC Index register is at offset 0x0
> +*/
> +#define RTC_INDEX_REG_OFFSET0x0ULL
> +
> +/** RTC Target register is at offset 0x1
> +*/
> +#define RTC_TARGET_REG_OFFSET   0x1ULL
> +
> +/** Add the RTC controller address range to the memory map.
> +
> +  @param [in]  ImageHandle  The handle to the image.
> +  @param [in]  RtcPageBase  Base address of the RTC controller.
> +
> +  @retval EFI_SUCCESS Success.
> +  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
> +  @retval EFI_NOT_FOUND   Flash device not found.
> +**/
> +STATIC
> +EFI_STATUS
> +KvmtoolRtcMapMemory (
> +  IN EFI_HANDLE   ImageHandle,
> +  IN EFI_PHYSICAL_ADDRESS RtcPageBase
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = gDS->AddMemorySpace (
> +  EfiGcdMemoryTypeMemoryMappedIo,
> +  RtcPageBase,
> +  EFI_PAGE_SIZE,
> +  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> +  );
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((
> +  DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
> +  Status
> +  ));
> +return Status;
> +  }
> +
> +  Status = gDS->AllocateMemorySpace (
> +  EfiGcdAllocateAddress,
> +  EfiGcdMemoryTypeMemoryMappedIo,
> +  0,
> +  EFI_PAGE_SIZE,
> +  &RtcPageBase,
> +  ImageHandle,
> +  NULL
> +  );
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((
> +  DEBUG_ERROR,
> +  "Failed to allocate memory space. Status = %r\n",
> +  Status
> +  ));
> +gDS->RemoveMemorySpace (
> +   RtcPageBase,
> +   EFI_PAGE_SIZE
> +   );
> +return Status;
> +  }
> +
> +  Status = gDS->SetMemorySpaceAttributes (
> +  RtcPageBase,
> +  EFI_PAGE_SIZE,
> +  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> +  );
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((
> +  DEBUG_ERROR,
> +  "Failed to set memory attributes. Status = %r\n",
> +  Status
> +  ));
> +gDS->FreeMemorySpace (
> +   RtcPageBase,

Re: [edk2-devel] [PATCH v5 3/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)

[Laszlo Ersek]

On 07/09/20 03:56, Guomin Jiang wrote:
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
> 
> Moves the GDT and IDT to permanent memory in a memory discovered
> callback. This is done to ensure the GDT and IDT authenticated in
> pre-memory is not fetched from outside a verified location after
> the permanent memory transition.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Signed-off-by: Michael Kubacki 
> Reviewed-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/CpuMpPei/CpuMpPei.inf |  1 +
>  UefiCpuPkg/CpuMpPei/CpuMpPei.h   | 12 +++
>  UefiCpuPkg/CpuMpPei/CpuMpPei.c   | 37 
>  UefiCpuPkg/CpuMpPei/CpuPaging.c  | 12 +--
>  4 files changed, 60 insertions(+), 2 deletions(-)

sorry, a question -- the subject line and the commit message reference
the IDT, not just the GDT. But the code handles the GDT only. Is this
intentional? Should the code handle the IDT, or should we remove the IDT
mentions from the subject and the commit message?

Thanks
Laszlo

> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf 
> b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> index caead3ce34d4..f4d11b861f77 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> @@ -63,6 +63,7 @@ [Pcd]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList  ## 
> SOMETIMES_CONSUMES
>gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize## 
> SOMETIMES_CONSUMES
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize   ## 
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes  ## 
> CONSUMES
>  
>  [Depex]
>TRUE
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> index 7d5c527d6006..309478cbe14c 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> @@ -397,6 +397,18 @@ SecPlatformInformation2 (
>   OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
>);
>  
> +/**
> +  Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> +  @retval   EFI_SUCCESS   The GDT was migrated successfully.
> +  @retval   EFI_OUT_OF_RESOURCES  The GDT could not be migrated due to lack 
> of available memory.
> +
> +**/
> +EFI_STATUS
> +MigrateGdt (
> +  VOID
> +  );
> +
>  /**
>Initializes MP and exceptions handlers.
>  
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> index 07ccbe7c6a91..d07540cf7471 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> @@ -429,6 +429,43 @@ GetGdtr (
>AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
>  }
>  
> +/**
> +  Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> +  @retval   EFI_SUCCESS   The GDT was migrated successfully.
> +  @retval   EFI_OUT_OF_RESOURCES  The GDT could not be migrated due to lack 
> of available memory.
> +
> +**/
> +EFI_STATUS
> +MigrateGdt (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINTN   GdtBufferSize;
> +  IA32_DESCRIPTOR Gdtr;
> +  VOID*GdtBuffer;
> +
> +  AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
> +  GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) -1 + Gdtr.Limit + 1;
> +
> +  Status =  PeiServicesAllocatePool (
> +  GdtBufferSize,
> +  &GdtBuffer
> +  );
> +  ASSERT (GdtBuffer != NULL);
> +  if (EFI_ERROR (Status)) {
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_SEGMENT_DESCRIPTOR));
> +  CopyMem (GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
> +  Gdtr.Base = (UINTN) GdtBuffer;
> +  AsmWriteGdtr (&Gdtr);
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>Initializes CPU exceptions handlers for the sake of stack switch 
> requirement.
>  
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index a462e7ee1e38..3bf0574b34c6 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -602,8 +602,16 @@ MemoryDiscoveredPpiNotifyCallback (
>IN VOID   *Ppi
>)
>  {
> -  EFI_STATUS  Status;
> -  BOOLEAN InitStackGuard;
> +  EFI_STATUS  Status;
> +  BOOLEAN InitStackGuard;
> +  BOOLEAN InterruptState;
> +
> +  if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
> +InterruptState = SaveAndDisableInterrupts ();
> +Status = MigrateGdt ();
> +ASSERT_EFI_ERROR (Status);
> +SetInterruptState (InterruptState);
> +  }
>  
>//
>// Paging must be setup first. Otherwise the exception TSS setup during MP
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62300): https://edk2.groups.io/g/devel/message/62300
Mute This Topic: https://groups.io/mt/75390175/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive

Re: [edk2-devel] [PATCH v5 4/9] UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098)

[Laszlo Ersek]

On 07/09/20 03:56, Guomin Jiang wrote:
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
> 
> Adds a PEIM that republishes structures produced in SEC. This
> is done because SEC modules may not be shadowed in some platforms
> due to space constraints or special alignment requirements. The
> SecMigrationPei module locates interfaces that may be published in
> SEC and reinstalls the interface with permanent memory addresses.
> 
> This is important if pre-memory address access is forbidden after
> memory initialization and data such as a PPI descriptor, PPI GUID,
> or PPI inteface reside in pre-memory.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Debkumar De 
> Cc: Harry Han 
> Cc: Catharine West 
> Signed-off-by: Michael Kubacki 
> Acked-by: Laszlo Ersek 
> ---
>  UefiCpuPkg/UefiCpuPkg.dec |   4 +
>  UefiCpuPkg/UefiCpuPkg.dsc |   1 +
>  UefiCpuPkg/SecCore/SecCore.inf|   2 +
>  .../SecMigrationPei/SecMigrationPei.inf   |  67 +++
>  UefiCpuPkg/Include/Ppi/RepublishSecPpi.h  |  54 +++
>  UefiCpuPkg/SecCore/SecMain.h  |   1 +
>  UefiCpuPkg/SecMigrationPei/SecMigrationPei.h  | 154 +++
>  UefiCpuPkg/SecCore/SecMain.c  |  26 +-
>  UefiCpuPkg/SecMigrationPei/SecMigrationPei.c  | 381 ++
>  .../SecMigrationPei/SecMigrationPei.uni   |  13 +
>  10 files changed, 701 insertions(+), 2 deletions(-)
>  create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
>  create mode 100644 UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
>  create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
>  create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
>  create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni

> +/**
> +  This function is the entry point which installs an instance of 
> REPUBLISH_SEC_PPI_PPI.
> +
> +  It install the RepublishSecPpi depent on 
> PcdMigrateTemporaryRamFirmwareVolumes, install
> +  the PPI when the PcdMigrateTemporaryRamFirmwareVolumes enabled.
> +
> +  @param[in]  FileHandle   Pointer to image file handle.
> +  @param[in]  PeiServices  Pointer to PEI Services Table
> +
> +  @retval EFI_ABORTED  Disable evacuate temporary memory feature by disable
> +   PcdMigrateTemporaryRamFirmwareVolumes.
> +  EFI_SUCCESS  An instance of REPUBLISH_SEC_PPI_PPI was installed 
> successfully.

I think "@retval" should be used in front of every single return value,
that is, EFI_SUCCESS as well.

But, please do not resubmit the series just because of this remark; it's
not important enough.

If you need to post a v6 for another reason, then please consider adding
the "@retval" word here.

My Acked-by stands with or without "@retval".

Thanks!
Laszlo

> +  @retval Others   An error occurred installing and instance of 
> REPUBLISH_SEC_PPI_PPI.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecMigrationPeiInitialize (
> +  IN EFI_PEI_FILE_HANDLE FileHandle,
> +  IN CONST EFI_PEI_SERVICES  **PeiServices
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = EFI_ABORTED;
> +
> +  if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
> +Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  return Status;
> +}



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62299): https://edk2.groups.io/g/devel/message/62299
Mute This Topic: https://groups.io/mt/75390177/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 8/9] UefiCpuPkg: Correct some typos.

[Laszlo Ersek]

On 07/09/20 03:56, Guomin Jiang wrote:
> Correct some typos.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Signed-off-by: Guomin Jiang 
> ---
>  UefiCpuPkg/CpuMpPei/CpuMpPei.h| 2 +-
>  .../Library/CpuExceptionHandlerLib/CpuExceptionCommon.h   | 4 ++--
>  UefiCpuPkg/CpuMpPei/CpuPaging.c   | 4 ++--
>  .../CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c| 4 ++--
>  .../Library/CpuExceptionHandlerLib/SecPeiCpuException.c   | 2 +-
>  .../Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 4 ++--
>  6 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> index 309478cbe14c..6a481a84dcc7 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> @@ -424,7 +424,7 @@ InitializeCpuMpWorker (
>);
>  
>  /**
> -  Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to 
> TRUE.
> +  Enable/setup stack guard for each processor if PcdCpuStackGuard is set to 
> TRUE.
>  
>Doing this in the memory-discovered callback is to make sure the Stack 
> Guard
>feature to cover as most PEI code as possible.
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> index 805dd9cbb4ff..0544d6dba631 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> @@ -90,8 +90,8 @@ AsmGetTemplateAddressMap (
>  **/
>  VOID
>  ArchUpdateIdtEntry (
> -  IN IA32_IDT_GATE_DESCRIPTOR*IdtEntry,
> -  IN UINTN   InterruptHandler
> +  OUT IA32_IDT_GATE_DESCRIPTOR   *IdtEntry,
> +  IN  UINTN  InterruptHandler
>);
>  
>  /**
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 04a16fb2b620..891d1ef50cac 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -153,7 +153,7 @@ GetPhysicalAddressWidth (
>Get the type of top level page table.
>  
>@retval Page512G  PML4 paging.
> -  @retval Page1GPAE paing.
> +  @retval Page1GPAE paging.
>  
>  **/
>  PAGE_ATTRIBUTE
> @@ -583,7 +583,7 @@ SetupStackGuardPage (
>  }
>  
>  /**
> -  Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to 
> TRUE.
> +  Enable/setup stack guard for each processor if PcdCpuStackGuard is set to 
> TRUE.
>  
>Doing this in the memory-discovered callback is to make sure the Stack 
> Guard
>feature to cover as most PEI code as possible.
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> index 1aafb7dac139..903449e0daa9 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> @@ -18,8 +18,8 @@
>  **/
>  VOID
>  ArchUpdateIdtEntry (
> -  IN IA32_IDT_GATE_DESCRIPTOR*IdtEntry,
> -  IN UINTN   InterruptHandler
> +  OUT IA32_IDT_GATE_DESCRIPTOR*IdtEntry,
> +  IN  UINTN   InterruptHandler
>)
>  {
>IdtEntry->Bits.OffsetLow   = (UINT16)(UINTN)InterruptHandler;
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> index 20148db74cf8..d4ae153c5742 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
>IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof 
> (IA32_IDT_GATE_DESCRIPTOR);
>if (IdtEntryCount > CPU_EXCEPTION_NUM) {
>  //
> -// CPU exeption library only setup CPU_EXCEPTION_NUM exception handler 
> at most
> +// CPU exception library only setup CPU_EXCEPTION_NUM exception handler 
> at most
>  //
>  IdtEntryCount = CPU_EXCEPTION_NUM;
>}
> diff --git 
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c 
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> index 894c1cfb7533..d3da16e4dfa2 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> @@ -17,8 +17,8 @@
>  **/
>  VOID
>  ArchUpdateIdtEntry (
> -  IN IA32_IDT_GATE_DESCRIPTOR*IdtEntry,
> -  IN UINTN   InterruptHandler
> +  OUT IA32_IDT_GATE_DESCRIPTOR   *IdtEntry,
> +  IN  UINTN  InterruptHandler
>)
>  {
>IdtEntry->Bits.OffsetLow   = (UINT16)(UINTN)InterruptHandler;
> 

Reviewed-by: Laszlo Ersek 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages s

Re: [edk2-devel] [PATCH v5 2/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098)

[Laszlo Ersek]

On 07/09/20 03:56, Guomin Jiang wrote:
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
> 
> Introduces new changes to PeiCore to move the contents of temporary
> RAM visible to the PeiCore to permanent memory. This expands on
> pre-existing shadowing support in the PeiCore to perform the following
> additional actions:
> 
>  1. Migrate pointers in PPIs installed in PeiCore to the permanent
> memory copy of PeiCore.
> 
>  2. Copy all installed firmware volumes to permanent memory.
> 
>  3. Relocate and fix up the PEIMs within the firmware volumes.
> 
>  4. Convert all PPIs into the migrated firmware volume to the corresponding
> PPI address in the permanent memory location.
> 
> This applies to PPIs and PEI notifications.
> 
>  5. Convert all status code callbacks in the migrated firmware volume to
> the corresponding address in the permanent memory location.
> 
>  6. Update the FV HOB to the corresponding firmware volume in permanent
> memory.
> 
>  7. Add PcdMigrateTemporaryRamFirmwareVolumes to control if enable the
> feature or not. when the PCD disable, the EvacuateTempRam() will
> never be called.
> 
> The function control flow as below:
>   PeiCore()
> DumpPpiList()
> EvacuateTempRam()
>   ConvertPeiCorePpiPointers()
> ConvertPpiPointersFv()
>   MigratePeimsInFv()
> MigratePeim()
>   PeiGetPe32Data()
>   LoadAndRelocatePeCoffImageInPlace()
>   MigrateSecModulesInFv()
>   ConvertPpiPointersFv()
>   ConvertStatusCodeCallbacks()
>   ConvertFvHob()
>   RemoveFvHobsInTemporaryMemory()
> DumpPpiList()
> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Dandan Bi 
> Cc: Liming Gao 
> Cc: Debkumar De 
> Cc: Harry Han 
> Cc: Catharine West 
> Signed-off-by: Michael Kubacki 
> ---
>  MdeModulePkg/Core/Pei/PeiMain.inf |   2 +
>  MdeModulePkg/Core/Pei/PeiMain.h   | 168 
>  MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 402 ++
>  MdeModulePkg/Core/Pei/Image/Image.c   | 115 +
>  MdeModulePkg/Core/Pei/Memory/MemoryServices.c |  82 
>  MdeModulePkg/Core/Pei/PeiMain/PeiMain.c   |  24 ++
>  MdeModulePkg/Core/Pei/Ppi/Ppi.c   | 287 +
>  7 files changed, 1080 insertions(+)
> 

Acked-by: Laszlo Ersek 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62297): https://edk2.groups.io/g/devel/message/62297
Mute This Topic: https://groups.io/mt/75390174/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision

[Laszlo Ersek]

On 07/09/20 04:46, Gao, Zhichao wrote:
> From: Terry Lee 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2697
> 
> Tcg2PhysicalPresenceLibConstructor set the module variable
> mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version comparision.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Chao Zhang 
> Signed-off-by: Zhichao Gao 
> ---
>  .../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c 
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
> index 1c46d5e69d..8afaa0a785 100644
> --- 
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
> +++ 
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
> @@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor (
>  {
>EFI_STATUS  Status;
>  
> -  if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8 
> *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer), sizeof(PP_INF_VERSION_1_2) - 
> 1) <= 0) {
> +  if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8 
> *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer), sizeof(PP_INF_VERSION_1_2) - 
> 1) >= 0) {
>  mIsTcg2PPVerLowerThan_1_3 = TRUE;
>}
>  
> 

What is the practical impact of this bug / fix?

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62296): https://edk2.groups.io/g/devel/message/62296
Mute This Topic: https://groups.io/mt/75390754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] "Error 4000" EDK II Build Issue

[Zachary Rinehart]

Hi all,

  I am trying to build the EDK II firmware for the MinnowBoard Turbot using 
Windows, as outlined 
here.
 When I try to execute the final command to build the firmware, it gives me an 
error saying that an "instance of library class [FmpDependencyLib] is not 
found". I believe I have installed all the prerequisites correctly, and I ran 
it a second time to check for previous error messages, none of which I could 
find. It does seem to build partway, though. I have attached a zip file of what 
I believe is the partial build and a text capture of the second attempt via 
Windows shell to build EDK II. Does anyone have any insight into what's going 
on?

Thanks,
  Zach

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62295): https://edk2.groups.io/g/devel/message/62295
Mute This Topic: https://groups.io/mt/75394040/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>