On Fri, Aug 26, 2022 at 4:12 PM Vipin Sharma <vipinsh@xxxxxxxxxx> wrote:
>
> On Fri, Aug 26, 2022 at 3:48 PM Jim Mattson <jmattson@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Fixes: 5b76a3cff011 ("KVM: VMX: Tell the nested hypervisor to skip L1D flush on vmentry")
> > Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
