diff mbox

[v4,03/12] KVM: arm64: Introduce new MMIO region for the ITS base address

Message ID 1458958450-19662-4-git-send-email-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andre Przywara March 26, 2016, 2:14 a.m. UTC
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 |  9 +++++++++
 arch/arm64/include/uapi/asm/kvm.h              |  2 ++
 include/kvm/vgic/vgic.h                        |  5 +++++
 virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
 virt/kvm/arm/vgic/vgic_init.c                  |  1 +
 virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
 6 files changed, 34 insertions(+)

Comments

Christoffer Dall April 3, 2016, 10:08 a.m. UTC | #1
On Sat, Mar 26, 2016 at 02:14:01AM +0000, 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 |  9 +++++++++
>  arch/arm64/include/uapi/asm/kvm.h              |  2 ++
>  include/kvm/vgic/vgic.h                        |  5 +++++
>  virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
>  virt/kvm/arm/vgic/vgic_init.c                  |  1 +
>  virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
>  6 files changed, 34 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
> index 59541d4..087e2d9 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
> @@ -39,6 +39,15 @@ 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
> +      control register frame. The ITS allows MSI(-X) interrupts to be
> +      injected into guests. This extension is optional, if the kernel

s/optional, if/optional. If/

> +      does not support the ITS, the call returns -ENODEV.
> +      This memory is solely for the guest to access the ITS control

s/memory/address region/ ?

> +      registers and does not cover the ITS translation register.
> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
> +      This address needs to be 64K aligned and the region covers 64 KByte.

s/KByte/KBytes/
...or you could you 64K both places in this line.

>  
>    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 f209ea1..c2b257d 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -87,9 +87,11 @@ struct kvm_regs {
>  /* Supported VGICv3 address types  */
>  #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>  #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
> +#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
> index 7c1d145..11344e6 100644
> --- a/include/kvm/vgic/vgic.h
> +++ b/include/kvm/vgic/vgic.h
> @@ -135,6 +135,9 @@ struct vgic_dist {
>  		gpa_t			vgic_redist_base;
>  	};
>  
> +	/* The base address of the ITS control register frame */
> +	gpa_t			vgic_its_base;
> +
>  	/* distributor enabled */
>  	u32			enabled;
>  
> @@ -253,4 +256,6 @@ static inline int kvm_vgic_get_max_vcpus(void)
>  	return kvm_vgic_global_state.max_gic_vcpus;
>  }
>  
> +bool vgic_has_its(struct kvm *kvm);
> +
>  #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index 65395af..1de2478 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -612,3 +612,13 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
>  
>  	return map_is_active;
>  }
> +
> +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);
> +}
> diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
> index ac655b5..2301e03 100644
> --- a/virt/kvm/arm/vgic/vgic_init.c
> +++ b/virt/kvm/arm/vgic/vgic_init.c
> @@ -132,6 +132,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;
>  
>  out_unlock:
>  	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
> diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
> index 7f78a16..3ec2ac3 100644
> --- a/virt/kvm/arm/vgic/vgic_kvm_device.c
> +++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
> @@ -109,6 +109,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;
> @@ -495,6 +501,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>  		switch (attr->attr) {
>  		case KVM_VGIC_V3_ADDR_TYPE_DIST:
>  		case KVM_VGIC_V3_ADDR_TYPE_REDIST:
> +		case KVM_VGIC_V3_ADDR_TYPE_ITS:
>  			return 0;
>  		}
>  		break;
> -- 
> 2.7.3
>
Eric Auger April 5, 2016, 12:47 p.m. UTC | #2
Hi Andre,
On 04/03/2016 12:08 PM, Christoffer Dall wrote:
> On Sat, Mar 26, 2016 at 02:14:01AM +0000, 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 |  9 +++++++++
>>  arch/arm64/include/uapi/asm/kvm.h              |  2 ++
>>  include/kvm/vgic/vgic.h                        |  5 +++++
>>  virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
>>  virt/kvm/arm/vgic/vgic_init.c                  |  1 +
>>  virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
>>  6 files changed, 34 insertions(+)
>>
>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
>> index 59541d4..087e2d9 100644
>> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
>> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
>> @@ -39,6 +39,15 @@ 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
>> +      control register frame. The ITS allows MSI(-X) interrupts to be
>> +      injected into guests. This extension is optional, if the kernel
> 
> s/optional, if/optional. If/
> 
>> +      does not support the ITS, the call returns -ENODEV.
>> +      This memory is solely for the guest to access the ITS control
> 
> s/memory/address region/ ?
> 
>> +      registers and does not cover the ITS translation register.
>> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
>> +      This address needs to be 64K aligned and the region covers 64 KByte.
> 
> s/KByte/KBytes/
> ...or you could you 64K both places in this line.
> 
>>  
>>    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 f209ea1..c2b257d 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -87,9 +87,11 @@ struct kvm_regs {
>>  /* Supported VGICv3 address types  */
>>  #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>>  #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
>> +#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
>> index 7c1d145..11344e6 100644
>> --- a/include/kvm/vgic/vgic.h
>> +++ b/include/kvm/vgic/vgic.h
>> @@ -135,6 +135,9 @@ struct vgic_dist {
>>  		gpa_t			vgic_redist_base;
>>  	};
>>  
>> +	/* The base address of the ITS control register frame */
>> +	gpa_t			vgic_its_base;
>> +
>>  	/* distributor enabled */
>>  	u32			enabled;
>>  
>> @@ -253,4 +256,6 @@ static inline int kvm_vgic_get_max_vcpus(void)
>>  	return kvm_vgic_global_state.max_gic_vcpus;
>>  }
>>  
>> +bool vgic_has_its(struct kvm *kvm);
is it mandated to expose this function outside of the vgic subsystem?
why not declaring it in /virt/kvm/arm/vgic/vgic.h?
>> +
>>  #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>> index 65395af..1de2478 100644
>> --- a/virt/kvm/arm/vgic/vgic.c
>> +++ b/virt/kvm/arm/vgic/vgic.c
>> @@ -612,3 +612,13 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
>>  
>>  	return map_is_active;
>>  }
>> +
>> +bool vgic_has_its(struct kvm *kvm)
I would implement this function vgic_init.c or vgic_kvm_device.c leaving
vgic.c for pure vgic state mgt.

