On 08/04/2025 06:19, Gavin Shan wrote:
> On 4/8/25 2:34 AM, Steven Price wrote:
>> On 04/03/2025 06:01, Gavin Shan wrote:
>>> On 2/14/25 2:14 AM, Steven Price wrote:
>>>> From: Joey Gouly <joey.gouly@xxxxxxx>
>>>>
>>>> Forward RSI_HOST_CALLS to KVM's HVC handler.
>>>>
>>>> Signed-off-by: Joey Gouly <joey.gouly@xxxxxxx>
>>>> Signed-off-by: Steven Price <steven.price@xxxxxxx>
>>>> ---
>>>> Changes since v4:
>>>> * Setting GPRS is now done by kvm_rec_enter() rather than
>>>> rec_exit_host_call() (see previous patch - arm64: RME: Handle
>>>> realm
>>>> enter/exit). This fixes a bug where the registers set by user
>>>> space
>>>> were being ignored.
>>>> ---
>>>> arch/arm64/kvm/rme-exit.c | 22 ++++++++++++++++++++++
>>>> 1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/kvm/rme-exit.c b/arch/arm64/kvm/rme-exit.c
>>>> index c785005f821f..4f7602aa3c6c 100644
>>>> --- a/arch/arm64/kvm/rme-exit.c
>>>> +++ b/arch/arm64/kvm/rme-exit.c
>>>> @@ -107,6 +107,26 @@ static int rec_exit_ripas_change(struct kvm_vcpu
>>>> *vcpu)
>>>> return -EFAULT;
>>>> }
>>>> +static int rec_exit_host_call(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + int ret, i;
>>>> + struct realm_rec *rec = &vcpu->arch.rec;
>>>> +
>>>> + vcpu->stat.hvc_exit_stat++;
>>>> +
>>>> + for (i = 0; i < REC_RUN_GPRS; i++)
>>>> + vcpu_set_reg(vcpu, i, rec->run->exit.gprs[i]);
>>>> +
>>>> + ret = kvm_smccc_call_handler(vcpu);
>>>> +
>>>> + if (ret < 0) {
>>>> + vcpu_set_reg(vcpu, 0, ~0UL);
>>>> + ret = 1;
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>
>>> I don't understand how a negative error can be returned from
>>> kvm_smccc_call_handler().
>>
>> I don't believe it really can. However kvm_smccc_call_handler() calls
>> kvm_psci_call() and that has a documentation block which states:
>>
>> * This function returns: > 0 (success), 0 (success but exit to user
>> * space), and < 0 (errors)
>> *
>> * Errors:
>> * -EINVAL: Unrecognized PSCI function
>>
>> But I can't actually see code which returns the negative value...
>>
>
> I think the comments for kvm_psci_call() aren't correct since its return
> value
> can't be negative after 7e484d2785e2 ("KVM: arm64: Return NOT_SUPPORTED
> to guest
> for unknown PSCI version"). The comments should have been adjusted in
> that commit.
>
> Please take a look on 37c8e4947947 ("KVM: arm64: Let errors from SMCCC
> emulation
> to reach userspace"). Similarly, the block of code to set GPR0 to ~0ULL
> when negative
> error is returned from kvm_smccc_call_handler() in this patch needs to
> be dropped.
>
>>> Besides, SMCCC_RET_NOT_SUPPORTED has been set to GPR[0 - 3] if the
>>> request can't be
>>> supported. Why we need to set GPR[0] to ~0UL, which corresponds to
>>> SMCCC_RET_NOT_SUPPORTED
>>> if I'm correct. I guess change log or a comment to explain the questions
>>> would be
>>> nice.
>>
>> I'll add a comment explaining we don't expect negative codes. And I'll
>> expand ~0UL to SMCCC_RET_NOT_SUPPORTED which is what it should be.
>>
>
> Please refer to the above reply. The block of code needs to be dropped.
Thanks for the pointers, I had not been aware of that change. Yes this
code should be updated to match.
Thanks,
Steve
>> Thanks,
>> Steve
>>
>>>> static void update_arch_timer_irq_lines(struct kvm_vcpu *vcpu)
>>>> {
>>>> struct realm_rec *rec = &vcpu->arch.rec;
>>>> @@ -168,6 +188,8 @@ int handle_rec_exit(struct kvm_vcpu *vcpu, int
>>>> rec_run_ret)
>>>> return rec_exit_psci(vcpu);
>>>> case RMI_EXIT_RIPAS_CHANGE:
>>>> return rec_exit_ripas_change(vcpu);
>>>> + case RMI_EXIT_HOST_CALL:
>>>> + return rec_exit_host_call(vcpu);
>>>> }
>>>> kvm_pr_unimpl("Unsupported exit reason: %u\n",
>>>
>
> Thanks,
> Gavin
>
