Re: [edk2-platforms][PATCH V2 10/11] Platform/Sgi: Add SMBIOS Type19 Table


Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

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

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 19 table (Memory Array Mapped Addr) that includes
information about the address mapping for a Physical Memory Array.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 6 ++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c | 92 ++++++++++++++++++++
4 files changed, 100 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 9061c491d461..f81494114188 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -22,6 +22,7 @@
Type7CacheInformation.c
Type16PhysicalMemoryArray.c
Type17MemoryDevice.c
+ Type19MemoryArrayMappedAddress.c
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 5413982e233b..c6dd72cb6b99 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -53,6 +53,12 @@ InstallMemoryDevice (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallMemoryArrayMappedAddress (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
SMBIOS_HANDLE_CLUSTER1,
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 223bf1d114e4..d5d1e6393184 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -33,6 +33,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallCacheInformation,
&InstallPhysicalMemoryArray,
&InstallMemoryDevice,
+ &InstallMemoryArrayMappedAddress,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c
new file mode 100644
index 000000000000..de458ab29e68
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c
@@ -0,0 +1,92 @@
+/** @file
+ SMBIOS Type 19 (Memory Array Mapped Address) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 19 (Memory Array Mapped Address) table for Arm's
+ Reference Design platforms. It includes information about the address mapping
+ for a Physical Memory Array.
+
+ 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.20
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE19_STRINGS \
+ "\0" /* Null string */
+
+/* SMBIOS Type19 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType19 {
+ SMBIOS_TABLE_TYPE19 Base;
+ UINT8 Strings[sizeof (TYPE19_STRINGS)];
+};
+#pragma pack()
+
+/* Memory Array Mapped Address */
+static struct ArmRdSmbiosType19 mArmRdSmbiosType19 = {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS, // Type 19
+ sizeof (SMBIOS_TABLE_TYPE19), // Length
+ SMBIOS_HANDLE_PI_RESERVED, // Assign an unused handle number
+ },
+ 0, // Starting address
+ 0, // Ending address
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Memory array handle
+ 1 // Partition width
+ },
+ // Text strings (unformatted area)
+ TYPE19_STRINGS
+};
+
+/**
+ Install SMBIOS memory array mapped address table
+
+ Install the SMBIOS memory array mapped address (type 19) table for RD
+ 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 in is already in use.
+**/
+EFI_STATUS
+InstallMemoryArrayMappedAddress (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType19)->Handle;
+
+ mArmRdSmbiosType19.Base.StartingAddress = PcdGet64 (PcdSystemMemoryBase) / SIZE_1KB;
+ mArmRdSmbiosType19.Base.EndingAddress = ((PcdGet64 (PcdSystemMemoryBase) +
+ (PcdGet64 (PcdSystemMemorySize) + SIZE_16MB)) / // 16MB Trusted DRAM
+ SIZE_1KB) - 1;
[SAMI] Same comment as for patch 9/11. Can you atleast add some validation to ensure that the encoding would be correct, please?
[/SAMI]
+
+ /* Install type 19 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType19
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type19 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}

Join devel@edk2.groups.io to automatically receive all group messages.