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 26ACAC433F5 for ; Mon, 9 May 2022 20:23:23 +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=2hDy3Hs04Mfu8wpPQGqiW4czjsTpWJSmU6AdSbvFi1s=; b=grUVw7MK/Fhqxa +WU+0D141iEj3MbiJhVLSk6dQ3sbVoCNRe5dsiSHAgkRVbFktvNHAAxs68dhFjDgW9e3vRKmFJ17C aum8NCBdK4zne2TFNde8z0RJRTuTJw6Z1uDdHugvDLUPqlu7k37D2hb5QRdQpOT8CUdq2iS6kIolL bQ9HQZBQfDUqu2vDrnfZBSkiJmjM+qkJbsNS0qzCxrEBfHfhQBDNoRndJmYcIUg+1KP0TOFsZJHbn V3l0jnxATCaPlrGbg9TRt2SH17Bj7Jz2xb46E8pQmSn2pDHuX0ULzFLEywSLtIQ1b/ft35t+YFIwo Uw5qj5ZMqegvv03GJzXw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1no9tE-00G8pv-8I; Mon, 09 May 2022 20:21:56 +0000 Received: from mail-il1-x12e.google.com ([2607:f8b0:4864:20::12e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1no9t9-00G8pI-Hm for linux-arm-kernel@lists.infradead.org; Mon, 09 May 2022 20:21:54 +0000 Received: by mail-il1-x12e.google.com with SMTP id h11so10092491ila.5 for ; Mon, 09 May 2022 13:21:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=UcREA7BXivHZcoO141I3GT0yFqoEx7+7587siIxHTCs=; b=rzQHujXFmtG4GrbKAcEdZerC8MZayevox98SlYiOKBpHgZxI+MCs2qpXt81KogHQCi woMfL4FpqxFQeEzAVtm+qCLrPiYDMOkMX63WlVY1h9QFkVLSVtY9bdeoMx6iy01ZeZEh TtnRiBmNdZk7vPJamrjc+CMxxX46FGt/6gq45NewOxi6Sgigx92NRYjFw5MwAIXu2IVc EY8gVGhAy/r3j2RxjE1RWpYnXcaZAMiuU3RLP46ZGEXF/wcPyxtLHoGPEhDqUyk9fHi3 DZYdhxpIkrY7SaFj2A64PbThu/aFwWfBnuhD4pzkGNtEOV5dMhAgWO0D8tG5jx25qSol TiCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=UcREA7BXivHZcoO141I3GT0yFqoEx7+7587siIxHTCs=; b=Ob1d9UE7iTjPLzO1AoEepre0wdNqb714nIMzXGap4R2/MmDAdm3qOQCPYE72M1yLDo 8hz9XO47YwWIWmp+KwIodi3Z1pPh6v1S1440bg34dhCXrufv/v7JOSxp0ygBMh2w5Hs1 3dyNphO3gW38YGuQOJdiOL5YSNJkS2zmkw7MeYFnD3bPtKoeE0RFCGe2Notz2U5zxkkU YWLA380ySaevE5QI2PpzHHWUZ9c5SrEdOnFCQ+PqYC3Gi4XxcZBprqvLxxQjBImWqW6v MngHwVYtwO8etAq2W41dKXh6WDJ8UR7yt2nDG4OJZzgID17AvVr/es2HtVQhhvq2jSIR TXeA== X-Gm-Message-State: AOAM532OYD7Xdar2AYcD7rRvOBwSlnn/mKY0Y+RhEuBV3TsfCsbZwxlN Htknc6ymG5l/g6re4eThwv1rrw== X-Google-Smtp-Source: ABdhPJzIg6TeygVAyYQlxXmyGBsqJKP8u8DBZXPknVy3rSzaza12eWCngsZhp+TCGq0KZEmun4BLzg== X-Received: by 2002:a05:6e02:1bce:b0:2cf:7adc:7a99 with SMTP id x14-20020a056e021bce00b002cf7adc7a99mr7382010ilv.166.1652127710063; Mon, 09 May 2022 13:21:50 -0700 (PDT) Received: from google.com (194.225.68.34.bc.googleusercontent.com. [34.68.225.194]) by smtp.gmail.com with ESMTPSA id m19-20020a023c13000000b0032b3a781781sm3794035jaa.69.2022.05.09.13.21.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 May 2022 13:21:49 -0700 (PDT) Date: Mon, 9 May 2022 20:21:45 +0000 From: Oliver Upton To: Fuad Tabba Cc: kvmarm@lists.cs.columbia.edu, kernel-team@android.com, maz@kernel.org, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 3/4] KVM: arm64: Pass pmu events to hyp via vcpu Message-ID: References: <20220509120330.3043773-1-tabba@google.com> <20220509120330.3043773-4-tabba@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220509120330.3043773-4-tabba@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220509_132153_012477_314C0389 X-CRM114-Status: GOOD ( 33.50 ) 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 On Mon, May 09, 2022 at 12:03:29PM +0000, Fuad Tabba wrote: > Instead of the host accessing hyp data directly, pass the pmu > events of the current cpu to hyp via the vcpu. > > This adds 64 bits (in two fields) to the vcpu that need to be > synced before every vcpu run in nvhe and protected modes. > However, it isolates the hypervisor from the host, which allows > us to use pmu in protected mode in a subsequent patch. > > No visible side effects in behavior intended. > > Signed-off-by: Fuad Tabba > --- > arch/arm64/include/asm/kvm_host.h | 8 ++------ > arch/arm64/kvm/arm.c | 15 +++++++++++++++ > arch/arm64/kvm/hyp/nvhe/switch.c | 20 ++++++-------------- > arch/arm64/kvm/pmu.c | 12 ++++-------- > include/kvm/arm_pmu.h | 6 ++++++ > 5 files changed, 33 insertions(+), 28 deletions(-) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index dfd360404dd8..90476e713643 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -273,14 +273,8 @@ struct kvm_cpu_context { > struct kvm_vcpu *__hyp_running_vcpu; > }; > > -struct kvm_pmu_events { > - u32 events_host; > - u32 events_guest; > -}; > - Looks like you're moving this to arm_pmu.h as well. Probably a better home for it, but unclear why it is done in this patch. > struct kvm_host_data { > struct kvm_cpu_context host_ctxt; > - struct kvm_pmu_events pmu_events; > }; > Are we going to need this struct any more since it now has a single member? > struct kvm_host_psci_config { > @@ -763,6 +757,7 @@ void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome); > struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr); > > DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data); > +DECLARE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events); Why do you need this declaration? I don't see the percpu data being accessed outside of pmu.c. > static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt) > { > @@ -821,6 +816,7 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu); > void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr); > void kvm_clr_pmu_events(u32 clr); > > +struct kvm_pmu_events *kvm_get_pmu_events(void); > void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu); > void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu); > #else > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 2adb5832a756..86bcdb2a23a8 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -817,6 +817,19 @@ static int noinstr kvm_arm_vcpu_enter_exit(struct kvm_vcpu *vcpu) > return ret; > } > > +/* > + * Updates the vcpu's view of the pmu events for this cpu. > + * Must be called before every vcpu run after disabling interrupts, to ensure > + * that an interrupt cannot fire and update the structure. > + */ > +static void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu) > +{ > + if (has_vhe() || !kvm_vcpu_has_pmu(vcpu)) > + return; > + > + vcpu->arch.pmu.events = *kvm_get_pmu_events(); > +} > + > /** > * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code > * @vcpu: The VCPU pointer > @@ -882,6 +895,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > > kvm_vgic_flush_hwstate(vcpu); > > + kvm_pmu_update_vcpu_events(vcpu); > + > /* > * Ensure we set mode to IN_GUEST_MODE after we disable > * interrupts and before the final VCPU requests check. > diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c > index 0716163313d6..c61120ec8d1a 100644 > --- a/arch/arm64/kvm/hyp/nvhe/switch.c > +++ b/arch/arm64/kvm/hyp/nvhe/switch.c > @@ -153,13 +153,9 @@ static void __hyp_vgic_restore_state(struct kvm_vcpu *vcpu) > /* > * Disable host events, enable guest events > */ > -static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt) > +static bool __pmu_switch_to_guest(struct kvm_vcpu *vcpu) > { > - struct kvm_host_data *host; > - struct kvm_pmu_events *pmu; > - > - host = container_of(host_ctxt, struct kvm_host_data, host_ctxt); > - pmu = &host->pmu_events; > + struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events; > > if (pmu->events_host) > write_sysreg(pmu->events_host, pmcntenclr_el0); > @@ -173,13 +169,9 @@ static bool __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt) > /* > * Disable guest events, enable host events > */ > -static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt) > +static void __pmu_switch_to_host(struct kvm_vcpu *vcpu) > { > - struct kvm_host_data *host; > - struct kvm_pmu_events *pmu; > - > - host = container_of(host_ctxt, struct kvm_host_data, host_ctxt); > - pmu = &host->pmu_events; > + struct kvm_pmu_events *pmu = &vcpu->arch.pmu.events; > > if (pmu->events_guest) > write_sysreg(pmu->events_guest, pmcntenclr_el0); > @@ -304,7 +296,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) > host_ctxt->__hyp_running_vcpu = vcpu; > guest_ctxt = &vcpu->arch.ctxt; > > - pmu_switch_needed = __pmu_switch_to_guest(host_ctxt); > + pmu_switch_needed = __pmu_switch_to_guest(vcpu); > > __sysreg_save_state_nvhe(host_ctxt); > /* > @@ -366,7 +358,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) > __debug_restore_host_buffers_nvhe(vcpu); > > if (pmu_switch_needed) > - __pmu_switch_to_host(host_ctxt); > + __pmu_switch_to_host(vcpu); > > /* Returning to host will clear PSR.I, remask PMR if needed */ > if (system_uses_irq_prio_masking()) > diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c > index 4bd38ff34221..c19bf6e4969e 100644 > --- a/arch/arm64/kvm/pmu.c > +++ b/arch/arm64/kvm/pmu.c > @@ -5,7 +5,8 @@ > */ > #include > #include > -#include > + > +DEFINE_PER_CPU(struct kvm_pmu_events, kvm_pmu_events); > > /* > * Given the perf event attributes and system type, determine > @@ -25,14 +26,9 @@ static bool kvm_pmu_switch_needed(struct perf_event_attr *attr) > return (attr->exclude_host != attr->exclude_guest); > } > > -static struct kvm_pmu_events *kvm_get_pmu_events(void) > +struct kvm_pmu_events *kvm_get_pmu_events(void) Why not make this function visible in patch 1? It seems benign even though there are no other users at that moment outside of the compilation unit. -- Thanks, Oliver _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel