Re: [PATCH v9 2/4] arm64: mte: change ASYNC and SYNC TCF settings into bitfields

On Wed, Jul 7, 2021 at 4:11 AM Will Deacon [off-list ref] wrote:

On Fri, Jul 02, 2021 at 12:41:08PM -0700, Peter Collingbourne wrote:

quoted

Allow the user program to specify both ASYNC and SYNC TCF modes by
repurposing the existing constants as bitfields. This will allow the
kernel to select one of the modes on behalf of the user program. With
this patch the kernel will always select async mode, but a subsequent
patch will make this configurable.

Link: https://linux-review.googlesource.com/id/Icc5923c85a8ea284588cc399ae74fd19ec291230
Signed-off-by: Peter Collingbourne <redacted>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
v9:
- make mte_update_sctlr_user static

 arch/arm64/include/asm/processor.h |  3 ++
 arch/arm64/kernel/mte.c            | 70 ++++++++++++------------------
 include/uapi/linux/prctl.h         | 11 ++---
 3 files changed, 37 insertions(+), 47 deletions(-)

...

quoted

 long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 {
-     u64 sctlr = task->thread.sctlr_user & ~SCTLR_EL1_TCF0_MASK;
      u64 mte_ctrl = (~((arg & PR_MTE_TAG_MASK) >> PR_MTE_TAG_SHIFT) &
                      SYS_GCR_EL1_EXCL_MASK) << MTE_CTRL_GCR_USER_EXCL_SHIFT;

      if (!system_supports_mte())
              return 0;

-     switch (arg & PR_MTE_TCF_MASK) {
-     case PR_MTE_TCF_NONE:
-             sctlr |= SCTLR_EL1_TCF0_NONE;
-             break;
-     case PR_MTE_TCF_SYNC:
-             sctlr |= SCTLR_EL1_TCF0_SYNC;
-             break;
-     case PR_MTE_TCF_ASYNC:
-             sctlr |= SCTLR_EL1_TCF0_ASYNC;
-             break;
-     default:
-             return -EINVAL;
-     }
+     if (arg & PR_MTE_TCF_ASYNC)
+             mte_ctrl |= MTE_CTRL_TCF_ASYNC;
+     if (arg & PR_MTE_TCF_SYNC)
+             mte_ctrl |= MTE_CTRL_TCF_SYNC;

-     if (task != current) {
-             task->thread.sctlr_user = sctlr;
-             task->thread.mte_ctrl = mte_ctrl;
-     } else {
-             set_task_sctlr_el1(sctlr);
-             set_gcr_el1_excl(mte_ctrl);
+     task->thread.mte_ctrl = mte_ctrl;
+     if (task == current) {
+             mte_update_sctlr_user(task);

In conjunction with the next patch, what happens if we migrate at this
point? I worry that we can install a stale sctlr_user value.

quoted

+             set_task_sctlr_el1(task->thread.sctlr_user);

In this case, we will call mte_update_sctlr_user when scheduled onto
the new CPU as a result of the change to mte_thread_switch, and both
the scheduler and prctl will set SCTLR_EL1 to the new (correct) value
for the current CPU.

Peter

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