Hi Mike,

No, we didn't guarantee this before. Add comments to descript BCD field is good 
point.
I have reviewed SMBIOS spec for SmbiosBcdRevision field, it mentions "If the 
value is 00h, only the Major and Minor Versions in offsets 6 and 7 of the Entry 
Point Structure provide the version information. ". So, I have new idea to 
implement this, I will filter range 0-9 for Major/Minor version to fill in 
SmbiosBcdRevision, if one of Major or Minor is greater than 9 then fill in 00h.
Do you think it is ok?
Thanks for your reply.

Thanks,
Horace Lien

-----Original Message-----
From: Kinney, Michael D <michael.d.kin...@intel.com> 
Sent: Friday, September 8, 2023 6:05 AM
To: Lien, HoraceX <horacex.l...@intel.com>; devel@edk2.groups.io; Gao, Liming 
<gaolim...@byosoft.com.cn>
Cc: Liu, Zhiguang <zhiguang....@intel.com>; Bi, Dandan <dandan...@intel.com>; 
Zeng, Star <star.z...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; Kinney, 
Michael D <michael.d.kin...@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is 
not match with SMBIOS version

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.l...@intel.com>
> Sent: Thursday, September 7, 2023 2:41 AM
> To: devel@edk2.groups.io; Kinney, Michael D 
> <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Bi, Dandan 
> <dandan...@intel.com>; Zeng, Star <star.z...@intel.com>; Gao, Zhichao 
> <zhichao....@intel.com>; Lien, HoraceX <horacex.l...@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lien, 
> HoraceX
> Sent: Friday, September 1, 2023 3:06 PM
> To: Kinney, Michael D <michael.d.kin...@intel.com>; 
> devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Bi, Dandan 
> <dandan...@intel.com>; Zeng, Star <star.z...@intel.com>; Gao, Zhichao 
> <zhichao....@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.kin...@intel.com>
> Sent: Thursday, August 31, 2023 11:56 PM
> To: devel@edk2.groups.io; Lien, HoraceX <horacex.l...@intel.com>
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Bi, Dandan 
> <dandan...@intel.com>; Zeng, Star <star.z...@intel.com>; Gao, Zhichao 
> <zhichao....@intel.com>; Kinney, Michael D 
> <michael.d.kin...@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > horacex.l...@intel.com
> > Sent: Wednesday, August 30, 2023 2:13 AM
> > To: devel@edk2.groups.io
> > Cc: Lien, HoraceX <horacex.l...@intel.com>; Liu, Zhiguang 
> > <zhiguang....@intel.com>; Bi, Dandan <dandan...@intel.com>; Zeng, 
> > Star <star.z...@intel.com>; Gao, Zhichao <zhichao....@intel.com>
> > Subject: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix 
> > BcdRevision is not match with SMBIOS version
> >
> > From: HoraceX Lien <horacex.l...@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....@intel.com>
> > Cc: Dandan Bi <dandan...@intel.com>
> > Cc: Star Zeng <star.z...@intel.com>
> > Cc: Zhichao Gao <zhichao....@intel.com>
> > Signed-off-by: HoraceX Lien <horacex.l...@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+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [michael.d.kin...@intel.com]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108442): https://edk2.groups.io/g/devel/message/108442
Mute This Topic: https://groups.io/mt/101057293/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to