[PATCH v9 05/12] ARM/ARM64: KVM: Make kvm_psci_call() return convention more flexible

Anup Patel anup at brainfault.org
Tue Apr 15 04:13:30 PDT 2014


On Tue, Apr 15, 2014 at 3:51 PM, Marc Zyngier <marc.zyngier at arm.com> wrote:
> On Tue, Apr 15 2014 at  7:14:08 am BST, Anup Patel <anup.patel at linaro.org> wrote:
>> Currently, the kvm_psci_call() returns 'true' or 'false' based on whether
>> the PSCI function call was handled successfully or not. This does not help
>> us emulate system-level PSCI functions where the actual emulation work will
>> be done by user space (QEMU or KVMTOOL). Examples of such system-level PSCI
>> functions are: PSCI v0.2 SYSTEM_OFF and SYSTEM_RESET.
>>
>> This patch updates kvm_psci_call() to return three types of values:
>> 1) > 0 (success)
>> 2) = 0 (success but exit to user space)
>> 3) < 0 (errors)
>>
>> Signed-off-by: Anup Patel <anup.patel at linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar at linaro.org>
>> Reviewed-by: Christoffer Dall <christoffer.dall at linaro.org>
>> ---
>>  arch/arm/include/asm/kvm_psci.h   |    2 +-
>>  arch/arm/kvm/handle_exit.c        |   10 +++++++---
>>  arch/arm/kvm/psci.c               |   28 ++++++++++++++++------------
>>  arch/arm64/include/asm/kvm_psci.h |    2 +-
>>  arch/arm64/kvm/handle_exit.c      |   10 +++++++---
>>  5 files changed, 32 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_psci.h b/arch/arm/include/asm/kvm_psci.h
>> index 4c0e3e1..6bda945 100644
>> --- a/arch/arm/include/asm/kvm_psci.h
>> +++ b/arch/arm/include/asm/kvm_psci.h
>> @@ -22,6 +22,6 @@
>>  #define KVM_ARM_PSCI_0_2     2
>>
>>  int kvm_psci_version(struct kvm_vcpu *vcpu);
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
>> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>>
>>  #endif /* __ARM_KVM_PSCI_H__ */
>> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
>> index 0de91fc..1270095 100644
>> --- a/arch/arm/kvm/handle_exit.c
>> +++ b/arch/arm/kvm/handle_exit.c
>> @@ -38,14 +38,18 @@ static int handle_svc_hyp(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>
>>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  {
>> +     int ret;
>> +
>>       trace_kvm_hvc(*vcpu_pc(vcpu), *vcpu_reg(vcpu, 0),
>>                     kvm_vcpu_hvc_get_imm(vcpu));
>>
>> -     if (kvm_psci_call(vcpu))
>> +     ret = kvm_psci_call(vcpu);
>> +     if (ret == -EINVAL) {
>
> Please check for (ret < 0), so it actually matches with the comment (and
> will same some debugging when we introduce another error code).

Should we be injecting undefined exception for all types errors?

The intention here was to only inject undefined exception when
PSCI function number is invalid.

>
>> +             kvm_inject_undefined(vcpu);
>>               return 1;
>> +     }
>>
>> -     kvm_inject_undefined(vcpu);
>> -     return 1;
>> +     return ret;
>>  }
>>
>>  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
>> index 8c42596c..14e6fa6 100644
>> --- a/arch/arm/kvm/psci.c
>> +++ b/arch/arm/kvm/psci.c
>> @@ -93,7 +93,7 @@ int kvm_psci_version(struct kvm_vcpu *vcpu)
>>       return KVM_ARM_PSCI_0_1;
>>  }
>>
>> -static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>> +static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>  {
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>> @@ -128,14 +128,14 @@ static bool kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>>               val = PSCI_RET_NOT_SUPPORTED;
>>               break;
>>       default:
>> -             return false;
>> +             return -EINVAL;
>>       }
>>
>>       *vcpu_reg(vcpu, 0) = val;
>> -     return true;
>> +     return 1;
>>  }
>>
>> -static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>> +static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>  {
>>       unsigned long psci_fn = *vcpu_reg(vcpu, 0) & ~((u32) 0);
>>       unsigned long val;
>> @@ -153,11 +153,11 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>               val = PSCI_RET_NOT_SUPPORTED;
>>               break;
>>       default:
>> -             return false;
>> +             return -EINVAL;
>>       }
>>
>>       *vcpu_reg(vcpu, 0) = val;
>> -     return true;
>> +     return 1;
>>  }
>>
>>  /**
>> @@ -165,12 +165,16 @@ static bool kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>>   * @vcpu: Pointer to the VCPU struct
>>   *
>>   * Handle PSCI calls from guests through traps from HVC instructions.
>> - * The calling convention is similar to SMC calls to the secure world where
>> - * the function number is placed in r0 and this function returns true if the
>> - * function number specified in r0 is withing the PSCI range, and false
>> - * otherwise.
>> + * The calling convention is similar to SMC calls to the secure world
>> + * where the function number is placed in r0.
>> + *
>> + * This function returns: > 0 (success), 0 (success but exit to user
>> + * space), and < 0 (errors)
>> + *
>> + * Errors:
>> + * -EINVAL: Unrecognized PSCI function
>>   */
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu)
>> +int kvm_psci_call(struct kvm_vcpu *vcpu)
>>  {
>>       switch (kvm_psci_version(vcpu)) {
>>       case KVM_ARM_PSCI_0_2:
>> @@ -178,6 +182,6 @@ bool kvm_psci_call(struct kvm_vcpu *vcpu)
>>       case KVM_ARM_PSCI_0_1:
>>               return kvm_psci_0_1_call(vcpu);
>>       default:
>> -             return false;
>> +             return -EINVAL;
>>       };
>>  }
>> diff --git a/arch/arm64/include/asm/kvm_psci.h b/arch/arm64/include/asm/kvm_psci.h
>> index e25c658..bc39e55 100644
>> --- a/arch/arm64/include/asm/kvm_psci.h
>> +++ b/arch/arm64/include/asm/kvm_psci.h
>> @@ -22,6 +22,6 @@
>>  #define KVM_ARM_PSCI_0_2     2
>>
>>  int kvm_psci_version(struct kvm_vcpu *vcpu);
>> -bool kvm_psci_call(struct kvm_vcpu *vcpu);
>> +int kvm_psci_call(struct kvm_vcpu *vcpu);
>>
>>  #endif /* __ARM64_KVM_PSCI_H__ */
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 7bc41ea..743a74d 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -30,11 +30,15 @@ typedef int (*exit_handle_fn)(struct kvm_vcpu *, struct kvm_run *);
>>
>>  static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  {
>> -     if (kvm_psci_call(vcpu))
>> +     int ret;
>> +
>> +     ret = kvm_psci_call(vcpu);
>> +     if (ret == -EINVAL) {
>
> Same here.
>
>> +             kvm_inject_undefined(vcpu);
>>               return 1;
>> +     }
>>
>> -     kvm_inject_undefined(vcpu);
>> -     return 1;
>> +     return ret;
>>  }
>>
>>  static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny.
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

--
Anup



More information about the linux-arm-kernel mailing list