From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v7 4/8] xen/arm: Use struct sgi_target instead of vcpu_mask in vgic_to_sgi Date: Thu, 11 Jun 2015 10:54:51 -0400 Message-ID: <5579A13B.1000301@citrix.com> References: <1434027910-28375-1-git-send-email-cbz@baozis.org> <1434027910-28375-5-git-send-email-cbz@baozis.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" 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 1Z33sg-0000JN-3J for xen-devel@lists.xenproject.org; Thu, 11 Jun 2015 14:54:58 +0000 In-Reply-To: <1434027910-28375-5-git-send-email-cbz@baozis.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: Chen Baozi , xen-devel@lists.xenproject.org Cc: Chen Baozi , Ian Campbell List-Id: xen-devel@lists.xenproject.org Hi Chen, On 11/06/2015 09:05, Chen Baozi wrote: > From: Chen Baozi > > The old unsigned long type of vcpu_mask can only express 64 cpus at the > most, which might not be enough for the guest which used vGICv3. We > introduce a new struct sgi_target for the target cpu list of SGI, which > holds the affinity path information. For GICv2 that has no affinity > level, we can just set the corresponding fields to be 0. The whole point of this patch is to add support for AFF1 in GICv3... not replacing vcpu_mask by sgi_target. The latter is just how you decided to implement AFF1. Please ensure that your commit message/tittle reflect it. [..] > static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr) > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 7b387b7..0173c8d 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -318,15 +318,14 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) > } > } > > -/* TODO: unsigned long is used to fit vcpu_mask.*/ > int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int virq, > - unsigned long vcpu_mask) > + struct sgi_target *target) const struct sgi_target... [..] > @@ -334,29 +333,40 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode, int > { > case SGI_TARGET_LIST: > perfc_incr(vgic_sgi_list); > + base = (target->aff1 << 4) | (target->aff2 << 12) | > + (target->aff3 << 20); > + bitmap = target->list; > + for_each_set_bit( i, &bitmap, sizeof(target->list)*8 ) Coding style. sizeof(target->list) * 8 [..] > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 463fffb..c6ef4bf 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -64,6 +64,7 @@ > #define GICD_SGI_TARGET_SELF_VAL (2) > #define GICD_SGI_TARGET_SHIFT (16) > #define GICD_SGI_TARGET_MASK (0xFFUL< +#define GICD_SGI_TARGET_BITS (8) > #define GICD_SGI_GROUP1 (1UL<<15) > #define GICD_SGI_INTID_MASK (0xFUL) > > diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h > index 556f114..333aa56 100644 > --- a/xen/include/asm-arm/gic_v3_defs.h > +++ b/xen/include/asm-arm/gic_v3_defs.h > @@ -152,6 +152,10 @@ > #define ICH_SGI_IRQ_SHIFT 24 > #define ICH_SGI_IRQ_MASK 0xf > #define ICH_SGI_TARGETLIST_MASK 0xffff > +#define ICH_SGI_TARGET_BITS 16 > +#define ICH_SGI_AFFx_MASK 0xff > +#define ICH_SGI_AFFINITY_LEVEL(x) (16 * (x)) > + > #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */ > > /* > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > index 6dcdf9f..cc68c27 100644 > --- a/xen/include/asm-arm/vgic.h > +++ b/xen/include/asm-arm/vgic.h > @@ -98,6 +98,13 @@ struct vgic_irq_rank { > }; > }; > > +struct sgi_target { > + uint8_t aff3; > + uint8_t aff2; I don't see why you are not using aff2 and aff3 at all the the vGICv* drivers. Please drop them. > + uint8_t aff1; > + uint16_t list; > +}; > + Regards, -- Julien Grall