Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Set Action for failed signed image

2021-10-12 Thread Yao, Jiewen
It seems make sense.

Would you please:
1) Fila a Bugzilla - https://bugzilla.tianocore.org/
2) Describe what unit test you have done. Especially, if multiple cert 
list/data have been tested.
I ask this because we use a for-loop to check cert in list one by one.

Thank you
Yao Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Joseph
> Hemann
> Sent: Tuesday, October 12, 2021 9:12 PM
> To: devel@edk2.groups.io
> Cc: n...@arm.com; Joseph Hemann ; Yao, Jiewen
> ; Wang, Jian J ; Xu, Min M
> 
> Subject: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Set Action
> for failed signed image
> 
> From: Joseph Hemann 
> 
> If the image is signed but not allowed by DB and the hash of
> image is not found in DB/DBX, then the EFI_IMAGE_INFO_ACTION
> of the load of said image should be set to,
> EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
> unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Min Xu 
> 
> Signed-off-by: Joseph Hemann 
> Change-Id: I271fb61384e5b8bb5ac23ccba5de9ba77adb85ad
> ---
>  .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index c48861cd64..0a804af216 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1957,6 +1957,7 @@ DxeImageVerificationHandler (
>if (!EFI_ERROR (DbStatus) && IsFound) {
>  IsVerified = TRUE;
>} else {
> +Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
>  DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but
> signature is not allowed by DB and %s hash of image is not found in 
> DB/DBX.\n",
> mHashTypeStr));
>}
>  }
> --
> 2.17.1
> 
> 
> 
> 
> 



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




Re: [edk2-devel] [PATCH V2 13/28] UefiCpuPkg: Enable Tdx support in MpInitLib

2021-10-12 Thread Ni, Ray
Min,
The change is to provide a totally different MP service in TDX case.
It makes the MpInitLib more complicated.

How about?
1. Change CpuMpPei/CpuMpDxe to return directly in TDX case.
2. Add new TdxCpuMpPei/TdxCpuMpDxe to provide a new set simple MP service in 
TDX case.

This makes the whole code easier to understand.

Thanks,
Ray

-Original Message-
From: Xu, Min M  
Sent: Tuesday, October 5, 2021 11:39 AM
To: devel@edk2.groups.io
Cc: Xu, Min M ; Brijesh Singh ; 
Erdem Aktas ; James Bottomley ; Yao, 
Jiewen ; Tom Lendacky ; Dong, 
Eric ; Ni, Ray ; Kumar, Rahul1 

Subject: [PATCH V2 13/28] UefiCpuPkg: Enable Tdx support in MpInitLib

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

In TDVF BSP and APs are simplified. BSP is the vCPU-0, while the others are 
treated as APs.

So MP intialization is rather simple. The processor info is retrieved by 
TDCALL, ApWorker is not supported, BSP is always the working processor, while 
the APs are just in a wait-for-precedure state.

Cc: Brijesh Singh 
Cc: Erdem Aktas 
Cc: James Bottomley 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar 
Signed-off-by: Min Xu 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   4 +
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   |  14 +-
 UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h | 107 ++
 UefiCpuPkg/Library/MpInitLib/MpLib.c  |  26 +++
 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c   | 186 ++
 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c   | 117 +++
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 +
 .../Library/MpInitLib/X64/IntelTdcall.nasm| 120 +++
 8 files changed, 577 insertions(+), 1 deletion(-)  create mode 100644 
UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
 create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdx.c
 create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLibTdxNull.c
 create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/IntelTdcall.nasm

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index d34419c2a524..084e025564ef 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -22,10 +22,13 @@
 #
 
 [Sources.IA32]
+  MpLibTdxNull.c
   Ia32/MpFuncs.nasm
 
 [Sources.X64]
+  MpLibTdx.c
   X64/MpFuncs.nasm
+  X64/IntelTdcall.nasm
 
 [Sources.common]
   MpEqu.inc
@@ -33,6 +36,7 @@
   MpLib.c
   MpLib.h
   Microcode.c
+  MpIntelTdx.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 93fc63bf93e3..b7275db3d564 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -7,6 +7,7 @@
 **/
 
 #include "MpLib.h"
+#include "MpIntelTdx.h"
 
 #include 
 #include 
@@ -15,7 +16,6 @@
 #include 
 #include 
 #include 
-
 #include 
 
 #define  AP_SAFE_STACK_SIZE128
