Date   

[edk2-platforms][PATCH V3 05/11] Platform/Sgi: Add SMBIOS Type3 Table

Pranav Madhu
 

Add the SMBIOS type 3 table (System Enclosure) that includes information
about manufacturer, type, serial number and other information related to
system enclosure.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | =
1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | =
22 +++++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | =
1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclosure.c | 1=
03 ++++++++++++++++++++
4 files changed, 127 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe=
.inf
index f7beb1c66c80..b3c1619ddc66 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -17,6 +17,7 @@
SmbiosPlatformDxe.c
Type0BiosInformation.c
Type1SystemInformation.c
+ Type3SystemEnclosure.c
=20
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index e2437b109899..fc18cfc9f369 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -48,4 +48,26 @@ InstallType1SystemInformation (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
=20
+/**
+ Install SMBIOS System Enclosure Table
+
+ Install the SMBIOS System Enclosure (type 3) table for Arm's Reference=
Design
+ platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed is already in us=
e.
+**/
+EFI_STATUS
+EFIAPI
+InstallType3SystemEnclosure (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
+typedef enum {
+ SMBIOS_HANDLE_ENCLOSURE =3D 0x1000,
+} SMBIOS_REFRENCE_HANDLES;
+
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index b9ce50c82b4c..ac4c6120841f 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -28,6 +28,7 @@ STATIC CONST
ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] =3D {
&InstallType0BiosInformation,
&InstallType1SystemInformation,
+ &InstallType3SystemEnclosure,
};
=20
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnc=
losure.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclo=
sure.c
new file mode 100644
index 000000000000..146852eb4ae6
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclosure.=
c
@@ -0,0 +1,103 @@
+/** @file
+ SMBIOS Type 3 (System enclosure) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 3 (System enclosure) table for Arm Refe=
rence
+ Design platforms. SMBIOS Type 3 table (System Enclosure) includes info=
rmation
+ about manufacturer, type, serial number and other information related =
to
+ system enclosure.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.4
+**/
+
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE3_STRINGS \
+ "ARM LTD\0" /* Manufacturer */ \
+ "Version not set\0" /* Version */ \
+ "Serial not set\0" /* Serial */ \
+ "Asset Tag not set\0" /* Asset Tag */
+
+typedef enum {
+ ManufacturerName =3D 1,
+ Version,
+ SerialNumber,
+ AssetTag
+} TYPE3_STRING_ELEMENTS;
+
+/* SMBIOS Type3 structure */
+#pragma pack(1)
+typedef struct {
+ SMBIOS_TABLE_TYPE3 Base;
+ CHAR8 Strings[sizeof (TYPE3_STRINGS)];
+} ARM_RD_SMBIOS_TYPE3;
+#pragma pack()
+
+/* System information */
+STATIC ARM_RD_SMBIOS_TYPE3 mArmRdSmbiosType3 =3D {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_SYSTEM_ENCLOSURE, // Type 3
+ sizeof (SMBIOS_TABLE_TYPE1), // Length
+ SMBIOS_HANDLE_ENCLOSURE, // Assign an unused handle num=
ber
+ },
+ ManufacturerName, // Manufacturer
+ 2, // Enclosure type unknown
+ Version, // Version
+ SerialNumber, // Serial
+ AssetTag, // Asset Tag
+ ChassisStateSafe, // Boot chassis state
+ ChassisStateSafe, // Power supply state
+ ChassisStateSafe, // Thermal state
+ ChassisSecurityStatusUnknown, // Security Status
+ {0}, // BIOS vendor specific Information
+ },
+ // Text strings (unformatted)
+ TYPE3_STRINGS
+};
+
+/**
+ Install SMBIOS System Enclosure Table
+
+ Install the SMBIOS System Enclosure (type 3) table for Arm's Reference=
Design
+ platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed is already in us=
e.
+**/
+EFI_STATUS
+InstallType3SystemEnclosure (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle =3D ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType3)->Hand=
le;
+
+ /* Install type 3 table */
+ Status =3D Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType3
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type3 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}
--=20
2.17.1


[edk2-platforms][PATCH V3 04/11] Platform/Sgi: Add SMBIOS Type1 Table

Pranav Madhu
 

Add the SMBIOS type 1 table (System Information) that includes
information about manufacturer, product name, version, serial number and
other information related to the system identification.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf |=
1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h |=
19 +++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c |=
1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInformation.c |=
142 ++++++++++++++++++++
4 files changed, 163 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe=
.inf
index 3568380f8404..f7beb1c66c80 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -16,6 +16,7 @@
[Sources]
SmbiosPlatformDxe.c
Type0BiosInformation.c
+ Type1SystemInformation.c
=20
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index a09f89ebe5bc..e2437b109899 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -29,4 +29,23 @@ InstallType0BiosInformation (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
=20
+/**
+ Install SMBIOS System information Table.
+
+ Install the SMBIOS system information (type 1) table for Arm's referen=
ce
+ design platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_NOT_FOUND Unknown product id.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed is already in us=
e.
+**/
+EFI_STATUS
+EFIAPI
+InstallType1SystemInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 20d4dedccb82..b9ce50c82b4c 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -27,6 +27,7 @@ typedef EFI_STATUS (*ARM_RD_SMBIOS_TABLE_INSTALL_FPTR)(=
EFI_SMBIOS_PROTOCOL *);
STATIC CONST
ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] =3D {
&InstallType0BiosInformation,
+ &InstallType1SystemInformation,
};
=20
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInf=
ormation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInf=
ormation.c
new file mode 100644
index 000000000000..367587c07673
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInformatio=
n.c
@@ -0,0 +1,142 @@
+/** @file
+ SMBIOS Type 1 (System information) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 1 (System information) table for Arm's
+ Reference Design platforms. Type 1 table defines attributes of the
+ overall system such as manufacturer, product name, UUID etc.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.2
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SgiPlatform.h"
+
+#define TYPE1_STRINGS \
+ "ARM LTD\0" /* Manufacturer */ \
+ "Version not set\0" /* Version */ \
+ "Serial not set\0" /* Serial number */ \
+ "Not Applicable\0" /* SKU */ \
+ "Not Applicable\0" /* Family */ \
+ "SGI575\0" /* Product Names */ \
+ "RdN1Edge\0" \
+ "RdN1EdgeX2\0" \
+ "RdE1Edge\0" \
+ "RdV1\0" \
+ "RdV1Mc\0" \
+ "RdN2\0"
+
+typedef enum {
+ ManufacturerName =3D 1,
+ Version,
+ SerialNumber,
+ SKU,
+ Family,
+ ProductNameBase
+} TYPE1_STRING_ELEMENTS;
+
+/* SMBIOS Type1 structure */
+#pragma pack(1)
+typedef struct {
+ SMBIOS_TABLE_TYPE1 Base;
+ CHAR8 Strings[sizeof (TYPE1_STRINGS)];
+} ARM_RD_SMBIOS_TYPE1;
+#pragma pack()
+
+STATIC GUID mSmbiosUid[] =3D {
+ /* Sgi575 */
+ {0xdd7cad0a, 0x227c, 0x4ed4, {0x9f, 0x42, 0xa9, 0x8b, 0xd6, 0xa2, 0x42=
, 0x6c}},
+ /* Rd-N1-Edge */
+ {0x80984efe, 0x404a, 0x43e0, {0xad, 0xa4, 0x63, 0xa0, 0xe0, 0xc4, 0x5e=
, 0x60}},
+ /* Rd-N1-Edge-X2 */
+ {0x2cc4f916, 0x267a, 0x4251, {0x95, 0x6e, 0xf0, 0x49, 0x82, 0xbe, 0x94=
, 0x58}},
+ /* Rd-E1-Edge */
+ {0x567f35c4, 0x104f, 0x447b, {0xa0, 0x94, 0x89, 0x2f, 0xbd, 0xb6, 0x5a=
, 0x55}},
+ /* Rd-V1 */
+ {0xc481f0b1, 0x237c, 0x42d7, {0x98, 0xb2, 0xb4, 0xb4, 0x8d, 0xb5, 0x4f=
, 0x50}},
+ /* Rd-V1Mc */
+ {0x1f3a0806, 0x18b5, 0x4eca, {0xad, 0xcd, 0xba, 0x9b, 0x07, 0xb1, 0x0a=
, 0xcf}},
+ /* Rd-N2 */
+ {0xf2cded73, 0x37f9, 0x4ec9, {0xd9, 0xf9, 0x89, 0x9b, 0x74, 0x91, 0x20=
, 0x49}}
+};
+
+/* System information */
+STATIC ARM_RD_SMBIOS_TYPE1 mArmRdSmbiosType1 =3D {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_SYSTEM_INFORMATION, // Type 1
+ sizeof (SMBIOS_TABLE_TYPE1), // Length
+ SMBIOS_HANDLE_PI_RESERVED, // Assign an unused handle num=
ber
+ },
+ ManufacturerName, // Manufacturer
+ ProductNameBase, // Product Name, update dynamically
+ Version, // Version
+ SerialNumber, // Serial
+ {0}, // UUID, Update dymanically
+ 1, // Wakeup type other
+ SKU, // SKU
+ Family, // Family
+ },
+ // Text strings (unformatted)
+ TYPE1_STRINGS
+};
+
+/**
+ Install SMBIOS System information Table.
+
+ Install the SMBIOS system information (type 1) table for Arm's referen=
ce
+ design platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_NOT_FOUND Unknown product id.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed is already in us=
e.
+**/
+EFI_STATUS
+InstallType1SystemInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+ UINT8 ProductId;
+
+ SmbiosHandle =3D ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType1)->Hand=
le;
+ ProductId =3D SgiGetProductId ();
+
+ /* Choose the product name from TYPE1_STRINGS based on the product ID =
*/
+ if (ProductId !=3D UnknownId) {
+ mArmRdSmbiosType1.Base.ProductName =3D
+ ProductNameBase + (ProductId - 1);
+ CopyGuid (&mArmRdSmbiosType1.Base.Uuid,
+ &mSmbiosUid[ProductId - 1]);
+ } else {
+ return EFI_NOT_FOUND;
+ }
+
+ /* Install type 1 table */
+ Status =3D Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType1
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type1 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}
--=20
2.17.1


