From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v10 06/18] arm64: fpsimd: Generalise context saving for non-task contexts Date: Wed, 23 May 2018 21:15:11 +0100 Message-ID: <87in7et9gw.fsf@linaro.org> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-7-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-7-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 aW9uIGZvciBhbGxvd2luZyBub24tdGFzayAoaS5lLiwgS1ZNIHZjcHUpIEZQU0lNRAo+IGNvbnRl eHRzIHRvIGJlIGhhbmRsZWQgYnkgdGhlIGZwc2ltZCBjb21tb24gY29kZSwgdGhpcyBwYXRjaCBh ZGFwdHMKPiB0YXNrX2Zwc2ltZF9zYXZlKCkgdG8gc2F2ZSBiYWNrIHRoZSBjdXJyZW50bHkgbG9h ZGVkIGNvbnRleHQsCj4gcmVtb3ZpbmcgdGhlIGV4cGxpY2l0IGRlcGVuZGVuY3kgb24gY3VycmVu dC4KPgo+IFRoZSByZWxldmFudCBzdG9yYWdlIHRvIHdyaXRlIGJhY2sgdG8gaW4gbWVtb3J5IGlz IG5vdyBmb3VuZCBieQo+IGV4YW1pbmluZyB0aGUgZnBzaW1kX2xhc3Rfc3RhdGUgcGVyY3B1IHN0 cnVjdC4KPgo+IGZwc2ltZF9zYXZlKCkgZG9lcyBub3RoaW5nIHVubGVzcyBUSUZfRk9SRUlHTl9G UFNUQVRFIGlzIGNsZWFyLCBhbmQKPiBmcHNpbWRfbGFzdF9zdGF0ZSBpcyB1cGRhdGVkIHVuZGVy IGxvY2FsX2JoX2Rpc2FibGUoKSBvcgo+IGxvY2FsX2lycV9kaXNhYmxlKCkgZXZlcnl3aGVyZSB0 aGF0IFRJRl9GT1JFSUdOX0ZQU1RBVEUgaXMgY2xlYXJlZDoKPiB0aHVzLCBmcHNpbWRfc2F2ZSgp IHdpbGwgd3JpdGUgYmFjayB0byB0aGUgY29ycmVjdCBzdG9yYWdlIGZvciB0aGUKPiBsb2FkZWQg Y29udGV4dC4KPgo+IE5vIGZ1bmN0aW9uYWwgY2hhbmdlLgo+Cj4gU2lnbmVkLW9mZi1ieTogRGF2 ZSBNYXJ0aW4gPERhdmUuTWFydGluQGFybS5jb20+Cj4gQWNrZWQtYnk6IE1hcmMgWnluZ2llciA8 bWFyYy56eW5naWVyQGFybS5jb20+Cj4gQWNrZWQtYnk6IENhdGFsaW4gTWFyaW5hcyA8Y2F0YWxp bi5tYXJpbmFzQGFybS5jb20+Cj4gLS0tCj4gIGFyY2gvYXJtNjQva2VybmVsL2Zwc2ltZC5jIHwg MjUgKysrKysrKysrKysrKy0tLS0tLS0tLS0tLQo+ICAxIGZpbGUgY2hhbmdlZCwgMTMgaW5zZXJ0 aW9ucygrKSwgMTIgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvYXJjaC9hcm02NC9rZXJu ZWwvZnBzaW1kLmMgYi9hcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYwo+IGluZGV4IDlkODUzNzMu LjNhYTEwMGEgMTAwNjQ0Cj4gLS0tIGEvYXJjaC9hcm02NC9rZXJuZWwvZnBzaW1kLmMKPiArKysg Yi9hcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYwo+IEBAIC0yNzAsMTMgKzI3MCwxNSBAQCBzdGF0 aWMgdm9pZCB0YXNrX2Zwc2ltZF9sb2FkKHZvaWQpCj4gIH0KPgo+ICAvKgo+IC0gKiBFbnN1cmUg Y3VycmVudCdzIEZQU0lNRC9TVkUgc3RvcmFnZSBpbiB0aHJlYWRfc3RydWN0IGlzIHVwIHRvIGRh dGUKPiAtICogd2l0aCByZXNwZWN0IHRvIHRoZSBDUFUgcmVnaXN0ZXJzLgo+ICsgKiBFbnN1cmUg RlBTSU1EL1NWRSBzdG9yYWdlIGluIG1lbW9yeSBmb3IgdGhlIGxvYWRlZCBjb250ZXh0IGlzIHVw IHRvCj4gKyAqIGRhdGUgd2l0aCByZXNwZWN0IHRvIHRoZSBDUFUgcmVnaXN0ZXJzLgo+ICAgKgo+ ICAgKiBTb2Z0aXJxcyAoYW5kIHByZWVtcHRpb24pIG11c3QgYmUgZGlzYWJsZWQuCj4gICAqLwo+ IC1zdGF0aWMgdm9pZCB0YXNrX2Zwc2ltZF9zYXZlKHZvaWQpCj4gK3N0YXRpYyB2b2lkIGZwc2lt ZF9zYXZlKHZvaWQpCj4gIHsKPiArCXN0cnVjdCB1c2VyX2Zwc2ltZF9zdGF0ZSAqc3QgPSBfX3Ro aXNfY3B1X3JlYWQoZnBzaW1kX2xhc3Rfc3RhdGUuc3QpOwo+ICsKCkkgdGhvdWdodCBJIHdhcyBt aXNzaW5nIHNvbWV0aGluZyBidXQgdGhlIG9ubHkgd3JpdGUgSSBzYXcgb2YgdGhpcyB3YXM6Cgog IF9fdGhpc19jcHVfd3JpdGUoZnBzaW1kX2xhc3Rfc3RhdGUuc3QsIE5VTEwpOwoKd2hpY2ggaW1w bGllZCB0byBtZSBpdCBpcyBwb3NzaWJsZSB0byBoYXZlIGFuIGludmFsaWQgZGUtcmVmZXJlbmNl LiBJCmRpZCBmaWd1cmUgaXQgb3V0IGV2ZW50dWFsbHkgYXMgZnBzaW1kX2JpbmRfc3RhdGVfdG9f Y3B1IHVzZXMgYSBtb3JlCmluZGlyZWN0IHRoaXNfY3B1X3B0ciBpZGlvbSBmb3IgdHdlYWtpbmcg dGhpcy4gSSBndWVzcyBhIHJlZmVyZW5jZSB0bwpmcHNpbWRfYmluZF9bdGFza3xzdGF0ZV1fdG9f Y3B1IGluIHRoZSBjb21tZW50IHdvdWxkIGhhdmUgaGVscGVkIG15CmNvbmZ1c2lvbi4KCkFueXdh eToKClJldmlld2VkLWJ5OiBBbGV4IEJlbm7DqWUgPGFsZXguYmVubmVlQGxpbmFyby5vcmc+CgoK PiAgCVdBUk5fT04oIWluX3NvZnRpcnEoKSAmJiAhaXJxc19kaXNhYmxlZCgpKTsKPgo+ICAJaWYg KCF0ZXN0X3RocmVhZF9mbGFnKFRJRl9GT1JFSUdOX0ZQU1RBVEUpKSB7Cj4gQEAgLTI5MSwxMCAr MjkzLDkgQEAgc3RhdGljIHZvaWQgdGFza19mcHNpbWRfc2F2ZSh2b2lkKQo+ICAJCQkJcmV0dXJu Owo+ICAJCQl9Cj4KPiAtCQkJc3ZlX3NhdmVfc3RhdGUoc3ZlX3BmZnIoY3VycmVudCksCj4gLQkJ CQkgICAgICAgJmN1cnJlbnQtPnRocmVhZC51dy5mcHNpbWRfc3RhdGUuZnBzcik7Cj4gKwkJCXN2 ZV9zYXZlX3N0YXRlKHN2ZV9wZmZyKGN1cnJlbnQpLCAmc3QtPmZwc3IpOwo+ICAJCX0gZWxzZQo+ IC0JCQlmcHNpbWRfc2F2ZV9zdGF0ZSgmY3VycmVudC0+dGhyZWFkLnV3LmZwc2ltZF9zdGF0ZSk7 Cj4gKwkJCWZwc2ltZF9zYXZlX3N0YXRlKHN0KTsKPiAgCX0KPiAgfQo+Cj4gQEAgLTU5OCw3ICs1 OTksNyBAQCBpbnQgc3ZlX3NldF92ZWN0b3JfbGVuZ3RoKHN0cnVjdCB0YXNrX3N0cnVjdCAqdGFz aywKPiAgCWlmICh0YXNrID09IGN1cnJlbnQpIHsKPiAgCQlsb2NhbF9iaF9kaXNhYmxlKCk7Cj4K PiAtCQl0YXNrX2Zwc2ltZF9zYXZlKCk7Cj4gKwkJZnBzaW1kX3NhdmUoKTsKPiAgCQlzZXRfdGhy ZWFkX2ZsYWcoVElGX0ZPUkVJR05fRlBTVEFURSk7Cj4gIAl9Cj4KPiBAQCAtODM3LDcgKzgzOCw3 IEBAIGFzbWxpbmthZ2Ugdm9pZCBkb19zdmVfYWNjKHVuc2lnbmVkIGludCBlc3IsIHN0cnVjdCBw dF9yZWdzICpyZWdzKQo+Cj4gIAlsb2NhbF9iaF9kaXNhYmxlKCk7Cj4KPiAtCXRhc2tfZnBzaW1k X3NhdmUoKTsKPiArCWZwc2ltZF9zYXZlKCk7Cj4gIAlmcHNpbWRfdG9fc3ZlKGN1cnJlbnQpOwo+ Cj4gIAkvKiBGb3JjZSByZXRfdG9fdXNlciB0byByZWxvYWQgdGhlIHJlZ2lzdGVyczogKi8KPiBA QCAtODk4LDcgKzg5OSw3IEBAIHZvaWQgZnBzaW1kX3RocmVhZF9zd2l0Y2goc3RydWN0IHRhc2tf c3RydWN0ICpuZXh0KQo+ICAJICogJ2N1cnJlbnQnLgo+ICAJICovCj4gIAlpZiAoY3VycmVudC0+ bW0pCj4gLQkJdGFza19mcHNpbWRfc2F2ZSgpOwo+ICsJCWZwc2ltZF9zYXZlKCk7Cj4KPiAgCWlm IChuZXh0LT5tbSkgewo+ICAJCS8qCj4gQEAgLTk4MCw3ICs5ODEsNyBAQCB2b2lkIGZwc2ltZF9w cmVzZXJ2ZV9jdXJyZW50X3N0YXRlKHZvaWQpCj4gIAkJcmV0dXJuOwo+Cj4gIAlsb2NhbF9iaF9k aXNhYmxlKCk7Cj4gLQl0YXNrX2Zwc2ltZF9zYXZlKCk7Cj4gKwlmcHNpbWRfc2F2ZSgpOwo+ICAJ bG9jYWxfYmhfZW5hYmxlKCk7Cj4gIH0KPgo+IEBAIC0xMTIxLDcgKzExMjIsNyBAQCB2b2lkIGtl cm5lbF9uZW9uX2JlZ2luKHZvaWQpCj4KPiAgCS8qIFNhdmUgdW5zYXZlZCB0YXNrIGZwc2ltZCBz dGF0ZSwgaWYgYW55OiAqLwo+ICAJaWYgKGN1cnJlbnQtPm1tKQo+IC0JCXRhc2tfZnBzaW1kX3Nh dmUoKTsKPiArCQlmcHNpbWRfc2F2ZSgpOwo+Cj4gIAkvKiBJbnZhbGlkYXRlIGFueSB0YXNrIHN0 YXRlIHJlbWFpbmluZyBpbiB0aGUgZnBzaW1kIHJlZ3M6ICovCj4gIAlmcHNpbWRfZmx1c2hfY3B1 X3N0YXRlKCk7Cj4gQEAgLTEyNDQsNyArMTI0NSw3IEBAIHN0YXRpYyBpbnQgZnBzaW1kX2NwdV9w bV9ub3RpZmllcihzdHJ1Y3Qgbm90aWZpZXJfYmxvY2sgKnNlbGYsCj4gIAlzd2l0Y2ggKGNtZCkg ewo+ICAJY2FzZSBDUFVfUE1fRU5URVI6Cj4gIAkJaWYgKGN1cnJlbnQtPm1tKQo+IC0JCQl0YXNr X2Zwc2ltZF9zYXZlKCk7Cj4gKwkJCWZwc2ltZF9zYXZlKCk7Cj4gIAkJZnBzaW1kX2ZsdXNoX2Nw dV9zdGF0ZSgpOwo+ICAJCWJyZWFrOwo+ICAJY2FzZSBDUFVfUE1fRVhJVDoKCgotLQpBbGV4IEJl bm7DqWUKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxp bnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFk ZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4 LWFybS1rZXJuZWwK 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:15:11 +0100 Subject: [PATCH v10 06/18] arm64: fpsimd: Generalise context saving for non-task contexts In-Reply-To: <1527005119-6842-7-git-send-email-Dave.Martin@arm.com> References: <1527005119-6842-1-git-send-email-Dave.Martin@arm.com> <1527005119-6842-7-git-send-email-Dave.Martin@arm.com> Message-ID: <87in7et9gw.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dave Martin writes: > In preparation for allowing non-task (i.e., KVM vcpu) FPSIMD > contexts to be handled by the fpsimd common code, this patch adapts > task_fpsimd_save() to save back the currently loaded context, > removing the explicit dependency on current. > > The relevant storage to write back to in memory is now found by > examining the fpsimd_last_state percpu struct. > > fpsimd_save() does nothing unless TIF_FOREIGN_FPSTATE is clear, and > fpsimd_last_state is updated under local_bh_disable() or > local_irq_disable() everywhere that TIF_FOREIGN_FPSTATE is cleared: > thus, fpsimd_save() will write back to the correct storage for the > loaded context. > > No functional change. > > Signed-off-by: Dave Martin > Acked-by: Marc Zyngier > Acked-by: Catalin Marinas > --- > arch/arm64/kernel/fpsimd.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 9d85373..3aa100a 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -270,13 +270,15 @@ static void task_fpsimd_load(void) > } > > /* > - * Ensure current's FPSIMD/SVE storage in thread_struct is up to date > - * with respect to the CPU registers. > + * Ensure FPSIMD/SVE storage in memory for the loaded context is up to > + * date with respect to the CPU registers. > * > * Softirqs (and preemption) must be disabled. > */ > -static void task_fpsimd_save(void) > +static void fpsimd_save(void) > { > + struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st); > + I thought I was missing something but the only write I saw of this was: __this_cpu_write(fpsimd_last_state.st, NULL); which implied to me it is possible to have an invalid de-reference. I did figure it out eventually as fpsimd_bind_state_to_cpu uses a more indirect this_cpu_ptr idiom for tweaking this. I guess a reference to fpsimd_bind_[task|state]_to_cpu in the comment would have helped my confusion. Anyway: Reviewed-by: Alex Benn?e > WARN_ON(!in_softirq() && !irqs_disabled()); > > if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) { > @@ -291,10 +293,9 @@ static void task_fpsimd_save(void) > return; > } > > - sve_save_state(sve_pffr(current), > - ¤t->thread.uw.fpsimd_state.fpsr); > + sve_save_state(sve_pffr(current), &st->fpsr); > } else > - fpsimd_save_state(¤t->thread.uw.fpsimd_state); > + fpsimd_save_state(st); > } > } > > @@ -598,7 +599,7 @@ int sve_set_vector_length(struct task_struct *task, > if (task == current) { > local_bh_disable(); > > - task_fpsimd_save(); > + fpsimd_save(); > set_thread_flag(TIF_FOREIGN_FPSTATE); > } > > @@ -837,7 +838,7 @@ asmlinkage void do_sve_acc(unsigned int esr, struct pt_regs *regs) > > local_bh_disable(); > > - task_fpsimd_save(); > + fpsimd_save(); > fpsimd_to_sve(current); > > /* Force ret_to_user to reload the registers: */ > @@ -898,7 +899,7 @@ void fpsimd_thread_switch(struct task_struct *next) > * 'current'. > */ > if (current->mm) > - task_fpsimd_save(); > + fpsimd_save(); > > if (next->mm) { > /* > @@ -980,7 +981,7 @@ void fpsimd_preserve_current_state(void) > return; > > local_bh_disable(); > - task_fpsimd_save(); > + fpsimd_save(); > local_bh_enable(); > } > > @@ -1121,7 +1122,7 @@ void kernel_neon_begin(void) > > /* Save unsaved task fpsimd state, if any: */ > if (current->mm) > - task_fpsimd_save(); > + fpsimd_save(); > > /* Invalidate any task state remaining in the fpsimd regs: */ > fpsimd_flush_cpu_state(); > @@ -1244,7 +1245,7 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self, > switch (cmd) { > case CPU_PM_ENTER: > if (current->mm) > - task_fpsimd_save(); > + fpsimd_save(); > fpsimd_flush_cpu_state(); > break; > case CPU_PM_EXIT: -- Alex Benn?e