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=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,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 A5CF4C47082 for ; Tue, 8 Jun 2021 08:18:55 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 19FAB6128B for ; Tue, 8 Jun 2021 08:18:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 19FAB6128B 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 955F54A198; Tue, 8 Jun 2021 04:18:54 -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 uLp7gkBlbkry; Tue, 8 Jun 2021 04:18:53 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4480440874; Tue, 8 Jun 2021 04:18:53 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 85A35407EF for ; Tue, 8 Jun 2021 04:18: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 idCaQI3MuKRv for ; Tue, 8 Jun 2021 04:18:51 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 07E3B407ED for ; Tue, 8 Jun 2021 04:18:51 -0400 (EDT) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id E62476128B; Tue, 8 Jun 2021 08:18:49 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1lqWwh-0068ES-Sk; Tue, 08 Jun 2021 09:18:48 +0100 Date: Tue, 08 Jun 2021 09:18:46 +0100 Message-ID: <87eedczs49.wl-maz@kernel.org> From: Marc Zyngier To: "Jain, Jinank" Subject: Re: [PATCH] KVM: arm64: Properly restore PMU state during live-migration In-Reply-To: References: <20210603110554.13643-1-jinankj@amazon.de> <87wnrbylxv.wl-maz@kernel.org> <0a694ea93303bfa04530cd940f692244e1ccd1e7.camel@amazon.de> <87lf7lzl8c.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: jinankj@amazon.de, james.morse@arm.com, kvmarm@lists.cs.columbia.edu, suzuki.poulose@arm.com, linux-kernel@vger.kernel.org, alexandru.elisei@arm.com, linux-arm-kernel@lists.infradead.org, will@kernel.org, catalin.marinas@arm.com, graf@amazon.de X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: "Graf \(AWS\), Alexander" , "will@kernel.org" , "linux-kernel@vger.kernel.org" , "catalin.marinas@arm.com" , "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 Mon, 07 Jun 2021 19:34:08 +0100, "Jain, Jinank" wrote: > > Hi Marc. > > On Mon, 2021-06-07 at 17:35 +0100, Marc Zyngier wrote: > > CAUTION: This email originated from outside of the organization. Do > > not click links or open attachments unless you can confirm the sender > > and know the content is safe. > > > > > > > > On Mon, 07 Jun 2021 17:05:01 +0100, > > "Jain, Jinank" wrote: > > > On Thu, 2021-06-03 at 17:03 +0100, Marc Zyngier wrote: > > > > Hi Jinank, > > > > > > > > On Thu, 03 Jun 2021 12:05:54 +0100, > > > > Jinank Jain wrote: > > > > > Currently if a guest is live-migrated while it is actively > > > > > using > > > > > perf > > > > > counters, then after live-migrate it will notice that all > > > > > counters > > > > > would > > > > > suddenly start reporting 0s. This is due to the fact we are not > > > > > re-creating the relevant perf events inside the kernel. > > > > > > > > > > Usually on live-migration guest state is restored using > > > > > KVM_SET_ONE_REG > > > > > ioctl interface, which simply restores the value of PMU > > > > > registers > > > > > values but does not re-program the perf events so that the > > > > > guest > > > > > can seamlessly > > > > > use these counters even after live-migration like it was doing > > > > > before > > > > > live-migration. > > > > > > > > > > Instead there are two completely different code path between > > > > > guest > > > > > accessing PMU registers and VMM restoring counters on > > > > > live-migration. > > > > > > > > > > In case of KVM_SET_ONE_REG: > > > > > > > > > > kvm_arm_set_reg() > > > > > ...... kvm_arm_sys_reg_set_reg() > > > > > ........... reg_from_user() > > > > > > > > > > but in case when guest tries to access these counters: > > > > > > > > > > handle_exit() > > > > > ..... kvm_handle_sys_reg() > > > > > ..........perform_access() > > > > > ...............access_pmu_evcntr() > > > > > ...................kvm_pmu_set_counter_value() > > > > > .......................kvm_pmu_create_perf_event() > > > > > > > > > > The drawback of using the KVM_SET_ONE_REG interface is that the > > > > > host pmu > > > > > events which were registered for the source instance and not > > > > > present for > > > > > the destination instance. > > > > > > > > I can't parse this sentence. Do you mean "are not present"? > > > > > > > > > Thus passively restoring PMCR_EL0 using > > > > > KVM_SET_ONE_REG interface would not create the necessary host > > > > > pmu > > > > > events > > > > > which are crucial for seamless guest experience across live > > > > > migration. > > > > > > > > > > In ordet to fix the situation, on first vcpu load we should > > > > > restore > > > > > PMCR_EL0 in the same exact way like the guest was trying to > > > > > access > > > > > these counters. And then we will also recreate the relevant > > > > > host > > > > > pmu > > > > > events. > > > > > > > > > > Signed-off-by: Jinank Jain > > > > > Cc: Alexander Graf (AWS) > > > > > Cc: Marc Zyngier > > > > > Cc: James Morse > > > > > Cc: Alexandru Elisei > > > > > Cc: Suzuki K Poulose > > > > > Cc: Catalin Marinas > > > > > Cc: Will Deacon > > > > > --- > > > > > arch/arm64/include/asm/kvm_host.h | 1 + > > > > > arch/arm64/kvm/arm.c | 1 + > > > > > arch/arm64/kvm/pmu-emul.c | 10 ++++++++-- > > > > > arch/arm64/kvm/pmu.c | 15 +++++++++++++++ > > > > > include/kvm/arm_pmu.h | 3 +++ > > > > > 5 files changed, 28 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/arch/arm64/include/asm/kvm_host.h > > > > > b/arch/arm64/include/asm/kvm_host.h > > > > > index 7cd7d5c8c4bc..2376ad3c2fc2 100644 > > > > > --- a/arch/arm64/include/asm/kvm_host.h > > > > > +++ b/arch/arm64/include/asm/kvm_host.h > > > > > @@ -745,6 +745,7 @@ static inline int > > > > > kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu) > > > > > void kvm_set_pmu_events(u32 set, struct perf_event_attr > > > > > *attr); > > > > > void kvm_clr_pmu_events(u32 clr); > > > > > > > > > > +void kvm_vcpu_pmu_restore(struct kvm_vcpu *vcpu); > > > > > 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 e720148232a0..c66f6d16ec06 100644 > > > > > --- a/arch/arm64/kvm/arm.c > > > > > +++ b/arch/arm64/kvm/arm.c > > > > > @@ -408,6 +408,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu > > > > > *vcpu, > > > > > int cpu) > > > > > if (has_vhe()) > > > > > kvm_vcpu_load_sysregs_vhe(vcpu); > > > > > kvm_arch_vcpu_load_fp(vcpu); > > > > > + kvm_vcpu_pmu_restore(vcpu); > > > > > > > > If this only needs to be run once per vcpu, why not trigger it > > > > from > > > > kvm_arm_pmu_v3_enable(), which is also called once per vcpu? > > > > > > > > This can done on the back of a request, saving most of the > > > > overhead > > > > and not requiring any extra field. Essentially, something like > > > > the > > > > (untested) patch below. > > > > > > > > > kvm_vcpu_pmu_restore_guest(vcpu); > > > > > if (kvm_arm_is_pvtime_enabled(&vcpu->arch)) > > > > > kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu); > > > > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu- > > > > > emul.c > > > > > index fd167d4f4215..12a40f4b5f0d 100644 > > > > > --- a/arch/arm64/kvm/pmu-emul.c > > > > > +++ b/arch/arm64/kvm/pmu-emul.c > > > > > @@ -574,10 +574,16 @@ void kvm_pmu_handle_pmcr(struct kvm_vcpu > > > > > *vcpu, u64 val) > > > > > kvm_pmu_disable_counter_mask(vcpu, mask); > > > > > } > > > > > > > > > > - if (val & ARMV8_PMU_PMCR_C) > > > > > + /* > > > > > + * Cycle counter needs to reset in case of first vcpu > > > > > load. > > > > > + */ > > > > > + if (val & ARMV8_PMU_PMCR_C || > > > > > !kvm_arm_pmu_v3_restored(vcpu)) > > > > > > > > Why? There is no architectural guarantee that a counter resets to > > > > 0 > > > > without writing PMCR_EL0.C. And if you want the guest to continue > > > > counting where it left off, resetting the counter is at best > > > > counter-productive. > > > > > > Without this we would not be resetting PMU which is required for > > > creating host perf events. With the patch that you suggested we are > > > restoring PMCR_EL0 properly but still missing recreation of host > > > perf > > > events. > > > > How? The request that gets set on the first vcpu run will call > > kvm_pmu_handle_pmcr() -> kvm_pmu_enable_counter_mask() -> > > kvm_pmu_create_perf_event(). What are we missing? > > > > I found out what I was missing. I was working with an older kernel > which was missing this upstream patch: > > https://lore.kernel.org/lkml/20200124142535.29386-3-eric.auger@redhat.com/ :-( Please test whatever you send with an upstream kernel. Actually, please *develop* on an upstream kernel. This will avoid this kind of discussion where we talk past each other, and make it plain that your production kernel is lacking all sorts of fixes. Now, can you please state whether or not this patch fixes it for you *on an upstream kernel*? I have no interest in results from a production kernel. M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm