From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 06/15] xen/arm: vgic-v3: Set stride during domain initialization Date: Mon, 2 Feb 2015 15:40:54 +0000 Message-ID: <1422891654.5838.10.camel@citrix.com> References: <1422555950-31821-1-git-send-email-julien.grall@linaro.org> <1422555950-31821-7-git-send-email-julien.grall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YIJ7T-0000Ri-Dq for xen-devel@lists.xenproject.org; Mon, 02 Feb 2015 15:40:59 +0000 In-Reply-To: <1422555950-31821-7-git-send-email-julien.grall@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall 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 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? > 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) > 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? > > perfc_incr(vgicr_reads); > > - if ( v->domain->arch.vgic.rdist_stride != 0 ) > - offset = info->gpa & (v->domain->arch.vgic.rdist_stride - 1); > - else > - /* If stride is not set. Default 128K */ > - offset = info->gpa & (SZ_128K - 1); > + offset = info->gpa & (v->domain->arch.vgic.rdist_stride - 1); > > if ( offset < SZ_64K ) > return __vgic_v3_rdistr_rd_mmio_read(v, info, offset);