From: Andrew Cooper <andrew.cooper3@citrix.com>
The alternative call infrastructure is x86-only for now, but the common iommu
code has a variant and more common code wants to use the infrastructure.
Introduce CONFIG_ALTERNATIVE_CALL and a conditional implemetnation so common
code can use the optimisation when available, without requiring all
architectures to implement no-op stubs.
Write some documentation, which was thus far entirely absent, covering the
requirements for an architecture to implement this optimsiation, and how to
use the infrastructure in general code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/x86/Kconfig | 1 +
xen/common/Kconfig | 3 ++
xen/include/xen/alternative-call.h | 65 ++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+)
create mode 100644 xen/include/xen/alternative-call.h
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 9b164db641..c91cdd83dc 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -9,6 +9,7 @@ config X86
select ARCH_SUPPORTS_INT128
select CORE_PARKING
select HAS_ALTERNATIVE
+ select ALTERNATIVE_CALL
select HAS_COMPAT
select HAS_CPUFREQ
select HAS_EHCI
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0ddd18e11a..1594ce4e73 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -25,6 +25,9 @@ config GRANT_TABLE
config HAS_ALTERNATIVE
bool
+config ALTERNATIVE_CALL
+ bool
+
config HAS_COMPAT
bool
diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
new file mode 100644
index 0000000000..11d1c26068
--- /dev/null
+++ b/xen/include/xen/alternative-call.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef XEN_ALTERNATIVE_CALL
+#define XEN_ALTERNATIVE_CALL
+
+/*
+ * Some subsystems in Xen may have multiple implementions, which can be
+ * resolved to a single implementation at boot time. By default, this will
+ * result in the use of function pointers.
+ *
+ * Some architectures may have mechanisms for dynamically modifying .text.
+ * Using this mechnaism, function pointers can be converted to direct calls
+ * which are typically more efficient at runtime.
+ *
+ * For architectures to support:
+ *
+ * - Implement alternative_{,v}call() in asm/alternative.h. Code 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.
+ * - Select ALTERNATIVE_CALL in Kconfig.
+ *
+ * To use:
+ *
+ * Consider the following simplified example.
+ *
+ * 1) struct foo_ops __alt_call_maybe_initdata ops;
+ *
+ * 2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
+ * struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
+ *
+ * void foo_init(void)
+ * {
+ * ...
+ * if ( use_impl_a )
+ * ops = *foo_a_ops;
+ * else if ( use_impl_b )
+ * ops = *foo_b_ops;
+ * ...
+ * }
+ *
+ * 3) alternative_call(ops.bar, ...);
+ *
+ * There needs to a single ops object (1) which will eventually contain the
+ * function pointers. This should be populated in foo's init() function (2)
+ * by one of the available implementations. To call functions, use
+ * alternative_{,v}call() referencing the main ops object (3).
+ */
+
+#ifdef CONFIG_ALTERNATIVE_CALL
+
+#include <asm/alternative.h>
+
+#define __alt_call_maybe_initdata __initdata
+#define __alt_call_maybe_initconst __initconst
+
+#else
+
+#define alternative_call(func, args...) (func)(args)
+#define alternative_vcall(func, args...) (func)(args)
+
+#define __alt_call_maybe_initdata
+#define __alt_call_maybe_initconst
+
+#endif /* !CONFIG_ALTERNATIVE_CALL */
+#endif /* XEN_ALTERNATIVE_CALL */
--
2.20.1
On 12/07/2021 21:32, Daniel P. Smith wrote:
> diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
> new file mode 100644
> index 0000000000..11d1c26068
> --- /dev/null
> +++ b/xen/include/xen/alternative-call.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef XEN_ALTERNATIVE_CALL
> +#define XEN_ALTERNATIVE_CALL
> +
> +/*
> + * Some subsystems in Xen may have multiple implementions, which can be
> + * resolved to a single implementation at boot time. By default, this will
> + * result in the use of function pointers.
> + *
> + * Some architectures may have mechanisms for dynamically modifying .text.
> + * Using this mechnaism, function pointers can be converted to direct calls
> + * which are typically more efficient at runtime.
> + *
> + * For architectures to support:
> + *
> + * - Implement alternative_{,v}call() in asm/alternative.h. Code 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.
> + * - Select ALTERNATIVE_CALL in Kconfig.
> + *
> + * To use:
> + *
> + * Consider the following simplified example.
> + *
> + * 1) struct foo_ops __alt_call_maybe_initdata ops;
> + *
> + * 2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
> + * struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
It occurs to me after reviewing patch 2 that these want to be const
struct foo_ops __initconst ..., and there is no need for
__alt_call_maybe_initconst at all.
The only thing wanting a conditional annotation like this is the single
ops object, and it needs to be initdata (or hopefully ro_after_init in
the future).
~Andrew
On 13.07.2021 01:48, Andrew Cooper wrote:
> On 12/07/2021 21:32, Daniel P. Smith wrote:
>> diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
>> new file mode 100644
>> index 0000000000..11d1c26068
>> --- /dev/null
>> +++ b/xen/include/xen/alternative-call.h
>> @@ -0,0 +1,65 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef XEN_ALTERNATIVE_CALL
>> +#define XEN_ALTERNATIVE_CALL
>> +
>> +/*
>> + * Some subsystems in Xen may have multiple implementions, which can be
>> + * resolved to a single implementation at boot time. By default, this will
>> + * result in the use of function pointers.
>> + *
>> + * Some architectures may have mechanisms for dynamically modifying .text.
>> + * Using this mechnaism, function pointers can be converted to direct calls
>> + * which are typically more efficient at runtime.
>> + *
>> + * For architectures to support:
>> + *
>> + * - Implement alternative_{,v}call() in asm/alternative.h. Code 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.
>> + * - Select ALTERNATIVE_CALL in Kconfig.
>> + *
>> + * To use:
>> + *
>> + * Consider the following simplified example.
>> + *
>> + * 1) struct foo_ops __alt_call_maybe_initdata ops;
>> + *
>> + * 2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
>> + * struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
>
> It occurs to me after reviewing patch 2 that these want to be const
> struct foo_ops __initconst ...,
__initconstrel then, I suppose.
> and there is no need for
> __alt_call_maybe_initconst at all.
>
> The only thing wanting a conditional annotation like this is the single
> ops object, and it needs to be initdata (or hopefully ro_after_init in
> the future).
ro_after_init and initdata can't be alternatives of one another; ops
(until be gain ro_after_init) need to remain in .bss (or .data).
Jan
On 13/07/2021 07:28, Jan Beulich wrote:
> On 13.07.2021 01:48, Andrew Cooper wrote:
>> On 12/07/2021 21:32, Daniel P. Smith wrote:
>>> diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
>>> new file mode 100644
>>> index 0000000000..11d1c26068
>>> --- /dev/null
>>> +++ b/xen/include/xen/alternative-call.h
>>> @@ -0,0 +1,65 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef XEN_ALTERNATIVE_CALL
>>> +#define XEN_ALTERNATIVE_CALL
>>> +
>>> +/*
>>> + * Some subsystems in Xen may have multiple implementions, which can be
>>> + * resolved to a single implementation at boot time. By default, this will
>>> + * result in the use of function pointers.
>>> + *
>>> + * Some architectures may have mechanisms for dynamically modifying .text.
>>> + * Using this mechnaism, function pointers can be converted to direct calls
>>> + * which are typically more efficient at runtime.
>>> + *
>>> + * For architectures to support:
>>> + *
>>> + * - Implement alternative_{,v}call() in asm/alternative.h. Code 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.
>>> + * - Select ALTERNATIVE_CALL in Kconfig.
>>> + *
>>> + * To use:
>>> + *
>>> + * Consider the following simplified example.
>>> + *
>>> + * 1) struct foo_ops __alt_call_maybe_initdata ops;
>>> + *
>>> + * 2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
>>> + * struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
>> It occurs to me after reviewing patch 2 that these want to be const
>> struct foo_ops __initconst ...,
> __initconstrel then, I suppose.
>
>> and there is no need for
>> __alt_call_maybe_initconst at all.
>>
>> The only thing wanting a conditional annotation like this is the single
>> ops object, and it needs to be initdata (or hopefully ro_after_init in
>> the future).
> ro_after_init and initdata can't be alternatives of one another; ops
> (until be gain ro_after_init) need to remain in .bss (or .data).
Once alternatives have run, the ops structure is no longer referenced by
anything at runtime, and can be reclaimed.
All the x86 examples are weird because we've either got extra data
fields which are referenced at runtime, or we've not accelerated all
function pointers.
In neither case does modifying an accelerated function pointer after the
fact do what the programmer expected, and conditionally initdata makes
this far more obvious.
~Andrew
On 13.07.2021 10:36, Andrew Cooper wrote:
> On 13/07/2021 07:28, Jan Beulich wrote:
>> On 13.07.2021 01:48, Andrew Cooper wrote:
>>> On 12/07/2021 21:32, Daniel P. Smith wrote:
>>>> diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
>>>> new file mode 100644
>>>> index 0000000000..11d1c26068
>>>> --- /dev/null
>>>> +++ b/xen/include/xen/alternative-call.h
>>>> @@ -0,0 +1,65 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +#ifndef XEN_ALTERNATIVE_CALL
>>>> +#define XEN_ALTERNATIVE_CALL
>>>> +
>>>> +/*
>>>> + * Some subsystems in Xen may have multiple implementions, which can be
>>>> + * resolved to a single implementation at boot time. By default, this will
>>>> + * result in the use of function pointers.
>>>> + *
>>>> + * Some architectures may have mechanisms for dynamically modifying .text.
>>>> + * Using this mechnaism, function pointers can be converted to direct calls
>>>> + * which are typically more efficient at runtime.
>>>> + *
>>>> + * For architectures to support:
>>>> + *
>>>> + * - Implement alternative_{,v}call() in asm/alternative.h. Code 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.
>>>> + * - Select ALTERNATIVE_CALL in Kconfig.
>>>> + *
>>>> + * To use:
>>>> + *
>>>> + * Consider the following simplified example.
>>>> + *
>>>> + * 1) struct foo_ops __alt_call_maybe_initdata ops;
>>>> + *
>>>> + * 2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
>>>> + * struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
>>> It occurs to me after reviewing patch 2 that these want to be const
>>> struct foo_ops __initconst ...,
>> __initconstrel then, I suppose.
>>
>>> and there is no need for
>>> __alt_call_maybe_initconst at all.
>>>
>>> The only thing wanting a conditional annotation like this is the single
>>> ops object, and it needs to be initdata (or hopefully ro_after_init in
>>> the future).
>> ro_after_init and initdata can't be alternatives of one another; ops
>> (until be gain ro_after_init) need to remain in .bss (or .data).
>
> Once alternatives have run, the ops structure is no longer referenced by
> anything at runtime, and can be reclaimed.
Oh, right - if all pointers have been consumed for patching, initdata
is of course fine.
Jan
> All the x86 examples are weird because we've either got extra data
> fields which are referenced at runtime, or we've not accelerated all
> function pointers.
>
> In neither case does modifying an accelerated function pointer after the
> fact do what the programmer expected, and conditionally initdata makes
> this far more obvious.
>
> ~Andrew
>
On 12.07.2021 22:32, Daniel P. Smith wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -9,6 +9,7 @@ config X86
> select ARCH_SUPPORTS_INT128
> select CORE_PARKING
> select HAS_ALTERNATIVE
> + select ALTERNATIVE_CALL
> select HAS_COMPAT
> select HAS_CPUFREQ
> select HAS_EHCI
Please respect the (at least largely) alphabetical ordering here and ...
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -25,6 +25,9 @@ config GRANT_TABLE
> config HAS_ALTERNATIVE
> bool
>
> +config ALTERNATIVE_CALL
> + bool
> +
> config HAS_COMPAT
> bool
... maybe also here.
> --- /dev/null
> +++ b/xen/include/xen/alternative-call.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef XEN_ALTERNATIVE_CALL
> +#define XEN_ALTERNATIVE_CALL
> +
> +/*
> + * Some subsystems in Xen may have multiple implementions, which can be
> + * resolved to a single implementation at boot time. By default, this will
> + * result in the use of function pointers.
> + *
> + * Some architectures may have mechanisms for dynamically modifying .text.
> + * Using this mechnaism, function pointers can be converted to direct calls
> + * which are typically more efficient at runtime.
> + *
> + * For architectures to support:
> + *
> + * - Implement alternative_{,v}call() in asm/alternative.h. Code 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.
> + * - Select ALTERNATIVE_CALL in Kconfig.
> + *
> + * To use:
> + *
> + * Consider the following simplified example.
> + *
> + * 1) struct foo_ops __alt_call_maybe_initdata ops;
> + *
> + * 2) struct foo_ops __alt_call_maybe_initconst foo_a_ops = { ... };
> + * struct foo_ops __alt_call_maybe_initconst foo_b_ops = { ... };
> + *
> + * void foo_init(void)
> + * {
> + * ...
> + * if ( use_impl_a )
> + * ops = *foo_a_ops;
> + * else if ( use_impl_b )
> + * ops = *foo_b_ops;
> + * ...
> + * }
> + *
> + * 3) alternative_call(ops.bar, ...);
> + *
> + * There needs to a single ops object (1) which will eventually contain the
> + * function pointers. This should be populated in foo's init() function (2)
> + * by one of the available implementations. To call functions, use
> + * alternative_{,v}call() referencing the main ops object (3).
> + */
May be worth adding a sentence about the section annotations, the
more that (as you did point out in an earlier reply) there are
pre-existing cases differing from the general goal here?
> +#ifdef CONFIG_ALTERNATIVE_CALL
> +
> +#include <asm/alternative.h>
> +
> +#define __alt_call_maybe_initdata __initdata
> +#define __alt_call_maybe_initconst __initconst
As indicated elsewhere, I think this wants to be __initconstrel,
as the function pointers to place in the structures will
definitely involve relocations. Otoh, given your initial reply,
__alt_call_maybe_initdata may be all that's actually needed.
> +#else
> +
> +#define alternative_call(func, args...) (func)(args)
> +#define alternative_vcall(func, args...) (func)(args)
> +
> +#define __alt_call_maybe_initdata
> +#define __alt_call_maybe_initconst
> +
> +#endif /* !CONFIG_ALTERNATIVE_CALL */
> +#endif /* XEN_ALTERNATIVE_CALL */
I'm surprised you have no "Local variables:" footer comment here.
Not that I need it for anything, but I thought you and others are
quite interested in it to be there.
Jan
© 2016 - 2026 Red Hat, Inc.