From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [RFC PATCH 01/16] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush Date: Fri, 06 Jul 2018 10:07:53 +0100 Message-ID: <87fu0w91g6.fsf@linaro.org> References: <1529593060-542-1-git-send-email-Dave.Martin@arm.com> <1529593060-542-2-git-send-email-Dave.Martin@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 3EFAD49F8C for ; Fri, 6 Jul 2018 04:55:24 -0400 (EDT) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id O2vqoMixTghU for ; Fri, 6 Jul 2018 04:55:23 -0400 (EDT) Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id E58A2408B1 for ; Fri, 6 Jul 2018 04:55:22 -0400 (EDT) Received: by mail-wm0-f65.google.com with SMTP id v16-v6so14151627wmv.5 for ; Fri, 06 Jul 2018 02:07:55 -0700 (PDT) In-reply-to: <1529593060-542-2-git-send-email-Dave.Martin@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Dave Martin Cc: Okamoto Takayuki , Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu CkRhdmUgTWFydGluIDxEYXZlLk1hcnRpbkBhcm0uY29tPiB3cml0ZXM6Cgo+IFRoaXMgcGF0Y2gg dXBkYXRlcyBmcHNpbWRfZmx1c2hfdGFza19zdGF0ZSgpIHRvIG1pcnJvciB0aGUgbmV3Cj4gc2Vt YW50aWNzIG9mIGZwc2ltZF9mbHVzaF9jcHVfc3RhdGUoKTogYm90aCBmdW5jdGlvbnMgbm93Cj4g aW1wbGljaXRseSBzZXQgVElGX0ZPUkVJR05fRlBTVEFURSB0byBpbmRpY2F0ZSB0aGF0IHRoZSB0 YXNrJ3MKPiBGUFNJTUQgc3RhdGUgaXMgbm90IGxvYWRlZCBpbnRvIHRoZSBjcHUuCj4KPiBBcyBh IHNpZGUtZWZmZWN0LCBmcHNpbWRfZmx1c2hfdGFza19zdGF0ZSgpIG5vdyBzZXRzCj4gVElGX0ZP UkVJR05fRlBTVEFURSBldmVuIGZvciBub24tcnVubmluZyB0YXNrcy4gIEluIHRoZSBjYXNlIG9m Cj4gbm9uLXJ1bm5pbmcgdGFza3MgdGhpcyBpcyBub3QgdXNlZnVsIGJ1dCBhbHNvIGhhcm1sZXNz LCBiZWNhdXNlIHRoZQo+IGZsYWcgaXMgbGl2ZSBvbmx5IHdoaWxlIHRoZSBjb3JyZXNwb25kaW5n IHRhc2sgaXMgcnVubmluZy4gIFRoaXMKPiBmdW5jdGlvbiBpcyBub3QgY2FsbGVkIGZyb20gZmFz dCBwYXRocywgc28gc3BlY2lhbC1jYXNpbmcgdGhpcyBmb3IKPiB0aGUgdGFzayA9PSBjdXJyZW50 IGNhc2UgaXMgbm90IHJlYWxseSB3b3J0aCBpdC4KPgo+IENvbXBpbGVyIGJhcnJpZXJzIHByZXZp b3VzbHkgcHJlc2VudCBpbiByZXN0b3JlX3N2ZV9mcHNpbWRfY29udGV4dCgpCj4gYXJlIHB1bGxl ZCBpbnRvIGZwc2ltZF9mbHVzaF90YXNrX3N0YXRlKCkgc28gdGhhdCBpdCBjYW4gYmUgc2FmZWx5 Cj4gY2FsbGVkIHdpdGggcHJlZW1wdGlvbiBlbmFibGVkIGlmIG5lY2Vzc2FyeS4KPgo+IEV4cGxp Y2l0IGNhbGxzIHRvIHNldCBUSUZfRk9SRUlHTl9GUFNUQVRFIHRoYXQgYWNjb21wYW55Cj4gZnBz aW1kX2ZsdXNoX3Rhc2tfc3RhdGUoKSBjYWxscyBhbmQgYXJlIG5vdyByZWR1bmRhbnQgYXJlIHJl bW92ZWQKPiBhcyBhcHByb3ByaWF0ZS4KPgo+IGZwc2ltZF9mbHVzaF90YXNrX3N0YXRlKCkgaXMg dXNlZCB0byBnZXQgZXhjbHVzaXZlIGFjY2VzcyB0byB0aGUKPiByZXByZXNlbnRhdGlvbiBvZiB0 aGUgdGFzaydzIHN0YXRlIHZpYSB0YXNrX3N0cnVjdCwgZm9yIHRoZSBwdXJwb3NlCj4gb2YgcmVw bGFjaW5nIHRoZSBzdGF0ZS4gIFRodXMsIHRoZSBjYWxsIHRvIHRoaXMgZnVuY3Rpb24gc2hvdWxk Cj4gaGFwcGVuIGJlZm9yZSBtYW5pcHVsYXRpbmcgZnBzaW1kX3N0YXRlIG9yIHN2ZV9zdGF0ZSBl dGMuIGluCj4gdGFza19zdHJ1Y3QuICBBbm9tYWxvdXMgY2FzZXMgYXJlIHJlb3JkZXJlZCBhcHBy b3ByaWF0ZWx5IGluIG9yZGVyCj4gdG8gbWFrZSB0aGUgY29kZSBtb3JlIGNvbnNpc3RlbnQsIGFs dGhvdWdoIHRoZXJlIHNob3VsZCBiZSBubwo+IGZ1bmN0aW9uYWwgZGlmZmVyZW5jZSBzaW5jZSB0 aGVzZSBjYXNlcyBhcmUgcHJvdGVjdGVkIGJ5Cj4gbG9jYWxfYmhfZGlzYWJsZSgpIGFueXdheS4K Pgo+IFNpZ25lZC1vZmYtYnk6IERhdmUgTWFydGluIDxEYXZlLk1hcnRpbkBhcm0uY29tPgoKUmV2 aWV3ZWQtYnk6IEFsZXggQmVubsOpZSA8YWxleC5iZW5uZWVAbGluYXJvLm9yZz4KCj4gLS0tCj4g IGFyY2gvYXJtNjQva2VybmVsL2Zwc2ltZC5jIHwgMjUgKysrKysrKysrKysrKysrKysrKy0tLS0t LQo+ICBhcmNoL2FybTY0L2tlcm5lbC9zaWduYWwuYyB8ICA1IC0tLS0tCj4gIDIgZmlsZXMgY2hh bmdlZCwgMTkgaW5zZXJ0aW9ucygrKSwgMTEgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEv YXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMgYi9hcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYwo+ IGluZGV4IDg0YzY4YjEuLjZiMWRkYWUgMTAwNjQ0Cj4gLS0tIGEvYXJjaC9hcm02NC9rZXJuZWwv ZnBzaW1kLmMKPiArKysgYi9hcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYwo+IEBAIC01NjksNyAr NTY5LDYgQEAgaW50IHN2ZV9zZXRfdmVjdG9yX2xlbmd0aChzdHJ1Y3QgdGFza19zdHJ1Y3QgKnRh c2ssCj4gIAkJbG9jYWxfYmhfZGlzYWJsZSgpOwo+Cj4gIAkJZnBzaW1kX3NhdmUoKTsKPiAtCQlz ZXRfdGhyZWFkX2ZsYWcoVElGX0ZPUkVJR05fRlBTVEFURSk7Cj4gIAl9Cj4KPiAgCWZwc2ltZF9m bHVzaF90YXNrX3N0YXRlKHRhc2spOwo+IEBAIC04MzUsMTIgKzgzNCwxMSBAQCBhc21saW5rYWdl IHZvaWQgZG9fc3ZlX2FjYyh1bnNpZ25lZCBpbnQgZXNyLCBzdHJ1Y3QgcHRfcmVncyAqcmVncykK PiAgCWxvY2FsX2JoX2Rpc2FibGUoKTsKPgo+ICAJZnBzaW1kX3NhdmUoKTsKPiAtCWZwc2ltZF90 b19zdmUoY3VycmVudCk7Cj4KPiAgCS8qIEZvcmNlIHJldF90b191c2VyIHRvIHJlbG9hZCB0aGUg cmVnaXN0ZXJzOiAqLwo+ICAJZnBzaW1kX2ZsdXNoX3Rhc2tfc3RhdGUoY3VycmVudCk7Cj4gLQlz ZXRfdGhyZWFkX2ZsYWcoVElGX0ZPUkVJR05fRlBTVEFURSk7Cj4KPiArCWZwc2ltZF90b19zdmUo Y3VycmVudCk7Cj4gIAlpZiAodGVzdF9hbmRfc2V0X3RocmVhZF9mbGFnKFRJRl9TVkUpKQo+ICAJ CVdBUk5fT04oMSk7IC8qIFNWRSBhY2Nlc3Mgc2hvdWxkbid0IGhhdmUgdHJhcHBlZCAqLwo+Cj4g QEAgLTkxNyw5ICs5MTUsOSBAQCB2b2lkIGZwc2ltZF9mbHVzaF90aHJlYWQodm9pZCkKPgo+ICAJ bG9jYWxfYmhfZGlzYWJsZSgpOwo+Cj4gKwlmcHNpbWRfZmx1c2hfdGFza19zdGF0ZShjdXJyZW50 KTsKPiAgCW1lbXNldCgmY3VycmVudC0+dGhyZWFkLnV3LmZwc2ltZF9zdGF0ZSwgMCwKPiAgCSAg ICAgICBzaXplb2YoY3VycmVudC0+dGhyZWFkLnV3LmZwc2ltZF9zdGF0ZSkpOwo+IC0JZnBzaW1k X2ZsdXNoX3Rhc2tfc3RhdGUoY3VycmVudCk7Cj4KPiAgCWlmIChzeXN0ZW1fc3VwcG9ydHNfc3Zl KCkpIHsKPiAgCQljbGVhcl90aHJlYWRfZmxhZyhUSUZfU1ZFKTsKPiBAQCAtOTU2LDggKzk1NCw2 IEBAIHZvaWQgZnBzaW1kX2ZsdXNoX3RocmVhZCh2b2lkKQo+ICAJCQljdXJyZW50LT50aHJlYWQu c3ZlX3ZsX29uZXhlYyA9IDA7Cj4gIAl9Cj4KPiAtCXNldF90aHJlYWRfZmxhZyhUSUZfRk9SRUlH Tl9GUFNUQVRFKTsKPiAtCj4gIAlsb2NhbF9iaF9lbmFibGUoKTsKPiAgfQo+Cj4gQEAgLTEwNjYs MTIgKzEwNjIsMjkgQEAgdm9pZCBmcHNpbWRfdXBkYXRlX2N1cnJlbnRfc3RhdGUoc3RydWN0IHVz ZXJfZnBzaW1kX3N0YXRlIGNvbnN0ICpzdGF0ZSkKPgo+ICAvKgo+ICAgKiBJbnZhbGlkYXRlIGxp dmUgQ1BVIGNvcGllcyBvZiB0YXNrIHQncyBGUFNJTUQgc3RhdGUKPiArICoKPiArICogVGhpcyBm dW5jdGlvbiBtYXkgYmUgY2FsbGVkIHdpdGggcHJlZW1wdGlvbiBlbmFibGVkLiAgVGhlIGJhcnJp ZXIoKQo+ICsgKiBlbnN1cmVzIHRoYXQgdGhlIGFzc2lnbm1lbnQgdG8gZnBzaW1kX2NwdSBpcyB2 aXNpYmxlIHRvIGFueQo+ICsgKiBwcmVlbXB0aW9uL3NvZnRpcnEgdGhhdCBjb3VsZCByYWNlIHdp dGggc2V0X3Rza190aHJlYWRfZmxhZygpLCBzbwo+ICsgKiB0aGF0IFRJRl9GT1JFSUdOX0ZQU1RB VEUgY2Fubm90IGJlIHNwdXJpb3VzbHkgcmUtY2xlYXJlZC4KPiArICoKPiArICogVGhlIGZpbmFs IGJhcnJpZXIgZW5zdXJlcyB0aGF0IFRJRl9GT1JFSUdOX0ZQU1RBVEUgaXMgc2VlbiBzZXQgYnkg YW55Cj4gKyAqIHN1YnNlcXVlbnQgY29kZS4KPiAgICovCj4gIHZvaWQgZnBzaW1kX2ZsdXNoX3Rh c2tfc3RhdGUoc3RydWN0IHRhc2tfc3RydWN0ICp0KQo+ICB7Cj4gIAl0LT50aHJlYWQuZnBzaW1k X2NwdSA9IE5SX0NQVVM7Cj4gKwo+ICsJYmFycmllcigpOwo+ICsJc2V0X3Rza190aHJlYWRfZmxh Zyh0LCBUSUZfRk9SRUlHTl9GUFNUQVRFKTsKPiArCj4gKwliYXJyaWVyKCk7Cj4gIH0KPgo+ICsv Kgo+ICsgKiBJbnZhbGlkYXRlIGFueSB0YXNrJ3MgRlBTSU1EIHN0YXRlIHRoYXQgaXMgcHJlc2Vu dCBvbiB0aGlzIGNwdS4KPiArICogVGhpcyBmdW5jdGlvbiBtdXN0IGJlIGNhbGxlZCB3aXRoIHNv ZnRpcnFzIGRpc2FibGVkLgo+ICsgKi8KPiAgdm9pZCBmcHNpbWRfZmx1c2hfY3B1X3N0YXRlKHZv aWQpCj4gIHsKPiAgCV9fdGhpc19jcHVfd3JpdGUoZnBzaW1kX2xhc3Rfc3RhdGUuc3QsIE5VTEwp Owo+IGRpZmYgLS1naXQgYS9hcmNoL2FybTY0L2tlcm5lbC9zaWduYWwuYyBiL2FyY2gvYXJtNjQv a2VybmVsL3NpZ25hbC5jCj4gaW5kZXggNTExYWYxMy4uNzYzNjk2NSAxMDA2NDQKPiAtLS0gYS9h cmNoL2FybTY0L2tlcm5lbC9zaWduYWwuYwo+ICsrKyBiL2FyY2gvYXJtNjQva2VybmVsL3NpZ25h bC5jCj4gQEAgLTI5NiwxMSArMjk2LDYgQEAgc3RhdGljIGludCByZXN0b3JlX3N2ZV9mcHNpbWRf Y29udGV4dChzdHJ1Y3QgdXNlcl9jdHhzICp1c2VyKQo+ICAJICovCj4KPiAgCWZwc2ltZF9mbHVz aF90YXNrX3N0YXRlKGN1cnJlbnQpOwo+IC0JYmFycmllcigpOwo+IC0JLyogRnJvbSBub3csIGZw c2ltZF90aHJlYWRfc3dpdGNoKCkgd29uJ3QgY2xlYXIgVElGX0ZPUkVJR05fRlBTVEFURSAqLwo+ IC0KPiAtCXNldF90aHJlYWRfZmxhZyhUSUZfRk9SRUlHTl9GUFNUQVRFKTsKPiAtCWJhcnJpZXIo KTsKPiAgCS8qIEZyb20gbm93LCBmcHNpbWRfdGhyZWFkX3N3aXRjaCgpIHdvbid0IHRvdWNoIHRo cmVhZC5zdmVfc3RhdGUgKi8KPgo+ICAJc3ZlX2FsbG9jKGN1cnJlbnQpOwoKCi0tCkFsZXggQmVu bsOpZQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwprdm1h cm0gbWFpbGluZyBsaXN0Cmt2bWFybUBsaXN0cy5jcy5jb2x1bWJpYS5lZHUKaHR0cHM6Ly9saXN0 cy5jcy5jb2x1bWJpYS5lZHUvbWFpbG1hbi9saXN0aW5mby9rdm1hcm0K From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Fri, 06 Jul 2018 10:07:53 +0100 Subject: [RFC PATCH 01/16] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush In-Reply-To: <1529593060-542-2-git-send-email-Dave.Martin@arm.com> References: <1529593060-542-1-git-send-email-Dave.Martin@arm.com> <1529593060-542-2-git-send-email-Dave.Martin@arm.com> Message-ID: <87fu0w91g6.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dave Martin writes: > This patch updates fpsimd_flush_task_state() to mirror the new > semantics of fpsimd_flush_cpu_state(): both functions now > implicitly set TIF_FOREIGN_FPSTATE to indicate that the task's > FPSIMD state is not loaded into the cpu. > > As a side-effect, fpsimd_flush_task_state() now sets > TIF_FOREIGN_FPSTATE even for non-running tasks. In the case of > non-running tasks this is not useful but also harmless, because the > flag is live only while the corresponding task is running. This > function is not called from fast paths, so special-casing this for > the task == current case is not really worth it. > > Compiler barriers previously present in restore_sve_fpsimd_context() > are pulled into fpsimd_flush_task_state() so that it can be safely > called with preemption enabled if necessary. > > Explicit calls to set TIF_FOREIGN_FPSTATE that accompany > fpsimd_flush_task_state() calls and are now redundant are removed > as appropriate. > > fpsimd_flush_task_state() is used to get exclusive access to the > representation of the task's state via task_struct, for the purpose > of replacing the state. Thus, the call to this function should > happen before manipulating fpsimd_state or sve_state etc. in > task_struct. Anomalous cases are reordered appropriately in order > to make the code more consistent, although there should be no > functional difference since these cases are protected by > local_bh_disable() anyway. > > Signed-off-by: Dave Martin Reviewed-by: Alex Benn?e > --- > arch/arm64/kernel/fpsimd.c | 25 +++++++++++++++++++------ > arch/arm64/kernel/signal.c | 5 ----- > 2 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 84c68b1..6b1ddae 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -569,7 +569,6 @@ int sve_set_vector_length(struct task_struct *task, > local_bh_disable(); > > fpsimd_save(); > - set_thread_flag(TIF_FOREIGN_FPSTATE); > } > > fpsimd_flush_task_state(task); > @@ -835,12 +834,11 @@ asmlinkage void do_sve_acc(unsigned int esr, struct pt_regs *regs) > local_bh_disable(); > > fpsimd_save(); > - fpsimd_to_sve(current); > > /* Force ret_to_user to reload the registers: */ > fpsimd_flush_task_state(current); > - set_thread_flag(TIF_FOREIGN_FPSTATE); > > + fpsimd_to_sve(current); > if (test_and_set_thread_flag(TIF_SVE)) > WARN_ON(1); /* SVE access shouldn't have trapped */ > > @@ -917,9 +915,9 @@ void fpsimd_flush_thread(void) > > local_bh_disable(); > > + fpsimd_flush_task_state(current); > memset(¤t->thread.uw.fpsimd_state, 0, > sizeof(current->thread.uw.fpsimd_state)); > - fpsimd_flush_task_state(current); > > if (system_supports_sve()) { > clear_thread_flag(TIF_SVE); > @@ -956,8 +954,6 @@ void fpsimd_flush_thread(void) > current->thread.sve_vl_onexec = 0; > } > > - set_thread_flag(TIF_FOREIGN_FPSTATE); > - > local_bh_enable(); > } > > @@ -1066,12 +1062,29 @@ void fpsimd_update_current_state(struct user_fpsimd_state const *state) > > /* > * Invalidate live CPU copies of task t's FPSIMD state > + * > + * This function may be called with preemption enabled. The barrier() > + * ensures that the assignment to fpsimd_cpu is visible to any > + * preemption/softirq that could race with set_tsk_thread_flag(), so > + * that TIF_FOREIGN_FPSTATE cannot be spuriously re-cleared. > + * > + * The final barrier ensures that TIF_FOREIGN_FPSTATE is seen set by any > + * subsequent code. > */ > void fpsimd_flush_task_state(struct task_struct *t) > { > t->thread.fpsimd_cpu = NR_CPUS; > + > + barrier(); > + set_tsk_thread_flag(t, TIF_FOREIGN_FPSTATE); > + > + barrier(); > } > > +/* > + * Invalidate any task's FPSIMD state that is present on this cpu. > + * This function must be called with softirqs disabled. > + */ > void fpsimd_flush_cpu_state(void) > { > __this_cpu_write(fpsimd_last_state.st, NULL); > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index 511af13..7636965 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -296,11 +296,6 @@ static int restore_sve_fpsimd_context(struct user_ctxs *user) > */ > > fpsimd_flush_task_state(current); > - barrier(); > - /* From now, fpsimd_thread_switch() won't clear TIF_FOREIGN_FPSTATE */ > - > - set_thread_flag(TIF_FOREIGN_FPSTATE); > - barrier(); > /* From now, fpsimd_thread_switch() won't touch thread.sve_state */ > > sve_alloc(current); -- Alex Benn?e