Sorry for late update.
> -----Original Message-----
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Friday, June 7, 2019 4:48 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; Carsey, Jaben <jaben.car...@intel.com>; Ni,
> Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>;
> matteo.carl...@arm.com; stephanie.hughes-f...@arm.com; n...@arm.com
> Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser
>
> The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field as part
> of the GICC structure.
>
> Update the MADT parser to decode this field and validate the interrupt ID
> used.
>
> References:
> - ACPI 6.3 Specification - January 2019
> - Arm Generic Interrupt Controller Architecture Specification,
> GIC architecture version 3 and version 4, issue E
> - Arm Server Base System Architecture 5.0
>
> Signed-off-by: Krzysztof Koch <krzysztof.k...@arm.com>
> ---
>
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2
>
> Notes:
> v2:
> - Add include sandwich in MadtParser.h [Zhichao]
> - Add references to specifications in commit message [Zhichao]
>
> v1:
> - Decode and validate SPE Overflow Interrupt field [Krzysztof]
>
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 86 ++++++++++++++++++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> h | 40 +++++++++
> 2 files changed, 118 insertions(+), 8 deletions(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> c63f1e4ab866f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.c
> @@ -1,17 +1,21 @@
> /** @file
> MADT table parser
>
> - Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> + Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @par Reference(s):
> - - ACPI 6.2 Specification - Errata A, September 2017
> + - ACPI 6.3 Specification - January 2019
> + - Arm Generic Interrupt Controller Architecture Specification,
> + GIC architecture version 3 and version 4, issue E
> + - Arm Server Base System Architecture 5.0
> **/
>
> #include <IndustryStandard/Acpi.h>
> #include <Library/UefiLib.h>
> #include "AcpiParser.h"
> #include "AcpiTableParser.h"
> +#include "MadtParser.h"
>
> // Local Variables
> STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@
> ValidateGICDSystemVectorBase (
> IN VOID* Context
> );
>
> +/**
> + This function validates the SPE Overflow Interrupt in the GICC.
> +
> + @param [in] Ptr Pointer to the start of the field data.
> + @param [in] Context Pointer to context specific information e.g. this
> + could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> + IN UINT8* Ptr,
> + IN VOID* Context
> + );
> +
> /**
> An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
> **/
> @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
> {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
> {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
> NULL},
> - {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
> + {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL}, {L"SPE
> + overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
> + ValidateSpeOverflowInterrupt, NULL}
> };
>
> /**
> @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
> }
> }
>
> +/**
> + This function validates the SPE Overflow Interrupt in the GICC.
> +
> + @param [in] Ptr Pointer to the start of the field data.
> + @param [in] Context Pointer to context specific information e.g. this
> + could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> + IN UINT8* Ptr,
> + IN VOID* Context
> + )
> +{
> + UINT16 SpeOverflowInterrupt;
> +
> + SpeOverflowInterrupt = *(UINT16*)Ptr;
> +
> + // SPE not supported by this processor if (SpeOverflowInterrupt ==
> + 0) {
> + return;
> + }
> +
> + if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
> + ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
> + (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
> + (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
> + IncrementErrorCount ();
> + Print (
> + L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI
> ID "
> + L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
> + SpeOverflowInterrupt,
> + ARM_PPI_ID_MIN,
> + ARM_PPI_ID_MAX,
> + ARM_PPI_ID_EXTENDED_MIN,
> + ARM_PPI_ID_EXTENDED_MAX
> + );
> + } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
> + IncrementWarningCount();
> + Print (
> + L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with
> SBSA "
> + L"Level 3 PPI ID assignment: %d.",
> + SpeOverflowInterrupt,
> + ARM_PPI_ID_PMBIRQ
> + );
> + }
> +}
> +
The checking values are named with ARM prefix. Can I think they are only for
ARM arch but not for IA32/X64?
If so, it is better to distinguish them. I used to view the MACRO MDE_CPU_ARM
and MDE_CPU_AARCH64. Did they work for this purpose?
There would always be a warning if the SpeOverflowInterrupt is unequal to
ARM_PPI_ID_PMBIRQ. Is that expected?
> /**
> This function parses the ACPI MADT table.
> When trace is enabled this function parses the MADT table and @@ -233,7
> +303,7 @@ ParseAcpiMadt (
> }
>
> switch (*MadtInterruptControllerType) {
> - case EFI_ACPI_6_2_GIC: {
> + case EFI_ACPI_6_3_GIC: {
> ParseAcpi (
> TRUE,
> 2,
> @@ -245,7 +315,7 @@ ParseAcpiMadt (
> break;
> }
>
> - case EFI_ACPI_6_2_GICD: {
> + case EFI_ACPI_6_3_GICD: {
> if (++GICDCount > 1) {
> IncrementErrorCount ();
> Print (
> @@ -265,7 +335,7 @@ ParseAcpiMadt (
> break;
> }
>
> - case EFI_ACPI_6_2_GIC_MSI_FRAME: {
> + case EFI_ACPI_6_3_GIC_MSI_FRAME: {
> ParseAcpi (
> TRUE,
> 2,
> @@ -277,7 +347,7 @@ ParseAcpiMadt (
> break;
> }
>
> - case EFI_ACPI_6_2_GICR: {
> + case EFI_ACPI_6_3_GICR: {
> ParseAcpi (
> TRUE,
> 2,
> @@ -289,7 +359,7 @@ ParseAcpiMadt (
> break;
> }
>
> - case EFI_ACPI_6_2_GIC_ITS: {
> + case EFI_ACPI_6_3_GIC_ITS: {
> ParseAcpi (
> TRUE,
> 2,
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.h
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..fbbc43e09adbdf9fea302a03a6
> 1e6dc179f06a62
> --- /dev/null
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.h
> @@ -0,0 +1,40 @@
> +/** @file
> + Header file for MADT table parser
> +
> + Copyright (c) 2019, 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 **/
> +
> +#ifndef MADT_PARSER_H_
> +#define MADT_PARSER_H_
> +
> +///
> +/// Level 3 base server system Private Peripheral Inerrupt (PPI) ID
> +assignments ///
> +#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTP 30
> +#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTPS 29
> +#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTHV 28
> +#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTV 27
> +#define ARM_PPI_ID_OVERFLOW_INTERRUPT_FROM_CNTHP 26
> +#define ARM_PPI_ID_GIC_MAINTENANCE_INTERRUPT 25
> +#define ARM_PPI_ID_CTIIRQ 24
> +#define ARM_PPI_ID_PERFORMANCE_MONITORS_INTERRUPT 23
> +#define ARM_PPI_ID_COMMIRQ 22
> +#define ARM_PPI_ID_PMBIRQ 21
> +#define ARM_PPI_ID_CNTHPS 20
> +#define ARM_PPI_ID_CNTHVS 19
> +
> +///
> +/// PPI ID allowed ranges
> +///
> +#define ARM_PPI_ID_MAX 31
> +#define ARM_PPI_ID_MIN 16
> +#define ARM_PPI_ID_EXTENDED_MAX 1119
> +#define ARM_PPI_ID_EXTENDED_MIN 1056
I can't find the info about "ARM_PPI_ID_EXTENDED_MAX 1119". Can you help to
clarify it? Is this for future SBSA usage.
I am not familiar with the ARM things. Someone is export in ARM could help to
review.
Thanks,
Zhichao
> +
> +#endif // MADT_PARSER_H_
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#42048): https://edk2.groups.io/g/devel/message/42048
Mute This Topic: https://groups.io/mt/31972729/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
