From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 03/10] xen/arm: vgic-v3: Correctly handle GICD_CTLR Date: Tue, 20 Jan 2015 15:51:23 +0000 Message-ID: <1421769083.10440.300.camel@citrix.com> References: <1421684957-29884-1-git-send-email-julien.grall@linaro.org> <1421684957-29884-4-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.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YDbIP-0005fg-Ke for xen-devel@lists.xenproject.org; Tue, 20 Jan 2015 16:04:49 +0000 In-Reply-To: <1421684957-29884-4-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, tim@xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org On Mon, 2015-01-19 at 16:29 +0000, Julien Grall wrote: > As backward GICv2 compatibility is not supported in the VGICv3 driver, > the bit ARE_NS should be set at any time. Looking at the docs, I think you mean it is RAO/WI if GICv2 compat is absent (rather than imply the guest should set it). > > Futhermore, when ARE_NS is set, the guest can only modify EnableGrp1A. "Furthermore" > At same time take the vgic_lock to write into domain.arch.vgic.ctrl. It > was already taken during read. > > Signed-off-by: Julien Grall > --- > xen/arch/arm/vgic-v3.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index 406ea93..1aa2f58 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -45,6 +45,12 @@ > #define GICV3_GICR_PIDR2 GICV3_GICD_PIDR2 > #define GICV3_GICR_PIDR4 GICV3_GICD_PIDR4 > > +/* > + * GICD_CTLR default value: > + * - No GICv2 compatibility => ARE = 1 > + */ > +#define VGICD_CTLR_DEFAULT (GICD_CTLR_ARE_NS) > + > static struct vcpu *vgic_v3_irouter_to_vcpu(struct vcpu *v, uint64_t irouter) > { > irouter &= ~(GICD_IROUTER_SPI_MODE_ANY); > @@ -834,8 +840,15 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info) > { > case GICD_CTLR: > if ( dabt.size != DABT_WORD ) goto bad_width; > - /* Ignore all but the enable bit */ > - v->domain->arch.vgic.ctlr = (*r) & GICD_CTL_ENABLE; > + > + vgic_lock(v); > + /* Only EnableGrp1A can be changed */ > + if ( *r & GICD_CTLR_ENABLE_G1A ) > + v->domain->arch.vgic.ctlr |= GICD_CTLR_ENABLE_G1A; > + else > + v->domain->arch.vgic.ctlr &= ~GICD_CTLR_ENABLE_G1A; > + vgic_unlock(v); > + > return 1; > case GICD_TYPER: > /* RO -- write ignored */ > @@ -1092,6 +1105,8 @@ static int vgic_v3_domain_init(struct domain *d) > register_mmio_handler(d, &vgic_rdistr_mmio_handler, > d->arch.vgic.rbase[i], d->arch.vgic.rbase_size[i]); > > + d->arch.vgic.ctlr = VGICD_CTLR_DEFAULT; > + > return 0; > } >