On 2022-11-11 22:19, Gavin Shan wrote:
Hi Marc,
On 11/11/22 11:19 PM, Marc Zyngier wrote:
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.
Thanks, it's the primary change in this patch.
#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?
Nice catch! This particular case needs to be warned explicitly. Without
the patch, kernel crash is triggered. With this patch applied, the
error
or warning is dropped silently. We either check memslot->dirty_bitmap
in mark_page_dirty_in_slot(), or check it in
kvm_arch_allow_write_without_running_vcpu().
I personally the later one. Let me post a formal patch on top of your
'next' branch where the commit log will be improved accordingly.
I personally prefer this memslot->dirty_bitmap, as this is
a completely legal case (the VMM may not want to track the
ITS dirtying).
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
