Re: [PATCH v1 2/3] x86/msr: Switch between WRMSRNS and WRMSR with the alternatives mechanism

From: Xin Li
Date: Sat Aug 17 2024 - 11:46:03 EST


On 8/17/2024 7:43 AM, Borislav Petkov wrote:
On Sat, Aug 17, 2024 at 05:23:07PM +0300, Borislav Petkov wrote:
static __always_inline void wrmsr(u32 msr, u64 val)
{
asm volatile("1: " ALTERNATIVE_2("wrmsr", WRMSRNS, X86_FEATURE_WRMSRNS,
"call asm_xen_write_msr", X86_FEATURE_XENPV)
"2: " _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_WRMSR)
: : "c" (msr), "a" (val), "d" ((u32)(val >> 32)),
"D" (msr), "S" (val));
}


As the CALL instruction is 5-byte long, and we need to pad nop for both
WRMSR and WRMSRNS, what about not using segment prefix at all?

The alternatives macros can handle arbitrary insn sizes - no need for any padding.

What you cannot do is have a CALL insn in only one of the alternative
insn groups because the compiler is free to clobber callee regs and
those might be live where you place the alternative and thus will have
a nice lovely corruption.

Yeah, asm_xen_write_msr() is a wrapper function to xen_write_msr() which
saves/restores those regs.

My first draft patch calls xen_write_msr() directly and works fine. But
hpa said the same thing as you said.

Thanks!
Xin