From: Jon Kohler <jon@nutanix.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Venkatesh Srinivas <venkateshs@chromium.org>,
Jon Kohler <jon@nutanix.com>, Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"x86@kernel.org" <x86@kernel.org>,
"H. Peter Anvin" <hpa@zytor.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KVM: x86: use X86_FEATURE_RSB_CTXSW for RSB stuffing in vmexit
Date: Fri, 7 May 2021 18:26:18 +0000 [thread overview]
Message-ID: <BDF4AF6B-DB1F-4BAF-999E-72CDDF408F37@nutanix.com> (raw)
In-Reply-To: <YJV/sZvgA8uN/23k@google.com>
> On May 7, 2021, at 1:58 PM, Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, May 07, 2021, Venkatesh Srinivas wrote:
>> On Fri, May 7, 2021 at 8:08 AM Jon Kohler <jon@nutanix.com> wrote:
>>>
>>> cpufeatures.h defines X86_FEATURE_RSB_CTXSW as "Fill RSB on context
>>> switches" which seems more accurate than using X86_FEATURE_RETPOLINE
>>> in the vmxexit path for RSB stuffing.
>>>
>>> X86_FEATURE_RSB_CTXSW is used for FILL_RETURN_BUFFER in
>>> arch/x86/entry/entry_{32|64}.S. This change makes KVM vmx and svm
>>> follow that same pattern. This pairs up nicely with the language in
>>> bugs.c, where this cpu_cap is enabled, which indicates that RSB
>>> stuffing should be unconditional with spectrev2 enabled.
>>> /*
>>> * If spectre v2 protection has been enabled, unconditionally fill
>>> * RSB during a context switch; this protects against two independent
>>> * issues:
>>> *
>>> * - RSB underflow (and switch to BTB) on Skylake+
>>> * - SpectreRSB variant of spectre v2 on X86_BUG_SPECTRE_V2 CPUs
>>> */
>>> setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
>>>
>>> Furthermore, on X86_FEATURE_IBRS_ENHANCED CPUs && SPECTRE_V2_CMD_AUTO,
>>> we're bypassing setting X86_FEATURE_RETPOLINE, where as far as I could
>>> find, we should still be doing RSB stuffing no matter what when
>>> CONFIG_RETPOLINE is enabled and spectrev2 is set to auto.
>>
>> If I'm reading https://urldefense.proofpoint.com/v2/url?u=https-3A__software.intel.com_security-2Dsoftware-2Dguidance_deep-2Ddives_deep-2Ddive-2Dindirect-2Dbranch-2Drestricted-2Dspeculation&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=d1CkIBCdVFwUtKYx3SRW9dZD0kA_IX9VKEPG2-x4kBo&s=KlK_T41o6UVpLhMKDcK9iZfsJnop72K3CveJVIak5K8&e=
>> correctly, I don't think an RSB fill sequence is required on VMExit on
>> processors w/ Enhanced IBRS. Specifically:
>> """
>> On processors with enhanced IBRS, an RSB overwrite sequence may not
>> suffice to prevent the predicted target of a near return from using an
>> RSB entry created in a less privileged predictor mode. Software can
>> prevent this by enabling SMEP (for transitions from user mode to
>> supervisor mode) and by having IA32_SPEC_CTRL.IBRS set during VM exits
>> """
>> On Enhanced IBRS processors, it looks like SPEC_CTRL.IBRS is set
>> across all #VMExits via x86_virt_spec_ctrl in kvm.
>>
>> So is this patch needed?
>
> Venkatesh belatedly pointed out (off list) that KVM VMX stops intercepting
> MSR_IA32_SPEC_CTRL after the first (successful) write by the guest. But, I
> believe that's a non-issue for ENHANCED_IBRS because of this blurb in Intel's
> documentation[*]:
>
> Processors with enhanced IBRS still support the usage model where IBRS is set
> only in the OS/VMM for OSes that enable SMEP. To do this, such processors will
> ensure that guest behavior cannot control the RSB after a VM exit once IBRS is
> set, even if IBRS was not set at the time of the VM exit.
>
> The code and changelog for commit 706d51681d63 ("x86/speculation: Support
> Enhanced IBRS on future CPUs") is more than a little confusing:
>
> spectre_v2_select_mitigation():
> if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> mode = SPECTRE_V2_IBRS_ENHANCED;
> /* Force it so VMEXIT will restore correctly */
> x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
> wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
> goto specv2_set_mode;
> }
Thanks Sean, that makes sense. The handling of the MSR part of things
is separate from what I was looking at, which is where it seems like
Intel is still recommending doing an RSB overwrite/stuff even with
eIBRS; however, in KVM we use X86_FEATURE_RETPOLINE to figure
that out, but in bugs.c eIBRS systems skip having set, as
goto specv2_set_mode skips cover retpoline_auto now.
>
> changelog:
> Kernel also has to make sure that IBRS bit remains set after
> VMEXIT because the guest might have cleared the bit. This is already
> covered by the existing x86_spec_ctrl_set_guest() and
> x86_spec_ctrl_restore_host() speculation control functions.
>
> but I _think_ that is simply saying that MSR_IA32_SPEC_CTRL.IBRS needs to be
> restored in order to keep the mitigations active in the host. I don't think it
> contradicts the documentation that says VM-Exit is automagically mitigated if
> IBRS has _ever_ been set.
>
> [*] https://urldefense.proofpoint.com/v2/url?u=https-3A__software.intel.com_security-2Dsoftware-2Dguidance_deep-2Ddives_deep-2Ddive-2Dindirect-2Dbranch-2Drestricted-2Dspeculation&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=d1CkIBCdVFwUtKYx3SRW9dZD0kA_IX9VKEPG2-x4kBo&s=KlK_T41o6UVpLhMKDcK9iZfsJnop72K3CveJVIak5K8&e=
prev parent reply other threads:[~2021-05-07 18:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-07 15:06 [PATCH] KVM: x86: use X86_FEATURE_RSB_CTXSW for RSB stuffing in vmexit Jon Kohler
2021-05-07 17:22 ` Venkatesh Srinivas
2021-05-07 17:46 ` Jon Kohler
2021-05-07 17:58 ` Sean Christopherson
2021-05-07 18:26 ` Jon Kohler [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BDF4AF6B-DB1F-4BAF-999E-72CDDF408F37@nutanix.com \
--to=jon@nutanix.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=venkateshs@chromium.org \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).