Date   

[PATCH] Platform/SoftIron/Overdrive1000Board: remove PcdEnableKcs

kettenis@...
 

From: Mark Kettenis <kettenis@xs4all.nl>

The SoftIron Overdrive 1000 does not have a BMC so don't advertise
the IPMI KCS interface.

Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc | 2 --
1 file changed, 2 deletions(-)

diff --git a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc b/=
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
index 005ffad..5cf865d 100644
--- a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
+++ b/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
@@ -405,8 +405,6 @@ DEFINE NUM_CORES =3D 4
# map the stack as non-executable when entering the DXE phase=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE=0D
=0D
- gAmdStyxTokenSpaceGuid.PcdEnableKcs|TRUE=0D
-=0D
[PcdsPatchableInModule]=0D
# PCIe Configuration: x4x2x2 (=3D2 See Include/FDKGionb.h)=0D
gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2=0D
--=20
1.9.1


Re: [PATCH 1/1] Update mailing list links from lists.01.org to groups.io

Laszlo Ersek
 

On 05/04/20 13:48, Philippe Mathieu-Daudé wrote:
On 4/30/20 11:06 PM, Rebecca Cran wrote:
I only updated links to the top-level mailing list, not any links to
individual messages.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
  HardFeatureFreeze.md                                          | 2 +-
  ...unkempt-git-guide-for-edk2-contributors-and-maintainers.md | 2 +-
  Member-FAQ.md                                                 | 4 ++--
  SoftFeatureFreeze.md                                          | 2 +-
  4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/HardFeatureFreeze.md b/HardFeatureFreeze.md
