public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: Rebecca Cran <rebecca@nuviainc.com>
Cc: devel@edk2.groups.io,
	"Ard Biesheuvel" <ardb+tianocore@kernel.org>,
	nd@arm.com, "Sami Mujawar" <Sami.Mujawar@arm.com>,
	"Liming Gao" <gaoliming@byosoft.com.cn>,
	"Michael D Kinney" <michael.d.kinney@intel.com>,
	"Zhiguang Liu" <zhiguang.liu@intel.com>,
	"Samer El-Haj-Mahmoud" <Samer.El-Haj-Mahmoud@arm.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH v8 00/21] ArmPkg,MdePkg: Add Universal/Smbios, and related changes
Date: Mon, 8 Feb 2021 18:55:03 +0000	[thread overview]
Message-ID: <20210208185502.GD1664@vanye> (raw)
In-Reply-To: <20210208005254.12176-1-rebecca@nuviainc.com>

On Sun, Feb 07, 2021 at 17:52:33 -0700, Rebecca Cran wrote:
> Much of the data for the SMBIOS tables is generic, and need not be
> duplicated for each platform. This patch series introduces
> ArmPkg/Universal/Smbios, which is largely copied from
> edk2-platforms/Silicon/HiSilicon/Drivers/Smbios and generates SMBIOS
> tables 0,1,2,3,4,7,13,32 and uses a combination of PCDs and calls into a
> new OemMiscLib to get information which varies between platforms.
> 
> I plan to submit a patch against SbsaQemu to update it to use this new
> functionality.
> 
> Changes from v7 to v8:
> 
> o Changed associativity function to return 1-based value.
> o Changed cache level parameter and return values to be 1-based.
> o Other changes based on Leif's review feedback.

There was also a change to a macro in 7/21 (more below).

I think this set is good to go in now, but I'd like to fold in the
following changes if you're OK with that:

diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
index b21eeade64b5..dc4e02966ca8 100644
--- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
+++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
@@ -28,4 +28,3 @@ [Packages]
 
 [LibraryClasses]
   BaseMemoryLib
-
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
index 60d8fe31c219..2d68cc0fe2e2 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
@@ -85,5 +85,3 @@ [Guids]
 
 [Depex]
   gEfiSmbiosProtocolGuid
-
-
diff --git a/ArmPkg/Library/ArmLib/ArmLibPrivate.h b/ArmPkg/Library/ArmLib/ArmLibPrivate.h
index 1818a1994dc3..42a49b4dec7d 100644
--- a/ArmPkg/Library/ArmLib/ArmLibPrivate.h
+++ b/ArmPkg/Library/ArmLib/ArmLibPrivate.h
@@ -145,7 +145,7 @@ typedef enum {
   ClidrCacheTypeMax
 } CLIDR_CACHE_TYPE;
 
-#define CLIDR_GET_CACHE_TYPE(x, level) ((x >> (3 * (level))) & 0b111)
+#define CLIDR_GET_CACHE_TYPE(x, level) (((x) >> (3 * (level))) & 0b111)
 
 VOID
 CPSRMaskInsert (
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
index ddd774b16f83..b214b6d0d217 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
@@ -90,4 +90,3 @@ SmbiosProcessorGetCacheAssociativity (
 
   return Associativity;
 }
-
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
index 0be4403c765f..546628934319 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
@@ -96,4 +96,3 @@ ArmGetCacheAssociativity (
 
   return Associativity;
 }
-
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c
index eb7f4cb4c16d..daf548cc2b4d 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c
@@ -220,4 +220,3 @@ SmbiosMiscGetLinkTypeHandle(
     }
   }
 }
-
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguages.uni b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguages.uni
index 3af7a01653d8..1d959b1a4bdb 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguages.uni
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguages.uni
@@ -39,5 +39,3 @@
 
 #string STR_MISC_BIOS_LANGUAGES_SIMPLECH_ABBREVIATE  #language en-US  "zhCN"
 #string STR_MISC_BIOS_LANGUAGES_SIMPLECH_LONG        #language en-US  "zh|CN|unicode"
-
-

These are all trailing newlines at EOF, which git am complains about
when importing the patches, and an additional fix to aforementioned
macro.

Are you OK with me doing this?

Best Regards,

Leif

