Hi Kofi,

Great start! Make the commit title to contain the morello tag as well, similar to the previous commits. Instead of "Begin translating" maybe just "translate"? Also, don't need the full stop:

arm64: morello: Translate helpers from asm to C

On 12-09-2023 17:41, Kofi Wilkinson wrote:

From: Kofi Wilkinson kofwil01@e129687.arm.com

I wonder how this "From: ..." ended up here. Anyway, no need for it, just remove it.

Translate __morello_cap_lo_hi_tag, __morello_merge_c_x, __morello_cap_has_executive, and morello_capcpy.

Many Morello helpers are currently implemented in assembly, mostly for historical resons. This patch is the beginning of an effort to

s/resons/reasons

rewrite them in C for the sake of readability.

Signed-off-by: Kofi Wilkinson <Kofi.Wilkinson@arm.com > --- arch/arm64/kernel/morello.c | 40 ++++++++++++++++++++++++++++----- arch/arm64/lib/morello.S | 45 ------------------------------------- 2 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index d999506509be..82b5cffb494e 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -20,13 +20,43 @@ #include <asm/morello.h> #include <asm/ptrace.h> -/* Private functions implemented in morello.S */ -void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val,

• 	     u8 *tag);
  

-void __morello_merge_c_x(uintcap_t *creg, u64 xreg); -bool __morello_cap_has_executive(uintcap_t cap); +void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, u8 *tag) +{

• *lo_val = cap;

Do the cast explicitly for readability: *lo_val = (u64)cap;

• *hi_val = cheri_copy_from_high((void *__capability) cap);

cheri_copy_from_high doesn't exist. You probably want the builtin: __builtin_cheri_copy_from_high(cap). Also nit: don't leave a space between the cast and variable and leave a space before __capability: i.e.: cheri_copy_from_high((void * __capability)cap)

• *tag = cheri_tag_get((void *__capability) cap);

The helpers from cheriintrin.h don't need the cast.

+}

+void __morello_merge_c_x(uintcap_t *creg, u64 xreg) +{

• if (cheri_address_get(*creg) != xreg)

• *creg = cheri_address_set(*creg, xreg);
  

+}

+bool __morello_cap_has_executive(uintcap_t cap) +{

• u32 perms = cheri_perms_get(cap);

Use cheri_perms_t instead of u32.

• return (perms >> MORELLO_CAP_PERM_EXECUTIVE_BIT) & 1;

nit: I would rewrite this as just: return perms & MORELLO_CAP_PERM_EXECUTIVE_MASK;

+}

• void __morello_thread_init_user(struct task_struct *tsk, uintcap_t ddc);

+void *morello_capcpy(void *dst, const void *src, size_t len) +{

• void *original_dst = dst;
• uintcap_t *cdst = (uintcap_t *) dst;
• uintcap_t *csrc = (uintcap_t *) src; > +
• while (len > 0) {

• *cdst = *csrc;
  

• csrc++;
  

• cdst++;
  

• len -= 16;
  

• }
• return original_dst;

+}

There is only one instance where this helper is used and in that instance you could just replace it with memcpy(dst, src, len). memcpy() preserves the tags as well. If you also agree that we can get rid of this helper, don't forget to also remove its prototype from "arch/arm64/include/asm/morello.h".

Great cleanup so far, well done! Looking forward to some more translated helpers.

Best, Tudor

• static uintcap_t morello_sentry_unsealcap __ro_after_init;

/* DDC_ELx reset value (low/high 64 bits), as defined in the Morello spec */ diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S index 5bc1540c829f..a009d9b3c2db 100644 --- a/arch/arm64/lib/morello.S +++ b/arch/arm64/lib/morello.S @@ -46,24 +46,6 @@ SYM_FUNC_START(morello_build_any_user_cap) ret SYM_FUNC_END(morello_build_any_user_cap) -SYM_FUNC_START(morello_capcpy)

• mov x3, x0
• and x4, x2, #0x10 // Bytes to reach 32-byte alignment (0 or 16)
• subs x5, x2, x4 // 32-byte aligned length
• b.eq 2f

-1:

• ldp c6, c7, [x1], #32 // 32-byte loop
• stp c6, c7, [x3], #32
• subs x5, x5, #32
• b.ne 1b

-2:

• cbz x4, 3f // 16-byte leftover (if any)
• ldr c6, [x1], #16
• str c6, [x3], #16

-3:

• ret

-SYM_FUNC_END(morello_capcpy)

• SYM_FUNC_START(__morello_thread_init_user) mov x9, #THREAD_MORELLO_USER_STATE add x0, x0, x9 // x0 = tsk->thread.morello_user_state

@@ -217,30 +199,3 @@ SYM_FUNC_START(__morello_put_user_cap_asm) str w3, [x2] ret SYM_FUNC_END(__morello_put_user_cap_asm)

-SYM_FUNC_START(__morello_cap_lo_hi_tag)

• str x0, [x1]
• cfhi x4, c0 // Extract upper 64 bits
• str x4, [x2]
• gctag x4, c0
• strb w4, [x3]
• ret

-SYM_FUNC_END(__morello_cap_lo_hi_tag)

-SYM_FUNC_START(__morello_merge_c_x)

• ldr c2, [x0]
• cmp x2, x1
• b.eq 1f
• scvalue c2, c2, x1
• str c2, [x0]

-1:

• ret

-SYM_FUNC_END(__morello_merge_c_x)

-SYM_FUNC_START(__morello_cap_has_executive)

• gcperm x0, c0
• ubfx x0, x0, #MORELLO_CAP_PERM_EXECUTIVE_BIT, #1
• ret

-SYM_FUNC_END(__morello_cap_has_executive)