* Re: [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI [not found] <1655124522-42030-1-git-send-email-wangguangju@baidu.com> @ 2022-06-13 17:16 ` Sean Christopherson 2022-06-14 2:54 ` Chao Gao 0 siblings, 1 reply; 6+ messages in thread From: Sean Christopherson @ 2022-06-13 17:16 UTC (permalink / raw) To: wangguangju Cc: pbonzini, vkuznets, wanpengli, jmattson, joro, kvm, dave.hansen, tglx, mingo, x86, linux-kernel The shortlog is not at all helpful, it doesn't say anything about what actual functional change. KVM: x86: Don't advertise PV IPI to userspace if IPIs are virtualized On Mon, Jun 13, 2022, wangguangju wrote: > Commit d588bb9be1da ("KVM: VMX: enable IPI virtualization") > enable IPI virtualization in Intel SPR platform.There is no point > in using PVIPI if IPIv is supported, it doesn't work less good > with PVIPI than without it. > > So add a bool variable to distinguish whether to use PVIPI. Similar complaint with the changelog, it doesn't actually call out why PV IPIs are unwanted. Don't advertise PV IPI support to userspace if IPI virtualization is supported by the CPU. Hardware virtualization of IPIs more performant as senders do not need to exit. That said, I'm not sure that KVM should actually hide PV_SEND_IPI. KVM still supports the feature, and unlike sched_info_on(), IPI virtualization is platform dependent and not fully controlled by software. E.g. hiding PV_SEND_IPI could cause problems when migrating from a platform without IPIv to a platform with IPIv, as a paranoid VMM might complain that an exposed feature isn't supported by KVM. There's also the question of what to do about AVIC. AVIC has many more inhibits than APICv, e.g. an x2APIC guest running on hardware that doesn't accelerate x2APIC IPIs will probably be better off with PV IPIs. Given that userspace should have read access to the module param, I'm tempted to say KVM should let userspace make the decision of whether or not to advertise PV IPIs to the guest. > Signed-off-by: wangguangju <wangguangju@baidu.com> > --- > arch/x86/include/asm/kvm_host.h | 1 + > arch/x86/kvm/cpuid.c | 4 +++- > arch/x86/kvm/vmx/vmx.c | 1 + > arch/x86/kvm/x86.c | 3 +++ > 4 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index a4b9282..239c1a992 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1671,6 +1671,7 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, unsigned irqchip, unsigned pin, > bool mask); > > extern bool tdp_enabled; > +extern bool kvm_ipiv_cap_supported; Please use "struct kvm_caps", which was added specifically to avoid these one-off bools. > u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu); > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index d47222a..9643572 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -1049,7 +1049,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > (1 << KVM_FEATURE_PV_UNHALT) | > (1 << KVM_FEATURE_PV_TLB_FLUSH) | > (1 << KVM_FEATURE_ASYNC_PF_VMEXIT) | > - (1 << KVM_FEATURE_PV_SEND_IPI) | > (1 << KVM_FEATURE_POLL_CONTROL) | > (1 << KVM_FEATURE_PV_SCHED_YIELD) | > (1 << KVM_FEATURE_ASYNC_PF_INT); > @@ -1057,6 +1056,9 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > if (sched_info_on()) > entry->eax |= (1 << KVM_FEATURE_STEAL_TIME); > > + if (!kvm_ipiv_cap_supported) > + entry->eax |= (1 << KVM_FEATURE_PV_SEND_IPI); > + > entry->ebx = 0; > entry->ecx = 0; > entry->edx = 0; > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index f741de4..21b67f4 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -4490,6 +4490,7 @@ static int vmx_alloc_ipiv_pid_table(struct kvm *kvm) > return -ENOMEM; > > kvm_vmx->pid_table = (void *)page_address(pages); > + kvm_ipiv_cap_supported = true; This is far too late, as allocation of the table doesn't happen until the first VM is created. E.g. diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index b959fe24c13b..73973b5901a3 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -8221,6 +8221,7 @@ static __init int hardware_setup(void) kvm_caps.tsc_scaling_ratio_frac_bits = 48; kvm_caps.has_bus_lock_exit = cpu_has_vmx_bus_lock_detection(); kvm_caps.has_notify_vmexit = cpu_has_notify_vmexit(); + kvm_caps.has_ipi_virtualization = enable_ipiv; set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */ diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 501b884b8cc4..9b80aa67349f 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -23,6 +23,7 @@ struct kvm_caps { bool has_bus_lock_exit; /* notify VM exit supported? */ bool has_notify_vmexit; + bool has_ipi_virtualization; u64 supported_mce_cap; u64 supported_xcr0; > return 0; > } > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index f9d0c56..099f76f 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -226,6 +226,9 @@ EXPORT_SYMBOL_GPL(enable_apicv); > u64 __read_mostly host_xss; > EXPORT_SYMBOL_GPL(host_xss); > > +bool __read_mostly kvm_ipiv_cap_supported = false; > +EXPORT_SYMBOL_GPL(kvm_ipiv_cap_supported); > + > const struct _kvm_stats_desc kvm_vm_stats_desc[] = { > KVM_GENERIC_VM_STATS(), > STATS_DESC_COUNTER(VM, mmu_shadow_zapped), > -- > 2.9.4 > ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI 2022-06-13 17:16 ` [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI Sean Christopherson @ 2022-06-14 2:54 ` Chao Gao 2022-06-14 14:41 ` Sean Christopherson 0 siblings, 1 reply; 6+ messages in thread From: Chao Gao @ 2022-06-14 2:54 UTC (permalink / raw) To: Sean Christopherson Cc: wangguangju, pbonzini, vkuznets, wanpengli, jmattson, joro, kvm, dave.hansen, tglx, mingo, x86, linux-kernel On Mon, Jun 13, 2022 at 05:16:48PM +0000, Sean Christopherson wrote: >The shortlog is not at all helpful, it doesn't say anything about what actual >functional change. > > KVM: x86: Don't advertise PV IPI to userspace if IPIs are virtualized > >On Mon, Jun 13, 2022, wangguangju wrote: >> Commit d588bb9be1da ("KVM: VMX: enable IPI virtualization") >> enable IPI virtualization in Intel SPR platform.There is no point >> in using PVIPI if IPIv is supported, it doesn't work less good >> with PVIPI than without it. >> >> So add a bool variable to distinguish whether to use PVIPI. > >Similar complaint with the changelog, it doesn't actually call out why PV IPIs >are unwanted. > > Don't advertise PV IPI support to userspace if IPI virtualization is > supported by the CPU. Hardware virtualization of IPIs more performant > as senders do not need to exit. PVIPI is mainly [*] for sending multi-cast IPIs. Intel IPI virtualization can virtualize only uni-cast IPIs. Their use cases don't overlap. So, I don't think it makes sense to disable PVIPI if intel IPI virtualization is supported. The question actually is how to deal with the exceptional case below. Considering the migration case Sean said below, it is hard to let VM always work in the ideal way unless KVM notifies VM of migration and VM changes its behavior on receiving such notifications. But since x2apic has better performance than xapic, if VM cares about performance, it can simply switch to x2apic mode. All things considered, I think the performance gain isn't worth the complexity added. So, I prefer to leave it as is. [*]: when linux guest is in *xapic* mode, it uses PVIPI to send uni-case IPI. > >That said, I'm not sure that KVM should actually hide PV_SEND_IPI. KVM still >supports the feature, and unlike sched_info_on(), IPI virtualization is platform >dependent and not fully controlled by software. E.g. hiding PV_SEND_IPI could >cause problems when migrating from a platform without IPIv to a platform with IPIv, >as a paranoid VMM might complain that an exposed feature isn't supported by KVM. > >There's also the question of what to do about AVIC. AVIC has many more inhibits >than APICv, e.g. an x2APIC guest running on hardware that doesn't accelerate x2APIC >IPIs will probably be better off with PV IPIs. > >Given that userspace should have read access to the module param, I'm tempted to >say KVM should let userspace make the decision of whether or not to advertise PV >IPIs to the guest. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI 2022-06-14 2:54 ` Chao Gao @ 2022-06-14 14:41 ` Sean Christopherson 2022-06-14 15:03 ` Chao Gao 0 siblings, 1 reply; 6+ messages in thread From: Sean Christopherson @ 2022-06-14 14:41 UTC (permalink / raw) To: Chao Gao Cc: wangguangju, pbonzini, vkuznets, wanpengli, jmattson, joro, kvm, dave.hansen, tglx, mingo, x86, linux-kernel On Tue, Jun 14, 2022, Chao Gao wrote: > On Mon, Jun 13, 2022 at 05:16:48PM +0000, Sean Christopherson wrote: > >The shortlog is not at all helpful, it doesn't say anything about what actual > >functional change. > > > > KVM: x86: Don't advertise PV IPI to userspace if IPIs are virtualized > > > >On Mon, Jun 13, 2022, wangguangju wrote: > >> Commit d588bb9be1da ("KVM: VMX: enable IPI virtualization") > >> enable IPI virtualization in Intel SPR platform.There is no point > >> in using PVIPI if IPIv is supported, it doesn't work less good > >> with PVIPI than without it. > >> > >> So add a bool variable to distinguish whether to use PVIPI. > > > >Similar complaint with the changelog, it doesn't actually call out why PV IPIs > >are unwanted. > > > > Don't advertise PV IPI support to userspace if IPI virtualization is > > supported by the CPU. Hardware virtualization of IPIs more performant > > as senders do not need to exit. > > PVIPI is mainly [*] for sending multi-cast IPIs. Intel IPI virtualization > can virtualize only uni-cast IPIs. Their use cases don't overlap. So, I > don't think it makes sense to disable PVIPI if intel IPI virtualization > is supported. > > The question actually is how to deal with the exceptional case below. > Considering the migration case Sean said below, it is hard to let VM > always work in the ideal way unless KVM notifies VM of migration and VM > changes its behavior on receiving such notifications. But since x2apic > has better performance than xapic, if VM cares about performance, it can > simply switch to x2apic mode. All things considered, I think the > performance gain isn't worth the complexity added. So, I prefer to leave > it as is. > > [*]: when linux guest is in *xapic* mode, it uses PVIPI to send uni-case IPI. Hmm, there are definitely guests that run xAPIC though, even if x2apic is supported. That said, I tend to agree that trying to handle this in KVM and/or the guest kernel is going to get messy. The easiest solution is for VMMs to not advertise PV IPIs when the VM is going to predominately run on hosts with IPIv. > >That said, I'm not sure that KVM should actually hide PV_SEND_IPI. KVM still > >supports the feature, and unlike sched_info_on(), IPI virtualization is platform > >dependent and not fully controlled by software. E.g. hiding PV_SEND_IPI could > >cause problems when migrating from a platform without IPIv to a platform with IPIv, > >as a paranoid VMM might complain that an exposed feature isn't supported by KVM. > > > >There's also the question of what to do about AVIC. AVIC has many more inhibits > >than APICv, e.g. an x2APIC guest running on hardware that doesn't accelerate x2APIC > >IPIs will probably be better off with PV IPIs. > > > >Given that userspace should have read access to the module param, I'm tempted to > >say KVM should let userspace make the decision of whether or not to advertise PV > >IPIs to the guest. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI 2022-06-14 14:41 ` Sean Christopherson @ 2022-06-14 15:03 ` Chao Gao 2022-06-15 4:21 ` 答复: " Wang,Guangju 0 siblings, 1 reply; 6+ messages in thread From: Chao Gao @ 2022-06-14 15:03 UTC (permalink / raw) To: Sean Christopherson Cc: wangguangju, pbonzini, vkuznets, wanpengli, jmattson, joro, kvm, dave.hansen, tglx, mingo, x86, linux-kernel On Tue, Jun 14, 2022 at 02:41:30PM +0000, Sean Christopherson wrote: >> PVIPI is mainly [*] for sending multi-cast IPIs. Intel IPI virtualization >> can virtualize only uni-cast IPIs. Their use cases don't overlap. So, I >> don't think it makes sense to disable PVIPI if intel IPI virtualization >> is supported. >> >> The question actually is how to deal with the exceptional case below. >> Considering the migration case Sean said below, it is hard to let VM >> always work in the ideal way unless KVM notifies VM of migration and VM >> changes its behavior on receiving such notifications. But since x2apic >> has better performance than xapic, if VM cares about performance, it can >> simply switch to x2apic mode. All things considered, I think the >> performance gain isn't worth the complexity added. So, I prefer to leave >> it as is. >> >> [*]: when linux guest is in *xapic* mode, it uses PVIPI to send uni-case IPI. > >Hmm, there are definitely guests that run xAPIC though, even if x2apic is supported. > >That said, I tend to agree that trying to handle this in KVM and/or the guest kernel >is going to get messy. The easiest solution is for VMMs to not advertise PV IPIs >when the VM is going to predominately run on hosts with IPIv. But it will hurt multi-cast IPIs in VMs. IMO, a feasible solution is to add a new hint to indicate IPI virtualization is enabled and VM uses native interface (writes to ICR) to send uni-cast IPIs in xapic mode if it sees the new hint. ^ permalink raw reply [flat|nested] 6+ messages in thread
* 答复: [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI 2022-06-14 15:03 ` Chao Gao @ 2022-06-15 4:21 ` Wang,Guangju 2022-06-15 5:33 ` Chao Gao 0 siblings, 1 reply; 6+ messages in thread From: Wang,Guangju @ 2022-06-15 4:21 UTC (permalink / raw) To: Chao Gao, Sean Christopherson Cc: pbonzini@redhat.com, vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org, kvm@vger.kernel.org, dave.hansen@linux.intel.co, tglx@linutronix.de, mingo@redhat.com, x86@kernel.org, linux-kernel@vger.kernel.orga >On Mon, Jun 13, 2022 at 05:16:48PM +0000, Sean Christopherson wrote: >>The shortlog is not at all helpful, it doesn't say anything about what >>actual functional change. >> >> KVM: x86: Don't advertise PV IPI to userspace if IPIs are virtualized >> >>On Mon, Jun 13, 2022, wangguangju wrote: >>> Commit d588bb9be1da ("KVM: VMX: enable IPI virtualization") enable >> >IPI virtualization in Intel SPR platform.There is no point in using >> >PVIPI if IPIv is supported, it doesn't work less good with PVIPI than >> >without it. >>> >> >So add a bool variable to distinguish whether to use PVIPI. >> >>Similar complaint with the changelog, it doesn't actually call out why >>PV IPIs are unwanted. >> >> Don't advertise PV IPI support to userspace if IPI virtualization is > >supported by the CPU. Hardware virtualization of IPIs more performant > >as senders do not need to exit. >PVIPI is mainly [*] for sending multi-cast IPIs. Intel IPI virtualization can virtualize only uni-cast IPIs. Their use cases don't overlap. So, I don't think it makes sense to disable PVIPI if intel IPI virtualization is supported. A question, like x2apic mode, guest uses PVIPI with replace apic->send_IPI_mask to kvm_send_ipi_mask. The original function implementation is __x2apic_send_IPI_mask , and it poll each CPU to send IPI. So in this case Intel virtualization can not work? Thanks. static void __x2apic_send_IPI_mask(const struct cpumask *mask, int vector, int apic_dest) { unsigned long query_cpu; unsigned long this_cpu; unsigned long flags; /* x2apic MSRs are special and need a special fence: */ weak_wrmsr_fence(); local_irq_save(flags); this_cpu = smp_processor_id(); for_each_cpu(query_cpu, mask) { if (apic_dest == APIC_DEST_ALLBUT && this_cpu == query_cpu) continue; __x2apic_send_IPI_dest(per_cpu(x86_cpu_to_apicid, query_cpu), vector, APIC_DEST_PHYSICAL); } local_irq_restore(flags); } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 答复: [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI 2022-06-15 4:21 ` 答复: " Wang,Guangju @ 2022-06-15 5:33 ` Chao Gao 0 siblings, 0 replies; 6+ messages in thread From: Chao Gao @ 2022-06-15 5:33 UTC (permalink / raw) To: Wang,Guangju Cc: Sean Christopherson, pbonzini@redhat.com, vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org, kvm@vger.kernel.org, dave.hansen@linux.intel.co, tglx@linutronix.de, mingo@redhat.com, x86@kernel.org, linux-kernel@vger.kernel.orga On Wed, Jun 15, 2022 at 04:21:21AM +0000, Wang,Guangju wrote: >>On Mon, Jun 13, 2022 at 05:16:48PM +0000, Sean Christopherson wrote: >>>The shortlog is not at all helpful, it doesn't say anything about what >>>actual functional change. >>> >>> KVM: x86: Don't advertise PV IPI to userspace if IPIs are virtualized >>> >>>On Mon, Jun 13, 2022, wangguangju wrote: >>>> Commit d588bb9be1da ("KVM: VMX: enable IPI virtualization") enable >>> >IPI virtualization in Intel SPR platform.There is no point in using >>> >PVIPI if IPIv is supported, it doesn't work less good with PVIPI than >>> >without it. >>>> >>> >So add a bool variable to distinguish whether to use PVIPI. >>> >>>Similar complaint with the changelog, it doesn't actually call out why >>>PV IPIs are unwanted. >>> >>> Don't advertise PV IPI support to userspace if IPI virtualization is >> >supported by the CPU. Hardware virtualization of IPIs more performant >> >as senders do not need to exit. > >>PVIPI is mainly [*] for sending multi-cast IPIs. Intel IPI virtualization can virtualize only uni-cast IPIs. Their use cases don't overlap. So, I don't think it makes sense to disable PVIPI if intel IPI virtualization is supported. >A question, like x2apic mode, guest uses PVIPI with replace apic->send_IPI_mask to kvm_send_ipi_mask. The original function implementation is __x2apic_send_IPI_mask , and it poll each CPU to send IPI. So in this case >Intel virtualization can not work? Thanks. Yes, it can work. But some experiments we conducted based on a modified kvm-unit-test showed that PVIPI outperforms native ICR writes (w/ IPI virtualization) in terms of sending multi-cast (i.e., dest vCPUs >=2) IPIs ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-15 5:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1655124522-42030-1-git-send-email-wangguangju@baidu.com>
2022-06-13 17:16 ` [PATCH] KVM: x86: add a bool variable to distinguish whether to use PVIPI Sean Christopherson
2022-06-14 2:54 ` Chao Gao
2022-06-14 14:41 ` Sean Christopherson
2022-06-14 15:03 ` Chao Gao
2022-06-15 4:21 ` 答复: " Wang,Guangju
2022-06-15 5:33 ` Chao Gao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.