Re: [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling

[Sean Christopherson <seanjc@google.com>]

On Tue, Sep 29, 2026, Leonardo Bras wrote:
> As of today, trying to enable dirty-ring with a size bigger than the
> maximum will return an "argument list too long" error.
> 
> Change vm_enable_dirty_ring() to get the maximum size, then compare it to
> the desired size before enabling. If the value is invalid, print a more
> precise error message.
> 
> Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 9ddc047d5c27..15a671b71553 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -168,24 +168,37 @@ unsigned int kvm_check_cap(long cap)
>  	ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, (void *)cap);
>  	TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret));
>  
>  	kvm_free_fd(kvm_fd);
>  
>  	return (unsigned int)ret;
>  }
>  
>  void vm_enable_dirty_ring(struct kvm_vm *vm, u32 ring_size)
>  {
> -	if (vm_check_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
> -		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL, ring_size);
> -	else
> -		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING, ring_size);
> +	long cap = KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
> +	int max_size = vm_check_cap(vm, cap);
> +
> +	if (!max_size) {
> +		cap = KVM_CAP_DIRTY_LOG_RING;
> +		max_size = vm_check_cap(vm, cap);
> +	}
> +
> +	TEST_ASSERT(max_size > 0,

Rather than open code this, which is kinda sorta going to show up in multiple
places, what if we do this as a prep patch?  Then vm_enable_dirty_ring() can use
kvm_get_dirty_ring_cap() (completely untested).

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index af5eb0334a74..558a71d631da 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -291,8 +291,7 @@ static void default_after_vcpu_run(struct kvm_vcpu *vcpu)
 
 static bool dirty_ring_supported(void)
 {
-	return (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING) ||
-		kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL));
+	return kvm_get_dirty_ring_cap();
 }
 
 static void dirty_ring_create_vm_done(struct kvm_vm *vm)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index e3b122719262..c08d1581221e 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -339,6 +339,17 @@ static inline bool kvm_has_cap(long cap)
 	return kvm_check_cap(cap);
 }
 
+static inline long kvm_get_dirty_ring_cap(void)
+{
+	if (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
+		return KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
+
+	if (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING))
+		return KVM_CAP_DIRTY_LOG_RING;
+
+	return 0;	
+}
+
 /*
  * Use the "inner", double-underscore macro when reporting errors from within
  * other macros so that the name of ioctl() and not its literal numeric value


> "Dirty-ring not supported in this kernel\n");

"this kernel" could be misleading, some architectures simply don't support the
dirty ring.

> +	TEST_ASSERT(ring_size <= max_size && is_power_of_2(ring_size) &&
> +		    ring_size >= getpagesize(),
> +		    "Invalid dirty-ring size: Should be a power of two "
> +		    "between %lu and %lu entries\n",

Don't wrap strings.  The "Invalid dirty-ring size:" part is redudant with the
expressions, just drop that to shorten things.  This should also spit out the
requested ring_size.  Stating the range as a number of entries is also confusing;
as a debugger, I don't want to have to go look at the size of kvm_dirty_gfn to
understand why ring_size is invalid.

And maybe split up the asserts?  If the goal is to make it easier for developers
to know when they messed up, might as well make it as easy as possible.  E.g.
leaning on the above diff, something like this?

	long cap = kvm_get_dirty_ring_cap();

	TEST_ASSERT(cap, "Dirty-ring not supported");

	TEST_ASSERT(is_power_of_2(ring_size),
		    "Dirty-ring size '0x%x' must be a power-of-2", ring_size);
	TEST_ASSERT(ring_size >= getpagesize(),
		    "Dirty-ring size '0x%x' must be at least one (host) page", ring_size);
	TEST_ASSERT(ring_size >= vm_check_cap(vm, cap),
		    "Dirty-ring size '0x%x' is larger than KVM's limit of '0x%x'",
		    ring_size, vm_check_cap(vm, cap));

	vm_enable_cap(vm, cap, ring_size);
	vm->dirty_ring_size = ring_size;

> +		    getpagesize() / sizeof(struct kvm_dirty_gfn),
> +		    max_size / sizeof(struct kvm_dirty_gfn));
> +
> +	vm_enable_cap(vm, cap, ring_size);
>  	vm->dirty_ring_size = ring_size;
>  }
>  
>  static void vm_open(struct kvm_vm *vm)
>  {
>  	vm->kvm_fd = _open_kvm_dev_path_or_exit(O_RDWR);
>  
>  	TEST_REQUIRE(kvm_has_cap(KVM_CAP_IMMEDIATE_EXIT));
>  
>  	vm->fd = __kvm_ioctl(vm->kvm_fd, KVM_CREATE_VM, (void *)vm->type);
> -- 
> 2.55.0
>