On Wed, Oct 12, 2022 at 05:18:02PM +0200, Kevin Brodsky wrote:
Commit title: there should be a space between each tag, i.e. "arm64: morello:".
On 12/10/2022 16:27, Beata Michalska wrote:
The alterations around the syscall framework made to enable support for capabilities, did spin the syscalls tracing out of control and broke a number of things on the way.
It would probably be helpful to say what was actually broken (i.e. it simply didn't build with CONFIG_FTRACE_SYSCALLS). Also worth separating the two issues - the SYSCALL_METADATA() override is required to fix things in any configuration for arm64, while the arch_syscall_addr() issue is specific to PCuABI.
I'll cover the details in the updated version. But while at it: maybe it would be better to further split the patch into two, with one covering the SYSCALL_METADATA changes that are needed for arm64 in general, and then the second one with Morello specific changes for arch_syscall_addr ? Or do you believe having the details in the commit message is enough even despite the title depicting Morello explicitly ?
In order to get it back in line (eventually), make necessary adjustments to SYSCALL_METADATA and provide dedicated implementation for arch_syscall_addr.
Fixes: bb3fbc44e783 ("arm64/syscalls: Allow syscalls to return capabilities") Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/ftrace.h | 4 ++++ arch/arm64/include/asm/syscall_wrapper.h | 5 ++++- arch/arm64/kernel/syscall.c | 12 ++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index 665373f441e1..89ff574a4a91 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -102,6 +102,10 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) return is_32bit_compat_task(); } +#if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_CHERI_PURECAP_UABI) +#define ARCH_HAS_SYSCALL_ADDR +#endif
- #define ARCH_HAS_SYSCALL_MATCH_SYM_NAME static inline bool arch_syscall_match_sym_name(const char *sym,
diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h index 97e08c4a6520..39957e54f8d3 100644 --- a/arch/arm64/include/asm/syscall_wrapper.h +++ b/arch/arm64/include/asm/syscall_wrapper.h @@ -141,7 +141,10 @@ struct pt_regs; #define __retptr__(name) name, _PTR #define __SYSCALL_ANNOTATE(name, ret_type) name, __SYSCALL_RET_T##ret_type #define SYSCALL_PREP(name, ...) __SYSCALL_ANNOTATE(_##name, __VA_ARGS__)
I still think that newline should stay.
Noted
+#ifdef CONFIG_FTRACE_SYSCALLS +#define SYSCALL_METADATA(x, name, ret_type, ...) \
- __SYSCALL_METADATA(x, name, __VA_ARGS__)
+#endif /*
- Some syscalls with no parameters return valid capabilities, so __SYSCALL_DEFINE0
- is added to handle such cases.
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index a0e91ea7b74b..4942fcb4d299 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -105,6 +105,18 @@ static inline bool has_syscall_work(unsigned long flags) return unlikely(flags & _TIF_SYSCALL_WORK); } +#if defined(CONFIG_CHERI_PURECAP_UABI) && defined(CONFIG_FTRACE_SYSCALLS)
Nit: would be better in the same order as above (FTRACE_SYSCALLS first).
Ditto
--- BR B.
Kevin
+unsigned long __init arch_syscall_addr(int nr) +{
- /*
* In this particular case, it makes no difference,* which member of the syscall_entry_t instance is being* provided - the address is all that matters here*/- return (unsigned long)sys_call_table[nr].syscall_fn;
+} +#endif
- int syscall_trace_enter(struct pt_regs *regs); void syscall_trace_exit(struct pt_regs *regs);