From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=BYhC26kc; spf=pass (domain: linaro.org, ip: 209.85.208.195, mailfrom: masahisa.kojima@linaro.org) Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) by groups.io with SMTP; Tue, 11 Jun 2019 22:23:33 -0700 Received: by mail-lj1-f195.google.com with SMTP id v24so9550810ljg.13 for ; Tue, 11 Jun 2019 22:23:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KChKGR0UueDwLyS453O8HQ7SonFYqZxKEu/ZaH9Lf4s=; b=BYhC26kcJQ9MCEGlE1oHSUtNl6zm4vbjsZhI5Dewp4RzMRpKXUST43r9PfP+GTtKEv DgFLzzQ7AehUyxmhu8D2Fbvh+8ZY+KuAP6QQr79Cvp3Hn4iEt7c543/Ws7VwxrsYJA2b 6i0dlfB9TnUBUC+wZl0qiJ8BqotvD1AT1lqY4mWC1T7EW2yfTtNnof0IbEgHRpjgAN9S npujBjZ6/ipQ2i0hSzt58Ss7ZIn6stM5rPlLsibmSXletfKsRpQmqi5jHVsqXt9dkfXB JzMjZg1DIwlA8JniC1fSOZvps9kOqFotgDoIwFBVBJ12vPJl4WZW/IX+CngavLQmCQ3o Sr2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KChKGR0UueDwLyS453O8HQ7SonFYqZxKEu/ZaH9Lf4s=; b=bOutcBUGuRd+gPON2v25zTb+V/UqEIEWFfrbX4p8iVIXmR0ckGeWFsLceZE8DEdGMa Dts5PZYFvzIoIxfdDD+IklNXztDBJfBpV0etX0BJ3Rg3+Jaj4ywkpA2ue8j+RTZkUwDi X6wCxO96zUy7O8CeGZyzxhQ6Hh56a2BOPBotlcdJ//qOyD/TKuk5tTClYvf1XGGWiSoP dmWWAHby9eGTbNXptmkSpUuqgt1czPfSY6BxjQt74v8u7cmSEkG6oQNZOOR8G5whEhQR wLq2VZY466ft840xN3ztR0NXszMZ+nZBKisRH0b86smiGz+w2y/osKsk3aZ80clbdb4f Xlrg== X-Gm-Message-State: APjAAAVV/AAV5CjjAyMeqHj5XyonVvLZOCwzFYUBd4T8MIrWklE9XZ0k 9I9FQsIIsGvdkjCaVQhj0TKLTV+edLnpHNkm5CPeMA== X-Google-Smtp-Source: APXvYqyFq1CO9xLr9j6iyUIY7uZbCjpQnKqfb+BwdHzOjhHFIn8LKIbVac9BH/g/ylsnbSa66BR8aGijkVOh4723YNQ= X-Received: by 2002:a2e:814e:: with SMTP id t14mr10905290ljg.167.1560317011154; Tue, 11 Jun 2019 22:23:31 -0700 (PDT) MIME-Version: 1.0 References: <20190611121750.3882-1-masahisa.kojima@linaro.org> In-Reply-To: From: "Masahisa Kojima" Date: Wed, 12 Jun 2019 14:23:17 +0900 Message-ID: Subject: Re: [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table To: Ard Biesheuvel Cc: edk2-devel-groups-io , Leif Lindholm Content-Type: text/plain; charset="UTF-8" Hi Ard, On Tue, 11 Jun 2019 at 22:01, Ard Biesheuvel wrote: > > On Tue, 11 Jun 2019 at 14:18, Masahisa Kojima > 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 > > 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 > > +#include > > #include > > #include > > #include > > #include > > #include > > #include > > +#include > > #include > > #include > > > > 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 > >