From: "Ard Biesheuvel" <ard.biesheuvel@linaro.org>
To: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table
Date: Tue, 11 Jun 2019 15:00:57 +0200 [thread overview]
Message-ID: <CAKv+Gu_fXvp-txhdM57L1xeY74jOeTKMy2UPkSPGNbOUFbf-cw@mail.gmail.com> (raw)
In-Reply-To: <20190611121750.3882-1-masahisa.kojima@linaro.org>
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
>
next prev parent reply other threads:[~2019-06-11 13:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2019-06-12 5:23 ` Masahisa Kojima
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu_fXvp-txhdM57L1xeY74jOeTKMy2UPkSPGNbOUFbf-cw@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox