Re: [edk2-devel] [patch V2 00/29] Add a new library class RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access

[Dandan Bi]

Thank you Liming.
If no more comments, I plan to submit following 2 patches firstly tomorrow.

[patch V2 01/29] MdePkg: Add RegisterFilterLib class and NULL instance 
(https://edk2.groups.io/g/devel/message/73072)
[patch V2 02/29] MdePkg: Add MdeLibs.dsc.inc file to MdePkg 
(https://edk2.groups.io/g/devel/message/73073)

Then we can submit following dsc patches and IoLib and BaseLib update patches.
Please let me know if you have any concern.

Thanks,
Dandan
> -Original Message-
> From: gaoliming 
> Sent: Tuesday, March 23, 2021 4:40 PM
> To: Bi, Dandan ; devel@edk2.groups.io
> Cc: 'Andrew Fish' ; 'Leif Lindholm' ;
> 'Laszlo Ersek' ; Kinney, Michael D
> ; Liu, Zhiguang 
> Subject: 回复: [edk2-devel] [patch V2 00/29] Add a new library class
> RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access
> 
> Dandan:
>   This change is good to me. Reviewed-by: Liming Gao 
> for the patch set.
> 
> Thanks
> Liming
> > -邮件原件-
> > 发件人: Bi, Dandan 
> > 发送时间: 2021年3月23日 10:06
> > 收件人: devel@edk2.groups.io; Bi, Dandan 
> > 抄送: Andrew Fish ; Leif Lindholm ;
> > Laszlo Ersek ; Kinney, Michael D
> > ; Liming Gao ;
> > Liu, Zhiguang 
> > 主题: RE: [edk2-devel] [patch V2 00/29] Add a new library class
> > RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access
> >
> > Hi All,
> >
> > Here is the branch for these changes in Edk2.
> > https://github.com/dandanbi/edk2/tree/RegisterFilterLibV2
> > https://github.com/tianocore/edk2/pull/1509
> >
> > Hi Mike, Liming and Zhiguang,
> >
> > Could you help review the first and last 2 patches in MdePkg?
> > Since we should submit the first 2 patches firstly before the changes
> > of
> dsc
> > files .
> >
> >
> > Thanks,
> > Dandan
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > Dandan Bi
> > > Sent: Monday, March 22, 2021 4:09 PM
> > > To: devel@edk2.groups.io
> > > Cc: Andrew Fish ; Leif Lindholm
> > > ; Laszlo Ersek ; Kinney,
> > > Michael D ; Liming Gao
> > > ; Liu, Zhiguang 
> > > Subject: [edk2-devel] [patch V2 00/29] Add a new library class
> > > RegisterFilterLib in edk2 to filter/trace port IO/MMIO/MSR access
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246
> > > RFC: https://edk2.groups.io/g/devel/message/72530
> > >
> > > Patch 1 is to add RegisterFilterLib Library Class in edk2 to
> filter/trace port
> > > IO/MMIO/MSR access and add a RegisterFilterLibNull instance.
> > > Patch 2 is to add the MdeLibs.dsc.inc file to MdePkg for some
> > > default
> > libraries
> > > provided by MdePkg and add RegisterFilterLib into it as the first
> version of
> > > MdeLibs.dsc.inc.
> > > Last 2 patches are to update APIs in IoLib and BaseLib to
> > > filter/trace
> port
> > > IO/MMIO/MSR access.
> > > Remaining patches are to update related dsc files to consume
> > > MdeLibs.dsc.inc for RegisterFilterLib.
> > >
> > > Will submit patch 1 and 2 firstly.
> > > And then update related dsc files in edk2 and edk2platform repo to
> consume
> > > MdeLibs.dsc.inc for RegisterFilterLib.
> > > At last will submit the patches to update IoLib and BaseLib to
> filter/trace
> > port
> > > IO/MMIO/MSR access.
> > >
> > > --
> > > V2:
> > > Introduce MdeLibs.dsc.inc and add RegisterFilterLib into it as the
> > > first
> > version
> > > of MdeLibs.dsc.inc.
> > > Update Platform dsc to consume the MdeLibs.dsc.inc.
> > > Add the description for the return flag in FilterBefore
> > > functions in header file source code.
> > > Extend the years for Intel copyright.
> > > Add mssing change the dsc files in OvmfPkg.
> > >
> > > Cc: Andrew Fish 
> > > Cc: Leif Lindholm 
> > > Cc: Laszlo Ersek 
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Cc: Zhiguang Liu 
> > >
> > > Dandan Bi (29):
> > >   MdePkg: Add RegisterFilterLib class and NULL instance
> > >   MdePkg: Add MdeLibs.dsc.inc file to MdePkg
> > >   ArmPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   ArmPlatformPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   ArmVirtPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   CryptoPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   DynamicTablesPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   EmbeddedPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   EmulatorPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   FatPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   FmpDevicePkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   IntelFsp2Pkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   IntelFsp2WrapperPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   MdeModulePkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   MdePkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   NetworkPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   OvmfPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   PcAtChipsetPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >   RedfishPkg: Consume MdeLibs.dsc.inc for RegisterFilterLib
> > >

Re: [edk2-devel] [PATCH V3 2/2] ShellPkg/UefiHandleParsingLib: Support EFI Redfish protocols

[Abner Chang]

> -Original Message-
> From: gaoliming [mailto:gaolim...@byosoft.com.cn]
> Sent: Thursday, March 25, 2021 10:04 AM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> 
> Cc: 'Ray Ni' ; 'Zhichao Gao' ;
> Wang, Nickle (HPS SW) ; O'Hanley, Peter (EXL)
> 
> Subject: 回复: [edk2-devel] [PATCH V3 2/2] ShellPkg/UefiHandleParsingLib:
> Support EFI Redfish protocols
> 
> Abner:
>   I have one comment.
> 
> > -邮件原件-
> > 发件人: devel@edk2.groups.io  代表 Abner Chang
> > 发送时间: 2021年3月24日 13:35
> > 收件人: devel@edk2.groups.io
> > 抄送: Ray Ni ; Zhichao Gao ;
> > Nickle Wang ; Peter O'Hanley
> > 
> > 主题: [edk2-devel] [PATCH V3 2/2] ShellPkg/UefiHandleParsingLib: Support
> > EFI Redfish protocols
> >
> > Add handle parsing for EFI Redfish Discover protocol.
> > Add handle parsing for EFI RestEx protocol.
> >
> > Signed-off-by: Abner Chang 
> > Cc: Ray Ni 
> > Cc: Zhichao Gao 
> > Cc: Nickle Wang 
> > Cc: Peter O'Hanley 
> > Reviewed-by: Zhichao Gao 
> > ---
> >  .../Library/UefiHandleParsingLib/UefiHandleParsingLib.inf | 4 +++-
> >  .../Library/UefiHandleParsingLib/UefiHandleParsingLib.c   | 8 ++--
> >  .../Library/UefiHandleParsingLib/UefiHandleParsingLib.uni | 4 +++-
> >  3 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git
> a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> > b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> > index 93b69cd8e9..446cd8d609 100644
> > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> > @@ -2,7 +2,7 @@
> >  #  Provides interface to advanced shell functionality for parsing both
> handle
> > and protocol database.
> >  #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> 
> >  #  (C) Copyright 2013-2015 Hewlett-Packard Development Company,
> > L.P.
> > -#  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > +#  (C) Copyright 2015-2020 Hewlett Packard Enterprise Development
> > LP
> >  #
> >  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> > @@ -269,6 +269,8 @@
> >gEfiHttpProtocolGuid##
> > UNDEFINED
> >gEfiHttpUtilitiesProtocolGuid   ##
> > UNDEFINED
> >gEfiRestProtocolGuid##
> > UNDEFINED
> > +  gEfiRestExProtocolGuid  ##
> > UNDEFINED
> > +  gEfiRedfishDiscoverProtocolGuid ##
> > UNDEFINED
> >gEfiMmEndOfDxeProtocolGuid  ##
> > UNDEFINED
> >gEfiMmIoTrapDispatchProtocolGuid##
> > UNDEFINED
> >gEfiMmPowerButtonDispatchProtocolGuid   ##
> > UNDEFINED
> > diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > index 500a95a89a..c00337d6b2 100644
> > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > @@ -3,7 +3,7 @@
> >
> >Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> >(C) Copyright 2013-2015 Hewlett-Packard Development Company,
> > L.P.
> > -  (C) Copyright 2015-2016 Hewlett Packard Enterprise Development
> LP
> > +  (C) Copyright 2015-2020 Hewlett Packard Enterprise Development
> > LP
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -2355,7 +2355,11 @@ STATIC CONST GUID_INFO_BLOCK
> > mGuidStringList[] = {
> >{STRING_TOKEN(STR_NET_HTTP),  &gEfiHttpProtocolGuid,
> > NULL},
> >{STRING_TOKEN(STR_NET_HTTP_U),
> > &gEfiHttpUtilitiesProtocolGuid,   NULL},
> >{STRING_TOKEN(STR_REST),  &gEfiRestProtocolGuid,
> > NULL},
> > -
> > +//
> > +// UEFI 2.8
> > +//
> > +  {STRING_TOKEN(STR_REST_EX),
> > &gEfiRestExProtocolGuid,  NULL},
> > +  {STRING_TOKEN(STR_REDFISH_DISCOVER),
> > &gEfiRedfishDiscoverProtocolGuid, NULL},
> 
> This change should be moved to the position after UEFI 2.7 in this table.
> 
> Thanks
> Liming
Yes, you are right. No idea why I added new entries in PI section.
Thanks and v4 patch sent.
Abner

> >  //
> >  // PI 1.5
> >  //
> > diff --git
> a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> > b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> > index 9c8028d0d5..69fcbdfe0e 100644
> > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> > @@ -2,7 +2,7 @@
> >  //
> >  // Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> 
> >  // (C) Copyright 2013-2015 Hewlett-Packard Development Company,
> > L.P.
> > -// (C) Copyright 2015-2016 Hewlett Packard Enterprise Development
> LP
> > +// (C) Copyright 2015-2020 Hewlett Packard Enterprise Development
> > LP
> >  // SPDX-License-Ide

[edk2-devel] [PATCH v4 1/2] MdePkg/Include: EFI Redfish Discover protocol

[Abner Chang]

Move GUID definition of EFI Redfish Discover protocol
to under MdePkg. With this we don't have dependency of
RedfishPkg in ShellPkg.

Signed-off-by: Abner Chang 

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Nickle Wang 
Cc: Peter O'Hanley 
Reviewed-by: Liming Gao 
---
 MdePkg/MdePkg.dec |  5 +-
 RedfishPkg/RedfishPkg.dec |  3 -
 .../Include/Protocol/RedfishDiscover.h| 72 +--
 3 files changed, 37 insertions(+), 43 deletions(-)
 rename {RedfishPkg => MdePkg}/Include/Protocol/RedfishDiscover.h (79%)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 1d2637acc2..e667d44db5 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -6,7 +6,7 @@
 #
 # Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
 # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
-# (C) Copyright 2016 - 2020 Hewlett Packard Enterprise Development LP
+# (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development LP
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -1863,6 +1863,9 @@
   ## Include/Protocol/RestJsonStructure.h
   gEfiRestJsonStructureProtocolGuid  = { 0xa9a048f6, 0x48a0, 0x4714, {0xb7, 
0xda, 0xa9, 0xad,0x87, 0xd4, 0xda, 0xc9 }}
 
+  ## Include/Protocol/RedfishDiscover.h
+  gEfiRedfishDiscoverProtocolGuid  = { 0x5db12509, 0x4550, 0x4347, { 0x96, 
0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
+
   #
   # Protocols defined in Shell2.0
   #
diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
index b3e25268a0..846c19fd5e 100644
--- a/RedfishPkg/RedfishPkg.dec
+++ b/RedfishPkg/RedfishPkg.dec
@@ -67,9 +67,6 @@
   RedfishLib|PrivateInclude/Library/RedfishLib.h
 
 [Protocols]
-  ## Include/Protocol/RedfishDiscover.h
-  gEfiRedfishDiscoverProtocolGuid  = { 0x5db12509, 0x4550, 0x4347, { 0x96, 
0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
-
   ## Include/Protocol/EdkIIRedfishCredential.h
   gEdkIIRedfishCredentialProtocolGuid = { 0x8804377, 0xaf7a, 0x4496, { 0x8a, 
0x7b, 0x17, 0x59, 0x0, 0xe9, 0xab, 0x46 } }
 
diff --git a/RedfishPkg/Include/Protocol/RedfishDiscover.h 
b/MdePkg/Include/Protocol/RedfishDiscover.h
similarity index 79%
rename from RedfishPkg/Include/Protocol/RedfishDiscover.h
rename to MdePkg/Include/Protocol/RedfishDiscover.h
index 4c91605c4e..8dbb70b082 100644
--- a/RedfishPkg/Include/Protocol/RedfishDiscover.h
+++ b/MdePkg/Include/Protocol/RedfishDiscover.h
@@ -1,20 +1,19 @@
 /** @file
   This file defines the EFI Redfish Discover Protocol interface.
 
-  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+  (C) Copyright 2021 Hewlett Packard Enterprise Development LP
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
+  @par Revision Reference:
+  - Some corrections and revises are added to UEFI Specification 2.9.
+  - This Protocol is introduced in UEFI Specification 2.8.
+
 **/
 
 #ifndef EFI_REDFISH_DISCOVER_PROTOCOL_H_
 #define EFI_REDFISH_DISCOVER_PROTOCOL_H_
 
-#include 
-#include 
-#include 
-#include 
-
 //
 // GUID definitions
 //
@@ -40,24 +39,21 @@ typedef UINT32 EFI_REDFISH_DISCOVER_FLAG;
///< 3 to 15. The 
corresponding duration is 8 to 2^15 seconds.
///< Duration is only 
valid when EFI_REDFISH_DISCOVER_KEEP_ALIVE
///< is set to 1.
-#define EFI_REDFISH_DISCOVER_DURATION_BIT_POS 8
-
 typedef struct _EFI_REDFISH_DISCOVER_PROTOCOL EFI_REDFISH_DISCOVER_PROTOCOL;
-typedef struct _EFI_REDFISH_DISCOVERED_INFORMATION 
EFI_REDFISH_DISCOVERED_INFORMATION;
-
-typedef struct _EFI_REDFISH_DISCOVERED_INFORMATION {
-  EFI_HANDLE RedfishRestExHandle;   ///< REST EX EFI handle associated 
with this Redfish service.
-  BOOLEAN IsUdp6;   ///< Indicates it's IP versino 6.
-  EFI_IP_ADDRESS  RedfishHostIpAddress; ///< IP address of Redfish service.
-  UINTN RedfishVersion; ///< Redfish service version.
-  CHAR16 *Location; ///< Redfish service location.
-  CHAR16 *Uuid; ///< Redfish service UUID.
-  CHAR16 *Os;   ///< Redfish service OS.
-  CHAR16 *OSVersion;///< Redfish service OS version.
-  CHAR16 *Product;  ///< Redfish service product name.
-  CHAR16 *ProductVer;   ///< Redfish service product 
version.
-  BOOLEAN UseHttps; ///< Using HTTPS.
-};
+
+typedef struct {
+  EFI_HANDLERedfishRestExHandle;///< REST EX EFI handle associated 
with this Redfish service.
+  BOOLEAN   IsUdp6; ///< Indicates it's IP versino 6.
+  EFI_IP_ADDRESSRedfishHostIpAddress;   ///< IP address of Redfish service.
+  UINTN RedfishVersion; ///< Redfish service version.
+  CHAR16*Location;  ///< Redfish servi

[edk2-devel] [PATCH v4 0/2] Support EFI Redfish protocols

[Abner Chang]

In v4: In UefiHandleParsingLib.c, move the changes for UEFI 2.8 to
   below UEFI 2.7 section
In v3: Correct the mismatched definitions in both
   RedfishDiscover.h and UEFI spec 2.8 (Refer to v2.9).
In v2: Address comments given by Liming on patch 1/2.

Add handle parsing for EFI Redfish Discover protocol and
EFI RestEx protocol.

This change also moves the GUID definition of EFI Redfish Discover
protocol to under MdePkg. With this we don't have dependency of
RedfishPkg in ShellPkg.

Signed-off-by: Abner Chang 

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Cc: Ray Ni 
Cc: Zhichao Gao 
Cc: Nickle Wang 
Cc: Peter O'Hanley 

*** BLURB HERE ***

Abner Chang (2):
  MdePkg/Include: EFI Redfish Discover protocol
  ShellPkg/UefiHandleParsingLib: Support EFI Redfish protocols

 MdePkg/MdePkg.dec |  5 +-
 RedfishPkg/RedfishPkg.dec |  3 -
 .../UefiHandleParsingLib.inf  |  4 +-
 .../Include/Protocol/RedfishDiscover.h| 72 +--
 .../UefiHandleParsingLib.c|  8 ++-
 .../UefiHandleParsingLib.uni  |  4 +-
 6 files changed, 50 insertions(+), 46 deletions(-)
 rename {RedfishPkg => MdePkg}/Include/Protocol/RedfishDiscover.h (79%)

-- 
2.17.1



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

[edk2-devel] [PATCH v4 2/2] ShellPkg/UefiHandleParsingLib: Support EFI Redfish protocols

[Abner Chang]

Add handle parsing for EFI Redfish Discover protocol.
Add handle parsing for EFI RestEx protocol.

Signed-off-by: Abner Chang 
Cc: Ray Ni 
Cc: Zhichao Gao 
Cc: Nickle Wang 
Cc: Peter O'Hanley 
Reviewed-by: Zhichao Gao 
---
 .../Library/UefiHandleParsingLib/UefiHandleParsingLib.inf | 4 +++-
 .../Library/UefiHandleParsingLib/UefiHandleParsingLib.c   | 8 +++-
 .../Library/UefiHandleParsingLib/UefiHandleParsingLib.uni | 4 +++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
index 93b69cd8e9..446cd8d609 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
@@ -2,7 +2,7 @@
 #  Provides interface to advanced shell functionality for parsing both handle 
and protocol database.
 #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. 
 #  (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-#  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+#  (C) Copyright 2015-2020 Hewlett Packard Enterprise Development LP
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -269,6 +269,8 @@
   gEfiHttpProtocolGuid## UNDEFINED
   gEfiHttpUtilitiesProtocolGuid   ## UNDEFINED
   gEfiRestProtocolGuid## UNDEFINED
+  gEfiRestExProtocolGuid  ## UNDEFINED
+  gEfiRedfishDiscoverProtocolGuid ## UNDEFINED
   gEfiMmEndOfDxeProtocolGuid  ## UNDEFINED
   gEfiMmIoTrapDispatchProtocolGuid## UNDEFINED
   gEfiMmPowerButtonDispatchProtocolGuid   ## UNDEFINED
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index 500a95a89a..e34cefd7b4 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -3,7 +3,7 @@
 
   Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-  (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
+  (C) Copyright 2015-2020 Hewlett Packard Enterprise Development LP
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -2250,6 +2250,12 @@ STATIC CONST GUID_INFO_BLOCK mGuidStringList[] = {
   {STRING_TOKEN(STR_PARTITION_INFO),&gEfiPartitionInfoProtocolGuid,
PartitionInfoProtocolDumpInformation},
   {STRING_TOKEN(STR_HII_POPUP), &gEfiHiiPopupProtocolGuid, 
NULL},
 
+//
+// UEFI 2.8
+//
+  {STRING_TOKEN(STR_REST_EX),   &gEfiRestExProtocolGuid,   
   NULL},
+  {STRING_TOKEN(STR_REDFISH_DISCOVER),  &gEfiRedfishDiscoverProtocolGuid,  
   NULL},
+
 //
 // PI Spec ones
 //
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
index 9c8028d0d5..69fcbdfe0e 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
@@ -2,7 +2,7 @@
 //
 // Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. 
 // (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-// (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
+// (C) Copyright 2015-2020 Hewlett Packard Enterprise Development LP
 // SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 // Module Name:
@@ -308,6 +308,8 @@
 #string STR_NET_HTTP  #language en-US "Http"
 #string STR_NET_HTTP_U#language en-US "HttpUtilities"
 #string STR_REST  #language en-US "Rest"
+#string STR_REST_EX   #language en-US "RestEx"
+#string STR_REDFISH_DISCOVER  #language en-US "RedfishDiscover"
 
 #string STR_MM_EOD#language en-US "MmEndOfDxe"
 #string STR_MM_ITD#language en-US "MmIoTrapDispatch"
-- 
2.17.1



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

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Andrew Fish via groups.io]

Ray,

The other option would be to define a Library Class for the AcpiTableDxe driver 
to consume and have a NULL version of it in the MdeModulePkg. That would allow 
more flexibility? It kind of depends if you want to make the implementation 
depend on a HOB or a LibraryClass? It at least gives the non pure PI 
implementations more potential options? I’m not sure with the problem you are 
trying to solve that helps or hurts, but I though I would throw out another 
option. The other advantage about the lib is could let you define the HOB in 
another package, if that makes more sense. 

Not trying to slow down the solution, but just giving some other options in 
case another partitioning of this problem would be helpful? 

Thanks,

Andrew Fish 

> On Mar 24, 2021, at 6:10 PM, Ni, Ray  wrote:
> 
> Ben,
> I understand your point.
> The purpose of changing the AcpiTableDxe to directly consume the HOB is to 
> reduce the overall system complexity.
> The complexity I see with your option is:
> 1. platform needs to include that driver to consume the ACPI table from 
> bootloader
> 2. that driver (consume HOB and install table through AcpiTable protocol) 
> needs to register a protocol notification on AcpiTable protocol and do the 
> table installation in the callback.
> 
> Laszlo,
> The change here is to meet the requirement that bootloader provides the ACPI 
> table.
> In OVMF case, it's not the bootloader but the virtual hardware who provides 
> the ACPI table.
> I can see the similarity between the two: the ACPI table is produced before 
> the UEFI Payload runs.
> Can you review the new option below and see whether it can meet the OVMF 
> needs as well?
> 1. Define a new GUID gPldHobAcpiTable in MdeModulePkg.dec and a structure as 
> below in MdeModulePkg/Include/Guid/Acpi.h.
> typedef struct { 
>   UINT64  TableAddress;
> }  PLD_HOB_ACPI_TABLE;
> 2. Change AcpiTableDxe driver to consume the HOB
> 3. Remove SYSTEM_TABLE_INFO. AcpiTableBase/AcpiTableSize.
> 4. Remove the BlSupportDxe code that consume the ACPI table in 
> SYSTEM_TABLE_INFO.
> 
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
>> Sent: Thursday, March 25, 2021 2:33 AM
>> To: Benjamin Doron ; devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install 
>> Acpi table from Hob
>> 
>> On 03/24/21 17:55, Benjamin Doron wrote:
 
 
> Hi all,
> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
> and then install the tables? It's a solution that uses the regular
> UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
> is in the configuration table, we probably always want those tables).
 
 I'm sorry, I don't understand how this would help.
>>> 
>>> As I understand it, the issue is that this patchset changes MdeModulePkg to 
>>> do platform-specific parsing.
>>> 
>>> Perhaps it would be an acceptable solution for platforms to retrieve the 
>>> tables, then add
>>> RSDP/them to the configuration table to be installed by 
>>> AcpiTableDxe/AcpiPlatformDxe.
>>> This allows MdeModulePkg to abstract away the parsing, only installing 
>>> tables
>>> available to it.
>> 
>> But this is already the best approach, and already what's happening --
>> when you call EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() from the
>> platform's AcpiPlatformDxe, that's *how* you tell AcpiTableDxe in
>> MdeModulePkg to pick up the table and hook it into the RSDT / XSDT /
>> wherever, and also to manage RSD PTR as a UEFI configuration table.
>> 
>> Are you (more or less) proposing a new EFI_ACPI_TABLE_PROTOCOL member
>> function for taking a forest of ACPI tables, passed in by RSD PTR?
>> 
>> Sorry about being dense. :)
>> 
>>> (Currently, UefiPayloadPkg's BlSupportDxe retrieves the data from a HOB and 
>>> calls
>>> `gBS->InstallConfigurationTable` with the address of RSDP).
>>> 
>>> I understand that this may not work for OVMF if tables are located 
>>> differently in memory.
>>> 
 
 
> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in
> DSC) but not added to a FV (not listed in FDF). So, how has this been
> tested?
 
 I assume through an out-of-tree platform. Many such core modules exist
 in edk2 that are not consumed by any of the virtual platforms in the
 edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg).
>>> 
>>> This makes sense, but AcpiTableDxe must be added to UefiPayloadPkg's FDF
>>> if patch 2/2 is merged. Otherwise, ACPI tables will not be advertised.
>>> 
>> 
>> Good point; I missed that. (I'm not familiar with the UefiPayloadPkg FDF
>> at all.)
>> 
>> Laszlo
>> 
>> 
>> 
>> 
>> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds

[Andrew Fish via groups.io]

While we are talking about what toolchain targets should do I don’t understand 
this [1]? Why does OVMF need to turn on —keepexceptiontable? If these 
toolchains require it why don’t they turn it on? I see Mike fixed it[2]. Is 
this another case of a global GENFW_FLAG breaking things? Looks like the 
CLANGPDB toolchain does what I would expect, and the other toolchains do not? 
Is there some history here I’m missing?

[1]  $ git grep "keepexceptiontable"
BaseTools/Conf/tools_def.template:2872:DEBUG_CLANGPDB_X64_GENFW_FLAGS  = 
--keepexceptiontable
BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS  = 
--keepexceptiontable
…
OvmfPkg/OvmfPkgIa32X64.dsc:80:  MSFT:*_*_X64_GENFW_FLAGS  = --keepexceptiontable
OvmfPkg/OvmfPkgIa32X64.dsc:81:  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
OvmfPkg/OvmfPkgIa32X64.dsc:82:  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:80:  MSFT:*_*_X64_GENFW_FLAGS  = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:81:  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:82:  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:71:  MSFT:*_*_X64_GENFW_FLAGS  = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:72:  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:73:  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable

[2] $ git show bbcafc442b2db
commit bbcafc442b2db91322dd3ba04e166236a41b111d
Author: mdkinney 
Date:   Wed Oct 3 21:00:26 2012 +

The exception table information in X64 PE/COFF images is being stripped by 
default in the OvmfPkg.

This patch preserves this information when SOURCE_DEBUG_ENABLE is set.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
Reviewed-by: Laszlo Ersek

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13780 
6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9b2ba8626ab3..40fd5e97b24e 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -41,7 +41,12 @@ [BuildOptions]
   INTEL:RELEASE_*_*_CC_FLAGS   = /D MDEPKG_NDEBUG
   MSFT:RELEASE_*_*_CC_FLAGS= /D MDEPKG_NDEBUG
   GCC:*_*_*_CC_FLAGS   = -mno-mmx -mno-sse
-
+!ifdef $(SOURCE_DEBUG_ENABLE)
+  MSFT:*_*_X64_GENFW_FLAGS  = --keepexceptiontable
+  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
+  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+!endif
+
 

 #
 # SKU Identification section - list of all SKU IDs supported by this Platform.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 9ff4a5de1005..c61d41dccbbe 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -41,6 +41,11 @@ [BuildOptions]
   INTEL:RELEASE_*_*_CC_FLAGS   = /D MDEPKG_NDEBUG
   MSFT:RELEASE_*_*_CC_FLAGS= /D MDEPKG_NDEBUG
   GCC:*_*_*_CC_FLAGS   = -mno-mmx -mno-sse
+!ifdef $(SOURCE_DEBUG_ENABLE)
+  MSFT:*_*_X64_GENFW_FLAGS  = --keepexceptiontable
+  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
+  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+!endif
 
 

 #


Thanks,

Andrew Fish

> On Mar 24, 2021, at 8:38 PM, Andrew Fish  wrote:
> 
> Liming,
> 
> I understand how the tool works, but that is not how it is used. To make it 
> work like other toolchains *_*_*_GENFW_FLAGS for ELF targets should pass 
> —zero for builds that do not contain symbols. This is how it works today [1].
> 
> The proposed fix is a global hammer ` RELEASE_*_*_GENFW_FLAGS = --zero` and 
> it prevents some one from overriding the toolchain definition to turn on 
> source level debugging for Release builds. In all the other places this is a 
> tool chain choice. So my complaint is solving this globally vs. on a per 
> toolchain basis. All the other toolchains don’t produce.
> 
> If you look at the XCODE5 toolchian that we override the edk2 default   
> DEBUG_XCODE5_X64_CC_FLAGS has -g and RELEASE_XCODE5_X64_CC_FLAGS does not. So 
> the source level debugging or not is part of the compiler target choice. 
>   DEBUG_XCODE5_*_MTOC_FLAGS = -align 0x20 -d $(DEBUG_DIR)/$(MODULE_NAME).dll
> RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20
> 
> For our override we pass -g to RELEASE builds CC_FLAGS and do 
> `RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20 -d $(MODULE_GUID)` so the override 
> to force --zero on every call to GenFw will break this. 
> 
> So my point is if the toolchain is generating a Debug Directory entry in a 
> RELEASE target that does not support debugging then that target should 
> setting `RELEASE__*_GENFW_FLAGS = —zero` to undo the unwanted work. I 
> understand why the debug info gets added as it reduces the complexity of the 
> ELF to PE/COFF conversion, so I’m not complaining about that just that the 
> ELF targets don’t handle 

Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds

[Andrew Fish via groups.io]

Liming,

I understand how the tool works, but that is not how it is used. To make it 
work like other toolchains *_*_*_GENFW_FLAGS for ELF targets should pass —zero 
for builds that do not contain symbols. This is how it works today [1].

The proposed fix is a global hammer ` RELEASE_*_*_GENFW_FLAGS = --zero` and it 
prevents some one from overriding the toolchain definition to turn on source 
level debugging for Release builds. In all the other places this is a tool 
chain choice. So my complaint is solving this globally vs. on a per toolchain 
basis. All the other toolchains don’t produce.

If you look at the XCODE5 toolchian that we override the edk2 default   
DEBUG_XCODE5_X64_CC_FLAGS has -g and RELEASE_XCODE5_X64_CC_FLAGS does not. So 
the source level debugging or not is part of the compiler target choice. 
  DEBUG_XCODE5_*_MTOC_FLAGS = -align 0x20 -d $(DEBUG_DIR)/$(MODULE_NAME).dll
RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20

For our override we pass -g to RELEASE builds CC_FLAGS and do 
`RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20 -d $(MODULE_GUID)` so the override 
to force --zero on every call to GenFw will break this. 

So my point is if the toolchain is generating a Debug Directory entry in a 
RELEASE target that does not support debugging then that target should setting 
`RELEASE__*_GENFW_FLAGS = —zero` to undo the unwanted work. I 
understand why the debug info gets added as it reduces the complexity of the 
ELF to PE/COFF conversion, so I’m not complaining about that just that the ELF 
targets don’t handle this themselves. So to me this fix is working the bug of 
the ELF targets not passing —zero on RELEASE builds to GenFw.

[1] $ git grep _GENFW_FLAGS -- *.template
BaseTools/Conf/tools_def.template:2872:DEBUG_CLANGPDB_X64_GENFW_FLAGS  = 
--keepexceptiontable
BaseTools/Conf/tools_def.template:2877:RELEASE_CLANGPDB_X64_GENFW_FLAGS=
BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS  = 
--keepexceptiontable
BaseTools/Conf/tools_def.template:3124:*_*_*_GENFW_FLAGS  =

Thanks,

Andrew Fish

> On Mar 24, 2021, at 7:24 PM, gaoliming  wrote:
> 
> Andrew:
>  GenFw generates debug entry, and zero debug entry. Its functionality is same 
> to the image without debug entry. So, new option is not introduced to control 
> whether GenFw generates debug entry when it converts ELF image to EFI image. 
>
> Thanks
> Liming
> 发件人: devel@edk2.groups.io   > 代表 Andrew Fish via groups.io 
> 
> 发送时间: 2021年3月25日 7:26
> 收件人: edk2-devel-groups-io  >; r...@burtonini.com 
> 抄送: ler...@redhat.com 
> 主题: Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds
>
> This breaks some usage we have have in our fork. We have symbols turned on 
> for Release builds, so this change would break that. 
>
> It looks to me that the root cause of this issue might be that the GenFw is 
> blindly writing the debug directory entry into the PE/COFF? For native 
> PE/COFF I think this is controlled by linker flags? For Xcode/clang it is 
> controlled by the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up to 
> each toolchain how they want to deal with symbols on release builds. 
>
>
> It seems kind of strange to insert a section and then zero it. Almost seems 
> like the intent of —zero was to post process compare images? 
> 
> 
>   -z, --zeroZero the Debug Data Fields in the PE input image file.
> It also zeros the time stamp fields.
> This option can be used to compare the binary efi 
> image.
> It can't be combined with other action options
> except for -o, -r option. It is a action option.
> If it is combined with other action options, the later
> input action option will override the previous one.
> 
> 
> And in case you are going to ask our fork uses relative paths from the Build 
> directory and/or a UUID string for the Debug Directory entry file name so it 
> is a constant value and does not impact build reproducibility. 
> 
> 
> From a feature stand point this change will break any hope of source level 
> debugging with RELEASE builds. I also think it changes the exception handler 
> code output in OVMF [1] for ELF toolchains. You are going to get the (No PDB) 
> vs. the file and path you were getting today. I assume if you had tools that 
> natively produce PE/COFF and did not have a Debug Directory entry the same 
> thing would happen prior to this change. 
> 
> 
> Status = PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoint);
> if (EFI_ERROR (Status)) {
>   EntryPoint = NULL;
> }
> InternalPrintMessage (" Find image based on IP(0x%x) ", CurrentEip);
> PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);
> i

回复: [edk2-devel] 回复: [RESEND v2] MdePkg: use CpuPause() in CpuDeadLoop()

[gaoliming]

Create PR https://github.com/tianocore/edk2/pull/1515

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 gaoliming
> 发送时间: 2021年3月22日 9:40
> 收件人: 'Ankur Arora' ; devel@edk2.groups.io
> 抄送: 'Michael D Kinney' ; 'Laszlo Ersek'
> 
> 主题: [edk2-devel] 回复: [RESEND v2] MdePkg: use CpuPause() in
> CpuDeadLoop()
> 
> Reviewed-by: Liming Gao 
> 
> Thanks
> Liming
> > -邮件原件-
> > 发件人: Ankur Arora 
> > 发送时间: 2021年3月20日 2:14
> > 收件人: devel@edk2.groups.io
> > 抄送: Ankur Arora ; Liming Gao
> > ; Michael D Kinney
> > ; Laszlo Ersek 
> > 主题: [RESEND v2] MdePkg: use CpuPause() in CpuDeadLoop()
> >
> > CpuPause() might allow the CPU to go into a lower power state
> > state while we spin.
> >
> > On X86, CpuPause() executes a PAUSE instruction which the Intel
> > and AMD specs describe as follows:
> >
> > Intel:
> >   "PAUSE: An additional function of the PAUSE instruction is to reduce
> >   the power consumed by a processor while executing a spin loop. A
> >   processor can execute a spin-wait loop extremely quickly, causing the
> >   processor to consume a lot of power while it waits for the resource it
> >   is spinning on to become available. Inserting a pause instruction in a
> >   spin-wait loop greatly reduces the processor’s power consumption."
> >
> > AMD:
> >   "PAUSE: Improves the performance of spin loops, by providing a hint to
> >   the processor that the current code is in a spin loop. The processor
> >   may use this to optimize power consumption while in the spin loop.
> >   Architecturally, this instruction behaves like a NOP instruction."
> >
> > On RISC-V and ARM64, CpuPause() executes a NOP, which is no worse than
> > the tight loop we have.
> >
> > Cc: Liming Gao 
> > Signed-off-by: Ankur Arora 
> > Reviewed-by: Michael D Kinney 
> > Reviewed-by: Laszlo Ersek 
> > ---
> >  MdePkg/Library/BaseLib/CpuDeadLoop.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdePkg/Library/BaseLib/CpuDeadLoop.c
> > b/MdePkg/Library/BaseLib/CpuDeadLoop.c
> > index 9e110cacbc96..3cd304351a65 100644
> > --- a/MdePkg/Library/BaseLib/CpuDeadLoop.c
> > +++ b/MdePkg/Library/BaseLib/CpuDeadLoop.c
> > @@ -28,5 +28,7 @@ CpuDeadLoop (
> >  {
> >volatile UINTN  Index;
> >
> > -  for (Index = 0; Index == 0;);
> > +  for (Index = 0; Index == 0;) {
> > +CpuPause();
> > +  }
> >  }
> > --
> > 2.9.3
> 
> 
> 
> 
> 
> 
> 





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

回复: [edk2-devel] reg: clarification on Form Browser Action on save changes in IP4 and IP6 configuration

[gaoliming]

Siva:

 This question prompt string is "Save Changes and Exit". So, its behavior is
to save and exit. In Edk2 MdeModulePkg SetupBrowser, exit behavior is to
exit current page and enter into its parent page.  Does your SetupBrowser
implement exit behavior to exit the whole UI?

 

Thanks

Liming

发件人: devel@edk2.groups.io  代表 Sivaraman Nainar
发送时间: 2021年3月24日 20:35
收件人: devel@edk2.groups.io; maciej.rab...@intel.com
抄送: RainChen [��宇翔] 
主题: [edk2-devel] reg: clarification on Form Browser Action on save changes
in IP4 and IP6 configuration

 

Hello all:

 

In the IPV4 and IPV6 Configuration Page after the changes are applied the
system proceed to boot to the boot device and its not staying in same form.

 

case KEY_SAVE_CHANGES:

  Status = Ip6ConvertIfrNvDataToConfigNvDataGeneral (IfrNvData,
Instance);

  if (EFI_ERROR (Status)) {

break;

  }

  *ActionRequest = EFI_BROWSER_ACTION_REQUEST_SUBMIT;

  *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_APPLY; //Modified
code

  break;

 

If the browser action changed as above the control retained in the same
page.

 

Is there any specific reason after the IP Configuration Settings applied it
proceed to Boot?

 

-Siva

P 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.





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

Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: Allow BMP with extra data

[Andrew Fish via groups.io]

> On Mar 24, 2021, at 11:26 AM, Laszlo Ersek  wrote:
> 
> On 03/24/21 16:25, Jeff Brasen wrote:
>> Some of the logo files we received for the group that makes our assets like 
>> this (not sure what tool they were created with) look like they pad the BMP 
>> size to 8 bytes.
>> 
>> TranslateBmpToGopBlt: invalid BmpImage...
>>   BmpHeader->Size: 0xE1038
>>   BmpHeader->ImageOffset: 0x36
>>   BmpImageSize: 0xE1038
>>   DataSize: 0xE1000
>> TranslateBmpToGopBlt: invalid BmpImage...
>>   BmpHeader->Size: 0x2A3038
>>   BmpHeader->ImageOffset: 0x36
>>   BmpImageSize: 0x2A3038
>>   DataSize: 0x2A3000
>> TranslateBmpToGopBlt: invalid BmpImage...
>>   BmpHeader->Size: 0x5EEC38
>>   BmpHeader->ImageOffset: 0x36
>>   BmpImageSize: 0x5EEC38
>>   DataSize: 0x5EEC00
>> 
>> So, each of these has 2 bytes of padding at the end of the file. We could 
>> write a tool that would do the same size recalculation in order to update 
>> the size in the header and remove the two bytes but it seems that this is a 
>> valid BMP file and it doesn't seem correct that UEFI is rejecting it. I can 
>> update the commit message with more context if needed as well.
> 
> If there's a spec describing the BMP format,

Yes and there are various flavors as at some point I had some graphics given to 
me in a format that did not work (I think it was BITMAPV4HEADER) :(. 

https://en.wikipedia.org/wiki/BMP_file_format#cite_note-DIBhelp-5 
 

edk2 supports ‘BM’ and the BITMAPINFOHEADER DIB. I seem to remember DIBs are 
defined by the size. So ‘BM' is a Microsoft Spec:
https://docs.microsoft.com/en-us/previous-versions/ms969901(v=msdn.10)?redirectedfrom=MSDN
 


The quote in that spec is:

The file extension of a Windows DIB file is BMP. The file consists of a 
BITMAPFILEHEADER structure followed by the DIB itself. Unfortunately, because 
the BITMAPFILEHEADER structure is never actually passed to the API, not every 
application that generates BMP files fills out the data structure carefully. To 
add to this confusion, the "proper" definition of the structure is at odds with 
the documentation. Properly, the data structure contains the following fields:

The explanation of size field is:
A DWORD that specifies the size of the file in bytes. The Microsoft Windows 
Software Development Kit (SDK) documentation claims otherwise. To be on the 
safe side, many applications calculate their own sizes for reading in a file.

I would say that is not exactly a ringing endorsement from a spec point of view 
on depending on that field. So it seems like that patch may be reasonable, but 
we should triple check it does not break any security related assumptions. 

Thanks,

Andrew Fish

> and edk2 is needlessly
> strict, and the check can be relaxed without security risks, then I
> think a patch would be fair.
> 
> Thanks
> Laszlo
> 
> 
> 
> 



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

回复: [edk2-devel] [RFC PATCH 09/19] MdePkg: Add AsmPvalidate() support

[gaoliming]

Is this API X64 only? Or IA32 and X64 both?

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Brijesh Singh
> 发送时间: 2021年3月24日 23:32
> 收件人: devel@edk2.groups.io
> 抄送: Brijesh Singh ; James Bottomley
> ; Min Xu ; Jiewen Yao
> ; Tom Lendacky ;
> Jordan Justen ; Ard Biesheuvel
> ; Laszlo Ersek 
> 主题: [edk2-devel] [RFC PATCH 09/19] MdePkg: Add AsmPvalidate() support
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> The PVALIDATE instruction validates or rescinds validation of a guest
> page RMP entry. Upon completion, a return code is stored in EAX, rFLAGS
> bits OF, ZF, AF, PF and SF are set based on this return code. If the
> instruction completed succesfully, the rFLAGS bit CF indicates if the
> contents of the RMP entry were changed or not.
> 
> For more information about the instruction see AMD APM volume 3.
> 
> Cc: James Bottomley 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Cc: Tom Lendacky 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Signed-off-by: Brijesh Singh 
> ---
>  MdePkg/Include/Library/BaseLib.h  | 37 +
>  MdePkg/Library/BaseLib/BaseLib.inf|  1 +
>  MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 43 
>  3 files changed, 81 insertions(+)
> 
> diff --git a/MdePkg/Include/Library/BaseLib.h
> b/MdePkg/Include/Library/BaseLib.h
> index 1171a0ffb5..fee27e9a1b 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -7495,5 +7495,42 @@ PatchInstructionX86 (
>IN  UINTNValueSize
>);
> 
> +/**
> + Execute a PVALIDATE instruction to validate or rescnids validation of a
guest
> + page's RMP entry.
> +
> + Upon completion, in addition to the return value the instruction also
> updates
> + the eFlags. A caller must check both the return code as well as eFlags
to
> + determine if the RMP entry has been updated.
> +
> + The function is available on x64.
> +
> + @param[in]AddressThe guest virtual address to validate.
> + @param[in]PageSize   The page size to use.
> + @param[i] Validate   Validate or rescinds.
> + @param[out]   Eflags The value of Eflags after PVALIDATE
> completion.
> +
> + @retval   PvalidateRetValue  The return value from the PVALIDATE
> instruction.
> +**/
> +typedef enum {
> +  PVALIDATE_PAGE_SIZE_4K = 0,
> +  PVALIDATE_PAGE_SIZE_2M,
> +} PvalidatePageSize;
> +
> +typedef enum {
> +  PVALIDATE_RET_SUCCESS = 0,
> +  PVALIDATE_RET_FAIL_INPUT = 1,
> +  PVALIDATE_RET_FAIL_SIZEMISMATCH = 6,
> +} PvalidateRetValue;
> +
> +PvalidateRetValue
> +EFIAPI
> +AsmPvalidate (
> +  IN   PvalidatePageSize   PageSize,
> +  IN   BOOLEAN Validate,
> +  IN   UINTN   Address,
> +  OUT  IA32_EFLAGS32   *Eflags
> +  );
> +
>  #endif // defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
>  #endif // !defined (__BASE_LIB__)
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> index 3b85c56c3c..01aa5cc7a4 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -319,6 +319,7 @@
>X64/RdRand.nasm
>X64/XGetBv.nasm
>X64/VmgExit.nasm
> +  X64/Pvalidate.nasm
>ChkStkGcc.c  | GCC
> 
>  [Sources.EBC]
> diff --git a/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
> b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
> new file mode 100644
> index 00..f2aba114ac
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
> @@ -0,0 +1,43 @@
>
+;--
---
> +;
> +; Copyright (c) 2020-2021, AMD. All rights reserved.
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +; Module Name:
> +;
> +;   Pvalidate.Asm
> +;
> +; Abstract:
> +;
> +;   AsmPvalidate function
> +;
> +; Notes:
> +;
>
+;--
---
> +
> +SECTION .text
> +
>
+;--
---
> +;  PvalidateRetValue
> +;  EFIAPI
> +;  AsmPvalidate (
> +;IN   UINT32  RmpPageSize
> +;IN   UINT32  Validate,
> +;IN   UINTN   Address,
> +;OUT  UINTN  *Eflags,
> +;)
>
+;--
---
> +global ASM_PFX(AsmPvalidate)
> +ASM_PFX(AsmPvalidate):
> +  mov rax, r8
> +
> +  ; PVALIDATE instruction opcode
> +  DB  0xF2, 0x0F, 0x01, 0xFF
> +
> +  ; Read the Eflags
> +  pushfq
> +  pop r8
> +  mov [r9], r8
> +
> +  ; The PVALIDATE instruction returns the status in rax register.
> +  ret
> --
> 2.17.1
> 
> 
> 
> 
> 





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

回复: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds

[gaoliming]

Andrew:

 GenFw generates debug entry, and zero debug entry. Its functionality is same 
to the image without debug entry. So, new option is not introduced to control 
whether GenFw generates debug entry when it converts ELF image to EFI image. 

 

Thanks

Liming

发件人: devel@edk2.groups.io  代表 Andrew Fish via groups.io
发送时间: 2021年3月25日 7:26
收件人: edk2-devel-groups-io ; r...@burtonini.com
抄送: ler...@redhat.com
主题: Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds

 

This breaks some usage we have have in our fork. We have symbols turned on for 
Release builds, so this change would break that. 

 

It looks to me that the root cause of this issue might be that the GenFw is 
blindly writing the debug directory entry into the PE/COFF? For native PE/COFF 
I think this is controlled by linker flags? For Xcode/clang it is controlled by 
the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up to each toolchain 
how they want to deal with symbols on release builds. 

 

 

It seems kind of strange to insert a section and then zero it. Almost seems 
like the intent of —zero was to post process compare images? 





  -z, --zeroZero the Debug Data Fields in the PE input image file.

It also zeros the time stamp fields.

This option can be used to compare the binary efi image.

It can't be combined with other action options

except for -o, -r option. It is a action option.

If it is combined with other action options, the later

input action option will override the previous one.





And in case you are going to ask our fork uses relative paths from the Build 
directory and/or a UUID string for the Debug Directory entry file name so it is 
a constant value and does not impact build reproducibility. 





>From a feature stand point this change will break any hope of source level 
>debugging with RELEASE builds. I also think it changes the exception handler 
>code output in OVMF [1] for ELF toolchains. You are going to get the (No PDB) 
>vs. the file and path you were getting today. I assume if you had tools that 
>natively produce PE/COFF and did not have a Debug Directory entry the same 
>thing would happen prior to this change. 






Status = PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoint);


if (EFI_ERROR (Status)) {


  EntryPoint = NULL;


}


InternalPrintMessage (" Find image based on IP(0x%x) ", CurrentEip);


PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);


if (PdbPointer != NULL) {


  InternalPrintMessage ("%a", PdbPointer);


} else {


  InternalPrintMessage ("(No PDB) " );


}


InternalPrintMessage (


  " (ImageBase=%016lp, EntryPoint=%016p) \n",


  (VOID *) Pe32Data,


  EntryPoint


  );







Not saying we have to "stop the presses", but just trying to point out the side 
effects of this change. It is not so much that this change is bad, but that we 
have no way to turn off the Debug Directory Entry for ELF conversion, so we 
seem to be working around that issue with a bigger hammer?

 

[1] 
https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c#L117

 

Thanks,





Andrew Fish





On Mar 24, 2021, at 4:58 AM, Ross Burton mailto:r...@burtonini.com> > wrote:

 

GenFw will embed a NB10 section which contains the path to the input file,
which means the output files have build paths embedded in them.  To reduce
information leakage and ensure reproducible builds, pass --zero in release
builds to remove this information.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3256
Signed-off-by: Ross Burton mailto:ross.bur...@arm.com> >
---
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/Bhyve/BhyveX64.dsc   | 1 +
OvmfPkg/OvmfPkgIa32.dsc  | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc   | 1 +
OvmfPkg/OvmfPkgX64.dsc   | 1 +
OvmfPkg/OvmfXen.dsc  | 1 +
6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 65c42284d9..69a05feea9 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -78,6 +78,7 @@
  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS = --zero

  #
  # Disable deprecated APIs.
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 4a1cdf5aca..132f55cf69 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -76,6 +76,7 @@
  GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
  INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
!endif
+  RELEASE_*_*_GENFW_FLAGS = --zero

  #
  # Disable deprecated APIs.
diff --git a/OvmfPkg/O

回复: [edk2-devel] [PATCH V3 2/2] ShellPkg/UefiHandleParsingLib: Support EFI Redfish protocols

[gaoliming]

Abner:
  I have one comment. 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Abner Chang
> 发送时间: 2021年3月24日 13:35
> 收件人: devel@edk2.groups.io
> 抄送: Ray Ni ; Zhichao Gao ;
> Nickle Wang ; Peter O'Hanley
> 
> 主题: [edk2-devel] [PATCH V3 2/2] ShellPkg/UefiHandleParsingLib: Support
> EFI Redfish protocols
> 
> Add handle parsing for EFI Redfish Discover protocol.
> Add handle parsing for EFI RestEx protocol.
> 
> Signed-off-by: Abner Chang 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Cc: Nickle Wang 
> Cc: Peter O'Hanley 
> Reviewed-by: Zhichao Gao 
> ---
>  .../Library/UefiHandleParsingLib/UefiHandleParsingLib.inf | 4 +++-
>  .../Library/UefiHandleParsingLib/UefiHandleParsingLib.c   | 8 ++--
>  .../Library/UefiHandleParsingLib/UefiHandleParsingLib.uni | 4 +++-
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git
a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> index 93b69cd8e9..446cd8d609 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
> @@ -2,7 +2,7 @@
>  #  Provides interface to advanced shell functionality for parsing both
handle
> and protocol database.
>  #  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.

>  #  (C) Copyright 2013-2015 Hewlett-Packard Development Company,
> L.P.
> -#  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> +#  (C) Copyright 2015-2020 Hewlett Packard Enterprise Development
> LP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -269,6 +269,8 @@
>gEfiHttpProtocolGuid##
> UNDEFINED
>gEfiHttpUtilitiesProtocolGuid   ##
> UNDEFINED
>gEfiRestProtocolGuid##
> UNDEFINED
> +  gEfiRestExProtocolGuid  ##
> UNDEFINED
> +  gEfiRedfishDiscoverProtocolGuid ##
> UNDEFINED
>gEfiMmEndOfDxeProtocolGuid  ##
> UNDEFINED
>gEfiMmIoTrapDispatchProtocolGuid##
> UNDEFINED
>gEfiMmPowerButtonDispatchProtocolGuid   ##
> UNDEFINED
> diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> index 500a95a89a..c00337d6b2 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> @@ -3,7 +3,7 @@
> 
>Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
>(C) Copyright 2013-2015 Hewlett-Packard Development Company,
> L.P.
> -  (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> +  (C) Copyright 2015-2020 Hewlett Packard Enterprise Development
> LP
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -2355,7 +2355,11 @@ STATIC CONST GUID_INFO_BLOCK
> mGuidStringList[] = {
>{STRING_TOKEN(STR_NET_HTTP),  &gEfiHttpProtocolGuid,
> NULL},
>{STRING_TOKEN(STR_NET_HTTP_U),
> &gEfiHttpUtilitiesProtocolGuid,   NULL},
>{STRING_TOKEN(STR_REST),  &gEfiRestProtocolGuid,
> NULL},
> -
> +//
> +// UEFI 2.8
> +//
> +  {STRING_TOKEN(STR_REST_EX),
> &gEfiRestExProtocolGuid,  NULL},
> +  {STRING_TOKEN(STR_REDFISH_DISCOVER),
> &gEfiRedfishDiscoverProtocolGuid, NULL},

This change should be moved to the position after UEFI 2.7 in this table. 

Thanks
Liming
>  //
>  // PI 1.5
>  //
> diff --git
a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> index 9c8028d0d5..69fcbdfe0e 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> @@ -2,7 +2,7 @@
>  //
>  // Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.

>  // (C) Copyright 2013-2015 Hewlett-Packard Development Company,
> L.P.
> -// (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
> +// (C) Copyright 2015-2020 Hewlett Packard Enterprise Development
> LP
>  // SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
>  // Module Name:
> @@ -308,6 +308,8 @@
>  #string STR_NET_HTTP  #language en-US "Http"
>  #string STR_NET_HTTP_U#language en-US "HttpUtilities"
>  #string STR_REST  #language en-US "Rest"
> +#string STR_REST_EX   #language en-US "RestEx"
> +#string STR_REDFISH_DISCOVER  #language en-US "RedfishDiscover"
> 
>  #string STR_MM_EOD#language en-US "MmEndOfDxe"
>  #string STR_MM_ITD#language en-US
> "MmIoTrapDispatch"
> --
> 2.17.1
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73247): https://edk2.groups.io/g/devel/messag

Re: [edk2-devel] [PATCH v1 02/15] StandaloneMmPkg: StandaloneMmCoreHobLib: Extend support for x64 Mm Core

[Dawn via groups.io]

Importing spamReporting Tracking successful 
  On Fri, Mar 12, 2021 at 4:46 AM, Yao, Jiewen wrote:   
Can we merge the common part between X64 and AArch64 into one Common.c?

> -Original Message-
> From: Kun Qin 
> Sent: Saturday, December 19, 2020 2:50 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Sami Mujawar
> ; Yao, Jiewen ; Supreeth
> Venkatesh 
> Subject: [PATCH v1 02/15] StandaloneMmPkg: StandaloneMmCoreHobLib:
> Extend support for x64 Mm Core
> 
> This change adds support of x64 version of StandaloneMmCoreHobLib. It
> brings in global variable "gHobList" through StandaloneMmCoreEntryPoint
> and imports implementation from DxeCoreHobLib.inf to support x64 Mm
> Core.
> 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Jiewen Yao 
> Cc: Supreeth Venkatesh 
> 
> Signed-off-by: Kun Qin 
> ---
>  StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{ =>
> AArch64}/StandaloneMmCoreHobLib.c |  6 +-
>  StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{ =>
> X64}/StandaloneMmCoreHobLib.c    | 426 ++--
> 
> StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCore
> HobLib.inf            |  8 +-
>  3 files changed, 215 insertions(+), 225 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCo
> reHobLib.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/Standal
> oneMmCoreHobLib.c
> similarity index 96%
> copy from
> StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCore
> HobLib.c
> copy to
> StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/Standalon
> eMmCoreHobLib.c
> index e3d4743b63f2..006bff816e39 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCo
> reHobLib.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/Standal
> oneMmCoreHobLib.c
> @@ -13,14 +13,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> -//
> -// Cache copy of HobList pointer.
> -//
> -VOID *gHobList = NULL;
> -
>  /**
>    Returns the pointer to the HOB list.
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCo
> reHobLib.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/X64/Standalone
> MmCoreHobLib.c
> similarity index 70%
> rename from
> StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCore
> HobLib.c
> rename to
> StandaloneMmPkg/Library/StandaloneMmCoreHobLib/X64/StandaloneM
> mCoreHobLib.c
> index e3d4743b63f2..69b20bf07a21 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCo
> reHobLib.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/X64/Standalone
> MmCoreHobLib.c
> @@ -13,14 +13,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> -//
> -// Cache copy of HobList pointer.
> -//
> -VOID *gHobList = NULL;
> -
>  /**
>    Returns the pointer to the HOB list.
> 
> @@ -203,48 +199,13 @@ GetBootModeHob (
>    return HandOffHob->BootMode;
>  }
> 
> -VOID *
> -CreateHob (
> -  IN  UINT16    HobType,
> -  IN  UINT16    HobLength
> -  )
> -{
> -  EFI_HOB_HANDOFF_INFO_TABLE  *HandOffHob;
> -  EFI_HOB_GENERIC_HEADER      *HobEnd;
> -  EFI_PHYSICAL_ADDRESS        FreeMemory;
> -  VOID                        *Hob;
> -
> -  HandOffHob = GetHobList ();
> -
> -  HobLength = (UINT16)((HobLength + 0x7) & (~0x7));
> -
> -  FreeMemory = HandOffHob->EfiFreeMemoryTop - HandOffHob-
> >EfiFreeMemoryBottom;
> -
> -  if (FreeMemory < HobLength) {
> -    return NULL;
> -  }
> -
> -  Hob = (VOID*) (UINTN) HandOffHob->EfiEndOfHobList;
> -  ((EFI_HOB_GENERIC_HEADER*) Hob)->HobType = HobType;
> -  ((EFI_HOB_GENERIC_HEADER*) Hob)->HobLength = HobLength;
> -  ((EFI_HOB_GENERIC_HEADER*) Hob)->Reserved = 0;
> -
> -  HobEnd = (EFI_HOB_GENERIC_HEADER*) ((UINTN)Hob + HobLength);
> -  HandOffHob->EfiEndOfHobList = (EFI_PHYSICAL_ADDRESS) (UINTN)
> HobEnd;
> -
> -  HobEnd->HobType  = EFI_HOB_TYPE_END_OF_HOB_LIST;
> -  HobEnd->HobLength = sizeof (EFI_HOB_GENERIC_HEADER);
> -  HobEnd->Reserved  = 0;
> -  HobEnd++;
> -  HandOffHob->EfiFreeMemoryBottom = (EFI_PHYSICAL_ADDRESS) (UINTN)
> HobEnd;
> -
> -  return Hob;
> -}
> -
>  /**
>    Builds a HOB for a loaded PE32 module.
> 
>    This function builds a HOB for a loaded PE32 module.
> +  It can only be invoked during PEI phase;
> +  for MM phase, it will ASSERT() because PEI HOB is read-only for MM
> phase.
> +
>    If ModuleName is NULL, then ASSERT().
>    If there is no additional space for HOB creation, then ASSERT().
> 
> @@ -263,31 +224,51 @@ BuildModuleHob (
>    IN EFI_PHYSICAL_ADDRESS  EntryPoint
>    )
>  {
> -  EFI_HOB_MEMORY_ALLOCATION_MODULE  *Hob;
> +  //
> +  // PEI HOB is read only for MM phase
> +  //
> +  ASSERT (FALSE);
> +}
> 
> -  ASSERT (((MemoryAllocationModule & (EFI_PAGE_SIZE - 1)) == 0) &&
> -          ((ModuleLength & (EFI_PAGE_SIZE - 1)) == 0));
> +/**
> +  Builds a HOB that describes a chunk of system memory with Owner GUID.
> 
> -  Hob = Creat

[edk2-devel] 回复: [PATCH V3 1/2] MdePkg/Include: EFI Redfish Discover protocol

[gaoliming]

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: Abner Chang 
> 发送时间: 2021年3月24日 13:35
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney ; Liming Gao
> ; Zhiguang Liu ; Nickle
> Wang ; Peter O'Hanley 
> 主题: [PATCH V3 1/2] MdePkg/Include: EFI Redfish Discover protocol
> 
> Move GUID definition of EFI Redfish Discover protocol
> to under MdePkg. With this we don't have dependency of
> RedfishPkg in ShellPkg.
> 
> Signed-off-by: Abner Chang 
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Nickle Wang 
> Cc: Peter O'Hanley 
> ---
>  MdePkg/MdePkg.dec |  5 +-
>  RedfishPkg/RedfishPkg.dec |  3 -
>  .../Include/Protocol/RedfishDiscover.h| 72 +--
>  3 files changed, 37 insertions(+), 43 deletions(-)
>  rename {RedfishPkg => MdePkg}/Include/Protocol/RedfishDiscover.h (79%)
> 
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 1d2637acc2..e667d44db5 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -6,7 +6,7 @@
>  #
>  # Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
>  # Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> -# (C) Copyright 2016 - 2020 Hewlett Packard Enterprise Development
> LP
> +# (C) Copyright 2016 - 2021 Hewlett Packard Enterprise Development
> LP
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -1863,6 +1863,9 @@
>## Include/Protocol/RestJsonStructure.h
>gEfiRestJsonStructureProtocolGuid  = { 0xa9a048f6, 0x48a0, 0x4714,
> {0xb7, 0xda, 0xa9, 0xad,0x87, 0xd4, 0xda, 0xc9 }}
> 
> +  ## Include/Protocol/RedfishDiscover.h
> +  gEfiRedfishDiscoverProtocolGuid  = { 0x5db12509, 0x4550, 0x4347,
> { 0x96, 0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
> +
>#
># Protocols defined in Shell2.0
>#
> diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> index b3e25268a0..846c19fd5e 100644
> --- a/RedfishPkg/RedfishPkg.dec
> +++ b/RedfishPkg/RedfishPkg.dec
> @@ -67,9 +67,6 @@
>RedfishLib|PrivateInclude/Library/RedfishLib.h
> 
>  [Protocols]
> -  ## Include/Protocol/RedfishDiscover.h
> -  gEfiRedfishDiscoverProtocolGuid  = { 0x5db12509, 0x4550, 0x4347,
> { 0x96, 0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
> -
>## Include/Protocol/EdkIIRedfishCredential.h
>gEdkIIRedfishCredentialProtocolGuid = { 0x8804377, 0xaf7a, 0x4496,
> { 0x8a, 0x7b, 0x17, 0x59, 0x0, 0xe9, 0xab, 0x46 } }
> 
> diff --git a/RedfishPkg/Include/Protocol/RedfishDiscover.h
> b/MdePkg/Include/Protocol/RedfishDiscover.h
> similarity index 79%
> rename from RedfishPkg/Include/Protocol/RedfishDiscover.h
> rename to MdePkg/Include/Protocol/RedfishDiscover.h
> index 4c91605c4e..8dbb70b082 100644
> --- a/RedfishPkg/Include/Protocol/RedfishDiscover.h
> +++ b/MdePkg/Include/Protocol/RedfishDiscover.h
> @@ -1,20 +1,19 @@
>  /** @file
>This file defines the EFI Redfish Discover Protocol interface.
> 
> -  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
> +  (C) Copyright 2021 Hewlett Packard Enterprise Development LP
> 
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +  @par Revision Reference:
> +  - Some corrections and revises are added to UEFI Specification 2.9.
> +  - This Protocol is introduced in UEFI Specification 2.8.
> +
>  **/
> 
>  #ifndef EFI_REDFISH_DISCOVER_PROTOCOL_H_
>  #define EFI_REDFISH_DISCOVER_PROTOCOL_H_
> 
> -#include 
> -#include 
> -#include 
> -#include 
> -
>  //
>  // GUID definitions
>  //
> @@ -40,24 +39,21 @@ typedef UINT32 EFI_REDFISH_DISCOVER_FLAG;
> ///< 3
> to 15. The corresponding duration is 8 to 2^15 seconds.
> ///<
> Duration is only valid when EFI_REDFISH_DISCOVER_KEEP_ALIVE
> ///< is
> set to 1.
> -#define EFI_REDFISH_DISCOVER_DURATION_BIT_POS 8
> -
>  typedef struct _EFI_REDFISH_DISCOVER_PROTOCOL
> EFI_REDFISH_DISCOVER_PROTOCOL;
> -typedef struct _EFI_REDFISH_DISCOVERED_INFORMATION
> EFI_REDFISH_DISCOVERED_INFORMATION;
> -
> -typedef struct _EFI_REDFISH_DISCOVERED_INFORMATION {
> -  EFI_HANDLE RedfishRestExHandle;   ///< REST EX EFI handle
> associated with this Redfish service.
> -  BOOLEAN IsUdp6;   ///< Indicates it's IP
> versino 6.
> -  EFI_IP_ADDRESS  RedfishHostIpAddress; ///< IP address of Redfish
> service.
> -  UINTN RedfishVersion; ///< Redfish service
> version.
> -  CHAR16 *Location; ///< Redfish service
> location.
> -  CHAR16 *Uuid; ///< Redfish service
> UUID.
> -  CHAR16 *Os;   ///< Redfish service
> OS.
> -  CHAR16 *OSVersion;///< Redfish service OS
> version.
> -  CHAR16 *Product;  ///< Redfish service
> product name.
> -  CHAR16 *ProductVer;   ///< Redfish service
> product version.
> -

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Benjamin Doron]

On Wed, Mar 24, 2021 at 02:33 PM, Laszlo Ersek wrote:

> 
> 
>> 
>>> 
 Hi all,
 Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
 MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
 and then install the tables? It's a solution that uses the regular
 UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
 is in the configuration table, we probably always want those tables).
>>> 
>>> I'm sorry, I don't understand how this would help.
>> 
>> As I understand it, the issue is that this patchset changes MdeModulePkg
>> to do platform-specific parsing.
>> 
>> Perhaps it would be an acceptable solution for platforms to retrieve the
>> tables, then add
>> RSDP/them to the configuration table to be installed by
>> AcpiTableDxe/AcpiPlatformDxe.
>> This allows MdeModulePkg to abstract away the parsing, only installing
>> tables
>> available to it.
> 
> But this is already the best approach, and already what's happening --
> when you call EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() from the
> platform's AcpiPlatformDxe, that's *how* you tell AcpiTableDxe in
> MdeModulePkg to pick up the table and hook it into the RSDT / XSDT /
> wherever, and also to manage RSD PTR as a UEFI configuration table.
> 
> Are you (more or less) proposing a new EFI_ACPI_TABLE_PROTOCOL member
> function for taking a forest of ACPI tables, passed in by RSD PTR?

Yes.
I thought the implementation of AcpiPlatformDxe in MdeModulePkg could take it, 
so it will install
ACPI tables from flash or from memory.

Regarding UefiPayloadPkg: gEfiAcpiTableGuid is not a HOB. It's an entry in
gUefiSystemTableInfoGuid (which is a HOB) that was installed in the config 
table. Therefore,
retrieving its GUID as a HOB in AcpiTableDxe fails.

To make this patchset work in UefiPayloadPkg (actually a fork), I have to add 
AcpiTableDxe to its
FDF, drop patch 2/2 and make `InstallAcpiTableFromHob` do
`Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **) 
&Rsdp);`.
Then, this patchset works: "acpiview" shell command shows tables from QEMU + 
coreboot, as well
as a BGRT.


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

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Ni, Ray]

Ben,
I understand your point.
The purpose of changing the AcpiTableDxe to directly consume the HOB is to 
reduce the overall system complexity.
The complexity I see with your option is:
1. platform needs to include that driver to consume the ACPI table from 
bootloader
2. that driver (consume HOB and install table through AcpiTable protocol) needs 
to register a protocol notification on AcpiTable protocol and do the table 
installation in the callback.

Laszlo,
The change here is to meet the requirement that bootloader provides the ACPI 
table.
In OVMF case, it's not the bootloader but the virtual hardware who provides the 
ACPI table.
I can see the similarity between the two: the ACPI table is produced before the 
UEFI Payload runs.
Can you review the new option below and see whether it can meet the OVMF needs 
as well?
1. Define a new GUID gPldHobAcpiTable in MdeModulePkg.dec and a structure as 
below in MdeModulePkg/Include/Guid/Acpi.h.
typedef struct { 
   UINT64  TableAddress;
}  PLD_HOB_ACPI_TABLE;
2. Change AcpiTableDxe driver to consume the HOB
3. Remove SYSTEM_TABLE_INFO. AcpiTableBase/AcpiTableSize.
4. Remove the BlSupportDxe code that consume the ACPI table in 
SYSTEM_TABLE_INFO.


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
> Sent: Thursday, March 25, 2021 2:33 AM
> To: Benjamin Doron ; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi 
> table from Hob
> 
> On 03/24/21 17:55, Benjamin Doron wrote:
> >>
> >>
> >>> Hi all,
> >>> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
> >>> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
> >>> and then install the tables? It's a solution that uses the regular
> >>> UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
> >>> is in the configuration table, we probably always want those tables).
> >>
> >> I'm sorry, I don't understand how this would help.
> >
> > As I understand it, the issue is that this patchset changes MdeModulePkg to 
> > do platform-specific parsing.
> >
> > Perhaps it would be an acceptable solution for platforms to retrieve the 
> > tables, then add
> > RSDP/them to the configuration table to be installed by 
> > AcpiTableDxe/AcpiPlatformDxe.
> > This allows MdeModulePkg to abstract away the parsing, only installing 
> > tables
> > available to it.
> 
> But this is already the best approach, and already what's happening --
> when you call EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() from the
> platform's AcpiPlatformDxe, that's *how* you tell AcpiTableDxe in
> MdeModulePkg to pick up the table and hook it into the RSDT / XSDT /
> wherever, and also to manage RSD PTR as a UEFI configuration table.
> 
> Are you (more or less) proposing a new EFI_ACPI_TABLE_PROTOCOL member
> function for taking a forest of ACPI tables, passed in by RSD PTR?
> 
> Sorry about being dense. :)
> 
> > (Currently, UefiPayloadPkg's BlSupportDxe retrieves the data from a HOB and 
> > calls
> > `gBS->InstallConfigurationTable` with the address of RSDP).
> >
> > I understand that this may not work for OVMF if tables are located 
> > differently in memory.
> >
> >>
> >>
> >>> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in
> >>> DSC) but not added to a FV (not listed in FDF). So, how has this been
> >>> tested?
> >>
> >> I assume through an out-of-tree platform. Many such core modules exist
> >> in edk2 that are not consumed by any of the virtual platforms in the
> >> edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg).
> >
> > This makes sense, but AcpiTableDxe must be added to UefiPayloadPkg's FDF
> > if patch 2/2 is merged. Otherwise, ACPI tables will not be advertised.
> >
> 
> Good point; I missed that. (I'm not familiar with the UefiPayloadPkg FDF
> at all.)
> 
> Laszlo
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH] RISC-V/PlatformPkg: Fix compilation breakage in OpenSBI

