* [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present @ 2022-06-29 13:05 Paul Durrant 2022-07-11 22:43 ` Sean Christopherson 0 siblings, 1 reply; 5+ messages in thread From: Paul Durrant @ 2022-06-29 13:05 UTC (permalink / raw) To: x86, kvm, linux-kernel Cc: Paul Durrant, David Woodhouse, Paolo Bonzini, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin The scaling information in subleaf 1 should match the values set by KVM in the 'vcpu_info' sub-structure 'time_info' (a.k.a. pvclock_vcpu_time_info) which is shared with the guest, but is not directly available to the VMM. The offset values are not set since a TSC offset is already applied. The TSC frequency should also be set in sub-leaf 2. Signed-off-by: Paul Durrant <pdurrant@amazon.com> --- Cc: David Woodhouse <dwmw2@infradead.org> v5: - Drop the caching of the CPUID entry pointers and only update the sub-leaves if the CPU frequency has actually changed v4: - Update commit comment v3: - Add leaf limit check in kvm_xen_set_cpuid() v2: - Make sure sub-leaf pointers are NULLed if the time leaf is removed --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/cpuid.c | 2 ++ arch/x86/kvm/x86.c | 1 + arch/x86/kvm/xen.c | 51 +++++++++++++++++++++++++++++++++ arch/x86/kvm/xen.h | 10 +++++++ 5 files changed, 65 insertions(+) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 88a3026ee163..abb0a39f60eb 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -638,6 +638,7 @@ struct kvm_vcpu_xen { struct hrtimer timer; int poll_evtchn; struct timer_list poll_timer; + u32 cpuid_tsc_info; }; struct kvm_vcpu_arch { diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index d47222ab8e6e..544d0f823ee5 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_after_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 031678eff28e..29ed665c51db 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3110,6 +3110,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) &vcpu->hv_clock.tsc_shift, &vcpu->hv_clock.tsc_to_system_mul); vcpu->hw_tsc_khz = tgt_tsc_khz; + kvm_xen_setup_tsc_info(v); } vcpu->hv_clock.tsc_timestamp = tsc_timestamp; diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index 610beba35907..c84424d5c8b6 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,51 @@ 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_after_set_cpuid(struct kvm_vcpu *vcpu) +{ + u32 base = 0; + u32 limit; + u32 function; + + vcpu->arch.xen.cpuid_tsc_info = 0; + + 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; + limit = entry->eax; + break; + } + } + if (!base) + return; + + function = base | XEN_CPUID_LEAF(3); + if (function > limit) + return; + + vcpu->arch.xen.cpuid_tsc_info = function; +} + +void kvm_xen_setup_tsc_info(struct kvm_vcpu *vcpu) +{ + struct kvm_cpuid_entry2 *entry; + + if (!vcpu->arch.xen.cpuid_tsc_info) + return; + + entry = kvm_find_cpuid_entry(vcpu, vcpu->arch.xen.cpuid_tsc_info, 1); + if (entry) { + entry->ecx = vcpu->arch.hv_clock.tsc_to_system_mul; + entry->edx = vcpu->arch.hv_clock.tsc_shift; + } + + entry = kvm_find_cpuid_entry(vcpu, vcpu->arch.xen.cpuid_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..b2ca434431d6 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_after_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_after_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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present 2022-06-29 13:05 [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present Paul Durrant @ 2022-07-11 22:43 ` Sean Christopherson 2022-07-12 8:37 ` Durrant, Paul 0 siblings, 1 reply; 5+ messages in thread From: Sean Christopherson @ 2022-07-11 22:43 UTC (permalink / raw) To: Paul Durrant Cc: x86, kvm, linux-kernel, David Woodhouse, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin On Wed, Jun 29, 2022, Paul Durrant wrote: > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 88a3026ee163..abb0a39f60eb 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -638,6 +638,7 @@ struct kvm_vcpu_xen { > struct hrtimer timer; > int poll_evtchn; > struct timer_list poll_timer; > + u32 cpuid_tsc_info; I would prefer to follow vcpu->arch.kvm_cpuid_base and capture the base CPUID function. I have a hard time believing this will be the only case where KVM needs to query XEN CPUID leafs. And cpuid_tsc_info is a confusing name given the helper kvm_xen_setup_tsc_info(); it's odd to see a "setup" helper immediately consume a variable with the same name. It'll incur another CPUID lookup in the update path to check the limit, but again that should be a rare operation so it doesn't seem too onerous. > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 031678eff28e..29ed665c51db 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3110,6 +3110,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > &vcpu->hv_clock.tsc_shift, > &vcpu->hv_clock.tsc_to_system_mul); > vcpu->hw_tsc_khz = tgt_tsc_khz; > + kvm_xen_setup_tsc_info(v); Any objection to s/setup/update? KVM Xen uses "setup" for things like configuring the event channel using userspace input, whereas this is purely updating existing data structures. > } > > vcpu->hv_clock.tsc_timestamp = tsc_timestamp; > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c > index 610beba35907..c84424d5c8b6 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,51 @@ 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_after_set_cpuid(struct kvm_vcpu *vcpu) > +{ > + u32 base = 0; > + u32 limit; > + u32 function; > + > + vcpu->arch.xen.cpuid_tsc_info = 0; > + > + 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; > + limit = entry->eax; > + break; > + } > + } > + if (!base) > + return; Rather than open code a variant of kvm_update_kvm_cpuid_base(), that helper can be tweaked to take a signature. Along with a patch to provide a #define for Xen's signature as a string, this entire function becomes a one-liner. If the below looks ok (won't compile, needs prep patches), I'll test and post a proper mini-series. --- arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/cpuid.c | 2 ++ arch/x86/kvm/x86.c | 1 + arch/x86/kvm/xen.c | 30 ++++++++++++++++++++++++++++++ arch/x86/kvm/xen.h | 22 +++++++++++++++++++++- 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index de5a149d0971..b2565d05fc86 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -638,6 +638,7 @@ struct kvm_vcpu_xen { struct hrtimer timer; int poll_evtchn; struct timer_list poll_timer; + u32 cpuid_base; }; struct kvm_vcpu_arch { diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 0abe3adc9ae3..54ed51799b8d 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 @@ -309,6 +310,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_after_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 567d13405445..a624293c66c8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3110,6 +3110,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) &vcpu->hv_clock.tsc_shift, &vcpu->hv_clock.tsc_to_system_mul); vcpu->hw_tsc_khz = tgt_tsc_khz; + kvm_xen_update_tsc_info(v); } vcpu->hv_clock.tsc_timestamp = tsc_timestamp; diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index 610beba35907..3fc0c194b813 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,30 @@ 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_update_tsc_info(struct kvm_vcpu *vcpu) +{ + struct kvm_cpuid_entry2 *entry; + u32 function; + + if (!vcpu->arch.xen.cpuid_base) + return; + + entry = kvm_find_cpuid_entry(vcpu, vcpu->arch.xen.cpuid_base, 0); + if (WARN_ON_ONCE(!entry)) + return; + + function = vcpu->arch.xen.cpuid_base | XEN_CPUID_LEAF(3); + if (function > entry->eax) + return; + + entry = kvm_find_cpuid_entry(vcpu, function, 1); + if (entry) { + entry->ecx = vcpu->arch.hv_clock.tsc_to_system_mul; + entry->edx = vcpu->arch.hv_clock.tsc_shift; + } + + entry = kvm_find_cpuid_entry(vcpu, function, 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..b8161b99b82a 100644 --- a/arch/x86/kvm/xen.h +++ b/arch/x86/kvm/xen.h @@ -9,9 +9,14 @@ #ifndef __ARCH_X86_KVM_XEN_H__ #define __ARCH_X86_KVM_XEN_H__ -#ifdef CONFIG_KVM_XEN #include <linux/jump_label_ratelimit.h> +#include <asm/xen/cpuid.h> + +#include "cpuid.h" + +#ifdef CONFIG_KVM_XEN + extern struct static_key_false_deferred kvm_xen_enabled; int __kvm_xen_has_interrupt(struct kvm_vcpu *vcpu); @@ -32,6 +37,13 @@ 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_update_tsc_info(struct kvm_vcpu *vcpu); + +static inline void kvm_xen_after_set_cpuid(struct kvm_vcpu *vcpu) +{ + vcpu->arch.xen.cpuid_base = + kvm_get_hypervisor_cpuid_base(vcpu, XEN_CPUID_SIGNATURE); +} static inline bool kvm_xen_msr_enabled(struct kvm *kvm) { @@ -135,6 +147,14 @@ static inline bool kvm_xen_timer_enabled(struct kvm_vcpu *vcpu) { return false; } + +static inline void kvm_xen_after_set_cpuid(struct kvm_vcpu *vcpu) +{ +} + +static inline void kvm_xen_update_tsc_info(struct kvm_vcpu *vcpu) +{ +} #endif int kvm_xen_hypercall(struct kvm_vcpu *vcpu); base-commit: b08b2f54c49d8f96a22107c444d500dff73ec2a6 -- ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present 2022-07-11 22:43 ` Sean Christopherson @ 2022-07-12 8:37 ` Durrant, Paul 2022-07-12 14:27 ` Sean Christopherson 0 siblings, 1 reply; 5+ messages in thread From: Durrant, Paul @ 2022-07-12 8:37 UTC (permalink / raw) To: Sean Christopherson Cc: x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, David Woodhouse, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin > -----Original Message----- > From: Sean Christopherson <seanjc@google.com> > Sent: 12 July 2022 00:44 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: x86@kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org; David Woodhouse > <dwmw2@infradead.org>; Paolo Bonzini <pbonzini@redhat.com>; Vitaly Kuznetsov <vkuznets@redhat.com>; > Wanpeng Li <wanpengli@tencent.com>; Jim Mattson <jmattson@google.com>; Joerg Roedel <joro@8bytes.org>; > Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; Borislav Petkov <bp@alien8.de>; > Dave Hansen <dave.hansen@linux.intel.com>; H. Peter Anvin <hpa@zytor.com> > Subject: RE: [EXTERNAL][PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if > present > > CAUTION: This email originated from outside of the organization. Do not click links or open > attachments unless you can confirm the sender and know the content is safe. > > > > On Wed, Jun 29, 2022, Paul Durrant wrote: > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index 88a3026ee163..abb0a39f60eb 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -638,6 +638,7 @@ struct kvm_vcpu_xen { > > struct hrtimer timer; > > int poll_evtchn; > > struct timer_list poll_timer; > > + u32 cpuid_tsc_info; > > I would prefer to follow vcpu->arch.kvm_cpuid_base and capture the base CPUID > function. I have a hard time believing this will be the only case where KVM needs > to query XEN CPUID leafs. And cpuid_tsc_info is a confusing name given the helper > kvm_xen_setup_tsc_info(); it's odd to see a "setup" helper immediately consume a > variable with the same name. Sure. It is rather shrink-to-fit at the moment... no problem with capturing the base. > > It'll incur another CPUID lookup in the update path to check the limit, but again > that should be a rare operation so it doesn't seem too onerous. > We could capture the limit leaf in the general case. It's not Xen-specific after all. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 031678eff28e..29ed665c51db 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -3110,6 +3110,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > > &vcpu->hv_clock.tsc_shift, > > &vcpu->hv_clock.tsc_to_system_mul); > > vcpu->hw_tsc_khz = tgt_tsc_khz; > > + kvm_xen_setup_tsc_info(v); > > Any objection to s/setup/update? KVM Xen uses "setup" for things like configuring > the event channel using userspace input, whereas this is purely updating existing > data structures. > Sure. > > } > > > > vcpu->hv_clock.tsc_timestamp = tsc_timestamp; > > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c > > index 610beba35907..c84424d5c8b6 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,51 @@ 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_after_set_cpuid(struct kvm_vcpu *vcpu) > > +{ > > + u32 base = 0; > > + u32 limit; > > + u32 function; > > + > > + vcpu->arch.xen.cpuid_tsc_info = 0; > > + > > + 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; > > + limit = entry->eax; > > + break; > > + } > > + } > > + if (!base) > > + return; > > Rather than open code a variant of kvm_update_kvm_cpuid_base(), that helper can > be tweaked to take a signature. Along with a patch to provide a #define for Xen's > signature as a string, this entire function becomes a one-liner. > Sure, but as said above, we could make capturing the limit part of the general function too. It could even be extended to capture the Hyper-V base/limit too. As for defining the sig as a string... I guess it would be neater to use the values from the Xen header, but it'll probably make the code more ugly so a secondary definition is reasonable. > If the below looks ok (won't compile, needs prep patches), I'll test and post a > proper mini-series. Ok. Thanks, Paul > > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/cpuid.c | 2 ++ > arch/x86/kvm/x86.c | 1 + > arch/x86/kvm/xen.c | 30 ++++++++++++++++++++++++++++++ > arch/x86/kvm/xen.h | 22 +++++++++++++++++++++- > 5 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index de5a149d0971..b2565d05fc86 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -638,6 +638,7 @@ struct kvm_vcpu_xen { > struct hrtimer timer; > int poll_evtchn; > struct timer_list poll_timer; > + u32 cpuid_base; > }; > > struct kvm_vcpu_arch { > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 0abe3adc9ae3..54ed51799b8d 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 > @@ -309,6 +310,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_after_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 567d13405445..a624293c66c8 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3110,6 +3110,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > &vcpu->hv_clock.tsc_shift, > &vcpu->hv_clock.tsc_to_system_mul); > vcpu->hw_tsc_khz = tgt_tsc_khz; > + kvm_xen_update_tsc_info(v); > } > > vcpu->hv_clock.tsc_timestamp = tsc_timestamp; > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c > index 610beba35907..3fc0c194b813 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,30 @@ 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_update_tsc_info(struct kvm_vcpu *vcpu) > +{ > + struct kvm_cpuid_entry2 *entry; > + u32 function; > + > + if (!vcpu->arch.xen.cpuid_base) > + return; > + > + entry = kvm_find_cpuid_entry(vcpu, vcpu->arch.xen.cpuid_base, 0); > + if (WARN_ON_ONCE(!entry)) > + return; > + > + function = vcpu->arch.xen.cpuid_base | XEN_CPUID_LEAF(3); > + if (function > entry->eax) > + return; > + > + entry = kvm_find_cpuid_entry(vcpu, function, 1); > + if (entry) { > + entry->ecx = vcpu->arch.hv_clock.tsc_to_system_mul; > + entry->edx = vcpu->arch.hv_clock.tsc_shift; > + } > + > + entry = kvm_find_cpuid_entry(vcpu, function, 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..b8161b99b82a 100644 > --- a/arch/x86/kvm/xen.h > +++ b/arch/x86/kvm/xen.h > @@ -9,9 +9,14 @@ > #ifndef __ARCH_X86_KVM_XEN_H__ > #define __ARCH_X86_KVM_XEN_H__ > > -#ifdef CONFIG_KVM_XEN > #include <linux/jump_label_ratelimit.h> > > +#include <asm/xen/cpuid.h> > + > +#include "cpuid.h" > + > +#ifdef CONFIG_KVM_XEN > + > extern struct static_key_false_deferred kvm_xen_enabled; > > int __kvm_xen_has_interrupt(struct kvm_vcpu *vcpu); > @@ -32,6 +37,13 @@ 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_update_tsc_info(struct kvm_vcpu *vcpu); > + > +static inline void kvm_xen_after_set_cpuid(struct kvm_vcpu *vcpu) > +{ > + vcpu->arch.xen.cpuid_base = > + kvm_get_hypervisor_cpuid_base(vcpu, XEN_CPUID_SIGNATURE); > +} > > static inline bool kvm_xen_msr_enabled(struct kvm *kvm) > { > @@ -135,6 +147,14 @@ static inline bool kvm_xen_timer_enabled(struct kvm_vcpu *vcpu) > { > return false; > } > + > +static inline void kvm_xen_after_set_cpuid(struct kvm_vcpu *vcpu) > +{ > +} > + > +static inline void kvm_xen_update_tsc_info(struct kvm_vcpu *vcpu) > +{ > +} > #endif > > int kvm_xen_hypercall(struct kvm_vcpu *vcpu); > > base-commit: b08b2f54c49d8f96a22107c444d500dff73ec2a6 > -- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present 2022-07-12 8:37 ` Durrant, Paul @ 2022-07-12 14:27 ` Sean Christopherson 2022-07-15 11:43 ` Vitaly Kuznetsov 0 siblings, 1 reply; 5+ messages in thread From: Sean Christopherson @ 2022-07-12 14:27 UTC (permalink / raw) To: Durrant, Paul Cc: x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, David Woodhouse, Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin On Tue, Jul 12, 2022, Durrant, Paul wrote: > > > @@ -1855,3 +1858,51 @@ 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_after_set_cpuid(struct kvm_vcpu *vcpu) > > > +{ > > > + u32 base = 0; > > > + u32 limit; > > > + u32 function; > > > + > > > + vcpu->arch.xen.cpuid_tsc_info = 0; > > > + > > > + 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; > > > + limit = entry->eax; > > > + break; > > > + } > > > + } > > > + if (!base) > > > + return; > > > > Rather than open code a variant of kvm_update_kvm_cpuid_base(), that helper can > > be tweaked to take a signature. Along with a patch to provide a #define for Xen's > > signature as a string, this entire function becomes a one-liner. > > > > Sure, but as said above, we could make capturing the limit part of the > general function too. It could even be extended to capture the Hyper-V > base/limit too. As for defining the sig as a string... I guess it would be > neater to use the values from the Xen header, but it'll probably make the > code more ugly so a secondary definition is reasonable. The base needs to be captured separately for KVM and Xen because KVM (and presumably Xen itself since Xen also allows a variable base) supports advertising multiple hypervisors to the guest. I don't know if there are any guests that will concurrently utilize multiple hypervisor's paravirt features, so maybe we could squeak by, but saving 4 bytes isn't worth the risk. AFAIK, Hyper-V doesn't allow for a variable base, and so doesn't utilize the for_each_possible... macro. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present 2022-07-12 14:27 ` Sean Christopherson @ 2022-07-15 11:43 ` Vitaly Kuznetsov 0 siblings, 0 replies; 5+ messages in thread From: Vitaly Kuznetsov @ 2022-07-15 11:43 UTC (permalink / raw) To: Sean Christopherson, Durrant, Paul Cc: x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, David Woodhouse, Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin Sean Christopherson <seanjc@google.com> writes: > On Tue, Jul 12, 2022, Durrant, Paul wrote: >> > > @@ -1855,3 +1858,51 @@ 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_after_set_cpuid(struct kvm_vcpu *vcpu) >> > > +{ >> > > + u32 base = 0; >> > > + u32 limit; >> > > + u32 function; >> > > + >> > > + vcpu->arch.xen.cpuid_tsc_info = 0; >> > > + >> > > + 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; >> > > + limit = entry->eax; >> > > + break; >> > > + } >> > > + } >> > > + if (!base) >> > > + return; >> > >> > Rather than open code a variant of kvm_update_kvm_cpuid_base(), that helper can >> > be tweaked to take a signature. Along with a patch to provide a #define for Xen's >> > signature as a string, this entire function becomes a one-liner. >> > >> >> Sure, but as said above, we could make capturing the limit part of the >> general function too. It could even be extended to capture the Hyper-V >> base/limit too. As for defining the sig as a string... I guess it would be >> neater to use the values from the Xen header, but it'll probably make the >> code more ugly so a secondary definition is reasonable. > > The base needs to be captured separately for KVM and Xen because KVM (and presumably > Xen itself since Xen also allows a variable base) supports advertising multiple > hypervisors to the guest. I don't know if there are any guests that will concurrently > utilize multiple hypervisor's paravirt features, so maybe we could squeak by, but > saving 4 bytes isn't worth the risk. > > AFAIK, Hyper-V doesn't allow for a variable base, and so doesn't utilize the > for_each_possible... macro. FWIW, this matches my understanding too: Windows guests don't seem to check anything besides 0x40000001 and give up if Hyper-V's id is not there. -- Vitaly ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-15 11:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-29 13:05 [PATCH v5] KVM: x86/xen: Update Xen CPUID Leaf 4 (tsc info) sub-leaves, if present Paul Durrant 2022-07-11 22:43 ` Sean Christopherson 2022-07-12 8:37 ` Durrant, Paul 2022-07-12 14:27 ` Sean Christopherson 2022-07-15 11:43 ` Vitaly Kuznetsov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox