Paolo Bonzini <pbonzini@xxxxxxxxxx> 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. > 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html