Re: [PATCHv3 13/17] arm64: uaccess cleanup macro naming

Robin Murphy
2020-10-27T18:45:57Z

On 2020-10-26 13:31, Mark Rutland wrote:
> Now the uaccess primitives use LDTR/STTR unconditionally, the the
> uao_{ldp,stp,user_alternative} asm macros are misnamed, and have a
> redunant argument. Let's remove te redundant argument and rename these

:set spell

> to user_{ldp,stp,ldst} respectively to clean this up.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
> arch/arm64/include/asm/asm-uaccess.h | 8 ++++----
> arch/arm64/lib/clear_user.S | 8 ++++----
> arch/arm64/lib/copy_from_user.S | 8 ++++----
> arch/arm64/lib/copy_in_user.S | 16 ++++++++--------
> arch/arm64/lib/copy_to_user.S | 8 ++++----
> arch/arm64/lib/mte.S | 6 +++---
> 6 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm64/include/asm/asm-uaccess.h b/arch/arm64/include/asm/asm-uaccess.h
> index 046196f08988b..5e3035fb2c802 100644
> --- a/arch/arm64/include/asm/asm-uaccess.h
> +++ b/arch/arm64/include/asm/asm-uaccess.h
> @@ -63,7 +63,7 @@ alternative_else_nop_endif
> * This is complicated as there is no post-increment or pair versions of the
> * unprivileged instructions, and USER() only works for single instructions.
> */
> - .macro uao_ldp l, reg1, reg2, addr, post_inc
> + .macro user_ldp l, reg1, reg2, addr, post_inc
> 8888: ldtr \reg1, [\addr];
> 8889: ldtr \reg2, [\addr, #8];
> add \addr, \addr, \post_inc;
> @@ -72,7 +72,7 @@ alternative_else_nop_endif
> _asm_extable 8889b,\l;
> .endm
>
> - .macro uao_stp l, reg1, reg2, addr, post_inc
> + .macro user_stp l, reg1, reg2, addr, post_inc
> 8888: sttr \reg1, [\addr];
> 8889: sttr \reg2, [\addr, #8];
> add \addr, \addr, \post_inc;
> @@ -81,8 +81,8 @@ alternative_else_nop_endif
> _asm_extable 8889b,\l;
> .endm
>
> - .macro uao_user_alternative l, inst, alt_inst, reg, addr, post_inc
> -8888: \alt_inst \reg, [\addr];
> + .macro user_ldst l, inst, reg, addr, post_inc
> +8888: \inst \reg, [\addr];
> add \addr, \addr, \post_inc;
>
> _asm_extable 8888b,\l;
> diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
> index 48a3a26eff663..af9afcbec92cd 100644
> --- a/arch/arm64/lib/clear_user.S
> +++ b/arch/arm64/lib/clear_user.S
> @@ -24,20 +24,20 @@ SYM_FUNC_START(__arch_clear_user)
> subs x1, x1, #8
> b.mi 2f
> 1:
> -uao_user_alternative 9f, str, sttr, xzr, x0, 8
> +user_ldst 9f, sttr, xzr, x0, 8
> subs x1, x1, #8
> b.pl 1b
> 2: adds x1, x1, #4
> b.mi 3f
> -uao_user_alternative 9f, str, sttr, wzr, x0, 4
> +user_ldst 9f, sttr, wzr, x0, 4
> sub x1, x1, #4
> 3: adds x1, x1, #2
> b.mi 4f
> -uao_user_alternative 9f, strh, sttrh, wzr, x0, 2
> +user_ldst 9f, sttrh, wzr, x0, 2
> sub x1, x1, #2
> 4: adds x1, x1, #1
> b.mi 5f
> -uao_user_alternative 9f, strb, sttrb, wzr, x0, 0
> +user_ldst 9f, sttrb, wzr, x0, 0
> 5: mov x0, #0
> ret
> SYM_FUNC_END(__arch_clear_user)
> diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
> index 0f8a3a9e3795b..95cd62d673711 100644
> --- a/arch/arm64/lib/copy_from_user.S
> +++ b/arch/arm64/lib/copy_from_user.S
> @@ -21,7 +21,7 @@
> */
>
> .macro ldrb1 reg, ptr, val
> - uao_user_alternative 9998f, ldrb, ldtrb, \reg, \ptr, \val
> + user_ldst 9998f, ldtrb, \reg, \ptr, \val
> .endm
>
> .macro strb1 reg, ptr, val
> @@ -29,7 +29,7 @@
> .endm
>
> .macro ldrh1 reg, ptr, val
> - uao_user_alternative 9998f, ldrh, ldtrh, \reg, \ptr, \val
> + user_ldst 9998f, ldtrh, \reg, \ptr, \val
> .endm
>
> .macro strh1 reg, ptr, val
> @@ -37,7 +37,7 @@
> .endm
>
> .macro ldr1 reg, ptr, val
> - uao_user_alternative 9998f, ldr, ldtr, \reg, \ptr, \val
> + user_ldst 9998f, ldtr, \reg, \ptr, \val
> .endm
>
> .macro str1 reg, ptr, val
> @@ -45,7 +45,7 @@
> .endm
>
> .macro ldp1 reg1, reg2, ptr, val
> - uao_ldp 9998f, \reg1, \reg2, \ptr, \val
> + user_ldp 9998f, \reg1, \reg2, \ptr, \val
> .endm
>
> .macro stp1 reg1, reg2, ptr, val
> diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
> index 80e37ada0ee1a..1f61cd0df0627 100644
> --- a/arch/arm64/lib/copy_in_user.S
> +++ b/arch/arm64/lib/copy_in_user.S
> @@ -22,35 +22,35 @@
> * x0 - bytes not copied
> */
> .macro ldrb1 reg, ptr, val
> - uao_user_alternative 9998f, ldrb, ldtrb, \reg, \ptr, \val
> + user_ldst 9998f, ldtrb, \reg, \ptr, \val
> .endm
>
> .macro strb1 reg, ptr, val
> - uao_user_alternative 9998f, strb, sttrb, \reg, \ptr, \val
> + user_ldst 9998f, sttrb, \reg, \ptr, \val
> .endm
>
> .macro ldrh1 reg, ptr, val
> - uao_user_alternative 9998f, ldrh, ldtrh, \reg, \ptr, \val
> + user_ldst 9998f, ldtrh, \reg, \ptr, \val
> .endm
>
> .macro strh1 reg, ptr, val
> - uao_user_alternative 9998f, strh, sttrh, \reg, \ptr, \val
> + user_ldst 9998f, sttrh, \reg, \ptr, \val
> .endm
>
> .macro ldr1 reg, ptr, val
> - uao_user_alternative 9998f, ldr, ldtr, \reg, \ptr, \val
> + user_ldst 9998f, ldtr, \reg, \ptr, \val
> .endm
>
> .macro str1 reg, ptr, val
> - uao_user_alternative 9998f, str, sttr, \reg, \ptr, \val
> + user_ldst 9998f, sttr, \reg, \ptr, \val
> .endm
>
> .macro ldp1 reg1, reg2, ptr, val
> - uao_ldp 9998f, \reg1, \reg2, \ptr, \val
> + user_ldp 9998f, \reg1, \reg2, \ptr, \val
> .endm
>
> .macro stp1 reg1, reg2, ptr, val
> - uao_stp 9998f, \reg1, \reg2, \ptr, \val
> + user_stp 9998f, \reg1, \reg2, \ptr, \val
> .endm
>
> end .req x5
> diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
> index 4ec59704b8f2d..043da90f5dd7d 100644
> --- a/arch/arm64/lib/copy_to_user.S
> +++ b/arch/arm64/lib/copy_to_user.S
> @@ -24,7 +24,7 @@
> .endm
>
> .macro strb1 reg, ptr, val
> - uao_user_alternative 9998f, strb, sttrb, \reg, \ptr, \val
> + user_ldst 9998f, sttrb, \reg, \ptr, \val
> .endm
>
> .macro ldrh1 reg, ptr, val
> @@ -32,7 +32,7 @@
> .endm
>
> .macro strh1 reg, ptr, val
> - uao_user_alternative 9998f, strh, sttrh, \reg, \ptr, \val
> + user_ldst 9998f, sttrh, \reg, \ptr, \val
> .endm
>
> .macro ldr1 reg, ptr, val
> @@ -40,7 +40,7 @@
> .endm
>
> .macro str1 reg, ptr, val
> - uao_user_alternative 9998f, str, sttr, \reg, \ptr, \val
> + user_ldst 9998f, sttr, \reg, \ptr, \val
> .endm
>
> .macro ldp1 reg1, reg2, ptr, val
> @@ -48,7 +48,7 @@
> .endm
>
> .macro stp1 reg1, reg2, ptr, val
> - uao_stp 9998f, \reg1, \reg2, \ptr, \val
> + user_stp 9998f, \reg1, \reg2, \ptr, \val
> .endm
>
> end .req x5
> diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
> index 03ca6d8b86706..351537c12f36e 100644
> --- a/arch/arm64/lib/mte.S
> +++ b/arch/arm64/lib/mte.S
> @@ -4,7 +4,7 @@
> */
> #include <linux/linkage.h>
>
> -#include <asm/alternative.h>
> +#include <asm/asm-uaccess.h>

Shouldn't that be in patch #8?

> #include <asm/assembler.h>
> #include <asm/mte.h>
> #include <asm/page.h>
> @@ -67,7 +67,7 @@ SYM_FUNC_START(mte_copy_tags_from_user)
> mov x3, x1
> cbz x2, 2f
> 1:
> - uao_user_alternative 2f, ldrb, ldtrb, w4, x1, 0
> + user_ldst 2f, ldtrb, w4, x1, 0
> lsl x4, x4, #MTE_TAG_SHIFT
> stg x4, [x0], #MTE_GRANULE_SIZE
> add x1, x1, #1
> @@ -94,7 +94,7 @@ SYM_FUNC_START(mte_copy_tags_to_user)
> 1:
> ldg x4, [x1]
> ubfx x4, x4, #MTE_TAG_SHIFT, #MTE_TAG_SIZE
> - uao_user_alternative 2f, strb, sttrb, w4, x0, 0
> + user_ldst 2f, sttrb, w4, x0, 0
> add x0, x0, #1
> add x1, x1, #MTE_GRANULE_SIZE
> subs x2, x2, #1
>

Otherwise, the rename looks OK to me, so with the nits picked,

Reviewed-by: Robin Murohy <robin.murphy@arm.com>