From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Guo, Gua" <gua.guo@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Lien, HoraceX" <horacex.lien@intel.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>
Cc: "Liu, Zhiguang" <zhiguang.liu@intel.com>,
"Bi, Dandan" <dandan.bi@intel.com>,
"Zeng, Star" <star.zeng@intel.com>,
"Gao, Zhichao" <zhichao.gao@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Date: Wed, 20 Sep 2023 01:35:19 +0000 [thread overview]
Message-ID: <CO1PR11MB492903D70C6A0EC95A6A1D29D2F9A@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <BL1PR11MB54785F12319DB28DA2ECAEDAEFF9A@BL1PR11MB5478.namprd11.prod.outlook.com>
[-- Attachment #1.1: Type: text/plain, Size: 14962 bytes --]
What is the difference between "version of the specification implemented in table structures" and "compliance with a revision of this specification"?
Is it really a spec violation for them to be difference values?
Mike
From: Guo, Gua <gua.guo@intel.com>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Lien, HoraceX <horacex.lien@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Hi Mike
May I get your comment and hope it can eliminate your concern ?
Question1: Let's back why this change need to do the enhance.
From
> EntryPointStructureData.MajorVersion = mPrivateData.Smbios.MajorVersion;
> EntryPointStructureData.MinorVersion = mPrivateData.Smbios.MinorVersion;
> EntryPointStructureData.SmbiosBcdRevision = (UINT8)((PcdGet16 (PcdSmbiosVersion) >> 4) & 0xf0) | (UINT8)(PcdGet16 (PcdSmbiosVersion) & 0x0f);
To
> EntryPointStructureData.MajorVersion = mPrivateData.Smbios.MajorVersion;
> EntryPointStructureData.MinorVersion = mPrivateData.Smbios.MinorVersion;
> EntryPointStructureData.SmbiosBcdRevision = ((mPrivateData.Smbios.MajorVersion & 0x0f) << 4) | (mPrivateData.Smbios.MinorVersion & 0x0f);
Answer1:
If mPrivateData.Smbios.MajorVersion equal to 3h and mPrivateData.Smbios.MajorVersion equal to 4h means (Spec Version 3.4), but PcdSmbiosVersion = 0305h will cause SmbiosBcdRevision become 35h (Spec Version 3.5).
3.4 != 3.5 violate SMBIOS spec, so I think the change is reasonable to prevent this case happening.
Question2: Forcing to 0 does not sound right.
Answer2: I think it maybe reasonable based on below description.
Ref to SMBIOS Spec: https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.0.pdf -> Section 5.2.1
SMBIOS Spec mention on MajorVersion (Offset 06h) = 0Ah and MinorVersion (Offset 07h) = 11 range have chance 10.22, this example is out of BCD format
[cid:image001.png@01D9EB28.07C94370]
For this case, Spec have allow to set to 00h to ignore the SMBIOS BCD Revision (Offset 1Eh), when it happen, only consider SMBIOS Major Version (Offset 06h) and SMBIOS Minor Version (Offset 07h).
[cid:image002.png@01D9EB28.07C94370]
Thanks,
Gua
-----Original Message-----
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Michael D Kinney
Sent: Friday, September 15, 2023 1:48 AM
To: Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Forcing to 0 does not sound right.
You did not answer my question about the property of the global variables.
Without knowing the format of the information in the global variables you cannot safely use them. If they are in BCD then no need to check for out of range. If they are hex values, then you have to use conversion functions.
Mike
> -----Original Message-----
> From: Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@intel.com>>
> Sent: Thursday, September 14, 2023 2:33 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>;
> devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan
> <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> BcdRevision is not match with SMBIOS version
>
> Hi Mike,
>
> https://github.com/tianocore/edk2/pull/4771
> I have changed code following rule: It is only accept range 0-9 for
> Major and Minor version to fill in SmbiosBcdRevision, if one of Major
> or Minor is greater than 9 then fill in 00h.
>
> Please help to review this, thanks :)
>
> Thanks,
> Horace Lien
>
> -----Original Message-----
> From: Lien, HoraceX
> Sent: Friday, September 8, 2023 5:35 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>;
> devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> Cc: Liu, Zhiguang <Zhiguang.Liu@intel.com<mailto:Zhiguang.Liu@intel.com>>; Bi, Dandan
> <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> BcdRevision is not match with SMBIOS version
>
> 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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Sent: Friday, September 8, 2023 6:05 AM
> To: Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Gao,
> Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan
> <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@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.lien@intel.com<mailto:horacex.lien@intel.com>>
> > Sent: Thursday, September 7, 2023 2:41 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan
> > <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao,
> > Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Lien, HoraceX
> > <horacex.lien@intel.com<mailto:horacex.lien@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<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Lien,
> > HoraceX
> > Sent: Friday, September 1, 2023 3:06 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>;
> > devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan
> > <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao,
> > Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@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@intel.com<mailto:michael.d.kinney@intel.com>>
> > Sent: Thursday, August 31, 2023 11:56 PM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@intel.com>>
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan
> > <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao,
> > Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@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<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of
> > > horacex.lien@intel.com<mailto:horacex.lien@intel.com>
> > > Sent: Wednesday, August 30, 2023 2:13 AM
> > > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > > Cc: Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@intel.com>>; Liu, Zhiguang
> > > <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Bi, Dandan <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>; Zeng,
> > > Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
> > > Subject: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> > > BcdRevision is not match with SMBIOS version
> > >
> > > From: HoraceX Lien <horacex.lien@intel.com<mailto:horacex.lien@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@intel.com<mailto:zhiguang.liu@intel.com>>
> > > Cc: Dandan Bi <dandan.bi@intel.com<mailto:dandan.bi@intel.com>>
> > > Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
> > > Signed-off-by: HoraceX Lien <horacex.lien@intel.com<mailto:horacex.lien@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@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > [michael.d.kinney@intel.com]
> > > -=-=-=-=-=-=
> > >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108893): https://edk2.groups.io/g/devel/message/108893
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 36411 bytes --]
[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 64751 bytes --]
next prev parent reply other threads:[~2023-09-20 1:35 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-30 9:12 [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version horacex.lien
2023-08-31 15:56 ` Michael D Kinney
2023-09-01 7:06 ` Lien, HoraceX
[not found] ` <1780B48BF009A270.23790@groups.io>
2023-09-07 9:40 ` Lien, HoraceX
2023-09-07 22:05 ` Michael D Kinney
2023-09-08 9:35 ` Lien, HoraceX
2023-09-14 9:32 ` Lien, HoraceX
2023-09-14 17:48 ` Michael D Kinney
2023-09-15 7:35 ` Lien, HoraceX
2023-09-20 1:03 ` Guo, Gua
2023-09-20 1:35 ` Michael D Kinney [this message]
2023-09-20 1:52 ` Guo, Gua
[not found] ` <17867875CCCD5E00.29111@groups.io>
2023-09-22 5:10 ` Guo, Gua
2023-09-22 16:19 ` Michael D Kinney
2023-09-23 0:21 ` Guo, Gua
2023-09-23 3:06 ` Michael D Kinney
2023-09-23 3:09 ` Guo, Gua
2023-09-25 2:13 ` Lien, HoraceX
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB492903D70C6A0EC95A6A1D29D2F9A@CO1PR11MB4929.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox