On 5/2/2025 1:02 AM, Ingo Molnar wrote:

* Xin Li (Intel) [off-list ref] wrote:

quoted

index 94408a784c8e..13335a130edf 100644

--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h

@@ -7,7 +7,81 @@
  
  #include <asm/cpufeature.h>
  #include <asm/processor.h>
-#include <asm/msr.h>
+
+/*
+ * both i386 and x86_64 returns 64-bit value in edx:eax, but gcc's "A"
+ * constraint has different meanings. For i386, "A" means exactly
+ * edx:eax, while for x86_64 it doesn't mean rdx:rax or edx:eax. Instead,
+ * it means rax *or* rdx.
+ */
+#ifdef CONFIG_X86_64
+/* Using 64-bit values saves one instruction clearing the high half of low */
+#define DECLARE_ARGS(val, low, high)	unsigned long low, high
+#define EAX_EDX_VAL(val, low, high)	((low) | (high) << 32)
+#define EAX_EDX_RET(val, low, high)	"=a" (low), "=d" (high)
+#else
+#define DECLARE_ARGS(val, low, high)	u64 val
+#define EAX_EDX_VAL(val, low, high)	(val)
+#define EAX_EDX_RET(val, low, high)	"=A" (val)
+#endif

Meh, this patch creates a duplicate copy of DECLARE_ARGS() et al in
<asm/tsc.h> now:

  arch/x86/include/asm/msr.h:#define DECLARE_ARGS(val, low, high) unsigned long low, high
  arch/x86/include/asm/msr.h:#define DECLARE_ARGS(val, low, high) u64 val
  arch/x86/include/asm/msr.h:     DECLARE_ARGS(val, low, high);
  arch/x86/include/asm/msr.h:     DECLARE_ARGS(val, low, high);
  arch/x86/include/asm/msr.h:     DECLARE_ARGS(val, low, high);
  arch/x86/include/asm/tsc.h:#define DECLARE_ARGS(val, low, high) unsigned long low, high
  arch/x86/include/asm/tsc.h:#define DECLARE_ARGS(val, low, high) u64 val
  arch/x86/include/asm/tsc.h:     DECLARE_ARGS(val, low, high);
  arch/x86/include/asm/tsc.h:     DECLARE_ARGS(val, low, high);
  arch/x86/include/asm/tsc.h:#undef DECLARE_ARGS

Which was both an undeclared change, bloats the code, causes various
problems, and is totally unnecessary to boot.

Please don't do that ...

Learned!

Especially that every change needs to explicitly called out.