Re: [PATCH v4 02/15] x86/msr: Move rdtsc{,_ordered}() to <asm/tsc.h>

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

 



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

* Xin Li (Intel) <xin@xxxxxxxxx> wrote:

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.




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux