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=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 950DFC56202 for ; Fri, 13 Nov 2020 20:14:36 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 334D220691 for ; Fri, 13 Nov 2020 20:14:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="sdGbGOIe"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="VXX3WkfB" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 334D220691 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From: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=bgiMenGoehtcy4ukUCRifDndfbKVo4h2ogP7XoSOxac=; b=sdGbGOIe6AI7tzSxdRa3d9Y6Y mgYinuiaGZbfiNj7M0eqStnxfcC6ePu+EIJn8BOdRchcREciBzFpURj2aJqcTPoo2LIKgbICKLjOD NnLI/b9+Q0c4DmnmFOTO7P7uEt9DhfcMDRC0pRFm+h9lSQ+ROMoIJYOgAt6sfqPna+i8fWuY08ueP v+euXMprrybHo2G+qJ3mhWPi9/6cDQ6YFUpr7nz+s+/KBBwqOt1pCPnHeZBwCULO+2yP11zBxLX1u 0kI6J4cJJ6kEVVLh7UZcD0nXopKV8A/UPKAzwCkpgQk7q/i18+cpCt11wAe5lZZZT0eXvT6HRduV7 RNXLDFplA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kdfS8-0004L2-9L; Fri, 13 Nov 2020 20:13:48 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kdfS6-0004KR-54 for linux-arm-kernel@lists.infradead.org; Fri, 13 Nov 2020 20:13:47 +0000 Received: from localhost (fw-tnat.cambridge.arm.com [217.140.96.140]) (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 4D9B120691; Fri, 13 Nov 2020 20:13:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1605298424; bh=xJP9kNM093+SGzkYegOda/g+6FERk9ZsocrZb/o1BUs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VXX3WkfBENSBWDO90mcpA8Kf3qCp487vn61AXfrOZ4sBS9fLDvVGdMhI50v2r2dQT LHxu/WGbZ7a/DtT1mWlGqtqb8MqI0RR9T3s1CksB/Zzklwhw7KOxlZQcHdW7o0zJvP CuAnGIq4j9DqeQN4g1cJta6bTznfxhBkk6qVJKgQ= Date: Fri, 13 Nov 2020 20:13:28 +0000 From: Mark Brown To: Catalin Marinas Subject: Re: [PATCH v5 1/2] arm64/sve: Don't disable SVE on syscalls return Message-ID: <20201113201328.GG4828@sirena.org.uk> References: <20201106193553.22946-1-broonie@kernel.org> <20201106193553.22946-2-broonie@kernel.org> <20201113184855.GF3212@gaia> MIME-Version: 1.0 In-Reply-To: <20201113184855.GF3212@gaia> X-Cookie: No solicitors. User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201113_151346_440902_68009E70 X-CRM114-Status: GOOD ( 33.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Julien Grall , Zhang Lei , Julien Grall , Will Deacon , Dave Martin , linux-arm-kernel@lists.infradead.org, Daniel Kiss Content-Type: multipart/mixed; boundary="===============2932546262704732924==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============2932546262704732924== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="sLx0z+5FKKtIVDwd" Content-Disposition: inline --sLx0z+5FKKtIVDwd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 13, 2020 at 06:48:56PM +0000, Catalin Marinas wrote: > On Fri, Nov 06, 2020 at 07:35:52PM +0000, Mark Brown wrote: > > From: Julien Grall > > We could instead handle flushing the SVE state in do_el0_svc() however > > doing this reduces the potential for further optimisations such as > > initializing the SVE registers directly from the FPSIMD state when > > taking a SVE access trap and has some potential edge cases if we > > schedule before we return to userspace after do_el0_svc(). > Ah, you covered the do_el0_svc() topic here already. Is this potential > optimisation prevented because TIF_SVE has two meanings: SVE access > enabled and sve_state valid (trying to page this code in)? For example, > task_fpsimd_load() uses TIF_SVE to check whether to load from sve_state > or from fpsimd. fpsimd_bind_task_to_cpu() uses TIF_SVE to enable the > user access. Yes, there's some overloading with the storage for the SVE register file and the new flag is effectively about the storage. > Maybe that's what TIF_SVE_NEEDS_FLUSH tries to address but I'd rather > have two clearly defined flags and better separate their meaning: > - TIF_SVE_ACC - it tells us whether the user access was enabled or it > needs to be enabled on return to user (maybe we can skip this and just > enable it directly in places like do_sve_acc). > - TIF_SVE_STATE_VALID - the validity of the kernel sve_state data. That would do the trick as well, it's close to what we have now but explained slightly differently - the main difference is that you can set each flag independently at the minute since both flags mean that SVE should be enabled on return to userspace. =20 IIRC I did start down a very similar route at one point but found it was making some of the decision making code more complex with more sites needing to check multiple flags, it was a while ago but one of the things causing issues was that we don't immediately save the state on kernel entry. Looking at your suggested naming that might be a naming thing with the "where is the state?" flag that was causing issues though, thinking again we might want to use something like TIF_SVE_FULL_STATE or similar instead. > I'll list what I think the requirement are for the SVE state and which > (not necessarily how the current code behaves): This sounds about right to me. > 3. On syscall entry we have to discard the hardware SVE state if access > was enabled in user. To be a bit more explicit that's SVE state that's not shared with FPSIMD. The documented ABI doesn't *require* us to discard the extra state but it's safer and cleaner to do that than doing it only sometimes. > 5. On return to user from syscall, we just restore the FPSIMD state (I > think we still need to do something with the predicates but sve_state > is invalid, so we just initialise them). As an optimisation, we want > to leave SVE access on. The predicate registers are like the unshared bits of the Z registers and explicitly documented as being unspecified on return from syscall so the handling is the same, they're reset to ensure that we're not leaking anything. > The patch may try to address part of the above but I find that the > addition of TIF_SVE_NEEDS_FLUSH makes this state machine more > complicated than necessary (well, based on my mental model; possibly I'm > missing some other cases). Yeah. The mental model I came up with was that both flags say that we should leave SVE on for userspace and which flag is set dictates where we get the state of the Z registers from and hence where the state we need to save should go, my main focus with the last version was trying to document that I guess not 100% successfully. I found that made things simpler since it detangles them for the most part and we don't need to have a "where did I put my data?" decision at as many of the sites that look at it. I'll wait for more review but in the absence of other suggestions I'll have another run at splitting the storage and trapping flags. --sLx0z+5FKKtIVDwd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl+u6OcACgkQJNaLcl1U h9Dh4Qf/eLX8ujZzbENUGnjRIFPLqjQEo2bcg2GE4WB5BKBqcHo1MUlX7rx9Yu6J Mnsr24H0YJeNX8NC33TWWsaR7nKJdvDFTxZRj7Q9QwAg64/7vpRlwy/H1EJLSLij eHoKFC9CLR47QSSkUzfKs7aq8Mrawmd8lGHfKPDvpFyqoqAgYdCX7m+/BUKOQAxP DG6OBImBs/lAlG7aLOyM/v68KDFuCZS/VkJIuknicg99G48Rt80D6affqDk/oP65 69vksX4ibtqed1UrziHPqxqcN3tbERr9eonibE1Na3PP0+OWdJVM/TrwclF7J+et 022/K4jBAh4SaSMbNc4hN5fOs7fPKw== =gIGs -----END PGP SIGNATURE----- --sLx0z+5FKKtIVDwd-- --===============2932546262704732924== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============2932546262704732924==--