Eric
>> +{
>> +	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);
>> +}
>> diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
>> index ac655b5..2301e03 100644
>> --- a/virt/kvm/arm/vgic/vgic_init.c
>> +++ b/virt/kvm/arm/vgic/vgic_init.c
>> @@ -132,6 +132,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;
>>  
>>  out_unlock:
>>  	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
>> diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
>> index 7f78a16..3ec2ac3 100644
>> --- a/virt/kvm/arm/vgic/vgic_kvm_device.c
>> +++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
>> @@ -109,6 +109,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;
>> @@ -495,6 +501,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
>>  		switch (attr->attr) {
>>  		case KVM_VGIC_V3_ADDR_TYPE_DIST:
>>  		case KVM_VGIC_V3_ADDR_TYPE_REDIST:
>> +		case KVM_VGIC_V3_ADDR_TYPE_ITS:
>>  			return 0;
>>  		}
>>  		break;
>> -- 
>> 2.7.3
>>
Marc Zyngier April 7, 2016, 1:44 p.m. UTC | #3
On 26/03/16 02:14, 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 |  9 +++++++++
>  arch/arm64/include/uapi/asm/kvm.h              |  2 ++
>  include/kvm/vgic/vgic.h                        |  5 +++++
>  virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
>  virt/kvm/arm/vgic/vgic_init.c                  |  1 +
>  virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
>  6 files changed, 34 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
> index 59541d4..087e2d9 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
> @@ -39,6 +39,15 @@ 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
> +      control 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.
> +      This memory is solely for the guest to access the ITS control
> +      registers and does not cover the ITS translation register.
> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
> +      This address needs to be 64K aligned and the region covers 64 KByte.
>  
>    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 f209ea1..c2b257d 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -87,9 +87,11 @@ struct kvm_regs {
>  /* Supported VGICv3 address types  */
>  #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>  #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
> +#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
> index 7c1d145..11344e6 100644
> --- a/include/kvm/vgic/vgic.h
> +++ b/include/kvm/vgic/vgic.h
> @@ -135,6 +135,9 @@ struct vgic_dist {
>  		gpa_t			vgic_redist_base;
>  	};
>  
> +	/* The base address of the ITS control register frame */
> +	gpa_t			vgic_its_base;
> +

This makes me cringe a bit. The ITS is not part of the distributor, so
having it in vgic_dist is a bit weird. But maybe it is vgic_dist that
has the wrong name, and it is really a bag of vgic-related information.

But my real worry is that this (and the code below) only allows a single
ITS per VM. Is it really what we want? I would have thought that using
the KVM IO bus abstraction would have given us the flexibility of adding
ITSs at will (all sharing the same LPI space).

The reason this is important is that we could want an ITS serving
emulated devices, and one doing direct interrupt injection (once we have
GICv4 support).

Thoughts?

Thanks,

	M.
Chalamarla, Tirumalesh May 5, 2016, 6:08 p.m. UTC | #4
On 3/25/16, 7:14 PM, "kvmarm-bounces@lists.cs.columbia.edu on behalf of Andre Przywara" <kvmarm-bounces@lists.cs.columbia.edu on behalf of andre.przywara@arm.com> 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 |  9 +++++++++
> arch/arm64/include/uapi/asm/kvm.h              |  2 ++
> include/kvm/vgic/vgic.h                        |  5 +++++
> virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
> virt/kvm/arm/vgic/vgic_init.c                  |  1 +
> virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
> 6 files changed, 34 insertions(+)
>
>diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
>index 59541d4..087e2d9 100644
>--- a/Documentation/virtual/kvm/devices/arm-vgic.txt
>+++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
>@@ -39,6 +39,15 @@ 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
>+      control 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.
>+      This memory is solely for the guest to access the ITS control
>+      registers and does not cover the ITS translation register.
>+      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
>+      This address needs to be 64K aligned and the region covers 64 KByte.
> 
>   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 f209ea1..c2b257d 100644
>--- a/arch/arm64/include/uapi/asm/kvm.h
>+++ b/arch/arm64/include/uapi/asm/kvm.h
>@@ -87,9 +87,11 @@ struct kvm_regs {
> /* Supported VGICv3 address types  */
> #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
> #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
>+#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
>index 7c1d145..11344e6 100644
>--- a/include/kvm/vgic/vgic.h
>+++ b/include/kvm/vgic/vgic.h
>@@ -135,6 +135,9 @@ struct vgic_dist {
> 		gpa_t			vgic_redist_base;
> 	};
> 
>+	/* The base address of the ITS control register frame */
>+	gpa_t			vgic_its_base;
>+
> 	/* distributor enabled */
> 	u32			enabled;
> 
>@@ -253,4 +256,6 @@ static inline int kvm_vgic_get_max_vcpus(void)
> 	return kvm_vgic_global_state.max_gic_vcpus;
> }
> 
>+bool vgic_has_its(struct kvm *kvm);
>+
> #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
>diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>index 65395af..1de2478 100644
>--- a/virt/kvm/arm/vgic/vgic.c
>+++ b/virt/kvm/arm/vgic/vgic.c
>@@ -612,3 +612,13 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
> 
> 	return map_is_active;
> }
>+
>+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);
>+}
>diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
>index ac655b5..2301e03 100644
>--- a/virt/kvm/arm/vgic/vgic_init.c
>+++ b/virt/kvm/arm/vgic/vgic_init.c
>@@ -132,6 +132,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;
> 
> out_unlock:
> 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
>diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
>index 7f78a16..3ec2ac3 100644
>--- a/virt/kvm/arm/vgic/vgic_kvm_device.c
>+++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
>@@ -109,6 +109,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;