[Abner Chang]

Hi Loic,
The current edk2 RISC-V is incorporated with opensbi v0.8, please check below 
link
https://github.com/riscv/riscv-uefi-edk2-docs

We have to update below library with the latest opensbi with your changes,
https://github.com/tianocore/edk2-platforms/tree/master/Silicon/RISC-V/ProcessorPkg/Library/RiscVOpensbiLib
 

I believe your changes don't impact the build and the functionalities because 
we don't use those parameters in edk2. However, this change should be adopted 
with Edk2OpenSbiLib upgrade.
Daniel is currently working on edk2 RISC-V OVMF and also updated opensbi to the 
latest version. However there is a compatible issue when switching mode and we 
are fixing it now.
We can share more information with you if you would like to know.

Thanks
Abner
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Loic Devulder via groups.io
> Sent: Wednesday, March 24, 2021 9:59 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH] RISC-V/PlatformPkg: Fix compilation breakage
> in OpenSBI
> 
> platform_ops and platform structures have older entries that don't exist
> anymore on recent OpenSBI versions, so we can remove them.
> 
> Cc: Abner Chang 
> Cc: Daniel Schaefer 
> Cc: Gilbert Chen 
> 
> Signed-off-by: Loic Devulder 
> ---
>  .../PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/Platform/RISC-
> V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c b/Platform/RISC-
> V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c
> index e78d811f4c..84b5f5dec8 100644
> --- a/Platform/RISC-
> V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c
> +++ b/Platform/RISC-V/PlatformPkg/Library/OpensbiPlatformLibNull/Platfor
> +++ m.c
> @@ -28,8 +28,7 @@ const struct sbi_platform_operations platform_ops = {
>  .timer_event_stop   = NULL,
>  .timer_event_start  = NULL,
>  .timer_init = NULL,
> -.system_reboot  = NULL,
> -.system_shutdown= NULL
> +.system_reset   = NULL
>  };
> 
>  const struct sbi_platform platform = {
> @@ -39,6 +38,5 @@ const struct sbi_platform platform = {
>  .features   = 0,
>  .hart_count = 0,
>  .hart_stack_size= 0,
> -.disabled_hart_mask = 0,
>  .platform_ops_addr  = 0
>  };
> --
> 2.30.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73242): https://edk2.groups.io/g/devel/message/73242
Mute This Topic: https://groups.io/mt/81577303/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] OvmfPkg: strip build paths in release builds

