From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752417AbcF1I5v (ORCPT ); Tue, 28 Jun 2016 04:57:51 -0400 Received: from mail-wm0-f45.google.com ([74.125.82.45]:37188 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751419AbcF1I5t (ORCPT ); Tue, 28 Jun 2016 04:57:49 -0400 Subject: Re: [PATCH 3/5] mmu: don't set the present bit unconditionally To: Bandan Das , kvm@vger.kernel.org References: <1467088360-10186-1-git-send-email-bsd@redhat.com> <1467088360-10186-4-git-send-email-bsd@redhat.com> Cc: guangrong.xiao@linux.intel.com, linux-kernel@vger.kernel.org From: Paolo Bonzini Message-ID: <60e083e8-596a-5641-fcb9-ede8bce32b58@redhat.com> Date: Tue, 28 Jun 2016 10:57:45 +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: <1467088360-10186-4-git-send-email-bsd@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 28/06/2016 06:32, Bandan Das wrote: > + bool execonly = !(context->guest_rsvd_check.bad_mt_xwr & > + (1ull << VMX_EPT_EXECUTABLE_MASK)); > > if (set_mmio_spte(vcpu, sptep, gfn, pfn, pte_access)) > return 0; > > - spte = PT_PRESENT_MASK; > + if (!execonly) > + spte |= PT_PRESENT_MASK; This needs a comment: /* * There are two cases in which execonly is false: 1) for * non-EPT page tables, in which case we need to set the * P bit; 2) for EPT page tables where an X-- page table * entry is invalid, in which case we need to force the R * bit of the page table entry to 1. */ BUILD_BUG_ON(PT_PRESENT_MASK != VMX_EPT_READABLE_MASK); if (!execonly) spte |= PT_PRESENT_MASK; > if (!speculative) > spte |= shadow_accessed_mask; > > if (enable_ept) { > - kvm_mmu_set_mask_ptes(0ull, > + kvm_mmu_set_mask_ptes(PT_PRESENT_MASK, This should be VMX_EPT_READABLE_MASK. > @@ -2531,6 +2535,7 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > else > spte |= shadow_nx_mask; > > + /* In the EPT case, shadow_user_mask is PT_PRESENT_MASK */ I don't think this comment is necessary, but it's better to add one in FNAME(gpte_access). /* * In the EPT case, a page table can be executable but not * readable (on some processors). Therefore, set_spte does not * automatically set bit 0 if execute-only is supported. * Instead, since EPT page tables do not have a U bit, we * repurpose ACC_USER_MASK to signify readability. Likewise, * when EPT is in use shadow_user_mask is set to * VMX_EPT_READABLE_MASK. */ Thanks, Paolo > if (pte_access & ACC_USER_MASK) > spte |= shadow_user_mask; Paolo > (enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull, > (enable_ept_ad_bits) ? VMX_EPT_DIRTY_BIT : 0ull, > 0ull, VMX_EPT_EXECUTABLE_MASK);