Should there be a check based on number of vcpus? 
> #endif
> 	default:
> 		r = -ENODEV;
>@@ -495,6 +501,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
> 		switch (attr->attr) {
> 		case KVM_VGIC_V3_ADDR_TYPE_DIST:
> 		case KVM_VGIC_V3_ADDR_TYPE_REDIST:
>+		case KVM_VGIC_V3_ADDR_TYPE_ITS:
> 			return 0;
> 		}
> 		break;
>-- 
>2.7.3
>
>_______________________________________________
>kvmarm mailing list
>kvmarm@lists.cs.columbia.edu
>https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier May 9, 2016, 3:47 p.m. UTC | #5
On 05/05/16 19:08, Chalamarla, Tirumalesh wrote:
> 
> 
> 
> 
> 
> On 3/25/16, 7:14 PM, "kvmarm-bounces@lists.cs.columbia.edu on behalf of Andre Przywara" <kvmarm-bounces@lists.cs.columbia.edu on behalf of andre.przywara@arm.com> 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 |  9 +++++++++
>> arch/arm64/include/uapi/asm/kvm.h              |  2 ++
>> include/kvm/vgic/vgic.h                        |  5 +++++
>> virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
>> virt/kvm/arm/vgic/vgic_init.c                  |  1 +
>> virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
>> 6 files changed, 34 insertions(+)
>>
>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
>> index 59541d4..087e2d9 100644
>> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
>> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
>> @@ -39,6 +39,15 @@ 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
>> +      control 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.
>> +      This memory is solely for the guest to access the ITS control
>> +      registers and does not cover the ITS translation register.
>> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
>> +      This address needs to be 64K aligned and the region covers 64 KByte.
>>
>>   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 f209ea1..c2b257d 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -87,9 +87,11 @@ struct kvm_regs {
>> /* Supported VGICv3 address types  */
>> #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>> #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
>> +#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
>> index 7c1d145..11344e6 100644
>> --- a/include/kvm/vgic/vgic.h
>> +++ b/include/kvm/vgic/vgic.h
>> @@ -135,6 +135,9 @@ struct vgic_dist {
>> 		gpa_t			vgic_redist_base;
>> 	};
>>
>> +	/* The base address of the ITS control register frame */
>> +	gpa_t			vgic_its_base;
>> +
>> 	/* distributor enabled */
>> 	u32			enabled;
>>
>> @@ -253,4 +256,6 @@ static inline int kvm_vgic_get_max_vcpus(void)
>> 	return kvm_vgic_global_state.max_gic_vcpus;
>> }
>>
>> +bool vgic_has_its(struct kvm *kvm);
>> +
>> #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>> index 65395af..1de2478 100644
>> --- a/virt/kvm/arm/vgic/vgic.c
>> +++ b/virt/kvm/arm/vgic/vgic.c
>> @@ -612,3 +612,13 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
>>
>> 	return map_is_active;
>> }
>> +
>> +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);
>> +}
>> diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
>> index ac655b5..2301e03 100644
>> --- a/virt/kvm/arm/vgic/vgic_init.c
>> +++ b/virt/kvm/arm/vgic/vgic_init.c
>> @@ -132,6 +132,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;
>>
>> out_unlock:
>> 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
>> diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
>> index 7f78a16..3ec2ac3 100644
>> --- a/virt/kvm/arm/vgic/vgic_kvm_device.c
>> +++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
>> @@ -109,6 +109,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;
> 
> Should there be a check based on number of vcpus? 

