Re: [PATCH v4 1/1] KVM: s390: disable migration mode when dirty tracking is disabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 27 Jan 2023 15:05:32 +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().
> 
> Also highlight in the documentation for KVM_S390_GET_CMMA_BITS that it
> can now fail with -EINVAL when dirty tracking is disabled while
> migration mode is on. Move all the error codes to a table to this stays
> readable.
> 
> 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/api.rst        | 16 ++++++----
>  Documentation/virt/kvm/devices/vm.rst |  4 +++
>  arch/s390/kvm/kvm-s390.c              | 43 +++++++++++++++++++--------
>  3 files changed, 45 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 9807b05a1b57..2978acfcafc4 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -4537,11 +4537,17 @@ mask is unused.
>  
>  values points to the userspace buffer where the result will be stored.
>  
> -This ioctl can fail with -ENOMEM if not enough memory can be allocated to
> -complete the task, with -ENXIO if CMMA is not enabled, with -EINVAL if
> -KVM_S390_CMMA_PEEK is not set but migration mode was not enabled, with
> --EFAULT if the userspace address is invalid or if no page table is
> -present for the addresses (e.g. when using hugepages).
> +Errors:
> +
> +  ======     =============================================================
> +  ENOMEM     not enough memory can be allocated to complete the task
> +  ENXIO      if CMMA is not enabled
> +  EINVAL     if KVM_S390_CMMA_PEEK is not set but migration mode was not enabled
> +  EINVAL     if KVM_S390_CMMA_PEEK is not set but dirty tracking has been
> +             disabled (and thus migration mode was automatically disabled)
> +  EFAULT     if the userspace address is invalid or if no page table is
> +             present for the addresses (e.g. when using hugepages).
> +  ======     =============================================================
>  
>  4.108 KVM_S390_SET_CMMA_BITS
>  ----------------------------
> 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..cb72f9a09fb3 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5633,23 +5633,40 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
>  	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;
> +	if (!kvm->arch.migration_mode)
> +		return 0;
> +
> +	/*
> +	 * Turn off migration mode when:
> +	 * - userspace creates a new memslot with dirty logging off,
> +	 * - userspace modifies an existing memslot (MOVE or FLAGS_ONLY) and
> +	 *   dirty logging is turned off.
> +	 * Migration mode expects dirty page logging being enabled to store
> +	 * its dirty bitmap.
> +	 */
> +	if (change != KVM_MR_DELETE &&
> +	    !(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
> +		WARN(kvm_s390_vm_stop_migration(kvm),
> +		     "Failed to stop migration mode");
>  
>  	return 0;
>  }




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux