From b1b8eb9301d6a9bbdec9bcd1a357e157ebeabb65 Mon Sep 17 00:00:00 2001 From: Yahan Zhou Date: Fri, 8 Mar 2024 15:39:38 -0800 Subject: [PATCH] Make it not crash during vk snapshot load We are trying to make vk snapshot load the google Chrome home page without crashing. With this CL it does not crash during load but would crash with a device lost after submitting a queue (which is unsurprising because everything is missing to snapshot vk queue). Content of this commit includes: - Add dependencies for VkImageView, VkGraphicsPipelines, VkFramebuffer. They are necessary to tell the snapshot module the order of vk object creation. - Add vkBindImageMemory into the loading sequencey of VkImage, so that it would be executed before vkCreateImageView. - Delay the destruction of VkShaderModule. This is because other objects can still refer to it after it is destroyed. - Initialize VK backend for color buffer properly on snapshot load. - Save and load vk images in the same order by sorting them according to their boxed handle. - Record all the placeholder handles for vkCreateDescriptorPool. For performance purpose this function creates a lot of extra handles without real contents. We need to snapshot those handles as well. Reviewed-by: Aaron Ruby Acked-by: Yonggang Luo Acked-by: Adam Jackson Part-of: --- .../codegen/scripts/cereal/decoder.py | 11 ++- .../codegen/scripts/cereal/decodersnapshot.py | 74 ++++++++++++++++--- 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/gfxstream/codegen/scripts/cereal/decoder.py b/src/gfxstream/codegen/scripts/cereal/decoder.py index 1b04bdfcd5b..4f10ef27918 100644 --- a/src/gfxstream/codegen/scripts/cereal/decoder.py +++ b/src/gfxstream/codegen/scripts/cereal/decoder.py @@ -22,6 +22,10 @@ DELAYED_DECODER_DELETES = [ "vkDestroyPipelineLayout", ] +DELAYED_DECODER_DELETE_DICT_ENTRIES = [ + "vkDestroyShaderModule", +] + global_state_prefix = "m_state->on_" decoder_decl_preamble = """ @@ -282,7 +286,10 @@ def emit_decode_parameters(typeInfo: VulkanTypeInfo, api: VulkanAPI, cgen, globa lenAccess = cgen.generalLengthAccess(p) if p.dispatchHandle: - emit_dispatch_unmarshal(typeInfo, p, cgen, globalWrapped) + if api.name in DELAYED_DECODER_DELETE_DICT_ENTRIES: + emit_dispatch_unmarshal(typeInfo, p, cgen, False) + else: + emit_dispatch_unmarshal(typeInfo, p, cgen, globalWrapped) else: destroy = p.nonDispatchableHandleDestroy or p.dispatchableHandleDestroy noUnbox = api.name in ["vkQueueFlushCommandsGOOGLE", "vkQueueFlushCommandsFromAuxMemoryGOOGLE"] and p.paramName == "commandBuffer" @@ -427,6 +434,8 @@ def emit_destroyed_handle_cleanup(api, cgen): if None == lenAccess or "1" == lenAccess: if api.name in DELAYED_DECODER_DELETES: cgen.stmt("delayed_delete_%s(boxed_%s_preserve, unboxed_device, delayed_remove_callback)" % (p.typeName, p.paramName)) + elif api.name in DELAYED_DECODER_DELETE_DICT_ENTRIES: + cgen.stmt("delayed_delete_%s(boxed_%s_preserve, unboxed_device, nullptr)" % (p.typeName, p.paramName)) else: cgen.stmt("delete_%s(boxed_%s_preserve)" % (p.typeName, p.paramName)) else: diff --git a/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py b/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py index a9660829e3b..ab8d93f2779 100644 --- a/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py +++ b/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py @@ -27,6 +27,7 @@ public: void save(android::base::Stream* stream); void load(android::base::Stream* stream, emugl::GfxApiLogger& gfx_logger, emugl::HealthMonitor<>* healthMonitor); + void createExtraHandlesForNextApi(const uint64_t* created, uint32_t count); """ decoder_snapshot_decl_postamble = """ @@ -56,6 +57,9 @@ public: mReconstruction.load(stream, gfx_logger, healthMonitor); } + void createExtraHandlesForNextApi(const uint64_t* created, uint32_t count) { + mReconstruction.createExtraHandlesForNextApi(created, count); + } """ decoder_snapshot_impl_postamble = """ @@ -76,6 +80,10 @@ void VkDecoderSnapshot::load(android::base::Stream* stream, GfxApiLogger& gfx_lo mImpl->load(stream, gfx_logger, healthMonitor); } +void VkDecoderSnapshot::createExtraHandlesForNextApi(const uint64_t* created, uint32_t count) { + mImpl->createExtraHandlesForNextApi(created, count); +} + VkDecoderSnapshot::~VkDecoderSnapshot() = default; """ @@ -102,20 +110,56 @@ def extract_deps_vkAllocateCommandBuffers(param, access, lenExpr, api, cgen): cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkCommandPool(pAllocateInfo->commandPool)")) +def extract_deps_vkCreateImageView(param, access, lenExpr, api, cgen): + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkImage(pCreateInfo->image)")) + +def extract_deps_vkCreateGraphicsPipelines(param, access, lenExpr, api, cgen): + cgen.beginFor("uint32_t i = 0", "i < createInfoCount", "++i") + cgen.beginFor("uint32_t j = 0", "j < pCreateInfos[i].stageCount", "++j") + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)(%s + i), %s, (uint64_t)(uintptr_t)%s)" % \ + (access, 1, "unboxed_to_boxed_non_dispatchable_VkShaderModule(pCreateInfos[i].pStages[j].module)")) + cgen.endFor() + cgen.endFor() + +def extract_deps_vkCreateFramebuffer(param, access, lenExpr, api, cgen): + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + (access, lenExpr, "unboxed_to_boxed_non_dispatchable_VkRenderPass(pCreateInfo->renderPass)")) + +def extract_deps_vkBindImageMemory(param, access, lenExpr, api, cgen): + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % \ + (access, lenExpr, "(uint64_t)(uintptr_t)unboxed_to_boxed_non_dispatchable_VkDeviceMemory(memory)")) + specialCaseDependencyExtractors = { "vkAllocateCommandBuffers" : extract_deps_vkAllocateCommandBuffers, + "vkCreateImageView" : extract_deps_vkCreateImageView, + "vkCreateGraphicsPipelines" : extract_deps_vkCreateGraphicsPipelines, + "vkCreateFramebuffer" : extract_deps_vkCreateFramebuffer, + "vkBindImageMemory": extract_deps_vkBindImageMemory, } apiSequences = { "vkAllocateMemory" : ["vkAllocateMemory", "vkMapMemoryIntoAddressSpaceGOOGLE"] } +# vkBindImageMemory needs to execute before vkCreateImageView +# Thus we inject it into VkImage creation sequence. +# TODO: add vkBindImageMemory2 into this list +apiInitializes = { + "vkBindImageMemory": ["image"], +} + apiModifies = { "vkMapMemoryIntoAddressSpaceGOOGLE" : ["memory"], "vkGetBlobGOOGLE" : ["memory"], - "vkBindImageMemory": ["image"], } +def is_initialize_operation(api, param): + if api.name in apiInitializes: + if param.paramName in apiInitializes[api.name]: + return True + return False + def is_modify_operation(api, param): if api.name in apiModifies: if param.paramName in apiModifies[api.name]: @@ -133,32 +177,42 @@ def emit_impl(typeInfo, api, cgen): if lenExpr is None: lenExpr = "1" + # Note that in vkCreate*, the last parameter (the output) is boxed. But all input parameters are unboxed. + if p.pointerIndirectionLevels > 0: access = p.paramName else: access = "(&%s)" % p.paramName - if p.isCreatedBy(api): + if p.isCreatedBy(api) or is_initialize_operation(api, p): + if p.isCreatedBy(api): + boxed_access = access + else: + cgen.stmt("%s boxed_%s = unboxed_to_boxed_non_dispatchable_%s(%s[0])" % (p.typeName, p.typeName, p.typeName, access)) + boxed_access = "&boxed_%s" % p.typeName + if p.pointerIndirectionLevels > 0: + cgen.stmt("if (!%s) return" % access) cgen.stmt("android::base::AutoLock lock(mLock)") cgen.line("// %s create" % p.paramName) - cgen.stmt("mReconstruction.addHandles((const uint64_t*)%s, %s)" % (access, lenExpr)); + if p.isCreatedBy(api): + cgen.stmt("mReconstruction.addHandles((const uint64_t*)%s, %s)" % (boxed_access, lenExpr)); - if p.typeName in handleDependenciesDict: + if p.isCreatedBy(api) and p.typeName in handleDependenciesDict: dependsOnType = handleDependenciesDict[p.typeName]; for p2 in api.parameters: if p2.typeName == dependsOnType: - cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % (access, lenExpr, p2.paramName)) - if api.name in specialCaseDependencyExtractors: - specialCaseDependencyExtractors[api.name](p, access, lenExpr, api, cgen) + cgen.stmt("mReconstruction.addHandleDependency((const uint64_t*)%s, %s, (uint64_t)(uintptr_t)%s)" % (boxed_access, lenExpr, p2.paramName)) + if api.name in specialCaseDependencyExtractors: + specialCaseDependencyExtractors[api.name](p, boxed_access, lenExpr, api, cgen) - cgen.stmt("if (!%s) return" % access) cgen.stmt("auto apiHandle = mReconstruction.createApiInfo()") cgen.stmt("auto apiInfo = mReconstruction.getApiInfo(apiHandle)") cgen.stmt("mReconstruction.setApiTrace(apiInfo, OP_%s, snapshotTraceBegin, snapshotTraceBytes)" % api.name) if lenAccessGuard is not None: cgen.beginIf(lenAccessGuard) - cgen.stmt("mReconstruction.forEachHandleAddApi((const uint64_t*)%s, %s, apiHandle)" % (access, lenExpr)) - cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiHandle, (const uint64_t*)%s, %s)" % (access, lenExpr)) + cgen.stmt("mReconstruction.forEachHandleAddApi((const uint64_t*)%s, %s, apiHandle)" % (boxed_access, lenExpr)) + if p.isCreatedBy(api): + cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiHandle, (const uint64_t*)%s, %s)" % (boxed_access, lenExpr)) if lenAccessGuard is not None: cgen.endIf()