public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table
@ 2019-06-11 12:17 Masahisa Kojima
  2019-06-11 13:00 ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Masahisa Kojima @ 2019-06-11 12:17 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif.lindholm, Masahisa Kojima

This adds the SMBIOS type 17 table support for Developerbox platform.
The SPDs on a I2C bus is only accessible by the SCP, so SCP-firmware
stores SPDs in non-secure SRAM.

This commit also reduces the uefi stack size to allocate space
for SPDs. It requires 2KB, 512bytes * 4 DIMMs.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
 .../Socionext/DeveloperBox/DeveloperBox.dsc   |   3 +
 .../DeveloperBox/DeveloperBox.dsc.inc         |   2 +-
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.c     | 494 +++++++++++++-----
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   2 +
 Silicon/Socionext/SynQuacer/SynQuacer.dec     |   3 +
 5 files changed, 381 insertions(+), 123 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 97fb8c410c..f247370694 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -147,6 +147,9 @@
 
   gSynQuacerTokenSpaceGuid.PcdDramInfoBase|0x2E00FFC0
 
+  # SCP-firmware stored SPD DDR4 data in non-secure SRAM
+  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address|0x2E00F000
+
   #
   # 96boards mezzanine support
   #
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
index a10e48ca07..abb113e858 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
@@ -137,7 +137,7 @@
 
   # non-secure SRAM
   gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
-  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xF000
 
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
 
diff --git a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 6227b77877..da0fd2e6a5 100644
--- a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -10,17 +10,48 @@
 **/
 
 #include <PiDxe.h>
+#include <IndustryStandard/SdramSpdDdr4.h>
 #include <IndustryStandard/SmBios.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/PrintLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Protocol/Smbios.h>
 
 STATIC EFI_SMBIOS_PROTOCOL       *mSmbios;
 
+#define SPD4_MEM_BUS_WIDTH_8BIT    (0x00)
+#define SPD4_MEM_BUS_WIDTH_16BIT   (BIT0)
+#define SPD4_MEM_BUS_WIDTH_32BIT   (BIT1)
+#define SPD4_MEM_BUS_WIDTH_64BIT   (BIT0 | BIT1)
+
+#define SPD4_MEM_DEV_WIDTH_4BIT    (0x00)
+#define SPD4_MEM_DEV_WIDTH_8BIT    (BIT0)
+#define SPD4_MEM_DEV_WIDTH_16BIT   (BIT1)
+#define SPD4_MEM_DEV_WIDTH_32BIT   (BIT0 | BIT1)
+
+#define SPD4_MEM_MODULE_TYPE_RDIMM    0x01
+#define SPD4_MEM_MODULE_TYPE_UDIMM    0x02
+#define SPD4_MEM_MODULE_TYPE_SODIMM   0x03
+
+#define TYPE17_DEVICE_LOCATOR_LEN     (8 + 1)
+#define TYPE17_BANK_LOCATOR_LEN       (20 + 1)
+#define TYPE17_MANUFACTURER_NAME_LEN  (30 + 1)
+#define TYPE17_SERIAL_NUMBER_LEN      (16 + 1)
+#define TYPE17_ASSETTAG_LEN           (16 + 1)
+#define TYPE17_MODULE_PART_NUMBER_LEN (20 + 1)
+
+#define TYPE17_STRINGS_MAX_LEN        (TYPE17_DEVICE_LOCATOR_LEN + \
+                                       TYPE17_BANK_LOCATOR_LEN + \
+                                       TYPE17_MANUFACTURER_NAME_LEN + \
+                                       TYPE17_SERIAL_NUMBER_LEN + \
+                                       TYPE17_ASSETTAG_LEN + \
+                                       TYPE17_MODULE_PART_NUMBER_LEN + \
+                                       1/* null SMBIOS_TABLE_STRING terminator */ )
+
 //
 // Type definition and contents of the default SMBIOS table.
 // This table covers only the minimum structures required by
