* [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
@ 2023-08-30 9:12 horacex.lien
2023-08-31 15:56 ` Michael D Kinney
0 siblings, 1 reply; 18+ messages in thread
From: horacex.lien @ 2023-08-30 9:12 UTC (permalink / raw)
To: devel; +Cc: HoraceX Lien, Zhiguang Liu, Dandan Bi, Star Zeng, Zhichao Gao
From: HoraceX Lien <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>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: HoraceX Lien <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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
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>
0 siblings, 2 replies; 18+ messages in thread
From: Michael D Kinney @ 2023-08-31 15:56 UTC (permalink / raw)
To: devel@edk2.groups.io, Lien, HoraceX
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Kinney, Michael D
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.lien@intel.com
> Sent: Wednesday, August 30, 2023 2:13 AM
> To: devel@edk2.groups.io
> Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not
> match with SMBIOS version
>
> From: HoraceX Lien <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>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: HoraceX Lien <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
> 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 (#108195): https://edk2.groups.io/g/devel/message/108195
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-08-31 15:56 ` Michael D Kinney
@ 2023-09-01 7:06 ` Lien, HoraceX
[not found] ` <1780B48BF009A270.23790@groups.io>
1 sibling, 0 replies; 18+ messages in thread
From: Lien, HoraceX @ 2023-09-01 7:06 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
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>
Sent: Thursday, August 31, 2023 11:56 PM
To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
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
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.lien@intel.com
> Sent: Wednesday, August 30, 2023 2:13 AM
> To: devel@edk2.groups.io
> Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
>
> From: HoraceX Lien <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>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: HoraceX Lien <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
> 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 (#108227): https://edk2.groups.io/g/devel/message/108227
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
[not found] ` <1780B48BF009A270.23790@groups.io>
@ 2023-09-07 9:40 ` Lien, HoraceX
2023-09-07 22:05 ` Michael D Kinney
0 siblings, 1 reply; 18+ messages in thread
From: Lien, HoraceX @ 2023-09-07 9:40 UTC (permalink / raw)
To: devel@edk2.groups.io, Kinney, Michael D, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Lien, HoraceX
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.kinney@intel.com>; devel@edk2.groups.io
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,
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>
Sent: Thursday, August 31, 2023 11:56 PM
To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
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
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.lien@intel.com
> Sent: Wednesday, August 30, 2023 2:13 AM
> To: devel@edk2.groups.io
> Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> is not match with SMBIOS version
>
> From: HoraceX Lien <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>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: HoraceX Lien <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
> 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 (#108370): https://edk2.groups.io/g/devel/message/108370
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-07 9:40 ` Lien, HoraceX
@ 2023-09-07 22:05 ` Michael D Kinney
2023-09-08 9:35 ` Lien, HoraceX
0 siblings, 1 reply; 18+ messages in thread
From: Michael D Kinney @ 2023-09-07 22:05 UTC (permalink / raw)
To: Lien, HoraceX, devel@edk2.groups.io, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Kinney, Michael D
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>
> Sent: Thursday, September 7, 2023 2:41 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@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>; Lien, HoraceX <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 <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>; devel@edk2.groups.io
> 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,
>
> 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>
> Sent: Thursday, August 31, 2023 11:56 PM
> To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
> 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
>
> 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.lien@intel.com
> > Sent: Wednesday, August 30, 2023 2:13 AM
> > To: devel@edk2.groups.io
> > Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision
> > is not match with SMBIOS version
> >
> > From: HoraceX Lien <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>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: HoraceX Lien <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
> > 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 (#108425): https://edk2.groups.io/g/devel/message/108425
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-07 22:05 ` Michael D Kinney
@ 2023-09-08 9:35 ` Lien, HoraceX
2023-09-14 9:32 ` Lien, HoraceX
0 siblings, 1 reply; 18+ messages in thread
From: Lien, HoraceX @ 2023-09-08 9:35 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
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>
Sent: Friday, September 8, 2023 6:05 AM
To: Lien, HoraceX <horacex.lien@intel.com>; devel@edk2.groups.io; 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
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>
> Sent: Thursday, September 7, 2023 2:41 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@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>; Lien, HoraceX <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 <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>;
> devel@edk2.groups.io
> 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,
>
> 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>
> Sent: Thursday, August 31, 2023 11:56 PM
> To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
> 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
>
> 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.lien@intel.com
> > Sent: Wednesday, August 30, 2023 2:13 AM
> > To: devel@edk2.groups.io
> > Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> > BcdRevision is not match with SMBIOS version
> >
> > From: HoraceX Lien <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>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: HoraceX Lien <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
> > 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 (#108442): https://edk2.groups.io/g/devel/message/108442
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-08 9:35 ` Lien, HoraceX
@ 2023-09-14 9:32 ` Lien, HoraceX
2023-09-14 17:48 ` Michael D Kinney
0 siblings, 1 reply; 18+ messages in thread
From: Lien, HoraceX @ 2023-09-14 9:32 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
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>; devel@edk2.groups.io; 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,
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>
Sent: Friday, September 8, 2023 6:05 AM
To: Lien, HoraceX <horacex.lien@intel.com>; devel@edk2.groups.io; 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
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>
> Sent: Thursday, September 7, 2023 2:41 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@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>; Lien, HoraceX <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 <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>;
> devel@edk2.groups.io
> 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,
>
> 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>
> Sent: Thursday, August 31, 2023 11:56 PM
> To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
> 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
>
> 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.lien@intel.com
> > Sent: Wednesday, August 30, 2023 2:13 AM
> > To: devel@edk2.groups.io
> > Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> > BcdRevision is not match with SMBIOS version
> >
> > From: HoraceX Lien <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>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: HoraceX Lien <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
> > 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 (#108646): https://edk2.groups.io/g/devel/message/108646
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
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
0 siblings, 2 replies; 18+ messages in thread
From: Michael D Kinney @ 2023-09-14 17:48 UTC (permalink / raw)
To: Lien, HoraceX, devel@edk2.groups.io, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Kinney, Michael D
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>
> Sent: Thursday, September 14, 2023 2:33 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io;
> 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,
>
> 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>; devel@edk2.groups.io;
> 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,
>
> 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>
> Sent: Friday, September 8, 2023 6:05 AM
> To: Lien, HoraceX <horacex.lien@intel.com>; devel@edk2.groups.io; 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
>
> 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>
> > Sent: Thursday, September 7, 2023 2:41 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@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>; Lien, HoraceX <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 <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>;
> > devel@edk2.groups.io
> > 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,
> >
> > 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>
> > Sent: Thursday, August 31, 2023 11:56 PM
> > To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
> > 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
> >
> > 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.lien@intel.com
> > > Sent: Wednesday, August 30, 2023 2:13 AM
> > > To: devel@edk2.groups.io
> > > Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> > > BcdRevision is not match with SMBIOS version
> > >
> > > From: HoraceX Lien <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>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Signed-off-by: HoraceX Lien <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
> > > 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 (#108653): https://edk2.groups.io/g/devel/message/108653
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]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-14 17:48 ` Michael D Kinney
@ 2023-09-15 7:35 ` Lien, HoraceX
2023-09-20 1:03 ` Guo, Gua
1 sibling, 0 replies; 18+ messages in thread
From: Lien, HoraceX @ 2023-09-15 7:35 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao, Guo, Gua
Hi Mike,
The SMBIOS Spec mentions: SMBIOS Major/Minor versions can be > 9. (it have an example on description: "the value is 0Ah for revision 10.22 and 02h for revision2.1").
Therefore, we cannot filter range 0-9 for property of the global variables(mPrivateData.Smbios.MajorVersion/MinorVersion), it can only filter range 0-9 for BcdRevision.
I don't think we need a conversion function to handle it, because of BcdRevision only ONE BYTE can be store Major/Minor version information, Major/Minor takes 4-bit only individually. If the Spec consider that 4-bit BCD format, it maximum can only represent 0-9(decimal) for one of version numbers.
For BcdRevision field, the Spec mentions:
"While the SMBIOS Major and Minor Versions (offsets 06h and 07h) currently duplicate the information that is present in the SMBIOS BCD Revision (offset 1Eh), they provide a path for future growth in this specification. The BCD Revision, for example, provides only a single digit for each of the major and minor version numbers. "
"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. "
I think that it is reasonable if fill in 0 when one of version numbers > 9.
For current SMBIOS Spec, there is no explicit mention how to do for BcdRevision if version > 9, I just provide an idea from my interpretation of Spec. In the past, it just masking 0x0F/0xF0, I filter 0-9 can more safely use them. Once SMBIOS version > 9, I think that Spec must be give a reasonable explanation for BcdRevision.
Thanks,
Horace Lien
-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Friday, September 15, 2023 1:48 AM
To: Lien, HoraceX <horacex.lien@intel.com>; devel@edk2.groups.io; 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
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>
> Sent: Thursday, September 14, 2023 2:33 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; 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,
>
> 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>;
> devel@edk2.groups.io; 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,
>
> 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>
> Sent: Friday, September 8, 2023 6:05 AM
> To: Lien, HoraceX <horacex.lien@intel.com>; devel@edk2.groups.io; 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
>
> 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>
> > Sent: Thursday, September 7, 2023 2:41 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@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>; Lien, HoraceX
> > <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 <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>;
> > devel@edk2.groups.io
> > 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,
> >
> > 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>
> > Sent: Thursday, August 31, 2023 11:56 PM
> > To: devel@edk2.groups.io; Lien, HoraceX <horacex.lien@intel.com>
> > 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
> >
> > 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.lien@intel.com
> > > Sent: Wednesday, August 30, 2023 2:13 AM
> > > To: devel@edk2.groups.io
> > > Cc: Lien, HoraceX <horacex.lien@intel.com>; 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: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix
> > > BcdRevision is not match with SMBIOS version
> > >
> > > From: HoraceX Lien <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>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Signed-off-by: HoraceX Lien <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
> > > 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 (#108698): https://edk2.groups.io/g/devel/message/108698
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
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
1 sibling, 1 reply; 18+ messages in thread
From: Guo, Gua @ 2023-09-20 1:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Kinney, Michael D, Lien, HoraceX,
Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
[-- Attachment #1.1: Type: text/plain, Size: 13902 bytes --]
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@01D9EB9F.378C2880]
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@01D9EB9F.C1DF3AE0]
Thanks,
Gua
-----Original Message-----
From: devel@edk2.groups.io <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>; devel@edk2.groups.io; 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
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 (#108891): https://edk2.groups.io/g/devel/message/108891
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 34440 bytes --]
[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 64751 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-20 1:03 ` Guo, Gua
@ 2023-09-20 1:35 ` Michael D Kinney
2023-09-20 1:52 ` Guo, Gua
[not found] ` <17867875CCCD5E00.29111@groups.io>
0 siblings, 2 replies; 18+ messages in thread
From: Michael D Kinney @ 2023-09-20 1:35 UTC (permalink / raw)
To: Guo, Gua, devel@edk2.groups.io, Lien, HoraceX, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Kinney, Michael D
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-20 1:35 ` Michael D Kinney
@ 2023-09-20 1:52 ` Guo, Gua
[not found] ` <17867875CCCD5E00.29111@groups.io>
1 sibling, 0 replies; 18+ messages in thread
From: Guo, Gua @ 2023-09-20 1:52 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Lien, HoraceX,
Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
[-- Attachment #1.1: Type: text/plain, Size: 16392 bytes --]
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image003.png@01D9EBA8.20F37700]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image004.png@01D9EBA8.20F37700]
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:image005.png@01D9EBA8.20F37700]
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 (#108894): https://edk2.groups.io/g/devel/message/108894
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 39322 bytes --]
[-- Attachment #2: image003.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #3: image004.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #4: image005.png --]
[-- Type: image/png, Size: 64751 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
[not found] ` <17867875CCCD5E00.29111@groups.io>
@ 2023-09-22 5:10 ` Guo, Gua
2023-09-22 16:19 ` Michael D Kinney
0 siblings, 1 reply; 18+ messages in thread
From: Guo, Gua @ 2023-09-22 5:10 UTC (permalink / raw)
To: devel@edk2.groups.io, Guo, Gua, Kinney, Michael D, Lien, HoraceX,
Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
[-- Attachment #1.1: Type: text/plain, Size: 17432 bytes --]
Hi Mike
Is it still have concern ? I think the patch just code enhance, it shouldn't have side effects, but also make the Edk2 code stronger for error handle, it should be good.
Thanks,
Gua
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guo, Gua
Sent: Wednesday, September 20, 2023 9:52 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; 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>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image001.png@01D9ED56.227DF230]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image002.png@01D9ED56.227DF230]
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:image003.png@01D9ED56.227DF230]
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 (#108976): https://edk2.groups.io/g/devel/message/108976
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 41461 bytes --]
[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #4: image003.png --]
[-- Type: image/png, Size: 64751 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-22 5:10 ` Guo, Gua
@ 2023-09-22 16:19 ` Michael D Kinney
2023-09-23 0:21 ` Guo, Gua
0 siblings, 1 reply; 18+ messages in thread
From: Michael D Kinney @ 2023-09-22 16:19 UTC (permalink / raw)
To: Guo, Gua, devel@edk2.groups.io, Lien, HoraceX, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Kinney, Michael D
[-- Attachment #1.1: Type: text/plain, Size: 18488 bytes --]
I agree that if either the Major or Minor value of the version can not be represented in BCD format that setting then entire value to zero is correct. Please update with that logic.
Mike
From: Guo, Gua <gua.guo@intel.com>
Sent: Thursday, September 21, 2023 10:10 PM
To: devel@edk2.groups.io; Guo, Gua <gua.guo@intel.com>; 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
Is it still have concern ? I think the patch just code enhance, it shouldn't have side effects, but also make the Edk2 code stronger for error handle, it should be good.
Thanks,
Gua
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Guo, Gua
Sent: Wednesday, September 20, 2023 9:52 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image001.png@01D9ED35.EBAB9340]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image002.png@01D9ED35.EBAB9340]
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:image003.png@01D9ED35.EBAB9340]
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 (#109018): https://edk2.groups.io/g/devel/message/109018
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: 43058 bytes --]
[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #4: image003.png --]
[-- Type: image/png, Size: 64751 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-22 16:19 ` Michael D Kinney
@ 2023-09-23 0:21 ` Guo, Gua
2023-09-23 3:06 ` Michael D Kinney
0 siblings, 1 reply; 18+ messages in thread
From: Guo, Gua @ 2023-09-23 0:21 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Lien, HoraceX,
Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
[-- Attachment #1.1: Type: text/plain, Size: 19941 bytes --]
I think currently MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version by liencx * Pull Request #4771 * tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/4771/> logic is correctly, so I want to give the patch Reviewed-by.
Currently,
EntryPointStructureData.MinorVersion allow HEX (mPrivateData.Smbios.MajorVersion) (0 - 0xFF)
EntryPointStructureData.MajorVersion allow HEX (mPrivateData.Smbios.MinorVersion) (0 - 0xFF)
EntryPointStructureData.SmbiosBcdRevision allow BCD only value (0x[0-9][0-9]), otherwise, 0
These items is assigned here
[cid:image004.png@01D9EDF6.3DEB5170]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Saturday, September 23, 2023 12:20 AM
To: Guo, Gua <gua.guo@intel.com>; 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
I agree that if either the Major or Minor value of the version can not be represented in BCD format that setting then entire value to zero is correct. Please update with that logic.
Mike
From: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Sent: Thursday, September 21, 2023 10:10 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Hi Mike
Is it still have concern ? I think the patch just code enhance, it shouldn't have side effects, but also make the Edk2 code stronger for error handle, it should be good.
Thanks,
Gua
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Guo, Gua
Sent: Wednesday, September 20, 2023 9:52 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image001.png@01D9EDF5.CA0AB4D0]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image002.png@01D9EDF5.CA0AB4D0]
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:image003.png@01D9EDF5.CA0AB4D0]
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 (#109020): https://edk2.groups.io/g/devel/message/109020
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 46527 bytes --]
[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #4: image003.png --]
[-- Type: image/png, Size: 64751 bytes --]
[-- Attachment #5: image004.png --]
[-- Type: image/png, Size: 57454 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-23 0:21 ` Guo, Gua
@ 2023-09-23 3:06 ` Michael D Kinney
2023-09-23 3:09 ` Guo, Gua
0 siblings, 1 reply; 18+ messages in thread
From: Michael D Kinney @ 2023-09-23 3:06 UTC (permalink / raw)
To: Guo, Gua, devel@edk2.groups.io, Lien, HoraceX, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao,
Kinney, Michael D
[-- Attachment #1.1: Type: text/plain, Size: 20913 bytes --]
The PR looks correct
I only see the original version of the patch on the mailing list. Can you send the V2 patch for review?
Mike
From: Guo, Gua <gua.guo@intel.com>
Sent: Friday, September 22, 2023 5:22 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; 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>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
I think currently MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version by liencx * Pull Request #4771 * tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/4771/> logic is correctly, so I want to give the patch Reviewed-by.
Currently,
EntryPointStructureData.MinorVersion allow HEX (mPrivateData.Smbios.MajorVersion) (0 - 0xFF)
EntryPointStructureData.MajorVersion allow HEX (mPrivateData.Smbios.MinorVersion) (0 - 0xFF)
EntryPointStructureData.SmbiosBcdRevision allow BCD only value (0x[0-9][0-9]), otherwise, 0
These items is assigned here
[cid:image004.png@01D9ED90.3BAA2460]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Saturday, September 23, 2023 12:20 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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 agree that if either the Major or Minor value of the version can not be represented in BCD format that setting then entire value to zero is correct. Please update with that logic.
Mike
From: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Sent: Thursday, September 21, 2023 10:10 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Hi Mike
Is it still have concern ? I think the patch just code enhance, it shouldn't have side effects, but also make the Edk2 code stronger for error handle, it should be good.
Thanks,
Gua
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Guo, Gua
Sent: Wednesday, September 20, 2023 9:52 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image005.png@01D9ED90.3BAA2460]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image006.png@01D9ED90.3BAA2460]
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:image007.png@01D9ED90.3BAA2460]
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 (#109021): https://edk2.groups.io/g/devel/message/109021
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: 48208 bytes --]
[-- Attachment #2: image004.png --]
[-- Type: image/png, Size: 57454 bytes --]
[-- Attachment #3: image005.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #4: image006.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #5: image007.png --]
[-- Type: image/png, Size: 64751 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-23 3:06 ` Michael D Kinney
@ 2023-09-23 3:09 ` Guo, Gua
2023-09-25 2:13 ` Lien, HoraceX
0 siblings, 1 reply; 18+ messages in thread
From: Guo, Gua @ 2023-09-23 3:09 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, Lien, HoraceX,
Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
[-- Attachment #1.1: Type: text/plain, Size: 22059 bytes --]
Thanks Mike.
Hi @Lien, HoraceX<mailto:horacex.lien@intel.com>,
please help to git send mail v2 patch, I think the concern has been eliminated by your latest PR MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version by liencx * Pull Request #4771 * tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/4771/>.
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Saturday, September 23, 2023 11:06 AM
To: Guo, Gua <gua.guo@intel.com>; 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
The PR looks correct
I only see the original version of the patch on the mailing list. Can you send the V2 patch for review?
Mike
From: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Sent: Friday, September 22, 2023 5:22 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
I think currently MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version by liencx * Pull Request #4771 * tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/4771/> logic is correctly, so I want to give the patch Reviewed-by.
Currently,
EntryPointStructureData.MinorVersion allow HEX (mPrivateData.Smbios.MajorVersion) (0 - 0xFF)
EntryPointStructureData.MajorVersion allow HEX (mPrivateData.Smbios.MinorVersion) (0 - 0xFF)
EntryPointStructureData.SmbiosBcdRevision allow BCD only value (0x[0-9][0-9]), otherwise, 0
These items is assigned here
[cid:image001.png@01D9EE0E.623C92B0]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Saturday, September 23, 2023 12:20 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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 agree that if either the Major or Minor value of the version can not be represented in BCD format that setting then entire value to zero is correct. Please update with that logic.
Mike
From: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Sent: Thursday, September 21, 2023 10:10 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Hi Mike
Is it still have concern ? I think the patch just code enhance, it shouldn't have side effects, but also make the Edk2 code stronger for error handle, it should be good.
Thanks,
Gua
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Guo, Gua
Sent: Wednesday, September 20, 2023 9:52 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image002.png@01D9EE0E.623C92B0]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image003.png@01D9EE0E.623C92B0]
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:image004.png@01D9EE0E.623C92B0]
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 (#109022): https://edk2.groups.io/g/devel/message/109022
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.2: Type: text/html, Size: 50871 bytes --]
[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 57454 bytes --]
[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #4: image003.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #5: image004.png --]
[-- Type: image/png, Size: 64751 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
2023-09-23 3:09 ` Guo, Gua
@ 2023-09-25 2:13 ` Lien, HoraceX
0 siblings, 0 replies; 18+ messages in thread
From: Lien, HoraceX @ 2023-09-25 2:13 UTC (permalink / raw)
To: Guo, Gua, Kinney, Michael D, devel@edk2.groups.io, Gao, Liming
Cc: Liu, Zhiguang, Bi, Dandan, Zeng, Star, Gao, Zhichao
[-- Attachment #1.1.1: Type: text/plain, Size: 22936 bytes --]
Thanks Gua, Mike.
I have already sent V2 patch.
Thanks,
Horace Lien
From: Guo, Gua <gua.guo@intel.com>
Sent: Saturday, September 23, 2023 11:10 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; 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>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Thanks Mike.
Hi @Lien, HoraceX<mailto:horacex.lien@intel.com>,
please help to git send mail v2 patch, I think the concern has been eliminated by your latest PR MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version by liencx * Pull Request #4771 * tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/4771/>.
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Saturday, September 23, 2023 11:06 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
The PR looks correct
I only see the original version of the patch on the mailing list. Can you send the V2 patch for review?
Mike
From: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Sent: Friday, September 22, 2023 5:22 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
I think currently MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version by liencx * Pull Request #4771 * tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/4771/> logic is correctly, so I want to give the patch Reviewed-by.
Currently,
EntryPointStructureData.MinorVersion allow HEX (mPrivateData.Smbios.MajorVersion) (0 - 0xFF)
EntryPointStructureData.MajorVersion allow HEX (mPrivateData.Smbios.MinorVersion) (0 - 0xFF)
EntryPointStructureData.SmbiosBcdRevision allow BCD only value (0x[0-9][0-9]), otherwise, 0
These items is assigned here
[cid:image001.png@01D9EF98.F5577FC0]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Saturday, September 23, 2023 12:20 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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 agree that if either the Major or Minor value of the version can not be represented in BCD format that setting then entire value to zero is correct. Please update with that logic.
Mike
From: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>
Sent: Thursday, September 21, 2023 10:10 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Hi Mike
Is it still have concern ? I think the patch just code enhance, it shouldn't have side effects, but also make the Edk2 code stronger for error handle, it should be good.
Thanks,
Gua
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Guo, Gua
Sent: Wednesday, September 20, 2023 9:52 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>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
In fact, spec say MajorVersion (Offset 06h) and MinorVersion (Offset 07h) are duplicate item with BCD Revision (Offset 1Eh), if it duplicate item, I think the value should be the same.
BCD Revision only exist on 32-bit Entry Point (_SM_), on 64-bit Entry Point (_SM3_) is obsolete, so I guess SMBIOS organization may think it make someone confusion, so it obsolete on 64-bit Entry Point, but for 32-bit Entry Point, maybe we need to let MajorVersion (Offset 06h) and MinorVersion (Offset 07h) as same as BCD Revision (Offset 1Eh). And also have better error handle for out of BCD format.
[cid:image002.png@01D9EF98.F5577FC0]
Thanks,
Gua
From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Wednesday, September 20, 2023 9:35 AM
To: Guo, Gua <gua.guo@intel.com<mailto:gua.guo@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>; 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
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<mailto:gua.guo@intel.com>>
Sent: Tuesday, September 19, 2023 6:03 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Lien, HoraceX <horacex.lien@intel.com<mailto:horacex.lien@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>>
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:image003.png@01D9EF98.F5577FC0]
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:image004.png@01D9EF98.F5577FC0]
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 (#109029): https://edk2.groups.io/g/devel/message/109029
Mute This Topic: https://groups.io/mt/101057293/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #1.1.2: Type: text/html, Size: 60200 bytes --]
[-- Attachment #1.2: image001.png --]
[-- Type: image/png, Size: 57454 bytes --]
[-- Attachment #1.3: image002.png --]
[-- Type: image/png, Size: 232151 bytes --]
[-- Attachment #1.4: image003.png --]
[-- Type: image/png, Size: 71822 bytes --]
[-- Attachment #1.5: image004.png --]
[-- Type: image/png, Size: 64751 bytes --]
[-- Attachment #2: Type: message/rfc822, Size: 9039 bytes --]
From: "Lien, HoraceX" <horacex.lien@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Lien, HoraceX" <horacex.lien@intel.com>, "Liu, Zhiguang" <zhiguang.liu@intel.com>, "Guo, Gua" <gua.guo@intel.com>, "Bi, Dandan" <dandan.bi@intel.com>, "Kinney, Michael D" <michael.d.kinney@intel.com>, "Zeng, Star" <star.zeng@intel.com>, "Gao, Zhichao" <zhichao.gao@intel.com>
Subject: [PATCH v2] MdeModulePkg/SmbiosDxe: Fix BcdRevision is not match with SMBIOS version
Date: Mon, 25 Sep 2023 02:06:42 +0000
Message-ID: <20230925020642.14874-1-horacex.lien@intel.com>
From: HoraceX Lien <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 BCD Revision from SMBIOS memory data,
to ensure that get consistent version value.
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Gua Guo <gua.guo@intel.com>
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: HoraceX Lien <horacex.lien@intel.com>
---
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
index 1a86e69d3c..2ef7b8e21c 100644
--- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
+++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c
@@ -1072,14 +1072,18 @@ 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);
- PhysicalAddress = 0xffffffff;
- Status = gBS->AllocatePages (
- AllocateMaxAddress,
- EfiRuntimeServicesData,
- EFI_SIZE_TO_PAGES (sizeof (SMBIOS_TABLE_ENTRY_POINT)),
- &PhysicalAddress
- );
+ EntryPointStructureData.SmbiosBcdRevision = 0;
+ if ((mPrivateData.Smbios.MajorVersion <= 9) && (mPrivateData.Smbios.MinorVersion <= 9)) {
+ EntryPointStructureData.SmbiosBcdRevision = ((mPrivateData.Smbios.MajorVersion & 0x0f) << 4) | (mPrivateData.Smbios.MinorVersion & 0x0f);
+ }
+
+ PhysicalAddress = 0xffffffff;
+ Status = gBS->AllocatePages (
+ AllocateMaxAddress,
+ EfiRuntimeServicesData,
+ EFI_SIZE_TO_PAGES (sizeof (SMBIOS_TABLE_ENTRY_POINT)),
+ &PhysicalAddress
+ );
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "SmbiosCreateTable () could not allocate EntryPointStructure < 4GB\n"));
Status = gBS->AllocatePages (
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-09-25 2:14 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox