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 B42B9ECAAD8 for ; Wed, 21 Sep 2022 17:48:48 +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=Z7ix3U4SRub0amHSN9n38/7SdDvaGz7xCayAisuImYY=; b=Lnvl+YThjWtr9+ MU3e8DtQNimTNsxj1pWeS/Pb5oxrwhAsvsCLLAh3m8hs/7A+BBDILI5oUeOyaY80QX0RUM1+myGU+ AvEnSUfPnyRq/y5eSMq4plaKozYf5+NqkbzhiaUcNmhQ/mzyVw7LRe6qv8NnrWhGuuRZ+4uw7Q9/u EekXDr7Z3ODBmz/I4Ai3ejEnXsjJlDazBmEX/ugYn9rwcCAqV22TdGHTDmPlbhLZsjwJ571+zx3qq NkuY8Bo/nJAMVx3SMOIn2AcLYwf8NzeQ1psDPpfWvmnkBypP+Dkr2KsHt7Vw/2jOxsaZoVoSa+yEA zcFLvaeIB6e1Gbbnmv2g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ob3op-00CEMj-HE; Wed, 21 Sep 2022 17:47:31 +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 1ob3om-00CELr-8D for linux-arm-kernel@lists.infradead.org; Wed, 21 Sep 2022 17:47:29 +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 854DEB82892; Wed, 21 Sep 2022 17:47:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2841CC433D6; Wed, 21 Sep 2022 17:47:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1663782445; bh=PA8aNMshIO9eu3SX73FrQHs+BvNP8ZcEiOhWgbPWNyU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=fO83c5LGV3Vi/Laur8vGFBAhClwTILYXi8npuCsfy0XPxyFkgAfFviJgH1q6nHx0f Bgz64sWHvwZCy6GHD+rtBxSFfUzfSkCEiKC6xKTVzmKikPCR/0FYpxbS+HFW2aGFTH hBAkK5zxZWyk3m9FObM4SCCsO8MjsJUYgwPEHVtubv/gDxW6OAGlDXyTCeW363s17Z GvZoLlANi8+SnCrdjoTbP+3Y5/0rWzXmp0o776BK/X2pqkZsUelN87l4kfKj+v8E5D s0UTvbEnP+KerT+mOD2ICK3002lAQrEJXFt90X5JdApRsOSboE+xe3w+BRi/MW02v8 VqZXNJPezr6Dw== Received: from 185-176-101-241.host.sccbroadband.ie ([185.176.101.241] helo=wait-a-minute.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 1ob3og-00BjUP-VW; Wed, 21 Sep 2022 18:47:23 +0100 Date: Wed, 21 Sep 2022 18:47:21 +0100 Message-ID: <87leqck4ba.wl-maz@kernel.org> From: Marc Zyngier To: Mark Brown Cc: Catalin Marinas , Will Deacon , Zhang Lei , James Morse , Alexandru Elisei , Andre Przywara , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 3/7] arm64/fpsimd: Have KVM explicitly say which FP registers to save In-Reply-To: References: <20220815225529.930315-1-broonie@kernel.org> <20220815225529.930315-4-broonie@kernel.org> <87wn9yj5l0.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: 185.176.101.241 X-SA-Exim-Rcpt-To: broonie@kernel.org, catalin.marinas@arm.com, will@kernel.org, zhang.lei@jp.fujitsu.com, james.morse@arm.com, alexandru.elisei@arm.com, andre.przywara@arm.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org 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-20220921_104728_628300_4E31D3D9 X-CRM114-Status: GOOD ( 40.77 ) 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 Tue, 20 Sep 2022 19:32:49 +0100, Mark Brown wrote: > > [1 ] > On Tue, Sep 20, 2022 at 06:52:59PM +0100, Marc Zyngier wrote: > > On Mon, 15 Aug 2022 23:55:25 +0100, > > Mark Brown wrote: > > > > enum fp_state { > > > + FP_STATE_TASK, /* Save based on current, invalid as fp_type */ > > > How is that related to the FP_TYPE_TASK in the commit message? What > > TYPE in the commit message should be STATE. > > > does this 'invalid as fp_type' mean? > > It means that using FP_STATE_TASK as a value for the fp_type > member of the task struck recording what type of state is > currently stored for the task is not valid, one of the other two > values representing what was actually saved must be chosen. Then this definitely represents something else, and shouldn't be a state or a type, whatever you decide to call it in the end. There is the state of the FP/SVE unit, and what some piece of SW wants to save. They match in some cases, and differ in other (the TASK value). I'd rather you encode them as them as different types to lift the ambiguity. > > > > + /* > > > + * For now we're just validating that the requested state is > > > + * consistent with what we'd otherwise work out. > > > Nit: work out? or worked out? the "we'd" doesn't help disambiguate it > > for a non-native speaker. > > we'd == we would so work out to match the tense. > > > > void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st, void *sve_state, > > > unsigned int sve_vl, void *za_state, > > > unsigned int sme_vl, u64 *svcr, > > > - enum fp_state *type) > > > + enum fp_state *type, enum fp_state to_save) > > > OK, how many discrete arguments are we going to pass to this function, > > which most of them are part the vcpu structure? It really feels like > > what you want is a getter for the per-cpu structure, and let the KVM > > code do the actual business. If this function was supposed to provide > > some level of abstraction, well, it's a fail. > > I agree that this is not an ideal interface, I am merely > following the previously chosen idiom since I haven't been able > to figure out why we were doing it in the first place and with a > lot of these things it turns out that there's some actual reason. Huh. If we're changing anything around this code, we'd better understand what we are doing... > It's not even like fpsimd_bind_task_to_cpu() has ever been > written in terms of this function, there's two parallel > implementations. My best guess was that it was some combination > of not peering at KVM internals and keeping struct > fpsimd_last_state_struct internal to fpsimd.c (since we're > effectively just passing one of those in in a more verbose form) > but never anything solid enough to be sure. Up to you, but adding extra parameters to this function really feels like the wrong thing to do. M. -- 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