| Message ID | 20220922143655.3721218-3-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Hyper-V invariant TSC control feature | expand |
Why do we need a new 'scattered' leaf? We are only using two bits in the first one, right? And now we're only going to use one bit in the second one? I thought the point of the 'scattered' leaves was to collect a bunch of feature bits from different CPUID leaves in a single feature word. Allocating a new feature word for one or two bits seems extravagant.
On Thu, Sep 22, 2022, Jim Mattson wrote: > Why do we need a new 'scattered' leaf? We are only using two bits in > the first one, right? And now we're only going to use one bit in the > second one? > > I thought the point of the 'scattered' leaves was to collect a bunch > of feature bits from different CPUID leaves in a single feature word. > > Allocating a new feature word for one or two bits seems extravagant. Ah, these leafs aren't scattered from KVM's perspective. The scattered terminology comes from the kernel side, where the KVM-only leafs _may_ be used to deal with features that are scattered by the kernel. But there is no requirement that KVM-only leafs _must_ be scattered by the kernel, e.g. we can and should use this for leafs that KVM wants to expose to the guest, but are completely ignored by the kernel. Intel's PSFD feature flag is a good example. A better shortlog would be: KVM: x86: Add a KVM-only leaf for CPUID_8000_0007_EDX
On Thu, Sep 22, 2022 at 10:53 AM Sean Christopherson <seanjc@google.com> wrote: > > On Thu, Sep 22, 2022, Jim Mattson wrote: > > Why do we need a new 'scattered' leaf? We are only using two bits in > > the first one, right? And now we're only going to use one bit in the > > second one? > > > > I thought the point of the 'scattered' leaves was to collect a bunch > > of feature bits from different CPUID leaves in a single feature word. > > > > Allocating a new feature word for one or two bits seems extravagant. > > Ah, these leafs aren't scattered from KVM's perspective. > > The scattered terminology comes from the kernel side, where the KVM-only leafs > _may_ be used to deal with features that are scattered by the kernel. But there > is no requirement that KVM-only leafs _must_ be scattered by the kernel, e.g. we > can and should use this for leafs that KVM wants to expose to the guest, but are > completely ignored by the kernel. Intel's PSFD feature flag is a good example. > > A better shortlog would be: > > KVM: x86: Add a KVM-only leaf for CPUID_8000_0007_EDX Thanks. The 'scattered' terminology seems more confusing than enlightening.
On Thu, Sep 22, 2022, Vitaly Kuznetsov wrote: > diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h > index a19d473d0184..a5514c89dc29 100644 > --- a/arch/x86/kvm/reverse_cpuid.h > +++ b/arch/x86/kvm/reverse_cpuid.h > @@ -12,7 +12,8 @@ > * "bug" caps, but KVM doesn't use those. > */ > enum kvm_only_cpuid_leafs { > - CPUID_12_EAX = NCAPINTS, > + CPUID_12_EAX = NCAPINTS, > + CPUID_8000_0007_EDX = NCAPINTS + 1, No need to explicitly initialize the new leaf, only the first enum entry needs explicit initialization to NCAPINTS, i.e. let all other entries automatically increment. The order doesn't matter, so not caring about the exact value will avoid bugs due to mismerge and/or bad copy+paste.
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index ffdc28684cb7..b95a4b7489ec 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -685,6 +685,10 @@ void kvm_set_cpu_caps(void) if (!tdp_enabled && IS_ENABLED(CONFIG_X86_64)) kvm_cpu_cap_set(X86_FEATURE_GBPAGES); + kvm_cpu_cap_init_scattered(CPUID_8000_0007_EDX, + SF(CONSTANT_TSC) + ); + kvm_cpu_cap_mask(CPUID_8000_0008_EBX, F(CLZERO) | F(XSAVEERPTR) | F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | @@ -1137,8 +1141,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) /* L2 cache and TLB: pass through host info. */ break; case 0x80000007: /* Advanced power management */ - /* invariant TSC is CPUID.80000007H:EDX[8] */ - entry->edx &= (1 << 8); + cpuid_entry_override(entry, CPUID_8000_0007_EDX); + /* mask against host */ entry->edx &= boot_cpu_data.x86_power; entry->eax = entry->ebx = entry->ecx = 0; diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h index a19d473d0184..a5514c89dc29 100644 --- a/arch/x86/kvm/reverse_cpuid.h +++ b/arch/x86/kvm/reverse_cpuid.h @@ -12,7 +12,8 @@ * "bug" caps, but KVM doesn't use those. */ enum kvm_only_cpuid_leafs { - CPUID_12_EAX = NCAPINTS, + CPUID_12_EAX = NCAPINTS, + CPUID_8000_0007_EDX = NCAPINTS + 1, NR_KVM_CPU_CAPS, NKVMCAPINTS = NR_KVM_CPU_CAPS - NCAPINTS, @@ -24,6 +25,9 @@ enum kvm_only_cpuid_leafs { #define KVM_X86_FEATURE_SGX1 KVM_X86_FEATURE(CPUID_12_EAX, 0) #define KVM_X86_FEATURE_SGX2 KVM_X86_FEATURE(CPUID_12_EAX, 1) +/* CPUID level 0x80000007 (EDX). */ +#define KVM_X86_FEATURE_CONSTANT_TSC KVM_X86_FEATURE(CPUID_8000_0007_EDX, 8) + struct cpuid_reg { u32 function; u32 index; @@ -48,6 +52,7 @@ static const struct cpuid_reg reverse_cpuid[] = { [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX}, [CPUID_12_EAX] = {0x00000012, 0, CPUID_EAX}, [CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX}, + [CPUID_8000_0007_EDX] = {0x80000007, 0, CPUID_EDX}, }; /* @@ -78,6 +83,8 @@ static __always_inline u32 __feature_translate(int x86_feature) return KVM_X86_FEATURE_SGX1; else if (x86_feature == X86_FEATURE_SGX2) return KVM_X86_FEATURE_SGX2; + else if (x86_feature == X86_FEATURE_CONSTANT_TSC) + return KVM_X86_FEATURE_CONSTANT_TSC; return x86_feature; }
CPUID_8000_0007_EDX may come handy when X86_FEATURE_CONSTANT_TSC needs to be checked. No functional change intended. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/kvm/cpuid.c | 8 ++++++-- arch/x86/kvm/reverse_cpuid.h | 9 ++++++++- 2 files changed, 14 insertions(+), 3 deletions(-)