Hi,
static DECLARE_BITMAP(vmid_mask, MAX_VMID);
>>
>> void p2m_vmid_allocator_init(void)
>> {
>> + unsigned int cpu;
>> +
>> set_bit(INVALID_VMID, vmid_mask);
>> +
>> + max_vmid = MAX_VMID;
>>
>
> max_vmid is only declared for ARM64 and will break compilation for ARM32.
> Please try to build each patch for both architecture before sending to the
> ML.
>
> I will compile for ARM32.
> However, in this case. It would make more sense to keep the maximum vmid
> static for ARM32 as it will never change.
>
>
I quite like what has been done for P2M_ROOT_{LEVEL,ORDER} where the define
> is a constant on ARM32 and point to a variable on ARM64.
>
> I will keep the bitmap statically defined for ARM32. For ARM64 bitmap will
be allocated dynamically.
> +
>> +#ifdef CONFIG_ARM_64
>> + /*
>> + * if any cpu does not support 16-bit VMID then restrict the
>> + * max VMIDs which can be allocated to 256
>> + */
>> + for_each_online_cpu ( cpu )
>> + {
>> + const struct cpuinfo_arm *info = &cpu_data[cpu];
>> +
>> + if ( info->mm64.vmid_bits != VMID_16_BITS_SUPPORT )
>> + {
>> + max_vmid = (1UL << 8);
>> + break;
>> + }
>> + }
>> +#endif
>>
>
> I would rework this code to have max_vmid set to 8 bits by default and the
> turn on 16 bits if available on every CPU.
>
> I will modify the code accordingly.
> }
>>
>> static int p2m_alloc_vmid(struct domain *d)
>> @@ -1254,11 +1282,11 @@ static int p2m_alloc_vmid(struct domain *d)
>>
>> spin_lock(&vmid_alloc_lock);
>>
>> - nr = find_first_zero_bit(vmid_mask, MAX_VMID);
>> + nr = find_first_zero_bit(vmid_mask, max_vmid);
>>
>> ASSERT(nr != INVALID_VMID);
>>
>> - if ( nr == MAX_VMID )
>> + if ( nr == max_vmid )
>> {
>> rc = -EBUSY;
>> printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n",
>> d->domain_id);
>> @@ -1646,6 +1674,10 @@ void __init setup_virt_paging(void)
>>
>> val |= VTCR_PS(pa_range);
>> val |= VTCR_TG0_4K;
>> +
>> + /* set the VS bit only if 16 bit VMID is supported */
>> + if ( max_vmid == MAX_VMID )
>>
>
> This check is confusing, I would be clearer to use MAX_VMID_16 or else.
>
> I will explicitly use 16 bit macro
> + val |= VTCR_VS;
>> val |= VTCR_SL0(pa_range_info[pa_range].sl0);
>> val |= VTCR_T0SZ(pa_range_info[pa_range].t0sz);
>>
>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
>> index fdb6b47..bfcdbf1 100644
>> --- a/xen/include/asm-arm/p2m.h
>> +++ b/xen/include/asm-arm/p2m.h
>> @@ -30,7 +30,7 @@ struct p2m_domain {
>> struct page_info *root;
>>
>> /* Current VMID in use */
>> - uint8_t vmid;
>> + uint16_t vmid;
>>
>> /* Current Translation Table Base Register for the p2m */
>> uint64_t vttbr;
>> diff --git a/xen/include/asm-arm/processor.h
>> b/xen/include/asm-arm/processor.h
>> index 15bf890..4b6be3d 100644
>> --- a/xen/include/asm-arm/processor.h
>> +++ b/xen/include/asm-arm/processor.h
>> @@ -215,6 +215,7 @@
>>
>> #define VTCR_PS(x) ((x)<<16)
>>
>> +#define VTCR_VS (_AC(0x1,UL)<<19)
>>
>
> Newline here please.
>
> Ok.
> #endif
>>
>> #define VTCR_RES1 (_AC(1,UL)<<31)
>> @@ -269,6 +270,11 @@
>> /* FSR long format */
>> #define FSRL_STATUS_DEBUG (_AC(0x22,UL)<<0)
>>
>> +#ifdef CONFIG_ARM_64
>> +#define VMID_8_BITS_SUPPORT 0x0
>> +#define VMID_16_BITS_SUPPORT 0x2
>> +#endif
>>
>
> I would prefix the 2 define with MM64_ so we know how the values will be
> matched.
>
> You mean I define them like this ?
#define MM64_VMID_8_BITS_SUPPORT 0x0
#define MM64_VMID_16_BITS_SUPPORT 0x2
+
>> #ifndef __ASSEMBLY__
>>
>> struct cpuinfo_arm {
>> @@ -337,7 +343,16 @@ struct cpuinfo_arm {
>> unsigned long tgranule_64K:4;
>> unsigned long tgranule_4K:4;
>> unsigned long __res0:32;
>> - };
>> +
>> + unsigned long hafdbs:4;
>> + unsigned long vmid_bits:4;
>> + unsigned long vh:4;
>> + unsigned long hpds:4;
>> + unsigned long lo:4;
>> + unsigned long pan:4;
>> + unsigned long __res1:8;
>> + unsigned long __res2:32;
>> + };
>> } mm64;
>>
>> struct {
>>
>>
> Regards,
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
