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

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 01, 2026, Leonardo Bras wrote:
> On Thu, Oct 01, 2026 at 03:46:51PM +0100, Leonardo Bras wrote:
> > On Wed, Sep 30, 2026 at 01:31:25PM -0700, Sean Christopherson wrote:
> > > On Tue, Sep 29, 2026, Leonardo Bras wrote:
> > > >  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).
> > 
> > Sure, if you think it's useful :)
> 
> Oh, vm_check_cap() is different than kvm_has_cap()/kvm_check_cap():
> IIUC, the vm* version will check if the extension is enabled in the current 
> VM,

No, the VM-scoped version checks if the VM *can* support the capability in its
current configuration, e.g. some capabilities are fundamentally incompatible with
certain VM types.

> while the kvm* version will open a new /dev/kvm fd and check the 
> extension there, probably meaning the CAP is available in the system.
> 
> So, maybe we would need a slightly different approach?
> I.E. have the kvm_get_dirty_ring_cap() to use the VM version, and 
> have dirty_ring_supported() to start the new /dev/kvm fd and free it later?
> 
> What do you think?

It's not necessary in this case.  All KVM_CHECK_EXTENSION calls feed into
kvm_vm_ioctl_check_extension_generic(), the only difference is that the @kvm
pointer will be NULL when called on /dev/kvm.  While some capabilities do behave
differently if @kvm is valid, the majority do not, including the dirty ring ones:

	case KVM_CAP_DIRTY_LOG_RING:
#ifdef CONFIG_HAVE_KVM_DIRTY_RING_TSO
		return KVM_DIRTY_RING_MAX_ENTRIES * sizeof(struct kvm_dirty_gfn);
#else
		return 0;
#endif
	case KVM_CAP_DIRTY_LOG_RING_ACQ_REL:
#ifdef CONFIG_HAVE_KVM_DIRTY_RING_ACQ_REL
		return KVM_DIRTY_RING_MAX_ENTRIES * sizeof(struct kvm_dirty_gfn);
#else
		return 0;
#endif

I.e. the VM version and /dev/kvm version will always yield the same result.  Very
theoretically that could change, but it's unlikely.  And we can always update
KVM selftests in the unlikely case a specific VM type comes along that doesn't
support the dirty ring even when the broader architectures does.