Why would the number of vcpus influence the size of the ITS?

Thanks,

	M.
Chalamarla, Tirumalesh May 9, 2016, 4:53 p.m. UTC | #6
On 5/9/16, 8:47 AM, "Marc Zyngier" <marc.zyngier@arm.com> wrote:

>On 05/05/16 19:08, Chalamarla, Tirumalesh wrote:
>> 
>> 
>> 
>> 
>> 
>> On 3/25/16, 7:14 PM, "kvmarm-bounces@lists.cs.columbia.edu on behalf of Andre Przywara" <kvmarm-bounces@lists.cs.columbia.edu on behalf of andre.przywara@arm.com> 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 |  9 +++++++++
>>> arch/arm64/include/uapi/asm/kvm.h              |  2 ++
>>> include/kvm/vgic/vgic.h                        |  5 +++++
>>> virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
>>> virt/kvm/arm/vgic/vgic_init.c                  |  1 +
>>> virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
>>> 6 files changed, 34 insertions(+)
>>>
>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
>>> index 59541d4..087e2d9 100644
>>> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
>>> @@ -39,6 +39,15 @@ 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
>>> +      control 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.
>>> +      This memory is solely for the guest to access the ITS control
>>> +      registers and does not cover the ITS translation register.
>>> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
>>> +      This address needs to be 64K aligned and the region covers 64 KByte.
>>>
>>>   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 f209ea1..c2b257d 100644
>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>> @@ -87,9 +87,11 @@ struct kvm_regs {
>>> /* Supported VGICv3 address types  */
>>> #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>>> #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
>>> +#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
>>> index 7c1d145..11344e6 100644
>>> --- a/include/kvm/vgic/vgic.h
>>> +++ b/include/kvm/vgic/vgic.h
>>> @@ -135,6 +135,9 @@ struct vgic_dist {
>>> 		gpa_t			vgic_redist_base;
>>> 	};
>>>
>>> +	/* The base address of the ITS control register frame */
>>> +	gpa_t			vgic_its_base;
>>> +
>>> 	/* distributor enabled */
>>> 	u32			enabled;
>>>
>>> @@ -253,4 +256,6 @@ static inline int kvm_vgic_get_max_vcpus(void)
>>> 	return kvm_vgic_global_state.max_gic_vcpus;
>>> }
>>>
>>> +bool vgic_has_its(struct kvm *kvm);
>>> +
>>> #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
>>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>>> index 65395af..1de2478 100644
>>> --- a/virt/kvm/arm/vgic/vgic.c
>>> +++ b/virt/kvm/arm/vgic/vgic.c
>>> @@ -612,3 +612,13 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
>>>
>>> 	return map_is_active;
>>> }
>>> +
>>> +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);
>>> +}
>>> diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
>>> index ac655b5..2301e03 100644
>>> --- a/virt/kvm/arm/vgic/vgic_init.c
>>> +++ b/virt/kvm/arm/vgic/vgic_init.c
>>> @@ -132,6 +132,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;
>>>
>>> out_unlock:
>>> 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
>>> diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
>>> index 7f78a16..3ec2ac3 100644
>>> --- a/virt/kvm/arm/vgic/vgic_kvm_device.c
>>> +++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
>>> @@ -109,6 +109,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;
>> 
>> Should there be a check based on number of vcpus? 
>
>Why would the number of vcpus influence the size of the ITS?

