[edk2-devel] [PATCH v6 15/22] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type00

[Samer El-Haj-Mahmoud]

With a minor feedback below:
Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>

+  }
> +
> +  (VOID)CopyMem (SmbiosRecord, InputData, sizeof (SMBIOS_TABLE_TYPE0));
> +
> +  SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE0);
> + SmbiosRecord->BiosSegment = (UINT16)(FixedPcdGet32 (PcdFdBaseAddress)
> + / SIZE_64KB);  if (BiosPhysicalSize < SIZE_16MB) {
> +    SmbiosRecord->BiosSize = Base2ToByteWith64KUnit (BiosPhysicalSize) - 1;
> +    SmbiosRecord->ExtendedBiosSize.Size = BiosPhysicalSize / SIZE_1MB;
> +    SmbiosRecord->ExtendedBiosSize.Unit = 0; // Size is in MB  } else {
> +    SmbiosRecord->BiosSize = 0xFF;
> +    if (BiosPhysicalSize > 0x3FFF) {
> +      SmbiosRecord->ExtendedBiosSize.Size = BiosPhysicalSize / SIZE_1GB;
> +      SmbiosRecord->ExtendedBiosSize.Unit = 1; // Size is in GB
> +    }
> +  }
> +
> +  SmbiosRecord->SystemBiosMajorRelease = (UINT8) (PcdGet16
> + (PcdSystemBiosRelease) >> 8);  SmbiosRecord->SystemBiosMinorRelease =
> + (UINT8) (PcdGet16 (PcdSystemBiosRelease) & 0xFF);
> +
> +  SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)
> +    (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)
> +    (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
> +

Not really an issue, but a function in OemMiscLib is possibly a better option than using PCDs for these. Platforms may have an easy way to build the Major.Minor FW values (such as by parsing PcdFirmwareVersionString and splitting into major and minor values). See for example for RPi4 at: https://github.com/tianocore/edk2-platforms/blob/master/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c#L754
Using PCDs will still work, but the platform will need to put this code in some other module/library just to initialize the dynamic PCD.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70457): https://edk2.groups.io/g/devel/message/70457
Mute This Topic: https://groups.io/mt/79679257/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-