From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 06/15] xen/arm: vgic-v3: Set stride during domain initialization Date: Mon, 02 Feb 2015 16:14:52 +0000 Message-ID: <54CFA27C.3010506@linaro.org> References: <1422555950-31821-1-git-send-email-julien.grall@linaro.org> <1422555950-31821-7-git-send-email-julien.grall@linaro.org> <1422891654.5838.10.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 1YIJej-0005Pw-8T for xen-devel@lists.xenproject.org; Mon, 02 Feb 2015 16:15:21 +0000 Received: by mail-wi0-f173.google.com with SMTP id r20so17970809wiv.0 for ; Mon, 02 Feb 2015 08:15:19 -0800 (PST) In-Reply-To: <1422891654.5838.10.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 Cc: xen-devel@lists.xenproject.org, Vijaya.Kumar@caviumnetworks.com, tim@xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org On 02/02/15 15:40, Ian Campbell wrote: > On Thu, 2015-01-29 at 18:25 +0000, Julien Grall wrote: >> The stride may not be set if the hardware GIC is using the default >> layout. It happens on the Foundation model. >> >> On GICv3, the default stride is 2 * 64K. Therefore it's possible to avoid >> checking at every redistributor MMIO access if the stride is not set. > > Can this defaulting not be pulled further to the initialisation of > gicv3.rdist_stride? With the upcoming GICv4, the stride may be different for each distributor (see the check on GICR_TYPER.VLPIS in gicv3_populate_rdist). So I'd like to avoid the check of rdist_stride. >> This is only happening for DOM0, > > Please say instead "Because domU uses a static stride configuration this > only happens for dom0..." or similar (i.e. include the reason why domU > is excluded) I will do. >> so we can move this code in >> gicv_v3_init. Take the opportunity to move the stride setting a bit ealier > > "earlier". > >> because the loop to set regions will require the stride. >> >> Also, use 2 * 64K rather than 128K and explain the reason. >> >> Signed-off-by: Julien Grall >> >> --- >> I wasn't not sure where to move this code. I find very confusion the >> splitting between vgic and gicv. Maybe we should introduce a >> hwdom_gicv_init and giccc_map callbacks. Then move most of the >> initialization in the vgic one. >> >> Changes in v2: >> - Patch added >> --- >> xen/arch/arm/gic-v3.c | 11 ++++++++++- >> xen/arch/arm/vgic-v3.c | 6 +----- >> 2 files changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c >> index 47452ca..7b33ff7 100644 >> --- a/xen/arch/arm/gic-v3.c >> +++ b/xen/arch/arm/gic-v3.c >> @@ -897,12 +897,21 @@ static int gicv_v3_init(struct domain *d) >> { >> d->arch.vgic.dbase = gicv3.dbase; >> d->arch.vgic.dbase_size = gicv3.dbase_size; >> + >> + d->arch.vgic.rdist_stride = gicv3.rdist_stride; >> + /* >> + * If the stride is not set, the default stride for GICv3 is 2 * 64K: >> + * - first 64k page for Control and Physical LPIs >> + * - second 64k page for Control and Generation of SGIs >> + */ >> + if ( !d->arch.vgic.rdist_stride ) >> + d->arch.vgic.rdist_stride = 2 * SZ_64K; >> + >> for ( i = 0; i < gicv3.rdist_count; i++ ) >> { >> d->arch.vgic.rbase[i] = gicv3.rdist_regions[i].base; >> d->arch.vgic.rbase_size[i] = gicv3.rdist_regions[i].size; >> } >> - d->arch.vgic.rdist_stride = gicv3.rdist_stride; >> d->arch.vgic.rdist_count = gicv3.rdist_count; >> } >> else >> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c >> index 2c14717..db6b514 100644 >> --- a/xen/arch/arm/vgic-v3.c >> +++ b/xen/arch/arm/vgic-v3.c >> @@ -625,11 +625,7 @@ static int vgic_v3_rdistr_mmio_read(struct vcpu *v, mmio_info_t *info) > > Why not the write case too? By mistake it has been dropped in a following patch ("Emulate correctly the re-distributor"). I will move the changes here. Regards, -- Julien Grall