Is this for ARM only? > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Joey > Gouly > Sent: Friday, January 15, 2021 10:44 PM > To: devel@edk2.groups.io > Cc: joey.go...@arm.com; ardb+tianoc...@kernel.org; l...@nuviainc.com; > sami.muja...@arm.com; n...@arm.com > Subject: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is > present in MADT > > The ACPI 6.3 Specification, January 2019, section 5.2.12.14 states that > the firmware must convey each processor’s GIC information to the OS using > the GICC structure. > > If a GICC structure for the boot CPU is missing some standards-based > operating system may crash with an error code. It may be difficult to > diagnose the reason for the crash as the error code may be too generic > and mean firmware error. > > Therefore add validation to the MADT table parser to check that a GICC is > present for the boot CPU. > > Signed-off-by: Joey Gouly <joey.go...@arm.com> > --- > > Changes since v1: > - Added 'm' prefix for global variables > - Reordered ci yaml file > > The changes can be seen at > https://github.com/jgouly/edk2/tree/1474_validate_boot_cpu_mpidr_v2 > > ShellPkg/ShellPkg.dsc > | 3 ++ > > ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman > dLib.inf | 5 ++ > > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser. > c | 54 +++++++++++++++++++- > ShellPkg/ShellPkg.ci.yaml > | 2 + > 4 files changed, 62 insertions(+), 2 deletions(-) > > diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc > index > c42bc9464a0f7be111ee3086a664506c8288928c..661cc8b02b0971280c3649e0f2 > 9e109305bcc776 100644 > --- a/ShellPkg/ShellPkg.dsc > +++ b/ShellPkg/ShellPkg.dsc > @@ -71,6 +71,9 @@ [LibraryClasses.ARM,LibraryClasses.AARCH64] > # Add support for GCC stack protector > NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > + # Add support for reading MPIDR > + ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf > + > [PcdsFixedAtBuild] > gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF > gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|16000 > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > andLib.inf > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > andLib.inf > index > 947fb1f375667e12f8603e4264fef5e69cb98919..00e770d677ec1f2a23c1650fe2f > 4a94f2f86649f 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > andLib.inf > +++ > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm > andLib.inf > @@ -60,6 +60,9 @@ [Packages] > MdePkg/MdePkg.dec > ShellPkg/ShellPkg.dec > > +[Packages.ARM, Packages.AARCH64] > + ArmPkg/ArmPkg.dec > + > [LibraryClasses] > BaseLib > BaseMemoryLib > @@ -75,6 +78,8 @@ [LibraryClasses] > UefiLib > UefiRuntimeServicesTableLib > > +[LibraryClasses.ARM, LibraryClasses.AARCH64] > + ArmLib > > [FixedPcd] > gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars > er.c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars > er.c > index > 15aa2392b60cee9e3843c7c560b0ab84e0be4174..9935537aaee28381fecec08d0 > 057db83aaca1e1d 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars > er.c > +++ > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars > er.c > @@ -1,7 +1,7 @@ > /** @file > MADT table parser > > - Copyright (c) 2016 - 2020, ARM Limited. All rights reserved. > + Copyright (c) 2016 - 2020, Arm Limited. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > @par Reference(s): > @@ -13,6 +13,9 @@ > > #include <IndustryStandard/Acpi.h> > #include <Library/UefiLib.h> > +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64) > +#include <Library/ArmLib.h> > +#endif > #include "AcpiParser.h" > #include "AcpiTableParser.h" > #include "AcpiViewConfig.h" > @@ -23,6 +26,11 @@ STATIC CONST UINT8* MadtInterruptControllerType; > STATIC CONST UINT8* MadtInterruptControllerLength; > STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo; > > +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64) > +STATIC UINT64 mBootMpidr; > +STATIC BOOLEAN mHasBootMpidrGicc = FALSE; > +#endif > + > /** > This function validates the System Vector Base in the GICD. > > @@ -95,6 +103,33 @@ ValidateSpeOverflowInterrupt ( > } > } > > +/** > + This function validates that the GICC structure contains an entry for > + the Boot CPU. > + > + @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 > +ValidateBootMpidr ( > + IN UINT8* Ptr, > + IN VOID* Context > + ) > +{ > +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64) > + UINT64 CurrentMpidr; > + > + CurrentMpidr = *(UINT64*)Ptr; > + > + if (CurrentMpidr == mBootMpidr) { > + mHasBootMpidrGicc = TRUE; > + } > +#endif > +} > + > /** > An ACPI_PARSER array describing the GICC Interrupt Controller Structure. > **/ > @@ -115,7 +150,7 @@ STATIC CONST ACPI_PARSER GicCParser[] = { > {L"GICH", 8, 48, L"0x%lx", NULL, NULL, NULL, NULL}, > {L"VGIC Maintenance interrupt", 4, 56, L"0x%x", NULL, NULL, NULL, NULL}, > {L"GICR Base Address", 8, 60, L"0x%lx", NULL, NULL, NULL, NULL}, > - {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL}, > + {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, ValidateBootMpidr, NULL}, > {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL, > NULL}, > {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL}, > @@ -234,6 +269,11 @@ ParseAcpiMadt ( > UINT8* InterruptContollerPtr; > UINT32 GICDCount; > > +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64) > + mBootMpidr = ArmReadMpidr () & > + (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2 | > ARM_CORE_AFF3); > +#endif > + > GICDCount = 0; > > if (!Trace) { > @@ -371,4 +411,14 @@ ParseAcpiMadt ( > InterruptContollerPtr += *MadtInterruptControllerLength; > Offset += *MadtInterruptControllerLength; > } // while > + > +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64) > + if (!mHasBootMpidrGicc) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: No GICC present for Boot CPU (MPIDR: 0x%lx)", > + mBootMpidr > + ); > + } > +#endif > } > diff --git a/ShellPkg/ShellPkg.ci.yaml b/ShellPkg/ShellPkg.ci.yaml > index > 30894d44bc3ae9a2a9796146c5bcdc62d4ce9801..637c33adbcb2a84fb5bbfe09e > afffda61e78cc8d 100644 > --- a/ShellPkg/ShellPkg.ci.yaml > +++ b/ShellPkg/ShellPkg.ci.yaml > @@ -3,6 +3,7 @@ > # > # Copyright (c) Microsoft Corporation > # Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2020, Arm Limited. All rights reserved. > # SPDX-License-Identifier: BSD-2-Clause-Patent > ## > { > @@ -28,6 +29,7 @@ > }, > "DependencyCheck": { > "AcceptableDependencies": [ > + "ArmPkg/ArmPkg.dec", > "MdePkg/MdePkg.dec", > "MdeModulePkg/MdeModulePkg.dec", > "ShellPkg/ShellPkg.dec", > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72602): https://edk2.groups.io/g/devel/message/72602 Mute This Topic: https://groups.io/mt/79702862/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
