Date   

[edk2-platforms][PATCH v2 0/5] Platform/Sgi: Miscellaneous updates

Pranav Madhu
 

Changes since V1:
- Rebase the patches on top of latest master branch

This patch series introduces few miscellaneous updates for Arm's
Reference Design platforms. The first two patches add a template
support for hardware reduced ACPI events. This allows helps the
platform get better compliance coverage with SystemReady test
suite. The third patch adds a build macro for enabling graphics
output via the HDLCD. The fourth patch allow build time control
for CPPC and LPI features on the platform. The fifth patch fixes
StandaloneMM build options for the RD platforms.

Link to github branch with the patches in this series -
https://github.com/Pranav-Madhu/edk2-platforms/tree/topic/rd_misc

Omkar Anand Kulkarni (1):
Platform/Sgi: Cleanup build options for StandaloneMM context

Pranav Madhu (3):
Platform/Sgi: Enable PrimeCell GPIO
Platform/Sgi: Add GED support
Platform/Sgi: update _OSC control method to control LPI and CPPC

Thomas Abraham (1):
Platform/Sgi: define the macro ENABLE_GOP

Platform/ARM/SgiPkg/SgiPlatform.dec | 14 +++
Platform/ARM/SgiPkg/SgiMemoryMap.dsc.inc | 10 ++
Platform/ARM/SgiPkg/SgiMemoryMap2.dsc.inc | 10 ++
Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 19 +++
Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc | 6 +-
Platform/ARM/SgiPkg/SgiPlatform.fdf | 2 +
.../SgiPkg/AcpiTables/RdE1EdgeAcpiTables.inf | 7 ++
.../SgiPkg/AcpiTables/RdN1EdgeAcpiTables.inf | 8 ++
.../AcpiTables/RdN1EdgeX2AcpiTables.inf | 8 ++
.../ARM/SgiPkg/AcpiTables/RdN2AcpiTables.inf | 9 ++
.../SgiPkg/AcpiTables/RdN2Cfg1AcpiTables.inf | 9 ++
.../ARM/SgiPkg/AcpiTables/RdV1AcpiTables.inf | 9 ++
.../SgiPkg/AcpiTables/RdV1McAcpiTables.inf | 9 ++
.../SgiPkg/AcpiTables/Sgi575AcpiTables.inf | 9 ++
Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h | 2 +
.../ARM/SgiPkg/AcpiTables/RdN1Edge/Dsdt.asl | 8 ++
.../ARM/SgiPkg/AcpiTables/RdN1EdgeX2/Dsdt.asl | 8 ++
Platform/ARM/SgiPkg/AcpiTables/RdN2/Dsdt.asl | 15 +++
.../ARM/SgiPkg/AcpiTables/RdN2Cfg1/Dsdt.asl | 15 +++
Platform/ARM/SgiPkg/AcpiTables/RdV1/Dsdt.asl | 15 +++
.../ARM/SgiPkg/AcpiTables/RdV1Mc/Dsdt.asl | 15 +++
.../ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl | 8 ++
Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl | 116 ++++++++++++++++++
23 files changed, 327 insertions(+), 4 deletions(-)
create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SsdtEvents.asl

--=20
2.17.1


Re: [edk2-test][PATCH v2 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Heinrich Schuchardt
 

On 11.06.21 10:35, Sunny Wang wrote:
The commits a9d1fb58 and ae7e5477b555 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commits a9d1fb58 and
ae7e5477b555 to not create event with TPL_HIGH_LEVEL to be compliant
with UEFI Spec and fix the failures.

For more information, https://edk2.groups.io/g/devel/message/76338

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
.../EventTimerTaskPriorityServicesBBTestCreateEventEx.c | 4 +---
2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
index eb458de5..03b7ae6e 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2016 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -228,7 +229,6 @@ BBTestCreateEventEx_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] = {
@@ -318,7 +318,6 @@ BBTestCreateEventEx_Conf_Sub2 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] = {
@@ -413,7 +412,6 @@ BBTestCreateEventEx_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] = {


Re: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Sunny Wang
 

Thanks for catching this, Heinrich.
I sent v2 to also revert TPL_HIGH_LEVEL changes in ae7e5477b555.

Best Regards,
Sunny Wang

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Heinrich Schuchardt via groups.io
Sent: Friday, June 11, 2021 4:24 PM
To: Sunny Wang <Sunny.Wang@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; G Edhaya Chandran <Edhaya.Chandran@arm.com>; Barton Gao <gaojie@byosoft.com.cn>; Michael D Kinney <michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

On 11.06.21 09:07, Sunny Wang wrote:
The commit a9d1fb58 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commit a9d1fb58 to not create
event with TPL_HIGH_LEVEL to be compliant with UEFI Spec and fix the
failures.

For more information, https://edk2.groups.io/g/devel/message/76338
Please, have a look at commit ae7e5477b555 ("uefi-sct/SctPkg: allowable
NotifyTpl in CreateEventEx") too.

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;





IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[edk2-test][PATCH v2 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Sunny Wang
 

The commits a9d1fb58 and ae7e5477b555 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commits a9d1fb58 and
ae7e5477b555 to not create event with TPL_HIGH_LEVEL to be compliant
with UEFI Spec and fix the failures.

For more information, https://edk2.groups.io/g/devel/message/76338

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
.../EventTimerTaskPriorityServicesBBTestCreateEventEx.c | 4 +---
2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTas=
kPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreate=
Event.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPr=
iorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEve=
nt.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@
=20
Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.
=20
This program and the accompanying materials
are licensed and made available under the terms and conditions of the =
BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTas=
kPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreate=
EventEx.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTask=
PriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateE=
ventEx.c
index eb458de5..03b7ae6e 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx=
.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx=
.c
@@ -2,6 +2,7 @@
=20
Copyright 2006 - 2016 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.
=20
This program and the accompanying materials
are licensed and made available under the terms and conditions of the =
BSD License
@@ -228,7 +229,6 @@ BBTestCreateEventEx_Conf_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] =3D {
@@ -318,7 +318,6 @@ BBTestCreateEventEx_Conf_Sub2 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] =3D {
@@ -413,7 +412,6 @@ BBTestCreateEventEx_Conf_Sub3 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_GUID *EventGroups[] =3D {
--=20
2.31.0.windows.1


Re: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Heinrich Schuchardt
 

On 11.06.21 09:07, Sunny Wang wrote:
The commit a9d1fb58 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commit a9d1fb58 to not create
event with TPL_HIGH_LEVEL to be compliant with UEFI Spec and fix the
failures.

For more information, https://edk2.groups.io/g/devel/message/76338
Please, have a look at commit ae7e5477b555 ("uefi-sct/SctPkg: allowable
NotifyTpl in CreateEventEx") too.

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@

Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.

This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] = {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;


Re: [PATCH v1 2/5] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate

Wu, Hao A
 

A couple of minor comments below:

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1 2/5] MdeModulePkg: PiSmmIpl: Update MessageLength
calculation for MmCommunicate

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

This change updated calculation routine for MM communication in PiSmmIpl.
It removes ambiguity brought in by UINTN variables from this routine and
paves way for updating definition of field MessageLength in
EFI_MM_COMMUNICATE_HEADER to definitive size.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 13 ++++++++++++-
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 599a0cd01d80..9508715fda24 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -34,6 +34,7 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h> // BZ3398

I suggest to remove the comment "// BZ3398" here.

I think users can use the 'blame' feature of the version control systems
together with the commit log message to find out the information.



#include "PiSmmCorePrivateData.h"

@@ -515,6 +516,7 @@ SmmCommunicationCommunicate (
EFI_STATUS Status;
EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader;
BOOLEAN OldInSmm;
+ UINT64 BZ3398_LongCommSize;

Suggest to drop the "BZ3398_" prefix for the variable name.


UINTN TempCommSize;

//
@@ -527,7 +529,16 @@ SmmCommunicationCommunicate (
CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)
CommBuffer;

if (CommSize == NULL) {
- TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data)
+ CommunicateHeader->MessageLength;
+ // BZ3398 Starts: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.

Suggest to drop the "// BZ3398 Starts:" and "// BZ3398 Ends" comments pair here.

With above handled:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


+ Status = SafeUint64Add (OFFSET_OF
(EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader-
MessageLength, &BZ3398_LongCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ Status = SafeUint64ToUintn (BZ3398_LongCommSize, &TempCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ // BZ3398 Ends
} else {
TempCommSize = *CommSize;
//
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
index 6109d6b5449c..87142e27fa47 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
DxeServicesLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib #BZ3398

[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES
--
2.31.1.windows.1


Re: [PATCH v1 4/5] MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation

Wu, Hao A
 

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1 4/5] MdeModulePkg: SmiHandlerProfileInfo: Updated
MessageLength calculation

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to consumers.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
| 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
.c
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
.c
index 4153074b7a80..56d80d1a9ce1 100644
---
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo
.c
+++
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileIn
+++ fo.c
@@ -116,7 +116,9 @@ GetSmiHandlerProfileDatabase(
CommGetInfo->Header.ReturnStatus = (UINT64)-1;
CommGetInfo->DataSize = 0;

- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.

Similar comments as in patch 3/5.

Best Regards,
Hao Wu


+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate(SmmCommunication,
CommBuffer, &CommSize);
if (EFI_ERROR(Status)) {
Print(L"SmiHandlerProfile: SmmCommunication - %r\n", Status); @@ -
149,7 +151,9 @@ GetSmiHandlerProfileDatabase(
CommGetData->Header.DataLength = sizeof(*CommGetData);
CommGetData->Header.ReturnStatus = (UINT64)-1;

- CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *)CommHeader + CommSize;
Size -= CommSize;

--
2.31.1.windows.1


Re: [PATCH v1 3/5] MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation

Wu, Hao A
 

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com>
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH v1 3/5] MdeModulePkg: MemoryProfileInfo: Updated
MessageLength calculation

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

This change replaced the calculation of communication buffer size from
explicitly adding the size of each member with the OFFSET macro function.
This will make the structure field defition change transparent to consumers.

I think there is one missing place in function GetSmramProfileData():

MinimalSizeNeeded = sizeof (EFI_GUID) +
sizeof (UINTN) +
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_INFO),
MAX (sizeof (SMRAM_PROFILE_PARAMETER_GET_PROFILE_DATA_BY_OFFSET),
sizeof (SMRAM_PROFILE_PARAMETER_RECORDING_STATE)));

More inline comments below:



Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 20
+++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git
a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
index 191c31068545..39ed8b2e0484 100644
--- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+++
b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
@@ -1190,7 +1190,9 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState =
MEMORY_PROFILE_RECORDING_DISABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.

Please help to drop the explicit mention of BZ3398 in the comment.
How about using:
//
// The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here.
//

There are 4 more similar cases below.

Best Regards,
Hao Wu


+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication,
CommBuffer, &CommSize);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "SmramProfile: SmmCommunication - %r\n",
Status)); @@ -1213,7 +1215,9 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState =
MEMORY_PROFILE_RECORDING_DISABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer,
&CommSize);
}

@@ -1230,7 +1234,9 @@ GetSmramProfileData (
CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1;
CommGetProfileInfo->ProfileSize = 0;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Status = SmmCommunication->Communicate (SmmCommunication,
CommBuffer, &CommSize);
ASSERT_EFI_ERROR (Status);

@@ -1261,7 +1267,9 @@ GetSmramProfileData (
CommGetProfileData->Header.DataLength = sizeof
(*CommGetProfileData);
CommGetProfileData->Header.ReturnStatus = (UINT64)-1;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
Buffer = (UINT8 *) CommHeader + CommSize;
Size -= CommSize;

@@ -1320,7 +1328,9 @@ GetSmramProfileData (
CommRecordingState->Header.ReturnStatus = (UINT64)-1;
CommRecordingState->RecordingState =
MEMORY_PROFILE_RECORDING_ENABLE;

- CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader-
MessageLength;
+ // BZ3398: Make MessageLength the same size in
EFI_MM_COMMUNICATE_HEADER for both IA32 and X64.
+ // The CommHeader->MessageLength contains a definitive value, thus
UINTN cast is safe here.
+ CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) +
+ (UINTN)CommHeader->MessageLength;
SmmCommunication->Communicate (SmmCommunication, CommBuffer,
&CommSize);
}

--
2.31.1.windows.1


Re: [PATCH v1 1/5] EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update

Wu, Hao A
 

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
Sent: Thursday, June 10, 2021 9:43 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>;
Andrew Fish <afish@apple.com>; Laszlo Ersek <lersek@redhat.com>; Leif
Lindholm <leif@nuviainc.com>
Subject: [edk2-devel] [PATCH v1 1/5] EDK2 Code First: PI Specification:
EFI_MM_COMMUNICATE_HEADER Update

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

This change includes specification update markdown file that describes the
proposed PI Specification v1.7 Errata A in detail and potential impact to the
existing codebase.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>

Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
BZ3430-SpecChange.md | 88 ++++++++++++++++++++
1 file changed, 88 insertions(+)

diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md new file mode
100644 index 000000000000..33a1ffda447b
--- /dev/null
+++ b/BZ3430-SpecChange.md
@@ -0,0 +1,88 @@
+# Title: Change MessageLength Field of EFI_MM_COMMUNICATE_HEADER
to
+UINT64
+
+## Status: Draft
+
+## Document: UEFI Platform Initialization Specification Version 1.7
+Errata A
+
+## License
+
+SPDX-License-Identifier: CC-BY-4.0
+
+## Submitter: [TianoCore Community](https://www.tianocore.org)
+
+## Summary of the change
+
+Change the `MessageLength` Field of `EFI_MM_COMMUNICATE_HEADER`
from UINTN to UINT64 to remove architecture dependency:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+## Benefits of the change
+
+In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the
MessageLength field of `EFI_MM_COMMUNICATE_HEADER` (also defined as
`EFI_SMM_COMMUNICATE_HEADER`) is defined as type UINTN.
+
+But this structure, as a generic definition, could be used for both PEI and
DXE MM communication. Thus for a system that supports PEI MM launch,
but operates PEI in 32bit mode and MM foundation in 64bit, the current
`EFI_MM_COMMUNICATE_HEADER` definition will cause structure parse
error due to UINTN used.
+
+## Impact of the change
+
+This change will impact the known structure consumers including:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl
+MdeModulePkg/Application/SmiHandlerProfileInfo
+MdeModulePkg/Application/MemoryProfileInfo
+```
+
+For consumers that are not using
`OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)`, but performing
explicit addition such as the existing
MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c,
one will need to change code implementation to match new structure
definition. Otherwise, the code compiled on IA32 architecture will
experience structure field dereference error.
+
+User who currently uses UINTN local variables as place holder of
MessageLength will need to use caution to make cast from UINTN to UINT64
and vice versa. It is recommended to use `SafeUint64ToUintn` for such
operations when the value is indeterministic.
+
+Note: MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib is also
consuming this structure, but it handled this size discrepancy internally. If this

Hello Kun,

Sorry for a question.
I am not sure why the current codes in file SmmLockBoxDxeLib.c will work properly after the UINTN -> UINT64 change.

For example:
LockBoxGetSmmCommBuffer():
MinimalSizeNeeded = sizeof (EFI_GUID) +
sizeof (UINTN) +
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SAVE),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE),
MAX (sizeof (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE),
sizeof (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)))));

SaveLockBox():
UINT8 TempCommBuffer[sizeof(EFI_GUID) + sizeof(UINTN) + sizeof(EFI_SMM_LOCK_BOX_PARAMETER_SAVE)];

Is the series missed changes for SmmLockBoxDxeLib or I got something wrong?

Best Regards,
Hao Wu


potential spec change is not applied, all applicable PEI MM communicate
callers will need to use the same routine as that of SmmLockBoxPeiLib to
invoke a properly populated EFI_MM_COMMUNICATE_HEADER to be used
in X64 MM foundation.
+
+## Detailed description of the change [normative updates]
+
+### Specification Changes
+
+1. In PI Specification v1.7 Errata A: Vol. 4 Page-91, the definition of
`EFI_MM_COMMUNICATE_HEADER` should be changed from current:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINTN MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+to:
+
+```c
+typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT64 MessageLength;
+ UINT8 Data[ANYSIZE_ARRAY];
+} EFI_MM_COMMUNICATE_HEADER;
+```
+
+### Code Changes
+
+1. Remove the explicit calculation of the offset of `Data` in
`EFI_MM_COMMUNICATE_HEADER`. Thus applicable calculations of
`sizeof(EFI_GUID) + sizeof(UINTN)` should be replaced with
`OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)` or similar
alternatives. These calculations are identified in:
+
+```bash
+MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.
c
+MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+```
+
+1. Resolve potentially mismatched type between `UINTN` and `UINT64`.
This would occur when `MessageLength` or its derivitive are used for local
calculation with existing `UINTN` typed variables. Code change regarding this
perspective is per case evaluation: if the variables involved are all
deterministic values, and there is no overflow or underflow risk, a cast
operation (from `UINTN` to `UINT64`) can be safely used. Otherwise, the
calculation will be performed in `UINT64` bitwidth and then convert to
`UINTN` using `SafeUint64*` and `SafeUint64ToUintn`, respectively. These
operations are identified in:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.
c
+MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c
+```
+
+1. After all above changes applied and specification updated,
`MdePkg/Include/Protocol/MmCommunication.h` will need to be updated to
match new definition that includes the field type update.
--
2.31.1.windows.1





