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).
>
> 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.
Thoughts?
M.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/dirty-log-ordered-bikeshed
--
Without deviation from the norm, progress is not possible.
