diff mbox series

[v5,01/10] xen: Implement xen/alternative-call.h for use in common code

Message ID 20210910021236.15082-2-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series xsm: refactoring xsm hooks | expand

Commit Message

Daniel P. Smith Sept. 10, 2021, 2:12 a.m. UTC
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>
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bob Eshleman <bobbyeshleman@gmail.com>
CC: Alistair Francis <alistair.francis@wdc.com>
CC: Connor Davis <connojdavis@gmail.com>
CC: Daniel P. Smith <dpsmith@apertussolutions.com>

v3:
 * Drop __alt_call_maybe_initconst

This is a pre-requisite to "xsm: refactor xsm_ops handling" to avoid breaking
the ARM build.

Build test for the XSM code:

  diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
  index 5eab21e1b168..592074e8f41c 100644
  --- a/xen/xsm/xsm_core.c
  +++ b/xen/xsm/xsm_core.c
  @@ -195,6 +195,16 @@ bool __init has_xsm_magic(paddr_t start)
   }
    #endif

  +#include <xen/alternative-call.h>
  +struct foo {
  +    int (*bar)(void *);
  +} foo __alt_call_maybe_initdata;
  +
  +int test_alternative_call(void)
  +{
  +    return alternative_call(foo.bar, NULL);
  +}
  +
   int __init register_xsm(struct xsm_operations *ops)
    {
         if ( verify(ops) )
---
 xen/arch/x86/Kconfig               |  1 +
 xen/common/Kconfig                 |  3 ++
 xen/include/xen/alternative-call.h | 63 ++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 xen/include/xen/alternative-call.h

Comments

Jan Beulich Sept. 10, 2021, 6:10 a.m. UTC | #1
On 10.09.2021 04:12, Daniel P. Smith wrote:
> 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>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

I did offer an A-b on v4, dependent upon two small adjustments. Was it
deliberate to neither make these adjustments nor add the ack? (At that
point I did offer making the adjustments while committing, but I think
it goes without saying that upon re-submission the changes should be
made. Unless of course there's disagreement about making them in the
first place. Yet such disagreement would be worth making explicit then,
imo.)

Jan
Daniel P. Smith Sept. 10, 2021, 10:12 a.m. UTC | #2
On 9/10/21 2:10 AM, Jan Beulich wrote:
> On 10.09.2021 04:12, Daniel P. Smith wrote:
>> 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>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> 
> I did offer an A-b on v4, dependent upon two small adjustments. Was it
> deliberate to neither make these adjustments nor add the ack? (At that
> point I did offer making the adjustments while committing, but I think
> it goes without saying that upon re-submission the changes should be
> made. Unless of course there's disagreement about making them in the
> first place. Yet such disagreement would be worth making explicit then,
> imo.)
> 
> Jan
> 

Yes you did, perhaps I did not follow the thread correctly. Re-reading 
it you mentioned they could be addressed during committing. Since I am 
going to have to respin this because the hard-tab foul up, I can pick up 
those to adjustments and add your Ack-by.

v/r,
dps
diff mbox series

Patch

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 9b164db641..1f83518ee0 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -6,6 +6,7 @@  config X86
 	def_bool y
 	select ACPI
 	select ACPI_LEGACY_TABLES_LOOKUP
+	select ALTERNATIVE_CALL
 	select ARCH_SUPPORTS_INT128
 	select CORE_PARKING
 	select HAS_ALTERNATIVE
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0ddd18e11a..ac5491b1cc 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -22,6 +22,9 @@  config GRANT_TABLE
 
 	  If unsure, say Y.
 
+config ALTERNATIVE_CALL
+	bool
+
 config HAS_ALTERNATIVE
 	bool
 
diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
new file mode 100644
index 0000000000..d10af90b1b
--- /dev/null
+++ b/xen/include/xen/alternative-call.h
@@ -0,0 +1,63 @@ 
+/* 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) const struct foo_ops __initconst foo_a_ops = { ... };
+ *     const struct foo_ops __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
+
+#else
+
+#define alternative_call(func, args...)  (func)(args)
+#define alternative_vcall(func, args...) (func)(args)
+
+#define __alt_call_maybe_initdata __read_mostly
+
+#endif /* !CONFIG_ALTERNATIVE_CALL */
+#endif /* XEN_ALTERNATIVE_CALL */