[PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Eric B Munson]

Currently, when qemu stops a guest kernel that guest will issue a soft lockup
message when it resumes.  This set provides the ability for qemu to comminucate
to the guest that it has been stopped.  When the guest hits the watchdog on
resume it will check if it was suspended before issuing the warning.

Eric B Munson (4):
  Add flag to indicate that a vm was stopped by the host
  Add functions to check if the host has stopped the vm
  Add generic stubs for kvm stop check functions
  Add check for suspended vm in softlockup detector

 arch/x86/include/asm/pvclock-abi.h |    1 +
 arch/x86/include/asm/pvclock.h     |    2 ++
 arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
 include/asm-generic/pvclock.h      |   14 ++++++++++++++
 kernel/watchdog.c                  |   12 ++++++++++++
 5 files changed, 43 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-generic/pvclock.h

-- 
1.7.4.1

[PATCH 1/4] Add flag to indicate that a vm was stopped by the host

[Eric B Munson]

This flag will be used to check if the vm was stopped by the host when a soft
lockup was detected.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
---
 arch/x86/include/asm/pvclock-abi.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/pvclock-abi.h b/arch/x86/include/asm/pvclock-abi.h
index 35f2d19..6167fd7 100644
--- a/arch/x86/include/asm/pvclock-abi.h
+++ b/arch/x86/include/asm/pvclock-abi.h
@@ -40,5 +40,6 @@ struct pvclock_wall_clock {
 } __attribute__((__packed__));
 
 #define PVCLOCK_TSC_STABLE_BIT	(1 << 0)
+#define PVCLOCK_GUEST_STOPPED	(1 << 1)
 #endif /* __ASSEMBLY__ */
 #endif /* _ASM_X86_PVCLOCK_ABI_H */
-- 
1.7.4.1

[PATCH 2/4] Add functions to check if the host has stopped the vm

[Eric B Munson]

When a host stops or suspends a VM it will set a flag to show this.  The
watchdog will use these functions to determine if a softlockup is real, or the
result of a suspended VM.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
---
 arch/x86/include/asm/pvclock.h |    2 ++
 arch/x86/kernel/kvmclock.c     |   14 ++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index a518c0a..dd59ad0 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -13,6 +13,8 @@ void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
 			    struct timespec *ts);
 void pvclock_resume(void);
 
+bool kvm_check_and_clear_host_stopped(int cpu);
+
 /*
  * Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction,
  * yielding a 64-bit result.
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index c1a0188..5f60d2b 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -113,6 +113,20 @@ static void kvm_get_preset_lpj(void)
 	preset_lpj = lpj;
 }
 
+bool kvm_check_and_clear_host_stopped(int cpu)
+{
+	bool ret = false;
+	struct pvclock_vcpu_time_info *src;
+
+	src = &per_cpu(hv_clock, cpu);
+	if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
+		src->flags = src->flags & (~PVCLOCK_GUEST_STOPPED);
+		ret = true;
+	}
+
+	return ret;
+}
+
 static struct clocksource kvm_clock = {
 	.name = "kvm-clock",
 	.read = kvm_clock_get_cycles,
-- 
1.7.4.1

[PATCH 3/4] Add generic stubs for kvm stop check functions

[Eric B Munson]

This function is called from the watchdog code when a soft lockup is detected.
If this is an arch that does not support pvclock, this function is used.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
---
 include/asm-generic/pvclock.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-generic/pvclock.h

diff --git a/include/asm-generic/pvclock.h b/include/asm-generic/pvclock.h
new file mode 100644
index 0000000..ff046b6
--- /dev/null
+++ b/include/asm-generic/pvclock.h
@@ -0,0 +1,14 @@
+#ifndef _ASM_GENERIC_PVCLOCK_H
+#define _ASM_GENERIC_PVCLOCK_H
+
+
+/*
+ * These functions are used by architectures that support kvm to avoid issuing
+ * false soft lockup messages.
+ */
+static inline bool kvm_check_and_clear_host_stopped(int cpu)
+{
+	return false;
+}
+
+#endif
-- 
1.7.4.1

[PATCH 4/4] Add check for suspended vm in softlockup detector

[Eric B Munson]

A suspended VM can cause spurious soft lockup warnings.  To avoid these, the
watchdog now checks if the kernel knows it was stopped by the host and skips
the warning if so.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
---
 kernel/watchdog.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 36491cd..4cbb69f 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -25,6 +25,7 @@
 #include <linux/sysctl.h>
 
 #include <asm/irq_regs.h>
+#include <asm/pvclock.h>
 #include <linux/perf_event.h>
 
 int watchdog_enabled = 1;
@@ -292,6 +293,17 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
 	 */
 	duration = is_softlockup(touch_ts);
 	if (unlikely(duration)) {
+		/*
+		 * If a virtual machine is stopped by the host it can look to
+		 * the watchdog like a soft lockup, check to see if the host
+		 * stopped the vm before we issue the warning
+		 */
+		if (kvm_check_and_clear_host_stopped(get_cpu())) {
+			put_cpu();
+			return HRTIMER_RESTART;
+		}
+		put_cpu();
+
 		/* only warn once */
 		if (__this_cpu_read(soft_watchdog_warn) == true)
 			return HRTIMER_RESTART;
-- 
1.7.4.1

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> Currently, when qemu stops a guest kernel that guest will issue a soft lockup
> message when it resumes.  This set provides the ability for qemu to comminucate
> to the guest that it has been stopped.  When the guest hits the watchdog on
> resume it will check if it was suspended before issuing the warning.
> 
> Eric B Munson (4):
>   Add flag to indicate that a vm was stopped by the host
>   Add functions to check if the host has stopped the vm
>   Add generic stubs for kvm stop check functions
>   Add check for suspended vm in softlockup detector
> 
>  arch/x86/include/asm/pvclock-abi.h |    1 +
>  arch/x86/include/asm/pvclock.h     |    2 ++
>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
>  kernel/watchdog.c                  |   12 ++++++++++++
>  5 files changed, 43 insertions(+), 0 deletions(-)
>  create mode 100644 include/asm-generic/pvclock.h
> 
> -- 
> 1.7.4.1

How is the host supposed to set this flag? 

As mentioned previously, if you save save/restore the offset added to
kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
paravirt infrastructure is required. Which means the issue is also fixed
for older guests.

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Ryan Harper]

* Marcelo Tosatti <mtosatti@redhat.com> [2011-08-30 07:35]:
> On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > Currently, when qemu stops a guest kernel that guest will issue a soft lockup
> > message when it resumes.  This set provides the ability for qemu to comminucate
> > to the guest that it has been stopped.  When the guest hits the watchdog on
> > resume it will check if it was suspended before issuing the warning.
> > 
> > Eric B Munson (4):
> >   Add flag to indicate that a vm was stopped by the host
> >   Add functions to check if the host has stopped the vm
> >   Add generic stubs for kvm stop check functions
> >   Add check for suspended vm in softlockup detector
> > 
> >  arch/x86/include/asm/pvclock-abi.h |    1 +
> >  arch/x86/include/asm/pvclock.h     |    2 ++
> >  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> >  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> >  kernel/watchdog.c                  |   12 ++++++++++++
> >  5 files changed, 43 insertions(+), 0 deletions(-)
> >  create mode 100644 include/asm-generic/pvclock.h
> > 
> > -- 
> > 1.7.4.1
> 
> How is the host supposed to set this flag? 
> 
> As mentioned previously, if you save save/restore the offset added to
> kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> paravirt infrastructure is required. Which means the issue is also fixed
> for older guests.

How is saving the offset going to prevent a large jump from triggering
the softlock message?  Won't we still have not touched the watchdog for
that long period of time?

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Tue, Aug 30, 2011 at 09:12:17AM -0500, Ryan Harper wrote:
> * Marcelo Tosatti <mtosatti@redhat.com> [2011-08-30 07:35]:
> > On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > > Currently, when qemu stops a guest kernel that guest will issue a soft lockup
> > > message when it resumes.  This set provides the ability for qemu to comminucate
> > > to the guest that it has been stopped.  When the guest hits the watchdog on
> > > resume it will check if it was suspended before issuing the warning.
> > > 
> > > Eric B Munson (4):
> > >   Add flag to indicate that a vm was stopped by the host
> > >   Add functions to check if the host has stopped the vm
> > >   Add generic stubs for kvm stop check functions
> > >   Add check for suspended vm in softlockup detector
> > > 
> > >  arch/x86/include/asm/pvclock-abi.h |    1 +
> > >  arch/x86/include/asm/pvclock.h     |    2 ++
> > >  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > >  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > >  kernel/watchdog.c                  |   12 ++++++++++++
> > >  5 files changed, 43 insertions(+), 0 deletions(-)
> > >  create mode 100644 include/asm-generic/pvclock.h
> > > 
> > > -- 
> > > 1.7.4.1
> > 
> > How is the host supposed to set this flag? 
> > 
> > As mentioned previously, if you save save/restore the offset added to
> > kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > paravirt infrastructure is required. Which means the issue is also fixed
> > for older guests.
> 
> How is saving the offset going to prevent a large jump from triggering
> the softlock message?  Won't we still have not touched the watchdog for
> that long period of time?

The idea is to adjust the offset and the TSC value so that the guest
does not notice the actual elapsed time. This is what happens when a
guest is migrated or saved/restore, for example.

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Ryan Harper]

* Marcelo Tosatti <mtosatti@redhat.com> [2011-08-30 09:40]:
> On Tue, Aug 30, 2011 at 09:12:17AM -0500, Ryan Harper wrote:
> > * Marcelo Tosatti <mtosatti@redhat.com> [2011-08-30 07:35]:
> > > On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > > > Currently, when qemu stops a guest kernel that guest will issue a soft lockup
> > > > message when it resumes.  This set provides the ability for qemu to comminucate
> > > > to the guest that it has been stopped.  When the guest hits the watchdog on
> > > > resume it will check if it was suspended before issuing the warning.
> > > > 
> > > > Eric B Munson (4):
> > > >   Add flag to indicate that a vm was stopped by the host
> > > >   Add functions to check if the host has stopped the vm
> > > >   Add generic stubs for kvm stop check functions
> > > >   Add check for suspended vm in softlockup detector
> > > > 
> > > >  arch/x86/include/asm/pvclock-abi.h |    1 +
> > > >  arch/x86/include/asm/pvclock.h     |    2 ++
> > > >  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > > >  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > > >  kernel/watchdog.c                  |   12 ++++++++++++
> > > >  5 files changed, 43 insertions(+), 0 deletions(-)
> > > >  create mode 100644 include/asm-generic/pvclock.h
> > > > 
> > > > -- 
> > > > 1.7.4.1
> > > 
> > > How is the host supposed to set this flag? 
> > > 
> > > As mentioned previously, if you save save/restore the offset added to
> > > kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > > paravirt infrastructure is required. Which means the issue is also fixed
> > > for older guests.
> > 
> > How is saving the offset going to prevent a large jump from triggering
> > the softlock message?  Won't we still have not touched the watchdog for
> > that long period of time?
> 
> The idea is to adjust the offset and the TSC value so that the guest
> does not notice the actual elapsed time. This is what happens when a
> guest is migrated or saved/restore, for example.

