[PATCH 04/13] KVM: arm64: Introduce new MMIO region for the ITS base address

[eric.auger@linaro.org (Eric Auger)]

On 05/29/2015 11:53 AM, Andre Przywara wrote:
> The ARM GICv3 ITS controller requires a separate register frame to
> cover ITS specific registers. Add a new VGIC address type and store
> the address in a field in the vgic_dist structure.
> Provide a function to check whether userland has provided the address,
> so ITS functionality can be guarded by that check.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Documentation/virtual/kvm/devices/arm-vgic.txt |  7 +++++++
>  arch/arm64/include/uapi/asm/kvm.h              |  3 +++
>  include/kvm/arm_vgic.h                         |  3 +++
>  virt/kvm/arm/vgic-v3-emul.c                    |  1 +
>  virt/kvm/arm/vgic.c                            | 17 +++++++++++++++++
>  virt/kvm/arm/vgic.h                            |  1 +
>  6 files changed, 32 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
> index 3fb9054..1f89001 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
> @@ -39,6 +39,13 @@ Groups:
>        Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
>        This address needs to be 64K aligned.
>  
> +    KVM_VGIC_V3_ADDR_TYPE_ITS (rw, 64-bit)
> +      Base address in the guest physical address space of the GICv3 ITS
> +      register frame. The ITS allows MSI(-X) interrupts to be injected
> +      into guests. This extension is optional, if the kernel does not
> +      support the ITS, the call returns -ENODEV.
> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
> +      This address needs to be 64K aligned and the region covers 64 KByte.
I would emphasize this is the control registers (ITS_Base) hence a
single page, not comprising the ITS translation register page.
>  
>    KVM_DEV_ARM_VGIC_GRP_DIST_REGS
>    Attributes:
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index d268320..e42435c 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -82,8 +82,11 @@ struct kvm_regs {
>  #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>  #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
>  
extra white line?
> +#define KVM_VGIC_V3_ADDR_TYPE_ITS	4
> +
>  #define KVM_VGIC_V3_DIST_SIZE		SZ_64K
>  #define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
> +#define KVM_VGIC_V3_ITS_SIZE		SZ_64K
>  
>  #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
>  #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index b18e2c5..37725bb 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -178,6 +178,9 @@ struct vgic_dist {
>  		phys_addr_t		vgic_redist_base;
>  	};
>  
> +	/* The base address for the MSI control block (V2M/ITS) */
why V2M here? It's it the GITS_TRANSLATER page that has a fellow page in
case of V2M?
> +	phys_addr_t		vgic_its_base;
> +
>  	/* Distributor enabled */
>  	u32			enabled;
>  
> diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
> index fbfdd6f..16c6d8a 100644
> --- a/virt/kvm/arm/vgic-v3-emul.c
> +++ b/virt/kvm/arm/vgic-v3-emul.c
> @@ -1012,6 +1012,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>  			return -ENXIO;
>  		case KVM_VGIC_V3_ADDR_TYPE_DIST:
>  		case KVM_VGIC_V3_ADDR_TYPE_REDIST:
> +		case KVM_VGIC_V3_ADDR_TYPE_ITS:
>  			return 0;
>  		}
>  		break;
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 6ea30e0..2e9723aa 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -932,6 +932,16 @@ int vgic_register_kvm_io_dev(struct kvm *kvm, gpa_t base, int len,
>  	return ret;
>  }
>  
> +bool vgic_has_its(struct kvm *kvm)
> +{
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +
> +	if (dist->vgic_model != KVM_DEV_TYPE_ARM_VGIC_V3)
> +		return false;
> +
> +	return !IS_VGIC_ADDR_UNDEF(dist->vgic_its_base);
> +}
> +
>  static int vgic_nr_shared_irqs(struct vgic_dist *dist)
>  {
>  	return dist->nr_irqs - VGIC_NR_PRIVATE_IRQS;
> @@ -1835,6 +1845,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
>  	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
>  	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
>  	kvm->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
> +	kvm->arch.vgic.vgic_its_base = VGIC_ADDR_UNDEF;
minor: given the fact we have specialized
init_vgic_model/vgic_vx_init_emulation wouldn't it make sense to move
those VGIC v3 specific assignment there? also CPU base is specific to v2?

Eric
>  
>  out_unlock:
>  	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
> @@ -1932,6 +1943,12 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
>  		block_size = KVM_VGIC_V3_REDIST_SIZE;
>  		alignment = SZ_64K;
>  		break;
> +	case KVM_VGIC_V3_ADDR_TYPE_ITS:
> +		type_needed = KVM_DEV_TYPE_ARM_VGIC_V3;
> +		addr_ptr = &vgic->vgic_its_base;
> +		block_size = KVM_VGIC_V3_ITS_SIZE;
> +		alignment = SZ_64K;
> +		break;
>  #endif
>  	default:
>  		r = -ENODEV;
> diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
> index 0df74cb..a093f5c 100644
> --- a/virt/kvm/arm/vgic.h
> +++ b/virt/kvm/arm/vgic.h
> @@ -136,5 +136,6 @@ int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
>  int vgic_init(struct kvm *kvm);
>  void vgic_v2_init_emulation(struct kvm *kvm);
>  void vgic_v3_init_emulation(struct kvm *kvm);
> +bool vgic_has_its(struct kvm *kvm);
>  
>  #endif
>