4.8-stable review patch. If anyone has any objections, please let me know. ------------------ From: Thomas Hellstrom <thellstrom@xxxxxxxxxx> commit a19440304db2d97aed5cee9bfa5017c98d2348bf upstream. When a view destruction command was present in the command stream, the view was validated to avoid a device error. That caused excessive and unnecessary validations of views, surfaces and mobs on view destruction. Replace this with a new relocation type that patches the view destruction command to a NOP if the view is not present in the device after the execbuf validation sequence. Also add checks for the member size of the vmw_res_relocation struct. Fixes sporadic command submission errors on google-earth exit. Reported-by: Brian Paul <brianp@xxxxxxxxxx> Signed-off-by: Thomas Hellstrom <thellstrom@xxxxxxxxxx> Reviewed-by: Brian Paul <brianp@xxxxxxxxxx> Reviewed-by: Sinclair Yeh <syeh@xxxxxxxxxx> Signed-off-by: Sinclair Yeh <syeh@xxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 70 ++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 13 deletions(-) --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c @@ -34,6 +34,24 @@ #define VMW_RES_HT_ORDER 12 + /** + * enum vmw_resource_relocation_type - Relocation type for resources + * + * @vmw_res_rel_normal: Traditional relocation. The resource id in the + * command stream is replaced with the actual id after validation. + * @vmw_res_rel_nop: NOP relocation. The command is unconditionally replaced + * with a NOP. + * @vmw_res_rel_cond_nop: Conditional NOP relocation. If the resource id + * after validation is -1, the command is replaced with a NOP. Otherwise no + * action. + */ +enum vmw_resource_relocation_type { + vmw_res_rel_normal, + vmw_res_rel_nop, + vmw_res_rel_cond_nop, + vmw_res_rel_max +}; + /** * struct vmw_resource_relocation - Relocation info for resources * @@ -41,11 +59,13 @@ * @res: Non-ref-counted pointer to the resource. * @offset: Offset of 4 byte entries into the command buffer where the * id that needs fixup is located. + * @rel_type: Type of relocation. */ struct vmw_resource_relocation { struct list_head head; const struct vmw_resource *res; - unsigned long offset; + u32 offset:29; + enum vmw_resource_relocation_type rel_type:3; }; /** @@ -410,10 +430,13 @@ static int vmw_resource_context_res_add( * @res: The resource. * @offset: Offset into the command buffer currently being parsed where the * id that needs fixup is located. Granularity is 4 bytes. + * @rel_type: Relocation type. */ static int vmw_resource_relocation_add(struct list_head *list, const struct vmw_resource *res, - unsigned long offset) + unsigned long offset, + enum vmw_resource_relocation_type + rel_type) { struct vmw_resource_relocation *rel; @@ -425,6 +448,7 @@ static int vmw_resource_relocation_add(s rel->res = res; rel->offset = offset; + rel->rel_type = rel_type; list_add_tail(&rel->head, list); return 0; @@ -459,11 +483,23 @@ static void vmw_resource_relocations_app { struct vmw_resource_relocation *rel; + /* Validate the struct vmw_resource_relocation member size */ + BUILD_BUG_ON(SVGA_CB_MAX_SIZE >= (1 << 29)); + BUILD_BUG_ON(vmw_res_rel_max >= (1 << 3)); + list_for_each_entry(rel, list, head) { - if (likely(rel->res != NULL)) + switch (rel->rel_type) { + case vmw_res_rel_normal: cb[rel->offset] = rel->res->id; - else + break; + case vmw_res_rel_nop: cb[rel->offset] = SVGA_3D_CMD_NOP; + break; + default: + if (rel->res->id == -1) + cb[rel->offset] = SVGA_3D_CMD_NOP; + break; + } } } @@ -655,7 +691,8 @@ static int vmw_cmd_res_reloc_add(struct *p_val = NULL; ret = vmw_resource_relocation_add(&sw_context->res_relocations, res, - id_loc - sw_context->buf_start); + id_loc - sw_context->buf_start, + vmw_res_rel_normal); if (unlikely(ret != 0)) return ret; @@ -721,7 +758,8 @@ vmw_cmd_res_check(struct vmw_private *de return vmw_resource_relocation_add (&sw_context->res_relocations, res, - id_loc - sw_context->buf_start); + id_loc - sw_context->buf_start, + vmw_res_rel_normal); } ret = vmw_user_resource_lookup_handle(dev_priv, @@ -2144,7 +2182,8 @@ static int vmw_cmd_shader_define(struct return vmw_resource_relocation_add(&sw_context->res_relocations, NULL, &cmd->header.id - - sw_context->buf_start); + sw_context->buf_start, + vmw_res_rel_nop); return 0; } @@ -2189,7 +2228,8 @@ static int vmw_cmd_shader_destroy(struct return vmw_resource_relocation_add(&sw_context->res_relocations, NULL, &cmd->header.id - - sw_context->buf_start); + sw_context->buf_start, + vmw_res_rel_nop); return 0; } @@ -2848,8 +2888,7 @@ static int vmw_cmd_dx_cid_check(struct v * @header: Pointer to the command header in the command stream. * * Check that the view exists, and if it was not created using this - * command batch, make sure it's validated (present in the device) so that - * the remove command will not confuse the device. + * command batch, conditionally make this command a NOP. */ static int vmw_cmd_dx_view_remove(struct vmw_private *dev_priv, struct vmw_sw_context *sw_context, @@ -2877,10 +2916,15 @@ static int vmw_cmd_dx_view_remove(struct return ret; /* - * Add view to the validate list iff it was not created using this - * command batch. + * If the view wasn't created during this command batch, it might + * have been removed due to a context swapout, so add a + * relocation to conditionally make this command a NOP to avoid + * device errors. */ - return vmw_view_res_val_add(sw_context, view); + return vmw_resource_relocation_add(&sw_context->res_relocations, + view, + &cmd->header.id - sw_context->buf_start, + vmw_res_rel_cond_nop); } /** -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html