[edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version

[Michael D Kinney]

I was asking about the property of the global variables
being used in this patch.  Are they already guaranteed to
be in BSD format and in range 0..9.  If so, then no additional
code changes would be required.  However, it would be good 
to add comments about the properties of those global variables
and why they can be used to directly assign to fields that
are required to be in BSD format.

Mike

> -----Original Message-----
> From: Lien, HoraceX <horacex.lien at intel.com>
> Sent: Thursday, September 7, 2023 2:41 AM
> To: devel at edk2.groups.io; Kinney, Michael D <michael.d.kinney at intel.com>;
> Gao, Liming <gaoliming at byosoft.com.cn>
> Cc: Liu, Zhiguang <zhiguang.liu at intel.com>; Bi, Dandan
> <dandan.bi at intel.com>; Zeng, Star <star.zeng at intel.com>; Gao, Zhichao
> <zhichao.gao at intel.com>; Lien, HoraceX <horacex.lien at intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
> 
> Hi Mike,
> 
> Could you please reply for me?
> If you want to filter range 0-9, then I will send PR again.
> 
> Thanks,
> Horace Lien
> 
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Lien,
> HoraceX
> Sent: Friday, September 1, 2023 3:06 PM
> To: Kinney, Michael D <michael.d.kinney at intel.com>; devel at edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu at intel.com>; Bi, Dandan
> <dandan.bi at intel.com>; Zeng, Star <star.zeng at intel.com>; Gao, Zhichao
> <zhichao.gao at intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
> 
> Hi Mike,
> 
> I have change code to
> EntryPointStructureData.SmbiosBcdRevision =
> ((mPrivateData.Smbios.MajorVersion & 0x0f) << 4) |
> (mPrivateData.Smbios.MinorVersion & 0x0f); Add &0x0F to mask upper nibble
> bit, do we still need to guarantee that range is between 0-9? Because the
> old code only filtered 4 bits, instead of accurately filtering the number
> range 0-9.
> 
> Thanks,
> Horace Lien
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney at intel.com>
> Sent: Thursday, August 31, 2023 11:56 PM
> To: devel at edk2.groups.io; Lien, HoraceX <horacex.lien at intel.com>
> Cc: Liu, Zhiguang <zhiguang.liu at intel.com>; Bi, Dandan
> <dandan.bi at intel.com>; Zeng, Star <star.zeng at intel.com>; Gao, Zhichao
> <zhichao.gao at intel.com>; Kinney, Michael D <michael.d.kinney at intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
> 
> Are mPrivateData.Smbios.MajorVersion and mPrivateData.Smbios.MinorVersion
> guaranteed to be in range 0..9?
> 
> Mike
> 
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > horacex.lien at intel.com
> > Sent: Wednesday, August 30, 2023 2:13 AM
> > To: devel at edk2.groups.io
> > Cc: Lien, HoraceX <horacex.lien at intel.com>; Liu, Zhiguang
> > <zhiguang.liu at intel.com>; Bi, Dandan <dandan.bi at intel.com>; Zeng, Star
> > <star.zeng at intel.com>; Gao, Zhichao <zhichao.gao at intel.com>
> > Subject: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> > is not match with SMBIOS version
> >
> > From: HoraceX Lien <horacex.lien at intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4544
> >
> > These value of Major/Minor version are updated from SMBIOS memory
> > data, but BCD Revision is updated from PCD PcdSmbiosVersion.
> > We should also update PCD PcdSmbiosVersion from SMBIOS memory data, to
> > ensure that get consistent version value.
> >
> > Cc: Zhiguang Liu <zhiguang.liu at intel.com>
> > Cc: Dandan Bi <dandan.bi at intel.com>
> > Cc: Star Zeng <star.zeng at intel.com>
> > Cc: Zhichao Gao <zhichao.gao at intel.com>
> > Signed-off-by: HoraceX Lien <horacex.lien at intel.com>
> > ---
> >  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > index 1a86e69d3c..e3f6215033 100644
> > --- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > +++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
> > @@ -1072,7 +1072,7 @@ SmbiosCreateTable (
> >      DEBUG ((DEBUG_INFO, "SmbiosCreateTable: Initialize 32-bit entry
> > point structure\n"));
> >
> >      EntryPointStructureData.MajorVersion      =
> > mPrivateData.Smbios.MajorVersion;
> >
> >      EntryPointStructureData.MinorVersion      =
> > mPrivateData.Smbios.MinorVersion;
> >
> > -    EntryPointStructureData.SmbiosBcdRevision = (UINT8)((PcdGet16
> > (PcdSmbiosVersion) >> 4) & 0xf0) | (UINT8)(PcdGet16 (PcdSmbiosVersion)
> > & 0x0f);
> >
> > +    EntryPointStructureData.SmbiosBcdRevision =
> > (mPrivateData.Smbios.MajorVersion << 4) |
> > mPrivateData.Smbios.MinorVersion;
> >
> >      PhysicalAddress                           = 0xffffffff;
> >
> >      Status                                    = gBS->AllocatePages (
> >
> >
> > AllocateMaxAddress,
> >
> > --
> > 2.31.1.windows.1
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#108150):
> > https://edk2.groups.io/g/devel/message/108150
> > Mute This Topic: https://groups.io/mt/101057293/1643496
> > Group Owner: devel+owner at edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [michael.d.kinney at intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 



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