<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 6, 2017 at 11:15 PM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The vkCmd*() functions do not report errors, instead, any errors should be reported by the time we call vkEndCommandBuffer(). This means that we need to track things such as "out of host memory" and use that information to avoid executing code that could lead to crashes (due to the fact that the command buffer could not allocate the memory it needs) and report the error when the client calls vkEndCommandBuffer(). Notice this patch is not sufficient to track and report all possible cases of out of host memory situations that can be produced while recording command buffers. Later patches will fix some of the missing cases. Also, some allocations may occur deep into the driver code (for example during batch emissions) and are more difficult to track because at that point we don't even have access to the command buffer object. Dealing with these scenarios would require more changes but for now this is sufficient to make CTS happy. </blockquote><div> </div><div>Thinking about things a bit more, I don't think that is actually all that hard... </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Fixes: dEQP-VK.api.out_of_host_memory.cmd_begin_render_pass --- src/intel/vulkan/anv_cmd_buffer.c | 5 +++++ src/intel/vulkan/anv_private.h | 7 +++++++ src/intel/vulkan/genX_cmd_buffer.c | 22 +++++++++++++++++++--- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c index cab1dd7..01447b0 100644 --- a/src/intel/vulkan/anv_cmd_buffer.c +++ b/src/intel/vulkan/anv_cmd_buffer.c @@ -117,6 +117,8 @@ anv_cmd_state_reset(struct anv_cmd_buffer *cmd_buffer) { struct anv_cmd_state *state = &cmd_buffer->state; + cmd_buffer->error_status = VK_SUCCESS; + memset(&state->descriptors, 0, sizeof(state->descriptors)); memset(&state->push_constants, 0, sizeof(state->push_constants)); memset(state->binding_tables, 0, sizeof(state->binding_tables)); @@ -185,6 +187,8 @@ static VkResult anv_create_cmd_buffer( if (cmd_buffer == NULL) return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY); + cmd_buffer->error_status = VK_SUCCESS; + cmd_buffer->_loader_data.loaderMagic = ICD_LOADER_MAGIC; cmd_buffer->device = device; cmd_buffer->pool = pool; @@ -217,6 +221,7 @@ static VkResult anv_create_cmd_buffer( return VK_SUCCESS; fail: + cmd_buffer->error_status = result; vk_free(&cmd_buffer->pool->alloc, cmd_buffer); return result; diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index d3e32bc..46b51a9 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1377,6 +1377,13 @@ struct anv_cmd_buffer { VkCommandBufferLevel level; struct anv_cmd_state state; + + /** + * Current error status of the command buffer. Used to track inconsistent + * or incomplete command buffer states that are the consequence of run-time + * errors such as out of memory scenarios. + */ + VkResult error_status; </blockquote><div> </div><div>If we put this in anv_batch, we can easily set the status from "deep" parts of the driver. Also, I'm not sure the "error" part of the name is needed. "status" should be ok. </div><div>One other thing we may want to do is to add a anv_batch_set_error helper and only take first error. That way we can track OUT_OF_HOST_MEMORY vs. OUT_OF_DEVICE_MEMORY. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> }; VkResult anv_cmd_buffer_init_batch_bo_chain(struct anv_cmd_buffer *cmd_buffer); diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index b46a922..cbad144 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -401,8 +401,8 @@ genX(cmd_buffer_setup_attachments)(struct anv_cmd_buffer *cmd_buffer, sizeof(state->attachments[0]), 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT); if (state->attachments == NULL) { - /* FIXME: Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */ - return VK_ERROR_OUT_OF_HOST_MEMORY; + /* Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */ + return cmd_buffer->error_status = VK_ERROR_OUT_OF_HOST_MEMORY; } bool need_null_state = false; @@ -615,6 +615,9 @@ genX(EndCommandBuffer)( { ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer); + if (cmd_buffer->error_status != VK_SUCCESS) + return cmd_buffer->error_status; + /* We want every command buffer to start with the PMA fix in a known state, * so we disable it at the end of the command buffer. */ @@ -2470,7 +2473,14 @@ void genX(CmdBeginRenderPass)( cmd_buffer->state.framebuffer = framebuffer; cmd_buffer->state.pass = pass; cmd_buffer->state.render_area = pRenderPassBegin->renderArea; - genX(cmd_buffer_setup_attachments)(cmd_buffer, pass, pRenderPassBegin); + VkResult result = + genX(cmd_buffer_setup_attachments)(cmd_buffer, pass, pRenderPassBegin); + + /* If we failed to setup the attachments we should not try to go further */ + if (result != VK_SUCCESS) { + assert(cmd_buffer->error_status != VK_SUCCESS); + return; + } genX(flush_pipeline_select_3d)(cmd_buffer); @@ -2483,6 +2493,9 @@ void genX(CmdNextSubpass)( { ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer); + if (cmd_buffer->error_status != VK_SUCCESS) + return; + assert(cmd_buffer->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY); anv_cmd_buffer_resolve_subpass(cmd_buffer); @@ -2499,6 +2512,9 @@ void genX(CmdEndRenderPass)( { ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer); + if (cmd_buffer->error_status != VK_SUCCESS) + return; + anv_cmd_buffer_resolve_subpass(cmd_buffer); /* Perform transitions to the final layout after all writes have occurred. -- 2.7.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>