Re: Commit a9d1fb58 - uefi-sct/SctPkg: allowable NotifyTpl in CreateEvent

Sunny Wang
 

I just sent a patch for this. https://edk2.groups.io/g/devel/message/76368
Please help review the patch so that we can pick this up in SCT stable release.

Best Regards,
Sunny Wang

-----Original Message-----
From: Andrew Fish <afish@apple.com>
Sent: Friday, June 11, 2021 2:50 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; xypron.glpk@gmx.de
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Barton Gao <gaojie@byosoft.com.cn>; G Edhaya Chandran <Edhaya.Chandran@arm.com>; Mike Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] Commit a9d1fb58 - uefi-sct/SctPkg: allowable NotifyTpl in CreateEvent



On Jun 10, 2021, at 11:05 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

On 6/10/21 7:22 PM, Samer El-Haj-Mahmoud wrote:
+ Mike

I agree with Sunny's evaluation. The original EDK2 patch from Jan had good feedback from Mike K.. The exact language from the UEFI spec section 7.1 is:

" Consequently, there is a fourth TPL, TPL_HIGH_LEVEL, designed for use exclusively by the firmware."
Firmware includes drivers. Drivers use CreateEvent().


" This is the highest priority level. It is not interruptible (interrupts are disabled) and is used sparingly by firmware to synchronize operations that need to be accessible from any priority level. For example, it must be possible to signal events while executing at any priority level. Therefore, firmware manipulates the internal event structure while at this priority level."

Given this, I think the following needs to happen:

1. The SCT commit (https://github.com/tianocore/edk2-test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58 ) needs to be reverted
2. The SCT change can be re-submitted as a new patch, without adding TPL_HIGH_LEVEL
3. The original EDK2 patch on Jan 6 (https://edk2.groups.io/g/devel/topic/79479680#69853) needs to be re-submitted per Mike's feedback. This means not adding the TPL_HIGH_LEVEL support in CreateEvent

In my opinion, (1) needs to be reverted right way, as it is causing SCT failures on many (all?) EDK2 based systems. This needs to happen before the edk2-test stable tag is created
Table 7-3 TPL Restrictions in UEFI spec 2.9 explicitly has:

Event Notification Levels
TPL_APPLICATION
<= TPL_HIGH_LEVEL

Nowhere does the specification say that CreateEvent() cannot be called
with with TPL_HIGH_LEVEL.

I can understand why Michael suggests that CreateEvent() should not be
callable at TPL_HIGH_LEVEL. But this requires a change request updating
the specification.
This is the other bit of the spec that might be relevant….

7.1 Event, Timer, and Task Priority Services
...
Execution in the boot services environment occurs at different task priority levels, or TPLs. The boot services environment exposes only three of these levels to UEFI applications and drivers:
• TPL_APPLICATION, the lowest priority level
• TPL_CALLBACK, an intermediate priority level
• TPL_NOTIFY, the highest priority level

Thanks,

Andrew Fish

I am missing this step in the action list above.

1) and 2) can be done with a single patch. Just don't revert the line
adding TPL_APPLICATION.

Best regards

Heinrich


Any thought about these suggested next steps?

Thanks,
--Samer

-----Original Message-----
From: Sunny Wang <Sunny.Wang@arm.com>
Sent: Thursday, June 10, 2021 4:26 AM
To: Heinrich Schuchardt <xypron.glpk@gmx.de>; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com>; Barton Gao
<gaojie@byosoft.com.cn>; G Edhaya Chandran
<Edhaya.Chandran@arm.com>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Subject: RE: Commit a9d1fb58

Thanks for the information, Heinrich.

I think we need to further work on this for Mike's comment with your
ongoing edk2 patch.
-
https://edk2.groups.io/g/devel/message/69853?p=,,,20,0,0,0::relevance,,M
deModulePkg%3A+correct+TPL+level+check+CoreCreateEventEx,20,2,0,794
79680
For addressing Mike's comment, we will need to update both of your edk2
and edk2-test patches.
Therefore, I would suggest NOT picking up your edk2-test commit into the
upcoming stable tag/release. What do you guys think?

In other words, the main problem is that SCT should not create an event with
TPL_HIGH_LEVEL, so we need to revert TPL_HIGH_LEVEL related change in
Heinrich's commit https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58.

By the way, I confirmed this issue on two of my AARCH64 platforms (RPi4 and
another one).

Best Regards,
Sunny Wang

-----Original Message-----
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Sent: Thursday, June 10, 2021 1:04 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Barton
Gao <gaojie@byosoft.com.cn>; G Edhaya Chandran
<Edhaya.Chandran@arm.com>; Sunny Wang <Sunny.Wang@arm.com>
Subject: Re: Commit a9d1fb58

On 6/10/21 4:10 AM, Samer El-Haj-Mahmoud wrote:
With commit
https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58
<https://github.com/tianocore/edk2-
test/commit/421a6997ef362c6286c4ef87d21d5367a9d1fb58>
, I am seeing the following new failure on some EDK2 based systems:

BS.CreateEvent - Create event with all valid event type and supported TPL.
--
*FAILURE*

EF317ADE-8668-456F-BED9-766056672DFF

/RELEASE_BUILD/edk2-test/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/
BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c:520:Statu
s - Invalid Parameter, EventType - 0x00000100, NotifyTpl -
31

BS.CreateEvent - Create event with all valid event type and supported TPL.
--
*FAILURE*

EF317ADE-8668-456F-BED9-766056672DFF

/RELEASE_BUILD/edk2-test/uefi-
sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/
BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c:520:Statu
s - Invalid Parameter, EventType - 0x00000200, NotifyTpl -
31

Are we sure this is implemented properly in EDK2? I imagine this code is
common in TianoCore/EDK2, and all systems will experience such failure.

I confirmed this on one AARCH64 system. I am trying to confirm on
another AARCH64 as well.

Can anyone confirm on other systems/archs?

If there is going to be a stable tag/release of edk2-test, we may want
to confirm this change does not cause failures in many systems before we
pick it up.

Thanks,

--Samer
An EDK II patch is available since 2021-01-06 via
https://bugzilla.tianocore.org/show_bug.cgi?id=3058
Cf. https://edk2.groups.io/g/devel/message/69851

Best regards

Heinrich
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.




IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Not create event with TPL_HIGH_LEVEL

Sunny Wang
 

The commit a9d1fb58 caused SCT BS.CreateEvent failures.

Section 7.1 of the UEFI Spec states that TPL_HIGH_LEVEL is designed for
exclusive use by the firmware. The creation of events by UEFI
applications, UEFI drivers, and UEFI OS Loaders should not use this TPL
level.

Therefore, revert TPL_HIGH_LEVEL change in commit a9d1fb58 to not create
event with TPL_HIGH_LEVEL to be compliant with UEFI Spec and fix the
failures.

