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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxx>
> 
> [...]
> 
> > >  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

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