Re: [PATCH] arm64: kvm: Expose timer offset directly via KVM_{GET,SET}_ONE_REG

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2983 bytes --]

On Thu, 2023-02-02 at 13:13 +0100, Simon Veith wrote:
> The virtual timer count register (CNTVCT_EL0) is virtualized by
> configuring offset register CNTVOFF_EL2 to subtract from the underlying
> raw hardware timer count when the guest reads the current count.
> 
> Currently, we offer userspace the ability to serialize and deserialize
> only the absolute count register value, using KVM_{GET,SET}_ONE_REG with
> KVM_REG_ARM_TIMER_CNT. Internally, we then compute and set the offset
> register accordingly to obtain the requested count value.
> 
> Allowing to set this timer count register only by absolute value poses
> some problems to virtual machine monitors that try to maintain the
> illusion of continuously ticking clocks to the guest: In workflows like
> live migration or liveupdate, the timers must be increased artificially
> to account for pause time.
> 
> Any delays between userspace computing the correct timer count value and
> actually setting it in kernel space by KVM_SET_ONE_REG (such as can be
> incurred by scheduling) become visible as under-accounted pause time in
> the guest, meaning the guest observes that its system clock seems to
> have fallen behind its NTP time reference.
> 
> The issue is further complicated when vCPU setup is performed by
> independent threads which may experience different delays, leading to
> jitter between the clocks of different vCPUs.
> 
> We could deliver a more stable timer in such scenarios if we allowed
> userspace to set the offset with regards to the physical counter
> directly.


LGTM in principle. To allow for correct timekeeping across a live
update — whether it just be restarting the VMM, or kexec into a new
kernel and start a new VMM — we absolutely need to have preserve the
*offset*, and none of this "the timer was <x> then and now it's about
<y> later so let's calculate what the timer should have been at the
start of this sentence and set it to roughly that..." as you describe
in your commit message.

> 
> Expose the KVM_REG_ARM_TIMER_OFF register directly to userspace, as an
> alternative view of the timer counts. By default, userspace still sees
> only the existing KVM_REG_ARM_TIMER_CNT register when querying the list
> with KVM_GET_REG_LIST, as that register value is portable across
> different VM hosts and thus safe to persist.
> 
> Signed-off-by: Simon Veith <sveith@amazon.de>
> 

I don't have a strong opinion on not counting it as a "new" register
and just continuing to list KVM_REG_ARM_TIMER_{CTL,CNT,CVAL} *but*
surely we do need a way for userspace to detect that this new feature
is present in the kernel? I don't think that just "let them try and get
-ENOENT" is the right approach.

So maybe explicitly reporting KVM_REG_ARM_TIMER_OFF *is* the better
choice? After all, userspace needs to make an informed decision about
which to use depending on whether the guest will be resumed on the same
hardware or not.


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel