[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen master] x86/MCE: switch some callback invocations to altcall



commit 85ba4d050f9f3c4286164f21660ae88435b7e83c
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Mon Jan 22 13:41:07 2024 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Jan 22 13:41:07 2024 +0100

    x86/MCE: switch some callback invocations to altcall
    
    While not performance critical, these hook invocations still would
    better be converted: This way all pre-filled (and newly introduced)
    struct mce_callback instances can become __initconst_cf_clobber, thus
    allowing to eliminate another 9 ENDBR during the 2nd phase of
    alternatives patching.
    
    While this means registering callbacks a little earlier, doing so is
    perhaps even advantageous, for having pointers be non-NULL earlier on.
    Only one set of callbacks would only ever be registered anyway, and
    neither of the respective initialization function can (subsequently)
    fail.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mcheck/mcaction.c  | 10 ++----
 xen/arch/x86/cpu/mcheck/mcaction.h  |  5 ---
 xen/arch/x86/cpu/mcheck/mce.c       | 71 ++++++++++--------------------------
 xen/arch/x86/cpu/mcheck/mce.h       | 72 +++++++++++++++++++------------------
 xen/arch/x86/cpu/mcheck/mce_amd.c   | 26 +++++++-------
 xen/arch/x86/cpu/mcheck/mce_intel.c | 14 ++++----
 6 files changed, 80 insertions(+), 118 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c 
b/xen/arch/x86/cpu/mcheck/mcaction.c
index 695fb61d7d..bf7a0de965 100644
--- a/xen/arch/x86/cpu/mcheck/mcaction.c
+++ b/xen/arch/x86/cpu/mcheck/mcaction.c
@@ -27,13 +27,6 @@ mci_action_add_pageoffline(int bank, struct mc_info *mi,
     return rec;
 }
 
-mce_check_addr_t mc_check_addr = NULL;
-
-void __init mce_register_addrcheck(mce_check_addr_t cbfunc)
-{
-    mc_check_addr = cbfunc;
-}
-
 void
 mc_memerr_dhandler(struct mca_binfo *binfo,
                    enum mce_result *result,
@@ -48,7 +41,8 @@ mc_memerr_dhandler(struct mca_binfo *binfo,
     int vmce_vcpuid;
     unsigned int mc_vcpuid;
 
-    if ( !mc_check_addr(bank->mc_status, bank->mc_misc, MC_ADDR_PHYSICAL) )
+    if ( !alternative_call(mce_callbacks.check_addr, bank->mc_status,
+                           bank->mc_misc, MC_ADDR_PHYSICAL) )
     {
         dprintk(XENLOG_WARNING,
                 "No physical address provided for memory error\n");
diff --git a/xen/arch/x86/cpu/mcheck/mcaction.h 
b/xen/arch/x86/cpu/mcheck/mcaction.h
index 9cb2ad6c9f..6c79498cd2 100644
--- a/xen/arch/x86/cpu/mcheck/mcaction.h
+++ b/xen/arch/x86/cpu/mcheck/mcaction.h
@@ -12,9 +12,4 @@ mc_memerr_dhandler(struct mca_binfo *binfo,
 #define MC_ADDR_PHYSICAL  0
 #define MC_ADDR_VIRTUAL   1
 
-typedef bool (*mce_check_addr_t)(uint64_t status, uint64_t misc, int 
addr_type);
-extern void mce_register_addrcheck(mce_check_addr_t cbfunc);
-
-extern mce_check_addr_t mc_check_addr;
-
 #endif
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 7960ef35ba..2844685983 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -82,47 +82,21 @@ static void cf_check unexpected_machine_check(const struct 
cpu_user_regs *regs)
     fatal_trap(regs, 1);
 }
 
-static x86_mce_vector_t _machine_check_vector = unexpected_machine_check;
-
-void __init x86_mce_vector_register(x86_mce_vector_t hdlr)
-{
-    _machine_check_vector = hdlr;
-}
+struct mce_callbacks __ro_after_init mce_callbacks = {
+    .handler = unexpected_machine_check,
+};
+static const typeof(mce_callbacks.handler) __initconst_cf_clobber __used
+    default_handler = unexpected_machine_check;
 
 /* Call the installed machine check handler for this CPU setup. */
 
 void do_machine_check(const struct cpu_user_regs *regs)
 {
     mce_enter();
-    _machine_check_vector(regs);
+    alternative_vcall(mce_callbacks.handler, regs);
     mce_exit();
 }
 
-/*
- * Init machine check callback handler
- * It is used to collect additional information provided by newer
- * CPU families/models without the need to duplicate the whole handler.
- * This avoids having many handlers doing almost nearly the same and each
- * with its own tweaks ands bugs.
- */
-static x86_mce_callback_t mc_callback_bank_extended = NULL;
-
-void __init x86_mce_callback_register(x86_mce_callback_t cbfunc)
-{
-    mc_callback_bank_extended = cbfunc;
-}
-
-/*
- * Machine check recoverable judgement callback handler
- * It is used to judge whether an UC error is recoverable by software
- */
-static mce_recoverable_t mc_recoverable_scan = NULL;
-
-void __init mce_recoverable_register(mce_recoverable_t cbfunc)
-{
-    mc_recoverable_scan = cbfunc;
-}
-
 struct mca_banks *mcabanks_alloc(unsigned int nr)
 {
     struct mca_banks *mb;
@@ -173,19 +147,6 @@ static void mcabank_clear(int banknum)
     mca_wrmsr(MSR_IA32_MCx_STATUS(banknum), 0x0ULL);
 }
 
-/*
- * Judging whether to Clear Machine Check error bank callback handler
- * According to Intel latest MCA OS Recovery Writer's Guide,
- * whether the error MCA bank needs to be cleared is decided by the mca_source
- * and MCi_status bit value.
- */
-static mce_need_clearbank_t mc_need_clearbank_scan = NULL;
-
-void __init mce_need_clearbank_register(mce_need_clearbank_t cbfunc)
-{
-    mc_need_clearbank_scan = cbfunc;
-}
-
 /*
  * mce_logout_lock should only be used in the trap handler,
  * while MCIP has not been cleared yet in the global status
@@ -226,7 +187,8 @@ static void mca_init_bank(enum mca_source who, struct 
mc_info *mi, int bank)
 
     if ( (mib->mc_status & MCi_STATUS_MISCV) &&
          (mib->mc_status & MCi_STATUS_ADDRV) &&
-         (mc_check_addr(mib->mc_status, mib->mc_misc, MC_ADDR_PHYSICAL)) &&
+         alternative_call(mce_callbacks.check_addr, mib->mc_status,
+                          mib->mc_misc, MC_ADDR_PHYSICAL) &&
          (who == MCA_POLLER || who == MCA_CMCI_HANDLER) &&
          (mfn_valid(_mfn(paddr_to_pfn(mib->mc_addr)))) )
     {
@@ -326,7 +288,7 @@ mcheck_mca_logout(enum mca_source who, struct mca_banks 
*bankmask,
      * If no mc_recovery_scan callback handler registered,
      * this error is not recoverable
      */
-    recover = mc_recoverable_scan ? 1 : 0;
+    recover = mce_callbacks.recoverable_scan;
 
     for ( i = 0; i < this_cpu(nr_mce_banks); i++ )
     {
@@ -343,8 +305,9 @@ mcheck_mca_logout(enum mca_source who, struct mca_banks 
*bankmask,
          * decide whether to clear bank by MCi_STATUS bit value such as
          * OVER/UC/EN/PCC/S/AR
          */
-        if ( mc_need_clearbank_scan )
-            need_clear = mc_need_clearbank_scan(who, status);
+        if ( mce_callbacks.need_clearbank_scan )
+            need_clear = alternative_call(mce_callbacks.need_clearbank_scan,
+                                          who, status);
 
         /*
          * If this is the first bank with valid MCA DATA, then
@@ -380,12 +343,12 @@ mcheck_mca_logout(enum mca_source who, struct mca_banks 
*bankmask,
 
         if ( recover && uc )
             /* uc = true, recover = true, we need not panic. */
-            recover = mc_recoverable_scan(status);
+            recover = alternative_call(mce_callbacks.recoverable_scan, status);
 
         mca_init_bank(who, mci, i);
 
-        if ( mc_callback_bank_extended )
-            mc_callback_bank_extended(mci, i, status);
+        if ( mce_callbacks.info_collect )
+            alternative_vcall(mce_callbacks.info_collect, mci, i, status);
 
         /* By default, need_clear = true */
         if ( who != MCA_MCE_SCAN && need_clear )
@@ -1913,9 +1876,11 @@ static void cf_check mce_softirq(void)
  * will help to collect and log those MCE errors.
  * Round2: Do all MCE processing logic as normal.
  */
-void __init mce_handler_init(void)
+void __init mce_handler_init(const struct mce_callbacks *cb)
 {
     /* callback register, do we really need so many callback? */
+    mce_callbacks = *cb;
+
     /* mce handler data initialization */
     spin_lock_init(&mce_logout_lock);
     open_softirq(MACHINE_CHECK_SOFTIRQ, mce_softirq);
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index 9f87211832..7f26afae23 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -63,20 +63,12 @@ void x86_mc_get_cpu_info(unsigned cpu, uint32_t *chipid, 
uint16_t *coreid,
                          unsigned *ncores, unsigned *ncores_active,
                          unsigned *nthreads);
 
-/* Register a handler for machine check exceptions. */
-typedef void (*x86_mce_vector_t)(const struct cpu_user_regs *regs);
-extern void x86_mce_vector_register(x86_mce_vector_t hdlr);
-
 /*
  * Common generic MCE handler that implementations may nominate
  * via x86_mce_vector_register.
  */
 void cf_check mcheck_cmn_handler(const struct cpu_user_regs *regs);
 
-/* Register a handler for judging whether mce is recoverable. */
-typedef bool (*mce_recoverable_t)(uint64_t status);
-extern void mce_recoverable_register(mce_recoverable_t cbfunc);
-
 /* Read an MSR, checking for an interposed value first */
 extern struct intpose_ent *intpose_lookup(unsigned int cpu_nr, uint64_t msr,
     uint64_t *valp);
@@ -137,30 +129,6 @@ extern mctelem_cookie_t mcheck_mca_logout(enum mca_source 
who,
                                           struct mca_summary *sp,
                                           struct mca_banks *clear_bank);
 
-/*
- * Register callbacks to be made during bank telemetry logout.
- * Those callbacks are only available to those machine check handlers
- * that call to the common mcheck_cmn_handler or who use the common
- * telemetry logout function mcheck_mca_logout in error polling.
- */
-
-/* Register a handler for judging whether the bank need to be cleared */
-typedef bool (*mce_need_clearbank_t)(enum mca_source who, u64 status);
-extern void mce_need_clearbank_register(mce_need_clearbank_t cbfunc);
-
-/*
- * Register a callback to collect additional information (typically non-
- * architectural) provided by newer CPU families/models without the need
- * to duplicate the whole handler resulting in various handlers each with
- * its own tweaks and bugs. The callback receives an struct mc_info pointer
- * which it can use with x86_mcinfo_reserve to add additional telemetry,
- * the current MCA bank number we are reading telemetry from, and the
- * MCi_STATUS value for that bank.
- */
-typedef struct mcinfo_extended *(*x86_mce_callback_t)
-    (struct mc_info *mi, uint16_t bank, uint64_t status);
-extern void x86_mce_callback_register(x86_mce_callback_t cbfunc);
-
 void *x86_mcinfo_reserve(struct mc_info *mi,
                          unsigned int size, unsigned int type);
 void x86_mcinfo_dump(struct mc_info *mi);
@@ -201,8 +169,44 @@ static inline int mce_bank_msr(const struct vcpu *v, 
uint32_t msr)
     return 0;
 }
 
-/* MC softirq */
-void mce_handler_init(void);
+struct mce_callbacks {
+    void (*handler)(const struct cpu_user_regs *regs);
+    bool (*check_addr)(uint64_t status, uint64_t misc, int addr_type);
+
+    /* Handler for judging whether mce is recoverable. */
+    bool (*recoverable_scan)(uint64_t status);
+
+    /*
+     * Callbacks to be made during bank telemetry logout.
+     * They are only available to those machine check handlers
+     * that call to the common mcheck_cmn_handler or who use the common
+     * telemetry logout function mcheck_mca_logout in error polling.
+     */
+
+    /*
+     * Judging whether to Clear Machine Check error bank callback handler.
+     * According to Intel latest MCA OS Recovery Writer's Guide, whether
+     * the error MCA bank needs to be cleared is decided by the mca_source
+     * and MCi_status bit value.
+     */
+    bool (*need_clearbank_scan)(enum mca_source who, u64 status);
+
+    /*
+     * Callback to collect additional information (typically non-
+     * architectural) provided by newer CPU families/models without the need
+     * to duplicate the whole handler resulting in various handlers each with
+     * its own tweaks and bugs. The callback receives an struct mc_info pointer
+     * which it can use with x86_mcinfo_reserve to add additional telemetry,
+     * the current MCA bank number we are reading telemetry from, and the
+     * MCi_STATUS value for that bank.
+     */
+    struct mcinfo_extended *(*info_collect)
+        (struct mc_info *mi, uint16_t bank, uint64_t status);
+};
+
+extern struct mce_callbacks mce_callbacks;
+
+void mce_handler_init(const struct mce_callbacks *cb);
 
 extern const struct mca_error_handler *mce_dhandlers;
 extern const struct mca_error_handler *mce_uhandlers;
diff --git a/xen/arch/x86/cpu/mcheck/mce_amd.c 
b/xen/arch/x86/cpu/mcheck/mce_amd.c
index c8891de84d..3318b8204f 100644
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c
@@ -271,6 +271,19 @@ int vmce_amd_rdmsr(const struct vcpu *v, uint32_t msr, 
uint64_t *val)
     return 1;
 }
 
+static const struct mce_callbacks __initconst_cf_clobber k8_callbacks = {
+    .handler = mcheck_cmn_handler,
+    .need_clearbank_scan = amd_need_clearbank_scan,
+};
+
+static const struct mce_callbacks __initconst_cf_clobber k10_callbacks = {
+    .handler = mcheck_cmn_handler,
+    .check_addr = mc_amd_addrcheck,
+    .recoverable_scan = mc_amd_recoverable_scan,
+    .need_clearbank_scan = amd_need_clearbank_scan,
+    .info_collect = amd_f10_handler,
+};
+
 enum mcheck_type
 amd_mcheck_init(const struct cpuinfo_x86 *c, bool bsp)
 {
@@ -283,11 +296,7 @@ amd_mcheck_init(const struct cpuinfo_x86 *c, bool bsp)
     /* Assume that machine check support is available.
      * The minimum provided support is at least the K8. */
     if ( bsp )
-    {
-        mce_handler_init();
-        x86_mce_vector_register(mcheck_cmn_handler);
-        mce_need_clearbank_register(amd_need_clearbank_scan);
-    }
+        mce_handler_init(c->x86 == 0xf ? &k8_callbacks : &k10_callbacks);
 
     for ( i = 0; i < this_cpu(nr_mce_banks); i++ )
     {
@@ -327,13 +336,6 @@ amd_mcheck_init(const struct cpuinfo_x86 *c, bool bsp)
             ppin_msr = MSR_AMD_PPIN;
     }
 
-    if ( bsp )
-    {
-        x86_mce_callback_register(amd_f10_handler);
-        mce_recoverable_register(mc_amd_recoverable_scan);
-        mce_register_addrcheck(mc_amd_addrcheck);
-    }
-
     return c->x86_vendor == X86_VENDOR_HYGON ?
             mcheck_hygon : mcheck_amd_famXX;
 }
diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c 
b/xen/arch/x86/cpu/mcheck/mce_intel.c
index 84619aadd3..a99f9cce9d 100644
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -849,11 +849,6 @@ static void intel_init_mce(bool bsp)
     if ( !bsp )
         return;
 
-    x86_mce_vector_register(mcheck_cmn_handler);
-    mce_recoverable_register(intel_recoverable_scan);
-    mce_need_clearbank_register(intel_need_clearbank_scan);
-    mce_register_addrcheck(intel_checkaddr);
-
     mce_dhandlers = intel_mce_dhandlers;
     mce_dhandler_num = ARRAY_SIZE(intel_mce_dhandlers);
     mce_uhandlers = intel_mce_uhandlers;
@@ -963,6 +958,13 @@ static int cf_check cpu_callback(
     return notifier_from_errno(rc);
 }
 
+static const struct mce_callbacks __initconst_cf_clobber intel_callbacks = {
+    .handler = mcheck_cmn_handler,
+    .check_addr = intel_checkaddr,
+    .recoverable_scan = intel_recoverable_scan,
+    .need_clearbank_scan = intel_need_clearbank_scan,
+};
+
 static struct notifier_block cpu_nfb = {
     .notifier_call = cpu_callback
 };
@@ -989,7 +991,7 @@ enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, 
bool bsp)
     intel_init_mca(c);
 
     if ( bsp )
-        mce_handler_init();
+        mce_handler_init(&intel_callbacks);
 
     intel_init_mce(bsp);
 
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.