> -----Original Message----- > From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On > Behalf Of Nadav Amit > Sent: Thursday, December 25, 2014 8:52 AM > To: pbonzini@xxxxxxxxxx > Cc: kvm@xxxxxxxxxxxxxxx; Nadav Amit > Subject: [PATCH 1/8] KVM: x86: #PF error-code on R/W operations is wrong > > When emulating an instruction that reads the destination memory operand > (i.e., > instructions without the Mov flag in the emulator), the operand is first read. > If a page-fault is detected in this phase, the error-code which would be > delivered to the VM does not indicate that the access that caused the > exception > is a write one. This does not conform with real hardware, and may cause the > VM > to enter the page-fault handler twice for no reason (once for read, once for > write). > > Signed-off-by: Nadav Amit <namit@xxxxxxxxxxxxxxxxx> > --- > arch/x86/include/asm/kvm_host.h | 12 ++++++++++++ > arch/x86/kvm/emulate.c | 6 +++++- > arch/x86/kvm/mmu.h | 12 ------------ > 3 files changed, 17 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h > b/arch/x86/include/asm/kvm_host.h > index 73ccb12..d6f90ca 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -162,6 +162,18 @@ enum { > #define DR7_FIXED_1 0x00000400 > #define DR7_VOLATILE 0xffff2bff > > +#define PFERR_PRESENT_BIT 0 > +#define PFERR_WRITE_BIT 1 > +#define PFERR_USER_BIT 2 > +#define PFERR_RSVD_BIT 3 > +#define PFERR_FETCH_BIT 4 > + > +#define PFERR_PRESENT_MASK (1U << PFERR_PRESENT_BIT) > +#define PFERR_WRITE_MASK (1U << PFERR_WRITE_BIT) > +#define PFERR_USER_MASK (1U << PFERR_USER_BIT) > +#define PFERR_RSVD_MASK (1U << PFERR_RSVD_BIT) > +#define PFERR_FETCH_MASK (1U << PFERR_FETCH_BIT) > + > /* apic attention bits */ > #define KVM_APIC_CHECK_VAPIC 0 > /* > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 7a9697f..e5a84be 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -4941,8 +4941,12 @@ int x86_emulate_insn(struct x86_emulate_ctxt > *ctxt) > /* optimisation - avoid slow emulated read if Mov */ > rc = segmented_read(ctxt, ctxt->dst.addr.mem, > &ctxt->dst.val, ctxt->dst.bytes); This is a write operation, do you know why we need to read the dst operand first here? Thanks, Feng > - if (rc != X86EMUL_CONTINUE) > + if (rc != X86EMUL_CONTINUE) { > + if (rc == X86EMUL_PROPAGATE_FAULT && > + ctxt->exception.vector == PF_VECTOR) > + ctxt->exception.error_code |= PFERR_WRITE_MASK; > goto done; > + } > } > ctxt->dst.orig_val = ctxt->dst.val; > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > index 6b34876b..daae711 100644 > --- a/arch/x86/kvm/mmu.h > +++ b/arch/x86/kvm/mmu.h > @@ -44,18 +44,6 @@ > #define PT_DIRECTORY_LEVEL 2 > #define PT_PAGE_TABLE_LEVEL 1 > > -#define PFERR_PRESENT_BIT 0 > -#define PFERR_WRITE_BIT 1 > -#define PFERR_USER_BIT 2 > -#define PFERR_RSVD_BIT 3 > -#define PFERR_FETCH_BIT 4 > - > -#define PFERR_PRESENT_MASK (1U << PFERR_PRESENT_BIT) > -#define PFERR_WRITE_MASK (1U << PFERR_WRITE_BIT) > -#define PFERR_USER_MASK (1U << PFERR_USER_BIT) > -#define PFERR_RSVD_MASK (1U << PFERR_RSVD_BIT) > -#define PFERR_FETCH_MASK (1U << PFERR_FETCH_BIT) > - > static inline u64 rsvd_bits(int s, int e) > { > return ((1ULL << (e - s + 1)) - 1) << s; > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html