[edk2-devel] [PATCH V1 1/1] OvmfPkg/QemuBootOrderLib: Measure the etc/boot-menu-wait

[sunceping]

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

Refer to the section 8.3.4 of tdx-virtual-firmware-design-guide spec,
OVMF would uses FW_CFG_IO_SELECTOR(0x510) and FW_CFG_IO_DATA(0x511)
to get configuration data from QEMU. From the security perspective,
if TDVF uses this method, configuration data must be measured into
RTMR[0].

Currently, the etc/boot-menu-wait is using in TDVF, it required to be
measured into RTMR[0].

This is the first patch and will continue to be updated to measure
additional configuration data.

Refernce:
spec: https://cdrdv2.intel.com/v1/dl/getContent/733585

Cc: Erdem Aktas 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Gerd Hoffmann 
Cc: Elena Reshetova 
Signed-off-by: Ceping Sun 
---
 .../QemuBootOrderLib/QemuBootOrderLib.c   | 21 ++-
 .../QemuBootOrderLib/QemuBootOrderLib.inf |  1 +
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c 
b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
index 2fe6ab30c032..63a290712002 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
@@ -20,6 +20,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "ExtraRootBusMap.h"
 
@@ -41,6 +43,9 @@
 #define REQUIRED_MMIO_OFW_NODES  1
 #define EXAMINED_OFW_NODES   6
 
+#define EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA  "QEMU BOOTMENU WAIT 
TIME"
+#define QEMU_BOOTMENU_WAIT_DATA_LEN
(sizeof(EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA) - 1)
+
 /**
   Simple character classification routines, corresponding to POSIX class names
   and ASCII encoding.
@@ -2418,5 +2423,19 @@ GetFrontPageTimeoutFromQemu (
   // seconds, round N up.
   //
   QemuFwCfgSelectItem (BootMenuWaitItem);
-  return (UINT16)((QemuFwCfgRead16 () + 999) / 1000);
+  Timeout = QemuFwCfgRead16 ();
+  //
+  // Measure the Timeout which is downloaded from QEMU.
+  // It has to be done before it is consumed.
+  //
+  TpmMeasureAndLogData (
+1,
+EV_PLATFORM_CONFIG_FLAGS,
+EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA,
+QEMU_BOOTMENU_WAIT_DATA_LEN,
+(VOID *)(UINTN)&Timeout,
+BootMenuWaitSize
+);
+
+  return (UINT16)((Timeout + 999) / 1000);
 }
diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf 
b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
index 6e320e3e8514..0231c9d5c5b8 100644
--- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
+++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
@@ -45,6 +45,7 @@
   DevicePathLib
   BaseMemoryLib
   OrderedCollectionLib
+  TpmMeasurementLib
 
 [Guids]
   gEfiGlobalVariableGuid
-- 
2.34.1



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

Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/QemuBootOrderLib: Measure the etc/boot-menu-wait

[Yao, Jiewen]

Thanks for the patch.

Is this the only missing configuration data?
Or do you have more on the way?



> -Original Message-
> From: Sun, CepingX 
> Sent: Wednesday, March 13, 2024 7:52 AM
> To: devel@edk2.groups.io
> Cc: Sun, CepingX ; Aktas, Erdem
> ; Yao, Jiewen ; Xu, Min M
> ; Gerd Hoffmann ; Reshetova, Elena
> 
> Subject: [PATCH V1 1/1] OvmfPkg/QemuBootOrderLib: Measure the etc/boot-
> menu-wait
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4415
> 
> Refer to the section 8.3.4 of tdx-virtual-firmware-design-guide spec,
> OVMF would uses FW_CFG_IO_SELECTOR(0x510) and FW_CFG_IO_DATA(0x511)
> to get configuration data from QEMU. From the security perspective,
> if TDVF uses this method, configuration data must be measured into
> RTMR[0].
> 
> Currently, the etc/boot-menu-wait is using in TDVF, it required to be
> measured into RTMR[0].
> 
> This is the first patch and will continue to be updated to measure
> additional configuration data.
> 
> Refernce:
> spec: https://cdrdv2.intel.com/v1/dl/getContent/733585
> 
> Cc: Erdem Aktas 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Gerd Hoffmann 
> Cc: Elena Reshetova 
> Signed-off-by: Ceping Sun 
> ---
>  .../QemuBootOrderLib/QemuBootOrderLib.c   | 21 ++-
>  .../QemuBootOrderLib/QemuBootOrderLib.inf |  1 +
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> index 2fe6ab30c032..63a290712002 100644
> --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c
> @@ -20,6 +20,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> 
>  #include "ExtraRootBusMap.h"
> 
> @@ -41,6 +43,9 @@
>  #define REQUIRED_MMIO_OFW_NODES  1
>  #define EXAMINED_OFW_NODES   6
> 
> +#define EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA  "QEMU
> BOOTMENU WAIT TIME"
> +#define QEMU_BOOTMENU_WAIT_DATA_LEN
> (sizeof(EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA) - 1)
> +
>  /**
>Simple character classification routines, corresponding to POSIX class 
> names
>and ASCII encoding.
> @@ -2418,5 +2423,19 @@ GetFrontPageTimeoutFromQemu (
>// seconds, round N up.
>//
>QemuFwCfgSelectItem (BootMenuWaitItem);
> -  return (UINT16)((QemuFwCfgRead16 () + 999) / 1000);
> +  Timeout = QemuFwCfgRead16 ();
> +  //
> +  // Measure the Timeout which is downloaded from QEMU.
> +  // It has to be done before it is consumed.
> +  //
> +  TpmMeasureAndLogData (
> +1,
> +EV_PLATFORM_CONFIG_FLAGS,
> +EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA,
> +QEMU_BOOTMENU_WAIT_DATA_LEN,
> +(VOID *)(UINTN)&Timeout,
> +BootMenuWaitSize
> +);
> +
> +  return (UINT16)((Timeout + 999) / 1000);
>  }
> diff --git a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> index 6e320e3e8514..0231c9d5c5b8 100644
> --- a/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> +++ b/OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> @@ -45,6 +45,7 @@
>DevicePathLib
>BaseMemoryLib
>OrderedCollectionLib
> +  TpmMeasurementLib
> 
>  [Guids]
>gEfiGlobalVariableGuid
> --
> 2.34.1



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

Re: [edk2-devel][PATCH v1 1/3] MdeModulePkg: ImagePropertiesRecordLib: Use SectionAlignment for CodeSize

[Ard Biesheuvel]

On Mon, 11 Mar 2024 at 20:34, Oliver Smith-Denny
 wrote:
>
> On 3/4/2024 2:38 PM, Oliver Smith-Denny wrote:
> > On 3/4/2024 11:24 AM, Oliver Smith-Denny wrote:
> >> On 3/4/2024 10:54 AM, Ard Biesheuvel wrote:
> >>> On Mon, 4 Mar 2024 at 18:49, Oliver Smith-Denny
> >>>  wrote:
> 
>  Hi Ard,
> 
>  On 3/1/2024 3:58 AM, Ard Biesheuvel wrote:
> > Hi Oliver,
> >
> > On Tue, 27 Feb 2024 at 21:27, Oliver Smith-Denny
> >  wrote:
> >>
> >> -  ImageRecordCodeSection->CodeSegmentSize =
> >> Section[Index].SizeOfRawData;
> >> +  ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE
> >> (Section[Index].SizeOfRawData, SectionAlignment);
> >>
> >
> > This should be the virtual size, not the file size, right?
> 
>  Correct, SectionAlignment is the alignment of the image as loaded in
>  memory, so in the case of a DXE runtime driver on ARM64, it will be
>  64k.
> 
> >>>
> >>> No, I mean we should not be using SizeOfRawData here but VirtualSize.
> >>>
> >>> I understand this is unlikely to make a difference in practice, but
> >>> VirtualSize may exceed SizeOfRawData by a wide margin, and we need the
> >>> whole region to be covered.
> >>>
> >>
> >> I see, yes I do believe VirtualSize could be used here instead. Two
> >> things give me pause. One is that the PE spec states that SizeOfRawData
> >> is rounded and VirtualSize is not, so that SizeOfRawData may be greater
> >> than the VirtualSize in some cases (which seems incorrect).
> >>
> >> The other is that the image loader partially uses VirtualSize when
> >> loading:
> >>
> >> https://github.com/tianocore/edk2/blob/918288ab5a7c3abe9c58d576ccc0ae32e2c7dea0/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L1399-L1400
> >>
> >> However, when determining the size of a loaded image (and therefore the
> >> number of pages to allocate) it will allocate an extra page:
> >>
> >> https://github.com/tianocore/edk2/blob/918288ab5a7c3abe9c58d576ccc0ae32e2c7dea0/MdeModulePkg/Core/Dxe/Image/Image.c#L646-L652
> >>
> >> as ImageSize here is:
> >>
> >> https://github.com/tianocore/edk2/blob/918288ab5a7c3abe9c58d576ccc0ae32e2c7dea0/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L312
> >>
> >> Which according to the spec, SizeOfImage is the size of the image as
> >> loaded into memory and must be a multiple of section alignment.
> >>
> >> So, reflecting on this, let me test with VirtualSize here, I think
> >> that is the right value to use, the only time we would have a
> >> SizeOfRawData that is greater than the VirtualSize would be if our
> >> FileAlignment is greater than our SectionAlignment, which would be
> >> a misconfiguration.
> >>
> >> I do think the ImageLoader should also be fixed to only allocate
> >> ImageSize number of pages (which should be the sum of the section
> >> VirtualSizes + any headers that aren't included). Might as well not
> >> waste an extra page for each image and then our image record code is
> >> simpler as well (ensuring we are protecting the right pages).
> >>
> >> I think this patch series should go in, as it is fixing an active bug,
> >> but I will also take a look at the image loader creating the image
> >> records and having a protocol it produces to retrieve the list, to
> >> attempt to avoid issues like this in the future.
> >>
> >
> > Surprisingly, I am seeing that the VirtualSize is not 64k aligned there.
> > I am looking deeper into GenFw to make sure it is correctly getting set
> > to align to SectionAlignment in the section headers. When I use dumpbin
> > to dump the headers, it shows each section having VirtualSize as 64k
> > aligned for a runtime image, but the same image doesn't show that in FW.
> >
> > I'll do some digging here.
> >
>
> Following up on this:
>
> Not surprisingly, different toolchains do different things here.
>
> gcc obviously creates ELF files and ElfConvert*.c converts these to PE
> images. However, when it does this, it always sets the section and file
> alignment to the same value (I'm not as familiar with ELF images, I'm
> not sure if there is the same concept as file vs section alignment).
> GenFw could probably update this information to shrink the file
> alignment, but that's a space optimization for gcc built binaries.
>
> In ElfConvert.c, the VirtualSize does get set to the section aligned
> value, which is what I would expect from the spec.
>
> In MSVC PE images, VirtualSize is not section aligned and the
> expectation appears to be that loaders will align the VirtualSize
> to the section alignment. I am planning on reaching out to the MSVC
> folks to learn why this is the case, is it intended, etc., as my
> understanding is that the VirtualSize in the section headers should
> be section aligned.
>
> We are stuck with this for existing MSVC toolchains, however, so we
> will need to align either the VirtualSize or SizeOfRawData to the
> section alignment when we create the image records. I don't have a
> preference between the two, they e

Re: [edk2-devel] [PATCH v2 1/3] MdeModulePkg: ImagePropertiesRecordLib: Use SectionAlignment for CodeSize

[Ard Biesheuvel]

On Mon, 11 Mar 2024 at 22:29, Oliver Smith-Denny
 wrote:
>
> When an ImageRecord is stored by ImagePropertiesRecordLib, it reports the
> CodeSegmentSize as the SizeOfRawData from the image. However, the image
> as loaded into memory is aligned to the SectionAlignment, so
> SizeOfRawData is under the actual size in memory. This is important,
> because the memory attributes table uses these image records to create
> its entries and it will report that the alignment of an image is
> incorrect, even though the actual image is correct.
>
> This was discovered on ARM64, which has a 64k runtime page granularity
> alignment, which is backed by a 64k section alignment for
> DXE_RUNTIME_DRIVERs. The runtime code and data was correctly being
> loaded into memory, however the memory attribute table was incorrectly
> reporting misaligned ranges to the OS, causing attributes to be
> ignored for these sections for OSes using greater than 4k pages.
>
> This patch correctly aligns the CodeSegmentSize to the SectionAlignment
> and the corresponding memory attribute table entries are now correctly
> aligned and pointing to the right places in memory.
>
> Cc: Liming Gao 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Taylor Beebe 
>
> Signed-off-by: Oliver Smith-Denny 
> ---
>  MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c | 4 
> +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

Reviewed-by: Ard Biesheuvel 

> diff --git 
> a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c 
> b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
> index e53ce086c54c..763a8d65d565 100644
> --- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
> +++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
> @@ -1090,7 +1090,9 @@ CreateImagePropertiesRecord (
>ImageRecordCodeSection->Signature = 
> IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
>
>ImageRecordCodeSection->CodeSegmentBase = (UINTN)ImageBase + 
> Section[Index].VirtualAddress;
> -  ImageRecordCodeSection->CodeSegmentSize = Section[Index].SizeOfRawData;
> +  // We still need to align the VirtualSize to the SectionAlignment 
> because MSVC does not do
> +  // this when creating a PE image. It expects the loader to do this.
> +  ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE 
> (Section[Index].Misc.VirtualSize, SectionAlignment);
>
>InsertTailList (&ImageRecord->CodeSegmentList, 
> &ImageRecordCodeSection->Link);
>ImageRecord->CodeSegmentCount++;
> --
> 2.40.1
>


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

[edk2-devel] [PATCH 0/2] ArmPkg/MdePkg: Move Chipset/* files to MdePkg

[PierreGondois]

This patch relies on [1].

Following the RFC v1: ArmPkg,MdePkg: move ArmLib.h to MdePkg [1],
move the Chipset/* files to the MdePkg as the Armlib.h relies on
them.

These patches span over multiple packages as these Chipset/* files
are relocated to a new directory and include paths must be updated.

[1] https://edk2.groups.io/g/devel/message/111566

Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Pierre Gondois 
Cc: Sami Mujawar 
Cc: Zhiguang Liu 

Pierre Gondois (2):
  ArmPkg,MdePkg: Move ArmPkg/Chipset/ArmV7[|Mmu].h to MdePkg
  ArmPkg,MdePkg: Move ArmPkg/Chipset/Aarch64[|Mmu].h to MdePkg

 ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c | 2 +-
 ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S | 2 +-
 ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c | 2 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c| 2 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S| 2 +-
 ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c  | 2 +-
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c  | 2 +-
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c   | 2 +-
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c  | 2 +-
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c| 2 +-
 ArmPlatformPkg/PrePeiCore/AArch64/Exception.S | 2 +-
 ArmPlatformPkg/PrePeiCore/AArch64/Helper.S| 2 +-
 ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c  | 2 +-
 ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S   | 2 +-
 ArmVirtPkg/PrePi/AArch64/ArchPrePi.c  | 2 +-
 {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h  | 2 +-
 .../Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h   | 0
 .../Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h   | 2 +-
 .../Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h | 0
 MdePkg/Include/Library/ArmLib.h   | 4 ++--
 20 files changed, 19 insertions(+), 19 deletions(-)
 rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h (94%)
 rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h (100%)
 rename ArmPkg/Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h (95%)
 rename ArmPkg/Include/Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h 
(100%)

-- 
2.25.1



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

[edk2-devel] [PATCH 1/2] ArmPkg,MdePkg: Move ArmPkg/Chipset/ArmV7[|Mmu].h to MdePkg

[PierreGondois]

Following the discussion at [1] and as the ArmLib relies on them,
move ArmPkg/Chipset/ArmV7[|Mmu].h files to the MdePkg.

Update the path to correctly include the moved files.

[1] https://edk2.groups.io/g/devel/message/111566

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Gerd Hoffmann 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Pierre Gondois 
---
 ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c   | 2 +-
 ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S   | 2 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c  | 2 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S  | 2 +-
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c| 2 +-
 ArmPlatformPkg/PrePeiCore/AArch64/Exception.S   | 2 +-
 ArmPlatformPkg/PrePeiCore/AArch64/Helper.S  | 2 +-
 ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c| 2 +-
 ArmVirtPkg/PrePi/AArch64/ArchPrePi.c| 2 +-
 {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h| 2 +-
 {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h | 0
 MdePkg/Include/Library/ArmLib.h | 2 +-
 12 files changed, 11 insertions(+), 11 deletions(-)
 rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h (94%)
 rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h (100%)

diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c 
b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
index ef6a132b8dfc..b0c2e39cd910 100644
--- a/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
+++ b/ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c
@@ -10,7 +10,7 @@
 
 #include 
 
-#include 
+#include 
 #include 
 #include  // for MAX_AARCH64_EXCEPTION
 
diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S 
b/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
index cd9437b6aab8..f5cbc2e97c3b 100644
--- a/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
+++ b/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S
@@ -7,7 +7,7 @@
 //
 
//--
 
-#include 
+#include 
 #include 
 #include 
 #include  // for exception type definitions
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
index 87285465871d..6739f5c37d38 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c
@@ -13,7 +13,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include "AArch64Lib.h"
 #include "ArmLibPrivate.h"
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index a7111e51882c..177d10e66d5d 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -9,7 +9,7 @@
 #
 #--
 
-#include 
+#include 
 #include 
 
 .set CTRL_M_BIT,  (1 << 0)
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 9d9c623581fe..6a1f3f947788 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -11,7 +11,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/ArmPlatformPkg/PrePeiCore/AArch64/Exception.S 
b/ArmPlatformPkg/PrePeiCore/AArch64/Exception.S
index ffb643a56df0..d0d6bc44f78c 100644
--- a/ArmPlatformPkg/PrePeiCore/AArch64/Exception.S
+++ b/ArmPlatformPkg/PrePeiCore/AArch64/Exception.S
@@ -5,7 +5,7 @@
 #
 #
 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S 
b/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S
index 2a604b719b26..9b81b96a4949 100644
--- a/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S
+++ b/ArmPlatformPkg/PrePeiCore/AArch64/Helper.S
@@ -6,7 +6,7 @@
 
#===
 
 #include 
-#include 
+#include 
 
 // Setup EL1 while in EL1
 ASM_FUNC(SetupExceptionLevel1)
diff --git a/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c 
b/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c
index 296b029e08bf..27a85049fa78 100644
--- a/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c
+++ b/ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c
@@ -8,7 +8,7 @@
 
 #include "PrePi.h"
 
-#include 
+#include 
 
 VOID
 ArchInitialize (
diff --git a/ArmVirtPkg/PrePi/AArch64/ArchPrePi.c 
b/ArmVirtPkg/PrePi/AArch64/ArchPrePi.c
index 9cab88ca086e..493628b8880d 100644
--- a/ArmVirtPkg/PrePi/AArch64/ArchPrePi.c
+++ b/ArmVirtPkg/PrePi/AArch64/ArchPrePi.c
@@ -8,7 +8,7 @@
 
 #include "PrePi.h"
 
-#include 
+#include 
 
 VOID
 ArchInitialize (
diff --git a/ArmPkg/Include/Chipset/AArch64.h b/MdePkg/Include/AArch64/AArch64.h
similarit

[edk2-devel] [PATCH 2/2] ArmPkg,MdePkg: Move ArmPkg/Chipset/Aarch64[|Mmu].h to MdePkg

[PierreGondois]

Following the discussion at [1] and as the ArmLib relies on them,
move ArmPkg/Chipset/Aarch64[|Mmu].h files to the MdePkg.

Update the path to correctly include the moved files.

[1] https://edk2.groups.io/g/devel/message/111566

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Pierre Gondois 
---
 ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c   | 2 +-
 ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c| 2 +-
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c | 2 +-
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c| 2 +-
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c  | 2 +-
 ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 2 +-
 ArmPkg/Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h  | 2 +-
 .../Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h   | 0
 MdePkg/Include/Library/ArmLib.h | 2 +-
 9 files changed, 8 insertions(+), 8 deletions(-)
 rename ArmPkg/Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h (95%)
 rename ArmPkg/Include/Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h 
(100%)

diff --git a/ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c 
b/ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c
index fc411b845d64..7652b97b43c7 100644
--- a/ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c
+++ b/ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c
@@ -11,7 +11,7 @@
 
 #include 
 
-#include 
+#include 
 
 #include 
 
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c 
b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
index 521d5be0de33..6acc4d3e7cc5 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
+++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c
@@ -13,7 +13,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #include "ArmV7Lib.h"
 #include "ArmLibPrivate.h"
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c 
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
index 52dbfd714029..bf3a874822ac 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c
@@ -11,7 +11,7 @@
 
 #include 
 
-#include 
+#include 
 
 UINT32
 ConvertSectionAttributesToPageAttributes (
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 28e4cd9f1a77..60dc6c987cbd 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -8,7 +8,7 @@
 **/
 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c 
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
index 548ee1303870..5e751cddd7d6 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c
@@ -17,7 +17,7 @@
 #include 
 #include 
 
