On Fri, 20 Jan 2023 08:54:06 +0100
Nico Boehr <nrb@xxxxxxxxxxxxx> wrote:
> Migration mode is a VM attribute which enables tracking of changes in
> storage attributes (PGSTE). It assumes dirty tracking is enabled on all
> memslots to keep a dirty bitmap of pages with changed storage attributes.
>
> When enabling migration mode, we currently check that dirty tracking is
> enabled for all memslots. However, userspace can disable dirty tracking
> without disabling migration mode.
>
> Since migration mode is pointless with dirty tracking disabled, disable
> migration mode whenever userspace disables dirty tracking on any slot.
>
> Also update the documentation to clarify that dirty tracking must be
> enabled when enabling migration mode, which is already enforced by the
> code in kvm_s390_vm_start_migration().
>
> To disable migration mode, slots_lock should be held, which is taken
> in kvm_set_memory_region() and thus held in
> kvm_arch_prepare_memory_region().
>
> Restructure the prepare code a bit so all the sanity checking is done
> before disabling migration mode. This ensures migration mode isn't
> disabled when some sanity check fails.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 190df4a212a7 ("KVM: s390: CMMA tracking, ESSA emulation, migration mode")
> Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx>
Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> ---
> Documentation/virt/kvm/devices/vm.rst | 4 +++
> arch/s390/kvm/kvm-s390.c | 41 ++++++++++++++++++---------
> 2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/virt/kvm/devices/vm.rst b/Documentation/virt/kvm/devices/vm.rst
> index 60acc39e0e93..147efec626e5 100644
> --- a/Documentation/virt/kvm/devices/vm.rst
> +++ b/Documentation/virt/kvm/devices/vm.rst
> @@ -302,6 +302,10 @@ Allows userspace to start migration mode, needed for PGSTE migration.
> Setting this attribute when migration mode is already active will have
> no effects.
>
> +Dirty tracking must be enabled on all memslots, else -EINVAL is returned. When
> +dirty tracking is disabled on any memslot, migration mode is automatically
> +stopped.
> +
> :Parameters: none
> :Returns: -ENOMEM if there is not enough free memory to start migration mode;
> -EINVAL if the state of the VM is invalid (e.g. no memory defined);
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index e4890e04b210..4785f002cd93 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5628,28 +5628,43 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
> enum kvm_mr_change change)
> {
> gpa_t size;
> + int rc;
>
> /* When we are protected, we should not change the memory slots */
> if (kvm_s390_pv_get_handle(kvm))
> return -EINVAL;
>
> - if (change == KVM_MR_DELETE || change == KVM_MR_FLAGS_ONLY)
> - return 0;
> + if (change != KVM_MR_DELETE && change != KVM_MR_FLAGS_ONLY) {
> + /* A few sanity checks. We can have memory slots which have to be
> + * located/ended at a segment boundary (1MB). The memory in userland is
> + * ok to be fragmented into various different vmas. It is okay to mmap()
> + * and munmap() stuff in this slot after doing this call at any time
> + */
>
> - /* A few sanity checks. We can have memory slots which have to be
> - located/ended at a segment boundary (1MB). The memory in userland is
> - ok to be fragmented into various different vmas. It is okay to mmap()
> - and munmap() stuff in this slot after doing this call at any time */
> + if (new->userspace_addr & 0xffffful)
> + return -EINVAL;
>
> - if (new->userspace_addr & 0xffffful)
> - return -EINVAL;
> + size = new->npages * PAGE_SIZE;
> + if (size & 0xffffful)
> + return -EINVAL;
>
> - size = new->npages * PAGE_SIZE;
> - if (size & 0xffffful)
> - return -EINVAL;
> + if ((new->base_gfn * PAGE_SIZE) + size > kvm->arch.mem_limit)
> + return -EINVAL;
> + }
>
> - if ((new->base_gfn * PAGE_SIZE) + size > kvm->arch.mem_limit)
> - return -EINVAL;
> + /* Turn off migration mode when userspace disables dirty page logging.
> + * Migration mode expects dirty page logging being enabled to store
> + * its dirty bitmap.
> + */
> + if (kvm->arch.migration_mode) {
> + if ((old->flags & KVM_MEM_LOG_DIRTY_PAGES) &&
> + !(new->flags & KVM_MEM_LOG_DIRTY_PAGES)) {
> + rc = kvm_s390_vm_stop_migration(kvm);
> +
> + if (rc)
> + pr_warn("Failed to stop migration mode\n");
> + }
> + }
>
> return 0;
> }
