Re: [PATCH v1 0/5] target/arm/kvm: Provide an option to adjust virtual time

From: Peter Maydell <peter.maydell@linaro.org>
To: Andrew Jones <drjones@redhat.com>
Cc: bijan.mottahedeh@oracle.com, Marc Zyngier <maz@kernel.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	Andrea Bolognani <abologna@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	qemu-arm <qemu-arm@nongnu.org>
Subject: Re: [PATCH v1 0/5] target/arm/kvm: Provide an option to adjust virtual time
Date: Tue, 10 Dec 2019 15:45:09 +0000	[thread overview]
Message-ID: <CAFEAcA_Y0bX4mg7PyTgcfKtZQkPw+DEibW6OjVRGyA29mC7TiQ@mail.gmail.com> (raw)
In-Reply-To: <20191210133254.22vcpvr3eabcnthe@kamzik.brq.redhat.com>

On Tue, 10 Dec 2019 at 13:33, Andrew Jones <drjones@redhat.com> wrote:
> So the ins and outs of this particular timekeeping issue (to the best of
> my knowledge) is that x86 has implemented this behavior since
> 00f4d64ee76e ("kvmclock: clock should count only if vm is running"), which
> was committed over six years ago. Possibly x86 VM time would behave more
> like arm VM time if kvmclock was disabled, but that's not a recommended
> configuration.
>
> PPC got an equivalent patch to the x86 one in 2017, 42043e4f1241 ("spapr:
> clock should count only if vm is running"), but when stopping time during
> pause on spapr they actually *keep* 'date' and 'hwclock' in synch. I guess
> whatever clocksource 'hwclock' uses on spapr was already stopping when
> paused? For x86 those values diverge, and for arm without this series they
> stay the same but experience jumps, and with this series they diverge like
> x86. I don't see any way to disable the behaviour 42043e4f1241 introduces.
>
> s390x got what appears to be its equivalent patch last year 9bc9d3d1ae3b
> ("s390x/tod: Properly stop the KVM TOD while the guest is not running").
> The commit message doesn't state how hwclock and date values change /
> don't change, and I don't see any way to disable the behavior.
>
> MIPS has had this implemented since KVM support was introduced. No way
> to disable it that I know of.
>
> So why is this arm-specific? arm is just trying to catch up, but also
> offer a switch allowing the current behavior to be selected. If other
> architectures see value in the switch then they're free to adopt it too.
> After having done this git mining, it looks more and more like we should
> at least consider naming this feature 'kvm-no-adjvtime' and probably
> also change arm's default.

Thanks for pulling up the handling by other architectures.
I think I agree that we should change the arm default (ie
we should just call this a bug fix, since the old behaviour
seems unhelpful generally and is more random accident than
a deliberate choice), with a switch provided just in case
anybody had something depending on the old behaviour.

-- PMM