Re: [PATCH v3 7/8] kvm: x86: mmu: Lockless access tracking for Intel CPUs without EPT A bits.

[Paolo Bonzini <pbonzini@redhat.com>]

On 16/12/2016 14:04, Xiao Guangrong wrote:
>> +    /*
>> +     * #PF can be fast if:
>> +     * 1. The shadow page table entry is not present, which could mean that
>> +     *    the fault is potentially caused by access tracking (if enabled).
>> +     * 2. The shadow page table entry is present and the fault
>> +     *    is caused by write-protect, that means we just need change the W
>> +     *    bit of the spte which can be done out of mmu-lock.
>> +     *
>> +     * However, if access tracking is disabled we know that a non-present
>> +     * page must be a genuine page fault where we have to create a new SPTE.
>> +     * So, if access tracking is disabled, we return true only for write
>> +     * accesses to a present page.
>> +     */
>> +
>> +    return shadow_acc_track_mask != 0 ||
>> +           ((error_code & (PFERR_WRITE_MASK | PFERR_PRESENT_MASK))
>> +        == (PFERR_WRITE_MASK | PFERR_PRESENT_MASK));
> 
> acc-track can not fix a WRITE-access, this should be:
> 
> !(error_code & (PFERR_WRITE_MASK)) && shadow_acc_track_mask != 0 || ...

Access tracking makes pages non-present, so a !W !P fault can sometimes
be fixed.

One possibility is to test is_access_track_pte, but it is handled a
little below the call to page_fault_can_be_fast:

            remove_acc_track = is_access_track_spte(spte);

            /* Verify that the fault can be handled in the fast path */
            if (!remove_acc_track && !remove_write_prot)
                    break;

It's not different from the way page_fault_can_be_fast return true for
writes, even if spte_can_locklessly_be_made_writable will return false
later.

So I think Junaid's patch is okay.

Junaid, of all comments from Guangrong I'm mostly interested in
kvm_mmu_clear_all_pte_masks.  What was the intended purpose?

Paolo