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 18:58:12 +0100 Message-ID: <556B4BB4.5020801@citrix.com> References: <1432984051-10838-1-git-send-email-cbz@baozis.org> <1432984051-10838-6-git-send-email-cbz@baozis.org> <556B092D.4020909@citrix.com> <4F63C548-AB76-441C-9D6C-B6CCF211E09E@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Yz7V4-00044T-1w for xen-devel@lists.xenproject.org; Sun, 31 May 2015 17:58:18 +0000 In-Reply-To: <4F63C548-AB76-441C-9D6C-B6CCF211E09E@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: Chen Baozi , Julien Grall Cc: xen-devel@lists.xenproject.org, Chen Baozi , Campbell Ian List-Id: xen-devel@lists.xenproject.org On 31/05/2015 16:25, Chen Baozi wrote: >> On May 31, 2015, at 21:14, Julien Grall wrote: >> 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 =3D sgir & ICH_SGI_TARGETLIST_MASK; >>> - bitmap_copy(cpumask_bits(cpumask), &target_list, ICH_SGI_TARGET_BI= TS); >>> + /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */ >>> + aff1 =3D (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 t= o all the vgic. It would have been more logical to put it in the function v= gic_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 cou= ld take down Xen. >> >>> + >>> + memcpy((uint16_t *)cpumask + aff1, &target_list, >> >> That's hackhish. You can't assume that the bitmap will be at the beginni= ng of cpumask_t. > > Sorry, I=92m not quite understand this. Why can not? cpumask_t is a structure containing a bitmap. You are assuming that the = address of cpumask_t is always equal to cpumask_t.bitmap. There is helper to get the correct address (see cpumask_bits(cpumask)). Furthermore, there is function to copy a bitmap (see bitmap_copy). = Although it will require more than 1 line of code. If you don't want to use it please use a direct assignation (it's only a = 16 bits data) which will always be faster than a memcpy. Regards, -- = Julien Grall