* [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
@ 2022-10-15 11:47 Chang, Abner
2022-10-18 6:01 ` [edk2-devel] " Ni, Ray
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Chang, Abner @ 2022-10-15 11:47 UTC (permalink / raw)
To: devel
Cc: Ray Ni, Michael D Kinney, Sunil V L, Abdul Lateef Attar,
Leif Lindholm
From: Abner Chang <abner.chang@amd.com>
Updates 4.2 Directory names and 4.3 file names for
the guidelines of module directory and file naming.
PR: https://github.com/tianocore-docs/edk2-CCodingStandardsSpecification/pull/2/files
Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Abdul Lateef Attar <abdattar@amd.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
---
4_naming_conventions/42_directory_names.md | 101 +++++++++++++++++++
4_naming_conventions/43_file_names.md | 108 ++++++++++++++++++++-
2 files changed, 208 insertions(+), 1 deletion(-)
diff --git a/4_naming_conventions/42_directory_names.md b/4_naming_conventions/42_directory_names.md
index 766ccb1..959a3c9 100644
--- a/4_naming_conventions/42_directory_names.md
+++ b/4_naming_conventions/42_directory_names.md
@@ -2,6 +2,7 @@
4.2 Directory Names
Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
+ Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
Redistribution and use in source (original document form) and 'compiled'
forms (converted to PDF, epub, HTML and other formats) with or without
@@ -28,3 +29,103 @@
ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-->
+
+## 4.2 Directory Names
+Below sections are the directory naming guidelines for EDK II modules. The guidelines are not
+just considering the the uniformity of directory naming, but it also provides the flexibility of
+directory name construction for the scenario of different EDK II module designs; such as the
+support for multiple processor architectures and vendors. It may require the further discussions
+between EDK II maintainers and contributors in order to determine the best naming of the EDK II
+module directory.
+
+#### 4.2.1 EDKII package directory
+
+```
+<PackageName>Pkg
+
+ <PackageName> REQUIRED *
+```
+
+#### 4.2.2 EDKII Module directory
+
+* The guideline below is applied to all CPU architectures support, specific CPU architecture and vendors support, or the implementation is shared by certain CPU archs:
+```
+<Feature><Phase>[<CpuArch>[<Vendor>]]
+ or
+<Feature><Phase>[/<CpuArch>[/<Vendor>]]
+
+ <Feature> REQUIRED *
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm, Smm,
+ Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+
+ <Vendor> OPTIONAL *
+
+Example:
+ - SmbiosDxe/
+ - CpuDxe/ # First implementation of CpuDxe.
+ - CpuDxeIa32X64Amd/ # Ia32 and X64 AMD specific implementation.
+ - CpuDxe/RiscV64/ # RiscV64 specific implementation.
+ / # Common files for the RiscV64 and other archs.
+ - CpuDxe/Ia32X64/Amd/ # Ia32 and X64 AMD specific implementation.
+ / # Common files for Ia32 and X64 archs.
+ /ArmAArch64/ # Arm and AArch64 implementation of CpuDxe.
+ / # Common files for the Arm, AArch64, Ia32 and X64.
+```
+
+* If the implementation does not have any shared code between phases (e.g.
+MdeModulePkg/Universal/PCD). The guideline below is applied to all CPU architectures support, specific CPU architecture and vendors support, or the implementation is shared by certain CPU archs:
+
+```
+<Feature>/<Phase>[/<CpuArch>[/<Vendor>]]
+
+ <Feature> REQUIRED *
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm, Smm,
+ Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+Example:
+ Pcd/Dxe/
+```
+
+#### 4.2.2 EDKII Library directory
+```
+<Phase>[<CpuArch>[<Vendor>]]<LibraryClassName>[<Dependency>]
+ or
+<Phase><LibraryClassName>[<Dependency>]/[<CpuArch>[/<Vendor>]]
+
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm,
+ StandaloneMm, Smm, Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+ <LibraryClassName> REQUIRED *
+ <Dependency> OPTIONAL * (Typically name of PPI, Protocol, LibraryClass
+ that the implementation is layered on top of).
+
+Example:
+ - BaseXApicLib/
+ - SmmIa32X64AmdSmmCpuFeaturesLib/
+ - SmmArmAArch64SmmCpuFeaturesLib/
+ - BaseMpInitLib/RiscV64/ # RiscV64 specific implementation.
+ /Ia32X64/ # Ia32 and X64 specific implementation.
+ /Ia32X64/Amd # Ia32 and X64 AMD specific implementation.
+ /ArmAArch64/ # Arm and AAch64 specific implementation.
+ /LoongArch64/ # LoongArch64 specific implementation.
+ / # Common files for RiscV64, Ia32, X64, Arm, AArch64 and
+ LoongArch64.
+```
diff --git a/4_naming_conventions/43_file_names.md b/4_naming_conventions/43_file_names.md
index 13e0c63..5714008 100644
--- a/4_naming_conventions/43_file_names.md
+++ b/4_naming_conventions/43_file_names.md
@@ -1,7 +1,8 @@
<!--- @file
4.3 File Names
- Copyright (c) 2006-2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2006-2022, Intel Corporation. All rights reserved.<BR>
+ Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
Redistribution and use in source (original document form) and 'compiled'
forms (converted to PDF, epub, HTML and other formats) with or without
@@ -58,3 +59,108 @@ regular expression:
That is, a letter followed by zero, or more, letters, underscores, dashes, or
digits followed by a period followed by one or more letters or digits.
+
+### 4.3.5 File naming guidelines for modules
+
+Below sections are the file naming guidelines for EDK II meta files and source files. The
+guidelines are not just considering the the uniformity of file naming, but it also provides
+the flexibility of file name construction for the scenario of different EDK II module
+designs; such as the support for multiple processor architectures and vendors. It may require the
+further discussions between EDK II maintainers and contributors in order to determine the best
+naming of the EDK II module file.
+
+#### 4.3.5.1 EDK II meta files within a package
+
+```
+<PackageName>Pkg.dec
+<PackageName>Pkg.dsc
+
+ <PackageName> REQUIRED *
+```
+
+#### 4.3.5.2 EDK II INF file within a Module instance
+* If the implementation is for all CPU architectures, specific CPU architectures, CPU vendors or the implementation is shared by certain CPU archs:
+```
+<Feature><Phase>[<CpuArch>][<Vendor>].inf
+
+ <Feature> REQUIRED *
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm,
+ Smm, Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+
+Example:
+ - SmbiosDxe.inf
+ - CpuIo2Dxe.inf
+ - CpuIo2DxeAmd.inf
+ - CpuIo2DxeIa32X64.inf
+ - CpuIo2DxeIa32X64Intel.inf
+```
+
+* If the implementation does not have any shared code between phases (e.g., Pcd/Dxe):
+
+```
+[<Feature>][<CpuArch>][<Vendor>].inf
+
+ <Feature> OPTIONAL *
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+Example:
+ Pcd.inf
+```
+
+#### 4.4.5.3 EDK II INF file within a Library instance
+```
+<Phase>[<CpuArch>][<Vendor>]<LibraryClassName>[<Dependency>].inf
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm,
+ StandaloneMm, Smm, Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64_Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+ <LibraryClassName> REQUIRED *
+ <Dependency> OPTIONAL * (Typically name of PPI, Protocol, LibraryClass
+ that the implementation is layered on top of)
+
+Example:
+ - SmmAmdSmmCpuFeaturesLib.inf
+ - SmmIa32X64SmmCpuFeaturesLib.inf
+```
+
+#### 4.3.5.4 EDK II source files within a Library/Module instance
+
+In generally, the file name is constructed as below:
+```
+
+[<CpuArch>][<Vendor>]<FileName>.*
+
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+ <FileName> REQUIRED *
+
+Example:
+ SmmCpuFeatureLib.c
+ SmmCpuFeatureLibCommon.c
+ Ia32X64SmmCpuFeaturesLib.c
+ Ia32X64IntelSmmCpuFeaturesLib.c
+ AmdSmmCpuFeaturesLib.c
+
+```
\ No newline at end of file
--
2.37.1.windows.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-15 11:47 [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections Chang, Abner
@ 2022-10-18 6:01 ` Ni, Ray
2022-10-18 9:50 ` Chang, Abner
2022-10-18 13:33 ` Sunil V L
2022-10-19 2:24 ` Attar, AbdulLateef (Abdul Lateef)
2 siblings, 1 reply; 9+ messages in thread
From: Ni, Ray @ 2022-10-18 6:01 UTC (permalink / raw)
To: Chang, Abner, devel
[-- Attachment #1: Type: text/plain, Size: 395 bytes --]
All look good to me. Thanks for addressing my comments regarding simplifying the rules.
Except for one minor comment: I still don't think we need to define rules for source file names (4.3.5.4 EDK II source files within a Library/Module instance).
And the rule "[<CpuArch>][<Vendor>]<FileName>.*" doesn't specify what <FileName> could be. That leads to allowing any style of the file name.
[-- Attachment #2: Type: text/html, Size: 419 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-18 6:01 ` [edk2-devel] " Ni, Ray
@ 2022-10-18 9:50 ` Chang, Abner
2022-10-19 1:31 ` Chang, Abner
2022-10-19 3:34 ` Ni, Ray
0 siblings, 2 replies; 9+ messages in thread
From: Chang, Abner @ 2022-10-18 9:50 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]
[AMD Official Use Only - General]
Hi Ray,
<CpuArch>][<Vendor>]<FileName>.*
The simple <FileName> naming rule was already defined in the 4.3.1 to 4.3.4 sections. We are not intending for defining the file naming style. This section is mainly for the format of attaching <CpuArch> and <Vendor> to the filename.
For example, how do we add AMD to SmmCpuFeaturesLib? Is "Amd"SmmCpuFeaturesLib or SmmCpuFeaturesLib"Amd"? This is what we would like to define for the processor archs and vendors.
My opinion is we can just leave the file format naming as it was defined in this spec.
Or how about if we say "Refer to 4.3.1 to 4.3.4 sections for the file naming format" for <FileNmae> in below?
[<CpuArch>][<Vendor>]<FileName>.*
<CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
<arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
'LoongArch64' | 'Ebc'
<CpuArch> ::= <arch>[<arch>]*
Example: Ia32X64Arm or RiscV64LoongArch64
<Vendor> OPTIONAL *
<FileName> REQUIRED Refer to 4.3.1 to 4.3.4 sections for the file
Naming format.
Thanks
Abner
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, October 18, 2022 2:02 PM
To: Chang; Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
All look good to me. Thanks for addressing my comments regarding simplifying the rules.
Except for one minor comment: I still don't think we need to define rules for source file names (4.3.5.4 EDK II source files within a Library/Module instance). And the rule "[][].*" doesn't specify what could be. That leads to allowing any style of the file name.
[-- Attachment #2: Type: text/html, Size: 8515 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-15 11:47 [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections Chang, Abner
2022-10-18 6:01 ` [edk2-devel] " Ni, Ray
@ 2022-10-18 13:33 ` Sunil V L
2022-10-18 15:23 ` Chang, Abner
2022-10-19 2:24 ` Attar, AbdulLateef (Abdul Lateef)
2 siblings, 1 reply; 9+ messages in thread
From: Sunil V L @ 2022-10-18 13:33 UTC (permalink / raw)
To: abner.chang
Cc: devel, Ray Ni, Michael D Kinney, Abdul Lateef Attar,
Leif Lindholm
Hi Abner,
Just few minor comments below. Otherwise,
Acked-by: Sunil V L <sunilvl@ventanamicro.com>
On Sat, Oct 15, 2022 at 07:47:57PM +0800, abner.chang@amd.com wrote:
> From: Abner Chang <abner.chang@amd.com>
>
> Updates 4.2 Directory names and 4.3 file names for
> the guidelines of module directory and file naming.
>
> PR: https://github.com/tianocore-docs/edk2-CCodingStandardsSpecification/pull/2/files
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> ---
> 4_naming_conventions/42_directory_names.md | 101 +++++++++++++++++++
> 4_naming_conventions/43_file_names.md | 108 ++++++++++++++++++++-
> 2 files changed, 208 insertions(+), 1 deletion(-)
>
> diff --git a/4_naming_conventions/42_directory_names.md b/4_naming_conventions/42_directory_names.md
> index 766ccb1..959a3c9 100644
> --- a/4_naming_conventions/42_directory_names.md
> +++ b/4_naming_conventions/42_directory_names.md
> @@ -2,6 +2,7 @@
> 4.2 Directory Names
>
> Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>
> Redistribution and use in source (original document form) and 'compiled'
> forms (converted to PDF, epub, HTML and other formats) with or without
> @@ -28,3 +29,103 @@
> ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>
> -->
> +
> +## 4.2 Directory Names
> +Below sections are the directory naming guidelines for EDK II modules. The guidelines are not
> +just considering the the uniformity of directory naming, but it also provides the flexibility of
^ An extra "the".
"it also provides" -> "they also provide" ?
> +directory name construction for the scenario of different EDK II module designs; such as the
> +support for multiple processor architectures and vendors. It may require the further discussions
"the further" -> further
> +between EDK II maintainers and contributors in order to determine the best naming of the EDK II
^ remove "in order" ?
> +module directory.
> +
> +#### 4.2.1 EDKII package directory
> +
> +```
> +<PackageName>Pkg
> +
> + <PackageName> REQUIRED *
> +```
> +
> +#### 4.2.2 EDKII Module directory
> +
> +* The guideline below is applied to all CPU architectures support, specific CPU architecture and vendors support, or the implementation is shared by certain CPU archs:
Better to use architectures instead of "archs" ?
Thanks
Sunil
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-18 13:33 ` Sunil V L
@ 2022-10-18 15:23 ` Chang, Abner
0 siblings, 0 replies; 9+ messages in thread
From: Chang, Abner @ 2022-10-18 15:23 UTC (permalink / raw)
To: Sunil V L
Cc: devel@edk2.groups.io, Ray Ni, Michael D Kinney,
Attar, AbdulLateef (Abdul Lateef), Leif Lindholm
[AMD Official Use Only - General]
Just fixed they all. Will send out the next version. Thanks Sunil.
Abner
> -----Original Message-----
> From: Sunil V L <sunilvl@ventanamicro.com>
> Sent: Tuesday, October 18, 2022 9:34 PM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: devel@edk2.groups.io; Ray Ni <ray.ni@intel.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> <AbdulLateef.Attar@amd.com>; Leif Lindholm <quic_llindhol@quicinc.com>
> Subject: Re: [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard:
> Updates 4.2 and 4.3 sections
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> Just few minor comments below. Otherwise,
>
> Acked-by: Sunil V L <sunilvl@ventanamicro.com>
>
>
> On Sat, Oct 15, 2022 at 07:47:57PM +0800, abner.chang@amd.com wrote:
> > From: Abner Chang <abner.chang@amd.com>
> >
> > Updates 4.2 Directory names and 4.3 file names for the guidelines of
> > module directory and file naming.
> >
> > PR:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Ftianocore-docs%2Fedk2-
> CCodingStandardsSpecification%2Fpull%2F
> >
> 2%2Ffiles&data=05%7C01%7Cabner.chang%40amd.com%7C61ac30c1dc
> 764eb57
> >
> cd708dab10d67d2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
> 38016968
> >
> 393253054%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMzIi
> >
> LCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4OFcJ
> qJ%2FNVV
> > KwYFDxe%2BASxPZGmMj8VchZE1%2BLF03LXQ%3D&reserved=0
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Sunil V L <sunilvl@ventanamicro.com>
> > Cc: Abdul Lateef Attar <abdattar@amd.com>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > ---
> > 4_naming_conventions/42_directory_names.md | 101
> +++++++++++++++++++
> > 4_naming_conventions/43_file_names.md | 108
> ++++++++++++++++++++-
> > 2 files changed, 208 insertions(+), 1 deletion(-)
> >
> > diff --git a/4_naming_conventions/42_directory_names.md
> > b/4_naming_conventions/42_directory_names.md
> > index 766ccb1..959a3c9 100644
> > --- a/4_naming_conventions/42_directory_names.md
> > +++ b/4_naming_conventions/42_directory_names.md
> > @@ -2,6 +2,7 @@
> > 4.2 Directory Names
> >
> > Copyright (C) 2022 Advanced Micro Devices, Inc. All rights
> > reserved.<BR>
> > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> >
> > Redistribution and use in source (original document form) and 'compiled'
> > forms (converted to PDF, epub, HTML and other formats) with or
> > without @@ -28,3 +29,103 @@
> > ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >
> > -->
> > +
> > +## 4.2 Directory Names
> > +Below sections are the directory naming guidelines for EDK II
> > +modules. The guidelines are not just considering the the uniformity
> > +of directory naming, but it also provides the flexibility of
> ^ An extra "the".
>
> "it also provides" -> "they also provide" ?
>
> > +directory name construction for the scenario of different EDK II
> > +module designs; such as the support for multiple processor
> > +architectures and vendors. It may require the further discussions
>
> "the further" -> further
>
> > +between EDK II maintainers and contributors in order to determine the
> > +best naming of the EDK II
> ^ remove "in order" ?
> > +module directory.
> > +
> > +#### 4.2.1 EDKII package directory
> > +
> > +```
> > +<PackageName>Pkg
> > +
> > + <PackageName> REQUIRED *
> > +```
> > +
> > +#### 4.2.2 EDKII Module directory
> > +
> > +* The guideline below is applied to all CPU architectures support, specific
> CPU architecture and vendors support, or the implementation is shared by
> certain CPU archs:
>
> Better to use architectures instead of "archs" ?
>
> Thanks
> Sunil
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-18 9:50 ` Chang, Abner
@ 2022-10-19 1:31 ` Chang, Abner
2022-10-19 3:34 ` Ni, Ray
1 sibling, 0 replies; 9+ messages in thread
From: Chang, Abner @ 2022-10-19 1:31 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
[-- Attachment #1.1: Type: text/plain, Size: 2618 bytes --]
[AMD Official Use Only - General]
BTW Ray, you can see SmmCpuFeaturesLib is renamed to IntelSmmCpuFeaturesLib in the 1/2 of SmmCpuFeatureLib rearchitecture patch. AMD will upstream its own SmmCpuFeaturesLib named as AmdSmmCpuFeaturesLib later. The naming is follow the above guidelines.
Abner
From: Chang, Abner
Sent: Tuesday, October 18, 2022 5:50 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
[AMD Official Use Only - General]
Hi Ray,
<CpuArch>][<Vendor>]<FileName>.*
The simple <FileName> naming rule was already defined in the 4.3.1 to 4.3.4 sections. We are not intending for defining the file naming style. This section is mainly for the format of attaching <CpuArch> and <Vendor> to the filename.
For example, how do we add AMD to SmmCpuFeaturesLib? Is "Amd"SmmCpuFeaturesLib or SmmCpuFeaturesLib"Amd"? This is what we would like to define for the processor archs and vendors.
My opinion is we can just leave the file format naming as it was defined in this spec.
Or how about if we say "Refer to 4.3.1 to 4.3.4 sections for the file naming format" for <FileNmae> in below?
[<CpuArch>][<Vendor>]<FileName>.*
<CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
<arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
'LoongArch64' | 'Ebc'
<CpuArch> ::= <arch>[<arch>]*
Example: Ia32X64Arm or RiscV64LoongArch64
<Vendor> OPTIONAL *
<FileName> REQUIRED Refer to 4.3.1 to 4.3.4 sections for the file
Naming format.
Thanks
Abner
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, October 18, 2022 2:02 PM
To: Chang; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
All look good to me. Thanks for addressing my comments regarding simplifying the rules.
Except for one minor comment: I still don't think we need to define rules for source file names (4.3.5.4 EDK II source files within a Library/Module instance). And the rule "[][].*" doesn't specify what could be. That leads to allowing any style of the file name.
[-- Attachment #1.2: Type: text/html, Size: 9996 bytes --]
[-- Attachment #2: Type: message/rfc822, Size: 44995 bytes --]
From: "Chang, Abner via groups.io" <abner.chang=amd.com@groups.io>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Attar, AbdulLateef (Abdul Lateef)" <AbdulLateef.Attar@amd.com>, "Kirkendall, Garrett" <Garrett.Kirkendall@amd.com>, "Grimes, Paul" <Paul.Grimes@amd.com>, Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>, Rahul Kumar <rahul1.kumar@intel.com>
Subject: [edk2-devel] [PATCH 1/2] UefiCpuPkg/SmmCpuFeaturesLib: Abstract arch dependent code
Date: Fri, 30 Sep 2022 09:52:28 +0000
Message-ID: <17199AA59D1EDE0C.23184@groups.io>
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
From: Abner Chang <abner.chang@amd.com>
This change strips away Intel X86 implementation and put
it in the IntelSmmCpuFeatureLib
Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Abdul Lateef Attar <abdattar@amd.com>
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Cc: Paul Grimes <paul.grimes@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
.../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 1 +
.../SmmCpuFeaturesLibStm.inf | 1 +
.../StandaloneMmCpuFeaturesLib.inf | 1 +
.../SmmCpuFeaturesLib/CpuFeaturesLib.h | 6 +
.../IntelSmmCpuFeaturesLib.c | 403 ++++++++++++++++++
.../SmmCpuFeaturesLibCommon.c | 391 +----------------
6 files changed, 413 insertions(+), 390 deletions(-)
create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 7b5cef97008..9ac7dde78f8 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -18,6 +18,7 @@
[Sources]
CpuFeaturesLib.h
+ IntelSmmCpuFeaturesLib.c
SmmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
SmmCpuFeaturesLibNoStm.c
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 85214ee31cd..86d367e0a09 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -19,6 +19,7 @@
[Sources]
CpuFeaturesLib.h
+ IntelSmmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
SmmStm.c
SmmStm.h
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
index 3eacab48db3..61890205e18 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
@@ -20,6 +20,7 @@
[Sources]
CpuFeaturesLib.h
+ IntelSmmCpuFeaturesLib.c
StandaloneMmCpuFeaturesLib.c
SmmCpuFeaturesLibCommon.c
SmmCpuFeaturesLibNoStm.c
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
index 8a1c2adc5c4..fd3e902547c 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -9,6 +9,12 @@
#ifndef CPU_FEATURES_LIB_H_
#define CPU_FEATURES_LIB_H_
+#include <Library/SmmCpuFeaturesLib.h>
+#include <Library/BaseLib.h>
+#include <Library/PcdLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/DebugLib.h>
+
/**
Performs library initialization.
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
new file mode 100644
index 00000000000..cb4897b21e3
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/IntelSmmCpuFeaturesLib.c
@@ -0,0 +1,403 @@
+/** @file
+Implementation shared across all library instances.
+
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "CpuFeaturesLib.h"
+
+#include <Library/MtrrLib.h>
+#include <Register/Intel/Cpuid.h>
+#include <Register/Intel/SmramSaveStateMap.h>
+
+//
+// Machine Specific Registers (MSRs)
+//
+#define SMM_FEATURES_LIB_IA32_MTRR_CAP 0x0FE
+#define SMM_FEATURES_LIB_IA32_FEATURE_CONTROL 0x03A
+#define SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE 0x1F2
+#define SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK 0x1F3
+#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE 0x0A0
+#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK 0x0A1
+#define EFI_MSR_SMRR_MASK 0xFFFFF000
+#define EFI_MSR_SMRR_PHYS_MASK_VALID BIT11
+#define SMM_FEATURES_LIB_SMM_FEATURE_CONTROL 0x4E0
+
+//
+// MSRs required for configuration of SMM Code Access Check
+//
+#define SMM_FEATURES_LIB_IA32_MCA_CAP 0x17D
+#define SMM_CODE_ACCESS_CHK_BIT BIT58
+
+//
+// Set default value to assume IA-32 Architectural MSRs are used
+//
+UINT32 mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
+UINT32 mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
+
+//
+// Set default value to assume MTRRs need to be configured on each SMI
+//
+BOOLEAN mNeedConfigureMtrrs = TRUE;
+
+//
+// Array for state of SMRR enable on all CPUs
+//
+BOOLEAN *mSmrrEnabled;
+
+/**
+ Performs library initialization.
+
+ This initialization function contains common functionality shared betwen all
+ library instance constructors.
+
+**/
+VOID
+CpuFeaturesLibInitialization (
+ VOID
+ )
+{
+ UINT32 RegEax;
+ UINT32 RegEdx;
+ UINTN FamilyId;
+ UINTN ModelId;
+
+ //
+ // Retrieve CPU Family and Model
+ //
+ AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
+ FamilyId = (RegEax >> 8) & 0xf;
+ ModelId = (RegEax >> 4) & 0xf;
+ if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
+ ModelId = ModelId | ((RegEax >> 12) & 0xf0);
+ }
+
+ //
+ // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability
+ //
+ if ((RegEdx & BIT12) != 0) {
+ //
+ // Check MTRR_CAP MSR bit 11 for SMRR support
+ //
+ if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) != 0) {
+ ASSERT (FeaturePcdGet (PcdSmrrEnable));
+ }
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.3 MSRs in the Intel(R) Atom(TM) Processor Family
+ //
+ // If CPU Family/Model is 06_1CH, 06_26H, 06_27H, 06_35H or 06_36H, then
+ // SMRR Physical Base and SMM Physical Mask MSRs are not available.
+ //
+ if (FamilyId == 0x06) {
+ if ((ModelId == 0x1C) || (ModelId == 0x26) || (ModelId == 0x27) || (ModelId == 0x35) || (ModelId == 0x36)) {
+ ASSERT (!FeaturePcdGet (PcdSmrrEnable));
+ }
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
+ //
+ // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
+ // Processor Family MSRs
+ //
+ if (FamilyId == 0x06) {
+ if ((ModelId == 0x17) || (ModelId == 0x0f)) {
+ mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
+ mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
+ }
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 34.4.2 SMRAM Caching
+ // An IA-32 processor does not automatically write back and invalidate its
+ // caches before entering SMM or before exiting SMM. Because of this behavior,
+ // care must be taken in the placement of the SMRAM in system memory and in
+ // the caching of the SMRAM to prevent cache incoherence when switching back
+ // and forth between SMM and protected mode operation.
+ //
+ // An IA-32 processor is a processor that does not support the Intel 64
+ // Architecture. Support for the Intel 64 Architecture can be detected from
+ // CPUID(CPUID_EXTENDED_CPU_SIG).EDX[29]
+ //
+ // If an IA-32 processor is detected, then set mNeedConfigureMtrrs to TRUE,
+ // so caches are flushed on SMI entry and SMI exit, the interrupted code
+ // MTRRs are saved/restored, and MTRRs for SMM are loaded.
+ //
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
+ AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & BIT29) != 0) {
+ mNeedConfigureMtrrs = FALSE;
+ }
+ }
+
+ //
+ // Allocate array for state of SMRR enable on all CPUs
+ //
+ mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * GetCpuMaxLogicalProcessorNumber ());
+ ASSERT (mSmrrEnabled != NULL);
+}
+
+/**
+ Called during the very first SMI into System Management Mode to initialize
+ CPU features, including SMBASE, for the currently executing CPU. Since this
+ is the first SMI, the SMRAM Save State Map is at the default address of
+ SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET. The currently executing
+ CPU is specified by CpuIndex and CpuIndex can be used to access information
+ about the currently executing CPU in the ProcessorInfo array and the
+ HotPlugCpuData data structure.
+
+ @param[in] CpuIndex The index of the CPU to initialize. The value
+ must be between 0 and the NumberOfCpus field in
+ the System Management System Table (SMST).
+ @param[in] IsMonarch TRUE if the CpuIndex is the index of the CPU that
+ was elected as monarch during System Management
+ Mode initialization.
+ FALSE if the CpuIndex is not the index of the CPU
+ that was elected as monarch during System
+ Management Mode initialization.
+ @param[in] ProcessorInfo Pointer to an array of EFI_PROCESSOR_INFORMATION
+ structures. ProcessorInfo[CpuIndex] contains the
+ information for the currently executing CPU.
+ @param[in] CpuHotPlugData Pointer to the CPU_HOT_PLUG_DATA structure that
+ contains the ApidId and SmBase arrays.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesInitializeProcessor (
+ IN UINTN CpuIndex,
+ IN BOOLEAN IsMonarch,
+ IN EFI_PROCESSOR_INFORMATION *ProcessorInfo,
+ IN CPU_HOT_PLUG_DATA *CpuHotPlugData
+ )
+{
+ SMRAM_SAVE_STATE_MAP *CpuState;
+ UINT64 FeatureControl;
+ UINT32 RegEax;
+ UINT32 RegEdx;
+ UINTN FamilyId;
+ UINTN ModelId;
+
+ //
+ // Configure SMBASE.
+ //
+ CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
+ CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
+ //
+ // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used, then
+ // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is set before
+ // accessing SMRR base/mask MSRs. If Lock(BIT0) of MSR_FEATURE_CONTROL MSR(0x3A)
+ // is set, then the MSR is locked and can not be modified.
+ //
+ if ((FeaturePcdGet (PcdSmrrEnable)) && (mSmrrPhysBaseMsr == SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE)) {
+ FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
+ if ((FeatureControl & BIT3) == 0) {
+ ASSERT ((FeatureControl & BIT0) == 0);
+ if ((FeatureControl & BIT0) == 0) {
+ AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureControl | BIT3);
+ }
+ }
+ }
+
+ //
+ // If SMRR is supported, then program SMRR base/mask MSRs.
+ // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first normal SMI.
+ // The code that initializes SMM environment is running in normal mode
+ // from SMRAM region. If SMRR is enabled here, then the SMRAM region
+ // is protected and the normal mode code execution will fail.
+ //
+ if (FeaturePcdGet (PcdSmrrEnable)) {
+ //
+ // SMRR size cannot be less than 4-KBytes
+ // SMRR size must be of length 2^n
+ // SMRR base alignment cannot be less than SMRR length
+ //
+ if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
+ (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData->SmrrSize)) ||
+ ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1)) != CpuHotPlugData->SmrrBase))
+ {
+ //
+ // Print message and halt if CPU is Monarch
+ //
+ if (IsMonarch) {
+ DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet alignment/size requirement!\n"));
+ CpuDeadLoop ();
+ }
+ } else {
+ AsmWriteMsr64 (mSmrrPhysBaseMsr, CpuHotPlugData->SmrrBase | MTRR_CACHE_WRITE_BACK);
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1) & EFI_MSR_SMRR_MASK));
+ mSmrrEnabled[CpuIndex] = FALSE;
+ }
+ }
+
+ //
+ // Retrieve CPU Family and Model
+ //
+ AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
+ FamilyId = (RegEax >> 8) & 0xf;
+ ModelId = (RegEax >> 4) & 0xf;
+ if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
+ ModelId = ModelId | ((RegEax >> 12) & 0xf0);
+ }
+
+ //
+ // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM)
+ // Processor Family.
+ //
+ // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation
+ // Intel(R) Core(TM) Processor Family MSRs.
+ //
+ if (FamilyId == 0x06) {
+ if ((ModelId == 0x3C) || (ModelId == 0x45) || (ModelId == 0x46) ||
+ (ModelId == 0x3D) || (ModelId == 0x47) || (ModelId == 0x4E) || (ModelId == 0x4F) ||
+ (ModelId == 0x3F) || (ModelId == 0x56) || (ModelId == 0x57) || (ModelId == 0x5C) ||
+ (ModelId == 0x8C))
+ {
+ //
+ // Check to see if the CPU supports the SMM Code Access Check feature
+ // Do not access this MSR unless the CPU supports the SmmRegFeatureControl
+ //
+ if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) & SMM_CODE_ACCESS_CHK_BIT) != 0) {
+ ASSERT (FeaturePcdGet (PcdSmmFeatureControlEnable));
+ }
+ }
+ }
+
+ //
+ // Call internal worker function that completes the CPU initialization
+ //
+ FinishSmmCpuFeaturesInitializeProcessor ();
+}
+
+/**
+ Determines if MTRR registers must be configured to set SMRAM cache-ability
+ when executing in System Management Mode.
+
+ @retval TRUE MTRR registers must be configured to set SMRAM cache-ability.
+ @retval FALSE MTRR registers do not need to be configured to set SMRAM
+ cache-ability.
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesNeedConfigureMtrrs (
+ VOID
+ )
+{
+ return mNeedConfigureMtrrs;
+}
+
+/**
+ Disable SMRR register if SMRR is supported and SmmCpuFeaturesNeedConfigureMtrrs()
+ returns TRUE.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesDisableSmrr (
+ VOID
+ )
+{
+ if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
+ }
+}
+
+/**
+ Enable SMRR register if SMRR is supported and SmmCpuFeaturesNeedConfigureMtrrs()
+ returns TRUE.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesReenableSmrr (
+ VOID
+ )
+{
+ if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
+ }
+}
+
+/**
+ Processor specific hook point each time a CPU enters System Management Mode.
+
+ @param[in] CpuIndex The index of the CPU that has entered SMM. The value
+ must be between 0 and the NumberOfCpus field in the
+ System Management System Table (SMST).
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesRendezvousEntry (
+ IN UINTN CpuIndex
+ )
+{
+ //
+ // If SMRR is supported and this is the first normal SMI, then enable SMRR
+ //
+ if (FeaturePcdGet (PcdSmrrEnable) && !mSmrrEnabled[CpuIndex]) {
+ AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
+ mSmrrEnabled[CpuIndex] = TRUE;
+ }
+}
+
+/**
+ Returns the current value of the SMM register for the specified CPU.
+ If the SMM register is not supported, then 0 is returned.
+
+ @param[in] CpuIndex The index of the CPU to read the SMM register. The
+ value must be between 0 and the NumberOfCpus field in
+ the System Management System Table (SMST).
+ @param[in] RegName Identifies the SMM register to read.
+
+ @return The value of the SMM register specified by RegName from the CPU
+ specified by CpuIndex.
+**/
+UINT64
+EFIAPI
+SmmCpuFeaturesGetSmmRegister (
+ IN UINTN CpuIndex,
+ IN SMM_REG_NAME RegName
+ )
+{
+ if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
+ return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
+ }
+
+ return 0;
+}
+
+/**
+ Sets the value of an SMM register on a specified CPU.
+ If the SMM register is not supported, then no action is performed.
+
+ @param[in] CpuIndex The index of the CPU to write the SMM register. The
+ value must be between 0 and the NumberOfCpus field in
+ the System Management System Table (SMST).
+ @param[in] RegName Identifies the SMM register to write.
+ registers are read-only.
+ @param[in] Value The value to write to the SMM register.
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesSetSmmRegister (
+ IN UINTN CpuIndex,
+ IN SMM_REG_NAME RegName,
+ IN UINT64 Value
+ )
+{
+ if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
+ AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL, Value);
+ }
+}
+
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 75a0ec8e948..7777e52740e 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -14,278 +14,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/PcdLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/DebugLib.h>
-#include <Register/Intel/Cpuid.h>
-#include <Register/Intel/SmramSaveStateMap.h>
-#include "CpuFeaturesLib.h"
-
-//
-// Machine Specific Registers (MSRs)
-//
-#define SMM_FEATURES_LIB_IA32_MTRR_CAP 0x0FE
-#define SMM_FEATURES_LIB_IA32_FEATURE_CONTROL 0x03A
-#define SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE 0x1F2
-#define SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK 0x1F3
-#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE 0x0A0
-#define SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK 0x0A1
-#define EFI_MSR_SMRR_MASK 0xFFFFF000
-#define EFI_MSR_SMRR_PHYS_MASK_VALID BIT11
-#define SMM_FEATURES_LIB_SMM_FEATURE_CONTROL 0x4E0
-
-//
-// MSRs required for configuration of SMM Code Access Check
-//
-#define SMM_FEATURES_LIB_IA32_MCA_CAP 0x17D
-#define SMM_CODE_ACCESS_CHK_BIT BIT58
-
-//
-// Set default value to assume IA-32 Architectural MSRs are used
-//
-UINT32 mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
-UINT32 mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
-
-//
-// Set default value to assume MTRRs need to be configured on each SMI
-//
-BOOLEAN mNeedConfigureMtrrs = TRUE;
-
-//
-// Array for state of SMRR enable on all CPUs
-//
-BOOLEAN *mSmrrEnabled;
-
-/**
- Performs library initialization.
-
- This initialization function contains common functionality shared betwen all
- library instance constructors.
-
-**/
-VOID
-CpuFeaturesLibInitialization (
- VOID
- )
-{
- UINT32 RegEax;
- UINT32 RegEdx;
- UINTN FamilyId;
- UINTN ModelId;
-
- //
- // Retrieve CPU Family and Model
- //
- AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
- FamilyId = (RegEax >> 8) & 0xf;
- ModelId = (RegEax >> 4) & 0xf;
- if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
- ModelId = ModelId | ((RegEax >> 12) & 0xf0);
- }
-
- //
- // Check CPUID(CPUID_VERSION_INFO).EDX[12] for MTRR capability
- //
- if ((RegEdx & BIT12) != 0) {
- //
- // Check MTRR_CAP MSR bit 11 for SMRR support
- //
- if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MTRR_CAP) & BIT11) != 0) {
- ASSERT (FeaturePcdGet (PcdSmrrEnable));
- }
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.3 MSRs in the Intel(R) Atom(TM) Processor Family
- //
- // If CPU Family/Model is 06_1CH, 06_26H, 06_27H, 06_35H or 06_36H, then
- // SMRR Physical Base and SMM Physical Mask MSRs are not available.
- //
- if (FamilyId == 0x06) {
- if ((ModelId == 0x1C) || (ModelId == 0x26) || (ModelId == 0x27) || (ModelId == 0x35) || (ModelId == 0x36)) {
- ASSERT (!FeaturePcdGet (PcdSmrrEnable));
- }
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
- //
- // If CPU Family/Model is 06_0F or 06_17, then use Intel(R) Core(TM) 2
- // Processor Family MSRs
- //
- if (FamilyId == 0x06) {
- if ((ModelId == 0x17) || (ModelId == 0x0f)) {
- mSmrrPhysBaseMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE;
- mSmrrPhysMaskMsr = SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSMASK;
- }
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 34.4.2 SMRAM Caching
- // An IA-32 processor does not automatically write back and invalidate its
- // caches before entering SMM or before exiting SMM. Because of this behavior,
- // care must be taken in the placement of the SMRAM in system memory and in
- // the caching of the SMRAM to prevent cache incoherence when switching back
- // and forth between SMM and protected mode operation.
- //
- // An IA-32 processor is a processor that does not support the Intel 64
- // Architecture. Support for the Intel 64 Architecture can be detected from
- // CPUID(CPUID_EXTENDED_CPU_SIG).EDX[29]
- //
- // If an IA-32 processor is detected, then set mNeedConfigureMtrrs to TRUE,
- // so caches are flushed on SMI entry and SMI exit, the interrupted code
- // MTRRs are saved/restored, and MTRRs for SMM are loaded.
- //
- AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
- if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
- AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &RegEdx);
- if ((RegEdx & BIT29) != 0) {
- mNeedConfigureMtrrs = FALSE;
- }
- }
-
- //
- // Allocate array for state of SMRR enable on all CPUs
- //
- mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * GetCpuMaxLogicalProcessorNumber ());
- ASSERT (mSmrrEnabled != NULL);
-}
-
-/**
- Called during the very first SMI into System Management Mode to initialize
- CPU features, including SMBASE, for the currently executing CPU. Since this
- is the first SMI, the SMRAM Save State Map is at the default address of
- SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET. The currently executing
- CPU is specified by CpuIndex and CpuIndex can be used to access information
- about the currently executing CPU in the ProcessorInfo array and the
- HotPlugCpuData data structure.
-
- @param[in] CpuIndex The index of the CPU to initialize. The value
- must be between 0 and the NumberOfCpus field in
- the System Management System Table (SMST).
- @param[in] IsMonarch TRUE if the CpuIndex is the index of the CPU that
- was elected as monarch during System Management
- Mode initialization.
- FALSE if the CpuIndex is not the index of the CPU
- that was elected as monarch during System
- Management Mode initialization.
- @param[in] ProcessorInfo Pointer to an array of EFI_PROCESSOR_INFORMATION
- structures. ProcessorInfo[CpuIndex] contains the
- information for the currently executing CPU.
- @param[in] CpuHotPlugData Pointer to the CPU_HOT_PLUG_DATA structure that
- contains the ApidId and SmBase arrays.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesInitializeProcessor (
- IN UINTN CpuIndex,
- IN BOOLEAN IsMonarch,
- IN EFI_PROCESSOR_INFORMATION *ProcessorInfo,
- IN CPU_HOT_PLUG_DATA *CpuHotPlugData
- )
-{
- SMRAM_SAVE_STATE_MAP *CpuState;
- UINT64 FeatureControl;
- UINT32 RegEax;
- UINT32 RegEdx;
- UINTN FamilyId;
- UINTN ModelId;
-
- //
- // Configure SMBASE.
- //
- CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
- CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.2 MSRs in the Intel(R) Core(TM) 2 Processor Family
- //
- // If Intel(R) Core(TM) Core(TM) 2 Processor Family MSRs are being used, then
- // make sure SMRR Enable(BIT3) of MSR_FEATURE_CONTROL MSR(0x3A) is set before
- // accessing SMRR base/mask MSRs. If Lock(BIT0) of MSR_FEATURE_CONTROL MSR(0x3A)
- // is set, then the MSR is locked and can not be modified.
- //
- if ((FeaturePcdGet (PcdSmrrEnable)) && (mSmrrPhysBaseMsr == SMM_FEATURES_LIB_IA32_CORE_SMRR_PHYSBASE)) {
- FeatureControl = AsmReadMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL);
- if ((FeatureControl & BIT3) == 0) {
- ASSERT ((FeatureControl & BIT0) == 0);
- if ((FeatureControl & BIT0) == 0) {
- AsmWriteMsr64 (SMM_FEATURES_LIB_IA32_FEATURE_CONTROL, FeatureControl | BIT3);
- }
- }
- }
- //
- // If SMRR is supported, then program SMRR base/mask MSRs.
- // The EFI_MSR_SMRR_PHYS_MASK_VALID bit is not set until the first normal SMI.
- // The code that initializes SMM environment is running in normal mode
- // from SMRAM region. If SMRR is enabled here, then the SMRAM region
- // is protected and the normal mode code execution will fail.
- //
- if (FeaturePcdGet (PcdSmrrEnable)) {
- //
- // SMRR size cannot be less than 4-KBytes
- // SMRR size must be of length 2^n
- // SMRR base alignment cannot be less than SMRR length
- //
- if ((CpuHotPlugData->SmrrSize < SIZE_4KB) ||
- (CpuHotPlugData->SmrrSize != GetPowerOfTwo32 (CpuHotPlugData->SmrrSize)) ||
- ((CpuHotPlugData->SmrrBase & ~(CpuHotPlugData->SmrrSize - 1)) != CpuHotPlugData->SmrrBase))
- {
- //
- // Print message and halt if CPU is Monarch
- //
- if (IsMonarch) {
- DEBUG ((DEBUG_ERROR, "SMM Base/Size does not meet alignment/size requirement!\n"));
- CpuDeadLoop ();
- }
- } else {
- AsmWriteMsr64 (mSmrrPhysBaseMsr, CpuHotPlugData->SmrrBase | MTRR_CACHE_WRITE_BACK);
- AsmWriteMsr64 (mSmrrPhysMaskMsr, (~(CpuHotPlugData->SmrrSize - 1) & EFI_MSR_SMRR_MASK));
- mSmrrEnabled[CpuIndex] = FALSE;
- }
- }
-
- //
- // Retrieve CPU Family and Model
- //
- AsmCpuid (CPUID_VERSION_INFO, &RegEax, NULL, NULL, &RegEdx);
- FamilyId = (RegEax >> 8) & 0xf;
- ModelId = (RegEax >> 4) & 0xf;
- if ((FamilyId == 0x06) || (FamilyId == 0x0f)) {
- ModelId = ModelId | ((RegEax >> 12) & 0xf0);
- }
-
- //
- // Intel(R) 64 and IA-32 Architectures Software Developer's Manual
- // Volume 3C, Section 35.10.1 MSRs in 4th Generation Intel(R) Core(TM)
- // Processor Family.
- //
- // If CPU Family/Model is 06_3C, 06_45, or 06_46 then use 4th Generation
- // Intel(R) Core(TM) Processor Family MSRs.
- //
- if (FamilyId == 0x06) {
- if ((ModelId == 0x3C) || (ModelId == 0x45) || (ModelId == 0x46) ||
- (ModelId == 0x3D) || (ModelId == 0x47) || (ModelId == 0x4E) || (ModelId == 0x4F) ||
- (ModelId == 0x3F) || (ModelId == 0x56) || (ModelId == 0x57) || (ModelId == 0x5C) ||
- (ModelId == 0x8C))
- {
- //
- // Check to see if the CPU supports the SMM Code Access Check feature
- // Do not access this MSR unless the CPU supports the SmmRegFeatureControl
- //
- if ((AsmReadMsr64 (SMM_FEATURES_LIB_IA32_MCA_CAP) & SMM_CODE_ACCESS_CHK_BIT) != 0) {
- ASSERT (FeaturePcdGet (PcdSmmFeatureControlEnable));
- }
- }
- }
-
- //
- // Call internal worker function that completes the CPU initialization
- //
- FinishSmmCpuFeaturesInitializeProcessor ();
-}
+#include "CpuFeaturesLib.h"
/**
This function updates the SMRAM save state on the currently executing CPU
@@ -345,75 +75,6 @@ SmmCpuFeaturesSmmRelocationComplete (
{
}
-/**
- Determines if MTRR registers must be configured to set SMRAM cache-ability
- when executing in System Management Mode.
-
- @retval TRUE MTRR registers must be configured to set SMRAM cache-ability.
- @retval FALSE MTRR registers do not need to be configured to set SMRAM
- cache-ability.
-**/
-BOOLEAN
-EFIAPI
-SmmCpuFeaturesNeedConfigureMtrrs (
- VOID
- )
-{
- return mNeedConfigureMtrrs;
-}
-
-/**
- Disable SMRR register if SMRR is supported and SmmCpuFeaturesNeedConfigureMtrrs()
- returns TRUE.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesDisableSmrr (
- VOID
- )
-{
- if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
- AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) & ~EFI_MSR_SMRR_PHYS_MASK_VALID);
- }
-}
-
-/**
- Enable SMRR register if SMRR is supported and SmmCpuFeaturesNeedConfigureMtrrs()
- returns TRUE.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesReenableSmrr (
- VOID
- )
-{
- if (FeaturePcdGet (PcdSmrrEnable) && mNeedConfigureMtrrs) {
- AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
- }
-}
-
-/**
- Processor specific hook point each time a CPU enters System Management Mode.
-
- @param[in] CpuIndex The index of the CPU that has entered SMM. The value
- must be between 0 and the NumberOfCpus field in the
- System Management System Table (SMST).
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesRendezvousEntry (
- IN UINTN CpuIndex
- )
-{
- //
- // If SMRR is supported and this is the first normal SMI, then enable SMRR
- //
- if (FeaturePcdGet (PcdSmrrEnable) && !mSmrrEnabled[CpuIndex]) {
- AsmWriteMsr64 (mSmrrPhysMaskMsr, AsmReadMsr64 (mSmrrPhysMaskMsr) | EFI_MSR_SMRR_PHYS_MASK_VALID);
- mSmrrEnabled[CpuIndex] = TRUE;
- }
-}
-
/**
Processor specific hook point each time a CPU exits System Management Mode.
@@ -456,56 +117,6 @@ SmmCpuFeaturesIsSmmRegisterSupported (
return FALSE;
}
-/**
- Returns the current value of the SMM register for the specified CPU.
- If the SMM register is not supported, then 0 is returned.
-
- @param[in] CpuIndex The index of the CPU to read the SMM register. The
- value must be between 0 and the NumberOfCpus field in
- the System Management System Table (SMST).
- @param[in] RegName Identifies the SMM register to read.
-
- @return The value of the SMM register specified by RegName from the CPU
- specified by CpuIndex.
-**/
-UINT64
-EFIAPI
-SmmCpuFeaturesGetSmmRegister (
- IN UINTN CpuIndex,
- IN SMM_REG_NAME RegName
- )
-{
- if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
- return AsmReadMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL);
- }
-
- return 0;
-}
-
-/**
- Sets the value of an SMM register on a specified CPU.
- If the SMM register is not supported, then no action is performed.
-
- @param[in] CpuIndex The index of the CPU to write the SMM register. The
- value must be between 0 and the NumberOfCpus field in
- the System Management System Table (SMST).
- @param[in] RegName Identifies the SMM register to write.
- registers are read-only.
- @param[in] Value The value to write to the SMM register.
-**/
-VOID
-EFIAPI
-SmmCpuFeaturesSetSmmRegister (
- IN UINTN CpuIndex,
- IN SMM_REG_NAME RegName,
- IN UINT64 Value
- )
-{
- if (FeaturePcdGet (PcdSmmFeatureControlEnable) && (RegName == SmmRegFeatureControl)) {
- AsmWriteMsr64 (SMM_FEATURES_LIB_SMM_FEATURE_CONTROL, Value);
- }
-}
-
/**
Read an SMM Save State register on the target processor. If this function
returns EFI_UNSUPPORTED, then the caller is responsible for reading the
--
2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94568): https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F94568&data=05%7C01%7Cabner.chang%40amd.com%7C35230bb798f04e2c9f4108daa2c98ebb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638001283822981371%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=SavK8Hd8gcaNZb%2B936BKS1FrR9sFTBqUolnnPm3G5Nc%3D&reserved=0
Mute This Topic: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F94013090%2F7039027&data=05%7C01%7Cabner.chang%40amd.com%7C35230bb798f04e2c9f4108daa2c98ebb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638001283822981371%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=hZ8DVHRBl4F%2FWXxrUbBuRaLHUnmTOPGM8FnBJW1Mq0M%3D&reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&data=05%7C01%7Cabner.chang%40amd.com%7C35230bb798f04e2c9f4108daa2c98ebb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638001283823137634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=efUfZi%2BTunz9zd1%2BLMfOdze7N2ld00mfpaa0X%2BZWURQ%3D&reserved=0 [abner.chang@amd.com]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-15 11:47 [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections Chang, Abner
2022-10-18 6:01 ` [edk2-devel] " Ni, Ray
2022-10-18 13:33 ` Sunil V L
@ 2022-10-19 2:24 ` Attar, AbdulLateef (Abdul Lateef)
2 siblings, 0 replies; 9+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2022-10-19 2:24 UTC (permalink / raw)
To: Chang, Abner, devel@edk2.groups.io
Cc: Ray Ni, Michael D Kinney, Sunil V L, Leif Lindholm
[AMD Official Use Only - General]
Looks good to me
Reviewed-by: Abdul Lateef Attar<AbdulLateef.Attar@amd.com>
-----Original Message-----
From: Chang, Abner <Abner.Chang@amd.com>
Sent: 15 October 2022 17:18
To: devel@edk2.groups.io
Cc: Ray Ni <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>; Sunil V L <sunilvl@ventanamicro.com>; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Leif Lindholm <quic_llindhol@quicinc.com>
Subject: [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
From: Abner Chang <abner.chang@amd.com>
Updates 4.2 Directory names and 4.3 file names for the guidelines of module directory and file naming.
PR: https://github.com/tianocore-docs/edk2-CCodingStandardsSpecification/pull/2/files
Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Abdul Lateef Attar <abdattar@amd.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
---
4_naming_conventions/42_directory_names.md | 101 +++++++++++++++++++
4_naming_conventions/43_file_names.md | 108 ++++++++++++++++++++-
2 files changed, 208 insertions(+), 1 deletion(-)
diff --git a/4_naming_conventions/42_directory_names.md b/4_naming_conventions/42_directory_names.md
index 766ccb1..959a3c9 100644
--- a/4_naming_conventions/42_directory_names.md
+++ b/4_naming_conventions/42_directory_names.md
@@ -2,6 +2,7 @@
4.2 Directory Names
Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
+ Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
Redistribution and use in source (original document form) and 'compiled'
forms (converted to PDF, epub, HTML and other formats) with or without @@ -28,3 +29,103 @@
ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-->
+
+## 4.2 Directory Names
+Below sections are the directory naming guidelines for EDK II modules.
+The guidelines are not just considering the the uniformity of directory
+naming, but it also provides the flexibility of directory name
+construction for the scenario of different EDK II module designs; such
+as the support for multiple processor architectures and vendors. It may
+require the further discussions between EDK II maintainers and contributors in order to determine the best naming of the EDK II module directory.
+
+#### 4.2.1 EDKII package directory
+
+```
+<PackageName>Pkg
+
+ <PackageName> REQUIRED *
+```
+
+#### 4.2.2 EDKII Module directory
+
+* The guideline below is applied to all CPU architectures support, specific CPU architecture and vendors support, or the implementation is shared by certain CPU archs:
+```
+<Feature><Phase>[<CpuArch>[<Vendor>]]
+ or
+<Feature><Phase>[/<CpuArch>[/<Vendor>]]
+
+ <Feature> REQUIRED *
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm, Smm,
+ Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+
+ <Vendor> OPTIONAL *
+
+Example:
+ - SmbiosDxe/
+ - CpuDxe/ # First implementation of CpuDxe.
+ - CpuDxeIa32X64Amd/ # Ia32 and X64 AMD specific implementation.
+ - CpuDxe/RiscV64/ # RiscV64 specific implementation.
+ / # Common files for the RiscV64 and other archs.
+ - CpuDxe/Ia32X64/Amd/ # Ia32 and X64 AMD specific implementation.
+ / # Common files for Ia32 and X64 archs.
+ /ArmAArch64/ # Arm and AArch64 implementation of CpuDxe.
+ / # Common files for the Arm, AArch64, Ia32 and X64.
+```
+
+* If the implementation does not have any shared code between phases (e.g.
+MdeModulePkg/Universal/PCD). The guideline below is applied to all CPU architectures support, specific CPU architecture and vendors support, or the implementation is shared by certain CPU archs:
+
+```
+<Feature>/<Phase>[/<CpuArch>[/<Vendor>]]
+
+ <Feature> REQUIRED *
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm, Smm,
+ Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+Example:
+ Pcd/Dxe/
+```
+
+#### 4.2.2 EDKII Library directory
+```
+<Phase>[<CpuArch>[<Vendor>]]<LibraryClassName>[<Dependency>]
+ or
+<Phase><LibraryClassName>[<Dependency>]/[<CpuArch>[/<Vendor>]]
+
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm,
+ StandaloneMm, Smm, Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+ <LibraryClassName> REQUIRED *
+ <Dependency> OPTIONAL * (Typically name of PPI, Protocol, LibraryClass
+ that the implementation is layered on top of).
+
+Example:
+ - BaseXApicLib/
+ - SmmIa32X64AmdSmmCpuFeaturesLib/
+ - SmmArmAArch64SmmCpuFeaturesLib/
+ - BaseMpInitLib/RiscV64/ # RiscV64 specific implementation.
+ /Ia32X64/ # Ia32 and X64 specific implementation.
+ /Ia32X64/Amd # Ia32 and X64 AMD specific implementation.
+ /ArmAArch64/ # Arm and AAch64 specific implementation.
+ /LoongArch64/ # LoongArch64 specific implementation.
+ / # Common files for RiscV64, Ia32, X64, Arm, AArch64 and
+ LoongArch64.
+```
diff --git a/4_naming_conventions/43_file_names.md b/4_naming_conventions/43_file_names.md
index 13e0c63..5714008 100644
--- a/4_naming_conventions/43_file_names.md
+++ b/4_naming_conventions/43_file_names.md
@@ -1,7 +1,8 @@
<!--- @file
4.3 File Names
- Copyright (c) 2006-2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2006-2022, Intel Corporation. All rights reserved.<BR>
+ Copyright (C) 2022 Advanced Micro Devices, Inc. All rights
+ reserved.<BR>
Redistribution and use in source (original document form) and 'compiled'
forms (converted to PDF, epub, HTML and other formats) with or without @@ -58,3 +59,108 @@ regular expression:
That is, a letter followed by zero, or more, letters, underscores, dashes, or digits followed by a period followed by one or more letters or digits.
+
+### 4.3.5 File naming guidelines for modules
+
+Below sections are the file naming guidelines for EDK II meta files and
+source files. The guidelines are not just considering the the
+uniformity of file naming, but it also provides the flexibility of file
+name construction for the scenario of different EDK II module designs;
+such as the support for multiple processor architectures and vendors.
+It may require the further discussions between EDK II maintainers and contributors in order to determine the best naming of the EDK II module file.
+
+#### 4.3.5.1 EDK II meta files within a package
+
+```
+<PackageName>Pkg.dec
+<PackageName>Pkg.dsc
+
+ <PackageName> REQUIRED *
+```
+
+#### 4.3.5.2 EDK II INF file within a Module instance
+* If the implementation is for all CPU architectures, specific CPU architectures, CPU vendors or the implementation is shared by certain CPU archs:
+```
+<Feature><Phase>[<CpuArch>][<Vendor>].inf
+
+ <Feature> REQUIRED *
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm, StandaloneMm,
+ Smm, Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+
+Example:
+ - SmbiosDxe.inf
+ - CpuIo2Dxe.inf
+ - CpuIo2DxeAmd.inf
+ - CpuIo2DxeIa32X64.inf
+ - CpuIo2DxeIa32X64Intel.inf
+```
+
+* If the implementation does not have any shared code between phases (e.g., Pcd/Dxe):
+
+```
+[<Feature>][<CpuArch>][<Vendor>].inf
+
+ <Feature> OPTIONAL *
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+Example:
+ Pcd.inf
+```
+
+#### 4.4.5.3 EDK II INF file within a Library instance ```
+<Phase>[<CpuArch>][<Vendor>]<LibraryClassName>[<Dependency>].inf
+ <Phase> REQUIRED Base, Sec, Pei, Dxe, DxeRuntime, Mm,
+ StandaloneMm, Smm, Uefi.
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64_Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+ <LibraryClassName> REQUIRED *
+ <Dependency> OPTIONAL * (Typically name of PPI, Protocol, LibraryClass
+ that the implementation is
+ layered on top of)
+
+Example:
+ - SmmAmdSmmCpuFeaturesLib.inf
+ - SmmIa32X64SmmCpuFeaturesLib.inf
+```
+
+#### 4.3.5.4 EDK II source files within a Library/Module instance
+
+In generally, the file name is constructed as below:
+```
+
+[<CpuArch>][<Vendor>]<FileName>.*
+
+ <CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
+ <arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
+ 'LoongArch64' | 'Ebc'
+ <CpuArch> ::= <arch>[<arch>]*
+
+ Example: Ia32X64Arm or RiscV64LoongArch64
+ <Vendor> OPTIONAL *
+ <FileName> REQUIRED *
+
+Example:
+ SmmCpuFeatureLib.c
+ SmmCpuFeatureLibCommon.c
+ Ia32X64SmmCpuFeaturesLib.c
+ Ia32X64IntelSmmCpuFeaturesLib.c
+ AmdSmmCpuFeaturesLib.c
+
+```
\ No newline at end of file
--
2.37.1.windows.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-18 9:50 ` Chang, Abner
2022-10-19 1:31 ` Chang, Abner
@ 2022-10-19 3:34 ` Ni, Ray
2022-10-19 4:17 ` Chang, Abner
1 sibling, 1 reply; 9+ messages in thread
From: Ni, Ray @ 2022-10-19 3:34 UTC (permalink / raw)
To: Chang, Abner, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]
Oh I see!
Reviewed-by: Ray Ni <ray.ni@intrel.com>
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Tuesday, October 18, 2022 5:50 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
[AMD Official Use Only - General]
Hi Ray,
<CpuArch>][<Vendor>]<FileName>.*
The simple <FileName> naming rule was already defined in the 4.3.1 to 4.3.4 sections. We are not intending for defining the file naming style. This section is mainly for the format of attaching <CpuArch> and <Vendor> to the filename.
For example, how do we add AMD to SmmCpuFeaturesLib? Is "Amd"SmmCpuFeaturesLib or SmmCpuFeaturesLib"Amd"? This is what we would like to define for the processor archs and vendors.
My opinion is we can just leave the file format naming as it was defined in this spec.
Or how about if we say "Refer to 4.3.1 to 4.3.4 sections for the file naming format" for <FileNmae> in below?
[<CpuArch>][<Vendor>]<FileName>.*
<CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
<arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
'LoongArch64' | 'Ebc'
<CpuArch> ::= <arch>[<arch>]*
Example: Ia32X64Arm or RiscV64LoongArch64
<Vendor> OPTIONAL *
<FileName> REQUIRED Refer to 4.3.1 to 4.3.4 sections for the file
Naming format.
Thanks
Abner
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, October 18, 2022 2:02 PM
To: Chang; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
All look good to me. Thanks for addressing my comments regarding simplifying the rules.
Except for one minor comment: I still don't think we need to define rules for source file names (4.3.5.4 EDK II source files within a Library/Module instance). And the rule "[][].*" doesn't specify what could be. That leads to allowing any style of the file name.
[-- Attachment #2: Type: text/html, Size: 9532 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
2022-10-19 3:34 ` Ni, Ray
@ 2022-10-19 4:17 ` Chang, Abner
0 siblings, 0 replies; 9+ messages in thread
From: Chang, Abner @ 2022-10-19 4:17 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]
[AMD Official Use Only - General]
Thanks! I will send out the V3 for the update. Will work on the spec publication with Mike.
Abner
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, October 19, 2022 11:34 AM
To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
Subject: RE: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
Oh I see!
Reviewed-by: Ray Ni <ray.ni@intrel.com<mailto:ray.ni@intrel.com>>
From: Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>
Sent: Tuesday, October 18, 2022 5:50 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: RE: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
[AMD Official Use Only - General]
Hi Ray,
<CpuArch>][<Vendor>]<FileName>.*
The simple <FileName> naming rule was already defined in the 4.3.1 to 4.3.4 sections. We are not intending for defining the file naming style. This section is mainly for the format of attaching <CpuArch> and <Vendor> to the filename.
For example, how do we add AMD to SmmCpuFeaturesLib? Is "Amd"SmmCpuFeaturesLib or SmmCpuFeaturesLib"Amd"? This is what we would like to define for the processor archs and vendors.
My opinion is we can just leave the file format naming as it was defined in this spec.
Or how about if we say "Refer to 4.3.1 to 4.3.4 sections for the file naming format" for <FileNmae> in below?
[<CpuArch>][<Vendor>]<FileName>.*
<CpuArch> OPTIONAL The <CpuArch> is represented with a BNF,
<arch> ::='Ia32' | 'X64' | 'Arm' | 'AArch64' | 'RiscV64' |
'LoongArch64' | 'Ebc'
<CpuArch> ::= <arch>[<arch>]*
Example: Ia32X64Arm or RiscV64LoongArch64
<Vendor> OPTIONAL *
<FileName> REQUIRED Refer to 4.3.1 to 4.3.4 sections for the file
Naming format.
Thanks
Abner
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, October 18, 2022 2:02 PM
To: Chang; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
All look good to me. Thanks for addressing my comments regarding simplifying the rules.
Except for one minor comment: I still don't think we need to define rules for source file names (4.3.5.4 EDK II source files within a Library/Module instance). And the rule "[][].*" doesn't specify what could be. That leads to allowing any style of the file name.
[-- Attachment #2: Type: text/html, Size: 11659 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-19 4:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-15 11:47 [tianocore-docs][PATCH V2 2/2] edk II C Coding Standard: Updates 4.2 and 4.3 sections Chang, Abner
2022-10-18 6:01 ` [edk2-devel] " Ni, Ray
2022-10-18 9:50 ` Chang, Abner
2022-10-19 1:31 ` Chang, Abner
2022-10-19 3:34 ` Ni, Ray
2022-10-19 4:17 ` Chang, Abner
2022-10-18 13:33 ` Sunil V L
2022-10-18 15:23 ` Chang, Abner
2022-10-19 2:24 ` Attar, AbdulLateef (Abdul Lateef)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox