On Thu, 10 Nov 2022 23:47:41 +0000,
Gavin Shan <gshan@xxxxxxxxxx> wrote:
>
> commit b05377ecbe003f12c8b79846fa3a300401dcab68 (HEAD -> kvm/arm64_dirtyring)
> Author: Gavin Shan <gshan@xxxxxxxxxx>
> Date: Fri Nov 11 07:13:12 2022 +0800
>
> KVM: Push dirty information unconditionally to backup bitmap
> In mark_page_dirty_in_slot(), we bail out when no running vcpu
> exists and
> a running vcpu context is strictly required by architecture. It may cause
> backwards compatible issue. Currently, saving vgic/its tables is the only
> case where no running vcpu context is required. We may have other unknown
> cases where no running vcpu context exists and it's reported by the warning
> message. For this, the application is going to enable the backup bitmap for
> the unknown cases. However, the dirty information can't be pushed to the
> backup bitmap even though the backup bitmap has been enabled, until the
> unknown cases are added to the allowed list of non-running vcpu context
> with extra code changes to the host kernel.
> In order to make the new application, where the backup bitmap
> has been
> enabled, to work with the unchanged host, we continue to push the dirty
> information to the backup bitmap instead of bailing out early.
> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Signed-off-by: Gavin Shan <gshan@xxxxxxxxxx>
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 2719e10dd37d..03e6a38094c1 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3308,8 +3308,7 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
> if (WARN_ON_ONCE(vcpu && vcpu->kvm != kvm))
> return;
> - if
> (WARN_ON_ONCE(!kvm_arch_allow_write_without_running_vcpu(kvm) &&
> !vcpu))
> - return;
> + WARN_ON_ONCE(!vcpu && !kvm_arch_allow_write_without_running_vcpu(kvm));
I'm happy with this.
> #endif
> if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
> @@ -3318,7 +3317,7 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
> if (kvm->dirty_ring_size && vcpu)
> kvm_dirty_ring_push(vcpu, slot, rel_gfn);
> - else
> + else if (memslot->dirty_bitmap)
> set_bit_le(rel_gfn, memslot->dirty_bitmap);
But that I don't get. Or rather, I don't get the commit message that
matches this hunk. Do we want to catch the case where all of the
following are true:
- we don't have a vcpu,
- we're allowed to log non-vcpu dirtying
- we *only* have the ring?
If so, can we please capture that in the commit message?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
