[RFC PATCH 0/3] KVM: Restore original behavior of kvm.halt_poll_ns

[RFC PATCH 0/3] KVM: Restore original behavior of kvm.halt_poll_ns

[David Matlack]

This series restores the original behavior of the module parameter
kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL. This should allow
admins to administer VMs not using KVM_CAP_HALT_POLL just as they were
before this capability was introduced.

VMs that use KVM_CAP_HALT_POLL can be administered through userspace
changes that invoke KVM_CAP_HALT_POLL (i.e. the capability can be
invoked multiple times to change the max halt poll time).

If any admin needs a system-wide override of KVM_CAP_HALT_POLL we can
add that through a new mechanism (e.g. a new bool module parameter).

Compile-tested only. I want to get feedback from Christian and Yunan if
this approach would address their concerns.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yanan Wang <wangyanan55@huawei.com>

David Matlack (3):
  KVM: Cap vcpu->halt_poll_ns before halting rather than after
  KVM: Avoid re-reading kvm->max_halt_poll_ns during halt-polling
  KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 52 ++++++++++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 10 deletions(-)


base-commit: d663b8a285986072428a6a145e5994bc275df994
-- 
2.38.1.431.g37b22c650d-goog

[RFC PATCH 1/3] KVM: Cap vcpu->halt_poll_ns before halting rather than after

[David Matlack]

Cap vcpu->halt_poll_ns based on the max halt polling time just before
halting, rather than after the last halt. This arguably provides better
accuracy if an admin disables halt polling in between halts, although
the improvement is nominal.

A side-effect of this change is that grow_halt_poll_ns() no longer needs
to access vcpu->kvm->max_halt_poll_ns, which will be useful in a future
commit where the max halt polling time can come from the module parameter
halt_poll_ns instead.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 virt/kvm/kvm_main.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 43bbe4fde078..4b868f33c45d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3385,9 +3385,6 @@ static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
 	if (val < grow_start)
 		val = grow_start;
 
-	if (val > vcpu->kvm->max_halt_poll_ns)
-		val = vcpu->kvm->max_halt_poll_ns;
-
 	vcpu->halt_poll_ns = val;
 out:
 	trace_kvm_halt_poll_ns_grow(vcpu->vcpu_id, val, old);
@@ -3500,11 +3497,16 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
 void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
 {
 	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
-	bool do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
 	ktime_t start, cur, poll_end;
 	bool waited = false;
+	bool do_halt_poll;
 	u64 halt_ns;
 
+	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
+		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
+
+	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
+
 	start = cur = poll_end = ktime_get();
 	if (do_halt_poll) {
 		ktime_t stop = ktime_add_ns(start, vcpu->halt_poll_ns);
-- 
2.38.1.431.g37b22c650d-goog

[RFC PATCH 2/3] KVM: Avoid re-reading kvm->max_halt_poll_ns during halt-polling

[David Matlack]

Avoid re-reading kvm->max_halt_poll_ns multiple times during
halt-polling except when it is explicitly useful, e.g. to check if the
max time changed across a halt. kvm->max_halt_poll_ns can be changed at
any time by userspace via KVM_CAP_HALT_POLL.

This bug is unlikely to cause any serious side-effects. In the worst
case one halt polls for shorter or longer than it should, and then is
fixed up on the next halt. Furthmore, this is still possible since
kvm->max_halt_poll_ns are not synchronized with halts.

Fixes: acd05785e48c ("kvm: add capability for halt polling")
Signed-off-by: David Matlack <dmatlack@google.com>
---
 virt/kvm/kvm_main.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4b868f33c45d..78caf19608eb 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3488,6 +3488,11 @@ 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);
+}
+
 /*
  * Emulate a vCPU halt condition, e.g. HLT on x86, WFI on arm, etc...  If halt
  * polling is enabled, busy wait for a short time before blocking to avoid the
@@ -3496,14 +3501,15 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
  */
 void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
 {
+	unsigned int max_halt_poll_ns = kvm_vcpu_max_halt_poll_ns(vcpu);
 	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
 	ktime_t start, cur, poll_end;
 	bool waited = false;
 	bool do_halt_poll;
 	u64 halt_ns;
 
-	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
-		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
+	if (vcpu->halt_poll_ns > max_halt_poll_ns)
+		vcpu->halt_poll_ns = max_halt_poll_ns;
 
 	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
 
@@ -3545,18 +3551,21 @@ void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
 		update_halt_poll_stats(vcpu, start, poll_end, !waited);
 
 	if (halt_poll_allowed) {
+		/* Recompute the max halt poll time in case it changed. */
+		max_halt_poll_ns = kvm_vcpu_max_halt_poll_ns(vcpu);
+
 		if (!vcpu_valid_wakeup(vcpu)) {
 			shrink_halt_poll_ns(vcpu);
-		} else if (vcpu->kvm->max_halt_poll_ns) {
+		} else if (max_halt_poll_ns) {
 			if (halt_ns <= vcpu->halt_poll_ns)
 				;
 			/* we had a long block, shrink polling */
 			else if (vcpu->halt_poll_ns &&
-				 halt_ns > vcpu->kvm->max_halt_poll_ns)
+				 halt_ns > max_halt_poll_ns)
 				shrink_halt_poll_ns(vcpu);
 			/* we had a short halt and our poll time is too small */
-			else if (vcpu->halt_poll_ns < vcpu->kvm->max_halt_poll_ns &&
-				 halt_ns < vcpu->kvm->max_halt_poll_ns)
+			else if (vcpu->halt_poll_ns < max_halt_poll_ns &&
+				 halt_ns < max_halt_poll_ns)
 				grow_halt_poll_ns(vcpu);
 		} else {
 			vcpu->halt_poll_ns = 0;
-- 
2.38.1.431.g37b22c650d-goog

[RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[David Matlack]

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
created. This restore the original behavior of kvm.halt_poll_ns for VMs
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.

Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Fixes: acd05785e48c ("kvm: add capability for halt polling")
Signed-off-by: David Matlack <dmatlack@google.com>
---
 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:
-- 
2.38.1.431.g37b22c650d-goog

Re: [RFC PATCH 1/3] KVM: Cap vcpu->halt_poll_ns before halting rather than after

[Christian Borntraeger]

Am 17.11.22 um 01:16 schrieb David Matlack:
> Cap vcpu->halt_poll_ns based on the max halt polling time just before
> halting, rather than after the last halt. This arguably provides better
> accuracy if an admin disables halt polling in between halts, although
> the improvement is nominal.
> 
> A side-effect of this change is that grow_halt_poll_ns() no longer needs
> to access vcpu->kvm->max_halt_poll_ns, which will be useful in a future
> commit where the max halt polling time can come from the module parameter
> halt_poll_ns instead.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>

Looks sane

Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>


> ---
>   virt/kvm/kvm_main.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 43bbe4fde078..4b868f33c45d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3385,9 +3385,6 @@ static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
>   	if (val < grow_start)
>   		val = grow_start;
>   
> -	if (val > vcpu->kvm->max_halt_poll_ns)
> -		val = vcpu->kvm->max_halt_poll_ns;
> -
>   	vcpu->halt_poll_ns = val;
>   out:
>   	trace_kvm_halt_poll_ns_grow(vcpu->vcpu_id, val, old);
> @@ -3500,11 +3497,16 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
>   void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   {
>   	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
> -	bool do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
>   	ktime_t start, cur, poll_end;
>   	bool waited = false;
> +	bool do_halt_poll;
>   	u64 halt_ns;
>   
> +	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
> +		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
> +
> +	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
> +
>   	start = cur = poll_end = ktime_get();
>   	if (do_halt_poll) {
>   		ktime_t stop = ktime_add_ns(start, vcpu->halt_poll_ns);

Re: [RFC PATCH 1/3] KVM: Cap vcpu->halt_poll_ns before halting rather than after

[wangyanan (Y)]

On 2022/11/17 8:16, David Matlack wrote:
> Cap vcpu->halt_poll_ns based on the max halt polling time just before
> halting, rather than after the last halt. This arguably provides better
> accuracy if an admin disables halt polling in between halts, although
> the improvement is nominal.
>
> A side-effect of this change is that grow_halt_poll_ns() no longer needs
> to access vcpu->kvm->max_halt_poll_ns, which will be useful in a future
> commit where the max halt polling time can come from the module parameter
> halt_poll_ns instead.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>   virt/kvm/kvm_main.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 43bbe4fde078..4b868f33c45d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3385,9 +3385,6 @@ static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
>   	if (val < grow_start)
>   		val = grow_start;
>   
> -	if (val > vcpu->kvm->max_halt_poll_ns)
> -		val = vcpu->kvm->max_halt_poll_ns;
> -
>   	vcpu->halt_poll_ns = val;
>   out:
>   	trace_kvm_halt_poll_ns_grow(vcpu->vcpu_id, val, old);
> @@ -3500,11 +3497,16 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
>   void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   {
>   	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
> -	bool do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
>   	ktime_t start, cur, poll_end;
>   	bool waited = false;
> +	bool do_halt_poll;
>   	u64 halt_ns;
>   
> +	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
> +		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
> +
> +	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
> +
>   	start = cur = poll_end = ktime_get();
>   	if (do_halt_poll) {
>   		ktime_t stop = ktime_add_ns(start, vcpu->halt_poll_ns);
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan

Re: [RFC PATCH 2/3] KVM: Avoid re-reading kvm->max_halt_poll_ns during halt-polling

[wangyanan (Y)]

On 2022/11/17 8:16, David Matlack wrote:
> Avoid re-reading kvm->max_halt_poll_ns multiple times during
> halt-polling except when it is explicitly useful, e.g. to check if the
> max time changed across a halt. kvm->max_halt_poll_ns can be changed at
> any time by userspace via KVM_CAP_HALT_POLL.
>
> This bug is unlikely to cause any serious side-effects. In the worst
> case one halt polls for shorter or longer than it should, and then is
> fixed up on the next halt. Furthmore, this is still possible since
s/Furthmore/Furthermore
> kvm->max_halt_poll_ns are not synchronized with halts.
>
> Fixes: acd05785e48c ("kvm: add capability for halt polling")
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>   virt/kvm/kvm_main.c | 21 +++++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
Looks good to me:
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 4b868f33c45d..78caf19608eb 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3488,6 +3488,11 @@ 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);
> +}
> +
>   /*
>    * Emulate a vCPU halt condition, e.g. HLT on x86, WFI on arm, etc...  If halt
>    * polling is enabled, busy wait for a short time before blocking to avoid the
> @@ -3496,14 +3501,15 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
>    */
>   void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   {
> +	unsigned int max_halt_poll_ns = kvm_vcpu_max_halt_poll_ns(vcpu);
>   	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
>   	ktime_t start, cur, poll_end;
>   	bool waited = false;
>   	bool do_halt_poll;
>   	u64 halt_ns;
>   
> -	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
> -		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
> +	if (vcpu->halt_poll_ns > max_halt_poll_ns)
> +		vcpu->halt_poll_ns = max_halt_poll_ns;
>   
>   	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
>   
> @@ -3545,18 +3551,21 @@ void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   		update_halt_poll_stats(vcpu, start, poll_end, !waited);
>   
>   	if (halt_poll_allowed) {
> +		/* Recompute the max halt poll time in case it changed. */
> +		max_halt_poll_ns = kvm_vcpu_max_halt_poll_ns(vcpu);
> +
>   		if (!vcpu_valid_wakeup(vcpu)) {
>   			shrink_halt_poll_ns(vcpu);
> -		} else if (vcpu->kvm->max_halt_poll_ns) {
> +		} else if (max_halt_poll_ns) {
>   			if (halt_ns <= vcpu->halt_poll_ns)
>   				;
>   			/* we had a long block, shrink polling */
>   			else if (vcpu->halt_poll_ns &&
> -				 halt_ns > vcpu->kvm->max_halt_poll_ns)
> +				 halt_ns > max_halt_poll_ns)
>   				shrink_halt_poll_ns(vcpu);
>   			/* we had a short halt and our poll time is too small */
> -			else if (vcpu->halt_poll_ns < vcpu->kvm->max_halt_poll_ns &&
> -				 halt_ns < vcpu->kvm->max_halt_poll_ns)
> +			else if (vcpu->halt_poll_ns < max_halt_poll_ns &&
> +				 halt_ns < max_halt_poll_ns)
>   				grow_halt_poll_ns(vcpu);
>   		} else {
>   			vcpu->halt_poll_ns = 0;

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[wangyanan (Y)]

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..."
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: acd05785e48c ("kvm: add capability for halt polling")
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>   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@huawei.com>

Thanks,
Yanan

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[David Matlack]

On Fri, Nov 18, 2022 at 12:28 AM wangyanan (Y) <wangyanan55@huawei.com> 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.

> > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Fixes: acd05785e48c ("kvm: add capability for halt polling")
> > Signed-off-by: David Matlack <dmatlack@google.com>
> > ---
> >   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@huawei.com>
>
> Thanks,
> Yanan

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[David Matlack]

On Fri, Nov 18, 2022 at 8:58 AM David Matlack <dmatlack@google.com> wrote:
>
> On Fri, Nov 18, 2022 at 12:28 AM wangyanan (Y) <wangyanan55@huawei.com> 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@de.ibm.com>
> > > Fixes: acd05785e48c ("kvm: add capability for halt polling")
> > > Signed-off-by: David Matlack <dmatlack@google.com>
> > > ---
> > >   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@huawei.com>
> >
> > Thanks,
> > Yanan

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[wangyanan (Y)]

On 2022/11/19 9:25, David Matlack wrote:
> On Fri, Nov 18, 2022 at 8:58 AM David Matlack <dmatlack@google.com> wrote:
>> On Fri, Nov 18, 2022 at 12:28 AM wangyanan (Y) <wangyanan55@huawei.com> 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.
OK, thanks.

Yanan
>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> Fixes: acd05785e48c ("kvm: add capability for halt polling")
>>>> Signed-off-by: David Matlack <dmatlack@google.com>
>>>> ---
>>>>    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@huawei.com>
>>>
>>> Thanks,
>>> Yanan
> .

Re: [RFC PATCH 2/3] KVM: Avoid re-reading kvm->max_halt_poll_ns during halt-polling

[Christian Borntraeger]

Am 17.11.22 um 01:16 schrieb David Matlack:
> Avoid re-reading kvm->max_halt_poll_ns multiple times during
> halt-polling except when it is explicitly useful, e.g. to check if the
> max time changed across a halt. kvm->max_halt_poll_ns can be changed at
> any time by userspace via KVM_CAP_HALT_POLL.
> 
> This bug is unlikely to cause any serious side-effects. In the worst
> case one halt polls for shorter or longer than it should, and then is
> fixed up on the next halt. Furthmore, this is still possible since
> kvm->max_halt_poll_ns are not synchronized with halts.
> 
> Fixes: acd05785e48c ("kvm: add capability for halt polling")
> Signed-off-by: David Matlack <dmatlack@google.com>

Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>   virt/kvm/kvm_main.c | 21 +++++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 4b868f33c45d..78caf19608eb 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3488,6 +3488,11 @@ 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);
> +}
> +
>   /*
>    * Emulate a vCPU halt condition, e.g. HLT on x86, WFI on arm, etc...  If halt
>    * polling is enabled, busy wait for a short time before blocking to avoid the
> @@ -3496,14 +3501,15 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
>    */
>   void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   {
> +	unsigned int max_halt_poll_ns = kvm_vcpu_max_halt_poll_ns(vcpu);
>   	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
>   	ktime_t start, cur, poll_end;
>   	bool waited = false;
>   	bool do_halt_poll;
>   	u64 halt_ns;
>   
> -	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
> -		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
> +	if (vcpu->halt_poll_ns > max_halt_poll_ns)
> +		vcpu->halt_poll_ns = max_halt_poll_ns;
>   
>   	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
>   
> @@ -3545,18 +3551,21 @@ void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   		update_halt_poll_stats(vcpu, start, poll_end, !waited);
>   
>   	if (halt_poll_allowed) {
> +		/* Recompute the max halt poll time in case it changed. */
> +		max_halt_poll_ns = kvm_vcpu_max_halt_poll_ns(vcpu);
> +
>   		if (!vcpu_valid_wakeup(vcpu)) {
>   			shrink_halt_poll_ns(vcpu);
> -		} else if (vcpu->kvm->max_halt_poll_ns) {
> +		} else if (max_halt_poll_ns) {
>   			if (halt_ns <= vcpu->halt_poll_ns)
>   				;
>   			/* we had a long block, shrink polling */
>   			else if (vcpu->halt_poll_ns &&
> -				 halt_ns > vcpu->kvm->max_halt_poll_ns)
> +				 halt_ns > max_halt_poll_ns)
>   				shrink_halt_poll_ns(vcpu);
>   			/* we had a short halt and our poll time is too small */
> -			else if (vcpu->halt_poll_ns < vcpu->kvm->max_halt_poll_ns &&
> -				 halt_ns < vcpu->kvm->max_halt_poll_ns)
> +			else if (vcpu->halt_poll_ns < max_halt_poll_ns &&
> +				 halt_ns < max_halt_poll_ns)
>   				grow_halt_poll_ns(vcpu);
>   		} else {
>   			vcpu->halt_poll_ns = 0;

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[Christian Borntraeger]

Am 17.11.22 um 01:16 schrieb David Matlack:
> 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
> created. This restore the original behavior of kvm.halt_poll_ns for VMs
> 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.
> 
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Fixes: acd05785e48c ("kvm: add capability for halt polling")
> Signed-off-by: David Matlack <dmatlack@google.com>

Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

One thing. This does not apply without the other 2 patches. Not sure
if we want to redo this somehow to allow for stable backports?


> ---
>   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:

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[Christian Borntraeger]

Am 21.11.22 um 10:22 schrieb Christian Borntraeger:
> Am 17.11.22 um 01:16 schrieb David Matlack:
>> 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
>> created. This restore the original behavior of kvm.halt_poll_ns for VMs
>> 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.
>>
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Fixes: acd05785e48c ("kvm: add capability for halt polling")
>> Signed-off-by: David Matlack <dmatlack@google.com>
> 
> Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> One thing. This does not apply without the other 2 patches. Not sure
> if we want to redo this somehow to allow for stable backports?


One option would be that Paolo, when applying uses the stable
syntax as outlined in
Documentation/process/stable-kernel-rules.rst
for dependencies, e.g.

      Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle
      Cc: <stable@vger.kernel.org> # 3.3.x: 1b9508f: sched: Rate-limit newidle
      Cc: <stable@vger.kernel.org> # 3.3.x: fd21073: sched: Fix affinity logic

Re: [RFC PATCH 3/3] KVM: Obey kvm.halt_poll_ns in VMs not using KVM_CAP_HALT_POLL

[David Matlack]

On Sun, Nov 20, 2022 at 5:42 PM wangyanan (Y) <wangyanan55@huawei.com> wrote:
>
> On 2022/11/19 9:25, David Matlack wrote:
> > On Fri, Nov 18, 2022 at 8:58 AM David Matlack <dmatlack@google.com> wrote:
> >> On Fri, Nov 18, 2022 at 12:28 AM wangyanan (Y) <wangyanan55@huawei.com> 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.
> OK, thanks.

I see that Linus already merged these patches into 6.1-rc7, so I've
sent the Documentation changes as a separate series.

https://lore.kernel.org/kvm/20221201195249.3369720-1-dmatlack@google.com/