From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH V5 05/10] xen/arm64: gicv3: Use AFF1 when translating ICC_SGI1R_EL1 to cpumask Date: Sun, 31 May 2015 14:14:21 +0100 Message-ID: <556B092D.4020909@citrix.com> References: <1432984051-10838-1-git-send-email-cbz@baozis.org> <1432984051-10838-6-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 1Yz34a-0004dw-B3 for xen-devel@lists.xenproject.org; Sun, 31 May 2015 13:14:40 +0000 In-Reply-To: <1432984051-10838-6-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: Julien Grall , Chen Baozi , Ian Campbell List-Id: xen-devel@lists.xenproject.org Hi Chen, On 30/05/2015 12:07, Chen Baozi wrote: > From: Chen Baozi > > To support more than 16 vCPUs, we have to calculate cpumask with AFF1 > field value in ICC_SGI1R_EL1. > > Signed-off-by: Chen Baozi > --- > xen/arch/arm/vgic-v3.c | 9 ++++++++- > xen/include/asm-arm/gic_v3_defs.h | 3 +++ > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index a283c8c..21d8d3f 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -976,10 +976,17 @@ static inline void gicv3_sgir_to_cpumask(cpumask_t *cpumask, > const register_t sgir) > { > unsigned long target_list; > + int aff1; unsigned int. > > target_list = sgir & ICH_SGI_TARGETLIST_MASK; > - bitmap_copy(cpumask_bits(cpumask), &target_list, ICH_SGI_TARGET_BITS); > + /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */ > + aff1 = (sgir >> ICH_SGI_AFFINITY_LEVEL(1)) & ICH_SGI_AFFx_MASK; > > + BUILD_BUG_ON(sizeof(cpumask_t)*8 < MAX_VIRT_CPUS); Ah, here is the BUILD_BUG_ON. This is not vgic-v3 specific but generic to all the vgic. It would have been more logical to put it in the function vgic_to_sgi in the previous patch (i.e #4). > + BUG_ON(((aff1+1) * ICH_SGI_TARGET_BITS) > NR_CPUS); NACK. This value is passed by the guest. With this a malicious guest could take down Xen. > + > + memcpy((uint16_t *)cpumask + aff1, &target_list, That's hackhish. You can't assume that the bitmap will be at the beginning of cpumask_t. > + (ICH_SGI_TARGET_BITS/8)); > } > > static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir) > diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h > index e106e67..3743e66 100644 > --- a/xen/include/asm-arm/gic_v3_defs.h > +++ b/xen/include/asm-arm/gic_v3_defs.h > @@ -153,6 +153,9 @@ > #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)) > + Spurious line. > > #endif /* __ASM_ARM_GIC_V3_DEFS_H__ */ Regards, -- Julien Grall