> More changes are needed to support reporting boot status etc., but with the
> series getting rather large I think it might be good to get this committed
> and make a follow-up series with the additional changes.
> 
> Testing:
> 
> o Ran Ecc tool 
> 
> o Ran smbiosview in the UEFI Shell
> 
> 
> Rebecca Cran (21):
>   ArmPkg: Add ARM SMC Architecture functions to ArmStdSmc.h
>   MdePkg: Update IndustryStandard/SmBios.h with processor status data
>   ArmPkg: Add register encoding definition for MMFR2
>   ArmPkg: Add helper to read the Memory Model Features Register 2
>   ArmPkg: Add helper function to read the Memory Model Feature Register
>     4
>   ArmPkg: Fix the return type of the ReadCCSIDR function
>   ArmPkg: Update ArmLibPrivate.h with cache register definitions
>   ArmPkg: Add definition of the maximum cache level in ARMv8-A
>   ArmPkg: Add helper to read CCIDX status
>   ArmPkg: Add helper to read the CCSIDR2 register
>   ArmPkg: Add Library/OemMiscLib.h
>   ArmPkg: Add Universal/Smbios/OemMiscLibNull
>   ArmPkg: Add Universal/Smbios/ProcessorSubClassDxe
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type00
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type01
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type02
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type03
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type13
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type32
>   ArmPkg: Add SMBIOS PCDs to ArmPkg.dec
>   ArmPkg: Add Universal/Smbios/SmbiosMiscDxe
> 
>  ArmPkg/ArmPkg.dec                                                                       |  17 +
>  ArmPkg/ArmPkg.dsc                                                                       |   5 +
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf                               |  31 +
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf                   |  66 ++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf                                 |  89 +++
>  ArmPkg/Include/Chipset/AArch64.h                                                        |   4 +
>  ArmPkg/Include/IndustryStandard/ArmStdSmc.h                                             |  16 +
>  ArmPkg/Include/Library/ArmLib.h                                                         |  15 +
>  ArmPkg/Include/Library/OemMiscLib.h                                                     | 167 +++++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.h                                              |  11 +
>  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.h                                                    |   8 +-
>  ArmPkg/Library/ArmLib/ArmLibPrivate.h                                                   | 117 ++-
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessor.h                          | 102 +++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMisc.h                                      | 134 ++++
>  MdePkg/Include/IndustryStandard/SmBios.h                                                |  13 +
>  ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c                                              |  19 +-
>  ArmPkg/Library/ArmLib/Arm/ArmV7Lib.c                                                    |  19 +-
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c                                     | 144 ++++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c                        | 752 ++++++++++++++++++++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c                   |  93 +++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c                       |  99 +++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c                 | 249 +++++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDataTable.c                             |  62 ++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c                            | 223 ++++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorData.c                       |  93 +++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c                   | 296 ++++++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerData.c               |  36 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c           | 196 +++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerData.c            |  46 ++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c        | 230 ++++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerData.c              |  52 ++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c          | 224 ++++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesData.c     |  33 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c | 166 +++++
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationData.c                  |  32 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c              |  73 ++
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S                                          |   3 +
>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupportV8.S                                         |   2 +-
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupportV7.S                                             |  16 +-
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupportV7.asm                                           |  16 +-
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassStrings.uni               |  24 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxeStrings.uni                          |  22 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendor.uni                         |  18 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturer.uni                 |  20 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturer.uni              |  20 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturer.uni                |  18 +
>  ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguages.uni       |  43 ++
>  47 files changed, 4126 insertions(+), 8 deletions(-)
>  create mode 100644 ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLibNull.inf
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassDxe.inf
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>  create mode 100644 ArmPkg/Include/Library/OemMiscLib.h
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessor.h
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMisc.h
>  create mode 100644 ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorAArch64.c
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArm.c
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/SmbiosProcessorArmCommon.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDataTable.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorData.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerData.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerData.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerData.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesData.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationData.c
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
>  create mode 100644 ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClassStrings.uni
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxeStrings.uni
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendor.uni
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturer.uni
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturer.uni
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturer.uni
>  create mode 100644 ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguages.uni
> 
> -- 
> 2.26.2
> 

  parent reply	other threads:[~2021-02-08 18:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-08  0:52 [PATCH v8 00/21] ArmPkg,MdePkg: Add Universal/Smbios, and related changes Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 01/21] ArmPkg: Add ARM SMC Architecture functions to ArmStdSmc.h Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 02/21] MdePkg: Update IndustryStandard/SmBios.h with processor status data Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 03/21] ArmPkg: Add register encoding definition for MMFR2 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 04/21] ArmPkg: Add helper to read the Memory Model Features Register 2 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 05/21] ArmPkg: Add helper function to read the Memory Model Feature Register 4 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 06/21] ArmPkg: Fix the return type of the ReadCCSIDR function Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 07/21] ArmPkg: Update ArmLibPrivate.h with cache register definitions Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 08/21] ArmPkg: Add definition of the maximum cache level in ARMv8-A Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 09/21] ArmPkg: Add helper to read CCIDX status Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 10/21] ArmPkg: Add helper to read the CCSIDR2 register Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 11/21] ArmPkg: Add Library/OemMiscLib.h Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 12/21] ArmPkg: Add Universal/Smbios/OemMiscLibNull Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 13/21] ArmPkg: Add Universal/Smbios/ProcessorSubClassDxe Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 14/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type00 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 15/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type01 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 16/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type02 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 17/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type03 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 18/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type13 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 19/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type32 Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 20/21] ArmPkg: Add SMBIOS PCDs to ArmPkg.dec Rebecca Cran
2021-02-08  0:52 ` [PATCH v8 21/21] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe Rebecca Cran
2021-02-08  0:57 ` [PATCH v8 00/21] ArmPkg,MdePkg: Add Universal/Smbios, and related changes Rebecca Cran
2021-02-08 18:55 ` Leif Lindholm [this message]
2021-02-08 19:02   ` Rebecca Cran
2021-02-08 19:47     ` Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210208185502.GD1664@vanye \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox