| Message ID | 20191203022937.1474-36-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement ARMv8.1-VHE | expand |
On Tue, 3 Dec 2019 at 02:30, Richard Henderson <richard.henderson@linaro.org> wrote: > > When VHE is enabled, we need to take the aa32-ness of EL0 > from PSTATE not HCR_EL2, which is controlling EL1. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/helper.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index f2d18bd51a..f3785d5ad6 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -8887,14 +8887,19 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) > * immediately lower than the target level is using AArch32 or AArch64 > */ > bool is_aa64; > + uint64_t hcr; > > switch (new_el) { > case 3: > is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0; > break; > case 2: > - is_aa64 = (env->cp15.hcr_el2 & HCR_RW) != 0; > - break; > + hcr = arm_hcr_el2_eff(env); > + if ((hcr & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) { > + is_aa64 = (hcr & HCR_RW) != 0; > + break; > + } > + /* fall through */ > case 1: > is_aa64 = is_a64(env); > break; > -- The commit message is confusing me. Per the comment in the code, what we're asking is "is the EL immediately lower than the target level using AArch64?". We never took the aa32ness of EL0 from HCR_EL2: that code is looking at "what's the aa32ness of EL1", because in a non-VHE setup EL1 is always the EL immediately lower than EL2. So I *think* what the code is doing is: When VHE is enabled, the exception level below EL2 is not EL1, but EL0, and so to identify the entry vector offset for exceptions targeting EL2 we need to look at the width of EL0, not of EL1. Is that right? thanks -- PMM
On 12/6/19 8:03 AM, Peter Maydell wrote: > So I *think* what the code is doing is: > > When VHE is enabled, the exception level below EL2 is > not EL1, but EL0, and so to identify the entry vector > offset for exceptions targeting EL2 we need to look > at the width of EL0, not of EL1. > > Is that right? Correct. Much better wording, thanks. Will update. r~
On Fri, 6 Dec 2019 at 18:51, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 12/6/19 8:03 AM, Peter Maydell wrote: > > So I *think* what the code is doing is: > > > > When VHE is enabled, the exception level below EL2 is > > not EL1, but EL0, and so to identify the entry vector > > offset for exceptions targeting EL2 we need to look > > at the width of EL0, not of EL1. > > > > Is that right? > > Correct. Much better wording, thanks. Will update. In that case Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index f2d18bd51a..f3785d5ad6 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -8887,14 +8887,19 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) * immediately lower than the target level is using AArch32 or AArch64 */ bool is_aa64; + uint64_t hcr; switch (new_el) { case 3: is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0; break; case 2: - is_aa64 = (env->cp15.hcr_el2 & HCR_RW) != 0; - break; + hcr = arm_hcr_el2_eff(env); + if ((hcr & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) { + is_aa64 = (hcr & HCR_RW) != 0; + break; + } + /* fall through */ case 1: is_aa64 = is_a64(env); break;
When VHE is enabled, we need to take the aa32-ness of EL0 from PSTATE not HCR_EL2, which is controlling EL1. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/helper.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 2.17.1