[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Anup Patel]

This patch allows us to have X-Gene guest VCPU when using
KVM arm64 on APM X-Gene host.

We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
running on X-Gene host.

Signed-off-by: Anup Patel <anup.patel@linaro.org>
Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
---
 arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
 arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
 arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 5031f42..8194707 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -55,8 +55,9 @@ struct kvm_regs {
 #define KVM_ARM_TARGET_AEM_V8		0
 #define KVM_ARM_TARGET_FOUNDATION_V8	1
 #define KVM_ARM_TARGET_CORTEX_A57	2
+#define KVM_ARM_TARGET_XGENE_V8		3
 
-#define KVM_ARM_NUM_TARGETS		3
+#define KVM_ARM_NUM_TARGETS		4
 
 /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
 #define KVM_ARM_DEVICE_TYPE_SHIFT	0
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 2c3ff67..e99b0a5 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
 	unsigned long implementor = read_cpuid_implementor();
 	unsigned long part_number = read_cpuid_part_number();
 
-	if (implementor != ARM_CPU_IMP_ARM)
-		return -EINVAL;
-
-	switch (part_number) {
-	case ARM_CPU_PART_AEM_V8:
-		return KVM_ARM_TARGET_AEM_V8;
-	case ARM_CPU_PART_FOUNDATION:
-		return KVM_ARM_TARGET_FOUNDATION_V8;
-	case ARM_CPU_PART_CORTEX_A57:
-		/* Currently handled by the generic backend */
-		return KVM_ARM_TARGET_CORTEX_A57;
+	switch (implementor) {
+	case ARM_CPU_IMP_ARM:
+		switch (part_number) {
+		case ARM_CPU_PART_AEM_V8:
+			return KVM_ARM_TARGET_AEM_V8;
+		case ARM_CPU_PART_FOUNDATION:
+			return KVM_ARM_TARGET_FOUNDATION_V8;
+		case ARM_CPU_PART_CORTEX_A57:
+			return KVM_ARM_TARGET_CORTEX_A57;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case ARM_CPU_IMP_APM:
+		switch (part_number) {
+		case APM_CPU_PART_POTENZA:
+			return KVM_ARM_TARGET_XGENE_V8;
+		default:
+			return -EINVAL;
+		}
+		break;
 	default:
-		return -EINVAL;
+ 		return -EINVAL;
 	}
 }
 
diff --git a/arch/arm64/kvm/sys_regs_generic_v8.c b/arch/arm64/kvm/sys_regs_generic_v8.c
index 4268ab9..87f0b3f4 100644
--- a/arch/arm64/kvm/sys_regs_generic_v8.c
+++ b/arch/arm64/kvm/sys_regs_generic_v8.c
@@ -90,6 +90,9 @@ static int __init sys_reg_genericv8_init(void)
 					  &genericv8_target_table);
 	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_CORTEX_A57,
 					  &genericv8_target_table);
+	kvm_register_target_sys_reg_table(KVM_ARM_TARGET_XGENE_V8,
+					  &genericv8_target_table);
+
 	return 0;
 }
 late_initcall(sys_reg_genericv8_init);
-- 
1.7.9.5

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Marc Zyngier]

Hi Anup,

