From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x236.google.com (mail-wm0-x236.google.com [IPv6:2a00:1450:400c:c09::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6858981DAF for ; Mon, 7 Nov 2016 15:58:31 -0800 (PST) Received: by mail-wm0-x236.google.com with SMTP id a197so214146668wmd.0 for ; Mon, 07 Nov 2016 15:58:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=PKf9/Z9fwcbkICddWSisbB7f/M11A7BIYBU1mCqxrcE=; b=gH7gr+F9YgDsY48EkeWBItwD9OfXOMQ2vAdbnvTuJ4gfbbv7mrI8ee2H+jV+vfmkSu rr9E80mDXJ3edUFZGmNyffucqwyboWHq8TRIjHyOWHRyjnDWSUm7xkZr/QDACj+R6PN7 8x7mRL5kaxwrsnKJTOnhN/4NPBXK15aQ1eY6I= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=PKf9/Z9fwcbkICddWSisbB7f/M11A7BIYBU1mCqxrcE=; b=JXe+paEHAeL3Sycj3o6Y7OmGMSczq0EWdnM9gVpsm/Dr3aUF773PfZX7UqQ1DV7d30 AVXOQPmix/CSafcFc7jNTZawBoa19XVcBaJr/h+HdCVy8z2SmhYfc91XMxC9kqfB1ygS T2W1BBWJZ0BkMqVgA5HTr6Qb6+Ht9mQfCW93oLuPnGEFd9P5Fibwog8i79O0gR/Luo13 DPXWZ227aUv1dUQJh7D22xhnB3ZJM767bGWrtGlGpWf0f2MBtOdApOgimIjycoKyXnIN iJv12ZI8N20qGH0nIF31k4jNdIGIi7yI5p3YqvMjVCsawFQRSEYIuZT5BcwoZ9bn4q39 4GFw== X-Gm-Message-State: ABUngverjp0FIywZfPSWYPet3J9a26o9ebi112Q09u2dr93kRrq+AvGxt9Hwjp75HC/c1xW8 X-Received: by 10.28.178.10 with SMTP id b10mr13190230wmf.83.1478563112748; Mon, 07 Nov 2016 15:58:32 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id h2sm33779624wjy.40.2016.11.07.15.58.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 07 Nov 2016 15:58:32 -0800 (PST) Date: Mon, 7 Nov 2016 23:58:30 +0000 From: Leif Lindholm To: evan.lloyd@arm.com Cc: edk2-devel@ml01.01.org, Ard Biesheuvel , Ryan Harkin Message-ID: <20161107235830.GG27644@bivouac.eciton.net> References: <20161104165252.12888-1-evan.lloyd@arm.com> MIME-Version: 1.0 In-Reply-To: <20161104165252.12888-1-evan.lloyd@arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH] ArmPkg: Update Redistributor granularity for GICv4 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Nov 2016 23:58:31 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Evan, Sami, On Fri, Nov 04, 2016 at 05:52:52PM +0100, evan.lloyd@arm.com wrote: > From: Sami Mujawar > > 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 > and calculates the Redistributor granularity accordingly. Am I understanding it correctly, that this patch is required to provide GICv4 support? > 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.0 > Signed-off-by: Sami Mujawar > Signed-off-by: Evan Lloyd > --- > Code is available at: > https://github.com/EvanLloyd/tianocore/tree/639_gicv4_v1 > > ArmPkg/Include/Library/ArmGicLib.h | 17 ++++++++- > ArmPkg/Drivers/ArmGic/ArmGicLib.c | 40 +++++++++++++------- > 2 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h > index 4364f3ffef464596f64cf59881d703cf54cf0ddd..079489fe76ab481915ce9da3702d351fd3cb5f0e 100644 > --- a/ArmPkg/Include/Library/ArmGicLib.h > +++ b/ArmPkg/Include/Library/ArmGicLib.h > @@ -55,12 +55,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 (0xFFFFFFFF << 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 > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c > index e658e9bff5d8107b3914bdf1e9e1e51a4e4d4cd7..b51d2b3ec55d277e36835669956b4dd866cfc5c6 100644 > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c > @@ -1,6 +1,6 @@ > /** @file > * > -* Copyright (c) 2011-2015, ARM Limited. All rights reserved. > +* Copyright (c) 2011-2016, 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 > #include > > +// 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 an additional 2 x 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) > + > /** > * > * Return whether the Source interrupt index refers to a shared interrupt (SPI) > @@ -40,6 +50,7 @@ SourceIsSpi ( > * > * @retval Base address of the associated GIC Redistributor > */ > + This added blank line between description and function does not appear to follow comon style (including earlier in this file). > STATIC > UINTN > GicGetCpuRedistributorBase ( > @@ -47,37 +58,38 @@ GicGetCpuRedistributorBase ( > IN ARM_GIC_ARCH_REVISION Revision > ) > { > - UINTN Index; > UINTN MpId; > - UINTN CpuAffinity; > - UINTN Affinity; > - UINTN GicRedistributorGranularity; > + UINT32 CpuAffinity; > + UINT32 Affinity; This is just sufficiently related to the change that I'll let it slide with a grumble. *grumble* > UINTN GicCpuRedistributorBase; > + UINT64 GicRTyper; > > MpId = ArmReadMpidr (); > // Define CPU affinity as Affinity0[0:8], Affinity1[9:15], Affinity2[16:23], Affinity3[24:32] > // whereas Affinity3 is defined at [32:39] in MPIDR > 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; > 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. > + GicCpuRedistributorBase += ((ARM_GICR_TYPER_VLPIS & GicRTyper) > + ? GIC_V4_REDISTRIBUTOR_GRANULARITY > + : GIC_V3_REDISTRIBUTOR_GRANULARITY); > + } while (0 == (GicRTyper & ARM_GICR_TYPER_LAST)); Rather than the jeopardy comparison, can that just be } while (!(GicRTyper & ARM_GICR_TYPR_LAST)) ? > > // The Redistributor has not been found for the current CPU > ASSERT_EFI_ERROR (EFI_NOT_FOUND); > -- > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >