Re: [PATCH v2 1/8] KVM: x86/xen: improve accuracy of Xen timers

[Sean Christopherson <seanjc@google.com>]

On Tue, Feb 27, 2024, David Woodhouse wrote:
> +	/* Xen has a 'Linux workaround' in do_set_timer_op() which
> +	 * checks for negative absolute timeout values (caused by
> +	 * integer overflow), and for values about 13 days in the
> +	 * future (2^50ns) which would be caused by jiffies
> +	 * overflow. For those cases, it sets the timeout 100ms in
> +	 * the future (not *too* soon, since if a guest really did
> +	 * set a long timeout on purpose we don't want to keep
> +	 * churning CPU time by waking it up).
> +	 */

I'm going to massage this slightly, partly to take advantage of reduced indentation,
but also to call out when the workaround is applied.  Though in all honesty, the
extra context may just be in response to a PEBKAC on my end, as I misread the diff
multiple times.

> +	if (linux_wa) {
> +		if ((unlikely((int64_t)guest_abs < 0 ||

No need for a second set of parantheses around the unlikely.

> +			      (delta > 0 && (uint32_t) (delta >> 50) != 0)))) {

And this can all easily fit into one if-statement.

> +			delta = 100 * NSEC_PER_MSEC;
> +			guest_abs = guest_now + delta;
> +		}
> +	}

This is what I'm going to commit, holler if it looks wrong (disclaimer: I've only
compile tested at this point).

	/*
	 * Xen has a 'Linux workaround' in do_set_timer_op() which checks for
	 * negative absolute timeout values (caused by integer overflow), and
	 * for values about 13 days in the future (2^50ns) which would be
	 * caused by jiffies overflow. For those cases, Xen sets the timeout
	 * 100ms in the future (not *too* soon, since if a guest really did
	 * set a long timeout on purpose we don't want to keep churning CPU
	 * time by waking it up).  Emulate Xen's workaround when starting the
	 * timer in response to __HYPERVISOR_set_timer_op.
	 */
	if (linux_wa &&
	    unlikely((int64_t)guest_abs < 0 ||
		     (delta > 0 && (uint32_t) (delta >> 50) != 0))) {
		delta = 100 * NSEC_PER_MSEC;
		guest_abs = guest_now + delta;
	}