Hello all,
Many of the Morello helper functions are implemented in assembly due to historical reasons. This patch series aims to translate the helpers into C functions to improve their readability, introducing additional helpers where necessary such as to access capability-based system registers. The newly translated helpers are defined directly in morello.c.
This patch series also removes the now empty morello.S file, and updates the Makefile accordingly. It also tidies up definitions of assembly-only values, removing these where necessary.
V1 separates the patch series into two patches; one to add the register helpers, and another to translate the assembly helpers (and tidy up). V1 also corrects the more complicated helpers, whilst removing unnecessary casts and finally implementing some feedback for improving code clarity.
The review branch is accessible here: https://git.morello-project.org/arkamnite/linux/-/commits/morello%2Farm64_v1
The related issue is here: https://git.morello-project.org/morello/kernel/linux/-/issues/61
Many thanks,
Akram
Akram Ahmad (2): arm64: add {read, write, clear}_cap_sysreg helpers for Morello arm64: rewrite Morello helpers in C
arch/arm64/include/asm/assembler.h | 10 -- arch/arm64/include/asm/morello.h | 4 + arch/arm64/include/asm/sysreg.h | 18 +++ arch/arm64/kernel/asm-offsets.c | 8 -- arch/arm64/kernel/morello.c | 125 +++++++++++++++++- arch/arm64/lib/Makefile | 3 +- arch/arm64/lib/morello.S | 205 ----------------------------- 7 files changed, 141 insertions(+), 232 deletions(-) delete mode 100644 arch/arm64/lib/morello.S
Currently, there is no way to access the system registers on Morello without writing inline assembly statements for each access. This patch directly adds the {read, write, clear}_cap_sysreg helper functions to sysreg.h
Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com --- arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 9988e289ce20..6ab068851422 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1134,6 +1134,24 @@ asm volatile(__msr_s(r, "%x0") : : "rZ" (__val)); \ } while (0)
+#ifdef CONFIG_ARM64_MORELLO +#define read_cap_sysreg(r) ({ \ + uintcap_t __cap; \ + asm volatile("mrs %0, " __stringify(r) : "=r" (__cap)); \ + __cap; \ +}) + +#define write_cap_sysreg(v, r) do { \ + uintcap_t __cap = (uintcap_t)(v); \ + asm volatile("msr " __stringify(r) ", %0" \ + : : "rZ" (__cap)); \ +} while (0) + +#define clear_cap_sysreg(r) ({ \ + write_cap_sysreg(0, r); \ +}) +#endif + /* * Modify bits in a sysreg. Bits in the clear mask are zeroed, then bits in the * set mask are set. Other bits are left as-is.
Many of the Morello helper functions are implemented in assembly due to historical reasons. This patch rewrites all of the helpers defined in morello.S into C to improve their readability. This patch also removes the original morello.S assembly file and edits the Makefile accordingly.
Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com --- arch/arm64/include/asm/assembler.h | 10 -- arch/arm64/include/asm/morello.h | 4 + arch/arm64/kernel/asm-offsets.c | 8 -- arch/arm64/kernel/morello.c | 125 +++++++++++++++++- arch/arm64/lib/Makefile | 3 +- arch/arm64/lib/morello.S | 205 ----------------------------- 6 files changed, 123 insertions(+), 232 deletions(-) delete mode 100644 arch/arm64/lib/morello.S
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 7d4046ded4df..11596bee98fe 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -597,16 +597,6 @@ alternative_endif mrs \rd, sp_el0 .endm
-/* - * Return a pointer to tsk's pt_regs. - */ - .macro get_task_pt_regs, rd:req, tsk:req - /* See task_pt_regs() in processor.h */ - ldr \rd, [\tsk, #TSK_STACK] - add \rd, \rd, #THREAD_SIZE - sub \rd, \rd, #PT_REGS_SIZE - .endm - /* * Offset ttbr1 to allow for 48-bit kernel VAs set with 52-bit PTRS_PER_PGD. * orr is used as it can cover the immediate value (and is idempotent). diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index 357f403d073d..be76926e5d0f 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -5,10 +5,14 @@ #ifndef __ASM_MORELLO_H #define __ASM_MORELLO_H
+#ifdef __ASSEMBLY__ + /* Architectural definitions */ #define MORELLO_CAP_PERM_EXECUTIVE_BIT 1 #define MORELLO_CAP_PERM_EXECUTIVE_MASK (1 << MORELLO_CAP_PERM_EXECUTIVE_BIT)
+#endif + #ifndef __ASSEMBLY__
struct pt_regs; diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 90ca25039583..defc1f6f5d11 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -55,14 +55,6 @@ int main(void) #endif #ifdef CONFIG_ARM64_MTE DEFINE(THREAD_MTE_CTRL, offsetof(struct task_struct, thread.mte_ctrl)); -#endif -#ifdef CONFIG_ARM64_MORELLO - DEFINE(THREAD_MORELLO_USER_STATE, offsetof(struct task_struct, thread.morello_user_state)); - BLANK(); - DEFINE(MORELLO_STATE_CTPIDR, offsetof(struct morello_state, ctpidr)); - DEFINE(MORELLO_STATE_DDC, offsetof(struct morello_state, ddc)); - DEFINE(MORELLO_STATE_CID, offsetof(struct morello_state, cid)); - DEFINE(MORELLO_STATE_CCTLR, offsetof(struct morello_state, cctlr)); #endif BLANK(); DEFINE(S_X0, offsetof(struct pt_regs, regs[0])); diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 7dfbca70e11d..8108e6256de2 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -20,12 +20,24 @@ #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_thread_init_user(struct task_struct *tsk, uintcap_t ddc); +static void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val, + u8 *tag) +{ + *lo_val = (u64)cap; + *hi_val = (u64)__builtin_cheri_copy_from_high((void *__capability)cap); + *tag = cheri_tag_get((void *__capability)cap); +} + +static void __morello_merge_c_x(uintcap_t *creg, u64 xreg) +{ + if (cheri_address_get(*creg) != xreg) + *creg = cheri_address_set(*creg, xreg); +} + +static bool __morello_cap_has_executive(uintcap_t cap) +{ + return cheri_perms_get(cap) & ARM_CAP_PERMISSION_EXECUTIVE; +}
static uintcap_t morello_sentry_unsealcap __ro_after_init;
@@ -53,6 +65,26 @@ static void update_regs_c64(struct pt_regs *regs, unsigned long pc) } }
+void morello_cap_get_val_tag(uintcap_t cap, __uint128_t *val, u8 *tag) +{ + *((uintcap_t *)val) = cheri_tag_clear(cap); + *tag = cheri_tag_get(cap); +} + +uintcap_t morello_build_any_user_cap(const __uint128_t *val, u8 tag) +{ + uintcap_t cap = *((uintcap_t *)val); + + if (!tag) + return cheri_tag_clear(cap); + + uintcap_t sealing_cap = cheri_type_copy(cheri_user_root_allperms_cap, cap); + + cap = (user_uintptr_t)cheri_cap_build(cap, cheri_user_root_allperms_cap); + cap = cheri_seal_conditionally(cap, sealing_cap); + return cap; +} + void morello_thread_start(struct pt_regs *regs, unsigned long pc) { update_regs_c64(regs, pc); @@ -79,7 +111,86 @@ void morello_thread_init_user(void) uintcap_t ddc = is_pure_task() ? cheri_user_root_cap : cheri_user_root_allperms_cap;
- __morello_thread_init_user(current, ddc); + struct morello_state *morello_state = ¤t->thread.morello_user_state; + + /* + * tls_thread_flush() does not touch rctpidr_el0 so this must be zeroed + * here. We do not need to initialise its value in morello_user_state. + * Only the ddc_el0 register must be initialised to the specific value; + * RDDC is set to a null capability as processes are always started in + * Executive. + */ + clear_cap_sysreg(rctpidr_el0); + + write_cap_sysreg(ddc, ddc_el0); + clear_cap_sysreg(rddc_el0); + morello_state->ddc = ddc; + morello_state->rddc = (uintcap_t)0; + clear_cap_sysreg(cid_el0); + morello_state->cid = 0; + write_sysreg(0, cctlr_el0); + morello_state->cctlr = 0; +} + +void morello_thread_save_user_state(struct task_struct *tsk) +{ + struct morello_state *morello_state = &tsk->thread.morello_user_state; + + /* (R)CTPIDR is handled by task_save_user_tls */ + morello_state->ddc = read_cap_sysreg(ddc_el0); + morello_state->rddc = read_cap_sysreg(rddc_el0); + morello_state->cid = read_cap_sysreg(cid_el0); + morello_state->cctlr = read_sysreg(cctlr_el0); +} + +void morello_thread_restore_user_state(struct task_struct *tsk) +{ + struct morello_state *morello_state = &tsk->thread.morello_user_state; + + /* (R)CTPIDR is handled by task_restore_user_tls */ + write_cap_sysreg(morello_state->ddc, ddc_el0); + write_cap_sysreg(morello_state->rddc, rddc_el0); + write_cap_sysreg(morello_state->cid, cid_el0); + write_sysreg(morello_state->cctlr, cctlr_el0); +} + +void morello_task_save_user_tls(struct task_struct *tsk, user_uintptr_t *tp_ptr) +{ + struct morello_state *morello_state = &tsk->thread.morello_user_state; + struct pt_regs *regs = task_pt_regs(tsk); + uintcap_t active_ctpidr; + + morello_state->ctpidr = read_cap_sysreg(ctpidr_el0); + morello_state->rctpidr = read_cap_sysreg(rctpidr_el0); + + if (__morello_cap_has_executive(regs->pcc)) + active_ctpidr = read_cap_sysreg(ctpidr_el0); + else + active_ctpidr = read_cap_sysreg(rctpidr_el0); + + *tp_ptr = (user_uintptr_t)active_ctpidr; +} + +void morello_task_restore_user_tls(struct task_struct *tsk, + const user_uintptr_t *tp_ptr) +{ + struct morello_state *morello_state = &tsk->thread.morello_user_state; + struct pt_regs *regs = task_pt_regs(tsk); + uintcap_t *active_ctpidr; + + if (__morello_cap_has_executive(regs->pcc)) + active_ctpidr = &morello_state->ctpidr; + else + active_ctpidr = &morello_state->rctpidr; + +#ifdef CONFIG_CHERI_PURECAP_UABI + *active_ctpidr = (uintcap_t)*tp_ptr; +#else + __morello_merge_c_x(active_ctpidr, (u64)*tp_ptr); +#endif + + write_cap_sysreg(morello_state->ctpidr, ctpidr_el0); + write_cap_sysreg(morello_state->rctpidr, rctpidr_el0); }
#ifdef CONFIG_CHERI_PURECAP_UABI diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile index 5fa6f0645edc..2e46ed437b69 100644 --- a/arch/arm64/lib/Makefile +++ b/arch/arm64/lib/Makefile @@ -27,8 +27,7 @@ obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
obj-$(CONFIG_ARM64_MTE) += mte.o
-obj-$(CONFIG_ARM64_MORELLO) += morello.o \ - copy_from_user_with_captags.o \ +obj-$(CONFIG_ARM64_MORELLO) += copy_from_user_with_captags.o \ copy_to_user_with_captags.o
obj-$(CONFIG_KASAN_SW_TAGS) += kasan_sw_tags.o diff --git a/arch/arm64/lib/morello.S b/arch/arm64/lib/morello.S deleted file mode 100644 index 0800375ef33d..000000000000 --- a/arch/arm64/lib/morello.S +++ /dev/null @@ -1,205 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright (C) 2020 Arm Ltd. - * - * TODO: most of the functions in this file should be reimplemented in C to - * improve readability. - */ - -#include <linux/linkage.h> - -#include <asm/asm-uaccess.h> -#include <asm/assembler.h> -#include <asm/morello.h> -#include <asm/sysreg.h> - -SYM_FUNC_START(morello_cap_get_val_tag) - gctag x3, c0 - clrtag c0, c0 // Store the 128-bit value without the tag - str c0, [x1] - strb w3, [x2] - - ret -SYM_FUNC_END(morello_cap_get_val_tag) - -SYM_FUNC_START(morello_build_any_user_cap) - ldr c0, [x0] - cbz w1, 1f - /* - * The tag should be set, build a valid capability from the root - * capability. - * In case c0 is sealed (non-zero object type), we need to extract the - * object type first to be able to reseal it after BUILD. The CSEAL - * instruction is used to cover the case where c0 was not sealed. - */ - adr_l x3, cheri_user_root_allperms_cap - ldr c3, [x3] - - cpytype c4, c3, c0 - build c0, c0, c3 - cseal c0, c0, c4 - b 2f -1: - /* The tag should be cleared, make sure it is. */ - clrtag c0, c0 -2: - ret -SYM_FUNC_END(morello_build_any_user_cap) - -SYM_FUNC_START(__morello_thread_init_user) - mov x9, #THREAD_MORELLO_USER_STATE - add x0, x0, x9 // x0 = tsk->thread.morello_user_state - - /* - * CTPIDR doesn't need to be initialised explicitly: - * - tls_thread_flush() already zeroes tpidr_el0, zeroing ctpidr_el0 as - * well - * - The value stored in thread.morello_user_state will be set the next - * time task_save_user_tls() is called, like thread_struct.uw.tp_value. - * - * tls_thread_flush() does not touch rcsp_el0, so we need to zero it - * here, but its value in morello_user_state does not need to be - * initialised here either. - */ - msr rctpidr_el0, czr - - /* DDC: initialised to the specified value */ - msr ddc_el0, c1 - /* RDDC: null capability (processes are always started in Executive) */ - msr rddc_el0, czr - stp c1, czr, [x0, #MORELLO_STATE_DDC] - /* CID: null capability */ - msr cid_el0, czr - str czr, [x0, #MORELLO_STATE_CID] - /* CCTLR: all bits cleared */ - msr cctlr_el0, xzr - str xzr, [x0, #MORELLO_STATE_CCTLR] - - ret -SYM_FUNC_END(__morello_thread_init_user) - -SYM_FUNC_START(morello_thread_save_user_state) - mov x9, #THREAD_MORELLO_USER_STATE - add x0, x0, x9 // x0 = tsk->thread.morello_user_state - - /* (R)CTPIDR is handled by task_save_user_tls */ - mrs c1, ddc_el0 - mrs c2, rddc_el0 - stp c1, c2, [x0, #MORELLO_STATE_DDC] - mrs c1, cid_el0 - str c1, [x0, #MORELLO_STATE_CID] - mrs x1, cctlr_el0 - str x1, [x0, #MORELLO_STATE_CCTLR] - - ret -SYM_FUNC_END(morello_thread_save_user_state) - -SYM_FUNC_START(morello_thread_restore_user_state) - mov x9, #THREAD_MORELLO_USER_STATE - add x0, x0, x9 // x0 = tsk->thread.morello_user_state - - /* (R)CTPIDR is handled by task_restore_user_tls */ - ldp c1, c2, [x0, #MORELLO_STATE_DDC] - msr ddc_el0, c1 - msr rddc_el0, c2 - ldr c1, [x0, #MORELLO_STATE_CID] - msr cid_el0, c1 - ldr x1, [x0, #MORELLO_STATE_CCTLR] - msr cctlr_el0, x1 - - ret -SYM_FUNC_END(morello_thread_restore_user_state) - -SYM_FUNC_START(morello_task_save_user_tls) - get_task_pt_regs x8, x0 - mov x9, #THREAD_MORELLO_USER_STATE - add x0, x0, x9 // x0 = tsk->thread.morello_user_state - - mrs c2, ctpidr_el0 - mrs c3, rctpidr_el0 - /* Save CTPIDR and RCTPIDR */ - stp c2, c3, [x0, #MORELLO_STATE_CTPIDR] - - ldr c5, [x8, #S_PCC] - /* - * If the task is currently running in Restricted, save the lower 64 bits - * of RCTPIDR (RTPIDR) in tsk->thread instead of TPIDR. - */ - morello_tst_cap_has_executive c5, x6 // Task running in Executive? -#ifdef CONFIG_CHERI_PURECAP_UABI - csel c2, c2, c3, ne // If not, save RTPIDR - str c2, [x1] -#else - csel x2, x2, x3, ne // If not, save RTPIDR - str x2, [x1] -#endif - ret -SYM_FUNC_END(morello_task_save_user_tls) - -SYM_FUNC_START(morello_task_restore_user_tls) - get_task_pt_regs x8, x0 - mov x9, #THREAD_MORELLO_USER_STATE - add x0, x0, x9 // x0 = tsk->thread.morello_user_state - - /* Load CTPIDR, RCTPIDR and the 64-bit TLS pointer */ - ldp c2, c3, [x0, #MORELLO_STATE_CTPIDR] -#ifdef CONFIG_CHERI_PURECAP_UABI - ldr c4, [x1] -#else - ldr x4, [x1] -#endif - - /* - * The 64-bit TLS pointer may have been modified from within the kernel - * (e.g. through ptrace) since morello_task_save_user_tls was called. - * Merge it back into the active capability TLS pointer, that is RCTPIDR - * if the task is running in Restricted and CTPIDR otherwise. - */ - ldr c5, [x8, #S_PCC] - morello_tst_cap_has_executive c5, x6 // Task running in Executive? - b.eq 1f // If not, merge into RCTPIDR -#ifdef CONFIG_CHERI_PURECAP_UABI - mov c2, c4 // Save the TLS pointer into CTPIDR -#else - morello_merge_c_x 2, x4 // Merge the TLS pointer into CTPIDR -#endif - b 2f -1: -#ifdef CONFIG_CHERI_PURECAP_UABI - mov c3, c4 // Save the TLS pointer into RCTPIDR -#else - morello_merge_c_x 3, x4 // Merge the TLS pointer into RCTPIDR -#endif -2: - msr ctpidr_el0, c2 - msr rctpidr_el0, c3 - - ret -SYM_FUNC_END(morello_task_restore_user_tls) - - -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)
Nit: you could use "arm64: morello: " as tag, as we've done so far for such Morello-specific changes. Of course no need to keep "for Morello" in that case. Patch 2 should use the same tag as well.
On 22/02/2024 12:15, Akram Ahmad wrote:
Currently, there is no way to access the system registers on Morello without writing inline assembly statements for each access. This patch directly adds the {read, write, clear}_cap_sysreg helper functions to sysreg.h
Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com
arch/arm64/include/asm/sysreg.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 9988e289ce20..6ab068851422 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1134,6 +1134,24 @@ asm volatile(__msr_s(r, "%x0") : : "rZ" (__val)); \ } while (0) +#ifdef CONFIG_ARM64_MORELLO +#define read_cap_sysreg(r) ({ \
- uintcap_t __cap; \
- asm volatile("mrs %0, " __stringify(r) : "=r" (__cap)); \
- __cap; \
+})
+#define write_cap_sysreg(v, r) do { \
- uintcap_t __cap = (uintcap_t)(v); \
- asm volatile("msr " __stringify(r) ", %0" \
: : "rZ" (__cap)); \+} while (0)
+#define clear_cap_sysreg(r) ({ \
- write_cap_sysreg(0, r); \
+})
I hadn't realised that there is no clear_sysreg() helper. I don't think there's a particular reason to have one for capability sysregs. The explicit write_sysreg(0, ...) is commonly used in arm64 code today, so we should probably do the same for capability sysregs.
Kevin
+#endif
/*
- Modify bits in a sysreg. Bits in the clear mask are zeroed, then bits in the
- set mask are set. Other bits are left as-is.
On 22/02/2024 12:15, Akram Ahmad wrote:
Many of the Morello helper functions are implemented in assembly due to historical reasons. This patch rewrites all of the helpers defined in morello.S into C to improve their readability. This patch also removes the original morello.S assembly file and edits the Makefile accordingly.
Signed-off-by: Akram Ahmad Akram.Ahmad@arm.com
arch/arm64/include/asm/assembler.h | 10 -- arch/arm64/include/asm/morello.h | 4 + arch/arm64/kernel/asm-offsets.c | 8 -- arch/arm64/kernel/morello.c | 125 +++++++++++++++++- arch/arm64/lib/Makefile | 3 +- arch/arm64/lib/morello.S | 205 ----------------------------- 6 files changed, 123 insertions(+), 232 deletions(-) delete mode 100644 arch/arm64/lib/morello.S
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 7d4046ded4df..11596bee98fe 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -597,16 +597,6 @@ alternative_endif mrs \rd, sp_el0 .endm -/*
- Return a pointer to tsk's pt_regs.
- */
- .macro get_task_pt_regs, rd:req, tsk:req
- /* See task_pt_regs() in processor.h */
- ldr \rd, [\tsk, #TSK_STACK]
- add \rd, \rd, #THREAD_SIZE
- sub \rd, \rd, #PT_REGS_SIZE
- .endm
/*
- Offset ttbr1 to allow for 48-bit kernel VAs set with 52-bit PTRS_PER_PGD.
- orr is used as it can cover the immediate value (and is idempotent).
diff --git a/arch/arm64/include/asm/morello.h b/arch/arm64/include/asm/morello.h index 357f403d073d..be76926e5d0f 100644 --- a/arch/arm64/include/asm/morello.h +++ b/arch/arm64/include/asm/morello.h @@ -5,10 +5,14 @@ #ifndef __ASM_MORELLO_H #define __ASM_MORELLO_H +#ifdef __ASSEMBLY__
The #ifndef __ASSEMBLY__ below is followed by an #else section, so it would be better to move the #define's there instead (avoids adding a new #ifdef).
/* Architectural definitions */ #define MORELLO_CAP_PERM_EXECUTIVE_BIT 1 #define MORELLO_CAP_PERM_EXECUTIVE_MASK (1 << MORELLO_CAP_PERM_EXECUTIVE_BIT) +#endif
#ifndef __ASSEMBLY__ struct pt_regs; diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 90ca25039583..defc1f6f5d11 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -55,14 +55,6 @@ int main(void) #endif #ifdef CONFIG_ARM64_MTE DEFINE(THREAD_MTE_CTRL, offsetof(struct task_struct, thread.mte_ctrl)); -#endif -#ifdef CONFIG_ARM64_MORELLO
- DEFINE(THREAD_MORELLO_USER_STATE, offsetof(struct task_struct, thread.morello_user_state));
- BLANK();
- DEFINE(MORELLO_STATE_CTPIDR, offsetof(struct morello_state, ctpidr));
- DEFINE(MORELLO_STATE_DDC, offsetof(struct morello_state, ddc));
- DEFINE(MORELLO_STATE_CID, offsetof(struct morello_state, cid));
- DEFINE(MORELLO_STATE_CCTLR, offsetof(struct morello_state, cctlr));
#endif BLANK(); DEFINE(S_X0, offsetof(struct pt_regs, regs[0])); diff --git a/arch/arm64/kernel/morello.c b/arch/arm64/kernel/morello.c index 7dfbca70e11d..8108e6256de2 100644 --- a/arch/arm64/kernel/morello.c +++ b/arch/arm64/kernel/morello.c @@ -20,12 +20,24 @@ #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_thread_init_user(struct task_struct *tsk, uintcap_t ddc); +static void __morello_cap_lo_hi_tag(uintcap_t cap, u64 *lo_val, u64 *hi_val,
u8 *tag)+{
- *lo_val = (u64)cap;
- *hi_val = (u64)__builtin_cheri_copy_from_high((void *__capability)cap);
No need to cast to u64, this is the type that copy_from_high returns.
- *tag = cheri_tag_get((void *__capability)cap);
No need for the cast here.
+}
+static void __morello_merge_c_x(uintcap_t *creg, u64 xreg) +{
- if (cheri_address_get(*creg) != xreg)
*creg = cheri_address_set(*creg, xreg);+}
+static bool __morello_cap_has_executive(uintcap_t cap) +{
- return cheri_perms_get(cap) & ARM_CAP_PERMISSION_EXECUTIVE;
+}
These functions should be moved with the other static functions. Generally speaking functions are defined last in a .c file (after #include's, constants, globals). The declarations were a bit of a special case (they normally belong to a header, but notionally they were private to this file).
We could also drop the __morello prefix, which was used to make clear the assembly functions were private. The existing static functions have no prefix, e.g. is_pure_task().
static uintcap_t morello_sentry_unsealcap __ro_after_init; @@ -53,6 +65,26 @@ static void update_regs_c64(struct pt_regs *regs, unsigned long pc) } } +void morello_cap_get_val_tag(uintcap_t cap, __uint128_t *val, u8 *tag) +{
- *((uintcap_t *)val) = cheri_tag_clear(cap);
Nit: two spaces between uintcap_t and *.
- *tag = cheri_tag_get(cap);
+}
+uintcap_t morello_build_any_user_cap(const __uint128_t *val, u8 tag) +{
- uintcap_t cap = *((uintcap_t *)val);
- if (!tag)
return cheri_tag_clear(cap);- uintcap_t sealing_cap = cheri_type_copy(cheri_user_root_allperms_cap, cap);
- cap = (user_uintptr_t)cheri_cap_build(cap, cheri_user_root_allperms_cap);
Arguments to cheri_cap_build() are reversed compared to the BUILD instruction. Unfortunately that's pretty hard to figure out, the safest is to check with Compiler Explorer [1].
[1] https://cheri-compiler-explorer.cl.cam.ac.uk/
- cap = cheri_seal_conditionally(cap, sealing_cap);
- return cap;
+}
void morello_thread_start(struct pt_regs *regs, unsigned long pc) { update_regs_c64(regs, pc); @@ -79,7 +111,86 @@ void morello_thread_init_user(void) uintcap_t ddc = is_pure_task() ? cheri_user_root_cap : cheri_user_root_allperms_cap;
Nit: generally no empty line between variable declarations.
- __morello_thread_init_user(current, ddc);
- struct morello_state *morello_state = ¤t->thread.morello_user_state;
- /*
* tls_thread_flush() does not touch rctpidr_el0 so this must be zeroed* here. We do not need to initialise its value in morello_user_state.
It's not clear why, because this is no longer preceded by the comment about CTPIDR. Is there a particular reason not to keep it?
* Only the ddc_el0 register must be initialised to the specific value;* RDDC is set to a null capability as processes are always started in* Executive.*/- clear_cap_sysreg(rctpidr_el0);
- write_cap_sysreg(ddc, ddc_el0);
- clear_cap_sysreg(rddc_el0);
- morello_state->ddc = ddc;
- morello_state->rddc = (uintcap_t)0;
There's no cast in the same situation two lines below. No strong preference but it would be better to do the same in both cases.
Also maybe add an empty line here to make the grouping clearer, and between CID and CCTLR too.
- clear_cap_sysreg(cid_el0);
- morello_state->cid = 0;
- write_sysreg(0, cctlr_el0);
- morello_state->cctlr = 0;
+}
+void morello_thread_save_user_state(struct task_struct *tsk) +{
- struct morello_state *morello_state = &tsk->thread.morello_user_state;
- /* (R)CTPIDR is handled by task_save_user_tls */
- morello_state->ddc = read_cap_sysreg(ddc_el0);
- morello_state->rddc = read_cap_sysreg(rddc_el0);
- morello_state->cid = read_cap_sysreg(cid_el0);
- morello_state->cctlr = read_sysreg(cctlr_el0);
+}
+void morello_thread_restore_user_state(struct task_struct *tsk) +{
- struct morello_state *morello_state = &tsk->thread.morello_user_state;
- /* (R)CTPIDR is handled by task_restore_user_tls */
- write_cap_sysreg(morello_state->ddc, ddc_el0);
- write_cap_sysreg(morello_state->rddc, rddc_el0);
- write_cap_sysreg(morello_state->cid, cid_el0);
- write_sysreg(morello_state->cctlr, cctlr_el0);
+}
+void morello_task_save_user_tls(struct task_struct *tsk, user_uintptr_t *tp_ptr) +{
- struct morello_state *morello_state = &tsk->thread.morello_user_state;
- struct pt_regs *regs = task_pt_regs(tsk);
- uintcap_t active_ctpidr;
- morello_state->ctpidr = read_cap_sysreg(ctpidr_el0);
- morello_state->rctpidr = read_cap_sysreg(rctpidr_el0);
- if (__morello_cap_has_executive(regs->pcc))
active_ctpidr = read_cap_sysreg(ctpidr_el0);
Duplicating calls to read*_sysreg should be avoided. Because they're implemented as asm volatile, they cannot be optimised out by the compiler (i.e. the register will be read every time the function is called). On the other hand, morello_state is a regular pointer, so the compiler can assume that morello_state->ctpidr doesn't change, and it doesn't need to be loaded again if we refer to it here.
- else
active_ctpidr = read_cap_sysreg(rctpidr_el0);- *tp_ptr = (user_uintptr_t)active_ctpidr;
+}
+void morello_task_restore_user_tls(struct task_struct *tsk,
const user_uintptr_t *tp_ptr)+{
- struct morello_state *morello_state = &tsk->thread.morello_user_state;
- struct pt_regs *regs = task_pt_regs(tsk);
- uintcap_t *active_ctpidr;
- if (__morello_cap_has_executive(regs->pcc))
active_ctpidr = &morello_state->ctpidr;- else
active_ctpidr = &morello_state->rctpidr;+#ifdef CONFIG_CHERI_PURECAP_UABI
- *active_ctpidr = (uintcap_t)*tp_ptr;
+#else
- __morello_merge_c_x(active_ctpidr, (u64)*tp_ptr);
Given the #ifdef, we can drop the casts in both cases here (user_uintptr_t is uintcap_t in PCuABI by definition, and u64 otherwise).
Kevin
+#endif
- write_cap_sysreg(morello_state->ctpidr, ctpidr_el0);
- write_cap_sysreg(morello_state->rctpidr, rctpidr_el0);
} [...]
linux-morello@op-lists.linaro.org
-
Akram Ahmad -
Kevin Brodsky