Re: [PATCH v2 0/6] KVM: Fix dirty-ring ordering on weakly ordered architectures

[Gavin Shan <gshan@redhat.com>]

On 9/27/22 12:51 AM, Marc Zyngier wrote:
> [Same distribution list as Gavin's dirty-ring on arm64 series]
> 
> This is an update on the initial series posted as [0].
> 
> As Gavin started posting patches enabling the dirty-ring infrastructure
> on arm64 [1], it quickly became apparent that the API was never intended
> to work on relaxed memory ordering architectures (owing to its x86
> origins).
> 
> This series tries to retrofit some ordering into the existing API by:
> 
> - relying on acquire/release semantics which are the default on x86,
>    but need to be explicit on arm64
> 
> - adding a new capability that indicate which flavor is supported, either
>    with explicit ordering (arm64) or both implicit and explicit (x86),
>    as suggested by Paolo at KVM Forum
> 
> - documenting the requirements for this new capability on weakly ordered
>    architectures
> 
> - updating the selftests to do the right thing
> 
> Ideally, this series should be a prefix of Gavin's, plus a small change
> to his series:
> 
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index 0309b2d0f2da..7785379c5048 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -32,7 +32,7 @@ menuconfig KVM
>   	select KVM_VFIO
>   	select HAVE_KVM_EVENTFD
>   	select HAVE_KVM_IRQFD
> -	select HAVE_KVM_DIRTY_RING
> +	select HAVE_KVM_DIRTY_RING_ACQ_REL
>   	select HAVE_KVM_MSI
>   	select HAVE_KVM_IRQCHIP
>   	select HAVE_KVM_IRQ_ROUTING
> 
> This has been very lightly tested on an arm64 box with Gavin's v3 [2] series.
> 
> * From v1:
>    - Repainted the config symbols and new capability so that their
>      naming is more acceptable and causes less churn
>    - Fixed a couple of blunders as pointed out by Peter and Paolo
>    - Updated the documentation
> 
> [0] https://lore.kernel.org/r/20220922170133.2617189-1-maz@kernel.org
> [1] https://lore.kernel.org/lkml/YyiV%2Fl7O23aw5aaO@xz-m1.local/T/
> [2] https://lore.kernel.org/r/20220922003214.276736-1-gshan@redhat.com
> 
> Marc Zyngier (6):
>    KVM: Use acquire/release semantics when accessing dirty ring GFN state
>    KVM: Add KVM_CAP_DIRTY_LOG_RING_ACQ_REL capability and config option
>    KVM: x86: Select CONFIG_HAVE_KVM_DIRTY_RING_ACQ_REL
>    KVM: Document weakly ordered architecture requirements for dirty ring
>    KVM: selftests: dirty-log: Upgrade flag accesses to acquire/release
>      semantics
>    KVM: selftests: dirty-log: Use KVM_CAP_DIRTY_LOG_RING_ACQ_REL if
>      available
> 
>   Documentation/virt/kvm/api.rst               | 17 +++++++++++++++--
>   arch/x86/kvm/Kconfig                         |  3 ++-
>   include/uapi/linux/kvm.h                     |  1 +
>   tools/testing/selftests/kvm/dirty_log_test.c |  8 +++++---
>   tools/testing/selftests/kvm/lib/kvm_util.c   |  5 ++++-
>   virt/kvm/Kconfig                             | 14 ++++++++++++++
>   virt/kvm/dirty_ring.c                        |  4 ++--
>   virt/kvm/kvm_main.c                          |  9 ++++++++-
>   8 files changed, 51 insertions(+), 10 deletions(-)
> 

This series looks good to me.

Reviewed-by: Gavin Shan <gshan@redhat.com>

Thanks,
Gavin

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm