The scaling information in sub-leaf 1 should match the values in the
'vcpu_info' sub-structure 'time_info' (a.k.a. pvclock_vcpu_time_info) which
is shared with the guest. The offset values are not set since a TSC offset
is already applied.
The host TSC frequency should also be set in sub-leaf 2.
This patch adds a new kvm_xen_set_cpuid() function that scans for the
relevant CPUID leaf when the CPUID information is updated by the VMM and
stashes pointers to the sub-leaves in the kvm_vcpu_xen structure.
The values are then updated by a call to the, also new,
kvm_xen_setup_tsc_info() function made at the end of
kvm_guest_time_update() just before entering the guest.
Signed-off-by: Paul Durrant <[email protected]>
---
v2:
- Make sure sub-leaf pointers are NULLed if the time leaf is removed
---
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/kvm/cpuid.c | 2 ++
arch/x86/kvm/x86.c | 1 +
arch/x86/kvm/xen.c | 44 +++++++++++++++++++++++++++++++++
arch/x86/kvm/xen.h | 10 ++++++++
5 files changed, 59 insertions(+)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1038ccb7056a..f77a4940542f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -638,6 +638,8 @@ struct kvm_vcpu_xen {
struct hrtimer timer;
int poll_evtchn;
struct timer_list poll_timer;
+ struct kvm_cpuid_entry2 *tsc_info_1;
+ struct kvm_cpuid_entry2 *tsc_info_2;
};
struct kvm_vcpu_arch {
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index d47222ab8e6e..eb6cd88c974a 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -25,6 +25,7 @@
#include "mmu.h"
#include "trace.h"
#include "pmu.h"
+#include "xen.h"
/*
* Unlike "struct cpuinfo_x86.x86_capability", kvm_cpu_caps doesn't need to be
@@ -310,6 +311,7 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
__cr4_reserved_bits(guest_cpuid_has, vcpu);
kvm_hv_set_cpuid(vcpu);
+ kvm_xen_set_cpuid(vcpu);
/* Invoke the vendor callback only after the above state is updated. */
static_call(kvm_x86_vcpu_after_set_cpuid)(vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 00e23dc518e0..8b45f9975e45 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3123,6 +3123,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
if (vcpu->xen.vcpu_time_info_cache.active)
kvm_setup_guest_pvclock(v, &vcpu->xen.vcpu_time_info_cache, 0);
kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
+ kvm_xen_setup_tsc_info(v);
return 0;
}
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 610beba35907..4f8d19df20f4 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -10,6 +10,9 @@
#include "xen.h"
#include "hyperv.h"
#include "lapic.h"
+#include "cpuid.h"
+
+#include <asm/xen/cpuid.h>
#include <linux/eventfd.h>
#include <linux/kvm_host.h>
@@ -1855,3 +1858,44 @@ void kvm_xen_destroy_vm(struct kvm *kvm)
if (kvm->arch.xen_hvm_config.msr)
static_branch_slow_dec_deferred(&kvm_xen_enabled);
}
+
+void kvm_xen_set_cpuid(struct kvm_vcpu *vcpu)
+{
+ u32 base = 0;
+ u32 function;
+
+ vcpu->arch.xen.tsc_info_1 = NULL;
+ vcpu->arch.xen.tsc_info_2 = NULL;
+
+ for_each_possible_hypervisor_cpuid_base(function) {
+ struct kvm_cpuid_entry2 *entry = kvm_find_cpuid_entry(vcpu, function, 0);
+
+ if (entry &&
+ entry->ebx == XEN_CPUID_SIGNATURE_EBX &&
+ entry->ecx == XEN_CPUID_SIGNATURE_ECX &&
+ entry->edx == XEN_CPUID_SIGNATURE_EDX) {
+ base = function;
+ break;
+ }
+ }
+ if (!base)
+ return;
+
+ function = base | XEN_CPUID_LEAF(3);
+ vcpu->arch.xen.tsc_info_1 = kvm_find_cpuid_entry(vcpu, function, 1);
+ vcpu->arch.xen.tsc_info_2 = kvm_find_cpuid_entry(vcpu, function, 2);
+}
+
+void kvm_xen_setup_tsc_info(struct kvm_vcpu *vcpu)
+{
+ struct kvm_cpuid_entry2 *entry = vcpu->arch.xen.tsc_info_1;
+
+ if (entry) {
+ entry->ecx = vcpu->arch.hv_clock.tsc_to_system_mul;
+ entry->edx = vcpu->arch.hv_clock.tsc_shift;
+ }
+
+ entry = vcpu->arch.xen.tsc_info_2;
+ if (entry)
+ entry->eax = vcpu->arch.hw_tsc_khz;
+}
diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
index 532a535a9e99..1afb663318a9 100644
--- a/arch/x86/kvm/xen.h
+++ b/arch/x86/kvm/xen.h
@@ -32,6 +32,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe,
int kvm_xen_setup_evtchn(struct kvm *kvm,
struct kvm_kernel_irq_routing_entry *e,
const struct kvm_irq_routing_entry *ue);
+void kvm_xen_set_cpuid(struct kvm_vcpu *vcpu);
+void kvm_xen_setup_tsc_info(struct kvm_vcpu *vcpu);
static inline bool kvm_xen_msr_enabled(struct kvm *kvm)
{
@@ -135,6 +137,14 @@ static inline bool kvm_xen_timer_enabled(struct kvm_vcpu *vcpu)
{
return false;
}
+
+static inline void kvm_xen_set_cpuid(struct kvm_vcpu *vcpu)
+{
+}
+
+static inline void kvm_xen_setup_tsc_info(struct kvm_vcpu *vcpu)
+{
+}
#endif
int kvm_xen_hypercall(struct kvm_vcpu *vcpu);
--
2.20.1
On Wed, 2022-06-22 at 10:57 +0100, Paul Durrant wrote:
> +void kvm_xen_set_cpuid(struct kvm_vcpu *vcpu)
> +{
> + u32 base = 0;
> + u32 function;
> +
> + vcpu->arch.xen.tsc_info_1 = NULL;
> + vcpu->arch.xen.tsc_info_2 = NULL;
> +
> + for_each_possible_hypervisor_cpuid_base(function) {
> + struct kvm_cpuid_entry2 *entry = kvm_find_cpuid_entry(vcpu, function, 0);
> +
> + if (entry &&
> + entry->ebx == XEN_CPUID_SIGNATURE_EBX &&
> + entry->ecx == XEN_CPUID_SIGNATURE_ECX &&
> + entry->edx == XEN_CPUID_SIGNATURE_EDX) {
> + base = function;
> + break;
> + }
> + }
Please make it return if entry->eax < 3 here. There probably aren't any
*good* reasons why a leaf at 0x40000x03 would exist and belong to some
other entity (Hyper-V, etc.). Those other extra ranges of CPUID are
generally aligned at multiples of 0x100, not just the *next* available
leaf.
But I don't care. Unless eax on the main Xen leaf is 3 or more, the
leaf at N+3 isn't yours to reason about :)
> + if (!base)
> + return;
> +
> + function = base | XEN_CPUID_LEAF(3);
> + vcpu->arch.xen.tsc_info_1 = kvm_find_cpuid_entry(vcpu, function, 1);
> + vcpu->arch.xen.tsc_info_2 = kvm_find_cpuid_entry(vcpu, function, 2);
> +}
> +
> -----Original Message-----
> From: David Woodhouse <[email protected]>
> Sent: 22 June 2022 14:48
> To: Durrant, Paul <[email protected]>; [email protected]; [email protected]; linux-
> [email protected]
> Cc: Paolo Bonzini <[email protected]>; Sean Christopherson <[email protected]>; Vitaly Kuznetsov
> <[email protected]>; Wanpeng Li <[email protected]>; Jim Mattson <[email protected]>; Joerg
> Roedel <[email protected]>; Thomas Gleixner <[email protected]>; Ingo Molnar <[email protected]>;
> Borislav Petkov <[email protected]>; Dave Hansen <[email protected]>; H. Peter Anvin
> <[email protected]>
> Subject: RE: [EXTERNAL][PATCH v2] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if
> present
>
> On Wed, 2022-06-22 at 10:57 +0100, Paul Durrant wrote:
> > +void kvm_xen_set_cpuid(struct kvm_vcpu *vcpu)
> > +{
> > + u32 base = 0;
> > + u32 function;
> > +
> > + vcpu->arch.xen.tsc_info_1 = NULL;
> > + vcpu->arch.xen.tsc_info_2 = NULL;
> > +
> > + for_each_possible_hypervisor_cpuid_base(function) {
> > + struct kvm_cpuid_entry2 *entry = kvm_find_cpuid_entry(vcpu, function, 0);
> > +
> > + if (entry &&
> > + entry->ebx == XEN_CPUID_SIGNATURE_EBX &&
> > + entry->ecx == XEN_CPUID_SIGNATURE_ECX &&
> > + entry->edx == XEN_CPUID_SIGNATURE_EDX) {
> > + base = function;
> > + break;
> > + }
> > + }
>
> Please make it return if entry->eax < 3 here. There probably aren't any
> *good* reasons why a leaf at 0x40000x03 would exist and belong to some
> other entity (Hyper-V, etc.). Those other extra ranges of CPUID are
> generally aligned at multiples of 0x100, not just the *next* available
> leaf.
>
> But I don't care. Unless eax on the main Xen leaf is 3 or more, the
> leaf at N+3 isn't yours to reason about :)
>
According to https://lwn.net/Articles/301888 (which is ancient), the Microsoft TLFS and my copy of the Xen source code, it seems everyone does agree that the hypervisor base leaf EAX does specify the maximum leaf (in absolute terms rather than the offset) so I'll add that check into v3.
Paul
> > + if (!base)
> > + return;
> > +
> > + function = base | XEN_CPUID_LEAF(3);
> > + vcpu->arch.xen.tsc_info_1 = kvm_find_cpuid_entry(vcpu, function, 1);
> > + vcpu->arch.xen.tsc_info_2 = kvm_find_cpuid_entry(vcpu, function, 2);
> > +}
> > +