Re: [PATCH v5 00/17] x86/msr: Inline rdmsr/wrmsr instructions

[Shreshth Srivastava <shreshth.srivastava@intel.com>]

On 11.09.26 10:41, Juergen Gross wrote:
> When building a kernel with CONFIG_PARAVIRT_XXL the paravirt
> infrastructure will always use functions for reading or writing MSRs,
> even when running on bare metal.

Hi Juergen,

This doesn't build with CONFIG_PARAVIRT_XXL=y. 16/17 and 17/17 are where
it breaks, but the cause is the .byte fallbacks: ASM_WRMSRNS_IMM from
08/17 and ASM_RDMSR_IMM from 10/17 don't end in a separator, unlike the
.insn variants above them.

  #define ASM_RDMSR_IMM				\
	" .byte 0xc4,0xe7,0x7b,0xf6,0xc0; .long %c[msr]"

That worked while they were only ever the last argument of an
ALTERNATIVE(), which appends its own newline. 16/17 concatenates them
with ASM_CLRERR:

	ASM_RDMSR_IMM ASM_CLRERR, X86_FEATURE_MSR_IMM,	\

so the .long operand runs into the xor. From
make arch/x86/kernel/cpu/common.s:

	.byte 0xc4,0xe7,0x7b,0xf6,0xc0; .long 266xor %rdx,%rdx

  paravirt-msr.h:165: Error: junk at end of line, first unrecognized character is `x'

clang reports "error: unexpected token" in the same place. 71 objects
fail, the same 71 either way, no vmlinux.

msr.h chooses between the .insn form and the .byte fallback with:

	#if defined(CONFIG_AS_IS_GNU) && CONFIG_AS_VERSION >= 24100

Two kinds of toolchain end up on the .byte side of that test:

  - GNU as older than 2.41. RHEL 9 and CentOS Stream 9 ship 2.35, and
    Documentation/process/changes.rst sets the minimum at 2.30, so this
    is a supported configuration rather than an old outlier.
  - clang, any version. CONFIG_AS_IS_GNU is never set for clang, so the
    && short-circuits and the version comparison is never reached. Your
    08/17 comment already notes that clang has no .insn support.

gcc with binutils 2.41 or newer takes the .insn path, where both macros
do end in a separator, and is unaffected.

Reproduced on v7.3-rc2 with your v3 00/13, v2 0/5 and v5 00/17 applied in
that order, x86_64 defconfig plus HYPERVISOR_GUEST, PARAVIRT, XEN and
XEN_PV. gcc 11.5.0 with GNU as 2.35.2, and clang 21.1.7.

Terminating both fallbacks fixes it, and both toolchains then build
vmlinux with no errors or warnings:

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index eba325ecfe4c..529c13553c63 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -78,9 +78,9 @@ static inline void do_trace_rdpmc(u32 msr, u64 val, int failed) {}
  * form MSR access instructions reference %rax as the register operand.
  */
 #define ASM_RDMSR_IMM				\
-	" .byte 0xc4,0xe7,0x7b,0xf6,0xc0; .long %c[msr]"
+	" .byte 0xc4,0xe7,0x7b,0xf6,0xc0; .long %c[msr]\n\t"
 #define ASM_WRMSRNS_IMM				\
-	" .byte 0xc4,0xe7,0x7a,0xf6,0xc0; .long %c[msr]"
+	" .byte 0xc4,0xe7,0x7a,0xf6,0xc0; .long %c[msr]\n\t"
 #endif

 #define RDMSR_AND_SAVE_RESULT			\

ASM_WRMSRNS needs no change, _ASM_BYTES() already emits a semicolon.

The WRMSRNS line belongs in 08/17 and the RDMSR line in 10/17.

Thanks,
Shreshth