From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: zhichao.gao@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Sun, 09 Jun 2019 18:08:16 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jun 2019 18:08:15 -0700 X-ExtLoop1: 1 Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga007.fm.intel.com with ESMTP; 09 Jun 2019 18:08:15 -0700 Received: from fmsmsx161.amr.corp.intel.com (10.18.125.9) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 9 Jun 2019 18:08:15 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX161.amr.corp.intel.com (10.18.125.9) with Microsoft SMTP Server (TLS) id 14.3.408.0; Sun, 9 Jun 2019 18:08:15 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.10]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.188]) with mapi id 14.03.0415.000; Mon, 10 Jun 2019 09:08:12 +0800 From: "Gao, Zhichao" To: Krzysztof Koch , "devel@edk2.groups.io" CC: "Sami.Mujawar@arm.com" , "Carsey, Jaben" , "Ni, Ray" , "Matteo.Carlini@arm.com" , "Stephanie.Hughes-Fitt@arm.com" , "nd@arm.com" Subject: Re: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser Thread-Topic: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser Thread-Index: AQHVHQ28QfuD3sQkeEC+0r3UnREtd6aUEBSw Date: Mon, 10 Jun 2019 01:08:12 +0000 Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B7E5031@SHSMSX101.ccr.corp.intel.com> References: <20190607084741.19300-1-krzysztof.koch@arm.com> In-Reply-To: <20190607084741.19300-1-krzysztof.koch@arm.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: zhichao.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sorry for late update. > -----Original Message----- > From: Krzysztof Koch [mailto:krzysztof.koch@arm.com] > Sent: Friday, June 7, 2019 4:48 PM > To: devel@edk2.groups.io > Cc: Sami.Mujawar@arm.com; Carsey, Jaben ; Ni, > Ray ; Gao, Zhichao ; > Matteo.Carlini@arm.com; Stephanie.Hughes-Fitt@arm.com; nd@arm.com > Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT pars= er >=20 > The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field as= part > of the GICC structure. >=20 > Update the MADT parser to decode this field and validate the interrupt ID > used. >=20 > 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 >=20 > Signed-off-by: Krzysztof Koch > --- >=20 > Changes can be seen at: > https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2 >=20 > Notes: > v2: > - Add include sandwich in MadtParser.h [Zhichao] > - Add references to specifications in commit message [Zhichao] >=20 > v1: > - Decode and validate SPE Overflow Interrupt field [Krzysztof] >=20 >=20 > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser. > c | 86 ++++++++++++++++++-- > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser. > h | 40 +++++++++ > 2 files changed, 118 insertions(+), 8 deletions(-) >=20 > 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 >=20 > - 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 >=20 > @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 > **/ >=20 > #include > #include > #include "AcpiParser.h" > #include "AcpiTableParser.h" > +#include "MadtParser.h" >=20 > // Local Variables > STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@ > ValidateGICDSystemVectorBase ( > IN VOID* Context > ); >=20 > +/** > + 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 Structur= e. > **/ > @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] =3D { > {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} > }; >=20 > /** > @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase ( > } > } >=20 > +/** > + 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 =3D *(UINT16*)Ptr; > + > + // SPE not supported by this processor if (SpeOverflowInterrupt =3D= =3D > + 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 P= PI 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 !=3D 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 fo= r ARM arch but not for IA32/X64? If so, it is better to distinguish them. I used to view the MACRO MDE_CPU_A= RM and MDE_CPU_AARCH64. Did they work for this purpose? There would always be a warning if the SpeOverflowInterrupt is unequal to A= RM_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 ( > } >=20 > switch (*MadtInterruptControllerType) { > - case EFI_ACPI_6_2_GIC: { > + case EFI_ACPI_6_3_GIC: { > ParseAcpi ( > TRUE, > 2, > @@ -245,7 +315,7 @@ ParseAcpiMadt ( > break; > } >=20 > - case EFI_ACPI_6_2_GICD: { > + case EFI_ACPI_6_3_GICD: { > if (++GICDCount > 1) { > IncrementErrorCount (); > Print ( > @@ -265,7 +335,7 @@ ParseAcpiMadt ( > break; > } >=20 > - 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; > } >=20 > - case EFI_ACPI_6_2_GICR: { > + case EFI_ACPI_6_3_GICR: { > ParseAcpi ( > TRUE, > 2, > @@ -289,7 +359,7 @@ ParseAcpiMadt ( > break; > } >=20 > - 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 hel= p 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)' >=20