Re: [PATCH 1/3] powerpc: Split user/kernel definitions of struct pt_regs

[Madhavan Srinivasan <maddy@linux.vnet.ibm.com>]

On Saturday 13 October 2018 04:26 PM, Michael Ellerman wrote:
> We use a shared definition for struct pt_regs in uapi/asm/ptrace.h.
> That means the layout of the structure is ABI, ie. we can't change it.
>
> That would be fine if it was only used to describe the user-visible
> register state of a process, but it's also the struct we use in the
> kernel to describe the registers saved in an interrupt frame.
>
> We'd like more flexibility in the content (and possibly layout) of the
> kernel version of the struct, but currently that's not possible.
>
> So split the definition into a user-visible definition which remains
> unchanged, and a kernel internal one.
>
> At the moment they're still identical, and we check that at build
> time. That's because we have code (in ptrace etc.) that assumes that
> they are the same. We will fix that code in future patches, and then
> we can break the strict symmetry between the two structs.

Nice and awesome. But just trying to understand. What will
*regs will point to in the "struct sigcontext".


>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>   arch/powerpc/include/asm/ptrace.h      | 27 ++++++++++++++++++
>   arch/powerpc/include/uapi/asm/ptrace.h |  7 ++++-
>   arch/powerpc/kernel/ptrace.c           | 39 ++++++++++++++++++++++++++
>   3 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index 447cbd1bee99..3dd15024db93 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -26,6 +26,33 @@
>   #include <uapi/asm/ptrace.h>
>   #include <asm/asm-const.h>
>
> +#ifndef __ASSEMBLY__
> +struct pt_regs
> +{
> +	union {
> +		struct user_pt_regs user_regs;
> +		struct {
> +			unsigned long gpr[32];
> +			unsigned long nip;
> +			unsigned long msr;
> +			unsigned long orig_gpr3;
> +			unsigned long ctr;
> +			unsigned long link;
> +			unsigned long xer;
> +			unsigned long ccr;
> +#ifdef CONFIG_PPC64
> +			unsigned long softe;
> +#else
> +			unsigned long mq;
> +#endif
> +			unsigned long trap;
> +			unsigned long dar;
> +			unsigned long dsisr;
> +			unsigned long result;
> +		};
> +	};
> +};
> +#endif
>
>   #ifdef __powerpc64__
>
> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> index 55c7a131d2ab..f5f1ccc740fc 100644
> --- a/arch/powerpc/include/uapi/asm/ptrace.h
> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> @@ -29,7 +29,12 @@
>
>   #ifndef __ASSEMBLY__
>
> -struct pt_regs {
> +#ifdef __KERNEL__
> +struct user_pt_regs
> +#else
> +struct pt_regs
> +#endif
> +{
>   	unsigned long gpr[32];
>   	unsigned long nip;
>   	unsigned long msr;
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 4e372f54088f..939d7f81bbbe 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -3335,3 +3335,42 @@ void do_syscall_trace_leave(struct pt_regs *regs)
>
>   	user_enter();
>   }
> +
> +void __init pt_regs_check(void)
> +{
> +	BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
> +		     offsetof(struct user_pt_regs, gpr));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, nip) !=
> +		     offsetof(struct user_pt_regs, nip));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, msr) !=
> +		     offsetof(struct user_pt_regs, msr));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, msr) !=
> +		     offsetof(struct user_pt_regs, msr));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, orig_gpr3) !=
> +		     offsetof(struct user_pt_regs, orig_gpr3));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, ctr) !=
> +		     offsetof(struct user_pt_regs, ctr));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, link) !=
> +		     offsetof(struct user_pt_regs, link));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, xer) !=
> +		     offsetof(struct user_pt_regs, xer));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, ccr) !=
> +		     offsetof(struct user_pt_regs, ccr));
> +#ifdef __powerpc64__
> +	BUILD_BUG_ON(offsetof(struct pt_regs, softe) !=
> +		     offsetof(struct user_pt_regs, softe));
> +#else
> +	BUILD_BUG_ON(offsetof(struct pt_regs, mq) !=
> +		     offsetof(struct user_pt_regs, mq));
> +#endif
> +	BUILD_BUG_ON(offsetof(struct pt_regs, trap) !=
> +		     offsetof(struct user_pt_regs, trap));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, dar) !=
> +		     offsetof(struct user_pt_regs, dar));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
> +		     offsetof(struct user_pt_regs, dsisr));
> +	BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> +		     offsetof(struct user_pt_regs, result));
> +
> +	BUILD_BUG_ON(sizeof(struct user_pt_regs) > sizeof(struct pt_regs));
> +}