From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9Z6k-0005D6-27 for qemu-devel@nongnu.org; Thu, 17 Dec 2015 09:00:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9Z6f-0005kr-Jf for qemu-devel@nongnu.org; Thu, 17 Dec 2015 09:00:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17507) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9Z6f-0005k8-AI for qemu-devel@nongnu.org; Thu, 17 Dec 2015 09:00:33 -0500 From: Paolo Bonzini Date: Thu, 17 Dec 2015 15:00:29 +0100 Message-Id: <1450360829-1955-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH] target-i386: do not duplicate page protection checks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: ehabkost@redhat.com, rth@twiddle.net x86_cpu_handle_mmu_fault is currently checking twice for writability and executability of pages; the first time to decide whether to trigger a page fault, the second time to compute the "prot" argument to tlb_set_page_with_attrs. Reorganize code so that first "prot" is computed, then it is used to check whether to raise a page fault, then finally PROT_WRITE is removed if the D bit will have to be set. Signed-off-by: Paolo Bonzini --- target-i386/helper.c | 65 +++++++++++++++++++--------------------------------- 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/target-i386/helper.c b/target-i386/helper.c index d18be95..bf58242 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -890,38 +890,30 @@ do_check_protect_pse36: goto do_fault_rsvd; } ptep ^= PG_NX_MASK; - if ((ptep & PG_NX_MASK) && is_write1 == 2) { + + /* can the page can be put in the TLB? prot will tell us */ + if (is_user && !(ptep & PG_USER_MASK)) { goto do_fault_protect; } - switch (mmu_idx) { - case MMU_USER_IDX: - if (!(ptep & PG_USER_MASK)) { - goto do_fault_protect; - } - if (is_write && !(ptep & PG_RW_MASK)) { - goto do_fault_protect; - } - break; - case MMU_KSMAP_IDX: - if (is_write1 != 2 && (ptep & PG_USER_MASK)) { - goto do_fault_protect; + prot = 0; + if (mmu_idx != MMU_KSMAP_IDX || !(ptep & PG_USER_MASK)) { + prot |= PAGE_READ; + if ((ptep & PG_RW_MASK) || (!is_user && !(env->cr[0] & CR0_WP_MASK))) { + prot |= PAGE_WRITE; } - /* fall through */ - case MMU_KNOSMAP_IDX: - if (is_write1 == 2 && (env->cr[4] & CR4_SMEP_MASK) && - (ptep & PG_USER_MASK)) { - goto do_fault_protect; - } - if ((env->cr[0] & CR0_WP_MASK) && - is_write && !(ptep & PG_RW_MASK)) { - goto do_fault_protect; - } - break; + } + if (!(ptep & PG_NX_MASK) && + (mmu_idx == MMU_USER_IDX || + !((env->cr[4] & CR4_SMEP_MASK) && (ptep & PG_USER_MASK)))) { + prot |= PAGE_EXEC; + } - default: /* cannot happen */ - break; + if ((prot & (1 << is_write1)) == 0) { + goto do_fault_protect; } + + /* yes, it can! */ is_dirty = is_write && !(pte & PG_DIRTY_MASK); if (!(pte & PG_ACCESSED_MASK) || is_dirty) { pte |= PG_ACCESSED_MASK; @@ -931,25 +923,13 @@ do_check_protect_pse36: x86_stl_phys_notdirty(cs, pte_addr, pte); } - /* the page can be put in the TLB */ - prot = PAGE_READ; - if (!(ptep & PG_NX_MASK) && - (mmu_idx == MMU_USER_IDX || - !((env->cr[4] & CR4_SMEP_MASK) && (ptep & PG_USER_MASK)))) { - prot |= PAGE_EXEC; - } - if (pte & PG_DIRTY_MASK) { + if (!(pte & PG_DIRTY_MASK)) { /* only set write access if already dirty... otherwise wait for dirty access */ - if (is_user) { - if (ptep & PG_RW_MASK) - prot |= PAGE_WRITE; - } else { - if (!(env->cr[0] & CR0_WP_MASK) || - (ptep & PG_RW_MASK)) - prot |= PAGE_WRITE; - } + assert(!is_write); + prot &= ~PROT_WRITE; } + do_mapping: pte = pte & env->a20_mask; @@ -962,6 +942,7 @@ do_check_protect_pse36: page_offset = vaddr & (page_size - 1); paddr = pte + page_offset; + assert(prot & (1 << is_write1)); tlb_set_page_with_attrs(cs, vaddr, paddr, cpu_get_mem_attrs(env), prot, mmu_idx, page_size); return 0; -- 2.5.0