On 5/23/2023 5:23 AM, Pawan Gupta wrote:
On Tue, May 23, 2023 at 03:31:44AM +0800, Xiaoyao Li wrote:
On 5/23/2023 1:43 AM, Sean Christopherson wrote:
6. Performance aside, KVM should not be speculating (ha!) on what the guest
will and will not do, and should instead honor whatever behavior is presented
to the guest. If the guest CPU model indicates that VERW flushes buffers,
then KVM damn well needs to let VERW flush buffers.
The current implementation allows guests to have VERW flush buffers when
they enumerate FB_CLEAR. It only restricts the flush behavior when the
guest is trying to mitigate against a vulnerability(like MDS) on a
hardware that is not affected. I guess its common for guests to be
running with older gen configuration on a newer hardware.
Right, I'm saying that that behavior is wrong. KVM shouldn't assume the guest
the guest will do things a certain way and should instead honor the "architectural"
definition, in quotes because I realize there probably is no architectural
definition for any of this.
It might be that the code does (unintentionally?) honor the "architecture", i.e.
this code might actually be accurrate with respect to when the guest can expect
VERW to flush buffers. But the comment is so, so wrong.
The comment is wrong and the code is wrong in some case as well.
If none of ARCH_CAP_FB_CLEAR, ARCH_CAP_MDS_NO, ARCH_CAP_TAA_NO,
ARCH_CAP_PSDP_NO, ARCH_CAP_FBSDP_NO and ARCH_CAP_SBDR_SSDP_NO are exposed to
VM, the VM is type of "affected by MDS".
And accroding to the page https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/processor-mmio-stale-data-vulnerabilities.html
if the VM enumerates support for both L1D_FLUSH and MD_CLEAR, it implicitly
enumerates FB_CLEAR as part of their MD_CLEAR support.
This is the excerpt from the link that you mentioned:
"For processors that are affected by MDS and support L1D_FLUSH
operations and MD_CLEAR operations, the VERW instruction flushes fill
buffers."
You are missing an important information here "For the processors
_affected_ by MDS". On such processors ...
However, the code will leave vmx->disable_fb_clear as 1 if hardware supports
it, and VERW intruction doesn't clear FB in the VM, which conflicts
"architectural" definition.
... Fill buffer clear is not enabled at all:
vmx_setup_fb_clear_ctrl()
{
u64 msr;
if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES) &&
!boot_cpu_has_bug(X86_BUG_MDS) &&
!boot_cpu_has_bug(X86_BUG_TAA)) {
rdmsrl(MSR_IA32_ARCH_CAPABILITIES, msr);
if (msr & ARCH_CAP_FB_CLEAR_CTRL)
vmx_fb_clear_ctrl_available = true;
}
}
This is the check of bare metal, while the check in
vmx_update_fb_clear_dis() is of guest VM.
For example, if the hardware (host) enumerates ARCH_CAP_TAA_NO,
ARCH_CAP_MDS_NO, ARCH_CAP_PSDP_NO, ARCH_CAP_FBSDP_NO,
ARCH_CAP_SBDR_SSDP_NO, ARCH_CAP_FB_CLEAR, and ARCH_CAP_FB_CLEAR_CTRL,
the VERW on this hardware clears Fill Buffer (if FB_CLEAR_DIS is not
enabled in MSR_IA32_MCU_OPT_CTRL). vmx_setup_fb_clear_ctrl() does set
vmx_fb_clear_ctrl_available to true.
If a guest is exposed without ARCH_CAP_TAA_NO, ARCH_CAP_MDS_NO,
ARCH_CAP_PSDP_NO, ARCH_CAP_FBSDP_NO, ARCH_CAP_SBDR_SSDP_NO and
ARCH_CAP_FB_CLEAR, vmx_update_fb_clear_dis() will leave
vmx->disable_fb_clear as true. So VERW doesn't clear Fill Buffer for
guest. But in the view of guset, it expects VERW to clear Fill Buffer.
