Re: [edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area

[Sughosh Ganu]

hi Achin,

On Sat Dec 01, 2018 at 05:08:50AM +0530, Achin Gupta wrote:
> Hi Sughosh,
> 
> +Jiewen
> 
> I took the patches for a spin and it looks like the FVP port is broken. Some
> reasons are:
> 
> 1. The build breaks due to a reference to ArmMmuLib in StandaloneMmPkg.dsc
> 2. There is a broken dependency on PL011UartClockLib in StandaloneMmPkg.dsc
> 3. GCC flags to enforce strict alignment and no fp are required in
>StandaloneMmPkg.dsc to avoid a runtime fault
> 4. There is a data structre in StandaloneMmCoreEntryPoint.c that needs to be
>memzeroed due to the alignment checks
> 
> Even after these fixes, I am unable to boot the MM SP. The SP boots with the
> previous revision of your patches and the above fixes. Something has broken
> between the two. I am suspecting the MMU library for S-EL0.

I had tested the patches which i had sent out for ArmPkg changes with
the error handling and error reporting feature on sgi575 before
posting the patches. In addition to the changes that you mention, i
was required to make a couple of more changes in the StandadloneMm
description file.

1) 
StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf

This was changed from ArmMmuLib which was used earlier.

2) 
PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf

I had changed this to reflect the change made in the patch
StandaloneMM: Update permissions for Standalone MM drivers memory area

Can you please confirm that you tested with these two additional
changes made. Meanwhile, I will incorporate your review comments which
you make below. Thanks.

-sughosh

> 
> Lets sort this out first. Apart from this, could you move this library into
> an AArch64 directory as is the case for other Arm specific libraries
> e.g. StandaloneMmCoreEntryPoint/AArch64
> 
> cheers,
> Achin
> 
> On Tue, Nov 27, 2018 at 11:52:53AM +0530, Sughosh Ganu wrote:
> > Changes since v1:
> > A new patch has been added to reflect the library class added for
> > changing the MMU attributes in StandaloneMM image, based on review
> > comments from Ard Biesheuvel.
> > 
> > 
> > These patches needs to be applied on top of the following patch series
> >  - "ArmPkg related changes for StandaloneMM package".
> > 
> > 
> > Sughosh Ganu (2):
> >   StandaloneMM: Include the newly added library class for MMU functions
> >   StandaloneMM: Update permissions for Standalone MM drivers memory area
> > 
> >  .../StandaloneMmCoreEntryPoint.inf |   2 +-
> >  .../StandaloneMmPeCoffExtraActionLib.inf   |  18 +-
> >  .../StandaloneMmPeCoffExtraActionLib.c | 222 
> > +
> >  3 files changed, 234 insertions(+), 8 deletions(-)
> >  copy ArmPkg/Library/RvdPeCoffExtraActionLib/RvdPeCoffExtraActionLib.inf => 
> > StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
> >  (72%)
> >  create mode 100644 
> > StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.c
> > 
> > -- 
> > 2.7.4
> > 

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

Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

[Chris Co via edk2-devel]

Hi Leif,

