[DTrace-devel] [PATCH 03/12] cg: reserve %r7 and %r8 in trampolines

[Eugene Loh]

I do not understand this patch.

Why isn't the 04/12 patch squashed back into this one?

Since when are we using the immature reg-management mechanism for the 
trampoline cg?

Why do %r7 and %r8 in the trampoline count against any DIF reg limit?  
Isn't their use outside the realm of DIF registers?

We use %r9 in the trampoline;  why doesn't this patch also reserve it?  
Same with %r6.

Those are all basically rhetorical questions.  The real point is that 
this patch is intended to support the -xiregs option but the right thing 
to do is to kill/deprecate that option.  The only legal value is 
-xiregs=goldilocks.  Okay, cultural reference, but there is only one 
value that works.  Because of BPF (and how we've wired things), iiuc 
there is only one legal value of -xiregs.  So we should simply 
back-support this option by telling users who try to change it that they 
cannot.

Or, what am I missing?

On 1/5/24 00:26, Kris Van Hees via DTrace-devel wrote:
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_cg.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 30b1da16..d9d56f0a 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1,6 +1,6 @@
>   /*
>    * Oracle Linux DTrace.
> - * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
>    * Licensed under the Universal Permissive License v 1.0 as shown at
>    * http://oss.oracle.com/licenses/upl.
>    */
> @@ -138,6 +138,7 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>   {
>   	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
> +	dt_regset_t	*drp = pcb->pcb_regs;
>   	dt_ident_t	*aggs = dt_dlib_get_map(dtp, "aggs");
>   	dt_ident_t	*mem = dt_dlib_get_map(dtp, "mem");
>   	dt_ident_t	*state = dt_dlib_get_map(dtp, "state");
> @@ -151,6 +152,10 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
>   	assert(prid != NULL);
>   	assert(ro_off != NULL);
>   
> +	/* Reserve %r7 and %r8. */
> +	dt_regset_xalloc(drp, BPF_REG_7);
> +	dt_regset_xalloc(drp, BPF_REG_8);
> +
>   	/*
>   	 * On input, %r1 is the BPF context.
>   	 *
> @@ -718,6 +723,7 @@ void
>   dt_cg_tramp_return(dt_pcb_t *pcb)
>   {
>   	dt_irlist_t	*dlp = &pcb->pcb_ir;
> +	dt_regset_t	*drp = pcb->pcb_regs;
>   
>   	/*
>   	 * exit:
> @@ -728,6 +734,10 @@ dt_cg_tramp_return(dt_pcb_t *pcb)
>   	emitl(dlp, pcb->pcb_exitlbl,
>   		   BPF_MOV_IMM(BPF_REG_0, 0));
>   	emit(dlp,  BPF_RETURN());
> +
> +	/* Free %r7 and %r8. */
> +	dt_regset_free(drp, BPF_REG_7);
> +	dt_regset_free(drp, BPF_REG_8);
>   }
>   
>   void