-#include 
+#include 
 
 #define __EFI_MEMORY_RWX  0 // no restrictions
 
diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S 
b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
index 6709dad0b9d1..60e530e4f1c5 100644
--- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
+++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
@@ -7,7 +7,7 @@
 
 #include 
 
-#include 
+#include 
 
 ASM_FUNC(_ModuleEntryPoint)
   // Do early platform specific actions
diff --git a/ArmPkg/Include/Chipset/ArmV7.h b/MdePkg/Include/Arm/AArch32.h
similarity index 95%
rename from ArmPkg/Include/Chipset/ArmV7.h
rename to MdePkg/Include/Arm/AArch32.h
index 94620c087df2..e7e6e4dcd0a2 100644
--- a/ArmPkg/Include/Chipset/ArmV7.h
+++ b/MdePkg/Include/Arm/AArch32.h
@@ -10,7 +10,7 @@
 #ifndef ARM_V7_H_
 #define ARM_V7_H_
 
-#include 
+#include 
 
 // ARM Interrupt ID in Exception Table
 #define ARM_ARCH_EXCEPTION_IRQ  EXCEPT_ARM_IRQ
diff --git a/ArmPkg/Include/Chipset/ArmV7Mmu.h b/MdePkg/Include/Arm/AArch32Mmu.h
similarity index 100%
rename from ArmPkg/Include/Chipset/ArmV7Mmu.h
rename to MdePkg/Include/Arm/AArch32Mmu.h
diff --git a/MdePkg/Include/Library/ArmLib.h b/MdePkg/Include/Library/ArmLib.h
index 852264dba289..71c2076652fa 100644
--- a/MdePkg/Include/Library/ArmLib.h
+++ b/MdePkg/Include/Library/ArmLib.h
@@ -14,7 +14,7 @@
 #include 
 
 #ifdef MDE_CPU_ARM
-  #include 
+  #include 
 #elif defined (MDE_CPU_AARCH64)
   #include 
 #else
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116675): https://edk2.groups.io/g/devel/message/116675
Mute This Topic: https://groups.io/mt/104881292/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-platforms 1/1] Platform/RPI4: Grow FV size to accommodate DEBUG build

[Ard Biesheuvel]

On Tue, 12 Mar 2024 at 11:29, Ard Biesheuvel  wrote:
>
> From: Ard Biesheuvel 
>
> The DEBUG build no longer fits when all build options used by the
> release script on github.com/pftf are used, presumably due to the
> OpenSSL upgrade.
>
> So bump the size for all builds.
>
> Signed-off-by: Ard Biesheuvel 
> ---
>  Platform/RaspberryPi/RPi4/RPi4.fdf | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf 
> b/Platform/RaspberryPi/RPi4/RPi4.fdf
> index 816927761513..951488260ed4 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.fdf
> +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
> @@ -25,11 +25,11 @@
>
>  [FD.RPI_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress
> -Size  = 0x001f|gArmTokenSpaceGuid.PcdFdSize
> +Size  = 0x0021|gArmTokenSpaceGuid.PcdFdSize
>  ErasePolarity = 1
>

Ugh just realized that this breaks DT loading from TFA, which places
the DT at a fixed offset of 0x1f


>  BlockSize = 0x1000|gRaspberryPiTokenSpaceGuid.PcdFirmwareBlockSize
> -NumBlocks = 0x1f0
> +NumBlocks = 0x210
>
>  
> 
>  #
> @@ -56,7 +56,7 @@ [FD.RPI_EFI]
>  #
>  # UEFI image
>  #
> -0x0002|0x001b
> +0x0002|0x001d
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>
> @@ -70,7 +70,7 @@ [FD.RPI_EFI]
>  #
>
>  # NV_VARIABLE_STORE
> -0x001d|0xe000
> +0x001f|0xe000
>  
> gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>
>  DATA = {
> @@ -113,11 +113,11 @@ [FD.RPI_EFI]
>  }
>
>  # NV_EVENT_LOG
> -0x001de000|0x1000
> +0x001fe000|0x1000
>  
> gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogBase|gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize
>
>  # NV_FTW_WORKING header
> -0x001df000|0x1000
> +0x001ff000|0x1000
>  
> gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>
>  DATA = {
> @@ -132,7 +132,7 @@ [FD.RPI_EFI]
>  }
>
>  # NV_FTW_WORKING data
> -0x001e|0x0001
> +0x0020|0x0001
>  
> gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>
>  #
> --
> 2.44.0.278.ge034bb2e1d-goog
>


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

Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/QemuBootOrderLib: Measure the etc/boot-menu-wait

[Gerd Hoffmann]

On Wed, Mar 13, 2024 at 07:51:46AM +0800, Ceping Sun wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4415
> 
> Refer to the section 8.3.4 of tdx-virtual-firmware-design-guide spec,
> OVMF would uses FW_CFG_IO_SELECTOR(0x510) and FW_CFG_IO_DATA(0x511)
> to get configuration data from QEMU. From the security perspective,
> if TDVF uses this method, configuration data must be measured into
> RTMR[0].
> 
> Currently, the etc/boot-menu-wait is using in TDVF, it required to be
> measured into RTMR[0].

That config item doesn't change the control flow.
Do we have to measure it?

> This is the first patch and will continue to be updated to measure
> additional configuration data.

What else is in the pipeline?  At least ACPI and smbios tables I assume?

I'd like to have a more complete picture first.  Also I think it makes
sense to have a single patch series implementing all of it instead of
merging it piece by piece, to avoid having multiple edk2 releases where
the measurements are changing.

Note that the current code (looking at a non-tdx build) reads several
fw_cfg items multiple times.  Entries 0 and 1 (used for probing fw_cfg
presence), 0x19 (file directory) are read most frequently.  etc/e820 is
scanned multiple times too; tvdf in tdx mode wouldn't use it though.

If we are going to measure the fw_cfg bits used by ovmf / tdvf I think
we have to:

  (1) Make sure we read + measure the data once.
  (2) Make sure we measure the fw_cfg entries in a deterministic
  order so the measurements are stable.
  (3) Cache the measured data somewhere if needed multiple times
  (or simply cache unconditionally).

We probably wouldn't measure all fw_cfg entries.  The ones used by
direct kernel boot can be skipped for example.  The kernel image will
be measured anyway before it is launched.

> +#define EV_POSTCODE_INFO_QEMU_BOOTMENU_WAIT_TIME_DATA  "QEMU BOOTMENU WAIT 
> TIME"

"QEMU FW CFG" ?

I think it makes sense to have one name and one struct for all qemu
fw_cfg items.  Or maybe two, one for the file-name based entries and
one for the others.

take care,
  Gerd



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

Re: [edk2-devel][PATCH v1 1/3] MdeModulePkg: ImagePropertiesRecordLib: Use SectionAlignment for CodeSize

[Oliver Smith-Denny]

On 3/12/2024 1:32 AM, Ard Biesheuvel wrote:

On Mon, 11 Mar 2024 at 20:34, Oliver Smith-Denny
 wrote:


On 3/4/2024 2:38 PM, Oliver Smith-Denny wrote:

On 3/4/2024 11:24 AM, Oliver Smith-Denny wrote:

On 3/4/2024 10:54 AM, Ard Biesheuvel wrote:

On Mon, 4 Mar 2024 at 18:49, Oliver Smith-Denny
 wrote:


Hi Ard,

On 3/1/2024 3:58 AM, Ard Biesheuvel wrote:

Hi Oliver,

On Tue, 27 Feb 2024 at 21:27, Oliver Smith-Denny
 wrote:


-  ImageRecordCodeSection->CodeSegmentSize =
Section[Index].SizeOfRawData;
+  ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE
(Section[Index].SizeOfRawData, SectionAlignment);



This should be the virtual size, not the file size, right?


Correct, SectionAlignment is the alignment of the image as loaded in
memory, so in the case of a DXE runtime driver on ARM64, it will be
64k.



No, I mean we should not be using SizeOfRawData here but VirtualSize.

I understand this is unlikely to make a difference in practice, but
VirtualSize may exceed SizeOfRawData by a wide margin, and we need the
whole region to be covered.



I see, yes I do believe VirtualSize could be used here instead. Two
things give me pause. One is that the PE spec states that SizeOfRawData
is rounded and VirtualSize is not, so that SizeOfRawData may be greater
than the VirtualSize in some cases (which seems incorrect).

The other is that the image loader partially uses VirtualSize when
loading:

https://github.com/tianocore/edk2/blob/918288ab5a7c3abe9c58d576ccc0ae32e2c7dea0/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L1399-L1400

However, when determining the size of a loaded image (and therefore the
number of pages to allocate) it will allocate an extra page:

https://github.com/tianocore/edk2/blob/918288ab5a7c3abe9c58d576ccc0ae32e2c7dea0/MdeModulePkg/Core/Dxe/Image/Image.c#L646-L652

as ImageSize here is:

https://github.com/tianocore/edk2/blob/918288ab5a7c3abe9c58d576ccc0ae32e2c7dea0/MdePkg/Library/BasePeCoffLib/BasePeCoff.c#L312

Which according to the spec, SizeOfImage is the size of the image as
loaded into memory and must be a multiple of section alignment.

So, reflecting on this, let me test with VirtualSize here, I think
that is the right value to use, the only time we would have a
SizeOfRawData that is greater than the VirtualSize would be if our
FileAlignment is greater than our SectionAlignment, which would be
a misconfiguration.

I do think the ImageLoader should also be fixed to only allocate
ImageSize number of pages (which should be the sum of the section
VirtualSizes + any headers that aren't included). Might as well not
waste an extra page for each image and then our image record code is
simpler as well (ensuring we are protecting the right pages).

I think this patch series should go in, as it is fixing an active bug,
but I will also take a look at the image loader creating the image
records and having a protocol it produces to retrieve the list, to
attempt to avoid issues like this in the future.



Surprisingly, I am seeing that the VirtualSize is not 64k aligned there.
I am looking deeper into GenFw to make sure it is correctly getting set
to align to SectionAlignment in the section headers. When I use dumpbin
to dump the headers, it shows each section having VirtualSize as 64k
aligned for a runtime image, but the same image doesn't show that in FW.

I'll do some digging here.



Following up on this:

Not surprisingly, different toolchains do different things here.

gcc obviously creates ELF files and ElfConvert*.c converts these to PE
images. However, when it does this, it always sets the section and file
alignment to the same value (I'm not as familiar with ELF images, I'm
not sure if there is the same concept as file vs section alignment).
GenFw could probably update this information to shrink the file
alignment, but that's a space optimization for gcc built binaries.

In ElfConvert.c, the VirtualSize does get set to the section aligned
value, which is what I would expect from the spec.

In MSVC PE images, VirtualSize is not section aligned and the
expectation appears to be that loaders will align the VirtualSize
to the section alignment. I am planning on reaching out to the MSVC
folks to learn why this is the case, is it intended, etc., as my
understanding is that the VirtualSize in the section headers should
be section aligned.

We are stuck with this for existing MSVC toolchains, however, so we
will need to align either the VirtualSize or SizeOfRawData to the
section alignment when we create the image records. I don't have a
preference between the two, they end up being the same when we align
them, so I can send a v2 with aligning the VirtualSize. This will be
a no-op on gcc built binaries and will set it to the correct value for
MSVC.



I don't think this is correct. The VirtualSize could be much larger
than the SizeOfRawData (to the extent where the delta exceeds the
alignment padding), in cases where some of the memory is
data-initialized and some of it is zero-initialized. 

[edk2-devel] [PATCH 0/4] Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[Qingyu]

Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
EFI_UNSUPPORTED to EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().

This patch series update ReadKeyStroke and ReadKeyStrokeEx implementation
in Edk2.

Qingyu (4):
  MdeModulePkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx
  ShellPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx
  EmbeddedPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx
  EmulatorPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx

 .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.c   | 10 ++
 .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.h   | 10 ++
 EmulatorPkg/EmuGopDxe/GopInput.c   |  2 ++
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c |  4 +++-
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  |  3 +++
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c |  2 ++
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h |  2 ++
 .../Universal/Console/ConSplitterDxe/ConSplitter.c |  6 ++
 .../Universal/Console/ConSplitterDxe/ConSplitter.h |  6 ++
 MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h  |  4 
 .../Universal/Console/TerminalDxe/TerminalConIn.c  |  4 
 ShellPkg/Application/Shell/ConsoleWrappers.c   |  4 +++-
 12 files changed, 47 insertions(+), 10 deletions(-)

Cc: Ming Tan 
Cc: Yi Li 
Cc: Gahan 
Signed-off-by: Qingyu 

-- 
2.39.1.windows.1



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

[edk2-devel] [PATCH 4/4] EmulatorPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[Qingyu]

Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
EFI_UNSUPPORTED to EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().

Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Michael D Kinney 
Signed-off-by: Qingyu 
---
 EmulatorPkg/EmuGopDxe/GopInput.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/EmulatorPkg/EmuGopDxe/GopInput.c b/EmulatorPkg/EmuGopDxe/GopInput.c
index a48293912d5a..94a4fcf26747 100644
--- a/EmulatorPkg/EmuGopDxe/GopInput.c
+++ b/EmulatorPkg/EmuGopDxe/GopInput.c
@@ -156,6 +156,7 @@ EmuGopSimpleTextInReset (
   @retval EFI_NOT_READYThere was no keystroke data available.
   @retval EFI_DEVICE_ERROR The keystroke information was not returned due to
hardware errors.
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read 
keystroke data.
 
 **/
 EFI_STATUS
@@ -339,6 +340,7 @@ EmuGopSimpleTextInExResetEx (
   EFI_DEVICE_ERROR The keystroke
   information was not returned due to
   hardware errors.
+  @retval EFI_UNSUPPORTED The device does not support the ability to read 
keystroke data.
 
 
 **/
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116684): https://edk2.groups.io/g/devel/message/116684
Mute This Topic: https://groups.io/mt/104886874/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] BaseTools: VfrCompiler Adds DefaultValueError

[Zhang, Zifeng]

Thanks for sharing.

Best Regards,
Zifeng

-Original Message-
From: gaoliming  
Sent: Friday, March 8, 2024 10:47 PM
To: Zhang, Zifeng ; Yang, Yuting2 
; devel@edk2.groups.io
Cc: 'Rebecca Cran' ; Feng, Bob C ; 
Chen, Christine ; Chen, Arthur G 
Subject: 回复: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError

Zifeng:
  For Structure PCD,  https://edk2.groups.io/g/devel/message/18432 is the 
summary of code change. 

  Below is two wikis about its usage. 
  https://github.com/lgao4/edk2/wiki/StrucutrePcd-Usage
  https://github.com/lgao4/edk2/wiki/StructurePcd-Enable-Steps
  
  For the patch table configuration, the structure PCD can be defined to map 
the patch table structure. 
  Then, the patch table value can be specified in DSC in the different SkuId
(boardId) to support multiple SKUs.
  Structure PCD can make DSC as the centralized way for the platform 
configuration. 
  The developer doesn't need to maintain the board setting in C source file.


Thanks
Liming
> -邮件原件-
> 发件人: Zhang, Zifeng 
> 发送时间: 2024年3月7日 14:54
> 收件人: Yang, Yuting2 ; devel@edk2.groups.io
> 抄送: Rebecca Cran ; Liming Gao 
> ; Feng, Bob C ; Chen, 
> Christine ; Chen, Arthur G 
> 
> 主题: RE: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError
> 
> Hi Liming,
> 
> Could you help to review the patch for VFR compiler change submitted 
> by Yuting?
> 
> Btw,
> Dell need some introduction of Hii StructurePCD implementation 
> especially for replacing patch table configurations.
> Would you like to share EDK2 wiki link or doc for it?
> 
> Best Regards,
> Zifeng
> 
> 
> -Original Message-
> From: Yang, Yuting2 
> Sent: Friday, January 26, 2024 10:54 AM
> To: devel@edk2.groups.io
> Cc: Rebecca Cran ; Liming Gao 
> ; Feng, Bob C ; Chen, 
> Christine ; Zhang, Zifeng 
> 
> Subject: [Patch V3] BaseTools: VfrCompiler Adds DefaultValueError
> 
> Add --catch_default option to raise a DefaultValueError when 
> encountering VFR default definitions to help remove default variables.
> 
> Signed-off-by: Yuting Yang 
> 
> Cc: Rebecca Cran 
> Cc: Liming Gao 
> Cc: Bob Feng 
> Cc: Christine Chen 
> Cc: Zifeng Zhang 
> Signed-off-by: Yuting Yang 
> ---
>  BaseTools/Source/C/VfrCompile/VfrCompiler.cpp |   8 +-
>  BaseTools/Source/C/VfrCompile/VfrCompiler.h   |   1 +
>  BaseTools/Source/C/VfrCompile/VfrError.cpp|   3 +-
>  BaseTools/Source/C/VfrCompile/VfrError.h  |   3 +-
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.h|   1 +
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 238 ++
>  6 files changed, 150 insertions(+), 104 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> index 5f4d262d85..4031af6e39 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.cpp
> @@ -78,6 +78,7 @@ CVfrCompiler::OptionInitialization (
>mOptions.WarningAsError= FALSE;
> mOptions.AutoDefault   = FALSE;
> mOptions.CheckDefault  = FALSE;+
> mOptions.IsCatchDefaultEnable  = FALSE;   memset
> (&mOptions.OverrideClassGuid, 0, sizeof (EFI_GUID));if (Argc == 1) {@@
> -95,6 +96,8 @@ CVfrCompiler::OptionInitialization (
>Version ();   SET_RUN_STATUS (STATUS_DEAD);
> return;+} else if (stricmp(Argv[Index], "--catch_default") == 0){+
> mOptions.IsCatchDefaultEnable = TRUE; } else if (stricmp(Argv[Index],
> "-l") == 0) {   mOptions.CreateRecordListFile = TRUE;
> gCIfrRecordInfoDB.TurnOn ();@@ -179,7 +182,6 @@ 
> CVfrCompiler::OptionInitialization (
>goto Fail; } strcpy (mOptions.VfrFileName, Argv[Index]);-
> if (mOptions.OutputDirectory == NULL) {   mOptions.OutputDirectory =
> (CHAR8 *) malloc (1);   if (mOptions.OutputDirectory == NULL) {@@
> -679,7 +681,7 @@ CVfrCompiler::Compile (
>  DebugError (NULL, 0, 0001, "Error opening the input file", "%s",
> InFileName); goto Fail;   }-+  InputInfo.IsCatchDefaultEnable =
> mOptions.IsCatchDefaultEnable;   if (mOptions.HasOverrideClassGuid)
> { InputInfo.OverrideClassGuid = &mOptions.OverrideClassGuid;   } else
> {@@ -937,5 +939,3 @@ main (
> return GetUtilityStatus (); }--diff --git 
> a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> index b6e207d2ce..974f37c4eb 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> +++ b/BaseTools/Source/C/VfrCompile/VfrCompiler.h
> @@ -52,6 +52,7 @@ typedef struct {
>BOOLEAN WarningAsError;   BOOLEAN AutoDefault;   BOOLEAN
> CheckDefault;+  BOOLEAN IsCatchDefaultEnable; } OPTIONS;  typedef enum 
> {diff --git a/BaseTools/Source/C/VfrCompile/VfrError.cpp
> b/BaseTools/Source/C/VfrCompile/VfrError.cpp
> index 65bb8e34fd..8a706f929b 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrError.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrError.cpp
> @@ -49,7 +49,8 @@ static SVFR_WARNING_HANDLE VFR_WARNING_HANDLE_TABLE 
> [] = {
>{ VFR_WARNING_DEFAU

[edk2-devel] [PATCH 3/4] EmbeddedPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[Qingyu]

Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
EFI_UNSUPPORTED to EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Abner Chang 
Cc: Liming Gao 
Signed-off-by: Qingyu 
---
 .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.c   | 10 ++
 .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.h   | 10 ++
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c 
b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
index 4bbc3ead2c87..48a99566df63 100644
--- a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
+++ b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.c
@@ -694,11 +694,12 @@ KeyboardReadKeyStrokeWorker (
 /**
   Read out the scan code of the key that has just been stroked.
 
-  @param  ThisPointer of simple text Protocol.
-  @param  Key Pointer for store the key that read out.
+  @param  This  Pointer of simple text Protocol.
+  @param  Key   Pointer for store the key that read out.
 
-  @retval EFI_SUCCESS The key is read out successfully.
-  @retval other   The key reading failed.
+  @retval EFI_SUCCESS   The key is read out successfully.
+  @retval other The key reading failed.
+  @retval EFI_UNSUPPORTED   The device does not support the ability to read 
keystroke data.
 
 **/
 EFI_STATUS
@@ -752,6 +753,7 @@ VirtualKeyboardReadKeyStroke (
   @retval  EFI_DEVICE_ERROR  The keystroke information was not returned
  due to hardware errors.
   @retval  EFI_INVALID_PARAMETER KeyData is NULL.
+  @retval  EFI_UNSUPPORTED   The device does not support the ability to 
read keystroke data.
 
 **/
 EFI_STATUS
diff --git a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.h 
b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.h
index f72bd6f9c6cc..25063c262721 100644
--- a/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.h
+++ b/EmbeddedPkg/Drivers/VirtualKeyboardDxe/VirtualKeyboard.h
@@ -496,11 +496,12 @@ KeyNotifyProcessHandler (
 /**
   Read out the scan code of the key that has just been stroked.
 
-  @param  ThisPointer of simple text Protocol.
-  @param  Key Pointer for store the key that read out.
+  @param  This  Pointer of simple text Protocol.
+  @param  Key   Pointer for store the key that read out.
 
-  @retval EFI_SUCCESS The key is read out successfully.
-  @retval other   The key reading failed.
+  @retval EFI_SUCCESS   The key is read out successfully.
+  @retval other The key reading failed.
+  @retval EFI_UNSUPPORTED   The device does not support the ability to read 
keystroke data.
 
 **/
 EFI_STATUS
@@ -523,6 +524,7 @@ VirtualKeyboardReadKeyStroke (
   @retval  EFI_DEVICE_ERROR  The keystroke information was not returned 
due to
  hardware errors.
   @retval  EFI_INVALID_PARAMETER KeyData is NULL.
+  @retval  EFI_UNSUPPORTED   The device does not support the ability to 
read keystroke data.
 
 **/
 EFI_STATUS
-- 
2.39.1.windows.1



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

[edk2-devel] [PATCH 1/4] MdeModulePkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[Qingyu]

Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
EFI_UNSUPPORTED to EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().

Cc: Liming Gao 
Cc: Michael D Kinney 
Signed-off-by: Qingyu 
---
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c  | 4 +++-
 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   | 3 +++
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c  | 2 ++
 MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h  | 2 ++
 MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c | 6 ++
 MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.h | 6 ++
 MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h   | 4 
 MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c  | 4 
 8 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c 
b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
index b1ab17af3788..81d3c6eb70ab 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
@@ -258,7 +258,8 @@ KeyboardEfiReset (
   @param ThisPointer to instance of EFI_SIMPLE_TEXT_INPUT_PROTOCOL
   @param Key The output buffer for key value
 
-  @retval EFI_SUCCESS success to read key stroke
+  @retval EFI_SUCCESS  success to read key stroke
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read 
keystroke data.
 **/
 EFI_STATUS
 EFIAPI
@@ -433,6 +434,7 @@ KeyboardEfiResetEx (
 @retval EFI_DEVICE_ERROR  The keystroke information was not returned 
due to
   hardware errors.
 @retval EFI_INVALID_PARAMETER KeyData is NULL.
+@retval EFI_UNSUPPORTED   The device does not support the ability to 
read keystroke data.
 
 **/
 EFI_STATUS
diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h 
b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
index ca1dd9b2c2c6..7b4db9c778de 100644
--- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
+++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
@@ -338,6 +338,7 @@ KeyboardEfiReset (
   @param Key The output buffer for key value
 
   @retval EFI_SUCCESS success to read key stroke
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read 
keystroke data.
 **/
 EFI_STATUS
 EFIAPI
@@ -441,6 +442,8 @@ KeyboardEfiResetEx (
 @retval EFI_DEVICE_ERROR  - The keystroke information was not returned 
due to
 hardware errors.
 @retval EFI_INVALID_PARAMETER - KeyData is NULL.
+@retval EFI_UNSUPPORTED   - The device does not support the ability to 
read
+keystroke data.
 
 **/
 EFI_STATUS
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c 
b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
index e889f422bbf7..bcda0724ee76 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c
@@ -692,6 +692,7 @@ USBKeyboardReset (
   @retval EFI_NOT_READYThere was no keystroke data available.
   @retval EFI_DEVICE_ERROR The keystroke information was not returned due 
to
hardware errors.
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read 
keystroke data.
 
 **/
 EFI_STATUS
@@ -975,6 +976,7 @@ USBKeyboardResetEx (
   @retval EFI_DEVICE_ERROR   The keystroke information was not returned 
due to
  hardware errors.
   @retval EFI_INVALID_PARAMETER  KeyData is NULL.
+  @retval EFI_UNSUPPORTEDThe device does not support the ability to 
read keystroke data.
 
 **/
 EFI_STATUS
diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h 
b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
index a9dfeafd6f10..b9e9a725ee33 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h
@@ -412,6 +412,7 @@ USBKeyboardReset (
   @retval EFI_NOT_READYThere was no keystroke data available.
   @retval EFI_DEVICE_ERROR The keystroke information was not returned due 
to
hardware errors.
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read 
keystroke data.
 
 **/
 EFI_STATUS
@@ -466,6 +467,7 @@ USBKeyboardResetEx (
   @retval EFI_DEVICE_ERROR   The keystroke information was not returned 
due to
  hardware errors.
   @retval EFI_INVALID_PARAMETER  KeyData is NULL.
+  @retval EFI_UNSUPPORTEDThe device does not support the ability to 
read keystroke data.
 
 **/
 EFI_STATUS
diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c 
b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
index 8b5e62e3a883..0a776f369b9a 100644
--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
@@ -3551,6 +3551,8 @@ ConSplitterTextInExDequeueKey (
   @retval EF

[edk2-devel] [PATCH 2/4] ShellPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[Qingyu]

Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
EFI_UNSUPPORTED to EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().

Cc: Liming Gao 
Cc: Zhichao Gao 
Cc: Chao Li 
Signed-off-by: Qingyu 
---
 ShellPkg/Application/Shell/ConsoleWrappers.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/ConsoleWrappers.c 
b/ShellPkg/Application/Shell/ConsoleWrappers.c
index eae11370e4e0..dbffae30cdf5 100644
--- a/ShellPkg/Application/Shell/ConsoleWrappers.c
+++ b/ShellPkg/Application/Shell/ConsoleWrappers.c
@@ -67,7 +67,9 @@ FileBasedSimpleTextInReset (
   @param[in] This  A pointer to the SimpleTextIn structure.
   @param[in, out] Key  A pointer to the Key structure to fill.
 
-  @retval   EFI_SUCCESS The read was successful.
+  @retval EFI_SUCCESS  The read was successful.
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read
+   keystroke data.
 **/
 EFI_STATUS
 EFIAPI
-- 
2.39.1.windows.1



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

[edk2-devel] [PATCH edk2-platforms 1/1] Platform/RPI4: Grow FV size to accommodate DEBUG build

[Ard Biesheuvel via groups.io]

From: Ard Biesheuvel 

The DEBUG build no longer fits when all build options used by the
release script on github.com/pftf are used, presumably due to the
OpenSSL upgrade.

So bump the size for all builds.

Signed-off-by: Ard Biesheuvel 
---
 Platform/RaspberryPi/RPi4/RPi4.fdf | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf 
b/Platform/RaspberryPi/RPi4/RPi4.fdf
index 816927761513..951488260ed4 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -25,11 +25,11 @@
 
 [FD.RPI_EFI]
 BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress
-Size  = 0x001f|gArmTokenSpaceGuid.PcdFdSize
+Size  = 0x0021|gArmTokenSpaceGuid.PcdFdSize
 ErasePolarity = 1
 
 BlockSize = 0x1000|gRaspberryPiTokenSpaceGuid.PcdFirmwareBlockSize
-NumBlocks = 0x1f0
+NumBlocks = 0x210
 
 

 #
@@ -56,7 +56,7 @@ [FD.RPI_EFI]
 #
 # UEFI image
 #
-0x0002|0x001b
+0x0002|0x001d
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
@@ -70,7 +70,7 @@ [FD.RPI_EFI]
 #
 
 # NV_VARIABLE_STORE
-0x001d|0xe000
+0x001f|0xe000
 
gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
 
 DATA = {
@@ -113,11 +113,11 @@ [FD.RPI_EFI]
 }
 
 # NV_EVENT_LOG
-0x001de000|0x1000
+0x001fe000|0x1000
 
gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogBase|gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize
 
 # NV_FTW_WORKING header
-0x001df000|0x1000
+0x001ff000|0x1000
 
gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
 
 DATA = {
@@ -132,7 +132,7 @@ [FD.RPI_EFI]
 }
 
 # NV_FTW_WORKING data
-0x001e|0x0001
+0x0020|0x0001
 
gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
 #
-- 
2.44.0.278.ge034bb2e1d-goog



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

[edk2-devel] [PATCH] IntelFsp2WrapperPkg: Error handling of TpmMeasureAndLogDataWithFlags()

[Du Lin]

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

TpmMeasureAndLogDataWithFlags() computes the measure the code and
log it into PCR 0. TpmMeasureAndLogData() computes the hash for the
configuration. The same "Status" variable is used to store the return
values for both of the functions. There is no error handling if
TpmMeasureAndLogDataWithFlags() returns an error Status.
Fix the issue by adding error handling for TpmMeasureAndLogDataWithFlags().

Signed-off-by: Du Lin 
Cc: Ashraf Ali S 
Cc: Chasel Chiu 
Cc: Chen Gang C 
Cc: Duggapu Chinni B 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Susovan Mohapatra 
Cc: Ted Kuo 
---
 .../Library/BaseFspMeasurementLib/FspMeasurementLib.c | 4 
 1 file changed, 4 insertions(+)

diff --git 
a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c 
b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
index 2c017a4250..228277649b 100644
--- a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
@@ -197,6 +197,10 @@ MeasureFspFirmwareBlobWithCfg (
  (UINTN)sizeof (DigestList),
  EDKII_TCG_PRE_HASH_LOG_ONLY
  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "TpmMeasureAndLogDataWithFlags failed - %r\n", 
Status));
+return Status;
+  }
 
   Status = TpmMeasureAndLogData (
  1,
-- 
2.26.2.windows.1



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

[edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

[Ard Biesheuvel via groups.io]

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.

Signed-off-by: Ard Biesheuvel 
---
This fixes a regression on Raspberry Pi4, which no longer boots.

 EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c 
b/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
index 4e7a5698c162..f02a76a62ea8 100644
--- a/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
+++ b/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
@@ -70,7 +70,7 @@ NonCoherentIoMmuSetAttribute (
   IN UINT64IoMmuAccess
   )
 {
-  return EFI_UNSUPPORTED;
+  return EFI_SUCCESS;
 }
 
 /**
-- 
2.44.0.278.ge034bb2e1d-goog



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

[edk2-devel] [PATCH] IntelFsp2WrapperPkg: Error handling of FspmWrapperInit()

[Du Lin]

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

The error handling of FspmWrapperInit() is limited to ASSERT
statements only, which only works in debug builds, but not in
release builds.
Fix the issue by enhancing the error handling of FspmWrapperInit()
to cover both debug builds and release builds.

Signed-off-by: Du Lin 
Cc: Ashraf Ali S 
Cc: Chasel Chiu 
Cc: Chen Gang C 
Cc: Duggapu Chinni B 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Susovan Mohapatra 
Cc: Ted Kuo 
---
 .../FspmWrapperPeim/FspmWrapperPeim.c| 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c 
b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index ba0c742fea..356baeeccf 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -196,13 +196,21 @@ FspmWrapperInit (
   EFI_PEI_PPI_DESCRIPTOR 
*MeasurementExcludedPpiList;
 
   MeasurementExcludedFvPpi = AllocatePool (sizeof (*MeasurementExcludedFvPpi));
-  ASSERT (MeasurementExcludedFvPpi != NULL);
+  if (MeasurementExcludedFvPpi == NULL) {
+ASSERT (FALSE);
+return EFI_OUT_OF_RESOURCES;
+  }
+
   MeasurementExcludedFvPpi->Count  = 1;
   MeasurementExcludedFvPpi->Fv[0].FvBase   = PcdGet32 (PcdFspmBaseAddress);
   MeasurementExcludedFvPpi->Fv[0].FvLength = ((EFI_FIRMWARE_VOLUME_HEADER 
*)(UINTN)PcdGet32 (PcdFspmBaseAddress))->FvLength;
 
   MeasurementExcludedPpiList = AllocatePool (sizeof 
(*MeasurementExcludedPpiList));
-  ASSERT (MeasurementExcludedPpiList != NULL);
+  if (MeasurementExcludedPpiList == NULL) {
+ASSERT (FALSE);
+return EFI_OUT_OF_RESOURCES;
+  }
+
   MeasurementExcludedPpiList->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI | 
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
   MeasurementExcludedPpiList->Guid  = 
&gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid;
   MeasurementExcludedPpiList->Ppi   = MeasurementExcludedFvPpi;
-- 
2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116686): https://edk2.groups.io/g/devel/message/116686
Mute This Topic: https://groups.io/mt/104886876/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] DynamicTablesPkg/SSDT: Require Package node in hierarchy

[Jeshua Smith via groups.io]

Can we get this reviewed/merged?

> -Original Message-
> From: Jeshua Smith 
> Sent: Monday, February 5, 2024 12:01 PM
> To: devel@edk2.groups.io
> Cc: ardb+tianoc...@kernel.org; quic_llind...@quicinc.com;
> pierre.gond...@arm.com; sami.muja...@arm.com; Jeshua Smith
> 
> Subject: [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in
> hierarchy
> 
> The code was incorrectly assuming that root nodes had to be physical package
> nodes and vice versa. This is not always true, so update the check to simply
> require exactly one package node somewhere in the hierarchy.
> 
> Signed-off-by: Jeshua Smith 
> Reviewed-by: Pierre Gondois 
> ---
> Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT:
> Remove incorrect root node check
> 
> Version 2: added documentation for the PackageNodeSeen parameter
> 
>  .../SsdtCpuTopologyGenerator.c| 32 +--
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> index 9e3efb49e6..40ed10eae6 100644
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> +++ uTopologyGenerator.c
> @@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
>@param [in]  IsLeaf   The ProcNode is a leaf.
>@param [in]  NodeTokenNodeToken of the ProcNode.
>@param [in]  ParentNodeToken  Parent NodeToken of the ProcNode.
> +  @param [in]  PackageNodeSeen  A parent of the ProcNode has the physical
> package flag set.
> 
>@retval EFI_SUCCESS Success.
>@retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -1083,23 +1084,24 @@ CheckProcNode (
>UINT32   NodeFlags,
>BOOLEAN  IsLeaf,
>CM_OBJECT_TOKEN  NodeToken,
> -  CM_OBJECT_TOKEN  ParentNodeToken
> +  CM_OBJECT_TOKEN  ParentNodeToken,
> +  BOOLEAN  PackageNodeSeen
>)
>  {
>BOOLEAN  InvalidFlags;
>BOOLEAN  HasPhysicalPackageBit;
> -  BOOLEAN  IsTopLevelNode;
> 
>HasPhysicalPackageBit = (NodeFlags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
>EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> -  IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
> 
> -  // A top-level node is a Physical Package and conversely.
> -  InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
> +  // Only one Physical Package flag is allowed in the hierarchy
> + InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
> 
>// Check Leaf specific flags.
>if (IsLeaf) {
>  InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
> +// Must have Physical Package flag somewhere in the hierarchy
> +InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
>} else {
>  InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
>}
> @@ -1130,6 +1132,7 @@ CheckProcNode (
>node to.
>@param [in,out] ProcContainerIndex  Pointer to the current processor
> container
>index to be used as UID.
> +  @param [in]  PackageNodeSeenA parent of the ProcNode has the
> physical package flag set.
> 
>@retval EFI_SUCCESS Success.
>@retval EFI_INVALID_PARAMETER   Invalid parameter.
> @@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
>IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST
> CfgMgrProtocol,
>INCM_OBJECT_TOKEN   NodeToken,
>INAML_NODE_HANDLE   ParentNode,
> -  IN OUTUINT32*ProcContainerIndex
> +  IN OUTUINT32
> *ProcContainerIndex,
> +  INBOOLEAN   PackageNodeSeen
>)
>  {
>EFI_STATUS  Status;
> @@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
>AML_OBJECT_NODE_HANDLE  ProcContainerNode;
>UINT32  Uid;
>UINT16  Name;
> +  BOOLEAN HasPhysicalPackageBit;
> 
>ASSERT (Generator != NULL);
>ASSERT (Generator->ProcNodeList != NULL); @@ -1175,7 +1180,8 @@
> CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> TRUE,
> Generator->ProcNodeList[Index].Token,
> -   NodeToken
> +   NodeToken,
> +   PackageNodeSeen
> );
>  if (EFI_ERROR (Status)) {
>ASSERT (0);
> @@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> FALSE,
> Generator->ProcNodeList[Index].Token,
> -   NodeToken
> +   NodeToken,

Re: [edk2-devel] [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in hierarchy

[Sami Mujawar]

Hi Jeshua,

Apologies, I somehow missed getting this merged.
I will get this in before end of this week.

Regards,

Sami Mujawar

On 12/03/2024, 16:12, "Jeshua Smith" mailto:jesh...@nvidia.com>> wrote:


Can we get this reviewed/merged?


> -Original Message-
> From: Jeshua Smith mailto:jesh...@nvidia.com>>
> Sent: Monday, February 5, 2024 12:01 PM
> To: devel@edk2.groups.io 
> Cc: ardb+tianoc...@kernel.org ; 
> quic_llind...@quicinc.com ;
> pierre.gond...@arm.com ; sami.muja...@arm.com 
> ; Jeshua Smith
> mailto:jesh...@nvidia.com>>
> Subject: [PATCH v2] DynamicTablesPkg/SSDT: Require Package node in
> hierarchy
> 
> The code was incorrectly assuming that root nodes had to be physical package
> nodes and vice versa. This is not always true, so update the check to simply
> require exactly one package node somewhere in the hierarchy.
> 
> Signed-off-by: Jeshua Smith mailto:jesh...@nvidia.com>>
> Reviewed-by: Pierre Gondois  >
> ---
> Note: This is a complete replacement for [PATCH] DynamicTablesPkg/SSDT:
> Remove incorrect root node check
> 
> Version 2: added documentation for the PackageNodeSeen parameter
> 
> .../SsdtCpuTopologyGenerator.c | 32 +--
> 1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> index 9e3efb49e6..40ed10eae6 100644
> ---
> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpu
> TopologyGenerator.c
> +++
> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCp
> +++ uTopologyGenerator.c
> @@ -1072,6 +1072,7 @@ CreateAmlProcessorContainer (
> @param [in] IsLeaf The ProcNode is a leaf.
> @param [in] NodeToken NodeToken of the ProcNode.
> @param [in] ParentNodeToken Parent NodeToken of the ProcNode.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the physical
> package flag set.
> 
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1083,23 +1084,24 @@ CheckProcNode (
> UINT32 NodeFlags,
> BOOLEAN IsLeaf,
> CM_OBJECT_TOKEN NodeToken,
> - CM_OBJECT_TOKEN ParentNodeToken
> + CM_OBJECT_TOKEN ParentNodeToken,
> + BOOLEAN PackageNodeSeen
> )
> {
> BOOLEAN InvalidFlags;
> BOOLEAN HasPhysicalPackageBit;
> - BOOLEAN IsTopLevelNode;
> 
> HasPhysicalPackageBit = (NodeFlags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> - IsTopLevelNode = (ParentNodeToken == CM_NULL_TOKEN);
> 
> - // A top-level node is a Physical Package and conversely.
> - InvalidFlags = HasPhysicalPackageBit ^ IsTopLevelNode;
> + // Only one Physical Package flag is allowed in the hierarchy
> + InvalidFlags = HasPhysicalPackageBit && PackageNodeSeen;
> 
> // Check Leaf specific flags.
> if (IsLeaf) {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != PPTT_LEAF_MASK);
> + // Must have Physical Package flag somewhere in the hierarchy
> + InvalidFlags |= !(HasPhysicalPackageBit || PackageNodeSeen);
> } else {
> InvalidFlags |= ((NodeFlags & PPTT_LEAF_MASK) != 0);
> }
> @@ -1130,6 +1132,7 @@ CheckProcNode (
> node to.
> @param [in,out] ProcContainerIndex Pointer to the current processor
> container
> index to be used as UID.
> + @param [in] PackageNodeSeen A parent of the ProcNode has the
> physical package flag set.
> 
> @retval EFI_SUCCESS Success.
> @retval EFI_INVALID_PARAMETER Invalid parameter.
> @@ -1143,7 +1146,8 @@ CreateAmlCpuTopologyTree (
> IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> CfgMgrProtocol,
> IN CM_OBJECT_TOKEN NodeToken,
> IN AML_NODE_HANDLE ParentNode,
> - IN OUT UINT32 *ProcContainerIndex
> + IN OUT UINT32 *ProcContainerIndex,
> + IN BOOLEAN PackageNodeSeen
> )
> {
> EFI_STATUS Status;
> @@ -1153,6 +1157,7 @@ CreateAmlCpuTopologyTree (
> AML_OBJECT_NODE_HANDLE ProcContainerNode;
> UINT32 Uid;
> UINT16 Name;
> + BOOLEAN HasPhysicalPackageBit;
> 
> ASSERT (Generator != NULL);
> ASSERT (Generator->ProcNodeList != NULL); @@ -1175,7 +1180,8 @@
> CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> TRUE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1208,7 +1214,8 @@ CreateAmlCpuTopologyTree (
> Generator->ProcNodeList[Index].Flags,
> FALSE,
> Generator->ProcNodeList[Index].Token,
> - NodeToken
> + NodeToken,
> + PackageNodeSeen
> );
> if (EFI_ERROR (Status)) {
> ASSERT (0);
> @@ -1249,13 +1256,17 @@ CreateAmlCpuTopologyTree (
> ProcContainerName++;
> }
> 
> + HasPhysicalPackageBit = (Generator->ProcNodeList[Index].Flags &
> EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL) ==
> + EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL;
> +
> // Recursively continue creating an AML tree.
> Status = CreateA

Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

[Leif Lindholm]

On 2024-03-12 08:17, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.


It's unclear to me why this change is safe (looking forward).
Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?

/
Leif


Signed-off-by: Ard Biesheuvel 
---
This fixes a regression on Raspberry Pi4, which no longer boots.

  EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c 
b/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
index 4e7a5698c162..f02a76a62ea8 100644
--- a/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
+++ b/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
@@ -70,7 +70,7 @@ NonCoherentIoMmuSetAttribute (
IN UINT64IoMmuAccess
)
  {
-  return EFI_UNSUPPORTED;
+  return EFI_SUCCESS;
  }
  
  /**




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116691): https://edk2.groups.io/g/devel/message/116691
Mute This Topic: https://groups.io/mt/104886877/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/2] ArmPkg/MdePkg: Move Chipset/* files to MdePkg

[Leif Lindholm]

On 2024-03-12 02:18, Pierre Gondois wrote:

This patch relies on [1].

Following the RFC v1: ArmPkg,MdePkg: move ArmLib.h to MdePkg [1],
move the Chipset/* files to the MdePkg as the Armlib.h relies on
them.

These patches span over multiple packages as these Chipset/* files
are relocated to a new directory and include paths must be updated.


I like this!
Traveling this week, so unable to test until Wednesday next week at the 
earliest, which I would like to do for something this core before giving 
a Reviewed-by. So for now, for the series:

Acked-by: Leif Lindholm 


[1] https://edk2.groups.io/g/devel/message/111566

Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Pierre Gondois 
Cc: Sami Mujawar 
Cc: Zhiguang Liu 

Pierre Gondois (2):
   ArmPkg,MdePkg: Move ArmPkg/Chipset/ArmV7[|Mmu].h to MdePkg
   ArmPkg,MdePkg: Move ArmPkg/Chipset/Aarch64[|Mmu].h to MdePkg

  ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c | 2 +-
  ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S | 2 +-
  ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c | 2 +-
  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c| 2 +-
  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S| 2 +-
  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c  | 2 +-
  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c  | 2 +-
  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c   | 2 +-
  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c  | 2 +-
  ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c| 2 +-
  ArmPlatformPkg/PrePeiCore/AArch64/Exception.S | 2 +-
  ArmPlatformPkg/PrePeiCore/AArch64/Helper.S| 2 +-
  ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c  | 2 +-
  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S   | 2 +-
  ArmVirtPkg/PrePi/AArch64/ArchPrePi.c  | 2 +-
  {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h  | 2 +-
  .../Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h   | 0
  .../Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h   | 2 +-
  .../Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h | 0
  MdePkg/Include/Library/ArmLib.h   | 4 ++--
  20 files changed, 19 insertions(+), 19 deletions(-)
  rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h (94%)
  rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h (100%)
  rename ArmPkg/Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h (95%)
  rename ArmPkg/Include/Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h 
(100%)





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116692): https://edk2.groups.io/g/devel/message/116692
Mute This Topic: https://groups.io/mt/104881290/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] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

[Ard Biesheuvel]

On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  wrote:
>
> On 2024-03-12 08:17, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel 
> >
> > NonCoherentIoMmuSetAttribute() does nothing except return
> > EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
> > bus driver will fail a PCI I/O Map() operation if the SetAttributes
> > fails.
> >
> > So return EFI_SUCCESS instead.
>
> It's unclear to me why this change is safe (looking forward).
> Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?
>

Basically. NonCoherentIoMmuDxe is just a vehicle to allow
NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
not intended to ever do anything more than that.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116693): https://edk2.groups.io/g/devel/message/116693
Mute This Topic: https://groups.io/mt/104886877/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/2] ArmPkg/MdePkg: Move Chipset/* files to MdePkg

[Michael D Kinney]

The MdePkg does have a standard location for CPU specific register
related includes.

Did you consider moving some of the content into

* MdePkg/Include/Register/Arm
* MdePkg/Include/Register/AArch64

Thanks,

Mike

> -Original Message-
> From: Leif Lindholm 
> Sent: Tuesday, March 12, 2024 9:47 AM
> To: Pierre Gondois ; devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Gerd Hoffmann
> ; Yao, Jiewen ; Liming Gao
> ; Kinney, Michael D ;
> Sami Mujawar ; Liu, Zhiguang 
> Subject: Re: [PATCH 0/2] ArmPkg/MdePkg: Move Chipset/* files to MdePkg
> 
> On 2024-03-12 02:18, Pierre Gondois wrote:
> > This patch relies on [1].
> >
> > Following the RFC v1: ArmPkg,MdePkg: move ArmLib.h to MdePkg [1],
> > move the Chipset/* files to the MdePkg as the Armlib.h relies on
> > them.
> >
> > These patches span over multiple packages as these Chipset/* files
> > are relocated to a new directory and include paths must be updated.
> 
> I like this!
> Traveling this week, so unable to test until Wednesday next week at the
> earliest, which I would like to do for something this core before giving
> a Reviewed-by. So for now, for the series:
> Acked-by: Leif Lindholm 
> 
> > [1] https://edk2.groups.io/g/devel/message/111566
> >
> > Cc: Ard Biesheuvel 
> > Cc: Gerd Hoffmann 
> > Cc: Jiewen Yao 
> > Cc: Leif Lindholm 
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Pierre Gondois 
> > Cc: Sami Mujawar 
> > Cc: Zhiguang Liu 
> >
> > Pierre Gondois (2):
> >ArmPkg,MdePkg: Move ArmPkg/Chipset/ArmV7[|Mmu].h to MdePkg
> >ArmPkg,MdePkg: Move ArmPkg/Chipset/Aarch64[|Mmu].h to MdePkg
> >
> >   ArmPkg/Library/ArmExceptionLib/AArch64/AArch64Exception.c | 2 +-
> >   ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S | 2 +-
> >   ArmPkg/Library/ArmExceptionLib/Arm/ArmException.c | 2 +-
> >   ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c| 2 +-
> >   ArmPkg/Library/ArmLib/AArch64/AArch64Support.S| 2 +-
> >   ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c  | 2 +-
> >   ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c  | 2 +-
> >   ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibConvert.c   | 2 +-
> >   ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c  | 2 +-
> >   ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibUpdate.c| 2 +-
> >   ArmPlatformPkg/PrePeiCore/AArch64/Exception.S | 2 +-
> >   ArmPlatformPkg/PrePeiCore/AArch64/Helper.S| 2 +-
> >   ArmPlatformPkg/PrePi/AArch64/ArchPrePi.c  | 2 +-
> >   ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S   | 2 +-
> >   ArmVirtPkg/PrePi/AArch64/ArchPrePi.c  | 2 +-
> >   {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h  | 2 +-
> >   .../Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h   | 0
> >   .../Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h   | 2 +-
> >   .../Chipset/ArmV7Mmu.h => MdePkg/Include/Arm/AArch32Mmu.h | 0
> >   MdePkg/Include/Library/ArmLib.h   | 4 ++--
> >   20 files changed, 19 insertions(+), 19 deletions(-)
> >   rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64.h
> (94%)
> >   rename {ArmPkg/Include/Chipset => MdePkg/Include/AArch64}/AArch64Mmu.h
> (100%)
> >   rename ArmPkg/Include/Chipset/ArmV7.h => MdePkg/Include/Arm/AArch32.h
> (95%)
> >   rename ArmPkg/Include/Chipset/ArmV7Mmu.h =>
> MdePkg/Include/Arm/AArch32Mmu.h (100%)
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116694): https://edk2.groups.io/g/devel/message/116694
Mute This Topic: https://groups.io/mt/104881290/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] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

[Leif Lindholm]

On 2024-03-12 09:50, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  wrote:


On 2024-03-12 08:17, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.


It's unclear to me why this change is safe (looking forward).
Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?



Basically. NonCoherentIoMmuDxe is just a vehicle to allow
NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
not intended to ever do anything more than that.


Not that it needs to happen for this
(Reviewed-by: Leif Lindholm )
but maybe we ought to consider renaming it then?
DummyIoMmuDxe?

/
Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116695): https://edk2.groups.io/g/devel/message/116695
Mute This Topic: https://groups.io/mt/104886877/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] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

[Ard Biesheuvel]

On Tue, 12 Mar 2024 at 17:56, Leif Lindholm  wrote:
>
> On 2024-03-12 09:50, Ard Biesheuvel wrote:
> > On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  
> > wrote:
> >>
> >> On 2024-03-12 08:17, Ard Biesheuvel wrote:
> >>> From: Ard Biesheuvel 
> >>>
> >>> NonCoherentIoMmuSetAttribute() does nothing except return
> >>> EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
> >>> bus driver will fail a PCI I/O Map() operation if the SetAttributes
> >>> fails.
> >>>
> >>> So return EFI_SUCCESS instead.
> >>
> >> It's unclear to me why this change is safe (looking forward).
> >> Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?
> >>
> >
> > Basically. NonCoherentIoMmuDxe is just a vehicle to allow
> > NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
> > not intended to ever do anything more than that.
>
> Not that it needs to happen for this
> (Reviewed-by: Leif Lindholm )
> but maybe we ought to consider renaming it then?
> DummyIoMmuDxe?
>

Fair point. Or PassThroughIoMmuDxe perhaps?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116696): https://edk2.groups.io/g/devel/message/116696
Mute This Topic: https://groups.io/mt/104886877/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] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

[Leif Lindholm]

On 2024-03-12 09:58, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 17:56, Leif Lindholm  wrote:


On 2024-03-12 09:50, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  wrote:


On 2024-03-12 08:17, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.


It's unclear to me why this change is safe (looking forward).
Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?



Basically. NonCoherentIoMmuDxe is just a vehicle to allow
NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
not intended to ever do anything more than that.


Not that it needs to happen for this
(Reviewed-by: Leif Lindholm )
but maybe we ought to consider renaming it then?
DummyIoMmuDxe?


Fair point. Or PassThroughIoMmuDxe perhaps?


Works for me.
Or, hmm...
Is there a risk that sounds a bit like a driver that actively configures 
IoMmus into passthrough mode?


/
Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116697): https://edk2.groups.io/g/devel/message/116697
Mute This Topic: https://groups.io/mt/104886877/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/4][edk2-redfish-client] RedfishClientPkg: fix deallocation of C-structures

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

For the patch set, Reviewed-by: Abner Chang 

Please update each patch with reviewed-by and update the PR.
Thanks Mike.
Abner

> -Original Message-
> From: Mike Maslenkin 
> Sent: Sunday, March 10, 2024 6:42 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Igor Kulchytskyy
> ; Nickle Wang ; Mike Maslenkin
> 
> Subject: [PATCH 0/4][edk2-redfish-client] RedfishClientPkg: fix deallocation 
> of
> C-structures
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> This set contains fixes for proper deallocation of the structures
> returned by JsonStructProtocol->ToStructure().
>
> PR: https://github.com/tianocore/edk2-redfish-client/pull/82
>
> Cc: Abner Chang 
> Cc: Igor Kulchytskyy 
> Cc: Nickle Wang 
> Signed-off-by: Mike Maslenkin 
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116698): https://edk2.groups.io/g/devel/message/116698
Mute This Topic: https://groups.io/mt/104841891/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: Error handling of FspmWrapperInit()

[Chen, Gang C]

Reviewed-by: Chen Gang C 

BR
Gang

-Original Message-
From: Lin, Du  
Sent: Tuesday, March 12, 2024 5:28 PM
To: devel@edk2.groups.io
Cc: Lin, Du ; S, Ashraf Ali ; Chiu, 
Chasel ; Chen, Gang C ; Duggapu, 
Chinni B ; Desimone, Nathaniel L 
; Zeng, Star ; Mohapatra, 
Susovan ; Kuo, Ted 
Subject: [PATCH] IntelFsp2WrapperPkg: Error handling of FspmWrapperInit()

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

The error handling of FspmWrapperInit() is limited to ASSERT statements only, 
which only works in debug builds, but not in release builds.
Fix the issue by enhancing the error handling of FspmWrapperInit() to cover 
both debug builds and release builds.

Signed-off-by: Du Lin 
Cc: Ashraf Ali S 
Cc: Chasel Chiu 
Cc: Chen Gang C 
Cc: Duggapu Chinni B 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Susovan Mohapatra 
Cc: Ted Kuo 
---
 .../FspmWrapperPeim/FspmWrapperPeim.c| 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c 
b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index ba0c742fea..356baeeccf 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -196,13 +196,21 @@ FspmWrapperInit (
   EFI_PEI_PPI_DESCRIPTOR 
*MeasurementExcludedPpiList;
 
   MeasurementExcludedFvPpi = AllocatePool (sizeof (*MeasurementExcludedFvPpi));
-  ASSERT (MeasurementExcludedFvPpi != NULL);
+  if (MeasurementExcludedFvPpi == NULL) {
+ASSERT (FALSE);
+return EFI_OUT_OF_RESOURCES;
+  }
+
   MeasurementExcludedFvPpi->Count  = 1;
   MeasurementExcludedFvPpi->Fv[0].FvBase   = PcdGet32 (PcdFspmBaseAddress);
   MeasurementExcludedFvPpi->Fv[0].FvLength = ((EFI_FIRMWARE_VOLUME_HEADER 
*)(UINTN)PcdGet32 (PcdFspmBaseAddress))->FvLength;
 
   MeasurementExcludedPpiList = AllocatePool (sizeof 
(*MeasurementExcludedPpiList));
-  ASSERT (MeasurementExcludedPpiList != NULL);
+  if (MeasurementExcludedPpiList == NULL) {
+ASSERT (FALSE);
+return EFI_OUT_OF_RESOURCES;
+  }
+
   MeasurementExcludedPpiList->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI | 
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
   MeasurementExcludedPpiList->Guid  = 
&gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid;
   MeasurementExcludedPpiList->Ppi   = MeasurementExcludedFvPpi;
--
2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116699): https://edk2.groups.io/g/devel/message/116699
Mute This Topic: https://groups.io/mt/104886876/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: Error handling of TpmMeasureAndLogDataWithFlags()

[Chen, Gang C]

Reviewed-by: Chen Gang C 

BR
Gang

-Original Message-
From: Lin, Du  
Sent: Tuesday, March 12, 2024 5:20 PM
To: devel@edk2.groups.io
Cc: Lin, Du ; S, Ashraf Ali ; Chiu, 
Chasel ; Chen, Gang C ; Duggapu, 
Chinni B ; Desimone, Nathaniel L 
; Zeng, Star ; Mohapatra, 
Susovan ; Kuo, Ted 
Subject: [PATCH] IntelFsp2WrapperPkg: Error handling of 
TpmMeasureAndLogDataWithFlags()

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

TpmMeasureAndLogDataWithFlags() computes the measure the code and log it into 
PCR 0. TpmMeasureAndLogData() computes the hash for the configuration. The same 
"Status" variable is used to store the return values for both of the functions. 
There is no error handling if
TpmMeasureAndLogDataWithFlags() returns an error Status.
Fix the issue by adding error handling for TpmMeasureAndLogDataWithFlags().

Signed-off-by: Du Lin 
Cc: Ashraf Ali S 
Cc: Chasel Chiu 
Cc: Chen Gang C 
Cc: Duggapu Chinni B 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Susovan Mohapatra 
Cc: Ted Kuo 
---
 .../Library/BaseFspMeasurementLib/FspMeasurementLib.c | 4 
 1 file changed, 4 insertions(+)

diff --git 
a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c 
b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
index 2c017a4250..228277649b 100644
--- a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLi
+++ b.c
@@ -197,6 +197,10 @@ MeasureFspFirmwareBlobWithCfg (
  (UINTN)sizeof (DigestList),
  EDKII_TCG_PRE_HASH_LOG_ONLY
  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "TpmMeasureAndLogDataWithFlags failed - %r\n", 
Status));
+return Status;
+  }
 
   Status = TpmMeasureAndLogData (
  1,
--
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116700): https://edk2.groups.io/g/devel/message/116700
Mute This Topic: https://groups.io/mt/104886875/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-platforms 1/1] Platform/RPI4: Grow FV size to accommodate DEBUG build

[Jeremy Linton]

Hi,

On 3/12/24 05:43, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 11:29, Ard Biesheuvel  wrote:


From: Ard Biesheuvel 

The DEBUG build no longer fits when all build options used by the
release script on github.com/pftf are used, presumably due to the
OpenSSL upgrade.

So bump the size for all builds.

Signed-off-by: Ard Biesheuvel 
---
  Platform/RaspberryPi/RPi4/RPi4.fdf | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf 
b/Platform/RaspberryPi/RPi4/RPi4.fdf
index 816927761513..951488260ed4 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -25,11 +25,11 @@

  [FD.RPI_EFI]
  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress
-Size  = 0x001f|gArmTokenSpaceGuid.PcdFdSize
+Size  = 0x0021|gArmTokenSpaceGuid.PcdFdSize
  ErasePolarity = 1



Ugh just realized that this breaks DT loading from TFA, which places
the DT at a fixed offset of 0x1f


Right, which can be changed within the config.txt file but its probably 
better not to. I hit this a few months back and have a version on my 
github which moves it too, along with the TFA fixes, but really we 
should be able to move one of the other regions around the DT, which 
IIRC may be pushed to one of my branches on github along with the CPPC 
patches for TFA/etc too.


That was set 4 or 5 that i've not posted because all the stuff in front 
of it is still up in the air.


I can probably pick off just this bit and post it next week if its still 
a problem, since i'm away from the rpi's at the moment.







  BlockSize = 0x1000|gRaspberryPiTokenSpaceGuid.PcdFirmwareBlockSize
-NumBlocks = 0x1f0
+NumBlocks = 0x210

  

  #
@@ -56,7 +56,7 @@ [FD.RPI_EFI]
  #
  # UEFI image
  #
-0x0002|0x001b
+0x0002|0x001d
  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
  FV = FVMAIN_COMPACT

@@ -70,7 +70,7 @@ [FD.RPI_EFI]
  #

  # NV_VARIABLE_STORE
-0x001d|0xe000
+0x001f|0xe000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize

  DATA = {
@@ -113,11 +113,11 @@ [FD.RPI_EFI]
  }

  # NV_EVENT_LOG
-0x001de000|0x1000
+0x001fe000|0x1000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogBase|gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize

  # NV_FTW_WORKING header
-0x001df000|0x1000
+0x001ff000|0x1000
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize

  DATA = {
@@ -132,7 +132,7 @@ [FD.RPI_EFI]
  }

  # NV_FTW_WORKING data
-0x001e|0x0001
+0x0020|0x0001
  
gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize

  #
--
2.44.0.278.ge034bb2e1d-goog





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116701): https://edk2.groups.io/g/devel/message/116701
Mute This Topic: https://groups.io/mt/104882052/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/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

[Jeremy Linton]

Hi,


On 3/11/24 09:13, Ard Biesheuvel via groups.io wrote:

On Wed, 17 Jan 2024 at 22:36, Jeremy Linton  wrote:


The rpi's config.txt controls which uart (pl011, or miniuart) is
selected as the console. TFA and edk2 follow its lead, but if the
miniuart is selected as the primary and the machine is booted in ACPI
mode the baud/etc is never configured for the pl011. The linux kernel
won't reconfigure it either as its listed as a "SBSA" uart, so it
simply won't work.

This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
to work again.

Signed-off-by: Jeremy Linton 
---
  .../DualSerialPortLib/DualSerialPortLib.c | 44 ---
  1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c 
b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index d2f983bf0a..09d3e33c00 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -76,6 +76,8 @@ SerialPortInitialize (
EFI_PARITY_TYPE Parity;
UINT8   DataBits;
EFI_STOP_BITS_TYPE  StopBits;
+  RETURN_STATUS   Ret;
+  UINTN   Timeout;

//
// First thing we need to do is determine which of PL011 or miniUART is 
selected
@@ -85,23 +87,34 @@ SerialPortInitialize (
  UsePl011UartSet = TRUE;
}

-  if (UsePl011Uart) {
-BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+  // always init the pl011 on the RPi4, linux expects a SBSA uart to be at 
115200
+  // this means we need to set the baud/etc even if we aren't using it as a 
console
+  if ((UsePl011Uart) || (RPI_MODEL == 4)) {
  ReceiveFifoDepth = 0; // Use default FIFO depth
+if (!UsePl011Uart)
+{
+  BaudRate = 115200;
+}
+else
+{
+  BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+}
  Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
  DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
  StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);

-return PL011UartInitializePort (
- PL011_UART_REGISTER_BASE,
- PL011UartClockGetFreq(),
- &BaudRate,
- &ReceiveFifoDepth,
- &Parity,
- &DataBits,
- &StopBits
- );
-  } else {
+Ret = PL011UartInitializePort (
+   PL011_UART_REGISTER_BASE,
+   PL011UartClockGetFreq(),
+   &BaudRate,
+   &ReceiveFifoDepth,
+   &Parity,
+   &DataBits,
+   &StopBits
+   );
+  }
+
+  if (!UsePl011Uart) {
  SerialRegisterBase = MINI_UART_REGISTER_BASE;
  Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));

@@ -127,7 +140,8 @@ SerialPortInitialize (
  // Wait for the serial port to be ready.
  // Verify that both the transmit FIFO and the shift register are empty.
  //
-while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+Timeout = 1000;
+while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) & 
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) && 
(Timeout--));



Why is this necessary, and what does it have to do with the rest of the patch?


I think last time I noted that it side steps a problem when both of the 
uarts are being unilaterally configured but the clock wasn't because the 
default selected in the config.txt was the pl011 (IIRC). But it 
shouldn't be hit by default unless the config.txt and edk2 configs is 
differing.


I will drop or move it, if that is whats holding up this set.




  //
  // Configure baud rate
@@ -158,9 +172,9 @@ SerialPortInitialize (
  // Put Modem Control Register(MCR) into its reset state of 0x00.
  //
  SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
-
-return RETURN_SUCCESS;
+Ret = RETURN_SUCCESS;
}
+  return Ret;
  }

  /**
--
2.43.0











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

Re: [edk2-devel] [PATCH 2/4] ShellPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[Chao Li]

Hi Qingyu,

It may not be my domain, I think what you are trying to say is "Refer to 
UEFI spec 2.10 section 12.3.3",  not 13.3.3, because the 13.3.3 is 
"Number and Location of System Partitions", see me comment below. I 
recommend fixing all of the patch commit messages in this series.


The changes looks good to me and would recommend Liming and Zhichao to 
check again.



Thanks,
Chao
On 2024/3/11 16:41, Qingyu wrote:

Refer to Uefi spec 2.10 section 13.3.3, Add a new retval

It should be section 12.3.3, not 13.3.3.

EFI_UNSUPPORTED to EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().

Cc: Liming Gao
Cc: Zhichao Gao
Cc: Chao Li
Signed-off-by: Qingyu
---
  ShellPkg/Application/Shell/ConsoleWrappers.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/ConsoleWrappers.c 
b/ShellPkg/Application/Shell/ConsoleWrappers.c
index eae11370e4e0..dbffae30cdf5 100644
--- a/ShellPkg/Application/Shell/ConsoleWrappers.c
+++ b/ShellPkg/Application/Shell/ConsoleWrappers.c
@@ -67,7 +67,9 @@ FileBasedSimpleTextInReset (
@param[in] This  A pointer to the SimpleTextIn structure.
@param[in, out] Key  A pointer to the Key structure to fill.
  
-  @retval   EFI_SUCCESS The read was successful.

+  @retval EFI_SUCCESS  The read was successful.
+  @retval EFI_UNSUPPORTED  The device does not support the ability to read
+   keystroke data.
  **/
  EFI_STATUS
  EFIAPI



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

Re: [edk2-devel] [PATCH v4 0/3] Update ReceiveData and SendData function description

[Michael D Kinney]

Merged: https://github.com/tianocore/edk2/pull/5463

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Qingyu
> Sent: Sunday, February 25, 2024 7:06 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v4 0/3] Update ReceiveData and SendData
> function description
> 
> Refer to Uefi spec 2.10 section 13.14, update the parameter 'MediaId'
> description for EFI_STORAGE_SECURITY_COMMAND_PROTOCOL function ReceiveData
> and SendData.
> 
> V4:
> Split patch so there is one patch per package
> 
> V3:
> Add maintainer to CC list.
> 
> V2:
> Update some functions description in C and H header files in MdeModulePkg
> and SecurityPkg.
> 
> V1:
> update the parameter description in
> MdePkg/Include/Protocol/StorageSecurityCommand.h.
> 
> Qingyu (3):
>   MdePkg: Update ReceiveData and SendData function description
>   MdeModulePkg: Update ReceiveData and SendData function description
>   SecurityPkg: Update ReceiveData and SendData function description
> 
>  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c  |  8 ++--
>  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h  |  8 ++--
>  .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c|  8 ++--
>  .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.h|  8 ++--
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c |  8 ++--
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h |  8 ++--
>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c| 12 +---
>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.h| 12 +---
>  MdePkg/Include/Protocol/StorageSecurityCommand.h |  8 ++--
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c  |  8 ++--
>  10 files changed, 66 insertions(+), 22 deletions(-)
> 
> --
> 2.39.1.windows.1
> 
> 
> 
> 
> 



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

回复: [edk2-devel] [PATCH 0/4] Update the comments of ReadKeyStroke and ReadKeyStrokeEx

[gaoliming via groups.io]

For this patch set, Reviewed-by: Liming Gao 

Besides, please also update
Edk2Platforms\Features\Intel\UserInterface\VirtualKeyboardFeaturePkg\Virtual
KeyboardDxe

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Qingyu
> 发送时间: 2024年3月11日 16:41
> 收件人: devel@edk2.groups.io
> 抄送: Ming Tan ; Yi Li ; Gahan
> 
> 主题: [edk2-devel] [PATCH 0/4] Update the comments of ReadKeyStroke and
> ReadKeyStrokeEx
> 
> Refer to Uefi spec 2.10 section 13.3.3, Add a new retval
> EFI_UNSUPPORTED to
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL.ReadKeyStrokeEx
> and EFI_SIMPLE_TEXT_INPUT_PROTOCOL.ReadKeyStroke().
> 
> This patch series update ReadKeyStroke and ReadKeyStrokeEx
> implementation
> in Edk2.
> 
> Qingyu (4):
>   MdeModulePkg: Update the comments of ReadKeyStroke and
> ReadKeyStrokeEx
>   ShellPkg: Update the comments of ReadKeyStroke and ReadKeyStrokeEx
>   EmbeddedPkg: Update the comments of ReadKeyStroke and
> ReadKeyStrokeEx
>   EmulatorPkg: Update the comments of ReadKeyStroke and
> ReadKeyStrokeEx
> 
>  .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.c   | 10 ++
>  .../Drivers/VirtualKeyboardDxe/VirtualKeyboard.h   | 10 ++
>  EmulatorPkg/EmuGopDxe/GopInput.c   |  2 ++
>  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c |  4 +++-
>  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h  |  3 +++
>  MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c |  2 ++
>  MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h |  2 ++
>  .../Universal/Console/ConSplitterDxe/ConSplitter.c |  6 ++
>  .../Universal/Console/ConSplitterDxe/ConSplitter.h |  6 ++
>  MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h  |  4 
>  .../Universal/Console/TerminalDxe/TerminalConIn.c  |  4 
>  ShellPkg/Application/Shell/ConsoleWrappers.c   |  4 +++-
>  12 files changed, 47 insertions(+), 10 deletions(-)
> 
> Cc: Ming Tan 
> Cc: Yi Li 
> Cc: Gahan 
> Signed-off-by: Qingyu 
> 
> --
> 2.39.1.windows.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116705): https://edk2.groups.io/g/devel/message/116705
Mute This Topic: https://groups.io/mt/104900075/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: Error handling of TpmMeasureAndLogDataWithFlags()

[Ashraf Ali S]

Reviewed-by: S, Ashraf Ali 

Thanks.,
S, Ashraf Ali

-Original Message-
From: Lin, Du  
Sent: Tuesday, March 12, 2024 2:50 PM
To: devel@edk2.groups.io
Cc: Lin, Du ; S, Ashraf Ali ; Chiu, 
Chasel ; Chen, Gang C ; Duggapu, 
Chinni B ; Desimone, Nathaniel L 
; Zeng, Star ; Mohapatra, 
Susovan ; Kuo, Ted 
Subject: [PATCH] IntelFsp2WrapperPkg: Error handling of 
TpmMeasureAndLogDataWithFlags()

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

TpmMeasureAndLogDataWithFlags() computes the measure the code and log it into 
PCR 0. TpmMeasureAndLogData() computes the hash for the configuration. The same 
"Status" variable is used to store the return values for both of the functions. 
There is no error handling if
TpmMeasureAndLogDataWithFlags() returns an error Status.
Fix the issue by adding error handling for TpmMeasureAndLogDataWithFlags().

Signed-off-by: Du Lin 
Cc: Ashraf Ali S 
Cc: Chasel Chiu 
Cc: Chen Gang C 
Cc: Duggapu Chinni B 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Susovan Mohapatra 
Cc: Ted Kuo 
---
 .../Library/BaseFspMeasurementLib/FspMeasurementLib.c | 4 
 1 file changed, 4 insertions(+)

diff --git 
a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c 
b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
index 2c017a4250..228277649b 100644
--- a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLi
+++ b.c
@@ -197,6 +197,10 @@ MeasureFspFirmwareBlobWithCfg (
  (UINTN)sizeof (DigestList),
  EDKII_TCG_PRE_HASH_LOG_ONLY
  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "TpmMeasureAndLogDataWithFlags failed - %r\n", 
Status));
+return Status;
+  }
 
   Status = TpmMeasureAndLogData (
  1,
--
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116706): https://edk2.groups.io/g/devel/message/116706
Mute This Topic: https://groups.io/mt/104886875/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: Error handling of FspmWrapperInit()

[Ashraf Ali S]

Reviewed-by: S, Ashraf Ali 

Thanks.,
S, Ashraf Ali

-Original Message-
From: Lin, Du  
Sent: Tuesday, March 12, 2024 2:58 PM
To: devel@edk2.groups.io
Cc: Lin, Du ; S, Ashraf Ali ; Chiu, 
Chasel ; Chen, Gang C ; Duggapu, 
Chinni B ; Desimone, Nathaniel L 
; Zeng, Star ; Mohapatra, 
Susovan ; Kuo, Ted 
Subject: [PATCH] IntelFsp2WrapperPkg: Error handling of FspmWrapperInit()

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

The error handling of FspmWrapperInit() is limited to ASSERT statements only, 
which only works in debug builds, but not in release builds.
Fix the issue by enhancing the error handling of FspmWrapperInit() to cover 
both debug builds and release builds.

Signed-off-by: Du Lin 
Cc: Ashraf Ali S 
Cc: Chasel Chiu 
Cc: Chen Gang C 
Cc: Duggapu Chinni B 
Cc: Nate DeSimone 
Cc: Star Zeng 
Cc: Susovan Mohapatra 
Cc: Ted Kuo 
---
 .../FspmWrapperPeim/FspmWrapperPeim.c| 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c 
b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index ba0c742fea..356baeeccf 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -196,13 +196,21 @@ FspmWrapperInit (
   EFI_PEI_PPI_DESCRIPTOR 
*MeasurementExcludedPpiList;
 
   MeasurementExcludedFvPpi = AllocatePool (sizeof (*MeasurementExcludedFvPpi));
-  ASSERT (MeasurementExcludedFvPpi != NULL);
+  if (MeasurementExcludedFvPpi == NULL) {
+ASSERT (FALSE);
+return EFI_OUT_OF_RESOURCES;
+  }
+
   MeasurementExcludedFvPpi->Count  = 1;
   MeasurementExcludedFvPpi->Fv[0].FvBase   = PcdGet32 (PcdFspmBaseAddress);
   MeasurementExcludedFvPpi->Fv[0].FvLength = ((EFI_FIRMWARE_VOLUME_HEADER 
*)(UINTN)PcdGet32 (PcdFspmBaseAddress))->FvLength;
 
   MeasurementExcludedPpiList = AllocatePool (sizeof 
(*MeasurementExcludedPpiList));
-  ASSERT (MeasurementExcludedPpiList != NULL);
+  if (MeasurementExcludedPpiList == NULL) {
+ASSERT (FALSE);
+return EFI_OUT_OF_RESOURCES;
+  }
+
   MeasurementExcludedPpiList->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI | 
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
   MeasurementExcludedPpiList->Guid  = 
&gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid;
   MeasurementExcludedPpiList->Ppi   = MeasurementExcludedFvPpi;
--
2.44.0.windows.1



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