Hi Sami, On Tue, Aug 21, 2018 at 11:14:43AM +0100, Sami Mujawar wrote: > Updated Redistributor base calculation to allow for the fact that > GICv4 has 2 additional 64KB frames (for VLPI and a reserved frame). > The code now tests the VLPIS bit in the GICR_TYPER register
Can you expand the name for this register (GIC Redistributor Type Register)? > and calculates the Redistributor granularity accordingly. > > The code changes are: > GICR_TYPER register fields, etc, added to the header. > Loop updated to pay attention to GICR_TYPER.Last. > Derive frame "stride" size from GICR_TYPER.VLPIS. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Sami Mujawar <sami.muja...@arm.com> > --- > The changes can be seen at: > https://github.com/samimujawar/edk2/tree/329_gicv4_granularity_v1 > > Notes: > v1: > - Added support for initializing GICv4 [SAMI] > > ArmPkg/Drivers/ArmGic/ArmGicLib.c | 37 ++++++++++++-------- > ArmPkg/Include/Library/ArmGicLib.h | 19 ++++++++-- Can you add a sort order file in your working tree (git config diff.orderFile) with the contents in Laszlo's unkempt guide? > 2 files changed, 39 insertions(+), 17 deletions(-) > > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c > b/ArmPkg/Drivers/ArmGic/ArmGicLib.c > index > 0087399fb1dba0e697f7a6ccd6f7432a59311ac6..033d98b75c1fc0414e7b70be1ca53d5c91cb6f3c > 100644 > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c > @@ -1,6 +1,6 @@ > /** @file > * > -* Copyright (c) 2011-2017, ARM Limited. All rights reserved. > +* Copyright (c) 2011-2018, ARM Limited. All rights reserved. > * > * This program and the accompanying materials > * are licensed and made available under the terms and conditions of the BSD > License > @@ -19,6 +19,16 @@ > #include <Library/IoLib.h> > #include <Library/PcdLib.h> > > +// In GICv3, there are 2 x 64KB frames: > +// Redistributor control frame + SGI Control & Generation frame > +#define GIC_V3_REDISTRIBUTOR_GRANULARITY (ARM_GICR_CTLR_FRAME_SIZE > \ > + + ARM_GICR_SGI_PPI_FRAME_SIZE) > + > +// In GICv4, there are 2 additional 64KB frames: > +// VLPI frame + Reserved page frame > +#define GIC_V4_REDISTRIBUTOR_GRANULARITY (GIC_V3_REDISTRIBUTOR_GRANULARITY > \ > + + ARM_GICR_SGI_VLPI_FRAME_SIZE > \ > + + > ARM_GICR_SGI_RESERVED_FRAME_SIZE) > > #define ISENABLER_ADDRESS(base,offset) ((base) + \ > ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * offset)) > @@ -54,12 +64,11 @@ GicGetCpuRedistributorBase ( > IN ARM_GIC_ARCH_REVISION Revision > ) > { > - UINTN Index; > UINTN MpId; > UINTN CpuAffinity; > UINTN Affinity; > - UINTN GicRedistributorGranularity; > UINTN GicCpuRedistributorBase; > + UINT64 GicRTyper; I know this is the official name of the register, but this is a local variable in a function specific to the redistributor, so could you change it to be just TypeRegister? > > MpId = ArmReadMpidr (); > // Define CPU affinity as: > @@ -68,27 +77,27 @@ GicGetCpuRedistributorBase ( > CpuAffinity = (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | ARM_CORE_AFF2)) | > ((MpId & ARM_CORE_AFF3) >> 8); > > - if (Revision == ARM_GIC_ARCH_REVISION_3) { > - // 2 x 64KB frame: > - // Redistributor control frame + SGI Control & Generation frame > - GicRedistributorGranularity = ARM_GICR_CTLR_FRAME_SIZE > - + ARM_GICR_SGI_PPI_FRAME_SIZE; > - } else { > + if (Revision < ARM_GIC_ARCH_REVISION_3) { > ASSERT_EFI_ERROR (EFI_UNSUPPORTED); > return 0; > } > > GicCpuRedistributorBase = GicRedistributorBase; > > - for (Index = 0; Index < PcdGet32 (PcdCoreCount); Index++) { > - Affinity = MmioRead64 (GicCpuRedistributorBase + ARM_GICR_TYPER) >> 32; > + do { > + GicRTyper = MmioRead64 (GicCpuRedistributorBase + ARM_GICR_TYPER); > + Affinity = GicRTyper >> 32; Could you add a macro for that >> 32? ARM_GICR_TYPER_AFFINITY(...) ? Hah, no, that's already used for the other way :) _GET_AFFINITY? > if (Affinity == CpuAffinity) { > return GicCpuRedistributorBase; > } > > - // Move to the next GIC Redistributor frame > - GicCpuRedistributorBase += GicRedistributorGranularity; > - } > + // Move to the next GIC Redistributor frame. > + // The GIC specification does not forbid a mixture of v3 and v4 frames, > + // so we test VLPIS for each frame. Could you expand VLPIS (Virtual LPIs Supported or somesuch)? "v3 and v4" frames ... is that a correct architectural description? / Leif > + GicCpuRedistributorBase += (((ARM_GICR_TYPER_VLPIS & GicRTyper) != 0) > + ? GIC_V4_REDISTRIBUTOR_GRANULARITY > + : GIC_V3_REDISTRIBUTOR_GRANULARITY); > + } while ((GicRTyper & ARM_GICR_TYPER_LAST) == 0); > > // The Redistributor has not been found for the current CPU > ASSERT_EFI_ERROR (EFI_NOT_FOUND); > diff --git a/ArmPkg/Include/Library/ArmGicLib.h > b/ArmPkg/Include/Library/ArmGicLib.h > index > 4b21ea9e4e76cb83c0c3421c1d9d88b456192687..4e3468648b255efa247dc4176ce4688986fcb226 > 100644 > --- a/ArmPkg/Include/Library/ArmGicLib.h > +++ b/ArmPkg/Include/Library/ArmGicLib.h > @@ -1,6 +1,6 @@ > /** @file > * > -* Copyright (c) 2011-2017, ARM Limited. All rights reserved. > +* Copyright (c) 2011-2018, ARM Limited. All rights reserved. > * > * This program and the accompanying materials > * are licensed and made available under the terms and conditions of the BSD > License > @@ -60,12 +60,25 @@ > > > // GIC Redistributor > -#define ARM_GICR_CTLR_FRAME_SIZE SIZE_64KB > -#define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB > +#define ARM_GICR_CTLR_FRAME_SIZE SIZE_64KB > +#define ARM_GICR_SGI_PPI_FRAME_SIZE SIZE_64KB > +#define ARM_GICR_SGI_VLPI_FRAME_SIZE SIZE_64KB > +#define ARM_GICR_SGI_RESERVED_FRAME_SIZE SIZE_64KB > > // GIC Redistributor Control frame > #define ARM_GICR_TYPER 0x0008 // Redistributor Type Register > > +// GIC Redistributor TYPER bit assignments > +#define ARM_GICR_TYPER_PLPIS (1 << 0) // Physical LPIs > +#define ARM_GICR_TYPER_VLPIS (1 << 1) // Virtual LPIs > +#define ARM_GICR_TYPER_DIRECTLPI (1 << 3) // Direct LPIs > +#define ARM_GICR_TYPER_LAST (1 << 4) // Last > Redistributor in series > +#define ARM_GICR_TYPER_DPGS (1 << 5) // Disable > Processor Group > + // Selection > Support > +#define ARM_GICR_TYPER_PROCNO (0xFFFF << 8) // Processor Number > +#define ARM_GICR_TYPER_COMMONLPIAFF (0x3 << 24) // Common LPI > Affinity > +#define ARM_GICR_TYPER_AFFINITY (0xFFFFFFFFULL << 32) // Redistributor > Affinity > + > // GIC SGI & PPI Redistributor frame > #define ARM_GICR_ISENABLER 0x0100 // Interrupt Set-Enable Registers > #define ARM_GICR_ICENABLER 0x0180 // Interrupt Clear-Enable Registers > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
