| Message ID | 1478855206-19185-1-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, 11 Nov 2016, Bhupinder Thakur wrote: > VMID space is increased to 16-bits from 8-bits in ARMv8 8.1 revision. > This allows more than 256 VMs to be supported by Xen. > > This change adds support for 16-bit VMIDs in Xen based on whether the > architecture supports it. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> Thanks for the patch! > xen/arch/arm/p2m.c | 44 +++++++++++++++++++++++++++++++++++------ > xen/include/asm-arm/p2m.h | 2 +- > xen/include/asm-arm/processor.h | 17 +++++++++++++++- > 3 files changed, 55 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index cc5634b..6ed7e5c 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -19,6 +19,7 @@ static unsigned int __read_mostly p2m_root_order; > static unsigned int __read_mostly p2m_root_level; > #define P2M_ROOT_ORDER p2m_root_order > #define P2M_ROOT_LEVEL p2m_root_level > +static unsigned int __read_mostly max_vmid; > #else > /* First level P2M is alway 2 consecutive pages */ > #define P2M_ROOT_LEVEL 1 > @@ -1219,7 +1220,7 @@ static int p2m_alloc_table(struct domain *d) > > p2m->root = page; > > - p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; > + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48); > > /* > * Make sure that all TLBs corresponding to the new VMID are flushed > @@ -1230,20 +1231,47 @@ static int p2m_alloc_table(struct domain *d) > return 0; > } > > -#define MAX_VMID 256 > +#ifdef CONFIG_ARM_64 > +#define MAX_VMID (1UL << 16) > +#else > +#define MAX_VMID (1UL << 8) > +#endif Given that MAX_VMID on ARM64 can be either 256 or 65536, and given that this patch also introduces max_vmid, I find these #defines confusing. It is not obvious how max_vmid and MAX_VMID differ. I would go for something like the following: #define MAX_VMID_8 (1UL << 8) #define MAX_VMID_16 (1UL << 16) #ifdef CONFIG_ARM_64 #define MAX_VMID_ARCH MAX_VMID_16 #else #define MAX_VMID_ARCH MAX_VMID_8 #endif > #define INVALID_VMID 0 /* VMID 0 is reserved */ > > static spinlock_t vmid_alloc_lock = SPIN_LOCK_UNLOCKED; > > /* > - * VTTBR_EL2 VMID field is 8 bits. Using a bitmap here limits us to > - * 256 concurrent domains. > + * VTTBR_EL2 VMID field is 8 or 16 bits. Aarch64 supports 16-bit VMID. > + * Using a bitmap here limits us to 256 or 65536 (for Aarch64) concurrent > + * domains. > */ > 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; > + > +#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 > } > > 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 ) > + 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) > #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 > + > #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 { > -- > 2.7.4 >
Hi Stefano, On 19/11/16 04:34, Stefano Stabellini wrote: > On Fri, 11 Nov 2016, Bhupinder Thakur wrote: >> VMID space is increased to 16-bits from 8-bits in ARMv8 8.1 revision. >> This allows more than 256 VMs to be supported by Xen. >> >> This change adds support for 16-bit VMIDs in Xen based on whether the >> architecture supports it. >> >> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> [...] >> xen/arch/arm/p2m.c | 44 +++++++++++++++++++++++++++++++++++------ >> xen/include/asm-arm/p2m.h | 2 +- >> xen/include/asm-arm/processor.h | 17 +++++++++++++++- >> 3 files changed, 55 insertions(+), 8 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index cc5634b..6ed7e5c 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -19,6 +19,7 @@ static unsigned int __read_mostly p2m_root_order; >> static unsigned int __read_mostly p2m_root_level; >> #define P2M_ROOT_ORDER p2m_root_order >> #define P2M_ROOT_LEVEL p2m_root_level >> +static unsigned int __read_mostly max_vmid; >> #else >> /* First level P2M is alway 2 consecutive pages */ >> #define P2M_ROOT_LEVEL 1 >> @@ -1219,7 +1220,7 @@ static int p2m_alloc_table(struct domain *d) >> >> p2m->root = page; >> >> - p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; >> + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48); >> >> /* >> * Make sure that all TLBs corresponding to the new VMID are flushed >> @@ -1230,20 +1231,47 @@ static int p2m_alloc_table(struct domain *d) >> return 0; >> } >> >> -#define MAX_VMID 256 >> +#ifdef CONFIG_ARM_64 >> +#define MAX_VMID (1UL << 16) >> +#else >> +#define MAX_VMID (1UL << 8) >> +#endif > > Given that MAX_VMID on ARM64 can be either 256 or 65536, and given that > this patch also introduces max_vmid, I find these #defines confusing. It > is not obvious how max_vmid and MAX_VMID differ. I would go for > something like the following: > > #define MAX_VMID_8 (1UL << 8) > #define MAX_VMID_16 (1UL << 16) > #ifdef CONFIG_ARM_64 > #define MAX_VMID_ARCH MAX_VMID_16 > #else > #define MAX_VMID_ARCH MAX_VMID_8 > #endif MAX_VMID is mostly used to define the vmid at compilation time. However, with the support of 16 bits VMID, the size is now 8KB. So we would increase Xen footprint by 8KB on all AArch64 platform (even non ARMv8.1 compliant). I would much prefer to see this bitmap allocating at runtime. With that we could drop the use of MAX_VMID. Regards,
Hello Bhupinder, On 11/11/16 09:06, Bhupinder Thakur wrote: > VMID space is increased to 16-bits from 8-bits in ARMv8 8.1 revision. > This allows more than 256 VMs to be supported by Xen. > > This change adds support for 16-bit VMIDs in Xen based on whether the > architecture supports it. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > --- > xen/arch/arm/p2m.c | 44 +++++++++++++++++++++++++++++++++++------ > xen/include/asm-arm/p2m.h | 2 +- > xen/include/asm-arm/processor.h | 17 +++++++++++++++- > 3 files changed, 55 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index cc5634b..6ed7e5c 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -19,6 +19,7 @@ static unsigned int __read_mostly p2m_root_order; > static unsigned int __read_mostly p2m_root_level; > #define P2M_ROOT_ORDER p2m_root_order > #define P2M_ROOT_LEVEL p2m_root_level > +static unsigned int __read_mostly max_vmid; > #else > /* First level P2M is alway 2 consecutive pages */ > #define P2M_ROOT_LEVEL 1 > @@ -1219,7 +1220,7 @@ static int p2m_alloc_table(struct domain *d) > > p2m->root = page; > > - p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; > + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48); > > /* > * Make sure that all TLBs corresponding to the new VMID are flushed > @@ -1230,20 +1231,47 @@ static int p2m_alloc_table(struct domain *d) > return 0; > } > > -#define MAX_VMID 256 > +#ifdef CONFIG_ARM_64 > +#define MAX_VMID (1UL << 16) > +#else > +#define MAX_VMID (1UL << 8) > +#endif > + > #define INVALID_VMID 0 /* VMID 0 is reserved */ > > static spinlock_t vmid_alloc_lock = SPIN_LOCK_UNLOCKED; > > /* > - * VTTBR_EL2 VMID field is 8 bits. Using a bitmap here limits us to > - * 256 concurrent domains. > + * VTTBR_EL2 VMID field is 8 or 16 bits. Aarch64 supports 16-bit VMID. > + * Using a bitmap here limits us to 256 or 65536 (for Aarch64) concurrent > + * domains. > */ > 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. 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. > + > +#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. > } > > 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. > + 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. > #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. > + > #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,
On Tue, 22 Nov 2016, Julien Grall wrote: > Hi Stefano, > > On 19/11/16 04:34, Stefano Stabellini wrote: > > On Fri, 11 Nov 2016, Bhupinder Thakur wrote: > > > VMID space is increased to 16-bits from 8-bits in ARMv8 8.1 revision. > > > This allows more than 256 VMs to be supported by Xen. > > > > > > This change adds support for 16-bit VMIDs in Xen based on whether the > > > architecture supports it. > > > > > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > > [...] > > > > xen/arch/arm/p2m.c | 44 > > > +++++++++++++++++++++++++++++++++++------ > > > xen/include/asm-arm/p2m.h | 2 +- > > > xen/include/asm-arm/processor.h | 17 +++++++++++++++- > > > 3 files changed, 55 insertions(+), 8 deletions(-) > > > > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > > > index cc5634b..6ed7e5c 100644 > > > --- a/xen/arch/arm/p2m.c > > > +++ b/xen/arch/arm/p2m.c > > > @@ -19,6 +19,7 @@ static unsigned int __read_mostly p2m_root_order; > > > static unsigned int __read_mostly p2m_root_level; > > > #define P2M_ROOT_ORDER p2m_root_order > > > #define P2M_ROOT_LEVEL p2m_root_level > > > +static unsigned int __read_mostly max_vmid; > > > #else > > > /* First level P2M is alway 2 consecutive pages */ > > > #define P2M_ROOT_LEVEL 1 > > > @@ -1219,7 +1220,7 @@ static int p2m_alloc_table(struct domain *d) > > > > > > p2m->root = page; > > > > > > - p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) > > > << 48; > > > + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48); > > > > > > /* > > > * Make sure that all TLBs corresponding to the new VMID are flushed > > > @@ -1230,20 +1231,47 @@ static int p2m_alloc_table(struct domain *d) > > > return 0; > > > } > > > > > > -#define MAX_VMID 256 > > > +#ifdef CONFIG_ARM_64 > > > +#define MAX_VMID (1UL << 16) > > > +#else > > > +#define MAX_VMID (1UL << 8) > > > +#endif > > > > Given that MAX_VMID on ARM64 can be either 256 or 65536, and given that > > this patch also introduces max_vmid, I find these #defines confusing. It > > is not obvious how max_vmid and MAX_VMID differ. I would go for > > something like the following: > > > > #define MAX_VMID_8 (1UL << 8) > > #define MAX_VMID_16 (1UL << 16) > > #ifdef CONFIG_ARM_64 > > #define MAX_VMID_ARCH MAX_VMID_16 > > #else > > #define MAX_VMID_ARCH MAX_VMID_8 > > #endif > > MAX_VMID is mostly used to define the vmid at compilation time. > However, with the support of 16 bits VMID, the size is now 8KB. So we would > increase Xen footprint by 8KB on all AArch64 platform (even non ARMv8.1 > compliant). > > I would much prefer to see this bitmap allocating at runtime. With that we > could drop the use of MAX_VMID. You are right
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 >
On 23/11/16 10:38, Bhupinder Thakur wrote: > Hi, Hi Bhupinder, Can you configure your e-mail client to use ">" to quote the previous answer? Using tab makes the mail quite confusing. Thank you. > > 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. I am not asking the bitmap to be static but the number of vmid to be a constant (not a variable) for ARM32. I prefer to have the bitmap dynamic on both architecture because it is will be easier to maintain the code in long term. What I was asking for is: #ifdef CONFIG_ARM_64 /* The VMID is encoded on 8-bit minimum. */ static unsigned int __read_mostly max_vmid = MAX_VMID_8BIT; #define MAX_VMID max_vmid #else /* The VMID is always encoded on 8-bit */ #define MAX_VMID MAX_VMID_8BIT #endif The code would use MAX_VMID and the variable max_vmid would only be defined for arm64 breaking the compilation directly if used in the common code. [...] > #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 Yes, or any other name mentioning the register name in the define. This is how are defined all the register constant within this file. Regards,
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index cc5634b..6ed7e5c 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -19,6 +19,7 @@ static unsigned int __read_mostly p2m_root_order; static unsigned int __read_mostly p2m_root_level; #define P2M_ROOT_ORDER p2m_root_order #define P2M_ROOT_LEVEL p2m_root_level +static unsigned int __read_mostly max_vmid; #else /* First level P2M is alway 2 consecutive pages */ #define P2M_ROOT_LEVEL 1 @@ -1219,7 +1220,7 @@ static int p2m_alloc_table(struct domain *d) p2m->root = page; - p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48); /* * Make sure that all TLBs corresponding to the new VMID are flushed @@ -1230,20 +1231,47 @@ static int p2m_alloc_table(struct domain *d) return 0; } -#define MAX_VMID 256 +#ifdef CONFIG_ARM_64 +#define MAX_VMID (1UL << 16) +#else +#define MAX_VMID (1UL << 8) +#endif + #define INVALID_VMID 0 /* VMID 0 is reserved */ static spinlock_t vmid_alloc_lock = SPIN_LOCK_UNLOCKED; /* - * VTTBR_EL2 VMID field is 8 bits. Using a bitmap here limits us to - * 256 concurrent domains. + * VTTBR_EL2 VMID field is 8 or 16 bits. Aarch64 supports 16-bit VMID. + * Using a bitmap here limits us to 256 or 65536 (for Aarch64) concurrent + * domains. */ 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; + +#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 } 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 ) + 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) #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 + #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 {
VMID space is increased to 16-bits from 8-bits in ARMv8 8.1 revision. This allows more than 256 VMs to be supported by Xen. This change adds support for 16-bit VMIDs in Xen based on whether the architecture supports it. Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- xen/arch/arm/p2m.c | 44 +++++++++++++++++++++++++++++++++++------ xen/include/asm-arm/p2m.h | 2 +- xen/include/asm-arm/processor.h | 17 +++++++++++++++- 3 files changed, 55 insertions(+), 8 deletions(-)