On Thu, Oct 01, 2026 at 02:56:38PM -0700, Sean Christopherson wrote:
> 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.
All right then, thanks!
Leo
