On Mon, 11 May 2026 at 15:58, Sean Christopherson <[email protected]> wrote:
>
> On Mon, May 11, 2026, Fuad Tabba wrote:
> > kvm_vm_release() closes vmp->fd and vmp->kvm_fd unconditionally, and
> > kvm_vm_free() calls kvm_vm_release() at teardown. A test that calls
> > kvm_vm_release() and then kvm_vm_free() without a
> > vm_recreate_with_one_vcpu() in between double-closes both FDs.
>
> That's a test bug, no?
Fair point. Let me describe the actual use case and then I'd like
your guidance on which way to fix it.
The pKVM selftests I'm preparing need to verify that protected guest
memory is zeroed when the VM is destroyed before the host regains
visibility of those pages. The test pattern is:
vm_userspace_mem_region_add(vm, ..., GPA_BASE, 1, GPA_PAGES, 0);
/* run protected guest, which writes/shares/unshares its memory */
...
TEST_ASSERT(get_proc_locked_vm_size() > GPA_SIZE); /* still pinned */
kvm_vm_release(vm); /* trigger destroy */
/* host_mem is still mmap'd in the test process; pKVM should have
* zeroed it before unpinning */
TEST_ASSERT(is_zero(region->host_mem, GPA_PAGES * PAGE_SIZE));
kvm_vm_free(vm); /* tidy up */
TEST_ASSERT(get_proc_locked_vm_size() == 0);
I used kvm_vm_release() because it's the only public API that closes
vm->fd to trigger kernel-side destruction. But the existing callers
follow it with vm_recreate_with_one_vcpu(), so the "release + later
kvm_vm_free()" path isn't exercised today.
I see three ways to make this clean:
a) This patch: kvm_vm_release() becomes idempotent for its three
FDs, matching the kvm_stats_release() idiom it already invokes.
b) Leave kvm_vm_release() as-is and add a dedicated helper, e.g.
kvm_vm_destroy_kernel(), that closes vm->fd to trigger kernel
destruction while leaving the kvm_vm struct intact for
post-destruction inspection. kvm_vm_free() learns to handle the
half-released state.
c) Something else entirely, e.g., the test should manage vm->fd
directly and not rely on library helpers for this pattern.
I lean (a). (b) doesn't really avoid the change, as kvm_vm_free()
still calls kvm_vm_release() internally, so either kvm_vm_release()
gains these guards, or kvm_vm_free() grows new "already half-released"
state, or kvm_vm_destroy_kernel() open-codes the same guarded closes
on every FD it touches. (a) is the minimal form, and it lines up
vmp->fd / vmp->kvm_fd with vmp->stats.fd, which kvm_vm_release()
already handles this way via kvm_stats_release(). But happy to take
(b) or (c) if you'd rather not broaden kvm_vm_release()'s contract.
Cheers,
/fuad