does the re-distributor per VCPU thing? I think its worth to add a check for re distributor size. 
>
>Thanks,
>
>	M.
>-- 
>Jazz is not dead. It just smells funny...
Marc Zyngier May 9, 2016, 5:09 p.m. UTC | #7
On 09/05/16 17:53, Chalamarla, Tirumalesh wrote:
> 
> 
> 
> 
> 
> On 5/9/16, 8:47 AM, "Marc Zyngier" <marc.zyngier@arm.com> wrote:
> 
>> On 05/05/16 19:08, Chalamarla, Tirumalesh wrote:
>>>
>>>
>>>
>>>
>>>
>>> On 3/25/16, 7:14 PM, "kvmarm-bounces@lists.cs.columbia.edu on behalf of Andre Przywara" <kvmarm-bounces@lists.cs.columbia.edu on behalf of andre.przywara@arm.com> 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 |  9 +++++++++
>>>> arch/arm64/include/uapi/asm/kvm.h              |  2 ++
>>>> include/kvm/vgic/vgic.h                        |  5 +++++
>>>> virt/kvm/arm/vgic/vgic.c                       | 10 ++++++++++
>>>> virt/kvm/arm/vgic/vgic_init.c                  |  1 +
>>>> virt/kvm/arm/vgic/vgic_kvm_device.c            |  7 +++++++
>>>> 6 files changed, 34 insertions(+)
>>>>
>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
>>>> index 59541d4..087e2d9 100644
>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic.txt
>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
>>>> @@ -39,6 +39,15 @@ 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
>>>> +      control 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.
>>>> +      This memory is solely for the guest to access the ITS control
>>>> +      registers and does not cover the ITS translation register.
>>>> +      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
>>>> +      This address needs to be 64K aligned and the region covers 64 KByte.
>>>>
>>>>   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 f209ea1..c2b257d 100644
>>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>>> @@ -87,9 +87,11 @@ struct kvm_regs {
>>>> /* Supported VGICv3 address types  */
>>>> #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
>>>> #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
>>>> +#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
>>>> index 7c1d145..11344e6 100644
>>>> --- a/include/kvm/vgic/vgic.h
>>>> +++ b/include/kvm/vgic/vgic.h
>>>> @@ -135,6 +135,9 @@ struct vgic_dist {
>>>> 		gpa_t			vgic_redist_base;
>>>> 	};
>>>>
>>>> +	/* The base address of the ITS control register frame */
>>>> +	gpa_t			vgic_its_base;
>>>> +
>>>> 	/* distributor enabled */
>>>> 	u32			enabled;
>>>>
>>>> @@ -253,4 +256,6 @@ static inline int kvm_vgic_get_max_vcpus(void)
>>>> 	return kvm_vgic_global_state.max_gic_vcpus;
>>>> }
>>>>
>>>> +bool vgic_has_its(struct kvm *kvm);
>>>> +
>>>> #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
>>>> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
>>>> index 65395af..1de2478 100644
>>>> --- a/virt/kvm/arm/vgic/vgic.c
>>>> +++ b/virt/kvm/arm/vgic/vgic.c
>>>> @@ -612,3 +612,13 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
>>>>
>>>> 	return map_is_active;
>>>> }
>>>> +
>>>> +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);
>>>> +}
>>>> diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
>>>> index ac655b5..2301e03 100644
>>>> --- a/virt/kvm/arm/vgic/vgic_init.c
>>>> +++ b/virt/kvm/arm/vgic/vgic_init.c
>>>> @@ -132,6 +132,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;
>>>>
>>>> out_unlock:
>>>> 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
>>>> diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
>>>> index 7f78a16..3ec2ac3 100644
>>>> --- a/virt/kvm/arm/vgic/vgic_kvm_device.c
>>>> +++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
>>>> @@ -109,6 +109,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;
>>>
>>> Should there be a check based on number of vcpus? 
>>
>> Why would the number of vcpus influence the size of the ITS?
> 
> does the re-distributor per VCPU thing? I think its worth to add a
> check for re distributor size.

This patch is implementing the ITS, not the redistributors.

If you have any comment on the GICv3 implementation (of which the
redistributors are part of), please comment on the relevant patch (see
the 55 patch series that has been posted on Friday).

Thanks,

	M.
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/devices/arm-vgic.txt b/Documentation/virtual/kvm/devices/arm-vgic.txt
index 59541d4..087e2d9 100644
--- a/Documentation/virtual/kvm/devices/arm-vgic.txt
+++ b/Documentation/virtual/kvm/devices/arm-vgic.txt
@@ -39,6 +39,15 @@  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
+      control 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.
+      This memory is solely for the guest to access the ITS control
+      registers and does not cover the ITS translation register.
+      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+      This address needs to be 64K aligned and the region covers 64 KByte.
 
   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 f209ea1..c2b257d 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -87,9 +87,11 @@  struct kvm_regs {
 /* Supported VGICv3 address types  */
 #define KVM_VGIC_V3_ADDR_TYPE_DIST	2
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
+#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/vgic/vgic.h b/include/kvm/vgic/vgic.h
index 7c1d145..11344e6 100644
--- a/include/kvm/vgic/vgic.h
+++ b/include/kvm/vgic/vgic.h
@@ -135,6 +135,9 @@  struct vgic_dist {
 		gpa_t			vgic_redist_base;
 	};
 
+	/* The base address of the ITS control register frame */
+	gpa_t			vgic_its_base;
+
 	/* distributor enabled */
 	u32			enabled;
 
@@ -253,4 +256,6 @@  static inline int kvm_vgic_get_max_vcpus(void)
 	return kvm_vgic_global_state.max_gic_vcpus;
 }
 
+bool vgic_has_its(struct kvm *kvm);
+
 #endif /* __ASM_ARM_KVM_VGIC_VGIC_H */
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 65395af..1de2478 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -612,3 +612,13 @@  bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, u32 intid)
 
 	return map_is_active;
 }
+
+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);
+}
diff --git a/virt/kvm/arm/vgic/vgic_init.c b/virt/kvm/arm/vgic/vgic_init.c
index ac655b5..2301e03 100644
--- a/virt/kvm/arm/vgic/vgic_init.c
+++ b/virt/kvm/arm/vgic/vgic_init.c
@@ -132,6 +132,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;
 
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
diff --git a/virt/kvm/arm/vgic/vgic_kvm_device.c b/virt/kvm/arm/vgic/vgic_kvm_device.c
index 7f78a16..3ec2ac3 100644
--- a/virt/kvm/arm/vgic/vgic_kvm_device.c
+++ b/virt/kvm/arm/vgic/vgic_kvm_device.c
@@ -109,6 +109,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;
@@ -495,6 +501,7 @@  static int vgic_v3_has_attr(struct kvm_device *dev,
 		switch (attr->attr) {
 		case KVM_VGIC_V3_ADDR_TYPE_DIST:
 		case KVM_VGIC_V3_ADDR_TYPE_REDIST:
+		case KVM_VGIC_V3_ADDR_TYPE_ITS:
 			return 0;
 		}
 		break;