index dae9f664b5d9..e0334f69ed7e 100644
--- a/HardFeatureFreeze.md
+++ b/HardFeatureFreeze.md
@@ -10,7 +10,7 @@
freeze](https://wiki.qemu.org/Planning/HardFeatureFreeze)).
  The proposed schedule for the active development cycle is shown in
the [EDK II
  Release Planning](EDK-II-Release-Planning) article. Shortly before
the hard
  feature freeze, an announcement email is sent to the
-[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel)
mailing list.
+[edk2-devel](https://edk2.groups.io/g/devel) mailing list.
  The announcement is posted by one of the maintainers that are listed in
 
[Maintainers.txt](https://github.com/tianocore/edk2/blob/master/Maintainers.txt),

  in section `EDK II Releases`.
diff --git
a/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
b/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
index 3469ef7b1845..6f85c782a710 100644
--- a/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
+++ b/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers.md
@@ -80,7 +80,7 @@ Contributor workflow
       git config notes.rewriteRef       refs/notes/commits
       git config sendemail.chainreplyto false
       git config sendemail.thread       true
-     git config sendemail.to           edk2-devel@lists.01.org
+     git config sendemail.to           devel@edk2.groups.io
       ```
    6.   <a name="contrib-06" href="#contrib-06">&sect;</a>
diff --git a/Member-FAQ.md b/Member-FAQ.md
index f3e5798f23ae..f277744ff6ce 100644
--- a/Member-FAQ.md
+++ b/Member-FAQ.md
@@ -7,6 +7,6 @@ TianoCore has accumulated a lot of information over
the years. We keep several F
  * [[Terms and Acronyms|Acronyms and Glossary]]
  * [[EDK II Documents]]
  -If you have a question and cannot find the answer, please try the
[EDK II developer e-mail
list](https://github.com/tianocore/tianocore.github.io/wiki/edk2-devel).
You can also [search the e-mail list
archive](https://lists.01.org/pipermail/edk2-devel/) for questions
already asked in email.
+If you have a question and cannot find the answer, please try the
[EDK II developer e-mail
list](https://github.com/tianocore/tianocore.github.io/wiki/edk2-devel).
You can also [search the e-mail list
archive](https://edk2.groups.io/g/devel) for questions already asked
in email.
  -_Note that e-mails prior to September 2015 are on a [sourceforge
archive](https://sourceforge.net/p/edk2/mailman/edk2-devel/)._
\ No newline at end of file
+_Note that e-mails prior to September 2015 are on a [sourceforge
archive](https://sourceforge.net/p/edk2/mailman/edk2-devel/)._
diff --git a/SoftFeatureFreeze.md b/SoftFeatureFreeze.md
index 9047a73c1d69..7a0f40bbeaf4 100644
--- a/SoftFeatureFreeze.md
+++ b/SoftFeatureFreeze.md
@@ -46,7 +46,7 @@
freeze](https://wiki.qemu.org/Planning/SoftFeatureFreeze).)
  The proposed schedule for the active development cycle is shown in
the [EDK II
  Release Planning](EDK-II-Release-Planning) article. Shortly before
the soft
  feature freeze, an announcement email is sent to the
-[edk2-devel](https://lists.01.org/mailman/listinfo/edk2-devel)
mailing list.
+[edk2-devel](https://edk2.groups.io/g/devel) mailing list.
  The announcement is posted by one of the maintainers that are listed in
 
[Maintainers.txt](https://github.com/tianocore/edk2/blob/master/Maintainers.txt),

  in section `EDK II Releases`.
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

edk2-wiki commit ae81cb24e5ba.

Thanks!
Laszlo


Re: [PATCH] Platform/SoftIron/Overdrive1000Board: remove PcdEnableKcs

Ard Biesheuvel
 

On 5/5/20 6:02 PM, kettenis@xs4all.nl wrote:
From: Mark Kettenis <kettenis@xs4all.nl>
The SoftIron Overdrive 1000 does not have a BMC so don't advertise
the IPMI KCS interface.
Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
Thanks Mark

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as 72d6a1e804cb..03fb86be4146

---
Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc | 2 --
1 file changed, 2 deletions(-)
diff --git a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc b/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
index 005ffad..5cf865d 100644
--- a/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
+++ b/Platform/SoftIron/Overdrive1000Board/Overdrive1000Board.dsc
@@ -405,8 +405,6 @@ DEFINE NUM_CORES = 4
# map the stack as non-executable when entering the DXE phase
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
- gAmdStyxTokenSpaceGuid.PcdEnableKcs|TRUE
-
[PcdsPatchableInModule]
# PCIe Configuration: x4x2x2 (=2 See Include/FDKGionb.h)
gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2


[PATCH v1 6/6] ShellPkg: acpiview: Make PPTT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Processor Topology Structure given should be parsed.

Enumerate all structures found in the Processor Properties Topology
Table (PPTT) on a per-type basis.

Consolidate all metadata about each Processor Topology Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for PPTT.

Count the number of instances of each Processor Topology Structure type.
Optionally report these counts after PPTT table parsing is finished.

Remove the definition of the DumpCacheTypeStructure and DumpIDStructure
functions. Their only purpose was to call ParseAcpi () and now this
process is streamlined.

Make DumpProcessorHierarchyNodeStructure function signature match that
of ACPI_STRUCT_PARSER_FUNC. This way, the function can be called from
ParseAcpiStruct ().

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.29

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make PPTT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 152 ++++++++++----------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h | 2 +-
2 files changed, 74 insertions(+), 80 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
index 0db272c16af0ad8824c8da4c88dd409c8550112a..b62f79b52cab989942f84b020e1d737e8ef65439 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
@@ -21,6 +21,11 @@ STATIC CONST UINT8* ProcessorTopologyStructureLength;
STATIC CONST UINT32* NumberOfPrivateResources;
STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;

+/**
+ Handler for each Processor Topology Structure
+**/
+STATIC ACPI_STRUCT_INFO PpttStructs[];
+
/**
This function validates the Cache Type Structure (Type 1) 'Number of sets'
field.
@@ -243,22 +248,34 @@ STATIC CONST ACPI_PARSER IdStructureParser[] = {
@param [in] Ptr Pointer to the start of the Processor Hierarchy Node
Structure data.
@param [in] Length Length of the Processor Hierarchy Node Structure.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used).
**/
STATIC
VOID
DumpProcessorHierarchyNodeStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Offset;
UINT32 Index;
CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
+ CHAR8 AsciiBuffer[80];
+
+ PrintAcpiStructName (
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Name,
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Count,
+ sizeof (AsciiBuffer),
+ AsciiBuffer
+ );

Offset = ParseAcpi (
TRUE,
2,
- "Processor Hierarchy Node Structure",
+ AsciiBuffer,
Ptr,
Length,
PARSER_PARAMS (ProcessorHierarchyNodeStructureParser)
@@ -269,7 +286,8 @@ DumpProcessorHierarchyNodeStructure (
if (NumberOfPrivateResources == NULL) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient Processor Hierarchy Node length. Length = %d.\n",
+ L"ERROR: Insufficient %a Structure length. Length = %d.\n",
+ PpttStructs[EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR].Name,
Length
);
return;
@@ -296,7 +314,7 @@ DumpProcessorHierarchyNodeStructure (
UnicodeSPrint (
Buffer,
sizeof (Buffer),
- L"Private resources [%d]",
+ L"Private resource [%d]",
Index
);

@@ -312,50 +330,37 @@ DumpProcessorHierarchyNodeStructure (
}

/**
- This function parses the Cache Type Structure (Type 1).
-
- @param [in] Ptr Pointer to the start of the Cache Type Structure data.
- @param [in] Length Length of the Cache Type Structure.
+ Information about each Processor Topology Structure type.
**/
-STATIC
-VOID
-DumpCacheTypeStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "Cache Type Structure",
- Ptr,
- Length,
- PARSER_PARAMS (CacheTypeStructureParser)
- );
-}
+STATIC ACPI_STRUCT_INFO PpttStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Processor",
+ EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpProcessorHierarchyNodeStructure
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Cache",
+ EFI_ACPI_6_3_PPTT_TYPE_CACHE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ CacheTypeStructureParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "ID",
+ EFI_ACPI_6_3_PPTT_TYPE_ID,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ IdStructureParser
+ )
+};

/**
- This function parses the ID Structure (Type 2).
-
- @param [in] Ptr Pointer to the start of the ID Structure data.
- @param [in] Length Length of the ID Structure.
+ PPTT structure database
**/
-STATIC
-VOID
-DumpIDStructure (
- IN UINT8* Ptr,
- IN UINT8 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
- "ID Structure",
- Ptr,
- Length,
- PARSER_PARAMS (IdStructureParser)
- );
-}
+STATIC ACPI_STRUCT_DATABASE PpttDatabase = {
+ "Processor Topology Structure",
+ PpttStructs,
+ ARRAY_SIZE (PpttStructs)
+};

/**
This function parses the ACPI PPTT table.
@@ -390,6 +395,8 @@ ParseAcpiPptt (
return;
}

+ ResetAcpiStructCounts (&PpttDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -419,7 +426,8 @@ ParseAcpiPptt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"processor topology structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ PpttDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -430,8 +438,9 @@ ParseAcpiPptt (
((Offset + (*ProcessorTopologyStructureLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Processor Topology Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ PpttDatabase.Name,
*ProcessorTopologyStructureLength,
Offset,
AcpiTableLength
@@ -439,39 +448,24 @@ ParseAcpiPptt (
return;
}

- PrintFieldName (2, L"* Structure Offset *");
- Print (L"0x%x\n", Offset);
-
- switch (*ProcessorTopologyStructureType) {
- case EFI_ACPI_6_2_PPTT_TYPE_PROCESSOR:
- DumpProcessorHierarchyNodeStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
- );
- break;
- case EFI_ACPI_6_2_PPTT_TYPE_CACHE:
- DumpCacheTypeStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
- );
- break;
- case EFI_ACPI_6_2_PPTT_TYPE_ID:
- DumpIDStructure (
- ProcessorTopologyStructurePtr,
- *ProcessorTopologyStructureLength
- );
- break;
- default:
- IncrementErrorCount ();
- Print (
- L"ERROR: Unknown processor topology structure:"
- L" Type = %d, Length = %d\n",
- *ProcessorTopologyStructureType,
- *ProcessorTopologyStructureLength
- );
- }
+ // Parse the Processor Topology Structure
+ ParseAcpiStruct (
+ 2,
+ ProcessorTopologyStructurePtr,
+ &PpttDatabase,
+ Offset,
+ *ProcessorTopologyStructureType,
+ *ProcessorTopologyStructureLength,
+ NULL,
+ NULL
+ );

ProcessorTopologyStructurePtr += *ProcessorTopologyStructureLength;
Offset += *ProcessorTopologyStructureLength;
} // while
+
+ // Report and validate processor topology structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&PpttDatabase);
+ }
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
index 2a671203fb0035bbc407ff4bb0ca9960706fa588..a0b0622ba00bda63379b670527145359b738bb7d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h
@@ -1,7 +1,7 @@
/** @file
Header file for PPTT parser

- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 5/6] ShellPkg: acpiview: Make IORT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each IORT
Node given should be parsed.

Enumerate all structures found in the IO Remapping Table (IORT) on a
per-type basis. Replace calls to AsciiSPrint () with the
PrintAcpiStructName () function.

Consolidate all metadata about each IORT Node. Define an array of
ACPI_STRUCT_INFO structures to store the name, instance count,
architecture support and handling information. Use this array to
construct the ACPI_STRUCT_DATABASE for IORT.

Count the number of instances of each IORT Node. Optionally report
these counts after IORT table parsing is finished.

Modify dedicated functions for parsing each IORT Node type such that
their signatures match ACPI_STRUCT_PARSER_FUNC. This way, they can be
used in the ParseAcpiStruct () call.

References:
- IO Remapping Table - Platform Design Document, Issue D, March 2018

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make IORT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 353 +++++++++++++-------
1 file changed, 234 insertions(+), 119 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 9a006a01448b897865cd7cd85651c816933acf05..0c40447b4363f10c7ea5c9eeb283cebeab24243a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -9,10 +9,12 @@
**/

#include <IndustryStandard/IoRemappingTable.h>
+#include <Library/DebugLib.h>
#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"

// Local variables
STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
@@ -32,6 +34,11 @@ STATIC CONST UINT32* PmuInterruptOffset;

STATIC CONST UINT32* ItsCount;

+/**
+ Handler for each IORT Node type
+**/
+STATIC ACPI_STRUCT_INFO IortStructs[];
+
/**
This function validates the ID Mapping array count for the ITS node.

@@ -273,12 +280,7 @@ DumpIortNodeIdMappings (

while ((Index < MappingCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "ID Mapping [%d]",
- Index
- );
+ PrintAcpiStructName ("ID Mapping", Index, sizeof (Buffer), Buffer);
Offset += ParseAcpi (
TRUE,
4,
@@ -296,27 +298,42 @@ DumpIortNodeIdMappings (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeSmmuV1V2 (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
UINT32 Index;
UINT32 Offset;
- CHAR8 Buffer[50]; // Used for AsciiName param of ParseAcpi
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Count,
+ sizeof (Buffer),
+ Buffer
+ );

ParseAcpi (
TRUE,
2,
- "SMMUv1 or SMMUv2 Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeSmmuV1V2Parser)
@@ -330,7 +347,8 @@ DumpIortNodeSmmuV1V2 (
(PmuInterruptOffset == NULL)) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient SMMUv1/2 node length. Length = %d\n",
+ L"ERROR: Insufficient %a Node length. Length = %d\n",
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv1v2].Name,
Length
);
return;
@@ -341,11 +359,11 @@ DumpIortNodeSmmuV1V2 (

while ((Index < *InterruptContextCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "Context Interrupts Array",
+ Index,
sizeof (Buffer),
- "Context Interrupts Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -363,11 +381,11 @@ DumpIortNodeSmmuV1V2 (

while ((Index < *PmuInterruptCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "PMU Interrupts Array",
+ Index,
sizeof (Buffer),
- "PMU Interrupts Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -392,23 +410,40 @@ DumpIortNodeSmmuV1V2 (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeSmmuV3 (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv3].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_SMMUv3].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "SMMUV3 Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeSmmuV3Parser)
@@ -424,24 +459,37 @@ DumpIortNodeSmmuV3 (
/**
This function parses the IORT ITS node.

+ ITS nodes have no ID mappings.
+
@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used)..
**/
STATIC
VOID
DumpIortNodeIts (
- IN UINT8* Ptr,
- IN UINT16 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Offset;
UINT32 Index;
CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi

+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
Offset = ParseAcpi (
TRUE,
2,
- "ITS Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeItsParser)
@@ -452,7 +500,8 @@ DumpIortNodeIts (
if (ItsCount == NULL) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient ITS group length. Length = %d.\n",
+ L"ERROR: Insufficient %a Node length. Length = %d.\n",
+ IortStructs[EFI_ACPI_IORT_TYPE_ITS_GROUP].Name,
Length
);
return;
@@ -462,11 +511,11 @@ DumpIortNodeIts (

while ((Index < *ItsCount) &&
(Offset < Length)) {
- AsciiSPrint (
- Buffer,
+ PrintAcpiStructName (
+ "GIC ITS Identifier Array",
+ Index,
sizeof (Buffer),
- "GIC ITS Identifier Array [%d]",
- Index
+ Buffer
);
Offset += ParseAcpi (
TRUE,
@@ -488,25 +537,41 @@ DumpIortNodeIts (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeNamedComponent (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
UINT32 Offset;
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_NAMED_COMP].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_NAMED_COMP].Count,
+ sizeof (Buffer),
+ Buffer
+ );

Offset = ParseAcpi (
TRUE,
2,
- "Named Component Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeNamedComponentParser)
@@ -534,23 +599,40 @@ DumpIortNodeNamedComponent (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodeRootComplex (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
)
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_ROOT_COMPLEX].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_ROOT_COMPLEX].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "Root Complex Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodeRootComplexParser)
@@ -568,23 +650,40 @@ DumpIortNodeRootComplex (

@param [in] Ptr Pointer to the start of the buffer.
@param [in] Length Length of the buffer.
- @param [in] MappingCount The ID Mapping count.
- @param [in] MappingOffset The offset of the ID Mapping array
+ @param [in] OptArg0 The ID Mapping count.
+ @param [in] OptArg1 The offset of the ID Mapping array
from the start of the IORT table.
**/
STATIC
VOID
DumpIortNodePmcg (
- IN UINT8* Ptr,
- IN UINT16 Length,
- IN UINT32 MappingCount,
- IN UINT32 MappingOffset
-)
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0,
+ IN CONST VOID* OptArg1
+ )
{
+ UINT32 MappingCount;
+ UINT32 MappingOffset;
+ CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
+
+ ASSERT (OptArg0 != NULL);
+ ASSERT (OptArg1 != NULL);
+
+ MappingCount = *((UINT32*)OptArg0);
+ MappingOffset = *((UINT32*)OptArg1);
+
+ PrintAcpiStructName (
+ IortStructs[EFI_ACPI_IORT_TYPE_PMCG].Name,
+ IortStructs[EFI_ACPI_IORT_TYPE_PMCG].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
ParseAcpi (
TRUE,
2,
- "PMCG Node",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (IortNodePmcgParser)
@@ -597,6 +696,57 @@ DumpIortNodePmcg (
);
}

+/**
+ Information about each IORT Node type
+**/
+STATIC ACPI_STRUCT_INFO IortStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "ITS Group",
+ EFI_ACPI_IORT_TYPE_ITS_GROUP,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeIts
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Named Component",
+ EFI_ACPI_IORT_TYPE_NAMED_COMP,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeNamedComponent
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "Root Complex",
+ EFI_ACPI_IORT_TYPE_ROOT_COMPLEX,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeRootComplex
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "SMMUv1 or SMMUv2",
+ EFI_ACPI_IORT_TYPE_SMMUv1v2,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeSmmuV1V2
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "SMMUv3",
+ EFI_ACPI_IORT_TYPE_SMMUv3,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodeSmmuV3
+ ),
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "PMCG",
+ EFI_ACPI_IORT_TYPE_PMCG,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpIortNodePmcg
+ )
+};
+
+/**
+ IORT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE IortDatabase = {
+ "IORT Node",
+ IortStructs,
+ ARRAY_SIZE (IortStructs)
+};
+
/**
This function parses the ACPI IORT table.
When trace is enabled this function parses the IORT table and traces the ACPI fields.
@@ -633,6 +783,8 @@ ParseAcpiIort (
return;
}

+ ResetAcpiStructCounts (&IortDatabase);
+
ParseAcpi (
TRUE,
0,
@@ -666,7 +818,7 @@ ParseAcpiIort (
ParseAcpi (
FALSE,
0,
- "IORT Node Header",
+ NULL,
NodePtr,
AcpiTableLength - Offset,
PARSER_PARAMS (IortNodeHeaderParser)
@@ -681,7 +833,8 @@ ParseAcpiIort (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"IORT node header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ IortDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -692,8 +845,9 @@ ParseAcpiIort (
((Offset + (*IortNodeLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid IORT Node length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ IortDatabase.Name,
*IortNodeLength,
Offset,
AcpiTableLength
@@ -701,63 +855,24 @@ ParseAcpiIort (
return;
}

- PrintFieldName (2, L"* Node Offset *");
- Print (L"0x%x\n", Offset);
-
- switch (*IortNodeType) {
- case EFI_ACPI_IORT_TYPE_ITS_GROUP:
- DumpIortNodeIts (
- NodePtr,
- *IortNodeLength
- );
- break;
- case EFI_ACPI_IORT_TYPE_NAMED_COMP:
- DumpIortNodeNamedComponent (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_ROOT_COMPLEX:
- DumpIortNodeRootComplex (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_SMMUv1v2:
- DumpIortNodeSmmuV1V2 (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_SMMUv3:
- DumpIortNodeSmmuV3 (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
- case EFI_ACPI_IORT_TYPE_PMCG:
- DumpIortNodePmcg (
- NodePtr,
- *IortNodeLength,
- *IortIdMappingCount,
- *IortIdMappingOffset
- );
- break;
-
- default:
- IncrementErrorCount ();
- Print (L"ERROR: Unsupported IORT Node type = %d\n", *IortNodeType);
- } // switch
+ // Parse the IORT Node
+ ParseAcpiStruct (
+ 2,
+ NodePtr,
+ &IortDatabase,
+ Offset,
+ *IortNodeType,
+ *IortNodeLength,
+ IortIdMappingCount,
+ IortIdMappingOffset
+ );

NodePtr += (*IortNodeLength);
Offset += (*IortNodeLength);
} // while
+
+ // Report and validate IORT Node counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&IortDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 4/6] ShellPkg: acpiview: Make GTDT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Platform Timer Structure given should be parsed.

Enumerate all structures found in the Generic Timer Description Table
(GTDT) on a per-type basis. Print the offset from the start of the table
for each structure.

Consolidate all metadata about each Platform Timer Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for GTDT.

Count the number of instances of each Platform Timer Structure type.
Optionally report these counts after GTDT table parsing is finished.

Modify DumpGTBlock () funtion signature so that it matches that of
ACPI_STRUCT_PARSER_FUNC. This way, the function can be used in the
ParseAcpiStruct () call.

Remove the definition of the DumpWatchdogTimer (). Its only purpose was
to call ParseAcpi () and now this process is streamlined.

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.24

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make GTDT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 123 ++++++++++++--------
1 file changed, 77 insertions(+), 46 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index bdd30ff45c61142c071ead63a27babab8998721b..9a9f8fda442081507768b1540f0b9b3c6c254329 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -9,13 +9,20 @@
**/

#include <IndustryStandard/Acpi.h>
+#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"

// "The number of GT Block Timers must be less than or equal to 8"
#define GT_BLOCK_TIMER_COUNT_MAX 8

+/**
+ Handler for each Platform Timer Structure type
+**/
+STATIC ACPI_STRUCT_INFO GtdtStructs[];
+
// Local variables
STATIC CONST UINT32* GtdtPlatformTimerCount;
STATIC CONST UINT32* GtdtPlatformTimerOffset;
@@ -167,23 +174,35 @@ STATIC CONST ACPI_PARSER SBSAGenericWatchdogParser[] = {
/**
This function parses the Platform GT Block.

- @param [in] Ptr Pointer to the start of the GT Block data.
- @param [in] Length Length of the GT Block structure.
+ @param [in] Ptr Pointer to the start of structure's buffer.
+ @param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument (Not used).
+ @param [in] OptArg1 Second optional argument (Not used).
**/
STATIC
VOID
DumpGTBlock (
- IN UINT8* Ptr,
- IN UINT16 Length
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
)
{
UINT32 Index;
UINT32 Offset;
+ CHAR8 Buffer[80];
+
+ PrintAcpiStructName (
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Count,
+ sizeof (Buffer),
+ Buffer
+ );

ParseAcpi (
TRUE,
2,
- "GT Block",
+ Buffer,
Ptr,
Length,
PARSER_PARAMS (GtBlockParser)
@@ -195,7 +214,8 @@ DumpGTBlock (
(GtBlockTimerOffset == NULL)) {
IncrementErrorCount ();
Print (
- L"ERROR: Insufficient GT Block Structure length. Length = %d.\n",
+ L"ERROR: Insufficient %a Structure length. Length = %d.\n",
+ GtdtStructs[EFI_ACPI_6_3_GTDT_GT_BLOCK].Name,
Length
);
return;
@@ -206,41 +226,47 @@ DumpGTBlock (

// Parse the specified number of GT Block Timer Structures or the GT Block
// Structure buffer length. Whichever is minimum.
- while ((Index++ < *GtBlockTimerCount) &&
+ while ((Index < *GtBlockTimerCount) &&
(Offset < Length)) {
+ PrintAcpiStructName ("GT Block Timer", Index, sizeof (Buffer), Buffer);
Offset += ParseAcpi (
TRUE,
- 2,
- "GT Block Timer",
+ 4,
+ Buffer,
Ptr + Offset,
Length - Offset,
PARSER_PARAMS (GtBlockTimerParser)
);
+ Index++;
}
}

/**
- This function parses the Platform Watchdog timer.
-
- @param [in] Ptr Pointer to the start of the watchdog timer data.
- @param [in] Length Length of the watchdog timer structure.
+ Information about each Platform Timer Structure type.
**/
-STATIC
-VOID
-DumpWatchdogTimer (
- IN UINT8* Ptr,
- IN UINT16 Length
- )
-{
- ParseAcpi (
- TRUE,
- 2,
+STATIC ACPI_STRUCT_INFO GtdtStructs[] = {
+ ADD_ACPI_STRUCT_INFO_FUNC (
+ "GT Block",
+ EFI_ACPI_6_3_GTDT_GT_BLOCK,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ DumpGTBlock
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
"SBSA Generic Watchdog",
- Ptr,
- Length,
- PARSER_PARAMS (SBSAGenericWatchdogParser)
- );
-}
+ EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SBSAGenericWatchdogParser
+ )
+};
+
+/**
+ GTDT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE GtdtDatabase = {
+ "Platform Timer Structure",
+ GtdtStructs,
+ ARRAY_SIZE (GtdtStructs)
+};

/**
This function parses the ACPI GTDT table.
@@ -275,6 +301,8 @@ ParseAcpiGtdt (
return;
}

+ ResetAcpiStructCounts (&GtdtDatabase);
+
ParseAcpi (
TRUE,
0,
@@ -321,7 +349,8 @@ ParseAcpiGtdt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Platform Timer Structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ GtdtDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -332,8 +361,9 @@ ParseAcpiGtdt (
((Offset + (*PlatformTimerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Platform Timer Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ GtdtDatabase.Name,
*PlatformTimerLength,
Offset,
AcpiTableLength
@@ -341,23 +371,24 @@ ParseAcpiGtdt (
return;
}

- switch (*PlatformTimerType) {
- case EFI_ACPI_6_3_GTDT_GT_BLOCK:
- DumpGTBlock (TimerPtr, *PlatformTimerLength);
- break;
- case EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG:
- DumpWatchdogTimer (TimerPtr, *PlatformTimerLength);
- break;
- default:
- IncrementErrorCount ();
- Print (
- L"ERROR: Invalid Platform Timer Type = %d\n",
- *PlatformTimerType
- );
- break;
- } // switch
+ // Parse the Platform Timer Structure
+ ParseAcpiStruct (
+ 2,
+ TimerPtr,
+ &GtdtDatabase,
+ Offset,
+ *PlatformTimerType,
+ *PlatformTimerLength,
+ NULL,
+ NULL
+ );

TimerPtr += *PlatformTimerLength;
Offset += *PlatformTimerLength;
} // while
+
+ // Report and validate Platform Timer Type Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&GtdtDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 2/6] ShellPkg: acpiview: Make MADT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Interrupt Controller Structure given should be parsed.

Enumerate all structures found in the Multiple APIC Description Table
(MADT) on a per-type basis. Print the offset from the start of the table
for each structure.

Consolidate all metadata about each Interrupt Controller Structure.
Define an array of ACPI_STRUCT_INFO structures to store the name,
instance count, architecture support and handling information. Use this
array to construct the ACPI_STRUCT_DATABASE for MADT.

Count the number of instances of each Interrupt Controller Structure
type. Optionally report these counts after MADT table parsing is
finished. Validate that Advanced Programmable Interrupt Controller
(APIC) structures are not present on Arm-based platforms.

For Arm-based platforms, make existing GIC Distributor (GICD) instance
count validation code use ACPI_STRUCT_INFO.

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.12

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make MADT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 217 ++++++++++++--------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 3 +-
2 files changed, 134 insertions(+), 86 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index f85d2b36532cfc5db36fe7bef9830cccc64969cc..00898a8853f45de1f813d71fe52920185bc92e2a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -15,6 +15,7 @@
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"
#include "MadtParser.h"

// Local Variables
@@ -200,6 +201,106 @@ STATIC CONST ACPI_PARSER MadtInterruptControllerHeaderParser[] = {
{L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL}
};

+/**
+ Information about each Interrupt Controller Structure type.
+**/
+STATIC ACPI_STRUCT_INFO MadtStructs[] = {
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Processor Local APIC",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "I/O APIC",
+ EFI_ACPI_6_3_IO_APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Interrupt Source Override",
+ EFI_ACPI_6_3_INTERRUPT_SOURCE_OVERRIDE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "NMI Source",
+ EFI_ACPI_6_3_NON_MASKABLE_INTERRUPT_SOURCE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local APIC NMI",
+ EFI_ACPI_6_3_LOCAL_APIC_NMI,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local APIC Address Override",
+ EFI_ACPI_6_3_LOCAL_APIC_ADDRESS_OVERRIDE,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "I/O SAPIC",
+ EFI_ACPI_6_3_IO_SAPIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local SAPIC",
+ EFI_ACPI_6_3_LOCAL_SAPIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Platform Interrupt Sources",
+ EFI_ACPI_6_3_PLATFORM_INTERRUPT_SOURCES,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Processor Local x2APIC",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED (
+ "Local x2APIC NMI",
+ EFI_ACPI_6_3_LOCAL_X2APIC_NMI,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICC",
+ EFI_ACPI_6_3_GIC,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicCParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICD",
+ EFI_ACPI_6_3_GICD,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicDParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC MSI Frame",
+ EFI_ACPI_6_3_GIC_MSI_FRAME,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicMSIFrameParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICR",
+ EFI_ACPI_6_3_GICR,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicRParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC ITS",
+ EFI_ACPI_6_3_GIC_ITS,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ GicITSParser
+ )
+};
+
+/**
+ MADT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE MadtDatabase = {
+ "Interrupt Controller Structure",
+ MadtStructs,
+ ARRAY_SIZE (MadtStructs)
+};
+
/**
This function parses the ACPI MADT table.
When trace is enabled this function parses the MADT table and
@@ -231,14 +332,13 @@ ParseAcpiMadt (
{
UINT32 Offset;
UINT8* InterruptContollerPtr;
- UINT32 GICDCount;
-
- GICDCount = 0;

if (!Trace) {
return;
}

+ ResetAcpiStructCounts (&MadtDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -267,7 +367,8 @@ ParseAcpiMadt (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Interrupt Controller Structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ MadtDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -278,8 +379,9 @@ ParseAcpiMadt (
((Offset + (*MadtInterruptControllerLength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Interrupt Controller Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ "AcpiTableLength = %d.\n",
+ MadtDatabase.Name,
*MadtInterruptControllerLength,
Offset,
AcpiTableLength
@@ -287,87 +389,32 @@ ParseAcpiMadt (
return;
}

- switch (*MadtInterruptControllerType) {
- case EFI_ACPI_6_3_GIC: {
- ParseAcpi (
- TRUE,
- 2,
- "GICC",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicCParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GICD: {
- if (++GICDCount > 1) {
- IncrementErrorCount ();
- Print (
- L"ERROR: Only one GICD must be present,"
- L" GICDCount = %d\n",
- GICDCount
- );
- }
- ParseAcpi (
- TRUE,
- 2,
- "GICD",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicDParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GIC_MSI_FRAME: {
- ParseAcpi (
- TRUE,
- 2,
- "GIC MSI Frame",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicMSIFrameParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GICR: {
- ParseAcpi (
- TRUE,
- 2,
- "GICR",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicRParser)
- );
- break;
- }
-
- case EFI_ACPI_6_3_GIC_ITS: {
- ParseAcpi (
- TRUE,
- 2,
- "GIC ITS",
- InterruptContollerPtr,
- *MadtInterruptControllerLength,
- PARSER_PARAMS (GicITSParser)
- );
- break;
- }
-
- default: {
- IncrementErrorCount ();
- Print (
- L"ERROR: Unknown Interrupt Controller Structure,"
- L" Type = %d, Length = %d\n",
- *MadtInterruptControllerType,
- *MadtInterruptControllerLength
- );
- }
- } // switch
+ // Parse the Interrupt Controller Structure
+ ParseAcpiStruct (
+ 2,
+ InterruptContollerPtr,
+ &MadtDatabase,
+ Offset,
+ *MadtInterruptControllerType,
+ *MadtInterruptControllerLength,
+ NULL,
+ NULL
+ );

InterruptContollerPtr += *MadtInterruptControllerLength;
Offset += *MadtInterruptControllerLength;
} // while
+
+ // Report and validate Interrupt Controller Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&MadtDatabase);
+
+ if (MadtStructs[EFI_ACPI_6_3_GICD].Count > 1) {
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: Only one %a must be present\n",
+ MadtStructs[EFI_ACPI_6_3_GICD].Name
+ );
+ }
+ }
}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
index fbbc43e09adbdf9fea302a03a61e6dc179f06a62..25128081816459106e43ef5c98acd23dc1f910c3 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h
@@ -1,13 +1,14 @@
/** @file
Header file for MADT table parser

- Copyright (c) 2019, ARM Limited. All rights reserved.
+ Copyright (c) 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent

@par Reference(s):
- Arm Generic Interrupt Controller Architecture Specification,
GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0
+ - ACPI 6.3 Specification - January 2019, Section 5.2.12
**/

#ifndef MADT_PARSER_H_
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 0/6] ShellPkg: acpiview: Refactor ACPI table parsing loops

Krzysztof Koch
 

This patch series modifies existing ACPI table parsers. Now, structure
type values are used as indexes to access a centralized database
containing information on how to parse each structure in the table.

Replacing a 'switch' statements with arrays indexed by the Type value
allows consolidation of metadata about buiding blocks of an ACPI table.
The additional data stored about each structure includes:
- ACPI-defined name
- instance count
- compatible architectures (x86, AArch64, etc...)
- information on how to parse the structure

The new metadata allows more extensive ACPI table contents validation in
acpiview, which is implemented in this patch series.

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/616_refactor_acpiview_parser_loops_v1

Krzysztof Koch (6):
ShellPkg: acpiview: Add interface for data-driven table parsing
ShellPkg: acpiview: Make MADT parsing logic data driven
ShellPkg: acpiview: Make SRAT parsing logic data driven
ShellPkg: acpiview: Make GTDT parsing logic data driven
ShellPkg: acpiview: Make IORT parsing logic data driven
ShellPkg: acpiview: Make PPTT parsing logic data driven

ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 263 +++++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 234 +++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 123 ++++---
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 353 +++++++++++++-------
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 217 +++++++-----
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 3 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 152 ++++-----
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.h | 2 +-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 204 +++++------
9 files changed, 1093 insertions(+), 458 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 3/6] ShellPkg: acpiview: Make SRAT parsing logic data driven

Krzysztof Koch
 

Replace the switch statement in the main parser loop with a table-driven
approach. Use the ParseAcpiStruct () method to resolve how each
Static Resource Allocation Structure given should be parsed.

Print the offset of each Static Resource Allocation Structure from the
start of the table.

Consolidate all metadata about each Static Resource Allocation
Structure. Define an array of ACPI_STRUCT_INFO structures to store the
name, instance count, architecture support and handling information.
Use this array to construct the ACPI_STRUCT_DATABASE for the System
Resource Affinity Table (SRAT).

Count the number of instances of each Static Resource Allocation
Structure type. Optionally report these counts after SRAT table parsing
is finished and validate that Advanced Programmable Interrupt Controller
(APIC) structures are not present on Arm-based platforms.

References:
- ACPI 6.3 Specification - January 2019, Section 5.2.16

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Make SRAT parsing logic data driven [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 204 ++++++++------------
1 file changed, 77 insertions(+), 127 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
index 6f66be68cc0bed14811a0432c61a79fd47c54890..8ec233a52861b039979bb8291b3c90193acd86fa 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -13,6 +13,7 @@
#include <Library/UefiLib.h>
#include "AcpiParser.h"
#include "AcpiTableParser.h"
+#include "AcpiView.h"

// Local Variables
STATIC CONST UINT8* SratRAType;
@@ -330,6 +331,57 @@ STATIC CONST ACPI_PARSER SratX2ApciAffinityParser[] = {
{L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL}
};

+/**
+ Information about each Static Resource Allocation Structure type.
+**/
+STATIC ACPI_STRUCT_INFO SratStructs[] = {
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Processor Local APIC/SAPIC Affinity",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64,
+ SratApciSapicAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Memory Affinity",
+ EFI_ACPI_6_3_MEMORY_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SratMemAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Processor Local x2APIC Affinity",
+ EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64,
+ SratX2ApciAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GICC Affinity Structure",
+ EFI_ACPI_6_3_GICC_AFFINITY,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SratGicCAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "GIC ITS Affinity",
+ EFI_ACPI_6_3_GIC_ITS_AFFINITY,
+ ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64,
+ SratGicITSAffinityParser
+ ),
+ ADD_ACPI_STRUCT_INFO_ARRAY (
+ "Generic Initiator Affinity",
+ EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY,
+ ARCH_COMPAT_IA32 | ARCH_COMPAT_X64 | ARCH_COMPAT_ARM |ARCH_COMPAT_AARCH64,
+ SratGenericInitiatorAffinityParser
+ )
+};
+
+/**
+ SRAT structure database
+**/
+STATIC ACPI_STRUCT_DATABASE SratDatabase = {
+ "Static Resource Allocation Structure",
+ SratStructs,
+ ARRAY_SIZE (SratStructs)
+};
+
/**
This function parses the ACPI SRAT table.
When trace is enabled this function parses the SRAT table and
@@ -357,27 +409,15 @@ ParseAcpiSrat (
IN UINT8 AcpiTableRevision
)
{
- UINT32 Offset;
- UINT8* ResourcePtr;
- UINT32 GicCAffinityIndex;
- UINT32 GicITSAffinityIndex;
- UINT32 GenericInitiatorAffinityIndex;
- UINT32 MemoryAffinityIndex;
- UINT32 ApicSapicAffinityIndex;
- UINT32 X2ApicAffinityIndex;
- CHAR8 Buffer[80]; // Used for AsciiName param of ParseAcpi
-
- GicCAffinityIndex = 0;
- GicITSAffinityIndex = 0;
- GenericInitiatorAffinityIndex = 0;
- MemoryAffinityIndex = 0;
- ApicSapicAffinityIndex = 0;
- X2ApicAffinityIndex = 0;
+ UINT32 Offset;
+ UINT8* ResourcePtr;

if (!Trace) {
return;
}

+ ResetAcpiStructCounts (&SratDatabase);
+
Offset = ParseAcpi (
TRUE,
0,
@@ -406,7 +446,8 @@ ParseAcpiSrat (
IncrementErrorCount ();
Print (
L"ERROR: Insufficient remaining table buffer length to read the " \
- L"Static Resource Allocation structure header. Length = %d.\n",
+ L"%a header. Length = %d.\n",
+ SratDatabase.Name,
AcpiTableLength - Offset
);
return;
@@ -417,8 +458,9 @@ ParseAcpiSrat (
((Offset + (*SratRALength)) > AcpiTableLength)) {
IncrementErrorCount ();
Print (
- L"ERROR: Invalid Static Resource Allocation Structure length. " \
- L"Length = %d. Offset = %d. AcpiTableLength = %d.\n",
+ L"ERROR: Invalid %a length. Length = %d. Offset = %d. " \
+ L"AcpiTableLength = %d.\n",
+ SratDatabase.Name,
*SratRALength,
Offset,
AcpiTableLength
@@ -426,116 +468,24 @@ ParseAcpiSrat (
return;
}

- switch (*SratRAType) {
- case EFI_ACPI_6_3_GICC_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "GICC Affinity Structure [%d]",
- GicCAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratGicCAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_GIC_ITS_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "GIC ITS Affinity Structure [%d]",
- GicITSAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratGicITSAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_GENERIC_INITIATOR_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "Generic Initiator Affinity Structure [%d]",
- GenericInitiatorAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratGenericInitiatorAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_MEMORY_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "Memory Affinity Structure [%d]",
- MemoryAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratMemAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_PROCESSOR_LOCAL_APIC_SAPIC_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "APIC/SAPIC Affinity Structure [%d]",
- ApicSapicAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratApciSapicAffinityParser)
- );
- break;
-
- case EFI_ACPI_6_3_PROCESSOR_LOCAL_X2APIC_AFFINITY:
- AsciiSPrint (
- Buffer,
- sizeof (Buffer),
- "X2APIC Affinity Structure [%d]",
- X2ApicAffinityIndex++
- );
- ParseAcpi (
- TRUE,
- 2,
- Buffer,
- ResourcePtr,
- *SratRALength,
- PARSER_PARAMS (SratX2ApciAffinityParser)
- );
- break;
-
- default:
- IncrementErrorCount ();
- Print (L"ERROR: Unknown SRAT Affinity type = 0x%x\n", *SratRAType);
- break;
- }
+ // Parse the Static Resource Allocation Structure
+ ParseAcpiStruct (
+ 2,
+ ResourcePtr,
+ &SratDatabase,
+ Offset,
+ *SratRAType,
+ *SratRALength,
+ NULL,
+ NULL
+ );

ResourcePtr += (*SratRALength);
Offset += (*SratRALength);
}
+
+ // Report and validate Static Resource Allocation Structure counts
+ if (GetConsistencyChecking ()) {
+ ValidateAcpiStructCounts (&SratDatabase);
+ }
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 1/6] ShellPkg: acpiview: Add interface for data-driven table parsing

Krzysztof Koch
 

Define and implement an interface to streamline metadata collection and
validation for structures present in each ACPI table.

Most ACPI tables define substructures which constitute the table.
These substructures are identified by their 'Type' field value. The
range of possible 'Type' values is defined on a per-table basis.

For more sophisticated ACPI table validation, additional data about
each structure type needs to be maintained. This patch defines a new
ACPI_STRUCT_INFO structure. It stores additional metadata about a
building block of an ACPI table. ACPI_STRUCT_INFO's are organised into
ACPI_STRUCT_DATABASE's. ACPI_STRUCT_DATABASE is an array of
ACPI_STRUCT_INFO elements which are indexed using structure's type
value.

For example, in the Multiple APIC Description Table (MADT) all
Interrupt Controller Structure types form a single database. In the
database, the GIC CPU Interface (GICC) structure's metadata is the 11th
entry (i.e. Type = 0xB).

ACPI_STRUCT_INFO structure consists of:
- ASCII name of the structure
- ACPI-defined stucture Type
- bitmask defining the validity of the structure for various
architectures
- instance counter
- a handler for the structure (ACPI_STRUCT_HANDLER)

The bitmask allows detection of structures in a table which
are not compatible with the target platform.

For example, the Multiple APIC Description Table (MADT) contains
Interrupt Controller Structure definitions which apply to either the
Advanced Programmable Interrupt Controller (APIC) model or the Generic
Interrupt Controller (GIC) model. Presence of APIC-related structures
on an Arm-based platform is a bug which is now detected and reported
by acpiview.

This patch adds support for compatibility checks with the Arm
architecture only. However, provisions are made to allow extensions to
other architectures.

ACPI_STRUCT_HANDLER describes how the contents of the structure can
be parsed. The possible options are:
- An ACPI_PARSER array which can be passed to the ParseAcpi() function
- A dedicated function for parsing the structure
(ACPI_STRUCT_PARSER_FUNC)

If neither of these options is provided, it is assumed that the parsing
logic is not implemented.

ACPI_STRUCT_PARSER_FUNC expects the the first two arguments to be the
pointer to the start of the structure to parse and the length of
structure's buffer. The remaining two optional arguments are context
specific.

This patch adds methods for:
- Resetting the instance count for all structure types in a table.
- Getting the combined instance count for all types in a table.
- Validating the compatibility of a structure with the target arch.
- Printing structure counts for the types which are compatible with
the target architecture and validating that the non-compatible
structures are not present in the table.
- Parsing the structure according to the information provided by its
handle.

Finally, define a helper PrintAcpiStructName () function to streamline
the printing of ACPI structure name together with the structure's
current occurrence count.

References:
- ACPI 6.3, January 2019

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
v1:
- Add interface for data-driven table parsing [Krzysztof]

ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 263 ++++++++++++++++++++
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 234 +++++++++++++++++
2 files changed, 497 insertions(+)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 3f12a33050a4e4ab3be2187c90ef8dcf0882283d..32566101e2de2eec3ccf44563ee79379404bff62 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -6,6 +6,8 @@
**/

#include <Uefi.h>
+#include <Library/DebugLib.h>
+#include <Library/PrintLib.h>
#include <Library/UefiLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include "AcpiParser.h"
@@ -466,6 +468,267 @@ PrintFieldName (
);
}

+/**
+ Produce a Null-terminated ASCII string with the name and index of an
+ ACPI structure.
+
+ The output string is in the following format: <Name> [<Index>]
+
+ @param [in] Name Structure name.
+ @param [in] Index Structure index.
+ @param [in] BufferSize The size, in bytes, of the output buffer.
+ @param [out] Buffer Buffer for the output string.
+
+ @return The number of bytes written to the buffer (not including Null-byte)
+**/
+UINTN
+EFIAPI
+PrintAcpiStructName (
+ IN CONST CHAR8* Name,
+ IN UINT32 Index,
+ IN UINTN BufferSize,
+ OUT CHAR8* Buffer
+ )
+{
+ ASSERT (Name != NULL);
+ ASSERT (Buffer != NULL);
+
+ return AsciiSPrint (Buffer, BufferSize, "%a [%d]", Name , Index);
+}
+
+/**
+ Set all ACPI structure instance counts to 0.
+
+ @param [in,out] StructDb ACPI structure database with counts to reset.
+**/
+VOID
+EFIAPI
+ResetAcpiStructCounts (
+ IN OUT ACPI_STRUCT_DATABASE* StructDb
+ )
+{
+ UINT32 Type;
+
+ ASSERT (StructDb != NULL);
+ ASSERT (StructDb->Entries != NULL);
+
+ for (Type = 0; Type < StructDb->EntryCount; Type++) {
+ StructDb->Entries[Type].Count = 0;
+ }
+}
+
+/**
+ Sum all ACPI structure instance counts.
+
+ @param [in] StructDb ACPI structure database with per-type counts to sum.
+
+ @return Total number of structure instances recorded in the database.
+**/
+UINT32
+EFIAPI
+SumAcpiStructCounts (
+ IN CONST ACPI_STRUCT_DATABASE* StructDb
+ )
+{
+ UINT32 Type;
+ UINT32 Total;
+
+ ASSERT (StructDb != NULL);
+ ASSERT (StructDb->Entries != NULL);
+
+ Total = 0;
+
+ for (Type = 0; Type < StructDb->EntryCount; Type++) {
+ Total += StructDb->Entries[Type].Count;
+ }
+
+ return Total;
+}
+
+/**
+ Validate that a structure with a given type value is defined for the given
+ ACPI table and target architecture.
+
+ The target architecture is evaluated from the firmare build parameters.
+
+ @param [in] Type ACPI-defined structure type.
+ @param [in] StructDb ACPI structure database with architecture
+ compatibility info.
+
+ @retval TRUE Structure is valid.
+ @retval FALSE Structure is not valid.
+**/
+BOOLEAN
+EFIAPI
+IsAcpiStructTypeValid (
+ IN UINT32 Type,
+ IN CONST ACPI_STRUCT_DATABASE* StructDb
+ )
+{
+ UINT32 Compatible;
+
+ ASSERT (StructDb != NULL);
+ ASSERT (StructDb->Entries != NULL);
+
+ if (Type >= StructDb->EntryCount) {
+ return FALSE;
+ }
+
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+ Compatible = StructDb->Entries[Type].CompatArch &
+ (ARCH_COMPAT_ARM | ARCH_COMPAT_AARCH64);
+#else
+ Compatible = StructDb->Entries[Type].CompatArch;
+#endif
+
+ return (Compatible != 0);
+}
+
+/**
+ Print the instance count of each structure in an ACPI table that is
+ compatible with the target architecture.
+
+ For structures which are not allowed for the target architecture,
+ validate that their instance counts are 0.
+
+ @param [in] StructDb ACPI structure database with counts to validate.
+
+ @retval TRUE All structures are compatible.
+ @retval FALSE One or more incompatible structures present.
+**/
+BOOLEAN
+EFIAPI
+ValidateAcpiStructCounts (
+ IN CONST ACPI_STRUCT_DATABASE* StructDb
+ )
+{
+ BOOLEAN AllValid;
+ UINT32 Type;
+
+ ASSERT (StructDb != NULL);
+ ASSERT (StructDb->Entries != NULL);
+
+ AllValid = TRUE;
+ Print (L"\nTable Breakdown:\n");
+
+ for (Type = 0; Type < StructDb->EntryCount; Type++) {
+ ASSERT (Type == StructDb->Entries[Type].Type);
+
+ if (IsAcpiStructTypeValid (Type, StructDb)) {
+ Print (
+ L"%*a%-*a : %d\n",
+ INSTANCE_COUNT_INDENT,
+ "",
+ OUTPUT_FIELD_COLUMN_WIDTH - INSTANCE_COUNT_INDENT,
+ StructDb->Entries[Type].Name,
+ StructDb->Entries[Type].Count
+ );
+ } else if (StructDb->Entries[Type].Count > 0) {
+ AllValid = FALSE;
+ IncrementErrorCount ();
+ Print (
+ L"ERROR: %a Structure is not valid for the target architecture " \
+ L"(found %d)\n",
+ StructDb->Entries[Type].Name,
+ StructDb->Entries[Type].Count
+ );
+ }
+ }
+
+ return AllValid;
+}
+
+/**
+ Parse the ACPI structure with the type value given according to instructions
+ defined in the ACPI structure database.
+
+ If the input structure type is defined in the database, increment structure's
+ instance count.
+
+ If ACPI_PARSER array is used to parse the input structure, the index of the
+ structure (instance count for the type before update) gets printed alongside
+ the structure name. This helps debugging if there are many instances of the
+ type in a table. For ACPI_STRUCT_PARSER_FUNC, the printing of the index must
+ be implemented separately.
+
+ @param [in] Indent Number of spaces to indent the output.
+ @param [in] Ptr Ptr to the start of the structure.
+ @param [in,out] StructDb ACPI structure database with instructions on how
+ parse every structure type.
+ @param [in] Offset Structure offset from the start of the table.
+ @param [in] Type ACPI-defined structure type.
+ @param [in] Length Length of the structure in bytes.
+ @param [in] OptArg0 First optional argument to pass to parser function.
+ @param [in] OptArg1 Second optional argument to pass to parser function.
+
+ @retval TRUE ACPI structure parsed successfully.
+ @retval FALSE Undefined structure type or insufficient data to parse.
+**/
+BOOLEAN
+EFIAPI
+ParseAcpiStruct (
+ IN UINT32 Indent,
+ IN UINT8* Ptr,
+ IN OUT ACPI_STRUCT_DATABASE* StructDb,
+ IN UINT32 Offset,
+ IN UINT32 Type,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
+ )
+{
+ ACPI_STRUCT_PARSER_FUNC ParserFunc;
+ CHAR8 Buffer[80];
+
+ ASSERT (Ptr != NULL);
+ ASSERT (StructDb != NULL);
+ ASSERT (StructDb->Entries != NULL);
+ ASSERT (StructDb->Name != NULL);
+
+ PrintFieldName (Indent, L"* Offset *");
+ Print (L"0x%x\n", Offset);
+
+ if (Type >= StructDb->EntryCount) {
+ IncrementErrorCount ();
+ Print (L"ERROR: Unknown %a. Type = %d\n", StructDb->Name, Type);
+ return FALSE;
+ }
+
+ if (StructDb->Entries[Type].Handler.ParserFunc != NULL) {
+ ParserFunc = StructDb->Entries[Type].Handler.ParserFunc;
+ ParserFunc (Ptr, Length, OptArg0, OptArg1);
+ } else if (StructDb->Entries[Type].Handler.ParserArray != NULL) {
+ ASSERT (StructDb->Entries[Type].Handler.Elements != 0);
+
+ PrintAcpiStructName (
+ StructDb->Entries[Type].Name,
+ StructDb->Entries[Type].Count,
+ sizeof (Buffer),
+ Buffer
+ );
+
+ ParseAcpi (
+ TRUE,
+ Indent,
+ Buffer,
+ Ptr,
+ Length,
+ StructDb->Entries[Type].Handler.ParserArray,
+ StructDb->Entries[Type].Handler.Elements
+ );
+ } else {
+ StructDb->Entries[Type].Count++;
+ Print (
+ L"ERROR: Parsing of %a Structure is not implemented\n",
+ StructDb->Entries[Type].Name
+ );
+ return FALSE;
+ }
+
+ StructDb->Entries[Type].Count++;
+ return TRUE;
+}
+
/**
This function is used to parse an ACPI table buffer.

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index f81ccac7e118378aa185db4b625e5bcd75f78347..70e540b3a76de0ff9ce70bcabed8548063bea0ff 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -300,6 +300,240 @@ typedef struct AcpiParser {
VOID* Context;
} ACPI_PARSER;

+/**
+ Produce a Null-terminated ASCII string with the name and index of an
+ ACPI structure.
+
+ The output string is in the following format: <Name> [<Index>]
+
+ @param [in] Name Structure name.
+ @param [in] Index Structure index.
+ @param [in] BufferSize The size, in bytes, of the output buffer.
+ @param [out] Buffer Buffer for the output string.
+
+ @return The number of bytes written to the buffer (not including Null-byte)
+**/
+UINTN
+EFIAPI
+PrintAcpiStructName (
+ IN CONST CHAR8* Name,
+ IN UINT32 Index,
+ IN UINTN BufferSize,
+ OUT CHAR8* Buffer
+ );
+
+/**
+ Indentation for printing instance counts for structures in an ACPI table.
+**/
+#define INSTANCE_COUNT_INDENT 2
+
+/**
+ Common signature for functions which parse ACPI structures.
+
+ @param [in] Ptr Pointer to the start of structure's buffer.
+ @param [in] Length Length of the buffer.
+ @param [in] OptArg0 First optional argument.
+ @param [in] OptArg1 Second optional argument.
+*/
+typedef VOID (*ACPI_STRUCT_PARSER_FUNC) (
+ IN UINT8* Ptr,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
+ );
+
+/**
+ Description of how an ACPI structure should be parsed.
+
+ One of ParserFunc or ParserArray should be not NULL. Otherwise, it is
+ assumed that parsing of an ACPI structure is not supported. If both
+ ParserFunc and ParserArray are defined, ParserFunc is used.
+**/
+typedef struct AcpiStructHandler {
+ /// Dedicated function for parsing an ACPI structure
+ ACPI_STRUCT_PARSER_FUNC ParserFunc;
+ /// Array of instructions on how each structure field should be parsed
+ CONST ACPI_PARSER* ParserArray;
+ /// Number of elements in ParserArray if ParserArray is defined
+ UINT32 Elements;
+} ACPI_STRUCT_HANDLER;
+
+/**
+ ACPI structure compatiblity with various architectures.
+
+ Some ACPI tables define structures which are, for example, only valid in
+ the X64 or Arm context. For instance, the Multiple APIC Description Table
+ (MADT) describes both APIC and GIC interrupt models.
+
+ These definitions provide means to describe the belonging of a structure
+ in an ACPI table to a particular architecture. This way, incompatible
+ structures can be detected.
+**/
+#define ARCH_COMPAT_IA32 BIT0
+#define ARCH_COMPAT_X64 BIT1
+#define ARCH_COMPAT_ARM BIT2
+#define ARCH_COMPAT_AARCH64 BIT3
+#define ARCH_COMPAT_RISCV64 BIT4
+
+/**
+ Information about a structure which constitutes an ACPI table
+**/
+typedef struct AcpiStructInfo {
+ /// ACPI-defined structure Name
+ CONST CHAR8* Name;
+ /// ACPI-defined structure Type
+ CONST UINT32 Type;
+ /// Architecture(s) for which this structure is valid
+ CONST UINT32 CompatArch;
+ /// Structure's instance count in a table
+ UINT32 Count;
+ /// Information on how to handle the structure
+ CONST ACPI_STRUCT_HANDLER Handler;
+} ACPI_STRUCT_INFO;
+
+/**
+ Macro for defining ACPI structure info when an ACPI_PARSER array must
+ be used to parse the structure.
+**/
+#define ADD_ACPI_STRUCT_INFO_ARRAY(Name, Type, Compat, Array) \
+{ \
+ Name, Type, Compat, 0, {NULL, Array, ARRAY_SIZE (Array)} \
+}
+
+/**
+ Macro for defining ACPI structure info when an ACPI_STRUCT_PARSER_FUNC
+ must be used to parse the structure.
+**/
+#define ADD_ACPI_STRUCT_INFO_FUNC(Name, Type, Compat, Func) \
+{ \
+ Name, Type, Compat, 0, {Func, NULL, 0} \
+}
+
+/**
+ Macro for defining ACPI structure info when the structure is defined in
+ the ACPI spec but no parsing information is provided.
+**/
+#define ACPI_STRUCT_INFO_PARSER_NOT_IMPLEMENTED(Name, Type, Compat) \
+{ \
+ Name, Type, Compat, 0, {NULL, NULL, 0} \
+}
+
+/**
+ Database collating information about every structure type defined by
+ an ACPI table.
+**/
+typedef struct AcpiStructDatabase {
+ /// ACPI-defined name for the structures being described in the database
+ CONST CHAR8* Name;
+ /// Per-structure-type information. The list must be ordered by the types
+ /// defined for the table. All entries must be unique and there should be
+ /// no gaps.
+ ACPI_STRUCT_INFO* Entries;
+ /// Total number of unique types defined for the table
+ CONST UINT32 EntryCount;
+} ACPI_STRUCT_DATABASE;
+
+/**
+ Set all ACPI structure instance counts to 0.
+
+ @param [in,out] StructDb ACPI structure database with counts to reset.
+**/
+VOID
+EFIAPI
+ResetAcpiStructCounts (
+ IN OUT ACPI_STRUCT_DATABASE* StructDb
+ );
+
+/**
+ Sum all ACPI structure instance counts.
+
+ @param [in] StructDb ACPI structure database with per-type counts to sum.
+
+ @return Total number of structure instances recorded in the database.
+**/
+UINT32
+EFIAPI
+SumAcpiStructCounts (
+ IN CONST ACPI_STRUCT_DATABASE* StructDb
+ );
+
+/**
+ Validate that a structure with a given type value is defined for the given
+ ACPI table and target architecture.
+
+ The target architecture is evaluated from the firmare build parameters.
+
+ @param [in] Type ACPI-defined structure type.
+ @param [in] StructDb ACPI structure database with architecture
+ compatibility info.
+
+ @retval TRUE Structure is valid.
+ @retval FALSE Structure is not valid.
+**/
+BOOLEAN
+EFIAPI
+IsAcpiStructTypeValid (
+ IN UINT32 Type,
+ IN CONST ACPI_STRUCT_DATABASE* StructDb
+ );
+
+/**
+ Print the instance count of each structure in an ACPI table that is
+ compatible with the target architecture.
+
+ For structures which are not allowed for the target architecture,
+ validate that their instance counts are 0.
+
+ @param [in] StructDb ACPI structure database with counts to validate.
+
+ @retval TRUE All structures are compatible.
+ @retval FALSE One or more incompatible structures present.
+**/
+BOOLEAN
+EFIAPI
+ValidateAcpiStructCounts (
+ IN CONST ACPI_STRUCT_DATABASE* StructDb
+ );
+
+/**
+ Parse the ACPI structure with the type value given according to instructions
+ defined in the ACPI structure database.
+
+ If the input structure type is defined in the database, increment structure's
+ instance count.
+
+ If ACPI_PARSER array is used to parse the input structure, the index of the
+ structure (instance count for the type before update) gets printed alongside
+ the structure name. This helps debugging if there are many instances of the
+ type in a table. For ACPI_STRUCT_PARSER_FUNC, the printing of the index must
+ be implemented separately.
+
+ @param [in] Indent Number of spaces to indent the output.
+ @param [in] Ptr Ptr to the start of the structure.
+ @param [in,out] StructDb ACPI structure database with instructions on how
+ parse every structure type.
+ @param [in] Offset Structure offset from the start of the table.
+ @param [in] Type ACPI-defined structure type.
+ @param [in] Length Length of the structure in bytes.
+ @param [in] OptArg0 First optional argument to pass to parser function.
+ @param [in] OptArg1 Second optional argument to pass to parser function.
+
+ @retval TRUE ACPI structure parsed successfully.
+ @retval FALSE Undefined structure type or insufficient data to parse.
+**/
+BOOLEAN
+EFIAPI
+ParseAcpiStruct (
+ IN UINT32 Indent,
+ IN UINT8* Ptr,
+ IN OUT ACPI_STRUCT_DATABASE* StructDb,
+ IN UINT32 Offset,
+ IN UINT32 Type,
+ IN UINT32 Length,
+ IN CONST VOID* OptArg0 OPTIONAL,
+ IN CONST VOID* OptArg1 OPTIONAL
+ );
+
/**
A structure used to store the pointers to the members of the
ACPI description header structure that was parsed.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


Re: [PATCH 0/5] EmbeddedPkg spring cleaning

Ard Biesheuvel
 

On 5/5/20 4:08 PM, Leif Lindholm wrote:
On Mon, May 04, 2020 at 15:57:22 +0200, Ard Biesheuvel wrote:
Delete a couple of obsole driver from EmbeddedPkg. They have been moved
into edk2-platforms alongside the small set of development platforms
that still use them, but they are no long fit for use beyond that.

Diffs have been omitted for files that are completely deleted. The full
set can be found here:
https://github.com/ardbiesheuvel/edk2/tree/embeddedpkg-spring-cleaning

Ard Biesheuvel (5):
EmbeddedPkg: remove DwEmmcDxe host controller driver
EmbeddedPkg: remove Lan91x network controller driver
EmbeddedPkg: remove Lan9118 network controller driver
EmbeddedPkg: remove SiI3132 SATA controller driver
EmbeddedPkg: remove ISP 1716 USB host controller driver
Many thanks for this! For the series:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Cheers

Pushed this, along with the PL180 one you acked earlier.

https://github.com/tianocore/edk2/pull/577


Re: [PATCH 1/1] BaseTools: add repo name option to SetupGit.py

Laszlo Ersek
 

CC Leif

On 05/01/20 22:00, Rebecca Cran wrote:
Allow users who didn't clone one of the TianoCore repos from a
canonical URL to specify the name of the repo (edk2, edk2-platforms
or edk2-non-osi) when running SetupGit.py to allow them to configure
their repo properly.

The new option is:

-n repo, --name repo set the repo name to configure for, if not
detected automatically

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
Tested with:
pylama : no new issues detected
Option tested:
-n : correctly said a parameter was needed
-n edk2 : configured the repo for edk2
-n edk2-foo : errored out with a list of repo names
-n edk2-platforms : updated the configuration for edk2-platforms

Note the error block in __main__ if the upstream isn't found is
redundant, since it already errors out and exits in get_upstream.

BaseTools/Scripts/SetupGit.py | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Scripts/SetupGit.py b/BaseTools/Scripts/SetupGit.py
index e320ba2f887e..4416111ac0a5 100644
--- a/BaseTools/Scripts/SetupGit.py
+++ b/BaseTools/Scripts/SetupGit.py
@@ -106,10 +106,11 @@ def fuzzy_match_repo_url(one, other):
return False


-def get_upstream(url):
+def get_upstream(url, name):
"""Extracts the dict for the current repo origin."""
for upstream in UPSTREAMS:
- if fuzzy_match_repo_url(upstream['repo'], url):
+ if (fuzzy_match_repo_url(upstream['repo'], url) or
+ upstream['name'] == name):
return upstream
print("Unknown upstream '%s' - aborting!" % url)
sys.exit(3)
@@ -143,6 +144,11 @@ if __name__ == '__main__':
help='overwrite existing settings conflicting with program defaults',
action='store_true',
required=False)
+ PARSER.add_argument('-n', '--name', type=str, metavar='repo',
+ choices=['edk2', 'edk2-platforms', 'edk2-non-osi'],
+ help='set the repo name to configure for, if not '
+ 'detected automatically',
+ required=False)
PARSER.add_argument('-v', '--verbose',
help='enable more detailed output',
action='store_true',
@@ -156,7 +162,7 @@ if __name__ == '__main__':

URL = REPO.remotes.origin.url

- UPSTREAM = get_upstream(URL)
+ UPSTREAM = get_upstream(URL, ARGS.name)
if not UPSTREAM:
print("Upstream '%s' unknown, aborting!" % URL)
sys.exit(7)


Re: [PATCH v7 01/43] MdeModulePkg: Create PCDs to be used in support of SEV-ES

Laszlo Ersek
 

On 05/04/20 18:41, Tom Lendacky wrote:

Is there an easy way to run everything that this link points, too? Is it
just creating a pull request that does this? I don't want to take up a
lot of your time, so if there's some documentation on how to run an
integration test to find and fix issues like this, just point me to it.
Just create a pull request; it will set off CI, and you can review VS
build errors there (if any).

Your PR will automatically be closed (rejected) regardless of whether CI
succeeds or not. PRs are merged -- in fact, *auto*-merged, by the
"mergify bot" -- if and only if (a) the CI run succeeds, and (b) the PR
has the "push" label set.

And only edk2 maintainers have permission to set the "push" label. Any
PR without the "push" label qualifies as a "personal test build". So you
can freely experiment with PRs, because you can't (even unwittingly)
satisfy condition (b).

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

Thanks,
Laszlo


Re: [PATCH v7 33/43] OvmfPkg: Reserve a page in memory for the SEV-ES usage

Laszlo Ersek
 

On 05/01/20 00:09, Tom Lendacky wrote:
On 4/30/20 4:12 PM, Tom Lendacky wrote:
On 4/30/20 1:58 PM, Laszlo Ersek wrote:
Hi Tom,
Hi Laszlo,


On 04/22/20 19:41, Lendacky, Thomas wrote:
BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&;data=02%7C01%7Cthomas.lendacky%40amd.com%7Cce256f35aa2e4748e8e008d7ed3874ae%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637238699042461059&amp;sdata=tXX8nkBo3fB4OVTs2avevW8pwL6AcqJHvFhvlshKySI%3D&amp;reserved=0


Reserve a fixed area of memory for SEV-ES use and set a fixed PCD,
PcdSevEsWorkAreaBase, to this value.

This area will be used by SEV-ES support for two purposes:
   1. Communicating the SEV-ES status during BSP boot to SEC:
      Using a byte of memory from the page, the BSP reset vector
code can
      communicate the SEV-ES status to SEC for use before exception
      handling can be enabled in SEC. After SEC, this field is no
longer
      valid and the standard way of determine if SEV-ES is active
should
      be used.

   2. Establishing an area of memory for AP boot support:
      A hypervisor is not allowed to update an SEV-ES guest's register
      state, so when booting an SEV-ES guest AP, the hypervisor is not
      allowed to set the RIP to the guest requested value. Instead an
      SEV-ES AP must be re-directed from within the guest to the actual
      requested staring location as specified in the INIT-SIPI-SIPI
      sequence.

      Use this memory for reset vector code that can be programmed
to have
      the AP jump to the desired RIP location after starting the AP.
This
      is required for only the very first AP reset.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
  OvmfPkg/OvmfPkgX64.fdf | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 36414c1f8b49..a0bea86f9875 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -82,6 +82,9 @@ [FD.MEMFD]
  0x009000|0x002000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize

+0x00B000|0x001000
+gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize

+
  0x010000|0x010000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize

in patch #28 ("OvmfPkg: Create a GHCB page for use during Sec phase"),
we carve out two ranges in FD.MEMFD, and introduce a matching set of
4 PCDs.

Then in patch #29 ("OvmfPkg/PlatformPei: Reserve GHCB-related areas if
S3 is supported"), we reserve those ranges from the OS, as AcpiNVS, if
S3 is supported. The reason we only reserve those ranges if S3 is
enabled because the ranges are only needed in SEC. (See the details in
the commit mesage of patch #29.)

In this patch (patch #33), we carve out a third region in FD.MEMFD. We
don't seem to ever reserve it. I think that's minimally a problem for
S3; the same argument should apply as to the other two areas. Do you
agree?
Nice catch! Yes, I missed this one.



Furthermore, I wonder if we should reserve this "work area" from the OS,
and even from the DXE phase (!), *regardless* of S3. I can't immediately
tell when it's the last time (with S3 disabled) when this area is used.

As I understand it, it is only used the first time the APs are booted
up. And that should happen still in the PEI phase, because CpuMpPei
boots up all the APs and counts them. Afterwards (still in the PEI
phase), the APs should be sleeping in ApWakeupFunction(), namely in the
code added by patch #40 ("UefiCpuPkg: Allow AP booting under SEV-ES").
If the AP is woken again, it is actually only "released" by the
hypervisor, and it goes through the special 64bit->16bit transition,
again implemented in patch#40.

So ultimately it shouldn't be necessary to reserve this third region (at
PcdSevEsWorkAreaBase), if S3 is disabled, because it is never used past
the very first AP boot (which happens when CpuMpPei counts the APs).

Do I understand right?
Yes, that is correct. So I just need to do the same thing for this
area that I did in patch #29.
I think I might want to protect the area from DXE allocations, too.
OK. In such cases, the area in question is always covered with a memory
allocation HOB; what depends on S3 is only whether the memory type is
"boot services data" (S3 disabled on the QEMU cmdline) or AcpiNVS (S3
enabled). The region is then always kept away from DXE, and S3
enablement determines only whether the OS is told to stay away as well.

Is
there an easy way to detect that PEI is active vs DXE? Even so, will it
*always* be the case that PEI will start the APs first? I'd hate to see
a change down the road where PEI doesn't start the APs and then things
break.
We'll have to continue including CpuMpPei, otherwise we couldn't re-set
the feature control MSR to the desired value at S3 resume. IOW, removing
CpuMpPei from OVMF would regress at least that feature:

https://bugzilla.tianocore.org/show_bug.cgi?id=86

Including CpuMpPei in OVMF was surprisingly quirky; I don't see it going
away anytime soon.

Thanks
Laszlo


Re: [edk2-rfc] [RFCv2] code-first process for UEFI-forum specifications

Samer El-Haj-Mahmoud
 

Acked-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Leif Lindholm
via Groups.Io
Sent: Monday, March 23, 2020 3:06 PM
To: devel@edk2.groups.io; rfc@edk2.groups.io
Cc: Felixp@ami.com; Mark Doran <mark.doran@intel.com>; Andrew Fish
<afish@apple.com>; Laszlo Ersek <lersek@redhat.com>; Michael D Kinney
<michael.d.kinney@intel.com>
Subject: [edk2-rfc] [RFCv2] code-first process for UEFI-forum specifications

Changes to v2 of this proposal:
- Feedback from Laszlo[a] and Mike[b] incorporated.
- I opted to view Mike's responses to Laszlo's questions as
accepted, as no follow-up was made.

Feedback from Felix[c] *not* incorporated, as while I agree with all of it, I am
not convinced that information should go here, but should instead reside with
the UEFI Forum. (This text documents the public part of the process - it would
cause me slight impedance mismatch to have it also document the non-public
part.)

[a] https://edk2.groups.io/g/devel/message/53422
[b] https://edk2.groups.io/g/devel/message/53738
[c] https://edk2.groups.io/g/devel/message/54453

/
Leif

---

This is a proposal for a process by which new features can be added to UEFI
forum specifications after first having been designed and prototyped in the
open.

This process lets changes and the development of new features happen in the
open, without violating the UEFI forum bylaws which prevent publication of
code for in-draft features/changes.

The process does not in fact change the UEFI bylaws - the change is that the
development (of both specification and code) happens in the open. The
resulting specification update is then submitted to the appropriate working
goup as an Engineering Change Request (ECR), and voted on. For the UEFI
Forum, this is a change in workflow, not a change in process.

ECRs are tracked in a UEFI Forum Mantis instance, access restricted to UEFI
Forum Members. TianoCore will enable this new process by providing areas
on https://bugzilla.tianocore.org/ to track both specification updates and
reference implementations and new repositories under
https://github.com/tianocore/ dedicated to hold "code first".


## Bugzilla

bugzilla.tianocore.org will have a product category each for
* ACPI Specification
* PI Specification
* UEFI Specification

Each product category will have a separate components for
* Specification
* Reference implementation


## Github
New repositories will be added for holding the text changes and the source
code.

Specification text changes will be held within the affected source repository,
in the Github flavour of markdown, in a file (or split across several files) with
.md suffix.
(This one may break down where we have a specification change affecting
multiple specifications, but at that point we can track it with multiple BZ
entries)

Reference implementations targeting EDK2 will be held in branches on edk2-
staging.
Additional repositories for implementing reference features in additional
open source projects can be added in the future, as required.


## Intended workflow
The entity initiating a specifiation update raises a Bugzilla in the appropriate
area in bugzilla.tianocore.org. This entry contains the outline of the change,
and the full initial draft text is attached.

If multiple specification updates are interdependent, especially if between
different specifications, then multiple bugzilla entries should be created.
These bugzilla entries *must* be linked together with dependencies.

After the BZs have been created, new branches should be created in the
relevant repositories for each bugzilla - the branch names should be BZ####,
where #### describes the bugzilla ID assigned, optionally followed by a '-' and
something more descriptive. If multiple bugzilla entries must coexist on a
single branch, one of them is designated the 'top-level', with dependencies
properly tracked.
That BZ will be the one naming the branch.


## Source code
In order to ensure draft code does not accidentally leak into production use,
and to signify when the changeover from draft to final happens, *all* new or
modified[1] identifiers need to be prefixed with the relevant BZ####.

[1] Modified in a non-backwards-compatible way. If, for example, a statically
sized array is grown - this does not need to be prefixed. But a tag in a
comment would be *highly* recommended.

### File names
New public header files need the prefix. I.e. `Bz1234MyNewProtocol.h`
Private header files do not need the prefix.

### Contents

The tagging must follow the coding style used by each affected codebase.
Examples:

| Released in spec | Draft version in tree | Comment |
| --- | --- | --- |
| `FunctionName` | `Bz1234FunctionName` | |
| `HEADER_MACRO` | `BZ1234_HEADER_MACRO` | |

For data structures or enums, any new or non-backwards-compatible structs
or fields require a prefix. As above, growing an existing array in an existing
struct requires no prefix.

| `typedef SOME_STRUCT` | `BZ1234_SOME_STRUCT` | Typedef only [2]
|
| `StructField` | `Bz1234StructField` | In existing struct[3] |
| `typedef SOME_ENUM` | `BZ1234_SOME_ENUM` | Typedef only [2]
|

[2] If the struct or enum definition is separate from the typedef in the public
header, the definition does not need the prefix.
[3] Individual fields in newly added typedefd struct do not need prefix, the
struct already carried the prefix.

Variable prefixes indicating global scope ('g' or 'm') go before the BZ prefix.

| `gSomeGuid` | `gBz1234SomeGuid` | |

Local identifiers, including module-global ones (m-prefixed) do not require a
BZ prefix.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH v7 33/43] OvmfPkg: Reserve a page in memory for the SEV-ES usage

Laszlo Ersek
 

On 04/30/20 23:12, Tom Lendacky wrote:
On 4/30/20 1:58 PM, Laszlo Ersek wrote:
Hi Tom,
Hi Laszlo,


On 04/22/20 19:41, Lendacky, Thomas wrote:
BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&;data=02%7C01%7Cthomas.lendacky%40amd.com%7Cce256f35aa2e4748e8e008d7ed3874ae%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637238699042461059&amp;sdata=tXX8nkBo3fB4OVTs2avevW8pwL6AcqJHvFhvlshKySI%3D&amp;reserved=0


Reserve a fixed area of memory for SEV-ES use and set a fixed PCD,
PcdSevEsWorkAreaBase, to this value.

This area will be used by SEV-ES support for two purposes:
   1. Communicating the SEV-ES status during BSP boot to SEC:
      Using a byte of memory from the page, the BSP reset vector code
can
      communicate the SEV-ES status to SEC for use before exception
      handling can be enabled in SEC. After SEC, this field is no longer
      valid and the standard way of determine if SEV-ES is active should
      be used.

   2. Establishing an area of memory for AP boot support:
      A hypervisor is not allowed to update an SEV-ES guest's register
      state, so when booting an SEV-ES guest AP, the hypervisor is not
      allowed to set the RIP to the guest requested value. Instead an
      SEV-ES AP must be re-directed from within the guest to the actual
      requested staring location as specified in the INIT-SIPI-SIPI
      sequence.

      Use this memory for reset vector code that can be programmed to
have
      the AP jump to the desired RIP location after starting the AP.
This
      is required for only the very first AP reset.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
  OvmfPkg/OvmfPkgX64.fdf | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 36414c1f8b49..a0bea86f9875 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -82,6 +82,9 @@ [FD.MEMFD]
  0x009000|0x002000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize

  +0x00B000|0x001000
+gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize

+
  0x010000|0x010000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize

 
in patch #28 ("OvmfPkg: Create a GHCB page for use during Sec phase"),
we carve out two ranges in FD.MEMFD, and introduce a matching set of 4
PCDs.

Then in patch #29 ("OvmfPkg/PlatformPei: Reserve GHCB-related areas if
S3 is supported"), we reserve those ranges from the OS, as AcpiNVS, if
S3 is supported. The reason we only reserve those ranges if S3 is
enabled because the ranges are only needed in SEC. (See the details in
the commit mesage of patch #29.)

In this patch (patch #33), we carve out a third region in FD.MEMFD. We
don't seem to ever reserve it. I think that's minimally a problem for
S3; the same argument should apply as to the other two areas. Do you
agree?
Nice catch! Yes, I missed this one.



Furthermore, I wonder if we should reserve this "work area" from the OS,
and even from the DXE phase (!), *regardless* of S3. I can't immediately
tell when it's the last time (with S3 disabled) when this area is used.

As I understand it, it is only used the first time the APs are booted
up. And that should happen still in the PEI phase, because CpuMpPei
boots up all the APs and counts them. Afterwards (still in the PEI
phase), the APs should be sleeping in ApWakeupFunction(), namely in the
code added by patch #40 ("UefiCpuPkg: Allow AP booting under SEV-ES").
If the AP is woken again, it is actually only "released" by the
hypervisor, and it goes through the special 64bit->16bit transition,
again implemented in patch#40.

So ultimately it shouldn't be necessary to reserve this third region (at
PcdSevEsWorkAreaBase), if S3 is disabled, because it is never used past
the very first AP boot (which happens when CpuMpPei counts the APs).

Do I understand right?
Yes, that is correct. So I just need to do the same thing for this area
that I did in patch #29.

I can probably shift patch #29 after #33 and have one patch for the S3
reservation instead of having two separate patches doing S3 reservation.
Sounds good, thanks!
Laszlo


Re: [PATCH edk2-platforms v4 01/24] Silicon/NXP: Add I2c lib

Leif Lindholm
 

On Fri, May 01, 2020 at 11:19:32 +0530, Pankaj Bansal wrote:
From: Pankaj Bansal <pankaj.bansal@nxp.com>

Add an I2c library to control the i2c controllers to communicate
with I2c device.

We need this functionality in a lib because this is going to be used
in PrePeiCore sec module to get the System clock information from
devices connected to i2c (like fpga or clock generator)

In subsequent patches I2c functionality in I2cDxe driver would be removed
and I2cLib APIs would be used in I2cDxe.

The I2cLib differs from I2c Controller functionality in I2c Dxe in
several aspects:
- I2cLib doesn't assume that each I2c Transaction would be of minimum
two operations
- I2cLib generates repeat start between operations which complies with
PI I2c specs.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
V4:
- Dividor -> divider
- Added line between license and copyright in files
- Fixed newline at the end of file errors
- removed I2cEarlyInitialize
- changed Ibfd from UINT16 to UINT8 in I2C_CLOCK_DIVISOR_PAIR in
I2cLibInternal.h
I don't recall this being mentioned as part of the review, and I did
not see it called out in the cover letter. I am not going to ask for
another revision now, and this may be tiny enough that it doesn't make
sense to break out as a separate patch - but that may be something to
consider in the future for bugs you find yourself during code rework.

Reviewed-by: Leif Lindholm <leif@nuviainc.com>

- Modify commit description

V3:
- moved I2cLib addition in NxpQoriqLs.dsc.inc to PATCH 2
- Added Qoriq Layerscape in description of CLOCK_DIVISOR_PAIR
- ERRXXXXXX -> A-XXXXXX
- Added u-boot patch reference to errata workaround description
- Added Glossary and Revision Reference in file header
- Dividor -> divider
- Added explaination before calling I2cErratumA009203
- PtrAddress -> AddressPtr
- Update commit description

Silicon/NXP/NxpQoriqLs.dec | 9 +-
Silicon/NXP/Library/I2cLib/I2cLib.inf | 30 ++
Silicon/NXP/Include/Library/I2cLib.h | 100 ++++
Silicon/NXP/Library/I2cLib/I2cLibInternal.h | 105 ++++
Silicon/NXP/Library/I2cLib/I2cLib.c | 545 ++++++++++++++++++++
5 files changed, 788 insertions(+), 1 deletion(-)

diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
index 764b9bb0e2d3..1aff519bfaf4 100644
--- a/Silicon/NXP/NxpQoriqLs.dec
+++ b/Silicon/NXP/NxpQoriqLs.dec
@@ -1,6 +1,6 @@
# @file.
#
-# Copyright 2017-2019 NXP
+# Copyright 2017-2020 NXP
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -13,6 +13,10 @@
[Includes]
Include

+[LibraryClasses]
+ ## @libraryclass Provides services to read/write to I2c devices
+ I2cLib|Include/Library/I2cLib.h
+
[Guids.common]
gNxpQoriqLsTokenSpaceGuid = {0x98657342, 0x4aee, 0x4fc6, {0xbc, 0xb5, 0xff, 0x45, 0xb7, 0xa8, 0x71, 0xf2}}
gNxpNonDiscoverableI2cMasterGuid = { 0x5f2c099c, 0x54a3, 0x4dd4, {0x9e, 0xc5, 0xe9, 0x12, 0x8c, 0x36, 0x81, 0x6a}}
@@ -101,3 +105,6 @@
gNxpQoriqLsTokenSpaceGuid.PcdPciLutBigEndian|FALSE|BOOLEAN|0x00000312
gNxpQoriqLsTokenSpaceGuid.PcdWatchdogBigEndian|FALSE|BOOLEAN|0x00000313
gNxpQoriqLsTokenSpaceGuid.PcdIfcBigEndian|FALSE|BOOLEAN|0x00000314
+
+[PcdsFeatureFlag]
+ gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000315
diff --git a/Silicon/NXP/Library/I2cLib/I2cLib.inf b/Silicon/NXP/Library/I2cLib/I2cLib.inf
new file mode 100644
index 000000000000..3da2331db37d
--- /dev/null
+++ b/Silicon/NXP/Library/I2cLib/I2cLib.inf
@@ -0,0 +1,30 @@
+# @file
+# Component description file for I2cLib module
+#
+# Copyright 2017, 2020 NXP
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = I2cLib
+ FILE_GUID = 8ecefc8f-a2c4-4091-b81f-20f7aeb0567f
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = I2cLib
+
+[Sources.common]
+ I2cLib.c
+
+[LibraryClasses]
+ IoLib
+ TimerLib
+
+[Packages]
+ MdePkg/MdePkg.dec
+ Silicon/NXP/NxpQoriqLs.dec
+
+[FeaturePcd]
+ gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203
diff --git a/Silicon/NXP/Include/Library/I2cLib.h b/Silicon/NXP/Include/Library/I2cLib.h
new file mode 100644
index 000000000000..f09324c00e47
--- /dev/null
+++ b/Silicon/NXP/Include/Library/I2cLib.h
@@ -0,0 +1,100 @@
+/** @file
+ I2c Lib to control I2c controller.
+
+ Copyright 2020 NXP
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef I2C_LIB_H__
+#define I2C_LIB_H__
+
+#include <Uefi.h>
+#include <Pi/PiI2c.h>
+
+/**
+ software reset of the entire I2C module.
+ The module is reset and disabled.
+ Status register fields (IBSR) are cleared.
+
+ @param[in] Base Base Address of I2c controller's registers
+
+ @return EFI_SUCCESS successfuly reset the I2c module
+**/
+EFI_STATUS
+I2cReset (
+ IN UINTN Base
+ );
+
+/**
+ Configure I2c bus to operate at a given speed
+
+ @param[in] Base Base Address of I2c controller's registers
+ @param[in] I2cBusClock Input clock to I2c controller
+ @param[in] Speed speed to be configured for I2c bus
+
+ @return EFI_SUCCESS successfuly setup the i2c bus
+**/
+EFI_STATUS
+I2cInitialize (
+ IN UINTN Base,
+ IN UINT64 I2cBusClock,
+ IN UINT64 Speed
+ );
+
+/**
+ Transfer data to/from I2c slave device
+
+ @param[in] Base Base Address of I2c controller's registers
+ @param[in] SlaveAddress Slave Address from which data is to be read
+ @param[in] RequestPacket Pointer to an EFI_I2C_REQUEST_PACKET structure
+ describing the I2C transaction
+
+ @return EFI_SUCCESS successfuly transfer the data
+ @return EFI_DEVICE_ERROR There was an error while transferring data through
+ I2c bus
+ @return EFI_NO_RESPONSE There was no Ack from i2c device
+ @return EFI_TIMEOUT I2c Bus is busy
+ @return EFI_NOT_READY I2c Bus Arbitration lost
+**/
+EFI_STATUS
+I2cBusXfer (
+ IN UINTN Base,
+ IN UINT32 SlaveAddress,
+ IN EFI_I2C_REQUEST_PACKET *RequestPacket
+ );
+
+/**
+ Read a register from I2c slave device. This API is wrapper around I2cBusXfer
+
+ @param[in] Base Base Address of I2c controller's registers
+ @param[in] SlaveAddress Slave Address from which register value is
+ to be read
+ @param[in] RegAddress Register Address in Slave's memory map
+ @param[in] RegAddressWidthInBytes Number of bytes in RegAddress to send to
+ I2c Slave for simple reads without any
+ register, make this value = 0
+ (RegAddress is don't care in that case)
+ @param[out] RegValue Value to be read from I2c slave's regiser
+ @param[in] RegValueNumBytes Number of bytes to read from I2c slave
+ register
+
+ @return EFI_SUCCESS successfuly read the registers
+ @return EFI_DEVICE_ERROR There was an error while transferring data through
+ I2c bus
+ @return EFI_NO_RESPONSE There was no Ack from i2c device
+ @return EFI_TIMEOUT I2c Bus is busy
+ @return EFI_NOT_READY I2c Bus Arbitration lost
+**/
+EFI_STATUS
+I2cBusReadReg (
+ IN UINTN Base,
+ IN UINT32 SlaveAddress,
+ IN UINT64 RegAddress,
+ IN UINT8 RegAddressWidthInBytes,
+ OUT UINT8 *RegValue,
+ IN UINT32 RegValueNumBytes
+ );
+
+#endif // I2C_LIB_H__
diff --git a/Silicon/NXP/Library/I2cLib/I2cLibInternal.h b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
new file mode 100644
index 000000000000..cd82c423947a
--- /dev/null
+++ b/Silicon/NXP/Library/I2cLib/I2cLibInternal.h
@@ -0,0 +1,105 @@
+/** @file
+ I2c Lib to control I2c controller.
+
+ Copyright 2020 NXP
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef I2C_LIB_INTERNAL_H__
+#define I2C_LIB_INTERNAL_H__
+
+#include <Pi/PiI2c.h>
+#include <Uefi.h>
+
+/** Module Disable
+ 0b - The module is enabled. You must clear this field before any other IBCR
+ fields have any effect.
+ 1b - The module is reset and disabled. This is the power-on reset situation.
+ When high, the interface is held in reset, but registers can still be
+ accessed. Status register fields (IBSR) are not valid when the module
+ is disabled.
+**/
+#define I2C_IBCR_MDIS BIT7
+// I2c Bus Interrupt Enable
+#define I2C_IBCR_IBIE BIT6
+/** Master / Slave Mode 0b - Slave mode 1b - Master mode
+ When you change this field from 0 to 1, the module generates a START signal
+ on the bus and selects the master mode. When you change this field from 1 to
+ 0, the module generates a STOP signal and changes the operation mode from
+ master to slave. You should generate a STOP signal only if IBSR[IBIF]=1.
+ The module clears this field without generating a STOP signal when the
+ master loses arbitration.
+*/
+#define I2C_IBCR_MSSL BIT5
+// 0b - Receive 1b - Transmit
+#define I2C_IBCR_TXRX BIT4
+/** Data acknowledge disable
+ Values written to this field are only used when the I2C module is a receiver,
+ not a transmitter.
+ 0b - The module sends an acknowledge signal to the bus at the 9th clock bit
+ after receiving one byte of data.
+ 1b - The module does not send an acknowledge-signal response (that is,
+ acknowledge bit = 1).
+**/
+#define I2C_IBCR_NOACK BIT3
+/**Repeat START
+ If the I2C module is the current bus master, and you program RSTA=1, the I2C
+ module generates a repeated START condition. This field always reads as a 0.
+ If you attempt a repeated START at the wrong time, if the bus is owned by
+ another master the result is loss of arbitration.
+**/
+#define I2C_IBCR_RSTA BIT2
+// DMA enable
+#define I2C_IBCR_DMAEN BIT1
+
+// Transfer Complete
+#define I2C_IBSR_TCF BIT7
+// I2C bus Busy. 0b - Bus is idle, 1b - Bus is busy
+#define I2C_IBSR_IBB BIT5
+// Arbitration Lost. software must clear this field by writing a one to it.
+#define I2C_IBSR_IBAL BIT4
+// I2C bus interrupt flag
+#define I2C_IBSR_IBIF BIT1
+// Received acknowledge 0b - Acknowledge received 1b - No acknowledge received
+#define I2C_IBSR_RXAK BIT0
+
+//Bus idle interrupt enable
+#define I2C_IBIC_BIIE BIT7
+
+// Glitch filter enable
+#define I2C_IBDBG_GLFLT_EN BIT3
+
+#define I2C_BUS_TEST_BUSY TRUE
+#define I2C_BUS_TEST_IDLE !I2C_BUS_TEST_BUSY
+#define I2C_BUS_TEST_RX_ACK TRUE
+#define I2C_BUS_NO_TEST_RX_ACK !I2C_BUS_TEST_RX_ACK
+
+#define ARRAY_LAST_ELEM(x) (x)[ARRAY_SIZE (x) - 1]
+#define I2C_NUM_RETRIES 500
+
+typedef struct _I2C_REGS {
+ UINT8 Ibad; // I2c Bus Address Register
+ UINT8 Ibfd; // I2c Bus Frequency Divider Register
+ UINT8 Ibcr; // I2c Bus Control Register
+ UINT8 Ibsr; // I2c Bus Status Register
+ UINT8 Ibdr; // I2C Bus Data I/O Register
+ UINT8 Ibic; // I2C Bus Interrupt Config Register
+ UINT8 Ibdbg; // I2C Bus Debug Register
+} I2C_REGS;
+
+/*
+ * sorted list of clock divisor, Ibfd register value pairs
+ */
+typedef struct _I2C_CLOCK_DIVISOR_PAIR {
+ UINT16 Divisor;
+ UINT8 Ibfd; // I2c Bus Frequency Divider Register value
+} I2C_CLOCK_DIVISOR_PAIR;
+
+typedef struct {
+ UINTN OperationCount;
+ EFI_I2C_OPERATION Operation[2];
+} I2C_REG_REQUEST;
+
+#endif // I2C_LIB_INTERNAL_H__
diff --git a/Silicon/NXP/Library/I2cLib/I2cLib.c b/Silicon/NXP/Library/I2cLib/I2cLib.c
new file mode 100644
index 000000000000..4b9055969112
--- /dev/null
+++ b/Silicon/NXP/Library/I2cLib/I2cLib.c
@@ -0,0 +1,545 @@
+/** @file
+ I2c Lib to control I2c controller.
+
+ Copyright 2017, 2020 NXP
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Revision Reference:
+ - PI Version 1.7
+
+ @par Glossary:
+ - Ibfd - I2c Bus Frequency Divider
+**/
+#include <Uefi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/I2cLib.h>
+#include <Library/IoLib.h>
+#include <Library/TimerLib.h>
+
+#include "I2cLibInternal.h"
+
+/**
+ I2C divisor and Ibfd register values when glitch filter is enabled
+
+ In case of duplicate SCL Divisor value, the Ibfd value with high MUL value
+ has been selected. A higher MUL value results in a lower sampling rate of
+ the I2C signals. This gives the I2C module greater immunity against glitches
+ in the I2C signals.
+
+ These values can be referred from NXP QorIQ Layerscape series SOC Reference
+ Manual in which this I2C ip has been used. e.g. LX2160ARM, LS1043ARM
+**/
+STATIC CONST I2C_CLOCK_DIVISOR_PAIR mI2cClockDivisorGlitchEnabled[] = {
+ { 34, 0x0 }, { 36, 0x1 }, { 38, 0x2 }, { 40, 0x3 },
+ { 42, 0x4 }, { 44, 0x8 }, { 48, 0x9 }, { 52, 0xA },
+ { 54, 0x7 }, { 56, 0xB }, { 60, 0xC }, { 64, 0x10 },
+ { 68, 0x40 }, { 72, 0x41 }, { 76, 0x42 }, { 80, 0x43 },
+ { 84, 0x44 }, { 88, 0x48 }, { 96, 0x49 }, { 104, 0x4A },
+ { 108, 0x47 }, { 112, 0x4B }, { 120, 0x4C }, { 128, 0x50 },
+ { 136, 0x80 }, { 144, 0x81 }, { 152, 0x82 }, { 160, 0x83 },
+ { 168, 0x84 }, { 176, 0x88 }, { 192, 0x89 }, { 208, 0x8A },
+ { 216, 0x87 }, { 224, 0x8B }, { 240, 0x8C }, { 256, 0x90 },
+ { 288, 0x91 }, { 320, 0x92 }, { 336, 0x8F }, { 352, 0x93 },
+ { 384, 0x98 }, { 416, 0x95 }, { 448, 0x99 }, { 480, 0x96 },
+ { 512, 0x9A }, { 576, 0x9B }, { 640, 0xA0 }, { 704, 0x9D },
+ { 768, 0xA1 }, { 832, 0x9E }, { 896, 0xA2 }, { 960, 0x67 },
+ { 1024, 0xA3 }, { 1152, 0xA4 }, { 1280, 0xA8 }, { 1536, 0xA9 },
+ { 1792, 0xAA }, { 1920, 0xA7 }, { 2048, 0xAB }, { 2304, 0xAC },
+ { 2560, 0xB0 }, { 3072, 0xB1 }, { 3584, 0xB2 }, { 3840, 0xAF },
+ { 4096, 0xB3 }, { 4608, 0xB4 }, { 5120, 0xB8 }, { 6144, 0xB9 },
+ { 7168, 0xBA }, { 7680, 0xB7 }, { 8192, 0xBB }, { 9216, 0xBC },
+ { 10240, 0xBD }, { 12288, 0xBE }, { 15360, 0xBF }
+};
+
+/**
+ I2C divisor and Ibfd register values when glitch filter is disabled
+
+ In case of duplicate SCL Divisor value, the Ibfd value with high MUL value
+ has been selected. A higher MUL value results in a lower sampling rate of
+ the I2C signals. This gives the I2C module greater immunity against glitches
+ in the I2C signals.
+
+ These values can be referred from NXP QorIQ Layerscape series SOC Reference
+ Manual in which this I2C ip has been used. e.g. LX2160ARM, LS1043ARM
+**/
+STATIC CONST I2C_CLOCK_DIVISOR_PAIR mI2cClockDivisorGlitchDisabled[] = {
+ { 20, 0x0 },{ 22, 0x1 },{ 24, 0x2 },{ 26, 0x3 },
+ { 28, 0x8 },{ 30, 0x5 },{ 32, 0x9 },{ 34, 0x6 },
+ { 36, 0x0A },{ 40, 0x40 },{ 44, 0x41 },{ 48, 0x42 },
+ { 52, 0x43 },{ 56, 0x48 },{ 60, 0x45 },{ 64, 0x49 },
+ { 68, 0x46 },{ 72, 0x4A },{ 80, 0x80 },{ 88, 0x81 },
+ { 96, 0x82 },{ 104, 0x83 },{ 112, 0x88 },{ 120, 0x85 },
+ { 128, 0x89 },{ 136, 0x86 },{ 144, 0x8A },{ 160, 0x8B },
+ { 176, 0x8C },{ 192, 0x90 },{ 208, 0x56 },{ 224, 0x91 },
+ { 240, 0x1F },{ 256, 0x92 },{ 272, 0x8F },{ 288, 0x93 },
+ { 320, 0x98 },{ 352, 0x95 },{ 384, 0x99 },{ 416, 0x96 },
+ { 448, 0x9A },{ 480, 0x5F },{ 512, 0x9B },{ 576, 0x9C },
+ { 640, 0xA0 },{ 768, 0xA1 },{ 896, 0xA2 },{ 960, 0x9F },
+ { 1024, 0xA3 },{ 1152, 0xA4 },{ 1280, 0xA8 },{ 1536, 0xA9 },
+ { 1792, 0xAA },{ 1920, 0xA7 },{ 2048, 0xAB },{ 2304, 0xAC },
+ { 2560, 0xAD },{ 3072, 0xB1 },{ 3584, 0xB2 },{ 3840, 0xAF },
+ { 4096, 0xB3 },{ 4608, 0xB4 },{ 5120, 0xB8 },{ 6144, 0xB9 },
+ { 7168, 0xBA },{ 7680, 0xB7 },{ 8192, 0xBB },{ 9216, 0xBC },
+ { 10240, 0xBD },{ 12288, 0xBE },{ 15360, 0xBF }
+};
+
+/**
+ A-009203 : I2C may not work reliably with the default setting
+
+ Description : The clocking circuitry of I2C module may not work reliably due
+ to the slow rise time of SCL signal.
+ Workaround : Enable the receiver digital filter by setting IBDBG[GLFLT_EN]
+ to 1. refer https://patchwork.ozlabs.org/patch/453575/
+**/
+STATIC
+VOID
+I2cErratumA009203 (
+ IN UINTN Base
+ )
+{
+ I2C_REGS *Regs;
+
+ Regs = (I2C_REGS *)Base;
+
+ MmioOr8 ((UINTN)&Regs->Ibdbg, I2C_IBDBG_GLFLT_EN);
+}
+
+/**
+ software reset of the entire I2C module.
+ The module is reset and disabled.
+ Status register fields (IBSR) are cleared.
+
+ @param[in] Base Base Address of I2c controller's registers
+
+ @return EFI_SUCCESS successfuly reset the I2c module
+**/
+EFI_STATUS
+I2cReset (
+ IN UINTN Base
+ )
+{
+ I2C_REGS *Regs;
+
+ Regs = (I2C_REGS *)Base;
+
+ MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
+ MmioOr8 ((UINTN)&Regs->Ibsr, (I2C_IBSR_IBAL | I2C_IBSR_IBIF));
+ MmioAnd8 ((UINTN)&Regs->Ibcr, ~(I2C_IBCR_IBIE | I2C_IBCR_DMAEN));
+ MmioAnd8 ((UINTN)&Regs->Ibic, (UINT8)(~I2C_IBIC_BIIE));
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Configure I2c bus to operate at a given speed
+
+ @param[in] Base Base Address of I2c controller's registers
+ @param[in] I2cBusClock Input clock to I2c controller
+ @param[in] Speed speed to be configured for I2c bus
+
+ @return EFI_SUCCESS successfuly setup the i2c bus
+**/
+EFI_STATUS
+I2cInitialize (
+ IN UINTN Base,
+ IN UINT64 I2cBusClock,
+ IN UINT64 Speed
+ )
+{
+ I2C_REGS *Regs;
+ UINT16 ClockDivisor;
+ UINT8 Ibfd; // I2c Bus Frequency Divider Register
+ CONST I2C_CLOCK_DIVISOR_PAIR *ClockDivisorPair;
+ UINT32 ClockDivisorPairSize;
+ UINT32 Index;
+
+ Regs = (I2C_REGS *)Base;
+ if (FeaturePcdGet (PcdI2cErratumA009203)) {
+ // Apply Erratum A-009203 before writing Ibfd register
+ I2cErratumA009203 (Base);
+ }
+
+ Ibfd = 0;
+ ClockDivisor = (I2cBusClock + Speed - 1) / Speed;
+
+ if (MmioRead8 ((UINTN)&Regs->Ibdbg) & I2C_IBDBG_GLFLT_EN) {
+ ClockDivisorPair = mI2cClockDivisorGlitchEnabled;
+ ClockDivisorPairSize = ARRAY_SIZE (mI2cClockDivisorGlitchEnabled);
+ } else {
+ ClockDivisorPair = mI2cClockDivisorGlitchDisabled;
+ ClockDivisorPairSize = ARRAY_SIZE (mI2cClockDivisorGlitchDisabled);
+ }
+
+ if (ClockDivisor > ClockDivisorPair[ClockDivisorPairSize - 1].Divisor) {
+ Ibfd = ClockDivisorPair[ClockDivisorPairSize - 1].Ibfd;
+ } else {
+ for (Index = 0; Index < ClockDivisorPairSize; Index++) {
+ if (ClockDivisorPair[Index].Divisor >= ClockDivisor) {
+ Ibfd = ClockDivisorPair[Index].Ibfd;
+ break;
+ }
+ }
+ }
+
+ MmioWrite8 ((UINTN)&Regs->Ibfd, Ibfd);
+
+ I2cReset (Base);
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+I2cBusTestBusBusy (
+ IN I2C_REGS *Regs,
+ IN BOOLEAN TestBusy
+ )
+{
+ UINT32 Index;
+ UINT8 Reg;
+
+ for (Index = 0; Index < I2C_NUM_RETRIES; Index++) {
+ Reg = MmioRead8 ((UINTN)&Regs->Ibsr);
+
+ if (Reg & I2C_IBSR_IBAL) {
+ MmioWrite8 ((UINTN)&Regs->Ibsr, Reg);
+ return EFI_NOT_READY;
+ }
+
+ if (TestBusy && (Reg & I2C_IBSR_IBB)) {
+ break;
+ }
+
+ if (!TestBusy && !(Reg & I2C_IBSR_IBB)) {
+ break;
+ }
+
+ MicroSecondDelay (1);
+ }
+
+ if (Index == I2C_NUM_RETRIES) {
+ return EFI_TIMEOUT;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+I2cTransferComplete (
+ IN I2C_REGS *Regs,
+ IN BOOLEAN TestRxAck
+)
+{
+ UINT32 Index;
+ UINT8 Reg;
+
+ for (Index = 0; Index < I2C_NUM_RETRIES; Index++) {
+ Reg = MmioRead8 ((UINTN)&Regs->Ibsr);
+
+ if (Reg & I2C_IBSR_IBIF) {
+ // Write 1 to clear the IBIF field
+ MmioWrite8 ((UINTN)&Regs->Ibsr, Reg);
+ break;
+ }
+
+ MicroSecondDelay (1);
+ }
+
+ if (Index == I2C_NUM_RETRIES) {
+ return EFI_TIMEOUT;
+ }
+
+ if (TestRxAck && (Reg & I2C_IBSR_RXAK)) {
+ return EFI_NO_RESPONSE;
+ }
+
+ if (Reg & I2C_IBSR_TCF) {
+ return EFI_SUCCESS;
+ }
+
+ return EFI_DEVICE_ERROR;
+}
+
+STATIC
+EFI_STATUS
+I2cRead (
+ IN I2C_REGS *Regs,
+ IN UINT32 SlaveAddress,
+ IN EFI_I2C_OPERATION *Operation,
+ IN BOOLEAN IsLastOperation
+)
+{
+ EFI_STATUS Status;
+ UINTN Index;
+
+ // Write Slave Address
+ MmioWrite8 ((UINTN)&Regs->Ibdr, (SlaveAddress << BIT0) | BIT0);
+ Status = I2cTransferComplete (Regs, I2C_BUS_TEST_RX_ACK);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ // select Receive mode.
+ MmioAnd8 ((UINTN)&Regs->Ibcr, ~I2C_IBCR_TXRX);
+ if (Operation->LengthInBytes > 1) {
+ // Set No ACK = 0
+ MmioAnd8 ((UINTN)&Regs->Ibcr, ~I2C_IBCR_NOACK);
+ }
+
+ // Perform a dummy read to initiate the receive operation.
+ MmioRead8 ((UINTN)&Regs->Ibdr);
+
+ for (Index = 0; Index < Operation->LengthInBytes; Index++) {
+ Status = I2cTransferComplete (Regs, I2C_BUS_NO_TEST_RX_ACK);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ if (Index == (Operation->LengthInBytes - 2)) {
+ // Set No ACK = 1
+ MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_NOACK);
+ } else if (Index == (Operation->LengthInBytes - 1)) {
+ if (!IsLastOperation) {
+ // select Transmit mode (for repeat start)
+ MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_TXRX);
+ } else {
+ // Generate Stop Signal
+ MmioAnd8 ((UINTN)&Regs->Ibcr, ~(I2C_IBCR_MSSL | I2C_IBCR_TXRX));
+ Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_IDLE);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+ }
+ Operation->Buffer[Index] = MmioRead8 ((UINTN)&Regs->Ibdr);
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+I2cWrite (
+ IN I2C_REGS *Regs,
+ IN UINT32 SlaveAddress,
+ IN EFI_I2C_OPERATION *Operation
+)
+{
+ EFI_STATUS Status;
+ UINTN Index;
+
+ // Write Slave Address
+ MmioWrite8 ((UINTN)&Regs->Ibdr, (SlaveAddress << BIT0) & (UINT8)(~BIT0));
+ Status = I2cTransferComplete (Regs, I2C_BUS_TEST_RX_ACK);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ // Write Data
+ for (Index = 0; Index < Operation->LengthInBytes; Index++) {
+ MmioWrite8 ((UINTN)&Regs->Ibdr, Operation->Buffer[Index]);
+ Status = I2cTransferComplete (Regs, I2C_BUS_TEST_RX_ACK);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+I2cStop (
+ IN I2C_REGS *Regs
+ )
+{
+ EFI_STATUS Status;
+ UINT8 Reg;
+
+ Status = EFI_SUCCESS;
+ Reg = MmioRead8 ((UINTN)&Regs->Ibsr);
+ if (Reg & I2C_IBSR_IBB) {
+ // Generate Stop Signal
+ MmioAnd8 ((UINTN)&Regs->Ibcr, ~(I2C_IBCR_MSSL | I2C_IBCR_TXRX));
+ Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_IDLE);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+
+ // Disable I2c Controller
+ MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_MDIS);
+
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+I2cStart (
+ IN I2C_REGS *Regs
+ )
+{
+ EFI_STATUS Status;
+
+ MmioOr8 ((UINTN)&Regs->Ibsr, (I2C_IBSR_IBAL | I2C_IBSR_IBIF));
+ MmioAnd8 ((UINTN)&Regs->Ibcr, (UINT8)(~I2C_IBCR_MDIS));
+
+ //Wait controller to be stable
+ MicroSecondDelay (1);
+
+ // Generate Start Signal
+ MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_MSSL);
+ Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_BUSY);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ // Select Transmit Mode. set No ACK = 1
+ MmioOr8 ((UINTN)&Regs->Ibcr, (I2C_IBCR_TXRX | I2C_IBCR_NOACK));
+
+ return Status;
+}
+
+/**
+ Transfer data to/from I2c slave device
+
+ @param[in] Base Base Address of I2c controller's registers
+ @param[in] SlaveAddress Slave Address from which data is to be read
+ @param[in] RequestPacket Pointer to an EFI_I2C_REQUEST_PACKET structure
+ describing the I2C transaction
+
+ @return EFI_SUCCESS successfuly transfer the data
+ @return EFI_DEVICE_ERROR There was an error while transferring data through
+ I2c bus
+ @return EFI_NO_RESPONSE There was no Ack from i2c device
+ @return EFI_TIMEOUT I2c Bus is busy
+ @return EFI_NOT_READY I2c Bus Arbitration lost
+**/
+EFI_STATUS
+I2cBusXfer (
+ IN UINTN Base,
+ IN UINT32 SlaveAddress,
+ IN EFI_I2C_REQUEST_PACKET *RequestPacket
+ )
+{
+ UINTN Index;
+ I2C_REGS *Regs;
+ EFI_I2C_OPERATION *Operation;
+ EFI_STATUS Status;
+ BOOLEAN IsLastOperation;
+
+ Regs = (I2C_REGS *)Base;
+ IsLastOperation = FALSE;
+
+ Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_IDLE);
+ if (EFI_ERROR (Status)) {
+ goto ErrorExit;
+ }
+
+ Status = I2cStart (Regs);
+ if (EFI_ERROR (Status)) {
+ goto ErrorExit;
+ }
+
+ for (Index = 0, Operation = RequestPacket->Operation;
+ Index < RequestPacket->OperationCount;
+ Index++, Operation++) {
+ if (Index == (RequestPacket->OperationCount - 1)) {
+ IsLastOperation = TRUE;
+ }
+ // Send repeat start after first transmit/recieve
+ if (Index) {
+ MmioOr8 ((UINTN)&Regs->Ibcr, I2C_IBCR_RSTA);
+ Status = I2cBusTestBusBusy (Regs, I2C_BUS_TEST_BUSY);
+ if (EFI_ERROR (Status)) {
+ goto ErrorExit;
+ }
+ }
+ // Read/write data
+ if (Operation->Flags & I2C_FLAG_READ) {
+ Status = I2cRead (Regs, SlaveAddress, Operation, IsLastOperation);
+ } else {
+ Status = I2cWrite (Regs, SlaveAddress, Operation);
+ }
+ if (EFI_ERROR (Status)) {
+ goto ErrorExit;
+ }
+ }
+
+ErrorExit:
+
+ I2cStop (Regs);
+
+ return Status;
+}
+
+/**
+ Read a register from I2c slave device. This API is wrapper around I2cBusXfer
+
+ @param[in] Base Base Address of I2c controller's registers
+ @param[in] SlaveAddress Slave Address from which register value is
+ to be read
+ @param[in] RegAddress Register Address in Slave's memory map
+ @param[in] RegAddressWidthInBytes Number of bytes in RegAddress to send to
+ I2c Slave for simple reads without any
+ register, make this value = 0
+ (RegAddress is don't care in that case)
+ @param[out] RegValue Value to be read from I2c slave's regiser
+ @param[in] RegValueNumBytes Number of bytes to read from I2c slave
+ register
+
+ @return EFI_SUCCESS successfuly read the registers
+ @return EFI_DEVICE_ERROR There was an error while transferring data through
+ I2c bus
+ @return EFI_NO_RESPONSE There was no Ack from i2c device
+ @return EFI_TIMEOUT I2c Bus is busy
+ @return EFI_NOT_READY I2c Bus Arbitration lost
+**/
+EFI_STATUS
+I2cBusReadReg (
+ IN UINTN Base,
+ IN UINT32 SlaveAddress,
+ IN UINT64 RegAddress,
+ IN UINT8 RegAddressWidthInBytes,
+ OUT UINT8 *RegValue,
+ IN UINT32 RegValueNumBytes
+ )
+{
+ EFI_I2C_OPERATION *Operations;
+ I2C_REG_REQUEST RequestPacket;
+ UINTN OperationCount;
+ UINT8 Address[sizeof (RegAddress)];
+ UINT8 *AddressPtr;
+ EFI_STATUS Status;
+
+ ZeroMem (&RequestPacket, sizeof (RequestPacket));
+ OperationCount = 0;
+ Operations = RequestPacket.Operation;
+ AddressPtr = Address;
+
+ if (RegAddressWidthInBytes > ARRAY_SIZE (Address)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (RegAddressWidthInBytes != 0) {
+ Operations[OperationCount].LengthInBytes = RegAddressWidthInBytes;
+ Operations[OperationCount].Buffer = AddressPtr;
+ while (RegAddressWidthInBytes--) {
+ *AddressPtr++ = RegAddress >> (8 * RegAddressWidthInBytes);
+ }
+ OperationCount++;
+ }
+
+ Operations[OperationCount].LengthInBytes = RegValueNumBytes;
+ Operations[OperationCount].Buffer = RegValue;
+ Operations[OperationCount].Flags = I2C_FLAG_READ;
+ OperationCount++;
+
+ RequestPacket.OperationCount = OperationCount;
+
+ Status = I2cBusXfer (
+ Base, SlaveAddress,
+ (EFI_I2C_REQUEST_PACKET *)&RequestPacket
+ );
+
+ return Status;
+}
--
2.17.1


[PATCH edk2-platforms 5/5] Platform/RaspberryPi: create DXE phase SerialPortLib version for RPi3

Ard Biesheuvel
 

The Raspberry Pi 3 derives its 16550 baud clock from the variable core
clock, and so any reprogramming of the baud rate needs to take the
actual core clock value into account.

Introduce a DXE phase version of DualSerialPortLib that discovers this
value in its constructor, using the RPi firmware protocol, and wire it
up for the RPi3 platform.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Platform/RaspberryPi/RPi3/RPi3.dsc | 2 +-
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf | 67 ++++++++++++++++++++
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLibConstructor.c | 40 ++++++++++++
3 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 96b27400eef8..2b8ad1c4bdbd 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -576,7 +576,7 @@ [Components.common]
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {
<LibraryClasses>
- SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf
+ SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf
}
Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf
new file mode 100644
index 000000000000..4c22b39daa7f
--- /dev/null
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf
@@ -0,0 +1,67 @@
+## @file
+#
+# DXE phase SerialPortLib instance for both PL011 and 16550 UART.
+#
+# Copyright (c) 2020, Pete Batard <pete@akeo.ie>
+# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 1.27
+ BASE_NAME = DualSerialPortDxeLib
+ FILE_GUID = d586667e-ec50-4bf6-9701-fb4e29055a60
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = SerialPortLib|DXE_DRIVER
+ CONSTRUCTOR = DualSerialPortDxeLibConstructor
+
+[Packages]
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/RaspberryPi/RaspberryPi.dec
+ Silicon/Broadcom/Bcm283x/Bcm283x.dec
+
+[LibraryClasses]
+ IoLib
+ PcdLib
+ PL011UartClockLib
+ PL011UartLib
+
+[Sources]
+ DualSerialPortLib.c
+ DualSerialPortLib.h
+ DualSerialPortLibCommon.c
+ DualSerialPortLibConstructor.c
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterAccessWidth ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialDetectCable ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialLineControl ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialPciDeviceInfo ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride ## CONSUMES
+
+[FixedPcd]
+ gArmPlatformTokenSpaceGuid.PL011UartClkInHz
+ gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
+
+[Protocols]
+ gRaspberryPiFirmwareProtocolGuid ## CONSUMES
+
+[PatchPcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate ## CONSUMES
+
+[Depex]
+ gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLibConstructor.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLibConstructor.c
new file mode 100644
index 000000000000..c6d695181ab7
--- /dev/null
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLibConstructor.c
@@ -0,0 +1,40 @@
+/** @file
+
+ Copyright (c) 2020, ARM Ltd. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <IndustryStandard/RpiMbox.h>
+#include <Library/SerialPortLib.h>
+#include <Library/PcdLib.h>
+#include <Protocol/RpiFirmware.h>
+
+EFI_STATUS
+EFIAPI
+DualSerialPortDxeLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ RASPBERRY_PI_FIRMWARE_PROTOCOL *Firmware;
+ UINT32 ClockRate;
+ EFI_STATUS Status;
+
+ Status = SystemTable->BootServices->LocateProtocol (
+ &gRaspberryPiFirmwareProtocolGuid,
+ NULL, (VOID **)&Firmware);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = Firmware->GetClockRate (RPI_MBOX_CLOCK_RATE_CORE, &ClockRate);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ PatchPcdSet32 (PcdSerialClockRate, ClockRate);
+ return EFI_SUCCESS;
+}
--
2.17.1


[PATCH edk2-platforms 4/5] Platform/RaspberryPi3: query firmware for 16550 input clock at boot

Ard Biesheuvel
 

Query the firmware for the clock rate that is used to drive the
16550 baud clock, so that we can program the correct baud rate.

Co-authored-by: Pete Batard <pete@akeo.ie>
Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
Co-authored-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Pete Batard <pete@akeo.ie>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 25 +++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 91dfe1bb981e..35580e4ed73a 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -3,7 +3,7 @@
* Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
* Copyright (c) 2019-2020, Pete Batard <pete@akeo.ie>
* Copyright (c) 2016, Linaro Limited. All rights reserved.
- * Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+ * Copyright (c) 2011-2020, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -85,6 +85,14 @@ ASM_FUNC (ArmPlatformPeiBootAction)
adr x2, mBoardRevision
str w0, [x2]

+#if (RPI_MODEL == 3)
+ run .Lclkinfo_buffer
+
+ ldr w0, .Lfrequency
+ adrp x2, _gPcd_BinaryPatch_PcdSerialClockRate
+ str w0, [x2, :lo12:_gPcd_BinaryPatch_PcdSerialClockRate]
+#endif
+
ret

.align 4
@@ -127,6 +135,21 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // end tag
.set .Lrevinfo_size, . - .Lrevinfo_buffer

+#if (RPI_MODEL == 3)
+ .align 4
+.Lclkinfo_buffer:
+ .long .Lclkinfo_size
+ .long 0x0
+ .long RPI_MBOX_GET_CLOCK_RATE
+ .long 8 // buf size
+ .long 4 // input len
+ .long 4 // clock id: 0x04 = Core/VPU
+.Lfrequency:
+ .long 0 // frequency
+ .long 0 // end tag
+ .set .Lclkinfo_size, . - .Lclkinfo_buffer
+#endif
+
//UINTN
//ArmPlatformGetPrimaryCoreMpId (
// VOID
--
2.17.1


[PATCH edk2-platforms 3/5] Platform/RaspberryPi: fix 16550 divisor calculation logic

Ard Biesheuvel
 

The 16550 'miniUART' on the Raspberry Pi gets its input clock from
different sources on RPi3 and RPi3. Fix the logic that derives the
divisor for the 16550 baud clock on the respective platforms.

While at it, make the input clock PCD patchable for RPi3 so we can
manipulate it at runtime in a future patch.

Co-authored-by: Pete Batard <pete@akeo.ie>
Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
Co-authored-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Pete Batard <pete@akeo.ie>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +++-
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 14 ++++++++------
3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index d7218219fc5a..96b27400eef8 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -409,7 +409,6 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
@@ -441,6 +440,9 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE

+[PcdsPatchableInModule]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|250000000
+
[PcdsDynamicHii.common.DEFAULT]

#
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 4fb015b077e6..5d8bd88d7e34 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -420,7 +420,7 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index b1d17d3fa04a..5e83bbf022eb 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -41,18 +41,20 @@ SerialPortGetDivisor (
// On the Raspberry Pi, the clock to use for the 16650-compatible UART
// is the base clock divided by the 12.12 fixed point VPU clock divisor.
//
- BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate) * 4;
+ BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);
+#if (RPI_MODEL == 4)
Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;
if (Divisor != 0)
BaseClockRate = (BaseClockRate << 12) / Divisor;
+#endif

//
- // Now calculate divisor for baud generator
- // Ref_Clk_Rate / Baud_Rate / 16
+ // As per the BCM2xxx datasheets:
+ // baudrate = system_clock_freq / (8 * (divisor + 1)).
//
- Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 16);
- if (((UINT32)BaseClockRate % (SerialBaudRate * 16)) >= SerialBaudRate * 8) {
- Divisor++;
+ Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);
+ if (Divisor != 0) {
+ Divisor--;
}
return Divisor;
}
--
2.17.1