[edk2-platforms][PATCH V3 03/11] Platform/Sgi: Add Initial SMBIOS support

Pranav Madhu
 

SMBIOS provides basic hardware and firmware configuration information
through table-driven data structure. This patch adds SMBIOS driver
support that allows for installation of multiple SMBIOS types. Also add
SMBIOS Type0 (BIOS Information) table, that include information about
BIOS vendor name, version, SMBIOS version and other information related
to BIOS.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | =
10 ++
Platform/ARM/SgiPkg/SgiPlatform.fdf | =
8 +-
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | =
46 +++++++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | =
32 +++++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | =
98 ++++++++++++++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInformation.c | 1=
35 ++++++++++++++++++++
6 files changed, 328 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPk=
g/SgiPlatform.dsc.inc
index 42e3600d15f4..a0f217f5107c 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
@@ -109,6 +109,10 @@
# ACPI Table Version
gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
=20
+ # SMBIOS entry point version
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0304
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
+
#
# PCIe
#
@@ -247,6 +251,12 @@
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
=20
+ #
+ # SMBIOS/DMI
+ #
+ MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+ Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+
#
# platform driver
#
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/Sg=
iPlatform.fdf
index da23804828e5..e11d943d6efc 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
+++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2018, ARM Limited. All rights reserved.
+# Copyright (c) 2018-2021, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -102,6 +102,12 @@ READ_LOCK_STATUS =3D TRUE
INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
!include $(BOARD_DXE_FV_COMPONENTS)
=20
+ #
+ # SMBIOS/DMI
+ #
+ INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+ INF Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.in=
f
+
# Required by PCI
INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
=20
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe=
.inf
new file mode 100644
index 000000000000..3568380f8404
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -0,0 +1,46 @@
+## @file
+# This driver installs SMBIOS information for RD Platforms
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION =3D 0x0001001A
+ BASE_NAME =3D SmbiosPlatformDxe
+ FILE_GUID =3D 86e0aa8b-4f8d-44a5-a140-1f693d529c7=
6
+ MODULE_TYPE =3D DXE_DRIVER
+ VERSION_STRING =3D 1.0
+ ENTRY_POINT =3D SmbiosTableEntryPoint
+
+[Sources]
+ SmbiosPlatformDxe.c
+ Type0BiosInformation.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+ ArmPlatformLib
+ DebugLib
+ HobLib
+ UefiDriverEntryPoint
+
+[Guids]
+ gEfiGlobalVariableGuid
+ gArmSgiPlatformIdDescriptorGuid
+
+[FixedPcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
+
+[Protocols]
+ gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+
+[Guids]
+
+[Depex]
+ gEfiSmbiosProtocolGuid
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
new file mode 100644
index 000000000000..a09f89ebe5bc
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -0,0 +1,32 @@
+/** @file
+ Declarations required for SMBIOS DXE driver.
+
+ Functions declarations and data type declarations required for SMBIOS =
DXE
+ driver of the Arm Reference Design platforms.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SMBIOS_PLATFORM_DXE_H_
+#define SMBIOS_PLATFORM_DXE_H_
+
+/**
+ Install SMBIOS BIOS information Table.
+
+ Install the SMBIOS BIOS information (type 0) table for Arm's reference=
design
+ platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed is already in us=
e.
+**/
+EFI_STATUS
+EFIAPI
+InstallType0BiosInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
+#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatform=
Dxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
new file mode 100644
index 000000000000..20d4dedccb82
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -0,0 +1,98 @@
+/** @file
+ Install SMBIOS tables for Arm's SGI/RD platforms.
+
+ This file is the driver entry point for installing SMBIOS tables on Ar=
m's
+ Reference Design platforms. For each SMBIOS table installation handler
+ registered, the driver invokes the handler to register the respective =
table.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0
+**/
+
+#include <IndustryStandard/SmBios.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <PiDxe.h>
+#include <Protocol/Smbios.h>
+
+#include "SgiPlatform.h"
+#include "SmbiosPlatformDxe.h"
+
+typedef EFI_STATUS (*ARM_RD_SMBIOS_TABLE_INSTALL_FPTR)(EFI_SMBIOS_PROTOC=
OL *);
+
+STATIC CONST
+ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] =3D {
+ &InstallType0BiosInformation,
+};
+
+/**
+ Driver entry point. Installs SMBIOS information.
+
+ For all the available SMBIOS table installation handlers, invoke each =
of
+ those handlers and let the handlers install the SMBIOS tables. The cou=
nt
+ of handlers that fail to install the SMBIOS tables is maintained for
+ additional logging.
+
+ @param ImageHandle Module's image handle.
+ @param SystemTable Pointer of EFI_SYSTEM_TABLE.
+
+ @retval EFI_SUCCESS All SMBIOS table install handlers invoked.
+ @retval EFI_NOT_FOUND Unsupported platform.
+ @retval Others Failed to invoke SMBIOS table install handlders=
.
+**/
+EFI_STATUS
+EFIAPI
+SmbiosTableEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_PROTOCOL *Smbios =3D NULL;
+ UINT8 CountFail =3D 0;
+ UINT8 Idx;
+
+ /* Install SMBIOS table only for supported product */
+ if (SgiGetProductId () =3D=3D UnknownId) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to install SMBIOS: Unknown product\n"
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ /* Find the SMBIOS protocol */
+ Status =3D gBS->LocateProtocol (
+ &gEfiSmbiosProtocolGuid,
+ NULL,
+ (VOID **)&Smbios
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to install SMBIOS: Unable to locate protocol\n"
+ ));
+ return Status;
+ }
+
+ /* Install the tables listed in mSmbiosTableList */
+ for (Idx =3D 0; Idx < ARRAY_SIZE (mSmbiosTableList); Idx++) {
+ Status =3D (*mSmbiosTableList[Idx]) (Smbios);
+ if (EFI_ERROR (Status)) {
+ CountFail++;
+ }
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "Installed %d of %d available SMBIOS tables\n",
+ ARRAY_SIZE (mSmbiosTableList) - CountFail,
+ ARRAY_SIZE (mSmbiosTableList)
+ ));
+
+ return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInfor=
mation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInforma=
tion.c
new file mode 100644
index 000000000000..f5c1b2366d04
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInformation.=
c
@@ -0,0 +1,135 @@
+/** @file
+ SMBIOS Type 0 (BIOS information) table for ARM RD platforms.
+
+ Install SMBIOS Type 0 (BIOS information) table for Arm's Reference Des=
ign
+ platforms.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.1
+**/
+
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#define TYPE0_STRINGS \
+ "ARM LTD\0" /* Vendor */ \
+ "EDK II\0" /* BiosVersion */ \
+ __DATE__"\0" /* BiosReleaseDate */ \
+ "\0"
+
+typedef enum {
+ VendorName =3D 1,
+ BiosVersion,
+ BiosReleaseDate
+} TYPE0_STRING_ELEMENTS;
+
+/* SMBIOS Type0 structure */
+#pragma pack(1)
+typedef struct {
+ SMBIOS_TABLE_TYPE0 Base;
+ CHAR8 Strings[sizeof (TYPE0_STRINGS)];
+} ARM_RD_SMBIOS_TYPE0;
+#pragma pack()
+
+/* BIOS information */
+STATIC ARM_RD_SMBIOS_TYPE0 mArmRdSmbiosType0 =3D {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_BIOS_INFORMATION, // Type 0
+ sizeof (SMBIOS_TABLE_TYPE0), // Length
+ SMBIOS_HANDLE_PI_RESERVED, // Assign an unused handle numbe=
r
+ },
+ VendorName, // String number of vendor name in TYPE0_STRINGS
+ BiosVersion, // String number of BiosVersion
+ 0, // Bios starting address segment
+ BiosReleaseDate, // String number of BiosReleaseDate
+ 0xFF, // Bios ROM size
+ { // MISC_BIOS_CHARACTERISTICS
+ 0, // Reserved
+ 0, // Unknown
+ 0, // BIOS Characteristics are not supported
+ 0, // ISA not supported
+ 0, // MCA not supported
+ 0, // EISA not supported
+ 1, // PCI supported
+ 0, // PC card (PCMCIA) not supported
+ 1, // Plug and Play is supported
+ 0, // APM not supported
+ 1, // BIOS upgradable
+ 0, // BIOS shadowing is not allowed
+ 0, // VL-VESA not supported
+ 0, // ESCD support is not available
+ 0, // Boot from CD not supported
+ 1, // selectable boot
+ },
+ { // BIOSCharacteristicsExtensionBytes
+ (
+ (1 << 0) | // ACPI Supported
+ (1 << 1) // Legacy USB supported
+ ),
+ (
+ (1 << 3) | // Content distribution enabled
+ (1 << 4) // UEFI spec supported
+ )
+ },
+ 0, // SMBIOS Major Release, updated dynamically
+ 0, // SMBIOS Minor Release, updated dynamically
+ 0xFF, // Embedded Controller Firmware Major Release
+ 0xFF, // Embedded Controller Firmware Minor Release
+ { // EXTENDED_BIOS_ROM_SIZE
+ 64, // Size
+ 0 // Unit MB
+ }
+ },
+ // Text strings (unformatted area)
+ TYPE0_STRINGS
+};
+
+/**
+ Install SMBIOS BIOS information Table.
+
+ Install the SMBIOS BIOS information (type 0) table for Arm's reference=
design
+ platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed is already in us=
e.
+**/
+EFI_STATUS
+InstallType0BiosInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle =3D ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType0)->Hand=
le;
+
+ /* Update firmware revision */
+ mArmRdSmbiosType0.Base.SystemBiosMajorRelease =3D
+ (PcdGet32 (PcdFirmwareRevision) >> 16) & 0xFF;
+ mArmRdSmbiosType0.Base.SystemBiosMinorRelease =3D
+ PcdGet32 (PcdFirmwareRevision) & 0xFF;
+
+ /* Install type 0 table */
+ Status =3D Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType0
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type0 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}
--=20
2.17.1


