lkml.org 
[lkml]   [2022]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 1/5] KVM: arm64: Enable ring-based dirty memory tracking
    On Tue, Aug 23, 2022 at 03:22:17PM +1000, Gavin Shan wrote:
    > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
    > > index 986cee6fbc7f..0b41feb6fb7d 100644
    > > --- a/arch/arm64/kvm/arm.c
    > > +++ b/arch/arm64/kvm/arm.c
    > > @@ -747,6 +747,12 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
    > > if (kvm_check_request(KVM_REQ_SUSPEND, vcpu))
    > > return kvm_vcpu_suspend(vcpu);
    > > +
    > > + if (kvm_check_request(KVM_REQ_RING_SOFT_FULL, vcpu)) {
    > > + vcpu->run->exit_reason = KVM_EXIT_DIRTY_RING_FULL;
    > > + trace_kvm_dirty_ring_exit(vcpu);
    > > + return 0;
    > > + }
    > > }
    > > return 1;
    > > diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
    > > index f4c2a6eb1666..08b2f01164fa 100644
    > > --- a/virt/kvm/dirty_ring.c
    > > +++ b/virt/kvm/dirty_ring.c
    > > @@ -149,6 +149,7 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring)
    > > void kvm_dirty_ring_push(struct kvm_dirty_ring *ring, u32 slot, u64 offset)
    > > {
    > > + struct kvm_vcpu *vcpu = container_of(ring, struct kvm_vcpu, dirty_ring);
    > > struct kvm_dirty_gfn *entry;
    > > /* It should never get full */
    > > @@ -166,6 +167,9 @@ void kvm_dirty_ring_push(struct kvm_dirty_ring *ring, u32 slot, u64 offset)
    > > kvm_dirty_gfn_set_dirtied(entry);
    > > ring->dirty_index++;
    > > trace_kvm_dirty_ring_push(ring, slot, offset);
    > > +
    > > + if (kvm_dirty_ring_soft_full(vcpu))
    > > + kvm_make_request(KVM_REQ_RING_SOFT_FULL, vcpu);
    > > }
    > > struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring, u32 offset)
    > >
    >
    > Ok, thanks for the details, Marc. I will adopt your code in next revision :)

    Note that there can be a slight difference with the old/new code, in that
    an (especially malicious) userapp can logically ignore the DIRTY_RING_FULL
    vmexit and keep kicking VCPU_RUN with the new code.

    Unlike the old code, the 2nd/3rd/... KVM_RUN will still run in the new code
    until the next dirty pfn being pushed to the ring, then it'll request ring
    full exit again.

    Each time it exits the ring grows 1.

    At last iiuc it can easily hit the ring full and trigger the warning at the
    entry of kvm_dirty_ring_push():

    /* It should never get full */
    WARN_ON_ONCE(kvm_dirty_ring_full(ring));

    We did that because kvm_dirty_ring_push() was previously designed to not be
    able to fail at all (e.g., in the old bitmap world we never will fail too).
    We can't because we can't lose any dirty page or migration could silently
    fail too (consider when we do user exit due to ring full and migration just
    completed; there could be unsynced pages on src/dst).

    So even though the old approach will need to read kvm->dirty_ring_size for
    every entrance which is a pity, it will avoid issue above.

    Side note: for x86 the dirty ring check was put at the entrance not because
    it needs to be the highest priority - it should really be the same when
    check kvm requests. It's just that it'll be the fastest way to fail
    properly if needed before loading mmu, disabling preemption/irq, etc.

    Thanks,

    --
    Peter Xu

    \
     
     \ /
      Last update: 2022-08-23 19:14    [W:2.993 / U:0.220 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site