Re: [PATCH 2/2] KVM: arm64: PSCI: Forbid 64bit functions for 32bit guests
Hello,
On 4/1/20 5:58 PM, Marc Zyngier wrote:
> Implementing (and even advertising) 64bit PSCI functions to 32bit
> guests is at least a bit odd, if not altogether violating the
> spec which says ("5.2.1 Register usage in arguments and return values"):
>
> "Adherence to the SMC Calling Conventions implies that any AArch32
> caller of an SMC64 function will get a return code of 0x(int32).
> This matches the NOT_SUPPORTED error code used in PSCI"
>
> Tighten the implementation by pretending these functions are not
> there for 32bit guests.
>
> Signed-off-by: Marc Zyngier
> ---
> virt/kvm/arm/psci.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index 69ff4a51ceb5..122795cdd984 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
> @@ -199,6 +199,21 @@ static void kvm_psci_narrow_to_32bit(struct kvm_vcpu
> *vcpu)
> vcpu_set_reg(vcpu, i, (u32)vcpu_get_reg(vcpu, i));
> }
>
> +static unsigned long kvm_psci_check_allowed_function(struct kvm_vcpu *vcpu,
> u32 fn)
> +{
> + switch(fn) {
> + case PSCI_0_2_FN64_CPU_SUSPEND:
> + case PSCI_0_2_FN64_CPU_ON:
> + case PSCI_0_2_FN64_AFFINITY_INFO:
I checked in ARM DEN 0022D, those are indeed the only 3 functions that KVM
implements and have a different function ID based on the calling convention.
> + /* Disallow these functions for 32bit guests */
> + if (vcpu_mode_is_32bit(vcpu))
> + return PSCI_RET_NOT_SUPPORTED;
> + break;
> + }
> +
> + return 0;
> +}
> +
> static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = vcpu->kvm;
> @@ -206,6 +221,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> unsigned long val;
> int ret = 1;
>
> + val = kvm_psci_check_allowed_function(vcpu, psci_fn);
> + if (val)
> + goto out;
> +
> switch (psci_fn) {
> case PSCI_0_2_FN_PSCI_VERSION:
> /*
> @@ -273,6 +292,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> break;
> }
>
> +out:
> smccc_set_retval(vcpu, val, 0, 0, 0);
> return ret;
> }
> @@ -290,6 +310,10 @@ static int kvm_psci_1_0_call(struct kvm_vcpu *vcpu)
> break;
> case PSCI_1_0_FN_PSCI_FEATURES:
> feature = smccc_get_arg1(vcpu);
> + val = kvm_psci_check_allowed_function(vcpu, feature);
> + if (val)
> + break;
> +
> switch(feature) {
> case PSCI_0_2_FN_PSCI_VERSION:
> case PSCI_0_2_FN_CPU_SUSPEND:
The patch makes sense to me:
Reviewed-by: Alexandru Elisei
Thanks,
Alex
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 2/2] KVM: arm64: PSCI: Forbid 64bit functions for 32bit guests
On Wed, Apr 01, 2020 at 05:58:16PM +0100, Marc Zyngier wrote:
> Implementing (and even advertising) 64bit PSCI functions to 32bit
> guests is at least a bit odd, if not altogether violating the
> spec which says ("5.2.1 Register usage in arguments and return values"):
>
> "Adherence to the SMC Calling Conventions implies that any AArch32
> caller of an SMC64 function will get a return code of 0x(int32).
> This matches the NOT_SUPPORTED error code used in PSCI"
>
> Tighten the implementation by pretending these functions are not
> there for 32bit guests.
>
> Signed-off-by: Marc Zyngier
> ---
> virt/kvm/arm/psci.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index 69ff4a51ceb5..122795cdd984 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
> @@ -199,6 +199,21 @@ static void kvm_psci_narrow_to_32bit(struct kvm_vcpu
> *vcpu)
> vcpu_set_reg(vcpu, i, (u32)vcpu_get_reg(vcpu, i));
> }
>
> +static unsigned long kvm_psci_check_allowed_function(struct kvm_vcpu *vcpu,
> u32 fn)
> +{
> + switch(fn) {
> + case PSCI_0_2_FN64_CPU_SUSPEND:
> + case PSCI_0_2_FN64_CPU_ON:
> + case PSCI_0_2_FN64_AFFINITY_INFO:
> + /* Disallow these functions for 32bit guests */
> + if (vcpu_mode_is_32bit(vcpu))
> + return PSCI_RET_NOT_SUPPORTED;
> + break;
> + }
> +
> + return 0;
> +}
> +
> static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = vcpu->kvm;
> @@ -206,6 +221,10 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> unsigned long val;
> int ret = 1;
>
> + val = kvm_psci_check_allowed_function(vcpu, psci_fn);
> + if (val)
> + goto out;
> +
> switch (psci_fn) {
> case PSCI_0_2_FN_PSCI_VERSION:
> /*
> @@ -273,6 +292,7 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> break;
> }
>
> +out:
> smccc_set_retval(vcpu, val, 0, 0, 0);
> return ret;
> }
> @@ -290,6 +310,10 @@ static int kvm_psci_1_0_call(struct kvm_vcpu *vcpu)
> break;
> case PSCI_1_0_FN_PSCI_FEATURES:
> feature = smccc_get_arg1(vcpu);
> + val = kvm_psci_check_allowed_function(vcpu, feature);
> + if (val)
> + break;
> +
> switch(feature) {
> case PSCI_0_2_FN_PSCI_VERSION:
> case PSCI_0_2_FN_CPU_SUSPEND:
> --
> 2.25.0
>
Reviewed-by: Christoffer Dall
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
