From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH 4/5] KVM: MMU: Optimize pte permission checks Date: Thu, 13 Sep 2012 20:09:39 +0800 Message-ID: <5051CD03.2080206@linux.vnet.ibm.com> References: <1347460194-11807-1-git-send-email-avi@redhat.com> <1347460194-11807-5-git-send-email-avi@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from e28smtp06.in.ibm.com ([122.248.162.6]:37945 "EHLO e28smtp06.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757730Ab2IMMJr (ORCPT ); Thu, 13 Sep 2012 08:09:47 -0400 Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 13 Sep 2012 17:39:45 +0530 Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8DC9gEw34341072 for ; Thu, 13 Sep 2012 17:39:42 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q8DC9evT028448 for ; Thu, 13 Sep 2012 22:09:41 +1000 In-Reply-To: <1347460194-11807-5-git-send-email-avi@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 09/12/2012 10:29 PM, Avi Kivity wrote: > walk_addr_generic() permission checks are a maze of branchy code, which is > performed four times per lookup. It depends on the type of access, efer.nxe, > cr0.wp, cr4.smep, and in the near future, cr4.smap. > > Optimize this away by precalculating all variants and storing them in a > bitmap. The bitmap is recalculated when rarely-changing variables change > (cr0, cr4) and is indexed by the often-changing variables (page fault error > code, pte access permissions). Really graceful! > > The result is short, branch-free code. > > Signed-off-by: Avi Kivity > +static void update_permission_bitmask(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu) > +{ > + unsigned bit, byte, pfec; > + u8 map; > + bool fault, x, w, u, wf, uf, ff, smep; > + > + smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP); > + for (byte = 0; byte < ARRAY_SIZE(mmu->permissions); ++byte) { > + pfec = byte << 1; > + map = 0; > + wf = pfec & PFERR_WRITE_MASK; > + uf = pfec & PFERR_USER_MASK; > + ff = pfec & PFERR_FETCH_MASK; > + for (bit = 0; bit < 8; ++bit) { > + x = bit & ACC_EXEC_MASK; > + w = bit & ACC_WRITE_MASK; > + u = bit & ACC_USER_MASK; > + > + /* Not really needed: !nx will cause pte.nx to fault */ > + x |= !mmu->nx; > + /* Allow supervisor writes if !cr0.wp */ > + w |= !is_write_protection(vcpu) && !uf; > + /* Disallow supervisor fetches if cr4.smep */ > + x &= !(smep && !uf); In the case of smep, supervisor mode can fetch the memory if pte.u == 0, so, it should be x &= !(smep && !uf && u)? > @@ -3672,20 +3672,18 @@ static int vcpu_mmio_gva_to_gpa(struct kvm_vcpu *vcpu, unsigned long gva, > gpa_t *gpa, struct x86_exception *exception, > bool write) > { > - u32 access = (kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0; > + u32 access = ((kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0) > + | (write ? PFERR_WRITE_MASK : 0); > + u8 bit = vcpu->arch.access; > > - if (vcpu_match_mmio_gva(vcpu, gva) && > - check_write_user_access(vcpu, write, access, > - vcpu->arch.access)) { > + if (vcpu_match_mmio_gva(vcpu, gva) > + && ((vcpu->arch.walk_mmu->permissions[access >> 1] >> bit) & 1)) { !((vcpu->arch.walk_mmu->permissions[access >> 1] >> bit) & 1) ? It is better introducing a function to do the permission check?