Re: [edk2-devel] [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table
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.
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|0x2E00
> - 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_RDIMM0x01
> +#define SPD4_MEM_MODULE_TYPE_UDIMM0x02
> +#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
Re: [edk2-devel] [edk2-platforms PATCH] Platform/Socionext/DeveloperBox: add SMBIOS type 17 table
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|0x2E00
> > - 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_RDIMM0x01
> > +#define SPD4_MEM_MODULE_TYPE_UDIMM0x02
> > +#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 JEP10
