From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752394AbcF1UWO (ORCPT ); Tue, 28 Jun 2016 16:22:14 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35021 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751977AbcF1UWM (ORCPT ); Tue, 28 Jun 2016 16:22:12 -0400 Subject: Re: [PATCH 3/5] mmu: don't set the present bit unconditionally To: Bandan Das References: <1467088360-10186-1-git-send-email-bsd@redhat.com> <1467088360-10186-4-git-send-email-bsd@redhat.com> <60e083e8-596a-5641-fcb9-ede8bce32b58@redhat.com> Cc: kvm@vger.kernel.org, guangrong.xiao@linux.intel.com, linux-kernel@vger.kernel.org From: Paolo Bonzini Message-ID: Date: Tue, 28 Jun 2016 22:21:56 +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: 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 28/06/2016 19:30, Bandan Das wrote: > Paolo Bonzini writes: >> 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. >> */ > > I think this should be: 2) for EPT page tables where an X-- page > table entry is invalid and a EPT misconfig is injected to the guest > before we reach here. Right, I messed this one up. shadow_user_mask and ACC_USER_MASK work the same way on processors that do not support execonly. Paolo >> 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); >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >