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"...
thanks
-- PMM
