Re: [edk2-devel] [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

[Yao, Jiewen]

Thanks. Code talks better.

I prefer option 2, which is a generic way for abstraction.

And you may enable option 1 under the cover of option 2, just create a lib 
instance to get config from Hob.

Thank you
Yao Jiewen

> -Original Message-
> From: Taylor Beebe 
> Sent: Friday, July 30, 2021 10:07 AM
> To: Yao, Jiewen ; Wang, Jian J ;
> devel@edk2.groups.io
> Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
> ; Kumar, Rahul1 ;
> mikub...@linux.microsoft.com; Wu, Hao A ; Bi, Dandan
> ; gaolim...@byosoft.com.cn; Dong, Guo
> ; Ma, Maurice ; You, Benjamin
> 
> Subject: Re: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings
> 
> Of course - here are a couple of rough drafts:
> 
> Option 1: https://github.com/TaylorBeebe/edk2/tree/memory_protection_lib_2
> Option 2: https://github.com/TaylorBeebe/edk2/tree/memory_protection_lib
> 
> On 7/29/2021 6:57 PM, Yao, Jiewen wrote:
> > Hi
> > Sorry, I am not able to follow the discussion.
> >
> > Is there any sample or POC code to show the concept?
> >
> >> -Original Message-
> >> From: Taylor Beebe 
> >> Sent: Friday, July 30, 2021 9:55 AM
> >> To: Wang, Jian J ; devel@edk2.groups.io
> >> Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
> >> ; Kumar, Rahul1 ;
> >> mikub...@linux.microsoft.com; Wu, Hao A ; Bi,
> Dandan
> >> ; gaolim...@byosoft.com.cn; Dong, Guo
> >> ; Ma, Maurice ; You,
> Benjamin
> >> ; Yao, Jiewen 
> >> Subject: Re: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings
> >>
> >> Thanks for your feedback, Jian.
> >>
> >> In option 2, a most basic implementation would returning the current
> >> FixedAtBuild PCDs assuming they are kept. If they aren't, the library
> >> implementer could simply hard-code the return value for each memory
> >> protection setting.
> >>
> >> In option 1, the HOB would be published in pre-mem and I'm not an expert
> >> on exploiting the pre-mem environment. Jiewen may have more to say on
> this.
> >>
> >> -Taylor
> >>
> >> On 7/28/2021 7:18 PM, Wang, Jian J wrote:
> >>> Thanks for the RFC. I'm not object to this idea. The only concern from me
> >>> is the potential security holes introduced by the changes. According to 
> >>> your
> >>> description, it allows 3rd party software to violate memory protection
> policy.
> >>> I'd like to see more explanations on how to avoid it to be exploited.
> >>>
> >>> +Jiewen, what's current process to evaluate the security threat?
> >>>
> >>> Regards,
> >>> Jian
> >>>
>  -Original Message-
>  From: Taylor Beebe 
>  Sent: Friday, July 23, 2021 8:33 AM
>  To: devel@edk2.groups.io
>  Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
>  ; Kumar, Rahul1 ;
>  mikub...@linux.microsoft.com; Wang, Jian J ;
> Wu,
>  Hao A ; Bi, Dandan ;
>  gaolim...@byosoft.com.cn; Dong, Guo ; Ma,
> >> Maurice
>  ; You, Benjamin 
>  Subject: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings
> 
>  Current memory protection settings rely on FixedAtBuild PCD values
>  (minus PcdSetNxForStack). Because of this, the memory protection
>  configuration interface is fixed in nature. Cases arise in which memory
>  protections might need to be adjusted between boots (if platform design
>  allows) to avoid disabling a system. For example, platforms might choose
>  to allow the user to control their protection policies such as allow
>  execution of critical 3rd party software that might violate memory
>  protections.
> 
>  This RFC seeks your feedback regarding introducing an interface that
>  allows dynamic configuration of memory protection settings.
> 
>  I would like to propose two options:
>  1. Describing the memory protection setting configuration in a HOB that
>  is produced by the platform.
>  2. Introducing a library class (e.g. MemoryProtectionLib) that allows
>  abstraction of the memory protection setting configuration data source.
> 
>  In addition, I would like to know if the memory protection FixedAtBuild
>  PCDs currently in MdeModulePkg can be removed so we can move the
>  configuration interface entirely to an option above.
> 
>  In any case, I would like the settings to be visible to environments
>  such as Standalone MM where dynamic PCDs are not accessible.
> 
>  I am seeking your feedback on this proposal in preparation for sending
>  an edk2 patch series.
> 
>  --
>  Taylor Beebe
>  Software Engineer @ Microsoft
> >>
> >> --
> >> Taylor Beebe
> >> Software Engineer @ Microsoft
> 
> --
> Taylor Beebe
> Software Engineer @ Microsoft


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

Re: [edk2-devel] [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

[Taylor Beebe]

Of course - here are a couple of rough drafts:

Option 1: https://github.com/TaylorBeebe/edk2/tree/memory_protection_lib_2
Option 2: https://github.com/TaylorBeebe/edk2/tree/memory_protection_lib

On 7/29/2021 6:57 PM, Yao, Jiewen wrote:

Hi
Sorry, I am not able to follow the discussion.

Is there any sample or POC code to show the concept?


-Original Message-
From: Taylor Beebe 
Sent: Friday, July 30, 2021 9:55 AM
To: Wang, Jian J ; devel@edk2.groups.io
Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
; Kumar, Rahul1 ;
mikub...@linux.microsoft.com; Wu, Hao A ; Bi, Dandan
; gaolim...@byosoft.com.cn; Dong, Guo
; Ma, Maurice ; You, Benjamin
; Yao, Jiewen 
Subject: Re: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

Thanks for your feedback, Jian.

In option 2, a most basic implementation would returning the current
FixedAtBuild PCDs assuming they are kept. If they aren't, the library
implementer could simply hard-code the return value for each memory
protection setting.

In option 1, the HOB would be published in pre-mem and I'm not an expert
on exploiting the pre-mem environment. Jiewen may have more to say on this.

-Taylor

On 7/28/2021 7:18 PM, Wang, Jian J wrote:

Thanks for the RFC. I'm not object to this idea. The only concern from me
is the potential security holes introduced by the changes. According to your
description, it allows 3rd party software to violate memory protection policy.
I'd like to see more explanations on how to avoid it to be exploited.

+Jiewen, what's current process to evaluate the security threat?

Regards,
Jian


-Original Message-
From: Taylor Beebe 
Sent: Friday, July 23, 2021 8:33 AM
To: devel@edk2.groups.io
Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
; Kumar, Rahul1 ;
mikub...@linux.microsoft.com; Wang, Jian J ; Wu,
Hao A ; Bi, Dandan ;
gaolim...@byosoft.com.cn; Dong, Guo ; Ma,

Maurice

; You, Benjamin 
Subject: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

Current memory protection settings rely on FixedAtBuild PCD values
(minus PcdSetNxForStack). Because of this, the memory protection
configuration interface is fixed in nature. Cases arise in which memory
protections might need to be adjusted between boots (if platform design
allows) to avoid disabling a system. For example, platforms might choose
to allow the user to control their protection policies such as allow
execution of critical 3rd party software that might violate memory
protections.

This RFC seeks your feedback regarding introducing an interface that
allows dynamic configuration of memory protection settings.

I would like to propose two options:
1. Describing the memory protection setting configuration in a HOB that
is produced by the platform.
2. Introducing a library class (e.g. MemoryProtectionLib) that allows
abstraction of the memory protection setting configuration data source.

In addition, I would like to know if the memory protection FixedAtBuild
PCDs currently in MdeModulePkg can be removed so we can move the
configuration interface entirely to an option above.

In any case, I would like the settings to be visible to environments
such as Standalone MM where dynamic PCDs are not accessible.

I am seeking your feedback on this proposal in preparation for sending
an edk2 patch series.

--
Taylor Beebe
Software Engineer @ Microsoft


--
Taylor Beebe
Software Engineer @ Microsoft


--
Taylor Beebe
Software Engineer @ Microsoft


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

Re: [edk2-devel] [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

[Yao, Jiewen]

Hi 
Sorry, I am not able to follow the discussion.

Is there any sample or POC code to show the concept?

> -Original Message-
> From: Taylor Beebe 
> Sent: Friday, July 30, 2021 9:55 AM
> To: Wang, Jian J ; devel@edk2.groups.io
> Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
> ; Kumar, Rahul1 ;
> mikub...@linux.microsoft.com; Wu, Hao A ; Bi, Dandan
> ; gaolim...@byosoft.com.cn; Dong, Guo
> ; Ma, Maurice ; You, Benjamin
> ; Yao, Jiewen 
> Subject: Re: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings
> 
> Thanks for your feedback, Jian.
> 
> In option 2, a most basic implementation would returning the current
> FixedAtBuild PCDs assuming they are kept. If they aren't, the library
> implementer could simply hard-code the return value for each memory
> protection setting.
> 
> In option 1, the HOB would be published in pre-mem and I'm not an expert
> on exploiting the pre-mem environment. Jiewen may have more to say on this.
> 
> -Taylor
> 
> On 7/28/2021 7:18 PM, Wang, Jian J wrote:
> > Thanks for the RFC. I'm not object to this idea. The only concern from me
> > is the potential security holes introduced by the changes. According to your
> > description, it allows 3rd party software to violate memory protection 
> > policy.
> > I'd like to see more explanations on how to avoid it to be exploited.
> >
> > +Jiewen, what's current process to evaluate the security threat?
> >
> > Regards,
> > Jian
> >
> >> -Original Message-
> >> From: Taylor Beebe 
> >> Sent: Friday, July 23, 2021 8:33 AM
> >> To: devel@edk2.groups.io
> >> Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
> >> ; Kumar, Rahul1 ;
> >> mikub...@linux.microsoft.com; Wang, Jian J ; Wu,
> >> Hao A ; Bi, Dandan ;
> >> gaolim...@byosoft.com.cn; Dong, Guo ; Ma,
> Maurice
> >> ; You, Benjamin 
> >> Subject: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings
> >>
> >> Current memory protection settings rely on FixedAtBuild PCD values
> >> (minus PcdSetNxForStack). Because of this, the memory protection
> >> configuration interface is fixed in nature. Cases arise in which memory
> >> protections might need to be adjusted between boots (if platform design
> >> allows) to avoid disabling a system. For example, platforms might choose
> >> to allow the user to control their protection policies such as allow
> >> execution of critical 3rd party software that might violate memory
> >> protections.
> >>
> >> This RFC seeks your feedback regarding introducing an interface that
> >> allows dynamic configuration of memory protection settings.
> >>
> >> I would like to propose two options:
> >> 1. Describing the memory protection setting configuration in a HOB that
> >> is produced by the platform.
> >> 2. Introducing a library class (e.g. MemoryProtectionLib) that allows
> >> abstraction of the memory protection setting configuration data source.
> >>
> >> In addition, I would like to know if the memory protection FixedAtBuild
> >> PCDs currently in MdeModulePkg can be removed so we can move the
> >> configuration interface entirely to an option above.
> >>
> >> In any case, I would like the settings to be visible to environments
> >> such as Standalone MM where dynamic PCDs are not accessible.
> >>
> >> I am seeking your feedback on this proposal in preparation for sending
> >> an edk2 patch series.
> >>
> >> --
> >> Taylor Beebe
> >> Software Engineer @ Microsoft
> 
> --
> Taylor Beebe
> Software Engineer @ Microsoft


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

Re: [edk2-devel] [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

[Taylor Beebe]

Thanks for your feedback, Jian.

In option 2, a most basic implementation would returning the current 
FixedAtBuild PCDs assuming they are kept. If they aren't, the library 
implementer could simply hard-code the return value for each memory 
protection setting.


In option 1, the HOB would be published in pre-mem and I'm not an expert 
on exploiting the pre-mem environment. Jiewen may have more to say on this.


-Taylor

On 7/28/2021 7:18 PM, Wang, Jian J wrote:

Thanks for the RFC. I'm not object to this idea. The only concern from me
is the potential security holes introduced by the changes. According to your
description, it allows 3rd party software to violate memory protection policy.
I'd like to see more explanations on how to avoid it to be exploited.

+Jiewen, what's current process to evaluate the security threat?

Regards,
Jian


-Original Message-
From: Taylor Beebe 
Sent: Friday, July 23, 2021 8:33 AM
To: devel@edk2.groups.io
Cc: spbro...@outlook.com; Dong, Eric ; Ni, Ray
; Kumar, Rahul1 ;
mikub...@linux.microsoft.com; Wang, Jian J ; Wu,
Hao A ; Bi, Dandan ;
gaolim...@byosoft.com.cn; Dong, Guo ; Ma, Maurice
; You, Benjamin 
Subject: [RFC] MemoryProtectionLib for Dynamic Memory Guard Settings

Current memory protection settings rely on FixedAtBuild PCD values
(minus PcdSetNxForStack). Because of this, the memory protection
configuration interface is fixed in nature. Cases arise in which memory
protections might need to be adjusted between boots (if platform design
allows) to avoid disabling a system. For example, platforms might choose
to allow the user to control their protection policies such as allow
execution of critical 3rd party software that might violate memory
protections.

This RFC seeks your feedback regarding introducing an interface that
allows dynamic configuration of memory protection settings.

I would like to propose two options:
1. Describing the memory protection setting configuration in a HOB that
is produced by the platform.
2. Introducing a library class (e.g. MemoryProtectionLib) that allows
abstraction of the memory protection setting configuration data source.

In addition, I would like to know if the memory protection FixedAtBuild
PCDs currently in MdeModulePkg can be removed so we can move the
configuration interface entirely to an option above.

In any case, I would like the settings to be visible to environments
such as Standalone MM where dynamic PCDs are not accessible.

I am seeking your feedback on this proposal in preparation for sending
an edk2 patch series.

--
Taylor Beebe
Software Engineer @ Microsoft


--
Taylor Beebe
Software Engineer @ Microsoft


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

回复: [edk2-devel] [PATCH 00/13] Add ACPI 6.4 header file

[gaoliming]

Chris:
  Please submit one BZ (https://bugzilla.tianocore.org/) for this new
feature. I will review the code. 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Chris Jones
> 发送时间: 2021年7月30日 0:23
> 收件人: devel@edk2.groups.io
> 抄送: sami.muja...@arm.com; akanksha.ja...@arm.com;
> ben.ander...@arm.com; michael.d.kin...@intel.com;
> gaolim...@byosoft.com.cn; zhiguang@intel.com;
> matteo.carl...@arm.com; n...@arm.com
> 主题: [edk2-devel] [PATCH 00/13] Add ACPI 6.4 header file
> 
> This patch series introduces a header file for the latest version of the
> ACPI 6.4 specification, January 2021. This header contains all updates
> to the ACPI specification in addition to addressing a few small errors
> from the previous ACPI header files.
> 
> The changes can be seen at:
> https://github.com/chris-jones-arm/edk2/tree/1661_add_acpi_64_header_v
> 1
> 
> Chris Jones (13):
>   MdePkg: Add ACPI 6.4 header file
>   MdePkg: Increment FADT version
>   MdePkg: Rename SBSA Generic Watchdog to Arm Generic Watchdog
>   MdePkg: Update PMTT to ACPI 6.4
>   MdePkg: Add SPA Location Cookie field to SPA Range structure
>   MdePkg: Remove DPPT table
>   MdePkg: Add flags and MinTransferSize to Generic Initiator
>   MdePkg: Add 'Type 5' PCC structure
>   MdePkg: Add Multiprocessor Wakeup structure
>   MdePkg: Add the Platform Health Assessment Table (PHAT)
>   MdePkg: Add Secure Access Components in the SDEV table
>   MdePkg: Add Cache ID to PPTT
>   MdePkg: Fix broken coding style in Acpi64.h
> 
>  MdePkg/Include/IndustryStandard/Acpi.h   |4 +-
>  MdePkg/Include/IndustryStandard/Acpi64.h | 3148
> 
>  2 files changed, 3150 insertions(+), 2 deletions(-)
>  create mode 100644 MdePkg/Include/IndustryStandard/Acpi64.h
> 
> --
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 
> 
> 
> 
> 





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

[edk2-devel] [PATCH] SecurityPkg: Debug code to audit BIOS TPM extend operations.

[Rodrigo Gonzalez del Cueto]

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

Add debug functionality to examine TPM extend operations
performed by BIOS and inspect the PCR 00 value prior to
any BIOS measurements.

Replaced usage of EFI_D_* for DEBUG_* definitions in debug
messages.

Signed-off-by: Rodrigo Gonzalez del Cueto 
Cc: Jiewen Yao 
Cc: Jian J Wang 
---
 SecurityPkg/Include/Library/Tpm2CommandLib.h   |  28 
++--
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c | 226 
+++---
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c  |  34 
--
 3 files changed, 245 insertions(+), 43 deletions(-)

diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h 
b/SecurityPkg/Include/Library/Tpm2CommandLib.h
index ee8eb62295..5e5c340893 100644
--- a/SecurityPkg/Include/Library/Tpm2CommandLib.h
+++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h
@@ -1,7 +1,7 @@
 /** @file
   This library is used by other modules to send TPM2 command.
 
-Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. 
+Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -505,7 +505,7 @@ EFIAPI
 Tpm2PcrEvent (
   IN  TPMI_DH_PCR   PcrHandle,
   IN  TPM2B_EVENT   *EventData,
- OUT  TPML_DIGEST_VALUES*Digests
+  OUT TPML_DIGEST_VALUES*Digests
   );
 
 /**
@@ -522,10 +522,10 @@ Tpm2PcrEvent (
 EFI_STATUS
 EFIAPI
 Tpm2PcrRead (
-  IN  TPML_PCR_SELECTION*PcrSelectionIn,
- OUT  UINT32*PcrUpdateCounter,
- OUT  TPML_PCR_SELECTION*PcrSelectionOut,
- OUT  TPML_DIGEST   *PcrValues
+  IN   TPML_PCR_SELECTION*PcrSelectionIn,
+  OUT  UINT32*PcrUpdateCounter,
+  OUT  TPML_PCR_SELECTION*PcrSelectionOut,
+  OUT  TPML_DIGEST   *PcrValues
   );
 
 /**
@@ -1113,4 +1113,20 @@ GetDigestFromDigestList(
   OUT VOID  *Digest
   );
 
+  /**
+   This function will query the TPM to determine which hashing algorithms and
+   get the digests of all active and supported PCR banks of a specific PCR 
register.
+
+   @param[in] PcrHandle The index of the PCR register to be read.
+   @param[out]HashList  List of digests from PCR register being read.
+
+   @retval EFI_SUCCESS   The Pcr was read successfully.
+   @retval EFI_DEVICE_ERROR  The command was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+Tpm2PcrReadForActiveBank (
+  IN  TPMI_DH_PCRPcrHandle,
+  OUT TPML_DIGEST*HashList
+  );
 #endif
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c 
b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c
index ddb15178fb..3b49192b93 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c
@@ -1,7 +1,7 @@
 /** @file
   Implement TPM2 Integrity related command.
 
-Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. 
+Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -109,7 +109,6 @@ Tpm2PcrExtend (
   Cmd.Header.commandCode = SwapBytes32(TPM_CC_PCR_Extend);
   Cmd.PcrHandle  = SwapBytes32(PcrHandle);
 
-
   //
   // Add in Auth session
   //
@@ -130,14 +129,26 @@ Tpm2PcrExtend (
 Buffer += sizeof(UINT16);
 DigestSize = GetHashSizeFromAlgo (Digests->digests[Index].hashAlg);
 if (DigestSize == 0) {
-  DEBUG ((EFI_D_ERROR, "Unknown hash algorithm %d\r\n", 
Digests->digests[Index].hashAlg));
+  DEBUG ((DEBUG_ERROR, "Unknown hash algorithm %d\r\n", 
Digests->digests[Index].hashAlg));
   return EFI_DEVICE_ERROR;
 }
+
 CopyMem(
   Buffer,
   >digests[Index].digest,
   DigestSize
   );
+
+DEBUG_CODE_BEGIN ();
+UINTN Index2;
+DEBUG ((DEBUG_VERBOSE, "Tpm2PcrExtend - Hash = 0x%04x, Pcr[%02d], digest = 
", Digests->digests[Index].hashAlg, (UINT8) PcrHandle));
+
+for (Index2 = 0; Index2 < DigestSize; Index2++) {
+  DEBUG ((DEBUG_VERBOSE, "%02x ", Buffer[Index2]));
+}
+DEBUG ((DEBUG_VERBOSE, "\n"));
+DEBUG_CODE_END ();
+
 Buffer += DigestSize;
   }
 
@@ -151,7 +162,7 @@ Tpm2PcrExtend (
   }
 
   if (ResultBufSize > sizeof(Res)) {
-DEBUG ((EFI_D_ERROR, "Tpm2PcrExtend: Failed ExecuteCommand: Buffer Too 
Small\r\n"));
+DEBUG ((DEBUG_ERROR, "Tpm2PcrExtend: Failed ExecuteCommand: Buffer Too 
Small\r\n"));
 return EFI_BUFFER_TOO_SMALL;
   }
 
@@ -160,7 +171,7 @@ Tpm2PcrExtend (
   //
   RespSize = SwapBytes32(Res.Header.paramSize);
   if (RespSize > sizeof(Res)) {
-DEBUG ((EFI_D_ERROR, "Tpm2PcrExtend: Response size too large! %d\r\n", 
RespSize));
+DEBUG 

Re: [edk2-devel] [PATCH v5 00/11] Measured SEV boot with kernel/initrd/cmdline

[Dov Murik]

On 29/07/2021 12:51, Ard Biesheuvel wrote:
> On Wed, 28 Jul 2021 at 19:30, Dov Murik  wrote:
>>
>>
>> On 28/07/2021 19:41, Yao, Jiewen wrote:
>>> For OvmfPkg, reviewed-by: Jiewen Yao 
>>> For ArmVirtPkg, acked-by: Jiewen Yao 
>>>
>>
>> Thanks Jiewen!
>>
> 
> 
> Merged as #1843
> 
> Note that I needed to add CryptoPkg/CryptoPkg.dec to the list of
> acceptable dependencies in OvmfPkg.ci.yaml for the CI checks to be
> able to pass.

Thanks Ard.

-Dov


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

[edk2-devel] [PATCH] Reallocate TPM Active PCRs based on platform support.

[Rodrigo Gonzalez del Cueto]

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

The current implementation of SyncPcrAllocationsAndPcrMask() triggers
PCR bank reallocation only based on the intersection between
TpmActivePcrBanks and PcdTpm2HashMask.

When the software HashLibBaseCryptoRouter solution is used, no PCR bank
reallocation is occurring based on the supported hashing algorithms
registered by the HashLib instances.

Need to have an additional check for the intersection between the
TpmActivePcrBanks and the PcdTcg2HashAlgorithmBitmap populated by the
HashLib instances present on the platform's BIOS.

Change-Id: I1cdabe14a4fb5adfc289a2dd60f1b467c64282ac
Signed-off-by: Rodrigo Gonzalez del Cueto 

Cc: Jian J Wang 
Cc: Jiewen Yao 
---
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c   | 18 +-
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |  1 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c 
b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index 93a8803ff6..5ad6a45cf3 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -262,6 +262,7 @@ SyncPcrAllocationsAndPcrMask (
 {
   EFI_STATUSStatus;
   EFI_TCG2_EVENT_ALGORITHM_BITMAP   TpmHashAlgorithmBitmap;
+  EFI_TCG2_EVENT_ALGORITHM_BITMAP   BiosHashAlgorithmBitmap;
   UINT32TpmActivePcrBanks;
   UINT32NewTpmActivePcrBanks;
   UINT32Tpm2PcrMask;
@@ -273,16 +274,27 @@ SyncPcrAllocationsAndPcrMask (
   // Determine the current TPM support and the Platform PCR mask.
   //
   Status = Tpm2GetCapabilitySupportedAndActivePcrs (, 
);
+
   ASSERT_EFI_ERROR (Status);
+  
+  DEBUG ((EFI_D_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - 
TpmHashAlgorithmBitmap: 0x%08x\n", TpmHashAlgorithmBitmap));
+  DEBUG ((EFI_D_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - 
TpmActivePcrBanks 0x%08x\n", TpmActivePcrBanks));
 
   Tpm2PcrMask = PcdGet32 (PcdTpm2HashMask);
   if (Tpm2PcrMask == 0) {
 //
 // if PcdTPm2HashMask is zero, use ActivePcr setting
 //
+DEBUG ((EFI_D_VERBOSE, "Initializing PcdTpm2HashMask to TpmActivePcrBanks 
0x%08x\n", TpmActivePcrBanks));
 PcdSet32S (PcdTpm2HashMask, TpmActivePcrBanks);
+DEBUG ((EFI_D_VERBOSE, "Initializing Tpm2PcrMask to TpmActivePcrBanks 
0x%08x\n", Tpm2PcrMask));
 Tpm2PcrMask = TpmActivePcrBanks;
   }
+  
+  BiosHashAlgorithmBitmap = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
+  DEBUG ((EFI_D_INFO, "PcdTcg2HashAlgorithmBitmap 0x%08x\n", 
BiosHashAlgorithmBitmap));
+  DEBUG ((EFI_D_INFO, "Tpm2PcrMask 0x%08x\n", Tpm2PcrMask)); // Active PCR 
banks from TPM input
+  DEBUG ((EFI_D_INFO, "TpmActivePcrBanks & BiosHashAlgorithmBitmap = 
0x%08x\n", NewTpmActivePcrBanks));
 
   //
   // Find the intersection of Pcd support and TPM support.
@@ -294,9 +306,12 @@ SyncPcrAllocationsAndPcrMask (
   // If there are active PCR banks that are not supported by the Platform mask,
   // update the TPM allocations and reboot the machine.
   //
-  if ((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) {
+  if (((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) ||
+  ((TpmActivePcrBanks & BiosHashAlgorithmBitmap) != TpmActivePcrBanks)) {
 NewTpmActivePcrBanks = TpmActivePcrBanks & Tpm2PcrMask;
+NewTpmActivePcrBanks &= BiosHashAlgorithmBitmap;
 
+DEBUG ((EFI_D_INFO, "NewTpmActivePcrBanks 0x%08x\n", 
NewTpmActivePcrBanks));
 DEBUG ((EFI_D_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", 
__FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks));
 if (NewTpmActivePcrBanks == 0) {
   DEBUG ((EFI_D_ERROR, "%a - No viable PCRs active! Please set a less 
restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
@@ -331,6 +346,7 @@ SyncPcrAllocationsAndPcrMask (
 }
 
 Status = PcdSet32S (PcdTpm2HashMask, NewTpm2PcrMask);
+DEBUG ((EFI_D_INFO, "Setting PcdTpm2Hash Mask to 0x%08x\n", 
NewTpm2PcrMask));
 ASSERT_EFI_ERROR (Status);
   }
 }
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf 
b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index 06c26a2904..17ad116126 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -86,6 +86,7 @@
   ## SOMETIMES_CONSUMES
   ## SOMETIMES_PRODUCES
   gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap  ## 
CONSUMES
 
 [Depex]
   gEfiPeiMasterBootModePpiGuid AND
-- 
2.31.1.windows.1



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

[edk2-devel] [PATCH 00/13] Add ACPI 6.4 header file

[Chris Jones]

This patch series introduces a header file for the latest version of the
ACPI 6.4 specification, January 2021. This header contains all updates
to the ACPI specification in addition to addressing a few small errors
from the previous ACPI header files.

The changes can be seen at: 
https://github.com/chris-jones-arm/edk2/tree/1661_add_acpi_64_header_v1

Chris Jones (13):
  MdePkg: Add ACPI 6.4 header file
  MdePkg: Increment FADT version
  MdePkg: Rename SBSA Generic Watchdog to Arm Generic Watchdog
  MdePkg: Update PMTT to ACPI 6.4
  MdePkg: Add SPA Location Cookie field to SPA Range structure
  MdePkg: Remove DPPT table
  MdePkg: Add flags and MinTransferSize to Generic Initiator
  MdePkg: Add 'Type 5' PCC structure
  MdePkg: Add Multiprocessor Wakeup structure
  MdePkg: Add the Platform Health Assessment Table (PHAT)
  MdePkg: Add Secure Access Components in the SDEV table
  MdePkg: Add Cache ID to PPTT
  MdePkg: Fix broken coding style in Acpi64.h

 MdePkg/Include/IndustryStandard/Acpi.h   |4 +-
 MdePkg/Include/IndustryStandard/Acpi64.h | 3148 
 2 files changed, 3150 insertions(+), 2 deletions(-)
 create mode 100644 MdePkg/Include/IndustryStandard/Acpi64.h

-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")



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

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Yao, Jiewen]

Indeed. Too many emails.

Glad that we can reach consensus finally. :-)

Thanks, Min and Brijesh.

> -Original Message-
> From: Xu, Min M 
> Sent: Thursday, July 29, 2021 9:22 PM
> To: Yao, Jiewen ; Brijesh Singh
> ; devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Justen, Jordan L
> ; Erdem Aktas ; James
> Bottomley ; Tom Lendacky 
> Subject: RE: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
> ResetVector
> 
> On July 29, 2021 8:13 PM, Yao Jiewen wrote:
> > Hey
> > I am not sure why Min did not response to my latest email.
> > I did give suggestion in my previous comment.
> >
> Ah, sorry I missed it. There are too many mails.
> > =
> > CcWorkArea.Type = 0;
> > InitCcWorkAreaSev(); // set Type=1 if SEV InitCcWorkAreaTdx(); // set Type=2
> if
> > TDX =
> >
> > That is option 1.
> >
> > Thank you
> > Yao Jiewen
> >
> > > -Original Message-
> > > From: Xu, Min M 
> > > Sent: Thursday, July 29, 2021 7:54 PM
> > > To: Brijesh Singh ; Yao, Jiewen
> > > ; devel@edk2.groups.io
> > > Cc: Ard Biesheuvel ; Justen, Jordan L
> > > ; Erdem Aktas ;
> > > James Bottomley ; Tom Lendacky
> > > 
> > > Subject: RE: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
> > > ResetVector
> > >
> > > On July 29, 2021 6:08 PM, Brijesh Singh wrote:
> > > > On 7/29/21 1:07 AM, Xu, Min M wrote:
> > > > > On July 29, 2021 12:29 PM, Brijesh Singh wrote:
> > > > >> On 7/28/21 9:44 PM, Xu, Min M wrote:
> > > > >>> Jiewen & Singh
> > > > >>>
> > > > >>> From the discussion I am thinking we have below rules to follow
> > > > >>> to the design the structure of TEE_WORK_AREA:
> > > > >>> 1. Design should be flexible but not too complicated 2. Reuse
> > > > >>> the current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as
> > > > TEE_WORK_AREA 3.
> > > > >>> TEE_WORK_AREA should be initialized to all-0 at the beginning of
> > > > >>> ResetVecotr 4. Reduce the changes to exiting code if possible
> > > > >>>
> > > > >>> So I try to make below conclusions below: (Please review) 1.
> > > > >>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and
> > > > SEV,
> > > > >>> maybe in the future it can be used by other CC technologies.
> > > > >>>
> > > > >>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
> > > > >>> guaranteed to be cleared in legacy guest. In TDX this memory
> > > > >>> region is initialized to be all-0 by host VMM. In SEV the memory
> > > > >>> region is
> > > > cleared as well.
> > > > >>>   0x00B000|0x001000
> > > > >>>
> > > > >>
> > > >
> > gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa
> > > > ce
> > > > >> Guid.PcdSevEsWorkAreaSize
> > > > >>>   DATA = {
> > > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> > > > >>>   }
> > > > >> Hmm, I thought the contents of the data pages are controlled by
> > > > >> the host
> > > > VMM.
> > > > >> If the backing pages are not zero filled then there is no
> > > > >> guarantee that memory will be zero.  To verify it:
> > > > >>
> > > > >> 1. I applied your above change in OvmfPkgX86.fdt. I modified the
> > > > >> DATA values from 0x00 -> 0xCC
> > > > >>
> > > > >> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
> > > > >>
> > > > >> And dump does not contain the 0xcc.
> > > > >>
> > > > >> And to confirm further,  I attached to the qemu with the GDB
> > > > >> before the booting the OVMF, and modified the SevEsWorkArea with
> > > > >> some garbage number  and this time the dump printed garbage value
> > > > >> I put
> > > > through the debugger.
> > > > >>
> > > > >> In summary, the OVMF to zero the workarea memory on the entry and
> > > > we
> > > > >> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
> > > > > So in legacy guest, CCWorkArea is cleared to all-0 without the
> > > > DATA={0x00,0x00...}, right?
> > > >
> > > > Okay, maybe I was not able to communicate it correctly.
> > > >
> > > > The run I did is for the legacy guest. For the legacy guest, the
> > > > contents of the CCWorkArea may *not* be always zero even when you
> > > > use the DATA={0x00, 0x00...}.
> > > >
> > > > Currently, Qemu uses zero filled backing pages, so we will get a
> > > > zero filled CCWorkArea; but nothing says that a backing page *must* be
> > zero.
> > > > Another VMM may choose to do things differently. In summary, the
> > > > OVMF reset vector code must zero  the CCWorkArea  before calling SEV
> > > > or TDX probes.
> > > >
> > > Ah, I see.
> > > In current CheckSevFeatures, byte[SEV_ES_WORK_AREA] is cleared to0.
> > > Then its values is set based on the result of SEV probe.
> > >
> > > There is a bug here. CheckTdxFeatures does the similar work and it
> > > sets the WORK_AREA to 2. If CheckSevFeatures is called after
> > > CheckTdxFeatures, then WORK_AREA is cleared and it is set to 0 because
> > > it 

[edk2-devel] [edk2-platforms PATCH v1 1/1] Platform/Intel/SimicsOpenBoardPkg: Fix PCD type of PcdVideo*Resolution

[Takuto Naito]

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

PcdVideoHorizontalResolution and PcdVideoVerticalResolutio are set in
the SimicsDxe module and consumed by the other module GraphicsConsoleDxe.
In this case, the type of these PCDs should be Dynamic.

Cc: Agyeman Prince 
Signed-off-by: Takuto Naito 
---
 .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc 
b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
index 251f46f812..88009b8f10 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc
@@ -221,8 +221,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmiHandlerProfilePropertyMask|0x1
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable|TRUE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|1024
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gPcAtChipsetPkgTokenSpaceGuid.PcdHpetBaseAddress|0xFED0
 
   ##
@@ -238,6 +236,8 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|1024
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
 
   ##
   # Board Configuration
-- 
2.25.1



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

[edk2-devel] [edk2-platforms PATCH v1 0/1] Platform/Intel/SimicsOpenBoardPkg: Fix PCD type of PcdVideo*Resolution

[Takuto Naito]

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

v1:
https://github.com/naitaku/edk2-platforms/tree/bug3514_v1

Cc: Agyeman Prince 

Takuto Naito (1):
  Platform/Intel/SimicsOpenBoardPkg: Fix PCD type of PcdVideo*Resolution

 .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkgPcd.dsc  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.25.1



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

Re: [edk2-devel] ArmPkg: TranslationTable exceeding TempRam on virtual systems

[Arti Gupta via groups.io]

Hi,
Hoping to find a solution here. Would someone mind helping me with this?
Thanks a lot,
Arti

From: Arti Gupta
Sent: Thursday, July 1, 2021 2:35 AM
To: Bret Barkelew ; devel@edk2.groups.io
Subject: RE: ArmPkg: TranslationTable exceeding TempRam on virtual systems

Hey everyone,
Wondering if there are more thoughts on the questions Bret has posted below?
Thanks,
Arti

From: Bret Barkelew 
mailto:bret.barke...@microsoft.com>>
Sent: Thursday, May 27, 2021 4:29 PM
To: devel@edk2.groups.io; Arti Gupta 
mailto:a...@microsoft.com>>
Subject: ArmPkg: TranslationTable exceeding TempRam on virtual systems

I'm fielding a series of questions coming out of our compatriot team that deals 
with virtual FW (for HyperV). They're seeing ARM64 systems that have lots of 
RAM vastly exceeding the TempRam that's passed into the system due to HOB 
allocations to create all the 4k entries for the early page tables.

Is this a known limitation or are we doing something dumb?
Is there a way to sparsely populate the page tables and fill them in more in 
DXE?
I've seen some questions about 64k pages on the list before. Is that an option?

Thanks in advance!

- Bret



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

[edk2-devel] [edk2 PATCH] MdeModulePkg: Fix typo in error message

[Seonghyun Park]

Fix typo in error message in CapsuleApp.

Signed-off-by: Seonghyun Park 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
index dba50b3202..712cf2e1f7 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
@@ -509,7 +509,7 @@ GetUpdateFileSystem (
 DevicePath = DuplicateDevicePath (MappedDevicePath);
 Status = GetEfiSysPartitionFromDevPath (DevicePath, , Fs);
 if (EFI_ERROR (Status)) {
-  Print (L"Error: Cannot get EFI system partiion from '%s' - %r\n", Map, 
Status);
+  Print (L"Error: Cannot get EFI system partition from '%s' - %r\n", Map, 
Status);
   return EFI_NOT_FOUND;
 }
 Print (L"Warning: Cannot find Boot Option on '%s'!\n", Map);
-- 
2.32.0



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

Re: [edk2-devel] Proposing a new area of the edk2-test repository

[Nelson, Eric]

Adding ResumeOK.efi tool under /edk2-test/test-tools/TestToolsPkg would be 
great.

Should I propose this in the RFC and DEVEL mailing lists as a next step?

Thanks,
__e


From: Samer El-Haj-Mahmoud 
Sent: Friday, July 9, 2021 1:12 PM
To: Bret Barkelew ; devel@edk2.groups.io; Nelson, 
Eric ; G Edhaya Chandran ; 
gao...@byosoft.com.cn; Kinney, Michael D 
Subject: RE: Proposing a new area of the edk2-test repository

Interesting, thanks for sharing Bret. Some of those tests seem to be x64 
specific (SMM tests), and some can be more generic like MorLockTestApp

Like I said earlier, I am not against adding test tools to edk2-test. That in 
fact is welcomed, especially if their usefulness in validating the solutions 
extend beyond specific implementations.

What would a good tree structure look like to accommodate misc tools? Today we 
have

/edk2-test/uefi-sct/SctPkg

How about something like this?
/edk2-test/test-tools/TestToolsPkg
or /edk2-test/ TestToolsPkg

The "ResumeOK" can be placed there

Any other ideas?


From: Bret Barkelew 
mailto:bret.barke...@microsoft.com>>
Sent: Thursday, June 24, 2021 12:25 AM
To: devel@edk2.groups.io; 
eric.nel...@intel.com; Samer El-Haj-Mahmoud 
mailto:samer.el-haj-mahm...@arm.com>>; G Edhaya 
Chandran mailto:edhaya.chand...@arm.com>>; 
gao...@byosoft.com.cn; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Subject: RE: Proposing a new area of the edk2-test repository

Fun fact! Mu also has a number of apps and things that we could work on moving 
to EDK2 if there were a suitable location. Right now, many of them are here:
mu_plus/UefiTestingPkg at release/202102 * microsoft/mu_plus 
(github.com)

- Bret

From: Nelson, Eric via groups.io
Sent: Wednesday, June 23, 2021 3:38 PM
To: Samer El-Haj-Mahmoud; G Edhaya 
Chandran; 
gao...@byosoft.com.cn; 
devel@edk2.groups.io; Kinney, Michael 
D
Subject: [EXTERNAL] Re: [edk2-devel] Proposing a new area of the edk2-test 
repository


I have created a few other internal apps that build under WinTestPkg, although 
ResumeOK.efi is the only one I have received permissions to release sources for 
at this time.
And yes, they are primarily intended for validating Windows requirements.
I had some issues with my apps, needing to use different libraries than 
MdeModulePkg, and found it easier to create my own package, and use the libs I 
want.

__e


From: Samer El-Haj-Mahmoud 
mailto:samer.el-haj-mahm...@arm.com>>
Sent: Wednesday, June 23, 2021 1:56 PM
To: Nelson, Eric mailto:eric.nel...@intel.com>>; G 
Edhaya Chandran mailto:edhaya.chand...@arm.com>>; 
gao...@byosoft.com.cn; 
devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud 
mailto:samer.el-haj-mahm...@arm.com>>
Subject: RE: Proposing a new area of the edk2-test repository

+edk2 list

I am not against adding additional test tools to edk2-test. Just feel like 
there is a need to organize and have a strategy, rather than just use edk2-test 
as a dumping group of miscellaneous tools.

There is already a place for apps under 
https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Application

We also have a number of EDK2 misc applications that use edk2-libc in 
https://github.com/tianocore/edk2-libc/tree/master/AppPkg/Applications

A couple of questions:

  *   Do you expect more apps from WinTestPkg to be contributed to TianoCore? 
And are they all around testing specific Windows requirements? If so, then 
having an edk2-test/WinTestPkg makes sense to me, as you will have a collection 
of useful testing app targeting specific area.

 *   But what about other OSes?

  *   If this is a one-off test app and other WinTestPkg apps are not going to 
be contributed, then does it make sense to put this under 

[edk2-devel] [PATCH] MdeModulePkg: CapsuleApp: Fix typo

[Seonghyun Park via groups.io]

Fix typo in comment

Signed-off-by: Seonghyun Park 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
index dba50b3202..712cf2e1f7 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
@@ -509,7 +509,7 @@ GetUpdateFileSystem (
 DevicePath = DuplicateDevicePath (MappedDevicePath);
 Status = GetEfiSysPartitionFromDevPath (DevicePath, , Fs);
 if (EFI_ERROR (Status)) {
-  Print (L"Error: Cannot get EFI system partiion from '%s' - %r\n", Map, 
Status);
+  Print (L"Error: Cannot get EFI system partition from '%s' - %r\n", Map, 
Status);
   return EFI_NOT_FOUND;
 }
 Print (L"Warning: Cannot find Boot Option on '%s'!\n", Map);
--
2.32.0




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

[edk2-devel] NetworkPkg: NetRandomInitSeed random seed generation

[Arti Gupta via groups.io]

Hello,
While reviewing the code for NetRandomInitSeed in the DDxeNetLib, I see that it 
uses the time of day for random seed generation instead of something like 
RDRAND. Is there a reason for NetRandomInitSeed to do it this way? Also, there 
is no error status checking in the code if GetTime fails.
Thanks,
Arti


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78373): https://edk2.groups.io/g/devel/message/78373
Mute This Topic: https://groups.io/mt/84528986/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] IntelSiliconPkg/VTd: Fix variables may be used uninitialized

[Chaganty, Rangasai V]

Reviewed-by: Sai Chaganty 

-Original Message-
From: Hsu, WesleyX  
Sent: Thursday, July 29, 2021 12:23 AM
To: devel@edk2.groups.io
Cc: Hsu, WesleyX ; Chan, Amy ; Yeh, 
HerbX ; Peng, NickX ; Ni, Ray 
; Chaganty, Rangasai V 
Subject: [PATCH v2] IntelSiliconPkg/VTd: Fix variables may be used uninitialized

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

Initialize several variables which may be used uninitialized after adding 
"-ffat-lto-objects" option in GCC5 tool chain.

Change-Id: Ib2684aa70637d449f8bbddb18cf0a458a2742909
Signed-off-by: WesleyX Hsu 
Cc: Amy Chan 
Cc: HerbX Yeh 
Cc: NickX Peng 
Cc: Ray Ni 
Cc: Rangasai V Chaganty 
---
 Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c | 
7 ++-
 Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c | 
9 +++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
index 341e2beb..6676b2a9 100644
--- 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/Translat
+++ ionTable.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2020, Intel Corporation. All rights reserved.
+  Copyright (c) 2020 - 2021, Intel Corporation. All rights 
+ reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -111,6 +111,11 @@ CreateSecondLevelPagingEntryTable (
 return EFI_SUCCESS;
   }
 
+  Lvl4PagesStart = 0;
+  Lvl4PagesEnd   = 0;
+  Lvl4PtEntry= NULL;
+  Lvl5PtEntry= NULL;
+
   BaseAddress = ALIGN_VALUE_LOW (MemoryBase, SIZE_2MB);
   EndAddress = ALIGN_VALUE_UP (MemoryLimit, SIZE_2MB);
   DEBUG ((DEBUG_INFO, "CreateSecondLevelPagingEntryTable: BaseAddress - 
0x%016lx, EndAddress - 0x%016lx\n", BaseAddress, EndAddress)); diff --git 
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c 
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
index d152039f..ca5f65a8 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationT
+++ able.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2017 - 2021, Intel Corporation. All rights 
+ reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -133,7 +133,7 @@ CreateContextEntry (
   mVtdUnitInformation[VtdIndex].Is5LevelPaging = TRUE;
   if ((mAcpiDmarTable->HostAddressWidth <= 48) &&
   ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) != 0)) {
-mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
+mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
   }
 } else if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) == 0) {
   DEBUG((DEBUG_ERROR, " Page-table type is not supported on VTD %d 
\n", VtdIndex)); @@ -199,6 +199,11 @@ CreateSecondLevelPagingEntryTable (
 return EFI_SUCCESS;
   }
 
+  Lvl4PagesStart = 0;
+  Lvl4PagesEnd   = 0;
+  Lvl4PtEntry= NULL;
+  Lvl5PtEntry= NULL;
+
   BaseAddress = ALIGN_VALUE_LOW(MemoryBase, SIZE_2MB);
   EndAddress = ALIGN_VALUE_UP(MemoryLimit, SIZE_2MB);
   DEBUG ((DEBUG_INFO,"CreateSecondLevelPagingEntryTable: BaseAddress - 
0x%016lx, EndAddress - 0x%016lx\n", BaseAddress, EndAddress));
--
2.32.0.windows.2



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

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Min Xu]

On July 29, 2021 8:13 PM, Yao Jiewen wrote:
> Hey
> I am not sure why Min did not response to my latest email.
> I did give suggestion in my previous comment.
> 
Ah, sorry I missed it. There are too many mails. 
> =
> CcWorkArea.Type = 0;
> InitCcWorkAreaSev(); // set Type=1 if SEV InitCcWorkAreaTdx(); // set Type=2 
> if
> TDX =
> 
> That is option 1.
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: Xu, Min M 
> > Sent: Thursday, July 29, 2021 7:54 PM
> > To: Brijesh Singh ; Yao, Jiewen
> > ; devel@edk2.groups.io
> > Cc: Ard Biesheuvel ; Justen, Jordan L
> > ; Erdem Aktas ;
> > James Bottomley ; Tom Lendacky
> > 
> > Subject: RE: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
> > ResetVector
> >
> > On July 29, 2021 6:08 PM, Brijesh Singh wrote:
> > > On 7/29/21 1:07 AM, Xu, Min M wrote:
> > > > On July 29, 2021 12:29 PM, Brijesh Singh wrote:
> > > >> On 7/28/21 9:44 PM, Xu, Min M wrote:
> > > >>> Jiewen & Singh
> > > >>>
> > > >>> From the discussion I am thinking we have below rules to follow
> > > >>> to the design the structure of TEE_WORK_AREA:
> > > >>> 1. Design should be flexible but not too complicated 2. Reuse
> > > >>> the current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as
> > > TEE_WORK_AREA 3.
> > > >>> TEE_WORK_AREA should be initialized to all-0 at the beginning of
> > > >>> ResetVecotr 4. Reduce the changes to exiting code if possible
> > > >>>
> > > >>> So I try to make below conclusions below: (Please review) 1.
> > > >>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and
> > > SEV,
> > > >>> maybe in the future it can be used by other CC technologies.
> > > >>>
> > > >>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
> > > >>> guaranteed to be cleared in legacy guest. In TDX this memory
> > > >>> region is initialized to be all-0 by host VMM. In SEV the memory
> > > >>> region is
> > > cleared as well.
> > > >>>   0x00B000|0x001000
> > > >>>
> > > >>
> > >
> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa
> > > ce
> > > >> Guid.PcdSevEsWorkAreaSize
> > > >>>   DATA = {
> > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> > > >>>   }
> > > >> Hmm, I thought the contents of the data pages are controlled by
> > > >> the host
> > > VMM.
> > > >> If the backing pages are not zero filled then there is no
> > > >> guarantee that memory will be zero.  To verify it:
> > > >>
> > > >> 1. I applied your above change in OvmfPkgX86.fdt. I modified the
> > > >> DATA values from 0x00 -> 0xCC
> > > >>
> > > >> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
> > > >>
> > > >> And dump does not contain the 0xcc.
> > > >>
> > > >> And to confirm further,  I attached to the qemu with the GDB
> > > >> before the booting the OVMF, and modified the SevEsWorkArea with
> > > >> some garbage number  and this time the dump printed garbage value
> > > >> I put
> > > through the debugger.
> > > >>
> > > >> In summary, the OVMF to zero the workarea memory on the entry and
> > > we
> > > >> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
> > > > So in legacy guest, CCWorkArea is cleared to all-0 without the
> > > DATA={0x00,0x00...}, right?
> > >
> > > Okay, maybe I was not able to communicate it correctly.
> > >
> > > The run I did is for the legacy guest. For the legacy guest, the
> > > contents of the CCWorkArea may *not* be always zero even when you
> > > use the DATA={0x00, 0x00...}.
> > >
> > > Currently, Qemu uses zero filled backing pages, so we will get a
> > > zero filled CCWorkArea; but nothing says that a backing page *must* be
> zero.
> > > Another VMM may choose to do things differently. In summary, the
> > > OVMF reset vector code must zero  the CCWorkArea  before calling SEV
> > > or TDX probes.
> > >
> > Ah, I see.
> > In current CheckSevFeatures, byte[SEV_ES_WORK_AREA] is cleared to0.
> > Then its values is set based on the result of SEV probe.
> >
> > There is a bug here. CheckTdxFeatures does the similar work and it
> > sets the WORK_AREA to 2. If CheckSevFeatures is called after
> > CheckTdxFeatures, then WORK_AREA is cleared and it is set to 0 because
> > it is not SEV. The value is override.
> >
> > I think there are 2 options:
> > Option 1:
> > Neither CheckTdxFeatures nor CheckSevFeatures should clear WORK_AREA.
> > Instead
> > It should be cleared to 0 outside and before these 2 calls. So in
> > Main16 after TransitionFromReal16To32BitFlat WORK_AREA is cleared to
> > 0. In Tdx guest this WORK_AREA is initialized to 0 by host VMM.
> >
> > Option 2:
> > Another option is to figure out a mechanism that only one
> > CheckXXXFeatures is called.
> > Since there are 2 entry point in Main.asm: Main16 and Main32.
> > In Main16 CheckSevFeatures is called after TransitionFromReal16To32BitFlat.
> 

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Brijesh Singh via groups.io]

On 7/29/21 7:12 AM, Yao, Jiewen wrote:
> Hey
> I am not sure why Min did not response to my latest email.
> I did give suggestion in my previous comment.
>
> =
> CcWorkArea.Type = 0;
> InitCcWorkAreaSev(); // set Type=1 if SEV
> InitCcWorkAreaTdx(); // set Type=2 if TDX
> =
>
> That is option 1.

Yes that is exactly what we want Jiewen. 

The OvmfPkg reset vector should initialize the type to zero on entry,
and SEV/TDX will update the value (only if the feature is detected).


> Thank you
> Yao Jiewen
>
>> -Original Message-
>> From: Xu, Min M 
>> Sent: Thursday, July 29, 2021 7:54 PM
>> To: Brijesh Singh ; Yao, Jiewen
>> ; devel@edk2.groups.io
>> Cc: Ard Biesheuvel ; Justen, Jordan L
>> ; Erdem Aktas ; James
>> Bottomley ; Tom Lendacky 
>> Subject: RE: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
>> ResetVector
>>
>> On July 29, 2021 6:08 PM, Brijesh Singh wrote:
>>> On 7/29/21 1:07 AM, Xu, Min M wrote:
 On July 29, 2021 12:29 PM, Brijesh Singh wrote:
> On 7/28/21 9:44 PM, Xu, Min M wrote:
>> Jiewen & Singh
>>
>> From the discussion I am thinking we have below rules to follow to
>> the design the structure of TEE_WORK_AREA:
>> 1. Design should be flexible but not too complicated 2. Reuse the
>> current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as
>>> TEE_WORK_AREA 3.
>> TEE_WORK_AREA should be initialized to all-0 at the beginning of
>> ResetVecotr 4. Reduce the changes to exiting code if possible
>>
>> So I try to make below conclusions below: (Please review) 1.
>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and
>>> SEV,
>> maybe in the future it can be used by other CC technologies.
>>
>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
>> guaranteed to be cleared in legacy guest. In TDX this memory region
>> is initialized to be all-0 by host VMM. In SEV the memory region is
>>> cleared as well.
>>   0x00B000|0x001000
>>
>>> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa
>>> ce
> Guid.PcdSevEsWorkAreaSize
>>   DATA = {
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>   }
> Hmm, I thought the contents of the data pages are controlled by the host
>>> VMM.
> If the backing pages are not zero filled then there is no guarantee
> that memory will be zero.  To verify it:
>
> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA
> values from 0x00 -> 0xCC
>
> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
>
> And dump does not contain the 0xcc.
>
> And to confirm further,  I attached to the qemu with the GDB before
> the booting the OVMF, and modified the SevEsWorkArea with some
> garbage number  and this time the dump printed garbage value I put
>>> through the debugger.
> In summary, the OVMF to zero the workarea memory on the entry and
>>> we
> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
 So in legacy guest, CCWorkArea is cleared to all-0 without the
>>> DATA={0x00,0x00...}, right?
>>>
>>> Okay, maybe I was not able to communicate it correctly.
>>>
>>> The run I did is for the legacy guest. For the legacy guest, the contents 
>>> of the
>>> CCWorkArea may *not* be always zero even when you use the DATA={0x00,
>>> 0x00...}.
>>>
>>> Currently, Qemu uses zero filled backing pages, so we will get a zero filled
>>> CCWorkArea; but nothing says that a backing page *must* be zero.
>>> Another VMM may choose to do things differently. In summary, the OVMF
>>> reset vector code must zero  the CCWorkArea  before calling SEV or TDX
>>> probes.
>>>
>> Ah, I see.
>> In current CheckSevFeatures, byte[SEV_ES_WORK_AREA] is cleared to0.
>> Then its values is set based on the result of SEV probe.
>>
>> There is a bug here. CheckTdxFeatures does the similar work and it sets the
>> WORK_AREA to 2. If CheckSevFeatures is called after CheckTdxFeatures, then
>> WORK_AREA is cleared and it is set to 0 because it is not SEV. The value is
>> override.
>>
>> I think there are 2 options:
>> Option 1:
>> Neither CheckTdxFeatures nor CheckSevFeatures should clear WORK_AREA.
>> Instead
>> It should be cleared to 0 outside and before these 2 calls. So in Main16 
>> after
>> TransitionFromReal16To32BitFlat WORK_AREA is cleared to 0. In Tdx guest this
>> WORK_AREA
>> is initialized to 0 by host VMM.
>>
>> Option 2:
>> Another option is to figure out a mechanism that only one CheckXXXFeatures is
>> called.
>> Since there are 2 entry point in Main.asm: Main16 and Main32.
>> In Main16 CheckSevFeatures is called after TransitionFromReal16To32BitFlat.
>> (eax should
>> be saved because it is used in SetCr3ForPageTables64)
>> In Main32 CheckTdxFeatures is called after ReloadFlat32.

Re: [edk2-devel] [EXTERNAL] RE: [edk2-platforms][PATCH V2] PurleyOpenBoardPkg : Support for LINUX Boot

[manickavasakam karpagavinayagam]

Nate :

If you see in this patch, linux.efi/initrd.cpio.xz are dummy files. These dummy 
files needs to be replaced by building the Linux Kernel.

How to build Linux Kernel is mentioned in the ReadMe document which is part of 
this patch.

1.  Follow directions on http://osresearch.net/Building/ to compile the 
heads kernel and initrd for qemu-system_x86_64
2.  Copy the following built files
(1) initrd.cpio.xz  to LinuxBootPkg/LinuxBinaries/initrd.cpio.xz
(2) bzimage to LinuxBootPkg/LinuxBinaries/linux.efi

Thank you

-Manic

-Original Message-
From: Desimone, Nathaniel L 
Sent: Thursday, July 29, 2021 3:17 AM
To: Manickavasakam Karpagavinayagam ; 
devel@edk2.groups.io
Cc: Oram, Isaac W ; Felix Polyudov ; 
Harikrishna Doppalapudi ; Manish Jha ; 
Zachary Bobroff ; Manickavasakam Karpagavinayagam 

Subject: [EXTERNAL] RE: [edk2-platforms][PATCH V2] PurleyOpenBoardPkg : Support 
for LINUX Boot


**CAUTION: The e-mail below is from an external source. Please exercise caution 
before opening attachments, clicking links, or following guidance.**

Hi Manic,

Unfortunately this patch cannot be merged as is. It appears to contain a 
pre-built binary of the Linux kernel that has been pre-configured for Linuxboot 
use. While this is very convenient, the Linux kernel is licensed under the GPL 
and hence we cannot add it to edk2-platforms, which must be kept as BSD only. 
It might be possible to add this to edk2-non-osi, but even then we would 
require that you provide a readme file that explains how to get and compile the 
source code that you used to build this exact Linux image, as required by the 
GPL.

The easiest and safest option would be to remove the Linux binary all together 
and provide instructions to the user for how to build their own image and add 
it to the tree.

Thanks,
Nate

> -Original Message-
> From: manickavasakam karpagavinayagam 
> Sent: Wednesday, June 30, 2021 2:57 PM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W ; Desimone, Nathaniel L
> ; fel...@ami.com; DOPPALAPUDI,
> HARIKRISHNA ; Jha, Manish ;
> Bobroff, Zachary ; KARPAGAVINAYAGAM, MANICKAVASAKAM
> 
> Subject: [edk2-platforms][PATCH V2] PurleyOpenBoardPkg : Support for
> LINUX Boot
>
> Support for LINUX Boot
> To enable/disable feature, PcdLinuxBootEnable can be used
> 1.Follow directions on 
> https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fosresearch.net%2FBuilding%2Fdata=04%7C01%7Cmanickavasakamk%40ami.com%7C3a64470864e64ee9f28d08d95260d55c%7C27e97857e15f486cb58e86c2b3040f93%7C1%7C0%7C637631398142258756%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=ylzkAVWRzAtPBLq%2FFLqn1i4Y%2Fa0o%2FjR%2B7GpsfWI6OCk%3Dreserved=0
>  to compile the
> heads kernel and initrd for qemu-system_x86_64
> 2.Copy the following built files
> (1) initrd.cpio.xz  to
> PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/initrd.cpio.xz
> (2) bzimage to
> PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/linux.efi
>
> Notes:
> V2 :
> - Rename LinuxBootPkg to LinuxBoot
> - Move LinuxBootPkg to PurleyOpenBoardPkg/Features/LinuxBoot
>   - Follow Coding Standard in LinuxBoot.C and LinuxBoot.h
>
> Signed-off-by: manickavasakam karpagavinayagam
> 
> ---
>  .../BoardTiogaPass/CoreDxeInclude.dsc |   5 +-
>  .../BoardTiogaPass/CoreUefiBootInclude.fdf|   5 +-
>  .../BoardTiogaPass/OpenBoardPkg.dsc   |   7 +
>  .../BoardTiogaPass/OpenBoardPkg.fdf   |  57 ++-
>  .../BoardTiogaPass/PlatformPkgConfig.dsc  |   7 +
>  .../LinuxBoot/LinuxBinaries/LinuxKernel.inf   |  17 +
>  .../LinuxBoot/LinuxBinaries/initrd.cpio.xz| Bin 0 -> 16 bytes
>  .../LinuxBoot/LinuxBinaries/linux.efi | Bin 0 -> 16 bytes
>  .../Features/LinuxBoot/LinuxBoot.c| 412 ++
>  .../Features/LinuxBoot/LinuxBoot.h| 185 
>  .../Features/LinuxBoot/LinuxBoot.inf  |  40 ++
>  .../Features/LinuxBoot/LinuxBootNull.c|  36 ++
>  .../Features/LinuxBoot/LinuxBootNull.inf  |  25 ++
>  .../Intel/PurleyOpenBoardPkg/OpenBoardPkg.dec |   2 +
>  .../DxePlatformBootManagerLib/BdsPlatform.c   |   9 +
>  .../DxePlatformBootManagerLib.inf |   2 +
>  Platform/Intel/Readme.md  |  42 ++
>  17 files changed, 843 insertions(+), 8 deletions(-)  create mode
> 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/Lin
> u
> xKernel.inf
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/ini
> tr
> d.cpio.xz
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/lin
> u
> x.efi
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBoot.c
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBoot.h
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBoot.inf
>  create mode 100644
> 

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Yao, Jiewen]

Hey
I am not sure why Min did not response to my latest email.
I did give suggestion in my previous comment.

=
CcWorkArea.Type = 0;
InitCcWorkAreaSev(); // set Type=1 if SEV
InitCcWorkAreaTdx(); // set Type=2 if TDX
=

That is option 1.

Thank you
Yao Jiewen

> -Original Message-
> From: Xu, Min M 
> Sent: Thursday, July 29, 2021 7:54 PM
> To: Brijesh Singh ; Yao, Jiewen
> ; devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Justen, Jordan L
> ; Erdem Aktas ; James
> Bottomley ; Tom Lendacky 
> Subject: RE: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
> ResetVector
> 
> On July 29, 2021 6:08 PM, Brijesh Singh wrote:
> > On 7/29/21 1:07 AM, Xu, Min M wrote:
> > > On July 29, 2021 12:29 PM, Brijesh Singh wrote:
> > >> On 7/28/21 9:44 PM, Xu, Min M wrote:
> > >>> Jiewen & Singh
> > >>>
> > >>> From the discussion I am thinking we have below rules to follow to
> > >>> the design the structure of TEE_WORK_AREA:
> > >>> 1. Design should be flexible but not too complicated 2. Reuse the
> > >>> current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as
> > TEE_WORK_AREA 3.
> > >>> TEE_WORK_AREA should be initialized to all-0 at the beginning of
> > >>> ResetVecotr 4. Reduce the changes to exiting code if possible
> > >>>
> > >>> So I try to make below conclusions below: (Please review) 1.
> > >>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and
> > SEV,
> > >>> maybe in the future it can be used by other CC technologies.
> > >>>
> > >>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
> > >>> guaranteed to be cleared in legacy guest. In TDX this memory region
> > >>> is initialized to be all-0 by host VMM. In SEV the memory region is
> > cleared as well.
> > >>>   0x00B000|0x001000
> > >>>
> > >>
> > gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa
> > ce
> > >> Guid.PcdSevEsWorkAreaSize
> > >>>   DATA = {
> > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> > >>>   }
> > >> Hmm, I thought the contents of the data pages are controlled by the host
> > VMM.
> > >> If the backing pages are not zero filled then there is no guarantee
> > >> that memory will be zero.  To verify it:
> > >>
> > >> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA
> > >> values from 0x00 -> 0xCC
> > >>
> > >> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
> > >>
> > >> And dump does not contain the 0xcc.
> > >>
> > >> And to confirm further,  I attached to the qemu with the GDB before
> > >> the booting the OVMF, and modified the SevEsWorkArea with some
> > >> garbage number  and this time the dump printed garbage value I put
> > through the debugger.
> > >>
> > >> In summary, the OVMF to zero the workarea memory on the entry and
> > we
> > >> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
> > > So in legacy guest, CCWorkArea is cleared to all-0 without the
> > DATA={0x00,0x00...}, right?
> >
> > Okay, maybe I was not able to communicate it correctly.
> >
> > The run I did is for the legacy guest. For the legacy guest, the contents 
> > of the
> > CCWorkArea may *not* be always zero even when you use the DATA={0x00,
> > 0x00...}.
> >
> > Currently, Qemu uses zero filled backing pages, so we will get a zero filled
> > CCWorkArea; but nothing says that a backing page *must* be zero.
> > Another VMM may choose to do things differently. In summary, the OVMF
> > reset vector code must zero  the CCWorkArea  before calling SEV or TDX
> > probes.
> >
> Ah, I see.
> In current CheckSevFeatures, byte[SEV_ES_WORK_AREA] is cleared to0.
> Then its values is set based on the result of SEV probe.
> 
> There is a bug here. CheckTdxFeatures does the similar work and it sets the
> WORK_AREA to 2. If CheckSevFeatures is called after CheckTdxFeatures, then
> WORK_AREA is cleared and it is set to 0 because it is not SEV. The value is
> override.
> 
> I think there are 2 options:
> Option 1:
> Neither CheckTdxFeatures nor CheckSevFeatures should clear WORK_AREA.
> Instead
> It should be cleared to 0 outside and before these 2 calls. So in Main16 after
> TransitionFromReal16To32BitFlat WORK_AREA is cleared to 0. In Tdx guest this
> WORK_AREA
> is initialized to 0 by host VMM.
> 
> Option 2:
> Another option is to figure out a mechanism that only one CheckXXXFeatures is
> called.
> Since there are 2 entry point in Main.asm: Main16 and Main32.
> In Main16 CheckSevFeatures is called after TransitionFromReal16To32BitFlat.
> (eax should
> be saved because it is used in SetCr3ForPageTables64)
> In Main32 CheckTdxFeatures is called after ReloadFlat32.
> 
> What's your opinion?



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78368): https://edk2.groups.io/g/devel/message/78368
Mute This Topic: 

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Min Xu]

On July 29, 2021 6:08 PM, Brijesh Singh wrote:
> On 7/29/21 1:07 AM, Xu, Min M wrote:
> > On July 29, 2021 12:29 PM, Brijesh Singh wrote:
> >> On 7/28/21 9:44 PM, Xu, Min M wrote:
> >>> Jiewen & Singh
> >>>
> >>> From the discussion I am thinking we have below rules to follow to
> >>> the design the structure of TEE_WORK_AREA:
> >>> 1. Design should be flexible but not too complicated 2. Reuse the
> >>> current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as
> TEE_WORK_AREA 3.
> >>> TEE_WORK_AREA should be initialized to all-0 at the beginning of
> >>> ResetVecotr 4. Reduce the changes to exiting code if possible
> >>>
> >>> So I try to make below conclusions below: (Please review) 1.
> >>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and
> SEV,
> >>> maybe in the future it can be used by other CC technologies.
> >>>
> >>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
> >>> guaranteed to be cleared in legacy guest. In TDX this memory region
> >>> is initialized to be all-0 by host VMM. In SEV the memory region is
> cleared as well.
> >>>   0x00B000|0x001000
> >>>
> >>
> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpa
> ce
> >> Guid.PcdSevEsWorkAreaSize
> >>>   DATA = {
> >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> >>>   }
> >> Hmm, I thought the contents of the data pages are controlled by the host
> VMM.
> >> If the backing pages are not zero filled then there is no guarantee
> >> that memory will be zero.  To verify it:
> >>
> >> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA
> >> values from 0x00 -> 0xCC
> >>
> >> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
> >>
> >> And dump does not contain the 0xcc.
> >>
> >> And to confirm further,  I attached to the qemu with the GDB before
> >> the booting the OVMF, and modified the SevEsWorkArea with some
> >> garbage number  and this time the dump printed garbage value I put
> through the debugger.
> >>
> >> In summary, the OVMF to zero the workarea memory on the entry and
> we
> >> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
> > So in legacy guest, CCWorkArea is cleared to all-0 without the
> DATA={0x00,0x00...}, right?
> 
> Okay, maybe I was not able to communicate it correctly.
> 
> The run I did is for the legacy guest. For the legacy guest, the contents of 
> the
> CCWorkArea may *not* be always zero even when you use the DATA={0x00,
> 0x00...}.
> 
> Currently, Qemu uses zero filled backing pages, so we will get a zero filled
> CCWorkArea; but nothing says that a backing page *must* be zero.
> Another VMM may choose to do things differently. In summary, the OVMF
> reset vector code must zero  the CCWorkArea  before calling SEV or TDX
> probes.
> 
Ah, I see. 
In current CheckSevFeatures, byte[SEV_ES_WORK_AREA] is cleared to0. 
Then its values is set based on the result of SEV probe. 

There is a bug here. CheckTdxFeatures does the similar work and it sets the 
WORK_AREA to 2. If CheckSevFeatures is called after CheckTdxFeatures, then
WORK_AREA is cleared and it is set to 0 because it is not SEV. The value is 
override.

I think there are 2 options:
Option 1:
Neither CheckTdxFeatures nor CheckSevFeatures should clear WORK_AREA. Instead
It should be cleared to 0 outside and before these 2 calls. So in Main16 after
TransitionFromReal16To32BitFlat WORK_AREA is cleared to 0. In Tdx guest this 
WORK_AREA
is initialized to 0 by host VMM.

Option 2:
Another option is to figure out a mechanism that only one CheckXXXFeatures is 
called.
Since there are 2 entry point in Main.asm: Main16 and Main32.
In Main16 CheckSevFeatures is called after TransitionFromReal16To32BitFlat. 
(eax should
be saved because it is used in SetCr3ForPageTables64)
In Main32 CheckTdxFeatures is called after ReloadFlat32.

What's your opinion?



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

Re: [edk2-devel] AcpiViewApp is for ARM platform

[Sami Mujawar]

Hi Tiger,

AcpiViewApp is an extension to the UEFI Shell debug command ‘acpiview’.  I have 
not tired it on x86 as I have no means of testing this, but the application 
should work and dump raw table data.
As it stands Acpiview[App] lacks support for detailed parsing and analysis of 
the x86 specific ACPI table fields. However, the parsing mechanism itself is 
designed to be generic and it should be possible to add support for parsing x86 
specific ACPI table fields.

For example, the GICC parsing is done by the following sections of the code:
https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c#L101..L124
https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c#L292..L302

Support for x86 specific MADT substructures can be added in a similar way.

Please do let me know if you have any queries or need any further information.

Regards,

Sami Mujawar

From:  on behalf of "Tiger Liu(BJ-RD) via groups.io" 

Reply to: "devel@edk2.groups.io" , "tiger...@zhaoxin.com" 

Date: Thursday, 29 July 2021 at 04:18
To: "devel@edk2.groups.io" 
Subject: [edk2-devel] AcpiViewApp is for ARM platform

Hi, All:
I tried to comple ShellPkg\Application\AcpiViewApp

I found the default compiled AcpiViewApp version is for ARM platform
For example:
I used it in shell on a x86 platform, found it checked GICC structure.

So, Is there a X86 version?

Thanks

保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.



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

Re: [edk2-devel] [PATCH] Maintainers.txt: remove Laszlo Ersek's entries

[Zeng, Star]

I'd like to also say thank you Laszlo.

Star

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo
> Ersek
> Sent: Friday, July 9, 2021 7:58 PM
> To: Ard Biesheuvel 
> Cc: edk2-devel-groups-io ; Andrew Fish
> ; Ard Biesheuvel ; Dong,
> Eric ; Justen, Jordan L ;
> Leif Lindholm ; Kinney, Michael D
> ; Philippe Mathieu-Daudé
> ; Kumar, Rahul1 ; Ni, Ray
> ; Sami Mujawar 
> Subject: Re: [edk2-devel] [PATCH] Maintainers.txt: remove Laszlo Ersek's
> entries
> 
> On 07/09/21 09:42, Ard Biesheuvel wrote:
> > On Thu, 8 Jul 2021 at 09:09, Laszlo Ersek  wrote:
> >>
> >> I'm relinquishing all my roles listed in "Maintainers.txt", for
> >> personal reasons.
> >>
> >> My email address  remains functional.
> >>
> >> To my understanding, my employer is working to assign others
> >> engineers to the edk2 project (at their discretion).
> >>
> >> Cc: Andrew Fish 
> >> Cc: Ard Biesheuvel 
> >> Cc: Eric Dong 
> >> Cc: Jordan Justen 
> >> Cc: Leif Lindholm 
> >> Cc: Michael D Kinney 
> >> Cc: Philippe Mathieu-Daudé 
> >> Cc: Rahul Kumar 
> >> Cc: Ray Ni 
> >> Cc: Sami Mujawar 
> >> Signed-off-by: Laszlo Ersek 
> >
> > Thanks for shaping the Tianocore project as you have done over the
> > past years. And apologies for my limited involvement as a
> > co-maintainer - I hope this was not a dominant factor in your
> > decision.
> 
> I want to be very clear about this: there is *zero* blaming others involved in
> my retirement from edk2.
> 
> Thank you!
> Laszlo
> 
> >
> > Reviewed-by: Ard Biesheuvel 
> >
> > With all the Confidential Computing work going on, we need to get your
> > position filled asap, although it seems unlikely that we will ever
> > find someone with the same knowledge level in both EDK2 and QEMU\/virt
> > topics. Recommendations welcome, I will ask around in Google as well.
> >
> >
> >> ---
> >>  Maintainers.txt | 4 
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/Maintainers.txt b/Maintainers.txt index
> >> f4e4c72d0628..e9dda5c5ca0c 100644
> >> --- a/Maintainers.txt
> >> +++ b/Maintainers.txt
> >> @@ -69,7 +69,6 @@ Tianocore Stewards
> >>  --
> >>  F: *
> >>  M: Andrew Fish 
> >> -M: Laszlo Ersek 
> >>  M: Leif Lindholm 
> >>  M: Michael D Kinney 
> >>
> >> @@ -143,7 +142,6 @@ M: Ard Biesheuvel 
> >> ArmVirtPkg
> >>  F: ArmVirtPkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/ArmVirtPkg
> >> -M: Laszlo Ersek 
> >>  M: Ard Biesheuvel 
> >>  R: Leif Lindholm 
> >>  R: Sami Mujawar  @@ -421,7 +419,6 @@ R:
> Siyuan
> >> Fu   OvmfPkg
> >>  F: OvmfPkg/
> >>  W: http://www.tianocore.org/ovmf/
> >> -M: Laszlo Ersek 
> >>  M: Ard Biesheuvel 
> >>  R: Jordan Justen 
> >>  S: Maintained
> >> @@ -567,7 +564,6 @@ F: UefiCpuPkg/
> >>  W: https://github.com/tianocore/tianocore.github.io/wiki/UefiCpuPkg
> >>  M: Eric Dong 
> >>  M: Ray Ni 
> >> -R: Laszlo Ersek 
> >>  R: Rahul Kumar 
> >>
> >>  UefiCpuPkg: Sec related modules
> >> --
> >> 2.19.1.3.g30247aa5d201
> >>
> >
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues

[Ard Biesheuvel]

On Fri, 23 Jul 2021 at 18:43, Pete Batard  wrote:
>
> Hi Sunny,
>
> Good catch for both these issues. Thanks for fixing them.
>
> With this:
>
> On 2021.07.23 10:04, Sunny Wang wrote:
> > This patch fixes two issues below:
> > 1. SCT SetTime_Func failures.
> > - https://github.com/pftf/RPi4/issues/164
> > 2. Using shell time and date commands to set time can't work.
> >
> > The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
> > error status.
> >
> > The root cause is that LibSetTime() sets RtcEpochSeconds variable with
> > inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
> > the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
> > variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
> > preexisting variable is rewritten with different attributes,
> > SetVariable() shall not modify the variable and shall return
> > EFI_INVALID_PARAMETER.
> >
> > Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
> > to the first EfiSetVariable() call to make two calls consistent.
> >
> > By the way, this patch also fix a minor issue with a debug message.
> >
> > Cc: Samer El-Haj-Mahmoud 
> > Cc: Sami Mujawar 
> > Cc: Jeremy Linton 
> > Cc: Ard Biesheuvel 
> > Cc: Pete Batard 
> > Cc: Leif Lindholm 
> >
> > Signed-off-by: Sunny Wang 
> > ---
> >   .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c   | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git 
> > a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c 
> > b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> > index de6fbb40e6..c10c91bc75 100644
> > --- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> > +++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> > @@ -4,7 +4,7 @@
> >*
> >*  Coypright (c) 2019, Pete Batard 
> >*  Copyright (c) 2018, Andrei Warkentin 
> > - *  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
> > + *  Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
> >*  Copyright (c) 2008-2010, Apple Inc. All rights reserved.
> >*  Copyright (c) Microsoft Corporation. All rights reserved.
> >*
> > @@ -96,7 +96,7 @@ LibGetTime (
> >   EfiSetVariable (
> > (CHAR16 *)mEpochVariableName,
> > ,
> > -  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > +  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> > EFI_VARIABLE_RUNTIME_ACCESS,
> > sizeof (EpochSeconds),
> > 
> > );
> > @@ -324,7 +324,7 @@ LibSetTime (
> >   DEBUG ((
> > DEBUG_ERROR,
> > "LibSetTime: Failed to save %s variable to non-volatile storage, 
> > Status = %r\n",
> > -  mDaylightVariableName,
> > +  mEpochVariableName,
> > Status
> > ));
> >   return Status;
> >
>
> Reviewed-by: Pete Batard 
> Tested-by: Pete Batard 

Merged, thanks,


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

Re: [edk2-devel] [PATCH v2 1/1] ArmVirtPkg: Remove meaningless comment

[Ard Biesheuvel]

On Wed, 21 Jul 2021 at 10:24, Sami Mujawar  wrote:
>
> Hi Philippe,
>
> Thank you for this patch.
>
> Reviewed-by: Sami Mujawar 
>

Merged, thanks.


> Regards,
>
> Sami Mujawar
>
> On 21/07/2021, 09:19, "Philippe Mathieu-Daudé"  wrote:
>
> From: Philippe Mathieu-Daude 
>
> The "Shell Embedded Boot Loader" description (added in
> commit 6f5872b1f401) does not add any value, remove it.
>
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Leif Lindholm 
> Cc: Sami Mujawar 
> Cc: Julien Grall 
> Suggested-by: Laszlo Ersek 
> Signed-off-by: Philippe Mathieu-Daude 
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc   | 3 ---
>  ArmVirtPkg/ArmVirtKvmTool.fdf| 3 ---
>  ArmVirtPkg/ArmVirtXen.fdf| 3 ---
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 3 ---
>  4 files changed, 12 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index d9abadbe708c..619b5f0b44c0 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -375,9 +375,6 @@ [Components.common]
>#
>MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
>
> -  #
> -  # UEFI application (Shell Embedded Boot Loader)
> -  #
>ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
>  
>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
> index 076155199905..152453dc4bb3 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.fdf
> +++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
> @@ -173,9 +173,6 @@ [FV.FvMain]
>INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
>INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
>
> -  #
> -  # UEFI application (Shell Embedded Boot Loader)
> -  #
>INF ShellPkg/Application/Shell/Shell.inf
>INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>
> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 8fbbc2313aff..9597465cf58a 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -177,9 +177,6 @@ [FV.FvMain]
>INF 
> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>
> -  #
> -  # UEFI application (Shell Embedded Boot Loader)
> -  #
>INF ShellPkg/Application/Shell/Shell.inf
>INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc 
> b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index 5b1d10057545..26f13f6a2115 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -99,9 +99,6 @@ [FV.FvMain]
>INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
>INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
>
> -  #
> -  # UEFI application (Shell Embedded Boot Loader)
> -  #
>INF ShellPkg/Application/Shell/Shell.inf
>INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>INF ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
> --
> 2.31.1
>
>


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

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Brijesh Singh via groups.io]

On 7/29/21 1:07 AM, Xu, Min M wrote:
> On July 29, 2021 12:29 PM, Brijesh Singh wrote:
>> On 7/28/21 9:44 PM, Xu, Min M wrote:
>>> Jiewen & Singh
>>>
>>> From the discussion I am thinking we have below rules to follow to the
>>> design the structure of TEE_WORK_AREA:
>>> 1. Design should be flexible but not too complicated 2. Reuse the
>>> current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as TEE_WORK_AREA 3.
>>> TEE_WORK_AREA should be initialized to all-0 at the beginning of
>>> ResetVecotr 4. Reduce the changes to exiting code if possible
>>>
>>> So I try to make below conclusions below: (Please review) 1.
>>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and SEV,
>>> maybe in the future it can be used by other CC technologies.
>>>
>>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
>>> guaranteed to be cleared in legacy guest. In TDX this memory region is
>>> initialized to be all-0 by host VMM. In SEV the memory region is cleared as 
>>> well.
>>>   0x00B000|0x001000
>>>
>> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpace
>> Guid.PcdSevEsWorkAreaSize
>>>   DATA = {
>>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>>   }
>> Hmm, I thought the contents of the data pages are controlled by the host VMM.
>> If the backing pages are not zero filled then there is no guarantee that 
>> memory
>> will be zero.  To verify it:
>>
>> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA values
>> from 0x00 -> 0xCC
>>
>> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
>>
>> And dump does not contain the 0xcc.
>>
>> And to confirm further,  I attached to the qemu with the GDB before the 
>> booting
>> the OVMF, and modified the SevEsWorkArea with some garbage number  and
>> this time the dump printed garbage value I put through the debugger.
>>
>> In summary, the OVMF to zero the workarea memory on the entry and we
>> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
> So in legacy guest, CCWorkArea is cleared to all-0 without the 
> DATA={0x00,0x00...}, right?

Okay, maybe I was not able to communicate it correctly.

The run I did is for the legacy guest. For the legacy guest, the
contents of the CCWorkArea may *not* be always zero even when you use
the DATA={0x00, 0x00...}.

Currently, Qemu uses zero filled backing pages, so we will get a zero
filled CCWorkArea; but nothing says that a backing page *must* be zero.
Another VMM may choose to do things differently. In summary, the OVMF
reset vector code must zero  the CCWorkArea  before calling SEV or TDX
probes.

thanks


>
>> Did I miss something ?
>>
>>
>>> 3. The structure of TEE_WORK_AREA
>>>   The current SEV_ES_WORK_AREA is defined as below:
>>>   typedef struct {
>>> UINT8SevEsEnabled;
>>> UINT8Reserved1[7];
>>> [Others...]
>>> } SEC_SEV_ES_WORK_AREA;
>>>
>>> So I think the TEE_WORK_AREA can be:
>>>   Byte[0] Type:
>>> 0: legacy 1: SEV2: TDX
>>>   Byte[1] Subtype:
>>>   If Type is 0, then it is 0
>>> If Type is 1, then it is up to SEV's definition
>>> If Type is 2, then it is up to TDX's definition  Byte[] other bytes
>>> are defined based on the Type/Subtype
>> I personally like Yao Jiewen's struct definition, but I can also live with 
>> this one as
>> well :). The only question I had was with his proposal was what if we remove 
>> the
>> Length and Version fields. If the header length was fixed then life would be 
>> much
>> easier in the ASM code.
> Yao Jiewen's structure is like below. If the HeaderVersion/HeaderLength are 
> removed
> you will find it is just what I am saying. The first 2 bytes are used to 
> distinguish the
> legacy/SEV/TDX. The left bytes are up to the first 2 bytes.
> typedef struct {
>  UINT8HeaderVersion; // 0
>  UINT8HeadLength; // 4
>  UINT8Type; // 0 - legacy, 1 - SEV, 2 - TDX
>  UINT8SubType; // Type specific sub type, if needed.
> } CC_COMMON_WORK_AREA_HEADER;
>
> typedef struct {
>  CC_COMMON_WORK_AREA_HEADER Header;
>  // reset is valid if Type == 1
>  UINT8Reserved1[4];
>  UINT64   RandomData;
>  UINT64   EncryptionMask;
> } SEC_SEV_ES_WORK_AREA;
>
> typedef struct {
>  CC_COMMON_WORK_AREA_HEADER Header;
>  // reset is valid if Type == 2
>  UINT8TdxSpecific[];  // TBD
> } TDX_WORK_AREA;
>>
>>> I check the code in SecMain.c.
>>>  SevEsIsEnabled() need updated to check SevEsWorkarea->SevEsEnabled == 1,
>> not non-0.
>>> @Brijesh Singh Is there any other code need update?
>> As noted before, the SevEsWorkAreas is used to pass the information from the
>> Sec to PEI phase. The workarea gets reused for totally different purpose 
>> after
>> the PEI phase.
> So only the above line in SecMain.c/SevEsIsEnabled() need updated, right?
> Thanks!

Re: [edk2-devel] [edk2-platforms PATCH 2/7] Marvell: Armada7k8k/OcteonTx: Add missing _STA methods in ACPI tables

[Ard Biesheuvel]

On Thu, 29 Jul 2021 at 11:46, Marcin Wojtas  wrote:
>
> Hi Ard,
>
> pon., 19 lip 2021 o 17:06 Marcin Wojtas  napisał(a):
> >
> > Hi Ard,
> >
> > pon., 19 lip 2021 o 11:54 Ard Biesheuvel  napisał(a):
> > >
> > > On Mon, 19 Jul 2021 at 11:31, Marcin Wojtas  wrote:
> > > >
> > > > BBR 1.0 spec says that _STA is required for each device in DSDT or SSDT.
> > > > Fix that for all platforms with the Marvell SoC's.
> > > >
> > >
> > > Can we fix the BBR instead? If ACPI itself does not require _STA, BBR
> > > should not require it either.
> > >
> > >
> >
> > I consulted with ARM on the matter. SBBR has requirements of things
> > that are otherwise optional in UEFI/ACPI/SMBIOS. Also some OS's may
> > require that and I can see those methods in most of the other ACPI
> > source files in the edk2-platfoms tree. I think the BBR requirements
> > discussions can follow, but it would be great if this change can be
> > applied, so that no to block other development.
> >
>
> Do you have any feedback to the patchset and the _STA methods concerns?
>

Yes. I would like to understand why _STA methods are now mandated by BBR.


> >
> > >
> > > > Signed-off-by: Marcin Wojtas 
> > > > ---
> > > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl| 56 
> > > > +++
> > > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl| 76 
> > > > 
> > > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl | 72 
> > > > +++
> > > >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl | 12 
> > > > 
> > > >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl | 56 
> > > > +++
> > > >  5 files changed, 272 insertions(+)
> > > >
> > > > diff --git 
> > > > a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl 
> > > > b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
> > > > index 345c1e4dd6..88e38efeeb 100644
> > > > --- a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
> > > > +++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
> > > > @@ -20,21 +20,37 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > > "ARMADA7K", 3)
> > > >  {
> > > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > > Hardware ID
> > > >  Name (_UID, 0x000)  // _UID: Unique ID
> > > > +Method (_STA)   // _STA: Device status
> > > > +{
> > > > +Return (0xF)
> > > > +}
> > > >  }
> > > >  Device (CPU1)
> > > >  {
> > > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > > Hardware ID
> > > >  Name (_UID, 0x001)  // _UID: Unique ID
> > > > +Method (_STA)   // _STA: Device status
> > > > +{
> > > > +Return (0xF)
> > > > +}
> > > >  }
> > > >  Device (CPU2)
> > > >  {
> > > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > > Hardware ID
> > > >  Name (_UID, 0x100)  // _UID: Unique ID
> > > > +Method (_STA)   // _STA: Device status
> > > > +{
> > > > +Return (0xF)
> > > > +}
> > > >  }
> > > >  Device (CPU3)
> > > >  {
> > > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > > Hardware ID
> > > >  Name (_UID, 0x101)  // _UID: Unique ID
> > > > +Method (_STA)   // _STA: Device status
> > > > +{
> > > > +Return (0xF)
> > > > +}
> > > >  }
> > > >
> > > >  Device (AHC0)
> > > > @@ -42,6 +58,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > > "ARMADA7K", 3)
> > > >  Name (_HID, "LNRO001E") // _HID: Hardware ID
> > > >  Name (_UID, 0x00)   // _UID: Unique ID
> > > >  Name (_CCA, 0x01)   // _CCA: Cache Coherency 
> > > > Attribute
> > > > +Method (_STA)   // _STA: Device status
> > > > +{
> > > > +Return (0xF)
> > > > +}
> > > >  Name (_CLS, Package (0x03)  // _CLS: Class Code
> > > >  {
> > > >  0x01,
> > > > @@ -67,6 +87,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > > "ARMADA7K", 3)
> > > >  Name (_HID, "MRVL0002") // _HID: Hardware ID
> > > >  Name (_UID, 0x00)   // _UID: Unique ID
> > > >  Name (_CCA, 0x01)   // _CCA: Cache Coherency 
> > > > Attribute
> > > > +Method (_STA)   // _STA: Device status
> > > > +{
> > > > +Return (0xF)
> > > > +}
> > > >
> > > >  Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource 
> > > > Settings
> > > >  {
> > > > @@ -96,6 +120,10 @@ DefinitionBlock ("DSDT.aml", 

Re: [edk2-devel] [PATCH v5 00/11] Measured SEV boot with kernel/initrd/cmdline

[Ard Biesheuvel]

On Wed, 28 Jul 2021 at 19:30, Dov Murik  wrote:
>
>
> On 28/07/2021 19:41, Yao, Jiewen wrote:
> > For OvmfPkg, reviewed-by: Jiewen Yao 
> > For ArmVirtPkg, acked-by: Jiewen Yao 
> >
>
> Thanks Jiewen!
>


Merged as #1843

Note that I needed to add CryptoPkg/CryptoPkg.dec to the list of
acceptable dependencies in OvmfPkg.ci.yaml for the CI checks to be
able to pass.

Thanks all,


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

Re: [edk2-devel] [edk2-platforms PATCH 2/7] Marvell: Armada7k8k/OcteonTx: Add missing _STA methods in ACPI tables

[Marcin Wojtas]

Hi Ard,

pon., 19 lip 2021 o 17:06 Marcin Wojtas  napisał(a):
>
> Hi Ard,
>
> pon., 19 lip 2021 o 11:54 Ard Biesheuvel  napisał(a):
> >
> > On Mon, 19 Jul 2021 at 11:31, Marcin Wojtas  wrote:
> > >
> > > BBR 1.0 spec says that _STA is required for each device in DSDT or SSDT.
> > > Fix that for all platforms with the Marvell SoC's.
> > >
> >
> > Can we fix the BBR instead? If ACPI itself does not require _STA, BBR
> > should not require it either.
> >
> >
>
> I consulted with ARM on the matter. SBBR has requirements of things
> that are otherwise optional in UEFI/ACPI/SMBIOS. Also some OS's may
> require that and I can see those methods in most of the other ACPI
> source files in the edk2-platfoms tree. I think the BBR requirements
> discussions can follow, but it would be great if this change can be
> applied, so that no to block other development.
>

Do you have any feedback to the patchset and the _STA methods concerns?

Best regards,
Marcin

>
> >
> > > Signed-off-by: Marcin Wojtas 
> > > ---
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl| 56 
> > > +++
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl| 76 
> > > 
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl | 72 
> > > +++
> > >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn9131DbA/Ssdt.asl | 12 
> > >  Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl | 56 
> > > +++
> > >  5 files changed, 272 insertions(+)
> > >
> > > diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl 
> > > b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
> > > index 345c1e4dd6..88e38efeeb 100644
> > > --- a/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
> > > +++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl
> > > @@ -20,21 +20,37 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > "ARMADA7K", 3)
> > >  {
> > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > Hardware ID
> > >  Name (_UID, 0x000)  // _UID: Unique ID
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +Return (0xF)
> > > +}
> > >  }
> > >  Device (CPU1)
> > >  {
> > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > Hardware ID
> > >  Name (_UID, 0x001)  // _UID: Unique ID
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +Return (0xF)
> > > +}
> > >  }
> > >  Device (CPU2)
> > >  {
> > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > Hardware ID
> > >  Name (_UID, 0x100)  // _UID: Unique ID
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +Return (0xF)
> > > +}
> > >  }
> > >  Device (CPU3)
> > >  {
> > >  Name (_HID, "ACPI0007" /* Processor Device */)  // _HID: 
> > > Hardware ID
> > >  Name (_UID, 0x101)  // _UID: Unique ID
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +Return (0xF)
> > > +}
> > >  }
> > >
> > >  Device (AHC0)
> > > @@ -42,6 +58,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > "ARMADA7K", 3)
> > >  Name (_HID, "LNRO001E") // _HID: Hardware ID
> > >  Name (_UID, 0x00)   // _UID: Unique ID
> > >  Name (_CCA, 0x01)   // _CCA: Cache Coherency 
> > > Attribute
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +Return (0xF)
> > > +}
> > >  Name (_CLS, Package (0x03)  // _CLS: Class Code
> > >  {
> > >  0x01,
> > > @@ -67,6 +87,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > "ARMADA7K", 3)
> > >  Name (_HID, "MRVL0002") // _HID: Hardware ID
> > >  Name (_UID, 0x00)   // _UID: Unique ID
> > >  Name (_CCA, 0x01)   // _CCA: Cache Coherency 
> > > Attribute
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +Return (0xF)
> > > +}
> > >
> > >  Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource 
> > > Settings
> > >  {
> > > @@ -96,6 +120,10 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "MVEBU ", 
> > > "ARMADA7K", 3)
> > >  Name (_HID, "MRVL0004") // _HID: Hardware ID
> > >  Name (_UID, 0x01)   // _UID: Unique ID
> > >  Name (_CCA, 0x01)   // _CCA: Cache Coherency 
> > > Attribute
> > > +Method (_STA)   // _STA: Device status
> > > +{
> > > +   

Re: [edk2-devel] [PATCH v6 00/11] Secure Boot default keys

[Grzegorz Bernacki]

Hi,

I will make the fixes and send a new version soon.
thanks,
greg

śr., 28 lip 2021 o 13:07 Ard Biesheuvel  napisał(a):
>
> On Wed, 28 Jul 2021 at 12:39, Ard Biesheuvel  wrote:
> >
> > On Wed, 28 Jul 2021 at 09:44, gaoliming  wrote:
> > >
> > > Sunny:
> > >   Yes. This patch set is ready to be merged.
> > >
> > > Samer:
> > >   Would you help merge this patch set?
> > >
> >
> > I can pick it up if you could please create the release notes entry? Thanks.
> >
>
> Submitted here:
>
> https://github.com/tianocore/edk2/pull/1839
>
> and failed with some errors. Could someone please diagnose/fix and submit a 
> v7?
>
>
> > > Thanks
> > > Liming
> > > > -邮件原件-
> > > > 发件人: devel@edk2.groups.io  代表 Sunny Wang
> > > > 发送时间: 2021年7月21日 11:41
> > > > 收件人: Samer El-Haj-Mahmoud ;
> > > > devel@edk2.groups.io; g...@semihalf.com; Ard Biesheuvel
> > > > ; gaolim...@byosoft.com.cn; ray...@intel.com
> > > > 抄送: l...@nuviainc.com; m...@semihalf.com; upstr...@semihalf.com;
> > > > jiewen@intel.com; jian.j.w...@intel.com; min.m...@intel.com;
> > > > ler...@redhat.com; Sami Mujawar ;
> > > > af...@apple.com; jordan.l.jus...@intel.com; rebe...@bsdio.com;
> > > > gre...@freebsd.org; Thomas Abraham ;
> > > > chasel.c...@intel.com; nathaniel.l.desim...@intel.com;
> > > > eric.d...@intel.com; michael.d.kin...@intel.com; zailiang@intel.com;
> > > > yi.q...@intel.com; gra...@nuviainc.com; r...@semihalf.com; p...@akeo.ie;
> > > > Sunny Wang 
> > > > 主题: Re: [edk2-devel] [PATCH v6 00/11] Secure Boot default keys
> > > >
> > > > Ard, Liming, Ray, Thanks for your review for ArmVirtPkg, ArmPlatformPkg,
> > > and
> > > > EmulatorPkg patches.
> > > >
> > > > As for the patch for Intel Platforms below, it is in another series for
> > > > edk2-platforms.
> > > >  - [edk2-platforms PATCH v6 1/4] Intel Platforms: add
> > > > SecureBootVariableLib class resolution
> > > > https://edk2.groups.io/g/devel/message/77781
> > > >
> > > > Therefore, I think this series already got all the necessary Reviewed-By
> > > and
> > > > Acked-By of all parts and is ready to be pushed now.
> > > >
> > > > Best Regards,
> > > > Sunny Wang
> > > >
> > > > -Original Message-
> > > > From: Samer El-Haj-Mahmoud 
> > > > Sent: Friday, July 16, 2021 8:00 PM
> > > > To: devel@edk2.groups.io; g...@semihalf.com
> > > > Cc: l...@nuviainc.com; ardb+tianoc...@kernel.org; Sunny Wang
> > > > ; m...@semihalf.com; upstr...@semihalf.com;
> > > > jiewen@intel.com; jian.j.w...@intel.com; min.m...@intel.com;
> > > > ler...@redhat.com; Sami Mujawar ;
> > > > af...@apple.com; ray...@intel.com; jordan.l.jus...@intel.com;
> > > > rebe...@bsdio.com; gre...@freebsd.org; Thomas Abraham
> > > > ; chasel.c...@intel.com;
> > > > nathaniel.l.desim...@intel.com; gaolim...@byosoft.com.cn;
> > > > eric.d...@intel.com; michael.d.kin...@intel.com; zailiang@intel.com;
> > > > yi.q...@intel.com; gra...@nuviainc.com; r...@semihalf.com; p...@akeo.ie;
> > > > Samer El-Haj-Mahmoud 
> > > > Subject: RE: [edk2-devel] [PATCH v6 00/11] Secure Boot default keys
> > > >
> > > > The v6 of this series seems to have all the necessary Reviewed-By (and
> > > some
> > > > Tested-By) of all parts, except the following platform specific parts.
> > > Could we
> > > > get help from maintainers to review these please?
> > > >
> > > > Much appreciated!
> > > >
> > > > - ArmVirtPkg : https://edk2.groups.io/g/devel/message/2
> > > > - ArmPlatformPkg: https://edk2.groups.io/g/devel/message/5
> > > > - EmulatorPkg: https://edk2.groups.io/g/devel/message/3
> > > > - Intel Platforms (Platform/Intel/QuarkPlatformPkg,
> > > > Platform/Intel/MinPlatformPkg, Platform/Intel/Vlv2TbltDevicePkg):
> > > > https://edk2.groups.io/g/devel/message/77781
> > > >
> > > > Thanks,
> > > > --Samer
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: devel@edk2.groups.io  On Behalf Of
> > > > > Grzegorz Bernacki via groups.io
> > > > > Sent: Wednesday, July 14, 2021 8:30 AM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: l...@nuviainc.com; ardb+tianoc...@kernel.org; Samer
> > > > El-Haj-Mahmoud
> > > > > ; Sunny Wang
> > > > > ; m...@semihalf.com; upstr...@semihalf.com;
> > > > > jiewen@intel.com; jian.j.w...@intel.com; min.m...@intel.com;
> > > > > ler...@redhat.com; Sami Mujawar ;
> > > > > af...@apple.com; ray...@intel.com; jordan.l.jus...@intel.com;
> > > > > rebe...@bsdio.com; gre...@freebsd.org; Thomas Abraham
> > > > > ; chasel.c...@intel.com;
> > > > > nathaniel.l.desim...@intel.com; gaolim...@byosoft.com.cn;
> > > > > eric.d...@intel.com; michael.d.kin...@intel.com; 
> > > > > zailiang@intel.com;
> > > > > yi.q...@intel.com; gra...@nuviainc.com; r...@semihalf.com;
> > > > > p...@akeo.ie; Grzegorz Bernacki 
> > > > > Subject: [edk2-devel] [PATCH v6 00/11] Secure Boot default keys
> > > > >
> > > > > This patchset adds support for initialization of default
> > > > > Secure Boot variables based on keys content embedded in
> > > > > flash 

Re: [edk2-rfc] [edk2-devel] RFC: Common Design Proposal on Confidential Computing Support in OVMF

[Ard Biesheuvel]

On Mon, 26 Jul 2021 at 10:59, Yao, Jiewen  wrote:
>
> Hi
>
> I would like to raise the topic on a confidential computing support in OVMF.
>
>
>
> The main target is AMD SEV feature and Intel TDX feature in OVMF package.
>
>
>
> The goal is to create a guidance for our future confidential computing work 
> and to better support review and maintenance.
>

Hello Jiewen,

Thanks for writing this up. As you know, ARM is a bit behind in the
CCA space, and so I will not be able to take part in these discussions
in great detail.

I will leave it to the contributors and other stakeholders to comment
on your proposal below. To me, it looks reasonable.

-- 
Ard.


>
>
>
>
> [Background]
>
>
>
> AMD is adding AMD Secure Encrypted Virtualization (SEV), SEV-Encrypted State 
> (SEV-ES), SEV-Secure Nested Paging (SEV-SNP) features to OVMF package. 
> (https://developer.amd.com/sev/)
>
> Intel is adding Intel Trust Domain Extensions (TDX) features to OVMF package. 
> (https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html)
>
>
>
> Both of them support confidential computing use case.
>
>
>
> ARM is creating Realm Management Extension (RME). It might be considered in 
> the future. 
> (https://www.arm.com/why-arm/architecture/security-features/arm-confidential-compute-architecture)
>
>
>
> So we need a good Confidential Computing infrastructure for EDKII.
>
>
>
>
>
> [Problem Statement]
>
>
>
> 1) Current OVMF package integrated some AMD SEV features. But not all 
> features.
>
> Some basic SEV features are in OvmfPkg and enabled as default build. Some 
> advanced SEV features are in OvmfPkg/AmdSev and only enable in AmdSev build.
>
> However, the criteria is NOT clear.
>
>
>
> It also brings problem when we want to add more TDX stuff. Where we should go?
>
> For example, I feel PlatformBootManagerLibGrub should be in OvmfPkg/AmdSev. 
> Why it is not there?
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/PlatformBootManagerLibGrub
>
>
>
> We need a clear and consistent rule on where the confidential computing 
> module should go, instead of making ad-hoc decision for each features.
>
>
>
> 2) Ideally, when we integrate SEV feature or TDX feature, there is some level 
> of isolation, such as
>
>A) standalone driver
>
>B) standalone library
>
>C) standalone file, if it has to be in one module
>
>D) standalone function, if it has to be in one file
>
> The preference is from A to D. A is most preferred. D is least preferred.
>
> As such, when people find something wrong, they can just focus on some SEV or 
> TDX specific files.
>
>
>
> We do see good examples, such as SecretDxe (BTW: The name should be 
> SevSecretDxe), AmdSevDxe.
>
> However, some code (ResetVector and Sec) mixed SEV support with normal OVMF 
> code together. That makes it extremely hard to review TDX extensions or 
> maintain a non-SEV code.
>
> https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia32/PageTables64.asm
>
> https://github.com/tianocore/edk2/blob/master/OvmfPkg/Sec/SecMain.c
>
>
>
> For latter (such as ResetVector and Sec), I suggest we make a clear 
> isolation. That can help the reviewer to understand better on SEV flow, TDX 
> flow and normal OVMF flow.
>
>
>
> 3) We may have more problem. For example, how to align the OVMF design 
> between SEV and TDX?
>
> I think, the most SEV OVMF design is good. The TDX OVMF should just follow. 
> For example,
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name 
> should be IoMmuSevDxe)
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
>
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib
>
>
>
>
>
> [Proposal]
>
>
>
> The current SEV OVMF design is understandable, because when the SEV code was 
> added years ago, it was the first example. We did not know what would be the 
> best way to handle that, and we did not know what TDX would look like.
>
> Today, we have more concrete answer, and let do some refinement step by step.
>
>
>
> Confidential computing (CC) == SEV or TDX (it may include RME in the future)
>
>
>
> 1) CC Feature support (DSC/FDF)
>
>
>
> * Try to limit the impact to existing normal OVMF binary.
>
>
>
> 1.1 - The OVMF packet common DSC/FDF supports OVMF boot in all CC modes or 
> normal mode.
>
>
>
> The one OVMF image can boot in normal OVMF mode, SEV mode, or TDX mode.
>
>
>
> 1.2 - The OVMF packet common DSC/FDF includes *mature* CC feature.
>
>
>
> The minimal scope is the image shall boot to OS successfully.
>
> The maximal scope is the feature shall be adopted by OS and will not change 
> for a period of time.
>
>
>
> Any immature, under discussion or under review feature shall NOT be put here, 
> such as attestation.
>
>
>
> 1.3 - The OVMF package CC specific DSC/FDF includes *all* CC feature.
>
>
>
> The CC specific DSC/FDF shall be in OvmfPkg/ 

Re: [edk2-devel] [PATCH EDK2 v2 1/1] MdeModulePkg/UefiSortLib:Add UefiSortLib unit test

[Wu, Hao A]

> -Original Message-
> From: Wenyi Xie 
> Sent: Thursday, July 29, 2021 4:01 PM
> To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A
> 
> Cc: songdongku...@huawei.com; xiewen...@huawei.com
> Subject: [PATCH EDK2 v2 1/1] MdeModulePkg/UefiSortLib:Add UefiSortLib
> unit test
> 
> Adding two unit test case for UefiSortLib. One is a test on sorting an array 
> of
> UINT32 by using PerformQuickSort, another is a test on comparing the same
> buffer by using StringCompare.


Thanks.
Reviewed-by: Hao A Wu 

I will wait a couple days before merging to see if any additional comment from 
other reviewers.

Best Regards,
Hao Wu


> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Signed-off-by: Wenyi Xie 
> ---
>  MdeModulePkg/Test/MdeModulePkgHostTest.dsc|   6 +
>  MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf |  32
> 
>  MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c   | 188
> 
>  3 files changed, 226 insertions(+)
> 
> diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> index 4da4692c8451..c9ec835df65d 100644
> --- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> +++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> @@ -41,3 +41,9 @@ [Components]
>  
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDis
> able|TRUE
>}
> +
> +  MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf {
> +
> +  UefiSortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
> +
> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +  }
> diff --git
> a/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
> b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
> new file mode 100644
> index ..85d8dcd69619
> --- /dev/null
> +++ b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
> @@ -0,0 +1,32 @@
> +## @file
> +# This is a unit test for the UefiSortLib.
> +#
> +# Copyright (C) Huawei Technologies Co., Ltd. All rights reserved #
> +SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +[Defines]
> +  INF_VERSION = 0x00010017
> +  BASE_NAME   = UefiSortLibUnitTest
> +  FILE_GUID   = 271337A3-0D79-BA3E-BC03-714E518E3B1B
> +  VERSION_STRING  = 1.0
> +  MODULE_TYPE = HOST_APPLICATION
> +
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +#  VALID_ARCHITECTURES   = IA32 X64
> +#
> +
> +[Sources]
> +  UefiSortLibUnitTest.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> +
> +[LibraryClasses]
> +  UnitTestLib
> +  DebugLib
> +  UefiSortLib
> diff --git
> a/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c
> b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c
> new file mode 100644
> index ..71f30d8b9f7f
> --- /dev/null
> +++ b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c
> @@ -0,0 +1,188 @@
> +/** @file
> +  Unit tests of the UefiSortLib
> +
> +  Copyright (C) Huawei Technologies Co., Ltd. All rights reserved
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define UNIT_TEST_APP_NAME"UefiSortLib Unit Tests"
> +#define UNIT_TEST_APP_VERSION "1.0"
> +
> +#define TEST_ARRAY_SIZE_9 9
> +
> +/**
> +  The function is called by PerformQuickSort to compare int values.
> +
> +  @param[in] LeftThe pointer to first buffer.
> +  @param[in] Right   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.
> +
> +**/
> +INTN
> +EFIAPI
> +TestCompareFunction (
> +  IN CONST VOID *Left,
> +  IN CONST VOID *Right
> +  )
> +{
> +  if (*(UINT32*)Right > *(UINT32*)Left) {
> +return 1;
> +  } else if (*(UINT32*)Right < *(UINT32*)Left) {
> +return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +/**
> +  Unit test for PerformQuickSort () API of the UefiSortLib.
> +
> +  @param[in]  Context[Optional] An optional parameter that enables:
> + 1) test-case reuse with varied parameters and
> + 2) test-case re-entry for Target tests that need a
> + reboot.  This parameter is a VOID* and it is the
> + responsibility of the test author to ensure that the
> + contents are well understood by all test cases that 
> may
> + consume it.
> +
> +  @retval  UNIT_TEST_PASSED The Unit test has completed and the
> test
> 

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei: remove redundant sizeof

[Wu, Hao A]

> -Original Message-
> From: Wu, Hao A
> Sent: Thursday, July 29, 2021 4:23 PM
> To: devel@edk2.groups.io; xiewen...@huawei.com; Wang, Jian J
> 
> Cc: songdongku...@huawei.com; Yao, Jiewen ;
> Laszlo Ersek 
> Subject: RE: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei:
> remove redundant sizeof
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > wenyi,xie via groups.io
> > Sent: Thursday, July 29, 2021 3:45 PM
> > To: devel@edk2.groups.io; Wang, Jian J ; Wu,
> > Hao A 
> > Cc: songdongku...@huawei.com; xiewen...@huawei.com; Yao, Jiewen
> > ; Laszlo Ersek 
> > Subject: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei:
> > remove redundant sizeof
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=
> >
> > In function InstallPreHashFvPpi, when calculating the size of struct
> > HASH_INFO，sizeof is used twice. This bug does not lead to buffer
> > overflow, "sizeof (HASH_INFO)" is 4, whereas "sizeof (sizeof
> > (HASH_INFO))" is 4 or 8.
> 
> 
> Thanks.
> Reviewed-by: Hao A Wu 
> 
> I will wait a couple days before merging to see if any additional comment
> from other reviewers.


Really sorry, please ignore the previous mail sent (giving the Reviewed-by tag).
It was sent by accident. Withdraw my R-b tag for this patch.

Best Regards,
Hao Wu


> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Cc: Jiewen Yao 
> > Cc: Jian J Wang 
> > Cc: Laszlo Ersek 
> > Signed-off-by: Wenyi Xie 
> > Reviewed-by: Laszlo Ersek 
> > ---
> >  SecurityPkg/FvReportPei/FvReportPei.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/SecurityPkg/FvReportPei/FvReportPei.c
> > b/SecurityPkg/FvReportPei/FvReportPei.c
> > index d709760ea3ce..e82413e090c0 100644
> > --- a/SecurityPkg/FvReportPei/FvReportPei.c
> > +++ b/SecurityPkg/FvReportPei/FvReportPei.c
> > @@ -67,7 +67,7 @@ InstallPreHashFvPpi (
> >HASH_INFO *HashInfo;
> >
> >PpiSize = sizeof
> > (EDKII_PEI_FIRMWARE_VOLUME_INFO_PREHASHED_FV_PPI)
> > -+ sizeof (sizeof (HASH_INFO))
> > ++ sizeof (HASH_INFO)
> >  + HashSize;
> >
> >PreHashedFvPpi = AllocatePool (PpiSize);
> > --
> > 2.20.1.windows.1
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78355): https://edk2.groups.io/g/devel/message/78355
Mute This Topic: https://groups.io/mt/84523794/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 v2 1/1] SecurityPkg/FvReportPei: remove redundant sizeof

[Yao, Jiewen]

I think  I have given R-B. If no, then
Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Wu, Hao A 
> Sent: Thursday, July 29, 2021 4:23 PM
> To: devel@edk2.groups.io; xiewen...@huawei.com; Wang, Jian J
> 
> Cc: songdongku...@huawei.com; Yao, Jiewen ; Laszlo
> Ersek 
> Subject: RE: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei: remove
> redundant sizeof
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> > wenyi,xie via groups.io
> > Sent: Thursday, July 29, 2021 3:45 PM
> > To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A
> > 
> > Cc: songdongku...@huawei.com; xiewen...@huawei.com; Yao, Jiewen
> > ; Laszlo Ersek 
> > Subject: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei: remove
> > redundant sizeof
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=
> >
> > In function InstallPreHashFvPpi, when calculating the size
> > of struct HASH_INFO，sizeof is used twice. This bug does
> > not lead to buffer overflow, "sizeof (HASH_INFO)" is 4,
> > whereas "sizeof (sizeof (HASH_INFO))" is 4 or 8.
> 
> 
> Thanks.
> Reviewed-by: Hao A Wu 
> 
> I will wait a couple days before merging to see if any additional comment from
> other reviewers.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Cc: Jiewen Yao 
> > Cc: Jian J Wang 
> > Cc: Laszlo Ersek 
> > Signed-off-by: Wenyi Xie 
> > Reviewed-by: Laszlo Ersek 
> > ---
> >  SecurityPkg/FvReportPei/FvReportPei.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/SecurityPkg/FvReportPei/FvReportPei.c
> > b/SecurityPkg/FvReportPei/FvReportPei.c
> > index d709760ea3ce..e82413e090c0 100644
> > --- a/SecurityPkg/FvReportPei/FvReportPei.c
> > +++ b/SecurityPkg/FvReportPei/FvReportPei.c
> > @@ -67,7 +67,7 @@ InstallPreHashFvPpi (
> >HASH_INFO *HashInfo;
> >
> >PpiSize = sizeof
> > (EDKII_PEI_FIRMWARE_VOLUME_INFO_PREHASHED_FV_PPI)
> > -+ sizeof (sizeof (HASH_INFO))
> > ++ sizeof (HASH_INFO)
> >  + HashSize;
> >
> >PreHashedFvPpi = AllocatePool (PpiSize);
> > --
> > 2.20.1.windows.1
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78354): https://edk2.groups.io/g/devel/message/78354
Mute This Topic: https://groups.io/mt/84523794/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 v2 1/1] SecurityPkg/FvReportPei: remove redundant sizeof

[Wu, Hao A]

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> wenyi,xie via groups.io
> Sent: Thursday, July 29, 2021 3:45 PM
> To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A
> 
> Cc: songdongku...@huawei.com; xiewen...@huawei.com; Yao, Jiewen
> ; Laszlo Ersek 
> Subject: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei: remove
> redundant sizeof
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=
> 
> In function InstallPreHashFvPpi, when calculating the size
> of struct HASH_INFO，sizeof is used twice. This bug does
> not lead to buffer overflow, "sizeof (HASH_INFO)" is 4,
> whereas "sizeof (sizeof (HASH_INFO))" is 4 or 8.


Thanks.
Reviewed-by: Hao A Wu 

I will wait a couple days before merging to see if any additional comment from 
other reviewers.

Best Regards,
Hao Wu


> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Laszlo Ersek 
> Signed-off-by: Wenyi Xie 
> Reviewed-by: Laszlo Ersek 
> ---
>  SecurityPkg/FvReportPei/FvReportPei.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/SecurityPkg/FvReportPei/FvReportPei.c
> b/SecurityPkg/FvReportPei/FvReportPei.c
> index d709760ea3ce..e82413e090c0 100644
> --- a/SecurityPkg/FvReportPei/FvReportPei.c
> +++ b/SecurityPkg/FvReportPei/FvReportPei.c
> @@ -67,7 +67,7 @@ InstallPreHashFvPpi (
>HASH_INFO *HashInfo;
> 
>PpiSize = sizeof
> (EDKII_PEI_FIRMWARE_VOLUME_INFO_PREHASHED_FV_PPI)
> -+ sizeof (sizeof (HASH_INFO))
> ++ sizeof (HASH_INFO)
>  + HashSize;
> 
>PreHashedFvPpi = AllocatePool (PpiSize);
> --
> 2.20.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78353): https://edk2.groups.io/g/devel/message/78353
Mute This Topic: https://groups.io/mt/84523794/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] MdeModulePkg/UefiSortLib:Add UefiSortLib unit test

[wenyi,xie via groups.io]

Hi, Wu Hao

Thank you for your reviewing.
I have create the v2 patch according to your comments.

Thanks
Wenyi

On 2021/7/29 13:42, Wu, Hao A wrote:
> Thanks for the patch.
> Some inline comments below:
> 
> 
>> -Original Message-
>> From: Wenyi Xie 
>> Sent: Wednesday, July 28, 2021 1:58 PM
>> To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A
>> 
>> Cc: songdongku...@huawei.com; xiewen...@huawei.com
>> Subject: [PATCH EDK2 v1 1/1] MdeModulePkg/UefiSortLib:Add UefiSortLib unit
>> test
>>
>> Adding unit test for UefiSortLib.
> 
> 
> Could you help to give a brief summary on what tests are added?
> 
> 
>>
>> Cc: Jian J Wang 
>> Cc: Hao A Wu 
>> Signed-off-by: Wenyi Xie 
>> ---
>>  MdeModulePkg/Test/MdeModulePkgHostTest.dsc|   6 +
>>  MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf |  32 
>>  MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c   | 188
>> 
>>  3 files changed, 226 insertions(+)
>>
>> diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
>> b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
>> index 4da4692c8451..c9ec835df65d 100644
>> --- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
>> +++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
>> @@ -41,3 +41,9 @@ [Components]
>>  
>>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable
>> |TRUE
>>}
>> +
>> +  MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf {
>> +
>> +  UefiSortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
>> +
>> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
>> +  }
>> diff --git 
>> a/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
>> b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
>> new file mode 100644
>> index ..d9dac307934e
>> --- /dev/null
>> +++ b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
>> @@ -0,0 +1,32 @@
>> +## @file
>> +# This is a unit test for the UefiSortLib.
>> +#
>> +# Copyright (c) Microsoft Corporation.
> 
> 
> Please help to use the 'copyright' information of Huawei like in file 
> UefiSortLibUnitTest.c.
> 
> 
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
>> +
>> +[Defines]
>> +  INF_VERSION = 0x00010017
>> +  BASE_NAME   = UefiSortLibUnitTest
>> +  FILE_GUID   = 271337A3-0D79-BA3E-BC03-714E518E3B1B
>> +  VERSION_STRING  = 1.0
>> +  MODULE_TYPE = HOST_APPLICATION
>> +
>> +#
>> +# The following information is for reference only and not required by the 
>> build
>> tools.
>> +#
>> +#  VALID_ARCHITECTURES   = IA32 X64
>> +#
>> +
>> +[Sources]
>> +  UefiSortLibUnitTest.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
>> +
>> +[LibraryClasses]
>> +  UnitTestLib
>> +  DebugLib
>> +  UefiSortLib
>> diff --git a/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c
>> b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c
>> new file mode 100644
>> index ..f2f89daef7ba
>> --- /dev/null
>> +++ b/MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c
>> @@ -0,0 +1,188 @@
>> +/** @file
>> +  Unit tests of the UefiSortLib
>> +
>> +  Copyright (C) Huawei Technologies Co., Ltd. All rights reserved
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +
>> +#define UNIT_TEST_APP_NAME"UefiSortLib Unit Tests"
>> +#define UNIT_TEST_APP_VERSION "1.0"
>> +
>> +#define TEST_ARRAY_SIZE_9 9
>> +
>> +/**
>> +  The function is called by PerformQuickSort to compare int values.
>> +
>> +  @param[in] LeftThe pointer to first buffer.
>> +  @param[in] Right   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.
>> +
>> +**/
>> +INTN
>> +EFIAPI
>> +TestCompareFunction (
>> +  IN CONST VOID *Left,
>> +  IN CONST VOID *Right
>> +  )
>> +{
>> +  if (*(UINT32*)Right > *(UINT32*)Left) {
>> +return 1;
>> +  } else if (*(UINT32*)Right < *(UINT32*)Left) {
>> +return -1;
>> +  }
>> +
>> +  return 0;
>> +}
>> +
>> +/**
>> +  Unit test for PerformQuickSort () API of the UefiSortLib.
>> +
>> +  @param[in]  Context[Optional] An optional parameter that enables:
>> + 1) test-case reuse with varied parameters and
>> + 2) test-case re-entry for Target tests that need a
>> + reboot.  This parameter is a VOID* and it is the
>> + responsibility of the test author to ensure that 
>> the
>> +

[edk2-devel] [PATCH EDK2 v2 0/1] MdeModulePkg/UefiSortLib:Add UefiSortLib unit test

[wenyi,xie via groups.io]

Main Changes since v1 :
1.add brief summary in commit message
2.change the copyright in UefiSortLibUnitTest.inf
3.refine the indent in UnitTestingEntry

Wenyi Xie (1):
  MdeModulePkg/UefiSortLib:Add UefiSortLib unit test

 MdeModulePkg/Test/MdeModulePkgHostTest.dsc|   6 +
 MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf |  32 
 MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c   | 188 

 3 files changed, 226 insertions(+)
 create mode 100644 
MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.inf
 create mode 100644 
MdeModulePkg/Library/UefiSortLib/UnitTest/UefiSortLibUnitTest.c

-- 
2.20.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78350): https://edk2.groups.io/g/devel/message/78350
Mute This Topic: https://groups.io/mt/84523898/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 v2 1/1] SecurityPkg/FvReportPei: remove redundant sizeof

[wenyi,xie via groups.io]

Execuse me, I made a mistake and sent the wrong patch. Please ignore it.

Thanks
Wenyi

On 2021/7/29 15:45, Wenyi Xie wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=
> 
> In function InstallPreHashFvPpi, when calculating the size
> of struct HASH_INFO，sizeof is used twice. This bug does
> not lead to buffer overflow, "sizeof (HASH_INFO)" is 4,
> whereas "sizeof (sizeof (HASH_INFO))" is 4 or 8.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Laszlo Ersek 
> Signed-off-by: Wenyi Xie 
> Reviewed-by: Laszlo Ersek 
> ---
>  SecurityPkg/FvReportPei/FvReportPei.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/SecurityPkg/FvReportPei/FvReportPei.c 
> b/SecurityPkg/FvReportPei/FvReportPei.c
> index d709760ea3ce..e82413e090c0 100644
> --- a/SecurityPkg/FvReportPei/FvReportPei.c
> +++ b/SecurityPkg/FvReportPei/FvReportPei.c
> @@ -67,7 +67,7 @@ InstallPreHashFvPpi (
>HASH_INFO *HashInfo;
>
>PpiSize = sizeof (EDKII_PEI_FIRMWARE_VOLUME_INFO_PREHASHED_FV_PPI)
> -+ sizeof (sizeof (HASH_INFO))
> ++ sizeof (HASH_INFO)
>  + HashSize;
>
>PreHashedFvPpi = AllocatePool (PpiSize);
> 


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

[edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/FvReportPei: remove redundant sizeof

[wenyi,xie via groups.io]

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

In function InstallPreHashFvPpi, when calculating the size
of struct HASH_INFO，sizeof is used twice. This bug does
not lead to buffer overflow, "sizeof (HASH_INFO)" is 4,
whereas "sizeof (sizeof (HASH_INFO))" is 4 or 8.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Laszlo Ersek 
Signed-off-by: Wenyi Xie 
Reviewed-by: Laszlo Ersek 
---
 SecurityPkg/FvReportPei/FvReportPei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/FvReportPei/FvReportPei.c 
b/SecurityPkg/FvReportPei/FvReportPei.c
index d709760ea3ce..e82413e090c0 100644
--- a/SecurityPkg/FvReportPei/FvReportPei.c
+++ b/SecurityPkg/FvReportPei/FvReportPei.c
@@ -67,7 +67,7 @@ InstallPreHashFvPpi (
   HASH_INFO *HashInfo;
 
   PpiSize = sizeof (EDKII_PEI_FIRMWARE_VOLUME_INFO_PREHASHED_FV_PPI)
-+ sizeof (sizeof (HASH_INFO))
++ sizeof (HASH_INFO)
 + HashSize;
 
   PreHashedFvPpi = AllocatePool (PpiSize);
-- 
2.20.1.windows.1



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

[edk2-devel] [PATCH EDK2 v2 0/1] SecurityPkg/FvReportPei: remove redundant sizeof

[wenyi,xie via groups.io]

Main Changes since v1 :
Change commit message.

Wenyi Xie (1):
  SecurityPkg/FvReportPei: remove redundant sizeof

 SecurityPkg/FvReportPei/FvReportPei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.20.1.windows.1



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

Re: [edk2-devel] [edk2-platforms][PATCH V2] PurleyOpenBoardPkg : Support for LINUX Boot

[Nate DeSimone]

Hi Manic,

Unfortunately this patch cannot be merged as is. It appears to contain a 
pre-built binary of the Linux kernel that has been pre-configured for Linuxboot 
use. While this is very convenient, the Linux kernel is licensed under the GPL 
and hence we cannot add it to edk2-platforms, which must be kept as BSD only. 
It might be possible to add this to edk2-non-osi, but even then we would 
require that you provide a readme file that explains how to get and compile the 
source code that you used to build this exact Linux image, as required by the 
GPL.

The easiest and safest option would be to remove the Linux binary all together 
and provide instructions to the user for how to build their own image and add 
it to the tree.

Thanks,
Nate

> -Original Message-
> From: manickavasakam karpagavinayagam 
> Sent: Wednesday, June 30, 2021 2:57 PM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W ; Desimone, Nathaniel L
> ; fel...@ami.com; DOPPALAPUDI,
> HARIKRISHNA ; Jha, Manish ;
> Bobroff, Zachary ; KARPAGAVINAYAGAM,
> MANICKAVASAKAM 
> Subject: [edk2-platforms][PATCH V2] PurleyOpenBoardPkg : Support for
> LINUX Boot
> 
> Support for LINUX Boot
> To enable/disable feature, PcdLinuxBootEnable can be used
> 1.Follow directions on http://osresearch.net/Building/ to compile the
> heads kernel and initrd for qemu-system_x86_64
> 2.Copy the following built files
> (1) initrd.cpio.xz  to
> PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/initrd.cpio.xz
> (2) bzimage to
> PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/linux.efi
> 
> Notes:
> V2 :
> - Rename LinuxBootPkg to LinuxBoot
> - Move LinuxBootPkg to PurleyOpenBoardPkg/Features/LinuxBoot
>   - Follow Coding Standard in LinuxBoot.C and LinuxBoot.h
> 
> Signed-off-by: manickavasakam karpagavinayagam
> 
> ---
>  .../BoardTiogaPass/CoreDxeInclude.dsc |   5 +-
>  .../BoardTiogaPass/CoreUefiBootInclude.fdf|   5 +-
>  .../BoardTiogaPass/OpenBoardPkg.dsc   |   7 +
>  .../BoardTiogaPass/OpenBoardPkg.fdf   |  57 ++-
>  .../BoardTiogaPass/PlatformPkgConfig.dsc  |   7 +
>  .../LinuxBoot/LinuxBinaries/LinuxKernel.inf   |  17 +
>  .../LinuxBoot/LinuxBinaries/initrd.cpio.xz| Bin 0 -> 16 bytes
>  .../LinuxBoot/LinuxBinaries/linux.efi | Bin 0 -> 16 bytes
>  .../Features/LinuxBoot/LinuxBoot.c| 412 ++
>  .../Features/LinuxBoot/LinuxBoot.h| 185 
>  .../Features/LinuxBoot/LinuxBoot.inf  |  40 ++
>  .../Features/LinuxBoot/LinuxBootNull.c|  36 ++
>  .../Features/LinuxBoot/LinuxBootNull.inf  |  25 ++
>  .../Intel/PurleyOpenBoardPkg/OpenBoardPkg.dec |   2 +
>  .../DxePlatformBootManagerLib/BdsPlatform.c   |   9 +
>  .../DxePlatformBootManagerLib.inf |   2 +
>  Platform/Intel/Readme.md  |  42 ++
>  17 files changed, 843 insertions(+), 8 deletions(-)  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/Linu
> xKernel.inf
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/initr
> d.cpio.xz
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBinaries/linu
> x.efi
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBoot.c
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBoot.h
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBoot.inf
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBootNull.c
>  create mode 100644
> Platform/Intel/PurleyOpenBoardPkg/Features/LinuxBoot/LinuxBootNull.inf
> 
> diff --git
> a/Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/CoreDxeInclude.dsc
> b/Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/CoreDxeInclude.dsc
> index b0660d72dd..a17015704b 100644
> ---
> a/Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/CoreDxeInclude.dsc
> +++
> b/Platform/Intel/PurleyOpenBoardPkg/BoardTiogaPass/CoreDxeInclude.ds
> +++ c
> @@ -83,6 +83,7 @@
> 
> $(PLATFORM_BOARD_PACKAGE)/Override/MdeModulePkg/Bus/Pci/PciBus
> Dxe/PciBusDxe.inf
>  #TiogaPass Override END
> 
> +!if gPlatformTokenSpaceGuid.PcdLinuxBootEnable == FALSE
>MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
>MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
>MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> @@ -97,10 +98,11 @@
>MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
> 
> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>FatPkg/EnhancedFatDxe/Fat.inf
> -
> +!endif
> 
> #MdeModulePkg/Universal/Console/GraphicsOutputDxe/GraphicsOutputD
> xe.inf
> 
> MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleD
> xe.inf
> 
> +!if gPlatformTokenSpaceGuid.PcdLinuxBootEnable == FALSE
>MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
>MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
> 
> 

Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

[Min Xu]

On July 29, 2021 12:29 PM, Brijesh Singh wrote:
> On 7/28/21 9:44 PM, Xu, Min M wrote:
> > Jiewen & Singh
> >
> > From the discussion I am thinking we have below rules to follow to the
> > design the structure of TEE_WORK_AREA:
> > 1. Design should be flexible but not too complicated 2. Reuse the
> > current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as TEE_WORK_AREA 3.
> > TEE_WORK_AREA should be initialized to all-0 at the beginning of
> > ResetVecotr 4. Reduce the changes to exiting code if possible
> >
> > So I try to make below conclusions below: (Please review) 1.
> > SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and SEV,
> > maybe in the future it can be used by other CC technologies.
> >
> > 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is
> > guaranteed to be cleared in legacy guest. In TDX this memory region is
> > initialized to be all-0 by host VMM. In SEV the memory region is cleared as 
> > well.
> >   0x00B000|0x001000
> >
> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpace
> Guid.PcdSevEsWorkAreaSize
> >   DATA = {
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> >   }
> 
> Hmm, I thought the contents of the data pages are controlled by the host VMM.
> If the backing pages are not zero filled then there is no guarantee that 
> memory
> will be zero.  To verify it:
> 
> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA values
> from 0x00 -> 0xCC
> 
> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry
> 
> And dump does not contain the 0xcc.
> 
> And to confirm further,  I attached to the qemu with the GDB before the 
> booting
> the OVMF, and modified the SevEsWorkArea with some garbage number  and
> this time the dump printed garbage value I put through the debugger.
> 
> In summary, the OVMF to zero the workarea memory on the entry and we
> cannot rely on the DATA={0x00, 0x00...} to zero the CCWorkArea.
So in legacy guest, CCWorkArea is cleared to all-0 without the 
DATA={0x00,0x00...}, right?

> 
> Did I miss something ?
> 
> 
> >
> > 3. The structure of TEE_WORK_AREA
> >   The current SEV_ES_WORK_AREA is defined as below:
> >   typedef struct {
> > UINT8SevEsEnabled;
> > UINT8Reserved1[7];
> > [Others...]
> > } SEC_SEV_ES_WORK_AREA;
> >
> > So I think the TEE_WORK_AREA can be:
> >   Byte[0] Type:
> > 0: legacy 1: SEV2: TDX
> >   Byte[1] Subtype:
> >   If Type is 0, then it is 0
> > If Type is 1, then it is up to SEV's definition
> > If Type is 2, then it is up to TDX's definition  Byte[] other bytes
> > are defined based on the Type/Subtype
> 
> I personally like Yao Jiewen's struct definition, but I can also live with 
> this one as
> well :). The only question I had was with his proposal was what if we remove 
> the
> Length and Version fields. If the header length was fixed then life would be 
> much
> easier in the ASM code.
Yao Jiewen's structure is like below. If the HeaderVersion/HeaderLength are 
removed
you will find it is just what I am saying. The first 2 bytes are used to 
distinguish the
legacy/SEV/TDX. The left bytes are up to the first 2 bytes.
typedef struct {
 UINT8HeaderVersion; // 0
 UINT8HeadLength; // 4
 UINT8Type; // 0 - legacy, 1 - SEV, 2 - TDX
 UINT8SubType; // Type specific sub type, if needed.
} CC_COMMON_WORK_AREA_HEADER;

typedef struct {
 CC_COMMON_WORK_AREA_HEADER Header;
 // reset is valid if Type == 1
 UINT8Reserved1[4];
 UINT64   RandomData;
 UINT64   EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

typedef struct {
 CC_COMMON_WORK_AREA_HEADER Header;
 // reset is valid if Type == 2
 UINT8TdxSpecific[];  // TBD
} TDX_WORK_AREA;
> 
> 
> > I check the code in SecMain.c.
> >  SevEsIsEnabled() need updated to check SevEsWorkarea->SevEsEnabled == 1,
> not non-0.
> > @Brijesh Singh Is there any other code need update?
> 
> As noted before, the SevEsWorkAreas is used to pass the information from the
> Sec to PEI phase. The workarea gets reused for totally different purpose after
> the PEI phase.
So only the above line in SecMain.c/SevEsIsEnabled() need updated, right?
> 
Thanks!
Xu, Min


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

Re: [edk2-devel] [edk2-platforms][PATCH v1] MinPlatformPkg/Test/TestPointCheckLib: Correctly print memory map entry

[Chiu, Chasel]

Reviewed-by: Chasel Chiu 


> -Original Message-
> From: Benjamin Doron 
> Sent: Tuesday, July 27, 2021 6:56 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Liming Gao ;
> Dong, Eric 
> Subject: [edk2-platforms][PATCH v1] MinPlatformPkg/Test/TestPointCheckLib:
> Correctly print memory map entry
> 
> In the case that there are too many EfiRuntimeServicesData entries, this was
> incorrectly printing the number of EfiRuntimeServicesCode entries.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Signed-off-by: Benjamin Doron 
> ---
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckMem
> oryMap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckMe
> moryMap.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckMe
> moryMap.c
> index fce44bf73e9d..b8ebac8fe304 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckMe
> moryMap.c
> +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckMemoryMap.c
> @@ -135,7 +135,7 @@ TestPointCheckUefiMemoryMapEntry (
>  DEBUG ((DEBUG_ERROR, "EfiRuntimeServicesCode entry - %d\n",
> EntryCount[EfiRuntimeServicesCode]));   }   if
> (EntryCount[EfiRuntimeServicesData] > 1) {-DEBUG ((DEBUG_ERROR,
> "EfiRuntimeServicesData entry - %d\n", EntryCount[EfiRuntimeServicesCode]));+
> DEBUG ((DEBUG_ERROR, "EfiRuntimeServicesData entry - %d\n",
> EntryCount[EfiRuntimeServicesData]));   }   if (EntryCount[EfiACPIMemoryNVS] >
> 1) { DEBUG ((DEBUG_ERROR, "EfiACPIMemoryNVS entry - %d\n",
> EntryCount[EfiACPIMemoryNVS]));@@ -300,4 +300,4 @@
> TestPointCheckUefiMemoryMap (
>  Done:   DEBUG ((DEBUG_INFO, " TestPointCheckUefiMemoryMap -
> Exit\n"));   return Status;-}
> \ No newline at end of file
> +}--
> 2.31.1



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