For more information, https://edk2.groups.io/g/devel/message/76338

Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: G Edhaya Chandran <edhaya.chandran@arm.com>
Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../EventTimerTaskPriorityServicesBBTestCreateEvent.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTas=
kPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreate=
Event.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPr=
iorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEve=
nt.c
index a7e7366e..d5c033f7 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriori=
tyServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEvent.c
@@ -2,6 +2,7 @@
=20
Copyright 2006 - 2012 Unified EFI, Inc.<BR>
Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021, ARM Limited. All rights reserved.
=20
This program and the accompanying materials
are licensed and made available under the terms and conditions of the =
BSD License
@@ -190,7 +191,6 @@ BBTestCreateEvent_Conf_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -342,7 +342,6 @@ BBTestCreateEvent_Conf_Sub3 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -407,7 +406,6 @@ BBTestCreateEvent_Conf_Sub4 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
@@ -482,7 +480,6 @@ BBTestCreateEvent_Func_Sub1 (
EFI_TPL NotifyTpls[] =3D {
TPL_CALLBACK,
TPL_NOTIFY,
- TPL_HIGH_LEVEL,
0
};
EFI_TEST_ASSERTION AssertionType;
--=20
2.31.0.windows.1


Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF

Min Xu
 

In today’s TianoCore Design Meeting we reviewed the Overview Section (from slide 1 to 20). Thanks much for the valuable feedbacks and comments. The meeting minutes will be sent out soon.

 

To address the concerns of the *one binary* solution in previous discussion, we propose 2 Configurations for TDVF to upstream. (slide 6 - 8)

 

Config-A:

  • Merge the *basic* TDVF feature to existing OvmfX64Pkg.dsc. (Align with existing SEV)
  • Threat model: VMM is NOT out of TCB. (We don’t make things worse.)
  • The OvmfX64Pkg.dsc includes SEV/TDX/normal OVMF basic boot capability. The final binary can run on SEV/TDX/normal OVMF
  • No changes to existing OvmfPkgX64 image layout.
  • No need to add additional security features if they do not exist today
  • No need to remove features if they exist today.
  • RTMR is not supported
  • PEI phase is NOT skipped in either Td or Non-Td

        

Config-B:

  • Add a standalone IntelTdx.dsc to a TDX specific directory for a *full* feature TDVF. (Align with existing SEV)
  • Threat model: VMM is out of TCB. (We need necessary change to prevent attack from VMM)
  • IntelTdx.dsc includes TDX/normal OVMF basic boot capability. The final binary can run on TDX/normal OVMF
  • It might eventually merge with AmdSev.dsc, but NOT at this point of time. And we don’t know when it will happen. We need sync with AMD in the community, after both of us think the solutions are mature to merge.
  • Need to add necessary security feature as mandatory requirement, such as RTMR based Trusted Boot support
  • Need to remove unnecessary attack surfaces, such as network stack.

 

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Min Xu
Sent: Friday, June 11, 2021 6:30 AM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>; rfc@edk2.groups.io
Cc: jejb@...; Laszlo Ersek <lersek@...>; Brijesh Singh <brijesh.singh@...>; Tom Lendacky <thomas.lendacky@...>; erdemaktas@...; cho@...; bret.barkelew@...; Jon Lange <jlange@...>; Karen Noel <knoel@...>; Paolo Bonzini <pbonzini@...>; Nathaniel McCallum <npmccallum@...>; Dr. David Alan Gilbert <dgilbert@...>; Ademar de Souza Reis Jr. <areis@...>
Subject: Re: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF

 

Hi, All

Thanks much for the valuable comments and discussion about the design.

We have updated the slides (v0.9) in below link. If some comments or concerns are not answered/addressed in the new slides, please don’t hesitate to tell us. We do want to answer/address all the comments/concerns. But to be honest it is a rather complicated one and we appreciate your feedbacks.

https://edk2.groups.io/g/devel/files/Designs/2021/0611/TDVF_Design_Review%28v0.9%29.pptx

 

Thanks much!

 

Xu Min

 

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
Sent: Thursday, June 3, 2021 9:51 PM
To: rfc@edk2.groups.io; devel@edk2.groups.io
Cc: jejb@...; Laszlo Ersek <lersek@...>; Brijesh Singh <brijesh.singh@...>; Tom Lendacky <thomas.lendacky@...>; erdemaktas@...; cho@...; bret.barkelew@...; Jon Lange <jlange@...>; Karen Noel <knoel@...>; Paolo Bonzini <pbonzini@...>; Nathaniel McCallum <npmccallum@...>; Dr. David Alan Gilbert <dgilbert@...>; Ademar de Souza Reis Jr. <areis@...>
Subject: [edk2-rfc] [edk2-devel] RFC: design review for TDVF in OVMF

 

Hi, All

We plan to do a design review for TDVF in OVMF package.

 

The TDVF Design slides for TinaoCore Design Review Meeting (Jun 11) is now available in blow link: https://edk2.groups.io/g/devel/files/Designs/2021/0611.

The Bugzilla is https://bugzilla.tianocore.org/show_bug.cgi?id=3429

 

You can have an offline review first. You comments will be warmly welcomed and we will continuously update the slides based on the feedbacks.

 

Thank you

Yao Jiewen

 

 

 


Re: [EXTERNAL] RE: [edk2][PATCH] MdePkg : Add IPMI Macro and Structure Defintions to resolve the IPMI build error

Manickavasakam Karpagavinayagam <manickavasakamk@...>
 

Issac :

We defined the macros as these macros are already used in edk2-platforms\Features\Intel\OutOfBandManagement\IpmiFeaturePkg\GenericIpmi\Dxe\IpmiInit.c GetDeviceId()
Please let us know who needs to correct the edk2-platforms\Features\Intel\OutOfBandManagement\IpmiFeaturePkg\GenericIpmi\Dxe\IpmiInit.c GetDeviceId()

Thank you

-Manic

-----Original Message-----
From: Oram, Isaac W <isaac.w.oram@intel.com>
Sent: Thursday, June 10, 2021 8:13 PM
To: Manickavasakam Karpagavinayagam <manickavasakamk@ami.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Felix Polyudov <Felixp@ami.com>; Harikrishna Doppalapudi <Harikrishnad@ami.com>; Manish Jha <manishj@ami.com>; Zachary Bobroff <zacharyb@ami.com>
Subject: [EXTERNAL] RE: [edk2][PATCH] MdePkg : Add IPMI Macro and Structure Defintions to resolve the IPMI build error


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

Please comment the following commenting per normal style
/*----------------------------------------------------------------------------------------
Definitions for Get BMC Execution Context ----------------------------------------------------------------------------------------*/

Which would correctly be:
//
// Definitions for Get BMC Execution Context //

Please don't use EFI_ or Efi prefix for items that are not part of UEFI owned specifications. Please correct:
EFI_IPMI_MSG_GET_BMC_EXEC_RSP
EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
EFI_FIRMWARE_BMC_IN_FULL_RUNTIME
EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE
and any uses.

Thanks,
Isaac

-----Original Message-----
From: manickavasakam karpagavinayagam <manickavasakamk@ami.com>
Sent: Thursday, June 10, 2021 4:41 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Felixp@ami.com; DOPPALAPUDI, HARIKRISHNA <harikrishnad@ami.com>; Jha, Manish <manishj@ami.com>; Bobroff, Zachary <zacharyb@ami.com>
Subject: [edk2][PATCH] MdePkg : Add IPMI Macro and Structure Defintions to resolve the IPMI build error

Build error reported for missing structures IPMI_SET_BOOT_OPTIONS_RESPONSE, EFI_IPMI_MSG_GET_BMC_EXEC_RSP and macros EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
EFI_FIRMWARE_BMC_IN_FULL_RUNTIME/EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE
when using edk2-platforms\Features\Intel\OutOfBandManagement\IpmiFeaturePkg
---
MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h | 4 ++++ MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h | 19 +++++++++++++++++++
2 files changed, 23 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h b/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
index 79db55523d..d7cdd3a865 100644
--- a/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
+++ b/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
@@ -186,6 +186,10 @@ typedef struct {
UINT8 ParameterData[0];

} IPMI_SET_BOOT_OPTIONS_REQUEST;



+typedef struct {

+ UINT8 CompletionCode:8;

+} IPMI_SET_BOOT_OPTIONS_RESPONSE;

+

//

// Definitions for Get System Boot options command

//

diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h b/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
index 2d892dbd5a..1c692cc792 100644
--- a/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
+++ b/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
@@ -17,4 +17,23 @@
// All Firmware commands and their structure definitions to follow here

//



+/*---------------------------------------------------------------------
+-------------------

+ Definitions for Get BMC Execution Context

+-----------------------------------------------------------------------
+-----------------*/

+#define EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT 0x23

+

+//

+// Constants and Structure definitions for "Get Device ID" command to
+follow here

+//

+typedef struct {

+ UINT8 CurrentExecutionContext;

+ UINT8 PartitionPointer;

+} EFI_IPMI_MSG_GET_BMC_EXEC_RSP;

+

+//

+// Current Execution Context responses

+//

+#define EFI_FIRMWARE_BMC_IN_FULL_RUNTIME 0x10

+#define EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE 0x11

+

#endif

--
2.25.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


[PATCH v2 2/2] Fixed Library references

Kenneth Lautner
 

From: Ken Lautner <kenlautner3@gmail.com>

Changed BdsEntry.c to use Variable Policy instead of Variable Lock
as Variable Lock will be Deprecated eventually

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Kenneth Lautner <kenlautner3@gmail.com>
---
MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 -
MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf b/MdeModulePkg/Univer=
sal/BdsDxe/BdsDxe.inf
index 76ff6a0f5fc3..5bac635def93 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
@@ -50,7 +50,6 @@
BaseMemoryLib=0D
DebugLib=0D
UefiBootManagerLib=0D
- VariablePolicyLib=0D
VariablePolicyHelperLib=0D
PlatformBootManagerLib=0D
PcdLib=0D
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Univer=
sal/BdsDxe/BdsEntry.c
index 13723bee299b..13c10bdc5bf8 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "Bds.h"=0D
#include "Language.h"=0D
#include "HwErrRecSupport.h"=0D
-#include "library/VariablePolicyHelperLib.h"=0D
+#include <Library/VariablePolicyHelperLib.h>=0D
=0D
#define SET_BOOT_OPTION_SUPPORT_KEY_COUNT(a, c) { \=0D
(a) =3D ((a) & ~EFI_BOOT_OPTION_SUPPORT_COUNT) | (((c) << LowBitSet3=
2 (EFI_BOOT_OPTION_SUPPORT_COUNT)) & EFI_BOOT_OPTION_SUPPORT_COUNT); \=0D
--=20
2.31.1.windows.1


[PATCH v1 1/2] MdeModulePkg/BdsDxe: Update BdsEntry to use Variable Policy

Kenneth Lautner
 

From: Ken Lautner <klautner@microsoft.com>

Changed BdsEntry.c to use Variable Policy instead of Variable Lock
as Variable Lock will be Deprecated eventually

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Kenneth Lautner <kenlautner3@gmail.com>
---
MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 4 +++-
MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 20 +++++++++++++++-----
2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf b/MdeModulePkg/Univer=
sal/BdsDxe/BdsDxe.inf
index 9310b4dccb18..76ff6a0f5fc3 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
@@ -50,6 +50,8 @@
BaseMemoryLib=0D
DebugLib=0D
UefiBootManagerLib=0D
+ VariablePolicyLib=0D
+ VariablePolicyHelperLib=0D
PlatformBootManagerLib=0D
PcdLib=0D
PrintLib=0D
@@ -77,7 +79,7 @@
[Protocols]=0D
gEfiBdsArchProtocolGuid ## PRODUCES=0D
gEfiSimpleTextInputExProtocolGuid ## CONSUMES=0D
- gEdkiiVariableLockProtocolGuid ## SOMETIMES_CONSUMES=0D
+ gEdkiiVariablePolicyProtocolGuid ## SOMETIMES_CONSUMES=0D
gEfiDeferredImageLoadProtocolGuid ## CONSUMES=0D
=0D
[FeaturePcd]=0D
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Univer=
sal/BdsDxe/BdsEntry.c
index 83b773a2fa5f..13723bee299b 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "Bds.h"=0D
#include "Language.h"=0D
#include "HwErrRecSupport.h"=0D
+#include "library/VariablePolicyHelperLib.h"=0D
=0D
#define SET_BOOT_OPTION_SUPPORT_KEY_COUNT(a, c) { \=0D
(a) =3D ((a) & ~EFI_BOOT_OPTION_SUPPORT_COUNT) | (((c) << LowBitSet3=
2 (EFI_BOOT_OPTION_SUPPORT_COUNT)) & EFI_BOOT_OPTION_SUPPORT_COUNT); \=0D
@@ -670,7 +671,7 @@ BdsEntry (
EFI_STATUS Status;=0D
UINT32 BootOptionSupport;=0D
UINT16 BootTimeOut;=0D
- EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;=0D
+ EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;=0D
UINTN Index;=0D
EFI_BOOT_MANAGER_LOAD_OPTION LoadOption;=0D
UINT16 *BootNext;=0D
@@ -716,12 +717,21 @@ BdsEntry (
//=0D
// Mark the read-only variables if the Variable Lock protocol exists=0D
//=0D
- Status =3D gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (=
VOID **) &VariableLock);=0D
- DEBUG ((EFI_D_INFO, "[BdsDxe] Locate Variable Lock protocol - %r\n", Sta=
tus));=0D
+ Status =3D gBS->LocateProtocol(&gEdkiiVariablePolicyProtocolGuid, NULL, =
(VOID**)&VariablePolicy);=0D
+ DEBUG((DEBUG_INFO, "[BdsDxe] Locate Variable Policy protocol - %r\n", St=
atus));=0D
if (!EFI_ERROR (Status)) {=0D
for (Index =3D 0; Index < ARRAY_SIZE (mReadOnlyVariables); Index++) {=
=0D
- Status =3D VariableLock->RequestToLock (VariableLock, mReadOnlyVaria=
bles[Index], &gEfiGlobalVariableGuid);=0D
- ASSERT_EFI_ERROR (Status);=0D
+ Status =3D RegisterBasicVariablePolicy(=0D
+ VariablePolicy,=0D
+ &gEfiGlobalVariableGuid,=0D
+ mReadOnlyVariables[Index],=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW=0D
+ );=0D
+ ASSERT_EFI_ERROR(Status);=0D
}=0D
}=0D
=0D
--=20
2.31.1.windows.1


Re: 回复: [PATCH v1 1/1] MdeModulePkg/BdsDxe: Update BdsEntry to use Variable Policy

Kenneth Lautner
 

Liming,
I plan on fixing those modules as well.  I created a new Bugzilla here: 3449 – VariablePolicy needs to replace VariableLock. (tianocore.org).

On Wed, Jun 9, 2021 at 5:53 PM gaoliming <gaoliming@...> wrote:
Ken:
  I add my comment below. Besides, some modules in MdeModulePkg still
consumes VariableLock, such as UefiBootManagerLib, PcdDxe. Have you plan to
fix them also?

> -----邮件原件-----
> 发件人: kenlautner3@... <kenlautner3@...>
> 发送时间: 2021年6月10日 7:39
> 收件人: devel@edk2.groups.io
> 抄送: Jian J Wang <jian.j.wang@...>; Hao A Wu
> <hao.a.wu@...>; Zhichao Gao <zhichao.gao@...>; Ray Ni
> <ray.ni@...>; Liming Gao <gaoliming@...>
> 主题: [PATCH v1 1/1] MdeModulePkg/BdsDxe: Update BdsEntry to use
> Variable Policy
>
> From: Ken Lautner <klautner@...>

Please add BZ link here. https://bugzilla.tianocore.org/show_bug.cgi?id=3421
>
> Changed BdsEntry.c to use Variable Policy instead of Variable Lock
> as Variable Lock will be Deprecated eventually
>
> Cc: Jian J Wang <jian.j.wang@...>
> Cc: Hao A Wu <hao.a.wu@...>
> Cc: Zhichao Gao <zhichao.gao@...>
> Cc: Ray Ni <ray.ni@...>
> Cc: Liming Gao <gaoliming@...>
> Signed-off-by: Kenneth Lautner <kenlautner3@...>
> ---
>  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  4 +++-
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 20 +++++++++++++++-----
>  2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> index 9310b4dccb18..76ff6a0f5fc3 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> @@ -50,6 +50,8 @@
>    BaseMemoryLib
>
>    DebugLib
>
>    UefiBootManagerLib
>
> +  VariablePolicyLib
>
> +  VariablePolicyHelperLib
>
>    PlatformBootManagerLib
>
>    PcdLib
>
>    PrintLib
>
> @@ -77,7 +79,7 @@
>  [Protocols]
>
>    gEfiBdsArchProtocolGuid                       ## PRODUCES
>
>    gEfiSimpleTextInputExProtocolGuid             ## CONSUMES
>
> -  gEdkiiVariableLockProtocolGuid                ##
> SOMETIMES_CONSUMES
>
> +  gEdkiiVariablePolicyProtocolGuid              ##
> SOMETIMES_CONSUMES
>
>    gEfiDeferredImageLoadProtocolGuid             ## CONSUMES
>
>
>
>  [FeaturePcd]
>
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 83b773a2fa5f..13723bee299b 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "Bds.h"
>
>  #include "Language.h"
>
>  #include "HwErrRecSupport.h"
>
> +#include "library/VariablePolicyHelperLib.h"
>
Use the correct directory name to pass Windows VS tool chain and Linux GCC
tool chain.

With this change, Reviewed-by: Liming Gao <gaoliming@...>

Thanks
Liming
>
>
>  #define SET_BOOT_OPTION_SUPPORT_KEY_COUNT(a, c) {  \
>
>        (a) = ((a) & ~EFI_BOOT_OPTION_SUPPORT_COUNT) | (((c) <<
> LowBitSet32 (EFI_BOOT_OPTION_SUPPORT_COUNT)) &
> EFI_BOOT_OPTION_SUPPORT_COUNT); \
>
> @@ -670,7 +671,7 @@ BdsEntry (
>    EFI_STATUS                      Status;
>
>    UINT32                          BootOptionSupport;
>
>    UINT16                          BootTimeOut;
>
> -  EDKII_VARIABLE_LOCK_PROTOCOL    *VariableLock;
>
> +  EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy;
>
>    UINTN                           Index;
>
>    EFI_BOOT_MANAGER_LOAD_OPTION    LoadOption;
>
>    UINT16                          *BootNext;
>
> @@ -716,12 +717,21 @@ BdsEntry (
>    //
>
>    // Mark the read-only variables if the Variable Lock protocol exists
>
>    //
>
> -  Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **) &VariableLock);
>
> -  DEBUG ((EFI_D_INFO, "[BdsDxe] Locate Variable Lock protocol - %r\n",
> Status));
>
> +  Status = gBS->LocateProtocol(&gEdkiiVariablePolicyProtocolGuid, NULL,
> (VOID**)&VariablePolicy);
>
> +  DEBUG((DEBUG_INFO, "[BdsDxe] Locate Variable Policy protocol - %r\n",
> Status));
>
>    if (!EFI_ERROR (Status)) {
>
>      for (Index = 0; Index < ARRAY_SIZE (mReadOnlyVariables); Index++) {
>
> -      Status = VariableLock->RequestToLock (VariableLock,
> mReadOnlyVariables[Index], &gEfiGlobalVariableGuid);
>
> -      ASSERT_EFI_ERROR (Status);
>
> +      Status = RegisterBasicVariablePolicy(
>
> +                 VariablePolicy,
>
> +                 &gEfiGlobalVariableGuid,
>
> +                 mReadOnlyVariables[Index],
>
> +                 VARIABLE_POLICY_NO_MIN_SIZE,
>
> +                 VARIABLE_POLICY_NO_MAX_SIZE,
>
> +                 VARIABLE_POLICY_NO_MUST_ATTR,
>
> +                 VARIABLE_POLICY_NO_CANT_ATTR,
>
> +                 VARIABLE_POLICY_TYPE_LOCK_NOW
>
> +                 );
>
> +      ASSERT_EFI_ERROR(Status);
>
>      }
>
>    }
>
>
>
> --
> 2.25.1.windows.1









回复: [edk2-devel] [edk2][PATCH] MdePkg : Add IPMI Macro and Structure Defintions to resolve the IPMI build error

gaoliming
 

KARP:
I don't find the source code in
edk2-platforms\Features\Intel\OutOfBandManagement\IpmiFeaturePkg to consume
new struct IPMI_SET_BOOT_OPTIONS_RESPONSE. Can you point which source file
depend on new definition?

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Oram, Isaac
W
发送时间: 2021年6月11日 8:13
收件人: KARPAGAVINAYAGAM, MANICKAVASAKAM
<manickavasakamk@ami.com>; devel@edk2.groups.io
抄送: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
Felixp@ami.com; DOPPALAPUDI, HARIKRISHNA <harikrishnad@ami.com>;
Jha, Manish <manishj@ami.com>; Bobroff, Zachary <zacharyb@ami.com>
主题: Re: [edk2-devel] [edk2][PATCH] MdePkg : Add IPMI Macro and
Structure Defintions to resolve the IPMI build error

Please comment the following commenting per normal style
/*--------------------------------------------------------------------------
--------------
Definitions for Get BMC Execution Context
----------------------------------------------------------------------------
------------*/

Which would correctly be:
//
// Definitions for Get BMC Execution Context
//

Please don't use EFI_ or Efi prefix for items that are not part of UEFI
owned
specifications. Please correct:
EFI_IPMI_MSG_GET_BMC_EXEC_RSP
EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
EFI_FIRMWARE_BMC_IN_FULL_RUNTIME
EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE
and any uses.

Thanks,
Isaac

-----Original Message-----
From: manickavasakam karpagavinayagam <manickavasakamk@ami.com>
Sent: Thursday, June 10, 2021 4:41 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Felixp@ami.com; DOPPALAPUDI,
HARIKRISHNA <harikrishnad@ami.com>; Jha, Manish <manishj@ami.com>;
Bobroff, Zachary <zacharyb@ami.com>
Subject: [edk2][PATCH] MdePkg : Add IPMI Macro and Structure Defintions to
resolve the IPMI build error

Build error reported for missing structures
IPMI_SET_BOOT_OPTIONS_RESPONSE,
EFI_IPMI_MSG_GET_BMC_EXEC_RSP and macros
EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT
EFI_FIRMWARE_BMC_IN_FULL_RUNTIME/EFI_FIRMWARE_BMC_IN_FORCE
D_UPDATE_MODE
when using
edk2-platforms\Features\Intel\OutOfBandManagement\IpmiFeaturePkg
---
MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h | 4 ++++
MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h | 19
+++++++++++++++++++
2 files changed, 23 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
b/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
index 79db55523d..d7cdd3a865 100644
--- a/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
+++ b/MdePkg/Include/IndustryStandard/IpmiNetFnChassis.h
@@ -186,6 +186,10 @@ typedef struct {
UINT8 ParameterData[0];

} IPMI_SET_BOOT_OPTIONS_REQUEST;



+typedef struct {

+ UINT8 CompletionCode:8;

+} IPMI_SET_BOOT_OPTIONS_RESPONSE;

+

//

// Definitions for Get System Boot options command

//

diff --git a/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
b/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
index 2d892dbd5a..1c692cc792 100644
--- a/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
+++ b/MdePkg/Include/IndustryStandard/IpmiNetFnFirmware.h
@@ -17,4 +17,23 @@
// All Firmware commands and their structure definitions to follow here

//



+/*-------------------------------------------------------------------------
---------------

+ Definitions for Get BMC Execution Context

+---------------------------------------------------------------------------
-------------*/

+#define EFI_FIRMWARE_GET_BMC_EXECUTION_CONTEXT 0x23

+

+//

+// Constants and Structure definitions for "Get Device ID" command to
follow here

+//

+typedef struct {

+ UINT8 CurrentExecutionContext;

+ UINT8 PartitionPointer;

+} EFI_IPMI_MSG_GET_BMC_EXEC_RSP;

+

+//

+// Current Execution Context responses

+//

+#define EFI_FIRMWARE_BMC_IN_FULL_RUNTIME 0x10

+#define EFI_FIRMWARE_BMC_IN_FORCED_UPDATE_MODE 0x11

+

#endif

--
2.25.0.windows.1


Please consider the environment before printing this email.

The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is intended
to be read only by the individual or entity to whom it is addressed or by
their
designee. If the reader of this message is not the intended recipient, you
are
on notice that any distribution of this message, in any form, is strictly
prohibited. Please promptly notify the sender by reply e-mail or by
telephone at 770-246-8600, and then delete or destroy all copies of the
transmission.




回复: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file mask across package matrices

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Kun Qin
发送时间: 2021年6月10日 9:48
收件人: devel@edk2.groups.io
抄送: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew
<Bret.Barkelew@microsoft.com>; Michael D Kinney
<michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
主题: [edk2-devel] [PATCH v1 1/1] Pytool: SpellCheck: Fix incorrect file
mask
across package matrices

From: Sean Brogan <spbrogan@live.com>

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

Existing implementation could modify class global data that causes
potential incorrect file mask to be used for execution of plugin.

This change switches class variable to be tuple so that it cannot be
accidently modified. Local usage of STANDARD_PLUGIN_DEFINED_PATHS is
also
changed to copy to new list before modification.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Sean Brogan <sean.brogan@microsoft.com>
---
.pytool/Plugin/SpellCheck/SpellCheck.py | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/.pytool/Plugin/SpellCheck/SpellCheck.py
b/.pytool/Plugin/SpellCheck/SpellCheck.py
index 43365441b91c..9ad57632a6e8 100644
--- a/.pytool/Plugin/SpellCheck/SpellCheck.py
+++ b/.pytool/Plugin/SpellCheck/SpellCheck.py
@@ -37,12 +37,12 @@ class SpellCheck(ICiBuildPlugin):
#
# A package can remove any of these using IgnoreStandardPaths
#
- STANDARD_PLUGIN_DEFINED_PATHS = ["*.c", "*.h",
+ STANDARD_PLUGIN_DEFINED_PATHS = ("*.c", "*.h",
"*.nasm", "*.asm", "*.masm",
"*.s",
"*.asl",
"*.dsc", "*.dec", "*.fdf",
"*.inf",
"*.md", "*.txt"
- ]
+ )

def GetTestName(self, packagename: str, environment: VarDict) ->
tuple:
""" Provide the testcase name and classname for use in reporting
@@ -107,7 +107,8 @@ class SpellCheck(ICiBuildPlugin):
version_aggregator.GetVersionAggregator().ReportVersion(
"CSpell", cspell_version,
version_aggregator.VersionTypes.INFO)

- package_relative_paths_to_spell_check =
SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS
+ # copy the default as a list
+ package_relative_paths_to_spell_check =
list(SpellCheck.STANDARD_PLUGIN_DEFINED_PATHS)

#
# Allow the ci.yaml to remove any of the above standard paths
--
2.31.1.windows.1





回复: [PATCH v2 2/2] Fixed Library references

gaoliming
 

Ken:
Please combine them into one commit. You just need to update the first
patch.

Thanks
Liming

-----邮件原件-----
发件人: kenlautner3@gmail.com <kenlautner3@gmail.com>
发送时间: 2021年6月11日 7:48
收件人: devel@edk2.groups.io
抄送: Jian J Wang <jian.j.wang@intel.com>; Hao A Wu
<hao.a.wu@intel.com>; Zhichao Gao <zhichao.gao@intel.com>; Ray Ni
<ray.ni@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
主题: [PATCH v2 2/2] Fixed Library references

From: Ken Lautner <kenlautner3@gmail.com>

Changed BdsEntry.c to use Variable Policy instead of Variable Lock
as Variable Lock will be Deprecated eventually

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Kenneth Lautner <kenlautner3@gmail.com>
---
MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 -
MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
index 76ff6a0f5fc3..5bac635def93 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
@@ -50,7 +50,6 @@
BaseMemoryLib

DebugLib

UefiBootManagerLib

- VariablePolicyLib

VariablePolicyHelperLib

PlatformBootManagerLib

PcdLib

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 13723bee299b..13c10bdc5bf8 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "Bds.h"

#include "Language.h"

#include "HwErrRecSupport.h"

-#include "library/VariablePolicyHelperLib.h"

+#include <Library/VariablePolicyHelperLib.h>



#define SET_BOOT_OPTION_SUPPORT_KEY_COUNT(a, c) { \

(a) = ((a) & ~EFI_BOOT_OPTION_SUPPORT_COUNT) | (((c) <<
LowBitSet32 (EFI_BOOT_OPTION_SUPPORT_COUNT)) &
EFI_BOOT_OPTION_SUPPORT_COUNT); \

--
2.31.1.windows.1


Event: TianoCore Community Meeting - APAC/NAMO - 06/10/2021 #cal-reminder

devel@edk2.groups.io Calendar <noreply@...>
 

Reminder: TianoCore Community Meeting - APAC/NAMO

When:
06/10/2021
7:30pm to 8:30pm
(UTC-07:00) America/Los Angeles

Where:
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZWNiZWM1MzgtNWEzMy00MTgwLTgwNjAtNWQ1ZWUwZmQzNjVh%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d

Organizer: Soumya Guptha

View Event

Description:

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 119 132 712 6

Alternate VTC dialing instructions

Or call in (audio only)

+1 916-245-6934,,494156131#   United States, Sacramento

Phone Conference ID: 494 156 131#

Find a local number | Reset PIN

Learn More | Meeting options

2121 - 2140 of 78424