[PATCH] KVM: x86: Enable PIT shutdown quirk

[PATCH] KVM: x86: Enable PIT shutdown quirk

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

KVM emulation of the PIT has a quirk such that the normal PIT shutdown
path doesn't work, because clearing the counter register restarts the
timer.

Disable the counter clearing on PIT shutdown as in Hyper-V

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 arch/x86/kernel/kvm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 1cceac5..14411b6 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -43,6 +43,7 @@
 #include <asm/reboot.h>
 #include <asm/svm.h>
 #include <asm/e820/api.h>
+#include <linux/i8253.h>
 
 DEFINE_STATIC_KEY_FALSE(kvm_async_pf_enabled);
 
@@ -978,6 +979,9 @@ static void __init kvm_init_platform(void)
 			wrmsrl(MSR_KVM_MIGRATION_CONTROL,
 			       KVM_MIGRATION_READY);
 	}
+
+	i8253_clear_counter_on_shutdown = false;
+
 	kvmclock_init();
 	x86_platform.apic_post_init = kvm_apic_init;
 }
-- 
2.9.4

Re: [PATCH] KVM: x86: Enable PIT shutdown quirk

[Sean Christopherson]

Please use "x86/kvm:" for guest side changes.

On Fri, Feb 03, 2023, lirongqing@baidu.com wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> KVM emulation of the PIT has a quirk such that the normal PIT shutdown
> path doesn't work, because clearing the counter register restarts the
> timer.
> 
> Disable the counter clearing on PIT shutdown as in Hyper-V
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  arch/x86/kernel/kvm.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 1cceac5..14411b6 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -43,6 +43,7 @@
>  #include <asm/reboot.h>
>  #include <asm/svm.h>
>  #include <asm/e820/api.h>
> +#include <linux/i8253.h>
>  
>  DEFINE_STATIC_KEY_FALSE(kvm_async_pf_enabled);
>  
> @@ -978,6 +979,9 @@ static void __init kvm_init_platform(void)
>  			wrmsrl(MSR_KVM_MIGRATION_CONTROL,
>  			       KVM_MIGRATION_READY);
>  	}
> +
> +	i8253_clear_counter_on_shutdown = false;

AFAICT, zeroing the counter isn't actually supposed to stop it from counting.
Copy pasting from the KVM host-side patch[*]:

  The largest possible initial count is 0; this is equivalent to 216 for
  binary counting and 104 for BCD counting.

  The Counter does not stop when it reaches zero. In Modes 0, 1, 4, and 5 the
  Counter ‘‘wraps around’’ to the highest count, either FFFF hex for binary count-
  ing or 9999 for BCD counting, and continues counting. 

  Mode 0 is typically used for event counting. After the Control Word is written,
  OUT is initially low, and will remain low until the Counter reaches zero. OUT
  then goes high and remains high until a new count or a new Mode 0 Control Word
  is written into the Counter.

Can we simply delete i8253_clear_counter_on_shutdown and the code it wraps?

[*] https://lore.kernel.org/kvm/Y91yLt3EZLA32csp@google.com

RE: [PATCH] KVM: x86: Enable PIT shutdown quirk

[Li,Rongqing]

> -----Original Message-----
> From: Sean Christopherson <seanjc@google.com>
> Sent: Saturday, February 4, 2023 4:50 AM
> To: Li,Rongqing <lirongqing@baidu.com>
> Cc: kvm@vger.kernel.org; x86@kernel.org
> Subject: Re: [PATCH] KVM: x86: Enable PIT shutdown quirk
> 
> Please use "x86/kvm:" for guest side changes.
> 
> On Fri, Feb 03, 2023, lirongqing@baidu.com wrote:
> > From: Li RongQing <lirongqing@baidu.com>
> >
> > KVM emulation of the PIT has a quirk such that the normal PIT shutdown
> > path doesn't work, because clearing the counter register restarts the
> > timer.
> >
> > Disable the counter clearing on PIT shutdown as in Hyper-V
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  arch/x86/kernel/kvm.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c index
> > 1cceac5..14411b6 100644
> > --- a/arch/x86/kernel/kvm.c
> > +++ b/arch/x86/kernel/kvm.c
> > @@ -43,6 +43,7 @@
> >  #include <asm/reboot.h>
> >  #include <asm/svm.h>
> >  #include <asm/e820/api.h>
> > +#include <linux/i8253.h>
> >
> >  DEFINE_STATIC_KEY_FALSE(kvm_async_pf_enabled);
> >
> > @@ -978,6 +979,9 @@ static void __init kvm_init_platform(void)
> >  			wrmsrl(MSR_KVM_MIGRATION_CONTROL,
> >  			       KVM_MIGRATION_READY);
> >  	}
> > +
> > +	i8253_clear_counter_on_shutdown = false;
> 
> AFAICT, zeroing the counter isn't actually supposed to stop it from counting.
> Copy pasting from the KVM host-side patch[*]:
> 
>   The largest possible initial count is 0; this is equivalent to 216 for
>   binary counting and 104 for BCD counting.
> 
>   The Counter does not stop when it reaches zero. In Modes 0, 1, 4, and 5 the
>   Counter ‘‘wraps around’’ to the highest count, either FFFF hex for binary
> count-
>   ing or 9999 for BCD counting, and continues counting.
> 
>   Mode 0 is typically used for event counting. After the Control Word is written,
>   OUT is initially low, and will remain low until the Counter reaches zero. OUT
>   then goes high and remains high until a new count or a new Mode 0 Control
> Word
>   is written into the Counter.
> 
> Can we simply delete i8253_clear_counter_on_shutdown and the code it
> wraps?
> 

The git log did not record why counter register should be zeroed

I will try to delete the zeroing counter register codes

Thanks

-Li


> [*] https://lore.kernel.org/kvm/Y91yLt3EZLA32csp@google.com