在 2022/4/13 1:08, Robin Murphy 写道:
On 12/04/2022 8:25 am, Tong Tiangen wrote:
[...]
diff --git a/arch/arm64/include/asm/asm-uaccess.h
b/arch/arm64/include/asm/asm-uaccess.h
index 0557af834e03..bb17f0829042 100644
--- a/arch/arm64/include/asm/asm-uaccess.h
+++ b/arch/arm64/include/asm/asm-uaccess.h
@@ -92,4 +92,20 @@ alternative_else_nop_endif
_asm_extable 8888b,\l;
.endm
+
+ .macro user_ldp_mc l, reg1, reg2, addr, post_inc
+8888: ldtr \reg1, [\addr];
+8889: ldtr \reg2, [\addr, #8];
+ add \addr, \addr, \post_inc;
+
+ _asm_extable_uaccess_mc 8888b, \l;
+ _asm_extable_uaccess_mc 8889b, \l;
+ .endm
You're replacing the only user of this, so please just
s/_asm_extable/_asm_extable_uaccess_mc/ in the existing macro and save
the rest of the churn.
Agreed, *user_ldp* -- This name has clearly explained the scences where
this macro is used. It is more appropriate to modify it directly.
Furthermore, how come you're not similarly updating user_stp, given that
you *are* updating the other stores in copy_to_user?
+
+ .macro user_ldst_mc l, inst, reg, addr, post_inc
+8888: \inst \reg, [\addr];
+ add \addr, \addr, \post_inc;
+
+ _asm_extable_uaccess_mc 8888b, \l;
+ .endm
Similarly, I think we can just update user_ldst itself. The two
instances that you're not replacing here are bogus anyway, and deserve
to be fixed with the patch below first.
OK, great thanks. will do next version.
[...]
@@ -62,7 +63,11 @@ SYM_FUNC_START(__arch_copy_from_user)
ret
// Exception fixups
-9997: cmp dst, dstin
+9997: mrs esr, esr_el1 // Check exception first
+ and esr, esr, #ESR_ELx_FSC
+ cmp esr, #ESR_ELx_FSC_EXTABT
Should we be checking EC to make sure it's a data abort - and thus FSC
is valid - in the first place? I'm a little fuzzy on all the possible
paths into fixup_exception(), and it's not entirely obvious whether this
is actually safe or not.
Thanks,
Robin.
I think checking EC here is more rigorous in code logic and it's doesn't
appear to be harmful.
It is really not appropriate to check the ESR at this stage (it has been
checked where the exception processing starts). At present, I haven't
thought of a better way. If anyone has a better way, please reply to me :)
Thanks Robin.
Tong.
----->8-----
Subject: [PATCH] arm64: mte: Clean up user tag accessors
Invoking user_ldst to explicitly add a post-increment of 0 is silly.
Just use a normal USER() annotation and save the redundant instruction.
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
---
arch/arm64/lib/mte.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
index 8590af3c98c0..eeb9e45bcce8 100644
--- a/arch/arm64/lib/mte.S
+++ b/arch/arm64/lib/mte.S
@@ -93,7 +93,7 @@ SYM_FUNC_START(mte_copy_tags_from_user)
mov x3, x1
cbz x2, 2f
1:
- user_ldst 2f, ldtrb, w4, x1, 0
+USER(2f, ldtrb w4, [x1])
lsl x4, x4, #MTE_TAG_SHIFT
stg x4, [x0], #MTE_GRANULE_SIZE
add x1, x1, #1
@@ -120,7 +120,7 @@ SYM_FUNC_START(mte_copy_tags_to_user)
1:
ldg x4, [x1]
ubfx x4, x4, #MTE_TAG_SHIFT, #MTE_TAG_SIZE
- user_ldst 2f, sttrb, w4, x0, 0
+USER(2f, sttrb w4, [x0])
add x0, x0, #1
add x1, x1, #MTE_GRANULE_SIZE
subs x2, x2, #1
