> > >> Let's separate the three conditions (R/W/X):
> > >>
> > >> if ((error_code & PFERR_FETCH_MASK) {
> > >> if ((spte & (shadow_x_mask|shadow_nx_mask))
> > >> == shadow_x_mask) {
> > >> fault_handled = true;
> > >> break;
> > >> }
> > >> }
> > >> if (error_code & PFERR_WRITE_MASK) {
> > >> if (is_writable_pte(spte)) {
> > >> fault_handled = true;
> > >> break;
> > >> }
> > >> remove_write_prot =
> > >> spte_can_locklessly_be_made_writable(spte);
> > >> }
> > >> if (!(error_code & PFERR_PRESENT_MASK)) {
> > >> if (!is_access_track_spte(spte)) {
> > >> fault_handled = true;
> > >> break;
> > >> }
> > >> remove_acc_track = true;
> > >> }
> > >
> > > I think the third block is incorrect e.g. it will set fault_handled =
> > > true even
> > > for a completely zero PTE.
> >
> > A completely zero PTE would have been filtered before by the
> > is_shadow_present_pte check, wouldn't it?
>
> Oh, the is_shadow_present_pte check was actually removed in the patch. We could
> add it back, minus the ret = true statement, and then it would filter the zero
> PTE case. But I still think that the other form:
>
> if ((error_code & PFERR_USER_MASK) &&
> (spte & PT_PRESENT_MASK)) {
> fault_handled = true;
> break;
> }
>
> is simpler as it is directly analogous to the cases for fetch and write.
> Please let me know if you think otherwise.
Fair enough, but add a comment to explain the error_code check because I
don't get it. :)
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
