From c46e8ca4e6ebeb654302fab9338e1a84d9f80bdd Mon Sep 17 00:00:00 2001 From: Serdar Kocdemir Date: Mon, 24 Feb 2025 18:29:43 +0000 Subject: [PATCH] gfxstream: Add dispatcher validity checks Add conditioning before making driver calls to be able to workaround some of the fatal errors, such as unboxing issues during or after snapshot load. This enables invalidating a host dispatcher based on the application state. A default error will be returned for vulkan calls. Builtin expectation function is used to reduce performance cost of the checks. Reviewed-by: Aaron Ruby Part-of: --- .../codegen/scripts/cereal/common/codegen.py | 22 ++++++++++++++++-- .../codegen/scripts/cereal/decoder.py | 23 ++++++++++++++++--- .../codegen/scripts/cereal/subdecode.py | 8 +++++-- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/gfxstream/codegen/scripts/cereal/common/codegen.py b/src/gfxstream/codegen/scripts/cereal/common/codegen.py index 91ccf2e84f8..3ffc30c42c9 100644 --- a/src/gfxstream/codegen/scripts/cereal/common/codegen.py +++ b/src/gfxstream/codegen/scripts/cereal/common/codegen.py @@ -629,7 +629,9 @@ class CodeGen(object): def generalLengthAccessGuard(self, vulkanType, parentVarName="parent"): return self.makeLengthAccess(vulkanType, parentVarName)[1] - def vkApiCall(self, api, customPrefix="", globalStatePrefix="", customParameters=None, checkForDeviceLost=False, checkForOutOfMemory=False): + def vkApiCall(self, api, customPrefix="", globalStatePrefix="", + customParameters=None, checkForDeviceLost=False, + checkForOutOfMemory=False, checkDispatcher=None): callLhs = None retTypeName = api.getRetTypeExpr() @@ -637,9 +639,22 @@ class CodeGen(object): if retTypeName != "void": retVar = api.getRetVarExpr() - self.stmt("%s %s = (%s)0" % (retTypeName, retVar, retTypeName)) + defaultReturn = "(%s)0" % retTypeName + if retTypeName == "VkResult": + # TODO: return a valid error code based on the call + # This is used to handle invalid dispatcher and snapshot states + deviceLostFunctions = ["vkQueueSubmit", + "vkQueueWaitIdle", + "vkWaitForFences"] + defaultReturn = "VK_ERROR_OUT_OF_HOST_MEMORY" + if api in deviceLostFunctions: + defaultReturn = "VK_ERROR_DEVICE_LOST" + self.stmt("%s %s = %s" % (retTypeName, retVar, defaultReturn)) callLhs = retVar + if (checkDispatcher): + self.beginIf(checkDispatcher) + if customParameters is None: self.funcCall( callLhs, customPrefix + api.name, [p.paramName for p in api.parameters]) @@ -647,6 +662,9 @@ class CodeGen(object): self.funcCall( callLhs, customPrefix + api.name, customParameters) + if (checkDispatcher): + self.endIf() + if retTypeName == "VkResult" and checkForDeviceLost: self.stmt("if ((%s) == VK_ERROR_DEVICE_LOST) %sDeviceLost()" % (callLhs, globalStatePrefix)) diff --git a/src/gfxstream/codegen/scripts/cereal/decoder.py b/src/gfxstream/codegen/scripts/cereal/decoder.py index 7e931a16d3b..674fd606d54 100644 --- a/src/gfxstream/codegen/scripts/cereal/decoder.py +++ b/src/gfxstream/codegen/scripts/cereal/decoder.py @@ -30,6 +30,8 @@ DELAYED_DECODER_DELETE_DICT_ENTRIES = [ ] GLOBAL_COMMANDS_WITHOUT_DISPATCH = [ + "vkCreateInstance", + "vkEnumerateInstanceVersion", "vkEnumerateInstanceExtensionProperties", "vkEnumerateInstanceLayerProperties", ] @@ -216,6 +218,10 @@ def emit_dispatch_unmarshal(typeInfo: VulkanTypeInfo, param: VulkanType, cgen, g cgen.stmt("auto vk = dispatch_%s(%s)" % (param.typeName, param.paramName)) cgen.stmt("// End manual dispatchable handle unboxing for %s" % param.paramName) + else: + # Still need to check dispatcher validity to handle threads with fatal errors + cgen.stmt("auto vk = dispatch_%s(%s)" % + (param.typeName, param.paramName)) def emit_transform(typeInfo, param, cgen, variant="tohost"): @@ -344,12 +350,14 @@ def emit_dispatch_call(api, cgen): cgen.stmt("m_state->lock()") whichDispatch = "vk->" + checkDispatcher = "CC_LIKELY(vk)" if api.name in GLOBAL_COMMANDS_WITHOUT_DISPATCH: whichDispatch = "m_vk->" + checkDispatcher = None cgen.vkApiCall(api, customPrefix=whichDispatch, customParameters=customParams, \ globalStatePrefix=global_state_prefix, checkForDeviceLost=True, - checkForOutOfMemory=True) + checkForOutOfMemory=True, checkDispatcher=checkDispatcher) if api.name in driver_workarounds_global_lock_apis: if not delay: @@ -366,7 +374,7 @@ def emit_global_state_wrapped_call(api, cgen, context): coreCustomParams = list(map(lambda p: p.paramName, api.parameters)) if delay: - cgen.line("std::function delayed_remove_callback = [%s]() {" % ", ".join(coreCustomParams)) + cgen.line("std::function delayed_remove_callback = [vk, %s]() {" % ", ".join(coreCustomParams)) cgen.stmt("auto m_state = VkDecoderGlobalState::get()") customParams = ["nullptr", "nullptr"] + coreCustomParams else: @@ -374,9 +382,13 @@ def emit_global_state_wrapped_call(api, cgen, context): if context: customParams += ["context"] + + checkDispatcher = "CC_LIKELY(vk)" + if api.name in GLOBAL_COMMANDS_WITHOUT_DISPATCH: + checkDispatcher = None cgen.vkApiCall(api, customPrefix=global_state_prefix, \ customParameters=customParams, globalStatePrefix=global_state_prefix, \ - checkForDeviceLost=True, checkForOutOfMemory=True) + checkForDeviceLost=True, checkForOutOfMemory=True, checkDispatcher=checkDispatcher) if delay: cgen.line("};") @@ -835,6 +847,11 @@ class VulkanDecoder(VulkanWrapperGenerator): def onBegin(self,): self.module.appendImpl( "#define MAX_PACKET_LENGTH %s\n" % MAX_PACKET_LENGTH) + self.module.appendImpl( + "#define CC_LIKELY(exp) (__builtin_expect( !!(exp), true ))\n") + self.module.appendImpl( + "#define CC_UNLIKELY(exp) (__builtin_expect( !!(exp), false ))\n") + self.module.appendHeader(decoder_decl_preamble) self.module.appendImpl(decoder_impl_preamble) diff --git a/src/gfxstream/codegen/scripts/cereal/subdecode.py b/src/gfxstream/codegen/scripts/cereal/subdecode.py index 6780aa5d255..47c8e58f698 100644 --- a/src/gfxstream/codegen/scripts/cereal/subdecode.py +++ b/src/gfxstream/codegen/scripts/cereal/subdecode.py @@ -261,7 +261,7 @@ def emit_dispatch_call(api, cgen): cgen.vkApiCall(api, customPrefix="vk->", customParameters=customParams, checkForDeviceLost=True, globalStatePrefix=global_state_prefix, - checkForOutOfMemory=True) + checkForOutOfMemory=True, checkDispatcher="CC_LIKELY(vk)") if api.name in driver_workarounds_global_lock_apis: cgen.stmt("unlock()") @@ -274,7 +274,7 @@ def emit_global_state_wrapped_call(api, cgen, context=False): customParams += ["context"]; cgen.vkApiCall(api, customPrefix=global_state_prefix, customParameters=customParams, checkForDeviceLost=True, - checkForOutOfMemory=True, globalStatePrefix=global_state_prefix) + checkForOutOfMemory=True, globalStatePrefix=global_state_prefix, checkDispatcher="CC_LIKELY(vk)") def emit_default_decoding(typeInfo, api, cgen): @@ -331,6 +331,10 @@ class VulkanSubDecoder(VulkanWrapperGenerator): self.module.appendImpl( "#define MAX_PACKET_LENGTH %s\n" % MAX_PACKET_LENGTH) + self.module.appendImpl( + "#define CC_LIKELY(exp) (__builtin_expect( !!(exp), true ))\n") + self.module.appendImpl( + "#define CC_UNLIKELY(exp) (__builtin_expect( !!(exp), false ))\n") self.module.appendImpl( "size_t subDecode(VulkanMemReadingStream* readStream, VulkanDispatch* vk, void* boxed_dispatchHandle, void* dispatchHandle, VkDeviceSize subDecodeDataSize, const void* pSubDecodeData, const VkDecoderContext& context)\n")