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=-10.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham 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 66558C55ABD for ; Fri, 13 Nov 2020 18:49:38 +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 DA61821D7F for ; Fri, 13 Nov 2020 18:49:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="RWqErvLC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DA61821D7F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com 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-Transfer-Encoding: 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-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=UyrFUCpUKPDT4E/Be4qnIVrtl36yVN/NxCowfY91s7I=; b=RWqErvLCLMWgxGhrjPWljqJjX /c/36s5Ngx6kWaMRLedaQfeqqVp/8lO66zknonOJ/6QreyeTQ/BULL4l3UCTOKroBTDqCajBD63jK qHkHqSy6/582LI5nhE/4F6nNcXjE55u/yd9inDtuarALk02zJnRpF0TzwhzDbaAxX/HptWbhnKrLe aHSKUqvAdRa05j0JyJc79IJ7jGjLzTSvTTSmbzE4lUR6taZoEc//DDH6DkELMwjel7T+V6SGVVNPD iFu2HoAdOUHfpl3jnp70j26KqY6C92Gjrh4iECTLI/RlYZvgeygrvusI902ezrY6vERENsK86I7Vl 4X+NrU47w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kde8C-00073l-QS; Fri, 13 Nov 2020 18:49:08 +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 1kde87-00072s-98 for linux-arm-kernel@lists.infradead.org; Fri, 13 Nov 2020 18:49:06 +0000 Received: from gaia (unknown [2.26.170.190]) (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 60EEC21D7F; Fri, 13 Nov 2020 18:48:59 +0000 (UTC) Date: Fri, 13 Nov 2020 18:48:56 +0000 From: Catalin Marinas To: Mark Brown Subject: Re: [PATCH v5 1/2] arm64/sve: Don't disable SVE on syscalls return Message-ID: <20201113184855.GF3212@gaia> References: <20201106193553.22946-1-broonie@kernel.org> <20201106193553.22946-2-broonie@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201106193553.22946-2-broonie@kernel.org> 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_134903_485404_17C895C2 X-CRM114-Status: GOOD ( 29.83 ) 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: 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 Hi Mark, This code is pretty complex, so it will take some time to understand. I'd like Dave to review them as well since he was involved in the early discussions with Julien. In the meantime, I updated my TLA+ model (mechanically, I can't claim I fully understand the patches) with these changes and hasn't found a failure yet (though it takes days to complete). Current model here if anyone is interested: https://git.kernel.org/pub/scm/linux/kernel/git/cmarinas/kernel-tla.git/tree/fpsimd.tla Some questions below. On Fri, Nov 06, 2020 at 07:35:52PM +0000, Mark Brown wrote: > From: Julien Grall > > Per the syscalls ABI the state of the SVE registers is unknown after a > syscall. In practice the kernel will disable SVE and zero all the > registers but the first 128-bits of the vector on the next SVE > instruction. In workloads mixing SVE and syscalls this will result in at > least one extra entry/exit to the kernel per syscall when the SVE > registers are accessed for the first time after the syscall. > > To avoid the second entry/exit a new flag TIF_SVE_NEEDS_FLUSH is > introduced to mark a task that needs to flush the SVE context on > return to userspace. I was thinking that we can we avoid this new TIF flag altogether and just zero the registers via do_el0_svc() (or sve_user_discard()) if TIF_SVE was set on entry. We'd have to skip clearing TIF_SVE on syscall entry though. This has the potential issue of over-saving during context switch but we may be able to use an in_syscall() check. > On entry to a syscall the flag TIF_SVE will still be cleared, it will > be restored on return to userspace once the SVE state has been flushed. > This means that if a task requires to synchronize the FP state during a > syscall (e.g context switch, signal) only the FPSIMD registers will be > saved. When the task is rescheduled the SVE state will be loaded from > FPSIMD state. > > 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. 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. I'll list what I think the requirement are for the SVE state and which (not necessarily how the current code behaves): 1. We can return to user code with SVE disabled (say on first use). 2. On SVE access fault, we populate the registers either from a previously saved SVE state or just zero them. I think we currently copy the fpsimd state to sve_state just to restore the latter but this could look better. Here the sve_state is considered discarded since we enabled it in user, so it's stale. 3. On syscall entry we have to discard the hardware SVE state if access was enabled in user. 4. On context switch, we have to save the hardware state if it was not discarded (in_syscall() and maybe check TIF_SVE_ACC). If we saved it, we'd need to set TIF_SVE_STATE_VALID. 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. 5. On return to user from other exception, we restore either from FPSIMD or saved sve_state (if valid). 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). -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel