Re: [PATCH v3 1/2] KVM: TDX: Allow userspace to return errors to guest for MAPGPA

[Sagi Shahar]

On Tue, Feb 17, 2026 at 1:20 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Feb 17, 2026, Michael Roth wrote:
> > On Tue, Feb 17, 2026 at 12:45:52PM -0600, Tom Lendacky wrote:
> > > On 2/17/26 12:05, Michael Roth wrote:
> > > >> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > >> index 2d7a4d52ccfb..056a44b9d78b 100644
> > > >> --- a/arch/x86/kvm/vmx/tdx.c
> > > >> +++ b/arch/x86/kvm/vmx/tdx.c
> > > >> @@ -1186,10 +1186,21 @@ static void __tdx_map_gpa(struct vcpu_tdx *tdx);
> > > >>
> > > >>  static int tdx_complete_vmcall_map_gpa(struct kvm_vcpu *vcpu)
> > > >>  {
> > > >> +        u64 hypercall_ret = READ_ONCE(vcpu->run->hypercall.ret);
> > > >>          struct vcpu_tdx *tdx = to_tdx(vcpu);
> > > >>
> > > >> -        if (vcpu->run->hypercall.ret) {
> > > >> -                tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
> > > >> +        if (hypercall_ret) {
> > > >> +                if (hypercall_ret == EAGAIN) {
> > > >> +                        tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_RETRY);
> > > >> +                } else if (vcpu->run->hypercall.ret == EINVAL) {
> > > >> +                        tdvmcall_set_return_code(
> > > >> +                                vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
> > > >> +                } else {
> > > >> +                        WARN_ON_ONCE(
> > > >> +                                kvm_is_valid_map_gpa_range_ret(hypercall_ret));
> > > >> +                        return -EINVAL;
> > > >> +                }
> > > >> +
> > > >>                  tdx->vp_enter_args.r11 = tdx->map_gpa_next;
> > > >>                  return 1;
> > > >>          }
> > > >
> > > > Maybe slightly more readable?
> > > >
> > > >     switch (hypercall_ret) {
> > > >     case EAGAIN:
> > > >         tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_RETRY);
> > > >         /* fallthrough */
> > >
> > > I think you want a break here, not a fallthrough, so that you don't set
> > > the return code twice with the last one not being correct for EAGAIN.
> >
> > Doh, thanks for the catch. I guess a break for the EINVAL case as well would
> > be more consistent then.
> >
> >     switch (hypercall_ret) {
> >     case EAGAIN:
> >         tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_RETRY);
> >         break;
> >     case EINVAL:
> >         tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
> >         break;
> >     case 0:
> >         break;
> >     case default:
> >         WARN_ON_ONCE(kvm_is_valid_map_gpa_range_ret(hypercall_ret));
> >         return -EINVAL;
> >     }
> >
> >     tdx->vp_enter_args.r11 = tdx->map_gpa_next;
> >     return 1;
>
> Heh, except then KVM will fail to handle the next chunk on success.  I like the
> idea of a switch statement, so what if we add that and dedup the error handling?
>
> static int tdx_complete_vmcall_map_gpa(struct kvm_vcpu *vcpu)
> {
>         u64 hypercall_ret = READ_ONCE(vcpu->run->hypercall.ret);
>         struct vcpu_tdx *tdx = to_tdx(vcpu);
>         long rc;
>
>         switch (hypercall_ret) {
>         case 0:
>                 break;
>         case EAGAIN:
>                 rc = TDVMCALL_STATUS_RETRY;
>                 goto propagate_error;
>         case EINVAL:
>                 rc = TDVMCALL_STATUS_INVALID_OPERAND;
>                 goto propagate_error;
>         default:
>                 WARN_ON_ONCE(kvm_is_valid_map_gpa_range_ret(hypercall_ret));
>                 return -EINVAL;
>         }
>
>         tdx->map_gpa_next += TDX_MAP_GPA_MAX_LEN;
>         if (tdx->map_gpa_next >= tdx->map_gpa_end)
>                 return 1;
>
>         /*
>          * Stop processing the remaining part if there is a pending interrupt,
>          * which could be qualified to deliver.  Skip checking pending RVI for
>          * TDVMCALL_MAP_GPA, see comments in tdx_protected_apic_has_interrupt().
>          */
>         if (kvm_vcpu_has_events(vcpu)) {
>                 rc = TDVMCALL_STATUS_RETRY;
>                 goto propagate_error;
>         }
>
>         __tdx_map_gpa(tdx);
>         return 0;
>
> propagate_error:
>         tdvmcall_set_return_code(vcpu, rc);
>         tdx->vp_enter_args.r11 = tdx->map_gpa_next;
>         return 1;
> }

Thanks for the review. I updated the code and sent out v4 for review.