All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 V11] Avoid soft lockup message when KVM is stopped by host
@ 2012-01-31 15:35 Eric B Munson
  2012-01-31 15:35 ` [PATCH 1/4 V11] Add flag to indicate that a vm was stopped by the host Eric B Munson
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Eric B Munson @ 2012-01-31 15:35 UTC (permalink / raw)
  To: avi
  Cc: Eric B Munson, mingo, hpa, ryanh, aliguori, mtosatti, kvm,
	linux-arch, x86, linux-kernel

Changes from V10:
Return ioctl to per vcpu instead of per vm

Changes from V9:
Use kvm_for_each_vcpu to iterate online vcpu's

Changes from V8:
Make KVM_GUEST_PAUSED a per vm ioctl instead of per vcpu

Changes from V7:
Define KVM_CAP_GUEST_PAUSED and support check
Call mark_page_dirty () after setting PVCLOCK_GUEST_STOPPED

Changes from V6:
Use __this_cpu_and when clearing the PVCLOCK_GUEST_STOPPED flag

Changes from V5:
Collapse generic check_and_clear_guest_stopped into patch 2
Include check_and_clear_guest_stopped defintion to ia64, s390, and powerpc
Change check_and_clear_guest_stopped to use __get_cpu_var instead of taking the
 cpuid arg.
Protect check_and_clear_guest_stopped declaration with CONFIG_KVM_CLOCK check

Changes from V4:
Rename KVM_GUEST_PAUSED to KVMCLOCK_GUEST_PAUSED
Add description of KVMCLOCK_GUEST_PAUSED ioctl to api.txt

Changes from V3:
Include CC's on patch 3
Drop clear flag ioctl and have the watchdog clear the flag when it is reset

Changes from V2:
A new kvm functions defined in kvm_para.h, the only change to pvclock is the
initial flag definition

Changes from V1:
(Thanks Marcelo)
Host code has all been moved to arch/x86/kvm/x86.c
KVM_PAUSE_GUEST was renamed to KVM_GUEST_PAUSED

Cc: mingo@redhat.com
Cc: hpa@zytor.com
Cc: ryanh@linux.vnet.ibm.com
Cc: aliguori@us.ibm.com
Cc: mtosatti@redhat.com
Cc: kvm@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org

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 ioctl for KVMCLOCK_GUEST_STOPPED
  Add check for suspended vm in softlockup detector

 Documentation/virtual/kvm/api.txt   |   13 +++++++++++++
 arch/ia64/include/asm/kvm_para.h    |    5 +++++
 arch/powerpc/include/asm/kvm_para.h |    5 +++++
 arch/s390/include/asm/kvm_para.h    |    5 +++++
 arch/x86/include/asm/kvm_para.h     |    8 ++++++++
 arch/x86/include/asm/pvclock-abi.h  |    1 +
 arch/x86/kernel/kvmclock.c          |   21 +++++++++++++++++++++
 arch/x86/kvm/x86.c                  |   21 +++++++++++++++++++++
 include/linux/kvm.h                 |    3 +++
 kernel/watchdog.c                   |   12 ++++++++++++
 10 files changed, 94 insertions(+), 0 deletions(-)

-- 
1.7.5.4


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

* [PATCH 1/4 V11] Add flag to indicate that a vm was stopped by the host
  2012-01-31 15:35 [PATCH 0/4 V11] Avoid soft lockup message when KVM is stopped by host Eric B Munson
@ 2012-01-31 15:35 ` Eric B Munson
  2012-01-31 15:35 ` [PATCH 2/4 V11] Add functions to check if the host has stopped the vm Eric B Munson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Eric B Munson @ 2012-01-31 15:35 UTC (permalink / raw)
  To: avi
  Cc: Eric B Munson, mingo, hpa, ryanh, aliguori, mtosatti, kvm,
	linux-arch, x86, linux-kernel

This flag will be used to check if the vm was stopped by the host when a soft
lockup was detected.  The host will set the flag when it stops the guest.  On
resume, the guest will check this flag if a soft lockup is detected and skip
issuing the warning.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
Cc: mingo@redhat.com
Cc: hpa@zytor.com
Cc: ryanh@linux.vnet.ibm.com
Cc: aliguori@us.ibm.com
Cc: mtosatti@redhat.com
Cc: kvm@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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.5.4


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

* [PATCH 2/4 V11] Add functions to check if the host has stopped the vm
  2012-01-31 15:35 [PATCH 0/4 V11] Avoid soft lockup message when KVM is stopped by host Eric B Munson
  2012-01-31 15:35 ` [PATCH 1/4 V11] Add flag to indicate that a vm was stopped by the host Eric B Munson
