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

["Jürgen Groß" <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3224 bytes --]

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

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]