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

2016-12-15 Thread Julien Grall



On 15/12/2016 17:35, Bhupinder Thakur wrote:

Hi Julien,


Hi Bhupinder,


Thanks for the clarification. One more confirmation: When the reviewer
asks you to add the reviewed-by tag to one of the patch in the series
then also the whole series should be resent with just the reviewed-by
tag added to that single patch.


No need, the committer will usually take care of adding the Reviewed-by 
tag while committing the series.


A rule of thumb is anything other than tag (reviewed-by, acked-by,...) 
will require to resend the series. If the patches contains few minor 
errors (e.g typoes), the committer will usually do it for you.


Cheers,

--
Julien Grall

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


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

2016-12-15 Thread Bhupinder Thakur
Hi Julien,

Thanks for the clarification. One more confirmation: When the reviewer
asks you to add the reviewed-by tag to one of the patch in the series
then also the whole series should be resent with just the reviewed-by
tag added to that single patch.

Regards,
Bhupinder


On 15 December 2016 at 22:53, Julien Grall  wrote:
>
>
> On 15/12/2016 17:14, Bhupinder Thakur wrote:
>>
>> Hi Julien,
>
>
> Hi Bhupinder,
>
>>
>> I did not send the 1st patch in the series as there is no change in
>> the first patch since the last one I sent. I understood from your last
>> mail that I need not send the patch again if there is no change.
>
>
> You misunderstood what I said:
>
> "No need to resend this series if there are no other comments."
>
> This series is composed of 2 patches. So if you modified one patch in the
> series you have to resend all the patches.
>
> There are an exceptions when a committer has applied a part of the series.
> In this case, you only have to resend those that haven't been committed.
>
> In the case of this series, the first patch was not committed. So you have
> to send the 2 patches.
>
>>
>> Should I send it again after updating the version as 5? Should I add
>> the reviewed-by tag in the first patch in the series?
>
>
> The reviewed-by tag has to be carried across all the version unless the
> patch has been heavily modified or the reviewer explicitly asked to
> withdraw.
>
> My comment on the previous version was only advice for the future on the
> netiquette :).
>
> I would prefer if you resend the series as the committer may not notice that
> there was a first patch.
>
> You don't need to bump the version (i.e v5 -> v6), instead add "RESEND" in
> the tag:
>
> [XEN VMID RESEND v5...]
>
> If you have any doubt how to proceed, feel free to ask it :).
>
> Cheers,
>
> --
> Julien Grall

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


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

2016-12-15 Thread Julien Grall



On 15/12/2016 17:14, Bhupinder Thakur wrote:

Hi Julien,


Hi Bhupinder,



I did not send the 1st patch in the series as there is no change in
the first patch since the last one I sent. I understood from your last
mail that I need not send the patch again if there is no change.


You misunderstood what I said:

"No need to resend this series if there are no other comments."

This series is composed of 2 patches. So if you modified one patch in 
the series you have to resend all the patches.


There are an exceptions when a committer has applied a part of the 
series. In this case, you only have to resend those that haven't been 
committed.


In the case of this series, the first patch was not committed. So you 
have to send the 2 patches.




Should I send it again after updating the version as 5? Should I add
the reviewed-by tag in the first patch in the series?


The reviewed-by tag has to be carried across all the version unless the 
patch has been heavily modified or the reviewer explicitly asked to 
withdraw.


My comment on the previous version was only advice for the future on the 
netiquette :).


I would prefer if you resend the series as the committer may not notice 
that there was a first patch.


You don't need to bump the version (i.e v5 -> v6), instead add "RESEND" 
in the tag:


[XEN VMID RESEND v5...]

If you have any doubt how to proceed, feel free to ask it :).

Cheers,

--
Julien Grall

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


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

2016-12-15 Thread Bhupinder Thakur
Hi Julien,

I did not send the 1st patch in the series as there is no change in
the first patch since the last one I sent. I understood from your last
mail that I need not send the patch again if there is no change.

Should I send it again after updating the version as 5? Should I add
the reviewed-by tag in the first patch in the series?

Regards,
Bhupinder

On 15 December 2016 at 22:32, Julien Grall  wrote:
> Hi,
>
> The patch indicates "2/2" but I can't find the first patch. Is it a mistake?
>
> Cheers,
>
>
> On 15/12/2016 16:55, 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 
>> ---
>>  xen/arch/arm/p2m.c  | 52
>> ++---
>>  xen/include/asm-arm/p2m.h   |  2 +-
>>  xen/include/asm-arm/processor.h | 18 +-
>>  3 files changed, 62 insertions(+), 10 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 6930f8c..61b9e49 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -7,6 +7,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -14,15 +15,25 @@
>>  #include 
>>  #include 
>>
>> +#define MAX_VMID_8_BIT  (1UL << 8)
>> +#define MAX_VMID_16_BIT (1UL << 16)
>> +
>> +#define INVALID_VMID 0 /* VMID 0 is reserved */
>> +
>>  #ifdef CONFIG_ARM_64
>>  static unsigned int __read_mostly p2m_root_order;
>>  static unsigned int __read_mostly p2m_root_level;
>>  #define P2M_ROOT_ORDERp2m_root_order
>>  #define P2M_ROOT_LEVEL p2m_root_level
>> +static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
>> +/* VMID is by default 8 bit width on AArch64 */
>> +#define MAX_VMID   max_vmid
>>  #else
>>  /* First level P2M is alway 2 consecutive pages */
>>  #define P2M_ROOT_LEVEL 1
>>  #define P2M_ROOT_ORDER1
>> +/* VMID is always 8 bit width on AArch32 */
>> +#define MAX_VMIDMAX_VMID_8_BIT
>>  #endif
>>
>>  #define P2M_ROOT_PAGES(1<> @@ -1217,7 +1228,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
>> @@ -1228,19 +1239,27 @@ static int p2m_alloc_table(struct domain *d)
>>  return 0;
>>  }
>>
>> -#define MAX_VMID 256
>> -#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 may support 16-bit VMID.
>> + * Using a bitmap here limits us to 256 or 65536 (for AArch64) concurrent
>> + * domains. The bitmap space will be allocated dynamically based on
>> + * whether 8 or 16 bit VMIDs are supported.
>>   */
>> -static DECLARE_BITMAP(vmid_mask, MAX_VMID);
>> +static unsigned long *vmid_mask;
>>
>>  static void p2m_vmid_allocator_init(void)
>>  {
>> +/*
>> + * allocate space for vmid_mask based on MAX_VMID
>> + */
>> +vmid_mask = xzalloc_array(unsigned long, BITS_TO_LONGS(MAX_VMID));
>> +
>> +if ( !vmid_mask )
>> +panic("Could not allocate VMID bitmap space");
>> +
>>  set_bit(INVALID_VMID, vmid_mask);
>>  }
>>
>> @@ -1630,20 +1649,36 @@ void __init setup_virt_paging(void)
>>
>>  unsigned int cpu;
>>  unsigned int pa_range = 0x10; /* Larger than any possible value */
>> +bool vmid_8_bit = false;
>>
>>  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 support 16 bit VMIDs.
>> */
>> +if ( info->mm64.vmid_bits != MM64_VMID_16_BITS_SUPPORT )
>> +vmid_8_bit = true;
>>  }
>>
>> +/*
>> + * If the flag is not set then it means all CPUs support 16-bit
>> + * VMIDs.
>> + */
>> +if ( !vmid_8_bit )
>> +max_vmid = MAX_VMID_16_BIT;
>> +
>>  /* pa_range is 4 bits, but the defined encodings are only 3 bits */
>>  if ( pa_range >= ARRAY_SIZE(pa_range_info) ||
>> !pa_range_info[pa_range].pabits )
>>  panic("Unknown encoding of ID_AA64MMFR0_EL1.PARange %x\n",
>> pa_range);
>>
>>  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_16_BIT )
>> +val |= VTCR_VS;
>>  val |= VTCR_SL0(pa_range_info[pa_range].sl0);
>>  val |= VTCR_T0SZ(pa_range_info[pa_range].t0sz);
>>
>> @@ -1651,9 +

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

2016-12-15 Thread Julien Grall

Hi,

The patch indicates "2/2" but I can't find the first patch. Is it a mistake?

Cheers,

On 15/12/2016 16:55, 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 
---
 xen/arch/arm/p2m.c  | 52 ++---
 xen/include/asm-arm/p2m.h   |  2 +-
 xen/include/asm-arm/processor.h | 18 +-
 3 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6930f8c..61b9e49 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -14,15 +15,25 @@
 #include 
 #include 

+#define MAX_VMID_8_BIT  (1UL << 8)
+#define MAX_VMID_16_BIT (1UL << 16)
+
+#define INVALID_VMID 0 /* VMID 0 is reserved */
+
 #ifdef CONFIG_ARM_64
 static unsigned int __read_mostly p2m_root_order;
 static unsigned int __read_mostly p2m_root_level;
 #define P2M_ROOT_ORDERp2m_root_order
 #define P2M_ROOT_LEVEL p2m_root_level
+static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
+/* VMID is by default 8 bit width on AArch64 */
+#define MAX_VMID   max_vmid
 #else
 /* First level P2M is alway 2 consecutive pages */
 #define P2M_ROOT_LEVEL 1
 #define P2M_ROOT_ORDER1
+/* VMID is always 8 bit width on AArch32 */
+#define MAX_VMIDMAX_VMID_8_BIT
 #endif

 #define P2M_ROOT_PAGES(1vttbr = 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
@@ -1228,19 +1239,27 @@ static int p2m_alloc_table(struct domain *d)
 return 0;
 }

-#define MAX_VMID 256
-#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 may support 16-bit VMID.
+ * Using a bitmap here limits us to 256 or 65536 (for AArch64) concurrent
+ * domains. The bitmap space will be allocated dynamically based on
+ * whether 8 or 16 bit VMIDs are supported.
  */
-static DECLARE_BITMAP(vmid_mask, MAX_VMID);
+static unsigned long *vmid_mask;

 static void p2m_vmid_allocator_init(void)
 {
+/*
+ * allocate space for vmid_mask based on MAX_VMID
+ */
+vmid_mask = xzalloc_array(unsigned long, BITS_TO_LONGS(MAX_VMID));
+
+if ( !vmid_mask )
+panic("Could not allocate VMID bitmap space");
+
 set_bit(INVALID_VMID, vmid_mask);
 }

@@ -1630,20 +1649,36 @@ void __init setup_virt_paging(void)

 unsigned int cpu;
 unsigned int pa_range = 0x10; /* Larger than any possible value */
+bool vmid_8_bit = false;

 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 support 16 bit VMIDs. */
+if ( info->mm64.vmid_bits != MM64_VMID_16_BITS_SUPPORT )
+vmid_8_bit = true;
 }

+/*
+ * If the flag is not set then it means all CPUs support 16-bit
+ * VMIDs.
+ */
+if ( !vmid_8_bit )
+max_vmid = MAX_VMID_16_BIT;
+
 /* pa_range is 4 bits, but the defined encodings are only 3 bits */
 if ( pa_range >= ARRAY_SIZE(pa_range_info) || 
!pa_range_info[pa_range].pabits )
 panic("Unknown encoding of ID_AA64MMFR0_EL1.PARange %x\n", pa_range);

 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_16_BIT )
+val |= VTCR_VS;
 val |= VTCR_SL0(pa_range_info[pa_range].sl0);
 val |= VTCR_T0SZ(pa_range_info[pa_range].t0sz);

@@ -1651,9 +1686,10 @@ void __init setup_virt_paging(void)
 p2m_root_level = 2 - pa_range_info[pa_range].sl0;
 p2m_ipa_bits = 64 - pa_range_info[pa_range].t0sz;

-printk("P2M: %d-bit IPA with %d-bit PA\n",
+printk("P2M: %d-bit IPA with %d-bit PA and %d-bit VMID\n",
p2m_ipa_bits,
-   pa_range_info[pa_range].pabits);
+   pa_range_info[pa_range].pabits,
+   ( MAX_VMID == MAX_VMID_16_BIT )?16:8);
 #endif
 printk("P2M: %d levels with order-%d root, VTCR 0x%lx\n",
4 - P2M_ROOT_LEVEL, P2M_ROOT_ORDER, val);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 0987be2..9de55fc 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 Tab