RE: [PATCH 1/8] KVM: x86: #PF error-code on R/W operations is wrong

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux