From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH 2/2] xen/arm: gic-v3: Allow Xen to run on hardware reporting GICv4 Date: Tue, 15 Sep 2015 11:28:35 +0100 Message-ID: <55F7F2D3.4050702@arm.com> References: <1442244744-30190-1-git-send-email-julien.grall@citrix.com> <1442244744-30190-3-git-send-email-julien.grall@citrix.com> <1442310586.3549.358.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZbnSO-0004J9-E9 for xen-devel@lists.xenproject.org; Tue, 15 Sep 2015 10:27:24 +0000 In-Reply-To: <1442310586.3549.358.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Julien Grall , "xen-devel@lists.xenproject.org" Cc: "stefano.stabellini@eu.citrix.com" List-Id: xen-devel@lists.xenproject.org Hi, On 15/09/15 10:49, Ian Campbell wrote: > On Mon, 2015-09-14 at 16:32 +0100, Julien Grall wrote: >> It seems that there is some hardware which report start to report GICv4 > > s/report start to reports/reports/ ? > > Also, this is an odd way to express it, what you mean is that some hardware > is now shipping with GICv4. Unless you are trying to imply that they are > claiming to be GICv4 without actually being so? > > (If we agree on some wording I can modify this text on commit, subject to > the discussion below). > >> in the GIC*_PIDR2 register. >> >> As GICv4 is a superset of GICv3, it should just work on Xen. >> >> Reported-by: Andre Przywara >> Signed-off-by: Julien Grall >> --- >> xen/arch/arm/gic-v3.c | 4 ++-- >> xen/include/asm-arm/gic_v3_defs.h | 1 + >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c >> index 4d623bf..1e3c19b 100644 >> --- a/xen/arch/arm/gic-v3.c >> +++ b/xen/arch/arm/gic-v3.c >> @@ -640,7 +640,7 @@ static int __init gicv3_populate_rdist(void) >> void __iomem *ptr = gicv3.rdist_regions[i].map_base; >> >> reg = readl_relaxed(ptr + GICR_PIDR2) & GIC_PIDR2_ARCH_MASK; >> - if ( reg != GIC_PIDR2_ARCH_GICv3 ) >> + if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 ) > > Once we have GICv5, 6, etc this is going to get unwieldy, shall we switch > to a switch now? The current GICv3 documentation is actually subtitled: "GIC architecture version 3.0 and version 4.0", so I'd view them both together. We can care about GICv >4 once this appears, but for now I'd just stick with that simple if extension. I just stumbled upon this because the Linux driver compares against v3 and v4 with a very similar statement (and-ed compares in if). > >> { >> dprintk(XENLOG_ERR, >> "GICv3: No redistributor present @%"PRIpaddr"\n", > > I wonder if GICv3 ought to become GICv%d, on the other hand this is really > the GICv3 driver driving a v4 in v3 "mode", so maybe v3 is the best > logging. I'd keep v3 as well, as this part is really a GICv3 property. Maybe we should quote the spec in the commit message: "Because GICv4 is an extension of GICv3, all references to GICv3 in this manual apply equally to GICv4." Cheers, Andre.