On Fri, Nov 18, 2022 at 8:58 AM David Matlack <dmatlack@xxxxxxxxxx> wrote: > > On Fri, Nov 18, 2022 at 12:28 AM wangyanan (Y) <wangyanan55@xxxxxxxxxx> wrote: > > > > Hi David, > > > > On 2022/11/17 8:16, David Matlack wrote: > > > Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL on every halt, > > > rather than just sampling the module parameter when the VM is first > > s/first/firstly > > > created. This restore the original behavior of kvm.halt_poll_ns for VMs > > s/restore/restores > > > that have not opted into KVM_CAP_HALT_POLL. > > > > > > Notably, this change restores the ability for admins to disable or > > > change the maximum halt-polling time system wide for VMs not using > > > KVM_CAP_HALT_POLL. > > Should we add more detailed comments about relationship > > between KVM_CAP_HALT_POLL and kvm.halt_poll_ns in > > Documentation/virt/kvm/api.rst? Something like: > > "once KVM_CAP_HALT_POLL is used for a target VM, it will > > completely ignores any future changes to kvm.halt_poll_ns..." > > Yes we should. > > I will do some testing on this series today and then resend the series > as a non-RFC with the Documentation changes. > > Thanks for the reviews. Initial testing looks good but I did not have time to finish due to a bug in how our VMM is currently using KVM_CAP_HALT_POLL. I will be out all next week so I'll pick this up the week after. > > > > Reported-by: Christian Borntraeger <borntraeger@xxxxxxxxxx> > > > Fixes: acd05785e48c ("kvm: add capability for halt polling") > > > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx> > > > --- > > > include/linux/kvm_host.h | 1 + > > > virt/kvm/kvm_main.c | 27 ++++++++++++++++++++++++--- > > > 2 files changed, 25 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > > index e6e66c5e56f2..253ad055b6ad 100644 > > > --- a/include/linux/kvm_host.h > > > +++ b/include/linux/kvm_host.h > > > @@ -788,6 +788,7 @@ struct kvm { > > > struct srcu_struct srcu; > > > struct srcu_struct irq_srcu; > > > pid_t userspace_pid; > > > + bool override_halt_poll_ns; > > > unsigned int max_halt_poll_ns; > > > u32 dirty_ring_size; > > > bool vm_bugged; > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > > index 78caf19608eb..7f73ce99bd0e 100644 > > > --- a/virt/kvm/kvm_main.c > > > +++ b/virt/kvm/kvm_main.c > > > @@ -1198,8 +1198,6 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) > > > goto out_err_no_arch_destroy_vm; > > > } > > > > > > - kvm->max_halt_poll_ns = halt_poll_ns; > > > - > > > r = kvm_arch_init_vm(kvm, type); > > > if (r) > > > goto out_err_no_arch_destroy_vm; > > > @@ -3490,7 +3488,20 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start, > > > > > > static unsigned int kvm_vcpu_max_halt_poll_ns(struct kvm_vcpu *vcpu) > > > { > > > - return READ_ONCE(vcpu->kvm->max_halt_poll_ns); > > > + struct kvm *kvm = vcpu->kvm; > > > + > > > + if (kvm->override_halt_poll_ns) { > > > + /* > > > + * Ensure kvm->max_halt_poll_ns is not read before > > > + * kvm->override_halt_poll_ns. > > > + * > > > + * Pairs with the smp_wmb() when enabling KVM_CAP_HALT_POLL. > > > + */ > > > + smp_rmb(); > > > + return READ_ONCE(kvm->max_halt_poll_ns); > > > + } > > > + > > > + return READ_ONCE(halt_poll_ns); > > > } > > > > > > /* > > > @@ -4600,6 +4611,16 @@ static int kvm_vm_ioctl_enable_cap_generic(struct kvm *kvm, > > > return -EINVAL; > > > > > > kvm->max_halt_poll_ns = cap->args[0]; > > > + > > > + /* > > > + * Ensure kvm->override_halt_poll_ns does not become visible > > > + * before kvm->max_halt_poll_ns. > > > + * > > > + * Pairs with the smp_rmb() in kvm_vcpu_max_halt_poll_ns(). > > > + */ > > > + smp_wmb(); > > > + kvm->override_halt_poll_ns = true; > > > + > > > return 0; > > > } > > > case KVM_CAP_DIRTY_LOG_RING: > > Looks good to me: > > Reviewed-by: Yanan Wang <wangyanan55@xxxxxxxxxx> > > > > Thanks, > > Yanan