[PATCH v5 0/9] Steal time for KVM

[PATCH v5 0/9] Steal time for KVM

[Glauber Costa]

This is a follow up on the last series. No significant changes.
Only change is in patch 7/9, where the steal time accounting
function is made inline, and also called from
irqtime_process_acount_tick(). At least in my box ( gcc 4.4.5),
gcc insists in not inlining it. So I am resorting to __always_inline.

Glauber Costa (8):
  KVM-HDR Add constant to represent KVM MSRs enabled bit
  KVM-HDR: KVM Steal time implementation
  KVM-HV: KVM Steal time implementation
  KVM-GST: Add a pv_ops stub for steal time
  add jump labels for ia64 paravirt
  KVM-GST: KVM Steal time accounting
  KVM-GST: adjust scheduler cpu power
  KVM-GST: KVM Steal time registration

Gleb Natapov (1):
  introduce kvm_read_guest_cached

 Documentation/kernel-parameters.txt   |    4 ++
 Documentation/virtual/kvm/msr.txt     |   35 +++++++++++++
 arch/ia64/include/asm/paravirt.h      |    4 ++
 arch/ia64/kernel/paravirt.c           |    2 +
 arch/x86/Kconfig                      |   12 +++++
 arch/x86/include/asm/kvm_host.h       |    8 +++
 arch/x86/include/asm/kvm_para.h       |   15 ++++++
 arch/x86/include/asm/paravirt.h       |    9 +++
 arch/x86/include/asm/paravirt_types.h |    1 +
 arch/x86/kernel/kvm.c                 |   73 +++++++++++++++++++++++++++
 arch/x86/kernel/kvmclock.c            |    2 +
 arch/x86/kernel/paravirt.c            |    9 +++
 arch/x86/kvm/Kconfig                  |    1 +
 arch/x86/kvm/x86.c                    |   56 ++++++++++++++++++++-
 include/linux/kvm_host.h              |    2 +
 kernel/sched.c                        |   88 +++++++++++++++++++++++++++++----
 kernel/sched_features.h               |    4 +-
 virt/kvm/kvm_main.c                   |   20 +++++++
 18 files changed, 330 insertions(+), 15 deletions(-)

-- 
1.7.3.4

[PATCH v5 1/9] introduce kvm_read_guest_cached

[Glauber Costa]

From: Gleb Natapov <gleb@redhat.com>

Introduce kvm_read_guest_cached() function in addition to write one we
already have.

[ by glauber: export function signature in kvm header ]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 include/linux/kvm_host.h |    2 ++
 virt/kvm/kvm_main.c      |   20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 31ebb59..f7df0a3 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -381,6 +381,8 @@ int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
 int kvm_read_guest_atomic(struct kvm *kvm, gpa_t gpa, void *data,
 			  unsigned long len);
 int kvm_read_guest(struct kvm *kvm, gpa_t gpa, void *data, unsigned long len);
+int kvm_read_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
+			   void *data, unsigned long len);
 int kvm_write_guest_page(struct kvm *kvm, gfn_t gfn, const void *data,
 			 int offset, int len);
 int kvm_write_guest(struct kvm *kvm, gpa_t gpa, const void *data,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 11d2783..d5ef9eb 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1418,6 +1418,26 @@ int kvm_write_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 }
 EXPORT_SYMBOL_GPL(kvm_write_guest_cached);
 
+int kvm_read_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
+			   void *data, unsigned long len)
+{
+	struct kvm_memslots *slots = kvm_memslots(kvm);
+	int r;
+
+	if (slots->generation != ghc->generation)
+		kvm_gfn_to_hva_cache_init(kvm, ghc, ghc->gpa);
+
+	if (kvm_is_error_hva(ghc->hva))
+		return -EFAULT;
+
+	r = __copy_from_user(data, (void __user *)ghc->hva, len);
+	if (r)
+		return -EFAULT;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kvm_read_guest_cached);
+
 int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len)
 {
 	return kvm_write_guest_page(kvm, gfn, (const void *) empty_zero_page,
-- 
1.7.3.4

Re: [PATCH v5 1/9] introduce kvm_read_guest_cached

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> From: Gleb Natapov <gleb@redhat.com>
> 
> Introduce kvm_read_guest_cached() function in addition to write one we
> already have.
> 
> [ by glauber: export function signature in kvm header ]
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> Signed-off-by: Glauber Costa <glommer@redhat.com>

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


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

Re: [PATCH v5 1/9] introduce kvm_read_guest_cached

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> From: Gleb Natapov<gleb@redhat.com>
>
> Introduce kvm_read_guest_cached() function in addition to write one we
> already have.
>
> [ by glauber: export function signature in kvm header ]
>
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> Signed-off-by: Glauber Costa<glommer@redhat.com>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

[PATCH v5 2/9] KVM-HDR Add constant to represent KVM MSRs enabled bit

[Glauber Costa]

This patch is simple, put in a different commit so it can be more easily
shared between guest and hypervisor. It just defines a named constant
to indicate the enable bit for KVM-specific MSRs.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 arch/x86/include/asm/kvm_para.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index a427bf7..d6cd79b 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -30,6 +30,7 @@
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
 
+#define KVM_MSR_ENABLED 1
 /* Custom MSRs falls in the range 0x4b564d00-0x4b564dff */
 #define MSR_KVM_WALL_CLOCK_NEW  0x4b564d00
 #define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
-- 
1.7.3.4

Re: [PATCH v5 2/9] KVM-HDR Add constant to represent KVM MSRs enabled bit

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> This patch is simple, put in a different commit so it can be more easily
> shared between guest and hypervisor. It just defines a named constant
> to indicate the enable bit for KVM-specific MSRs.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 2/9] KVM-HDR Add constant to represent KVM MSRs enabled bit

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> This patch is simple, put in a different commit so it can be more easily
> shared between guest and hypervisor. It just defines a named constant
> to indicate the enable bit for KVM-specific MSRs.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

[PATCH v5 3/9] KVM-HDR: KVM Steal time implementation

[Glauber Costa]

To implement steal time, we need the hypervisor to pass the guest information
about how much time was spent running other processes outside the VM.
This is per-vcpu, and using the kvmclock structure for that is an abuse
we decided not to make.

In this patchset, I am introducing a new msr, KVM_MSR_STEAL_TIME, that
holds the memory area address containing information about steal time

This patch contains the headers for it. I am keeping it separate to facilitate
backports to people who wants to backport the kernel part but not the
hypervisor, or the other way around.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 Documentation/virtual/kvm/msr.txt |   35 +++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/kvm_para.h   |    9 +++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/Documentation/virtual/kvm/msr.txt b/Documentation/virtual/kvm/msr.txt
index d079aed..38db3f8 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -185,3 +185,38 @@ MSR_KVM_ASYNC_PF_EN: 0x4b564d02
 
 	Currently type 2 APF will be always delivered on the same vcpu as
 	type 1 was, but guest should not rely on that.
+
+MSR_KVM_STEAL_TIME: 0x4b564d03
+
+	data: 64-byte alignment physical address of a memory area which must be
+	in guest RAM, plus an enable bit in bit 0. This memory is expected to
+	hold a copy of the following structure:
+
+	struct kvm_steal_time {
+	  	__u64 steal;
+ 		__u32 version;
+ 		__u32 flags;
+	 	__u32 pad[12];
+	}
+
+	whose data will be filled in by the hypervisor periodically. Only one
+	write, or registration, is needed for each VCPU. The interval between
+	updates of this structure is arbitrary and implementation-dependent.
+	The hypervisor may update this structure at any time it sees fit until
+	anything with bit0 == 0 is written to it. Guest is required to make sure
+	this structure is initialized to zero.
+
+	Fields have the following meanings:
+
+		version: a sequence counter. In other words, guest has to check
+		this field before and after grabbing time information and make 
+		sure they are both equal and even. An odd version indicates an
+		in-progress update.
+
+		flags: At this point, always zero. May be used to indicate
+		changes in this structure in the future.
+
+		steal: the amount of time in which this vCPU did not run, in
+		nanoseconds. Time during which the vcpu is idle, will not be
+		reported as steal time.
+
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index d6cd79b..65f8bb9 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -21,6 +21,7 @@
  */
 #define KVM_FEATURE_CLOCKSOURCE2        3
 #define KVM_FEATURE_ASYNC_PF		4
+#define KVM_FEATURE_STEAL_TIME		5
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -35,6 +36,14 @@
 #define MSR_KVM_WALL_CLOCK_NEW  0x4b564d00
 #define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
+#define MSR_KVM_STEAL_TIME  0x4b564d03
+
+struct kvm_steal_time {
+	__u64 steal;
+	__u32 version;
+	__u32 flags;
+	__u32 pad[12];
+};
 
 #define KVM_MAX_MMU_OP_BATCH           32
 
-- 
1.7.3.4

Re: [PATCH v5 3/9] KVM-HDR: KVM Steal time implementation

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> To implement steal time, we need the hypervisor to pass the guest information
> about how much time was spent running other processes outside the VM.
> This is per-vcpu, and using the kvmclock structure for that is an abuse
> we decided not to make.
> 
> In this patchset, I am introducing a new msr, KVM_MSR_STEAL_TIME, that
> holds the memory area address containing information about steal time
> 
> This patch contains the headers for it. I am keeping it separate to facilitate
> backports to people who wants to backport the kernel part but not the
> hypervisor, or the other way around.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 3/9] KVM-HDR: KVM Steal time implementation

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> To implement steal time, we need the hypervisor to pass the guest information
> about how much time was spent running other processes outside the VM.
> This is per-vcpu, and using the kvmclock structure for that is an abuse
> we decided not to make.
>
> In this patchset, I am introducing a new msr, KVM_MSR_STEAL_TIME, that
> holds the memory area address containing information about steal time
>
> This patch contains the headers for it. I am keeping it separate to facilitate
> backports to people who wants to backport the kernel part but not the
> hypervisor, or the other way around.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>


-- 
All rights reversed

[PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Glauber Costa]

To implement steal time, we need the hypervisor to pass the guest
information about how much time was spent running other processes
outside the VM, while the vcpu had meaningful work to do - halt
time does not count.

This information is acquired through the run_delay field of
delayacct/schedstats infrastructure, that counts time spent in a
runqueue but not running.

Steal time is a per-cpu information, so the traditional MSR-based
infrastructure is used. A new msr, KVM_MSR_STEAL_TIME, holds the
memory area address containing information about steal time

This patch contains the hypervisor part of the steal time infrasructure,
and can be backported independently of the guest portion.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 arch/x86/include/asm/kvm_host.h |    8 +++++
 arch/x86/include/asm/kvm_para.h |    4 +++
 arch/x86/kvm/Kconfig            |    1 +
 arch/x86/kvm/x86.c              |   56 ++++++++++++++++++++++++++++++++++++--
 4 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index da6bbee..9ba354d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -389,6 +389,14 @@ struct kvm_vcpu_arch {
 	unsigned int hw_tsc_khz;
 	unsigned int time_offset;
 	struct page *time_page;
+
+	struct {
+		u64 msr_val;
+		u64 last_steal;
+		struct gfn_to_hva_cache stime;
+		struct kvm_steal_time steal;
+	} st;
+
 	u64 last_guest_tsc;
 	u64 last_kernel_ns;
 	u64 last_tsc_nsec;
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 65f8bb9..c484ba8 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -45,6 +45,10 @@ struct kvm_steal_time {
 	__u32 pad[12];
 };
 
+#define KVM_STEAL_ALIGNMENT_BITS 5
+#define KVM_STEAL_VALID_BITS ((-1ULL << (KVM_STEAL_ALIGNMENT_BITS + 1)))
+#define KVM_STEAL_RESERVED_MASK (((1 << KVM_STEAL_ALIGNMENT_BITS) - 1 ) << 1)
+
 #define KVM_MAX_MMU_OP_BATCH           32
 
 #define KVM_ASYNC_PF_ENABLED			(1 << 0)
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 50f6364..99c3f05 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -31,6 +31,7 @@ config KVM
 	select KVM_ASYNC_PF
 	select USER_RETURN_NOTIFIER
 	select KVM_MMIO
+	select TASK_DELAY_ACCT
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7167717..237bcdc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -808,12 +808,12 @@ EXPORT_SYMBOL_GPL(kvm_get_dr);
  * kvm-specific. Those are put in the beginning of the list.
  */
 
-#define KVM_SAVE_MSRS_BEGIN	8
+#define KVM_SAVE_MSRS_BEGIN	9
 static u32 msrs_to_save[] = {
 	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
 	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
 	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
-	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN,
+	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
 	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
 	MSR_STAR,
 #ifdef CONFIG_X86_64
@@ -1491,6 +1491,27 @@ static void kvmclock_reset(struct kvm_vcpu *vcpu)
 	}
 }
 
+static void record_steal_time(struct kvm_vcpu *vcpu)
+{
+	u64 delta;
+
+	if (!(vcpu->arch.st.msr_val & KVM_MSR_ENABLED))
+		return;
+
+	if (unlikely(kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.st.stime,
+		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time))))
+		return;
+
+	delta = current->sched_info.run_delay - vcpu->arch.st.last_steal;
+	vcpu->arch.st.last_steal = current->sched_info.run_delay;
+
+	vcpu->arch.st.steal.steal += delta;
+	vcpu->arch.st.steal.version += 2;
+
+	kvm_write_guest_cached(vcpu->kvm, &vcpu->arch.st.stime,
+		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time));
+}
+
 int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 {
 	switch (msr) {
@@ -1573,6 +1594,28 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 		if (kvm_pv_enable_async_pf(vcpu, data))
 			return 1;
 		break;
+	case MSR_KVM_STEAL_TIME:
+		vcpu->arch.st.msr_val = data;
+
+		if (!(data & KVM_MSR_ENABLED)) {
+			break;
+		}
+
+		if (unlikely(!sched_info_on()))
+			break;
+
+		if (data & KVM_STEAL_RESERVED_MASK)
+			return 1;
+
+		if (kvm_gfn_to_hva_cache_init(vcpu->kvm, &vcpu->arch.st.stime,
+							data & KVM_STEAL_VALID_BITS))
+			return 1;
+
+		vcpu->arch.st.last_steal = current->sched_info.run_delay;
+
+		record_steal_time(vcpu);
+		break;
+
 	case MSR_IA32_MCG_CTL:
 	case MSR_IA32_MCG_STATUS:
 	case MSR_IA32_MC0_CTL ... MSR_IA32_MC0_CTL + 4 * KVM_MAX_MCE_BANKS - 1:
@@ -1858,6 +1901,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_KVM_ASYNC_PF_EN:
 		data = vcpu->arch.apf.msr_val;
 		break;
+	case MSR_KVM_STEAL_TIME:
+		data = vcpu->arch.st.msr_val;
+		break;
 	case MSR_IA32_P5_MC_ADDR:
 	case MSR_IA32_P5_MC_TYPE:
 	case MSR_IA32_MCG_CAP:
@@ -2169,6 +2215,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 			kvm_migrate_timers(vcpu);
 		vcpu->cpu = cpu;
 	}
+
+	record_steal_time(vcpu);
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
@@ -2489,7 +2537,8 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			     (1 << KVM_FEATURE_NOP_IO_DELAY) |
 			     (1 << KVM_FEATURE_CLOCKSOURCE2) |
 			     (1 << KVM_FEATURE_ASYNC_PF) |
-			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
+			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT) |
+			     (1 << KVM_FEATURE_STEAL_TIME);
 		entry->ebx = 0;
 		entry->ecx = 0;
 		entry->edx = 0;
@@ -6209,6 +6258,7 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
 
 	kvm_make_request(KVM_REQ_EVENT, vcpu);
 	vcpu->arch.apf.msr_val = 0;
+	vcpu->arch.st.msr_val = 0;
 
 	kvmclock_reset(vcpu);
 
-- 
1.7.3.4

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> To implement steal time, we need the hypervisor to pass the guest
> information about how much time was spent running other processes
> outside the VM, while the vcpu had meaningful work to do - halt
> time does not count.
> 
> This information is acquired through the run_delay field of
> delayacct/schedstats infrastructure, that counts time spent in a
> runqueue but not running.
> 
> Steal time is a per-cpu information, so the traditional MSR-based
> infrastructure is used. A new msr, KVM_MSR_STEAL_TIME, holds the
> memory area address containing information about steal time
> 
> This patch contains the hypervisor part of the steal time infrasructure,
> and can be backported independently of the guest portion.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> To implement steal time, we need the hypervisor to pass the guest
> information about how much time was spent running other processes
> outside the VM, while the vcpu had meaningful work to do - halt
> time does not count.
>
> This information is acquired through the run_delay field of
> delayacct/schedstats infrastructure, that counts time spent in a
> runqueue but not running.
>
> Steal time is a per-cpu information, so the traditional MSR-based
> infrastructure is used. A new msr, KVM_MSR_STEAL_TIME, holds the
> memory area address containing information about steal time
>
> This patch contains the hypervisor part of the steal time infrasructure,
> and can be backported independently of the guest portion.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>


-- 
All rights reversed

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Marcelo Tosatti]

On Mon, Jul 04, 2011 at 11:32:23AM -0400, Glauber Costa wrote:
> To implement steal time, we need the hypervisor to pass the guest
> information about how much time was spent running other processes
> outside the VM, while the vcpu had meaningful work to do - halt
> time does not count.
> 
> This information is acquired through the run_delay field of
> delayacct/schedstats infrastructure, that counts time spent in a
> runqueue but not running.
> 
> Steal time is a per-cpu information, so the traditional MSR-based
> infrastructure is used. A new msr, KVM_MSR_STEAL_TIME, holds the
> memory area address containing information about steal time
> 
> This patch contains the hypervisor part of the steal time infrasructure,
> and can be backported independently of the guest portion.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>
> ---
>  arch/x86/include/asm/kvm_host.h |    8 +++++
>  arch/x86/include/asm/kvm_para.h |    4 +++
>  arch/x86/kvm/Kconfig            |    1 +
>  arch/x86/kvm/x86.c              |   56 ++++++++++++++++++++++++++++++++++++--
>  4 files changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index da6bbee..9ba354d 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -389,6 +389,14 @@ struct kvm_vcpu_arch {
>  	unsigned int hw_tsc_khz;
>  	unsigned int time_offset;
>  	struct page *time_page;
> +
> +	struct {
> +		u64 msr_val;
> +		u64 last_steal;
> +		struct gfn_to_hva_cache stime;
> +		struct kvm_steal_time steal;
> +	} st;
> +
>  	u64 last_guest_tsc;
>  	u64 last_kernel_ns;
>  	u64 last_tsc_nsec;
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 65f8bb9..c484ba8 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -45,6 +45,10 @@ struct kvm_steal_time {
>  	__u32 pad[12];
>  };
>  
> +#define KVM_STEAL_ALIGNMENT_BITS 5
> +#define KVM_STEAL_VALID_BITS ((-1ULL << (KVM_STEAL_ALIGNMENT_BITS + 1)))
> +#define KVM_STEAL_RESERVED_MASK (((1 << KVM_STEAL_ALIGNMENT_BITS) - 1 ) << 1)
> +
>  #define KVM_MAX_MMU_OP_BATCH           32
>  
>  #define KVM_ASYNC_PF_ENABLED			(1 << 0)
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 50f6364..99c3f05 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -31,6 +31,7 @@ config KVM
>  	select KVM_ASYNC_PF
>  	select USER_RETURN_NOTIFIER
>  	select KVM_MMIO
> +	select TASK_DELAY_ACCT
>  	---help---
>  	  Support hosting fully virtualized guest machines using hardware
>  	  virtualization extensions.  You will need a fairly recent
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 7167717..237bcdc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -808,12 +808,12 @@ EXPORT_SYMBOL_GPL(kvm_get_dr);
>   * kvm-specific. Those are put in the beginning of the list.
>   */
>  
> -#define KVM_SAVE_MSRS_BEGIN	8
> +#define KVM_SAVE_MSRS_BEGIN	9
>  static u32 msrs_to_save[] = {
>  	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>  	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
>  	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
> -	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN,
> +	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>  	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
>  	MSR_STAR,
>  #ifdef CONFIG_X86_64
> @@ -1491,6 +1491,27 @@ static void kvmclock_reset(struct kvm_vcpu *vcpu)
>  	}
>  }
>  
> +static void record_steal_time(struct kvm_vcpu *vcpu)
> +{
> +	u64 delta;
> +
> +	if (!(vcpu->arch.st.msr_val & KVM_MSR_ENABLED))
> +		return;
> +
> +	if (unlikely(kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.st.stime,
> +		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time))))
> +		return;

The guest memory page is not pinned, sleeping via
__copy_from_user/to_user is not allowed in vcpu_load context. Either pin
it or use atomic acessors.

> +	case MSR_KVM_STEAL_TIME:
> +		vcpu->arch.st.msr_val = data;
> +
> +		if (!(data & KVM_MSR_ENABLED)) {
> +			break;
> +		}

On failure below this point, msr_val should be cleared of KVM_MSR_ENABLED?

> +
> +		if (unlikely(!sched_info_on()))
> +			break;
> +
> +		if (data & KVM_STEAL_RESERVED_MASK)
> +			return 1;
> +
> +		if (kvm_gfn_to_hva_cache_init(vcpu->kvm, &vcpu->arch.st.stime,
> +							data & KVM_STEAL_VALID_BITS))
> +			return 1;
> +
> +		vcpu->arch.st.last_steal = current->sched_info.run_delay;
> +
> +		record_steal_time(vcpu);
> +		break;
> +

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Glauber Costa]

On 07/07/2011 07:51 AM, Marcelo Tosatti wrote:
> On Mon, Jul 04, 2011 at 11:32:23AM -0400, Glauber Costa wrote:
>> To implement steal time, we need the hypervisor to pass the guest
>> information about how much time was spent running other processes
>> outside the VM, while the vcpu had meaningful work to do - halt
>> time does not count.
>>
>> This information is acquired through the run_delay field of
>> delayacct/schedstats infrastructure, that counts time spent in a
>> runqueue but not running.
>>
>> Steal time is a per-cpu information, so the traditional MSR-based
>> infrastructure is used. A new msr, KVM_MSR_STEAL_TIME, holds the
>> memory area address containing information about steal time
>>
>> This patch contains the hypervisor part of the steal time infrasructure,
>> and can be backported independently of the guest portion.
>>
>> Signed-off-by: Glauber Costa<glommer@redhat.com>
>> CC: Rik van Riel<riel@redhat.com>
>> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
>> CC: Peter Zijlstra<peterz@infradead.org>
>> CC: Avi Kivity<avi@redhat.com>
>> CC: Anthony Liguori<aliguori@us.ibm.com>
>> CC: Eric B Munson<emunson@mgebm.net>
>> ---
>>   arch/x86/include/asm/kvm_host.h |    8 +++++
>>   arch/x86/include/asm/kvm_para.h |    4 +++
>>   arch/x86/kvm/Kconfig            |    1 +
>>   arch/x86/kvm/x86.c              |   56 ++++++++++++++++++++++++++++++++++++--
>>   4 files changed, 66 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index da6bbee..9ba354d 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -389,6 +389,14 @@ struct kvm_vcpu_arch {
>>   	unsigned int hw_tsc_khz;
>>   	unsigned int time_offset;
>>   	struct page *time_page;
>> +
>> +	struct {
>> +		u64 msr_val;
>> +		u64 last_steal;
>> +		struct gfn_to_hva_cache stime;
>> +		struct kvm_steal_time steal;
>> +	} st;
>> +
>>   	u64 last_guest_tsc;
>>   	u64 last_kernel_ns;
>>   	u64 last_tsc_nsec;
>> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
>> index 65f8bb9..c484ba8 100644
>> --- a/arch/x86/include/asm/kvm_para.h
>> +++ b/arch/x86/include/asm/kvm_para.h
>> @@ -45,6 +45,10 @@ struct kvm_steal_time {
>>   	__u32 pad[12];
>>   };
>>
>> +#define KVM_STEAL_ALIGNMENT_BITS 5
>> +#define KVM_STEAL_VALID_BITS ((-1ULL<<  (KVM_STEAL_ALIGNMENT_BITS + 1)))
>> +#define KVM_STEAL_RESERVED_MASK (((1<<  KVM_STEAL_ALIGNMENT_BITS) - 1 )<<  1)
>> +
>>   #define KVM_MAX_MMU_OP_BATCH           32
>>
>>   #define KVM_ASYNC_PF_ENABLED			(1<<  0)
>> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
>> index 50f6364..99c3f05 100644
>> --- a/arch/x86/kvm/Kconfig
>> +++ b/arch/x86/kvm/Kconfig
>> @@ -31,6 +31,7 @@ config KVM
>>   	select KVM_ASYNC_PF
>>   	select USER_RETURN_NOTIFIER
>>   	select KVM_MMIO
>> +	select TASK_DELAY_ACCT
>>   	---help---
>>   	  Support hosting fully virtualized guest machines using hardware
>>   	  virtualization extensions.  You will need a fairly recent
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 7167717..237bcdc 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -808,12 +808,12 @@ EXPORT_SYMBOL_GPL(kvm_get_dr);
>>    * kvm-specific. Those are put in the beginning of the list.
>>    */
>>
>> -#define KVM_SAVE_MSRS_BEGIN	8
>> +#define KVM_SAVE_MSRS_BEGIN	9
>>   static u32 msrs_to_save[] = {
>>   	MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK,
>>   	MSR_KVM_SYSTEM_TIME_NEW, MSR_KVM_WALL_CLOCK_NEW,
>>   	HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL,
>> -	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN,
>> +	HV_X64_MSR_APIC_ASSIST_PAGE, MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
>>   	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
>>   	MSR_STAR,
>>   #ifdef CONFIG_X86_64
>> @@ -1491,6 +1491,27 @@ static void kvmclock_reset(struct kvm_vcpu *vcpu)
>>   	}
>>   }
>>
>> +static void record_steal_time(struct kvm_vcpu *vcpu)
>> +{
>> +	u64 delta;
>> +
>> +	if (!(vcpu->arch.st.msr_val&  KVM_MSR_ENABLED))
>> +		return;
>> +
>> +	if (unlikely(kvm_read_guest_cached(vcpu->kvm,&vcpu->arch.st.stime,
>> +		&vcpu->arch.st.steal, sizeof(struct kvm_steal_time))))
>> +		return;
>
> The guest memory page is not pinned, sleeping via
> __copy_from_user/to_user is not allowed in vcpu_load context. Either pin
> it or use atomic acessors.

I do recognize the problem.
Avi, what's your take here?

>> +	case MSR_KVM_STEAL_TIME:
>> +		vcpu->arch.st.msr_val = data;
>> +
>> +		if (!(data&  KVM_MSR_ENABLED)) {
>> +			break;
>> +		}
>
> On failure below this point, msr_val should be cleared of KVM_MSR_ENABLED?
No, msr_val has to hold whatever the guest wrote into it.
We should probably use an independent variable here to indicate that we 
failed to activate it.

>
>> +
>> +		if (unlikely(!sched_info_on()))
>> +			break;
>> +
>> +		if (data&  KVM_STEAL_RESERVED_MASK)
>> +			return 1;
>> +
>> +		if (kvm_gfn_to_hva_cache_init(vcpu->kvm,&vcpu->arch.st.stime,
>> +							data&  KVM_STEAL_VALID_BITS))
>> +			return 1;
>> +
>> +		vcpu->arch.st.last_steal = current->sched_info.run_delay;
>> +
>> +		record_steal_time(vcpu);
>> +		break;
>> +

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Avi Kivity]

On 07/07/2011 08:07 PM, Glauber Costa wrote:
>>> +static void record_steal_time(struct kvm_vcpu *vcpu)
>>> +{
>>> +    u64 delta;
>>> +
>>> +    if (!(vcpu->arch.st.msr_val&  KVM_MSR_ENABLED))
>>> +        return;
>>> +
>>> +    if (unlikely(kvm_read_guest_cached(vcpu->kvm,&vcpu->arch.st.stime,
>>> + &vcpu->arch.st.steal, sizeof(struct kvm_steal_time))))
>>> +        return;
>>
>> The guest memory page is not pinned, sleeping via
>> __copy_from_user/to_user is not allowed in vcpu_load context. Either pin
>> it or use atomic acessors.
>
>
> I do recognize the problem.
> Avi, what's your take here?
>

The easiest solution is to set a KVM_REQ bit in atomic context, and move 
the sleepy code to vcpu_enter_guest().

>>> +    case MSR_KVM_STEAL_TIME:
>>> +        vcpu->arch.st.msr_val = data;
>>> +
>>> +        if (!(data&  KVM_MSR_ENABLED)) {
>>> +            break;
>>> +        }
>>
>> On failure below this point, msr_val should be cleared of 
>> KVM_MSR_ENABLED?
> No, msr_val has to hold whatever the guest wrote into it.
> We should probably use an independent variable here to indicate that 
> we failed to activate it.

If we fail, we return a #GP to the guest (and don't write any value into 
the msr).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Glauber Costa]

On 07/11/2011 09:58 AM, Avi Kivity wrote:
> On 07/07/2011 08:07 PM, Glauber Costa wrote:
>>>> +static void record_steal_time(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + u64 delta;
>>>> +
>>>> + if (!(vcpu->arch.st.msr_val& KVM_MSR_ENABLED))
>>>> + return;
>>>> +
>>>> + if (unlikely(kvm_read_guest_cached(vcpu->kvm,&vcpu->arch.st.stime,
>>>> + &vcpu->arch.st.steal, sizeof(struct kvm_steal_time))))
>>>> + return;
>>>
>>> The guest memory page is not pinned, sleeping via
>>> __copy_from_user/to_user is not allowed in vcpu_load context. Either pin
>>> it or use atomic acessors.
>>
>>
>> I do recognize the problem.
>> Avi, what's your take here?
>>
>
> The easiest solution is to set a KVM_REQ bit in atomic context, and move
> the sleepy code to vcpu_enter_guest().

Or I can move it all inside vcpu_run, or close enough to it. This will 
account more hypervisor time as steal time, but it seemed to be what 
some people wanted in the first place.

Given the simplification we would win - not needing a REQ set, it might 
be worth it.

>>>> + case MSR_KVM_STEAL_TIME:
>>>> + vcpu->arch.st.msr_val = data;
>>>> +
>>>> + if (!(data& KVM_MSR_ENABLED)) {
>>>> + break;
>>>> + }
>>>
>>> On failure below this point, msr_val should be cleared of
>>> KVM_MSR_ENABLED?
>> No, msr_val has to hold whatever the guest wrote into it.
>> We should probably use an independent variable here to indicate that
>> we failed to activate it.
>
> If we fail, we return a #GP to the guest (and don't write any value into
> the msr).
>

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Avi Kivity]

On 07/04/2011 06:32 PM, Glauber Costa wrote:
> To implement steal time, we need the hypervisor to pass the guest
> information about how much time was spent running other processes
> outside the VM, while the vcpu had meaningful work to do - halt
> time does not count.
>
> This information is acquired through the run_delay field of
> delayacct/schedstats infrastructure, that counts time spent in a
> runqueue but not running.
>
> Steal time is a per-cpu information, so the traditional MSR-based
> infrastructure is used. A new msr, KVM_MSR_STEAL_TIME, holds the
> memory area address containing information about steal time
>
> This patch contains the hypervisor part of the steal time infrasructure,
> and can be backported independently of the guest portion.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

I think Peter acked this, yes?  Please include his acks.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Avi Kivity]

On 07/11/2011 04:10 PM, Avi Kivity wrote:
>
> I think Peter acked this, yes?  Please include his acks.
>

Sorry, I meant this for the kernel/sched.c bits.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

[Glauber Costa]

On 07/11/2011 10:11 AM, Avi Kivity wrote:
> On 07/11/2011 04:10 PM, Avi Kivity wrote:
>>
>> I think Peter acked this, yes? Please include his acks.
>>
>
> Sorry, I meant this for the kernel/sched.c bits.
>
Yes he did, after I posted the patches.
I will include his acks in the respin, since they won't change the 
scheduler bits.

[PATCH v5 5/9] KVM-GST: Add a pv_ops stub for steal time

[Glauber Costa]

