[{"id":1767931,"web_url":"http://patchwork.ozlabs.org/comment/1767931/","msgid":"<20170913143325.hi4cfcajbts3bbao@localhost>","list_archive_url":null,"date":"2017-09-13T14:33:25","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> +/*\n> + * Handle SVE state across fork():\n> + *\n> + * dst and src must not end up with aliases of the same sve_state.\n> + * Because a task cannot fork except in a syscall, we can discard SVE\n> + * state for dst here, so long as we take care to retain the FPSIMD\n> + * subset of the state if SVE is in use. Reallocation of the SVE state\n> + * will be deferred until dst tries to use SVE.\n> + */\n> +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> +{\n> +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> +\t\tsve_to_fpsimd(dst);\n> +\t}\n> +\n> +\tdst->thread.sve_state = NULL;\n> +}\n\nI first thought the thread flags are not visible in dst yet since\ndup_task_struct() calls arch_dup_task_struct() before\nsetup_thread_stack(). However, at the end of the last year we enabled\nCONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\non this.\n\nAnyway, IIUC we don't need sve_to_fpsimd() here. The\narch_dup_task_struct() already called fpsimd_preserve_current_state()\nfor src, so the FPSIMD state (which we care about) is transferred during\nthe *dst = *src assignment. So you'd only need the last statement,\npossibly with a different function name like fpsimd_erase_sve (and maybe\nmake the function static inline in the header).\n\n[...]\n> int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)\n> @@ -246,6 +247,9 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)\n> \tif (current->mm)\n> \t\tfpsimd_preserve_current_state();\n> \t*dst = *src;\n> +\n> +\tfpsimd_dup_sve(dst, src);\n> +\n> \treturn 0;\n> }","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"Lu3vbujj\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xskgY3pFKz9sNr\n\tfor ;\n\tThu, 14 Sep 2017 00:34:01 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1ds8jl-0007Zy-9J; Wed, 13 Sep 2017 14:33:57 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1ds8jd-0007Y1-FI for linux-arm-kernel@lists.infradead.org;\n\tWed, 13 Sep 2017 14:33:55 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A8D8C1529;\n\tWed, 13 Sep 2017 07:33:28 -0700 (PDT)","from localhost (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t931AC3F578; Wed, 13 Sep 2017 07:33:28 -0700 (PDT)","from cmarinas by localhost with local (Exim 4.89)\n\t(envelope-from )\n\tid 1ds8jG-0006vk-6O; Wed, 13 Sep 2017 07:33:26 -0700"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=vF3uZoWQTonbIGij718Z99oc8F9qonJzRf8Fzfk6LeQ=;\n\tb=Lu3vbujjE3vgTr\n\t+94FayrFyYykrAIxQxGLHksGaTcxagbj0ZpfAp9gprrWaP7FqXOfN7nQQGap2RIA4fbSOfcY02m37\n\tmBx8ljqi++JZImmPHEvCfSg2EcF1NCuTXjtJ7CqER9RNbBT9bkw105aAmIZunatdeP5jUqUEt9tBU\n\tZP+7keB5FKSjGow8oPH2XQlA9AsTToZfEm+TRLB8/0uNTs4+42yRvRJ9JvT1lnk9LjfYiSW+s5Lvg\n\tEgmiyw7hN43hMM9gQO1PtCUuBEN6SRd/N04/P1c420qfPTj96ropyhWOxdwo3QwgCNty/ZJhy4ZVc\n\tRtx36aQzepneQkp0fZXw==;","Date":"Wed, 13 Sep 2017 07:33:25 -0700","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170913143325.hi4cfcajbts3bbao@localhost>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170913_073349_529499_37A9F154 ","X-CRM114-Status":"GOOD ( 13.02 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768049,"web_url":"http://patchwork.ozlabs.org/comment/1768049/","msgid":"<20170913172605.hjrwjf34kyy7coh4@localhost>","list_archive_url":null,"date":"2017-09-13T17:26:05","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> +el0_sve_acc:\n> +\t/*\n> +\t * Scalable Vector Extension access\n> +\t */\n> +\tenable_dbg\n> +\tct_user_exit\n> +\tmov\tx0, x25\n> +\tmov\tx1, sp\n> +\tbl\tdo_sve_acc\n> +\tb\tret_to_user\n\nI think do_sve_acc() runs with interrupts disabled. We may have some\nhigh latency for large SVE states.\n\n> +/*\n> + * Trapped SVE access\n> + */\n> +void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> +{\n> +\t/* Even if we chose not to use SVE, the hardware could still trap: */\n> +\tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> +\t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> +\t\treturn;\n> +\t}\n> +\n> +\ttask_fpsimd_save();\n> +\n> +\tsve_alloc(current);\n> +\tfpsimd_to_sve(current);\n> +\tif (test_and_set_thread_flag(TIF_SVE))\n> +\t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> +\n> +\ttask_fpsimd_load();\n> +}\n\nWhen this function is entered, do we expect TIF_SVE to always be\ncleared? It's worth adding a comment on the expected conditions. If\nthat's the case, task_fpsimd_save() would only save the FPSIMD state\nwhich is fine. However, you subsequently transfer the FPSIMD state to\nSVE, set TIF_SVE and restore the full SVE state. If we don't care about\nthe SVE state here, can we call task_fpsimd_load() *before* setting\nTIF_SVE?\n\nI may as well have confused myself with the state bouncing between\nFPSIMD and SVE (more reasons to document the data flow better ;)).","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"YWj+0b7g\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xspVf5spbz9sNw\n\tfor ;\n\tThu, 14 Sep 2017 03:26:34 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsBQm-0007sQ-6i; Wed, 13 Sep 2017 17:26:32 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsBQj-0007r1-Ir for linux-arm-kernel@lists.infradead.org;\n\tWed, 13 Sep 2017 17:26:31 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4A19F1435;\n\tWed, 13 Sep 2017 10:26:09 -0700 (PDT)","from localhost (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t363833F483; Wed, 13 Sep 2017 10:26:09 -0700 (PDT)","from cmarinas by localhost with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dsBQL-0008UT-KS; Wed, 13 Sep 2017 10:26:05 -0700"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=/XhjnHk32+zw6lEAbqjWMFX4+Dqkxdu2nbHvr0ZakX0=;\n\tb=YWj+0b7gMrDtRO\n\twxYYEYjzWYSX769WOa+9St/okdZ4PCxcJsNbnFcU8uoC+DwJLQOqYtk2QDPAi32uzwTxLV/pktWGv\n\tvmLsmwpKK4RXrJ3J7fPBixg+zsAerVoChz1xsFO1EUVRL5g7JdebRBKgUG+PRrrAFP2e2heIVwkTZ\n\t/gPTnZl+519ZJafRUbTrXFggDphsv8SdXqu1R52KO9PwiBFt6/5+y+aWskRKIg0jw1GVQio9VO30o\n\tuLfieESw3FVjBWYszGE1lS48qF/CyjnWfcSeQG3M0QTU7cy5ixhcVbyxGkMsXVaFGYbLl6MxjBRu+\n\th+if712Mnv/xIG3NH/8Q==;","Date":"Wed, 13 Sep 2017 10:26:05 -0700","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170913172605.hjrwjf34kyy7coh4@localhost>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170913_102629_633065_74F795E7 ","X-CRM114-Status":"GOOD ( 10.75 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768096,"web_url":"http://patchwork.ozlabs.org/comment/1768096/","msgid":"<20170913191707.GD23415@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-09-13T19:17:07","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Wed, Sep 13, 2017 at 10:26:05AM -0700, Catalin Marinas wrote:\n> On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > +el0_sve_acc:\n> > +\t/*\n> > +\t * Scalable Vector Extension access\n> > +\t */\n> > +\tenable_dbg\n> > +\tct_user_exit\n> > +\tmov\tx0, x25\n> > +\tmov\tx1, sp\n> > +\tbl\tdo_sve_acc\n> > +\tb\tret_to_user\n> \n> I think do_sve_acc() runs with interrupts disabled. We may have some\n> high latency for large SVE states.\n\nHistorically I wanted to play safe.\n\nI meant to change this to enable_dbg_and_irq now, but it looks like I\nforgot to do it.\n\nI'll double-check that do_sve_acc() does nothing that makes it unsafe to\nenable irqs, but it should be OK. It's certainly _intended_ to be\nirq-safe.\n\n> > +/*\n> > + * Trapped SVE access\n> > + */\n> > +void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> > +{\n> > +\t/* Even if we chose not to use SVE, the hardware could still trap: */\n> > +\tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> > +\t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\ttask_fpsimd_save();\n> > +\n> > +\tsve_alloc(current);\n> > +\tfpsimd_to_sve(current);\n> > +\tif (test_and_set_thread_flag(TIF_SVE))\n> > +\t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> > +\n> > +\ttask_fpsimd_load();\n> > +}\n> \n> When this function is entered, do we expect TIF_SVE to always be\n> cleared? It's worth adding a comment on the expected conditions. If\n\nYes, and this is required for correctness, as you observe.\n\nI had a BUG_ON() here which I removed, but it makes sense to add a\ncomment to capture the precondition here, and how it is satisfied.\n\n> that's the case, task_fpsimd_save() would only save the FPSIMD state\n> which is fine. However, you subsequently transfer the FPSIMD state to\n> SVE, set TIF_SVE and restore the full SVE state. If we don't care about\n> the SVE state here, can we call task_fpsimd_load() *before* setting\n> TIF_SVE?\n\nThere should be no way to reach this code with TIF_SVE set, unless\ntask_fpsimd_load() sets the CPACR trap bit wrongly, or the hardware is\nbroken -- either of which is a bug.\n\n> I may as well have confused myself with the state bouncing between\n> FPSIMD and SVE (more reasons to document the data flow better ;)).\n\nAgreed, I think this does need more explanation... I'm currently\ntrying to figure out the best way to describe it.\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"pSTFOkMU\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsryr3qTnz9ryQ\n\tfor ;\n\tThu, 14 Sep 2017 05:17:40 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsDAG-0002ay-N9; Wed, 13 Sep 2017 19:17:36 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsDAC-0002YU-EG for linux-arm-kernel@lists.infradead.org;\n\tWed, 13 Sep 2017 19:17:33 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0AE1A1529;\n\tWed, 13 Sep 2017 12:17:12 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t3AA103F483; Wed, 13 Sep 2017 12:17:10 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=aBwLCN6xlFOIlLUe+KBmg1bIeb+fT5eFR3akHhCiyBc=;\n\tb=pSTFOkMU1NR9js\n\tlgB+UrYq61pmxpDcliSbkdKMkAwIcY3tIZfpVurpFyvy5RdkhEDplNznRK+JXsRFEZJYfCL9d4KIS\n\t2yv+GhCzIOCaUTbFzqoFEMzN2aZSBZUdGVWTZ8VjwLLGiayK60swqpwZ0A2Y7qti26KQRBFBSwStb\n\tp4kWxiJC4rcYGM7axS6GEWEauW2D+O32L0pqXqPi2AZovMWaZZqtMbVRIGv3i/XJhwrtcLsQzxswk\n\t31/BxPlQmJLvcekctjtHlM2i4VIFRR1KEvWCCN8gydoFeR0yYv7WIvoAetcsbmi9+mWwcAZweyytA\n\t4vY4mRPqg9bVaioedSyw==;","Date":"Wed, 13 Sep 2017 20:17:07 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170913191707.GD23415@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913172605.hjrwjf34kyy7coh4@localhost>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913172605.hjrwjf34kyy7coh4@localhost>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170913_121732_494715_A05A3808 ","X-CRM114-Status":"GOOD ( 19.02 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768225,"web_url":"http://patchwork.ozlabs.org/comment/1768225/","msgid":"<20170913222129.vi3lidawdp33zzjd@localhost>","list_archive_url":null,"date":"2017-09-13T22:21:29","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Wed, Sep 13, 2017 at 08:17:07PM +0100, Dave P Martin wrote:\n> On Wed, Sep 13, 2017 at 10:26:05AM -0700, Catalin Marinas wrote:\n> > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > +/*\n> > > + * Trapped SVE access\n> > > + */\n> > > +void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> > > +{\n> > > +\t/* Even if we chose not to use SVE, the hardware could still trap: */\n> > > +\tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> > > +\t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > +\ttask_fpsimd_save();\n> > > +\n> > > +\tsve_alloc(current);\n> > > +\tfpsimd_to_sve(current);\n> > > +\tif (test_and_set_thread_flag(TIF_SVE))\n> > > +\t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> > > +\n> > > +\ttask_fpsimd_load();\n> > > +}\n> > \n> > When this function is entered, do we expect TIF_SVE to always be\n> > cleared? It's worth adding a comment on the expected conditions. If\n> \n> Yes, and this is required for correctness, as you observe.\n> \n> I had a BUG_ON() here which I removed, but it makes sense to add a\n> comment to capture the precondition here, and how it is satisfied.\n> \n> > that's the case, task_fpsimd_save() would only save the FPSIMD state\n> > which is fine. However, you subsequently transfer the FPSIMD state to\n> > SVE, set TIF_SVE and restore the full SVE state. If we don't care about\n> > the SVE state here, can we call task_fpsimd_load() *before* setting\n> > TIF_SVE?\n> \n> There should be no way to reach this code with TIF_SVE set, unless\n> task_fpsimd_load() sets the CPACR trap bit wrongly, or the hardware is\n> broken -- either of which is a bug.\n\nThanks for confirming my assumptions. What I meant was rewriting the\nabove function as:\n\n\t/* reset the SVE state (other than FPSIMD) */\n\ttask_fpsimd_save();\n\ttask_fpsimd_load();\n\n\tsve_alloc(current);\n\tset_thread_flag(TIF_SVE);","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"k0zUJdtN\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsx3X3MnRz9sxR\n\tfor ;\n\tThu, 14 Sep 2017 08:22:00 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsG2e-0000lq-UK; Wed, 13 Sep 2017 22:21:56 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsG2a-0000ko-IK for linux-arm-kernel@lists.infradead.org;\n\tWed, 13 Sep 2017 22:21:54 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 500C41529;\n\tWed, 13 Sep 2017 15:21:32 -0700 (PDT)","from localhost (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t3D4413F578; Wed, 13 Sep 2017 15:21:32 -0700 (PDT)","from cmarinas by localhost with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dsG2E-00030W-6C; Wed, 13 Sep 2017 15:21:30 -0700"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=3C+CzFYtDHHUvqEFzO+35gD1OML3vauRT+SX05hayVE=;\n\tb=k0zUJdtN014p7y\n\tdyX6c7ykIf39xwtHWzzrUkbPcGiqVB/Q+TodDm2KV19UBRfdcnz3/vzw2NjoYv+fzJ+mqIY5TZJOa\n\t4h6IcX8U1TG+rS9ly7iCp3GYO+u2A2SLatGfzQAuvMF+DuqUCJFLJReGoq7C33E35Z6ZmjLUk9cq/\n\tba7wRz+XrwpXE07yjj2WyhyFWgQleqfw77d7yEu9Ba2z2aY3Hdiwq9IUmTQiGzsGtS+uxUbGk2y+V\n\txuB0vEvVSTBHyp4JO4dqv+VGabXvIZmoNm+gAo4iWRktxeitUxL36HydmwNr+bAeT06XB2YeDuaIu\n\tDljhUuNrvVUR+56tvNyw==;","Date":"Wed, 13 Sep 2017 15:21:29 -0700","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170913222129.vi3lidawdp33zzjd@localhost>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913172605.hjrwjf34kyy7coh4@localhost>\n\t<20170913191707.GD23415@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913191707.GD23415@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170913_152152_625216_70505772 ","X-CRM114-Status":"GOOD ( 18.01 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768800,"web_url":"http://patchwork.ozlabs.org/comment/1768800/","msgid":"<20170914193944.GA24231@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-09-14T19:40:41","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Wed, Sep 13, 2017 at 03:21:29PM -0700, Catalin Marinas wrote:\n> On Wed, Sep 13, 2017 at 08:17:07PM +0100, Dave P Martin wrote:\n> > On Wed, Sep 13, 2017 at 10:26:05AM -0700, Catalin Marinas wrote:\n> > > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > > +/*\n> > > > + * Trapped SVE access\n> > > > + */\n> > > > +void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> > > > +{\n> > > > +\t/* Even if we chose not to use SVE, the hardware could still trap: */\n> > > > +\tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> > > > +\t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> > > > +\t\treturn;\n> > > > +\t}\n> > > > +\n> > > > +\ttask_fpsimd_save();\n> > > > +\n> > > > +\tsve_alloc(current);\n> > > > +\tfpsimd_to_sve(current);\n> > > > +\tif (test_and_set_thread_flag(TIF_SVE))\n> > > > +\t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> > > > +\n> > > > +\ttask_fpsimd_load();\n> > > > +}\n> > > \n> > > When this function is entered, do we expect TIF_SVE to always be\n> > > cleared? It's worth adding a comment on the expected conditions. If\n> > \n> > Yes, and this is required for correctness, as you observe.\n> > \n> > I had a BUG_ON() here which I removed, but it makes sense to add a\n> > comment to capture the precondition here, and how it is satisfied.\n> > \n> > > that's the case, task_fpsimd_save() would only save the FPSIMD state\n> > > which is fine. However, you subsequently transfer the FPSIMD state to\n> > > SVE, set TIF_SVE and restore the full SVE state. If we don't care about\n> > > the SVE state here, can we call task_fpsimd_load() *before* setting\n> > > TIF_SVE?\n> > \n> > There should be no way to reach this code with TIF_SVE set, unless\n> > task_fpsimd_load() sets the CPACR trap bit wrongly, or the hardware is\n> > broken -- either of which is a bug.\n> \n> Thanks for confirming my assumptions. What I meant was rewriting the\n> above function as:\n> \n> \t/* reset the SVE state (other than FPSIMD) */\n> \ttask_fpsimd_save();\n> \ttask_fpsimd_load();\n\nI think this works, but can you explain your rationale?\n\nI think the main effect of your suggestion is that it is cheaper, due\nto eliminating some unnecessary load/store operations.\n\nWe could go one better, and do\n\n\tmov\tv0.16b, v0.16b\n\tmov\tv1.16b, v1.16b\n\t// ...\n\tmov\tv31.16b, v31.16b\n\nwhich doesn't require any memory access.\n\nBut I still prefer to zero p0..p15, ffr for cleanliness, even though\nthe SVE programmer's model doesn't require this (unlike for the Z-reg\nhigh bits where we do need to zero them in order not to violate the\nprogrammer's model).\n\nCurrently sve_alloc()+task_fpsimd_load() ensures that all the non-FPSIMD\nregs are zeroed too, in addition to the Z-reg high bits.\n\nSo we might want a special-purpose helper -- if so, we can do it all\nwith no memory access.\n\n\tpfalse\tp0.b\n\t// ..\n\tpfalse\tp15.b\n\twrffr\tp0.b\n\nThis would allow the memset-zero an sve_alloc() to be removed, but I\nwould need to check what other code is relying on it.\n\nI guess I hadn't done this because I viewed it as an optimisation.\n\nThoughts?\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"Z559bKiu\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtTRj1mpzz9s3T\n\tfor ;\n\tFri, 15 Sep 2017 05:41:21 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsa0k-0005Kv-7M; Thu, 14 Sep 2017 19:41:18 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsa0f-0005Eq-Og for linux-arm-kernel@lists.infradead.org;\n\tThu, 14 Sep 2017 19:41:15 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A8CC11435;\n\tThu, 14 Sep 2017 12:40:51 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tD7F6C3F58C; Thu, 14 Sep 2017 12:40:49 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=alBdcIxirbvY1Gi/7ZdtSA4TwAfM3vii/sVeiHi5VO8=;\n\tb=Z559bKiuxILtd0\n\tzf6VBqtPCq1x/tXfo4be3ZuhP//B2H8Xyn1aRknxT9P/21l448SrS/Wt1zD4O2jafmgSRWfVBBx2u\n\tjV9vWBnCWBLkUJTA4+CC5zuLhnIjcksJT7MJfC1aIkZZK0hQEZ2esxNAdt39bUsjpClZ/qW8nzEMs\n\tD8ZLd0+A+AX4YzNQdslZPGS8G+v6x02wrRthIcO8Ht18cGZyxfA7EnRlpugSajXU69qRqNX624zbt\n\tT3zwJULnSMoFVtCbJiJsXd/3Q/NDNBBzRR5Zl9cnfkUg88G9jX+umxXdT/roV/auOXSU2dMoNLMqW\n\tocW8Cpd6WoqD7kYOerPw==;","Date":"Thu, 14 Sep 2017 20:40:41 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170914193944.GA24231@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913172605.hjrwjf34kyy7coh4@localhost>\n\t<20170913191707.GD23415@e103592.cambridge.arm.com>\n\t<20170913222129.vi3lidawdp33zzjd@localhost>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913222129.vi3lidawdp33zzjd@localhost>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170914_124113_883845_D9FE28F2 ","X-CRM114-Status":"GOOD ( 24.48 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1768808,"web_url":"http://patchwork.ozlabs.org/comment/1768808/","msgid":"<20170914195555.GB24231@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-09-14T19:55:56","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > +/*\n> > + * Handle SVE state across fork():\n> > + *\n> > + * dst and src must not end up with aliases of the same sve_state.\n> > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > + * state for dst here, so long as we take care to retain the FPSIMD\n> > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > + * will be deferred until dst tries to use SVE.\n> > + */\n> > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > +{\n> > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > +\t\tsve_to_fpsimd(dst);\n> > +\t}\n> > +\n> > +\tdst->thread.sve_state = NULL;\n> > +}\n> \n> I first thought the thread flags are not visible in dst yet since\n> dup_task_struct() calls arch_dup_task_struct() before\n> setup_thread_stack(). However, at the end of the last year we enabled\n> CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> on this.\n\nHmmm, I see your point, but there are some sequencing issues here.\n\n> Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> arch_dup_task_struct() already called fpsimd_preserve_current_state()\n\nI consider SVE discard as an optional side effect of task_fpsimd_save(),\nnot something that is guaranteed to happen -- the decision about whether\nto do so may become more intelligent later on. So, for src, we may\ndiscard SVE (because syscall), but for dst we must NULL .sve_state (and\ntherefore clear TIF_SVE) simply to avoid aliasing of src->sve_state and\ndst->sve_state.\n\nThe latter requires operating on the thread_flags of dst. I'll need to\ncheck whether there's another suitable hook for updating the thread flags\nof dst, if we aren't confident that they will always have been\ninitialised by the time arch_dup_task_struct() is called.\n\n\nEither way, there would be an intra-thread ordering requirement between\nthe task_struct and thread_info updates here, _if_ dst were schedulable:\ndst:TIF_SVE must be cleared before dst->sve_state is NULLed.\n\ndst is not schedulable until fork is done though, so maybe this doesn't\nreally matter...\n\n> for src, so the FPSIMD state (which we care about) is transferred during\n> the *dst = *src assignment. So you'd only need the last statement,\n> possibly with a different function name like fpsimd_erase_sve (and maybe\n> make the function static inline in the header).\n\nNot quite: TIF_SVE must be cleared so that a context switch or\nkernel_neon_begin() after dst is scheduled doesn't try to save state in\nthe (NULL) dst->sve_state.\n\n> \n> [...]\n> > int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)\n> > @@ -246,6 +247,9 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)\n> > \tif (current->mm)\n> > \t\tfpsimd_preserve_current_state();\n> > \t*dst = *src;\n> > +\n> > +\tfpsimd_dup_sve(dst, src);\n> > +\n> > \treturn 0;\n> > }\n\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"VAY4PuiS\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtTn90zL6z9s7f\n\tfor ;\n\tFri, 15 Sep 2017 05:56:29 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsaFN-0003ko-Ir; Thu, 14 Sep 2017 19:56:25 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsaFI-0003hv-PG for linux-arm-kernel@lists.infradead.org;\n\tThu, 14 Sep 2017 19:56:23 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7C1001435;\n\tThu, 14 Sep 2017 12:56:00 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tABB3E3F58C; Thu, 14 Sep 2017 12:55:58 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=cnCITAiSSf6bFjGZxhW21aqe5syzntcNjQzrMKej8Jk=;\n\tb=VAY4PuiSsrDBdn\n\tgCaAD0DwRObZsgkjYPuFIsa2yA0MM8ER+qeOoepW1iC8kBvuGL+exNqHRPOJfM8Er+Vy/zoX16Ov+\n\tWf1GFt5wgubJIqkCjGNp4j8Ddb/AvvDnc6Nac8HmpYiFZ7UgIiTDXSLn+kQ7yyjt4aQ70L7z3bWfF\n\tyCBCoZPjBlEBaURCY2P51TxQKF9Mk9hjdw5N4iICIUktUUvIIwW40yV2CfW1Jb/jLHO5l22ody7EX\n\teJcLzYG2mAzGJ10NNqhdO4+Ewr8qgsrXWHEBoIl7CQFeJm9QVdbE/5iTgE89VZBHovDS6NjIl56Xq\n\t9FJ07spae39j0S3dTY2A==;","Date":"Thu, 14 Sep 2017 20:55:56 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170914195555.GB24231@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913143325.hi4cfcajbts3bbao@localhost>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170914_125621_015480_1C72B66E ","X-CRM114-Status":"GOOD ( 19.30 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1771221,"web_url":"http://patchwork.ozlabs.org/comment/1771221/","msgid":"<20170919171332.sjlhwnxqklmb5wsx@armageddon.cambridge.arm.com>","list_archive_url":null,"date":"2017-09-19T17:13:33","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Thu, Sep 14, 2017 at 08:40:41PM +0100, Dave P Martin wrote:\n> On Wed, Sep 13, 2017 at 03:21:29PM -0700, Catalin Marinas wrote:\n> > On Wed, Sep 13, 2017 at 08:17:07PM +0100, Dave P Martin wrote:\n> > > On Wed, Sep 13, 2017 at 10:26:05AM -0700, Catalin Marinas wrote:\n> > > > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > > > +/*\n> > > > > + * Trapped SVE access\n> > > > > + */\n> > > > > +void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> > > > > +{\n> > > > > +\t/* Even if we chose not to use SVE, the hardware could still trap: */\n> > > > > +\tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> > > > > +\t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> > > > > +\t\treturn;\n> > > > > +\t}\n> > > > > +\n> > > > > +\ttask_fpsimd_save();\n> > > > > +\n> > > > > +\tsve_alloc(current);\n> > > > > +\tfpsimd_to_sve(current);\n> > > > > +\tif (test_and_set_thread_flag(TIF_SVE))\n> > > > > +\t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> > > > > +\n> > > > > +\ttask_fpsimd_load();\n> > > > > +}\n> > > > \n> > > > When this function is entered, do we expect TIF_SVE to always be\n> > > > cleared? It's worth adding a comment on the expected conditions. If\n> > > \n> > > Yes, and this is required for correctness, as you observe.\n> > > \n> > > I had a BUG_ON() here which I removed, but it makes sense to add a\n> > > comment to capture the precondition here, and how it is satisfied.\n> > > \n> > > > that's the case, task_fpsimd_save() would only save the FPSIMD state\n> > > > which is fine. However, you subsequently transfer the FPSIMD state to\n> > > > SVE, set TIF_SVE and restore the full SVE state. If we don't care about\n> > > > the SVE state here, can we call task_fpsimd_load() *before* setting\n> > > > TIF_SVE?\n> > > \n> > > There should be no way to reach this code with TIF_SVE set, unless\n> > > task_fpsimd_load() sets the CPACR trap bit wrongly, or the hardware is\n> > > broken -- either of which is a bug.\n> > \n> > Thanks for confirming my assumptions. What I meant was rewriting the\n> > above function as:\n> > \n> > \t/* reset the SVE state (other than FPSIMD) */\n> > \ttask_fpsimd_save();\n> > \ttask_fpsimd_load();\n> \n> I think this works, but can you explain your rationale?\n> \n> I think the main effect of your suggestion is that it is cheaper, due\n> to eliminating some unnecessary load/store operations.\n\nMy rationale was to avoid copying between the in-memory FPSIMD and SVE\nstate.\n\n> We could go one better, and do\n> \n> \tmov\tv0.16b, v0.16b\n> \tmov\tv1.16b, v1.16b\n> \t// ...\n> \tmov\tv31.16b, v31.16b\n> \n> which doesn't require any memory access.\n\nYes, that's even better.\n\n> But I still prefer to zero p0..p15, ffr for cleanliness, even though\n> the SVE programmer's model doesn't require this (unlike for the Z-reg\n> high bits where we do need to zero them in order not to violate the\n> programmer's model).\n\nI missed the px, ffr aspect. Can you not have a clear_sve_state() (or a\nbetter name) function to zero the predicate regs, ffr and the top bits\nof the vectors?\n\n> Currently sve_alloc()+task_fpsimd_load() ensures that all the non-FPSIMD\n> regs are zeroed too, in addition to the Z-reg high bits.\n\nYes, just wondering if this can be implemented with less memory accesses\nsince the SVE state is irrelevant at this stage.\n\n> \n> So we might want a special-purpose helper -- if so, we can do it all\n> with no memory access.\n> \n> \tpfalse\tp0.b\n> \t// ..\n> \tpfalse\tp15.b\n> \twrffr\tp0.b\n> \n> This would allow the memset-zero an sve_alloc() to be removed, but I\n> would need to check what other code is relying on it.\n> \n> I guess I hadn't done this because I viewed it as an optimisation.\n\nIt looked like some low-hanging optimisation to slightly accelerate the\nallocation of the SVE state on access, though I'm also worried I don't\nfully understand all the corner cases (like what happens if we allow\ninterrupts during this function and get preempted).\n\nAnyway, I'm fine to leave this as it is for now and try to optimise it\nlater with additional patches on top.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"KyA66Grq\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxTxS6XFSz9sMN\n\tfor ;\n\tWed, 20 Sep 2017 03:14:04 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1duM5x-0007pa-O5; Tue, 19 Sep 2017 17:14:01 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1duM5u-0007nj-6v for linux-arm-kernel@lists.infradead.org;\n\tTue, 19 Sep 2017 17:13:59 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7E12C1435;\n\tTue, 19 Sep 2017 10:13:37 -0700 (PDT)","from armageddon.cambridge.arm.com (armageddon.cambridge.arm.com\n\t[10.1.206.84])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tAD4E63F58C; Tue, 19 Sep 2017 10:13:35 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=L9W+8h2w4x+K8H6Nbfo4/q4g/HnZxMVYK5Wzy11Qf+w=;\n\tb=KyA66Grqrlg76y\n\txDteeluBbtZ2TqAW4NW4nbEZOK8Uvu2OMdAYxPBY0JFqkm35Fw0t4BEE/FV10VComr2pP98ScqBJX\n\tbCB5JxNrk9TUSr7NEtmcmmaculNSbH5pkcY8hCesM5WeeQ2RpannJLu07WDh+GvdLNsFqaKWC4yfi\n\twEN4cBSuB5Au/m8UZjaUROl+p3rQi9chnvOh3giI4dwDLs1ZitqttNPW9Hzn9MF+8UoydbUD2hUaU\n\tb64tcLFw14kWozB4jBzBKrjbTdLY6l3KegMIPggR1IFFQj8TKCrmTtgnN4seDouP6X+z8G19D1wNL\n\tU6/q2AXQCfBCrsDz6G6w==;","Date":"Tue, 19 Sep 2017 18:13:33 +0100","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170919171332.sjlhwnxqklmb5wsx@armageddon.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913172605.hjrwjf34kyy7coh4@localhost>\n\t<20170913191707.GD23415@e103592.cambridge.arm.com>\n\t<20170913222129.vi3lidawdp33zzjd@localhost>\n\t<20170914193944.GA24231@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170914193944.GA24231@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170919_101358_283635_B162ADFB ","X-CRM114-Status":"GOOD ( 29.27 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1771886,"web_url":"http://patchwork.ozlabs.org/comment/1771886/","msgid":"<20170920135856.dfuowttwi4bk4nqb@localhost>","list_archive_url":null,"date":"2017-09-20T13:58:56","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Thu, Sep 14, 2017 at 08:55:56PM +0100, Dave P Martin wrote:\n> On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > +/*\n> > > + * Handle SVE state across fork():\n> > > + *\n> > > + * dst and src must not end up with aliases of the same sve_state.\n> > > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > > + * state for dst here, so long as we take care to retain the FPSIMD\n> > > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > > + * will be deferred until dst tries to use SVE.\n> > > + */\n> > > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > > +{\n> > > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > > +\t\tsve_to_fpsimd(dst);\n> > > +\t}\n> > > +\n> > > +\tdst->thread.sve_state = NULL;\n> > > +}\n> > \n> > I first thought the thread flags are not visible in dst yet since\n> > dup_task_struct() calls arch_dup_task_struct() before\n> > setup_thread_stack(). However, at the end of the last year we enabled\n> > CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> > on this.\n> \n> Hmmm, I see your point, but there are some sequencing issues here.\n> \n> > Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> > arch_dup_task_struct() already called fpsimd_preserve_current_state()\n> \n> I consider SVE discard as an optional side effect of task_fpsimd_save(),\n> not something that is guaranteed to happen -- the decision about whether\n> to do so may become more intelligent later on. So, for src, we may\n> discard SVE (because syscall), but for dst we must NULL .sve_state (and\n> therefore clear TIF_SVE) simply to avoid aliasing of src->sve_state and\n> dst->sve_state.\n\nMy point was that the SVE state of src is already preserved at this\npoint and copied into dst. You don't need the sve_to_fpsimd(dst) again\nwhich basically does the same copying of the src SVE saved state into\nthe FPSIMD one in dst. This has already been done in\narch_dup_task_struct() by the combination of\nfpsimd_preserve_current_state() and *dst = *src (and, of course,\nclearing TIF_SVE in dst).\n\nI don't think the TIF_SVE clearing in src is just a side effect of\ntask_fpsimd_save() here but rather a requirement. When returning from\nfork(), both src and dst would need to have the same state. However,\nyour fpsimd_dup_sve() implementation makes it very clear that the SVE\nstate is lost in dst. This is only allowed if we also lose it in src (as\na result of a syscall). So making dst->sve_state = NULL requires that\nTIF_SVE is also cleared in both src and dst. Alternatively, you'd have\nto allocate a new state here and copy the full src SVE state across to\ndst, together with setting TIF_SVE (that's not necessary, however, since\nwe get here as a result of a syscall).\n\n> > for src, so the FPSIMD state (which we care about) is transferred during\n> > the *dst = *src assignment. So you'd only need the last statement,\n> > possibly with a different function name like fpsimd_erase_sve (and maybe\n> > make the function static inline in the header).\n> \n> Not quite: TIF_SVE must be cleared so that a context switch or\n> kernel_neon_begin() after dst is scheduled doesn't try to save state in\n> the (NULL) dst->sve_state.\n\nYes, TIF_SVE must also be cleared in dst when dst->sve_state = NULL (I\nmay have forgotten to mention this).","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"fG3gFsW2\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy1ZW1brfz9s7h\n\tfor ;\n\tWed, 20 Sep 2017 23:59:31 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dufXC-00031e-1t; Wed, 20 Sep 2017 13:59:26 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dufX8-0002wD-B1 for linux-arm-kernel@lists.infradead.org;\n\tWed, 20 Sep 2017 13:59:24 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 28C4515A2;\n\tWed, 20 Sep 2017 06:59:01 -0700 (PDT)","from localhost (unknown [10.1.32.58])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tDB4383F58C; Wed, 20 Sep 2017 06:59:00 -0700 (PDT)","from cmarinas by localhost with local (Exim 4.89)\n\t(envelope-from )\n\tid 1dufWj-0001ju-7v; Wed, 20 Sep 2017 14:58:57 +0100"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=GugHE1q64PVTwdFRDKnihIFdkrzI0jV5bVwGq/4uRUM=;\n\tb=fG3gFsW2Rb6Q2E\n\txQq1NEsrPH5oMHqPcaHGcEEVUXtl/uD2WcpNFN5KPV2WrdOz3ytZgtA/LWCHJ6Jr+CeR1Iy30OQHK\n\tw3HTtREZzTOPD5dz4ZNTvriy14AZkfPRTB27GzZl3e2X9OpFwAyLd8r0AAvmf5LHmyGCvO8UV+woV\n\t5692OiABqMaUsoWu7cBjYTAk/OfkCjANS8MaLXvhMXtHR4YOT2rdMja6DrD7VhDC4vgbgzTj3ddHE\n\tSXvbbWenTichUdSYKQ44QQ0i5K7DMdEUD/a+mQc1vbHpU1IBnPvsvwXEaG//L2l7Yrxdzfo7F9+k9\n\tnUzo0jPa08DQQW2CMVbg==;","Date":"Wed, 20 Sep 2017 14:58:56 +0100","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20170920135856.dfuowttwi4bk4nqb@localhost>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20170914195555.GB24231@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170914195555.GB24231@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170920_065922_493435_53815865 ","X-CRM114-Status":"GOOD ( 22.04 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1778842,"web_url":"http://patchwork.ozlabs.org/comment/1778842/","msgid":"<20171003111100.GQ3611@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-03T11:11:01","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Wed, Sep 20, 2017 at 02:58:56PM +0100, Catalin Marinas wrote:\n> On Thu, Sep 14, 2017 at 08:55:56PM +0100, Dave P Martin wrote:\n> > On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> > > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > > +/*\n> > > > + * Handle SVE state across fork():\n> > > > + *\n> > > > + * dst and src must not end up with aliases of the same sve_state.\n> > > > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > > > + * state for dst here, so long as we take care to retain the FPSIMD\n> > > > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > > > + * will be deferred until dst tries to use SVE.\n> > > > + */\n> > > > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > > > +{\n> > > > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > > > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > > > +\t\tsve_to_fpsimd(dst);\n> > > > +\t}\n> > > > +\n> > > > +\tdst->thread.sve_state = NULL;\n> > > > +}\n> > > \n> > > I first thought the thread flags are not visible in dst yet since\n> > > dup_task_struct() calls arch_dup_task_struct() before\n> > > setup_thread_stack(). However, at the end of the last year we enabled\n> > > CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> > > on this.\n> > \n> > Hmmm, I see your point, but there are some sequencing issues here.\n> > \n> > > Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> > > arch_dup_task_struct() already called fpsimd_preserve_current_state()\n> > \n> > I consider SVE discard as an optional side effect of task_fpsimd_save(),\n> > not something that is guaranteed to happen -- the decision about whether\n> > to do so may become more intelligent later on. So, for src, we may\n> > discard SVE (because syscall), but for dst we must NULL .sve_state (and\n> > therefore clear TIF_SVE) simply to avoid aliasing of src->sve_state and\n> > dst->sve_state.\n> \n> My point was that the SVE state of src is already preserved at this\n> point and copied into dst. You don't need the sve_to_fpsimd(dst) again\n> which basically does the same copying of the src SVE saved state into\n> the FPSIMD one in dst. This has already been done in\n> arch_dup_task_struct() by the combination of\n> fpsimd_preserve_current_state() and *dst = *src (and, of course,\n> clearing TIF_SVE in dst).\n> \n> I don't think the TIF_SVE clearing in src is just a side effect of\n> task_fpsimd_save() here but rather a requirement. When returning from\n> fork(), both src and dst would need to have the same state. However,\n> your fpsimd_dup_sve() implementation makes it very clear that the SVE\n> state is lost in dst. This is only allowed if we also lose it in src (as\n> a result of a syscall). So making dst->sve_state = NULL requires that\n> TIF_SVE is also cleared in both src and dst. Alternatively, you'd have\n> to allocate a new state here and copy the full src SVE state across to\n> dst, together with setting TIF_SVE (that's not necessary, however, since\n> we get here as a result of a syscall).\n\nThe currently intended ABI is that the SVE bits are unspecified after a\nsyscall, so it is legitimate (though perhaps surprising) for different\nthings to happen in dst and src.\n\nThis complicates things a lot though, just to avoid the next SVE usage\nexception in src after the fork.\n\n\nIt should be simpler to do what you suggest and discard the SVE state of\nsrc unconditionally before the copy: then we really are just cloning the\nthread apart from the need to set dst->thread.sve_state to NULL.\n\nfpsimd_preserve_current_state() does not necessarily write back to\ncurrent->thread.fpsmid_state though: at the moment, it does do this as a\nside effect of task_fpsimd_save() because we happen to be in a syscall\n(i.e., fork). \n\nWhat we really want is unconditional discarding of the state. This\nwasn't needed anywhere else yet, so there's no explicit helper for it.\nBut it makes sense to add one.\n\nWhat about refactoring along these lines:\n\n\nfpsimd.c:\n/* Unconditionally discard the SVE state */\nvoid task_sve_discard(struct task_struct *task)\n{\n\tif (!system_supports_sve())\n\t\treturn;\n\n\tlocal_bh_disable();\n\tif (test_and_clear_tsk_thread_flag(task, TIF_SVE))\n\t\tsve_to_fpsimd(task);\n\tlocal_bh_enable();\n}\n\nprocess.c:\nint arch_dup_task_struct(sturct task_struct *dst, struct task_struct *src)\n{\n\tif (current->mm) {\n\t\tfpsimd_preserve_current_state();\n\t\ttask_sve_discard(src);\n\t}\n\n\t*dst = *src;\n\n\tdst->thread.sve_state = NULL;\n}\n\n\nThis also avoids having to touch dst's thread flags, since now we\nare just cloning the task except for assigning NULL to\ndst->thread.sve_state.\n\n\n> > > for src, so the FPSIMD state (which we care about) is transferred during\n> > > the *dst = *src assignment. So you'd only need the last statement,\n> > > possibly with a different function name like fpsimd_erase_sve (and maybe\n> > > make the function static inline in the header).\n> > \n> > Not quite: TIF_SVE must be cleared so that a context switch or\n> > kernel_neon_begin() after dst is scheduled doesn't try to save state in\n> > the (NULL) dst->sve_state.\n> \n> Yes, TIF_SVE must also be cleared in dst when dst->sve_state = NULL (I\n> may have forgotten to mention this).\n\nWith the above factoring, the constraint \"TIF_SVE implies sve_state\nvalid\" is then never false, because TIF_SVE is already cleared before\nthe NULL assignment.\n\nWhat do you think?\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"hATV70Fq\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y5xDl3G8Dz9sxR\n\tfor ;\n\tTue, 3 Oct 2017 22:11:35 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dzL6o-0005ZU-Gj; Tue, 03 Oct 2017 11:11:30 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dzL6k-0005Xs-Tl for linux-arm-kernel@lists.infradead.org;\n\tTue, 03 Oct 2017 11:11:28 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A8D9680D;\n\tTue, 3 Oct 2017 04:11:05 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tDA0753F578; Tue, 3 Oct 2017 04:11:03 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=2nFVTxvKz5JIpfz+T9x3R+11xtToSkGJs3jM19sv53M=;\n\tb=hATV70FqJVTppy\n\tmKzZySIO5qYDiiwD1wn7FtSSvJvqKTi02HrkV6yCAVF7tNMIpa5iZSj2cvRZrC/5EwQd105k9w9iy\n\t/RJYtI24QzSae8Cuk+G3k/I46quFKnTgsavG7F1AhD0HeJmm4kYJquSvDdxHjqRYbxy2BhN2Q5TkC\n\tbNZYJsS+QufrtQ3mG1T8Pag1ZC4fPoYAsobNUyuBZdeo6ObhQC55UMlx/v8Dx9C9B8KykIXboW9a0\n\tckMgW72VPovvqYR/ZZB8FkPqTwsaQAvc3C2jf3qfG6l8QdTZXqOkLe70vu6dS+6L8vRAdtNeJnp5U\n\ttgZGtLw13q05m/K9qTeQ==;","Date":"Tue, 3 Oct 2017 12:11:01 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171003111100.GQ3611@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20170914195555.GB24231@e103592.cambridge.arm.com>\n\t<20170920135856.dfuowttwi4bk4nqb@localhost>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170920135856.dfuowttwi4bk4nqb@localhost>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171003_041126_976971_6DAFA176 ","X-CRM114-Status":"GOOD ( 29.36 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1778851,"web_url":"http://patchwork.ozlabs.org/comment/1778851/","msgid":"<20171003113302.GR3611@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-03T11:33:03","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > +/*\n> > + * Handle SVE state across fork():\n> > + *\n> > + * dst and src must not end up with aliases of the same sve_state.\n> > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > + * state for dst here, so long as we take care to retain the FPSIMD\n> > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > + * will be deferred until dst tries to use SVE.\n> > + */\n> > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > +{\n> > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > +\t\tsve_to_fpsimd(dst);\n> > +\t}\n> > +\n> > +\tdst->thread.sve_state = NULL;\n> > +}\n> \n> I first thought the thread flags are not visible in dst yet since\n> dup_task_struct() calls arch_dup_task_struct() before\n> setup_thread_stack(). However, at the end of the last year we enabled\n> CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> on this.\n> \n> Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> arch_dup_task_struct() already called fpsimd_preserve_current_state()\n> for src, so the FPSIMD state (which we care about) is transferred during\n> the *dst = *src assignment. So you'd only need the last statement,\n> possibly with a different function name like fpsimd_erase_sve (and maybe\n> make the function static inline in the header).\n\nRegarding the intended meanings of the thread flags, does this help?\n\n--8<--\n\nTIF_FOREIGN_FPSTATE's meaning is expanded to cover SVE, but otherwise\nunchanged:\n\n * If a task is running and !TIF_FOREIGN_FPSTATE, then the the CPU\n registers of the CPU the task is running on contain the authoritative\n FPSIMD/SVE state of the task. The backing memory may be stale.\n\n * Otherwise (i.e., task not running, or task running and\n TIF_FOREIGN_FPSTATE), the task's FPSIMD/SVE backing memory is\n authoritative. If additionally per_cpu(fpsimd_last_state,\n task->fpsimd_state.cpu) == &task->fpsimd_state.cpu, then\n task->fpsimd_state.cpu's registers are also up to date for task, but\n not authorititive: the current FPSIMD/SVE state may be read from\n them, but they must not be written.\n \nThe FPSIMD/SVE backing memory is selected by TIF_SVE:\n\n * TIF_SVE set: Zn (incorporating Vn in bits[127:0]), Pn and FFR are\n stored in task->thread.sve_state, formatted appropriately for vector\n length task->thread.sve_vl. task->thread.sve_state must point to a\n valid buffer at least sve_state_size(task) bytes in size.\n\n * TIF_SVE clear: Vn are stored in task->fpsimd_state; Zn[max : 128] are\n logically zero[*] but not stored anywhere; Pn, FFR are not stored and\n have unspecified values from userspace's point of view.\n task->thread.sve_state does not need to be non-null, valid or any\n particular size: it must not be dereferenced.\n\n In practice I don't exploit the \"unspecifiedness\" much. The Zn high\n bits, Pn and FFR are all zeroed when setting TIF_SVE again:\n sve_alloc() is the common path for this.\n\n * FPSR and FPCR are always stored in task->fpsimd_state irrespctive of\n whether TIF_SVE is clear or set, since these are not vector length\n dependent.\n\n\n[*] theoretically unspecified, which is what I've written in sve.txt.\nHowever, on any FPSIMD Vn write the upper bits of the corresponding Zn\nmust become logically zero in order to conform to the SVE programmer's\nmodel. It's not feasible to track which Vn have been written\nindividually because that would involve trapping every FPSIMD insn until\nall possible Vn have been written. So the kernel ensures that the Zn\nhigh bits become zero.\n\nMaybe we should just guarantee \"zero-or-preserved\" behaviour for\nuserspace. This may close down some future optimisation opportunities,\nbut maybe it's better to be simple.\n\nThoughts?\n\n[...]\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"iJid21vh\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y5xk65xkMz9t2Q\n\tfor ;\n\tTue, 3 Oct 2017 22:33:34 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dzLS7-0007tZ-9v; Tue, 03 Oct 2017 11:33:31 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dzLS3-0007qS-NF for linux-arm-kernel@lists.infradead.org;\n\tTue, 03 Oct 2017 11:33:29 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6D1F880D;\n\tTue, 3 Oct 2017 04:33:07 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tA13503F578; Tue, 3 Oct 2017 04:33:05 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=VPISZP3NDr7in/ba+tDC/sTwkUF7wDZ/nc/Cd7dSVmM=;\n\tb=iJid21vhDoc3mG\n\ty+s6WEstoaLNlSCd588/iMrTKU7OIes0AHXxn7syg9X5ufk8dg+H8qVMJiUCOtQ7KGfxLvu3M5DzO\n\tQje0l3qw9zJ9nBmGqeZ7jPOuq/uhaENmbuW2rBlMMvPmv+B08O+hSFw9Q5LJsF98PwqDhS2fZiNoh\n\t8ETRkKFaZrCRmbJ1F+DzVxLSvmzEFr/HIM1XUTaTOBdAa3nbjrFSCNNG3W40Oq0/UBVQHpJSlDd2/\n\t9eQvZvS4Q/tLnPeSZHdnOieJBUQdcuf5e4IHunkEnBYXQoVgXtOh8ATXHqUsX5NoX+22VUXBEbg6J\n\tzyhY3yLz8QRxtnkF37dw==;","Date":"Tue, 3 Oct 2017 12:33:03 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171003113302.GR3611@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20170913143325.hi4cfcajbts3bbao@localhost>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171003_043327_780402_0116F332 ","X-CRM114-Status":"GOOD ( 21.34 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1780004,"web_url":"http://patchwork.ozlabs.org/comment/1780004/","msgid":"<20171004172955.tyooqxdoo73iq66u@armageddon.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-04T17:29:56","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Tue, Oct 03, 2017 at 12:11:01PM +0100, Dave P Martin wrote:\n> On Wed, Sep 20, 2017 at 02:58:56PM +0100, Catalin Marinas wrote:\n> > On Thu, Sep 14, 2017 at 08:55:56PM +0100, Dave P Martin wrote:\n> > > On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> > > > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > > > +/*\n> > > > > + * Handle SVE state across fork():\n> > > > > + *\n> > > > > + * dst and src must not end up with aliases of the same sve_state.\n> > > > > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > > > > + * state for dst here, so long as we take care to retain the FPSIMD\n> > > > > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > > > > + * will be deferred until dst tries to use SVE.\n> > > > > + */\n> > > > > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > > > > +{\n> > > > > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > > > > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > > > > +\t\tsve_to_fpsimd(dst);\n> > > > > +\t}\n> > > > > +\n> > > > > +\tdst->thread.sve_state = NULL;\n> > > > > +}\n> > > > \n> > > > I first thought the thread flags are not visible in dst yet since\n> > > > dup_task_struct() calls arch_dup_task_struct() before\n> > > > setup_thread_stack(). However, at the end of the last year we enabled\n> > > > CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> > > > on this.\n> > > \n> > > Hmmm, I see your point, but there are some sequencing issues here.\n> > > \n> > > > Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> > > > arch_dup_task_struct() already called fpsimd_preserve_current_state()\n> > > \n> > > I consider SVE discard as an optional side effect of task_fpsimd_save(),\n> > > not something that is guaranteed to happen -- the decision about whether\n> > > to do so may become more intelligent later on. So, for src, we may\n> > > discard SVE (because syscall), but for dst we must NULL .sve_state (and\n> > > therefore clear TIF_SVE) simply to avoid aliasing of src->sve_state and\n> > > dst->sve_state.\n> > \n> > My point was that the SVE state of src is already preserved at this\n> > point and copied into dst. You don't need the sve_to_fpsimd(dst) again\n> > which basically does the same copying of the src SVE saved state into\n> > the FPSIMD one in dst. This has already been done in\n> > arch_dup_task_struct() by the combination of\n> > fpsimd_preserve_current_state() and *dst = *src (and, of course,\n> > clearing TIF_SVE in dst).\n> > \n> > I don't think the TIF_SVE clearing in src is just a side effect of\n> > task_fpsimd_save() here but rather a requirement. When returning from\n> > fork(), both src and dst would need to have the same state. However,\n> > your fpsimd_dup_sve() implementation makes it very clear that the SVE\n> > state is lost in dst. This is only allowed if we also lose it in src (as\n> > a result of a syscall). So making dst->sve_state = NULL requires that\n> > TIF_SVE is also cleared in both src and dst. Alternatively, you'd have\n> > to allocate a new state here and copy the full src SVE state across to\n> > dst, together with setting TIF_SVE (that's not necessary, however, since\n> > we get here as a result of a syscall).\n> \n> The currently intended ABI is that the SVE bits are unspecified after a\n> syscall, so it is legitimate (though perhaps surprising) for different\n> things to happen in dst and src.\n> \n> This complicates things a lot though, just to avoid the next SVE usage\n> exception in src after the fork.\n> \n> \n> It should be simpler to do what you suggest and discard the SVE state of\n> src unconditionally before the copy: then we really are just cloning the\n> thread apart from the need to set dst->thread.sve_state to NULL.\n> \n> fpsimd_preserve_current_state() does not necessarily write back to\n> current->thread.fpsmid_state though: at the moment, it does do this as a\n> side effect of task_fpsimd_save() because we happen to be in a syscall\n> (i.e., fork). \n> \n> What we really want is unconditional discarding of the state. This\n> wasn't needed anywhere else yet, so there's no explicit helper for it.\n> But it makes sense to add one.\n> \n> What about refactoring along these lines:\n> \n> \n> fpsimd.c:\n> /* Unconditionally discard the SVE state */\n> void task_sve_discard(struct task_struct *task)\n> {\n> \tif (!system_supports_sve())\n> \t\treturn;\n> \n> \tlocal_bh_disable();\n> \tif (test_and_clear_tsk_thread_flag(task, TIF_SVE))\n> \t\tsve_to_fpsimd(task);\n> \tlocal_bh_enable();\n> }\n> \n> process.c:\n> int arch_dup_task_struct(sturct task_struct *dst, struct task_struct *src)\n> {\n> \tif (current->mm) {\n> \t\tfpsimd_preserve_current_state();\n> \t\ttask_sve_discard(src);\n> \t}\n> \n> \t*dst = *src;\n> \n> \tdst->thread.sve_state = NULL;\n> }\n> \n> \n> This also avoids having to touch dst's thread flags, since now we\n> are just cloning the task except for assigning NULL to\n> dst->thread.sve_state.\n\nThis looks fine to me, the execution of task_sve_discard() is nearly a\nno-op with the current code.\n\nWe still have some local_bh_disable/enable() calls, though I don't think\nit's worth optimising them now (e.g. having a\nfpsimd_preserve_current_state_and_discard_sve() function with a\n\"sve_discard\" argument to task_fpsimd_save() to force this).","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"Dp6fz44n\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y6jbV46H7z9t4X\n\tfor ;\n\tThu, 5 Oct 2017 04:30:30 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dznV4-0004nc-I2; Wed, 04 Oct 2017 17:30:26 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dznV0-0003aI-Bj for linux-arm-kernel@lists.infradead.org;\n\tWed, 04 Oct 2017 17:30:24 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B55441529;\n\tWed, 4 Oct 2017 10:30:00 -0700 (PDT)","from armageddon.cambridge.arm.com (armageddon.cambridge.arm.com\n\t[10.1.206.84])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tE852B3F58C; Wed, 4 Oct 2017 10:29:58 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=/rn+fv6c2L7G7hu8KcRT4gPoZZqRkv69O3+0XzZ13X0=;\n\tb=Dp6fz44nllRMTL\n\t9PizVkrIvrb0r0XddWFFpJjWTiI3w7E7DKUADHQbwp1qTmW9ZP5yGbMLGSTpoGFZQL84teZDCKMAq\n\tIIm76+t82MQsJMHB2p7tSZ/zJ/QsQ7vjuzKBbhr/YGjbcChmsZ68uhLE/+c5VqkYMLxtUhyjkMARr\n\tVaVdTFCgnSWcXXJG01m8QszUHntCJBhnIuCfZo5J5EVtAApQbLQd6+uGSlvl/4A9CRmuDUvjicQKy\n\tAAayF/bejcZflqhvy0KDvGF1XKOcUPNKk/BeYrd8yLWykHXp9L3OewUMhvupA0qnOBQBhyHecS4Uh\n\tPDFaRciaempDnJJA3grg==;","Date":"Wed, 4 Oct 2017 18:29:56 +0100","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171004172955.tyooqxdoo73iq66u@armageddon.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20170914195555.GB24231@e103592.cambridge.arm.com>\n\t<20170920135856.dfuowttwi4bk4nqb@localhost>\n\t<20171003111100.GQ3611@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20171003111100.GQ3611@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171004_103022_420623_821135B3 ","X-CRM114-Status":"GOOD ( 30.24 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1780509,"web_url":"http://patchwork.ozlabs.org/comment/1780509/","msgid":"<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-05T11:28:35","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Tue, Oct 03, 2017 at 12:33:03PM +0100, Dave P Martin wrote:\n> On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > +/*\n> > > + * Handle SVE state across fork():\n> > > + *\n> > > + * dst and src must not end up with aliases of the same sve_state.\n> > > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > > + * state for dst here, so long as we take care to retain the FPSIMD\n> > > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > > + * will be deferred until dst tries to use SVE.\n> > > + */\n> > > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > > +{\n> > > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > > +\t\tsve_to_fpsimd(dst);\n> > > +\t}\n> > > +\n> > > +\tdst->thread.sve_state = NULL;\n> > > +}\n> > \n> > I first thought the thread flags are not visible in dst yet since\n> > dup_task_struct() calls arch_dup_task_struct() before\n> > setup_thread_stack(). However, at the end of the last year we enabled\n> > CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> > on this.\n> > \n> > Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> > arch_dup_task_struct() already called fpsimd_preserve_current_state()\n> > for src, so the FPSIMD state (which we care about) is transferred during\n> > the *dst = *src assignment. So you'd only need the last statement,\n> > possibly with a different function name like fpsimd_erase_sve (and maybe\n> > make the function static inline in the header).\n> \n> Regarding the intended meanings of the thread flags, does this help?\n> \n> --8<--\n> \n> TIF_FOREIGN_FPSTATE's meaning is expanded to cover SVE, but otherwise\n> unchanged:\n> \n> * If a task is running and !TIF_FOREIGN_FPSTATE, then the the CPU\n> registers of the CPU the task is running on contain the authoritative\n> FPSIMD/SVE state of the task. The backing memory may be stale.\n> \n> * Otherwise (i.e., task not running, or task running and\n> TIF_FOREIGN_FPSTATE), the task's FPSIMD/SVE backing memory is\n> authoritative. If additionally per_cpu(fpsimd_last_state,\n> task->fpsimd_state.cpu) == &task->fpsimd_state.cpu, then\n> task->fpsimd_state.cpu's registers are also up to date for task, but\n> not authorititive: the current FPSIMD/SVE state may be read from\n> them, but they must not be written.\n> \n> The FPSIMD/SVE backing memory is selected by TIF_SVE:\n> \n> * TIF_SVE set: Zn (incorporating Vn in bits[127:0]), Pn and FFR are\n> stored in task->thread.sve_state, formatted appropriately for vector\n> length task->thread.sve_vl. task->thread.sve_state must point to a\n> valid buffer at least sve_state_size(task) bytes in size.\n> \n> * TIF_SVE clear: Vn are stored in task->fpsimd_state; Zn[max : 128] are\n> logically zero[*] but not stored anywhere; Pn, FFR are not stored and\n> have unspecified values from userspace's point of view.\n> task->thread.sve_state does not need to be non-null, valid or any\n> particular size: it must not be dereferenced.\n> \n> In practice I don't exploit the \"unspecifiedness\" much. The Zn high\n> bits, Pn and FFR are all zeroed when setting TIF_SVE again:\n> sve_alloc() is the common path for this.\n> \n> * FPSR and FPCR are always stored in task->fpsimd_state irrespctive of\n> whether TIF_SVE is clear or set, since these are not vector length\n> dependent.\n\nThis looks fine. I think we need to make sure (with a warning) that\ntask_fpsimd_save() (and probably load) is always called in a\nnon-preemptible context. \n\n> [*] theoretically unspecified, which is what I've written in sve.txt.\n> However, on any FPSIMD Vn write the upper bits of the corresponding Zn\n> must become logically zero in order to conform to the SVE programmer's\n> model. It's not feasible to track which Vn have been written\n> individually because that would involve trapping every FPSIMD insn until\n> all possible Vn have been written. So the kernel ensures that the Zn\n> high bits become zero.\n> \n> Maybe we should just guarantee \"zero-or-preserved\" behaviour for\n> userspace. This may close down some future optimisation opportunities,\n> but maybe it's better to be simple.\n\nDoes it work if we leave it as \"unspecified\" in the document but just do\nzero-or-preserved in the kernel code?\n\nJust wondering, as an optimisation for do_sve_acc() - instead of\nsve_alloc() and fpsimd_to_sve(), can we not force the loading of the\nFPSIMD regs on the return to user via TIF_FOREIGN_FPSTATE? This would\nensure the zeroing of the top SVE bits and we only need to allocate the\nSVE state on the saving path. This means enabling SVE for user and\nsetting TIF_SVE without having the backing storage allocated.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"X3pBn9WA\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y79X25MZyz9t2h\n\tfor ;\n\tThu, 5 Oct 2017 22:29:06 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e04Kt-0007A7-Ge; Thu, 05 Oct 2017 11:29:03 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e04Kq-00075X-CE for linux-arm-kernel@lists.infradead.org;\n\tThu, 05 Oct 2017 11:29:02 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 07E4D80D;\n\tThu, 5 Oct 2017 04:28:40 -0700 (PDT)","from armageddon.cambridge.arm.com (armageddon.cambridge.arm.com\n\t[10.1.206.84])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t3A9913F483; Thu, 5 Oct 2017 04:28:38 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=1o20zUtd+WY/lqHGFkGjmojKs56jE0mz11HTx6SZTIs=;\n\tb=X3pBn9WANOExd5\n\tt5VyHjzy9T37ow3NNiKeH19rw0qS5aDa1q07HOKP1fo0dQJRZ5yxuW8qHvv8jcO2mvXWqHpOaliro\n\tCV5M9fdh43dOSujOory0ixpg3JSWAXydkdH9PuWf8cU1j9koyHEzzQ7HG7ZF8fHsA59gtfPUNpync\n\t8eOtfynEQysbNSKuVpdiSWPOzoUy1EAJ9ZfxuRZtkhVLkx1E0FHt4Q94P3ReySP3qSaMS9SlHsf7m\n\tRoKqp3AuRiskHdZiXhxTvDJrQV7qbeI18flWYEtTQxgI92eyNbILjR4iD0hYj6ieMyVs1N1uZn/wI\n\tY2rd7WNMiY+iuHRC3IZg==;","Date":"Thu, 5 Oct 2017 12:28:35 +0100","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20171003113302.GR3611@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20171003113302.GR3611@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171005_042900_456927_16560131 ","X-CRM114-Status":"GOOD ( 30.61 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1781621,"web_url":"http://patchwork.ozlabs.org/comment/1781621/","msgid":"<20171006131005.GA3611@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-06T13:10:09","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Thu, Oct 05, 2017 at 12:28:35PM +0100, Catalin Marinas wrote:\n> On Tue, Oct 03, 2017 at 12:33:03PM +0100, Dave P Martin wrote:\n> > On Wed, Sep 13, 2017 at 07:33:25AM -0700, Catalin Marinas wrote:\n> > > On Thu, Aug 31, 2017 at 06:00:43PM +0100, Dave P Martin wrote:\n> > > > +/*\n> > > > + * Handle SVE state across fork():\n> > > > + *\n> > > > + * dst and src must not end up with aliases of the same sve_state.\n> > > > + * Because a task cannot fork except in a syscall, we can discard SVE\n> > > > + * state for dst here, so long as we take care to retain the FPSIMD\n> > > > + * subset of the state if SVE is in use. Reallocation of the SVE state\n> > > > + * will be deferred until dst tries to use SVE.\n> > > > + */\n> > > > +void fpsimd_dup_sve(struct task_struct *dst, struct task_struct const *src)\n> > > > +{\n> > > > +\tif (test_and_clear_tsk_thread_flag(dst, TIF_SVE)) {\n> > > > +\t\tWARN_ON(dst->mm && !in_syscall(task_pt_regs(dst)));\n> > > > +\t\tsve_to_fpsimd(dst);\n> > > > +\t}\n> > > > +\n> > > > +\tdst->thread.sve_state = NULL;\n> > > > +}\n> > > \n> > > I first thought the thread flags are not visible in dst yet since\n> > > dup_task_struct() calls arch_dup_task_struct() before\n> > > setup_thread_stack(). However, at the end of the last year we enabled\n> > > CONFIG_THREAD_INFO_IN_TASK_STRUCT. But I don't particularly like relying\n> > > on this.\n> > > \n> > > Anyway, IIUC we don't need sve_to_fpsimd() here. The\n> > > arch_dup_task_struct() already called fpsimd_preserve_current_state()\n> > > for src, so the FPSIMD state (which we care about) is transferred during\n> > > the *dst = *src assignment. So you'd only need the last statement,\n> > > possibly with a different function name like fpsimd_erase_sve (and maybe\n> > > make the function static inline in the header).\n> > \n> > Regarding the intended meanings of the thread flags, does this help?\n> > \n> > --8<--\n> > \n> > TIF_FOREIGN_FPSTATE's meaning is expanded to cover SVE, but otherwise\n> > unchanged:\n> > \n> > * If a task is running and !TIF_FOREIGN_FPSTATE, then the the CPU\n> > registers of the CPU the task is running on contain the authoritative\n> > FPSIMD/SVE state of the task. The backing memory may be stale.\n> > \n> > * Otherwise (i.e., task not running, or task running and\n> > TIF_FOREIGN_FPSTATE), the task's FPSIMD/SVE backing memory is\n> > authoritative. If additionally per_cpu(fpsimd_last_state,\n> > task->fpsimd_state.cpu) == &task->fpsimd_state.cpu, then\n> > task->fpsimd_state.cpu's registers are also up to date for task, but\n> > not authorititive: the current FPSIMD/SVE state may be read from\n> > them, but they must not be written.\n> > \n> > The FPSIMD/SVE backing memory is selected by TIF_SVE:\n> > \n> > * TIF_SVE set: Zn (incorporating Vn in bits[127:0]), Pn and FFR are\n> > stored in task->thread.sve_state, formatted appropriately for vector\n> > length task->thread.sve_vl. task->thread.sve_state must point to a\n> > valid buffer at least sve_state_size(task) bytes in size.\n> > \n> > * TIF_SVE clear: Vn are stored in task->fpsimd_state; Zn[max : 128] are\n> > logically zero[*] but not stored anywhere; Pn, FFR are not stored and\n> > have unspecified values from userspace's point of view.\n> > task->thread.sve_state does not need to be non-null, valid or any\n> > particular size: it must not be dereferenced.\n> > \n> > In practice I don't exploit the \"unspecifiedness\" much. The Zn high\n> > bits, Pn and FFR are all zeroed when setting TIF_SVE again:\n> > sve_alloc() is the common path for this.\n> > \n> > * FPSR and FPCR are always stored in task->fpsimd_state irrespctive of\n> > whether TIF_SVE is clear or set, since these are not vector length\n> > dependent.\n> \n> This looks fine. I think we need to make sure (with a warning) that\n> task_fpsimd_save() (and probably load) is always called in a\n> non-preemptible context. \n> \n> > [*] theoretically unspecified, which is what I've written in sve.txt.\n> > However, on any FPSIMD Vn write the upper bits of the corresponding Zn\n> > must become logically zero in order to conform to the SVE programmer's\n> > model. It's not feasible to track which Vn have been written\n> > individually because that would involve trapping every FPSIMD insn until\n> > all possible Vn have been written. So the kernel ensures that the Zn\n> > high bits become zero.\n> > \n> > Maybe we should just guarantee \"zero-or-preserved\" behaviour for\n> > userspace. This may close down some future optimisation opportunities,\n> > but maybe it's better to be simple.\n> \n> Does it work if we leave it as \"unspecified\" in the document but just do\n> zero-or-preserved in the kernel code?\n\nSure, that's the behaviour today in effect.\n\nI'll leave the documentation unchanged, then we can take advantage of\nthis flexibility later if is proves to be useful.\n\n> Just wondering, as an optimisation for do_sve_acc() - instead of\n> sve_alloc() and fpsimd_to_sve(), can we not force the loading of the\n> FPSIMD regs on the return to user via TIF_FOREIGN_FPSTATE? This would\n> ensure the zeroing of the top SVE bits and we only need to allocate the\n> SVE state on the saving path. This means enabling SVE for user and\n> setting TIF_SVE without having the backing storage allocated.\n\nCurrently the set of places where the \"TIF_SVE implies sve_state valid\"\nassumption is applied is not very constrained, so while your suggestion\nis reasonable I'd rather not mess with it just now, if possible.\n\n\nBut we can do this (which is what my current fixup has):\n\nel0_sve_acc:\n\tenable_dbg_and_irq\n\t// ...\n\tbl\tdo_sve_acc\n\tb\tret_to_user\n\nvoid do_sve_acc(unsigned int esr, struct pt_regs *regs)\n{\n\t/* Even if we chose not to use SVE, the hardware could still trap: */\n\tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n\t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n\t\treturn;\n\t}\n\n\tsve_alloc(current);\n\n\tlocal_bh_disable();\n\tif (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {\n\t\ttask_fpsimd_load(); /* flushes high Zn bits as a side-effect */\n\t\tsve_flush_pregs();\n\t} else {\n\t\tsve_flush_all(); /* flush all the SVE bits in-place */\n\t}\n\n\tif (test_and_set_thread_flag(TIF_SVE))\n\t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n\tlocal_bh_enable();\n}\n\nwhere sve_flush_all() zeroes all the high Zn bits via a series of\nMOV Vn, Vn instructions, and also zeroes Pn and FFR. sve_fplush_pregs()\njust does the latter.\n\nIn the common case the sve_alloc() does a memzero, but doesn't\nallocate memory because more often than not, it will already have\nbeen allocated.\n\nOn this path, the memzero is now pointless -- I'll need to double-check\nwhat else may be impacted by its removal (probably only ptrace, and I'm\nnot sure if the zeroing is strictly required there).\n\n\nThere is still a bit of room for improvement, but it's still a step up\nfrom v2.\n\nWhat do you think?\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"HkfThFIw\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y7qkt4gWpz9t2f\n\tfor ;\n\tSat, 7 Oct 2017 00:10:46 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0SOp-0001qO-EO; Fri, 06 Oct 2017 13:10:43 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0SOg-00012A-V0 for linux-arm-kernel@lists.infradead.org;\n\tFri, 06 Oct 2017 13:10:40 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5BCED1435;\n\tFri, 6 Oct 2017 06:10:13 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t8ECF73F578; Fri, 6 Oct 2017 06:10:11 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=AwQbxRoR4ByoC/aInXRBHU2GGFnimIdQrxMpNmpJ2BY=;\n\tb=HkfThFIwckEocv\n\tpYxH5CfdN2kWqyp6Rqof1lmU5Rs9RZ5DRJtxX6TLj2LDkBP9D/kTucCucjZvSLbBdAw13nVp1GMi9\n\t1tMEFuu4nXbfGlBtq6F8A18S8b7F9syyZEeh1U8UdsD5AdODy8SPniNWXxh/WIjEicDkJGL9DgL5S\n\tqJccFXPV/QzlXwgWc8swzFe2VsdVuSggX1UjwNqkdOK4C71RRylGevXrBf51LT+uhFJ2mlz1AJkEF\n\tT/QSVHgrFz3dIclWUgsQ2EPpts0vmRobHQWsJkQVRyanl7YtlkK/KbSc/3TMRizxb7edBlnkNxtB9\n\tPDLt4i5WocHJPh0MfcWA==;","Date":"Fri, 6 Oct 2017 14:10:09 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171006131005.GA3611@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20171003113302.GR3611@e103592.cambridge.arm.com>\n\t<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171006_061035_139312_AEB7A332 ","X-CRM114-Status":"GOOD ( 38.38 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1781662,"web_url":"http://patchwork.ozlabs.org/comment/1781662/","msgid":"<20171006133639.ialgglabo7yca5bm@armageddon.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-06T13:36:40","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Fri, Oct 06, 2017 at 02:10:09PM +0100, Dave P Martin wrote:\n> On Thu, Oct 05, 2017 at 12:28:35PM +0100, Catalin Marinas wrote:\n> > On Tue, Oct 03, 2017 at 12:33:03PM +0100, Dave P Martin wrote:\n> > > TIF_FOREIGN_FPSTATE's meaning is expanded to cover SVE, but otherwise\n> > > unchanged:\n> > > \n> > > * If a task is running and !TIF_FOREIGN_FPSTATE, then the the CPU\n> > > registers of the CPU the task is running on contain the authoritative\n> > > FPSIMD/SVE state of the task. The backing memory may be stale.\n> > > \n> > > * Otherwise (i.e., task not running, or task running and\n> > > TIF_FOREIGN_FPSTATE), the task's FPSIMD/SVE backing memory is\n> > > authoritative. If additionally per_cpu(fpsimd_last_state,\n> > > task->fpsimd_state.cpu) == &task->fpsimd_state.cpu, then\n> > > task->fpsimd_state.cpu's registers are also up to date for task, but\n> > > not authorititive: the current FPSIMD/SVE state may be read from\n> > > them, but they must not be written.\n> > > \n> > > The FPSIMD/SVE backing memory is selected by TIF_SVE:\n> > > \n> > > * TIF_SVE set: Zn (incorporating Vn in bits[127:0]), Pn and FFR are\n> > > stored in task->thread.sve_state, formatted appropriately for vector\n> > > length task->thread.sve_vl. task->thread.sve_state must point to a\n> > > valid buffer at least sve_state_size(task) bytes in size.\n\n\"Zn [...] stored in task->thread.sve_state\" - is this still true with\nthe changes you proposed? I guess even without these changes, you have\nsituations where the hardware regs are out of sync with sve_state (see\nmore below).\n\n> > > * TIF_SVE clear: Vn are stored in task->fpsimd_state; Zn[max : 128] are\n> > > logically zero[*] but not stored anywhere; Pn, FFR are not stored and\n> > > have unspecified values from userspace's point of view.\n> > > task->thread.sve_state does not need to be non-null, valid or any\n> > > particular size: it must not be dereferenced.\n> > > \n> > > In practice I don't exploit the \"unspecifiedness\" much. The Zn high\n> > > bits, Pn and FFR are all zeroed when setting TIF_SVE again:\n> > > sve_alloc() is the common path for this.\n> > > \n> > > * FPSR and FPCR are always stored in task->fpsimd_state irrespctive of\n> > > whether TIF_SVE is clear or set, since these are not vector length\n> > > dependent.\n[...]\n> > Just wondering, as an optimisation for do_sve_acc() - instead of\n> > sve_alloc() and fpsimd_to_sve(), can we not force the loading of the\n> > FPSIMD regs on the return to user via TIF_FOREIGN_FPSTATE? This would\n> > ensure the zeroing of the top SVE bits and we only need to allocate the\n> > SVE state on the saving path. This means enabling SVE for user and\n> > setting TIF_SVE without having the backing storage allocated.\n> \n> Currently the set of places where the \"TIF_SVE implies sve_state valid\"\n> assumption is applied is not very constrained, so while your suggestion\n> is reasonable I'd rather not mess with it just now, if possible.\n> \n> \n> But we can do this (which is what my current fixup has):\n> \n> el0_sve_acc:\n> \tenable_dbg_and_irq\n> \t// ...\n> \tbl\tdo_sve_acc\n> \tb\tret_to_user\n> \n> void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> {\n> \t/* Even if we chose not to use SVE, the hardware could still trap: */\n> \tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> \t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> \t\treturn;\n> \t}\n> \n> \tsve_alloc(current);\n> \n> \tlocal_bh_disable();\n> \tif (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {\n> \t\ttask_fpsimd_load(); /* flushes high Zn bits as a side-effect */\n> \t\tsve_flush_pregs();\n> \t} else {\n> \t\tsve_flush_all(); /* flush all the SVE bits in-place */\n> \t}\n> \n> \tif (test_and_set_thread_flag(TIF_SVE))\n> \t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> \tlocal_bh_enable();\n> }\n> \n> where sve_flush_all() zeroes all the high Zn bits via a series of\n> MOV Vn, Vn instructions, and also zeroes Pn and FFR. sve_fplush_pregs()\n> just does the latter.\n\nThis looks fine to me but I added a comment above. IIUC, we can now have\nTIF_SVE set while sve_state contains stale data. I don't see an issue\ngiven that every time you enter the kernel from user space you have\nTIF_SVE set and the sve_state storage out of sync. Maybe tweak the\nTIF_SVE description above slightly.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"tfCee5mc\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y7rY52MlSz9t3m\n\tfor ;\n\tSat, 7 Oct 2017 00:47:21 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0SyE-0000E3-Hl; Fri, 06 Oct 2017 13:47:18 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0SoK-0002A1-Dm for linux-arm-kernel@lists.infradead.org;\n\tFri, 06 Oct 2017 13:37:20 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B77101435;\n\tFri, 6 Oct 2017 06:36:44 -0700 (PDT)","from armageddon.cambridge.arm.com (armageddon.cambridge.arm.com\n\t[10.1.206.84])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tEAE8C3F578; Fri, 6 Oct 2017 06:36:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=F0OK+5RP+OJBtz6yAfO0mWKAj6S0Dp2MObC3yxjqU7o=;\n\tb=tfCee5mcWbP8TR\n\t1RGxbYEiYgAf5jyOfhVqDCcxZQ47iddH12CJbFYJf0cshRB+RKATCQnV314AW8N7EjM8WmIUrxjIy\n\t9gbEPgcCOhBEiLv+kteQl5uag73Y4+8weipHn9T0hfu4LkF1K/qav+F1+nwDgJsoOyO7NWBvUxyqi\n\t3JRSSqnm6mtmFJg1QOoNB3OsNnwFjLYYjzUV3CCdjfLgAXT6EZKvR963yLImLQLt9/INi4FBKMK8j\n\tiIo855pRp9F0KwpXKHwfKJq4q7HIi9Q5AzF+qDRh+yWD/aTixspW0R0U6aT3EnnWf9K7nmsF0R8ut\n\tWMEJdB/PZyK58k1R+BXQ==;","Date":"Fri, 6 Oct 2017 14:36:40 +0100","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171006133639.ialgglabo7yca5bm@armageddon.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20171003113302.GR3611@e103592.cambridge.arm.com>\n\t<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>\n\t<20171006131005.GA3611@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20171006131005.GA3611@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171006_063704_609697_151F0572 ","X-CRM114-Status":"GOOD ( 25.19 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1781720,"web_url":"http://patchwork.ozlabs.org/comment/1781720/","msgid":"<20171006151528.GB3611@e103592.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-06T15:15:28","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":26612,"url":"http://patchwork.ozlabs.org/api/people/26612/","name":"Dave Martin","email":"Dave.Martin@arm.com"},"content":"On Fri, Oct 06, 2017 at 02:36:40PM +0100, Catalin Marinas wrote:\n> On Fri, Oct 06, 2017 at 02:10:09PM +0100, Dave P Martin wrote:\n> > On Thu, Oct 05, 2017 at 12:28:35PM +0100, Catalin Marinas wrote:\n> > > On Tue, Oct 03, 2017 at 12:33:03PM +0100, Dave P Martin wrote:\n> > > > TIF_FOREIGN_FPSTATE's meaning is expanded to cover SVE, but otherwise\n> > > > unchanged:\n> > > > \n> > > > * If a task is running and !TIF_FOREIGN_FPSTATE, then the the CPU\n> > > > registers of the CPU the task is running on contain the authoritative\n> > > > FPSIMD/SVE state of the task. The backing memory may be stale.\n> > > > \n> > > > * Otherwise (i.e., task not running, or task running and\n> > > > TIF_FOREIGN_FPSTATE), the task's FPSIMD/SVE backing memory is\n> > > > authoritative. If additionally per_cpu(fpsimd_last_state,\n> > > > task->fpsimd_state.cpu) == &task->fpsimd_state.cpu, then\n> > > > task->fpsimd_state.cpu's registers are also up to date for task, but\n> > > > not authorititive: the current FPSIMD/SVE state may be read from\n> > > > them, but they must not be written.\n> > > > \n> > > > The FPSIMD/SVE backing memory is selected by TIF_SVE:\n> > > > \n> > > > * TIF_SVE set: Zn (incorporating Vn in bits[127:0]), Pn and FFR are\n> > > > stored in task->thread.sve_state, formatted appropriately for vector\n> > > > length task->thread.sve_vl. task->thread.sve_state must point to a\n> > > > valid buffer at least sve_state_size(task) bytes in size.\n> \n> \"Zn [...] stored in task->thread.sve_state\" - is this still true with\n> the changes you proposed? I guess even without these changes, you have\n> situations where the hardware regs are out of sync with sve_state (see\n> more below).\n\nI guess I need to tweak the wording here.\n\nTIF_SVE says where the vector state should be loaded/stored from,\nbut does not say whether the data is up to date in memory, or when\nit should be loaded/stored.\n\nThe latter is described by a cocktail of different things including\nwhich bit of kernel code we are executing if any, whether the task\nis running/stopped etc., TIF_FOREIGN_FPSTATE,\ntask->thread.fpsimd_state.cpu and per_cpu(fpsimd_last_state).\n\n\nDoes this make any better sense of my code below?\n\n> \n> > > > * TIF_SVE clear: Vn are stored in task->fpsimd_state; Zn[max : 128] are\n> > > > logically zero[*] but not stored anywhere; Pn, FFR are not stored and\n> > > > have unspecified values from userspace's point of view.\n> > > > task->thread.sve_state does not need to be non-null, valid or any\n> > > > particular size: it must not be dereferenced.\n> > > > \n> > > > In practice I don't exploit the \"unspecifiedness\" much. The Zn high\n> > > > bits, Pn and FFR are all zeroed when setting TIF_SVE again:\n> > > > sve_alloc() is the common path for this.\n> > > > \n> > > > * FPSR and FPCR are always stored in task->fpsimd_state irrespctive of\n> > > > whether TIF_SVE is clear or set, since these are not vector length\n> > > > dependent.\n> [...]\n> > > Just wondering, as an optimisation for do_sve_acc() - instead of\n> > > sve_alloc() and fpsimd_to_sve(), can we not force the loading of the\n> > > FPSIMD regs on the return to user via TIF_FOREIGN_FPSTATE? This would\n> > > ensure the zeroing of the top SVE bits and we only need to allocate the\n> > > SVE state on the saving path. This means enabling SVE for user and\n> > > setting TIF_SVE without having the backing storage allocated.\n> > \n> > Currently the set of places where the \"TIF_SVE implies sve_state valid\"\n> > assumption is applied is not very constrained, so while your suggestion\n> > is reasonable I'd rather not mess with it just now, if possible.\n> > \n> > \n> > But we can do this (which is what my current fixup has):\n> > \n> > el0_sve_acc:\n> > \tenable_dbg_and_irq\n> > \t// ...\n> > \tbl\tdo_sve_acc\n> > \tb\tret_to_user\n> > \n> > void do_sve_acc(unsigned int esr, struct pt_regs *regs)\n> > {\n> > \t/* Even if we chose not to use SVE, the hardware could still trap: */\n> > \tif (unlikely(!system_supports_sve()) || WARN_ON(is_compat_task())) {\n> > \t\tforce_signal_inject(SIGILL, ILL_ILLOPC, regs, 0);\n> > \t\treturn;\n> > \t}\n> > \n> > \tsve_alloc(current);\n> > \n> > \tlocal_bh_disable();\n> > \tif (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) {\n> > \t\ttask_fpsimd_load(); /* flushes high Zn bits as a side-effect */\n> > \t\tsve_flush_pregs();\n> > \t} else {\n> > \t\tsve_flush_all(); /* flush all the SVE bits in-place */\n> > \t}\n> > \n> > \tif (test_and_set_thread_flag(TIF_SVE))\n> > \t\tWARN_ON(1); /* SVE access shouldn't have trapped */\n> > \tlocal_bh_enable();\n> > }\n> > \n> > where sve_flush_all() zeroes all the high Zn bits via a series of\n> > MOV Vn, Vn instructions, and also zeroes Pn and FFR. sve_fplush_pregs()\n> > just does the latter.\n> \n> This looks fine to me but I added a comment above. IIUC, we can now have\n> TIF_SVE set while sve_state contains stale data. I don't see an issue\n> given that every time you enter the kernel from user space you have\n> TIF_SVE set and the sve_state storage out of sync. Maybe tweak the\n> TIF_SVE description above slightly.\n> \n\nSee my comment above ... any better?\n\nIf so, I'll paste some of that explanatory text into fpsimd.c (in lieu\nof a better place to put it).\n\nCheers\n---Dave","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"CVvaek7Z\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y7tWR1S84z9t3m\n\tfor ;\n\tSat, 7 Oct 2017 02:16:03 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0UM0-0002Xj-PD; Fri, 06 Oct 2017 15:15:56 +0000","from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]\n\thelo=foss.arm.com)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0ULx-0002VU-8f for linux-arm-kernel@lists.infradead.org;\n\tFri, 06 Oct 2017 15:15:55 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 07EE01435;\n\tFri, 6 Oct 2017 08:15:33 -0700 (PDT)","from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com\n\t[10.72.51.249])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t3B1653F53D; Fri, 6 Oct 2017 08:15:31 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=vlw32FILOfGVKZO4df1QR6r0SHyfAjDYXMiNzvtr8jw=;\n\tb=CVvaek7ZZNpc9V\n\tF9Bx0YOl32vqexyR2fjzFw+5xCqUQ/oeKm7vCyWoKQ5WCkKQx1Lfg3tC92z2xkJxAr+ueB3plO+6E\n\toLtqwBh8ayc4pzKnD5/UzRj817TG8s1MWGcByeRbq7/srmSsnJCVzlQu9O6JgXAwOGqKASrySvNTX\n\t8xOa+oyGopmUTSl83XHst15lR5Q/FkFsJBD6v48HfeFt/3NohIgbiCefiLVHGbtUlGQpepywlGXx/\n\tIAYzSZqXKTPNLpp5hzlVF9JAmjAMIgb5mh7nAzSNLNu2NVfcXHbDlfv2IWx+fvK8klezyZldSqEVq\n\t7qO4nm8LnjrN45RGOqdw==;","Date":"Fri, 6 Oct 2017 16:15:28 +0100","From":"Dave Martin ","To":"Catalin Marinas ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171006151528.GB3611@e103592.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20171003113302.GR3611@e103592.cambridge.arm.com>\n\t<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>\n\t<20171006131005.GA3611@e103592.cambridge.arm.com>\n\t<20171006133639.ialgglabo7yca5bm@armageddon.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20171006133639.ialgglabo7yca5bm@armageddon.cambridge.arm.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171006_081553_336321_9C89B5D4 ","X-CRM114-Status":"GOOD ( 30.54 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}},{"id":1781733,"web_url":"http://patchwork.ozlabs.org/comment/1781733/","msgid":"<20171006153312.cushhcb4m2qc4kwd@armageddon.cambridge.arm.com>","list_archive_url":null,"date":"2017-10-06T15:33:13","subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","submitter":{"id":938,"url":"http://patchwork.ozlabs.org/api/people/938/","name":"Catalin Marinas","email":"catalin.marinas@arm.com"},"content":"On Fri, Oct 06, 2017 at 04:15:28PM +0100, Dave P Martin wrote:\n> On Fri, Oct 06, 2017 at 02:36:40PM +0100, Catalin Marinas wrote:\n> > On Fri, Oct 06, 2017 at 02:10:09PM +0100, Dave P Martin wrote:\n> > > On Thu, Oct 05, 2017 at 12:28:35PM +0100, Catalin Marinas wrote:\n> > > > On Tue, Oct 03, 2017 at 12:33:03PM +0100, Dave P Martin wrote:\n> > > > > TIF_FOREIGN_FPSTATE's meaning is expanded to cover SVE, but otherwise\n> > > > > unchanged:\n> > > > > \n> > > > > * If a task is running and !TIF_FOREIGN_FPSTATE, then the the CPU\n> > > > > registers of the CPU the task is running on contain the authoritative\n> > > > > FPSIMD/SVE state of the task. The backing memory may be stale.\n> > > > > \n> > > > > * Otherwise (i.e., task not running, or task running and\n> > > > > TIF_FOREIGN_FPSTATE), the task's FPSIMD/SVE backing memory is\n> > > > > authoritative. If additionally per_cpu(fpsimd_last_state,\n> > > > > task->fpsimd_state.cpu) == &task->fpsimd_state.cpu, then\n> > > > > task->fpsimd_state.cpu's registers are also up to date for task, but\n> > > > > not authorititive: the current FPSIMD/SVE state may be read from\n> > > > > them, but they must not be written.\n> > > > > \n> > > > > The FPSIMD/SVE backing memory is selected by TIF_SVE:\n> > > > > \n> > > > > * TIF_SVE set: Zn (incorporating Vn in bits[127:0]), Pn and FFR are\n> > > > > stored in task->thread.sve_state, formatted appropriately for vector\n> > > > > length task->thread.sve_vl. task->thread.sve_state must point to a\n> > > > > valid buffer at least sve_state_size(task) bytes in size.\n> > \n> > \"Zn [...] stored in task->thread.sve_state\" - is this still true with\n> > the changes you proposed? I guess even without these changes, you have\n> > situations where the hardware regs are out of sync with sve_state (see\n> > more below).\n> \n> I guess I need to tweak the wording here.\n> \n> TIF_SVE says where the vector state should be loaded/stored from,\n> but does not say whether the data is up to date in memory, or when\n> it should be loaded/stored.\n> \n> The latter is described by a cocktail of different things including\n> which bit of kernel code we are executing if any, whether the task\n> is running/stopped etc., TIF_FOREIGN_FPSTATE,\n> task->thread.fpsimd_state.cpu and per_cpu(fpsimd_last_state).\n> \n> Does this make any better sense of my code below?\n\nYes, it looks fine.","headers":{"Return-Path":"","X-Original-To":"incoming-imx@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming-imx@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.infradead.org\n\t(client-ip=65.50.211.133; helo=bombadil.infradead.org;\n\tenvelope-from=linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org\n\theader.b=\"WMfPeaqx\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y7tvr4dJmz9t3m\n\tfor ;\n\tSat, 7 Oct 2017 02:33:44 +1100 (AEDT)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0Ud8-0002yX-A2; Fri, 06 Oct 2017 15:33:38 +0000","from foss.arm.com ([217.140.101.70])\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1e0Ud4-0002km-Cb for linux-arm-kernel@lists.infradead.org;\n\tFri, 06 Oct 2017 15:33:36 +0000","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9B39415A2;\n\tFri, 6 Oct 2017 08:33:17 -0700 (PDT)","from armageddon.cambridge.arm.com (armageddon.cambridge.arm.com\n\t[10.1.206.84])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\tCF7093F53D; Fri, 6 Oct 2017 08:33:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:\n\tMessage-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=Nrfhoy0LvwGLHDPfLQMRNxLEYe2zgY0ceDnOimxfn3w=;\n\tb=WMfPeaqxsf0xug\n\tjYDvjRbKYQ0FBki9MokJvjJ4NWBHGkMZtRbE12eQx6bXxX0BOtAHD3VYAWbOLQ9IOsSIQYm3wAFPH\n\tkytSodA+8Xohk5qQCfw1xA0ioj4/akheKA9CiI0b8a9H7ZTgXdnCxaqNtplqwaoLoUFVJ44ngl6EN\n\tghClduB83KCYBBST8isGpDodA2yhH38GUN5cF2anujqTK14kupJwQa+Uorn6VzESRdbMZ3uS28vzj\n\t/S3w0xpC+vOJwGrbHRYi3zpZ3pWuSE07rQYrmA3tHSZeAc7PwbkOoWtRnxA+8R/z94U1nWtN7e2Tw\n\t9se8kZ+gFJnurN9jZEAw==;","Date":"Fri, 6 Oct 2017 16:33:13 +0100","From":"Catalin Marinas ","To":"Dave Martin ","Subject":"Re: [PATCH v2 11/28] arm64/sve: Core task context handling","Message-ID":"<20171006153312.cushhcb4m2qc4kwd@armageddon.cambridge.arm.com>","References":"<1504198860-12951-1-git-send-email-Dave.Martin@arm.com>\n\t<1504198860-12951-12-git-send-email-Dave.Martin@arm.com>\n\t<20170913143325.hi4cfcajbts3bbao@localhost>\n\t<20171003113302.GR3611@e103592.cambridge.arm.com>\n\t<20171005112835.2rhaqx23bhe6tnwl@armageddon.cambridge.arm.com>\n\t<20171006131005.GA3611@e103592.cambridge.arm.com>\n\t<20171006133639.ialgglabo7yca5bm@armageddon.cambridge.arm.com>\n\t<20171006151528.GB3611@e103592.cambridge.arm.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20171006151528.GB3611@e103592.cambridge.arm.com>","User-Agent":"NeoMutt/20170113 (1.7.2)","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20171006_083334_472462_3D454274 ","X-CRM114-Status":"GOOD ( 20.72 )","X-Spam-Score":"-6.9 (------)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-6.9 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/,\n\thigh trust [217.140.101.70 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay\n\tdomain\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-arm-kernel@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-arch@vger.kernel.org, libc-alpha@sourceware.org, Ard Biesheuvel\n\t, Szabolcs Nagy ,\n\tRichard Sandiford , \n\tWill Deacon , Alex =?iso-8859-1?q?Benn=E9e?=\n\t, kvmarm@lists.cs.columbia.edu,\n\tlinux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-arm-kernel\" ","Errors-To":"linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org","List-Id":"linux-imx-kernel.lists.patchwork.ozlabs.org"}}]