* [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups
@ 2023-05-24 6:16 Chao Gao
2023-05-24 6:16 ` [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps Chao Gao
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Chao Gao @ 2023-05-24 6:16 UTC (permalink / raw)
To: kvm, x86
Cc: xiaoyao.li, Chao Gao, Alexandre Chartre, Ashok Raj,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar,
Juergen Gross, Kim Phillips, linux-kernel, Paolo Bonzini,
Pawan Gupta, Peter Zijlstra, Sean Christopherson, Thomas Gleixner
Do some cleanups about MSR_IA32_ARCH_CAPABILITIES in KVM e.g, fix stale
comments, elimite open-coded MSR reads, track features that can be exposed to
guests to avoid computing them runtime. They are found when reading the code.
No functional change intented.
Changes from v1 -> v2:
1. toggle ARCH_CAP_SKIP_VMENTRY_L1DFLUSH bit when l1tf mitigation state is
changed by module param. [Xiaoyao, Sean]
2. add more cleanups (patch 2/3)
Chao Gao (3):
KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps
KVM: x86: Correct the name for skipping VMENTER l1d flush
x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES
arch/x86/kernel/cpu/common.c | 1 +
arch/x86/kvm/vmx/vmx.c | 44 +++++++++++++++++++++++-------------
arch/x86/kvm/x86.c | 16 +++++--------
arch/x86/kvm/x86.h | 1 +
4 files changed, 36 insertions(+), 26 deletions(-)
base-commit: b9846a698c9aff4eb2214a06ac83638ad098f33f
--
2.40.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps 2023-05-24 6:16 [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Chao Gao @ 2023-05-24 6:16 ` Chao Gao 2023-05-24 8:14 ` Xiaoyao Li 2023-06-06 16:40 ` Sean Christopherson 2023-05-24 6:16 ` [PATCH v2 2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush Chao Gao ` (2 subsequent siblings) 3 siblings, 2 replies; 11+ messages in thread From: Chao Gao @ 2023-05-24 6:16 UTC (permalink / raw) To: kvm, x86 Cc: xiaoyao.li, Chao Gao, Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel to avoid computing the supported value at runtime every time. Toggle the ARCH_CAP_SKIP_VMENTRY_L1DFLUSH bit when l1tf_vmx_mitigation is modified to achieve the same result as runtime computing. Opportunistically, add a comment to document the problem of allowing changing the supported value of ARCH_CAPABILITIES and the reason why we don't fix it. No functional change intended. Link: https://lore.kernel.org/all/ZGZhW%2Fx5OWPmx1qD@google.com/ Link: https://lore.kernel.org/all/ZGeU9sYTPxqNGSqI@google.com/ Signed-off-by: Chao Gao <chao.gao@intel.com> --- arch/x86/kvm/vmx/vmx.c | 25 +++++++++++++++++++++++-- arch/x86/kvm/x86.c | 7 ++++--- arch/x86/kvm/x86.h | 1 + 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 44fb619803b8..8274ef5e89e5 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -309,10 +309,31 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) l1tf_vmx_mitigation = l1tf; - if (l1tf != VMENTER_L1D_FLUSH_NEVER) + /* + * Update static keys and supported arch capabilities according to + * the new mitigation state. + * + * ARCH_CAP_SKIP_VMENTRY_L1DFLUSH is toggled because if we do cache + * flushes for L1 guests on (nested) vmlaunch/vmresume to L2, L1 + * guests can skip the flush and if we don't, then L1 guests need + * to do a flush. + * + * Toggling ARCH_CAP_SKIP_VMENTRY_L1DFLUSH may present inconsistent + * model to the guest, e.g., if userspace isn't careful, a VM can + * have vCPUs with different values for ARCH_CAPABILITIES. But + * there is almost no chance to fix the issue. Because, to present + * a consistent model, KVM essentially needs to disallow changing + * the module param after VMs/vCPUs have been created, but that + * would prevent userspace from toggling the param while VMs are + * running, e.g., in response to a new vulnerability. + */ + if (l1tf != VMENTER_L1D_FLUSH_NEVER) { static_branch_enable(&vmx_l1d_should_flush); - else + kvm_caps.supported_arch_cap |= ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; + } else { static_branch_disable(&vmx_l1d_should_flush); + kvm_caps.supported_arch_cap &= ~ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; + } if (l1tf == VMENTER_L1D_FLUSH_COND) static_branch_enable(&vmx_l1d_flush_cond); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c0778ca39650..2408b5f554b7 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1672,7 +1672,7 @@ static int kvm_get_msr_feature(struct kvm_msr_entry *msr) { switch (msr->index) { case MSR_IA32_ARCH_CAPABILITIES: - msr->data = kvm_get_arch_capabilities(); + msr->data = kvm_caps.supported_arch_cap; break; case MSR_IA32_PERF_CAPABILITIES: msr->data = kvm_caps.supported_perf_cap; @@ -7156,7 +7156,7 @@ static void kvm_probe_msr_to_save(u32 msr_index) return; break; case MSR_IA32_TSX_CTRL: - if (!(kvm_get_arch_capabilities() & ARCH_CAP_TSX_CTRL_MSR)) + if (!(kvm_caps.supported_arch_cap & ARCH_CAP_TSX_CTRL_MSR)) return; break; default: @@ -9532,6 +9532,7 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) kvm_caps.max_guest_tsc_khz = max; } kvm_caps.default_tsc_scaling_ratio = 1ULL << kvm_caps.tsc_scaling_ratio_frac_bits; + kvm_caps.supported_arch_cap = kvm_get_arch_capabilities(); kvm_init_msr_lists(); return 0; @@ -11895,7 +11896,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) if (r) goto free_guest_fpu; - vcpu->arch.arch_capabilities = kvm_get_arch_capabilities(); + vcpu->arch.arch_capabilities = kvm_caps.supported_arch_cap; vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT; kvm_xen_init_vcpu(vcpu); kvm_vcpu_mtrr_init(vcpu); diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index c544602d07a3..d3e524bcc169 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -29,6 +29,7 @@ struct kvm_caps { u64 supported_xcr0; u64 supported_xss; u64 supported_perf_cap; + u64 supported_arch_cap; }; void kvm_spurious_fault(void); -- 2.40.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps 2023-05-24 6:16 ` [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps Chao Gao @ 2023-05-24 8:14 ` Xiaoyao Li 2023-05-24 8:32 ` Chao Gao 2023-06-06 16:40 ` Sean Christopherson 1 sibling, 1 reply; 11+ messages in thread From: Xiaoyao Li @ 2023-05-24 8:14 UTC (permalink / raw) To: Chao Gao, kvm, x86 Cc: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel On 5/24/2023 2:16 PM, Chao Gao wrote: > to avoid computing the supported value at runtime every time. > > Toggle the ARCH_CAP_SKIP_VMENTRY_L1DFLUSH bit when l1tf_vmx_mitigation > is modified to achieve the same result as runtime computing. It's not the same result. In kvm_get_arch_capabilities(), host's value is honored. I.e., when host supports ARCH_CAP_SKIP_VMENTRY_L1DFLUSH, l1tf_vmx_mitigation doesn't make any difference to the result. > Opportunistically, add a comment to document the problem of allowing > changing the supported value of ARCH_CAPABILITIES and the reason why > we don't fix it. > > No functional change intended. > > Link: https://lore.kernel.org/all/ZGZhW%2Fx5OWPmx1qD@google.com/ > Link: https://lore.kernel.org/all/ZGeU9sYTPxqNGSqI@google.com/ > Signed-off-by: Chao Gao <chao.gao@intel.com> > --- > arch/x86/kvm/vmx/vmx.c | 25 +++++++++++++++++++++++-- > arch/x86/kvm/x86.c | 7 ++++--- > arch/x86/kvm/x86.h | 1 + > 3 files changed, 28 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 44fb619803b8..8274ef5e89e5 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -309,10 +309,31 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) > > l1tf_vmx_mitigation = l1tf; > > - if (l1tf != VMENTER_L1D_FLUSH_NEVER) > + /* > + * Update static keys and supported arch capabilities according to > + * the new mitigation state. > + * > + * ARCH_CAP_SKIP_VMENTRY_L1DFLUSH is toggled because if we do cache > + * flushes for L1 guests on (nested) vmlaunch/vmresume to L2, L1 > + * guests can skip the flush and if we don't, then L1 guests need > + * to do a flush. > + * > + * Toggling ARCH_CAP_SKIP_VMENTRY_L1DFLUSH may present inconsistent > + * model to the guest, e.g., if userspace isn't careful, a VM can > + * have vCPUs with different values for ARCH_CAPABILITIES. But > + * there is almost no chance to fix the issue. Because, to present > + * a consistent model, KVM essentially needs to disallow changing > + * the module param after VMs/vCPUs have been created, but that > + * would prevent userspace from toggling the param while VMs are > + * running, e.g., in response to a new vulnerability. > + */ > + if (l1tf != VMENTER_L1D_FLUSH_NEVER) { > static_branch_enable(&vmx_l1d_should_flush); > - else > + kvm_caps.supported_arch_cap |= ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; > + } else { > static_branch_disable(&vmx_l1d_should_flush); > + kvm_caps.supported_arch_cap &= ~ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; > + } > > if (l1tf == VMENTER_L1D_FLUSH_COND) > static_branch_enable(&vmx_l1d_flush_cond); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c0778ca39650..2408b5f554b7 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1672,7 +1672,7 @@ static int kvm_get_msr_feature(struct kvm_msr_entry *msr) > { > switch (msr->index) { > case MSR_IA32_ARCH_CAPABILITIES: > - msr->data = kvm_get_arch_capabilities(); > + msr->data = kvm_caps.supported_arch_cap; > break; > case MSR_IA32_PERF_CAPABILITIES: > msr->data = kvm_caps.supported_perf_cap; > @@ -7156,7 +7156,7 @@ static void kvm_probe_msr_to_save(u32 msr_index) > return; > break; > case MSR_IA32_TSX_CTRL: > - if (!(kvm_get_arch_capabilities() & ARCH_CAP_TSX_CTRL_MSR)) > + if (!(kvm_caps.supported_arch_cap & ARCH_CAP_TSX_CTRL_MSR)) > return; > break; > default: > @@ -9532,6 +9532,7 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) > kvm_caps.max_guest_tsc_khz = max; > } > kvm_caps.default_tsc_scaling_ratio = 1ULL << kvm_caps.tsc_scaling_ratio_frac_bits; > + kvm_caps.supported_arch_cap = kvm_get_arch_capabilities(); > kvm_init_msr_lists(); > return 0; > > @@ -11895,7 +11896,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > if (r) > goto free_guest_fpu; > > - vcpu->arch.arch_capabilities = kvm_get_arch_capabilities(); > + vcpu->arch.arch_capabilities = kvm_caps.supported_arch_cap; > vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT; > kvm_xen_init_vcpu(vcpu); > kvm_vcpu_mtrr_init(vcpu); > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index c544602d07a3..d3e524bcc169 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -29,6 +29,7 @@ struct kvm_caps { > u64 supported_xcr0; > u64 supported_xss; > u64 supported_perf_cap; > + u64 supported_arch_cap; > }; > > void kvm_spurious_fault(void); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps 2023-05-24 8:14 ` Xiaoyao Li @ 2023-05-24 8:32 ` Chao Gao 2023-05-25 2:17 ` Xiaoyao Li 0 siblings, 1 reply; 11+ messages in thread From: Chao Gao @ 2023-05-24 8:32 UTC (permalink / raw) To: Xiaoyao Li Cc: kvm, x86, Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel On Wed, May 24, 2023 at 04:14:10PM +0800, Xiaoyao Li wrote: >On 5/24/2023 2:16 PM, Chao Gao wrote: >> to avoid computing the supported value at runtime every time. >> >> Toggle the ARCH_CAP_SKIP_VMENTRY_L1DFLUSH bit when l1tf_vmx_mitigation >> is modified to achieve the same result as runtime computing. > >It's not the same result. it is because ... > >In kvm_get_arch_capabilities(), host's value is honored. I.e., when host >supports ARCH_CAP_SKIP_VMENTRY_L1DFLUSH, l1tf_vmx_mitigation doesn't make any >difference to the result. ... l1tf_vmx_mitigation should be VMENTER_L1D_FLUSH_NOT_REQUIRED in this case. l1tf_vmx_mitigation cannot be VMENTER_L1D_FLUSH_NEVER. > >> Opportunistically, add a comment to document the problem of allowing >> changing the supported value of ARCH_CAPABILITIES and the reason why >> we don't fix it. >> >> No functional change intended. >> >> Link: https://lore.kernel.org/all/ZGZhW%2Fx5OWPmx1qD@google.com/ >> Link: https://lore.kernel.org/all/ZGeU9sYTPxqNGSqI@google.com/ >> Signed-off-by: Chao Gao <chao.gao@intel.com> >> --- >> arch/x86/kvm/vmx/vmx.c | 25 +++++++++++++++++++++++-- >> arch/x86/kvm/x86.c | 7 ++++--- >> arch/x86/kvm/x86.h | 1 + >> 3 files changed, 28 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >> index 44fb619803b8..8274ef5e89e5 100644 >> --- a/arch/x86/kvm/vmx/vmx.c >> +++ b/arch/x86/kvm/vmx/vmx.c >> @@ -309,10 +309,31 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) >> l1tf_vmx_mitigation = l1tf; >> - if (l1tf != VMENTER_L1D_FLUSH_NEVER) >> + /* >> + * Update static keys and supported arch capabilities according to >> + * the new mitigation state. >> + * >> + * ARCH_CAP_SKIP_VMENTRY_L1DFLUSH is toggled because if we do cache >> + * flushes for L1 guests on (nested) vmlaunch/vmresume to L2, L1 >> + * guests can skip the flush and if we don't, then L1 guests need >> + * to do a flush. >> + * >> + * Toggling ARCH_CAP_SKIP_VMENTRY_L1DFLUSH may present inconsistent >> + * model to the guest, e.g., if userspace isn't careful, a VM can >> + * have vCPUs with different values for ARCH_CAPABILITIES. But >> + * there is almost no chance to fix the issue. Because, to present >> + * a consistent model, KVM essentially needs to disallow changing >> + * the module param after VMs/vCPUs have been created, but that >> + * would prevent userspace from toggling the param while VMs are >> + * running, e.g., in response to a new vulnerability. >> + */ >> + if (l1tf != VMENTER_L1D_FLUSH_NEVER) { >> static_branch_enable(&vmx_l1d_should_flush); >> - else >> + kvm_caps.supported_arch_cap |= ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; >> + } else { >> static_branch_disable(&vmx_l1d_should_flush); >> + kvm_caps.supported_arch_cap &= ~ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; >> + } >> if (l1tf == VMENTER_L1D_FLUSH_COND) >> static_branch_enable(&vmx_l1d_flush_cond); >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index c0778ca39650..2408b5f554b7 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -1672,7 +1672,7 @@ static int kvm_get_msr_feature(struct kvm_msr_entry *msr) >> { >> switch (msr->index) { >> case MSR_IA32_ARCH_CAPABILITIES: >> - msr->data = kvm_get_arch_capabilities(); >> + msr->data = kvm_caps.supported_arch_cap; >> break; >> case MSR_IA32_PERF_CAPABILITIES: >> msr->data = kvm_caps.supported_perf_cap; >> @@ -7156,7 +7156,7 @@ static void kvm_probe_msr_to_save(u32 msr_index) >> return; >> break; >> case MSR_IA32_TSX_CTRL: >> - if (!(kvm_get_arch_capabilities() & ARCH_CAP_TSX_CTRL_MSR)) >> + if (!(kvm_caps.supported_arch_cap & ARCH_CAP_TSX_CTRL_MSR)) >> return; >> break; >> default: >> @@ -9532,6 +9532,7 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) >> kvm_caps.max_guest_tsc_khz = max; >> } >> kvm_caps.default_tsc_scaling_ratio = 1ULL << kvm_caps.tsc_scaling_ratio_frac_bits; >> + kvm_caps.supported_arch_cap = kvm_get_arch_capabilities(); >> kvm_init_msr_lists(); >> return 0; >> @@ -11895,7 +11896,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) >> if (r) >> goto free_guest_fpu; >> - vcpu->arch.arch_capabilities = kvm_get_arch_capabilities(); >> + vcpu->arch.arch_capabilities = kvm_caps.supported_arch_cap; >> vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT; >> kvm_xen_init_vcpu(vcpu); >> kvm_vcpu_mtrr_init(vcpu); >> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h >> index c544602d07a3..d3e524bcc169 100644 >> --- a/arch/x86/kvm/x86.h >> +++ b/arch/x86/kvm/x86.h >> @@ -29,6 +29,7 @@ struct kvm_caps { >> u64 supported_xcr0; >> u64 supported_xss; >> u64 supported_perf_cap; >> + u64 supported_arch_cap; >> }; >> void kvm_spurious_fault(void); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps 2023-05-24 8:32 ` Chao Gao @ 2023-05-25 2:17 ` Xiaoyao Li 0 siblings, 0 replies; 11+ messages in thread From: Xiaoyao Li @ 2023-05-25 2:17 UTC (permalink / raw) To: Chao Gao Cc: kvm, x86, Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel On 5/24/2023 4:32 PM, Chao Gao wrote: > On Wed, May 24, 2023 at 04:14:10PM +0800, Xiaoyao Li wrote: >> On 5/24/2023 2:16 PM, Chao Gao wrote: >>> to avoid computing the supported value at runtime every time. >>> >>> Toggle the ARCH_CAP_SKIP_VMENTRY_L1DFLUSH bit when l1tf_vmx_mitigation >>> is modified to achieve the same result as runtime computing. >> >> It's not the same result. > > it is because ... > >> >> In kvm_get_arch_capabilities(), host's value is honored. I.e., when host >> supports ARCH_CAP_SKIP_VMENTRY_L1DFLUSH, l1tf_vmx_mitigation doesn't make any >> difference to the result. > > ... l1tf_vmx_mitigation should be VMENTER_L1D_FLUSH_NOT_REQUIRED in this > case. l1tf_vmx_mitigation cannot be VMENTER_L1D_FLUSH_NEVER. yes. you are right. Maybe we can clarify it in the changelog anyway, Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> >> >>> Opportunistically, add a comment to document the problem of allowing >>> changing the supported value of ARCH_CAPABILITIES and the reason why >>> we don't fix it. >>> >>> No functional change intended. >>> >>> Link: https://lore.kernel.org/all/ZGZhW%2Fx5OWPmx1qD@google.com/ >>> Link: https://lore.kernel.org/all/ZGeU9sYTPxqNGSqI@google.com/ >>> Signed-off-by: Chao Gao <chao.gao@intel.com> >>> --- >>> arch/x86/kvm/vmx/vmx.c | 25 +++++++++++++++++++++++-- >>> arch/x86/kvm/x86.c | 7 ++++--- >>> arch/x86/kvm/x86.h | 1 + >>> 3 files changed, 28 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >>> index 44fb619803b8..8274ef5e89e5 100644 >>> --- a/arch/x86/kvm/vmx/vmx.c >>> +++ b/arch/x86/kvm/vmx/vmx.c >>> @@ -309,10 +309,31 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) >>> l1tf_vmx_mitigation = l1tf; >>> - if (l1tf != VMENTER_L1D_FLUSH_NEVER) >>> + /* >>> + * Update static keys and supported arch capabilities according to >>> + * the new mitigation state. >>> + * >>> + * ARCH_CAP_SKIP_VMENTRY_L1DFLUSH is toggled because if we do cache >>> + * flushes for L1 guests on (nested) vmlaunch/vmresume to L2, L1 >>> + * guests can skip the flush and if we don't, then L1 guests need >>> + * to do a flush. >>> + * >>> + * Toggling ARCH_CAP_SKIP_VMENTRY_L1DFLUSH may present inconsistent >>> + * model to the guest, e.g., if userspace isn't careful, a VM can >>> + * have vCPUs with different values for ARCH_CAPABILITIES. But >>> + * there is almost no chance to fix the issue. Because, to present >>> + * a consistent model, KVM essentially needs to disallow changing >>> + * the module param after VMs/vCPUs have been created, but that >>> + * would prevent userspace from toggling the param while VMs are >>> + * running, e.g., in response to a new vulnerability. >>> + */ >>> + if (l1tf != VMENTER_L1D_FLUSH_NEVER) { >>> static_branch_enable(&vmx_l1d_should_flush); >>> - else >>> + kvm_caps.supported_arch_cap |= ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; >>> + } else { >>> static_branch_disable(&vmx_l1d_should_flush); >>> + kvm_caps.supported_arch_cap &= ~ARCH_CAP_SKIP_VMENTRY_L1DFLUSH; >>> + } >>> if (l1tf == VMENTER_L1D_FLUSH_COND) >>> static_branch_enable(&vmx_l1d_flush_cond); >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index c0778ca39650..2408b5f554b7 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -1672,7 +1672,7 @@ static int kvm_get_msr_feature(struct kvm_msr_entry *msr) >>> { >>> switch (msr->index) { >>> case MSR_IA32_ARCH_CAPABILITIES: >>> - msr->data = kvm_get_arch_capabilities(); >>> + msr->data = kvm_caps.supported_arch_cap; >>> break; >>> case MSR_IA32_PERF_CAPABILITIES: >>> msr->data = kvm_caps.supported_perf_cap; >>> @@ -7156,7 +7156,7 @@ static void kvm_probe_msr_to_save(u32 msr_index) >>> return; >>> break; >>> case MSR_IA32_TSX_CTRL: >>> - if (!(kvm_get_arch_capabilities() & ARCH_CAP_TSX_CTRL_MSR)) >>> + if (!(kvm_caps.supported_arch_cap & ARCH_CAP_TSX_CTRL_MSR)) >>> return; >>> break; >>> default: >>> @@ -9532,6 +9532,7 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) >>> kvm_caps.max_guest_tsc_khz = max; >>> } >>> kvm_caps.default_tsc_scaling_ratio = 1ULL << kvm_caps.tsc_scaling_ratio_frac_bits; >>> + kvm_caps.supported_arch_cap = kvm_get_arch_capabilities(); >>> kvm_init_msr_lists(); >>> return 0; >>> @@ -11895,7 +11896,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) >>> if (r) >>> goto free_guest_fpu; >>> - vcpu->arch.arch_capabilities = kvm_get_arch_capabilities(); >>> + vcpu->arch.arch_capabilities = kvm_caps.supported_arch_cap; >>> vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT; >>> kvm_xen_init_vcpu(vcpu); >>> kvm_vcpu_mtrr_init(vcpu); >>> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h >>> index c544602d07a3..d3e524bcc169 100644 >>> --- a/arch/x86/kvm/x86.h >>> +++ b/arch/x86/kvm/x86.h >>> @@ -29,6 +29,7 @@ struct kvm_caps { >>> u64 supported_xcr0; >>> u64 supported_xss; >>> u64 supported_perf_cap; >>> + u64 supported_arch_cap; >>> }; >>> void kvm_spurious_fault(void); >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps 2023-05-24 6:16 ` [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps Chao Gao 2023-05-24 8:14 ` Xiaoyao Li @ 2023-06-06 16:40 ` Sean Christopherson 1 sibling, 0 replies; 11+ messages in thread From: Sean Christopherson @ 2023-06-06 16:40 UTC (permalink / raw) To: Chao Gao Cc: kvm, x86, xiaoyao.li, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel On Wed, May 24, 2023, Chao Gao wrote: > @@ -9532,6 +9532,7 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) > kvm_caps.max_guest_tsc_khz = max; > } > kvm_caps.default_tsc_scaling_ratio = 1ULL << kvm_caps.tsc_scaling_ratio_frac_bits; > + kvm_caps.supported_arch_cap = kvm_get_arch_capabilities(); > kvm_init_msr_lists(); > return 0; > > @@ -11895,7 +11896,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > if (r) > goto free_guest_fpu; > > - vcpu->arch.arch_capabilities = kvm_get_arch_capabilities(); > + vcpu->arch.arch_capabilities = kvm_caps.supported_arch_cap; > vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT; > kvm_xen_init_vcpu(vcpu); > kvm_vcpu_mtrr_init(vcpu); > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index c544602d07a3..d3e524bcc169 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -29,6 +29,7 @@ struct kvm_caps { > u64 supported_xcr0; > u64 supported_xss; > u64 supported_perf_cap; > + u64 supported_arch_cap; Hrm, I take back my earlier vote about using a dynamic snapshot. "supported" isn't quite right. KVM always "supports" advertising SKIP_VMENTRY_L1DFLUSH to the guest. And KVM really does treat the MSR like a CPUID leaf, in that KVM doesn't sanity check the value coming in from userspace. Whether or not that's a good idea is debatable, but it is was it is. The value is more like KVM's current default. Looking at all the uses of both the default/supported value, and the host MSR, I think it makes more sense to snapshot the host value than it does to snapshot and update the default/supported value. The default value is used only when a vCPU is created and when userspace does a system-scoped KVM_GET_MSRS, i.e. avoiding the RDMSR is nice, but making the read super fast isn't necessary, e.g. the overhead of the boot_cpu_has() and boot_cpu_has_bug() checks is negligible. And if KVM snapshots the MSR, the other usage of the host value can be cleaned up too. I'm leaning towards doing this instead of patches [1/3] and [3/3]: From: Sean Christopherson <seanjc@google.com> Date: Tue, 6 Jun 2023 09:20:31 -0700 Subject: [PATCH 1/2] KVM: x86: Snapshot host's MSR_IA32_ARCH_CAPABILITIES Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/vmx.c | 22 ++++++---------------- arch/x86/kvm/x86.c | 13 +++++++------ arch/x86/kvm/x86.h | 1 + 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 2d9d155691a7..42d1148f933c 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -255,14 +255,9 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) return 0; } - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { - u64 msr; - - rdmsrl(MSR_IA32_ARCH_CAPABILITIES, msr); - if (msr & ARCH_CAP_SKIP_VMENTRY_L1DFLUSH) { - l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED; - return 0; - } + if (host_arch_capabilities & ARCH_CAP_SKIP_VMENTRY_L1DFLUSH) { + l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED; + return 0; } /* If set to auto use the default l1tf mitigation method */ @@ -373,15 +368,10 @@ static int vmentry_l1d_flush_get(char *s, const struct kernel_param *kp) static void vmx_setup_fb_clear_ctrl(void) { - u64 msr; - - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES) && + if ((host_arch_capabilities & ARCH_CAP_FB_CLEAR_CTRL) && !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; - } + !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) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7c7be4815eaa..7c2e796fa460 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -237,6 +237,9 @@ EXPORT_SYMBOL_GPL(enable_apicv); u64 __read_mostly host_xss; EXPORT_SYMBOL_GPL(host_xss); +u64 __read_mostly host_arch_capabilities; +EXPORT_SYMBOL_GPL(host_arch_capabilities); + const struct _kvm_stats_desc kvm_vm_stats_desc[] = { KVM_GENERIC_VM_STATS(), STATS_DESC_COUNTER(VM, mmu_shadow_zapped), @@ -1612,12 +1615,7 @@ static bool kvm_is_immutable_feature_msr(u32 msr) static u64 kvm_get_arch_capabilities(void) { - u64 data = 0; - - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { - rdmsrl(MSR_IA32_ARCH_CAPABILITIES, data); - data &= KVM_SUPPORTED_ARCH_CAP; - } + u64 data = host_arch_capabilities & KVM_SUPPORTED_ARCH_CAP; /* * If nx_huge_pages is enabled, KVM's shadow paging will ensure that @@ -9492,6 +9490,9 @@ static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) kvm_init_pmu_capability(ops->pmu_ops); + if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) + rdmsrl(MSR_IA32_ARCH_CAPABILITIES, host_arch_capabilities); + r = ops->hardware_setup(); if (r != 0) goto out_mmu_exit; diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 82e3dafc5453..1e7be1f6ab29 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -323,6 +323,7 @@ fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vcpu *vcpu); extern u64 host_xcr0; extern u64 host_xss; +extern u64 host_arch_capabilities; extern struct kvm_caps kvm_caps; base-commit: 02f1b0b736606f9870595b3089d9c124f9da8be9 -- 2.41.0.rc2.161.g9c6817b8e7-goog From bd4b41fea5d5fa0347c5ef1ffb73dbdf783b23d0 Mon Sep 17 00:00:00 2001 From: Sean Christopherson <seanjc@google.com> Date: Tue, 6 Jun 2023 09:25:55 -0700 Subject: [PATCH 2/2] KVM: VMX: Drop unnecessary vmx_fb_clear_ctrl_available "cache" 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)); -- ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush 2023-05-24 6:16 [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Chao Gao 2023-05-24 6:16 ` [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps Chao Gao @ 2023-05-24 6:16 ` Chao Gao 2023-05-24 8:25 ` Xiaoyao Li 2023-05-24 6:16 ` [PATCH v2 3/3] x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES Chao Gao 2023-06-06 17:26 ` [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Sean Christopherson 3 siblings, 1 reply; 11+ messages in thread From: Chao Gao @ 2023-05-24 6:16 UTC (permalink / raw) To: kvm, x86 Cc: xiaoyao.li, Chao Gao, Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel There is no VMENTER_L1D_FLUSH_NESTED_VM. It should be ARCH_CAP_SKIP_VMENTRY_L1DFLUSH. Signed-off-by: Chao Gao <chao.gao@intel.com> --- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2408b5f554b7..f7838260c183 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1631,7 +1631,7 @@ static u64 kvm_get_arch_capabilities(void) * If we're doing cache flushes (either "always" or "cond") * we will do one whenever the guest does a vmlaunch/vmresume. * If an outer hypervisor is doing the cache flush for us - * (VMENTER_L1D_FLUSH_NESTED_VM), we can safely pass that + * (ARCH_CAP_SKIP_VMENTRY_L1DFLUSH), we can safely pass that * capability to the guest too, and if EPT is disabled we're not * vulnerable. Overall, only VMENTER_L1D_FLUSH_NEVER will * require a nested hypervisor to do a flush of its own. -- 2.40.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush 2023-05-24 6:16 ` [PATCH v2 2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush Chao Gao @ 2023-05-24 8:25 ` Xiaoyao Li 0 siblings, 0 replies; 11+ messages in thread From: Xiaoyao Li @ 2023-05-24 8:25 UTC (permalink / raw) To: Chao Gao, kvm, x86 Cc: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, linux-kernel On 5/24/2023 2:16 PM, Chao Gao wrote: > There is no VMENTER_L1D_FLUSH_NESTED_VM. It should be > ARCH_CAP_SKIP_VMENTRY_L1DFLUSH. Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> > Signed-off-by: Chao Gao <chao.gao@intel.com> > --- > arch/x86/kvm/x86.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2408b5f554b7..f7838260c183 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1631,7 +1631,7 @@ static u64 kvm_get_arch_capabilities(void) > * If we're doing cache flushes (either "always" or "cond") > * we will do one whenever the guest does a vmlaunch/vmresume. > * If an outer hypervisor is doing the cache flush for us > - * (VMENTER_L1D_FLUSH_NESTED_VM), we can safely pass that > + * (ARCH_CAP_SKIP_VMENTRY_L1DFLUSH), we can safely pass that > * capability to the guest too, and if EPT is disabled we're not > * vulnerable. Overall, only VMENTER_L1D_FLUSH_NEVER will > * require a nested hypervisor to do a flush of its own. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES 2023-05-24 6:16 [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Chao Gao 2023-05-24 6:16 ` [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps Chao Gao 2023-05-24 6:16 ` [PATCH v2 2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush Chao Gao @ 2023-05-24 6:16 ` Chao Gao 2023-05-24 8:19 ` Xiaoyao Li 2023-06-06 17:26 ` [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Sean Christopherson 3 siblings, 1 reply; 11+ messages in thread From: Chao Gao @ 2023-05-24 6:16 UTC (permalink / raw) To: kvm, x86 Cc: xiaoyao.li, Chao Gao, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, Sean Christopherson, Paolo Bonzini, Peter Zijlstra, Kim Phillips, Juergen Gross, Ashok Raj, Pawan Gupta, Alexandre Chartre, linux-kernel KVM open-codes x86_read_arch_cap_msr() in a few places. Eliminate them by exposing the helper function and using it directly. No functional change intended. Signed-off-by: Chao Gao <chao.gao@intel.com> --- arch/x86/kernel/cpu/common.c | 1 + arch/x86/kvm/vmx/vmx.c | 19 +++++-------------- arch/x86/kvm/x86.c | 7 +------ 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 80710a68ef7d..b34dd3f3e6c4 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1315,6 +1315,7 @@ u64 x86_read_arch_cap_msr(void) return ia32_cap; } +EXPORT_SYMBOL_GPL(x86_read_arch_cap_msr); static bool arch_cap_mmio_immune(u64 ia32_cap) { diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 8274ef5e89e5..3dec4a4f637e 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -255,14 +255,9 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) return 0; } - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { - u64 msr; - - rdmsrl(MSR_IA32_ARCH_CAPABILITIES, msr); - if (msr & ARCH_CAP_SKIP_VMENTRY_L1DFLUSH) { - l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED; - return 0; - } + if (x86_read_arch_cap_msr() & ARCH_CAP_SKIP_VMENTRY_L1DFLUSH) { + l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED; + return 0; } /* If set to auto use the default l1tf mitigation method */ @@ -394,13 +389,9 @@ static int vmentry_l1d_flush_get(char *s, const struct kernel_param *kp) static void vmx_setup_fb_clear_ctrl(void) { - u64 msr; - - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES) && - !boot_cpu_has_bug(X86_BUG_MDS) && + if (!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) + if (x86_read_arch_cap_msr() & ARCH_CAP_FB_CLEAR_CTRL) vmx_fb_clear_ctrl_available = true; } } diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index f7838260c183..b1bdb84ac10f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1612,12 +1612,7 @@ static bool kvm_is_immutable_feature_msr(u32 msr) static u64 kvm_get_arch_capabilities(void) { - u64 data = 0; - - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { - rdmsrl(MSR_IA32_ARCH_CAPABILITIES, data); - data &= KVM_SUPPORTED_ARCH_CAP; - } + u64 data = x86_read_arch_cap_msr() & KVM_SUPPORTED_ARCH_CAP; /* * If nx_huge_pages is enabled, KVM's shadow paging will ensure that -- 2.40.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/3] x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES 2023-05-24 6:16 ` [PATCH v2 3/3] x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES Chao Gao @ 2023-05-24 8:19 ` Xiaoyao Li 0 siblings, 0 replies; 11+ messages in thread From: Xiaoyao Li @ 2023-05-24 8:19 UTC (permalink / raw) To: Chao Gao, kvm, x86 Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, Sean Christopherson, Paolo Bonzini, Peter Zijlstra, Kim Phillips, Juergen Gross, Ashok Raj, Pawan Gupta, Alexandre Chartre, linux-kernel On 5/24/2023 2:16 PM, Chao Gao wrote: > KVM open-codes x86_read_arch_cap_msr() in a few places. Eliminate them > by exposing the helper function and using it directly. > > No functional change intended. > Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> > Signed-off-by: Chao Gao <chao.gao@intel.com> > --- > arch/x86/kernel/cpu/common.c | 1 + > arch/x86/kvm/vmx/vmx.c | 19 +++++-------------- > arch/x86/kvm/x86.c | 7 +------ > 3 files changed, 7 insertions(+), 20 deletions(-) > > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c > index 80710a68ef7d..b34dd3f3e6c4 100644 > --- a/arch/x86/kernel/cpu/common.c > +++ b/arch/x86/kernel/cpu/common.c > @@ -1315,6 +1315,7 @@ u64 x86_read_arch_cap_msr(void) > > return ia32_cap; > } > +EXPORT_SYMBOL_GPL(x86_read_arch_cap_msr); > > static bool arch_cap_mmio_immune(u64 ia32_cap) > { > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 8274ef5e89e5..3dec4a4f637e 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -255,14 +255,9 @@ static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf) > return 0; > } > > - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { > - u64 msr; > - > - rdmsrl(MSR_IA32_ARCH_CAPABILITIES, msr); > - if (msr & ARCH_CAP_SKIP_VMENTRY_L1DFLUSH) { > - l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED; > - return 0; > - } > + if (x86_read_arch_cap_msr() & ARCH_CAP_SKIP_VMENTRY_L1DFLUSH) { > + l1tf_vmx_mitigation = VMENTER_L1D_FLUSH_NOT_REQUIRED; > + return 0; > } > > /* If set to auto use the default l1tf mitigation method */ > @@ -394,13 +389,9 @@ static int vmentry_l1d_flush_get(char *s, const struct kernel_param *kp) > > static void vmx_setup_fb_clear_ctrl(void) > { > - u64 msr; > - > - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES) && > - !boot_cpu_has_bug(X86_BUG_MDS) && > + if (!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) > + if (x86_read_arch_cap_msr() & ARCH_CAP_FB_CLEAR_CTRL) > vmx_fb_clear_ctrl_available = true; > } > } > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index f7838260c183..b1bdb84ac10f 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1612,12 +1612,7 @@ static bool kvm_is_immutable_feature_msr(u32 msr) > > static u64 kvm_get_arch_capabilities(void) > { > - u64 data = 0; > - > - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { > - rdmsrl(MSR_IA32_ARCH_CAPABILITIES, data); > - data &= KVM_SUPPORTED_ARCH_CAP; > - } > + u64 data = x86_read_arch_cap_msr() & KVM_SUPPORTED_ARCH_CAP; > > /* > * If nx_huge_pages is enabled, KVM's shadow paging will ensure that ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups 2023-05-24 6:16 [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Chao Gao ` (2 preceding siblings ...) 2023-05-24 6:16 ` [PATCH v2 3/3] x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES Chao Gao @ 2023-06-06 17:26 ` Sean Christopherson 3 siblings, 0 replies; 11+ messages in thread From: Sean Christopherson @ 2023-06-06 17:26 UTC (permalink / raw) To: Sean Christopherson, kvm, x86, Chao Gao Cc: xiaoyao.li, Alexandre Chartre, Ashok Raj, Borislav Petkov, Dave Hansen, H. Peter Anvin, Ingo Molnar, Juergen Gross, Kim Phillips, linux-kernel, Paolo Bonzini, Pawan Gupta, Peter Zijlstra, Thomas Gleixner On Wed, 24 May 2023 14:16:30 +0800, Chao Gao wrote: > Do some cleanups about MSR_IA32_ARCH_CAPABILITIES in KVM e.g, fix stale > comments, elimite open-coded MSR reads, track features that can be exposed to > guests to avoid computing them runtime. They are found when reading the code. > > No functional change intented. > > Changes from v1 -> v2: > 1. toggle ARCH_CAP_SKIP_VMENTRY_L1DFLUSH bit when l1tf mitigation state is > changed by module param. [Xiaoyao, Sean] > 2. add more cleanups (patch 2/3) > > [...] Applied [2/3] to kvm-x86 misc, no need for that one to wait for the dust to settle on the "supported" snapshot. Thanks! [2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush https://github.com/kvm-x86/linux/commit/02f1b0b73660 -- https://github.com/kvm-x86/linux/tree/next https://github.com/kvm-x86/linux/tree/fixes ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-06-06 17:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-24 6:16 [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Chao Gao 2023-05-24 6:16 ` [PATCH v2 1/3] KVM: x86: Track supported ARCH_CAPABILITIES in kvm_caps Chao Gao 2023-05-24 8:14 ` Xiaoyao Li 2023-05-24 8:32 ` Chao Gao 2023-05-25 2:17 ` Xiaoyao Li 2023-06-06 16:40 ` Sean Christopherson 2023-05-24 6:16 ` [PATCH v2 2/3] KVM: x86: Correct the name for skipping VMENTER l1d flush Chao Gao 2023-05-24 8:25 ` Xiaoyao Li 2023-05-24 6:16 ` [PATCH v2 3/3] x86/cpu, KVM: Use helper function to read MSR_IA32_ARCH_CAPABILITIES Chao Gao 2023-05-24 8:19 ` Xiaoyao Li 2023-06-06 17:26 ` [PATCH v2 0/3] MSR_IA32_ARCH_CAPABILITIES cleanups Sean Christopherson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.