but that's within a rather short period of time... if we stop the guest
for a day, I don't think what you are suggesting can happen without time
being wrong in the guest, and as soon as it's corrected and we jump
forward larger than the watchdog timeout, won't we trigger it again?


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Tue, Aug 30, 2011 at 09:43:19AM -0500, Ryan Harper wrote:
> * Marcelo Tosatti <mtosatti@redhat.com> [2011-08-30 09:40]:
> > On Tue, Aug 30, 2011 at 09:12:17AM -0500, Ryan Harper wrote:
> > > * Marcelo Tosatti <mtosatti@redhat.com> [2011-08-30 07:35]:
> > > > On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > > > > Currently, when qemu stops a guest kernel that guest will issue a soft lockup
> > > > > message when it resumes.  This set provides the ability for qemu to comminucate
> > > > > to the guest that it has been stopped.  When the guest hits the watchdog on
> > > > > resume it will check if it was suspended before issuing the warning.
> > > > > 
> > > > > Eric B Munson (4):
> > > > >   Add flag to indicate that a vm was stopped by the host
> > > > >   Add functions to check if the host has stopped the vm
> > > > >   Add generic stubs for kvm stop check functions
> > > > >   Add check for suspended vm in softlockup detector
> > > > > 
> > > > >  arch/x86/include/asm/pvclock-abi.h |    1 +
> > > > >  arch/x86/include/asm/pvclock.h     |    2 ++
> > > > >  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > > > >  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > > > >  kernel/watchdog.c                  |   12 ++++++++++++
> > > > >  5 files changed, 43 insertions(+), 0 deletions(-)
> > > > >  create mode 100644 include/asm-generic/pvclock.h
> > > > > 
> > > > > -- 
> > > > > 1.7.4.1
> > > > 
> > > > How is the host supposed to set this flag? 
> > > > 
> > > > As mentioned previously, if you save save/restore the offset added to
> > > > kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > > > paravirt infrastructure is required. Which means the issue is also fixed
> > > > for older guests.
> > > 
> > > How is saving the offset going to prevent a large jump from triggering
> > > the softlock message?  Won't we still have not touched the watchdog for
> > > that long period of time?
> > 
> > The idea is to adjust the offset and the TSC value so that the guest
> > does not notice the actual elapsed time. This is what happens when a
> > guest is migrated or saved/restore, for example.
> 
> but that's within a rather short period of time... if we stop the guest
> for a day, I don't think what you are suggesting can happen without time
> being wrong in the guest, 

Right.

> and as soon as it's corrected and we jump forward larger than the
> watchdog timeout, won't we trigger it again?

The time correction within the guest happens separately from the
kvmclock clocksource, so no, watchdog timeout will not trigger.

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Eric B Munson]

On Tue, Aug 30, 2011 at 06:26, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
>> Currently, when qemu stops a guest kernel that guest will issue a soft lockup
>> message when it resumes.  This set provides the ability for qemu to comminucate
>> to the guest that it has been stopped.  When the guest hits the watchdog on
>> resume it will check if it was suspended before issuing the warning.
>>
>> Eric B Munson (4):
>>   Add flag to indicate that a vm was stopped by the host
>>   Add functions to check if the host has stopped the vm
>>   Add generic stubs for kvm stop check functions
>>   Add check for suspended vm in softlockup detector
>>
>>  arch/x86/include/asm/pvclock-abi.h |    1 +
>>  arch/x86/include/asm/pvclock.h     |    2 ++
>>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
>>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
>>  kernel/watchdog.c                  |   12 ++++++++++++
>>  5 files changed, 43 insertions(+), 0 deletions(-)
>>  create mode 100644 include/asm-generic/pvclock.h
>>
>> --
>> 1.7.4.1
>
> How is the host supposed to set this flag?

The hypervisor will set the flag (the second bit in the struct
pvclock_vcpu_time_info.flags field) when it stops the guest.  I could
be reading pvclock-abi.h incorrectly, but the way I understand it is
that this structure is used to communicate between hypervisor and
guest.

>
> As mentioned previously, if you save save/restore the offset added to
> kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> paravirt infrastructure is required. Which means the issue is also fixed
> for older guests.
>

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Tue, Aug 30, 2011 at 10:37:13AM -0600, Eric B Munson wrote:
> On Tue, Aug 30, 2011 at 06:26, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> >> Currently, when qemu stops a guest kernel that guest will issue a soft lockup
> >> message when it resumes.  This set provides the ability for qemu to comminucate
> >> to the guest that it has been stopped.  When the guest hits the watchdog on
> >> resume it will check if it was suspended before issuing the warning.
> >>
> >> Eric B Munson (4):
> >>   Add flag to indicate that a vm was stopped by the host
> >>   Add functions to check if the host has stopped the vm
> >>   Add generic stubs for kvm stop check functions
> >>   Add check for suspended vm in softlockup detector
> >>
> >>  arch/x86/include/asm/pvclock-abi.h |    1 +
> >>  arch/x86/include/asm/pvclock.h     |    2 ++
> >>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> >>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> >>  kernel/watchdog.c                  |   12 ++++++++++++
> >>  5 files changed, 43 insertions(+), 0 deletions(-)
> >>  create mode 100644 include/asm-generic/pvclock.h
> >>
> >> --
> >> 1.7.4.1
> >
> > How is the host supposed to set this flag?
> 
> The hypervisor will set the flag (the second bit in the struct
> pvclock_vcpu_time_info.flags field) when it stops the guest.  I could
> be reading pvclock-abi.h incorrectly, but the way I understand it is
> that this structure is used to communicate between hypervisor and
> guest.

Yes, its awkward though (to have an interface to modify a bit in
pvclock->flags from QEMU).