> -Original Message-
> From: Leif Lindholm 
> Sent: Thursday, November 1, 2018 11:20 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Michael D Kinney 
> Subject: Re: [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-
> specific i.MX packages to use
> 
> On Fri, Sep 21, 2018 at 08:26:00AM +, Chris Co wrote:
> > This adds common headers for other NXP i.MX SoC packages.
> > More specifically, this adds i.MX-generic GPIO, IoMux, and Platform
> > definitions.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Christopher Co 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Michael D Kinney 
> > ---
> >  Silicon/NXP/iMXPlatformPkg/Include/Platform.h | 67 ++
> > Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h  | 92
> >   Silicon/NXP/iMXPlatformPkg/Include/iMXIoMux.h |
> > 24 +
> >  3 files changed, 183 insertions(+)
> >
> > diff --git a/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> > b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> > new file mode 100644
> > index ..8a1e828f68ea
> > --- /dev/null
> > +++ b/Silicon/NXP/iMXPlatformPkg/Include/Platform.h
> > @@ -0,0 +1,67 @@
> > +/** @file
> > +*
> > +*  i.MX Platform specific defines for constructing ACPI tables
> > +*
> > +*  Copyright (c) 2018 Microsoft Corporation. All rights reserved.
> > +*
> > +*  This program and the accompanying materials
> > +*  are licensed and made available under the terms and conditions of
> > +the BSD License
> > +*  which accompanies this distribution.  The full text of the license
> > +may be found at
> > +*
> > +https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopens
> > +ource.org%2Flicenses%2Fbsd-
> license.phpdata=02%7C01%7CChristopher
> >
> +.Co%40microsoft.com%7C53adf4afe364416fab0c08d64026b170%7C72f988bf8
> 6f1
> >
> +41af91ab2d7cd011db47%7C1%7C0%7C636766932265064004sdata=lEf
> %2Bq2l
> > +nuxJ3cw%2BL91rhCTJ2e3jzWZfvgZZY8cyHswY%3Dreserved=0
> > +*
> > +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > +BASIS,
> > +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#ifndef _PLATFORM_IMX_H_
> > +#define _PLATFORM_IMX_H_
> > +
> > +#include 
> > +
> > +#define EFI_ACPI_OEM_ID   {'M','C','R','S','F','T'} // OEMID 6 
> > bytes
> > +#define EFI_ACPI_VENDOR_IDSIGNATURE_32('N','X','P','I')
> > +#define EFI_ACPI_CSRT_REVISION0x0005
> > +#define EFI_ACPI_5_0_CSRT_REVISION0x
> > +
> > +// Resource Descriptor Types
> > +#define EFI_ACPI_CSRT_RD_TYPE_INTERRUPT 1 #define
> > +EFI_ACPI_CSRT_RD_TYPE_TIMER 2 #define EFI_ACPI_CSRT_RD_TYPE_DMA 3
> > +#define EFI_ACPI_CSRT_RD_TYPE_CACHE 4
> > +
> > +// Resource Descriptor Subtypes
> > +#define EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_LINES 0 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_INTERRUPT_CONTROLLER 1 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_TIMER 0 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CHANNEL 0 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_DMA_CONTROLLER 1 #define
> > +EFI_ACPI_CSRT_RD_SUBTYPE_CACHE 0
> 
> If using EFI_ACPI prefix, these #defines really should be in edk2 MdePkg. And
> CSRT itself is, so that might not be a bad idea.
> 
> > +
> > +#pragma pack(push, 1)
> 
> I don't see this #pragma making any difference to the structs below, can it be
> dropped?
> 

The pragma pack is defensive. Without it, we rely on the compiler packing 
structures by default and this may not happen on 64 bit compiles.

I have addressed the remaining feedback and will resubmit with v2.

Thanks,
Chris

> > +//---
> > +- // CSRT Resource Group header 24 bytes long
> > +//---
> > +-
> > +typedef struct {
> > +  UINT32 Length;
> > +  UINT32 VendorID;
> > +  UINT32 SubVendorId;
> > +  UINT16 DeviceId;
> > +  UINT16 SubdeviceId;
> > +  UINT16 Revision;
> > +  UINT16 Reserved;
> > +  UINT32 SharedInfoLength;
> > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > +
> > +//---
> > +- // CSRT Resource Descriptor 12 bytes total
> > +//---
> > +-
> > +typedef struct {
> > +  UINT32 Length;
> > +  UINT16 ResourceType;
> > +  UINT16 ResourceSubType;
> > +  UINT32 UID;
> > +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> > +#pragma pack (pop)
> > +
> > +#endif // !_PLATFORM_IMX_H_
> > diff --git a/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> > b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> > new file mode 100644
> > index ..dce01f789058
> > --- /dev/null
> > +++ b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> > @@ -0,0 +1,92 @@
> > +/** @file
> > +*
> > +*  Copyright (c) 2018 Microsoft Corporation. All rights reserved.
> > +*
> > +*  This program and the accompanying materials

Re: [edk2] [PATCH v2 0/2] StandaloneMM: Update permissions for Standalone MM drivers memory area

[Achin Gupta]

Hi Sughosh,

+Jiewen

I took the patches for a spin and it looks like the FVP port is broken. Some
reasons are:

1. The build breaks due to a reference to ArmMmuLib in StandaloneMmPkg.dsc
2. There is a broken dependency on PL011UartClockLib in StandaloneMmPkg.dsc
3. GCC flags to enforce strict alignment and no fp are required in
   StandaloneMmPkg.dsc to avoid a runtime fault
4. There is a data structre in StandaloneMmCoreEntryPoint.c that needs to be
   memzeroed due to the alignment checks

Even after these fixes, I am unable to boot the MM SP. The SP boots with the
previous revision of your patches and the above fixes. Something has broken
between the two. I am suspecting the MMU library for S-EL0.

Lets sort this out first. Apart from this, could you move this library into
an AArch64 directory as is the case for other Arm specific libraries
e.g. StandaloneMmCoreEntryPoint/AArch64

cheers,
Achin

On Tue, Nov 27, 2018 at 11:52:53AM +0530, Sughosh Ganu wrote:
> Changes since v1:
> A new patch has been added to reflect the library class added for
> changing the MMU attributes in StandaloneMM image, based on review
> comments from Ard Biesheuvel.
> 
> 
> These patches needs to be applied on top of the following patch series
>  - "ArmPkg related changes for StandaloneMM package".
> 
> 
> Sughosh Ganu (2):
>   StandaloneMM: Include the newly added library class for MMU functions
>   StandaloneMM: Update permissions for Standalone MM drivers memory area
> 
>  .../StandaloneMmCoreEntryPoint.inf |   2 +-
>  .../StandaloneMmPeCoffExtraActionLib.inf   |  18 +-
>  .../StandaloneMmPeCoffExtraActionLib.c | 222 
> +
>  3 files changed, 234 insertions(+), 8 deletions(-)
>  copy ArmPkg/Library/RvdPeCoffExtraActionLib/RvdPeCoffExtraActionLib.inf => 
> StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
>  (72%)
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.c
> 
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

[Ard Biesheuvel]

The maximum value that can be represented by the native word size
of the *target* should be irrelevant when compiling tools that
run on the build *host*. So drop the definition of MAX_UINTN, now
that we no longer use it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jaben Carsey 
---
 BaseTools/Source/C/Common/CommonLib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.h 
b/BaseTools/Source/C/Common/CommonLib.h
index 6930d9227b87..b1c6c00a3478 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #define MAX_LONG_FILE_PATH 500
 
-#define MAX_UINTN MAX_ADDRESS
 #define MAX_UINT64 ((UINT64)0xULL)
 #define MAX_UINT16  ((UINT16)0x)
 #define MAX_UINT8   ((UINT8)0xFF)
-- 
2.19.1

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

[edk2] [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines

[Ard Biesheuvel]

Parsing a string into an integer variable of the native word size
is not defined for the BaseTools, since the same tools may be used
to build firmware for different targets with different native word
sizes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/Common/CommonLib.h |  24 ---
 BaseTools/Source/C/Common/CommonLib.c | 174 +---
 2 files changed, 5 insertions(+), 193 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.h 
b/BaseTools/Source/C/Common/CommonLib.h
index fa10fac4682a..6930d9227b87 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -250,16 +250,6 @@ StrSize (
   CONST CHAR16  *String
   );
 
-UINTN
-StrHexToUintn (
-  CONST CHAR16  *String
-  );
-
-UINTN
-StrDecimalToUintn (
-  CONST CHAR16  *String
-  );
-
 UINT64
 StrHexToUint64 (
   CONST CHAR16 *String
@@ -277,13 +267,6 @@ StrHexToUint64S (
 UINT64 *Data
   );
 
-RETURN_STATUS
-StrHexToUintnS (
-CONST CHAR16 *String,
- CHAR16 **EndPointer,  OPTIONAL
- UINTN  *Data
-  );
-
 RETURN_STATUS
 StrDecimalToUint64S (
 CONST CHAR16 *String,
@@ -291,13 +274,6 @@ StrDecimalToUint64S (
  UINT64 *Data
   );
 
-RETURN_STATUS
-StrDecimalToUintnS (
-CONST CHAR16 *String,
- CHAR16 **EndPointer,  OPTIONAL
- UINTN  *Data
-  );
-
 VOID *
 ReallocatePool (
UINTN  OldSize,
diff --git a/BaseTools/Source/C/Common/CommonLib.c 
b/BaseTools/Source/C/Common/CommonLib.c
index 4a28f635f3a8..42dfa821624d 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
   return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, 
Size2 * sizeof(CHAR16));
 }
 
-RETURN_STATUS
-StrDecimalToUintnS (
-CONST CHAR16 *String,
- CHAR16 **EndPointer,  OPTIONAL
- UINTN  *Data
-  )
-{
-  ASSERT (((UINTN) String & BIT0) == 0);
-
-  //
-  // 1. Neither String nor Data shall be a null pointer.
-  //
-  SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
-  SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
-
-  //
-  // 2. The length of String shall not be greater than RSIZE_MAX.
-  //
-  if (RSIZE_MAX != 0) {
-SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= 
RSIZE_MAX), RETURN_INVALID_PARAMETER);
-  }
-
-  if (EndPointer != NULL) {
-*EndPointer = (CHAR16 *) String;
-  }
-
-  //
-  // Ignore the pad spaces (space or tab)
-  //
-  while ((*String == L' ') || (*String == L'\t')) {
-String++;
-  }
-
-  //
-  // Ignore leading Zeros after the spaces
-  //
-  while (*String == L'0') {
-String++;
-  }
-
-  *Data = 0;
-
-  while (InternalIsDecimalDigitCharacter (*String)) {
-//
-// If the number represented by String overflows according to the range
-// defined by UINTN, then MAX_UINTN is stored in *Data and
-// RETURN_UNSUPPORTED is returned.
-//
-if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
-  *Data = MAX_UINTN;
-  if (EndPointer != NULL) {
-*EndPointer = (CHAR16 *) String;
-  }
-  return RETURN_UNSUPPORTED;
-}
-
-*Data = *Data * 10 + (*String - L'0');
-String++;
-  }
-
-  if (EndPointer != NULL) {
-*EndPointer = (CHAR16 *) String;
-  }
-  return RETURN_SUCCESS;
-}
-
 /**
   Convert a Null-terminated Unicode decimal string to a value of type UINT64.
 
@@ -1064,9 +998,9 @@ StrDecimalToUint64S (
 
 /**
   Convert a Null-terminated Unicode hexadecimal string to a value of type
-  UINTN.
+  UINT64.
 
-  This function outputs a value of type UINTN by interpreting the contents of
+  This function outputs a value of type UINT64 by interpreting the contents of
   the Unicode string specified by String as a hexadecimal number. The format of
   the input Unicode string String is:
 
@@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
-  If the number represented by String exceeds the range defined by UINTN, then
-  MAX_UINTN is stored at the location pointed to by Data.
+  If the number represented by String exceeds the range defined by UINT64, then
+  MAX_UINT64 is stored at the location pointed to by Data.
 
   If EndPointer is not NULL, a pointer to the character that stopped the scan
   is stored at the location pointed to by EndPointer. If String has no valid
@@ -1112,86 +1046,10 @@ StrDecimalToUint64S (
characters, not including the
Null-terminator.
   @retval RETURN_UNSUPPORTED   If the number represented by String exceeds
- 

[edk2] [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

[Ard Biesheuvel]

Replace invocations of StrHexToUintn() with StrHexToUint64(), so
that we can drop the former.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jaben Carsey 
---
 BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c 
b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
index 555efa1acdde..6151926af9aa 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
@@ -520,7 +520,7 @@ EisaIdFromText (
   return (((Text[0] - 'A' + 1) & 0x1f) << 10)
+ (((Text[1] - 'A' + 1) & 0x1f) <<  5)
+ (((Text[2] - 'A' + 1) & 0x1f) <<  0)
-   + (UINT32) (StrHexToUintn ([3]) << 16)
+   + (UINT32) (StrHexToUint64 ([3]) << 16)
;
 }
 
@@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
 
   Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
   while (Index-- != 0) {
-Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (, L'-'));
+Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (, L'-'));
   }
 
   return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
-- 
2.19.1

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

[edk2] [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

[Ard Biesheuvel]

Don't use the native word size string to number parsing routines,
but instead, use the 64-bit one and cast to UINTN.

Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
which takes care to use Strtoi64 () unless it assumes the value fits
in 32-bit, so this change is a no-op even on 32-bit build hosts.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jaben Carsey 
---
 BaseTools/Source/C/Common/CommonLib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c 
b/BaseTools/Source/C/Common/CommonLib.c
index 7c559bc3c222..4a28f635f3a8 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -2252,9 +2252,9 @@ Strtoi (
   )
 {
   if (IsHexStr (Str)) {
-return StrHexToUintn (Str);
+return (UINTN)StrHexToUint64 (Str);
   } else {
-return StrDecimalToUintn (Str);
+return (UINTN)StrDecimalToUint64 (Str);
   }
 }
 
-- 
2.19.1

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

[edk2] [PATCH v2 0/6] BaseTools: get rid of MAX_UINTN

[Ard Biesheuvel]

There should be no reason for the build tools to care about the native
word size of a particular target, so relying on a definition of MAX_UINTN
is definitely wrong, and most likely inaccurate on 32-bit build hosts.

So refactor the code in CommonLib and DevicePath so we no longer rely
on this definition.

Changes since v1:
- miss type change in #1 causing a build failure on MSVC
- add acks from Jaben

Cc: Laszlo Ersek 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Jaben Carsey 

Ard Biesheuvel (6):
  BaseTools/CommonLib: avoid using 'native' word size in IP address
handling
  BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
  BaseTools/DevicePath: use explicit 64-bit number parsing routines
  BaseTools/DevicePath: use MAX_UINT16 as default device path max size
  BaseTools/CommonLib: get rid of 'native' type string parsing routines
  BaseTools/CommonLib: drop definition of MAX_UINTN

 BaseTools/Source/C/Common/CommonLib.h |  25 ---
 BaseTools/Source/C/Common/CommonLib.c | 206 ++
 .../Source/C/DevicePath/DevicePathFromText.c  |   4 +-
 .../Source/C/DevicePath/DevicePathUtilities.c |   4 +-
 4 files changed, 25 insertions(+), 214 deletions(-)

-- 
2.19.1

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

[edk2] [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Ard Biesheuvel]

In the context of the BaseTools, there is no such thing as a native word
size, given that the same set of tools may be used to build a firmware
image consisting of both 32-bit and 64-bit modules.

So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
types instead of UINTN types when parsing strings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/Common/CommonLib.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c 
b/BaseTools/Source/C/Common/CommonLib.c
index 618aadac781a..7c559bc3c222 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -1785,7 +1785,7 @@ StrToIpv4Address (
 {
   RETURN_STATUS  Status;
   UINTN  AddressIndex;
-  UINTN  Uintn;
+  UINT64 Uint64;
   EFI_IPv4_ADDRESS   LocalAddress;
   UINT8  LocalPrefixLength;
   CHAR16 *Pointer;
@@ -1812,7 +1812,7 @@ StrToIpv4Address (
 //
 // Get D or P.
 //
-Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, , );
+Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, , );
 if (RETURN_ERROR (Status)) {
   return RETURN_UNSUPPORTED;
 }
@@ -1820,18 +1820,18 @@ StrToIpv4Address (
   //
   // It's P.
   //
-  if (Uintn > 32) {
+  if (Uint64 > 32) {
 return RETURN_UNSUPPORTED;
   }
-  LocalPrefixLength = (UINT8) Uintn;
+  LocalPrefixLength = (UINT8) Uint64;
 } else {
   //
   // It's D.
   //
-  if (Uintn > MAX_UINT8) {
+  if (Uint64 > MAX_UINT8) {
 return RETURN_UNSUPPORTED;
   }
-  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
+  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
   AddressIndex++;
 }
 
@@ -1888,7 +1888,7 @@ StrToIpv6Address (
 {
   RETURN_STATUS  Status;
   UINTN  AddressIndex;
-  UINTN  Uintn;
+  UINT64 Uint64;
   EFI_IPv6_ADDRESS   LocalAddress;
   UINT8  LocalPrefixLength;
   CONST CHAR16   *Pointer;
@@ -1969,7 +1969,7 @@ StrToIpv6Address (
 //
 // Get X.
 //
-Status = StrHexToUintnS (Pointer, , );
+Status = StrHexToUint64S (Pointer, , );
 if (RETURN_ERROR (Status) || End - Pointer > 4) {
   //
   // Number of hexadecimal digit characters is no more than 4.
@@ -1978,24 +1978,24 @@ StrToIpv6Address (
 }
 Pointer = End;
 //
-// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
characters is no more than 4.
+// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
characters is no more than 4.
 //
 ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
-LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
-LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
+LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
+LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
 AddressIndex += 2;
   } else {
 //
 // Get P, then exit the loop.
 //
-Status = StrDecimalToUintnS (Pointer, , );
-if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
+Status = StrDecimalToUint64S (Pointer, , );
+if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
   //
   // Prefix length should not exceed 128.
   //
   return RETURN_UNSUPPORTED;
 }
-LocalPrefixLength = (UINT8) Uintn;
+LocalPrefixLength = (UINT8) Uint64;
 Pointer = End;
 break;
   }
-- 
2.19.1

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

[edk2] [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size

[Ard Biesheuvel]

Replace the default size limit of IsDevicePathValid() with a value
that does not depend on the native word size of the build host.

64 KB seems sufficient as the upper bound of a device path handled
by UEFI.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jaben Carsey 
---
 BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c 
b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
index d4ec2742b7c8..ba7f83e53070 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
@@ -62,7 +62,7 @@ IsDevicePathValid (
   ASSERT (DevicePath != NULL);
 
   if (MaxSize == 0) {
-MaxSize = MAX_UINTN;
+MaxSize = MAX_UINT16;
  }
 
   //
@@ -78,7 +78,7 @@ IsDevicePathValid (
   return FALSE;
 }
 
-if (NodeLength > MAX_UINTN - Size) {
+if (NodeLength > MAX_UINT16 - Size) {
   return FALSE;
 }
 Size += NodeLength;
-- 
2.19.1

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

Re: [edk2] [edk2-test][Patch 3/3] uefi-sct/SctPkg:Add VerifySignature() Conf Test

[Supreeth Venkatesh]

Commit message to mention what this patch is imlementing.

In addition, there is cleanup to remove #if 0 block below.
Both of this should be in commit message.

On Thu, 2018-11-29 at 16:46 +0800, Eric Jin wrote:
> Cc: Supreeth Venkatesh 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin 
> ---
>  .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |   5 +-
>  .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |  16 ++
>  .../BlackBoxTest/Pkcs7BBTestConformance.c  | 303
> -
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |   2 -
>  4 files changed, 319 insertions(+), 7 deletions(-)
> 
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> index 4d433c3..8f6546a 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> @@ -36,7 +36,10 @@ EFI_GUID gPkcs7BBTestConformanceAssertionGuid007 =
> EFI_TEST_PKCS7BBTESTCONFORMAN
>  EFI_GUID gPkcs7BBTestConformanceAssertionGuid008 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_008_GUID;
>  EFI_GUID gPkcs7BBTestConformanceAssertionGuid009 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_009_GUID;
>  EFI_GUID gPkcs7BBTestConformanceAssertionGuid010 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_010_GUID;
> -
> +EFI_GUID gPkcs7BBTestConformanceAssertionGuid011 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_011_GUID;
> +EFI_GUID gPkcs7BBTestConformanceAssertionGuid012 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_012_GUID;
> +EFI_GUID gPkcs7BBTestConformanceAssertionGuid013 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_013_GUID;
> +EFI_GUID gPkcs7BBTestConformanceAssertionGuid014 =
> EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_014_GUID;
>  
>  EFI_GUID gPkcs7BBTestFunctionAssertionGuid001 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_001_GUID;
>  EFI_GUID gPkcs7BBTestFunctionAssertionGuid002 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_002_GUID;
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> index 32a00f6..f8d1b8f 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> @@ -65,6 +65,22 @@ extern EFI_GUID
> gPkcs7BBTestConformanceAssertionGuid009;
>  { 0xb136e016, 0x4f80, 0x44bd, {0xba, 0xb0, 0x1c, 0x34, 0x8a, 0x2d,
> 0xa1, 0x8a }}
>  extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid010;
>  
> +#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_011_GUID \
> +{ 0xa58f3626, 0xf16e, 0x4cd5, { 0xba, 0x12, 0x7a, 0x9d, 0xd6, 0x9a,
> 0x7a, 0x71 }}
> +extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid011;
> +
> +#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_012_GUID \
> +{ 0xbe4a0bf2, 0x2d46, 0x4d96, { 0xa6, 0x11, 0x21, 0x99, 0xa5, 0x5f,
> 0xa4, 0xee }}
> +extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid012;
> +
> +#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_013_GUID \
> +{ 0xc0536a27, 0x304e, 0x497a, { 0xa5, 0xe3, 0x94, 0x11, 0x38, 0x53,
> 0x6e, 0x40 }}
> +extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid013;
> +
> +#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_014_GUID \
> +{ 0x8c5aa0e8, 0x17ff, 0x49cd, { 0x8f, 0xec, 0x37, 0xc3, 0xfd, 0x5f,
> 0x77, 0x0 }}
> +extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid014;
> +
>  
>  #define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_001_GUID \
>  { 0x5c0eec50, 0xa6ea, 0x413c, {0x8a, 0x46, 0x4a, 0xd1, 0x4a, 0x77,
> 0x76, 0xf1 }}
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestConformance.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestConformance.c
> index b7bc19b..ce7d5bb 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestConformance.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestConformance.c
> @@ -278,10 +278,6 @@ BBTestVerifyBufferConformanceTest (
>   Status
>   );
>  
> -
> -
> -
> -
>// Signed data embedded in SignedData but InData is not NULL
>AllowedDb[0] = DbEntry1;
>RevokedDb[0] = NULL;
> @@ -507,4 +503,303 @@ BBTestVerifyBufferConformanceTest (
>return EFI_SUCCESS;
>  }
>  
> +EFI_STATUS
> +BBTestVerifySignatureConformanceTest (
> +  IN EFI_BB_TEST_PROTOCOL*This,
> +  IN VOID*ClientInterface,
> +  IN EFI_TEST_LEVEL  TestLevel,
> +  IN EFI_HANDLE  SupportHandle
> +  )
> +{
> +  EFI_STANDARD_TEST_LIBRARY_PROTOCOL*StandardLib;
> +  EFI_STATUSStatus;
> +  EFI_PKCS7_VERIFY_PROTOCOL *Pkcs7Verify;
> +  EFI_TEST_ASSERTION

Re: [edk2] [edk2-test][Patch 2/3] uefi-sct/SctPkg:Add VerifySignature() Func Test

[Supreeth Venkatesh]

Could we add some details on VerfiySignature() func test
in the commit message?
Also, looks like there are additional cleanup of the file
in addition to VerfiySignature() func test. (see inline below)
Coulyou please update commit message with the details 
of the cleanup. 

On Thu, 2018-11-29 at 16:42 +0800, Eric Jin wrote:
> Cc: Supreeth Venkatesh 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin 
> ---
>  .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |2 +
>  .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |7 +
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c | 1466
> +---
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestFunction.c |   87 +-
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |   52 +-
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.h |   22 +-
>  6 files changed, 1080 insertions(+), 556 deletions(-)
> 
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> index 142f6d4..4d433c3 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
> @@ -42,3 +42,5 @@ EFI_GUID gPkcs7BBTestFunctionAssertionGuid001 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASS
>  EFI_GUID gPkcs7BBTestFunctionAssertionGuid002 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_002_GUID;
>  EFI_GUID gPkcs7BBTestFunctionAssertionGuid003 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_003_GUID;
>  EFI_GUID gPkcs7BBTestFunctionAssertionGuid004 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_004_GUID;
> +EFI_GUID gPkcs7BBTestFunctionAssertionGuid005 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_005_GUID;
> +EFI_GUID gPkcs7BBTestFunctionAssertionGuid006 =
> EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_006_GUID;
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> index 2b5cd85..32a00f6 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
> @@ -82,4 +82,11 @@ extern EFI_GUID
> gPkcs7BBTestFunctionAssertionGuid003;
>  { 0x912e23ef, 0x299c, 0x41ab, {0xa0, 0xf5, 0xfc, 0xbc, 0xf6, 0xfd,
> 0xd3, 0x32 }}
>  extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid004;
>  
> +#define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_005_GUID \
> +{ 0x93740b06, 0xa186, 0x47ff, { 0xba, 0xc3, 0xdd, 0xa8, 0xcb, 0x7b,
> 0x18, 0x5e }}
> +extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid005;
> +
> +#define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_006_GUID \
> +{ 0x37253616, 0xca42, 0x4082, { 0x90, 0xda, 0xdb, 0x69, 0x98, 0x22,
> 0xa0, 0xe6 }}
> +extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid006;
>  
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestData.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestData.c
> index 0511e00..9b66938 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestData.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7B
> BTestData.c
> @@ -25,541 +25,979 @@ Abstract:
>  --*/
>  
>  //
> +// Test Root Certificate ("TestRoot.cer")
> +//
> +GLOBAL_REMOVE_IF_UNREFERENCED UINT8 TestRootCert[781] = {
> +  0x30, 0x82, 0x03, 0x09, 0x30, 0x82, 0x01, 0xF1, 0xA0, 0x03, 0x02,
> 0x01,
> +  0x02, 0x02, 0x10, 0xDE, 0x9F, 0x42, 0x91, 0x68, 0x16, 0xEA, 0x97,
> 0x4D,
> +  0xA1, 0x8A, 0x32, 0x25, 0xD6, 0xEE, 0x8D, 0x30, 0x0D, 0x06, 0x09,
> 0x2A,
> +  0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x0B, 0x05, 0x00, 0x30,
> 0x13,
> +  0x31, 0x11, 0x30, 0x0F, 0x06, 0x03, 0x55, 0x04, 0x03, 0x13, 0x08,
> 0x54,
> +  0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F, 0x74, 0x30, 0x1E, 0x17, 0x0D,
> 0x31,
> +  0x38, 0x30, 0x31, 0x32, 0x35, 0x30, 0x32, 0x30, 0x35, 0x35, 0x30,
> 0x5A,
> +  0x17, 0x0D, 0x33, 0x39, 0x31, 0x32, 0x33, 0x31, 0x32, 0x33, 0x35,
> 0x39,
> +  0x35, 0x39, 0x5A, 0x30, 0x13, 0x31, 0x11, 0x30, 0x0F, 0x06, 0x03,
> 0x55,
> +  0x04, 0x03, 0x13, 0x08, 0x54, 0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F,
> 0x74,
> +  0x30, 0x82, 0x01, 0x22, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48,
> 0x86,
> +  0xF7, 0x0D, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, 0x0F,
> 0x00,
> +  0x30, 0x82, 0x01, 0x0A, 0x02, 0x82, 0x01, 0x01, 0x00, 0xA5, 0x97,
> 0x23,
> +  0x48, 0xBE, 0xCA, 0xC8, 0xE0, 0x88, 0xC6, 0xA2, 0xAF, 0x78, 0x60,
> 0x94,
> +  0x48, 0x3E, 0x82, 0xE7, 0xD5, 0x62, 0x01, 0x73, 0x00, 0xEA, 0x42,
> 0x7A,
> +  0x32, 0x0A, 0xD7, 0x3F, 0x4D, 0x0B, 0x71, 0x6D, 0xD3, 0x50, 0x5E,
> 0x26,
> +  0x20, 0xE8, 0xCC, 0xB6, 0x0A, 0xAF, 0xD9, 0x07, 0x22, 0x17, 0x45,
> 0xD8,
> +  0x91, 0x75, 0x75, 0x52, 0xD8, 0x8C, 0xAB, 0x63, 0x0A, 0xF0, 0x23,
> 0x14,
> +  0x34, 0x92, 0x3F, 0xE0, 0x05, 0x24, 0x28, 0xED, 0x74, 

Re: [edk2] [PATCH v3 00/16] [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit

[Ard Biesheuvel]

On Thu, 29 Nov 2018 at 18:59, Ard Biesheuvel  wrote:
>
> On Wed, 28 Nov 2018 at 15:34, Ard Biesheuvel  
> wrote:
> >
> > This v3 subsumes and/or supersedes
> >
> > [PATCH v2 00/13] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit
> > [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> > [PATCH v2 0/2] ArmVirtPkg: remove high peripheral space mapping
> >
> > The ArmVirtQemu targets currently limit the size of the IPA space to
> > 40 bits because that is all what KVM supports. However, this is about
> > to change, and so we need to update the code if we want to ensure that
> > our UEFI firmware builds can keep running on systems that set values
> > other than 40 (which could be > 40 or < 40)
> >
> > This series refactors how we handle the maximum size of the physical
> > address space supported by the CPU in relation with the size of UEFI's
> > 1:1 mapping and the size of the GCD memory space map, taking the following
> > observations into account:
> > - the range of the linear mapping can be tied to whatever the CPU supports
> >   (as long as it doesn't exceed what the architecture permits for 4k pages)
> >   since we mostly already use the maximum of 4 levels anyway, and there is
> >   no memory cost involved beyond that
> > - there is usually no point in mapping the entire address space, which does
> >   involve a memory cost
> > - the GCD memory space may be required to cover more than what UEFI can
> >   address itself, since it is the based for the UEFI memory map that is
> >   provided to the OS
> >
> > Patches #1 and #2 remove some unused code to avoid having to fix it.
> >
> > Patches #3 and #4 update ArmVirtQemu and ArmVirtQemuKernel to drop the high
> > peripheral space mapping, and map whatever may reside there explicitly
> > (currently only the ECAM space in practice, but the MMIO view of the PCI
> > I/O space is mapped explicitly as well)
> >
> > Patch #5 was sent out before individually, and sets MAX_ADDRESS to the
> > maximum value AArch64 can map in UEFI which runs with 4k pages.
> >
> > Patch #6 adds a helper to ArmLib to read the number of supported address
> > bits and take this into account in the page table code (#8), which allows
> > PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of
> > the CPU.
> >
> > Patch #7 is mostly a cleanup patch, to switch to the new helper added in
> > patch #6. No functional changes intended.
> >
> > Patches #9 to #12 modify building of the CPU hob (and thus the size of the
> > GCD memory space) based on the CPU capabilities rather than the value of
> > PcdPrePiCpuMemorySize, which is dropped in the last patch.
> >
> > Pacthes #13 and #14 remove some needless references to PcdPrePiCpuMemorySize
> >
> > Patch #15 drops the overrides of PcdPrePiCpuMemorySize from all ArmVirtPkg
> > platforms.
> >
> > Cc: Laszlo Ersek 
> > Cc: Leif Lindholm 
> > Cc: Eric Auger 
> > Cc: Andrew Jones 
> > Cc: Philippe Mathieu-Daude 
> > Cc: Julien Grall 
> >
> > Ard Biesheuvel (16):
> >   EmbeddedPkg/TemplateSec: remove unused module
> >   EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library
> >   ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory
> > map
> >   ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range
> >   MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> >   ArmPkg/ArmLib: add support for reading the max physical address space
> > size
> >   ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size
> >   ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
> >   ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
> >   ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
> >   ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
> >   BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
> >   ArmPlatformPkg/PlatformPei: drop unused PCD references
> >   EmbeddedPkg/PrePiLib: drop unused PCD reference
> >   ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
> >   EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations
> >
>
> Thanks all for the reviews.
>
> Patches #1 .. #15 pushed as e979ea74aa14..55342094fb86
>
> Patch #16 needs to wait until edk2-platforms is brought up to date
> with the removal of PcdPrePiCpuMemorySize

Patch #16 now merged as 55342094fb86..bcf2a9db1f8e
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP

[Andrew Fish]

Felix,

I agree the obfuscation may not really be helpful. I've got a couple of 
thoughts.

1) I agree that I don't think the DebugLib is inherently safe on APs. I wonder 
if we could make a library class that was DebugLibMpSafe and have the same API 
as DebugLib. That way the library class matches how the module was coded. 
2) If we want to enforce the rules we should add code to the PEI or DXE Core to 
CpuBreakpoint() etc. if an AP calls a core service. 

Adding detection code is possible, but it is not trivial. For example if you 
have remembered the BSP and if the WhoAmI returns something different you need 
to check to see if some one changed the BSP. 

I guess for PEI the other option could be to have a fake PEI Services Table 
that stubs out all the functions with CpuBreakpoint() or some such?

Thanks,

Andrew Fish


> On Nov 30, 2018, at 6:33 AM, Felix Polyudov  wrote:
> 
> Ray,
> 
> I agree with the premise that calling PEI services from AP should generally 
> be avoided.
> However, the PEI services can be used on AP under certain special 
> circumstances.
> A couple of examples:
> 1. For debugging purposes. The MpInitLib contains 12 DEBUG calls and 19 
> ASSERT calls. Depending on the DebugLib instance used in the project, these 
> calls may lead to PEI services invocation.
> 2. MpInitLib provides ability to call AP in a serialized manner (only one AP 
> is running, other APs and BSP are waiting), when it is safe to call PEI 
> services.
> 
> Additionally, I think even if PEI services should not be used on AP, there is 
> still a reason to keep PEI services table pointer initialized.
> On one hand, given the complexity of modern firmware projects comprised of 
> modules coming from multiple vendors, making sure PEI services are not used 
> on AP can be a challenge.
> For example, in my case the call was coming from the chipset reference code.
> On the other hand, with the current implementation, when somebody does try to 
> use PEI services on AP the behavior is unpredictable. 
> Depending on the content of the uninitialized PEI service table pointer, the 
> system may either crash with one of the handful of different exceptions, 
> or it may start executing code from a random location. It's very difficult to 
> debug such issues. One can spend weeks chasing a problem like this.
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org 
> ] On Behalf Of Ni, Ruiyu
> Sent: Thursday, November 29, 2018 10:43 PM
> To: Felix Polyudov; edk2-devel@lists.01.org 
> Cc: ler...@redhat.com ; Dong, Eric
> Subject: Re: [edk2] [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer 
> on AP
> 
> Felix,
> I disagree:) Sorry about that. :)
> 
> The commit you mentioned might be made by me (didn't checked).
> Because I aimed to avoid calling PEI services from AP. That's a violation of 
> PI spec and not safe by design.
> 
> The AP calling standard services concern was raised by Andrew initially.
> 
> Thanks,
> Ray
> 
>> -Original Message-
>> From: Felix Polyudov [mailto:fel...@ami.com]
>> Sent: Friday, November 30, 2018 8:36 AM
>> To: edk2-devel@lists.01.org
>> Cc: Dong, Eric ; Ni, Ruiyu ;
>> ler...@redhat.com
>> Subject: [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP
>> 
>> According to PI specification PEI Services table pointer is stored right 
>> before ITD
>> base. Starting from commit c563077a380437c1 BSP and AP have different IDT
>> instances.
>> PEI Services table pointer was not initialized in the AP IDT instance.
>> As a result, any attempt to use functions from PeiServicesTablePointerLib or
>> PeiServicesLib on AP caused CPU exception.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Felix Polyudov 
>> ---
>> UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> index 7f4d6e6..0e3e362 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> @@ -1567,6 +1567,7 @@ MpInitLibInitialize (
>>   BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
>>   BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
>>   BufferSize += ApResetVectorSize;
>> +  BufferSize += sizeof(UINTN);
>>   BufferSize  = ALIGN_VALUE (BufferSize, 8);
>>   BufferSize += VolatileRegisters.Idtr.Limit + 1;
>>   BufferSize += sizeof (CPU_MP_DATA);
>> @@ -1587,6 +1588,8 @@ MpInitLibInitialize (
>>   // Backup Buffer
>>   //++
>>   //   Padding
>> +  //++
>> +  //PEI Services Table Pointer
>>   //++ <-- ApIdtBase (8-byte boundary)
>>   //   AP IDT  All APs share one separate IDT. So AP can get 
>> address of
>> CPU_MP_DATA from IDT Base.
>>   //

Re: [edk2] [RFC] Proposal to add edk2-apps repository

[Kinney, Michael D]

One of the goals with this proposal is to minimize
the amount of source code that needs to be setup in
a WORKSPACE to build firmware for a given platform.
If a platform does not require any applications that
require the libc, then it would be better if the libc
related packages and applications that use libc do not
have to be present in WORKSPACE.

Likewise, there are platforms that do not require the
UEFI Shell or any other UEFI Applications in order to
build the platform firmware image.

Perhaps we need one repo for libc related content and
another repo for UEFI Application related content.

Best regards,

Mike


> -Original Message-
> From: Carsey, Jaben
> Sent: Friday, November 30, 2018 7:50 AM
> To: Ni, Ruiyu ; krishnaLee
> ; Kinney, Michael D
> 
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [RFC] Proposal to add edk2-apps
> repository
> 
> I do not think that expanding shellPkg would work since
> there is no requirement that any of these apps depend
> on it.  As was stated, MicroPythonPkg does not.
> 
> I also do not think that moving ShellPkg makes lots of
> sense since it is used by many platforms.
> 
> -Jaben
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Ni,
> > Ruiyu
> > Sent: Thursday, November 29, 2018 7:40 PM
> > To: krishnaLee ; Kinney, Michael D
> > 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [RFC] Proposal to add edk2-apps
> repository
> > Importance: High
> >
> > Krishna,
> > The reason there are applications inside
> MdeModulePkg/Application is that
> > the shell protocol was in ShellPkg when the app was
> developed and
> > MdeModulePkg cannot depend on ShellPkg (rule).
> > Now since shell protocol is moved to MdePkg, any apps
> can depend on shell
> > protocol. (In fact they wanted to but just wasn't
> allowed due to reason
> > above.)
> >
> > I even prefer to move the ShellPkg to the edk2-app
> repo Mike proposed.
> > Instead of enlarge the ShellPkg:)
> >
> > I don't prefer edk2-libc unless we have a
> strategy/plan to make ordinary C
> > developer easy by promoting the std-c pkg.
> > The other reason I prefer edk2-app is then ShellPkg
> might be moved to that
> > new repo.
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of
> > > krishnaLee
> > > Sent: Friday, November 30, 2018 9:45 AM
> > > To: edk2-devel@lists.01.org
> > > Subject: Re: [edk2] [RFC] Proposal to add edk2-apps
> repository
> > >
> > > Kinney,
> > > I always think there may be two kinds of apps:
> > > 1,some apps have dependency on uefi_shell(shell-
> > lib,efi_shell_protocol,...they
> > > usually execute under uefi_shell),I would call them
> > "uefi_shell_application";
> > > 2,some apps have no dependency on uefi_shell(such
> as apps in
> > > MdeModulePkg/Application),I would call them
> > "standard_uefi_application".
> > >
> > > The "AppPkg / StdLib / StdLibPrivateInternalFiles"
> packages are usually
> > used by
> > > uefi_shell_application,I think they can all move to
> ShellPkg,no need to
> > create
> > > new package ?
> > >
> > >
> > > Thanks,
> > > krishna.
> > >
> > > At 2018-11-30 08:46:58, "Kinney, Michael D"
> 
> > > wrote:
> > > >Leif,
> > > >
> > > >I did consider the edk2-libc name.  The port of
> Python 2.7 is in the
> > > >AppPkg as well and it uses libc.
> > > >
> > > >So the content of this new package is a
> combination of libc And apps
> > > >that use libc.
> > > >
> > > >I am definitely open to alternate names.  2
> options so far:
> > > >
> > > >* edk2-apps
> > > >* edk2-libc
> > > >
> > > >Thanks,
> > > >
> > > >Mike
> > > >
> > > >> -Original Message-
> > > >> From: Leif Lindholm
> [mailto:leif.lindh...@linaro.org]
> > > >> Sent: Thursday, November 29, 2018 2:41 PM
> > > >> To: Kinney, Michael D
> 
> > > >> Cc: edk2-devel@lists.01.org
> > > >> Subject: Re: [edk2] [RFC] Proposal to add edk2-
> apps repository
> > > >>
> > > >> On Thu, Nov 29, 2018 at 05:58:08PM +,
> Kinney, Michael D wrote:
> > > >> > Hello,
> > > >> >
> > > >> > I would like to propose the creation of a new
> repository called
> > > >> > edk2-apps.  This repository would initially be
> used to host the
> > > >> > following packages from the edk2 repository:
> > > >> >
> > > >> > * AppPkg
> > > >> > * StdLib
> > > >> > * StdLibPrivateInternalFiles
> > > >>
> > > >> Let me start by saying I 100% back moving these
> out of the main edk2
> > > >> repository.
> > > >>
> > > >> > These 3 packages provide support for the libc
> along with
> > > >> > applications that depend on libc.  None of the
> other packages in
> > > >> > the edk2 repository use these packages, so
> these 3 package can be
> > > >> > safely moved without any impacts to platform
> firmware builds.
> > > >> > Build configurations that do use libc features
> can clone the
> > > >> > edk2-apps repository and add it to
> PACKAGES_PATH.
> > > >>
> > > >> I must confess to never having properly
> 

Re: [edk2] [RFC] Proposal to add edk2-apps repository

[Carsey, Jaben]

I do not think that expanding shellPkg would work since there is no requirement 
that any of these apps depend on it.  As was stated, MicroPythonPkg does not.

I also do not think that moving ShellPkg makes lots of sense since it is used 
by many platforms.

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Thursday, November 29, 2018 7:40 PM
> To: krishnaLee ; Kinney, Michael D
> 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps repository
> Importance: High
> 
> Krishna,
> The reason there are applications inside MdeModulePkg/Application is that
> the shell protocol was in ShellPkg when the app was developed and
> MdeModulePkg cannot depend on ShellPkg (rule).
> Now since shell protocol is moved to MdePkg, any apps can depend on shell
> protocol. (In fact they wanted to but just wasn't allowed due to reason
> above.)
> 
> I even prefer to move the ShellPkg to the edk2-app repo Mike proposed.
> Instead of enlarge the ShellPkg:)
> 
> I don't prefer edk2-libc unless we have a strategy/plan to make ordinary C
> developer easy by promoting the std-c pkg.
> The other reason I prefer edk2-app is then ShellPkg might be moved to that
> new repo.
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > krishnaLee
> > Sent: Friday, November 30, 2018 9:45 AM
> > To: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [RFC] Proposal to add edk2-apps repository
> >
> > Kinney,
> > I always think there may be two kinds of apps:
> > 1,some apps have dependency on uefi_shell(shell-
> lib,efi_shell_protocol,...they
> > usually execute under uefi_shell),I would call them
> "uefi_shell_application";
> > 2,some apps have no dependency on uefi_shell(such as apps in
> > MdeModulePkg/Application),I would call them
> "standard_uefi_application".
> >
> > The "AppPkg / StdLib / StdLibPrivateInternalFiles" packages are usually
> used by
> > uefi_shell_application,I think they can all move to ShellPkg,no need to
> create
> > new package ?
> >
> >
> > Thanks,
> > krishna.
> >
> > At 2018-11-30 08:46:58, "Kinney, Michael D" 
> > wrote:
> > >Leif,
> > >
> > >I did consider the edk2-libc name.  The port of Python 2.7 is in the
> > >AppPkg as well and it uses libc.
> > >
> > >So the content of this new package is a combination of libc And apps
> > >that use libc.
> > >
> > >I am definitely open to alternate names.  2 options so far:
> > >
> > >* edk2-apps
> > >* edk2-libc
> > >
> > >Thanks,
> > >
> > >Mike
> > >
> > >> -Original Message-
> > >> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > >> Sent: Thursday, November 29, 2018 2:41 PM
> > >> To: Kinney, Michael D 
> > >> Cc: edk2-devel@lists.01.org
> > >> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps repository
> > >>
> > >> On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael D wrote:
> > >> > Hello,
> > >> >
> > >> > I would like to propose the creation of a new repository called
> > >> > edk2-apps.  This repository would initially be used to host the
> > >> > following packages from the edk2 repository:
> > >> >
> > >> > * AppPkg
> > >> > * StdLib
> > >> > * StdLibPrivateInternalFiles
> > >>
> > >> Let me start by saying I 100% back moving these out of the main edk2
> > >> repository.
> > >>
> > >> > These 3 packages provide support for the libc along with
> > >> > applications that depend on libc.  None of the other packages in
> > >> > the edk2 repository use these packages, so these 3 package can be
> > >> > safely moved without any impacts to platform firmware builds.
> > >> > Build configurations that do use libc features can clone the
> > >> > edk2-apps repository and add it to PACKAGES_PATH.
> > >>
> > >> I must confess to never having properly understood the scope of
> > >> AppPkg to begin with.
> > >>
> > >> AppPkg/Applications/Hello does not appear to have any further (real)
> > >> dependency on libc than MdeModulePkg/Application/HelloWorld/, and
> .
> > >>
> > >> And certainly MdeModulePkg/Applications contain plenty of ...
> > >> applications.
> > >>
> > >> So, if the purpose is simply to provide some examples of application
> > >> written to libc rather than UEFI - should this be edk2- libc instead?
> > >>
> > >> Best Regards,
> > >>
> > >> Leif
> > >>
> > >> > The history of these 3 packages would be preserved when importing
> > >> > the content into edk2-apps.  After The import is verified, these 3
> > >> > packages would be deleted from the edk2 repository.
> > >> >
> > >> > This proposal helps reduce the size of the edk2 repository and
> > >> > focuses edk2 repository on packages used to provide UEFI/PI
> > >> > conformant firmware.
> > >> >
> > >> > If there are no concerns with this proposal, I will enter a
> > >> > Tianocore BZs for the two steps.
> > >> >
> > >> > Best regards,
> > >> >
> > >> > Mike
> > >> > ___
> > >> > 

Re: [edk2] [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Gao, Liming]

Ard: 
  OK. Will you send v2 patch to fix this issue?

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, November 29, 2018 11:18 PM
> To: Gao, Liming 
> Cc: edk2-devel@lists.01.org; Laszlo Ersek ; Zhu, Yonghong 
> ; Feng, Bob C
> 
> Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size 
> in IP address handling
> 
> On Thu, 29 Nov 2018 at 16:15, Gao, Liming  wrote:
> >
> > Do you verify which GCC arch? 32bit or 64bit or ARM?
> >
> 
> 64-bit ARM
> 
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Thursday, November 29, 2018 11:14 PM
> > > To: Gao, Liming 
> > > Cc: edk2-devel@lists.01.org; Laszlo Ersek ; Zhu, 
> > > Yonghong ; Feng, Bob C
> > > 
> > > Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word 
> > > size in IP address handling
> > >
> > > On Thu, 29 Nov 2018 at 16:11, Gao, Liming  wrote:
> > > >
> > > > Ard:
> > > >   I mean the build error. Besides, what test have you done with this 
> > > > patch set?
> > > >
> > > > CommonLib.c(1651): error C2220: warning treated as error - no 'object' 
> > > > file generated
> > > > CommonLib.c(1651): warning C4133: 'function': incompatible types - from 
> > > > 'UINTN *' to 'UINT64 *'
> > > > NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual 
> > > > Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'
> > > >
> > >
> > > Apologies, i missed this change at line 1624
> > >
> > > -  UINTN  Uint64;
> > > +  UINT64 Uint64;
> > >
> > > It builds fine with GCC though.
> > >
> > > > Below > +  UINTN  Uint64; ==> > +  UINT64   
> > > >Uint64;
> > > > > -Original Message-
> > > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > > Sent: Thursday, November 29, 2018 8:31 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ard Biesheuvel ; Laszlo Ersek 
> > > > > ; Zhu, Yonghong ;
> > > Gao,
> > > > > Liming ; Feng, Bob C 
> > > > > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word 
> > > > > size in IP address handling
> > > > >
> > > > > In the context of the BaseTools, there is no such thing as a native 
> > > > > word
> > > > > size, given that the same set of tools may be used to build a firmware
> > > > > image consisting of both 32-bit and 64-bit modules.
> > > > >
> > > > > So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> > > > > types instead of UINTN types when parsing strings.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel 
> > > > > ---
> > > > >  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
> > > > >  1 file changed, 14 insertions(+), 14 deletions(-)
> > > > >
> > > > > diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> > > > > b/BaseTools/Source/C/Common/CommonLib.c
> > > > > index 618aadac781a..bea6af0a45b1 100644
> > > > > --- a/BaseTools/Source/C/Common/CommonLib.c
> > > > > +++ b/BaseTools/Source/C/Common/CommonLib.c
> > > > > @@ -1785,7 +1785,7 @@ StrToIpv4Address (
> > > > >  {
> > > > >RETURN_STATUS  Status;
> > > > >UINTN  AddressIndex;
> > > > > -  UINTN  Uintn;
> > > > > +  UINTN  Uint64;
> > > > >EFI_IPv4_ADDRESS   LocalAddress;
> > > > >UINT8  LocalPrefixLength;
> > > > >CHAR16 *Pointer;
> > > > > @@ -1812,7 +1812,7 @@ StrToIpv4Address (
> > > > >  //
> > > > >  // Get D or P.
> > > > >  //
> > > > > -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, , 
> > > > > );
> > > > > +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, 
> > > > > , );
> > > > >  if (RETURN_ERROR (Status)) {
> > > > >return RETURN_UNSUPPORTED;
> > > > >  }
> > > > > @@ -1820,18 +1820,18 @@ StrToIpv4Address (
> > > > >//
> > > > >// It's P.
> > > > >//
> > > > > -  if (Uintn > 32) {
> > > > > +  if (Uint64 > 32) {
> > > > >  return RETURN_UNSUPPORTED;
> > > > >}
> > > > > -  LocalPrefixLength = (UINT8) Uintn;
> > > > > +  LocalPrefixLength = (UINT8) Uint64;
> > > > >  } else {
> > > > >//
> > > > >// It's D.
> > > > >//
> > > > > -  if (Uintn > MAX_UINT8) {
> > > > > +  if (Uint64 > MAX_UINT8) {
> > > > >  return RETURN_UNSUPPORTED;
> > > > >}
> > > > > -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> > > > > +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
> > > > >AddressIndex++;
> > > > >  }
> > > > >
> > > > > @@ -1888,7 +1888,7 @@ StrToIpv6Address (
> > > > >  {
> > > > >RETURN_STATUS  Status;
> > > > >UINTN  AddressIndex;
> > > > > -  UINTN  Uintn;
> > > > > +  UINT64 Uint64;
> > > > >EFI_IPv6_ADDRESS   LocalAddress;
> > > > 

Re: [edk2] [PATCH] MdePkg-BaseLib: PathCleanUpDirectories fix

[Jim.Dailey]

Oops!  I think this change may have an issue.

Hold off and I'll let you know if that's the case.

--Jim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dailey, 
Jim
Sent: Friday, November 30, 2018 9:12 AM
To: liming@intel.com; michael.d.kin...@intel.com
Cc: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdePkg-BaseLib: PathCleanUpDirectories fix


PathCleanUpDirectories does not handle "\..\" properly; it
returns "\" instead of "".  This change fixes that
problem so that "" is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jim Dailey 
---
 MdePkg/Library/BaseLib/FilePaths.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/FilePaths.c 
b/MdePkg/Library/BaseLib/FilePaths.c
index 92e4c350ff..69e46dd135 100644
--- a/MdePkg/Library/BaseLib/FilePaths.c
+++ b/MdePkg/Library/BaseLib/FilePaths.c
@@ -110,7 +110,12 @@ PathCleanUpDirectories(
  ((*(TempString + 3) == L'\\') || (*(TempString + 3) == CHAR_NULL))
 ) {
 *(TempString + 1) = CHAR_NULL;
-PathRemoveLastItem(Path);
+if (!PathRemoveLastItem(Path)) {
+  //
+  // We had "\.."
+  //
+  *Path = CHAR_NULL;
+}
 if (*(TempString + 3) != CHAR_NULL) {
   CopyMem (Path + StrLen (Path), TempString + 4, StrSize (TempString + 4));
 }
-- 
2.17.0.windows.1

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

[edk2] [PATCH] MdePkg-BaseLib: PathCleanUpDirectories fix

[Jim.Dailey]

PathCleanUpDirectories does not handle "\..\" properly; it
returns "\" instead of "".  This change fixes that
problem so that "" is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jim Dailey 
---
 MdePkg/Library/BaseLib/FilePaths.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/FilePaths.c 
b/MdePkg/Library/BaseLib/FilePaths.c
index 92e4c350ff..69e46dd135 100644
--- a/MdePkg/Library/BaseLib/FilePaths.c
+++ b/MdePkg/Library/BaseLib/FilePaths.c
@@ -110,7 +110,12 @@ PathCleanUpDirectories(
  ((*(TempString + 3) == L'\\') || (*(TempString + 3) == CHAR_NULL))
 ) {
 *(TempString + 1) = CHAR_NULL;
-PathRemoveLastItem(Path);
+if (!PathRemoveLastItem(Path)) {
+  //
+  // We had "\.."
+  //
+  *Path = CHAR_NULL;
+}
 if (*(TempString + 3) != CHAR_NULL) {
   CopyMem (Path + StrLen (Path), TempString + 4, StrSize (TempString + 4));
 }
-- 
2.17.0.windows.1

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

Re: [edk2] [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP

[Felix Polyudov]

Ray,

I agree with the premise that calling PEI services from AP should generally be 
avoided.
However, the PEI services can be used on AP under certain special circumstances.
A couple of examples:
1. For debugging purposes. The MpInitLib contains 12 DEBUG calls and 19 ASSERT 
calls. Depending on the DebugLib instance used in the project, these calls may 
lead to PEI services invocation.
2. MpInitLib provides ability to call AP in a serialized manner (only one AP is 
running, other APs and BSP are waiting), when it is safe to call PEI services.

Additionally, I think even if PEI services should not be used on AP, there is 
still a reason to keep PEI services table pointer initialized.
On one hand, given the complexity of modern firmware projects comprised of 
modules coming from multiple vendors, making sure PEI services are not used on 
AP can be a challenge.
For example, in my case the call was coming from the chipset reference code.
On the other hand, with the current implementation, when somebody does try to 
use PEI services on AP the behavior is unpredictable. 
Depending on the content of the uninitialized PEI service table pointer, the 
system may either crash with one of the handful of different exceptions, 
or it may start executing code from a random location. It's very difficult to 
debug such issues. One can spend weeks chasing a problem like this.


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni, Ruiyu
Sent: Thursday, November 29, 2018 10:43 PM
To: Felix Polyudov; edk2-devel@lists.01.org
Cc: ler...@redhat.com; Dong, Eric
Subject: Re: [edk2] [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on 
AP

Felix,
I disagree:) Sorry about that. :)

The commit you mentioned might be made by me (didn't checked).
Because I aimed to avoid calling PEI services from AP. That's a violation of PI 
spec and not safe by design.

The AP calling standard services concern was raised by Andrew initially.

Thanks,
Ray

> -Original Message-
> From: Felix Polyudov [mailto:fel...@ami.com]
> Sent: Friday, November 30, 2018 8:36 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Ni, Ruiyu ;
> ler...@redhat.com
> Subject: [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP
> 
> According to PI specification PEI Services table pointer is stored right 
> before ITD
> base. Starting from commit c563077a380437c1 BSP and AP have different IDT
> instances.
> PEI Services table pointer was not initialized in the AP IDT instance.
> As a result, any attempt to use functions from PeiServicesTablePointerLib or
> PeiServicesLib on AP caused CPU exception.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Felix Polyudov 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 7f4d6e6..0e3e362 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1567,6 +1567,7 @@ MpInitLibInitialize (
>BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
>BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
>BufferSize += ApResetVectorSize;
> +  BufferSize += sizeof(UINTN);
>BufferSize  = ALIGN_VALUE (BufferSize, 8);
>BufferSize += VolatileRegisters.Idtr.Limit + 1;
>BufferSize += sizeof (CPU_MP_DATA);
> @@ -1587,6 +1588,8 @@ MpInitLibInitialize (
>// Backup Buffer
>//++
>//   Padding
> +  //++
> +  //PEI Services Table Pointer
>//++ <-- ApIdtBase (8-byte boundary)
>//   AP IDT  All APs share one separate IDT. So AP can get 
> address of
> CPU_MP_DATA from IDT Base.
>//++ <-- CpuMpData
> @@ -1599,7 +1602,7 @@ MpInitLibInitialize (
>//
>MonitorBuffer= (UINT8 *) (Buffer + ApStackSize *
> MaxLogicalProcessorNumber);
>BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize *
> MaxLogicalProcessorNumber;
> -  ApIdtBase= ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8);
> +  ApIdtBase= ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize +
> sizeof(UINTN), 8);
>CpuMpData= (CPU_MP_DATA *) (ApIdtBase + 
> VolatileRegisters.Idtr.Limit +
> 1);
>CpuMpData->Buffer   = Buffer;
>CpuMpData->CpuApStackSize   = ApStackSize;
> @@ -1653,6 +1656,11 @@ MpInitLibInitialize (
>Buffer + BufferSize);
> 
>//
> +  // Initialize PEI Services table pointer. Copy the address from BSP.
> +  //
> +  *(UINTN*)(ApIdtBase - sizeof(UINTN)) =
> + *(UINTN*)(VolatileRegisters.Idtr.Base - sizeof (UINTN));
> +
> +  //
>// Duplicate BSP's IDT to APs.
>// All APs share one separate IDT. So AP can get the address of CpuMpData 
> by
> using IDTR.BASE + IDTR.LIMIT + 1
>//
> --
> 2.10.0.windows.1
> 
> 
> 
> 

[edk2] [edk2-platforms] [PATCH v6 2/2] Platform/ARM: Add Readme.md

[Nariman Poushin]

This covers the bulk of the information originally present in
https://github.com/tianocore/tianocore.github.io/wiki/ArmPlatformPkg-AArch64
regarding building and running the Foundation/Base FVP Platforms.

The sections on fetching source have been delegated to the root Readme.md

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Nariman Poushin 
---

Changes from v5:

- Properly fixed (rather tha NOT) fix the prebuilt_tools repo url

 Platform/ARM/Readme.md | 62 ++
 Readme.md  |  4 +---
 2 files changed, 63 insertions(+), 3 deletions(-)
 create mode 100644 Platform/ARM/Readme.md

diff --git a/Platform/ARM/Readme.md b/Platform/ARM/Readme.md
new file mode 100644
index 000..d343e7a
--- /dev/null
+++ b/Platform/ARM/Readme.md
@@ -0,0 +1,62 @@
+== Introduction ==
+
+These instructions explain how to get an edk2/edk2-platforms build running
+on the ARM Base FVP, which is a software model provided by ARM (for free)
+, which models a Cortex A core with various peripherals. More information
+can be found here:
+https://developer.arm.com/products/system-design/fixed-virtual-platforms
+
+Requirement:
+* A 32-bit or 64-bit Linux host machine.
+* Visual Studio is not officially supported, experimental support can be found 
here:
+[https://git.linaro.org/people/leif.lindholm/edk2.git/log/?h=aarch64-vs]
+
+== Build EDK2 Tianocore ==
+
+cd $(WORKSPACE)/edk2For the Foundation and Base FVPs (defined by 
the DSC file Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc):
+build -a AARCH64 -p Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
-t GCC5
+
+Once built, the edk2 image is the following file 
Build/ArmVExpress-FVP-AArch64/DEBUG_GCC5/FV/FVP_AARCH64_EFI.fd
+
+=== Run edk2/edk2-platforms on the ARM Base Platform FVP ===
+
+In order to run the binary we have just built there are a few steps we need to
+go through, we need to get a model, a set of prebuilts (where we will swap out
+the edk2 image with our own) and the tool with which we will swap out the
+prebuilt edk2 image.
+
+We will also rely on the "run_model" script that comes with the prebuilts, it
+is entirely possible to run the model without this but would require quite a 
bit
+of knowledge regarding the areguments ARM fastmodel (documentation can be 
found here:
+https://developer.arm.com/docs/100966/1101/programming-reference-for-base-fvps/base-platform-revc-features)
+however the manual set of the FVP is outside the scope of this document. If 
you are interested
+please consult the documentation.
+
+It's recommended you create a folder where you download the prebuilts and
+required tool and copy your edk2 image in to it, as the run script expects
+the binaries in the same directory.
+
+1) Download the Base FVP from here 
https://developer.arm.com/products/system-design/fixed-virtual-platforms
+
+   - Select Armv8-A Base Platform FVP based on Fast Models 11.4
+   - It has a click through license but is free.
+
+2) Download the 18.10 Linaro ARM Landing Team release for FVP booting UEFI
+https://releases.linaro.org/members/arm/platforms/18.10/fvp-uefi.zip
+
+3) Download the prebuilt fiptool from 
https://git.linaro.org/landing-teams/working/arm/prebuilt_tools.git
+
+4) Update the fip.bin image from fvp-uefi.zip by running the following command:
+
+   fiptool update --nt-fw=[path to binary built above] fip.bin
+
+5) Execute the FVP run_model.sh script from fvp-uefi.zip and provide a path to 
the FVP binaries
+downloaded in step 1):
+
+   MODEL=[path to FVP binary] ./run_model.sh
+
+This expects the contents of fvp-uefi.zip, the bl1.bin and fip.bin (which is
+the file we modify), to be in the same directory as the run_model.sh script.
+
+This should be sufficient to provide a build/run/debug environment for aarch64.
+
diff --git a/Readme.md b/Readme.md
index 6ad5953..6748826 100644
--- a/Readme.md
+++ b/Readme.md
@@ -206,9 +206,7 @@ they will be documented with the platform.
 * [Overdrive](Platform/AMD/OverdriveBoard)
 * [Overdrive 1000](Platform/SoftIron/Overdrive1000Board)
 
-## ARM
-* [Juno](Platform/ARM/JunoPkg)
-* [Versatile Express](Platform/ARM/VExpressPkg)
+## [ARM](Platform/ARM/Readme.md)
 
 ## Hisilicon
 * [D02](Platform/Hisilicon/D02)
-- 
2.7.4

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

Re: [edk2] [PATCH v2 2/4] ArmPkg/ArmMmuLib ARM: handle unmapped sections when updating permissions

[Leif Lindholm]

On Fri, Nov 30, 2018 at 12:28:27PM +0100, Ard Biesheuvel wrote:
> The ARM ArmMmuLib code currently does not take into account that
> setting permissions on a region should take into account that a
> region may not be mapped yet to begin with.
> 
> So when updating a section descriptor whose old value is zero,
> pass in the address explicitly.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c 
> b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> index ec51e072ab43..889b22867dc7 100644
> --- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
> @@ -695,8 +695,12 @@ UpdateSectionEntries (
>  } else {
>// still a section entry
>  
> -  // mask off appropriate fields
> -  Descriptor = CurrentDescriptor & ~EntryMask;
> +  if (CurrentDescriptor != 0) {
> +// mask off appropriate fields
> +Descriptor = CurrentDescriptor & ~EntryMask;
> +  } else {
> +Descriptor = ((UINTN)FirstLevelIdx + i) << 
> TT_DESCRIPTOR_SECTION_BASE_SHIFT;
> +  }
>  
>// mask in new attributes and/or permissions
>Descriptor |= EntryValue;
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/4] ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()

[Leif Lindholm]

On Fri, Nov 30, 2018 at 12:28:26PM +0100, Ard Biesheuvel wrote:
> GetMemoryRegion() is used to obtain the attributes of an existing
> mapping, to permit permission attribute changes to be optimized
> away if the attributes don't actually change.
> 
> The current ARM code assumes that a section mapping or a page mapping
> exists for any region passed into GetMemoryRegion(), but the region
> may be unmapped entirely, in which case the code will crash. So check
> if a section mapping exists before dereferencing it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> index 12ca5b26673e..3b29d33d0a9c 100644
> --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> @@ -457,6 +457,9 @@ GetMemoryRegion (
>  
>// Get the section at the given index
>SectionDescriptor = FirstLevelTable[TableIndex];
> +  if (!SectionDescriptor) {
> +return EFI_NOT_FOUND;
> +  }
>  
>// If 'BaseAddress' belongs to the section then round it to the section 
> boundary
>if (((SectionDescriptor & TT_DESCRIPTOR_SECTION_TYPE_MASK) == 
> TT_DESCRIPTOR_SECTION_TYPE_SECTION) ||
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 3/4] ArmVirtPkg/NorFlashQemuLib: disregard our primary FV

[Ard Biesheuvel]

The primary FV contains the firmware boot image, which is not
runtime updatable in our case. So exposing it to the NOR flash
driver is undesirable, since it may attempt to modify the NOR
flash contents. It is also rather pointless, since we don't
keep anything there that we care to expose. (the SEC and PEI
phase modules are not executable from DXE context, and the
contents of the embedded DXE phase FV are exposed by the DXE
core directly via the FVB2 protocol)

So let's disregard the NOR flash block that covers the primary
FV.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf |  5 +
 ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c   | 13 +++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf 
b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
index d86ff36dbd58..c5752a243e6b 100644
--- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
+++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
@@ -28,6 +28,7 @@ [Sources.common]
 [Packages]
   MdePkg/MdePkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
+  ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
 
 [LibraryClasses]
@@ -40,3 +41,7 @@ [Protocols]
 
 [Depex]
   gFdtClientProtocolGuid
+
+[Pcd]
+  gArmTokenSpaceGuid.PcdFvBaseAddress
+  gArmTokenSpaceGuid.PcdFvSize
diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c 
b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
index 2678f57eaaad..d238e39a59f1 100644
--- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
+++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
@@ -75,13 +75,22 @@ NorFlashPlatformGetDevices (
   Size = SwapBytes64 (ReadUnaligned64 ((VOID *)[2]));
   Reg += 4;
 
+  PropSize -= 4 * sizeof (UINT32);
+
+  //
+  // Disregard any flash devices that overlap with the primary FV.
+  // The firmware is not updatable from inside the guest anyway.
+  //
+  if ((PcdGet64 (PcdFvBaseAddress) + PcdGet32 (PcdFvSize) > Base) &&
+  (Base + Size) > PcdGet64 (PcdFvBaseAddress)) {
+continue;
+  }
+
   mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)Base;
   mNorFlashDevices[Num].RegionBaseAddress = (UINTN)Base;
   mNorFlashDevices[Num].Size  = (UINTN)Size;
   mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
   Num++;
-
-  PropSize -= 4 * sizeof (UINT32);
 }
   }
 
-- 
2.19.1

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

[edk2] [PATCH v2 0/4] ArmVirtQemu: unmap page #0 to catch NULL pointer dereferences

[Ard Biesheuvel]

Rationale in patch #4. Patch #3 is a prerequisite patch that ensures
that we no longer need page #0 to be mapped for the NOR flash driver
to be able to expose it as a read/write block device.

Patches #1 and #2 are fixes for the ARM version of the ArmMmuLib driver
for bugs that get triggered by these changes.

Cc: Leif Lindholm 
Cc: Laszlo Ersek 
Cc: Eric Auger 
Cc: Andrew Jones 
Cc: Philippe Mathieu-Daude 

Ard Biesheuvel (4):
  ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()
  ArmPkg/ArmMmuLib ARM: handle unmapped sections when updating
permissions
  ArmVirtPkg/NorFlashQemuLib: disregard our primary FV
  ArmVirtPkg/QemuVirtMemInfoLib: trim the MMIO region mapping

 .../NorFlashQemuLib/NorFlashQemuLib.inf   |  5 
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf |  4 ++--
 .../QemuVirtMemInfoPeiLib.inf |  2 ++
 ArmPkg/Drivers/CpuDxe/Arm/Mmu.c   |  3 +++
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c  |  8 +--
 .../Library/NorFlashQemuLib/NorFlashQemuLib.c | 13 +--
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.c   | 23 +--
 7 files changed, 45 insertions(+), 13 deletions(-)

-- 
2.19.1

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

[edk2] [PATCH v2 1/4] ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()

[Ard Biesheuvel]

GetMemoryRegion() is used to obtain the attributes of an existing
mapping, to permit permission attribute changes to be optimized
away if the attributes don't actually change.

The current ARM code assumes that a section mapping or a page mapping
exists for any region passed into GetMemoryRegion(), but the region
may be unmapped entirely, in which case the code will crash. So check
if a section mapping exists before dereferencing it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 12ca5b26673e..3b29d33d0a9c 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -457,6 +457,9 @@ GetMemoryRegion (
 
   // Get the section at the given index
   SectionDescriptor = FirstLevelTable[TableIndex];
+  if (!SectionDescriptor) {
+return EFI_NOT_FOUND;
+  }
 
   // If 'BaseAddress' belongs to the section then round it to the section 
boundary
   if (((SectionDescriptor & TT_DESCRIPTOR_SECTION_TYPE_MASK) == 
TT_DESCRIPTOR_SECTION_TYPE_SECTION) ||
-- 
2.19.1

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

[edk2] [PATCH v2 4/4] ArmVirtPkg/QemuVirtMemInfoLib: trim the MMIO region mapping

[Ard Biesheuvel]

QEMU/mach-virt is rather unhelpful when it comes to tracking down
NULL pointer dereferences that occur while running in UEFI: since
we have NOR flash mapped at address 0x0, inadvertent reads go
unnoticed, and even most writes are silently dropped, unless you're
unlucky and the instruction in question is one that KVM cannot
emulate, in which case you end up with a QEMU crash like this:

  error: kvm run failed Function not implemented
   PC=00013f7ff804 X00=00013f7ab108 X01=0064
  X02=00013f801988 X03=83c4 X04=
  X05=9644 X06=fffd8270 X07=00013f7ab4a0
  X08=0001 X09=00013f803b88 X10=00013f7e88d0
  X11=0009 X12=00013f7ab554 X13=0008
  X14=0002 X15= X16=
  X17= X18= X19=
  X20=00013f81c000 X21=00013f7ab170 X22=00013f81c000
  X23=0918 X24=00013f407020 X25=00013f81c000
  X26=00013f803530 X27=00013f802000 X28=00013f7ab270
  X29=00013f7ab0d0 X30=00013f7fee10  SP=00013f7a6f30
  PSTATE=83c5 N--- EL1h

and a warning in the host kernel log that load/store instruction
decoding is not supported by KVM.

Given that the first page of the flash device is not actually
used anyway, let's reduce the mappings of the peripheral space
and the flash device (both of which cover page #0) to only cover
what is actually required:

  ArmVirtQemu.fdf:
  > 0x1000|0x001ff000
  > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize

  ArmVirtQemuKernel.fdf:
  > 0x8000|0x001f8000
  > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize

For ArmVirtQemu, the resulting virtual mapping looks roughly like:
- [0, 4K)   : flash, unmapped
- [4K, 2M)  : flash, mapped as WB+X RAM
- [2M, 64M) : flash, unmapped
- [64M, 128M)   : varstore flash, will be mapped by the NOR flash driver
- [128M, 256M)  : peripherals, mapped as device
- [256M, 1GB)   : 32-bit MMIO aperture, translated IO aperture, ECAM,
  will be mapped by the PCI host bridge driver
- [1GB, ...): RAM, mapped.

After this change, any inadvertent read or write from/to the first
physical page will trigger a translation fault inside the guest,
regardless of the nature of the instruction, without crashing QEMU.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf|  4 ++--
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf |  2 ++
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c  | 23 
++--
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
index 5c5b841051ad..b6abc52531a8 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
@@ -39,9 +39,9 @@ [LibraryClasses]
   PcdLib
 
 [Pcd]
-  gArmTokenSpaceGuid.PcdFdBaseAddress
+  gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdSystemMemoryBase
   gArmTokenSpaceGuid.PcdSystemMemorySize
 
 [FixedPcd]
-  gArmTokenSpaceGuid.PcdFdSize
+  gArmTokenSpaceGuid.PcdFvSize
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
index d12089760b22..16802c5c414b 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
@@ -43,9 +43,11 @@ [LibraryClasses]
 
 [Pcd]
   gArmTokenSpaceGuid.PcdFdBaseAddress
+  gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdSystemMemoryBase
   gArmTokenSpaceGuid.PcdSystemMemorySize
 
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFdSize
+  gArmTokenSpaceGuid.PcdFvSize
   gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
index 0285a11b1d77..a26b2fbad9be 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
@@ -21,6 +21,15 @@
 // Number of Virtual Memory Map Descriptors
 #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  5
 
+//
+// mach-virt's core peripherals such as the UART, the GIC and the RTC are
+// all mapped in the 'miscellaneous device I/O' region, which we just map
+// in its entirety rather than device by device. Note that it does not
+// cover any of the NOR flash banks or PCI resource windows.
+//
+#define MACH_VIRT_PERIPH_BASE   0x0800
+#define MACH_VIRT_PERIPH_SIZE   SIZE_128MB
+
 /**
   Return the Virtual Memory Map of your platform
 
@@ -66,16 +75,16 @@ ArmVirtGetMemoryMap (
   

[edk2] [PATCH v2 2/4] ArmPkg/ArmMmuLib ARM: handle unmapped sections when updating permissions

[Ard Biesheuvel]

The ARM ArmMmuLib code currently does not take into account that
setting permissions on a region should take into account that a
region may not be mapped yet to begin with.

So when updating a section descriptor whose old value is zero,
pass in the address explicitly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index ec51e072ab43..889b22867dc7 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -695,8 +695,12 @@ UpdateSectionEntries (
 } else {
   // still a section entry
 
-  // mask off appropriate fields
-  Descriptor = CurrentDescriptor & ~EntryMask;
+  if (CurrentDescriptor != 0) {
+// mask off appropriate fields
+Descriptor = CurrentDescriptor & ~EntryMask;
+  } else {
+Descriptor = ((UINTN)FirstLevelIdx + i) << 
TT_DESCRIPTOR_SECTION_BASE_SHIFT;
+  }
 
   // mask in new attributes and/or permissions
   Descriptor |= EntryValue;
-- 
2.19.1

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

Re: [edk2] [PATCH edk2-platforms] Platform, Silicon: drop gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize

[Ard Biesheuvel]

On Fri, 30 Nov 2018 at 12:00, Leif Lindholm  wrote:
>
> On Fri, Nov 30, 2018 at 11:55:03AM +0100, Ard Biesheuvel wrote:
> > gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize will be removed, so
> > drop any overrides from the platforms in edk2-platforms.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
>
> Reviewed-by: Leif Lindholm 
>

Thanks

Pushed as 77ae8610df3e..07c6bc27730d

> > ---
> >  Silicon/Hisilicon/Hisilicon.dsc.inc  | 1 -
> >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc| 1 -
> >  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 5 -
> >  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 1 -
> >  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 3 ---
> >  Platform/Comcast/RDKQemu/RDKQemu.dsc | 4 
> >  Platform/Hisilicon/D06/D06.dsc   | 1 -
> >  Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 5 -
> >  Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 -
> >  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 -
> >  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 5 -
> >  11 files changed, 28 deletions(-)
> >
> > diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
> > b/Silicon/Hisilicon/Hisilicon.dsc.inc
> > index 3ac8e202322d..63d28a57406b 100644
> > --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> > +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> > @@ -253,7 +253,6 @@
> >gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE
> >
> >  [PcdsFixedAtBuild.common]
> > -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|44
> >#
> ># IO is mapped to memory space, so we use the same size of
> ># PcdPrePiCpuMemorySize
> > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
> > b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > index 14a1bda7b8b4..b3fd1846c0bf 100644
> > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > @@ -375,7 +375,6 @@
> >gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
> >
> >gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
> > -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
> >
> >gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F
> >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1
> > diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
> > b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> > index 51327a67dffb..b062f671f57f 100644
> > --- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> > +++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> > @@ -385,11 +385,6 @@ DEFINE DO_CAPSULE   = FALSE
> ># Size of the region used by UEFI in permanent memory (Reserved 64MB)
> >gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
> >
> > -  # 40 bits of VA space is sufficient to support up to 512 GB of RAM in the
> > -  # range 0x80__ - 0xFF__ (all platform and PCI MMIO is 
> > below
> > -  # that)
> > -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> > -
> >#
> ># ARM PrimeCell
> >#
> > diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc 
> > b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > index 68249add3127..52ca796a4f28 100644
> > --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> > @@ -145,7 +145,6 @@
> >gArmTokenSpaceGuid.PcdPciMmio64Translation|0x0
> >gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x6000
> >gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
> > -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> >
> >## PL011 - Serial Terminal
> >gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF8
> > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
> > b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > index d20f1a738710..7094e57ee13a 100644
> > --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> > @@ -159,9 +159,6 @@
> ># Set tick frequency value to 100Mhz
> >gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|1
> >
> > -  # the entire FVP address space can be covered by 36 bit VAs
> > -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
> > -
> >#
> ># ACPI Table Version
> >#
> > diff --git a/Platform/Comcast/RDKQemu/RDKQemu.dsc 
> > b/Platform/Comcast/RDKQemu/RDKQemu.dsc
> > index b36c7cb7842f..f22f14aed99d 100644
> > --- a/Platform/Comcast/RDKQemu/RDKQemu.dsc
> > +++ b/Platform/Comcast/RDKQemu/RDKQemu.dsc
> > @@ -154,10 +154,6 @@
> >gRdkTokenSpaceGuid.PcdRdkConfFileDevicePath|L"PciRoot(0x0)/Pci(0x2,0x0)"
> >
> >  [PcdsFixedAtBuild.AARCH64]
> > -  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
> > -  # support anything bigger, even if the host hardware does
> > -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> > -
> ># Clearing BIT0 in this PCD 

Re: [edk2] [PATCH edk2-platforms] Platform, Silicon: drop gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize

[Leif Lindholm]

On Fri, Nov 30, 2018 at 11:55:03AM +0100, Ard Biesheuvel wrote:
> gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize will be removed, so
> drop any overrides from the platforms in edk2-platforms.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  Silicon/Hisilicon/Hisilicon.dsc.inc  | 1 -
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc| 1 -
>  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 5 -
>  Platform/ARM/SgiPkg/SgiPlatform.dsc  | 1 -
>  Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 3 ---
>  Platform/Comcast/RDKQemu/RDKQemu.dsc | 4 
>  Platform/Hisilicon/D06/D06.dsc   | 1 -
>  Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 5 -
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 -
>  Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 -
>  Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 5 -
>  11 files changed, 28 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
> b/Silicon/Hisilicon/Hisilicon.dsc.inc
> index 3ac8e202322d..63d28a57406b 100644
> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> @@ -253,7 +253,6 @@
>gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE
>  
>  [PcdsFixedAtBuild.common]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|44
>#
># IO is mapped to memory space, so we use the same size of
># PcdPrePiCpuMemorySize
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
> b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 14a1bda7b8b4..b3fd1846c0bf 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -375,7 +375,6 @@
>gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
>  
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>  
>gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F
>gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1
> diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
> b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> index 51327a67dffb..b062f671f57f 100644
> --- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> +++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
> @@ -385,11 +385,6 @@ DEFINE DO_CAPSULE   = FALSE
># Size of the region used by UEFI in permanent memory (Reserved 64MB)
>gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
>  
> -  # 40 bits of VA space is sufficient to support up to 512 GB of RAM in the
> -  # range 0x80__ - 0xFF__ (all platform and PCI MMIO is below
> -  # that)
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> -
>#
># ARM PrimeCell
>#
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc 
> b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> index 68249add3127..52ca796a4f28 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -145,7 +145,6 @@
>gArmTokenSpaceGuid.PcdPciMmio64Translation|0x0
>gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x6000
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>  
>## PL011 - Serial Terminal
>gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF8
> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
> b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> index d20f1a738710..7094e57ee13a 100644
> --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> @@ -159,9 +159,6 @@
># Set tick frequency value to 100Mhz
>gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|1
>  
> -  # the entire FVP address space can be covered by 36 bit VAs
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
> -
>#
># ACPI Table Version
>#
> diff --git a/Platform/Comcast/RDKQemu/RDKQemu.dsc 
> b/Platform/Comcast/RDKQemu/RDKQemu.dsc
> index b36c7cb7842f..f22f14aed99d 100644
> --- a/Platform/Comcast/RDKQemu/RDKQemu.dsc
> +++ b/Platform/Comcast/RDKQemu/RDKQemu.dsc
> @@ -154,10 +154,6 @@
>gRdkTokenSpaceGuid.PcdRdkConfFileDevicePath|L"PciRoot(0x0)/Pci(0x2,0x0)"
>  
>  [PcdsFixedAtBuild.AARCH64]
> -  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
> -  # support anything bigger, even if the host hardware does
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> -
># Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry 
> point,
># if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
># presence of the 32-bit entry point anyway (because many AARCH64 systems
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 

[edk2] [PATCH edk2-platforms] Platform, Silicon: drop gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize

[Ard Biesheuvel]

gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize will be removed, so
drop any overrides from the platforms in edk2-platforms.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Silicon/Hisilicon/Hisilicon.dsc.inc  | 1 -
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc| 1 -
 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc   | 5 -
 Platform/ARM/SgiPkg/SgiPlatform.dsc  | 1 -
 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 3 ---
 Platform/Comcast/RDKQemu/RDKQemu.dsc | 4 
 Platform/Hisilicon/D06/D06.dsc   | 1 -
 Platform/LeMaker/CelloBoard/CelloBoard.dsc   | 5 -
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 -
 Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 -
 Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc  | 5 -
 11 files changed, 28 deletions(-)

diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc 
b/Silicon/Hisilicon/Hisilicon.dsc.inc
index 3ac8e202322d..63d28a57406b 100644
--- a/Silicon/Hisilicon/Hisilicon.dsc.inc
+++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
@@ -253,7 +253,6 @@
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE
 
 [PcdsFixedAtBuild.common]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|44
   #
   # IO is mapped to memory space, so we use the same size of
   # PcdPrePiCpuMemorySize
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 14a1bda7b8b4..b3fd1846c0bf 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -375,7 +375,6 @@
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x4000
 
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
 
   gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x41F
   gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x1
diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
index 51327a67dffb..b062f671f57f 100644
--- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
+++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
@@ -385,11 +385,6 @@ DEFINE DO_CAPSULE   = FALSE
   # Size of the region used by UEFI in permanent memory (Reserved 64MB)
   gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
 
-  # 40 bits of VA space is sufficient to support up to 512 GB of RAM in the
-  # range 0x80__ - 0xFF__ (all platform and PCI MMIO is below
-  # that)
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
   #
   # ARM PrimeCell
   #
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc 
b/Platform/ARM/SgiPkg/SgiPlatform.dsc
index 68249add3127..52ca796a4f28 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
@@ -145,7 +145,6 @@
   gArmTokenSpaceGuid.PcdPciMmio64Translation|0x0
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x6000
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
 
   ## PL011 - Serial Terminal
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF8
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
index d20f1a738710..7094e57ee13a 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
@@ -159,9 +159,6 @@
   # Set tick frequency value to 100Mhz
   gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|1
 
-  # the entire FVP address space can be covered by 36 bit VAs
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
-
   #
   # ACPI Table Version
   #
diff --git a/Platform/Comcast/RDKQemu/RDKQemu.dsc 
b/Platform/Comcast/RDKQemu/RDKQemu.dsc
index b36c7cb7842f..f22f14aed99d 100644
--- a/Platform/Comcast/RDKQemu/RDKQemu.dsc
+++ b/Platform/Comcast/RDKQemu/RDKQemu.dsc
@@ -154,10 +154,6 @@
   gRdkTokenSpaceGuid.PcdRdkConfFileDevicePath|L"PciRoot(0x0)/Pci(0x2,0x0)"
 
 [PcdsFixedAtBuild.AARCH64]
-  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
-  # support anything bigger, even if the host hardware does
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
   # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
   # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
   # presence of the 32-bit entry point anyway (because many AARCH64 systems
diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
index 742fe30b62c3..396bd03c9d24 100644
--- a/Platform/Hisilicon/D06/D06.dsc
+++ b/Platform/Hisilicon/D06/D06.dsc
@@ -128,7 +128,6 @@
 
 [PcdsFixedAtBuild.common]
   gArmPlatformTokenSpaceGuid.PcdCoreCount|48
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|48
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
 
diff --git 

Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

[Udit Kumar]

Thanks Laszlo/Andrew
Finally I manage to get logs from user-space,  problem  was fifo of PL011 uart 
was not getting enable in case of direct boot.
But in case of boot via UiApp, some piece of code was setting serial port 
attribute to enable this ( I still to figure out from where). 
OS rely on boot-loader to enable this bit.
 
Regards
Udit

> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, November 29, 2018 11:31 PM
> To: Udit Kumar ; af...@apple.com
> Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Zeng, Star
> 
> Subject: Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot
> 
> On 11/29/18 14:12, Udit Kumar wrote:
> > Thanks Laszlo,
> >
> >
> >> I can only think of some terminal control sequences that are *not*
> >> printed to the terminal when you don't enter UiApp manually. I don't
> >> understand how that could cause the exact symptom you describe, but I have
> no better explanation.
> >>
> >> Can you try other serial communication programs on your desktop? Such
> >> as "minicom" or "screen"?
> >
> > Screen didn't help.
> > Moreover , using different OS distributions show same similar behavior !!
> >
> >> Also, can you try changing your "console=..." kernel param(s)?
> >
> > You meant baud-rate ?
> 
> Yes, and more. The options that the "console=" kernel parameter takes.
> 
> >
> > On uefi side,  could you help me if there is some extra information
> > passed to OS in path UiApp -> BootDevice,
> 
> I don't think so. Nothing comes to my mind anyway.
> 
> > I could see , some of additional protocols are installed in above
> > path, I am not sure if those are used  by  OS or OS Loader (grub in my case)
> somehow.
> 
> Well, UiApp generally connects all drivers to all devices -- normally a 
> platform
> BDS would not want to do this, for the sake of booting quickly --, which 
> likely
> results in more protocol instances being installed in the system. That 
> shouldn't
> cause a difference for how serial behaves once the OS has booted.
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 0/2] Remove DuetPkg and unused tools

[Gao, Liming]

Good point. For BaseTools part, we also need to remove their user manual and 
PosixLike in BaseTools\UserManuals and BaseTools\BinWrappers\PosixLike 
directory. 

Thanks
Liming
>-Original Message-
>From: Wu, Hao A
>Sent: Friday, November 30, 2018 4:15 PM
>To: Zhang, Shenglei ; edk2-devel@lists.01.org
>Cc: Ni, Ruiyu ; Gao, Liming 
>Subject: RE: [edk2] [PATCH v3 0/2] Remove DuetPkg and unused tools
>
>For the DuetPkg change:
>Reviewed-by: Hao Wu 
>
>
>For the BaseTools change:
>I am not sure whether cleanup should also be done within directory:
>edk2\BaseTools\UserManuals
>
>I will let Liming to decide.
>Sorry for not spotting this earlier.
>
>
>Best Regards,
>Hao Wu
>
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Shenglei Zhang
>> Sent: Friday, November 30, 2018 10:45 AM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu; Wu, Hao A; Gao, Liming
>> Subject: [edk2] [PATCH v3 0/2] Remove DuetPkg and unused tools
>>
>> DuetPkg depends on Legacy BIOS to provide a UEFI environment.
>> It was invented in the era when UEFI environment is hard to find.
>> Since now UEFI is very popular in PC area, we could stop the
>> official support of this package and remove it from the master.
>> And moreover, the tools only used by DuetPkg can also be removed.
>> https://bugzilla.tianocore.org/show_bug.cgi?id=1322
>>
>> The changes are placed in
>> https://github.com/shenglei10/edk2/commits/duet
>>
>> v2:Remove these tools in Makefile and GNUmakefile.
>>
>> v3:Change the commit order.
>>
>> Cc: Ruiyu Ni 
>> Cc: Hao Wu 
>> Cc: Yonghong Zhu 
>> Cc: Liming Gao 
>> Shenglei Zhang (2):
>>   DuetPkg: Remove DuetPkg
>>   BaseTools: Remove tools only used by DuetPkg
>>
>>  BaseTools/Source/BinaryFiles.txt  |4 -
>>  BaseTools/Source/C/BootSectImage/GNUmakefile  |   21 -
>>  BaseTools/Source/C/BootSectImage/Makefile |   22 -
>>  .../Source/C/BootSectImage/bootsectimage.c|  955 --
>>  BaseTools/Source/C/BootSectImage/fat.h|  152 -
>>  BaseTools/Source/C/BootSectImage/mbr.h|   58 -
>>  BaseTools/Source/C/EfiLdrImage/EfiLdrImage.c  |  319 --
>>  BaseTools/Source/C/EfiLdrImage/GNUmakefile|   21 -
>>  BaseTools/Source/C/EfiLdrImage/Makefile   |   22 -
>>  BaseTools/Source/C/GNUmakefile|5 -
>>  BaseTools/Source/C/GenBootSector/FatFormat.h  |  152 -
>>  .../Source/C/GenBootSector/GenBootSector.c|  823 -
>>  .../Source/C/GenBootSector/GetDrvNumOffset.c  |   73 -
>>  BaseTools/Source/C/GenBootSector/Makefile |   22 -
>>  BaseTools/Source/C/GenPage/GNUmakefile|   21 -
>>  BaseTools/Source/C/GenPage/GenPage.c  |  441 ---
>>  BaseTools/Source/C/GenPage/Makefile   |   22 -
>>  BaseTools/Source/C/GenPage/VirtualMemory.h|  122 -
>>  .../Source/C/GnuGenBootSector/FatFormat.h |  152 -
>>  .../Source/C/GnuGenBootSector/GNUmakefile |   21 -
>>  .../C/GnuGenBootSector/GnuGenBootSector.c |  455 ---
>>  BaseTools/Source/C/Makefile   |4 -
>>  BaseTools/toolsetup.bat   |4 -
>>  DuetPkg/AcpiResetDxe/Reset.c  |  212 --
>>  DuetPkg/AcpiResetDxe/Reset.inf|   47 -
>>  DuetPkg/BiosVideoThunkDxe/BiosVideo.c | 2822 -
>>  DuetPkg/BiosVideoThunkDxe/BiosVideo.h |  504 ---
>>  DuetPkg/BiosVideoThunkDxe/BiosVideo.inf   |   50 -
>>  DuetPkg/BiosVideoThunkDxe/ComponentName.c |  166 -
>>  DuetPkg/BiosVideoThunkDxe/LegacyBiosThunk.c   |  220 --
>>  .../BiosVideoThunkDxe/VesaBiosExtensions.h|  457 ---
>>  DuetPkg/BootSector/BootSector.inf |   79 -
>>  DuetPkg/BootSector/FILE.LST   |   39 -
>>  DuetPkg/BootSector/GNUmakefile|  140 -
>>  DuetPkg/BootSector/Gpt.S  |  297 --
>>  DuetPkg/BootSector/Gpt.asm|  294 --
>>  DuetPkg/BootSector/Makefile   |  173 -
>>  DuetPkg/BootSector/Mbr.S  |  262 --
>>  DuetPkg/BootSector/Mbr.asm|  261 --
>>  DuetPkg/BootSector/bin/Gpt.com|  Bin 512 -> 0 bytes
>>  DuetPkg/BootSector/bin/Mbr.com|  Bin 512 -> 0 bytes
>>  DuetPkg/BootSector/bin/Readme.txt |8 -
>>  DuetPkg/BootSector/bin/St16_64.com|  Bin 4096 -> 0 bytes
>>  DuetPkg/BootSector/bin/St32_64.com|  Bin 4096 -> 0 bytes
>>  DuetPkg/BootSector/bin/Start.com  |  Bin 4096 -> 0 bytes
>>  DuetPkg/BootSector/bin/Start16.com|  Bin 4096 -> 0 bytes
>>  DuetPkg/BootSector/bin/Start32.com|  Bin 4096 -> 0 bytes
>>  DuetPkg/BootSector/bin/Start64.com|  Bin 4096 -> 0 bytes
>>  DuetPkg/BootSector/bin/bootsect.com   |  Bin 512 -> 0 bytes
>>  DuetPkg/BootSector/bin/bs16.com   |  Bin 512 -> 0 bytes
>>  DuetPkg/BootSector/bin/bs32.com   |  Bin 512 -> 0 bytes
>>  DuetPkg/BootSector/bin/efi32.com  |  Bin 139264 

Re: [edk2] [Patch] Maintainers.txt: Update BaseTools maintainers

[Feng, Bob C]

Reviewed-by: Bob Feng 

Thanks Yonghong for his great works as Basetools maintainer.

-Original Message-
From: Zhu, Yonghong 
Sent: Thursday, November 29, 2018 4:09 PM
To: edk2-devel@lists.01.org
Cc: Feng, Bob C ; Gao, Liming 
Subject: [Patch] Maintainers.txt: Update BaseTools maintainers

As Yonghong has some other focus, change him from maintainer to reviewer, Bob 
will be the new maintainer.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 Maintainers.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt index 8a9f8df..2031db6 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -82,12 +82,13 @@ M: Laszlo Ersek 
 M: Ard Biesheuvel 
 R: Julien Grall 
 
 BaseTools
 W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
-M: Yonghong Zhu 
+M: Bob Feng 
 M: Liming Gao 
+R: Yonghong Zhu 
 
 BeagleBoardPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/BeagleBoardPkg
 M: Leif Lindholm 
 M: Ard Biesheuvel 
--
2.6.1.windows.1

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

Re: [edk2] [RFC PATCH v3 11/11] CryptoPkg/BaseCryptLib: Hack to get time in MM Standalone mode

[Ye, Ting]

Hi Jagadeesh,

I don't suggest to update BaseCryptLib to use a PCD in StandaloneMmPkg, it 
makes other consumer of BaseCryptLib has dependency of StandaloneMmPkg. 
Also ToDo code below is not acceptable to me.  I am thinking you could consider 
creating a new instance for BaseCryptLib such as "MmCryptLib" if the update to 
TimerWrapp.c is necessary for MM Standalone mode.


Thanks,
Ting

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Jagadeesh Ujja
Sent: Wednesday, November 28, 2018 5:35 PM
To: edk2-devel@lists.01.org; Gao, Liming ; Zhang, Chao B 
; leif.lindh...@linaro.org; ard.biesheu...@linaro.org
Subject: [edk2] [RFC PATCH v3 11/11] CryptoPkg/BaseCryptLib: Hack to get time 
in MM Standalone mode

This is hack to get the time when executing in MM Standalone mode. It is not 
clear how to implement a function that gets the current time. So using this as 
a hack for now.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja 
---
 CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf   |  5 
 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf|  5 
 CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c | 27 +++-
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index c8aafefbab9c..df4aca6c20e2 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -76,6 +76,7 @@ [Sources.AARCH64]
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -86,6 +87,10 @@ [LibraryClasses]
   OpensslLib
   IntrinsicLib
   PrintLib
+  PcdLib
+
+[FeaturePcd]
+  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
 
 #
 # Remove these [BuildOptions] after this library is cleaned up diff --git 
a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf 
b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
index 32628c8835a6..651a6736ba48 100644
--- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -80,6 +80,7 @@ [Sources.AARCH64]
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -91,6 +92,10 @@ [LibraryClasses]
   OpensslLib
   IntrinsicLib
   PrintLib
+  PcdLib
+
+[FeaturePcd]
+  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
 
 #
 # Remove these [BuildOptions] after this library is cleaned up diff --git 
a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c 
b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
index 5f9b0c20d75d..d01b5c5fc113 100644
--- a/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
+++ b/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c
@@ -3,6 +3,7 @@
   for OpenSSL-based Cryptographic Library (used in DXE & RUNTIME).
 
 Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018, ARM Limited. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -77,12 +78,26 @@ 
time_t time (time_t *timer)
   time_t  CalTime;
   UINTN   Year;
 
-  //
-  // Get the current time and date information
-  //
-  Status = gRT->GetTime (, NULL);
-  if (EFI_ERROR (Status) || (Time.Year < 1970)) {
-return 0;
+  if (!PcdGetBool (PcdStandaloneMmEnable)) {
+//
+// Get the current time and date information
+//
+Status = gRT->GetTime (, NULL);
+if (EFI_ERROR (Status) || (Time.Year < 1970)) {
+  return 0;
+}
+  } else {
+//
+//[ToDo] Find out a way to get the current time for code executing as 
MM_STANDALONE
+//
+Time.Year = 2007;
+Time.Month = 11;
+Time.Day = 29;
+Time.Hour = 17;
+Time.Minute = 43;
+Time.Second = 30;
+
+Year  = (UINTN) (Time.Year % 100);
   }
 
   //
--
2.7.4

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

Re: [edk2] [PATCH v3 0/2] Remove DuetPkg and unused tools

[Wu, Hao A]

For the DuetPkg change:
Reviewed-by: Hao Wu 


For the BaseTools change:
I am not sure whether cleanup should also be done within directory:
edk2\BaseTools\UserManuals

I will let Liming to decide.
Sorry for not spotting this earlier.


Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Shenglei Zhang
> Sent: Friday, November 30, 2018 10:45 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Wu, Hao A; Gao, Liming
> Subject: [edk2] [PATCH v3 0/2] Remove DuetPkg and unused tools
> 
> DuetPkg depends on Legacy BIOS to provide a UEFI environment.
> It was invented in the era when UEFI environment is hard to find.
> Since now UEFI is very popular in PC area, we could stop the
> official support of this package and remove it from the master.
> And moreover, the tools only used by DuetPkg can also be removed.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1322
> 
> The changes are placed in
> https://github.com/shenglei10/edk2/commits/duet
> 
> v2:Remove these tools in Makefile and GNUmakefile.
> 
> v3:Change the commit order.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Shenglei Zhang (2):
>   DuetPkg: Remove DuetPkg
>   BaseTools: Remove tools only used by DuetPkg
> 
>  BaseTools/Source/BinaryFiles.txt  |4 -
>  BaseTools/Source/C/BootSectImage/GNUmakefile  |   21 -
>  BaseTools/Source/C/BootSectImage/Makefile |   22 -
>  .../Source/C/BootSectImage/bootsectimage.c|  955 --
>  BaseTools/Source/C/BootSectImage/fat.h|  152 -
>  BaseTools/Source/C/BootSectImage/mbr.h|   58 -
>  BaseTools/Source/C/EfiLdrImage/EfiLdrImage.c  |  319 --
>  BaseTools/Source/C/EfiLdrImage/GNUmakefile|   21 -
>  BaseTools/Source/C/EfiLdrImage/Makefile   |   22 -
>  BaseTools/Source/C/GNUmakefile|5 -
>  BaseTools/Source/C/GenBootSector/FatFormat.h  |  152 -
>  .../Source/C/GenBootSector/GenBootSector.c|  823 -
>  .../Source/C/GenBootSector/GetDrvNumOffset.c  |   73 -
>  BaseTools/Source/C/GenBootSector/Makefile |   22 -
>  BaseTools/Source/C/GenPage/GNUmakefile|   21 -
>  BaseTools/Source/C/GenPage/GenPage.c  |  441 ---
>  BaseTools/Source/C/GenPage/Makefile   |   22 -
>  BaseTools/Source/C/GenPage/VirtualMemory.h|  122 -
>  .../Source/C/GnuGenBootSector/FatFormat.h |  152 -
>  .../Source/C/GnuGenBootSector/GNUmakefile |   21 -
>  .../C/GnuGenBootSector/GnuGenBootSector.c |  455 ---
>  BaseTools/Source/C/Makefile   |4 -
>  BaseTools/toolsetup.bat   |4 -
>  DuetPkg/AcpiResetDxe/Reset.c  |  212 --
>  DuetPkg/AcpiResetDxe/Reset.inf|   47 -
>  DuetPkg/BiosVideoThunkDxe/BiosVideo.c | 2822 -
>  DuetPkg/BiosVideoThunkDxe/BiosVideo.h |  504 ---
>  DuetPkg/BiosVideoThunkDxe/BiosVideo.inf   |   50 -
>  DuetPkg/BiosVideoThunkDxe/ComponentName.c |  166 -
>  DuetPkg/BiosVideoThunkDxe/LegacyBiosThunk.c   |  220 --
>  .../BiosVideoThunkDxe/VesaBiosExtensions.h|  457 ---
>  DuetPkg/BootSector/BootSector.inf |   79 -
>  DuetPkg/BootSector/FILE.LST   |   39 -
>  DuetPkg/BootSector/GNUmakefile|  140 -
>  DuetPkg/BootSector/Gpt.S  |  297 --
>  DuetPkg/BootSector/Gpt.asm|  294 --
>  DuetPkg/BootSector/Makefile   |  173 -
>  DuetPkg/BootSector/Mbr.S  |  262 --
>  DuetPkg/BootSector/Mbr.asm|  261 --
>  DuetPkg/BootSector/bin/Gpt.com|  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/Mbr.com|  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/Readme.txt |8 -
>  DuetPkg/BootSector/bin/St16_64.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/St32_64.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start.com  |  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start16.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start32.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start64.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/bootsect.com   |  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/bs16.com   |  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/bs32.com   |  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/efi32.com  |  Bin 139264 -> 0 bytes
>  DuetPkg/BootSector/bin/efi32.com2 |  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/efi64.com  |  Bin 139264 -> 0 bytes
>  DuetPkg/BootSector/bin/efi64.com2 |  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bootsect.S |  303 --
>  DuetPkg/BootSector/bootsect.asm   |  301 --
>  DuetPkg/BootSector/bs16.S |  291 --
>  DuetPkg/BootSector/bs16.asm   |  288 --
>  DuetPkg/BootSector/bs32.S  

Re: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.

[Wu, Hao A]

Hello,

Please refer to the inline comments below:

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 30, 2018 3:15 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v6 2/2] MdeModulePkg/SdMmcPciHcDxe: Add
> SDMMC HC v4 and above Support.
> 
> Add SDMA, ADMA2 and 26b data length support.
> 
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> SDMA registers supporting 64 bit addresses.
> 
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode and use appropriate
> ADMA descriptors supporting 64 bit addresses.
> 
> If host controller version is above V4.0, enable ADMA2 with 26b data
> length support for better performance. HC 2 register is configured to
> use 26 bit data lengths and ADMA2 descriptors are configured appropriately.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   4 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 328
> ++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  48 ++-
>  3 files changed, 322 insertions(+), 58 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 8c1a589..3af7c95 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,6 +2,7 @@
> 
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
> 
> +Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -150,7 +151,8 @@ typedef struct {
>BOOLEAN Started;
>UINT64  Timeout;
> 
> -  SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
> +  SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> +  SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
>EFI_PHYSICAL_ADDRESSAdmaDescPhy;
>VOID*AdmaMap;
>UINT32  AdmaPages;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 598b6a3..debc3be 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -4,6 +4,7 @@
> 
>It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
> 
> +  Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the
> BSD License
> @@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
>  }
> 
>  /**
> +  Get the controller version information from the specified slot.
> +
> +  @param[in]  PciIo   The PCI IO protocol instance.
> +  @param[in]  SlotThe slot number of the SD card to send the
> command to.
> +  @param[out] Version The buffer to store the version information.
> +
> +  @retval EFI_SUCCESS The operation executes successfully.
> +  @retval Others  The operation fails.
> +
> +**/
> +EFI_STATUS
> +SdMmcHcGetControllerVersion (
> +  IN EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN UINT8Slot,
> + OUT UINT16   *Version
> +  )
> +{
> +  EFI_STATUSStatus;
> +
> +  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE,
> sizeof (UINT16), Version);
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  *Version &= 0xFF;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
>Software reset the specified SD/MMC host controller and enable all
> interrupts.
> 
>@param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA
> instance.
> @@ -776,18 +807,19 @@ SdMmcHcClockSupply (
> 
>DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d
> ClockFreq %dKhz\n", BaseClkFreq, Divisor, ClockFreq));
> 
> -  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE,
> sizeof (ControllerVer), );
> +  Status = SdMmcHcGetControllerVersion (PciIo, Slot, );
>if (EFI_ERROR (Status)) {
>  return Status;
>}
>//
>// Set SDCLK Frequency Select and Internal Clock Enable fields in Clock
> Control register.
>//
> -  if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
> -  ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
> +  if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
> +  (ControllerVer <= 

Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

[Wu, Hao A]

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 30, 2018 2:48 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add
> V4 64bit SDMA and ADMA2 support.
> 
> Hello Hao,
> 
> Answers to your comments:
> 
> 1. Patch 5 fixes the issue with accessing SD as well as eMMC files.

Hello,

The issue still exists on my side.
Please refer to the comments within the V6 2/2 patch.

> 2. Using Private variable for controller version would mean I can only access 
> it
> in functions having an instance of the installed protocol which is not 
> possible
> in some functions for example where we set clock dividers. We either need
> to use what I have or pass Private or controller version as an argument to
> functions using it.

The solution for this is not very complex.

I checked all the callers of SdMmcHcGetControllerVersion() in this patch:

SdMmcHcClockSupply()
SdMmcHcInitV4Enhancements()
BuildAdmaDescTable()
SdMmcExecTrb()
SdMmcCheckTrbResult()

For SdMmcExecTrb() and SdMmcCheckTrbResult(), they already have the access to
'Private'.

For SdMmcHcInitV4Enhancements() and BuildAdmaDescTable(), their callers all
have the access to 'Private'. So we can either:

1. Add 'Private' to the input parameter
2. Add 'ControllerVersion' to the input parameter

for the above 2 functions. Personally, I prefer the 2nd option.

As for SdMmcHcClockSupply(), among its callers, only SdMmcHcInitClockFreq()
does not have direct access to 'Private'. However, SdMmcHcInitClockFreq() is
able to get the ControllerVersion or 'Private' from its caller.

Hence, ultimately, we can call function SdMmcHcGetControllerVersion() only once
at function SdMmcPciHcDriverBindingStart() right after the
SdMmcHcGetMaxCurrent() call. The result can be saved in the 'ControllerVersion'
field of the "SD_MMC_HC_PRIVATE_DATA" structure.

Also, the type of field 'ControllerVersion' in "SD_MMC_HC_PRIVATE_DATA" can be
updated to UINT16 from UINT32.

> 3. I think there is a bigger issue here. During host controller 
> initialization we
> need to setup 64b addressing which happens before we initialize 64b DMA in
> PCI layer. So what you are suggesting may not be that helpful. Also, what we
> are doing right now is that we go through all slots and if controller on any 
> slot
> does not support 64b, we do not enable 64b DMA in PCI layer. What is the
> likelihood of all controllers on an SOC not supporting similar address width?

I am not sure on this one.

All the devices I own seem to only have 1 slot on the controller. And all the
SD host controllers in a system seem to have the same address width support.

But since this driver is a general one, let us assume there will be such a
case here.

> Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller
> supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled
> by default?

I think your handling in the proposed v6 1/2 patch is good.

Since the Pci IO protocol is managing all the slots on one SD controller, when
64-bit DMA is enabled in the PCI layer, there will be potential issues if one
or more slots only support 32-bit system addressing.

But since this driver is a general one, let us assume there will be such a
case that controllers may have different address width here.

> 4. Patch 5 has been rebased on tip of edk2.

Thanks.

Best Regards,
Hao Wu

> 
> Thanks
> Ashish
> 
> -Original Message-
> From: Wu, Hao A 
> Sent: Wednesday, November 28, 2018 12:25 AM
> To: Ashish Singhal ; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4
> 64bit SDMA and ADMA2 support.
> 
> Hello,
> 
> Sorry for the delayed response.
> 
> Apart from inserting the Bugzilla tracker information, several more general
> level
> comments:
> 
> 1. Cannot access the files (content) on SD & eMMC devices
> 
> After applying (rebasing onto the latest codes) your patches, I found that
> though the SD & eMMC devices can be detected, yet the content cannot be
> accessed.
> 
> There are 1 SD card and 1 embedded eMMC device within the system. Under
> Shell, I can see 2 file systems on SD & eMMC devices being detected, but
> when I try to access the files on those file systems by using the 'ls' 
> command,
> it fails, saying "ls: File Not Found - 'FS0:\'". I can confirm that files can 
> be listed
> successfully without applying this patch.
> 
> I also tried the 'dblk' command for display the data via BlockIO protocols
> created on those devices. I found that for SD, the command works fine. But
> for eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".
> 
> For the hardware I own, the controllers are all version 3.0. I tested on both
> IA32 and X64 arch image. The results were the same as described above.
> 
> Unfortunately, I do not have access to boards with SD controller with version
> newer than 3.0. So I 

Re: [edk2] [PATCH v6 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64 bit address capability

[Wu, Hao A]

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 30, 2018 3:15 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v6 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4
> 64 bit address capability
> 
> Add capability declaration for V4.x 64 bit system address support.
> This would be used for host controllers working in version 4. Enable
> 64 bit DMA support in PCI layer if V3 or V4 64 bit support is
> enabled in host capability register.
> 
> The usage of this new field does not need a guard for version check as
> spec for previous SDMMC versions defines this field as reserved with
> default value of 0.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  9 -
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  3 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   | 10 +-
>  3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index a87f8de..2bfd758 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -649,7 +649,14 @@ SdMmcPciHcDriverBindingStart (
>Private->BaseClkFreq[Slot]
>));
> 
> -Support64BitDma &= Private->Capability[Slot].SysBus64;
> +//
> +// If any of the slots does not support 64b system bus
> +// do not enable 64b DMA in the PCI layer.
> +//
> +if (Private->Capability[Slot].SysBus64V3 == 0 &&^M
> +Private->Capability[Slot].SysBus64V4 == 0) {^M
> +  Support64BitDma &= FALSE;^M

The above lines seem not well-formatted at EOL.

With the that addressed:
Reviewed-by: Hao Wu 


Best Regards,
Hao Wu

> +}
> 
>  Status = SdMmcHcGetMaxCurrent (PciIo, Slot, 
> >MaxCurrent[Slot]);
>  if (EFI_ERROR (Status)) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index ddf6dcf..598b6a3 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -45,7 +45,8 @@ DumpCapabilityReg (
>DEBUG ((DEBUG_INFO, "   Voltage 3.3   %a\n", Capability->Voltage33 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, "   Voltage 3.0   %a\n", Capability->Voltage30 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, "   Voltage 1.8   %a\n", Capability->Voltage18 ?
> "TRUE" : "FALSE"));
> -  DEBUG ((DEBUG_INFO, "   64-bit Sys Bus%a\n", Capability->SysBus64 ?
> "TRUE" : "FALSE"));
> +  DEBUG ((DEBUG_INFO, "   V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ?
> "TRUE" : "FALSE"));
> +  DEBUG ((DEBUG_INFO, "   V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, "   Async Interrupt   %a\n", Capability->AsyncInt ?
> "TRUE" : "FALSE"));
>DEBUG ((DEBUG_INFO, "   SlotType  "));
>if (Capability->SlotType == 0x00) {
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index dd45cbd..ad9ce64 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -129,24 +129,24 @@ typedef struct {
>UINT32   Voltage33:1;   // bit 24
>UINT32   Voltage30:1;   // bit 25
>UINT32   Voltage18:1;   // bit 26
> -  UINT32   Reserved3:1;   // bit 27
> -  UINT32   SysBus64:1;// bit 28
> +  UINT32   SysBus64V4:1;  // bit 27
> +  UINT32   SysBus64V3:1;  // bit 28
>UINT32   AsyncInt:1;// bit 29
>UINT32   SlotType:2;// bit 30:31
>UINT32   Sdr50:1;   // bit 32
>UINT32   Sdr104:1;  // bit 33
>UINT32   Ddr50:1;   // bit 34
> -  UINT32   Reserved4:1;   // bit 35
> +  UINT32   Reserved3:1;   // bit 35
>UINT32   DriverTypeA:1; // bit 36
>UINT32   DriverTypeC:1; // bit 37
>UINT32   DriverTypeD:1; // bit 38
>UINT32   DriverType4:1; // bit 39
>UINT32   TimerCount:4;  // bit 40:43
> -  UINT32   Reserved5:1;   // bit 44
> +  UINT32   Reserved4:1;   // bit 44
>UINT32   TuningSDR50:1; // bit 45
>UINT32   RetuningMod:2; // bit 46:47
>UINT32   ClkMultiplier:8;   // bit 48:55
> -  UINT32   Reserved6:7;   // bit 56:62
> +  UINT32   Reserved5:7;   // bit 56:62
>UINT32   Hs400:1;   // bit 63
>  } SD_MMC_HC_SLOT_CAP;
> 
> --
> 2.7.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org