On Mon, Nov 7, 2022 at 11:08 AM Paolo Bonzini pbonzini@redhat.com wrote:
On 11/7/22 19:45, Jim Mattson wrote:
+.macro RESTORE_GUEST_SPEC_CTRL
/* No need to do anything if SPEC_CTRL is unset or V_SPEC_CTRL is set */ALTERNATIVE_2 "jmp 999f", \"", X86_FEATURE_MSR_SPEC_CTRL, \"jmp 999f", X86_FEATURE_V_SPEC_CTRL/** SPEC_CTRL handling: if the guest's SPEC_CTRL value differs from the* host's, write the MSR.** IMPORTANT: To avoid RSB underflow attacks and any other nastiness,* there must not be any returns or indirect branches between this code* and vmentry.*/movl SVM_spec_ctrl(%_ASM_DI), %eaxcmp PER_CPU_VAR(x86_spec_ctrl_current), %eaxje 999fmov $MSR_IA32_SPEC_CTRL, %ecxxor %edx, %edxwrmsr+999:
+.endm
+.macro RESTORE_HOST_SPEC_CTRL
/* No need to do anything if SPEC_CTRL is unset or V_SPEC_CTRL is set */ALTERNATIVE_2 "jmp 999f", \"", X86_FEATURE_MSR_SPEC_CTRL, \"jmp 999f", X86_FEATURE_V_SPEC_CTRLmov $MSR_IA32_SPEC_CTRL, %ecx/** Load the value that the guest had written into MSR_IA32_SPEC_CTRL,* if it was not intercepted during guest execution.*/cmpb $0, (%_ASM_SP)jnz 998frdmsrmovl %eax, SVM_spec_ctrl(%_ASM_DI)+998:
/* Now restore the host value of the MSR if different from the guest's. */movl PER_CPU_VAR(x86_spec_ctrl_current), %eaxcmp SVM_spec_ctrl(%_ASM_DI), %eaxje 999fxor %edx, %edxwrmsr+999:
+.endm
It seems unfortunate to have the unconditional branches in the more common cases.
One way to do it could be something like
.macro RESTORE_HOST_SPEC_CTRL ALTERNATIVE_2 "", \ "jmp 900f", X86_FEATURE_MSR_SPEC_CTRL, \ "", X86_FEATURE_V_SPEC_CTRL \ 901: .endm
.macro RESTORE_SPEC_CTRL_BODY \ 800: /* restore guest spec ctrl ... */ jmp 801b
900: /* save guest spec ctrl + restore host ... */ jmp 901b .endm
The cmp/je pair can also jump back to 801b/901b.
What do you think? I'll check if objtool is happy and if so include it in v2.
Paolo
This seems reasonable, if you trust a direct branch prior to the IBPB.