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 78DF6C004D4 for ; Sat, 21 Jan 2023 12:03: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: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc: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=AVCONVWJ9MwgL+/6VAwE3m7LRfT94ES2SIWfcaRfxqQ=; b=Faa9jzEnVNhoym Llj5ZRU5h5Gqv6iBlBJIQMiJ4q6Za6cUmwvDzKNIdlSMsoE6FjXkWH7/vMV4AYgzXUgrhVF3THOtd icBYIFx/zmYHH/Lw937o3v7CJzwSgrnbupFOs5sK7j4WtqXouiej7GBsqknG0uZKNjjzjEriNiB3T e+zV70RV0Cyf0oVAMbTL6KWS+qphME8RcW8p3Lqju8AU++37szDFbcKryojtnbgoEQO9cJWnRrv+r LnLPIQbr8wSLiCEHdDsQfANaMymw5cyGJ1zJY85003NK508BEarSFqVZiK+mIzzK99srBU+8W7VG+ 6sePMGxtPZEQBD0LEM3Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pJCZf-00DoK4-Ew; Sat, 21 Jan 2023 12:02:19 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pJCZb-00DoJ9-LX for linux-arm-kernel@lists.infradead.org; Sat, 21 Jan 2023 12:02:17 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 0AA4FB80123; Sat, 21 Jan 2023 12:02:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A40FEC4339B; Sat, 21 Jan 2023 12:02:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1674302526; bh=oYtg75ka9VvkbzbkuCBtvmOJl2dncX28jjo0X1mZ/H8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=XqWtOPUpTRni7OvcYGsxDhJJK09DEUcvQtbAYQe9PNTKtS6elkHPTS06OTTGds6XP WFy1V2d2fc9MVlQYa817/88emTdu1VLLRt8QxmNQ4SPKdtXj5xnklZRazsPSlZKOVs jOWav8X4qFHdu7MBpDfk4sneNsJzeAxruPUj3PxT9EPE+wmK7Fmb7T8hriNK1mxrdc SIoA3FEghqCuzDuavNtT4MTYWpH7EVUdzw/WVnQGZ1oFVbe7pFED52Q9pGyiI30Op/ Z6MRoaXpWdXl+isoDtE2M22ujd4frYehlVZcJQ0Y/JEIN0wypg7AzAgrAitDxo8Fx3 tHUQDnvDr/CvQ== 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 1pJCZQ-003d6M-2q; Sat, 21 Jan 2023 12:02:04 +0000 Date: Sat, 21 Jan 2023 12:02:03 +0000 Message-ID: <86k01gm6ys.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton , Akihiko Odaki Cc: Mark Brown , linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Mathieu Poirier , Suzuki K Poulose , Alexandru Elisei , James Morse , Will Deacon , Catalin Marinas , asahi@lists.linux.dev, Alyssa Rosenzweig , Sven Peter , Hector Martin Subject: Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration In-Reply-To: References: <20230112023852.42012-1-akihiko.odaki@daynix.com> <20230112023852.42012-8-akihiko.odaki@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/28.2 (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.219.108.64 X-SA-Exim-Rcpt-To: oliver.upton@linux.dev, akihiko.odaki@daynix.com, broonie@kernel.org, linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, mathieu.poirier@linaro.org, suzuki.poulose@arm.com, alexandru.elisei@arm.com, james.morse@arm.com, will@kernel.org, catalin.marinas@arm.com, asahi@lists.linux.dev, alyssa@rosenzweig.io, sven@svenpeter.dev, marcan@marcan.st 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-20230121_040216_042839_0D988767 X-CRM114-Status: GOOD ( 35.44 ) 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 Thu, 19 Jan 2023 19:46:16 +0000, Oliver Upton wrote: > > Hi Akihiko, > > On Thu, Jan 12, 2023 at 11:38:52AM +0900, Akihiko Odaki wrote: > > Before this change, the cache configuration of the physical CPU was > > exposed to vcpus. This is problematic because the cache configuration a > > vcpu sees varies when it migrates between vcpus with different cache > > configurations. > > > > Fabricate cache configuration from the sanitized value, which holds the > > CTR_EL0 value the userspace sees regardless of which physical CPU it > > resides on. > > > > CLIDR_EL1 and CCSIDR_EL1 are now writable from the userspace so that > > the VMM can restore the values saved with the old kernel. > > > > Suggested-by: Marc Zyngier > > Signed-off-by: Akihiko Odaki > > I needed to squash in the patch below to get all of this working. > Writing back the value read for a given cache level was failing, which I > caught with the get-reg-list selftest. > > Pushed the result here if you want to have a look: > > https://github.com/oupton/linux/tree/kvm-arm64/virtual-cache-geometry > > -- > Thanks, > Oliver > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 459e6d358dab..b6228f7d1d8d 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -148,17 +148,19 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr) > > static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val) > { > - u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val); > + u8 line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, val); > + u32 cur = get_ccsidr(vcpu, csselr); > + u8 min_line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, cur); > u32 *ccsidr = vcpu->arch.ccsidr; > u32 i; > > - if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr)) > + if (cur == val) > + return 0; > + > + if ((val & CCSIDR_EL1_RES0) || line_size < min_line_size) > return -EINVAL; This doesn't look right. You're comparing the value userspace is trying to set for a given level with the value that is already set for that level, and forbid the cache line size to be smaller. It works if no value has been set yet (you fallback to something derived from CTR_EL0), but this fails if userspace does multiple writes. The original check is against CTR_EL0, which makes absolute sense because we want to check across the whole hierarchy. It is just that the original code has two bugs: - It fails to convert the CCSIDR_EL1.LineSize value to a number of words (the missing +4). Admire how the architecture is actively designed to be hostile to SW by providing two different formats for the cache line size, none of which is in... bytes. - It passes the full CSSELR value to get_min_cache_line_size(), while this function wants a bool... Yes, there are times where you'd want a stronger type system (did anyone say Rust? ;-) I propose that we fold something like the patch below in instead (tested with get-reg-list). Thanks, M. diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 3b3024c42e61..ac943dcb4610 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -148,11 +148,12 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr) static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val) { - u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val); + u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val) + 4; u32 *ccsidr = vcpu->arch.ccsidr; u32 i; - if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr)) + if ((val & CCSIDR_EL1_RES0) || + line_size < get_min_cache_line_size(csselr & CSSELR_EL1_InD)) return -EINVAL; if (!ccsidr) { -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel