On 1/31/20 5:06 AM, Peter Maydell wrote: > On Wed, 29 Jan 2020 at 23:56, Richard Henderson > <richard.hender...@linaro.org> wrote: >> >> Reviewed-by: Alex Bennée <alex.ben...@linaro.org> >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> >> --- >> target/arm/cpu.h | 7 ------- >> target/arm/helper.c | 6 +++++- >> 2 files changed, 5 insertions(+), 8 deletions(-) >> >> diff --git a/target/arm/cpu.h b/target/arm/cpu.h >> index 0477cef1ea..239c9eb783 100644 >> --- a/target/arm/cpu.h >> +++ b/target/arm/cpu.h >> @@ -1417,13 +1417,6 @@ static inline void xpsr_write(CPUARMState *env, >> uint32_t val, uint32_t mask) >> #define HCR_ATA (1ULL << 56) >> #define HCR_DCT (1ULL << 57) >> >> -/* >> - * When we actually implement ARMv8.1-VHE we should add HCR_E2H to >> - * HCR_MASK and then clear it again if the feature bit is not set in >> - * hcr_write(). >> - */ >> -#define HCR_MASK ((1ULL << 34) - 1) >> - >> #define SCR_NS (1U << 0) >> #define SCR_IRQ (1U << 1) >> #define SCR_FIQ (1U << 2) >> diff --git a/target/arm/helper.c b/target/arm/helper.c >> index 19a57a17da..f5ce05fdf3 100644 >> --- a/target/arm/helper.c >> +++ b/target/arm/helper.c >> @@ -4721,7 +4721,8 @@ static const ARMCPRegInfo el3_no_el2_v8_cp_reginfo[] = >> { >> static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t >> value) >> { >> ARMCPU *cpu = env_archcpu(env); >> - uint64_t valid_mask = HCR_MASK; >> + /* Begin with bits defined in base ARMv8.0. */ >> + uint64_t valid_mask = MAKE_64BIT_MASK(0, 34); >> >> if (arm_feature(env, ARM_FEATURE_EL3)) { >> valid_mask &= ~HCR_HCD; >> @@ -4735,6 +4736,9 @@ static void hcr_write(CPUARMState *env, const >> ARMCPRegInfo *ri, uint64_t value) >> */ >> valid_mask &= ~HCR_TSC; >> } >> + if (cpu_isar_feature(aa64_vh, cpu)) { >> + valid_mask |= HCR_E2H; >> + } >> if (cpu_isar_feature(aa64_lor, cpu)) { >> valid_mask |= HCR_TLOR; >> } > > Should HCR_E2H be in the list of bits for which we do > a tlb_flush () in hcr_write()? (Currently we do this for > VM, PTW and DC.) Given some of the later TLB-flushing > changes have code that is "we only need to flush these > TLB indexes when this register is written if E2H==1", > it makes it easier to be sure we have the right behaviour > if we don't need to think through scenarios of "write > to the register, then set E2H==1"...
I don't think so, because when E2H changes, we change mmu_idx entirely. Between E2 and E20_2 for el2, and (if TGE is also set) E10_0 and E20_0 for el0. r~