From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 12/15] xen/arm: gic: Store the necessary HW information per vGIC ... Date: Tue, 30 Jun 2015 14:38:18 +0100 Message-ID: <55929BCA.1060107@citrix.com> References: <1435311269-3189-1-git-send-email-julien.grall@citrix.com> <1435311269-3189-13-git-send-email-julien.grall@citrix.com> <1435668976.21469.138.camel@citrix.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 1Z9vkX-000713-Oi for xen-devel@lists.xenproject.org; Tue, 30 Jun 2015 13:38:57 +0000 In-Reply-To: <1435668976.21469.138.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, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 30/06/15 13:56, Ian Campbell wrote: > On Fri, 2015-06-26 at 10:34 +0100, Julien Grall wrote: >> ... in order to decouple the vGIC driver from the GIC driver. >> >> Each vGIC HW structure contains a boolean to indicate if the current GIC is >> able to support this specific version of virtual GIC. >> >> Helpers have been introduced in order to help the GIC to setup correctly >> the vGIC. The GIC will have to call them to announce the support of this >> specific version. >> > > "...to help the GIC correctly setup the vGIC" > > "...to announce support for this specific version" > > >> Also drop fields that become unecessary in each global state. > > "unnecessary" I will fix it in the next version. >> @@ -228,6 +232,55 @@ static inline int vgic_allocate_spi(struct domain *d) >> >> extern void vgic_free_virq(struct domain *d, unsigned int virq); >> >> +struct vgic_v2_hw_config >> +{ >> + bool_t enabled; >> + /* Distributor interface address */ >> + paddr_t dbase; >> + /* CPU interface address */ >> + paddr_t cbase; >> + /* Virtual CPU interface address */ >> + paddr_t vbase; >> +}; >> + >> +extern struct vgic_v2_hw_config vgic_v2_hw; > > My inclination is to call this either vgic_v2_hwdom(_config) (since it > is vgic config for the hw dom) or to call it gic_v2_hw_config (since it > contains config info of the physical gic which we happen to be going to > use for vgic). > > I think given the expected usage the former makes more sense. I think that the 2 names don't work for the usage of the structure: - vgic_v2_hwdom: vbase is used to map the guest CPU interface into the virtual CPU interface - gic_v2_hw_config: it gives the impression to be physical GIC specific rather the virtual GIC. I would prefer to stick in vgic_v2_hw_config which show that we use it for the virtual GIC. >> + >> +static inline void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, >> + paddr_t vbase) >> +{ >> + vgic_v2_hw.enabled = 1; >> + vgic_v2_hw.dbase = dbase; >> + vgic_v2_hw.cbase = cbase; >> + vgic_v2_hw.vbase = vbase; >> +} > > If you were to move this out of line into vgic-v2.c would that mean that > vgic_v2_hw_config etc could be static to that file? No, we have to access the field enabled in domain_vgic_init to verify the GIC is supporting the version of the vGIC. Regards, -- Julien Grall