| Message ID | 20231218113305.2511480-25-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement emulation of nested virtualization | expand |
On 12/18/23 22:32, Peter Maydell wrote: > With FEAT_NV2, the condition for when SPSR_EL1.M should report that > an exception was taken from EL2 changes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/helper.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 45444360f95..38e16c2f8a5 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -11405,10 +11405,18 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) > aarch64_save_sp(env, arm_current_el(env)); > env->elr_el[new_el] = env->pc; > > - if (cur_el == 1 && new_el == 1 && > - ((arm_hcr_el2_eff(env) & (HCR_NV | HCR_NV1)) == HCR_NV)) { > - /* I_ZJRNN: report EL2 in the SPSR by setting M[3:2] to 0b10 */ > - old_mode = deposit32(old_mode, 2, 2, 2); > + if (cur_el == 1 && new_el == 1) { > + uint64_t hcr = arm_hcr_el2_eff(env); > + if ((hcr & (HCR_NV | HCR_NV1 | HCR_NV2)) == HCR_NV || > + (hcr & (HCR_NV | HCR_NV2)) == (HCR_NV | HCR_NV2)) { Maybe clearer as if ((hcr & HCR_NV) && ((hcr & HCR_NV2) || !(hcr & HCR_NV1))) ? Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > + /* > + * FEAT_NV, FEAT_NV2 may need to report EL2 in the SPSR > + * by setting M[3:2] to 0b10. > + * If NV2 is disabled, change SPSR when NV,NV1 == 1,0 (I_ZJRNN) > + * If NV2 is enabled, change SPSR when NV is 1 (I_DBTLM) > + */ > + old_mode = deposit32(old_mode, 2, 2, 2); > + } > } > } else { > old_mode = cpsr_read_for_spsr_elx(env);
On Wed, 27 Dec 2023 at 23:06, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 12/18/23 22:32, Peter Maydell wrote: > > With FEAT_NV2, the condition for when SPSR_EL1.M should report that > > an exception was taken from EL2 changes. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > target/arm/helper.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/target/arm/helper.c b/target/arm/helper.c > > index 45444360f95..38e16c2f8a5 100644 > > --- a/target/arm/helper.c > > +++ b/target/arm/helper.c > > @@ -11405,10 +11405,18 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) > > aarch64_save_sp(env, arm_current_el(env)); > > env->elr_el[new_el] = env->pc; > > > > - if (cur_el == 1 && new_el == 1 && > > - ((arm_hcr_el2_eff(env) & (HCR_NV | HCR_NV1)) == HCR_NV)) { > > - /* I_ZJRNN: report EL2 in the SPSR by setting M[3:2] to 0b10 */ > > - old_mode = deposit32(old_mode, 2, 2, 2); > > + if (cur_el == 1 && new_el == 1) { > > + uint64_t hcr = arm_hcr_el2_eff(env); > > + if ((hcr & (HCR_NV | HCR_NV1 | HCR_NV2)) == HCR_NV || > > + (hcr & (HCR_NV | HCR_NV2)) == (HCR_NV | HCR_NV2)) { > > Maybe clearer as > > if ((hcr & HCR_NV) && ((hcr & HCR_NV2) || !(hcr & HCR_NV1))) > > ? I dunno; I went back and forth a bit on how to write this, but I think what I have is a little closer to how the Arm ARM defines it (as separate FEAT_NV vs FEAT_NV2 conditions). At any rate, I don't think your suggestion sufficiently better to do the work to make the change :-) -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index 45444360f95..38e16c2f8a5 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -11405,10 +11405,18 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) aarch64_save_sp(env, arm_current_el(env)); env->elr_el[new_el] = env->pc; - if (cur_el == 1 && new_el == 1 && - ((arm_hcr_el2_eff(env) & (HCR_NV | HCR_NV1)) == HCR_NV)) { - /* I_ZJRNN: report EL2 in the SPSR by setting M[3:2] to 0b10 */ - old_mode = deposit32(old_mode, 2, 2, 2); + if (cur_el == 1 && new_el == 1) { + uint64_t hcr = arm_hcr_el2_eff(env); + if ((hcr & (HCR_NV | HCR_NV1 | HCR_NV2)) == HCR_NV || + (hcr & (HCR_NV | HCR_NV2)) == (HCR_NV | HCR_NV2)) { + /* + * FEAT_NV, FEAT_NV2 may need to report EL2 in the SPSR + * by setting M[3:2] to 0b10. + * If NV2 is disabled, change SPSR when NV,NV1 == 1,0 (I_ZJRNN) + * If NV2 is enabled, change SPSR when NV is 1 (I_DBTLM) + */ + old_mode = deposit32(old_mode, 2, 2, 2); + } } } else { old_mode = cpsr_read_for_spsr_elx(env);
With FEAT_NV2, the condition for when SPSR_EL1.M should report that an exception was taken from EL2 changes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)