Re: [RFC] xen/arm64: livepatch: enable attaching callbacks

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

Hello,

On Mon, Jun 29, 2026 at 11:01:28AM +0900, Ryo Takakura wrote:
> Linux ftrace allows registering callbacks which is useful
> for debugging and tracing events. On Linux, it is done by
> reserving function entry points at compile time which can
> later be patched to branch to a trampoline.
> 
> This patch implements similar callback feature, but with
> different approach using existing livepatch infrastructure.
> Instead of reserving function entry points at compile time,
> the traced function will be livepatched so that it branches
> to the trampoline.

While this is an interesting usage of the livepatch logic in new ways,
may I ask why not do as Linux and add an empty function preamble that
can be replaced at run-time with calls to hooks?

You could still re-use most of the livepatch logic for handling the
addition of the hook calls, but it would be nicer in that we won't
need to move the original function.

> The role of the trampoline(illustrated below) is to preserve
> the context while jumping to the tracer function, and return
> back to the traced function with its context restored.

Alternatively - why not use livepatch-build-tools against a build with
the added hooks to generate a proper livepatch?  This looks a bit
fragile to me (see the question from Andrew about fixing up
instruction pointer relative references).

On x86 at least we would also need to adjust the bug frames and
exception table contents, and the contents of the symbol table to
account for the function being moved.

IOW: it looks like overall this is a lot more work than possibly
reserving a function preamble to add hook calls?

> trampoline:
>     Save regs
>     Call tracer function
>     Restore regs
>     old_addr
>     return old_addr + 4
> 
> One can request the feature by setting @trampoline_buf to 1
> which will allocate a buffer for trampoline.
> 
> Signed-off-by: Ryo Takakura <takakura@xxxxxxxxxxxxx>
> ---
> 
> Hi!
> 
> For the future, I'm thinking of linux-like extensions
> which help tracing and debugging by passing:
> - saved registers
> - caller information
> - private data
> - and so on ...
> 
> I would appreciate any advice or suggestion.
> Thanks!
> 
> Example payload file:
> 
> #include <xen/lib.h>
> #include <xen/livepatch.h>
> 
> static void my_tracer(void)
> {
>     printk("livepatch: do_domctl was called\n");
> }
> 
> static struct livepatch_func funcs[]
>     __attribute__((section(".livepatch.funcs"))) =
> {
>     {
>         .name = "do_domctl",
>         .old_size = 4572,
>         .new_addr = my_tracer,
>         .new_size = 32,
>         .trampoline_buf = (void *)1,
>         .version = LIVEPATCH_PAYLOAD_VERSION,
>     }
> };
> 
> Sample output:
> 
> $ tools/misc/xen-livepatch list
>  ID                                     | status     | metadata
> ----------------------------------------+------------+---------------
> trace_do_domctl                         | APPLIED    |
> $ xl vcpu-list Domain-0
> Name                                ID  VCPU   CPU State   Time(s) Affinity 
> (Hard / Soft)
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> (XEN) livepatch: do_domctl was called
> Domain-0                             0     0    1   -b-      67.7  all / all
> Domain-0                             0     1    3   -b-     457.2  all / all
> Domain-0                             0     2    2   -b-      42.4  all / all
> Domain-0                             0     3    0   r--      32.4  all / all
> 
> Sincerely,
> Ryo Takakura
> 
> ---
>  xen/arch/arm/arm64/livepatch.c      | 104 +++++++++++++++++++++++++++-
>  xen/common/livepatch.c              |  40 +++++++++--
>  xen/include/public/sysctl.h         |   3 +-
>  xen/include/xen/livepatch.h         |  13 +++-
>  xen/include/xen/livepatch_payload.h |   2 +
>  5 files changed, 150 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
> index e135bd5bf9..b7c9aba94e 100644
> --- a/xen/arch/arm/arm64/livepatch.c
> +++ b/xen/arch/arm/arm64/livepatch.c
> @@ -15,6 +15,29 @@
>  #include <asm/insn.h>
>  #include <asm/livepatch.h>
>  
> +
> +#define AARCH64_REG_SP 31
> +
> +static uint32_t aarch64_insn_gen_stp_pre(unsigned int rt,
> +                                         unsigned int rt2)
> +{
> +    return 0xa9800000 |
> +           (((-16 / 8) & 0x7f) << 15) |
> +           (rt2 << 10) |
> +           (AARCH64_REG_SP << 5) |
> +           rt;
> +}
> +
> +static uint32_t aarch64_insn_gen_ldp_post(unsigned int rt,
> +                                          unsigned int rt2)
> +{
> +    return 0xa8c00000 |
> +           (((16 / 8) & 0x7f) << 15) |
> +           (rt2 << 10) |
> +           (AARCH64_REG_SP << 5) |
> +           rt;
> +}
> +
>  void arch_livepatch_apply(const struct livepatch_func *func,
>                            struct livepatch_fstate *state)
>  {
> @@ -34,12 +57,87 @@ void arch_livepatch_apply(const struct livepatch_func 
> *func,
>      /* Save old ones. */
>      memcpy(state->insn_buffer, func->old_addr, len);
>  
> -    if ( func->new_addr )
> +    if ( !func->new_addr )
> +    {
> +        insn = aarch64_insn_gen_nop();
> +    }
> +    else if ( func->trampoline_buf )
> +    {
> +        int rc;
> +        uint32_t *trampoline = func->trampoline_buf;
> +        uint32_t *tp = trampoline;
> +        void *orig_cont_addr = (void *)func->old_addr + len;
> +        unsigned int trampoline_code_size = len + 12 * ARCH_PATCH_INSN_SIZE;
> +        unsigned long trampoline_start = (unsigned long)trampoline & 
> PAGE_MASK;
> +        unsigned long trampoline_end =
> +            PAGE_ALIGN((unsigned long)trampoline + trampoline_code_size);
> +
> +        /*
> +         * Make the payload text area writeable while generating
> +         * the trampoline instructions.
> +         */
> +        rc = modify_xen_mappings(trampoline_start, trampoline_end,
> +                                 PAGE_HYPERVISOR);
> +        if ( rc )
> +        {
> +            printk(XENLOG_ERR LIVEPATCH
> +                   "Failed to make trampoline writable: %d\n", rc);
> +            return;
> +        }
> +
> +        /* Save state before calling the tracer. */
> +        *tp++ = aarch64_insn_gen_stp_pre(0, 1);
> +        *tp++ = aarch64_insn_gen_stp_pre(2, 3);
> +        *tp++ = aarch64_insn_gen_stp_pre(4, 5);
> +        *tp++ = aarch64_insn_gen_stp_pre(6, 7);
> +        *tp++ = aarch64_insn_gen_stp_pre(29, 30);
> +
> +        /* Call user's tracing function. */
> +        insn = aarch64_insn_gen_branch_imm(
> +            (unsigned long)tp,
> +            (unsigned long)func->new_addr,
> +            AARCH64_INSN_BRANCH_LINK);
> +        *tp++ = insn;
> +
> +        /* Restore state before continuing original function. */
> +        *tp++ = aarch64_insn_gen_ldp_post(29, 30);
> +        *tp++ = aarch64_insn_gen_ldp_post(6, 7);
> +        *tp++ = aarch64_insn_gen_ldp_post(4, 5);
> +        *tp++ = aarch64_insn_gen_ldp_post(2, 3);
> +        *tp++ = aarch64_insn_gen_ldp_post(0, 1);
> +
> +        /* Original instruction. */
> +        memcpy(tp, state->insn_buffer, len);
> +        tp += len / ARCH_PATCH_INSN_SIZE;
> +
> +        /* Branch back to original function. */
> +        insn = aarch64_insn_gen_branch_imm(
> +            (unsigned long)tp,
> +            (unsigned long)orig_cont_addr,
> +            AARCH64_INSN_BRANCH_NOLINK);
> +        *tp++ = insn;
> +
> +        clean_and_invalidate_dcache_va_range(trampoline, 
> trampoline_code_size);
> +
> +        rc = modify_xen_mappings(trampoline_start, trampoline_end,
> +                                 PAGE_HYPERVISOR_RX);
> +        if ( rc )
> +        {
> +            printk(XENLOG_ERR LIVEPATCH
> +                   "Failed to restore trampoline RX mapping: %d\n", rc);
> +            return;
> +        }
> +
> +        /* Branch from original function to trampoline. */
> +        insn = aarch64_insn_gen_branch_imm(
> +            (unsigned long)func->old_addr,
> +            (unsigned long)func->trampoline_buf,
> +            AARCH64_INSN_BRANCH_NOLINK);
> +    }
> +    else if ( func->new_addr )
>          insn = aarch64_insn_gen_branch_imm((unsigned long)func->old_addr,
>                                             (unsigned long)func->new_addr,
>                                             AARCH64_INSN_BRANCH_NOLINK);
> -    else
> -        insn = aarch64_insn_gen_nop();

If we want to go this route, and use livepatching for this purpose, we
need to branch the use-cases in common code, and have arches provide
both a replacement and a preface addition hooks IMO.

>  
>      /* Verified in livepatch_verify_distance. */
>      ASSERT(insn != AARCH64_BREAK_FAULT);
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 7515a040ad..8863ad5ca3 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -280,10 +280,30 @@ static int move_payload(struct payload *payload, struct 
> livepatch_elf *elf)
>  {
>      void *text_buf, *ro_buf, *rw_buf;
>      unsigned int i, rw_buf_sec, rw_buf_cnt = 0;
> -    size_t size = 0;
> +    const struct livepatch_elf_sec *sec;
> +    const struct livepatch_func *funcs;
> +    unsigned int nfuncs, trampolines_needed = 0;
> +    size_t size = 0, trampoline_size = 0;
>      unsigned int *offset;
>      int rc = 0;
>  
> +    sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_FUNC);
> +    if ( sec )
> +    {
> +        funcs = sec->addr;
> +        nfuncs = sec->sec->sh_size / sizeof(*funcs);
> +
> +        for ( i = 0; i < nfuncs; ++i )
> +            if ( funcs[i].trampoline_buf == (void *)1 )
> +                trampolines_needed++;
> +
> +        if ( trampolines_needed )
> +        {
> +            payload->n_trampolines = trampolines_needed;
> +            trampoline_size = trampolines_needed * LIVEPATCH_TRAMPOLINE_SIZE;
> +        }
> +    }
> +
>      offset = xmalloc_array(unsigned int, elf->hdr->e_shnum);
>      if ( !offset )
>          return -ENOMEM;
> @@ -323,8 +343,8 @@ static int move_payload(struct payload *payload, struct 
> livepatch_elf *elf)
>       * them on separate pages. The last one will by default fall on its
>       * own page.
>       */
> -    size = PAGE_ALIGN(payload->text_size) + PAGE_ALIGN(payload->rw_size) +
> -                      payload->ro_size;
> +    size = PAGE_ALIGN(payload->text_size + trampoline_size) +
> +           PAGE_ALIGN(payload->rw_size) + payload->ro_size;
>  
>      size = PFN_UP(size); /* Nr of pages. */
>      text_buf = vmalloc_xen(size * PAGE_SIZE);
> @@ -335,9 +355,12 @@ static int move_payload(struct payload *payload, struct 
> livepatch_elf *elf)
>          rc = -ENOMEM;
>          goto out;
>      }
> -    rw_buf = text_buf + PAGE_ALIGN(payload->text_size);
> +    rw_buf = text_buf + PAGE_ALIGN(payload->text_size + trampoline_size);
>      ro_buf = rw_buf + PAGE_ALIGN(payload->rw_size);
>  
> +    if ( trampoline_size )
> +        payload->trampoline_addr = text_buf + payload->text_size;
> +
>      payload->pages = size;
>      payload->text_addr = text_buf;
>      payload->rw_addr = rw_buf;
> @@ -690,7 +713,7 @@ static int prepare_payload(struct payload *payload,
>  {
>      const struct livepatch_elf_sec *sec;
>      const struct payload *data;
> -    unsigned int i;
> +    unsigned int i, trampoline_idx = 0;
>      struct livepatch_func *funcs;
>      struct livepatch_func *f;
>      struct virtual_region *region;
> @@ -737,6 +760,13 @@ static int prepare_payload(struct payload *payload,
>              if ( rc )
>                  return rc;
>  
> +            if ( f->trampoline_buf == (void *)1 )
> +            {
> +                f->trampoline_buf = (char *)payload->trampoline_addr +

You don't need to cast to char *, the type of trampoline_addr is void
*, and we use the GNU extension to allow void pointer arithmetic by
treating the size of a void or of a function as 1.

> +                                    trampoline_idx * 
> LIVEPATCH_TRAMPOLINE_SIZE;
> +                trampoline_idx++;
> +            }
> +
>              rc = livepatch_verify_distance(f);
>              if ( rc )
>                  return rc;
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index c7cd9b4eb0..e79615d7c9 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1010,10 +1010,11 @@ struct livepatch_func {
>      const char *name;       /* Name of function to be patched. */
>      void *new_addr;
>      void *old_addr;
> +    void *trampoline_buf;   /* Trampoline buffer when set to (void *)1. */
>      uint32_t new_size;
>      uint32_t old_size;
>      uint8_t version;        /* MUST be LIVEPATCH_PAYLOAD_VERSION. */
> -    uint8_t _pad[39];
> +    uint8_t _pad[31];

New fields should be preferably added at the tail of the structure,
and the change here needs to be propagated into livepatch-build-tools
livepatch_patch_func structure.  See:

https://xenbits.xen.org/gitweb/?p=livepatch-build-tools.git;a=blob;f=common.h;h=7f3a82ffdb29d2d1d117c1ccb20cc328bdb0529a;hb=HEAD#l135

This is sadly all very fragile.

>      livepatch_expectation_t expect;
>  };
>  typedef struct livepatch_func livepatch_func_t;
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index 45c8924f34..7a81763cf2 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -48,6 +48,8 @@ struct xen_sysctl_livepatch_op;
>  #define ELF_LIVEPATCH_POSTREVERT_HOOK ".livepatch.hooks.postrevert"
>  /* Arbitrary limit for payload size and .bss section size. */
>  #define LIVEPATCH_MAX_SIZE     MB(2)
> +/* Size of a trampoline used for function tracing */
> +#define LIVEPATCH_TRAMPOLINE_SIZE 128
>  
>  struct livepatch_symbol {
>      const char *name;
> @@ -109,13 +111,18 @@ unsigned int livepatch_insn_len(const struct 
> livepatch_func *func,
>  
>  static inline int livepatch_verify_distance(const struct livepatch_func 
> *func)
>  {
> +    const void *target;
>      long offset;
>      long range = ARCH_LIVEPATCH_RANGE;
>  
> -    if ( !func->new_addr ) /* Ignore NOPs. */
> -        return 0;
> +    if ( func->trampoline_buf )
> +     target = func->trampoline_buf;
> +    else if ( func->new_addr )
> +     target = func->new_addr;
> +    else
> +     return 0; /* Ignore NOPs. */

FWIW, indentation is wrong here, you are adding hard tabs.

Thanks, Roger.