[edk2-platforms][PATCH V3 02/11] Platform/Sgi: Add GetProductId API for SGI/RD Platforms

Pranav Madhu
 

Add GetProductId API for SGI/RD Platform. The API returns a product id
in integer format based on the platform description data. The product id
is required for other drivers such as SMBIOS.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 30 +++++++
Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 86 +++++++++++++=
++++++-
2 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/Sgi=
Pkg/Include/SgiPlatform.h
index 1c5366878712..4999c9870b49 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -68,4 +68,34 @@ typedef struct {
UINTN MultiChipMode;
} SGI_PLATFORM_DESCRIPTOR;
=20
+// Arm SGI/RD Product IDs
+typedef enum {
+ UnknownId =3D 0,
+ Sgi575,
+ RdN1Edge,
+ RdN1EdgeX2,
+ RdE1Edge,
+ RdV1,
+ RdV1Mc,
+ RdN2
+} ARM_RD_PRODUCT_ID;
+
+// Arm ProductId look-up table
+typedef struct {
+ UINTN ProductId;
+ UINTN PlatformId;
+ UINTN ConfigId;
+ UINTN MultiChipMode;
+} SGI_PRODUCT_ID_LOOKUP;
+
+/**
+ Derermine the product ID.
+
+ Determine the product ID by using the data in the Platform ID Descript=
or HOB
+ to lookup for a matching product ID.
+
+ @retval Zero Failed identify platform.
+ @retval Others ARM_RD_PRODUCT_ID of the identified platform.
+**/
+UINT8 SgiGetProductId (VOID);
#endif // __SGI_PLATFORM_H__
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Plat=
form/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
index 9731d7cccede..f27c949dbc24 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2018, ARM Limited. All rights reserved.
+* Copyright (c) 2018-2021, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -8,9 +8,12 @@
=20
#include <Library/ArmPlatformLib.h>
#include <Library/BaseLib.h>
+#include <Library/HobLib.h>
#include <Ppi/ArmMpCoreInfo.h>
#include <Ppi/SgiPlatformId.h>
=20
+#include "SgiPlatform.h"
+
UINT64 NtFwConfigDtBlob;
STATIC SGI_NT_FW_CONFIG_INFO_PPI mNtFwConfigDtInfoPpi;
=20
@@ -21,6 +24,51 @@ STATIC ARM_CORE_INFO mCoreInfoTable[] =3D {
},
};
=20
+STATIC CONST SGI_PRODUCT_ID_LOOKUP SgiProductIdLookup[] =3D {
+ {
+ Sgi575,
+ SGI575_PART_NUM,
+ SGI575_CONF_NUM,
+ 0
+ },
+ {
+ RdN1Edge,
+ RD_N1E1_EDGE_PART_NUM,
+ RD_N1_EDGE_CONF_ID,
+ 0
+ },
+ {
+ RdN1EdgeX2,
+ RD_N1E1_EDGE_PART_NUM,
+ RD_N1_EDGE_CONF_ID,
+ 1
+ },
+ {
+ RdE1Edge,
+ RD_N1E1_EDGE_PART_NUM,
+ RD_E1_EDGE_CONF_ID,
+ 0
+ },
+ {
+ RdV1,
+ RD_V1_PART_NUM,
+ RD_V1_CONF_ID,
+ 0
+ },
+ {
+ RdV1Mc,
+ RD_V1_PART_NUM,
+ RD_V1_MC_CONF_ID,
+ 1
+ },
+ {
+ RdN2,
+ RD_N2_PART_NUM,
+ RD_N2_CONF_ID,
+ 0
+ }
+};
+
EFI_BOOT_MODE
ArmPlatformGetBootMode (
VOID
@@ -75,3 +123,39 @@ ArmPlatformGetPlatformPpiList (
*PpiListSize =3D sizeof (gPlatformPpiTable);
*PpiList =3D gPlatformPpiTable;
}
+
+/**
+ Derermine the product ID.
+
+ Determine the product ID by using the data in the Platform ID Descript=
or HOB
+ to lookup for a matching product ID.
+
+ @retval Zero Failed identify platform.
+ @retval Others ARM_RD_PRODUCT_ID of the identified platform.
+**/
+UINT8
+SgiGetProductId (
+ VOID
+ )
+{
+ VOID *SystemIdHob;
+ UINT8 Idx;
+ SGI_PLATFORM_DESCRIPTOR *HobData;
+
+ SystemIdHob =3D GetFirstGuidHob (&gArmSgiPlatformIdDescriptorGuid);
+ if (SystemIdHob =3D=3D NULL) {
+ return UnknownId;
+ }
+
+ HobData =3D (SGI_PLATFORM_DESCRIPTOR *)GET_GUID_HOB_DATA (SystemIdHob)=
;
+
+ for (Idx =3D 0; Idx < ARRAY_SIZE (SgiProductIdLookup); Idx++) {
+ if ((HobData->PlatformId =3D=3D SgiProductIdLookup[Idx].PlatformId) =
&&
+ (HobData->ConfigId =3D=3D SgiProductIdLookup[Idx].ConfigId) &&
+ (HobData->MultiChipMode =3D=3D SgiProductIdLookup[Idx].MultiChip=
Mode)) {
+ return SgiProductIdLookup[Idx].ProductId;
+ }
+ }
+
+ return UnknownId;
+}
--=20
2.17.1


[edk2-platforms][PATCH V3 01/11] Platform/Sgi: Define RD-N2 platform id values

Pranav Madhu
 

Add RD-N2 platform identification values including the part number
and configuration number. This information will be used in populating
the SMBIOS tables.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
---
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/Sgi=
Pkg/Include/SgiPlatform.h
index 818879b5f81e..1c5366878712 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2018-2020, ARM Limited. All rights reserved.
+* Copyright (c) 2018-2021, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -39,6 +39,10 @@
#define RD_V1_CONF_ID 0x1
#define RD_V1_MC_CONF_ID 0x2
=20
+// RD-N2 Platform Identification values
+#define RD_N2_PART_NUM 0x7B7
+#define RD_N2_CONF_ID 0x1
+
#define SGI_CONFIG_MASK 0x0F
#define SGI_CONFIG_SHIFT 0x1C
#define SGI_PART_NUM_MASK 0xFFF
--=20
2.17.1


[edk2-platforms][PATCH V3 00/11] Add SMBIOS tables for Arm's Reference Design platforms

Pranav Madhu
 

Changes since V2:
- Addressed comments from Sami
- Picked up Sami's reviewed-by tags.

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

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.

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

Pranav Madhu (11):
Platform/Sgi: Define RD-N2 platform id values
Platform/Sgi: Add GetProductId API for SGI/RD Platforms
Platform/Sgi: Add Initial SMBIOS support
Platform/Sgi: Add SMBIOS Type1 Table
Platform/Sgi: Add SMBIOS Type3 Table
Platform/Sgi: Add SMBIOS Type4 Table
Platform/Sgi: Add SMBIOS Type7 Table
Platform/Sgi: Add SMBIOS Type16 Table
Platform/Sgi: Add SMBIOS Type17 Table
Platform/Sgi: Add SMBIOS Type19 Table
Platform/Sgi: Add SMBIOS Type32 Table

Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 11 +
Platform/ARM/SgiPkg/SgiPlatform.fdf | 8 +-
.../SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 62 ++++
.../SmbiosPlatformDxe/SmbiosPlatformDxe.h | 197 ++++++++++
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 36 +-
.../SmbiosPlatformDxe/SmbiosPlatformDxe.c | 106 ++++++
.../SmbiosPlatformDxe/Type0BiosInformation.c | 135 +++++++
.../Type16PhysicalMemoryArray.c | 106 ++++++
.../SmbiosPlatformDxe/Type17MemoryDevice.c | 298 +++++++++++++++
.../Type19MemoryArrayMappedAddress.c | 97 +++++
.../Type1SystemInformation.c | 142 ++++++++
.../Type32SystemBootInformation.c | 84 +++++
.../SmbiosPlatformDxe/Type3SystemEnclosure.c | 103 ++++++
.../Type4ProcessorInformation.c | 219 +++++++++++
.../SmbiosPlatformDxe/Type7CacheInformation.c | 342 ++++++++++++++++++
.../SgiPkg/Library/PlatformLib/PlatformLib.c | 86 ++++-
16 files changed, 2029 insertions(+), 3 deletions(-)
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosP=
latformDxe.inf
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosP=
latformDxe.h
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosP=
latformDxe.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0Bi=
osInformation.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type16P=
hysicalMemoryArray.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17M=
emoryDevice.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19M=
emoryArrayMappedAddress.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1Sy=
stemInformation.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type32S=
ystemBootInformation.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3Sy=
stemEnclosure.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type4Pr=
ocessorInformation.c
create mode 100644 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type7Ca=
cheInformation.c

--=20
2.17.1


GSoC 2021: audio output protocol

Ethin Probst
 

Greetings everyone!

I've been selected as a student coder for the audio output protocol
project with Ray Ni and Leif Lindholm as my mentors. This is my first
time doing GSoC and working on EDK II so this will be very enjoyable
and a wonderful learning opportunity for me. (It'll also give me
something to do over the summer, which is always a plus.)

Some of you have already communicated with me in the past, both about
this proposal and about UEFI accessibility in general, and I'm glad to
be working with you guys again. I'm primarily on Linux (Arch Linux to
be exact) and I've already got my development environment set up and
can fully compile OVMF. However, though I've read a bit through the
UEFI driver manual, I certainly haven't read all of it, and I'm still
quite unfamiliar with the EDK II code as a whole. So, how can I use
this community bonding period to get to grips with the development
process? What else do I need to learn?

I apologize for not having many questions to ask -- this is my first
time so I'm not really sure. :-) I can't wait to begin working with
all of you this summer!

--
Signed,
Ethin D. Probst


[PATCH v2 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use

Sergei Dmitrouk <sergei@...>
 

`Result` can be used uninitialized in both functions after following
either first or second `goto` statement.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Guomin Jiang <guomin.jiang@...>
Signed-off-by: Sergei Dmitrouk <sergei@...>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c | 1 +
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
index 4009d37d5f91..0b2960f06c4c 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
@@ -82,6 +82,7 @@ RsaPssVerify (
EVP_PKEY_CTX *KeyCtx;
CONST EVP_MD *HashAlg;

+ Result = FALSE;
EvpRsaKey = NULL;
EvpVerifyCtx = NULL;
KeyCtx = NULL;
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
index b66b6f7296ad..ece765f9ae0a 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
@@ -97,6 +97,7 @@ RsaPssSign (
EVP_PKEY_CTX *KeyCtx;
CONST EVP_MD *HashAlg;

+ Result = FALSE;
EvpRsaKey = NULL;
EvpVerifyCtx = NULL;
KeyCtx = NULL;
--
2.17.6


[PATCH v2 2/3] MdeModulePkg/PciBusDxe: Fix possible uninitialized use

Sergei Dmitrouk <sergei@...>
 

If the function gets invalid value for the `ResizableBarOp` parameter
and asserts are disabled, `Bit` can be used uninitialized.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Sergei Dmitrouk <sergei@...>
---

Notes:
v2:
- simplify if-statement to avoid unused branches

MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 6bba28367165..4caac56f1dcd 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -1778,10 +1778,9 @@ PciProgramResizableBar (

if (ResizableBarOp == PciResizableBarMax) {
Bit = HighBitSet64(Capabilities);
- } else if (ResizableBarOp == PciResizableBarMin) {
+ } else {
+ ASSERT (ResizableBarOp == PciResizableBarMin);
Bit = LowBitSet64(Capabilities);
- } else {
- ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp == PciResizableBarMin));
}

ASSERT (Bit >= 0);
--
2.17.6


[PATCH v2 1/3] ShellPkg/HttpDynamicCommand: Fix possible uninitialized use

Sergei Dmitrouk <sergei@...>
 

`Status` can be used uninitialized:

/* Evaluates to FALSE */
if (ShellGetExecutionBreakFlag ()) {
Status = EFI_ABORTED;
break;
}

/* Evaluates to FALSE */
if (!Context->ContentDownloaded && !Context->ResponseToken.Event) {
Status = ...;
ASSERT_EFI_ERROR (Status);
} else {
ResponseMessage.Data.Response = NULL;
}

/* UNINITIALIZED USE */
if (EFI_ERROR (Status)) {
break;
}

Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Sergei Dmitrouk <sergei@...>
---
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
index 3735a4a7e645..7b9b2d238015 100644
--- a/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
@@ -1524,6 +1524,7 @@ GetResponse (
Context->ResponseToken.Message = &ResponseMessage;
Context->ContentLength = 0;
Context->Status = REQ_OK;
+ Status = EFI_SUCCESS;
MsgParser = NULL;
ResponseData.StatusCode = HTTP_STATUS_UNSUPPORTED_STATUS;
ResponseMessage.Data.Response = &ResponseData;
--
2.17.6


[PATCH v1 0/3] Fix possible uninitialized uses

Sergei Dmitrouk <sergei@...>
 

v1:

Compiling for IA32 target with gcc-5.5.0 emits "maybe-uninitialized" warnings.
Compilation command: build -a IA32 -p OvmfPkg/OvmfPkgIa32.dsc -t GCC49

Unlike other cases mentioned in
https://bugzilla.tianocore.org/show_bug.cgi?id=3228
these seem to be actual issues in the code. Read patches for specifics.

v2:

Second patch was simplified.

Sergei Dmitrouk (3):
ShellPkg/HttpDynamicCommand: Fix possible uninitialized use
MdeModulePkg/PciBusDxe: Fix possible uninitialized use
CryptoPkg/BaseCryptLib: Fix possible uninitialized use

CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c | 1 +
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 ++---
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c | 1 +
4 files changed, 5 insertions(+), 3 deletions(-)

--
2.17.6


Re: 回复: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible uninitialized use

Sergei Dmitrouk <sergei@...>
 

Yes, adding `-ffat-lto-objects` makes the warning appear even with GCC 5.5.0.

Regards,
Sergei

On Tue, May 18, 2021 at 08:59:05AM +0800, gaoliming wrote:
Sergei:
Yes. GCC49 is LTO disable GCC tool chain. GCC5 is LTO enable tool chain.
They both work on the different GCC version, such as gcc5, gcc6..

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844 mentions
-ffat-lto-objects option that can trig the warning with LTO option. Do you
try it?

If this option works, we can update GCC5 tool chain definition in
tools_def.txt, then this issue can be detected in CI GCC5 build.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sergei
Dmitrouk
发送时间: 2021年5月15日 21:01
收件人: devel@edk2.groups.io; jiewen.yao@...
抄送: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
<xiaoyux.lu@...>; Jiang, Guomin <guomin.jiang@...>
主题: Re: [edk2-devel] [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
uninitialized use

Hello Jiewen,

I get the error only for GCC49 and not for GCC5 toolchain. CI uses GCC5.

So I compared build commands and this seems to depend on LTO. Adding
`-flto`
impedes compiler's ability to detect such simple issues.

I've found relevant bug report, there is even fix suggestion from last
month:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90844

Regards,
Sergei

On Sat, May 15, 2021 at 12:30:44AM +0000, Yao, Jiewen wrote:
Hi Sergei
Thank you very much for the fix.
Reviewed-by: Jiewen Yao <Jiewen.yao@...>

I am a little surprised why it is not caught before. It is an obvious
logic issue.

Do you think we can do anything on CI, to catch it during pre-check-in
in the
future?
I just feel it is burden to make it post-check-in fix.


Thank you
Yao Jiewen

-----Original Message-----
From: Sergei Dmitrouk <sergei@...>
Sent: Friday, May 14, 2021 8:17 PM
To: devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@...>; Wang, Jian J
<jian.j.wang@...>;
Lu, XiaoyuX <xiaoyux.lu@...>; Jiang, Guomin
<guomin.jiang@...>
Subject: [PATCH v1 3/3] CryptoPkg/BaseCryptLib: Fix possible
uninitialized
use

`Result` can be used uninitialized in both functions after following
either first or second `goto` statement.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Guomin Jiang <guomin.jiang@...>
Signed-off-by: Sergei Dmitrouk <sergei@...>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c | 1 +
CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
index 4009d37d5f91..0b2960f06c4c 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPss.c
@@ -82,6 +82,7 @@ RsaPssVerify (
EVP_PKEY_CTX *KeyCtx;
CONST EVP_MD *HashAlg;

+ Result = FALSE;
EvpRsaKey = NULL;
EvpVerifyCtx = NULL;
KeyCtx = NULL;
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
index b66b6f7296ad..ece765f9ae0a 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptRsaPssSign.c
@@ -97,6 +97,7 @@ RsaPssSign (
EVP_PKEY_CTX *KeyCtx;
CONST EVP_MD *HashAlg;

+ Result = FALSE;
EvpRsaKey = NULL;
EvpVerifyCtx = NULL;
KeyCtx = NULL;
--
2.17.6









Re: [PATCH v2 04/13] MdePkg/Register/Amd: define GHCB macro for Register GPA structure

Laszlo Ersek
 

On 05/17/21 20:25, Erdem Aktas wrote:
I verified that the values align with the GHCB spec publication:
#56421 Revision: 2.00

Just one question: is there any reason why GHCB_* defines are decimal
while the SVM_EXIT_* are all in hexadecimal? Does EDK2 have any
preference?
(I'm unaware of any preference in edk2 -- it's probably best to stick
with the base that the spec itself uses, but even using a different base
is not a huge deal, if the numbers in question are not large.)

Thanks!
Laszlo


Reviewed-by: Erdem Aktas <erdemaktas@...>

-Erdem

On Wed, May 12, 2021 at 4:46 PM Brijesh Singh <brijesh.singh@...> wrote:

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

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

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewed-by: Laszlo Ersek <lersek@...>
Reviewed-by: Liming Gao <gaoliming@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index cdb8f588ccf8..542e4cdf4782 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -53,6 +53,11 @@ typedef union {
UINT64 Features:52;
} GhcbHypervisorFeatures;

+ struct {
+ UINT64 Function:12;
+ UINT64 GuestFrameNumber:52;
+ } GhcbGpaRegister;
+
VOID *Ghcb;

UINT64 GhcbPhysicalAddress;
@@ -62,6 +67,8 @@ typedef union {
#define GHCB_INFO_SEV_INFO_GET 2
#define GHCB_INFO_CPUID_REQUEST 4
#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST 18
+#define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE 19
#define GHCB_HYPERVISOR_FEATURES_REQUEST 128
#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
#define GHCB_INFO_TERMINATE_REQUEST 256
--
2.17.1


Re: [PATCH v2 06/13] MdePkg/Register/Amd: define GHCB macros for SNP AP creation

Laszlo Ersek
 

On 05/17/21 16:17, Tom Lendacky wrote:
On 5/16/21 10:08 PM, Laszlo Ersek wrote:
Patches v2 01-05 look good to me, thanks for the updates. Now on to this
one:

On 05/13/21 01:46, Brijesh Singh wrote:
From: Tom Lendacky <thomas.lendacky@...>

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3D3275&;data=04%7C01%7Cthomas.lendacky%40amd.com%7C55d74e19a5554988e0b608d918e1215c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637568177488739589%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZIy6xEXWmxgVxZm6mrdlroM7OPQajEjUSD8W5z2ohu0%3D&amp;reserved=0
(1) The "3D" seems like a typo in the bug ticket URL. (This crept into
v2 somehow; v1 didn't have it.)


Version 2 of GHCB introduces NAE for creating AP when SEV-SNP is enabled
in the guest VM. See the GHCB specification, Table 5 "List of Supported
Non-Automatic Events" and sections 4.1.9 and 4.3.2, for further details.

While at it, define the VMSA state save area that is required for creating
the AP. The save area format is defined in AMD APM volume 2, Table B-4
(there is a mistake in the table that defines the size of the reserved
area at offset 0xc8 as a dword, when it is actually a word). The format of
the save area segment registers is further defined in AMD APM volume 2,
sections 10 and 15.5.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewed-by: Liming Gao <gaoliming@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Ghcb.h | 76 ++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 029904b1c63a..4d1ee29e0a5e 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -55,6 +55,7 @@
#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
#define SVM_EXIT_SNP_PAGE_STATE_CHANGE 0x80000010ULL
+#define SVM_EXIT_SNP_AP_CREATION 0x80000013ULL
#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL
#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

@@ -83,6 +84,12 @@
#define IOIO_SEG_ES 0
#define IOIO_SEG_DS (BIT11 | BIT10)

+//
+// AP Creation Information
+//
+#define SVM_VMGEXIT_SNP_AP_CREATE_ON_INIT 0
+#define SVM_VMGEXIT_SNP_AP_CREATE 1
+#define SVM_VMGEXIT_SNP_AP_DESTROY 2

typedef PACKED struct {
UINT8 Reserved1[203];
@@ -195,4 +202,73 @@ typedef struct {
SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
} SNP_PAGE_STATE_CHANGE_INFO;

+//
+// SEV-ES save area mapping structures used for SEV-SNP AP Creation.
+// Only the fields required to be set to a non-zero value are defined.
+//
+#define SEV_ES_RESET_CODE_SEGMENT_TYPE 0xA
+#define SEV_ES_RESET_DATA_SEGMENT_TYPE 0x2
+
+#define SEV_ES_RESET_LDT_TYPE 0x2
+#define SEV_ES_RESET_TSS_TYPE 0x3
+
+#pragma pack (1)
+typedef union {
+ struct {
+ UINT16 Type:4;
+ UINT16 Sbit:1;
+ UINT16 Dpl:2;
+ UINT16 Present:1;
+ UINT16 Avl:1;
+ UINT16 Reserved1:1;
+ UINT16 Db:1;
+ UINT16 Granularity:1;
+ } Bits;
+ UINT16 Uint16;
+} SEV_ES_SEGMENT_REGISTER_ATTRIBUTES;
+
+typedef struct {
+ UINT16 Selector;
+ SEV_ES_SEGMENT_REGISTER_ATTRIBUTES Attributes;
+ UINT32 Limit;
+ UINT64 Base;
+} SEV_ES_SEGMENT_REGISTER;
+
I'm not saying anything is incorrect about this, but I *am* going to
rant about the APM.
Yes, the APM is definitely lacking in this area.


It's simply impenetrable. I've been staring at it for ~50 minutes now,
and I still cannot fully connect it to your code.

[1] In sections "4.8.1 Code-Segment Descriptors" and "4.8.2 Data-Segment
Descriptors", the reader is introduced to the "normal" (not SEV-ES, not
virtualized, not SMM) segment descriptors. Why *these* are relevant
*here* is nothing short of mind-boggling, but please bear with me.

[2] In section "10.2.3 SMRAM State-Save Area", "Table 10-1. AMD64
Architecture SMM State-Save Area", the reader is introduced to the
2+2+4+8 segment register representation. The table only lists "Selector,
Attributes, Limit, Base" as fields, and nothing about the actual
contents. Way too little information. I guess this is covered by the
commit message reference "section 10".

[3] In section "15.5 VMRUN Instruction", "15.5.1 Basic Operation",
paragraph "Segment State in the VMCB", we're given a long-winded,
*textual* only -- no diagram! -- and *differential* (relative)
explanation, on top of the two, above-quoted, locations of the spec. I'm
sorry but this is just impossible to follow. Would it have been a
unaffordable to insert a self-contained diagram here, with
self-contained, absolute explanation?

So let me quote:

The segment registers are stored in the VMCB in a format similar to
that for SMM: both base and limit are fully expanded; segment
attributes are stored as 12-bit values formed by the concatenation
of bits 55:52 and 47:40 from the original 64-bit (in-memory) segment
descriptors; the descriptor “P” bit is used to signal NULL segments
(P=0) where permissible and/or relevant.

So, if we apply this to [1] and [2], the "Selector", "Limit" and "Base"
fields of the SMM and VMCB segment register representation are
explained. Further, we get the following for the Attributes field, by
manual reconstruction:

55 54 53 52 47 46 45 44 43 42 41 40

G D L AVL P [DPL] 1 1 C R A Code Segment Descriptor
* * * * * * ignored bits in 64-bit mode

G D/B - AVL P [DPL] 1 0 E W A Data Segment Descriptor
* * * * * * * * * * ignored bits in 64-bit mode

In other words, in the code segment descriptor, D, L, P, DPL, and C
matter. In the data segment descriptor, only P matters.
The APs won't be in 64-bit mode when being started. In reset, they will be
in real mode, so the attributes will be set up for that. Please see Table
4-3 and Table 4-4 for how these will matter.


In particular, what [3] says, quoted above, about the "P" bit, seems
sensible -- "P" is indeed *not* ignored for either segment descriptor
type (code or data).

But then we continure reading [3], and we find (quote again):

The loading of segment attributes from the VMCB (which may have been
overwritten by software) may result in attribute bit values that are
otherwise not allowed. However, only some of the attribute bits are
actually observed by hardware, depending on the segment register in
question:
* CS—D, L, P, and R.
* SS—B, P, E, W, and Code/Data
* DS, ES, FS, GS —D, P, DPL, E, W, and Code/Data.
* LDTR—P, S, and Type (LDT)
* TR—P, S, and Type (32- or 16-bit TSS)

I'm going to ignore SS, LDTR, and TR, but let's check CS and DS.

For CS, the spec says, "D, L, P, and R" are observed by the hardware.
But we've just shown that R is ignored *in general* for code segments in
64-bit mode! In other words, { D, L, P, R } is *not a subset* of { D, L,
P, DPL, C }.

For DS, the spec says here, "D, P, DPL, E, W, and Code/Data" are
observed. I'm going to give "Code/Data" a pass (bit 43 in the original
representation), but the rest is {D, P, DPL, E, W}, which is *not a
subset* of { P }.

Again, from [1], section 4.8.2 specifically, E (expand-down) and W
(writeable) are ignored, DPL is ignored, and D isn't even called D but
"D/B", and it is ignored. So how can the spec say in [3] that the
hardware observes { D, DPL, E, W } when all those are ignored per prior
definition [1]?

- And then I see no reference that SEV-ES uses the same
(circumstantially-defined) segment register format.


So anyway, I'll just accept that I don't understand the spec -- I think
it has inconsistencies. Let's look at the code:

- Bits 43:40 are packed into the "Type" bit-field. That means [1,C,R,A]
for the code segment descriptor, and [0,E,W,A] for data segment descriptors

SEV_ES_RESET_CODE_SEGMENT_TYPE has value 0xA (binary 1010), which maps
to: 1=1, C=0, R=1, A=0. Problem: per [1], the R bit is ignored, so I
have no clue why we set it.
For reset, when we are in real mode, that would correspond to executable /
readable type.


(2) Can you please explain that? Just in this discussion thread, no need
ot change the code or the commit message.
The idea is that we need to support real mode for the AP creation support,
but actually AP creation isn't limited to that. I didn't want to
overly-define the segment register for all the different modes, since it
would only be real mode that would be used by OVMF, but maybe that would
make it eaiser / clearer to understand...


The C ("conforming") bit actually matters. It is documented in section
"4.7.2 Code-Segment Descriptors" (Code-Segment Descriptor—Legacy Mode).
It seems like "non-conforming" is not a problem here, as long as we
don't use multiple privilege levels, which I think we don't, in
firmware-land. OK.

Then, on to SEV_ES_RESET_DATA_SEGMENT_TYPE. Value 0x2 (binary 0010).
Maps to [0,E,W,A], meaning 0=0, E=0, W=1, A=0.

(3) Why is W (writeable) set to 1, when, according to [1], it is ignored
in 64-bit mode?
Same as previous comment, the AP will be starting in real mode.



- "Sbit" seems to stand for bit 44 from the original representation --
constant 1. OK I think.

- "Dpl", "Present" and "Avl" look OK to me.

- Should "Reserved1" really be called that? It seems to map to bit 53 in
the original representation, and the L (long mode) bit actually matters
for the code segment. (It indeed appears undefined / reserved for data
segments.)

Am I *totally* mistaken here and we're not even talking long mode?
:)


- "Db" and "Granularity" look OK.

- SEV_ES_RESET_LDT_TYPE (value 2) matches "64-bit LDT" in "4.8.3 System
Descriptors", "Table 4-6. System-Segment Descriptor Types—Long Mode". OK.

- SEV_ES_RESET_TSS_TYPE (value 3) seems wrong. In Table 4-6, value 3 is
associated with "Reserved (Illegal)". And, according to "12.2.2 TSS
Descriptor", "The TSS descriptor is a system-segment descriptor", which
makes me think that I'm looking at value 3 in the proper table (Table 4-6).
I'll add a reference to table 14-2 under the "Processor Initialization"
section.



(4) Can you please explain why SEV_ES_RESET_TSS_TYPE is 3, and not (say)
0x9 ("Available 64-bit TSS") or 0xB ("Busy 64-bit TSS")?
For processor reset, type 3 represents a busy 16-bit TSS.

So I can work on clarifying the comments around all of the definitions and
indicate that values are defined for processor reset support and that more
definitions would be needed if the segment registers want to be examined /
set in different modes. Thoughts?
My bad -- I feel really dumb now.

The four important macros which I got hung upon are all named
SEV_ES_RESET_*. Keyword being "reset". :/

With the typo (1) fixed:

Reviewed-by: Laszlo Ersek <lersek@...>

Thanks & sorry!
Laszlo



Thanks,
Tom


(Please note that this is only superficial pattern matching from my side
("TSS"); I don't know the first thing about "hardware task management".)


+typedef struct {
+ SEV_ES_SEGMENT_REGISTER Es;
+ SEV_ES_SEGMENT_REGISTER Cs;
+ SEV_ES_SEGMENT_REGISTER Ss;
+ SEV_ES_SEGMENT_REGISTER Ds;
+ SEV_ES_SEGMENT_REGISTER Fs;
+ SEV_ES_SEGMENT_REGISTER Gs;
+ SEV_ES_SEGMENT_REGISTER Gdtr;
+ SEV_ES_SEGMENT_REGISTER Ldtr;
+ SEV_ES_SEGMENT_REGISTER Idtr;
+ SEV_ES_SEGMENT_REGISTER Tr;
+ UINT8 Reserved1[42];
+ UINT8 Vmpl;
+ UINT8 Reserved2[5];
+ UINT64 Efer;
+ UINT8 Reserved3[112];
+ UINT64 Cr4;
+ UINT8 Reserved4[8];
+ UINT64 Cr0;
+ UINT64 Dr7;
+ UINT64 Dr6;
+ UINT64 Rflags;
+ UINT64 Rip;
+ UINT8 Reserved5[232];
+ UINT64 GPat;
+ UINT8 Reserved6[320];
+ UINT64 SevFeatures;
+ UINT8 Reserved7[48];
+ UINT64 XCr0;
+ UINT8 Reserved8[24];
+ UINT32 Mxcsr;
+ UINT16 X87Ftw;
+ UINT8 Reserved9[2];
+ UINT16 X87Fcw;
+} SEV_ES_SAVE_AREA;
+#pragma pack ()
+
#endif
This part looks good to me.

(I spent almost two hours reviewing this patch.)

Thanks!
Laszlo


Re: [PATCH] UefiCpuPkg/MpInitLib: Allocate a separate SEV-ES AP reset stack area

Laszlo Ersek
 

On 05/17/21 00:15, Marvin Häuser wrote:
Am 16.05.2021 um 03:17 schrieb Laszlo Ersek:
On 05/14/21 17:44, Marvin Häuser wrote:
On 14.05.21 17:23, Lendacky, Thomas wrote:
On 5/14/21 10:04 AM, Marvin Häuser wrote:
+      // Check to be sure that the "allocate below" behavior hasn't
changed.
+      // This will also catch a failed allocation, as "-1" is
returned on
+      // failure.
+      //
+      if (CpuMpData->SevEsAPResetStackStart >=
CpuMpData->WakeupBuffer) {
+        DEBUG ((DEBUG_ERROR,
+          "SEV-ES AP reset stack is not below wakeup buffer\n"));
+
+        ASSERT (FALSE);
Should the ASSERT not only catch the broken "allocate below"
behaviour,
i.e. not trigger on failed allocation?
I think it's best to trigger on a failed allocation as well rather than
continuing and allowing a page fault or some other problem to occur.
Well, it should handle the error in a safe way, i.e. the deadloop below.
To not ASSERT on plausible conditions is a common design guideline in
most low-level projects including Linux kernel.

Best regards,
Marvin

Thanks,
Tom

+        CpuDeadLoop ();
"DEBUG + ASSERT(FALSE) + CpuDeadLoop()" is a pattern in edk2.

In RELEASE builds, it will lead to a CpuDeadLoop(). That's the main goal
-- don't continue execution if the condition controlling the whole block
fired.

In DEBUG and NOOPT builds, the pattern will lead to a debug message
(usually at the "error" level), followed by an assertion failure. The
error message of the assertion failure is irrelevant ("FALSE"). The
point of adding ASSERT ahead of CpuDeadLoop() is that the way ASSERT
hangs execution is customizable, via "PcdDebugPropertyMask", unlike
CpuDeadLoop(). In many cases, ASSERT() uses CpuDeadLoop() itself, so the
effect is the same -- the explicit CpuDeadLoop is not reached. In other
configs, ASSERT() can raise a debug exception (CpuBreakpoint()).
I absolutely do not *expect* Tom to change this, it was just a slight
remark (as many places have this anyway). I'll still try to explain why
I made that remark, but for whom it is of no interest, I do not expect
it to be read. I'm fine with the patch as-is myself. Thank you a lot, Tom!



I know it, unfortunately, is a pattern in EDK II - taking this pattern
too far is what caused the 8-revision patch regarding untrusted inputs
we submitted previously. :)

There are many concerns about unconventional ASSERTs, though I must
admit none but one (and that one barely) really apply here, which is why
I have trouble explaining why I believe it should be changed. Here are
some reasons outside the context of this patch:

1) Consistency between DEBUG and RELEASE builds: I think one can justify
to have a breakpoint on a condition that may realistically occur. But a
deadloop can give a wrong impression about how production code works.
E.g. it also is a common pattern in EDK II to ASSERT on memory
allocation failure but *not* have a proper check after, so DEBUG builds
will nicely error or deadloop, while RELEASE goes ahead and causes a CPU
exception or memory corruption depending on the context. Thus,
real-world error handling cannot really be tested. This does not apply
because there *is* a RELEASE deadloop.

2) Static analysis: Some static analysers use ASSERT information for
their own analysis, and try to give hints about unsafe or unreachable
code based on own annotations. This kind of applies, but only when
substituting EDK II ASSERT with properly recognisable ASSERTs (e.g.
__builtin_unreachable).

2) Dynamic analysis: ASSERTs can be useful when fuzzing for example.
Enabled Sanitizers will only catch unsafe behaviour, but maybe you have
some extra code in place to sanity-check the results further. An ASSERT
yields an error dump (usually followed by the worker dying). However, as
allocation failures are perfectly expected, this can cause a dramatic
about of False Positives and testing interruption. This does not apply
because deadloop'd code cannot really be fuzz-tested anyway.

ASSERTs really are designed as unbreakable conditions, i.e. 1)
preconditions 2) invariants 3) postconditions. No allocator in early
kernel-space or lower can really guarantee allocation success, thus it
cannot be a postcondition of any such function. And while it might make
debugging look a bit easier (but you will see from the backtrace anyway
where you halted), it messes with all tools that assume proper usage.

Also, I just realised, you can of course see it from the address value
when debugging, but you cannot see it from the ASSERT or DEBUG message
*which* of the two logical error conditions failed (i.e. broken
allocator or OOM). Changing the ASSERT would fix that. :)
I'm OK if the ASSERT() is dropped; from my perspective it's really just
a small convenience / developer/debugging aid. We still have the debug
message and the explicit deadloop.

Thanks
Laszlo


Re: [edk2-platforms][PATCH v2 6/6] Platform/StandaloneMm: build StandaloneMmRpmb for 32bit architectures

Sami Mujawar
 

Hi Etienn,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 17/05/2021 06:50 AM, Etienne Carriere wrote:
Build PlatformStandaloneMmRpmb for ARM architecture (32bit arm machine).
The generated image targets an execution environment similar to AArch64
StMM secure partition in OP-TEE but in 32bit mode.

GCC flag -fno-stack-protector
added. The stack protection code bring
GOT dependencies we prefer avoid when StMM runs in OP-TEE.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Ilias Apalodimas <ilias.apalodimas@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Etienne Carriere <etienne.carriere@...>
---
Changes since v1:
- Remove useless duplication of ArmSvcLib loading.
- Move BaseStackCheckLib to generic library classes instead of ARM only.
- include MdePkg/MdeLibs.dsc.inc
instead of loading
RegisterFilterLibNull.inf for ARM architecture.
---
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
index cb3f1ddf52..33364deb1e 100644
--- a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
+++ b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
@@ -16,12 +16,14 @@
PLATFORM_VERSION = 1.0
DSC_SPECIFICATION = 0x0001001C
OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME)
- SUPPORTED_ARCHITECTURES = AARCH64
+ SUPPORTED_ARCHITECTURES = ARM|AARCH64
BUILD_TARGETS = DEBUG|RELEASE|NOOPT
SKUID_IDENTIFIER = DEFAULT
FLASH_DEFINITION = Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf
DEFINE DEBUG_MESSAGE = TRUE
+!include MdePkg/MdeLibs.dsc.inc
+
################################################################################
#
# Library Class section - list of all Library Classes needed by this Platform.
@@ -39,6 +41,7 @@
FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
@@ -68,6 +71,9 @@
#
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
+[LibraryClasses.ARM]
+ ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
+
[LibraryClasses.common.MM_STANDALONE]
HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
@@ -160,3 +166,7 @@
[BuildOptions.AARCH64]
GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv8-a+nofp
GCC:*_*_*_CC_FLAGS = -mstrict-align
+
+[BuildOptions.ARM]
+GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv7-a
+GCC:*_*_*_CC_FLAGS = -fno-stack-protector


Re: [edk2-platforms][PATCH v2 5/6] Drivers/OpTee: address cast build warning issue in 32b mode

Sami Mujawar
 

Hi Etienn,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 17/05/2021 06:50 AM, Etienne Carriere wrote:
Use (UINTN) cast to cast physical or virtual address values to the
pointer size before casting from/to a pointer value.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Ilias Apalodimas <ilias.apalodimas@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Etienne Carriere <etienne.carriere@...>
---
No change since v1
---
Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c | 21 +++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c b/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
index 6eb19bed0e..83c2750368 100644
--- a/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
+++ b/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
@@ -305,7 +305,8 @@ OpTeeRpmbFvbRead (
}
}
- Base = (VOID *)Instance->MemBaseAddress + (Lba * Instance->BlockSize) + Offset;
+ Base = (VOID *)(UINTN)Instance->MemBaseAddress + (Lba * Instance->BlockSize) +
+ Offset;
// We could read the data from the RPMB instead of memory
// The 2 copies should already be identical
// Copy from memory image
@@ -387,7 +388,8 @@ OpTeeRpmbFvbWrite (
return Status;
}
}
- Base = (VOID *)Instance->MemBaseAddress + Lba * Instance->BlockSize + Offset;
+ Base = (VOID *)(UINTN)Instance->MemBaseAddress + (Lba * Instance->BlockSize) +
+ Offset;
Status = ReadWriteRpmb (
SP_SVC_RPMB_WRITE,
(UINTN)Buffer,
@@ -477,7 +479,8 @@ OpTeeRpmbFvbErase (
return EFI_INVALID_PARAMETER;
}
NumBytes = NumLba * Instance->BlockSize;
- Base = (VOID *)Instance->MemBaseAddress + Start * Instance->BlockSize;
+ Base = (VOID *)(UINTN)Instance->MemBaseAddress +
+ (Start * Instance->BlockSize);
Buf = AllocatePool (NumLba * Instance->BlockSize);
if (Buf == NULL) {
return EFI_DEVICE_ERROR;
@@ -689,7 +692,7 @@ InitializeFvAndVariableStoreHeaders (
goto Exit;
}
// Install the combined header in memory
- CopyMem ((VOID*)Instance->MemBaseAddress, Headers, HeadersLength);
+ CopyMem ((VOID*)(UINTN)Instance->MemBaseAddress, Headers, HeadersLength);
Exit:
FreePool (Headers);
@@ -747,14 +750,18 @@ FvbInitialize (
// Read the file from disk and copy it to memory
ReadEntireFlash (Instance);
- FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Instance->MemBaseAddress;
+ FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)Instance->MemBaseAddress;
Status = ValidateFvHeader (FwVolHeader);
if (EFI_ERROR (Status)) {
// There is no valid header, so time to install one.
DEBUG ((DEBUG_INFO, "%a: The FVB Header is not valid.\n", __FUNCTION__));
// Reset memory
- SetMem64 ((VOID *)Instance->MemBaseAddress, Instance->NBlocks * Instance->BlockSize, ~0UL);
+ SetMem64 (
+ (VOID *)(UINTN)Instance->MemBaseAddress,
+ Instance->NBlocks * Instance->BlockSize,
+ ~0UL
+ );
DEBUG ((DEBUG_INFO, "%a: Erasing Flash.\n", __FUNCTION__));
Status = ReadWriteRpmb (
SP_SVC_RPMB_WRITE,
@@ -827,7 +834,7 @@ OpTeeRpmbFvbInit (
mInstance.FvbProtocol.Write = OpTeeRpmbFvbWrite;
mInstance.FvbProtocol.Read = OpTeeRpmbFvbRead;
- mInstance.MemBaseAddress = (EFI_PHYSICAL_ADDRESS)Addr;
+ mInstance.MemBaseAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)Addr;
mInstance.Signature = FLASH_SIGNATURE;
mInstance.Initialize = FvbInitialize;
mInstance.BlockSize = EFI_PAGE_SIZE;


Re: [edk2-platforms][PATCH v2 4/6] Drivers/OpTee: Add Aarch32 SVC IDs for 32bit Arm targets

Sami Mujawar
 

Hi Etienn,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 17/05/2021 06:50 AM, Etienne Carriere wrote:
Add SMCCC function IDs for RPMB read/write service on 32bit architectures.
Define generic SP_SVC_RPMB_READ/SP_SVC_RPMB_WRITE IDs for native target
architecture (32b or 64b).

Changes OpTeeRpmbFvb.c to use architecture agnostic macro
ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ for 32b and 64b support.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Ilias Apalodimas <ilias.apalodimas@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Etienne Carriere <etienne.carriere@...>
---
Changes since v1:
- Use _AARCH64 (resp. _AARCH32) suffix instead of _64 (resp. _32) in
the added macros.
---
Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c | 2 +-
Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c b/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
index 5197c95abd..6eb19bed0e 100644
--- a/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
+++ b/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c
@@ -68,7 +68,7 @@ ReadWriteRpmb (
ZeroMem (&SvcArgs, sizeof (SvcArgs));
- SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
+ SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ;
SvcArgs.Arg1 = mStorageId;
SvcArgs.Arg2 = 0;
SvcArgs.Arg3 = SvcAct;
diff --git a/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h b/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h
index c17fc287ef..9c2a4ea6a5 100644
--- a/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h
+++ b/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.h
@@ -13,8 +13,20 @@
contract between OP-TEE and EDK2.
For more details check core/arch/arm/include/kernel/stmm_sp.h in OP-TEE
**/
-#define SP_SVC_RPMB_READ 0xC4000066
-#define SP_SVC_RPMB_WRITE 0xC4000067
+#define SP_SVC_RPMB_READ_AARCH64 0xC4000066
+#define SP_SVC_RPMB_WRITE_AARCH64 0xC4000067
+
+#define SP_SVC_RPMB_READ_AARCH32 0x84000066
+#define SP_SVC_RPMB_WRITE_AARCH32 0x84000067
+
+#ifdef MDE_CPU_AARCH64
+#define SP_SVC_RPMB_READ SP_SVC_RPMB_READ_AARCH64
+#define SP_SVC_RPMB_WRITE SP_SVC_RPMB_WRITE_AARCH64
+#endif
+#ifdef MDE_CPU_ARM
+#define SP_SVC_RPMB_READ SP_SVC_RPMB_READ_AARCH32
+#define SP_SVC_RPMB_WRITE SP_SVC_RPMB_WRITE_AARCH32
+#endif
#define FLASH_SIGNATURE SIGNATURE_32 ('r', 'p', 'm', 'b')
#define INSTANCE_FROM_FVB_THIS(a) CR (a, MEM_INSTANCE, FvbProtocol, \


Re: [edk2-platforms][PATCH v2 3/6] Platform/StandaloneMm: sync with edk2 StandaloneMmCpu path change

Sami Mujawar
 

Hi Etienn,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 17/05/2021 06:50 AM, Etienne Carriere wrote:
Synchronize with edk2 package where StandaloneMmCpu component has moved
from StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
to StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Ilias Apalodimas <ilias.apalodimas@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Sughosh Ganu <sughosh.ganu@...>
Cc: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Etienne Carriere <etienne.carriere@...>
---
Changes since v1:
- split change in 3: this change relates to StandaloneMm package only.
---
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc | 2 +-
Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
index f99a47ebf6..cb3f1ddf52 100644
--- a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
+++ b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.dsc
@@ -133,7 +133,7 @@
#
Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFv.inf
StandaloneMmPkg/Core/StandaloneMmCore.inf
- StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+ StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf {
<LibraryClasses>
NULL|Drivers/OpTee/OpteeRpmbPkg/FixupPcd.inf
diff --git a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf
index e175dc7b2d..c4295a3e63 100644
--- a/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf
+++ b/Platform/StandaloneMm/PlatformStandaloneMmPkg/PlatformStandaloneMmRpmb.fdf
@@ -68,7 +68,8 @@ READ_LOCK_STATUS = TRUE
INF Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFv.inf
INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
- INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+ INF StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
+
################################################################################
#
# Rules are use with the [FV] section's module INF type to define


Re: [edk2-platforms][PATCH v2 2/6] Platform/Socionext/DeveloperBox: sync with edk2 StandaloneMmCpu path change

Sami Mujawar
 

Hi Etienn,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 17/05/2021 06:50 AM, Etienne Carriere wrote:
Synchronize with edk2 package where StandaloneMmCpu component has moved
from StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
to StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Ilias Apalodimas <ilias.apalodimas@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Sughosh Ganu <sughosh.ganu@...>
Cc: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Etienne Carriere <etienne.carriere@...>
---
Changes since v1:
- split change in 3: this change relates to DeveloperBox only.
---
Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc | 2 +-
Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
index e078de4bbb..b5524f87a6 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
@@ -80,7 +80,7 @@
gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x2
}
- StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+ StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf {
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
index 33de03c8e7..89453477c9 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
@@ -111,7 +111,7 @@ READ_LOCK_STATUS = TRUE
INF Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
- INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+ INF StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
################################################################################
#