| Message ID | 20240419184608.2675213-3-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Make the counter frequency default 1GHz for new CPUs, machines | expand |
On 4/19/24 11:46, Peter Maydell wrote: > The generic timer frequency is settable by board code via a QOM > property "cntfrq", but otherwise defaults to 62.5MHz. The way this > is done includes some complication resulting from how this was > originally a fixed value with no QOM property. Clean it up: > > * always set cpu->gt_cntfrq_hz to some sensible value, whether > the CPU has the generic timer or not, and whether it's system > or user-only emulation > * this means we can always use gt_cntfrq_hz, and never need > the old GTIMER_SCALE define > * set the default value in exactly one place, in the realize fn > > The aim here is to pave the way for handling the ARMv8.6 requirement > that the generic timer frequency is always 1GHz. We're going to do > that by having old CPU types keep their legacy-in-QEMU behaviour and > having the default for any new CPU types be a 1GHz rather han 62.5MHz > cntfrq, so we want the point where the default is decided to be in > one place, and in code, not in a DEFINE_PROP_UINT64() initializer. > > This commit should have no behavioural changes. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/internals.h | 7 ++++--- > target/arm/cpu.c | 31 +++++++++++++++++-------------- > target/arm/helper.c | 16 ++++++++-------- > 3 files changed, 29 insertions(+), 25 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 19/4/24 20:46, Peter Maydell wrote: > The generic timer frequency is settable by board code via a QOM > property "cntfrq", but otherwise defaults to 62.5MHz. The way this > is done includes some complication resulting from how this was > originally a fixed value with no QOM property. Clean it up: > > * always set cpu->gt_cntfrq_hz to some sensible value, whether > the CPU has the generic timer or not, and whether it's system > or user-only emulation > * this means we can always use gt_cntfrq_hz, and never need > the old GTIMER_SCALE define > * set the default value in exactly one place, in the realize fn > > The aim here is to pave the way for handling the ARMv8.6 requirement > that the generic timer frequency is always 1GHz. We're going to do > that by having old CPU types keep their legacy-in-QEMU behaviour and > having the default for any new CPU types be a 1GHz rather han 62.5MHz > cntfrq, so we want the point where the default is decided to be in > one place, and in code, not in a DEFINE_PROP_UINT64() initializer. > > This commit should have no behavioural changes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/internals.h | 7 ++++--- > target/arm/cpu.c | 31 +++++++++++++++++-------------- > target/arm/helper.c | 16 ++++++++-------- > 3 files changed, 29 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/target/arm/internals.h b/target/arm/internals.h index dd3da211a3f..74d4b1b0990 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -59,10 +59,11 @@ static inline bool excp_is_internal(int excp) || excp == EXCP_SEMIHOST; } -/* Scale factor for generic timers, ie number of ns per tick. - * This gives a 62.5MHz timer. +/* + * Default frequency for the generic timer, in Hz. + * This is 62.5MHz, which gives a 16 ns tick period. */ -#define GTIMER_SCALE 16 +#define GTIMER_DEFAULT_HZ 62500000 /* Bit definitions for the v7M CONTROL register */ FIELD(V7M_CONTROL, NPRIV, 0, 1) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index ab8d007a86c..b248b283423 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1381,9 +1381,12 @@ static void arm_cpu_initfn(Object *obj) } } +/* + * 0 means "unset, use the default value". That default might vary depending + * on the CPU type, and is set in the realize fn. + */ static Property arm_cpu_gt_cntfrq_property = - DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz, - NANOSECONDS_PER_SECOND / GTIMER_SCALE); + DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz, 0); static Property arm_cpu_reset_cbar_property = DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0); @@ -1829,6 +1832,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) return; } + if (!cpu->gt_cntfrq_hz) { + /* + * 0 means "the board didn't set a value, use the default". + * The default value of the generic timer frequency (as seen in + * CNTFRQ_EL0) is 62.5MHz, which corresponds to a period of 16ns. + * This is what you get (a) for a CONFIG_USER_ONLY CPU (b) if the + * board doesn't set it. + */ + cpu->gt_cntfrq_hz = GTIMER_DEFAULT_HZ; + } + #ifndef CONFIG_USER_ONLY /* The NVIC and M-profile CPU are two halves of a single piece of * hardware; trying to use one without the other is a command line @@ -1877,18 +1891,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) } { - uint64_t scale; - - if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) { - if (!cpu->gt_cntfrq_hz) { - error_setg(errp, "Invalid CNTFRQ: %"PRId64"Hz", - cpu->gt_cntfrq_hz); - return; - } - scale = gt_cntfrq_period_ns(cpu); - } else { - scale = GTIMER_SCALE; - } + uint64_t scale = gt_cntfrq_period_ns(cpu); cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, scale, arm_gt_ptimer_cb, cpu); diff --git a/target/arm/helper.c b/target/arm/helper.c index 8bdbb404195..01cf231a861 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -2461,6 +2461,13 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = { .resetvalue = 0 }, }; +static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque) +{ + ARMCPU *cpu = env_archcpu(env); + + cpu->env.cp15.c14_cntfrq = cpu->gt_cntfrq_hz; +} + #ifndef CONFIG_USER_ONLY static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri, @@ -3215,13 +3222,6 @@ void arm_gt_hvtimer_cb(void *opaque) gt_recalc_timer(cpu, GTIMER_HYPVIRT); } -static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque) -{ - ARMCPU *cpu = env_archcpu(env); - - cpu->env.cp15.c14_cntfrq = cpu->gt_cntfrq_hz; -} - static const ARMCPRegInfo generic_timer_cp_reginfo[] = { /* * Note that CNTFRQ is purely reads-as-written for the benefit @@ -3501,7 +3501,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = { .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 0, .type = ARM_CP_CONST, .access = PL0_R /* no PL1_RW in linux-user */, .fieldoffset = offsetof(CPUARMState, cp15.c14_cntfrq), - .resetvalue = NANOSECONDS_PER_SECOND / GTIMER_SCALE, + .resetfn = arm_gt_cntfrq_reset, }, { .name = "CNTVCT_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 2,
The generic timer frequency is settable by board code via a QOM property "cntfrq", but otherwise defaults to 62.5MHz. The way this is done includes some complication resulting from how this was originally a fixed value with no QOM property. Clean it up: * always set cpu->gt_cntfrq_hz to some sensible value, whether the CPU has the generic timer or not, and whether it's system or user-only emulation * this means we can always use gt_cntfrq_hz, and never need the old GTIMER_SCALE define * set the default value in exactly one place, in the realize fn The aim here is to pave the way for handling the ARMv8.6 requirement that the generic timer frequency is always 1GHz. We're going to do that by having old CPU types keep their legacy-in-QEMU behaviour and having the default for any new CPU types be a 1GHz rather han 62.5MHz cntfrq, so we want the point where the default is decided to be in one place, and in code, not in a DEFINE_PROP_UINT64() initializer. This commit should have no behavioural changes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 7 ++++--- target/arm/cpu.c | 31 +++++++++++++++++-------------- target/arm/helper.c | 16 ++++++++-------- 3 files changed, 29 insertions(+), 25 deletions(-)