Re: [PATCH 6/8] xen/livepatch: Support new altcall scheme

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

On Wed, Apr 23, 2025 at 02:02:35AM +0100, Andrew Cooper wrote:
> The new altcall scheme uses an .alt_call_sites section.  Wire this up in very
> much the same way as the .altinstructions section, although there is less
> sanity checking necessary.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

One nit/comment below.

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> ---
>  xen/arch/x86/alternative.c         |  6 ++++
>  xen/common/livepatch.c             | 58 ++++++++++++++++++++++++++++++
>  xen/include/xen/alternative-call.h |  8 +++--
>  3 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
> index f6594e21a14c..22af224f08f7 100644
> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -479,6 +479,12 @@ int apply_alternatives(struct alt_instr *start, struct 
> alt_instr *end)
>  {
>      return _apply_alternatives(start, end, true);
>  }
> +
> +int livepatch_apply_alt_calls(const struct alt_call *start,
> +                              const struct alt_call *end)
> +{
> +    return apply_alt_calls(start, end);
> +}
>  #endif
>  
>  #define ALT_INSNS (1U << 0)
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 6ce77bf021b7..be9b7e367553 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -905,6 +905,64 @@ static int prepare_payload(struct payload *payload,
>  #endif
>      }
>  
> +    sec = livepatch_elf_sec_by_name(elf, ".alt_call_sites");
> +    if ( sec )
> +    {
> +#ifdef CONFIG_ALTERNATIVE_CALL
> +        const struct alt_call *a, *start, *end;
> +
> +        if ( !section_ok(elf, sec, sizeof(*a)) )
> +            return -EINVAL;
> +
> +        /* Tolerate an empty .alt_call_sites section... */
> +        if ( sec->sec->sh_size == 0 )

You could possibly move this check to the outer `if` condition, and
avoid the alt_call_done label?

As even in the !CONFIG_ALTERNATIVE_CALL case skipping an empty section
would be OK.

Thanks, Roger.