On 02/09/2022 11:45, Sherwin da Cruz wrote:
In the transitional ABI specific registers are initialised for purecap
This is true in PCuABI in general, the transitional ABI just doesn't specify them so much. Since you don't mention the transitional ABI in patch 2, I think it's fine not to mention it here either (the TODO is clear enough).
applications to make use of. This is set in start_thread(). The macro START_THREAD is specialised to allow returning the value of argc to be placed in x0.
Signed-off-by: Sherwin da Cruz sherwin.dacruz@arm.com
arch/arm64/include/asm/elf.h | 3 +++ arch/arm64/include/asm/processor.h | 30 ++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h index 9eb86b4cb691..a666e1ccfee1 100644 --- a/arch/arm64/include/asm/elf.h +++ b/arch/arm64/include/asm/elf.h @@ -164,6 +164,9 @@ typedef unsigned long elf_greg_t; typedef elf_greg_t elf_gregset_t[ELF_NGREG]; typedef struct user_fpsimd_state elf_fpregset_t; +#define START_THREAD(elf_ex, regs, elf_entry, bprm) \
You have a mix of tab and spaces aligning , make sure to use just tabs.
- start_thread(regs, elf_entry, bprm->p, bprm->argc, bprm->envc)
- #define SET_PERSONALITY_AARCH64() \ ({ \ current->personality &= ~READ_IMPLIES_EXEC; \
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h index 5bdecb185f63..88de88ba506e 100644 --- a/arch/arm64/include/asm/processor.h +++ b/arch/arm64/include/asm/processor.h @@ -238,6 +238,31 @@ void tls_preserve_current_state(void); .fpsimd_cpu = NR_CPUS, \ } +static inline int init_gp_regs(struct pt_regs *regs, unsigned long sp,
int argc, int envc) {
{ is always at the start of a line when defining a function (you can use checkpatch.pl to find about this kind of style guide issues). Also, there are various styles used to indent hanging parameters in the kernel, but other functions in this file align them on the opening parenthesis so you should do the same here.
- int retval = 0;
+#ifdef CONFIG_CHERI_PURECAP_UABI
- unsigned long argv, envp, auxv;
- /*
* TODO [PCuABI]: When argv/envp/auxv is moved off the stack, update the registers* x1-x3 with the new pointer values, and c1-c3 with new capabilities to them.
Alright so there are two things here really: 1. when the initial args are moved, as you mention, and 2. restricting bounds / permissions. 1. is about x1-x3, and 2. is about c1-c3. The two are not interdependent - you don't have to do both at the same time. Let's make it clear here.
** In ret_to_user c regs are first loaded then merged with x regs if their values* are different. Hence we load capabilities in c regs and the value in x regs.*/- retval = argc; /* Placed in x0 */
- argv = sp + 1 * sizeof(user_uintptr_t); /* Increment past argc on stack */
- envp = argv + (argc + 1) * sizeof(user_uintptr_t); /* Go past arg vals + NULL */
- auxv = envp + (envc + 1) * sizeof(user_uintptr_t); /* Go past env vals + NULL */
- regs->cregs[1] = regs->cregs[2] = regs->cregs[3] = regs->csp;
- regs->regs[1] = argv;
- regs->regs[2] = envp;
- regs->regs[3] = auxv;
+#endif /* CONFIG_CHERI_PURECAP_UABI */
- return retval;
+}
- static inline void start_thread_common(struct pt_regs *regs, unsigned long pc) { memset(regs, 0, sizeof(*regs));
@@ -248,8 +273,8 @@ static inline void start_thread_common(struct pt_regs *regs, unsigned long pc) regs->pmr_save = GIC_PRIO_IRQON; } -static inline void start_thread(struct pt_regs *regs, unsigned long pc,
unsigned long sp)+static inline int start_thread(struct pt_regs *regs, unsigned long pc,
unsigned long sp, int argc, int envc)@@ -258,6 +283,7 @@ static inline void start_thread(struct pt_regs *regs, unsigned long pc, if (system_supports_morello()) morello_thread_start(regs, pc);
Nit: would look nicer with an empty line here.
Kevin
- return init_gp_regs(regs, sp, argc, envc); }
#ifdef CONFIG_COMPAT