On 15/07/13 12:46, Anup Patel wrote:
> This patch allows us to have X-Gene guest VCPU when using
> KVM arm64 on APM X-Gene host.
> 
> We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
> and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
> running on X-Gene host.
> 
> Signed-off-by: Anup Patel <anup.patel@linaro.org>
> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
> ---
>  arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
>  arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
>  arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 5031f42..8194707 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -55,8 +55,9 @@ struct kvm_regs {
>  #define KVM_ARM_TARGET_AEM_V8		0
>  #define KVM_ARM_TARGET_FOUNDATION_V8	1
>  #define KVM_ARM_TARGET_CORTEX_A57	2
> +#define KVM_ARM_TARGET_XGENE_V8		3
>  
> -#define KVM_ARM_NUM_TARGETS		3
> +#define KVM_ARM_NUM_TARGETS		4
>  
>  /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
>  #define KVM_ARM_DEVICE_TYPE_SHIFT	0
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 2c3ff67..e99b0a5 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
>  	unsigned long implementor = read_cpuid_implementor();
>  	unsigned long part_number = read_cpuid_part_number();
>  
> -	if (implementor != ARM_CPU_IMP_ARM)
> -		return -EINVAL;
> -
> -	switch (part_number) {
> -	case ARM_CPU_PART_AEM_V8:
> -		return KVM_ARM_TARGET_AEM_V8;
> -	case ARM_CPU_PART_FOUNDATION:
> -		return KVM_ARM_TARGET_FOUNDATION_V8;
> -	case ARM_CPU_PART_CORTEX_A57:
> -		/* Currently handled by the generic backend */
> -		return KVM_ARM_TARGET_CORTEX_A57;
> +	switch (implementor) {
> +	case ARM_CPU_IMP_ARM:
> +		switch (part_number) {
> +		case ARM_CPU_PART_AEM_V8:
> +			return KVM_ARM_TARGET_AEM_V8;
> +		case ARM_CPU_PART_FOUNDATION:
> +			return KVM_ARM_TARGET_FOUNDATION_V8;
> +		case ARM_CPU_PART_CORTEX_A57:
> +			return KVM_ARM_TARGET_CORTEX_A57;
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case ARM_CPU_IMP_APM:
> +		switch (part_number) {
> +		case APM_CPU_PART_POTENZA:
> +			return KVM_ARM_TARGET_XGENE_V8;

Why don't we have KVM_ARM_TARGET_XGENE_POTENZA (or something similar)
instead? I don't expect all the X-Gene CPUs to be the same forever...

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Anup Patel]

Hi Marc,

On Mon, Jul 15, 2013 at 6:09 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Anup,
>
> On 15/07/13 12:46, Anup Patel wrote:
>> This patch allows us to have X-Gene guest VCPU when using
>> KVM arm64 on APM X-Gene host.
>>
>> We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
>> and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
>> running on X-Gene host.
>>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
>>  arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
>>  arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
>>  3 files changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index 5031f42..8194707 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -55,8 +55,9 @@ struct kvm_regs {
>>  #define KVM_ARM_TARGET_AEM_V8                0
>>  #define KVM_ARM_TARGET_FOUNDATION_V8 1
>>  #define KVM_ARM_TARGET_CORTEX_A57    2
>> +#define KVM_ARM_TARGET_XGENE_V8              3
>>
>> -#define KVM_ARM_NUM_TARGETS          3
>> +#define KVM_ARM_NUM_TARGETS          4
>>
>>  /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
>>  #define KVM_ARM_DEVICE_TYPE_SHIFT    0
>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>> index 2c3ff67..e99b0a5 100644
>> --- a/arch/arm64/kvm/guest.c
>> +++ b/arch/arm64/kvm/guest.c
>> @@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
>>       unsigned long implementor = read_cpuid_implementor();
>>       unsigned long part_number = read_cpuid_part_number();
>>
>> -     if (implementor != ARM_CPU_IMP_ARM)
>> -             return -EINVAL;
>> -
>> -     switch (part_number) {
>> -     case ARM_CPU_PART_AEM_V8:
>> -             return KVM_ARM_TARGET_AEM_V8;
>> -     case ARM_CPU_PART_FOUNDATION:
>> -             return KVM_ARM_TARGET_FOUNDATION_V8;
>> -     case ARM_CPU_PART_CORTEX_A57:
>> -             /* Currently handled by the generic backend */
>> -             return KVM_ARM_TARGET_CORTEX_A57;
>> +     switch (implementor) {
>> +     case ARM_CPU_IMP_ARM:
>> +             switch (part_number) {
>> +             case ARM_CPU_PART_AEM_V8:
>> +                     return KVM_ARM_TARGET_AEM_V8;
>> +             case ARM_CPU_PART_FOUNDATION:
>> +                     return KVM_ARM_TARGET_FOUNDATION_V8;
>> +             case ARM_CPU_PART_CORTEX_A57:
>> +                     return KVM_ARM_TARGET_CORTEX_A57;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>> +     case ARM_CPU_IMP_APM:
>> +             switch (part_number) {
>> +             case APM_CPU_PART_POTENZA:
>> +                     return KVM_ARM_TARGET_XGENE_V8;
>
> Why don't we have KVM_ARM_TARGET_XGENE_POTENZA (or something similar)
> instead? I don't expect all the X-Gene CPUs to be the same forever...

OK, I will rename it to KVM_ARM_TARGET_XGENE_POTENZA.

Does this mean that with every new ARM64 CPU we will have to add a new
target for KVM ARM64 ?

If so then I think the list of targets will grow very fast.

I also realized that if we add a new target type in KVM ARM64 then we have
to also update KVMTOOL to use the new target else KVMTOOL fails to
recognize the target provided by KVM ARM64.

Do you think we can have KVM_ARM_TARGET_xxx to represent a common
target for a family of CPUs from given ARM64 vendor?

>
>         M.
> --
> Jazz is not dead. It just smells funny...
>
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm

Regards,
Anup

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Alexander Graf]

On 15.07.2013, at 14:56, Anup Patel wrote:

> Hi Marc,
> 
> On Mon, Jul 15, 2013 at 6:09 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Hi Anup,
>> 
>> On 15/07/13 12:46, Anup Patel wrote:
>>> This patch allows us to have X-Gene guest VCPU when using
>>> KVM arm64 on APM X-Gene host.
>>> 
>>> We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
>>> and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
>>> running on X-Gene host.
>>> 
>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>> ---
>>> arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
>>> arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
>>> arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
>>> 3 files changed, 27 insertions(+), 13 deletions(-)
>>> 
>>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>>> index 5031f42..8194707 100644
>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>> @@ -55,8 +55,9 @@ struct kvm_regs {
>>> #define KVM_ARM_TARGET_AEM_V8                0
>>> #define KVM_ARM_TARGET_FOUNDATION_V8 1
>>> #define KVM_ARM_TARGET_CORTEX_A57    2
>>> +#define KVM_ARM_TARGET_XGENE_V8              3
>>> 
>>> -#define KVM_ARM_NUM_TARGETS          3
>>> +#define KVM_ARM_NUM_TARGETS          4
>>> 
>>> /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
>>> #define KVM_ARM_DEVICE_TYPE_SHIFT    0
>>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>>> index 2c3ff67..e99b0a5 100644
>>> --- a/arch/arm64/kvm/guest.c
>>> +++ b/arch/arm64/kvm/guest.c
>>> @@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
>>>      unsigned long implementor = read_cpuid_implementor();
>>>      unsigned long part_number = read_cpuid_part_number();
>>> 
>>> -     if (implementor != ARM_CPU_IMP_ARM)
>>> -             return -EINVAL;
>>> -
>>> -     switch (part_number) {
>>> -     case ARM_CPU_PART_AEM_V8:
>>> -             return KVM_ARM_TARGET_AEM_V8;
>>> -     case ARM_CPU_PART_FOUNDATION:
>>> -             return KVM_ARM_TARGET_FOUNDATION_V8;
>>> -     case ARM_CPU_PART_CORTEX_A57:
>>> -             /* Currently handled by the generic backend */
>>> -             return KVM_ARM_TARGET_CORTEX_A57;
>>> +     switch (implementor) {
>>> +     case ARM_CPU_IMP_ARM:
>>> +             switch (part_number) {
>>> +             case ARM_CPU_PART_AEM_V8:
>>> +                     return KVM_ARM_TARGET_AEM_V8;
>>> +             case ARM_CPU_PART_FOUNDATION:
>>> +                     return KVM_ARM_TARGET_FOUNDATION_V8;
>>> +             case ARM_CPU_PART_CORTEX_A57:
>>> +                     return KVM_ARM_TARGET_CORTEX_A57;
>>> +             default:
>>> +                     return -EINVAL;
>>> +             }
>>> +             break;
>>> +     case ARM_CPU_IMP_APM:
>>> +             switch (part_number) {
>>> +             case APM_CPU_PART_POTENZA:
>>> +                     return KVM_ARM_TARGET_XGENE_V8;
>> 
>> Why don't we have KVM_ARM_TARGET_XGENE_POTENZA (or something similar)
>> instead? I don't expect all the X-Gene CPUs to be the same forever...
> 
> OK, I will rename it to KVM_ARM_TARGET_XGENE_POTENZA.
> 
> Does this mean that with every new ARM64 CPU we will have to add a new
> target for KVM ARM64 ?

Only for different core types, no? Any Cortex-A57 should still behave the same.

> If so then I think the list of targets will grow very fast.
> 
> I also realized that if we add a new target type in KVM ARM64 then we have
> to also update KVMTOOL to use the new target else KVMTOOL fails to
> recognize the target provided by KVM ARM64.

Right. It might make sense to have a fetch mechanism for the host cpu part. So you can ask KVM for the host cpu type and pass that back in here.

> Do you think we can have KVM_ARM_TARGET_xxx to represent a common
> target for a family of CPUs from given ARM64 vendor?

Anything that is compatible is compatible :). I don't know the product roadmaps for X-Gene cores, but you will want to make the field here as coarse grained as possible, while maintaining the guarantee that a guest still behaves the same.


Alex

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Anup Patel]

On Mon, Jul 15, 2013 at 6:32 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 15.07.2013, at 14:56, Anup Patel wrote:
>
>> Hi Marc,
>>
>> On Mon, Jul 15, 2013 at 6:09 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> Hi Anup,
>>>
>>> On 15/07/13 12:46, Anup Patel wrote:
>>>> This patch allows us to have X-Gene guest VCPU when using
>>>> KVM arm64 on APM X-Gene host.
>>>>
>>>> We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
>>>> and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
>>>> running on X-Gene host.
>>>>
>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>> ---
>>>> arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
>>>> arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
>>>> arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
>>>> 3 files changed, 27 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>>>> index 5031f42..8194707 100644
>>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>>> @@ -55,8 +55,9 @@ struct kvm_regs {
>>>> #define KVM_ARM_TARGET_AEM_V8                0
>>>> #define KVM_ARM_TARGET_FOUNDATION_V8 1
>>>> #define KVM_ARM_TARGET_CORTEX_A57    2
>>>> +#define KVM_ARM_TARGET_XGENE_V8              3
>>>>
>>>> -#define KVM_ARM_NUM_TARGETS          3
>>>> +#define KVM_ARM_NUM_TARGETS          4
>>>>
>>>> /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
>>>> #define KVM_ARM_DEVICE_TYPE_SHIFT    0
>>>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>>>> index 2c3ff67..e99b0a5 100644
>>>> --- a/arch/arm64/kvm/guest.c
>>>> +++ b/arch/arm64/kvm/guest.c
>>>> @@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
>>>>      unsigned long implementor = read_cpuid_implementor();
>>>>      unsigned long part_number = read_cpuid_part_number();
>>>>
>>>> -     if (implementor != ARM_CPU_IMP_ARM)
>>>> -             return -EINVAL;
>>>> -
>>>> -     switch (part_number) {
>>>> -     case ARM_CPU_PART_AEM_V8:
>>>> -             return KVM_ARM_TARGET_AEM_V8;
>>>> -     case ARM_CPU_PART_FOUNDATION:
>>>> -             return KVM_ARM_TARGET_FOUNDATION_V8;
>>>> -     case ARM_CPU_PART_CORTEX_A57:
>>>> -             /* Currently handled by the generic backend */
>>>> -             return KVM_ARM_TARGET_CORTEX_A57;
>>>> +     switch (implementor) {
>>>> +     case ARM_CPU_IMP_ARM:
>>>> +             switch (part_number) {
>>>> +             case ARM_CPU_PART_AEM_V8:
>>>> +                     return KVM_ARM_TARGET_AEM_V8;
>>>> +             case ARM_CPU_PART_FOUNDATION:
>>>> +                     return KVM_ARM_TARGET_FOUNDATION_V8;
>>>> +             case ARM_CPU_PART_CORTEX_A57:
>>>> +                     return KVM_ARM_TARGET_CORTEX_A57;
>>>> +             default:
>>>> +                     return -EINVAL;
>>>> +             }
>>>> +             break;
>>>> +     case ARM_CPU_IMP_APM:
>>>> +             switch (part_number) {
>>>> +             case APM_CPU_PART_POTENZA:
>>>> +                     return KVM_ARM_TARGET_XGENE_V8;
>>>
>>> Why don't we have KVM_ARM_TARGET_XGENE_POTENZA (or something similar)
>>> instead? I don't expect all the X-Gene CPUs to be the same forever...
>>
>> OK, I will rename it to KVM_ARM_TARGET_XGENE_POTENZA.
>>
>> Does this mean that with every new ARM64 CPU we will have to add a new
>> target for KVM ARM64 ?
>
> Only for different core types, no? Any Cortex-A57 should still behave the same.
>
>> If so then I think the list of targets will grow very fast.
>>
>> I also realized that if we add a new target type in KVM ARM64 then we have
>> to also update KVMTOOL to use the new target else KVMTOOL fails to
>> recognize the target provided by KVM ARM64.
>
> Right. It might make sense to have a fetch mechanism for the host cpu part. So you can ask KVM for the host cpu type and pass that back in here.
>
>> Do you think we can have KVM_ARM_TARGET_xxx to represent a common
>> target for a family of CPUs from given ARM64 vendor?
>
> Anything that is compatible is compatible :). I don't know the product roadmaps for X-Gene cores, but you will want to make the field here as coarse grained as possible, while maintaining the guarantee that a guest still behaves the same.

Actually, I don't see X-Gene cores changing in-terms of register interface
available to EL1 and EL0 in near future. This is the reason why I had named
the target as KVM_ARM_TARGET_XGENE_V8.

My only concern in having a more specific target type (such as
KVM_ARM_TARGET_XGENE_POTENZA) is that we might unnecessarily
add new target types without any difference on KVM-side.

>
>
> Alex
>

--Anup

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Alexander Graf]

On 15.07.2013, at 16:04, Anup Patel wrote:

> On Mon, Jul 15, 2013 at 6:32 PM, Alexander Graf <agraf@suse.de> wrote:
>> 
>> On 15.07.2013, at 14:56, Anup Patel wrote:
>> 
>>> Hi Marc,
>>> 
>>> On Mon, Jul 15, 2013 at 6:09 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> Hi Anup,
>>>> 
>>>> On 15/07/13 12:46, Anup Patel wrote:
>>>>> This patch allows us to have X-Gene guest VCPU when using
>>>>> KVM arm64 on APM X-Gene host.
>>>>> 
>>>>> We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
>>>>> and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
>>>>> running on X-Gene host.
>>>>> 
>>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>>> ---
>>>>> arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
>>>>> arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
>>>>> arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
>>>>> 3 files changed, 27 insertions(+), 13 deletions(-)
>>>>> 
>>>>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>>>>> index 5031f42..8194707 100644
>>>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>>>> @@ -55,8 +55,9 @@ struct kvm_regs {
>>>>> #define KVM_ARM_TARGET_AEM_V8                0
>>>>> #define KVM_ARM_TARGET_FOUNDATION_V8 1
>>>>> #define KVM_ARM_TARGET_CORTEX_A57    2
>>>>> +#define KVM_ARM_TARGET_XGENE_V8              3
>>>>> 
>>>>> -#define KVM_ARM_NUM_TARGETS          3
>>>>> +#define KVM_ARM_NUM_TARGETS          4
>>>>> 
>>>>> /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
>>>>> #define KVM_ARM_DEVICE_TYPE_SHIFT    0
>>>>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>>>>> index 2c3ff67..e99b0a5 100644
>>>>> --- a/arch/arm64/kvm/guest.c
>>>>> +++ b/arch/arm64/kvm/guest.c
>>>>> @@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
>>>>>     unsigned long implementor = read_cpuid_implementor();
>>>>>     unsigned long part_number = read_cpuid_part_number();
>>>>> 
>>>>> -     if (implementor != ARM_CPU_IMP_ARM)
>>>>> -             return -EINVAL;
>>>>> -
>>>>> -     switch (part_number) {
>>>>> -     case ARM_CPU_PART_AEM_V8:
>>>>> -             return KVM_ARM_TARGET_AEM_V8;
>>>>> -     case ARM_CPU_PART_FOUNDATION:
>>>>> -             return KVM_ARM_TARGET_FOUNDATION_V8;
>>>>> -     case ARM_CPU_PART_CORTEX_A57:
>>>>> -             /* Currently handled by the generic backend */
>>>>> -             return KVM_ARM_TARGET_CORTEX_A57;
>>>>> +     switch (implementor) {
>>>>> +     case ARM_CPU_IMP_ARM:
>>>>> +             switch (part_number) {
>>>>> +             case ARM_CPU_PART_AEM_V8:
>>>>> +                     return KVM_ARM_TARGET_AEM_V8;
>>>>> +             case ARM_CPU_PART_FOUNDATION:
>>>>> +                     return KVM_ARM_TARGET_FOUNDATION_V8;
>>>>> +             case ARM_CPU_PART_CORTEX_A57:
>>>>> +                     return KVM_ARM_TARGET_CORTEX_A57;
>>>>> +             default:
>>>>> +                     return -EINVAL;
>>>>> +             }
>>>>> +             break;
>>>>> +     case ARM_CPU_IMP_APM:
>>>>> +             switch (part_number) {
>>>>> +             case APM_CPU_PART_POTENZA:
>>>>> +                     return KVM_ARM_TARGET_XGENE_V8;
>>>> 
>>>> Why don't we have KVM_ARM_TARGET_XGENE_POTENZA (or something similar)
>>>> instead? I don't expect all the X-Gene CPUs to be the same forever...
>>> 
>>> OK, I will rename it to KVM_ARM_TARGET_XGENE_POTENZA.
>>> 
>>> Does this mean that with every new ARM64 CPU we will have to add a new
>>> target for KVM ARM64 ?
>> 
>> Only for different core types, no? Any Cortex-A57 should still behave the same.
>> 
>>> If so then I think the list of targets will grow very fast.
>>> 
>>> I also realized that if we add a new target type in KVM ARM64 then we have
>>> to also update KVMTOOL to use the new target else KVMTOOL fails to
>>> recognize the target provided by KVM ARM64.
>> 
>> Right. It might make sense to have a fetch mechanism for the host cpu part. So you can ask KVM for the host cpu type and pass that back in here.
>> 
>>> Do you think we can have KVM_ARM_TARGET_xxx to represent a common
>>> target for a family of CPUs from given ARM64 vendor?
>> 
>> Anything that is compatible is compatible :). I don't know the product roadmaps for X-Gene cores, but you will want to make the field here as coarse grained as possible, while maintaining the guarantee that a guest still behaves the same.
> 
> Actually, I don't see X-Gene cores changing in-terms of register interface
> available to EL1 and EL0 in near future. This is the reason why I had named
> the target as KVM_ARM_TARGET_XGENE_V8.

So where does the v8 come from? Is there any non-ARMv8 XGene? If not, this is v1 really, right? What if we just call it v1 instead? Then when a new core comes up that needs different treatment, we create a new target.

But this really is Marc's call.


Alex

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Anup Patel]

Hi Marc,

On Mon, Jul 15, 2013 at 7:51 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 15.07.2013, at 16:04, Anup Patel wrote:
>
>> On Mon, Jul 15, 2013 at 6:32 PM, Alexander Graf <agraf@suse.de> wrote:
>>>
>>> On 15.07.2013, at 14:56, Anup Patel wrote:
>>>
>>>> Hi Marc,
>>>>
>>>> On Mon, Jul 15, 2013 at 6:09 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>> Hi Anup,
>>>>>
>>>>> On 15/07/13 12:46, Anup Patel wrote:
>>>>>> This patch allows us to have X-Gene guest VCPU when using
>>>>>> KVM arm64 on APM X-Gene host.
>>>>>>
>>>>>> We add KVM_ARM_TARGET_XGENE_V8 for X-Gene compatible guest VCPU
>>>>>> and we return KVM_ARM_TARGET_XGENE_V8 in kvm_target_cpu() when
>>>>>> running on X-Gene host.
>>>>>>
>>>>>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>>>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>>>>>> ---
>>>>>> arch/arm64/include/uapi/asm/kvm.h    |    3 ++-
>>>>>> arch/arm64/kvm/guest.c               |   34 ++++++++++++++++++++++------------
>>>>>> arch/arm64/kvm/sys_regs_generic_v8.c |    3 +++
>>>>>> 3 files changed, 27 insertions(+), 13 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>>>>>> index 5031f42..8194707 100644
>>>>>> --- a/arch/arm64/include/uapi/asm/kvm.h
>>>>>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>>>>>> @@ -55,8 +55,9 @@ struct kvm_regs {
>>>>>> #define KVM_ARM_TARGET_AEM_V8                0
>>>>>> #define KVM_ARM_TARGET_FOUNDATION_V8 1
>>>>>> #define KVM_ARM_TARGET_CORTEX_A57    2
>>>>>> +#define KVM_ARM_TARGET_XGENE_V8              3
>>>>>>
>>>>>> -#define KVM_ARM_NUM_TARGETS          3
>>>>>> +#define KVM_ARM_NUM_TARGETS          4
>>>>>>
>>>>>> /* KVM_ARM_SET_DEVICE_ADDR ioctl id encoding */
>>>>>> #define KVM_ARM_DEVICE_TYPE_SHIFT    0
>>>>>> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
>>>>>> index 2c3ff67..e99b0a5 100644
>>>>>> --- a/arch/arm64/kvm/guest.c
>>>>>> +++ b/arch/arm64/kvm/guest.c
>>>>>> @@ -207,19 +207,29 @@ int __attribute_const__ kvm_target_cpu(void)
>>>>>>     unsigned long implementor = read_cpuid_implementor();
>>>>>>     unsigned long part_number = read_cpuid_part_number();
>>>>>>
>>>>>> -     if (implementor != ARM_CPU_IMP_ARM)
>>>>>> -             return -EINVAL;
>>>>>> -
>>>>>> -     switch (part_number) {
>>>>>> -     case ARM_CPU_PART_AEM_V8:
>>>>>> -             return KVM_ARM_TARGET_AEM_V8;
>>>>>> -     case ARM_CPU_PART_FOUNDATION:
>>>>>> -             return KVM_ARM_TARGET_FOUNDATION_V8;
>>>>>> -     case ARM_CPU_PART_CORTEX_A57:
>>>>>> -             /* Currently handled by the generic backend */
>>>>>> -             return KVM_ARM_TARGET_CORTEX_A57;
>>>>>> +     switch (implementor) {
>>>>>> +     case ARM_CPU_IMP_ARM:
>>>>>> +             switch (part_number) {
>>>>>> +             case ARM_CPU_PART_AEM_V8:
>>>>>> +                     return KVM_ARM_TARGET_AEM_V8;
>>>>>> +             case ARM_CPU_PART_FOUNDATION:
>>>>>> +                     return KVM_ARM_TARGET_FOUNDATION_V8;
>>>>>> +             case ARM_CPU_PART_CORTEX_A57:
>>>>>> +                     return KVM_ARM_TARGET_CORTEX_A57;
>>>>>> +             default:
>>>>>> +                     return -EINVAL;
>>>>>> +             }
>>>>>> +             break;
>>>>>> +     case ARM_CPU_IMP_APM:
>>>>>> +             switch (part_number) {
>>>>>> +             case APM_CPU_PART_POTENZA:
>>>>>> +                     return KVM_ARM_TARGET_XGENE_V8;
>>>>>
>>>>> Why don't we have KVM_ARM_TARGET_XGENE_POTENZA (or something similar)
>>>>> instead? I don't expect all the X-Gene CPUs to be the same forever...
>>>>
>>>> OK, I will rename it to KVM_ARM_TARGET_XGENE_POTENZA.
>>>>
>>>> Does this mean that with every new ARM64 CPU we will have to add a new
>>>> target for KVM ARM64 ?
>>>
>>> Only for different core types, no? Any Cortex-A57 should still behave the same.
>>>
>>>> If so then I think the list of targets will grow very fast.
>>>>
>>>> I also realized that if we add a new target type in KVM ARM64 then we have
>>>> to also update KVMTOOL to use the new target else KVMTOOL fails to
>>>> recognize the target provided by KVM ARM64.
>>>
>>> Right. It might make sense to have a fetch mechanism for the host cpu part. So you can ask KVM for the host cpu type and pass that back in here.
>>>
>>>> Do you think we can have KVM_ARM_TARGET_xxx to represent a common
>>>> target for a family of CPUs from given ARM64 vendor?
>>>
>>> Anything that is compatible is compatible :). I don't know the product roadmaps for X-Gene cores, but you will want to make the field here as coarse grained as possible, while maintaining the guarantee that a guest still behaves the same.
>>
>> Actually, I don't see X-Gene cores changing in-terms of register interface
>> available to EL1 and EL0 in near future. This is the reason why I had named
>> the target as KVM_ARM_TARGET_XGENE_V8.
>
> So where does the v8 come from? Is there any non-ARMv8 XGene? If not, this is v1 really, right? What if we just call it v1 instead? Then when a new core comes up that needs different treatment, we create a new target.
>
> But this really is Marc's call.

I like Alex's suggestion.

How about having KVM_ARM_TARGET_XGENE_V1 now and
KVM_ARM_TARGET_XGENE_V2 in future ?

>
>
> Alex
>

--Anup

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Peter Maydell]

On 15 July 2013 15:21, Alexander Graf <agraf@suse.de> wrote:
> On 15.07.2013, at 16:04, Anup Patel wrote:
>> On Mon, Jul 15, 2013 at 6:32 PM, Alexander Graf <agraf@suse.de> wrote:
>>> Anything that is compatible is compatible :). I don't know
>>> the product roadmaps for X-Gene cores, but you will want to
>>> make the field here as coarse grained as possible, while
>>> maintaining the guarantee that a guest still behaves the same.
>>
>> Actually, I don't see X-Gene cores changing in-terms of register
>> interface available to EL1 and EL0 in near future. This is the
>> reason why I had named the target as KVM_ARM_TARGET_XGENE_V8.
>
> So where does the v8 come from? Is there any non-ARMv8 XGene? If
> not, this is v1 really, right? What if we just call it v1 instead?
> Then when a new core comes up that needs different treatment, we
> create a new target.

I think what we're actually seeing here is that we started with
a design that specifically said (in the v7 space) "only A15
on A15", and we're now trying to broaden that out, only we
haven't actually set out anywhere how that broadening is
supposed to work (ie what range of hosts will work with
what range of guests, and how, and whether we want to have
a "give me a guest CPU that looks like the host CPU" option).

I think the original idea was that userspace would say
which CPU it wanted, and the kernel would know about that
CPU (including details like ID and feature registers which
are going to vary from core to core).

thanks
-- PMM

[PATCH] arm64: KVM: Support X-Gene guest VCPU on APM X-Gene host

[Anup Patel]

On Mon, Jul 15, 2013 at 9:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 15 July 2013 15:21, Alexander Graf <agraf@suse.de> wrote:
>> On 15.07.2013, at 16:04, Anup Patel wrote:
>>> On Mon, Jul 15, 2013 at 6:32 PM, Alexander Graf <agraf@suse.de> wrote:
>>>> Anything that is compatible is compatible :). I don't know
>>>> the product roadmaps for X-Gene cores, but you will want to
>>>> make the field here as coarse grained as possible, while
>>>> maintaining the guarantee that a guest still behaves the same.
>>>
>>> Actually, I don't see X-Gene cores changing in-terms of register
>>> interface available to EL1 and EL0 in near future. This is the
>>> reason why I had named the target as KVM_ARM_TARGET_XGENE_V8.
>>
>> So where does the v8 come from? Is there any non-ARMv8 XGene? If
>> not, this is v1 really, right? What if we just call it v1 instead?
>> Then when a new core comes up that needs different treatment, we
>> create a new target.
>
> I think what we're actually seeing here is that we started with
> a design that specifically said (in the v7 space) "only A15
> on A15", and we're now trying to broaden that out, only we
> haven't actually set out anywhere how that broadening is
> supposed to work (ie what range of hosts will work with
> what range of guests, and how, and whether we want to have
> a "give me a guest CPU that looks like the host CPU" option).
>
> I think the original idea was that userspace would say
> which CPU it wanted, and the kernel would know about that
> CPU (including details like ID and feature registers which
> are going to vary from core to core).
>
> thanks
> -- PMM

I am fine in naming the target as KVM_ARM_TARGET_XGENE_POTENZA.
Selecting this target for given X-Gene host (now and in-future) would mean
that KVM VCPU is X-Gene Potenza compatible. I will send revised patch
for this.

Also, further refining PMM's suggestion, we can have user-space specify
what CPU type it wants and what CPU features it wants. The KVM ARM
or KVM ARM64 will try to give all desired CPU features but it may change
the CPU features based on underlying host CPU features.

IMHO, user-space should be capable of enabling/disabling certain CPU
features of KVM VCPU. Of course, some features like arch_timer would
be mandatory and always available to all VCPU target types.

Regards,
Anup