| Message ID | 20220826224755.1330512-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Mask off unsupported and unknown bits of IA32_ARCH_CAPABILITIES | expand |
On Fri, Aug 26, 2022 at 3:48 PM Jim Mattson <jmattson@google.com> wrote: > > KVM should not claim to virtualize unknown IA32_ARCH_CAPABILITIES > bits. When kvm_get_arch_capabilities() was originally written, there > were only a few bits defined in this MSR, and KVM could virtualize all > of them. However, over the years, several bits have been defined that > KVM cannot just blindly pass through to the guest without additional > work (such as virtualizing an MSR promised by the > IA32_ARCH_CAPABILITES feature bit). > > Define a mask of supported IA32_ARCH_CAPABILITIES bits, and mask off > any other bits that are set in the hardware MSR. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Fixes: 5b76a3cff011 ("KVM: VMX: Tell the nested hypervisor to skip L1D flush on vmentry") > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/x86.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 205ebdc2b11b..ae6be8b2ecfe 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1557,12 +1557,32 @@ static const u32 msr_based_features_all[] = { > static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)]; > static unsigned int num_msr_based_features; > > +/* > + * IA32_ARCH_CAPABILITIES bits deliberately omitted are: Why are these deliberately omitted? Maybe a comment will help future readers and present ones like me who don't know. > + * 10 - MISC_PACKAGE_CTRLS > + * 11 - ENERGY_FILTERING_CTL > + * 12 - DOITM > + * 18 - FB_CLEAR_CTRL > + * 21 - XAPIC_DISABLE_STATUS > + * 23 - OVERCLOCKING_STATUS > + */ > + > +#define KVM_SUPPORTED_ARCH_CAP \ > + (ARCH_CAP_RDCL_NO | ARCH_CAP_IBRS_ALL | ARCH_CAP_RSBA | \ > + ARCH_CAP_SKIP_VMENTRY_L1DFLUSH | ARCH_CAP_SSB_NO | ARCH_CAP_MDS_NO | \ > + ARCH_CAP_PSCHANGE_MC_NO | ARCH_CAP_TSX_CTRL_MSR | ARCH_CAP_TAA_NO | \ > + ARCH_CAP_SBDR_SSDP_NO | ARCH_CAP_FBSDP_NO | ARCH_CAP_PSDP_NO | \ > + ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO) > + > + Nit: Extra blank line. > static u64 kvm_get_arch_capabilities(void) > { > u64 data = 0; > > - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) > + if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { > rdmsrl(MSR_IA32_ARCH_CAPABILITIES, data); > + data &= KVM_SUPPORTED_ARCH_CAP; > + } > > /* > * If nx_huge_pages is enabled, KVM's shadow paging will ensure that > @@ -1610,9 +1630,6 @@ static u64 kvm_get_arch_capabilities(void) > */ > } > > - /* Guests don't need to know "Fill buffer clear control" exists */ > - data &= ~ARCH_CAP_FB_CLEAR_CTRL; > - > return data; > } > > -- > 2.37.2.672.g94769d06f0-goog > Verified, at least all the capabilities in the kvm_get_arch_capabilities() are in the KVM_SUPPORTED_ARCH_CAP macro. Reviewed-By: Vipin Sharma <vipinsh@google.com>
On Fri, Aug 26, 2022 at 4:12 PM Vipin Sharma <vipinsh@google.com> wrote: > > On Fri, Aug 26, 2022 at 3:48 PM Jim Mattson <jmattson@google.com> wrote: > > > > KVM should not claim to virtualize unknown IA32_ARCH_CAPABILITIES > > bits. When kvm_get_arch_capabilities() was originally written, there > > were only a few bits defined in this MSR, and KVM could virtualize all > > of them. However, over the years, several bits have been defined that > > KVM cannot just blindly pass through to the guest without additional > > work (such as virtualizing an MSR promised by the > > IA32_ARCH_CAPABILITES feature bit). > > > > Define a mask of supported IA32_ARCH_CAPABILITIES bits, and mask off > > any other bits that are set in the hardware MSR. > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Fixes: 5b76a3cff011 ("KVM: VMX: Tell the nested hypervisor to skip L1D flush on vmentry") > > Signed-off-by: Jim Mattson <jmattson@google.com> > > --- > > arch/x86/kvm/x86.c | 25 +++++++++++++++++++++---- > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 205ebdc2b11b..ae6be8b2ecfe 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -1557,12 +1557,32 @@ static const u32 msr_based_features_all[] = { > > static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)]; > > static unsigned int num_msr_based_features; > > > > +/* > > + * IA32_ARCH_CAPABILITIES bits deliberately omitted are: > > Why are these deliberately omitted? Maybe a comment will help future > readers and present ones like me who don't know. All of these bits require KVM to virtualize an MSR not currently virtualized. I'll add a comment to that effect. Note that some trivially virtualizable bits are also omitted simply because there are no macros defined for them in msr-index.h. I have said nothing about those. > > + * 10 - MISC_PACKAGE_CTRLS > > + * 11 - ENERGY_FILTERING_CTL > > + * 12 - DOITM > > + * 18 - FB_CLEAR_CTRL > > + * 21 - XAPIC_DISABLE_STATUS > > + * 23 - OVERCLOCKING_STATUS > > + */ > > + > > +#define KVM_SUPPORTED_ARCH_CAP \ > > + (ARCH_CAP_RDCL_NO | ARCH_CAP_IBRS_ALL | ARCH_CAP_RSBA | \ > > + ARCH_CAP_SKIP_VMENTRY_L1DFLUSH | ARCH_CAP_SSB_NO | ARCH_CAP_MDS_NO | \ > > + ARCH_CAP_PSCHANGE_MC_NO | ARCH_CAP_TSX_CTRL_MSR | ARCH_CAP_TAA_NO | \ > > + ARCH_CAP_SBDR_SSDP_NO | ARCH_CAP_FBSDP_NO | ARCH_CAP_PSDP_NO | \ > > + ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO) > > + > > + > > Nit: Extra blank line. > > > static u64 kvm_get_arch_capabilities(void) > > { > > u64 data = 0; > > > > - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) > > + if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { > > rdmsrl(MSR_IA32_ARCH_CAPABILITIES, data); > > + data &= KVM_SUPPORTED_ARCH_CAP; > > + } > > > > /* > > * If nx_huge_pages is enabled, KVM's shadow paging will ensure that > > @@ -1610,9 +1630,6 @@ static u64 kvm_get_arch_capabilities(void) > > */ > > } > > > > - /* Guests don't need to know "Fill buffer clear control" exists */ > > - data &= ~ARCH_CAP_FB_CLEAR_CTRL; > > - > > return data; > > } > > > > -- > > 2.37.2.672.g94769d06f0-goog > > > > Verified, at least all the capabilities in the > kvm_get_arch_capabilities() are in the KVM_SUPPORTED_ARCH_CAP macro. > > Reviewed-By: Vipin Sharma <vipinsh@google.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 205ebdc2b11b..ae6be8b2ecfe 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1557,12 +1557,32 @@ static const u32 msr_based_features_all[] = { static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)]; static unsigned int num_msr_based_features; +/* + * IA32_ARCH_CAPABILITIES bits deliberately omitted are: + * 10 - MISC_PACKAGE_CTRLS + * 11 - ENERGY_FILTERING_CTL + * 12 - DOITM + * 18 - FB_CLEAR_CTRL + * 21 - XAPIC_DISABLE_STATUS + * 23 - OVERCLOCKING_STATUS + */ + +#define KVM_SUPPORTED_ARCH_CAP \ + (ARCH_CAP_RDCL_NO | ARCH_CAP_IBRS_ALL | ARCH_CAP_RSBA | \ + ARCH_CAP_SKIP_VMENTRY_L1DFLUSH | ARCH_CAP_SSB_NO | ARCH_CAP_MDS_NO | \ + ARCH_CAP_PSCHANGE_MC_NO | ARCH_CAP_TSX_CTRL_MSR | ARCH_CAP_TAA_NO | \ + ARCH_CAP_SBDR_SSDP_NO | ARCH_CAP_FBSDP_NO | ARCH_CAP_PSDP_NO | \ + ARCH_CAP_FB_CLEAR | ARCH_CAP_RRSBA | ARCH_CAP_PBRSB_NO) + + static u64 kvm_get_arch_capabilities(void) { u64 data = 0; - if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) + if (boot_cpu_has(X86_FEATURE_ARCH_CAPABILITIES)) { rdmsrl(MSR_IA32_ARCH_CAPABILITIES, data); + data &= KVM_SUPPORTED_ARCH_CAP; + } /* * If nx_huge_pages is enabled, KVM's shadow paging will ensure that @@ -1610,9 +1630,6 @@ static u64 kvm_get_arch_capabilities(void) */ } - /* Guests don't need to know "Fill buffer clear control" exists */ - data &= ~ARCH_CAP_FB_CLEAR_CTRL; - return data; }
KVM should not claim to virtualize unknown IA32_ARCH_CAPABILITIES bits. When kvm_get_arch_capabilities() was originally written, there were only a few bits defined in this MSR, and KVM could virtualize all of them. However, over the years, several bits have been defined that KVM cannot just blindly pass through to the guest without additional work (such as virtualizing an MSR promised by the IA32_ARCH_CAPABILITES feature bit). Define a mask of supported IA32_ARCH_CAPABILITIES bits, and mask off any other bits that are set in the hardware MSR. Cc: Paolo Bonzini <pbonzini@redhat.com> Fixes: 5b76a3cff011 ("KVM: VMX: Tell the nested hypervisor to skip L1D flush on vmentry") Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/x86.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-)