On 28/06/2016 19:30, Bandan Das wrote:
> Paolo Bonzini <pbonz...@redhat.com> 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 majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