[Andrew Fish via groups.io]

This breaks some usage we have have in our fork. We have symbols turned on for 
Release builds, so this change would break that. 

It looks to me that the root cause of this issue might be that the GenFw is 
blindly writing the debug directory entry into the PE/COFF? For native PE/COFF 
I think this is controlled by linker flags? For Xcode/clang it is controlled by 
the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up to each toolchain 
how they want to deal with symbols on release builds. 


It seems kind of strange to insert a section and then zero it. Almost seems 
like the intent of —zero was to post process compare images? 

  -z, --zeroZero the Debug Data Fields in the PE input image file.
It also zeros the time stamp fields.
This option can be used to compare the binary efi image.
It can't be combined with other action options
except for -o, -r option. It is a action option.
If it is combined with other action options, the later
input action option will override the previous one.

And in case you are going to ask our fork uses relative paths from the Build 
directory and/or a UUID string for the Debug Directory entry file name so it is 
a constant value and does not impact build reproducibility. 

>From a feature stand point this change will break any hope of source level 
>debugging with RELEASE builds. I also think it changes the exception handler 
>code output in OVMF [1] for ELF toolchains. You are going to get the (No PDB) 
>vs. the file and path you were getting today. I assume if you had tools that 
>natively produce PE/COFF and did not have a Debug Directory entry the same 
>thing would happen prior to this change. 

Status = PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoint);
if (EFI_ERROR (Status)) {
  EntryPoint = NULL;
}
InternalPrintMessage (" Find image based on IP(0x%x) ", CurrentEip);
PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);
if (PdbPointer != NULL) {
  InternalPrintMessage ("%a", PdbPointer);
} else {
  InternalPrintMessage ("(No PDB) " );
}
InternalPrintMessage (
  " (ImageBase=%016lp, EntryPoint=%016p) \n",
  (VOID *) Pe32Data,
  EntryPoint
  );

Not saying we have to "stop the presses", but just trying to point out the side 
effects of this change. It is not so much that this change is bad, but that we 
have no way to turn off the Debug Directory Entry for ELF conversion, so we 
seem to be working around that issue with a bigger hammer?

[1] 
https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c#L117

Thanks,

Andrew Fish

> On Mar 24, 2021, at 4:58 AM, Ross Burton  wrote:
> 
> GenFw will embed a NB10 section which contains the path to the input file,
> which means the output files have build paths embedded in them.  To reduce
> information leakage and ensure reproducible builds, pass --zero in release
> builds to remove this information.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3256
> Signed-off-by: Ross Burton 
> ---
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/Bhyve/BhyveX64.dsc   | 1 +
> OvmfPkg/OvmfPkgIa32.dsc  | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc   | 1 +
> OvmfPkg/OvmfPkgX64.dsc   | 1 +
> OvmfPkg/OvmfXen.dsc  | 1 +
> 6 files changed, 6 insertions(+)
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 65c42284d9..69a05feea9 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -78,6 +78,7 @@
>   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
> !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
> 
>   #
>   # Disable deprecated APIs.
> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
> index 4a1cdf5aca..132f55cf69 100644
> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
> @@ -76,6 +76,7 @@
>   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
> !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
> 
>   #
>   # Disable deprecated APIs.
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 1eaf3e99c6..93c209950c 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -80,6 +80,7 @@
> !if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANGPDB"
>   GCC:*_*_*_CC_FLAGS   = -mno-mmx -mno-sse
> !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
> 
>   #
>   # Disable deprecated APIs.
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 4a5a430147..97cc438250 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -84,6 +84,7 @@
>   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
> !en

Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds

[Laszlo Ersek]

On 03/24/21 12:58, Ross Burton wrote:
> GenFw will embed a NB10 section which contains the path to the input file,
> which means the output files have build paths embedded in them.  To reduce
> information leakage and ensure reproducible builds, pass --zero in release
> builds to remove this information.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3256
> Signed-off-by: Ross Burton 
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
>  OvmfPkg/Bhyve/BhyveX64.dsc   | 1 +
>  OvmfPkg/OvmfPkgIa32.dsc  | 1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc   | 1 +
>  OvmfPkg/OvmfPkgX64.dsc   | 1 +
>  OvmfPkg/OvmfXen.dsc  | 1 +
>  6 files changed, 6 insertions(+)

Reviewed-by: Laszlo Ersek 

Merged as commit f037af6ecbc3, via
.

Thanks,
Laszlo


> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 65c42284d9..69a05feea9 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -78,6 +78,7 @@
>GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>  !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
>  
>#
># Disable deprecated APIs.
> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
> index 4a1cdf5aca..132f55cf69 100644
> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
> @@ -76,6 +76,7 @@
>GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>  !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
>  
>#
># Disable deprecated APIs.
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 1eaf3e99c6..93c209950c 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -80,6 +80,7 @@
>  !if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANGPDB"
>GCC:*_*_*_CC_FLAGS   = -mno-mmx -mno-sse
>  !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
>  
>#
># Disable deprecated APIs.
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 4a5a430147..97cc438250 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -84,6 +84,7 @@
>GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>  !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
>  
>#
># Disable deprecated APIs.
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index d4d601b444..f544fb04bf 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -84,6 +84,7 @@
>GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>  !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
>  
>#
># Disable deprecated APIs.
> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
> index 507029404f..fcaa35acf1 100644
> --- a/OvmfPkg/OvmfXen.dsc
> +++ b/OvmfPkg/OvmfXen.dsc
> @@ -74,6 +74,7 @@
>GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
>INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>  !endif
> +  RELEASE_*_*_GENFW_FLAGS = --zero
>  
>#
># Disable deprecated APIs.
> 



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

Re: [edk2-devel] [RFC PATCH 18/19] OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc attribute

[Brijesh Singh]

On 3/24/21 10:32 AM, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
>
> The MemEncryptSev{Set,Clear}PageEncMask() functions are used to set or
> clear the memory encryption attribute in the page table. When SEV-SNP is
> active, we also need to validate or invalidate the pages and update the
> RMP entry.
>
> Before clearing the encryption attribute we need to invalidate the memory,
> and then make the page shared in the RMP entry. Similarly, after setting
> the encryption attribute in the page table, we add the memory as private
> in the RMP entry and validate it.
>
> Cc: James Bottomley 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Cc: Tom Lendacky 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Signed-off-by: Brijesh Singh 
> ---
>  OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf |  3 ++
>  OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf |  1 +
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c | 32 
> +
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c   | 36 
> ++--
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h   | 27 
> +++
>  5 files changed, 97 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf 
> b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> index f2e162d680..fa8f7719a7 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
> @@ -36,6 +36,8 @@
>  [Sources.X64]
>X64/MemEncryptSevLib.c
>X64/PeiDxeVirtualMemory.c
> +  X64/SnpPageStateChangeInternal.c
> +  X64/PeiDxeSnpSetPageState.c
>X64/VirtualMemory.c
>X64/VirtualMemory.h
>  
> @@ -49,6 +51,7 @@
>DebugLib
>MemoryAllocationLib
>PcdLib
> +  VmgExitLib
>  
>  [FeaturePcd]
>gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf 
> b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> index cb9dd2bb21..d16ec44954 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
> @@ -37,6 +37,7 @@
>  [Sources.X64]
>X64/MemEncryptSevLib.c
>X64/PeiDxeVirtualMemory.c
> +  X64/PeiDxeSnpSetPageState.c
>X64/PeiSnpSystemRamValidate.c
>X64/SnpPageStateTrack.c
>X64/SnpPageStateChangeInternal.c
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c 
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
> new file mode 100644
> index 00..0a3d58ac22
> --- /dev/null
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
> @@ -0,0 +1,32 @@
> +/** @file
> +
> +  SEV-SNP Page Validation functions.
> +
> +  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include 
> +#include 
> +
> +#include "PeiSnpPageStateChange.h"


There is one minor error in this hunk, Please ignore it during your
reviews. I will fix it in next version.

The include should be "../SnpPageStateChange.h" and not
"PeiSnppageStateChange.h".


> +
> +VOID
> +SnpSetMemoryPrivate (
> +  IN  PHYSICAL_ADDRESS  PhysicalAddress,
> +  IN  UINTN Length
> +  )
> +{
> +  SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), 
> SevSnpPagePrivate, FALSE);
> +}
> +
> +VOID
> +SnpSetMemoryShared (
> +  IN  PHYSICAL_ADDRESS  PhysicalAddress,
> +  IN  UINTN Length
> +  )
> +{
> +  SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), 
> SevSnpPageShared, FALSE);
> +}
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c 
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> index 33d9bafe9f..26d363d427 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -17,6 +17,7 @@
>  #include 
>  
>  #include "VirtualMemory.h"
> +#include "SnpSetPageState.h"
>  
>  STATIC BOOLEAN mAddressEncMaskChecked = FALSE;
>  STATIC UINT64  mAddressEncMask;
> @@ -700,22 +701,34 @@ SetMemoryEncDec (
>UINT64 AddressEncMask;
>BOOLEANIsWpEnabled;
>RETURN_STATUS  Status;
> +  BOOLEANNeedPageStateChange;
> +  PHYSICAL_ADDRESS   OrigPhysicalAddress;
> +  UINTN  OrigLength;
>  
>//
>// Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
>//
>PageMapLevel4Entry = NULL;
>  
> +  //
> +  // When SEV-SNP is active, before clearing the encryption attribute from
> +  // the page table we also need to update the RMP entry for the memory
> +  // region to make the region shared. And after setting th

Re: [edk2-devel] [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS

[Jeremy Linton]

Hi,

On 3/24/21 4:45 AM, Sunny Wang wrote:

Merge changes in edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
Bluetooth and serial port (Mini UART) in Windows OS.

Testing Done:
   - Successfully booted Windows 10 (20279.1) on SD (made by WOR) with
 the RPi-Windows-Drivers release ver 0.5 downloaded from
 https://github.com/worproject/RPi-Windows-Drivers/releases
 and checked that both Bluetooth and serial port (Mini UART) can
 work fine.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Pete Batard 
Cc: Ard Biesheuvel 
Signed-off-by: Sunny Wang 
---
  Platform/RaspberryPi/AcpiTables/Uart.asl | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl 
b/Platform/RaspberryPi/AcpiTables/Uart.asl
index 8ce297078d..e3165911a6 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -30,6 +30,12 @@ Device (URT0)
{
  MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
  Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { 
BCM2836_PL011_UART_INTERRUPT }
+
+PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 32, 33 }
+
+// fake the CTS signal as we don't support HW flow control yet
+// BCM_ALT2 is set as output (low) by default
+PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 31 }
})
Method (_CRS, 0x0, Serialized)
{
@@ -142,7 +148,7 @@ Device(BTH0)
//
// RPIQ connection for BT_ON/OFF
//
-  GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, 
ResourceConsumer, , ) { 128 }
+  //GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, 
ResourceConsumer, , ) { 128 }
  })
  Return (RBUF)
}



Having come to this a bit late, its always made me a bit uncomfortable 
that some of these tables are doing blanket power/etc configuration in 
_CRS, is there are reason for continuing this rather than doing this via 
power states?





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73238): https://edk2.groups.io/g/devel/message/73238
Mute This Topic: https://groups.io/mt/81572764/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/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

[Jeremy Linton]

Hi,

Thanks for working on this!

Comments inline:


On 3/24/21 4:45 AM, Sunny Wang wrote:

Changes:
   1. Add code to ConfigDxe driver and AcpiTables module to dynamically
  build either Mini UART or PL011 UART info in ACPI. This fixes the
  issue discussed in https://github.com/pftf/RPi4/issues/118.
   2. Cleanup by moving duplicate Debug Port 2 table related defines and
  structures to a newly created header file (RpiDebugPort2Table.h).

Testing Done:
   - Booted to UEFI shell and use acpiview command to check the result of
 the different UART settings in config.txt (enabling either Mini UART
 or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
 changed as expected.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Pete Batard 
Cc: Ard Biesheuvel 
Signed-off-by: Sunny Wang 
---
  .../RaspberryPi/AcpiTables/AcpiTables.inf |   9 +-
  .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 
  .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 -
  .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +
  .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +-
  Platform/RaspberryPi/AcpiTables/Uart.asl  |  10 +-
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +-
  .../IndustryStandard/RpiDebugPort2Table.h |  34 
  8 files changed, 497 insertions(+), 212 deletions(-)
  create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
  rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
  create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
  rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
  create mode 100644 
Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d3363a76a1..fc8e927138 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
  #
  #  ACPI table data and ASL sources required to boot the platform.
  #
-#  Copyright (c) 2019, ARM Limited. All rights reserved.
+#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
  #  Copyright (c) 2017, Andrey Warkentin 
  #  Copyright (c) Microsoft Corporation. All rights reserved.
  #
@@ -28,12 +28,14 @@
Emmc.asl
Madt.aslc
Fadt.aslc
-  Dbg2.aslc
+  Dbg2MiniUart.aslc
+  Dbg2Pl011.aslc
Gtdt.aslc
Iort.aslc
Dsdt.asl
Csrt.aslc
-  Spcr.aslc
+  SpcrMiniUart.aslc
+  SpcrPl011.aslc
Pptt.aslc
SsdtThermal.asl

@@ -71,3 +73,4 @@

  [BuildOptions]
GCC:*_*_*_ASL_FLAGS   = -vw3133 -vw3150
+
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc 
b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
new file mode 100644
index 00..c83b978731
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
@@ -0,0 +1,82 @@
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard 
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "AcpiTables.h"
+
+#pragma pack(1)
+
+#define RPI_UART_INTERFACE_TYPE 
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS   
BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_LENGTH 
BCM2836_MINI_UART_LENGTH
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR{ '\\', '_', 'S', 'B', 
'.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
+
+#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, 
UartNameStr) {\
+{  
   \
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION, /* UINT8 
Revision */\
+  sizeof (DBG2_DEBUG_DEVICE_INFORMATION), /* 
UINT16Length */  \
+  NumReg, /* UINT8 
NumberofGenericAddressRegisters */ \
+  RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,/* 
UINT16NameSpaceStringLength */   \
+  OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString), /* 
UINT16NameSpaceStringOffset */   \
+  0,  /* 
UINT16OemDataLength */   \
+  0,  /* 
UINT16OemDataOffset */   \
+  EFI_ACPI_DBG2_PORT_TYPE_SERIAL, /* 
UINT16Port Type */   \
+  SubType

Re: [edk2-devel] [RFC PATCH 00/19] Add AMD Secure Nested Paging (SEV-SNP) support

[Laszlo Ersek]

On 03/24/21 16:31, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
> new hardware-based memory protections. SEV-SNP adds strong memory integrity
> protection to help prevent malicious hypervisor-based attacks like data
> replay, memory re-mapping and more in order to create an isolated memory
> encryption environment.
>  
> This series provides the basic building blocks to support booting the SEV-SNP
> VMs, it does not cover all the security enhancement introduced by the SEV-SNP
> such as interrupt protection.

Thanks, Brijesh. I'm adding the series to my review queue. Due to some
PTO coming up, I'll probably start reviewing this work only in April.
Other reviewers, please feel free to have at it.

Cheers
Laszlo

> 
> Many of the integrity guarantees of SEV-SNP are enforced through a new
> structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
> VM requires a 2-step process. First, the hypervisor assigns a page to the
> guest using the new RMPUPDATE instruction. This transitions the page to
> guest-invalid. Second, the guest validates the page using the new PVALIDATE
> instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
> defined in the GHCB specification to ask hypervisor to add or remove page
> from the RMP table.
>  
> Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
> as indicated by the Validated flag in the page's RMP entry. There are two
> approaches that can be taken for the page validation: Pre-validation and
> Lazy Validation.
>   
> Under pre-validation, the pages are validated prior to first use. And under
> lazy validation, pages are validated when first accessed. An access to a
> unvalidated page results in a #VC exception, at which time the exception
> handler may validate the page. Lazy validation requires careful tracking of
> the validated pages to avoid validating the same GPA more than once. The
> recently introduced "Unaccepted" memory type can be used to communicate the
> unvalidated memory ranges to the Guest OS.
> 
> At this time we only support the pre-validation. OVMF detects all the 
> available
> system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
> before it is made available to the EDK2 core.
> 
> This series does not implements the following SEV-SNP features yet:
> 
> * CPUID filtering
> * AP bring up using the new SEV-SNP NAE
> * Lazy validation
> * Interrupt security
> 
> The series is based on commit:
> e542e05d4f UefiCpuPkg/SmmCpuFeaturesLib: Abstract 
> PcdCpuMaxLogicalProcessorNumber
> 
> Additional resources
> -
> SEV-SNP whitepaper
> https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf
>  
> APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)
> 
> The complete source is available at
> https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-1
> 
> GHCB spec v2:
>   The draft specification is posted on AMD-SEV-SNP mailing list:
>https://lists.suse.com/mailman/private/amd-sev-snp/
> 
>   Copy of the spec is also available at 
>   
> https://github.com/AMDESE/AMDSEV/blob/sev-snp-devel/docs/56421-Guest_Hypervisor_Communication_Block_Standardization.pdf
> 
> GHCB spec v1:
> SEV-SNP firmware specification:
>  https://developer.amd.com/sev/
>   
> Cc: James Bottomley 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Cc: Tom Lendacky 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> 
> Brijesh Singh (19):
>   OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest
>   OvmfPkg: validate the data pages used in the SEC phase
>   MdePkg: Expand the SEV MSR to include the SNP definition
>   OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
>   MdePkg: Define the GHCB GPA structure
>   UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
> enabled
>   OvmfPkg: Add a library to support registering GHCB GPA
>   OvmfPkg: register GHCB gpa for the SEV-SNP guest
>   MdePkg: Add AsmPvalidate() support
>   OvmfPkg: Define the Page State Change VMGEXIT structures
>   OvmfPkg/ResetVector: Invalidate the GHCB page
>   OvmfPkg/MemEncryptSevLib: Add support to validate system RAM
>   OvmfPkg/SecMain: Validate the data/code pages used for the PEI phase
>   OvmfPkg/MemEncryptSevLib: Add support to validate RAM in PEI phase
>   OvmfPkg/PlatformPei: Validate the system RAM when SNP is active
>   OvmfPkg/MemEncryptSevLib: Add support to validate > 4GB memory in PEI
> phase
>   OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase
>   OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc
> attribute
>   OvmfPkg/MemEncryptSevLib: Skip page state change for non RAM region
> 
>  MdePkg/Include/Library/BaseLib.h  |  37 +++
>  MdePkg/Include/Register/Amd/Fam17Msr.h|  31 ++-
>  MdePkg/Include/Register/Amd/Ghcb.h   

[edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

[Brijesh Singh]

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

During the SEV-SNP guest launch sequence, two special pages need to
be inserted, the secrets page and cpuid page. The secrets page,
contain the VM platform communication keys. The guest BIOS and OS
can use this key to communicate with the SEV firmware to get the
attestation report. The Cpuid page, contain the CPUIDs entries
filtered through the AMD-SEV firmware.

The VMM will locate the secrets and cpuid page addresses through a
fixed GUID and pass them to SEV firmware to populate further.
For more information about the page content, see the SEV-SNP spec.

To simplify the pre-validation range calculation in the next patch,
the CPUID and Secrets pages are moved to the start of the
MEMFD_BASE_ADDRESS.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/OvmfPkg.dec  |  8 +++
 OvmfPkg/OvmfPkgX64.fdf   | 24 
 OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 19 
 OvmfPkg/ResetVector/ResetVector.inf  |  4 
 OvmfPkg/ResetVector/ResetVector.nasmb|  2 ++
 5 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4348bb45c6..062926772d 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -317,6 +317,14 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
 
+  ## The base address of the CPUID page used by SEV-SNP
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|0|UINT32|0x48
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize|0|UINT32|0x49
+
+  ## The base address of the Secrets page used by SEV-SNP
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|0|UINT32|0x50
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize|0|UINT32|0x51
+
 [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index d519f85328..ea214600be 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -67,27 +67,33 @@ ErasePolarity = 1
 BlockSize = 0x1
 NumBlocks = 0xD0
 
-0x00|0x006000
+0x00|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize
+
+0x001000|0x001000
+gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+
+0x002000|0x006000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
 
-0x006000|0x001000
+0x008000|0x001000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
 
-0x007000|0x001000
+0x009000|0x001000
 
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
 
-0x008000|0x001000
+0x00A000|0x001000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
 
-0x009000|0x002000
+0x00B000|0x002000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
 
-0x00B000|0x001000
-gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
-
-0x00C000|0x001000
+0x00D000|0x001000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
 
+0x00F000|0x001000
+gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
+
 0x01|0x01
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
 
diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm 
b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
index 9c0b5853a4..5456f02924 100644
--- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
+++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
@@ -47,6 +47,25 @@ TIMES (15 - ((guidedStructureEnd - guidedStructureStart + 
15) % 16)) DB 0
 ;
 guidedStructureStart:
 
+;
+; SEV-SNP boot support
+;
+; sevSnpBlock:
+;   For the initial boot of SEV-SNP guest, a Secrets and CPUID page must be
+;   reserved by the BIOS at a RAM area defined by SEV_SNP_SECRETS_PAGE
+;   and SEV_SNP_CPUID_PAGE. A VMM will locate this information using the
+;   SEV-SNP boot block.
+;
+; GUID (SEV-SNP boot block): bd39c0c2-2f8e-4243-83e8-1b74cebcb7d9
+;
+sevSnpBootBlockStart:
+DD  SEV_SNP_SECRETS_PAGE
+DD  SEV_SNP_CPUID_PAGE
+DW  sevSnpBootBlockEnd - sevSnpBootBlockStart
+DB  0xC2, 0xC0, 0x39, 0xBD, 0x8e, 0x2F, 0x43, 0x42
+DB  0x83, 0xE8, 0x1B, 0x74, 0xCE, 0xBC, 0xB7, 0xD9
+sevSnpBootBlockEnd:
+
 ;
 ; SEV Secret block
 ;
diff --git a/OvmfPkg/ResetVector/ResetVector.inf 
b/OvmfPkg/ResetVector/ResetVector.inf
i

[edk2-devel] [RFC PATCH 03/19] MdePkg: Expand the SEV MSR to include the SNP definition

[Brijesh Singh]

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

Define the SEV-SNP MSR bits.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 MdePkg/Include/Register/Amd/Fam17Msr.h | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h 
b/MdePkg/Include/Register/Amd/Fam17Msr.h
index e4db09c518..4d33bef220 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -87,7 +87,12 @@ typedef union {
 ///
 UINT32  SevEsBit:1;
 
-UINT32  Reserved:30;
+///
+/// [Bit 2] Secure Nested Paging (SevSnp) is enabled
+///
+UINT32  SevSnpBit:1;
+
+UINT32  Reserved:29;
   } Bits;
   ///
   /// All bit fields as a 32-bit value
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 13/19] OvmfPkg/SecMain: Validate the data/code pages used for the PEI phase

[Brijesh Singh]

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

The VMM launch sequence should have validated all the data pages used
in the SEC phase. Before decompressing the firmware volume, validate
the data/code pages used during the decompression steps, and any other
pages used during the PEI phase entry.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Sec/SecMain.c   | 26 
 OvmfPkg/Sec/SecMain.inf |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index df6722b546..b491810376 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -351,6 +351,32 @@ DecompressMemFvs (
 return Status;
   }
 
+  if (MemEncryptSevSnpIsEnabled ()) {
+EFI_PHYSICAL_ADDRESSLaunchValidatedBase, LaunchValidatedEnd;
+UINTN   Size;
+
+//
+// The VMM launch sequence should have validated the memory range from
+// MEMFD_BASE_ADDRESS to PcdOvmfPeiMemFvBase. The PCD values are also
+// accessible through PcdOvmfSnpLaunchValidatedStart, and 
PcdOvmfSnpLaunchValidatedEnd.
+// The pre-validation was sufficent to access the data pages used in the 
SEC
+// phase.
+//
+// Now that we are getting ready to decompress firmware volumes, and enter
+// to PEI phase. Lets validate the code/data pages used for entering to the
+// PEI phase.
+//
+// See FvmainCompactScratchEnd.fdf.inc for more detail.
+//
+LaunchValidatedBase =
+(EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 
(PcdOvmfSnpLaunchValidatedStart);
+LaunchValidatedEnd = LaunchValidatedBase +
+(EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfSnpLaunchValidatedEnd);
+Size = PcdGet32 (PcdOvmfDecompressionScratchEnd) - LaunchValidatedEnd;
+
+MemEncryptSevSnpValidateSystemRam (LaunchValidatedEnd, EFI_SIZE_TO_PAGES 
(Size));
+  }
+
   Status = ExtractGuidedSectionGetInfo (
  Section,
  &OutputBufferSize,
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 7f78dcee27..207accb53c 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -70,6 +70,8 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpLaunchValidatedStart
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpLaunchValidatedEnd
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 19/19] OvmfPkg/MemEncryptSevLib: Skip page state change for non RAM region

[Brijesh Singh]

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

The PEI phases uses MemEncryptSevSnpValidateSystemRam() to validate
the system RAM. MemEncryptSev{Set,Clear}PageEncMask() is
later used by libraries/drivers to change the page state from private
to shared and vice versa.

The AmdSevDxe driver calls the MemEncryptSevClearPageEncMask() to remove
the encryption attribute from the reserved and non-existent memory regions.
Those regions where not part of system RAM, and was not pre-validated
during PEI phase, so we fail to change the page state for it.

There are multiple approaches to fix it:

1) Add a new parameter to MemEncryptSevClearPageEncMask(), that should be
   set by the caller to indicate whether the region is RAM.
OR
2) Lookup the address in the interval tree maintained by the
   MemEncryptSevSnpValidateSystemRam() to determine whether we validated
   the page in the past.
OR
3) Iterate through the Memory space GCD to calculate if the address range
   is RAM.

For now, we have chosen #2, it does not require a changes to the
caller of MemEncryptSevClearPageEncMask() and lookup routine is
already available.

Extend the SEV-ES workarea to pass the interval tree root pointer
so that we can perform the lookup later. If the specified address
was not present in the tree, then do not invalidate the page as it
could result in page state change failure.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h |  3 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf   |  4 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c   | 31 
+++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c | 42 

 4 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h 
b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 47d6802b61..712590b64d 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -54,6 +54,9 @@ typedef struct _SEC_SEV_ES_WORK_AREA {
   UINT64   RandomData;
 
   UINT64   EncryptionMask;
+
+  UINT64   SnpSystemRamValidatedRootAddress;
+
 } SEC_SEV_ES_WORK_AREA;
 
 //
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index fa8f7719a7..43b842254f 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -38,6 +38,7 @@
   X64/PeiDxeVirtualMemory.c
   X64/SnpPageStateChangeInternal.c
   X64/PeiDxeSnpSetPageState.c
+  X64/SnpPageStateTrack.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
@@ -58,3 +59,6 @@
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
+
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
index 0a3d58ac22..9fe6831368 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
@@ -10,8 +10,12 @@
 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #include "PeiSnpPageStateChange.h"
+#include "SnpPageStateTrack.h"
 
 VOID
 SnpSetMemoryPrivate (
@@ -28,5 +32,32 @@ SnpSetMemoryShared (
   IN  UINTN Length
   )
 {
+  SEC_SEV_ES_WORK_AREA   *SevEsWorkArea;
+  SNP_VALIDATED_RANGE*RootNode, *Range;
+
+  //
+  // Get the Page State tracker root node. The information will be used to 
lookup
+  // the address in the page state tracker.
+  //
+  SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 
(PcdSevEsWorkAreaBase);
+  RootNode = (SNP_VALIDATED_RANGE *) 
SevEsWorkArea->SnpSystemRamValidatedRootAddress;
+
+  //
+  // Check if the region is validated during the System RAM validation process.
+  // If region is not validated then do nothing. This typically will happen if
+  // we are getting called to make the page state change for the MMIO region.
+  // The MMIO regions fall within reserved memory type and does not require
+  // page state changes.
+  //
+  Range = FindOverlapRange (RootNode, PhysicalAddress, PhysicalAddress + 
Length);
+  if (Range == NULL) {
+DEBUG ((EFI_D_INFO, "%a:%a %Lx - %Lx is not RAM, skipping it.\n",
+  gEfiCallerBaseName,
+  __FUNCTION__,
+  PhysicalAddress,
+  PhysicalAddress + Length));
+return;
+  }
+
   SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), 
SevSnpPageShared, FALSE);
 }
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
index 41bf301efe..2e049d3df7 100644
--- a/OvmfPkg/Library/Base

[edk2-devel] [RFC PATCH 16/19] OvmfPkg/MemEncryptSevLib: Add support to validate > 4GB memory in PEI phase

[Brijesh Singh]

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

The initial page built during the SEC phase is used by the
MemEncryptSevSnpValidateSystemRam() for the system RAM validation. The
page validation process requires using the PVALIDATE instruction;  the
instruction accepts a virtual address of the memory region that needs
to be validated. If hardware encounters a page table walk failure
(due to page-not-present) then it raises #GP.

The initial page table built in SEC phase address up to 4GB. Add an
internal function to extend the page table to cover > 4GB. The function
builds 1GB entries in the page table for access > 4GB. This will provide
the support to call PVALIDATE instruction for the virtual address >
4GB in PEI phase.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 115 

 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c |  16 +++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h   |  19 
 3 files changed, 150 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index d3455e812b..33d9bafe9f 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -536,6 +536,121 @@ EnableReadOnlyPageWriteProtect (
   AsmWriteCr0 (AsmReadCr0() | BIT16);
 }
 
+RETURN_STATUS
+EFIAPI
+InternalMemEncryptSevCreateIdentityMap1G (
+  INPHYSICAL_ADDRESS  Cr3BaseAddress,
+  INPHYSICAL_ADDRESS  PhysicalAddress,
+  INUINTN Length
+  )
+{
+  PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
+  PAGE_TABLE_1G_ENTRY*PageDirectory1GEntry;
+  UINT64 PgTableMask;
+  UINT64 AddressEncMask;
+  BOOLEANIsWpEnabled;
+  RETURN_STATUS  Status;
+
+  //
+  // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
+  //
+  PageMapLevel4Entry = NULL;
+
+  DEBUG ((
+DEBUG_VERBOSE,
+"%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx\n",
+gEfiCallerBaseName,
+__FUNCTION__,
+Cr3BaseAddress,
+PhysicalAddress,
+(UINT64)Length
+));
+
+  if (Length == 0) {
+return RETURN_INVALID_PARAMETER;
+  }
+
+  //
+  // Check if we have a valid memory encryption mask
+  //
+  AddressEncMask = InternalGetMemEncryptionAddressMask ();
+  if (!AddressEncMask) {
+return RETURN_ACCESS_DENIED;
+  }
+
+  PgTableMask = AddressEncMask | EFI_PAGE_MASK;
+
+
+  //
+  // Make sure that the page table is changeable.
+  //
+  IsWpEnabled = IsReadOnlyPageWriteProtected ();
+  if (IsWpEnabled) {
+DisableReadOnlyPageWriteProtect ();
+  }
+
+  Status = EFI_SUCCESS;
+
+  while (Length)
+  {
+//
+// If Cr3BaseAddress is not specified then read the current CR3
+//
+if (Cr3BaseAddress == 0) {
+  Cr3BaseAddress = AsmReadCr3();
+}
+
+PageMapLevel4Entry = (VOID*) (Cr3BaseAddress & ~PgTableMask);
+PageMapLevel4Entry += PML4_OFFSET(PhysicalAddress);
+if (!PageMapLevel4Entry->Bits.Present) {
+  DEBUG ((
+DEBUG_ERROR,
+"%a:%a: bad PML4 for Physical=0x%Lx\n",
+gEfiCallerBaseName,
+__FUNCTION__,
+PhysicalAddress
+));
+  Status = RETURN_NO_MAPPING;
+  goto Done;
+}
+
+PageDirectory1GEntry = (VOID *)(
+ (PageMapLevel4Entry->Bits.PageTableBaseAddress <<
+  12) & ~PgTableMask
+ );
+PageDirectory1GEntry += PDP_OFFSET(PhysicalAddress);
+if (!PageDirectory1GEntry->Bits.Present) {
+  PageDirectory1GEntry->Bits.Present = 1;
+  PageDirectory1GEntry->Bits.MustBe1 = 1;
+  PageDirectory1GEntry->Bits.MustBeZero = 0;
+  PageDirectory1GEntry->Bits.ReadWrite = 1;
+  PageDirectory1GEntry->Uint64 |= (UINT64)PhysicalAddress | AddressEncMask;
+}
+
+if (Length <= BIT30) {
+  Length = 0;
+} else {
+  Length -= BIT30;
+}
+
+PhysicalAddress += BIT30;
+  }
+
+  //
+  // Flush TLB
+  //
+  CpuFlushTlb();
+
+Done:
+  //
+  // Restore page table write protection, if any.
+  //
+  if (IsWpEnabled) {
+EnableReadOnlyPageWriteProtect ();
+  }
+
+  return Status;
+}
 
 /**
   This function either sets or clears memory encryption bit for the memory
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
index ce8a05bb1f..41bf301efe 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -16,6 +16,7 @@
 
 #include "../SnpPageStateChange.h"
 #include "SnpPageStateTrack.h"
+#i

[edk2-devel] [RFC PATCH 11/19] OvmfPkg/ResetVector: Invalidate the GHCB page

[Brijesh Singh]

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

When SEV-SNP is active, the GHCB page is mapped un-encrypted in the
initial page table built by the reset vector code. Just clearing the
encryption attribute from the page table is not enough. The page also
needs to be added as shared in the RMP table.

The GHCB page was part of the pre-validated memory range specified
through the SnpBootBlock GUID. To maintain the security guarantees,
we must invalidate the GHCB page before clearing the encryption
attribute from the page table, and add the page shared in the RMP
table.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 106 
 1 file changed, 106 insertions(+)

diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm 
b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 5fae8986d9..63b864bcf2 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -62,6 +62,99 @@ BITS32
 %define GHCB_CPUID_REGISTER_SHIFT  30
 %define CPUID_INSN_LEN  2
 
+; GHCB Page Invalidate request and response protocol values
+;
+%define GHCB_PAGE_STATE_CHANGE_REQUEST20
+%define GHCB_PAGE_STATE_CHANGE_RESPONSE   21
+%define   GHCB_PAGE_STATE_SHARED  2
+
+; GHCB request to terminate protocol values
+%define GHCB_GENERAL_TERMINATE_REQUEST255
+
+;
+; If SEV-SNP is enabled, invalidate the GHCB page
+InvalidateGHCBPage:
+; Read the SEV_STATUS MSR to check whether SEV-SNP is enabled.
+;  MSR_0xC0010131 - Bit 2 (SEV-SNP enabled)
+mov ecx, 0xc0010131
+rdmsr
+bt  eax, 2
+jnc InvalidateGHCBPageDone
+
+; Use PVALIDATE instruction to invalidate the page
+mov eax, GHCB_BASE
+mov ecx, 0
+mov edx, 0
+DB  0xF2, 0x0F, 0x01, 0xFF
+cmp eax, 0
+jnz ValidationFailure
+
+; Ask hypervisor to change the page state to shared using the
+; Page State Change VMGEXIT.
+;
+; Setup GHCB MSR
+;   GHCB_MSR[55:52] = Page Operation
+;   GHCB_MSR[51:12] = Guest Physical Frame Number
+;   GHCB_MSR[11:0]  = Page State Change Request
+;
+mov eax, (GHCB_BASE >> 12)
+shl eax, 12
+or  eax, GHCB_PAGE_STATE_CHANGE_REQUEST
+mov edx, (GHCB_PAGE_STATE_SHARED << 20)
+mov ecx, 0xc0010130
+wrmsr
+
+;
+; Issue VMGEXIT - NASM doesn't support the vmmcall instruction in 32-bit
+; mode, so work around this by temporarily switching to 64-bit mode.
+;
+BITS64
+rep vmmcall
+BITS32
+
+;
+; Response GHCB MSR
+;   GHCB_MSR[51:12] = Guest Physical Frame Number
+;   GHCB_MSR[11:0]  = Page State Change Response
+;
+mov ecx, 0xc0010130
+rdmsr
+and eax, 0xfff
+cmp eax, GHCB_PAGE_STATE_CHANGE_RESPONSE
+jnz ValidationFailure
+cmp edx, 0
+jnz ValidationFailure
+
+jmp InvalidateGHCBPageDone
+
+ValidationFailure:
+;
+; Setup GHCB MSR
+;   GHCB_MSR[23:16] = 0
+;   GHCB_MSR[15:12] = 0
+;   GHCB_MSR[11:0]  = Terminate Request
+;
+mov edx, 0
+mov eax, GHCB_GENERAL_TERMINATE_REQUEST
+mov ecx, 0xc0010130
+wrmsr
+
+;
+; Issue VMGEXIT - NASM doesn't support the vmmcall instruction in 32-bit
+; mode, so work around this by temporarily switching to 64-bit mode.
+;
+BITS64
+rep vmmcall
+BITS32
+
+SnpPageStateFailureHlt:
+cli
+hlt
+jmp SnpPageStateFailureHlt
+
+InvalidateGHCBPageDone:
+OneTimeCallRet InvalidateGHCBPage
+
 
 ; Check if Secure Encrypted Virtualization (SEV) features are enabled.
 ;
@@ -316,6 +409,19 @@ clearGhcbMemoryLoop:
 mov dword[ecx * 4 + GHCB_BASE - 4], eax
 loopclearGhcbMemoryLoop
 
+;
+; The page table built above cleared the memory encryption mask from the
+; GHCB_BASE (aka made it shared). When SEV-SNP is enabled, to maintain
+; the security guarantees, the page state transition from private to
+; shared must go through the page invalidation steps. Invalidate the
+; memory range before loading the page table below.
+;
+; NOTE: the invalidation must happen after zeroing the GHCB memory. This
+;   is because, in the 32-bit mode all the access are considered 
private.
+;   The invalidation before the zero'ing will cause a #VC.
+;
+OneTimeCall  InvalidateGHCBPage
+
 SetCr3:
 ;
 ; Set CR3 now that the paging structures are available
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 09/19] MdePkg: Add AsmPvalidate() support

[Brijesh Singh]

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

The PVALIDATE instruction validates or rescinds validation of a guest
page RMP entry. Upon completion, a return code is stored in EAX, rFLAGS
bits OF, ZF, AF, PF and SF are set based on this return code. If the
instruction completed succesfully, the rFLAGS bit CF indicates if the
contents of the RMP entry were changed or not.

For more information about the instruction see AMD APM volume 3.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 MdePkg/Include/Library/BaseLib.h  | 37 +
 MdePkg/Library/BaseLib/BaseLib.inf|  1 +
 MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 43 
 3 files changed, 81 insertions(+)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 1171a0ffb5..fee27e9a1b 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -7495,5 +7495,42 @@ PatchInstructionX86 (
   IN  UINTNValueSize
   );
 
+/**
+ Execute a PVALIDATE instruction to validate or rescnids validation of a guest
+ page's RMP entry.
+
+ Upon completion, in addition to the return value the instruction also updates
+ the eFlags. A caller must check both the return code as well as eFlags to
+ determine if the RMP entry has been updated.
+
+ The function is available on x64.
+
+ @param[in]AddressThe guest virtual address to validate.
+ @param[in]PageSize   The page size to use.
+ @param[i] Validate   Validate or rescinds.
+ @param[out]   Eflags The value of Eflags after PVALIDATE completion.
+
+ @retval   PvalidateRetValue  The return value from the PVALIDATE 
instruction.
+**/
+typedef enum {
+  PVALIDATE_PAGE_SIZE_4K = 0,
+  PVALIDATE_PAGE_SIZE_2M,
+} PvalidatePageSize;
+
+typedef enum {
+  PVALIDATE_RET_SUCCESS = 0,
+  PVALIDATE_RET_FAIL_INPUT = 1,
+  PVALIDATE_RET_FAIL_SIZEMISMATCH = 6,
+} PvalidateRetValue;
+
+PvalidateRetValue
+EFIAPI
+AsmPvalidate (
+  IN   PvalidatePageSize   PageSize,
+  IN   BOOLEAN Validate,
+  IN   UINTN   Address,
+  OUT  IA32_EFLAGS32   *Eflags
+  );
+
 #endif // defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
 #endif // !defined (__BASE_LIB__)
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
b/MdePkg/Library/BaseLib/BaseLib.inf
index 3b85c56c3c..01aa5cc7a4 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -319,6 +319,7 @@
   X64/RdRand.nasm
   X64/XGetBv.nasm
   X64/VmgExit.nasm
+  X64/Pvalidate.nasm
   ChkStkGcc.c  | GCC
 
 [Sources.EBC]
diff --git a/MdePkg/Library/BaseLib/X64/Pvalidate.nasm 
b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
new file mode 100644
index 00..f2aba114ac
--- /dev/null
+++ b/MdePkg/Library/BaseLib/X64/Pvalidate.nasm
@@ -0,0 +1,43 @@
+;-
+;
+; Copyright (c) 2020-2021, AMD. All rights reserved.
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+;   Pvalidate.Asm
+;
+; Abstract:
+;
+;   AsmPvalidate function
+;
+; Notes:
+;
+;-
+
+SECTION .text
+
+;-
+;  PvalidateRetValue
+;  EFIAPI
+;  AsmPvalidate (
+;IN   UINT32  RmpPageSize
+;IN   UINT32  Validate,
+;IN   UINTN   Address,
+;OUT  UINTN  *Eflags,
+;)
+;-
+global ASM_PFX(AsmPvalidate)
+ASM_PFX(AsmPvalidate):
+  mov rax, r8
+
+  ; PVALIDATE instruction opcode
+  DB  0xF2, 0x0F, 0x01, 0xFF
+
+  ; Read the Eflags
+  pushfq
+  pop r8
+  mov [r9], r8
+
+  ; The PVALIDATE instruction returns the status in rax register.
+  ret
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 18/19] OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc attribute

[Brijesh Singh]

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

The MemEncryptSev{Set,Clear}PageEncMask() functions are used to set or
clear the memory encryption attribute in the page table. When SEV-SNP is
active, we also need to validate or invalidate the pages and update the
RMP entry.

Before clearing the encryption attribute we need to invalidate the memory,
and then make the page shared in the RMP entry. Similarly, after setting
the encryption attribute in the page table, we add the memory as private
in the RMP entry and validate it.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf |  3 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf |  1 +
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c | 32 
+
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c   | 36 
++--
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h   | 27 
+++
 5 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index f2e162d680..fa8f7719a7 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -36,6 +36,8 @@
 [Sources.X64]
   X64/MemEncryptSevLib.c
   X64/PeiDxeVirtualMemory.c
+  X64/SnpPageStateChangeInternal.c
+  X64/PeiDxeSnpSetPageState.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
@@ -49,6 +51,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  VmgExitLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
index cb9dd2bb21..d16ec44954 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
@@ -37,6 +37,7 @@
 [Sources.X64]
   X64/MemEncryptSevLib.c
   X64/PeiDxeVirtualMemory.c
+  X64/PeiDxeSnpSetPageState.c
   X64/PeiSnpSystemRamValidate.c
   X64/SnpPageStateTrack.c
   X64/SnpPageStateChangeInternal.c
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
new file mode 100644
index 00..0a3d58ac22
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
@@ -0,0 +1,32 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+
+#include "PeiSnpPageStateChange.h"
+
+VOID
+SnpSetMemoryPrivate (
+  IN  PHYSICAL_ADDRESS  PhysicalAddress,
+  IN  UINTN Length
+  )
+{
+  SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), 
SevSnpPagePrivate, FALSE);
+}
+
+VOID
+SnpSetMemoryShared (
+  IN  PHYSICAL_ADDRESS  PhysicalAddress,
+  IN  UINTN Length
+  )
+{
+  SetPageStateInternal (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), 
SevSnpPageShared, FALSE);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index 33d9bafe9f..26d363d427 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -17,6 +17,7 @@
 #include 
 
 #include "VirtualMemory.h"
+#include "SnpSetPageState.h"
 
 STATIC BOOLEAN mAddressEncMaskChecked = FALSE;
 STATIC UINT64  mAddressEncMask;
@@ -700,22 +701,34 @@ SetMemoryEncDec (
   UINT64 AddressEncMask;
   BOOLEANIsWpEnabled;
   RETURN_STATUS  Status;
+  BOOLEANNeedPageStateChange;
+  PHYSICAL_ADDRESS   OrigPhysicalAddress;
+  UINTN  OrigLength;
 
   //
   // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings.
   //
   PageMapLevel4Entry = NULL;
 
+  //
+  // When SEV-SNP is active, before clearing the encryption attribute from
+  // the page table we also need to update the RMP entry for the memory
+  // region to make the region shared. And after setting the encryption
+  // attribute, the region must be made private in the RMP table.
+  //
+  NeedPageStateChange = MemEncryptSevSnpIsEnabled ();
+
   DEBUG ((
 DEBUG_VERBOSE,
-"%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a CacheFlush=%u\n",
+"%a:%a: Cr3Base=0x%Lx Physical=0x%Lx Length=0x%Lx Mode=%a CacheFlush=%u 
Rmpupdate=%u\n",
 gEfiCallerBaseName,
 __FUNCTION__,
 Cr3BaseAddress,
 PhysicalAddress,
 (UINT64)Length,
 (Mode == SetCBit) ? "Encrypt" : "Decrypt",
-  

[edk2-devel] [RFC PATCH 07/19] OvmfPkg: Add a library to support registering GHCB GPA

[Brijesh Singh]

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

An SEV-SNP guest us required to perform GHCB GPA registration before
using a GHCB. See the GHCB spec section 2.5.2 for more details.

Add a library that can be called to perform the GHCB GPA registration.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Include/Library/GhcbRegisterLib.h   | 27 ++
 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c   | 97 
 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf | 33 +++
 OvmfPkg/OvmfPkgX64.dsc  |  1 +
 4 files changed, 158 insertions(+)

diff --git a/OvmfPkg/Include/Library/GhcbRegisterLib.h 
b/OvmfPkg/Include/Library/GhcbRegisterLib.h
new file mode 100644
index 00..7d98b6eb36
--- /dev/null
+++ b/OvmfPkg/Include/Library/GhcbRegisterLib.h
@@ -0,0 +1,27 @@
+/** @file
+
+  Declarations of utility functions used for GHCB GPA registration.
+
+  Copyright (C) 2021, AMD Inc, All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _GHCB_REGISTER_LIB_H_
+#define _GHCB_REGISTER_LIB_H_
+
+/**
+
+  This function can be used to register the GHCB GPA.
+
+  @param[in]  Address   The physical address to registered.
+
+**/
+VOID
+EFIAPI
+GhcbRegister (
+  IN  EFI_PHYSICAL_ADDRESS   Address
+  );
+
+#endif // _GHCB_REGISTER_LIB_H_
diff --git a/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c 
b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
new file mode 100644
index 00..7fe0aad75a
--- /dev/null
+++ b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
@@ -0,0 +1,97 @@
+/** @file
+  GHCBRegister Support Library.
+
+  Copyright (C) 2021, Advanced Micro Devices, Inc. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Handle an SEV-SNP/GHCB protocol check failure.
+
+  Notify the hypervisor using the VMGEXIT instruction that the SEV-SNP guest
+  wishes to be terminated.
+
+  @param[in] ReasonCode  Reason code to provide to the hypervisor for the
+ termination request.
+
+**/
+STATIC
+VOID
+SevEsProtocolFailure (
+  IN UINT8  ReasonCode
+  )
+{
+  MSR_SEV_ES_GHCB_REGISTER  Msr;
+
+  //
+  // Use the GHCB MSR Protocol to request termination by the hypervisor
+  //
+  Msr.GhcbPhysicalAddress = 0;
+  Msr.GhcbTerminate.Function = GHCB_INFO_TERMINATE_REQUEST;
+  Msr.GhcbTerminate.ReasonCodeSet = GHCB_TERMINATE_GHCB;
+  Msr.GhcbTerminate.ReasonCode = ReasonCode;
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+  AsmVmgExit ();
+
+  ASSERT (FALSE);
+  CpuDeadLoop ();
+}
+
+/**
+
+  This function can be used to register the GHCB GPA.
+
+  @param[in]  Address   The physical address to be registered.
+
+**/
+VOID
+EFIAPI
+GhcbRegister (
+  IN  EFI_PHYSICAL_ADDRESS   Address
+  )
+{
+  MSR_SEV_ES_GHCB_REGISTER  Msr;
+  MSR_SEV_ES_GHCB_REGISTER  CurrentMsr;
+  EFI_PHYSICAL_ADDRESS  GuestFrameNumber;
+
+  GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
+
+  //
+  // Save the current MSR Value
+  //
+  CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // Use the GHCB MSR Protocol to request to register the GPA.
+  //
+  Msr.GhcbPhysicalAddress = 0;
+  Msr.GhcbGpaRegister.Function = GHCB_INFO_GHCB_GPA_REGISTER_REQUEST;
+  Msr.GhcbGpaRegister.GuestFrameNumber = GuestFrameNumber;
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+  AsmVmgExit ();
+
+  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // If hypervisor responded with a different GPA than requested then fail.
+  //
+  if ((Msr.GhcbGpaRegister.Function != GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE) ||
+  (Msr.GhcbGpaRegister.GuestFrameNumber != GuestFrameNumber)) {
+SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL);
+  }
+
+  //
+  // Restore the MSR
+  //
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
+}
diff --git a/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf 
b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
new file mode 100644
index 00..8cc39ef715
--- /dev/null
+++ b/OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf
@@ -0,0 +1,33 @@
+## @file
+#  GHCBRegisterLib Support Library.
+#
+#  Copyright (C) 2021, Advanced Micro Devices, Inc. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = GhcbRegisterLib
+  FILE_GUID  = 0e913c15-12cd-430b-8714-ffe85672a77b
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = GhcbRegisterLib
+
+#
+# The following information is for reference only and not required by the 
build tools.
+#
+#  VALID_ARCHITECTURES   = X64
+#
+
+[Sources.common]
+  GhcbRe

[edk2-devel] [RFC PATCH 02/19] OvmfPkg: validate the data pages used in the SEC phase

[Brijesh Singh]

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

An SEV-SNP guest requires that private memory (aka pages mapped as
encrypted) must be validated before being accessed.

The validation process consist of the following sequence:

1) Set the memory encryption attribute in the page table (aka C-bit).
   Note: If the processor is in non-PAE mode, then all the memory accesses
   are considered private.
2) Add the memory range as private in the RMP table. This can be performed
   using the Page State Change VMGEXIT defined in the GHCB specification.
3) Use the PVALIDATE instruction to set the Validated Bit in the RMP table.

During the guest creation time, the VMM encrypts the OVMF_CODE.fd using
the SEV-SNP firmware provided LAUNCH_UPDATE_DATA command. In addition to
encrypting the content, the command also validates the memory region.
This allows us to execute the code without going through the validation
sequence.

During execution, the reset vector need to access some data pages
(such as page tables, SevESWorkarea, Sec stack). The data pages are
accessed as private memory. The data pages are not part of the
OVMF_CODE.fd, so they were not validated during the guest creation.

There are two approaches we can take to validate the data pages before
the access:

a) Enhance the OVMF reset vector code to validate the pages as described
   above (go through step 2 - 3).
OR
b) Validate the pages during the guest creation time. The SEV firmware
   provides a command which can be used by the VMM to validate the pages
   without affecting the measurement of the launch.

Approach #b seems much simpler; it does not require any changes to the
OVMF reset vector code.

Extend the SnpBootBlock to provide the range that can be pre-validated
using the SEV-SNP firmware command during the guest creation time.

At the end of the guest creation the pre-validated range looks like this:

0x80   0x801000   (CPUID Page [Validated + Measured])
0x801000   0x802000   (Secrets Page   [Validated + Measured])
0x802000   0x82   (Data Pages for the SEC phase  [Validated + unmeasured])

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/OvmfPkg.dec  | 4 
 OvmfPkg/OvmfPkgX64.fdf   | 9 -
 OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 5 +
 OvmfPkg/ResetVector/ResetVector.inf  | 1 +
 OvmfPkg/ResetVector/ResetVector.nasmb| 2 ++
 5 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 062926772d..6fb70e2c10 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -325,6 +325,10 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|0|UINT32|0x50
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize|0|UINT32|0x51
 
+  ## The range of memory pre-validated through the SEV-SNP launch sequence
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpLaunchValidatedStart|0|UINT32|0x52
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpLaunchValidatedEnd|0|UINT32|0x53
+
 [PcdsDynamic, PcdsDynamicEx]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index ea214600be..16383453f1 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -105,7 +105,14 @@ FV = PEIFV
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = DXEFV
 
-
+##
+#
+# The range of the pages validated through the SEV-SNP launch sequence.
+#
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpLaunchValidatedStart = 
$(MEMFD_BASE_ADDRESS)
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpLaunchValidatedEnd = 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
+
+###
 
 [FV.SECFV]
 FvNameGuid = 763BED0D-DE9F-48F5-81F1-3E90E1B1A015
diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm 
b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
index 5456f02924..9be887c4fc 100644
--- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
+++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
@@ -56,11 +56,16 @@ guidedStructureStart:
 ;   and SEV_SNP_CPUID_PAGE. A VMM will locate this information using the
 ;   SEV-SNP boot block.
 ;
+;   In addition to Secret and CPUID page, the SEV-SNP boot block also contain
+;   the range of memory that must be pre-validated by the VMM before the 
execution.
+;
 ; GUID (SEV-SNP boot block): bd39c0c2-2f8e-4243-83e8-1b74cebcb7d9
 ;
 sevSnpBootBlockStart:
 DD  SEV_SNP_SECRETS_PAGE
 DD  SEV_SNP_CPUID_PAGE
+DD  SEV_SNP_LAUNCH_VALIDATED_START
+DD  SEV_SNP_LAUNCH_VALIDATED_END
 DW  sevSnpBootBl

[edk2-devel] [RFC PATCH 15/19] OvmfPkg/PlatformPei: Validate the system RAM when SNP is active

[Brijesh Singh]

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

When SEV-SNP is active, a memory region mapped encrypted in the page
table must be validated before access. There are two approaches that
can be taken to validate the system RAM detected during the PEI phase:

1) Validate on-demand
OR
2) Validate before access

On-demand
=
If memory is not validated before access, it will cause a #VC
exception with the page-not-validated error code. The VC exception
handler can perform the validation steps.

The pages that have been validated will need to be tracked to avoid
the double validation scenarios. The range of memory that has not
been validated will need to be communicated to the OS through the
recently introduced unaccepted memory type
https://github.com/microsoft/mu_basecore/pull/66, so that OS can
validate those ranges before using them.

Validate before access
==
Since the PEI phase detects all the available system RAM, use the
MemEncryptSevSnpValidateSystemRam() function to pre-validate the
system RAM in the PEI phase.

For now, we have chosen option 2 due to the dependency and the complexity
of the on-demand validation.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/PlatformPei/AmdSev.c | 41 
 1 file changed, 41 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 95c5ad235f..abbbef54c1 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -140,6 +140,42 @@ AmdSevEsInitialize (
   AsmWriteGdtr (&Gdtr);
 }
 
+/**
+
+  Initialize SEV-SNP support if running as an SEV-SNP guest.
+
+  **/
+STATIC
+VOID
+AmdSevSnpInitialize (
+  VOID
+  )
+{
+  EFI_PEI_HOB_POINTERS  Hob;
+  EFI_HOB_RESOURCE_DESCRIPTOR   *ResourceHob;
+
+  if (!MemEncryptSevSnpIsEnabled ()) {
+return;
+  }
+
+  DEBUG ((EFI_D_INFO, "SEV-SNP is enabled.\n"));
+
+  //
+  // Iterate through the system RAM and validate it.
+  //
+  for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB 
(Hob)) {
+if (Hob.Raw != NULL && GET_HOB_TYPE (Hob) == 
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
+  ResourceHob = Hob.ResourceDescriptor;
+
+  if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
+MemEncryptSevSnpValidateSystemRam (ResourceHob->PhysicalStart,
+   EFI_SIZE_TO_PAGES 
(ResourceHob->ResourceLength)
+  );
+  }
+}
+  }
+}
+
 /**
 
   Function checks if SEV support is available, if present then it sets
@@ -217,6 +253,11 @@ AmdSevInitialize (
 }
   }
 
+  //
+  // Check and perform SEV-SNP initialization if required.
+  //
+  AmdSevSnpInitialize ();
+
   //
   // Check and perform SEV-ES initialization if required.
   //
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 17/19] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase

[Brijesh Singh]

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

Commit 85b8eac59b8c5bd9c7eb9afdb64357ce1aa2e803 added support to ensure
that MMIO is only performed against the un-encrypted memory. If MMIO
is performed against encrypted memory, a #GP is raised.

The VmgExitLib library depends on ApicTimerLib to get the APIC base
address so that it can exclude the APIC range from the un-encrypted
check. The OvmfPkg provides ApicTimerLib for the DXE phase. The
constructor AcpiTimerLibConstructor() used in the ApicTimerLib uses
the PciRead to get the PMBASE register. The PciRead() will cause an
MMIO access.

The AmdSevDxe driver clears the memory encryption attribute from the
MMIO ranges. However, if VmgExitLib is linked to AmdSevDxe driver then the
AcpiTimerLibConstructor() will be called before AmdSevDxe driver can
clear the encryption attributes for the MMIO regions.

Exclude the PMBASE register from the encrypted check so that we
can link VmgExitLib to the MemEncryptSevLib; which gets linked to
AmdSevDxe driver.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf  |  4 ++
 OvmfPkg/Library/VmgExitLib/VmgExitLib.inf |  7 +++
 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 45 
 3 files changed, 56 insertions(+)

diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf 
b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
index e6f6ea7972..22435a0590 100644
--- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
@@ -27,6 +27,7 @@
   SecVmgExitVcHandler.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
@@ -42,4 +43,7 @@
 [FixedPcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
 
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf 
b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
index c66c68726c..d3175c260e 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
@@ -27,6 +27,7 @@
   PeiDxeVmgExitVcHandler.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
@@ -37,4 +38,10 @@
   DebugLib
   LocalApicLib
   MemEncryptSevLib
+  PcdLib
 
+[FixedPcd]
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c 
b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 24259060fd..01ac5d8c19 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -14,7 +14,10 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
+#include 
 
 #include "VmgExitVcHandler.h"
 
@@ -596,6 +599,40 @@ UnsupportedExit (
   return Status;
 }
 
+STATIC
+BOOLEAN
+IsPmbaBaseAddress (
+  IN  UINTN Address
+  )
+{
+  UINT16 HostBridgeDevId;
+  UINTN Pmba;
+
+  //
+  // Query Host Bridge DID to determine platform type
+  //
+  HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
+  switch (HostBridgeDevId) {
+case INTEL_82441_DEVICE_ID:
+  Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);
+  break;
+case INTEL_Q35_MCH_DEVICE_ID:
+  Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);
+  //
+  // Add the MMCONFIG base address to get the Pmba base access address
+  //
+  Pmba += FixedPcdGet64 (PcdPciExpressBaseAddress);
+  break;
+default:
+  return FALSE;
+  }
+
+  // Round up the offset to page size
+  Pmba = Pmba & ~(SIZE_4KB - 1);
+
+  return (Address == Pmba);
+}
+
 /**
   Validate that the MMIO memory access is not to encrypted memory.
 
@@ -640,6 +677,14 @@ ValidateMmioMemory (
 return 0;
   }
 
+  //
+  // Allow PMBASE accesses (which will have the encryption bit set before
+  // AmdSevDxe runs in the DXE phase)
+  //
+  if (IsPmbaBaseAddress (Address)) {
+return 0;
+  }
+
   //
   // Any state other than unencrypted is an error, issue a #GP.
   //
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 00/19] Add AMD Secure Nested Paging (SEV-SNP) support

[Brijesh Singh]

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

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.
 
This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.
 
Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.
  
Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* AP bring up using the new SEV-SNP NAE
* Lazy validation
* Interrupt security

The series is based on commit:
e542e05d4f UefiCpuPkg/SmmCpuFeaturesLib: Abstract 
PcdCpuMaxLogicalProcessorNumber

Additional resources
-
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf
 
APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section 15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/sev-snp-rfc-1

GHCB spec v2:
  The draft specification is posted on AMD-SEV-SNP mailing list:
   https://lists.suse.com/mailman/private/amd-sev-snp/

  Copy of the spec is also available at 
  
https://github.com/AMDESE/AMDSEV/blob/sev-snp-devel/docs/56421-Guest_Hypervisor_Communication_Block_Standardization.pdf

GHCB spec v1:
SEV-SNP firmware specification:
 https://developer.amd.com/sev/
  
Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 

Brijesh Singh (19):
  OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest
  OvmfPkg: validate the data pages used in the SEC phase
  MdePkg: Expand the SEV MSR to include the SNP definition
  OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
  MdePkg: Define the GHCB GPA structure
  UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
  OvmfPkg: Add a library to support registering GHCB GPA
  OvmfPkg: register GHCB gpa for the SEV-SNP guest
  MdePkg: Add AsmPvalidate() support
  OvmfPkg: Define the Page State Change VMGEXIT structures
  OvmfPkg/ResetVector: Invalidate the GHCB page
  OvmfPkg/MemEncryptSevLib: Add support to validate system RAM
  OvmfPkg/SecMain: Validate the data/code pages used for the PEI phase
  OvmfPkg/MemEncryptSevLib: Add support to validate RAM in PEI phase
  OvmfPkg/PlatformPei: Validate the system RAM when SNP is active
  OvmfPkg/MemEncryptSevLib: Add support to validate > 4GB memory in PEI
phase
  OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase
  OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc
attribute
  OvmfPkg/MemEncryptSevLib: Skip page state change for non RAM region

 MdePkg/Include/Library/BaseLib.h  |  37 +++
 MdePkg/Include/Register/Amd/Fam17Msr.h|  31 ++-
 MdePkg/Include/Register/Amd/Ghcb.h|  39 ++-
 MdePkg/Library/BaseLib/BaseLib.inf|   1 +
 MdePkg/Library/BaseLib/X64/Pvalidate.nasm |  43 +++
 OvmfPkg/Include/Library/GhcbRegisterLib.h |  27 ++
 OvmfPkg/Include/Library/MemEncryptSevLib.h|  30 +++
 .../DxeMemEncryptSevLib.inf   |   7 +
 .../DxeMemEncryptSevLibInternal.c |  27 ++
 .../Ia32/SnpPageStateChange.c |  17 ++
 .../PeiMemEncryptSevLib.inf   |   9 +

[edk2-devel] [RFC PATCH 12/19] OvmfPkg/MemEncryptSevLib: Add support to validate system RAM

[Brijesh Singh]

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

Many of the integrity guarantees of SEV-SNP are enforced through the
Reverse Map Table (RMP). Each RMP entry contains the GPA at which a
particular page of DRAM should be mapped. The guest can request the
hypervisor to add pages in the RMP table via the Page State Change VMGEXIT
defined in the GHCB specification section 2.5.1 and 4.1.6. Inside each RMP
entry is a Validated flag; this flag is automatically cleared to 0 by the
CPU hardware when a new RMP entry is created for a guest. Each VM page
can be either validated or invalidated, as indicated by the Validated
flag in the RMP entry. Memory access to a private page that is not
validated generates a #VC. A VM can use the PVALIDATE instruction to
validate the private page before using it.

During the guest creation, the boot ROM memory is pre-validated by the
AMD-SEV firmware. The MemEncryptSevSnpValidateSystemRam() can be called
during the SEC and PEI phase to validate the detected system RAM.

One of the fields in the Page State Change NAE is the RMP page size. The
page size input parameter indicates that either a 4KB or 2MB page should
be used while adding the RMP entry. During the validation, when possible,
the MemEncryptSevSnpValidateSystemRam() will use the 2MB entry. A
hypervisor backing the memory may choose to use the different page size
in the RMP entry. In those cases, the PVALIDATE instruction should return
SIZEMISMATCH. If a SIZEMISMATCH is detected, then validate all 512-pages
constituting a 2MB region.

Upon completion, the PVALIDATE instruction sets the rFLAGS.CF to 0 if
instruction changed the RMP entry and to 1 if the instruction did not
change the RMP entry. The rFlags.CF will be 1 only when a memory region
is already validated. We should not double validate a memory
as it could lead to a security compromise. If double validation is
detected, terminate the boot.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h|  15 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c|  17 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf  |   4 +
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c|  20 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h |  37 +++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c|  23 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 254 

 7 files changed, 370 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h 
b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 03d9eda392..47d6802b61 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -215,4 +215,19 @@ MemEncryptSevGetAddressRangeState (
   IN UINTNLength
   );
 
+/**
+  If SEV-SNP is active then set the page state of the specified virtual
+  address range. This should be called in SEC and PEI phases only.
+
+  @param[in]  BaseAddress Base address
+  @param[in]  NumPagesNumber of pages starting from the base 
address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpValidateSystemRam (
+  IN PHYSICAL_ADDRESS   BaseAddress,
+  IN UINTN  NumPages
+  );
+
 #endif // _MEM_ENCRYPT_SEV_LIB_H_
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
new file mode 100644
index 00..dace5c0bcf
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
@@ -0,0 +1,17 @@
+#include 
+
+#include "../SnpPageStateChange.h"
+
+/**
+ The function is used to set the page state when SEV-SNP is active. The page 
state
+ transition consist of changing the page ownership in the RMP table, and using 
the
+ PVALIDATE instruction to update the Validated bit in RMP table.
+
+ */
+VOID
+SevSnpValidateSystemRamInternal (
+  IN EFI_PHYSICAL_ADDRESS BaseAddress,
+  IN UINTNNumPages
+  )
+{
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
index 279c38bfbc..8595e244c2 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
@@ -31,15 +31,19 @@
 
 [Sources]
   SecMemEncryptSevLibInternal.c
+  SnpPageStateChange.h
 
 [Sources.X64]
   X64/MemEncryptSevLib.c
   X64/SecVirtualMemory.c
+  X64/SecSnpSystemRamValidate.c
+  X64/SnpPageStateChangeInternal.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
 [Sources.IA32]
   Ia32/MemEncryptSevLib.c
+  Ia32/SnpPageStateChange.c
 
 [LibraryClasses]
   BaseLib
diff --git a/OvmfPkg/Library/BaseMemEncrypt

[edk2-devel] [RFC PATCH 14/19] OvmfPkg/MemEncryptSevLib: Add support to validate RAM in PEI phase

[Brijesh Singh]

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

MemEncryptSevSnpValidateSystemRam() is used for validating the system
RAM. During the validation process, we must avoid double validation
cases. The double validation can lead to security issues. Extend the
MemEncryptSevSnpValidateSystemRam() to use the interval search tree to
keep track of the validated range; if the requested range is already
validated, then do nothing. If the requested range overlaps with the
previous validation, then validate only non-overlapped ranges.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf   |   8 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c |  20 
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c | 105 
+
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateTrack.c   | 119 

 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateTrack.h   |  36 ++
 5 files changed, 288 insertions(+)

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf 
b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
index 03a78c32df..cb9dd2bb21 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
@@ -32,10 +32,15 @@
 [Sources]
   PeiDxeMemEncryptSevLibInternal.c
   PeiMemEncryptSevLibInternal.c
+  SnpPageStateChange.h
 
 [Sources.X64]
   X64/MemEncryptSevLib.c
   X64/PeiDxeVirtualMemory.c
+  X64/PeiSnpSystemRamValidate.c
+  X64/SnpPageStateTrack.c
+  X64/SnpPageStateChangeInternal.c
+  X64/SnpPageStateTrack.h
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
@@ -49,9 +54,12 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  VmgExitLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
 [FixedPcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index b561f211f5..9863722e9d 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -17,6 +17,8 @@
 #include 
 #include 
 
+#include "SnpPageStateChange.h"
+
 STATIC BOOLEAN mSevStatus = FALSE;
 STATIC BOOLEAN mSevEsStatus = FALSE;
 STATIC BOOLEAN mSevSnpStatus = FALSE;
@@ -184,3 +186,21 @@ MemEncryptSevGetEncryptionMask (
 
   return mSevEncryptionMask;
 }
+
+/**
+  If SEV-SNP is active then set the page state of the specified virtual
+  address range. This should be called in SEC and PEI phases only.
+
+  @param[in]  BaseAddress Base address
+  @param[in]  NumPagesNumber of pages starting from the base 
address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpValidateSystemRam (
+  IN PHYSICAL_ADDRESS   BaseAddress,
+  IN UINTN  NumPages
+  )
+{
+  SevSnpValidateSystemRam (BaseAddress, NumPages);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
new file mode 100644
index 00..ce8a05bb1f
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -0,0 +1,105 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../SnpPageStateChange.h"
+#include "SnpPageStateTrack.h"
+
+STATIC SNP_VALIDATED_RANGE *mRootNode;
+
+STATIC
+SNP_VALIDATED_RANGE *
+SetPageStateChangeInitialize (
+  VOID
+  )
+{
+  UINTN  StartAddress, EndAddress;
+  SNP_VALIDATED_RANGE*RootNode;
+
+  //
+  // The memory range from PcdOvmfSnpCpuidBase to 
PcdOvmfDecompressionScratchEnd is
+  // prevalidated before we enter into the Pei phase. The pre-validation 
breakdown
+  // looks like this:
+  //
+  //SnpCpuidBase  (VMM)
+  //SnpSecretBase (VMM)
+  //SnpLaunchValidatedStart - SnpLaunchValidatedEnd   (VMM)
+  //SnpLaunchValidatedEnd - DecompressionScratchEnd   (SecMain)
+  //
+  // Add the range in system ram region tracker interval tree. The interval 
tree will
+  // used to check whether there is an overlap with the pre-validated region. 
We will
+  // skip validating the pre-validated region.
+  //
+  StartAddress = (UINTN) PcdGet32 (PcdOvmfSnpCpuidBase);
+  EndAddress = (UINTN) PcdGet32 (PcdOvmfDecompressionScratchEnd);
+
+  RootNode = AddRangeToIntervalTree (NULL, StartAddr

[edk2-devel] [RFC PATCH 10/19] OvmfPkg: Define the Page State Change VMGEXIT structures

[Brijesh Singh]

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

The Page State Change NAE exit will be used by the SEV-SNP guest to
request a page state change using the GHCB protocol. See the GHCB
spec section 4.1.6 and 2.3.1 for more detail on the structure
definitions.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 MdePkg/Include/Register/Amd/Fam17Msr.h | 17 +
 MdePkg/Include/Register/Amd/Ghcb.h | 39 +---
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h 
b/MdePkg/Include/Register/Amd/Fam17Msr.h
index c074e871a7..084e7fb63a 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -53,6 +53,19 @@ typedef union {
 UINT64  GuestFrameNumber:52;
   } GhcbGpaRegister;
 
+  struct {
+UINT64 Function:12;
+UINT64 GuestFrameNumber:40;
+UINT64 Operation:4;
+UINT64 Reserved:8;
+  } SnpPageStateChangeRequest;
+
+  struct {
+UINT32 Function:12;
+UINT32 Reserved:20;
+UINT32 ErrorCode;
+  } SnpPageStateChangeResponse;
+
   VOID*Ghcb;
 
   UINT64  GhcbPhysicalAddress;
@@ -65,6 +78,10 @@ typedef union {
 #define GHCB_INFO_TERMINATE_REQUEST256
 #define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST   18
 #define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE  19
+#define GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST   20
+#define GHCB_INFO_SNP_PAGE_STATE_CHANGE_RESPONSE  21
+#define   SNP_PAGE_STATE_PRIVATE  1
+#define   SNP_PAGE_STATE_SHARED   2
 
 #define GHCB_TERMINATE_GHCB0
 #define GHCB_TERMINATE_GHCB_GENERAL0
diff --git a/MdePkg/Include/Register/Amd/Ghcb.h 
b/MdePkg/Include/Register/Amd/Ghcb.h
index ccdb662af7..f3c719e70e 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -49,12 +49,13 @@
 //
 // VMG Special Exit Codes
 //
-#define SVM_EXIT_MMIO_READ  0x8001ULL
-#define SVM_EXIT_MMIO_WRITE 0x8002ULL
-#define SVM_EXIT_NMI_COMPLETE   0x8003ULL
-#define SVM_EXIT_AP_RESET_HOLD  0x8004ULL
-#define SVM_EXIT_AP_JUMP_TABLE  0x8005ULL
-#define SVM_EXIT_UNSUPPORTED0x8000ULL
+#define SVM_EXIT_MMIO_READ0x8001ULL
+#define SVM_EXIT_MMIO_WRITE   0x8002ULL
+#define SVM_EXIT_NMI_COMPLETE 0x8003ULL
+#define SVM_EXIT_AP_RESET_HOLD0x8004ULL
+#define SVM_EXIT_AP_JUMP_TABLE0x8005ULL
+#define SVM_EXIT_SNP_PAGE_STATE_CHANGE0x8010ULL
+#define SVM_EXIT_UNSUPPORTED  0x8000ULL
 
 //
 // IOIO Exit Information
@@ -154,4 +155,30 @@ typedef union {
 #define GHCB_EVENT_INJECTION_TYPE_EXCEPTION  3
 #define GHCB_EVENT_INJECTION_TYPE_SOFT_INT   4
 
+#define SNP_PAGE_STATE_MAX_NPAGES   4095
+#define SNP_PAGE_STATE_MAX_ENTRY253
+#define SNP_PAGE_STATE_PRIVATE  1
+#define SNP_PAGE_STATE_SHARED   2
+#define SNP_PAGE_STATE_PSMASH   3
+#define SNP_PAGE_STATE_UNSMASH  4
+
+typedef PACKED struct {
+  UINT64  CurrentPage:12;
+  UINT64  GuestFrameNumber:40;
+  UINT64  Op:4;
+  UINT64  PageSize:1;
+  UINT64  Rsvd: 7;
+} SNP_PAGE_STATE_ENTRY;
+
+typedef PACKED struct {
+  UINT16 CurrentEntry;
+  UINT16 EndEntry;
+  UINT32 Rsvd;
+} SNP_PAGE_STATE_HEADER;
+
+typedef struct {
+  SNP_PAGE_STATE_HEADER  Header;
+  SNP_PAGE_STATE_ENTRY   Entry[SNP_PAGE_STATE_MAX_ENTRY];
+} SNP_PAGE_STATE_CHANGE_INFO;
+
 #endif
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 08/19] OvmfPkg: register GHCB gpa for the SEV-SNP guest

[Brijesh Singh]

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

The SEV-SNP guest requires that GHCB GPA must be registered before using.
The GHCB GPA can be registred using the GhcbGPARegister().

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/PlatformPei/AmdSev.c| 11 +++
 OvmfPkg/PlatformPei/PlatformPei.inf |  2 +
 OvmfPkg/Sec/SecMain.c   | 76 
 3 files changed, 89 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index dddffdebda..95c5ad235f 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -110,6 +111,16 @@ AmdSevEsInitialize (
 "SEV-ES is enabled, %lu GHCB backup pages allocated starting at 0x%p\n",
 (UINT64)GhcbBackupPageCount, GhcbBackupBase));
 
+  if (MemEncryptSevSnpIsEnabled ()) {
+//
+// SEV-SNP guest requires that GHCB GPA must be registered before using it.
+//
+GhcbRegister (GhcbBasePa);
+
+PcdStatus = PcdSetBoolS (PcdSevSnpIsEnabled, TRUE);
+ASSERT_RETURN_ERROR (PcdStatus);
+  }
+
   AsmWriteMsr64 (MSR_SEV_ES_GHCB, GhcbBasePa);
 
   //
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 6ef77ba7bb..cb6f5ac091 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -52,6 +52,7 @@
   BaseLib
   CacheMaintenanceLib
   DebugLib
+  GhcbRegisterLib
   HobLib
   IoLib
   PciLib
@@ -110,6 +111,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled
+  gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled
 
 [FixedPcd]
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2..df6722b546 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -750,6 +750,76 @@ SevEsProtocolFailure (
   CpuDeadLoop ();
 }
 
+/**
+  Determine if SEV-SNP is active. There is a MemEncryptIsSnpEnabled() in 
MemEncryptSevLib
+  but we can not use it because the SEV-SNP check need to be done before the
+  ProcessLibraryConstructorList() is called.
+
+  @retval TRUE   SEV-SNP is enabled
+  @retval FALSE  SEV-SNP is not enabled
+
+**/
+STATIC
+BOOLEAN
+SevSnpIsEnabled (
+  VOID
+  )
+{
+   MSR_SEV_STATUS_REGISTER   Msr;
+
+   Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+
+   return Msr.Bits.SevSnpBit ? TRUE : FALSE;
+}
+
+/**
+ The GHCB GPA registeration need to be done before the 
ProcessLibraryConstructorList()
+ is called. So use a local implementation instead of including the 
GhcbRegisterLib.
+
+ */
+STATIC
+VOID
+SevSnpGhcbRegister (
+  UINTN   Address
+  )
+{
+  MSR_SEV_ES_GHCB_REGISTER  Msr;
+  MSR_SEV_ES_GHCB_REGISTER  CurrentMsr;
+  EFI_PHYSICAL_ADDRESS  GuestFrameNumber;
+
+  GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
+
+  //
+  // Save the current MSR Value
+  //
+  CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // Use the GHCB MSR Protocol to request to register the GPA.
+  //
+  Msr.GhcbPhysicalAddress = 0;
+  Msr.GhcbGpaRegister.Function = GHCB_INFO_GHCB_GPA_REGISTER_REQUEST;
+  Msr.GhcbGpaRegister.GuestFrameNumber = GuestFrameNumber;
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+  AsmVmgExit ();
+
+  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // If hypervisor responded with a different GPA than requested then fail.
+  //
+  if ((Msr.GhcbGpaRegister.Function != GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE) ||
+  (Msr.GhcbGpaRegister.GuestFrameNumber != GuestFrameNumber)) {
+SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL);
+  }
+
+  //
+  // Restore the MSR
+  //
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
+}
+
 /**
   Validate the SEV-ES/GHCB protocol level.
 
@@ -791,6 +861,12 @@ SevEsProtocolCheck (
 SevEsProtocolFailure (GHCB_TERMINATE_GHCB_PROTOCOL);
   }
 
+  if (SevSnpIsEnabled ()) {
+//
+// SEV-SNP guest requires that GHCB GPA must be registered before using it.
+//
+SevSnpGhcbRegister (FixedPcdGet32 (PcdOvmfSecGhcbBase));
+  }
   //
   // SEV-ES protocol checking succeeded, set the initial GHCB address
   //
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 06/19] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled

[Brijesh Singh]

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

An SEV-SNP guest requires that the physical address of the GHCB must
be registered with the hypervisor before using it. See the GHCB specification
for the futher detail.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc|  1 +
 UefiCpuPkg/Library/MpInitLib/MpLib.c  |  2 +
 UefiCpuPkg/Library/MpInitLib/MpLib.h  |  2 +
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 
 UefiCpuPkg/UefiCpuPkg.dec |  6 +++
 7 files changed, 64 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 860a9750e2..9a366ca5b1 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -75,3 +75,4 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase   ## 
SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard  ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase   ## 
CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled ## 
CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc 
b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 2e9368a374..01668638f2 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -92,6 +92,7 @@ struc MP_CPU_EXCHANGE_INFO
   .ModeHighSegment:  CTYPE_UINT16 1
   .Enable5LevelPaging:   CTYPE_BOOLEAN 1
   .SevEsIsEnabled:   CTYPE_BOOLEAN 1
+  .SevSnpIsEnabled   CTYPE_BOOLEAN 1
   .GhcbBase: CTYPE_UINTN 1
 endstruc
 
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 5040053dad..da6fbbc1cc 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1040,6 +1040,7 @@ FillExchangeInfoData (
   DEBUG ((DEBUG_INFO, "%a: 5-Level Paging = %d\n", gEfiCallerBaseName, 
ExchangeInfo->Enable5LevelPaging));
 
   ExchangeInfo->SevEsIsEnabled  = CpuMpData->SevEsIsEnabled;
+  ExchangeInfo->SevSnpIsEnabled  = CpuMpData->SevSnpIsEnabled;
   ExchangeInfo->GhcbBase= (UINTN) CpuMpData->GhcbBase;
 
   //
@@ -2016,6 +2017,7 @@ MpInitLibInitialize (
   CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + 
MaxLogicalProcessorNumber);
   InitializeSpinLock(&CpuMpData->MpLock);
   CpuMpData->SevEsIsEnabled = PcdGetBool (PcdSevEsIsEnabled);
+  CpuMpData->SevSnpIsEnabled = PcdGetBool (PcdSevSnpIsEnabled);
   CpuMpData->SevEsAPBuffer  = (UINTN) -1;
   CpuMpData->GhcbBase   = PcdGet64 (PcdGhcbBase);
 
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 0bd60388b1..7d3ce61d63 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -216,6 +216,7 @@ typedef struct {
   //
   BOOLEAN   Enable5LevelPaging;
   BOOLEAN   SevEsIsEnabled;
+  BOOLEAN   SevSnpIsEnabled;
   UINTN GhcbBase;
 } MP_CPU_EXCHANGE_INFO;
 
@@ -285,6 +286,7 @@ struct _CPU_MP_DATA {
   BOOLEANWakeUpByInitSipiSipi;
 
   BOOLEANSevEsIsEnabled;
+  BOOLEANSevSnpIsEnabled;
   UINTN  SevEsAPBuffer;
   UINTN  SevEsAPResetStackStart;
   CPU_MP_DATA*NewCpuMpData;
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 49b0ffe8be..4477dd1b9f 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -64,6 +64,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled  ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase   ## 
SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase   ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled ## CONSUMES
 
 [Ppis]
   gEdkiiPeiShadowMicrocodePpiGuid## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm 
b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 50df802d1f..19939c093d 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -194,9 +194,60 @@ LongModeStart:
 movrdx, rax
 shrrdx, 32
 movrcx, 0xc0010130
+
+;
+; Register GHCB GPA when SEV-SNP is enabled
+;
+leaedi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevSnpIsEnabled)]
+cmpbyte [edi], 1; SevSnpIsEnabled
+jneSetGhcbA

[edk2-devel] [RFC PATCH 05/19] MdePkg: Define the GHCB GPA structure

[Brijesh Singh]

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

An SEV-SNP guest is required to perform the GHCB GPA registration. See
the GHCB specification for further details.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h 
b/MdePkg/Include/Register/Amd/Fam17Msr.h
index 4d33bef220..c074e871a7 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -48,6 +48,11 @@ typedef union {
 UINT32  Reserved2:32;
   } GhcbTerminate;
 
+  struct {
+UINT64  Function:12;
+UINT64  GuestFrameNumber:52;
+  } GhcbGpaRegister;
+
   VOID*Ghcb;
 
   UINT64  GhcbPhysicalAddress;
@@ -58,6 +63,8 @@ typedef union {
 #define GHCB_INFO_CPUID_REQUEST4
 #define GHCB_INFO_CPUID_RESPONSE   5
 #define GHCB_INFO_TERMINATE_REQUEST256
+#define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST   18
+#define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE  19
 
 #define GHCB_TERMINATE_GHCB0
 #define GHCB_TERMINATE_GHCB_GENERAL0
-- 
2.17.1



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

[edk2-devel] [RFC PATCH 04/19] OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()

[Brijesh Singh]

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

Create a function can be used to determine if VM is running as an
SEV-SNP guest.

Cc: James Bottomley 
Cc: Min Xu 
Cc: Jiewen Yao 
Cc: Tom Lendacky 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Signed-off-by: Brijesh Singh 
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 
+
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 27 

 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 27 

 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 19 
++
 4 files changed, 85 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h 
b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 99f15a7d12..03d9eda392 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -66,6 +66,18 @@ typedef enum {
   MemEncryptSevAddressRangeError,
 } MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE;
 
+/**
+  Returns a boolean to indicate whether SEV-SNP is enabled
+
+  @retval TRUE   SEV-SNP is enabled
+  @retval FALSE  SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+  VOID
+  );
+
 /**
   Returns a boolean to indicate whether SEV-ES is enabled.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
index 2816f859a0..0571297238 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c
@@ -19,6 +19,7 @@
 
 STATIC BOOLEAN mSevStatus = FALSE;
 STATIC BOOLEAN mSevEsStatus = FALSE;
+STATIC BOOLEAN mSevSnpStatus = FALSE;
 STATIC BOOLEAN mSevStatusChecked = FALSE;
 
 STATIC UINT64  mSevEncryptionMask = 0;
@@ -82,11 +83,37 @@ InternalMemEncryptSevStatus (
 if (Msr.Bits.SevEsBit) {
   mSevEsStatus = TRUE;
 }
+
+//
+// Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled)
+//
+if (Msr.Bits.SevSnpBit) {
+  mSevSnpStatus = TRUE;
+}
   }
 
   mSevStatusChecked = TRUE;
 }
 
+/**
+  Returns a boolean to indicate whether SEV-SNP is enabled.
+
+  @retval TRUE   SEV-SNP is enabled
+  @retval FALSE  SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+  VOID
+  )
+{
+  if (!mSevStatusChecked) {
+InternalMemEncryptSevStatus ();
+  }
+
+  return mSevSnpStatus;
+}
+
 /**
   Returns a boolean to indicate whether SEV-ES is enabled.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index e2fd109d12..b561f211f5 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -19,6 +19,7 @@
 
 STATIC BOOLEAN mSevStatus = FALSE;
 STATIC BOOLEAN mSevEsStatus = FALSE;
+STATIC BOOLEAN mSevSnpStatus = FALSE;
 STATIC BOOLEAN mSevStatusChecked = FALSE;
 
 STATIC UINT64  mSevEncryptionMask = 0;
@@ -82,11 +83,37 @@ InternalMemEncryptSevStatus (
 if (Msr.Bits.SevEsBit) {
   mSevEsStatus = TRUE;
 }
+
+//
+// Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled)
+//
+if (Msr.Bits.SevSnpBit) {
+  mSevSnpStatus = TRUE;
+}
   }
 
   mSevStatusChecked = TRUE;
 }
 
+/**
+  Returns a boolean to indicate whether SEV-SNP is enabled.
+
+  @retval TRUE   SEV-SNP is enabled
+  @retval FALSE  SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+  VOID
+  )
+{
+  if (!mSevStatusChecked) {
+InternalMemEncryptSevStatus ();
+  }
+
+  return mSevSnpStatus;
+}
+
 /**
   Returns a boolean to indicate whether SEV-ES is enabled.
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c 
b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
index 56d8f3f318..69852779e2 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c
@@ -62,6 +62,25 @@ InternalMemEncryptSevStatus (
   return ReadSevMsr ? AsmReadMsr32 (MSR_SEV_STATUS) : 0;
 }
 
+/**
+  Returns a boolean to indicate whether SEV-SNP is enabled.
+
+  @retval TRUE   SEV-SNP is enabled
+  @retval FALSE  SEV-SNP is not enabled
+**/
+BOOLEAN
+EFIAPI
+MemEncryptSevSnpIsEnabled (
+  VOID
+  )
+{
+  MSR_SEV_STATUS_REGISTER   Msr;
+
+  Msr.Uint32 = InternalMemEncryptSevStatus ();
+
+  return Msr.Bits.SevSnpBit ? TRUE : FALSE;
+}
+
 /**
   Returns a boolean to indicate whether SEV-ES is enabled.
 
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73220): https://edk2.groups.io/g/devel/message/73220
Mute This Topic: https://groups.io/mt/81584580/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Laszlo Ersek]

On 03/24/21 17:55, Benjamin Doron wrote:
>>
>>
>>> Hi all,
>>> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
>>> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
>>> and then install the tables? It's a solution that uses the regular
>>> UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
>>> is in the configuration table, we probably always want those tables).
>>
>> I'm sorry, I don't understand how this would help.
> 
> As I understand it, the issue is that this patchset changes MdeModulePkg to 
> do platform-specific parsing.
> 
> Perhaps it would be an acceptable solution for platforms to retrieve the 
> tables, then add
> RSDP/them to the configuration table to be installed by 
> AcpiTableDxe/AcpiPlatformDxe.
> This allows MdeModulePkg to abstract away the parsing, only installing tables
> available to it.

But this is already the best approach, and already what's happening --
when you call EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() from the
platform's AcpiPlatformDxe, that's *how* you tell AcpiTableDxe in
MdeModulePkg to pick up the table and hook it into the RSDT / XSDT /
wherever, and also to manage RSD PTR as a UEFI configuration table.

Are you (more or less) proposing a new EFI_ACPI_TABLE_PROTOCOL member
function for taking a forest of ACPI tables, passed in by RSD PTR?

Sorry about being dense. :)

> (Currently, UefiPayloadPkg's BlSupportDxe retrieves the data from a HOB and 
> calls
> `gBS->InstallConfigurationTable` with the address of RSDP).
> 
> I understand that this may not work for OVMF if tables are located 
> differently in memory.
> 
>>
>>
>>> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in
>>> DSC) but not added to a FV (not listed in FDF). So, how has this been
>>> tested?
>>
>> I assume through an out-of-tree platform. Many such core modules exist
>> in edk2 that are not consumed by any of the virtual platforms in the
>> edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg).
> 
> This makes sense, but AcpiTableDxe must be added to UefiPayloadPkg's FDF
> if patch 2/2 is merged. Otherwise, ACPI tables will not be advertised.
> 

Good point; I missed that. (I'm not familiar with the UefiPayloadPkg FDF
at all.)

Laszlo



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

Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: Allow BMP with extra data

[Laszlo Ersek]

On 03/24/21 16:25, Jeff Brasen wrote:
> Some of the logo files we received for the group that makes our assets like 
> this (not sure what tool they were created with) look like they pad the BMP 
> size to 8 bytes.
> 
> TranslateBmpToGopBlt: invalid BmpImage...
>BmpHeader->Size: 0xE1038
>BmpHeader->ImageOffset: 0x36
>BmpImageSize: 0xE1038
>DataSize: 0xE1000
> TranslateBmpToGopBlt: invalid BmpImage...
>BmpHeader->Size: 0x2A3038
>BmpHeader->ImageOffset: 0x36
>BmpImageSize: 0x2A3038
>DataSize: 0x2A3000
> TranslateBmpToGopBlt: invalid BmpImage...
>BmpHeader->Size: 0x5EEC38
>BmpHeader->ImageOffset: 0x36
>BmpImageSize: 0x5EEC38
>DataSize: 0x5EEC00
> 
> So, each of these has 2 bytes of padding at the end of the file. We could 
> write a tool that would do the same size recalculation in order to update the 
> size in the header and remove the two bytes but it seems that this is a valid 
> BMP file and it doesn't seem correct that UEFI is rejecting it. I can update 
> the commit message with more context if needed as well.

If there's a spec describing the BMP format, and edk2 is needlessly
strict, and the check can be relaxed without security risks, then I
think a patch would be fair.

Thanks
Laszlo



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

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Benjamin Doron]

> 
> 
>> Hi all,
>> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
>> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
>> and then install the tables? It's a solution that uses the regular
>> UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
>> is in the configuration table, we probably always want those tables).
> 
> I'm sorry, I don't understand how this would help.

As I understand it, the issue is that this patchset changes MdeModulePkg to do 
platform-specific parsing.

Perhaps it would be an acceptable solution for platforms to retrieve the 
tables, then add
RSDP/them to the configuration table to be installed by 
AcpiTableDxe/AcpiPlatformDxe.
This allows MdeModulePkg to abstract away the parsing, only installing tables
available to it.
(Currently, UefiPayloadPkg's BlSupportDxe retrieves the data from a HOB and 
calls
`gBS->InstallConfigurationTable` with the address of RSDP).

I understand that this may not work for OVMF if tables are located differently 
in memory.

> 
> 
>> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in
>> DSC) but not added to a FV (not listed in FDF). So, how has this been
>> tested?
> 
> I assume through an out-of-tree platform. Many such core modules exist
> in edk2 that are not consumed by any of the virtual platforms in the
> edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg).

This makes sense, but AcpiTableDxe must be added to UefiPayloadPkg's FDF
if patch 2/2 is merged. Otherwise, ACPI tables will not be advertised.


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

Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: Allow BMP with extra data

[Jeff Brasen]

Some of the logo files we received for the group that makes our assets like 
this (not sure what tool they were created with) look like they pad the BMP 
size to 8 bytes.

TranslateBmpToGopBlt: invalid BmpImage...
   BmpHeader->Size: 0xE1038
   BmpHeader->ImageOffset: 0x36
   BmpImageSize: 0xE1038
   DataSize: 0xE1000
TranslateBmpToGopBlt: invalid BmpImage...
   BmpHeader->Size: 0x2A3038
   BmpHeader->ImageOffset: 0x36
   BmpImageSize: 0x2A3038
   DataSize: 0x2A3000
TranslateBmpToGopBlt: invalid BmpImage...
   BmpHeader->Size: 0x5EEC38
   BmpHeader->ImageOffset: 0x36
   BmpImageSize: 0x5EEC38
   DataSize: 0x5EEC00

So, each of these has 2 bytes of padding at the end of the file. We could write 
a tool that would do the same size recalculation in order to update the size in 
the header and remove the two bytes but it seems that this is a valid BMP file 
and it doesn't seem correct that UEFI is rejecting it. I can update the commit 
message with more context if needed as well.


Thanks,

Jeff


From: Laszlo Ersek 
Sent: Wednesday, March 24, 2021 5:31 AM
To: devel@edk2.groups.io ; bret.barke...@microsoft.com 
; Jeff Brasen 
Cc: jian.j.w...@intel.com ; ao.a...@intel.com 

Subject: Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: 
Allow BMP with extra data

External email: Use caution opening links or attachments


On 03/23/21 18:41, Bret Barkelew via groups.io wrote:
> Is this a *good* idea?
>
> What is considered valid extra data? If it’s immaterial to the FW displaying 
> the image, our policy has been to strip it off BEFORE adding it to the FW 
> image.

Not counting any potential security aspects, stripping out undisplayed
portions helps with flash usage too (I think?), so at least some
concrete justification in the commit message would be nice...

Thanks
Laszlo

>
> - Bret
>
> From: Jeff Brasen via groups.io
> Sent: Tuesday, March 23, 2021 10:29 AM
> To: devel@edk2.groups.io
> Cc: jian.j.w...@intel.com; 
> ao.a...@intel.com; Jeff 
> Brasen
> Subject: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: 
> Allow BMP with extra data
>
> Add support for processing BMP data that contains extra data after the
> image array, this data will not be parsed in anyway in the library but
> images that contain this will not be rejected from processing.
>
> ---
>  MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c 
> b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c
> index 3ac31f6723d0..944d01fe7cdf 100644
> --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c
> +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c
> @@ -213,7 +213,7 @@ TranslateBmpToGopBlt (
>
>if ((BmpHeader->Size != BmpImageSize) ||
>(BmpHeader->Size < BmpHeader->ImageOffset) ||
> -  (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) {
> +  (BmpHeader->Size - BmpHeader->ImageOffset < DataSize)) {
>
>  DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... \n"));
>  DEBUG ((DEBUG_ERROR, "   BmpHeader->Size: 0x%x\n", BmpHeader->Size));
> --
> 2.25.1
>
>
>
>
>
>
>
>
> 
>
>
>



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

[edk2-devel] [PATCH] RISC-V/PlatformPkg: Fix compilation breakage in OpenSBI

[Loic Devulder via groups.io]

platform_ops and platform structures have older entries that don't exist
anymore on recent OpenSBI versions, so we can remove them.

Cc: Abner Chang 
Cc: Daniel Schaefer 
Cc: Gilbert Chen 

Signed-off-by: Loic Devulder 
---
 .../PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git 
a/Platform/RISC-V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c 
b/Platform/RISC-V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c
index e78d811f4c..84b5f5dec8 100644
--- a/Platform/RISC-V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c
+++ b/Platform/RISC-V/PlatformPkg/Library/OpensbiPlatformLibNull/Platform.c
@@ -28,8 +28,7 @@ const struct sbi_platform_operations platform_ops = {
 .timer_event_stop   = NULL,
 .timer_event_start  = NULL,
 .timer_init = NULL,
-.system_reboot  = NULL,
-.system_shutdown= NULL
+.system_reset   = NULL
 };
 
 const struct sbi_platform platform = {
@@ -39,6 +38,5 @@ const struct sbi_platform platform = {
 .features   = 0,
 .hart_count = 0,
 .hart_stack_size= 0,
-.disabled_hart_mask = 0,
 .platform_ops_addr  = 0
 };
-- 
2.30.1



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

[edk2-devel] reg: clarification on Form Browser Action on save changes in IP4 and IP6 configuration

[Sivaraman Nainar]

Hello all:

In the IPV4 and IPV6 Configuration Page after the changes are applied the 
system proceed to boot to the boot device and its not staying in same form.

case KEY_SAVE_CHANGES:
  Status = Ip6ConvertIfrNvDataToConfigNvDataGeneral (IfrNvData, Instance);
  if (EFI_ERROR (Status)) {
break;
  }
  *ActionRequest = EFI_BROWSER_ACTION_REQUEST_SUBMIT;
  *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_APPLY; //Modified code
  break;

If the browser action changed as above the control retained in the same page.

Is there any specific reason after the IP Configuration Settings applied it 
proceed to Boot?

-Siva

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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73210): https://edk2.groups.io/g/devel/message/73210
Mute This Topic: https://groups.io/mt/81577302/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/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

[Pete Batard]

Hi Sunny,

I understand that you are going to re-send these patches due to e-mail 
mangling, but let me add a few of notes that might help you as you are 
doing that.


First of all, your patch series was missing a cover letter (there should 
have been a [PATCH v2 0/2]), that doesn't get applied, but that 
describes the series or the changes applied between 2 revisions.


This isn't a big deal for a series that has only 2 patches, but it seems 
to indicate that you missed some steps from Laszlo's helpful guide on 
how to configure your environment to send patches to EDK2, which you can 
find at:


https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers

Especially, you should have run the command:

git config format.coverletter true

Now, with regards to patch mangling, as you may be able to see, what I 
got is not as bad as what you seem to have been getting. In fact, 
barring a few issues (which I am pointing out below) your patch is 
almost usable, even with the double line feeds I am seeing from my 
e-mail client (because that's actually how I currently receive about 
half the patches that are posted on this list anyway, so much so that I 
have had to craft a quick script to remove them).


There are however some issues, that actually prevent your patch from 
applying properly, and, while I'm at it, I also added a few comments 
about some extra lines you added, that you should be able to fix for 
your next submission:


On 2021.03.24 09:45, Sunny Wang wrote:

Changes:

   1. Add code to ConfigDxe driver and AcpiTables module to dynamically

  build either Mini UART or PL011 UART info in ACPI. This fixes the

  issue discussed in https://github.com/pftf/RPi4/issues/118.

   2. Cleanup by moving duplicate Debug Port 2 table related defines and

  structures to a newly created header file (RpiDebugPort2Table.h).



Testing Done:

   - Booted to UEFI shell and use acpiview command to check the result of

 the different UART settings in config.txt (enabling either Mini UART

 or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically

 changed as expected.



Cc: Samer El-Haj-Mahmoud 

Cc: Sami Mujawar 

Cc: Jeremy Linton 

Cc: Pete Batard 

Cc: Ard Biesheuvel 

Signed-off-by: Sunny Wang 

---

  .../RaspberryPi/AcpiTables/AcpiTables.inf |   9 +-

  .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 

  .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 -

  .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +

  .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +-

  Platform/RaspberryPi/AcpiTables/Uart.asl  |  10 +-

  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +-

  .../IndustryStandard/RpiDebugPort2Table.h |  34 

  8 files changed, 497 insertions(+), 212 deletions(-)

  create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc

  rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)

  create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc

  rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)

  create mode 100644 
Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h



diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf

index d3363a76a1..fc8e927138 100644

--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf

+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf

@@ -2,7 +2,7 @@

  #

  #  ACPI table data and ASL sources required to boot the platform.

  #

-#  Copyright (c) 2019, ARM Limited. All rights reserved.

+#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.

  #  Copyright (c) 2017, Andrey Warkentin 

  #  Copyright (c) Microsoft Corporation. All rights reserved.

  #

@@ -28,12 +28,14 @@

Emmc.asl

Madt.aslc

Fadt.aslc

-  Dbg2.aslc

+  Dbg2MiniUart.aslc

+  Dbg2Pl011.aslc

Gtdt.aslc

Iort.aslc

Dsdt.asl

Csrt.aslc

-  Spcr.aslc

+  SpcrMiniUart.aslc

+  SpcrPl011.aslc

Pptt.aslc

SsdtThermal.asl




For the patch to be applicable, there should have been an additional 
space on the line above.

In other words, is should be ">  " and not just "> ".



@@ -71,3 +73,4 @@




Same here, and for about 4-5 more instances further down, that I'm not 
going to document.


If using 'git format-patch' and 'git send-email' as documented in 
Laszlo's guide, I would expect these to go away.




  [BuildOptions]

GCC:*_*_*_ASL_FLAGS   = -vw3133 -vw3150

+


This doesn't have to do with patch mangling, but you are adding an 
unwarranted extra line here. Can you please remove it from your next 
submission?




diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc 
b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc

new file mode 100644

index 00..c83b978731

--- /dev/null

+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniU

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Benjamin Doron]

Hi all,
Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in 
MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP and then 
install the tables? It's a solution that uses the regular UefiLib, so it avoids 
platform-specific quirks (and as I see it, if RSDP is in the configuration 
table, we probably always want those tables).

Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in DSC) 
but not added to a FV (not listed in FDF). So, how has this been tested?

Regards,
Benjamin Doron


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

[edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds

[Ross Burton]

GenFw will embed a NB10 section which contains the path to the input file,
which means the output files have build paths embedded in them.  To reduce
information leakage and ensure reproducible builds, pass --zero in release
builds to remove this information.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3256
Signed-off-by: Ross Burton 
---
 OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
 OvmfPkg/Bhyve/BhyveX64.dsc   | 1 +
 OvmfPkg/OvmfPkgIa32.dsc  | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc   | 1 +
 OvmfPkg/OvmfPkgX64.dsc   | 1 +
 OvmfPkg/OvmfXen.dsc  | 1 +
 6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 65c42284d9..69a05feea9 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -78,6 +78,7 @@
   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
 !endif
+  RELEASE_*_*_GENFW_FLAGS = --zero
 
   #
   # Disable deprecated APIs.
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 4a1cdf5aca..132f55cf69 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -76,6 +76,7 @@
   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
 !endif
+  RELEASE_*_*_GENFW_FLAGS = --zero
 
   #
   # Disable deprecated APIs.
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 1eaf3e99c6..93c209950c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -80,6 +80,7 @@
 !if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANGPDB"
   GCC:*_*_*_CC_FLAGS   = -mno-mmx -mno-sse
 !endif
+  RELEASE_*_*_GENFW_FLAGS = --zero
 
   #
   # Disable deprecated APIs.
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 4a5a430147..97cc438250 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -84,6 +84,7 @@
   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
 !endif
+  RELEASE_*_*_GENFW_FLAGS = --zero
 
   #
   # Disable deprecated APIs.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d4d601b444..f544fb04bf 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -84,6 +84,7 @@
   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
 !endif
+  RELEASE_*_*_GENFW_FLAGS = --zero
 
   #
   # Disable deprecated APIs.
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 507029404f..fcaa35acf1 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -74,6 +74,7 @@
   GCC:*_*_X64_GENFW_FLAGS   = --keepexceptiontable
   INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
 !endif
+  RELEASE_*_*_GENFW_FLAGS = --zero
 
   #
   # Disable deprecated APIs.
-- 
2.25.1



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

Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: Allow BMP with extra data

[Laszlo Ersek]

On 03/23/21 18:41, Bret Barkelew via groups.io wrote:
> Is this a *good* idea?
> 
> What is considered valid extra data? If it’s immaterial to the FW displaying 
> the image, our policy has been to strip it off BEFORE adding it to the FW 
> image.

Not counting any potential security aspects, stripping out undisplayed
portions helps with flash usage too (I think?), so at least some
concrete justification in the commit message would be nice...

Thanks
Laszlo

> 
> - Bret
> 
> From: Jeff Brasen via groups.io
> Sent: Tuesday, March 23, 2021 10:29 AM
> To: devel@edk2.groups.io
> Cc: jian.j.w...@intel.com; 
> ao.a...@intel.com; Jeff 
> Brasen
> Subject: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: 
> Allow BMP with extra data
> 
> Add support for processing BMP data that contains extra data after the
> image array, this data will not be parsed in anyway in the library but
> images that contain this will not be rejected from processing.
> 
> ---
>  MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c 
> b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c
> index 3ac31f6723d0..944d01fe7cdf 100644
> --- a/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c
> +++ b/MdeModulePkg/Library/BaseBmpSupportLib/BmpSupportLib.c
> @@ -213,7 +213,7 @@ TranslateBmpToGopBlt (
> 
>if ((BmpHeader->Size != BmpImageSize) ||
>(BmpHeader->Size < BmpHeader->ImageOffset) ||
> -  (BmpHeader->Size - BmpHeader->ImageOffset != DataSize)) {
> +  (BmpHeader->Size - BmpHeader->ImageOffset < DataSize)) {
> 
>  DEBUG ((DEBUG_ERROR, "TranslateBmpToGopBlt: invalid BmpImage... \n"));
>  DEBUG ((DEBUG_ERROR, "   BmpHeader->Size: 0x%x\n", BmpHeader->Size));
> --
> 2.25.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Laszlo Ersek]

On 03/24/21 05:09, Ni, Ray wrote:
>>
>> (1) Currently, BlSupportDxe expects the ACPI content to come from
>> "SYSTEM_TABLE_INFO.AcpiTableBase"
>> [Include/Guid/SystemTableInfoGuid.h].
>> That header file is at least *moderately* documented (it's better than
>> nothing). Additionally, BlSupportDxe is a DXE-phase component.
>>
>> The patch set removes the handling of
>> "SYSTEM_TABLE_INFO.AcpiTableBase"
>> from BlSupportDxe. That means that platforms currently relying on
>> BlSupportDxe to expose the ACPI content will break (until they start
>> producing the new HOB). I don't see the HOB (with this particular GUID)
>> being produced in UefiPayloadPkg anywhere.
> 
> The concern is about PEI passing ACPI Table location through two kinds
> of HOBs. It looks like a flaw in the design. I agree.
> 
> The HOB is produced by PEI phase, by some code that doesn't belong
> to edk2 repo.
> 
>>
>> (2) The UefiPayloadEntry module ("This is the first module for UEFI
>> payload") still relies on "SYSTEM_TABLE_INFO.AcpiTableBase", for parsing
>> various pieces of information into the "AcpiBoardInfo" structure. So
>> even if the HOB producer phase exposes the ACPI payload via a dedicated
>> HOB, it will only create inconsistency between the information parsed by
>> UefiPayloadEntry (from "SYSTEM_TABLE_INFO.AcpiTableBase") and the OS
>> (which will the ACPI contents from the dedicated HOB).
> 
> I agree. At least the SYSTEM_TABLE_INFO.AcpiTableBase field should be removed
> and the accordingly code that consumes ACPI Table in BlSupportDxe should
> be updated to consume the new HOB.
> 
>>
>> (3) The new HOB's structure (regardless of GUID) is not declared in any
>> MdeModulePkg header file, nor the "MdeModulePkg.dec" file. All the info
>> we have is hidden in the source code:
>>
>>   Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER*)
>> (UINTN)(*((UINT64*)GET_GUID_HOB_DATA (GuidHob)));
>>
>> If a platform's PEI phase actually inteded to produce this new HOB, it
>> couldn't rely on a header file / DEC file.
>>
>> This is actually a *step back* from the SystemTableInfoGuid declaration
>> -- header file and DEC file -- that we currently have in UefiPayloadPkg.
> 
> gEfiAcpiTableGuid is defined in MdePkg/Include/Guid/Acpi.h.
> The file header says the GUID is used for entry in EFI system table.
> Now we reuse this GUID for HOB data.
> I think it's ok to use a spec defined GUID for another implementation purpose.
> 
> I can create a file MdeModulePkg/Include/Guid/Acpi.h to define the HOB 
> structure.
> Do you think it's ok?

I'd prefer a new GUID. New GUIDs are very easy to introduce; we won't
run out of them. It's always possible to use a new GUID for some purpose
that relates to an existent GUID. However, in the other direction, it's
difficult to split one use of a GUID from another use of the same GUID.

It's OK if the PI and UEFI specs agree on reusing gEfiAcpiTableGuid for
a new GUIDed HOB -- because they are managed by the same organization.
There's going to be coordination between those two spec working groups.
But the use case here is different.

It's not a problem to use "Acpi" in the GUID's symbolic name, in edk2.
An example for that is "gEfiAcpiVariableGuid" in
"MdeModulePkg/Include/Guid/AcpiS3Context.h". (Well, I think it should
not have used "Efi" in the name, but "Acpi" was fine.
"gEdkiiAcpiVariableGuid" would have been better.)

Especially if this is going to be documented in the universal boot
loader spec, then that spec should propose some C-language prefixes
anyway, such as (just a guess) "Ubl", and then one idea would be
"gUblAcpiTablesGuid", for the HOB.

> 
>>
>>
>> So how can this be called "standardizing and modularizing"?
>>
>> You need a new GUID, a new GUID HOB structure (declared in
>> MdeModulePkg
>> DEC and GUID header); you need to spell out the priority order between
>> the HOB and "SYSTEM_TABLE_INFO.AcpiTableBase" for UefiPayloadPkg, and
>> you need to update all driver in UefiPayloadPkg accordingly.
> 
> Again. I agree it's a flaw in the design. We should remove AcpiTableBase 
> field.
> 
>>
>>
>> I will also not make a secret of my annoyance that, the first time Intel
>> needs such a core extension for some platform feature, it immediately
>> gets all approvals. Whereas, when we needed the exact same feature in
>> OVMF, we struggled for months, if not *years*, to reliably split the
>> ACPI content that OVMF downloaded from QEMU, into blobs that were
>> suitable for the standard ACPI table protocol interfaces. For years I've
>> been telling my colleagues that all this complexity in OVMF's ACPI
>> platform driver is necessary because the EFI_ACPI_TABLE_PROTOCOL
>> implementation in edk2 cannot simply accept a "root pointer", to the
>> ACPI table "forest" that's already laid out in memory. Now I find it
>> just a little bit too convenient that the first time Intel needs the
>> same, we immediately call it "standardizing and modularizing" -- without
>> as much as a header file describing the actual

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Laszlo Ersek]

On 03/24/21 00:52, Benjamin Doron wrote:
> Hi all,
> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
> and then install the tables? It's a solution that uses the regular
> UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
> is in the configuration table, we probably always want those tables).

I'm sorry, I don't understand how this would help.

> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in
> DSC) but not added to a FV (not listed in FDF). So, how has this been
> tested?

I assume through an out-of-tree platform. Many such core modules exist
in edk2 that are not consumed by any of the virtual platforms in the
edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg).

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73204): https://edk2.groups.io/g/devel/message/73204
Mute This Topic: https://groups.io/mt/81543419/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/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

[Sunny Wang]

Somehow my first patch got messed up. Please ignore this.
I will check the problem and send the patches again.

Best Regards,
Sunny Wang

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Sunny Wang via 
groups.io
Sent: Wednesday, March 24, 2021 5:45 PM
To: devel@edk2.groups.io
Cc: Sunny Wang ; Samer El-Haj-Mahmoud 
; Sami Mujawar ; Jeremy 
Linton ; Pete Batard ; Ard Biesheuvel 
; Sunny Wang 
Subject: [edk2-devel] [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build 
UARTs info in ACPI

Changes:  1. Add code to ConfigDxe driver and AcpiTables module to dynamically  
   build either Mini UART or PL011 UART info in ACPI. This fixes the issue 
discussed in https://github.com/pftf/RPi4/issues/118.  2. Cleanup by moving 
duplicate Debug Port 2 table related defines and structures to a newly 
created header file (RpiDebugPort2Table.h).Testing Done:  - Booted to UEFI 
shell and use acpiview command to check the result ofthe different UART 
settings in config.txt (enabling either Mini UARTor PL011) and SPCR, DBG2 
tables and device BTH0 are dynamicallychanged as expected.Cc: Samer 
El-Haj-Mahmoud Cc: Sami Mujawar 
Cc: Jeremy Linton Cc: Pete Batard 
Cc: Ard Biesheuvel Signed-off-by: 
Sunny Wang --- .../RaspberryPi/AcpiTables/AcpiTables.inf
 |   9 +- .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82  
.../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 - 
.../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 + 
.../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +- 
Platform/RaspberryPi/AcpiTables/Uart.asl  |  10 +- 
.../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +- 
.../IndustryStandard/RpiDebugPort2Table.h |  34  8 files changed, 497 
insertions(+), 212 deletions(-) create mode 100644 
Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc rename 
Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%) create mode 
100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc rename 
Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%) create mode 
100644 Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.hdiff 
--git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.infindex d3363a76a1..fc8e927138 
100644--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf+++ 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf@@ -2,7 +2,7 @@ # #  ACPI table 
data and ASL sources required to boot the platform. #-#  Copyright (c) 2019, 
ARM Limited. All rights reserved.+#  Copyright (c) 2019-2021, ARM Limited. All 
rights reserved. #  Copyright (c) 2017, Andrey Warkentin 
 #  Copyright (c) Microsoft Corporation. All rights 
reserved. #@@ -28,12 +28,14 @@   Emmc.asl   Madt.aslc   Fadt.aslc-  Dbg2.aslc+  
Dbg2MiniUart.aslc+  Dbg2Pl011.aslc   Gtdt.aslc   Iort.aslc   Dsdt.asl   
Csrt.aslc-  Spcr.aslc+  SpcrMiniUart.aslc+  SpcrPl011.aslc   Pptt.aslc   
SsdtThermal.asl@@ -71,3 +73,4 @@ [BuildOptions]   GCC:*_*_*_ASL_FLAGS   = 
-vw3133 -vw3150+diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc 
b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslcnew file mode 100644index 
00..c83b978731--- /dev/null+++ 
b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc@@ -0,0 +1,82 @@+/** @file+ 
*+ *  Debug Port Table (DBG2)+ *+ *  Copyright (c) 2019, Pete Batard 
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.+ 
*+ *  SPDX-License-Identifier: BSD-2-Clause-Patent+ *+ **/++#include 
+#include +#include 
+#include +#include 
++#include "AcpiTables.h"++#pragma pack(1)++#define 
RPI_UART_INTERFACE_TYPE 
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART+#define RPI_UART_BASE_ADDRESS
   BCM2836_MINI_UART_BASE_ADDRESS+#define RPI_UART_LENGTH   
  BCM2836_MINI_UART_LENGTH+//+// RPI_UART_STR 
should match the value used Uart.asl+//+#define RPI_UART_STR
{ '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 
'T', 'M', 0x00 }++#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, 
UartAddrLen, UartNameStr) {\+{  

   \+  
EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION, /* UINT8 
Revision */\+  sizeof 
(DBG2_DEBUG_DEVICE_INFORMATION), /* UINT16Length */ 
 \+  NumReg,
 /* UINT8 NumberofGenericAddressRegisters */ \+  
RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,/* UINT16
NameSpaceStringLength */   \+  OFFSET_OF 
(DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString), /* UINT16
NameSpaceStringOffset */   \+  0,   
 

Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

[Laszlo Ersek]

On 03/23/21 18:15, Dong, Guo wrote:
> 
> Hi Laszlo,
> 
> I don't mean currently UEFI payload is already standardized and modularized.
> There are still lots of things to do and I think the AcpiTableDxe patch is 
> the one it needed.
> 
> More information on ideas behind could be found from 
> https://universalpayload.github.io/documentation/spec/spec.html.
> A universal payload interface is proposed between the bootloader and the 
> payload to allow reuse for the same payload for different boot firmware 
> solutions on different platforms. It describes the interface between the 
> bootloader and the payload, including what parameters need pass to payload, 
> how to pass parameters to payload, the parameters format, the payload image 
> format, and the payload boot mode, etc.

Sounds good, but then I would say a new GUID is needed even more.

Laszlo

> 
> I am not saying UefiPayloadPkg would fully follow this proposed interface for 
> now. But we are trying to align with the proposed interface under EDKII 
> framework.
> 
> Thanks,
> Guo
> 
>> -Original Message-
>> From: Laszlo Ersek 
>> Sent: Tuesday, March 23, 2021 9:48 AM
>> To: devel@edk2.groups.io; Dong, Guo ; Liu, Zhiguang
>> ; Ni, Ray 
>> Cc: Wang, Jian J ; Wu, Hao A ;
>> Bi, Dandan ; Liming Gao ;
>> Andrew Fish 
>> Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi
>> table from Hob
>>
>> On 03/23/21 16:45, Guo Dong wrote:
>>>
>>> Add my comments on the ideas behind.
>>> UefiPayloadPkg is not a platform specific package, it tries to provide a
>> generic payload using platform independent Modules. This allows to reuse the
>> payload for different boot firmware solutions (Slim Bootloader, Coreboot, 
>> EDK2
>> bootloader) on different platforms.
>>>
>>> Just like other DXE modules (e.g. variable DXE driver,  firmware performance
>> DXE driver), standardizing and modularizing platform independent modules
>> through a HOB as the AcpiTableDxe patch did to support potential data from
>> PEI/bootloader is a nature way for EDKII modules reuse. Understood the
>> concerns to keep AcpiTableDxe as simple as possible. I think it deserve for 
>> code
>> reuse by adding PEI/bootloader HOB support.
>>
>> I don't understand this argument.
>>
>> (1) Currently, BlSupportDxe expects the ACPI content to come from
>> "SYSTEM_TABLE_INFO.AcpiTableBase" [Include/Guid/SystemTableInfoGuid.h].
>> That header file is at least *moderately* documented (it's better than
>> nothing). Additionally, BlSupportDxe is a DXE-phase component.
>>
>> The patch set removes the handling of "SYSTEM_TABLE_INFO.AcpiTableBase"
>> from BlSupportDxe. That means that platforms currently relying on
>> BlSupportDxe to expose the ACPI content will break (until they start
>> producing the new HOB). I don't see the HOB (with this particular GUID)
>> being produced in UefiPayloadPkg anywhere.
>>
>> (2) The UefiPayloadEntry module ("This is the first module for UEFI
>> payload") still relies on "SYSTEM_TABLE_INFO.AcpiTableBase", for parsing
>> various pieces of information into the "AcpiBoardInfo" structure. So
>> even if the HOB producer phase exposes the ACPI payload via a dedicated
>> HOB, it will only create inconsistency between the information parsed by
>> UefiPayloadEntry (from "SYSTEM_TABLE_INFO.AcpiTableBase") and the OS
>> (which will the ACPI contents from the dedicated HOB).
>>
>> (3) The new HOB's structure (regardless of GUID) is not declared in any
>> MdeModulePkg header file, nor the "MdeModulePkg.dec" file. All the info
>> we have is hidden in the source code:
>>
>>   Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER*)
>> (UINTN)(*((UINT64*)GET_GUID_HOB_DATA (GuidHob)));
>>
>> If a platform's PEI phase actually inteded to produce this new HOB, it
>> couldn't rely on a header file / DEC file.
>>
>> This is actually a *step back* from the SystemTableInfoGuid declaration
>> -- header file and DEC file -- that we currently have in UefiPayloadPkg.
>>
>>
>> So how can this be called "standardizing and modularizing"?
>>
>> You need a new GUID, a new GUID HOB structure (declared in MdeModulePkg
>> DEC and GUID header); you need to spell out the priority order between
>> the HOB and "SYSTEM_TABLE_INFO.AcpiTableBase" for UefiPayloadPkg, and
>> you need to update all driver in UefiPayloadPkg accordingly.
>>
>>
>> I will also not make a secret of my annoyance that, the first time Intel
>> needs such a core extension for some platform feature, it immediately
>> gets all approvals. Whereas, when we needed the exact same feature in
>> OVMF, we struggled for months, if not *years*, to reliably split the
>> ACPI content that OVMF downloaded from QEMU, into blobs that were
>> suitable for the standard ACPI table protocol interfaces. For years I've
>> been telling my colleagues that all this complexity in OVMF's ACPI
>> platform driver is necessary because the EFI_ACPI_TABLE_PROTOCOL
>> implementation in edk2 cannot simply accept a "root pointer", to the
>> ACPI table "f

[edk2-devel] [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI

[Sunny Wang]

Changes:
  1. Add code to ConfigDxe driver and AcpiTables module to dynamically
 build either Mini UART or PL011 UART info in ACPI. This fixes the
 issue discussed in https://github.com/pftf/RPi4/issues/118.
  2. Cleanup by moving duplicate Debug Port 2 table related defines and
 structures to a newly created header file (RpiDebugPort2Table.h).

Testing Done:
  - Booted to UEFI shell and use acpiview command to check the result of
the different UART settings in config.txt (enabling either Mini UART
or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
changed as expected.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Pete Batard 
Cc: Ard Biesheuvel 
Signed-off-by: Sunny Wang 
---
 .../RaspberryPi/AcpiTables/AcpiTables.inf |   9 +-
 .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 
 .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 -
 .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +
 .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +-
 Platform/RaspberryPi/AcpiTables/Uart.asl  |  10 +-
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +-
 .../IndustryStandard/RpiDebugPort2Table.h |  34 
 8 files changed, 497 insertions(+), 212 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
 rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
 rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
 create mode 100644 
Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf 
b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d3363a76a1..fc8e927138 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -2,7 +2,7 @@
 #
 #  ACPI table data and ASL sources required to boot the platform.
 #
-#  Copyright (c) 2019, ARM Limited. All rights reserved.
+#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
 #  Copyright (c) 2017, Andrey Warkentin 
 #  Copyright (c) Microsoft Corporation. All rights reserved.
 #
@@ -28,12 +28,14 @@
   Emmc.asl
   Madt.aslc
   Fadt.aslc
-  Dbg2.aslc
+  Dbg2MiniUart.aslc
+  Dbg2Pl011.aslc
   Gtdt.aslc
   Iort.aslc
   Dsdt.asl
   Csrt.aslc
-  Spcr.aslc
+  SpcrMiniUart.aslc
+  SpcrPl011.aslc
   Pptt.aslc
   SsdtThermal.asl

@@ -71,3 +73,4 @@

 [BuildOptions]
   GCC:*_*_*_ASL_FLAGS   = -vw3133 -vw3150
+
diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc 
b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
new file mode 100644
index 00..c83b978731
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
@@ -0,0 +1,82 @@
+/** @file
+ *
+ *  Debug Port Table (DBG2)
+ *
+ *  Copyright (c) 2019, Pete Batard 
+ *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "AcpiTables.h"
+
+#pragma pack(1)
+
+#define RPI_UART_INTERFACE_TYPE 
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
+#define RPI_UART_BASE_ADDRESS   
BCM2836_MINI_UART_BASE_ADDRESS
+#define RPI_UART_LENGTH 
BCM2836_MINI_UART_LENGTH
+//
+// RPI_UART_STR should match the value used Uart.asl
+//
+#define RPI_UART_STR{ '\\', '_', 'S', 'B', 
'.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
+
+#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, 
UartNameStr) {\
+{  
   \
+  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION, /* UINT8 
Revision */\
+  sizeof (DBG2_DEBUG_DEVICE_INFORMATION), /* 
UINT16Length */  \
+  NumReg, /* UINT8 
NumberofGenericAddressRegisters */ \
+  RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,/* 
UINT16NameSpaceStringLength */   \
+  OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString), /* 
UINT16NameSpaceStringOffset */   \
+  0,  /* 
UINT16OemDataLength */   \
+  0,  /* 
UINT16OemDataOffset */   \
+  EFI_ACPI_DBG2_PORT_TYPE_SERIAL, /* 
UINT16Port Type */   \
+  SubType,/* 
UINT16Port Subtype */\
+  {EFI_ACPI_RESERVED

[edk2-devel] [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS

[Sunny Wang]

Merge changes in edk2-platforms-raspberrypi-pl011-bth-noflow.diff in
https://github.com/worproject/RPi-Bluetooth-Testing/ for enabling
Bluetooth and serial port (Mini UART) in Windows OS.

Testing Done:
  - Successfully booted Windows 10 (20279.1) on SD (made by WOR) with
the RPi-Windows-Drivers release ver 0.5 downloaded from
https://github.com/worproject/RPi-Windows-Drivers/releases
and checked that both Bluetooth and serial port (Mini UART) can
work fine.

Cc: Samer El-Haj-Mahmoud 
Cc: Sami Mujawar 
Cc: Jeremy Linton 
Cc: Pete Batard 
Cc: Ard Biesheuvel 
Signed-off-by: Sunny Wang 
---
 Platform/RaspberryPi/AcpiTables/Uart.asl | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl 
b/Platform/RaspberryPi/AcpiTables/Uart.asl
index 8ce297078d..e3165911a6 100644
--- a/Platform/RaspberryPi/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
@@ -30,6 +30,12 @@ Device (URT0)
   {
 MEMORY32FIXED (ReadWrite, 0, BCM2836_PL011_UART_LENGTH, RMEM)
 Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { 
BCM2836_PL011_UART_INTERRUPT }
+
+PinFunction (Exclusive, PullDown, BCM_ALT3, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 32, 33 }
+
+// fake the CTS signal as we don't support HW flow control yet
+// BCM_ALT2 is set as output (low) by default
+PinFunction (Exclusive, PullNone, BCM_ALT2, "\\_SB.GDV0.GPI0", 0, 
ResourceConsumer, , ) { 31 }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -142,7 +148,7 @@ Device(BTH0)
   //
   // RPIQ connection for BT_ON/OFF
   //
-  GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, 
ResourceConsumer, , ) { 128 }
+  //GpioIO (Shared, PullUp, 0, 0, IoRestrictionNone, "\\_SB.GDV0.RPIQ", 0, 
ResourceConsumer, , ) { 128 }
 })
 Return (RBUF)
   }
-- 
2.31.0.windows.1



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