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

From: H. Peter Anvin
Date: Fri Aug 16 2024 - 17:26:59 EST


On 8/16/24 11:40, Andrew Cooper wrote:

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?


You can use up to 4 prefixes of any kind (which includes opcode prefixes before 0F) before most decoders start hurting, so we can pad it out to 5 bytes by doing 3f 3f .. .. ..


My suggestion, not that I've had time to experiment, was to change
paravirt to use a non-C ABI and have asm_xen_write_msr() recombine
edx:eax into rsi.  That way the top level wrmsr() retains sensible
codegen for native even when paravirt is active.


I have attached what should be an "obvious" example... famous last words.

-hpa
/*
* Input in %rax, MSR number in %ecx
* %rdx is clobbered, as the native stub is assumed to do
*
* Change xen_do_write_msr to return its error code instead
* of passing a pointer - this gives the extra benefit that
* we can get the *actual invocation address* in the error
* messages.
*
* ex_handler_msr() should be changed to get the MSR data
* from %rax regardless of Xen vs native; alternatively the
* Xen handler can set up %edx as well.
*
* Let the native pattern look like:
*
* 48 89 c2 mov %rax,%rdx
* 48 c1 ea 20 shr $32,%rdx
* 3e 0f 30 ds wrmsr <--- trap point
*
* ... which can be replaced with ...
* 48 89 c2 mov %rax,%rdx
* 48 c1 ea 20 shr $32,%rdx
* 0f 01 c6 wrmsrns <--- trap point
*
* ... or ...
* e8 xx xx xx xx call asm_xen_write_msr
* 74 02 jz 1f
* 3e 0f 0b ds ud2 <--- trap point
* 1:
* ds ud2 can of course be replaced with any other 3-byte trapping
* instruction.
*
* This also removes the need for Xen to maintain different safe and
* unsafe MSR routines, as the difference is handled by the same
* trap handler as is used natively.
*/
SYM_FUNC_START(asm_xen_write_msr)
FRAME_BEGIN
push %rax /* Save in case of error */
push %rcx
push %rsi
push %rdi
push %r8
push %r9
push %r10
push %r11
mov %rax,%rdi
mov %ecx,%esi
call xen_do_write_msr
test %eax,%eax /* ZF=0 on error */
pop %r11
pop %r10
pop %r9
pop %r8
pop %rdi
pop %rsi
pop %rcx
#ifdef OPTIONAL
mov 4(%rsp),%edx
#endif
pop %rax
FRAME_END
RET
SYM_FUNC_END(asm_xen_write_msr)