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 834CBC28B28 for ; Sun, 9 Mar 2025 19:18:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VvOgJFn9TkFKQJBUDZbRZ4zbDWU/v7GPOH4dGEZMqm0=; b=C7FwxtPLeBD34s6IIxxHfzCUfD QmIwrOhUGyVQzrbhFWJcqvn6NauRgtvFzWyIINVQ2llZgQ2xdAPpbAgU3JgOwGrzNSl5HmbeWZksN JsKxInG82jRXRqVdqLMXxP28JDaikJJcQc+c/sGrBoyUFjKBoqNVTcHF3Ekqque2GWdK+TjoshCFj wnG5/pzObyuFGHAvbvOJVrrUMI88m8NeOD0XZE5HTKkWj/pxzscGenBGwLCtiLEdRmJAeVJJxPuNs UAGlnnwO7C+zXopGDfnr2ea7jimyMcB+onL7p3kUs6pUthJeJ0+63zRFt0ofkzLvBcxZQfUSwI/op pH6qKGWA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1trMA8-000000012mL-31E6; Sun, 09 Mar 2025 19:18:12 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1trLtd-000000011fM-2vXE for linux-arm-kernel@lists.infradead.org; Sun, 09 Mar 2025 19:01:11 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 0076F5C0E7A; Sun, 9 Mar 2025 18:58:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 661E5C4CEE3; Sun, 9 Mar 2025 19:01:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741546868; bh=kH5JdLfYUUDkMU6DKgUeLSwXtGrR71Mgz2D16I88pkU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=nUDSBA8QYKj4ib20a3S0gzNzWPwUXQt5HPtjgFPa9psXoTr6RhK9erJQDi8sH2xAQ CJ8Nq6I6tPx27GzYGUBlIIUISD8I0Q2ORxaoLJNzGHrqZz/92qzYOyDsSlFW0MCwlX tlw0L69rvsptL+078lnlCHn725mVxmdOW1wfi/phzTe7UVzAwtDRoD+qL5wmun1rUJ GT5Bf8mH/JduCeYEkUVsuVqTQWjwa4+YRVINd7rvlfdcbtW4aTjF7r+AY5Dbgk86jO wZ8Sif6RHVVQevmi0EAleR9w8jAtgB/h/aju8GsdTfNCIpeLPcE4BENTdB9oJa27Mz fyH3qk+/qY1dQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1trLta-00ByG4-3R; Sun, 09 Mar 2025 19:01:06 +0000 Date: Sun, 09 Mar 2025 19:01:05 +0000 Message-ID: <864j02owpa.wl-maz@kernel.org> From: Marc Zyngier To: Akihiko Odaki Cc: Oliver Upton , Joey Gouly , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Andrew Jones , kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, devel@daynix.com Subject: Re: [PATCH v2 3/3] KVM: arm64: PMU: Set raw values from user to PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR} In-Reply-To: <20250307-pmc-v2-3-6c3375a5f1e4@daynix.com> References: <20250307-pmc-v2-0-6c3375a5f1e4@daynix.com> <20250307-pmc-v2-3-6c3375a5f1e4@daynix.com> 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/29.4 (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.219.108.64 X-SA-Exim-Rcpt-To: akihiko.odaki@daynix.com, oliver.upton@linux.dev, joey.gouly@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, andrew.jones@linux.dev, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, devel@daynix.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250309_120109_853240_BE640B9C X-CRM114-Status: GOOD ( 24.34 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 07 Mar 2025 10:55:30 +0000, Akihiko Odaki wrote: > > Commit a45f41d754e0 ("KVM: arm64: Add {get,set}_user for > PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}") changed KVM_SET_ONE_REG to update > the mentioned registers in a way matching with the behavior of guest > register writes. This is a breaking change of a UAPI though the new > semantics looks cleaner and VMMs are not prepared for this. > > Firecracker, QEMU, and crosvm perform migration by listing registers > with KVM_GET_REG_LIST, getting their values with KVM_GET_ONE_REG and > setting them with KVM_SET_ONE_REG. This algorithm assumes > KVM_SET_ONE_REG restores the values retrieved with KVM_GET_ONE_REG > without any alteration. However, bit operations added by the earlier > commit do not preserve the values retried with KVM_GET_ONE_REG and > potentially break migration. > > Remove the bit operations that alter the values retrieved with > KVM_GET_ONE_REG. > > Fixes: a45f41d754e0 ("KVM: arm64: Add {get,set}_user for PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR}") > Signed-off-by: Akihiko Odaki > --- > arch/arm64/kvm/sys_regs.c | 21 +-------------------- > 1 file changed, 1 insertion(+), 20 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 51054b7befc0b4bd822cecf717ee4a4740c4a685..2f44d4d4f54112787683dd75ea93fd60e92dd31f 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1142,26 +1142,7 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > > static int set_pmreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r, u64 val) > { > - bool set; > - > - val &= kvm_pmu_valid_counter_mask(vcpu); > - > - switch (r->reg) { > - case PMOVSSET_EL0: > - /* CRm[1] being set indicates a SET register, and CLR otherwise */ > - set = r->CRm & 2; > - break; > - default: > - /* Op2[0] being set indicates a SET register, and CLR otherwise */ > - set = r->Op2 & 1; > - break; > - } > - > - if (set) > - __vcpu_sys_reg(vcpu, r->reg) |= val; > - else > - __vcpu_sys_reg(vcpu, r->reg) &= ~val; > - > + __vcpu_sys_reg(vcpu, r->reg) = val & kvm_pmu_valid_counter_mask(vcpu); > kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu); > > return 0; > Yup, this code was definitely a brain fart. Thanks for spotting it. One of the big mistake was to expose both CLR and SET registers to userspace (one of the two should have been hidden). This requires a Cc to stable@vger.kernel.org so that this can be backported to anything from 6.12. It would also help if you put this patch at the head of the series, before adding the PMU request (it is then likely to be very easy to backport). With that, Acked-by: Marc Zyngier Thanks, M. -- Without deviation from the norm, progress is not possible.