| Message ID | 20220922170133.2617189-4-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] KVM: Use acquire/release semantics when accessing dirty ring GFN state | expand |
On Thu, Sep 22, 2022 at 06:01:30PM +0100, Marc Zyngier wrote: > Since x86 is TSO (give or take), allow it to advertise the new > ORDERED version of the dirty ring capability. No other change is > required for it. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/x86/kvm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index e3cbd7706136..eb63bc31ed1d 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -29,6 +29,7 @@ config KVM > select HAVE_KVM_PFNCACHE > select HAVE_KVM_IRQFD > select HAVE_KVM_DIRTY_RING > + select HAVE_KVM_DIRTY_RING_ORDERED > select IRQ_BYPASS_MANAGER > select HAVE_KVM_IRQ_BYPASS > select HAVE_KVM_IRQ_ROUTING Before patch 2-3, we only have HAVE_KVM_DIRTY_RING. After that, we'll have: HAVE_KVM_DIRTY_LOG HAVE_KVM_DIRTY_RING HAVE_KVM_DIRTY_RING_ORDERED I'm wondering whether we can just keep using the old HAVE_KVM_DIRTY_RING, but just declare a new KVM_CAP_DIRTY_LOG_RING_ORDERED only after all memory barrier patches merged (after patch 1). IIUC it's a matter of whether any of the arch would like to support !ORDERED version of dirty ring at all, but then IIUC we'll need to have the memory barriers conditional too or not sure how it'll help. Thanks,
On Fri, 23 Sep 2022 23:46:40 +0100, Peter Xu <peterx@redhat.com> wrote: > > On Thu, Sep 22, 2022 at 06:01:30PM +0100, Marc Zyngier wrote: > > Since x86 is TSO (give or take), allow it to advertise the new > > ORDERED version of the dirty ring capability. No other change is > > required for it. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/x86/kvm/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > > index e3cbd7706136..eb63bc31ed1d 100644 > > --- a/arch/x86/kvm/Kconfig > > +++ b/arch/x86/kvm/Kconfig > > @@ -29,6 +29,7 @@ config KVM > > select HAVE_KVM_PFNCACHE > > select HAVE_KVM_IRQFD > > select HAVE_KVM_DIRTY_RING > > + select HAVE_KVM_DIRTY_RING_ORDERED > > select IRQ_BYPASS_MANAGER > > select HAVE_KVM_IRQ_BYPASS > > select HAVE_KVM_IRQ_ROUTING > > Before patch 2-3, we only have HAVE_KVM_DIRTY_RING. > > After that, we'll have: > > HAVE_KVM_DIRTY_LOG > HAVE_KVM_DIRTY_RING > HAVE_KVM_DIRTY_RING_ORDERED > > I'm wondering whether we can just keep using the old HAVE_KVM_DIRTY_RING, > but just declare a new KVM_CAP_DIRTY_LOG_RING_ORDERED only after all memory > barrier patches merged (after patch 1). The problem is to decide, on a per architecture basis, how to expose the old property. I'm happy to key it on being x86 specific, but that feels pretty gross, and totally unnecessary for strongly ordered architectures (s390?). > IIUC it's a matter of whether any of the arch would like to support > !ORDERED version of dirty ring at all, but then IIUC we'll need to have the > memory barriers conditional too or not sure how it'll help. Memory barriers do not affect the semantics of the userspace, while the lack thereof do. On strongly ordered architectures, acquire/release is usually "free", because that's implied by their memory model. If it thus free for these to implement both versions of the API. So are we discussing the cost of couple of (mostly invisible) config options? M.
On Sat, Sep 24, 2022 at 09:47:59AM +0100, Marc Zyngier wrote: > On Fri, 23 Sep 2022 23:46:40 +0100, > Peter Xu <peterx@redhat.com> wrote: > > > > On Thu, Sep 22, 2022 at 06:01:30PM +0100, Marc Zyngier wrote: > > > Since x86 is TSO (give or take), allow it to advertise the new > > > ORDERED version of the dirty ring capability. No other change is > > > required for it. > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > arch/x86/kvm/Kconfig | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > > > index e3cbd7706136..eb63bc31ed1d 100644 > > > --- a/arch/x86/kvm/Kconfig > > > +++ b/arch/x86/kvm/Kconfig > > > @@ -29,6 +29,7 @@ config KVM > > > select HAVE_KVM_PFNCACHE > > > select HAVE_KVM_IRQFD > > > select HAVE_KVM_DIRTY_RING > > > + select HAVE_KVM_DIRTY_RING_ORDERED > > > select IRQ_BYPASS_MANAGER > > > select HAVE_KVM_IRQ_BYPASS > > > select HAVE_KVM_IRQ_ROUTING > > > > Before patch 2-3, we only have HAVE_KVM_DIRTY_RING. > > > > After that, we'll have: > > > > HAVE_KVM_DIRTY_LOG > > HAVE_KVM_DIRTY_RING > > HAVE_KVM_DIRTY_RING_ORDERED > > > > I'm wondering whether we can just keep using the old HAVE_KVM_DIRTY_RING, > > but just declare a new KVM_CAP_DIRTY_LOG_RING_ORDERED only after all memory > > barrier patches merged (after patch 1). > > The problem is to decide, on a per architecture basis, how to expose > the old property. I'm happy to key it on being x86 specific, but that > feels pretty gross, and totally unnecessary for strongly ordered > architectures (s390?). > > > IIUC it's a matter of whether any of the arch would like to support > > !ORDERED version of dirty ring at all, but then IIUC we'll need to have the > > memory barriers conditional too or not sure how it'll help. > > Memory barriers do not affect the semantics of the userspace, while > the lack thereof do. On strongly ordered architectures, > acquire/release is usually "free", because that's implied by their > memory model. If it thus free for these to implement both versions of > the API. Right, that's why I thought it won't help. now I see what you meant, indeed if without the three config we'll need a x86 ifdef which may not be as clean as this approach. Thanks for explaining.
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index e3cbd7706136..eb63bc31ed1d 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -29,6 +29,7 @@ config KVM select HAVE_KVM_PFNCACHE select HAVE_KVM_IRQFD select HAVE_KVM_DIRTY_RING + select HAVE_KVM_DIRTY_RING_ORDERED select IRQ_BYPASS_MANAGER select HAVE_KVM_IRQ_BYPASS select HAVE_KVM_IRQ_ROUTING
Since x86 is TSO (give or take), allow it to advertise the new ORDERED version of the dirty ring capability. No other change is required for it. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/x86/kvm/Kconfig | 1 + 1 file changed, 1 insertion(+)