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

[Ryo Takakura <takakura@xxxxxxxxxxxxx>]

Hi Andrew,

On Tue, 30 Jun 2026 10:57:38 +0000, Andrew Cooper wrote:
>On 30/06/2026 11:43 am, Ryo Takakura wrote:
>>>> 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
>>>> @@ -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;
>>>> +        }
>>> This ought not to be necessary.
>>>
>>> The trampoline is executable code, so should have space reserved for it
>>> in .text of the livepatch.
>>>
>>> Then, you can identify it simply by references in a new section, without
>>> having to have a pointer with a sentinel value (void *)1 in (which MISRA
>>> will have a fit at).
>> I like this idea as well! I'll try this together with the earlier
>> suggestion using __attribute__((no_caller_saved_registers)).
>
>If you reserve space in the function preamble, and use
>__attribute__((no_caller_saved_registers)), then you don't need
>trampolines at all.
>
>The preamble just needs to turn into `call newfunc` when the callback
>function is attached, and then it's regular return will do the right thing.

Understood!

Sincerely,
Ryo Takakura

>~Andrew