From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Martin Subject: [PATCH v11 01/19] arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs Date: Thu, 24 May 2018 17:56:30 +0100 Message-ID: <1527181008-13549-2-git-send-email-Dave.Martin@arm.com> References: <1527181008-13549-1-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 4AD654A10C for ; Thu, 24 May 2018 12:47:23 -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 tT0BWqcQ906c for ; Thu, 24 May 2018 12:47:00 -0400 (EDT) Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 51DD54A0D5 for ; Thu, 24 May 2018 12:46:54 -0400 (EDT) In-Reply-To: <1527181008-13549-1-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: kvmarm@lists.cs.columbia.edu Cc: Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu ZnBzaW1kX2xhc3Rfc3RhdGUuc3QgaXMgc2V0IHRvIE5VTEwgYXMgYSB3YXkgb2YgaW5kaWNhdGlu ZyB0aGF0CmN1cnJlbnQncyBGUFNJTUQgcmVnaXN0ZXJzIGFyZSBubyBsb25nZXIgbG9hZGVkIGlu IHRoZSBjcHUuICBJbgpwYXJ0aWN1bGFyLCB0aGlzIGlzIGRvbmUgd2hlbiB0aGUga2VybmVsIHRl bXBvcmFyaWx5IHVzZXMgb3IKY2xvYmJlcnMgdGhlIEZQU0lNRCByZWdpc3RlcnMgZm9yIGl0cyBv d24gcHVycG9zZXMsIGFzIGluIENQVSBQTSBvcgprZXJuZWwtbW9kZSBORU9OLCByZXN1bHRpbmcg aW4gdGhlbSBiZWluZyBwb3B1bGF0ZWQgd2l0aCBnYXJiYWdlCmRhdGEgbm90IGJlbG9uZ2luZyB0 byBhIHRhc2suCgpDb21taXQgMTdlZWQyN2IwMmRhICgiYXJtNjQvc3ZlOiBLVk06IFByZXZlbnQg Z3Vlc3RzIGZyb20gdXNpbmcKU1ZFIikgZmFjdG9ycyB0aGlzIG9wZXJhdGlvbiBvdXQgYXMgYSBu ZXcgaGVscGVyCmZwc2ltZF9mbHVzaF9jcHVfc3RhdGUoKSB0byBtYWtlIGl0IGNsZWFyZXIgd2hh dCBpcyBiZWluZyBkb25lCmhlcmUsIGFuZCBvbiBTVkUgc3lzdGVtcyB0aGlzIGhlbHBlciBpcyBu b3cgdXNlZCwgdmlhCmt2bV9mcHNpbWRfZmx1c2hfY3B1X3N0YXRlKCksIHRvIGludmFsaWRhdGUg dGhlIHJlZ2lzdGVycyBhZnRlciBLVk0KaGFzIHJ1biBhIHZjcHUuICBUaGUgcmVhc29uIGZvciB0 aGlzIGlzIHRoYXQgS1ZNIGRvZXMgbm90IHlldAp1bmRlcnN0YW5kIGhvdyB0byByZXN0b3JlIHRo ZSBmdWxsIGhvc3QgU1ZFIHJlZ2lzdGVycyBpdHNlbGYgYWZ0ZXIKbG9hZGluZyB0aGUgZ3Vlc3Qg RlBTSU1EIGNvbnRleHQgaW50byB0aGVtLgoKVGhpcyBleHBvc2VzIGEgcGFydGljdWxhciBwcm9i bGVtOiBpZiBmcHNpbWRfbGFzdF9zdGF0ZS5zdCBpcyBzZXQKdG8gTlVMTCB3aXRob3V0IGFsc28g c2V0dGluZyBUSUZfRk9SRUlHTl9GUFNUQVRFLCB0aGUga2VybmVsIG1heQpjb250aW51ZSB0byB0 aGluayB0aGF0IGN1cnJlbnQncyBGUFNJTUQgcmVnaXN0ZXJzIGFyZSBsaXZlIGV2ZW4KdGhvdWdo IHRoZXkgaGF2ZSBhY3R1YWxseSBiZWVuIGNsb2JiZXJlZC4KClByaW9yIHRvIHRoZSBhZm9yZW1l bnRpb25lZCBjb21taXQsIHRoZSBvbmx5IHBhdGggd2hlcmUKZnBzaW1kX2xhc3Rfc3RhdGUuc3Qg aXMgc2V0IHRvIE5VTEwgd2l0aG91dCBzZXR0aW5nClRJRl9GT1JFSUdOX0ZQU1RBVEUgaXMgd2hl biBrZXJuZWxfbmVvbl9iZWdpbigpIGlzIGNhbGxlZCBieSBhCmtlcm5lbCB0aHJlYWQgKHdoZXJl IGN1cnJlbnQtPm1tIGNhbiBiZSBOVUxMKS4gIFRoaXMgZG9lcyBub3QKbWF0dGVyLCBiZWNhdXNl IHRoZSBvbmx5IGhhcm0gaXMgdGhhdCBhdCBjb250ZXh0LXN3aXRjaCB0aW1lCmZwc2ltZF90aHJl YWRfc3dpdGNoKCkgbWF5IHVubmVjZXNzYXJpbHkgc2F2ZSB0aGUgRlBTSU1EIHJlZ2lzdGVycwpi YWNrIHRvIGN1cnJlbnQncyB0aHJlYWRfc3RydWN0IChldmVuIHRob3VnaCBrZXJuZWwgdGhyZWFk cyBhcmUgbm90CmNvbnNpZGVyZWQgdG8gaGF2ZSBhbnkgRlBTSU1EIGNvbnRleHQgb2YgdGhlaXIg b3duIGFuZCB0aGUKcmVnaXN0ZXJzIHdpbGwgbmV2ZXIgYmUgcmVsb2FkZWQpLgoKTm90ZSB0aGF0 IGFsdGhvdWdoIENQVV9QTV9FTlRFUiBsYWNrcyB0aGUgVElGX0ZPUkVJR05fRlBTVEFURQpzZXR0 aW5nLCBldmVyeSBDUFUgcGFzc2luZyB0aHJvdWdoIHRoYXQgcGF0aCBtdXN0IHN1YnNlcXVlbnRs eSBwYXNzCnRocm91Z2ggQ1BVX1BNX0VYSVQgYmVmb3JlIGl0IGNhbiByZS1lbnRlciB0aGUga2Vy bmVsIHByb3Blci4KQ1BVX1BNX0VYSVQgc2V0cyB0aGUgZmxhZy4KClRoZSBzdmVfZmx1c2hfY3B1 X3N0YXRlKCkgZnVuY3Rpb24gYWRkZWQgYnkgY29tbWl0IDE3ZWVkMjdiMDJkYQphbHNvIGxhY2tz IHRoZSBwcm9wZXIgbWFpbnRlbmFuY2Ugb2YgVElGX0ZPUkVJR05fRlBTVEFURS4gIFRoaXMgbWF5 CmNhdXNlIHRoZSBiaXRzIG9mIGEgaG9zdCB0YXNrJ3MgU1ZFIHJlZ2lzdGVycyB0aGF0IGRvIG5v dCBhbGlhcyB0aGUKRlBTSU1EIHJlZ2lzdGVyIGZpbGUgdG8gc3BvbnRhbmVvdXNseSBhcHBlYXIg emVyb2VkIGlmIGEgS1ZNIHZjcHUKcnVucyBpbiB0aGUgc2FtZSB0YXNrIGluIHRoZSBtZWFudGlt ZS4gIEFsdGhvdWdoIHRoaXMgZWZmZWN0IGlzCmhpZGRlbiBieSB0aGUgZmFjdCB0aGF0IHRoZSBu b24tRlBTSU1EIGJpdHMgb2YgdGhlIFNWRSByZWdpc3RlcnMKYXJlIHplcm9lZCBieSBhIHN5c2Nh bGwgYW55d2F5LCBpdCBpcyBkb3VidGxlc3MgYSBiYWQgaWRlYSB0byByZWx5Cm9uIHRoZXNlIGRp ZmZlcmVudCBjb2RlIHBhdGhzIGludGVyYWN0aW5nIGNvcnJlY3RseSB1bmRlciBmdXR1cmUKbWFp bnRlbmFuY2UuCgpUaGlzIHBhdGNoIG1ha2VzIFRJRl9GT1JFSUdOX0ZQU1RBVEUgYW4gdW5jb25k aXRpb25hbCBzaWRlLWVmZmVjdApvZiBmcHNpbWRfZmx1c2hfY3B1X3N0YXRlKCksIGFuZCByZW1v dmVzIHRoZSBzZXRfdGhyZWFkX2ZsYWcoKQpjYWxscyB0aGF0IGJlY29tZSByZWR1bmRhbnQgYXMg YSByZXN1bHQuICBUaGlzIGVuc3VyZXMgdGhhdApUSUZfRk9SRUlHTl9GUFNUQVRFIGNhbm5vdCBy ZW1haW4gY2xlYXIgaWYgdGhlIEZQU0lNRCBzdGF0ZSBpbiB0aGUKRlBTSU1EIHJlZ2lzdGVycyBp cyBpbnZhbGlkLgoKU2lnbmVkLW9mZi1ieTogRGF2ZSBNYXJ0aW4gPERhdmUuTWFydGluQGFybS5j b20+ClJldmlld2VkLWJ5OiBDaHJpc3RvZmZlciBEYWxsIDxjaHJpc3RvZmZlci5kYWxsQGFybS5j b20+ClJldmlld2VkLWJ5OiBBbGV4IEJlbm7DqWUgPGFsZXguYmVubmVlQGxpbmFyby5vcmc+ClJl dmlld2VkLWJ5OiBDYXRhbGluIE1hcmluYXMgPGNhdGFsaW4ubWFyaW5hc0Bhcm0uY29tPgpDYzog V2lsbCBEZWFjb24gPHdpbGwuZGVhY29uQGFybS5jb20+CkNjOiBBcmQgQmllc2hldXZlbCA8YXJk LmJpZXNoZXV2ZWxAbGluYXJvLm9yZz4KLS0tCiBhcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYyB8 IDcgKystLS0tLQogMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMo LSkKCmRpZmYgLS1naXQgYS9hcmNoL2FybTY0L2tlcm5lbC9mcHNpbWQuYyBiL2FyY2gvYXJtNjQv a2VybmVsL2Zwc2ltZC5jCmluZGV4IDg3YTM1MzYuLjEyZTFjOTYgMTAwNjQ0Ci0tLSBhL2FyY2gv YXJtNjQva2VybmVsL2Zwc2ltZC5jCisrKyBiL2FyY2gvYXJtNjQva2VybmVsL2Zwc2ltZC5jCkBA IC0xMDY3LDYgKzEwNjcsNyBAQCB2b2lkIGZwc2ltZF9mbHVzaF90YXNrX3N0YXRlKHN0cnVjdCB0 YXNrX3N0cnVjdCAqdCkKIHN0YXRpYyBpbmxpbmUgdm9pZCBmcHNpbWRfZmx1c2hfY3B1X3N0YXRl KHZvaWQpCiB7CiAJX190aGlzX2NwdV93cml0ZShmcHNpbWRfbGFzdF9zdGF0ZS5zdCwgTlVMTCk7 CisJc2V0X3RocmVhZF9mbGFnKFRJRl9GT1JFSUdOX0ZQU1RBVEUpOwogfQogCiAvKgpAQCAtMTEy MSwxMCArMTEyMiw4IEBAIHZvaWQga2VybmVsX25lb25fYmVnaW4odm9pZCkKIAlfX3RoaXNfY3B1 X3dyaXRlKGtlcm5lbF9uZW9uX2J1c3ksIHRydWUpOwogCiAJLyogU2F2ZSB1bnNhdmVkIHRhc2sg ZnBzaW1kIHN0YXRlLCBpZiBhbnk6ICovCi0JaWYgKGN1cnJlbnQtPm1tKSB7CisJaWYgKGN1cnJl bnQtPm1tKQogCQl0YXNrX2Zwc2ltZF9zYXZlKCk7Ci0JCXNldF90aHJlYWRfZmxhZyhUSUZfRk9S RUlHTl9GUFNUQVRFKTsKLQl9CiAKIAkvKiBJbnZhbGlkYXRlIGFueSB0YXNrIHN0YXRlIHJlbWFp bmluZyBpbiB0aGUgZnBzaW1kIHJlZ3M6ICovCiAJZnBzaW1kX2ZsdXNoX2NwdV9zdGF0ZSgpOwpA QCAtMTI1MSw4ICsxMjUwLDYgQEAgc3RhdGljIGludCBmcHNpbWRfY3B1X3BtX25vdGlmaWVyKHN0 cnVjdCBub3RpZmllcl9ibG9jayAqc2VsZiwKIAkJZnBzaW1kX2ZsdXNoX2NwdV9zdGF0ZSgpOwog CQlicmVhazsKIAljYXNlIENQVV9QTV9FWElUOgotCQlpZiAoY3VycmVudC0+bW0pCi0JCQlzZXRf dGhyZWFkX2ZsYWcoVElGX0ZPUkVJR05fRlBTVEFURSk7CiAJCWJyZWFrOwogCWNhc2UgQ1BVX1BN X0VOVEVSX0ZBSUxFRDoKIAlkZWZhdWx0OgotLSAKMi4xLjQKCl9fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fCmt2bWFybSBtYWlsaW5nIGxpc3QKa3ZtYXJtQGxp c3RzLmNzLmNvbHVtYmlhLmVkdQpodHRwczovL2xpc3RzLmNzLmNvbHVtYmlhLmVkdS9tYWlsbWFu L2xpc3RpbmZvL2t2bWFybQo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave.Martin@arm.com (Dave Martin) Date: Thu, 24 May 2018 17:56:30 +0100 Subject: [PATCH v11 01/19] arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs In-Reply-To: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com> References: <1527181008-13549-1-git-send-email-Dave.Martin@arm.com> Message-ID: <1527181008-13549-2-git-send-email-Dave.Martin@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org fpsimd_last_state.st is set to NULL as a way of indicating that current's FPSIMD registers are no longer loaded in the cpu. In particular, this is done when the kernel temporarily uses or clobbers the FPSIMD registers for its own purposes, as in CPU PM or kernel-mode NEON, resulting in them being populated with garbage data not belonging to a task. Commit 17eed27b02da ("arm64/sve: KVM: Prevent guests from using SVE") factors this operation out as a new helper fpsimd_flush_cpu_state() to make it clearer what is being done here, and on SVE systems this helper is now used, via kvm_fpsimd_flush_cpu_state(), to invalidate the registers after KVM has run a vcpu. The reason for this is that KVM does not yet understand how to restore the full host SVE registers itself after loading the guest FPSIMD context into them. This exposes a particular problem: if fpsimd_last_state.st is set to NULL without also setting TIF_FOREIGN_FPSTATE, the kernel may continue to think that current's FPSIMD registers are live even though they have actually been clobbered. Prior to the aforementioned commit, the only path where fpsimd_last_state.st is set to NULL without setting TIF_FOREIGN_FPSTATE is when kernel_neon_begin() is called by a kernel thread (where current->mm can be NULL). This does not matter, because the only harm is that at context-switch time fpsimd_thread_switch() may unnecessarily save the FPSIMD registers back to current's thread_struct (even though kernel threads are not considered to have any FPSIMD context of their own and the registers will never be reloaded). Note that although CPU_PM_ENTER lacks the TIF_FOREIGN_FPSTATE setting, every CPU passing through that path must subsequently pass through CPU_PM_EXIT before it can re-enter the kernel proper. CPU_PM_EXIT sets the flag. The sve_flush_cpu_state() function added by commit 17eed27b02da also lacks the proper maintenance of TIF_FOREIGN_FPSTATE. This may cause the bits of a host task's SVE registers that do not alias the FPSIMD register file to spontaneously appear zeroed if a KVM vcpu runs in the same task in the meantime. Although this effect is hidden by the fact that the non-FPSIMD bits of the SVE registers are zeroed by a syscall anyway, it is doubtless a bad idea to rely on these different code paths interacting correctly under future maintenance. This patch makes TIF_FOREIGN_FPSTATE an unconditional side-effect of fpsimd_flush_cpu_state(), and removes the set_thread_flag() calls that become redundant as a result. This ensures that TIF_FOREIGN_FPSTATE cannot remain clear if the FPSIMD state in the FPSIMD registers is invalid. Signed-off-by: Dave Martin Reviewed-by: Christoffer Dall Reviewed-by: Alex Benn?e Reviewed-by: Catalin Marinas Cc: Will Deacon Cc: Ard Biesheuvel --- arch/arm64/kernel/fpsimd.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 87a3536..12e1c96 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -1067,6 +1067,7 @@ void fpsimd_flush_task_state(struct task_struct *t) static inline void fpsimd_flush_cpu_state(void) { __this_cpu_write(fpsimd_last_state.st, NULL); + set_thread_flag(TIF_FOREIGN_FPSTATE); } /* @@ -1121,10 +1122,8 @@ void kernel_neon_begin(void) __this_cpu_write(kernel_neon_busy, true); /* Save unsaved task fpsimd state, if any: */ - if (current->mm) { + if (current->mm) task_fpsimd_save(); - set_thread_flag(TIF_FOREIGN_FPSTATE); - } /* Invalidate any task state remaining in the fpsimd regs: */ fpsimd_flush_cpu_state(); @@ -1251,8 +1250,6 @@ static int fpsimd_cpu_pm_notifier(struct notifier_block *self, fpsimd_flush_cpu_state(); break; case CPU_PM_EXIT: - if (current->mm) - set_thread_flag(TIF_FOREIGN_FPSTATE); break; case CPU_PM_ENTER_FAILED: default: -- 2.1.4