| Message ID | 20241127170143.1664829-1-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] target/arm: implement SEL2 physical and virtual timers | expand |
On Wed, 27 Nov 2024 at 17:01, Alex Bennée <alex.bennee@linaro.org> wrote: > > When FEAT_SEL2 was implemented the SEL2 timers where missed. This > shows up when building the latest Hafnium with SPMC_AT_EL=2. The > actual implementation utilises the same logic as the rest of the > timers so all we need to do is: > > - define the timers and their access functions > - conditionally add the correct system registers > - create a new accessfn as the rules are subtly different to the > existing secure timer > > Fixes: e9152ee91c (target/arm: add ARMv8.4-SEL2 system registers) > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Andrei Homescu <ahomescu@google.com> > Cc: Arve Hjønnevåg <arve@google.com> > Cc: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> > --- > include/hw/arm/bsa.h | 2 + > target/arm/cpu.h | 2 + > target/arm/gtimer.h | 4 +- > hw/arm/virt.c | 2 + > target/arm/cpu.c | 8 +++ > target/arm/helper.c | 155 +++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 172 insertions(+), 1 deletion(-) > > diff --git a/include/hw/arm/bsa.h b/include/hw/arm/bsa.h > index 8eaab603c0..b4ecca1b1c 100644 > --- a/include/hw/arm/bsa.h > +++ b/include/hw/arm/bsa.h > @@ -22,6 +22,8 @@ > #define QEMU_ARM_BSA_H > > /* These are architectural INTID values */ > +#define ARCH_TIMER_S_VIRT_EL2_IRQ 19 > +#define ARCH_TIMER_S_EL2_IRQ 20 > #define VIRTUAL_PMU_IRQ 23 > #define ARCH_GIC_MAINT_IRQ 25 > #define ARCH_TIMER_NS_EL2_IRQ 26 > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index d86e641280..10b5354d6f 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1139,6 +1139,8 @@ void arm_gt_vtimer_cb(void *opaque); > void arm_gt_htimer_cb(void *opaque); > void arm_gt_stimer_cb(void *opaque); > void arm_gt_hvtimer_cb(void *opaque); > +void arm_gt_sel2timer_cb(void *opaque); > +void arm_gt_sel2vtimer_cb(void *opaque); > > unsigned int gt_cntfrq_period_ns(ARMCPU *cpu); > void gt_rme_post_el_change(ARMCPU *cpu, void *opaque); > diff --git a/target/arm/gtimer.h b/target/arm/gtimer.h > index b992941bef..3c097c59c7 100644 > --- a/target/arm/gtimer.h > +++ b/target/arm/gtimer.h > @@ -15,7 +15,9 @@ enum { > GTIMER_HYP = 2, > GTIMER_SEC = 3, > GTIMER_HYPVIRT = 4, > -#define NUM_GTIMERS 5 > + GTIMER_SEC_PEL2 = 5, > + GTIMER_SEC_VEL2 = 6, > +#define NUM_GTIMERS 7 Maybe we should add comments to this enum giving the architectural names of these timers? GTIMER_PHYS = 0, /* EL1 physical timer */ GTIMER_VIRT = 1, /* EL1 virtual timer */ GTIMER_HYP = 2, /* EL2 physical timer */ GTIMER_SEC = 3, /* EL3 physical timer */ GTIMER_HYPVIRT = 4, /* EL2 virtual timer */ GTIMER_SEC_PEL2 = 5, /* Secure EL2 physical timer */ GTIMER_SEC_VEL2 = 6, /* Secure EL2 virtual timer */ > }; > > #endif > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 1a381e9a2b..451d154459 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -873,6 +873,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem) > [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ, > [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ, > [GTIMER_HYPVIRT] = ARCH_TIMER_NS_EL2_VIRT_IRQ, > + [GTIMER_SEC_PEL2] = ARCH_TIMER_S_EL2_IRQ, > + [GTIMER_SEC_VEL2] = ARCH_TIMER_S_VIRT_EL2_IRQ, > }; > > for (unsigned irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { We probably need to also update at least sbsa-ref for this new pair of interrupts, and perhaps others (if they allow CPUs with FEAT_SEL2). > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 6938161b95..e42ab8ce8b 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -2078,6 +2078,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) > arm_gt_stimer_cb, cpu); > cpu->gt_timer[GTIMER_HYPVIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale, > arm_gt_hvtimer_cb, cpu); > + > + /* FEAT_SEL2 also has physical and virtual timers */ > + if (cpu_isar_feature(aa64_sel2, cpu)) { > + cpu->gt_timer[GTIMER_SEC_PEL2] = timer_new(QEMU_CLOCK_VIRTUAL, scale, > + arm_gt_sel2timer_cb, cpu); > + cpu->gt_timer[GTIMER_SEC_VEL2] = timer_new(QEMU_CLOCK_VIRTUAL, scale, > + arm_gt_sel2vtimer_cb, cpu); > + } We create all the other gt_timer[] entries regardless of whether the feature that uses them exists, and I think we should do the same here. It's pretty harmless and it means code elsewhere doesn't have to be super-careful about "is cpu->gt_timer[n] valid or is it zeroed out memory?". > } > #endif > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index f38eb054c0..b5a8a5846e 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -2668,6 +2668,41 @@ static CPAccessResult gt_stimer_access(CPUARMState *env, > } > } > > +static CPAccessResult gt_sel2timer_access(CPUARMState *env, > + const ARMCPRegInfo *ri, > + bool isread) > +{ > + /* > + * The AArch64 register view of the secure EL2 timers are mostly > + * accessible from EL3 and EL2 although can also be trapped to EL2 > + * from EL1 depending on nested virt config. > + */ > + switch (arm_current_el(env)) { > + case 0: > + return CP_ACCESS_TRAP; > + case 1: > + if (!arm_is_secure(env)) { > + return CP_ACCESS_TRAP; > + } else if (arm_hcr_el2_eff(env) & HCR_NV) { > + return CP_ACCESS_TRAP_EL2; > + } > + return CP_ACCESS_TRAP; > + case 2: > + if (!arm_is_secure(env)) { > + return CP_ACCESS_TRAP; > + } > + return CP_ACCESS_OK; > + case 3: > + if (env->cp15.scr_el3 & SCR_EEL2) { > + return CP_ACCESS_OK; > + } else { > + return CP_ACCESS_TRAP; > + } > + default: > + g_assert_not_reached(); > + } > +} > + > uint64_t gt_get_countervalue(CPUARMState *env) > { > ARMCPU *cpu = env_archcpu(env); > @@ -3175,6 +3210,62 @@ static void gt_sec_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, > gt_ctl_write(env, ri, GTIMER_SEC, value); > } > > +static void gt_sec_pel2_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) > +{ > + gt_timer_reset(env, ri, GTIMER_SEC_PEL2); > +} > + > +static void gt_sec_pel2_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + gt_cval_write(env, ri, GTIMER_SEC_PEL2, value); > +} > + > +static uint64_t gt_sec_pel2_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) > +{ > + return gt_tval_read(env, ri, GTIMER_SEC_PEL2); > +} > + > +static void gt_sec_pel2_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + gt_tval_write(env, ri, GTIMER_SEC_PEL2, value); > +} gt_tval_read() and gt_tval_write() have a switch which determines whether they obey the physical offset (CNTPOFF_EL2) or the virtual offset (CNTVOFF_EL2) or neither. For GTIMER_SEC_PEL2 the correct answer is "neither", which is what you get by default, but for GTIMER_SEC_VEL2 we should be honouring CNTVOFF_EL2, so you need to add a case to the switch for that. We also need to fix a pre-existing bug in gt_recalc_timer(), which does uint64_t offset = timeridx == GTIMER_VIRT ? cpu->env.cp15.cntvoff_el2 : gt_phys_raw_cnt_offset(&cpu->env); and so fails to include CNTVOFF_EL2 when working with GTIMER_HYPVIRT; then you can also add GTIMER_SEC_VEL2 to that. > + > +static void gt_sec_pel2_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + gt_ctl_write(env, ri, GTIMER_SEC_PEL2, value); > +} > + > +static void gt_sec_vel2_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) > +{ > + gt_timer_reset(env, ri, GTIMER_SEC_VEL2); > +} > + > +static void gt_sec_vel2_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + gt_cval_write(env, ri, GTIMER_SEC_VEL2, value); > +} > + > +static uint64_t gt_sec_vel2_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) > +{ > + return gt_tval_read(env, ri, GTIMER_SEC_VEL2); > +} > + > +static void gt_sec_vel2_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + gt_tval_write(env, ri, GTIMER_SEC_VEL2, value); > +} > + > +static void gt_sec_vel2_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, > + uint64_t value) > +{ > + gt_ctl_write(env, ri, GTIMER_SEC_VEL2, value); > +} > + > static void gt_hv_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) > { > gt_timer_reset(env, ri, GTIMER_HYPVIRT); > @@ -3231,6 +3322,20 @@ void arm_gt_stimer_cb(void *opaque) > gt_recalc_timer(cpu, GTIMER_SEC); > } > > +void arm_gt_sel2timer_cb(void *opaque) > +{ > + ARMCPU *cpu = opaque; > + > + gt_recalc_timer(cpu, GTIMER_SEC_PEL2); > +} > + > +void arm_gt_sel2vtimer_cb(void *opaque) > +{ > + ARMCPU *cpu = opaque; > + > + gt_recalc_timer(cpu, GTIMER_SEC_VEL2); > +} > + > void arm_gt_hvtimer_cb(void *opaque) > { > ARMCPU *cpu = opaque; > @@ -6613,6 +6718,56 @@ static const ARMCPRegInfo el2_sec_cp_reginfo[] = { > .access = PL2_RW, .accessfn = sel2_access, > .nv2_redirect_offset = 0x48, > .fieldoffset = offsetof(CPUARMState, cp15.vstcr_el2) }, > +#ifndef CONFIG_USER_ONLY > + /* Secure EL2 Physical Timer */ > + { .name = "CNTHPS_TVAL_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 0, > + .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL2_RW, > + .accessfn = gt_sel2timer_access, > + .readfn = gt_sec_pel2_tval_read, > + .writefn = gt_sec_pel2_tval_write, > + .resetfn = gt_sec_pel2_timer_reset, > + }, > + { .name = "CNTHPS_CTL_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 1, > + .type = ARM_CP_IO, .access = PL2_RW, > + .accessfn = gt_sel2timer_access, > + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_PEL2].ctl), > + .resetvalue = 0, > + .writefn = gt_sec_pel2_ctl_write, .raw_writefn = raw_write, > + }, > + { .name = "CNTHPS_CVAL_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 2, > + .type = ARM_CP_IO, .access = PL2_RW, > + .accessfn = gt_sel2timer_access, > + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_PEL2].cval), > + .writefn = gt_sec_pel2_cval_write, .raw_writefn = raw_write, > + }, > + /* Secure EL2 Virtual Timer */ > + { .name = "CNTHVS_TVAL_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 0, > + .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL2_RW, > + .accessfn = gt_sel2timer_access, > + .readfn = gt_sec_vel2_tval_read, > + .writefn = gt_sec_vel2_tval_write, > + .resetfn = gt_sec_vel2_timer_reset, > + }, > + { .name = "CNTHVS_CTL_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 1, > + .type = ARM_CP_IO, .access = PL2_RW, > + .accessfn = gt_sel2timer_access, > + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_VEL2].ctl), > + .resetvalue = 0, > + .writefn = gt_sec_vel2_ctl_write, .raw_writefn = raw_write, > + }, > + { .name = "CNTHVS_CVAL_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 2, > + .type = ARM_CP_IO, .access = PL2_RW, > + .accessfn = gt_sel2timer_access, > + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_VEL2].cval), > + .writefn = gt_sec_vel2_cval_write, .raw_writefn = raw_write, > + }, > +#endif > }; thanks -- PMM
diff --git a/include/hw/arm/bsa.h b/include/hw/arm/bsa.h index 8eaab603c0..b4ecca1b1c 100644 --- a/include/hw/arm/bsa.h +++ b/include/hw/arm/bsa.h @@ -22,6 +22,8 @@ #define QEMU_ARM_BSA_H /* These are architectural INTID values */ +#define ARCH_TIMER_S_VIRT_EL2_IRQ 19 +#define ARCH_TIMER_S_EL2_IRQ 20 #define VIRTUAL_PMU_IRQ 23 #define ARCH_GIC_MAINT_IRQ 25 #define ARCH_TIMER_NS_EL2_IRQ 26 diff --git a/target/arm/cpu.h b/target/arm/cpu.h index d86e641280..10b5354d6f 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1139,6 +1139,8 @@ void arm_gt_vtimer_cb(void *opaque); void arm_gt_htimer_cb(void *opaque); void arm_gt_stimer_cb(void *opaque); void arm_gt_hvtimer_cb(void *opaque); +void arm_gt_sel2timer_cb(void *opaque); +void arm_gt_sel2vtimer_cb(void *opaque); unsigned int gt_cntfrq_period_ns(ARMCPU *cpu); void gt_rme_post_el_change(ARMCPU *cpu, void *opaque); diff --git a/target/arm/gtimer.h b/target/arm/gtimer.h index b992941bef..3c097c59c7 100644 --- a/target/arm/gtimer.h +++ b/target/arm/gtimer.h @@ -15,7 +15,9 @@ enum { GTIMER_HYP = 2, GTIMER_SEC = 3, GTIMER_HYPVIRT = 4, -#define NUM_GTIMERS 5 + GTIMER_SEC_PEL2 = 5, + GTIMER_SEC_VEL2 = 6, +#define NUM_GTIMERS 7 }; #endif diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 1a381e9a2b..451d154459 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -873,6 +873,8 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem) [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ, [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ, [GTIMER_HYPVIRT] = ARCH_TIMER_NS_EL2_VIRT_IRQ, + [GTIMER_SEC_PEL2] = ARCH_TIMER_S_EL2_IRQ, + [GTIMER_SEC_VEL2] = ARCH_TIMER_S_VIRT_EL2_IRQ, }; for (unsigned irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 6938161b95..e42ab8ce8b 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -2078,6 +2078,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) arm_gt_stimer_cb, cpu); cpu->gt_timer[GTIMER_HYPVIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale, arm_gt_hvtimer_cb, cpu); + + /* FEAT_SEL2 also has physical and virtual timers */ + if (cpu_isar_feature(aa64_sel2, cpu)) { + cpu->gt_timer[GTIMER_SEC_PEL2] = timer_new(QEMU_CLOCK_VIRTUAL, scale, + arm_gt_sel2timer_cb, cpu); + cpu->gt_timer[GTIMER_SEC_VEL2] = timer_new(QEMU_CLOCK_VIRTUAL, scale, + arm_gt_sel2vtimer_cb, cpu); + } } #endif diff --git a/target/arm/helper.c b/target/arm/helper.c index f38eb054c0..b5a8a5846e 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -2668,6 +2668,41 @@ static CPAccessResult gt_stimer_access(CPUARMState *env, } } +static CPAccessResult gt_sel2timer_access(CPUARMState *env, + const ARMCPRegInfo *ri, + bool isread) +{ + /* + * The AArch64 register view of the secure EL2 timers are mostly + * accessible from EL3 and EL2 although can also be trapped to EL2 + * from EL1 depending on nested virt config. + */ + switch (arm_current_el(env)) { + case 0: + return CP_ACCESS_TRAP; + case 1: + if (!arm_is_secure(env)) { + return CP_ACCESS_TRAP; + } else if (arm_hcr_el2_eff(env) & HCR_NV) { + return CP_ACCESS_TRAP_EL2; + } + return CP_ACCESS_TRAP; + case 2: + if (!arm_is_secure(env)) { + return CP_ACCESS_TRAP; + } + return CP_ACCESS_OK; + case 3: + if (env->cp15.scr_el3 & SCR_EEL2) { + return CP_ACCESS_OK; + } else { + return CP_ACCESS_TRAP; + } + default: + g_assert_not_reached(); + } +} + uint64_t gt_get_countervalue(CPUARMState *env) { ARMCPU *cpu = env_archcpu(env); @@ -3175,6 +3210,62 @@ static void gt_sec_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, gt_ctl_write(env, ri, GTIMER_SEC, value); } +static void gt_sec_pel2_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + gt_timer_reset(env, ri, GTIMER_SEC_PEL2); +} + +static void gt_sec_pel2_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_cval_write(env, ri, GTIMER_SEC_PEL2, value); +} + +static uint64_t gt_sec_pel2_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ + return gt_tval_read(env, ri, GTIMER_SEC_PEL2); +} + +static void gt_sec_pel2_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_tval_write(env, ri, GTIMER_SEC_PEL2, value); +} + +static void gt_sec_pel2_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_ctl_write(env, ri, GTIMER_SEC_PEL2, value); +} + +static void gt_sec_vel2_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + gt_timer_reset(env, ri, GTIMER_SEC_VEL2); +} + +static void gt_sec_vel2_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_cval_write(env, ri, GTIMER_SEC_VEL2, value); +} + +static uint64_t gt_sec_vel2_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ + return gt_tval_read(env, ri, GTIMER_SEC_VEL2); +} + +static void gt_sec_vel2_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_tval_write(env, ri, GTIMER_SEC_VEL2, value); +} + +static void gt_sec_vel2_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_ctl_write(env, ri, GTIMER_SEC_VEL2, value); +} + static void gt_hv_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) { gt_timer_reset(env, ri, GTIMER_HYPVIRT); @@ -3231,6 +3322,20 @@ void arm_gt_stimer_cb(void *opaque) gt_recalc_timer(cpu, GTIMER_SEC); } +void arm_gt_sel2timer_cb(void *opaque) +{ + ARMCPU *cpu = opaque; + + gt_recalc_timer(cpu, GTIMER_SEC_PEL2); +} + +void arm_gt_sel2vtimer_cb(void *opaque) +{ + ARMCPU *cpu = opaque; + + gt_recalc_timer(cpu, GTIMER_SEC_VEL2); +} + void arm_gt_hvtimer_cb(void *opaque) { ARMCPU *cpu = opaque; @@ -6613,6 +6718,56 @@ static const ARMCPRegInfo el2_sec_cp_reginfo[] = { .access = PL2_RW, .accessfn = sel2_access, .nv2_redirect_offset = 0x48, .fieldoffset = offsetof(CPUARMState, cp15.vstcr_el2) }, +#ifndef CONFIG_USER_ONLY + /* Secure EL2 Physical Timer */ + { .name = "CNTHPS_TVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 0, + .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .readfn = gt_sec_pel2_tval_read, + .writefn = gt_sec_pel2_tval_write, + .resetfn = gt_sec_pel2_timer_reset, + }, + { .name = "CNTHPS_CTL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 1, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_PEL2].ctl), + .resetvalue = 0, + .writefn = gt_sec_pel2_ctl_write, .raw_writefn = raw_write, + }, + { .name = "CNTHPS_CVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 2, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_PEL2].cval), + .writefn = gt_sec_pel2_cval_write, .raw_writefn = raw_write, + }, + /* Secure EL2 Virtual Timer */ + { .name = "CNTHVS_TVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 0, + .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .readfn = gt_sec_vel2_tval_read, + .writefn = gt_sec_vel2_tval_write, + .resetfn = gt_sec_vel2_timer_reset, + }, + { .name = "CNTHVS_CTL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 1, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_VEL2].ctl), + .resetvalue = 0, + .writefn = gt_sec_vel2_ctl_write, .raw_writefn = raw_write, + }, + { .name = "CNTHVS_CVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 2, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_SEC_VEL2].cval), + .writefn = gt_sec_vel2_cval_write, .raw_writefn = raw_write, + }, +#endif }; static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri,
When FEAT_SEL2 was implemented the SEL2 timers where missed. This shows up when building the latest Hafnium with SPMC_AT_EL=2. The actual implementation utilises the same logic as the rest of the timers so all we need to do is: - define the timers and their access functions - conditionally add the correct system registers - create a new accessfn as the rules are subtly different to the existing secure timer Fixes: e9152ee91c (target/arm: add ARMv8.4-SEL2 system registers) Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Andrei Homescu <ahomescu@google.com> Cc: Arve Hjønnevåg <arve@google.com> Cc: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> --- include/hw/arm/bsa.h | 2 + target/arm/cpu.h | 2 + target/arm/gtimer.h | 4 +- hw/arm/virt.c | 2 + target/arm/cpu.c | 8 +++ target/arm/helper.c | 155 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 172 insertions(+), 1 deletion(-)