Re: [PATCH v2 1/3] KVM: x86: Exit to user-mode on #UD intercept when emulator requires

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/11/2017 09:12, Liran Alon wrote:
> 
> 
> On 07/11/17 02:47, Wanpeng Li wrote:
>> 2017-11-06 22:39 GMT+08:00 Liran Alon <liran.alon@oracle.com>:
>>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>>> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
>>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: stable@vger.kernel.org
>>
>> Except the changelog.
> Thanks for the review.
> Currently both you and Paolo added "Reviewed-by" to this commit.
> 
> Is there anything else you wish me to add to the commit message before
> this commit being accepted? Do you have a suggestion? I though the
> commit-title explains it enough for this trivial patch and didn't saw
> any complain about not having body by ./scripts/checkpatch.pl.
> 
> In addition, if I would need to edit commit message body, should I send
> the next version of this commit as a standalone or re-send the entire
> series?

No, don't worry.  Generally, when a maintainer adds a Reviewed-by it
means that it's just a matter of time before the patch goes in.

For the commit message, I was thinking of something like:

---
Instruction emulation after trapping a #UD exception can result in an
MMIO access, for example when emulating a MOVBE on a processor that
doesn't support the instruction.  In this case, the #UD vmexit handler
must exit to user mode, but there wasn't any code to do so.  Add it for
both VMX and SVM.
---

Sounds good?

Paolo

> Thanks,
> -Liran
> 
>>
>> Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>
>>
>>> ---
>>>   arch/x86/kvm/svm.c | 2 ++
>>>   arch/x86/kvm/vmx.c | 2 ++
>>>   2 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>>> index 0e68f0b3cbf7..e0162b20e3c9 100644
>>> --- a/arch/x86/kvm/svm.c
>>> +++ b/arch/x86/kvm/svm.c
>>> @@ -2189,6 +2189,8 @@ static int ud_interception(struct vcpu_svm *svm)
>>>          int er;
>>>
>>>          er = emulate_instruction(&svm->vcpu, EMULTYPE_TRAP_UD);
>>> +       if (er == EMULATE_USER_EXIT)
>>> +               return 0;
>>>          if (er != EMULATE_DONE)
>>>                  kvm_queue_exception(&svm->vcpu, UD_VECTOR);
>>>          return 1;
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 95a01609d7ee..2b63d9edc207 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -5886,6 +5886,8 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>>>                          return 1;
>>>                  }
>>>                  er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
>>> +               if (er == EMULATE_USER_EXIT)
>>> +                       return 0;
>>>                  if (er != EMULATE_DONE)
>>>                          kvm_queue_exception(vcpu, UD_VECTOR);
>>>                  return 1;
>>> -- 
>>> 1.9.1
>>>