Re: [PATCH RFT 3/3] drm/ttm, drm/vmwgfx: directly create writable PTEs when mkwrite is in use

[Christian König]

On 7/31/26 18:43, Paolo Bonzini wrote:
> This ensures that fixup_user_fault() users see a writable PTE when
> they request one.  The flip side is that vmw_bo_vm_fault() now has
> to record by hand the write fault, because .pfn_mkwrite() is
> not invoked.
> 
> Prefaulting works as before because only the first entry comes
> out writable, while the following ones still end up executing
> the .pfn_mkwrite() callback.

Please split that patch for TTM/VMWGFX. The TTM part looks reasonable, but VMGFX is a completely different beast.

Regards,
Christian.

> 
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/ttm/ttm_bo_vm.c            |  3 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 42 ++++++++++++----------
>  2 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index a80510489c45..ef27a2d7afc0 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -263,7 +263,8 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
>  		 * at arbitrary times while the data is mmap'ed.
>  		 * See vmf_insert_pfn_prot() for a discussion.
>  		 */
> -		ret = vmf_insert_pfn_prot(vma, address, pfn, prot);
> +		ret = __vmf_insert_pfn_prot(vma, address, pfn, prot,
> +					    i == 0 && !!(vmf->flags & FAULT_FLAG_WRITE));
>  
>  		/* Never error on prefaulted PTEs */
>  		if (unlikely((ret & VM_FAULT_ERROR))) {
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> index 45561bc1c9ef..2cc490e7d758 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
> @@ -398,15 +398,33 @@ void vmw_bo_dirty_clear_res(struct vmw_resource *res)
>  		dirty->end = res_start;
>  }
>  
> +static vm_fault_t vmw_bo_dirty_mkwrite(struct vm_fault *vmf, struct ttm_buffer_object *bo)
> +{
> +	unsigned long page_offset;
> +	struct vmw_bo *vbo = to_vmw_bo(&bo->base);
> +
> +	page_offset = vmf->pgoff - drm_vma_node_start(&bo->base.vma_node);
> +	if (unlikely(page_offset >= PFN_UP(bo->resource->size)))
> +		return VM_FAULT_SIGBUS;
> +
> +	if (vbo->dirty && vbo->dirty->method == VMW_BO_DIRTY_MKWRITE &&
> +	    !test_bit(page_offset, &vbo->dirty->bitmap[0])) {
> +		struct vmw_bo_dirty *dirty = vbo->dirty;
> +
> +		__set_bit(page_offset, &dirty->bitmap[0]);
> +		dirty->start = min(dirty->start, page_offset);
> +		dirty->end = max(dirty->end, page_offset + 1);
> +	}
> +	return 0;
> +}
> +
>  vm_fault_t vmw_bo_vm_mkwrite(struct vm_fault *vmf)
>  {
>  	struct vm_area_struct *vma = vmf->vma;
>  	struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
>  	    vma->vm_private_data;
>  	vm_fault_t ret;
> -	unsigned long page_offset;
>  	unsigned int save_flags;
> -	struct vmw_bo *vbo = to_vmw_bo(&bo->base);
>  
>  	/*
>  	 * mkwrite() doesn't handle the VM_FAULT_RETRY return value correctly.
> @@ -419,22 +437,7 @@ vm_fault_t vmw_bo_vm_mkwrite(struct vm_fault *vmf)
>  	if (ret)
>  		return ret;
>  
> -	page_offset = vmf->pgoff - drm_vma_node_start(&bo->base.vma_node);
> -	if (unlikely(page_offset >= PFN_UP(bo->resource->size))) {
> -		ret = VM_FAULT_SIGBUS;
> -		goto out_unlock;
> -	}
> -
> -	if (vbo->dirty && vbo->dirty->method == VMW_BO_DIRTY_MKWRITE &&
> -	    !test_bit(page_offset, &vbo->dirty->bitmap[0])) {
> -		struct vmw_bo_dirty *dirty = vbo->dirty;
> -
> -		__set_bit(page_offset, &dirty->bitmap[0]);
> -		dirty->start = min(dirty->start, page_offset);
> -		dirty->end = max(dirty->end, page_offset + 1);
> -	}
> -
> -out_unlock:
> +	ret = vmw_bo_dirty_mkwrite(vmf, bo);
>  	dma_resv_unlock(bo->base.resv);
>  	return ret;
>  }
> @@ -484,6 +487,9 @@ vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)
>  		prot = vm_get_page_prot(vma->vm_flags);
>  
>  	ret = ttm_bo_vm_fault_reserved(vmf, prot, num_prefault);
> +	if (ret == VM_FAULT_NOPAGE && (vmf->flags & FAULT_FLAG_WRITE))
> +		WARN_ON_ONCE(vmw_bo_dirty_mkwrite(vmf, bo));
> +
>  	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
>  		return ret;
>