[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YwPRO0r2sfzcbgyz@xz-m1.local>
Date: Mon, 22 Aug 2022 14:55:55 -0400
From: Peter Xu <peterx@...hat.com>
To: Gavin Shan <gshan@...hat.com>
Cc: Marc Zyngier <maz@...nel.org>, kvmarm@...ts.cs.columbia.edu,
linux-arm-kernel@...ts.infradead.org, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kselftest@...r.kernel.org, pbonzini@...hat.com,
corbet@....net, james.morse@....com, alexandru.elisei@....com,
suzuki.poulose@....com, oliver.upton@...ux.dev,
catalin.marinas@....com, will@...nel.org, shuah@...nel.org,
seanjc@...gle.com, drjones@...hat.com, dmatlack@...gle.com,
bgardon@...gle.com, ricarkol@...gle.com, zhenyzha@...hat.com,
shan.gavin@...il.com
Subject: Re: [PATCH v1 1/5] KVM: arm64: Enable ring-based dirty memory
tracking
Hi, Gavin,
On Mon, Aug 22, 2022 at 11:58:20AM +1000, Gavin Shan wrote:
> > For context, the documentation says:
> >
> > <quote>
> > - if KVM_CAP_DIRTY_LOG_RING is available, a number of pages at
> > KVM_DIRTY_LOG_PAGE_OFFSET * PAGE_SIZE. [...]
> > </quote>
> >
> > What is the reason for picking this particular value?
> >
>
> It's inherited from x86. I don't think it has to be this particular value.
> The value is used to distinguish the region's owners like kvm_run, KVM_PIO_PAGE_OFFSET,
> KVM_COALESCED_MMIO_PAGE_OFFSET, and KVM_DIRTY_LOG_PAGE_OFFSET.
>
> How about to have 2 for KVM_DIRTY_LOG_PAGE_OFFSET in next revision?
> The virtual area is cheap, I guess it's also nice to use x86's
> pattern to have 64 for KVM_DIRTY_LOG_PAGE_OFFSET.
>
> #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
> #define KVM_DIRTY_LOG_PAGE_OFFSET 2
It was chosen not to be continuous of previous used offset because it'll be
the 1st vcpu region that can cover multiple & dynamic number of pages. I
wanted to leave the 3-63 (x86 used offset 2 already) for small fields so
they can be continuous, which looks a little bit cleaner.
Currently how many pages it'll use depends on the size set by the user,
though there's a max size limited by KVM_DIRTY_RING_MAX_ENTRIES, which is a
maximum of 1MB memory.
So I think setting it to 2 is okay, as long as we keep the rest 1MB address
space for the per-vcpu ring structure, so any new vcpu fields (even if only
1 page will be needed) need to be after that maximum size of the ring.
Thanks,
--
Peter Xu
Powered by blists - more mailing lists