Re: [PATCH RFT 0/3] mm, drm: ensure .fault() does not have to be followed by .pfn_mkwrite() for write faults

On 8/3/26 13:54, David Hildenbrand (Arm) wrote:
> On 7/31/26 18:43, Paolo Bonzini wrote:
>> Warning - DRM parts (i.e. most of the patches) untested; I have Cc'd
>> the reporter to help with testing these patches.
>>
>> Right now, users of .pfn_mkwrite() have no way to create a PTE
>> that has gone through maybe_mkwrite().  Because vma_set_page_prot()
>> will have cleared the writable PTE bit, users of fixup_user_fault()
>> will see a read-only PTE and have no clue that the page needs
>> a *second* fault to reach its final status.
>>
>> Handling this in fixup_user_fault() is problematic: the information
>> about the presence of *_mkwrite is only recorded in vma->vm_page_prot,
>> which is an opaque pgprot_t, therefore only follow_pfnmap_start()
>> knows how to retrieve it.
> 
> How is mprotect() supposed to work in that case?

Hi David,

not sure what you are worried about specifically, but fixup_user_fault() 
catches !VM_WRITE VMAs and returns early (see vma_permits_fault()).

Also, do_wp_page() has the comment:

         /*
          * Shared mapping: we are guaranteed to have VM_WRITE and
          * FAULT_FLAG_WRITE set at this point.
          */

before the call to wp_pfn_shared() which is where .pfn_mkwrite() is called.

Let me know if this was not what you were asking.

Thanks for the review of patch 1---I mentioned here in the cover letter 
that the name was temporary and I'll take your suggestion.  I can either 
use EXPORT_SYMBOL_GPL or switch to inlines, but not both because the 
existing functions like vmf_insert_pfn_prot() need to stay non-GPL-only.

Anyhow, now that the series has a Tested-by I'll clean up everything, 
and repost later this week.

Paolo