From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752596AbcGMIKe (ORCPT ); Wed, 13 Jul 2016 04:10:34 -0400 Received: from mail-lf0-f68.google.com ([209.85.215.68]:35427 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751227AbcGMIKV (ORCPT ); Wed, 13 Jul 2016 04:10:21 -0400 Subject: Re: [PATCH v2 2/5] mmu: don't set the present bit unconditionally To: Bandan Das , kvm@vger.kernel.org References: <1468361932-16580-1-git-send-email-bsd@redhat.com> <1468361932-16580-3-git-send-email-bsd@redhat.com> Cc: guangrong.xiao@linux.intel.com, kernellwp@gmail.com, linux-kernel@vger.kernel.org From: Paolo Bonzini Message-ID: <942237ee-a673-5cbf-0e7e-8e9075c904ec@redhat.com> Date: Wed, 13 Jul 2016 10:10:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <1468361932-16580-3-git-send-email-bsd@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13/07/2016 00:18, Bandan Das wrote: > + /* > + * In the non-EPT case, execonly is not valid and so > + * the following line is equivalent to spte |= PT_PRESENT_MASK. I think the comment need not mention non-EPT. > + * For the EPT case, shadow_present_mask is 0 if hardware > + * supports it and we honor whatever way the guest set it. if hardware supports exec-only page table entries. > + * See: FNAME(gpte_access) in paging_tmpl.h > + */ > + spte |= shadow_present_mask; > if (!speculative) > spte |= shadow_accessed_mask; > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index bc019f7..f2741db 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -181,13 +181,19 @@ no_present: > return true; > } > > +/* > + * For PTTYPE_EPT, a page table can be executable but not readable > + * on supported processors. Therefore, set_spte does not automatically > + * set bit 0 if execute only is supported. Here, we repurpose ACC_USER_MASK > + * to signify readability since it isn't used in the EPT case > + */ > static inline unsigned FNAME(gpte_access)(struct kvm_vcpu *vcpu, u64 gpte) > { > unsigned access; > #if PTTYPE == PTTYPE_EPT > access = ((gpte & VMX_EPT_WRITABLE_MASK) ? ACC_WRITE_MASK : 0) | > ((gpte & VMX_EPT_EXECUTABLE_MASK) ? ACC_EXEC_MASK : 0) | > - ACC_USER_MASK; > + ((gpte & VMX_EPT_READABLE_MASK) ? ACC_USER_MASK : 0); > #else > BUILD_BUG_ON(ACC_EXEC_MASK != PT_PRESENT_MASK); > BUILD_BUG_ON(ACC_EXEC_MASK != 1); > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 64a79f2..f73b5dc 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -6366,10 +6366,13 @@ static __init int hardware_setup(void) > vmx_disable_intercept_msr_write_x2apic(0x83f); > > if (enable_ept) { > - kvm_mmu_set_mask_ptes(0ull, > + kvm_mmu_set_mask_ptes(VMX_EPT_READABLE_MASK, > (enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull, > (enable_ept_ad_bits) ? VMX_EPT_DIRTY_BIT : 0ull, > - 0ull, VMX_EPT_EXECUTABLE_MASK); > + 0ull, VMX_EPT_EXECUTABLE_MASK, > + cpu_has_vmx_ept_execute_only() ? > + 0ull : PT_PRESENT_MASK); Better to use VMX_EPT_READABLE_MASK here, which removes the need for the BUILD_BUG_ON. I can do these changes myself. Paolo > + BUILD_BUG_ON(PT_PRESENT_MASK != VMX_EPT_READABLE_MASK);