Anyway, it appears that all elements to fix this in QEMU without the
need for a paravirt interface are available... did you look into my
suggestion?

> > As mentioned previously, if you save save/restore the offset added to
> > kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > paravirt infrastructure is required. Which means the issue is also fixed
> > for older guests.
> >

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[emunson]

 On Tue, 30 Aug 2011 15:36:13 -0300, Marcelo Tosatti wrote:
> On Tue, Aug 30, 2011 at 10:37:13AM -0600, Eric B Munson wrote:
>> On Tue, Aug 30, 2011 at 06:26, Marcelo Tosatti <mtosatti@redhat.com> 
>> wrote:
>> > On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
>> >> Currently, when qemu stops a guest kernel that guest will issue a 
>> soft lockup
>> >> message when it resumes.  This set provides the ability for qemu 
>> to comminucate
>> >> to the guest that it has been stopped.  When the guest hits the 
>> watchdog on
>> >> resume it will check if it was suspended before issuing the 
>> warning.
>> >>
>> >> Eric B Munson (4):
>> >>   Add flag to indicate that a vm was stopped by the host
>> >>   Add functions to check if the host has stopped the vm
>> >>   Add generic stubs for kvm stop check functions
>> >>   Add check for suspended vm in softlockup detector
>> >>
>> >>  arch/x86/include/asm/pvclock-abi.h |    1 +
>> >>  arch/x86/include/asm/pvclock.h     |    2 ++
>> >>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
>> >>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
>> >>  kernel/watchdog.c                  |   12 ++++++++++++
>> >>  5 files changed, 43 insertions(+), 0 deletions(-)
>> >>  create mode 100644 include/asm-generic/pvclock.h
>> >>
>> >> --
>> >> 1.7.4.1
>> >
>> > How is the host supposed to set this flag?
>>
>> The hypervisor will set the flag (the second bit in the struct
>> pvclock_vcpu_time_info.flags field) when it stops the guest.  I 
>> could
>> be reading pvclock-abi.h incorrectly, but the way I understand it is
>> that this structure is used to communicate between hypervisor and
>> guest.
>
> Yes, its awkward though (to have an interface to modify a bit in
> pvclock->flags from QEMU).
>
> Anyway, it appears that all elements to fix this in QEMU without the
> need for a paravirt interface are available... did you look into my
> suggestion?

 I have not yet, but will look into it.  I sent these out because I was
 under the impression that there was some discussion at LinuxCon about
 your suggestion and that it was agreed that the all user space solution
 was not feasible.

>
>> > As mentioned previously, if you save save/restore the offset added 
>> to
>> > kvmclock on stop/cont (and the TSC MSR, forgot to mention that), 
>> no
>> > paravirt infrastructure is required. Which means the issue is also 
>> fixed
>> > for older guests.
>> >

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Anthony Liguori]

On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
> On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
>> Currently, when qemu stops a guest kernel that guest will issue a soft lockup
>> message when it resumes.  This set provides the ability for qemu to comminucate
>> to the guest that it has been stopped.  When the guest hits the watchdog on
>> resume it will check if it was suspended before issuing the warning.
>>
>> Eric B Munson (4):
>>    Add flag to indicate that a vm was stopped by the host
>>    Add functions to check if the host has stopped the vm
>>    Add generic stubs for kvm stop check functions
>>    Add check for suspended vm in softlockup detector
>>
>>   arch/x86/include/asm/pvclock-abi.h |    1 +
>>   arch/x86/include/asm/pvclock.h     |    2 ++
>>   arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
>>   include/asm-generic/pvclock.h      |   14 ++++++++++++++
>>   kernel/watchdog.c                  |   12 ++++++++++++
>>   5 files changed, 43 insertions(+), 0 deletions(-)
>>   create mode 100644 include/asm-generic/pvclock.h
>>
>> --
>> 1.7.4.1
>
> How is the host supposed to set this flag?
>
> As mentioned previously, if you save save/restore the offset added to
> kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> paravirt infrastructure is required. Which means the issue is also fixed
> for older guests.

IIRC, the steal time patches have some logic that basically say:

if there was steal time:
    kick soft lockup detector

I wonder if that serves this purpose provided that time spent in stop is 
accounted as steal time.  If it isn't, perhaps it should be?

Regards,

Anthony Liguori

> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[emunson]

 On Thu, 01 Sep 2011 14:24:12 -0500, Anthony Liguori wrote:
