Re: [PATCH v2 1/6] KVM: SVM: Use maxphyaddr in emulator RAX check for VMRUN/VMLOAD/VMSAVE

[Yosry Ahmed]

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c8e292e9a24d..74df977a38ca 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3867,18 +3867,10 @@ static int check_svme(struct x86_emulate_ctxt *ctxt)
>         if (!(efer & EFER_SVME))
>                 return emulate_ud(ctxt);
>
> -       return X86EMUL_CONTINUE;
> -}
> -
> -static int check_svme_pa(struct x86_emulate_ctxt *ctxt)
> -{
> -       u64 rax = reg_read(ctxt, VCPU_REGS_RAX);
> -
> -       /* Valid physical address? */
> -       if (rax & 0xffff000000000000ULL)
> +       if (ctxt->ops->cpl(ctxt))
>                 return emulate_gp(ctxt, 0);
>
> -       return check_svme(ctxt);
> +       return X86EMUL_CONTINUE;
>  }
>
>  static int check_rdtsc(struct x86_emulate_ctxt *ctxt)
> @@ -3984,10 +3976,10 @@ static const struct opcode group7_rm2[] = {
>  };
>
>  static const struct opcode group7_rm3[] = {
> -       DIP(SrcNone | Prot | Priv,              vmrun,          check_svme_pa),
> +       DIP(SrcNone | Prot | Priv,              vmrun,          check_svme),
>         II(SrcNone  | Prot | EmulateOnUD,       em_hypercall,   vmmcall),
> -       DIP(SrcNone | Prot | Priv,              vmload,         check_svme_pa),
> -       DIP(SrcNone | Prot | Priv,              vmsave,         check_svme_pa),
> +       DIP(SrcNone | Prot | Priv,              vmload,         check_svme),
> +       DIP(SrcNone | Prot | Priv,              vmsave,         check_svme),
>         DIP(SrcNone | Prot | Priv,              stgi,           check_svme),
>         DIP(SrcNone | Prot | Priv,              clgi,           check_svme),
>         DIP(SrcNone | Prot | Priv,              skinit,         check_svme),
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e6691c044913..e1223c07593b 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2294,7 +2294,7 @@ static int gp_interception(struct kvm_vcpu *vcpu)
>                                 EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
>         } else {
>                 /* All SVM instructions expect page aligned RAX */
> -               if (svm->vmcb->save.rax & ~PAGE_MASK)
> +               if (!page_address_valid(vcpu, svm->vmcb->save.rax))
>                         goto reinject;

Final observation (hopefully), this check needs to be moved to the
VMRUN/VMLOAD/VMSAVE interception functions. As kvm_vcpu_map() failures
will stop injecting #GP, we still need to handle the case where
allow_smaller_maxphyaddr is used and the GPA is illegal from the
vCPU's perspective but not the host. In this case, the CPU does not
inject a #GP and the instructions are intercepted, so we need to make
sure the interception functions do the legality check on RAX.

I am doing this in a separate patch after fixing the check, and
opportunistically cleaning up gp_interception() by doing an early
return for the non-SVM insn case to reduce indentation. Will send v3
out after I am done testing.