Re: [edk2-devel] [PATCH v3 2/8] DynamicTablesPkg: FdtHwInfoParserLib: Parse Pmu info

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 28 Jan 2022 at 16:41, PierreGondois <pierre.gondois@arm.com> wrote:
>
> From: Pierre Gondois <Pierre.Gondois@arm.com>
>
> Parse the Pmu interrupts if a pmu compatible node is present,
> and populate the MADT GicC structure accordingly.
>
> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
> ---
>
> Notes:
>     v3:
>      - New patch. [Pierre]
>
>  .../FdtHwInfoParserLib/Gic/ArmGicCParser.c    | 131 +++++++++++++++++-
>  .../FdtHwInfoParserLib/Gic/ArmGicCParser.h    |   8 +-
>  2 files changed, 135 insertions(+), 4 deletions(-)
>
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> index b4e6729a4ab2..961607378449 100644
> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.c
> @@ -1,13 +1,14 @@
>  /** @file
>    Arm Gic cpu parser.
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>    @par Reference(s):
>    - linux/Documentation/devicetree/bindings/arm/cpus.yaml
>    - linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
>    - linux/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
> +  - linux/Documentation/devicetree/bindings/arm/pmu.yaml
>  **/
>
>  #include "FdtHwInfoParser.h"
> @@ -34,6 +35,21 @@ STATIC CONST COMPATIBILITY_INFO  CpuCompatibleInfo = {
>    CpuCompatibleStr
>  };
>
> +/** Pmu compatible strings.
> +
> +  Any other "compatible" value is not supported by this module.
> +*/
> +STATIC CONST COMPATIBILITY_STR  PmuCompatibleStr[] = {
> +  { "arm,armv8-pmuv3" }
> +};
> +
> +/** COMPATIBILITY_INFO structure for the PmuCompatibleStr.
> +*/
> +CONST COMPATIBILITY_INFO  PmuCompatibleInfo = {
> +  ARRAY_SIZE (PmuCompatibleStr),
> +  PmuCompatibleStr
> +};
> +
>  /** Parse a "cpu" node.
>
>    @param [in]  Fdt              Pointer to a Flattened Device Tree (Fdt).
> @@ -639,6 +655,110 @@ GicCv3IntcNodeParser (
>    return EFI_SUCCESS;
>  }
>
> +/** Parse a Pmu compatible node, extracting Pmu information.
> +
> +  This function modifies a CM_OBJ_DESCRIPTOR object.
> +  The following CM_ARM_GICC_INFO fields are patched:
> +    - PerformanceInterruptGsiv;
> +
> +  @param [in]       Fdt              Pointer to a Flattened Device Tree (Fdt).
> +  @param [in]       GicIntcNode      Offset of a Gic compatible
> +                                     interrupt-controller node.
> +  @param [in, out]  GicCCmObjDesc    The CM_ARM_GICC_INFO to patch.
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_ABORTED             An error occurred.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GicCPmuNodeParser (
> +  IN      CONST VOID               *Fdt,
> +  IN            INT32              GicIntcNode,
> +  IN  OUT       CM_OBJ_DESCRIPTOR  *GicCCmObjDesc
> +  )
> +{
> +  EFI_STATUS        Status;
> +  INT32             IntCells;
> +  INT32             PmuNode;
> +  UINT32            PmuNodeCount;
> +  UINT32            PmuIrq;
> +  UINT32            Index;
> +  CM_ARM_GICC_INFO  *GicCInfo;
> +  CONST UINT8       *Data;
> +  INT32             DataSize;
> +
> +  if (GicCCmObjDesc == NULL) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  GicCInfo = (CM_ARM_GICC_INFO *)GicCCmObjDesc->Data;
> +  PmuNode  = 0;
> +
> +  // Count the number of pmu nodes.
> +  Status = FdtCountCompatNodeInBranch (
> +             Fdt,
> +             0,
> +             &PmuCompatibleInfo,
> +             &PmuNodeCount
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);

If you use ASSERT_EFI_ERROR() here, you at least get the type of error
as DEBUG output, whereas ASSERT(0) does not give any context
whatsoever.

> +    return Status;
> +  }
> +
> +  if (PmuNodeCount == 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Status = FdtGetNextCompatNodeInBranch (
> +             Fdt,
> +             0,
> +             &PmuCompatibleInfo,
> +             &PmuNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);

Same here

> +    if (Status == EFI_NOT_FOUND) {
> +      // Should have found the node.
> +      Status = EFI_ABORTED;
> +    }
> +  }
> +
> +  // Get the number of cells used to encode an interrupt.
> +  Status = FdtGetInterruptCellsInfo (Fdt, GicIntcNode, &IntCells);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);

and here

> +    return Status;
> +  }
> +
> +  Data = fdt_getprop (Fdt, PmuNode, "interrupts", &DataSize);
> +  if ((Data == NULL) || (DataSize != (IntCells * sizeof (UINT32)))) {
> +    // If error or not 1 interrupt.
> +    ASSERT (0);
> +    return EFI_ABORTED;
> +  }
> +
> +  PmuIrq = FdtGetInterruptId ((CONST UINT32 *)Data);
> +
> +  // Only supports PPI 23 for now.
> +  // According to BSA 1.0 s3.6 PPI assignments, PMU IRQ ID is 23. A non BSA
> +  // compliant system may assign a different IRQ for the PMU, however this
> +  // is not implemented for now.
> +  if (PmuIrq != BSA_PMU_IRQ) {
> +    ASSERT (0);
> +    return EFI_ABORTED;
> +  }
> +
> +  for (Index = 0; Index < GicCCmObjDesc->Count; Index++) {
> +    GicCInfo[Index].PerformanceInterruptGsiv = PmuIrq;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /** CM_ARM_GICC_INFO parser function.
>
>    This parser expects FdtBranch to be the "\cpus" node node.
> @@ -649,7 +769,7 @@ GicCv3IntcNodeParser (
>      UINT32  AcpiProcessorUid;                 // {Populated}
>      UINT32  Flags;                            // {Populated}
>      UINT32  ParkingProtocolVersion;           // {default = 0}
> -    UINT32  PerformanceInterruptGsiv;         // {default = 0}
> +    UINT32  PerformanceInterruptGsiv;         // {Populated}
>      UINT64  ParkedAddress;                    // {default = 0}
>      UINT64  PhysicalBaseAddress;              // {Populated}
>      UINT64  GICV;                             // {Populated}
> @@ -764,6 +884,13 @@ ArmGicCInfoParser (
>      goto exit_handler;
>    }
>
> +  // Parse the Pmu Interrupt.
> +  Status = GicCPmuNodeParser (Fdt, IntcNode, NewCmObjDesc);
> +  if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
> +    ASSERT (0);

and here

> +    goto exit_handler;

shouldn't this be CamelCase?

> +  }
> +
>    // Add all the CmObjs to the Configuration Manager.
>    Status = AddMultipleCmObj (FdtParserHandle, NewCmObjDesc, 0, NULL);
>    if (EFI_ERROR (Status)) {
> diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
> index 2a0f966bf0c2..fd980484a28d 100644
> --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
> +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Gic/ArmGicCParser.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Arm Gic cpu parser.
>
> -  Copyright (c) 2021, ARM Limited. All rights reserved.<BR>
> +  Copyright (c) 2021 - 2022, Arm Limited. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
>    @par Reference(s):
> @@ -12,6 +12,10 @@
>  #ifndef ARM_GICC_PARSER_H_
>  #define ARM_GICC_PARSER_H_
>
> +/* According to BSA 1.0 s3.6 PPI assignments, PMU IRQ ID is 23.
> +*/
> +#define BSA_PMU_IRQ  23
> +
>  /** CM_ARM_GICC_INFO parser function.
>
>    This parser expects FdtBranch to be the "\cpus" node node.
> @@ -22,7 +26,7 @@
>      UINT32  AcpiProcessorUid;                 // {Populated}
>      UINT32  Flags;                            // {Populated}
>      UINT32  ParkingProtocolVersion;           // {default = 0}
> -    UINT32  PerformanceInterruptGsiv;         // {default = 0}
> +    UINT32  PerformanceInterruptGsiv;         // {Populated}
>      UINT64  ParkedAddress;                    // {default = 0}
>      UINT64  PhysicalBaseAddress;              // {Populated}
>      UINT64  GICV;                             // {Populated}
> --
> 2.25.1
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86169): https://edk2.groups.io/g/devel/message/86169
> Mute This Topic: https://groups.io/mt/88746969/1131722
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb@kernel.org]
> ------------
>
>