Re: [edk2-devel] [PATCH] RedfishPkg/RedfishPlatformConfigDxe: fix value type issue.

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Nickle Wang 
> Sent: Monday, May 29, 2023 2:24 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> 
> Subject: [PATCH] RedfishPkg/RedfishPlatformConfigDxe: fix value type issue.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Fix incorrect value type issue for checked-box op-code.
> When the variable for checked-box is defined as UINT8 in
> varstore structure, IFR compiler assign its value type to
> EFI_IFR_TYPE_NUM_SIZE_8 instead of EFI_IFR_TYPE_BOOLEAN.
> However, the value type for checked-box is boolean value.
> Redfish service may return error because of incorrect value
> type passed to BIOS attribute registry.
>
> Signed-off-by: Nickle Wang 
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> ---
>  .../RedfishPlatformConfigDxe.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> index 1172d1094b06..462f269f6a3f 100644
> --- a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> +++ b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> @@ -1221,6 +1221,16 @@ HiiValueToRedfishValue (
>RedfishValue->Type = RedfishValueTypeString;
>break;
>  case EFI_IFR_CHECKBOX_OP:
> +  //
> +  // There is case where HII driver defines UINT8 for checked-box opcode
> storage.
> +  // IFR compiler will assign EFI_IFR_TYPE_NUM_SIZE_8 to its value type
> instead of
> +  // EFI_IFR_TYPE_BOOLEAN. We do a patch here and use boolean value
> type for this
> +  // case.
> +  //
> +  if (Value->Type != EFI_IFR_TYPE_BOOLEAN) {
> +Value->Type = EFI_IFR_TYPE_BOOLEAN;
> +  }
> +
>  case EFI_IFR_NUMERIC_OP:
>Status = HiiValueToRedfishNumeric (Value, RedfishValue);
>if (EFI_ERROR (Status)) {
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105361): https://edk2.groups.io/g/devel/message/105361
Mute This Topic: https://groups.io/mt/99195197/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 00/11] Permit DXE drivers to execute in place

[Ard Biesheuvel]

TL;DR - allow DXE drivers to execute in place from the decompressed FV
loaded into memory by DxeIpl so we can apply strict permissions before
dispatching DXE core.

Currently, executable images loaded from firmware volumes are copied at
least three times: once in the firmware volume driver, once in the DXE
core load image code, and finally when the PE sections are populated in
memory based on the section descriptions in the file.

At least two of these copies serve little purpose, given that most
drivers are typically dispatched from a memory-mapped firmware volume
that is loaded into DRAM by DxeIpl from a compressed image in the boot
FV, and so we can take a short-cut in the DXE image loader so that the
PE/COFF library that performs the load uses the image in the memory
mapped FV as its source directly. This is implemented by the first 6
patches (where the first 3 are just cleanups)

With this logic in place, we can go one step further, and actually
dispatch the image in place (similar to how XIP PEIMs are dispatched),
without over moving it out of the decompressed firmware volume. This
requires the image to be aligned sufficiently inside the FV, but this is
also the same logic that applies to XIP PEIMs, and this can be achieved
trivially by tweaking some FDF image generation rules. (Note that this
adds padding to the FV, but this generally compresses well, and we
ultimately uses less memory at runtime by not making a copy of the
image).

This requires the DXE IPL (which is the component that decompresses the
firmware volumes to memory) to iterate over the contents and relocate
these drivers in place. Given that DXE IPL is already in charge of
applying NX permissions to the stack and to other memory regions, we can
trivially extend it to apply restricted permissions to the XIP DXE
drivers after relocation.

This means we enter DXE core with those DXE drivers ready to be
dispatched, removing the need to perform manipulation of memory
attributes before the CPU arch protocol is dispatched, which is a bit of
a catch-22 otherwise.

With these changes in place, the platform no longer needs to map memory
writable and executable by default, and all DRAM can be mapped
non-executable right out of reset.

Cc: Ray Ni 
Cc: Jiewen Yao 
Cc: Gerd Hoffmann 
Cc: Taylor Beebe 
Cc: Oliver Smith-Denny 
Cc: Dandan Bi 
Cc: Liming Gao 
Cc: "Kinney, Michael D" 
Cc: Leif Lindholm 
Cc: Michael Kubacki 

Ard Biesheuvel (11):
  MdeModulePkg/DxeCore: Remove unused 'EntryPoint' argument to LoadImage
  MdeModulePkg/DxeCore: Remove unused DstBuffer arg from LoadImage
  MdeModulePkg/DxeCore: Remove FreePage argument from CoreUnloadImage
  MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files
  MdeModulePkg/DxeCore: Use memory mapped FV protocol to avoid image
copy
  MdeModulePkg/DxeCore: Expose memory mapped FV protocol when possible
  MdeModulePkg/DxeCore: Execute loaded images in place if possible
  MdeModulePkg/DxeIpl: Relocate and remap XIP capable DXE drivers
  MdeModulePkg/DxeCore: Add PCD NX policy bit for default NX state
  ArmVirtPkg/ArmVirtQemu: Allow CPU arch protocol DXE to execute in
place
  ArmVirtPkg/ArmVirtQemu: Map all DRAM non-execute by default

 ArmVirtPkg/ArmVirtQemu.dsc |   1 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc   |  17 +-
 ArmVirtPkg/ArmVirtRules.fdf.inc|   9 +
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c |   4 +-
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf|   2 +-
 MdeModulePkg/Core/Dxe/DxeMain.h|   1 +
 MdeModulePkg/Core/Dxe/DxeMain.inf  |   3 +
 MdeModulePkg/Core/Dxe/FwVol/FwVol.c| 113 ++-
 MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h  |  31 ++
 MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c|  22 --
 MdeModulePkg/Core/Dxe/Image/Image.c| 322 
++--
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c  |   7 +
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf|   1 +
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 196 
 MdeModulePkg/Include/Protocol/MemoryMappedFv.h |  59 
 MdeModulePkg/MdeModulePkg.dec  |   6 +
 16 files changed, 607 insertions(+), 187 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/MemoryMappedFv.h

-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105362): https://edk2.groups.io/g/devel/message/105362
Mute This Topic: https://groups.io/mt/99197132/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 02/11] MdeModulePkg/DxeCore: Remove unused DstBuffer arg from LoadImage

[Ard Biesheuvel]

The DstBuffer and NumberOfPages arguments to CoreLoadImageCommon () are
never set by its only caller CoreLoadImage() so let's drop them from the
prototype.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 174 +++-
 1 file changed, 56 insertions(+), 118 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index 2f2dfe5d0496dc4f..6625d0cd0ff82107 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -559,7 +559,6 @@ CoreIsImageTypeSupported (
   boot selection.
   @param  Pe32Handle  The handle of PE32 image
   @param  Image   PE image to be loaded
-  @param  DstBuffer   The buffer to store the image
   @param  Attribute   The bit mask of attributes to set for the 
load
   PE image
 
@@ -570,17 +569,16 @@ CoreIsImageTypeSupported (
   @retval EFI_BUFFER_TOO_SMALLBuffer for image is too small
 
 **/
+STATIC
 EFI_STATUS
 CoreLoadPeImage (
   IN BOOLEANBootPolicy,
   IN VOID   *Pe32Handle,
   IN LOADED_IMAGE_PRIVATE_DATA  *Image,
-  IN EFI_PHYSICAL_ADDRESS   DstBufferOPTIONAL,
   IN  UINT32Attribute
   )
 {
   EFI_STATUS  Status;
-  BOOLEAN DstBufAlocated;
   UINTN   Size;
 
   ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext));
@@ -633,99 +631,67 @@ CoreLoadPeImage (
   }
 
   //
-  // Allocate memory of the correct memory type aligned on the required image 
boundary
+  // Allocate Destination Buffer as caller did not pass it in
   //
-  DstBufAlocated = FALSE;
-  if (DstBuffer == 0) {
-//
-// Allocate Destination Buffer as caller did not pass it in
-//
 
-if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
-  Size = (UINTN)Image->ImageContext.ImageSize + 
Image->ImageContext.SectionAlignment;
-} else {
-  Size = (UINTN)Image->ImageContext.ImageSize;
-}
+  if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
+Size = (UINTN)Image->ImageContext.ImageSize + 
Image->ImageContext.SectionAlignment;
+  } else {
+Size = (UINTN)Image->ImageContext.ImageSize;
+  }
 
-Image->NumberOfPages = EFI_SIZE_TO_PAGES (Size);
+  Image->NumberOfPages = EFI_SIZE_TO_PAGES (Size);
 
