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
--
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
