[PATCH v11 0/5] xen/arm/arm64: CONFIG_PARAVIRT and stolen ticks accounting

[PATCH v11 0/5] xen/arm/arm64: CONFIG_PARAVIRT and stolen ticks accounting

[Stefano Stabellini]

Hi all,

I dusted off this series from Jan 2014. Patch #2 and #3 still need an ack.


This patch series introduces stolen ticks accounting for Xen on ARM and
ARM64.  Stolen ticks are clocksource ticks that have been "stolen" from
the cpu, typically because Linux is running in a virtual machine and the
vcpu has been descheduled.  To account for these ticks we introduce
CONFIG_PARAVIRT and pv_time_ops so that we can make use of:

kernel/sched/cputime.c:steal_account_process_tick


Changes in v11:
- add ifdef CONFIG_PARAVIRT to kernel/sched/cputime.c, because not all
  architectures have an asm/paravirt.h header file to include
- drop the removal of ifdef CONFIG_PARAVIRT from kernel/sched/core.c for
  the same reason


Stefano Stabellini (5):
      xen: move xen_setup_runstate_info and get_runstate_snapshot to drivers/xen/time.c
      missing include asm/paravirt.h in cputime.c
      arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops
      arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops
      xen/arm: account for stolen ticks

 arch/arm/Kconfig                  |   20 ++++++++
 arch/arm/include/asm/paravirt.h   |   20 ++++++++
 arch/arm/kernel/Makefile          |    1 +
 arch/arm/kernel/paravirt.c        |   25 ++++++++++
 arch/arm/xen/enlighten.c          |   21 +++++++++
 arch/arm64/Kconfig                |   20 ++++++++
 arch/arm64/include/asm/paravirt.h |   20 ++++++++
 arch/arm64/kernel/Makefile        |    1 +
 arch/arm64/kernel/paravirt.c      |   25 ++++++++++
 arch/x86/xen/time.c               |   76 +------------------------------
 drivers/xen/Makefile              |    2 +-
 drivers/xen/time.c                |   91 +++++++++++++++++++++++++++++++++++++
 include/xen/xen-ops.h             |    5 ++
 kernel/sched/cputime.c            |    3 ++
 14 files changed, 254 insertions(+), 76 deletions(-)
 create mode 100644 arch/arm/include/asm/paravirt.h
 create mode 100644 arch/arm/kernel/paravirt.c
 create mode 100644 arch/arm64/include/asm/paravirt.h
 create mode 100644 arch/arm64/kernel/paravirt.c
 create mode 100644 drivers/xen/time.c



Cheers,

Stefano

[PATCH v11 1/5] xen: move xen_setup_runstate_info and get_runstate_snapshot to drivers/xen/time.c

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: konrad.wilk at oracle.com

---

Changes in v10:
- rebase
---
 arch/x86/xen/time.c   |   76 +----------------------------------------
 drivers/xen/Makefile  |    2 +-
 drivers/xen/time.c    |   91 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/xen/xen-ops.h |    5 +++
 4 files changed, 98 insertions(+), 76 deletions(-)
 create mode 100644 drivers/xen/time.c

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index f1ba6a0..041d4cd 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -32,86 +32,12 @@
 #define TIMER_SLOP	100000
 #define NS_PER_TICK	(1000000000LL / HZ)
 
-/* runstate info updated by Xen */
-static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate);
-
 /* snapshots of runstate info */
 static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate_snapshot);
 
 /* unused ns of stolen time */
 static DEFINE_PER_CPU(u64, xen_residual_stolen);
 
-/* return an consistent snapshot of 64-bit time/counter value */
-static u64 get64(const u64 *p)
-{
-	u64 ret;
-
-	if (BITS_PER_LONG < 64) {
-		u32 *p32 = (u32 *)p;
-		u32 h, l;
-
-		/*
-		 * Read high then low, and then make sure high is
-		 * still the same; this will only loop if low wraps
-		 * and carries into high.
-		 * XXX some clean way to make this endian-proof?
-		 */
-		do {
-			h = p32[1];
-			barrier();
-			l = p32[0];
-			barrier();
-		} while (p32[1] != h);
-
-		ret = (((u64)h) << 32) | l;
-	} else
-		ret = *p;
-
-	return ret;
-}
-
-/*
- * Runstate accounting
- */
-static void get_runstate_snapshot(struct vcpu_runstate_info *res)
-{
-	u64 state_time;
-	struct vcpu_runstate_info *state;
-
-	BUG_ON(preemptible());
-
-	state = this_cpu_ptr(&xen_runstate);
-
-	/*
-	 * The runstate info is always updated by the hypervisor on
-	 * the current CPU, so there's no need to use anything
-	 * stronger than a compiler barrier when fetching it.
-	 */
-	do {
-		state_time = get64(&state->state_entry_time);
-		barrier();
-		*res = *state;
-		barrier();
-	} while (get64(&state->state_entry_time) != state_time);
-}
-
-/* return true when a vcpu could run but has no real cpu to run on */
-bool xen_vcpu_stolen(int vcpu)
-{
-	return per_cpu(xen_runstate, vcpu).state == RUNSTATE_runnable;
-}
-
-void xen_setup_runstate_info(int cpu)
-{
-	struct vcpu_register_runstate_memory_area area;
-
-	area.addr.v = &per_cpu(xen_runstate, cpu);
-
-	if (HYPERVISOR_vcpu_op(VCPUOP_register_runstate_memory_area,
-			       cpu, &area))
-		BUG();
-}
-
 static void do_stolen_accounting(void)
 {
 	struct vcpu_runstate_info state;
@@ -119,7 +45,7 @@ static void do_stolen_accounting(void)
 	s64 runnable, offline, stolen;
 	cputime_t ticks;
 
-	get_runstate_snapshot(&state);
+	xen_get_runstate_snapshot(&state);
 
 	WARN_ON(state.state != RUNSTATE_running);
 
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index aa8a7f7..9b7a35c 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,6 +1,6 @@
 obj-$(CONFIG_HOTPLUG_CPU)		+= cpu_hotplug.o
 obj-$(CONFIG_X86)			+= fallback.o
-obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o
+obj-y	+= grant-table.o features.o balloon.o manage.o preempt.o time.o
 obj-y	+= events/
 obj-y	+= xenbus/
 
diff --git a/drivers/xen/time.c b/drivers/xen/time.c
new file mode 100644
index 0000000..433fe24
--- /dev/null
+++ b/drivers/xen/time.c
@@ -0,0 +1,91 @@
+/*
+ * Xen stolen ticks accounting.
+ */
+#include <linux/kernel.h>
+#include <linux/kernel_stat.h>
+#include <linux/math64.h>
+#include <linux/gfp.h>
+
+#include <asm/xen/hypervisor.h>
+#include <asm/xen/hypercall.h>
+
+#include <xen/events.h>
+#include <xen/features.h>
+#include <xen/interface/xen.h>
+#include <xen/interface/vcpu.h>
+#include <xen/xen-ops.h>
+
+/* runstate info updated by Xen */
+static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate);
+
+/* return an consistent snapshot of 64-bit time/counter value */
+static u64 get64(const u64 *p)
+{
+	u64 ret;
+
+	if (BITS_PER_LONG < 64) {
+		u32 *p32 = (u32 *)p;
+		u32 h, l;
+
+		/*
+		 * Read high then low, and then make sure high is
+		 * still the same; this will only loop if low wraps
+		 * and carries into high.
+		 * XXX some clean way to make this endian-proof?
+		 */
+		do {
+			h = p32[1];
+			barrier();
+			l = p32[0];
+			barrier();
+		} while (p32[1] != h);
+
+		ret = (((u64)h) << 32) | l;
+	} else
+		ret = *p;
+
+	return ret;
+}
+
+/*
+ * Runstate accounting
+ */
+void xen_get_runstate_snapshot(struct vcpu_runstate_info *res)
+{
+	u64 state_time;
+	struct vcpu_runstate_info *state;
+
+	BUG_ON(preemptible());
+
+	state = this_cpu_ptr(&xen_runstate);
+
+	/*
+	 * The runstate info is always updated by the hypervisor on
+	 * the current CPU, so there's no need to use anything
+	 * stronger than a compiler barrier when fetching it.
+	 */
+	do {
+		state_time = get64(&state->state_entry_time);
+		barrier();
+		*res = *state;
+		barrier();
+	} while (get64(&state->state_entry_time) != state_time);
+}
+
+/* return true when a vcpu could run but has no real cpu to run on */
+bool xen_vcpu_stolen(int vcpu)
+{
+	return per_cpu(xen_runstate, vcpu).state == RUNSTATE_runnable;
+}
+
+void xen_setup_runstate_info(int cpu)
+{
+	struct vcpu_register_runstate_memory_area area;
+
+	area.addr.v = &per_cpu(xen_runstate, cpu);
+
+	if (HYPERVISOR_vcpu_op(VCPUOP_register_runstate_memory_area,
+			       cpu, &area))
+		BUG();
+}
+
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index e4e214a..86abe07 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -5,6 +5,7 @@
 #include <linux/notifier.h>
 #include <linux/efi.h>
 #include <asm/xen/interface.h>
+#include <xen/interface/vcpu.h>
 
 DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
 
@@ -18,6 +19,10 @@ void xen_arch_suspend(void);
 void xen_resume_notifier_register(struct notifier_block *nb);
 void xen_resume_notifier_unregister(struct notifier_block *nb);
 
+bool xen_vcpu_stolen(int vcpu);
+void xen_setup_runstate_info(int cpu);
+void xen_get_runstate_snapshot(struct vcpu_runstate_info *res);
+
 int xen_setup_shutdown_event(void);
 
 extern unsigned long *xen_contiguous_bitmap;
-- 
1.7.10.4

[PATCH v11 1/5] xen: move xen_setup_runstate_info and get_runstate_snapshot to drivers/xen/time.c

[Mark Rutland]

Hi,

> +static u64 get64(const u64 *p)
> +{
> +	u64 ret;
> +
> +	if (BITS_PER_LONG < 64) {
> +		u32 *p32 = (u32 *)p;
> +		u32 h, l;
> +
> +		/*
> +		 * Read high then low, and then make sure high is
> +		 * still the same; this will only loop if low wraps
> +		 * and carries into high.
> +		 * XXX some clean way to make this endian-proof?
> +		 */
> +		do {
> +			h = p32[1];
> +			barrier();
> +			l = p32[0];
> +			barrier();
> +		} while (p32[1] != h);

I realise this is simply a move of existing code, but it may be better
to instead have:

do {
	h = READ_ONCE(p32[1]);
	l = READ_ONCE(p32[0]);
} while (READ_ONCE(p32[1] != h);

Which ensures that each load is a single access (though it almost
certainly would be anyway), and prevents the compiler from having to
reload any other memory locations (which the current barrier() usage
forces).

> +
> +		ret = (((u64)h) << 32) | l;
> +	} else
> +		ret = *p;

Likewise, this would be better as READ_ONCE(*p), to force a single
access.

> +
> +	return ret;
> +}

> +	do {
> +		state_time = get64(&state->state_entry_time);
> +		barrier();
> +		*res = *state;
> +		barrier();

You can also have:

	*res = READ_ONCE(*state);

That will which will handle the barriers implicitly.

Thanks,
Mark.

> +	} while (get64(&state->state_entry_time) != state_time);
> +}

[PATCH v11 1/5] xen: move xen_setup_runstate_info and get_runstate_snapshot to drivers/xen/time.c

[Stefano Stabellini]

On Thu, 5 Nov 2015, Mark Rutland wrote:
> Hi,
> 
> > +static u64 get64(const u64 *p)
> > +{
> > +	u64 ret;
> > +
> > +	if (BITS_PER_LONG < 64) {
> > +		u32 *p32 = (u32 *)p;
> > +		u32 h, l;
> > +
> > +		/*
> > +		 * Read high then low, and then make sure high is
> > +		 * still the same; this will only loop if low wraps
> > +		 * and carries into high.
> > +		 * XXX some clean way to make this endian-proof?
> > +		 */
> > +		do {
> > +			h = p32[1];
> > +			barrier();
> > +			l = p32[0];
> > +			barrier();
> > +		} while (p32[1] != h);
> 
> I realise this is simply a move of existing code, but it may be better
> to instead have:
> 
> do {
> 	h = READ_ONCE(p32[1]);
> 	l = READ_ONCE(p32[0]);
> } while (READ_ONCE(p32[1] != h);
> 
> Which ensures that each load is a single access (though it almost
> certainly would be anyway), and prevents the compiler from having to
> reload any other memory locations (which the current barrier() usage
> forces).

I am happy to make these changes, however for code clarity and review
simplicity I'll keep them on a separate patch (I like code movement to
remain code movement). I can squash the two patches together when
committing, if necessary.


> > +
> > +		ret = (((u64)h) << 32) | l;
> > +	} else
> > +		ret = *p;
> 
> Likewise, this would be better as READ_ONCE(*p), to force a single
> access.
> 
> > +
> > +	return ret;
> > +}
> 
> > +	do {
> > +		state_time = get64(&state->state_entry_time);
> > +		barrier();
> > +		*res = *state;
> > +		barrier();
> 
> You can also have:
> 
> 	*res = READ_ONCE(*state);
> 
> That will which will handle the barriers implicitly.
> 
> Thanks,
> Mark.
> 
> > +	} while (get64(&state->state_entry_time) != state_time);
> > +}
> 

[PATCH v11 1/5] xen: move xen_setup_runstate_info and get_runstate_snapshot to drivers/xen/time.c

[Mark Rutland]

On Fri, Nov 06, 2015 at 11:11:40AM +0000, Stefano Stabellini wrote:
> On Thu, 5 Nov 2015, Mark Rutland wrote:
> > Hi,
> > 
> > > +static u64 get64(const u64 *p)
> > > +{
> > > +	u64 ret;
> > > +
> > > +	if (BITS_PER_LONG < 64) {
> > > +		u32 *p32 = (u32 *)p;
> > > +		u32 h, l;
> > > +
> > > +		/*
> > > +		 * Read high then low, and then make sure high is
> > > +		 * still the same; this will only loop if low wraps
> > > +		 * and carries into high.
> > > +		 * XXX some clean way to make this endian-proof?
> > > +		 */
> > > +		do {
> > > +			h = p32[1];
> > > +			barrier();
> > > +			l = p32[0];
> > > +			barrier();
> > > +		} while (p32[1] != h);
> > 
> > I realise this is simply a move of existing code, but it may be better
> > to instead have:
> > 
> > do {
> > 	h = READ_ONCE(p32[1]);
> > 	l = READ_ONCE(p32[0]);
> > } while (READ_ONCE(p32[1] != h);
> > 
> > Which ensures that each load is a single access (though it almost
> > certainly would be anyway), and prevents the compiler from having to
> > reload any other memory locations (which the current barrier() usage
> > forces).
> 
> I am happy to make these changes, however for code clarity and review
> simplicity I'll keep them on a separate patch (I like code movement to
> remain code movement). I can squash the two patches together when
> committing, if necessary.

Sure, I also prefer to separate code movement from code rework, so that
makes sense to me.

Thanks,
Mark.

[PATCH v11 2/5] missing include asm/paravirt.h in cputime.c

[Stefano Stabellini]

Add include asm/paravirt.h to cputime.c, as steal_account_process_tick
calls paravirt_steal_clock, which is defined in asm/paravirt.h.

The ifdef CONFIG_PARAVIRT is necessary because not all archs have an
asm/paravirt.h to include.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: mingo at redhat.com
CC: peterz at infradead.org

---

Changes in v11:
- add ifdef CONFIG_PARAVIRT to cputime.c, because not all architectures
  have an asm/paravirt.h header file to include
- drop the removal of ifdef CONFIG_PARAVIRT from kernel/sched/core.c for
  the same reason
---
 kernel/sched/cputime.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 8cbc3db..c7a27c4 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -5,6 +5,9 @@
 #include <linux/static_key.h>
 #include <linux/context_tracking.h>
 #include "sched.h"
+#ifdef CONFIG_PARAVIRT
+#include <asm/paravirt.h>
+#endif
 
 
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING
-- 
1.7.10.4

[PATCH v11 2/5] missing include asm/paravirt.h in cputime.c

[Peter Zijlstra]

How can this be missing? Things compile fine now, right? So please
better explain why we do this change.

[PATCH v11 2/5] missing include asm/paravirt.h in cputime.c

[Stefano Stabellini]

On Thu, 5 Nov 2015, Peter Zijlstra wrote:
> How can this be missing? Things compile fine now, right?

Fair enough.


> So please better explain why we do this change.

asm/paravirt.h is included by one of the other headers included in
kernel/sched/cputime.c on x86, but not on other architecures. On arm and
arm64, where I am about to introduce asm/paravirt.h and stolen time
support, without #include <asm/paravirt.h> in cputime.c I would get:

kernel/sched/cputime.c: In function ?steal_account_process_tick?:
kernel/sched/cputime.c:260:24: error: ?paravirt_steal_enabled? undeclared (first use in this function)
  if (static_key_false(&paravirt_steal_enabled)) {

A bit of digging on x86 (using gcc -E on cputime.c) tells me that
asm/paravirt.h is coming from the following include chain:

#include <kernel/sched/sched.h>
#include <linux/spinlock.h>
#include <linux/preempt.h>
#include <asm/preempt.h>
#include <linux/thread_info.h>
#include <asm/thread_info.h>
#include <asm/processor.h>
#include <asm/msr.h>
#include <asm/paravirt.h>

[PATCH v11 2/5] missing include asm/paravirt.h in cputime.c

[Peter Zijlstra]

On Thu, Nov 05, 2015 at 05:30:01PM +0000, Stefano Stabellini wrote:
> On Thu, 5 Nov 2015, Peter Zijlstra wrote:
> > How can this be missing? Things compile fine now, right?
> 
> Fair enough.
> 
> 
> > So please better explain why we do this change.
> 
> asm/paravirt.h is included by one of the other headers included in
> kernel/sched/cputime.c on x86, but not on other architecures. On arm and
> arm64, where I am about to introduce asm/paravirt.h and stolen time
> support, without #include <asm/paravirt.h> in cputime.c I would get:
> 
> kernel/sched/cputime.c: In function ?steal_account_process_tick?:
> kernel/sched/cputime.c:260:24: error: ?paravirt_steal_enabled? undeclared (first use in this function)
>   if (static_key_false(&paravirt_steal_enabled)) {
> 
> A bit of digging on x86 (using gcc -E on cputime.c) tells me that
> asm/paravirt.h is coming from the following include chain:
> 
> #include <kernel/sched/sched.h>
> #include <linux/spinlock.h>
> #include <linux/preempt.h>
> #include <asm/preempt.h>
> #include <linux/thread_info.h>
> #include <asm/thread_info.h>
> #include <asm/processor.h>
> #include <asm/msr.h>
> #include <asm/paravirt.h>


Fair enough; a slightly shorter version of that for a changelog will do
nicely.

Thanks!

[PATCH v11 2/5] missing include asm/paravirt.h in cputime.c

[Stefano Stabellini]

On Mon, 9 Nov 2015, Peter Zijlstra wrote:
> On Thu, Nov 05, 2015 at 05:30:01PM +0000, Stefano Stabellini wrote:
> > On Thu, 5 Nov 2015, Peter Zijlstra wrote:
> > > How can this be missing? Things compile fine now, right?
> > 
> > Fair enough.
> > 
> > 
> > > So please better explain why we do this change.
> > 
> > asm/paravirt.h is included by one of the other headers included in
> > kernel/sched/cputime.c on x86, but not on other architecures. On arm and
> > arm64, where I am about to introduce asm/paravirt.h and stolen time
> > support, without #include <asm/paravirt.h> in cputime.c I would get:
> > 
> > kernel/sched/cputime.c: In function ?steal_account_process_tick?:
> > kernel/sched/cputime.c:260:24: error: ?paravirt_steal_enabled? undeclared (first use in this function)
> >   if (static_key_false(&paravirt_steal_enabled)) {
> > 
> > A bit of digging on x86 (using gcc -E on cputime.c) tells me that
> > asm/paravirt.h is coming from the following include chain:
> > 
> > #include <kernel/sched/sched.h>
> > #include <linux/spinlock.h>
> > #include <linux/preempt.h>
> > #include <asm/preempt.h>
> > #include <linux/thread_info.h>
> > #include <asm/thread_info.h>
> > #include <asm/processor.h>
> > #include <asm/msr.h>
> > #include <asm/paravirt.h>
> 
> 
> Fair enough; a slightly shorter version of that for a changelog will do
> nicely.

Sure. Can I add your Acked-by to it?

[PATCH v11 2/5] missing include asm/paravirt.h in cputime.c

[Peter Zijlstra]

On Tue, Nov 10, 2015 at 11:27:33AM +0000, Stefano Stabellini wrote:
> On Mon, 9 Nov 2015, Peter Zijlstra wrote:
> > On Thu, Nov 05, 2015 at 05:30:01PM +0000, Stefano Stabellini wrote:
> > > On Thu, 5 Nov 2015, Peter Zijlstra wrote:
> > > > How can this be missing? Things compile fine now, right?
> > > 
> > > Fair enough.
> > > 
> > > 
> > > > So please better explain why we do this change.
> > > 
> > > asm/paravirt.h is included by one of the other headers included in
> > > kernel/sched/cputime.c on x86, but not on other architecures. On arm and
> > > arm64, where I am about to introduce asm/paravirt.h and stolen time
> > > support, without #include <asm/paravirt.h> in cputime.c I would get:
> > > 
> > > kernel/sched/cputime.c: In function ?steal_account_process_tick?:
> > > kernel/sched/cputime.c:260:24: error: ?paravirt_steal_enabled? undeclared (first use in this function)
> > >   if (static_key_false(&paravirt_steal_enabled)) {
> > > 
> > > A bit of digging on x86 (using gcc -E on cputime.c) tells me that
> > > asm/paravirt.h is coming from the following include chain:
> > > 
> > > #include <kernel/sched/sched.h>
> > > #include <linux/spinlock.h>
> > > #include <linux/preempt.h>
> > > #include <asm/preempt.h>
> > > #include <linux/thread_info.h>
> > > #include <asm/thread_info.h>
> > > #include <asm/processor.h>
> > > #include <asm/msr.h>
> > > #include <asm/paravirt.h>
> > 
> > 
> > Fair enough; a slightly shorter version of that for a changelog will do
> > nicely.
> 
> Sure. Can I add your Acked-by to it?

Yep, no objection if the changelog is updated as per the above.

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

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.

The only paravirt interface supported is pv_time_ops.steal_clock, so no
runtime pvops patching needed.

This allows us to make use of steal_account_process_tick for stolen
ticks accounting.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Christopher Covington <cov@codeaurora.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: linux at arm.linux.org.uk
CC: will.deacon at arm.com
CC: nico at linaro.org
CC: marc.zyngier at arm.com
CC: cov at codeaurora.org
CC: arnd at arndb.de
CC: olof at lixom.net

---

Changes in v10:
- replace "---help---" with "help"

Changes in v7:
- ifdef CONFIG_PARAVIRT the content of paravirt.h.

Changes in v3:
- improve commit description and Kconfig help text;
- no need to initialize pv_time_ops;
- add PARAVIRT_TIME_ACCOUNTING.
---
 arch/arm/Kconfig                |   20 ++++++++++++++++++++
 arch/arm/include/asm/paravirt.h |   20 ++++++++++++++++++++
 arch/arm/kernel/Makefile        |    1 +
 arch/arm/kernel/paravirt.c      |   25 +++++++++++++++++++++++++
 4 files changed, 66 insertions(+)
 create mode 100644 arch/arm/include/asm/paravirt.h
 create mode 100644 arch/arm/kernel/paravirt.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f1ed110..60be104 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1823,6 +1823,25 @@ config SWIOTLB
 config IOMMU_HELPER
 	def_bool SWIOTLB
 
+config PARAVIRT
+	bool "Enable paravirtualization code"
+	help
+	  This changes the kernel so it can modify itself when it is run
+	  under a hypervisor, potentially improving performance significantly
+	  over full virtualization.
+
+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.
+
 config XEN_DOM0
 	def_bool y
 	depends on XEN
@@ -1836,6 +1855,7 @@ config XEN
 	select ARCH_DMA_ADDR_T_64BIT
 	select ARM_PSCI
 	select SWIOTLB_XEN
+	select PARAVIRT
 	help
 	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
 
diff --git a/arch/arm/include/asm/paravirt.h b/arch/arm/include/asm/paravirt.h
new file mode 100644
index 0000000..8435ff59
--- /dev/null
+++ b/arch/arm/include/asm/paravirt.h
@@ -0,0 +1,20 @@
+#ifndef _ASM_ARM_PARAVIRT_H
+#define _ASM_ARM_PARAVIRT_H
+
+#ifdef CONFIG_PARAVIRT
+struct static_key;
+extern struct static_key paravirt_steal_enabled;
+extern struct static_key paravirt_steal_rq_enabled;
+
+struct pv_time_ops {
+	unsigned long long (*steal_clock)(int cpu);
+};
+extern struct pv_time_ops pv_time_ops;
+
+static inline u64 paravirt_steal_clock(int cpu)
+{
+	return pv_time_ops.steal_clock(cpu);
+}
+#endif
+
+#endif
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index af9e59b..3e6e937 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_VDSO)		+= vdso.o
 ifneq ($(CONFIG_ARCH_EBSA110),y)
   obj-y		+= io.o
 endif
+obj-$(CONFIG_PARAVIRT)	+= paravirt.o
 
 head-y			:= head$(MMUEXT).o
 obj-$(CONFIG_DEBUG_LL)	+= debug.o
diff --git a/arch/arm/kernel/paravirt.c b/arch/arm/kernel/paravirt.c
new file mode 100644
index 0000000..53f371e
--- /dev/null
+++ b/arch/arm/kernel/paravirt.c
@@ -0,0 +1,25 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Copyright (C) 2013 Citrix Systems
+ *
+ * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ */
+
+#include <linux/export.h>
+#include <linux/jump_label.h>
+#include <linux/types.h>
+#include <asm/paravirt.h>
+
+struct static_key paravirt_steal_enabled;
+struct static_key paravirt_steal_rq_enabled;
+
+struct pv_time_ops pv_time_ops;
+EXPORT_SYMBOL_GPL(pv_time_ops);
-- 
1.7.10.4

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.
> 
> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> runtime pvops patching needed.
> 
> This allows us to make use of steal_account_process_tick for stolen
> ticks accounting.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Acked-by: Christopher Covington <cov@codeaurora.org>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> CC: linux at arm.linux.org.uk
> CC: will.deacon at arm.com
> CC: nico at linaro.org
> CC: marc.zyngier at arm.com
> CC: cov at codeaurora.org
> CC: arnd at arndb.de
> CC: olof at lixom.net


Russell,
are you OK with this patch?




> 
> Changes in v10:
> - replace "---help---" with "help"
> 
> Changes in v7:
> - ifdef CONFIG_PARAVIRT the content of paravirt.h.
> 
> Changes in v3:
> - improve commit description and Kconfig help text;
> - no need to initialize pv_time_ops;
> - add PARAVIRT_TIME_ACCOUNTING.
> ---
>  arch/arm/Kconfig                |   20 ++++++++++++++++++++
>  arch/arm/include/asm/paravirt.h |   20 ++++++++++++++++++++
>  arch/arm/kernel/Makefile        |    1 +
>  arch/arm/kernel/paravirt.c      |   25 +++++++++++++++++++++++++
>  4 files changed, 66 insertions(+)
>  create mode 100644 arch/arm/include/asm/paravirt.h
>  create mode 100644 arch/arm/kernel/paravirt.c
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index f1ed110..60be104 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1823,6 +1823,25 @@ config SWIOTLB
>  config IOMMU_HELPER
>  	def_bool SWIOTLB
>  
> +config PARAVIRT
> +	bool "Enable paravirtualization code"
> +	help
> +	  This changes the kernel so it can modify itself when it is run
> +	  under a hypervisor, potentially improving performance significantly
> +	  over full virtualization.
> +
> +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.
> +
>  config XEN_DOM0
>  	def_bool y
>  	depends on XEN
> @@ -1836,6 +1855,7 @@ config XEN
>  	select ARCH_DMA_ADDR_T_64BIT
>  	select ARM_PSCI
>  	select SWIOTLB_XEN
> +	select PARAVIRT
>  	help
>  	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
>  
> diff --git a/arch/arm/include/asm/paravirt.h b/arch/arm/include/asm/paravirt.h
> new file mode 100644
> index 0000000..8435ff59
> --- /dev/null
> +++ b/arch/arm/include/asm/paravirt.h
> @@ -0,0 +1,20 @@
> +#ifndef _ASM_ARM_PARAVIRT_H
> +#define _ASM_ARM_PARAVIRT_H
> +
> +#ifdef CONFIG_PARAVIRT
> +struct static_key;
> +extern struct static_key paravirt_steal_enabled;
> +extern struct static_key paravirt_steal_rq_enabled;
> +
> +struct pv_time_ops {
> +	unsigned long long (*steal_clock)(int cpu);
> +};
> +extern struct pv_time_ops pv_time_ops;
> +
> +static inline u64 paravirt_steal_clock(int cpu)
> +{
> +	return pv_time_ops.steal_clock(cpu);
> +}
> +#endif
> +
> +#endif
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index af9e59b..3e6e937 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -81,6 +81,7 @@ obj-$(CONFIG_VDSO)		+= vdso.o
>  ifneq ($(CONFIG_ARCH_EBSA110),y)
>    obj-y		+= io.o
>  endif
> +obj-$(CONFIG_PARAVIRT)	+= paravirt.o
>  
>  head-y			:= head$(MMUEXT).o
>  obj-$(CONFIG_DEBUG_LL)	+= debug.o
> diff --git a/arch/arm/kernel/paravirt.c b/arch/arm/kernel/paravirt.c
> new file mode 100644
> index 0000000..53f371e
> --- /dev/null
> +++ b/arch/arm/kernel/paravirt.c
> @@ -0,0 +1,25 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Copyright (C) 2013 Citrix Systems
> + *
> + * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> + */
> +
> +#include <linux/export.h>
> +#include <linux/jump_label.h>
> +#include <linux/types.h>
> +#include <asm/paravirt.h>
> +
> +struct static_key paravirt_steal_enabled;
> +struct static_key paravirt_steal_rq_enabled;
> +
> +struct pv_time_ops pv_time_ops;
> +EXPORT_SYMBOL_GPL(pv_time_ops);
> -- 
> 1.7.10.4
> 

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

On Tue, 10 Nov 2015, Stefano Stabellini wrote:
> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> > Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.
> > 
> > The only paravirt interface supported is pv_time_ops.steal_clock, so no
> > runtime pvops patching needed.
> > 
> > This allows us to make use of steal_account_process_tick for stolen
> > ticks accounting.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Acked-by: Christopher Covington <cov@codeaurora.org>
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > CC: linux at arm.linux.org.uk
> > CC: will.deacon at arm.com
> > CC: nico at linaro.org
> > CC: marc.zyngier at arm.com
> > CC: cov at codeaurora.org
> > CC: arnd at arndb.de
> > CC: olof at lixom.net
> 
> 
> Russell,
> are you OK with this patch?

Russell,

I am going to drop this patch and add a small #ifdef to
arch/arm/xen/enlighten.c to be able to use this functionality on arm64.

If you change your mind let me know.



> 
> 
> > 
> > Changes in v10:
> > - replace "---help---" with "help"
> > 
> > Changes in v7:
> > - ifdef CONFIG_PARAVIRT the content of paravirt.h.
> > 
> > Changes in v3:
> > - improve commit description and Kconfig help text;
> > - no need to initialize pv_time_ops;
> > - add PARAVIRT_TIME_ACCOUNTING.
> > ---
> >  arch/arm/Kconfig                |   20 ++++++++++++++++++++
> >  arch/arm/include/asm/paravirt.h |   20 ++++++++++++++++++++
> >  arch/arm/kernel/Makefile        |    1 +
> >  arch/arm/kernel/paravirt.c      |   25 +++++++++++++++++++++++++
> >  4 files changed, 66 insertions(+)
> >  create mode 100644 arch/arm/include/asm/paravirt.h
> >  create mode 100644 arch/arm/kernel/paravirt.c
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index f1ed110..60be104 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1823,6 +1823,25 @@ config SWIOTLB
> >  config IOMMU_HELPER
> >  	def_bool SWIOTLB
> >  
> > +config PARAVIRT
> > +	bool "Enable paravirtualization code"
> > +	help
> > +	  This changes the kernel so it can modify itself when it is run
> > +	  under a hypervisor, potentially improving performance significantly
> > +	  over full virtualization.
> > +
> > +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.
> > +
> >  config XEN_DOM0
> >  	def_bool y
> >  	depends on XEN
> > @@ -1836,6 +1855,7 @@ config XEN
> >  	select ARCH_DMA_ADDR_T_64BIT
> >  	select ARM_PSCI
> >  	select SWIOTLB_XEN
> > +	select PARAVIRT
> >  	help
> >  	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM.
> >  
> > diff --git a/arch/arm/include/asm/paravirt.h b/arch/arm/include/asm/paravirt.h
> > new file mode 100644
> > index 0000000..8435ff59
> > --- /dev/null
> > +++ b/arch/arm/include/asm/paravirt.h
> > @@ -0,0 +1,20 @@
> > +#ifndef _ASM_ARM_PARAVIRT_H
> > +#define _ASM_ARM_PARAVIRT_H
> > +
> > +#ifdef CONFIG_PARAVIRT
> > +struct static_key;
> > +extern struct static_key paravirt_steal_enabled;
> > +extern struct static_key paravirt_steal_rq_enabled;
> > +
> > +struct pv_time_ops {
> > +	unsigned long long (*steal_clock)(int cpu);
> > +};
> > +extern struct pv_time_ops pv_time_ops;
> > +
> > +static inline u64 paravirt_steal_clock(int cpu)
> > +{
> > +	return pv_time_ops.steal_clock(cpu);
> > +}
> > +#endif
> > +
> > +#endif
> > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> > index af9e59b..3e6e937 100644
> > --- a/arch/arm/kernel/Makefile
> > +++ b/arch/arm/kernel/Makefile
> > @@ -81,6 +81,7 @@ obj-$(CONFIG_VDSO)		+= vdso.o
> >  ifneq ($(CONFIG_ARCH_EBSA110),y)
> >    obj-y		+= io.o
> >  endif
> > +obj-$(CONFIG_PARAVIRT)	+= paravirt.o
> >  
> >  head-y			:= head$(MMUEXT).o
> >  obj-$(CONFIG_DEBUG_LL)	+= debug.o
> > diff --git a/arch/arm/kernel/paravirt.c b/arch/arm/kernel/paravirt.c
> > new file mode 100644
> > index 0000000..53f371e
> > --- /dev/null
> > +++ b/arch/arm/kernel/paravirt.c
> > @@ -0,0 +1,25 @@
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * Copyright (C) 2013 Citrix Systems
> > + *
> > + * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > + */
> > +
> > +#include <linux/export.h>
> > +#include <linux/jump_label.h>
> > +#include <linux/types.h>
> > +#include <asm/paravirt.h>
> > +
> > +struct static_key paravirt_steal_enabled;
> > +struct static_key paravirt_steal_rq_enabled;
> > +
> > +struct pv_time_ops pv_time_ops;
> > +EXPORT_SYMBOL_GPL(pv_time_ops);
> > -- 
> > 1.7.10.4
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Christopher Covington]

Hi Stefano,

On 11/20/2015 09:31 AM, Stefano Stabellini wrote:
> On Tue, 10 Nov 2015, Stefano Stabellini wrote:
>> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
>>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.
>>>
>>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
>>> runtime pvops patching needed.
>>>
>>> This allows us to make use of steal_account_process_tick for stolen
>>> ticks accounting.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Acked-by: Christopher Covington <cov@codeaurora.org>
>>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>> CC: linux at arm.linux.org.uk
>>> CC: will.deacon at arm.com
>>> CC: nico at linaro.org
>>> CC: marc.zyngier at arm.com
>>> CC: cov at codeaurora.org
>>> CC: arnd at arndb.de
>>> CC: olof at lixom.net
>>
>>
>> Russell,
>> are you OK with this patch?
> 
> Russell,
> 
> I am going to drop this patch and add a small #ifdef to
> arch/arm/xen/enlighten.c to be able to use this functionality on arm64.
> 
> If you change your mind let me know.

It appears to me as though he's not copied on this message.

Regards,
Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

On Fri, 20 Nov 2015, Christopher Covington wrote:
> Hi Stefano,
> 
> On 11/20/2015 09:31 AM, Stefano Stabellini wrote:
> > On Tue, 10 Nov 2015, Stefano Stabellini wrote:
> >> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> >>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.
> >>>
> >>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> >>> runtime pvops patching needed.
> >>>
> >>> This allows us to make use of steal_account_process_tick for stolen
> >>> ticks accounting.
> >>>
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>> Acked-by: Christopher Covington <cov@codeaurora.org>
> >>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >>> CC: linux at arm.linux.org.uk
> >>> CC: will.deacon at arm.com
> >>> CC: nico at linaro.org
> >>> CC: marc.zyngier at arm.com
> >>> CC: cov at codeaurora.org
> >>> CC: arnd at arndb.de
> >>> CC: olof at lixom.net
> >>
> >>
> >> Russell,
> >> are you OK with this patch?
> > 
> > Russell,
> > 
> > I am going to drop this patch and add a small #ifdef to
> > arch/arm/xen/enlighten.c to be able to use this functionality on arm64.
> > 
> > If you change your mind let me know.
> 
> It appears to me as though he's not copied on this message.

He was. He is now in To:.

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Russell King - ARM Linux]

On Fri, Nov 20, 2015 at 02:40:31PM +0000, Stefano Stabellini wrote:
> On Fri, 20 Nov 2015, Christopher Covington wrote:
> > Hi Stefano,
> > 
> > On 11/20/2015 09:31 AM, Stefano Stabellini wrote:
> > > On Tue, 10 Nov 2015, Stefano Stabellini wrote:
> > >> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> > >>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.
> > >>>
> > >>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> > >>> runtime pvops patching needed.
> > >>>
> > >>> This allows us to make use of steal_account_process_tick for stolen
> > >>> ticks accounting.
> > >>>
> > >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >>> Acked-by: Christopher Covington <cov@codeaurora.org>
> > >>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > >>> CC: linux at arm.linux.org.uk
> > >>> CC: will.deacon at arm.com
> > >>> CC: nico at linaro.org
> > >>> CC: marc.zyngier at arm.com
> > >>> CC: cov at codeaurora.org
> > >>> CC: arnd at arndb.de
> > >>> CC: olof at lixom.net
> > >>
> > >>
> > >> Russell,
> > >> are you OK with this patch?
> > > 
> > > Russell,
> > > 
> > > I am going to drop this patch and add a small #ifdef to
> > > arch/arm/xen/enlighten.c to be able to use this functionality on arm64.
> > > 
> > > If you change your mind let me know.
> > 
> > It appears to me as though he's not copied on this message.
> 
> He was. He is now in To:.

I think the patch is fine.

Sorry, but I no longer read every email that passes by due to the amount
of email I now receive, and due to the nature of modern email clients with
their stupid ideas about how to formulate the To: and Cc: headers for
replies[*], I attach no significance to being mentioned in either the To:
or Cc: headers.

Overall, what this means is it's now difficult to attact my attention to
any particular thread.  Sorry about that, I have no solution to this
problem.


* - modern mailers have started to preserve the To: and Cc: headers from
the message being replied to, which means that if I'm mentioned in the
To: header initially, my address stays in the To: header despite the
discussion not being directed _at_ me.  Hence, deciding what to reply to
based on where my address appears in the headers is meaningless with
modern mail clients.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v11 3/5] arm: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

On Fri, 20 Nov 2015, Russell King - ARM Linux wrote:
> On Fri, Nov 20, 2015 at 02:40:31PM +0000, Stefano Stabellini wrote:
> > On Fri, 20 Nov 2015, Christopher Covington wrote:
> > > Hi Stefano,
> > > 
> > > On 11/20/2015 09:31 AM, Stefano Stabellini wrote:
> > > > On Tue, 10 Nov 2015, Stefano Stabellini wrote:
> > > >> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> > > >>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM.
> > > >>>
> > > >>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> > > >>> runtime pvops patching needed.
> > > >>>
> > > >>> This allows us to make use of steal_account_process_tick for stolen
> > > >>> ticks accounting.
> > > >>>
> > > >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >>> Acked-by: Christopher Covington <cov@codeaurora.org>
> > > >>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > >>> CC: linux at arm.linux.org.uk
> > > >>> CC: will.deacon at arm.com
> > > >>> CC: nico at linaro.org
> > > >>> CC: marc.zyngier at arm.com
> > > >>> CC: cov at codeaurora.org
> > > >>> CC: arnd at arndb.de
> > > >>> CC: olof at lixom.net
> > > >>
> > > >>
> > > >> Russell,
> > > >> are you OK with this patch?
> > > > 
> > > > Russell,
> > > > 
> > > > I am going to drop this patch and add a small #ifdef to
> > > > arch/arm/xen/enlighten.c to be able to use this functionality on arm64.
> > > > 
> > > > If you change your mind let me know.
> > > 
> > > It appears to me as though he's not copied on this message.
> > 
> > He was. He is now in To:.
> 
> I think the patch is fine.

Thanks


> Sorry, but I no longer read every email that passes by due to the amount
> of email I now receive, and due to the nature of modern email clients with
> their stupid ideas about how to formulate the To: and Cc: headers for
> replies[*], I attach no significance to being mentioned in either the To:
> or Cc: headers.
> 
> Overall, what this means is it's now difficult to attact my attention to
> any particular thread.  Sorry about that, I have no solution to this
> problem.

FWIW as somebody that receives pretty large amounts of emails myself
with my name in CC or To, I understand and I don't have a solution
either :-(


> * - modern mailers have started to preserve the To: and Cc: headers from
> the message being replied to, which means that if I'm mentioned in the
> To: header initially, my address stays in the To: header despite the
> discussion not being directed _at_ me.  Hence, deciding what to reply to
> based on where my address appears in the headers is meaningless with
> modern mail clients.
> 
> -- 
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
> 

[PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM64.
Necessary duplication of paravirt.h and paravirt.c with ARM.

The only paravirt interface supported is pv_time_ops.steal_clock, so no
runtime pvops patching needed.

This allows us to make use of steal_account_process_tick for stolen
ticks accounting.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
CC: will.deacon at arm.com
CC: nico at linaro.org
CC: marc.zyngier at arm.com
CC: cov at codeaurora.org
CC: arnd at arndb.de
CC: olof at lixom.net
CC: Catalin.Marinas at arm.com

---

Changes in v10:
- replace "---help---" with "help"

Changes in v7:
- ifdef CONFIG_PARAVIRT the content of paravirt.h.
---
 arch/arm64/Kconfig                |   20 ++++++++++++++++++++
 arch/arm64/include/asm/paravirt.h |   20 ++++++++++++++++++++
 arch/arm64/kernel/Makefile        |    1 +
 arch/arm64/kernel/paravirt.c      |   25 +++++++++++++++++++++++++
 4 files changed, 66 insertions(+)
 create mode 100644 arch/arm64/include/asm/paravirt.h
 create mode 100644 arch/arm64/kernel/paravirt.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7b10647..659e286 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -533,6 +533,25 @@ config SECCOMP
 	  and the task is only allowed to execute a few safe syscalls
 	  defined by each seccomp mode.
 
+config PARAVIRT
+	bool "Enable paravirtualization code"
+	help
+	  This changes the kernel so it can modify itself when it is run
+	  under a hypervisor, potentially improving performance significantly
+	  over full virtualization.
+
+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.
+
 config XEN_DOM0
 	def_bool y
 	depends on XEN
@@ -541,6 +560,7 @@ config XEN
 	bool "Xen guest support on ARM64"
 	depends on ARM64 && OF
 	select SWIOTLB_XEN
+	select PARAVIRT
 	help
 	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM64.
 
diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h
new file mode 100644
index 0000000..fd5f428
--- /dev/null
+++ b/arch/arm64/include/asm/paravirt.h
@@ -0,0 +1,20 @@
+#ifndef _ASM_ARM64_PARAVIRT_H
+#define _ASM_ARM64_PARAVIRT_H
+
+#ifdef CONFIG_PARAVIRT
+struct static_key;
+extern struct static_key paravirt_steal_enabled;
+extern struct static_key paravirt_steal_rq_enabled;
+
+struct pv_time_ops {
+	unsigned long long (*steal_clock)(int cpu);
+};
+extern struct pv_time_ops pv_time_ops;
+
+static inline u64 paravirt_steal_clock(int cpu)
+{
+	return pv_time_ops.steal_clock(cpu);
+}
+#endif
+
+#endif
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 474691f..ca9fbe1 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
+arm64-obj-$(CONFIG_PARAVIRT)		+= paravirt.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
new file mode 100644
index 0000000..53f371e
--- /dev/null
+++ b/arch/arm64/kernel/paravirt.c
@@ -0,0 +1,25 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Copyright (C) 2013 Citrix Systems
+ *
+ * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ */
+
+#include <linux/export.h>
+#include <linux/jump_label.h>
+#include <linux/types.h>
+#include <asm/paravirt.h>
+
+struct static_key paravirt_steal_enabled;
+struct static_key paravirt_steal_rq_enabled;
+
+struct pv_time_ops pv_time_ops;
+EXPORT_SYMBOL_GPL(pv_time_ops);
-- 
1.7.10.4

[PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM64.
> Necessary duplication of paravirt.h and paravirt.c with ARM.
> 
> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> runtime pvops patching needed.
> 
> This allows us to make use of steal_account_process_tick for stolen
> ticks accounting.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Ping?

Catalin, Will,
are you happy with this change?


> CC: will.deacon at arm.com
> CC: nico at linaro.org
> CC: marc.zyngier at arm.com
> CC: cov at codeaurora.org
> CC: arnd at arndb.de
> CC: olof at lixom.net
> CC: Catalin.Marinas at arm.com
> 
> ---
> 
> Changes in v10:
> - replace "---help---" with "help"
> 
> Changes in v7:
> - ifdef CONFIG_PARAVIRT the content of paravirt.h.
> ---
>  arch/arm64/Kconfig                |   20 ++++++++++++++++++++
>  arch/arm64/include/asm/paravirt.h |   20 ++++++++++++++++++++
>  arch/arm64/kernel/Makefile        |    1 +
>  arch/arm64/kernel/paravirt.c      |   25 +++++++++++++++++++++++++
>  4 files changed, 66 insertions(+)
>  create mode 100644 arch/arm64/include/asm/paravirt.h
>  create mode 100644 arch/arm64/kernel/paravirt.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 7b10647..659e286 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -533,6 +533,25 @@ config SECCOMP
>  	  and the task is only allowed to execute a few safe syscalls
>  	  defined by each seccomp mode.
>  
> +config PARAVIRT
> +	bool "Enable paravirtualization code"
> +	help
> +	  This changes the kernel so it can modify itself when it is run
> +	  under a hypervisor, potentially improving performance significantly
> +	  over full virtualization.
> +
> +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.
> +
>  config XEN_DOM0
>  	def_bool y
>  	depends on XEN
> @@ -541,6 +560,7 @@ config XEN
>  	bool "Xen guest support on ARM64"
>  	depends on ARM64 && OF
>  	select SWIOTLB_XEN
> +	select PARAVIRT
>  	help
>  	  Say Y if you want to run Linux in a Virtual Machine on Xen on ARM64.
>  
> diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h
> new file mode 100644
> index 0000000..fd5f428
> --- /dev/null
> +++ b/arch/arm64/include/asm/paravirt.h
> @@ -0,0 +1,20 @@
> +#ifndef _ASM_ARM64_PARAVIRT_H
> +#define _ASM_ARM64_PARAVIRT_H
> +
> +#ifdef CONFIG_PARAVIRT
> +struct static_key;
> +extern struct static_key paravirt_steal_enabled;
> +extern struct static_key paravirt_steal_rq_enabled;
> +
> +struct pv_time_ops {
> +	unsigned long long (*steal_clock)(int cpu);
> +};
> +extern struct pv_time_ops pv_time_ops;
> +
> +static inline u64 paravirt_steal_clock(int cpu)
> +{
> +	return pv_time_ops.steal_clock(cpu);
> +}
> +#endif
> +
> +#endif
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 474691f..ca9fbe1 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
>  arm64-obj-$(CONFIG_PCI)			+= pci.o
>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>  arm64-obj-$(CONFIG_ACPI)		+= acpi.o
> +arm64-obj-$(CONFIG_PARAVIRT)		+= paravirt.o
>  
>  obj-y					+= $(arm64-obj-y) vdso/
>  obj-m					+= $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c
> new file mode 100644
> index 0000000..53f371e
> --- /dev/null
> +++ b/arch/arm64/kernel/paravirt.c
> @@ -0,0 +1,25 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Copyright (C) 2013 Citrix Systems
> + *
> + * Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> + */
> +
> +#include <linux/export.h>
> +#include <linux/jump_label.h>
> +#include <linux/types.h>
> +#include <asm/paravirt.h>
> +
> +struct static_key paravirt_steal_enabled;
> +struct static_key paravirt_steal_rq_enabled;
> +
> +struct pv_time_ops pv_time_ops;
> +EXPORT_SYMBOL_GPL(pv_time_ops);
> -- 
> 1.7.10.4
> 

[PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Will Deacon]

On Tue, Nov 10, 2015 at 02:11:38PM +0000, Stefano Stabellini wrote:
> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> > Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM64.
> > Necessary duplication of paravirt.h and paravirt.c with ARM.
> > 
> > The only paravirt interface supported is pv_time_ops.steal_clock, so no
> > runtime pvops patching needed.
> > 
> > This allows us to make use of steal_account_process_tick for stolen
> > ticks accounting.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> Ping?
> 
> Catalin, Will,
> are you happy with this change?

I'm happy if Marc's happy. Marc?

Will

[PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Marc Zyngier]

On 17/11/15 17:29, Will Deacon wrote:
> On Tue, Nov 10, 2015 at 02:11:38PM +0000, Stefano Stabellini wrote:
>> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
>>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM64.
>>> Necessary duplication of paravirt.h and paravirt.c with ARM.
>>>
>>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
>>> runtime pvops patching needed.
>>>
>>> This allows us to make use of steal_account_process_tick for stolen
>>> ticks accounting.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> Ping?
>>
>> Catalin, Will,
>> are you happy with this change?
> 
> I'm happy if Marc's happy. Marc?

My Ack is already on the tin! ;-)

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Will Deacon]

On Tue, Nov 17, 2015 at 05:34:36PM +0000, Marc Zyngier wrote:
> On 17/11/15 17:29, Will Deacon wrote:
> > On Tue, Nov 10, 2015 at 02:11:38PM +0000, Stefano Stabellini wrote:
> >> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> >>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM64.
> >>> Necessary duplication of paravirt.h and paravirt.c with ARM.
> >>>
> >>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> >>> runtime pvops patching needed.
> >>>
> >>> This allows us to make use of steal_account_process_tick for stolen
> >>> ticks accounting.
> >>>
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> >>
> >> Ping?
> >>
> >> Catalin, Will,
> >> are you happy with this change?
> > 
> > I'm happy if Marc's happy. Marc?
> 
> My Ack is already on the tin! ;-)

Ah yes, I only saw the cc line. In which case, I assume Stefano will
take this via the xen tree.

Will

[PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

[Stefano Stabellini]

On Tue, 17 Nov 2015, Will Deacon wrote:
> On Tue, Nov 17, 2015 at 05:34:36PM +0000, Marc Zyngier wrote:
> > On 17/11/15 17:29, Will Deacon wrote:
> > > On Tue, Nov 10, 2015 at 02:11:38PM +0000, Stefano Stabellini wrote:
> > >> On Thu, 5 Nov 2015, Stefano Stabellini wrote:
> > >>> Introduce CONFIG_PARAVIRT and PARAVIRT_TIME_ACCOUNTING on ARM64.
> > >>> Necessary duplication of paravirt.h and paravirt.c with ARM.
> > >>>
> > >>> The only paravirt interface supported is pv_time_ops.steal_clock, so no
> > >>> runtime pvops patching needed.
> > >>>
> > >>> This allows us to make use of steal_account_process_tick for stolen
> > >>> ticks accounting.
> > >>>
> > >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > >>
> > >> Ping?
> > >>
> > >> Catalin, Will,
> > >> are you happy with this change?
> > > 
> > > I'm happy if Marc's happy. Marc?
> > 
> > My Ack is already on the tin! ;-)
> 
> Ah yes, I only saw the cc line. In which case, I assume Stefano will
> take this via the xen tree.

Unfortunately I am still missing Russell's ack on the arm patch (3/5).

Would you be OK if I dropped stolen time support for arm, only keeping
the arm64 part? The price to pay is two small ifdefs in arch/arm/xen/enlighten.c:

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 15621b1..87e2fec 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -16,7 +16,9 @@
 #include <asm/xen/hypercall.h>
 #include <asm/arch_timer.h>
 #include <asm/system_misc.h>
+#ifdef ARM64
 #include <asm/paravirt.h>
+#endif
 #include <linux/jump_label.h>
 #include <linux/interrupt.h>
 #include <linux/irqreturn.h>
@@ -289,9 +291,10 @@ static int __init xen_guest_init(void)
 
 	register_cpu_notifier(&xen_cpu_notifier);
 
+#ifdef ARM64
 	pv_time_ops.steal_clock = xen_stolen_accounting;
 	static_key_slow_inc(&paravirt_steal_enabled);
-
+#endif
 	return 0;
 }
 early_initcall(xen_guest_init);

[PATCH v11 5/5] xen/arm: account for stolen ticks

[Stefano Stabellini]

Register the runstate_memory_area with the hypervisor.
Use pv_time_ops.steal_clock to account for stolen ticks.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v4:
- don't use paravirt_steal_rq_enabled: we do not support retrieving
stolen ticks for vcpus other than one we are running on.

Changes in v3:
- use BUG_ON and smp_processor_id.
---
 arch/arm/xen/enlighten.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index fc7ea52..15621b1 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -14,7 +14,10 @@
 #include <xen/xen-ops.h>
 #include <asm/xen/hypervisor.h>
 #include <asm/xen/hypercall.h>
+#include <asm/arch_timer.h>
 #include <asm/system_misc.h>
+#include <asm/paravirt.h>
+#include <linux/jump_label.h>
 #include <linux/interrupt.h>
 #include <linux/irqreturn.h>
 #include <linux/module.h>
@@ -79,6 +82,19 @@ int xen_unmap_domain_gfn_range(struct vm_area_struct *vma,
 }
 EXPORT_SYMBOL_GPL(xen_unmap_domain_gfn_range);
 
+static unsigned long long xen_stolen_accounting(int cpu)
+{
+	struct vcpu_runstate_info state;
+
+	BUG_ON(cpu != smp_processor_id());
+
+	xen_get_runstate_snapshot(&state);
+
+	WARN_ON(state.state != RUNSTATE_running);
+
+	return state.time[RUNSTATE_runnable] + state.time[RUNSTATE_offline];
+}
+
 static void xen_percpu_init(void)
 {
 	struct vcpu_register_vcpu_info info;
@@ -104,6 +120,8 @@ static void xen_percpu_init(void)
 	BUG_ON(err);
 	per_cpu(xen_vcpu, cpu) = vcpup;
 
+	xen_setup_runstate_info(cpu);
+
 after_register_vcpu_info:
 	enable_percpu_irq(xen_events_irq, 0);
 	put_cpu();
@@ -271,6 +289,9 @@ static int __init xen_guest_init(void)
 
 	register_cpu_notifier(&xen_cpu_notifier);
 
+	pv_time_ops.steal_clock = xen_stolen_accounting;
+	static_key_slow_inc(&paravirt_steal_enabled);
+
 	return 0;
 }
 early_initcall(xen_guest_init);
-- 
1.7.10.4

[PATCH v11 5/5] xen/arm: account for stolen ticks

[Mark Rutland]

>  static void xen_percpu_init(void)
>  {
>  	struct vcpu_register_vcpu_info info;
> @@ -104,6 +120,8 @@ static void xen_percpu_init(void)
>  	BUG_ON(err);
>  	per_cpu(xen_vcpu, cpu) = vcpup;
>  
> +	xen_setup_runstate_info(cpu);

Does the runstate memory area get unregsitered when a kernel tears
things down, or is kexec somehow inhibited for xen guests?

i couldn't spot either happening, but I may have missed it.

Mark.

> +
>  after_register_vcpu_info:
>  	enable_percpu_irq(xen_events_irq, 0);
>  	put_cpu();
> @@ -271,6 +289,9 @@ static int __init xen_guest_init(void)
>  
>  	register_cpu_notifier(&xen_cpu_notifier);
>  
> +	pv_time_ops.steal_clock = xen_stolen_accounting;
> +	static_key_slow_inc(&paravirt_steal_enabled);
> +
>  	return 0;
>  }
>  early_initcall(xen_guest_init);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v11 5/5] xen/arm: account for stolen ticks

[Stefano Stabellini]

On Thu, 5 Nov 2015, Mark Rutland wrote:
> >  static void xen_percpu_init(void)
> >  {
> >  	struct vcpu_register_vcpu_info info;
> > @@ -104,6 +120,8 @@ static void xen_percpu_init(void)
> >  	BUG_ON(err);
> >  	per_cpu(xen_vcpu, cpu) = vcpup;
> >  
> > +	xen_setup_runstate_info(cpu);
> 
> Does the runstate memory area get unregsitered when a kernel tears
> things down, or is kexec somehow inhibited for xen guests?
> 
> i couldn't spot either happening, but I may have missed it.

I don't think that the runstate memory area needs to be unregistered for
kexec, but I am not very knowledgeble on kexec and Xen, CC'ing Vitaly
and David.


> > +
> >  after_register_vcpu_info:
> >  	enable_percpu_irq(xen_events_irq, 0);
> >  	put_cpu();
> > @@ -271,6 +289,9 @@ static int __init xen_guest_init(void)
> >  
> >  	register_cpu_notifier(&xen_cpu_notifier);
> >  
> > +	pv_time_ops.steal_clock = xen_stolen_accounting;
> > +	static_key_slow_inc(&paravirt_steal_enabled);
> > +
> >  	return 0;
> >  }
> >  early_initcall(xen_guest_init);
> > -- 
> > 1.7.10.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 

[PATCH v11 5/5] xen/arm: account for stolen ticks

[David Vrabel]

On 06/11/15 11:39, Stefano Stabellini wrote:
> On Thu, 5 Nov 2015, Mark Rutland wrote:
>>>  static void xen_percpu_init(void)
>>>  {
>>>  	struct vcpu_register_vcpu_info info;
>>> @@ -104,6 +120,8 @@ static void xen_percpu_init(void)
>>>  	BUG_ON(err);
>>>  	per_cpu(xen_vcpu, cpu) = vcpup;
>>>  
>>> +	xen_setup_runstate_info(cpu);
>>
>> Does the runstate memory area get unregsitered when a kernel tears
>> things down, or is kexec somehow inhibited for xen guests?
>>
>> i couldn't spot either happening, but I may have missed it.
> 
> I don't think that the runstate memory area needs to be unregistered for
> kexec, but I am not very knowledgeble on kexec and Xen, CC'ing Vitaly
> and David.

There's a whole pile of other state needing to be reset for kexec (event
channels and grant tables for example).  The guest needs to soft reset
itself (available in Xen 4.6) before kexec'ing another kernel.

This soft reset would also including cleaning up this shared memory region.

David

[PATCH v11 5/5] xen/arm: account for stolen ticks

[Mark Rutland]

On Fri, Nov 06, 2015 at 11:41:49AM +0000, David Vrabel wrote:
> On 06/11/15 11:39, Stefano Stabellini wrote:
> > On Thu, 5 Nov 2015, Mark Rutland wrote:
> >>>  static void xen_percpu_init(void)
> >>>  {
> >>>  	struct vcpu_register_vcpu_info info;
> >>> @@ -104,6 +120,8 @@ static void xen_percpu_init(void)
> >>>  	BUG_ON(err);
> >>>  	per_cpu(xen_vcpu, cpu) = vcpup;
> >>>  
> >>> +	xen_setup_runstate_info(cpu);
> >>
> >> Does the runstate memory area get unregsitered when a kernel tears
> >> things down, or is kexec somehow inhibited for xen guests?
> >>
> >> i couldn't spot either happening, but I may have missed it.
> > 
> > I don't think that the runstate memory area needs to be unregistered for
> > kexec, but I am not very knowledgeble on kexec and Xen, CC'ing Vitaly
> > and David.
> 
> There's a whole pile of other state needing to be reset for kexec (event
> channels and grant tables for example).  The guest needs to soft reset
> itself (available in Xen 4.6) before kexec'ing another kernel.
> 
> This soft reset would also including cleaning up this shared memory region.

Ok. So we don't currently have the code kernel-side, but it looks like
it would be relatively simple to add (having just spotted [1]), and
everything should be ready on the Xen side.`

Thanks,
Mark.

[1] https://lkml.org/lkml/2015/9/25/152

[PATCH v11 5/5] xen/arm: account for stolen ticks

[Vitaly Kuznetsov]

Mark Rutland <mark.rutland@arm.com> writes:

> On Fri, Nov 06, 2015 at 11:41:49AM +0000, David Vrabel wrote:
>> On 06/11/15 11:39, Stefano Stabellini wrote:
>> > On Thu, 5 Nov 2015, Mark Rutland wrote:
>> >>>  static void xen_percpu_init(void)
>> >>>  {
>> >>>  	struct vcpu_register_vcpu_info info;
>> >>> @@ -104,6 +120,8 @@ static void xen_percpu_init(void)
>> >>>  	BUG_ON(err);
>> >>>  	per_cpu(xen_vcpu, cpu) = vcpup;
>> >>>  
>> >>> +	xen_setup_runstate_info(cpu);
>> >>
>> >> Does the runstate memory area get unregsitered when a kernel tears
>> >> things down, or is kexec somehow inhibited for xen guests?
>> >>
>> >> i couldn't spot either happening, but I may have missed it.
>> > 
>> > I don't think that the runstate memory area needs to be unregistered for
>> > kexec, but I am not very knowledgeble on kexec and Xen, CC'ing Vitaly
>> > and David.
>> 
>> There's a whole pile of other state needing to be reset for kexec (event
>> channels and grant tables for example).  The guest needs to soft reset
>> itself (available in Xen 4.6) before kexec'ing another kernel.
>> 
>> This soft reset would also including cleaning up this shared memory region.
>
> Ok. So we don't currently have the code kernel-side, but it looks like
> it would be relatively simple to add (having just spotted [1])

already merged in 4.3 and several stable trees.

> , and everything should be ready on the Xen side.`
>

Yes, but for x86 only. arch_domain_soft_reset() is -ENOSYS for ARM
now. It should be relatively easy to implement, one should unmap
shared_info page and do some GIC cleanup (if it's needed at all). I'd be
happy to help if someone's interested but unfortunately I don't have ARM
hardware to test at this moment...

> Thanks,
> Mark.
>
> [1] https://lkml.org/lkml/2015/9/25/152

-- 
  Vitaly

[PATCH v11 5/5] xen/arm: account for stolen ticks

[Vitaly Kuznetsov]

David Vrabel <david.vrabel@citrix.com> writes:

> On 06/11/15 11:39, Stefano Stabellini wrote:
>> On Thu, 5 Nov 2015, Mark Rutland wrote:
>>>>  static void xen_percpu_init(void)
>>>>  {
>>>>  	struct vcpu_register_vcpu_info info;
>>>> @@ -104,6 +120,8 @@ static void xen_percpu_init(void)
>>>>  	BUG_ON(err);
>>>>  	per_cpu(xen_vcpu, cpu) = vcpup;
>>>>  
>>>> +	xen_setup_runstate_info(cpu);
>>>
>>> Does the runstate memory area get unregsitered when a kernel tears
>>> things down, or is kexec somehow inhibited for xen guests?
>>>
>>> i couldn't spot either happening, but I may have missed it.
>> 
>> I don't think that the runstate memory area needs to be unregistered for
>> kexec, but I am not very knowledgeble on kexec and Xen, CC'ing Vitaly
>> and David.
>
> There's a whole pile of other state needing to be reset for kexec (event
> channels and grant tables for example).  The guest needs to soft reset
> itself (available in Xen 4.6) before kexec'ing another kernel.

Unfortunately, it's 4.7. Soft reset patch series was merged after 4.6
freeze so it is only available in current master git branch.

>
> This soft reset would also including cleaning up this shared memory region.
>
> David

-- 
  Vitaly