@@ -69,7 +100,7 @@ typedef struct {
 
 typedef struct {
   SMBIOS_TABLE_TYPE17 Base;
-  CHAR8               Strings[];
+  CHAR8               Strings[TYPE17_STRINGS_MAX_LEN];
 } ARM_TYPE17;
 
 typedef struct {
@@ -94,6 +125,69 @@ enum {
   SMBIOS_HANDLE_MEMORY,
 };
 
+struct JEP106_MANUFACTURER_TABLE {
+  UINT16 ManufacturerId;
+  CHAR8  ManufacturerName[TYPE17_MANUFACTURER_NAME_LEN];
+};
+
+STATIC CONST struct JEP106_MANUFACTURER_TABLE Manufacturer[] = {
+  {0x0010, "NEC\0"},
+  {0x002C, "Micron Technology\0"},
+  {0x003D, "Tektronix\0"},
+  {0x0097, "Texas Instruments\0"},
+  {0x00AD, "SK Hynix\0"},
+  {0x00B3, "IDT\0"},
+  {0x00C1, "Infineon\0"},
+  {0x00CE, "Samsung\0"},
+  {0x00DA, "Winbond Electronic\0"},
+  {0x014F, "Transcend Information\0"},
+  {0x0194, "Smart Modular\0"},
+  {0x0198, "Kingston\0"},
+  {0x02C8, "Agilent Technologies\0"},
+  {0x02FE, "Elpida\0"},
+  {0x030B, "Nanya Technology\0"},
+  {0x0443, "Ramaxel Technology\0"},
+  {0x04B3, "Inphi Corporation\0"},
+  {0x04C8, "Powerchip Semiconductor\0"},
+  {0x0551, "Qimonda\0"},
+  {0x0557, "AENEON\0"},
+  {0x059B, "Crucial Technology\0"},
+  {0xFFFF, "Unknown\0"}
+};
+
+enum SPD4_SDRAM_CAPACITY {
+  SPD4_SDRAM_CAPACITY_256MBIT = 0,
+  SPD4_SDRAM_CAPACITY_512MBIT,
+  SPD4_SDRAM_CAPACITY_1GBIT,
+  SPD4_SDRAM_CAPACITY_2GBIT,
+  SPD4_SDRAM_CAPACITY_4GBIT,
+  SPD4_SDRAM_CAPACITY_8GBIT,
+  SPD4_SDRAM_CAPACITY_16GBIT,
+  SPD4_SDRAM_CAPACITY_32GBIT,
+  SPD4_SDRAM_CAPACITY_12GBIT,
+  SPD4_SDRAM_CAPACITY_24GBIT,
+  SPD4_SDRAM_CAPACITY_INVALID = 0xFF,
+};
+
+struct SPD4_SDRAM_CAPACITY_TABLE {
+  enum SPD4_SDRAM_CAPACITY        Capacity;
+  UINT16                          SizeMbit;
+};
+
+STATIC CONST struct SPD4_SDRAM_CAPACITY_TABLE CapacityTable[]  = {
+  {SPD4_SDRAM_CAPACITY_256MBIT,     256        },
+  {SPD4_SDRAM_CAPACITY_512MBIT,     512        },
+  {SPD4_SDRAM_CAPACITY_1GBIT,       (1 * 1024) },
+  {SPD4_SDRAM_CAPACITY_2GBIT,       (2 * 1024) },
+  {SPD4_SDRAM_CAPACITY_4GBIT,       (4 * 1024) },
+  {SPD4_SDRAM_CAPACITY_8GBIT,       (8 * 1024) },
+  {SPD4_SDRAM_CAPACITY_16GBIT,      (16 * 1024)},
+  {SPD4_SDRAM_CAPACITY_32GBIT,      (32 * 1024)},
+  {SPD4_SDRAM_CAPACITY_12GBIT,      (12 * 1024)},
+  {SPD4_SDRAM_CAPACITY_24GBIT,      (24 * 1024)},
+  {SPD4_SDRAM_CAPACITY_INVALID,     0          },
+};
+
 // BIOS information (section 7.1)
 STATIC CONST ARM_TYPE0 mArmDefaultType0 = {
   {
@@ -394,123 +488,6 @@ STATIC CONST ARM_TYPE16 mArmDefaultType16 = {
   }
 };
 
-// Memory device
-STATIC CONST ARM_TYPE17 mArmDefaultType17_1 = {
-  {
-    { // SMBIOS_STRUCTURE Hdr
-      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
-      sizeof (SMBIOS_TABLE_TYPE17),
-      SMBIOS_HANDLE_PI_RESERVED,
-    },
-    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
-    0xFFFE,                         // no errors
-    72,                             // single DIMM with ECC
-    64,                             // data width of this device (64-bits)
-    0xFFFF,                         // size unknown
-    0x09,                           // DIMM
-    1,                              // part of a set
-    1,                              // device locator
-    0,                              // bank locator
-    MemoryTypeDdr4,                 // DDR4
-    {},                             // type detail
-    0,                              // ? MHz
-    0,                              // manufacturer
-    0,                              // serial
-    0,                              // asset tag
-    0,                              // part number
-    0,                              // rank
-  }, {
-    "DIMM1\0"
-  }
-};
-
-STATIC CONST ARM_TYPE17 mArmDefaultType17_2 = {
-  {
-    { // SMBIOS_STRUCTURE Hdr
-      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
-      sizeof (SMBIOS_TABLE_TYPE17),
-      SMBIOS_HANDLE_PI_RESERVED,
-    },
-    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
-    0xFFFE,                         // no errors
-    72,                             // single DIMM with ECC
-    64,                             // data width of this device (64-bits)
-    0xFFFF,                         // size unknown
-    0x09,                           // DIMM
-    1,                              // part of a set
-    1,                              // device locator
-    0,                              // bank locator
-    MemoryTypeDdr4,                 // DDR4
-    {},                             // type detail
-    0,                              // ? MHz
-    0,                              // manufacturer
-    0,                              // serial
-    0,                              // asset tag
-    0,                              // part number
-    0,                              // rank
-  }, {
-    "DIMM2\0"
-  }
-};
-
-STATIC CONST ARM_TYPE17 mArmDefaultType17_3 = {
-  {
-    { // SMBIOS_STRUCTURE Hdr
-      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
-      sizeof (SMBIOS_TABLE_TYPE17),
-      SMBIOS_HANDLE_PI_RESERVED,
-    },
-    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
-    0xFFFE,                         // no errors
-    72,                             // single DIMM with ECC
-    64,                             // data width of this device (64-bits)
-    0xFFFF,                         // size unknown
-    0x09,                           // DIMM
-    1,                              // part of a set
-    1,                              // device locator
-    0,                              // bank locator
-    MemoryTypeDdr4,                 // DDR4
-    {},                             // type detail
-    0,                              // ? MHz
-    0,                              // manufacturer
-    0,                              // serial
-    0,                              // asset tag
-    0,                              // part number
-    0,                              // rank
-  }, {
-    "DIMM3\0"
-  }
-};
-
-STATIC CONST ARM_TYPE17 mArmDefaultType17_4 = {
-  {
-    { // SMBIOS_STRUCTURE Hdr
-      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
-      sizeof (SMBIOS_TABLE_TYPE17),
-      SMBIOS_HANDLE_PI_RESERVED,
-    },
-    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
-    0xFFFE,                         // no errors
-    72,                             // single DIMM with ECC
-    64,                             // data width of this device (64-bits)
-    0xFFFF,                         // size unknown
-    0x09,                           // DIMM
-    1,                              // part of a set
-    1,                              // device locator
-    0,                              // bank locator
-    MemoryTypeDdr4,                 // DDR4
-    {},                             // type detail
-    0,                              // ? MHz
-    0,                              // manufacturer
-    0,                              // serial
-    0,                              // asset tag
-    0,                              // part number
-    0,                              // rank
-  }, {
-    "DIMM4\0"
-  }
-};
-
 // Memory array mapped address, this structure
 // is overridden by InstallMemoryStructure
 STATIC CONST ARM_TYPE19 mArmDefaultType19 = {
@@ -559,13 +536,275 @@ STATIC SMBIOS_STRUCTURE * CONST FixedTables[] = {
   (SMBIOS_STRUCTURE *)&mArmDefaultType9_1.Base.Hdr,
   (SMBIOS_STRUCTURE *)&mArmDefaultType9_2.Base.Hdr,
   (SMBIOS_STRUCTURE *)&mArmDefaultType16.Base.Hdr,
-  (SMBIOS_STRUCTURE *)&mArmDefaultType17_1.Base.Hdr,
-  (SMBIOS_STRUCTURE *)&mArmDefaultType17_2.Base.Hdr,
-  (SMBIOS_STRUCTURE *)&mArmDefaultType17_3.Base.Hdr,
-  (SMBIOS_STRUCTURE *)&mArmDefaultType17_4.Base.Hdr,
   (SMBIOS_STRUCTURE *)&mArmDefaultType32.Base.Hdr,
 };
 
+STATIC
+UINT16
+GetPrimaryBusWidth (
+  IN UINT8           SpdPrimaryBusWidth
+  )
+{
+  UINT16                    PrimaryBusWidth;
+
+  switch (SpdPrimaryBusWidth) {
+  case SPD4_MEM_BUS_WIDTH_8BIT:
+    PrimaryBusWidth = 8;
+    break;
+  case SPD4_MEM_BUS_WIDTH_16BIT:
+    PrimaryBusWidth = 16;
+    break;
+  case SPD4_MEM_BUS_WIDTH_32BIT:
+    PrimaryBusWidth = 32;
+    break;
+  case SPD4_MEM_BUS_WIDTH_64BIT:
+    PrimaryBusWidth = 64;
+    break;
+  default:
+    PrimaryBusWidth = 0xFFFF;
+    break;
+  }
+
+  return PrimaryBusWidth;
+}
+
+STATIC
+UINT16
+GetSdramDeviceWidth (
+  IN UINT8            SpdSdramDeviceWidth
+  )
+{
+  UINT16                    SdramDeviceWidth;
+
+  switch (SpdSdramDeviceWidth) {
+  case SPD4_MEM_DEV_WIDTH_4BIT:
+    SdramDeviceWidth = 4;
+    break;
+  case SPD4_MEM_DEV_WIDTH_8BIT:
+    SdramDeviceWidth = 8;
+    break;
+  case SPD4_MEM_DEV_WIDTH_16BIT:
+    SdramDeviceWidth = 16;
+    break;
+  case SPD4_MEM_DEV_WIDTH_32BIT:
+    SdramDeviceWidth = 32;
+    break;
+  default:
+    SdramDeviceWidth = 0;
+    break;
+  }
+
+  return SdramDeviceWidth;
+}
+
+STATIC
+UINT16
+CaluculateModuleDramCapacityMB (
+ IN SPD4_BASE_SECTION            *Spd4Base
+ )
+{
+  UINT32                    SdramCapacityMbit = 0;
+  UINT16                    PrimaryBusWidth = 0;
+  UINT8                     SdramDeviceWidth = 0;
+  UINT8                     RankCount = 0;
+  UINT16                    DramSize;
+  UINT32                    i;
+
+  for (i = 0; CapacityTable[i].Capacity != SPD4_SDRAM_CAPACITY_INVALID; i++) {
+    if (Spd4Base->SdramDensityAndBanks.Bits.Density == CapacityTable[i].Capacity) {
+      SdramCapacityMbit = CapacityTable[i].SizeMbit;
+      break;
+    }
+  }
+
+  PrimaryBusWidth = GetPrimaryBusWidth (Spd4Base->ModuleMemoryBusWidth.Bits.PrimaryBusWidth);
+  SdramDeviceWidth = GetSdramDeviceWidth (Spd4Base->ModuleOrganization.Bits.SdramDeviceWidth);
+  RankCount = Spd4Base->ModuleOrganization.Bits.RankCount + 1;
+
+  if ((SdramCapacityMbit == 0) || (PrimaryBusWidth == 0xFFFF) ||
+      (SdramDeviceWidth == 0) || RankCount == 0) {
+    DEBUG ((DEBUG_ERROR, "Calculate DRAM size failed. Cap:%d, BusWidth:%d, "
+                         "DevWidth:%d, Rank:%d\n", SdramCapacityMbit,
+                         PrimaryBusWidth, SdramDeviceWidth, RankCount));
+    return 0xFFFF;
+  }
+
+  //
+  //Total[MB] = SDRAM Capacity[Mb] / 8 * Primary Bus Width /
+  //            SDRAM Width * Logical Ranks per DIMM
+  //
+  DramSize = (((SdramCapacityMbit >> 3) * PrimaryBusWidth) / SdramDeviceWidth) * RankCount;
+
+  return DramSize;
+}
+
+STATIC
+VOID
+GetManufacturerName (
+  IN UINT16           SpdManufacturerId,
+  OUT CHAR8           *ManufacturerStr
+  )
+{
+  UINT16                    ManufacturerId;
+  UINT32                    i;
+
+  ManufacturerId = SwapBytes16 (SpdManufacturerId);
+  ManufacturerId &= 0x7FFF; // ignore odd parity bit
+
+  for (i = 0; Manufacturer[i].ManufacturerId != 0xFFFF; i++) {
+    if (ManufacturerId == Manufacturer[i].ManufacturerId) {
+      AsciiStrCpyS (ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN,
+                    Manufacturer[i].ManufacturerName);
+      return;
+    }
+  }
+
+  AsciiStrCpyS (ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN,
+                Manufacturer[i].ManufacturerName);
+}
+
+STATIC
+EFI_STATUS
+InstallMemoryDeviceStructure (
+  VOID
+  )
+{
+  EFI_SMBIOS_HANDLE         SmbiosHandle;
+  ARM_TYPE17                *Descriptor;
+  SPD_DDR4                  *Spd;
+  UINT8                     Slot;
+
+  CHAR8                     DeviceLocatorStr[TYPE17_DEVICE_LOCATOR_LEN] = {0};
+  CHAR8                     BankLocatorStr[TYPE17_BANK_LOCATOR_LEN] = {0};
+  CHAR8                     ManufacturerStr[TYPE17_MANUFACTURER_NAME_LEN] = {0};
+  CHAR8                     SerialNumberStr[TYPE17_SERIAL_NUMBER_LEN] = {0};
+  CHAR8                     AssetTagStr[] = "Unknown\0";
+  CHAR8                     PartNumberStr[TYPE17_MODULE_PART_NUMBER_LEN] = {0};
+
+  Descriptor = AllocateZeroPool(sizeof (ARM_TYPE17));
+  if (Descriptor == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  Spd = (SPD_DDR4 *) FixedPcdGet32 (PcdSmbiosStoredSpdDDR4Address);
+
+  for (Slot = 0; Slot < 4; Slot++, Spd++) {
+    SetMem (Descriptor, sizeof (ARM_TYPE17), 0);
+    // fill fixed parameters
+    Descriptor->Base.Hdr.Type = EFI_SMBIOS_TYPE_MEMORY_DEVICE;
+    Descriptor->Base.Hdr.Length = sizeof (SMBIOS_TABLE_TYPE17);
+    Descriptor->Base.Hdr.Handle = SMBIOS_HANDLE_PI_RESERVED;
+    Descriptor->Base.MemoryArrayHandle = SMBIOS_HANDLE_MEMORY;
+    Descriptor->Base.MemoryErrorInformationHandle = 0xFFFE;
+    Descriptor->Base.DeviceSet = 1;
+    Descriptor->Base.MemoryType = MemoryTypeDdr4;
+    Descriptor->Base.DeviceLocator = 1;
+    Descriptor->Base.BankLocator = 2;
+    Descriptor->Base.Manufacturer = 3;
+    Descriptor->Base.SerialNumber = 4;
+    Descriptor->Base.AssetTag = 5;
+    Descriptor->Base.PartNumber = 6;
+
+    if (Spd->Base.Description.Data == 0) {
+      // No DIMM inserted, fill the default parameters
+      CHAR8                 DefaultStrings[] = "NO DIMM\0NO DIMM\0NO DIMM\0NO DIMM\0NO DIMM\0";
+      UINT8                 DefaultStringsLen = 40;
+
+      Descriptor->Base.TotalWidth = 0xFFFF;
+      Descriptor->Base.DataWidth = 0xFFFF;
+      Descriptor->Base.Size = 0;
+      Descriptor->Base.FormFactor = 0x09;
+      Descriptor->Base.DeviceSet = 1;
+
+      Descriptor->Base.Attributes = 0;
+      Descriptor->Base.Speed = 0;
+      Descriptor->Base.ConfiguredMemoryClockSpeed = 0;
+      Descriptor->Base.MinimumVoltage = 0;
+      Descriptor->Base.MaximumVoltage = 0;
+      Descriptor->Base.ConfiguredVoltage = 0;
+
+      AsciiSPrint (Descriptor->Strings, TYPE17_STRINGS_MAX_LEN, "DIMM %d\0", (Slot + 1));
+      CopyMem ((Descriptor->Strings + (AsciiStrLen(Descriptor->Strings) + 1)),
+              DefaultStrings, DefaultStringsLen);
+    } else {
+      CHAR8                 *Temp;
+
+      Descriptor->Base.TotalWidth = Descriptor->Base.DataWidth =
+        GetPrimaryBusWidth (Spd->Base.ModuleMemoryBusWidth.Bits.PrimaryBusWidth);
+      if (Spd->Base.ModuleMemoryBusWidth.Bits.BusWidthExtension) {
+        if (Descriptor->Base.TotalWidth != 0xFFFF) {
+          Descriptor->Base.TotalWidth += 8;
+        }
+      }
+
+      Descriptor->Base.Size = CaluculateModuleDramCapacityMB ((SPD4_BASE_SECTION *)Spd);
+
+      switch (Spd->Base.ModuleType.Bits.ModuleType) {
+      case SPD4_MEM_MODULE_TYPE_RDIMM:
+        Descriptor->Base.FormFactor = 0x09;
+        Descriptor->Base.TypeDetail.Registered = 1;
+        Descriptor->Base.TypeDetail.Unbuffered = 0;
+        break;
+      case SPD4_MEM_MODULE_TYPE_UDIMM:
+        Descriptor->Base.FormFactor = 0x09;
+        Descriptor->Base.TypeDetail.Registered = 0;
+        Descriptor->Base.TypeDetail.Unbuffered = 1;
+        break;
+      case SPD4_MEM_MODULE_TYPE_SODIMM:
+        Descriptor->Base.FormFactor = 0x0D;
+        Descriptor->Base.TypeDetail.Registered = 0;
+        Descriptor->Base.TypeDetail.Unbuffered = 1;
+        break;
+      default:
+        Descriptor->Base.FormFactor = 0x01;
+        Descriptor->Base.TypeDetail.Registered = 0;
+        Descriptor->Base.TypeDetail.Unbuffered = 0;
+        break;
+      }
+
+      Descriptor->Base.Attributes = Spd->Base.ModuleOrganization.Bits.RankCount + 1;
+      Descriptor->Base.Speed = 2133;
+      Descriptor->Base.ConfiguredMemoryClockSpeed = 2133;
+      Descriptor->Base.MinimumVoltage = 1200;
+      Descriptor->Base.MaximumVoltage = 1200;
+      Descriptor->Base.ConfiguredVoltage = 1200;
+
+      AsciiSPrint (DeviceLocatorStr, sizeof(DeviceLocatorStr), "DIMM %d\0", (Slot + 1));
+      AsciiSPrint (BankLocatorStr, sizeof(BankLocatorStr), "CHANNEL %d SLOT %d\0", (Slot / 2), (Slot % 2));
+      GetManufacturerName (Spd->ManufactureInfo.ModuleId.IdCode.Data, ManufacturerStr);
+      AsciiSPrint (SerialNumberStr, TYPE17_SERIAL_NUMBER_LEN, "0x%08X\0",
+                   SwapBytes32(Spd->ManufactureInfo.ModuleId.SerialNumber.Data));
+      //
+      // Module part number is not null terminated string in SPD DDR4,
+      // unused bytes are filled with 0x20(space).
+      //
+      CopyMem (PartNumberStr,
+               (CHAR8 *)Spd->ManufactureInfo.ModulePartNumber.ModulePartNumber,
+               TYPE17_MODULE_PART_NUMBER_LEN - 1);
+
+      Temp = Descriptor->Strings;
+      AsciiStrnCpy (Temp, DeviceLocatorStr, TYPE17_DEVICE_LOCATOR_LEN);
+      Temp += (AsciiStrLen (DeviceLocatorStr) + 1);
+      AsciiStrnCpy (Temp, BankLocatorStr, TYPE17_BANK_LOCATOR_LEN);
+      Temp += (AsciiStrLen (BankLocatorStr) + 1);
+      AsciiStrnCpy (Temp, ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN);
+      Temp += (AsciiStrLen (ManufacturerStr) + 1);
+      AsciiStrnCpy (Temp, SerialNumberStr, TYPE17_SERIAL_NUMBER_LEN);
+      Temp += (AsciiStrLen (SerialNumberStr) + 1);
+      AsciiStrnCpy (Temp, AssetTagStr, TYPE17_ASSETTAG_LEN);
+      Temp += (AsciiStrLen (AssetTagStr) + 1);
+      AsciiStrnCpy (Temp, PartNumberStr, TYPE17_MODULE_PART_NUMBER_LEN);
+    }
+
+    SmbiosHandle = Descriptor->Base.Hdr.Handle;
+    mSmbios->Add (mSmbios, NULL, &SmbiosHandle, &Descriptor->Base.Hdr);
+  }
+
+  FreePool (Descriptor);
+
+  return EFI_SUCCESS;
+}
+
 STATIC
 EFI_STATUS
 InstallMemoryStructure (
@@ -587,6 +826,7 @@ InstallMemoryStructure (
   return mSmbios->Add (mSmbios, NULL, &SmbiosHandle, &Descriptor->Base.Hdr);
 }
 
+
 STATIC
 VOID
 InstallAllStructures (
@@ -608,6 +848,16 @@ InstallAllStructures (
     }
   }
 
+  //
+  // SPD_DDR4 data is stored in Non Secure SRAM by SCP-firmware.
+  // Install Type17 record by analyzing SPD_DDR4 information.
+  //
+  Status = InstallMemoryDeviceStructure();
+  if (EFI_ERROR(Status)) {
+      DEBUG ((DEBUG_WARN, "%a: failed to add SMBIOS type 17 table - %r\n",
+        __FUNCTION__, Status));
+  }
+
   for (Hob.Raw = GetHobList ();
        !END_OF_HOB_LIST (Hob);
        Hob.Raw = GET_NEXT_HOB (Hob)) {
diff --git a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index e711cbf6dc..abd4602e34 100644
--- a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -21,6 +21,7 @@
   ArmPkg/ArmPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
+  Silicon/Socionext/SynQuacer/SynQuacer.dec
 
 [LibraryClasses]
   BaseMemoryLib
@@ -34,6 +35,7 @@
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFdSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
+  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address
 
 [Protocols]
   gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
diff --git a/Silicon/Socionext/SynQuacer/SynQuacer.dec b/Silicon/Socionext/SynQuacer/SynQuacer.dec
index e7197e2319..98b574bd32 100644
--- a/Silicon/Socionext/SynQuacer/SynQuacer.dec
+++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec
@@ -43,6 +43,9 @@
   # for capsule update
   gSynQuacerTokenSpaceGuid.PcdLowestSupportedFirmwareVersion|1|UINT32|0x00000009
 
+  # for SMBIOS Type17
+  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address|0|UINT32|0x0000000A
+
 [PcdsPatchableInModule, PcdsDynamic]
   # Enable both RC #0 and RC #1 by default
   gSynQuacerTokenSpaceGuid.PcdPcieEnableMask|0x3|UINT8|0x00000007
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table
  2019-06-11 12:17 [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table Masahisa Kojima
@ 2019-06-11 13:00 ` Ard Biesheuvel
  2019-06-12  5:23   ` Masahisa Kojima
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2019-06-11 13:00 UTC (permalink / raw)
  To: Masahisa Kojima; +Cc: edk2-devel-groups-io, Leif Lindholm

On Tue, 11 Jun 2019 at 14:18, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> This adds the SMBIOS type 17 table support for Developerbox platform.
> The SPDs on a I2C bus is only accessible by the SCP, so SCP-firmware
> stores SPDs in non-secure SRAM.
>
> This commit also reduces the uefi stack size to allocate space
> for SPDs. It requires 2KB, 512bytes * 4 DIMMs.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>

So this patch depends on SCP fw changes that are not yet available,
right? That is not a problem, but I will hold off on merging it until
we have a new SCP firmware build in the tree.

Some comments below.

> ---
>  .../Socionext/DeveloperBox/DeveloperBox.dsc   |   3 +
>  .../DeveloperBox/DeveloperBox.dsc.inc         |   2 +-
>  .../SmbiosPlatformDxe/SmbiosPlatformDxe.c     | 494 +++++++++++++-----
>  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   2 +
>  Silicon/Socionext/SynQuacer/SynQuacer.dec     |   3 +
>  5 files changed, 381 insertions(+), 123 deletions(-)
>
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 97fb8c410c..f247370694 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -147,6 +147,9 @@
>
>    gSynQuacerTokenSpaceGuid.PcdDramInfoBase|0x2E00FFC0
>
> +  # SCP-firmware stored SPD DDR4 data in non-secure SRAM
> +  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address|0x2E00F000
> +

Please drop 'Smbios' from this PCD name - we may want to use the SPD
data in a different way in the future.

>    #
>    # 96boards mezzanine support
>    #
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> index a10e48ca07..abb113e858 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> @@ -137,7 +137,7 @@
>
>    # non-secure SRAM
>    gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
> -  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xF000
>
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
>
> diff --git a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> index 6227b77877..da0fd2e6a5 100644
> --- a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> +++ b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> @@ -10,17 +10,48 @@
>  **/
>
>  #include <PiDxe.h>
> +#include <IndustryStandard/SdramSpdDdr4.h>
>  #include <IndustryStandard/SmBios.h>
>  #include <Library/BaseLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>
> +#include <Library/PrintLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Protocol/Smbios.h>
>
>  STATIC EFI_SMBIOS_PROTOCOL       *mSmbios;
>
> +#define SPD4_MEM_BUS_WIDTH_8BIT    (0x00)
> +#define SPD4_MEM_BUS_WIDTH_16BIT   (BIT0)
> +#define SPD4_MEM_BUS_WIDTH_32BIT   (BIT1)
> +#define SPD4_MEM_BUS_WIDTH_64BIT   (BIT0 | BIT1)
> +
> +#define SPD4_MEM_DEV_WIDTH_4BIT    (0x00)
> +#define SPD4_MEM_DEV_WIDTH_8BIT    (BIT0)
> +#define SPD4_MEM_DEV_WIDTH_16BIT   (BIT1)
> +#define SPD4_MEM_DEV_WIDTH_32BIT   (BIT0 | BIT1)
> +
> +#define SPD4_MEM_MODULE_TYPE_RDIMM    0x01
> +#define SPD4_MEM_MODULE_TYPE_UDIMM    0x02
> +#define SPD4_MEM_MODULE_TYPE_SODIMM   0x03
> +
> +#define TYPE17_DEVICE_LOCATOR_LEN     (8 + 1)
> +#define TYPE17_BANK_LOCATOR_LEN       (20 + 1)
> +#define TYPE17_MANUFACTURER_NAME_LEN  (30 + 1)
> +#define TYPE17_SERIAL_NUMBER_LEN      (16 + 1)
> +#define TYPE17_ASSETTAG_LEN           (16 + 1)
> +#define TYPE17_MODULE_PART_NUMBER_LEN (20 + 1)
> +
> +#define TYPE17_STRINGS_MAX_LEN        (TYPE17_DEVICE_LOCATOR_LEN + \
> +                                       TYPE17_BANK_LOCATOR_LEN + \
> +                                       TYPE17_MANUFACTURER_NAME_LEN + \
> +                                       TYPE17_SERIAL_NUMBER_LEN + \
> +                                       TYPE17_ASSETTAG_LEN + \
> +                                       TYPE17_MODULE_PART_NUMBER_LEN + \
> +                                       1/* null SMBIOS_TABLE_STRING terminator */ )
> +
>  //
>  // Type definition and contents of the default SMBIOS table.
>  // This table covers only the minimum structures required by
> @@ -69,7 +100,7 @@ typedef struct {
>
>  typedef struct {
>    SMBIOS_TABLE_TYPE17 Base;
> -  CHAR8               Strings[];
> +  CHAR8               Strings[TYPE17_STRINGS_MAX_LEN];
>  } ARM_TYPE17;
>
>  typedef struct {
> @@ -94,6 +125,69 @@ enum {
>    SMBIOS_HANDLE_MEMORY,
>  };
>
> +struct JEP106_MANUFACTURER_TABLE {
> +  UINT16 ManufacturerId;
> +  CHAR8  ManufacturerName[TYPE17_MANUFACTURER_NAME_LEN];
> +};
> +
> +STATIC CONST struct JEP106_MANUFACTURER_TABLE Manufacturer[] = {
> +  {0x0010, "NEC\0"},
> +  {0x002C, "Micron Technology\0"},
> +  {0x003D, "Tektronix\0"},
> +  {0x0097, "Texas Instruments\0"},
> +  {0x00AD, "SK Hynix\0"},
> +  {0x00B3, "IDT\0"},
> +  {0x00C1, "Infineon\0"},
> +  {0x00CE, "Samsung\0"},
> +  {0x00DA, "Winbond Electronic\0"},
> +  {0x014F, "Transcend Information\0"},
> +  {0x0194, "Smart Modular\0"},
> +  {0x0198, "Kingston\0"},
> +  {0x02C8, "Agilent Technologies\0"},
> +  {0x02FE, "Elpida\0"},
> +  {0x030B, "Nanya Technology\0"},
> +  {0x0443, "Ramaxel Technology\0"},
> +  {0x04B3, "Inphi Corporation\0"},
> +  {0x04C8, "Powerchip Semiconductor\0"},
> +  {0x0551, "Qimonda\0"},
> +  {0x0557, "AENEON\0"},
> +  {0x059B, "Crucial Technology\0"},
> +  {0xFFFF, "Unknown\0"}
> +};
> +
> +enum SPD4_SDRAM_CAPACITY {
> +  SPD4_SDRAM_CAPACITY_256MBIT = 0,
> +  SPD4_SDRAM_CAPACITY_512MBIT,
> +  SPD4_SDRAM_CAPACITY_1GBIT,
> +  SPD4_SDRAM_CAPACITY_2GBIT,
> +  SPD4_SDRAM_CAPACITY_4GBIT,
> +  SPD4_SDRAM_CAPACITY_8GBIT,
> +  SPD4_SDRAM_CAPACITY_16GBIT,
> +  SPD4_SDRAM_CAPACITY_32GBIT,
> +  SPD4_SDRAM_CAPACITY_12GBIT,
> +  SPD4_SDRAM_CAPACITY_24GBIT,
> +  SPD4_SDRAM_CAPACITY_INVALID = 0xFF,
> +};
> +
> +struct SPD4_SDRAM_CAPACITY_TABLE {
> +  enum SPD4_SDRAM_CAPACITY        Capacity;
> +  UINT16                          SizeMbit;
> +};
> +
> +STATIC CONST struct SPD4_SDRAM_CAPACITY_TABLE CapacityTable[]  = {
> +  {SPD4_SDRAM_CAPACITY_256MBIT,     256        },
> +  {SPD4_SDRAM_CAPACITY_512MBIT,     512        },
> +  {SPD4_SDRAM_CAPACITY_1GBIT,       (1 * 1024) },
> +  {SPD4_SDRAM_CAPACITY_2GBIT,       (2 * 1024) },
> +  {SPD4_SDRAM_CAPACITY_4GBIT,       (4 * 1024) },
> +  {SPD4_SDRAM_CAPACITY_8GBIT,       (8 * 1024) },
> +  {SPD4_SDRAM_CAPACITY_16GBIT,      (16 * 1024)},
> +  {SPD4_SDRAM_CAPACITY_32GBIT,      (32 * 1024)},
> +  {SPD4_SDRAM_CAPACITY_12GBIT,      (12 * 1024)},
> +  {SPD4_SDRAM_CAPACITY_24GBIT,      (24 * 1024)},
> +  {SPD4_SDRAM_CAPACITY_INVALID,     0          },
> +};
> +
>  // BIOS information (section 7.1)
>  STATIC CONST ARM_TYPE0 mArmDefaultType0 = {
>    {
> @@ -394,123 +488,6 @@ STATIC CONST ARM_TYPE16 mArmDefaultType16 = {
>    }
>  };
>
> -// Memory device
> -STATIC CONST ARM_TYPE17 mArmDefaultType17_1 = {
> -  {
> -    { // SMBIOS_STRUCTURE Hdr
> -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> -      sizeof (SMBIOS_TABLE_TYPE17),
> -      SMBIOS_HANDLE_PI_RESERVED,
> -    },
> -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> -    0xFFFE,                         // no errors
> -    72,                             // single DIMM with ECC
> -    64,                             // data width of this device (64-bits)
> -    0xFFFF,                         // size unknown
> -    0x09,                           // DIMM
> -    1,                              // part of a set
> -    1,                              // device locator
> -    0,                              // bank locator
> -    MemoryTypeDdr4,                 // DDR4
> -    {},                             // type detail
> -    0,                              // ? MHz
> -    0,                              // manufacturer
> -    0,                              // serial
> -    0,                              // asset tag
> -    0,                              // part number
> -    0,                              // rank
> -  }, {
> -    "DIMM1\0"
> -  }
> -};
> -
> -STATIC CONST ARM_TYPE17 mArmDefaultType17_2 = {
> -  {
> -    { // SMBIOS_STRUCTURE Hdr
> -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> -      sizeof (SMBIOS_TABLE_TYPE17),
> -      SMBIOS_HANDLE_PI_RESERVED,
> -    },
> -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> -    0xFFFE,                         // no errors
> -    72,                             // single DIMM with ECC
> -    64,                             // data width of this device (64-bits)
> -    0xFFFF,                         // size unknown
> -    0x09,                           // DIMM
> -    1,                              // part of a set
> -    1,                              // device locator
> -    0,                              // bank locator
> -    MemoryTypeDdr4,                 // DDR4
> -    {},                             // type detail
> -    0,                              // ? MHz
> -    0,                              // manufacturer
> -    0,                              // serial
> -    0,                              // asset tag
> -    0,                              // part number
> -    0,                              // rank
> -  }, {
> -    "DIMM2\0"
> -  }
> -};
> -
> -STATIC CONST ARM_TYPE17 mArmDefaultType17_3 = {
> -  {
> -    { // SMBIOS_STRUCTURE Hdr
> -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> -      sizeof (SMBIOS_TABLE_TYPE17),
> -      SMBIOS_HANDLE_PI_RESERVED,
> -    },
> -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> -    0xFFFE,                         // no errors
> -    72,                             // single DIMM with ECC
> -    64,                             // data width of this device (64-bits)
> -    0xFFFF,                         // size unknown
> -    0x09,                           // DIMM
> -    1,                              // part of a set
> -    1,                              // device locator
> -    0,                              // bank locator
> -    MemoryTypeDdr4,                 // DDR4
> -    {},                             // type detail
> -    0,                              // ? MHz
> -    0,                              // manufacturer
> -    0,                              // serial
> -    0,                              // asset tag
> -    0,                              // part number
> -    0,                              // rank
> -  }, {
> -    "DIMM3\0"
> -  }
> -};
> -
> -STATIC CONST ARM_TYPE17 mArmDefaultType17_4 = {
> -  {
> -    { // SMBIOS_STRUCTURE Hdr
> -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> -      sizeof (SMBIOS_TABLE_TYPE17),
> -      SMBIOS_HANDLE_PI_RESERVED,
> -    },
> -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> -    0xFFFE,                         // no errors
> -    72,                             // single DIMM with ECC
> -    64,                             // data width of this device (64-bits)
> -    0xFFFF,                         // size unknown
> -    0x09,                           // DIMM
> -    1,                              // part of a set
> -    1,                              // device locator
> -    0,                              // bank locator
> -    MemoryTypeDdr4,                 // DDR4
> -    {},                             // type detail
> -    0,                              // ? MHz
> -    0,                              // manufacturer
> -    0,                              // serial
> -    0,                              // asset tag
> -    0,                              // part number
> -    0,                              // rank
> -  }, {
> -    "DIMM4\0"
> -  }
> -};
> -
>  // Memory array mapped address, this structure
>  // is overridden by InstallMemoryStructure
>  STATIC CONST ARM_TYPE19 mArmDefaultType19 = {
> @@ -559,13 +536,275 @@ STATIC SMBIOS_STRUCTURE * CONST FixedTables[] = {
>    (SMBIOS_STRUCTURE *)&mArmDefaultType9_1.Base.Hdr,
>    (SMBIOS_STRUCTURE *)&mArmDefaultType9_2.Base.Hdr,
>    (SMBIOS_STRUCTURE *)&mArmDefaultType16.Base.Hdr,
> -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_1.Base.Hdr,
> -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_2.Base.Hdr,
> -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_3.Base.Hdr,
> -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_4.Base.Hdr,
>    (SMBIOS_STRUCTURE *)&mArmDefaultType32.Base.Hdr,
>  };
>
> +STATIC
> +UINT16
> +GetPrimaryBusWidth (
> +  IN UINT8           SpdPrimaryBusWidth
> +  )
> +{
> +  UINT16                    PrimaryBusWidth;
> +
> +  switch (SpdPrimaryBusWidth) {
> +  case SPD4_MEM_BUS_WIDTH_8BIT:
> +    PrimaryBusWidth = 8;
> +    break;
> +  case SPD4_MEM_BUS_WIDTH_16BIT:
> +    PrimaryBusWidth = 16;
> +    break;
> +  case SPD4_MEM_BUS_WIDTH_32BIT:
> +    PrimaryBusWidth = 32;
> +    break;
> +  case SPD4_MEM_BUS_WIDTH_64BIT:
> +    PrimaryBusWidth = 64;
> +    break;
> +  default:
> +    PrimaryBusWidth = 0xFFFF;

Put an ASSERT (FALSE); here so we catch this error condition in DEBUG builds.

> +    break;
> +  }
> +
> +  return PrimaryBusWidth;
> +}
> +
> +STATIC
> +UINT16
> +GetSdramDeviceWidth (
> +  IN UINT8            SpdSdramDeviceWidth
> +  )
> +{
> +  UINT16                    SdramDeviceWidth;
> +
> +  switch (SpdSdramDeviceWidth) {
> +  case SPD4_MEM_DEV_WIDTH_4BIT:
> +    SdramDeviceWidth = 4;
> +    break;
> +  case SPD4_MEM_DEV_WIDTH_8BIT:
> +    SdramDeviceWidth = 8;
> +    break;
> +  case SPD4_MEM_DEV_WIDTH_16BIT:
> +    SdramDeviceWidth = 16;
> +    break;
> +  case SPD4_MEM_DEV_WIDTH_32BIT:
> +    SdramDeviceWidth = 32;
> +    break;
> +  default:
> +    SdramDeviceWidth = 0;

Same

> +    break;
> +  }
> +
> +  return SdramDeviceWidth;
> +}
> +
> +STATIC
> +UINT16
> +CaluculateModuleDramCapacityMB (

'Calculate'

> + IN SPD4_BASE_SECTION            *Spd4Base
> + )
> +{
> +  UINT32                    SdramCapacityMbit = 0;
> +  UINT16                    PrimaryBusWidth = 0;
> +  UINT8                     SdramDeviceWidth = 0;
> +  UINT8                     RankCount = 0;

Please do the zero assignments as separate statements.

> +  UINT16                    DramSize;
> +  UINT32                    i;
> +

'Index' is more idiomatic than 'i' in Tianocore.

> +  for (i = 0; CapacityTable[i].Capacity != SPD4_SDRAM_CAPACITY_INVALID; i++) {
> +    if (Spd4Base->SdramDensityAndBanks.Bits.Density == CapacityTable[i].Capacity) {
> +      SdramCapacityMbit = CapacityTable[i].SizeMbit;
> +      break;
> +    }
> +  }
> +
> +  PrimaryBusWidth = GetPrimaryBusWidth (Spd4Base->ModuleMemoryBusWidth.Bits.PrimaryBusWidth);
> +  SdramDeviceWidth = GetSdramDeviceWidth (Spd4Base->ModuleOrganization.Bits.SdramDeviceWidth);
> +  RankCount = Spd4Base->ModuleOrganization.Bits.RankCount + 1;
> +
> +  if ((SdramCapacityMbit == 0) || (PrimaryBusWidth == 0xFFFF) ||
> +      (SdramDeviceWidth == 0) || RankCount == 0) {
> +    DEBUG ((DEBUG_ERROR, "Calculate DRAM size failed. Cap:%d, BusWidth:%d, "
> +                         "DevWidth:%d, Rank:%d\n", SdramCapacityMbit,
> +                         PrimaryBusWidth, SdramDeviceWidth, RankCount));
> +    return 0xFFFF;

What is this magic number?

> +  }
> +
> +  //
> +  //Total[MB] = SDRAM Capacity[Mb] / 8 * Primary Bus Width /
> +  //            SDRAM Width * Logical Ranks per DIMM
> +  //
> +  DramSize = (((SdramCapacityMbit >> 3) * PrimaryBusWidth) / SdramDeviceWidth) * RankCount;

Could we use a shift for the device width? In general, we avoid
integer divisions with divisor values that are only known at runtime.

> +
> +  return DramSize;
> +}
> +
> +STATIC
> +VOID
> +GetManufacturerName (
> +  IN UINT16           SpdManufacturerId,
> +  OUT CHAR8           *ManufacturerStr
> +  )
> +{
> +  UINT16                    ManufacturerId;
> +  UINT32                    i;

Index

> +
> +  ManufacturerId = SwapBytes16 (SpdManufacturerId);
> +  ManufacturerId &= 0x7FFF; // ignore odd parity bit
> +
> +  for (i = 0; Manufacturer[i].ManufacturerId != 0xFFFF; i++) {
> +    if (ManufacturerId == Manufacturer[i].ManufacturerId) {
> +      AsciiStrCpyS (ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN,
> +                    Manufacturer[i].ManufacturerName);

These return a value on failure. If this code takes user input, you
should handle the return value. In this case, it only takes SPD data
as input, so it is sufficient to do something like

RETURN_STATUS RetStatus;

..
RetStatus = AsciiStrCpyS (...)
ASSERT_RETURN_ERROR (RetStatus);

but ignore RetStatus in the code flow.

> +      return;
> +    }
> +  }
> +
> +  AsciiStrCpyS (ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN,
> +                Manufacturer[i].ManufacturerName);

Same here

> +}
> +
> +STATIC
> +EFI_STATUS
> +InstallMemoryDeviceStructure (
> +  VOID
> +  )
> +{
> +  EFI_SMBIOS_HANDLE         SmbiosHandle;
> +  ARM_TYPE17                *Descriptor;
> +  SPD_DDR4                  *Spd;
> +  UINT8                     Slot;
> +
> +  CHAR8                     DeviceLocatorStr[TYPE17_DEVICE_LOCATOR_LEN] = {0};
> +  CHAR8                     BankLocatorStr[TYPE17_BANK_LOCATOR_LEN] = {0};
> +  CHAR8                     ManufacturerStr[TYPE17_MANUFACTURER_NAME_LEN] = {0};
> +  CHAR8                     SerialNumberStr[TYPE17_SERIAL_NUMBER_LEN] = {0};
> +  CHAR8                     AssetTagStr[] = "Unknown\0";
> +  CHAR8                     PartNumberStr[TYPE17_MODULE_PART_NUMBER_LEN] = {0};
> +

Drop the initialisers. If you really need the buffers to be cleared,
use ZeroMem()

> +  Descriptor = AllocateZeroPool(sizeof (ARM_TYPE17));
> +  if (Descriptor == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Spd = (SPD_DDR4 *) FixedPcdGet32 (PcdSmbiosStoredSpdDDR4Address);
> +
> +  for (Slot = 0; Slot < 4; Slot++, Spd++) {
> +    SetMem (Descriptor, sizeof (ARM_TYPE17), 0);
> +    // fill fixed parameters
> +    Descriptor->Base.Hdr.Type = EFI_SMBIOS_TYPE_MEMORY_DEVICE;
> +    Descriptor->Base.Hdr.Length = sizeof (SMBIOS_TABLE_TYPE17);
> +    Descriptor->Base.Hdr.Handle = SMBIOS_HANDLE_PI_RESERVED;
> +    Descriptor->Base.MemoryArrayHandle = SMBIOS_HANDLE_MEMORY;
> +    Descriptor->Base.MemoryErrorInformationHandle = 0xFFFE;
> +    Descriptor->Base.DeviceSet = 1;
> +    Descriptor->Base.MemoryType = MemoryTypeDdr4;
> +    Descriptor->Base.DeviceLocator = 1;
> +    Descriptor->Base.BankLocator = 2;
> +    Descriptor->Base.Manufacturer = 3;
> +    Descriptor->Base.SerialNumber = 4;
> +    Descriptor->Base.AssetTag = 5;
> +    Descriptor->Base.PartNumber = 6;
> +
> +    if (Spd->Base.Description.Data == 0) {
> +      // No DIMM inserted, fill the default parameters
> +      CHAR8                 DefaultStrings[] = "NO DIMM\0NO DIMM\0NO DIMM\0NO DIMM\0NO DIMM\0";
> +      UINT8                 DefaultStringsLen = 40;
> +
> +      Descriptor->Base.TotalWidth = 0xFFFF;
> +      Descriptor->Base.DataWidth = 0xFFFF;
> +      Descriptor->Base.Size = 0;
> +      Descriptor->Base.FormFactor = 0x09;
> +      Descriptor->Base.DeviceSet = 1;
> +
> +      Descriptor->Base.Attributes = 0;
> +      Descriptor->Base.Speed = 0;
> +      Descriptor->Base.ConfiguredMemoryClockSpeed = 0;
> +      Descriptor->Base.MinimumVoltage = 0;
> +      Descriptor->Base.MaximumVoltage = 0;
> +      Descriptor->Base.ConfiguredVoltage = 0;
> +
> +      AsciiSPrint (Descriptor->Strings, TYPE17_STRINGS_MAX_LEN, "DIMM %d\0", (Slot + 1));
> +      CopyMem ((Descriptor->Strings + (AsciiStrLen(Descriptor->Strings) + 1)),
> +              DefaultStrings, DefaultStringsLen);
> +    } else {
> +      CHAR8                 *Temp;
> +
> +      Descriptor->Base.TotalWidth = Descriptor->Base.DataWidth =
> +        GetPrimaryBusWidth (Spd->Base.ModuleMemoryBusWidth.Bits.PrimaryBusWidth);
> +      if (Spd->Base.ModuleMemoryBusWidth.Bits.BusWidthExtension) {
> +        if (Descriptor->Base.TotalWidth != 0xFFFF) {
> +          Descriptor->Base.TotalWidth += 8;
> +        }
> +      }
> +
> +      Descriptor->Base.Size = CaluculateModuleDramCapacityMB ((SPD4_BASE_SECTION *)Spd);
> +
> +      switch (Spd->Base.ModuleType.Bits.ModuleType) {
> +      case SPD4_MEM_MODULE_TYPE_RDIMM:
> +        Descriptor->Base.FormFactor = 0x09;
> +        Descriptor->Base.TypeDetail.Registered = 1;
> +        Descriptor->Base.TypeDetail.Unbuffered = 0;
> +        break;
> +      case SPD4_MEM_MODULE_TYPE_UDIMM:
> +        Descriptor->Base.FormFactor = 0x09;
> +        Descriptor->Base.TypeDetail.Registered = 0;
> +        Descriptor->Base.TypeDetail.Unbuffered = 1;
> +        break;
> +      case SPD4_MEM_MODULE_TYPE_SODIMM:
> +        Descriptor->Base.FormFactor = 0x0D;
> +        Descriptor->Base.TypeDetail.Registered = 0;
> +        Descriptor->Base.TypeDetail.Unbuffered = 1;
> +        break;
> +      default:
> +        Descriptor->Base.FormFactor = 0x01;
> +        Descriptor->Base.TypeDetail.Registered = 0;
> +        Descriptor->Base.TypeDetail.Unbuffered = 0;
> +        break;
> +      }
> +
> +      Descriptor->Base.Attributes = Spd->Base.ModuleOrganization.Bits.RankCount + 1;
> +      Descriptor->Base.Speed = 2133;
> +      Descriptor->Base.ConfiguredMemoryClockSpeed = 2133;
> +      Descriptor->Base.MinimumVoltage = 1200;
> +      Descriptor->Base.MaximumVoltage = 1200;
> +      Descriptor->Base.ConfiguredVoltage = 1200;
> +
> +      AsciiSPrint (DeviceLocatorStr, sizeof(DeviceLocatorStr), "DIMM %d\0", (Slot + 1));
> +      AsciiSPrint (BankLocatorStr, sizeof(BankLocatorStr), "CHANNEL %d SLOT %d\0", (Slot / 2), (Slot % 2));
> +      GetManufacturerName (Spd->ManufactureInfo.ModuleId.IdCode.Data, ManufacturerStr);
> +      AsciiSPrint (SerialNumberStr, TYPE17_SERIAL_NUMBER_LEN, "0x%08X\0",
> +                   SwapBytes32(Spd->ManufactureInfo.ModuleId.SerialNumber.Data));
> +      //
> +      // Module part number is not null terminated string in SPD DDR4,
> +      // unused bytes are filled with 0x20(space).
> +      //
> +      CopyMem (PartNumberStr,
> +               (CHAR8 *)Spd->ManufactureInfo.ModulePartNumber.ModulePartNumber,
> +               TYPE17_MODULE_PART_NUMBER_LEN - 1);
> +
> +      Temp = Descriptor->Strings;
> +      AsciiStrnCpy (Temp, DeviceLocatorStr, TYPE17_DEVICE_LOCATOR_LEN);
> +      Temp += (AsciiStrLen (DeviceLocatorStr) + 1);
> +      AsciiStrnCpy (Temp, BankLocatorStr, TYPE17_BANK_LOCATOR_LEN);
> +      Temp += (AsciiStrLen (BankLocatorStr) + 1);
> +      AsciiStrnCpy (Temp, ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN);
> +      Temp += (AsciiStrLen (ManufacturerStr) + 1);
> +      AsciiStrnCpy (Temp, SerialNumberStr, TYPE17_SERIAL_NUMBER_LEN);
> +      Temp += (AsciiStrLen (SerialNumberStr) + 1);
> +      AsciiStrnCpy (Temp, AssetTagStr, TYPE17_ASSETTAG_LEN);
> +      Temp += (AsciiStrLen (AssetTagStr) + 1);
> +      AsciiStrnCpy (Temp, PartNumberStr, TYPE17_MODULE_PART_NUMBER_LEN);
> +    }
> +
> +    SmbiosHandle = Descriptor->Base.Hdr.Handle;
> +    mSmbios->Add (mSmbios, NULL, &SmbiosHandle, &Descriptor->Base.Hdr);
> +  }
> +
> +  FreePool (Descriptor);
> +
> +  return EFI_SUCCESS;
> +}
> +
>  STATIC
>  EFI_STATUS
>  InstallMemoryStructure (
> @@ -587,6 +826,7 @@ InstallMemoryStructure (
>    return mSmbios->Add (mSmbios, NULL, &SmbiosHandle, &Descriptor->Base.Hdr);
>  }
>
> +
>  STATIC
>  VOID
>  InstallAllStructures (
> @@ -608,6 +848,16 @@ InstallAllStructures (
>      }
>    }
>
> +  //
> +  // SPD_DDR4 data is stored in Non Secure SRAM by SCP-firmware.
> +  // Install Type17 record by analyzing SPD_DDR4 information.
> +  //
> +  Status = InstallMemoryDeviceStructure();
> +  if (EFI_ERROR(Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: failed to add SMBIOS type 17 table - %r\n",
> +        __FUNCTION__, Status));
> +  }
> +
>    for (Hob.Raw = GetHobList ();
>         !END_OF_HOB_LIST (Hob);
>         Hob.Raw = GET_NEXT_HOB (Hob)) {
> diff --git a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> index e711cbf6dc..abd4602e34 100644
> --- a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> +++ b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> @@ -21,6 +21,7 @@
>    ArmPkg/ArmPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
> +  Silicon/Socionext/SynQuacer/SynQuacer.dec
>
>  [LibraryClasses]
>    BaseMemoryLib
> @@ -34,6 +35,7 @@
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
> +  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address
>
>  [Protocols]
>    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> diff --git a/Silicon/Socionext/SynQuacer/SynQuacer.dec b/Silicon/Socionext/SynQuacer/SynQuacer.dec
> index e7197e2319..98b574bd32 100644
> --- a/Silicon/Socionext/SynQuacer/SynQuacer.dec
> +++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec
> @@ -43,6 +43,9 @@
>    # for capsule update
>    gSynQuacerTokenSpaceGuid.PcdLowestSupportedFirmwareVersion|1|UINT32|0x00000009
>
> +  # for SMBIOS Type17
> +  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address|0|UINT32|0x0000000A
> +
>  [PcdsPatchableInModule, PcdsDynamic]
>    # Enable both RC #0 and RC #1 by default
>    gSynQuacerTokenSpaceGuid.PcdPcieEnableMask|0x3|UINT8|0x00000007
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table
  2019-06-11 13:00 ` Ard Biesheuvel
@ 2019-06-12  5:23   ` Masahisa Kojima
  0 siblings, 0 replies; 3+ messages in thread
From: Masahisa Kojima @ 2019-06-12  5:23 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Leif Lindholm

Hi Ard,

On Tue, 11 Jun 2019 at 22:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 11 Jun 2019 at 14:18, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
> >
> > This adds the SMBIOS type 17 table support for Developerbox platform.
> > The SPDs on a I2C bus is only accessible by the SCP, so SCP-firmware
> > stores SPDs in non-secure SRAM.
> >
> > This commit also reduces the uefi stack size to allocate space
> > for SPDs. It requires 2KB, 512bytes * 4 DIMMs.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
>
> So this patch depends on SCP fw changes that are not yet available,
> right? That is not a problem, but I will hold off on merging it until
> we have a new SCP firmware build in the tree.

Yes, this patch depends on SCP firmware changes.
It is better to wait SCP fw available, thank you.

> Some comments below.

I will fix all comments.

> > ---
> >  .../Socionext/DeveloperBox/DeveloperBox.dsc   |   3 +
> >  .../DeveloperBox/DeveloperBox.dsc.inc         |   2 +-
> >  .../SmbiosPlatformDxe/SmbiosPlatformDxe.c     | 494 +++++++++++++-----
> >  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   2 +
> >  Silicon/Socionext/SynQuacer/SynQuacer.dec     |   3 +
> >  5 files changed, 381 insertions(+), 123 deletions(-)
> >
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > index 97fb8c410c..f247370694 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > @@ -147,6 +147,9 @@
> >
> >    gSynQuacerTokenSpaceGuid.PcdDramInfoBase|0x2E00FFC0
> >
> > +  # SCP-firmware stored SPD DDR4 data in non-secure SRAM
> > +  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address|0x2E00F000
> > +
>
> Please drop 'Smbios' from this PCD name - we may want to use the SPD
> data in a different way in the future.
>
> >    #
> >    # 96boards mezzanine support
> >    #
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > index a10e48ca07..abb113e858 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > @@ -137,7 +137,7 @@
> >
> >    # non-secure SRAM
> >    gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
> > -  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
> > +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xF000
> >
> >    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
> >
> > diff --git a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > index 6227b77877..da0fd2e6a5 100644
> > --- a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > +++ b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > @@ -10,17 +10,48 @@
> >  **/
> >
> >  #include <PiDxe.h>
> > +#include <IndustryStandard/SdramSpdDdr4.h>
> >  #include <IndustryStandard/SmBios.h>
> >  #include <Library/BaseLib.h>
> >  #include <Library/BaseMemoryLib.h>
> >  #include <Library/DebugLib.h>
> >  #include <Library/HobLib.h>
> >  #include <Library/MemoryAllocationLib.h>
> > +#include <Library/PrintLib.h>
> >  #include <Library/UefiBootServicesTableLib.h>
> >  #include <Protocol/Smbios.h>
> >
> >  STATIC EFI_SMBIOS_PROTOCOL       *mSmbios;
> >
> > +#define SPD4_MEM_BUS_WIDTH_8BIT    (0x00)
> > +#define SPD4_MEM_BUS_WIDTH_16BIT   (BIT0)
> > +#define SPD4_MEM_BUS_WIDTH_32BIT   (BIT1)
> > +#define SPD4_MEM_BUS_WIDTH_64BIT   (BIT0 | BIT1)
> > +
> > +#define SPD4_MEM_DEV_WIDTH_4BIT    (0x00)
> > +#define SPD4_MEM_DEV_WIDTH_8BIT    (BIT0)
> > +#define SPD4_MEM_DEV_WIDTH_16BIT   (BIT1)
> > +#define SPD4_MEM_DEV_WIDTH_32BIT   (BIT0 | BIT1)
> > +
> > +#define SPD4_MEM_MODULE_TYPE_RDIMM    0x01
> > +#define SPD4_MEM_MODULE_TYPE_UDIMM    0x02
> > +#define SPD4_MEM_MODULE_TYPE_SODIMM   0x03
> > +
> > +#define TYPE17_DEVICE_LOCATOR_LEN     (8 + 1)
> > +#define TYPE17_BANK_LOCATOR_LEN       (20 + 1)
> > +#define TYPE17_MANUFACTURER_NAME_LEN  (30 + 1)
> > +#define TYPE17_SERIAL_NUMBER_LEN      (16 + 1)
> > +#define TYPE17_ASSETTAG_LEN           (16 + 1)
> > +#define TYPE17_MODULE_PART_NUMBER_LEN (20 + 1)
> > +
> > +#define TYPE17_STRINGS_MAX_LEN        (TYPE17_DEVICE_LOCATOR_LEN + \
> > +                                       TYPE17_BANK_LOCATOR_LEN + \
> > +                                       TYPE17_MANUFACTURER_NAME_LEN + \
> > +                                       TYPE17_SERIAL_NUMBER_LEN + \
> > +                                       TYPE17_ASSETTAG_LEN + \
> > +                                       TYPE17_MODULE_PART_NUMBER_LEN + \
> > +                                       1/* null SMBIOS_TABLE_STRING terminator */ )
> > +
> >  //
> >  // Type definition and contents of the default SMBIOS table.
> >  // This table covers only the minimum structures required by
> > @@ -69,7 +100,7 @@ typedef struct {
> >
> >  typedef struct {
> >    SMBIOS_TABLE_TYPE17 Base;
> > -  CHAR8               Strings[];
> > +  CHAR8               Strings[TYPE17_STRINGS_MAX_LEN];
> >  } ARM_TYPE17;
> >
> >  typedef struct {
> > @@ -94,6 +125,69 @@ enum {
> >    SMBIOS_HANDLE_MEMORY,
> >  };
> >
> > +struct JEP106_MANUFACTURER_TABLE {
> > +  UINT16 ManufacturerId;
> > +  CHAR8  ManufacturerName[TYPE17_MANUFACTURER_NAME_LEN];
> > +};
> > +
> > +STATIC CONST struct JEP106_MANUFACTURER_TABLE Manufacturer[] = {
> > +  {0x0010, "NEC\0"},
> > +  {0x002C, "Micron Technology\0"},
> > +  {0x003D, "Tektronix\0"},
> > +  {0x0097, "Texas Instruments\0"},
> > +  {0x00AD, "SK Hynix\0"},
> > +  {0x00B3, "IDT\0"},
> > +  {0x00C1, "Infineon\0"},
> > +  {0x00CE, "Samsung\0"},
> > +  {0x00DA, "Winbond Electronic\0"},
> > +  {0x014F, "Transcend Information\0"},
> > +  {0x0194, "Smart Modular\0"},
> > +  {0x0198, "Kingston\0"},
> > +  {0x02C8, "Agilent Technologies\0"},
> > +  {0x02FE, "Elpida\0"},
> > +  {0x030B, "Nanya Technology\0"},
> > +  {0x0443, "Ramaxel Technology\0"},
> > +  {0x04B3, "Inphi Corporation\0"},
> > +  {0x04C8, "Powerchip Semiconductor\0"},
> > +  {0x0551, "Qimonda\0"},
> > +  {0x0557, "AENEON\0"},
> > +  {0x059B, "Crucial Technology\0"},
> > +  {0xFFFF, "Unknown\0"}
> > +};
> > +
> > +enum SPD4_SDRAM_CAPACITY {
> > +  SPD4_SDRAM_CAPACITY_256MBIT = 0,
> > +  SPD4_SDRAM_CAPACITY_512MBIT,
> > +  SPD4_SDRAM_CAPACITY_1GBIT,
> > +  SPD4_SDRAM_CAPACITY_2GBIT,
> > +  SPD4_SDRAM_CAPACITY_4GBIT,
> > +  SPD4_SDRAM_CAPACITY_8GBIT,
> > +  SPD4_SDRAM_CAPACITY_16GBIT,
> > +  SPD4_SDRAM_CAPACITY_32GBIT,
> > +  SPD4_SDRAM_CAPACITY_12GBIT,
> > +  SPD4_SDRAM_CAPACITY_24GBIT,
> > +  SPD4_SDRAM_CAPACITY_INVALID = 0xFF,
> > +};
> > +
> > +struct SPD4_SDRAM_CAPACITY_TABLE {
> > +  enum SPD4_SDRAM_CAPACITY        Capacity;
> > +  UINT16                          SizeMbit;
> > +};
> > +
> > +STATIC CONST struct SPD4_SDRAM_CAPACITY_TABLE CapacityTable[]  = {
> > +  {SPD4_SDRAM_CAPACITY_256MBIT,     256        },
> > +  {SPD4_SDRAM_CAPACITY_512MBIT,     512        },
> > +  {SPD4_SDRAM_CAPACITY_1GBIT,       (1 * 1024) },
> > +  {SPD4_SDRAM_CAPACITY_2GBIT,       (2 * 1024) },
> > +  {SPD4_SDRAM_CAPACITY_4GBIT,       (4 * 1024) },
> > +  {SPD4_SDRAM_CAPACITY_8GBIT,       (8 * 1024) },
> > +  {SPD4_SDRAM_CAPACITY_16GBIT,      (16 * 1024)},
> > +  {SPD4_SDRAM_CAPACITY_32GBIT,      (32 * 1024)},
> > +  {SPD4_SDRAM_CAPACITY_12GBIT,      (12 * 1024)},
> > +  {SPD4_SDRAM_CAPACITY_24GBIT,      (24 * 1024)},
> > +  {SPD4_SDRAM_CAPACITY_INVALID,     0          },
> > +};
> > +
> >  // BIOS information (section 7.1)
> >  STATIC CONST ARM_TYPE0 mArmDefaultType0 = {
> >    {
> > @@ -394,123 +488,6 @@ STATIC CONST ARM_TYPE16 mArmDefaultType16 = {
> >    }
> >  };
> >
> > -// Memory device
> > -STATIC CONST ARM_TYPE17 mArmDefaultType17_1 = {
> > -  {
> > -    { // SMBIOS_STRUCTURE Hdr
> > -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> > -      sizeof (SMBIOS_TABLE_TYPE17),
> > -      SMBIOS_HANDLE_PI_RESERVED,
> > -    },
> > -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> > -    0xFFFE,                         // no errors
> > -    72,                             // single DIMM with ECC
> > -    64,                             // data width of this device (64-bits)
> > -    0xFFFF,                         // size unknown
> > -    0x09,                           // DIMM
> > -    1,                              // part of a set
> > -    1,                              // device locator
> > -    0,                              // bank locator
> > -    MemoryTypeDdr4,                 // DDR4
> > -    {},                             // type detail
> > -    0,                              // ? MHz
> > -    0,                              // manufacturer
> > -    0,                              // serial
> > -    0,                              // asset tag
> > -    0,                              // part number
> > -    0,                              // rank
> > -  }, {
> > -    "DIMM1\0"
> > -  }
> > -};
> > -
> > -STATIC CONST ARM_TYPE17 mArmDefaultType17_2 = {
> > -  {
> > -    { // SMBIOS_STRUCTURE Hdr
> > -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> > -      sizeof (SMBIOS_TABLE_TYPE17),
> > -      SMBIOS_HANDLE_PI_RESERVED,
> > -    },
> > -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> > -    0xFFFE,                         // no errors
> > -    72,                             // single DIMM with ECC
> > -    64,                             // data width of this device (64-bits)
> > -    0xFFFF,                         // size unknown
> > -    0x09,                           // DIMM
> > -    1,                              // part of a set
> > -    1,                              // device locator
> > -    0,                              // bank locator
> > -    MemoryTypeDdr4,                 // DDR4
> > -    {},                             // type detail
> > -    0,                              // ? MHz
> > -    0,                              // manufacturer
> > -    0,                              // serial
> > -    0,                              // asset tag
> > -    0,                              // part number
> > -    0,                              // rank
> > -  }, {
> > -    "DIMM2\0"
> > -  }
> > -};
> > -
> > -STATIC CONST ARM_TYPE17 mArmDefaultType17_3 = {
> > -  {
> > -    { // SMBIOS_STRUCTURE Hdr
> > -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> > -      sizeof (SMBIOS_TABLE_TYPE17),
> > -      SMBIOS_HANDLE_PI_RESERVED,
> > -    },
> > -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> > -    0xFFFE,                         // no errors
> > -    72,                             // single DIMM with ECC
> > -    64,                             // data width of this device (64-bits)
> > -    0xFFFF,                         // size unknown
> > -    0x09,                           // DIMM
> > -    1,                              // part of a set
> > -    1,                              // device locator
> > -    0,                              // bank locator
> > -    MemoryTypeDdr4,                 // DDR4
> > -    {},                             // type detail
> > -    0,                              // ? MHz
> > -    0,                              // manufacturer
> > -    0,                              // serial
> > -    0,                              // asset tag
> > -    0,                              // part number
> > -    0,                              // rank
> > -  }, {
> > -    "DIMM3\0"
> > -  }
> > -};
> > -
> > -STATIC CONST ARM_TYPE17 mArmDefaultType17_4 = {
> > -  {
> > -    { // SMBIOS_STRUCTURE Hdr
> > -      EFI_SMBIOS_TYPE_MEMORY_DEVICE,
> > -      sizeof (SMBIOS_TABLE_TYPE17),
> > -      SMBIOS_HANDLE_PI_RESERVED,
> > -    },
> > -    SMBIOS_HANDLE_MEMORY,           // array to which this module belongs
> > -    0xFFFE,                         // no errors
> > -    72,                             // single DIMM with ECC
> > -    64,                             // data width of this device (64-bits)
> > -    0xFFFF,                         // size unknown
> > -    0x09,                           // DIMM
> > -    1,                              // part of a set
> > -    1,                              // device locator
> > -    0,                              // bank locator
> > -    MemoryTypeDdr4,                 // DDR4
> > -    {},                             // type detail
> > -    0,                              // ? MHz
> > -    0,                              // manufacturer
> > -    0,                              // serial
> > -    0,                              // asset tag
> > -    0,                              // part number
> > -    0,                              // rank
> > -  }, {
> > -    "DIMM4\0"
> > -  }
> > -};
> > -
> >  // Memory array mapped address, this structure
> >  // is overridden by InstallMemoryStructure
> >  STATIC CONST ARM_TYPE19 mArmDefaultType19 = {
> > @@ -559,13 +536,275 @@ STATIC SMBIOS_STRUCTURE * CONST FixedTables[] = {
> >    (SMBIOS_STRUCTURE *)&mArmDefaultType9_1.Base.Hdr,
> >    (SMBIOS_STRUCTURE *)&mArmDefaultType9_2.Base.Hdr,
> >    (SMBIOS_STRUCTURE *)&mArmDefaultType16.Base.Hdr,
> > -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_1.Base.Hdr,
> > -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_2.Base.Hdr,
> > -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_3.Base.Hdr,
> > -  (SMBIOS_STRUCTURE *)&mArmDefaultType17_4.Base.Hdr,
> >    (SMBIOS_STRUCTURE *)&mArmDefaultType32.Base.Hdr,
> >  };
> >
> > +STATIC
> > +UINT16
> > +GetPrimaryBusWidth (
> > +  IN UINT8           SpdPrimaryBusWidth
> > +  )
> > +{
> > +  UINT16                    PrimaryBusWidth;
> > +
> > +  switch (SpdPrimaryBusWidth) {
> > +  case SPD4_MEM_BUS_WIDTH_8BIT:
> > +    PrimaryBusWidth = 8;
> > +    break;
> > +  case SPD4_MEM_BUS_WIDTH_16BIT:
> > +    PrimaryBusWidth = 16;
> > +    break;
> > +  case SPD4_MEM_BUS_WIDTH_32BIT:
> > +    PrimaryBusWidth = 32;
> > +    break;
> > +  case SPD4_MEM_BUS_WIDTH_64BIT:
> > +    PrimaryBusWidth = 64;
> > +    break;
> > +  default:
> > +    PrimaryBusWidth = 0xFFFF;
>
> Put an ASSERT (FALSE); here so we catch this error condition in DEBUG builds.
>
> > +    break;
> > +  }
> > +
> > +  return PrimaryBusWidth;
> > +}
> > +
> > +STATIC
> > +UINT16
> > +GetSdramDeviceWidth (
> > +  IN UINT8            SpdSdramDeviceWidth
> > +  )
> > +{
> > +  UINT16                    SdramDeviceWidth;
> > +
> > +  switch (SpdSdramDeviceWidth) {
> > +  case SPD4_MEM_DEV_WIDTH_4BIT:
> > +    SdramDeviceWidth = 4;
> > +    break;
> > +  case SPD4_MEM_DEV_WIDTH_8BIT:
> > +    SdramDeviceWidth = 8;
> > +    break;
> > +  case SPD4_MEM_DEV_WIDTH_16BIT:
> > +    SdramDeviceWidth = 16;
> > +    break;
> > +  case SPD4_MEM_DEV_WIDTH_32BIT:
> > +    SdramDeviceWidth = 32;
> > +    break;
> > +  default:
> > +    SdramDeviceWidth = 0;
>
> Same
>
> > +    break;
> > +  }
> > +
> > +  return SdramDeviceWidth;
> > +}
> > +
> > +STATIC
> > +UINT16
> > +CaluculateModuleDramCapacityMB (
>
> 'Calculate'
>
> > + IN SPD4_BASE_SECTION            *Spd4Base
> > + )
> > +{
> > +  UINT32                    SdramCapacityMbit = 0;
> > +  UINT16                    PrimaryBusWidth = 0;
> > +  UINT8                     SdramDeviceWidth = 0;
> > +  UINT8                     RankCount = 0;
>
> Please do the zero assignments as separate statements.
>
> > +  UINT16                    DramSize;
> > +  UINT32                    i;
> > +
>
> 'Index' is more idiomatic than 'i' in Tianocore.
>
> > +  for (i = 0; CapacityTable[i].Capacity != SPD4_SDRAM_CAPACITY_INVALID; i++) {
> > +    if (Spd4Base->SdramDensityAndBanks.Bits.Density == CapacityTable[i].Capacity) {
> > +      SdramCapacityMbit = CapacityTable[i].SizeMbit;
> > +      break;
> > +    }
> > +  }
> > +
> > +  PrimaryBusWidth = GetPrimaryBusWidth (Spd4Base->ModuleMemoryBusWidth.Bits.PrimaryBusWidth);
> > +  SdramDeviceWidth = GetSdramDeviceWidth (Spd4Base->ModuleOrganization.Bits.SdramDeviceWidth);
> > +  RankCount = Spd4Base->ModuleOrganization.Bits.RankCount + 1;
> > +
> > +  if ((SdramCapacityMbit == 0) || (PrimaryBusWidth == 0xFFFF) ||
> > +      (SdramDeviceWidth == 0) || RankCount == 0) {
> > +    DEBUG ((DEBUG_ERROR, "Calculate DRAM size failed. Cap:%d, BusWidth:%d, "
> > +                         "DevWidth:%d, Rank:%d\n", SdramCapacityMbit,
> > +                         PrimaryBusWidth, SdramDeviceWidth, RankCount));
> > +    return 0xFFFF;
>
> What is this magic number?
>
> > +  }
> > +
> > +  //
> > +  //Total[MB] = SDRAM Capacity[Mb] / 8 * Primary Bus Width /
> > +  //            SDRAM Width * Logical Ranks per DIMM
> > +  //
> > +  DramSize = (((SdramCapacityMbit >> 3) * PrimaryBusWidth) / SdramDeviceWidth) * RankCount;
>
> Could we use a shift for the device width? In general, we avoid
> integer divisions with divisor values that are only known at runtime.
>
> > +
> > +  return DramSize;
> > +}
> > +
> > +STATIC
> > +VOID
> > +GetManufacturerName (
> > +  IN UINT16           SpdManufacturerId,
> > +  OUT CHAR8           *ManufacturerStr
> > +  )
> > +{
> > +  UINT16                    ManufacturerId;
> > +  UINT32                    i;
>
> Index
>
> > +
> > +  ManufacturerId = SwapBytes16 (SpdManufacturerId);
> > +  ManufacturerId &= 0x7FFF; // ignore odd parity bit
> > +
> > +  for (i = 0; Manufacturer[i].ManufacturerId != 0xFFFF; i++) {
> > +    if (ManufacturerId == Manufacturer[i].ManufacturerId) {
> > +      AsciiStrCpyS (ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN,
> > +                    Manufacturer[i].ManufacturerName);
>
> These return a value on failure. If this code takes user input, you
> should handle the return value. In this case, it only takes SPD data
> as input, so it is sufficient to do something like
>
> RETURN_STATUS RetStatus;
>
> ..
> RetStatus = AsciiStrCpyS (...)
> ASSERT_RETURN_ERROR (RetStatus);
>
> but ignore RetStatus in the code flow.
>
> > +      return;
> > +    }
> > +  }
> > +
> > +  AsciiStrCpyS (ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN,
> > +                Manufacturer[i].ManufacturerName);
>
> Same here
>
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +InstallMemoryDeviceStructure (
> > +  VOID
> > +  )
> > +{
> > +  EFI_SMBIOS_HANDLE         SmbiosHandle;
> > +  ARM_TYPE17                *Descriptor;
> > +  SPD_DDR4                  *Spd;
> > +  UINT8                     Slot;
> > +
> > +  CHAR8                     DeviceLocatorStr[TYPE17_DEVICE_LOCATOR_LEN] = {0};
> > +  CHAR8                     BankLocatorStr[TYPE17_BANK_LOCATOR_LEN] = {0};
> > +  CHAR8                     ManufacturerStr[TYPE17_MANUFACTURER_NAME_LEN] = {0};
> > +  CHAR8                     SerialNumberStr[TYPE17_SERIAL_NUMBER_LEN] = {0};
> > +  CHAR8                     AssetTagStr[] = "Unknown\0";
> > +  CHAR8                     PartNumberStr[TYPE17_MODULE_PART_NUMBER_LEN] = {0};
> > +
>
> Drop the initialisers. If you really need the buffers to be cleared,
> use ZeroMem()
>
> > +  Descriptor = AllocateZeroPool(sizeof (ARM_TYPE17));
> > +  if (Descriptor == NULL) {
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  Spd = (SPD_DDR4 *) FixedPcdGet32 (PcdSmbiosStoredSpdDDR4Address);
> > +
> > +  for (Slot = 0; Slot < 4; Slot++, Spd++) {
> > +    SetMem (Descriptor, sizeof (ARM_TYPE17), 0);
> > +    // fill fixed parameters
> > +    Descriptor->Base.Hdr.Type = EFI_SMBIOS_TYPE_MEMORY_DEVICE;
> > +    Descriptor->Base.Hdr.Length = sizeof (SMBIOS_TABLE_TYPE17);
> > +    Descriptor->Base.Hdr.Handle = SMBIOS_HANDLE_PI_RESERVED;
> > +    Descriptor->Base.MemoryArrayHandle = SMBIOS_HANDLE_MEMORY;
> > +    Descriptor->Base.MemoryErrorInformationHandle = 0xFFFE;
> > +    Descriptor->Base.DeviceSet = 1;
> > +    Descriptor->Base.MemoryType = MemoryTypeDdr4;
> > +    Descriptor->Base.DeviceLocator = 1;
> > +    Descriptor->Base.BankLocator = 2;
> > +    Descriptor->Base.Manufacturer = 3;
> > +    Descriptor->Base.SerialNumber = 4;
> > +    Descriptor->Base.AssetTag = 5;
> > +    Descriptor->Base.PartNumber = 6;
> > +
> > +    if (Spd->Base.Description.Data == 0) {
> > +      // No DIMM inserted, fill the default parameters
> > +      CHAR8                 DefaultStrings[] = "NO DIMM\0NO DIMM\0NO DIMM\0NO DIMM\0NO DIMM\0";
> > +      UINT8                 DefaultStringsLen = 40;
> > +
> > +      Descriptor->Base.TotalWidth = 0xFFFF;
> > +      Descriptor->Base.DataWidth = 0xFFFF;
> > +      Descriptor->Base.Size = 0;
> > +      Descriptor->Base.FormFactor = 0x09;
> > +      Descriptor->Base.DeviceSet = 1;
> > +
> > +      Descriptor->Base.Attributes = 0;
> > +      Descriptor->Base.Speed = 0;
> > +      Descriptor->Base.ConfiguredMemoryClockSpeed = 0;
> > +      Descriptor->Base.MinimumVoltage = 0;
> > +      Descriptor->Base.MaximumVoltage = 0;
> > +      Descriptor->Base.ConfiguredVoltage = 0;
> > +
> > +      AsciiSPrint (Descriptor->Strings, TYPE17_STRINGS_MAX_LEN, "DIMM %d\0", (Slot + 1));
> > +      CopyMem ((Descriptor->Strings + (AsciiStrLen(Descriptor->Strings) + 1)),
> > +              DefaultStrings, DefaultStringsLen);
> > +    } else {
> > +      CHAR8                 *Temp;
> > +
> > +      Descriptor->Base.TotalWidth = Descriptor->Base.DataWidth =
> > +        GetPrimaryBusWidth (Spd->Base.ModuleMemoryBusWidth.Bits.PrimaryBusWidth);
> > +      if (Spd->Base.ModuleMemoryBusWidth.Bits.BusWidthExtension) {
> > +        if (Descriptor->Base.TotalWidth != 0xFFFF) {
> > +          Descriptor->Base.TotalWidth += 8;
> > +        }
> > +      }
> > +
> > +      Descriptor->Base.Size = CaluculateModuleDramCapacityMB ((SPD4_BASE_SECTION *)Spd);
> > +
> > +      switch (Spd->Base.ModuleType.Bits.ModuleType) {
> > +      case SPD4_MEM_MODULE_TYPE_RDIMM:
> > +        Descriptor->Base.FormFactor = 0x09;
> > +        Descriptor->Base.TypeDetail.Registered = 1;
> > +        Descriptor->Base.TypeDetail.Unbuffered = 0;
> > +        break;
> > +      case SPD4_MEM_MODULE_TYPE_UDIMM:
> > +        Descriptor->Base.FormFactor = 0x09;
> > +        Descriptor->Base.TypeDetail.Registered = 0;
> > +        Descriptor->Base.TypeDetail.Unbuffered = 1;
> > +        break;
> > +      case SPD4_MEM_MODULE_TYPE_SODIMM:
> > +        Descriptor->Base.FormFactor = 0x0D;
> > +        Descriptor->Base.TypeDetail.Registered = 0;
> > +        Descriptor->Base.TypeDetail.Unbuffered = 1;
> > +        break;
> > +      default:
> > +        Descriptor->Base.FormFactor = 0x01;
> > +        Descriptor->Base.TypeDetail.Registered = 0;
> > +        Descriptor->Base.TypeDetail.Unbuffered = 0;
> > +        break;
> > +      }
> > +
> > +      Descriptor->Base.Attributes = Spd->Base.ModuleOrganization.Bits.RankCount + 1;
> > +      Descriptor->Base.Speed = 2133;
> > +      Descriptor->Base.ConfiguredMemoryClockSpeed = 2133;
> > +      Descriptor->Base.MinimumVoltage = 1200;
> > +      Descriptor->Base.MaximumVoltage = 1200;
> > +      Descriptor->Base.ConfiguredVoltage = 1200;
> > +
> > +      AsciiSPrint (DeviceLocatorStr, sizeof(DeviceLocatorStr), "DIMM %d\0", (Slot + 1));
> > +      AsciiSPrint (BankLocatorStr, sizeof(BankLocatorStr), "CHANNEL %d SLOT %d\0", (Slot / 2), (Slot % 2));
> > +      GetManufacturerName (Spd->ManufactureInfo.ModuleId.IdCode.Data, ManufacturerStr);
> > +      AsciiSPrint (SerialNumberStr, TYPE17_SERIAL_NUMBER_LEN, "0x%08X\0",
> > +                   SwapBytes32(Spd->ManufactureInfo.ModuleId.SerialNumber.Data));
> > +      //
> > +      // Module part number is not null terminated string in SPD DDR4,
> > +      // unused bytes are filled with 0x20(space).
> > +      //
> > +      CopyMem (PartNumberStr,
> > +               (CHAR8 *)Spd->ManufactureInfo.ModulePartNumber.ModulePartNumber,
> > +               TYPE17_MODULE_PART_NUMBER_LEN - 1);
> > +
> > +      Temp = Descriptor->Strings;
> > +      AsciiStrnCpy (Temp, DeviceLocatorStr, TYPE17_DEVICE_LOCATOR_LEN);
> > +      Temp += (AsciiStrLen (DeviceLocatorStr) + 1);
> > +      AsciiStrnCpy (Temp, BankLocatorStr, TYPE17_BANK_LOCATOR_LEN);
> > +      Temp += (AsciiStrLen (BankLocatorStr) + 1);
> > +      AsciiStrnCpy (Temp, ManufacturerStr, TYPE17_MANUFACTURER_NAME_LEN);
> > +      Temp += (AsciiStrLen (ManufacturerStr) + 1);
> > +      AsciiStrnCpy (Temp, SerialNumberStr, TYPE17_SERIAL_NUMBER_LEN);
> > +      Temp += (AsciiStrLen (SerialNumberStr) + 1);
> > +      AsciiStrnCpy (Temp, AssetTagStr, TYPE17_ASSETTAG_LEN);
> > +      Temp += (AsciiStrLen (AssetTagStr) + 1);
> > +      AsciiStrnCpy (Temp, PartNumberStr, TYPE17_MODULE_PART_NUMBER_LEN);
> > +    }
> > +
> > +    SmbiosHandle = Descriptor->Base.Hdr.Handle;
> > +    mSmbios->Add (mSmbios, NULL, &SmbiosHandle, &Descriptor->Base.Hdr);
> > +  }
> > +
> > +  FreePool (Descriptor);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> >  STATIC
> >  EFI_STATUS
> >  InstallMemoryStructure (
> > @@ -587,6 +826,7 @@ InstallMemoryStructure (
> >    return mSmbios->Add (mSmbios, NULL, &SmbiosHandle, &Descriptor->Base.Hdr);
> >  }
> >
> > +
> >  STATIC
> >  VOID
> >  InstallAllStructures (
> > @@ -608,6 +848,16 @@ InstallAllStructures (
> >      }
> >    }
> >
> > +  //
> > +  // SPD_DDR4 data is stored in Non Secure SRAM by SCP-firmware.
> > +  // Install Type17 record by analyzing SPD_DDR4 information.
> > +  //
> > +  Status = InstallMemoryDeviceStructure();
> > +  if (EFI_ERROR(Status)) {
> > +      DEBUG ((DEBUG_WARN, "%a: failed to add SMBIOS type 17 table - %r\n",
> > +        __FUNCTION__, Status));
> > +  }
> > +
> >    for (Hob.Raw = GetHobList ();
> >         !END_OF_HOB_LIST (Hob);
> >         Hob.Raw = GET_NEXT_HOB (Hob)) {
> > diff --git a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > index e711cbf6dc..abd4602e34 100644
> > --- a/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > +++ b/Platform/Socionext/DeveloperBox/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > @@ -21,6 +21,7 @@
> >    ArmPkg/ArmPkg.dec
> >    MdeModulePkg/MdeModulePkg.dec
> >    MdePkg/MdePkg.dec
> > +  Silicon/Socionext/SynQuacer/SynQuacer.dec
> >
> >  [LibraryClasses]
> >    BaseMemoryLib
> > @@ -34,6 +35,7 @@
> >  [FixedPcd]
> >    gArmTokenSpaceGuid.PcdFdSize
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
> > +  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address
> >
> >  [Protocols]
> >    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> > diff --git a/Silicon/Socionext/SynQuacer/SynQuacer.dec b/Silicon/Socionext/SynQuacer/SynQuacer.dec
> > index e7197e2319..98b574bd32 100644
> > --- a/Silicon/Socionext/SynQuacer/SynQuacer.dec
> > +++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec
> > @@ -43,6 +43,9 @@
> >    # for capsule update
> >    gSynQuacerTokenSpaceGuid.PcdLowestSupportedFirmwareVersion|1|UINT32|0x00000009
> >
> > +  # for SMBIOS Type17
> > +  gSynQuacerTokenSpaceGuid.PcdSmbiosStoredSpdDDR4Address|0|UINT32|0x0000000A
> > +
> >  [PcdsPatchableInModule, PcdsDynamic]
> >    # Enable both RC #0 and RC #1 by default
> >    gSynQuacerTokenSpaceGuid.PcdPcieEnableMask|0x3|UINT8|0x00000007
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-12  5:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-11 12:17 [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table Masahisa Kojima
2019-06-11 13:00 ` Ard Biesheuvel
2019-06-12  5:23   ` Masahisa Kojima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox