In the pre_hvc helper, we have a "bool secure = false" and a FIXME comment that we should use the actual Secure state. This is a signpost to the fact that we're not getting the UNDEF cases right. Specifically, we get most of them correct but are missing the ones related to Secure state:
* For A64: HVC must UNDEF at EL1 if EL2 is not enabled in the current Security state * For v8 A32/T32, HVC must UNDEF in Secure state, if EL2 is not enabled in the current Security state * For v7, HVC must UNDEF in Secure state The v7 requirement is identical to the v8 A32/T32 one, since there is no Secure EL2 in v7. This fixes a bug where HVC should have UNDEFed but instead trapped when in Secure state with Secure EL2 disabled. The bug is unlikely to affect real-world code, because only test code puts itself in a situation where HVC should UNDEF and then executes HVC and requires it to UNDEF. This was noticed by Coverity, because the "if (secure && ...)" check was dead code. CID: 1689834 Cc: [email protected] Fixes: 35979d71c401 ("target-arm: A64: Emulate the HVC insn") Signed-off-by: Peter Maydell <[email protected]> --- target/arm/tcg/op_helper.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c index 6a673e5ae6e..3d3c850a198 100644 --- a/target/arm/tcg/op_helper.c +++ b/target/arm/tcg/op_helper.c @@ -1295,8 +1295,6 @@ void HELPER(pre_hvc)(CPUARMState *env) { ARMCPU *cpu = env_archcpu(env); int cur_el = arm_current_el(env); - /* FIXME: Use actual secure state. */ - bool secure = false; bool undef; if (arm_is_psci_call(cpu, EXCP_HVC)) { @@ -1306,6 +1304,8 @@ void HELPER(pre_hvc)(CPUARMState *env) return; } + /* Note that we have already UNDEFed HVC from EL0 at translation time */ + if (!arm_feature(env, ARM_FEATURE_EL2)) { /* If EL2 doesn't exist, HVC always UNDEFs */ undef = true; @@ -1316,12 +1316,19 @@ void HELPER(pre_hvc)(CPUARMState *env) undef = env->cp15.hcr_el2 & HCR_HCD; } - /* In ARMv7 and ARMv8/AArch32, HVC is undef in secure state. - * For ARMv8/AArch64, HVC is allowed in EL3. - * Note that we've already trapped HVC from EL0 at translation - * time. + /* + * In ARMv7 and ARMv8/AArch32, HVC is undef in Secure state if + * EL2 is not enabled in the current security state (which is + * always for Secure in v7, as Secure EL2 doesn't exist there). + * + * For ARMv8/AArch64, HVC is undef at EL1 if EL2 is not enabled in + * the current security state. + * + * Because we've already dealt with "EL2 not implemented" above, + * arm_is_el2_enabled() is effectively always true here in NonSecure, + * and we don't need to explicitly also check for secure/non-secure. */ - if (secure && (!is_a64(env) || cur_el == 1)) { + if (!arm_is_el2_enabled(env) && (!is_a64(env) || cur_el == 1)) { undef = true; } -- 2.43.0
