Re: [PATCH 02/22] x86/msr: Rename wrmsr_ns() to wrmsrns(), and take 64bit value

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/08/2025 9:22 pm, Andrew Cooper wrote:
> In hindsight, having the wrapper name not be the instruction mnemonic was a
> poor choice.  Also, PKS turns out to be quite rare in wanting a split value.
>
> Switch to using a single 64bit value in preparation for new users.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/x86/include/asm/msr.h      | 4 ++--
>  xen/arch/x86/include/asm/prot-key.h | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
> index 4c4f18b3a54d..b6b85b04c3fd 100644
> --- a/xen/arch/x86/include/asm/msr.h
> +++ b/xen/arch/x86/include/asm/msr.h
> @@ -39,7 +39,7 @@ static inline void wrmsrl(unsigned int msr, uint64_t val)
>  }
>  
>  /* Non-serialising WRMSR, when available.  Falls back to a serialising 
> WRMSR. */
> -static inline void wrmsr_ns(uint32_t msr, uint32_t lo, uint32_t hi)
> +static inline void wrmsrns(uint32_t msr, uint64_t val)
>  {
>      /*
>       * WRMSR is 2 bytes.  WRMSRNS is 3 bytes.  Pad WRMSR with a redundant CS
> @@ -47,7 +47,7 @@ static inline void wrmsr_ns(uint32_t msr, uint32_t lo, 
> uint32_t hi)
>       */
>      alternative_input(".byte 0x2e; wrmsr",
>                        ".byte 0x0f,0x01,0xc6", X86_FEATURE_WRMSRNS,
> -                      "c" (msr), "a" (lo), "d" (hi));
> +                      "c" (msr), "a" (val), "d" (val >> 32));
>  }

It turns out this is the case poor code generation for MSR_STAR.

I've adjusted it to:

@@ -39,8 +39,10 @@ static inline void wrmsrl(unsigned int msr, uint64_t val)
 }
 
 /* Non-serialising WRMSR, when available.  Falls back to a serialising WRMSR. 
*/
-static inline void wrmsr_ns(uint32_t msr, uint32_t lo, uint32_t hi)
+static inline void wrmsrns(uint32_t msr, uint64_t val)
 {
+    uint32_t lo = val, hi = val >> 32;
+
     /*
      * WRMSR is 2 bytes.  WRMSRNS is 3 bytes.  Pad WRMSR with a redundant CS
      * prefix to avoid a trailing NOP.


which stops the compiler from loading the high half of %rax too.

~Andrew