Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is present in MADT

2021-03-10 Thread Joey Gouly
> From: devel@edk2.groups.io  on behalf of Ni, Ray via 
> groups.io 
> Is this for ARM only?

Yes, the GIC interrupt controller is for Arm only.

Thanks,
Joey
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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72631): https://edk2.groups.io/g/devel/message/72631
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]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is present in MADT

2021-03-09 Thread Ni, Ray
Is this for ARM only?

> -Original Message-
> From: 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 
> ---
> 
> 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 
>  #include 
> +#if defined(MDE_CPU_ARM) || defined(MDE_CPU_AARCH64)
> +#include 
> +#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 

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: Validate that the Boot CPU is present in MADT

2021-03-09 Thread Joey Gouly
> From: Joey Gouly 
> Subject: [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.

Ping.

Is anyone able to review this patch adding some more validation checks to 
Acpiview / MADT?

Thanks,
Joey

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72594): https://edk2.groups.io/g/devel/message/72594
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]
-=-=-=-=-=-=-=-=-=-=-=-