From: "Masahisa Kojima" <masahisa.kojima@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@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: Wed, 12 Jun 2019 14:23:17 +0900 [thread overview]
Message-ID: <CADQ0-X91k=OqGt3F0bhSOf5Bdq3N9pHpmVKuhnyC66oTdGE8wg@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu_fXvp-txhdM57L1xeY74jOeTKMy2UPkSPGNbOUFbf-cw@mail.gmail.com>
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
> >
prev parent reply other threads:[~2019-06-12 5:23 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
2019-06-12 5:23 ` Masahisa Kojima [this message]
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='CADQ0-X91k=OqGt3F0bhSOf5Bdq3N9pHpmVKuhnyC66oTdGE8wg@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