From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [RFC 22/22] xen/arm: gic-v3: Add support of vGICv2 when available Date: Mon, 8 Jun 2015 11:01:12 +0100 Message-ID: <1433757672.7108.417.camel@citrix.com> References: <1431091783-29090-1-git-send-email-julien.grall@citrix.com> <1431091783-29090-23-git-send-email-julien.grall@citrix.com> <1433508510.7108.275.camel@citrix.com> <5571CFBC.2020702@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z1ts1-0000Xq-Ky for xen-devel@lists.xenproject.org; Mon, 08 Jun 2015 10:01:29 +0000 In-Reply-To: <5571CFBC.2020702@gmail.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: Julien Grall Cc: Julien Grall , xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org On Fri, 2015-06-05 at 17:35 +0100, Julien Grall wrote: > >> */ > >> static void gicv3_enable_sre(void) > >> { > >> uint32_t val; > >> > >> val = READ_SYSREG32(ICC_SRE_EL2); > >> - val |= GICC_SRE_EL2_SRE | GICC_SRE_EL2_ENEL1; > >> + val |= GICC_SRE_EL2_SRE; > >> > >> WRITE_SYSREG32(val, ICC_SRE_EL2); > >> isb(); > >> @@ -373,6 +372,20 @@ static void gicv3_save_state(struct vcpu *v) > >> > >> static void gicv3_restore_state(const struct vcpu *v) > >> { > >> + uint32_t val; > >> + > >> + val = READ_SYSREG32(ICC_SRE_EL2); > >> + /* > >> + * Don't give access to system registers when the guest is using > >> + * GICv2 > >> + */ > >> + if ( v->domain->arch.vgic.version == GIC_V2 ) > >> + val &= ~GICC_SRE_EL2_ENEL1; > >> + else > >> + val |= GICC_SRE_EL2_ENEL1; > >> + WRITE_SYSREG32(val, ICC_SRE_EL2); > > > > Perhaps save/restore v->arch.gic.v3.sre_el2 rather than reading and > > recalculating each time? Then you just need to set sre_el2 appropriately > > during domain init. > > Hmmm that would mean to reintroduce gicv_setup for setting sre_el2. > > What is your concern here? I don't really like read/modify/write idiom in the restore path, I'd prefer a straight save/restore model (I know we have some R/M/W already). > >> @@ -1107,6 +1127,32 @@ static int __init cmp_rdist(const void *a, const void *b) > >> return ( l->base < r->base) ? -1 : 0; > >> } > >> > >> +/* If the GICv3 supports GICv2, initialize it */ > >> +static void __init gicv3_init_v2(const struct dt_device_node *node) > >> +{ > >> + int res; > >> + > >> + /* > >> + * For GICv3 supporting GICv2, GICC and GICV base address will be > >> + * provided. > >> + */ > >> + > >> + res = dt_device_get_address(node, 1 + gicv3_info.nr_rdist_regions, > >> + &gicv3_info.cbase, NULL); > >> + if ( res ) > >> + return; > >> + > >> + res = dt_device_get_address(node, 1 + gicv3_info.nr_rdist_regions + 2, > >> + &gicv3_info.vbase, NULL); > >> + if ( res ) > >> + return; > >> + > >> + printk("GICv3 compatible with GICv2 cbase %#"PRIpaddr" vbase %#"PRIpaddr"\n", > >> + gicv3_info.cbase, gicv3_info.vbase); > >> + > >> + gicv3_info.vgic_versions |= GIC_V2; > > > > So I think this is a second type of compat, right? After this we > > support: > > > > Guests using GICv2, via vgic-v2.c > > Guests using GICv3, via vgic-v3.c > > > > But also: > > > > Guests using GICv2, via vgic-v3.c, i.e. vgicv3 in compat mode. > > > > Is that right? Is it intended? > > No, we don't support the last one. This variable is used to know which > callback set we have to use. > > It may be clearer with the suggested solution in patch #16. Lets see ;-)