[PATCH] xen/arm: Restricted access to IFSR32_EL2 and FPEXC32_EL2

[PATCH] xen/arm: Restricted access to IFSR32_EL2 and FPEXC32_EL2

[vijay.kilari]

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

Some platforms does not support aarch32 mode. So access only if
platform supports it. Accessing these registers will
raise exception

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
---
 xen/arch/arm/arm64/vfp.c |    6 ++++--
 xen/arch/arm/traps.c     |    7 +++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/arm64/vfp.c b/xen/arch/arm/arm64/vfp.c
index 3cd2b1b..c550a68 100644
--- a/xen/arch/arm/arm64/vfp.c
+++ b/xen/arch/arm/arm64/vfp.c
@@ -28,7 +28,8 @@ void vfp_save_state(struct vcpu *v)
 
     v->arch.vfp.fpsr = READ_SYSREG32(FPSR);
     v->arch.vfp.fpcr = READ_SYSREG32(FPCR);
-    v->arch.vfp.fpexc32_el2 = READ_SYSREG32(FPEXC32_EL2);
+    if ( cpu_has_el2_32 )
+        v->arch.vfp.fpexc32_el2 = READ_SYSREG32(FPEXC32_EL2);
 }
 
 void vfp_restore_state(struct vcpu *v)
@@ -56,5 +57,6 @@ void vfp_restore_state(struct vcpu *v)
 
     WRITE_SYSREG32(v->arch.vfp.fpsr, FPSR);
     WRITE_SYSREG32(v->arch.vfp.fpcr, FPCR);
-    WRITE_SYSREG32(v->arch.vfp.fpexc32_el2, FPEXC32_EL2);
+    if ( cpu_has_el2_32 )
+        WRITE_SYSREG32(v->arch.vfp.fpexc32_el2, FPEXC32_EL2);
 }
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1ac01ee..d2748d9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -37,6 +37,7 @@
 #include <asm/cpregs.h>
 #include <asm/psci.h>
 #include <asm/mmio.h>
+#include <asm/cpufeature.h>
 
 #include "decode.h"
 #include "vtimer.h"
@@ -364,7 +365,8 @@ static void inject_abt32_exception(struct cpu_user_regs *regs,
         register_t far = READ_SYSREG(FAR_EL1) & 0xffffffffUL;
         far |= addr << 32;
         WRITE_SYSREG(far, FAR_EL1);
-        WRITE_SYSREG(fsr, IFSR32_EL2);
+        if ( cpu_has_el2_32 )
+            WRITE_SYSREG(fsr, IFSR32_EL2);
 
 #endif
     }
@@ -756,7 +758,8 @@ void show_registers(struct cpu_user_regs *regs)
 #else
     ctxt.far = READ_SYSREG(FAR_EL1);
     ctxt.esr_el1 = READ_SYSREG(ESR_EL1);
-    ctxt.ifsr32_el2 = READ_SYSREG(IFSR32_EL2);
+    if ( cpu_has_el2_32 )
+        ctxt.ifsr32_el2 = READ_SYSREG(IFSR32_EL2);
 #endif
     ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
 
-- 
1.7.9.5

Re: [PATCH] xen/arm: Restricted access to IFSR32_EL2 and FPEXC32_EL2

[Julien Grall]

Hi Vijaya,

On 05/27/2014 11:37 AM, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Some platforms does not support aarch32 mode. So access only if
> platform supports it. Accessing these registers will
> raise exception
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
>  xen/arch/arm/arm64/vfp.c |    6 ++++--
>  xen/arch/arm/traps.c     |    7 +++++--
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/vfp.c b/xen/arch/arm/arm64/vfp.c
> index 3cd2b1b..c550a68 100644
> --- a/xen/arch/arm/arm64/vfp.c
> +++ b/xen/arch/arm/arm64/vfp.c
> @@ -28,7 +28,8 @@ void vfp_save_state(struct vcpu *v)
>  
>      v->arch.vfp.fpsr = READ_SYSREG32(FPSR);
>      v->arch.vfp.fpcr = READ_SYSREG32(FPCR);
> -    v->arch.vfp.fpexc32_el2 = READ_SYSREG32(FPEXC32_EL2);
> +    if ( cpu_has_el2_32 )

