* [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