Re: [RFC PATCH v1 13/15] x86/msr: Use the alternatives mechanism to read MSR

[Francesco Lavra <francescolavra.fl@xxxxxxxxx>]

On 2025-03-31 at 8:22, Xin Li (Intel) wrote:
> diff --git a/arch/x86/xen/xen-asm.S b/arch/x86/xen/xen-asm.S
> index e672632b1cc0..6e7a9daa03d4 100644
> --- a/arch/x86/xen/xen-asm.S
> +++ b/arch/x86/xen/xen-asm.S
> @@ -399,3 +399,37 @@ SYM_CODE_END(xen_entry_SYSCALL_compat)
>       RET
>  SYM_FUNC_END(asm_xen_write_msr)
>  EXPORT_SYMBOL_GPL(asm_xen_write_msr)
> +
> +/*
> + * The prototype of the Xen C code:
> + *   struct { u64 val, bool done } xen_do_read_msr(u32 msr)
> + */
> +SYM_FUNC_START(asm_xen_read_msr)
> +     ENDBR
> +     FRAME_BEGIN
> +     XEN_SAVE_CALLEE_REGS_FOR_MSR
> +     mov %ecx, %edi          /* MSR number */
> +     call xen_do_read_msr
> +     test %dl, %dl           /* %dl=1, i.e., ZF=0, meaning
> successfully done */
> +     XEN_RESTORE_CALLEE_REGS_FOR_MSR
> +     jnz 2f
> +
> +1:   rdmsr
> +     _ASM_EXTABLE_FUNC_REWIND(1b, -5, FRAME_OFFSET /
> (BITS_PER_LONG / 8))
> +     shl $0x20, %rdx
> +     or %rdx, %rax
> +     /*
> +      * The top of the stack points directly at the return
> address;
> +      * back up by 5 bytes from the return address.
> +      */

This works only if this function has been called directly (e.g. via
`call asm_xen_write_msr`), but doesn't work with alternative call types
(like indirect calls). Not sure why one might want to use an indirect
call to invoke asm_xen_write_msr, but this creates a hidden coupling
between caller and callee.
I don't have a suggestion on how to get rid of this coupling, other
than setting ipdelta in _ASM_EXTABLE_FUNC_REWIND() to 0 and adjusting
the _ASM_EXTABLE_TYPE entries at the call sites to consider the
instruction that follows the function call (instead of the call
instruction) as the faulting instruction (which seems pretty ugly, at
least because what follows the function call could be an instruction
that might itself fault). But you may want to make this caveat explicit
in the comment.