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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B0197C7EE29 for ; Thu, 25 May 2023 23:36:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233619AbjEYXgd (ORCPT ); Thu, 25 May 2023 19:36:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56664 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230389AbjEYXgb (ORCPT ); Thu, 25 May 2023 19:36:31 -0400 Received: from out-31.mta1.migadu.com (out-31.mta1.migadu.com [95.215.58.31]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5226FA3 for ; Thu, 25 May 2023 16:36:30 -0700 (PDT) Date: Thu, 25 May 2023 23:36:23 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1685057788; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=P6zAesvCX1np6qowsiTDFn1Ttg3LeLNqimbKFFrO44k=; b=hFk/P8VbvqCXvnUFBR0TDZqH7RTktZxcVeToKNNL9QKObZGJFbHkEb7Cxwj065MufplhgQ GPjZk2yUiY55AT2nqCd4XmNXwUuD4QkGZi4SwSPCII8JWa8KxwTAQTzJq+7cieq65NF1/v 946/dwlxo+XduDWeqmM9BP7IolOhSR4= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Reiji Watanabe Cc: Marc Zyngier , Mark Rutland , Will Deacon , Catalin Marinas , kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, James Morse , Alexandru Elisei , Zenghui Yu , Suzuki K Poulose , Paolo Bonzini , Ricardo Koller , Jing Zhang , Raghavendra Rao Anata , Shaoqin Huang , Rob Herring Subject: Re: [PATCH v3 2/2] KVM: arm64: PMU: Don't overwrite PMUSERENR with vcpu loaded Message-ID: References: <20230415164029.526895-1-reijiw@google.com> <20230415164029.526895-3-reijiw@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230415164029.526895-3-reijiw@google.com> X-Migadu-Flow: FLOW_OUT Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Reiji, Apologies, this fell off my list of reviews. On Sat, Apr 15, 2023 at 09:40:29AM -0700, Reiji Watanabe wrote: [...] > static void armv8pmu_enable_event(struct perf_event *event) > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 6718731729fd..7e73be12cfaf 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -82,12 +82,24 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu) > */ > if (kvm_arm_support_pmu_v3()) { > struct kvm_cpu_context *hctxt; > + unsigned long flags; > > write_sysreg(0, pmselr_el0); > > hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > + > + /* > + * Disable IRQs to prevent a race condition between the > + * following code and IPIs that attempts to update > + * PMUSERENR_EL0. See also kvm_set_pmuserenr(). > + */ > + local_irq_save(flags); > + > ctxt_sys_reg(hctxt, PMUSERENR_EL0) = read_sysreg(pmuserenr_el0); > write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0); > + vcpu_set_flag(vcpu, PMUSERENR_ON_CPU); > + > + local_irq_restore(flags); Can the IRQ save/restore be moved to {activate,deactivate}_traps_vhe_{load,put}()? That'd eliminate the dance to avoid using kernel-only symbols in nVHE and would be consistent with the existing usage of __{activate,deactivate}_traps_common() from nVHE (IRQs already disabled). IMO, the less nVHE knows about the kernel the better. > } > > vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2); > @@ -112,9 +124,21 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > write_sysreg(0, hstr_el2); > if (kvm_arm_support_pmu_v3()) { > struct kvm_cpu_context *hctxt; > + unsigned long flags; > > hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > + > + /* > + * Disable IRQs to prevent a race condition between the > + * following code and IPIs that attempts to update > + * PMUSERENR_EL0. See also kvm_set_pmuserenr(). > + */ > + local_irq_save(flags); > + > write_sysreg(ctxt_sys_reg(hctxt, PMUSERENR_EL0), pmuserenr_el0); > + vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU); > + > + local_irq_restore(flags); > } > > if (cpus_have_final_cap(ARM64_SME)) { > diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile > index 530347cdebe3..2c08a54ca7d9 100644 > --- a/arch/arm64/kvm/hyp/nvhe/Makefile > +++ b/arch/arm64/kvm/hyp/nvhe/Makefile > @@ -10,7 +10,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__ -D__DISABLE_EXPORTS > # will explode instantly (Words of Marc Zyngier). So introduce a generic flag > # __DISABLE_TRACE_MMIO__ to disable MMIO tracing for nVHE KVM. > ccflags-y := -D__KVM_NVHE_HYPERVISOR__ -D__DISABLE_EXPORTS -D__DISABLE_TRACE_MMIO__ > -ccflags-y += -fno-stack-protector \ > +ccflags-y += -fno-stack-protector -DNO_TRACE_IRQFLAGS \ > -DDISABLE_BRANCH_PROFILING \ > $(DISABLE_STACKLEAK_PLUGIN) > > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c > index 7887133d15f0..d6a863853bfe 100644 > --- a/arch/arm64/kvm/pmu.c > +++ b/arch/arm64/kvm/pmu.c > @@ -209,3 +209,28 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) > kvm_vcpu_pmu_enable_el0(events_host); > kvm_vcpu_pmu_disable_el0(events_guest); > } > + > +/* > + * With VHE, keep track of the PMUSERENR_EL0 value for the host EL0 on the pCPU > + * where PMUSERENR_EL0 for the guest is loaded, since PMUSERENR_EL0 is switched > + * to the value for the guest on vcpu_load(). The value for the host EL0 > + * will be restored on vcpu_put(), before returning to the EL0. wording: s/the EL0/EL0. Or, alternatively, to avoid repeating yourself you can just say "returning to userspace". You may also want to mention in passing why this isn't necessary for nVHE, as the register is context switched for every guest enter/exit. > + * > + * Return true if KVM takes care of the register. Otherwise return false. > + */ > +bool kvm_set_pmuserenr(u64 val) > +{ > + struct kvm_cpu_context *hctxt; > + struct kvm_vcpu *vcpu; > + > + if (!kvm_arm_support_pmu_v3() || !has_vhe()) > + return false; > + > + vcpu = kvm_get_running_vcpu(); > + if (!vcpu || !vcpu_get_flag(vcpu, PMUSERENR_ON_CPU)) > + return false; > + > + hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > + ctxt_sys_reg(hctxt, PMUSERENR_EL0) = val; > + return true; > +} -- Thanks, Oliver 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 32CDFC7EE29 for ; Thu, 25 May 2023 23:37:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=dfsklOU4NeAin5okOK6bE7aPE/r2cj03Yxcf8cOPzls=; b=49qYMoxezj2S35 XuNPFy2WizMXUMgVj9zh7jdcEkpOs2NTl621634qnLVhn2j2KTUEUXWJ+6NpdLCfTbgMdby0BRurZ Kj2LksjQPjPncOoZur9bqPit/0UTgEJla14Y1qqCvqDVV1dt7SeplMgZXr/dI5zsMjcXDHy6cW1dw wYOE6tvcRN7YSWvcke1UJbbZ0Gjipzh3qFevVKaujyp8NZH8AZwSDicd3JIhKyxmhW8LOMptHfROX 3n6FC3s4yTPw0TRKMbAU6hc67FEWrkJ39ew1YBWaANoksO9gexYI8iBAl2PRaqDbTbAlXYxOJFy6m ZcJ746RyDdjCg42fF4uA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q2KVX-000QAj-0a; Thu, 25 May 2023 23:36:35 +0000 Received: from out-3.mta1.migadu.com ([95.215.58.3]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q2KVU-000Q9v-04 for linux-arm-kernel@lists.infradead.org; Thu, 25 May 2023 23:36:34 +0000 Date: Thu, 25 May 2023 23:36:23 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1685057788; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=P6zAesvCX1np6qowsiTDFn1Ttg3LeLNqimbKFFrO44k=; b=hFk/P8VbvqCXvnUFBR0TDZqH7RTktZxcVeToKNNL9QKObZGJFbHkEb7Cxwj065MufplhgQ GPjZk2yUiY55AT2nqCd4XmNXwUuD4QkGZi4SwSPCII8JWa8KxwTAQTzJq+7cieq65NF1/v 946/dwlxo+XduDWeqmM9BP7IolOhSR4= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Reiji Watanabe Cc: Marc Zyngier , Mark Rutland , Will Deacon , Catalin Marinas , kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, James Morse , Alexandru Elisei , Zenghui Yu , Suzuki K Poulose , Paolo Bonzini , Ricardo Koller , Jing Zhang , Raghavendra Rao Anata , Shaoqin Huang , Rob Herring Subject: Re: [PATCH v3 2/2] KVM: arm64: PMU: Don't overwrite PMUSERENR with vcpu loaded Message-ID: References: <20230415164029.526895-1-reijiw@google.com> <20230415164029.526895-3-reijiw@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230415164029.526895-3-reijiw@google.com> X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230525_163632_623991_83C350E5 X-CRM114-Status: GOOD ( 21.54 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Reiji, Apologies, this fell off my list of reviews. On Sat, Apr 15, 2023 at 09:40:29AM -0700, Reiji Watanabe wrote: [...] > static void armv8pmu_enable_event(struct perf_event *event) > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h > index 6718731729fd..7e73be12cfaf 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h > @@ -82,12 +82,24 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu) > */ > if (kvm_arm_support_pmu_v3()) { > struct kvm_cpu_context *hctxt; > + unsigned long flags; > > write_sysreg(0, pmselr_el0); > > hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > + > + /* > + * Disable IRQs to prevent a race condition between the > + * following code and IPIs that attempts to update > + * PMUSERENR_EL0. See also kvm_set_pmuserenr(). > + */ > + local_irq_save(flags); > + > ctxt_sys_reg(hctxt, PMUSERENR_EL0) = read_sysreg(pmuserenr_el0); > write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0); > + vcpu_set_flag(vcpu, PMUSERENR_ON_CPU); > + > + local_irq_restore(flags); Can the IRQ save/restore be moved to {activate,deactivate}_traps_vhe_{load,put}()? That'd eliminate the dance to avoid using kernel-only symbols in nVHE and would be consistent with the existing usage of __{activate,deactivate}_traps_common() from nVHE (IRQs already disabled). IMO, the less nVHE knows about the kernel the better. > } > > vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2); > @@ -112,9 +124,21 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu) > write_sysreg(0, hstr_el2); > if (kvm_arm_support_pmu_v3()) { > struct kvm_cpu_context *hctxt; > + unsigned long flags; > > hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > + > + /* > + * Disable IRQs to prevent a race condition between the > + * following code and IPIs that attempts to update > + * PMUSERENR_EL0. See also kvm_set_pmuserenr(). > + */ > + local_irq_save(flags); > + > write_sysreg(ctxt_sys_reg(hctxt, PMUSERENR_EL0), pmuserenr_el0); > + vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU); > + > + local_irq_restore(flags); > } > > if (cpus_have_final_cap(ARM64_SME)) { > diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile > index 530347cdebe3..2c08a54ca7d9 100644 > --- a/arch/arm64/kvm/hyp/nvhe/Makefile > +++ b/arch/arm64/kvm/hyp/nvhe/Makefile > @@ -10,7 +10,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__ -D__DISABLE_EXPORTS > # will explode instantly (Words of Marc Zyngier). So introduce a generic flag > # __DISABLE_TRACE_MMIO__ to disable MMIO tracing for nVHE KVM. > ccflags-y := -D__KVM_NVHE_HYPERVISOR__ -D__DISABLE_EXPORTS -D__DISABLE_TRACE_MMIO__ > -ccflags-y += -fno-stack-protector \ > +ccflags-y += -fno-stack-protector -DNO_TRACE_IRQFLAGS \ > -DDISABLE_BRANCH_PROFILING \ > $(DISABLE_STACKLEAK_PLUGIN) > > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c > index 7887133d15f0..d6a863853bfe 100644 > --- a/arch/arm64/kvm/pmu.c > +++ b/arch/arm64/kvm/pmu.c > @@ -209,3 +209,28 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu) > kvm_vcpu_pmu_enable_el0(events_host); > kvm_vcpu_pmu_disable_el0(events_guest); > } > + > +/* > + * With VHE, keep track of the PMUSERENR_EL0 value for the host EL0 on the pCPU > + * where PMUSERENR_EL0 for the guest is loaded, since PMUSERENR_EL0 is switched > + * to the value for the guest on vcpu_load(). The value for the host EL0 > + * will be restored on vcpu_put(), before returning to the EL0. wording: s/the EL0/EL0. Or, alternatively, to avoid repeating yourself you can just say "returning to userspace". You may also want to mention in passing why this isn't necessary for nVHE, as the register is context switched for every guest enter/exit. > + * > + * Return true if KVM takes care of the register. Otherwise return false. > + */ > +bool kvm_set_pmuserenr(u64 val) > +{ > + struct kvm_cpu_context *hctxt; > + struct kvm_vcpu *vcpu; > + > + if (!kvm_arm_support_pmu_v3() || !has_vhe()) > + return false; > + > + vcpu = kvm_get_running_vcpu(); > + if (!vcpu || !vcpu_get_flag(vcpu, PMUSERENR_ON_CPU)) > + return false; > + > + hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt; > + ctxt_sys_reg(hctxt, PMUSERENR_EL0) = val; > + return true; > +} -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel