diff mbox series

x86/apic: Fix function typechecking in TSC Deadline errata check

Message ID 20220321141207.18422-1-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/apic: Fix function typechecking in TSC Deadline errata check | expand

Commit Message

Andrew Cooper March 21, 2022, 2:12 p.m. UTC 
Sadly, cf_check typechecking doesn't work through casts.  Introduce an ad-hoc
typecheck and fix *_readline_rev() checks to be cf_check.

This is a latent bug.  The affected models don't have CET-IBT, so won't
actually explode from lacking endbr64 instructions.

Reported-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/apic.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Jan Beulich March 21, 2022, 2:26 p.m. UTC | #1 
On 21.03.2022 15:12, Andrew Cooper wrote:
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -1092,12 +1092,17 @@ static void setup_APIC_timer(void)
>      local_irq_restore(flags);
>  }
>  
> +#define DEADLINE_MODEL_FUNCT(m, fn) \
> +    { .vendor = X86_VENDOR_INTEL, .family = 6, .model = (m), \
> +      .feature = X86_FEATURE_TSC_DEADLINE, \
> +      .driver_data = fn + (0 * sizeof(fn == ((unsigned int (*)(void))NULL))) }

Are you sure all compiler versions we support are happy about +
of a function pointer and a constant? Even if that constant is zero,
this is not legal as per the plain C spec.

Also strictly speaking you would want to parenthesize both uses of
fn.

>  #define DEADLINE_MODEL_MATCH(m, fr) \
>      { .vendor = X86_VENDOR_INTEL, .family = 6, .model = (m), \
>        .feature = X86_FEATURE_TSC_DEADLINE, \
>        .driver_data = (void *)(unsigned long)(fr) }

As long as we leave this in place, there's a (small) risk of the
wrong macro being used again if another hook would need adding here.
We might be safer if driver_data became "unsigned long" and the
void * cast was dropped from here (with an "unsigned long" cast
added in the new macro, which at the same time would address my
other concern above).

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 96d73a744964..794bbc21ae2c 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -1092,12 +1092,17 @@  static void setup_APIC_timer(void)
     local_irq_restore(flags);
 }
 
+#define DEADLINE_MODEL_FUNCT(m, fn) \
+    { .vendor = X86_VENDOR_INTEL, .family = 6, .model = (m), \
+      .feature = X86_FEATURE_TSC_DEADLINE, \
+      .driver_data = fn + (0 * sizeof(fn == ((unsigned int (*)(void))NULL))) }
+
 #define DEADLINE_MODEL_MATCH(m, fr) \
     { .vendor = X86_VENDOR_INTEL, .family = 6, .model = (m), \
       .feature = X86_FEATURE_TSC_DEADLINE, \
       .driver_data = (void *)(unsigned long)(fr) }
 
-static unsigned int __init hsx_deadline_rev(void)
+static unsigned int __init cf_check hsx_deadline_rev(void)
 {
     switch ( boot_cpu_data.x86_mask )
     {
@@ -1108,7 +1113,7 @@  static unsigned int __init hsx_deadline_rev(void)
     return ~0U;
 }
 
-static unsigned int __init bdx_deadline_rev(void)
+static unsigned int __init cf_check bdx_deadline_rev(void)
 {
     switch ( boot_cpu_data.x86_mask )
     {
@@ -1121,7 +1126,7 @@  static unsigned int __init bdx_deadline_rev(void)
     return ~0U;
 }
 
-static unsigned int __init skx_deadline_rev(void)
+static unsigned int __init cf_check skx_deadline_rev(void)
 {
     switch ( boot_cpu_data.x86_mask )
     {
@@ -1135,17 +1140,17 @@  static unsigned int __init skx_deadline_rev(void)
 
 static const struct x86_cpu_id __initconstrel deadline_match[] = {
     DEADLINE_MODEL_MATCH(0x3c, 0x22),             /* Haswell */
-    DEADLINE_MODEL_MATCH(0x3f, hsx_deadline_rev), /* Haswell EP/EX */
+    DEADLINE_MODEL_FUNCT(0x3f, hsx_deadline_rev), /* Haswell EP/EX */
     DEADLINE_MODEL_MATCH(0x45, 0x20),             /* Haswell D */
     DEADLINE_MODEL_MATCH(0x46, 0x17),             /* Haswell H */
 
     DEADLINE_MODEL_MATCH(0x3d, 0x25),             /* Broadwell */
     DEADLINE_MODEL_MATCH(0x47, 0x17),             /* Broadwell H */
     DEADLINE_MODEL_MATCH(0x4f, 0x0b000020),       /* Broadwell EP/EX */
-    DEADLINE_MODEL_MATCH(0x56, bdx_deadline_rev), /* Broadwell D */
+    DEADLINE_MODEL_FUNCT(0x56, bdx_deadline_rev), /* Broadwell D */
 
     DEADLINE_MODEL_MATCH(0x4e, 0xb2),             /* Skylake M */
-    DEADLINE_MODEL_MATCH(0x55, skx_deadline_rev), /* Skylake X */
+    DEADLINE_MODEL_FUNCT(0x55, skx_deadline_rev), /* Skylake X */
     DEADLINE_MODEL_MATCH(0x5e, 0xb2),             /* Skylake D */
 
     DEADLINE_MODEL_MATCH(0x8e, 0x52),             /* Kabylake M */