A sunsequent patch will need to acquire the CPUID leaf range for emulated
Xen so explicitly pass the signature of the hypervisor we're interested in
to the new function. Also introduce a new kvm_hypervisor_cpuid structure
so we can neatly store both the base and limit leaf indices.
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
v6:
- New in this version
---
arch/x86/include/asm/kvm_host.h | 7 ++++++-
arch/x86/kvm/cpuid.c | 15 ++++++++-------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f35f1ff4427b..ff201ad35551 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -710,6 +710,11 @@ struct kvm_queued_exception {
bool has_payload;
};
+struct kvm_hypervisor_cpuid {
+ u32 base;
+ u32 limit;
+};
+
struct kvm_vcpu_arch {
/*
* rip and regs accesses must go through
@@ -826,7 +831,7 @@ struct kvm_vcpu_arch {
int cpuid_nent;
struct kvm_cpuid_entry2 *cpuid_entries;
- u32 kvm_cpuid_base;
+ struct kvm_hypervisor_cpuid kvm_cpuid;
u64 reserved_gpa_bits;
int maxphyaddr;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0b5bf013fcb8..2468720f8d84 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -180,12 +180,13 @@ static int kvm_cpuid_check_equal(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2
return 0;
}
-static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu)
+static void kvm_update_hypervisor_cpuid(struct kvm_vcpu *vcpu, const char *hypervisor_signature,
+ struct kvm_hypervisor_cpuid *hypervisor_cpuid)
{
u32 function;
struct kvm_cpuid_entry2 *entry;
- vcpu->arch.kvm_cpuid_base = 0;
+ memset(hypervisor_cpuid, 0, sizeof(*hypervisor_cpuid));
for_each_possible_hypervisor_cpuid_base(function) {
entry = kvm_find_cpuid_entry(vcpu, function);
@@ -197,9 +198,9 @@ static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu)
signature[1] = entry->ecx;
signature[2] = entry->edx;
- BUILD_BUG_ON(sizeof(signature) > sizeof(KVM_SIGNATURE));
- if (!memcmp(signature, KVM_SIGNATURE, sizeof(signature))) {
- vcpu->arch.kvm_cpuid_base = function;
+ if (!memcmp(signature, hypervisor_signature, sizeof(signature))) {
+ hypervisor_cpuid->base = function;
+ hypervisor_cpuid->limit = entry->eax;
break;
}
}
@@ -209,7 +210,7 @@ static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu)
static struct kvm_cpuid_entry2 *__kvm_find_kvm_cpuid_features(struct kvm_vcpu *vcpu,
struct kvm_cpuid_entry2 *entries, int nent)
{
- u32 base = vcpu->arch.kvm_cpuid_base;
+ u32 base = vcpu->arch.kvm_cpuid.base;
if (!base)
return NULL;
@@ -439,7 +440,7 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,
vcpu->arch.cpuid_entries = e2;
vcpu->arch.cpuid_nent = nent;
- kvm_update_kvm_cpuid_base(vcpu);
+ kvm_update_hypervisor_cpuid(vcpu, KVM_SIGNATURE, &vcpu->arch.kvm_cpuid);
kvm_vcpu_after_set_cpuid(vcpu);
return 0;
--
2.20.1
On Tue, Dec 20, 2022, Paul Durrant wrote: > A sunsequent patch will need to acquire the CPUID leaf range for emulated > Xen so explicitly pass the signature of the hypervisor we're interested in > to the new function. Also introduce a new kvm_hypervisor_cpuid structure > so we can neatly store both the base and limit leaf indices. > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > --- > Cc: Sean Christopherson <seanjc@google.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: David Woodhouse <dwmw2@infradead.org> > > v6: > - New in this version > --- > arch/x86/include/asm/kvm_host.h | 7 ++++++- > arch/x86/kvm/cpuid.c | 15 ++++++++------- > 2 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index f35f1ff4427b..ff201ad35551 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -710,6 +710,11 @@ struct kvm_queued_exception { > bool has_payload; > }; > > +struct kvm_hypervisor_cpuid { > + u32 base; > + u32 limit; > +}; Probably makes sense to place this above "struct kvm_vcpu_xen" right away to avoid the (very minor) churn. > struct kvm_vcpu_arch { > /* > * rip and regs accesses must go through > @@ -826,7 +831,7 @@ struct kvm_vcpu_arch { > > int cpuid_nent; > struct kvm_cpuid_entry2 *cpuid_entries; > - u32 kvm_cpuid_base; > + struct kvm_hypervisor_cpuid kvm_cpuid; > > u64 reserved_gpa_bits; > int maxphyaddr; > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 0b5bf013fcb8..2468720f8d84 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -180,12 +180,13 @@ static int kvm_cpuid_check_equal(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 > return 0; > } > > -static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu) > +static void kvm_update_hypervisor_cpuid(struct kvm_vcpu *vcpu, const char *hypervisor_signature, Please wrap. The 80 char limit is a soft limit, but should still be honored unless there's a good reason to run over. I also vote to name the param "sig" to keep line lengths short. > + struct kvm_hypervisor_cpuid *hypervisor_cpuid) Since the struct is a 64-bit value, what about making this a pure getter that returns a copy? static struct kvm_hypervisor_cpuid kvm_get_hypervisor_cpuid(struct kvm_vcpu *vcpu, const char *sig) { struct kvm_hypervisor_cpuid cpuid = {}; struct kvm_cpuid_entry2 *entry; u32 function; for_each_possible_hypervisor_cpuid_base(cpuid.base) { entry = kvm_find_cpuid_entry(vcpu, function); if (entry) { u32 signature[3]; signature[0] = entry->ebx; signature[1] = entry->ecx; signature[2] = entry->edx; if (!memcmp(signature, sig, sizeof(signature))) { cpuid.base = function; cpuid.limit = entry->eax; break; } } } return cpuid; } vcpu->arch.kvm_cpuid = kvm_get_hypervisor_cpuid(vcpu, KVM_SIGNATURE); vcpu->arch.xen.cpuid = kvm_get_hypervisor_cpuid(vcpu, XEN_SIGNATURE);
On 04/01/2023 19:34, Sean Christopherson wrote: > On Tue, Dec 20, 2022, Paul Durrant wrote: >> A sunsequent patch will need to acquire the CPUID leaf range for emulated >> Xen so explicitly pass the signature of the hypervisor we're interested in >> to the new function. Also introduce a new kvm_hypervisor_cpuid structure >> so we can neatly store both the base and limit leaf indices. >> >> Signed-off-by: Paul Durrant <pdurrant@amazon.com> >> --- >> Cc: Sean Christopherson <seanjc@google.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: David Woodhouse <dwmw2@infradead.org> >> >> v6: >> - New in this version >> --- >> arch/x86/include/asm/kvm_host.h | 7 ++++++- >> arch/x86/kvm/cpuid.c | 15 ++++++++------- >> 2 files changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index f35f1ff4427b..ff201ad35551 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -710,6 +710,11 @@ struct kvm_queued_exception { >> bool has_payload; >> }; >> >> +struct kvm_hypervisor_cpuid { >> + u32 base; >> + u32 limit; >> +}; > > Probably makes sense to place this above "struct kvm_vcpu_xen" right away to > avoid the (very minor) churn. > Sure. >> struct kvm_vcpu_arch { >> /* >> * rip and regs accesses must go through >> @@ -826,7 +831,7 @@ struct kvm_vcpu_arch { >> >> int cpuid_nent; >> struct kvm_cpuid_entry2 *cpuid_entries; >> - u32 kvm_cpuid_base; >> + struct kvm_hypervisor_cpuid kvm_cpuid; >> >> u64 reserved_gpa_bits; >> int maxphyaddr; >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >> index 0b5bf013fcb8..2468720f8d84 100644 >> --- a/arch/x86/kvm/cpuid.c >> +++ b/arch/x86/kvm/cpuid.c >> @@ -180,12 +180,13 @@ static int kvm_cpuid_check_equal(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 >> return 0; >> } >> >> -static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu) >> +static void kvm_update_hypervisor_cpuid(struct kvm_vcpu *vcpu, const char *hypervisor_signature, > > Please wrap. The 80 char limit is a soft limit, but should still be honored unless > there's a good reason to run over. Ok. > > I also vote to name the param "sig" to keep line lengths short. > >> + struct kvm_hypervisor_cpuid *hypervisor_cpuid) > > Since the struct is a 64-bit value, what about making this a pure getter that > returns a copy? > > static struct kvm_hypervisor_cpuid kvm_get_hypervisor_cpuid(struct kvm_vcpu *vcpu, > const char *sig) > { > struct kvm_hypervisor_cpuid cpuid = {}; > struct kvm_cpuid_entry2 *entry; > u32 function; > > for_each_possible_hypervisor_cpuid_base(cpuid.base) { > entry = kvm_find_cpuid_entry(vcpu, function); > > if (entry) { > u32 signature[3]; > > signature[0] = entry->ebx; > signature[1] = entry->ecx; > signature[2] = entry->edx; > > if (!memcmp(signature, sig, sizeof(signature))) { > cpuid.base = function; > cpuid.limit = entry->eax; > break; > } > } > } > > return cpuid; > } > > > vcpu->arch.kvm_cpuid = kvm_get_hypervisor_cpuid(vcpu, KVM_SIGNATURE); > vcpu->arch.xen.cpuid = kvm_get_hypervisor_cpuid(vcpu, XEN_SIGNATURE); Yes, if that's preferable then no problem. Paul
On Thu, Jan 05, 2023, Paul Durrant wrote: > On 04/01/2023 19:34, Sean Christopherson wrote: > > Since the struct is a 64-bit value, what about making this a pure getter that > > returns a copy? > > > > static struct kvm_hypervisor_cpuid kvm_get_hypervisor_cpuid(struct kvm_vcpu *vcpu, > > const char *sig) > > { > > struct kvm_hypervisor_cpuid cpuid = {}; > > struct kvm_cpuid_entry2 *entry; > > u32 function; > > > > for_each_possible_hypervisor_cpuid_base(cpuid.base) { > > entry = kvm_find_cpuid_entry(vcpu, function); > > > > if (entry) { > > u32 signature[3]; > > > > signature[0] = entry->ebx; > > signature[1] = entry->ecx; > > signature[2] = entry->edx; > > > > if (!memcmp(signature, sig, sizeof(signature))) { > > cpuid.base = function; > > cpuid.limit = entry->eax; > > break; > > } > > } > > } > > > > return cpuid; > > } > > > > > > vcpu->arch.kvm_cpuid = kvm_get_hypervisor_cpuid(vcpu, KVM_SIGNATURE); > > vcpu->arch.xen.cpuid = kvm_get_hypervisor_cpuid(vcpu, XEN_SIGNATURE); > > Yes, if that's preferable then no problem. I like it (obviously), but it's probably worth waiting a few days to see what others think before posting a new version.
On 05/01/2023 18:09, Sean Christopherson wrote: > On Thu, Jan 05, 2023, Paul Durrant wrote: >> On 04/01/2023 19:34, Sean Christopherson wrote: >>> Since the struct is a 64-bit value, what about making this a pure getter that >>> returns a copy? >>> >>> static struct kvm_hypervisor_cpuid kvm_get_hypervisor_cpuid(struct kvm_vcpu *vcpu, >>> const char *sig) >>> { >>> struct kvm_hypervisor_cpuid cpuid = {}; >>> struct kvm_cpuid_entry2 *entry; >>> u32 function; >>> >>> for_each_possible_hypervisor_cpuid_base(cpuid.base) { >>> entry = kvm_find_cpuid_entry(vcpu, function); >>> >>> if (entry) { >>> u32 signature[3]; >>> >>> signature[0] = entry->ebx; >>> signature[1] = entry->ecx; >>> signature[2] = entry->edx; >>> >>> if (!memcmp(signature, sig, sizeof(signature))) { >>> cpuid.base = function; >>> cpuid.limit = entry->eax; >>> break; >>> } >>> } >>> } >>> >>> return cpuid; >>> } >>> >>> >>> vcpu->arch.kvm_cpuid = kvm_get_hypervisor_cpuid(vcpu, KVM_SIGNATURE); >>> vcpu->arch.xen.cpuid = kvm_get_hypervisor_cpuid(vcpu, XEN_SIGNATURE); >> >> Yes, if that's preferable then no problem. > > I like it (obviously), but it's probably worth waiting a few days to see what > others think before posting a new version. I think it's cleaner too, and I already did the typing so I may as well post. I don't think anyone else has expressed any strong opinions on the code either way. Paul
On Tue, 2022-12-20 at 13:40 +0000, Paul Durrant wrote: > A sunsequent patch will need to acquire the CPUID leaf range for emulated > Xen so explicitly pass the signature of the hypervisor we're interested in > to the new function. Also introduce a new kvm_hypervisor_cpuid structure > so we can neatly store both the base and limit leaf indices. > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > --- Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > Cc: Sean Christopherson <seanjc@google.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: David Woodhouse <dwmw2@infradead.org> > > v6: > - New in this version > --- > arch/x86/include/asm/kvm_host.h | 7 ++++++- > arch/x86/kvm/cpuid.c | 15 ++++++++------- > 2 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index f35f1ff4427b..ff201ad35551 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -710,6 +710,11 @@ struct kvm_queued_exception { > bool has_payload; > }; > > +struct kvm_hypervisor_cpuid { > + u32 base; > + u32 limit; > +}; > + > struct kvm_vcpu_arch { > /* > * rip and regs accesses must go through > @@ -826,7 +831,7 @@ struct kvm_vcpu_arch { > > int cpuid_nent; > struct kvm_cpuid_entry2 *cpuid_entries; > - u32 kvm_cpuid_base; > + struct kvm_hypervisor_cpuid kvm_cpuid; > > u64 reserved_gpa_bits; > int maxphyaddr; > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 0b5bf013fcb8..2468720f8d84 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -180,12 +180,13 @@ static int kvm_cpuid_check_equal(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 > return 0; > } > > -static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu) > +static void kvm_update_hypervisor_cpuid(struct kvm_vcpu *vcpu, const char *hypervisor_signature, > + struct kvm_hypervisor_cpuid *hypervisor_cpuid) > { > u32 function; > struct kvm_cpuid_entry2 *entry; > > - vcpu->arch.kvm_cpuid_base = 0; > + memset(hypervisor_cpuid, 0, sizeof(*hypervisor_cpuid)); > > for_each_possible_hypervisor_cpuid_base(function) { > entry = kvm_find_cpuid_entry(vcpu, function); > @@ -197,9 +198,9 @@ static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu) > signature[1] = entry->ecx; > signature[2] = entry->edx; > > - BUILD_BUG_ON(sizeof(signature) > sizeof(KVM_SIGNATURE)); > - if (!memcmp(signature, KVM_SIGNATURE, sizeof(signature))) { > - vcpu->arch.kvm_cpuid_base = function; > + if (!memcmp(signature, hypervisor_signature, sizeof(signature))) { > + hypervisor_cpuid->base = function; > + hypervisor_cpuid->limit = entry->eax; > break; > } > } > @@ -209,7 +210,7 @@ static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu) > static struct kvm_cpuid_entry2 *__kvm_find_kvm_cpuid_features(struct kvm_vcpu *vcpu, > struct kvm_cpuid_entry2 *entries, int nent) > { > - u32 base = vcpu->arch.kvm_cpuid_base; > + u32 base = vcpu->arch.kvm_cpuid.base; > > if (!base) > return NULL; > @@ -439,7 +440,7 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2, > vcpu->arch.cpuid_entries = e2; > vcpu->arch.cpuid_nent = nent; > > - kvm_update_kvm_cpuid_base(vcpu); > + kvm_update_hypervisor_cpuid(vcpu, KVM_SIGNATURE, &vcpu->arch.kvm_cpuid); > kvm_vcpu_after_set_cpuid(vcpu); > > return 0;
© 2016 - 2025 Red Hat, Inc.