This patch adds a function pointer in one of the many paravirt_ops
structs, to allow guests to register a steal time function. Besides
a steal time function, we also declare two jump_labels. They will be
used to allow the steal time code to be easily bypassed when not
in use.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 arch/x86/include/asm/paravirt.h       |    9 +++++++++
 arch/x86/include/asm/paravirt_types.h |    1 +
 arch/x86/kernel/paravirt.c            |    9 +++++++++
 3 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index ebbc4d8..a7d2db9 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -230,6 +230,15 @@ static inline unsigned long long paravirt_sched_clock(void)
 	return PVOP_CALL0(unsigned long long, pv_time_ops.sched_clock);
 }
 
+struct jump_label_key;
+extern struct jump_label_key paravirt_steal_enabled;
+extern struct jump_label_key paravirt_steal_rq_enabled;
+
+static inline u64 paravirt_steal_clock(int cpu)
+{
+	return PVOP_CALL1(u64, pv_time_ops.steal_clock, cpu);
+}
+
 static inline unsigned long long paravirt_read_pmc(int counter)
 {
 	return PVOP_CALL1(u64, pv_cpu_ops.read_pmc, counter);
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8288509..2c76521 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -89,6 +89,7 @@ struct pv_lazy_ops {
 
 struct pv_time_ops {
 	unsigned long long (*sched_clock)(void);
+	unsigned long long (*steal_clock)(int cpu);
 	unsigned long (*get_tsc_khz)(void);
 };
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 869e1ae..613a793 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -202,6 +202,14 @@ static void native_flush_tlb_single(unsigned long addr)
 	__native_flush_tlb_single(addr);
 }
 
+struct jump_label_key paravirt_steal_enabled;
+struct jump_label_key paravirt_steal_rq_enabled;
+
+static u64 native_steal_clock(int cpu)
+{
+	return 0;
+}
+
 /* These are in entry.S */
 extern void native_iret(void);
 extern void native_irq_enable_sysexit(void);
@@ -307,6 +315,7 @@ struct pv_init_ops pv_init_ops = {
 
 struct pv_time_ops pv_time_ops = {
 	.sched_clock = native_sched_clock,
+	.steal_clock = native_steal_clock,
 };
 
 struct pv_irq_ops pv_irq_ops = {
-- 
1.7.3.4

Re: [PATCH v5 5/9] KVM-GST: Add a pv_ops stub for steal time

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> This patch adds a function pointer in one of the many paravirt_ops
> structs, to allow guests to register a steal time function. Besides
> a steal time function, we also declare two jump_labels. They will be
> used to allow the steal time code to be easily bypassed when not
> in use.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 5/9] KVM-GST: Add a pv_ops stub for steal time

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> This patch adds a function pointer in one of the many paravirt_ops
> structs, to allow guests to register a steal time function. Besides
> a steal time function, we also declare two jump_labels. They will be
> used to allow the steal time code to be easily bypassed when not
> in use.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

[PATCH v5 6/9] add jump labels for ia64 paravirt

[Glauber Costa]

Since in a later patch I intend to call jump labels inside
CONFIG_PARAVIRT, IA64 would fail to compile if they are not
provided. This patch provides those jump labels for the IA64
architecture.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Isaku Yamahata <yamahata@valinux.co.jp>
CC: Eddie Dong <eddie.dong@intel.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 arch/ia64/include/asm/paravirt.h |    4 ++++
 arch/ia64/kernel/paravirt.c      |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/include/asm/paravirt.h b/arch/ia64/include/asm/paravirt.h
index 2eb0a98..32551d3 100644
--- a/arch/ia64/include/asm/paravirt.h
+++ b/arch/ia64/include/asm/paravirt.h
@@ -281,6 +281,10 @@ paravirt_init_missing_ticks_accounting(int cpu)
 		pv_time_ops.init_missing_ticks_accounting(cpu);
 }
 
+struct jump_label_key;
+extern struct jump_label_key paravirt_steal_enabled;
+extern struct jump_label_key paravirt_steal_rq_enabled;
+
 static inline int
 paravirt_do_steal_accounting(unsigned long *new_itm)
 {
diff --git a/arch/ia64/kernel/paravirt.c b/arch/ia64/kernel/paravirt.c
index a21d7bb..1008682 100644
--- a/arch/ia64/kernel/paravirt.c
+++ b/arch/ia64/kernel/paravirt.c
@@ -634,6 +634,8 @@ struct pv_irq_ops pv_irq_ops = {
  * pv_time_ops
  * time operations
  */
+struct jump_label_key paravirt_steal_enabled;
+struct jump_label_key paravirt_steal_rq_enabled;
 
 static int
 ia64_native_do_steal_accounting(unsigned long *new_itm)
-- 
1.7.3.4

Re: [PATCH v5 6/9] add jump labels for ia64 paravirt

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> Since in a later patch I intend to call jump labels inside
> CONFIG_PARAVIRT, IA64 would fail to compile if they are not
> provided. This patch provides those jump labels for the IA64
> architecture.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Isaku Yamahata <yamahata@valinux.co.jp>
> CC: Eddie Dong <eddie.dong@intel.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 6/9] add jump labels for ia64 paravirt

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> Since in a later patch I intend to call jump labels inside
> CONFIG_PARAVIRT, IA64 would fail to compile if they are not
> provided. This patch provides those jump labels for the IA64
> architecture.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Isaku Yamahata<yamahata@valinux.co.jp>
> CC: Eddie Dong<eddie.dong@intel.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

Re: [PATCH v5 6/9] add jump labels for ia64 paravirt

[Avi Kivity]

On 07/04/2011 06:32 PM, Glauber Costa wrote:
> Since in a later patch I intend to call jump labels inside
> CONFIG_PARAVIRT, IA64 would fail to compile if they are not
> provided. This patch provides those jump labels for the IA64
> architecture.
>

Please get an ack for this from the ia64 maintainer.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH v5 6/9] add jump labels for ia64 paravirt

[Glauber Costa]

On 07/11/2011 10:09 AM, Avi Kivity wrote:
> On 07/04/2011 06:32 PM, Glauber Costa wrote:
>> Since in a later patch I intend to call jump labels inside
>> CONFIG_PARAVIRT, IA64 would fail to compile if they are not
>> provided. This patch provides those jump labels for the IA64
>> architecture.
>>
>
> Please get an ack for this from the ia64 maintainer.
>

The ones listed in paravirt.c are CC'd. I am CC'ing all the other
folks related to IA64 listed in MAINTAINERS now.

Just a heads up so the new folks CC'd can get up to speed:
I am proposing moving the steal time calculation to inside the core 
scheduler. This move will make it easier for us to make decisions based 
on steal time accounting on virtual machines. Ia64 KVM may or may not 
follow this route - nothing that already works should break with this 
change.

This patch is needed only to avoid breaking compilation, since it 
introduces two new variables that are expected be present when 
CONFIG_PARAVIRT, to paravirt.c.

Re: [PATCH v5 6/9] add jump labels for ia64 paravirt

[Isaku Yamahata]

On Mon, Jul 11, 2011 at 10:24:00AM -0300, Glauber Costa wrote:
> On 07/11/2011 10:09 AM, Avi Kivity wrote:
>> On 07/04/2011 06:32 PM, Glauber Costa wrote:
>>> Since in a later patch I intend to call jump labels inside
>>> CONFIG_PARAVIRT, IA64 would fail to compile if they are not
>>> provided. This patch provides those jump labels for the IA64
>>> architecture.
>>>
>>
>> Please get an ack for this from the ia64 maintainer.
>>
>
> The ones listed in paravirt.c are CC'd. I am CC'ing all the other
> folks related to IA64 listed in MAINTAINERS now.
>
> Just a heads up so the new folks CC'd can get up to speed:
> I am proposing moving the steal time calculation to inside the core  
> scheduler. This move will make it easier for us to make decisions based  
> on steal time accounting on virtual machines. Ia64 KVM may or may not  
> follow this route - nothing that already works should break with this  
> change.
>
> This patch is needed only to avoid breaking compilation, since it  
> introduces two new variables that are expected be present when  
> CONFIG_PARAVIRT, to paravirt.c.

Although the ia64 maintainer is Tony Luck,
I'm fine with the change as the ia64 paravirt author.

Acked-by: Isaku Yamahata <yamahata@valinux.co.jp>

-- 
yamahata

RE: [PATCH v5 6/9] add jump labels for ia64 paravirt

[Luck, Tony]

The paravirt/xen bits in ia64 are already in bit-rot.  I have this
build error in upstream building the arch/ia64/configs/xen_domu_defconfig
(even before applying your latest patch):

drivers/xen/grant-table.c: In function 'gnttab_map_refs':
drivers/xen/grant-table.c:491: error: implicit declaration of function 'm2p_add_override'
drivers/xen/grant-table.c: In function 'gnttab_unmap_refs':
drivers/xen/grant-table.c:514: error: implicit declaration of function 'm2p_remove_override'

Does anyone still care about virtualization support in ia64?

-Tony

[PATCH v5 7/9] KVM-GST: KVM Steal time accounting

[Glauber Costa]

This patch accounts steal time time in account_process_tick.
If one or more tick is considered stolen in the current
accounting cycle, user/system accounting is skipped. Idle is fine,
since the hypervisor does not report steal time if the guest
is halted.

Accounting steal time from the core scheduler give us the
advantage of direct acess to the runqueue data. In a later
opportunity, it can be used to tweak cpu power and make
the scheduler aware of the time it lost.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 kernel/sched.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 3f2e502..aa6c030 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -75,6 +75,7 @@
 #include <asm/tlb.h>
 #include <asm/irq_regs.h>
 #include <asm/mutex.h>
+#include <asm/paravirt.h>
 
 #include "sched_cpupri.h"
 #include "workqueue_sched.h"
@@ -528,6 +529,9 @@ struct rq {
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING
 	u64 prev_irq_time;
 #endif
+#ifdef CONFIG_PARAVIRT
+	u64 prev_steal_time;
+#endif
 
 	/* calc_load related fields */
 	unsigned long calc_load_update;
@@ -1953,6 +1957,18 @@ void account_system_vtime(struct task_struct *curr)
 }
 EXPORT_SYMBOL_GPL(account_system_vtime);
 
+#endif /* CONFIG_IRQ_TIME_ACCOUNTING */
+
+#ifdef CONFIG_PARAVIRT
+static inline u64 steal_ticks(u64 steal)
+{
+	if (unlikely(steal > NSEC_PER_SEC))
+		return div_u64(steal, TICK_NSEC);
+
+	return __iter_div_u64_rem(steal, TICK_NSEC, &steal);
+}
+#endif
+
 static void update_rq_clock_task(struct rq *rq, s64 delta)
 {
 	s64 irq_delta;
@@ -3845,6 +3861,25 @@ void account_idle_time(cputime_t cputime)
 		cpustat->idle = cputime64_add(cpustat->idle, cputime64);
 }
 
+static __always_inline bool steal_account_process_tick(void)
+{
+#ifdef CONFIG_PARAVIRT
+	if (static_branch(&paravirt_steal_enabled)) {
+		u64 steal, st = 0;
+
+		steal = paravirt_steal_clock(smp_processor_id());
+		steal -= this_rq()->prev_steal_time;
+
+		st = steal_ticks(steal);
+		this_rq()->prev_steal_time += st * TICK_NSEC;
+
+		account_steal_time(st);
+		return st;
+	}
+#endif
+	return false;
+}
+
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
 
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING
@@ -3876,6 +3911,9 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 	cputime64_t tmp = cputime_to_cputime64(cputime_one_jiffy);
 	struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat;
 
+	if (steal_account_process_tick())
+		return;
+
 	if (irqtime_account_hi_update()) {
 		cpustat->irq = cputime64_add(cpustat->irq, tmp);
 	} else if (irqtime_account_si_update()) {
@@ -3929,6 +3967,9 @@ void account_process_tick(struct task_struct *p, int user_tick)
 		return;
 	}
 
+	if (steal_account_process_tick())
+		return;
+
 	if (user_tick)
 		account_user_time(p, cputime_one_jiffy, one_jiffy_scaled);
 	else if ((p != rq->idle) || (irq_count() != HARDIRQ_OFFSET))
-- 
1.7.3.4

Re: [PATCH v5 7/9] KVM-GST: KVM Steal time accounting

[Peter Zijlstra]

On Mon, 2011-07-04 at 11:32 -0400, Glauber Costa wrote:
> This patch accounts steal time time in account_process_tick.
> If one or more tick is considered stolen in the current
> accounting cycle, user/system accounting is skipped. Idle is fine,
> since the hypervisor does not report steal time if the guest
> is halted.
> 
> Accounting steal time from the core scheduler give us the
> advantage of direct acess to the runqueue data. In a later
> opportunity, it can be used to tweak cpu power and make
> the scheduler aware of the time it lost.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Acked-by: Peter Zijlstra <peterz@infradead.org>

Venki, can you have a look at that irqtime_account_process_tick(), I
think adding the steal time up front like this is fine, because it
suffers from the same 'problem' as both irqtime thingies.

> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>
> ---
>  kernel/sched.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3f2e502..aa6c030 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -75,6 +75,7 @@
>  #include <asm/tlb.h>
>  #include <asm/irq_regs.h>
>  #include <asm/mutex.h>
> +#include <asm/paravirt.h>
>  
>  #include "sched_cpupri.h"
>  #include "workqueue_sched.h"
> @@ -528,6 +529,9 @@ struct rq {
>  #ifdef CONFIG_IRQ_TIME_ACCOUNTING
>  	u64 prev_irq_time;
>  #endif
> +#ifdef CONFIG_PARAVIRT
> +	u64 prev_steal_time;
> +#endif
>  
>  	/* calc_load related fields */
>  	unsigned long calc_load_update;
> @@ -1953,6 +1957,18 @@ void account_system_vtime(struct task_struct *curr)
>  }
>  EXPORT_SYMBOL_GPL(account_system_vtime);
>  
> +#endif /* CONFIG_IRQ_TIME_ACCOUNTING */
> +
> +#ifdef CONFIG_PARAVIRT
> +static inline u64 steal_ticks(u64 steal)
> +{
> +	if (unlikely(steal > NSEC_PER_SEC))
> +		return div_u64(steal, TICK_NSEC);
> +
> +	return __iter_div_u64_rem(steal, TICK_NSEC, &steal);
> +}
> +#endif
> +
>  static void update_rq_clock_task(struct rq *rq, s64 delta)
>  {
>  	s64 irq_delta;
> @@ -3845,6 +3861,25 @@ void account_idle_time(cputime_t cputime)
>  		cpustat->idle = cputime64_add(cpustat->idle, cputime64);
>  }
>  
> +static __always_inline bool steal_account_process_tick(void)
> +{
> +#ifdef CONFIG_PARAVIRT
> +	if (static_branch(&paravirt_steal_enabled)) {
> +		u64 steal, st = 0;
> +
> +		steal = paravirt_steal_clock(smp_processor_id());
> +		steal -= this_rq()->prev_steal_time;
> +
> +		st = steal_ticks(steal);
> +		this_rq()->prev_steal_time += st * TICK_NSEC;
> +
> +		account_steal_time(st);
> +		return st;
> +	}
> +#endif
> +	return false;
> +}
> +
>  #ifndef CONFIG_VIRT_CPU_ACCOUNTING
>  
>  #ifdef CONFIG_IRQ_TIME_ACCOUNTING
> @@ -3876,6 +3911,9 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
>  	cputime64_t tmp = cputime_to_cputime64(cputime_one_jiffy);
>  	struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat;
>  
> +	if (steal_account_process_tick())
> +		return;
> +
>  	if (irqtime_account_hi_update()) {
>  		cpustat->irq = cputime64_add(cpustat->irq, tmp);
>  	} else if (irqtime_account_si_update()) {
> @@ -3929,6 +3967,9 @@ void account_process_tick(struct task_struct *p, int user_tick)
>  		return;
>  	}
>  
> +	if (steal_account_process_tick())
> +		return;
> +
>  	if (user_tick)
>  		account_user_time(p, cputime_one_jiffy, one_jiffy_scaled);
>  	else if ((p != rq->idle) || (irq_count() != HARDIRQ_OFFSET))

Re: [PATCH v5 7/9] KVM-GST: KVM Steal time accounting

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> This patch accounts steal time time in account_process_tick.
> If one or more tick is considered stolen in the current
> accounting cycle, user/system accounting is skipped. Idle is fine,
> since the hypervisor does not report steal time if the guest
> is halted.
> 
> Accounting steal time from the core scheduler give us the
> advantage of direct acess to the runqueue data. In a later
> opportunity, it can be used to tweak cpu power and make
> the scheduler aware of the time it lost.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 7/9] KVM-GST: KVM Steal time accounting

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> This patch accounts steal time time in account_process_tick.
> If one or more tick is considered stolen in the current
> accounting cycle, user/system accounting is skipped. Idle is fine,
> since the hypervisor does not report steal time if the guest
> is halted.
>
> Accounting steal time from the core scheduler give us the
> advantage of direct acess to the runqueue data. In a later
> opportunity, it can be used to tweak cpu power and make
> the scheduler aware of the time it lost.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All rights reversed

[PATCH v5 8/9] KVM-GST: adjust scheduler cpu power

[Glauber Costa]

This patch makes update_rq_clock() aware of steal time.
The mechanism of operation is not different from irq_time,
and follows the same principles. This lives in a CONFIG
option itself, and can be compiled out independently of
the rest of steal time reporting. The effect of disabling it
is that the scheduler will still report steal time (that cannot be
disabled), but won't use this information for cpu power adjustments.

Everytime update_rq_clock_task() is invoked, we query information
about how much time was stolen since last call, and feed it into
sched_rt_avg_update().

Although steal time reporting in account_process_tick() keeps
track of the last time we read the steal clock, in prev_steal_time,
this patch do it independently using another field,
prev_steal_time_rq. This is because otherwise, information about time
accounted in update_process_tick() would never reach us in update_rq_clock().

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 arch/x86/Kconfig        |   12 ++++++++++++
 kernel/sched.c          |   47 +++++++++++++++++++++++++++++++++++++----------
 kernel/sched_features.h |    4 ++--
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index da34972..b26f312 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -512,6 +512,18 @@ menuconfig PARAVIRT_GUEST
 
 if PARAVIRT_GUEST
 
+config PARAVIRT_TIME_ACCOUNTING
+	bool "Paravirtual steal time accounting"
+	select PARAVIRT
+	default n
+	---help---
+	  Select this option to enable fine granularity task steal time 
+	  accounting. Time spent executing other tasks in parallel with
+	  the current vCPU is discounted from the vCPU power. To account for
+	  that, there can be a small performance impact.
+
+	  If in doubt, say N here.
+
 source "arch/x86/xen/Kconfig"
 
 config KVM_CLOCK
diff --git a/kernel/sched.c b/kernel/sched.c
index aa6c030..8d57196 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -532,6 +532,9 @@ struct rq {
 #ifdef CONFIG_PARAVIRT
 	u64 prev_steal_time;
 #endif
+#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
+	u64 prev_steal_time_rq;
+#endif
 
 	/* calc_load related fields */
 	unsigned long calc_load_update;
@@ -1971,8 +1974,14 @@ static inline u64 steal_ticks(u64 steal)
 
 static void update_rq_clock_task(struct rq *rq, s64 delta)
 {
-	s64 irq_delta;
-
+/*
+ * In theory, the compile should just see 0 here, and optimize out the call
+ * to sched_rt_avg_update. But I don't trust it...
+ */
+#if defined(CONFIG_IRQ_TIME_ACCOUNTING) || defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
+	s64 steal = 0, irq_delta = 0;
+#endif
+#ifdef CONFIG_IRQ_TIME_ACCOUNTING
 	irq_delta = irq_time_read(cpu_of(rq)) - rq->prev_irq_time;
 
 	/*
@@ -1995,12 +2004,35 @@ static void update_rq_clock_task(struct rq *rq, s64 delta)
 
 	rq->prev_irq_time += irq_delta;
 	delta -= irq_delta;
+#endif
+#ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
+	if (static_branch((&paravirt_steal_rq_enabled))) {
+		u64 st;
+
+		steal = paravirt_steal_clock(cpu_of(rq));
+		steal -= rq->prev_steal_time_rq;
+
+		if (unlikely(steal > delta))
+			steal = delta;
+
+		st = steal_ticks(steal);
+		steal = st * TICK_NSEC;
+
+		rq->prev_steal_time_rq += steal;
+
+		delta -= steal;
+	}
+#endif
+
 	rq->clock_task += delta;
 
-	if (irq_delta && sched_feat(NONIRQ_POWER))
-		sched_rt_avg_update(rq, irq_delta);
+#if defined(CONFIG_IRQ_TIME_ACCOUNTING) || defined(CONFIG_PARAVIRT_TIME_ACCOUNTING)
+	if ((irq_delta + steal) && sched_feat(NONTASK_POWER))
+		sched_rt_avg_update(rq, irq_delta + steal);
+#endif
 }
 
+#ifdef CONFIG_IRQ_TIME_ACCOUNTING
 static int irqtime_account_hi_update(void)
 {
 	struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat;
@@ -2035,12 +2067,7 @@ static int irqtime_account_si_update(void)
 
 #define sched_clock_irqtime	(0)
 
-static void update_rq_clock_task(struct rq *rq, s64 delta)
-{
-	rq->clock_task += delta;
-}
-
-#endif /* CONFIG_IRQ_TIME_ACCOUNTING */
+#endif
 
 #include "sched_idletask.c"
 #include "sched_fair.c"
diff --git a/kernel/sched_features.h b/kernel/sched_features.h
index be40f73..ca3b025 100644
--- a/kernel/sched_features.h
+++ b/kernel/sched_features.h
@@ -61,9 +61,9 @@ SCHED_FEAT(LB_BIAS, 1)
 SCHED_FEAT(OWNER_SPIN, 1)
 
 /*
- * Decrement CPU power based on irq activity
+ * Decrement CPU power based on time not spent running tasks
  */
-SCHED_FEAT(NONIRQ_POWER, 1)
+SCHED_FEAT(NONTASK_POWER, 1)
 
 /*
  * Queue remote wakeups on the target CPU and process them
-- 
1.7.3.4

Re: [PATCH v5 8/9] KVM-GST: adjust scheduler cpu power

[Peter Zijlstra]

On Mon, 2011-07-04 at 11:32 -0400, Glauber Costa wrote:
> This patch makes update_rq_clock() aware of steal time.
> The mechanism of operation is not different from irq_time,
> and follows the same principles. This lives in a CONFIG
> option itself, and can be compiled out independently of
> the rest of steal time reporting. The effect of disabling it
> is that the scheduler will still report steal time (that cannot be
> disabled), but won't use this information for cpu power adjustments.
> 
> Everytime update_rq_clock_task() is invoked, we query information
> about how much time was stolen since last call, and feed it into
> sched_rt_avg_update().
> 
> Although steal time reporting in account_process_tick() keeps
> track of the last time we read the steal clock, in prev_steal_time,
> this patch do it independently using another field,
> prev_steal_time_rq. This is because otherwise, information about time
> accounted in update_process_tick() would never reach us in
> update_rq_clock().
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Acked-by: Peter Zijlstra <peterz@infradead.org> 

Re: [PATCH v5 8/9] KVM-GST: adjust scheduler cpu power

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> This patch makes update_rq_clock() aware of steal time.
> The mechanism of operation is not different from irq_time,
> and follows the same principles. This lives in a CONFIG
> option itself, and can be compiled out independently of
> the rest of steal time reporting. The effect of disabling it
> is that the scheduler will still report steal time (that cannot be
> disabled), but won't use this information for cpu power adjustments.
> 
> Everytime update_rq_clock_task() is invoked, we query information
> about how much time was stolen since last call, and feed it into
> sched_rt_avg_update().
> 
> Although steal time reporting in account_process_tick() keeps
> track of the last time we read the steal clock, in prev_steal_time,
> this patch do it independently using another field,
> prev_steal_time_rq. This is because otherwise, information about time
> accounted in update_process_tick() would never reach us in update_rq_clock().
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 8/9] KVM-GST: adjust scheduler cpu power

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> This patch makes update_rq_clock() aware of steal time.
> The mechanism of operation is not different from irq_time,
> and follows the same principles. This lives in a CONFIG
> option itself, and can be compiled out independently of
> the rest of steal time reporting. The effect of disabling it
> is that the scheduler will still report steal time (that cannot be
> disabled), but won't use this information for cpu power adjustments.
>
> Everytime update_rq_clock_task() is invoked, we query information
> about how much time was stolen since last call, and feed it into
> sched_rt_avg_update().
>
> Although steal time reporting in account_process_tick() keeps
> track of the last time we read the steal clock, in prev_steal_time,
> this patch do it independently using another field,
> prev_steal_time_rq. This is because otherwise, information about time
> accounted in update_process_tick() would never reach us in update_rq_clock().
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>


-- 
All rights reversed

[PATCH v5 9/9] KVM-GST: KVM Steal time registration

[Glauber Costa]

This patch implements the kvm bits of the steal time infrastructure.
The most important part of it, is the steal time clock. It is an
continuous clock that shows the accumulated amount of steal time
since vcpu creation. It is supposed to survive cpu offlining/onlining.

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Rik van Riel <riel@redhat.com>
CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Avi Kivity <avi@redhat.com>
CC: Anthony Liguori <aliguori@us.ibm.com>
CC: Eric B Munson <emunson@mgebm.net>
---
 Documentation/kernel-parameters.txt |    4 ++
 arch/x86/include/asm/kvm_para.h     |    1 +
 arch/x86/kernel/kvm.c               |   73 +++++++++++++++++++++++++++++++++++
 arch/x86/kernel/kvmclock.c          |    2 +
 4 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fd248a31..a722574 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1737,6 +1737,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	no-kvmapf	[X86,KVM] Disable paravirtualized asynchronous page
 			fault handling.
 
+	no-steal-acc    [X86,KVM] Disable paravirtualized steal time accounting.
+			steal time is computed, but won't influence scheduler
+			behaviour
+
 	nolapic		[X86-32,APIC] Do not enable or use the local APIC.
 
 	nolapic_timer	[X86-32,APIC] Do not use the local APIC timer.
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index c484ba8..35d732d 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -94,6 +94,7 @@ struct kvm_vcpu_pv_apf_data {
 
 extern void kvmclock_init(void);
 extern int kvm_register_clock(char *txt);
+extern void kvm_disable_steal_time(void);
 
 
 /* This instruction is vmcall.  On non-VT architectures, it will generate a
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 33c07b0..58331c2 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -51,6 +51,15 @@ static int parse_no_kvmapf(char *arg)
 
 early_param("no-kvmapf", parse_no_kvmapf);
 
+static int steal_acc = 1;
+static int parse_no_stealacc(char *arg)
+{
+        steal_acc = 0;
+        return 0;
+}
+
+early_param("no-steal-acc", parse_no_stealacc);
+
 struct kvm_para_state {
 	u8 mmu_queue[MMU_QUEUE_SIZE];
 	int mmu_queue_len;
@@ -58,6 +67,8 @@ struct kvm_para_state {
 
 static DEFINE_PER_CPU(struct kvm_para_state, para_state);
 static DEFINE_PER_CPU(struct kvm_vcpu_pv_apf_data, apf_reason) __aligned(64);
+static DEFINE_PER_CPU(struct kvm_steal_time, steal_time) __aligned(64);
+static int has_steal_clock = 0;
 
 static struct kvm_para_state *kvm_para_state(void)
 {
@@ -441,6 +452,21 @@ static void __init paravirt_ops_setup(void)
 #endif
 }
 
+static void kvm_register_steal_time(void)
+{
+	int cpu = smp_processor_id();
+	struct kvm_steal_time *st = &per_cpu(steal_time, cpu);
+
+	if (!has_steal_clock)
+		return;
+
+	memset(st, 0, sizeof(*st));
+
+	wrmsrl(MSR_KVM_STEAL_TIME, (__pa(st) | KVM_MSR_ENABLED));
+	printk(KERN_INFO "kvm-stealtime: cpu %d, msr %lx\n",
+		cpu, __pa(st));
+}
+
 void __cpuinit kvm_guest_cpu_init(void)
 {
 	if (!kvm_para_available())
@@ -457,6 +483,9 @@ void __cpuinit kvm_guest_cpu_init(void)
 		printk(KERN_INFO"KVM setup async PF for cpu %d\n",
 		       smp_processor_id());
 	}
+
+	if (has_steal_clock)
+		kvm_register_steal_time();
 }
 
 static void kvm_pv_disable_apf(void *unused)
@@ -483,6 +512,31 @@ static struct notifier_block kvm_pv_reboot_nb = {
 	.notifier_call = kvm_pv_reboot_notify,
 };
 
+static u64 kvm_steal_clock(int cpu)
+{
+	u64 steal;
+	struct kvm_steal_time *src;
+	int version;
+
+	src = &per_cpu(steal_time, cpu);
+	do {
+		version = src->version;
+		rmb();
+		steal = src->steal;
+		rmb();
+	} while ((version & 1) || (version != src->version));
+
+	return steal;
+}
+
+void kvm_disable_steal_time(void)
+{
+	if (!has_steal_clock)
+		return;
+
+	wrmsr(MSR_KVM_STEAL_TIME, 0, 0);
+}
+
 #ifdef CONFIG_SMP
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
@@ -500,6 +554,7 @@ static void __cpuinit kvm_guest_cpu_online(void *dummy)
 
 static void kvm_guest_cpu_offline(void *dummy)
 {
+	kvm_disable_steal_time();
 	kvm_pv_disable_apf(NULL);
 	apf_task_wake_all();
 }
@@ -548,6 +603,11 @@ void __init kvm_guest_init(void)
 	if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
 		x86_init.irqs.trap_init = kvm_apf_trap_init;
 
+	if (kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
+		has_steal_clock = 1;
+		pv_time_ops.steal_clock = kvm_steal_clock;
+	}
+
 #ifdef CONFIG_SMP
 	smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
 	register_cpu_notifier(&kvm_cpu_notifier);
@@ -555,3 +615,16 @@ void __init kvm_guest_init(void)
 	kvm_guest_cpu_init();
 #endif
 }
+
+static __init int activate_jump_labels(void)
+{
+	if (has_steal_clock) {
+		jump_label_inc(&paravirt_steal_enabled);
+		if (steal_acc)
+			jump_label_inc(&paravirt_steal_rq_enabled);
+	}
+
+	return 0;
+}
+arch_initcall(activate_jump_labels);
+
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 6389a6b..c1a0188 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -160,6 +160,7 @@ static void __cpuinit kvm_setup_secondary_clock(void)
 static void kvm_crash_shutdown(struct pt_regs *regs)
 {
 	native_write_msr(msr_kvm_system_time, 0, 0);
+	kvm_disable_steal_time();
 	native_machine_crash_shutdown(regs);
 }
 #endif
@@ -167,6 +168,7 @@ static void kvm_crash_shutdown(struct pt_regs *regs)
 static void kvm_shutdown(void)
 {
 	native_write_msr(msr_kvm_system_time, 0, 0);
+	kvm_disable_steal_time();
 	native_machine_shutdown();
 }
 
-- 
1.7.3.4

Re: [PATCH v5 9/9] KVM-GST: KVM Steal time registration

[Eric B Munson]

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

On Mon, 04 Jul 2011, Glauber Costa wrote:

> This patch implements the kvm bits of the steal time infrastructure.
> The most important part of it, is the steal time clock. It is an
> continuous clock that shows the accumulated amount of steal time
> since vcpu creation. It is supposed to survive cpu offlining/onlining.
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Avi Kivity <avi@redhat.com>
> CC: Anthony Liguori <aliguori@us.ibm.com>
> CC: Eric B Munson <emunson@mgebm.net>

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

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

Re: [PATCH v5 9/9] KVM-GST: KVM Steal time registration

[Rik van Riel]

On 07/04/2011 11:32 AM, Glauber Costa wrote:
> This patch implements the kvm bits of the steal time infrastructure.
> The most important part of it, is the steal time clock. It is an
> continuous clock that shows the accumulated amount of steal time
> since vcpu creation. It is supposed to survive cpu offlining/onlining.
>
> Signed-off-by: Glauber Costa<glommer@redhat.com>
> CC: Rik van Riel<riel@redhat.com>
> CC: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
> CC: Peter Zijlstra<peterz@infradead.org>
> CC: Avi Kivity<avi@redhat.com>
> CC: Anthony Liguori<aliguori@us.ibm.com>
> CC: Eric B Munson<emunson@mgebm.net>

Acked-by: Rik van Riel <riel@redhat.com>


-- 
All rights reversed