Re: [Devel] [PATCH hci-8.1] KVM: nVMX: Keep the emulated VMX-preemption timer on L1's TSC

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] KVM: nVMX: no slew margin in the last 100us of the emulated preemption timer
- [Low] KVM: nVMX: commit message carries downstream-only Jira URL and Feature tag
--

commit 88673c58251bfdcecfe5cfb0887dacefd547c04a
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

KVM: nVMX: Keep the emulated VMX-preemption timer on L1's TSC

This patch arms the hrtimer that emulates the L2 VMX-preemption timer an
eighth early. The timer callback re-arms it until L1's TSC reaches the saved
deadline. The remaining timer value is now computed from that deadline, both
when starting the timer and when saving it to vmcs12.

> KVM: nVMX: Keep the emulated VMX-preemption timer on L1's TSC

[ ... ]

> Fixes: f4124500c2c1 ("KVM: nVMX: Fully emulate preemption timer")
> https://virtuozzo.atlassian.net/browse/VSTOR-152991
> Feature: fix ms/kvm

[Severity: Low]
This isn't a bug, but the commit message carries a private Jira URL
(virtuozzo.atlassian.net/browse/VSTOR-152991) and a "Feature: fix ms/kvm"
tag. Neither is meaningful to upstream reviewers. Could they be dropped
before this is posted upstream?

> diff --git a/arch/x86/kernel/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c

[ ... ]

> +static ktime_t vmx_preemption_timer_expiry(struct kvm_vcpu *vcpu, u64 ticks)
> +{
> +	u64 ns = (ticks << VMX_MISC_EMULATED_PREEMPTION_TIMER_RATE) * 1000000;
> +
> +	do_div(ns, vcpu->arch.virtual_tsc_khz);
> +	if (ns > 100 * NSEC_PER_USEC)
> +		ns -= ns >> 3;
> +	return ktime_add_ns(ktime_get(), ns);
> +}

[Severity: Low]
Can the emulated timer still fire late in L1's TSC when CLOCK_MONOTONIC is
slewed slower than the TSC?

The early margin is only applied when more than 100us remain:

    if (ns > 100 * NSEC_PER_USEC)
        ns -= ns >> 3;

When 100us or less remain, either at the initial hrtimer_start() or on the
last re-arm hop, the hrtimer is armed for the unadjusted conversion.

vmx_preemption_timer_fn() only re-arms when the timer fires early. A late
fire is not corrected. The lateness is bounded by roughly slew times 100us,
which is about 10us at the 10% slew the comment assumes.

The commit message says the "L2 PT expiry TSC > L2 TSC deadline" failure is
fixed for a slowed clock. Could a strict test such as
vmx_preemption_timer_test still fail intermittently for short timeouts?
Should the commit message mention this limitation?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261106134316.84189-1-eva.kurchatova@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel