[RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency
to high resolution timers with the comment:

    KVM lapic timer and tsc deadline timer based on hrtimer,
    setting a leftmost node to rb tree and then do hrtimer reprogram.
    If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
    do nothing and then make kvm lapic timer and tsc deadline timer fail.

That was back in 2012, where hrtimer_start_range_ns() would do the
reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with
high resolution timers disabled, this did not work. But a lot has changed
in the last 12 years.

For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on
timer enqueue") modifies __hrtimer_start_range_ns() to work with low res
timers. There's been lots of other changes that make low res work.

I added this change to my main server that runs all my VMs (my mail
server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the
system has been running just fine for over a month.

ChromeOS has tested this before as well, and it hasn't seen any issues with
running KVM with high res timers disabled.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 arch/x86/kvm/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 472a1537b7a9..c65127e796a9 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -19,7 +19,6 @@ if VIRTUALIZATION
 
 config KVM
 	tristate "Kernel-based Virtual Machine (KVM) support"
-	depends on HIGH_RES_TIMERS
 	depends on X86_LOCAL_APIC
 	select KVM_COMMON
 	select KVM_GENERIC_MMU_NOTIFIER
-- 
2.43.0

Re: [RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[Sean Christopherson]

On Wed, Aug 21, 2024, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency
> to high resolution timers with the comment:
> 
>     KVM lapic timer and tsc deadline timer based on hrtimer,
>     setting a leftmost node to rb tree and then do hrtimer reprogram.
>     If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
>     do nothing and then make kvm lapic timer and tsc deadline timer fail.
> 
> That was back in 2012, where hrtimer_start_range_ns() would do the
> reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with
> high resolution timers disabled, this did not work. But a lot has changed
> in the last 12 years.
> 
> For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on
> timer enqueue") modifies __hrtimer_start_range_ns() to work with low res
> timers. There's been lots of other changes that make low res work.
> 
> I added this change to my main server that runs all my VMs (my mail
> server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the
> system has been running just fine for over a month.
> 
> ChromeOS has tested this before as well, and it hasn't seen any issues with
> running KVM with high res timers disabled.

Can you provide some background on why this is desirable, and what the effective
tradeoffs are?  Mostly so that future users have some chance of making an
informed decision.  Realistically, anyone running with HIGH_RES_TIMERS=n is likely
already aware of the tradeoffs, but it'd be nice to capture the info here.

> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  arch/x86/kvm/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 472a1537b7a9..c65127e796a9 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -19,7 +19,6 @@ if VIRTUALIZATION
>  
>  config KVM
>  	tristate "Kernel-based Virtual Machine (KVM) support"
> -	depends on HIGH_RES_TIMERS

I did some very basic testing and nothing exploded on me either.  So long as
nothing in the host catches fire, I don't see a good reason to make high resolution
timers a hard requirement.

My only concern is that this could, at least in theory, result in people
unintentionally breaking their setups, but that seems quite unlikely.

One thought would be to require the user to enable EXPERT in order to break the
HIGH_RES_TIMERS dependency.  In practice, I doubt that will be much of a deterrent
since (IIRC) many distros ship with EXPERT=y.  But it would at least document that
using KVM x86 without HIGH_RES_TIMERS may come with caveats.  E.g.

	depends on HIGH_RES_TIMERS || EXPERT

Re: [RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[Suleiman Souhlal]

On Wed, Aug 28, 2024 at 12:34:26PM -0700, Sean Christopherson wrote:
> On Wed, Aug 21, 2024, Steven Rostedt wrote:
> > From: Steven Rostedt <rostedt@goodmis.org>
> > 
> > Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency
> > to high resolution timers with the comment:
> > 
> >     KVM lapic timer and tsc deadline timer based on hrtimer,
> >     setting a leftmost node to rb tree and then do hrtimer reprogram.
> >     If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
> >     do nothing and then make kvm lapic timer and tsc deadline timer fail.
> > 
> > That was back in 2012, where hrtimer_start_range_ns() would do the
> > reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with
> > high resolution timers disabled, this did not work. But a lot has changed
> > in the last 12 years.
> > 
> > For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on
> > timer enqueue") modifies __hrtimer_start_range_ns() to work with low res
> > timers. There's been lots of other changes that make low res work.
> > 
> > I added this change to my main server that runs all my VMs (my mail
> > server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the
> > system has been running just fine for over a month.
> > 
> > ChromeOS has tested this before as well, and it hasn't seen any issues with
> > running KVM with high res timers disabled.
> 
> Can you provide some background on why this is desirable, and what the effective
> tradeoffs are?  Mostly so that future users have some chance of making an
> informed decision.  Realistically, anyone running with HIGH_RES_TIMERS=n is likely
> already aware of the tradeoffs, but it'd be nice to capture the info here.

We have found that disabling HR timers saves power without degrading
the user experience too much.

> 
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > ---
> >  arch/x86/kvm/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> > index 472a1537b7a9..c65127e796a9 100644
> > --- a/arch/x86/kvm/Kconfig
> > +++ b/arch/x86/kvm/Kconfig
> > @@ -19,7 +19,6 @@ if VIRTUALIZATION
> >  
> >  config KVM
> >  	tristate "Kernel-based Virtual Machine (KVM) support"
> > -	depends on HIGH_RES_TIMERS
> 
> I did some very basic testing and nothing exploded on me either.  So long as
> nothing in the host catches fire, I don't see a good reason to make high resolution
> timers a hard requirement.
> 
> My only concern is that this could, at least in theory, result in people
> unintentionally breaking their setups, but that seems quite unlikely.
> 
> One thought would be to require the user to enable EXPERT in order to break the
> HIGH_RES_TIMERS dependency.  In practice, I doubt that will be much of a deterrent
> since (IIRC) many distros ship with EXPERT=y.  But it would at least document that
> using KVM x86 without HIGH_RES_TIMERS may come with caveats.  E.g.
> 
> 	depends on HIGH_RES_TIMERS || EXPERT

This sounds like a good compromise.

-- Suleiman

Re: [RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[Paolo Bonzini]

On 9/4/24 09:35, Suleiman Souhlal wrote:
> On Wed, Aug 28, 2024 at 12:34:26PM -0700, Sean Christopherson wrote:
>> On Wed, Aug 21, 2024, Steven Rostedt wrote:
>>> From: Steven Rostedt <rostedt@goodmis.org>
>>>
>>> Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency
>>> to high resolution timers with the comment:
>>>
>>>      KVM lapic timer and tsc deadline timer based on hrtimer,
>>>      setting a leftmost node to rb tree and then do hrtimer reprogram.
>>>      If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
>>>      do nothing and then make kvm lapic timer and tsc deadline timer fail.
>>>
>>> That was back in 2012, where hrtimer_start_range_ns() would do the
>>> reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with
>>> high resolution timers disabled, this did not work. But a lot has changed
>>> in the last 12 years.
>>>
>>> For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on
>>> timer enqueue") modifies __hrtimer_start_range_ns() to work with low res
>>> timers. There's been lots of other changes that make low res work.
>>>
>>> I added this change to my main server that runs all my VMs (my mail
>>> server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the
>>> system has been running just fine for over a month.
>>>
>>> ChromeOS has tested this before as well, and it hasn't seen any issues with
>>> running KVM with high res timers disabled.
>>
>> Can you provide some background on why this is desirable, and what the effective
>> tradeoffs are?  Mostly so that future users have some chance of making an
>> informed decision.  Realistically, anyone running with HIGH_RES_TIMERS=n is likely
>> already aware of the tradeoffs, but it'd be nice to capture the info here.
> 
> We have found that disabling HR timers saves power without degrading
> the user experience too much.

This might have some issues on guests that do not support kvmclock, 
because they rely on precise delivery of periodic timers to keep their 
clock running.  This can be the APIC timer (provided by the kernel), the 
RTC (provided by userspace), or the i8254 (choice of kernel/userspace).

These guests are few and far between these days, and in the case of the 
APIC timer + Intel hosts we can use the preemption timer (which is 
TSC-based and has better latency _and_ accuracy).  Furthermore, only x86 
is requiring CONFIG_HIGH_RES_TIMERS, so it's probably just excessive 
care and we can even apply Steven's patch as is.

Alternatively, the "depends on HIGH_RES_TIMERS || EXPERT" could be added 
to virt/kvm.  Or a pr_warn could be added to kvm_init if HIGH_RES_TIMERS 
are not enabled.

But in general, it seems that Linux has a laissez-faire approach to 
disabling CONFIG_HIGH_RES_TIMERS - there must be other code in the 
kernel (maybe sound/?) that is relying on having high-enough HZ or 
hrtimers but that's not documented anywhere.  I don't have an objection 
to doing the same in KVM, honestly, since most systems are running 
CONFIG_HIGH_RES_TIMERS anyway.

Paolo

Re: [RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[Suleiman Souhlal]

On Wed, Sep 04, 2024 at 03:25:51PM +0200, Paolo Bonzini wrote:
> On 9/4/24 09:35, Suleiman Souhlal wrote:
> > On Wed, Aug 28, 2024 at 12:34:26PM -0700, Sean Christopherson wrote:
> > > On Wed, Aug 21, 2024, Steven Rostedt wrote:
> > > > From: Steven Rostedt <rostedt@goodmis.org>
> > > > 
> > > > Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency
> > > > to high resolution timers with the comment:
> > > > 
> > > >      KVM lapic timer and tsc deadline timer based on hrtimer,
> > > >      setting a leftmost node to rb tree and then do hrtimer reprogram.
> > > >      If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
> > > >      do nothing and then make kvm lapic timer and tsc deadline timer fail.
> > > > 
> > > > That was back in 2012, where hrtimer_start_range_ns() would do the
> > > > reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with
> > > > high resolution timers disabled, this did not work. But a lot has changed
> > > > in the last 12 years.
> > > > 
> > > > For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on
> > > > timer enqueue") modifies __hrtimer_start_range_ns() to work with low res
> > > > timers. There's been lots of other changes that make low res work.
> > > > 
> > > > I added this change to my main server that runs all my VMs (my mail
> > > > server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the
> > > > system has been running just fine for over a month.
> > > > 
> > > > ChromeOS has tested this before as well, and it hasn't seen any issues with
> > > > running KVM with high res timers disabled.
> > > 
> > > Can you provide some background on why this is desirable, and what the effective
> > > tradeoffs are?  Mostly so that future users have some chance of making an
> > > informed decision.  Realistically, anyone running with HIGH_RES_TIMERS=n is likely
> > > already aware of the tradeoffs, but it'd be nice to capture the info here.
> > 
> > We have found that disabling HR timers saves power without degrading
> > the user experience too much.
> 
> This might have some issues on guests that do not support kvmclock, because
> they rely on precise delivery of periodic timers to keep their clock
> running.  This can be the APIC timer (provided by the kernel), the RTC
> (provided by userspace), or the i8254 (choice of kernel/userspace).
> 
> These guests are few and far between these days, and in the case of the APIC
> timer + Intel hosts we can use the preemption timer (which is TSC-based and
> has better latency _and_ accuracy).  Furthermore, only x86 is requiring
> CONFIG_HIGH_RES_TIMERS, so it's probably just excessive care and we can even
> apply Steven's patch as is.
> 
> Alternatively, the "depends on HIGH_RES_TIMERS || EXPERT" could be added to
> virt/kvm.  Or a pr_warn could be added to kvm_init if HIGH_RES_TIMERS are
> not enabled.
> 
> But in general, it seems that Linux has a laissez-faire approach to
> disabling CONFIG_HIGH_RES_TIMERS - there must be other code in the kernel
> (maybe sound/?) that is relying on having high-enough HZ or hrtimers but
> that's not documented anywhere.  I don't have an objection to doing the same
> in KVM, honestly, since most systems are running CONFIG_HIGH_RES_TIMERS
> anyway.

I'm not sure how much my opinion on the matter counts, but I would be more
than happy to see Steven's current patch get applied as is.
It would make our (ChromeOS) life a bit simpler.

-- Suleiman

Re: [RFC][PATCH] KVM: Remove HIGH_RES_TIMERS dependency

[Paolo Bonzini]

> Commit 92b5265d38f6a ("KVM: Depend on HIGH_RES_TIMERS") added a dependency
> to high resolution timers with the comment:
> 
>     KVM lapic timer and tsc deadline timer based on hrtimer,
>     setting a leftmost node to rb tree and then do hrtimer reprogram.
>     If hrtimer not configured as high resolution, hrtimer_enqueue_reprogram
>     do nothing and then make kvm lapic timer and tsc deadline timer fail.
> 
> That was back in 2012, where hrtimer_start_range_ns() would do the
> reprogramming with hrtimer_enqueue_reprogram(). But as that was a nop with
> high resolution timers disabled, this did not work. But a lot has changed
> in the last 12 years.
> 
> For example, commit 49a2a07514a3a ("hrtimer: Kick lowres dynticks targets on
> timer enqueue") modifies __hrtimer_start_range_ns() to work with low res
> timers. There's been lots of other changes that make low res work.
> 
> I added this change to my main server that runs all my VMs (my mail
> server, my web server, my ssh server) and disabled HIGH_RES_TIMERS and the
> system has been running just fine for over a month.
> 
> ChromeOS has tested this before as well, and it hasn't seen any issues with
> running KVM with high res timers disabled.
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Queued, thanks.

Paolo