> On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
>> On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
>>> Currently, when qemu stops a guest kernel that guest will issue a 
>>> soft lockup
>>> message when it resumes.  This set provides the ability for qemu to 
>>> comminucate
>>> to the guest that it has been stopped.  When the guest hits the 
>>> watchdog on
>>> resume it will check if it was suspended before issuing the 
>>> warning.
>>>
>>> Eric B Munson (4):
>>>    Add flag to indicate that a vm was stopped by the host
>>>    Add functions to check if the host has stopped the vm
>>>    Add generic stubs for kvm stop check functions
>>>    Add check for suspended vm in softlockup detector
>>>
>>>   arch/x86/include/asm/pvclock-abi.h |    1 +
>>>   arch/x86/include/asm/pvclock.h     |    2 ++
>>>   arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
>>>   include/asm-generic/pvclock.h      |   14 ++++++++++++++
>>>   kernel/watchdog.c                  |   12 ++++++++++++
>>>   5 files changed, 43 insertions(+), 0 deletions(-)
>>>   create mode 100644 include/asm-generic/pvclock.h
>>>
>>> --
>>> 1.7.4.1
>>
>> How is the host supposed to set this flag?
>>
>> As mentioned previously, if you save save/restore the offset added 
>> to
>> kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
>> paravirt infrastructure is required. Which means the issue is also 
>> fixed
>> for older guests.
>
> IIRC, the steal time patches have some logic that basically say:
>
> if there was steal time:
>    kick soft lockup detector
>
> I wonder if that serves this purpose provided that time spent in stop
> is accounted as steal time.  If it isn't, perhaps it should be?
>
> Regards,
>
> Anthony Liguori
>

 I could be missing it, but I don't see anywhere in the steal time 
 patches that kicks the watchdog.

 Accounting stopped time as stolen time opens a possible problem when 
 the accounting (CPU power modification) part is turned on.  As a process 
 accumulates steal time its CPU power is increased.  If we account 
 stopped time as stolen, it will do strange things with CPU power for 
 that process.  I believe that it is for this reason that patch 4 of 
 Glauber's series explicitly states that halted time is not stolen time.  
 Stolen time is only accumulated when a vCPU actually has work to do.

 Eric

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Thu, Sep 01, 2011 at 02:27:49PM -0600, emunson@mgebm.net wrote:
> On Thu, 01 Sep 2011 14:24:12 -0500, Anthony Liguori wrote:
> >On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
> >>On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> >>>Currently, when qemu stops a guest kernel that guest will
> >>>issue a soft lockup
> >>>message when it resumes.  This set provides the ability for
> >>>qemu to comminucate
> >>>to the guest that it has been stopped.  When the guest hits
> >>>the watchdog on
> >>>resume it will check if it was suspended before issuing the
> >>>warning.
> >>>
> >>>Eric B Munson (4):
> >>>   Add flag to indicate that a vm was stopped by the host
> >>>   Add functions to check if the host has stopped the vm
> >>>   Add generic stubs for kvm stop check functions
> >>>   Add check for suspended vm in softlockup detector
> >>>
> >>>  arch/x86/include/asm/pvclock-abi.h |    1 +
> >>>  arch/x86/include/asm/pvclock.h     |    2 ++
> >>>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> >>>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> >>>  kernel/watchdog.c                  |   12 ++++++++++++
> >>>  5 files changed, 43 insertions(+), 0 deletions(-)
> >>>  create mode 100644 include/asm-generic/pvclock.h
> >>>
> >>>--
> >>>1.7.4.1
> >>
> >>How is the host supposed to set this flag?
> >>
> >>As mentioned previously, if you save save/restore the offset
> >>added to
> >>kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> >>paravirt infrastructure is required. Which means the issue is
> >>also fixed
> >>for older guests.
> >
> >IIRC, the steal time patches have some logic that basically say:
> >
> >if there was steal time:
> >   kick soft lockup detector
> >
> >I wonder if that serves this purpose provided that time spent in stop
> >is accounted as steal time.  If it isn't, perhaps it should be?

Accounting "in userspace (qemu)" as stolen time is problematic. Think
irqchip in userspace (halt emulation), excessive time spent in device
emulation that would trigger genuine watchdog warnings.

The latest patches from Glauber dropped this logic.


> >
> >Regards,
> >
> >Anthony Liguori
> >
> 
> I could be missing it, but I don't see anywhere in the steal time
> patches that kicks the watchdog.
> 
> Accounting stopped time as stolen time opens a possible problem when
> the accounting (CPU power modification) part is turned on.  As a
> process accumulates steal time its CPU power is increased.  If we
> account stopped time as stolen, it will do strange things with CPU
> power for that process.  I believe that it is for this reason that
> patch 4 of Glauber's series explicitly states that halted time is
> not stolen time.  Stolen time is only accumulated when a vCPU
> actually has work to do.
> 
> Eric
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Eric B Munson]

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

On Fri, 09 Sep 2011, Marcelo Tosatti wrote:

> On Thu, Sep 01, 2011 at 02:27:49PM -0600, emunson@mgebm.net wrote:
> > On Thu, 01 Sep 2011 14:24:12 -0500, Anthony Liguori wrote:
> > >On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
> > >>On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > >>>Currently, when qemu stops a guest kernel that guest will
> > >>>issue a soft lockup
> > >>>message when it resumes.  This set provides the ability for
> > >>>qemu to comminucate
> > >>>to the guest that it has been stopped.  When the guest hits
> > >>>the watchdog on
> > >>>resume it will check if it was suspended before issuing the
> > >>>warning.
> > >>>
> > >>>Eric B Munson (4):
> > >>>   Add flag to indicate that a vm was stopped by the host
> > >>>   Add functions to check if the host has stopped the vm
> > >>>   Add generic stubs for kvm stop check functions
> > >>>   Add check for suspended vm in softlockup detector
> > >>>
> > >>>  arch/x86/include/asm/pvclock-abi.h |    1 +
> > >>>  arch/x86/include/asm/pvclock.h     |    2 ++
> > >>>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > >>>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > >>>  kernel/watchdog.c                  |   12 ++++++++++++
> > >>>  5 files changed, 43 insertions(+), 0 deletions(-)
> > >>>  create mode 100644 include/asm-generic/pvclock.h
> > >>>
> > >>>--
> > >>>1.7.4.1
> > >>
> > >>How is the host supposed to set this flag?
> > >>
> > >>As mentioned previously, if you save save/restore the offset
> > >>added to
> > >>kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > >>paravirt infrastructure is required. Which means the issue is
> > >>also fixed
> > >>for older guests.
> > >

Marcelo,

I think that stopping the TSC is the wrong approach because it will break time
between the two systems so timething that expects the monotonic clock to move
consistently will be wrong.  IMO, messing with the TSC at run time to avoid a
watchdog message is the wrong solution, better to teach the watchdog to ignore
this special case.

Thanks,
Eric

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Tue, Sep 13, 2011 at 04:49:55PM -0400, Eric B Munson wrote:
> On Fri, 09 Sep 2011, Marcelo Tosatti wrote:
> 
> > On Thu, Sep 01, 2011 at 02:27:49PM -0600, emunson@mgebm.net wrote:
> > > On Thu, 01 Sep 2011 14:24:12 -0500, Anthony Liguori wrote:
> > > >On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
> > > >>On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > > >>>Currently, when qemu stops a guest kernel that guest will
> > > >>>issue a soft lockup
> > > >>>message when it resumes.  This set provides the ability for
> > > >>>qemu to comminucate
> > > >>>to the guest that it has been stopped.  When the guest hits
> > > >>>the watchdog on
> > > >>>resume it will check if it was suspended before issuing the
> > > >>>warning.
> > > >>>
> > > >>>Eric B Munson (4):
> > > >>>   Add flag to indicate that a vm was stopped by the host
> > > >>>   Add functions to check if the host has stopped the vm
> > > >>>   Add generic stubs for kvm stop check functions
> > > >>>   Add check for suspended vm in softlockup detector
> > > >>>
> > > >>>  arch/x86/include/asm/pvclock-abi.h |    1 +
> > > >>>  arch/x86/include/asm/pvclock.h     |    2 ++
> > > >>>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > > >>>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > > >>>  kernel/watchdog.c                  |   12 ++++++++++++
> > > >>>  5 files changed, 43 insertions(+), 0 deletions(-)
> > > >>>  create mode 100644 include/asm-generic/pvclock.h
> > > >>>
> > > >>>--
> > > >>>1.7.4.1
> > > >>
> > > >>How is the host supposed to set this flag?
> > > >>
> > > >>As mentioned previously, if you save save/restore the offset
> > > >>added to
> > > >>kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > > >>paravirt infrastructure is required. Which means the issue is
> > > >>also fixed
> > > >>for older guests.
> > > >
> 
> Marcelo,
> 
> I think that stopping the TSC is the wrong approach because it will break time
> between the two systems so timething that expects the monotonic clock to move
> consistently will be wrong.

In case the VM stops for whatever reason, the host system is not
supposed to adjust time related hardware state to compensate, in an
attempt to present apparent continuous time.

If you save a VM and then restore it later, it is the guest
responsability to adjust its time representation.

QEMU exposing continuous TSC and kvmclock state between "stop" and
"cont" should not be a reason to introduce new paravirt infrastructure.

>  IMO, messing with the TSC at run time to avoid a watchdog message
> is the wrong solution, better to teach the watchdog to ignore this
> special case.

OK then, it is not a harmful addition, can you post the QEMU patches to
set the "ignore watchdog" bit.

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Eric B Munson]

[-- Attachment #1: Type: text/plain, Size: 4856 bytes --]

On Thu, 15 Sep 2011, Marcelo Tosatti wrote:

> On Tue, Sep 13, 2011 at 04:49:55PM -0400, Eric B Munson wrote:
> > On Fri, 09 Sep 2011, Marcelo Tosatti wrote:
> > 
> > > On Thu, Sep 01, 2011 at 02:27:49PM -0600, emunson@mgebm.net wrote:
> > > > On Thu, 01 Sep 2011 14:24:12 -0500, Anthony Liguori wrote:
> > > > >On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
> > > > >>On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > > > >>>Currently, when qemu stops a guest kernel that guest will
> > > > >>>issue a soft lockup
> > > > >>>message when it resumes.  This set provides the ability for
> > > > >>>qemu to comminucate
> > > > >>>to the guest that it has been stopped.  When the guest hits
> > > > >>>the watchdog on
> > > > >>>resume it will check if it was suspended before issuing the
> > > > >>>warning.
> > > > >>>
> > > > >>>Eric B Munson (4):
> > > > >>>   Add flag to indicate that a vm was stopped by the host
> > > > >>>   Add functions to check if the host has stopped the vm
> > > > >>>   Add generic stubs for kvm stop check functions
> > > > >>>   Add check for suspended vm in softlockup detector
> > > > >>>
> > > > >>>  arch/x86/include/asm/pvclock-abi.h |    1 +
> > > > >>>  arch/x86/include/asm/pvclock.h     |    2 ++
> > > > >>>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > > > >>>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > > > >>>  kernel/watchdog.c                  |   12 ++++++++++++
> > > > >>>  5 files changed, 43 insertions(+), 0 deletions(-)
> > > > >>>  create mode 100644 include/asm-generic/pvclock.h
> > > > >>>
> > > > >>>--
> > > > >>>1.7.4.1
> > > > >>
> > > > >>How is the host supposed to set this flag?
> > > > >>
> > > > >>As mentioned previously, if you save save/restore the offset
> > > > >>added to
> > > > >>kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > > > >>paravirt infrastructure is required. Which means the issue is
> > > > >>also fixed
> > > > >>for older guests.
> > > > >
> > 
> > Marcelo,
> > 
> > I think that stopping the TSC is the wrong approach because it will break time
> > between the two systems so timething that expects the monotonic clock to move
> > consistently will be wrong.
> 
> In case the VM stops for whatever reason, the host system is not
> supposed to adjust time related hardware state to compensate, in an
> attempt to present apparent continuous time.
> 
> If you save a VM and then restore it later, it is the guest
> responsability to adjust its time representation.
> 
> QEMU exposing continuous TSC and kvmclock state between "stop" and
> "cont" should not be a reason to introduce new paravirt infrastructure.
> 
> >  IMO, messing with the TSC at run time to avoid a watchdog message
> > is the wrong solution, better to teach the watchdog to ignore this
> > special case.
> 
> OK then, it is not a harmful addition, can you post the QEMU patches to
> set the "ignore watchdog" bit.

I am about to start working on the interface to set this bit, but to double
check I tried your suggestion to save the clock value on stop and restore it on
continue (see qemu patch below) and I am seeing very odd behavior from it.  On
random resumes the vm won't come back immediately (it took one 15 mintues to
respond) and the wall clock stays behind my host wall clock by the amount of
time it took to resume.

