Re: [Xen-devel] [XEN VMID PATCH 2/2 v3] xen/arm: Add support for 16 bit VMIDs

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

Hi Julien,


On 13 December 2016 at 19:43, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi Bhupinder,
>
> On 12/12/16 07:26, Bhupinder Thakur wrote:
>
> [...]
>
>> -void p2m_vmid_allocator_init(void)
>> +int p2m_vmid_allocator_init(void)
>>  {
>> -    set_bit(INVALID_VMID, vmid_mask);
>> +    int ret = 0;
>> +
>> +    /*
>> +     * allocate space for vmid_mask based on MAX_VMID
>> +     */
>> +    vmid_mask = xzalloc_array(unsigned long, BITS_TO_LONGS(MAX_VMID));
>
>
> I would directly handle the panic within this function rather than return an
> error. I.e
>
> if ( !vmid_mask )
>   panic(....);
>
> This would simplify the logic a bit.
>
ok. I will modify the code accordingly.


>>  static int p2m_alloc_vmid(struct domain *d)
>> @@ -1632,20 +1653,36 @@ void __init setup_virt_paging(void)
>>
>>      unsigned int cpu;
>>      unsigned int pa_range = 0x10; /* Larger than any possible value */
>> +    unsigned int vmid_8_bit_flag = 0;
>
>
> Please use bool:
>
> bool vmid_8_bit_flag = false;
>
> I would also drop _flag as it is not necessary.
ok. I will modify the code accordingly.

>
>>
>>      for_each_online_cpu ( cpu )
>>      {
>>          const struct cpuinfo_arm *info = &cpu_data[cpu];
>>          if ( info->mm64.pa_range < pa_range )
>>              pa_range = info->mm64.pa_range;
>> +
>> +        /* set a flag if the current cpu does not suppot 16 bit VMIDs */
>> +        if ( info->mm64.vmid_bits != MM64_VMID_16_BITS_SUPPORT )
>> +            vmid_8_bit_flag = 1;
>
>
> s/1/true/
>
ok

> The rest of the patch looks good to me.
>
> Cheers,
>
> --
> Julien Grall

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