2015-11-10 14:16:01

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

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 <[email protected]>
> Acked-by: Marc Zyngier <[email protected]>

Ping?

Catalin, Will,
are you happy with this change?


> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
>
> ---
>
> 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 <[email protected]>
> + */
> +
> +#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
>


2015-11-17 17:29:22

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

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 <[email protected]>
> > Acked-by: Marc Zyngier <[email protected]>
>
> Ping?
>
> Catalin, Will,
> are you happy with this change?

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

Will

2015-11-17 17:34:42

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

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 <[email protected]>
>>> Acked-by: Marc Zyngier <[email protected]>
>>
>> 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...

2015-11-17 17:36:04

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

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 <[email protected]>
> >>> Acked-by: Marc Zyngier <[email protected]>
> >>
> >> 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

2015-11-20 12:20:05

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH v11 4/5] arm64: introduce CONFIG_PARAVIRT, PARAVIRT_TIME_ACCOUNTING and pv_time_ops

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 <[email protected]>
> > >>> Acked-by: Marc Zyngier <[email protected]>
> > >>
> > >> 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);