-//
-// If the image relocations have not been stripped, then load at any 
address.
-// Otherwise load at the address at which it was linked.
-//
-// Memory below 1MB should be treated reserved for CSM and there should be
-// no modules whose preferred load addresses are below 1MB.
-//
-Status = EFI_OUT_OF_RESOURCES;
-//
-// If Loading Module At Fixed Address feature is enabled, the module 
should be loaded to
-// a specified address.
-//
-if (PcdGet64 (PcdLoadModuleAtFixAddressEnable) != 0 ) {
-  Status = GetPeCoffImageFixLoadingAssignedAddress 
(&(Image->ImageContext));
-
-  if (EFI_ERROR (Status)) {
-//
-// If the code memory is not ready, invoke CoreAllocatePage with 
AllocateAnyPages to load the driver.
-//
-DEBUG ((DEBUG_INFO|DEBUG_LOAD, "LOADING MODULE FIXED ERROR: Loading 
module at fixed address failed since specified memory is not available.\n"));
-
-Status = CoreAllocatePages (
-   AllocateAnyPages,
-   (EFI_MEMORY_TYPE)(Image->ImageContext.ImageCodeMemoryType),
-   Image->NumberOfPages,
-   &Image->ImageContext.ImageAddress
-   );
-  }
-} else {
-  if ((Image->ImageContext.ImageAddress >= 0x10) || 
Image->ImageContext.RelocationsStripped) {
-Status = CoreAllocatePages (
-   AllocateAddress,
-   (EFI_MEMORY_TYPE)(Image->ImageContext.ImageCodeMemoryType),
-   Image->NumberOfPages,
-   &Image->ImageContext.ImageAddress
-   );
-  }
-
-  if (EFI_ERROR (Status) && !Image->ImageContext.RelocationsStripped) {
-Status = CoreAllocatePages (
-   AllocateAnyPages,
-   (EFI_MEMORY_TYPE)(Image->ImageContext.ImageCodeMemoryType),
-   Image->NumberOfPages,
-   &Image->ImageContext.ImageAddress
-   );
-  }
-}
+  //
+  // If the image relocations have not been stripped, then load at any address.
+  // Otherwise load at the address at which it was linked.
+  //
+  // Memory below 1MB should be treated reserved for CSM and there should be
+  // no modules whose preferred load addresses are below 1MB.
+  //
+  Status = EFI_OUT_OF_RESOURCES;
+  //
+  // If Loading Module At Fixed Address feature is enabled, the module should 
be loaded to
+  // a specified address.
+  //
+  if (PcdGet64 (PcdLoadModuleAtFixAddressEnable) != 0 ) {
+Status = GetPeCoffImageFixLoadingAssignedAddress (&(Image->Image

[edk2-devel] [RFC PATCH 03/11] MdeModulePkg/DxeCore: Remove FreePage argument from CoreUnloadImage

[Ard Biesheuvel]

The FreePage argument to CoreUnloadAndCloseImage () is now always TRUE
so drop it from the prototype. While at it, make the function static as
it is never called from another translation unit.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index 6625d0cd0ff82107..f30e369370a09609 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -888,13 +888,12 @@ CoreLoadedImageInfo (
   Unloads EFI image from memory.
 
   @param  Image   EFI image
-  @param  FreePageFree allocated pages
 
 **/
+STATIC
 VOID
 CoreUnloadAndCloseImage (
-  IN LOADED_IMAGE_PRIVATE_DATA  *Image,
-  IN BOOLEANFreePage
+  IN LOADED_IMAGE_PRIVATE_DATA  *Image
   )
 {
   EFI_STATUS   Status;
@@ -1022,7 +1021,7 @@ CoreUnloadAndCloseImage (
   //
   // Free the Image from memory
   //
-  if ((Image->ImageBasePage != 0) && FreePage) {
+  if (Image->ImageBasePage != 0) {
 CoreFreePages (Image->ImageBasePage, Image->NumberOfPages);
   }
 
@@ -1413,7 +1412,7 @@ CoreLoadImageCommon (
   //
   if (EFI_ERROR (Status)) {
 if (Image != NULL) {
-  CoreUnloadAndCloseImage (Image, TRUE);
+  CoreUnloadAndCloseImage (Image);
   Image = NULL;
 }
   } else if (EFI_ERROR (SecurityStatus)) {
@@ -1711,7 +1710,7 @@ CoreStartImage (
   // unload it
   //
   if (EFI_ERROR (Image->Status) || (Image->Type == 
EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION)) {
-CoreUnloadAndCloseImage (Image, TRUE);
+CoreUnloadAndCloseImage (Image);
 //
 // ImageHandle may be invalid after the image is unloaded, so use NULL 
handle to record perf log.
 //
@@ -1776,7 +1775,7 @@ CoreExit (
 //
 // The image has not been started so just free its resources
 //
-CoreUnloadAndCloseImage (Image, TRUE);
+CoreUnloadAndCloseImage (Image);
 Status = EFI_SUCCESS;
 goto Done;
   }
@@ -1874,7 +1873,7 @@ CoreUnloadImage (
 //
 // if the Image was not started or Unloaded O.K. then clean up
 //
-CoreUnloadAndCloseImage (Image, TRUE);
+CoreUnloadAndCloseImage (Image);
   }
 
 Done:
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105365): https://edk2.groups.io/g/devel/message/105365
Mute This Topic: https://groups.io/mt/99197135/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 01/11] MdeModulePkg/DxeCore: Remove unused 'EntryPoint' argument to LoadImage

[Ard Biesheuvel]

CoreLoadImageCommon's only user passes NULL for its EntryPoint argument,
so it has no purpose and can simply be dropped. While at it, make
CoreLoadImageCommon STATIC to prevent it from being accessed from other
translation units.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index 9dbfb2a1fad22ced..2f2dfe5d0496dc4f 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -560,7 +560,6 @@ CoreIsImageTypeSupported (
   @param  Pe32Handle  The handle of PE32 image
   @param  Image   PE image to be loaded
   @param  DstBuffer   The buffer to store the image
-  @param  EntryPoint  A pointer to the entry point
   @param  Attribute   The bit mask of attributes to set for the 
load
   PE image
 
@@ -577,7 +576,6 @@ CoreLoadPeImage (
   IN VOID   *Pe32Handle,
   IN LOADED_IMAGE_PRIVATE_DATA  *Image,
   IN EFI_PHYSICAL_ADDRESS   DstBufferOPTIONAL,
-  OUT EFI_PHYSICAL_ADDRESS  *EntryPoint  OPTIONAL,
   IN  UINT32Attribute
   )
 {
@@ -810,13 +808,6 @@ CoreLoadPeImage (
 }
   }
 
-  //
-  // Fill in the entry point of the image if it is available
-  //
-  if (EntryPoint != NULL) {
-*EntryPoint = Image->ImageContext.EntryPoint;
-  }
-
   //
   // Print the load address and the PDB file name if it is available
   //
@@ -,7 +1102,6 @@ CoreUnloadAndCloseImage (
   this parameter contains the required number.
   @param  ImageHandle Pointer to the returned image handle that is
   created when the image is successfully 
loaded.
-  @param  EntryPoint  A pointer to the entry point
   @param  Attribute   The bit mask of attributes to set for the 
load
   PE image
 
@@ -1134,6 +1124,7 @@ CoreUnloadAndCloseImage (
   platform policy specifies that the image 
should not be started.
 
 **/
+STATIC
 EFI_STATUS
 CoreLoadImageCommon (
   IN  BOOLEAN   BootPolicy,
@@ -1144,7 +1135,6 @@ CoreLoadImageCommon (
   IN  EFI_PHYSICAL_ADDRESS  DstBuffer   OPTIONAL,
   IN OUT UINTN  *NumberOfPages  OPTIONAL,
   OUT EFI_HANDLE*ImageHandle,
-  OUT EFI_PHYSICAL_ADDRESS  *EntryPoint OPTIONAL,
   IN  UINT32Attribute
   )
 {
@@ -1375,9 +1365,9 @@ CoreLoadImageCommon (
   }
 
   //
-  // Load the image.  If EntryPoint is Null, it will not be set.
+  // Load the image.
   //
-  Status = CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer, EntryPoint, 
Attribute);
+  Status = CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer, Attribute);
   if (EFI_ERROR (Status)) {
 if ((Status == EFI_BUFFER_TOO_SMALL) || (Status == EFI_OUT_OF_RESOURCES)) {
   if (NumberOfPages != NULL) {
@@ -1559,7 +1549,6 @@ CoreLoadImage (
  (EFI_PHYSICAL_ADDRESS)(UINTN)NULL,
  NULL,
  ImageHandle,
- NULL,
  EFI_LOAD_PE_IMAGE_ATTRIBUTE_RUNTIME_REGISTRATION | 
EFI_LOAD_PE_IMAGE_ATTRIBUTE_DEBUG_IMAGE_INFO_TABLE_REGISTRATION
  );
 
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105363): https://edk2.groups.io/g/devel/message/105363
Mute This Topic: https://groups.io/mt/99197133/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files

[Ard Biesheuvel]

If a firmware volume is memory mapped, it means we can access it
contents directly, and so caching serves little purpose beyond
potentially a minor performance improvement. However, given that most
files are read only a single time, and dispatched from a decompressed
firmware volume in DRAM, we can just avoid the redundant caching here.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c | 22 
 1 file changed, 22 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c 
b/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
index 2ff22c93aad48d7e..69df96685d680868 100644
--- a/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
+++ b/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
@@ -284,7 +284,6 @@ FvReadFile (
   UINT8   *SrcPtr;
   EFI_FFS_FILE_HEADER *FfsHeader;
   UINTN   InputBufferSize;
-  UINTN   WholeFileSize;
 
   if (NameGuid == NULL) {
 return EFI_INVALID_PARAMETER;
@@ -316,27 +315,6 @@ FvReadFile (
   // Get a pointer to the header
   //
   FfsHeader = FvDevice->LastKey->FfsHeader;
-  if (FvDevice->IsMemoryMapped) {
-//
-// Memory mapped FV has not been cached, so here is to cache by file.
-//
-if (!FvDevice->LastKey->FileCached) {
-  //
-  // Cache FFS file to memory buffer.
-  //
-  WholeFileSize = IS_FFS_FILE2 (FfsHeader) ? FFS_FILE2_SIZE (FfsHeader) : 
FFS_FILE_SIZE (FfsHeader);
-  FfsHeader = AllocateCopyPool (WholeFileSize, FfsHeader);
-  if (FfsHeader == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Let FfsHeader in FfsFileEntry point to the cached file buffer.
-  //
-  FvDevice->LastKey->FfsHeader  = FfsHeader;
-  FvDevice->LastKey->FileCached = TRUE;
-}
-  }
 
   //
   // Remember callers buffer size
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105366): https://edk2.groups.io/g/devel/message/105366
Mute This Topic: https://groups.io/mt/99197136/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use memory mapped FV protocol to avoid image copy

[Ard Biesheuvel]

Use the memory mapped FV protocol to obtain the existing location in
memory and the size of an image being loaded from a firmware volume.
This removes the need to do a memcopy of the file data.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/DxeMain.h|   1 +
 MdeModulePkg/Core/Dxe/DxeMain.inf  |   3 +
 MdeModulePkg/Core/Dxe/Image/Image.c| 111 +---
 MdeModulePkg/Include/Protocol/MemoryMappedFv.h |  59 +++
 MdeModulePkg/MdeModulePkg.dec  |   3 +
 5 files changed, 163 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 43daa037be441150..a695b457c79b65bb 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -45,6 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 35d5bf0dee6f7f3f..a7175cb364b9b5de 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -153,6 +153,9 @@ [Protocols]
   gEfiLoadedImageDevicePathProtocolGuid ## PRODUCES
   gEfiHiiPackageListProtocolGuid## SOMETIMES_PRODUCES
   gEfiSmmBase2ProtocolGuid  ## SOMETIMES_CONSUMES
+  ## PRODUCES
+  ## CONSUMES
+  gEdkiiMemoryMappedFvProtocolGuid
   gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
 
   # Arch Protocols
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index f30e369370a09609..3dfab4829b3ca17f 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1043,6 +1043,76 @@ CoreUnloadAndCloseImage (
   CoreFreePool (Image);
 }
 
+/**
+  Get the image file data and size directly from a memory mapped FV
+
+  If FilePath is NULL, then NULL is returned.
+  If FileSize is NULL, then NULL is returned.
+  If AuthenticationStatus is NULL, then NULL is returned.
+
+  @param[in]   FvHandle The firmware volume handle
+  @param[in]   FilePath The pointer to the device path of the 
file
+that is abstracted to the file buffer.
+  @param[out]  FileSize The pointer to the size of the 
abstracted
+file buffer.
+  @param[out]  AuthenticationStatus Pointer to the authentication status.
+
+  @retval NULL   FilePath is NULL, or FileSize is NULL, or AuthenticationStatus
+ is NULL, or the file is not memory mapped
+  @retval other  The abstracted file buffer.
+**/
+STATIC
+VOID *
+GetFileFromMemoryMappedFv (
+  IN  EFI_HANDLE  FvHandle,
+  IN  CONST EFI_DEVICE_PATH_PROTOCOL  *FilePath,
+  OUT UINTN   *FileSize,
+  OUT UINT32  *AuthenticationStatus
+  )
+{
+  EDKII_MEMORY_MAPPED_FV_PROTOCOL  *MemMappedFv;
+  CONST EFI_GUID   *NameGuid;
+  EFI_PHYSICAL_ADDRESS Address;
+  EFI_STATUS   Status;
+
+  if ((FilePath == NULL) ||
+  (FileSize == NULL) ||
+  (AuthenticationStatus == NULL))
+  {
+return NULL;
+  }
+
+  NameGuid = EfiGetNameGuidFromFwVolDevicePathNode (
+   (CONST MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)FilePath);
+  if (NameGuid == NULL) {
+return NULL;
+  }
+
+  Status = gBS->HandleProtocol (
+  FvHandle,
+  &gEdkiiMemoryMappedFvProtocolGuid,
+  (VOID **)&MemMappedFv
+  );
+  if (EFI_ERROR (Status)) {
+ASSERT (Status == EFI_UNSUPPORTED);
+return NULL;
+  }
+
+  Status = MemMappedFv->GetLocationAndSize (
+  MemMappedFv,
+  NameGuid,
+  EFI_SECTION_PE32,
+  &Address,
+  FileSize,
+  AuthenticationStatus
+  );
+  if (EFI_ERROR (Status) || (Address > (MAX_ADDRESS - *FileSize))) {
+return NULL;
+  }
+
+  return (VOID *)(UINTN)Address;
+}
+
 /**
   Loads an EFI image into memory and returns a handle to the image.
 
@@ -1164,6 +1234,16 @@ CoreLoadImageCommon (
 Status = CoreLocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid, 
&HandleFilePath, &DeviceHandle);
 if (!EFI_ERROR (Status)) {
   ImageIsFromFv = TRUE;
+
+  //
+  // If possible, use the memory mapped file image directly, rather than 
copying it into a buffer
+  //
+  FHand.Source = GetFileFromMemoryMappedFv (
+   DeviceHandle,
+   HandleFilePath,
+   &FHand.SourceSize,
+   &AuthenticationStatus
+   );
 } else {
   HandleFilePath = FilePath;
   Status = CoreLocateDevicePath 
(&gEfiSimpleFileSystemProtocolGuid, &HandleFilePat

[edk2-devel] [RFC PATCH 06/11] MdeModulePkg/DxeCore: Expose memory mapped FV protocol when possible

[Ard Biesheuvel]

Expose the EDK2 specific memory mapped FV protocol in addition to the
firmware volume protocol defined by PI when the underlying firmware
volume block protocol informs us that the firmware volume is memory
mapped. This permits the image loader to access any FFS files in the
image directly, rather than requiring it to load a cached copy into
memory via the ReadFile() API. This avoids a redundant memcpy().

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/FwVol/FwVol.c   | 113 +++-
 MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h |  31 ++
 2 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c 
b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
index 153bfecafa7772ea..f7f236496686bc36 100644
--- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
@@ -41,7 +41,8 @@ FV_DEVICE  mFvDevice = {
   0,
   0,
   FALSE,
-  FALSE
+  FALSE,
+  { MemoryMappedFvGetLocationAndSize },
 };
 
 //
@@ -676,11 +677,13 @@ NotifyFwVolBlock (
 //
 // Install an New FV protocol on the existing handle
 //
-Status = CoreInstallProtocolInterface (
+Status = CoreInstallMultipleProtocolInterfaces (
&Handle,
&gEfiFirmwareVolume2ProtocolGuid,
-   EFI_NATIVE_INTERFACE,
-   &FvDevice->Fv
+   &FvDevice->Fv,
+   (FvDevice->IsMemoryMapped ? 
&gEdkiiMemoryMappedFvProtocolGuid : NULL),
+   &FvDevice->MemoryMappedFv,
+   NULL
);
 ASSERT_EFI_ERROR (Status);
   } else {
@@ -722,3 +725,105 @@ FwVolDriverInit (
   );
   return EFI_SUCCESS;
 }
+
+/**
+  Get the physical address and size of a file's section in a memory mapped FV
+
+  @param[in]  This  The protocol pointer
+  @param[in]  NameGuid  The name GUID of the file
+  @param[in]  SectionType   The file section from which to retrieve address 
and size
+  @param[out] FileAddress   The physical address of the file
+  @param[out] FileSize  The size of the file
+  @param[out] AuthStatusThe authentication status associated with the file
+
+  @retval EFI_SUCCESSInformation about the file was retrieved 
successfully.
+  @retval EFI_INVALID_PARAMETER  FileAddress was NULL, FileSize was NULL, 
AuthStatus
+ was NULL.
+  @retval EFI_NOT_FOUND  No section of the specified type could be 
located in
+ the specified file.
+
+**/
+EFI_STATUS
+EFIAPI
+MemoryMappedFvGetLocationAndSize (
+  IN  EDKII_MEMORY_MAPPED_FV_PROTOCOL  *This,
+  IN  CONST EFI_GUID   *NameGuid,
+  IN  EFI_SECTION_TYPE SectionType,
+  OUT EFI_PHYSICAL_ADDRESS *FileAddress,
+  OUT UINTN*FileSize,
+  OUT UINT32   *AuthStatus
+  )
+{
+  FV_DEVICE  *FvDevice;
+  EFI_STATUS Status;
+  EFI_FV_FILETYPEFileType;
+  EFI_FV_FILE_ATTRIBUTES FileAttributes;
+  EFI_GUID   FileNameGuid;
+  FFS_FILE_LIST_ENTRY*FfsFileEntry;
+  EFI_COMMON_SECTION_HEADER  *Section;
+  UINTN  FvFileSize;
+  UINTN  SectionLength;
+  UINTN  HeaderLength;
+
+  FvDevice = FV_DEVICE_FROM_MMFV (This);
+  FfsFileEntry = NULL;
+
+  do {
+FileType = 0;
+Status   = FvGetNextFile (
+ &FvDevice->Fv,
+ &FfsFileEntry,
+ &FileType,
+ &FileNameGuid,
+ &FileAttributes,
+ &FvFileSize
+ );
+if (EFI_ERROR (Status)) {
+  return EFI_NOT_FOUND;
+}
+  } while (!CompareGuid (&FileNameGuid, NameGuid));
+
+  //
+  // Skip over file header
+  //
+  if (IS_FFS_FILE2 (FfsFileEntry->FfsHeader)) {
+Section = (VOID *)((UINT8 *)FfsFileEntry->FfsHeader +
+   sizeof (EFI_FFS_FILE_HEADER2));
+  } else {
+Section = (VOID *)((UINT8 *)FfsFileEntry->FfsHeader +
+   sizeof (EFI_FFS_FILE_HEADER));
+  }
+
+  do {
+if (IS_SECTION2 (Section)) {
+  SectionLength = SECTION2_SIZE (Section);
+  HeaderLength  = sizeof (EFI_COMMON_SECTION_HEADER2);
+} else {
+  SectionLength = SECTION_SIZE (Section);
+  HeaderLength  = sizeof (EFI_COMMON_SECTION_HEADER);
+}
+
+if (SectionLength > FvFileSize) {
+  DEBUG ((DEBUG_WARN, "%a: %x > %x\n", __func__, SectionLength, 
FvFileSize));
+  break;
+}
+
+if (Section->Type == SectionType) {
+  *FileAddress = (UINTN)Section + HeaderLength;
+  *FileSize= SectionLength - HeaderLength;
+  *AuthStatus  = FvDevice->AuthenticationStatus;
+
+  return EFI_SUCCESS;
+}
+
+//
+// SectionLength is adjusted it is 4 byte aligned.
+// Go to the next section
+//
+SectionLength = ALIGN_VALUE (SectionLeng

[edk2-devel] [RFC PATCH 07/11] MdeModulePkg/DxeCore: Execute loaded images in place if possible

[Ard Biesheuvel]

In the image loader, check whether an image has already been relocated
to the address from which it is being loaded. This is not something that
can happen by accident, and so we can assume that this means that the
image was intended to be executed in place.

This removes a redundant copy of the image contents, and also permits
the image to be mapped with restricted permissions even before the CPU
arch protocol has been dispatched.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index 3dfab4829b3ca17f..621637e869daf62d 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -573,7 +573,7 @@ STATIC
 EFI_STATUS
 CoreLoadPeImage (
   IN BOOLEANBootPolicy,
-  IN VOID   *Pe32Handle,
+  IN IMAGE_FILE_HANDLE  *Pe32Handle,
   IN LOADED_IMAGE_PRIVATE_DATA  *Image,
   IN  UINT32Attribute
   )
@@ -630,10 +630,16 @@ CoreLoadPeImage (
   return EFI_UNSUPPORTED;
   }
 
+  //
+  // Check whether the loaded image can be executed in place
+  //
+  if (Image->ImageContext.ImageAddress == 
(PHYSICAL_ADDRESS)(UINTN)Pe32Handle->Source) {
+goto ExecuteInPlace;
+  }
+
   //
   // Allocate Destination Buffer as caller did not pass it in
   //
-
   if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
 Size = (UINTN)Image->ImageContext.ImageSize + 
Image->ImageContext.SectionAlignment;
   } else {
@@ -704,6 +710,7 @@ CoreLoadPeImage (
   //
   // Load the image from the file into the allocated memory
   //
+ExecuteInPlace:
   Status = PeCoffLoaderLoadImage (&Image->ImageContext);
   if (EFI_ERROR (Status)) {
 goto Done;
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105369): https://edk2.groups.io/g/devel/message/105369
Mute This Topic: https://groups.io/mt/99197140/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and remap XIP capable DXE drivers

[Ard Biesheuvel]

Before handing over to the DXE core, iterate over all known FFS files
and find the ones that can execute in place. These files are then
relocated in place and mapped with restricted permissions, allowing the
DXE core to dispatch them without the need to perform any manipulation
of the file contents or the page table permissions.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf |   1 +
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c  | 196 
 2 files changed, 197 insertions(+)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index f1990eac77607854..60112100df78b396 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -65,6 +65,7 @@ [LibraryClasses]
   PeimEntryPoint
   DebugLib
   DebugAgentLib
+  PeCoffLib
   PeiServicesTablePointerLib
   PerformanceLib
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c 
b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index 2c19f1a507baf34a..1f20db1faffbd1d2 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -10,6 +10,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include "DxeIpl.h"
 
+#include 
+#include 
+
 //
 // Module Globals used in the DXE to PEI hand off
 // These must be module globals, so the stack can be switched
@@ -228,6 +231,197 @@ ValidateMemoryTypeInfoVariable (
   return TRUE;
 }
 
+/**
+  Support routine for the PE/COFF Loader that reads a buffer from a PE/COFF 
file.
+  The function is used for XIP code to have optimized memory copy.
+
+  @param FileHandle  The handle to the PE/COFF file
+  @param FileOffset  The offset, in bytes, into the file to read
+  @param ReadSizeThe number of bytes to read from the file starting at 
FileOffset
+  @param Buffer  A pointer to the buffer to read the data into.
+
+  @return EFI_SUCCESSReadSize bytes of data were read into Buffer from the
+ PE/COFF file starting at FileOffset
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+PeiImageRead (
+  IN VOID   *FileHandle,
+  IN UINTN  FileOffset,
+  IN UINTN  *ReadSize,
+  OUTVOID   *Buffer
+  )
+{
+  CHAR8  *Destination8;
+  CHAR8  *Source8;
+
+  Destination8 = Buffer;
+  Source8  = (CHAR8 *)((UINTN)FileHandle + FileOffset);
+  if (Destination8 != Source8) {
+CopyMem (Destination8, Source8, *ReadSize);
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+VOID
+RemapImage (
+  IN  EDKII_MEMORY_ATTRIBUTE_PPI  *MemoryPpi,
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
+  )
+{
+  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr;
+  EFI_IMAGE_SECTION_HEADER *Section;
+  PHYSICAL_ADDRESS SectionAddress;
+  EFI_STATUS   Status;
+  UINT64   Permissions;
+  UINTNIndex;
+
+  if (MemoryPpi == NULL) {
+return;
+  }
+
+  Hdr.Union = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((UINT8 
*)ImageContext->Handle +
+  
ImageContext->PeCoffHeaderOffset);
+  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
+
+  Section = (EFI_IMAGE_SECTION_HEADER *)((UINT8 *)Hdr.Union + sizeof (UINT32) +
+ sizeof (EFI_IMAGE_FILE_HEADER) +
+ 
Hdr.Pe32->FileHeader.SizeOfOptionalHeader
+ );
+
+  for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
+SectionAddress = ImageContext->ImageAddress + 
Section[Index].VirtualAddress;
+Permissions= 0;
+
+if ((Section[Index].Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0) {
+  Permissions |= EFI_MEMORY_RO;
+}
+
+if ((Section[Index].Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) {
+  Permissions |= EFI_MEMORY_XP;
+}
+
+Status = MemoryPpi->SetPermissions (
+  MemoryPpi,
+  SectionAddress,
+  Section[Index].Misc.VirtualSize,
+  Permissions,
+  Permissions ^ EFI_MEMORY_RO ^ EFI_MEMORY_XP
+  );
+ASSERT_EFI_ERROR (Status);
+  }
+}
+
+STATIC
+VOID
+RelocateAndRemapDriversInPlace (
+  VOID
+  )
+{
+  EFI_STATUSStatus;
+  UINTN Instance;
+  EFI_PEI_FV_HANDLE VolumeHandle;
+  EFI_PEI_FILE_HANDLE   FileHandle;
+  PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext;
+  UINT32AuthenticationState;
+  EDKII_MEMORY_ATTRIBUTE_PPI*MemoryPpi;
+
+  MemoryPpi = NULL;
+  PeiServicesLocatePpi (&gEdkiiMemoryAttributePpiGuid, 0, NULL, (VOID 
**)&MemoryPpi);
+
+  Instance = 0;
+  do {
+//
+// Traverse all firmware volume instances
+//
+Status = PeiServicesFfsFindNextVolume (Instance, &VolumeHandle);
+if (Status == EFI_NOT_FOUND) {
+  return;
+}
+
+ASS

[edk2-devel] [RFC PATCH 09/11] MdeModulePkg/DxeCore: Add PCD NX policy bit for default NX state

[Ard Biesheuvel]

Introduce a new bit in the NX memory protection policy PCD mask that
specifies that the platform enters DXE with all unused and all non-code
regions mapped with non-execute permissions.

This removes the need to do a pass over all memory regions to update
their NX memory attributes.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 7 +++
 MdeModulePkg/MdeModulePkg.dec | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 7cc829b17402c2bc..983ed450f143d62d 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -861,6 +861,13 @@ InitializeDxeNxMemoryProtectionPolicy (
 ASSERT (StackBase != 0);
   }
 
+  //
+  // If the platform maps all DRAM non-execute by default, we are done here.
+  //
+  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT61) != 0) {
+return;
+  }
+
   DEBUG ((
 DEBUG_INFO,
 "%a: applying strict permissions to active memory regions\n",
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 2d72ac733d82195e..d2bd0cbb40300889 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1416,12 +1416,15 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   #  EfiMemoryMappedIOPortSpace 0x1000
   #  EfiPalCode 0x2000
   #  EfiPersistentMemory0x4000
+  #  Default state  0x2000
   #  OEM Reserved   0x4000
   #  OS Reserved0x8000
   #
   # NOTE: User must NOT set NX protection for EfiLoaderCode / 
EfiBootServicesCode / EfiRuntimeServicesCode. 
   #   User MUST set the same NX protection for EfiBootServicesData and 
EfiConventionalMemory. 
   #
+  # If the platform enters DXE with all unused and non-code regions mapped NX, 
bit 61 should be set.
+  #
   # e.g. 0x7FD5 can be used for all memory except Code. 
   # e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved. 

   #
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105371): https://edk2.groups.io/g/devel/message/105371
Mute This Topic: https://groups.io/mt/99197143/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 10/11] ArmVirtPkg/ArmVirtQemu: Allow CPU arch protocol DXE to execute in place

[Ard Biesheuvel]

Add ArmCpuDxe and its dependencies to the APRIORI DXE section, and use a
rule override to emit the executable images in a way that permits them
to execute in place from the firmware volume. This allows them to be
mapped with the appropriate permissions before dispatching the DXE core.

Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 17 -
 ArmVirtPkg/ArmVirtRules.fdf.inc  |  9 +
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc 
b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 8a063bac04ac287c..24d5c8dd1dc99ca6 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -38,16 +38,23 @@ [FV.FvMain]
 READ_LOCK_CAP  = TRUE
 READ_LOCK_STATUS   = TRUE
 
+  APRIORI DXE {
+INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
+INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+INF EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
+INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+  }
   INF MdeModulePkg/Core/Dxe/DxeMain.inf
-  INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+  INF RuleOverride=DXE_XIP MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
   INF OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf
-  INF EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
+  INF RuleOverride=DXE_XIP EmbeddedPkg/Drivers/FdtClientDxe/FdtClientDxe.inf
   INF OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf
 
   #
   # PI DXE Drivers producing Architectural Protocols (EFI Services)
   #
-  INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+  INF RuleOverride=DXE_XIP ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
   INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
@@ -71,7 +78,7 @@ [FV.FvMain]
   INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
   INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
 
-  INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+  INF RuleOverride=DXE_XIP ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
   INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
   INF OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
   INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
@@ -107,7 +114,7 @@ [FV.FvMain]
   #
   # Bds
   #
-  INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
+  INF RuleOverride=DXE_XIP 
MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
   INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
   INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
   INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
diff --git a/ArmVirtPkg/ArmVirtRules.fdf.inc b/ArmVirtPkg/ArmVirtRules.fdf.inc
index b8ec040d2330deb3..0b9acc6d9031d9cf 100644
--- a/ArmVirtPkg/ArmVirtRules.fdf.inc
+++ b/ArmVirtPkg/ArmVirtRules.fdf.inc
@@ -79,6 +79,15 @@ [Rule.Common.DXE_DRIVER]
 RAW  ASL   Optional   |.aml
   }
 
+[Rule.Common.DXE_DRIVER.DXE_XIP]
+  FILE DRIVER = $(NAMED_GUID) {
+DXE_DEPEXDXE_DEPEX  Optional 
$(INF_OUTPUT)/$(MODULE_NAME).depex
+PE32 PE32 Align = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
+UI   STRING="$(MODULE_NAME)" Optional
+RAW  ACPI  Optional   |.acpi
+RAW  ASL   Optional   |.aml
+  }
+
 [Rule.Common.DXE_RUNTIME_DRIVER]
   FILE DRIVER = $(NAMED_GUID) {
 DXE_DEPEXDXE_DEPEX  Optional 
$(INF_OUTPUT)/$(MODULE_NAME).depex
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105372): https://edk2.groups.io/g/devel/message/105372
Mute This Topic: https://groups.io/mt/99197144/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 11/11] ArmVirtPkg/ArmVirtQemu: Map all DRAM non-execute by default

[Ard Biesheuvel]

Now that both PEI and DXE can deal with memory being mapped non-execute
by default, update the early mapping code to create non-execute mappings
for all of DRAM. While at it, map the NOR flash read-only as well.

Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc | 1 +
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 4 ++--
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf| 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 449e73b9e1329111..7d159f27cfea8790 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -151,6 +151,7 @@ [PcdsFeatureFlag.common]
 [PcdsFixedAtBuild.common]
 !if $(ARCH) == AARCH64
   gArmTokenSpaceGuid.PcdVFPEnabled|1
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xE0007FD5
 !endif
 
   gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c 
b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
index 23bd0fe68ef79d98..1ed5815989594ebd 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
@@ -91,7 +91,7 @@ ArmVirtGetMemoryMap (
   VirtualMemoryTable[0].PhysicalBase = PcdGet64 (PcdSystemMemoryBase);
   VirtualMemoryTable[0].VirtualBase  = VirtualMemoryTable[0].PhysicalBase;
   VirtualMemoryTable[0].Length   = *(UINT64 *)GET_GUID_HOB_DATA 
(MemorySizeHob);
-  VirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+  VirtualMemoryTable[0].Attributes   = 
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_XP;
 
   DEBUG ((
 DEBUG_INFO,
@@ -115,7 +115,7 @@ ArmVirtGetMemoryMap (
   VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFvBaseAddress);
   VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
   VirtualMemoryTable[2].Length   = FixedPcdGet32 (PcdFvSize);
-  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+  VirtualMemoryTable[2].Attributes   = 
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_RO;
 
   // End of Table
   ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf 
b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
index 2039f71a0ebecd5d..6e70bf6eaa245b7a 100644
--- a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
+++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
@@ -56,4 +56,4 @@ [FixedPcd]
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData
 
 [Depex]
-  TRUE
+  gEdkiiMemoryAttributePpiGuid
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105373): https://edk2.groups.io/g/devel/message/105373
Mute This Topic: https://groups.io/mt/99197146/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/3] OvmfPkg: gigabyte page tweaks

[Ard Biesheuvel]

On Thu, 25 May 2023 at 17:48, Ard Biesheuvel  wrote:
>
> On Wed, 17 May 2023 at 12:24, Gerd Hoffmann  wrote:
> >
> >
> >
> > Gerd Hoffmann (3):
> >   OvmfPkg/PlatformInitLib: check PcdUse1GPageTable
> >   OvmfPkg/OvmfPkgIa32X64: enable 1G pages
> >   OvmfPkg/MicrovmX64: enable 1G pages
> >
>
> Acked-by: Ard Biesheuvel 
>


Merged as #4441


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105374): https://edk2.groups.io/g/devel/message/105374
Mute This Topic: https://groups.io/mt/98944976/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/PciHotPlugInitDxe: Do not reserve IO ports by default.

[Ard Biesheuvel]

On Thu, 25 May 2023 at 17:47, Ard Biesheuvel  wrote:
>
> On Tue, 16 May 2023 at 11:48, Gerd Hoffmann  wrote:
> >
> > Flip the default for IO address space reservations for PCI(e) bridges
> > and root ports with hotplug support from TRUE to FALSE.
> >
> > PCI(e) bridges will still get IO address space assigned in case:
> >
> >   (a) Downstream devices actually need IO address space, or
> >   (b) Explicit configuration, using "qemu -device
> >   pcie-root-port,io-reserve=".
> >
> > In case IO address space is exhausted edk2 will stop assigning resources
> > to PCI(e) bridges.  This is not limited to IO resources, the affected
> > bridges will not get any memory resources assigned either.
> >
> > This patch solves this issue by not handing out the scare IO address
> > space, which is in most cases not needed anyway.  Result is a more
> > consistent PCI configuration in virtual machine configurations with many
> > PCie root ports.
> >
> > Signed-off-by: Gerd Hoffmann 
>
> Reviewed-by: Ard Biesheuvel 
>

Merged as #4441

> > ---
> >  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c 
> > b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> > index 6b2b6797b3b6..69903a600981 100644
> > --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> > +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> > @@ -589,7 +589,7 @@ GetResourcePadding (
> >  return EFI_INVALID_PARAMETER;
> >}
> >
> > -  DefaultIo   = TRUE;
> > +  DefaultIo   = FALSE;
> >DefaultMmio = TRUE;
> >DefaultPrefMmio = TRUE;
> >
> > --
> > 2.40.1
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105375): https://edk2.groups.io/g/devel/message/105375
Mute This Topic: https://groups.io/mt/98922836/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg/PlatformPei: drop S3Verification()

[Ard Biesheuvel]

On Wed, 24 May 2023 at 14:29, Ard Biesheuvel  wrote:
>
> On Wed, 24 May 2023 at 14:28, Laszlo Ersek  wrote:
> >
> > On 5/24/23 11:11, Gerd Hoffmann wrote:
> > > Not needed any more, SMM + 64-bit PEI + S3 suspend works now.  While
> > > being at it also remove it from Bhyve (where it is dead code).
> > >
> > > Fixed by commits:
> > >  - 8bd2028f9ac3 ("MdeModulePkg: Supporting S3 in 64bit PEI")
> > >  - 6acf72901a2e ("UefiCpuPkg: Supporting S3 in 64bit PEI")
> > > See also https://bugzilla.tianocore.org/show_bug.cgi?id=4195
> > >
> > > Signed-off-by: Gerd Hoffmann 
> > > Reviewed-by: Ard Biesheuvel 
> > > Reviewed-by: Laszlo Ersek 
> > > Reviewed-by: Ray Ni 
> > > ---
> > >  OvmfPkg/Bhyve/PlatformPei/Platform.c | 29 --
> > >  OvmfPkg/PlatformPei/Platform.c   | 31 
> > >  2 files changed, 60 deletions(-)
> >
> > I disagree with this (v2) update. Bhyve platform code and QEMU platform
> > code should not be fused into a common patch, if there's a way to avoid
> > that. The reviewers for these C files are also different. If we want to
> > modify Bhyve as a courtesy, we can certainly propose a *separate* patch
> > for that.
> >
> > If others approve this patch, I can't block it from going in; since I've
> > been CC'd (and I thought I'd comment this time), this is my view on it.
> >
>
> Thanks Laszlo. I'll split them out when applying.

Merged as #4441


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105376): https://edk2.groups.io/g/devel/message/105376
Mute This Topic: https://groups.io/mt/99104839/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] PATCH [1/1] ArmPkg: Fix GicV2 BaseAddress types

[Ard Biesheuvel]

On Wed, 24 May 2023 at 12:48, Sami Mujawar  wrote:
>
> Hi Neil,
>
> Thank you for this patch.
> These changes look good to me.
>
> Reviewed-by: Sami Mujawar 
>


I cannot apply this. Please resend this (with me on cc) using git send-email


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105377): https://edk2.groups.io/g/devel/message/105377
Mute This Topic: https://groups.io/mt/98831775/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] failed Pr

[Ard Biesheuvel]

Hello all,

Regarding

https://github.com/tianocore/edk2/pull/4442

could someone please explain to me:
- what went wrong,
- where to find the button to push to resubmit this PR to CI

Thanks,
Ard.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105378): https://edk2.groups.io/g/devel/message/105378
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 04/10] OvmfPkg/RiscVVirt: Remove unimplemented NxForStack configuration

[Sunil V L]

On Thu, May 25, 2023 at 04:30:35PM +0200, Ard Biesheuvel wrote:
> The RISC-V version of the DXE IPL does not implement setting the stack
> NX, so before switching to an implementation that will ASSERT() on the
> missing support, drop the PCD setting that enables it.
> 
> Signed-off-by: Ard Biesheuvel 
> ---

Reviewed-by: Sunil V L 

Thanks!
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105379): https://edk2.groups.io/g/devel/message/105379
Mute This Topic: https://groups.io/mt/99131182/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 7/7] OvmfPkg/RiscVVirt: SEC: Add IO memory resource hob for platform devices

[Ard Biesheuvel]

On Sat, 27 May 2023 at 01:18, Tuan Phan  wrote:
>
> Normally, DXE driver would add device resource to GCD before start using.
> But some key resources such as uart, flash base address are being accessing
> directly in some core modules.
>
> Those resources should be populated to HOB in SEC phase so they are
> added to GCD before anyone can access them.
>

Why should these be in the GCD to begin with?

> Signed-off-by: Tuan Phan 
> Reviewed-by: Andrei Warkentin 

> ---
>  OvmfPkg/RiscVVirt/Sec/Platform.c  | 62 +++
>  OvmfPkg/RiscVVirt/Sec/SecMain.inf |  1 +
>  2 files changed, 63 insertions(+)
>
> diff --git a/OvmfPkg/RiscVVirt/Sec/Platform.c 
> b/OvmfPkg/RiscVVirt/Sec/Platform.c
> index 3645c27b0b12..944b82c84a6e 100644
> --- a/OvmfPkg/RiscVVirt/Sec/Platform.c
> +++ b/OvmfPkg/RiscVVirt/Sec/Platform.c
> @@ -21,6 +21,63 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>
> +/**
> +  Build memory map I/O range resource HOB using the
> +  base address and size.
> +
> +  @param  MemoryBase Memory map I/O base.
> +  @param  MemorySize Memory map I/O size.
> +
> +**/
> +STATIC
> +VOID
> +AddIoMemoryBaseSizeHob (
> +  EFI_PHYSICAL_ADDRESS  MemoryBase,
> +  UINT64MemorySize
> +  )
> +{
> +  /* Align to EFI_PAGE_SIZE */
> +  MemorySize = ALIGN_VALUE (MemorySize, EFI_PAGE_SIZE);
> +  BuildResourceDescriptorHob (
> +EFI_RESOURCE_MEMORY_MAPPED_IO,
> +EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> +EFI_RESOURCE_ATTRIBUTE_TESTED,
> +MemoryBase,
> +MemorySize
> +);
> +}
> +
> +/**
> +  Populate IO resources from FDT that not added to GCD by its
> +  driver in the DXE phase.
> +
> +  @param  FdtBase   Fdt base address
> +  @param  CompatibleCompatible string
> +
> +**/
> +STATIC
> +VOID
> +PopulateIoResources (
> +  VOID  *FdtBase,
> +  CONST CHAR8*  Compatible
> +  )
> +{
> +  UINT64  *Reg;
> +  INT32   Node, LenP;
> +
> +  Node = fdt_node_offset_by_compatible (FdtBase, -1, Compatible);
> +  while (Node != -FDT_ERR_NOTFOUND) {
> +Reg = (UINT64 *)fdt_getprop (FdtBase, Node, "reg", &LenP);
> +if (Reg) {
> +  ASSERT (LenP == (2 * sizeof (UINT64)));
> +  AddIoMemoryBaseSizeHob (SwapBytes64 (Reg[0]), SwapBytes64 (Reg[1]));
> +}
> +Node = fdt_node_offset_by_compatible (FdtBase, Node, Compatible);
> +  }
> +}
> +
>  /**
>@retval EFI_SUCCESSThe address of FDT is passed in HOB.
>EFI_UNSUPPORTEDCan't locate FDT.
> @@ -80,5 +137,10 @@ PlatformPeimInitialization (
>
>BuildFvHob (PcdGet32 (PcdOvmfDxeMemFvBase), PcdGet32 
> (PcdOvmfDxeMemFvSize));
>
> +  PopulateIoResources (Base, "ns16550a");
> +  PopulateIoResources (Base, "qemu,fw-cfg-mmio");
> +  PopulateIoResources (Base, "virtio,mmio");
> +  AddIoMemoryBaseSizeHob (PcdGet32 (PcdOvmfFdBaseAddress), PcdGet32 
> (PcdOvmfFirmwareFdSize));
> +
>return EFI_SUCCESS;
>  }
> diff --git a/OvmfPkg/RiscVVirt/Sec/SecMain.inf 
> b/OvmfPkg/RiscVVirt/Sec/SecMain.inf
> index 0e2a5785e8a4..75d5b74b3d3f 100644
> --- a/OvmfPkg/RiscVVirt/Sec/SecMain.inf
> +++ b/OvmfPkg/RiscVVirt/Sec/SecMain.inf
> @@ -62,6 +62,7 @@
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize
>
>  [Guids]
>gFdtHobGuid
> --
> 2.25.1
>
>
>
> 
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#105346): https://edk2.groups.io/g/devel/message/105346
> Mute This Topic: https://groups.io/mt/9916/1131722
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [a...@kernel.org]
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105380): https://edk2.groups.io/g/devel/message/105380
Mute This Topic: https://groups.io/mt/9916/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] failed Pr

[Rebecca Cran]

It looks like the agent or machine running the CI task crashed.

"##[error]We stopped hearing from agent Azure Pipelines 18. Verify the 
agent machine is running and has a healthy network connection. Anything 
that terminates an agent process, starves it for CPU, or blocks its 
network access can cause this error. For more information, see: 
https://go.microsoft.com/fwlink/?linkid=846610";










The only way I've found to make CI run again is to do something to cause 
the commit hash to change, for example by making a change to ReadMe.rst 
then reverting it.



--

Rebecca Cran


On 5/29/23 6:48 AM, Ard Biesheuvel wrote:

Hello all,

Regarding

https://github.com/tianocore/edk2/pull/4442

could someone please explain to me:
- what went wrong,
- where to find the button to push to resubmit this PR to CI

Thanks,
Ard.








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105381): https://edk2.groups.io/g/devel/message/105381
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/2] AMD/AmdMinBoardPkg: Implements PCI hotplug init protocol

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Abdul Lateef Attar 
> Sent: Monday, May 22, 2023 5:29 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) ; Chang,
> Abner 
> Subject: [PATCH v2 1/2] AMD/AmdMinBoardPkg: Implements PCI hotplug init
> protocol
>
> From: Abdul Lateef Attar 
>
> Implements PCI hotplug init protocol.
> Adds resources padding based on PCD values.
>
> Cc: Abner Chang 
>
> Signed-off-by: Abdul Lateef Attar 
> ---
>  .../AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec |  16 +
>  .../AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc |  14 +-
>  .../PciHotPlug/PciHotPlugInit.inf |  39 +++
>  .../PciHotPlug/PciHotPlugInit.c   | 331 ++
>  4 files changed, 399 insertions(+), 1 deletion(-)
>  create mode 100755
> Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
>  create mode 100755
> Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.c
>
> diff --git a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> index e37b02c4cf5a..65ba08545021 100644
> --- a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> +++ b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> @@ -17,3 +17,19 @@ [Defines]
>PACKAGE_GUID   = 44F9D761-9ECB-43DD-A5AC-177E5048701B
>PACKAGE_VERSION= 0.1
>
> +[Guids]
> +  gAmdMinBoardPkgTokenSpaceGuid  = {0xd4d23d79, 0x73bf, 0x460a,
> {0xa1, 0xc7, 0x85, 0xa3, 0xca, 0x71, 0xb9, 0x4c}}
> +
> +[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> +  #
> +  # PCI HotPlug Resource Padding
> +  #
> +  # PCI bus padding, number of bus to reserve, default 2 bus
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus|2|UINT8
> |0x1003
> +  # IO Resource padding in bytes, default 4KB
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIo|0x10
> 00|UINT32|0x1000
> +  # Non-PreFetch Memory padding in bytes, default 1MB
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem|0x0010
> |UINT32|0x1002
> +  # PreFetch Memory padding in bytes, default 2MB
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x002
> 0|UINT32|0x1001
> +
> diff --git a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> index 273cd74f7842..1a8407250c56 100644
> --- a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> +++ b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> @@ -9,7 +9,7 @@
>
>  [Defines]
>DSC_SPECIFICATION   = 1.30
> -  PLATFORM_GUID   = 88F8A9AE-2FA0-4D58-A6F9-05F635C05F4E
> +  PLATFORM_GUID   = 939B559B-269B-4B8F-9637-44DF6575C1E2
>PLATFORM_NAME   = AmdMinBoardPkg
>PLATFORM_VERSION= 0.1
>OUTPUT_DIRECTORY= Build/$(PLATFORM_NAME)
> @@ -25,6 +25,16 @@ [Packages]
>  [LibraryClasses]
>SpcrDeviceLib|AmdMinBoardPkg/Library/SpcrDeviceLib/SpcrDeviceLib.inf
>
> +[LibraryClasses.common]
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +
> BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRe
> pStr.inf
> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> +
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemor
> yAllocationLib.inf
> +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +
> RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
> +
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBoo
> tServicesTableLib.inf
> +
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryP
> oint.inf
> +
>  [LibraryClasses.common.PEIM]
>
> SetCacheMtrrLib|AmdMinBoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
> b.inf
>
> @@ -34,3 +44,5 @@ [Components]
>  [Components.IA32]
>AmdMinBoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
>
> +[Components.X64]
> +  AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> diff --git a/Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> b/Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> new file mode 100755
> index ..44564df38718
> --- /dev/null
> +++ b/Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> @@ -0,0 +1,39 @@
> +## @file
> +# This driver implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL.
> +# Adds resource padding information, for PCIe hotplug purposes.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved
> +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +[Defines]
> +  INF_VERSION= 1.29
> +  BASE_NAME  = PciHotPlugInit
> +  FILE_GUID  = 85F78A6D-6438-4BCC-B796-759A48D00C72
> +  MODULE_TYPE= DXE_DRIVER
> +  VERSION_STRING = 0.1
> +  ENTRY_POINT= PciHotPlugInitialize
> +
> +[Sources]
> +  PciHotPlugInit.c
> +
> +[Packages]
> +  AmdMinBoardPkg/AmdMinBoardPkg.dec
> +  MdeModulePkg/MdeMod

Re: [edk2-devel] [PATCH v2 2/2] AMD/AmdMinBoardkPkg: Implements PeiReportFvLib Library

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang 

> -Original Message-
> From: Abdul Lateef Attar 
> Sent: Monday, May 22, 2023 5:29 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) ; Chang,
> Abner 
> Subject: [PATCH v2 2/2] AMD/AmdMinBoardkPkg: Implements
> PeiReportFvLib Library
>
> Customize PeiReportFvLib library for AMD platforms by
> adding below changes.
>   Installs Advanced Security FV.
>   Adds facility to install FV above 4GB address space.
>
> Cc: Abner Chang 
>
> Signed-off-by: Abdul Lateef Attar 
> ---
>  .../AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec |   8 +
>  .../AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc |   2 +
>  .../Library/PeiReportFvLib/PeiReportFvLib.inf |  57 +
>  .../Library/PeiReportFvLib/PeiReportFvLib.c   | 239 ++
>  4 files changed, 306 insertions(+)
>  create mode 100644
> Platform/AMD/AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.inf
>  create mode 100644
> Platform/AMD/AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.c
>
> diff --git a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> index 65ba08545021..03d1d77c34eb 100644
> --- a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> +++ b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> @@ -33,3 +33,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule,
> PcdsDynamic, PcdsDynamicEx]
># PreFetch Memory padding in bytes, default 2MB
>
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x002
> 0|UINT32|0x1001
>
> +  # PCDs to support loading of FV above 4GB address space
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedBase
> |0x|UINT64|0x1004
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvOsBootBase
> |0x|UINT64|0x1005
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvUefiBootBase
> |0x|UINT64|0x1006
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedSecurityBase
> |0x|UINT64|0x1007
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedSecuritySize
> |0x|UINT32|0x1008
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedSecurityOffset
> |0x|UINT32|0x1009
> +
> diff --git a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> index 1a8407250c56..be33089a45ef 100644
> --- a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> +++ b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> @@ -24,6 +24,7 @@ [Packages]
>
>  [LibraryClasses]
>SpcrDeviceLib|AmdMinBoardPkg/Library/SpcrDeviceLib/SpcrDeviceLib.inf
> +  ReportFvLib|AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.inf
>
>  [LibraryClasses.common]
>BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> @@ -43,6 +44,7 @@ [Components]
>
>  [Components.IA32]
>AmdMinBoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +  AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.inf
>
>  [Components.X64]
>AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> diff --git
> a/Platform/AMD/AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.in
> f
> b/Platform/AMD/AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.i
> nf
> new file mode 100644
> index ..23ee503c42be
> --- /dev/null
> +++
> b/Platform/AMD/AmdMinBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.i
> nf
> @@ -0,0 +1,57 @@
> +### @file
> +# Component information file for the Report Firmware Volume (FV) library.
> +#
> +# Copyright (c) 2018 - 2020, Intel Corporation. All rights reserved.
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +###
> +
> +[Defines]
> +  INF_VERSION= 1.29
> +  BASE_NAME  = PeiReportFvLib
> +  FILE_GUID  = 3C207C28-DC43-4A3A-B572-6794C77AB519
> +  VERSION_STRING = 1.0
> +  MODULE_TYPE= PEIM
> +  LIBRARY_CLASS  = ReportFvLib
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  HobLib
> +  PeiServicesLib
> +
> +[Packages]
> +  AmdMinBoardPkg/AmdMinBoardPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  MinPlatformPkg/MinPlatformPkg.dec
> +
> +[Sources]
> +  PeiReportFvLib.c
> +
> +[Pcd]
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedBase ##
> CONSUMES
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedSecurityBase
> ## CONSUMES
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvAdvancedSecuritySize
> ## CONSUMES
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvOsBootBase   ##
> CONSUMES
> +  gAmdMinBoardPkgTokenSpaceGuid.PcdAmdFlashFvUefiBootBase ##
> CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdBootStage  ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress   ##
> CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize  ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashFvAd

Re: [edk2-devel] failed Pr

[Ard Biesheuvel]

On Mon, 29 May 2023 at 16:37, Rebecca Cran  wrote:
>
> It looks like the agent or machine running the CI task crashed.
>
> "##[error]We stopped hearing from agent Azure Pipelines 18. Verify the
> agent machine is running and has a healthy network connection. Anything
> that terminates an agent process, starves it for CPU, or blocks its
> network access can cause this error. For more information, see:
> https://go.microsoft.com/fwlink/?linkid=846610";
>

Hmm, it would be nice if this thing could distinguish between 'error
in your code' and 'internal error' where the latter does not mark your
PR as being rejected.

>
>
>
>
> The only way I've found to make CI run again is to do something to cause
> the commit hash to change, for example by making a change to ReadMe.rst
> then reverting it.
>

Mike Kinney mentioned last time that there is a button I could push. Mike?

I am reluctant to make unnecessary changes to the state of the branch
just to trick the CI into having another go at it.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105384): https://edk2.groups.io/g/devel/message/105384
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] failed Pr

[Sean]

It has been rerun.


From: devel@edk2.groups.io  on behalf of Ard Biesheuvel 

Sent: Monday, May 29, 2023 7:44:37 AM
To: Rebecca Cran 
Cc: devel@edk2.groups.io ; Michael Kinney 
; Michael Kubacki ; 
Liming Gao (Byosoft address) 
Subject: Re: [edk2-devel] failed Pr

On Mon, 29 May 2023 at 16:37, Rebecca Cran  wrote:
>
> It looks like the agent or machine running the CI task crashed.
>
> "##[error]We stopped hearing from agent Azure Pipelines 18. Verify the
> agent machine is running and has a healthy network connection. Anything
> that terminates an agent process, starves it for CPU, or blocks its
> network access can cause this error. For more information, see:
> https://go.microsoft.com/fwlink/?linkid=846610";
>

Hmm, it would be nice if this thing could distinguish between 'error
in your code' and 'internal error' where the latter does not mark your
PR as being rejected.

>
>
>
>
> The only way I've found to make CI run again is to do something to cause
> the commit hash to change, for example by making a change to ReadMe.rst
> then reverting it.
>

Mike Kinney mentioned last time that there is a button I could push. Mike?

I am reluctant to make unnecessary changes to the state of the branch
just to trick the CI into having another go at it.







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105385): https://edk2.groups.io/g/devel/message/105385
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH edk2-non-osi 1/1] Platform/Qemu/Sbsa: Update TF-A binaries to enable FEAT_FGT

[Marcin Juszkiewicz]

W dniu 16.05.2023 o 12:21, Graeme Gregory pisze:

On Mon, May 15, 2023 at 06:23:30PM +0200, Marcin Juszkiewicz via groups.io 
wrote:

Update the TF-A binaries to have FEAT_FGT support.

This support was merged into TF-A:

https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/19459

This allows SBSA Reference Platform to boot Linux on "max" cpu.


The instructions give a way to do this patch without sending the binary
blob to list.

But this looks like a good idea to me.


EDK2 stable got released. Can this patch get merged into edk2-non-osi? 
Or is there a reason not to?


Would like to be able to build fresh firmware for sbsa-ref and add some 
tests to QEMU.



Reviewed-by: Graeme Gregory 

Signed-off-by: Marcin Juszkiewicz 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105386): https://edk2.groups.io/g/devel/message/105386
Mute This Topic: https://groups.io/mt/98913492/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] failed Pr

[Michael D Kinney]

Hi Sean,

Do you know what GitHub permissions are required to see the re-run button?

I think it is reasonable for all Maintainers to have that available.

Mike

From: devel@edk2.groups.io  On Behalf Of Sean
Sent: Monday, May 29, 2023 8:12 AM
To: devel@edk2.groups.io; a...@kernel.org; Rebecca Cran 
Cc: devel@edk2.groups.io; Kinney, Michael D ; 
Kubacki, Michael ; Gao, Liming 

Subject: Re: [edk2-devel] failed Pr

It has been rerun.


From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> on behalf of Ard Biesheuvel 
mailto:a...@kernel.org>>
Sent: Monday, May 29, 2023 7:44:37 AM
To: Rebecca Cran mailto:rebe...@bsdio.com>>
Cc: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Michael Kinney 
mailto:michael.d.kin...@intel.com>>; Michael 
Kubacki mailto:michael.kuba...@microsoft.com>>; 
Liming Gao (Byosoft address) 
mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] failed Pr

On Mon, 29 May 2023 at 16:37, Rebecca Cran 
mailto:rebe...@bsdio.com>> wrote:
>
> It looks like the agent or machine running the CI task crashed.
>
> "##[error]We stopped hearing from agent Azure Pipelines 18. Verify the
> agent machine is running and has a healthy network connection. Anything
> that terminates an agent process, starves it for CPU, or blocks its
> network access can cause this error. For more information, see:
> https://go.microsoft.com/fwlink/?linkid=846610";
>

Hmm, it would be nice if this thing could distinguish between 'error
in your code' and 'internal error' where the latter does not mark your
PR as being rejected.

>
>
>
>
> The only way I've found to make CI run again is to do something to cause
> the commit hash to change, for example by making a change to ReadMe.rst
> then reverting it.
>

Mike Kinney mentioned last time that there is a button I could push. Mike?

I am reluctant to make unnecessary changes to the state of the branch
just to trick the CI into having another go at it.







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105387): https://edk2.groups.io/g/devel/message/105387
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] failed Pr

[Sean]

It has changed over time and I no longer see a button on the "checks" page of 
GitHub.  I only see the button in the azure pipeline which would mean 
maintainers would need to be "contributors" in the azure DevOps 
organization/project. I have no problem with that and it looks like you can 
sign in with a GitHub account so it should just be a matter of collecting the 
email address associated with their GitHub and inviting them to join.  We only 
get 5 free basic users but we get unlimited free "stakeholder" accounts. I 
assume we might need basic accounts to interact with the pipeline stuff but I 
don't know for sure.  Anyone who has paid Microsoft visual studio subscriptions 
(like from an employer) usually doesn't count towards the 5.

Regardless for those actively contributing and interested we should add them up 
to the limit.  This was one of the reasons we had desired long term to align on 
GitHub build services instead.

Other thoughts?

Thanks
Sean

From: Kinney, Michael D 
Sent: Monday, May 29, 2023 8:13:32 AM
To: devel@edk2.groups.io ; spbro...@outlook.com 
; a...@kernel.org ; Rebecca Cran 

Cc: Kubacki, Michael ; Gao, Liming 
; Kinney, Michael D 
Subject: RE: [edk2-devel] failed Pr


Hi Sean,



Do you know what GitHub permissions are required to see the re-run button?



I think it is reasonable for all Maintainers to have that available.



Mike



From: devel@edk2.groups.io  On Behalf Of Sean
Sent: Monday, May 29, 2023 8:12 AM
To: devel@edk2.groups.io; a...@kernel.org; Rebecca Cran 
Cc: devel@edk2.groups.io; Kinney, Michael D ; 
Kubacki, Michael ; Gao, Liming 

Subject: Re: [edk2-devel] failed Pr



It has been rerun.





From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> on behalf of Ard Biesheuvel 
mailto:a...@kernel.org>>
Sent: Monday, May 29, 2023 7:44:37 AM
To: Rebecca Cran mailto:rebe...@bsdio.com>>
Cc: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Michael Kinney 
mailto:michael.d.kin...@intel.com>>; Michael 
Kubacki mailto:michael.kuba...@microsoft.com>>; 
Liming Gao (Byosoft address) 
mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] failed Pr



On Mon, 29 May 2023 at 16:37, Rebecca Cran 
mailto:rebe...@bsdio.com>> wrote:
>
> It looks like the agent or machine running the CI task crashed.
>
> "##[error]We stopped hearing from agent Azure Pipelines 18. Verify the
> agent machine is running and has a healthy network connection. Anything
> that terminates an agent process, starves it for CPU, or blocks its
> network access can cause this error. For more information, see:
> https://go.microsoft.com/fwlink/?linkid=846610";
>

Hmm, it would be nice if this thing could distinguish between 'error
in your code' and 'internal error' where the latter does not mark your
PR as being rejected.

>
>
>
>
> The only way I've found to make CI run again is to do something to cause
> the commit hash to change, for example by making a change to ReadMe.rst
> then reverting it.
>

Mike Kinney mentioned last time that there is a button I could push. Mike?

I am reluctant to make unnecessary changes to the state of the branch
just to trick the CI into having another go at it.








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105388): https://edk2.groups.io/g/devel/message/105388
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] failed Pr

[Rebecca Cran]

I pay for a Visual Studio subscription, so I shouldn’t count towards the limit 
then.
My GitHub email address is the same as I use here.

— 
Rebecca

On Mon, May 29, 2023, at 9:37 AM, Sean wrote:
> It has changed over time and I no longer see a button on the "checks" 
> page of GitHub.  I only see the button in the azure pipeline which 
> would mean maintainers would need to be "contributors" in the azure 
> DevOps organization/project. I have no problem with that and it looks 
> like you can sign in with a GitHub account so it should just be a 
> matter of collecting the email address associated with their GitHub and 
> inviting them to join.  We only get 5 free basic users but we get 
> unlimited free "stakeholder" accounts. I assume we might need basic 
> accounts to interact with the pipeline stuff but I don't know for sure. 
>  Anyone who has paid Microsoft visual studio subscriptions (like from 
> an employer) usually doesn't count towards the 5.   
>
> Regardless for those actively contributing and interested we should add 
> them up to the limit.  This was one of the reasons we had desired long 
> term to align on GitHub build services instead.
>
> Other thoughts?
>
> Thanks
> Sean 
> *From:* Kinney, Michael D 
> *Sent:* Monday, May 29, 2023 8:13:32 AM
> *To:* devel@edk2.groups.io ; spbro...@outlook.com 
> ; a...@kernel.org ; Rebecca Cran 
> 
> *Cc:* Kubacki, Michael ; Gao, Liming 
> ; Kinney, Michael D 
> 
> *Subject:* RE: [edk2-devel] failed Pr 
> 
> Hi Sean,
>
> 
>
> Do you know what GitHub permissions are required to see the re-run button?
>
> 
>
> I think it is reasonable for all Maintainers to have that available.
>
> 
>
> Mike
>
> 
>
> *From:* devel@edk2.groups.io  * On Behalf Of *Sean
> *Sent:* Monday, May 29, 2023 8:12 AM
> *To:* devel@edk2.groups.io; a...@kernel.org; Rebecca Cran 
> 
> *Cc:* devel@edk2.groups.io; Kinney, Michael D 
> ; Kubacki, Michael 
> ; Gao, Liming 
> *Subject:* Re: [edk2-devel] failed Pr
>
> 
>
> It has been rerun. 
>
> 
>
> *From:* devel@edk2.groups.io  on behalf of Ard 
> Biesheuvel 
> *Sent:* Monday, May 29, 2023 7:44:37 AM
> *To:* Rebecca Cran 
> *Cc:* devel@edk2.groups.io ; Michael Kinney 
> ; Michael Kubacki 
> ; Liming Gao (Byosoft address) 
> 
> *Subject:* Re: [edk2-devel] failed Pr 
>
> 
>
> On Mon, 29 May 2023 at 16:37, Rebecca Cran  wrote:
>>
>> It looks like the agent or machine running the CI task crashed.
>>
>> "##[error]We stopped hearing from agent Azure Pipelines 18. Verify the
>> agent machine is running and has a healthy network connection. Anything
>> that terminates an agent process, starves it for CPU, or blocks its
>> network access can cause this error. For more information, see:
>> https://go.microsoft.com/fwlink/?linkid=846610"; 
>> 
>>
>
> Hmm, it would be nice if this thing could distinguish between 'error
> in your code' and 'internal error' where the latter does not mark your
> PR as being rejected.
>
>>
>>
>>
>>
>> The only way I've found to make CI run again is to do something to cause
>> the commit hash to change, for example by making a change to ReadMe.rst
>> then reverting it.
>>
>
> Mike Kinney mentioned last time that there is a button I could push. Mike?
>
> I am reluctant to make unnecessary changes to the state of the branch
> just to trick the CI into having another go at it.
>
>
>
>
>
>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105389): https://edk2.groups.io/g/devel/message/105389
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] failed Pr

[Michael D Kinney]

Hi Sean,

I see the "rerun" button on the "checks" page of GitHub from my login.

We have a mix of both Azure Pipelines and GitHub Actions today, so I think it 
is important to see the "rerun" from GitHub view for Maintainers.

This github page describes the feature, but not the permissions required.

https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs

We currently have EDK II Maintainer Team set to "Triage Role" permissions.  
From the table below, only "Write" and above are allowed to re-run.

https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/repository-roles-for-an-organization#permissions-for-each-role

"Write" permissions add a lot of options, so I think we should discuss 
tradeoffs in the next Tools/CI meeting.

One option is to add a bot based on a new label to request failed jobs to be 
re-run.  The bot can use a token with permissions to re-run.

Best regards,

Mike

From: Sean Brogan 
Sent: Monday, May 29, 2023 8:37 AM
To: Kinney, Michael D ; devel@edk2.groups.io; 
a...@kernel.org; Rebecca Cran 
Cc: Kubacki, Michael ; Gao, Liming 
; Kinney, Michael D 
Subject: Re: [edk2-devel] failed Pr

It has changed over time and I no longer see a button on the "checks" page of 
GitHub.  I only see the button in the azure pipeline which would mean 
maintainers would need to be "contributors" in the azure DevOps 
organization/project. I have no problem with that and it looks like you can 
sign in with a GitHub account so it should just be a matter of collecting the 
email address associated with their GitHub and inviting them to join.  We only 
get 5 free basic users but we get unlimited free "stakeholder" accounts. I 
assume we might need basic accounts to interact with the pipeline stuff but I 
don't know for sure.  Anyone who has paid Microsoft visual studio subscriptions 
(like from an employer) usually doesn't count towards the 5.

Regardless for those actively contributing and interested we should add them up 
to the limit.  This was one of the reasons we had desired long term to align on 
GitHub build services instead.

Other thoughts?

Thanks
Sean

From: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Sent: Monday, May 29, 2023 8:13:32 AM
To: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; 
spbro...@outlook.com 
mailto:spbro...@outlook.com>>; 
a...@kernel.org 
mailto:a...@kernel.org>>; Rebecca Cran 
mailto:rebe...@bsdio.com>>
Cc: Kubacki, Michael 
mailto:michael.kuba...@microsoft.com>>; Gao, 
Liming mailto:gaolim...@byosoft.com.cn>>; Kinney, 
Michael D mailto:michael.d.kin...@intel.com>>
Subject: RE: [edk2-devel] failed Pr


Hi Sean,



Do you know what GitHub permissions are required to see the re-run button?



I think it is reasonable for all Maintainers to have that available.



Mike



From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Sean
Sent: Monday, May 29, 2023 8:12 AM
To: devel@edk2.groups.io; 
a...@kernel.org; Rebecca Cran 
mailto:rebe...@bsdio.com>>
Cc: devel@edk2.groups.io; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Kubacki, 
Michael mailto:michael.kuba...@microsoft.com>>; 
Gao, Liming mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] failed Pr



It has been rerun.





From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> on behalf of Ard Biesheuvel 
mailto:a...@kernel.org>>
Sent: Monday, May 29, 2023 7:44:37 AM
To: Rebecca Cran mailto:rebe...@bsdio.com>>
Cc: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>>; Michael Kinney 
mailto:michael.d.kin...@intel.com>>; Michael 
Kubacki mailto:michael.kuba...@microsoft.com>>; 
Liming Gao (Byosoft address) 
mailto:gaolim...@byosoft.com.cn>>
Subject: Re: [edk2-devel] failed Pr



On Mon, 29 May 2023 at 16:37, Rebecca Cran 
mailto:rebe...@bsdio.com>> wrote:
>
> It looks like the agent or machine running the CI task crashed.
>
> "##[error]We stopped hearing from agent Azure Pipelines 18. Verify the
> agent machine is running and has a healthy network connection. Anything
> that terminates an agent process, starves it for CPU, or blocks its
> network access can cause this error. For more information, see:
> https://go.microsoft.com/fwlink/?linkid=846610";
>

Hmm, it would be nice if this thing could distinguish between 'error
in your code' and 'internal error' where the latter does not mark your
PR as being rejected.

>
>
>
>
> The only way I've found to make CI run again is to do something to cause
> the commit hash to change, for example by making a change to ReadMe.rst
> then reverting it.
>

Mike Kinney mentioned last time that there is a butto

Re: [edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, May 29, 2023 #cal-reminder

[Rebecca Cran]

Since it’s Memorial Day, I guess this meeting won’t happen?

On Sun, May 28, 2023, at 5:30 PM, Group Notification wrote:
> *Reminder: Tools, CI, Code base construction meeting series* 
>
> *When:*
> Monday, May 29, 2023
> 4:30pm to 5:30pm
> (UTC-07:00) America/Los Angeles 
>
> *Where:*
> https://github.com/tianocore/edk2/discussions/2614 
>
> View Event 
>
> *Description:*
>
> TianoCore community,
>
> Microsoft and Intel will be hosting a series of open meetings to 
> discuss build, CI, tools, and other related topics. If you are 
> interested, have ideas/opinions please join us. These meetings will be 
> Monday 4:30pm Pacific Time on Microsoft Teams.
>
> MS Teams Link in following discussion: * 
> https://github.com/tianocore/edk2/discussions/2614
>
> Anyone is welcome to join.
>
>  • tianocore/edk2: EDK II (github.com)
>  • tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP 
> module (github.com) https://github.com/tianocore/edk2-basetools
>  • tianocore/edk2-pytool-extensions: Extensions to the edk2 build 
> system allowing for a more robust and plugin based build system and 
> tool execution environment (github.com) 
> https://github.com/tianocore/edk2-pytool-extensions
>  • tianocore/edk2-pytool-library: Python library package that supports 
> UEFI development (github.com) 
> https://github.com/tianocore/edk2-pytool-library
> MS Teams Browser Clients * 
> https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client
>
>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105391): https://edk2.groups.io/g/devel/message/105391
Mute This Topic: https://groups.io/mt/99190986/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch 0/2] Address C++ keyword collisions

[Michael D Kinney]

Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
operator and xor in C structures to support use of these
include files when building with a C++ compiler.  

Update SecurityPkg Tpm2CommandLib to use updated field names.

* Change operator -> Operator
* Change xor -> Xor

NOTE: This is a non-backwards compatible change to Tpm12.h
and Tmp20.h. And consumers of these include files that access
the "operator" or "xor" fields must be updated.

Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Oliver Smith-Denny 
Cc: Pedro Falcato 
Cc: Aaron Pop 
Signed-off-by: Michael D Kinney 

Michael D Kinney (2):
  MdePkg/Include/IndustryStandard: Address C++ keyword collisions
  SecurityPkg/Library/TpmCommandLib: Change xor to Xor

 MdePkg/Include/IndustryStandard/Tpm12.h | 4 ++--
 MdePkg/Include/IndustryStandard/Tpm20.h | 4 ++--
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c| 2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105392): https://edk2.groups.io/g/devel/message/105392
Mute This Topic: https://groups.io/mt/99204099/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch 2/2] SecurityPkg/Library/TpmCommandLib: Change xor to Xor

[Michael D Kinney]

Change xor to Xor to avoid C++ reserved work name collisions
when building with C++ compilers.

NOTE: This is a non-backwards compatible change to Tpm12.h
and Tmp20.h. And consumers of these include files that access
the "operator" or "xor" fields must be updated.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Oliver Smith-Denny 
Cc: Pedro Falcato 
Cc: Aaron Pop 
Signed-off-by: Michael D Kinney 
---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c| 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c 
b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
index f0e6019a47be..f8c781a445a1 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
@@ -734,9 +734,9 @@ Tpm2TestParms (
   Buffer += sizeof (UINT16);
   break;
 case TPM_ALG_XOR:
-  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Parameters->parameters.keyedHashDetail.scheme.details.xor.hashAlg));
+  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Parameters->parameters.keyedHashDetail.scheme.details.Xor.hashAlg));
   Buffer += sizeof (UINT16);
-  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Parameters->parameters.keyedHashDetail.scheme.details.xor.kdf));
+  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Parameters->parameters.keyedHashDetail.scheme.details.Xor.kdf));
   Buffer += sizeof (UINT16);
   break;
 default:
@@ -761,7 +761,7 @@ Tpm2TestParms (
   Buffer += sizeof (UINT16);
   break;
 case TPM_ALG_XOR:
-  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Parameters->parameters.symDetail.keyBits.xor));
+  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Parameters->parameters.symDetail.keyBits.Xor));
   Buffer += sizeof (UINT16);
   break;
 case TPM_ALG_NULL:
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c 
b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c
index 335957d6cedc..578a61b20339 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c
@@ -169,9 +169,9 @@ Tpm2ReadPublic (
   Buffer   
   += sizeof (UINT16);
   break;
 case TPM_ALG_XOR:
-  
OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.xor.hashAlg = 
SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
+  
OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.Xor.hashAlg = 
SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
   Buffer   
  += sizeof (UINT16);
-  
OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.xor.kdf = 
SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
+  
OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.Xor.kdf = 
SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
   Buffer   
  += sizeof (UINT16);
   break;
 default:
@@ -195,7 +195,7 @@ Tpm2ReadPublic (
   Buffer+= sizeof 
(UINT16);
   break;
 case TPM_ALG_XOR:
-  OutPublic->publicArea.parameters.symDetail.keyBits.xor = SwapBytes16 
(ReadUnaligned16 ((UINT16 *)Buffer));
+  OutPublic->publicArea.parameters.symDetail.keyBits.Xor = SwapBytes16 
(ReadUnaligned16 ((UINT16 *)Buffer));
   Buffer+= sizeof 
(UINT16);
   break;
 case TPM_ALG_NULL:
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c 
b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c
index 7f247da301fe..e17318b6e6fb 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c
@@ -119,7 +119,7 @@ Tpm2StartAuthSession (
   Buffer += sizeof (UINT16);
   break;
 case TPM_ALG_XOR:
-  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Symmetric->keyBits.xor));
+  WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 
(Symmetric->keyBits.Xor));
   Buffer += sizeof (UINT16);
   break;
 default:
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105394): https://edk2.groups.io/g/devel/message/105394
Mute This Topic: https://groups.io/mt/99204101/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch 1/2] MdePkg/Include/IndustryStandard: Address C++ keyword collisions

[Michael D Kinney]

Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
operator and xor in C structures to support use of these
include files when building with a C++ compiler.

* Change operator -> Operator
* Change xor -> Xor

NOTE: This is a non-backwards compatible change to Tpm12.h
and Tmp20.h. And consumers of these include files that access
the "operator" or "xor" fields must be updated.

Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Oliver Smith-Denny 
Cc: Pedro Falcato 
Cc: Aaron Pop 
Signed-off-by: Michael D Kinney 
---
 MdePkg/Include/IndustryStandard/Tpm12.h | 4 ++--
 MdePkg/Include/IndustryStandard/Tpm20.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h 
b/MdePkg/Include/IndustryStandard/Tpm12.h
index 155dcc9f5f99..147c0863fffd 100644
--- a/MdePkg/Include/IndustryStandard/Tpm12.h
+++ b/MdePkg/Include/IndustryStandard/Tpm12.h
@@ -744,8 +744,8 @@ typedef struct tdTPM_PERMANENT_FLAGS {
   BOOLEAN  TPMpost;
   BOOLEAN  TPMpostLock;
   BOOLEAN  FIPS;
-  BOOLEAN   operator;
-  BOOLEAN   enableRevokeEK;
+  BOOLEAN  Operator;
+  BOOLEAN  enableRevokeEK;
   BOOLEAN  nvLocked;
   BOOLEAN  readSRKPub;
   BOOLEAN  tpmEstablished;
diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h 
b/MdePkg/Include/IndustryStandard/Tpm20.h
index 4440f3769f26..c827af13efd0 100644
--- a/MdePkg/Include/IndustryStandard/Tpm20.h
+++ b/MdePkg/Include/IndustryStandard/Tpm20.h
@@ -1247,7 +1247,7 @@ typedef union {
   TPMI_AES_KEY_BITSaes;
   TPMI_SM4_KEY_BITSSM4;
   TPM_KEY_BITS sym;
-  TPMI_ALG_HASH xor;
+  TPMI_ALG_HASHXor;
 } TPMU_SYM_KEY_BITS;
 
 // Table 123 - TPMU_SYM_MODE Union
@@ -1320,7 +1320,7 @@ typedef struct {
 // Table 136 - TPMU_SCHEME_KEYEDHASH Union
 typedef union {
   TPMS_SCHEME_HMAChmac;
-  TPMS_SCHEME_XOR  xor;
+  TPMS_SCHEME_XOR Xor;
 } TPMU_SCHEME_KEYEDHASH;
 
 // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105393): https://edk2.groups.io/g/devel/message/105393
Mute This Topic: https://groups.io/mt/99204100/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, May 29, 2023 #cal-reminder

[Group Notification]

*Reminder: Tools, CI, Code base construction meeting series*

*When:*
Monday, May 29, 2023
4:30pm to 5:30pm
(UTC-07:00) America/Los Angeles

*Where:*
https://github.com/tianocore/edk2/discussions/2614

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1890536 )

*Description:*

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, 
CI, tools, and other related topics. If you are interested, have ideas/opinions 
please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft 
Teams.

MS Teams Link in following discussion: * 
https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

* tianocore/edk2: EDK II (github.com)
* tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module 
(github.com) https://github.com/tianocore/edk2-basetools
* tianocore/edk2-pytool-extensions: Extensions to the edk2 build system 
allowing for a more robust and plugin based build system and tool execution 
environment (github.com) https://github.com/tianocore/edk2-pytool-extensions
* tianocore/edk2-pytool-library: Python library package that supports UEFI 
development (github.com) https://github.com/tianocore/edk2-pytool-library

MS Teams Browser Clients * 
https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105395): https://edk2.groups.io/g/devel/message/105395
Mute This Topic: https://groups.io/mt/99190986/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1] UefiCpuPkg: Get processor extended information for SmmCpuServiceProtocol

[Zhang, Hongbin1]

Some features like RAS need to use processor extended information
under smm, So add code to support it

Signed-off-by: Hongbin1 Zhang 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Cc: Gerd Hoffmann 
Cc: Star Zeng 
Cc: Jiaxin Wu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index c0e368ea94..8311c3b9de 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -929,7 +929,7 @@ PiCpuSmmEntry (
 gSmmCpuPrivate->Operation[Index]= SmmCpuNone;
 
 if (Index < mNumberOfCpus) {
-  Status = MpServices->GetProcessorInfo (MpServices, Index, 
&gSmmCpuPrivate->ProcessorInfo[Index]);
+  Status = MpServices->GetProcessorInfo (MpServices, Index | 
CPU_V2_EXTENDED_TOPOLOGY, &gSmmCpuPrivate->ProcessorInfo[Index]);
   ASSERT_EFI_ERROR (Status);
   mCpuHotPlugData.ApicId[Index] = 
gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId;
 
-- 
2.37.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105396): https://edk2.groups.io/g/devel/message/105396
Mute This Topic: https://groups.io/mt/99209786/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/2] AMD/AmdMinBoardPkg: Implements PCI hotplug init protocol

[Attar, AbdulLateef (Abdul Lateef) via groups.io]

[AMD Official Use Only - General]

Hi Abner,
Yes, its same patch series with V2 version.
Thanks
AbduL

-Original Message-
From: Chang, Abner 
Sent: Friday, May 26, 2023 7:07 AM
To: Attar, AbdulLateef (Abdul Lateef) ; 
devel@edk2.groups.io
Cc: Attar, AbdulLateef (Abdul Lateef) 
Subject: RE: [PATCH v2 1/2] AMD/AmdMinBoardPkg: Implements PCI hotplug init 
protocol

[AMD Official Use Only - General]

Hi Abdul,
Is this a duplicate patch?
There was a patch you sent:  [edk2-devel] [PATCH 1/1] AMD/AmdMinBoardPkg: 
Implements PCI hotplug init protocol, they both create PciHotPlug.c and 
PciHotPlug.inf.

Thanks
Abner

> -Original Message-
> From: Abdul Lateef Attar 
> Sent: Monday, May 22, 2023 5:29 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) ;
> Chang, Abner 
> Subject: [PATCH v2 1/2] AMD/AmdMinBoardPkg: Implements PCI hotplug
> init protocol
>
> From: Abdul Lateef Attar 
>
> Implements PCI hotplug init protocol.
> Adds resources padding based on PCD values.
>
> Cc: Abner Chang 
>
> Signed-off-by: Abdul Lateef Attar 
> ---
>  .../AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec |  16 +
>  .../AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc |  14 +-
>  .../PciHotPlug/PciHotPlugInit.inf |  39 +++
>  .../PciHotPlug/PciHotPlugInit.c   | 331 ++
>  4 files changed, 399 insertions(+), 1 deletion(-)  create mode 100755
> Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
>  create mode 100755
> Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.c
>
> diff --git a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> index e37b02c4cf5a..65ba08545021 100644
> --- a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> +++ b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dec
> @@ -17,3 +17,19 @@ [Defines]
>PACKAGE_GUID   = 44F9D761-9ECB-43DD-A5AC-177E5048701B
>PACKAGE_VERSION= 0.1
>
> +[Guids]
> +  gAmdMinBoardPkgTokenSpaceGuid  = {0xd4d23d79, 0x73bf, 0x460a,
> {0xa1, 0xc7, 0x85, 0xa3, 0xca, 0x71, 0xb9, 0x4c}}
> +
> +[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> +  #
> +  # PCI HotPlug Resource Padding
> +  #
> +  # PCI bus padding, number of bus to reserve, default 2 bus
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus|2|UINT8
> |0x1003
> +  # IO Resource padding in bytes, default 4KB
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIo|0x10
> 00|UINT32|0x1000
> +  # Non-PreFetch Memory padding in bytes, default 1MB
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem|0x0010
> |UINT32|0x1002
> +  # PreFetch Memory padding in bytes, default 2MB
> +
> gAmdMinBoardPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x002
> 0|UINT32|0x1001
> +
> diff --git a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> index 273cd74f7842..1a8407250c56 100644
> --- a/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> +++ b/Platform/AMD/AmdMinBoardPkg/AmdMinBoardPkg.dsc
> @@ -9,7 +9,7 @@
>
>  [Defines]
>DSC_SPECIFICATION   = 1.30
> -  PLATFORM_GUID   = 88F8A9AE-2FA0-4D58-A6F9-05F635C05F4E
> +  PLATFORM_GUID   = 939B559B-269B-4B8F-9637-44DF6575C1E2
>PLATFORM_NAME   = AmdMinBoardPkg
>PLATFORM_VERSION= 0.1
>OUTPUT_DIRECTORY= Build/$(PLATFORM_NAME)
> @@ -25,6 +25,16 @@ [Packages]
>  [LibraryClasses]
>
> SpcrDeviceLib|AmdMinBoardPkg/Library/SpcrDeviceLib/SpcrDeviceLib.inf
>
> +[LibraryClasses.common]
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +
> BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRe
> pStr.inf
> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> +
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemor
> yAllocationLib.inf
> +  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +
> RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterL
> RegisterFilterLib|ibNull.inf
> +
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiB
> UefiBootServicesTableLib|oo
> tServicesTableLib.inf
> +
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEnt
> UefiDriverEntryPoint|ryP
> oint.inf
> +
>  [LibraryClasses.common.PEIM]
>
> SetCacheMtrrLib|AmdMinBoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLi
> b.inf
>
> @@ -34,3 +44,5 @@ [Components]
>  [Components.IA32]
>AmdMinBoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
>
> +[Components.X64]
> +  AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> diff --git a/Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> b/Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> new file mode 100755
> index ..44564df38718
> --- /dev/null
> +++ b/Platform/AMD/AmdMinBoardPkg/PciHotPlug/PciHotPlugInit.inf
> @@ -0,0 +1,39 @@
> +## @file
> +# This driver implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL.
> +# Adds resour

Re: [edk2-devel] failed Pr

[Attar, AbdulLateef (Abdul Lateef) via groups.io]

[AMD Official Use Only - General]

Emulator PR failed, that might be the reason its not allowing to push
https://github.com/tianocore/edk2/pull/4442/checks?check_run_id=13834435150

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Ard Biesheuvel 
via groups.io
Sent: Monday, May 29, 2023 6:19 PM
To: Michael Kinney ; Michael Kubacki 
; Liming Gao (Byosoft address) 
; edk2-devel-groups-io 
Subject: [edk2-devel] failed Pr

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


Hello all,

Regarding

https://github.com/tianocore/edk2/pull/4442

could someone please explain to me:
- what went wrong,
- where to find the button to push to resubmit this PR to CI

Thanks,
Ard.







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105398): https://edk2.groups.io/g/devel/message/105398
Mute This Topic: https://groups.io/mt/99199003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Now: Tools, CI, Code base construction meeting series - Monday, May 29, 2023 #cal-notice

[Group Notification]

*Tools, CI, Code base construction meeting series*

*When:*
Monday, May 29, 2023
4:30pm to 5:30pm
(UTC-07:00) America/Los Angeles

*Where:*
https://github.com/tianocore/edk2/discussions/2614

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1890536 )

*Description:*

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, 
CI, tools, and other related topics. If you are interested, have ideas/opinions 
please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft 
Teams.

MS Teams Link in following discussion: * 
https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

* tianocore/edk2: EDK II (github.com)
* tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module 
(github.com) https://github.com/tianocore/edk2-basetools
* tianocore/edk2-pytool-extensions: Extensions to the edk2 build system 
allowing for a more robust and plugin based build system and tool execution 
environment (github.com) https://github.com/tianocore/edk2-pytool-extensions
* tianocore/edk2-pytool-library: Python library package that supports UEFI 
development (github.com) https://github.com/tianocore/edk2-pytool-library

MS Teams Browser Clients * 
https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105399): https://edk2.groups.io/g/devel/message/105399
Mute This Topic: https://groups.io/mt/99210163/21656
Mute #cal-notice:https://edk2.groups.io/g/devel/mutehashtag/cal-notice
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, May 30, 2023 #cal-reminder

[Group Notification]

*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, May 30, 2023
6:30pm to 7:30pm
(UTC-07:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d

*Organizer:* Liming Gao gaolim...@byosoft.com.cn ( 
gaolim...@byosoft.com.cn?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1890531 )

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions ( 
https://conf.intel.com/teams/?conf=1160620940&ivr=teams&d=conf.intel.com&test=test_call
 )

*Or call in (audio only)*

+1 916-245-6934,,77463821# ( tel:+19162456934,,77463821# ) United States, 
Sacramento

Phone Conference ID: 774 638 21#

Find a local number ( 
https://dialin.teams.microsoft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821
 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2&messageId=0&language=en-US
 )


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105400): https://edk2.groups.io/g/devel/message/105400
Mute This Topic: https://groups.io/mt/99211840/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN Coverity issue

[Chiu, Chasel]

That’s good suggestion Pedro!
Ranbir, would you like me to modify your patch to "return 0" during merging?

Thanks,
Chasel


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Pedro
> Falcato
> Sent: Friday, May 19, 2023 5:29 AM
> To: devel@edk2.groups.io; rsi...@ventanamicro.com
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star ; Ranbir
> Singh 
> Subject: Re: [edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib:
> Fix OVERRUN Coverity issue
> 
> On Thu, May 18, 2023 at 4:16 PM Ranbir Singh 
> wrote:
> >
> > FspData->PerfIdx is getting increased for every call unconditionally
> > in the function SetFspMeasurePoint and hence memory access can happen
> > for out of bound FspData->PerfData[] array entries also.
> >
> > Example -
> >FspData->PerfData is an array of 32 UINT64 entries. Assume a call
> >is made to SetFspMeasurePoint function when the FspData->PerfIdx
> >last value is 31. It gets incremented to 32 at line 400.
> >Any subsequent call to SetFspMeasurePoint functions leads to
> >FspData->PerfData[32] getting accessed which is out of the PerfData
> >array as well as the FSP_GLOBAL_DATA structure boundary.
> >
> > Hence keep array access and index increment inside if block only and
> > return invalid performance timestamp when PerfIdx is invalid.
> >
> > Cc: Chasel Chiu 
> > Cc: Nate DeSimone 
> > Cc: Star Zeng 
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4200
> > Signed-off-by: Ranbir Singh 
> > Signed-off-by: Ranbir Singh 
> > ---
> >  IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> > b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> > index a22b0e7825ad..cda2a7b2478e 100644
> > --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> > +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> > @@ -377,7 +377,8 @@ GetFspSiliconInitUpdDataPointer (
> >
> >@param[in] Id   Measurement point ID.
> >
> > -  @return performance timestamp.
> > +  @return performance timestamp if current PerfIdx is valid,
> > +  else return 0 as invalid performance timestamp
> >  **/
> >  UINT64
> >  EFIAPI
> > @@ -395,9 +396,10 @@ SetFspMeasurePoint (
> >if (FspData->PerfIdx < sizeof (FspData->PerfData) / sizeof (FspData-
> >PerfData[0])) {
> >  FspData->PerfData[FspData->PerfIdx]  = AsmReadTsc ();
> >  ((UINT8 *)(&FspData->PerfData[FspData->PerfIdx]))[7] = Id;
> > +return FspData->PerfData[(FspData->PerfIdx)++];
> >}
> >
> > -  return FspData->PerfData[(FspData->PerfIdx)++];
> > +  return (UINT64)0x;
> 
> return 0;
> 
> Works just as well. You also don't need a cast.
> 
> https://godbolt.org/z/e5vvGcWWo
> 
> --
> Pedro
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105401): https://edk2.groups.io/g/devel/message/105401
Mute This Topic: https://groups.io/mt/98993286/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] IntelFsp2WrapperPkg: Fix ASSERT when FSP-S/M use FFS3

[Chiu, Chasel]

Patch merged: 
https://github.com/tianocore/edk2/commit/69e10f02111bf7e719237f05233abff4e50016fa

Thanks,
Chasel


> -Original Message-
> From: Tan, Ming 
> Sent: Sunday, May 21, 2023 11:13 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Duggapu, Chinni B
> 
> Subject: [PATCH] IntelFsp2WrapperPkg: Fix ASSERT when FSP-S/M use FFS3
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4458
> 
> Original code call PeiServicesInstallFvInfoPpi() with NULL for the FvFormat
> parameter, then PeiServicesInstallFvInfoPpi() will assume it use FFS2, then
> ASSERT if FSP-S/M use FFS3.
> Now set the FvFormat to the info got from FvHeader.
> 
> Cc: Chasel Chiu 
> Cc: Duggapu Chinni B 
> Signed-off-by: Ming Tan 
> ---
>  IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c | 2 +-
> IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> index ea206a7960..ba0c742fea 100644
> --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
> @@ -217,7 +217,7 @@ FspmWrapperInit (
>  ASSERT_EFI_ERROR (Status);  PeiServicesInstallFvInfoPpi (-  
> NULL,+
> &((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)PcdGet32
> (PcdFspmBaseAddress))->FileSystemGuid,   (VOID *)(UINTN)PcdGet32
> (PcdFspmBaseAddress),   (UINT32)((EFI_FIRMWARE_VOLUME_HEADER
> *)(UINTN)PcdGet32 (PcdFspmBaseAddress))->FvLength,   NULL,diff --git
> a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> index 091ddb697a..08fe0fdb7e 100644
> --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
> @@ -438,7 +438,7 @@ FspsWrapperInitDispatchMode (
>// FSP-S Wrapper running in Dispatch mode and reports FSP-S FV to PEI
> dispatcher.   //   PeiServicesInstallFvInfoPpi (-NULL,+
> &((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)PcdGet32
> (PcdFspsBaseAddress))->FileSystemGuid, (VOID *)(UINTN)PcdGet32
> (PcdFspsBaseAddress), (UINT32)((EFI_FIRMWARE_VOLUME_HEADER
> *)(UINTN)PcdGet32 (PcdFspsBaseAddress))->FvLength, NULL,--
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105402): https://edk2.groups.io/g/devel/message/105402
Mute This Topic: https://groups.io/mt/99059532/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 1/2] MdePkg/Include/IndustryStandard: Address C++ keyword collisions

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Michael D
> Kinney
> Sent: Tuesday, May 30, 2023 1:07 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Liu, Zhiguang
> ; Oliver Smith-Denny ;
> Pedro Falcato ; Pop, Aaron
> 
> Subject: [edk2-devel] [Patch 1/2] MdePkg/Include/IndustryStandard: Address
> C++ keyword collisions
> 
> Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> operator and xor in C structures to support use of these
> include files when building with a C++ compiler.
> 
> * Change operator -> Operator
> * Change xor -> Xor
> 
> NOTE: This is a non-backwards compatible change to Tpm12.h
> and Tmp20.h. And consumers of these include files that access
> the "operator" or "xor" fields must be updated.
> 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Oliver Smith-Denny 
> Cc: Pedro Falcato 
> Cc: Aaron Pop 
> Signed-off-by: Michael D Kinney 
> ---
>  MdePkg/Include/IndustryStandard/Tpm12.h | 4 ++--
>  MdePkg/Include/IndustryStandard/Tpm20.h | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
> b/MdePkg/Include/IndustryStandard/Tpm12.h
> index 155dcc9f5f99..147c0863fffd 100644
> --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> @@ -744,8 +744,8 @@ typedef struct tdTPM_PERMANENT_FLAGS {
>BOOLEAN  TPMpost;
>BOOLEAN  TPMpostLock;
>BOOLEAN  FIPS;
> -  BOOLEAN   operator;
> -  BOOLEAN   enableRevokeEK;
> +  BOOLEAN  Operator;
> +  BOOLEAN  enableRevokeEK;
>BOOLEAN  nvLocked;
>BOOLEAN  readSRKPub;
>BOOLEAN  tpmEstablished;
> diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h
> b/MdePkg/Include/IndustryStandard/Tpm20.h
> index 4440f3769f26..c827af13efd0 100644
> --- a/MdePkg/Include/IndustryStandard/Tpm20.h
> +++ b/MdePkg/Include/IndustryStandard/Tpm20.h
> @@ -1247,7 +1247,7 @@ typedef union {
>TPMI_AES_KEY_BITSaes;
>TPMI_SM4_KEY_BITSSM4;
>TPM_KEY_BITS sym;
> -  TPMI_ALG_HASH xor;
> +  TPMI_ALG_HASHXor;
>  } TPMU_SYM_KEY_BITS;
> 
>  // Table 123 - TPMU_SYM_MODE Union
> @@ -1320,7 +1320,7 @@ typedef struct {
>  // Table 136 - TPMU_SCHEME_KEYEDHASH Union
>  typedef union {
>TPMS_SCHEME_HMAChmac;
> -  TPMS_SCHEME_XOR  xor;
> +  TPMS_SCHEME_XOR Xor;
>  } TPMU_SCHEME_KEYEDHASH;
> 
>  // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
> --
> 2.40.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105403): https://edk2.groups.io/g/devel/message/105403
Mute This Topic: https://groups.io/mt/99204100/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN Coverity issue

[Chiu, Chasel]

Patch merged:
https://github.com/tianocore/edk2/commit/48c53994e649d51a388dc414944c9a9b717a1c3c

Thanks,
Chasel



> -Original Message-
> From: Ranbir Singh 
> Sent: Wednesday, May 17, 2023 11:29 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star ; Ranbir
> Singh 
> Subject: [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN
> Coverity issue
> 
> FspData->PerfIdx is getting increased for every call unconditionally
> in the function SetFspMeasurePoint and hence memory access can happen for
> out of bound FspData->PerfData[] array entries also.
> 
> Example -
>FspData->PerfData is an array of 32 UINT64 entries. Assume a call
>is made to SetFspMeasurePoint function when the FspData->PerfIdx
>last value is 31. It gets incremented to 32 at line 400.
>Any subsequent call to SetFspMeasurePoint functions leads to
>FspData->PerfData[32] getting accessed which is out of the PerfData
>array as well as the FSP_GLOBAL_DATA structure boundary.
> 
> Hence keep array access and index increment inside if block only and return
> invalid performance timestamp when PerfIdx is invalid.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4200
> Signed-off-by: Ranbir Singh 
> Signed-off-by: Ranbir Singh 
> ---
>  IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> index a22b0e7825ad..cda2a7b2478e 100644
> --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c
> @@ -377,7 +377,8 @@ GetFspSiliconInitUpdDataPointer (
> @param[in] Id   Measurement point ID. -  @return performance
> timestamp.+  @return performance timestamp if current PerfIdx is valid,+
> else return 0 as invalid performance timestamp **/ UINT64 EFIAPI@@ -395,9
> +396,10 @@ SetFspMeasurePoint (
>if (FspData->PerfIdx < sizeof (FspData->PerfData) / sizeof (FspData-
> >PerfData[0])) { FspData->PerfData[FspData->PerfIdx]  =
> AsmReadTsc (); ((UINT8 *)(&FspData->PerfData[FspData->PerfIdx]))[7] = Id;+
> return FspData->PerfData[(FspData->PerfIdx)++];   } -  return FspData-
> >PerfData[(FspData->PerfIdx)++];+  return (UINT64)0x; }
> /**--
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105404): https://edk2.groups.io/g/devel/message/105404
Mute This Topic: https://groups.io/mt/98993286/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 01/11] MdeModulePkg/DxeCore: Remove unused 'EntryPoint' argument to LoadImage

[Ni, Ray]

I really like the code simplification! Just 2 comments below:

1. Can you describe the calling flow explicitly in commit message:
  CoreLoadImage (public api) -> CoreLoadImageCommon -> CoreLoadPeImage

2. You added "static" to CoreLoadImageCommon but didn't add to CoreLoadPeImage.
I think you are trying to guarantee no lib implementation that silently calls 
to the
internal functions like CoreLoadImageCommon.
But, why?
Without adding "static", the linking still fails if there is such lib 
implementation because
of the function prototype change. An error such as "parameter mismatch" would 
appear.
And if there is a reason for adding "static", why not add that to 
CoreLoadPeImage as well?

Thanks,
Ray

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 01/11] MdeModulePkg/DxeCore: Remove unused
> 'EntryPoint' argument to LoadImage
> 
> CoreLoadImageCommon's only user passes NULL for its EntryPoint argument,
> so it has no purpose and can simply be dropped. While at it, make
> CoreLoadImageCommon STATIC to prevent it from being accessed from other
> translation units.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/Image/Image.c | 17 +++--
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 9dbfb2a1fad22ced..2f2dfe5d0496dc4f 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -560,7 +560,6 @@ CoreIsImageTypeSupported (
>@param  Pe32Handle  The handle of PE32 image
> 
>@param  Image   PE image to be loaded
> 
>@param  DstBuffer   The buffer to store the image
> 
> -  @param  EntryPoint  A pointer to the entry point
> 
>@param  Attribute   The bit mask of attributes to set for the 
> load
> 
>PE image
> 
> 
> 
> @@ -577,7 +576,6 @@ CoreLoadPeImage (
>IN VOID   *Pe32Handle,
> 
>IN LOADED_IMAGE_PRIVATE_DATA  *Image,
> 
>IN EFI_PHYSICAL_ADDRESS   DstBufferOPTIONAL,
> 
> -  OUT EFI_PHYSICAL_ADDRESS  *EntryPoint  OPTIONAL,
> 
>IN  UINT32Attribute
> 
>)
> 
>  {
> 
> @@ -810,13 +808,6 @@ CoreLoadPeImage (
>  }
> 
>}
> 
> 
> 
> -  //
> 
> -  // Fill in the entry point of the image if it is available
> 
> -  //
> 
> -  if (EntryPoint != NULL) {
> 
> -*EntryPoint = Image->ImageContext.EntryPoint;
> 
> -  }
> 
> -
> 
>//
> 
>// Print the load address and the PDB file name if it is available
> 
>//
> 
> @@ -,7 +1102,6 @@ CoreUnloadAndCloseImage (
>this parameter contains the required 
> number.
> 
>@param  ImageHandle Pointer to the returned image handle that 
> is
> 
>created when the image is successfully 
> loaded.
> 
> -  @param  EntryPoint  A pointer to the entry point
> 
>@param  Attribute   The bit mask of attributes to set for the 
> load
> 
>PE image
> 
> 
> 
> @@ -1134,6 +1124,7 @@ CoreUnloadAndCloseImage (
>platform policy specifies that the image 
> should not be started.
> 
> 
> 
>  **/
> 
> +STATIC
> 
>  EFI_STATUS
> 
>  CoreLoadImageCommon (
> 
>IN  BOOLEAN   BootPolicy,
> 
> @@ -1144,7 +1135,6 @@ CoreLoadImageCommon (
>IN  EFI_PHYSICAL_ADDRESS  DstBuffer   OPTIONAL,
> 
>IN OUT UINTN  *NumberOfPages  OPTIONAL,
> 
>OUT EFI_HANDLE*ImageHandle,
> 
> -  OUT EFI_PHYSICAL_ADDRESS  *EntryPoint OPTIONAL,
> 
>IN  UINT32Attribute
> 
>)
> 
>  {
> 
> @@ -1375,9 +1365,9 @@ CoreLoadImageCommon (
>}
> 
> 
> 
>//
> 
> -  // Load the image.  If EntryPoint is Null, it will not be set.
> 
> +  // Load the image.
> 
>//
> 
> -  Status = CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer, EntryPoint,
> Attribute);
> 
> +  Status = CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer, Attribute);
> 
>if (EFI_ERROR (Status)) {
> 
>  if ((Status == EFI_BUFFER_TOO_SMALL) || (Status ==
> EFI_OUT_OF_RESOURCES)) {
> 
>if (NumberOfPages != NULL) {
> 
> @@ -1559,7 +1549,6 @@ CoreLoadImage (
>   (EFI_PHYSICAL_ADDRESS)(UINTN)NULL,
> 
>   NULL,
> 
>   ImageHandle,
> 
> - NULL,
> 
>   EFI_LOAD_PE_IMAGE_ATTRIBUTE_RUNTIME_REGISTRATION |
> EFI_LOAD_PE_IMAGE_ATTRIBUTE_DEBUG_IMAGE_INFO_TABLE_REGISTRATION
> 
>   );
> 
> 
> 
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: Y

Re: [edk2-devel] [RFC PATCH 02/11] MdeModulePkg/DxeCore: Remove unused DstBuffer arg from LoadImage

[Ni, Ray]

Reviewed-by: Ray Ni 

One comment regarding the needing of "static".
(I saw you added static" to CoreLoadPeImage in this patch😊)
But let's discuss in the first patch thread.

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 02/11] MdeModulePkg/DxeCore: Remove unused DstBuffer
> arg from LoadImage
> 
> The DstBuffer and NumberOfPages arguments to CoreLoadImageCommon () are
> never set by its only caller CoreLoadImage() so let's drop them from the
> prototype.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/Image/Image.c | 174 +++-
>  1 file changed, 56 insertions(+), 118 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 2f2dfe5d0496dc4f..6625d0cd0ff82107 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -559,7 +559,6 @@ CoreIsImageTypeSupported (
>boot selection.
> 
>@param  Pe32Handle  The handle of PE32 image
> 
>@param  Image   PE image to be loaded
> 
> -  @param  DstBuffer   The buffer to store the image
> 
>@param  Attribute   The bit mask of attributes to set for the 
> load
> 
>PE image
> 
> 
> 
> @@ -570,17 +569,16 @@ CoreIsImageTypeSupported (
>@retval EFI_BUFFER_TOO_SMALLBuffer for image is too small
> 
> 
> 
>  **/
> 
> +STATIC
> 
>  EFI_STATUS
> 
>  CoreLoadPeImage (
> 
>IN BOOLEANBootPolicy,
> 
>IN VOID   *Pe32Handle,
> 
>IN LOADED_IMAGE_PRIVATE_DATA  *Image,
> 
> -  IN EFI_PHYSICAL_ADDRESS   DstBufferOPTIONAL,
> 
>IN  UINT32Attribute
> 
>)
> 
>  {
> 
>EFI_STATUS  Status;
> 
> -  BOOLEAN DstBufAlocated;
> 
>UINTN   Size;
> 
> 
> 
>ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext));
> 
> @@ -633,99 +631,67 @@ CoreLoadPeImage (
>}
> 
> 
> 
>//
> 
> -  // Allocate memory of the correct memory type aligned on the required image
> boundary
> 
> +  // Allocate Destination Buffer as caller did not pass it in
> 
>//
> 
> -  DstBufAlocated = FALSE;
> 
> -  if (DstBuffer == 0) {
> 
> -//
> 
> -// Allocate Destination Buffer as caller did not pass it in
> 
> -//
> 
> 
> 
> -if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
> 
> -  Size = (UINTN)Image->ImageContext.ImageSize + Image-
> >ImageContext.SectionAlignment;
> 
> -} else {
> 
> -  Size = (UINTN)Image->ImageContext.ImageSize;
> 
> -}
> 
> +  if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
> 
> +Size = (UINTN)Image->ImageContext.ImageSize + Image-
> >ImageContext.SectionAlignment;
> 
> +  } else {
> 
> +Size = (UINTN)Image->ImageContext.ImageSize;
> 
> +  }
> 
> 
> 
> -Image->NumberOfPages = EFI_SIZE_TO_PAGES (Size);
> 
> +  Image->NumberOfPages = EFI_SIZE_TO_PAGES (Size);
> 
> 
> 
> -//
> 
> -// If the image relocations have not been stripped, then load at any 
> address.
> 
> -// Otherwise load at the address at which it was linked.
> 
> -//
> 
> -// Memory below 1MB should be treated reserved for CSM and there should
> be
> 
> -// no modules whose preferred load addresses are below 1MB.
> 
> -//
> 
> -Status = EFI_OUT_OF_RESOURCES;
> 
> -//
> 
> -// If Loading Module At Fixed Address feature is enabled, the module 
> should be
> loaded to
> 
> -// a specified address.
> 
> -//
> 
> -if (PcdGet64 (PcdLoadModuleAtFixAddressEnable) != 0 ) {
> 
> -  Status = GetPeCoffImageFixLoadingAssignedAddress (&(Image-
> >ImageContext));
> 
> -
> 
> -  if (EFI_ERROR (Status)) {
> 
> -//
> 
> -// If the code memory is not ready, invoke CoreAllocatePage with
> AllocateAnyPages to load the driver.
> 
> -//
> 
> -DEBUG ((DEBUG_INFO|DEBUG_LOAD, "LOADING MODULE FIXED ERROR:
> Loading module at fixed address failed since specified memory is not
> available.\n"));
> 
> -
> 
> -Status = CoreAllocatePages (
> 
> -   AllocateAnyPages,
> 
> -   (EFI_MEMORY_TYPE)(Image-
> >ImageContext.ImageCodeMemoryType),
> 
> -   Image->NumberOfPages,
> 
> -   &Image->ImageContext.ImageAddress
> 
> -   );
> 
> -  }
> 
> -} else {
> 
> -  if ((Image->ImageContext.ImageAddress >= 0x10) || Image-
> >ImageContext.RelocationsStripped) {
> 
> -Status = CoreAllocatePages (
> 
> -   AllocateAddress,
> 
> -   (EFI_MEMORY_TYPE)(Image-
> >ImageContext.ImageCodeMemoryType),
> 
> -   Image->NumberOf

Re: [edk2-devel] [RFC PATCH 03/11] MdeModulePkg/DxeCore: Remove FreePage argument from CoreUnloadImage

[Ni, Ray]

Reviewed-by: Ray Ni 

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 03/11] MdeModulePkg/DxeCore: Remove FreePage
> argument from CoreUnloadImage
> 
> The FreePage argument to CoreUnloadAndCloseImage () is now always TRUE
> so drop it from the prototype. While at it, make the function static as
> it is never called from another translation unit.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/Image/Image.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 6625d0cd0ff82107..f30e369370a09609 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -888,13 +888,12 @@ CoreLoadedImageInfo (
>Unloads EFI image from memory.
> 
> 
> 
>@param  Image   EFI image
> 
> -  @param  FreePageFree allocated pages
> 
> 
> 
>  **/
> 
> +STATIC
> 
>  VOID
> 
>  CoreUnloadAndCloseImage (
> 
> -  IN LOADED_IMAGE_PRIVATE_DATA  *Image,
> 
> -  IN BOOLEANFreePage
> 
> +  IN LOADED_IMAGE_PRIVATE_DATA  *Image
> 
>)
> 
>  {
> 
>EFI_STATUS   Status;
> 
> @@ -1022,7 +1021,7 @@ CoreUnloadAndCloseImage (
>//
> 
>// Free the Image from memory
> 
>//
> 
> -  if ((Image->ImageBasePage != 0) && FreePage) {
> 
> +  if (Image->ImageBasePage != 0) {
> 
>  CoreFreePages (Image->ImageBasePage, Image->NumberOfPages);
> 
>}
> 
> 
> 
> @@ -1413,7 +1412,7 @@ CoreLoadImageCommon (
>//
> 
>if (EFI_ERROR (Status)) {
> 
>  if (Image != NULL) {
> 
> -  CoreUnloadAndCloseImage (Image, TRUE);
> 
> +  CoreUnloadAndCloseImage (Image);
> 
>Image = NULL;
> 
>  }
> 
>} else if (EFI_ERROR (SecurityStatus)) {
> 
> @@ -1711,7 +1710,7 @@ CoreStartImage (
>// unload it
> 
>//
> 
>if (EFI_ERROR (Image->Status) || (Image->Type ==
> EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION)) {
> 
> -CoreUnloadAndCloseImage (Image, TRUE);
> 
> +CoreUnloadAndCloseImage (Image);
> 
>  //
> 
>  // ImageHandle may be invalid after the image is unloaded, so use NULL
> handle to record perf log.
> 
>  //
> 
> @@ -1776,7 +1775,7 @@ CoreExit (
>  //
> 
>  // The image has not been started so just free its resources
> 
>  //
> 
> -CoreUnloadAndCloseImage (Image, TRUE);
> 
> +CoreUnloadAndCloseImage (Image);
> 
>  Status = EFI_SUCCESS;
> 
>  goto Done;
> 
>}
> 
> @@ -1874,7 +1873,7 @@ CoreUnloadImage (
>  //
> 
>  // if the Image was not started or Unloaded O.K. then clean up
> 
>  //
> 
> -CoreUnloadAndCloseImage (Image, TRUE);
> 
> +CoreUnloadAndCloseImage (Image);
> 
>}
> 
> 
> 
>  Done:
> 
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105407): https://edk2.groups.io/g/devel/message/105407
Mute This Topic: https://groups.io/mt/99197135/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory mapped FFS files

[Ni, Ray]

I don't know why the FFS file is cached. Without knowing the reason of caching, 
I cannot say it's good to remove the caching logic.
I will let @Gao, Liming, @Kinney, Michael D to comment.


> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 04/11] MdeModulePkg/DxeCore: Avoid caching memory
> mapped FFS files
> 
> If a firmware volume is memory mapped, it means we can access it
> contents directly, and so caching serves little purpose beyond
> potentially a minor performance improvement. However, given that most
> files are read only a single time, and dispatched from a decompressed
> firmware volume in DRAM, we can just avoid the redundant caching here.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c | 22 
>  1 file changed, 22 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> b/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> index 2ff22c93aad48d7e..69df96685d680868 100644
> --- a/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c
> @@ -284,7 +284,6 @@ FvReadFile (
>UINT8   *SrcPtr;
> 
>EFI_FFS_FILE_HEADER *FfsHeader;
> 
>UINTN   InputBufferSize;
> 
> -  UINTN   WholeFileSize;
> 
> 
> 
>if (NameGuid == NULL) {
> 
>  return EFI_INVALID_PARAMETER;
> 
> @@ -316,27 +315,6 @@ FvReadFile (
>// Get a pointer to the header
> 
>//
> 
>FfsHeader = FvDevice->LastKey->FfsHeader;
> 
> -  if (FvDevice->IsMemoryMapped) {
> 
> -//
> 
> -// Memory mapped FV has not been cached, so here is to cache by file.
> 
> -//
> 
> -if (!FvDevice->LastKey->FileCached) {
> 
> -  //
> 
> -  // Cache FFS file to memory buffer.
> 
> -  //
> 
> -  WholeFileSize = IS_FFS_FILE2 (FfsHeader) ? FFS_FILE2_SIZE (FfsHeader) :
> FFS_FILE_SIZE (FfsHeader);
> 
> -  FfsHeader = AllocateCopyPool (WholeFileSize, FfsHeader);
> 
> -  if (FfsHeader == NULL) {
> 
> -return EFI_OUT_OF_RESOURCES;
> 
> -  }
> 
> -
> 
> -  //
> 
> -  // Let FfsHeader in FfsFileEntry point to the cached file buffer.
> 
> -  //
> 
> -  FvDevice->LastKey->FfsHeader  = FfsHeader;
> 
> -  FvDevice->LastKey->FileCached = TRUE;
> 
> -}
> 
> -  }
> 
> 
> 
>//
> 
>// Remember callers buffer size
> 
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105408): https://edk2.groups.io/g/devel/message/105408
Mute This Topic: https://groups.io/mt/99197136/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use memory mapped FV protocol to avoid image copy

[Ni, Ray]

GetFileBufferByFilePath() always returns a copy of file buffer even when the 
file
is in a memory-mapped device.
So, your patch adds a new implementation (abstracted through the new MM FV 
protocol) that can directly return the file location in the MMIO device.

Several comments:
1. I am not sure if any negative impact due to this change. For example: old 
logic reads the MMIO device but doesn't execute in the MMIO device. Does MMIO 
device always support execution in place?
2. If the MMFV protocol is only produced by DxeCore and consumed by DxeCore, 
can we just implement a local function instead? The challenge might be how to 
pass the FV_DEVICE instance to the local function. Can we "handle" the 
"FirmwareVolume2" protocol from the Handle first and use CR_FROM_THIS() macro 
to retrieve the FV_DEVICE pointer? This helps to not add the MMFV protocol, 
letting the change a pure DxeCore internal thing.


Thanks,
Ray

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use memory mapped FV
> protocol to avoid image copy
> 
> Use the memory mapped FV protocol to obtain the existing location in
> memory and the size of an image being loaded from a firmware volume.
> This removes the need to do a memcopy of the file data.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h|   1 +
>  MdeModulePkg/Core/Dxe/DxeMain.inf  |   3 +
>  MdeModulePkg/Core/Dxe/Image/Image.c| 111 +---
>  MdeModulePkg/Include/Protocol/MemoryMappedFv.h |  59 +++
>  MdeModulePkg/MdeModulePkg.dec  |   3 +
>  5 files changed, 163 insertions(+), 14 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 43daa037be441150..a695b457c79b65bb 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -45,6 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
> 
>  #include 
> 
>  #include 
> 
> +#include 
> 
>  #include 
> 
>  #include 
> 
>  #include 
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 35d5bf0dee6f7f3f..a7175cb364b9b5de 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -153,6 +153,9 @@ [Protocols]
>gEfiLoadedImageDevicePathProtocolGuid ## PRODUCES
> 
>gEfiHiiPackageListProtocolGuid## SOMETIMES_PRODUCES
> 
>gEfiSmmBase2ProtocolGuid  ## SOMETIMES_CONSUMES
> 
> +  ## PRODUCES
> 
> +  ## CONSUMES
> 
> +  gEdkiiMemoryMappedFvProtocolGuid
> 
>gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
> 
> 
> 
># Arch Protocols
> 
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index f30e369370a09609..3dfab4829b3ca17f 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -1043,6 +1043,76 @@ CoreUnloadAndCloseImage (
>CoreFreePool (Image);
> 
>  }
> 
> 
> 
> +/**
> 
> +  Get the image file data and size directly from a memory mapped FV
> 
> +
> 
> +  If FilePath is NULL, then NULL is returned.
> 
> +  If FileSize is NULL, then NULL is returned.
> 
> +  If AuthenticationStatus is NULL, then NULL is returned.
> 
> +
> 
> +  @param[in]   FvHandle The firmware volume handle
> 
> +  @param[in]   FilePath The pointer to the device path of 
> the file
> 
> +that is abstracted to the file 
> buffer.
> 
> +  @param[out]  FileSize The pointer to the size of the 
> abstracted
> 
> +file buffer.
> 
> +  @param[out]  AuthenticationStatus Pointer to the authentication status.
> 
> +
> 
> +  @retval NULL   FilePath is NULL, or FileSize is NULL, or 
> AuthenticationStatus
> 
> + is NULL, or the file is not memory mapped
> 
> +  @retval other  The abstracted file buffer.
> 
> +**/
> 
> +STATIC
> 
> +VOID *
> 
> +GetFileFromMemoryMappedFv (
> 
> +  IN  EFI_HANDLE  FvHandle,
> 
> +  IN  CONST EFI_DEVICE_PATH_PROTOCOL  *FilePath,
> 
> +  OUT UINTN   *FileSize,
> 
> +  OUT UINT32  *AuthenticationStatus
> 
> +  )
> 
> +{
> 
> +  EDKII_MEMORY_MAPPED_FV_PROTOCOL  *MemMappedFv;
> 
> +  CONST EFI_GUID   *NameGuid;
> 
> +  EFI_PHYSICAL_ADDRESS Address;
> 
> +  EFI_STATUS   Status;
> 
> +
> 
> +  if ((FilePath == NULL) ||
> 
> +  (FileSize == NULL) ||
> 
> +  (AuthenticationStatus == NULL))
> 
> +  {
> 
> +return NULL;
> 
> +  }
> 
> +
> 
> +  

Re: [edk2-devel] [RFC PATCH 06/11] MdeModulePkg/DxeCore: Expose memory mapped FV protocol when possible

[Ni, Ray]

As I replied to patch #5, we may avoid installing the MMFV protocol.

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 06/11] MdeModulePkg/DxeCore: Expose memory mapped
> FV protocol when possible
> 
> Expose the EDK2 specific memory mapped FV protocol in addition to the
> firmware volume protocol defined by PI when the underlying firmware
> volume block protocol informs us that the firmware volume is memory
> mapped. This permits the image loader to access any FFS files in the
> image directly, rather than requiring it to load a cached copy into
> memory via the ReadFile() API. This avoids a redundant memcpy().
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/FwVol/FwVol.c   | 113 +++-
>  MdeModulePkg/Core/Dxe/FwVol/FwVolDriver.h |  31 ++
>  2 files changed, 140 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> index 153bfecafa7772ea..f7f236496686bc36 100644
> --- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
> @@ -41,7 +41,8 @@ FV_DEVICE  mFvDevice = {
>0,
> 
>0,
> 
>FALSE,
> 
> -  FALSE
> 
> +  FALSE,
> 
> +  { MemoryMappedFvGetLocationAndSize },
> 
>  };
> 
> 
> 
>  //
> 
> @@ -676,11 +677,13 @@ NotifyFwVolBlock (
>  //
> 
>  // Install an New FV protocol on the existing handle
> 
>  //
> 
> -Status = CoreInstallProtocolInterface (
> 
> +Status = CoreInstallMultipleProtocolInterfaces (
> 
> &Handle,
> 
> &gEfiFirmwareVolume2ProtocolGuid,
> 
> -   EFI_NATIVE_INTERFACE,
> 
> -   &FvDevice->Fv
> 
> +   &FvDevice->Fv,
> 
> +   (FvDevice->IsMemoryMapped ?
> &gEdkiiMemoryMappedFvProtocolGuid : NULL),
> 
> +   &FvDevice->MemoryMappedFv,
> 
> +   NULL
> 
> );
> 
>  ASSERT_EFI_ERROR (Status);
> 
>} else {
> 
> @@ -722,3 +725,105 @@ FwVolDriverInit (
>);
> 
>return EFI_SUCCESS;
> 
>  }
> 
> +
> 
> +/**
> 
> +  Get the physical address and size of a file's section in a memory mapped FV
> 
> +
> 
> +  @param[in]  This  The protocol pointer
> 
> +  @param[in]  NameGuid  The name GUID of the file
> 
> +  @param[in]  SectionType   The file section from which to retrieve address 
> and
> size
> 
> +  @param[out] FileAddress   The physical address of the file
> 
> +  @param[out] FileSize  The size of the file
> 
> +  @param[out] AuthStatusThe authentication status associated with the 
> file
> 
> +
> 
> +  @retval EFI_SUCCESSInformation about the file was retrieved
> successfully.
> 
> +  @retval EFI_INVALID_PARAMETER  FileAddress was NULL, FileSize was NULL,
> AuthStatus
> 
> + was NULL.
> 
> +  @retval EFI_NOT_FOUND  No section of the specified type could be
> located in
> 
> + the specified file.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MemoryMappedFvGetLocationAndSize (
> 
> +  IN  EDKII_MEMORY_MAPPED_FV_PROTOCOL  *This,
> 
> +  IN  CONST EFI_GUID   *NameGuid,
> 
> +  IN  EFI_SECTION_TYPE SectionType,
> 
> +  OUT EFI_PHYSICAL_ADDRESS *FileAddress,
> 
> +  OUT UINTN*FileSize,
> 
> +  OUT UINT32   *AuthStatus
> 
> +  )
> 
> +{
> 
> +  FV_DEVICE  *FvDevice;
> 
> +  EFI_STATUS Status;
> 
> +  EFI_FV_FILETYPEFileType;
> 
> +  EFI_FV_FILE_ATTRIBUTES FileAttributes;
> 
> +  EFI_GUID   FileNameGuid;
> 
> +  FFS_FILE_LIST_ENTRY*FfsFileEntry;
> 
> +  EFI_COMMON_SECTION_HEADER  *Section;
> 
> +  UINTN  FvFileSize;
> 
> +  UINTN  SectionLength;
> 
> +  UINTN  HeaderLength;
> 
> +
> 
> +  FvDevice = FV_DEVICE_FROM_MMFV (This);
> 
> +  FfsFileEntry = NULL;
> 
> +
> 
> +  do {
> 
> +FileType = 0;
> 
> +Status   = FvGetNextFile (
> 
> + &FvDevice->Fv,
> 
> + &FfsFileEntry,
> 
> + &FileType,
> 
> + &FileNameGuid,
> 
> + &FileAttributes,
> 
> + &FvFileSize
> 
> + );
> 
> +if (EFI_ERROR (Status)) {
> 
> +  return EFI_NOT_FOUND;
> 
> +}
> 
> +  } while (!CompareGuid (&FileNameGuid, NameGuid));
> 
> +
> 
> +  //
> 
> +  // Skip over file header
> 
> +  //
> 
> +  if (IS_FFS_FILE2 (FfsFileEntry->FfsHeader)) {
> 
> +Section = (VOID *)((UINT8 *)FfsFileEntry->FfsHeader +
> 
> +

Re: [edk2-devel] [RFC PATCH 07/11] MdeModulePkg/DxeCore: Execute loaded images in place if possible

[Ni, Ray]

I didn't review the existing logic carefully. Several comments:
1. Assignments of several fields are skipped when executing in place. Do they 
matter?
a). Image->NumberOfPages
b). Image->ImageBasePage

2. PeCoffLoaderRelocateImage() is called even for XIP case. But I don't think 
it's expected that relocation really happens. Even if the fixed data is the 
same as the original data stored in MMIO device, MMIO writing might cause 
unexpected behavior.

3.  CoreFreePages() is called when image is not loaded successfully. Is it 
expected for XIP case?

Thanks,
Ray

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 07/11] MdeModulePkg/DxeCore: Execute loaded images in
> place if possible
> 
> In the image loader, check whether an image has already been relocated
> to the address from which it is being loaded. This is not something that
> can happen by accident, and so we can assume that this means that the
> image was intended to be executed in place.
> 
> This removes a redundant copy of the image contents, and also permits
> the image to be mapped with restricted permissions even before the CPU
> arch protocol has been dispatched.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/Image/Image.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 3dfab4829b3ca17f..621637e869daf62d 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -573,7 +573,7 @@ STATIC
>  EFI_STATUS
> 
>  CoreLoadPeImage (
> 
>IN BOOLEANBootPolicy,
> 
> -  IN VOID   *Pe32Handle,
> 
> +  IN IMAGE_FILE_HANDLE  *Pe32Handle,
> 
>IN LOADED_IMAGE_PRIVATE_DATA  *Image,
> 
>IN  UINT32Attribute
> 
>)
> 
> @@ -630,10 +630,16 @@ CoreLoadPeImage (
>return EFI_UNSUPPORTED;
> 
>}
> 
> 
> 
> +  //
> 
> +  // Check whether the loaded image can be executed in place
> 
> +  //
> 
> +  if (Image->ImageContext.ImageAddress ==
> (PHYSICAL_ADDRESS)(UINTN)Pe32Handle->Source) {
> 
> +goto ExecuteInPlace;
> 
> +  }
> 
> +
> 
>//
> 
>// Allocate Destination Buffer as caller did not pass it in
> 
>//
> 
> -
> 
>if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {
> 
>  Size = (UINTN)Image->ImageContext.ImageSize + Image-
> >ImageContext.SectionAlignment;
> 
>} else {
> 
> @@ -704,6 +710,7 @@ CoreLoadPeImage (
>//
> 
>// Load the image from the file into the allocated memory
> 
>//
> 
> +ExecuteInPlace:
> 
>Status = PeCoffLoaderLoadImage (&Image->ImageContext);
> 
>if (EFI_ERROR (Status)) {
> 
>  goto Done;
> 
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105411): https://edk2.groups.io/g/devel/message/105411
Mute This Topic: https://groups.io/mt/99197140/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg: Use DEBUG_MANAGEABILITY

[Chang, Abner via groups.io]

From: Abner Chang 

Use debug print level DEBUG_MANAGEABILITY in
ManageabilityPkg.

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../Library/ManageabilityTransportHelperLib.h  |  2 +-
 .../BaseManageabilityTransportHelper.c |  2 +-
 .../Universal/IpmiBmcAcpi/BmcAcpi.c|  6 --
 .../Universal/IpmiBmcElog/BmcElog.c|  4 +++-
 .../ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c|  8 +---
 .../PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c  | 14 +++---
 6 files changed, 21 insertions(+), 15 deletions(-)

diff --git 
a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.h 
b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.h
index 11a1bd0521..dfe32189ad 100644
--- 
a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.h
+++ 
b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.h
@@ -11,7 +11,7 @@
 
 #include 
 
-#define DEBUG_MANAGEABILITY_INFO  DEBUG_INFO
+#define DEBUG_MANAGEABILITY_INFO  DEBUG_MANAGEABILITY
 
 typedef struct _MANAGEABILITY_PROTOCOL_NAME MANAGEABILITY_PROTOCOL_NAME;
 
diff --git 
a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
 
b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
index f72957ea7f..27bc5eaddf 100644
--- 
a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
+++ 
b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
@@ -163,7 +163,7 @@ HelperAcquireManageabilityTransport (
   CHAR16  *ManageabilityProtocolName;
   CHAR16  *ManageabilityTransportName;
 
-  DEBUG ((DEBUG_INFO, "%a: Entry\n", __func__));
+  DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Entry\n", __func__));
   if ((TransportToken == NULL) || (ManageabilityProtocolSpec == NULL)) {
 DEBUG ((DEBUG_ERROR, "%a: One of the required input parameters is 
NULL.\n", __func__));
 return EFI_INVALID_PARAMETER;
diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c 
b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
index cf066dd095..d04623ecad 100644
--- a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
+++ b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
@@ -24,6 +24,8 @@
 #include 
 #include 
 
+#include 
+
 #ifndef EFI_ACPI_CREATOR_ID
 #define EFI_ACPI_CREATOR_ID  SIGNATURE_32 ('M', 'S', 'F', 'T')
 #endif
@@ -140,7 +142,7 @@ UpdateDeviceSsdtTable (
   //
   // Update IO(Decode16, 0xCA2, 0xCA2, 0, 2)
   //
-  DEBUG ((DEBUG_INFO, "UpdateDeviceSsdtTable - IPMI\n"));
+  DEBUG ((DEBUG_MANAGEABILITY_INFO, "UpdateDeviceSsdtTable - IPMI\n"));
   for (DataPtr = (UINT8 *)(Table + 1);
DataPtr < (UINT8 *)((UINT8 *)Table + Table->Length - 4);
DataPtr++)
@@ -158,7 +160,7 @@ UpdateDeviceSsdtTable (
   ASSERT (IoRsc->Header.Bits.Type == ACPI_SMALL_ITEM_FLAG);
   ASSERT (IoRsc->Header.Bits.Name == ACPI_SMALL_IO_PORT_DESCRIPTOR_NAME);
   ASSERT (IoRsc->Header.Bits.Length == sizeof 
(EFI_ACPI_IO_PORT_DESCRIPTOR) - sizeof (ACPI_SMALL_RESOURCE_HEADER));
-  DEBUG ((DEBUG_INFO, "IPMI IO Base in ASL update - 0x%04x <= 0x%04x\n", 
IoRsc->BaseAddressMin, PcdGet16 (PcdIpmiKcsIoBaseAddress)));
+  DEBUG ((DEBUG_MANAGEABILITY_INFO, "IPMI IO Base in ASL update - 0x%04x 
<= 0x%04x\n", IoRsc->BaseAddressMin, PcdGet16 (PcdIpmiKcsIoBaseAddress)));
   IoRsc->BaseAddressMin = PcdGet16 (PcdIpmiKcsIoBaseAddress);
   IoRsc->BaseAddressMax = PcdGet16 (PcdIpmiKcsIoBaseAddress);
 }
diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c 
b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
index 02873fc4c6..8b34b2d2d5 100644
--- a/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
+++ b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
@@ -15,6 +15,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 
+#include 
+
 EFI_STATUS
 EFIAPI
 CheckIfSelIsFull (
@@ -186,7 +188,7 @@ CheckIfSelIsFull (
   // Check the Bit7 of the OperationByte if SEL is OverFlow.
   //
   SelIsFull = (SelInfo.OperationSupport & 0x80);
-  DEBUG ((DEBUG_INFO, "SelIsFull - 0x%x\n", SelIsFull));
+  DEBUG ((DEBUG_MANAGEABILITY_INFO, "SelIsFull - 0x%x\n", SelIsFull));
 
   return EFI_SUCCESS;
 }
diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c 
b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
index 46f741eed1..40ae0c3ecc 100644
--- a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
+++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
@@ -16,6 +16,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 
+#include 
+
 /**
   This routine disables the specified FRB timer.
 
@@ -159,7 +161,7 @@ ReportFrb2Status (
   //
   Status = IpmiGetWatchdogTimer (&GetWatchdogTimer);

[edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg: Replace __FUNCTION__ with __func__

[Chang, Abner via groups.io]

From: Abner Chang 

Signed-off-by: Abner Chang 
Cc: Isaac Oram 
Cc: Abdul Lateef Attar 
Cc: Nickle Wang 
Cc: Tinh Nguyen 
---
 .../BaseManageabilityTransportHelper.c| 36 +--
 .../Common/KcsCommon.c| 12 +++
 .../Dxe/ManageabilityTransportKcs.c   | 28 +++
 .../Dxe/ManageabilityTransportMctp.c  | 22 ++--
 .../PldmProtocolLibrary/Dxe/PldmProtocolLib.c |  4 +--
 .../IpmiProtocol/Common/IpmiProtocolCommon.c  | 14 
 .../Universal/IpmiProtocol/Dxe/IpmiProtocol.c | 14 
 .../Universal/IpmiProtocol/Pei/IpmiPpi.c  | 18 +-
 .../Universal/IpmiProtocol/Smm/IpmiProtocol.c | 14 
 .../MctpProtocol/Common/MctpProtocolCommon.c  | 30 
 .../Universal/MctpProtocol/Dxe/MctpProtocol.c | 18 +-
 .../PldmProtocol/Common/PldmProtocolCommon.c  | 20 +--
 .../Universal/PldmProtocol/Dxe/PldmProtocol.c | 14 
 .../PldmSmbiosTransferDxe.c   | 22 ++--
 14 files changed, 133 insertions(+), 133 deletions(-)

diff --git 
a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
 
b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
index ce68f89531..f72957ea7f 100644
--- 
a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
+++ 
b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/BaseManageabilityTransportHelper.c
@@ -49,7 +49,7 @@ HelperManageabilitySpecName (
   }
 
   if ((SpecificationGuid == NULL) || IsZeroGuid (SpecificationGuid)) {
-DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero 
GUID.\n", __FUNCTION__));
+DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero 
GUID.\n", __func__));
 return NULL;
   }
 
@@ -106,7 +106,7 @@ HelperManageabilityCheckSupportedSpec (
   IsZeroGuid (ManageabilityProtocolToCheck)
   )
   {
-DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero 
GUID.\n", __FUNCTION__));
+DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero 
GUID.\n", __func__));
 return EFI_INVALID_PARAMETER;
   }
 
@@ -120,7 +120,7 @@ HelperManageabilityCheckSupportedSpec (
   DEBUG ((
 DEBUG_MANAGEABILITY_INFO,
 "%a: Transport interface %s supports %s manageability 
specification.\n",
-__FUNCTION__,
+__func__,
 HelperManageabilitySpecName (TransportGuid),
 HelperManageabilitySpecName (ManageabilityProtocolToCheck)
 ));
@@ -133,7 +133,7 @@ HelperManageabilityCheckSupportedSpec (
   DEBUG ((
 DEBUG_ERROR,
 "%a: Transport interface %s doesn't support %s manageability 
specification.\n",
-__FUNCTION__,
+__func__,
 HelperManageabilitySpecName (TransportGuid),
 HelperManageabilitySpecName (ManageabilityProtocolToCheck)
 ));
@@ -163,16 +163,16 @@ HelperAcquireManageabilityTransport (
   CHAR16  *ManageabilityProtocolName;
   CHAR16  *ManageabilityTransportName;
 
-  DEBUG ((DEBUG_INFO, "%a: Entry\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a: Entry\n", __func__));
   if ((TransportToken == NULL) || (ManageabilityProtocolSpec == NULL)) {
-DEBUG ((DEBUG_ERROR, "%a: One of the required input parameters is 
NULL.\n", __FUNCTION__));
+DEBUG ((DEBUG_ERROR, "%a: One of the required input parameters is 
NULL.\n", __func__));
 return EFI_INVALID_PARAMETER;
   }
 
   *TransportToken   = NULL;
   ManageabilityProtocolName = HelperManageabilitySpecName 
(ManageabilityProtocolSpec);
   if (ManageabilityProtocolName == NULL) {
-DEBUG ((DEBUG_ERROR, "%a: Unsupported Manageability Protocol 
Specification.\n", __FUNCTION__));
+DEBUG ((DEBUG_ERROR, "%a: Unsupported Manageability Protocol 
Specification.\n", __func__));
 return EFI_UNSUPPORTED;
   }
 
@@ -180,7 +180,7 @@ HelperAcquireManageabilityTransport (
 
   Status = AcquireTransportSession (ManageabilityProtocolSpec, TransportToken);
   if (Status == EFI_UNSUPPORTED) {
-DEBUG ((DEBUG_ERROR, "%a: No supported transport interface for %s 
packet.\n", __FUNCTION__, ManageabilityProtocolName));
+DEBUG ((DEBUG_ERROR, "%a: No supported transport interface for %s 
packet.\n", __func__, ManageabilityProtocolName));
 return Status;
   }
 
@@ -188,7 +188,7 @@ HelperAcquireManageabilityTransport (
 DEBUG ((
   DEBUG_ERROR,
   "%a: Fail to acquire Manageability transport token for %s (%r).\n",
-  __FUNCTION__,
+  __func__,
   ManageabilityProtocolName,
   Status
   ));
@@ -197,11 +197,11 @@ HelperAcquireManageabilityTransport (
 
   ManageabilityTransportName = HelperManageabilitySpecName 
((*TransportToken)->Transport->ManageabilityTransportSpecification);
   if (ManageabilityTransportName == NULL) {
-DEBUG ((DEBUG_ERROR, "%a: Unsupported Manageability Transport Interface 
Specification\n", _

Re: [edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and remap XIP capable DXE drivers

[Ni, Ray]

1. is it possible that a PE image sits in the right location but the 
SectionAlignment is larger than FileAlignment so each section still needs to be 
copied to the aligned memory location?

2. PeCoffLoaderRelocateImage() might not be called for XIP 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ard
> Biesheuvel
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [edk2-devel] [RFC PATCH 08/11] MdeModulePkg/DxeIpl: Relocate and
> remap XIP capable DXE drivers
> 
> Before handing over to the DXE core, iterate over all known FFS files
> and find the ones that can execute in place. These files are then
> relocated in place and mapped with restricted permissions, allowing the
> DXE core to dispatch them without the need to perform any manipulation
> of the file contents or the page table permissions.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf |   1 +
>  MdeModulePkg/Core/DxeIplPeim/DxeLoad.c  | 196 
>  2 files changed, 197 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index f1990eac77607854..60112100df78b396 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -65,6 +65,7 @@ [LibraryClasses]
>PeimEntryPoint
> 
>DebugLib
> 
>DebugAgentLib
> 
> +  PeCoffLib
> 
>PeiServicesTablePointerLib
> 
>PerformanceLib
> 
> 
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> index 2c19f1a507baf34a..1f20db1faffbd1d2 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> @@ -10,6 +10,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
>  #include "DxeIpl.h"
> 
> 
> 
> +#include 
> 
> +#include 
> 
> +
> 
>  //
> 
>  // Module Globals used in the DXE to PEI hand off
> 
>  // These must be module globals, so the stack can be switched
> 
> @@ -228,6 +231,197 @@ ValidateMemoryTypeInfoVariable (
>return TRUE;
> 
>  }
> 
> 
> 
> +/**
> 
> +  Support routine for the PE/COFF Loader that reads a buffer from a PE/COFF 
> file.
> 
> +  The function is used for XIP code to have optimized memory copy.
> 
> +
> 
> +  @param FileHandle  The handle to the PE/COFF file
> 
> +  @param FileOffset  The offset, in bytes, into the file to read
> 
> +  @param ReadSizeThe number of bytes to read from the file starting 
> at
> FileOffset
> 
> +  @param Buffer  A pointer to the buffer to read the data into.
> 
> +
> 
> +  @return EFI_SUCCESSReadSize bytes of data were read into Buffer from 
> the
> 
> + PE/COFF file starting at FileOffset
> 
> +
> 
> +**/
> 
> +STATIC
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +PeiImageRead (
> 
> +  IN VOID   *FileHandle,
> 
> +  IN UINTN  FileOffset,
> 
> +  IN UINTN  *ReadSize,
> 
> +  OUTVOID   *Buffer
> 
> +  )
> 
> +{
> 
> +  CHAR8  *Destination8;
> 
> +  CHAR8  *Source8;
> 
> +
> 
> +  Destination8 = Buffer;
> 
> +  Source8  = (CHAR8 *)((UINTN)FileHandle + FileOffset);
> 
> +  if (Destination8 != Source8) {
> 
> +CopyMem (Destination8, Source8, *ReadSize);
> 
> +  }
> 
> +
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> +
> 
> +STATIC
> 
> +VOID
> 
> +RemapImage (
> 
> +  IN  EDKII_MEMORY_ATTRIBUTE_PPI  *MemoryPpi,
> 
> +  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
> 
> +  )
> 
> +{
> 
> +  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr;
> 
> +  EFI_IMAGE_SECTION_HEADER *Section;
> 
> +  PHYSICAL_ADDRESS SectionAddress;
> 
> +  EFI_STATUS   Status;
> 
> +  UINT64   Permissions;
> 
> +  UINTNIndex;
> 
> +
> 
> +  if (MemoryPpi == NULL) {
> 
> +return;
> 
> +  }
> 
> +
> 
> +  Hdr.Union = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((UINT8
> *)ImageContext->Handle +
> 
> +  
> ImageContext->PeCoffHeaderOffset);
> 
> +  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
> 
> +
> 
> +  Section = (EFI_IMAGE_SECTION_HEADER *)((UINT8 *)Hdr.Union + sizeof
> (UINT32) +
> 
> + sizeof (EFI_IMAGE_FILE_HEADER) +
> 
> + 
> Hdr.Pe32->FileHeader.SizeOfOptionalHeader
> 
> + );
> 
> +
> 
> +  for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
> 
> +SectionAddress = ImageContext->ImageAddress +
> Section[Index].VirtualAddress;
> 
> +Permissions= 0;
> 
> +
> 
> +if ((Section[Index].Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0) {
> 
> +  Permissions |= EFI_MEMORY_RO;
> 
> +}
> 
> +
> 
> + 

Re: [edk2-devel] [edk2-platforms][PATCH 1/2] ManageabilityPkg: Replace __FUNCTION__ with __func__

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang 

Regards,
Nickle

> -Original Message-
> From: abner.ch...@amd.com 
> Sent: Tuesday, May 30, 2023 2:33 PM
> To: devel@edk2.groups.io
> Cc: Isaac Oram ; Abdul Lateef Attar
> ; Nickle Wang ; Tinh Nguyen
> 
> Subject: [edk2-platforms][PATCH 1/2] ManageabilityPkg: Replace __FUNCTION__
> with __func__
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Abner Chang 
> 
> Signed-off-by: Abner Chang 
> Cc: Isaac Oram 
> Cc: Abdul Lateef Attar 
> Cc: Nickle Wang 
> Cc: Tinh Nguyen 
> ---
>  .../BaseManageabilityTransportHelper.c| 36 +--
>  .../Common/KcsCommon.c| 12 +++
>  .../Dxe/ManageabilityTransportKcs.c   | 28 +++
>  .../Dxe/ManageabilityTransportMctp.c  | 22 ++--
>  .../PldmProtocolLibrary/Dxe/PldmProtocolLib.c |  4 +--
>  .../IpmiProtocol/Common/IpmiProtocolCommon.c  | 14 
>  .../Universal/IpmiProtocol/Dxe/IpmiProtocol.c | 14 
>  .../Universal/IpmiProtocol/Pei/IpmiPpi.c  | 18 +-
>  .../Universal/IpmiProtocol/Smm/IpmiProtocol.c | 14 
>  .../MctpProtocol/Common/MctpProtocolCommon.c  | 30 
>  .../Universal/MctpProtocol/Dxe/MctpProtocol.c | 18 +-
>  .../PldmProtocol/Common/PldmProtocolCommon.c  | 20 +--
>  .../Universal/PldmProtocol/Dxe/PldmProtocol.c | 14 
>  .../PldmSmbiosTransferDxe.c   | 22 ++--
>  14 files changed, 133 insertions(+), 133 deletions(-)
> 
> diff --git
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> index ce68f89531..f72957ea7f 100644
> ---
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> @@ -49,7 +49,7 @@ HelperManageabilitySpecName (
>}
> 
>if ((SpecificationGuid == NULL) || IsZeroGuid (SpecificationGuid)) {
> -DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero
> GUID.\n", __FUNCTION__));
> +DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero
> GUID.\n", __func__));
>  return NULL;
>}
> 
> @@ -106,7 +106,7 @@ HelperManageabilityCheckSupportedSpec (
>IsZeroGuid (ManageabilityProtocolToCheck)
>)
>{
> -DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero
> GUID.\n", __FUNCTION__));
> +DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or zero
> GUID.\n", __func__));
>  return EFI_INVALID_PARAMETER;
>}
> 
> @@ -120,7 +120,7 @@ HelperManageabilityCheckSupportedSpec (
>DEBUG ((
>  DEBUG_MANAGEABILITY_INFO,
>  "%a: Transport interface %s supports %s manageability 
> specification.\n",
> -__FUNCTION__,
> +__func__,
>  HelperManageabilitySpecName (TransportGuid),
>  HelperManageabilitySpecName (ManageabilityProtocolToCheck)
>  ));
> @@ -133,7 +133,7 @@ HelperManageabilityCheckSupportedSpec (
>DEBUG ((
>  DEBUG_ERROR,
>  "%a: Transport interface %s doesn't support %s manageability
> specification.\n",
> -__FUNCTION__,
> +__func__,
>  HelperManageabilitySpecName (TransportGuid),
>  HelperManageabilitySpecName (ManageabilityProtocolToCheck)
>  ));
> @@ -163,16 +163,16 @@ HelperAcquireManageabilityTransport (
>CHAR16  *ManageabilityProtocolName;
>CHAR16  *ManageabilityTransportName;
> 
> -  DEBUG ((DEBUG_INFO, "%a: Entry\n", __FUNCTION__));
> +  DEBUG ((DEBUG_INFO, "%a: Entry\n", __func__));
>if ((TransportToken == NULL) || (ManageabilityProtocolSpec == NULL)) {
> -DEBUG ((DEBUG_ERROR, "%a: One of the required input parameters is
> NULL.\n", __FUNCTION__));
> +DEBUG ((DEBUG_ERROR, "%a: One of the required input parameters is
> NULL.\n", __func__));
>  return EFI_INVALID_PARAMETER;
>}
> 
>*TransportToken   = NULL;
>ManageabilityProtocolName = HelperManageabilitySpecName
> (ManageabilityProtocolSpec);
>if (ManageabilityProtocolName == NULL) {
> -DEBUG ((DEBUG_ERROR, "%a: Unsupported Manageability Protocol
> Specification.\n", __FUNCTION__));
> +DEBUG ((DEBUG_ERROR, "%a: Unsupported Manageability Protocol
> Specification.\n", __func__));
>  return EFI_UNSUPPORTED;
>}
> 
> @@ -180,7 +180,7 @@ HelperAcquireManageabilityTransport (
> 
>Status = AcquireTransportSession (ManageabilityProtocolSpec, 
> TransportToken);
>if (Status == EFI_UNSUPPORTED) {
> -DEBUG ((DEBUG_ERROR, "%a: No supported transport interface for %s
> packet.\n", __FUNCTION__, ManageabilityProtocolName));
> +DEBUG ((DEBUG_ERROR, "%a: No supported transport interface for %s
> packet.\n", __func__, ManageabilityProtocolName));
>  return Status

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] ManageabilityPkg: Use DEBUG_MANAGEABILITY

[Nickle Wang via groups.io]

Reviewed-by: Nickle Wang 

Regards,
Nickle

> -Original Message-
> From: abner.ch...@amd.com 
> Sent: Tuesday, May 30, 2023 2:33 PM
> To: devel@edk2.groups.io
> Cc: Isaac Oram ; Abdul Lateef Attar
> ; Nickle Wang ; Tinh Nguyen
> 
> Subject: [edk2-platforms][PATCH 2/2] ManageabilityPkg: Use
> DEBUG_MANAGEABILITY
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Abner Chang 
> 
> Use debug print level DEBUG_MANAGEABILITY in
> ManageabilityPkg.
> 
> Signed-off-by: Abner Chang 
> Cc: Isaac Oram 
> Cc: Abdul Lateef Attar 
> Cc: Nickle Wang 
> Cc: Tinh Nguyen 
> ---
>  .../Library/ManageabilityTransportHelperLib.h  |  2 +-
>  .../BaseManageabilityTransportHelper.c |  2 +-
>  .../Universal/IpmiBmcAcpi/BmcAcpi.c|  6 --
>  .../Universal/IpmiBmcElog/BmcElog.c|  4 +++-
>  .../ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c|  8 +---
>  .../PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c  | 14 +++---
>  6 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.
> h
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.
> h
> index 11a1bd0521..dfe32189ad 100644
> ---
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.
> h
> +++
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelperLib.
> h
> @@ -11,7 +11,7 @@
> 
>  #include 
> 
> -#define DEBUG_MANAGEABILITY_INFO  DEBUG_INFO
> +#define DEBUG_MANAGEABILITY_INFO  DEBUG_MANAGEABILITY
> 
>  typedef struct _MANAGEABILITY_PROTOCOL_NAME
> MANAGEABILITY_PROTOCOL_NAME;
> 
> diff --git
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> index f72957ea7f..27bc5eaddf 100644
> ---
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/Bas
> eManageabilityTransportHelper.c
> @@ -163,7 +163,7 @@ HelperAcquireManageabilityTransport (
>CHAR16  *ManageabilityProtocolName;
>CHAR16  *ManageabilityTransportName;
> 
> -  DEBUG ((DEBUG_INFO, "%a: Entry\n", __func__));
> +  DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Entry\n", __func__));
>if ((TransportToken == NULL) || (ManageabilityProtocolSpec == NULL)) {
>  DEBUG ((DEBUG_ERROR, "%a: One of the required input parameters is
> NULL.\n", __func__));
>  return EFI_INVALID_PARAMETER;
> diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> index cf066dd095..d04623ecad 100644
> --- a/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> +++ b/Features/ManageabilityPkg/Universal/IpmiBmcAcpi/BmcAcpi.c
> @@ -24,6 +24,8 @@
>  #include 
>  #include 
> 
> +#include 
> +
>  #ifndef EFI_ACPI_CREATOR_ID
>  #define EFI_ACPI_CREATOR_ID  SIGNATURE_32 ('M', 'S', 'F', 'T')
>  #endif
> @@ -140,7 +142,7 @@ UpdateDeviceSsdtTable (
>//
>// Update IO(Decode16, 0xCA2, 0xCA2, 0, 2)
>//
> -  DEBUG ((DEBUG_INFO, "UpdateDeviceSsdtTable - IPMI\n"));
> +  DEBUG ((DEBUG_MANAGEABILITY_INFO, "UpdateDeviceSsdtTable - IPMI\n"));
>for (DataPtr = (UINT8 *)(Table + 1);
> DataPtr < (UINT8 *)((UINT8 *)Table + Table->Length - 4);
> DataPtr++)
> @@ -158,7 +160,7 @@ UpdateDeviceSsdtTable (
>ASSERT (IoRsc->Header.Bits.Type == ACPI_SMALL_ITEM_FLAG);
>ASSERT (IoRsc->Header.Bits.Name ==
> ACPI_SMALL_IO_PORT_DESCRIPTOR_NAME);
>ASSERT (IoRsc->Header.Bits.Length == sizeof
> (EFI_ACPI_IO_PORT_DESCRIPTOR) - sizeof (ACPI_SMALL_RESOURCE_HEADER));
> -  DEBUG ((DEBUG_INFO, "IPMI IO Base in ASL update - 0x%04x <= 0x%04x\n",
> IoRsc->BaseAddressMin, PcdGet16 (PcdIpmiKcsIoBaseAddress)));
> +  DEBUG ((DEBUG_MANAGEABILITY_INFO, "IPMI IO Base in ASL update -
> 0x%04x <= 0x%04x\n", IoRsc->BaseAddressMin, PcdGet16
> (PcdIpmiKcsIoBaseAddress)));
>IoRsc->BaseAddressMin = PcdGet16 (PcdIpmiKcsIoBaseAddress);
>IoRsc->BaseAddressMax = PcdGet16 (PcdIpmiKcsIoBaseAddress);
>  }
> diff --git a/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
> b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
> index 02873fc4c6..8b34b2d2d5 100644
> --- a/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
> +++ b/Features/ManageabilityPkg/Universal/IpmiBmcElog/BmcElog.c
> @@ -15,6 +15,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
> 
> +#include 
> +
>  EFI_STATUS
>  EFIAPI
>  CheckIfSelIsFull (
> @@ -186,7 +188,7 @@ CheckIfSelIsFull (
>// Check the Bit7 of the OperationByte if SEL is OverFlow.
>//
>SelIsFull = (SelInfo.OperationSupport & 0x80);
> -  DEBUG ((DEBUG_INFO, "SelIsFull - 0x%x\n", SelIsFull));
> +  DEBUG ((DEBUG

Re: [edk2-devel] [RFC PATCH 09/11] MdeModulePkg/DxeCore: Add PCD NX policy bit for default NX state

[Ni, Ray]

1. Do we want to catch a case that platform wrongly sets BIT61 but drivers run 
before CpuDxe are not XIP?
2. Why BIT61 set is the "Default state"?

The setting of BIT61 is a bit confusing. Is there a way to avoid adding BIT61 
through code optimization?

Thanks,
Ray

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, May 29, 2023 6:17 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Ni, Ray ; Yao, Jiewen
> ; Gerd Hoffmann ; Taylor Beebe
> ; Oliver Smith-Denny ; Bi, Dandan
> ; Gao, Liming ; Kinney,
> Michael D ; Leif Lindholm
> ; Michael Kubacki 
> Subject: [RFC PATCH 09/11] MdeModulePkg/DxeCore: Add PCD NX policy bit for
> default NX state
> 
> Introduce a new bit in the NX memory protection policy PCD mask that
> specifies that the platform enters DXE with all unused and all non-code
> regions mapped with non-execute permissions.
> 
> This removes the need to do a pass over all memory regions to update
> their NX memory attributes.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 7 +++
>  MdeModulePkg/MdeModulePkg.dec | 3 +++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 7cc829b17402c2bc..983ed450f143d62d 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -861,6 +861,13 @@ InitializeDxeNxMemoryProtectionPolicy (
>  ASSERT (StackBase != 0);
> 
>}
> 
> 
> 
> +  //
> 
> +  // If the platform maps all DRAM non-execute by default, we are done here.
> 
> +  //
> 
> +  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT61) != 0) {
> 
> +return;
> 
> +  }
> 
> +
> 
>DEBUG ((
> 
>  DEBUG_INFO,
> 
>  "%a: applying strict permissions to active memory regions\n",
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 2d72ac733d82195e..d2bd0cbb40300889 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1416,12 +1416,15 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>#  EfiMemoryMappedIOPortSpace 0x1000
> 
>#  EfiPalCode 0x2000
> 
>#  EfiPersistentMemory0x4000
> 
> +  #  Default state  0x2000
> 
>#  OEM Reserved   0x4000
> 
>#  OS Reserved0x8000
> 
>#
> 
># NOTE: User must NOT set NX protection for EfiLoaderCode /
> EfiBootServicesCode / EfiRuntimeServicesCode. 
> 
>#   User MUST set the same NX protection for EfiBootServicesData and
> EfiConventionalMemory. 
> 
>#
> 
> +  # If the platform enters DXE with all unused and non-code regions mapped 
> NX,
> bit 61 should be set.
> 
> +  #
> 
># e.g. 0x7FD5 can be used for all memory except Code. 
> 
># e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved.
> 
> 
>#
> 
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105417): https://edk2.groups.io/g/devel/message/105417
Mute This Topic: https://groups.io/mt/99197143/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-