| Message ID | 1401434911-26992-15-git-send-email-edgar.iglesias@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 30 May 2014 02:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target-arm/cpu.h | 1 + > target-arm/helper-a64.c | 1 + > target-arm/helper.c | 6 ++++++ > target-arm/helper.h | 1 + > target-arm/internals.h | 6 ++++++ > target-arm/op_helper.c | 11 +++++++++++ > target-arm/translate-a64.c | 10 ++++++++++ > 7 files changed, 36 insertions(+) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 1a26ed4..b3631f2 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -52,6 +52,7 @@ > #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */ > #define EXCP_STREX 10 > #define EXCP_HVC 11 /* HyperVisor Call */ > +#define EXCP_SMC 12 /* Secure Monitor Call */ > > #define ARMV7M_EXCP_RESET 1 > #define ARMV7M_EXCP_NMI 2 > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index 974fa66..3894a6f 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > case EXCP_UDEF: > case EXCP_SWI: > case EXCP_HVC: > + case EXCP_SMC: > env->cp15.esr_el[new_el] = env->exception.syndrome; > break; > case EXCP_IRQ: > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 5b2070c..35091ea 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, > unsigned int excp_idx) > case EXCP_HVC: > target_el = MAX(target_el, 2); > break; > + case EXCP_SMC: > + target_el = 3; > + if (env->cp15.hcr_el2 & HCR_TSC) { > + target_el = 2; > + } > + break; > } > return target_el; > } > diff --git a/target-arm/helper.h b/target-arm/helper.h > index fb711be..6c3d84d 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, > i32) > DEF_HELPER_1(wfi, void, env) > DEF_HELPER_1(wfe, void, env) > DEF_HELPER_2(hvc, void, env, i32) > +DEF_HELPER_2(smc, void, env, i32) > > DEF_HELPER_3(cpsr_write, void, env, i32, i32) > DEF_HELPER_1(cpsr_read, i32, env) > diff --git a/target-arm/internals.h b/target-arm/internals.h > index b08381c..e50a68e 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -54,6 +54,7 @@ static const char * const excnames[] = { > [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage", > [EXCP_STREX] = "QEMU intercept of STREX", > [EXCP_HVC] = "Hypervisor Call", > + [EXCP_SMC] = "Secure Monitor Call", > }; > > static inline void arm_log_exception(int idx) > @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16) > return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > 0xffff); > } > > +static inline uint32_t syn_aa64_smc(uint32_t imm16) > +{ > + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > 0xffff); > +} > + > static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) > { > return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 6bf34b0..6840828 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome) > raise_exception(env, EXCP_HVC); > } > > +void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > +{ > + /* We've already checked that EL3 exists at translation time. */ > + if (env->cp15.scr_el3 & SCR_SMD) { > In ARMv7 isn't this is this only a valid check if the virtualization extension is present? If so, should we be checking for the virt extension on AArch32 prior to raising undefined? > + env->exception.syndrome = syn_uncategorized(); > + raise_exception(env, EXCP_UDEF); > + } > + env->exception.syndrome = syndrome; > + raise_exception(env, EXCP_SMC); > +} > + > void HELPER(exception_return)(CPUARMState *env) > { > int cur_el = arm_current_pl(env); > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 3981ee1..a02fe06 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t > insn) > gen_helper_hvc(cpu_env, tmp); > tcg_temp_free_i32(tmp); > break; > + case 3: > + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == > 0) { > + unallocated_encoding(s); > + break; > + } > + tmp = tcg_const_i32(syn_aa64_smc(imm16)); > + gen_a64_set_pc_im(s->pc); > + gen_helper_smc(cpu_env, tmp); > + tcg_temp_free_i32(tmp); > + break; > default: > unallocated_encoding(s); > break; > -- > 1.8.3.2 > >
On Fri, May 30, 2014 at 11:50:23AM -0500, Greg Bellows wrote: > On 30 May 2014 02:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > --- > > target-arm/cpu.h | 1 + > > target-arm/helper-a64.c | 1 + > > target-arm/helper.c | 6 ++++++ > > target-arm/helper.h | 1 + > > target-arm/internals.h | 6 ++++++ > > target-arm/op_helper.c | 11 +++++++++++ > > target-arm/translate-a64.c | 10 ++++++++++ > > 7 files changed, 36 insertions(+) > > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > > index 1a26ed4..b3631f2 100644 > > --- a/target-arm/cpu.h > > +++ b/target-arm/cpu.h > > @@ -52,6 +52,7 @@ > > #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */ > > #define EXCP_STREX 10 > > #define EXCP_HVC 11 /* HyperVisor Call */ > > +#define EXCP_SMC 12 /* Secure Monitor Call */ > > > > #define ARMV7M_EXCP_RESET 1 > > #define ARMV7M_EXCP_NMI 2 > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > > index 974fa66..3894a6f 100644 > > --- a/target-arm/helper-a64.c > > +++ b/target-arm/helper-a64.c > > @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > case EXCP_UDEF: > > case EXCP_SWI: > > case EXCP_HVC: > > + case EXCP_SMC: > > env->cp15.esr_el[new_el] = env->exception.syndrome; > > break; > > case EXCP_IRQ: > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 5b2070c..35091ea 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, > > unsigned int excp_idx) > > case EXCP_HVC: > > target_el = MAX(target_el, 2); > > break; > > + case EXCP_SMC: > > + target_el = 3; > > + if (env->cp15.hcr_el2 & HCR_TSC) { > > + target_el = 2; > > + } > > + break; > > } > > return target_el; > > } > > diff --git a/target-arm/helper.h b/target-arm/helper.h > > index fb711be..6c3d84d 100644 > > --- a/target-arm/helper.h > > +++ b/target-arm/helper.h > > @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, > > i32) > > DEF_HELPER_1(wfi, void, env) > > DEF_HELPER_1(wfe, void, env) > > DEF_HELPER_2(hvc, void, env, i32) > > +DEF_HELPER_2(smc, void, env, i32) > > > > DEF_HELPER_3(cpsr_write, void, env, i32, i32) > > DEF_HELPER_1(cpsr_read, i32, env) > > diff --git a/target-arm/internals.h b/target-arm/internals.h > > index b08381c..e50a68e 100644 > > --- a/target-arm/internals.h > > +++ b/target-arm/internals.h > > @@ -54,6 +54,7 @@ static const char * const excnames[] = { > > [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage", > > [EXCP_STREX] = "QEMU intercept of STREX", > > [EXCP_HVC] = "Hypervisor Call", > > + [EXCP_SMC] = "Secure Monitor Call", > > }; > > > > static inline void arm_log_exception(int idx) > > @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16) > > return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > > 0xffff); > > } > > > > +static inline uint32_t syn_aa64_smc(uint32_t imm16) > > +{ > > + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > > 0xffff); > > +} > > + > > static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) > > { > > return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > > index 6bf34b0..6840828 100644 > > --- a/target-arm/op_helper.c > > +++ b/target-arm/op_helper.c > > @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome) > > raise_exception(env, EXCP_HVC); > > } > > > > +void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > > +{ > > + /* We've already checked that EL3 exists at translation time. */ > > + if (env->cp15.scr_el3 & SCR_SMD) { > > > > In ARMv7 isn't this is this only a valid check if the virtualization > extension is present? If so, should we be checking for the virt extension > on AArch32 prior to raising undefined? My understanding is that on AArch32 SCR.SMD only applies to NS mode and only if HCR.TSC=0 (always the case if EL2 does not exist but can also be zero even if EL2 exists). So it's not dependant on virtual extensions/EL2 non-existance? Am I missing something? I'm not adding AArch32 SMC here but am happy to try to get it right in preparation for the AArch32 support. The patch misses that the HCR.TSC routing of SMC to EL2 takes priority over SMD for NS EL1. For AArch32, I need to check for non-secure state aswell. I'll update those for v2. Need to run a few tests but I'm considering something along these lines: void HELPER(smc)(CPUARMState *env, uint32_t syndrome) { int cur_el = arm_current_pl(env); /* FIXME: Use real secure state. */ bool secure = false; bool smd = env->cp15.scr_el3 & SCR_SMD; /* On AArch32, SMD only applies to NS mode. */ bool udef = is_a64(env) ? smd : !secure && smd; /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */ if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) { udef = false; } /* We've already checked that EL3 exists at translation time. */ if (udef) { env->exception.syndrome = syn_uncategorized(); raise_exception(env, EXCP_UDEF); } env->exception.syndrome = syndrome; raise_exception(env, EXCP_SMC); } Thanks for the comments, Edgar > > > > + env->exception.syndrome = syn_uncategorized(); > > + raise_exception(env, EXCP_UDEF); > > + } > > + env->exception.syndrome = syndrome; > > + raise_exception(env, EXCP_SMC); > > +} > > + > > void HELPER(exception_return)(CPUARMState *env) > > { > > int cur_el = arm_current_pl(env); > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > > index 3981ee1..a02fe06 100644 > > --- a/target-arm/translate-a64.c > > +++ b/target-arm/translate-a64.c > > @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t > > insn) > > gen_helper_hvc(cpu_env, tmp); > > tcg_temp_free_i32(tmp); > > break; > > + case 3: > > + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == > > 0) { > > + unallocated_encoding(s); > > + break; > > + } > > + tmp = tcg_const_i32(syn_aa64_smc(imm16)); > > + gen_a64_set_pc_im(s->pc); > > + gen_helper_smc(cpu_env, tmp); > > + tcg_temp_free_i32(tmp); > > + break; > > default: > > unallocated_encoding(s); > > break; > > -- > > 1.8.3.2 > > > >
On Fri, May 30, 2014 at 05:28:29PM +1000, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target-arm/cpu.h | 1 + > target-arm/helper-a64.c | 1 + > target-arm/helper.c | 6 ++++++ > target-arm/helper.h | 1 + > target-arm/internals.h | 6 ++++++ > target-arm/op_helper.c | 11 +++++++++++ > target-arm/translate-a64.c | 10 ++++++++++ > 7 files changed, 36 insertions(+) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 1a26ed4..b3631f2 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -52,6 +52,7 @@ > #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */ > #define EXCP_STREX 10 > #define EXCP_HVC 11 /* HyperVisor Call */ > +#define EXCP_SMC 12 /* Secure Monitor Call */ > > #define ARMV7M_EXCP_RESET 1 > #define ARMV7M_EXCP_NMI 2 > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index 974fa66..3894a6f 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > case EXCP_UDEF: > case EXCP_SWI: > case EXCP_HVC: > + case EXCP_SMC: > env->cp15.esr_el[new_el] = env->exception.syndrome; > break; > case EXCP_IRQ: > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 5b2070c..35091ea 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx) > case EXCP_HVC: > target_el = MAX(target_el, 2); > break; > + case EXCP_SMC: > + target_el = 3; > + if (env->cp15.hcr_el2 & HCR_TSC) { The HCR.TSC routing of SMC to EL2 only applies in NS EL1 mode, will fix for v2 as: case EXCP_SMC: target_el = 3; if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) { target_el = 2; } break; > + target_el = 2; > + } > + break; > } > return target_el; > } > diff --git a/target-arm/helper.h b/target-arm/helper.h > index fb711be..6c3d84d 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32) > DEF_HELPER_1(wfi, void, env) > DEF_HELPER_1(wfe, void, env) > DEF_HELPER_2(hvc, void, env, i32) > +DEF_HELPER_2(smc, void, env, i32) > > DEF_HELPER_3(cpsr_write, void, env, i32, i32) > DEF_HELPER_1(cpsr_read, i32, env) > diff --git a/target-arm/internals.h b/target-arm/internals.h > index b08381c..e50a68e 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -54,6 +54,7 @@ static const char * const excnames[] = { > [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage", > [EXCP_STREX] = "QEMU intercept of STREX", > [EXCP_HVC] = "Hypervisor Call", > + [EXCP_SMC] = "Secure Monitor Call", > }; > > static inline void arm_log_exception(int idx) > @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16) > return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); > } > > +static inline uint32_t syn_aa64_smc(uint32_t imm16) > +{ > + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); > +} > + > static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) > { > return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 6bf34b0..6840828 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome) > raise_exception(env, EXCP_HVC); > } > > +void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > +{ > + /* We've already checked that EL3 exists at translation time. */ > + if (env->cp15.scr_el3 & SCR_SMD) { > + env->exception.syndrome = syn_uncategorized(); > + raise_exception(env, EXCP_UDEF); > + } > + env->exception.syndrome = syndrome; > + raise_exception(env, EXCP_SMC); > +} > + > void HELPER(exception_return)(CPUARMState *env) > { > int cur_el = arm_current_pl(env); > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 3981ee1..a02fe06 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t insn) > gen_helper_hvc(cpu_env, tmp); > tcg_temp_free_i32(tmp); > break; > + case 3: > + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) { > + unallocated_encoding(s); > + break; > + } > + tmp = tcg_const_i32(syn_aa64_smc(imm16)); > + gen_a64_set_pc_im(s->pc); > + gen_helper_smc(cpu_env, tmp); > + tcg_temp_free_i32(tmp); > + break; > default: > unallocated_encoding(s); > break; > -- > 1.8.3.2 >
On 30 May 2014 22:49, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Fri, May 30, 2014 at 11:50:23AM -0500, Greg Bellows wrote: > > On 30 May 2014 02:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> > wrote: > > > > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > > --- > > > target-arm/cpu.h | 1 + > > > target-arm/helper-a64.c | 1 + > > > target-arm/helper.c | 6 ++++++ > > > target-arm/helper.h | 1 + > > > target-arm/internals.h | 6 ++++++ > > > target-arm/op_helper.c | 11 +++++++++++ > > > target-arm/translate-a64.c | 10 ++++++++++ > > > 7 files changed, 36 insertions(+) > > > > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > > > index 1a26ed4..b3631f2 100644 > > > --- a/target-arm/cpu.h > > > +++ b/target-arm/cpu.h > > > @@ -52,6 +52,7 @@ > > > #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */ > > > #define EXCP_STREX 10 > > > #define EXCP_HVC 11 /* HyperVisor Call */ > > > +#define EXCP_SMC 12 /* Secure Monitor Call */ > > > > > > #define ARMV7M_EXCP_RESET 1 > > > #define ARMV7M_EXCP_NMI 2 > > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > > > index 974fa66..3894a6f 100644 > > > --- a/target-arm/helper-a64.c > > > +++ b/target-arm/helper-a64.c > > > @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > > case EXCP_UDEF: > > > case EXCP_SWI: > > > case EXCP_HVC: > > > + case EXCP_SMC: > > > env->cp15.esr_el[new_el] = env->exception.syndrome; > > > break; > > > case EXCP_IRQ: > > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > > index 5b2070c..35091ea 100644 > > > --- a/target-arm/helper.c > > > +++ b/target-arm/helper.c > > > @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, > > > unsigned int excp_idx) > > > case EXCP_HVC: > > > target_el = MAX(target_el, 2); > > > break; > > > + case EXCP_SMC: > > > + target_el = 3; > > > + if (env->cp15.hcr_el2 & HCR_TSC) { > > > + target_el = 2; > > > + } > > > + break; > > > } > > > return target_el; > > > } > > > diff --git a/target-arm/helper.h b/target-arm/helper.h > > > index fb711be..6c3d84d 100644 > > > --- a/target-arm/helper.h > > > +++ b/target-arm/helper.h > > > @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, > > > i32) > > > DEF_HELPER_1(wfi, void, env) > > > DEF_HELPER_1(wfe, void, env) > > > DEF_HELPER_2(hvc, void, env, i32) > > > +DEF_HELPER_2(smc, void, env, i32) > > > > > > DEF_HELPER_3(cpsr_write, void, env, i32, i32) > > > DEF_HELPER_1(cpsr_read, i32, env) > > > diff --git a/target-arm/internals.h b/target-arm/internals.h > > > index b08381c..e50a68e 100644 > > > --- a/target-arm/internals.h > > > +++ b/target-arm/internals.h > > > @@ -54,6 +54,7 @@ static const char * const excnames[] = { > > > [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage", > > > [EXCP_STREX] = "QEMU intercept of STREX", > > > [EXCP_HVC] = "Hypervisor Call", > > > + [EXCP_SMC] = "Secure Monitor Call", > > > }; > > > > > > static inline void arm_log_exception(int idx) > > > @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t > imm16) > > > return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > > > 0xffff); > > > } > > > > > > +static inline uint32_t syn_aa64_smc(uint32_t imm16) > > > +{ > > > + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > > > 0xffff); > > > +} > > > + > > > static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) > > > { > > > return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > > > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > > > index 6bf34b0..6840828 100644 > > > --- a/target-arm/op_helper.c > > > +++ b/target-arm/op_helper.c > > > @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t > syndrome) > > > raise_exception(env, EXCP_HVC); > > > } > > > > > > +void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > > > +{ > > > + /* We've already checked that EL3 exists at translation time. */ > > > + if (env->cp15.scr_el3 & SCR_SMD) { > > > > > > > In ARMv7 isn't this is this only a valid check if the virtualization > > extension is present? If so, should we be checking for the virt > extension > > on AArch32 prior to raising undefined? > > My understanding is that on AArch32 SCR.SMD only applies > to NS mode and only if HCR.TSC=0 (always the case if EL2 does not exist but > can also be zero even if EL2 exists). > So it's not dependant on virtual extensions/EL2 non-existance? Am I missing > something? > I'm not sure how we handle this as the ARMv7 spec states the following for the SCD bit: "*SCD, bit[7], when implementation includes the Virtualization Extensions*" This is not worded the same in the ARMv8 spec, possibly because the extensions are assumed to always be present? The QQMU code still supports the notion of features in which case sometimes are extensions. > > I'm not adding AArch32 SMC here but am happy to try to get it right > in preparation for the AArch32 support. > Sure, I suspect that Fabian's commits on top of yours will close any AArch32 gaps. > > The patch misses that the HCR.TSC routing of SMC to EL2 takes priority over > SMD for NS EL1. For AArch32, I need to check for non-secure state > aswell. I'll update those for v2. > > Need to run a few tests but I'm considering something along these lines: > > void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > { > int cur_el = arm_current_pl(env); > /* FIXME: Use real secure state. */ > bool secure = false; > bool smd = env->cp15.scr_el3 & SCR_SMD; > /* On AArch32, SMD only applies to NS mode. */ > bool udef = is_a64(env) ? smd : !secure && smd; > Is it possible to get in here at EL0? A quick check, did not turn up a check for this in the code calling this helper, but I may have missed it. > > /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */ > if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) { > udef = false; > } > > /* We've already checked that EL3 exists at translation time. */ > if (udef) { > env->exception.syndrome = syn_uncategorized(); > raise_exception(env, EXCP_UDEF); > } > env->exception.syndrome = syndrome; > raise_exception(env, EXCP_SMC); > } > > Thanks for the comments, > Edgar > > > > > > > > > + env->exception.syndrome = syn_uncategorized(); > > > + raise_exception(env, EXCP_UDEF); > > > + } > > > + env->exception.syndrome = syndrome; > > > + raise_exception(env, EXCP_SMC); > > > +} > > > + > > > void HELPER(exception_return)(CPUARMState *env) > > > { > > > int cur_el = arm_current_pl(env); > > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > > > index 3981ee1..a02fe06 100644 > > > --- a/target-arm/translate-a64.c > > > +++ b/target-arm/translate-a64.c > > > @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t > > > insn) > > > gen_helper_hvc(cpu_env, tmp); > > > tcg_temp_free_i32(tmp); > > > break; > > > + case 3: > > > + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl > == > > > 0) { > > > + unallocated_encoding(s); > > > + break; > > > + } > > > + tmp = tcg_const_i32(syn_aa64_smc(imm16)); > > > + gen_a64_set_pc_im(s->pc); > > > + gen_helper_smc(cpu_env, tmp); > > > + tcg_temp_free_i32(tmp); > > > + break; > > > default: > > > unallocated_encoding(s); > > > break; > > > -- > > > 1.8.3.2 > > > > > > > >
On Mon, Jun 02, 2014 at 11:12:11AM -0500, Greg Bellows wrote: > On 30 May 2014 22:49, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > > On Fri, May 30, 2014 at 11:50:23AM -0500, Greg Bellows wrote: > > > On 30 May 2014 02:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> > > wrote: > > > > > > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > > > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > > > --- > > > > target-arm/cpu.h | 1 + > > > > target-arm/helper-a64.c | 1 + > > > > target-arm/helper.c | 6 ++++++ > > > > target-arm/helper.h | 1 + > > > > target-arm/internals.h | 6 ++++++ > > > > target-arm/op_helper.c | 11 +++++++++++ > > > > target-arm/translate-a64.c | 10 ++++++++++ > > > > 7 files changed, 36 insertions(+) > > > > > > > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > > > > index 1a26ed4..b3631f2 100644 > > > > --- a/target-arm/cpu.h > > > > +++ b/target-arm/cpu.h > > > > @@ -52,6 +52,7 @@ > > > > #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */ > > > > #define EXCP_STREX 10 > > > > #define EXCP_HVC 11 /* HyperVisor Call */ > > > > +#define EXCP_SMC 12 /* Secure Monitor Call */ > > > > > > > > #define ARMV7M_EXCP_RESET 1 > > > > #define ARMV7M_EXCP_NMI 2 > > > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > > > > index 974fa66..3894a6f 100644 > > > > --- a/target-arm/helper-a64.c > > > > +++ b/target-arm/helper-a64.c > > > > @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > > > > case EXCP_UDEF: > > > > case EXCP_SWI: > > > > case EXCP_HVC: > > > > + case EXCP_SMC: > > > > env->cp15.esr_el[new_el] = env->exception.syndrome; > > > > break; > > > > case EXCP_IRQ: > > > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > > > index 5b2070c..35091ea 100644 > > > > --- a/target-arm/helper.c > > > > +++ b/target-arm/helper.c > > > > @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, > > > > unsigned int excp_idx) > > > > case EXCP_HVC: > > > > target_el = MAX(target_el, 2); > > > > break; > > > > + case EXCP_SMC: > > > > + target_el = 3; > > > > + if (env->cp15.hcr_el2 & HCR_TSC) { > > > > + target_el = 2; > > > > + } > > > > + break; > > > > } > > > > return target_el; > > > > } > > > > diff --git a/target-arm/helper.h b/target-arm/helper.h > > > > index fb711be..6c3d84d 100644 > > > > --- a/target-arm/helper.h > > > > +++ b/target-arm/helper.h > > > > @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, > > > > i32) > > > > DEF_HELPER_1(wfi, void, env) > > > > DEF_HELPER_1(wfe, void, env) > > > > DEF_HELPER_2(hvc, void, env, i32) > > > > +DEF_HELPER_2(smc, void, env, i32) > > > > > > > > DEF_HELPER_3(cpsr_write, void, env, i32, i32) > > > > DEF_HELPER_1(cpsr_read, i32, env) > > > > diff --git a/target-arm/internals.h b/target-arm/internals.h > > > > index b08381c..e50a68e 100644 > > > > --- a/target-arm/internals.h > > > > +++ b/target-arm/internals.h > > > > @@ -54,6 +54,7 @@ static const char * const excnames[] = { > > > > [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage", > > > > [EXCP_STREX] = "QEMU intercept of STREX", > > > > [EXCP_HVC] = "Hypervisor Call", > > > > + [EXCP_SMC] = "Secure Monitor Call", > > > > }; > > > > > > > > static inline void arm_log_exception(int idx) > > > > @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t > > imm16) > > > > return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > > > > 0xffff); > > > > } > > > > > > > > +static inline uint32_t syn_aa64_smc(uint32_t imm16) > > > > +{ > > > > + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > > > > 0xffff); > > > > +} > > > > + > > > > static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) > > > > { > > > > return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > > > > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > > > > index 6bf34b0..6840828 100644 > > > > --- a/target-arm/op_helper.c > > > > +++ b/target-arm/op_helper.c > > > > @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t > > syndrome) > > > > raise_exception(env, EXCP_HVC); > > > > } > > > > > > > > +void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > > > > +{ > > > > + /* We've already checked that EL3 exists at translation time. */ > > > > + if (env->cp15.scr_el3 & SCR_SMD) { > > > > > > > > > > In ARMv7 isn't this is this only a valid check if the virtualization > > > extension is present? If so, should we be checking for the virt > > extension > > > on AArch32 prior to raising undefined? > > > > My understanding is that on AArch32 SCR.SMD only applies > > to NS mode and only if HCR.TSC=0 (always the case if EL2 does not exist but > > can also be zero even if EL2 exists). > > So it's not dependant on virtual extensions/EL2 non-existance? Am I missing > > something? > > > > I'm not sure how we handle this as the ARMv7 spec states the following for > the SCD bit: > > "*SCD, bit[7], when implementation includes the Virtualization Extensions*" > > > This is not worded the same in the ARMv8 spec, possibly because the > extensions are assumed to always be present? The QQMU code still supports > the notion of features in which case sometimes are extensions. Hi, This is a bit confusing. I was looking at the spec for the Cortex A15 which does not mention the virtualization condition. It does refer to the ARMv7 specs for "more" info though so I suspect we need to differentiate them. I can do that in v2. > > > > > > I'm not adding AArch32 SMC here but am happy to try to get it right > > in preparation for the AArch32 support. > > > > Sure, I suspect that Fabian's commits on top of yours will close any > AArch32 gaps. > > > > > > The patch misses that the HCR.TSC routing of SMC to EL2 takes priority over > > SMD for NS EL1. For AArch32, I need to check for non-secure state > > aswell. I'll update those for v2. > > > > Need to run a few tests but I'm considering something along these lines: > > > > void HELPER(smc)(CPUARMState *env, uint32_t syndrome) > > { > > int cur_el = arm_current_pl(env); > > /* FIXME: Use real secure state. */ > > bool secure = false; > > bool smd = env->cp15.scr_el3 & SCR_SMD; > > /* On AArch32, SMD only applies to NS mode. */ > > bool udef = is_a64(env) ? smd : !secure && smd; > > > > Is it possible to get in here at EL0? A quick check, did not turn up a > check for this in the code calling this helper, but I may have missed it. We trap SMC from EL0 at translation time further down the patch. > > > > > > /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */ > > if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) { > > udef = false; > > } > > > > /* We've already checked that EL3 exists at translation time. */ > > if (udef) { > > env->exception.syndrome = syn_uncategorized(); > > raise_exception(env, EXCP_UDEF); > > } > > env->exception.syndrome = syndrome; > > raise_exception(env, EXCP_SMC); > > } > > > > Thanks for the comments, > > Edgar > > > > > > > > > > > > > > + env->exception.syndrome = syn_uncategorized(); > > > > + raise_exception(env, EXCP_UDEF); > > > > + } > > > > + env->exception.syndrome = syndrome; > > > > + raise_exception(env, EXCP_SMC); > > > > +} > > > > + > > > > void HELPER(exception_return)(CPUARMState *env) > > > > { > > > > int cur_el = arm_current_pl(env); > > > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > > > > index 3981ee1..a02fe06 100644 > > > > --- a/target-arm/translate-a64.c > > > > +++ b/target-arm/translate-a64.c > > > > @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t > > > > insn) > > > > gen_helper_hvc(cpu_env, tmp); > > > > tcg_temp_free_i32(tmp); > > > > break; > > > > + case 3: > > > > + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl > > == > > > > 0) { ^^^^^ here > > > > + unallocated_encoding(s); > > > > + break; > > > > + } > > > > + tmp = tcg_const_i32(syn_aa64_smc(imm16)); > > > > + gen_a64_set_pc_im(s->pc); > > > > + gen_helper_smc(cpu_env, tmp); > > > > + tcg_temp_free_i32(tmp); > > > > + break; > > > > default: > > > > unallocated_encoding(s); > > > > break; > > > > -- > > > > 1.8.3.2 > > > > > > > > > > > >
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 1a26ed4..b3631f2 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -52,6 +52,7 @@ #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */ #define EXCP_STREX 10 #define EXCP_HVC 11 /* HyperVisor Call */ +#define EXCP_SMC 12 /* Secure Monitor Call */ #define ARMV7M_EXCP_RESET 1 #define ARMV7M_EXCP_NMI 2 diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index 974fa66..3894a6f 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs) case EXCP_UDEF: case EXCP_SWI: case EXCP_HVC: + case EXCP_SMC: env->cp15.esr_el[new_el] = env->exception.syndrome; break; case EXCP_IRQ: diff --git a/target-arm/helper.c b/target-arm/helper.c index 5b2070c..35091ea 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx) case EXCP_HVC: target_el = MAX(target_el, 2); break; + case EXCP_SMC: + target_el = 3; + if (env->cp15.hcr_el2 & HCR_TSC) { + target_el = 2; + } + break; } return target_el; } diff --git a/target-arm/helper.h b/target-arm/helper.h index fb711be..6c3d84d 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32) DEF_HELPER_1(wfi, void, env) DEF_HELPER_1(wfe, void, env) DEF_HELPER_2(hvc, void, env, i32) +DEF_HELPER_2(smc, void, env, i32) DEF_HELPER_3(cpsr_write, void, env, i32, i32) DEF_HELPER_1(cpsr_read, i32, env) diff --git a/target-arm/internals.h b/target-arm/internals.h index b08381c..e50a68e 100644 --- a/target-arm/internals.h +++ b/target-arm/internals.h @@ -54,6 +54,7 @@ static const char * const excnames[] = { [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage", [EXCP_STREX] = "QEMU intercept of STREX", [EXCP_HVC] = "Hypervisor Call", + [EXCP_SMC] = "Secure Monitor Call", }; static inline void arm_log_exception(int idx) @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16) return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); } +static inline uint32_t syn_aa64_smc(uint32_t imm16) +{ + return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); +} + static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) { return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 6bf34b0..6840828 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome) raise_exception(env, EXCP_HVC); } +void HELPER(smc)(CPUARMState *env, uint32_t syndrome) +{ + /* We've already checked that EL3 exists at translation time. */ + if (env->cp15.scr_el3 & SCR_SMD) { + env->exception.syndrome = syn_uncategorized(); + raise_exception(env, EXCP_UDEF); + } + env->exception.syndrome = syndrome; + raise_exception(env, EXCP_SMC); +} + void HELPER(exception_return)(CPUARMState *env) { int cur_el = arm_current_pl(env); diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index 3981ee1..a02fe06 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_helper_hvc(cpu_env, tmp); tcg_temp_free_i32(tmp); break; + case 3: + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) { + unallocated_encoding(s); + break; + } + tmp = tcg_const_i32(syn_aa64_smc(imm16)); + gen_a64_set_pc_im(s->pc); + gen_helper_smc(cpu_env, tmp); + tcg_temp_free_i32(tmp); + break; default: unallocated_encoding(s); break;