@@ -801,6 +801,10 @@ MpInitLibStartupThisAP (  {
   EFI_STATUS  Status;
 
+  if (MpTdxIsEnabled ()) {
+return EFI_UNSUPPORTED;
+  }
+
   //
   // temporarily stop checkAllApsStatus for avoid resource dead-lock.
   //
@@ -857,6 +861,10 @@ MpInitLibSwitchBSP (
   EFI_TIMER_ARCH_PROTOCOL  *Timer;
   UINT64   TimerPeriod;
 
+  if (MpTdxIsEnabled ()) {
+return EFI_UNSUPPORTED;
+  }
+
   TimerPeriod = 0;
   //
   // Locate Timer Arch Protocol
@@ -930,6 +938,10 @@ MpInitLibEnableDisableAP (
   EFI_STATUS Status;
   BOOLEANTempStopCheckState;
 
+  if (MpTdxIsEnabled ()) {
+return EFI_UNSUPPORTED;
+  }
+
   TempStopCheckState = FALSE;
   //
   // temporarily stop checkAllAPsStatus for initialize parameters.
diff --git a/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h 
b/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
new file mode 100644
index ..59bd739eed22
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/MpIntelTdx.h
@@ -0,0 +1,107 @@
+/** @file
+  Intel Tdx header file.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MP_INTEL_TDX_H_
+#define MP_INTEL_TDX_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Gets detailed MP-related information on the requested processor at 
+the
+  instant this call is made. This service may only be called from the BSP.
+
+  @param[in]  ProcessorNumber   The handle number of processor.
+  @param[out] ProcessorInfoBuffer   A pointer to the buffer where information 
for
+the requested processor is deposited.
+  @param[out]  HealthDataReturn processor health data.
+
+  @retval EFI_SUCCESS Processor information was returned.
+  @retval EFI_DEVICE_ERRORThe calling processor is an AP.
+  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL.
+  @retval EFI_NOT_FOUND   The processor with the handle specified by
+  ProcessorNumber doe

[edk2-devel] [PATCH 4/4] Silicon/ChaosKeyDxe: Test the ControllerHandle is managed by this driver

2021-10-12 Thread Masami Hiramatsu
From: Kazuhiko Sakamoto 

Test the @ControllerHandle is not NULL and is actually managed by
this driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Kazuhiko Sakamoto 
Signed-off-by: Masami Hiramatsu 
---
 Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h |1 +
 Silicon/Openmoko/ChaosKeyDxe/ComponentName.c  |   13 +
 Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c  |1 -
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h 
b/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h
index 97cfbbb755..59582b3bf9 100644
--- a/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h
+++ b/Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h
@@ -38,6 +38,7 @@ typedef struct {
   CR(a, CHAOSKEY_DEV, Rng, CHAOSKEY_DEV_SIGNATURE)
 
 extern EFI_COMPONENT_NAME2_PROTOCOL gChaosKeyDriverComponentName2;
+extern EFI_DRIVER_BINDING_PROTOCOL  gUsbDriverBinding;
 
 EFI_STATUS
 ChaosKeyInit (
diff --git a/Silicon/Openmoko/ChaosKeyDxe/ComponentName.c 
b/Silicon/Openmoko/ChaosKeyDxe/ComponentName.c
index 25117e2500..5b0f42cb67 100644
--- a/Silicon/Openmoko/ChaosKeyDxe/ComponentName.c
+++ b/Silicon/Openmoko/ChaosKeyDxe/ComponentName.c
@@ -159,6 +159,19 @@ ChaosKeyGetControllerName (
   OUT CHAR16  **ControllerName
   )
 {
+  EFI_STATUS  Status;
+
+  if (!ControllerHandle) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = EfiTestManagedDevice (ControllerHandle,
+ gUsbDriverBinding.DriverBindingHandle,
+ &gEfiUsbIoProtocolGuid);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
   if (ChildHandle != NULL) {
 return EFI_UNSUPPORTED;
   }
diff --git a/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c 
b/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c
index e7d0d3fe56..4cbd23ad36 100644
--- a/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c
+++ b/Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c
@@ -146,7 +146,6 @@ UsbHwrngDriverBindingStop (
 }
 
 
-STATIC
 EFI_DRIVER_BINDING_PROTOCOL  gUsbDriverBinding = {
   UsbHwrngDriverBindingSupported,
   UsbHwrngDriverBindingStart,



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




[edk2-devel] [PATCH 3/4] Silicon/AtSha204a: Test the ControllerHandle is managed by this driver

2021-10-12 Thread Masami Hiramatsu
From: Kazuhiko Sakamoto 

Test the @ControllerHandle is not NULL and is actually managed by
this driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Kazuhiko Sakamoto 
Signed-off-by: Masami Hiramatsu 
---
 Silicon/Atmel/AtSha204a/AtSha204aDriver.h |1 +
 Silicon/Atmel/AtSha204a/ComponentName.c   |   13 +
 Silicon/Atmel/AtSha204a/DriverBinding.c   |1 -
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Silicon/Atmel/AtSha204a/AtSha204aDriver.h 
b/Silicon/Atmel/AtSha204a/AtSha204aDriver.h
index 615959baf4..11be15b25b 100644
--- a/Silicon/Atmel/AtSha204a/AtSha204aDriver.h
+++ b/Silicon/Atmel/AtSha204a/AtSha204aDriver.h
@@ -59,6 +59,7 @@ typedef struct {
 #define ATSHA204A_OPCODE_RANDOM   0x1b
 
 extern EFI_COMPONENT_NAME2_PROTOCOL gAtSha204aDriverComponentName2;
+extern EFI_DRIVER_BINDING_PROTOCOL  gI2cHwrngDriverBinding;
 
 EFI_STATUS
 AtSha204aInit (
diff --git a/Silicon/Atmel/AtSha204a/ComponentName.c 
b/Silicon/Atmel/AtSha204a/ComponentName.c
index eec7b9120b..cc1d970bcf 100644
--- a/Silicon/Atmel/AtSha204a/ComponentName.c
+++ b/Silicon/Atmel/AtSha204a/ComponentName.c
@@ -159,6 +159,19 @@ AtSha204aGetControllerName (
   OUT CHAR16  **ControllerName
   )
 {
+  EFI_STATUS  Status;
+
+  if (!ControllerHandle) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = EfiTestManagedDevice (ControllerHandle,
+ gI2cHwrngDriverBinding.DriverBindingHandle,
+ &gEfiI2cIoProtocolGuid);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
   if (ChildHandle != NULL) {
 return EFI_UNSUPPORTED;
   }
diff --git a/Silicon/Atmel/AtSha204a/DriverBinding.c 
b/Silicon/Atmel/AtSha204a/DriverBinding.c
index 38ffd80df9..e278ab0554 100644
--- a/Silicon/Atmel/AtSha204a/DriverBinding.c
+++ b/Silicon/Atmel/AtSha204a/DriverBinding.c
@@ -132,7 +132,6 @@ I2cHwrngDriverBindingStop (
 }
 
 
-STATIC
 EFI_DRIVER_BINDING_PROTOCOL  gI2cHwrngDriverBinding = {
   I2cHwrngDriverBindingSupported,
   I2cHwrngDriverBindingStart,



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




[edk2-devel] [PATCH 2/4] Silicon/SynQuacerI2cDxe: Test the ControllerHandle is managed by this driver

2021-10-12 Thread Masami Hiramatsu
From: Kazuhiko Sakamoto 

Test the @ControllerHandle is not NULL and is actually managed by
this driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Kazuhiko Sakamoto 
Signed-off-by: Masami Hiramatsu 
---
 .../Drivers/SynQuacerI2cDxe/ComponentName.c|   13 +
 .../Drivers/SynQuacerI2cDxe/DriverBinding.c|2 +-
 .../Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.h  |1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/ComponentName.c 
b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/ComponentName.c
index 9e7f189c13..ed5f11f107 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/ComponentName.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/ComponentName.c
@@ -158,6 +158,19 @@ SynQuacerI2cGetControllerName (
   OUT CHAR16  **ControllerName
   )
 {
+  EFI_STATUS  Status;
+
+  if (!ControllerHandle) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = EfiTestManagedDevice (ControllerHandle,
+ 
gSynQuacerI2cDriverBinding.DriverBindingHandle,
+ &gEdkiiNonDiscoverableDeviceProtocolGuid);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
   if (ChildHandle != NULL) {
 return EFI_UNSUPPORTED;
   }
diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/DriverBinding.c 
b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/DriverBinding.c
index 4e265aacf1..912ae79d14 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/DriverBinding.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/DriverBinding.c
@@ -129,7 +129,7 @@ SynQuacerI2cDriverBindingStop (
 }
 
 
-STATIC EFI_DRIVER_BINDING_PROTOCOL  gSynQuacerI2cDriverBinding = {
+EFI_DRIVER_BINDING_PROTOCOL  gSynQuacerI2cDriverBinding = {
   SynQuacerI2cDriverBindingSupported,
   SynQuacerI2cDriverBindingStart,
   SynQuacerI2cDriverBindingStop,
diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.h 
b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.h
index f891e4bf2f..c6534e6972 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.h
@@ -25,6 +25,7 @@
 #include 
 
 extern EFI_COMPONENT_NAME2_PROTOCOL gSynQuacerI2cDriverComponentName2;
+extern EFI_DRIVER_BINDING_PROTOCOL  gSynQuacerI2cDriverBinding;
 
 #define SYNQUACER_I2C_SIGNATURE SIGNATURE_32 ('S', 'I', '2', 'C')
 #define SYNQUACER_I2C_FROM_THIS(a)  CR ((a), SYNQUACER_I2C_MASTER, \



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




[edk2-devel] [PATCH 1/4] Silicon/SynQuacerNetsecDxe: Test the ControllerHandle is managed by this driver

2021-10-12 Thread Masami Hiramatsu
From: Kazuhiko Sakamoto 

Test the @ControllerHandle is not NULL and is actually managed by
this driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Kazuhiko Sakamoto 
Signed-off-by: Masami Hiramatsu 
---
 .../Drivers/Net/NetsecDxe/ComponentName.c  |   13 +
 .../Drivers/Net/NetsecDxe/DriverBinding.c  |1 -
 .../SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h|1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/ComponentName.c 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/ComponentName.c
index 44b3daa0af..743fa88384 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/ComponentName.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/ComponentName.c
@@ -159,6 +159,19 @@ NetsecGetControllerName (
   OUT CHAR16  **ControllerName
   )
 {
+  EFI_STATUS  Status;
+
+  if (!ControllerHandle) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = EfiTestManagedDevice (ControllerHandle,
+ gNetsecDriverBinding.DriverBindingHandle,
+ &gEdkiiNonDiscoverableDeviceProtocolGuid);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
   if (ChildHandle != NULL) {
 return EFI_UNSUPPORTED;
   }
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/DriverBinding.c 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/DriverBinding.c
index 392d1b474f..fa0c415e98 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/DriverBinding.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/DriverBinding.c
@@ -131,7 +131,6 @@ NetsecDriverBindingStop (
 }
 
 
-STATIC
 EFI_DRIVER_BINDING_PROTOCOL  gNetsecDriverBinding = {
   NetsecDriverBindingSupported,
   NetsecDriverBindingStart,
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h 
b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
index cf2abb0ab1..9b3d19c033 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h
@@ -27,6 +27,7 @@
 #include "netsec_for_uefi/pfdep.h"
 
 extern EFI_COMPONENT_NAME2_PROTOCOL gNetsecDriverComponentName2;
+extern EFI_DRIVER_BINDING_PROTOCOL  gNetsecDriverBinding;
 
 /*--- Simple Network Driver entry point functions 
*/
 



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




[edk2-devel] [PATCH 0/4] SynQuacer drivers test the ControllerHandle correctly

2021-10-12 Thread Masami Hiramatsu
Hello,

Here are the patches to fix the SynQuacer related drivers to test
whether the ControllerHandle is managed by that driver correctly.
These bugs are found by edk2-test.

Thank you,

---

Kazuhiko Sakamoto (4):
  Silicon/SynQuacerNetsecDxe: Test the ControllerHandle is managed by this 
driver
  Silicon/SynQuacerI2cDxe: Test the ControllerHandle is managed by this 
driver
  Silicon/AtSha204a: Test the ControllerHandle is managed by this driver
  Silicon/ChaosKeyDxe: Test the ControllerHandle is managed by this driver


 Silicon/Atmel/AtSha204a/AtSha204aDriver.h  |1 +
 Silicon/Atmel/AtSha204a/ComponentName.c|   13 +
 Silicon/Atmel/AtSha204a/DriverBinding.c|1 -
 Silicon/Openmoko/ChaosKeyDxe/ChaosKeyDriver.h  |1 +
 Silicon/Openmoko/ChaosKeyDxe/ComponentName.c   |   13 +
 Silicon/Openmoko/ChaosKeyDxe/DriverBinding.c   |1 -
 .../Drivers/Net/NetsecDxe/ComponentName.c  |   13 +
 .../Drivers/Net/NetsecDxe/DriverBinding.c  |1 -
 .../SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.h|1 +
 .../Drivers/SynQuacerI2cDxe/ComponentName.c|   13 +
 .../Drivers/SynQuacerI2cDxe/DriverBinding.c|2 +-
 .../Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.h  |1 +
 12 files changed, 57 insertions(+), 4 deletions(-)

--
Masami Hiramatsu 


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




Re: [edk2-devel] [PATCH] Silicon/SynQuacerI2cDxe: Wait for bus busy

2021-10-12 Thread Masami Hiramatsu
Hello Ard,

Would you have any comment on this fix?

Thank you,

2021年9月30日(木) 14:44 Masami Hiramatsu via groups.io :

> If an EFI application frequently repeats SetTime and GetTime,
> the I2C bus can be busy and failed to start. To fix this issue,
> add waiting loop for the bus busy status. (Usually, it is
> enough to read 3 times for checking, but for safety this
> sets 10 for timeout.)
>
> This also clean up the code path a bit so that it is easy to
> understand what should do on each combinations of BSR.BB and
> BCR.MSS.
>
> Signed-off-by: Masami Hiramatsu 
> Reported-by: Kazuhiko Sakamoto 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  .../Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c  |   38
> ++--
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git
> a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c
> b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c
> index 31f6e3072f..380eba8059 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerI2cDxe/SynQuacerI2cDxe.c
> @@ -16,6 +16,8 @@
>  //
>  #define WAIT_FOR_INTERRUPT_TIMEOUT5
>
> +#define WAIT_FOR_BUS_BUSY_TIMEOUT10
> +
>  /**
>Set the frequency for the I2C clock line.
>
> @@ -152,6 +154,7 @@ SynQuacerI2cMasterStart (
>IN  EFI_I2C_OPERATION   *Op
>)
>  {
> +  UINTN   Timeout = WAIT_FOR_BUS_BUSY_TIMEOUT;
>UINT8   Bsr;
>UINT8   Bcr;
>
> @@ -167,24 +170,35 @@ SynQuacerI2cMasterStart (
>Bsr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR);
>Bcr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BCR);
>
> -  if ((Bsr & F_I2C_BSR_BB) && !(Bcr & F_I2C_BCR_MSS)) {
> -DEBUG ((DEBUG_INFO, "%a: bus is busy\n", __FUNCTION__));
> -return EFI_ALREADY_STARTED;
> -  }
> +  if (!(Bcr & F_I2C_BCR_MSS)) {
>
> -  if (Bsr & F_I2C_BSR_BB) { // Bus is busy
> -DEBUG ((DEBUG_INFO, "%a: Continuous Start\n", __FUNCTION__));
> -MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, Bcr | F_I2C_BCR_SCC);
> -  } else {
> -if (Bcr & F_I2C_BCR_MSS) {
> -  DEBUG ((DEBUG_WARN,
> -"%a: is not in master mode\n", __FUNCTION__));
> -  return EFI_DEVICE_ERROR;
> +if (Bsr & F_I2C_BSR_BB) { // Bus is busy
> +do {
> +  Bsr = MmioRead8 (I2c->MmioBase + F_I2C_REG_BSR);
> +} while (Timeout-- && (Bsr & F_I2C_BSR_BB));
> +
> +if (Bsr & F_I2C_BSR_BB) {
> +  DEBUG ((DEBUG_INFO, "%a: bus is busy\n", __FUNCTION__));
> +  return EFI_ALREADY_STARTED;
> +}
>  }
> +
>  DEBUG ((DEBUG_INFO, "%a: Start Condition\n", __FUNCTION__));
>  MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR,
>  Bcr | F_I2C_BCR_MSS | F_I2C_BCR_INTE | F_I2C_BCR_BEIE);
> +
> +  } else { // F_I2C_BCR_MSS is set
> +
> +if (!(Bsr & F_I2C_BSR_BB)) {
> +  DEBUG ((DEBUG_WARN,
> +"%a: is not in master mode\n", __FUNCTION__));
> +  return EFI_DEVICE_ERROR;
> +}
> +
> +DEBUG ((DEBUG_INFO, "%a: Continuous Start\n", __FUNCTION__));
> +MmioWrite8 (I2c->MmioBase + F_I2C_REG_BCR, Bcr | F_I2C_BCR_SCC);
>}
> +
>return EFI_SUCCESS;
>  }
>
>
>
>
> 
>
>
>

-- 
Masami Hiramatsu


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




Re: [edk2-devel] [PATCH V2 07/28] UefiCpuPkg: Support TDX in BaseXApicX2ApicLib

2021-10-12 Thread Ni, Ray
Min,
Comments below:

-Original Message-
From: Xu, Min M  
Sent: Tuesday, October 5, 2021 11:39 AM
To: devel@edk2.groups.io
Cc: Xu, Min M ; Dong, Eric ; Ni, Ray 
; Kumar, Rahul1 ; Brijesh Singh 
; Erdem Aktas ; James Bottomley 
; Yao, Jiewen ; Tom Lendacky 

Subject: [PATCH V2 07/28] UefiCpuPkg: Support TDX in BaseXApicX2ApicLib

+**/
+BOOLEAN
+EFIAPI

1. EFIAPI is for public lib API. Is this a public API?

+BaseXApicIsTdxGuest (
+  VOID
+  )
+{
+  UINT32Eax;
+  UINT32Ebx;
+  UINT32Ecx;
+  UINT32Edx;
+  UINT32LargestEax;
+
+  if (mBaseXApicTdxProbed) {
+return mBaseXApicIsTdxEnabled;
+  }
+
+  mBaseXApicIsTdxEnabled = FALSE;

2. ApicLib can be used in pre-mem running directly in flash.
The global variable cannot be modified in that case.


+
+  do {
+AsmCpuid (0, &LargestEax, &Ebx, &Ecx, &Edx);

+
+if (Ebx != SIGNATURE_32 ('G', 'e', 'n', 'u')
+  || Edx != SIGNATURE_32 ('i', 'n', 'e', 'I')
+  || Ecx != SIGNATURE_32 ('n', 't', 'e', 'l')) {
+  break;
+}
+
+AsmCpuid (1, NULL, NULL, &Ecx, NULL);
+if ((Ecx & BIT31) == 0) {
+  break;
+}
+
+if (LargestEax < 0x21) {
+  break;
+}
+
+AsmCpuidEx (0x21, 0, &Eax, &Ebx, &Ecx, &Edx);
+if (Ebx != SIGNATURE_32 ('I', 'n', 't', 'e')
+  || Edx != SIGNATURE_32 ('l', 'T', 'D', 'X')
+  || Ecx != SIGNATURE_32 (' ', ' ', ' ', ' ')) {
+  break;
+}


3. Can you use definition from MdePkg\Include\Register\Intel\Cpuid.h instead of 
hardcode 0, 1, 0x21, "Genu" and etc.?

+
+mBaseXApicIsTdxEnabled = TRUE;

4. avoid relying on global variable for caching the result.

+
+  switch (MsrIndex) {
+  case MSR_IA32_X2APIC_TPR:
+  case MSR_IA32_X2APIC_PPR:
+  case MSR_IA32_X2APIC_EOI:
+  case MSR_IA32_X2APIC_ISR0:
+  case MSR_IA32_X2APIC_ISR1:
+  case MSR_IA32_X2APIC_ISR2:
+  case MSR_IA32_X2APIC_ISR3:
+  case MSR_IA32_X2APIC_ISR4:
+  case MSR_IA32_X2APIC_ISR5:
+  case MSR_IA32_X2APIC_ISR6:
+  case MSR_IA32_X2APIC_ISR7:
+  case MSR_IA32_X2APIC_TMR0:
+  case MSR_IA32_X2APIC_TMR1:
+  case MSR_IA32_X2APIC_TMR2:
+  case MSR_IA32_X2APIC_TMR3:
+  case MSR_IA32_X2APIC_TMR4:
+  case MSR_IA32_X2APIC_TMR5:
+  case MSR_IA32_X2APIC_TMR6:
+  case MSR_IA32_X2APIC_TMR7:
+  case MSR_IA32_X2APIC_IRR0:
+  case MSR_IA32_X2APIC_IRR1:
+  case MSR_IA32_X2APIC_IRR2:
+  case MSR_IA32_X2APIC_IRR3:
+  case MSR_IA32_X2APIC_IRR4:
+  case MSR_IA32_X2APIC_IRR5:
+  case MSR_IA32_X2APIC_IRR6:
+  case MSR_IA32_X2APIC_IRR7:

5. Can you explain in the comments about  what spec says that above MSR can be 
accessed directly while others cannot?


+  UINT64Val;
+  UINT64Status;
+  if (!AccessMsrNative (MsrIndex) && BaseXApicIsTdxGuest ()) {

6. can we simplify the above check with " if (!AccessMsrNative (MsrIndex))"?
IsTdxGuest() can be called inside AccessMsrNative().

+UINT32
+EFIAPI

7. No EFIAPI please.

+ReadMsrReg32 (
+  IN UINT32 MsrIndex
+  )
+{
+  UINT64Val;
+  UINT64Status;
+  if (!AccessMsrNative (MsrIndex) && BaseXApicIsTdxGuest ()) {
+Status = TdVmCall (TDVMCALL_RDMSR, (UINT64) MsrIndex, 0, 0, 0, &Val);
+if (Status != 0) {
+  TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
+}
+  } else {
+Val = AsmReadMsr32 (MsrIndex);
+  }
+  return (UINT32)(UINTN) Val;

8. Can you directly call ReadMsrReg64()?


+VOID
+EFIAPI
+WriteMsrReg32 (
+  IN UINT32 MsrIndex,
+  IN UINT32 Val
+  )
+{
+  UINT64Status;
+  if (!AccessMsrNative (MsrIndex) && BaseXApicIsTdxGuest ()) {
+Status = TdVmCall (TDVMCALL_WRMSR, (UINT64) MsrIndex, (UINT64) Val, 0, 0, 
0);
+if (Status != 0) {
+  DEBUG((DEBUG_ERROR, "WriteMsrReg32 returned failure. Status=0x%llx\n", 
Status));
+  TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
+}
+  } else {
+AsmWriteMsr32 (MsrIndex, Val);

8. Can you directly call WriteMsrReg64()?

 
-  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+  ApicBaseMsr.Uint64 = ReadMsrReg64 (MSR_IA32_APIC_BASE);

9. I prefer to use "LocalApicLibReadMsr64()". It indicates two meanings:
a. it's a local function which can be found within this lib
b. it's consistent with "AsmReadMsr64".




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




Re: [edk2-devel] [PATCH V9 4/4] OvmfPkg: Enable TDX in ResetVector

2021-10-12 Thread Min Xu
On October 12, 2021 3:43 PM, Gerd Hoffmann wrote:
>   Hi,
> 
> > +; Load the GDT and set the CR0.
> > +;
> > +; Modified:  EAX, EBX, CR0, CR4, DS, ES, FS, GS, SS, CS ;
> > +ReloadFlat32:
> > +
> > +cli
> > +mov ebx, ADDR_OF(gdtr)
> > +lgdt[ebx]
> 
> No need to modify ebx here, eax should do fine.
You're right. It will be updated in next version.
> 
> > +mov eax, SEC_DEFAULT_CR0
> > +mov cr0, eax
> > +
> > +jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpToFlat32BitAndLandHere)
> > +
> > +jumpToFlat32BitAndLandHere:
> 
> Strictly speaking this is not correct, you are already in Flat32 mode, so 
> this only
> loads cs.
TDX: 
https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-module-1.0-public-spec-v0.931.pdf
In [TDX] Section 10.1.3 CR0 and CR4 are initialized when Tdx guest is created. 
So initialization of CR0 and CR4 are not needed.
It will be updated in next version.
> 
> > +InitTdx:
> > +;
> > +; Save EBX in EBP because EBX will be changed in ReloadFlat32
> > +;
> > +mov ebp, ebx
> 
> See above, there is no need to modify ebx in ReloadFlat32.
> Also: seems ebx is never restored ...
In [TDX] Section 10.1.2 EBX[5:0] contains the GPAW. Since EBX is not changed in 
ReloadFlat32, *mov ebp, ebx* is not needed.
It will be removed in next version.
> 

Thanks!
Min


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




Re: [edk2-devel] [PATCH v2] UefiPayloadPkg: Remove SystemTableInfo GUID.

2021-10-12 Thread Ni, Ray
Reviewed-by: Ray Ni 

-Original Message-
From: Kesavan Balakrishnan, ThiyaguX  
Sent: Wednesday, October 13, 2021 1:08 PM
To: devel@edk2.groups.io
Cc: Kesavan Balakrishnan, ThiyaguX ; 
Ma, Maurice ; Dong, Guo ; Ni, Ray 
; You, Benjamin ; Liu, Zhiguang 

Subject: [PATCH v2] UefiPayloadPkg: Remove SystemTableInfo GUID.

SystemTableInfo GUID is not a Spec defined GUID.
But the latest SBL uses SystemTableInfo to get ACPI
and SMBIOS table information. So moving the SystemTableInfo
GUID implementation to SblParseLib.

Cc: Maurice Ma 
Cc: Guo Dong 
Cc: Ray Ni 
Cc: Benjamin You 
Cc: Zhiguang Liu 

Signed-off-by: Guo Dong 
Signed-off-by: Thiyagu Kesavan Balakrishnan 

---
 UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf  |  1 -
 UefiPayloadPkg/Include/Library/BlParseLib.h   | 24 +--
 .../Library/CbParseLib/CbParseLib.c   | 40 ++-
 .../Library/SblParseLib/SblParseLib.c | 38 +++---
 .../UefiPayloadEntry/UefiPayloadEntry.c   | 28 +
 .../UefiPayloadEntry/UefiPayloadEntry.h   |  1 -
 .../UniversalPayloadEntry.inf |  1 -
 7 files changed, 94 insertions(+), 39 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf 
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
index 1ccb250991..96d85d2b1d 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -42,7 +42,6 @@
   HobLib
 
 [Guids]
-  gUefiSystemTableInfoGuid
   gUefiAcpiBoardInfoGuid
   gEfiGraphicsInfoHobGuid
 
diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h 
b/UefiPayloadPkg/Include/Library/BlParseLib.h
index 1244190d4e..89e728164d 100644
--- a/UefiPayloadPkg/Include/Library/BlParseLib.h
+++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #ifndef __BOOTLOADER_PARSE_LIB__
 #define __BOOTLOADER_PARSE_LIB__
@@ -55,9 +57,9 @@ ParseMemoryInfo (
   );
 
 /**
-  Acquire acpi table and smbios table from slim bootloader
+  Acquire SMBIOS table from bootloader.
 
-  @param  SystemTableInfo   Pointer to the system table info
+  @param  SmbiosTable   Pointer to the SMBIOS table info.
 
   @retval RETURN_SUCCESSSuccessfully find out the tables.
   @retval RETURN_NOT_FOUND  Failed to find the tables.
@@ -65,10 +67,24 @@ ParseMemoryInfo (
 **/
 RETURN_STATUS
 EFIAPI
-ParseSystemTable (
-  OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
   );
 
+/**
+  Acquire ACPI table from bootloader.
+
+  @param  AcpiTableHob  Pointer to the ACPI table info.
+
+  @retval RETURN_SUCCESSSuccessfully find out the tables.
+  @retval RETURN_NOT_FOUND  Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+  OUT UNIVERSAL_PAYLOAD_ACPI_TABLE*AcpiTableHob
+  );
 
 /**
   Find the serial port information
diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c 
b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
index 4f90687e40..db22fdd926 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -410,9 +410,9 @@ ParseMemoryInfo (
 
 
 /**
-  Acquire acpi table and smbios table from coreboot
+  Acquire SMBIOS table from coreboot.
 
-  @param  SystemTableInfo  Pointer to the system table info
+  @param  SmbiosTable   Pointer to the SMBIOS table info.
 
   @retval RETURN_SUCCESSSuccessfully find out the tables.
   @retval RETURN_NOT_FOUND  Failed to find the tables.
@@ -420,8 +420,8 @@ ParseMemoryInfo (
 **/
 RETURN_STATUS
 EFIAPI
-ParseSystemTable (
-  OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+  OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
   )
 {
   EFI_STATUS   Status;
@@ -432,17 +432,39 @@ ParseSystemTable (
   if (EFI_ERROR (Status)) {
 return EFI_NOT_FOUND;
   }
-  SystemTableInfo->SmbiosTableBase = (UINT64) (UINTN)MemTable;
-  SystemTableInfo->SmbiosTableSize = MemTableSize;
+  SmbiosTable->SmBiosEntryPoint = (UINT64) (UINTN)MemTable;
+
+  return RETURN_SUCCESS;
+}
+
+
+/**
+  Acquire ACPI table from coreboot.
+
+  @param  AcpiTableHob  Pointer to the ACPI table info.
+
+  @retval RETURN_SUCCESSSuccessfully find out the tables.
+  @retval RETURN_NOT_FOUND  Failed to find the tables.
+
+**/
+
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+  OUT UNIVERSAL_PAYLOAD_ACPI_TABLE*AcpiTableHob
+  )
+{
+  EFI_STATUS   Status;
+  VOID *MemTable;
+  UINT32   MemTableSize;
 
   Status = ParseCbMemTable (SIGNATURE_32 ('I', 'P', 'C', 'A'), &MemTable, 
&MemTableSize);
   if (EFI_ERROR (Status)) {
 return EFI_NOT_FOUND;
   }
-  SystemTableInfo->AcpiTableBase = (UINT64) (UINTN)MemTable;
-  SystemTableInfo->AcpiTableSize = MemTableSize;
+  AcpiTableHob->Rsdp = (UINT64) (UINTN)MemTable;
 
-  return Status;
+  

[edk2-devel] [PATCH v2] UefiPayloadPkg: Remove SystemTableInfo GUID.

2021-10-12 Thread thiyagukb
SystemTableInfo GUID is not a Spec defined GUID.
But the latest SBL uses SystemTableInfo to get ACPI
and SMBIOS table information. So moving the SystemTableInfo
GUID implementation to SblParseLib.

Cc: Maurice Ma 
Cc: Guo Dong 
Cc: Ray Ni 
Cc: Benjamin You 
Cc: Zhiguang Liu 

Signed-off-by: Guo Dong 
Signed-off-by: Thiyagu Kesavan Balakrishnan 

---
 UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf  |  1 -
 UefiPayloadPkg/Include/Library/BlParseLib.h   | 24 +--
 .../Library/CbParseLib/CbParseLib.c   | 40 ++-
 .../Library/SblParseLib/SblParseLib.c | 38 +++---
 .../UefiPayloadEntry/UefiPayloadEntry.c   | 28 +
 .../UefiPayloadEntry/UefiPayloadEntry.h   |  1 -
 .../UniversalPayloadEntry.inf |  1 -
 7 files changed, 94 insertions(+), 39 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf 
b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
index 1ccb250991..96d85d2b1d 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
@@ -42,7 +42,6 @@
   HobLib
 
 [Guids]
-  gUefiSystemTableInfoGuid
   gUefiAcpiBoardInfoGuid
   gEfiGraphicsInfoHobGuid
 
diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h 
b/UefiPayloadPkg/Include/Library/BlParseLib.h
index 1244190d4e..89e728164d 100644
--- a/UefiPayloadPkg/Include/Library/BlParseLib.h
+++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #ifndef __BOOTLOADER_PARSE_LIB__
 #define __BOOTLOADER_PARSE_LIB__
@@ -55,9 +57,9 @@ ParseMemoryInfo (
   );
 
 /**
-  Acquire acpi table and smbios table from slim bootloader
+  Acquire SMBIOS table from bootloader.
 
-  @param  SystemTableInfo   Pointer to the system table info
+  @param  SmbiosTable   Pointer to the SMBIOS table info.
 
   @retval RETURN_SUCCESSSuccessfully find out the tables.
   @retval RETURN_NOT_FOUND  Failed to find the tables.
@@ -65,10 +67,24 @@ ParseMemoryInfo (
 **/
 RETURN_STATUS
 EFIAPI
-ParseSystemTable (
-  OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
   );
 
+/**
+  Acquire ACPI table from bootloader.
+
+  @param  AcpiTableHob  Pointer to the ACPI table info.
+
+  @retval RETURN_SUCCESSSuccessfully find out the tables.
+  @retval RETURN_NOT_FOUND  Failed to find the tables.
+
+**/
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+  OUT UNIVERSAL_PAYLOAD_ACPI_TABLE*AcpiTableHob
+  );
 
 /**
   Find the serial port information
diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c 
b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
index 4f90687e40..db22fdd926 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -410,9 +410,9 @@ ParseMemoryInfo (
 
 
 /**
-  Acquire acpi table and smbios table from coreboot
+  Acquire SMBIOS table from coreboot.
 
-  @param  SystemTableInfo  Pointer to the system table info
+  @param  SmbiosTable   Pointer to the SMBIOS table info.
 
   @retval RETURN_SUCCESSSuccessfully find out the tables.
   @retval RETURN_NOT_FOUND  Failed to find the tables.
@@ -420,8 +420,8 @@ ParseMemoryInfo (
 **/
 RETURN_STATUS
 EFIAPI
-ParseSystemTable (
-  OUT SYSTEM_TABLE_INFO *SystemTableInfo
+ParseSmbiosTable (
+  OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
   )
 {
   EFI_STATUS   Status;
@@ -432,17 +432,39 @@ ParseSystemTable (
   if (EFI_ERROR (Status)) {
 return EFI_NOT_FOUND;
   }
-  SystemTableInfo->SmbiosTableBase = (UINT64) (UINTN)MemTable;
-  SystemTableInfo->SmbiosTableSize = MemTableSize;
+  SmbiosTable->SmBiosEntryPoint = (UINT64) (UINTN)MemTable;
+
+  return RETURN_SUCCESS;
+}
+
+
+/**
+  Acquire ACPI table from coreboot.
+
+  @param  AcpiTableHob  Pointer to the ACPI table info.
+
+  @retval RETURN_SUCCESSSuccessfully find out the tables.
+  @retval RETURN_NOT_FOUND  Failed to find the tables.
+
+**/
+
+RETURN_STATUS
+EFIAPI
+ParseAcpiTableInfo (
+  OUT UNIVERSAL_PAYLOAD_ACPI_TABLE*AcpiTableHob
+  )
+{
+  EFI_STATUS   Status;
+  VOID *MemTable;
+  UINT32   MemTableSize;
 
   Status = ParseCbMemTable (SIGNATURE_32 ('I', 'P', 'C', 'A'), &MemTable, 
&MemTableSize);
   if (EFI_ERROR (Status)) {
 return EFI_NOT_FOUND;
   }
-  SystemTableInfo->AcpiTableBase = (UINT64) (UINTN)MemTable;
-  SystemTableInfo->AcpiTableSize = MemTableSize;
+  AcpiTableHob->Rsdp = (UINT64) (UINTN)MemTable;
 
-  return Status;
+  return RETURN_SUCCESS;
 }
 
 
diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c 
b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
index 7214fd87d2..0f83771e6e 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
@@ -110,9 +110,9 @@ ParseMemoryInfo (

Re: [edk2-devel] [PATCH V2 21/28] OvmfPkg: Update PlatformPei to support TDX

2021-10-12 Thread Gerd Hoffmann
  Hi,

> 2. Relocate mailbox
>   At the beginning of system boot, a 4K-aligned, 4K-size memory (Td
>   mailbox) is pre-allocated by host VMM. BSP & APs do the page accept
>   together in that memory region.
>   After that TDVF is designed to relocate the mailbox to a 4K-aligned,
>   4K-size memory block which is allocated in the ACPI Nvs memory. APs
>   are waken up and spin around the relocated mailbox waiting for
>   further command.

Why is the mailbox relocated?  Are there any problems when simply
continuing to use the memfd page?

The memfd page location is known to qemu, so when not relocating the
mailbox the MADT update could be done on the host side.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH v2] MdeModulePkg/Core/Dxe: Acquire a lock when iterating gHandleList

2021-10-12 Thread Wang, Jian J
Hi Hua,

It looks a bit odd to me to add 'IsLocked' parameter and acquire lock
inside CoreValidateHandle() if it's FALSE. Maybe we can keep the
function prototype as-is but do something like below:

a) Just keep ASSERT_LOCKED(&gProtocolDatabaseLock) in CoreValidateHandle()
b) Call CoreAcquireProtocolLock() before any calling of CoreValidateHandle()
 and CoreReleaseProtocolLock() afterwards.

Actually, CoreAcquireProtocolLock() is always called wherever 
CoreValidateHandle()
is called. The problem is that, in many cases, CoreAcquireProtocolLock() is 
called
after CoreValidateHandle(). We can simply move the calling of 
CoreAcquireProtocolLock()
before CoreValidateHandle() to fix this problem.

For those cases CoreAcquireProtocolLock() is not called at all, just simply add 
it.

Regards,
Jian

> -Original Message-
> From: Ma, Hua 
> Sent: Tuesday, October 12, 2021 4:34 PM
> To: devel@edk2.groups.io
> Cc: Ma, Hua ; Wang, Jian J ;
> Liming Gao ; Bi, Dandan ;
> Ni, Ray 
> Subject: [PATCH v2] MdeModulePkg/Core/Dxe: Acquire a lock when iterating
> gHandleList
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3680
> 
> This patch fixes the following issue:
> 
> The global variable gHandleList is a linked list.
> This list is locked when a entry is added or removed from the list,
> but there is no lock when iterating this list in function
> CoreValidateHandle().
> It can lead to "Handle.c (76): CR has Bad Signature" assertion if the
> iterated entry in the list is just removed by other task during iterating.
> Locking the list when iterating can fix this issue.
> 
> v2 changes:
>  - Add lock check and comments in CoreGetProtocolInterface() before
> calling CoreValidateHandle()
>  - Update the comments in CoreValidateHandle() header file
> 
> v1: https://edk2.groups.io/g/devel/topic/86233569
> 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> Cc: Dandan Bi 
> Cc: Ray Ni 
> Signed-off-by: Hua Ma 
> ---
>  MdeModulePkg/Core/Dxe/Hand/DriverSupport.c | 10 ++--
>  MdeModulePkg/Core/Dxe/Hand/Handle.c| 56 +++---
>  MdeModulePkg/Core/Dxe/Hand/Handle.h|  5 +-
>  MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
>  4 files changed, 50 insertions(+), 23 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> index feabf12faf..eb8a765d2c 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> @@ -68,7 +68,7 @@ CoreConnectController (
>//
>// Make sure ControllerHandle is valid
>//
> -  Status = CoreValidateHandle (ControllerHandle);
> +  Status = CoreValidateHandle (ControllerHandle, FALSE);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> @@ -154,7 +154,7 @@ CoreConnectController (
>  //
>  // Make sure the DriverBindingHandle is valid
>  //
> -Status = CoreValidateHandle (ControllerHandle);
> +Status = CoreValidateHandle (ControllerHandle, TRUE);
>  if (EFI_ERROR (Status)) {
>//
>// Release the protocol lock on the handle database
> @@ -268,7 +268,7 @@ AddSortedDriverBindingProtocol (
>//
>// Make sure the DriverBindingHandle is valid
>//
> -  Status = CoreValidateHandle (DriverBindingHandle);
> +  Status = CoreValidateHandle (DriverBindingHandle, FALSE);
>if (EFI_ERROR (Status)) {
>  return;
>}
> @@ -746,7 +746,7 @@ CoreDisconnectController (
>//
>// Make sure ControllerHandle is valid
>//
> -  Status = CoreValidateHandle (ControllerHandle);
> +  Status = CoreValidateHandle (ControllerHandle, FALSE);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> @@ -755,7 +755,7 @@ CoreDisconnectController (
>// Make sure ChildHandle is valid if it is not NULL
>//
>if (ChildHandle != NULL) {
> -Status = CoreValidateHandle (ChildHandle);
> +Status = CoreValidateHandle (ChildHandle, FALSE);
>  if (EFI_ERROR (Status)) {
>return Status;
>  }
> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 6eccb41ecb..46f67d3d6a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -55,31 +55,46 @@ CoreReleaseProtocolLock (
>Check whether a handle is a valid EFI_HANDLE
> 
>@param  UserHandle The handle to check
> +  @param  IsLocked   The protocol lock is acquried or not
> 
>@retval EFI_INVALID_PARAMETER  The handle is NULL or not a valid
> EFI_HANDLE.
> +  @retval EFI_NOT_FOUND  The handle is not found in the handle 
> database.
>@retval EFI_SUCCESSThe handle is valid EFI_HANDLE.
> 
>  **/
>  EFI_STATUS
>  CoreValidateHandle (
> -  IN  EFI_HANDLEUserHandle
> +  IN  EFI_HANDLEUserHandle,
> +  IN  BOOLEAN   IsLocked
>)
>  {
>IHANDLE *Handle;
>LIST_ENTRY  *Link;
> +  EFI_STATUS  Status;
> 
>if (Use

Re: [edk2-devel] [PATCH v2] MdeModulePkg/Core/Dxe: Acquire a lock when iterating gHandleList

2021-10-12 Thread Dandan Bi
Reviewed-by: Dandan Bi 



Thanks,
Dandan

> -Original Message-
> From: Ma, Hua 
> Sent: Tuesday, October 12, 2021 4:34 PM
> To: devel@edk2.groups.io
> Cc: Ma, Hua ; Wang, Jian J ;
> Liming Gao ; Bi, Dandan ;
> Ni, Ray 
> Subject: [PATCH v2] MdeModulePkg/Core/Dxe: Acquire a lock when iterating
> gHandleList
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3680
> 
> This patch fixes the following issue:
> 
> The global variable gHandleList is a linked list.
> This list is locked when a entry is added or removed from the list, but there 
> is no
> lock when iterating this list in function CoreValidateHandle().
> It can lead to "Handle.c (76): CR has Bad Signature" assertion if the iterated
> entry in the list is just removed by other task during iterating.
> Locking the list when iterating can fix this issue.
> 
> v2 changes:
>  - Add lock check and comments in CoreGetProtocolInterface() before calling
> CoreValidateHandle()
>  - Update the comments in CoreValidateHandle() header file
> 
> v1: https://edk2.groups.io/g/devel/topic/86233569
> 
> Cc: Jian J Wang 
> Cc: Liming Gao 
> Cc: Dandan Bi 
> Cc: Ray Ni 
> Signed-off-by: Hua Ma 
> ---
>  MdeModulePkg/Core/Dxe/Hand/DriverSupport.c | 10 ++--
>  MdeModulePkg/Core/Dxe/Hand/Handle.c| 56 +++---
>  MdeModulePkg/Core/Dxe/Hand/Handle.h|  5 +-
>  MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
>  4 files changed, 50 insertions(+), 23 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> index feabf12faf..eb8a765d2c 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> @@ -68,7 +68,7 @@ CoreConnectController (
>//
>// Make sure ControllerHandle is valid
>//
> -  Status = CoreValidateHandle (ControllerHandle);
> +  Status = CoreValidateHandle (ControllerHandle, FALSE);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> @@ -154,7 +154,7 @@ CoreConnectController (
>  //
>  // Make sure the DriverBindingHandle is valid
>  //
> -Status = CoreValidateHandle (ControllerHandle);
> +Status = CoreValidateHandle (ControllerHandle, TRUE);
>  if (EFI_ERROR (Status)) {
>//
>// Release the protocol lock on the handle database @@ -268,7 +268,7 @@
> AddSortedDriverBindingProtocol (
>//
>// Make sure the DriverBindingHandle is valid
>//
> -  Status = CoreValidateHandle (DriverBindingHandle);
> +  Status = CoreValidateHandle (DriverBindingHandle, FALSE);
>if (EFI_ERROR (Status)) {
>  return;
>}
> @@ -746,7 +746,7 @@ CoreDisconnectController (
>//
>// Make sure ControllerHandle is valid
>//
> -  Status = CoreValidateHandle (ControllerHandle);
> +  Status = CoreValidateHandle (ControllerHandle, FALSE);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> @@ -755,7 +755,7 @@ CoreDisconnectController (
>// Make sure ChildHandle is valid if it is not NULL
>//
>if (ChildHandle != NULL) {
> -Status = CoreValidateHandle (ChildHandle);
> +Status = CoreValidateHandle (ChildHandle, FALSE);
>  if (EFI_ERROR (Status)) {
>return Status;
>  }
> diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> index 6eccb41ecb..46f67d3d6a 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> @@ -55,31 +55,46 @@ CoreReleaseProtocolLock (
>Check whether a handle is a valid EFI_HANDLE
> 
>@param  UserHandle The handle to check
> +  @param  IsLocked   The protocol lock is acquried or not
> 
>@retval EFI_INVALID_PARAMETER  The handle is NULL or not a valid
> EFI_HANDLE.
> +  @retval EFI_NOT_FOUND  The handle is not found in the handle
> database.
>@retval EFI_SUCCESSThe handle is valid EFI_HANDLE.
> 
>  **/
>  EFI_STATUS
>  CoreValidateHandle (
> -  IN  EFI_HANDLEUserHandle
> +  IN  EFI_HANDLEUserHandle,
> +  IN  BOOLEAN   IsLocked
>)
>  {
>IHANDLE *Handle;
>LIST_ENTRY  *Link;
> +  EFI_STATUS  Status;
> 
>if (UserHandle == NULL) {
>  return EFI_INVALID_PARAMETER;
>}
> 
> +  if (IsLocked) {
> +ASSERT_LOCKED(&gProtocolDatabaseLock);
> +  } else {
> +CoreAcquireProtocolLock ();
> +  }
> +
> +  Status = EFI_NOT_FOUND;
>for (Link = gHandleList.BackLink; Link != &gHandleList; Link = 
> Link->BackLink) {
>  Handle = CR (Link, IHANDLE, AllHandles, EFI_HANDLE_SIGNATURE);
>  if (Handle == (IHANDLE *) UserHandle) {
> -  return EFI_SUCCESS;
> +  Status = EFI_SUCCESS;
> +  break;
>  }
>}
> 
> -  return EFI_INVALID_PARAMETER;
> +  if (!IsLocked) {
> +CoreReleaseProtocolLock ();
> +  }
> +  return Status;
>  }
> 
> 
> @@ -428,7 +443,7 @@ CoreInstallProtocolInterfaceNotify (
>  //
>  InsertTailList (&gHandleList, &Handle->AllHandl

Re: [edk2-devel] [PATCH 2/2] Allow wildcards in hostname

2021-10-12 Thread Yao, Jiewen
It seems the Bugzilla only describes the ECC, but no much info on why we need 
allow wildcards in hostname.

The git log in mu is also unclear to me - "This enables certain local network 
recovery stories. May re-evaluate as those stories change. "

I am OK with ECC change, and give R-B.

But I would like to understand more on why we need allow wildcards in general. 
What are the stories?

If this is only for "recovery stories", should we also allow wildcards in 
recovery boot path?

For example, should we have a PCD to platform owner make decision? E.g. normal 
boot - NO. recovery boot - YES ?

Thank you
Yao Jiewen



> -Original Message-
> From: Vineel Kovvuri 
> Sent: Tuesday, October 12, 2021 1:38 PM
> To: devel@edk2.groups.io; Yao, Jiewen ;
> sean.bro...@microsoft.com; bret.barke...@microsoft.com;
> michael.tur...@microsoft.com
> Cc: Vineel Kovvuri 
> Subject: [PATCH 2/2] Allow wildcards in hostname
> 
> This PR is cherry-picked from
> https://github.com/microsoft/mu_basecore/commit/d0c7733400c35722499ee
> dcd4279042a9bcb0eb4
> 
> BugZilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3679
> 
> Signed-off-by: Vineel Kovvuri 
> ---
>  NetworkPkg/HttpDxe/HttpsSupport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c
> b/NetworkPkg/HttpDxe/HttpsSupport.c
> index 7e0bf85c3c..0f28ae9447 100644
> --- a/NetworkPkg/HttpDxe/HttpsSupport.c
> +++ b/NetworkPkg/HttpDxe/HttpsSupport.c
> @@ -625,7 +625,7 @@ TlsConfigureSession (
>//
>HttpInstance->TlsConfigData.ConnectionEnd   = EfiTlsClient;
>HttpInstance->TlsConfigData.VerifyMethod= EFI_TLS_VERIFY_PEER;
> -  HttpInstance->TlsConfigData.VerifyHost.Flags=
> EFI_TLS_VERIFY_FLAG_NO_WILDCARDS;
> +  HttpInstance->TlsConfigData.VerifyHost.Flags=
> EFI_TLS_VERIFY_FLAG_NONE;
>HttpInstance->TlsConfigData.VerifyHost.HostName = HttpInstance-
> >RemoteHost;
>HttpInstance->TlsConfigData.SessionState= EfiTlsSessionNotStarted;
> 
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81848): https://edk2.groups.io/g/devel/message/81848
Mute This Topic: https://groups.io/mt/86257811/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] Reconfigure OpensslLib to add elliptic curve chipher algorithms

2021-10-12 Thread Yao, Jiewen
Reviewed-by: Jiewen Yao 


> -Original Message-
> From: Vineel Kovvuri 
> Sent: Tuesday, October 12, 2021 1:38 PM
> To: devel@edk2.groups.io; Yao, Jiewen ;
> sean.bro...@microsoft.com; bret.barke...@microsoft.com;
> michael.tur...@microsoft.com
> Cc: Vineel Kovvuri 
> Subject: [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher
> algorithms
> 
> This commit is a cherry pick of project mu's commit
> https://github.com/microsoft/mu_tiano_plus/commit/1f3b135ddc821718a78c3
> 52316197889c5d3e0c2
> 
> Reconfigure OpensslLib to add elliptic curve chipher algorithms.
> The only file manually changed is process_files.pl.
> Running the script changes the other three files.
> 
> BugZilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3679
> 
> Signed-off-by: Vineel Kovvuri 
> ---
>  .../Library/Include/openssl/opensslconf.h | 25 ++
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf   | 50 +++
>  .../Library/OpensslLib/OpensslLibCrypto.inf   | 50 +++
>  CryptoPkg/Library/OpensslLib/process_files.pl |  1 -
>  4 files changed, 105 insertions(+), 21 deletions(-)
> 
> diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h
> b/CryptoPkg/Library/Include/openssl/opensslconf.h
> index b8d59aebe8..09a6641ffc 100644
> --- a/CryptoPkg/Library/Include/openssl/opensslconf.h
> +++ b/CryptoPkg/Library/Include/openssl/opensslconf.h
> @@ -55,9 +55,6 @@ extern "C" {
>  #ifndef OPENSSL_NO_DSA
>  # define OPENSSL_NO_DSA
>  #endif
> -#ifndef OPENSSL_NO_EC
> -# define OPENSSL_NO_EC
> -#endif
>  #ifndef OPENSSL_NO_IDEA
>  # define OPENSSL_NO_IDEA
>  #endif
> @@ -88,9 +85,6 @@ extern "C" {
>  #ifndef OPENSSL_NO_SEED
>  # define OPENSSL_NO_SEED
>  #endif
> -#ifndef OPENSSL_NO_SM2
> -# define OPENSSL_NO_SM2
> -#endif
>  #ifndef OPENSSL_NO_SRP
>  # define OPENSSL_NO_SRP
>  #endif
> @@ -154,12 +148,6 @@ extern "C" {
>  #ifndef OPENSSL_NO_EC_NISTP_64_GCC_128
>  # define OPENSSL_NO_EC_NISTP_64_GCC_128
>  #endif
> -#ifndef OPENSSL_NO_ECDH
> -# define OPENSSL_NO_ECDH
> -#endif
> -#ifndef OPENSSL_NO_ECDSA
> -# define OPENSSL_NO_ECDSA
> -#endif
>  #ifndef OPENSSL_NO_EGD
>  # define OPENSSL_NO_EGD
>  #endif
> @@ -226,9 +214,6 @@ extern "C" {
>  #ifndef OPENSSL_NO_TESTS
>  # define OPENSSL_NO_TESTS
>  #endif
> -#ifndef OPENSSL_NO_TLS1_3
> -# define OPENSSL_NO_TLS1_3
> -#endif
>  #ifndef OPENSSL_NO_UBSAN
>  # define OPENSSL_NO_UBSAN
>  #endif
> @@ -265,11 +250,11 @@ extern "C" {
>  #   undef DECLARE_DEPRECATED
>  #   define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
>  #  endif
> -#elif defined(__SUNPRO_C)
> -#if (__SUNPRO_C >= 0x5130)
> -#undef DECLARE_DEPRECATED
> -#define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
> -#endif
> +# elif defined(__SUNPRO_C)
> +#  if (__SUNPRO_C >= 0x5130)
> +#   undef DECLARE_DEPRECATED
> +#   define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
> +#  endif
>  # endif
>  #endif
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index d84bde056a..bd3d9cc90f 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -199,6 +199,43 @@
>$(OPENSSL_PATH)/crypto/dso/dso_vms.c
>$(OPENSSL_PATH)/crypto/dso/dso_win32.c
>$(OPENSSL_PATH)/crypto/ebcdic.c
> +  $(OPENSSL_PATH)/crypto/ec/curve25519.c
> +  $(OPENSSL_PATH)/crypto/ec/curve448/arch_32/f_impl.c
> +  $(OPENSSL_PATH)/crypto/ec/curve448/curve448.c
> +  $(OPENSSL_PATH)/crypto/ec/curve448/curve448_tables.c
> +  $(OPENSSL_PATH)/crypto/ec/curve448/eddsa.c
> +  $(OPENSSL_PATH)/crypto/ec/curve448/f_generic.c
> +  $(OPENSSL_PATH)/crypto/ec/curve448/scalar.c
> +  $(OPENSSL_PATH)/crypto/ec/ec2_oct.c
> +  $(OPENSSL_PATH)/crypto/ec/ec2_smpl.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_ameth.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_asn1.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_check.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_curve.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_cvt.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_err.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_key.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_kmeth.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_lib.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_mult.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_oct.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_pmeth.c
> +  $(OPENSSL_PATH)/crypto/ec/ec_print.c
> +  $(OPENSSL_PATH)/crypto/ec/ecdh_kdf.c
> +  $(OPENSSL_PATH)/crypto/ec/ecdh_ossl.c
> +  $(OPENSSL_PATH)/crypto/ec/ecdsa_ossl.c
> +  $(OPENSSL_PATH)/crypto/ec/ecdsa_sign.c
> +  $(OPENSSL_PATH)/crypto/ec/ecdsa_vrf.c
> +  $(OPENSSL_PATH)/crypto/ec/eck_prn.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_mont.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_nist.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_nistp224.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_nistp256.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_nistp521.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_nistputil.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_oct.c
> +  $(OPENSSL_PATH)/crypto/ec/ecp_smpl.c
> +  $(OPENSSL_PATH)/crypto/ec/ecx_meth.c
>$(OPENSSL_PATH)/crypto/err/err.c
>$(OPENSSL_P

Re: [edk2-devel] [PATCH] UserAuthFeaturePkg/UserAuthenticationDxeSmm: The SMI to handle the user authentication should be unregister before booting to OS

2021-10-12 Thread Dandan Bi
Patch is submitted via commit 23ca68c23dd600973e961de4368abacf4db8c5c0
https://github.com/tianocore/edk2-platforms/commit/23ca68c23dd600973e961de4368abacf4db8c5c0



Thanks,
Dandan

> -Original Message-
> From: Bi, Dandan
> Sent: Friday, October 8, 2021 9:44 AM
> To: Shi, Hao ; devel@edk2.groups.io
> Cc: Liming Gao 
> Subject: RE: [PATCH] UserAuthFeaturePkg/UserAuthenticationDxeSmm: The
> SMI to handle the user authentication should be unregister before booting to 
> OS
> 
> Reviewed-by: Dandan Bi 
> 
> 
> 
> Thanks,
> Dandan
> 
> > -Original Message-
> > From: Shi, Hao 
> > Sent: Tuesday, September 28, 2021 10:09 AM
> > To: devel@edk2.groups.io
> > Cc: Shi, Hao ; Bi, Dandan ;
> > Liming Gao 
> > Subject: [PATCH] UserAuthFeaturePkg/UserAuthenticationDxeSmm: The SMI
> > to handle the user authentication should be unregister before booting
> > to OS
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3648
> >
> > Register SmmExitBootServices and SmmLegacyBoot callback function to
> > unregister this handler.
> >
> > Signed-off-by: Hao Shi 
> > Cc: Dandan Bi 
> > Cc: Liming Gao 
> > ---
> >  .../UserAuthenticationSmm.c   | 39 +--
> >  .../UserAuthenticationSmm.inf |  2 +
> >  2 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx
> > eSmm/UserAuthenticationSmm.c
> > b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx
> > eSmm/UserAuthenticationSmm.c
> > index 07e834eb..3d66010b 100644
> > ---
> > a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx
> > eSmm/UserAuthenticationSmm.c
> > +++
> > b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthentication
> > +++ DxeSmm/UserAuthenticationSmm.c
> > @@ -13,6 +13,7 @@ UINTN   mAdminPasswordTryCount = 
> > 0;
> >
> >  BOOLEAN mNeedReVerify = TRUE;
> >  BOOLEAN mPasswordVerified = FALSE;
> > +EFI_HANDLE  mSmmHandle = NULL;
> >
> >  /**
> >Verify if the password is correct.
> > @@ -612,6 +613,30 @@ EXIT:
> >return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  Performs Exit Boot Services UserAuthentication actions
> > +
> > +  @param[in] Protocol   Points to the protocol's unique identifier.
> > +  @param[in] Interface  Points to the interface instance.
> > +  @param[in] Handle The handle on which the interface was installed.
> > +
> > +  @retval EFI_SUCCESS   Notification runs successfully.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +UaExitBootServices (
> > +  IN CONST EFI_GUID *Protocol,
> > +  IN VOID   *Interface,
> > +  IN EFI_HANDLE Handle
> > +  )
> > +{
> > +  DEBUG ((DEBUG_INFO, "Unregister User Authentication Smi\n"));
> > +
> > +  gSmst->SmiHandlerUnRegister(mSmmHandle);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> >  /**
> >Main entry for this driver.
> >
> > @@ -629,10 +654,11 @@ PasswordSmmInit (
> >)
> >  {
> >EFI_STATUSStatus;
> > -  EFI_HANDLESmmHandle;
> >EDKII_VARIABLE_LOCK_PROTOCOL  *VariableLock;
> >CHAR16
> > PasswordHistoryName[sizeof(USER_AUTHENTICATION_VAR_NAME)/sizeof(
> > CHAR16) + 5];
> >UINTN Index;
> > +  EFI_EVENT ExitBootServicesEvent;
> > +  EFI_EVENT LegacyBootEvent;
> >
> >ASSERT (PASSWORD_HASH_SIZE == SHA256_DIGEST_SIZE);
> >ASSERT (PASSWORD_HISTORY_CHECK_COUNT < 0x); @@ -657,13
> > +683,20 @@ PasswordSmmInit (
> >  ASSERT_EFI_ERROR (Status);
> >}
> >
> > -  SmmHandle = NULL;
> > -  Status= gSmst->SmiHandlerRegister (SmmPasswordHandler,
> > &gUserAuthenticationGuid, &SmmHandle);
> > +  Status = gSmst->SmiHandlerRegister (SmmPasswordHandler,
> > + &gUserAuthenticationGuid, &mSmmHandle);
> >ASSERT_EFI_ERROR (Status);
> >if (EFI_ERROR (Status)) {
> >  return Status;
> >}
> >
> > +  //
> > +  // Register for SmmExitBootServices and SmmLegacyBoot notification.
> > +  //
> > +  Status = gSmst->SmmRegisterProtocolNotify
> > + (&gEdkiiSmmExitBootServicesProtocolGuid, UaExitBootServices,
> > + &ExitBootServicesEvent);  ASSERT_EFI_ERROR (Status);  Status =
> > + gSmst->SmmRegisterProtocolNotify (&gEdkiiSmmLegacyBootProtocolGuid,
> > + UaExitBootServices, &LegacyBootEvent);  ASSERT_EFI_ERROR (Status);
> > +
> >if (IsPasswordCleared()) {
> >  DEBUG ((DEBUG_INFO, "IsPasswordCleared\n"));
> >  SavePasswordToVariable (&gUserAuthenticationGuid, NULL, 0); diff
> > --git
> > a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx
> > eSmm/UserAuthenticationSmm.inf
> > b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx
> > eSmm/UserAuthenticationSmm.inf
> > index 0b33b194..d73a2fe2 100644
> > ---
> > a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDx

Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

2021-10-12 Thread Jeff Fan
OVMF did a similare change on Time Driver, please refer to 
https://github.com/tianocore/edk2/commit/239b50a863704f7960525799eda82de061c7c458
 

I do not think this will be apply for ArmPkg/TimerDxe. 

If one real issue happened on platform, it seems that interrupt was reenabled 
by reigstered timer event functions.

Jeff
 
From: Ashish Singhal via groups.io
Date: 2021-10-12 23:38
To: Marc Zyngier; Ard Biesheuvel; Shanker Donthineni
CC: edk2-devel-groups-io; Leif Lindholm; Ard Biesheuvel
Subject: Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt 
Signal
+ Shaker
From: Ashish Singhal 
Sent: Tuesday, October 12, 2021 8:56:58 AM
To: Marc Zyngier ; Ard Biesheuvel 
Cc: edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal 
 
Marc,

In the document ARM062-1010708621-30 (AArch64 Programmer's Guides Generic 
Timer), towards the end of section 3.4 it says: "When writing an interrupt 
handler for the timers, it is important that software clears the interrupt 
before deactivating the interrupt in the GIC. Otherwise, the GIC will re-signal 
the same interrupt again."

My change was in accordance with this. We only clear the interrupt when we 
update the compare value but were signaling EOI before that going against the 
guidance in the document.

Thanks
Ashish


From: Marc Zyngier 
Sent: Tuesday, October 12, 2021 2:57 AM
To: Ard Biesheuvel ; Ashish Singhal 
Cc: edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal 
 
External email: Use caution opening links or attachments


On Mon, 11 Oct 2021 23:24:38 +0100,
Ard Biesheuvel  wrote:
>
> (+ Marc)
>
> On Mon, 11 Oct 2021 at 23:40, Ashish Singhal  wrote:
> >
> > In an interrupt handler for the timers, it is important that
> > software clears the interrupt before deactivating the interrupt
> > in the GIC. Otherwise the GIC will re-signal the same interrupt
> > again.
> >
> > Signed-off-by: Ashish Singhal 
>
> Please don't spam us with almost identical versions of the same patch
> without even documenting what the difference is.
>
>
> > ---
> >  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
> > b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > index 0370620fae..46e5bbf287 100644
> > --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > @@ -300,10 +300,6 @@ TimerInterruptHandler (
> >//
> >OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> >
> > -  // Signal end of interrupt early to help avoid losing subsequent ticks
> > -  // from long duration handlers
> > -  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > -
> >// Check if the timer interrupt is active
> >if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
> >
> > @@ -335,6 +331,11 @@ TimerInterruptHandler (
> >  ArmInstructionSynchronizationBarrier ();
> >}
> >
> > +  // In an interrupt handler for the timers, it is important that software 
> > clears the interrupt
> > +  // before deactivating the interrupt in the GIC. Otherwise the GIC will 
> > re-signal the same
> > +  // interrupt again.
> > +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > +
> >gBS->RestoreTPL (OriginalTPL);
> >  }
> >

This isn't a requirement if you haven't re-enabled interrupts in
PSTATE (and the TPL being raised seems to indicate that interrupts are
actively masked here).

The timer is a level interrupt, and lowering the level takes time. If
your GIC implementation is good, it will notice that the lowering of
the level quickly, before you can reach the point where you re-enable
interrupts. If it is slow (or badly emulated if this is actually done
in a hypervisor), you'll get a spurious interrupt.

In any case, moving the EOI around doesn't make things any better. You
are just moving the goal post, without additional guarantees that the
level has been retired.

As a consequence, I don't think this patch makes much sense.

M.

--
Without deviation from the norm, progress is not possible.



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




Re: [edk2-devel] [PATCH EDK2 v1 1/1] EmbeddedPkg:Fix compiler warning

2021-10-12 Thread Abner Chang
Acked-by: Abner Chang 

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> wenyi,xie via groups.io
> Sent: Tuesday, October 12, 2021 4:16 PM
> To: devel@edk2.groups.io; l...@nuviainc.com; ardb+tianoc...@kernel.org;
> Chang, Abner (HPS SW/FW Technologist) ;
> Schaefer, Daniel 
> Cc: songdongku...@huawei.com; xiewen...@huawei.com
> Subject: [edk2-devel] [PATCH EDK2 v1 1/1] EmbeddedPkg:Fix compiler
> warning
> 
> Fixes the following compiler warning in VS2019.
> 
> edk2\EmbeddedPkg\Library\GdbSerialDebugPortLib\GdbSerialDebugPortLib
> .c(127):
> error C2220: the following warning is treated as an error
> edk2\EmbeddedPkg\Library\GdbSerialDebugPortLib\GdbSerialDebugPortLib
> .c(127):
> warning C4244: 'function': conversion from 'UINTN' to 'UINT32', possible
> loss of data
> 
> edk2\EmbeddedPkg\Library\PrePiLib\FwVol.c(347): error C2220: the
> following
> warning is treated as an error
> edk2\EmbeddedPkg\Library\PrePiLib\FwVol.c(347): warning C4244:
> 'function':
> conversion from 'UINTN' to 'UINT32', possible loss of data
> 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Abner Chang 
> Cc: Daniel Schaefer 
> Signed-off-by: Wenyi Xie 
> ---
>  EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c | 2
> +-
>  EmbeddedPkg/Library/PrePiLib/FwVol.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
> b/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
> index d2bafcf69b60..0f50a8b64191 100644
> ---
> a/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
> +++
> b/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
> @@ -18,7 +18,7 @@
> 
> 
>  EFI_DEBUGPORT_PROTOCOL  *gDebugPort = NULL;
> -UINTN   gTimeOut = 0;
> +UINT32  gTimeOut = 0;
> 
>  /**
>The constructor function initializes the UART.
> diff --git a/EmbeddedPkg/Library/PrePiLib/FwVol.c
> b/EmbeddedPkg/Library/PrePiLib/FwVol.c
> index 881506edddaf..46ea5f733f60 100644
> --- a/EmbeddedPkg/Library/PrePiLib/FwVol.c
> +++ b/EmbeddedPkg/Library/PrePiLib/FwVol.c
> @@ -298,7 +298,7 @@ FfsProcessSection (
>UINT16  SectionAttribute;
>UINT32  AuthenticationStatus;
>CHAR8   *CompressedData;
> -  UINTN   CompressedDataLength;
> +  UINT32  CompressedDataLength;
> 
> 
>*OutputBuffer = NULL;
> --
> 2.20.1.windows.1
> 
> 
> 
> 
> 



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




Re: [edk2-devel] [PATCH v2] CryptoPkg/BaseCryptLib: Eliminate extra buffer copy in Pkcs7Verify()

2021-10-12 Thread Yao, Jiewen
https://github.com/tianocore/edk2/pull/2055

pushed:
https://github.com/tianocore/edk2/commit/f22feb0e3b3f08b95201b258b104c45a2acef71f

> -Original Message-
> From: Yao, Jiewen
> Sent: Saturday, September 11, 2021 11:30 PM
> To: Bob Morgan ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Lu, XiaoyuX ;
> Jiang, Guomin 
> Subject: RE: [PATCH v2] CryptoPkg/BaseCryptLib: Eliminate extra buffer copy in
> Pkcs7Verify()
> 
> Reviewed-by: Jiewen Yao 
> 
> > -Original Message-
> > From: Bob Morgan 
> > Sent: Saturday, September 11, 2021 5:34 AM
> > To: devel@edk2.groups.io
> > Cc: Bob Morgan ; Yao, Jiewen ;
> > Wang, Jian J ; Lu, XiaoyuX ;
> > Jiang, Guomin 
> > Subject: [PATCH v2] CryptoPkg/BaseCryptLib: Eliminate extra buffer copy in
> > Pkcs7Verify()
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3617
> >
> > Create a read-only openSSL BIO wrapper for the existing input
> > buffer passed to Pkcs7Verify() instead of copying the buffer
> > into an empty writable BIO which causes memory allocations
> > within openSSL.
> >
> > Cc: Jiewen Yao 
> > Cc: Jian J Wang 
> > Cc: Xiaoyu Lu 
> > Cc: Guomin Jiang 
> > Signed-off-by: Bob Morgan 
> > ---
> >  CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c | 6 +-
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > index d99597d181..8eda98f7b2 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > @@ -864,15 +864,11 @@ Pkcs7Verify (
> >// For generic PKCS#7 handling, InData may be NULL if the content is 
> > present
> >// in PKCS#7 structure. So ignore NULL checking here.
> >//
> > -  DataBio = BIO_new (BIO_s_mem ());
> > +  DataBio = BIO_new_mem_buf (InData, (int) DataLength);
> >if (DataBio == NULL) {
> >  goto _Exit;
> >}
> >
> > -  if (BIO_write (DataBio, InData, (int) DataLength) <= 0) {
> > -goto _Exit;
> > -  }
> > -
> >//
> >// Allow partial certificate chains, terminated by a non-self-signed but
> >// still trusted intermediate certificate. Also disable time checks.
> > --
> > 2.17.1



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




[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - 10/12/2021 #cal-reminder

2021-10-12 Thread devel@edk2.groups.io Calendar
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:PUBLISH
REFRESH-INTERVAL;VALUE=DURATION:PT1H
X-PUBLISHED-TTL:PT1H
CALSCALE:GREGORIAN
BEGIN:VTIMEZONE
TZID:America/Los_Angeles
LAST-MODIFIED:20201011T015911Z
TZURL:http://tzurl.org/zoneinfo-outlook/America/Los_Angeles
X-LIC-LOCATION:America/Los_Angeles
BEGIN:DAYLIGHT
TZNAME:PDT
TZOFFSETFROM:-0800
TZOFFSETTO:-0700
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZNAME:PST
TZOFFSETFROM:-0700
TZOFFSETTO:-0800
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
X-GIOIDS:Event:1238670 
UID:mlda.1580078539586725120.r...@groups.io
DTSTAMP:20211013T011501Z
ORGANIZER;CN=Liming Gao:mailto:gaolim...@byosoft.com.cn
DTSTART:20211013T013000Z
DTEND:20211013T023000Z
SUMMARY:TianoCore Bug Triage - APAC / NAMO
DESCRIPTION:TianoCore Bug Triage - APAC / NAMO\n\nHosted by Liming Gao\n\
 n
 \n\nMicrosoft Teams meeting\n\n*Join on your computer or mobile a
 pp*\n\nClick here to join the meeting ( https://teams.microsoft.com/l/mee
 tup-join/19%3ameeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40
 thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255
 d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d )\n\n*Jo
 in with a video conferencing device*\n\nte...@conf.intel.com\n\nVideo Con
 ference ID: 116 062 094 0\n\nAlternate VTC dialing instructions ( https:/
 /conf.intel.com/teams/?conf=1160620940&ivr=teams&d=conf.intel.com&test=te
 st_call )\n\n*Or call in (audio only)*\n\n+1 916-245-6934\,\,77463821# ( 
 tel:+19162456934\,\,77463821# ) United States\, Sacramento\n\nPhone Confe
 rence ID: 774 638 21#\n\nFind a local number ( https://dialin.teams.micro
 soft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821 ) | Reset PIN (
  https://mysettings.lync.com/pstnconferencing )\n\nLearn More ( https://a
 ka.ms/JoinTeamsMeeting ) | Meeting options ( https://teams.microsoft.com/
 meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e&tenantId
 =46c98d88-e344-4ed4-8496-4ed7712e255d&threadId=19_meeting_OTUyZTg2NjgtNDh
 lNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2&messageId=0&language=en-US )
LOCATION:https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTUyZTg2N
 jgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40thread.v2/0?context=%7b%22Tid%2
 2%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-
 1218-4db3-bfc9-3d4c5aa7669e%22%7d
SEQUENCE:1
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics


[edk2-devel] 回复: [PATCH v9 1/1] MdePkg/BaseLib: Add QuickSort function on BaseLib

2021-10-12 Thread gaoliming
I have no other comments. Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Kuo, IanX 
> 发送时间: 2021年10月12日 11:51
> 收件人: Ni, Ray ; devel@edk2.groups.io; Kinney, Michael
> D ; Liming Gao 
> 抄送: Chan, Amy ; Liu, Zhiguang
> 
> 主题: RE: [PATCH v9 1/1] MdePkg/BaseLib: Add QuickSort function on
> BaseLib
> 
> @Liming Gao and @Kinney, Michael D
> 
> May I get one of yours help for the reviewed from MdePkg maintainer side ?
> Have any concern, please also share for me.
> 
> Thanks,
> Ian Kuo
> 
> -Original Message-
> From: Ni, Ray 
> Sent: Tuesday, October 12, 2021 10:22 AM
> To: Kuo, IanX ; devel@edk2.groups.io
> Cc: Chan, Amy ; Kinney, Michael D
> ; Liming Gao ; Liu,
> Zhiguang 
> Subject: RE: [PATCH v9 1/1] MdePkg/BaseLib: Add QuickSort function on
> BaseLib
> 
> Reviewed-by: Ray Ni 
> 
> Ian, please take the approval from maintainers of MdePkg as the formal
> approval.
> 
> Thanks,
> Ray
> 
> > -Original Message-
> > From: Kuo, IanX 
> > Sent: Tuesday, October 12, 2021 8:06 AM
> > To: devel@edk2.groups.io
> > Cc: Chan, Amy ; Ni, Ray ; Kuo,
> > IanX ; Kinney, Michael D
> > ; Liming Gao ;
> > Liu, Zhiguang 
> > Subject: [PATCH v9 1/1] MdePkg/BaseLib: Add QuickSort function on
> > BaseLib
> >
> > From: IanX Kuo 
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3675
> >
> > Add QuickSort function into BaseLib
> >
> > Cc: Ray Ni 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Signed-off-by: IanX Kuo 
> > ---
> >  MdePkg/Include/Library/BaseLib.h  |  49 
> >  MdePkg/Library/BaseLib/BaseLib.inf|   1 +
> >  MdePkg/Library/BaseLib/QuickSort.c| 116
> ++
> >  .../Library/BaseLib/UnitTestHostBaseLib.inf   |   3 +-
> >  4 files changed, 168 insertions(+), 1 deletion(-)  create mode 100644
> > MdePkg/Library/BaseLib/QuickSort.c
> >
> > diff --git a/MdePkg/Include/Library/BaseLib.h
> > b/MdePkg/Include/Library/BaseLib.h
> > index 2452c1d92e..0ae0f4e6af 100644
> > --- a/MdePkg/Include/Library/BaseLib.h
> > +++ b/MdePkg/Include/Library/BaseLib.h
> > @@ -2856,6 +2856,55 @@ RemoveEntryList (  //
> >
> >  // Math Services
> >
> >  //
> >
> > +/**
> >
> > +  Prototype for comparison function for any two element types.
> >
> > +
> >
> > +  @param[in] Buffer1  The pointer to first buffer.
> >
> > +  @param[in] Buffer2  The pointer to second buffer.
> >
> > +
> >
> > +  @retval 0   Buffer1 equal to Buffer2.
> >
> > +  @return <0  Buffer1 is less than Buffer2.
> >
> > +  @return >0  Buffer1 is greater than
> Buffer2.
> >
> > +**/
> >
> > +typedef
> >
> > +INTN
> >
> > +(EFIAPI *BASE_SORT_COMPARE)(
> >
> > +  IN CONST VOID *Buffer1,
> >
> > +  IN CONST VOID *Buffer2
> >
> > +  );
> >
> > +
> >
> > +/**
> >
> > +  This function is identical to perform QuickSort,
> >
> > +  except that is uses the pre-allocated buffer so the in place
> > + sorting does not need to
> >
> > +  allocate and free buffers constantly.
> >
> > +
> >
> > +  Each element must be equal sized.
> >
> > +
> >
> > +  if BufferToSort is NULL, then ASSERT.
> >
> > +  if CompareFunction is NULL, then ASSERT.
> >
> > +  if BufferOneElement is NULL, then ASSERT.
> >
> > +  if ElementSize is < 1, then ASSERT.
> >
> > +
> >
> > +  if Count is < 2 then perform no action.
> >
> > +
> >
> > +  @param[in, out] BufferToSort   on call a Buffer of (possibly sorted)
> elements
> >
> > + on return a buffer of sorted
> > + elements
> >
> > +  @param[in] Count   the number of elements in the
> buffer to sort
> >
> > +  @param[in] ElementSize Size of an element in bytes
> >
> > +  @param[in] CompareFunction The function to call to perform the
> comparison
> >
> > + of any 2 elements
> >
> > +  @param[out] BufferOneElement   Caller provided buffer whose size
> equals to ElementSize.
> >
> > + It's used by QuickSort() for
> swapping in sorting.
> >
> > +**/
> >
> > +VOID
> >
> > +EFIAPI
> >
> > +QuickSort (
> >
> > +  IN OUT VOID   *BufferToSort,
> >
> > +  IN CONST UINTNCount,
> >
> > +  IN CONST UINTNElementSize,
> >
> > +  IN   BASE_SORT_COMPARECompareFunction,
> >
> > +  OUT VOID  *BufferOneElement
> >
> > +  );
> >
> >
> >
> >  /**
> >
> >Shifts a 64-bit integer left between 0 and 63 bits. The low bits
> > are filled
> >
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> > b/MdePkg/Library/BaseLib/BaseLib.inf
> > index 6efa5315b6..cebda3b210 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -32,6 +32,7 @@
> >SwapBytes16.c
> >
> >LongJump.c
> >
> >SetJump.c
> >
> > +  QuickSort.c
> >
> >RShiftU64.c
> >
> >RRotU64.c
> >
> >RRotU3

Re: [edk2-devel] [PATCH V9 2/4] OvmfPkg: Clear WORK_AREA_GUEST_TYPE in Main.asm

2021-10-12 Thread Min Xu
On October 12, 2021 9:23 PM, Lendacky Thomas wrote:
> On 10/11/21 9:37 PM, Min Xu wrote:
> > diff --git a/OvmfPkg/ResetVector/Main.asm
> > b/OvmfPkg/ResetVector/Main.asm index ae90a148fce7..a501fbe880f2
> 100644
> > --- a/OvmfPkg/ResetVector/Main.asm
> > +++ b/OvmfPkg/ResetVector/Main.asm
> > @@ -36,6 +36,14 @@ Main16:
> >
> >   BITS32
> >
> > +%ifdef ARCH_X64
> 
> A regular SEV guest can be built in the hybrid IA32 and X64 configuration, so 
> this
> will break existing SEV firmwares built in that manner. Only SEV-ES and 
> SEV-SNP
> require the full X64 confguration.
> 
WORK_AREA_GUEST_TYPE is defined in 
https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/ResetVector.nasmb#L75
 and previously it was cleared in 
https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia32/PageTables64.asm#L47.
 These 2 lines are both surrounded by ARCH_X64.
So in this commit, the clearance of WORK_AREA_GUEST_TYPE in Main.asm is 
surrounded by ARCH_X64 too.
Brijesh, what's your comments on this change?

Thanks!
Min


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




Re: [edk2-devel] [PATCH V9 1/4] OvmfPkg: Copy Main.asm from UefiCpuPkg to OvmfPkg's ResetVector

2021-10-12 Thread Min Xu
On October 12, 2021 3:02 PM, Gerd Hoffmann wrote:
> On Tue, Oct 12, 2021 at 10:37:47AM +0800, Min Xu wrote:
> > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> >
> > Previously OvmfPkg/ResetVector uses the Main.asm in
> > UefiCpuPkg/ReseteVector/Vtf0. In this Main.asm there is only Main16
> > entry point.
> >
> > This patch-set is to introduce Intel TDX into Ovmf. Main32 entry point
> > is needed in Main.asm by Intel TDX. To reduce the complexity of
> > Main.asm in UefiCpuPkg, OvmfPkg create its own Main.asm to meet the
> > requirement of Intel TDX.
> >
> > UefiCpuPkg/ResetVector/Vtf0/main.asm ->
> OvmfPkg/ResetVector/Main.asm
> 
> You should mention that this is an unmodified copy (so no functional
> change) and the actual changes for tdx come as incremental patches.
>
Thanks for reminder. It will be updated in next version.
> 
> With that:
> Acked-by: Gerd Hoffmann 
> 
Thanks!
Min


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




Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

2021-10-12 Thread Ashish Singhal via groups.io




From: Ashish Singhal 
Sent: Tuesday, October 12, 2021 10:32 AM
To: Marc Zyngier 
Cc: Shanker Donthineni ; Ard Biesheuvel 
; edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal 
 



From: Marc Zyngier 
Sent: Tuesday, October 12, 2021 10:27 AM
To: Ashish Singhal 
Cc: Shanker Donthineni ; Ard Biesheuvel 
; edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal 
 
External email: Use caution opening links or attachments


On Tue, 12 Oct 2021 17:11:36 +0100,
Ashish Singhal  wrote:
>
> Marc,
>
> What do you suggest should be the proper fix for getting timer
> interrupts even when ISTATUS bit is not set? Should we ignore them
> the way it is in current implementation? I am OK to file a bug for
> this if you think that is a better way to discuss this.

I don't think there is anything to fix.

Yes, the order in EDKII is odd. No, changing the order doesn't give
any extra guarantee. Spurious interrupts can always happen. Broken (or
slow) HW and bad emulation are more susceptible to it.

Now, how often do you see that? On which HW?

    M.

--
Without deviation from the norm, progress is not possible.

Marc,

We see at least one spurious interrupt after every valid timer interrupt. While 
both valid and spurious interrupt has the correct source, spurious interrupt 
does not have ISTATUS bit set. We are seeing this on Silicon and not on the 
emulation platform. Delaying EOI signal to GIC does take the spurious interrupt 
out as with the new flow we clear the interrupt before signaling EOI so that 
next time only a valid interrupt can be triggered and not the old interrupt 
which was still not cleared while signaling EOI to GIC.

Thanks
Ashish

Thanks
Ashish

Marc,

I can confirm that with the current code on edk2, we get 1 spurious interrupt 
for every 1 valid interrupt from GIC. With the change I proposed, we do not get 
the spurious interrupt at all.

Thanks
Ashish

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




[edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set Action for failed signed image

2021-10-12 Thread Joseph Hemann
If the image is signed but not allowed by DB and the hash of
image is not found in DB/DBX, then the EFI_IMAGE_INFO_ACTION
of the load of said image should be set to,
EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Joseph Hemann 
Change-Id: I6b2777bd7aeb57773b8876e44c2179ea7501bc8c
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index c48861cd6496..0a804af2162f 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1957,6 +1957,7 @@ DxeImageVerificationHandler (
   if (!EFI_ERROR (DbStatus) && IsFound) {
 IsVerified = TRUE;
   } else {
+Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
 DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but 
signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", 
mHashTypeStr));
   }
 }
-- 
2.17.1



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




[edk2-devel] [PATCH 1/1] SecurityPkg/DxeImageVerificationLib: Set Action for failed unsigned image

2021-10-12 Thread Joseph Hemann
If the image is not signed and the hash of image is not found
in DB/DBX, then the EFI_IMAGE_INFO_ACTION of the load of said
image should be set to,
EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Joseph Hemann 
Change-Id: Ia432ebf4ec811e36d67b80bc438a6aff60bc9b67
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 0a804af2162f..e5fae732bb1f 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1848,6 +1848,7 @@ DxeImageVerificationHandler (
 //
 // Image Hash is not found in both forbidden and allowed database.
 //
+Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
 DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s 
hash of image is not found in DB/DBX.\n", mHashTypeStr));
 goto Failed;
   }
-- 
2.17.1



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




Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

2021-10-12 Thread Ashish Singhal via groups.io




From: Marc Zyngier 
Sent: Tuesday, October 12, 2021 10:27 AM
To: Ashish Singhal 
Cc: Shanker Donthineni ; Ard Biesheuvel 
; edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal 
 
External email: Use caution opening links or attachments


On Tue, 12 Oct 2021 17:11:36 +0100,
Ashish Singhal  wrote:
>
> Marc,
>
> What do you suggest should be the proper fix for getting timer
> interrupts even when ISTATUS bit is not set? Should we ignore them
> the way it is in current implementation? I am OK to file a bug for
> this if you think that is a better way to discuss this.

I don't think there is anything to fix.

Yes, the order in EDKII is odd. No, changing the order doesn't give
any extra guarantee. Spurious interrupts can always happen. Broken (or
slow) HW and bad emulation are more susceptible to it.

Now, how often do you see that? On which HW?

    M.

--
Without deviation from the norm, progress is not possible.

Marc,

We see at least one spurious interrupt after every valid timer interrupt. While 
both valid and spurious interrupt has the correct source, spurious interrupt 
does not have ISTATUS bit set. We are seeing this on Silicon and not on the 
emulation platform. Delaying EOI signal to GIC does take the spurious interrupt 
out as with the new flow we clear the interrupt before signaling EOI so that 
next time only a valid interrupt can be triggered and not the old interrupt 
which was still not cleared while signaling EOI to GIC.

Thanks
Ashish

Thanks
Ashish

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




Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

2021-10-12 Thread Ashish Singhal via groups.io




From: Marc Zyngier 
Sent: Tuesday, October 12, 2021 9:44 AM
To: Ashish Singhal 
Cc: Ard Biesheuvel ; edk2-devel-groups-io 
; Leif Lindholm ; Ard Biesheuvel 

Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal 
 
External email: Use caution opening links or attachments


Ashish,

Please don't top post, and please use plain text.

On Tue, 12 Oct 2021 15:56:58 +0100,
Ashish Singhal  wrote:
>
> Marc,
>
> In the document ARM062-1010708621-30 (AArch64 Programmer's Guides
> Generic Timer), towards the end of section 3.4 it says: "When
> writing an interrupt handler for the timers, it is important that
> software clears the interrupt before deactivating the interrupt in
> the GIC. Otherwise, the GIC will re-signal the same interrupt
> again."

This document is a waste of valuable bits, unfortunately, and isn't an
architecture reference.

> My change was in accordance with this. We only clear the interrupt
> when we update the compare value but were signaling EOI before that
> going against the guidance in the document.

There is no such requirement in the GIC architecture, as it makes no
guarantee on how much time it takes for a change of level to be
observed. Given that, this change is pretty much immaterial.

    M.

--
Without deviation from the norm, progress is not possible.

Marc,

What do you suggest should be the proper fix for getting timer interrupts even 
when ISTATUS bit is not set? Should we ignore them the way it is in current 
implementation? I am OK to file a bug for this if you think that is a better 
way to discuss this.

Shanker,

Any thoughts on this?

Thanks
Ashish

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




Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

2021-10-12 Thread Ashish Singhal via groups.io
+ Shaker

Get Outlook for iOS

From: Ashish Singhal 
Sent: Tuesday, October 12, 2021 8:56:58 AM
To: Marc Zyngier ; Ard Biesheuvel 
Cc: edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

Marc,

In the document ARM062-1010708621-30 (AArch64 Programmer's Guides Generic 
Timer), towards the end of section 3.4 it says: "When writing an interrupt 
handler for the timers, it is important that software clears the interrupt 
before deactivating the interrupt in the GIC. Otherwise, the GIC will re-signal 
the same interrupt again."

My change was in accordance with this. We only clear the interrupt when we 
update the compare value but were signaling EOI before that going against the 
guidance in the document.

Thanks
Ashish

From: Marc Zyngier 
Sent: Tuesday, October 12, 2021 2:57 AM
To: Ard Biesheuvel ; Ashish Singhal 
Cc: edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

External email: Use caution opening links or attachments


On Mon, 11 Oct 2021 23:24:38 +0100,
Ard Biesheuvel  wrote:
>
> (+ Marc)
>
> On Mon, 11 Oct 2021 at 23:40, Ashish Singhal  wrote:
> >
> > In an interrupt handler for the timers, it is important that
> > software clears the interrupt before deactivating the interrupt
> > in the GIC. Otherwise the GIC will re-signal the same interrupt
> > again.
> >
> > Signed-off-by: Ashish Singhal 
>
> Please don't spam us with almost identical versions of the same patch
> without even documenting what the difference is.
>
>
> > ---
> >  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
> > b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > index 0370620fae..46e5bbf287 100644
> > --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > @@ -300,10 +300,6 @@ TimerInterruptHandler (
> >//
> >OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> >
> > -  // Signal end of interrupt early to help avoid losing subsequent ticks
> > -  // from long duration handlers
> > -  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > -
> >// Check if the timer interrupt is active
> >if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
> >
> > @@ -335,6 +331,11 @@ TimerInterruptHandler (
> >  ArmInstructionSynchronizationBarrier ();
> >}
> >
> > +  // In an interrupt handler for the timers, it is important that software 
> > clears the interrupt
> > +  // before deactivating the interrupt in the GIC. Otherwise the GIC will 
> > re-signal the same
> > +  // interrupt again.
> > +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > +
> >gBS->RestoreTPL (OriginalTPL);
> >  }
> >

This isn't a requirement if you haven't re-enabled interrupts in
PSTATE (and the TPL being raised seems to indicate that interrupts are
actively masked here).

The timer is a level interrupt, and lowering the level takes time. If
your GIC implementation is good, it will notice that the lowering of
the level quickly, before you can reach the point where you re-enable
interrupts. If it is slow (or badly emulated if this is actually done
in a hypervisor), you'll get a spurious interrupt.

In any case, moving the EOI around doesn't make things any better. You
are just moving the goal post, without additional guarantees that the
level has been retired.

As a consequence, I don't think this patch makes much sense.

M.

--
Without deviation from the norm, progress is not possible.


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




Re: [edk2-devel] [PATCH V2 0/3] Introduce TdProtocol into EDK2

2021-10-12 Thread Sami Mujawar
Hi Min,

Thank you for this patch.

I think it would greatly help if the EFI_TD_PROTOCOL is changed to something 
more architecture neutral. As I understand, this patch series is removing the 
dependency on TPM for measurement and is instead providing a lightweight 
interface for extending measurements for Confidential Compute Architecture 
(CCA) guests.

Considering this, it would be good to generalise EFI_TD_PROTOCOL as a 
Confidential Compute Architecture Measurement (CCAM) protocol.
In fact, your v2 series demonstrates this need with the introduction of 
MEASURE_BOOT_PROTOCOLS in "[PATCH V2 2/3] SecurityPkg: Support TdProtocol in 
DxeTpm2MeasureBootLib [https://edk2.groups.io/g/devel/message/81651]";.

As it stands, I feel most of the code can be reused/common.  Some interfaces 
may need to use an architecture specific library, and some configuration 
options would need to be defined using PCDs.

Kindly let me know your thoughts.

Regards,

Sami Mujawar

On 08/10/2021, 06:24, "devel@edk2.groups.io on behalf of Min Xu via groups.io" 
 wrote:

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

If TD-Guest firmware supports measurement and an event is created,
TD-Guest firmware is designed to report the event log with the same data
structure in TCG-Platform-Firmware-Profile specification with
EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.

The TD-Guest firmware supports measurement, the TD Guest Firmware is
designed to produce EFI_TD_PROTOCOL with new GUID EFI_TD_PROTOCOL_GUID
to report event log and provides hash capability.

https://software.intel.com/content/dam/develop/external/us/en/documents/
intel-tdx-guest-hypervisor-communication-interface-1.0-344426-002.pdf
Section 4.3.2 includes the EFI_TD_PROTOCOL.

Patch #1:
Introduce the TD Protocol definition into MdePkg

Patch #2:
Update DxeTpm2MeasureBootLib to support TD based measure boot.

Patch #3:
Update DxeTpmMeasurementLib to support TD based measurement.

Code is at https://github.com/mxu9/edk2/tree/td_protocol.v2

v2 changes:
 - TD based measure boot is implemented in DxeTpm2MeasureBootLib.
   This minimize the code changes.
 - TD based measurement is added. It is implemented in
   DxeTpmMeasurementLib.
 - Fix the typo in comments.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Ken Lu 
Signed-off-by: Min Xu 

Min Xu (3):
  MdePkg: Introduce TdProtocol for TD-Guest firmware
  SecurityPkg: Support TdProtocol in DxeTpm2MeasureBootLib
  SecurityPkg: Support TdProtocol in DxeTpmMeasurementLib

 MdePkg/Include/Protocol/TdProtocol.h  | 305 +++
 MdePkg/MdePkg.dec |   3 +
 .../DxeTpm2MeasureBootLib.c   | 346 ++
 .../DxeTpm2MeasureBootLib.inf |   1 +
 .../DxeTpmMeasurementLib.c|  87 -
 .../DxeTpmMeasurementLib.inf  |   1 +
 6 files changed, 672 insertions(+), 71 deletions(-)
 create mode 100644 MdePkg/Include/Protocol/TdProtocol.h

-- 
2.29.2.windows.2









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




Re: [edk2-devel] [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

2021-10-12 Thread Ashish Singhal via groups.io
Marc,

In the document ARM062-1010708621-30 (AArch64 Programmer's Guides Generic 
Timer), towards the end of section 3.4 it says: "When writing an interrupt 
handler for the timers, it is important that software clears the interrupt 
before deactivating the interrupt in the GIC. Otherwise, the GIC will re-signal 
the same interrupt again."

My change was in accordance with this. We only clear the interrupt when we 
update the compare value but were signaling EOI before that going against the 
guidance in the document.

Thanks
Ashish

From: Marc Zyngier 
Sent: Tuesday, October 12, 2021 2:57 AM
To: Ard Biesheuvel ; Ashish Singhal 
Cc: edk2-devel-groups-io ; Leif Lindholm 
; Ard Biesheuvel 
Subject: Re: [PATCH v2] ArmPkg/TimerDxe: Delay End Of Interrupt Signal

External email: Use caution opening links or attachments


On Mon, 11 Oct 2021 23:24:38 +0100,
Ard Biesheuvel  wrote:
>
> (+ Marc)
>
> On Mon, 11 Oct 2021 at 23:40, Ashish Singhal  wrote:
> >
> > In an interrupt handler for the timers, it is important that
> > software clears the interrupt before deactivating the interrupt
> > in the GIC. Otherwise the GIC will re-signal the same interrupt
> > again.
> >
> > Signed-off-by: Ashish Singhal 
>
> Please don't spam us with almost identical versions of the same patch
> without even documenting what the difference is.
>
>
> > ---
> >  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
> > b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > index 0370620fae..46e5bbf287 100644
> > --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> > @@ -300,10 +300,6 @@ TimerInterruptHandler (
> >//
> >OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> >
> > -  // Signal end of interrupt early to help avoid losing subsequent ticks
> > -  // from long duration handlers
> > -  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > -
> >// Check if the timer interrupt is active
> >if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
> >
> > @@ -335,6 +331,11 @@ TimerInterruptHandler (
> >  ArmInstructionSynchronizationBarrier ();
> >}
> >
> > +  // In an interrupt handler for the timers, it is important that software 
> > clears the interrupt
> > +  // before deactivating the interrupt in the GIC. Otherwise the GIC will 
> > re-signal the same
> > +  // interrupt again.
> > +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> > +
> >gBS->RestoreTPL (OriginalTPL);
> >  }
> >

This isn't a requirement if you haven't re-enabled interrupts in
PSTATE (and the TPL being raised seems to indicate that interrupts are
actively masked here).

The timer is a level interrupt, and lowering the level takes time. If
your GIC implementation is good, it will notice that the lowering of
the level quickly, before you can reach the point where you re-enable
interrupts. If it is slow (or badly emulated if this is actually done
in a hypervisor), you'll get a spurious interrupt.

In any case, moving the EOI around doesn't make things any better. You
are just moving the goal post, without additional guarantees that the
level has been retired.

As a consequence, I don't think this patch makes much sense.

M.

--
Without deviation from the norm, progress is not possible.


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




Re: [edk2-devel] [PATCH V9 2/4] OvmfPkg: Clear WORK_AREA_GUEST_TYPE in Main.asm

2021-10-12 Thread Lendacky, Thomas via groups.io

On 10/11/21 9:37 PM, Min Xu wrote:

RFC: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3429&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cc4c4ac9654a940ada92308d98d2994d0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637696032012206979%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=1SVRKXztfFcaVVer1AYOhLIhs6sVW%2BwtYQNxuuHHbTE%3D&reserved=0

Previously WORK_AREA_GUEST_TYPE was cleared in SetCr3ForPageTables64.
This is workable for Legacy guest and SEV guest. But it doesn't work
after Intel TDX is introduced. It is because all TDX CPUs (BSP and APs)
start to run from 0xfff0, thus WORK_AREA_GUEST_TYPE will be cleared
multi-times if it is TDX guest. So the clearance of WORK_AREA_GUEST_TYPE
is moved to Main16 entry point in Main.asm.
Note: WORK_AREA_GUEST_TYPE is only defined for ARCH_X64.

For Intel TDX, its corresponding entry point is Main32 (which will be
introduced in next commit in this patch-set). WORK_AREA_GUEST_TYPE will
be cleared there.

Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Brijesh Singh 
Cc: Erdem Aktas 
Cc: James Bottomley 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Signed-off-by: Min Xu 
---
  OvmfPkg/ResetVector/Ia32/PageTables64.asm | 4 
  OvmfPkg/ResetVector/Main.asm  | 8 
  2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 07b6ca070909..02528221e560 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -42,10 +42,6 @@ BITS32
  ;
  SetCr3ForPageTables64:
  
-; Clear the WorkArea header. The SEV probe routines will populate the

-; work area when detected.
-mov byte[WORK_AREA_GUEST_TYPE], 0
-
  ; Check whether the SEV is active and populate the SevEsWorkArea
  OneTimeCall   CheckSevFeatures
  
diff --git a/OvmfPkg/ResetVector/Main.asm b/OvmfPkg/ResetVector/Main.asm

index ae90a148fce7..a501fbe880f2 100644
--- a/OvmfPkg/ResetVector/Main.asm
+++ b/OvmfPkg/ResetVector/Main.asm
@@ -36,6 +36,14 @@ Main16:
  
  BITS32
  
+%ifdef ARCH_X64


A regular SEV guest can be built in the hybrid IA32 and X64 configuration, 
so this will break existing SEV firmwares built in that manner. Only 
SEV-ES and SEV-SNP require the full X64 confguration.


Thanks,
Tom


+
+; Clear the WorkArea header. The SEV probe routines will populate the
+; work area when detected.
+mov byte[WORK_AREA_GUEST_TYPE], 0
+
+%endif
+
  ;
  ; Search for the Boot Firmware Volume (BFV)
  ;




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




[edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Set Action for failed signed image

2021-10-12 Thread Joseph Hemann
From: Joseph Hemann 

If the image is signed but not allowed by DB and the hash of
image is not found in DB/DBX, then the EFI_IMAGE_INFO_ACTION
of the load of said image should be set to,
EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND, rather then being left
unset as EFI_IMAGE_EXECUTION_AUTH_UNTESTED.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Joseph Hemann 
Change-Id: I271fb61384e5b8bb5ac23ccba5de9ba77adb85ad
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index c48861cd64..0a804af216 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1957,6 +1957,7 @@ DxeImageVerificationHandler (
   if (!EFI_ERROR (DbStatus) && IsFound) {
 IsVerified = TRUE;
   } else {
+Action = EFI_IMAGE_EXECUTION_AUTH_SIG_NOT_FOUND;
 DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but 
signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", 
mHashTypeStr));
   }
 }
-- 
2.17.1



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




Re: [edk2-devel] [PATCH V2 28/28] OvmfPkg: Add LocalApicTimerDxe

2021-10-12 Thread Gerd Hoffmann
On Tue, Oct 05, 2021 at 11:39:39AM +0800, Min Xu wrote:
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> 
> TDX guest supports LocalApicTimer. But in current OvmfPkg the supported
> timer is 8254TimerDxe. So gUefiOvmfPkgTokenSpaceGuid.PcdTimerSelector
> is introduced to select the running Timer. The Timer driver will check
> the TimerSelector in its entry point. The default Timer is 8254.

Hmm.

We already have a local apic timer implementation (XenTimerDxe).  Works
fine with kvm, microvm already uses that.  See commit 76602f45dcd9
("OvmfPkg/Microvm: use XenTimerDxe (lapic timer)").

So, first I'd suggest to just use that (maybe rename the thing to avoid
confusion as it isn't really Xen specific).

Next question is whenever there is a need for a runtime switch.  I doubt
it is possible to create a virtual machine without lapic, so switching
ovmf from 8254 (aka pit) to lapic unconditionally should work fine.
Quick smoke test (patch below) shows no obvious problems.

take care,
  Gerd

>From 948aaf555a864da1d6f2ccc8dddc7e22cefb76e1 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Tue, 12 Oct 2021 14:58:18 +0200
Subject: [PATCH 1/1] OvmfPkgX64: use lapic timer

---
 OvmfPkg/OvmfPkgX64.dsc | 4 ++--
 OvmfPkg/OvmfPkgX64.fdf | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 52f7598cf1c7..687aae6e3e68 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -620,6 +620,7 @@ [PcdsDynamicDefault]
   gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size|0x8
 !endif
 
+  gEfiMdePkgTokenSpaceGuid.PcdFSBClock|10
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
 
   # Set video resolution for text setup.
@@ -765,10 +766,9 @@ [Components]
   }
 
   MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-  OvmfPkg/8259InterruptControllerDxe/8259.inf
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
-  OvmfPkg/8254TimerDxe/8254Timer.inf
+  OvmfPkg/XenTimerDxe/XenTimerDxe.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index b6cc3cabdd69..9ca723dc8604 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -232,10 +232,9 @@ [FV.DXEFV]
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
-INF  OvmfPkg/8259InterruptControllerDxe/8259.inf
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
-INF  OvmfPkg/8254TimerDxe/8254Timer.inf
+INF  OvmfPkg/XenTimerDxe/XenTimerDxe.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
-- 
2.31.1



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




Re: [edk2-devel] [PATCH V2 27/28] OvmfPkg: Update IoMmuDxe to support TDX

2021-10-12 Thread Gerd Hoffmann
  Hi,

> +#define IO_MMU_LEGACY  0x0
> +#define IO_MMU_SEV 0x01
> +#define IO_MMU_TDX 0x02
> +
> +UINTN mIoMmuType = IO_MMU_LEGACY;

Yet another place where you should be able to just use the
ConfidentialComputing PCD.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V2 25/28] OvmfPkg/BaseMemEncryptTdxLib: Add TDX helper library

2021-10-12 Thread Gerd Hoffmann
> +/**
> +  Initialize a buffer pool for page table use only.
> +
> +  To reduce the potential split operation on page table, the pages reserved 
> for
> +  page table should be allocated in the times of PAGE_TABLE_POOL_UNIT_PAGES 
> and
> +  at the boundary of PAGE_TABLE_POOL_ALIGNMENT. So the page pool is always
> +  initialized with number of pages greater than or equal to the given
> +  PoolPages.

Hmm, doesn't have tianocore a lib for such allocation pools?

> +  //
> +  // Link all pools into a list for easier track later.
> +  //

Seems to be never used.

> +/**
> + Disable Write Protect on pages marked as read-only.
> +**/
> +STATIC
> +VOID
> +DisableReadOnlyPageWriteProtect (

Why this is needed?

> +typedef union {
> +  struct {
> +UINT64  Present:1;// 0 = Not present in memory,
> +  //   1 = Present in memory
> +UINT64  ReadWrite:1;  // 0 = Read-Only, 1= Read/Write
> +UINT64  UserSupervisor:1; // 0 = Supervisor, 1=User
> +UINT64  WriteThrough:1;   // 0 = Write-Back caching,
> +  //   1 = Write-Through caching
> +UINT64  CacheDisabled:1;  // 0 = Cached, 1=Non-Cached
> +UINT64  Accessed:1;   // 0 = Not accessed,
> +  //   1 = Accessed (set by CPU)
> +UINT64  Reserved:1;   // Reserved
> +UINT64  MustBeZero:2; // Must Be Zero
> +UINT64  Available:3;  // Available for use by system software
> +UINT64  PageTableBaseAddress:40;  // Page Table Base Address
> +UINT64  AvabilableHigh:11;// Available for use by system software
> +UINT64  Nx:1; // No Execute bit
> +  } Bits;
> +  UINT64Uint64;
> +} PAGE_MAP_AND_DIRECTORY_POINTER;

No need to add that, there is OvmfPkg/Include/IndustryStandard/PageTable.h

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V2 24/28] OvmfPkg: Add TdxDxe driver

2021-10-12 Thread Gerd Hoffmann
  Hi,

> Besides above features, TdxDxe driver will update the ACPI MADT
> Mutiprocessor Wakeup Table.

> +  ACPI_MADT_MPWK_STRUCT   *MadtMpWk;

> +  NewBufferSize = 1 * sizeof (*Madt) +
> +  CpuCount  * sizeof (*LocalApic) +
> +  1 * sizeof (*IoApic) +
> +  NUM_8259_IRQS * sizeof (*Iso) +
> +  1 * sizeof (*LocalApicNmi);

+ sizeof(MadtMpWk)

> +  CopyMem (&(Madt->Header), AcpiTableBuffer, sizeof 
> (EFI_ACPI_DESCRIPTION_HEADER));
> +  Madt->Header.Length= (UINT32) NewBufferSize;
> +  Madt->LocalApicAddress = PcdGet32 (PcdCpuLocalApicBaseAddress);
> +  Madt->Flags= EFI_ACPI_1_0_PCAT_COMPAT;
> +  Ptr = Madt + 1;

[ ... ]

You are not updating the MADT.  You create a new one from scratch.  Not
a good plan.  I think you should simply get the installed table, copy it
to a larger buffer and append the ACPI_MADT_MPWK_STRUCT to that.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V2 14/28] OvmfPkg: Update SecEntry.nasm to support Tdx

2021-10-12 Thread Gerd Hoffmann
  Hi,

>  - AcceptPages:
>To mitigate the performance impact of accepting pages in SEC phase on
>BSP, BSP will parse memory resources and assign each AP the task of
>accepting a subset of pages. This command may be called several times
>until all memory resources are processed. In accepting pages, PageLevel
>may fall back to smaller one if SIZE_MISMATCH error is returned.

Hmm, I'm wondering whenever it is useful to have this in the first place
with the longer-term plan to implement lazy on-demand acceptance.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V2 13/28] UefiCpuPkg: Enable Tdx support in MpInitLib

2021-10-12 Thread Gerd Hoffmann
  Hi,

> +  do {
> +AsmCpuid (0, &LargestEax, &Ebx, &Ecx, &Edx);

Again: this should use PCD.

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V2 12/28] UefiCpuPkg/CpuExceptionHandler: Add base support for the #VE exception

2021-10-12 Thread Gerd Hoffmann
  Hi,

> +  if (ExceptionType == VE_EXCEPTION) {
> +EFI_STATUS  Status;
> +//
> +// #VE needs to be handled immediately upon enabling exception handling
> +// and therefore can't use the RegisterCpuInterruptHandler() interface.

Can please you explain in more detail why this is the case?

thanks,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81822): https://edk2.groups.io/g/devel/message/81822
Mute This Topic: https://groups.io/mt/86085742/21656
Mute #ve:https://edk2.groups.io/g/devel/mutehashtag/ve
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 00/15] Changes to compliant with BZ:#3665

2021-10-12 Thread Leif Lindholm
No issues with these, as long as they're merged (very shortly) after
the edk2 set.

Although please fix the typo in all of the commit messages
complaint ->
compliant

(But please don't send a v4 just for that.)

For the ones where you need it:
Acked-by: Leif Lindholm 



On Mon, Oct 04, 2021 at 17:29:46 +0800, Abner Chang wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=3665
> BZ:#3665 is to migrate some modules from ArmVirtPkg
> to under OvmfPkg for the upcoming RiscVVirtPkg that can leverage
> those modules without the dependency with Arm*Pkg.
> Refer to below message of the pacthes of edk2 portion.
> https://edk2.groups.io/g/devel/message/81306
> 
> This edk2-platforms patch set is the corresponding platform DSC/FDF/INF
> changes to compliant with BZ:#3665.
> We will merge this patch set when the time the changes of edk2 are merged
> once this patch set is reviewed.
> 
> FYI,
> The modules moved from ArmVirtPkg to OvmfPkg are:
> - FdtClientDxe
> - PciPcdProducerLib
> - HighMemDxe
> - QemuFwCfgLib
> - FdtPciHostBridgeLib
> - VirtioFdtDxe
> 
> Below PCDs are moved to under MdePkg and leverage by RiscVVirtPkg.
> This change also remove the dependency on ArmPkg of OvmfPkg.
> - PcdPciIoTranslation
> - PcdPciIoTranslation
> 
> Signed-off-by: Abner Chang 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Daniel Schaefer 
> Cc: Peng Xie 
> Cc: Ling Jia 
> Cc: Yiqi Shu 
> Cc: Thomas Abraham 
> Cc: Sami Mujawar 
> Cc: Graeme Gregory 
> Cc: Radoslaw Biernacki 
> Cc: Wenyi Xie 
> 
> Abner Chang (15):
>   Platform/Comcast: Use FdtClientDxe from EmbeddedPkg
>   Platform/Hisilicon: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/SbsaQemu: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/AMD: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/ARM: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/Comcast: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/LeMaker: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/Phytium: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/SoftIron: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/Socionext: Use PcdPciIoTranslation PCD from MdePkg
>   Platform/Comcast: Use PciPcdProducerLib in OvmfPkg.
>   Platform/Comcast: Use HighMemDxe provided by OvmfPkg
>   Platform/Comcast: Use QemuFwCfgMmio provided by OvmfPkg
>   Platform/Comcast: Use FdtPciHostBridgeLib provided by OvmfPkg.
>   Platform/Comcast: Use VirtioFdtDxe provided by OvmfPkg.
> 
>  Platform/ARM/SgiPkg/SgiPlatform.dsc.inc   |  3 ++-
>  .../AMD/OverdriveBoard/OverdriveBoard.dsc |  2 +-
>  Platform/ARM/JunoPkg/ArmJuno.dsc  |  3 ++-
>  Platform/Comcast/RDKQemu/RDKQemu.dsc  | 20 +--
>  Platform/Hisilicon/D03/D03.dsc|  2 +-
>  Platform/Hisilicon/D05/D05.dsc|  2 +-
>  Platform/Hisilicon/D06/D06.dsc|  2 +-
>  Platform/LeMaker/CelloBoard/CelloBoard.dsc|  2 +-
>  Platform/Phytium/DurianPkg/DurianPkg.dsc  |  2 +-
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc   |  2 +-
>  .../Overdrive1000Board/Overdrive1000Board.dsc |  2 +-
>  .../JunoPciHostBridgeLib.inf  |  3 ++-
>  .../Library/PlatformLib/PlatformLib.inf   |  2 +-
>  .../Qemu/SbsaQemu/AcpiTables/AcpiTables.inf   |  2 +-
>  .../SynQuacerPciHostBridgeLib.inf |  2 +-
>  15 files changed, 27 insertions(+), 24 deletions(-)
> 
> -- 
> 2.17.1
> 
> 
> 
> 
> 
> 


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




Re: [edk2-devel] [PATCH edk2-platforms 1/1] Silicon/Qemu: don't advertise GICC legacy mmio interface in SbsaQamu MADT

2021-10-12 Thread Leif Lindholm
On Fri, Oct 08, 2021 at 16:18:44 +0200, Ard Biesheuvel wrote:
> On Fri, 8 Oct 2021 at 12:34, Leif Lindholm  wrote:
> >
> > The MADT GICC structure contains the field PhysicalBaseAddress, which
> > is needed for a GICv1/v2 implementation, or to indicate legacy
> > compatibility in modern GICs.
> >
> > Linux commit 9739f6ef053f1, included in v5.12, adds a warning message
> > when this field is populated but invalid:
> >   [Firmware Bug]: CPU interface incapable of MMIO access
> >
> > As it happens, we currently initialize this to PcdGicDistributorBase
> > instead of PcdGicInterruptInterfaceBase, and as a result we now trigger
> > this warning.
> >
> > Since this is an SBSA reference implementation, and legacy GIC support
> > has never worked for this port, set the field to 0.
> >
> > Signed-off-by: Leif Lindholm 
> > Cc: Ard Biesheuvel 
> > Cc: Graeme Gregory 
> > Cc: Radoslaw Biernacki 
> 
> Acked-by: Ard Biesheuvel 

Thanks - pushed as 69035742d336.

/
Leif

> > ---
> >  Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git 
> > a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
> > b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > index b8901030ecd0..dbc5e9475358 100644
> > --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> > @@ -71,7 +71,7 @@ AddMadtTable (
> >  0, /* Mpidr */
> >  EFI_ACPI_6_0_GIC_ENABLED,  /* Flags */
> >  SBSAQEMU_MADT_GIC_PMU_IRQ, /* PMU Irq */
> > -FixedPcdGet32 (PcdGicDistributorBase), /* PhysicalBaseAddress */
> > +0, /* PhysicalBaseAddress */
> >  SBSAQEMU_MADT_GIC_VBASE,   /* GicVBase */
> >  SBSAQEMU_MADT_GIC_HBASE,   /* GicHBase */
> >  25,/* GsivId */
> > --
> > 2.30.2
> >


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




Re: [edk2-devel] [PATCH V2 07/28] UefiCpuPkg: Support TDX in BaseXApicX2ApicLib

2021-10-12 Thread Gerd Hoffmann
  Hi,

> +  do {
> +AsmCpuid (0, &LargestEax, &Ebx, &Ecx, &Edx);

Use ConfidentialComputing PCD ?

> +BOOLEAN
> +EFIAPI
> +AccessMsrNative (

I'd suggest to reverse the logic, i.e. have a AccessMsrTdxCall() which
returns true in case (a) tdx is active and (b) the msr is not on the
white list for native access ...

> +{
> +  UINT64Val;
> +  UINT64Status;
> +  if (!AccessMsrNative (MsrIndex) && BaseXApicIsTdxGuest ()) {

... the just use "if (AccessMsrTdxCall(MsrIndex)) { ..." here.

Beside that:  Are the apic msr registers the only ones which can
be accessed directly?

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V2 06/28] MdePkg: Update BaseIoLibIntrinsicSev to support Tdx

2021-10-12 Thread Gerd Hoffmann
On Tue, Oct 05, 2021 at 11:39:17AM +0800, Min Xu wrote:
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> 
> Intel TDX architecture does not prescribe a specific software convention
> to perform I/O from the guest TD. Guest TD providers have many choices to
> provide I/O to the guest. The common I/O models are emulated devices,
> para-virtualized devices, SRIOV devices and Direct Device assignments.

This monster patch needs splitting up.  At least into io + mmio + fifo.
Adding the tdx helper functions can be a separate patch too.

Calling CPUID should not be needed, we have a new fancy
ConfidentialComputing PCD for that now.

The new wrappers in IoLibFifo.c should also check for sev, so we have
something along the lines of ...

  switch (getpcd(cc)) {
  case tdx:
TdxFifo(...)
break;
  case sev:
SevFifo(...)
break;
  default:
DefaultFifo(...)
break;
  }

... instead of hiding the default case in IoFifoSev.nasm.

Maybe that's something to cleanup for amd (Brijesh?) beforehand, so the
structure is there already and the tdx patches just need to add the
"case tdx:" bits.

take care,
  Gerd
 



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




Re: [edk2-devel] [Patch V4 1/1] BaseTools: Change RealPath to AbsPath

2021-10-12 Thread Bob Feng
Reviewed-by: Bob Feng 

-Original Message-
From: Chen, Christine  
Sent: Tuesday, October 12, 2021 12:08 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C ; Liming Gao 
Subject: [Patch V4 1/1] BaseTools: Change RealPath to AbsPath

Currently the realpath is used when parse modules, which shows the path with a 
drive letter in build log. In Windows 'subst' comand is used to associates a 
path with a drive letter, when use the mapped drive letter for build, with 
realpath function the build log will have different disk letter info which will 
cause confusion. In this situation, if use adspath function to show the path 
info, it will keep same letter with the mapped drive letter, which avoids 
confusion.
This patch modifies the realpath to abspath.

Cc: Bob Feng 
Cc: Liming Gao 
Signed-off-by: Yuwei Chen 
---
 BaseTools/Source/Python/Ecc/EccMain.py| 2 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py | 4 ++--
 BaseTools/Source/Python/GenFds/GenFds.py  | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/EccMain.py 
b/BaseTools/Source/Python/Ecc/EccMain.py
index 72edbea3b883..a349cd80147f 100644
--- a/BaseTools/Source/Python/Ecc/EccMain.py
+++ b/BaseTools/Source/Python/Ecc/EccMain.py
@@ -105,7 +105,7 @@ class Ecc(object):
 
 def InitDefaultConfigIni(self):
 paths = map(lambda p: os.path.join(p, 'Ecc', 'config.ini'), sys.path)
-paths = (os.path.realpath('config.ini'),) + tuple(paths)
+paths = (os.path.abspath('config.ini'),) + tuple(paths)
 for path in paths:
 if os.path.exists(path):
 self.ConfigFile = path
diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py 
b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
index 20573ca28d2f..568efb6d7685 100644
--- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
+++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
@@ -707,8 +707,8 @@ class FfsInfStatement(FfsInfStatementClassObject):
   FileName,
   'DEBUG'
   )
-OutputPath = os.path.realpath(OutputPath)
-DebugPath = os.path.realpath(DebugPath)
+OutputPath = os.path.abspath(OutputPath)
+DebugPath = os.path.abspath(DebugPath)
 return OutputPath, DebugPath
 
 ## __GenSimpleFileSection__() method diff --git 
a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
index c34104500059..17b71b7cd347 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -153,7 +153,7 @@ def GenFdsApi(FdsCommandDict, WorkSpaceDataBase=None):
 FdfFilename = 
GenFdsGlobalVariable.ReplaceWorkspaceMacro(FdfFilename)
 
 if FdfFilename[0:2] == '..':
-FdfFilename = os.path.realpath(FdfFilename)
+FdfFilename = os.path.abspath(FdfFilename)
 if not os.path.isabs(FdfFilename):
 FdfFilename = mws.join(GenFdsGlobalVariable.WorkSpaceDir, 
FdfFilename)
 if not os.path.exists(FdfFilename):
@@ -175,7 +175,7 @@ def GenFdsApi(FdsCommandDict, WorkSpaceDataBase=None):
 ActivePlatform = 
GenFdsGlobalVariable.ReplaceWorkspaceMacro(ActivePlatform)
 
 if ActivePlatform[0:2] == '..':
-ActivePlatform = os.path.realpath(ActivePlatform)
+ActivePlatform = os.path.abspath(ActivePlatform)
 
 if not os.path.isabs (ActivePlatform):
 ActivePlatform = mws.join(GenFdsGlobalVariable.WorkSpaceDir, 
ActivePlatform) @@ -299,7 +299,7 @@ def GenFdsApi(FdsCommandDict, 
WorkSpaceDataBase=None):
 for Key in GenFdsGlobalVariable.OutputDirDict:
 OutputDir = GenFdsGlobalVariable.OutputDirDict[Key]
 if OutputDir[0:2] == '..':
-OutputDir = os.path.realpath(OutputDir)
+OutputDir = os.path.abspath(OutputDir)
 
 if OutputDir[1] != ':':
 OutputDir = os.path.join (GenFdsGlobalVariable.WorkSpaceDir, 
OutputDir)
--
2.27.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81817): https://edk2.groups.io/g/devel/message/81817
Mute This Topic: https://groups.io/mt/86254954/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] Reconfigure OpensslLib to add elliptic curve chipher algorithms

2021-10-12 Thread Vineel Kovvuri
This commit is a cherry pick of project mu's commit
https://github.com/microsoft/mu_tiano_plus/commit/1f3b135ddc821718a78c352316197889c5d3e0c2

Reconfigure OpensslLib to add elliptic curve chipher algorithms.
The only file manually changed is process_files.pl.
Running the script changes the other three files.

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

Signed-off-by: Vineel Kovvuri 
---
 .../Library/Include/openssl/opensslconf.h | 25 ++
 CryptoPkg/Library/OpensslLib/OpensslLib.inf   | 50 +++
 .../Library/OpensslLib/OpensslLibCrypto.inf   | 50 +++
 CryptoPkg/Library/OpensslLib/process_files.pl |  1 -
 4 files changed, 105 insertions(+), 21 deletions(-)

diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h 
b/CryptoPkg/Library/Include/openssl/opensslconf.h
index b8d59aebe8..09a6641ffc 100644
--- a/CryptoPkg/Library/Include/openssl/opensslconf.h
+++ b/CryptoPkg/Library/Include/openssl/opensslconf.h
@@ -55,9 +55,6 @@ extern "C" {
 #ifndef OPENSSL_NO_DSA
 # define OPENSSL_NO_DSA
 #endif
-#ifndef OPENSSL_NO_EC
-# define OPENSSL_NO_EC
-#endif
 #ifndef OPENSSL_NO_IDEA
 # define OPENSSL_NO_IDEA
 #endif
@@ -88,9 +85,6 @@ extern "C" {
 #ifndef OPENSSL_NO_SEED
 # define OPENSSL_NO_SEED
 #endif
-#ifndef OPENSSL_NO_SM2
-# define OPENSSL_NO_SM2
-#endif
 #ifndef OPENSSL_NO_SRP
 # define OPENSSL_NO_SRP
 #endif
@@ -154,12 +148,6 @@ extern "C" {
 #ifndef OPENSSL_NO_EC_NISTP_64_GCC_128
 # define OPENSSL_NO_EC_NISTP_64_GCC_128
 #endif
-#ifndef OPENSSL_NO_ECDH
-# define OPENSSL_NO_ECDH
-#endif
-#ifndef OPENSSL_NO_ECDSA
-# define OPENSSL_NO_ECDSA
-#endif
 #ifndef OPENSSL_NO_EGD
 # define OPENSSL_NO_EGD
 #endif
@@ -226,9 +214,6 @@ extern "C" {
 #ifndef OPENSSL_NO_TESTS
 # define OPENSSL_NO_TESTS
 #endif
-#ifndef OPENSSL_NO_TLS1_3
-# define OPENSSL_NO_TLS1_3
-#endif
 #ifndef OPENSSL_NO_UBSAN
 # define OPENSSL_NO_UBSAN
 #endif
@@ -265,11 +250,11 @@ extern "C" {
 #   undef DECLARE_DEPRECATED
 #   define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
 #  endif
-#elif defined(__SUNPRO_C)
-#if (__SUNPRO_C >= 0x5130)
-#undef DECLARE_DEPRECATED
-#define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
-#endif
+# elif defined(__SUNPRO_C)
+#  if (__SUNPRO_C >= 0x5130)
+#   undef DECLARE_DEPRECATED
+#   define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated));
+#  endif
 # endif
 #endif
 
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index d84bde056a..bd3d9cc90f 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -199,6 +199,43 @@
   $(OPENSSL_PATH)/crypto/dso/dso_vms.c
   $(OPENSSL_PATH)/crypto/dso/dso_win32.c
   $(OPENSSL_PATH)/crypto/ebcdic.c
+  $(OPENSSL_PATH)/crypto/ec/curve25519.c
+  $(OPENSSL_PATH)/crypto/ec/curve448/arch_32/f_impl.c
+  $(OPENSSL_PATH)/crypto/ec/curve448/curve448.c
+  $(OPENSSL_PATH)/crypto/ec/curve448/curve448_tables.c
+  $(OPENSSL_PATH)/crypto/ec/curve448/eddsa.c
+  $(OPENSSL_PATH)/crypto/ec/curve448/f_generic.c
+  $(OPENSSL_PATH)/crypto/ec/curve448/scalar.c
+  $(OPENSSL_PATH)/crypto/ec/ec2_oct.c
+  $(OPENSSL_PATH)/crypto/ec/ec2_smpl.c
+  $(OPENSSL_PATH)/crypto/ec/ec_ameth.c
+  $(OPENSSL_PATH)/crypto/ec/ec_asn1.c
+  $(OPENSSL_PATH)/crypto/ec/ec_check.c
+  $(OPENSSL_PATH)/crypto/ec/ec_curve.c
+  $(OPENSSL_PATH)/crypto/ec/ec_cvt.c
+  $(OPENSSL_PATH)/crypto/ec/ec_err.c
+  $(OPENSSL_PATH)/crypto/ec/ec_key.c
+  $(OPENSSL_PATH)/crypto/ec/ec_kmeth.c
+  $(OPENSSL_PATH)/crypto/ec/ec_lib.c
+  $(OPENSSL_PATH)/crypto/ec/ec_mult.c
+  $(OPENSSL_PATH)/crypto/ec/ec_oct.c
+  $(OPENSSL_PATH)/crypto/ec/ec_pmeth.c
+  $(OPENSSL_PATH)/crypto/ec/ec_print.c
+  $(OPENSSL_PATH)/crypto/ec/ecdh_kdf.c
+  $(OPENSSL_PATH)/crypto/ec/ecdh_ossl.c
+  $(OPENSSL_PATH)/crypto/ec/ecdsa_ossl.c
+  $(OPENSSL_PATH)/crypto/ec/ecdsa_sign.c
+  $(OPENSSL_PATH)/crypto/ec/ecdsa_vrf.c
+  $(OPENSSL_PATH)/crypto/ec/eck_prn.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_mont.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_nist.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_nistp224.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_nistp256.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_nistp521.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_nistputil.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_oct.c
+  $(OPENSSL_PATH)/crypto/ec/ecp_smpl.c
+  $(OPENSSL_PATH)/crypto/ec/ecx_meth.c
   $(OPENSSL_PATH)/crypto/err/err.c
   $(OPENSSL_PATH)/crypto/err/err_prn.c
   $(OPENSSL_PATH)/crypto/evp/bio_b64.c
@@ -384,6 +421,10 @@
   $(OPENSSL_PATH)/crypto/siphash/siphash.c
   $(OPENSSL_PATH)/crypto/siphash/siphash_ameth.c
   $(OPENSSL_PATH)/crypto/siphash/siphash_pmeth.c
+  $(OPENSSL_PATH)/crypto/sm2/sm2_crypt.c
+  $(OPENSSL_PATH)/crypto/sm2/sm2_err.c
+  $(OPENSSL_PATH)/crypto/sm2/sm2_pmeth.c
+  $(OPENSSL_PATH)/crypto/sm2/sm2_sign.c
   $(OPENSSL_PATH)/crypto/sm3/m_sm3.c
   $(OPENSSL_PATH)/crypto/sm3/sm3.c
   $(OPENSSL_PATH)/crypto/sm4/sm4.c
@@ -496,6 +537,15 @@
   $(OPENSSL_PATH)/crypto/conf/conf_local.h
   $(OPENSSL_PATH)/crypto/dh/dh_local.h
   $(OPENSSL_PAT

[edk2-devel] [PATCH] IntelSiliconPkg/FirmwareInterfaceTable: Define FIT 4 record

2021-10-12 Thread Holland, Michael



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




0001-IntelSiliconPkg-FirmwareInterfaceTable-Define-FIT-4-.patch
Description: 0001-IntelSiliconPkg-FirmwareInterfaceTable-Define-FIT-4-.patch


[edk2-devel] [PATCH 2/2] Allow wildcards in hostname

2021-10-12 Thread Vineel Kovvuri
This PR is cherry-picked from
https://github.com/microsoft/mu_basecore/commit/d0c7733400c35722499eedcd4279042a9bcb0eb4

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

Signed-off-by: Vineel Kovvuri 
---
 NetworkPkg/HttpDxe/HttpsSupport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c 
b/NetworkPkg/HttpDxe/HttpsSupport.c
index 7e0bf85c3c..0f28ae9447 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -625,7 +625,7 @@ TlsConfigureSession (
   //
   HttpInstance->TlsConfigData.ConnectionEnd   = EfiTlsClient;
   HttpInstance->TlsConfigData.VerifyMethod= EFI_TLS_VERIFY_PEER;
-  HttpInstance->TlsConfigData.VerifyHost.Flags= 
EFI_TLS_VERIFY_FLAG_NO_WILDCARDS;
+  HttpInstance->TlsConfigData.VerifyHost.Flags= EFI_TLS_VERIFY_FLAG_NONE;
   HttpInstance->TlsConfigData.VerifyHost.HostName = HttpInstance->RemoteHost;
   HttpInstance->TlsConfigData.SessionState= EfiTlsSessionNotStarted;
 
-- 
2.17.1



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




Re: [edk2-devel] [PATCH] MdeModulePkg/Core/Dxe: Acquire a lock when iterating gHandleList

2021-10-12 Thread Ma, Hua
Hi Dandan,

Thanks for the comment.
Patch v2 is just sent with the update.
Please help to review.

Thank you,
Ma Hua

> -Original Message-
> From: Bi, Dandan 
> Sent: Tuesday, October 12, 2021 12:13 PM
> To: Ma, Hua ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Liming Gao
> ; Ni, Ray 
> Subject: RE: [PATCH] MdeModulePkg/Core/Dxe: Acquire a lock when
> iterating gHandleList
> 
> Hi Hua,
> 
> One minor comment inline, please check.
> 
> 
> 
> Thanks,
> Dandan
> 
> > -Original Message-
> > From: Ma, Hua 
> > Sent: Monday, October 11, 2021 6:45 PM
> > To: devel@edk2.groups.io
> > Cc: Ma, Hua ; Wang, Jian J ;
> > Liming Gao ; Bi, Dandan
> ;
> > Ni, Ray 
> > Subject: [PATCH] MdeModulePkg/Core/Dxe: Acquire a lock when iterating
> > gHandleList
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3680
> >
> > This patch fixes the following issue:
> >
> > The global variable gHandleList is a linked list.
> > This list is locked when a entry is added or removed from the list, but 
> > there
> is no
> > lock when iterating this list in function CoreValidateHandle().
> > It can lead to "Handle.c (76): CR has Bad Signature" assertion if the 
> > iterated
> > entry in the list is just removed by other task during iterating.
> > Locking the list when iterating can fix this issue.
> >
> > Cc: Jian J Wang 
> > Cc: Liming Gao 
> > Cc: Dandan Bi 
> > Cc: Ray Ni 
> > Signed-off-by: Hua Ma 
> > ---
> >  MdeModulePkg/Core/Dxe/Hand/DriverSupport.c | 10 ++---
> >  MdeModulePkg/Core/Dxe/Hand/Handle.c| 47 ++--
> --
> >  MdeModulePkg/Core/Dxe/Hand/Handle.h|  3 +-
> >  MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
> >  4 files changed, 39 insertions(+), 23 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> > b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> > index feabf12faf..eb8a765d2c 100644
> > --- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> > +++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
> > @@ -68,7 +68,7 @@ CoreConnectController (
> >//
> >// Make sure ControllerHandle is valid
> >//
> > -  Status = CoreValidateHandle (ControllerHandle);
> > +  Status = CoreValidateHandle (ControllerHandle, FALSE);
> >if (EFI_ERROR (Status)) {
> >  return Status;
> >}
> > @@ -154,7 +154,7 @@ CoreConnectController (
> >  //
> >  // Make sure the DriverBindingHandle is valid
> >  //
> > -Status = CoreValidateHandle (ControllerHandle);
> > +Status = CoreValidateHandle (ControllerHandle, TRUE);
> >  if (EFI_ERROR (Status)) {
> >//
> >// Release the protocol lock on the handle database @@ -268,7 +268,7
> @@
> > AddSortedDriverBindingProtocol (
> >//
> >// Make sure the DriverBindingHandle is valid
> >//
> > -  Status = CoreValidateHandle (DriverBindingHandle);
> > +  Status = CoreValidateHandle (DriverBindingHandle, FALSE);
> >if (EFI_ERROR (Status)) {
> >  return;
> >}
> > @@ -746,7 +746,7 @@ CoreDisconnectController (
> >//
> >// Make sure ControllerHandle is valid
> >//
> > -  Status = CoreValidateHandle (ControllerHandle);
> > +  Status = CoreValidateHandle (ControllerHandle, FALSE);
> >if (EFI_ERROR (Status)) {
> >  return Status;
> >}
> > @@ -755,7 +755,7 @@ CoreDisconnectController (
> >// Make sure ChildHandle is valid if it is not NULL
> >//
> >if (ChildHandle != NULL) {
> > -Status = CoreValidateHandle (ChildHandle);
> > +Status = CoreValidateHandle (ChildHandle, FALSE);
> >  if (EFI_ERROR (Status)) {
> >return Status;
> >  }
> > diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> > b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> > index 6eccb41ecb..b4f389bb35 100644
> > --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
> > +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
> > @@ -55,31 +55,46 @@ CoreReleaseProtocolLock (
> >Check whether a handle is a valid EFI_HANDLE
> >
> >@param  UserHandle The handle to check
> > +  @param  IsLocked   The protocol lock is acquried or not
> >
> >@retval EFI_INVALID_PARAMETER  The handle is NULL or not a valid
> > EFI_HANDLE.
> > +  @retval EFI_NOT_FOUND  The handle is not found in the handle
> > database.
> >@retval EFI_SUCCESSThe handle is valid EFI_HANDLE.
> >
> >  **/
> >  EFI_STATUS
> >  CoreValidateHandle (
> > -  IN  EFI_HANDLEUserHandle
> > +  IN  EFI_HANDLEUserHandle,
> > +  IN  BOOLEAN   IsLocked
> >)
> >  {
> >IHANDLE *Handle;
> >LIST_ENTRY  *Link;
> > +  EFI_STATUS  Status;
> >
> >if (UserHandle == NULL) {
> >  return EFI_INVALID_PARAMETER;
> >}
> >
> > +  if (IsLocked) {
> > +ASSERT_LOCKED(&gProtocolDatabaseLock);
> > +  } else {
> > +CoreAcquireProtocolLock ();
> > +  }
> > +
> > +  Status = EFI_NOT_FOUND;
> >for (Link = gHandleList.BackLink; Link != &gHandleList; Link = Link-
> >BackLink) {
> >  Handle = CR

[edk2-devel] [PATCH v2] MdeModulePkg/Core/Dxe: Acquire a lock when iterating gHandleList

2021-10-12 Thread Ma, Hua
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3680

This patch fixes the following issue:

The global variable gHandleList is a linked list.
This list is locked when a entry is added or removed from the list,
but there is no lock when iterating this list in function
CoreValidateHandle().
It can lead to "Handle.c (76): CR has Bad Signature" assertion if the
iterated entry in the list is just removed by other task during iterating.
Locking the list when iterating can fix this issue.

v2 changes:
 - Add lock check and comments in CoreGetProtocolInterface() before
calling CoreValidateHandle()
 - Update the comments in CoreValidateHandle() header file

v1: https://edk2.groups.io/g/devel/topic/86233569

Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
Cc: Ray Ni 
Signed-off-by: Hua Ma 
---
 MdeModulePkg/Core/Dxe/Hand/DriverSupport.c | 10 ++--
 MdeModulePkg/Core/Dxe/Hand/Handle.c| 56 +++---
 MdeModulePkg/Core/Dxe/Hand/Handle.h|  5 +-
 MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
 4 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c 
b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
index feabf12faf..eb8a765d2c 100644
--- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
+++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
@@ -68,7 +68,7 @@ CoreConnectController (
   //
   // Make sure ControllerHandle is valid
   //
-  Status = CoreValidateHandle (ControllerHandle);
+  Status = CoreValidateHandle (ControllerHandle, FALSE);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -154,7 +154,7 @@ CoreConnectController (
 //
 // Make sure the DriverBindingHandle is valid
 //
-Status = CoreValidateHandle (ControllerHandle);
+Status = CoreValidateHandle (ControllerHandle, TRUE);
 if (EFI_ERROR (Status)) {
   //
   // Release the protocol lock on the handle database
@@ -268,7 +268,7 @@ AddSortedDriverBindingProtocol (
   //
   // Make sure the DriverBindingHandle is valid
   //
-  Status = CoreValidateHandle (DriverBindingHandle);
+  Status = CoreValidateHandle (DriverBindingHandle, FALSE);
   if (EFI_ERROR (Status)) {
 return;
   }
@@ -746,7 +746,7 @@ CoreDisconnectController (
   //
   // Make sure ControllerHandle is valid
   //
-  Status = CoreValidateHandle (ControllerHandle);
+  Status = CoreValidateHandle (ControllerHandle, FALSE);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -755,7 +755,7 @@ CoreDisconnectController (
   // Make sure ChildHandle is valid if it is not NULL
   //
   if (ChildHandle != NULL) {
-Status = CoreValidateHandle (ChildHandle);
+Status = CoreValidateHandle (ChildHandle, FALSE);
 if (EFI_ERROR (Status)) {
   return Status;
 }
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index 6eccb41ecb..46f67d3d6a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -55,31 +55,46 @@ CoreReleaseProtocolLock (
   Check whether a handle is a valid EFI_HANDLE
 
   @param  UserHandle The handle to check
+  @param  IsLocked   The protocol lock is acquried or not
 
   @retval EFI_INVALID_PARAMETER  The handle is NULL or not a valid EFI_HANDLE.
+  @retval EFI_NOT_FOUND  The handle is not found in the handle 
database.
   @retval EFI_SUCCESSThe handle is valid EFI_HANDLE.
 
 **/
 EFI_STATUS
 CoreValidateHandle (
-  IN  EFI_HANDLEUserHandle
+  IN  EFI_HANDLEUserHandle,
+  IN  BOOLEAN   IsLocked
   )
 {
   IHANDLE *Handle;
   LIST_ENTRY  *Link;
+  EFI_STATUS  Status;
 
   if (UserHandle == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
+  if (IsLocked) {
+ASSERT_LOCKED(&gProtocolDatabaseLock);
+  } else {
+CoreAcquireProtocolLock ();
+  }
+
+  Status = EFI_NOT_FOUND;
   for (Link = gHandleList.BackLink; Link != &gHandleList; Link = 
Link->BackLink) {
 Handle = CR (Link, IHANDLE, AllHandles, EFI_HANDLE_SIGNATURE);
 if (Handle == (IHANDLE *) UserHandle) {
-  return EFI_SUCCESS;
+  Status = EFI_SUCCESS;
+  break;
 }
   }
 
-  return EFI_INVALID_PARAMETER;
+  if (!IsLocked) {
+CoreReleaseProtocolLock ();
+  }
+  return Status;
 }
 
 
@@ -428,7 +443,7 @@ CoreInstallProtocolInterfaceNotify (
 //
 InsertTailList (&gHandleList, &Handle->AllHandles);
   } else {
-Status = CoreValidateHandle (Handle);
+Status = CoreValidateHandle (Handle, TRUE);
 if (EFI_ERROR (Status)) {
   DEBUG((DEBUG_ERROR, "InstallProtocolInterface: input handle at 0x%x is 
invalid\n", Handle));
   goto Done;
@@ -723,7 +738,7 @@ CoreUninstallProtocolInterface (
   //
   // Check that UserHandle is a valid handle
   //
-  Status = CoreValidateHandle (UserHandle);
+  Status = CoreValidateHandle (UserHandle, FALSE);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -899,7 +914,16 @@ CoreGetProtocolInterface (
   IHANDLE *H

Re: [edk2-devel] [PATCH edk2-test v2 1/1] uefi-sct/SctPkg: fix BuildAtaDeviceNode()

2021-10-12 Thread G Edhaya Chandran
Reviewed-by: G Edhaya Chandran


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




Re: [edk2-devel] [PATCH V2 05/28] MdePkg: Add TdxLib to wrap Tdx operations

2021-10-12 Thread Gerd Hoffmann
> +// PageSize is mapped to PageLevel like below:
> +// 4KB - 0, 2MB - 1
> +UINT64  mTdxAcceptPageLevelMap[2] = {
> +  SIZE_4KB,
> +  SIZE_2MB

No 1G pages?

> +UINTN
> +GetGpaPageLevel (
> +  UINT64 PageSize

Uh, UINT32 is not enough?  Keep the door open for 512G pages?

> +{
> +  UINTN Index;
> +
> +  for (Index = 0; Index < sizeof (mTdxAcceptPageLevelMap) / sizeof 
> (mTdxAcceptPageLevelMap[0]); Index++) {

There is an ARRAY_SIZE() macro, no need to open code the sizeof() trick.

> +if (mTdxAcceptPageLevelMap[Index] == PageSize) {
> +  break;
> +}
> +  }
> +
> +  return Index;
> +}

No error handling (invalid PageSize) here?

> +/**
> +  This function accept a pending private page, and initialize the page to
> +  all-0 using the TD ephemeral private key.
> +
> +  Sometimes TDCALL [TDG.MEM.PAGE.ACCEPT] may return
> +  TDX_EXIT_REASON_PAGE_SIZE_MISMATCH. It indicates the input PageLevel is
> +  not workable. In this case we need to try to fallback to a smaller
> +  PageLevel if possible.
> +
> +  @param[in]  StartAddress  Guest physical address of the private
> +page to accept.
> +  @param[in]  NumberOfPages Number of the pages to be accepted.
> +  @param[in]  PageSize  GPA page size. Only accept 1G/2M/4K size.
> +
> +  @return EFI_SUCCESS   Accept successfully
> +  @return othersIndicate other errors
> +**/
> +EFI_STATUS
> +EFIAPI
> +TdAcceptPages (
> +  IN UINT64  StartAddress,
> +  IN UINT64  NumberOfPages,
> +  IN UINT64  PageSize
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT64  Address;
> +  UINT64  TdxStatus;
> +  UINT64  Index;
> +  UINT64  GpaPageLevel;
> +  UINT64  PageSize2;
> +
> +  Address = StartAddress;
> +
> +  GpaPageLevel = (UINT64) GetGpaPageLevel (PageSize);

Why cast?

> +  if (GpaPageLevel > sizeof (mTdxAcceptPageLevelMap) / sizeof 
> (mTdxAcceptPageLevelMap[0])) {
> +DEBUG ((DEBUG_ERROR, "Accept page size must be 4K/2M. Invalid page size 
> - 0x%llx\n", PageSize));

Ah.  Errors are catched here.  Well, no.  The check is wrong,
it should be ">=" not ">".

Better would be GetGpaPageLevel explicitly returning a specific value
(for example -1) on error.

> +  Status = EFI_SUCCESS;
> +  for (Index = 0; Index < NumberOfPages; Index++) {
> +TdxStatus = TdCall (TDCALL_TDACCEPTPAGE, Address | GpaPageLevel, 0, 0, 
> 0);
> +if (TdxStatus != TDX_EXIT_REASON_SUCCESS) {
> +if ((TdxStatus & ~0xULL) == 
> TDX_EXIT_REASON_PAGE_ALREADY_ACCEPTED) {
> +  //
> +  // Already accepted
> +  //
> +  mNumberOfDuplicatedAcceptedPages++;

Hmm.  When this happens we have a bug somewhere, right?
So should this be an assert()?
Or should we at least log the address?

> +#define RTMR_COUNT  4
> +#define TD_EXTEND_BUFFER_LEN(64 + 64)
> +#define EXTEND_BUFFER_ADDRESS_MASK  0x3f
> +
> +
> +#pragma pack(16)
> +typedef struct {
> +  UINT8   Buffer[TD_EXTEND_BUFFER_LEN];
> +} TDX_EXTEND_BUFFER;
> +#pragma pack()
> +
> +UINT8   *mExtendBufferAddress = NULL;
> +TDX_EXTEND_BUFFER   mExtendBuffer;
> +
> +/**
> +  TD.RTMR.EXTEND requires 64B-aligned guest physical address of
> +  48B-extension data. In runtime we walk thru the Buffer to find
> +  out a 64B-aligned start address.

Can't you just use __attribute__((aligned(64))) for that?

take care,
  Gerd



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




[edk2-devel] [PATCH EDK2 v1 1/1] EmbeddedPkg:Fix compiler warning

2021-10-12 Thread wenyi,xie via groups.io
Fixes the following compiler warning in VS2019.

edk2\EmbeddedPkg\Library\GdbSerialDebugPortLib\GdbSerialDebugPortLib.c(127):
error C2220: the following warning is treated as an error
edk2\EmbeddedPkg\Library\GdbSerialDebugPortLib\GdbSerialDebugPortLib.c(127):
warning C4244: 'function': conversion from 'UINTN' to 'UINT32', possible
loss of data

edk2\EmbeddedPkg\Library\PrePiLib\FwVol.c(347): error C2220: the following
warning is treated as an error
edk2\EmbeddedPkg\Library\PrePiLib\FwVol.c(347): warning C4244: 'function':
conversion from 'UINTN' to 'UINT32', possible loss of data

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Abner Chang 
Cc: Daniel Schaefer 
Signed-off-by: Wenyi Xie 
---
 EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c | 2 +-
 EmbeddedPkg/Library/PrePiLib/FwVol.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c 
b/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
index d2bafcf69b60..0f50a8b64191 100644
--- a/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
+++ b/EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c
@@ -18,7 +18,7 @@
 
 
 EFI_DEBUGPORT_PROTOCOL  *gDebugPort = NULL;
-UINTN   gTimeOut = 0;
+UINT32  gTimeOut = 0;
 
 /**
   The constructor function initializes the UART.
diff --git a/EmbeddedPkg/Library/PrePiLib/FwVol.c 
b/EmbeddedPkg/Library/PrePiLib/FwVol.c
index 881506edddaf..46ea5f733f60 100644
--- a/EmbeddedPkg/Library/PrePiLib/FwVol.c
+++ b/EmbeddedPkg/Library/PrePiLib/FwVol.c
@@ -298,7 +298,7 @@ FfsProcessSection (
   UINT16  SectionAttribute;
   UINT32  AuthenticationStatus;
   CHAR8   *CompressedData;
-  UINTN   CompressedDataLength;
+  UINT32  CompressedDataLength;
 
 
   *OutputBuffer = NULL;
-- 
2.20.1.windows.1



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




[edk2-devel] [PATCH EDK2 v1 0/1] EmbeddedPkg:Fix compiler warning

2021-10-12 Thread wenyi,xie via groups.io
Main Changes :
1.Changing definition of gTimeOut from UINTN to UINT32
2.Changing definition of CompressedDataLength from UINTN to UINT32

Wenyi Xie (1):
  EmbeddedPkg:Fix compiler warning

 EmbeddedPkg/Library/GdbSerialDebugPortLib/GdbSerialDebugPortLib.c | 2 +-
 EmbeddedPkg/Library/PrePiLib/FwVol.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.20.1.windows.1



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




Re: [edk2-devel] [PATCH edk2-test 1/1] uefi-sct/SctPkg: unsupported TEXT_INPUT_EX.SetState

2021-10-12 Thread G Edhaya Chandran
On Thu, Sep 9, 2021 at 12:45 AM, Heinrich Schuchardt wrote:

> 
> BBTestReadKeyStrokeExFunctionAutoTestCheckpoint1

Reviewed-by: G Edhaya Chandran


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




Re: [edk2-devel] [PATCH V2 04/28] MdePkg: Add Tdx.h

2021-10-12 Thread Gerd Hoffmann
On Tue, Oct 05, 2021 at 11:39:15AM +0800, Min Xu wrote:
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> 
> Tdx.h includes the Intel Trust Domain Extension definitions.
> 
> Detailed information can be found in below document:
> https://software.intel.com/content/dam/develop/external/us/en/
> documents/tdx-module-1eas-v0.85.039.pdf
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Jiewen Yao 
> Signed-off-by: Min Xu 

Acked-by: Gerd Hoffmann 



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




Re: [edk2-devel] [PATCH V9 4/4] OvmfPkg: Enable TDX in ResetVector

2021-10-12 Thread Gerd Hoffmann
  Hi,

> +; Load the GDT and set the CR0.
> +;
> +; Modified:  EAX, EBX, CR0, CR4, DS, ES, FS, GS, SS, CS
> +;
> +ReloadFlat32:
> +
> +cli
> +mov ebx, ADDR_OF(gdtr)
> +lgdt[ebx]

No need to modify ebx here, eax should do fine.

> +mov eax, SEC_DEFAULT_CR0
> +mov cr0, eax
> +
> +jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpToFlat32BitAndLandHere)
> +
> +jumpToFlat32BitAndLandHere:

Strictly speaking this is not correct, you are already in Flat32 mode,
so this only loads cs.

> +InitTdx:
> +;
> +; Save EBX in EBP because EBX will be changed in ReloadFlat32
> +;
> +mov ebp, ebx

See above, there is no need to modify ebx in ReloadFlat32.
Also: seems ebx is never restored ...

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V9 3/4] OvmfPkg: Add IntelTdxMetadata.asm

2021-10-12 Thread Gerd Hoffmann
  Hi,

> +; - Type field means this section is of BFV. This field is designed for the
> +;   purpose that in some case host VMM may do some additional processing 
> based
> +;   upon the section type. TdHob section is an example. Host VMM pass the
> +;   physical memory information to the guest firmware by writing the data in
> +;   the memory region designated by TdHob section.

Brijesh?  What are your plans?  Do you want use this too?

If so, then the section types need some more structure.  I think best
would be reserve blocks, something like 0x00 -> 0xff common types (bfv,
cfv, temp_mem), 0x100 -> 0x1ff tdx (td_hob), 0x200 -> 0x2ff sev (cpuid,
secrets, ...).

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V9 2/4] OvmfPkg: Clear WORK_AREA_GUEST_TYPE in Main.asm

2021-10-12 Thread Gerd Hoffmann
On Tue, Oct 12, 2021 at 10:37:48AM +0800, Min Xu wrote:
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> 
> Previously WORK_AREA_GUEST_TYPE was cleared in SetCr3ForPageTables64.
> This is workable for Legacy guest and SEV guest. But it doesn't work
> after Intel TDX is introduced. It is because all TDX CPUs (BSP and APs)
> start to run from 0xfff0, thus WORK_AREA_GUEST_TYPE will be cleared
> multi-times if it is TDX guest. So the clearance of WORK_AREA_GUEST_TYPE
> is moved to Main16 entry point in Main.asm.
> Note: WORK_AREA_GUEST_TYPE is only defined for ARCH_X64.
> 
> For Intel TDX, its corresponding entry point is Main32 (which will be
> introduced in next commit in this patch-set). WORK_AREA_GUEST_TYPE will
> be cleared there.

Acked-by: Gerd Hoffmann 

take care,
  Gerd



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




Re: [edk2-devel] [PATCH V9 1/4] OvmfPkg: Copy Main.asm from UefiCpuPkg to OvmfPkg's ResetVector

2021-10-12 Thread Gerd Hoffmann
On Tue, Oct 12, 2021 at 10:37:47AM +0800, Min Xu wrote:
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> 
> Previously OvmfPkg/ResetVector uses the Main.asm in
> UefiCpuPkg/ReseteVector/Vtf0. In this Main.asm there is only Main16
> entry point.
> 
> This patch-set is to introduce Intel TDX into Ovmf. Main32 entry point
> is needed in Main.asm by Intel TDX. To reduce the complexity of Main.asm
> in UefiCpuPkg, OvmfPkg create its own Main.asm to meet the requirement
> of Intel TDX.
> 
> UefiCpuPkg/ResetVector/Vtf0/main.asm -> OvmfPkg/ResetVector/Main.asm

You should mention that this is an unmodified copy (so no functional
change) and the actual changes for tdx come as incremental patches.

With that:
Acked-by: Gerd Hoffmann 

take care,
  Gerd



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