---
 cpus.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index 54c188c..4573e23 100644
--- a/cpus.c
+++ b/cpus.c
@@ -63,6 +63,7 @@
 #endif /* CONFIG_LINUX */
 
 static CPUState *next_cpu;
+struct kvm_clock_data data;
 
 /***********************************************************/
 void hw_error(const char *fmt, ...)
@@ -788,6 +789,7 @@ static int all_vcpus_paused(void)
 void pause_all_vcpus(void)
 {
     CPUState *penv = first_cpu;
+    int ret;
 
     while (penv) {
         penv->stop = 1;
@@ -803,11 +805,18 @@ void pause_all_vcpus(void)
             penv = (CPUState *)penv->next_cpu;
         }
     }
+
+    ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data);
+    if (ret < 0) {
+        fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret));
+        data.clock = 0;
+    }
 }
 
 void resume_all_vcpus(void)
 {
     CPUState *penv = first_cpu;
+    int ret;
 
     while (penv) {
         penv->stop = 0;
@@ -815,6 +824,11 @@ void resume_all_vcpus(void)
         qemu_cpu_kick(penv);
         penv = (CPUState *)penv->next_cpu;
     }
+    if (data.clock != 0) {
+        ret = kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data);
+        if (ret < 0)
+            fprintf(stderr, "KVM_SET_CLOCK failed: %s\n", strerror(ret));
+    }
 }
 
 static void qemu_tcg_init_vcpu(void *_env)
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Tue, Sep 20, 2011 at 03:00:54PM -0400, Eric B Munson wrote:
> On Thu, 15 Sep 2011, Marcelo Tosatti wrote:
> 
> > On Tue, Sep 13, 2011 at 04:49:55PM -0400, Eric B Munson wrote:
> > > On Fri, 09 Sep 2011, Marcelo Tosatti wrote:
> > > 
> > > > On Thu, Sep 01, 2011 at 02:27:49PM -0600, emunson@mgebm.net wrote:
> > > > > On Thu, 01 Sep 2011 14:24:12 -0500, Anthony Liguori wrote:
> > > > > >On 08/30/2011 07:26 AM, Marcelo Tosatti wrote:
> > > > > >>On Mon, Aug 29, 2011 at 05:27:11PM -0600, Eric B Munson wrote:
> > > > > >>>Currently, when qemu stops a guest kernel that guest will
> > > > > >>>issue a soft lockup
> > > > > >>>message when it resumes.  This set provides the ability for
> > > > > >>>qemu to comminucate
> > > > > >>>to the guest that it has been stopped.  When the guest hits
> > > > > >>>the watchdog on
> > > > > >>>resume it will check if it was suspended before issuing the
> > > > > >>>warning.
> > > > > >>>
> > > > > >>>Eric B Munson (4):
> > > > > >>>   Add flag to indicate that a vm was stopped by the host
> > > > > >>>   Add functions to check if the host has stopped the vm
> > > > > >>>   Add generic stubs for kvm stop check functions
> > > > > >>>   Add check for suspended vm in softlockup detector
> > > > > >>>
> > > > > >>>  arch/x86/include/asm/pvclock-abi.h |    1 +
> > > > > >>>  arch/x86/include/asm/pvclock.h     |    2 ++
> > > > > >>>  arch/x86/kernel/kvmclock.c         |   14 ++++++++++++++
> > > > > >>>  include/asm-generic/pvclock.h      |   14 ++++++++++++++
> > > > > >>>  kernel/watchdog.c                  |   12 ++++++++++++
> > > > > >>>  5 files changed, 43 insertions(+), 0 deletions(-)
> > > > > >>>  create mode 100644 include/asm-generic/pvclock.h
> > > > > >>>
> > > > > >>>--
> > > > > >>>1.7.4.1
> > > > > >>
> > > > > >>How is the host supposed to set this flag?
> > > > > >>
> > > > > >>As mentioned previously, if you save save/restore the offset
> > > > > >>added to
> > > > > >>kvmclock on stop/cont (and the TSC MSR, forgot to mention that), no
> > > > > >>paravirt infrastructure is required. Which means the issue is
> > > > > >>also fixed
> > > > > >>for older guests.
> > > > > >
> > > 
> > > Marcelo,
> > > 
> > > I think that stopping the TSC is the wrong approach because it will break time
> > > between the two systems so timething that expects the monotonic clock to move
> > > consistently will be wrong.
> > 
> > In case the VM stops for whatever reason, the host system is not
> > supposed to adjust time related hardware state to compensate, in an
> > attempt to present apparent continuous time.
> > 
> > If you save a VM and then restore it later, it is the guest
> > responsability to adjust its time representation.
> > 
> > QEMU exposing continuous TSC and kvmclock state between "stop" and
> > "cont" should not be a reason to introduce new paravirt infrastructure.
> > 
> > >  IMO, messing with the TSC at run time to avoid a watchdog message
> > > is the wrong solution, better to teach the watchdog to ignore this
> > > special case.
> > 
> > OK then, it is not a harmful addition, can you post the QEMU patches to
> > set the "ignore watchdog" bit.
> 
> I am about to start working on the interface to set this bit, but to double
> check I tried your suggestion to save the clock value on stop and restore it on
> continue (see qemu patch below) and I am seeing very odd behavior from it.  On
> random resumes the vm won't come back immediately (it took one 15 mintues to
> respond) 

