public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop.
@ 2025-03-17  9:19 Liam Ni
  2025-03-17 13:38 ` Sean Christopherson
  0 siblings, 1 reply; 5+ messages in thread
From: Liam Ni @ 2025-03-17  9:19 UTC (permalink / raw)
  To: kvm, linux-kernel, x86
  Cc: seanjc, pbonzini, tglx, mingo, bp, dave.hansen, hpa, LiamNi,
	CobeChen, LouisQi, EwanHai, FrankZhu

When using the dump-guest-memory command in QEMU to dump
the virtual machine's memory,the virtual machine will be
paused for a period of time.If the guest (i.e., UEFI) uses
the PIT as the system clock,it will be observed that the
HRTIMER used by the PIT continues to run during the guest
stop process, imposing an additional burden on the system.
Moreover, during the guest restart process,the previously
established HRTIMER will be canceled,and the accumulated
timer events will be flushed.However, before the old
HRTIMER is canceled,the accumulated timer events
will "surreptitiously" inject interrupts into the guest.

SO during the process of saving the KVM PIT state,
the HRTIMER need to be canceled to reduce the performance overhead
caused by HRTIMER during the guest stop process.

i.e. if guest

Signed-off-by: Liam Ni <liamni-oc@zhaoxin.com>
---
 arch/x86/kvm/x86.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 045c61cc7e54..75355b315aca 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6405,6 +6405,8 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
 
 	mutex_lock(&kps->lock);
 	memcpy(ps, &kps->channels, sizeof(*ps));
+	hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
+	kthread_flush_work(&kvm->arch.vpit->expired);
 	mutex_unlock(&kps->lock);
 	return 0;
 }
@@ -6428,6 +6430,8 @@ static int kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
 	memcpy(ps->channels, &kvm->arch.vpit->pit_state.channels,
 		sizeof(ps->channels));
 	ps->flags = kvm->arch.vpit->pit_state.flags;
+	hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
+	kthread_flush_work(&kvm->arch.vpit->expired);
 	mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 	memset(&ps->reserved, 0, sizeof(ps->reserved));
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop.
  2025-03-17  9:19 [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop Liam Ni
@ 2025-03-17 13:38 ` Sean Christopherson
  2025-03-25  6:10   ` LiamNioc
  0 siblings, 1 reply; 5+ messages in thread
From: Sean Christopherson @ 2025-03-17 13:38 UTC (permalink / raw)
  To: Liam Ni
  Cc: kvm, linux-kernel, x86, pbonzini, tglx, mingo, bp, dave.hansen,
	hpa, LiamNi, CobeChen, LouisQi, EwanHai, FrankZhu

On Mon, Mar 17, 2025, Liam Ni wrote:
> When using the dump-guest-memory command in QEMU to dump
> the virtual machine's memory,the virtual machine will be
> paused for a period of time.If the guest (i.e., UEFI) uses
> the PIT as the system clock,it will be observed that the
> HRTIMER used by the PIT continues to run during the guest
> stop process, imposing an additional burden on the system.
> Moreover, during the guest restart process,the previously
> established HRTIMER will be canceled,and the accumulated
> timer events will be flushed.However, before the old
> HRTIMER is canceled,the accumulated timer events
> will "surreptitiously" inject interrupts into the guest.
> 
> SO during the process of saving the KVM PIT state,
> the HRTIMER need to be canceled to reduce the performance overhead
> caused by HRTIMER during the guest stop process.
> 
> i.e. if guest
> 
> Signed-off-by: Liam Ni <liamni-oc@zhaoxin.com>
> ---
>  arch/x86/kvm/x86.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 045c61cc7e54..75355b315aca 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6405,6 +6405,8 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
>  
>  	mutex_lock(&kps->lock);
>  	memcpy(ps, &kps->channels, sizeof(*ps));
> +	hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
> +	kthread_flush_work(&kvm->arch.vpit->expired);

KVM cannot assume userspace wants to stop the PIT when grabbing a snapshot.  It's
a significant ABI change, and not desirable in all cases.

>  	mutex_unlock(&kps->lock);
>  	return 0;
>  }
> @@ -6428,6 +6430,8 @@ static int kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
>  	memcpy(ps->channels, &kvm->arch.vpit->pit_state.channels,
>  		sizeof(ps->channels));
>  	ps->flags = kvm->arch.vpit->pit_state.flags;
> +	hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
> +	kthread_flush_work(&kvm->arch.vpit->expired);
>  	mutex_unlock(&kvm->arch.vpit->pit_state.lock);
>  	memset(&ps->reserved, 0, sizeof(ps->reserved));
>  	return 0;
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop.
  2025-03-17 13:38 ` Sean Christopherson
@ 2025-03-25  6:10   ` LiamNioc
  2025-04-04 14:48     ` Sean Christopherson
  0 siblings, 1 reply; 5+ messages in thread
From: LiamNioc @ 2025-03-25  6:10 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, linux-kernel, x86, pbonzini, tglx, mingo, bp, dave.hansen,
	hpa, LiamNi, CobeChen, LouisQi, EwanHai, FrankZhu



On 2025/3/17 21:38, Sean Christopherson wrote:
> 
> 
> 
> On Mon, Mar 17, 2025, Liam Ni wrote:
>> When using the dump-guest-memory command in QEMU to dump
>> the virtual machine's memory,the virtual machine will be
>> paused for a period of time.If the guest (i.e., UEFI) uses
>> the PIT as the system clock,it will be observed that the
>> HRTIMER used by the PIT continues to run during the guest
>> stop process, imposing an additional burden on the system.
>> Moreover, during the guest restart process,the previously
>> established HRTIMER will be canceled,and the accumulated
>> timer events will be flushed.However, before the old
>> HRTIMER is canceled,the accumulated timer events
>> will "surreptitiously" inject interrupts into the guest.
>>
>> SO during the process of saving the KVM PIT state,
>> the HRTIMER need to be canceled to reduce the performance overhead
>> caused by HRTIMER during the guest stop process.
>>
>> i.e. if guest
>>
>> Signed-off-by: Liam Ni <liamni-oc@zhaoxin.com>
>> ---
>>   arch/x86/kvm/x86.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 045c61cc7e54..75355b315aca 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -6405,6 +6405,8 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
>>
>>        mutex_lock(&kps->lock);
>>        memcpy(ps, &kps->channels, sizeof(*ps));
>> +     hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
>> +     kthread_flush_work(&kvm->arch.vpit->expired);
> 
> KVM cannot assume userspace wants to stop the PIT when grabbing a snapshot.  It's
> a significant ABI change, and not desirable in all cases.

When VM Pause, all devices of the virtual machine are frozen, so the PIT 
freeze only saves the PIT device status, but does not cancel HRTIMER, 
but chooses to cancel HRTIMER when VM resumes and refresh the pending 
task. According to my observation, before refreshing the pending task, 
these pending tasks will secretly inject interrupts into the guest.

So do we need to cancel the HRTIMER when VM pause?

> 
>>        mutex_unlock(&kps->lock);
>>        return 0;
>>   }
>> @@ -6428,6 +6430,8 @@ static int kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
>>        memcpy(ps->channels, &kvm->arch.vpit->pit_state.channels,
>>                sizeof(ps->channels));
>>        ps->flags = kvm->arch.vpit->pit_state.flags;
>> +     hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
>> +     kthread_flush_work(&kvm->arch.vpit->expired);
>>        mutex_unlock(&kvm->arch.vpit->pit_state.lock);
>>        memset(&ps->reserved, 0, sizeof(ps->reserved));
>>        return 0;
>> --
>> 2.25.1
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop.
  2025-03-25  6:10   ` LiamNioc
@ 2025-04-04 14:48     ` Sean Christopherson
  2025-04-07  2:19       ` LiamNioc
  0 siblings, 1 reply; 5+ messages in thread
From: Sean Christopherson @ 2025-04-04 14:48 UTC (permalink / raw)
  To: LiamNioc
  Cc: kvm, linux-kernel, x86, pbonzini, tglx, mingo, bp, dave.hansen,
	hpa, LiamNi, CobeChen, LouisQi, EwanHai, FrankZhu

On Tue, Mar 25, 2025, LiamNioc wrote:
> On 2025/3/17 21:38, Sean Christopherson wrote:
> > On Mon, Mar 17, 2025, Liam Ni wrote:
> > > When using the dump-guest-memory command in QEMU to dump
> > > the virtual machine's memory,the virtual machine will be
> > > paused for a period of time.If the guest (i.e., UEFI) uses
> > > the PIT as the system clock,it will be observed that the
> > > HRTIMER used by the PIT continues to run during the guest
> > > stop process, imposing an additional burden on the system.
> > > Moreover, during the guest restart process,the previously
> > > established HRTIMER will be canceled,and the accumulated
> > > timer events will be flushed.However, before the old
> > > HRTIMER is canceled,the accumulated timer events
> > > will "surreptitiously" inject interrupts into the guest.
> > > 
> > > SO during the process of saving the KVM PIT state,
> > > the HRTIMER need to be canceled to reduce the performance overhead
> > > caused by HRTIMER during the guest stop process.
> > > 
> > > i.e. if guest
> > > 
> > > Signed-off-by: Liam Ni <liamni-oc@zhaoxin.com>
> > > ---
> > >   arch/x86/kvm/x86.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 045c61cc7e54..75355b315aca 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -6405,6 +6405,8 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
> > > 
> > >        mutex_lock(&kps->lock);
> > >        memcpy(ps, &kps->channels, sizeof(*ps));
> > > +     hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
> > > +     kthread_flush_work(&kvm->arch.vpit->expired);
> > 
> > KVM cannot assume userspace wants to stop the PIT when grabbing a snapshot.  It's
> > a significant ABI change, and not desirable in all cases.
> 
> When VM Pause, all devices of the virtual machine are frozen, so the PIT
> freeze only saves the PIT device status, but does not cancel HRTIMER, but
> chooses to cancel HRTIMER when VM resumes and refresh the pending task.
> According to my observation, before refreshing the pending task, these
> pending tasks will secretly inject interrupts into the guest.
> 
> So do we need to cancel the HRTIMER when VM pause?

The problem is that KVM has no real concept of pausing a VM.  There are a variety
of hacky hooks here and there, but no KVM-wide notion of "pause".

For this case, after getting PIT state, you should be able to use KVM_SET_PIT2 to
set the mode of channel[0] to 0xff, which call destroy_pit_timer() via
pit_load_count().

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop.
  2025-04-04 14:48     ` Sean Christopherson
@ 2025-04-07  2:19       ` LiamNioc
  0 siblings, 0 replies; 5+ messages in thread
From: LiamNioc @ 2025-04-07  2:19 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: kvm, linux-kernel, x86, pbonzini, tglx, mingo, bp, dave.hansen,
	hpa, LiamNi, CobeChen, LouisQi, EwanHai, FrankZhu



On 2025/4/4 22:48, Sean Christopherson wrote:
> 
> 
> [这封邮件来自外部发件人 谨防风险]
> 
> On Tue, Mar 25, 2025, LiamNioc wrote:
>> On 2025/3/17 21:38, Sean Christopherson wrote:
>>> On Mon, Mar 17, 2025, Liam Ni wrote:
>>>> When using the dump-guest-memory command in QEMU to dump
>>>> the virtual machine's memory,the virtual machine will be
>>>> paused for a period of time.If the guest (i.e., UEFI) uses
>>>> the PIT as the system clock,it will be observed that the
>>>> HRTIMER used by the PIT continues to run during the guest
>>>> stop process, imposing an additional burden on the system.
>>>> Moreover, during the guest restart process,the previously
>>>> established HRTIMER will be canceled,and the accumulated
>>>> timer events will be flushed.However, before the old
>>>> HRTIMER is canceled,the accumulated timer events
>>>> will "surreptitiously" inject interrupts into the guest.
>>>>
>>>> SO during the process of saving the KVM PIT state,
>>>> the HRTIMER need to be canceled to reduce the performance overhead
>>>> caused by HRTIMER during the guest stop process.
>>>>
>>>> i.e. if guest
>>>>
>>>> Signed-off-by: Liam Ni <liamni-oc@zhaoxin.com>
>>>> ---
>>>>    arch/x86/kvm/x86.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>>> index 045c61cc7e54..75355b315aca 100644
>>>> --- a/arch/x86/kvm/x86.c
>>>> +++ b/arch/x86/kvm/x86.c
>>>> @@ -6405,6 +6405,8 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
>>>>
>>>>         mutex_lock(&kps->lock);
>>>>         memcpy(ps, &kps->channels, sizeof(*ps));
>>>> +     hrtimer_cancel(&kvm->arch.vpit->pit_state.timer);
>>>> +     kthread_flush_work(&kvm->arch.vpit->expired);
>>>
>>> KVM cannot assume userspace wants to stop the PIT when grabbing a snapshot.  It's
>>> a significant ABI change, and not desirable in all cases.
>>
>> When VM Pause, all devices of the virtual machine are frozen, so the PIT
>> freeze only saves the PIT device status, but does not cancel HRTIMER, but
>> chooses to cancel HRTIMER when VM resumes and refresh the pending task.
>> According to my observation, before refreshing the pending task, these
>> pending tasks will secretly inject interrupts into the guest.
>>
>> So do we need to cancel the HRTIMER when VM pause?
> 
> The problem is that KVM has no real concept of pausing a VM.  There are a variety
> of hacky hooks here and there, but no KVM-wide notion of "pause".
> 
> For this case, after getting PIT state, you should be able to use KVM_SET_PIT2 to
> set the mode of channel[0] to 0xff, which call destroy_pit_timer() via
> pit_load_count().

Got it.
so i need to modify the behavior of QEMU's code.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-04-07  7:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-17  9:19 [PATCH] KVM: x86:Cancel hrtimer in the process of saving PIT state to reduce the performance overhead caused by hrtimer during guest stop Liam Ni
2025-03-17 13:38 ` Sean Christopherson
2025-03-25  6:10   ` LiamNioc
2025-04-04 14:48     ` Sean Christopherson
2025-04-07  2:19       ` LiamNioc

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox