All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: kvmarm@lists.cs.columbia.edu
Cc: Okamoto Takayuki <tokamoto@jp.fujitsu.com>,
	Christoffer Dall <cdall@kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 01/16] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush
Date: Thu, 21 Jun 2018 15:57:25 +0100	[thread overview]
Message-ID: <1529593060-542-2-git-send-email-Dave.Martin@arm.com> (raw)
In-Reply-To: <1529593060-542-1-git-send-email-Dave.Martin@arm.com>

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 <Dave.Martin@arm.com>
---
 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(&current->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);
-- 
2.1.4

WARNING: multiple messages have this Message-ID (diff)
From: Dave.Martin@arm.com (Dave Martin)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 01/16] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush
Date: Thu, 21 Jun 2018 15:57:25 +0100	[thread overview]
Message-ID: <1529593060-542-2-git-send-email-Dave.Martin@arm.com> (raw)
In-Reply-To: <1529593060-542-1-git-send-email-Dave.Martin@arm.com>

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 <Dave.Martin@arm.com>
---
 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(&current->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);
-- 
2.1.4

  reply	other threads:[~2018-06-21 14:47 UTC|newest]

Thread overview: 178+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-21 14:57 [RFC PATCH 00/16] KVM: arm64: Initial support for SVE guests Dave Martin
2018-06-21 14:57 ` Dave Martin
2018-06-21 14:57 ` Dave Martin [this message]
2018-06-21 14:57   ` [RFC PATCH 01/16] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush Dave Martin
2018-07-06  9:07   ` Alex Bennée
2018-07-06  9:07     ` Alex Bennée
2018-06-21 14:57 ` [RFC PATCH 02/16] KVM: arm64: Delete orphaned declaration for __fpsimd_enabled() Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-06  9:08   ` Alex Bennée
2018-07-06  9:08     ` Alex Bennée
2018-06-21 14:57 ` [RFC PATCH 03/16] KVM: arm64: Refactor kvm_arm_num_regs() for easier maintenance Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-06  9:20   ` Alex Bennée
2018-07-06  9:20     ` Alex Bennée
2018-06-21 14:57 ` [RFC PATCH 04/16] KVM: arm64: Add missing #include of <linux/bitmap.h> to kvm_host.h Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-06  9:21   ` Alex Bennée
2018-07-06  9:21     ` Alex Bennée
2018-06-21 14:57 ` [RFC PATCH 05/16] KVM: arm: Add arch init/uninit hooks Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-06 10:02   ` Alex Bennée
2018-07-06 10:02     ` Alex Bennée
2018-07-09 15:15     ` Dave Martin
2018-07-09 15:15       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 06/16] arm64/sve: Determine virtualisation-friendly vector lengths Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-06 13:20   ` Marc Zyngier
2018-07-06 13:20     ` Marc Zyngier
2018-06-21 14:57 ` [RFC PATCH 07/16] arm64/sve: Enable SVE state tracking for non-task contexts Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-25 13:58   ` Alex Bennée
2018-07-25 13:58     ` Alex Bennée
2018-07-25 14:39     ` Dave Martin
2018-07-25 14:39       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 08/16] KVM: arm64: Support dynamically hideable system registers Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-25 14:12   ` Alex Bennée
2018-07-25 14:12     ` Alex Bennée
2018-07-25 14:36     ` Dave Martin
2018-07-25 14:36       ` Dave Martin
2018-07-25 15:41       ` Alex Bennée
2018-07-25 15:41         ` Alex Bennée
2018-07-26 12:53         ` Dave Martin
2018-07-26 12:53           ` Dave Martin
2018-08-07 19:20   ` Christoffer Dall
2018-08-07 19:20     ` Christoffer Dall
2018-08-08  8:33     ` Dave Martin
2018-08-08  8:33       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 09/16] KVM: arm64: Allow ID registers to by dynamically read-as-zero Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-25 15:46   ` Alex Bennée
2018-07-25 15:46     ` Alex Bennée
2018-08-06 13:03   ` Christoffer Dall
2018-08-06 13:03     ` Christoffer Dall
2018-08-07 11:09     ` Dave Martin
2018-08-07 11:09       ` Dave Martin
2018-08-07 19:35       ` Christoffer Dall
2018-08-07 19:35         ` Christoffer Dall
2018-08-08  9:11         ` Dave Martin
2018-08-08  9:11           ` Dave Martin
2018-08-08  9:58           ` Christoffer Dall
2018-08-08  9:58             ` Christoffer Dall
2018-08-08 14:03           ` Peter Maydell
2018-08-08 14:03             ` Peter Maydell
2018-08-09 10:19             ` Dave Martin
2018-08-09 10:19               ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 10/16] KVM: arm64: Add a vcpu flag to control SVE visibility for the guest Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-19 11:08   ` Andrew Jones
2018-07-19 11:08     ` Andrew Jones
2018-07-25 11:41     ` Dave Martin
2018-07-25 11:41       ` Dave Martin
2018-07-25 13:43       ` Andrew Jones
2018-07-25 13:43         ` Andrew Jones
2018-07-25 14:41         ` Dave Martin
2018-07-25 14:41           ` Dave Martin
2018-07-19 15:02   ` Andrew Jones
2018-07-19 15:02     ` Andrew Jones
2018-07-25 11:48     ` Dave Martin
2018-07-25 11:48       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 11/16] KVM: arm64/sve: System register context switch and access support Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-19 11:11   ` Andrew Jones
2018-07-19 11:11     ` Andrew Jones
2018-07-25 11:45     ` Dave Martin
2018-07-25 11:45       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 12/16] KVM: arm64/sve: Context switch the SVE registers Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-19 13:13   ` Andrew Jones
2018-07-19 13:13     ` Andrew Jones
2018-07-25 11:50     ` Dave Martin
2018-07-25 11:50       ` Dave Martin
2018-07-25 13:57       ` Andrew Jones
2018-07-25 13:57         ` Andrew Jones
2018-07-25 14:12         ` Dave Martin
2018-07-25 14:12           ` Dave Martin
2018-08-06 13:19   ` Christoffer Dall
2018-08-06 13:19     ` Christoffer Dall
2018-08-07 11:15     ` Dave Martin
2018-08-07 11:15       ` Dave Martin
2018-08-07 19:43       ` Christoffer Dall
2018-08-07 19:43         ` Christoffer Dall
2018-08-08  8:23         ` Dave Martin
2018-08-08  8:23           ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 13/16] KVM: Allow 2048-bit register access via KVM_{GET, SET}_ONE_REG Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-25 15:58   ` Alex Bennée
2018-07-25 15:58     ` Alex Bennée
2018-07-26 12:58     ` Dave Martin
2018-07-26 12:58       ` Dave Martin
2018-07-26 13:55       ` Alex Bennée
2018-07-26 13:55         ` Alex Bennée
2018-07-27  9:26         ` Dave Martin
2018-07-27  9:26           ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 14/16] KVM: arm64/sve: Add SVE support to register access ioctl interface Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-19 13:04   ` Andrew Jones
2018-07-19 13:04     ` Andrew Jones
2018-07-25 14:06     ` Dave Martin
2018-07-25 14:06       ` Dave Martin
2018-07-25 17:20       ` Andrew Jones
2018-07-25 17:20         ` Andrew Jones
2018-07-26 13:10         ` Dave Martin
2018-07-26 13:10           ` Dave Martin
2018-08-03 14:57     ` Dave Martin
2018-08-03 14:57       ` Dave Martin
2018-08-03 15:11       ` Andrew Jones
2018-08-03 15:11         ` Andrew Jones
2018-08-03 15:38         ` Dave Martin
2018-08-03 15:38           ` Dave Martin
2018-08-06 13:25   ` Christoffer Dall
2018-08-06 13:25     ` Christoffer Dall
2018-08-07 11:17     ` Dave Martin
2018-08-07 11:17       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 15/16] KVM: arm64: Enumerate SVE register indices for KVM_GET_REG_LIST Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-19 14:12   ` Andrew Jones
2018-07-19 14:12     ` Andrew Jones
2018-07-25 14:50     ` Dave Martin
2018-07-25 14:50       ` Dave Martin
2018-06-21 14:57 ` [RFC PATCH 16/16] KVM: arm64/sve: Report and enable SVE API extensions for userspace Dave Martin
2018-06-21 14:57   ` Dave Martin
2018-07-19 14:59   ` Andrew Jones
2018-07-19 14:59     ` Andrew Jones
2018-07-25 15:27     ` Dave Martin
2018-07-25 15:27       ` Dave Martin
2018-07-25 16:52       ` Andrew Jones
2018-07-25 16:52         ` Andrew Jones
2018-07-26 13:18         ` Dave Martin
2018-07-26 13:18           ` Dave Martin
2018-08-06 13:41           ` Christoffer Dall
2018-08-06 13:41             ` Christoffer Dall
2018-08-07 11:23             ` Dave Martin
2018-08-07 11:23               ` Dave Martin
2018-08-07 20:08               ` Christoffer Dall
2018-08-07 20:08                 ` Christoffer Dall
2018-08-08  8:30                 ` Dave Martin
2018-08-08  8:30                   ` Dave Martin
2018-07-19 15:24   ` Andrew Jones
2018-07-19 15:24     ` Andrew Jones
2018-07-26 13:23     ` Dave Martin
2018-07-26 13:23       ` Dave Martin
2018-07-06  8:22 ` [RFC PATCH 00/16] KVM: arm64: Initial support for SVE guests Alex Bennée
2018-07-06  8:22   ` Alex Bennée
2018-07-06  9:05   ` Dave Martin
2018-07-06  9:05     ` Dave Martin
2018-07-06  9:20     ` Alex Bennée
2018-07-06  9:20       ` Alex Bennée
2018-07-06  9:23       ` Peter Maydell
2018-07-06  9:23         ` Peter Maydell
2018-07-06 10:11         ` Alex Bennée
2018-07-06 10:11           ` Alex Bennée
2018-07-06 10:14           ` Peter Maydell
2018-07-06 10:14             ` Peter Maydell
2018-08-06 13:05 ` Christoffer Dall
2018-08-06 13:05   ` Christoffer Dall
2018-08-07 11:18   ` Dave Martin
2018-08-07 11:18     ` Dave Martin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1529593060-542-2-git-send-email-Dave.Martin@arm.com \
    --to=dave.martin@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=cdall@kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=tokamoto@jp.fujitsu.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.