@ 2012-01-31 15:35 ` Eric B Munson
  2012-02-07 18:56   ` Marcelo Tosatti
  2012-01-31 15:35 ` [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED Eric B Munson
  2012-01-31 15:35 ` [PATCH 4/4 V11] Add check for suspended vm in softlockup detector Eric B Munson
  3 siblings, 1 reply; 8+ messages in thread
From: Eric B Munson @ 2012-01-31 15:35 UTC (permalink / raw)
  To: avi
  Cc: Eric B Munson, mingo, hpa, ryanh, aliguori, mtosatti, kvm,
	linux-arch, x86, linux-kernel

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>
asm-generic changes Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: mingo@redhat.com
Cc: hpa@zytor.com
Cc: ryanh@linux.vnet.ibm.com
Cc: aliguori@us.ibm.com
Cc: mtosatti@redhat.com
Cc: kvm@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes from V6:
 Use __this_cpu_and when clearing the PVCLOCK_GUEST_STOPPED flag
Changes from V5:
 Collapse generic stubs into this patch
 check_and_clear_guest_stopped() takes no args and uses __get_cpu_var()
 Include individual definitions in ia64, s390, and powerpc

 arch/ia64/include/asm/kvm_para.h    |    5 +++++
 arch/powerpc/include/asm/kvm_para.h |    5 +++++
 arch/s390/include/asm/kvm_para.h    |    5 +++++
 arch/x86/include/asm/kvm_para.h     |    8 ++++++++
 arch/x86/kernel/kvmclock.c          |   21 +++++++++++++++++++++
 5 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
index 1588aee..2019cb9 100644
--- a/arch/ia64/include/asm/kvm_para.h
+++ b/arch/ia64/include/asm/kvm_para.h
@@ -26,6 +26,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline bool kvm_check_and_clear_guest_paused(void)
+{
+	return false;
+}
+
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 50533f9..1f80293 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -169,6 +169,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return r;
 }
 
+static inline bool kvm_check_and_clear_guest_paused(void)
+{
+	return false;
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* __POWERPC_KVM_PARA_H__ */
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index 6964db2..a988329 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -149,6 +149,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline bool kvm_check_and_clear_guest_paused(void)
+{
+	return false;
+}
+
 #endif
 
 #endif /* __S390_KVM_PARA_H */
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 734c376..99c4bbe 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -95,6 +95,14 @@ struct kvm_vcpu_pv_apf_data {
 extern void kvmclock_init(void);
 extern int kvm_register_clock(char *txt);
 
+#ifdef CONFIG_KVM_CLOCK
+bool kvm_check_and_clear_guest_paused(void);
+#else
+static inline bool kvm_check_and_clear_guest_paused(void)
+{
+	return false;
+}
+#endif /* CONFIG_KVMCLOCK */
 
 /* This instruction is vmcall.  On non-VT architectures, it will generate a
  * trap that we will then rewrite to the appropriate instruction.
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 44842d7..bdf6423 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -22,6 +22,7 @@
 #include <asm/msr.h>
 #include <asm/apic.h>
 #include <linux/percpu.h>
+#include <linux/hardirq.h>
 
 #include <asm/x86_init.h>
 #include <asm/reboot.h>
@@ -114,6 +115,26 @@ static void kvm_get_preset_lpj(void)
 	preset_lpj = lpj;
 }
 
+bool kvm_check_and_clear_guest_paused(void)
+{
+	bool ret = false;
+	struct pvclock_vcpu_time_info *src;
+
+	/*
+	 * per_cpu() is safe here because this function is only called from
+	 * timer functions where preemption is already disabled.
+	 */
+	WARN_ON(!in_atomic());
+	src = &__get_cpu_var(hv_clock);
+	if ((src->flags & PVCLOCK_GUEST_STOPPED) != 0) {
+		__this_cpu_and(hv_clock.flags, ~PVCLOCK_GUEST_STOPPED);
+		ret = true;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kvm_check_and_clear_guest_paused);
+
 static struct clocksource kvm_clock = {
 	.name = "kvm-clock",
 	.read = kvm_clock_get_cycles,
-- 
1.7.5.4


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

* [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED
  2012-01-31 15:35 [PATCH 0/4 V11] Avoid soft lockup message when KVM is stopped by host Eric B Munson
  2012-01-31 15:35 ` [PATCH 1/4 V11] Add flag to indicate that a vm was stopped by the host Eric B Munson
  2012-01-31 15:35 ` [PATCH 2/4 V11] Add functions to check if the host has stopped the vm Eric B Munson
@ 2012-01-31 15:35 ` Eric B Munson
  2012-02-07 18:59   ` Marcelo Tosatti
  2012-01-31 15:35 ` [PATCH 4/4 V11] Add check for suspended vm in softlockup detector Eric B Munson
  3 siblings, 1 reply; 8+ messages in thread
From: Eric B Munson @ 2012-01-31 15:35 UTC (permalink / raw)
  To: avi
  Cc: Eric B Munson, mingo, hpa, ryanh, aliguori, mtosatti, kvm,
	linux-arch, x86, linux-kernel

Now that we have a flag that will tell the guest it was suspended, create an
interface for that communication using a KVM ioctl.

Signed-off-by: Eric B Munson <emunson@mgebm.net>

Cc: mingo@redhat.com
Cc: hpa@zytor.com
Cc: ryanh@linux.vnet.ibm.com
Cc: aliguori@us.ibm.com
Cc: mtosatti@redhat.com
Cc: kvm@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes from V10:
 Return the ioctl to per vcpu instead of per vm
Changes from V9:
 Use kvm_for_each_vcpu to iterate online vcpu's
Changes from V8:
 Make KVM_GUEST_PAUSED a per vm ioctl instead of per vcpu
Changes from V7:
 Define KVM_CAP_GUEST_PAUSED and support check
 Call mark_page_dirty () after setting PVCLOCK_GUEST_STOPPED
Changes from V4:
 Rename KVM_GUEST_PAUSED to KVMCLOCK_GUEST_PAUSED
 Add new ioctl description to api.txt

 Documentation/virtual/kvm/api.txt |   13 +++++++++++++
 arch/x86/kvm/x86.c                |   21 +++++++++++++++++++++
 include/linux/kvm.h               |    3 +++
 3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index e1d94bf..1931e5c 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1491,6 +1491,19 @@ following algorithm:
 Some guests configure the LINT1 NMI input to cause a panic, aiding in
 debugging.
 
+4.65 KVMCLOCK_GUEST_PAUSED
+
+Capability: KVM_CAP_GUEST_PAUSED
+Architechtures: Any that implement pvclocks (currently x86 only)
+Type: vcpu ioctl
+Parameters: None
+Returns: 0 on success, -1 on error
+
+This signals to the host kernel that the specified guest is being paused by
+userspace.  The host will set a flag in the pvclock structure that is checked
+from the soft lockup watchdog.  This ioctl can be called during pause or
+unpause.
+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 14d6cad..c9cabba 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2056,6 +2056,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ASYNC_PF:
+	case KVM_CAP_GUEST_PAUSED:
 	case KVM_CAP_GET_TSC_KHZ:
 		r = 1;
 		break;
@@ -2503,6 +2504,22 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
 	return r;
 }
 
+/*
+ * kvm_set_guest_paused() indicates to the guest kernel that it has been
+ * stopped by the hypervisor.  This function will be called from the host only.
+ * EINVAL is returned when the host attempts to set the flag for a guest that
+ * does not support pv clocks.
+ */
+static int kvm_set_guest_paused(struct kvm_vcpu *vcpu)
+{
+	struct pvclock_vcpu_time_info *src = &vcpu->arch.hv_clock;
+	if (!vcpu->arch.time_page)
+		return -EINVAL;
+	src->flags |= PVCLOCK_GUEST_STOPPED;
+	mark_page_dirty(vcpu->kvm, vcpu->arch.time >> PAGE_SHIFT);
+	return 0;
+}
+
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg)
 {
@@ -2784,6 +2801,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 
 		goto out;
 	}
+	case KVMCLOCK_GUEST_PAUSED: {
+		r = kvm_set_guest_paused(vcpu);
+		break;
+	}
 	default:
 		r = -EINVAL;
 	}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 68e67e5..4ffe0df 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -558,6 +558,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_PPC_PAPR 68
 #define KVM_CAP_S390_GMAP 71
 #define KVM_CAP_TSC_DEADLINE_TIMER 72
+#define KVM_CAP_GUEST_PAUSED 73
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -763,6 +764,8 @@ struct kvm_clock_data {
 #define KVM_CREATE_SPAPR_TCE	  _IOW(KVMIO,  0xa8, struct kvm_create_spapr_tce)
 /* Available with KVM_CAP_RMA */
 #define KVM_ALLOCATE_RMA	  _IOR(KVMIO,  0xa9, struct kvm_allocate_rma)
+/* VM is being stopped by host */
+#define KVMCLOCK_GUEST_PAUSED	  _IO(KVMIO,   0xaa)
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 
-- 
1.7.5.4


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

* [PATCH 4/4 V11] Add check for suspended vm in softlockup detector
  2012-01-31 15:35 [PATCH 0/4 V11] Avoid soft lockup message when KVM is stopped by host Eric B Munson
                   ` (2 preceding siblings ...)
  2012-01-31 15:35 ` [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED Eric B Munson
@ 2012-01-31 15:35 ` Eric B Munson
  3 siblings, 0 replies; 8+ messages in thread
From: Eric B Munson @ 2012-01-31 15:35 UTC (permalink / raw)
  To: avi
  Cc: Eric B Munson, mingo, hpa, ryanh, aliguori, mtosatti, kvm,
	linux-arch, x86, linux-kernel

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.  When the watchdog is reset successfully, clear the guest
paused flag.

Signed-off-by: Eric B Munson <emunson@mgebm.net>
Cc: mingo@redhat.com
Cc: hpa@zytor.com
Cc: ryanh@linux.vnet.ibm.com
Cc: aliguori@us.ibm.com
Cc: mtosatti@redhat.com
Cc: kvm@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes from V3:
 Clear the PAUSED flag when the watchdog is reset

 kernel/watchdog.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 1d7bca7..91485e5 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -25,6 +25,7 @@
 #include <linux/sysctl.h>
 
 #include <asm/irq_regs.h>
+#include <linux/kvm_para.h>
 #include <linux/perf_event.h>
 
 int watchdog_enabled = 1;
@@ -280,6 +281,9 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
 			__this_cpu_write(softlockup_touch_sync, false);
 			sched_clock_tick();
 		}
+
+		/* Clear the guest paused flag on watchdog reset */
+		kvm_check_and_clear_guest_paused();
 		__touch_watchdog();
 		return HRTIMER_RESTART;
 	}
@@ -292,6 +296,14 @@ 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_guest_paused())
+			return HRTIMER_RESTART;
+
 		/* only warn once */
 		if (__this_cpu_read(soft_watchdog_warn) == true)
 			return HRTIMER_RESTART;
-- 
1.7.5.4


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

* Re: [PATCH 2/4 V11] Add functions to check if the host has stopped the vm
  2012-01-31 15:35 ` [PATCH 2/4 V11] Add functions to check if the host has stopped the vm Eric B Munson
@ 2012-02-07 18:56   ` Marcelo Tosatti
  0 siblings, 0 replies; 8+ messages in thread
From: Marcelo Tosatti @ 2012-02-07 18:56 UTC (permalink / raw)
  To: Eric B Munson
  Cc: avi, mingo, hpa, ryanh, aliguori, kvm, linux-arch, x86, linux-kernel

On Tue, Jan 31, 2012 at 10:35:30AM -0500, Eric B Munson wrote:
> 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>
> asm-generic changes Acked-by: Arnd Bergmann <arnd@arndb.de>
> Cc: mingo@redhat.com
> Cc: hpa@zytor.com
> Cc: ryanh@linux.vnet.ibm.com
> Cc: aliguori@us.ibm.com
> Cc: mtosatti@redhat.com
> Cc: kvm@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> Changes from V6:
>  Use __this_cpu_and when clearing the PVCLOCK_GUEST_STOPPED flag
> Changes from V5:
>  Collapse generic stubs into this patch
>  check_and_clear_guest_stopped() takes no args and uses __get_cpu_var()
>  Include individual definitions in ia64, s390, and powerpc
> 
>  arch/ia64/include/asm/kvm_para.h    |    5 +++++
>  arch/powerpc/include/asm/kvm_para.h |    5 +++++
>  arch/s390/include/asm/kvm_para.h    |    5 +++++
>  arch/x86/include/asm/kvm_para.h     |    8 ++++++++
>  arch/x86/kernel/kvmclock.c          |   21 +++++++++++++++++++++
>  5 files changed, 44 insertions(+), 0 deletions(-)

Where is the check_and_clear_guest_stopped() stub in
asm-generic/kvm_para.h? I think it is necessary for 
other architectures.



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

* Re: [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED
  2012-01-31 15:35 ` [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED Eric B Munson
@ 2012-02-07 18:59   ` Marcelo Tosatti
  2012-02-07 21:28     ` Jan Kiszka
  0 siblings, 1 reply; 8+ messages in thread
From: Marcelo Tosatti @ 2012-02-07 18:59 UTC (permalink / raw)
  To: Eric B Munson
  Cc: avi, mingo, hpa, ryanh, aliguori, kvm, linux-arch, x86, linux-kernel

On Tue, Jan 31, 2012 at 10:35:31AM -0500, Eric B Munson wrote:
> Now that we have a flag that will tell the guest it was suspended, create an
> interface for that communication using a KVM ioctl.
> 
> Signed-off-by: Eric B Munson <emunson@mgebm.net>
> 
> Cc: mingo@redhat.com
> Cc: hpa@zytor.com
> Cc: ryanh@linux.vnet.ibm.com
> Cc: aliguori@us.ibm.com
> Cc: mtosatti@redhat.com
> Cc: kvm@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> Changes from V10:
>  Return the ioctl to per vcpu instead of per vm
> Changes from V9:
>  Use kvm_for_each_vcpu to iterate online vcpu's
> Changes from V8:
>  Make KVM_GUEST_PAUSED a per vm ioctl instead of per vcpu
> Changes from V7:
>  Define KVM_CAP_GUEST_PAUSED and support check
>  Call mark_page_dirty () after setting PVCLOCK_GUEST_STOPPED
> Changes from V4:
>  Rename KVM_GUEST_PAUSED to KVMCLOCK_GUEST_PAUSED
>  Add new ioctl description to api.txt
> 
>  Documentation/virtual/kvm/api.txt |   13 +++++++++++++
>  arch/x86/kvm/x86.c                |   21 +++++++++++++++++++++
>  include/linux/kvm.h               |    3 +++
>  3 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index e1d94bf..1931e5c 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1491,6 +1491,19 @@ following algorithm:
>  Some guests configure the LINT1 NMI input to cause a panic, aiding in
>  debugging.
>  
> +4.65 KVMCLOCK_GUEST_PAUSED
> +
> +Capability: KVM_CAP_GUEST_PAUSED
> +Architechtures: Any that implement pvclocks (currently x86 only)

typo: Architectures.


> +Type: vcpu ioctl
> +Parameters: None
> +Returns: 0 on success, -1 on error
> +
> +This signals to the host kernel that the specified guest is being paused by
> +userspace.  The host will set a flag in the pvclock structure that is checked
> +from the soft lockup watchdog.  This ioctl can be called during pause or
> +unpause.
> +
>  5. The kvm_run structure
>  
>  Application code obtains a pointer to the kvm_run structure by
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 14d6cad..c9cabba 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2056,6 +2056,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_X86_ROBUST_SINGLESTEP:
>  	case KVM_CAP_XSAVE:
>  	case KVM_CAP_ASYNC_PF:
> +	case KVM_CAP_GUEST_PAUSED:
>  	case KVM_CAP_GET_TSC_KHZ:
>  		r = 1;
>  		break;
> @@ -2503,6 +2504,22 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
>  	return r;
>  }
>  
> +/*
> + * kvm_set_guest_paused() indicates to the guest kernel that it has been
> + * stopped by the hypervisor.  This function will be called from the host only.
> + * EINVAL is returned when the host attempts to set the flag for a guest that
> + * does not support pv clocks.
> + */
> +static int kvm_set_guest_paused(struct kvm_vcpu *vcpu)
> +{
> +	struct pvclock_vcpu_time_info *src = &vcpu->arch.hv_clock;
> +	if (!vcpu->arch.time_page)
> +		return -EINVAL;
> +	src->flags |= PVCLOCK_GUEST_STOPPED;
> +	mark_page_dirty(vcpu->kvm, vcpu->arch.time >> PAGE_SHIFT);
> +	return 0;
> +}

It should also call kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu), so
that the hv_clock is written to guest memory as soon as possible.

Please fix the comments and rebase as soon as possible, it is annoying 
that this feature is taking so long to be integrated.

Thanks


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

* Re: [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED
  2012-02-07 18:59   ` Marcelo Tosatti
@ 2012-02-07 21:28     ` Jan Kiszka
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2012-02-07 21:28 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Eric B Munson, avi, mingo, hpa, ryanh, aliguori, kvm, linux-arch,
	x86, linux-kernel

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

On 2012-02-07 19:59, Marcelo Tosatti wrote:
> On Tue, Jan 31, 2012 at 10:35:31AM -0500, Eric B Munson wrote:
>> Now that we have a flag that will tell the guest it was suspended, create an
>> interface for that communication using a KVM ioctl.
>>
>> Signed-off-by: Eric B Munson <emunson@mgebm.net>
>>
>> Cc: mingo@redhat.com
>> Cc: hpa@zytor.com
>> Cc: ryanh@linux.vnet.ibm.com
>> Cc: aliguori@us.ibm.com
>> Cc: mtosatti@redhat.com
>> Cc: kvm@vger.kernel.org
>> Cc: linux-arch@vger.kernel.org
>> Cc: x86@kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> ---
>> Changes from V10:
>>  Return the ioctl to per vcpu instead of per vm
>> Changes from V9:
>>  Use kvm_for_each_vcpu to iterate online vcpu's
>> Changes from V8:
>>  Make KVM_GUEST_PAUSED a per vm ioctl instead of per vcpu
>> Changes from V7:
>>  Define KVM_CAP_GUEST_PAUSED and support check
>>  Call mark_page_dirty () after setting PVCLOCK_GUEST_STOPPED
>> Changes from V4:
>>  Rename KVM_GUEST_PAUSED to KVMCLOCK_GUEST_PAUSED
>>  Add new ioctl description to api.txt
>>
>>  Documentation/virtual/kvm/api.txt |   13 +++++++++++++
>>  arch/x86/kvm/x86.c                |   21 +++++++++++++++++++++
>>  include/linux/kvm.h               |    3 +++
>>  3 files changed, 37 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index e1d94bf..1931e5c 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -1491,6 +1491,19 @@ following algorithm:
>>  Some guests configure the LINT1 NMI input to cause a panic, aiding in
>>  debugging.
>>  
>> +4.65 KVMCLOCK_GUEST_PAUSED
>> +
>> +Capability: KVM_CAP_GUEST_PAUSED
>> +Architechtures: Any that implement pvclocks (currently x86 only)
> 
> typo: Architectures.
> 
> 
>> +Type: vcpu ioctl
>> +Parameters: None
>> +Returns: 0 on success, -1 on error
>> +
>> +This signals to the host kernel that the specified guest is being paused by
>> +userspace.  The host will set a flag in the pvclock structure that is checked
>> +from the soft lockup watchdog.  This ioctl can be called during pause or
>> +unpause.
>> +
>>  5. The kvm_run structure
>>  
>>  Application code obtains a pointer to the kvm_run structure by
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 14d6cad..c9cabba 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2056,6 +2056,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>>  	case KVM_CAP_X86_ROBUST_SINGLESTEP:
>>  	case KVM_CAP_XSAVE:
>>  	case KVM_CAP_ASYNC_PF:
>> +	case KVM_CAP_GUEST_PAUSED:
>>  	case KVM_CAP_GET_TSC_KHZ:
>>  		r = 1;
>>  		break;
>> @@ -2503,6 +2504,22 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
>>  	return r;
>>  }
>>  
>> +/*
>> + * kvm_set_guest_paused() indicates to the guest kernel that it has been
>> + * stopped by the hypervisor.  This function will be called from the host only.
>> + * EINVAL is returned when the host attempts to set the flag for a guest that
>> + * does not support pv clocks.
>> + */
>> +static int kvm_set_guest_paused(struct kvm_vcpu *vcpu)
>> +{
>> +	struct pvclock_vcpu_time_info *src = &vcpu->arch.hv_clock;
>> +	if (!vcpu->arch.time_page)
>> +		return -EINVAL;
>> +	src->flags |= PVCLOCK_GUEST_STOPPED;
>> +	mark_page_dirty(vcpu->kvm, vcpu->arch.time >> PAGE_SHIFT);
>> +	return 0;
>> +}
> 
> It should also call kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu), so
> that the hv_clock is written to guest memory as soon as possible.
> 
> Please fix the comments and rebase as soon as possible, it is annoying 
> that this feature is taking so long to be integrated.

In addition:

- Avi asked for a KVM_XXX_ naming pattern of the IOCTL, to align it
  with all the rest.
- He even suggested KVM_CLOCK_CTRL, thus an extensible interface for
  passing flags via kvmclock on a per-vpcu basis.
- As this is now supposed to be per-vcpu, constants should be called
  XXX_VCPU_STOPPED and no longer refer to "guest".

Thanks,
Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

end of thread, other threads:[~2012-02-07 21:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-31 15:35 [PATCH 0/4 V11] Avoid soft lockup message when KVM is stopped by host Eric B Munson
2012-01-31 15:35 ` [PATCH 1/4 V11] Add flag to indicate that a vm was stopped by the host Eric B Munson
2012-01-31 15:35 ` [PATCH 2/4 V11] Add functions to check if the host has stopped the vm Eric B Munson
2012-02-07 18:56   ` Marcelo Tosatti
2012-01-31 15:35 ` [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED Eric B Munson
2012-02-07 18:59   ` Marcelo Tosatti
2012-02-07 21:28     ` Jan Kiszka
2012-01-31 15:35 ` [PATCH 4/4 V11] Add check for suspended vm in softlockup detector Eric B Munson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.