Re: KVM: Warn if mark_page_dirty() is called without an active vCPU

From: Sean Christopherson
Date: Mon Nov 22 2021 - 12:02:07 EST


On Sat, Nov 20, 2021, David Woodhouse wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6c5083f2eb50..72c6453bcef4 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3020,12 +3020,17 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
> struct kvm_memory_slot *memslot,
> gfn_t gfn)
> {
> + struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> +
> + if (WARN_ON_ONCE(!vcpu) || WARN_ON_ONCE(vcpu->kvm != kvm))

Maybe use KVM_BUG_ON? And two separate WARNs are probably overkill.

if (KVM_BUG_ON(!vcpu || vcpu->kvm != kvm, kvm))


I'd also prefer to not retrieve the vCPU in the dirty_bitmap path, at least not
until it's necessary (for the proposed dirty quota throttling), though that's not
a strong preference.

> + return;
> +
> if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
> unsigned long rel_gfn = gfn - memslot->base_gfn;
> u32 slot = (memslot->as_id << 16) | memslot->id;
>
> if (kvm->dirty_ring_size)
> - kvm_dirty_ring_push(kvm_dirty_ring_get(kvm),
> + kvm_dirty_ring_push(&vcpu->dirty_ring,
> slot, rel_gfn);

This can now squeeze on a single line.

kvm_dirty_ring_push(&vcpu->dirty_ring, slot, rel_gfn);

> else
> set_bit_le(rel_gfn, memslot->dirty_bitmap);
> --
> 2.31.1
>