On Wed, Aug 07, 2024, Amit Shah wrote:
> From: Amit Shah <amit.shah@xxxxxxx>
>
> Remove superfluous RSB filling after a VMEXIT when the CPU already has
> flushed the RSB after a VMEXIT when AutoIBRS is enabled.
>
> The initial implementation for adding RETPOLINES added an ALTERNATIVES
> implementation for filling the RSB after a VMEXIT in
>
> commit 117cc7a908c836 ("x86/retpoline: Fill return stack buffer on vmexit")
Nit, no need for 14 digits, 12 is still the "official" recommendation. To make
this flow better, I would also prefer to not have each commit reference be on
its own line.
> Later, X86_FEATURE_RSB_VMEXIT was added in
>
> commit 2b129932201673 ("x86/speculation: Add RSB VM Exit protections")
Oh, the irony. That commit didn't add RSB_VMEXIT, it added RSB_VMEXIT_LITE.
Commit 9756bba28470 ("x86/speculation: Fill RSB on vmexit for IBRS") added the
"heavy" version. This is also a good opportunity to call out with RSB_VMEXIT
actually does.
> The AutoIBRS (on AMD CPUs) feature implementation added in
>
> commit e7862eda309ecf ("x86/cpu: Support AMD Automatic IBRS")
>
> used the already-implemented logic for EIBRS in
> spectre_v2_determine_rsb_fill_type_on_vmexit() -- but did not update the
> code at VMEXIT to act on the mode selected in that function -- resulting
> in VMEXITs continuing to clear the RSB when RETPOLINES are enabled,
> despite the presence of AutoIBRS.
>
> Signed-off-by: Amit Shah <amit.shah@xxxxxxx>
>
> ---
> diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
> index a0c8eb37d3e1..69d9825ebdd9 100644
> --- a/arch/x86/kvm/svm/vmenter.S
> +++ b/arch/x86/kvm/svm/vmenter.S
> @@ -209,10 +209,14 @@ SYM_FUNC_START(__svm_vcpu_run)
> 7: vmload %_ASM_AX
> 8:
>
> -#ifdef CONFIG_MITIGATION_RETPOLINE
> - /* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */
> - FILL_RETURN_BUFFER %_ASM_AX, RSB_CLEAR_LOOPS, X86_FEATURE_RETPOLINE
> -#endif
> + /*
> + * IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET!
> + *
> + * Unlike VMX, AMD does not have the hardware bug that necessitates
> + * RSB_VMEXIT_LITE
> + */
I would rather do nothing than carry these comments. Long term, I would still
prefer to have RSB_VMEXIT_LITE, but that's not a hill worth dying on, and I am
a-ok waiting to deal with that if/when I (or someone else) takes on the task
of unifying the VM-Enter/VM-Exit flows.
So, with the comment changes dropped and the changelog massaged to this:
Remove superfluous RSB filling after a VMEXIT when the CPU already has
flushed the RSB after a VMEXIT when AutoIBRS is enabled.
The initial implementation for adding RETPOLINES added an ALTERNATIVES
implementation for filling the RSB after a VMEXIT in commit 117cc7a908c8
("x86/retpoline: Fill return stack buffer on vmexit").
Later, X86_FEATURE_RSB_VMEXIT was added in commit 9756bba28470
("x86/speculation: Fill RSB on vmexit for IBRS") to handle stuffing the
RSB if RETPOLINE=y *or* KERNEL_IBRS=y, i.e. to also stuff the RSB if the
kernel is configured to do IBRS mitigations on entry/exit.
The AutoIBRS (on AMD) feature implementation added in commit e7862eda309e
("x86/cpu: Support AMD Automatic IBRS") used the already-implemented logic
for EIBRS in spectre_v2_determine_rsb_fill_type_on_vmexit() -- but did not
update the code at VMEXIT to act on the mode selected in that function --
resulting in VMEXITs continuing to clear the RSB when RETPOLINES are
enabled, despite the presence of AutoIBRS.
Applied to kvm-x86 svm.
[1/1] KVM: SVM: let alternatives handle the cases when RSB filling is required
https://github.com/kvm-x86/linux/commit/4440337af4d4
--
https://github.com/kvm-x86/linux/tree/next
