diff mbox series

x86/mce: Compile do_mca() for CONFIG_PV only

Message ID 20241119125904.2681402-1-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86/mce: Compile do_mca() for CONFIG_PV only | expand

Commit Message

Andrew Cooper Nov. 19, 2024, 12:59 p.m. UTC 
Eclair reports a Misra Rule 8.4 violation; that do_mca() can't see it's
declaration.  It turns out that this is a consequence of do_mca() being
PV-only, and the declaration being compiled out in !PV builds.

Therefore, arrange for do_mca() to be compiled out in !PV builds.  This in
turn requires a number of static functions to become static inline.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: consulting@bugseng.com <consulting@bugseng.com>

Bloat-o-meter on a !PV build reports:

  add/remove: 0/6 grow/shrink: 0/0 up/down: 0/-3717 (-3717)
  Function                                     old     new   delta
  x86_mc_mceinject                              31       -     -31
  do_mca.cold                                  117       -    -117
  x86_mc_msrinject                             147       -    -147
  x86_mc_msrinject.cold                        230       -    -230
  do_mc_get_cpu_info                           500       -    -500
  do_mca                                      2692       -   -2692
---
 xen/arch/x86/cpu/mcheck/mce.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)


base-commit: 3128d7248f2ad389b8e9a3e252958cbfbd1898ee
prerequisite-patch-id: 46b8fc2e9df2fd6be1bbbd6b50463e0e15a8f94d
prerequisite-patch-id: c122b170f57ab96fe52c37aebf1f4bb366194637
prerequisite-patch-id: 1c2d96bf17c5da0981b6c62939d3b7cc1e05933e
prerequisite-patch-id: b3e43902729416e18b4fada7f529b4cb02b1815e
prerequisite-patch-id: a06452180f71021893259bb3b883185f57742a31

Comments

Jan Beulich Nov. 19, 2024, 2:34 p.m. UTC | #1 
On 19.11.2024 13:59, Andrew Cooper wrote:
> Eclair reports a Misra Rule 8.4 violation; that do_mca() can't see it's
> declaration.  It turns out that this is a consequence of do_mca() being
> PV-only, and the declaration being compiled out in !PV builds.
> 
> Therefore, arrange for do_mca() to be compiled out in !PV builds.  This in
> turn requires a number of static functions to become static inline.

Which generally we advocate against. The #ifdef variant you pointed at on
Matrix wasn't all that bad. Plus ...

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: consulting@bugseng.com <consulting@bugseng.com>
> 
> Bloat-o-meter on a !PV build reports:
> 
>   add/remove: 0/6 grow/shrink: 0/0 up/down: 0/-3717 (-3717)
>   Function                                     old     new   delta
>   x86_mc_mceinject                              31       -     -31
>   do_mca.cold                                  117       -    -117
>   x86_mc_msrinject                             147       -    -147
>   x86_mc_msrinject.cold                        230       -    -230
>   do_mc_get_cpu_info                           500       -    -500
>   do_mca                                      2692       -   -2692

... what's the effect of the addition of "inline" on a PV=y build? By
using the keyword, we may end up talking the compiler into inlining of
code that better wouldn't be inlined.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 32c1b2756b90..2a88590525f0 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -932,7 +932,7 @@  void x86_mcinfo_dump(struct mc_info *mi)
     } while ( 1 );
 }
 
-static void cf_check do_mc_get_cpu_info(void *v)
+static inline void cf_check do_mc_get_cpu_info(void *v)
 {
     int cpu = smp_processor_id();
     int cindex, cpn;
@@ -1114,7 +1114,7 @@  bool intpose_inval(unsigned int cpu_nr, uint64_t msr)
      (r) <= MSR_IA32_MCx_MISC(per_cpu(nr_mce_banks, cpu) - 1) && \
      ((r) - MSR_IA32_MC0_CTL) % 4) /* excludes MCi_CTL */
 
-static bool x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
+static inline bool x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
 {
     const struct cpuinfo_x86 *c = &cpu_data[mci->mcinj_cpunr];
     int i, errs = 0;
@@ -1192,7 +1192,7 @@  static bool x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
     return !errs;
 }
 
-static uint64_t x86_mc_hwcr_wren(void)
+static inline uint64_t x86_mc_hwcr_wren(void)
 {
     uint64_t old;
 
@@ -1207,13 +1207,13 @@  static uint64_t x86_mc_hwcr_wren(void)
     return old;
 }
 
-static void x86_mc_hwcr_wren_restore(uint64_t hwcr)
+static inline void x86_mc_hwcr_wren_restore(uint64_t hwcr)
 {
     if ( !(hwcr & K8_HWCR_MCi_STATUS_WREN) )
         wrmsrl(MSR_K8_HWCR, hwcr);
 }
 
-static void cf_check x86_mc_msrinject(void *data)
+static inline void cf_check x86_mc_msrinject(void *data)
 {
     struct xen_mc_msrinject *mci = data;
     struct mcinfo_msr *msr;
@@ -1244,13 +1244,14 @@  static void cf_check x86_mc_msrinject(void *data)
         x86_mc_hwcr_wren_restore(hwcr);
 }
 
-/*ARGSUSED*/
-static void cf_check x86_mc_mceinject(void *data)
+static inline void cf_check x86_mc_mceinject(void *data)
 {
     printk("Simulating #MC on cpu %d\n", smp_processor_id());
     __asm__ __volatile__("int $0x12");
 }
 
+#ifdef CONFIG_PV /* do_mca() hypercall is PV-only */
+
 #if BITS_PER_LONG == 64
 
 #define ID2COOKIE(id) ((mctelem_cookie_t)(id))
@@ -1654,6 +1655,8 @@  long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
     return ret;
 }
 
+#endif /* CONFIG_PV */
+
 static int mcinfo_dumped;
 
 static int cf_check x86_mcinfo_dump_panic(mctelem_cookie_t mctc)