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

[Jürgen Groß]

On 23.09.26 21:36, Shreshth Srivastava wrote:

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, will be fixed in V6.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature