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@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 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 )
+            goto alt_call_done;
+
+        /* ... but otherwise, there needs to be something to alter... */
+        if ( payload->text_size == 0 )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s Alternative calls provided, but no .text\n",
+                   elf->name);
+            return -EINVAL;
+        }
+
+        start = sec->addr;
+        end = sec->addr + sec->sec->sh_size;
+
+        for ( a = start; a < end; a++ )
+        {
+            const void *orig = ALT_CALL_PTR(a);
+            size_t len = ALT_CALL_LEN(a);
+
+            /* orig must be fully within .text. */
+            if ( orig       < payload->text_addr ||
+                 len        > payload->text_size ||
+                 orig + len > payload->text_addr + payload->text_size )
+            {
+                printk(XENLOG_ERR LIVEPATCH
+                       "%s: Alternative call %p+%#zx outside payload text %p+%#zx\n",
+                       elf->name, orig, len,
+                       payload->text_addr, payload->text_size);
+                return -EINVAL;
+            }
+        }
+
+        rc = livepatch_apply_alt_calls(start, end);
+        if ( rc )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s: Applying alternative calls failed: %d\n",
+                   elf->name, rc);
+            return rc;
+        }
+
+    alt_call_done:;
+#else /* CONFIG_ALTERNATIVE_CALL */
+        printk(XENLOG_ERR LIVEPATCH "%s: Alternative calls not supported\n",
+               elf->name);
+        return -EOPNOTSUPP;
+#endif /* !CONFIG_ALTERNATIVE_CALL */
+    }
+
     sec = livepatch_elf_sec_by_name(elf, ".ex_table");
     if ( sec )
     {
diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
index 3c855bfa44f5..767c2149bce7 100644
--- a/xen/include/xen/alternative-call.h
+++ b/xen/include/xen/alternative-call.h
@@ -17,8 +17,8 @@
  *   generation requirements are to emit a function pointer call at build
  *   time, and stash enough metadata to simplify the call at boot once the
  *   implementation has been resolved.
- * - Implement boot_apply_alt_calls() to convert the function pointer calls
- *   into direct calls on boot.
+ * - Implement {boot,livepatch}_apply_alt_calls() to convert the function
+ *   pointer calls into direct calls on boot/livepatch.
  * - Select ALTERNATIVE_CALL in Kconfig.
  *
  * To use:
@@ -65,6 +65,10 @@
  */
 void boot_apply_alt_calls(void);
 
+/* As per boot_apply_alt_calls() but for a livepatch. */
+int livepatch_apply_alt_calls(const struct alt_call *start,
+                              const struct alt_call *end);
+
 #else /* CONFIG_ALTERNATIVE_CALL */
 
 #define alternative_call(func, args...)  (func)(args)
-- 
2.39.5

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@citrix.com>

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

One nit/comment below.

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  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.

On 23/04/2025 12:16 pm, Roger Pau Monné wrote:
> 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@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>
> One nit/comment below.
>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
>> ---
>>  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.

.altinstructions is like this.  It was put in as part of e74360e4ba4a, I
believe because it was decided that an empty section wasn't wanted.

We can revisit the decision, but the logic should be consistent.

~Andrew

On 23.04.2025 03:02, 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@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>