From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v10 08/18] arm64/sve: Refactor user SVE trap maintenance for external use Date: Wed, 23 May 2018 21:16:52 +0100 Message-ID: <87h8myt9e3.fsf@linaro.org> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-9-git-send-email-Dave.Martin@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <1527005119-6842-9-git-send-email-Dave.Martin@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Dave Martin Cc: 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+IEluIHByZXBhcmF0 aW9uIGZvciBvcHRpbWlzaW5nIHRoZSB3YXkgS1ZNIG1hbmFnZXMgc3dpdGNoaW5nIHRoZQo+IGd1 ZXN0IGFuZCBob3N0IEZQU0lNRCBzdGF0ZSwgaXQgaXMgbmVjZXNzYXJ5IHRvIHByb3ZpZGUgYSBt ZWFucyBmb3IKPiBjb2RlIG91dHNpZGUgYXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMgdG8gcmVz dG9yZSB0aGUgdXNlciB0cmFwCj4gY29uZmlndXJhdGlvbiBmb3IgU1ZFIGNvcnJlY3RseSBmb3Ig dGhlIGN1cnJlbnQgdGFzay4KPgo+IFJhdGhlciB0aGFuIHJlcXVpcmluZyBleHRlcm5hbCBjb2Rl IHRvIGR1cGxpY2F0ZSB0aGUgbWFpbnRlbmFuY2UKPiBleHBsaWNpdGx5LCB0aGlzIHBhdGNoIHdy YXBzIG1vdmVzIHRoZSB0cmFwIG1haW50ZW5lbmFjZSB0bwo+IGZwc2ltZF9iaW5kX3RvX2NwdSgp LCBzaW5jZSBpdCBpcyBsb2dpY2FsbHkgcGFydCBvZiB0aGUgd29yayBvZgo+IGFzc29jaWF0aW5n IHRoZSBjdXJyZW50IHRhc2sgd2l0aCB0aGUgY3B1Lgo+Cj4gQmVjYXVzZSBmcHNpbWRfYmluZF90 b19jcHUoKSBpcyByYXRoZXIgYSBjcnlwdGljIG5hbWUgdG8gcHVibGlzaAo+IGFsb25nc2lkZSBm cHNpbWRfYmluZF9zdGF0ZV90b19jcHUoKSwgdGhlIGZvcm1lciBmdW5jdGlvbiBpcwo+IHJlbmFt ZWQgdG8gZnBzaW1kX2JpbmRfdGFza190b19jcHUoKSB0byBtYWtlIGl0cyBwdXJwb3NlIG1vcmUK PiBleHBsaWNpdC4KPgo+IFRoaXMgcGF0Y2ggbWFrZXMgYXBwcm9wcmlhdGUgY2hhbmdlcyB0byBl bnN1cmUgdGhhdAo+IGZwc2ltZF9iaW5kX3Rhc2tfdG9fY3B1KCkgaXMgYWx3YXlzIGNhbGxlZCBh bG9uZ3NpZGUKPiB0YXNrX2Zwc2ltZF9sb2FkKCksIHNvIHRoYXQgdGhlIHRyYXAgbWFpbnRlbmFu Y2UgY29udGludWVzIHRvIGJlCj4gZG9uZSBpbiBldmVyeSBzaXR1YXRpb24gd2hlcmUgaXQgd2Fz IGRvbmUgcHJpb3IgdG8gdGhpcyBwYXRjaC4KPgo+IEFzIGEgc2lkZS1lZmZlY3QsIHRoZSBtZXRh ZGF0YSB1cGRhdGVzIGRvbmUgYnkKPiBmcHNpbWRfYmluZF90YXNrX3RvX2NwdSgpIG5vdyBjaGFu Z2UgZnJvbSBjb25kaXRpb25hbCB0bwo+IHVuY29uZGl0aW9uYWwgaW4gdGhlICJhbHJlYWR5IGJv dW5kIiBjYXNlIG9mIHNpZ3JldHVybi4gIFRoaXMgaXMKPiBoYXJtbGVzcywgYW5kIGEgY291cGxl IG9mIGV4dHJhIHN0b3JlcyBvbiB0aGlzIHNsb3cgcGF0aCB3aWxsIG5vdAo+IGltcGFjdCBwZXJm b3JtYW5jZS4gIEkgY29uc2lkZXIgdGhpcyBhIHJlYXNvbmFibGUgcHJpY2UgdG8gcGF5IGZvcgo+ IGEgc2xpZ2h0bHkgY2xlYW5lciBpbnRlcmZhY2UuCj4KPiBTaWduZWQtb2ZmLWJ5OiBEYXZlIE1h cnRpbiA8RGF2ZS5NYXJ0aW5AYXJtLmNvbT4KPiBBY2tlZC1ieTogTWFyYyBaeW5naWVyIDxtYXJj Lnp5bmdpZXJAYXJtLmNvbT4KPiBBY2tlZC1ieTogQ2F0YWxpbiBNYXJpbmFzIDxjYXRhbGluLm1h cmluYXNAYXJtLmNvbT4KCkluIGZhY3QgdGhlIGNvbW1lbnQgSSBhbGx1ZGVkIHRvIGluIDYvMTgg Y291bGQgYmUgYXBwbGllZCBpbiB0aGlzLgoKUmV2aWV3ZWQtYnk6IEFsZXggQmVubsOpZSA8YWxl eC5iZW5uZWVAbGluYXJvLm9yZz4KCgo+IC0tLQo+ICBhcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQu YyB8IDI4ICsrKysrKysrKysrKysrLS0tLS0tLS0tLS0tLS0KPiAgMSBmaWxlIGNoYW5nZWQsIDE0 IGluc2VydGlvbnMoKyksIDE0IGRlbGV0aW9ucygtKQo+Cj4gZGlmZiAtLWdpdCBhL2FyY2gvYXJt NjQva2VybmVsL2Zwc2ltZC5jIGIvYXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMKPiBpbmRleCAx MjIyNDkxLi5iYTllN2RmIDEwMDY0NAo+IC0tLSBhL2FyY2gvYXJtNjQva2VybmVsL2Zwc2ltZC5j Cj4gKysrIGIvYXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMKPiBAQCAtMjU3LDE2ICsyNTcsNiBA QCBzdGF0aWMgdm9pZCB0YXNrX2Zwc2ltZF9sb2FkKHZvaWQpCj4gIAkJCSAgICAgICBzdmVfdnFf ZnJvbV92bChjdXJyZW50LT50aHJlYWQuc3ZlX3ZsKSAtIDEpOwo+ICAJZWxzZQo+ICAJCWZwc2lt ZF9sb2FkX3N0YXRlKCZjdXJyZW50LT50aHJlYWQudXcuZnBzaW1kX3N0YXRlKTsKPiAtCj4gLQlp ZiAoc3lzdGVtX3N1cHBvcnRzX3N2ZSgpKSB7Cj4gLQkJLyogVG9nZ2xlIFNWRSB0cmFwcGluZyBm b3IgdXNlcnNwYWNlIGlmIG5lZWRlZCAqLwo+IC0JCWlmICh0ZXN0X3RocmVhZF9mbGFnKFRJRl9T VkUpKQo+IC0JCQlzdmVfdXNlcl9lbmFibGUoKTsKPiAtCQllbHNlCj4gLQkJCXN2ZV91c2VyX2Rp c2FibGUoKTsKPiAtCj4gLQkJLyogU2VyaWFsaXNlZCBieSBleGNlcHRpb24gcmV0dXJuIHRvIHVz ZXIgKi8KPiAtCX0KPiAgfQo+Cj4gIC8qCj4gQEAgLTk5MSw3ICs5ODEsNyBAQCB2b2lkIGZwc2lt ZF9zaWduYWxfcHJlc2VydmVfY3VycmVudF9zdGF0ZSh2b2lkKQo+ICAgKiBBc3NvY2lhdGUgY3Vy cmVudCdzIEZQU0lNRCBjb250ZXh0IHdpdGggdGhpcyBjcHUKPiAgICogUHJlZW1wdGlvbiBtdXN0 IGJlIGRpc2FibGVkIHdoZW4gY2FsbGluZyB0aGlzIGZ1bmN0aW9uLgo+ICAgKi8KPiAtc3RhdGlj IHZvaWQgZnBzaW1kX2JpbmRfdG9fY3B1KHZvaWQpCj4gK3N0YXRpYyB2b2lkIGZwc2ltZF9iaW5k X3Rhc2tfdG9fY3B1KHZvaWQpCj4gIHsKPiAgCXN0cnVjdCBmcHNpbWRfbGFzdF9zdGF0ZV9zdHJ1 Y3QgKmxhc3QgPQo+ICAJCXRoaXNfY3B1X3B0cigmZnBzaW1kX2xhc3Rfc3RhdGUpOwo+IEBAIC05 OTksNiArOTg5LDE2IEBAIHN0YXRpYyB2b2lkIGZwc2ltZF9iaW5kX3RvX2NwdSh2b2lkKQo+ICAJ bGFzdC0+c3QgPSAmY3VycmVudC0+dGhyZWFkLnV3LmZwc2ltZF9zdGF0ZTsKPiAgCWxhc3QtPnN2 ZV9pbl91c2UgPSB0ZXN0X3RocmVhZF9mbGFnKFRJRl9TVkUpOwo+ICAJY3VycmVudC0+dGhyZWFk LmZwc2ltZF9jcHUgPSBzbXBfcHJvY2Vzc29yX2lkKCk7Cj4gKwo+ICsJaWYgKHN5c3RlbV9zdXBw b3J0c19zdmUoKSkgewo+ICsJCS8qIFRvZ2dsZSBTVkUgdHJhcHBpbmcgZm9yIHVzZXJzcGFjZSBp ZiBuZWVkZWQgKi8KPiArCQlpZiAodGVzdF90aHJlYWRfZmxhZyhUSUZfU1ZFKSkKPiArCQkJc3Zl X3VzZXJfZW5hYmxlKCk7Cj4gKwkJZWxzZQo+ICsJCQlzdmVfdXNlcl9kaXNhYmxlKCk7Cj4gKwo+ ICsJCS8qIFNlcmlhbGlzZWQgYnkgZXhjZXB0aW9uIHJldHVybiB0byB1c2VyICovCj4gKwl9Cj4g IH0KPgo+ICAvKgo+IEBAIC0xMDE1LDcgKzEwMTUsNyBAQCB2b2lkIGZwc2ltZF9yZXN0b3JlX2N1 cnJlbnRfc3RhdGUodm9pZCkKPgo+ICAJaWYgKHRlc3RfYW5kX2NsZWFyX3RocmVhZF9mbGFnKFRJ Rl9GT1JFSUdOX0ZQU1RBVEUpKSB7Cj4gIAkJdGFza19mcHNpbWRfbG9hZCgpOwo+IC0JCWZwc2lt ZF9iaW5kX3RvX2NwdSgpOwo+ICsJCWZwc2ltZF9iaW5kX3Rhc2tfdG9fY3B1KCk7Cj4gIAl9Cj4K PiAgCWxvY2FsX2JoX2VuYWJsZSgpOwo+IEBAIC0xMDM4LDkgKzEwMzgsOSBAQCB2b2lkIGZwc2lt ZF91cGRhdGVfY3VycmVudF9zdGF0ZShzdHJ1Y3QgdXNlcl9mcHNpbWRfc3RhdGUgY29uc3QgKnN0 YXRlKQo+ICAJCWZwc2ltZF90b19zdmUoY3VycmVudCk7Cj4KPiAgCXRhc2tfZnBzaW1kX2xvYWQo KTsKPiArCWZwc2ltZF9iaW5kX3Rhc2tfdG9fY3B1KCk7Cj4KPiAtCWlmICh0ZXN0X2FuZF9jbGVh cl90aHJlYWRfZmxhZyhUSUZfRk9SRUlHTl9GUFNUQVRFKSkKPiAtCQlmcHNpbWRfYmluZF90b19j cHUoKTsKPiArCWNsZWFyX3RocmVhZF9mbGFnKFRJRl9GT1JFSUdOX0ZQU1RBVEUpOwo+Cj4gIAls b2NhbF9iaF9lbmFibGUoKTsKPiAgfQoKCi0tCkFsZXggQmVubsOpZQoKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtlcm5lbCBtYWlsaW5n IGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5p bmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Wed, 23 May 2018 21:16:52 +0100 Subject: [PATCH v10 08/18] arm64/sve: Refactor user SVE trap maintenance for external use In-Reply-To: <1527005119-6842-9-git-send-email-Dave.Martin@arm.com> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-9-git-send-email-Dave.Martin@arm.com> Message-ID: <87h8myt9e3.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dave Martin writes: > In preparation for optimising the way KVM manages switching the > guest and host FPSIMD state, it is necessary to provide a means for > code outside arch/arm64/kernel/fpsimd.c to restore the user trap > configuration for SVE correctly for the current task. > > Rather than requiring external code to duplicate the maintenance > explicitly, this patch wraps moves the trap maintenenace to > fpsimd_bind_to_cpu(), since it is logically part of the work of > associating the current task with the cpu. > > Because fpsimd_bind_to_cpu() is rather a cryptic name to publish > alongside fpsimd_bind_state_to_cpu(), the former function is > renamed to fpsimd_bind_task_to_cpu() to make its purpose more > explicit. > > This patch makes appropriate changes to ensure that > fpsimd_bind_task_to_cpu() is always called alongside > task_fpsimd_load(), so that the trap maintenance continues to be > done in every situation where it was done prior to this patch. > > As a side-effect, the metadata updates done by > fpsimd_bind_task_to_cpu() now change from conditional to > unconditional in the "already bound" case of sigreturn. This is > harmless, and a couple of extra stores on this slow path will not > impact performance. I consider this a reasonable price to pay for > a slightly cleaner interface. > > Signed-off-by: Dave Martin > Acked-by: Marc Zyngier > Acked-by: Catalin Marinas In fact the comment I alluded to in 6/18 could be applied in this. Reviewed-by: Alex Benn?e > --- > arch/arm64/kernel/fpsimd.c | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 1222491..ba9e7df 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -257,16 +257,6 @@ static void task_fpsimd_load(void) > sve_vq_from_vl(current->thread.sve_vl) - 1); > else > fpsimd_load_state(¤t->thread.uw.fpsimd_state); > - > - if (system_supports_sve()) { > - /* Toggle SVE trapping for userspace if needed */ > - if (test_thread_flag(TIF_SVE)) > - sve_user_enable(); > - else > - sve_user_disable(); > - > - /* Serialised by exception return to user */ > - } > } > > /* > @@ -991,7 +981,7 @@ void fpsimd_signal_preserve_current_state(void) > * Associate current's FPSIMD context with this cpu > * Preemption must be disabled when calling this function. > */ > -static void fpsimd_bind_to_cpu(void) > +static void fpsimd_bind_task_to_cpu(void) > { > struct fpsimd_last_state_struct *last = > this_cpu_ptr(&fpsimd_last_state); > @@ -999,6 +989,16 @@ static void fpsimd_bind_to_cpu(void) > last->st = ¤t->thread.uw.fpsimd_state; > last->sve_in_use = test_thread_flag(TIF_SVE); > current->thread.fpsimd_cpu = smp_processor_id(); > + > + if (system_supports_sve()) { > + /* Toggle SVE trapping for userspace if needed */ > + if (test_thread_flag(TIF_SVE)) > + sve_user_enable(); > + else > + sve_user_disable(); > + > + /* Serialised by exception return to user */ > + } > } > > /* > @@ -1015,7 +1015,7 @@ void fpsimd_restore_current_state(void) > > if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) { > task_fpsimd_load(); > - fpsimd_bind_to_cpu(); > + fpsimd_bind_task_to_cpu(); > } > > local_bh_enable(); > @@ -1038,9 +1038,9 @@ void fpsimd_update_current_state(struct user_fpsimd_state const *state) > fpsimd_to_sve(current); > > task_fpsimd_load(); > + fpsimd_bind_task_to_cpu(); > > - if (test_and_clear_thread_flag(TIF_FOREIGN_FPSTATE)) > - fpsimd_bind_to_cpu(); > + clear_thread_flag(TIF_FOREIGN_FPSTATE); > > local_bh_enable(); > } -- Alex Benn?e