From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Sami Mujawar <sami.mujawar@arm.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Leif Lindholm <leif.lindholm@linaro.org>,
Matteo Carlini <Matteo.Carlini@arm.com>,
Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>,
Evan Lloyd <evan.lloyd@arm.com>, nd <nd@arm.com>
Subject: Re: [PATCH v2 1/1] ArmPkg: Add support for GICv4
Date: Tue, 9 Oct 2018 16:25:48 +0200 [thread overview]
Message-ID: <CAKv+Gu_LGoqFGbUj3YTNZKHHjp0QmUrQi4gszXBWa27v6ZhqDg@mail.gmail.com> (raw)
In-Reply-To: <20181009112024.10592-1-sami.mujawar@arm.com>
On 9 October 2018 at 13:20, Sami Mujawar <sami.mujawar@arm.com> 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 GIC Redistributor Type
> Register (GICR_TYPER) 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.mujawar@arm.com>
> ---
> The changes can be seen at:
> https://github.com/samimujawar/edk2/tree/329_gicv4_granularity_v2
>
> Notes:
> v2:
> - Code review comments to change variable name, expand
> name for register, local variable name, addition of
> macro for returning affinity & code comment update. [LEIF]
> - Updated code based on review comments. [SAMI]
>
> v1:
> - Added support for initializing GICv4 [SAMI]
>
> ArmPkg/Drivers/ArmGic/ArmGicLib.c | 38 ++++++++++++--------
> ArmPkg/Include/Library/ArmGicLib.h | 22 ++++++++++--
> 2 files changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> index 0087399fb1dba0e697f7a6ccd6f7432a59311ac6..161bd4408fb30e44eab89eb5eccfca110b8a210d 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 TypeRegister;
>
> MpId = ArmReadMpidr ();
> // Define CPU affinity as:
> @@ -68,27 +77,28 @@ 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++) {
This removes the last reference to
gArmPlatformTokenSpaceGuid.PcdCoreCount in this module, so please drop
it from the .inf as well.
> - Affinity = MmioRead64 (GicCpuRedistributorBase + ARM_GICR_TYPER) >> 32;
> + do {
> + TypeRegister = MmioRead64 (GicCpuRedistributorBase + ARM_GICR_TYPER);
> + Affinity = ARM_GICR_TYPER_GET_AFFINITY (TypeRegister);
> 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 redistributors
> + // with or without support for virtual LPIs, so we test Virtual LPIs
> + // Support (VLPIS) bit for each frame to decide the granularity.
> + GicCpuRedistributorBase += (((ARM_GICR_TYPER_VLPIS & TypeRegister) != 0)
> + ? GIC_V4_REDISTRIBUTOR_GRANULARITY
> + : GIC_V3_REDISTRIBUTOR_GRANULARITY);
> + } while ((TypeRegister & ARM_GICR_TYPER_LAST) == 0);
>
AFAICT this code still assumes that the redistributors are adjacent
for all CPUs, which IIUC may not be the case on NUMA systems.
That is fine, but it might deserve a comment now that we are making
changes to this code.
> // 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..5775905ca91baabca13de47b8b9f7ac507becd55 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,28 @@
>
>
> // 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
> +
> +#define ARM_GICR_TYPER_GET_AFFINITY(TypeReg) (((TypeReg) & \
> + ARM_GICR_TYPER_AFFINITY) >> 32)
> +
> // 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)'
>
>
prev parent reply other threads:[~2018-10-09 14:25 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-09 11:20 [PATCH v2 1/1] ArmPkg: Add support for GICv4 Sami Mujawar
2018-10-09 14:25 ` Ard Biesheuvel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu_LGoqFGbUj3YTNZKHHjp0QmUrQi4gszXBWa27v6ZhqDg@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox