[PATCH v6 16/24] KVM: arm64: vgic-its: KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES

[christoffer.dall@linaro.org (Christoffer Dall)]

On Thu, May 04, 2017 at 01:44:36PM +0200, Eric Auger wrote:
> Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group:
> - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM
> - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal
>   structures.
> 
> We hold the vcpus lock during the save and restore to make
> sure no vcpu is running.
> 
> At this stage the functionality is not yet implemented. Only
> the skeleton is put in place.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v5 -> v6:
> - remove the pending table sync from the ITS table restore
> 
> v4 -> v5:
> - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES
> - rename *flush* into *save*
> - call its_sync_lpi_pending_table at the end of restore
> - use abi framework
> 
> v3 -> v4:
> - pass kvm struct handle to vgic_its_flush/restore_pending_tables
> - take the kvm lock and vcpu locks
> - ABI revision check
> - check attr->attr is null
> 
> v1 -> v2:
> - remove useless kvm parameter
> ---
>  arch/arm/include/uapi/asm/kvm.h   |   4 +-
>  arch/arm64/include/uapi/asm/kvm.h |   4 +-
>  virt/kvm/arm/vgic/vgic-its.c      | 101 ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 4beb83b..8e6563c 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -199,7 +199,9 @@ struct kvm_arch_memory_slot {
>  #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
>  #define VGIC_LEVEL_INFO_LINE_LEVEL	0
>  
> -#define   KVM_DEV_ARM_VGIC_CTRL_INIT    0
> +#define   KVM_DEV_ARM_VGIC_CTRL_INIT		0
> +#define   KVM_DEV_ARM_ITS_SAVE_TABLES		1
> +#define   KVM_DEV_ARM_ITS_RESTORE_TABLES	2
>  
>  /* KVM_IRQ_LINE irq field index values */
>  #define KVM_ARM_IRQ_TYPE_SHIFT		24
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 7e8dd69..1e35115 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -219,7 +219,9 @@ struct kvm_arch_memory_slot {
>  #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK	0x3ff
>  #define VGIC_LEVEL_INFO_LINE_LEVEL	0
>  
> -#define   KVM_DEV_ARM_VGIC_CTRL_INIT	0
> +#define   KVM_DEV_ARM_VGIC_CTRL_INIT		0
> +#define   KVM_DEV_ARM_ITS_SAVE_TABLES           1
> +#define   KVM_DEV_ARM_ITS_RESTORE_TABLES        2
>  
>  /* Device Control API on vcpu fd */
>  #define KVM_ARM_VCPU_PMU_V3_CTRL	0
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 3ad615a..5b251dc 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1669,12 +1669,68 @@ int vgic_its_attr_regs_access(struct kvm_device *dev,
>  }
>  
>  /**
> + * vgic_its_save_device_tables - Save the device table and all ITT
> + * into guest RAM
> + */
> +static int vgic_its_save_device_tables(struct vgic_its *its)
> +{
> +	return -ENXIO;
> +}
> +
> +/**
> + * vgic_its_restore_device_tables - Restore the device table and all ITT
> + * from guest RAM to internal data structs
> + */
> +static int vgic_its_restore_device_tables(struct vgic_its *its)
> +{
> +	return -ENXIO;
> +}
> +
> +/**
> + * vgic_its_save_collection_table - Save the collection table into
> + * guest RAM
> + */
> +static int vgic_its_save_collection_table(struct vgic_its *its)
> +{
> +	return -ENXIO;
> +}
> +
> +/**
> + * vgic_its_restore_collection_table - reads the collection table
> + * in guest memory and restores the ITS internal state. Requires the
> + * BASER registers to be restored before.
> + */
> +static int vgic_its_restore_collection_table(struct vgic_its *its)
> +{
> +	return -ENXIO;
> +}
> +
> +/**
>   * vgic_its_save_tables_v0 - Save the ITS tables into guest ARM
>   * according to v0 ABI
>   */
>  static int vgic_its_save_tables_v0(struct vgic_its *its)
>  {
> -	return -ENXIO;
> +	struct kvm *kvm = its->dev->kvm;
> +	int ret;
> +
> +	mutex_lock(&kvm->lock);
> +
> +	if (!lock_all_vcpus(kvm)) {
> +		mutex_unlock(&kvm->lock);
> +		return -EBUSY;
> +	}
> +
> +	ret = vgic_its_save_device_tables(its);
> +	if (ret)
> +		goto out;
> +
> +	ret = vgic_its_save_collection_table(its);
> +
> +out:
> +	unlock_all_vcpus(kvm);
> +	mutex_unlock(&kvm->lock);
> +	return ret;
>  }
>  
>  /**
> @@ -1684,7 +1740,34 @@ static int vgic_its_save_tables_v0(struct vgic_its *its)
>   */
>  static int vgic_its_restore_tables_v0(struct vgic_its *its)
>  {
> -	return -ENXIO;
> +	struct kvm *kvm = its->dev->kvm;
> +	int ret;
> +
> +	mutex_lock(&kvm->lock);
> +
> +	if (!lock_all_vcpus(kvm)) {
> +		mutex_unlock(&kvm->lock);
> +		return -EBUSY;
> +	}
> +
> +	ret = vgic_its_restore_collection_table(its);
> +	if (ret)
> +		goto out;
> +
> +	ret = vgic_its_restore_device_tables(its);
> +
> +out:
> +	unlock_all_vcpus(kvm);
> +	mutex_unlock(&kvm->lock);
> +
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * On restore path, MSI injections can happen before the
> +	 * first VCPU run so let's complete the GIC init here.
> +	 */
> +	return kvm_vgic_map_resources(its->dev->kvm);

So not sure where we left this one in our other discussion.  I feel like
this is a bit out of place currently, and registering the iodevs should
rather be done when setting the base addresses used by that, but I think
we can fix that later, or are we implying ordering towards userspace
here?

Marc, any thoughts?

Thanks,
-Christoffer