You should use is_32bit_domain here. If your platform doesn't support
aarch32 mode, then we should not create 32bit domain.

I think you will have to deny somewhere the domain creation in this case.

Regards,

-- 
Julien Grall

Re: [PATCH] xen/arm: Restricted access to IFSR32_EL2 and FPEXC32_EL2

[Vijay Kilari]

Hi Julien,

On Tue, May 27, 2014 at 4:51 PM, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Vijaya,
>
> On 05/27/2014 11:37 AM, vijay.kilari@gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>>
>> Some platforms does not support aarch32 mode. So access only if
>> platform supports it. Accessing these registers will
>> raise exception
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>> ---
>>  xen/arch/arm/arm64/vfp.c |    6 ++++--
>>  xen/arch/arm/traps.c     |    7 +++++--
>>  2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/vfp.c b/xen/arch/arm/arm64/vfp.c
>> index 3cd2b1b..c550a68 100644
>> --- a/xen/arch/arm/arm64/vfp.c
>> +++ b/xen/arch/arm/arm64/vfp.c
>> @@ -28,7 +28,8 @@ void vfp_save_state(struct vcpu *v)
>>
>>      v->arch.vfp.fpsr = READ_SYSREG32(FPSR);
>>      v->arch.vfp.fpcr = READ_SYSREG32(FPCR);
>> -    v->arch.vfp.fpexc32_el2 = READ_SYSREG32(FPEXC32_EL2);
>> +    if ( cpu_has_el2_32 )
>
> You should use is_32bit_domain here. If your platform doesn't support
> aarch32 mode, then we should not create 32bit domain.

I agree as per ARM Spec, it says if aarch32 mode is supported and so if
the domain is 32-bit then only these registers are allowed

"The following registers are defined to allow access from AArch64 state to
registers that are only used in AArch32 state

SDER32_EL3    If EL1 cannot use AArch32, this register is UNDEFINED.
DACR32_EL2    If EL1 cannot use AArch32, this register is UNDEFINED.
IFSR32_EL2     If EL1 cannot use AArch32, this register is UNDEFINED.
FPEXC32_EL2  If EL1 cannot use AArch32, this register is UNDEFINED."


>
> I think you will have to deny somewhere the domain creation in this case.

Here I found that for dom0 a check is made in construct_dom0() where
domain type is being set to allow only 64-bit domains if platform supports
only aarch64 bit mode.

However for domU, arch.type is being set using  hypercall
"XEN_DOMCTL_set_address_size:" defined in arch/arm/arm64/domctl.c in
subarch_do_domctl()

So questions are :
1) Is this the hypercall where domain type is set?. If so, can we
return error for this
    hypercall if domU type is not supported by Xen?
2) Can xl tool allow to load domU that platform supports?. May be it require
    hardware info here which may not be right way to do.

Regards
Vijay

Re: [PATCH] xen/arm: Restricted access to IFSR32_EL2 and FPEXC32_EL2

[Ian Campbell]

On Wed, 2014-09-17 at 14:29 +0530, Vijay Kilari wrote:
> So questions are :
> 1) Is this the hypercall where domain type is set?.

For domU, yes.

> If so, can we return error for this hypercall if domU type is not supported by Xen?

I think that would indeed be the right thing to do.

In addition we need to modifiy arch_get_xen_caps to only include
xen-X.Y-armv7l in the result if the hardware does in fact support domU
as 32-bit. The toolstack will consult this when trying to figure out if
a domain can be built (I think the end result is that
XEN_DOMCTL_set_address_size should never fail because it should never be
called if it could fail, but add the check anyway)

> 2) Can xl tool allow to load domU that platform supports?. May be it require
>     hardware info here which may not be right way to do.

Not sure what you are asking here. If you try and build a 32-bit domain
on a 64-bit-only platform then the above changes will cause it to fail,
which is the right thing to happen. Does that answer your question?

Ian.