Re: [Xen-devel] [PATCH V5 05/10] xen/arm64: gicv3: Use AFF1 when translating ICC_SGI1R_EL1 to cpumask

[Chen Baozi <baozich@xxxxxxxxx>]

Hi Julien,

> On May 31, 2015, at 21:14, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> 
> Hi Chen,
> 
> On 30/05/2015 12:07, Chen Baozi wrote:
>> From: Chen Baozi <baozich@xxxxxxxxx>
>> 
>> To support more than 16 vCPUs, we have to calculate cpumask with AFF1
>> field value in ICC_SGI1R_EL1.
>> 
>> Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
>> ---
>>  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.

Sorry, I’m not quite understand this. Why can not?

Furthermore, considering that the size of cpumask_t might be greater than any 
int type, do you
get a better way to set the bitmap without a loop?

Cheers,

Baozi.

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel