[AMD Official Use Only - General]
> -----Original Message-----
> From: Sami Mujawar <sami.muja...@arm.com>
> Sent: Wednesday, September 14, 2022 11:35 PM
> To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io; Girish
> Mahadevan <gmahade...@nvidia.com>; Alexei Fedorov
> <alexei.fedo...@arm.com>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>; Jeff Brasen
> (jbra...@nvidia.com) <jbra...@nvidia.com>; Ashish Singhal
> <ashishsin...@nvidia.com>; Akanksha Jain <akanksha.ja...@arm.com>;
> Matteo Carlini <matteo.carl...@arm.com>; Hemendra Dassanayake
> <hemendra.dassanay...@arm.com>; Nick Ramirez <nrami...@nvidia.com>;
> William Watson <wwat...@nvidia.com>; nd <n...@arm.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] DynamicTablesPkg: Add Smbios Type17
> Table generator
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> Please see my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 13/09/2022, 04:00, "Chang, Abner" <abner.ch...@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> One question in below with tag [Abner],
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami
> > Mujawar via groups.io
> > Sent: Monday, September 12, 2022 10:57 PM
> > To: Girish Mahadevan <gmahade...@nvidia.com>; devel@edk2.groups.io;
> > Alexei Fedorov <alexei.fedo...@arm.com>
> > Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>; Jeff
> > Brasen <jbra...@nvidia.com>; Ashish Singhal <ashishsin...@nvidia.com>;
> > Akanksha Jain <akanksha.ja...@arm.com>; Matteo Carlini
> > <matteo.carl...@arm.com>; Hemendra Dassanayake
> > <hemendra.dassanay...@arm.com>; Nick Ramirez
> <nrami...@nvidia.com>;
> > William Watson <wwat...@nvidia.com>; Akanksha Jain
> > <akanksha.ja...@arm.com>; n...@arm.com
> > Subject: Re: [edk2-devel] [PATCH 2/2] DynamicTablesPkg: Add Smbios
> > Type17 Table generator
> ...
>
> > > +STATIC
> > > +EFI_STATUS
> > > +EFIAPI
> > > +FreeSmbiosType17TableEx (
> > > + IN CONST SMBIOS_TABLE_GENERATOR *CONST
> This,
> > > + IN CONST CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST
> > SmbiosTableInfo,
> > > + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> > CfgMgrProtocol,
> > > + IN OUT SMBIOS_STRUCTURE ***CONST
> Table,
> > > + IN CONST UINTN
> TableCount
> > > + )
> > > +{
> > > + return EFI_SUCCESS;
> > > +}
> > > +
> > > +/** Construct SMBIOS Type17 Table describing memory devices.
> > > +
> > > + If this function allocates any resources then they must be freed
> > > + in the FreeXXXXTableResources function.
> > > +
> > > + @param [in] This Pointer to the SMBIOS table generator.
> > > + @param [in] SmbiosTableInfo Pointer to the SMBIOS table
> information.
> > > + @param [in] CfgMgrProtocol Pointer to the Configuration Manager
> > > + Protocol interface.
> > > + @param [out] Table Pointer to the SMBIOS table.
> > > +
> > > + @retval EFI_SUCCESS Table generated successfully.
> > > + @retval EFI_BAD_BUFFER_SIZE The size returned by the
> Configuration
> > > + Manager is less than the Object
> size for
> > > + the requested object.
> > > + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> > > + @retval EFI_NOT_FOUND Could not find information.
> > > + @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
> > > + @retval EFI_UNSUPPORTED Unsupported configuration.
> > > +**/
> > > +STATIC
> > > +EFI_STATUS
> > > +EFIAPI
> > > +BuildSmbiosType17TableEx (
> > > + IN CONST SMBIOS_TABLE_GENERATOR *This,
> > > + IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST
> > SmbiosTableInfo,
> > > + IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
> > CfgMgrProtocol,
> > > + OUT SMBIOS_STRUCTURE ***Table,
> > > + OUT UINTN *CONST TableCount
> > > + )
> > > +{
> > > + EFI_STATUS Status;
> > > + UINT32 NumMemDevices;
> > > + SMBIOS_STRUCTURE **TableList;
> > > + CM_ARM_MEMORY_DEVICE_INFO *MemoryDevicesInfo;
> > > + UINTN Index;
> > > + UINTN SerialNumLen;
> > > + CHAR8 *SerialNum;
> > > + UINTN AssetTagLen;
> > > + CHAR8 *AssetTag;
> > > + UINTN DeviceLocatorLen;
> > > + CHAR8 *DeviceLocator;
> > > + UINTN BankLocatorLen;
> > > + CHAR8 *BankLocator;
> > > + UINTN FirmwareVersionLen;
> > > + CHAR8 *FirmwareVersion;
> > > + CHAR8 *OptionalStrings;
> > > + SMBIOS_TABLE_TYPE17 *SmbiosRecord;
> > > +
> > > + ASSERT (This != NULL);
> > > + ASSERT (SmbiosTableInfo != NULL);
> > > + ASSERT (CfgMgrProtocol != NULL);
> > > + ASSERT (Table != NULL);
> > > + ASSERT (TableCount != NULL);
> > > + ASSERT (SmbiosTableInfo->TableGeneratorId == This->GeneratorID);
> > > +
> > > + DEBUG ((DEBUG_ERROR, "%a : Start \n", __FUNCTION__)); *Table =
> > > + NULL; Status = GetEArmObjMemoryDeviceInfo (
> > > + CfgMgrProtocol,
> > > + CM_NULL_TOKEN,
> > > + &MemoryDevicesInfo,
> > > + &NumMemDevices
> > > + );
> [Abner]
> SMBIOS type 17 record is generic to all platform architectures, however
> here
> we have the dependency with ARM namespace object.
> [SAMI] It would certainly be very good to have a common codebase across
> architectures. We welcome contribution from community members towards this
> effort.
> So my question is what should we do if non-ARM platforms would like to
> leverage this library?
> [SAMI] I think we could define the SMBIOS specific objects in a separate
> namespace ID e.g. 1010b - SMBIOS Objects , see
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
> om%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FDynamicTablesPkg%2FInclude
> %2FConfigurationManagerObject.h%23L30-
> L34&data=05%7C01%7CAbner.Chang%40amd.com%7C425be8c5f1464596
> 4ada08da9666aa5f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
> 37987664940879922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&am
> p;sdata=trKkXJe7yDBkoKIlSk5kCI0tZ6nG573wXqORHfd5y2o%3D&reserved=
> 0
> We can then define the SMBIOS objects as SMBIOS namespace objects.
[Abner]
This sounds good. We can define SMBIOS name space and the corresponding object
in SmbiosNameSpaceObjects.h under \Include. So platform has to install
EDKII_CONFIGURATION_MANAGER_PROTOCOL and returns the SMBIOS object, right?
> However, I would like to avoid duplicating any information between the ARM
> namespace objects and SMBIOS namespace objects (e.g. information about CPU,
> Cache, etc.).
> I have some initial thoughts on how this could be done by introducing an
> object
> mapper.
[Abner]
I think you refer to have SMBIOS namespace object as the generic one, but
introduce a wrapper on it for the processor arch specific information?
However, I would first like to understand if you intend to use the
> Dynamic SMBIOS support only or you will use the Dynamic ACPI support as well?
[ Abner]
I am not sure if we will also use Dynamic ACPI table at the moment but that
would be possible. The current use case I can think of is to build up SMBIOS
42h using dynamic SMBIOS table generator.
This also brings another question, the current DynamicTableFactoryDxe pulls in
ACPI/SMBIOS/DT table generator functions; what if the platform only uses SMBIOS
functionality? Pull in ACPI/DT code into DynamicTableFactoryDxe seems
redundant. I think we can consider to make
AcpiTableFactory/DeviceTreeTableFactory/SmbiosTableFactor as libraries, use
NULL instances (empty function for Get/Register/Deregister generator) as
default for DynamicTableFactoryDxe. Platform can pull in the library that has
the implementation into build on demand. Not for now but we can do it later if
you think this makes sense.
BTW what is the 'E' prefix to "Arm" means? E.g. Get'E'ArmObjMemoryDeviceInfo
Thanks
Abner
> [/SAMI]
> Thanks
> Abner
>
> > > + if (EFI_ERROR (Status)) {
> > > + DEBUG ((
> > > + DEBUG_ERROR,
> > > + "Failed to get Memory Devices CM Object %r\n",
> > > + Status
> > > + ));
> > > + return Status;
> > > + }
> ...
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93839): https://edk2.groups.io/g/devel/message/93839
Mute This Topic: https://groups.io/mt/93275299/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
