Re: [edk2-devel] [PATCH v0] Platform/NXP: Add Dynamic Acpi for layerscape platforms

2021-01-26 Thread Vikas Singh via groups.io
On Tue, Jan 26, 2021 at 4:49 PM Leif Lindholm  wrote:
>
> Hi Vikas,
>
> On Tue, Jan 19, 2021 at 10:11:43 +0530, Vikas Singh wrote:
> > > > > > +/** A helper macro for returning configuration manager objects
> > > > > > +*/
> > > > > > +#define HANDLE_CM_OBJECT(ObjId, CmObjectId, Object, ObjectCount)   
> > > > > >\
> > > > > > +  case ObjId: {
> > > > > >\
> > > > > > +CmObject->ObjectId = CmObjectId;   
> > > > > >\
> > > > > > +CmObject->Size = sizeof (Object);  
> > > > > >\
> > > > > > +CmObject->Data = (VOID*)   
> > > > > >\
> > > > > > +CmObject->Count = ObjectCount; 
> > > > > >\
> > > > > > +DEBUG ((   
> > > > > >\
> > > > > > +  DEBUG_INFO,  
> > > > > >\
> > > > > > +  #CmObjectId ": Ptr = 0x%p, Size = %d, Count = %d\n", 
> > > > > >\
> > > > > > +  CmObject->Data,  
> > > > > >\
> > > > > > +  CmObject->Size,  
> > > > > >\
> > > > > > +  CmObject->Count  
> > > > > >\
> > > > > > +  ));  
> > > > > >\
> > > > > > +break; 
> > > > > >\
> > > > > > +  }
> > > > >
> > > > > This is code obfuscation. Please don't invent your own programming
> > > > > languages. In C, the case, the start bracket, the break and the end
> > > > > bracket always go inline.
> > > > > The rest would be better as a static helper function than a macro.
> > > > >
> > > > Leif, changes are in accordance with :
> > > > https://raw.githubusercontent.com/tianocore-docs/Docs/master/Specifications/CCS_2_1_Draft.pdf
> > >
> > > Do you mean 5.5.2.1:
> > > Functional macros are generally discouraged.
> > > ?
> >
> > Leif + Sami, I was referring section 5.7.3.7, since you commented on
> > switch case & break statement.
> > However keeping section 5.5.2.1 in consideration, I have done few
> > changes and shared updated V1 series.
> > Could you please have a look on it and revert, if in case you have any 
> > concerns.
>
> I have not seen any update?
>
> Please have a look at Sami's updates to the ARM platform code, which I
> merged yeaterday.
>
> Best Regards,
>
> Leif
>
Leif , I have sent an updated V2 patch series today. FYI.

Thnx.


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




[edk2-devel] [PATCH v2 2/2] Platform/NXP: Add OEM specific DSDT generator

2021-01-26 Thread Vikas Singh via groups.io
These changes add platform specific DSDT generator
and Clk dsdt properties for LX2160ARDB.

Signed-off-by: Vikas Singh 
---
 .../ConfigurationManagerDxe/ConfigurationManager.c |   9 ++
 .../ConfigurationManagerDxe/ConfigurationManager.h |   4 +-
 .../ConfigurationManagerDxe.inf|   1 +
 .../ConfigurationManagerPkg.dec|  23 
 .../Include/PlatformAcpiTableGenerator.h   |  20 +++
 .../LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Clk.asl   |  40 ++
 .../LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Dsdt.asl  |  15 +++
 .../AcpiTablesInclude/PlatformAcpiDsdtLib.inf  |  39 ++
 .../PlatformAcpiDsdtLib/RawDsdtGenerator.c | 146 +
 .../AcpiTablesInclude/PlatformAcpiLib.h|  24 
 Platform/NXP/LX2160aRdbPkg/Include/Platform.h  |   6 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec   |   3 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc   |   1 +
 13 files changed, 330 insertions(+), 1 deletion(-)
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/Include/PlatformAcpiTableGenerator.h
 create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Clk.asl
 create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Dsdt.asl
 create mode 100644 
Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
 create mode 100644 
Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
 create mode 100644 
Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiLib.h

diff --git 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
index bec9131..80ce841 100644
--- 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
+++ 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
@@ -78,6 +78,15 @@ FSL_PLATFORM_REPOSITORY_INFO FslPlatformRepositoryInfo = {
   CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSpcr),
   NULL,
   CFG_MGR_TABLE_ID
+},
+
+// DSDT (OEM) Table
+{
+  EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+  0,
+  CREATE_OEM_ACPI_TABLE_GEN_ID (PlatAcpiTableIdDsdt),
+  NULL,
+  CFG_MGR_TABLE_ID
 }
 
   },
diff --git 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
index ab320e5..0c7dd41 100644
--- 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
+++ 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
@@ -15,6 +15,7 @@
 #define CONFIGURATION_MANAGER_H
 
 #include 
+#include 
 
 /** The configuration manager version
 */
@@ -62,7 +63,8 @@ typedef EFI_STATUS (*CM_OBJECT_HANDLER_PROC) (
 
 /** The number of ACPI tables to install
 */
-#define PLAT_ACPI_TABLE_COUNT   5
+#define CM_MANDATORY_ACPI_TABLES  5
+#define PLAT_ACPI_TABLE_COUNT (CM_MANDATORY_ACPI_TABLES + OEM_ACPI_TABLES)
 
 /** A structure describing the platform configuration
 manager repository information
diff --git 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
index 496c8bf..7c082cb 100644
--- 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
+++ 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
@@ -31,6 +31,7 @@
   EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
+  Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
   Silicon/NXP/NxpQoriqLs.dec
 
 [LibraryClasses]
diff --git a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
new file mode 100644
index 000..e4af8e4
--- /dev/null
+++ b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
@@ -0,0 +1,23 @@
+#  ConfigurationManager.dec
+#
+#  Copyright 2020 NXP
+#  Copyright 2020 Puresoftware Ltd
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+[Defines]
+  PACKAGE_NAME   = ConfigurationManagerPkg
+  PACKAGE_GUID   = 0222b1b1-247f-404e-bdc3-baab65f2ddd3
+
+
+#
+# Include Section - list of Include Paths that are provided by this package.
+#   Comments are used for Keywords and Module Types.
+#
+# Supported Module Types:
+#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER 
DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
+#
+

[edk2-devel] [PATCH v2 1/2] Platform/NXP: Add Dynamic Acpi for layerscape platforms

2021-01-26 Thread Vikas Singh via groups.io
These changes intend to add Common Configuration Manager (CM)
for all fsl platforms and Platform headers consumed by CM for
LX2160ARDB.

Signed-off-by: Vikas Singh 
---
 .../ConfigurationManagerDxe/ConfigurationManager.c | 834 +
 .../ConfigurationManagerDxe/ConfigurationManager.h | 151 
 .../ConfigurationManagerDxe.inf|  51 ++
 Platform/NXP/Env.sh|  67 ++
 Platform/NXP/LX2160aRdbPkg/Include/Platform.h  | 240 ++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec   |   1 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc   |  28 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf   |  12 +
 Platform/NXP/Readme.md |  35 +-
 Platform/NXP/build.sh  | 115 +++
 Silicon/NXP/LX2160A/LX2160A.dsc.inc|   9 +
 11 files changed, 1542 insertions(+), 1 deletion(-)
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
 create mode 100755 Platform/NXP/Env.sh
 create mode 100644 Platform/NXP/LX2160aRdbPkg/Include/Platform.h
 create mode 100755 Platform/NXP/build.sh

diff --git 
a/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
new file mode 100644
index 000..bec9131
--- /dev/null
+++ 
b/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
@@ -0,0 +1,834 @@
+/** @file
+  Configuration Manager Dxe
+
+  Copyright 2020 NXP
+  Copyright 2020 Puresoftware Ltd
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Glossary:
+- Cm or CM   - Configuration Manager
+- Obj or OBJ - Object
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/** The platform specific headers
+*/
+#include "ConfigurationManager.h"
+#include 
+
+/** The platform configuration repository information.
+*/
+STATIC
+FSL_PLATFORM_REPOSITORY_INFO FslPlatformRepositoryInfo = {
+  /// Configuration Manager information
+  { CONFIGURATION_MANAGER_REVISION, CFG_MGR_OEM_ID },
+
+  // ACPI Table List
+  {
+// FADT Table
+{
+  EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
+  EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_REVISION,
+  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdFadt),
+  NULL,
+  CFG_MGR_TABLE_ID
+},
+
+// GTDT Table
+{
+  EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
+  EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION,
+  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdGtdt),
+  NULL,
+  CFG_MGR_TABLE_ID
+},
+
+// MADT Table
+{
+  EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
+  EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION,
+  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMadt),
+  NULL,
+  CFG_MGR_TABLE_ID
+},
+
+// PCI MCFG Table
+{
+  
EFI_ACPI_6_2_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
+  EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION,
+  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdMcfg),
+  NULL,
+  CFG_MGR_TABLE_ID
+},
+
+// SPCR Table
+{
+  EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
+  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION,
+  CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSpcr),
+  NULL,
+  CFG_MGR_TABLE_ID
+}
+
+  },
+
+  // Boot architecture information
+  { EFI_ACPI_6_2_ARM_PSCI_COMPLIANT },// BootArchFlags
+
+  // Power management profile information
+  { EFI_ACPI_6_2_PM_PROFILE_ENTERPRISE_SERVER }, // PowerManagement Profile
+
+  // Generic Timer Info
+  {
+// The physical base address for the counter control frame
+TIMER_BASE_ADDRESS,
+// The physical base address for the counter read frame
+TIMER_READ_BASE_ADDRESS,
+// The secure PL1 timer interrupt
+TIMER_SEC_IT,
+// The secure PL1 timer flags
+GTDT_GTIMER_FLAGS,
+// The non-secure PL1 timer interrupt
+TIMER_NON_SEC_IT,
+// The non-secure PL1 timer flags
+GTDT_GTIMER_FLAGS,
+// The virtual timer interrupt
+TIMER_VIRT_IT,
+// The virtual timer flags
+GTDT_GTIMER_FLAGS,
+// The non-secure PL2 timer interrupt
+TIMER_HYP_IT,
+// The non-secure PL2 timer flags
+GTDT_GTIMER_FLAGS
+  },
+
+  // Generic Timer Block Information
+  PLAT_TIMER_BLOCK_INFO,
+
+  // GTDT Frames
+  PLAT_TIMER_FRAME_INFO,
+
+  // Watchdog info
+  PLAT_WATCHDOG_INFO,
+
+  // GIC CPU Interface information
+  PLAT_GIC_CPU_INTERFACE,
+
+  // GIC Distributor Info
+  PLAT_GIC_DISTRIBUTOR_INFO,
+
+  // GIC 

[edk2-devel] [PATCH v2 0/2] Dynamic ACPI framework for fsl layerscape platforms

2021-01-26 Thread Vikas Singh via groups.io
This patch series sets the foundation of Dynamic ACPI framework for all
fsl layerscape platforms. In order to achieve:
  - Configurable firmware builds.
  - Unify firmware build for similar platforms.
  - Minimize/eliminate human induced errors.
  - Validate and generate firmware that complies with relevant specifications.
this change set will introduce following changes in below defined order under
edk2-platforms/NXP for LX2160ARDB platform.

(1) Introduced edk2-platforms/NXP/ConfigurationManager
It creates the platform repositories dynamically during build time
and initializes with platform specific information and serves all requestes
coming from OEM/standard firmware table generators. Configuration Manager(CM)
will be common for all fsl platforms.

(2) Introduced edk2-platforms/NXP/LX2160ARDB/Include/Platform.h
It has all the declarations and the definitions specified for the platforms.
These definitions will be inturn consumed by Configuration Manager.
Additionally the placement of this header under "Include" dir will make these
macro's availale to other translation units of the platform built.

(3) Introduced edk2-platforms/NXP/LX2160ARDB/AcpiTablesInclude
This is a placeholder for - OEM specific firmware acpi table generators.
This also includes IP specific - DSDT/SSDT generators for the OEM's platform.
Currently Dsdt.asl is a place holder having only platform's clock related dsdt
properties for LX2160ARDB but it is intended to extend this "Dsdt.asl" to hold
other table as well in next patch series.

(4) Introduced a new "DYNAMIC_ACPI_ENABLE" flag to control DACPI framework.
Currently this flag is used for LX2160ARDB and by default it is enabled.
This flag can also extend to other fsl layerscape platforms in future.
Changes can be referred under:
  - LX2160ARDb.dsc
  - LX2160ARDB.fdf

(5) Introduced a "build.sh" script under edk2-platforms/NXP/ to automate builds.
And also mandatorily need to support Dynamic ACPI framework for all fsl 
platforms.
It exports build environment variables and also invokes "Env.sh".

Vikas Singh (2):
  Platform/NXP: Add Dynamic Acpi for layerscape platforms
  Platform/NXP: Add OEM specific DSDT generator

 .../ConfigurationManagerDxe/ConfigurationManager.c | 843 +
 .../ConfigurationManagerDxe/ConfigurationManager.h | 153 
 .../ConfigurationManagerDxe.inf|  52 ++
 .../ConfigurationManagerPkg.dec|  23 +
 .../Include/PlatformAcpiTableGenerator.h   |  20 +
 Platform/NXP/Env.sh|  67 ++
 .../LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Clk.asl   |  40 +
 .../LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Dsdt.asl  |  15 +
 .../AcpiTablesInclude/PlatformAcpiDsdtLib.inf  |  39 +
 .../PlatformAcpiDsdtLib/RawDsdtGenerator.c | 146 
 .../AcpiTablesInclude/PlatformAcpiLib.h|  24 +
 Platform/NXP/LX2160aRdbPkg/Include/Platform.h  | 246 ++
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec   |   4 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc   |  29 +
 Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf   |  12 +
 Platform/NXP/Readme.md |  35 +-
 Platform/NXP/build.sh  | 115 +++
 Silicon/NXP/LX2160A/LX2160A.dsc.inc|   9 +
 18 files changed, 1871 insertions(+), 1 deletion(-)
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
 create mode 100644 
Platform/NXP/ConfigurationManagerPkg/Include/PlatformAcpiTableGenerator.h
 create mode 100755 Platform/NXP/Env.sh
 create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Clk.asl
 create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Dsdt.asl
 create mode 100644 
Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
 create mode 100644 
Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
 create mode 100644 
Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiLib.h
 create mode 100644 Platform/NXP/LX2160aRdbPkg/Include/Platform.h
 create mode 100755 Platform/NXP/build.sh

-- 
2.7.4



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




[edk2-devel] [Wiki] Add update notes for incomaptible patches that fix typos in SmBios.h

2021-01-26 Thread Zhiguang Liu
Cc: Liming Gao 
Signed-off-by: Zhiguang Liu 
---
 EDK-II-Release-Planning.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/EDK-II-Release-Planning.md b/EDK-II-Release-Planning.md
index 336d8d2..bbb5f18 100644
--- a/EDK-II-Release-Planning.md
+++ b/EDK-II-Release-Planning.md
@@ -32,6 +32,7 @@
 * If the user has the windows bat script that calls Split in it,it needs to 
change to "call Split" because Split will be a bat script but not an executable 
file.
 * Shell depends on library class OrderedCollectionLib. Platform DSC needs to 
configure it in [LibraryClasses]
 
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
+* Some struct fields in SmBios.h have typos and get fixed in these code change 
[0db8](https://github.com/tianocore/edk2/commit/0db89a661f38b10012ff4f62e1853bfc48efd462),
 
[bd9d](https://github.com/tianocore/edk2/commit/bd9da7b1da2639fcea8a156fa92a32bbc4209367),
 
[e157](https://github.com/tianocore/edk2/commit/e157c8f9ed173a390d2c9d29069a46e9662e0d04).
 Platform code that uses those fields need modifications.
 * TBD
 
 # edk2-stable202105 tag planning
-- 
2.30.0.windows.2



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




Re: [edk2-devel] Questions/doubts about UFS device on UEFI

2021-01-26 Thread Nate DeSimone
Hi Renan,



The UEFI driver model uses a stack of several modular DXE drivers to translate 
from lower level to higher level interfaces. As a baseline reference, this is 
what the driver stack would look like for a UFS device on an Intel SoC:



   +--+

   | EFI_BLOCK_IO(2)_PROTOCOL | <-- Represents a generic Hard Drive

   +--+

|

V

 *-*

 | ScsiDiskDxe |

 *-*

|

V

 +--+

 | EFI_SCSI_IO_PROTOCOL | <-- Represents a SCSI device on a SCSI Bus

 +--+

|

V

 **

 | ScsiBusDxe |

 **

|

V

+-+

| EFI_EXT_SCSI_PASS_THRU_PROTOCOL | <-- Represents a SCSI HBA (Host-Bus 
Adapter)

+-+

|

V

   **

   | UfsPassThruDxe |

   **

|

V

   +--+

   | EFI_UFS_HOST_CONTROLLER_PROTOCOL | <-- Represents a UFS Controller

   +--+

|

V

 *-*

 | UfsPciHcDxe |

 *-*

|

V

  +-+

  | EFI_PCI_IO_PROTOCOL | <-- Represents a PCI device on a PCI bus

  +-+

|

V

  *---*

  | PciBusDxe |

  *---*

|

V

+-+

| EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL | <-- Represents the PCIe root complex

+-+

|

V

*---*

| MdeModulePkg/Bus/Pci/PciHostBridgeDxe | <-- Requires SoCs specific 
PciHostBridgeLib

*---*

|

V

 +--+

 | EFI_CPU_IO2_PROTOCOL | <-- Provides the access to memory mapped I/O 
and legacy I/O ports

 +--+

|

V

 *--*

 | UefiCpuPkg/CpuIo2Dxe | <-- The only x86 specific driver in this stack

 *--*



The important thing to note about this diagram is that the UfsPciHcDxe device 
driver that is present in MdeModulePkg can only be used for UFS controllers 
that the SoC exposes as a PCI bus attached UFS controller. If the SoC exposes 
the UFS controller as a PCI device, this driver should work. However, if the 
SoC does not expose the UFS controller as a PCI device, then a SoC specific 
driver will be required to produce a device handle with an instance of 
EFI_UFS_HOST_CONTROLLER_PROTOCOL. You mentioned that no PCI devices with class 
01h and subclass 09h exist on this SoC, which means that a SoC specific driver 
is needed.



After the EFI_UFS_HOST_CONTROLLER_PROTOCOL instance is created, everything 
above EFI_UFS_HOST_CONTROLLER_PROTOCOL in the diagram above (including 
UfsPassThruDxe) will work without code change. The drivers below 
EFI_UFS_HOST_CONTROLLER_PROTOCOL will obviously be different.



Hope that Helps,

Nate

From: devel@edk2.groups.io  On Behalf Of renan.moraes2 
via groups.io
Sent: Tuesday, January 26, 2021 9:24 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] Questions/doubts about UFS device on UEFI

Good afternoon,

We are studying how to communicate with Universal Flash Storage (UFS) device on 
UEFI environment. The first step is identify the UFS device, we are using the 
system Lenovo Flex 5G (It has Snapdragon processor + UFS device).

We tried to identify the UFS device in three ways:

1 - Specific UEFI protocols (UFS_DEVICE_CONFIG_PROTOCOL / 
UFS_HOST_CONTROLLER_PROTOCOL) -> None of these protocols could be found on our 
test systems, we compiled the EDK II 2020 (edk2-stable202005) available drivers 
(UfsPassThru and UfsPciHc) and loaded them. However, these drivers can't detect 
the UFS device.

2 - Device Path -> Tried to identify by its type (Type 3 – Messaging Device 
Path) and subtype (Sub-Type 25 – UFS), but could not find any matches.

3 - Pci bus -> Tried to identify by its class (01h - Mass Storage Controller) 
and subclass (09h - Universal Flash Storage controller), but could not find any 
matches.

We would like to know if there is an alternative UEFI UFS driver or other 
methods to identify and communicate with this type of storage device.

Thank you in advance.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io 

Re: [edk2-devel] [PATCH v4 11/20] MdeModulePkg: SmmSmiHandlerProfileLib: Support StandaloneMm Instance

2021-01-26 Thread Wu, Hao A
> -Original Message-
> From: Kun Qin 
> Sent: Wednesday, January 27, 2021 3:47 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Wu, Hao A ;
> Dong, Eric ; Ni, Ray 
> Subject: [PATCH v4 11/20] MdeModulePkg: SmmSmiHandlerProfileLib:
> Support StandaloneMm Instance
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3185
> 
> This change added support of SMI handler profile library router under
> StandaloneMm. This change replaces gSmst with gMmst. It also abstracts
> standalone and traditional MM driver entrypoints into separate files to allow
> maximal common implementations.
> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Eric Dong 
> Cc: Ray Ni 
> 
> Signed-off-by: Kun Qin 
> ---
> 
> Notes:
> v4:
> - Newly created for SmmSmiHandlerProfileLib coverage.



Reviewed-by: Hao A Wu 

Best Regards,
Hao Wu


> 
> 
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/{SmmSmiHandlerProfileL
> ib.c => MmSmiHandlerProfileLib.c} | 20 ++---
> 
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLi
> b.c   | 90 ++--
> 
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/StandaloneMmSmiHandl
> erProfileLib.c  | 31 +++
> 
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfileLib
> .h| 23 +
> 
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLi
> b.inf |  4 +-
> 
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/StandaloneMmSmiHandl
> erProfileLib.inf| 44 ++
>  MdeModulePkg/MdeModulePkg.dsc
> |  1
> +
>  7 files changed, 117 insertions(+), 96 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfile
> Lib.c
> b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfile
> Lib.c
> similarity index 86%
> copy from
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLi
> b.c
> copy to
> MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfileLib
> .c
> index b76e8f0dc132..f800220b549c 100644
> ---
> a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfile
> Lib.c
> +++
> b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfile
> Li
> +++ b.c
> @@ -1,14 +1,15 @@
>  /** @file
> -  SMM driver instance of SmiHandlerProfile Library.
> +  MM driver instance of SmiHandlerProfile Library.
> 
>Copyright (c) 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) Microsoft Corporation.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> -#include 
> +#include 
>  #include  -#include
> 
> +#include 
>  #include 
> 
>  SMI_HANDLER_PROFILE_PROTOCOL  *mSmiHandlerProfile; @@ -82,21
> +83,16 @@ SmiHandlerProfileUnregisterHandler (  }
> 
>  /**
> -  The constructor function for SMI handler profile.
> -
> -  @param  ImageHandle   The firmware allocated handle for the EFI image.
> -  @param  SystemTable   A pointer to the EFI System Table.
> +  The common constructor function for SMI handler profile.
> 
>@retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
>  **/
>  EFI_STATUS
> -EFIAPI
> -SmmSmiHandlerProfileLibConstructor (
> -  IN EFI_HANDLEImageHandle,
> -  IN EFI_SYSTEM_TABLE  *SystemTable
> +MmSmiHandlerProfileLibInitialization (
> +  VOID
>)
>  {
> -  gSmst->SmmLocateProtocol (
> +  gMmst->MmLocateProtocol (
> ,
> NULL,
> (VOID **)  diff --git
> a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfile
> Lib.c
> b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfil
> eLib.c
> index b76e8f0dc132..0167d81b880d 100644
> ---
> a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfile
> Lib.c
> +++
> b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfil
> eL
> +++ ib.c
> @@ -2,87 +2,17 @@
>SMM driver instance of SmiHandlerProfile Library.
> 
>Copyright (c) 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) Microsoft Corporation.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> -#include 
> -#include  -#include
>  -#include 
> +#include 
> 
> -SMI_HANDLER_PROFILE_PROTOCOL  *mSmiHandlerProfile;
> +#include "MmSmiHandlerProfileLib.h"
> 
>  /**
> -  This function is called by SmmChildDispatcher module to report
> -  a new SMI handler is registered, to SmmCore.
> -
> -  @param HandlerGuid The GUID to identify the type of the handler.
> - For the SmmChildDispatch protocol, the HandlerGuid
> - must be the GUID of SmmChildDispatch protocol.
> -  @param Handler The SMI handler.
> -  @param CallerAddress   The address of the module who registers the SMI
> handler.
> -  @param Context The context of the SMI handler.
> - For the SmmChildDispatch protocol, the Context
> - must match the one defined for 

Re: [edk2-devel] Questions/doubts about UFS device on UEFI

2021-01-26 Thread Andrew Fish via groups.io

> On Jan 26, 2021, at 9:23 AM, renan.moraes2 via groups.io 
>  wrote:
> 
> Good afternoon,
>
> We are studying how to communicate with Universal Flash Storage (UFS) device 
> on UEFI environment. The first step is identify the UFS device, we are using 
> the system Lenovo Flex 5G (It has Snapdragon processor + UFS device).
>
> We tried to identify the UFS device in three ways:
>
> 1 - Specific UEFI protocols (UFS_DEVICE_CONFIG_PROTOCOL / 
> UFS_HOST_CONTROLLER_PROTOCOL) -> None of these protocols could be found on 
> our test systems, we compiled the EDK II 2020 (edk2-stable202005) available 
> drivers (UfsPassThru and UfsPciHc) and loaded them. However, these drivers 
> can't detect the UFS device.
>
> 2 - Device Path -> Tried to identify by its type (Type 3 – Messaging Device 
> Path) and subtype (Sub-Type 25 – UFS), but could not find any matches.
>
> 3 - Pci bus -> Tried to identify by its class (01h - Mass Storage Controller) 
> and subclass (09h - Universal Flash Storage controller), but could not find 
> any matches.
>
> We would like to know if there is an alternative UEFI UFS driver or other 
> methods to identify and communicate with this type of storage device.
>

Some times people chose to abstract devices more generically so for example 
just produce EFI_BLOCK_IO_PROTOCOL for a generic disk like device. 

Some times people chose to not have EFI drivers for devices they don’t support 
booting from, it is firmware after all. 

Sometimes on ARM devices the SoC just have MMIO devices at magic memory 
addresses. So your best bet might be figuring out how the hardware works on 
your device from a data sheet. 

Thanks,

Andrew Fish

> Thank you in advance.
> 



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




[edk2-devel] Spell check corrections, package by package

2021-01-26 Thread brbarkel via groups.io
When we added the SpellCheck to CI, we intended to move through package by 
package and turn on the PR gate checks (right now they're all in "audit only"). 
To do this, we must first fix all the existing spelling errors and/or update 
the ignore lists and dictionaries with expected terms.

I would like to start this process so that we can get enforcement on in the 
next couple of months. To pick a random package, I was going to start with 
NetworkPkg. Before starting, I wanted to ask what the community would like to 
see in terms of the number of patches and/or associated bugs. I'm comfortable 
with whatever because I will likely automate the bug opening and patch 
association (so beware if you want one bug per fix... there will be a LOT of 
bugs).

Thoughts?


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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Ankur Arora

On 2021-01-26 1:32 p.m., Laszlo Ersek wrote:

On 01/26/21 22:17, Ankur Arora wrote:

On 2021-01-26 1:07 p.m., Laszlo Ersek wrote:

On 01/26/21 20:15, Ankur Arora wrote:

On 2021-01-26 11:01 a.m., Laszlo Ersek wrote:



I'll continue the review later this week.


Acking the comments above.


Thank you!


Meanwhile let me reprocess the series in light of the comments above.


I'm at such a point now, during the v5 review, that I think I can easily
re-sync.

In general, I don't mind the posting of a new version of a series
mid-review, *IF* we agree about it in advance.

If you prefer to post a v6, for addressing the comments I've made thus
far, I'm OK with that. If you'd like me to continue reviewing v5, I'm
also OK with that.

So it's up to you -- please state your decision, so that I know if I
should proceed with v5 (later this week), or wait for v6.


I think I would prefer to send v6. Looking at the v5 comments so far, I'm
sure that there's a lot of non conforming coding style issues.
Addressing them now (or at least a hopefully significant subset) would
probably save time.


I agree; thank you.

(And, for all the yelling that ECC does, I'm really surprised it didn't
catch the "missing space between function designator and opening paren"
wart!)


Heh. Yeah, I was surprised at how little fault ECC found in the code.

Thanks for reviewing.
Ankur




I'm looking at sending these out by Thursday morning PT, and given that
you plan to continue later this week, sounds like it might not lose too
much review time either.


Yes, that should work fine.

Thank you, Ankur!
Laszlo




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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Laszlo Ersek
On 01/26/21 22:17, Ankur Arora wrote:
> On 2021-01-26 1:07 p.m., Laszlo Ersek wrote:
>> On 01/26/21 20:15, Ankur Arora wrote:
>>> On 2021-01-26 11:01 a.m., Laszlo Ersek wrote:
>>
 I'll continue the review later this week.
>>>
>>> Acking the comments above.
>>
>> Thank you!
>>
>>> Meanwhile let me reprocess the series in light of the comments above.
>>
>> I'm at such a point now, during the v5 review, that I think I can easily
>> re-sync.
>>
>> In general, I don't mind the posting of a new version of a series
>> mid-review, *IF* we agree about it in advance.
>>
>> If you prefer to post a v6, for addressing the comments I've made thus
>> far, I'm OK with that. If you'd like me to continue reviewing v5, I'm
>> also OK with that.
>>
>> So it's up to you -- please state your decision, so that I know if I
>> should proceed with v5 (later this week), or wait for v6.
> 
> I think I would prefer to send v6. Looking at the v5 comments so far, I'm
> sure that there's a lot of non conforming coding style issues.
> Addressing them now (or at least a hopefully significant subset) would
> probably save time.

I agree; thank you.

(And, for all the yelling that ECC does, I'm really surprised it didn't
catch the "missing space between function designator and opening paren"
wart!)

> I'm looking at sending these out by Thursday morning PT, and given that
> you plan to continue later this week, sounds like it might not lose too
> much review time either.

Yes, that should work fine.

Thank you, Ankur!
Laszlo



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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Ankur Arora

On 2021-01-26 1:07 p.m., Laszlo Ersek wrote:

On 01/26/21 20:15, Ankur Arora wrote:

On 2021-01-26 11:01 a.m., Laszlo Ersek wrote:



I'll continue the review later this week.


Acking the comments above.


Thank you!


Meanwhile let me reprocess the series in light of the comments above.


I'm at such a point now, during the v5 review, that I think I can easily
re-sync.

In general, I don't mind the posting of a new version of a series
mid-review, *IF* we agree about it in advance.

If you prefer to post a v6, for addressing the comments I've made thus
far, I'm OK with that. If you'd like me to continue reviewing v5, I'm
also OK with that.

So it's up to you -- please state your decision, so that I know if I
should proceed with v5 (later this week), or wait for v6.


I think I would prefer to send v6. Looking at the v5 comments so far, I'm
sure that there's a lot of non conforming coding style issues.
Addressing them now (or at least a hopefully significant subset) would
probably save time.

I'm looking at sending these out by Thursday morning PT, and given that
you plan to continue later this week, sounds like it might not lose too
much review time either.

Thanks
Ankur



Thanks!
Laszlo




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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Laszlo Ersek
On 01/26/21 20:15, Ankur Arora wrote:
> On 2021-01-26 11:01 a.m., Laszlo Ersek wrote:

>> I'll continue the review later this week.
> 
> Acking the comments above.

Thank you!

> Meanwhile let me reprocess the series in light of the comments above.

I'm at such a point now, during the v5 review, that I think I can easily
re-sync.

In general, I don't mind the posting of a new version of a series
mid-review, *IF* we agree about it in advance.

If you prefer to post a v6, for addressing the comments I've made thus
far, I'm OK with that. If you'd like me to continue reviewing v5, I'm
also OK with that.

So it's up to you -- please state your decision, so that I know if I
should proceed with v5 (later this week), or wait for v6.

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70811): https://edk2.groups.io/g/devel/message/70811
Mute This Topic: https://groups.io/mt/80125308/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 00/20] Extends Support of MM_STANDALONE Type Modules to X64

2021-01-26 Thread Laszlo Ersek
On 01/26/21 20:44, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3129
> 
> This patch series is a follow up of previous submission:
> https://edk2.groups.io/g/devel/message/70329
> 
> These module changes are validated on two different physical platforms.
> Standalone and traditional MM are both tested to be functional on these
> systems.
> 
> v4 patches mainly focus on feedback for reviewed commits in v3 patches,
> including:
> a. Adding "Reviewed-by" tags for applicable patches;
> b. Breaking CpuIo2Smm patch for file renaming and abstraction purpose;
> c. Adding SmmSmiHandlerProfileLib coverage;
> 
> Patch v4 branch: https://github.com/kuqin12/edk2/tree/standalone_x64_v4

>   UefiCpuPkg: CpuIo2Smm: Abstract SMM specific functions into separate
> file
>   UefiCpuPkg: CpuIo2Smm: Support of CpuIo driver under StandaloneMm

On these patches (v4 #18, v4 #19), I defer to other UefiCpuPkg reviewers.

Thanks
Laszlo



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




[edk2-devel] Questions/doubts about UFS device on UEFI

2021-01-26 Thread renan.moraes2 via groups.io
Good afternoon,

We are studying how to communicate with Universal Flash Storage (UFS) device on 
UEFI environment. The first step is identify the UFS device, we are using the 
system Lenovo Flex 5G (It has Snapdragon processor + UFS device).

We tried to identify the UFS device in three ways:

1 - Specific UEFI protocols (UFS_DEVICE_CONFIG_PROTOCOL / 
UFS_HOST_CONTROLLER_PROTOCOL) -> None of these protocols could be found on our 
test systems, we compiled the EDK II 2020 (edk2-stable202005) available drivers 
(UfsPassThru and UfsPciHc) and loaded them. However, these drivers can't detect 
the UFS device.

2 - Device Path -> Tried to identify by its type (Type 3 – Messaging Device 
Path) and subtype (Sub-Type 25 – UFS), but could not find any matches.

3 - Pci bus -> Tried to identify by its class (01h - Mass Storage Controller) 
and subclass (09h - Universal Flash Storage controller), but could not find any 
matches.

We would like to know if there is an alternative UEFI UFS driver or other 
methods to identify and communicate with this type of storage device.

Thank you in advance.


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




Re: [edk2-devel] [PATCH v5 0/9] support CPU hot-unplug

2021-01-26 Thread Ankur Arora

On 2021-01-26 10:03 a.m., Laszlo Ersek wrote:

Hi Ankur,

On 01/26/21 07:44, Ankur Arora wrote:

Hi,

This series adds support for CPU hot-unplug with OVMF.

Please see this in conjunction with the QEMU secureboot hot-unplug v2
series posted here (now upstreamed):
   
https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imamm...@redhat.com/

Patches 1 and 3,
   ("OvmfPkg/CpuHotplugSmm: refactor hotplug logic")
   ("OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper")
are either refactors or add support functions.

   OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
   OvmfPkg/CpuHotplugSmm: add CpuEject()
   OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection

Patch 2 and 9,
   ("OvmfPkg/CpuHotplugSmm: collect hot-unplug events")
   ("OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug")
handle the QEMU protocol logic for collection of CPU hot-unplug events
or the protocol negotiation.

Patch 4,
   ("OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()")
adds the MMI logic for CPU hot-unplug handling and informing
the PiSmmCpuDxeSmm of CPU removal.

Patches 5 and 6,
   ("OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA")
   ("OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state")
sets up state for doing the CPU ejection as part of hot-unplug.

Patches 7, and 8,
   ("OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()")
   ("OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection")
add the CPU ejection logic.

Testing (with QEMU 5.2.50):
  - Stable with randomized CPU plug/unplug (guest maxcpus=1,8,128)
  - Synthetic tests with simultaneous multi CPU hot-unplug
  - Negotiation with/without CPU hotplug enabled

Also at:
   github.com/terminus/edk2/ hot-unplug-v5

Changelog:
v5:
   - fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
 add Qemu Cpu Status helper").

v4:
   - Gets rid of unnecessary UefiCpuPkg changes
   URL: 
https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.ar...@oracle.com/

v3:
   - Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
 and OvmfPkg/CpuHotplugSmm
   - Cleaner split of the hot-unplug code
   URL: 
https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.ar...@oracle.com/

v2:
   - Do the ejection via SmmCpuFeaturesRendezvousExit()
   URL: 
https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.ar...@oracle.com/

RFC:
   URL: 
https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.ar...@oracle.com/


Please review.

Thanks
Ankur

Ankur Arora (9):
   OvmfPkg/CpuHotplugSmm: refactor hotplug logic
   OvmfPkg/CpuHotplugSmm: collect hot-unplug events
   OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
   OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
   OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
   OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
   OvmfPkg/CpuHotplugSmm: add CpuEject()
   OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection
   OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug

  OvmfPkg/OvmfPkg.dec|  10 +
  OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf|   1 +
  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   3 +
  OvmfPkg/CpuHotplugSmm/QemuCpuhp.h  |   6 +
  OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h  |   2 +
  OvmfPkg/Include/Library/CpuHotEjectData.h  |  32 ++
  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 450 -
  OvmfPkg/CpuHotplugSmm/QemuCpuhp.c  |  57 ++-
  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  68 
  OvmfPkg/SmmControl2Dxe/SmiFeatures.c   |  25 +-
  10 files changed, 552 insertions(+), 102 deletions(-)
  create mode 100644 OvmfPkg/Include/Library/CpuHotEjectData.h



This series is still mal-formatted; for some reason it has one too many
CRs per line.

Did you try to set up the "base64" or the "8bit"
content-transfer-encoding? The emails reflected through the list are
still "quoted-printable".


Confirming that this was the problem. And, thanks for debugging this.

Ankur



Anyway, I managed to apply the patches with some trickery on a local branch.

Thanks
Laszlo




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




[edk2-devel] [PATCH v4 20/20] UefiCpuPkg: SmmCpuExceptionHandlerLib: Added StandaloneMm module support

2021-01-26 Thread Kun Qin
This change of SmmCpuExceptionHandlerLib adds support for StandaloneMm
components to allow x64 StandaloneMm environment setting up exception
handlers.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Ray Ni 
---

Notes:
v4:
- Added reviewed-by tag [Ray]

v3:
- Added reviewed-by tag [Laszlo]

v2:
- No review, no change.

 UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
index 4cdb11c04ea0..ea5b10b5c8e4 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
@@ -13,7 +13,7 @@ [Defines]
   FILE_GUID  = 8D2C439B-3981-42ff-9CE5-1B50ECA502D6
   MODULE_TYPE= DXE_SMM_DRIVER
   VERSION_STRING = 1.1
-  LIBRARY_CLASS  = CpuExceptionHandlerLib|DXE_SMM_DRIVER
+  LIBRARY_CLASS  = CpuExceptionHandlerLib|DXE_SMM_DRIVER 
MM_STANDALONE MM_CORE_STANDALONE
 
 #
 # The following information is for reference only and not required by the 
build tools.
-- 
2.30.0.windows.1



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




[edk2-devel] [PATCH v4 19/20] UefiCpuPkg: CpuIo2Smm: Support of CpuIo driver under StandaloneMm

2021-01-26 Thread Kun Qin
This change adds a new CpuIo driver instance for MM_STANDALONE type. The
new driver entrypoint is implemented in a separate file to match the
interface definition of MM_STANDALONE modules.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
---

Notes:
v4:
- Break SMM instance abstraction and file renaming into a separate
patch [Ray]

v3:
- Revert file name change of "CpuIo2Smm" for review and git history
concern [Laszlo]
- Break driver entrypoint into separate patch [Laszlo]

v2:
- Removed "EFIAPI" for internal functions.

 UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.c   | 32 ++
 UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.inf | 45 
 UefiCpuPkg/UefiCpuPkg.dsc   |  5 +++
 3 files changed, 82 insertions(+)

diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.c 
b/UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.c
new file mode 100644
index ..9cff4b7166db
--- /dev/null
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.c
@@ -0,0 +1,32 @@
+/** @file
+  Produces the SMM CPU I/O Protocol.
+
+Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+#include "CpuIo2Mm.h"
+
+/**
+  The module Entry Point for Standalone MM CpuIoProtocol driver
+
+  @param[in] ImageHandle  The firmware allocated handle for the EFI image.
+  @param[in] SystemTable  A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS  The entry point is executed successfully.
+  @retval OtherSome error occurs when executing this entry point.
+
+**/
+EFI_STATUS
+EFIAPI
+StandaloneMmCpuIo2Initialize (
+  IN EFI_HANDLE   ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE  *SystemTable
+  )
+{
+  return CommonCpuIo2Initialize ();
+}
diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.inf 
b/UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.inf
new file mode 100644
index ..ec37a9d9198a
--- /dev/null
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.inf
@@ -0,0 +1,45 @@
+## @file
+#  Produces the SMM CPU I/O 2 Protocol by using the services of the I/O 
Library.
+#
+#  Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+#  Copyright (c) Microsoft Corporation.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = CpuIo2StandaloneMm
+  FILE_GUID  = E3121A26-BB1C-4A18-8E23-2EA3F0412248
+  MODULE_TYPE= MM_STANDALONE
+  VERSION_STRING = 1.0
+  PI_SPECIFICATION_VERSION   = 0x00010032
+  ENTRY_POINT= StandaloneMmCpuIo2Initialize
+
+#
+# The following information is for reference only and not required by the 
build tools.
+#
+#  VALID_ARCHITECTURES   = IA32 X64
+#
+
+[Sources]
+  CpuIo2StandaloneMm.c
+  CpuIo2Mm.c
+  CpuIo2Mm.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  StandaloneMmDriverEntryPoint
+  BaseLib
+  DebugLib
+  IoLib
+  MmServicesTableLib
+  BaseMemoryLib
+
+[Protocols]
+  gEfiSmmCpuIo2ProtocolGuid   ## PRODUCES
+
+[Depex]
+  TRUE
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index c3c27afff88e..9128cef076dd 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -38,6 +38,7 @@ [LibraryClasses]
   
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
   
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
   
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
+  
StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
   
DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
   PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
   PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
@@ -96,6 +97,9 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
 
+[LibraryClasses.common.MM_STANDALONE]
+  
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
+
 [LibraryClasses.common.UEFI_APPLICATION]
   
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
   
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
@@ -126,6 +130,7 @@ [Components.IA32, Components.X64]
   NULL|UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf
   }
   UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
+  UefiCpuPkg/CpuIo2Smm/CpuIo2StandaloneMm.inf
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
   UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
   UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
-- 
2.30.0.windows.1




[edk2-devel] [PATCH v4 12/20] MdePkg: UefiDevicePathLib: Support UefiDevicePathLib under StandaloneMm

2021-01-26 Thread Kun Qin
This change added an instance of UefiDevicePathLib for StandaloneMm. It
abstracts DevicePathFromHandle function into different files for
Standalone MM and other instances to avoid linking gBS into MM_STANDALONE
drivers.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 

Signed-off-by: Kun Qin 
Reviewed-by: Liming Gao 
---

Notes:
v4:
- Reviewed previously. No change.

v3:
- Added reviewed-by tag [Liming]

v2:
- No review, no change.

 MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c 
   | 33 -
 MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesDxeSmm.c   
   | 51 
 MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c 
   | 40 +++
 MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf 
   |  1 +
 
MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibOptionalDevicePathProtocol.inf
  |  1 +
 MdePkg/Library/UefiDevicePathLib/{UefiDevicePathLib.inf => 
UefiDevicePathLibStandaloneMm.inf} | 11 +++--
 MdePkg/MdePkg.dsc  
   |  1 +
 7 files changed, 100 insertions(+), 38 deletions(-)

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c 
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
index 9274ef8dda98..7d5fb18d2516 100644
--- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
@@ -806,39 +806,6 @@ UefiDevicePathLibIsDevicePathMultiInstance (
 }
 
 
-/**
-  Retrieves the device path protocol from a handle.
-
-  This function returns the device path protocol from the handle specified by 
Handle.
-  If Handle is NULL or Handle does not contain a device path protocol, then 
NULL
-  is returned.
-
-  @param  Handle The handle from which to retrieve the 
device
- path protocol.
-
-  @return The device path protocol from the handle specified by Handle.
-
-**/
-EFI_DEVICE_PATH_PROTOCOL *
-EFIAPI
-DevicePathFromHandle (
-  IN EFI_HANDLE  Handle
-  )
-{
-  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
-  EFI_STATUSStatus;
-
-  Status = gBS->HandleProtocol (
-  Handle,
-  ,
-  (VOID *) 
-  );
-  if (EFI_ERROR (Status)) {
-DevicePath = NULL;
-  }
-  return DevicePath;
-}
-
 /**
   Allocates a device path for a file and appends it to an existing device path.
 
diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesDxeSmm.c 
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesDxeSmm.c
new file mode 100644
index ..7f3b6076ef34
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesDxeSmm.c
@@ -0,0 +1,51 @@
+/** @file
+  Device Path services. The thing to remember is device paths are built out of
+  nodes. The device path is terminated by an end node that is length
+  sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is 
sizeof(EFI_DEVICE_PATH_PROTOCOL)
+  all over this file.
+
+  The only place where multi-instance device paths are supported is in
+  environment varibles. Multi-instance device paths should never be placed
+  on a Handle.
+
+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UefiDevicePathLib.h"
+
+
+/**
+  Retrieves the device path protocol from a handle.
+
+  This function returns the device path protocol from the handle specified by 
Handle.
+  If Handle is NULL or Handle does not contain a device path protocol, then 
NULL
+  is returned.
+
+  @param  Handle The handle from which to retrieve the 
device
+ path protocol.
+
+  @return The device path protocol from the handle specified by Handle.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+DevicePathFromHandle (
+  IN EFI_HANDLE  Handle
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+  EFI_STATUSStatus;
+
+  Status = gBS->HandleProtocol (
+  Handle,
+  ,
+  (VOID *) 
+  );
+  if (EFI_ERROR (Status)) {
+DevicePath = NULL;
+  }
+  return DevicePath;
+}
diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c 
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
new file mode 100644
index ..930e778d373a
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
@@ -0,0 +1,40 @@
+/** @file
+  Device Path services. The thing to remember is device paths are built out of
+  nodes. The device path is terminated by an end node that is length
+  sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is 

[edk2-devel] [PATCH v4 16/20] SecurityPkg: Tpm2DeviceLibDTpm: Introduce StandaloneMm instance

2021-01-26 Thread Kun Qin
This change added a new instance of Tpm2DeviceLibDTpm to support drivers
of type MM_STANDALONE. It abstracts dynamic Pcd access into separate file
for different instances to avoid dynamic usage for StandaloneMm modules.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Previously reviewed. No change.

v2:
- Added Reviewed-by tag [Jiewen]
- Removed "EFIAPI" for internal functions

 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c  
| 42 +---
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpmBase.c  
| 68 
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpmStandaloneMm.c  
| 66 +++
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.c
| 40 +---
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
| 15 +++--
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.h  
| 67 +++
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
|  3 +
 SecurityPkg/Library/Tpm2DeviceLibDTpm/{Tpm2DeviceLibDTpm.inf => 
Tpm2DeviceLibDTpmStandaloneMm.inf} | 13 ++--
 SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf  
|  3 +
 SecurityPkg/SecurityPkg.dsc
|  1 +
 10 files changed, 228 insertions(+), 90 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c 
b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
index 42e1ecbce95a..238389dbdb1b 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.c
@@ -13,29 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 
-/**
-  Return PTP interface type.
-
-  @param[in] RegisterPointer to PTP register.
-
-  @return PTP interface type.
-**/
-TPM2_PTP_INTERFACE_TYPE
-Tpm2GetPtpInterface (
-  IN VOID *Register
-  );
-
-/**
-  Return PTP CRB interface IdleByPass state.
-
-  @param[in] RegisterPointer to PTP register.
-
-  @return PTP CRB interface IdleByPass state.
-**/
-UINT8
-Tpm2GetIdleByPass (
-  IN VOID *Register
-  );
+#include "Tpm2DeviceLibDTpm.h"
 
 /**
   This service enables the sending of commands to the TPM2.
@@ -145,21 +123,5 @@ Tpm2DeviceLibConstructor (
   VOID
   )
 {
-  TPM2_PTP_INTERFACE_TYPE  PtpInterface;
-  UINT8IdleByPass;
-
-  //
-  // Cache current active TpmInterfaceType only when needed
-  //
-  if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
-PtpInterface = Tpm2GetPtpInterface ((VOID *) (UINTN) PcdGet64 
(PcdTpmBaseAddress));
-PcdSet8S(PcdActiveTpmInterfaceType, PtpInterface);
-  }
-
-  if (PcdGet8(PcdActiveTpmInterfaceType) == Tpm2PtpInterfaceCrb && 
PcdGet8(PcdCRBIdleByPass) == 0xFF) {
-IdleByPass = Tpm2GetIdleByPass((VOID *) (UINTN) PcdGet64 
(PcdTpmBaseAddress));
-PcdSet8S(PcdCRBIdleByPass, IdleByPass);
-  }
-
-  return EFI_SUCCESS;
+  return InternalTpm2DeviceLibDTpmCommonConstructor ();
 }
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpmBase.c 
b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpmBase.c
new file mode 100644
index ..bc35e257e105
--- /dev/null
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpmBase.c
@@ -0,0 +1,68 @@
+/** @file
+  This file abstract internal interfaces of which implementation differs per 
library instance.
+
+Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. 
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+
+#include "Tpm2DeviceLibDTpm.h"
+
+/**
+  Return cached PTP CRB interface IdleByPass state.
+
+  @return Cached PTP CRB interface IdleByPass state.
+**/
+UINT8
+GetCachedIdleByPass (
+  VOID
+  )
+{
+  return PcdGet8(PcdCRBIdleByPass);
+}
+
+/**
+  Return cached PTP interface type.
+
+  @return Cached PTP interface type.
+**/
+TPM2_PTP_INTERFACE_TYPE
+GetCachedPtpInterface (
+  VOID
+  )
+{
+  return PcdGet8(PcdActiveTpmInterfaceType);
+}
+
+/**
+  The common function cache current active TpmInterfaceType when needed.
+
+  @retval EFI_SUCCESS   DTPM2.0 instance is registered, or system does not 
support register DTPM2.0 instance
+**/
+EFI_STATUS
+InternalTpm2DeviceLibDTpmCommonConstructor (
+  VOID
+  )
+{
+  TPM2_PTP_INTERFACE_TYPE  PtpInterface;
+  UINT8IdleByPass;
+
+  //
+  // Cache current active TpmInterfaceType only when needed
+  //
+  if (PcdGet8(PcdActiveTpmInterfaceType) == 0xFF) {
+PtpInterface = Tpm2GetPtpInterface ((VOID *) 

[edk2-devel] [PATCH v4 10/20] MdeModulePkg: ReportStatusCodeRouter: Support StandaloneMm RSC Router

2021-01-26 Thread Kun Qin
This change added support of RSC router under StandaloneMm. It replaces
SMM version ReportStatusCode protocol definitions with MM version. This
patch also switched to use gMmst instead of gSmst. Lastly, it abstracts
standalone and traditional MM driver entrypoints into separate files to
allow maximal common implementations.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Dandan Bi 
Cc: Liming Gao 

Signed-off-by: Kun Qin 
Reviewed-by: Hao A Wu 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Added reviewed-by tag [Hao]
- Updated function descriptions to match function interface [Hao]

v2:
- Removed "EFIAPI" for internally abstracted functions [Hao]
- Updated function descriptions [Hao]
- Updated ReportDispatcher to make *.h and *.c consistent [Hao]

 MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/{ReportStatusCodeRouterSmm.c 
=> ReportStatusCodeRouterCommon.c} | 59 +---
 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterStandaloneMm.c
| 33 +++
 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterTraditional.c
 | 33 +++
 MdeModulePkg/MdeModulePkg.dsc  
   |  1 +
 MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/{ReportStatusCodeRouterSmm.h 
=> ReportStatusCodeRouterCommon.h} | 46 +--
 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterSmm.inf 
  | 13 +++--
 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterStandaloneMm.inf
  | 49 
 7 files changed, 179 insertions(+), 55 deletions(-)

diff --git 
a/MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterSmm.c 
b/MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterCommon.c
similarity index 74%
rename from 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterSmm.c
rename to 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterCommon.c
index c3ab5cd05045..c4843a745d69 100644
--- 
a/MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterSmm.c
+++ 
b/MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterCommon.c
@@ -7,7 +7,7 @@
 
 **/
 
-#include "ReportStatusCodeRouterSmm.h"
+#include "ReportStatusCodeRouterCommon.h"
 
 LIST_ENTRY   mCallbackListHead  = INITIALIZE_LIST_HEAD_VARIABLE 
(mCallbackListHead);
 
@@ -17,11 +17,11 @@ LIST_ENTRY   mCallbackListHead  = 
INITIALIZE_LIST_HEAD_VARIABLE (mCallba
 //
 UINT32   mStatusCodeNestStatus = 0;
 
-EFI_SMM_STATUS_CODE_PROTOCOL  mSmmStatusCodeProtocol  = {
+EFI_MM_STATUS_CODE_PROTOCOL   mSmmStatusCodeProtocol  = {
   ReportDispatcher
 };
 
-EFI_SMM_RSC_HANDLER_PROTOCOL  mSmmRscHandlerProtocol = {
+EFI_MM_RSC_HANDLER_PROTOCOL   mSmmRscHandlerProtocol = {
   Register,
   Unregister
   };
@@ -45,18 +45,18 @@ EFI_SMM_RSC_HANDLER_PROTOCOL  mSmmRscHandlerProtocol = {
 EFI_STATUS
 EFIAPI
 Register (
-  IN EFI_SMM_RSC_HANDLER_CALLBACK   Callback
+  IN EFI_MM_RSC_HANDLER_CALLBACKCallback
   )
 {
   LIST_ENTRY  *Link;
-  SMM_RSC_HANDLER_CALLBACK_ENTRY  *CallbackEntry;
+  MM_RSC_HANDLER_CALLBACK_ENTRY  *CallbackEntry;
 
   if (Callback == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
   for (Link = GetFirstNode (); !IsNull (, 
Link); Link = GetNextNode (, Link)) {
-CallbackEntry = CR (Link, SMM_RSC_HANDLER_CALLBACK_ENTRY, Node, 
SMM_RSC_HANDLER_CALLBACK_ENTRY_SIGNATURE);
+CallbackEntry = CR (Link, MM_RSC_HANDLER_CALLBACK_ENTRY, Node, 
MM_RSC_HANDLER_CALLBACK_ENTRY_SIGNATURE);
 if (CallbackEntry->RscHandlerCallback == Callback) {
   //
   // If the function was already registered. It can't be registered again.
@@ -65,10 +65,10 @@ Register (
 }
   }
 
-  CallbackEntry = (SMM_RSC_HANDLER_CALLBACK_ENTRY *)AllocatePool (sizeof 
(SMM_RSC_HANDLER_CALLBACK_ENTRY));
+  CallbackEntry = (MM_RSC_HANDLER_CALLBACK_ENTRY *)AllocatePool (sizeof 
(MM_RSC_HANDLER_CALLBACK_ENTRY));
   ASSERT (CallbackEntry != NULL);
 
-  CallbackEntry->Signature  = SMM_RSC_HANDLER_CALLBACK_ENTRY_SIGNATURE;
+  CallbackEntry->Signature  = MM_RSC_HANDLER_CALLBACK_ENTRY_SIGNATURE;
   CallbackEntry->RscHandlerCallback = Callback;
 
   InsertTailList (, >Node);
@@ -92,18 +92,18 @@ Register (
 EFI_STATUS
 EFIAPI
 Unregister (
-  IN EFI_SMM_RSC_HANDLER_CALLBACK Callback
+  IN EFI_MM_RSC_HANDLER_CALLBACK  Callback
   )
 {
   LIST_ENTRY*Link;
-  SMM_RSC_HANDLER_CALLBACK_ENTRY*CallbackEntry;
+  MM_RSC_HANDLER_CALLBACK_ENTRY*CallbackEntry;
 
   if (Callback == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
   for (Link = GetFirstNode (); !IsNull (, 
Link); Link = GetNextNode (, Link)) {
-CallbackEntry = CR (Link, SMM_RSC_HANDLER_CALLBACK_ENTRY, Node, 

[edk2-devel] [PATCH v4 18/20] UefiCpuPkg: CpuIo2Smm: Abstract SMM specific functions into separate file

2021-01-26 Thread Kun Qin
This change abstracts CpuIo2Smm driver entrypoint into separate file and
moves functions/definitions that are not substantially specific to
Traditional MM (SMM) into CpuIo2Mm.* in order to set ways for Standalone
MM support in the future.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
---

Notes:
v4:
- Newly created patch to rename files for existed SMM driver [Ray]

 UefiCpuPkg/CpuIo2Smm/{CpuIo2Smm.c => CpuIo2Mm.c} |  11 +-
 UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c | 385 +---
 UefiCpuPkg/CpuIo2Smm/{CpuIo2Smm.h => CpuIo2Mm.h} |  12 +
 UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf   |   3 +-
 4 files changed, 22 insertions(+), 389 deletions(-)

diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c b/UefiCpuPkg/CpuIo2Smm/CpuIo2Mm.c
similarity index 95%
copy from UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
copy to UefiCpuPkg/CpuIo2Smm/CpuIo2Mm.c
index c0a2baecee03..7e314eaa1558 100644
--- a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2Mm.c
@@ -6,7 +6,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include "CpuIo2Smm.h"
+#include "CpuIo2Mm.h"
 
 //
 // Handle for the SMM CPU I/O Protocol
@@ -371,18 +371,13 @@ CpuIoServiceWrite (
 /**
   The module Entry Point SmmCpuIoProtocol driver
 
-  @param[in] ImageHandle  The firmware allocated handle for the EFI image.
-  @param[in] SystemTable  A pointer to the EFI System Table.
-
   @retval EFI_SUCCESS  The entry point is executed successfully.
   @retval OtherSome error occurs when executing this entry point.
 
 **/
 EFI_STATUS
-EFIAPI
-SmmCpuIo2Initialize (
-  IN EFI_HANDLEImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+CommonCpuIo2Initialize (
+  VOID
   )
 {
   EFI_STATUS  Status;
diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
index c0a2baecee03..1acce9f3d462 100644
--- a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
@@ -2,374 +2,17 @@
   Produces the SMM CPU I/O Protocol.
 
 Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include "CpuIo2Smm.h"
+#include 
 
-//
-// Handle for the SMM CPU I/O Protocol
-//
-EFI_HANDLE  mHandle = NULL;
-
-//
-// SMM CPU I/O Protocol instance
-//
-EFI_SMM_CPU_IO2_PROTOCOL mSmmCpuIo2 = {
-  {
-CpuMemoryServiceRead,
-CpuMemoryServiceWrite
-  },
-  {
-CpuIoServiceRead,
-CpuIoServiceWrite
-  }
-};
-
-//
-// Lookup table for increment values based on transfer widths
-//
-UINT8 mStride[] = {
-  1, // SMM_IO_UINT8
-  2, // SMM_IO_UINT16
-  4, // SMM_IO_UINT32
-  8  // SMM_IO_UINT64
-};
-
-/**
-  Check parameters to a SMM CPU I/O Protocol service request.
-
-  @param[in]  MmioOperation  TRUE for an MMIO operation, FALSE for I/O Port 
operation.
-  @param[in]  Width  Signifies the width of the I/O operations.
-  @param[in]  AddressThe base address of the I/O operations.  The 
caller is
- responsible for aligning the Address if required.
-  @param[in]  Count  The number of I/O operations to perform.
-  @param[in]  Buffer For read operations, the destination buffer to 
store
- the results.  For write operations, the source 
buffer
- from which to write data.
-
-  @retval EFI_SUCCESSThe data was read from or written to the 
device.
-  @retval EFI_UNSUPPORTEDThe Address is not valid for this system.
-  @retval EFI_INVALID_PARAMETER  Width or Count, or both, were invalid.
-
-**/
-EFI_STATUS
-CpuIoCheckParameter (
-  IN BOOLEAN   MmioOperation,
-  IN EFI_SMM_IO_WIDTH  Width,
-  IN UINT64Address,
-  IN UINTN Count,
-  IN VOID  *Buffer
-  )
-{
-  UINT64  MaxCount;
-  UINT64  Limit;
-
-  //
-  // Check to see if Buffer is NULL
-  //
-  if (Buffer == NULL) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // Check to see if Width is in the valid range
-  //
-  if ((UINT32)Width > SMM_IO_UINT64) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // Check to see if Width is in the valid range for I/O Port operations
-  //
-  if (!MmioOperation && (Width == SMM_IO_UINT64)) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // Check to see if any address associated with this transfer exceeds the 
maximum
-  // allowed address.  The maximum address implied by the parameters passed in 
is
-  // Address + Size * Count.  If the following condition is met, then the 
transfer
-  // is not supported.
-  //
-  //Address + Size * Count > (MmioOperation ? MAX_ADDRESS : 
MAX_IO_PORT_ADDRESS) + 1
-  //
-  // Since MAX_ADDRESS can be the maximum integer value supported by the CPU 
and Count
-  // can also be the maximum integer value supported by the CPU, this range
-  // check must be adjusted to avoid all overflow conditions.
-  //
-  // The following form of the range check is 

[edk2-devel] [PATCH v4 11/20] MdeModulePkg: SmmSmiHandlerProfileLib: Support StandaloneMm Instance

2021-01-26 Thread Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3185

This change added support of SMI handler profile library router under
StandaloneMm. This change replaces gSmst with gMmst. It also abstracts
standalone and traditional MM driver entrypoints into separate files to
allow maximal common implementations.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Eric Dong 
Cc: Ray Ni 

Signed-off-by: Kun Qin 
---

Notes:
v4:
- Newly created for SmmSmiHandlerProfileLib coverage.

 MdeModulePkg/Library/SmmSmiHandlerProfileLib/{SmmSmiHandlerProfileLib.c => 
MmSmiHandlerProfileLib.c} | 20 ++---
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c 
  | 90 ++--
 
MdeModulePkg/Library/SmmSmiHandlerProfileLib/StandaloneMmSmiHandlerProfileLib.c 
 | 31 +++
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfileLib.h  
  | 23 +
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.inf   
  |  4 +-
 
MdeModulePkg/Library/SmmSmiHandlerProfileLib/StandaloneMmSmiHandlerProfileLib.inf
| 44 ++
 MdeModulePkg/MdeModulePkg.dsc  
  |  1 +
 7 files changed, 117 insertions(+), 96 deletions(-)

diff --git 
a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c 
b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfileLib.c
similarity index 86%
copy from MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
copy to MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfileLib.c
index b76e8f0dc132..f800220b549c 100644
--- a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
+++ b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/MmSmiHandlerProfileLib.c
@@ -1,14 +1,15 @@
 /** @file
-  SMM driver instance of SmiHandlerProfile Library.
+  MM driver instance of SmiHandlerProfile Library.
 
   Copyright (c) 2017, Intel Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include 
+#include 
 #include 
-#include 
+#include 
 #include 
 
 SMI_HANDLER_PROFILE_PROTOCOL  *mSmiHandlerProfile;
@@ -82,21 +83,16 @@ SmiHandlerProfileUnregisterHandler (
 }
 
 /**
-  The constructor function for SMI handler profile.
-
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
+  The common constructor function for SMI handler profile.
 
   @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
 **/
 EFI_STATUS
-EFIAPI
-SmmSmiHandlerProfileLibConstructor (
-  IN EFI_HANDLEImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+MmSmiHandlerProfileLibInitialization (
+  VOID
   )
 {
-  gSmst->SmmLocateProtocol (
+  gMmst->MmLocateProtocol (
,
NULL,
(VOID **) 
diff --git 
a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c 
b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
index b76e8f0dc132..0167d81b880d 100644
--- a/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
+++ b/MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c
@@ -2,87 +2,17 @@
   SMM driver instance of SmiHandlerProfile Library.
 
   Copyright (c) 2017, Intel Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include 
-#include 
-#include 
-#include 
+#include 
 
-SMI_HANDLER_PROFILE_PROTOCOL  *mSmiHandlerProfile;
+#include "MmSmiHandlerProfileLib.h"
 
 /**
-  This function is called by SmmChildDispatcher module to report
-  a new SMI handler is registered, to SmmCore.
-
-  @param HandlerGuid The GUID to identify the type of the handler.
- For the SmmChildDispatch protocol, the HandlerGuid
- must be the GUID of SmmChildDispatch protocol.
-  @param Handler The SMI handler.
-  @param CallerAddress   The address of the module who registers the SMI 
handler.
-  @param Context The context of the SMI handler.
- For the SmmChildDispatch protocol, the Context
- must match the one defined for SmmChildDispatch 
protocol.
-  @param ContextSize The size of the context in bytes.
- For the SmmChildDispatch protocol, the Context
- must match the one defined for SmmChildDispatch 
protocol.
-
-  @retval EFI_SUCCESS   The information is recorded.
-  @retval EFI_UNSUPPORTED   The feature is unsupported.
-  @retval EFI_OUT_OF_RESOURCES  There is no enough resource to record the 
information.
-**/
-EFI_STATUS
-EFIAPI
-SmiHandlerProfileRegisterHandler (
-  IN EFI_GUID   *HandlerGuid,
-  IN EFI_SMM_HANDLER_ENTRY_POINT2   Handler,
-  IN 

[edk2-devel] [PATCH v4 17/20] UefiCpuPkg: CpuIo2Smm: Move CpuIo2Smm driver to consume gMmst

2021-01-26 Thread Kun Qin
This change replaced gSmst with gMmst to support broader compatibility
under MM environment for CpuIo2Smm driver.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Ray Ni 
---

Notes:
v4:
- Added reviewed-by tag [Laszlo]
- Added reviewed-by tag [Ray]

v3:
- Break gMmst replacement into separate PR [Laszlo]

v2:
- Removed "EFIAPI" for internal functions.

 UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c   | 6 +++---
 UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.h   | 2 +-
 UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf | 2 +-
 UefiCpuPkg/UefiCpuPkg.dsc  | 1 +
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
index b840d3e10cae..c0a2baecee03 100644
--- a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.c
@@ -390,12 +390,12 @@ SmmCpuIo2Initialize (
   //
   // Copy the SMM CPU I/O Protocol instance into the System Management System 
Table
   //
-  CopyMem (>SmmIo, , sizeof (mSmmCpuIo2));
+  CopyMem (>MmIo, , sizeof (mSmmCpuIo2));
 
   //
-  // Install the SMM CPU I/O Protocol into the SMM protocol database
+  // Install the SMM CPU I/O Protocol into the MM protocol database
   //
-  Status = gSmst->SmmInstallProtocolInterface (
+  Status = gMmst->MmInstallProtocolInterface (
 ,
 ,
 EFI_NATIVE_INTERFACE,
diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.h b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.h
index 4c133b58c9f4..c80261945f71 100644
--- a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.h
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.h
@@ -16,7 +16,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #define MAX_IO_PORT_ADDRESS   0x
diff --git a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf 
b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
index bc78fa4e42d2..b743a5e0e316 100644
--- a/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
+++ b/UefiCpuPkg/CpuIo2Smm/CpuIo2Smm.inf
@@ -34,7 +34,7 @@ [LibraryClasses]
   BaseLib
   DebugLib
   IoLib
-  SmmServicesTableLib
+  MmServicesTableLib
   BaseMemoryLib
 
 [Protocols]
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 5834eafaa200..c3c27afff88e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -91,6 +91,7 @@ [LibraryClasses.common.DXE_DRIVER]
 
 [LibraryClasses.common.DXE_SMM_DRIVER]
   
SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
+  MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
   
MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
-- 
2.30.0.windows.1



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




[edk2-devel] [PATCH v4 08/20] MdeModulePkg: StatusCodeHandler: StatusCodeHandler driver in StandaloneMm

2021-01-26 Thread Kun Qin
This change added support of StandaloneMm for StatusCodeHandler. It
adds a new instance of StatusCodeHandler of MM_STANDALONE type, and
abstracts the driver entrypoint into separate files, replaced gSmst with
gMmst, and switched to MM version of RscHandlerProtocol.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Dandan Bi 
Cc: Liming Gao 
Cc: Jiewen Yao 

Signed-off-by: Kun Qin 
Reviewed-by: Hao A Wu 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Added reviewed-by tag [Hao]

v2:
- New patch to support StatusCodeHandler in standalone mm [Liming]

 MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c  
  | 36 ++--
 MdeModulePkg/Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.c  
  |  2 +-
 MdeModulePkg/Universal/StatusCodeHandler/Smm/{StatusCodeHandlerSmm.c => 
StatusCodeHandlerMm.c}   | 23 +
 MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerStandalone.c 
  | 31 +
 MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerTraditional.c
  | 31 +
 MdeModulePkg/MdeModulePkg.dsc  
  |  1 +
 MdeModulePkg/Universal/StatusCodeHandler/Smm/{StatusCodeHandlerSmm.h => 
StatusCodeHandlerMm.h}   | 23 ++---
 MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf  
  | 15 
 MdeModulePkg/Universal/StatusCodeHandler/Smm/{StatusCodeHandlerSmm.inf => 
StatusCodeHandlerStandaloneMm.inf} | 32 -
 9 files changed, 132 insertions(+), 62 deletions(-)

diff --git 
a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c 
b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
index c9b43fd2468f..14bac8ec3c18 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c
@@ -7,15 +7,15 @@
 
 **/
 
-#include "StatusCodeHandlerSmm.h"
+#include "StatusCodeHandlerMm.h"
 
-RUNTIME_MEMORY_STATUSCODE_HEADER  *mSmmMemoryStatusCodeTable;
+RUNTIME_MEMORY_STATUSCODE_HEADER  *mMmMemoryStatusCodeTable;
 
 /**
-  Initialize SMM memory status code table as initialization for memory status 
code worker
+  Initialize MM memory status code table as initialization for memory status 
code worker
 
-  @retval EFI_SUCCESS  SMM memory status code table successfully initialized.
-  @retval others   Errors from gSmst->SmmInstallConfigurationTable().
+  @retval EFI_SUCCESS  MM memory status code table successfully initialized.
+  @retval others   Errors from gMmst->MmInstallConfigurationTable().
 **/
 EFI_STATUS
 MemoryStatusCodeInitializeWorker (
@@ -25,17 +25,17 @@ MemoryStatusCodeInitializeWorker (
   EFI_STATUSStatus;
 
   //
-  // Allocate SMM memory status code pool.
+  // Allocate MM memory status code pool.
   //
-  mSmmMemoryStatusCodeTable = (RUNTIME_MEMORY_STATUSCODE_HEADER 
*)AllocateZeroPool (sizeof (RUNTIME_MEMORY_STATUSCODE_HEADER) + PcdGet16 
(PcdStatusCodeMemorySize) * 1024);
-  ASSERT (mSmmMemoryStatusCodeTable != NULL);
+  mMmMemoryStatusCodeTable = (RUNTIME_MEMORY_STATUSCODE_HEADER 
*)AllocateZeroPool (sizeof (RUNTIME_MEMORY_STATUSCODE_HEADER) + PcdGet16 
(PcdStatusCodeMemorySize) * 1024);
+  ASSERT (mMmMemoryStatusCodeTable != NULL);
 
-  mSmmMemoryStatusCodeTable->MaxRecordsNumber = (PcdGet16 
(PcdStatusCodeMemorySize) * 1024) / sizeof (MEMORY_STATUSCODE_RECORD);
-  Status = gSmst->SmmInstallConfigurationTable (
-gSmst,
+  mMmMemoryStatusCodeTable->MaxRecordsNumber = (PcdGet16 
(PcdStatusCodeMemorySize) * 1024) / sizeof (MEMORY_STATUSCODE_RECORD);
+  Status = gMmst->MmInstallConfigurationTable (
+gMmst,
 ,
-,
-sizeof (mSmmMemoryStatusCodeTable)
+,
+sizeof (mMmMemoryStatusCodeTable)
 );
   return Status;
 }
@@ -74,8 +74,8 @@ MemoryStatusCodeReportWorker (
   //
   // Locate current record buffer.
   //
-  Record = (MEMORY_STATUSCODE_RECORD *) (mSmmMemoryStatusCodeTable + 1);
-  Record = [mSmmMemoryStatusCodeTable->RecordIndex++];
+  Record = (MEMORY_STATUSCODE_RECORD *) (mMmMemoryStatusCodeTable + 1);
+  Record = [mMmMemoryStatusCodeTable->RecordIndex++];
 
   //
   // Save status code.
@@ -92,12 +92,12 @@ MemoryStatusCodeReportWorker (
   // so the first record is pointed by record index.
   // If it is less then max number, index of the first record is zero.
   //
-  mSmmMemoryStatusCodeTable->NumberOfRecords++;
-  if (mSmmMemoryStatusCodeTable->RecordIndex == 
mSmmMemoryStatusCodeTable->MaxRecordsNumber) {
+  mMmMemoryStatusCodeTable->NumberOfRecords++;
+  if 

[edk2-devel] [PATCH v4 15/20] SecurityPkg: Tcg2PpVendorLibNull: Added support for MM_STANDALONE type

2021-01-26 Thread Kun Qin
This change extends this null instance of Tcg2PpVendorLib to support
MM_STANDALONE drivers.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Previously reviewed. No change.

v2:
- Added Reviewed-by tag [Jiewen]

 SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf 
b/SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
index b969cbf9afff..9b18a1ab82f1 100644
--- a/SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
+++ b/SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
@@ -13,7 +13,7 @@ [Defines]
   FILE_GUID  = 51924AE9-BE81-4820-94BA-7C9546E702D0
   MODULE_TYPE= DXE_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = Tcg2PpVendorLib|DXE_RUNTIME_DRIVER 
DXE_SMM_DRIVER DXE_DRIVER
+  LIBRARY_CLASS  = Tcg2PpVendorLib|DXE_RUNTIME_DRIVER 
DXE_SMM_DRIVER DXE_DRIVER MM_STANDALONE
 
 #
 # The following information is for reference only and not required by the 
build tools.
-- 
2.30.0.windows.1



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




[edk2-devel] [PATCH v4 07/20] MdeModulePkg: SmmReportStatusCodeLib: ReportStatusCodeLib in StandaloneMm

2021-01-26 Thread Kun Qin
This change added support of StandaloneMm for ReportStatusCodeLib. It
adds a new instance of ReportStatusCodeLib for MM_STANDALONE type, and
abstracts the references of gMmst and gSmst functionalities into separate
files in order to link in proper Service Table for SMM core/drivers.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Dandan Bi 
Cc: Liming Gao 
Cc: Jiewen Yao 

Signed-off-by: Kun Qin 
Reviewed-by: Hao A Wu 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Added reviewed-by tag [Hao]

v2:
- Removed "EFIAPI" for internal functions.
- Minor new file description update.

 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.c  
 | 16 +
 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibStandaloneMm.c  
 | 38 
 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibTraditional.c   
 | 38 
 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.h  
 | 36 +++
 MdeModulePkg/Library/SmmReportStatusCodeLib/SmmReportStatusCodeLib.inf 
 |  4 ++-
 MdeModulePkg/Library/SmmReportStatusCodeLib/{SmmReportStatusCodeLib.inf => 
StandaloneMmReportStatusCodeLib.inf} | 22 ++--
 MdeModulePkg/MdeModulePkg.dsc  
 |  1 +
 7 files changed, 137 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.c 
b/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.c
index 3a1772538cdf..fb1769db9223 100644
--- a/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.c
@@ -1,5 +1,5 @@
 /** @file
-  Report Status Code Library for SMM Phase.
+  Report Status Code Library for MM Phase.
 
   Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -8,7 +8,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -16,10 +16,12 @@
 
 #include 
 #include 
-#include 
+#include 
 
-EFI_SMM_REPORT_STATUS_CODE mReportStatusCode = NULL;
-EFI_SMM_STATUS_CODE_PROTOCOL   *mStatusCodeProtocol = NULL;
+#include "ReportStatusCodeLib.h"
+
+EFI_MM_REPORT_STATUS_CODE mReportStatusCode = NULL;
+EFI_MM_STATUS_CODE_PROTOCOL   *mStatusCodeProtocol = NULL;
 
 
 /**
@@ -29,14 +31,14 @@ EFI_SMM_STATUS_CODE_PROTOCOL   *mStatusCodeProtocol = NULL;
 NULL is returned if no status code service is available.
 
 **/
-EFI_SMM_REPORT_STATUS_CODE
+EFI_MM_REPORT_STATUS_CODE
 InternalGetReportStatusCode (
   VOID
   )
 {
   EFI_STATUSStatus;
 
-  Status = gSmst->SmmLocateProtocol (, NULL, 
(VOID**));
+  Status = InternalLocateProtocol (, NULL, 
(VOID**));
   if (!EFI_ERROR (Status) && mStatusCodeProtocol != NULL) {
 return mStatusCodeProtocol->ReportStatusCode;
   }
diff --git 
a/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibStandaloneMm.c 
b/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibStandaloneMm.c
new file mode 100644
index ..a4c428dc88a9
--- /dev/null
+++ 
b/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibStandaloneMm.c
@@ -0,0 +1,38 @@
+/** @file
+  Abstraction layer for MM service table used by MM ReportStatusCodeLib.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+#include 
+
+/**
+  Returns the first protocol instance that matches the given protocol.
+
+  @param[in]  Protocol  Provides the protocol to search for.
+  @param[in]  Registration  Optional registration key returned from
+RegisterProtocolNotify().
+  @param[out]  InterfaceOn return, a pointer to the first interface 
that matches Protocol and
+Registration.
+
+  @retval EFI_SUCCESS   A protocol instance matching Protocol was 
found and returned in
+Interface.
+  @retval EFI_NOT_FOUND No protocol instances were found that match 
Protocol and
+Registration.
+  @retval EFI_INVALID_PARAMETER Interface is NULL.
+Protocol is NULL.
+
+**/
+EFI_STATUS
+InternalLocateProtocol (
+  IN  EFI_GUID  *Protocol,
+  IN  VOID  *Registration, OPTIONAL
+  OUT VOID  **Interface
+  )
+{
+  return gMmst->MmLocateProtocol (Protocol, Registration, Interface);
+}
diff --git 
a/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibTraditional.c 
b/MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibTraditional.c
new file mode 100644
index ..603e222f5508
--- /dev/null
+++ 

[edk2-devel] [PATCH v4 14/20] SecurityPkg: Tcg2PhysicalPresenceLib: Introduce StandaloneMm instance

2021-01-26 Thread Kun Qin
This change added a new instance of Tcg2PhysicalPresenceLib to support
MM_STANDALONE type drivers. It centralizes the common routines into
shared files and abstract the library constructor into corresponding
files to implement each constructor function prototypes.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Previously reviewed. No change.

v2:
- Added Reviewed-by tag [Jiewen]
- Removed "EFIAPI" for internal functions
- Updated internal function description

 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/{SmmTcg2PhysicalPresenceLib.c 
=> MmTcg2PhysicalPresenceLibCommon.c} |  33 +-
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
| 368 +---
 
SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/StandaloneMmTcg2PhysicalPresenceLib.c
   |  42 +++
 
SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.h
   |  34 ++
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf  
|   6 +-
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/{SmmTcg2PhysicalPresenceLib.inf 
=> StandaloneMmTcg2PhysicalPresenceLib.inf} |  22 +-
 SecurityPkg/SecurityPkg.dsc
|   2 +
 7 files changed, 113 insertions(+), 394 deletions(-)

diff --git 
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c 
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
similarity index 90%
copy from 
SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
copy to 
SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
index 8afaa0a7857d..3788537db318 100644
--- 
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
+++ 
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include 
+#include 
 
 #include 
 
@@ -25,7 +25,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
-#include 
+#include 
 
 #define PP_INF_VERSION_1_2"1.2"
 
@@ -55,7 +55,7 @@ Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction (
   UINTN DataSize;
   EFI_TCG2_PHYSICAL_PRESENCEPpData;
 
-  DEBUG ((EFI_D_INFO, "[TPM2] ReturnOperationResponseToOsFunction\n"));
+  DEBUG ((DEBUG_INFO, "[TPM2] ReturnOperationResponseToOsFunction\n"));
 
   //
   // Get the Physical Presence variable
@@ -71,7 +71,7 @@ Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction (
   if (EFI_ERROR (Status)) {
 *MostRecentRequest = 0;
 *Response  = 0;
-DEBUG ((EFI_D_ERROR, "[TPM2] Get PP variable failure! Status = %r\n", 
Status));
+DEBUG ((DEBUG_ERROR, "[TPM2] Get PP variable failure! Status = %r\n", 
Status));
 return TCG_PP_RETURN_TPM_OPERATION_RESPONSE_FAILURE;
   }
 
@@ -108,7 +108,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
   EFI_TCG2_PHYSICAL_PRESENCEPpData;
   EFI_TCG2_PHYSICAL_PRESENCE_FLAGS  Flags;
 
-  DEBUG ((EFI_D_INFO, "[TPM2] SubmitRequestToPreOSFunction, Request = %x, 
%x\n", *OperationRequest, *RequestParameter));
+  DEBUG ((DEBUG_INFO, "[TPM2] SubmitRequestToPreOSFunction, Request = %x, 
%x\n", *OperationRequest, *RequestParameter));
   ReturnCode = TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS;
 
   //
@@ -123,7 +123,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
  
  );
   if (EFI_ERROR (Status)) {
-DEBUG ((EFI_D_ERROR, "[TPM2] Get PP variable failure! Status = %r\n", 
Status));
+DEBUG ((DEBUG_ERROR, "[TPM2] Get PP variable failure! Status = %r\n", 
Status));
 ReturnCode = TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
 goto EXIT;
   }
@@ -147,7 +147,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (

);
 if (EFI_ERROR (Status)) {
-  DEBUG ((EFI_D_ERROR, "[TPM2] Set PP variable failure! Status = %r\n", 
Status));
+  DEBUG ((DEBUG_ERROR, "[TPM2] Set PP variable failure! Status = %r\n", 
Status));
   ReturnCode = TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
   goto EXIT;
 }
@@ -173,7 +173,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
   // Sync PPRQ/PPRM from PP Variable if PP submission fails
   //
   if (ReturnCode != TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS) {
-DEBUG ((EFI_D_ERROR, "[TPM2] Submit PP Request failure! Sync PPRQ/PPRM 
with PP variable.\n", Status));
+DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request 

[edk2-devel] [PATCH v4 06/20] MdeModulePkg: SmmLockBoxSmmLib: Support StandaloneMm for SmmLockBoxLib

2021-01-26 Thread Kun Qin
This change added support of StandaloneMm for SmmLockBoxLib. It replaces
gSmst with gMmst to support both traditional MM and standalone MM. The
contructor and desctructor functions are abstracted to support different
function prototype definitions.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Eric Dong 

Signed-off-by: Kun Qin 
Reviewed-by: Hao A Wu 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Previously reviewed. No change.

v2:
- Removed "EFIAPI" for internal library functions [Hao]
- Added Reviewed-by tag [Hao]

 MdeModulePkg/Library/SmmLockBoxLib/{SmmLockBoxSmmLib.c => SmmLockBoxMmLib.c}   
| 82 +---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandaloneMmLib.c 
| 53 +
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxTraditionalMmLib.c
| 53 +
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxLibPrivate.h  
| 25 ++
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
| 15 ++--
 MdeModulePkg/Library/SmmLockBoxLib/{SmmLockBoxSmmLib.inf => 
SmmLockBoxStandaloneMmLib.inf} | 26 ---
 MdeModulePkg/MdeModulePkg.dsc  
|  2 +
 7 files changed, 191 insertions(+), 65 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxMmLib.c
similarity index 89%
rename from MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
rename to MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxMmLib.c
index 4cc0e8b78e5b..a709851806eb 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxMmLib.c
@@ -6,16 +6,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include 
-#include 
+#include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
-#include 
+#include 
+#include 
 #include 
 
 #include "SmmLockBoxLibPrivate.h"
@@ -49,13 +49,13 @@ InternalGetSmmLockBoxContext (
   //
   // Check if gEfiSmmLockBoxCommunicationGuid is installed by someone
   //
-  for (Index = 0; Index < gSmst->NumberOfTableEntries; Index++) {
-if (CompareGuid (>SmmConfigurationTable[Index].VendorGuid, 
)) {
+  for (Index = 0; Index < gMmst->NumberOfTableEntries; Index++) {
+if (CompareGuid (>MmConfigurationTable[Index].VendorGuid, 
)) {
   //
   // Found. That means some other library instance is already run.
   // No need to install again, just return.
   //
-  return (SMM_LOCK_BOX_CONTEXT 
*)gSmst->SmmConfigurationTable[Index].VendorTable;
+  return (SMM_LOCK_BOX_CONTEXT 
*)gMmst->MmConfigurationTable[Index].VendorTable;
 }
   }
 
@@ -142,8 +142,8 @@ SmmLockBoxSmmEndOfDxeNotify (
   //
   // Locate SmmSxDispatch2 protocol.
   //
-  Status = gSmst->SmmLocateProtocol (
-,
+  Status = gMmst->MmLocateProtocol (
+,
 NULL,
 (VOID **)
 );
@@ -191,29 +191,24 @@ SmmLockBoxEndOfS3ResumeNotify (
   Constructor for SmmLockBox library.
   This is used to set SmmLockBox context, which will be used in PEI phase in 
S3 boot path later.
 
-  @param[in] ImageHandle  Image handle of this driver.
-  @param[in] SystemTable  A Pointer to the EFI System Table.
-
   @retval EFI_SUCEESS
   @return Others  Some error occurs.
 **/
 EFI_STATUS
-EFIAPI
-SmmLockBoxSmmConstructor (
-  IN EFI_HANDLEImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+SmmLockBoxMmConstructor (
+  VOID
   )
 {
   EFI_STATUS   Status;
   SMM_LOCK_BOX_CONTEXT *SmmLockBoxContext;
 
-  DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib SmmLockBoxSmmConstructor - Enter\n"));
+  DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib SmmLockBoxMmConstructor - Enter\n"));
 
   //
   // Register SmmReadyToLock notification.
   //
-  Status = gSmst->SmmRegisterProtocolNotify (
-,
+  Status = gMmst->MmRegisterProtocolNotify (
+,
 SmmLockBoxSmmReadyToLockNotify,
 
 );
@@ -222,8 +217,8 @@ SmmLockBoxSmmConstructor (
   //
   // Register SmmEndOfDxe notification.
   //
-  Status = gSmst->SmmRegisterProtocolNotify (
-,
+  Status = gMmst->MmRegisterProtocolNotify (
+,
 SmmLockBoxSmmEndOfDxeNotify,
 
 );
@@ -232,7 +227,7 @@ SmmLockBoxSmmConstructor (
   //
   // Register EndOfS3Resume notification.
   //
-  Status = gSmst->SmmRegisterProtocolNotify (
+  Status = gMmst->MmRegisterProtocolNotify (
 ,
 SmmLockBoxEndOfS3ResumeNotify,
 
@@ -249,7 +244,7 @@ SmmLockBoxSmmConstructor (
 // No need to install again, just return.
 //
 DEBUG ((DEBUG_INFO, "SmmLockBoxSmmLib SmmLockBoxContext - already 

[edk2-devel] [PATCH v4 05/20] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

2021-01-26 Thread Kun Qin
This change extends StandaloneMmMemLib library to support X64
architecture. The implementation is ported from MdePkg/Library/SmmMemLib.

Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Jiewen Yao 
Cc: Supreeth Venkatesh 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Added reviewed-by tag [Jiewen]

v3:
- Updated destructor description of varibales to pass CI build.

v2:
- Added routine to fix bug of not initializing MmRanges [Jiewen]
- Extends support to x86 instead of x64 only [Hao]

 
StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c 
|  27 
 StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
 |  52 +++
 StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c 
 | 155 
 StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf  
 |  13 +-
 4 files changed, 246 insertions(+), 1 deletion(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
index cb7c5e677a6b..4124959e0435 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
@@ -40,4 +40,31 @@ MmMemLibInternalCalculateMaximumSupportAddress (
   DEBUG ((DEBUG_INFO, "mMmMemLibInternalMaximumSupportAddress = 0x%lx\n", 
mMmMemLibInternalMaximumSupportAddress));
 }
 
+/**
+  Initialize cached Mmram Ranges from HOB.
+
+  @retval EFI_UNSUPPORTED   The routine is unable to extract MMRAM information.
+  @retval EFI_SUCCESS   MmRanges are populated successfully.
+
+**/
+EFI_STATUS
+MmMemLibInternalPopulateMmramRanges (
+  VOID
+  )
+{
+  // Not implemented for AARCH64.
+  return EFI_SUCCESS;
+}
+
+/**
+  Deinitialize cached Mmram Ranges.
+
+**/
+VOID
+MmMemLibInternalFreeMmramRanges (
+  VOID
+  )
+{
+  // Not implemented for AARCH64.
+}
 
diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
index b533bd8390cd..2737f95315eb 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
@@ -37,6 +37,27 @@ MmMemLibInternalCalculateMaximumSupportAddress (
   VOID
   );
 
+/**
+  Initialize cached Mmram Ranges from HOB.
+
+  @retval EFI_UNSUPPORTED   The routine is unable to extract MMRAM information.
+  @retval EFI_SUCCESS   MmRanges are populated successfully.
+
+**/
+EFI_STATUS
+MmMemLibInternalPopulateMmramRanges (
+  VOID
+  );
+
+/**
+  Deinitialize cached Mmram Ranges.
+
+**/
+VOID
+MmMemLibInternalFreeMmramRanges (
+  VOID
+  );
+
 /**
   This function check if the buffer is valid per processor architecture and 
not overlap with MMRAM.
 
@@ -253,11 +274,42 @@ MemLibConstructor (
   IN EFI_MM_SYSTEM_TABLE*MmSystemTable
   )
 {
+  EFI_STATUS Status;
 
   //
   // Calculate and save maximum support address
   //
   MmMemLibInternalCalculateMaximumSupportAddress ();
 
+  //
+  // Initialize cached Mmram Ranges from HOB.
+  //
+  Status = MmMemLibInternalPopulateMmramRanges ();
+
+  return Status;
+}
+
+/**
+  Destructor for Mm Mem library.
+
+  @param ImageHandleThe image handle of the process.
+  @param MmSystemTable  The EFI System Table pointer.
+
+  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+MemLibDestructor (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE*MmSystemTable
+  )
+{
+
+  //
+  // Deinitialize cached Mmram Ranges.
+  //
+  MmMemLibInternalFreeMmramRanges ();
+
   return EFI_SUCCESS;
 }
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c
new file mode 100644
index ..1a978541716a
--- /dev/null
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c
@@ -0,0 +1,155 @@
+/** @file
+  Internal ARCH Specific file of MM memory check library.
+
+  MM memory check library implementation. This library consumes 
MM_ACCESS_PROTOCOL
+  to get MMRAM information. In order to use this library instance, the 
platform should produce
+  all MMRAM range via MM_ACCESS_PROTOCOL, including the range for firmware 
(like MM Core
+  and MM driver) and/or specific dedicated hardware.
+
+  Copyright (c) 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) Microsoft Corporation.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+//
+// Maximum support address used to check input buffer
+//
+extern EFI_PHYSICAL_ADDRESS  mMmMemLibInternalMaximumSupportAddress;
+extern 

[edk2-devel] [PATCH v4 13/20] PcAtChipsetPkg: AcpiTimerLib: Added StandaloneMm instance of AcpiTimerLib

2021-01-26 Thread Kun Qin
This change added a new instance of AcpiTimerLib for StandaloneMm core
and drivers. It centralizes the common routines into shared files and
abstract the library constructor into corresponding files to accommodate
each constructor function prototypes.

Cc: Ray Ni 

Signed-off-by: Kun Qin 
Reviewed-by: Ray Ni 
---

Notes:
v4:
- Added reviewed-by tag [Ray]

v3:
- Renamed "CommonAcpiTimerLib" to "DxeStandaloneMmAcpiTimerLib" to
avoid confusion [Ray]
- Renamed BASE NAME (and file name) to StandaloneMmAcpiTimerLib [Ray]

v2:
- Removed "EFIAPI" for internal functions.

 PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c  
   | 81 +---
 PcAtChipsetPkg/Library/AcpiTimerLib/{DxeAcpiTimerLib.c => 
DxeStandaloneMmAcpiTimerLib.c}  |  9 +--
 PcAtChipsetPkg/Library/AcpiTimerLib/StandaloneMmAcpiTimerLib.c 
   | 31 
 PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
   |  2 +
 PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.h  
   | 24 ++
 PcAtChipsetPkg/Library/AcpiTimerLib/{DxeAcpiTimerLib.inf => 
StandaloneMmAcpiTimerLib.inf} | 19 +++--
 PcAtChipsetPkg/PcAtChipsetPkg.dsc  
   |  1 +
 7 files changed, 74 insertions(+), 93 deletions(-)

diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c 
b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
index 3ad831b15e8a..9ac2a446e365 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
@@ -2,72 +2,14 @@
   ACPI Timer implements one instance of Timer Library.
 
   Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) Microsoft Corporation.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include 
-#include 
-#include 
-#include 
 
-extern GUID mFrequencyHobGuid;
-
-/**
-  The constructor function enables ACPI IO space.
-
-  If ACPI I/O space not enabled, this function will enable it.
-  It will always return RETURN_SUCCESS.
-
-  @retval EFI_SUCCESS   The constructor always returns RETURN_SUCCESS.
-
-**/
-RETURN_STATUS
-EFIAPI
-AcpiTimerLibConstructor (
-  VOID
-  );
-
-/**
-  Calculate TSC frequency.
-
-  The TSC counting frequency is determined by comparing how far it counts
-  during a 101.4 us period as determined by the ACPI timer.
-  The ACPI timer is used because it counts at a known frequency.
-  The TSC is sampled, followed by waiting 363 counts of the ACPI timer,
-  or 101.4 us. The TSC is then sampled again. The difference multiplied by
-  9861 is the TSC frequency. There will be a small error because of the
-  overhead of reading the ACPI timer. An attempt is made to determine and
-  compensate for this error.
-
-  @return The number of TSC counts per second.
-
-**/
-UINT64
-InternalCalculateTscFrequency (
-  VOID
-  );
-
-//
-// Cached performance counter frequency
-//
-UINT64  mPerformanceCounterFrequency = 0;
-
-/**
-  Internal function to retrieves the 64-bit frequency in Hz.
-
-  Internal function to retrieves the 64-bit frequency in Hz.
-
-  @return The frequency in Hz.
-
-**/
-UINT64
-InternalGetPerformanceCounterFrequency (
-  VOID
-  )
-{
-  return  mPerformanceCounterFrequency;
-}
+#include "DxeStandaloneMmAcpiTimerLib.h"
 
 /**
   The constructor function enables ACPI IO space, and caches 
PerformanceCounterFrequency.
@@ -85,22 +27,5 @@ DxeAcpiTimerLibConstructor (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_HOB_GUID_TYPE   *GuidHob;
-
-  //
-  // Enable ACPI IO space.
-  //
-  AcpiTimerLibConstructor ();
-
-  //
-  // Initialize PerformanceCounterFrequency
-  //
-  GuidHob = GetFirstGuidHob ();
-  if (GuidHob != NULL) {
-mPerformanceCounterFrequency = *(UINT64*)GET_GUID_HOB_DATA (GuidHob);
-  } else {
-mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
-  }
-
-  return EFI_SUCCESS;
+  return CommonAcpiTimerLibConstructor ();
 }
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c 
b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
similarity index 86%
copy from PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
copy to PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
index 3ad831b15e8a..0e401194d01d 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
@@ -72,17 +72,12 @@ InternalGetPerformanceCounterFrequency (
 /**
   The constructor function enables ACPI IO space, and caches 
PerformanceCounterFrequency.
 
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
-
   @retval EFI_SUCCESS   The constructor always returns RETURN_SUCCESS.
 
 **/
 EFI_STATUS
-EFIAPI
-DxeAcpiTimerLibConstructor (
-  IN EFI_HANDLEImageHandle,
-  IN 

[edk2-devel] [PATCH v4 04/20] StandaloneMmPkg: StandaloneMmCoreMemoryAllocationLib: Fix compiler warning

2021-01-26 Thread Kun Qin
Assigning MmramRangeCount from MmCorePrivate (UINT64) to local variable
MmramRangeCount (UINT32) will cause compilation failure due to "warning
C4244: '=': conversion from 'UINT64' to 'UINT32', possible loss of data".
This changes defines local MmramRangeCount as UINTN type and adds type
cast before value assignment.

Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Jiewen Yao 
Cc: Supreeth Venkatesh 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Reviewed previously, no change

v3:
- Added reviewed-by tag [Jiewen]

v2:
- Changed variable type to UINTN and cast before assignments [Jiewen]

 
StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.c
 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.c
index 8fbd4d934784..ba5470dd7156 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.c
@@ -841,7 +841,7 @@ MemoryAllocationLibConstructor (
   VOID*HobStart;
   EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHobData;
   EFI_MMRAM_DESCRIPTOR*MmramRanges;
-  UINT32   MmramRangeCount;
+  UINTNMmramRangeCount;
   EFI_HOB_GUID_TYPE   *MmramRangesHob;
 
   HobStart = GetHobList ();
@@ -868,7 +868,7 @@ MemoryAllocationLibConstructor (
   return EFI_UNSUPPORTED;
 }
 
-MmramRangeCount = MmramRangesHobData->NumberOfMmReservedRegions;
+MmramRangeCount = (UINTN) MmramRangesHobData->NumberOfMmReservedRegions;
 if (MmramRanges == NULL) {
   return EFI_UNSUPPORTED;
 }
@@ -877,7 +877,7 @@ MemoryAllocationLibConstructor (
 DataInHob  = GET_GUID_HOB_DATA (GuidHob);
 MmCorePrivate = (MM_CORE_PRIVATE_DATA *)(UINTN)DataInHob->Address;
 MmramRanges = (EFI_MMRAM_DESCRIPTOR 
*)(UINTN)MmCorePrivate->MmramRanges;
-MmramRangeCount = MmCorePrivate->MmramRangeCount;
+MmramRangeCount = (UINTN) MmCorePrivate->MmramRangeCount;
   }
 
   {
-- 
2.30.0.windows.1



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




[edk2-devel] [PATCH v4 09/20] MdeModulePkg: FirmwarePerformanceDataTable: Added StandaloneMm support

2021-01-26 Thread Kun Qin
This change added support of FPDT driver under StandaloneMm. It replaces
SMM version ReportStatusCode protocol with MM version. This patch also
abstracts standalone and traditional MM interfaces into separate files to
support each corresponding function prototypes and implementations.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Dandan Bi 
Cc: Liming Gao 

Signed-off-by: Kun Qin 
Reviewed-by: Hao A Wu 
---

Notes:
v4:
- Previously reviewed. No change.

v3:
- Added reviewed-by tag [Hao]

v2:
- Removed "EFIAPI" for internally abstracted functions [Hao]
- Support driver for both IA32 and X64 in dsc file [Hao]

 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{FirmwarePerformanceSmm.c
 => FirmwarePerformanceCommon.c}   | 76 +---
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceStandaloneMm.c
   | 61 
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceTraditional.c
| 61 
 MdeModulePkg/MdeModulePkg.dsc  
 |  2 +
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.h
 | 50 +
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf
  | 11 +--
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{FirmwarePerformanceSmm.inf
 => FirmwarePerformanceStandaloneMm.inf} | 31 
 7 files changed, 230 insertions(+), 62 deletions(-)

diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
similarity index 75%
rename from 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
rename to 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
index d6c6e7693e4d..ecadef8711ed 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c
+++ 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceCommon.c
@@ -1,11 +1,11 @@
 /** @file
-  This module collects performance data for SMM driver boot records and S3 
Suspend Performance Record.
+  This module collects performance data for MM driver boot records and S3 
Suspend Performance Record.
 
   This module registers report status code listener to collect performance data
-  for SMM driver boot records and S3 Suspend Performance Record.
+  for MM driver boot records and S3 Suspend Performance Record.
 
   Caution: This module requires additional review when modified.
-  This driver will have external input - communicate buffer in SMM mode.
+  This driver will have external input - communicate buffer in MM mode.
   This external input must be validated carefully to avoid security issue like
   buffer overflow, integer overflow.
 
@@ -16,13 +16,13 @@
 
 **/
 
-#include 
+#include 
 
-#include 
+#include 
 
 #include 
 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -30,23 +30,22 @@
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
+#include "FirmwarePerformanceCommon.h"
 
-SMM_BOOT_PERFORMANCE_TABLE*mSmmBootPerformanceTable = NULL;
+SMM_BOOT_PERFORMANCE_TABLE*mMmBootPerformanceTable = NULL;
 
-EFI_SMM_RSC_HANDLER_PROTOCOL  *mRscHandlerProtocol= NULL;
+EFI_MM_RSC_HANDLER_PROTOCOL   *mRscHandlerProtocol= NULL;
 UINT64mSuspendStartTime   = 0;
 BOOLEAN   mS3SuspendLockBoxSaved  = FALSE;
 UINT32mBootRecordSize = 0;
 UINT8 *mBootRecordBuffer = NULL;
 
-SPIN_LOCK mSmmFpdtLock;
-BOOLEAN   mSmramIsOutOfResource = FALSE;
+SPIN_LOCK mMmFpdtLock;
+BOOLEAN   mMmramIsOutOfResource = FALSE;
 
 /**
-  Report status code listener for SMM. This is used to record the performance
+  Report status code listener for MM. This is used to record the performance
   data for S3 Suspend Start and S3 Suspend End in FPDT.
 
   @param[in]  CodeTypeIndicates the type of status code being 
reported.
@@ -66,7 +65,7 @@ BOOLEAN   mSmramIsOutOfResource = FALSE;
 **/
 EFI_STATUS
 EFIAPI
-FpdtStatusCodeListenerSmm (
+FpdtStatusCodeListenerMm (
   IN EFI_STATUS_CODE_TYPE CodeType,
   IN EFI_STATUS_CODE_VALUEValue,
   IN UINT32   Instance,
@@ -89,19 +88,19 @@ FpdtStatusCodeListenerSmm (
   // Collect one or more Boot records in boot time
   //
   if (Data != NULL && CompareGuid (>Type, 
)) {
-AcquireSpinLock ();
+AcquireSpinLock ();
 //
 // Get the boot performance data.
 //
-CopyMem (, Data + 

[edk2-devel] [PATCH v4 03/20] StandaloneMmPkg: StandaloneMmCoreHobLib: Extend support for x64 Mm Core

2021-01-26 Thread Kun Qin
This change adds support of x64 version of StandaloneMmCoreHobLib. It
brings in global variable "gHobList" through StandaloneMmCoreEntryPoint,
imports implementation from DxeCoreHobLib.inf to support x64 Mm Core and
moved shared functional plementations into a common file.

Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Jiewen Yao 
Cc: Supreeth Venkatesh 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Added reviewed-by tag [Jiewen]

v3:
- Pertains gHobList for AARCH64 instance.

v2:
- Moved common function implementations into Common.c [Jiewen]

 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{ => 
AArch64}/StandaloneMmCoreHobLib.c | 272 --
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Common.c
   | 291 +++
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/X64/StandaloneMmCoreHobLib.c
   | 298 
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf  
   |  11 +-
 4 files changed, 597 insertions(+), 275 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
similarity index 55%
rename from 
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.c
rename to 
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
index e3d4743b63f2..0ec2d4ad6f6b 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
@@ -21,188 +21,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 VOID *gHobList = NULL;
 
-/**
-  Returns the pointer to the HOB list.
-
-  This function returns the pointer to first HOB in the list.
-  If the pointer to the HOB list is NULL, then ASSERT().
-
-  @return The pointer to the HOB list.
-
-**/
-VOID *
-EFIAPI
-GetHobList (
-  VOID
-  )
-{
-  ASSERT (gHobList != NULL);
-  return gHobList;
-}
-
-/**
-  Returns the next instance of a HOB type from the starting HOB.
-
-  This function searches the first instance of a HOB type from the starting 
HOB pointer.
-  If there does not exist such HOB type from the starting HOB pointer, it will 
return NULL.
-  In contrast with macro GET_NEXT_HOB(), this function does not skip the 
starting HOB pointer
-  unconditionally: it returns HobStart back if HobStart itself meets the 
requirement;
-  caller is required to use GET_NEXT_HOB() if it wishes to skip current 
HobStart.
-
-  If HobStart is NULL, then ASSERT().
-
-  @param  Type  The HOB type to return.
-  @param  HobStart  The starting HOB pointer to search from.
-
-  @return The next instance of a HOB type from the starting HOB.
-
-**/
-VOID *
-EFIAPI
-GetNextHob (
-  IN UINT16 Type,
-  IN CONST VOID *HobStart
-  )
-{
-  EFI_PEI_HOB_POINTERS  Hob;
-
-  ASSERT (HobStart != NULL);
-
-  Hob.Raw = (UINT8 *) HobStart;
-  //
-  // Parse the HOB list until end of list or matching type is found.
-  //
-  while (!END_OF_HOB_LIST (Hob)) {
-if (Hob.Header->HobType == Type) {
-  return Hob.Raw;
-}
-Hob.Raw = GET_NEXT_HOB (Hob);
-  }
-  return NULL;
-}
-
-/**
-  Returns the first instance of a HOB type among the whole HOB list.
-
-  This function searches the first instance of a HOB type among the whole HOB 
list.
-  If there does not exist such HOB type in the HOB list, it will return NULL.
-
-  If the pointer to the HOB list is NULL, then ASSERT().
-
-  @param  Type  The HOB type to return.
-
-  @return The next instance of a HOB type from the starting HOB.
-
-**/
-VOID *
-EFIAPI
-GetFirstHob (
-  IN UINT16 Type
-  )
-{
-  VOID  *HobList;
-
-  HobList = GetHobList ();
-  return GetNextHob (Type, HobList);
-}
-
-/**
-  Returns the next instance of the matched GUID HOB from the starting HOB.
-
-  This function searches the first instance of a HOB from the starting HOB 
pointer.
-  Such HOB should satisfy two conditions:
-  its HOB type is EFI_HOB_TYPE_GUID_EXTENSION, and its GUID Name equals to the 
input Guid.
-  If such a HOB from the starting HOB pointer does not exist, it will return 
NULL.
-  Caller is required to apply GET_GUID_HOB_DATA () and GET_GUID_HOB_DATA_SIZE 
()
-  to extract the data section and its size information, respectively.
-  In contrast with macro GET_NEXT_HOB(), this function does not skip the 
starting HOB pointer
-  unconditionally: it returns HobStart back if HobStart itself meets the 
requirement;
-  caller is required to use GET_NEXT_HOB() if it wishes to skip current 
HobStart.
-
-  If Guid is NULL, then ASSERT().
-  If HobStart is NULL, then ASSERT().
-
-  @param  Guid  The GUID to match with in the HOB list.
-  @param  HobStart  A pointer to a Guid.
-
-  @return The next instance of the matched GUID HOB from the starting HOB.
-
-**/
-VOID *
-EFIAPI

[edk2-devel] [PATCH v4 02/20] StandaloneMmPkg: StandaloneMmCoreEntryPoint: Extends support for X64

2021-01-26 Thread Kun Qin
This change extends StandaloneMmCoreEntryPoint library to support X64
architecture.

Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Jiewen Yao 
Cc: Supreeth Venkatesh 

Signed-off-by: Kun Qin 
Reviewed-by: Jiewen Yao 
---

Notes:
v4:
- Reviewed previously. No change.

v3:
- Reviewed previously. No change.

v2:
- Added Reviewed-by tag [Jiewen]

 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/X64/StandaloneMmCoreEntryPoint.c
 | 71 
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
   |  3 +
 2 files changed, 74 insertions(+)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/X64/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/X64/StandaloneMmCoreEntryPoint.c
new file mode 100644
index ..dffa965b8425
--- /dev/null
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/X64/StandaloneMmCoreEntryPoint.c
@@ -0,0 +1,71 @@
+/** @file
+  Entry point to the Standalone Mm Core.
+
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+
+#include 
+
+#include 
+#include 
+#include 
+
+//
+// Cache copy of HobList pointer.
+//
+VOID *gHobList = NULL;
+
+/**
+  The entry point of PE/COFF Image for the STANDALONE MM Core.
+
+  This function is the entry point for the STANDALONE MM Core. This function 
is required to call
+  ProcessModuleEntryPointList() and ProcessModuleEntryPointList() is never 
expected to return.
+  The STANDALONE MM Core is responsible for calling 
ProcessLibraryConstructorList() as soon as the EFI
+  System Table and the image handle for the STANDALONE MM Core itself have 
been established.
+  If ProcessModuleEntryPointList() returns, then ASSERT() and halt the system.
+
+  @param  HobStart  Pointer to the beginning of the HOB List passed in from 
the PEI Phase.
+
+**/
+VOID
+EFIAPI
+_ModuleEntryPoint (
+  IN VOID  *HobStart
+  )
+{
+  //
+  // Cache a pointer to the HobList
+  //
+  gHobList = HobStart;
+
+  //
+  // Call the Standalone MM Core entry point
+  //
+  ProcessModuleEntryPointList (HobStart);
+
+  //
+  // TODO: Set page table here?? AARCH64 has this step for some reason
+  //
+}
+
+
+/**
+  Required by the EBC compiler and identical in functionality to 
_ModuleEntryPoint().
+
+  This function is required to call _ModuleEntryPoint() passing in HobStart.
+
+  @param  HobStart  Pointer to the beginning of the HOB List passed in from 
the PEI Phase.
+
+**/
+VOID
+EFIAPI
+EfiMain (
+  IN VOID  *HobStart
+  )
+{
+  _ModuleEntryPoint (HobStart);
+}
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 75a654b06d51..313bc6f7bdad 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -26,6 +26,9 @@ [Sources.AARCH64]
   AArch64/SetPermissions.c
   AArch64/CreateHobList.c
 
+[Sources.X64]
+  X64/StandaloneMmCoreEntryPoint.c
+
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
-- 
2.30.0.windows.1



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




[edk2-devel] [PATCH v4 00/20] Extends Support of MM_STANDALONE Type Modules to X64

2021-01-26 Thread Kun Qin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3129

This patch series is a follow up of previous submission:
https://edk2.groups.io/g/devel/message/70329

These module changes are validated on two different physical platforms.
Standalone and traditional MM are both tested to be functional on these
systems.

v4 patches mainly focus on feedback for reviewed commits in v3 patches,
including:
a. Adding "Reviewed-by" tags for applicable patches;
b. Breaking CpuIo2Smm patch for file renaming and abstraction purpose;
c. Adding SmmSmiHandlerProfileLib coverage;

Patch v4 branch: https://github.com/kuqin12/edk2/tree/standalone_x64_v4

Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Eric Dong 
Cc: Dandan Bi 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Zhiguang Liu 
Cc: Ray Ni 
Cc: Jiewen Yao 
Cc: Qi Zhang 
Cc: Rahul Kumar 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
Cc: Supreeth Venkatesh 
Cc: Laszlo Ersek 

Kun Qin (20):
  BaseTools: Ecc/exception: Added _ModuleEntryPoint into exception list
  StandaloneMmPkg: StandaloneMmCoreEntryPoint: Extends support for X64
  StandaloneMmPkg: StandaloneMmCoreHobLib: Extend support for x64 Mm
Core
  StandaloneMmPkg: StandaloneMmCoreMemoryAllocationLib: Fix compiler
warning
  StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64
architecture
  MdeModulePkg: SmmLockBoxSmmLib: Support StandaloneMm for SmmLockBoxLib
  MdeModulePkg: SmmReportStatusCodeLib: ReportStatusCodeLib in
StandaloneMm
  MdeModulePkg: StatusCodeHandler: StatusCodeHandler driver in
StandaloneMm
  MdeModulePkg: FirmwarePerformanceDataTable: Added StandaloneMm support
  MdeModulePkg: ReportStatusCodeRouter: Support StandaloneMm RSC Router
  MdeModulePkg: SmmSmiHandlerProfileLib: Support StandaloneMm Instance
  MdePkg: UefiDevicePathLib: Support UefiDevicePathLib under
StandaloneMm
  PcAtChipsetPkg: AcpiTimerLib: Added StandaloneMm instance of
AcpiTimerLib
  SecurityPkg: Tcg2PhysicalPresenceLib: Introduce StandaloneMm instance
  SecurityPkg: Tcg2PpVendorLibNull: Added support for MM_STANDALONE type
  SecurityPkg: Tpm2DeviceLibDTpm: Introduce StandaloneMm instance
  UefiCpuPkg: CpuIo2Smm: Move CpuIo2Smm driver to consume gMmst
  UefiCpuPkg: CpuIo2Smm: Abstract SMM specific functions into separate
file
  UefiCpuPkg: CpuIo2Smm: Support of CpuIo driver under StandaloneMm
  UefiCpuPkg: SmmCpuExceptionHandlerLib: Added StandaloneMm module
support

 MdeModulePkg/Library/SmmLockBoxLib/{SmmLockBoxSmmLib.c => SmmLockBoxMmLib.c}   
 |  82 ++---
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandaloneMmLib.c 
 |  53 +++
 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxTraditionalMmLib.c
 |  53 +++
 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLib.c  
 |  16 +-
 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibStandaloneMm.c  
 |  38 ++
 MdeModulePkg/Library/SmmReportStatusCodeLib/ReportStatusCodeLibTraditional.c   
 |  38 ++
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/{SmmSmiHandlerProfileLib.c => 
MmSmiHandlerProfileLib.c}|  20 +-
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c 
 |  90 +
 
MdeModulePkg/Library/SmmSmiHandlerProfileLib/StandaloneMmSmiHandlerProfileLib.c 
|  31 ++
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/{FirmwarePerformanceSmm.c
 => FirmwarePerformanceCommon.c}   |  76 ++--
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceStandaloneMm.c
   |  61 
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceTraditional.c
|  61 
 MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/{ReportStatusCodeRouterSmm.c 
=> ReportStatusCodeRouterCommon.c}   |  59 ++-
 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterStandaloneMm.c
  |  33 ++
 
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterTraditional.c
   |  33 ++
 MdeModulePkg/Universal/StatusCodeHandler/Smm/MemoryStatusCodeWorker.c  
 |  36 +-
 MdeModulePkg/Universal/StatusCodeHandler/Smm/SerialStatusCodeWorker.c  
 |   2 +-
 MdeModulePkg/Universal/StatusCodeHandler/Smm/{StatusCodeHandlerSmm.c => 
StatusCodeHandlerMm.c}  |  23 +-
 

[edk2-devel] [PATCH v4 01/20] BaseTools: Ecc/exception: Added _ModuleEntryPoint into exception list

2021-01-26 Thread Kun Qin
Function '_ModuleEntryPoint' is a pre-defined interface for various EFI
module types and should not be caught violating EFI coding style. This
change added '_ModuleEntryPoint' into exception list to fix EFI coding
style error 8006 during CI build.

Cc: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 

Signed-off-by: Kun Qin 
Reviewed-by: Liming Gao 
Reviewed-by: Bob Feng 
---

Notes:
v4:
- Added reviewed-by tags [Bob]
- Added reviewed-by tags [Liming]

v3:
- Newly added to fix CI build on changing '_ModuleEntryPoint'

 BaseTools/Source/Python/Ecc/exception.xml | 4 
 1 file changed, 4 insertions(+)

diff --git a/BaseTools/Source/Python/Ecc/exception.xml 
b/BaseTools/Source/Python/Ecc/exception.xml
index 8133904fbc7f..f2334aab8e52 100644
--- a/BaseTools/Source/Python/Ecc/exception.xml
+++ b/BaseTools/Source/Python/Ecc/exception.xml
@@ -296,6 +296,10 @@
 _DriverUnloadHandler
 8006
   
+  
+_ModuleEntryPoint
+8006
+  
   
 ASSERT
 10015
-- 
2.30.0.windows.1



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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Ankur Arora

On 2021-01-26 11:01 a.m., Laszlo Ersek wrote:

On 01/26/21 07:44, Ankur Arora wrote:

Refactor CpuHotplugMmi() to pull out the CPU hotplug logic into
PlugCpus(). This is in preparation for supporting CPU hot-unplug.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---
  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 208 ++---
  1 file changed, 123 insertions(+), 85 deletions(-)

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index cfe698ed2b5e..a5052a501e5a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -62,6 +62,124 @@ STATIC UINT32 mPostSmmPenAddress;
  //
  STATIC EFI_HANDLE mDispatchHandle;

+/**
+  CPU Hotplug handler function.
+
+  @param[in] PluggedApicIds  List of APIC IDs to be plugged.
+
+  @param[in] PluggedCountCount of APIC IDs to be plugged.


(1) These comments are not optimal.

The variable names say "Plugged", meaning that the CPUs have already
been plugged, from the QEMU perspective. The purpose of this function is
to go over those CPUs whose APIC IDs have been collected with events
pending, relocate the SMBASE on each, and then expose each such CPU to
EFI_SMM_CPU_SERVICE_PROTOCOL. For a given CPU, I think the comment on
the EFI_SMM_ADD_PROCESSOR prototype
[UefiCpuPkg/Include/Protocol/SmmCpuService.h] best expresses the goal:
"Notify that a new processor has been added to the system ... The SMM
CPU driver should add the processor to the SMM CPU list".

See also the comment on QemuCpuhpCollectApicIds():

"""
   Collect the APIC IDs of
   - the CPUs that have been hot-plugged,
   - the CPUs that are about to be hot-unplugged.
"""

This closely reflects which agent (firmware vs. QEMU) is driving each
particular operation / direction.

(1a) So please replace the leading comment with:

   Process those CPUs that have been hot-added, according to
   QemuCpuhpCollectApicIds().

   For each such CPU, relocate the SMBASE, and report the CPU to PiSmmCpuDxeSmm
   via EFI_SMM_CPU_SERVICE_PROTOCOL. If a supposedly hot-added CPU is already
   known, skip it silently.

(1b) Similarly, in the parameter comments, "to be plugged" is wrong. I
suggest copying the parameter descriptions from
QemuCpuhpCollectApicIds():

   @param[out] PluggedApicIds   The APIC IDs of the CPUs that have been
hot-plugged.

   @param[out] PluggedCount The number of filled-in APIC IDs in
PluggedApicIds.



+
+  @retval EFI_SUCCESSSome of the requested APIC IDs were 
hot-plugged.


(2) This is inexact; on successful return, each one of the collected
CPUs has been relocated and exposed to the SMM CPU driver. (Either in
this particular invocation, or in an earlier invocation, but on success,
there is no entry in PluggedApicIds that is *not known* to the SMM CPU
driver, or whose SMBASE is not relocated.)



+
+  @retval EFI_INTERRUPT_PENDING  Fatal error while hot-plugging.


(3) This error code is very uncommon, and it is mostly/only required
from the function -- CpuHotplugMmi() -- that is registered with
gMmst->MmiHandlerRegister(). The meaning of the error code is, "The MMI
source could not be quiesced", which is a situation that can be
identified at the level of CpuHotplugMmi(), but not at the level of
PlugCpus().

Please list the following return values instead:

   @retval EFI_OUT_OF_RESOURCES  Out of APIC ID space in "mCpuHotPlugData".

   @return   Error codes propagated from SmbaseRelocate()
 and mMmCpuService->AddProcessor().

(General remark, in addition: please note the difference between
"@retval" and "@return". The latter does not name a particular value;
the set of values is described in natural language instead.)



+
+**/
+STATIC
+EFI_STATUS
+EFIAPI


(4) There is no need to make this function EFIAPI.



+PlugCpus(


(5) Space character missing between function name and opening
parenthesis.

Please check every function prototype and function call for this -- one
space char before the opening paren is required, except in the
definition of function-like macros (where the language standard requires
the "(" to be directly adjacent).


(6) According to the discussion above, this function should be called
ProcessHotAddedCpus().

... The best hint for this function name is actually the comment that
sits atop the stretch of code you are extracting, namely:

   //
   // Process hot-added CPUs.
   //



+  IN APIC_ID  *PluggedApicIds,
+  IN UINT32   PluggedCount
+  )
+{
+  EFI_STATUS Status;
+  UINT32 PluggedIdx;
+  UINT32 NewSlot;
+
+  //
+  // Process hot-added CPUs.


(7) This short introductory comment is no longer needed, as it should be
promoted to the name of the function.



+  

Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Laszlo Ersek
On 01/26/21 20:01, Laszlo Ersek wrote:
> On 01/26/21 07:44, Ankur Arora wrote:

>> +  if (EFI_ERROR(Status)) {
>> +goto Fatal;
>>}
> 
> (13) Without having seen the rest of the patches, I think this error
> check should be nested under the same (PluggedCount > 0) condition; in
> other words, I think it only makes sense to check Status after we
> actually call ProcessHotAddedCpus().

(14) Missing space after "EFI_ERROR".

(I'll not point out further instances of this issue; please review all
the patches with regard to it.)

Thanks
Laszlo



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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Laszlo Ersek
On 01/26/21 20:01, Laszlo Ersek wrote:

> (1b) Similarly, in the parameter comments, "to be plugged" is wrong. I
> suggest copying the parameter descriptions from
> QemuCpuhpCollectApicIds():
> 
>   @param[out] PluggedApicIds   The APIC IDs of the CPUs that have been
>hot-plugged.
> 
>   @param[out] PluggedCount The number of filled-in APIC IDs in
>PluggedApicIds.

(Of course, in this location, the parameters are [in], not [out].)

Thanks
Laszlo



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




Re: [edk2-devel] [PATCH v5 1/9] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-01-26 Thread Laszlo Ersek
On 01/26/21 07:44, Ankur Arora wrote:
> Refactor CpuHotplugMmi() to pull out the CPU hotplug logic into
> PlugCpus(). This is in preparation for supporting CPU hot-unplug.
>
> Cc: Laszlo Ersek 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Igor Mammedov 
> Cc: Boris Ostrovsky 
> Cc: Aaron Young 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
> Signed-off-by: Ankur Arora 
> ---
>  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 208 
> ++---
>  1 file changed, 123 insertions(+), 85 deletions(-)
>
> diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
> b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
> index cfe698ed2b5e..a5052a501e5a 100644
> --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
> +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
> @@ -62,6 +62,124 @@ STATIC UINT32 mPostSmmPenAddress;
>  //
>  STATIC EFI_HANDLE mDispatchHandle;
>
> +/**
> +  CPU Hotplug handler function.
> +
> +  @param[in] PluggedApicIds  List of APIC IDs to be plugged.
> +
> +  @param[in] PluggedCountCount of APIC IDs to be plugged.

(1) These comments are not optimal.

The variable names say "Plugged", meaning that the CPUs have already
been plugged, from the QEMU perspective. The purpose of this function is
to go over those CPUs whose APIC IDs have been collected with events
pending, relocate the SMBASE on each, and then expose each such CPU to
EFI_SMM_CPU_SERVICE_PROTOCOL. For a given CPU, I think the comment on
the EFI_SMM_ADD_PROCESSOR prototype
[UefiCpuPkg/Include/Protocol/SmmCpuService.h] best expresses the goal:
"Notify that a new processor has been added to the system ... The SMM
CPU driver should add the processor to the SMM CPU list".

See also the comment on QemuCpuhpCollectApicIds():

"""
  Collect the APIC IDs of
  - the CPUs that have been hot-plugged,
  - the CPUs that are about to be hot-unplugged.
"""

This closely reflects which agent (firmware vs. QEMU) is driving each
particular operation / direction.

(1a) So please replace the leading comment with:

  Process those CPUs that have been hot-added, according to
  QemuCpuhpCollectApicIds().

  For each such CPU, relocate the SMBASE, and report the CPU to PiSmmCpuDxeSmm
  via EFI_SMM_CPU_SERVICE_PROTOCOL. If a supposedly hot-added CPU is already
  known, skip it silently.

(1b) Similarly, in the parameter comments, "to be plugged" is wrong. I
suggest copying the parameter descriptions from
QemuCpuhpCollectApicIds():

  @param[out] PluggedApicIds   The APIC IDs of the CPUs that have been
   hot-plugged.

  @param[out] PluggedCount The number of filled-in APIC IDs in
   PluggedApicIds.


> +
> +  @retval EFI_SUCCESSSome of the requested APIC IDs were 
> hot-plugged.

(2) This is inexact; on successful return, each one of the collected
CPUs has been relocated and exposed to the SMM CPU driver. (Either in
this particular invocation, or in an earlier invocation, but on success,
there is no entry in PluggedApicIds that is *not known* to the SMM CPU
driver, or whose SMBASE is not relocated.)


> +
> +  @retval EFI_INTERRUPT_PENDING  Fatal error while hot-plugging.

(3) This error code is very uncommon, and it is mostly/only required
from the function -- CpuHotplugMmi() -- that is registered with
gMmst->MmiHandlerRegister(). The meaning of the error code is, "The MMI
source could not be quiesced", which is a situation that can be
identified at the level of CpuHotplugMmi(), but not at the level of
PlugCpus().

Please list the following return values instead:

  @retval EFI_OUT_OF_RESOURCES  Out of APIC ID space in "mCpuHotPlugData".

  @return   Error codes propagated from SmbaseRelocate()
and mMmCpuService->AddProcessor().

(General remark, in addition: please note the difference between
"@retval" and "@return". The latter does not name a particular value;
the set of values is described in natural language instead.)


> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI

(4) There is no need to make this function EFIAPI.


> +PlugCpus(

(5) Space character missing between function name and opening
parenthesis.

Please check every function prototype and function call for this -- one
space char before the opening paren is required, except in the
definition of function-like macros (where the language standard requires
the "(" to be directly adjacent).


(6) According to the discussion above, this function should be called
ProcessHotAddedCpus().

... The best hint for this function name is actually the comment that
sits atop the stretch of code you are extracting, namely:

  //
  // Process hot-added CPUs.
  //


> +  IN APIC_ID  *PluggedApicIds,
> +  IN UINT32   PluggedCount
> +  )
> +{
> +  EFI_STATUS Status;
> +  UINT32 PluggedIdx;
> +  UINT32 NewSlot;
> +
> +  //
> +  // Process hot-added CPUs.

(7) This short introductory comment is no longer needed, as it should be
promoted to the name of 

Re: [edk2-devel] [PATCH v5 0/9] support CPU hot-unplug

2021-01-26 Thread Laszlo Ersek
On 01/26/21 07:44, Ankur Arora wrote:
> Hi,
> 
> This series adds support for CPU hot-unplug with OVMF.
> 
> Please see this in conjunction with the QEMU secureboot hot-unplug v2
> series posted here (now upstreamed):
>   
> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imamm...@redhat.com/
> 
> Patches 1 and 3,
>   ("OvmfPkg/CpuHotplugSmm: refactor hotplug logic")
>   ("OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper")
> are either refactors or add support functions.
> 
>   OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
>   OvmfPkg/CpuHotplugSmm: add CpuEject()
>   OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection
> 
> Patch 2 and 9,
>   ("OvmfPkg/CpuHotplugSmm: collect hot-unplug events")
>   ("OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug")
> handle the QEMU protocol logic for collection of CPU hot-unplug events
> or the protocol negotiation.
> 
> Patch 4,
>   ("OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()")
> adds the MMI logic for CPU hot-unplug handling and informing
> the PiSmmCpuDxeSmm of CPU removal.
> 
> Patches 5 and 6,
>   ("OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA")
>   ("OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state")
> sets up state for doing the CPU ejection as part of hot-unplug.
> 
> Patches 7, and 8,
>   ("OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()")
>   ("OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection")
> add the CPU ejection logic.
> 
> Testing (with QEMU 5.2.50):
>  - Stable with randomized CPU plug/unplug (guest maxcpus=1,8,128)
>  - Synthetic tests with simultaneous multi CPU hot-unplug
>  - Negotiation with/without CPU hotplug enabled
> 
> Also at:
>   github.com/terminus/edk2/ hot-unplug-v5
> 
> Changelog:
> v5:
>   - fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
> add Qemu Cpu Status helper").
> 
> v4:
>   - Gets rid of unnecessary UefiCpuPkg changes
>   URL: 
> https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.ar...@oracle.com/
> 
> v3:
>   - Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
> and OvmfPkg/CpuHotplugSmm
>   - Cleaner split of the hot-unplug code
>   URL: 
> https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.ar...@oracle.com/
> 
> v2:
>   - Do the ejection via SmmCpuFeaturesRendezvousExit()
>   URL: 
> https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.ar...@oracle.com/
> 
> RFC:
>   URL: 
> https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.ar...@oracle.com/
> 
> 
> Please review.
> 
> Thanks
> Ankur
> 
> Ankur Arora (9):
>   OvmfPkg/CpuHotplugSmm: refactor hotplug logic
>   OvmfPkg/CpuHotplugSmm: collect hot-unplug events
>   OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
>   OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
>   OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
>   OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
>   OvmfPkg/CpuHotplugSmm: add CpuEject()
>   OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection
>   OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
> 
>  OvmfPkg/OvmfPkg.dec|  10 +
>  OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf|   1 +
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   3 +
>  OvmfPkg/CpuHotplugSmm/QemuCpuhp.h  |   6 +
>  OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h  |   2 +
>  OvmfPkg/Include/Library/CpuHotEjectData.h  |  32 ++
>  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 450 
> -
>  OvmfPkg/CpuHotplugSmm/QemuCpuhp.c  |  57 ++-
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  68 
>  OvmfPkg/SmmControl2Dxe/SmiFeatures.c   |  25 +-
>  10 files changed, 552 insertions(+), 102 deletions(-)
>  create mode 100644 OvmfPkg/Include/Library/CpuHotEjectData.h
> 

Comparing the maximum line lengths for each file modified, before and
after the series:

- before:

73 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
78 OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
79 OvmfPkg/CpuHotplugSmm/CpuHotplug.c
79 OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
79 OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
79 OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
79 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
79 OvmfPkg/SmmControl2Dxe/SmiFeatures.c
   120 OvmfPkg/OvmfPkg.dec
   120 total

- after:

73 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
78 OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
79 OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
79 OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
79 OvmfPkg/SmmControl2Dxe/SmiFeatures.c
85 OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
90 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
91 OvmfPkg/Include/Library/CpuHotEjectData.h
97 OvmfPkg/CpuHotplugSmm/CpuHotplug.c
   120 OvmfPkg/OvmfPkg.dec
   120 total

Please rework the patches that modify:

85 OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
90 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
91 OvmfPkg/Include/Library/CpuHotEjectData.h
97 

Re: [edk2-devel] [PATCH v5 0/9] support CPU hot-unplug

2021-01-26 Thread Laszlo Ersek
Hi Ankur,

On 01/26/21 07:44, Ankur Arora wrote:
> Hi,
> 
> This series adds support for CPU hot-unplug with OVMF.
> 
> Please see this in conjunction with the QEMU secureboot hot-unplug v2
> series posted here (now upstreamed):
>   
> https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imamm...@redhat.com/
> 
> Patches 1 and 3,
>   ("OvmfPkg/CpuHotplugSmm: refactor hotplug logic")
>   ("OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper")
> are either refactors or add support functions.
> 
>   OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
>   OvmfPkg/CpuHotplugSmm: add CpuEject()
>   OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection
> 
> Patch 2 and 9,
>   ("OvmfPkg/CpuHotplugSmm: collect hot-unplug events")
>   ("OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug")
> handle the QEMU protocol logic for collection of CPU hot-unplug events
> or the protocol negotiation.
> 
> Patch 4,
>   ("OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()")
> adds the MMI logic for CPU hot-unplug handling and informing
> the PiSmmCpuDxeSmm of CPU removal.
> 
> Patches 5 and 6,
>   ("OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA")
>   ("OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state")
> sets up state for doing the CPU ejection as part of hot-unplug.
> 
> Patches 7, and 8,
>   ("OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()")
>   ("OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection")
> add the CPU ejection logic.
> 
> Testing (with QEMU 5.2.50):
>  - Stable with randomized CPU plug/unplug (guest maxcpus=1,8,128)
>  - Synthetic tests with simultaneous multi CPU hot-unplug
>  - Negotiation with/without CPU hotplug enabled
> 
> Also at:
>   github.com/terminus/edk2/ hot-unplug-v5
> 
> Changelog:
> v5:
>   - fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
> add Qemu Cpu Status helper").
> 
> v4:
>   - Gets rid of unnecessary UefiCpuPkg changes
>   URL: 
> https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.ar...@oracle.com/
> 
> v3:
>   - Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
> and OvmfPkg/CpuHotplugSmm
>   - Cleaner split of the hot-unplug code
>   URL: 
> https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.ar...@oracle.com/
> 
> v2:
>   - Do the ejection via SmmCpuFeaturesRendezvousExit()
>   URL: 
> https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.ar...@oracle.com/
> 
> RFC:
>   URL: 
> https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.ar...@oracle.com/
> 
> 
> Please review.
> 
> Thanks
> Ankur
> 
> Ankur Arora (9):
>   OvmfPkg/CpuHotplugSmm: refactor hotplug logic
>   OvmfPkg/CpuHotplugSmm: collect hot-unplug events
>   OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
>   OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
>   OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
>   OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
>   OvmfPkg/CpuHotplugSmm: add CpuEject()
>   OvmfPkg/CpuHotplugSmm: add worker to do CPU ejection
>   OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug
> 
>  OvmfPkg/OvmfPkg.dec|  10 +
>  OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf|   1 +
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   3 +
>  OvmfPkg/CpuHotplugSmm/QemuCpuhp.h  |   6 +
>  OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h  |   2 +
>  OvmfPkg/Include/Library/CpuHotEjectData.h  |  32 ++
>  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 450 
> -
>  OvmfPkg/CpuHotplugSmm/QemuCpuhp.c  |  57 ++-
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  68 
>  OvmfPkg/SmmControl2Dxe/SmiFeatures.c   |  25 +-
>  10 files changed, 552 insertions(+), 102 deletions(-)
>  create mode 100644 OvmfPkg/Include/Library/CpuHotEjectData.h
> 

This series is still mal-formatted; for some reason it has one too many
CRs per line.

Did you try to set up the "base64" or the "8bit"
content-transfer-encoding? The emails reflected through the list are
still "quoted-printable".

Anyway, I managed to apply the patches with some trickery on a local branch.

Thanks
Laszlo



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




Re: [edk2-devel] [edk2-platforms][PATCH V1 00/11] Add SMBIOS tables for SGI/RD platforms

2021-01-26 Thread Thomas Abraham


On Friday, January 15, 2021 11:57 PM, Pranav Madhu wrote:
> SMBIOS provides basic hardware and firmware configuration information
> through table-driven data structure. This patch series adds SMBIOS
> support for Arm's SGI/RD platforms.
> 
> The first patch in this series defines platform-id values for the
> RD-N2 platform library header. The second patch add SgiGetProductId API,
> which is used by the SMBIOS driver to distinguish between the platforms,
> and install the right table. The third patch in this series adds SMBIOS
> driver support that allows for installation of multiple SMBIOS tables.
> And subsequent patches in this series add SMBIOS tables, which are
> mandatory as per Arm serverready SBBR specification.
> 
> This patch should be kept on top of 'Update ACPI revision' patch to
> avoid conflict.
> 
> Pranav Madhu (11):
>   Platform/ARM/SgiPkg: Define RD-N2 platform id values
>   Platform/ARM/SgiPkg: Add GetProductId API for SGI/RD Platforms
>   Platform/ARM/SgiPkg: Add Initial SMBIOS support
>   Platform/ARM/SgiPkg: Add SMBIOS Type1 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type3 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type4 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type7 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type16 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type17 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type19 Table
>   Platform/ARM/SgiPkg: Add SMBIOS Type32 Table

For the series:
Reviewed-by: Thomas Abraham 

[...]


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




Re: [edk2-devel] [PATCH EDK2-non-osi v1 0/1] Silicon/Hisilicon: fix compile issues triggered by VariablePolicy

2021-01-26 Thread Leif Lindholm
Many thanks.

Reviewed-by: Leif Lindholm 
Pushed as 04744d2432ba.

On Tue, Jan 12, 2021 at 15:06:21 +0800, wenyi,xie via groups.io wrote:
> Hi Leif,
> 
> I have pushed my patch to github, here's the link:
> https://github.com/leadsama/edk2-non-osi/tree/compile_issue_fix_v1
> 
> Thanks
> Wenyi
> 
> On 2021/1/11 19:50, Leif Lindholm wrote:
> > Hi Xie,
> > 
> > If I received such a link without having expressly asked for it, I
> > would have deleted it automatically.
> > 
> > Please push, to *any* publicly accessible git repository, a branch
> > containing these patches, and then send me a link to that branch in
> > reply to this email.
> > 
> > Best Regards,
> > 
> > Leif
> > 
> > On Mon, Jan 11, 2021 at 14:36:41 +0800, wenyi,xie via groups.io wrote:
> >> Hi Leif,
> >>
> >> Happy new year.
> >>
> >> For some reason I couldn't push the commit to the github, so I
> >> created the binary files and sent them to a file transfer system,
> >> this system would sent you an email which include the web link to
> >> download binary files, have you ever received the email which
> >> include download link ?(The transfer system only sent email with
> >> downlink to receiver, so I could not see the link and I was not sure
> >> whether you received the email.)
> >>
> >> As the link is only available for two weeks, so I will apply for
> >> another download link again this week.
> >>
> >> Thanks
> >> Wenyi
> >>
> >>
> >> On 2021/1/9 0:50, Leif Lindholm wrote:
> >>> Hi Wenyi,
> >>>
> >>> Apologies for very late response - this arrived just before I left for
> >>> Christmas holidays.
> >>>
> >>> However, could you also include the link to the repository and branch
> >>> I can fetch the commits from?
> >>>
> >>> /
> >>> Leif
> >>>
> >>> On Wed, Dec 16, 2020 at 09:41:06 +0800, Wenyi Xie wrote:
>  Main Changes :
>  Replace binary files to fix compile issue caused by AsciiStrnCpy
>  and UnicodeStrToAsciiStr missing.
> 
>  Wenyi Xie (1):
>    Silicon/Hisilicon: fix compile issues triggered by VariablePolicy
> 
>   Silicon/Hisilicon/Hi1610/Library/Hi1610Serdes/Hi1610SerdesLib.lib | Bin 
>  587188 -> 598188 bytes
>   Silicon/Hisilicon/Hi1616/Library/Hi1616Serdes/Hi1616SerdesLib.lib | Bin 
>  726884 -> 725820 bytes
>   Silicon/Hisilicon/Library/IpmiCmdLib/IpmiCmdLib.lib   | Bin 
>  210280 -> 258210 bytes
>   3 files changed, 0 insertions(+), 0 deletions(-)
> 
>  -- 
>  2.20.1.windows.1
> 
> >>> .
> >>>
> >>
> >>
> >> 
> >>
> >>
> > .
> > 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70778): https://edk2.groups.io/g/devel/message/70778
Mute This Topic: https://groups.io/mt/78991624/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 03/18] StandaloneMmPkg: StandaloneMmCoreHobLib: Extend support for x64 Mm Core

2021-01-26 Thread Yao, Jiewen
Reviewed-by: Jiewen Yao 

From: devel@edk2.groups.io  On Behalf Of Kun Qin
Sent: Thursday, January 21, 2021 9:35 AM
To: devel@edk2.groups.io; Ard Biesheuvel ; Sami Mujawar 
; Yao, Jiewen ; Supreeth Venkatesh 

Subject: Re: [edk2-devel] [PATCH v3 03/18] StandaloneMmPkg: 
StandaloneMmCoreHobLib: Extend support for x64 Mm Core

Hi Jiewen/Ard/Sami/Supreeth,

I updated this patch in v2 to centralize common implementations for certain 
library functions. Do you by any chance have more comments on this patch? Any 
input is appreciated.

Regards,
Kun

From: Kun Qin
Sent: Thursday, January 14, 2021 14:34
To: devel@edk2.groups.io
Cc: Ard Biesheuvel; Sami 
Mujawar; Jiewen Yao; 
Supreeth Venkatesh
Subject: [edk2-devel] [PATCH v3 03/18] StandaloneMmPkg: StandaloneMmCoreHobLib: 
Extend support for x64 Mm Core

This change adds support of x64 version of StandaloneMmCoreHobLib. It
brings in global variable "gHobList" through StandaloneMmCoreEntryPoint,
imports implementation from DxeCoreHobLib.inf to support x64 Mm Core and
moved shared functional plementations into a common file.

Cc: Ard Biesheuvel mailto:ard.biesheu...@arm.com>>
Cc: Sami Mujawar mailto:sami.muja...@arm.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Supreeth Venkatesh 
mailto:supreeth.venkat...@arm.com>>

Signed-off-by: Kun Qin mailto:ku...@outlook.com>>
---

Notes:
v3:
- Pertains gHobList for AARCH64 instance.

v2:
- Moved common function implementations into Common.c [Jiewen]

 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/{ => 
AArch64}/StandaloneMmCoreHobLib.c | 272 --
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/Common.c
   | 291 +++
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/X64/StandaloneMmCoreHobLib.c
   | 298 
 StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf  
   |  11 +-
 4 files changed, 597 insertions(+), 275 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
similarity index 55%
rename from 
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.c
rename to 
StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
index e3d4743b63f2..0ec2d4ad6f6b 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreHobLib/AArch64/StandaloneMmCoreHobLib.c
@@ -21,188 +21,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 VOID *gHobList = NULL;

-/**
-  Returns the pointer to the HOB list.
-
-  This function returns the pointer to first HOB in the list.
-  If the pointer to the HOB list is NULL, then ASSERT().
-
-  @return The pointer to the HOB list.
-
-**/
-VOID *
-EFIAPI
-GetHobList (
-  VOID
-  )
-{
-  ASSERT (gHobList != NULL);
-  return gHobList;
-}
-
-/**
-  Returns the next instance of a HOB type from the starting HOB.
-
-  This function searches the first instance of a HOB type from the starting 
HOB pointer.
-  If there does not exist such HOB type from the starting HOB pointer, it will 
return NULL.
-  In contrast with macro GET_NEXT_HOB(), this function does not skip the 
starting HOB pointer
-  unconditionally: it returns HobStart back if HobStart itself meets the 
requirement;
-  caller is required to use GET_NEXT_HOB() if it wishes to skip current 
HobStart.
-
-  If HobStart is NULL, then ASSERT().
-
-  @param  Type  The HOB type to return.
-  @param  HobStart  The starting HOB pointer to search from.
-
-  @return The next instance of a HOB type from the starting HOB.
-
-**/
-VOID *
-EFIAPI
-GetNextHob (
-  IN UINT16 Type,
-  IN CONST VOID *HobStart
-  )
-{
-  EFI_PEI_HOB_POINTERS  Hob;
-
-  ASSERT (HobStart != NULL);
-
-  Hob.Raw = (UINT8 *) HobStart;
-  //
-  // Parse the HOB list until end of list or matching type is found.
-  //
-  while (!END_OF_HOB_LIST (Hob)) {
-if (Hob.Header->HobType == Type) {
-  return Hob.Raw;
-}
-Hob.Raw = GET_NEXT_HOB (Hob);
-  }
-  return NULL;
-}
-
-/**
-  Returns the first instance of a HOB type among the whole HOB list.
-
-  This function searches the first instance of a HOB type among the whole HOB 
list.
-  If there does not exist such HOB type in the HOB list, it will return NULL.
-
-  If the pointer to the HOB list is NULL, then ASSERT().
-
-  @param  Type  The HOB type to return.
-
-  @return The next instance of a HOB type from the starting HOB.
-
-**/
-VOID *
-EFIAPI
-GetFirstHob (
-  IN UINT16 Type
-  )
-{
-  VOID  *HobList;
-
-  HobList = GetHobList ();
-  return GetNextHob (Type, HobList);
-}
-
-/**
-  Returns the next instance of the matched 

Re: [edk2-devel] [PATCH v3 05/18] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

2021-01-26 Thread Yao, Jiewen
Thanks. Looks good to me.

Reviewed-by: Jiewen Yao 

From: Kun Qin 
Sent: Thursday, January 21, 2021 9:44 AM
To: devel@edk2.groups.io; Yao, Jiewen 
Cc: Ard Biesheuvel ; Sami Mujawar 
; Supreeth Venkatesh 
Subject: RE: [edk2-devel] [PATCH v3 05/18] StandaloneMmPkg: StandaloneMmMemLib: 
Extends support for X64 architecture

Hi Jiewen,

Do you have further concerns about this specific patch? I did created a 
Bugzilla ticket to track the OS memory protection concern here: 3168 - Add 
non-MMRAM memory protection for Standalone MM environment 
(tianocore.org). It 
introduces a new proposal to allow access of DXE/RT regions from MMRAM, but 
requires non trivial change. Please let me know if you hold different opinions 
towards the proposal or this patch.

Thanks,
Kun

From: Kun Qin
Sent: Thursday, January 14, 2021 14:36
To: devel@edk2.groups.io
Cc: Ard Biesheuvel; Sami 
Mujawar; Jiewen Yao; 
Supreeth Venkatesh
Subject: [edk2-devel] [PATCH v3 05/18] StandaloneMmPkg: StandaloneMmMemLib: 
Extends support for X64 architecture

This change extends StandaloneMmMemLib library to support X64
architecture. The implementation is ported from MdePkg/Library/SmmMemLib.

Cc: Ard Biesheuvel mailto:ard.biesheu...@arm.com>>
Cc: Sami Mujawar mailto:sami.muja...@arm.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Supreeth Venkatesh 
mailto:supreeth.venkat...@arm.com>>

Signed-off-by: Kun Qin mailto:ku...@outlook.com>>
---

Notes:
v3:
- Updated destructor description of varibales to pass CI build.

v2:
- Added routine to fix bug of not initializing MmRanges [Jiewen]
- Extends support to x86 instead of x64 only [Hao]

 
StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c 
|  27 
 StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
 |  52 +++
 StandaloneMmPkg/Library/StandaloneMmMemLib/X86StandaloneMmMemLibInternal.c 
 | 155 
 StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf  
 |  13 +-
 4 files changed, 246 insertions(+), 1 deletion(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
index cb7c5e677a6b..4124959e0435 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/AArch64/StandaloneMmMemLibInternal.c
@@ -40,4 +40,31 @@ MmMemLibInternalCalculateMaximumSupportAddress (
   DEBUG ((DEBUG_INFO, "mMmMemLibInternalMaximumSupportAddress = 0x%lx\n", 
mMmMemLibInternalMaximumSupportAddress));
 }

+/**
+  Initialize cached Mmram Ranges from HOB.
+
+  @retval EFI_UNSUPPORTED   The routine is unable to extract MMRAM information.
+  @retval EFI_SUCCESS   MmRanges are populated successfully.
+
+**/
+EFI_STATUS
+MmMemLibInternalPopulateMmramRanges (
+  VOID
+  )
+{
+  // Not implemented for AARCH64.
+  return EFI_SUCCESS;
+}
+
+/**
+  Deinitialize cached Mmram Ranges.
+
+**/
+VOID
+MmMemLibInternalFreeMmramRanges (
+  VOID
+  )
+{
+  // Not implemented for AARCH64.
+}

diff --git a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c 
b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
index b533bd8390cd..2737f95315eb 100644
--- a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
+++ b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.c
@@ -37,6 +37,27 @@ MmMemLibInternalCalculateMaximumSupportAddress (
   VOID
   );

+/**
+  Initialize cached Mmram Ranges from HOB.
+
+  @retval EFI_UNSUPPORTED   The routine is unable to extract MMRAM information.
+  @retval EFI_SUCCESS   MmRanges are populated successfully.
+
+**/
+EFI_STATUS
+MmMemLibInternalPopulateMmramRanges (
+  VOID
+  );
+
+/**
+  Deinitialize cached Mmram Ranges.
+
+**/
+VOID
+MmMemLibInternalFreeMmramRanges (
+  VOID
+  );
+
 /**
   This function check if the buffer is valid per processor architecture and 
not overlap with MMRAM.

@@ -253,11 +274,42 @@ MemLibConstructor (
   IN EFI_MM_SYSTEM_TABLE*MmSystemTable
   )
 {
+  EFI_STATUS Status;

   //
   // Calculate and save maximum support address
   //
   MmMemLibInternalCalculateMaximumSupportAddress ();

+  //
+  // Initialize cached Mmram Ranges from HOB.
+  //
+  Status = MmMemLibInternalPopulateMmramRanges ();
+
+  return Status;
+}
+
+/**
+  Destructor for Mm Mem library.
+
+  @param ImageHandleThe image handle of the process.
+  @param MmSystemTable  The EFI System Table pointer.
+
+  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+MemLibDestructor (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE

Re: [edk2-devel] [PATCH] RedfishPkg/RedfishCrtLib: Add more CRT functions

2021-01-26 Thread Leif Lindholm
On Mon, Jan 25, 2021 at 12:36:53 +0800, Abner Chang wrote:
> Add more functions which were missed in the first time commit,
> that causes the build error with EDK2 Redfish feature driver.
> 
> strerror - We don't support this on edk2 environment.
> strpbrk  - Cloned this function from edk2-LibC
> File operation functions - Not supported on edk2 environment.
> 
> Signed-off-by: Abner Chang 
> 
> Cc: Nickle Wang 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> ---
>  .../RedfishCrtLib/RedfishCrtLib.c | 119 ++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.c 
> b/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.c
> index 0696341bc0..c64f14534c 100644
> --- a/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.c
> +++ b/RedfishPkg/PrivateLibrary/RedfishCrtLib/RedfishCrtLib.c
> @@ -15,6 +15,10 @@
>  #include 
>  
>  int  errno = 0;
> +char errnum_message [] = "We don't support to map errnum to the error 
> message on edk2 Redfish\n";
> +
> +// This is required to keep VC++ happy if you use floating-point
> +int _fltused  = 1;
>  
>  /**
>Determine if a particular character is an alphanumeric character
> @@ -465,6 +469,76 @@ strtod (const char * __restrict nptr, char ** __restrict 
> endptr) {
>  return (double)0;
>  }
>  
> +static UINT8  BitMask[] = {
> +  0x01, 0x02, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80
> +  };
> +
> +#define WHICH8(c) ((unsigned char)(c) >> 3)
> +#define WHICH_BIT(c)  (BitMask[((c) & 0x7)])
> +#define BITMAP64  ((UINT64 *)bitmap)
> +
> +static
> +void
> +BuildBitmap(unsigned char * bitmap, const char *s2, int n)
> +{
> +  unsigned char bit;
> +  int   index;
> +
> +  // Initialize bitmap.  Bit 0 is always 1 which corresponds to '\0'
> +  for (BITMAP64[0] = index = 1; index < n; index++)
> +BITMAP64[index] = 0;

Please use {}.

> +
> +  // Set bits in bitmap corresponding to the characters in s2
> +  for (; *s2 != '\0'; s2++) {
> +index = WHICH8(*s2);
> +bit = WHICH_BIT(*s2);
> +bitmap[index] = bitmap[index] | bit;
> +  }
> +}
> +
> +/** The strpbrk function locates the first occurrence in the string pointed 
> to
> +by s1 of any character from the string pointed to by s2.
> +
> +@return   The strpbrk function returns a pointer to the character, or a
> +  null pointer if no character from s2 occurs in s1.
> +**/
> +char *
> +strpbrk(const char *s1, const char *s2)
> +{
> +  UINT8 bitmap[ (((UCHAR_MAX + 1) / CHAR_BIT) + (CHAR_BIT - 1)) & ~7U];
> +  UINT8 bit;
> +  int index;
> +
> +  BuildBitmap( bitmap, s2, sizeof(bitmap) / sizeof(UINT64));
> +
> +  for( ; *s1 != '\0'; ++s1) {
> +index = WHICH8(*s1);
> +bit = WHICH_BIT(*s1);
> +if( (bitmap[index] & bit) != 0) {
> +  return (char *)s1;
> +}
> +  }
> +  return NULL;
> +}
> +
> +/** The strerror function maps the number in errnum to a message string.
> +Typically, the values for errnum come from errno, but strerror shall map
> +any value of type int to a message.
> +
> +The implementation shall behave as if no library function calls the
> +strerror function.
> +
> +@return   The strerror function returns a pointer to the string, the
> +  contents of which are locale specific.  The array pointed to
> +  shall not be modified by the program, but may be overwritten by
> +  a subsequent call to the strerror function.
> +**/
> +char *
> +strerror(int errnum)
> +{
> +  return errnum_message;
> +}
> +
>  /**
>Allocate and zero-initialize array.
>  **/
> @@ -594,6 +668,51 @@ void qsort (void *base, size_t num, size_t width, int 
> (*compare)(const void *, c
>  int fgetc(FILE * _File){
> return 0;
>  }
> +/**
> +  Open stream file, we don't support file operastion on edk2 JSON library.
> +
> +  @return 0 Unsupported
> +
> +**/
> +FILE *fopen (const char *filename, const char *mode) {
> +  return 0;

NULL.

> +}
> +/**
> +  Read stream from file, we don't support file operastion on edk2 JSON 
> library.
> +
> +  @return 0 Unsupported
> +
> +**/
> +size_t fread (void * ptr, size_t size, size_t count, FILE * stream) {
> +  return 0;
> +}
> +/**
> +  Write stream from file, we don't support file operastion on edk2 JSON 
> library.
> +
> +  @return 0 Unsupported
> +
> +**/
> +size_t fwrite (const void * ptr, size_t size, size_t count, FILE * stream) {
> +  return 0;
> +}
> +/**
> +  Close file, we don't support file operastion on edk2 JSON library.
> +
> +  @return 0 Unsupported
> +
> +**/
> +int fclose (FILE * stream) {
> +  return 0;

0 indicated success. EOF should be returned otherwise.

> +}
> +/**
> +  Write the formatted string to file, we don't support file operastion on 
> edk2 JSON library.
> +
> +  @return 0 Unsupported
> +
> +**/
> +int fprintf (FILE * stream, const char * format, ...) {
> +  return 0;

>From my linux manpage:
"If an output error is encountered, a negative value is returned."

/
Leif

> +}
>  

Re: [edk2-devel] [PATCH] RedfishPkg/JsonLib: Fix build errors

2021-01-26 Thread Leif Lindholm
Hi Abner,

On Mon, Jan 25, 2021 at 12:31:54 +0800, Abner Chang wrote:
> This patch fixes the build errors when build JsonLib with EDK2
> Redfish feature driver.
> 
> - Add JsonLoadString function to load a NULL terminated-string JSON
> - json_string_value() in JsonValueGetAsciiString () is removed
> by accident.
> 
> Signed-off-by: Abner Chang 
> 
> Cc: Leif Lindholm 
> Cc: Nickle Wang 
> Cc: Michael D Kinney 
> ---
>  RedfishPkg/Library/JsonLib/JsonLib.inf |  5 +++--
>  RedfishPkg/Include/Library/JsonLib.h   | 21 ++
>  RedfishPkg/Library/JsonLib/JsonLib.c   | 30 --
>  3 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/RedfishPkg/Library/JsonLib/JsonLib.inf 
> b/RedfishPkg/Library/JsonLib/JsonLib.inf
> index 48b094a78a..9d52a622e1 100644
> --- a/RedfishPkg/Library/JsonLib/JsonLib.inf
> +++ b/RedfishPkg/Library/JsonLib/JsonLib.inf
> @@ -75,12 +75,13 @@
>#   C4244: conversion from type1 to type2, possible loss of data
>#   C4334: 32-bit shift implicitly converted to 64-bit
>#   C4204: nonstandard extension used: non-constant aggregate initializer
> +  #   C4706: assignment within conditional expression
>#
># Define macro HAVE_CONFIG_H to include jansson_private_config.h to build.
># Undefined _WIN32, WIN64, _MSC_VER macros
># On GCC, no error on the unused-function and unused-but-set-variable.
>#
> -  MSFT:*_*_X64_CC_FLAGS = /wd4204 /wd4244 /wd4090 /wd4334 /DHAVE_CONFIG_H=1 
> /U_WIN32 /UWIN64 /U_MSC_VER
> -  MSFT:*_*_IA32_CC_FLAGS = /wd4204 /wd4244 /wd4090 /DHAVE_CONFIG_H=1 
> /U_WIN32 /UWIN64 /U_MSC_VER
> +  MSFT:*_*_X64_CC_FLAGS = /wd4204 /wd4244 /wd4090 /wd4334 /wd4706 
> /DHAVE_CONFIG_H=1 /U_WIN32 /UWIN64 /U_MSC_VER
> +  MSFT:*_*_IA32_CC_FLAGS = /wd4204 /wd4244 /wd4090 /wd4706 /DHAVE_CONFIG_H=1 
> /U_WIN32 /UWIN64 /U_MSC_VER

Urgh, please don't do this.
C4706 is what gives you a warning for accidentally assigning instead
of comparing. It's our last defence against the jeopardy-comparing
hordes that think
  if (NULL == Ptr)
is a sensible way of writing C.

What is the actual problem being encountered?

Beyond that, this will probably be an issue for all architectures, so
why add explicit (identical) workarounds for IA32/X64?

Secondly, I understand these changes were added for a single reason
"fix build failures" - but these are two distinct changes, so should
be two distinct patches.

/
Leif

>GCC:*_*_*_CC_FLAGS = -Wno-unused-function -Wno-unused-but-set-variable
>  
> diff --git a/RedfishPkg/Include/Library/JsonLib.h 
> b/RedfishPkg/Include/Library/JsonLib.h
> index 3c10f67d27..82ca4bad60 100644
> --- a/RedfishPkg/Include/Library/JsonLib.h
> +++ b/RedfishPkg/Include/Library/JsonLib.h
> @@ -664,6 +664,27 @@ JsonDumpString (
>INUINTN   Flags
>);
>  
> +/**
> +  Convert a string to JSON object.
> +  The function is used to convert a NULL terminated UTF8 encoded string to a 
> JSON
> +  value. Only object and array represented strings can be converted 
> successfully,
> +  since they are the only valid root values of a JSON text for UEFI usage.
> +
> +  Real number and number with exponent part are not supportted by UEFI.
> +
> +  Caller needs to cleanup the root value by calling JsonValueFree().
> +
> +  @param[in]   StringThe NULL terminated UTF8 encoded string to 
> convert
> +
> +  @retval  Array JSON value or object JSON value, or NULL when any error 
> occurs.
> +
> +**/
> +EDKII_JSON_VALUE
> +EFIAPI
> +JsonLoadString (
> +  IN   CONST CHAR8*String
> +  );
> +
>  /**
>Load JSON from a buffer.
>  
> diff --git a/RedfishPkg/Library/JsonLib/JsonLib.c 
> b/RedfishPkg/Library/JsonLib/JsonLib.c
> index 34ff381aee..1762c6f5af 100644
> --- a/RedfishPkg/Library/JsonLib/JsonLib.c
> +++ b/RedfishPkg/Library/JsonLib/JsonLib.c
> @@ -430,10 +430,10 @@ JsonValueGetAsciiString (
>INEDKII_JSON_VALUEJson
>)
>  {
> -  CHAR8  *AsciiStr;
> +  CONST CHAR8*AsciiStr;
>UINTN  Index;
>  
> -  AsciiStr = (CHAR8 *)  ((json_t *) Json);
> +  AsciiStr = json_string_value ((json_t *) Json);
>if (AsciiStr == NULL) {
>  return NULL;
>}
> @@ -819,6 +819,32 @@ JsonDumpString (
>  return json_dumps((json_t *)JsonValue, Flags);
>  }
>  
> +/**
> +  Convert a string to JSON object.
> +  The function is used to convert a NULL terminated UTF8 encoded string to a 
> JSON
> +  value. Only object and array represented strings can be converted 
> successfully,
> +  since they are the only valid root values of a JSON text for UEFI usage.
> +
> +  Real number and number with exponent part are not supportted by UEFI.
> +
> +  Caller needs to cleanup the root value by calling JsonValueFree().
> +
> +  @param[in]   StringThe NULL terminated UTF8 encoded string to 
> convert
> +
> +  @retval  Array JSON value or object JSON value, or NULL when any error 
> occurs.
> +
> +**/
> +EDKII_JSON_VALUE
> +EFIAPI
> +JsonLoadString (
> +  INCONST 

Re: [edk2-devel] [PATCH v0] Platform/NXP: Add Dynamic Acpi for layerscape platforms

2021-01-26 Thread Leif Lindholm
Hi Vikas,

On Tue, Jan 19, 2021 at 10:11:43 +0530, Vikas Singh wrote:
> > > > > +/** A helper macro for returning configuration manager objects
> > > > > +*/
> > > > > +#define HANDLE_CM_OBJECT(ObjId, CmObjectId, Object, ObjectCount) 
> > > > >  \
> > > > > +  case ObjId: {  
> > > > >  \
> > > > > +CmObject->ObjectId = CmObjectId; 
> > > > >  \
> > > > > +CmObject->Size = sizeof (Object);
> > > > >  \
> > > > > +CmObject->Data = (VOID*) 
> > > > >  \
> > > > > +CmObject->Count = ObjectCount;   
> > > > >  \
> > > > > +DEBUG (( 
> > > > >  \
> > > > > +  DEBUG_INFO,
> > > > >  \
> > > > > +  #CmObjectId ": Ptr = 0x%p, Size = %d, Count = %d\n",   
> > > > >  \
> > > > > +  CmObject->Data,
> > > > >  \
> > > > > +  CmObject->Size,
> > > > >  \
> > > > > +  CmObject->Count
> > > > >  \
> > > > > +  ));
> > > > >  \
> > > > > +break;   
> > > > >  \
> > > > > +  }
> > > >
> > > > This is code obfuscation. Please don't invent your own programming
> > > > languages. In C, the case, the start bracket, the break and the end
> > > > bracket always go inline.
> > > > The rest would be better as a static helper function than a macro.
> > > >
> > > Leif, changes are in accordance with :
> > > https://raw.githubusercontent.com/tianocore-docs/Docs/master/Specifications/CCS_2_1_Draft.pdf
> >
> > Do you mean 5.5.2.1:
> > Functional macros are generally discouraged.
> > ?
> 
> Leif + Sami, I was referring section 5.7.3.7, since you commented on
> switch case & break statement.
> However keeping section 5.5.2.1 in consideration, I have done few
> changes and shared updated V1 series.
> Could you please have a look on it and revert, if in case you have any 
> concerns.

I have not seen any update?

Please have a look at Sami's updates to the ARM platform code, which I
merged yeaterday.

Best Regards,

Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70773): https://edk2.groups.io/g/devel/message/70773
Mute This Topic: https://groups.io/mt/79415929/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: Align include guards policy

2021-01-26 Thread Leif Lindholm
Hi Liming,

If it was purely a question of style, I would agree that whatever is
70% used should be the norm. But this is not really an issue under our
control.

Macros starting with leading _ are reserved for toolchain use.
Some toolchains, i.e. clang, have dedicated warnings for this.

Whether we want to enforce this lazily (prevent new additions, change
existing ones on rename) or with an all-out search-replace is a
different question.

Either way, this patch sounds like a useful change.
Adding the check for the end of the string would also help improving
code consistency.

/
Leif

On Tue, Jan 26, 2021 at 09:22:06 +0800, gaoliming wrote:
> Pierre:
>   There are some discussion on the syntax of the header file macro. I
> suggest we align the syntax first, then add this checker in ECC tool. 
> 
>   In MdePkg, there are 555 header files. 70% header files use the style
> __BASE_H__ as the file header macro. Others use the style _BTT_H_. 
> 
>   For this case, I would propose to update EDK II C Coding Standards
> Specification to align the code. 
>  
> Thanks
> Liming
> > -邮件原件-
> > 发件人: pierre.gond...@arm.com 
> > 发送时间: 2021年1月25日 23:45
> > 收件人: devel@edk2.groups.io; bob.c.f...@intel.com;
> > gaolim...@byosoft.com.cn
> > 抄送: sami.muja...@arm.com
> > 主题: [PATCH v1 1/1] BaseTools: Align include guards policy
> > 
> > From: Pierre Gondois 
> > 
> > The EDK II C Coding Standards Specification states that:
> > "Names starting with one or two underscores, such as
> > _MACRO_GUARD_FILE_NAME_H_, must not be used. They are
> > reserved for compiler implementation." [1]
> > 
> > The Ecc tool currently checks that the include guard end with
> > a trailing underscore. Thus, the check and the error message
> > should both be modified.
> > 
> > The new check forces having one sole trailing underscore
> > character, as the example in the specification shows:
> > "FILE_NAME_H_" [1]
> > This would allow to have more consistency.
> > 
> > [1] Section 5.3.5 "All include file contents must be protected
> > by a #include guard":
> > https://edk2-docs.gitbook.io/
> > edk-ii-c-coding-standards-specification/
> > 5_source_files/53_include_files
> > 
> > Signed-off-by: Pierre Gondois 
> > ---
> >  BaseTools/Source/Python/Ecc/Check.py| 3 ++-
> >  BaseTools/Source/Python/Ecc/EccToolError.py | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/BaseTools/Source/Python/Ecc/Check.py
> > b/BaseTools/Source/Python/Ecc/Check.py
> > index 6087abfa4d8d..14759d21f5d8 100644
> > --- a/BaseTools/Source/Python/Ecc/Check.py
> > +++ b/BaseTools/Source/Python/Ecc/Check.py
> > @@ -2,6 +2,7 @@
> >  # This file is used to define checkpoints used by ECC tool
> >  #
> >  # Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2021, Arm Limited. All rights reserved.
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> >  from __future__ import absolute_import
> > @@ -1438,7 +1439,7 @@ class Check(object):
> >  RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
> >  for Record in RecordSet:
> >  Name = Record[1].replace('#ifndef', '').strip()
> > -if Name[-1] != '_':
> > +if Name[0] == '_' or Name[-1] != '_' or Name[-2] == '_':
> >  if not
> > EccGlobalData.gException.IsException(ERROR_NAMING_CONVENTION_CHE
> > CK_IFNDEF_STATEMENT, Name):
> > 
> > EccGlobalData.gDb.TblReport.Insert(ERROR_NAMING_CONVENTION_CHECK
> > _IFNDEF_STATEMENT, OtherMsg="The #ifndef name [%s] does not follow the
> > rules" % (Name), BelongsToTable=FileTable, BelongsToItem=Record[0])
> > 
> > diff --git a/BaseTools/Source/Python/Ecc/EccToolError.py
> > b/BaseTools/Source/Python/Ecc/EccToolError.py
> > index 0ff3b42674d4..58d0749477b2 100644
> > --- a/BaseTools/Source/Python/Ecc/EccToolError.py
> > +++ b/BaseTools/Source/Python/Ecc/EccToolError.py
> > @@ -2,6 +2,7 @@
> >  # Standardized Error Handling infrastructures.
> >  #
> >  # Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> > +# Copyright (c) 2021, Arm Limited. All rights reserved.
> >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #
> > 
> > @@ -161,7 +162,7 @@ gEccErrorMessage = {
> >  ERROR_NAMING_CONVENTION_CHECK_ALL : "",
> >  ERROR_NAMING_CONVENTION_CHECK_DEFINE_STATEMENT : "Only
> > capital letters are allowed to be used for #define declarations",
> >  ERROR_NAMING_CONVENTION_CHECK_TYPEDEF_STATEMENT : "Only
> > capital letters are allowed to be used for typedef declarations",
> > -ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT : "The
> > #ifndef at the start of an include file should use both prefix and postfix
> > underscore characters, '_'",
> > +ERROR_NAMING_CONVENTION_CHECK_IFNDEF_STATEMENT : "The
> > #ifndef at the start of an include file should have one postfix
> underscore, and
> > no prefix underscore character '_'",
> >  ERROR_NAMING_CONVENTION_CHECK_PATH_NAME : """Path name
> 

Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Don't increase CpuCount in ApWakeupFunction

2021-01-26 Thread Laszlo Ersek
On 01/26/21 06:50, Ni, Ray wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3179
> 
> When BSP first time wakes all APs, each AP atomically increases
> CpuMpData->CpuCount and CpuMpData->FinishedCount.
> Each AP atomically increases CpuMpData->NumApsExecuting
> in early assembly code and decreases it before it enters to HLT or
> MWAIT state.
> 
> Putting them together, the 3 variables are changed in the following order:
> 1. NumApsExecuting++ // in assembly
> 2. CpuCpunt++
> 4. FinishedCount++
> 3. NumApsExecuting-- // in C
> 
> BSP waits for a certain timeout and then polls NumApsExecuting
> until it drops to zero. It assumes all APs are waken up concurrently
> and NumApsExecuting only drops to zero when all APs have checked in.
> 
> Then it additionally waits for FinishedCount == CpuCount - 1.
> (FinishedCount doesn't include BSP while CpuCount includes BSP.)
> 
> There is no need to additionally wait for
> FinishedCount == CpuCount - 1 because when NumApsExecuting == 0,
> the number of increments of FinishedCount and CpuCount should equal.
> 
> This patch simplifies the code to remove "CpuCount++" in
> ApWakeupFunction() and assigns FinishedCount + 1 to CpuCount after
> WakeUpAP().
> 
> Signed-off-by: Ray Ni 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 16 +---
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 8b1f7f84ba..2568986d8c 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>CPU MP Initialize Library common functions.
>  
> -  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.
>Copyright (c) 2020, AMD Inc. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -485,14 +485,12 @@ CollectProcessorCount (
>CpuMpData->InitFlag = ApInitConfig;
>WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
>CpuMpData->InitFlag = ApInitDone;
> -  ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
>//
> -  // Wait for all APs finished the initialization
> +  // When InitFlag == ApInitConfig, WakeUpAP () guarantees all APs are 
> checked in.
> +  // FinishedCount is the number of check-in APs.
>//
> -  while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
> -CpuPause ();
> -  }
> -
> +  CpuMpData->CpuCount = CpuMpData->FinishedCount + 1;
> +  ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
>  
>//
>// Enable x2APIC mode if
> @@ -751,10 +749,6 @@ ApWakeupFunction (
>CurrentApicMode = GetApicMode ();
>while (TRUE) {
>  if (CpuMpData->InitFlag == ApInitConfig) {
> -  //
> -  // Add CPU number
> -  //
> -  InterlockedIncrement ((UINT32 *) >CpuCount);
>ProcessorNumber = ApIndex;
>//
>// This is first time AP wakeup, get BIST information from AP stack
> 

In ApWakeupFunction(), there is a stretch of code (pre-patch) where
CpuCount has been incremented, but FinishedCount hasn't, yet. In that
part of the AP code, the PAUSE loop in CollectProcessorCount() is
waiting (running on the BSP).

But, said part of the AP code is (more broadly) bracketed by the
NumApsExecuting increment / decrement as well. And NumApsExecuting==0 is
waited-for in WakeUpAP(). So this patch looks OK to me.

I didn't try to verify the patch more thoroughly than described above;
OTOH, on QEMU, the *other* branch in WakeUpAP() is supposed to be active
(i.e. the one where "PcdCpuBootLogicalProcessorNumber" is nonzero).

In that case, TimedWaitForApFinish() will wait until FinishedCount
reaches (PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1) exactly, so
the NumApsExecuting==0 check is not relevant in the first place. IOW, I
think this patch cannot regress behavior on QEMU.

Acked-by: Laszlo Ersek 

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70771): https://edk2.groups.io/g/devel/message/70771
Mute This Topic: https://groups.io/mt/80124850/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 05/22] ArmPkg: Add helper to read the Memory Model Features Register 2

2021-01-26 Thread Philippe Mathieu-Daudé
On 1/14/21 5:36 PM, Rebecca Cran wrote:
> Add helper function to read the MMFR2 register. We will need this to
> determine CCIDX support.
> 
> Signed-off-by: Rebecca Cran 
> Reviewed-by: Leif Lindholm 
> Reviewed-by: Sami Mujawar 
> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h | 11 +++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S |  3 +++
>  2 files changed, 14 insertions(+)

Reviewed-by: Philippe Mathieu-Daude 



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