| Message ID | 20230607004311.1420507-3-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Snapshot host MSR_IA32_ARCH_CAPABILITIES | expand |
On Tue, Jun 06, 2023 at 05:43:10PM -0700, Sean Christopherson wrote: > Now that KVM snapshots the host's MSR_IA32_ARCH_CAPABILITIES, drop the > similar snapshot/cache of whether or not KVM is allowed to manipulate > ARCH_CAPABILITIES.FB_CLEAR_CTRL. The motivation for the cache was FB_CLEAR_CTRL is a read-only bit, I think you mean MSR_IA32_MCU_OPT_CTRL.FB_CLEAR_DIS. > presumably to avoid the RDMSR, e.g. boot_cpu_has_bug() is quite cheap, and > modifying the vCPU's MSR_IA32_ARCH_CAPABILITIES is an infrequent option > and a relatively slow path. > > Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> LGTM. Reviewed-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
On 6/7/2023 8:43 AM, Sean Christopherson wrote: > Now that KVM snapshots the host's MSR_IA32_ARCH_CAPABILITIES, drop the > similar snapshot/cache of whether or not KVM is allowed to manipulate > ARCH_CAPABILITIES.FB_CLEAR_CTRL. The motivation for the cache was > presumably to avoid the RDMSR, e.g. boot_cpu_has_bug() is quite cheap, and > modifying the vCPU's MSR_IA32_ARCH_CAPABILITIES is an infrequent option > and a relatively slow path. Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> > Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/vmx/vmx.c | 17 +++-------------- > 1 file changed, 3 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 42d1148f933c..17003660138a 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -237,9 +237,6 @@ static const struct { > #define L1D_CACHE_ORDER 4 > static void *vmx_l1d_flush_pages; > > -/* Control for disabling CPU Fill buffer clear */ > -static bool __read_mostly vmx_fb_clear_ctrl_available; > - > static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) > { > struct page *page; > @@ -366,14 +363,6 @@ static int vmentry_l1d_flush_get(char *s, const struct kernel_param *kp) > return sprintf(s, "%s\n", vmentry_l1d_param[l1tf_vmx_mitigation].option); > } > > -static void vmx_setup_fb_clear_ctrl(void) > -{ > - if ((host_arch_capabilities & ARCH_CAP_FB_CLEAR_CTRL) && > - !boot_cpu_has_bug(X86_BUG_MDS) && > - !boot_cpu_has_bug(X86_BUG_TAA)) > - vmx_fb_clear_ctrl_available = true; > -} > - > static __always_inline void vmx_disable_fb_clear(struct vcpu_vmx *vmx) > { > u64 msr; > @@ -399,7 +388,9 @@ static __always_inline void vmx_enable_fb_clear(struct vcpu_vmx *vmx) > > static void vmx_update_fb_clear_dis(struct kvm_vcpu *vcpu, struct vcpu_vmx *vmx) > { > - vmx->disable_fb_clear = vmx_fb_clear_ctrl_available; > + vmx->disable_fb_clear = (host_arch_capabilities & ARCH_CAP_FB_CLEAR_CTRL) && > + !boot_cpu_has_bug(X86_BUG_MDS) && > + !boot_cpu_has_bug(X86_BUG_TAA); > > /* > * If guest will not execute VERW, there is no need to set FB_CLEAR_DIS > @@ -8580,8 +8571,6 @@ static int __init vmx_init(void) > if (r) > goto err_l1d_flush; > > - vmx_setup_fb_clear_ctrl(); > - > for_each_possible_cpu(cpu) { > INIT_LIST_HEAD(&per_cpu(loaded_vmcss_on_cpu, cpu)); >
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 42d1148f933c..17003660138a 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -237,9 +237,6 @@ static const struct { #define L1D_CACHE_ORDER 4 static void *vmx_l1d_flush_pages; -/* Control for disabling CPU Fill buffer clear */ -static bool __read_mostly vmx_fb_clear_ctrl_available; - static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) { struct page *page; @@ -366,14 +363,6 @@ static int vmentry_l1d_flush_get(char *s, const struct kernel_param *kp) return sprintf(s, "%s\n", vmentry_l1d_param[l1tf_vmx_mitigation].option); } -static void vmx_setup_fb_clear_ctrl(void) -{ - if ((host_arch_capabilities & ARCH_CAP_FB_CLEAR_CTRL) && - !boot_cpu_has_bug(X86_BUG_MDS) && - !boot_cpu_has_bug(X86_BUG_TAA)) - vmx_fb_clear_ctrl_available = true; -} - static __always_inline void vmx_disable_fb_clear(struct vcpu_vmx *vmx) { u64 msr; @@ -399,7 +388,9 @@ static __always_inline void vmx_enable_fb_clear(struct vcpu_vmx *vmx) static void vmx_update_fb_clear_dis(struct kvm_vcpu *vcpu, struct vcpu_vmx *vmx) { - vmx->disable_fb_clear = vmx_fb_clear_ctrl_available; + vmx->disable_fb_clear = (host_arch_capabilities & ARCH_CAP_FB_CLEAR_CTRL) && + !boot_cpu_has_bug(X86_BUG_MDS) && + !boot_cpu_has_bug(X86_BUG_TAA); /* * If guest will not execute VERW, there is no need to set FB_CLEAR_DIS @@ -8580,8 +8571,6 @@ static int __init vmx_init(void) if (r) goto err_l1d_flush; - vmx_setup_fb_clear_ctrl(); - for_each_possible_cpu(cpu) { INIT_LIST_HEAD(&per_cpu(loaded_vmcss_on_cpu, cpu));
Now that KVM snapshots the host's MSR_IA32_ARCH_CAPABILITIES, drop the similar snapshot/cache of whether or not KVM is allowed to manipulate ARCH_CAPABILITIES.FB_CLEAR_CTRL. The motivation for the cache was presumably to avoid the RDMSR, e.g. boot_cpu_has_bug() is quite cheap, and modifying the vCPU's MSR_IA32_ARCH_CAPABILITIES is an infrequent option and a relatively slow path. Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/vmx.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-)