On Thu, Aug 24, 2023 at 09:29:33AM +0200, Andrew Jones wrote:
> On Wed, Aug 23, 2023 at 08:04:08PM +0000, Colton Lewis wrote:
> > Use the virtual instead of the physical timer for measuring the time
> > taken to execute the microbenchmark.
> >
> > Internal testing discovered a performance regression on this test
> > starting with Linux commit 680232a94c12 "KVM: arm64: timers: Allow
> > save/restoring of the physical timer". Oliver Upton speculates QEMU is
> > changing the guest physical counter to have a nonzero offset since it
> > gained the ability as of that commit. As a consequence KVM is
> > trap-and-emulating here on architectures without FEAT_ECV.
>
> No need to speculate, QEMU is open source :-) QEMU is setting on offset,
> but not because it explicitly wants to. Simply reading the CNT register
> and writing back the same value is enough to set an offset, since the
> timer will have certainly moved past whatever value was read by the time
> it's written. QEMU frequently saves and restores all registers in the
> get-reg-list array, unless they've been explicitly filtered out (with
> Linux commit 680232a94c12, KVM_REG_ARM_PTIMER_CNT is now in the array).
> So, to restore trapless ptimer accesses, we need a QEMU patch like below
> to filter out the register.
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 94bbd9661fd3..f89ea31f170d 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -674,6 +674,7 @@ typedef struct CPRegStateLevel {
> */
> static const CPRegStateLevel non_runtime_cpregs[] = {
> { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> + { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
> };
>
> int kvm_arm_cpreg_level(uint64_t regidx)
>
>
> >
> > While this isn't a correctness issue, the trap-and-emulate overhead of
Actually, the QEMU fix above is necessary for more than just trap
avoidance. The ptimer will lag more and more with respect to other time
sources, even with respect to the vtimer (QEMU only updates the vtimer
offset when the VM is "paused".)
I'm not sure when I'll have time to test and post the QEMU patch. It may
be better if somebody else picks it up.
Thanks,
drew
> > physical counter emulation on systems without ECV leads to surprising
> > microbenchmark results.
> >
> > Signed-off-by: Colton Lewis <coltonlewis@xxxxxxxxxx>
> > ---
> > arm/micro-bench.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> > index bfd181dc..fbe59d03 100644
> > --- a/arm/micro-bench.c
> > +++ b/arm/micro-bench.c
> > @@ -348,10 +348,10 @@ static void loop_test(struct exit_test *test)
> >
> > while (ntimes < test->times && total_ns.ns < NS_5_SECONDS) {
> > isb();
> > - start = read_sysreg(cntpct_el0);
> > + start = read_sysreg(cntvct_el0);
> > test->exec();
> > isb();
> > - end = read_sysreg(cntpct_el0);
> > + end = read_sysreg(cntvct_el0);
>
> This patch should be merged too, though, to avoid hitting the issue on
> certain versions of QEMU. And, it's pretty clear that the original
> authors just picked the ptimer arbitrarily.
>
> >
> > ntimes++;
> > total_ticks += (end - start);
> > --
> > 2.42.0.rc1.204.g551eb34607-goog
>
> Queued.
>
> Thanks,
> drew
