Mailing List Archive

commit f7bd03b6080b1601730fd702a449399aa8e6942c
Author: Jan Beulich <jbeulich@suse.com>
AuthorDate: Mon Jan 22 13:41:07 2024 +0100
Commit: Andrew Cooper <andrew.cooper3@citrix.com>
CommitDate: Tue Apr 9 16:45:01 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@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 85ba4d050f9f3c4286164f21660ae88435b7e83c)
---
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 5cbe558fb0..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);
-
-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 57044f7804..b07eb95dc2 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 )
@@ -1912,9 +1875,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 f2f70a0bb8..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 *, uint16_t, uint64_t);
-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#stable-4.18