Re: [PATCH v7 28/45] arm64: rme: support RSI_HOST_CALL

[Gavin Shan]

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,
> 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