On 10.01.2023 18:18, Andrew Cooper wrote:
> WRMSR Non-Serialising is an optimisation intended for cases where an MSR needs
> updating, but architectural serialising properties are not needed.
>
> In is anticipated that this will apply to most if not all MSRs modified on
> context switch paths.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
This will allow me to drop half of what the respective emulator patch
consists of, which I'm yet to post (but which in turn is sitting on
top of many other already posted emulator patches). Comparing with
that patch, one nit though:
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -189,6 +189,7 @@ static const char *const str_7a1[32] =
>
> [10] = "fzrm", [11] = "fsrs",
> [12] = "fsrcs",
> + /* 18 */ [19] = "wrmsrns",
> };
We commonly leave a blank line to indicate dis-contiguous entries.
> --- a/xen/arch/x86/include/asm/msr.h
> +++ b/xen/arch/x86/include/asm/msr.h
> @@ -38,6 +38,18 @@ static inline void wrmsrl(unsigned int msr, __u64 val)
> wrmsr(msr, lo, hi);
> }
>
> +/* 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)
> +{
> + /*
> + * WRMSR is 2 bytes. WRMSRNS is 3 bytes. Pad WRMSR with a redundant CS
> + * prefix to avoid a trailing NOP.
> + */
> + alternative_input(".byte 0x2e; wrmsr",
> + ".byte 0x0f,0x01,0xc6", X86_FEATURE_WRMSRNS,
> + "c" (msr), "a" (lo), "d" (hi));
> +}
No wrmsrl_ns() and/or wrmsr_ns_safe() variants right away?
Do you have any indications towards a CS prefix being the least risky
one to use here (or in general)? Recognizing that segment prefixes have
gained alternative meaning in certain contexts, I would otherwise wonder
whether an address or operand size prefix wouldn't be more suitable.
Jan
