[PATCH v2] xen/arm: check on domain type against hardware support

[PATCH v2] xen/arm: check on domain type against hardware support

[vijay.kilari]

From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

Some arm64 platforms implement only aarch64 mode. So allow
domains that are only 64-bit

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
---
v2:-Fixed coding style comments
   -Make cpu_has_aarch32 check for both A32 and T32
   -Updated aarch32 capabilities info print
---
 xen/arch/arm/arm64/domctl.c      |    3 +++
 xen/arch/arm/domain_build.c      |    7 +++++++
 xen/arch/arm/setup.c             |   10 +++++++---
 xen/include/asm-arm/cpufeature.h |    3 ++-
 4 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
index 41e2562..c0ff248 100644
--- a/xen/arch/arm/arm64/domctl.c
+++ b/xen/arch/arm/arm64/domctl.c
@@ -11,6 +11,7 @@
 #include <xen/sched.h>
 #include <xen/hypercall.h>
 #include <public/domctl.h>
+#include <asm/cpufeature.h>
 
 static long switch_mode(struct domain *d, enum domain_type type)
 {
@@ -35,6 +36,8 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
         switch ( domctl->u.address_size.size )
         {
         case 32:
+            if ( !cpu_has_el1_32 )
+                return -EINVAL;
             return switch_mode(d, DOMAIN_32BIT);
         case 64:
             return switch_mode(d, DOMAIN_64BIT);
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 90abc3a..138ca89 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -17,6 +17,7 @@
 #include <asm/platform.h>
 #include <asm/psci.h>
 #include <asm/setup.h>
+#include <asm/cpufeature.h>
 
 #include <asm/gic.h>
 #include <xen/irq.h>
@@ -1274,6 +1275,12 @@ int construct_dom0(struct domain *d)
         return rc;
 
 #ifdef CONFIG_ARM_64
+    /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
+    if ( !(cpu_has_el1_32) && kinfo.type == DOMAIN_32BIT )
+    {
+        printk("Platform does not support 32-bit domain\n");
+        return -EINVAL;
+    }
     d->arch.type = kinfo.type;
 #endif
 
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 446de8a..ceb5af8 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -127,8 +127,9 @@ static void __init processor_id(void)
         printk("32-bit Execution:\n");
         printk("  Processor Features: %08"PRIx32":%08"PRIx32"\n",
                boot_cpu_data.pfr32.bits[0], boot_cpu_data.pfr32.bits[1]);
-        printk("    Instruction Sets:%s%s%s%s%s\n",
+        printk("    Instruction Sets:%s%s%s%s%s%s\n",
                cpu_has_aarch32 ? " AArch32" : "",
+               cpu_has_a32 ? " A32" : "",
                cpu_has_thumb ? " Thumb" : "",
                cpu_has_thumb2 ? " Thumb-2" : "",
                cpu_has_thumbee ? " ThumbEE" : "",
@@ -851,8 +852,11 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
     snprintf(s, sizeof(s), "xen-%d.%d-aarch64 ", major, minor);
     safe_strcat(*info, s);
 #endif
-    snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
-    safe_strcat(*info, s);
+    if ( cpu_has_aarch32 )
+    {
+        snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
+        safe_strcat(*info, s);
+    }
 }
 
 /*
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 7a6d3de..e605bf5 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -21,11 +21,12 @@
 #define cpu_feature32(c, feat)         ((c)->pfr32.feat)
 #define boot_cpu_feature32(feat)       (boot_cpu_data.pfr32.feat)
 
-#define cpu_has_aarch32   (boot_cpu_feature32(arm) == 1)
+#define cpu_has_a32       (boot_cpu_feature32(arm) == 1)
 #define cpu_has_thumb     (boot_cpu_feature32(thumb) >= 1)
 #define cpu_has_thumb2    (boot_cpu_feature32(thumb) >= 3)
 #define cpu_has_jazelle   (boot_cpu_feature32(jazelle) >= 0)
 #define cpu_has_thumbee   (boot_cpu_feature32(thumbee) == 1)
+#define cpu_has_aarch32   (cpu_has_a32 || cpu_has_thumb)
 
 #ifdef CONFIG_ARM_32
 #define cpu_has_gentimer  (boot_cpu_feature32(gentimer) == 1)
-- 
1.7.9.5

Re: [PATCH v2] xen/arm: check on domain type against hardware support

[Ian Campbell]

On Fri, 2014-09-26 at 11:28 +0530, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Some arm64 platforms implement only aarch64 mode. So allow
> domains that are only 64-bit
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I think this is a bug fix so we should take it for 4.5.

Ian.

> ---
> v2:-Fixed coding style comments
>    -Make cpu_has_aarch32 check for both A32 and T32
>    -Updated aarch32 capabilities info print
> ---
>  xen/arch/arm/arm64/domctl.c      |    3 +++
>  xen/arch/arm/domain_build.c      |    7 +++++++
>  xen/arch/arm/setup.c             |   10 +++++++---
>  xen/include/asm-arm/cpufeature.h |    3 ++-
>  4 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
> index 41e2562..c0ff248 100644
> --- a/xen/arch/arm/arm64/domctl.c
> +++ b/xen/arch/arm/arm64/domctl.c
> @@ -11,6 +11,7 @@
>  #include <xen/sched.h>
>  #include <xen/hypercall.h>
>  #include <public/domctl.h>
> +#include <asm/cpufeature.h>
>  
>  static long switch_mode(struct domain *d, enum domain_type type)
>  {
> @@ -35,6 +36,8 @@ long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>          switch ( domctl->u.address_size.size )
>          {
>          case 32:
> +            if ( !cpu_has_el1_32 )
> +                return -EINVAL;
>              return switch_mode(d, DOMAIN_32BIT);
>          case 64:
>              return switch_mode(d, DOMAIN_64BIT);
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 90abc3a..138ca89 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -17,6 +17,7 @@
>  #include <asm/platform.h>
>  #include <asm/psci.h>
>  #include <asm/setup.h>
> +#include <asm/cpufeature.h>
>  
>  #include <asm/gic.h>
>  #include <xen/irq.h>
> @@ -1274,6 +1275,12 @@ int construct_dom0(struct domain *d)
>          return rc;
>  
>  #ifdef CONFIG_ARM_64
> +    /* if aarch32 mode is not supported at EL1 do not allow 32-bit domain */
> +    if ( !(cpu_has_el1_32) && kinfo.type == DOMAIN_32BIT )
> +    {
> +        printk("Platform does not support 32-bit domain\n");
> +        return -EINVAL;
> +    }
>      d->arch.type = kinfo.type;
>  #endif
>  
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 446de8a..ceb5af8 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -127,8 +127,9 @@ static void __init processor_id(void)
>          printk("32-bit Execution:\n");
>          printk("  Processor Features: %08"PRIx32":%08"PRIx32"\n",
>                 boot_cpu_data.pfr32.bits[0], boot_cpu_data.pfr32.bits[1]);
> -        printk("    Instruction Sets:%s%s%s%s%s\n",
> +        printk("    Instruction Sets:%s%s%s%s%s%s\n",
>                 cpu_has_aarch32 ? " AArch32" : "",
> +               cpu_has_a32 ? " A32" : "",
>                 cpu_has_thumb ? " Thumb" : "",
>                 cpu_has_thumb2 ? " Thumb-2" : "",
>                 cpu_has_thumbee ? " ThumbEE" : "",
> @@ -851,8 +852,11 @@ void arch_get_xen_caps(xen_capabilities_info_t *info)
>      snprintf(s, sizeof(s), "xen-%d.%d-aarch64 ", major, minor);
>      safe_strcat(*info, s);
>  #endif
> -    snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
> -    safe_strcat(*info, s);
> +    if ( cpu_has_aarch32 )
> +    {
> +        snprintf(s, sizeof(s), "xen-%d.%d-armv7l ", major, minor);
> +        safe_strcat(*info, s);
> +    }
>  }
>  
>  /*
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 7a6d3de..e605bf5 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -21,11 +21,12 @@
>  #define cpu_feature32(c, feat)         ((c)->pfr32.feat)
>  #define boot_cpu_feature32(feat)       (boot_cpu_data.pfr32.feat)
>  
> -#define cpu_has_aarch32   (boot_cpu_feature32(arm) == 1)
> +#define cpu_has_a32       (boot_cpu_feature32(arm) == 1)
>  #define cpu_has_thumb     (boot_cpu_feature32(thumb) >= 1)
>  #define cpu_has_thumb2    (boot_cpu_feature32(thumb) >= 3)
>  #define cpu_has_jazelle   (boot_cpu_feature32(jazelle) >= 0)
>  #define cpu_has_thumbee   (boot_cpu_feature32(thumbee) == 1)
> +#define cpu_has_aarch32   (cpu_has_a32 || cpu_has_thumb)
>  
>  #ifdef CONFIG_ARM_32
>  #define cpu_has_gentimer  (boot_cpu_feature32(gentimer) == 1)

Re: [PATCH v2] xen/arm: check on domain type against hardware support

[Julien Grall]

Hi Ian,

On 09/29/2014 01:23 PM, Ian Campbell wrote:
> On Fri, 2014-09-26 at 11:28 +0530, vijay.kilari@gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>>
>> Some arm64 platforms implement only aarch64 mode. So allow
>> domains that are only 64-bit
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> I think this is a bug fix so we should take it for 4.5.

I made a quick comment in response to your answer on V1.

In any case, I agree that it should go in 4.5.

Regards,

-- 
Julien Grall

Re: [PATCH v2] xen/arm: check on domain type against hardware support

[Konrad Rzeszutek Wilk]

On Mon, Sep 29, 2014 at 01:28:16PM +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 09/29/2014 01:23 PM, Ian Campbell wrote:
> > On Fri, 2014-09-26 at 11:28 +0530, vijay.kilari@gmail.com wrote:
> >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >>
> >> Some arm64 platforms implement only aarch64 mode. So allow
> >> domains that are only 64-bit
> >>
> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > I think this is a bug fix so we should take it for 4.5.
> 
> I made a quick comment in response to your answer on V1.
> 
> In any case, I agree that it should go in 4.5.

Released-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Regards,
> 
> -- 
> Julien Grall
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2] xen/arm: check on domain type against hardware support

[Ian Campbell]

On Mon, 2014-09-29 at 13:28 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 09/29/2014 01:23 PM, Ian Campbell wrote:
> > On Fri, 2014-09-26 at 11:28 +0530, vijay.kilari@gmail.com wrote:
> >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >>
> >> Some arm64 platforms implement only aarch64 mode. So allow
> >> domains that are only 64-bit
> >>
> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > I think this is a bug fix so we should take it for 4.5.
> 
> I made a quick comment in response to your answer on V1.

Vijay, based on Julien's comments on v1 I'm inclined to
s/cpu_has_a32/cpu_has_arm/, so we refer to arm+thumb not a32+thumb which
combines to different naming schemes. An alternative would be
s/cpu_has_thumb/cpu_has_t32/ but I don't like that much.

Does that work for you? I can make the trivial change so no need to
resend (unless you want to).

Ian.

Re: [PATCH v2] xen/arm: check on domain type against hardware support

[Vijay Kilari]

On Wed, Oct 1, 2014 at 4:25 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2014-09-29 at 13:28 +0100, Julien Grall wrote:
>> Hi Ian,
>>
>> On 09/29/2014 01:23 PM, Ian Campbell wrote:
>> > On Fri, 2014-09-26 at 11:28 +0530, vijay.kilari@gmail.com wrote:
>> >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>> >>
>> >> Some arm64 platforms implement only aarch64 mode. So allow
>> >> domains that are only 64-bit
>> >>
>> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>> >
>> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
>> >
>> > I think this is a bug fix so we should take it for 4.5.
>>
>> I made a quick comment in response to your answer on V1.
>
> Vijay, based on Julien's comments on v1 I'm inclined to
> s/cpu_has_a32/cpu_has_arm/, so we refer to arm+thumb not a32+thumb which
> combines to different naming schemes. An alternative would be
> s/cpu_has_thumb/cpu_has_t32/ but I don't like that much.
>
> Does that work for you? I can make the trivial change so no need to
> resend (unless you want to).

Sorry for late reply, I was on vacation.
I am ok with this change.

Regards
Vijay

Re: [PATCH v2] xen/arm: check on domain type against hardware support

[Ian Campbell]

On Mon, 2014-10-06 at 12:13 +0530, Vijay Kilari wrote:
> On Wed, Oct 1, 2014 at 4:25 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Mon, 2014-09-29 at 13:28 +0100, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 09/29/2014 01:23 PM, Ian Campbell wrote:
> >> > On Fri, 2014-09-26 at 11:28 +0530, vijay.kilari@gmail.com wrote:
> >> >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >> >>
> >> >> Some arm64 platforms implement only aarch64 mode. So allow
> >> >> domains that are only 64-bit
> >> >>
> >> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >> >
> >> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >> >
> >> > I think this is a bug fix so we should take it for 4.5.
> >>
> >> I made a quick comment in response to your answer on V1.
> >
> > Vijay, based on Julien's comments on v1 I'm inclined to
> > s/cpu_has_a32/cpu_has_arm/, so we refer to arm+thumb not a32+thumb which
> > combines to different naming schemes. An alternative would be
> > s/cpu_has_thumb/cpu_has_t32/ but I don't like that much.
> >
> > Does that work for you? I can make the trivial change so no need to
> > resend (unless you want to).
> 
> Sorry for late reply, I was on vacation.
> I am ok with this change.

Thanks, I've committed this change.