Re: [PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
On Thu, Sep 04, 2014 at 07:44:51PM +0200, Paolo Bonzini wrote:
Il 04/09/2014 17:05, Gleb Natapov ha scritto:
If you do that, KVM gets down to the if (writeback) and writes the
ctxt-eip from L2 into the L1 EIP.
Heh, that's a bummer. We should not write back if an instruction caused a
vmexit.
You're right, that works.
Looks good!
Reviewed-by: Gleb Natapov g...@kernel.org
Paolo
-- 8 -
Subject: [PATCH] KVM: x86: skip writeback on injection of nested exception
If a nested page fault happens during emulation, we will inject a vmexit,
not a page fault. However because writeback happens after the injection,
we will write ctxt-eip from L2 into the L1 EIP. We do not write back
if an instruction caused an interception vmexit---do the same for page
faults.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
arch/x86/include/asm/kvm_host.h | 1 -
arch/x86/kvm/x86.c | 15 ++-
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 08cc299..c989651 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -893,7 +893,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, struct
x86_exception *fault);
int kvm_read_guest_page_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
gfn_t gfn, void *data, int offset, int len,
u32 access);
-void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception *fault);
bool kvm_require_cpl(struct kvm_vcpu *vcpu, int required_cpl);
static inline int __kvm_irq_line_state(unsigned long *irq_state,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e4ed85e..3541946 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -408,12 +408,14 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu,
struct x86_exception *fault)
}
EXPORT_SYMBOL_GPL(kvm_inject_page_fault);
-void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception *fault)
+static bool kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception
*fault)
{
if (mmu_is_nested(vcpu) !fault-nested_page_fault)
vcpu-arch.nested_mmu.inject_page_fault(vcpu, fault);
else
vcpu-arch.mmu.inject_page_fault(vcpu, fault);
+
+ return fault-nested_page_fault;
}
void kvm_inject_nmi(struct kvm_vcpu *vcpu)
@@ -4929,16 +4931,18 @@ static void toggle_interruptibility(struct kvm_vcpu
*vcpu, u32 mask)
}
}
-static void inject_emulated_exception(struct kvm_vcpu *vcpu)
+static bool inject_emulated_exception(struct kvm_vcpu *vcpu)
{
struct x86_emulate_ctxt *ctxt = vcpu-arch.emulate_ctxt;
if (ctxt-exception.vector == PF_VECTOR)
- kvm_propagate_fault(vcpu, ctxt-exception);
- else if (ctxt-exception.error_code_valid)
+ return kvm_propagate_fault(vcpu, ctxt-exception);
+
+ if (ctxt-exception.error_code_valid)
kvm_queue_exception_e(vcpu, ctxt-exception.vector,
ctxt-exception.error_code);
else
kvm_queue_exception(vcpu, ctxt-exception.vector);
+ return false;
}
static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
@@ -5300,8 +5304,9 @@ restart:
}
if (ctxt-have_exception) {
- inject_emulated_exception(vcpu);
r = EMULATE_DONE;
+ if (inject_emulated_exception(vcpu))
+ return r;
} else if (vcpu-arch.pio.count) {
if (!vcpu-arch.pio.in) {
/* FIXME: return into emulator if single-stepping. */
--
1.9.3
--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
On Tue, Sep 02, 2014 at 05:13:49PM +0200, Paolo Bonzini wrote:
This is required for the following patch to work correctly. If a nested page
fault happens during emulation, we must inject a vmexit, not a page fault.
Luckily we already have the required machinery: it is enough to return
X86EMUL_INTERCEPTED instead of X86EMUL_PROPAGATE_FAULT.
I wonder why this patch is needed. X86EMUL_PROPAGATE_FAULT causes
ctxt-have_exception to be set to true in x86_emulate_insn().
x86_emulate_instruction() checks ctxt-have_exception and calls
inject_emulated_exception() if it is true. inject_emulated_exception()
calls kvm_propagate_fault() where we check if the fault was nested and
generate vmexit or a page fault accordingly.
Reported-by: Valentine Sinitsyn valentine.sinit...@gmail.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
arch/x86/kvm/x86.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e4ed85e07a01..9e3b74c044ed 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -416,6 +416,16 @@ void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct
x86_exception *fault)
vcpu-arch.mmu.inject_page_fault(vcpu, fault);
}
+static inline int kvm_propagate_or_intercept(struct kvm_vcpu *vcpu,
+ struct x86_exception *exception)
+{
+ if (likely(!exception-nested_page_fault))
+ return X86EMUL_PROPAGATE_FAULT;
+
+ vcpu-arch.mmu.inject_page_fault(vcpu, exception);
+ return X86EMUL_INTERCEPTED;
+}
+
void kvm_inject_nmi(struct kvm_vcpu *vcpu)
{
atomic_inc(vcpu-arch.nmi_queued);
@@ -4122,7 +4132,7 @@ static int kvm_read_guest_virt_helper(gva_t addr, void
*val, unsigned int bytes,
int ret;
if (gpa == UNMAPPED_GVA)
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
ret = kvm_read_guest_page(vcpu-kvm, gpa PAGE_SHIFT, data,
offset, toread);
if (ret 0) {
@@ -4152,7 +4162,7 @@ static int kvm_fetch_guest_virt(struct x86_emulate_ctxt
*ctxt,
gpa_t gpa = vcpu-arch.walk_mmu-gva_to_gpa(vcpu, addr,
access|PFERR_FETCH_MASK,
exception);
if (unlikely(gpa == UNMAPPED_GVA))
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
offset = addr (PAGE_SIZE-1);
if (WARN_ON(offset + bytes PAGE_SIZE))
@@ -4203,7 +4213,7 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt
*ctxt,
int ret;
if (gpa == UNMAPPED_GVA)
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
ret = kvm_write_guest(vcpu-kvm, gpa, data, towrite);
if (ret 0) {
r = X86EMUL_IO_NEEDED;
@@ -4350,7 +4360,7 @@ static int emulator_read_write_onepage(unsigned long
addr, void *val,
ret = vcpu_mmio_gva_to_gpa(vcpu, addr, gpa, exception, write);
if (ret 0)
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
/* For APIC access vmexit */
if (ret)
--
1.8.3.1
--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
Il 04/09/2014 09:02, Gleb Natapov ha scritto:
On Tue, Sep 02, 2014 at 05:13:49PM +0200, Paolo Bonzini wrote:
This is required for the following patch to work correctly. If a nested
page
fault happens during emulation, we must inject a vmexit, not a page fault.
Luckily we already have the required machinery: it is enough to return
X86EMUL_INTERCEPTED instead of X86EMUL_PROPAGATE_FAULT.
I wonder why this patch is needed. X86EMUL_PROPAGATE_FAULT causes
ctxt-have_exception to be set to true in x86_emulate_insn().
x86_emulate_instruction() checks ctxt-have_exception and calls
inject_emulated_exception() if it is true. inject_emulated_exception()
calls kvm_propagate_fault() where we check if the fault was nested and
generate vmexit or a page fault accordingly.
Good question. :)
If you do that, KVM gets down to the if (writeback) and writes the
ctxt-eip from L2 into the L1 EIP.
Possibly this patch can be replaced by just this?
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 022513b..475e979 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5312,7 +5312,7 @@ restart:
if (ctxt-have_exception) {
inject_emulated_exception(vcpu);
- r = EMULATE_DONE;
+ return EMULATE_DONE;
} else if (vcpu-arch.pio.count) {
if (!vcpu-arch.pio.in) {
/* FIXME: return into emulator if single-stepping. */
But I'm not sure how to test it, and I like the idea of treating nested page
faults like other nested vmexits during emulation (which is what this patch
does).
If I included this patch, I could then remove kvm_propagate_fault
like (I think) this:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 92493e10937c..e096db566ac2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4910,9 +4902,10 @@ static void toggle_interruptibility(struct kvm_vcpu
*vcpu, u32 mask)
static void inject_emulated_exception(struct kvm_vcpu *vcpu)
{
struct x86_emulate_ctxt *ctxt = vcpu-arch.emulate_ctxt;
- if (ctxt-exception.vector == PF_VECTOR)
- kvm_propagate_fault(vcpu, ctxt-exception);
- else if (ctxt-exception.error_code_valid)
+ if (ctxt-exception.vector == PF_VECTOR) {
+ WARN_ON(fault-nested_page_fault);
+ vcpu-arch.walk_mmu-inject_page_fault(vcpu, fault);
+ } else if (ctxt-exception.error_code_valid)
kvm_queue_exception_e(vcpu, ctxt-exception.vector,
ctxt-exception.error_code);
else
What do you think?
Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
On Thu, Sep 04, 2014 at 04:12:19PM +0200, Paolo Bonzini wrote:
Il 04/09/2014 09:02, Gleb Natapov ha scritto:
On Tue, Sep 02, 2014 at 05:13:49PM +0200, Paolo Bonzini wrote:
This is required for the following patch to work correctly. If a nested
page
fault happens during emulation, we must inject a vmexit, not a page
fault.
Luckily we already have the required machinery: it is enough to return
X86EMUL_INTERCEPTED instead of X86EMUL_PROPAGATE_FAULT.
I wonder why this patch is needed. X86EMUL_PROPAGATE_FAULT causes
ctxt-have_exception to be set to true in x86_emulate_insn().
x86_emulate_instruction() checks ctxt-have_exception and calls
inject_emulated_exception() if it is true. inject_emulated_exception()
calls kvm_propagate_fault() where we check if the fault was nested and
generate vmexit or a page fault accordingly.
Good question. :)
If you do that, KVM gets down to the if (writeback) and writes the
ctxt-eip from L2 into the L1 EIP.
Heh, that's a bummer. We should not write back if an instruction caused a
vmexit.
Possibly this patch can be replaced by just this?
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 022513b..475e979 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5312,7 +5312,7 @@ restart:
if (ctxt-have_exception) {
inject_emulated_exception(vcpu);
- r = EMULATE_DONE;
+ return EMULATE_DONE;
If there was no vmexit we still want to writeback. Perhaps:
writeback = inject_emulated_exception(vcpu);
and return false if there was vmexit due to nested page fault (or any fault,
can't L1 ask for #GP/#UD intercept that need to be handled here too?)
} else if (vcpu-arch.pio.count) {
if (!vcpu-arch.pio.in) {
/* FIXME: return into emulator if single-stepping. */
But I'm not sure how to test it, and I like the idea of treating nested page
faults like other nested vmexits during emulation (which is what this patch
does).
IMO exits due to instruction intercept and exits due to other interceptable
events
that may happen during instruction emulation are sufficiently different to be
handled
slightly different. If my assumption about #GP above are correct with current
approach it
can be easily handled inside inject_emulated_exception().
--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
Il 04/09/2014 17:05, Gleb Natapov ha scritto:
if (ctxt-have_exception) {
inject_emulated_exception(vcpu);
- r = EMULATE_DONE;
+ return EMULATE_DONE;
If there was no vmexit we still want to writeback. Perhaps:
writeback = inject_emulated_exception(vcpu);
and return false if there was vmexit due to nested page fault (or any fault,
can't L1 ask for #GP/#UD intercept that need to be handled here too?)
Sounds good.
Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
Il 04/09/2014 17:05, Gleb Natapov ha scritto:
If you do that, KVM gets down to the if (writeback) and writes the
ctxt-eip from L2 into the L1 EIP.
Heh, that's a bummer. We should not write back if an instruction caused a
vmexit.
You're right, that works.
Paolo
-- 8 -
Subject: [PATCH] KVM: x86: skip writeback on injection of nested exception
If a nested page fault happens during emulation, we will inject a vmexit,
not a page fault. However because writeback happens after the injection,
we will write ctxt-eip from L2 into the L1 EIP. We do not write back
if an instruction caused an interception vmexit---do the same for page
faults.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
arch/x86/include/asm/kvm_host.h | 1 -
arch/x86/kvm/x86.c | 15 ++-
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 08cc299..c989651 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -893,7 +893,6 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, struct
x86_exception *fault);
int kvm_read_guest_page_mmu(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
gfn_t gfn, void *data, int offset, int len,
u32 access);
-void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception *fault);
bool kvm_require_cpl(struct kvm_vcpu *vcpu, int required_cpl);
static inline int __kvm_irq_line_state(unsigned long *irq_state,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e4ed85e..3541946 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -408,12 +408,14 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, struct
x86_exception *fault)
}
EXPORT_SYMBOL_GPL(kvm_inject_page_fault);
-void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception *fault)
+static bool kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception
*fault)
{
if (mmu_is_nested(vcpu) !fault-nested_page_fault)
vcpu-arch.nested_mmu.inject_page_fault(vcpu, fault);
else
vcpu-arch.mmu.inject_page_fault(vcpu, fault);
+
+ return fault-nested_page_fault;
}
void kvm_inject_nmi(struct kvm_vcpu *vcpu)
@@ -4929,16 +4931,18 @@ static void toggle_interruptibility(struct kvm_vcpu
*vcpu, u32 mask)
}
}
-static void inject_emulated_exception(struct kvm_vcpu *vcpu)
+static bool inject_emulated_exception(struct kvm_vcpu *vcpu)
{
struct x86_emulate_ctxt *ctxt = vcpu-arch.emulate_ctxt;
if (ctxt-exception.vector == PF_VECTOR)
- kvm_propagate_fault(vcpu, ctxt-exception);
- else if (ctxt-exception.error_code_valid)
+ return kvm_propagate_fault(vcpu, ctxt-exception);
+
+ if (ctxt-exception.error_code_valid)
kvm_queue_exception_e(vcpu, ctxt-exception.vector,
ctxt-exception.error_code);
else
kvm_queue_exception(vcpu, ctxt-exception.vector);
+ return false;
}
static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
@@ -5300,8 +5304,9 @@ restart:
}
if (ctxt-have_exception) {
- inject_emulated_exception(vcpu);
r = EMULATE_DONE;
+ if (inject_emulated_exception(vcpu))
+ return r;
} else if (vcpu-arch.pio.count) {
if (!vcpu-arch.pio.in) {
/* FIXME: return into emulator if single-stepping. */
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] KVM: x86: inject nested page faults on emulated instructions
This is required for the following patch to work correctly. If a nested page
fault happens during emulation, we must inject a vmexit, not a page fault.
Luckily we already have the required machinery: it is enough to return
X86EMUL_INTERCEPTED instead of X86EMUL_PROPAGATE_FAULT.
Reported-by: Valentine Sinitsyn valentine.sinit...@gmail.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
arch/x86/kvm/x86.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e4ed85e07a01..9e3b74c044ed 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -416,6 +416,16 @@ void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct
x86_exception *fault)
vcpu-arch.mmu.inject_page_fault(vcpu, fault);
}
+static inline int kvm_propagate_or_intercept(struct kvm_vcpu *vcpu,
+struct x86_exception *exception)
+{
+ if (likely(!exception-nested_page_fault))
+ return X86EMUL_PROPAGATE_FAULT;
+
+ vcpu-arch.mmu.inject_page_fault(vcpu, exception);
+ return X86EMUL_INTERCEPTED;
+}
+
void kvm_inject_nmi(struct kvm_vcpu *vcpu)
{
atomic_inc(vcpu-arch.nmi_queued);
@@ -4122,7 +4132,7 @@ static int kvm_read_guest_virt_helper(gva_t addr, void
*val, unsigned int bytes,
int ret;
if (gpa == UNMAPPED_GVA)
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
ret = kvm_read_guest_page(vcpu-kvm, gpa PAGE_SHIFT, data,
offset, toread);
if (ret 0) {
@@ -4152,7 +4162,7 @@ static int kvm_fetch_guest_virt(struct x86_emulate_ctxt
*ctxt,
gpa_t gpa = vcpu-arch.walk_mmu-gva_to_gpa(vcpu, addr,
access|PFERR_FETCH_MASK,
exception);
if (unlikely(gpa == UNMAPPED_GVA))
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
offset = addr (PAGE_SIZE-1);
if (WARN_ON(offset + bytes PAGE_SIZE))
@@ -4203,7 +4213,7 @@ int kvm_write_guest_virt_system(struct x86_emulate_ctxt
*ctxt,
int ret;
if (gpa == UNMAPPED_GVA)
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
ret = kvm_write_guest(vcpu-kvm, gpa, data, towrite);
if (ret 0) {
r = X86EMUL_IO_NEEDED;
@@ -4350,7 +4360,7 @@ static int emulator_read_write_onepage(unsigned long
addr, void *val,
ret = vcpu_mmio_gva_to_gpa(vcpu, addr, gpa, exception, write);
if (ret 0)
- return X86EMUL_PROPAGATE_FAULT;
+ return kvm_propagate_or_intercept(vcpu, exception);
/* For APIC access vmexit */
if (ret)
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
