[PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[Rebecca Cran]

Fix some typos in smbiosview and SmBios.h.

Add the SMC ARM64 SoC ID supported bit from SMBIOS 3.4.0 to SmBios.h
and smbiosview.

Fix the type of the PROCESSOR_CHARACTERISTIC_FLAGS bitfield.

Pull request: https://github.com/tianocore/edk2/pull/1038
Also built OVMF X64 and verified smbiosview still runs.

Rebecca Cran (6):
  ShellPkg: Fix a couple of typos in smbiosview
  ShellPkg: Add ARM64 SoC ID to Processor Characteristics in smbiosview
  MdePkg: Update SmBios.h to add SMBIOS 3.4.0 ARM64 SoC ID field
  MdePkg: Fix typos in SmBios.h PROCESSOR_CHARACTERISTIC_FLAGS struct
  MdePkg: Update SmBios.h to fix PROCESSOR_CHARACTERISTIC_FLAGS
  ShellPkg: Fix smbiosview string definition name for "boot from CD"

 MdePkg/Include/IndustryStandard/SmBios.h      | 21 ++++++++++---------
 .../SmbiosView/PrintInfo.c                    |  2 +-
 .../SmbiosView/QueryTable.c                   |  4 ++++
 .../SmbiosView/SmbiosViewStrings.uni          |  4 ++--
 4 files changed, 18 insertions(+), 13 deletions(-)

-- 
2.26.2

[PATCH 1/6] ShellPkg: Fix a couple of typos in smbiosview

[Rebecca Cran]

Fix a couple of typos in SmbiosViewStrings.uni:
"Boot form CD" should be "Boot from CD", and "plugged from the wall"
should be "plugged into the wall".

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 .../SmbiosView/SmbiosViewStrings.uni                          | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
index 7fbcc9a31afb..4160ca40a988 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
@@ -128,7 +128,7 @@
 #string STR_SMBIOSVIEW_PRINTINFO_BIOS_SHADOWING                 #language en-US "BIOS shadowing is allowed\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_VESA_SUPPORTED                 #language en-US "VL-VESA is supported\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_ECSD_SUPPORT                   #language en-US "ESCD support is available\r\n"
-#string STR_SMBIOSVIEW_PRINTINFO_BOOT_FORM_CD_SUPPORTED         #language en-US "Boot form CD is supported\r\n"
+#string STR_SMBIOSVIEW_PRINTINFO_BOOT_FORM_CD_SUPPORTED         #language en-US "Boot from CD is supported\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_SELECTED_BOOT_SUPPORTED        #language en-US "Selectable Boot is supported\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_BIOS_ROM_SOCKETED              #language en-US "BIOS ROM is socketed\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_BOOT_FROM_PC_CARD              #language en-US "Boot From PC Card(PCMCIA)is supported\r\n"
@@ -373,7 +373,7 @@
 #string STR_SMBIOSVIEW_PRINTINFO_NOT_APPLICABLE                 #language en-US " Not applicable\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_RESERVED_3                     #language en-US " Reserved  \r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_UNPLUGGED         #language en-US "Power supply is unplugged from the wall\r\n"
-#string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_PLUGGED           #language en-US "Power supply is plugged from the wall\r\n"
+#string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_PLUGGED           #language en-US "Power supply is plugged into the wall\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_PRESENT           #language en-US "Power supply is present\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_NOT_PRESENT       #language en-US "Power supply is not present\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_POWER_SUPPLY_REPLACE           #language en-US "Power supply is hot replaceable\r\n"
-- 
2.26.2

[PATCH 2/6] ShellPkg: Add ARM64 SoC ID to Processor Characteristics in smbiosview

[Rebecca Cran]

SMBIOS 3.4.0 defines bit 9 of the Type 4 table Processor Characteristics
field to be the ARM64 SoC ID support. Add support for it to the
smbiosview command.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 .../UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c        | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
index 17a2a89d512e..d0796a0e5141 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
@@ -624,6 +624,10 @@ TABLE_ITEM  ProcessorCharacteristicsTable[] = {
   {
     8,
     L" 128-bit Capable"
+  },
+  {
+    9,
+    L" ARM64 SoC ID"
   }
 };
 
-- 
2.26.2

[PATCH 3/6] MdePkg: Update SmBios.h to add SMBIOS 3.4.0 ARM64 SoC ID field

[Rebecca Cran]

SMBIOS 3.4.0 defines bit 9 of the Type 4 table Processor Characteristics
field to be the ARM64 SoC ID support. Add it to the
PROCESSOR_CHARACTERISTIC_FLAGS struct bitfield.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 MdePkg/Include/IndustryStandard/SmBios.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h b/MdePkg/Include/IndustryStandard/SmBios.h
index f2db11f947d1..c495c48f3b47 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -871,7 +871,8 @@ typedef struct {
   UINT32  ProcessorEnhancedVirtulization :1;
   UINT32  ProcessorPowerPerformanceCtrl  :1;
   UINT32  Processor128bitCapble          :1;
-  UINT32  ProcessorReserved2             :7;
+  UINT32  ProcessorArm64SocId            :1;
+  UINT32  ProcessorReserved2             :6;
 } PROCESSOR_CHARACTERISTIC_FLAGS;
 
 typedef struct {
-- 
2.26.2

[PATCH 4/6] MdePkg: Fix typos in SmBios.h PROCESSOR_CHARACTERISTIC_FLAGS struct

[Rebecca Cran]

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 MdePkg/Include/IndustryStandard/SmBios.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h b/MdePkg/Include/IndustryStandard/SmBios.h
index c495c48f3b47..1981e551187b 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -862,17 +862,17 @@ typedef struct {
 } PROCESSOR_FEATURE_FLAGS;
 
 typedef struct {
-  UINT32  ProcessorReserved1             :1;
-  UINT32  ProcessorUnknown               :1;
-  UINT32  Processor64BitCapble           :1;
-  UINT32  ProcessorMultiCore             :1;
-  UINT32  ProcessorHardwareThread        :1;
-  UINT32  ProcessorExecuteProtection     :1;
-  UINT32  ProcessorEnhancedVirtulization :1;
-  UINT32  ProcessorPowerPerformanceCtrl  :1;
-  UINT32  Processor128bitCapble          :1;
-  UINT32  ProcessorArm64SocId            :1;
-  UINT32  ProcessorReserved2             :6;
+  UINT32  ProcessorReserved1              :1;
+  UINT32  ProcessorUnknown                :1;
+  UINT32  Processor64BitCapable           :1;
+  UINT32  ProcessorMultiCore              :1;
+  UINT32  ProcessorHardwareThread         :1;
+  UINT32  ProcessorExecuteProtection      :1;
+  UINT32  ProcessorEnhancedVirtualization :1;
+  UINT32  ProcessorPowerPerformanceCtrl   :1;
+  UINT32  Processor128BitCapable          :1;
+  UINT32  ProcessorArm64SocId             :1;
+  UINT32  ProcessorReserved2              :6;
 } PROCESSOR_CHARACTERISTIC_FLAGS;
 
 typedef struct {
-- 
2.26.2

[PATCH 5/6] MdePkg: Update SmBios.h to fix PROCESSOR_CHARACTERISTIC_FLAGS

[Rebecca Cran]

The ProcessorCharacteristics field is a UINT16, so the
PROCESSOR_CHARACTERISTIC_FLAGS bitfield should be too.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 MdePkg/Include/IndustryStandard/SmBios.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h b/MdePkg/Include/IndustryStandard/SmBios.h
index 1981e551187b..400bdef90b4f 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -862,17 +862,17 @@ typedef struct {
 } PROCESSOR_FEATURE_FLAGS;
 
 typedef struct {
-  UINT32  ProcessorReserved1              :1;
-  UINT32  ProcessorUnknown                :1;
-  UINT32  Processor64BitCapable           :1;
-  UINT32  ProcessorMultiCore              :1;
-  UINT32  ProcessorHardwareThread         :1;
-  UINT32  ProcessorExecuteProtection      :1;
-  UINT32  ProcessorEnhancedVirtualization :1;
-  UINT32  ProcessorPowerPerformanceCtrl   :1;
-  UINT32  Processor128BitCapable          :1;
-  UINT32  ProcessorArm64SocId             :1;
-  UINT32  ProcessorReserved2              :6;
+  UINT16  ProcessorReserved1              :1;
+  UINT16  ProcessorUnknown                :1;
+  UINT16  Processor64BitCapable           :1;
+  UINT16  ProcessorMultiCore              :1;
+  UINT16  ProcessorHardwareThread         :1;
+  UINT16  ProcessorExecuteProtection      :1;
+  UINT16  ProcessorEnhancedVirtualization :1;
+  UINT16  ProcessorPowerPerformanceCtrl   :1;
+  UINT16  Processor128BitCapable          :1;
+  UINT16  ProcessorArm64SocId             :1;
+  UINT16  ProcessorReserved2              :6;
 } PROCESSOR_CHARACTERISTIC_FLAGS;
 
 typedef struct {
-- 
2.26.2

[PATCH 6/6] ShellPkg: Fix smbiosview string definition name for "boot from CD"

[Rebecca Cran]

The "boot from CD supported" string definition name should contain the
word "from" instead of "form".

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 .../Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c   | 2 +-
 .../UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
index 7517465ddd49..478811f0fd40 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
@@ -1284,7 +1284,7 @@ DisplayBiosCharacteristics (
   }
 
   if (BIT (Chara, 15) != 0) {
-    ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_BOOT_FORM_CD_SUPPORTED), gShellDebug1HiiHandle);
+    ShellPrintHiiEx(-1,-1,NULL,STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_BOOT_FROM_CD_SUPPORTED), gShellDebug1HiiHandle);
   }
 
   if (BIT (Chara, 16) != 0) {
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
index 4160ca40a988..b14d1cb6dac8 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosViewStrings.uni
@@ -128,7 +128,7 @@
 #string STR_SMBIOSVIEW_PRINTINFO_BIOS_SHADOWING                 #language en-US "BIOS shadowing is allowed\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_VESA_SUPPORTED                 #language en-US "VL-VESA is supported\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_ECSD_SUPPORT                   #language en-US "ESCD support is available\r\n"
-#string STR_SMBIOSVIEW_PRINTINFO_BOOT_FORM_CD_SUPPORTED         #language en-US "Boot from CD is supported\r\n"
+#string STR_SMBIOSVIEW_PRINTINFO_BOOT_FROM_CD_SUPPORTED         #language en-US "Boot from CD is supported\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_SELECTED_BOOT_SUPPORTED        #language en-US "Selectable Boot is supported\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_BIOS_ROM_SOCKETED              #language en-US "BIOS ROM is socketed\r\n"
 #string STR_SMBIOSVIEW_PRINTINFO_BOOT_FROM_PC_CARD              #language en-US "Boot From PC Card(PCMCIA)is supported\r\n"
-- 
2.26.2

回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[gaoliming]

Rebecca:
  They are the good change. But for patch 4/6, it changes the structure
field name. It may bring the impact to the platform code. I don't suggest to
do the renaming field only for typo purpose. Other changes are good to me.
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: Rebecca Cran <rebecca@nuviainc.com>
> 发送时间: 2020年10月22日 2:41
> 收件人: devel@edk2.groups.io
> 抄送: Rebecca Cran <rebecca@nuviainc.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Zhiguang Liu <zhiguang.liu@intel.com>; Ray Ni <ray.ni@intel.com>; Zhichao
> Gao <zhichao.gao@intel.com>
> 主题: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64
> SoC ID definition
> 
> Fix some typos in smbiosview and SmBios.h.
> 
> Add the SMC ARM64 SoC ID supported bit from SMBIOS 3.4.0 to SmBios.h
> and smbiosview.
> 
> Fix the type of the PROCESSOR_CHARACTERISTIC_FLAGS bitfield.
> 
> Pull request: https://github.com/tianocore/edk2/pull/1038
> Also built OVMF X64 and verified smbiosview still runs.
> 
> Rebecca Cran (6):
>   ShellPkg: Fix a couple of typos in smbiosview
>   ShellPkg: Add ARM64 SoC ID to Processor Characteristics in smbiosview
>   MdePkg: Update SmBios.h to add SMBIOS 3.4.0 ARM64 SoC ID field
>   MdePkg: Fix typos in SmBios.h PROCESSOR_CHARACTERISTIC_FLAGS
> struct
>   MdePkg: Update SmBios.h to fix PROCESSOR_CHARACTERISTIC_FLAGS
>   ShellPkg: Fix smbiosview string definition name for "boot from CD"
> 
>  MdePkg/Include/IndustryStandard/SmBios.h      | 21
> ++++++++++---------
>  .../SmbiosView/PrintInfo.c                    |  2 +-
>  .../SmbiosView/QueryTable.c                   |  4 ++++
>  .../SmbiosView/SmbiosViewStrings.uni          |  4 ++--
>  4 files changed, 18 insertions(+), 13 deletions(-)
> 
> --
> 2.26.2

Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[Rebecca Cran]

On 10/21/20 7:46 PM, gaoliming wrote:
>    They are the good change. But for patch 4/6, it changes the structure
> field name. It may bring the impact to the platform code. I don't suggest to
> do the renaming field only for typo purpose. Other changes are good to me.

 From looking at the code in edk2-platforms, it seems the common pattern 
is to use positional arguments instead of designated initializers. Of 
course there could be platforms outside of TianoCore which use the field 
names, but it looks like a relatively safe change.


-- 
Rebecca Cran

Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[Gao, Zhichao]

For 1 - 3, 5 - 6, Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca
> Cran
> Sent: Thursday, October 22, 2020 10:21 AM
> To: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and
> SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> 
> On 10/21/20 7:46 PM, gaoliming wrote:
> >    They are the good change. But for patch 4/6, it changes the
> > structure field name. It may bring the impact to the platform code. I
> > don't suggest to do the renaming field only for typo purpose. Other
> changes are good to me.
> 
>  From looking at the code in edk2-platforms, it seems the common pattern is
> to use positional arguments instead of designated initializers. Of course
> there could be platforms outside of TianoCore which use the field names,
> but it looks like a relatively safe change.
> 
> 
> --
> Rebecca Cran
> 
> 
> 
> 
> 

回复: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[gaoliming]

Rebecca:
  Patch 4/6 may be safe. But, we may need more time to collect the feedback.


  Other patches are clear. I suggest to merge them first. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+66523+4905953+8761045@groups.io
> <bounce+27952+66523+4905953+8761045@groups.io> 代表 Rebecca Cran
> 发送时间: 2020年10月22日 10:21
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> <zhiguang.liu@intel.com>; 'Ray Ni' <ray.ni@intel.com>; 'Zhichao Gao'
> <zhichao.gao@intel.com>
> 主题: Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and
> SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> 
> On 10/21/20 7:46 PM, gaoliming wrote:
> >    They are the good change. But for patch 4/6, it changes the structure
> > field name. It may bring the impact to the platform code. I don't
suggest to
> > do the renaming field only for typo purpose. Other changes are good to
me.
> 
>  From looking at the code in edk2-platforms, it seems the common pattern
> is to use positional arguments instead of designated initializers. Of
> course there could be platforms outside of TianoCore which use the field
> names, but it looks like a relatively safe change.
> 
> 
> --
> Rebecca Cran
> 
> 
> 
> 
> 

Re: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[Leif Lindholm]

Hi Liming,

I agree 4/6 is invasive, but I also think industry standard headers
should match the specifications they implement.

If 5-6/6 do not depend on 4/6, then I agree the others can be merged
first.
Depending on perceived impact, perhaps 4/6 could be held back until
just after next stable tag?

Best Regards,

Leif

On Mon, Oct 26, 2020 at 09:24:21 +0800, gaoliming wrote:
> Rebecca:
>   Patch 4/6 may be safe. But, we may need more time to collect the feedback.
> 
> 
>   Other patches are clear. I suggest to merge them first. 
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: bounce+27952+66523+4905953+8761045@groups.io
> > <bounce+27952+66523+4905953+8761045@groups.io> 代表 Rebecca Cran
> > 发送时间: 2020年10月22日 10:21
> > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> > <zhiguang.liu@intel.com>; 'Ray Ni' <ray.ni@intel.com>; 'Zhichao Gao'
> > <zhichao.gao@intel.com>
> > 主题: Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and
> > SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> > 
> > On 10/21/20 7:46 PM, gaoliming wrote:
> > >    They are the good change. But for patch 4/6, it changes the structure
> > > field name. It may bring the impact to the platform code. I don't
> suggest to
> > > do the renaming field only for typo purpose. Other changes are good to
> me.
> > 
> >  From looking at the code in edk2-platforms, it seems the common pattern
> > is to use positional arguments instead of designated initializers. Of
> > course there could be platforms outside of TianoCore which use the field
> > names, but it looks like a relatively safe change.
> > 
> > 
> > --
> > Rebecca Cran
> > 
> > 
> > 
> > 
> > 
> 
> 
> 
> 
> 
> 
> 
> 

回复: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[gaoliming]

Leif:
  If no objection, I agree to merge patch 4/6 just after next stable tag 202011. 

Rebecca:
  Can you create PR for other 5 patches? I can help merge them. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+66605+4905953+8761045@groups.io
> <bounce+27952+66605+4905953+8761045@groups.io> 代表 Leif Lindholm
> 发送时间: 2020年10月27日 1:48
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: rebecca@nuviainc.com; 'Michael D Kinney'
> <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>; 'Ray
> Ni' <ray.ni@intel.com>; 'Zhichao Gao' <zhichao.gao@intel.com>
> 主题: Re: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h
> fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> 
> Hi Liming,
> 
> I agree 4/6 is invasive, but I also think industry standard headers
> should match the specifications they implement.
> 
> If 5-6/6 do not depend on 4/6, then I agree the others can be merged
> first.
> Depending on perceived impact, perhaps 4/6 could be held back until
> just after next stable tag?
> 
> Best Regards,
> 
> Leif
> 
> On Mon, Oct 26, 2020 at 09:24:21 +0800, gaoliming wrote:
> > Rebecca:
> >   Patch 4/6 may be safe. But, we may need more time to collect the
> feedback.
> >
> >
> >   Other patches are clear. I suggest to merge them first.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: bounce+27952+66523+4905953+8761045@groups.io
> > > <bounce+27952+66523+4905953+8761045@groups.io> 代表 Rebecca
> Cran
> > > 发送时间: 2020年10月22日 10:21
> > > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > > 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> > > <zhiguang.liu@intel.com>; 'Ray Ni' <ray.ni@intel.com>; 'Zhichao Gao'
> > > <zhichao.gao@intel.com>
> > > 主题: Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and
> > > SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> > >
> > > On 10/21/20 7:46 PM, gaoliming wrote:
> > > >    They are the good change. But for patch 4/6, it changes the
> structure
> > > > field name. It may bring the impact to the platform code. I don't
> > suggest to
> > > > do the renaming field only for typo purpose. Other changes are good to
> > me.
> > >
> > >  From looking at the code in edk2-platforms, it seems the common
> pattern
> > > is to use positional arguments instead of designated initializers. Of
> > > course there could be platforms outside of TianoCore which use the field
> > > names, but it looks like a relatively safe change.
> > >
> > >
> > > --
> > > Rebecca Cran
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 

Re: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[Gao, Zhichao]

Hi Liming,

I used to get the complain about the develop progress. The PR for merge should be created by the Maintainers. Refer to https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process#the-maintainer-process-for-the-edk-ii-project.
I suggest to let the submitter give the finial patches' fork branch link. Then you can create the PR base on the fork branch and give the push label.

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
> Sent: Tuesday, October 27, 2020 2:33 PM
> To: devel@edk2.groups.io; leif@nuviainc.com; gaoliming@byosoft.com.cn
> Cc: rebecca@nuviainc.com; Kinney, Michael D <michael.d.kinney@intel.com>;
> Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao,
> Zhichao <zhichao.gao@intel.com>
> Subject: 回复: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h
> fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> 
> Leif:
>   If no objection, I agree to merge patch 4/6 just after next stable tag 202011.
> 
> Rebecca:
>   Can you create PR for other 5 patches? I can help merge them.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: bounce+27952+66605+4905953+8761045@groups.io
> > <bounce+27952+66605+4905953+8761045@groups.io> 代表 Leif Lindholm
> > 发送时间: 2020年10月27日 1:48
> > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > 抄送: rebecca@nuviainc.com; 'Michael D Kinney'
> > <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>;
> > 'Ray Ni' <ray.ni@intel.com>; 'Zhichao Gao' <zhichao.gao@intel.com>
> > 主题: Re: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h
> > fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> >
> > Hi Liming,
> >
> > I agree 4/6 is invasive, but I also think industry standard headers
> > should match the specifications they implement.
> >
> > If 5-6/6 do not depend on 4/6, then I agree the others can be merged
> > first.
> > Depending on perceived impact, perhaps 4/6 could be held back until
> > just after next stable tag?
> >
> > Best Regards,
> >
> > Leif
> >
> > On Mon, Oct 26, 2020 at 09:24:21 +0800, gaoliming wrote:
> > > Rebecca:
> > >   Patch 4/6 may be safe. But, we may need more time to collect the
> > feedback.
> > >
> > >
> > >   Other patches are clear. I suggest to merge them first.
> > >
> > > Thanks
> > > Liming
> > > > -----邮件原件-----
> > > > 发件人: bounce+27952+66523+4905953+8761045@groups.io
> > > > <bounce+27952+66523+4905953+8761045@groups.io> 代表 Rebecca
> > Cran
> > > > 发送时间: 2020年10月22日 10:21
> > > > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> > > > 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> > > > <zhiguang.liu@intel.com>; 'Ray Ni' <ray.ni@intel.com>; 'Zhichao Gao'
> > > > <zhichao.gao@intel.com>
> > > > 主题: Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and
> > > > SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
> > > >
> > > > On 10/21/20 7:46 PM, gaoliming wrote:
> > > > >    They are the good change. But for patch 4/6, it changes the
> > structure
> > > > > field name. It may bring the impact to the platform code. I
> > > > > don't
> > > suggest to
> > > > > do the renaming field only for typo purpose. Other changes are
> > > > > good to
> > > me.
> > > >
> > > >  From looking at the code in edk2-platforms, it seems the common
> > pattern
> > > > is to use positional arguments instead of designated initializers.
> > > > Of course there could be platforms outside of TianoCore which use
> > > > the field names, but it looks like a relatively safe change.
> > > >
> > > >
> > > > --
> > > > Rebecca Cran
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> 
> 
> 
> 
> 
> 
> 

Re: 回复: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition

[Rebecca Cran]

Thanks. Yes, I'll send a new patch series for 1-4,6 and create a new PR.


-- 

Rebecca Cran


On 10/27/20 12:32 AM, gaoliming wrote:
> Leif:
>    If no objection, I agree to merge patch 4/6 just after next stable tag 202011.
>
> Rebecca:
>    Can you create PR for other 5 patches? I can help merge them.
>
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: bounce+27952+66605+4905953+8761045@groups.io
>> <bounce+27952+66605+4905953+8761045@groups.io> 代表 Leif Lindholm
>> 发送时间: 2020年10月27日 1:48
>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
>> 抄送: rebecca@nuviainc.com; 'Michael D Kinney'
>> <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>; 'Ray
>> Ni' <ray.ni@intel.com>; 'Zhichao Gao' <zhichao.gao@intel.com>
>> 主题: Re: 回复: [edk2-devel] 回复: [PATCH 0/6] smbiosview and SmBios.h
>> fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
>>
>> Hi Liming,
>>
>> I agree 4/6 is invasive, but I also think industry standard headers
>> should match the specifications they implement.
>>
>> If 5-6/6 do not depend on 4/6, then I agree the others can be merged
>> first.
>> Depending on perceived impact, perhaps 4/6 could be held back until
>> just after next stable tag?
>>
>> Best Regards,
>>
>> Leif
>>
>> On Mon, Oct 26, 2020 at 09:24:21 +0800, gaoliming wrote:
>>> Rebecca:
>>>    Patch 4/6 may be safe. But, we may need more time to collect the
>> feedback.
>>>
>>>    Other patches are clear. I suggest to merge them first.
>>>
>>> Thanks
>>> Liming
>>>> -----邮件原件-----
>>>> 发件人: bounce+27952+66523+4905953+8761045@groups.io
>>>> <bounce+27952+66523+4905953+8761045@groups.io> 代表 Rebecca
>> Cran
>>>> 发送时间: 2020年10月22日 10:21
>>>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
>>>> 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
>>>> <zhiguang.liu@intel.com>; 'Ray Ni' <ray.ni@intel.com>; 'Zhichao Gao'
>>>> <zhichao.gao@intel.com>
>>>> 主题: Re: [EXTERNAL] [edk2-devel] 回复: [PATCH 0/6] smbiosview and
>>>> SmBios.h fixes, add SMBIOS 3.4.0 ARM64 SoC ID definition
>>>>
>>>> On 10/21/20 7:46 PM, gaoliming wrote:
>>>>>     They are the good change. But for patch 4/6, it changes the
>> structure
>>>>> field name. It may bring the impact to the platform code. I don't
>>> suggest to
>>>>> do the renaming field only for typo purpose. Other changes are good to
>>> me.
>>>>   From looking at the code in edk2-platforms, it seems the common
>> pattern
>>>> is to use positional arguments instead of designated initializers. Of
>>>> course there could be platforms outside of TianoCore which use the field
>>>> names, but it looks like a relatively safe change.
>>>>
>>>>
>>>> --
>>>> Rebecca Cran
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>
>
> 
>
>