[edk2-devel] [PATCH] MdeModulePkg/Usb/Keyboard.c: Don't request protocol before setting

[Sean Rhodes]

No need to check the interface protocol then conditionally setting,
just set it to BOOT_PROTOCOL and check for error.

This is what Linux does for HID devices as some don't follow the USB spec.
One example is the Aspeed BMC HID keyboard device, which adds a massive
boot delay without this patch as it doesn't respond to 'GetProtocolRequest'.

Cc: Hao A Wu 
Cc: Ray Ni 
Signed-off-by: Matt DeVillier 
Signed-off-by: Patrick Rudolph 
Signed-off-by: Sean Rhodes 
---
 MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c| 17 +
 .../Library/BrotliCustomDecompressLib/brotli|  2 +-
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c 
b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
index 5a94a4dda7..73b5df2b64 100644
--- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
+++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
@@ -854,22 +854,15 @@ InitUSBKeyboard (
 }
   }
 
-  UsbGetProtocolRequest (
-UsbKeyboardDevice->UsbIo,
-UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-
-);
   //
   // Set boot protocol for the USB Keyboard.
   // This driver only supports boot protocol.
   //
-  if (Protocol != BOOT_PROTOCOL) {
-UsbSetProtocolRequest (
-  UsbKeyboardDevice->UsbIo,
-  UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
-  BOOT_PROTOCOL
-  );
-  }
+  UsbSetProtocolRequest (
+UsbKeyboardDevice->UsbIo,
+UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
+BOOT_PROTOCOL
+);
 
   UsbKeyboardDevice->CtrlOn= FALSE;
   UsbKeyboardDevice->AltOn = FALSE;
diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli 
b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
index f4153a09f8..666c3280cc 16
--- a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
+++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
@@ -1 +1 @@
-Subproject commit f4153a09f87cbb9c826d8fc12c74642bb2d879ea
+Subproject commit 666c3280cc11dc433c303d79a83d4ffbdd12cc8d
-- 
2.32.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86756): https://edk2.groups.io/g/devel/message/86756
Mute This Topic: https://groups.io/mt/89228276/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] MdeModulePkg/SdMmcPciHcDxe: Make timeout for SD card configurable

[Sean Rhodes]

From: Matt DeVillier 

The default 1s timeout can delay boot splash on some hardware with no
benefit.

Cc: Hao A Wu 
Cc: Ray Ni 
Cc: Jian J Wang 
Cc: Liming Gao 
Signed-off-by: Matt DeVillier 
Signed-off-by: Sean Rhodes 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h| 3 ++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf  | 4 
 MdeModulePkg/Library/BrotliCustomDecompressLib/brotli | 2 +-
 MdeModulePkg/MdeModulePkg.dec | 4 
 MdeModulePkg/MdeModulePkg.uni | 4 
 5 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 85e09cf114..b76c7cffa2 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -24,6 +24,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -49,7 +50,7 @@ extern EDKII_SD_MMC_OVERRIDE  *mOverride;
 //
 // Generic time out value, 1 microsecond as unit.
 //
-#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 1000
+#define SD_MMC_HC_GENERIC_TIMEOUT  (PcdGet32 (PcdSdMmcGenericTimeoutValue))
 
 //
 // SD/MMC async transfer timer interval, set by experience.
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
index 453ecde7fd..a9d05736d7 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
@@ -56,6 +56,7 @@
   BaseLib
   UefiDriverEntryPoint
   DebugLib
+  PcdLib
 
 [Protocols]
   gEdkiiSdMmcOverrideProtocolGuid   ## SOMETIMES_CONSUMES
@@ -68,3 +69,6 @@
 
 [UserExtensions.TianoCore."ExtraFiles"]
   SdMmcPciHcDxeExtra.uni
+
+[Pcd.IA32,Pcd.X64]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue   ## 
CONSUMES
diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli 
b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
index f4153a09f8..666c3280cc 16
--- a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
+++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
@@ -1 +1 @@
-Subproject commit f4153a09f87cbb9c826d8fc12c74642bb2d879ea
+Subproject commit 666c3280cc11dc433c303d79a83d4ffbdd12cc8d
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 463e889e9a..40601c9583 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1559,6 +1559,10 @@
   # @Prompt Maximum permitted FwVol section nesting depth (exclusive).
   
gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|0x10|UINT32|0x0030
 
+  ## Indicates the default timeout value for SD/MMC Host Controller operations 
in microseconds.
+  # @Prompt SD/MMC Host Controller Operations Timeout (us).
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|100|UINT32|0x0031
+
 [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## This PCD defines the Console output row. The default value is 25 
according to UEFI spec.
   #  This PCD could be set to 0 then console output would be at max column and 
max row.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 27889a7280..b070f15ff2 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1173,6 +1173,10 @@

   " TRUE  - Capsule In Ram is supported."

   " FALSE - Capsule In Ram is not supported."
 
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_PROMPT 
#language en-US "SD/MMC Host Controller Operations Timeout (us)."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_HELP   
#language en-US "Indicates the default timeout value for SD/MMC Host Controller 
operations in microseconds."
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCodRelocationDevPath_PROMPT  
#language en-US "Capsule On Disk relocation device path."
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCodRelocationDevPath_HELP  
#language en-US   "Full device path of platform specific device to store 
Capsule On Disk temp relocation file."
-- 
2.32.0



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

[edk2-devel] [PATCH 2/2] UefiPayloadPkg: Hookup SD Card timeout

[Sean Rhodes]

Hook SD_CARD_TIMEOUT build option to SdMmcGenericTimeoutValue PCD.

Cc: Guo Dong 
Cc: Ray Ni 
Cc: Maurice Ma 
Cc: Benjamin You 
Signed-off-by: Sean Rhodes 
---
 UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc 
b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 1ce96a51c1..d75fe26426 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -33,6 +33,7 @@
   DEFINE UNIVERSAL_PAYLOAD= FALSE
   DEFINE SECURITY_STUB_ENABLE = TRUE
   DEFINE SMM_SUPPORT  = FALSE
+  DEFINE SD_CARD_TIMEOUT  = 100
   #
   # SBL:  UEFI payload for Slim Bootloader
   # COREBOOT: UEFI payload for coreboot
@@ -398,6 +399,7 @@
 !if $(PERFORMANCE_MEASUREMENT_ENABLE)
   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask   | 0x1
 !endif
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|$(SD_CARD_TIMEOUT)
 
 [PcdsPatchableInModule.X64]
   gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER)
-- 
2.32.0



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

Re: [edk2-devel] [edk2-platforms: PATCH] MinPlatformPkg/SaveMemoryConfig: Fix GCC build failure.

[Chiu, Chasel]

Fix has pushed: 
https://github.com/tianocore/edk2-platforms/commit/c398756604f54515da7af75a3f60a0380429a78c

Thanks,
Chasel


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chiu, Chasel
> Sent: Friday, February 18, 2022 10:52 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Gao, Liming ;
> Dong, Eric 
> Subject: [edk2-devel] [edk2-platforms: PATCH]
> MinPlatformPkg/SaveMemoryConfig: Fix GCC build failure.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
> 
> Commit cbc8e420ac4505e9c51aa0d4f049026691024ca5 caused GCC build
> failure which should be fixed.
> 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Signed-off-by: Chasel Chiu 
> ---
> 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWri
> teLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable
> WriteLib.c
> b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable
> WriteLib.c
> index 3a9869ff9c..de23ae6160 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable
> WriteLib.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVa
> +++ riableWriteLib.c
> @@ -485,7 +485,7 @@ LockLargeVariable (
>) {   CHAR16TempVariableName[MAX_VARIABLE_NAME_SIZE];-  UINT64
> VariableSize;+  UINTN VariableSize;   EFI_STATUSStatus;   UINTN
> Index; --
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86750): https://edk2.groups.io/g/devel/message/86750
> Mute This Topic: https://groups.io/mt/89225574/1777047
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.c...@intel.com] -=-
> =-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86753): https://edk2.groups.io/g/devel/message/86753
Mute This Topic: https://groups.io/mt/89225574/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] Reduce the ASSERT patch to save the binary size

[Michael D Kinney]

Guomin,

I think there is a cleaner solution to this problem using
compiler flags to change the contents of the __FILE__ macro.

https://blog.conan.io/2019/09/02/Deterministic-builds-with-C-C++.html
https://reproducible-builds.org/docs/build-path/
https://developercommunity.visualstudio.com/t/map-file-to-a-relative-path/1393881

I found this content when I was working on the CompareBuild tool
and using these techniques can guarantee the same binaries are
produced when the path to WORKSPACE is different on different
build systems.

Does your change provide a module-relative, package-relative,
or workspace-relative file path in the ASSERT()?

How does ASSERT() in autogen work?

Thanks,

Mike



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Guomin Jiang
> Sent: Thursday, February 17, 2022 6:30 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH 0/2] Reduce the ASSERT patch to save the binary 
> size
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3840
> 
> 1. Use DEBUG_FILE to control ASSERT path
> 2. Default use file name as ASSERT path
> 
> Motivation and Goal:
> 1. The path will occupy many size in DEBUG build when file path is long
> 2. We hope can reduce the size but not impact the debug capability
> 3. If only use filename, we can search the filename to locate file. It
>can save many size meanwhile.
> 
> Guomin Jiang (2):
>   BaseTools/Conf: Reduce the ASSERT patch to save the binary size
>   MdePkg/Include: Define new DEBUG_FILE to specify path.
> 
>  BaseTools/Conf/build_rule.template | 10 
>  MdePkg/Include/Library/DebugLib.h  | 39 +-
>  2 files changed, 33 insertions(+), 16 deletions(-)
> 
> --
> 2.35.1.windows.2
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [edk2-platforms: PATCH] MinPlatformPkg/SaveMemoryConfig: Fix GCC build failure.

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: Chiu, Chasel  
Sent: Friday, February 18, 2022 10:52 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L 
; Gao, Liming ; Dong, 
Eric 
Subject: [edk2-platforms: PATCH] MinPlatformPkg/SaveMemoryConfig: Fix GCC build 
failure.

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

Commit cbc8e420ac4505e9c51aa0d4f049026691024ca5 caused GCC build failure which 
should be fixed.

Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Eric Dong 
Signed-off-by: Chasel Chiu 
---
 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
 
b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
index 3a9869ff9c..de23ae6160 100644
--- 
a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
+++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVa
+++ riableWriteLib.c
@@ -485,7 +485,7 @@ LockLargeVariable (
   ) {   CHAR16TempVariableName[MAX_VARIABLE_NAME_SIZE];-  UINT64   
 VariableSize;+  UINTN VariableSize;   EFI_STATUSStatus;   UINTN
 Index; -- 
2.28.0.windows.1



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

[edk2-devel] [edk2-platforms: PATCH] MinPlatformPkg/SaveMemoryConfig: Fix GCC build failure.

[Chiu, Chasel]

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

Commit cbc8e420ac4505e9c51aa0d4f049026691024ca5 caused GCC build failure
which should be fixed.

Cc: Nate DeSimone 
Cc: Liming Gao 
Cc: Eric Dong 
Signed-off-by: Chasel Chiu 
---
 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
 
b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
index 3a9869ff9c..de23ae6160 100644
--- 
a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
+++ 
b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
@@ -485,7 +485,7 @@ LockLargeVariable (
   )
 {
   CHAR16TempVariableName[MAX_VARIABLE_NAME_SIZE];
-  UINT64VariableSize;
+  UINTN VariableSize;
   EFI_STATUSStatus;
   UINTN Index;
 
-- 
2.28.0.windows.1



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

Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different values for PEI and DXE?

[Andrew Fish via groups.io]

> On Feb 17, 2022, at 5:36 PM, Michael D Kinney  
> wrote:
> 
> Hi Andrew,
>  
> I forgot about the library constraints.
>  
> Unless we want to first add a feature to build a library instance for each 
> module type into
> a different build output directory, we cannot support setting a FixedAtBuild 
> PCD for each
> module type.
>  

Mike,

Yea I’m thinking this is a good reason to maybe not add more knobs. 

I was trying to get tricky and have code conditionally use a global so I could 
use the lib XIP when I shot myself in the foot. 

Thanks,

Andrew Fish

> If you use PatchableInModule, you can post process all the modules to set 
> individual
> module PCD values.  You have to do two steps.  One DSC to build all the 
> modules and
> generate AsBuiltInf.  2nd DSC/FDF that references the AsBuildInfs and sets 
> the 
> PatchableInModule PCD values in the scope of the modules needed.
>  
> The only reason the different ARCHs for PEI and DXE works is because the libs
> for each ARCH are build separately and already have different build output 
> locations.
>  
> Mike
>  
>  
>  
> From: Andrew Fish mailto:af...@apple.com>> 
> Sent: Thursday, February 17, 2022 4:42 PM
> To: devel@edk2.groups.io ; Oram, Isaac W 
> mailto:isaac.w.o...@intel.com>>
> Cc: Kinney, Michael D  >
> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
> values for PEI and DXE?
>  
>  
> 
> 
> On Feb 17, 2022, at 4:26 PM, Oram, Isaac W  > wrote:
>  
> Andrew,
> 
> It is a reasonable ask, but I lean towards voting no.  We can more or less do 
> the equivalent by breaking a monolithic build into a set of phase specific 
> build/DSC/FDF or something similar.  That seems more flexible anyway.  Maybe 
> we want binaries with settings based on maturity more than phase.  And so on.
> 
> 
> My primary concern that the complexity added to an already complex feature.  
> PCD have a lot of binary compatibility challenges anyway.  And this doesn't 
> add any that don't already exist, but it does make it a lot easier to 
> introduce a mismatch issue.
> 
>  
> I shot my self in the foot with the Fixed* macro in a library so I did not 
> get the drivers override. So I feel that pain. You need to used the non Fixed 
> form from a lib to get the value from the global produced by the driver. 
> 
> 
> 
> My more recreational question is if this makes sense in most cases, or if 
> there are a limited set of PCD that it makes sense for.  A lot of similar PCD 
> are much more like shared constants and thus it doesn't make a lot of sense 
> for them to vary across executables.  E.G. drivers are asking for problems if 
> they change max string lengths inconsistently. There are definitely more than 
> one PCD where it seems reasonable.  Maybe they are the "wrong" type though.  
> Maybe they should be a dynamic type if a flexible usage is desired.
> 
>  
> I was trying to have different settings for 
> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask is regards to the action taken 
> on an ASSERT. 
>  
> To be honest it was not my 1st choice and I was working around some early 
> boot exception handling code not saving the EFI_SYSTEM_CONTEXT and blocking 
> the scheme I was trying to use. 
> 
> 
> 
> Tangentially related, there was some previous work to make macros for phase 
> architectures.
> 
> [Defines]
>  PEI_ARCH= IA32
>  DXE_ARCH= X64
> 
> Enabling [Components.$(PEI_ARCH)] and [Components.$(DXE_ARCH)]
> 
>  
> In my case PEI_ARCH == DXE_ARCH so this kind of trick is not helpful. The 
> reality is this trick only works for non virtualized X64 platforms and the 
> edk2 supports a lot more flavors of platforms than X64. 
>  
> Thanks,
>  
> Andrew Fish
> 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2308 
> 
> 
> If we do something with PCD, we should probably try to be consistent with 
> this and maybe extend it or integrate it into DSC syntax.  With standalone 
> MM, it probably makes more sense to have PEI, DXE, and SMM.  RT would also 
> make sense to have different settings.  I have mixed feelings about BDS.  I 
> think it makes sense to see it as a potentially independent phase that isn't 
> currently.  But also probably won't ever be independent from DXE.
> 
> Regards,
> Isaac
> 
> 
> -Original Message-
> From: devel@edk2.groups.io  
> mailto:devel@edk2.groups.io>> On Behalf Of Andrew Fish 
> via groups.io 
> Sent: Thursday, February 17, 2022 12:22 PM
> To: Kinney, Michael D  >
> Cc: devel@edk2.groups.io 
> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
> values for PEI and DXE?
> 
> 
> 
> 
> On Feb 17, 2022, at 10:52 AM, Kinney, Michael D  

[edk2-devel] [PATCH 2/2] MdePkg/Include: Define new DEBUG_FILE to specify path.

[Guomin Jiang]

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

Use DEBUG_FILE to control ASSERT path

Motivation and Goal:
1. The path will occupy many size in DEBUG build when file path is long
2. We hope can reduce the size but not impact the debug capability
3. If only use filename, we can search the filename to locate file. It
   can save many size meanwhile.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Guomin Jiang 
---
 MdePkg/Include/Library/DebugLib.h | 39 ++-
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/MdePkg/Include/Library/DebugLib.h 
b/MdePkg/Include/Library/DebugLib.h
index 8d3d08638d73..5469c6308422 100644
--- a/MdePkg/Include/Library/DebugLib.h
+++ b/MdePkg/Include/Library/DebugLib.h
@@ -8,7 +8,7 @@
   of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
   defined, then debug and assert related macros wrapped by it are the NULL 
implementations.
 
-Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -85,6 +85,31 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define DEBUG_LINE_NUMBER  __LINE__
 #endif
 
+//
+// Source file.
+// Default is use the to compiler provided __FILE__ macro value. The __FILE__
+// mapping can be overriden by predefining DEBUG_FILE
+//
+// Defining DEBUG_FILE to a fixed value is useful when comparing builds
+// across machine or configuration with different slash or path
+// file.
+//
+// Another benefit is we can customize the ASSERT path without depending on
+// compiler ability
+//
+// It's for all no matter VS, GCC, CLANG
+//
+#ifdef DEBUG_FILE
+#else
+#define DEBUG_FILE  __FILE__
+#endif
+
+// Blow override for keep clang behavior
+#if defined (__clang__) && defined (__FILE_NAME__)
+#undef DEBUG_FILE
+#define DEBUG_FILE __FILE_NAME__
+#endif
+
 /**
   Macro that converts a Boolean expression to a Null-terminated ASCII string.
 
@@ -337,17 +362,9 @@ UnitTestDebugAssert (
   IN CONST CHAR8  *Description
   );
 
-  #if defined (__clang__) && defined (__FILE_NAME__)
-#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE_NAME__, 
DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING (Expression))
-  #else
-#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE__, DEBUG_LINE_NUMBER, 
DEBUG_EXPRESSION_STRING (Expression))
-  #endif
+#define _ASSERT(Expression)  UnitTestDebugAssert (DEBUG_FILE, 
DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING (Expression))
 #else
-  #if defined (__clang__) && defined (__FILE_NAME__)
-#define _ASSERT(Expression)  DebugAssert (__FILE_NAME__, DEBUG_LINE_NUMBER, 
DEBUG_EXPRESSION_STRING (Expression))
-  #else
-#define _ASSERT(Expression)  DebugAssert (__FILE__, DEBUG_LINE_NUMBER, 
DEBUG_EXPRESSION_STRING (Expression))
-  #endif
+#define _ASSERT(Expression)  DebugAssert (DEBUG_FILE, DEBUG_LINE_NUMBER, 
DEBUG_EXPRESSION_STRING (Expression))
 #endif
 
 /**
-- 
2.35.1.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86748): https://edk2.groups.io/g/devel/message/86748
Mute This Topic: https://groups.io/mt/89225322/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] BaseTools/Conf: Reduce the ASSERT patch to save the binary size

[Guomin Jiang]

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

Use DEBUG_FILE to control ASSERT path

Motivation and Goal:
1. The path will occupy many size in DEBUG build when file path is long
2. We hope can reduce the size but not impact the debug capability
3. If only use filename, we can search the filename to locate file. It
   can save many size meanwhile.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 
Signed-off-by: Guomin Jiang 
---
 BaseTools/Conf/build_rule.template | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index f40118234471..ad0bae42be62 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.
 #  Portions copyright (c) 2008 - 2010, Apple Inc. All rights reserved.
 #  Copyright (c) 2020, ARM Ltd. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -126,14 +126,14 @@
 $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
 
 
-"$(CC)" /Fo${dst} $(DEPS_FLAGS) $(CC_FLAGS) $(INC) ${src}
+"$(CC)" /Fo${dst} $(DEPS_FLAGS) /D DEBUG_FILE="\"${s_base}.c\"" 
$(CC_FLAGS) $(INC) ${src}
 
 
 # For RVCTCYGWIN CC_FLAGS must be first to work around pathing issues
-"$(CC)" $(DEPS_FLAGS) $(CC_FLAGS) -c -o ${dst} $(INC) ${src}
+"$(CC)" $(DEPS_FLAGS) -D DEBUG_FILE="\"${s_base}.c\"" $(CC_FLAGS) -c 
-o ${dst} $(INC) ${src}
 
 
-"$(CC)" $(DEPS_FLAGS) $(CC_FLAGS) -o ${dst} $(INC) ${src}
+"$(CC)" $(DEPS_FLAGS) -D DEBUG_FILE="\"${s_base}.c\"" $(CC_FLAGS) -o 
${dst} $(INC) ${src}
 
 
[C-Code-File.BASE.AARCH64,C-Code-File.SEC.AARCH64,C-Code-File.PEI_CORE.AARCH64,C-Code-File.PEIM.AARCH64,C-Code-File.BASE.ARM,C-Code-File.SEC.ARM,C-Code-File.PEI_CORE.ARM,C-Code-File.PEIM.ARM]
 
@@ -146,7 +146,7 @@
 $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
 
 
-"$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) ${src}
+"$(CC)" -D DEBUG_FILE="\"${s_base}.c\"" $(CC_FLAGS) $(CC_XIPFLAGS) -c 
-o ${dst} $(INC) ${src}
 
 [C-Header-File]
 
-- 
2.35.1.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86747): https://edk2.groups.io/g/devel/message/86747
Mute This Topic: https://groups.io/mt/89225321/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] Reduce the ASSERT patch to save the binary size

[Guomin Jiang]

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

1. Use DEBUG_FILE to control ASSERT path
2. Default use file name as ASSERT path

Motivation and Goal:
1. The path will occupy many size in DEBUG build when file path is long
2. We hope can reduce the size but not impact the debug capability
3. If only use filename, we can search the filename to locate file. It
   can save many size meanwhile.

Guomin Jiang (2):
  BaseTools/Conf: Reduce the ASSERT patch to save the binary size
  MdePkg/Include: Define new DEBUG_FILE to specify path.

 BaseTools/Conf/build_rule.template | 10 
 MdePkg/Include/Library/DebugLib.h  | 39 +-
 2 files changed, 33 insertions(+), 16 deletions(-)

-- 
2.35.1.windows.2



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

Re: [edk2-devel] [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting

[Wu, Hao A]

Inline comments below:


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> MrChromebox
> Sent: Saturday, February 12, 2022 6:46 AM
> To: Wu, Hao A 
> Cc: Rhodes, Sean ; Patrick Rudolph
> ; devel@edk2.groups.io; Wang, Jian J
> ; Gao, Liming ; Ni, Ray
> 
> Subject: Re: [edk2-devel] [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c:
> don't request protocol before setting
> 
> seems this patch got truncated during a rebase, here is the correct version.
> 
> For device that does not respond to GetProtocolRequest(), the original flow
> is:
> a.1 Execute GetProtocolRequest()
> a.2 Timeout occurs for GetProtocolRequest()
> a.3 Conditionally execute UsbSetProtocolRequest()
> 
> The proposed new flow is:
> b.1 Always execute UsbSetProtocolRequest()
> 
> The timeout delay for keyboards which do not support
> GetProtocolRequest() that we've seen is significant (>5s, IIRC)
> 
>  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++
> --
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> index 5a94a4dda7..ad55fa3330 100644
> --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> @@ -805,7 +805,6 @@ InitUSBKeyboard (
>)
>  {
>UINT16  ConfigValue;
> -  UINT8   Protocol;
>EFI_STATUS  Status;
>UINT32  TransferResult;
> 
> @@ -854,21 +853,28 @@ InitUSBKeyboard (
>  }
>}
> 
> -  UsbGetProtocolRequest (
> -UsbKeyboardDevice->UsbIo,
> -UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> -
> -);
>//
>// Set boot protocol for the USB Keyboard.
>// This driver only supports boot protocol.
>//
> -  if (Protocol != BOOT_PROTOCOL) {
> -UsbSetProtocolRequest (
> -  UsbKeyboardDevice->UsbIo,
> -  UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> -  BOOT_PROTOCOL
> +  Status = UsbSetProtocolRequest (
> + UsbKeyboardDevice->UsbIo,
> + UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> + BOOT_PROTOCOL
> + );
> +  if (EFI_ERROR (Status)) {
> +//
> +// If protocol could not be set here, it means
> +// the keyboard interface has some errors and could
> +// not be initialized
> +//
> +REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> +  EFI_ERROR_CODE | EFI_ERROR_MINOR,
> +  (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR),
> +  UsbKeyboardDevice->DevicePath
>);
> +
> +return EFI_DEVICE_ERROR;
>}


I am okay with unconditionally sending the Set_Protocol command.
Could you help to:
1. Just keep the previous behavior for calling UsbSetProtocolRequest() with no 
error handling logic?

2. Check if similar changes need to be applied in:
  * MdeModulePkg\Bus\Usb\UsbMouseAbsolutePointerDxe
  * MdeModulePkg\Bus\Usb\UsbMouseDxe
If you do not have device for verifying the change, I suggest to make the 
change only in UsbKbDxe.

3. Send out a new V2 patch rather than replying the previous patch mail.

Thanks in advance.

Best Regards,
Hao Wu


> 
>UsbKeyboardDevice->CtrlOn= FALSE;
> --
> 2.32.0
> 
> On Thu, Feb 10, 2022 at 9:32 PM Wu, Hao A  wrote:
> >
> > Hello,
> >
> > Inline comments below:
> >
> >
> > > -Original Message-
> > > From: Sean Rhodes 
> > > Sent: Friday, February 11, 2022 5:27 AM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A ; Matt DeVillier
> > > ; Patrick Rudolph
> > > 
> > > Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request
> > > protocol before setting
> > >
> > > From: Matt DeVillier 
> > >
> > > No need to check the interface protocol then conditionally setting,
> > > just set it to BOOT_PROTOCOL and check for error.
> > >
> > > This is what Linux does for HID devices as some don't follow the USB spec.
> > > One example is the Aspeed BMC HID keyboard device, which adds a
> > > massive boot delay without this patch as it doesn't respond to
> 'GetProtocolRequest'.
> > >
> > > Signed-off-by: Matt DeVillier 
> > > Signed-off-by: Patrick Rudolph 
> > > ---
> > >  MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22
> > > +-
> > >  1 file changed, 17 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > index 5a94a4dda7..56d249f937 100644
> > > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c
> > > @@ -863,12 +863,24 @@ InitUSBKeyboard (
> > >// Set boot protocol for the USB Keyboard.
> > >
> > >// This driver only supports boot protocol.
> > >
> > >//
> > >
> > > -  if (Protocol != BOOT_PROTOCOL) {
> > >
> > > -UsbSetProtocolRequest (
> > >
> > > -  UsbKeyboardDevice->UsbIo,
> > >
> > > -  UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber,
> > >
> > > -  BOOT_PROTOCOL
> > >
> > > +  Status = UsbSetProtocolRequest (
> > >
> > > +   

Re: [edk2-devel] [PATCH] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-fatal

[Wu, Hao A]

Acked-by: Hao A Wu 
I recommend to get Ray's input for this patch before merging.

Best Regards,
Hao Wu

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sean
> Rhodes
> Sent: Friday, February 11, 2022 4:05 PM
> To: devel@edk2.groups.io
> Cc: Dong, Guo ; Matt DeVillier
> ; Wu, Hao A ; Ni, Ray
> ; Rhodes, Sean 
> Subject: [edk2-devel] [PATCH] Ps2KbdCtrller: Make wait for SUCCESS after
> BAT non-fatal
> 
> From: Matt DeVillier 
> 
> Recent model Chromebooks only return ACK, but not
> BAT_SUCCESS, which causes hanging and failed ps2k init.
> To mitigate this, make the absence of BAT_SUCCESS reply
> non-fatal, and reduce the no-reply timeout from 4s to 1s.
> 
> Tested on google/dracia and purism/librem_14
> 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> Signed-off-by: Matt DeVillier 
> Signed-off-by: Sean Rhodes 
> ---
>  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 6 +-
>  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   | 2 +-
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> index 77dc226222..6c71355edd 100644
> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c
> @@ -1733,11 +1733,7 @@ InitKeyboard (
>  //
> 
>  mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT;
> 
> 
> 
> -Status = KeyboardWaitForValue (ConsoleIn,
> KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);
> 
> -if (EFI_ERROR (Status)) {
> 
> -  KeyboardError (ConsoleIn, L"Keyboard self test failed!\n\r");
> 
> -  goto Done;
> 
> -}
> 
> +KeyboardWaitForValue (ConsoleIn,
> KEYBOARD_8048_RETURN_8042_BAT_SUCCESS);
> 
> 
> 
>  mWaitForValueTimeOut = KEYBOARD_WAITFORVALUE_TIMEOUT;
> 
> 
> 
> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
> index ca1dd9b2c2..38df3e092d 100644
> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
> +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h
> @@ -157,7 +157,7 @@ InstallPs2KeyboardDriver (
>  #define KEYBOARD_MAX_TRY 256 // 256
> 
>  #define KEYBOARD_TIMEOUT 65536   // 0.07s
> 
>  #define KEYBOARD_WAITFORVALUE_TIMEOUT100 // 1s
> 
> -#define KEYBOARD_BAT_TIMEOUT 400 // 4s
> 
> +#define KEYBOARD_BAT_TIMEOUT 100 // 1s
> 
>  #define KEYBOARD_TIMER_INTERVAL  20  // 0.02s
> 
>  #define SCANCODE_EXTENDED0   0xE0
> 
>  #define SCANCODE_EXTENDED1   0xE1
> 
> --
> 2.32.0
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86611): https://edk2.groups.io/g/devel/message/86611
> Mute This Topic: https://groups.io/mt/89066601/1768737
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a...@intel.com]
> -=-=-=-=-=-=
> 



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

Re: [edk2-devel] [PATCH 1/2] SdMmcPciDxe: Make timeout for SD card configurable

[Wu, Hao A]

Inline comments below:



> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sean
> Rhodes
> Sent: Friday, February 11, 2022 4:43 PM
> To: devel@edk2.groups.io
> Cc: Dong, Guo ; Matt DeVillier
> ; Wu, Hao A ; Ni, Ray
> ; Wang, Jian J ; Gao, Liming
> ; Rhodes, Sean 
> Subject: [edk2-devel] [PATCH 1/2] SdMmcPciDxe: Make timeout for SD card
> configurable


1. Please help to update the commit subject to "MdeModulePkg/SdMmcPciHcDxe: 
Make timeout for SD card configurable"?


> 
> From: Matt DeVillier 
> 
> The default 1s timeout can delay boot splash on some hardware with no
> benefit.
> 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> Signed-off-by: Matt DeVillier 
> Signed-off-by: Sean Rhodes 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 2 +-
>  MdeModulePkg/MdeModulePkg.dec  | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 85e09cf114..c9a21e01bd 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -49,7 +49,7 @@ extern EDKII_SD_MMC_OVERRIDE  *mOverride;
>  //
> 
>  // Generic time out value, 1 microsecond as unit.
> 
>  //
> 
> -#define SD_MMC_HC_GENERIC_TIMEOUT  1 * 1000 * 1000
> 
> +#define SD_MMC_HC_GENERIC_TIMEOUT  ((PcdGet32
> (PcdSdMmcGenericTimeoutValue)


2. The parentheses in the macro definition are not matched.
How about "... (PcdGet32 (PcdSdMmcGenericTimeoutValue))"?
Could you help to verify the patch before sending it out? Thanks in advance.


> 
> 
> 
>  //
> 
>  // SD/MMC async transfer timer interval, set by experience.
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 463e889e9a..092660f7f0 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1559,6 +1559,9 @@
># @Prompt Maximum permitted FwVol section nesting depth (exclusive).
> 
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|
> 0x10|UINT32|0x0030
> 
> 
> 
> +  ## SD Card timeout


3. Could you help to update the PCD description to:
  ## Indicates the default timeout value for SD/MMC Host Controller operations 
in microseconds.
  # @Prompt SD/MMC Host Controller Operations Timeout (us).

> 
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|1000
> 000|UINT32|0x0031
> 
> +


4. I think the patch is missing the PcdLib reference in files SdMmcPciHcDxe.h 
and SdMmcPciHcDxe.inf.

5. The patch is also missing the PcdSdMmcGenericTimeoutValue PCD reference in 
SdMmcPciHcDxe.inf.
Please follow other INF files in edk2 to add the PCD reference.
Also please help to verify the patch before sending it out. Thanks in advance.

6. When adding a new PCD in DEC file, please also help to update the UNI file 
as well.
Could you help to add below lines in file MdeModulePkg.uni:
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_PROMPT 
#language en-US "SD/MMC Host Controller Operations Timeout (us)."

#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSdMmcGenericTimeoutValue_HELP   
#language en-US "Indicates the default timeout value for SD/MMC Host Controller 
operations in microseconds."


Best Regards,
Hao Wu


> 
>  [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> 
>## This PCD defines the Console output row. The default value is 25
> according to UEFI spec.
> 
>#  This PCD could be set to 0 then console output would be at max column
> and max row.
> 
> --
> 2.32.0
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86612): https://edk2.groups.io/g/devel/message/86612
> Mute This Topic: https://groups.io/mt/89066986/1768737
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a...@intel.com]
> -=-=-=-=-=-=
> 



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

Re: [edk2-devel] [PATCH] NetworkPkg: Fix incorrect unicode string of the AKM/Cipher Suite

[Heng Luo]

Dear maintainers,
Could you review this patch?

Thanks,
heng

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Heng Luo
> Sent: Wednesday, January 26, 2022 1:12 PM
> To: devel@edk2.groups.io
> Cc: Maciej Rabeda ; Fu, Siyuan
> ; Wu, Jiaxin 
> Subject: [edk2-devel] [PATCH] NetworkPkg: Fix incorrect unicode string of the
> AKM/Cipher Suite
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3813
> 
> The size of buffer should be 3 CHAR16 for Null-terminated Unicode string.
> The first char is the AKM/Cipher Suite number, the second char is ' ', the 
> third
> char is '\0'.
> 
> Cc: Maciej Rabeda 
> Cc: Fu Siyuan 
> Cc: Wu Jiaxin 
> Signed-off-by: Heng Luo 
> ---
> 
> NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess.c
> | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess
> .c
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess
> .c
> index b49825bcb7..7cb2bfc281 100644
> ---
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess
> .c
> +++
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAcce
> +++ ss.c
> @@ -280,12 +280,16 @@ WifiMgrGetStrAKMList (
>  //
>  // Current AKM Suite is between 1-9
>  //
> -AKMListDisplay = (CHAR16 *)AllocateZeroPool (sizeof (CHAR16) *
> AKMSuiteCount * 2);
> +AKMListDisplay = (CHAR16 *)AllocateZeroPool (sizeof (CHAR16) *
> + (AKMSuiteCount * 2 + 1));
>  if (AKMListDisplay != NULL) {
>for (Index = 0; Index < AKMSuiteCount; Index++) {
> +//
> +// The size of buffer should be 3 CHAR16 for Null-terminated Unicode
> string.
> +// The first char is the AKM Suite number, the second char is ' ', 
> the third
> char is '\0'.
> +//
>  UnicodeSPrint (
>AKMListDisplay + (Index * 2),
> -  sizeof (CHAR16) * 2,
> +  sizeof (CHAR16) * 3,
>L"%d ",
>Profile->Network.AKMSuite->AKMSuiteList[Index].SuiteType
>);
> @@ -333,12 +337,16 @@ WifiMgrGetStrCipherList (
>  //
>  // Current Cipher Suite is between 1-9
>  //
> -CipherListDisplay = (CHAR16 *)AllocateZeroPool (sizeof (CHAR16) *
> CipherSuiteCount * 2);
> +CipherListDisplay = (CHAR16 *)AllocateZeroPool (sizeof (CHAR16) *
> + (CipherSuiteCount * 2 + 1));
>  if (CipherListDisplay != NULL) {
>for (Index = 0; Index < CipherSuiteCount; Index++) {
> +//
> +// The size of buffer should be 3 CHAR16 for Null-terminated Unicode
> string.
> +// The first char is the Cipher Suite number, the second char is ' 
> ', the third
> char is '\0'.
> +//
>  UnicodeSPrint (
>CipherListDisplay + (Index * 2),
> -  sizeof (CHAR16) * 2,
> +  sizeof (CHAR16) * 3,
>L"%d ",
>Profile->Network.CipherSuite->CipherSuiteList[Index].SuiteType
>);
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [edk2-platforms: PATCH v4] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.

[Chiu, Chasel]

Fix pushed: 
https://github.com/tianocore/edk2-platforms/commit/cbc8e420ac4505e9c51aa0d4f049026691024ca5

Thanks,
Chasel


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chiu, Chasel
> Sent: Friday, February 18, 2022 7:56 AM
> To: Desimone, Nathaniel L ;
> devel@edk2.groups.io
> Cc: Gao, Liming ; Dong, Eric
> ; Oram, Isaac W 
> Subject: Re: [edk2-devel] [edk2-platforms: PATCH v4]
> MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> 
> 
> Thanks Nate, Isaac for good catch! I will correct them before pushing.
> 
> > -Original Message-
> > From: Desimone, Nathaniel L 
> > Sent: Friday, February 18, 2022 7:22 AM
> > To: devel@edk2.groups.io; Chiu, Chasel 
> > Cc: Gao, Liming ; Dong, Eric
> > ; Oram, Isaac W 
> > Subject: RE: [edk2-devel] [edk2-platforms: PATCH v4]
> > MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> >
> > Hi Chasel,
> >
> > There are a couple of typos and minor code style issues pointed out below.
> > Please fix those during the push... no need to send another patch.
> >
> > Reviewed-by: Nate DeSimone 
> >
> > Thanks,
> > Nate
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Chiu,
> > > Chasel
> > > Sent: Wednesday, February 16, 2022 10:21 PM
> > > To: devel@edk2.groups.io
> > > Cc: Chiu, Chasel ; Desimone, Nathaniel L
> > > ; Gao, Liming
> > > ; Dong, Eric ; Oram,
> > > Isaac W 
> > > Subject: [edk2-devel] [edk2-platforms: PATCH v4]
> > > MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> > >
> > > From: "Chiu, Chasel" 
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
> > >
> > > Fixed the bug that existing variable will not be locked when it is
> > > identical with hob data by creating LockLargeVariable function, also
> > > switched to VariablePolicyProtocol for locking variables.
> > >
> > > Failing to lock variable could be security vulnerability, so the
> > > LargeVariableWriteLib functions will return EFI_ABORTED if locking
> > > was failed and SaveMemoryConfig driver will delete variable to
> > > prevent from using unlocked variable.
> > >
> > > This patch also modified SaveMemoryConfig driver to be unloaded
> > > after execution because it does not produce any service protocol. To
> > > achieve this goal the DxeRuntimeVariableWriteLib should close
> > > registered ExitBootService events in its DESTRUCTOR.
> > >
> > > Cc: Nate DeSimone 
> > > Cc: Liming Gao 
> > > Cc: Eric Dong 
> > > Signed-off-by: Chasel Chiu 
> > > Reviewed-by: Isaac Oram  ---Patch V4:
> > > 1.Removed CpuDeadLoop and delete variable if Lock failed in
> > SaveMemoryConfig. 2. LargeVariableWrite will not delete variable if
> > Lock failed, let caller to do it.
> > >
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> > onfig.c|  37 ++---
> > >
> >
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWri
> > teLib.c  | 142
> >
> +
> >
> +
> > 
> > >
> >
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> > meVariableWriteLib.c   |  61
> > +
> > >
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> > onfig.inf  |   3 ++-
> > >
> > > Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.
> > > h
> > |  26 --
> > >
> >
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> > meVariableWriteLib.inf |   8 +---
> > >  6 files changed, 248 insertions(+), 29 deletions(-)
> > >
> > > diff --git
> > >
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > y
> > > Config.c
> > >
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > y
> > > Config.c
> > > index 820585f676..fb51edd5cb 100644
> > > ---
> > >
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> > y
> > > Config.c
> > > +++
> > b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMe
> > > +++ moryConfig.c
> > > @@ -2,13 +2,14 @@
> > >This is the driver that locates the MemoryConfigurationData HOB, if it
> > >exists, and saves the data to nvRAM.
> > >
> > > -Copyright (c) 2017 - 2021, Intel Corporation. All rights
> > > reserved.
> > > +Copyright (c) 2017 - 2022, Intel Corporation. All rights
> > > +reserved.
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > >
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > #include   #include
> > >   #include
> > > 
> > > +#include 
> > >  #include 
> > >
> > >  /**
> > > @@ -86,6 +88,23 @@ SaveMemoryConfigEntryPoint (
> > >

Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different values for PEI and DXE?

[Michael D Kinney]

Hi Andrew,

I forgot about the library constraints.

Unless we want to first add a feature to build a library instance for each 
module type into
a different build output directory, we cannot support setting a FixedAtBuild 
PCD for each
module type.

If you use PatchableInModule, you can post process all the modules to set 
individual
module PCD values.  You have to do two steps.  One DSC to build all the modules 
and
generate AsBuiltInf.  2nd DSC/FDF that references the AsBuildInfs and sets the
PatchableInModule PCD values in the scope of the modules needed.

The only reason the different ARCHs for PEI and DXE works is because the libs
for each ARCH are build separately and already have different build output 
locations.

Mike



From: Andrew Fish 
Sent: Thursday, February 17, 2022 4:42 PM
To: devel@edk2.groups.io; Oram, Isaac W 
Cc: Kinney, Michael D 
Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
values for PEI and DXE?




On Feb 17, 2022, at 4:26 PM, Oram, Isaac W 
mailto:isaac.w.o...@intel.com>> wrote:

Andrew,

It is a reasonable ask, but I lean towards voting no.  We can more or less do 
the equivalent by breaking a monolithic build into a set of phase specific 
build/DSC/FDF or something similar.  That seems more flexible anyway.  Maybe we 
want binaries with settings based on maturity more than phase.  And so on.


My primary concern that the complexity added to an already complex feature.  
PCD have a lot of binary compatibility challenges anyway.  And this doesn't add 
any that don't already exist, but it does make it a lot easier to introduce a 
mismatch issue.


I shot my self in the foot with the Fixed* macro in a library so I did not get 
the drivers override. So I feel that pain. You need to used the non Fixed form 
from a lib to get the value from the global produced by the driver.



My more recreational question is if this makes sense in most cases, or if there 
are a limited set of PCD that it makes sense for.  A lot of similar PCD are 
much more like shared constants and thus it doesn't make a lot of sense for 
them to vary across executables.  E.G. drivers are asking for problems if they 
change max string lengths inconsistently. There are definitely more than one 
PCD where it seems reasonable.  Maybe they are the "wrong" type though.  Maybe 
they should be a dynamic type if a flexible usage is desired.


I was trying to have different settings for 
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask is regards to the action taken on 
an ASSERT.

To be honest it was not my 1st choice and I was working around some early boot 
exception handling code not saving the EFI_SYSTEM_CONTEXT and blocking the 
scheme I was trying to use.



Tangentially related, there was some previous work to make macros for phase 
architectures.

[Defines]
 PEI_ARCH= IA32
 DXE_ARCH= X64

Enabling [Components.$(PEI_ARCH)] and [Components.$(DXE_ARCH)]


In my case PEI_ARCH == DXE_ARCH so this kind of trick is not helpful. The 
reality is this trick only works for non virtualized X64 platforms and the edk2 
supports a lot more flavors of platforms than X64.

Thanks,

Andrew Fish


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

If we do something with PCD, we should probably try to be consistent with this 
and maybe extend it or integrate it into DSC syntax.  With standalone MM, it 
probably makes more sense to have PEI, DXE, and SMM.  RT would also make sense 
to have different settings.  I have mixed feelings about BDS.  I think it makes 
sense to see it as a potentially independent phase that isn't currently.  But 
also probably won't ever be independent from DXE.

Regards,
Isaac


-Original Message-
From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Andrew Fish 
via groups.io
Sent: Thursday, February 17, 2022 12:22 PM
To: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
values for PEI and DXE?




On Feb 17, 2022, at 10:52 AM, Kinney, Michael D 
mailto:michael.d.kin...@intel.com>> wrote:

I agree that your approach is the only way right now.

Do you have a feature request???

Well it does seem like a reasonable thing to be able to do? What do other 
people think?

I was able to refactor my code and I did not actually end up needing to 
override every PEIM and the PEI Core.

Thanks,

Andrew Fish



Mike


-Original Message-
From: Andrew Fish mailto:af...@apple.com>>
Sent: Wednesday, February 16, 2022 7:26 PM
To: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
values for PEI and DXE?




On Feb 16, 2022, at 2:10 PM, Kinney, Michael D 

Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different values for PEI and DXE?

[Andrew Fish via groups.io]

> On Feb 17, 2022, at 4:26 PM, Oram, Isaac W  wrote:
> 
> Andrew,
> 
> It is a reasonable ask, but I lean towards voting no.  We can more or less do 
> the equivalent by breaking a monolithic build into a set of phase specific 
> build/DSC/FDF or something similar.  That seems more flexible anyway.  Maybe 
> we want binaries with settings based on maturity more than phase.  And so on.
> 
> 
> My primary concern that the complexity added to an already complex feature.  
> PCD have a lot of binary compatibility challenges anyway.  And this doesn't 
> add any that don't already exist, but it does make it a lot easier to 
> introduce a mismatch issue.
> 

I shot my self in the foot with the Fixed* macro in a library so I did not get 
the drivers override. So I feel that pain. You need to used the non Fixed form 
from a lib to get the value from the global produced by the driver. 

> 
> My more recreational question is if this makes sense in most cases, or if 
> there are a limited set of PCD that it makes sense for.  A lot of similar PCD 
> are much more like shared constants and thus it doesn't make a lot of sense 
> for them to vary across executables.  E.G. drivers are asking for problems if 
> they change max string lengths inconsistently. There are definitely more than 
> one PCD where it seems reasonable.  Maybe they are the "wrong" type though.  
> Maybe they should be a dynamic type if a flexible usage is desired.
> 

I was trying to have different settings for 
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask is regards to the action taken on 
an ASSERT. 

To be honest it was not my 1st choice and I was working around some early boot 
exception handling code not saving the EFI_SYSTEM_CONTEXT and blocking the 
scheme I was trying to use. 

> 
> Tangentially related, there was some previous work to make macros for phase 
> architectures.
> 
> [Defines]
>  PEI_ARCH= IA32
>  DXE_ARCH= X64
> 
> Enabling [Components.$(PEI_ARCH)] and [Components.$(DXE_ARCH)]
> 

In my case PEI_ARCH == DXE_ARCH so this kind of trick is not helpful. The 
reality is this trick only works for non virtualized X64 platforms and the edk2 
supports a lot more flavors of platforms than X64. 

Thanks,

Andrew Fish

> https://bugzilla.tianocore.org/show_bug.cgi?id=2308 
> 
> 
> If we do something with PCD, we should probably try to be consistent with 
> this and maybe extend it or integrate it into DSC syntax.  With standalone 
> MM, it probably makes more sense to have PEI, DXE, and SMM.  RT would also 
> make sense to have different settings.  I have mixed feelings about BDS.  I 
> think it makes sense to see it as a potentially independent phase that isn't 
> currently.  But also probably won't ever be independent from DXE.
> 
> Regards,
> Isaac
> 
> 
> -Original Message-
> From: devel@edk2.groups.io  
> mailto:devel@edk2.groups.io>> On Behalf Of Andrew Fish 
> via groups.io 
> Sent: Thursday, February 17, 2022 12:22 PM
> To: Kinney, Michael D  >
> Cc: devel@edk2.groups.io 
> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
> values for PEI and DXE?
> 
> 
> 
>> On Feb 17, 2022, at 10:52 AM, Kinney, Michael D  
>> wrote:
>> 
>> I agree that your approach is the only way right now.
>> 
>> Do you have a feature request???
> 
> Well it does seem like a reasonable thing to be able to do? What do other 
> people think?
> 
> I was able to refactor my code and I did not actually end up needing to 
> override every PEIM and the PEI Core.
> 
> Thanks,
> 
> Andrew Fish
> 
>> 
>> Mike
>> 
>>> -Original Message-
>>> From: Andrew Fish 
>>> Sent: Wednesday, February 16, 2022 7:26 PM
>>> To: Kinney, Michael D 
>>> Cc: devel@edk2.groups.io
>>> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set 
>>> different values for PEI and DXE?
>>> 
>>> 
>>> 
 On Feb 16, 2022, at 2:10 PM, Kinney, Michael D 
  wrote:
 
 Hi Andrew,
 
 Current DSC syntax for platform scoped [PcdsXXX] sections only 
 supports CPU Arch and SKUID.
 
 So there is no mechanism today to specify different PCD values based 
 on module type.
 
 You can manage this in the DSC file, but it does require the module 
 scoped  section for each module INF that requires a 
 different value that the platform scoped [PcdXXX] section.
 
>>> 
>>> Mike,
>>> 
>>> That is what I ended up doing, but it required overriding every PEIM and 
>>> PEI Core. Seemed kind of excessive.
>>> 
>>> I think people cheat and use IA32 vs X64 to mean PEI vs. DXE on X64 
>>> platforms.
>>> 
>>> Thanks,
>>> 
>>> Andrew Fish
>>> 
 Mike
 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of 
> Andrew Fish via groups.io
> Sent: Wednesday, February 

Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different values for PEI and DXE?

[Oram, Isaac W]

Andrew,

It is a reasonable ask, but I lean towards voting no.  We can more or less do 
the equivalent by breaking a monolithic build into a set of phase specific 
build/DSC/FDF or something similar.  That seems more flexible anyway.  Maybe we 
want binaries with settings based on maturity more than phase.  And so on.


My primary concern that the complexity added to an already complex feature.  
PCD have a lot of binary compatibility challenges anyway.  And this doesn't add 
any that don't already exist, but it does make it a lot easier to introduce a 
mismatch issue.


My more recreational question is if this makes sense in most cases, or if there 
are a limited set of PCD that it makes sense for.  A lot of similar PCD are 
much more like shared constants and thus it doesn't make a lot of sense for 
them to vary across executables.  E.G. drivers are asking for problems if they 
change max string lengths inconsistently.  There are definitely more than one 
PCD where it seems reasonable.  Maybe they are the "wrong" type though.  Maybe 
they should be a dynamic type if a flexible usage is desired.


Tangentially related, there was some previous work to make macros for phase 
architectures.

[Defines]
  PEI_ARCH= IA32
  DXE_ARCH= X64

Enabling [Components.$(PEI_ARCH)] and [Components.$(DXE_ARCH)]

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

If we do something with PCD, we should probably try to be consistent with this 
and maybe extend it or integrate it into DSC syntax.  With standalone MM, it 
probably makes more sense to have PEI, DXE, and SMM.  RT would also make sense 
to have different settings.  I have mixed feelings about BDS.  I think it makes 
sense to see it as a potentially independent phase that isn't currently.  But 
also probably won't ever be independent from DXE.

Regards,
Isaac


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Andrew Fish via 
groups.io
Sent: Thursday, February 17, 2022 12:22 PM
To: Kinney, Michael D 
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
values for PEI and DXE?



> On Feb 17, 2022, at 10:52 AM, Kinney, Michael D  
> wrote:
> 
> I agree that your approach is the only way right now.
> 
> Do you have a feature request???

Well it does seem like a reasonable thing to be able to do? What do other 
people think?

I was able to refactor my code and I did not actually end up needing to 
override every PEIM and the PEI Core.

Thanks,

Andrew Fish

> 
> Mike
> 
>> -Original Message-
>> From: Andrew Fish 
>> Sent: Wednesday, February 16, 2022 7:26 PM
>> To: Kinney, Michael D 
>> Cc: devel@edk2.groups.io
>> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set 
>> different values for PEI and DXE?
>> 
>> 
>> 
>>> On Feb 16, 2022, at 2:10 PM, Kinney, Michael D  
>>> wrote:
>>> 
>>> Hi Andrew,
>>> 
>>> Current DSC syntax for platform scoped [PcdsXXX] sections only 
>>> supports CPU Arch and SKUID.
>>> 
>>> So there is no mechanism today to specify different PCD values based 
>>> on module type.
>>> 
>>> You can manage this in the DSC file, but it does require the module 
>>> scoped  section for each module INF that requires a 
>>> different value that the platform scoped [PcdXXX] section.
>>> 
>> 
>> Mike,
>> 
>> That is what I ended up doing, but it required overriding every PEIM and PEI 
>> Core. Seemed kind of excessive.
>> 
>> I think people cheat and use IA32 vs X64 to mean PEI vs. DXE on X64 
>> platforms.
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> Mike
>>> 
 -Original Message-
 From: devel@edk2.groups.io  On Behalf Of 
 Andrew Fish via groups.io
 Sent: Wednesday, February 16, 2022 1:02 PM
 To: edk2-devel-groups-io 
 Subject: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
 values for PEI and DXE?
 
 I’m trying to have a different platform policy for 
 PcdDebugPropertyMask in PEI and DXE. I can’t figure out how to do without 
 overriding every PEIM that I build?
 
 My PEI and DXE has the same arch so I can’t use the CPU Arch to tell them 
 apart.
 
 Is there something I’m missing?
 
 Thanks,
 
 Andrew Fish
 
 
 
>>> 
> 








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

Re: [edk2-devel] [edk2-platforms: PATCH v4] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.

[Chiu, Chasel]

Thanks Nate, Isaac for good catch! I will correct them before pushing.

> -Original Message-
> From: Desimone, Nathaniel L 
> Sent: Friday, February 18, 2022 7:22 AM
> To: devel@edk2.groups.io; Chiu, Chasel 
> Cc: Gao, Liming ; Dong, Eric
> ; Oram, Isaac W 
> Subject: RE: [edk2-devel] [edk2-platforms: PATCH v4]
> MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> 
> Hi Chasel,
> 
> There are a couple of typos and minor code style issues pointed out below.
> Please fix those during the push... no need to send another patch.
> 
> Reviewed-by: Nate DeSimone 
> 
> Thanks,
> Nate
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Chiu,
> > Chasel
> > Sent: Wednesday, February 16, 2022 10:21 PM
> > To: devel@edk2.groups.io
> > Cc: Chiu, Chasel ; Desimone, Nathaniel L
> > ; Gao, Liming
> > ; Dong, Eric ; Oram,
> > Isaac W 
> > Subject: [edk2-devel] [edk2-platforms: PATCH v4]
> > MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> >
> > From: "Chiu, Chasel" 
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
> >
> > Fixed the bug that existing variable will not be locked when it is
> > identical with hob data by creating LockLargeVariable function, also
> > switched to VariablePolicyProtocol for locking variables.
> >
> > Failing to lock variable could be security vulnerability, so the
> > LargeVariableWriteLib functions will return EFI_ABORTED if locking was
> > failed and SaveMemoryConfig driver will delete variable to prevent
> > from using unlocked variable.
> >
> > This patch also modified SaveMemoryConfig driver to be unloaded after
> > execution because it does not produce any service protocol. To achieve
> > this goal the DxeRuntimeVariableWriteLib should close registered
> > ExitBootService events in its DESTRUCTOR.
> >
> > Cc: Nate DeSimone 
> > Cc: Liming Gao 
> > Cc: Eric Dong 
> > Signed-off-by: Chasel Chiu 
> > Reviewed-by: Isaac Oram  ---Patch V4:
> > 1.Removed CpuDeadLoop and delete variable if Lock failed in
> SaveMemoryConfig. 2. LargeVariableWrite will not delete variable if Lock 
> failed,
> let caller to do it.
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> onfig.c|  37 ++---
> >
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWri
> teLib.c  | 142
> +
> +
> 
> >
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> meVariableWriteLib.c   |  61
> +
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> onfig.inf  |   3 ++-
> >  Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> |  26 --
> >
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> meVariableWriteLib.inf |   8 +---
> >  6 files changed, 248 insertions(+), 29 deletions(-)
> >
> > diff --git
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.c
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.c
> > index 820585f676..fb51edd5cb 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.c
> > +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMe
> > +++ moryConfig.c
> > @@ -2,13 +2,14 @@
> >This is the driver that locates the MemoryConfigurationData HOB, if it
> >exists, and saves the data to nvRAM.
> >
> > -Copyright (c) 2017 - 2021, Intel Corporation. All rights
> > reserved.
> > +Copyright (c) 2017 - 2022, Intel Corporation. All rights
> > +reserved.
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> >
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include   #include
> >   #include
> > 
> > +#include 
> >  #include 
> >
> >  /**
> > @@ -86,6 +88,23 @@ SaveMemoryConfigEntryPoint (
> >  Status = GetLargeVariable (L"FspNvsBuffer",
> , , VariableData);
> >  if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 ==
> CompareMem (HobData, VariableData, DataSize))) {
> >DataIsIdentical = TRUE;
> > +  //
> > +  // No need to update Variable, only lock it.
> > +  //
> > +  Status = LockLargeVariable (L"FspNvsBuffer",
> );
> > +  if (EFI_ERROR (Status)) {
> > +//
> > +// Fail to lock variable is security vulnerability and 
> > should not happen.
> > +//
> > +ASSERT_EFI_ERROR (Status);
> > +//
> > +// When building without ASSERT_EFI_ERROR hang, 

Re: [edk2-devel] [edk2-platforms: PATCH v4] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.

[Nate DeSimone]

Hi Chasel,

There are a couple of typos and minor code style issues pointed out below. 
Please fix those during the push... no need to send another patch.

Reviewed-by: Nate DeSimone 

Thanks,
Nate

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chiu,
> Chasel
> Sent: Wednesday, February 16, 2022 10:21 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Gao, Liming
> ; Dong, Eric ; Oram,
> Isaac W 
> Subject: [edk2-devel] [edk2-platforms: PATCH v4]
> MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> 
> From: "Chiu, Chasel" 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
> 
> Fixed the bug that existing variable will not be locked when it is
> identical with hob data by creating LockLargeVariable function, also
> switched to VariablePolicyProtocol for locking variables.
> 
> Failing to lock variable could be security vulnerability, so the
> LargeVariableWriteLib functions will return EFI_ABORTED if locking
> was failed and SaveMemoryConfig driver will delete variable to
> prevent from using unlocked variable.
> 
> This patch also modified SaveMemoryConfig driver to be unloaded after
> execution because it does not produce any service protocol. To achieve
> this goal the DxeRuntimeVariableWriteLib should close registered
> ExitBootService events in its DESTRUCTOR.
> 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Signed-off-by: Chasel Chiu 
> Reviewed-by: Isaac Oram 
> ---Patch V4: 1.Removed CpuDeadLoop and delete variable if Lock failed in 
> SaveMemoryConfig. 2. LargeVariableWrite will not delete variable if Lock 
> failed, let caller to do it.
>  Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c 
>|  37 ++---
>  
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
>   | 142 
> ++
>  
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
>|  61 +
>  
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
>   |   3 ++-
>  Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
>|  26 --
>  
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
>  |   8 +---
>  6 files changed, 248 insertions(+), 29 deletions(-)
> 
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
>  
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
> index 820585f676..fb51edd5cb 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
> +++ 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
> @@ -2,13 +2,14 @@
>This is the driver that locates the MemoryConfigurationData HOB, if it
>exists, and saves the data to nvRAM.
>  
> -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
> +Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /**
> @@ -86,6 +88,23 @@ SaveMemoryConfigEntryPoint (
>  Status = GetLargeVariable (L"FspNvsBuffer", 
> , , VariableData);
>  if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 == 
> CompareMem (HobData, VariableData, DataSize))) {
>DataIsIdentical = TRUE;
> +  //
> +  // No need to update Variable, only lock it.
> +  //
> +  Status = LockLargeVariable (L"FspNvsBuffer",  
> );
> +  if (EFI_ERROR (Status)) {
> +//
> +// Fail to lock variable is security vulnerability and 
> should not happen.
> +//
> +ASSERT_EFI_ERROR (Status);
> +//
> +// When building without ASSERT_EFI_ERROR hang, delete the 
> varialbe so it will not be consumed.

Typo here, "varialbe" should be "variable".

> +//
> +DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
> +DataSize = 0;
> +Status = SetLargeVariable (L"FspNvsBuffer", 
> , FALSE, DataSize, HobData);
> +ASSERT_EFI_ERROR (Status);
> +  }
>  }
>  FreePool (VariableData);
>}
> @@ -95,6 +114,18 @@ SaveMemoryConfigEntryPoint (
>  
>if (!DataIsIdentical) {
>  Status = SetLargeVariable 

Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different values for PEI and DXE?

[Andrew Fish via groups.io]

> On Feb 17, 2022, at 10:52 AM, Kinney, Michael D  
> wrote:
> 
> I agree that your approach is the only way right now.
> 
> Do you have a feature request???

Well it does seem like a reasonable thing to be able to do? What do other 
people think?

I was able to refactor my code and I did not actually end up needing to 
override every PEIM and the PEI Core.

Thanks,

Andrew Fish

> 
> Mike
> 
>> -Original Message-
>> From: Andrew Fish 
>> Sent: Wednesday, February 16, 2022 7:26 PM
>> To: Kinney, Michael D 
>> Cc: devel@edk2.groups.io
>> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set 
>> different values for PEI and DXE?
>> 
>> 
>> 
>>> On Feb 16, 2022, at 2:10 PM, Kinney, Michael D  
>>> wrote:
>>> 
>>> Hi Andrew,
>>> 
>>> Current DSC syntax for platform scoped [PcdsXXX] sections only supports
>>> CPU Arch and SKUID.
>>> 
>>> So there is no mechanism today to specify different PCD values based on
>>> module type.
>>> 
>>> You can manage this in the DSC file, but it does require the module
>>> scoped  section for each module INF that requires a different
>>> value that the platform scoped [PcdXXX] section.
>>> 
>> 
>> Mike,
>> 
>> That is what I ended up doing, but it required overriding every PEIM and PEI 
>> Core. Seemed kind of excessive.
>> 
>> I think people cheat and use IA32 vs X64 to mean PEI vs. DXE on X64 
>> platforms.
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> Mike
>>> 
 -Original Message-
 From: devel@edk2.groups.io  On Behalf Of Andrew Fish 
 via groups.io
 Sent: Wednesday, February 16, 2022 1:02 PM
 To: edk2-devel-groups-io 
 Subject: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
 values for PEI and DXE?
 
 I’m trying to have a different platform policy for PcdDebugPropertyMask in 
 PEI and DXE. I can’t figure out how to do without
 overriding every PEIM that I build?
 
 My PEI and DXE has the same arch so I can’t use the CPU Arch to tell them 
 apart.
 
 Is there something I’m missing?
 
 Thanks,
 
 Andrew Fish
 
 
 
>>> 
> 



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

Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different values for PEI and DXE?

[Michael D Kinney]

I agree that your approach is the only way right now.

Do you have a feature request???

Mike

> -Original Message-
> From: Andrew Fish 
> Sent: Wednesday, February 16, 2022 7:26 PM
> To: Kinney, Michael D 
> Cc: devel@edk2.groups.io
> Subject: Re: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
> values for PEI and DXE?
> 
> 
> 
> > On Feb 16, 2022, at 2:10 PM, Kinney, Michael D  
> > wrote:
> >
> > Hi Andrew,
> >
> > Current DSC syntax for platform scoped [PcdsXXX] sections only supports
> > CPU Arch and SKUID.
> >
> > So there is no mechanism today to specify different PCD values based on
> > module type.
> >
> > You can manage this in the DSC file, but it does require the module
> > scoped  section for each module INF that requires a different
> > value that the platform scoped [PcdXXX] section.
> >
> 
> Mike,
> 
> That is what I ended up doing, but it required overriding every PEIM and PEI 
> Core. Seemed kind of excessive.
> 
> I think people cheat and use IA32 vs X64 to mean PEI vs. DXE on X64 platforms.
> 
> Thanks,
> 
> Andrew Fish
> 
> > Mike
> >
> >> -Original Message-
> >> From: devel@edk2.groups.io  On Behalf Of Andrew Fish 
> >> via groups.io
> >> Sent: Wednesday, February 16, 2022 1:02 PM
> >> To: edk2-devel-groups-io 
> >> Subject: [edk2-devel] PcdDebugPropertyMask in DSC: How to I set different 
> >> values for PEI and DXE?
> >>
> >> I’m trying to have a different platform policy for PcdDebugPropertyMask in 
> >> PEI and DXE. I can’t figure out how to do without
> >> overriding every PEIM that I build?
> >>
> >> My PEI and DXE has the same arch so I can’t use the CPU Arch to tell them 
> >> apart.
> >>
> >> Is there something I’m missing?
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >> 
> >>
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86734): https://edk2.groups.io/g/devel/message/86734
Mute This Topic: https://groups.io/mt/89195478/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 0/2] EDK2 Code First: PI Specification: Expand PI Status Codes

[Michael D Kinney]

Hi Kun,

The EDK II Code First Process says the branch should be 
against the edk2-staging repo.  

Your patches are for the edk2 repo.

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Kun Qin
> Sent: Wednesday, February 16, 2022 6:13 PM
> To: devel@edk2.groups.io
> Cc: Andrew Fish ; Leif Lindholm ; Kinney, 
> Michael D ; Gao, Liming
> ; Liu, Zhiguang 
> Subject: Re: [edk2-devel] [PATCH v3 0/2] EDK2 Code First: PI Specification: 
> Expand PI Status Codes
> 
> Hi community maintainers,
> 
> It has been a few weeks since the patches being sent out. Could anyone
> please provide any feedback on this code-first series so that we can
> keep the wheels rolling as the first step of N?
> 
> Thanks in advance.
> 
> Regards,
> Kun
> 
> On 02/03/2022 11:03, Kun Qin via groups.io wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794
> >
> > This patch series is a rebase of previous submission:
> > https://edk2.groups.io/g/devel/message/85335
> >
> > In current Status Codes definitions of PI spec v1.7 errata, there are a
> > few instances where the software could trigger system reboots while the
> > corresponding case were not covered by the already defined status codes.
> >
> > One scenario that OEMs would be interested is that fragmented memory map
> > from boot to boot would fail to meet certain OS ACPI requirements (i.e.
> > S4 resume boot requires consistent memory maps) and trigger system
> > reboots. Yet the corresponding case was not covered by the already
> > defined status codes.
> >
> > The unexpected system reboots above could indicate decay of system health
> > and reporting of such generic events would provide helpful information to
> > OEMs to investigate/prevent system failures in general.
> >
> > The change intends to expand definitions of `EFI_SW_EC_**` under Status
> > Codes to cover more unexpected system reboot events, which could improve
> > Status Code futility and readability.
> >
> > Compared to previous series, v3 patch changes mainly include:
> > a. Added BZ3794 prefix for newly added macro definitions;
> >
> > Patch v3 branch: 
> > https://github.com/kuqin12/edk2/tree/BZ3794-expand_status_codes_v3
> >
> > Cc: Andrew Fish 
> > Cc: Leif Lindholm 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> >
> > Kun Qin (2):
> >EDK2 Code First: PI Specification: New error codes of Host Software
> >  class
> >MdePkg: MmCommunication: Add new Host Software class Error Code to
> >  MdePkg
> >
> >   CodeFirst/BZ3794-SpecChange.md   | 55 
> >   MdePkg/Include/Pi/PiStatusCode.h |  1 +
> >   2 files changed, 56 insertions(+)
> >   create mode 100644 CodeFirst/BZ3794-SpecChange.md
> >
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH 1/2] UefiPayloadPkg: Add i801 SMBus controller DXE

[Ma, Maurice]

This vendor specific device driver implementation does not seem to fit into 
UEFI payload package scope well.   

Do you think https://github.com/tianocore/edk2-platforms/tree/master/Silicon 
could be a better location ?

Thanks
Maurice
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sean
> Rhodes
> Sent: Thursday, February 17, 2022 9:51
> To: devel@edk2.groups.io
> Cc: Dong, Guo ; Patrick Rudolph
> ; Ni, Ray ; Ma,
> Maurice ; You, Benjamin
> 
> Subject: [edk2-devel] [PATCH 1/2] UefiPayloadPkg: Add i801 SMBus
> controller DXE
> 
> From: Patrick Rudolph 
> 
> Implement a subset of the gEfiSmbusHcProtocolGuid using a generic PCI i801
> SMBus controller.
> 
> Cc: Guo Dong 
> Cc: Ray Ni 
> Cc: Maurice Ma 
> Cc: Benjamin You 
> Signed-off-by: Patrick Rudolph 
> ---
>  .../Library/BrotliCustomDecompressLib/brotli  |   2 +-
>  UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c| 556
> ++
>  UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.h|  17 +
>  UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.inf  |  45 ++
>  UefiPayloadPkg/UefiPayloadPkg.dsc |   7 +
>  UefiPayloadPkg/UefiPayloadPkg.fdf |   5 +
>  6 files changed, 631 insertions(+), 1 deletion(-)  create mode 100644
> UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
>  create mode 100644 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.h
>  create mode 100644 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.inf
> 
> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
> b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
> index f4153a09f8..666c3280cc 16
> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
> @@ -1 +1 @@
> -Subproject commit f4153a09f87cbb9c826d8fc12c74642bb2d879ea
> +Subproject commit 666c3280cc11dc433c303d79a83d4ffbdd12cc8d
> diff --git a/UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
> b/UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
> new file mode 100644
> index 00..7bf7b893ad
> --- /dev/null
> +++ b/UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
> @@ -0,0 +1,556 @@
> +/** @file+  Implementation for a generic i801 SMBus driver.++Copyright (c)
> 2016, Intel Corporation. All rights reserved.+SPDX-License-Identifier:
> BSD-2-Clause-Patent+++**/++#include "SMBusi801Dxe.h"+#include
> +#include +#include
> +#include +#include
> +#include
> ++EFI_HANDLE  mDriverHandle =
> NULL;+UINT32  PciDevice = 0;++/* SMBus register offsets. */+#define
> SMBHSTSTAT  0x0+#define SMBHSTCTL   0x2+#define SMBHSTCMD
> 0x3+#define SMBXMITADD  0x4+#define SMBHSTDAT0  0x5+#define
> SMBHSTDAT1  0x6+#define SMBBLKDAT   0x7+#define SMBTRNSADD
> 0x9+#define SMBSLVDATA  0xa+#define SMLINK_PIN_CTL  0xe+#define
> SMBUS_PIN_CTL   0xf+#define SMBSLVCMD   0x11++/* I801 command
> constants */+#define I801_QUICK   (0 << 2)+#define I801_BYTE  
>   (1
> << 2)+#define I801_BYTE_DATA   (2 << 2)+#define I801_WORD_DATA   (3
> << 2)+#define I801_PROCESS_CALL(4 << 2)+#define I801_BLOCK_DATA
> (5 << 2)+#define I801_I2C_BLOCK_DATA  (6 << 2) /* ICH5 and later */++/*
> I801 Host Control register bits */+#define SMBHSTCNT_INTREN (1 <<
> 0)+#define SMBHSTCNT_KILL   (1 << 1)+#define SMBHSTCNT_LAST_BYTE
> (1 << 5)+#define SMBHSTCNT_START  (1 << 6)+#define
> SMBHSTCNT_PEC_EN (1 << 7)/* ICH3 and later */++/* I801 Hosts Status
> register bits */+#define SMBHSTSTS_BYTE_DONE (1 << 7)+#define
> SMBHSTSTS_INUSE_STS (1 << 6)+#define SMBHSTSTS_SMBALERT_STS  (1
> << 5)+#define SMBHSTSTS_FAILED(1 << 4)+#define
> SMBHSTSTS_BUS_ERR   (1 << 3)+#define SMBHSTSTS_DEV_ERR   (1 <<
> 2)+#define SMBHSTSTS_INTR  (1 << 1)+#define SMBHSTSTS_HOST_BUSY
> (1 << 0)++/* For SMBXMITADD register. */+#define XMIT_WRITE(dev)
> (((dev) << 1) | 0)+#define XMIT_READ(dev)   (((dev) << 1) |
> 1)++STATIC+UINT16+EFIAPI+SmbusGetSMBaseAddress (+  IN VOID+  )+{+
> UINT16  Cmd;+  UINT32  Reg32;+  UINT16  IoBase;++  IoBase = 0;+  //+  // Test
> if I/O decoding is enabled+  //+  Cmd = PciRead16 (PciDevice + 0x4);+  if
> (!(Cmd & 1)) {+goto CloseAndReturn;+  }++  //+  // Test if BAR0 is I/O bar
> and enabled+  //+  Reg32 = PciRead16 (PciDevice + 0x20);+  if (!(Reg32 & 1)
> || !(Reg32 & 0xfffc) || ((Reg32 & 0xfffc) == 0xfffc)) {+goto
> CloseAndReturn;+  }++  IoBase = Reg32 & 0xfffc;++CloseAndReturn:+  return
> IoBase;+}++STATIC+EFI_STATUS+SmbusSetupCommand (+  IN UINT16
> IoBase,+  IN UINT8   Ctrl,+  IN UINT8   Xmitadd+  )+{+  UINTN  Loops = 1;+
> UINT8  host_busy;++  do {+MicroSecondDelay (100);+host_busy =
> IoRead8 (IoBase + SMBHSTSTAT) & SMBHSTSTS_HOST_BUSY;+  } while (--
> Loops && host_busy);++  if (Loops == 0) {+return EFI_TIMEOUT;+  }++  /*
> Clear any lingering errors, so the transaction will run. */+  IoWrite8 
> (IoBase +
> SMBHSTSTAT, IoRead8 (IoBase + SMBHSTSTAT));++  /* Set up transaction */+
> /* Disable interrupts */+  IoWrite8 (IoBase + SMBHSTCTL, Ctrl);++  /* Set the
> device I'm 

[edk2-devel] [PATCH] UefiPayloadPkg: Add RNG support

[Sean Rhodes]

From: Patrick Rudolph 

Uses the RDRAND instruction if available and install EfiRngProtocol.
The protocol may be used by iPXE or the Linux kernel to gather entropy.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Guo Dong 
Cc: Ray Ni 
Cc: Maurice Ma 
Cc: Benjamin You 
Signed-off-by: Patrick Rudolph 
---
 SecurityPkg/Library/BaseRngLib/BaseRng.c  | 199 ++
 SecurityPkg/Library/BaseRngLib/BaseRngLib.inf |  32 +++
 SecurityPkg/Library/BaseRngLib/BaseRngLib.uni |  17 ++
 UefiPayloadPkg/UefiPayloadPkg.dsc |   8 +
 UefiPayloadPkg/UefiPayloadPkg.fdf |   4 +
 5 files changed, 260 insertions(+)
 create mode 100644 SecurityPkg/Library/BaseRngLib/BaseRng.c
 create mode 100644 SecurityPkg/Library/BaseRngLib/BaseRngLib.inf
 create mode 100644 SecurityPkg/Library/BaseRngLib/BaseRngLib.uni

diff --git a/SecurityPkg/Library/BaseRngLib/BaseRng.c 
b/SecurityPkg/Library/BaseRngLib/BaseRng.c
new file mode 100644
index 00..c21e713cb0
--- /dev/null
+++ b/SecurityPkg/Library/BaseRngLib/BaseRng.c
@@ -0,0 +1,199 @@
+/** @file
+  Random number generator services that uses RdRand instruction access
+  to provide high-quality random numbers.
+
+Copyright (c) 2015, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+
+STATIC BOOLEAN  mHasRdRand;
+
+//
+// Bit mask used to determine if RdRand instruction is supported.
+//
+#define RDRAND_MASK  BIT30
+
+//
+// Limited retry number when valid random data is returned.
+// Uses the recommended value defined in Section 7.3.17 of "Intel 64 and IA-32
+// Architectures Software Developer's Mannual".
+//
+#define RDRAND_RETRY_LIMIT  10
+
+/**
+  The constructor function checks whether or not RDRAND instruction is 
supported
+  by the host hardware.
+
+  The constructor function checks whether or not RDRAND instruction is 
supported.
+  It will always return RETURN_SUCCESS.
+
+  @retval RETURN_SUCCESS   The constructor always returns EFI_SUCCESS.
+
+**/
+RETURN_STATUS
+EFIAPI
+BaseRngLibConstructor (
+  VOID
+  )
+{
+  UINT32  RegEax;
+  UINT32  RegEcx;
+
+  AsmCpuid (CPUID_SIGNATURE, , NULL, NULL, NULL);
+  if (RegEax < 1) {
+mHasRdRand = FALSE;
+return RETURN_SUCCESS;
+  }
+
+  //
+  // Determine RDRAND support by examining bit 30 of the ECX register returned 
by
+  // CPUID. A value of 1 indicates that processor support RDRAND instruction.
+  //
+  AsmCpuid (CPUID_VERSION_INFO, 0, 0, , 0);
+
+  mHasRdRand = ((RegEcx & RDRAND_MASK) == RDRAND_MASK);
+
+  return RETURN_SUCCESS;
+}
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber16 (
+  OUT UINT16  *Rand
+  )
+{
+  UINT32  Index;
+
+  ASSERT (Rand != NULL);
+
+  if (mHasRdRand) {
+//
+// A loop to fetch a 16 bit random value with a retry count limit.
+//
+for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
+  if (AsmRdRand16 (Rand)) {
+return TRUE;
+  }
+}
+  }
+
+  return FALSE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber32 (
+  OUT UINT32  *Rand
+  )
+{
+  UINT32  Index;
+
+  ASSERT (Rand != NULL);
+
+  if (mHasRdRand) {
+//
+// A loop to fetch a 32 bit random value with a retry count limit.
+//
+for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
+  if (AsmRdRand32 (Rand)) {
+return TRUE;
+  }
+}
+  }
+
+  return FALSE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber64 (
+  OUT UINT64  *Rand
+  )
+{
+  UINT32  Index;
+
+  ASSERT (Rand != NULL);
+
+  if (mHasRdRand) {
+//
+// A loop to fetch a 64 bit random value with a retry count limit.
+//
+for (Index = 0; Index < RDRAND_RETRY_LIMIT; Index++) {
+  if (AsmRdRand64 (Rand)) {
+return TRUE;
+  }
+}
+  }
+
+  return FALSE;
+}
+
+/**
+  Generates a 128-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 128-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber128 (
+  OUT UINT64  *Rand
+  )
+{
+  ASSERT (Rand != NULL);
+
+  //
+  // Read 

[edk2-devel] [PATCH 1/2] UefiPayloadPkg: Add i801 SMBus controller DXE

[Sean Rhodes]

From: Patrick Rudolph 

Implement a subset of the gEfiSmbusHcProtocolGuid using a generic
PCI i801 SMBus controller.

Cc: Guo Dong 
Cc: Ray Ni 
Cc: Maurice Ma 
Cc: Benjamin You 
Signed-off-by: Patrick Rudolph 
---
 .../Library/BrotliCustomDecompressLib/brotli  |   2 +-
 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c| 556 ++
 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.h|  17 +
 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.inf  |  45 ++
 UefiPayloadPkg/UefiPayloadPkg.dsc |   7 +
 UefiPayloadPkg/UefiPayloadPkg.fdf |   5 +
 6 files changed, 631 insertions(+), 1 deletion(-)
 create mode 100644 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
 create mode 100644 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.h
 create mode 100644 UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.inf

diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli 
b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
index f4153a09f8..666c3280cc 16
--- a/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
+++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
@@ -1 +1 @@
-Subproject commit f4153a09f87cbb9c826d8fc12c74642bb2d879ea
+Subproject commit 666c3280cc11dc433c303d79a83d4ffbdd12cc8d
diff --git a/UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c 
b/UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
new file mode 100644
index 00..7bf7b893ad
--- /dev/null
+++ b/UefiPayloadPkg/SmbusDxe/SMBusi801Dxe.c
@@ -0,0 +1,556 @@
+/** @file
+  Implementation for a generic i801 SMBus driver.
+
+Copyright (c) 2016, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+
+**/
+
+#include "SMBusi801Dxe.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+EFI_HANDLE  mDriverHandle = NULL;
+UINT32  PciDevice = 0;
+
+/* SMBus register offsets. */
+#define SMBHSTSTAT  0x0
+#define SMBHSTCTL   0x2
+#define SMBHSTCMD   0x3
+#define SMBXMITADD  0x4
+#define SMBHSTDAT0  0x5
+#define SMBHSTDAT1  0x6
+#define SMBBLKDAT   0x7
+#define SMBTRNSADD  0x9
+#define SMBSLVDATA  0xa
+#define SMLINK_PIN_CTL  0xe
+#define SMBUS_PIN_CTL   0xf
+#define SMBSLVCMD   0x11
+
+/* I801 command constants */
+#define I801_QUICK   (0 << 2)
+#define I801_BYTE(1 << 2)
+#define I801_BYTE_DATA   (2 << 2)
+#define I801_WORD_DATA   (3 << 2)
+#define I801_PROCESS_CALL(4 << 2)
+#define I801_BLOCK_DATA  (5 << 2)
+#define I801_I2C_BLOCK_DATA  (6 << 2) /* ICH5 and later */
+
+/* I801 Host Control register bits */
+#define SMBHSTCNT_INTREN (1 << 0)
+#define SMBHSTCNT_KILL   (1 << 1)
+#define SMBHSTCNT_LAST_BYTE  (1 << 5)
+#define SMBHSTCNT_START  (1 << 6)
+#define SMBHSTCNT_PEC_EN (1 << 7)/* ICH3 and later */
+
+/* I801 Hosts Status register bits */
+#define SMBHSTSTS_BYTE_DONE (1 << 7)
+#define SMBHSTSTS_INUSE_STS (1 << 6)
+#define SMBHSTSTS_SMBALERT_STS  (1 << 5)
+#define SMBHSTSTS_FAILED(1 << 4)
+#define SMBHSTSTS_BUS_ERR   (1 << 3)
+#define SMBHSTSTS_DEV_ERR   (1 << 2)
+#define SMBHSTSTS_INTR  (1 << 1)
+#define SMBHSTSTS_HOST_BUSY (1 << 0)
+
+/* For SMBXMITADD register. */
+#define XMIT_WRITE(dev)  (((dev) << 1) | 0)
+#define XMIT_READ(dev)   (((dev) << 1) | 1)
+
+STATIC
+UINT16
+EFIAPI
+SmbusGetSMBaseAddress (
+  IN VOID
+  )
+{
+  UINT16  Cmd;
+  UINT32  Reg32;
+  UINT16  IoBase;
+
+  IoBase = 0;
+  //
+  // Test if I/O decoding is enabled
+  //
+  Cmd = PciRead16 (PciDevice + 0x4);
+  if (!(Cmd & 1)) {
+goto CloseAndReturn;
+  }
+
+  //
+  // Test if BAR0 is I/O bar and enabled
+  //
+  Reg32 = PciRead16 (PciDevice + 0x20);
+  if (!(Reg32 & 1) || !(Reg32 & 0xfffc) || ((Reg32 & 0xfffc) == 0xfffc)) {
+goto CloseAndReturn;
+  }
+
+  IoBase = Reg32 & 0xfffc;
+
+CloseAndReturn:
+  return IoBase;
+}
+
+STATIC
+EFI_STATUS
+SmbusSetupCommand (
+  IN UINT16  IoBase,
+  IN UINT8   Ctrl,
+  IN UINT8   Xmitadd
+  )
+{
+  UINTN  Loops = 1;
+  UINT8  host_busy;
+
+  do {
+MicroSecondDelay (100);
+host_busy = IoRead8 (IoBase + SMBHSTSTAT) & SMBHSTSTS_HOST_BUSY;
+  } while (--Loops && host_busy);
+
+  if (Loops == 0) {
+return EFI_TIMEOUT;
+  }
+
+  /* Clear any lingering errors, so the transaction will run. */
+  IoWrite8 (IoBase + SMBHSTSTAT, IoRead8 (IoBase + SMBHSTSTAT));
+
+  /* Set up transaction */
+  /* Disable interrupts */
+  IoWrite8 (IoBase + SMBHSTCTL, Ctrl);
+
+  /* Set the device I'm talking to. */
+  IoWrite8 (IoBase + SMBXMITADD, Xmitadd);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+SmbusExecuteCommand (
+  IN UINT16  IoBase
+  )
+{
+  UINTN  Loops = 1;
+  UINT8  Status;
+
+  /* Start the command. */
+  IoWrite8 (IoBase + SMBHSTCTL, IoRead8 (IoBase + SMBHSTCTL) | 
SMBHSTCNT_START);
+  Status = IoRead8 (IoBase + SMBHSTSTAT);
+
+  /* Poll for it to start. */
+  do {
+MicroSecondDelay (100);
+
+/* If we poll too slow, we could miss HOST_BUSY flag
+ * set and detect INTR or x_ERR flags instead here.
+ */
+Status = IoRead8 

[edk2-devel] [PATCH 2/2] UefipayloadPkg: Add SmbusConfigLoaderDxe

[Sean Rhodes]

From: Patrick Rudolph 

Reads board specific configuration values from an EEPROM that is written
to by the BMC.

Cc: Guo Dong 
Cc: Ray Ni 
Cc: Maurice Ma 
Cc: Benjamin You 
Signed-off-by: Patrick Rudolph 
---
 .../Include/Guid/BoardSettingsGuid.h  |  36 +++
 .../SmbusConfigLoaderDxe/SMBusConfigLoader.c  | 269 ++
 .../SmbusConfigLoaderDxe/SMBusConfigLoader.h  |  23 ++
 .../SMBusConfigLoader.inf |  54 
 UefiPayloadPkg/UefiPayloadPkg.dec |   1 +
 UefiPayloadPkg/UefiPayloadPkg.dsc |   1 +
 UefiPayloadPkg/UefiPayloadPkg.fdf |  11 +-
 7 files changed, 390 insertions(+), 5 deletions(-)
 create mode 100644 UefiPayloadPkg/Include/Guid/BoardSettingsGuid.h
 create mode 100644 UefiPayloadPkg/SmbusConfigLoaderDxe/SMBusConfigLoader.c
 create mode 100644 UefiPayloadPkg/SmbusConfigLoaderDxe/SMBusConfigLoader.h
 create mode 100644 UefiPayloadPkg/SmbusConfigLoaderDxe/SMBusConfigLoader.inf

diff --git a/UefiPayloadPkg/Include/Guid/BoardSettingsGuid.h 
b/UefiPayloadPkg/Include/Guid/BoardSettingsGuid.h
new file mode 100644
index 00..415b49dcde
--- /dev/null
+++ b/UefiPayloadPkg/Include/Guid/BoardSettingsGuid.h
@@ -0,0 +1,36 @@
+/** @file
+  This file defines the hob structure for board settings
+
+  Copyright (c) 2020, 9elements GmbH. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __BOARD_SETTINGS_GUID_H__
+#define __BOARD_SETTINGS_GUID_H__
+
+///
+/// Board information GUID
+///
+extern EFI_GUID gEfiBoardSettingsVariableGuid;
+
+#pragma pack(1)
+
+typedef struct {
+  UINT32 Signature;
+  UINT8 SecureBoot;
+  UINT8 PrimaryVideo;
+  UINT8 DeepSx;
+  UINT8 WakeOnUSB;
+  UINT8 USBPowerinS5;
+  UINT8 PowerStateAfterG3;
+  UINT8 BlueRearPort;
+  UINT8 InternalAudioConnection;
+  UINT8 PxeBootCapability;
+} BOARD_SETTINGS;
+
+#pragma pack()
+
+#define BOARD_SETTINGS_NAME L"BoardSettings"
+
+#endif // __BOARD_SETTINGS_GUID_H__
diff --git a/UefiPayloadPkg/SmbusConfigLoaderDxe/SMBusConfigLoader.c 
b/UefiPayloadPkg/SmbusConfigLoaderDxe/SMBusConfigLoader.c
new file mode 100644
index 00..c2edbaa74f
--- /dev/null
+++ b/UefiPayloadPkg/SmbusConfigLoaderDxe/SMBusConfigLoader.c
@@ -0,0 +1,269 @@
+/** @file
+  Implementation for a generic i801 SMBus driver.
+
+Copyright (c) 2016, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+
+**/
+
+#include "SMBusConfigLoader.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  GetPciConfigSpaceAddress
+
+  Return the PCI Config Space Address if found, zero otherwise.
+
+  @retval 0 Can not find any SMBusController
+  @retval UINT32PCI Config Space Address
+**/
+STATIC UINT32
+EFIAPI
+GetPciConfigSpaceAddress (
+  )
+{
+  UINT8  Device;
+  UINT8  Function;
+  UINT8  BaseClass;
+  UINT8  SubClass;
+
+  //
+  // Search for SMBus Controller within PCI Devices on root bus
+  //
+  for (Device = 0; Device <= PCI_MAX_DEVICE; Device++) {
+for (Function = 0; Function <= PCI_MAX_FUNC; Function++) {
+  if (PciRead16 (PCI_LIB_ADDRESS (0, Device, Function, 0x00)) != 0x8086) {
+continue;
+  }
+
+  BaseClass = PciRead8 (PCI_LIB_ADDRESS (0, Device, Function, 0x0B));
+
+  if (BaseClass == PCI_CLASS_SERIAL) {
+SubClass = PciRead8 (PCI_LIB_ADDRESS (0, Device, Function, 0xA));
+
+if (SubClass == PCI_CLASS_SERIAL_SMB) {
+  return PCI_LIB_ADDRESS (0, Device, Function, 0x00);
+}
+  }
+}
+  }
+
+  return 0;
+}
+
+/**
+  ReadBoardOptionFromEEPROM
+
+  Reads the Board options like Primary Video from the EEPROM
+
+  @param Buffer Pointer to the Buffer Array
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ReadBoardOptionFromEEPROM (
+  IN OUT UINT8   *Buffer,
+  IN UINT32  Size
+  )
+{
+  EFI_STATUS  Status;
+  UINT16  Index;
+  UINT16  Value;
+
+  for (Index = BOARD_SETTINGS_OFFSET; Index < BOARD_SETTINGS_OFFSET + Size; 
Index += 2) {
+Value = SmBusProcessCall (SMBUS_LIB_ADDRESS (0x57, 0, 0, 0), ((Index & 
0xff) << 8) | ((Index & 0xff00) >> 8), );
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "Failed to read SMBUS byte at offset 0x%x\n", 
Index));
+  return Status;
+}
+
+DEBUG ((EFI_D_ERROR, "Read %x\n", Value));
+CopyMem ([Index-BOARD_SETTINGS_OFFSET], , sizeof (Value));
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+UINT32
+EFIAPI
+crc32_byte (
+  UINT32  prev_crc,
+  UINT8   data
+  )
+{
+  prev_crc ^= (UINT32)data << 24;
+
+  for (int i = 0; i < 8; i++) {
+if ((prev_crc & 0x8000UL) != 0) {
+  prev_crc = ((prev_crc << 1) ^ 0x04C11DB7UL);
+} else {
+  prev_crc <<= 1;
+}
+  }
+
+  return prev_crc;
+}
+
+/**
+  Computes and returns a 32-bit CRC for a data buffer.
+  CRC32 value bases on ITU-T V.42.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param[in]  Buffer   A pointer to the buffer on 

[edk2-devel] Cancelled Event: TianoCore Design Meeting - APAC/NAMO - Friday, February 18, 2022 #cal-cancelled

[devel@edk2.groups.io Calendar]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
REFRESH-INTERVAL;VALUE=DURATION:PT1H
X-PUBLISHED-TTL:PT1H
CALSCALE:GREGORIAN
BEGIN:VTIMEZONE
TZID:Asia/Shanghai
LAST-MODIFIED:20201011T015911Z
TZURL:http://tzurl.org/zoneinfo-outlook/Asia/Shanghai
X-LIC-LOCATION:Asia/Shanghai
BEGIN:STANDARD
TZNAME:CST
TZOFFSETFROM:+0800
TZOFFSETTO:+0800
DTSTART:19700101T00
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
X-GIOIDS:Event:1238710 
UID:ccyo.1615368347866508187.a...@groups.io
DTSTAMP:20220217T153953Z
ORGANIZER;CN=Ray Ni:mailto:ray...@intel.com
DTSTART:20220218T013000Z
DTEND:20220218T023000Z
SUMMARY:TianoCore Design Meeting - APAC/NAMO
DESCRIPTION:## TOPIC\n\n1. NA\n\nFor more info\, see here: https://www.ti
 anocore.org/design-meeting/\n\n---\n## Microsoft Teams meeting\n\n### Joi
 n on your computer or mobile app\n\n[Click here to join the meeting](http
 s://teams.microsoft.com/l/meetup-join/19%3ameeting_OTNmZTNhMWEtOWQwNi00ZT
 dkLWI5NDgtYTFmYjNkOWI0ZDg4%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d
 88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%2255d36a50-78be-4ced-bc2
 7-3d06c576cc19%22%7d)\n\n### Join with a video conferencing device\n\ntea
 m...@conf.intel.com\n\nVideo Conference ID: 119 715 416 0\n\n[Alternate VTC
  dialing instructions](https://conf.intel.com/teams/?conf=1197154160=
 teams=conf.intel.com=test_call)\n\n[Learn More](https://aka.ms/Joi
 nTeamsMeeting) | [Meeting options](https://teams.microsoft.com/meetingOpt
 ions/?organizerId=55d36a50-78be-4ced-bc27-3d06c576cc19=46c98d88-
 e344-4ed4-8496-4ed7712e255d=19_meeting_OTNmZTNhMWEtOWQwNi00ZTdkL
 WI5NDgtYTFmYjNkOWI0ZDg4@thread.v2=0=en-US)
LOCATION:Microsoft Teams
SEQUENCE:1
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU Service with rendezvous support.

[Li, Zhihao]

Hi, Michael
With your comment:
1. Decide to define a new Protocol with just the new 
services(gEdkiiSmmCpuRedezvousProtocolGuid)
2. Modified it to 0x00.
3. keep v1
4. keep v1
Other modification:
1. SmmCpuRendezvousLib.inf: add MM_STANDALONE support
2. SmmCpuRendezvousLib.c: remove *constructor function, move its
action into  SmmWaitForAllProcessor function.

> -Original Message-
> From: Fu, Siyuan 
> Sent: Thursday, February 10, 2022 4:19 PM
> To: devel@edk2.groups.io; Kinney, Michael D ;
> Li, Zhihao ; Ni, Ray 
> Cc: Dong, Eric ; Kumar, Rahul1
> 
> Subject: RE: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU
> Service with rendezvous support.
> 
> Hi, Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> Michael
> > D Kinney
> > Sent: 2022年2月9日 0:31
> > To: devel@edk2.groups.io; Li, Zhihao ; Kinney,
> > Michael D 
> > Cc: Dong, Eric ; Ni, Ray ;
> > Kumar,
> > Rahul1 
> > Subject: Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU
> > Service with rendezvous support.
> >
> > Hi Zhihao,
> >
> > gEfiSmmCpuServiceProtocolGuid is defined in the UefiCpuPkg and is
> > already an EDK II specific feature protocol.  Adding an Edkii names
> > version of the protocol does not make it clear that there is a
> > relationship between the two versions of this protocol.  You have
> > added one new service to the existing protocol.  The existing protocol
> > does not have a Revision field so we do have to create a new Protocol
> > Name/Protocol GUID.  Based on previous use cases, we have a few options:
> >
> > 1) If Revision field is present, add to end and increase Revision
> > value
> > 2) If Revision field not present
> >   a) Define an _2 or _Ex version of the protocol with new service(s) added
> >  to end of structure and implement original version of the protocol on
> >  top of the _2 version of the protocol.
> >   b) Define a new Protocol with just the new services. (e.g.
> > gEdkiiSmmCpuRedezvousProtocolGuid)
> We previously discussed with Ray when deciding the protocol name and
> choose the edk2 prefix.
> @Ni, Ray
> Any opinion on using an _Ex version protocol name or a separate protocol?
Decide to define a new Protocol with just the new 
services(gEdkiiSmmCpuRedezvousProtocolGuid)
> 
> >
> > The patch also changes the DEC default value of
> > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode
> > from 0x00 to 0x01.  Changing the default value of a PCD in a DEC file
> > is a non backwards compatible change.  This should not be done.
> > Instead, platforms that need the different sync mode should set that
> > PCD in their DSC file.
Modified it to 0x00.
> >
> > Is a new lib class really required at this time.  The reason to add a
> > new lib class is if there are multiple consumers.
> There are lots of consumers but no in edk2 repo, mostly inside platform code
> like edk2platforms.
> Technically the SMI handler which require all processors in SMM mode to
> complete its task (either due to security consideration or hardware/silicon
> restriction) will need to consume this library interface to complete the
> rendezvous in relax AP mode.
> 
> >
> > I see the lib instance uses a RegisterProtocolNotify in its
> > constructor.  Is it possible to use a Depex instead and eliminate the
> > additional complexity of a constructor and RegisterProtocolNotify?
> We can't use Depex since this is an optional protocol. It's not required to
> those platforms which only have traditional sync mode support.
> 
> Thanks,
> Siyuan
> 
> >
> > Best regards,
> >
> > Mike
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Li,
> > > Zhihao
> > > Sent: Monday, February 7, 2022 9:36 PM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric ; Ni, Ray ;
> > > Kumar,
> > Rahul1 
> > > Subject: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU
> > > Service
> > with rendezvous support.
> > >
> > > From: Zhihao Li 
> > >
> > > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3815
> > >
> > > This patch extends the SMM CPU Service protocol with new interface
> > > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > > optionally wait for other APs to complete SMM rendezvous in relaxed AP
> mode.
> > >
> > > A new library SmmCpuRendezvousLib is provided to abstract the
> > > service into library API to simple SMI handler code.
> > >
> > > Cc: Eric Dong 
> > > Cc: Ray Ni 
> > > Cc: Rahul Kumar 
> > >
> > > Signed-off-by: Zhihao Li 
> > > ---
> > >  UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c
> | 109
> > 
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c |  65
> > 
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  14 ++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/SyncTimer.c  |   2 +-
> > >  UefiCpuPkg/Include/Library/SmmCpuRendezvousLib.h   |  27
> +
> > >  UefiCpuPkg/Include/Protocol/SmmCpuService.h|  40 
> 

Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU Service with rendezvous support.

[Li, Zhihao]

Hi Marvin
With your comments:
1. According to granular of prototype (EFI_STATUS).
2. will delete assert and return status
3. Will delete assert and return first error(if so).
Other modification:
1. SmmCpuRendezvousLib.inf: add MM_STANDALONE support
2. SmmCpuRendezvousLib.c: remove *constructor function, move its
action into  SmmWaitForAllProcessor function.


> -Original Message-
> From: Marvin Häuser 
> Sent: Friday, February 11, 2022 6:30 PM
> To: devel@edk2.groups.io; Li, Zhihao 
> Cc: Dong, Eric ; Ni, Ray ; Kumar,
> Rahul1 
> Subject: Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU
> Service with rendezvous support.
> 
> Good day,
> 
> On 08.02.22 06:35, Li, Zhihao wrote:
> > From: Zhihao Li 
> >
> > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3815
> >
> > This patch extends the SMM CPU Service protocol with new interface
> > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > optionally wait for other APs to complete SMM rendezvous in relaxed AP
> mode.
> >
> > A new library SmmCpuRendezvousLib is provided to abstract the service
> > into library API to simple SMI handler code.
> >
> > Cc: Eric Dong 
> > Cc: Ray Ni 
> > Cc: Rahul Kumar 
> >
> > Signed-off-by: Zhihao Li 
> > ---
> >   UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c   |
> 109 
> >   UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c |  65
> 
> >   UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  14 ++-
> >   UefiCpuPkg/PiSmmCpuDxeSmm/SyncTimer.c  |   2 +-
> >   UefiCpuPkg/Include/Library/SmmCpuRendezvousLib.h   |  27 +
> >   UefiCpuPkg/Include/Protocol/SmmCpuService.h|  40 
> > +++
> >   UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf
> |  32 ++
> >   UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  28
> +
> >   UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   3 +-
> >   UefiCpuPkg/UefiCpuPkg.dec  |   5 +-
> >   10 files changed, 318 insertions(+), 7 deletions(-)
> >
> > diff --git
> > a/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c
> > b/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c
> > new file mode 100644
> > index 00..3c5cd51d0c
> > --- /dev/null
> > +++
> b/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c
> > @@ -0,0 +1,109 @@
> > +/** @file
> >
> > +SMM CPU Rendezvous library header file.
> >
> > +
> >
> > +Copyright (c) 2021, Intel Corporation. All rights reserved.
> >
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > +
> >
> > +**/
> >
> > +
> >
> > +
> >
> > +#include 
> >
> > +#include 
> >
> > +#include 
> >
> > +#include 
> >
> > +#include 
> >
> > +#include 
> >
> > +#include 
> >
> > +#include 
> >
> > +
> >
> > +STATIC EDKII_SMM_CPU_SERVICE_PROTOCOL  *mSmmCpuService =
> NULL;
> >
> > +
> >
> > +/**
> >
> > +  This routine wait for all AP processors to arrive in SMM.
> >
> > +
> >
> > +  @param  BlockingMode  Blocking mode or non-blocking mode.
> >
> > +
> >
> > +  @retval TRUE   All processors checked in to SMM
> >
> > +  @retval FALSE  Some processor not checked in to SMM
> >
> > +
> >
> > +**/
> >
> > +EFI_STATUS
> >
> > +EFIAPI
> >
> > +SmmWaitForAllProcessor (
> >
> > +  IN  BOOLEAN  BlockingMode
> >
> > +  )
> >
> > +{
> >
> > +  EFI_STATUS Status;
> >
> > +
> >
> > +  if (mSmmCpuService == NULL) {
> >
> > +return TRUE;
> >
> > +  }
> >
> > +
> >
> > +  Status = mSmmCpuService->WaitForAllProcessor (
> >
> > +  mSmmCpuService,
> >
> > +  BlockingMode
> >
> > +  );
> >
> > +  return EFI_ERROR(Status) ? FALSE : TRUE;
> 
> Hmm, if there is a granular error code, why make it less granular by
> conversion? Also the prototype says EFI_STATUS, and the docs say BOOLEAN.
According to granular of prototype (EFI_STATUS).
> 
> >
> > +}
> >
> > +
> >
> > +/**
> >
> > +  Register status code callback function only when Report Status Code
> > + protocol
> >
> > +  is installed.
> >
> > +
> >
> > +  @param Protocol   Points to the protocol's unique identifier.
> >
> > +  @param Interface  Points to the interface instance.
> >
> > +  @param Handle The handle on which the interface was installed.
> >
> > +
> >
> > +  @retval EFI_SUCCESS   Notification runs successfully.
> >
> > +
> >
> > +**/
> >
> > +EFI_STATUS
> >
> > +EFIAPI
> >
> > +SmmCpuServiceProtocolNotify (
> >
> > +  IN CONST EFI_GUID*Protocol,
> >
> > +  IN VOID  *Interface,
> >
> > +  IN EFI_HANDLEHandle
> >
> > +  )
> >
> > +{
> >
> > +  EFI_STATUS   Status;
> >
> > +
> >
> > +  Status = gSmst->SmmLocateProtocol (
> >
> > +,
> >
> > +NULL,
> >
> > +(VOID **) 
> >
> > +);
> >
> > +  ASSERT_EFI_ERROR (Status);
> >
> > +
> >
> > +  return EFI_SUCCESS;
> >
> > +}
> >

[edk2-devel] [PATCH v2 1/1] CryptoPkg: Add new hash algorithm ParallelHash256HashAll in BaseCryptLib.

[Li, Zhihao]

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

Parallel hash function ParallelHash256HashAll, as defined in NIST's
Special Publication 800-185, published December 2016. It utilizes
multi-process to calculate the digest.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Xiaoyu Lu 
Cc: Guomin Jiang 
Cc: Siyuan Fu 

Signed-off-by: Zhihao Li 
---
 CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c   | 317 

 CryptoPkg/Library/BaseCryptLib/Hash/CryptParallelHash.c| 273 
+
 CryptoPkg/Library/BaseCryptLib/Hash/CryptSha3.c| 102 
+++
 CryptoPkg/Library/BaseCryptLib/Hash/CryptXkcp.c|  53 

 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/ParallelhashTests.c   | 145 
+
 CryptoPkg/Include/Library/BaseCryptLib.h   |  31 +-
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf |   8 +-
 CryptoPkg/Library/Include/CrtLibSupport.h  |  16 +-
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf  |   2 +
 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf |   2 +
 10 files changed, 946 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c 
b/CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c
new file mode 100644
index ..60d89ecfe43b
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c
@@ -0,0 +1,317 @@
+/** @file

+  cSHAKE-256 Digest Wrapper Implementations.

+

+Copyright (c) 2022, Intel Corporation. All rights reserved.

+SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#include "InternalCryptLib.h"

+

+#define  CSHAKE256_SECURITY_STRENGTH256

+#define  CSHAKE256_RATE_IN_BYTES136

+

+const CHAR8 mZeroPadding[CSHAKE256_RATE_IN_BYTES] = {0};

+

+unsigned int left_encode(unsigned char * encbuf, size_t value);

+unsigned int right_encode(unsigned char * encbuf, size_t value);

+int init (struct KECCAK1600_CTX *ctx, unsigned char pad, size_t bsz, size_t 
md_size);

+int sha3_update (struct KECCAK1600_CTX *ctx, const void *_inp, size_t len);

+int sha3_final (struct KECCAK1600_CTX *ctx, unsigned char *md);

+

+UINTN

+EFIAPI

+LeftEncode (

+  OUT UINT8 *Encbuf,

+  IN  UINTN Value

+  )

+{

+  return left_encode (Encbuf, Value);

+}

+

+UINTN

+EFIAPI

+RightEncode (

+  OUT UINT8 *Encbuf,

+  IN  UINTN Value

+  )

+{

+  return right_encode (Encbuf, Value);

+}

+

+/**

+  Retrieves the size, in bytes, of the context buffer required for cSHAKE-256 
hash operations.

+

+  @return  The size, in bytes, of the context buffer required for cSHAKE-256 
hash operations.

+

+**/

+UINTN

+EFIAPI

+CShake256GetContextSize (

+  VOID

+  )

+{

+  return (UINTN) (sizeof (struct KECCAK1600_CTX));

+}

+

+/**

+  Initializes user-supplied memory pointed by CShake256Context as cSHAKE-256 
hash context for

+  subsequent use.

+

+  @param[out] CShake256Context   Pointer to cSHAKE-256 context being 
initialized.

+  @param[in]  OutputLen  The desired number of output length in bytes.

+  @param[in]  Name   Pointer to the function name string.

+  @param[in]  NameLenThe length of the function name in bytes.

+  @param[in]  Customization  Pointer to the customization string.

+  @param[in]  CustomizationLen   The length of the customization string in 
bytes.

+

+  @retval TRUE   cSHAKE-256 context initialization succeeded.

+  @retval FALSE  cSHAKE-256 context initialization failed.

+  @retval FALSE  This interface is not supported.

+

+**/

+BOOLEAN

+EFIAPI

+CShake256Init (

+  OUT   VOID  *CShake256Context,

+  INUINTN OutputLen,

+  INCONST VOID*Name,

+  INUINTN NameLen,

+  INCONST VOID*Customization,

+  INUINTN CustomizationLen

+  )

+{

+  BOOLEAN   Status;

+  unsigned char EncBuf[sizeof(size_t)+1];

+  UINTN EncLen;

+  UINTN AbsorbLen;

+  UINTN PadLen;

+

+  //

+  // Check input parameters.

+  //

+  if (CShake256Context == NULL ||

+  OutputLen == 0 ||

+  (NameLen != 0 && Name == NULL) ||

+  (CustomizationLen != 0 && Customization == NULL)) {

+return FALSE;

+  }

+

+  //

+  // Initialize KECCAK context with pad value and block size.

+  //

+  if (NameLen == 0 && CustomizationLen == 0) {

+//

+// When N and S are both empty strings, cSHAKE(X, L, N, S) is equivalent to

+// SHAKE as defined in FIPS 202.

+//

+return (BOOLEAN) init (

+  (struct KECCAK1600_CTX *) CShake256Context,

+  '\x1f',

+  (KECCAK1600_WIDTH - CSHAKE256_SECURITY_STRENGTH * 2) / 8,

+  OutputLen

+  );

+  }

+

+  Status = (BOOLEAN) init (

+  (struct KECCAK1600_CTX *) CShake256Context,

+  '\x04',

Re: [edk2-devel] [PATCH V5 25/33] OvmfPkg: Update PlatformPei to support Tdx guest

[Min Xu]

Hi
 
> > +/**
> > +  This Function checks if TDX is available, if present then it sets
> > +  the dynamic PCDs for Tdx guest. It also builds Guid hob which
> > +contains
> > +  the Host Bridge DevId.
> > +  **/
> > +VOID
> > +IntelTdxInitialize (
> > +  VOID
> > +  )
> > +{
> > + #ifdef MDE_CPU_X64
> > +  EFI_HOB_PLATFORM_INFO  PlatformInfoHob;
> > +  RETURN_STATUS  PcdStatus;
> > +
> > +  if (!TdIsEnabled ()) {
> > +return;
> > +  }
> > +
> > +  PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr,
> > + CCAttrIntelTdx);  ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  PcdStatus = PcdSetBoolS (PcdIa32EferChangeAllowed, FALSE);
> > + ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  PcdStatus = PcdSet64S (PcdTdxSharedBitMask, TdSharedPageMask ());
> > + ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  PcdStatus = PcdSetBoolS (PcdSetNxForStack, TRUE);
> > + ASSERT_RETURN_ERROR (PcdStatus);
> > +
> > +  ZeroMem (, sizeof (PlatformInfoHob));
> > + PlatformInfoHob.HostBridgePciDevId = mHostBridgeDevId;
> > +
> > +  BuildGuidDataHob (, ,
> > +sizeof (EFI_HOB_PLATFORM_INFO));  #endif }
> 
> So, what is the plan for this with pei-less boot?
In Pei-less boot PCDs cannot be set. So these settings are saved in 
PlatformInfoHob which will be set in early Dxe phase.
> 
> I think we should move this to PlatformInitLib, then link either into
> PlatformPei or the early dxe module for pei-less boot?
> 
1. PlatformInitLib is designed without Dynamic PCDs because it is for both SEC 
and PEI.

2. When boot with PEI phase, some PCDs are mandatory.
For example, PcdIa32EferChangeAllowed indicates whether IA32_EFER can be 
modified or not. Because in Tdx guest change of IA32_EFER is not allowed. But 
in boot with PEI phase, DxeIplPei tries to update IA32_EFER (see 
IsEnableNonExecNeeded).
Another example is PcdConfidentialComputingGuestAttr. It is used in 
MpInitLib/MpLib.c (for CpuMpPei).

3. In Pei-less boot, there is no such limitation. Because the PCDs can be set 
before they're consumed.

Based on above consideration, I would suggest keep above logics.

Thanks
Min


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