Re: [PATCH edk2-platforms v2 5/8] Silicon/AMD/StyxDtbLoaderLib: add interrupt-affinity property to PMU node

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Thu, 28 Nov 2019 at 14:22, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Wed, Nov 27, 2019 at 19:44:36 +0100, Ard Biesheuvel wrote:
> > AMD Seattle uses a range of SPIs to signal PMU events, and this requires
> > a description in the DT which SPI maps to which CPU. This requires us to
> > defer the generation of the PMU node to a point where the CPU phandles
> > have been allocated.
>
> Ah, so these were previously populated with garbage? Oops.
>

No, not really. Moving the code obfuscates it a bit, but the new code
has an additional fdt_appendprop_cell() to set "interrupt-affinity"
which was missing entirely before.

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks,

> > ---
> >  Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c | 55 ++++++++++----------
> >  1 file changed, 28 insertions(+), 27 deletions(-)
> >
> > diff --git a/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c b/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c
> > index 261b5f59c8df..2f7b5e2a7b25 100644
> > --- a/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c
> > +++ b/Silicon/AMD/Styx/Library/StyxDtbLoaderLib/StyxDtbLoaderLib.c
> > @@ -331,33 +331,6 @@ PrepareFdt (
> >    // Get Id from primary CPU
> >    MpId = (UINTN)ArmReadMpidr ();
> >
> > -  // Create /pmu node
> > -  PmuNode = fdt_add_subnode(Fdt, 0, "pmu");
> > -  if (PmuNode >= 0) {
> > -    fdt_setprop_string (Fdt, PmuNode, "compatible", "arm,armv8-pmuv3");
> > -
> > -    // append PMU interrupts
> > -    for (Index = 0; Index < ArmCoreCount; Index++) {
> > -      MpId = (UINTN)GET_MPID (ArmCoreInfoTable[Index].ClusterId,
> > -                              ArmCoreInfoTable[Index].CoreId);
> > -
> > -      Status = AmdMpCoreInfoProtocol->GetPmuSpiFromMpId (MpId, &PmuInt.IntId);
> > -      if (EFI_ERROR (Status)) {
> > -        DEBUG ((DEBUG_ERROR,
> > -          "FDT: Error getting PMU interrupt for MpId '0x%x'\n", MpId));
> > -        return Status;
> > -      }
> > -
> > -      PmuInt.Flag = cpu_to_fdt32 (PMU_INT_FLAG_SPI);
> > -      PmuInt.IntId = cpu_to_fdt32 (PmuInt.IntId);
> > -      PmuInt.Type = cpu_to_fdt32 (PMU_INT_TYPE_HIGH_LEVEL);
> > -      fdt_appendprop (Fdt, PmuNode, "interrupts", &PmuInt, sizeof(PmuInt));
> > -    }
> > -  } else {
> > -    DEBUG ((DEBUG_ERROR, "FDT: Error creating 'pmu' node\n"));
> > -    return EFI_INVALID_PARAMETER;
> > -  }
> > -
> >    // Create /cpus noide
> >    Node = fdt_add_subnode (Fdt, 0, "cpus");
> >    if (Node >= 0) {
> > @@ -449,6 +422,34 @@ PrepareFdt (
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > +  // Create /pmu node
> > +  PmuNode = fdt_add_subnode(Fdt, 0, "pmu");
> > +  if (PmuNode >= 0) {
> > +    fdt_setprop_string (Fdt, PmuNode, "compatible", "arm,armv8-pmuv3");
> > +
> > +    // append PMU interrupts
> > +    for (Index = 0; Index < ArmCoreCount; Index++) {
> > +      MpId = (UINTN)GET_MPID (ArmCoreInfoTable[Index].ClusterId,
> > +                              ArmCoreInfoTable[Index].CoreId);
> > +
> > +      Status = AmdMpCoreInfoProtocol->GetPmuSpiFromMpId (MpId, &PmuInt.IntId);
> > +      if (EFI_ERROR (Status)) {
> > +        DEBUG ((DEBUG_ERROR,
> > +          "FDT: Error getting PMU interrupt for MpId '0x%x'\n", MpId));
> > +        return Status;
> > +      }
> > +
> > +      PmuInt.Flag = cpu_to_fdt32 (PMU_INT_FLAG_SPI);
> > +      PmuInt.IntId = cpu_to_fdt32 (PmuInt.IntId);
> > +      PmuInt.Type = cpu_to_fdt32 (PMU_INT_TYPE_HIGH_LEVEL);
> > +      fdt_appendprop (Fdt, PmuNode, "interrupts", &PmuInt, sizeof(PmuInt));
> > +      fdt_appendprop_cell (Fdt, PmuNode, "interrupt-affinity", Phandle[Index]);
> > +    }
> > +  } else {
> > +    DEBUG ((DEBUG_ERROR, "FDT: Error creating 'pmu' node\n"));
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> >    SetSocIdStatus (Fdt);
> >    SetXgbeStatus (Fdt);
> >
> > --
> > 2.17.1
> >