Re: [PATCH v4 2/9] KVM: SVM: Refactor SVM instruction handling on #GP intercept

[Yosry Ahmed]

On Fri, Apr 3, 2026 at 11:18 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> > +/* Return 0 if not SVM instr, otherwise return associated exit_code */
> > +static u64 svm_instr_exit_code(struct kvm_vcpu *vcpu)
>
> To make it very clear what this is doing how about:
>
> static u64 svm_get_decoded_instr_exit_code(struct kvm_vcpu *vcpu)

Yeah I like this better.

>
> >  {
> >       struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
> >
> >       if (ctxt->b != 0x1 || ctxt->opcode_len != 2)
> > -             return NONE_SVM_INSTR;
> > +             return 0;
>
> This should assert to ensure there's no collision with '0', i.e.
>
>         BUILD_BUG_ON(!SVM_EXIT_VMRUN || !SVM_EXIT_VMLOAD || !SVM_EXIT_VMSAVE);

I was gonna add this but I don't think these values can change anyway.
It's good for readability tho so I am fine with it.

>
>
> > +     if (!is_guest_mode(vcpu))
> > +             return kvm_emulate_instruction(vcpu,
> > +                             EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
>
> Since you're moving this anyways:
>
>         if (!is_guest_mode(vcpu))
>                 return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP |
>                                                      EMULTYPE_NO_DECODE);
>
>
> Actually!  Better idea, for this code and for the page_address_valid() checks:
> invert the checks to reduce indentation, i.e. end up with:
>
>         /* FIXME: Handle SVM instructions through the emulator */
>         svm_exit_code = svm_get_decoded_instr_exit_code(vcpu);
>         if (svm_exit_code) {
>                 if (!is_guest_mode(vcpu))
>                         return svm_invoke_exit_handler(vcpu, svm_exit_code);
>
>                 if (nested_svm_check_permissions(vcpu))
>                         return 1;
>
>                 if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))
>                         goto reinject;
>
>                 /*
>                  * FIXME: Only synthesize a #VMEXIT if L1 sets the intercept,
>                  * but only after the VMLOAD/VMSAVE exit handlers can properly
>                  * handle VMLOAD/VMSAVE from L2 with VLS enabled in L1 (i.e.
>                  * RAX is an L2 GPA that needs translation through L1's NPT).
>                  */
>                 nested_svm_simple_vmexit(svm, svm_exit_code);
>                 return 1;
>         }
>
>         /*
>          * VMware backdoor emulation on #GP interception only handles
>          * IN{S}, OUT{S}, and RDPMC, and only for L1.
>          */
>         if (!enable_vmware_backdoor || is_guest_mode(vcpu))
>                 goto reinject;
>
>         return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
>
> > +
> >  reinject:
> >       kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
> >       return 1;


The end result looks good to me.