Re: [PATCH v4 4/5] arm64: mte: Enable async tag check fault

On 1/19/21 2:34 PM, Catalin Marinas wrote:

On Mon, Jan 18, 2021 at 06:30:32PM +0000, Vincenzo Frascino wrote:

quoted

 static void update_sctlr_el1_tcf0(u64 tcf0)
 {
 	/* ISB required for the kernel uaccess routines */

@@ -235,6 +273,15 @@ void mte_thread_switch(struct task_struct *next)
 	/* avoid expensive SCTLR_EL1 accesses if no change */
 	if (current->thread.sctlr_tcf0 != next->thread.sctlr_tcf0)
 		update_sctlr_el1_tcf0(next->thread.sctlr_tcf0);
+
+	/*
+	 * Check if an async tag exception occurred at EL1.
+	 *
+	 * Note: On the context switch path we rely on the dsb() present
+	 * in __switch_to() to guarantee that the indirect writes to TFSR_EL1
+	 * are synchronized before this point.
+	 */
+	mte_check_tfsr_el1();
 }

We need an isb() before mte_check_tfsr_el1() here as well, we only have
a dsb() in __switch_to(). We do have an isb() in update_sctlr_el1_tcf0()
but only if the check passed. Now, it's worth benchmarking how expensive
update_sctlr_el1_tcf0() is (i.e. an SCTLR_EL1 access + isb with
something like hackbench) and we could probably remove the check
altogether. In the meantime, you can add an isb() on the "else" path of
the above check.

Good catch, I saw the isb() in update_sctlr_el1_tcf0() and for some reasons that
it is not escaping me I thought it was sufficient, but clearly it is not.

I am happy to benchmark what you are suggesting and provide some data after this
series is merged (if it works for you) so that we can decide. In the meantime as
you suggested I will fix the "else" for v5.

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel