On Sat, Sep 24, 2022 at 12:26:53PM +0100, Marc Zyngier wrote:
> On Sat, 24 Sep 2022 09:51:39 +0100,
> Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >
> > On Fri, 23 Sep 2022 19:26:18 +0100,
> > Peter Xu <peterx@xxxxxxxxxx> wrote:
> > >
> > > On Fri, Sep 23, 2022 at 03:28:34PM +0100, Marc Zyngier wrote:
> > > > On Thu, 22 Sep 2022 22:48:19 +0100,
> > > > Peter Xu <peterx@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Thu, Sep 22, 2022 at 06:01:29PM +0100, Marc Zyngier wrote:
> > > > > > In order to differenciate between architectures that require no extra
> > > > > > synchronisation when accessing the dirty ring and those who do,
> > > > > > add a new capability (KVM_CAP_DIRTY_LOG_RING_ORDERED) that identify
> > > > > > the latter sort. TSO architectures can obviously advertise both, while
> > > > > > relaxed architectures most only advertise the ORDERED version.
> > > > > >
> > > > > > Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > > > > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> > > > > > ---
> > > > > > include/linux/kvm_dirty_ring.h | 6 +++---
> > > > > > include/uapi/linux/kvm.h | 1 +
> > > > > > virt/kvm/Kconfig | 14 ++++++++++++++
> > > > > > virt/kvm/Makefile.kvm | 2 +-
> > > > > > virt/kvm/kvm_main.c | 11 +++++++++--
> > > > > > 5 files changed, 28 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h
> > > > > > index 906f899813dc..7a0c90ae9a3f 100644
> > > > > > --- a/include/linux/kvm_dirty_ring.h
> > > > > > +++ b/include/linux/kvm_dirty_ring.h
> > > > > > @@ -27,7 +27,7 @@ struct kvm_dirty_ring {
> > > > > > int index;
> > > > > > };
> > > > > >
> > > > > > -#ifndef CONFIG_HAVE_KVM_DIRTY_RING
> > > > > > +#ifndef CONFIG_HAVE_KVM_DIRTY_LOG
> > > > >
> > > > > s/LOG/LOG_RING/ according to the commit message? Or the name seems too
> > > > > generic.
> > > >
> > > > The commit message talks about the capability, while the above is the
> > > > config option. If you find the names inappropriate, feel free to
> > > > suggest alternatives (for all I care, they could be called FOO, BAR
> > > > and BAZ).
> > >
> > > The existing name from David looks better than the new one.. to me.
> >
> > I'm happy to bikeshed, but please spell it out for me. If we follow
> > the current scheme, we need 3 configuration symbols (of which we
> > already have one), and 2 capabilities (of which we already have one).
I hope it's not bikeshedding. I normally don't comment on namings at all
because many of them can be "bikeshedding" to me. But this one is so
special because it directly collides with KVM_GET_DIRTY_LOG, which is other
method of dirty tracking.
> >
> > Do you have any concrete proposal for those?
>
> In order to make some forward progress, I've reworked the series[1]
> with another proposal for those:
>
> Config symbols:
>
> - HAVE_KVM_DIRTY_RING:
> * mostly the same meaning as today
> * not directly selected by any architecture
> * doesn't expose any capability on its own
>
> - HAVE_KVM_DIRTY_RING_TSO:
> * only for strongly ordered architectures
> * selects HAVE_KVM_DIRTY_RING
> * exposes KVM_CAP_DIRTY_LOG_RING
> * selected by x86
>
> - HAVE_KVM_DIRTY_RING_ACQ_REL:
> * selects HAVE_KVM_DIRTY_RING
> * exposes KVM_CAP_DIRTY_LOG_RING_ACQ_REL
> * selected by arm64 and x86
>
> Capabilities:
>
> - KVM_CAP_DIRTY_LOG_RING: the good old x86-specific stuff, advertised
> when HAVE_KVM_DIRTY_RING_TSO is selected
>
> - KVM_CAP_DIRTY_LOG_RING_ACQ_REL: the new acquire/release semantics,
> advertised when HAVE_KVM_DIRTY_RING_ACQ_REL is selected
>
> This significantly reduces the churn and makes things slightly more
> explicit.
This looks good to me, thanks.
--
Peter Xu
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
