lkml.org 
[lkml]   [2019]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH 6/9] drm/vmwgfx: Implement an infrastructure for write-coherent resources
    Date
    Hi Thomas,

    With minor comments below

    Reviewed-by: Deepak Rawat <drawat@vmware.com>

    On Fri, 2019-04-12 at 09:04 -0700, Thomas Hellstrom wrote:
    > This infrastructure will, for coherent resources, make sure that
    > from the user-space point of view, data written by the CPU is
    > immediately
    > automatically available to the GPU at resource validation time.
    >
    > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
    > ---
    > drivers/gpu/drm/vmwgfx/Kconfig | 1 +
    > drivers/gpu/drm/vmwgfx/Makefile | 2 +-
    > drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 5 +-
    > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +
    > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 26 +-
    > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 1 -
    > drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 410
    > ++++++++++++++++++
    > drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 57 +++
    > drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h | 11 +
    > drivers/gpu/drm/vmwgfx/vmwgfx_validation.c | 74 ++++
    > drivers/gpu/drm/vmwgfx/vmwgfx_validation.h | 16 +-
    > 11 files changed, 588 insertions(+), 20 deletions(-)
    > create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
    >
    > diff --git a/drivers/gpu/drm/vmwgfx/Kconfig
    > b/drivers/gpu/drm/vmwgfx/Kconfig
    > index 6b28a326f8bb..d5fd81a521f6 100644
    > --- a/drivers/gpu/drm/vmwgfx/Kconfig
    > +++ b/drivers/gpu/drm/vmwgfx/Kconfig
    > @@ -8,6 +8,7 @@ config DRM_VMWGFX
    > select FB_CFB_IMAGEBLIT
    > select DRM_TTM
    > select FB
    > + select AS_DIRTY_HELPERS
    > # Only needed for the transitional use of drm_crtc_init - can
    > be removed
    > # again once vmwgfx sets up the primary plane itself.
    > select DRM_KMS_HELPER
    > diff --git a/drivers/gpu/drm/vmwgfx/Makefile
    > b/drivers/gpu/drm/vmwgfx/Makefile
    > index 8841bd30e1e5..c877a21a0739 100644
    > --- a/drivers/gpu/drm/vmwgfx/Makefile
    > +++ b/drivers/gpu/drm/vmwgfx/Makefile
    > @@ -8,7 +8,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o
    > vmwgfx_kms.o vmwgfx_drv.o \
    > vmwgfx_cmdbuf_res.o vmwgfx_cmdbuf.o vmwgfx_stdu.o \
    > vmwgfx_cotable.o vmwgfx_so.o vmwgfx_binding.o vmwgfx_msg.o
    > \
    > vmwgfx_simple_resource.o vmwgfx_va.o vmwgfx_blit.o \
    > - vmwgfx_validation.o \
    > + vmwgfx_validation.o vmwgfx_page_dirty.o \
    > ttm_object.o ttm_lock.o
    >
    > obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
    > index c0829d50eecc..90ca866640fe 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
    > @@ -463,6 +463,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
    > {
    > struct vmw_buffer_object *vmw_bo = vmw_buffer_object(bo);
    >
    > + WARN_ON(vmw_bo->dirty);
    > vmw_bo_unmap(vmw_bo);
    > kfree(vmw_bo);
    > }
    > @@ -476,8 +477,10 @@ void vmw_bo_bo_free(struct ttm_buffer_object
    > *bo)
    > static void vmw_user_bo_destroy(struct ttm_buffer_object *bo)
    > {
    > struct vmw_user_buffer_object *vmw_user_bo =
    > vmw_user_buffer_object(bo);
    > + struct vmw_buffer_object *vbo = &vmw_user_bo->vbo;
    >
    > - vmw_bo_unmap(&vmw_user_bo->vbo);
    > + WARN_ON(vbo->dirty);

    Is it possible for user-space to exploit this WARN? If yes then you
    might want to change the logic?

    > + vmw_bo_unmap(vbo);
    > ttm_prime_object_kfree(vmw_user_bo, prime);
    > }
    >
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
    > index 6165fe2c4504..74e94138877e 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
    > @@ -857,6 +857,11 @@ static int vmw_driver_load(struct drm_device
    > *dev, unsigned long chipset)
    > DRM_ERROR("Failed initializing TTM buffer object
    > driver.\n");
    > goto out_no_bdev;
    > }
    > + dev_priv->vm_ops = *dev_priv->bdev.vm_ops;
    > + dev_priv->vm_ops.fault = vmw_bo_vm_fault;
    > + dev_priv->vm_ops.pfn_mkwrite = vmw_bo_vm_mkwrite;
    > + dev_priv->vm_ops.page_mkwrite = vmw_bo_vm_mkwrite;
    > + dev_priv->bdev.vm_ops = &dev_priv->vm_ops;
    >
    > /*
    > * Enable VRAM, but initially don't use it until SVGA is
    > enabled and
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
    > index bd6919b90519..f05fce52fbb4 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
    > @@ -95,6 +95,7 @@ struct vmw_fpriv {
    > * @dx_query_ctx: DX context if this buffer object is used as a DX
    > query MOB
    > * @map: Kmap object for semi-persistent mappings
    > * @res_prios: Eviction priority counts for attached resources
    > + * @dirty: structure for user-space dirty-tracking
    > */
    > struct vmw_buffer_object {
    > struct ttm_buffer_object base;
    > @@ -105,6 +106,7 @@ struct vmw_buffer_object {
    > /* Protected by reservation */
    > struct ttm_bo_kmap_obj map;
    > u32 res_prios[TTM_MAX_BO_PRIORITY];
    > + struct vmw_bo_dirty *dirty;
    > };
    >
    > /**
    > @@ -135,7 +137,8 @@ struct vmw_res_func;
    > * @res_dirty: Resource contains data not yet in the backup buffer.
    > Protected
    > * by resource reserved.
    > * @backup_dirty: Backup buffer contains data not yet in the HW
    > resource.
    > - * Protecte by resource reserved.
    > + * Protected by resource reserved.
    > + * @coherent: Emulate coherency by tracking vm accesses.
    > * @backup: The backup buffer if any. Protected by resource
    > reserved.
    > * @backup_offset: Offset into the backup buffer if any. Protected
    > by resource
    > * reserved. Note that only a few resource types can have a
    > @backup_offset
    > @@ -152,14 +155,16 @@ struct vmw_res_func;
    > * @hw_destroy: Callback to destroy the resource on the device, as
    > part of
    > * resource destruction.
    > */
    > +struct vmw_resource_dirty;
    > struct vmw_resource {
    > struct kref kref;
    > struct vmw_private *dev_priv;
    > int id;
    > u32 used_prio;
    > unsigned long backup_size;
    > - bool res_dirty;
    > - bool backup_dirty;
    > + u32 res_dirty : 1;
    > + u32 backup_dirty : 1;

    Is there a reason you changed res_dirty and backup_dirty from bool to
    u32. They are still areused as bool, right?

    > + u32 coherent : 1;
    > struct vmw_buffer_object *backup;
    > unsigned long backup_offset;
    > unsigned long pin_count;
    > @@ -167,6 +172,7 @@ struct vmw_resource {
    > struct list_head lru_head;
    > struct list_head mob_head;
    > struct list_head binding_head;
    > + struct vmw_resource_dirty *dirty;
    > void (*res_free) (struct vmw_resource *res);
    > void (*hw_destroy) (struct vmw_resource *res);
    > };
    > @@ -607,6 +613,9 @@ struct vmw_private {
    >
    > /* Validation memory reservation */
    > struct vmw_validation_mem vvm;
    > +
    > + /* VM operations */
    > + struct vm_operations_struct vm_ops;
    > };
    >
    > static inline struct vmw_surface *vmw_res_to_srf(struct vmw_resource
    > *res)
    > @@ -723,6 +732,8 @@ extern void vmw_resource_evict_all(struct
    > vmw_private *dev_priv);
    > extern void vmw_resource_unbind_list(struct vmw_buffer_object *vbo);
    > void vmw_resource_mob_attach(struct vmw_resource *res);
    > void vmw_resource_mob_detach(struct vmw_resource *res);
    > +void vmw_resource_dirty_update(struct vmw_resource *res, pgoff_t
    > start,
    > + pgoff_t end);
    >
    > /**
    > * vmw_resource_mob_attached - Whether a resource currently has a
    > mob attached
    > @@ -1411,6 +1422,15 @@ int vmw_host_log(const char *log);
    > #define VMW_DEBUG_USER(fmt,
    > ...) \
    > DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
    >
    > +/* Resource dirtying - vmwgfx_page_dirty.c */
    > +void vmw_bo_dirty_scan(struct vmw_buffer_object *vbo);
    > +int vmw_bo_dirty_add(struct vmw_buffer_object *vbo);
    > +void vmw_bo_dirty_transfer_to_res(struct vmw_resource *res);
    > +void vmw_bo_dirty_clear_res(struct vmw_resource *res);
    > +void vmw_bo_dirty_release(struct vmw_buffer_object *vbo);
    > +vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf);
    > +vm_fault_t vmw_bo_vm_mkwrite(struct vm_fault *vmf);
    > +
    > /**
    > * Inline helper functions
    > */
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
    > index da3ac0bc2e14..7cb22119f516 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
    > @@ -2483,7 +2483,6 @@ static int vmw_cmd_dx_check_subresource(struct
    > vmw_private *dev_priv,
    > offsetof(typeof(*cmd), sid));
    >
    > cmd = container_of(header, typeof(*cmd), header);
    > -
    > return vmw_cmd_res_check(dev_priv, sw_context, vmw_res_surface,
    > VMW_RES_DIRTY_NONE,
    > user_surface_converter,
    > &cmd->sid, NULL);
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
    > new file mode 100644
    > index 000000000000..87e4a73b1175
    > --- /dev/null
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c
    > @@ -0,0 +1,410 @@
    > +// SPDX-License-Identifier: GPL-2.0 OR MIT
    > +/*******************************************************************
    > *******
    > + *
    > + * Copyright 2019 VMware, Inc., Palo Alto, CA., USA
    > + *
    > + * Permission is hereby granted, free of charge, to any person
    > obtaining a
    > + * copy of this software and associated documentation files (the
    > + * "Software"), to deal in the Software without restriction,
    > including
    > + * without limitation the rights to use, copy, modify, merge,
    > publish,
    > + * distribute, sub license, and/or sell copies of the Software, and
    > to
    > + * permit persons to whom the Software is furnished to do so,
    > subject to
    > + * the following conditions:
    > + *
    > + * The above copyright notice and this permission notice (including
    > the
    > + * next paragraph) shall be included in all copies or substantial
    > portions
    > + * of the Software.
    > + *
    > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
    > EXPRESS OR
    > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
    > MERCHANTABILITY,
    > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO
    > EVENT SHALL
    > + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR
    > ANY CLAIM,
    > + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
    > TORT OR
    > + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
    > SOFTWARE OR THE
    > + * USE OR OTHER DEALINGS IN THE SOFTWARE.
    > + *
    > +
    > *********************************************************************
    > *****/
    > +#include "vmwgfx_drv.h"
    > +
    > +/*
    > + * Different methods for tracking dirty:
    > + * VMW_BO_DIRTY_PAGETABLE - Scan the pagetable for hardware dirty
    > bits
    > + * VMW_BO_DIRTY_MKWRITE - Write-protect page table entries and
    > record write-
    > + * accesses in the VM mkwrite() callback
    > + */
    > +enum vmw_bo_dirty_method {
    > + VMW_BO_DIRTY_PAGETABLE,
    > + VMW_BO_DIRTY_MKWRITE,
    > +};
    > +
    > +/*
    > + * No dirtied pages at scan trigger a transition to the _MKWRITE
    > method,
    > + * similarly a certain percentage of dirty pages trigger a
    > transition to
    > + * the _PAGETABLE method. How many triggers should we wait for
    > before
    > + * changing method?
    > + */
    > +#define VMW_DIRTY_NUM_CHANGE_TRIGGERS 2
    > +
    > +/* Percentage to trigger a transition to the _PAGETABLE method */
    > +#define VMW_DIRTY_PERCENTAGE 10
    > +
    > +/**
    > + * struct vmw_bo_dirty - Dirty information for buffer objects
    > + * @start: First currently dirty bit
    > + * @end: Last currently dirty bit + 1
    > + * @method: The currently used dirty method
    > + * @change_count: Number of consecutive method change triggers
    > + * @ref_count: Reference count for this structure
    > + * @bitmap_size: The size of the bitmap in bits. Typically equal to
    > the
    > + * nuber of pages in the bo.
    > + * @size: The accounting size for this struct.
    > + * @bitmap: A bitmap where each bit represents a page. A set bit
    > means a
    > + * dirty page.
    > + */
    > +struct vmw_bo_dirty {
    > + unsigned long start;
    > + unsigned long end;
    > + enum vmw_bo_dirty_method method;
    > + unsigned int change_count;
    > + unsigned int ref_count;
    > + unsigned long bitmap_size;
    > + size_t size;
    > + unsigned long bitmap[0];
    > +};
    > +
    > +/**
    > + * vmw_bo_dirty_scan_pagetable - Perform a pagetable scan for dirty
    > bits
    > + * @vbo: The buffer object to scan
    > + *
    > + * Scans the pagetable for dirty bits. Clear those bits and modify
    > the
    > + * dirty structure with the results. This function may change the
    > + * dirty-tracking method.
    > + */
    > +static void vmw_bo_dirty_scan_pagetable(struct vmw_buffer_object
    > *vbo)
    > +{
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > + pgoff_t offset = drm_vma_node_start(&vbo->base.vma_node);
    > + struct address_space *mapping = vbo->base.bdev->dev_mapping;
    > + pgoff_t num_marked;
    > +
    > + num_marked = apply_as_clean(mapping,
    > + offset, dirty->bitmap_size,
    > + offset, &dirty->bitmap[0],
    > + &dirty->start, &dirty->end);
    > + if (num_marked == 0)
    > + dirty->change_count++;
    > + else
    > + dirty->change_count = 0;
    > +
    > + if (dirty->change_count > VMW_DIRTY_NUM_CHANGE_TRIGGERS) {
    > + dirty->change_count = 0;
    > + dirty->method = VMW_BO_DIRTY_MKWRITE;
    > + apply_as_wrprotect(mapping,
    > + offset, dirty->bitmap_size);
    > + apply_as_clean(mapping,
    > + offset, dirty->bitmap_size,
    > + offset, &dirty->bitmap[0],
    > + &dirty->start, &dirty->end);
    > + }
    > +}
    > +
    > +/**
    > + * vmw_bo_dirty_scan_mkwrite - Reset the mkwrite dirty-tracking
    > method
    > + * @vbo: The buffer object to scan
    > + *
    > + * Write-protect pages written to so that consecutive write accesses
    > will
    > + * trigger a call to mkwrite.
    > + *
    > + * This function may change the dirty-tracking method.
    > + */
    > +static void vmw_bo_dirty_scan_mkwrite(struct vmw_buffer_object *vbo)
    > +{
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > + unsigned long offset = drm_vma_node_start(&vbo->base.vma_node);
    > + struct address_space *mapping = vbo->base.bdev->dev_mapping;
    > + pgoff_t num_marked;
    > +
    > + if (dirty->end <= dirty->start)
    > + return;
    > +
    > + num_marked = apply_as_wrprotect(vbo->base.bdev->dev_mapping,
    > + dirty->start + offset,
    > + dirty->end - dirty->start);
    > +
    > + if (100UL * num_marked / dirty->bitmap_size >
    > + VMW_DIRTY_PERCENTAGE) {
    > + dirty->change_count++;
    > + } else {
    > + dirty->change_count = 0;
    > + }
    > +
    > + if (dirty->change_count > VMW_DIRTY_NUM_CHANGE_TRIGGERS) {
    > + pgoff_t start = 0;
    > + pgoff_t end = dirty->bitmap_size;
    > +
    > + dirty->method = VMW_BO_DIRTY_PAGETABLE;
    > + apply_as_clean(mapping, offset, end, offset, &dirty-
    > >bitmap[0],
    > + &start, &end);
    > + bitmap_clear(&dirty->bitmap[0], 0, dirty->bitmap_size);
    > + if (dirty->start < dirty->end)
    > + bitmap_set(&dirty->bitmap[0], dirty->start,
    > + dirty->end - dirty->start);
    > + dirty->change_count = 0;
    > + }
    > +}
    > +
    > +
    > +/**
    > + * vmw_bo_dirty_scan - Scan for dirty pages and add them to the
    > dirty
    > + * tracking structure
    > + * @vbo: The buffer object to scan
    > + *
    > + * This function may change the dirty tracking method.
    > + */
    > +void vmw_bo_dirty_scan(struct vmw_buffer_object *vbo)
    > +{
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > +
    > + if (dirty->method == VMW_BO_DIRTY_PAGETABLE)
    > + vmw_bo_dirty_scan_pagetable(vbo);
    > + else
    > + vmw_bo_dirty_scan_mkwrite(vbo);
    > +}
    > +
    > +/**
    > + * vmw_bo_dirty_add - Add a dirty-tracking user to a buffer object
    > + * @vbo: The buffer object
    > + *
    > + * This function registers a dirty-tracking user to a buffer object.
    > + * A user can be for example a resource or a vma in a special user-
    > space
    > + * mapping.
    > + *
    > + * Return: Zero on success, -ENOMEM on memory allocation failure.
    > + */
    > +int vmw_bo_dirty_add(struct vmw_buffer_object *vbo)
    > +{
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > + pgoff_t num_pages = vbo->base.num_pages;
    > + size_t size, acc_size;
    > + int ret;
    > + static struct ttm_operation_ctx ctx = {
    > + .interruptible = false,
    > + .no_wait_gpu = false
    > + };
    > +
    > + if (dirty) {
    > + dirty->ref_count++;
    > + return 0;
    > + }
    > +
    > + size = sizeof(*dirty) + BITS_TO_LONGS(num_pages) *
    > sizeof(long);
    > + acc_size = ttm_round_pot(size);
    > + ret = ttm_mem_global_alloc(&ttm_mem_glob, acc_size, &ctx);
    > + if (ret) {
    > + VMW_DEBUG_USER("Out of graphics memory for buffer
    > object "
    > + "dirty tracker.\n");
    > + return ret;
    > + }
    > + dirty = kvzalloc(size, GFP_KERNEL);
    > + if (!dirty) {
    > + ret = -ENOMEM;
    > + goto out_no_dirty;
    > + }
    > +
    > + dirty->size = acc_size;
    > + dirty->bitmap_size = num_pages;
    > + dirty->start = dirty->bitmap_size;
    > + dirty->end = 0;
    > + dirty->ref_count = 1;
    > + if (num_pages < PAGE_SIZE / sizeof(pte_t)) {
    > + dirty->method = VMW_BO_DIRTY_PAGETABLE;
    > + } else {
    > + struct address_space *mapping = vbo->base.bdev-
    > >dev_mapping;
    > + pgoff_t offset = drm_vma_node_start(&vbo-
    > >base.vma_node);
    > +
    > + dirty->method = VMW_BO_DIRTY_MKWRITE;
    > +
    > + /* Write-protect and then pick up already dirty bits */
    > + apply_as_wrprotect(mapping, offset, num_pages);
    > + apply_as_clean(mapping, offset, num_pages, offset,
    > + &dirty->bitmap[0], &dirty->start,
    > &dirty->end);
    > + }
    > +
    > + vbo->dirty = dirty;
    > +
    > + return 0;
    > +
    > +out_no_dirty:
    > + ttm_mem_global_free(&ttm_mem_glob, acc_size);
    > + return ret;
    > +}
    > +
    > +/**
    > + * vmw_bo_dirty_release - Release a dirty-tracking user from a
    > buffer object
    > + * @vbo: The buffer object
    > + *
    > + * This function releases a dirty-tracking user from a buffer
    > object.
    > + * If the reference count reaches zero, then the dirty-tracking
    > object is
    > + * freed and the pointer to it cleared.
    > + *
    > + * Return: Zero on success, -ENOMEM on memory allocation failure.
    > + */
    > +void vmw_bo_dirty_release(struct vmw_buffer_object *vbo)
    > +{
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > +
    > + if (dirty && --dirty->ref_count == 0) {
    > + size_t acc_size = dirty->size;
    > +
    > + kvfree(dirty);
    > + ttm_mem_global_free(&ttm_mem_glob, acc_size);
    > + vbo->dirty = NULL;
    > + }
    > +}
    > +
    > +/**
    > + * vmw_bo_dirty_transfer_to_res - Pick up a resource's dirty region
    > from
    > + * its backing mob.
    > + * @res: The resource
    > + *
    > + * This function will pick up all dirty ranges affecting the
    > resource from
    > + * it's backup mob, and call vmw_resource_dirty_update() once for
    > each
    > + * range. The transferred ranges will be cleared from the backing
    > mob's
    > + * dirty tracking.
    > + */
    > +void vmw_bo_dirty_transfer_to_res(struct vmw_resource *res)
    > +{
    > + struct vmw_buffer_object *vbo = res->backup;
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > + pgoff_t start, cur, end;
    > + unsigned long res_start = res->backup_offset;
    > + unsigned long res_end = res->backup_offset + res->backup_size;
    > +
    > + WARN_ON_ONCE(res_start & ~PAGE_MASK);
    > + res_start >>= PAGE_SHIFT;
    > + res_end = DIV_ROUND_UP(res_end, PAGE_SIZE);
    > +
    > + if (res_start >= dirty->end || res_end <= dirty->start)
    > + return;
    > +
    > + cur = max(res_start, dirty->start);
    > + res_end = max(res_end, dirty->end);
    > + while (cur < res_end) {
    > + unsigned long num;
    > +
    > + start = find_next_bit(&dirty->bitmap[0], res_end, cur);
    > + if (start >= res_end)
    > + break;
    > +
    > + end = find_next_zero_bit(&dirty->bitmap[0], res_end,
    > start + 1);
    > + cur = end + 1;
    > + num = end - start;
    > + bitmap_clear(&dirty->bitmap[0], start, num);
    > + vmw_resource_dirty_update(res, start, end);
    > + }
    > +
    > + if (res_start <= dirty->start && res_end > dirty->start)
    > + dirty->start = res_end;
    > + if (res_start < dirty->end && res_end >= dirty->end)
    > + dirty->end = res_start;
    > +}
    > +
    > +/**
    > + * vmw_bo_dirty_clear_res - Clear a resource's dirty region from
    > + * its backing mob.
    > + * @res: The resource
    > + *
    > + * This function will clar all dirty ranges affecting the resource
    s/clar/clear

    > from
    > + * it's backup mob's dirty tracking.
    > + */
    > +void vmw_bo_dirty_clear_res(struct vmw_resource *res)
    > +{
    > + unsigned long res_start = res->backup_offset;
    > + unsigned long res_end = res->backup_offset + res->backup_size;
    > + struct vmw_buffer_object *vbo = res->backup;
    > + struct vmw_bo_dirty *dirty = vbo->dirty;
    > +
    > + res_start >>= PAGE_SHIFT;
    > + res_end = DIV_ROUND_UP(res_end, PAGE_SIZE);
    > +
    > + if (res_start >= dirty->end || res_end <= dirty->start)
    > + return;
    > +
    > + res_start = max(res_start, dirty->start);
    > + res_end = min(res_end, dirty->end);
    > + bitmap_clear(&dirty->bitmap[0], res_start, res_end -
    > res_start);
    > +
    > + if (res_start <= dirty->start && res_end > dirty->start)
    > + dirty->start = res_end;
    > + if (res_start < dirty->end && res_end >= dirty->end)
    > + dirty->end = res_start;
    > +}
    > +
    > +/* vmw_bo_vm_mkwrite - The vmwgfx page_mkwrite() or pfn_mkwrite()
    > callback */

    Function documentation format, IMO since this is callback you can get
    rid of the above line, same for vmw_bo_vm_fault.

    > +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;
    > + struct vmw_buffer_object *vbo =
    > + container_of(bo, typeof(*vbo), base);
    > +
    > + ret = ttm_bo_vm_reserve(bo, vmf);
    > + if (ret)
    > + return ret;
    > +
    > + page_offset = vmf->pgoff - drm_vma_node_start(&bo->vma_node);
    > + if (unlikely(page_offset >= bo->num_pages)) {
    > + 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:
    > + reservation_object_unlock(bo->resv);
    > + return ret;
    > +}
    > +
    > +
    > +/* vmw_bo_vm_fault - The vmwgfx fault() callback */
    > +vm_fault_t vmw_bo_vm_fault(struct vm_fault *vmf)
    > +{
    > + struct vm_area_struct *vma = vmf->vma;
    > + struct vm_area_struct cvma = *vma;
    > + struct ttm_buffer_object *bo = (struct ttm_buffer_object *)
    > + vma->vm_private_data;
    > + struct vmw_buffer_object *vbo =
    > + container_of(bo, struct vmw_buffer_object, base);
    > + pgoff_t num_prefault;
    > + vm_fault_t ret;
    > +
    > + ret = ttm_bo_vm_reserve(bo, vmf);
    > + if (ret)
    > + return ret;
    > +
    > + /*
    > + * This will cause mkwrite() to be called for each pte on
    > + * write-enable vmas.
    > + */
    > + if (vbo->dirty && vbo->dirty->method == VMW_BO_DIRTY_MKWRITE)
    > + cvma.vm_flags &= ~VM_WRITE;
    > +
    > + num_prefault = (vma->vm_flags & VM_RAND_READ) ? 0 :
    > + TTM_BO_VM_NUM_PREFAULT;
    > + ret = ttm_bo_vm_fault_reserved(vmf, &cvma, num_prefault);
    > + if (ret == VM_FAULT_RETRY && !(vmf->flags &
    > FAULT_FLAG_RETRY_NOWAIT))
    > + return ret;
    > +
    > + reservation_object_unlock(bo->resv);
    > + return ret;
    > +}
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
    > index 88981c4dbae3..d35f4bd32cd9 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
    > @@ -121,6 +121,10 @@ static void vmw_resource_release(struct kref
    > *kref)
    > }
    > res->backup_dirty = false;
    > vmw_resource_mob_detach(res);
    > + if (res->dirty)
    > + res->func->dirty_free(res);
    > + if (res->coherent)
    > + vmw_bo_dirty_release(res->backup);
    > ttm_bo_unreserve(bo);
    > vmw_bo_unreference(&res->backup);
    > }
    > @@ -210,7 +214,9 @@ int vmw_resource_init(struct vmw_private
    > *dev_priv, struct vmw_resource *res,
    > res->backup_offset = 0;
    > res->backup_dirty = false;
    > res->res_dirty = false;
    > + res->coherent = false;
    > res->used_prio = 3;
    > + res->dirty = NULL;
    > if (delay_id)
    > return 0;
    > else
    > @@ -397,6 +403,30 @@ static int vmw_resource_do_validate(struct
    > vmw_resource *res,
    > vmw_resource_mob_attach(res);
    > }
    >
    > + /*
    > + * Handle the case where the backup mob is marked coherent but
    > + * the resource isn't.
    > + */
    > + if (func->dirty_alloc && vmw_resource_mob_attached(res) &&
    > + !res->coherent) {
    > + if (res->backup->dirty && !res->dirty) {
    > + ret = func->dirty_alloc(res);
    > + if (ret)
    > + return ret;
    > + } else if (!res->backup->dirty && res->dirty) {
    > + func->dirty_free(res);
    > + }
    > + }
    > +
    > + /*
    > + * Transfer the dirty regions to the resource and update
    > + * the resource.
    > + */
    > + if (res->dirty) {
    > + vmw_bo_dirty_transfer_to_res(res);
    > + return func->dirty_sync(res);
    > + }
    > +
    > return 0;
    >
    > out_bind_failed:
    > @@ -435,16 +465,28 @@ void vmw_resource_unreserve(struct vmw_resource
    > *res,
    > if (switch_backup && new_backup != res->backup) {
    > if (res->backup) {
    > vmw_resource_mob_detach(res);
    > + if (res->coherent)
    > + vmw_bo_dirty_release(res->backup);
    > vmw_bo_unreference(&res->backup);
    > }
    >
    > if (new_backup) {
    > res->backup = vmw_bo_reference(new_backup);
    > +
    > + /*
    > + * The validation code should already have
    > added a
    > + * dirty tracker here.
    > + */
    > + WARN_ON(res->coherent && !new_backup->dirty);
    > +
    > vmw_resource_mob_attach(res);
    > } else {
    > res->backup = NULL;
    > }
    > + } else if (switch_backup && res->coherent) {
    > + vmw_bo_dirty_release(res->backup);
    > }
    > +
    > if (switch_backup)
    > res->backup_offset = new_backup_offset;
    >
    > @@ -1009,3 +1051,18 @@ enum vmw_res_type vmw_res_type(const struct
    > vmw_resource *res)
    > {
    > return res->func->res_type;
    > }
    > +
    > +/**
    > + * vmw_resource_update_dirty - Update a resource's dirty tracker
    > with a
    > + * sequential range of touched backing store memory.
    > + * @res: The resource.
    > + * @start: The first page touched.
    > + * @end: The last page touched + 1.
    > + */
    > +void vmw_resource_dirty_update(struct vmw_resource *res, pgoff_t
    > start,
    > + pgoff_t end)
    > +{
    > + if (res->dirty)
    > + res->func->dirty_range_add(res, start << PAGE_SHIFT,
    > + end << PAGE_SHIFT);
    > +}
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h
    > index 984e588c62ca..c85144286cfe 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource_priv.h
    > @@ -71,6 +71,12 @@ struct vmw_user_resource_conv {
    > * @commit_notify: If the resource is a command buffer managed
    > resource,
    > * callback to notify that a define or remove
    > command
    > * has been committed to the device.
    > + * @dirty_alloc: Allocate a dirty tracker. NULL if dirty-
    > tracking is not
    > + * supported.
    > + * @dirty_free: Free the dirty tracker.
    > + * @dirty_sync: Upload the dirty mob contents to the
    > resource.
    > + * @dirty_add_range: Add a sequential dirty range to the resource
    > + * dirty tracker.
    > */
    > struct vmw_res_func {
    > enum vmw_res_type res_type;
    > @@ -90,6 +96,11 @@ struct vmw_res_func {
    > struct ttm_validate_buffer *val_buf);
    > void (*commit_notify)(struct vmw_resource *res,
    > enum vmw_cmdbuf_res_state state);
    > + int (*dirty_alloc)(struct vmw_resource *res);
    > + void (*dirty_free)(struct vmw_resource *res);
    > + int (*dirty_sync)(struct vmw_resource *res);
    > + void (*dirty_range_add)(struct vmw_resource *res, size_t start,
    > + size_t end);
    > };
    >
    > /**
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
    > index f611b2290a1b..5b0c928bb5ba 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.c
    > @@ -33,8 +33,13 @@
    > * struct vmw_validation_bo_node - Buffer object validation
    > metadata.
    > * @base: Metadata used for TTM reservation- and validation.
    > * @hash: A hash entry used for the duplicate detection hash table.
    > + * @coherent_count: If switching backup buffers, number of new
    > coherent
    > + * resources that will have this buffer as a backup buffer.
    > * @as_mob: Validate as mob.
    > * @cpu_blit: Validate for cpu blit access.
    > + * @coherent_alloced: In switching backup buffers for coherent
    > resources:

    I don't see the field below, remove this.

    > + * The bo dirty tracker has been allocated and needs to be freed if
    > + * reverting.
    > *
    > * Bit fields are used since these structures are allocated and
    > freed in
    > * large numbers and space conservation is desired.
    > @@ -42,6 +47,7 @@
    > struct vmw_validation_bo_node {
    > struct ttm_validate_buffer base;
    > struct drm_hash_item hash;
    > + unsigned int coherent_count;
    > u32 as_mob : 1;
    > u32 cpu_blit : 1;
    > };
    > @@ -459,6 +465,19 @@ int vmw_validation_res_reserve(struct
    > vmw_validation_context *ctx,
    > if (ret)
    > goto out_unreserve;
    > }
    > +
    > + if (val->switching_backup && val->new_backup &&
    > + res->coherent) {
    > + struct vmw_validation_bo_node *bo_node =
    > + vmw_validation_find_bo_dup(ctx,
    > + val-
    > >new_backup);
    > +
    > + if (WARN_ON(!bo_node)) {
    > + ret = -EINVAL;
    > + goto out_unreserve;
    > + }
    > + bo_node->coherent_count++;
    > + }
    > }
    >
    > return 0;
    > @@ -562,6 +581,9 @@ int vmw_validation_bo_validate(struct
    > vmw_validation_context *ctx, bool intr)
    > int ret;
    >
    > list_for_each_entry(entry, &ctx->bo_list, base.head) {
    > + struct vmw_buffer_object *vbo =
    > + container_of(entry->base.bo, typeof(*vbo),
    > base);
    > +
    > if (entry->cpu_blit) {
    > struct ttm_operation_ctx ctx = {
    > .interruptible = intr,
    > @@ -576,6 +598,27 @@ int vmw_validation_bo_validate(struct
    > vmw_validation_context *ctx, bool intr)
    > }
    > if (ret)
    > return ret;
    > +
    > + /*
    > + * Rather than having the resource code allocating the
    > bo
    > + * dirty tracker in resource_unreserve() where we can't
    > fail,
    > + * Do it here when validating the buffer object.
    > + */
    > + if (entry->coherent_count) {
    > + unsigned int coherent_count = entry-
    > >coherent_count;
    > +
    > + while (coherent_count) {
    > + ret = vmw_bo_dirty_add(vbo);
    > + if (ret)
    > + return ret;
    > +
    > + coherent_count--;
    > + }
    > + entry->coherent_count -= coherent_count;
    > + }
    > +
    > + if (vbo->dirty)
    > + vmw_bo_dirty_scan(vbo);
    > }
    > return 0;
    > }
    > @@ -828,3 +871,34 @@ int vmw_validation_preload_res(struct
    > vmw_validation_context *ctx,
    > ctx->mem_size_left += size;
    > return 0;
    > }
    > +
    > +/**
    > + * vmw_validation_bo_backoff - Unreserve buffer objects registered
    > with a
    > + * validation context
    > + * @ctx: The validation context
    > + *
    > + * This function unreserves the buffer objects previously reserved
    > using
    > + * vmw_validation_bo_reserve. It's typically used as part of an
    > error path
    > + */
    > +void vmw_validation_bo_backoff(struct vmw_validation_context *ctx)
    > +{
    > + struct vmw_validation_bo_node *entry;
    > +
    > + /*
    > + * Switching coherent resource backup buffers failed.
    > + * Release corresponding buffer object dirty trackers.
    > + */
    > + list_for_each_entry(entry, &ctx->bo_list, base.head) {
    > + if (entry->coherent_count) {
    > + unsigned int coherent_count = entry-
    > >coherent_count;
    > + struct vmw_buffer_object *vbo =
    > + container_of(entry->base.bo,
    > typeof(*vbo),
    > + base);
    > +
    > + while (coherent_count--)
    > + vmw_bo_dirty_release(vbo);
    > + }
    > + }
    > +
    > + ttm_eu_backoff_reservation(&ctx->ticket, &ctx->bo_list);
    > +}
    > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
    > b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
    > index 523f6ac5c335..058c7f2fbf83 100644
    > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
    > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
    > @@ -172,20 +172,6 @@ vmw_validation_bo_reserve(struct
    > vmw_validation_context *ctx,
    > NULL);
    > }
    >
    > -/**
    > - * vmw_validation_bo_backoff - Unreserve buffer objects registered
    > with a
    > - * validation context
    > - * @ctx: The validation context
    > - *
    > - * This function unreserves the buffer objects previously reserved
    > using
    > - * vmw_validation_bo_reserve. It's typically used as part of an
    > error path
    > - */
    > -static inline void
    > -vmw_validation_bo_backoff(struct vmw_validation_context *ctx)
    > -{
    > - ttm_eu_backoff_reservation(&ctx->ticket, &ctx->bo_list);
    > -}
    > -
    > /**
    > * vmw_validation_bo_fence - Unreserve and fence buffer objects
    > registered
    > * with a validation context
    > @@ -268,4 +254,6 @@ int vmw_validation_preload_res(struct
    > vmw_validation_context *ctx,
    > unsigned int size);
    > void vmw_validation_res_set_dirty(struct vmw_validation_context
    > *ctx,
    > void *val_private, u32 dirty);
    > +void vmw_validation_bo_backoff(struct vmw_validation_context *ctx);
    > +
    > #endif
    > --
    > 2.20.1
    >

    \
     
     \ /
      Last update: 2019-04-22 20:56    [W:4.791 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site