From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DDB39CA9EAC for ; Sat, 19 Oct 2019 20:29:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 987ED222BD for ; Sat, 19 Oct 2019 20:29:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1571516942; bh=8Kaz9ajmU9n+k2Gy/rnFl8gRn4yVH1HlPBnXfYxnktE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=ZQFqAtFWXruxsYciX7IaDRIZlFB7GG/M45ywIEQ2BJ7ONXU+xKqhqhuN8I/odjWlp I7GVgiQPAnbYg2P0JAzEcvAGqp3MBGYD1ApGEYHu1iFYVTgN2BG121a3A7eQAJTD7F jDko5NjiyO4OAQ8lUrcd9jstgs5IkXynDosLChV8= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726265AbfJSU3A (ORCPT ); Sat, 19 Oct 2019 16:29:00 -0400 Received: from inca-roads.misterjones.org ([213.251.177.50]:46519 "EHLO inca-roads.misterjones.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726129AbfJSU3A (ORCPT ); Sat, 19 Oct 2019 16:29:00 -0400 Received: from [185.104.136.17] (helo=big-swifty.misterjones.org) by cheepnis.misterjones.org with esmtpsa (TLSv1.2:AES256-GCM-SHA384:256) (Exim 4.80) (envelope-from ) id 1iLvL8-0003d4-Uo; Sat, 19 Oct 2019 22:28:43 +0200 Date: Sat, 19 Oct 2019 21:28:39 +0100 Message-ID: <86a79wzdhk.wl-maz@kernel.org> From: Marc Zyngier To: Steven Price Cc: Will Deacon , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, Catalin Marinas , Paolo Bonzini , Radim =?UTF-8?B?S3LEjW3DocWZ?= , Russell King , James Morse , Julien Thierry , Suzuki K Pouloze , Mark Rutland , kvm@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 10/10] arm64: Retrieve stolen time as paravirtualized guest In-Reply-To: <20191011125930.40834-11-steven.price@arm.com> References: <20191011125930.40834-1-steven.price@arm.com> <20191011125930.40834-11-steven.price@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.104.136.17 X-SA-Exim-Rcpt-To: steven.price@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, catalin.marinas@arm.com, pbonzini@redhat.com, rkrcmar@redhat.com, linux@armlinux.org.uk, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, mark.rutland@arm.com, kvm@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on cheepnis.misterjones.org); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 11 Oct 2019 13:59:30 +0100, Steven Price wrote: > > Enable paravirtualization features when running under a hypervisor > supporting the PV_TIME_ST hypercall. > > For each (v)CPU, we ask the hypervisor for the location of a shared > page which the hypervisor will use to report stolen time to us. We set > pv_time_ops to the stolen time function which simply reads the stolen > value from the shared page for a VCPU. We guarantee single-copy > atomicity using READ_ONCE which means we can also read the stolen > time for another VCPU than the currently running one while it is > potentially being updated by the hypervisor. > > Signed-off-by: Steven Price > --- > .../admin-guide/kernel-parameters.txt | 6 +- > arch/arm64/include/asm/paravirt.h | 9 +- > arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++ > arch/arm64/kernel/time.c | 3 + > include/linux/cpuhotplug.h | 1 + > 5 files changed, 163 insertions(+), 4 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index c7ac2f3ac99f..346b1c7a4afb 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3083,9 +3083,9 @@ > [X86,PV_OPS] Disable paravirtualized VMware scheduler > clock and use the default one. > > - no-steal-acc [X86,KVM] Disable paravirtualized steal time accounting. > - steal time is computed, but won't influence scheduler > - behaviour > + no-steal-acc [X86,KVM,ARM64] 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. > > diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h > index 799d9dd6f7cc..125c26c42902 100644 > --- a/arch/arm64/include/asm/paravirt.h > +++ b/arch/arm64/include/asm/paravirt.h > @@ -21,6 +21,13 @@ static inline u64 paravirt_steal_clock(int cpu) > { > return pv_ops.time.steal_clock(cpu); > } > -#endif > + > +int __init kvm_guest_init(void); > + > +#else > + > +#define kvm_guest_init() > + > +#endif // CONFIG_PARAVIRT > > #endif > diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c > index 4cfed91fe256..de73dbec238c 100644 > --- a/arch/arm64/kernel/paravirt.c > +++ b/arch/arm64/kernel/paravirt.c > @@ -6,13 +6,161 @@ > * Author: Stefano Stabellini > */ > > +#define pr_fmt(fmt) "kvmarm-pv: " fmt > + > +#include > +#include > #include > +#include > #include > +#include > +#include > +#include > +#include > #include > + > #include > +#include > +#include > > struct static_key paravirt_steal_enabled; > struct static_key paravirt_steal_rq_enabled; > > struct paravirt_patch_template pv_ops; > EXPORT_SYMBOL_GPL(pv_ops); > + > +struct kvmarm_stolen_time_region { > + struct pvclock_vcpu_stolen_time *kaddr; > +}; > + > +static DEFINE_PER_CPU(struct kvmarm_stolen_time_region, stolen_time_region); > + > +static bool steal_acc = true; > +static int __init parse_no_stealacc(char *arg) > +{ > + steal_acc = false; > + return 0; > +} > + > +early_param("no-steal-acc", parse_no_stealacc); > + > +/* return stolen time in ns by asking the hypervisor */ > +static u64 kvm_steal_clock(int cpu) This isn't KVM specific. > +{ > + struct kvmarm_stolen_time_region *reg; > + > + reg = per_cpu_ptr(&stolen_time_region, cpu); > + if (!reg->kaddr) { > + pr_warn_once("stolen time enabled but not configured for cpu %d\n", > + cpu); > + return 0; > + } > + > + return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time)); > +} > + > +static int disable_stolen_time_current_cpu(void) > +{ > + struct kvmarm_stolen_time_region *reg; > + > + reg = this_cpu_ptr(&stolen_time_region); > + if (!reg->kaddr) > + return 0; > + > + memunmap(reg->kaddr); > + memset(reg, 0, sizeof(*reg)); > + > + return 0; > +} > + > +static int stolen_time_dying_cpu(unsigned int cpu) > +{ > + return disable_stolen_time_current_cpu(); > +} You can merge these two functions, as there is no other caller. > + > +static int init_stolen_time_cpu(unsigned int cpu) > +{ > + struct kvmarm_stolen_time_region *reg; > + struct arm_smccc_res res; > + > + reg = this_cpu_ptr(&stolen_time_region); > + > + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_ST, &res); > + > + if ((long)res.a0 < 0) > + return -EINVAL; I'd rather you check an actual error code, just in case the memory map starts growing to a point where we have 64bit (I)PAs... > + > + reg->kaddr = memremap(res.a0, > + sizeof(struct pvclock_vcpu_stolen_time), > + MEMREMAP_WB); > + > + if (!reg->kaddr) { > + pr_warn("Failed to map stolen time data structure\n"); > + return -ENOMEM; > + } > + > + if (le32_to_cpu(reg->kaddr->revision) != 0 || > + le32_to_cpu(reg->kaddr->attributes) != 0) { > + pr_warn("Unexpected revision or attributes in stolen time data\n"); WARN_ONCE instead? You probably don't want to scream for each and every CPU that boots... > + return -ENXIO; > + } > + > + return 0; > +} > + > +static int kvm_arm_init_stolen_time(void) > +{ > + int ret; > + > + ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING, > + "hypervisor/kvmarm/pv:starting", > + init_stolen_time_cpu, stolen_time_dying_cpu); > + if (ret < 0) > + return ret; > + return 0; > +} > + > +static bool has_kvm_steal_clock(void) This is not KVM specific either. > +{ > + struct arm_smccc_res res; > + > + /* To detect the presence of PV time support we require SMCCC 1.1+ */ > + if (psci_ops.smccc_version < SMCCC_VERSION_1_1) > + return false; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, > + ARM_SMCCC_HV_PV_TIME_FEATURES, &res); > + > + if (res.a0 != SMCCC_RET_SUCCESS) > + return false; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_FEATURES, > + ARM_SMCCC_HV_PV_TIME_ST, &res); > + > + if (res.a0 != SMCCC_RET_SUCCESS) > + return false; > + > + return true; return (res.a0 == SMCCC_RET_SUCCESS); > +} > + > +int __init kvm_guest_init(void) How about something like pv_time_init() instead? In the guest, this is no way KVM specific, and I still hope for this to work on things like Xen/HyperV/VMware (yeah, I'm foolishly optimistic). All the references to KVM should go, and be replaced by something more generic (after all, you're only implementing the spec, so feel free to call it den0057_* if you really want). > +{ > + int ret; > + > + if (!has_kvm_steal_clock()) > + return 0; > + > + ret = kvm_arm_init_stolen_time(); > + if (ret) > + return ret; > + > + pv_ops.time.steal_clock = kvm_steal_clock; > + > + static_key_slow_inc(¶virt_steal_enabled); > + if (steal_acc) > + static_key_slow_inc(¶virt_steal_rq_enabled); > + > + pr_info("using stolen time PV\n"); > + > + return 0; > +} > diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c > index 0b2946414dc9..a52aea14c6ec 100644 > --- a/arch/arm64/kernel/time.c > +++ b/arch/arm64/kernel/time.c > @@ -30,6 +30,7 @@ > > #include > #include > +#include > > unsigned long profile_pc(struct pt_regs *regs) > { > @@ -65,4 +66,6 @@ void __init time_init(void) > > /* Calibrate the delay loop directly */ > lpj_fine = arch_timer_rate / HZ; > + > + kvm_guest_init(); > } > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > index 068793a619ca..89d75edb5750 100644 > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -136,6 +136,7 @@ enum cpuhp_state { > /* Must be the last timer callback */ > CPUHP_AP_DUMMY_TIMER_STARTING, > CPUHP_AP_ARM_XEN_STARTING, > + CPUHP_AP_ARM_KVMPV_STARTING, > CPUHP_AP_ARM_CORESIGHT_STARTING, > CPUHP_AP_ARM64_ISNDEP_STARTING, > CPUHP_AP_SMPCFD_DYING, > -- > 2.20.1 > > Thanks, M. -- Jazz is not dead, it just smells funny. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4A7C7CA9EAC for ; Sat, 19 Oct 2019 20:28:56 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id C4618222BD for ; Sat, 19 Oct 2019 20:28:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C4618222BD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 32D374A97C; Sat, 19 Oct 2019 16:28:55 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IbpBqkftkmsC; Sat, 19 Oct 2019 16:28:53 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id B62794AA12; Sat, 19 Oct 2019 16:28:53 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 514F64AA04 for ; Sat, 19 Oct 2019 16:28:52 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F9qBRG0oZBsE for ; Sat, 19 Oct 2019 16:28:50 -0400 (EDT) Received: from inca-roads.misterjones.org (inca-roads.misterjones.org [213.251.177.50]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 70C264AA03 for ; Sat, 19 Oct 2019 16:28:50 -0400 (EDT) Received: from [185.104.136.17] (helo=big-swifty.misterjones.org) by cheepnis.misterjones.org with esmtpsa (TLSv1.2:AES256-GCM-SHA384:256) (Exim 4.80) (envelope-from ) id 1iLvL8-0003d4-Uo; Sat, 19 Oct 2019 22:28:43 +0200 Date: Sat, 19 Oct 2019 21:28:39 +0100 Message-ID: <86a79wzdhk.wl-maz@kernel.org> From: Marc Zyngier To: Steven Price Subject: Re: [PATCH v6 10/10] arm64: Retrieve stolen time as paravirtualized guest In-Reply-To: <20191011125930.40834-11-steven.price@arm.com> References: <20191011125930.40834-1-steven.price@arm.com> <20191011125930.40834-11-steven.price@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.104.136.17 X-SA-Exim-Rcpt-To: steven.price@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, catalin.marinas@arm.com, pbonzini@redhat.com, rkrcmar@redhat.com, linux@armlinux.org.uk, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, mark.rutland@arm.com, kvm@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on cheepnis.misterjones.org); SAEximRunCond expanded to false Cc: kvm@vger.kernel.org, Catalin Marinas , linux-doc@vger.kernel.org, Russell King , linux-kernel@vger.kernel.org, Paolo Bonzini , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Fri, 11 Oct 2019 13:59:30 +0100, Steven Price wrote: > > Enable paravirtualization features when running under a hypervisor > supporting the PV_TIME_ST hypercall. > > For each (v)CPU, we ask the hypervisor for the location of a shared > page which the hypervisor will use to report stolen time to us. We set > pv_time_ops to the stolen time function which simply reads the stolen > value from the shared page for a VCPU. We guarantee single-copy > atomicity using READ_ONCE which means we can also read the stolen > time for another VCPU than the currently running one while it is > potentially being updated by the hypervisor. > > Signed-off-by: Steven Price > --- > .../admin-guide/kernel-parameters.txt | 6 +- > arch/arm64/include/asm/paravirt.h | 9 +- > arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++ > arch/arm64/kernel/time.c | 3 + > include/linux/cpuhotplug.h | 1 + > 5 files changed, 163 insertions(+), 4 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index c7ac2f3ac99f..346b1c7a4afb 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3083,9 +3083,9 @@ > [X86,PV_OPS] Disable paravirtualized VMware scheduler > clock and use the default one. > > - no-steal-acc [X86,KVM] Disable paravirtualized steal time accounting. > - steal time is computed, but won't influence scheduler > - behaviour > + no-steal-acc [X86,KVM,ARM64] 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. > > diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h > index 799d9dd6f7cc..125c26c42902 100644 > --- a/arch/arm64/include/asm/paravirt.h > +++ b/arch/arm64/include/asm/paravirt.h > @@ -21,6 +21,13 @@ static inline u64 paravirt_steal_clock(int cpu) > { > return pv_ops.time.steal_clock(cpu); > } > -#endif > + > +int __init kvm_guest_init(void); > + > +#else > + > +#define kvm_guest_init() > + > +#endif // CONFIG_PARAVIRT > > #endif > diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c > index 4cfed91fe256..de73dbec238c 100644 > --- a/arch/arm64/kernel/paravirt.c > +++ b/arch/arm64/kernel/paravirt.c > @@ -6,13 +6,161 @@ > * Author: Stefano Stabellini > */ > > +#define pr_fmt(fmt) "kvmarm-pv: " fmt > + > +#include > +#include > #include > +#include > #include > +#include > +#include > +#include > +#include > #include > + > #include > +#include > +#include > > struct static_key paravirt_steal_enabled; > struct static_key paravirt_steal_rq_enabled; > > struct paravirt_patch_template pv_ops; > EXPORT_SYMBOL_GPL(pv_ops); > + > +struct kvmarm_stolen_time_region { > + struct pvclock_vcpu_stolen_time *kaddr; > +}; > + > +static DEFINE_PER_CPU(struct kvmarm_stolen_time_region, stolen_time_region); > + > +static bool steal_acc = true; > +static int __init parse_no_stealacc(char *arg) > +{ > + steal_acc = false; > + return 0; > +} > + > +early_param("no-steal-acc", parse_no_stealacc); > + > +/* return stolen time in ns by asking the hypervisor */ > +static u64 kvm_steal_clock(int cpu) This isn't KVM specific. > +{ > + struct kvmarm_stolen_time_region *reg; > + > + reg = per_cpu_ptr(&stolen_time_region, cpu); > + if (!reg->kaddr) { > + pr_warn_once("stolen time enabled but not configured for cpu %d\n", > + cpu); > + return 0; > + } > + > + return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time)); > +} > + > +static int disable_stolen_time_current_cpu(void) > +{ > + struct kvmarm_stolen_time_region *reg; > + > + reg = this_cpu_ptr(&stolen_time_region); > + if (!reg->kaddr) > + return 0; > + > + memunmap(reg->kaddr); > + memset(reg, 0, sizeof(*reg)); > + > + return 0; > +} > + > +static int stolen_time_dying_cpu(unsigned int cpu) > +{ > + return disable_stolen_time_current_cpu(); > +} You can merge these two functions, as there is no other caller. > + > +static int init_stolen_time_cpu(unsigned int cpu) > +{ > + struct kvmarm_stolen_time_region *reg; > + struct arm_smccc_res res; > + > + reg = this_cpu_ptr(&stolen_time_region); > + > + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_ST, &res); > + > + if ((long)res.a0 < 0) > + return -EINVAL; I'd rather you check an actual error code, just in case the memory map starts growing to a point where we have 64bit (I)PAs... > + > + reg->kaddr = memremap(res.a0, > + sizeof(struct pvclock_vcpu_stolen_time), > + MEMREMAP_WB); > + > + if (!reg->kaddr) { > + pr_warn("Failed to map stolen time data structure\n"); > + return -ENOMEM; > + } > + > + if (le32_to_cpu(reg->kaddr->revision) != 0 || > + le32_to_cpu(reg->kaddr->attributes) != 0) { > + pr_warn("Unexpected revision or attributes in stolen time data\n"); WARN_ONCE instead? You probably don't want to scream for each and every CPU that boots... > + return -ENXIO; > + } > + > + return 0; > +} > + > +static int kvm_arm_init_stolen_time(void) > +{ > + int ret; > + > + ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING, > + "hypervisor/kvmarm/pv:starting", > + init_stolen_time_cpu, stolen_time_dying_cpu); > + if (ret < 0) > + return ret; > + return 0; > +} > + > +static bool has_kvm_steal_clock(void) This is not KVM specific either. > +{ > + struct arm_smccc_res res; > + > + /* To detect the presence of PV time support we require SMCCC 1.1+ */ > + if (psci_ops.smccc_version < SMCCC_VERSION_1_1) > + return false; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, > + ARM_SMCCC_HV_PV_TIME_FEATURES, &res); > + > + if (res.a0 != SMCCC_RET_SUCCESS) > + return false; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_FEATURES, > + ARM_SMCCC_HV_PV_TIME_ST, &res); > + > + if (res.a0 != SMCCC_RET_SUCCESS) > + return false; > + > + return true; return (res.a0 == SMCCC_RET_SUCCESS); > +} > + > +int __init kvm_guest_init(void) How about something like pv_time_init() instead? In the guest, this is no way KVM specific, and I still hope for this to work on things like Xen/HyperV/VMware (yeah, I'm foolishly optimistic). All the references to KVM should go, and be replaced by something more generic (after all, you're only implementing the spec, so feel free to call it den0057_* if you really want). > +{ > + int ret; > + > + if (!has_kvm_steal_clock()) > + return 0; > + > + ret = kvm_arm_init_stolen_time(); > + if (ret) > + return ret; > + > + pv_ops.time.steal_clock = kvm_steal_clock; > + > + static_key_slow_inc(¶virt_steal_enabled); > + if (steal_acc) > + static_key_slow_inc(¶virt_steal_rq_enabled); > + > + pr_info("using stolen time PV\n"); > + > + return 0; > +} > diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c > index 0b2946414dc9..a52aea14c6ec 100644 > --- a/arch/arm64/kernel/time.c > +++ b/arch/arm64/kernel/time.c > @@ -30,6 +30,7 @@ > > #include > #include > +#include > > unsigned long profile_pc(struct pt_regs *regs) > { > @@ -65,4 +66,6 @@ void __init time_init(void) > > /* Calibrate the delay loop directly */ > lpj_fine = arch_timer_rate / HZ; > + > + kvm_guest_init(); > } > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > index 068793a619ca..89d75edb5750 100644 > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -136,6 +136,7 @@ enum cpuhp_state { > /* Must be the last timer callback */ > CPUHP_AP_DUMMY_TIMER_STARTING, > CPUHP_AP_ARM_XEN_STARTING, > + CPUHP_AP_ARM_KVMPV_STARTING, > CPUHP_AP_ARM_CORESIGHT_STARTING, > CPUHP_AP_ARM64_ISNDEP_STARTING, > CPUHP_AP_SMPCFD_DYING, > -- > 2.20.1 > > Thanks, M. -- Jazz is not dead, it just smells funny. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 49317CA9EAE for ; Sat, 19 Oct 2019 20:29:04 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1EBDB222BD for ; Sat, 19 Oct 2019 20:29:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="maiJMcmZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1EBDB222BD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=otAssRemZl4WcAbLQO/AhVgAcI+YyUtBffcYn0NXiUc=; b=maiJMcmZ5p7LQz AVxFmpy6NMhLjPeZ3O0t/wqxWZCYI7JC3BDIzyTEFwNjCaGcdhId7GZhtyg3cd1mPgs0DHogKQfpp wQdZ1QWSqsZCFKajn9mKHo5wZ78E/HGnHsTMzvyQxk5dOnpgoPMhJ4aeKt7+COZMNfbAuwn1oxWfJ Kx7001xv8bji3rCX6DaNFeKgzY88Vy+AI3XuWEXZL1CMx6jo3+XK/XZNAH16d4kTIzU00+b3++gXd HIxDs6r1tUH41VbqRDd4EPc3Dkgv2TeiyWVQ6qp/Xja5+t5oU249ffpYz0MqKq8XfrswFtYAz/Bcm vGeaQCwDzYLQ/9epCq4g==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iLvLH-0005D4-BE; Sat, 19 Oct 2019 20:28:51 +0000 Received: from inca-roads.misterjones.org ([213.251.177.50]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iLvLD-0005Ca-IR for linux-arm-kernel@lists.infradead.org; Sat, 19 Oct 2019 20:28:49 +0000 Received: from [185.104.136.17] (helo=big-swifty.misterjones.org) by cheepnis.misterjones.org with esmtpsa (TLSv1.2:AES256-GCM-SHA384:256) (Exim 4.80) (envelope-from ) id 1iLvL8-0003d4-Uo; Sat, 19 Oct 2019 22:28:43 +0200 Date: Sat, 19 Oct 2019 21:28:39 +0100 Message-ID: <86a79wzdhk.wl-maz@kernel.org> From: Marc Zyngier To: Steven Price Subject: Re: [PATCH v6 10/10] arm64: Retrieve stolen time as paravirtualized guest In-Reply-To: <20191011125930.40834-11-steven.price@arm.com> References: <20191011125930.40834-1-steven.price@arm.com> <20191011125930.40834-11-steven.price@arm.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.104.136.17 X-SA-Exim-Rcpt-To: steven.price@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, catalin.marinas@arm.com, pbonzini@redhat.com, rkrcmar@redhat.com, linux@armlinux.org.uk, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, mark.rutland@arm.com, kvm@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on cheepnis.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191019_132847_761831_BCE8299C X-CRM114-Status: GOOD ( 26.00 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , kvm@vger.kernel.org, Radim =?UTF-8?B?S3LEjW3DocWZ?= , Catalin Marinas , Suzuki K Pouloze , linux-doc@vger.kernel.org, Russell King , linux-kernel@vger.kernel.org, James Morse , Julien Thierry , Paolo Bonzini , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 11 Oct 2019 13:59:30 +0100, Steven Price wrote: > > Enable paravirtualization features when running under a hypervisor > supporting the PV_TIME_ST hypercall. > > For each (v)CPU, we ask the hypervisor for the location of a shared > page which the hypervisor will use to report stolen time to us. We set > pv_time_ops to the stolen time function which simply reads the stolen > value from the shared page for a VCPU. We guarantee single-copy > atomicity using READ_ONCE which means we can also read the stolen > time for another VCPU than the currently running one while it is > potentially being updated by the hypervisor. > > Signed-off-by: Steven Price > --- > .../admin-guide/kernel-parameters.txt | 6 +- > arch/arm64/include/asm/paravirt.h | 9 +- > arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++ > arch/arm64/kernel/time.c | 3 + > include/linux/cpuhotplug.h | 1 + > 5 files changed, 163 insertions(+), 4 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index c7ac2f3ac99f..346b1c7a4afb 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3083,9 +3083,9 @@ > [X86,PV_OPS] Disable paravirtualized VMware scheduler > clock and use the default one. > > - no-steal-acc [X86,KVM] Disable paravirtualized steal time accounting. > - steal time is computed, but won't influence scheduler > - behaviour > + no-steal-acc [X86,KVM,ARM64] 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. > > diff --git a/arch/arm64/include/asm/paravirt.h b/arch/arm64/include/asm/paravirt.h > index 799d9dd6f7cc..125c26c42902 100644 > --- a/arch/arm64/include/asm/paravirt.h > +++ b/arch/arm64/include/asm/paravirt.h > @@ -21,6 +21,13 @@ static inline u64 paravirt_steal_clock(int cpu) > { > return pv_ops.time.steal_clock(cpu); > } > -#endif > + > +int __init kvm_guest_init(void); > + > +#else > + > +#define kvm_guest_init() > + > +#endif // CONFIG_PARAVIRT > > #endif > diff --git a/arch/arm64/kernel/paravirt.c b/arch/arm64/kernel/paravirt.c > index 4cfed91fe256..de73dbec238c 100644 > --- a/arch/arm64/kernel/paravirt.c > +++ b/arch/arm64/kernel/paravirt.c > @@ -6,13 +6,161 @@ > * Author: Stefano Stabellini > */ > > +#define pr_fmt(fmt) "kvmarm-pv: " fmt > + > +#include > +#include > #include > +#include > #include > +#include > +#include > +#include > +#include > #include > + > #include > +#include > +#include > > struct static_key paravirt_steal_enabled; > struct static_key paravirt_steal_rq_enabled; > > struct paravirt_patch_template pv_ops; > EXPORT_SYMBOL_GPL(pv_ops); > + > +struct kvmarm_stolen_time_region { > + struct pvclock_vcpu_stolen_time *kaddr; > +}; > + > +static DEFINE_PER_CPU(struct kvmarm_stolen_time_region, stolen_time_region); > + > +static bool steal_acc = true; > +static int __init parse_no_stealacc(char *arg) > +{ > + steal_acc = false; > + return 0; > +} > + > +early_param("no-steal-acc", parse_no_stealacc); > + > +/* return stolen time in ns by asking the hypervisor */ > +static u64 kvm_steal_clock(int cpu) This isn't KVM specific. > +{ > + struct kvmarm_stolen_time_region *reg; > + > + reg = per_cpu_ptr(&stolen_time_region, cpu); > + if (!reg->kaddr) { > + pr_warn_once("stolen time enabled but not configured for cpu %d\n", > + cpu); > + return 0; > + } > + > + return le64_to_cpu(READ_ONCE(reg->kaddr->stolen_time)); > +} > + > +static int disable_stolen_time_current_cpu(void) > +{ > + struct kvmarm_stolen_time_region *reg; > + > + reg = this_cpu_ptr(&stolen_time_region); > + if (!reg->kaddr) > + return 0; > + > + memunmap(reg->kaddr); > + memset(reg, 0, sizeof(*reg)); > + > + return 0; > +} > + > +static int stolen_time_dying_cpu(unsigned int cpu) > +{ > + return disable_stolen_time_current_cpu(); > +} You can merge these two functions, as there is no other caller. > + > +static int init_stolen_time_cpu(unsigned int cpu) > +{ > + struct kvmarm_stolen_time_region *reg; > + struct arm_smccc_res res; > + > + reg = this_cpu_ptr(&stolen_time_region); > + > + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_ST, &res); > + > + if ((long)res.a0 < 0) > + return -EINVAL; I'd rather you check an actual error code, just in case the memory map starts growing to a point where we have 64bit (I)PAs... > + > + reg->kaddr = memremap(res.a0, > + sizeof(struct pvclock_vcpu_stolen_time), > + MEMREMAP_WB); > + > + if (!reg->kaddr) { > + pr_warn("Failed to map stolen time data structure\n"); > + return -ENOMEM; > + } > + > + if (le32_to_cpu(reg->kaddr->revision) != 0 || > + le32_to_cpu(reg->kaddr->attributes) != 0) { > + pr_warn("Unexpected revision or attributes in stolen time data\n"); WARN_ONCE instead? You probably don't want to scream for each and every CPU that boots... > + return -ENXIO; > + } > + > + return 0; > +} > + > +static int kvm_arm_init_stolen_time(void) > +{ > + int ret; > + > + ret = cpuhp_setup_state(CPUHP_AP_ARM_KVMPV_STARTING, > + "hypervisor/kvmarm/pv:starting", > + init_stolen_time_cpu, stolen_time_dying_cpu); > + if (ret < 0) > + return ret; > + return 0; > +} > + > +static bool has_kvm_steal_clock(void) This is not KVM specific either. > +{ > + struct arm_smccc_res res; > + > + /* To detect the presence of PV time support we require SMCCC 1.1+ */ > + if (psci_ops.smccc_version < SMCCC_VERSION_1_1) > + return false; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID, > + ARM_SMCCC_HV_PV_TIME_FEATURES, &res); > + > + if (res.a0 != SMCCC_RET_SUCCESS) > + return false; > + > + arm_smccc_1_1_invoke(ARM_SMCCC_HV_PV_TIME_FEATURES, > + ARM_SMCCC_HV_PV_TIME_ST, &res); > + > + if (res.a0 != SMCCC_RET_SUCCESS) > + return false; > + > + return true; return (res.a0 == SMCCC_RET_SUCCESS); > +} > + > +int __init kvm_guest_init(void) How about something like pv_time_init() instead? In the guest, this is no way KVM specific, and I still hope for this to work on things like Xen/HyperV/VMware (yeah, I'm foolishly optimistic). All the references to KVM should go, and be replaced by something more generic (after all, you're only implementing the spec, so feel free to call it den0057_* if you really want). > +{ > + int ret; > + > + if (!has_kvm_steal_clock()) > + return 0; > + > + ret = kvm_arm_init_stolen_time(); > + if (ret) > + return ret; > + > + pv_ops.time.steal_clock = kvm_steal_clock; > + > + static_key_slow_inc(¶virt_steal_enabled); > + if (steal_acc) > + static_key_slow_inc(¶virt_steal_rq_enabled); > + > + pr_info("using stolen time PV\n"); > + > + return 0; > +} > diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c > index 0b2946414dc9..a52aea14c6ec 100644 > --- a/arch/arm64/kernel/time.c > +++ b/arch/arm64/kernel/time.c > @@ -30,6 +30,7 @@ > > #include > #include > +#include > > unsigned long profile_pc(struct pt_regs *regs) > { > @@ -65,4 +66,6 @@ void __init time_init(void) > > /* Calibrate the delay loop directly */ > lpj_fine = arch_timer_rate / HZ; > + > + kvm_guest_init(); > } > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > index 068793a619ca..89d75edb5750 100644 > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -136,6 +136,7 @@ enum cpuhp_state { > /* Must be the last timer callback */ > CPUHP_AP_DUMMY_TIMER_STARTING, > CPUHP_AP_ARM_XEN_STARTING, > + CPUHP_AP_ARM_KVMPV_STARTING, > CPUHP_AP_ARM_CORESIGHT_STARTING, > CPUHP_AP_ARM64_ISNDEP_STARTING, > CPUHP_AP_SMPCFD_DYING, > -- > 2.20.1 > > Thanks, M. -- Jazz is not dead, it just smells funny. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel