From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wu, Feng" Subject: RE: [PATCH 1/8] KVM: x86: #PF error-code on R/W operations is wrong Date: Fri, 26 Dec 2014 07:25:27 +0000 Message-ID: References: <1419468743-23732-1-git-send-email-namit@cs.technion.ac.il> <1419468743-23732-2-git-send-email-namit@cs.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: "kvm@vger.kernel.org" , "Wu, Feng" To: Nadav Amit , "pbonzini@redhat.com" Return-path: Received: from mga01.intel.com ([192.55.52.88]:12649 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751118AbaLZH0O convert rfc822-to-8bit (ORCPT ); Fri, 26 Dec 2014 02:26:14 -0500 In-Reply-To: <1419468743-23732-2-git-send-email-namit@cs.technion.ac.il> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: > -----Original Message----- > From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On > Behalf Of Nadav Amit > Sent: Thursday, December 25, 2014 8:52 AM > To: pbonzini@redhat.com > Cc: kvm@vger.kernel.org; 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 > --- > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html