You should also save/restore the TSC MSR for each VCPU. And it appears
there is a missing

kvm_for_each_vcpu(vcpu)
    kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu)

in KVM_SET_CLOCK ioctl handling.

> and the wall clock stays behind my host wall clock by the amount of
> time it took to resume.

This is expected, similar to savevm/loadvm.

> ---
>  cpus.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 54c188c..4573e23 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -63,6 +63,7 @@
>  #endif /* CONFIG_LINUX */
>  
>  static CPUState *next_cpu;
> +struct kvm_clock_data data;
>  
>  /***********************************************************/
>  void hw_error(const char *fmt, ...)
> @@ -788,6 +789,7 @@ static int all_vcpus_paused(void)
>  void pause_all_vcpus(void)
>  {
>      CPUState *penv = first_cpu;
> +    int ret;
>  
>      while (penv) {
>          penv->stop = 1;
> @@ -803,11 +805,18 @@ void pause_all_vcpus(void)
>              penv = (CPUState *)penv->next_cpu;
>          }
>      }
> +
> +    ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data);
> +    if (ret < 0) {
> +        fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret));
> +        data.clock = 0;
> +    }
>  }
>  
>  void resume_all_vcpus(void)
>  {
>      CPUState *penv = first_cpu;
> +    int ret;
>  
>      while (penv) {
>          penv->stop = 0;
> @@ -815,6 +824,11 @@ void resume_all_vcpus(void)
>          qemu_cpu_kick(penv);
>          penv = (CPUState *)penv->next_cpu;
>      }
> +    if (data.clock != 0) {
> +        ret = kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data);
> +        if (ret < 0)
> +            fprintf(stderr, "KVM_SET_CLOCK failed: %s\n", strerror(ret));
> +    }
>  }
>  
>  static void qemu_tcg_init_vcpu(void *_env)
> > 

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Dave Hansen]

On Tue, 2011-09-20 at 16:55 -0300, Marcelo Tosatti wrote:
> > and the wall clock stays behind my host wall clock by the amount of
> > time it took to resume.
> 
> This is expected, similar to savevm/loadvm. 

That seems like pretty undesirable behavior to me.  It's too bad that it
does that with savevm/loadvm, but is it really behavior that we want to
spread?

-- Dave

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Wed, Sep 21, 2011 at 07:11:08AM -0700, Dave Hansen wrote:
> On Tue, 2011-09-20 at 16:55 -0300, Marcelo Tosatti wrote:
> > > and the wall clock stays behind my host wall clock by the amount of
> > > time it took to resume.
> > 
> > This is expected, similar to savevm/loadvm. 
> 
> That seems like pretty undesirable behavior to me.  It's too bad that it
> does that with savevm/loadvm, but is it really behavior that we want to
> spread?

Not really. savevm/loadvm has more widespread use, so a solution could
apply equally to both cases.

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Jamie Lokier]

Marcelo Tosatti wrote:
> In case the VM stops for whatever reason, the host system is not
> supposed to adjust time related hardware state to compensate, in an
> attempt to present apparent continuous time.
> 
> If you save a VM and then restore it later, it is the guest
> responsability to adjust its time representation.

If the guest doesn't know it's been stopped, then its time
representation will be wrong until it finds out, e.g. after a few
minutes with NTP, or even a seconds can be too long.

That is sad when it happens because it breaks the coherence of any
timed-lease caching the guest is involved in.  I.e. where the guest
acquires a lock on some data object (like a file in NFS) that it can
efficiently access without network round trips (similar to MESI), with
all nodes having agreed that it's coherent for, say, 5 seconds before
renewing or breaking.  (It's just a way to reduce latency.)

But we can't trust CLOCK_MONOTONIC when a VM is involved, it's just
one of those facts of life.  So instead the effort is to try and
detect when a VM is involved and then distrust the clock.

(Non-VM) suspend/resume is similar, but there's usually a way to
be notified about that as it happens.

-- Jamie

Re: [PATCH 0/4] Avoid soft lockup message when KVM is stopped by host

[Marcelo Tosatti]

On Tue, Sep 20, 2011 at 09:03:51PM +0100, Jamie Lokier wrote:
> Marcelo Tosatti wrote:
> > In case the VM stops for whatever reason, the host system is not
> > supposed to adjust time related hardware state to compensate, in an
> > attempt to present apparent continuous time.
> > 
> > If you save a VM and then restore it later, it is the guest
> > responsability to adjust its time representation.
> 
> If the guest doesn't know it's been stopped, then its time
> representation will be wrong until it finds out, e.g. after a few
> minutes with NTP, or even a seconds can be too long.
> 
> That is sad when it happens because it breaks the coherence of any
> timed-lease caching the guest is involved in.  I.e. where the guest
> acquires a lock on some data object (like a file in NFS) that it can
> efficiently access without network round trips (similar to MESI), with
> all nodes having agreed that it's coherent for, say, 5 seconds before
> renewing or breaking.  (It's just a way to reduce latency.)
> 
> But we can't trust CLOCK_MONOTONIC when a VM is involved, it's just
> one of those facts of life.  So instead the effort is to try and
> detect when a VM is involved and then distrust the clock.
> 
> (Non-VM) suspend/resume is similar, but there's usually a way to
> be notified about that as it happens.
> 
> -- Jamie

Thats pretty bad.

Time leased caching cannot be enabled in conjunction with save/restore
without VM involvement (the hypervisor management application must
notify the guest so it can drop such cache/writeback pending data).

But, if paravirtual clock is available (Linux VMs), it should be
possible to add an offset similarly to how suspend/resume is performed,
via a high priority channel, before resuming operation. We should look
into that possibility.

Thanks