date:20230214

[edk2-devel] 回复: [PATCH] MdeModulePkg/Library: PcdAcpiS3Enable set FALSE cause Assert

2023-02-14 Thread gaoliming via groups.io

Xueshengfeng:
  Create PR https://github.com/tianocore/edk2/pull/4047 for this patch. 

Thanks
Liming
> -邮件原件-
> 发件人: xueshengfeng 
> 发送时间: 2023年2月15日 12:18
> 收件人: 'gaoliming' ; devel@edk2.groups.io;
> jian.j.w...@intel.com
> 抄送: heinrich.schucha...@canonical.com; edhaya.chand...@arm.com;
> samer.el-haj-mahm...@arm.com; 'lijun10x' ; 'Sunny
> Wang' 
> 主题: 答复: [PATCH] MdeModulePkg/Library: PcdAcpiS3Enable set FALSE
> cause Assert
> 
> Hi  Liming,
> 
> Yes, it is good to us if catch this stable tag 202302.
> Thank you very much
> 
> -邮件原件-
> 发件人: gaoliming [mailto:gaolim...@byosoft.com.cn]
> 发送时间: 2023年2月15日 11:59
> 收件人: 'xueshengfeng'; devel@edk2.groups.io; jian.j.w...@intel.com
> 抄送: heinrich.schucha...@canonical.com; edhaya.chand...@arm.com;
> samer.el-haj-mahm...@arm.com; 'lijun10x'; 'Sunny Wang'
> 主题: 回复: [PATCH] MdeModulePkg/Library: PcdAcpiS3Enable set FALSE
> cause Assert
> 
> Xueshengfeng:
>   This patch is good to me. Reviewed-by: Liming Gao
> 
> 
>   Does this patch plan to catch this stable tag 202302?
> 
> Thanks
> Liming
> > -邮件原件-
> > 发件人: xueshengfeng 
> > 发送时间: 2023年2月2日 14:21
> > 收件人: devel@edk2.groups.io; jian.j.w...@intel.com;
> > gaolim...@byosoft.com.cn
> > 抄送: heinrich.schucha...@canonical.com; edhaya.chand...@arm.com;
> > samer.el-haj-mahm...@arm.com; lijun10x ; Sunny
> > Wang 
> > 主题: [PATCH] MdeModulePkg/Library: PcdAcpiS3Enable set FALSE cause
> > Assert
> >
> > From: lijun10x 
> >
> > Some platforms don't support S3 with PcdAcpiS3Enable set as False.
> > Debug mode bios will ASSERT at this time as Follows.
> > ASSERT_RETURN_ERROR (Status = Out of Resources)
> > DXE_ASSERT!:
> > Edk2\MdePkg\Library\BaseS3PciSegmentLib\S3PciSegmentLib.c
> > (61): !(((INTN)(RETURN_STATUS)(Status)) < 0)
> >
> > Steps to reproduce the issue:
> > 1.Set PcdAcpiS3Enable to FALSE.
> > 2.Build the bios in debug mode.
> > 3.Power on and Check the serial log.
> > Note: Prerequisite is that S3PciSegmentLib is Called and
> > the caller's code is run.
> >
> > Root Cause:
> > S3PciSegmentLib call S3BootScriptLib controlled by PcdAcpiS3Enable.
> > If PcdAcpiS3Enable set as false, S3BootScriptLib will return error
> > status(Out of Resources).
> > S3PciSegmentLib will ASSERT if S3BootScriptLib return error.
> >
> > Solution:
> > Make S3BootScriptLib return success if PcdAcpiS3Enable was disabled,
> > which behave as a null S3BootScriptLib instance which just return success
> > for no action is required to do.
> >
> > Signed-off-by: JunX1 Li 
> > Cc: Liming Gao 
> > Cc: Sunny Wang 
> > Cc: Heinrich Schuchardt 
> > Cc: G Edhaya Chandran 
> > Cc: Samer El-Haj-Mahmoud 
> > ---
> >  .../PiDxeS3BootScriptLib/BootScriptSave.c | 132
> ++
> >  1 file changed, 108 insertions(+), 24 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > index f8d4983d81..b9b8562f14 100644
> > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> > @@ -1004,7 +1004,7 @@ S3BootScriptCloseTable (
> >@param Buffer  The source buffer from which to write data.
> >
> >@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> > table do operation.
> > -  @retval RETURN_SUCCESS   Opcode is added.
> > +  @retval RETURN_SUCCESS   Opcode is added or no action is
> > required as ACPI S3 was disabled.
> >  **/
> >  RETURN_STATUS
> >  EFIAPI
> > @@ -1021,6 +1021,10 @@ S3BootScriptSaveIoWrite (
> >UINT8 WidthInByte;
> >EFI_BOOT_SCRIPT_IO_WRITE  ScriptIoWrite;
> >
> > +  if (!mS3BootScriptAcpiS3Enable) {
> > +return RETURN_SUCCESS;
> > +  }
> > +
> >WidthInByte = (UINT8)(0x01 << (Width & 0x03));
> >
> >//
> > @@ -1064,7 +1068,7 @@ S3BootScriptSaveIoWrite (
> >@param DataMask  A pointer to the data mask to be AND-ed with the
> > data read from the register
> >
> >@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> > table do operation.
> > -  @retval RETURN_SUCCESS   Opcode is added.
> > +  @retval RETURN_SUCCESS   Opcode is added or no action is
> > required as ACPI S3 was disabled.
> >  **/
> >  RETURN_STATUS
> >  EFIAPI
> > @@ -1080,6 +1084,10 @@ S3BootScriptSaveIoReadWrite (
> >UINT8  WidthInByte;
> >EFI_BOOT_SCRIPT_IO_READ_WRITE  ScriptIoReadWrite;
> >
> > +  if (!mS3BootScriptAcpiS3Enable) {
> > +return RETURN_SUCCESS;
> > +  }
> > +
> >WidthInByte = (UINT8)(0x01 << (Width & 0x03));
> >Length  = (UINT8)(sizeof (EFI_BOOT_SCRIPT_IO_READ_WRITE) +
> > (WidthInByte * 2));
> >
> > @@ -1114,7 +1122,7 @@ S3BootScriptSaveIoReadWrite (
> >@param Buffer  The source buffer from which to write the data.
> >
> >@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> > table do operation.
> > -  @retval RETURN_SUCCESS   Opcode is added.
> > +  @retval RETURN_SUCCESS

Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile information in build report

2023-02-14 Thread Yuwei Chen

Hi Mike, thanks for reminder.

Hi Willy, currently, BaseTools related changes will be implemented on the 
edk2-basetools repo. Please send the patch based on the edk2-basetools repo~ 

Thanks,
Christine

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, February 15, 2023 11:43 AM
> To: devel@edk2.groups.io; Chen, Christine ;
> Palomino Sosa, Guillermo A 
> Cc: Feng, Bob C ; Gao, Liming
> ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile
> information in build report
> 
> Has this been reviewed for edk2-basetools repo?
> 
> Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Yuwei
> > Chen
> > Sent: Tuesday, February 14, 2023 6:44 PM
> > To: Palomino Sosa, Guillermo A ;
> > devel@edk2.groups.io
> > Cc: Feng, Bob C ; Gao, Liming
> > 
> > Subject: Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile
> > information in build report
> >
> > Reviewed-by: Yuwei Chen 
> >
> > > -Original Message-
> > > From: Palomino Sosa, Guillermo A
> > > 
> > > Sent: Tuesday, February 7, 2023 11:07 AM
> > > To: devel@edk2.groups.io
> > > Cc: Feng, Bob C ; Gao, Liming
> > > ; Chen, Christine 
> > > Subject: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile
> > > information in build report
> > >
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2850
> > >
> > > Add "-Y REPORT_INFO" option to build command to generate compile
> > > information as part of BuildReport.
> > > This option generates files to be used by external tools as IDE's to
> > > enhance functionality.
> > > Files are created inside build folder:
> > > ///CompileInfo
> > >
> > > Files created:
> > > * compile_commands.json - Compilation Database. To be used by IDE's
> > >   to enable advance features
> > > * cscope.files - List of files used in compilation. Used by Cscope to 
> > > parse
> > >   C code and provide browse functionality.
> > > * module_report.json - Module data form buildReport in Json format.
> > >
> > > Signed-off-by: Guillermo Antonio Palomino Sosa
> > > 
> > > ---
> > >  BaseTools/Source/Python/build/BuildReport.py  | 139
> > > +++-
> > >  BaseTools/Source/Python/build/buildoptions.py |   4 +-
> > >  2 files changed, 140 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/BaseTools/Source/Python/build/BuildReport.py
> > > b/BaseTools/Source/Python/build/BuildReport.py
> > > index 468772930c..33b43d471f 100644
> > > --- a/BaseTools/Source/Python/build/BuildReport.py
> > > +++ b/BaseTools/Source/Python/build/BuildReport.py
> > > @@ -10,6 +10,8 @@
> > >
> > >  ## Import Modules
> > >  #
> > > +import json
> > > +from pathlib import Path
> > >  import Common.LongFilePathOs as os
> > >  import re
> > >  import platform
> > > @@ -41,6 +43,7 @@ from Common.DataType import *  import
> collections
> > > from Common.Expression import *  from GenFds.AprioriSection import
> > > DXE_APRIORI_GUID, PEI_APRIORI_GUID
> > > +from AutoGen.IncludesAutoGen import IncludesAutoGen
> > >
> > >  ## Pattern to extract contents in EDK DXS files
> > > gDxsDependencyPattern =
> > > re.compile(r"DEPENDENCY_START(.+)DEPENDENCY_END", re.DOTALL)
> @@ -
> > > 2298,6 +2301,10 @@ class BuildReport(object):
> > >  def GenerateReport(self, BuildDuration, AutoGenTime, MakeTime,
> > > GenFdsTime):
> > >  if self.ReportFile:
> > >  try:
> > > +
> > > +if "COMPILE_INFO" in self.ReportType:
> > > +self.GenerateCompileInfo()
> > > +
> > >  File = []
> > >  for (Wa, MaList) in self.ReportList:
> > >  PlatformReport(Wa, MaList,
> > > self.ReportType).GenerateReport(File, BuildDuration, AutoGenTime,
> > > MakeTime, GenFdsTime, self.ReportType) @@ -2310,7 +2317,137 @@
> class
> > > BuildReport(object):
> > >  EdkLogger.error("BuildReport", CODE_ERROR, "Unknown
> > > fatal error when generating build report",
> > > ExtraData=self.ReportFile,
> > > RaiseError=False)
> > >  EdkLogger.quiet("(Python %s on %s\n%s)" %
> > > (platform.python_version(), sys.platform, traceback.format_exc()))
> > >
> > > +
> > > +##
> > > +# Generates compile data files to be used by external tools.
> > > +# Compile information will be generated in
> > > ///CompileInfo
> > > +# Files generated: compile_commands.json, cscope.files,
> > > modules_report.json
> > > +#
> > > +# @param selfThe object pointer
> > > +#
> > > +def GenerateCompileInfo(self):
> > > +try:
> > > +# Lists for the output elements
> > > +compile_commands = []
> > > +used_files = set()
> > > +module_report = []
> > > +
> > > +for (Wa, MaList) in self.ReportList:
> > > +# Obtain list of all processed Workspace files
> > > +for file_path in Wa._GetMetaFiles(Wa.BuildTarget,
> Wa.ToolChain):
> > > +

[edk2-devel] [PATCH v3 3/3] UsbNetworkPkg/UsbCdcNcm: Add USB Cdc NCM devices support

2023-02-14 Thread RichardHo [何明忠] via groups . io

This driver provides UEFI driver for USB CDC NCM device

Signed-off-by: Richard Ho 
Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Michael Kubacki 
Cc: Zhiguang Liu 
Cc: Liming Gao 
Reviewed-by: Tony Lo 
---
 UsbNetworkPkg/UsbCdcNcm/ComponentName.c  | 170 
 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.c  | 506 
 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.h  | 245 ++
 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf|  42 +
 UsbNetworkPkg/UsbCdcNcm/UsbNcmFunction.c | 962 +++
 5 files changed, 1925 insertions(+)
 create mode 100644 UsbNetworkPkg/UsbCdcNcm/ComponentName.c
 create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.c
 create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.h
 create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbCdcNcm.inf
 create mode 100644 UsbNetworkPkg/UsbCdcNcm/UsbNcmFunction.c

diff --git a/UsbNetworkPkg/UsbCdcNcm/ComponentName.c 
b/UsbNetworkPkg/UsbCdcNcm/ComponentName.c
new file mode 100644
index 00..3cf3dc222d
--- /dev/null
+++ b/UsbNetworkPkg/UsbCdcNcm/ComponentName.c
@@ -0,0 +1,170 @@
+/** @file
+  This file contains code for USB Ncm Driver Component Name definitions
+
+  Copyright (c) 2023, American Megatrends International LLC. All rights 
reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include "UsbCdcNcm.h"
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE  
gUsbNcmDriverNameTable[] = {
+  {
+"eng;en",
+L"USB NCM Driver"
+  },
+  {
+NULL,
+NULL
+  }
+};
+
+EFI_STATUS
+EFIAPI
+UsbNcmComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL  *This,
+  IN  CHAR8*Language,
+  OUT CHAR16   **DriverName
+  );
+
+EFI_STATUS
+EFIAPI
+UsbNcmComponentNameGetControllerName (
+  IN EFI_COMPONENT_NAME_PROTOCOL  *This,
+  IN EFI_HANDLE   Controller,
+  IN EFI_HANDLE   ChildHandleOPTIONAL,
+  IN CHAR8*Language,
+  OUT CHAR16  **ControllerName
+  );
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL  
gUsbNcmComponentName = {
+  UsbNcmComponentNameGetDriverName,
+  UsbNcmComponentNameGetControllerName,
+  "eng"
+};
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME2_PROTOCOL  
gUsbNcmComponentName2 = {
+  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME)UsbNcmComponentNameGetDriverName,
+  
(EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME)UsbNcmComponentNameGetControllerName,
+  "en"
+};
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language,
+  then EFI_UNSUPPORTED is returned.
+
+  @param[in]  This  A pointer to the EFI_COMPONENT_NAME2_PROTOCOL 
or
+EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param[in]  Language  A pointer to a Null-terminated ASCII string
+array indicating the language. This is the
+language of the driver name that the caller is
+requesting, and it must match one of the
+languages specified in SupportedLanguages. The
+number of languages supported by a driver is up
+to the driver writer. Language is specified
+in RFC 4646 or ISO 639-2 language code format.
+  @param[out] DriverNameA pointer to the Unicode string to return.
+This Unicode string is the name of the
+driver specified by This in the language
+specified by Language.
+
+  @retval EFI_SUCCESS   The Unicode string for the Driver specified by
+This and the language specified by Language was
+returned in DriverName.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+  @retval EFI_UNSUPPORTED   The driver specified by This does not support
+the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+UsbNcmComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL  *This,
+  IN  CHAR8*Language,
+  OUT CHAR16   **DriverName
+  )
+{
+  return LookupUnicodeString2 (
+   Language,
+   This->SupportedLanguages,
+   gUsbNcmDriverNameTable,
+   DriverName,
+   (BOOLEAN)(This == )
+   );
+}
+
+/**
+  Retrieves a Unicode string that is the user readable name of the controller
+  that is being

[edk2-devel] [PATCH v3 2/3] UsbNetworkPkg/UsbCdcEcm: Add USB Cdc ECM devices support

2023-02-14 Thread RichardHo [何明忠] via groups . io

This driver provides UEFI driver for USB CDC ECM device

Signed-off-by: Richard Ho 
Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Michael Kubacki 
Cc: Zhiguang Liu 
Cc: Liming Gao 
Reviewed-by: Tony Lo 
---
 UsbNetworkPkg/UsbCdcEcm/ComponentName.c  | 170 +
 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.c  | 502 +
 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.h  | 211 ++
 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf|  42 ++
 UsbNetworkPkg/UsbCdcEcm/UsbEcmFunction.c | 876 +++
 5 files changed, 1801 insertions(+)
 create mode 100644 UsbNetworkPkg/UsbCdcEcm/ComponentName.c
 create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.c
 create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.h
 create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbCdcEcm.inf
 create mode 100644 UsbNetworkPkg/UsbCdcEcm/UsbEcmFunction.c

diff --git a/UsbNetworkPkg/UsbCdcEcm/ComponentName.c 
b/UsbNetworkPkg/UsbCdcEcm/ComponentName.c
new file mode 100644
index 00..e37eecf229
--- /dev/null
+++ b/UsbNetworkPkg/UsbCdcEcm/ComponentName.c
@@ -0,0 +1,170 @@
+/** @file
+  This file contains code for USB Ecm Driver Component Name definitions
+
+  Copyright (c) 2023, American Megatrends International LLC. All rights 
reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include "UsbCdcEcm.h"
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE  
gUsbEcmDriverNameTable[] = {
+  {
+"eng;en",
+L"USB ECM Driver"
+  },
+  {
+NULL,
+NULL
+  }
+};
+
+EFI_STATUS
+EFIAPI
+UsbEcmComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL  *This,
+  IN  CHAR8*Language,
+  OUT CHAR16   **DriverName
+  );
+
+EFI_STATUS
+EFIAPI
+UsbEcmComponentNameGetControllerName (
+  IN EFI_COMPONENT_NAME_PROTOCOL  *This,
+  IN EFI_HANDLE   Controller,
+  IN EFI_HANDLE   ChildHandleOPTIONAL,
+  IN CHAR8*Language,
+  OUT CHAR16  **ControllerName
+  );
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL  
gUsbEcmComponentName = {
+  UsbEcmComponentNameGetDriverName,
+  UsbEcmComponentNameGetControllerName,
+  "eng"
+};
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME2_PROTOCOL  
gUsbEcmComponentName2 = {
+  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME)UsbEcmComponentNameGetDriverName,
+  
(EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME)UsbEcmComponentNameGetControllerName,
+  "en"
+};
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language,
+  then EFI_UNSUPPORTED is returned.
+
+  @param[in]  This  A pointer to the EFI_COMPONENT_NAME2_PROTOCOL 
or
+EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param[in]  Language  A pointer to a Null-terminated ASCII string
+array indicating the language. This is the
+language of the driver name that the caller is
+requesting, and it must match one of the
+languages specified in SupportedLanguages. The
+number of languages supported by a driver is up
+to the driver writer. Language is specified
+in RFC 4646 or ISO 639-2 language code format.
+  @param[out] DriverNameA pointer to the Unicode string to return.
+This Unicode string is the name of the
+driver specified by This in the language
+specified by Language.
+
+  @retval EFI_SUCCESS   The Unicode string for the Driver specified by
+This and the language specified by Language was
+returned in DriverName.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+  @retval EFI_UNSUPPORTED   The driver specified by This does not support
+the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+UsbEcmComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL  *This,
+  IN  CHAR8*Language,
+  OUT CHAR16   **DriverName
+  )
+{
+  return LookupUnicodeString2 (
+   Language,
+   This->SupportedLanguages,
+   gUsbEcmDriverNameTable,
+   DriverName,
+   (BOOLEAN)(This == )
+   );
+}
+
+/**
+  Retrieves a Unicode string that is the user readable name of the controller
+  that is being

[edk2-devel] 回复: [PATCH] MdeModulePkg/Library: PcdAcpiS3Enable set FALSE cause Assert

2023-02-14 Thread gaoliming via groups.io

Xueshengfeng:
  This patch is good to me. Reviewed-by: Liming Gao 

  Does this patch plan to catch this stable tag 202302?

Thanks
Liming
> -邮件原件-
> 发件人: xueshengfeng 
> 发送时间: 2023年2月2日 14:21
> 收件人: devel@edk2.groups.io; jian.j.w...@intel.com;
> gaolim...@byosoft.com.cn
> 抄送: heinrich.schucha...@canonical.com; edhaya.chand...@arm.com;
> samer.el-haj-mahm...@arm.com; lijun10x ; Sunny
> Wang 
> 主题: [PATCH] MdeModulePkg/Library: PcdAcpiS3Enable set FALSE cause
> Assert
> 
> From: lijun10x 
> 
> Some platforms don't support S3 with PcdAcpiS3Enable set as False.
> Debug mode bios will ASSERT at this time as Follows.
> ASSERT_RETURN_ERROR (Status = Out of Resources)
> DXE_ASSERT!:
> Edk2\MdePkg\Library\BaseS3PciSegmentLib\S3PciSegmentLib.c
> (61): !(((INTN)(RETURN_STATUS)(Status)) < 0)
> 
> Steps to reproduce the issue:
> 1.Set PcdAcpiS3Enable to FALSE.
> 2.Build the bios in debug mode.
> 3.Power on and Check the serial log.
> Note: Prerequisite is that S3PciSegmentLib is Called and
> the caller's code is run.
> 
> Root Cause:
> S3PciSegmentLib call S3BootScriptLib controlled by PcdAcpiS3Enable.
> If PcdAcpiS3Enable set as false, S3BootScriptLib will return error
> status(Out of Resources).
> S3PciSegmentLib will ASSERT if S3BootScriptLib return error.
> 
> Solution:
> Make S3BootScriptLib return success if PcdAcpiS3Enable was disabled,
> which behave as a null S3BootScriptLib instance which just return success
> for no action is required to do.
> 
> Signed-off-by: JunX1 Li 
> Cc: Liming Gao 
> Cc: Sunny Wang 
> Cc: Heinrich Schuchardt 
> Cc: G Edhaya Chandran 
> Cc: Samer El-Haj-Mahmoud 
> ---
>  .../PiDxeS3BootScriptLib/BootScriptSave.c | 132 ++
>  1 file changed, 108 insertions(+), 24 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> index f8d4983d81..b9b8562f14 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> @@ -1004,7 +1004,7 @@ S3BootScriptCloseTable (
>@param Buffer  The source buffer from which to write data.
> 
>@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> table do operation.
> -  @retval RETURN_SUCCESS   Opcode is added.
> +  @retval RETURN_SUCCESS   Opcode is added or no action is
> required as ACPI S3 was disabled.
>  **/
>  RETURN_STATUS
>  EFIAPI
> @@ -1021,6 +1021,10 @@ S3BootScriptSaveIoWrite (
>UINT8 WidthInByte;
>EFI_BOOT_SCRIPT_IO_WRITE  ScriptIoWrite;
> 
> +  if (!mS3BootScriptAcpiS3Enable) {
> +return RETURN_SUCCESS;
> +  }
> +
>WidthInByte = (UINT8)(0x01 << (Width & 0x03));
> 
>//
> @@ -1064,7 +1068,7 @@ S3BootScriptSaveIoWrite (
>@param DataMask  A pointer to the data mask to be AND-ed with the
> data read from the register
> 
>@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> table do operation.
> -  @retval RETURN_SUCCESS   Opcode is added.
> +  @retval RETURN_SUCCESS   Opcode is added or no action is
> required as ACPI S3 was disabled.
>  **/
>  RETURN_STATUS
>  EFIAPI
> @@ -1080,6 +1084,10 @@ S3BootScriptSaveIoReadWrite (
>UINT8  WidthInByte;
>EFI_BOOT_SCRIPT_IO_READ_WRITE  ScriptIoReadWrite;
> 
> +  if (!mS3BootScriptAcpiS3Enable) {
> +return RETURN_SUCCESS;
> +  }
> +
>WidthInByte = (UINT8)(0x01 << (Width & 0x03));
>Length  = (UINT8)(sizeof (EFI_BOOT_SCRIPT_IO_READ_WRITE) +
> (WidthInByte * 2));
> 
> @@ -1114,7 +1122,7 @@ S3BootScriptSaveIoReadWrite (
>@param Buffer  The source buffer from which to write the data.
> 
>@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> table do operation.
> -  @retval RETURN_SUCCESS   Opcode is added.
> +  @retval RETURN_SUCCESS   Opcode is added or no action is
> required as ACPI S3 was disabled.
>  **/
>  RETURN_STATUS
>  EFIAPI
> @@ -1130,6 +1138,10 @@ S3BootScriptSaveMemWrite (
>UINT8  WidthInByte;
>EFI_BOOT_SCRIPT_MEM_WRITE  ScriptMemWrite;
> 
> +  if (!mS3BootScriptAcpiS3Enable) {
> +return RETURN_SUCCESS;
> +  }
> +
>WidthInByte = (UINT8)(0x01 << (Width & 0x03));
> 
>//
> @@ -1174,7 +1186,7 @@ S3BootScriptSaveMemWrite (
>@param DataMask  A pointer to the data mask to be AND-ed with the
> data read from the register.
> 
>@retval RETURN_OUT_OF_RESOURCES  Not enough memory for the
> table do operation.
> -  @retval RETURN_SUCCESS   Opcode is added.
> +  @retval RETURN_SUCCESS   Opcode is added or no action is
> required as ACPI S3 was disabled.
>  **/
>  RETURN_STATUS
>  EFIAPI
> @@ -1190,6 +1202,10 @@ S3BootScriptSaveMemReadWrite (
>UINT8   WidthInByte;
>EFI_BOOT_SCRIPT_MEM_READ_WRITE  ScriptMemReadWrite;
> 
> +  if (!mS3BootScriptAcpiS3Enable) {
> +return RETURN_SUCCESS;
> +  }
> +
>WidthInByte = (UINT8)(0x01 << (Width &

回复: [edk2-devel] [PATCH v1 1/1] BaseTools: Update WindowsVsToolChain plugin

2023-02-14 Thread gaoliming via groups.io

Joey:

 This patch is good to me. Reviewed-by: Liming Gao 

 

 Dose this patch plan to catch this stable tag 202302?

 

Thanks

Liming

发件人: Joey Vagedes  
发送时间: 2023年2月7日 0:53
收件人: Liming Gao ; Michael D Kinney 
; Bob Feng ; Yuwei Chen 

抄送: devel@edk2.groups.io; Michael Kubacki 
主题: Re: [edk2-devel] [PATCH v1 1/1] BaseTools: Update WindowsVsToolChain plugin

 

I received an Acked-by, but would like to request a Reviewed-by from a 
BaseTools maintainer.
@gaolim...@byosoft.com.cn   I would like to 
request that this change be included in the stable tag.

 

This change updates edk2-pytool-library and edk2-pytool-extensions to the 
latest release, which
brings in additional features and bug fixes for those building via edk2-pytools.

 

There are no breaking changes, and no changes in the required python version.

 

One of the changes made in edk2-pytool-library did result in a change in 
interface, which was necessary
to provide a better user experience in handling errors for said function, 
however this PR provides a tested
change (used by Project MU) for a script using the method in BaseTools.

 

This change is also critical for the edk2-pytools project itself as that 
project can not successfully perform
integration checks (which happen on every change to pytools) for edk2 on future 
PRs until this interface
is changed.

 

Thanks,

Joey

 

On Fri, Feb 3, 2023 at 7:43 PM Michael Kubacki mailto:mikub...@linux.microsoft.com> > wrote:

Acked-by: Michael Kubacki mailto:michael.kuba...@microsoft.com> >

On 2/1/2023 3:22 PM, Joey Vagedes wrote:
> This patch updates edk2-pytool-library dependency to v0.13.0, which has
> an interface change to FindWithVsWhere. The BaseTools plugin uses this
> function, so it is being updated to account for the interface change.
> 
> Cc: Bob Feng mailto:bob.c.f...@intel.com> >
> Cc: Liming Gao mailto:gaolim...@byosoft.com.cn> >
> Cc: Yuwei Chen mailto:yuwei.c...@intel.com> >
> Cc: Michael D Kinney   >
> 
> Signed-off-by: Joey Vagedes   >
> ---
>   BaseTools/Plugin/WindowsVsToolChain/WindowsVsToolChain.py | 16 
> 
>   pip-requirements.txt  |  4 ++--
>   2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/BaseTools/Plugin/WindowsVsToolChain/WindowsVsToolChain.py 
> b/BaseTools/Plugin/WindowsVsToolChain/WindowsVsToolChain.py
> index 0fba2c1b5325..615b5ed6d131 100644
> --- a/BaseTools/Plugin/WindowsVsToolChain/WindowsVsToolChain.py
> +++ b/BaseTools/Plugin/WindowsVsToolChain/WindowsVsToolChain.py
> @@ -177,15 +177,23 @@ class WindowsVsToolChain(IUefiBuildPlugin):
>   
> 
>   def _get_vs_install_path(self, vs_version, varname):
> 
>   # check if already specified
> 
> -path = shell_environment.GetEnvironment().get_shell_var(varname)
> 
> +path = None
> 
> +if varname is not None:
> 
> +path = shell_environment.GetEnvironment().get_shell_var(varname)
> 
> +
> 
>   if(path is None):
> 
>   # Not specified...find latest
> 
> -(rc, path) = FindWithVsWhere(vs_version=vs_version)
> 
> -if rc == 0 and path is not None and os.path.exists(path):
> 
> +try:
> 
> +path = FindWithVsWhere(vs_version=vs_version)
> 
> +except (EnvironmentError, ValueError, RuntimeError) as e:
> 
> +self.Logger.error(str(e))
> 
> +return None
> 
> +
> 
> +if path is not None and os.path.exists(path):
> 
>   self.Logger.debug("Found VS instance for %s", vs_version)
> 
>   else:
> 
>   self.Logger.error(
> 
> -"Failed to find VS instance with VsWhere (%d)" % rc)
> 
> +f"VsWhere successfully executed, but could not find VS 
> instance for {vs_version}.")
> 
>   return path
> 
>   
> 
>   def _get_vc_version(self, path, varname):
> 
> diff --git a/pip-requirements.txt b/pip-requirements.txt
> index 4ffca8cf..d3256ff1ade7 100644
> --- a/pip-requirements.txt
> +++ b/pip-requirements.txt
> @@ -12,8 +12,8 @@
>   # https://www.python.org/dev/peps/pep-0440/#version-specifiers
> 
>   ##
> 
>   
> 
> -edk2-pytool-library==0.12.1
> 
> -edk2-pytool-extensions~=0.20.0
> 
> +edk2-pytool-library==0.13.1
> 
> +edk2-pytool-extensions~=0.21.2
> 
>   edk2-basetools==0.1.39
> 
>   antlr4-python3-runtime==4.7.1
> 
>   lcov-cobertura==2.0.2
> 



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

回复: [edk2-devel] [PATCH] MdeModulePkg/Variable: Attribute combination should return EFI_UNSUPPORTED

2023-02-14 Thread gaoliming via groups.io

Create PR https://github.com/tianocore/edk2/pull/4045 to merge it. 

Thanks
Liming

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Sunny Wang
> 发送时间: 2023年2月13日 18:43
> 收件人: Stuart Yoder ; devel@edk2.groups.io
> 抄送: gaolim...@byosoft.com.cn; hao.a...@intel.com; Sunny Wang
> 
> 主题: Re: [edk2-devel] [PATCH] MdeModulePkg/Variable: Attribute
> combination should return EFI_UNSUPPORTED
> 
> Looks good to me. Thanks, Stuart.
> Reviewed-by: Sunny Wang 
> 
> -Original Message-
> From: Stuart Yoder 
> Sent: 09 February 2023 22:31
> To: devel@edk2.groups.io
> Cc: gaolim...@byosoft.com.cn; hao.a...@intel.com; Sunny Wang
> 
> Subject: [PATCH] MdeModulePkg/Variable: Attribute combination should
> return EFI_UNSUPPORTED
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4341
> 
> Commit 21320ef66989 broke some tests in the AuthVar_Conf test
> in edk2-test.  There are 2 testcases that invoke SetVariable
> with the following attribute value:
> 
> (EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
> 
> EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and the UEFI
> spec
> says this should return EFI_UNSUPPORTED.
> 
> Cc: Liming Gao 
> Cc: Hao A Wu 
> Cc: Sunny Wang 
> 
> Signed-off-by: Stuart Yoder 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 6c1a3440ac..14c176887a 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2676,7 +2676,11 @@ VariableServiceSetVariable (
>  //
> 
>  // Only EFI_VARIABLE_NON_VOLATILE attribute is invalid
> 
>  //
> 
> -return EFI_INVALID_PARAMETER;
> 
> +if ((Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) != 0)
> {
> 
> +  return EFI_UNSUPPORTED;
> 
> +} else {
> 
> +  return EFI_INVALID_PARAMETER;
> 
> +}
> 
>} else if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> 
>  if (!mVariableModuleGlobal->VariableGlobal.AuthSupport) {
> 
>//
> 
> --
> 2.34.1
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
recipient,
> please notify the sender immediately and do not disclose the contents to
any
> other person, use it for any purpose, or store or copy the information in
any
> medium. Thank you.
> 
> 
> 
> 





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

Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile information in build report

2023-02-14 Thread Michael D Kinney

Has this been reviewed for edk2-basetools repo?

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Yuwei Chen
> Sent: Tuesday, February 14, 2023 6:44 PM
> To: Palomino Sosa, Guillermo A ; 
> devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming 
> Subject: Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile 
> information in build report
> 
> Reviewed-by: Yuwei Chen 
> 
> > -Original Message-
> > From: Palomino Sosa, Guillermo A 
> > Sent: Tuesday, February 7, 2023 11:07 AM
> > To: devel@edk2.groups.io
> > Cc: Feng, Bob C ; Gao, Liming
> > ; Chen, Christine 
> > Subject: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile
> > information in build report
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2850
> >
> > Add "-Y REPORT_INFO" option to build command to generate compile
> > information as part of BuildReport.
> > This option generates files to be used by external tools as IDE's to enhance
> > functionality.
> > Files are created inside build folder:
> > ///CompileInfo
> >
> > Files created:
> > * compile_commands.json - Compilation Database. To be used by IDE's
> >   to enable advance features
> > * cscope.files - List of files used in compilation. Used by Cscope to parse
> >   C code and provide browse functionality.
> > * module_report.json - Module data form buildReport in Json format.
> >
> > Signed-off-by: Guillermo Antonio Palomino Sosa
> > 
> > ---
> >  BaseTools/Source/Python/build/BuildReport.py  | 139
> > +++-
> >  BaseTools/Source/Python/build/buildoptions.py |   4 +-
> >  2 files changed, 140 insertions(+), 3 deletions(-)
> >
> > diff --git a/BaseTools/Source/Python/build/BuildReport.py
> > b/BaseTools/Source/Python/build/BuildReport.py
> > index 468772930c..33b43d471f 100644
> > --- a/BaseTools/Source/Python/build/BuildReport.py
> > +++ b/BaseTools/Source/Python/build/BuildReport.py
> > @@ -10,6 +10,8 @@
> >
> >  ## Import Modules
> >  #
> > +import json
> > +from pathlib import Path
> >  import Common.LongFilePathOs as os
> >  import re
> >  import platform
> > @@ -41,6 +43,7 @@ from Common.DataType import *  import collections
> > from Common.Expression import *  from GenFds.AprioriSection import
> > DXE_APRIORI_GUID, PEI_APRIORI_GUID
> > +from AutoGen.IncludesAutoGen import IncludesAutoGen
> >
> >  ## Pattern to extract contents in EDK DXS files  gDxsDependencyPattern =
> > re.compile(r"DEPENDENCY_START(.+)DEPENDENCY_END", re.DOTALL) @@ -
> > 2298,6 +2301,10 @@ class BuildReport(object):
> >  def GenerateReport(self, BuildDuration, AutoGenTime, MakeTime,
> > GenFdsTime):
> >  if self.ReportFile:
> >  try:
> > +
> > +if "COMPILE_INFO" in self.ReportType:
> > +self.GenerateCompileInfo()
> > +
> >  File = []
> >  for (Wa, MaList) in self.ReportList:
> >  PlatformReport(Wa, MaList,
> > self.ReportType).GenerateReport(File, BuildDuration, AutoGenTime,
> > MakeTime, GenFdsTime, self.ReportType) @@ -2310,7 +2317,137 @@ class
> > BuildReport(object):
> >  EdkLogger.error("BuildReport", CODE_ERROR, "Unknown fatal
> > error when generating build report", ExtraData=self.ReportFile,
> > RaiseError=False)
> >  EdkLogger.quiet("(Python %s on %s\n%s)" %
> > (platform.python_version(), sys.platform, traceback.format_exc()))
> >
> > +
> > +##
> > +# Generates compile data files to be used by external tools.
> > +# Compile information will be generated in
> > ///CompileInfo
> > +# Files generated: compile_commands.json, cscope.files,
> > modules_report.json
> > +#
> > +# @param selfThe object pointer
> > +#
> > +def GenerateCompileInfo(self):
> > +try:
> > +# Lists for the output elements
> > +compile_commands = []
> > +used_files = set()
> > +module_report = []
> > +
> > +for (Wa, MaList) in self.ReportList:
> > +# Obtain list of all processed Workspace files
> > +for file_path in Wa._GetMetaFiles(Wa.BuildTarget, 
> > Wa.ToolChain):
> > +used_files.add(file_path)
> > +
> > +for autoGen in Wa.AutoGenObjectList:
> > +
> > +# Loop through all modules
> > +for module in (autoGen.LibraryAutoGenList +
> > autoGen.ModuleAutoGenList):
> > +
> > +used_files.add(module.MetaFile.Path)
> > +
> > +# Main elements of module report
> > +module_report_data = {}
> > +module_report_data["Name"] = module.Name
> > +module_report_data["Arch"] = module.Arch
> > +module_report_data["Path"] = module.MetaFile.Path
> > +module_report_data["Guid"] = module.Guid
> > +module_report_data["BuildType"] =

Re: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile information in build report

2023-02-14 Thread Yuwei Chen

Reviewed-by: Yuwei Chen 

> -Original Message-
> From: Palomino Sosa, Guillermo A 
> Sent: Tuesday, February 7, 2023 11:07 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming
> ; Chen, Christine 
> Subject: [edk2-devel][PATCH V1 1/1] BaseTools: Generate compile
> information in build report
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2850
> 
> Add "-Y REPORT_INFO" option to build command to generate compile
> information as part of BuildReport.
> This option generates files to be used by external tools as IDE's to enhance
> functionality.
> Files are created inside build folder:
> ///CompileInfo
> 
> Files created:
> * compile_commands.json - Compilation Database. To be used by IDE's
>   to enable advance features
> * cscope.files - List of files used in compilation. Used by Cscope to parse
>   C code and provide browse functionality.
> * module_report.json - Module data form buildReport in Json format.
> 
> Signed-off-by: Guillermo Antonio Palomino Sosa
> 
> ---
>  BaseTools/Source/Python/build/BuildReport.py  | 139
> +++-
>  BaseTools/Source/Python/build/buildoptions.py |   4 +-
>  2 files changed, 140 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/build/BuildReport.py
> b/BaseTools/Source/Python/build/BuildReport.py
> index 468772930c..33b43d471f 100644
> --- a/BaseTools/Source/Python/build/BuildReport.py
> +++ b/BaseTools/Source/Python/build/BuildReport.py
> @@ -10,6 +10,8 @@
> 
>  ## Import Modules
>  #
> +import json
> +from pathlib import Path
>  import Common.LongFilePathOs as os
>  import re
>  import platform
> @@ -41,6 +43,7 @@ from Common.DataType import *  import collections
> from Common.Expression import *  from GenFds.AprioriSection import
> DXE_APRIORI_GUID, PEI_APRIORI_GUID
> +from AutoGen.IncludesAutoGen import IncludesAutoGen
> 
>  ## Pattern to extract contents in EDK DXS files  gDxsDependencyPattern =
> re.compile(r"DEPENDENCY_START(.+)DEPENDENCY_END", re.DOTALL) @@ -
> 2298,6 +2301,10 @@ class BuildReport(object):
>  def GenerateReport(self, BuildDuration, AutoGenTime, MakeTime,
> GenFdsTime):
>  if self.ReportFile:
>  try:
> +
> +if "COMPILE_INFO" in self.ReportType:
> +self.GenerateCompileInfo()
> +
>  File = []
>  for (Wa, MaList) in self.ReportList:
>  PlatformReport(Wa, MaList,
> self.ReportType).GenerateReport(File, BuildDuration, AutoGenTime,
> MakeTime, GenFdsTime, self.ReportType) @@ -2310,7 +2317,137 @@ class
> BuildReport(object):
>  EdkLogger.error("BuildReport", CODE_ERROR, "Unknown fatal
> error when generating build report", ExtraData=self.ReportFile,
> RaiseError=False)
>  EdkLogger.quiet("(Python %s on %s\n%s)" %
> (platform.python_version(), sys.platform, traceback.format_exc()))
> 
> +
> +##
> +# Generates compile data files to be used by external tools.
> +# Compile information will be generated in
> ///CompileInfo
> +# Files generated: compile_commands.json, cscope.files,
> modules_report.json
> +#
> +# @param selfThe object pointer
> +#
> +def GenerateCompileInfo(self):
> +try:
> +# Lists for the output elements
> +compile_commands = []
> +used_files = set()
> +module_report = []
> +
> +for (Wa, MaList) in self.ReportList:
> +# Obtain list of all processed Workspace files
> +for file_path in Wa._GetMetaFiles(Wa.BuildTarget, 
> Wa.ToolChain):
> +used_files.add(file_path)
> +
> +for autoGen in Wa.AutoGenObjectList:
> +
> +# Loop through all modules
> +for module in (autoGen.LibraryAutoGenList +
> autoGen.ModuleAutoGenList):
> +
> +used_files.add(module.MetaFile.Path)
> +
> +# Main elements of module report
> +module_report_data = {}
> +module_report_data["Name"] = module.Name
> +module_report_data["Arch"] = module.Arch
> +module_report_data["Path"] = module.MetaFile.Path
> +module_report_data["Guid"] = module.Guid
> +module_report_data["BuildType"] = module.BuildType
> +module_report_data["IsLibrary"] = module.IsLibrary
> +module_report_data["SourceDir"] = module.SourceDir
> +module_report_data["Files"] = []
> +
> +# Files used by module
> +for data_file in module.SourceFileList:
> +module_report_data["Files"].append({"Name":
> + data_file.Name, "Path": data_file.Path})
> +
> +# Libraries used by module
> +module_report_data["Libraries"] = []
> +

[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, February 14, 2023 #cal-reminder

2023-02-14 Thread Group Notification

*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, February 14, 2023
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d

*Organizer:* Liming Gao gaolim...@byosoft.com.cn ( 
gaolim...@byosoft.com.cn?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1753753 )

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions ( 
https://conf.intel.com/teams/?conf=1160620940=teams=conf.intel.com=test_call
 )

*Or call in (audio only)*

+1 916-245-6934,,77463821# ( tel:+19162456934,,77463821# ) United States, 
Sacramento

Phone Conference ID: 774 638 21#

Find a local number ( 
https://dialin.teams.microsoft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821
 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e=46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2=0=en-US
 )


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

Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

2023-02-14 Thread Michael D Kinney

Merged

PR: https://github.com/tianocore/edk2/pull/4043
Commit: 
https://github.com/tianocore/edk2/commit/090642db7ac124c336da72e1954e1fb09e816fb0

Mike

> -Original Message-
> From: Kinney, Michael D 
> Sent: Tuesday, February 14, 2023 3:50 PM
> To: Jeff Brasen ; devel@edk2.groups.io; Demeter, Miki 
> 
> Cc: Kinney, Michael D 
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support 
> multiple devices with 0 HardwareInstance
> 
> Hi Jeff,
> 
> I have been studying the code for side effects from Version >= 3 and 
> HardwareInstance = 0.
> 
> I think the only side effect with your patch is extra entries in 
> HardwareIntances with the
> same GUID value and HardwareInstance value of 0.
> 
>   IN OUT GUID_HARDWAREINSTANCE_PAIR *HardwareInstances,
>   IN OUT UINT32 *NumberOfDescriptors,
> 
> This table is only used to look for duplicate GUID/HardwareInstance pairs and 
> is not used
> anywhere else and is freed after ESRT entries are determined.  The same 
> number of ESRT entries
> are be created even if these extra HardwareInstances entries are present.
> 
> I have also not seen any comments or concerns from anyone else on this topic.
> 
> 
> 
> Reviewed-by: Michael D Kinney 
> 
> 
> 
> Best regards,
> 
> Mike
> 
> > -Original Message-
> > From: Jeff Brasen 
> > Sent: Friday, February 10, 2023 1:21 PM
> > To: Kinney, Michael D ; devel@edk2.groups.io; 
> > Demeter, Miki 
> > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support 
> > multiple devices with 0 HardwareInstance
> >
> > This is unclear in my opinion. The spec does have
> > This number must be unique within the namespace of the ImageTypeId GUID and 
> > ImageIndex. And
> > For implementations that will never have more than one instance a zero can 
> > be used.
> >
> > But it also states the parameter is optional and states that zero can be 
> > used when if it is not able to determine a unique
> > hardware instance number or a hardware instance number is not needed.
> >
> > I could see reasonable arguments on both sides.
> >
> > -Jeff
> >
> > > -Original Message-
> > > From: Kinney, Michael D 
> > > Sent: Friday, February 10, 2023 2:17 PM
> > > To: devel@edk2.groups.io; Jeff Brasen ; Demeter,
> > > Miki 
> > > Cc: Kinney, Michael D 
> > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > > multiple devices with 0 HardwareInstance
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > Based on UEFI Spec, would you consider those option roms to be non-
> > > conformant?
> > >
> > > Mike
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of Jeff
> > > > Brasen via groups.io
> > > > Sent: Friday, February 10, 2023 12:43 PM
> > > > To: Kinney, Michael D ;
> > > > devel@edk2.groups.io; Demeter, Miki 
> > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > Support
> > > > multiple devices with 0 HardwareInstance
> > > >
> > > > It was not as we were seeing devices expose FMP via an option rom
> > > > driver with Version == 3 and HardwareInstance = 0. If there are 
> > > > multiple of
> > > these device it would hit the error in the EsrtCode. The patch included 
> > > does
> > > resolve the issues we were seeing.
> > > >
> > > > -Jeff
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Kinney, Michael D 
> > > > > Sent: Friday, February 10, 2023 12:48 PM
> > > > > To: devel@edk2.groups.io; Jeff Brasen ; Demeter,
> > > > > Miki 
> > > > > Cc: Kinney, Michael D 
> > > > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > > Support multiple devices with 0 HardwareInstance
> > > > >
> > > > > External email: Use caution opening links or attachments
> > > > >
> > > > >
> > > > > Did your original approach work for all your use cases?
> > > > >
> > > > > Mike
> > > > >
> > > > > > -Original Message-
> > > > > > From: devel@edk2.groups.io  On Behalf Of
> > > > > > Jeff Brasen via groups.io
> > > > > > Sent: Friday, February 10, 2023 8:45 AM
> > > > > > To: Kinney, Michael D ;
> > > > > > devel@edk2.groups.io; Demeter, Miki 
> > > > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > > Support
> > > > > > multiple devices with 0 HardwareInstance
> > > > > >
> > > > > > That was my original approach when I saw this but I was seeing
> > > > > > this occur on a device with FmpVersion == 3. The UEFI spec isn't
> > > > > > completely
> > > > > clear but could be read that 0 is legal if this part isn't being used.
> > > > > >
> > > > > >
> > > > >
> > > https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.ht
> > > > > m
> > > > > l
> > > > > > #efi-firmware-management-protocol-getimageinfo
> > > > > >
> > > > > > It does state that this is Optional, and has a statement "A zero
> > > > > > means the FMP provider is not able to determine a unique hardware
> > > > > > instance
> > > > > number or a

Re: [edk2-devel] [PATCH] OvmfPkg: Close mAcceptAllMemoryEvent

2023-02-14 Thread Gupta, Pankaj via groups.io


On 2/15/2023 12:07 AM, Dionna Glaze via groups.io wrote:

This event should only trigger once. It should be idempotent, but the
allocation of the memory map itself is observable and can cause
ExitBootServices to fail with a modified map key.

Cc: Ard Biesheuvel 
Cc: Thomas Lendacky 
Cc: Erdem Aktas 
Cc: James Bottomley 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Michael Roth 

Signed-off-by: Dionna Glaze 
---
  OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..f9baca90bd 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -124,6 +124,7 @@ AcceptAllMemory (
}
  
gBS->FreePool (AllDescMap);

+  gBS->CloseEvent (mAcceptAllMemoryEvent);
return Status;
  }


Reviewed-by: Pankaj Gupta 
Tested-by: Pankaj Gupta 
Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")




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

Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more information of PCIe devices

2023-02-14 Thread Michael D Kinney

Merged

PR: https://github.com/tianocore/edk2/pull/4042
Commit: 
https://github.com/tianocore/edk2/commit/f9c6b5134e5e2c5ca977f422119b1255412acd53

Mike

> -Original Message-
> From: Kinney, Michael D 
> Sent: Tuesday, February 14, 2023 3:27 PM
> To: devel@edk2.groups.io; Ni, Ray ; abner.ch...@amd.com
> Cc: Wu, Hao A ; Garrett Kirkendall 
> ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more 
> information of PCIe devices
> 
> Acked-by: Michael D Kinney 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> > Sent: Monday, February 13, 2023 6:18 PM
> > To: devel@edk2.groups.io; abner.ch...@amd.com
> > Cc: Wu, Hao A ; Garrett Kirkendall 
> > 
> > Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more 
> > information of PCIe devices
> >
> > Reviewed-by: Ray Ni 
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Chang,
> > > Abner via groups.io
> > > Sent: Thursday, January 12, 2023 1:14 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A ; Ni, Ray ; Garrett
> > > Kirkendall ; Abner Chang
> > > 
> > > Subject: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > > information of PCIe devices
> > >
> > > From: Abner Chang 
> > >
> > > In V4: Update the copyright to 2023.
> > > In V3: Add AMD copyright.
> > > In V2: Remove the signed-off-by: Abner Chang
> > >
> > > Display PCIe Vendor ID and Device ID in DEBUG message.
> > >
> > > Signed-off-by: Jiangang He 
> > > Cc: Hao A Wu 
> > > Cc: Ray Ni 
> > > Cc: Garrett Kirkendall 
> > > Cc: Abner Chang 
> > > ---
> > >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 7 +--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > index 8eca8596958..6594b8eae83 100644
> > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > @@ -3,6 +3,7 @@
> > >
> > >  Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.
> > >  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > > +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -227,13 +228,15 @@ PciSearchDevice (
> > >
> > >DEBUG ((
> > >  DEBUG_INFO,
> > > -"PciBus: Discovered %s @ [%02x|%02x|%02x]\n",
> > > +"PciBus: Discovered %s @ [%02x|%02x|%02x]  [VID = 0x%x, DID =
> > > 0x%0x]\n",
> > >  IS_PCI_BRIDGE (Pci) ? L"PPB" :
> > >  IS_CARDBUS_BRIDGE (Pci) ? L"P2C" :
> > >  L"PCI",
> > >  Bus,
> > >  Device,
> > > -Func
> > > +Func,
> > > +Pci->Hdr.VendorId,
> > > +Pci->Hdr.DeviceId
> > >  ));
> > >
> > >if (!IS_PCI_BRIDGE (Pci)) {
> > > --
> > > 2.37.1.windows.1
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> > 
> >



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

Re: [edk2-devel] [PATCH v1 1/1] uefi-sct/SctPkg: update path to edk2-test-parser, check for repo

2023-02-14 Thread Gao Jie via groups.io

Reviewed-by: Barton Gao < gao...@byosoft.com.cn >


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

Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more information of PCIe devices

2023-02-14 Thread Chang, Abner via groups.io

[AMD Official Use Only - General]

Ah ok, thanks!

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, February 15, 2023 8:41 AM
> To: devel@edk2.groups.io; Chang, Abner ; Ni, Ray
> 
> Cc: Wu, Hao A ; Kirkendall, Garrett
> ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> information of PCIe devices
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> It is already in process of being merged
> 
> https://github.com/tianocore/edk2/pull/4042
> 
> Mike
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Chang,
> > Abner via groups.io
> > Sent: Tuesday, February 14, 2023 4:29 PM
> > To: Kinney, Michael D ;
> > devel@edk2.groups.io; Ni, Ray 
> > Cc: Wu, Hao A ; Kirkendall, Garrett
> > 
> > Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > information of PCIe devices
> >
> > [AMD Official Use Only - General]
> >
> > Thanks Ray and Mike, I will create PR and merge it.
> >
> > Abner
> >
> > > -Original Message-
> > > From: Kinney, Michael D 
> > > Sent: Wednesday, February 15, 2023 7:27 AM
> > > To: devel@edk2.groups.io; Ni, Ray ; Chang, Abner
> > > 
> > > Cc: Wu, Hao A ; Kirkendall, Garrett
> > > ; Kinney, Michael D
> > > 
> > > Subject: RE: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > > information of PCIe devices
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > Acked-by: Michael D Kinney 
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of Ni,
> > > Ray
> > > > Sent: Monday, February 13, 2023 6:18 PM
> > > > To: devel@edk2.groups.io; abner.ch...@amd.com
> > > > Cc: Wu, Hao A ; Garrett Kirkendall
> > > > 
> > > > Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display
> > > > more information of PCIe devices
> > > >
> > > > Reviewed-by: Ray Ni 
> > > >
> > > > > -Original Message-
> > > > > From: devel@edk2.groups.io  On Behalf Of
> > > > > Chang, Abner via groups.io
> > > > > Sent: Thursday, January 12, 2023 1:14 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Wu, Hao A ; Ni, Ray ;
> > > > > Garrett Kirkendall ; Abner Chang
> > > > > 
> > > > > Subject: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > > > > information of PCIe devices
> > > > >
> > > > > From: Abner Chang 
> > > > >
> > > > > In V4: Update the copyright to 2023.
> > > > > In V3: Add AMD copyright.
> > > > > In V2: Remove the signed-off-by: Abner Chang
> > > > >
> > > > > Display PCIe Vendor ID and Device ID in DEBUG message.
> > > > >
> > > > > Signed-off-by: Jiangang He 
> > > > > Cc: Hao A Wu 
> > > > > Cc: Ray Ni 
> > > > > Cc: Garrett Kirkendall 
> > > > > Cc: Abner Chang 
> > > > > ---
> > > > >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 7
> > > > > +-
> > > -
> > > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > > index 8eca8596958..6594b8eae83 100644
> > > > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > > @@ -3,6 +3,7 @@
> > > > >
> > > > >  Copyright (c) 2006 - 2021, Intel Corporation. All rights
> > > > > reserved.
> > > > >  (C) Copyright 2015 Hewlett Packard Enterprise Development
> > > > > LP
> > > > > +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> > > > > +reserved.
> > > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >  **/
> > > > > @@ -227,13 +228,15 @@ PciSearchDevice (
> > > > >
> > > > >DEBUG ((
> > > > >  DEBUG_INFO,
> > > > > -"PciBus: Discovered %s @ [%02x|%02x|%02x]\n",
> > > > > +"PciBus: Discovered %s @ [%02x|%02x|%02x]  [VID = 0x%x, DID
> > > > > + =
> > > > > 0x%0x]\n",
> > > > >  IS_PCI_BRIDGE (Pci) ? L"PPB" :
> > > > >  IS_CARDBUS_BRIDGE (Pci) ? L"P2C" :
> > > > >  L"PCI",
> > > > >  Bus,
> > > > >  Device,
> > > > > -Func
> > > > > +Func,
> > > > > +Pci->Hdr.VendorId,
> > > > > +Pci->Hdr.DeviceId
> > > > >  ));
> > > > >
> > > > >if (!IS_PCI_BRIDGE (Pci)) {
> > > > > --
> > > > > 2.37.1.windows.1
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> >
> >
> > 
> >


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

Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more information of PCIe devices

2023-02-14 Thread Michael D Kinney

It is already in process of being merged

https://github.com/tianocore/edk2/pull/4042

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chang, Abner 
> via groups.io
> Sent: Tuesday, February 14, 2023 4:29 PM
> To: Kinney, Michael D ; devel@edk2.groups.io; Ni, 
> Ray 
> Cc: Wu, Hao A ; Kirkendall, Garrett 
> 
> Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more 
> information of PCIe devices
> 
> [AMD Official Use Only - General]
> 
> Thanks Ray and Mike, I will create PR and merge it.
> 
> Abner
> 
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Wednesday, February 15, 2023 7:27 AM
> > To: devel@edk2.groups.io; Ni, Ray ; Chang, Abner
> > 
> > Cc: Wu, Hao A ; Kirkendall, Garrett
> > ; Kinney, Michael D
> > 
> > Subject: RE: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > information of PCIe devices
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Acked-by: Michael D Kinney 
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Ni,
> > Ray
> > > Sent: Monday, February 13, 2023 6:18 PM
> > > To: devel@edk2.groups.io; abner.ch...@amd.com
> > > Cc: Wu, Hao A ; Garrett Kirkendall
> > > 
> > > Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > > information of PCIe devices
> > >
> > > Reviewed-by: Ray Ni 
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of
> > > > Chang, Abner via groups.io
> > > > Sent: Thursday, January 12, 2023 1:14 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wu, Hao A ; Ni, Ray ;
> > > > Garrett Kirkendall ; Abner Chang
> > > > 
> > > > Subject: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > > > information of PCIe devices
> > > >
> > > > From: Abner Chang 
> > > >
> > > > In V4: Update the copyright to 2023.
> > > > In V3: Add AMD copyright.
> > > > In V2: Remove the signed-off-by: Abner Chang
> > > >
> > > > Display PCIe Vendor ID and Device ID in DEBUG message.
> > > >
> > > > Signed-off-by: Jiangang He 
> > > > Cc: Hao A Wu 
> > > > Cc: Ray Ni 
> > > > Cc: Garrett Kirkendall 
> > > > Cc: Abner Chang 
> > > > ---
> > > >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 7 +-
> > -
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > index 8eca8596958..6594b8eae83 100644
> > > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > > @@ -3,6 +3,7 @@
> > > >
> > > >  Copyright (c) 2006 - 2021, Intel Corporation. All rights
> > > > reserved.
> > > >  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > > > +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> > > > +reserved.
> > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  **/
> > > > @@ -227,13 +228,15 @@ PciSearchDevice (
> > > >
> > > >DEBUG ((
> > > >  DEBUG_INFO,
> > > > -"PciBus: Discovered %s @ [%02x|%02x|%02x]\n",
> > > > +"PciBus: Discovered %s @ [%02x|%02x|%02x]  [VID = 0x%x, DID =
> > > > 0x%0x]\n",
> > > >  IS_PCI_BRIDGE (Pci) ? L"PPB" :
> > > >  IS_CARDBUS_BRIDGE (Pci) ? L"P2C" :
> > > >  L"PCI",
> > > >  Bus,
> > > >  Device,
> > > > -Func
> > > > +Func,
> > > > +Pci->Hdr.VendorId,
> > > > +Pci->Hdr.DeviceId
> > > >  ));
> > > >
> > > >if (!IS_PCI_BRIDGE (Pci)) {
> > > > --
> > > > 2.37.1.windows.1
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more information of PCIe devices

2023-02-14 Thread Chang, Abner via groups.io

[AMD Official Use Only - General]

Thanks Ray and Mike, I will create PR and merge it.

Abner

> -Original Message-
> From: Kinney, Michael D 
> Sent: Wednesday, February 15, 2023 7:27 AM
> To: devel@edk2.groups.io; Ni, Ray ; Chang, Abner
> 
> Cc: Wu, Hao A ; Kirkendall, Garrett
> ; Kinney, Michael D
> 
> Subject: RE: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> information of PCIe devices
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Acked-by: Michael D Kinney 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ni,
> Ray
> > Sent: Monday, February 13, 2023 6:18 PM
> > To: devel@edk2.groups.io; abner.ch...@amd.com
> > Cc: Wu, Hao A ; Garrett Kirkendall
> > 
> > Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > information of PCIe devices
> >
> > Reviewed-by: Ray Ni 
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of
> > > Chang, Abner via groups.io
> > > Sent: Thursday, January 12, 2023 1:14 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A ; Ni, Ray ;
> > > Garrett Kirkendall ; Abner Chang
> > > 
> > > Subject: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > > information of PCIe devices
> > >
> > > From: Abner Chang 
> > >
> > > In V4: Update the copyright to 2023.
> > > In V3: Add AMD copyright.
> > > In V2: Remove the signed-off-by: Abner Chang
> > >
> > > Display PCIe Vendor ID and Device ID in DEBUG message.
> > >
> > > Signed-off-by: Jiangang He 
> > > Cc: Hao A Wu 
> > > Cc: Ray Ni 
> > > Cc: Garrett Kirkendall 
> > > Cc: Abner Chang 
> > > ---
> > >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 7 +-
> -
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > index 8eca8596958..6594b8eae83 100644
> > > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > > @@ -3,6 +3,7 @@
> > >
> > >  Copyright (c) 2006 - 2021, Intel Corporation. All rights
> > > reserved.
> > >  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > > +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> > > +reserved.
> > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  **/
> > > @@ -227,13 +228,15 @@ PciSearchDevice (
> > >
> > >DEBUG ((
> > >  DEBUG_INFO,
> > > -"PciBus: Discovered %s @ [%02x|%02x|%02x]\n",
> > > +"PciBus: Discovered %s @ [%02x|%02x|%02x]  [VID = 0x%x, DID =
> > > 0x%0x]\n",
> > >  IS_PCI_BRIDGE (Pci) ? L"PPB" :
> > >  IS_CARDBUS_BRIDGE (Pci) ? L"P2C" :
> > >  L"PCI",
> > >  Bus,
> > >  Device,
> > > -Func
> > > +Func,
> > > +Pci->Hdr.VendorId,
> > > +Pci->Hdr.DeviceId
> > >  ));
> > >
> > >if (!IS_PCI_BRIDGE (Pci)) {
> > > --
> > > 2.37.1.windows.1
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> > 
> >


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

Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

2023-02-14 Thread Michael D Kinney

Hi Jeff,

I have been studying the code for side effects from Version >= 3 and 
HardwareInstance = 0.

I think the only side effect with your patch is extra entries in 
HardwareIntances with the
same GUID value and HardwareInstance value of 0.

  IN OUT GUID_HARDWAREINSTANCE_PAIR *HardwareInstances,
  IN OUT UINT32 *NumberOfDescriptors,

This table is only used to look for duplicate GUID/HardwareInstance pairs and 
is not used
anywhere else and is freed after ESRT entries are determined.  The same number 
of ESRT entries
are be created even if these extra HardwareInstances entries are present.

I have also not seen any comments or concerns from anyone else on this topic.



Reviewed-by: Michael D Kinney 



Best regards,

Mike

> -Original Message-
> From: Jeff Brasen 
> Sent: Friday, February 10, 2023 1:21 PM
> To: Kinney, Michael D ; devel@edk2.groups.io; 
> Demeter, Miki 
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support 
> multiple devices with 0 HardwareInstance
> 
> This is unclear in my opinion. The spec does have
> This number must be unique within the namespace of the ImageTypeId GUID and 
> ImageIndex. And
> For implementations that will never have more than one instance a zero can be 
> used.
> 
> But it also states the parameter is optional and states that zero can be used 
> when if it is not able to determine a unique
> hardware instance number or a hardware instance number is not needed.
> 
> I could see reasonable arguments on both sides.
> 
> -Jeff
> 
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Friday, February 10, 2023 2:17 PM
> > To: devel@edk2.groups.io; Jeff Brasen ; Demeter,
> > Miki 
> > Cc: Kinney, Michael D 
> > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > multiple devices with 0 HardwareInstance
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Based on UEFI Spec, would you consider those option roms to be non-
> > conformant?
> >
> > Mike
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Jeff
> > > Brasen via groups.io
> > > Sent: Friday, February 10, 2023 12:43 PM
> > > To: Kinney, Michael D ;
> > > devel@edk2.groups.io; Demeter, Miki 
> > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > Support
> > > multiple devices with 0 HardwareInstance
> > >
> > > It was not as we were seeing devices expose FMP via an option rom
> > > driver with Version == 3 and HardwareInstance = 0. If there are multiple 
> > > of
> > these device it would hit the error in the EsrtCode. The patch included does
> > resolve the issues we were seeing.
> > >
> > > -Jeff
> > >
> > >
> > > > -Original Message-
> > > > From: Kinney, Michael D 
> > > > Sent: Friday, February 10, 2023 12:48 PM
> > > > To: devel@edk2.groups.io; Jeff Brasen ; Demeter,
> > > > Miki 
> > > > Cc: Kinney, Michael D 
> > > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > Support multiple devices with 0 HardwareInstance
> > > >
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > Did your original approach work for all your use cases?
> > > >
> > > > Mike
> > > >
> > > > > -Original Message-
> > > > > From: devel@edk2.groups.io  On Behalf Of
> > > > > Jeff Brasen via groups.io
> > > > > Sent: Friday, February 10, 2023 8:45 AM
> > > > > To: Kinney, Michael D ;
> > > > > devel@edk2.groups.io; Demeter, Miki 
> > > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > Support
> > > > > multiple devices with 0 HardwareInstance
> > > > >
> > > > > That was my original approach when I saw this but I was seeing
> > > > > this occur on a device with FmpVersion == 3. The UEFI spec isn't
> > > > > completely
> > > > clear but could be read that 0 is legal if this part isn't being used.
> > > > >
> > > > >
> > > >
> > https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.ht
> > > > m
> > > > l
> > > > > #efi-firmware-management-protocol-getimageinfo
> > > > >
> > > > > It does state that this is Optional, and has a statement "A zero
> > > > > means the FMP provider is not able to determine a unique hardware
> > > > > instance
> > > > number or a hardware instance number is not needed."
> > > > >
> > > > > Also, our ESRT implementation just merges all instances to one
> > > > > anyways so it doesn't currently consume the hardware instance
> > > > > except for this
> > > > check.
> > > > >
> > > > > Thanks,
> > > > > Jeff
> > > > >
> > > > > > -Original Message-
> > > > > > From: Kinney, Michael D 
> > > > > > Sent: Thursday, February 9, 2023 8:43 PM
> > > > > > To: devel@edk2.groups.io; Jeff Brasen ;
> > > > > > Demeter, Miki 
> > > > > > Cc: Kinney, Michael D 
> > > > > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > > > > > multiple devices with 0 HardwareInstance
> > > > > >
> > > > > > External email: Use caution opening links

Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more information of PCIe devices

2023-02-14 Thread Michael D Kinney

Acked-by: Michael D Kinney 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ni, Ray
> Sent: Monday, February 13, 2023 6:18 PM
> To: devel@edk2.groups.io; abner.ch...@amd.com
> Cc: Wu, Hao A ; Garrett Kirkendall 
> 
> Subject: Re: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more 
> information of PCIe devices
> 
> Reviewed-by: Ray Ni 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Chang,
> > Abner via groups.io
> > Sent: Thursday, January 12, 2023 1:14 PM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A ; Ni, Ray ; Garrett
> > Kirkendall ; Abner Chang
> > 
> > Subject: [edk2-devel] [PATCH V4] MdeModulePkg/Pci: Display more
> > information of PCIe devices
> >
> > From: Abner Chang 
> >
> > In V4: Update the copyright to 2023.
> > In V3: Add AMD copyright.
> > In V2: Remove the signed-off-by: Abner Chang
> >
> > Display PCIe Vendor ID and Device ID in DEBUG message.
> >
> > Signed-off-by: Jiangang He 
> > Cc: Hao A Wu 
> > Cc: Ray Ni 
> > Cc: Garrett Kirkendall 
> > Cc: Abner Chang 
> > ---
> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > index 8eca8596958..6594b8eae83 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > @@ -3,6 +3,7 @@
> >
> >  Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.
> >  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  **/
> > @@ -227,13 +228,15 @@ PciSearchDevice (
> >
> >DEBUG ((
> >  DEBUG_INFO,
> > -"PciBus: Discovered %s @ [%02x|%02x|%02x]\n",
> > +"PciBus: Discovered %s @ [%02x|%02x|%02x]  [VID = 0x%x, DID =
> > 0x%0x]\n",
> >  IS_PCI_BRIDGE (Pci) ? L"PPB" :
> >  IS_CARDBUS_BRIDGE (Pci) ? L"P2C" :
> >  L"PCI",
> >  Bus,
> >  Device,
> > -Func
> > +Func,
> > +Pci->Hdr.VendorId,
> > +Pci->Hdr.DeviceId
> >  ));
> >
> >if (!IS_PCI_BRIDGE (Pci)) {
> > --
> > 2.37.1.windows.1
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH] OvmfPkg: Close mAcceptAllMemoryEvent

2023-02-14 Thread Ard Biesheuvel

On Wed, 15 Feb 2023 at 00:07, Dionna Glaze  wrote:
>
> This event should only trigger once. It should be idempotent, but the
> allocation of the memory map itself is observable and can cause
> ExitBootServices to fail with a modified map key.
>
> Cc: Ard Biesheuvel 
> Cc: Thomas Lendacky 
> Cc: Erdem Aktas 
> Cc: James Bottomley 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Michael Roth 
>
> Signed-off-by: Dionna Glaze 
> ---
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 6391d1f775..f9baca90bd 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -124,6 +124,7 @@ AcceptAllMemory (
>}
>
>gBS->FreePool (AllDescMap);
> +  gBS->CloseEvent (mAcceptAllMemoryEvent);
>return Status;
>  }
>
> --
> 2.39.1.637.g21b0678d19-goog
>

Reviewed-by: Ard Biesheuvel 

Queued as #4041


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

[edk2-devel] [PATCH] OvmfPkg: Close mAcceptAllMemoryEvent

2023-02-14 Thread Dionna Glaze via groups.io

This event should only trigger once. It should be idempotent, but the
allocation of the memory map itself is observable and can cause
ExitBootServices to fail with a modified map key.

Cc: Ard Biesheuvel 
Cc: Thomas Lendacky 
Cc: Erdem Aktas 
Cc: James Bottomley 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Michael Roth 

Signed-off-by: Dionna Glaze 
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..f9baca90bd 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -124,6 +124,7 @@ AcceptAllMemory (
   }
 
   gBS->FreePool (AllDescMap);
+  gBS->CloseEvent (mAcceptAllMemoryEvent);
   return Status;
 }
 
-- 
2.39.1.637.g21b0678d19-goog



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

Re: [edk2-devel] [PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

2023-02-14 Thread Ard Biesheuvel

On Tue, 14 Feb 2023 at 23:35, Ard Biesheuvel  wrote:
>
> On Tue, 14 Feb 2023 at 23:15, Pankaj Gupta  wrote:
> >
> > System Memory map is changed when a memory range is Accepted.
> > While returning from AcceptAllMemory(), "gBS->FreePool" is wrongly
> > used which results in changing memory map and hence return an error.
> > Fix this by removing the "gBs->FreePool" call altogether.
> >
> > Before this patch, KVM guest throws an error and control goes to the
> > boat loader menu every time we select an OS:
> >
> > EFI stub: ERROR: exit_boot() failed!
> > EFI stub: ERROR: efi_main() failed!
> > StartImage failed: Invalid Parameter
> >
> > Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")
> > Signed-off-by: Pankaj Gupta 
>
> Queued as #4040 - thanks!
>

I've dropped it again - it seems AcceptAllMemory() should never be
called a second time (and it is debatable whether the
gEfiEventBeforeExitBootServicesGuid event should be signaled the
second time), so it would be better to address that instead.


> > ---
> >  OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index 6391d1f775..f52dbfe597 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -123,7 +123,6 @@ AcceptAllMemory (
> >  }
> >}
> >
> > -  gBS->FreePool (AllDescMap);
> >return Status;
> >  }
> >
> > --
> > 2.34.1
> >


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

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Lendacky, Thomas via groups.io


On 2/14/23 11:28, Dionna Amalie Glaze wrote:


Do you have any pointers on the IVARS service?  Documentation, guest
code, host code?



Agh, I thought for sure there was a public API for VM owners to view
or change their UEFI variables, but I guess not. It's an
instance-specific small data store for nonvolatile memory like vTPM
and UEFI variables. It appears you can only set the variables through
cloud API at instance creation time. But this is how instances can be
shut down and brought back up on different machines and/or live
migrate to other machines and still have access to UEFI variables'
current values. Host code is all in Google's proprietary VMM,
Vanadium, but the device backend is really rather simple. The data
store service though, that's a matter of Cloud Scale Engineering.


Background:  When moving to a SVSM-based setup where the svsm (with
vtpm emulation) runs in vmpl0 and the edk2 firmware in vmpl1 we might
likewise add a efi variable service to the svsm.



I thought EFI variables in Qemu were loaded and measured at launch
(OVMF_VARS.fd). If you want the current values of all uefi variables


The variables are not encrypted and not measured at launch because they 
need to be modified and stored on the host side.


You can choose to use a single vars/code file, which keeps the variables 
in memory, but you then lose any changes made to them upon VM termination.


Thanks,
Tom


in your SVSM attestation report, I think it's probably better to use
the EFI_CC_MEASUREMENT_PROTOCOL, right? Or is it specifically going to
be an SVSM service that attests itself with current stored variables,
or at least variables that are considered important enough to measure?

In any case, persistence in The Cloud (TM) remains a challenge in the
CC space. Discussion about what we should do about that should remain
on the coco mailing list. IVARS encrypts data with Google-managed
keys, so it wouldn't be directly applicable to SVSM NVRAM.


If something usable already exists we don't need to reinvent the wheel.



Don't have to tell me twice. In the spirit of OSS collaboration and
product integrity, I think any CC offering's firmware should be public
and verifiably built. I'll keep pushing for that.


thanks & take care,
   Gerd







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

Re: [edk2-devel] [PATCH] OvmfPkg: Fix SevMemoryAcceptance memory attributes

2023-02-14 Thread Ard Biesheuvel

On Thu, 2 Feb 2023 at 21:42, Dionna Amalie Glaze  wrote:
>
> > >
> > > This change is made given a request from Ard. The CC capability is not
> > > applied to other system memory ranges that probably should also have
> > > that capability, given that it's encrypted and accepted. I haven't
> > > considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to
> > > conventional memory, given the acceptance happens before DXE
> > > initializes. Perhaps
> > > CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more
> > > of a question to Ard and Thomas.
> > >
> >
> > It's not clear to me whether the CC attribute applies to the host or
> > the guest. From the guest PoV, there is really no distinction, whereas
> > on the host, I could imagine that only CC capable memory can be used
> > for handing out to VMs.
> >
>
> That's a good point. The UEFI spec language is hard to interpret here.
> Min or Jiewen, do you have more context on the EFI_MEMORY_CPU_CRYPTO
> attribute?
>

To keep things moving, I've queued this up (as #4040) with the
EFI_MEMORY_CPU_CRYPTO flag dropped.

I don't think we need to add it here, given that EDK2 nor Linux ever
set or test this flag anywhere else, but if this changes, we can add
it back.


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

Re: [edk2-devel] [PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

2023-02-14 Thread Ard Biesheuvel

On Tue, 14 Feb 2023 at 23:15, Pankaj Gupta  wrote:
>
> System Memory map is changed when a memory range is Accepted.
> While returning from AcceptAllMemory(), "gBS->FreePool" is wrongly
> used which results in changing memory map and hence return an error.
> Fix this by removing the "gBs->FreePool" call altogether.
>
> Before this patch, KVM guest throws an error and control goes to the
> boat loader menu every time we select an OS:
>
> EFI stub: ERROR: exit_boot() failed!
> EFI stub: ERROR: efi_main() failed!
> StartImage failed: Invalid Parameter
>
> Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")
> Signed-off-by: Pankaj Gupta 

Queued as #4040 - thanks!

> ---
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 6391d1f775..f52dbfe597 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -123,7 +123,6 @@ AcceptAllMemory (
>  }
>}
>
> -  gBS->FreePool (AllDescMap);
>return Status;
>  }
>
> --
> 2.34.1
>


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

[edk2-devel] [PATCH] OvmfPkg: Remove gbs FreePool in AcceptAllMemory()

2023-02-14 Thread Gupta, Pankaj via groups.io

System Memory map is changed when a memory range is Accepted.
While returning from AcceptAllMemory(), "gBS->FreePool" is wrongly
used which results in changing memory map and hence return an error.
Fix this by removing the "gBs->FreePool" call altogether.

Before this patch, KVM guest throws an error and control goes to the
boat loader menu every time we select an OS:

EFI stub: ERROR: exit_boot() failed!
EFI stub: ERROR: efi_main() failed!
StartImage failed: Invalid Parameter

Fixes: a00e2e5513 ("OvmfPkg: Add memory acceptance event in AmdSevDxe")
Signed-off-by: Pankaj Gupta 
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..f52dbfe597 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -123,7 +123,6 @@ AcceptAllMemory (
 }

   }

 

-  gBS->FreePool (AllDescMap);

   return Status;

 }

 

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100194): https://edk2.groups.io/g/devel/message/100194
Mute This Topic: https://groups.io/mt/96971535/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 1/1] MdePkg: Add library to parse SPD data and create SMBIOS Type 17 table

2023-02-14 Thread Rebecca Cran

Obviously this will need split up into several patches, but I wanted to 
get it sent out as an rfc so it doesn't get lost.


--
Rebecca Cran

On 2/14/23 14:58, Rebecca Cran wrote:

SmbiosType17SpdLib can parse a buffer containing SPD data from a DDR4
or DDR5 DIMM and construct an SMBIOS Type17 table.

Signed-off-by: Rebecca Cran 
---
  MdePkg/MdePkg.dec 
  |   3 +
  MdePkg/MdeLibs.dsc.inc
  |   2 +
  MdePkg/MdePkg.dsc 
  |   2 +
  MdePkg/Test/MdePkgHostTest.dsc
  |   5 +
  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf  
  |  40 ++
  
MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SmbiosType17SpdLibUnitTestsHost.inf
 |  35 ++
  MdePkg/Include/IndustryStandard/SdramSpd.h
  |  53 ++-
  MdePkg/Include/IndustryStandard/SdramSpdDdr4.h
  |  23 ++
  MdePkg/Include/Library/SmbiosType17SpdLib.h   
  |  37 ++
  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLibInternal.h
  | 108 ++
  MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SpdTestData.h 
  |  20 +
  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17Ddr4.c  
  | 369 ++
  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17Ddr5.c  
  | 348 +
  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17Utils.c 
  | 405 
  MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SmbiosSpdUnitTest.c   
  | 187 +
  MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SpdTestData.c 
  | 164 
  16 files changed, 1787 insertions(+), 14 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15b0..fa92e884dcbe 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -257,6 +257,9 @@ [LibraryClasses]
#
UnitTestLib|Include/Library/UnitTestLib.h
  
+  ## @libraryclass Provides service to generate SMBIOS Type 17 table from SPD data.

+  SmbiosType17SpdLib|Include/Library/SmbiosType17SpdLib.h
+
## @libraryclass Extension to BaseLib for host based unit tests that allows 
a
#subset of BaseLib services to be hooked for emulation.
#
diff --git a/MdePkg/MdeLibs.dsc.inc b/MdePkg/MdeLibs.dsc.inc
index 4580481cb580..4855972b9f0a 100644
--- a/MdePkg/MdeLibs.dsc.inc
+++ b/MdePkg/MdeLibs.dsc.inc
@@ -15,4 +15,6 @@ [LibraryClasses]
ArmTrngLib|MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.inf

RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+  JedecJep106Lib|MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf
+  SmbiosType17SpdLib|MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf

SmmCpuRendezvousLib|MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 32a852dc466e..fde11e7331e2 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -136,6 +136,8 @@ [Components]
MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf
  
+  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf

+
  [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
#
# Add UEFI Target Based Unit Tests
diff --git a/MdePkg/Test/MdePkgHostTest.dsc b/MdePkg/Test/MdePkgHostTest.dsc
index b8b186dd8b17..0bf4a248ae97 100644
--- a/MdePkg/Test/MdePkgHostTest.dsc
+++ b/MdePkg/Test/MdePkgHostTest.dsc
@@ -20,7 +20,9 @@ [Defines]
  !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
  
  [LibraryClasses]

+  JedecJep106Lib|MdePkg/Library/BaseJedecJep106Lib/BaseJedecJep106Lib.inf
SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+  SmbiosType17SpdLib|MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
  
  [Components]

#
@@ -30,6 +32,9 @@ [Components]
MdePkg/Test/UnitTest/Library/BaseLib/BaseLibUnitTestsHost.inf
MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/GoogleTestBaseSafeIntLib.inf
  
+  # Build HOST_APPLICATION that tests the SmbiosType17SpdLib

+  
MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SmbiosType17SpdLibUnitTestsHost.inf
+
#
# Build HOST_APPLICATION Libraries
#
diff --git a/MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf 
b/MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
new file mode 100644
index ..97b6125e3673
--- /dev/null
+++ b/MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
@@ -0,0 +1,40 @@
+## @file
+#  Library to parse DDR SPD buffers and put that data into SMBIOS Type17 
tables.
+#
+#  Copyright (c) 2022, Qualcomm

[edk2-devel] [RFC PATCH 1/1] MdePkg: Add library to parse SPD data and create SMBIOS Type 17 table

2023-02-14 Thread Rebecca Cran

SmbiosType17SpdLib can parse a buffer containing SPD data from a DDR4
or DDR5 DIMM and construct an SMBIOS Type17 table.

Signed-off-by: Rebecca Cran 
---
 MdePkg/MdePkg.dec  
 |   3 +
 MdePkg/MdeLibs.dsc.inc 
 |   2 +
 MdePkg/MdePkg.dsc  
 |   2 +
 MdePkg/Test/MdePkgHostTest.dsc 
 |   5 +
 MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf   
 |  40 ++
 
MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SmbiosType17SpdLibUnitTestsHost.inf
 |  35 ++
 MdePkg/Include/IndustryStandard/SdramSpd.h 
 |  53 ++-
 MdePkg/Include/IndustryStandard/SdramSpdDdr4.h 
 |  23 ++
 MdePkg/Include/Library/SmbiosType17SpdLib.h
 |  37 ++
 MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLibInternal.h 
 | 108 ++
 MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SpdTestData.h  
 |  20 +
 MdePkg/Library/SmbiosType17SpdLib/SmbiosType17Ddr4.c   
 | 369 ++
 MdePkg/Library/SmbiosType17SpdLib/SmbiosType17Ddr5.c   
 | 348 +
 MdePkg/Library/SmbiosType17SpdLib/SmbiosType17Utils.c  
 | 405 
 MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SmbiosSpdUnitTest.c
 | 187 +
 MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SpdTestData.c  
 | 164 
 16 files changed, 1787 insertions(+), 14 deletions(-)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15b0..fa92e884dcbe 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -257,6 +257,9 @@ [LibraryClasses]
   #
   UnitTestLib|Include/Library/UnitTestLib.h
 
+  ## @libraryclass Provides service to generate SMBIOS Type 17 table from SPD 
data.
+  SmbiosType17SpdLib|Include/Library/SmbiosType17SpdLib.h
+
   ## @libraryclass Extension to BaseLib for host based unit tests that allows a
   #subset of BaseLib services to be hooked for emulation.
   #
diff --git a/MdePkg/MdeLibs.dsc.inc b/MdePkg/MdeLibs.dsc.inc
index 4580481cb580..4855972b9f0a 100644
--- a/MdePkg/MdeLibs.dsc.inc
+++ b/MdePkg/MdeLibs.dsc.inc
@@ -15,4 +15,6 @@ [LibraryClasses]
   ArmTrngLib|MdePkg/Library/BaseArmTrngLibNull/BaseArmTrngLibNull.inf
   
RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
   CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+  JedecJep106Lib|MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf
+  SmbiosType17SpdLib|MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
   
SmmCpuRendezvousLib|MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 32a852dc466e..fde11e7331e2 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -136,6 +136,8 @@ [Components]
   MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf
 
+  MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
+
 [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
   #
   # Add UEFI Target Based Unit Tests
diff --git a/MdePkg/Test/MdePkgHostTest.dsc b/MdePkg/Test/MdePkgHostTest.dsc
index b8b186dd8b17..0bf4a248ae97 100644
--- a/MdePkg/Test/MdePkgHostTest.dsc
+++ b/MdePkg/Test/MdePkgHostTest.dsc
@@ -20,7 +20,9 @@ [Defines]
 !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
 
 [LibraryClasses]
+  JedecJep106Lib|MdePkg/Library/BaseJedecJep106Lib/BaseJedecJep106Lib.inf
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+  SmbiosType17SpdLib|MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
 
 [Components]
   #
@@ -30,6 +32,9 @@ [Components]
   MdePkg/Test/UnitTest/Library/BaseLib/BaseLibUnitTestsHost.inf
   MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/GoogleTestBaseSafeIntLib.inf
 
+  # Build HOST_APPLICATION that tests the SmbiosType17SpdLib
+  
MdePkg/Test/UnitTest/Library/SmbiosType17SpdLib/SmbiosType17SpdLibUnitTestsHost.inf
+
   #
   # Build HOST_APPLICATION Libraries
   #
diff --git a/MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf 
b/MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
new file mode 100644
index ..97b6125e3673
--- /dev/null
+++ b/MdePkg/Library/SmbiosType17SpdLib/SmbiosType17SpdLib.inf
@@ -0,0 +1,40 @@
+## @file
+#  Library to parse DDR SPD buffers and put that data into SMBIOS Type17 
tables.
+#
+#  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights 
reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 1.29
+  BASE_NAME  = SmbiosType17SpdLib
+  FILE_GUID

Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add DDR5 SPD defs to IndustryStandard per JESD400-5A.01

2023-02-14 Thread Rebecca Cran


+Liming

On 2/14/23 08:36, Rebecca Cran wrote:

Copying the style of SdramSpdDdr4.h, add DDR5 SPD definitions to
SpdDdr5.h per JEDEC JESD400-5A.01 specification
(https://www.jedec.org/standards-documents/docs/jesd400-5a01).

Signed-off-by: Rebecca Cran 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Zhiguang Liu 
---
  MdePkg/Include/IndustryStandard/SpdDdr5.h | 539 ++
  1 file changed, 539 insertions(+)
  create mode 100644 MdePkg/Include/IndustryStandard/SpdDdr5.h

diff --git a/MdePkg/Include/IndustryStandard/SpdDdr5.h 
b/MdePkg/Include/IndustryStandard/SpdDdr5.h
new file mode 100644
index ..0646064ea0fd
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/SpdDdr5.h
@@ -0,0 +1,539 @@
+/** @file
+  This file contains definitions for DDR5 SPD.
+
+  Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
+  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Revision Reference:
+- DDR5 Serial Presence Detect (SPD), Document Release 1.1 (Jan 2023)
+  https://www.jedec.org/standards-documents/docs/jesd400-5a01
+**/
+
+#ifndef SPD_DDR5_H_
+#define SPD_DDR5_H_
+
+#pragma pack (push, 1)
+
+typedef union {
+  struct {
+UINT8BetaLevel3_0 :  4;///< Bits 3:0
+UINT8BytesTotal   :  3;///< Bits 6:4
+UINT8BetaLevel4   :  1;///< Bits 7:7
+  } Bits;
+  UINT8Data;
+} SPD5_DEVICE_DESCRIPTION_STRUCT;
+
+typedef union {
+  struct {
+UINT8Minor :  4;   ///< Bits 3:0
+UINT8Major :  4;   ///< Bits 7:4
+  } Bits;
+  UINT8Data;
+} SPD5_REVISION_STRUCT;
+
+typedef union {
+  struct {
+UINT8Type :  8;///< Bits 7:0
+  } Bits;
+  UINT8Data;
+} SPD5_DEVICE_TYPE_STRUCT;
+
+typedef union {
+  struct {
+UINT8ModuleType  :  4; ///< Bits 3:0
+UINT8HybridMedia :  3; ///< Bits 6:4
+UINT8Hybrid  :  1; ///< Bits 7:7
+  } Bits;
+  UINT8Data;
+} SPD5_MODULE_TYPE_STRUCT;
+
+typedef union {
+  struct {
+UINT8Density :  5; ///< Bits 4:0
+UINT8Die :  3; ///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_DENSITY_PACKAGE_STRUCT;
+
+typedef union {
+  struct {
+UINT8RowAddress:  5;   ///< Bits 4:0
+UINT8ColumnAddress :  3;   ///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_ADDRESSING_STRUCT;
+
+typedef union {
+  struct {
+UINT8Reserved : 5; ///< Bits 4:0
+UINT8IoWidth  : 3; ///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_IO_WIDTH_STRUCT;
+
+typedef union {
+  struct {
+UINT8BanksPerBankGroup : 3;///< Bits 2:0
+UINT8Reserved  : 2;///< Bits 4:3
+UINT8BankGroups: 3;///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_BANKS_STRUCT;
+
+typedef union {
+  struct {
+UINT8Reserved0   : 1;  ///< Bits 0:0
+UINT8Mbist_mPpr  : 1;  ///< Bits 1:1
+UINT8Reserved1   : 2;  ///< Bits 3:2
+UINT8BL32: 1;  ///< Bits 4:4
+UINT8SPprUndo_Lock   : 1;  ///< Bits 5:5
+UINT8Reserved2   : 1;  ///< Bits 6:6
+UINT8SPprGranularity : 1;  ///< Bits 7:7
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_BL32_POST_PACKAGE_REPAIR_STRUCT;
+
+typedef union {
+  struct {
+UINT8DcaTypesSupported : 2;///< Bits 1:0
+UINT8Reserved0 : 2;///< Bits 3:2
+UINT8Pasr  : 1;///< Bits 4:4
+UINT8Reserved1 : 3;///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_DCA_AND_PASR_STRUCT;
+
+typedef union {
+  struct {
+UINT8BoundedFault: 1;  ///< Bits 0:0
+UINT8x4RmwEcsWbSuppressionMrSelector : 1;  ///< Bits 1:1
+UINT8x4RmwEcsWriteBackSuppresion : 1;  ///< Bits 2:2
+UINT8WideTemperatureSense: 1;  ///< Bits 3:3
+UINT8Reserved: 4;  ///< Bits 7:4
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_FAULT_HANDLING_TEMP_SENSE_STRUCT;
+
+typedef union {
+  struct {
+UINT8Endurant:  2; ///< Bits 1:0
+UINT8Operational :  2; ///< Bits 3:2
+UINT8Nominal :  4; ///< Bits 7:4
+  } Bits;
+  UINT8Data;
+} SPD5_MODULE_NOMINAL_VOLTAGE_STRUCT;
+
+typedef union {
+  struct {
+UINT8

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Gupta, Pankaj via groups.io


On 2/14/2023 9:44 PM, Dionna Amalie Glaze wrote:


Adding the diff.

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..df51c2c050 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -123,7 +123,7 @@ AcceptAllMemory (
   }
 }

-  gBS->FreePool (AllDescMap);
+  //gBS->FreePool (AllDescMap);^M
 return Status;
   }


Thanks,
Pankaj





Do you want to propose this patch or shall I? Seems like a necessary fix.


I am doing that. Wanted confirmation from you.

Thanks,
Pankaj



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

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Dionna Glaze via groups.io

>
> Adding the diff.
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 6391d1f775..df51c2c050 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -123,7 +123,7 @@ AcceptAllMemory (
>   }
> }
>
> -  gBS->FreePool (AllDescMap);
> +  //gBS->FreePool (AllDescMap);^M
> return Status;
>   }
> >
> > Thanks,
> > Pankaj
> >
>

Do you want to propose this patch or shall I? Seems like a necessary fix.

-- 
-Dionna Glaze, PhD (she/her)


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

[edk2-devel] [PATCH 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer

2023-02-14 Thread Rebecca Cran

Add a new library, JedecJep106Lib which provides a service to return the
JEDEC JEP106 manufacturer string given the code and continuation bytes
values.

Signed-off-by: Rebecca Cran 
---
 MdePkg/MdePkg.dec|3 +
 MdePkg/MdePkg.dsc|2 +
 MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf |   25 +
 MdePkg/Include/Library/JedecJep106Lib.h  |   39 +
 MdePkg/Library/JedecJep106Lib/JedecJep106Lib.c   | 1862 
 5 files changed, 1931 insertions(+)

diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15b0..af4f013c9b22 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -257,6 +257,9 @@ [LibraryClasses]
   #
   UnitTestLib|Include/Library/UnitTestLib.h
 
+  ## @libraryclass Provides service to get the manufacturer given JEP106 bytes.
+  JedecJep106Lib|Include/Library/JedecJep106Lib.h
+
   ## @libraryclass Extension to BaseLib for host based unit tests that allows a
   #subset of BaseLib services to be hooked for emulation.
   #
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 32a852dc466e..e41d4947db9e 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -136,6 +136,8 @@ [Components]
   MdePkg/Library/CcProbeLibNull/CcProbeLibNull.inf
   MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf
 
+  MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf
+
 [Components.IA32, Components.X64, Components.ARM, Components.AARCH64]
   #
   # Add UEFI Target Based Unit Tests
diff --git a/MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf 
b/MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf
new file mode 100644
index ..b49e2ba720fd
--- /dev/null
+++ b/MdePkg/Library/JedecJep106Lib/JedecJep106Lib.inf
@@ -0,0 +1,25 @@
+## @file
+#  Instance of JEDEC JEP106 Library
+#
+#  JedecJep106Lib fetches the manufacturer string given the JEP106
+#  Code and Continuation Bytes.
+#
+#  Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights 
reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001d
+  BASE_NAME  = JedecJep106Lib
+  FILE_GUID  = d48d43d7-ba31-4463-9433-ccb233cf0df7
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = JedecJep106Lib
+
+[Sources]
+  JedecJep106Lib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
diff --git a/MdePkg/Include/Library/JedecJep106Lib.h 
b/MdePkg/Include/Library/JedecJep106Lib.h
new file mode 100644
index ..e40372330693
--- /dev/null
+++ b/MdePkg/Include/Library/JedecJep106Lib.h
@@ -0,0 +1,39 @@
+/** @file
+  Provides JEDEC JEP-106 Manufacturer functions.
+
+  Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef JEDEC_JEP106_LIB_H_
+#define JEDEC_JEP106_LIB_H_
+
+/**
+  Looks up the JEP-106 manufacturer.
+
+  @param Code  Last non-zero byte of the manufacturer's ID code.
+  @param ContinuationBytes Number of continuation bytes indicated in JEP-106.
+
+  @return The manufacturer string, or NULL if an error occurred or the
+  combination of Code and ContinuationBytes are not valid.
+
+**/
+CONST CHAR8 *
+Jep106GetManufacturerName (
+  IN UINT8  Code,
+  IN UINT8  ContinuationBytes
+  );
+
+/**
+ Returns the length of the longest manufacturer name.
+
+@return The length of the longest manufacturer name.
+
+**/
+UINTN
+Jep106GetLongestManufacturerName (
+  VOID
+  );
+
+#endif /* JEDEC_JEP106_LIB_H_ */
diff --git a/MdePkg/Library/JedecJep106Lib/JedecJep106Lib.c 
b/MdePkg/Library/JedecJep106Lib/JedecJep106Lib.c
new file mode 100644
index ..eb832115d61d
--- /dev/null
+++ b/MdePkg/Library/JedecJep106Lib/JedecJep106Lib.c
@@ -0,0 +1,1862 @@
+/** @file
+  Provides JEDEC JEP-106 Manufacturer functions.
+
+  Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+
+typedef struct {
+  UINT8  Code;
+  CONST CHAR8*Manufacturer;
+} JEDEC_MANUFACTURERS;
+
+// From JEP106BE, published Jan 2022.
+STATIC CONST JEDEC_MANUFACTURERS  Jep106ManufacturersBank1[] = {
+  { 0x01, "AMD"  },
+  { 0x02, "AMI"  },
+  { 0x83, "Fairchild"},
+  { 0x04, "Fujitsu"  },
+  { 0x85, "GTE"  },
+  { 0x86, "Harris"   },
+  { 0x07, "Hitachi"  },
+  { 0x08, "Inmos"},
+  { 0x89, "Intel"},
+  { 0x8A, "I.T.T."   },
+  { 0x0B, "Intersil" },
+  { 0x8C, "Monolithic Technologies"  },
+  { 0x0D, "Mostek"   },
+  { 0x0E, "Freescale (Motorola)" },
+  { 0x8F, "National"

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Dionna Glaze via groups.io

>
> Do you have any pointers on the IVARS service?  Documentation, guest
> code, host code?
>

Agh, I thought for sure there was a public API for VM owners to view
or change their UEFI variables, but I guess not. It's an
instance-specific small data store for nonvolatile memory like vTPM
and UEFI variables. It appears you can only set the variables through
cloud API at instance creation time. But this is how instances can be
shut down and brought back up on different machines and/or live
migrate to other machines and still have access to UEFI variables'
current values. Host code is all in Google's proprietary VMM,
Vanadium, but the device backend is really rather simple. The data
store service though, that's a matter of Cloud Scale Engineering.

> Background:  When moving to a SVSM-based setup where the svsm (with
> vtpm emulation) runs in vmpl0 and the edk2 firmware in vmpl1 we might
> likewise add a efi variable service to the svsm.
>

I thought EFI variables in Qemu were loaded and measured at launch
(OVMF_VARS.fd). If you want the current values of all uefi variables
in your SVSM attestation report, I think it's probably better to use
the EFI_CC_MEASUREMENT_PROTOCOL, right? Or is it specifically going to
be an SVSM service that attests itself with current stored variables,
or at least variables that are considered important enough to measure?

In any case, persistence in The Cloud (TM) remains a challenge in the
CC space. Discussion about what we should do about that should remain
on the coco mailing list. IVARS encrypts data with Google-managed
keys, so it wouldn't be directly applicable to SVSM NVRAM.

> If something usable already exists we don't need to reinvent the wheel.
>

Don't have to tell me twice. In the spirit of OSS collaboration and
product integrity, I think any CC offering's firmware should be public
and verifiably built. I'll keep pushing for that.

> thanks & take care,
>   Gerd
>


-- 
-Dionna Glaze, PhD (she/her)


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

Re: [edk2-devel] [PATCH] MdeModulePkg: allow PlatformBootManagerLib to use BootNext

2023-02-14 Thread Jeshua Smith via groups.io

Thanks for the reply.

The issue (and associated patch) is actually in BdsEntry.c, not 
PlatformBootManagerLib. The BdsEntry.c code is preventing our own 
PlatformBootManagerLib implementation from doing what it needs to do. We have 
copied and modified PlatformBootManagerLib for our platform as you suggest, but 
because BdsEntry.c is explicitly taking control of BootNext away from it, we're 
unable to do something we need to do on our platform (in this case responding 
to BMC's request via IPMI to temporarily modify the boot order of the upcoming 
boot device selection, similar to how the OS uses BootNext to request to 
temporarily modify the boot order of the upcoming boot device selection). My 
patch is adding a PCD to give that control back to PlatformBootManagerLib if 
the platform needs it, while still defaulting to keeping the restriction in 
place for platforms that don't want to allow their PlatformBootManagerLib to be 
able to use BootNext. To me it seems like the BdsEntry.c code is intentionally 
prohibiting something useful with no clear benefit. I understand that if 
PlatformBootManagerLib is allowed to modify BootNext it could potentially 
override a BootNext value that the OS had previously set, but it seems to me 
that if a platform is creating their own PlatformBootManagerLib and is writing 
BootNext in it, then it is the platform's responsibility to take that scenario 
into account and handle it in the best way for the platform.

We could fork BdsEntry.c to apply our patch, but it seems like the whole 
purpose of PlatformBootManagerLib is to allow the platform to do the 
customizations it needs in the library instead of needing to fork BdsEntry.c.

Does that make sense?

-Original Message-
From: Kinney, Michael D  
Sent: Monday, February 13, 2023 3:43 PM
To: Jeshua Smith ; Ni, Ray ; 
devel@edk2.groups.io; Demeter, Miki ; Wang, Jian J 
; Gao, Liming ; Gao, Zhichao 

Cc: Kinney, Michael D 
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: allow PlatformBootManagerLib to 
use BootNext

External email: Use caution opening links or attachments


Hi Jeshua,

Some comments on the interpretation of 'next' in BootNext.

The UEFI Specification's main objective is an interface between the platform 
firmware and an operating system.

This is a 2-way communications path.  The firmware passes critical information 
to the OS required for the OS to take over control of the platform from 
firmware and manage the platform from that point forward.  The UEFI 
Specification also provides interfaces for the OS to pass information back to 
the firmware.  Some of these are while the OS is running such as UEFI Runtime 
Services and ACPI methods.  Some other OS->FW communication mechanisms are 
through the state of UEFI Variables and passing UEFI Capsules across 
reset/reboot.

BootNext is an example of an OS->FW communication path for the OS to set the 
BootNext policy for the OS to perform some specific action by requesting the FW 
to perform a one-time alternate boot path after the next system reset/reboot.  
This is the intended use of BootNext within the UEFI Specification context.

The use of BootNext to select an alternate boot option between different FW 
components within the platform firmware in the current FW boot is not defined 
by the UEFI Specification.  Any conventions in that area are really and 
attribute of the firmware design/implementation.  In fact, firmware has to be 
very careful if it modifies the value of BootNext because the current value 
could have been set by the OS and the firmware must honor the OS request for 
BootNext.

The other element to be aware of is the first boot scenario where the platform 
firmware is booting for the very first time and no UEFI variables exist.  This 
is a case where the default set of boot options are established before 
additional ones are set by OS installation or by end users.  The firmware can 
set BootOrder and/or BootNext to any values it wants because there is no OS->FW 
communications in this first boot scenario.

Another case to consider is when a UEFI Platform is used as an appliance where 
there are a fixed set of boot options that can never be modified, and the 
platform will always force the same options with the same priority every boot.

The challenge for edk2 is that these various uses cases (and there are many 
more) are difficult to support in a single open-source implementation.  
PlatformBootManagerLib is an implementation that tries to cover common use 
cases, but this library is intended to be copied and modified if needed if a 
platform requires behavior that is not covered by the common use cases.

Mike


> -Original Message-
> From: Jeshua Smith 
> Sent: Monday, February 13, 2023 2:12 PM
> To: Ni, Ray ; Kinney, Michael D 
> ; devel@edk2.groups.io; Demeter, Miki 
> ; Wang, Jian J ; Gao, 
> Liming ; Gao, Zhichao 
> 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: allow 
> PlatformBootManagerLib to use BootNext
>
>

Re: [edk2-devel] [PATCH v1 1/1] ShellPkg: UefiShellDebug1CommandsLib: Uefi Config Tables in Dmem.c

2023-02-14 Thread Sunny Wang

Looks good to me. Thanks for working on this, Sam.
Just for others' information, I also had an offline discussion with Sam.
- This is change is based on UEFI 2.10 section 4.6. EFI Configuration Table 
& Properties Table 
https://uefi.org/specs/UEFI/2.10/04_EFI_System_Table.html#efi-configuration-table-properties-table.
- The link of pull request is https://github.com/tianocore/edk2/pull/4038

Reviewed-by: Sunny Wang 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Sam Kaynor via 
groups.io
Sent: 07 February 2023 21:20
To: devel@edk2.groups.io
Cc: Sam Kaynor ; Ray Ni ; Zhichao Gao 

Subject: [edk2-devel] [PATCH v1 1/1] ShellPkg: UefiShellDebug1CommandsLib: Uefi 
Config Tables in Dmem.c

Added entries for UEFI Config Tables not present in current
Dmem output.

Cc: Ray Ni 
Cc: Zhichao Gao 
Signed-off-by: Sam Kaynor 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf |  
9 ++
 ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c | 
89 ++--
 ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni | 
28 --
 3 files changed, 112 insertions(+), 14 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
index 74ad5facf6b1..3741dac5d94c 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
@@ -121,6 +121,7 @@ [Protocols]
   gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES

   gEfiSimplePointerProtocolGuid   ## SOMETIMES_CONSUMES

   gEfiCpuIo2ProtocolGuid  ## SOMETIMES_CONSUMES

+  gEfiHiiDatabaseProtocolGuid ## SOMETIMES_CONSUMES



 [Guids]

   gEfiGlobalVariableGuid  ## SOMETIMES_CONSUMES ## GUID

@@ -130,3 +131,11 @@ [Guids]
   gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable

   gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable

   gShellDebug1HiiGuid ## SOMETIMES_CONSUMES ## HII

+  gEfiMemoryAttributesTableGuid   ## SOMETIMES_CONSUMES ## SystemTable

+  gEfiRtPropertiesTableGuid   ## SOMETIMES_CONSUMES ## SystemTable

+  gEfiSystemResourceTableGuid ## SOMETIMES_CONSUMES ## SystemTable

+  gEfiDebugImageInfoTableGuid ## SOMETIMES_CONSUMES ## SystemTable

+  gEfiImageSecurityDatabaseGuid   ## SOMETIMES_CONSUMES ## SystemTable

+  gEfiJsonConfigDataTableGuid ## SOMETIMES_CONSUMES ## SystemTable

+  gEfiJsonCapsuleDataTableGuid## SOMETIMES_CONSUMES ## SystemTable

+  gEfiJsonCapsuleResultTableGuid  ## SOMETIMES_CONSUMES ## SystemTable

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
index c52c212a56f8..e2aed306d466 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
@@ -10,9 +10,16 @@


 #include "UefiShellDebug1CommandsLib.h"

 #include 

+#include 

 #include 

 #include 

 #include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 



 /**

   Make a printable character.

@@ -108,6 +115,18 @@ ShellCommandRunDmem (
   UINT64SalTableAddress;

   UINT64SmbiosTableAddress;

   UINT64MpsTableAddress;

+  UINT64DtbTableAddress;

+  UINT64MemoryAttributesTableAddress;

+  UINT64RtPropertiesTableAddress;

+  UINT64SystemResourceTableAddress;

+  UINT64DebugImageInfoTableAddress;

+  UINT64ImageExecutionTableAddress;

+  UINT64JsonConfigDataTableAddress;

+  UINT64JsonCapsuleDataTableAddress;

+  UINT64JsonCapsuleResultTableAddress;

+  UINT64MemoryRangeCapsuleAddress;

+  UINT64HiiDatabaseExportBufferAddress;

+  UINT64ConformanceProfileTableAddress;

   UINTN TableWalker;



   ShellStatus = SHELL_SUCCESS;

@@ -168,11 +187,23 @@ ShellCommandRunDmem (
 ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMEM_HEADER_ROW), 
gShellDebug1HiiHandle, (UINT64)(UINTN)Address, Size);

 DumpHex (2, (UINTN)Address, (UINTN)Size, Address);

 if (Address == (VOID *)gST) {

-  Acpi20TableAddress = 0;

-  AcpiTableAddress   = 0;

-  SalTableAddress= 0;

-  SmbiosTableAddress = 0;

-  MpsTableAddress= 0;

+  Acpi20TableAddress = 0;

+  AcpiTableAddress   = 0;

+  SalTableAddress= 0;

+  SmbiosTableAddress = 0;

+  MpsTableAddress= 0;

+  DtbTableAddress= 0;

+  MemoryAttributesTableAddress   = 0;

+  RtPropertiesTableAddress   = 0;

+  SystemResourceTableAddress = 0;

+  DebugImageInfoTableAddress = 0;

+

Re: [edk2-devel] [PATCH 1/1] MdePkg: Add DDR5 SPD defs to IndustryStandard per JESD400-5A.01

2023-02-14 Thread Rebecca Cran


On 2/10/23 11:01, Kinney, Michael D wrote:

We usually do not include basetype includes from .h files in 
Protocol/PPI/GUID/IndustryStandard.

This is because we always get base types from AutoGen.h and from C/H files in 
modules/libs
That have to include top level include files for their module type. Base.h, 
PiPei.h, PiDxe,h, Uefi.h.

Also UINT8 is available from Base.h that does not assume UEFI or PI env.


Thanks. I've sent out a v2 patch with the include line removed.

--
Rebecca Cran



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

[edk2-devel] [PATCH v2 1/1] MdePkg: Add DDR5 SPD defs to IndustryStandard per JESD400-5A.01

2023-02-14 Thread Rebecca Cran

Copying the style of SdramSpdDdr4.h, add DDR5 SPD definitions to
SpdDdr5.h per JEDEC JESD400-5A.01 specification
(https://www.jedec.org/standards-documents/docs/jesd400-5a01).

Signed-off-by: Rebecca Cran 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Zhiguang Liu 
---
 MdePkg/Include/IndustryStandard/SpdDdr5.h | 539 ++
 1 file changed, 539 insertions(+)
 create mode 100644 MdePkg/Include/IndustryStandard/SpdDdr5.h

diff --git a/MdePkg/Include/IndustryStandard/SpdDdr5.h 
b/MdePkg/Include/IndustryStandard/SpdDdr5.h
new file mode 100644
index ..0646064ea0fd
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/SpdDdr5.h
@@ -0,0 +1,539 @@
+/** @file
+  This file contains definitions for DDR5 SPD.
+
+  Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
+  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Revision Reference:
+- DDR5 Serial Presence Detect (SPD), Document Release 1.1 (Jan 2023)
+  https://www.jedec.org/standards-documents/docs/jesd400-5a01
+**/
+
+#ifndef SPD_DDR5_H_
+#define SPD_DDR5_H_
+
+#pragma pack (push, 1)
+
+typedef union {
+  struct {
+UINT8BetaLevel3_0 :  4;///< Bits 3:0
+UINT8BytesTotal   :  3;///< Bits 6:4
+UINT8BetaLevel4   :  1;///< Bits 7:7
+  } Bits;
+  UINT8Data;
+} SPD5_DEVICE_DESCRIPTION_STRUCT;
+
+typedef union {
+  struct {
+UINT8Minor :  4;   ///< Bits 3:0
+UINT8Major :  4;   ///< Bits 7:4
+  } Bits;
+  UINT8Data;
+} SPD5_REVISION_STRUCT;
+
+typedef union {
+  struct {
+UINT8Type :  8;///< Bits 7:0
+  } Bits;
+  UINT8Data;
+} SPD5_DEVICE_TYPE_STRUCT;
+
+typedef union {
+  struct {
+UINT8ModuleType  :  4; ///< Bits 3:0
+UINT8HybridMedia :  3; ///< Bits 6:4
+UINT8Hybrid  :  1; ///< Bits 7:7
+  } Bits;
+  UINT8Data;
+} SPD5_MODULE_TYPE_STRUCT;
+
+typedef union {
+  struct {
+UINT8Density :  5; ///< Bits 4:0
+UINT8Die :  3; ///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_DENSITY_PACKAGE_STRUCT;
+
+typedef union {
+  struct {
+UINT8RowAddress:  5;   ///< Bits 4:0
+UINT8ColumnAddress :  3;   ///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_ADDRESSING_STRUCT;
+
+typedef union {
+  struct {
+UINT8Reserved : 5; ///< Bits 4:0
+UINT8IoWidth  : 3; ///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_IO_WIDTH_STRUCT;
+
+typedef union {
+  struct {
+UINT8BanksPerBankGroup : 3;///< Bits 2:0
+UINT8Reserved  : 2;///< Bits 4:3
+UINT8BankGroups: 3;///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_BANKS_STRUCT;
+
+typedef union {
+  struct {
+UINT8Reserved0   : 1;  ///< Bits 0:0
+UINT8Mbist_mPpr  : 1;  ///< Bits 1:1
+UINT8Reserved1   : 2;  ///< Bits 3:2
+UINT8BL32: 1;  ///< Bits 4:4
+UINT8SPprUndo_Lock   : 1;  ///< Bits 5:5
+UINT8Reserved2   : 1;  ///< Bits 6:6
+UINT8SPprGranularity : 1;  ///< Bits 7:7
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_BL32_POST_PACKAGE_REPAIR_STRUCT;
+
+typedef union {
+  struct {
+UINT8DcaTypesSupported : 2;///< Bits 1:0
+UINT8Reserved0 : 2;///< Bits 3:2
+UINT8Pasr  : 1;///< Bits 4:4
+UINT8Reserved1 : 3;///< Bits 7:5
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_DCA_AND_PASR_STRUCT;
+
+typedef union {
+  struct {
+UINT8BoundedFault: 1;  ///< Bits 0:0
+UINT8x4RmwEcsWbSuppressionMrSelector : 1;  ///< Bits 1:1
+UINT8x4RmwEcsWriteBackSuppresion : 1;  ///< Bits 2:2
+UINT8WideTemperatureSense: 1;  ///< Bits 3:3
+UINT8Reserved: 4;  ///< Bits 7:4
+  } Bits;
+  UINT8Data;
+} SPD5_SDRAM_FAULT_HANDLING_TEMP_SENSE_STRUCT;
+
+typedef union {
+  struct {
+UINT8Endurant:  2; ///< Bits 1:0
+UINT8Operational :  2; ///< Bits 3:2
+UINT8Nominal :  4; ///< Bits 7:4
+  } Bits;
+  UINT8Data;
+} SPD5_MODULE_NOMINAL_VOLTAGE_STRUCT;
+
+typedef union {
+  struct {
+UINT8NonStandardCoreTimings :  1;  ///< Bits 0:0
+

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Michael Kubacki


I know the second case was missed, that will be updated.

I agree calculating the remaining buffer space is more straightforward 
here without the library so I'll go with that approach in a v4 of the 
series.


Thanks for the detailed feedback.

On 2/14/2023 9:11 AM, Gerd Hoffmann wrote:

   Hi,


[ ... details snipped ... ]

I'd prefer it if the code were updated to avoid SafeUintnAdd() altogether.
But if not, then at a minimum the redundant check should be removed, and the
calculation involving Smbios.Hdr->Length should also be updated to use
SafeUintnAdd().


Fully agreeing to that.

take care,
   Gerd








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

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Gerd Hoffmann

  Hi,

> [ ... details snipped ... ]
> 
> I'd prefer it if the code were updated to avoid SafeUintnAdd() altogether.
> But if not, then at a minimum the redundant check should be removed, and the
> calculation involving Smbios.Hdr->Length should also be updated to use
> SafeUintnAdd().

Fully agreeing to that.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Gupta, Pankaj via groups.io


On 2/14/2023 1:55 PM, Gupta, Pankaj via groups.io wrote:

On 2/14/2023 1:51 PM, Gupta, Pankaj wrote:



ConvertPages: range 100 - 41AEFFF covers multiple entries
ConvertPages: range 100 - 41AEFFF covers multiple entries
Accepting all memory
Accepting all memory
Accepting all memory
Accepting all memory
EFI stub: ERROR: exit_boot() failed!
EFI stub: ERROR: efi_main() failed!
StartImage failed: Invalid Parameter
Thanks,
Pankaj


4 calls is telling me that "Accepting all memory" is somehow modifying
the memory map each call, but that shouldn't be happening. You've
confirmed that the body of the loop is getting skipped after the first
call?


yes. This also changes the memory key every time. Below change solves 
the issue
and don't even get even the second invocation of "Accepting all 
memory" message. It seems "gBS->FreePool" changes the memory map every 
time?


See like there is a typo s/gBS/gDS :) Now also getting two printfs for
"Accepting all memory", which seems right thing. Thank you, for the 
pointers!


--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -123,7 +123,7 @@ AcceptAllMemory (
 }
   }

-  gBS->FreePool (AllDescMap);
+  gDS->FreePool (AllDescMap);^M
   return Status;
 }



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

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Michael Kubacki


Either approach works for me.

I understand the desire to avoid code bloat that comes with the library.

The most common classes of issues I see at the moment are asserts being 
misused for error handling (which is significant), general issues with 
integer conversion/evaluation, and unsafe arithmetic operations.


I suppose this is in the spirit of Gerd's comment as I thought 
additional library usage might increase awareness to help with the latter.


Do the MdeModulePkg / SMBIOS maintainers have a preference?

Thanks,
Michael

On 2/14/2023 8:01 AM, Gerd Hoffmann wrote:

On Mon, Feb 13, 2023 at 04:15:30PM +, Michael Brown wrote:

On 13/02/2023 15:48, Michael Kubacki wrote:

@@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable (
   //
   // Make sure not to access memory beyond SmbiosEnd
   //
-if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) ||
-(Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw))
-{
+Status = SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUCTURE), 
);
+if (EFI_ERROR (Status)) {
+  return EFI_INVALID_PARAMETER;
+}
+
+if ((SafeIntResult > (UINTN)SmbiosEnd.Raw) || (SafeIntResult < 
(UINTN)Smbios.Raw)) {
 return EFI_INVALID_PARAMETER;
   }


Could this not be expressed much more cleanly as just:

   if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < sizeof (SMBIOS_STRUCTURE)) {
 return EFI_INVALID_PARAMETER;
   }

without the need to hide a basic arithmetic operation behind an ugly wrapper
function and drag in an external library?


Well, the advantage of using SafeIntLib is that (a) it is obvious even
to untrained code readers what is going on here, and (b) it is hard to
get it wrong.

When looking at the quality some of the patches being posted to the list
have I'd say that is important to consider even if you personally have
no problems avoiding integer overflows without the help of SafeIntLib.

take care,
   Gerd



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

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Michael Brown


On 14/02/2023 13:01, Gerd Hoffmann wrote:

On Mon, Feb 13, 2023 at 04:15:30PM +, Michael Brown wrote:

On 13/02/2023 15:48, Michael Kubacki wrote:

@@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable (
   //
   // Make sure not to access memory beyond SmbiosEnd
   //
-if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) ||
-(Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw))
-{
+Status = SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUCTURE), 
);
+if (EFI_ERROR (Status)) {
+  return EFI_INVALID_PARAMETER;
+}
+
+if ((SafeIntResult > (UINTN)SmbiosEnd.Raw) || (SafeIntResult < 
(UINTN)Smbios.Raw)) {
 return EFI_INVALID_PARAMETER;
   }


Could this not be expressed much more cleanly as just:

   if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < sizeof (SMBIOS_STRUCTURE)) {
 return EFI_INVALID_PARAMETER;
   }

without the need to hide a basic arithmetic operation behind an ugly wrapper
function and drag in an external library?


Well, the advantage of using SafeIntLib is that (a) it is obvious even
to untrained code readers what is going on here, and (b) it is hard to
get it wrong.

When looking at the quality some of the patches being posted to the list
have I'd say that is important to consider even if you personally have
no problems avoiding integer overflows without the help of SafeIntLib.


Fair enough.  But in that case it should be used in a way that makes it 
clear what it's actually doing.  Specifically, the check for


  if (... || (SafeIntResult < (UINTN)Smbios.Raw)) {
...
  }

then becomes meaningless, since the whole point of SafeUintnAdd() is 
that it cannot result in this kind of unsigned integer wraparound.  The 
code as modified by this patch is *harder* to understand, because the 
reader has to dig through to figure out why this check still exists, 
look at the implementation of SafeUintnAdd() to see what is meant by 
"safe" in this context, and finally come to the conclusion that the 
remaining underflow check is redundant code that should have been removed.


The reader is also going to be confused by the fact that the code as 
modified by this patch then contains two separate methods for checking 
for integer overflows, since the patch does not similarly update the 
very similar code found almost immediately afterwards:


  if ((Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) > 
SmbiosEnd.Raw) ||
(Smbios.Raw + Smbios.Hdr->Length + 2 * sizeof (UINT8) < 
Smbios.Raw))

{
  return EFI_INVALID_PARAMETER;
}


Lastly: the actual operation being made safe here is "check that buffer 
contains at least this much data remaining".  This is most obviously 
done by calculating the remaining buffer space (i.e. 
(UINTN)(SmbiosEnd.Raw - Smbios.Raw)) and comparing against that.  That 
logic is clear, simple, and obviously correct at first glance.  Using 
the SafeUintnAdd() call does something that *is* mathematically 
equivalent to this check, but where the logic is much harder to parse.



I'd prefer it if the code were updated to avoid SafeUintnAdd() 
altogether.  But if not, then at a minimum the redundant check should be 
removed, and the calculation involving Smbios.Hdr->Length should also be 
updated to use SafeUintnAdd().


Thanks,

Michael



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

Re: [edk2-devel] [PATCH 1/1] ArmPkg/PlatformBootManagerLib: Add path to boot UEFI Shell over UiApp

2023-02-14 Thread Patrik Berglund


Tested-by: Patrik Berglund 

Regards,
Patrik

On 2023-02-07 09:06, pierre.gond...@arm.com wrote:

From: Pierre Gondois 

The UEFI Shell is a non-active boot option, at the opposite of UiApp.
If no valid boot option is found, UiApp is selected. UiApp requires a
human interaction. When installing a new EDKII image in CIs or when
scripting is required, this is problematic.

If no valid boot option is discovered, add a path to directly go to
the UEFI Shell where the startup.nsh script is automatically executed.
The UEFI Shell is launched after connecting possible devices, but
before the reset that is meant to automatically make them visible.

The new PcdUefiShellDefaultBootEnable must be set to TRUE to enable
this behaviour. The Pcd is set to false by default.

Signed-off-by: Pierre Gondois 
---
  ArmPkg/ArmPkg.dec |  9 ++-
  .../PlatformBootManagerLib/PlatformBm.c   | 69 ++-
  .../PlatformBootManagerLib.inf|  4 +-
  3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index f17ba913e6de..257ae58a 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -2,7 +2,7 @@
  # ARM processor package.
  #
  # Copyright (c) 2009 - 2010, Apple Inc. All rights reserved.
-# Copyright (c) 2011 - 2022, ARM Limited. All rights reserved.
+# Copyright (c) 2011 - 2023, ARM Limited. All rights reserved.
  # Copyright (c) 2021, Ampere Computing LLC. All rights reserved.
  #
  #SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -221,6 +221,13 @@ [PcdsFixedAtBuild.common]
#
gArmTokenSpaceGuid.PcdArmDmaDeviceOffset|0x0|UINT64|0x044
  
+  #

+  # Boot the Uefi Shell instead of UiApp when no valid boot option is found.
+  # This is useful in CI environment so that startup.nsh can be launched.
+  # The default value is FALSE.
+  #
+  gArmTokenSpaceGuid.PcdUefiShellDefaultBootEnable|FALSE|BOOLEAN|0x052
+
  [PcdsFixedAtBuild.common, PcdsPatchableInModule.common]
gArmTokenSpaceGuid.PcdFdBaseAddress|0|UINT64|0x002B
gArmTokenSpaceGuid.PcdFvBaseAddress|0|UINT64|0x002D
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c 
b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 2fb1a4aa4fb8..9bdc44d86b54 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -2,7 +2,7 @@
Implementation for PlatformBootManagerLib library class interfaces.
  
Copyright (C) 2015-2016, Red Hat, Inc.

-  Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.
+  Copyright (c) 2014 - 2023, Arm Ltd. All rights reserved.
Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
Copyright (c) 2016, Linaro Ltd. All rights reserved.
Copyright (c) 2021, Semihalf All rights reserved.
@@ -470,6 +470,61 @@ PlatformRegisterFvBootOption (
EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
  }
  
+/** Boot a Fv Boot Option.

+ *
+ * This function is useful for booting the UEFI Shell as it is loaded
+ * as a non active boot option.
+ *
+ * @param[in] FileGuid  The File GUID.
+ * @param[in] Description   String describing the Boot Option.
+ */
+STATIC
+VOID
+PlatformBootFvBootOption (
+  IN  CONST EFI_GUID  *FileGuid,
+  IN  CHAR16  *Description
+  )
+{
+  EFI_STATUS Status;
+  EFI_BOOT_MANAGER_LOAD_OPTION   NewOption;
+  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;
+  EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage;
+  EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
+
+  Status = gBS->HandleProtocol (
+  gImageHandle,
+  ,
+  (VOID **)
+  );
+  ASSERT_EFI_ERROR (Status);
+
+  EfiInitializeFwVolDevicepathNode (, FileGuid);
+  DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
+  ASSERT (DevicePath != NULL);
+  DevicePath = AppendDevicePathNode (
+ DevicePath,
+ (EFI_DEVICE_PATH_PROTOCOL *)
+ );
+  ASSERT (DevicePath != NULL);
+
+  Status = EfiBootManagerInitializeLoadOption (
+ ,
+ LoadOptionNumberUnassigned,
+ LoadOptionTypeBoot,
+ LOAD_OPTION_ACTIVE,
+ Description,
+ DevicePath,
+ NULL,
+ 0
+ );
+  ASSERT_EFI_ERROR (Status);
+  FreePool (DevicePath);
+
+  for ( ; ;) {
+EfiBootManagerBoot ();
+  }
+}
+
  STATIC
  VOID
  GetPlatformOptions (
@@ -1075,6 +1130,18 @@ PlatformBootManagerUnableToBoot (
EfiBootManagerConnectAll ();
EfiBootManagerRefreshAllBootOption ();
  
+  //

+  // Boot the 'UEFI Shell'. If the Pcd is not set, the UEFI Shell is not
+  // an active boot option and must be manually selected through UiApp
+  // (at least during the fist boot).
+  //
+  if (FixedPcdGetBool (PcdUefiShellDefaultBootEnable)) {
+PlatformBootFvBootOption (
+  ,
+  L"UEFI Shell (default)"
+  );
+  }
+
//
// Record

Re: [edk2-devel] [PATCH] RedfishPkg/RedfishConfigHandler: fix FreePool issue

2023-02-14 Thread Igor Kulchytskyy via groups.io

Hi Abner,
Yes, I will work on this to create the linked list.
Thank you,
Igor

-Original Message-
From: Chang, Abner 
Sent: Tuesday, February 14, 2023 1:55 AM
To: Igor Kulchytskyy ; devel@edk2.groups.io
Cc: Nickle Wang 
Subject: [EXTERNAL] RE: [PATCH] RedfishPkg/RedfishConfigHandler: fix FreePool 
issue


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

[AMD Official Use Only - General]

Hi Igor,
Thanks for catching this issue, I have a comment below inline.

> -Original Message-
> From: Igor Kulchytskyy 
> Sent: Saturday, February 11, 2023 5:43 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Nickle Wang
> 
> Subject: [PATCH] RedfishPkg/RedfishConfigHandler: fix FreePool issue
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> gRedfishDiscoveredToken buffer is allocated as one piece during protocol
> installed process, but deleted by parts during driver unload process.
>
> Cc: Abner Chang 
> Cc: Nickle Wang 
> Cc: Igor Kulchytskyy 
> Signed-off-by: Igor Kulchytskyy 
> ---
>  RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> index 96ac70f418..64b7fb7841 100644
> --- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> +++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> @@ -481,6 +481,7 @@ RedfishDiscoverProtocolInstalled (
>  ErrorReturn:
>if (gRedfishDiscoveredToken != NULL) {
>  FreePool (gRedfishDiscoveredToken);
> +gRedfishDiscoveredToken = NULL;
>}
>  }
>
> @@ -511,10 +512,10 @@ RedfishConfigHandlerDriverUnload (
>  gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
>}
>
> -  FreePool (ThisRedfishDiscoveredToken);
>ThisRedfishDiscoveredToken++;
>  }
>
> +FreePool (gRedfishDiscoveredToken);
I found here is a potential issue of gRedfishDiscoveredToken:
Due to RedfishDiscoverProtocolInstalled() may be called more than once in the 
case of multiple NIC installed on the system, means gRedfishDiscoveredToken 
will be overwritten by AllocateZeroPool() and result in memory leakage when 
unload the RedfishConfigHandler driver.
Could you please help to create an linked list (e.g. 
mRedfishDiscoveredTokenList) to record the newly allocated memory for 
gRedfishDiscoveredToken (I think we can rename it to just 
RedfishDiscoveredToken)?
So we can go through the link list to free RedfishDiscoveredToken allocated for 
each NIC when unload the driver.
Does this make sense?
Thanks
Abner

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


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

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Gerd Hoffmann

On Mon, Feb 13, 2023 at 04:15:30PM +, Michael Brown wrote:
> On 13/02/2023 15:48, Michael Kubacki wrote:
> > @@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable (
> >   //
> >   // Make sure not to access memory beyond SmbiosEnd
> >   //
> > -if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) ||
> > -(Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw))
> > -{
> > +Status = SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUCTURE), 
> > );
> > +if (EFI_ERROR (Status)) {
> > +  return EFI_INVALID_PARAMETER;
> > +}
> > +
> > +if ((SafeIntResult > (UINTN)SmbiosEnd.Raw) || (SafeIntResult < 
> > (UINTN)Smbios.Raw)) {
> > return EFI_INVALID_PARAMETER;
> >   }
> 
> Could this not be expressed much more cleanly as just:
> 
>   if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < sizeof (SMBIOS_STRUCTURE)) {
> return EFI_INVALID_PARAMETER;
>   }
> 
> without the need to hide a basic arithmetic operation behind an ugly wrapper
> function and drag in an external library?

Well, the advantage of using SafeIntLib is that (a) it is obvious even
to untrained code readers what is going on here, and (b) it is hard to
get it wrong.

When looking at the quality some of the patches being posted to the list
have I'd say that is important to consider even if you personally have
no problems avoiding integer overflows without the help of SafeIntLib.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Gupta, Pankaj via groups.io


On 2/14/2023 1:51 PM, Gupta, Pankaj wrote:



ConvertPages: range 100 - 41AEFFF covers multiple entries
ConvertPages: range 100 - 41AEFFF covers multiple entries
Accepting all memory
Accepting all memory
Accepting all memory
Accepting all memory
EFI stub: ERROR: exit_boot() failed!
EFI stub: ERROR: efi_main() failed!
StartImage failed: Invalid Parameter
Thanks,
Pankaj


4 calls is telling me that "Accepting all memory" is somehow modifying
the memory map each call, but that shouldn't be happening. You've
confirmed that the body of the loop is getting skipped after the first
call?


yes. This also changes the memory key every time. Below change solves 
the issue
and don't even get even the second invocation of "Accepting all memory" 
message. It seems "gBS->FreePool" changes the memory map every time?


+  //gBS->FreePool (AllDescMap);^M


Adding the diff.

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..df51c2c050 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -123,7 +123,7 @@ AcceptAllMemory (
 }
   }

-  gBS->FreePool (AllDescMap);
+  //gBS->FreePool (AllDescMap);^M
   return Status;
 }


Thanks,
Pankaj





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

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Gupta, Pankaj via groups.io





ConvertPages: range 100 - 41AEFFF covers multiple entries
ConvertPages: range 100 - 41AEFFF covers multiple entries
Accepting all memory
Accepting all memory
Accepting all memory
Accepting all memory
EFI stub: ERROR: exit_boot() failed!
EFI stub: ERROR: efi_main() failed!
StartImage failed: Invalid Parameter
Thanks,
Pankaj


4 calls is telling me that "Accepting all memory" is somehow modifying
the memory map each call, but that shouldn't be happening. You've
confirmed that the body of the loop is getting skipped after the first
call?


yes. This also changes the memory key every time. Below change solves 
the issue
and don't even get even the second invocation of "Accepting all memory" 
message. It seems "gBS->FreePool" changes the memory map every time?


+  //gBS->FreePool (AllDescMap);^M

Thanks,
Pankaj



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

[edk2-devel] [edk2-staging][PATCH] RedfishClientPkg: Add mechanism to reboot system if config is changed

2023-02-14 Thread Nickle Wang via groups.io

When system configuration is updated from RESTful interface, we need a
system reboot so that the changes can be applied. Introduce PCD
"PcdSystemRebootRequired" to RedfishClientPkg. RedfishFeatureUtility
library will enable this flag when system config is updated.
RedfishFeatureCore driver will check this flag and perform cold reboot
after all Redfish operations are finished. PCD "PcdSystemRebootTimeout"
is used to specify how many second BIOS will wait before reboot system.

Signed-off-by: Nickle Wang 
Cc: Abner Chang 
Cc: Igor Kulchytskyy 
Cc: Nick Ramirez 
---
 .../Library/RedfishFeatureUtilityLib.h|  3 +
 .../RedfishFeatureUtilityLib.c| 65 ---
 .../RedfishFeatureUtilityLib.inf  |  2 +
 RedfishClientPkg/RedfishClientPkg.dec |  8 ++-
 .../RedfishFeatureCoreDxe.c   | 18 +
 .../RedfishFeatureCoreDxe.h   |  3 +
 .../RedfishFeatureCoreDxe.inf |  5 ++
 7 files changed, 93 insertions(+), 11 deletions(-)

diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h 
b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
index 1325976d8c..bb5dc4f4ac 100644
--- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
+++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
@@ -2,6 +2,7 @@
   This file defines the Redfish Feature Utility Library interface.
 
   (C) Copyright 2021-2022 Hewlett Packard Enterprise Development LP
+  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -14,6 +15,8 @@
 #include 
 #include 
 
+#define REDFISH_ENABLE_SYSTEM_REBOOT() PcdSetBoolS(PcdSystemRebootRequired, 
TRUE)
+
 //
 // Definition of REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG
 //
diff --git 
a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c 
b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index bfd6fff2a7..9883a4d919 100644
--- 
a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ 
b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -2,7 +2,7 @@
   Redfish feature utility library implementation
 
   (C) Copyright 2020-2022 Hewlett Packard Enterprise Development LP
-  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -324,7 +324,12 @@ ApplyFeatureSettingsStringType (
   RedfishValue.Value.Buffer = FeatureValue;
 
   Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, 
RedfishValue);
-  if (EFI_ERROR (Status)) {
+  if (!EFI_ERROR (Status)) {
+//
+// Configuration changed. Enable system reboot flag.
+//
+REDFISH_ENABLE_SYSTEM_REBOOT();
+  } else {
 DEBUG ((DEBUG_ERROR, "%a, apply %s to %s failed: %r\n", __FUNCTION__, 
ConfigureLang, FeatureValue, Status));
   }
 } else {
@@ -385,7 +390,12 @@ ApplyFeatureSettingsNumericType (
   RedfishValue.Value.Integer = (INT64)FeatureValue;
 
   Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, 
RedfishValue);
-  if (EFI_ERROR (Status)) {
+  if (!EFI_ERROR (Status)) {
+//
+// Configuration changed. Enable system reboot flag.
+//
+REDFISH_ENABLE_SYSTEM_REBOOT();
+  } else {
 DEBUG ((DEBUG_ERROR, "%a, apply %s to 0x%x failed: %r\n", 
__FUNCTION__, ConfigureLang, FeatureValue, Status));
   }
 } else {
@@ -446,7 +456,12 @@ ApplyFeatureSettingsBooleanType (
   RedfishValue.Value.Boolean = FeatureValue;
 
   Status = RedfishPlatformConfigSetValue (Schema, Version, ConfigureLang, 
RedfishValue);
-  if (EFI_ERROR (Status)) {
+  if (!EFI_ERROR (Status)) {
+//
+// Configuration changed. Enable system reboot flag.
+//
+REDFISH_ENABLE_SYSTEM_REBOOT();
+  } else {
 DEBUG ((DEBUG_ERROR, "%a, apply %s to %a failed: %r\n", __FUNCTION__, 
ConfigureLang, (FeatureValue ? "True" : "False"), Status));
   }
 } else {
@@ -561,7 +576,12 @@ ApplyFeatureSettingsVagueType (
   FreePool (RedfishValue.Value.Buffer);
   RedfishValue.Value.Buffer = 
CurrentVagueValuePtr->Value->DataValue.CharPtr;
   Status = RedfishPlatformConfigSetValue (Schema, Version, 
ConfigureKeyLang, RedfishValue);
-  if (EFI_ERROR (Status)) {
+  if (!EFI_ERROR (Status)) {
+//
+// Configuration changed. Enable system reboot flag.
+//
+REDFISH_ENABLE_SYSTEM_REBOOT();
+  } else {
 DEBUG ((DEBUG_ERROR, "%a, apply %a to %a failed: %r\n", 
__FUNCTION__, ConfigureKeyLang, CurrentVagueValuePtr->Value->DataValue.CharPtr, 
Status));
   }
 } else {
@@ -585,7 +605,12 @@ ApplyFeatureSettingsVagueType (

Re: [edk2-devel] [PATCH 0/4] CryptoPkg/BaseCryptLib: avoid certain openssl library calls

2023-02-14 Thread Gerd Hoffmann

On Tue, Feb 14, 2023 at 01:17:55AM +, Yao, Jiewen wrote:
> Good work, Gerd!
> 
> Do you have any data on how many K can be saved?

Essentially we are down to a handfull of source files for SEC and PEI,
assuming both only need hash functions for tdx/tpm measurements.

https://github.com/kraxel/edk2/commit/58f323f68dfaeaf4b88a8658790f0b0a4b578642

SMM and DXE are still a significant increase in size and I don't see an
easy way around that.  Switching to the crypto driver should mitigate
that somewhat.  Don't have detailed numbers at hand.

take care,
  Gerd

> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Gerd
> > Hoffmann
> > Sent: Tuesday, February 14, 2023 3:20 AM
> > To: devel@edk2.groups.io
> > Cc: Oliver Steffen ; Pawel Polawski
> > ; Gerd Hoffmann 
> > Subject: [edk2-devel] [PATCH 0/4] CryptoPkg/BaseCryptLib: avoid certain
> > openssl library calls
> > 
> > In preparation for the openssl 3.0 switch ...
> > 
> > openssl 3.0 sneak preview (WIP still, does not yet pass CI) is at
> > https://github.com/kraxel/edk2/commits/openssl3
> > 
> > Gerd Hoffmann (4):
> >   CryptoPkg/BaseCryptLib: avoid using SHA1()
> >   CryptoPkg/BaseCryptLib: avoid using SHA256()
> >   CryptoPkg/BaseCryptLib: avoid using SHA384()
> >   CryptoPkg/BaseCryptLib: avoid using SHA512()
> > 
> >  .../Library/BaseCryptLib/Hash/CryptSha1.c | 16 --
> >  .../Library/BaseCryptLib/Hash/CryptSha256.c   | 16 --
> >  .../Library/BaseCryptLib/Hash/CryptSha512.c   | 32 +++
> >  3 files changed, 52 insertions(+), 12 deletions(-)
> > 
> > --
> > 2.39.1
> > 
> > 
> > 
> > 
> > 
> 

-- 



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

Re: [edk2-devel] [PATCH v7 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check

2023-02-14 Thread Gerd Hoffmann

On Tue, Feb 14, 2023 at 04:33:10PM +0800, Jiaxin Wu wrote:
> This patch is to replace mIsBsp by mBspApicId check.
> mIsBsp becomes the local variable (IsBsp), then it can be
> checked dynamically in the function. Instead, we define the
> mBspApicId, which is to record the BSP ApicId used for
> compare in SmmInitHandler. With this change, SmmInitHandler
> can be run in parallel during SMM init.
> 
> Note:
> This patch is the per-prepared work by refining the
> SmmInitHandler, then, we can do the next step to
> combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate)
> into one (gcSmiHandlerTemplate), the new SMI handler
> will call the SmmInitHandler in parallel to do the init.

Much better, thanks.

Reviewed-by: Gerd Hoffmann 

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v6 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info

2023-02-14 Thread Gerd Hoffmann

  Hi,

> In PEI module, it also has such assumption, so we don't pass in the
> HOB for the resolved smbase mem size, because we have avoided the
> possibility of error in the reference pi smm cpu driver.

So you essentially are hoping this will never ever change and hard-code
the 8k in both PEI module and PiSmmCpuDxeSmm.  I'd suggest to add a
field to the HOB struct instead.  If you want stick to the hardcoded 8k
please add a note saying so to the HOB struct description.

> > > +BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize *
> > (mMaxNumberOfCpus - 1));
> > 
> > I think correct is:
> > BufferPages = EFI_SIZE_TO_PAGES(TileSize * mMaxNumberOfCpus);
> > 
> 
> This is the existing code logic & it's correct, not wrong, I don't change it. 
> To understand that, we need understand the algorithm of smbase:
> 
> The SIZE_32KB covers the *several* SMI Entry and Save State of CPU 0, while 
> TileSize * (mMaxNumberOfCpus - 1) to cover Save State of CPU 1+, not include 
> the cpu0, so, it's the mMaxNumberOfCpus - 1. 

Ok, there is a longish comment in the source code explaining the tiling
(starting at line 639).

smram is 64k (16 pages), with pages 0-7 being unused, page 8 being the
smi handler, 9-14 unused again, page 15 holding cpu state.  Due to the
smi handler having an even page index and the cpu state page having a
odd page index you can use that tiling trick so you need only 8k not 32k
per additional cpu.

I agree, the existing code is correct.

> > > +if ((FamilyId == 4) || (FamilyId == 5)) {
> > > +  Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB);
> > 
> > Does that actually matter still?  I'm pretty sure we can safely use
> > "ASSERT(FamilyId > 5)" here.  Pentium processors have been built in
> > the last century, predating x64.
> 
> This is the existing code logic. I don't change it. If you think we don't 
> need it, please file Bugzilla for change.
> 
> > Beside that the code is broken for SMP, only cpu0 will get a properly
> > aligned smbase.  Not sure penium processors support SMP in the first
> > place though ...
> 
> I don't understand why "only cpu0 will get a properly aligned smbase"

When the cpu expects the smbase being 32k-aligned (as the comment in the
code explains) the tiling trick just doesn't work.  The whole buffer and
the smbase for cpu0 are properly aligned to 32k, but the smbase for cpu1
is not.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v7 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call

2023-02-14 Thread Gerd Hoffmann

On Tue, Feb 14, 2023 at 04:33:09PM +0800, Jiaxin Wu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4338
> 
> No need call InitializeMpSyncData during normal boot SMI init,
> because mSmmMpSyncData is NULL at that time. mSmmMpSyncData is
> allocated in InitializeMpServiceData, which is invoked after
> normal boot SMI init (SmmRelocateBases).
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Zeng Star 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Signed-off-by: Jiaxin Wu 
> Reviewed-by: Ray Ni 

Acked-by: Gerd Hoffmann 



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

Re: [edk2-devel] [PATCH v7 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data

2023-02-14 Thread Gerd Hoffmann

On Tue, Feb 14, 2023 at 04:33:11PM +0800, Jiaxin Wu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4337
> 
> The default SMBASE for the x86 processor is 0x3. When
> SMI happens, CPU runs the SMI handler at SMBASE+0x8000.
> Also, the SMM save state area is within SMBASE+0x1.
> 
> One of the SMM initialization from CPU perspective is to relocate
> and program the new SMBASE (in TSEG range) for each CPU thread. When
> the SMBASE relocation happens in a PEI module, the PEI module shall
> produce the SMM_BASE_HOB in HOB database which tells the
> PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
> for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
> at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
> the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
> program the new SMBASE itself.
> 
> This patch adds the SMM Base HOB for any PEI module to do
> the SmBase relocation ahead of PiSmmCpuDxeSmm driver and
> store the relocated SmBase address in array for reach
> Processors.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Zeng Star 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: Rahul Kumar 
> Signed-off-by: Jiaxin Wu 
> ---
>  UefiCpuPkg/Include/Guid/SmmBaseHob.h | 64 
> 
>  UefiCpuPkg/UefiCpuPkg.dec|  5 ++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h
> 
> diff --git a/UefiCpuPkg/Include/Guid/SmmBaseHob.h 
> b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> new file mode 100644
> index 00..4aae0d23ff
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
> @@ -0,0 +1,64 @@
> +/** @file
> +  The Smm Base HOB is used to store the information of:
> +  * The relocated SmBase address in array for each Processors.
> +
> +  The default SMBASE for the x86 processor is 0x3. When SMI happens, CPU
> +  runs the SMI handler at SMBASE+0x8000. Also, the SMM save state area is 
> within
> +  SMBASE+0x1.
> +
> +  One of the SMM initialization from CPU perspective is to relocate and 
> program
> +  the new SMBASE (in TSEG range) for each CPU thread. When the SMBASE 
> relocation
> +  happens in a PEI module, the PEI module shall produce the SMM_BASE_HOB in 
> HOB
> +  database which tells the PiSmmCpuDxeSmm driver (which runs at a later 
> phase)
> +  about the new SMBASE for each CPU thread. PiSmmCpuDxeSmm driver installs 
> the
> +  SMI handler at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index.
> +  When the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and 
> program
> +  the new SMBASE itself.

This should also explain the consequences of the tiling allocation, i.e.
each cpu has one page at SMBASE+0x8000 for the SMI handler and one page
at SMBASE+0xF000 for the CPU state.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH v10 1/4] OvmfPkg: Add memory acceptance event in AmdSevDxe

2023-02-14 Thread Gerd Hoffmann

  Hi,

> A recent import found incompatibilities with measured boot only in
> SEV-SNP that we have disabled, but that's related to NVdata, which we
> deal with differently in GCE due to the cloud IVARS service and our
> allergy to SMM emulation. Should be unrelated.

Do you have any pointers on the IVARS service?  Documentation, guest
code, host code?

Background:  When moving to a SVSM-based setup where the svsm (with
vtpm emulation) runs in vmpl0 and the edk2 firmware in vmpl1 we might
likewise add a efi variable service to the svsm.

If something usable already exists we don't need to reinvent the wheel.

thanks & take care,
  Gerd



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

[edk2-devel] [PATCH v7 6/6] OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not

2023-02-14 Thread Wu, Jiaxin

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

This patch is to check SmBase relocation supported or not.
If gSmmBaseHobGuid found, means SmBase info has been relocated
and recorded in the SmBase array. ASSERT it's not supported in OVMF.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Ray Ni 
---
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c   | 10 +-
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf |  6 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 6693666d04..a1dd10c9f2 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -1,9 +1,9 @@
 /** @file
   The CPU specific programming for PiSmmCpuDxeSmm module.
 
-  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
 #include 
@@ -15,14 +15,16 @@
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 //
 // EFER register LMA bit
 //
 #define LMA  BIT10
@@ -41,10 +43,16 @@ EFIAPI
 SmmCpuFeaturesLibConstructor (
   IN EFI_HANDLEImageHandle,
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
+  //
+  // If gSmmBaseHobGuid found, means SmBase info has been relocated and 
recorded
+  // in the SmBase array. ASSERT it's not supported in OVMF.
+  //
+  ASSERT (GetFirstGuidHob () == NULL);
+
   //
   // No need to program SMRRs on our virtual platform.
   //
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf 
b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 8a426a4c10..2697a90525 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -1,9 +1,9 @@
 ## @file
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
-#  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
 
@@ -33,10 +33,14 @@
   MemoryAllocationLib
   PcdLib
   SafeIntLib
   SmmServicesTableLib
   UefiBootServicesTableLib
+  HobLib
+
+[Guids]
+  gSmmBaseHobGuid## CONSUMES
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
   gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
-- 
2.16.2.windows.1



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

[edk2-devel] [PATCH v7 5/6] UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration

2023-02-14 Thread Wu, Jiaxin

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

This patch is to avoid configure SMBASE if SmBase relocation has been
done. If gSmmBaseHobGuid found, means SmBase info has been relocated
and recorded in the SmBase array. No need to do the relocation in
SmmCpuFeaturesInitializeProcessor().

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Ray Ni 
---
 .../Library/SmmCpuFeaturesLib/CpuFeaturesLib.h |  2 ++
 .../SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c | 25 ++
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  6 +-
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  3 ++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c  |  3 +--
 .../StandaloneMmCpuFeaturesLib.inf |  6 +-
 6 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
index fd3e902547..c2e4fbe96b 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -7,15 +7,17 @@
 **/
 
 #ifndef CPU_FEATURES_LIB_H_
 #define CPU_FEATURES_LIB_H_
 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 /**
   Performs library initialization.
 
   This initialization function contains common functionality shared betwen all
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
index d5eaaa7a99..1a2c706fa1 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
@@ -1,9 +1,9 @@
 /** @file
 Implementation shared across all library instances.
 
-Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2023, Intel Corporation. All rights reserved.
 Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
@@ -36,10 +36,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 // Set default value to assume IA-32 Architectural MSRs are used
 //
 UINT32  mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
 UINT32  mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
 
+//
+// Indicate SmBase for each Processors has been relocated or not. If TRUE,
+// means no need to do the relocation in SmmCpuFeaturesInitializeProcessor().
+//
+BOOLEAN  mSmmCpuFeaturesSmmRelocated;
+
 //
 // Set default value to assume MTRRs need to be configured on each SMI
 //
 BOOLEAN  mNeedConfigureMtrrs = TRUE;
 
@@ -142,10 +148,16 @@ CpuFeaturesLibInitialization (
   //
   // Allocate array for state of SMRR enable on all CPUs
   //
   mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * 
GetCpuMaxLogicalProcessorNumber ());
   ASSERT (mSmrrEnabled != NULL);
+
+  //
+  // If gSmmBaseHobGuid found, means SmBase info has been relocated and 
recorded
+  // in the SmBase array.
+  //
+  mSmmCpuFeaturesSmmRelocated = (BOOLEAN)(GetFirstGuidHob () 
!= NULL);
 }
 
 /**
   Called during the very first SMI into System Management Mode to initialize
   CPU features, including SMBASE, for the currently executing CPU.  Since this
@@ -185,14 +197,19 @@ SmmCpuFeaturesInitializeProcessor (
   UINT32RegEdx;
   UINTN FamilyId;
   UINTN ModelId;
 
   //
-  // Configure SMBASE.
+  // No need to configure SMBASE if SmBase relocation has been done.
   //
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
-  CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+  if (!mSmmCpuFeaturesSmmRelocated) {
+//
+// Configure SMBASE.
+//
+CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE 
+ SMRAM_SAVE_STATE_MAP_OFFSET);
+CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+  }
 
   //
   // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
   // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
   //
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 9ac7dde78f..46ae2bf85e 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -1,9 +1,9 @@
 ## @file
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
-#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
 
 [Defines]
@@ -31,10 +31,14 @@
 [LibraryClasses]
   BaseLib
   PcdLib
   MemoryAllocationLib
   DebugLib
+  HobLib
+
+[Guids]
+  gSmmBaseHobGuid## CONSUMES
 
 [Pcd]

[edk2-devel] [PATCH v7 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info

2023-02-14 Thread Wu, Jiaxin

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

Existing SMBASE Relocation is in the PiSmmCpuDxeSmm driver, which
will relocate the SMBASE of each processor by setting the SMBASE
field in the saved state map (at offset 7EF8h) to a new value.
The RSM instruction reloads the internal SMBASE register with the
value in SMBASE field when each time it exits SMM. All subsequent
SMI requests will use the new SMBASE to find the starting address
for the SMI handler (at SMBASE + 8000h).

Due to the default SMBASE for all x86 processors is 0x3, the
APs' 1st SMI for rebase has to be executed one by one to avoid
the CPUs over-writing each other's SMM Save State Area (see
existing SmmRelocateBases() function), which means the next AP has
to wait for the previous AP to finish its 1st SMI, then it can call
into its 1st SMI for rebase via Smi Ipi command, thus leading the
existing SMBASE Relocation has to be running in series. Besides, it
needs very complex code to handle the AP exit semaphore
(mRebased[Index]), which will hook return address of SMM Save State
so that semaphore code can be executed immediately after AP exits
SMM for SMBASE relocation (see existing SemaphoreHook() function).

With SMM Base Hob support, PiSmmCpuDxeSmm does not need the RSM
instruction to do the SMBASE Relocation. SMBASE Register for each
processors have already been programmed and all SMBASE address have
recorded in SMM Base Hob. So the same default SMBASE Address
(0x3) will not be used, thus the CPUs over-writing each other's
SMM Save State Area will not happen in PiSmmCpuDxeSmm driver. This
way makes the first SMI init can be executed in parallel and save
boot time on multi-core system. Besides, Semaphore Hook code logic
is also not required, which will greatly simplify the SMBASE
Relocation flow.

Mainly changes as below:
* Assume the biggest possibility of tile size is 8k.
* Combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate) into one
(gcSmiHandlerTemplate), the new SMI handler needs to run to 2 paths:
one to SmmCpuFeaturesInitializeProcessor(), the other to SMM Core
Entry Point.
* Issue SMI IPI (All Excluding Self SMM IPI + BSP SMM IPI) for first
SMI init before normal SMI sources happen.
* Call SmmCpuFeaturesInitializeProcessor() in parallel.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c|  31 -
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c|  25 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 166 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |  26 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |   3 +-
 5 files changed, 214 insertions(+), 37 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index fb4a44eab6..d408b3f9f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -1,9 +1,9 @@
 /** @file
 Code for Processor S3 restoration
 
-Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include "PiSmmCpuDxeSmm.h"
@@ -822,13 +822,38 @@ SmmRestoreCpu (
 //
 InitializeCpuBeforeRebase ();
   }
 
   //
-  // Restore SMBASE for BSP and all APs
+  // Make sure the gSmmBaseHobGuid existence status is the same between normal 
and S3 boot.
   //
-  SmmRelocateBases ();
+  ASSERT (mSmmRelocated == (BOOLEAN)(GetFirstGuidHob () != 
NULL));
+  if (mSmmRelocated != (BOOLEAN)(GetFirstGuidHob () != NULL)) {
+DEBUG ((
+  DEBUG_ERROR,
+  "gSmmBaseHobGuid %a produced in normal boot but %a in S3 boot!",
+  mSmmRelocated ? "is" : "is not",
+  mSmmRelocated ? "is not" : "is"
+  ));
+CpuDeadLoop ();
+  }
+
+  //
+  // Check whether Smm Relocation is done or not.
+  // If not, will do the SmmBases Relocation here!!!
+  //
+  if (!mSmmRelocated) {
+//
+// Restore SMBASE for BSP and all APs
+//
+SmmRelocateBases ();
+  } else {
+//
+// Issue SMI IPI (All Excluding  Self SMM IPI + BSP SMM IPI) to execute 
first SMI init.
+//
+ExecuteFirstSmiInit ();
+  }
 
   //
   // Skip initialization if mAcpiCpuData is not valid
   //
   if (mAcpiCpuData.NumberOfCpus > 0) {
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index a0967eb69c..baf827cf9d 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1,9 +1,9 @@
 /** @file
 SMM MP service implementation
 
-Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -1721,17 +1721,40 @@ SmiRendezvous (
   UINTN   Index;
   UINTN   Cr2;
 
   ASSERT (CpuIndex <

[edk2-devel] [PATCH v7 3/6] UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data

2023-02-14 Thread Wu, Jiaxin

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

The default SMBASE for the x86 processor is 0x3. When
SMI happens, CPU runs the SMI handler at SMBASE+0x8000.
Also, the SMM save state area is within SMBASE+0x1.

One of the SMM initialization from CPU perspective is to relocate
and program the new SMBASE (in TSEG range) for each CPU thread. When
the SMBASE relocation happens in a PEI module, the PEI module shall
produce the SMM_BASE_HOB in HOB database which tells the
PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
program the new SMBASE itself.

This patch adds the SMM Base HOB for any PEI module to do
the SmBase relocation ahead of PiSmmCpuDxeSmm driver and
store the relocated SmBase address in array for reach
Processors.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
---
 UefiCpuPkg/Include/Guid/SmmBaseHob.h | 64 
 UefiCpuPkg/UefiCpuPkg.dec|  5 ++-
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h

diff --git a/UefiCpuPkg/Include/Guid/SmmBaseHob.h 
b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
new file mode 100644
index 00..4aae0d23ff
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/SmmBaseHob.h
@@ -0,0 +1,64 @@
+/** @file
+  The Smm Base HOB is used to store the information of:
+  * The relocated SmBase address in array for each Processors.
+
+  The default SMBASE for the x86 processor is 0x3. When SMI happens, CPU
+  runs the SMI handler at SMBASE+0x8000. Also, the SMM save state area is 
within
+  SMBASE+0x1.
+
+  One of the SMM initialization from CPU perspective is to relocate and program
+  the new SMBASE (in TSEG range) for each CPU thread. When the SMBASE 
relocation
+  happens in a PEI module, the PEI module shall produce the SMM_BASE_HOB in HOB
+  database which tells the PiSmmCpuDxeSmm driver (which runs at a later phase)
+  about the new SMBASE for each CPU thread. PiSmmCpuDxeSmm driver installs the
+  SMI handler at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index.
+  When the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and program
+  the new SMBASE itself.
+
+  Note:
+  SMBASE relocation process needs to program the vender specific hardware
+  interface to set SMBASE, it should be in the thread scope. It's doable to
+  program the hardware interface using DXE MP service protocol in 
PiSmmCpuDxeSmm
+  entry point. But, considering the standalone MM environment where the CpuMm
+  driver runs in a isolated environment and it cannot invoke any DXE or PEI MP
+  service, we recommend to put the hardware interface programming in a separate
+  PEI module instead of in the PiSmmCpuDxeSmm driver.
+
+  Copyright (c) 2023, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMM_BASE_HOB_H_
+#define SMM_BASE_HOB_H_
+
+#define SMM_BASE_HOB_DATA_GUID \
+  { \
+0xc2217ba7, 0x03bb, 0x4f63, {0xa6, 0x47, 0x7c, 0x25, 0xc5, 0xfc, 0x9d, 
0x73}  \
+  }
+
+#pragma pack(1)
+typedef struct {
+  ///
+  /// CpuIndex tells which CPU range this specific HOB instance described.
+  /// If CpuIndex is set to 0, it indicats the HOB describes the CPU from 0 to
+  /// NumberOfCpus - 1. The HOB list may contains multiple this HOB instances.
+  /// Each HOB instances describe the information for CPU from CpuIndex to
+  /// CpuIndex + NumberOfCpus - 1. The instance order in the HOB list is random
+  /// so consumer can not assume the CpuIndex of first instance is 0.
+  ///
+  UINT32CpuIndex;
+  ///
+  /// Describes the Number of all max supported processors.
+  ///
+  UINT32NumberOfProcessors;
+  ///
+  /// Pointer to SmBase address for each Processors.
+  ///
+  UINT64SmBase[];
+} SMM_BASE_HOB_DATA;
+#pragma pack()
+
+extern EFI_GUID  gSmmBaseHobGuid;
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index cff239d528..7003a2ba77 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -1,9 +1,9 @@
 ## @file  UefiCpuPkg.dec
 # This Package provides UEFI compatible CPU modules and libraries.
 #
-# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2023, Intel Corporation. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
 
@@ -76,10 +76,13 @@
   gEdkiiCpuFeaturesInitDoneGuid  = { 0xc77c3a41, 0x61ab, 0x4143, { 0x98, 0x3e, 
0x33, 0x39, 0x28, 0x6, 0x28, 0xe5 }}
 
   ## Include/Guid/MicrocodePatchHob.h
   gEdkiiMicrocodePatchHobGuid= { 0xd178f11d, 0x8716, 0x418e, { 0xa1, 0x31, 
0x96, 0x7d, 0x2a, 0xc4, 0x28, 0x43 }}
 
+  ## Include/Guid/SmmBaseHob.h
+  gSmmBaseHobGuid  = { 0xc2217ba7, 0x03bb, 0x4f63, {0xa6,

[edk2-devel] [PATCH v7 2/6] UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check

2023-02-14 Thread Wu, Jiaxin

This patch is to replace mIsBsp by mBspApicId check.
mIsBsp becomes the local variable (IsBsp), then it can be
checked dynamically in the function. Instead, we define the
mBspApicId, which is to record the BSP ApicId used for
compare in SmmInitHandler. With this change, SmmInitHandler
can be run in parallel during SMM init.

Note:
This patch is the per-prepared work by refining the
SmmInitHandler, then, we can do the next step to
combine 2 SMIs (gcSmmInitTemplate & gcSmiHandlerTemplate)
into one (gcSmiHandlerTemplate), the new SMI handler
will call the SmmInitHandler in parallel to do the init.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Ray Ni 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 2ac655d032..6e795d1756 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -57,11 +57,10 @@ SMM_CPU_PRIVATE_DATA  *gSmmCpuPrivate = 
 
 //
 // SMM Relocation variables
 //
 volatile BOOLEAN  *mRebased;
-volatile BOOLEAN  mIsBsp;
 
 ///
 /// Handle for the SMM CPU Protocol
 ///
 EFI_HANDLE  mSmmCpuHandle = NULL;
@@ -83,10 +82,12 @@ EDKII_SMM_MEMORY_ATTRIBUTE_PROTOCOL  mSmmMemoryAttribute = {
   EdkiiSmmClearMemoryAttributes
 };
 
 EFI_CPU_INTERRUPT_HANDLER  mExternalVectorTable[EXCEPTION_VECTOR_NUMBER];
 
+UINT32mBspApicId   = 0;
+
 //
 // SMM stack information
 //
 UINTN  mSmmStackArrayBase;
 UINTN  mSmmStackArrayEnd;
@@ -341,39 +342,42 @@ VOID
 EFIAPI
 SmmInitHandler (
   VOID
   )
 {
-  UINT32  ApicId;
-  UINTN   Index;
+  UINT32   ApicId;
+  UINTNIndex;
+  BOOLEAN  IsBsp;
 
   //
   // Update SMM IDT entries' code segment and load IDT
   //
   AsmWriteIdtr ();
   ApicId = GetApicId ();
 
+  IsBsp = (BOOLEAN)(mBspApicId == ApicId);
+
   ASSERT (mNumberOfCpus <= mMaxNumberOfCpus);
 
   for (Index = 0; Index < mNumberOfCpus; Index++) {
 if (ApicId == (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
   //
   // Initialize SMM specific features on the currently executing CPU
   //
   SmmCpuFeaturesInitializeProcessor (
 Index,
-mIsBsp,
+IsBsp,
 gSmmCpuPrivate->ProcessorInfo,
 
 );
 
   if (!mSmmS3Flag) {
 //
 // Check XD and BTS features on each processor on normal boot
 //
 CheckFeatureSupported ();
-  } else if (mIsBsp) {
+  } else if (IsBsp) {
 //
 // BSP rebase is already done above.
 // Initialize private data during S3 resume
 //
 InitializeMpSyncData ();
@@ -405,11 +409,10 @@ SmmRelocateBases (
 {
   UINT8 BakBuf[BACK_BUF_SIZE];
   SMRAM_SAVE_STATE_MAP  BakBuf2;
   SMRAM_SAVE_STATE_MAP  *CpuStatePtr;
   UINT8 *U8Ptr;
-  UINT32ApicId;
   UINTN Index;
   UINTN BspIndex;
 
   //
   // Make sure the reserved size is large enough for procedure SmmInitTemplate.
@@ -446,21 +449,20 @@ SmmRelocateBases (
   CopyMem (U8Ptr, gcSmmInitTemplate, gcSmmInitSize);
 
   //
   // Retrieve the local APIC ID of current processor
   //
-  ApicId = GetApicId ();
+  mBspApicId = GetApicId ();
 
   //
   // Relocate SM bases for all APs
   // This is APs' 1st SMI - rebase will be done here, and APs' default SMI 
handler will be overridden by gcSmmInitTemplate
   //
-  mIsBsp   = FALSE;
   BspIndex = (UINTN)-1;
   for (Index = 0; Index < mNumberOfCpus; Index++) {
 mRebased[Index] = FALSE;
-if (ApicId != (UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
+if (mBspApicId != 
(UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId) {
   SendSmiIpi ((UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId);
   //
   // Wait for this AP to finish its 1st SMI
   //
   while (!mRebased[Index]) {
@@ -475,12 +477,11 @@ SmmRelocateBases (
 
   //
   // Relocate BSP's SMM base
   //
   ASSERT (BspIndex != (UINTN)-1);
-  mIsBsp = TRUE;
-  SendSmiIpi (ApicId);
+  SendSmiIpi (mBspApicId);
   //
   // Wait for the BSP to finish its 1st SMI
   //
   while (!mRebased[BspIndex]) {
   }
-- 
2.16.2.windows.1



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

[edk2-devel] [PATCH v7 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call

2023-02-14 Thread Wu, Jiaxin

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

No need call InitializeMpSyncData during normal boot SMI init,
because mSmmMpSyncData is NULL at that time. mSmmMpSyncData is
allocated in InitializeMpServiceData, which is invoked after
normal boot SMI init (SmmRelocateBases).

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 
Reviewed-by: Ray Ni 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 655175a2c6..2ac655d032 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1,9 +1,9 @@
 /** @file
 Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
 
-Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -369,13 +369,11 @@ SmmInitHandler (
   if (!mSmmS3Flag) {
 //
 // Check XD and BTS features on each processor on normal boot
 //
 CheckFeatureSupported ();
-  }
-
-  if (mIsBsp) {
+  } else if (mIsBsp) {
 //
 // BSP rebase is already done above.
 // Initialize private data during S3 resume
 //
 InitializeMpSyncData ();
-- 
2.16.2.windows.1



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

[edk2-devel] [PATCH v7 0/6] Simplify SMM Relocation Process

2023-02-14 Thread Wu, Jiaxin

Existing SMBASE Relocation is in the PiSmmCpuDxeSmm driver, which
will relocate the SMBASE of each processor by setting the SMBASE
field in the saved state map (at offset 7EF8h) to a new value.
The RSM instruction reloads the internal SMBASE register with the
value in SMBASE field when each time it exits SMM. All subsequent
SMI requests will use the new SMBASE to find the starting address
for the SMI handler (at SMBASE + 8000h).

Due to the default SMBASE for all x86 processors is 0x3, the
APs' 1st SMI for rebase has to be executed one by one to avoid
the CPUs over-writing each other's SMM Save State Area (see
existing SmmRelocateBases() function), which means the next AP has
to wait for the previous AP to finish its 1st SMI, then it can call
into its 1st SMI for rebase via Smi Ipi command, thus leading the
existing SMBASE Relocation has to be running in series. Besides, it
needs very complex code to handle the AP exit semaphore
(mRebased[Index]), which will hook return address of SMM Save State
so that semaphore code can be executed immediately after AP exits
SMM for SMBASE relocation (see existing SemaphoreHook() function).

This series is to add the new SMM Base HOB for any PEI module to do
the SmBase relocation ahead of PiSmmCpuDxeSmm driver and store the
relocated SmBase address in array for each Processors. When the
SMBASE relocation happens in a PEI module, the PEI module shall
produce the SMM_BASE_HOB in HOB database which tells the
PiSmmCpuDxeSmm driver (runs at a later phase) about the new SMBASE
for each CPU thread. PiSmmCpuDxeSmm driver installs the SMI handler
at the SMM_BASE_HOB.SmBase[Index]+0x8000 for CPU thread Index. When
the HOB doesn't exist, PiSmmCpuDxeSmm driver shall relocate and
program the new SMBASE itself (keep existing SMBASE Relocation way).

With SMM Base Hob support, PiSmmCpuDxeSmm does not need the RSM
instruction to do the SMBASE Relocation. SMBASE Register for each
processors have already been programmed and all SMBASE address have
recorded in SMM Base Hob. So the same default SMBASE Address
(0x3) will not be used, thus the CPUs over-writing each other's
SMM Save State Area will not happen in PiSmmCpuDxeSmm driver. This
way makes the first SMI init can be executed in parallel and save
boot time on multi-core system. Besides, Semaphore Hook code logic
is also not required, which will greatly simplify the SMBASE
Relocation flow.

Note:
This is the new way that firmware can program the SMBASE
independently of the RSM instruction. The PEI code performing
this logic will not be open sourced, similarly to other things
that are kept binary-only in the FSP. Due to the register
difference in different vender, and it has not been documented
in the Intel SDM yet, we need a new binary-only interface for
SMM Base HOB.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Zeng Star 
Cc: Laszlo Ersek 
Cc: Gerd Hoffmann 
Cc: Rahul Kumar 
Signed-off-by: Jiaxin Wu 

Jiaxin Wu (6):
  UefiCpuPkg/PiSmmCpuDxeSmm: Fix invalid InitializeMpSyncData call
  UefiCpuPkg/PiSmmCpuDxeSmm: Replace mIsBsp by mBspApicId check
  UefiCpuPkg/SmmBaseHob.h: Add SMM Base HOB Data
  UefiCpuPkg/PiSmmCpuDxeSmm: Consume SMM Base Hob for SmBase info
  UefiCpuPkg/SmmCpuFeaturesLib: Skip SMBASE configuration
  OvmfPkg/SmmCpuFeaturesLib: Check SmBase relocation supported or not

 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  10 +-
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   6 +-
 UefiCpuPkg/Include/Guid/SmmBaseHob.h   |  64 +++
 .../Library/SmmCpuFeaturesLib/CpuFeaturesLib.h |   2 +
 .../SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c |  25 ++-
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   6 +-
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |   3 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c  |   3 +-
 .../StandaloneMmCpuFeaturesLib.inf |   6 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c  |  31 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  25 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 193 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  26 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   3 +-
 UefiCpuPkg/UefiCpuPkg.dec  |   5 +-
 15 files changed, 345 insertions(+), 63 deletions(-)
 create mode 100644 UefiCpuPkg/Include/Guid/SmmBaseHob.h

-- 
2.16.2.windows.1



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

61 matches

Mail list logo