On Thu, Jul 25, 2024 at 05:00:18PM +0100, Dave Martin wrote:
> Hi,
>
> On Fri, May 03, 2024 at 02:01:36PM +0100, Joey Gouly wrote:
> > Add PKEY support to signals, by saving and restoring POR_EL0 from the stackframe.
> >
> > Signed-off-by: Joey Gouly <joey.gouly@xxxxxxx>
> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > Cc: Will Deacon <will@xxxxxxxxxx>
> > Reviewed-by: Mark Brown <broonie@xxxxxxxxxx>
> > Acked-by: Szabolcs Nagy <szabolcs.nagy@xxxxxxx>
> > ---
> > arch/arm64/include/uapi/asm/sigcontext.h | 7 ++++
> > arch/arm64/kernel/signal.c | 52 ++++++++++++++++++++++++
> > 2 files changed, 59 insertions(+)
> >
> > diff --git a/arch/arm64/include/uapi/asm/sigcontext.h b/arch/arm64/include/uapi/asm/sigcontext.h
> > index 8a45b7a411e0..e4cba8a6c9a2 100644
> > --- a/arch/arm64/include/uapi/asm/sigcontext.h
> > +++ b/arch/arm64/include/uapi/asm/sigcontext.h
>
> [...]
>
> > @@ -980,6 +1013,13 @@ static int setup_sigframe_layout(struct rt_sigframe_user_layout *user,
> > return err;
> > }
> >
> > + if (system_supports_poe()) {
> > + err = sigframe_alloc(user, &user->poe_offset,
> > + sizeof(struct poe_context));
> > + if (err)
> > + return err;
> > + }
> > +
> > return sigframe_alloc_end(user);
> > }
> >
> > @@ -1020,6 +1060,15 @@ static int setup_sigframe(struct rt_sigframe_user_layout *user,
> > __put_user_error(current->thread.fault_code, &esr_ctx->esr, err);
> > }
> >
> > + if (system_supports_poe() && err == 0 && user->poe_offset) {
> > + struct poe_context __user *poe_ctx =
> > + apply_user_offset(user, user->poe_offset);
> > +
> > + __put_user_error(POE_MAGIC, &poe_ctx->head.magic, err);
> > + __put_user_error(sizeof(*poe_ctx), &poe_ctx->head.size, err);
> > + __put_user_error(read_sysreg_s(SYS_POR_EL0), &poe_ctx->por_el0, err);
> > + }
> > +
>
> Does the AArch64 procedure call standard say anything about whether
> POR_EL0 is caller-saved?
I asked about this, and it doesn't say anything and they don't plan on it,
since it's very application specific.
>
> <bikeshed>
>
> In theory we could skip saving this register if it is already
> POR_EL0_INIT (which it often will be), and if the signal handler is not
> supposed to modify and leave the modified value in the register when
> returning.
>
> The complexity of the additional check my be a bit pointless though,
> and the the handler might theoretically want to change the interrupted
> code's POR_EL0 explicitly, which would be complicated if POE_MAGIC is
> sometimes there and sometimes not.
>
> </bikeshed>
>
I think trying to skip/optimise something here would be more effort than any
possible benefits!
Thanks,
Joey
