2018-09-06 08:03:50

by Yi Wang

[permalink] [raw]
Subject: [PATCH] KVM: x86: fix failure of injecting exceptions in x86_emulate_instruction

In order to fix a page table walk issue, commit 6ea6e84309ca
("KVM: x86: inject exceptions produced by x86_decode_insn") check
if variable ctxt->have_exception true and inject the exception.
Unfortunately, ctxt->have_exception is set to true only in function
x86_emulate_insn(), which won't be called before invoking
inject_emulated_exception() in the 6ea6e84309ca.
This patch fix this issue.

Signed-off-by: Yi Wang <[email protected]>
Reviewed-by: Xi Xu <[email protected]>
---
arch/x86/kvm/emulate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 106482d..aecf9a72 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -5105,8 +5105,11 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
memcpy(ctxt->fetch.data, insn, insn_len);
else {
rc = __do_insn_fetch_bytes(ctxt, 1);
- if (rc != X86EMUL_CONTINUE)
+ if (rc != X86EMUL_CONTINUE) {
+ if (rc == X86EMUL_PROPAGATE_FAULT)
+ ctxt->have_exception = true;
return rc;
+ }
}

switch (mode) {
--
1.8.3.1



2018-09-20 16:32:38

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] KVM: x86: fix failure of injecting exceptions in x86_emulate_instruction

On 06/09/2018 10:02, Yi Wang wrote:
> In order to fix a page table walk issue, commit 6ea6e84309ca
> ("KVM: x86: inject exceptions produced by x86_decode_insn") check
> if variable ctxt->have_exception true and inject the exception.
> Unfortunately, ctxt->have_exception is set to true only in function
> x86_emulate_insn(), which won't be called before invoking
> inject_emulated_exception() in the 6ea6e84309ca.
> This patch fix this issue.
>
> Signed-off-by: Yi Wang <[email protected]>
> Reviewed-by: Xi Xu <[email protected]>

Queued, thanks. Do you have a testcase for this?

> ---
> arch/x86/kvm/emulate.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 106482d..aecf9a72 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -5105,8 +5105,11 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
> memcpy(ctxt->fetch.data, insn, insn_len);
> else {
> rc = __do_insn_fetch_bytes(ctxt, 1);
> - if (rc != X86EMUL_CONTINUE)
> + if (rc != X86EMUL_CONTINUE) {
> + if (rc == X86EMUL_PROPAGATE_FAULT)
> + ctxt->have_exception = true;
> return rc;
> + }
> }
>
> switch (mode) {
>


2018-09-20 16:54:20

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] KVM: x86: fix failure of injecting exceptions in x86_emulate_instruction

On 20/09/2018 18:30, Paolo Bonzini wrote:
> On 06/09/2018 10:02, Yi Wang wrote:
>> In order to fix a page table walk issue, commit 6ea6e84309ca
>> ("KVM: x86: inject exceptions produced by x86_decode_insn") check
>> if variable ctxt->have_exception true and inject the exception.
>> Unfortunately, ctxt->have_exception is set to true only in function
>> x86_emulate_insn(), which won't be called before invoking
>> inject_emulated_exception() in the 6ea6e84309ca.
>> This patch fix this issue.
>>
>> Signed-off-by: Yi Wang <[email protected]>
>> Reviewed-by: Xi Xu <[email protected]>
>
> Queued, thanks. Do you have a testcase for this?

Unqueued, sorry. The hypercall test from kvm-unit-tests fails. A
VMCALL on the "edge" of canonical address space, i.e. at 0x7ffffffffffd,
raises a general protection fault before this patch and a double fault
afterwards.

Paolo

2018-09-20 17:51:13

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH] KVM: x86: fix failure of injecting exceptions in x86_emulate_instruction

+cc Peng Hao and Jiang Biao

On Thu, Sep 20, 2018 at 06:53:55PM +0200, Paolo Bonzini wrote:
> On 20/09/2018 18:30, Paolo Bonzini wrote:
> > On 06/09/2018 10:02, Yi Wang wrote:
> >> In order to fix a page table walk issue, commit 6ea6e84309ca
> >> ("KVM: x86: inject exceptions produced by x86_decode_insn") check
> >> if variable ctxt->have_exception true and inject the exception.
> >> Unfortunately, ctxt->have_exception is set to true only in function
> >> x86_emulate_insn(), which won't be called before invoking
> >> inject_emulated_exception() in the 6ea6e84309ca.
> >> This patch fix this issue.
> >>
> >> Signed-off-by: Yi Wang <[email protected]>
> >> Reviewed-by: Xi Xu <[email protected]>
> >
> > Queued, thanks. Do you have a testcase for this?
>
> Unqueued, sorry. The hypercall test from kvm-unit-tests fails. A
> VMCALL on the "edge" of canonical address space, i.e. at 0x7ffffffffffd,
> raises a general protection fault before this patch and a double fault
> afterwards.

Peng Hao submitted a very similar patch[1], the difference being that
it also modified x86_emulate_instruction() to ignore the result of
inject_emulated_exception(). I have no idea if that change is correct
but it may be related to the aforementioned unit test failing.

[1] https://lkml.org/lkml/2018/9/19/38