From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][PATCH 2/2] kvm: x86: mmio: fix setting the present bit of mmio spte Date: Mon, 17 Nov 2014 09:55:10 +0800 Message-ID: <5469557E.9020402@intel.com> References: <1415957488-27490-1-git-send-email-tiejun.chen@intel.com> <1415957488-27490-2-git-send-email-tiejun.chen@intel.com> <5465D53C.9040007@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Paolo Bonzini Return-path: Received: from mga14.intel.com ([192.55.52.115]:26229 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752018AbaKQBzN (ORCPT ); Sun, 16 Nov 2014 20:55:13 -0500 In-Reply-To: <5465D53C.9040007@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2014/11/14 18:11, Paolo Bonzini wrote: > > > On 14/11/2014 10:31, Tiejun Chen wrote: >> In PAE case maxphyaddr may be 52bit as well, we also need to >> disable mmio page fault. Here we can check MMIO_SPTE_GEN_HIGH_SHIFT >> directly to determine if we should set the present bit, and >> bring a little cleanup. >> >> Signed-off-by: Tiejun Chen >> --- >> arch/x86/include/asm/kvm_host.h | 1 + >> arch/x86/kvm/mmu.c | 23 +++++++++++++++++++++++ >> arch/x86/kvm/x86.c | 30 ------------------------------ >> 3 files changed, 24 insertions(+), 30 deletions(-) >> >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index dc932d3..667f2b6 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -809,6 +809,7 @@ void kvm_mmu_write_protect_pt_masked(struct kvm *kvm, >> struct kvm_memory_slot *slot, >> gfn_t gfn_offset, unsigned long mask); >> void kvm_mmu_zap_all(struct kvm *kvm); >> +void kvm_set_mmio_spte_mask(void); >> void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm); >> unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm); >> void kvm_mmu_change_mmu_pages(struct kvm *kvm, unsigned int kvm_nr_mmu_pages); >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >> index ac1c4de..8e4be36 100644 >> --- a/arch/x86/kvm/mmu.c >> +++ b/arch/x86/kvm/mmu.c >> @@ -295,6 +295,29 @@ static bool check_mmio_spte(struct kvm *kvm, u64 spte) >> return likely(kvm_gen == spte_gen); >> } >> >> +/* >> + * Set the reserved bits and the present bit of an paging-structure >> + * entry to generate page fault with PFER.RSV = 1. >> + */ >> +void kvm_set_mmio_spte_mask(void) >> +{ >> + u64 mask; >> + int maxphyaddr = boot_cpu_data.x86_phys_bits; >> + >> + /* Mask the reserved physical address bits. */ >> + mask = rsvd_bits(maxphyaddr, MMIO_SPTE_GEN_HIGH_SHIFT - 1); >> + >> + /* Magic bits are always reserved for 32bit host. */ >> + mask |= 0x3ull << 62; > > This should be enough to trigger the page fault on PAE systems. > > The problem is specific to non-EPT 64-bit hosts, where the PTEs have no > reserved bits beyond 51:MAXPHYADDR. On EPT we use WX- permissions to > trigger EPT misconfig, on 32-bit systems we have bit 62. Thanks for your explanation. > >> + /* Set the present bit to enable mmio page fault. */ >> + if (maxphyaddr < MMIO_SPTE_GEN_HIGH_SHIFT) >> + mask = PT_PRESENT_MASK; > > Shouldn't this be "|=" anyway, instead of "="? > Yeah, just miss this. Thanks a lot, I will fix this in next revision. Thanks Tiejun