Pass VkSnapshotApiCallInfo-s through VkDecoderGlobalState

... so that `VkDecoderGlobalState` can append additional information
needed for snapshotting. Specifically, `VkDecoderGlobalState` may create
additional boxed handles that are not visible directly in the API surface.
For example, `vkCreateDevice()` creates boxed handles for the `VkQueue`-s
and `vkCreateDescriptorPool()` creates boxed handles for pre-allocated
`VkDescriptorSets`. These boxed handles are not recoverable from the API
for `vkCreateDevice()` nor `vkCreateDescriptorPool()` directly. This was
previously worked around by just sticking the extra boxed handles in
`VkReconstruction::mExtraHandlesForNextApi` but this is not thread safe.
Instead, let's give `VkDecoderGlobalState` and `VkDecoderSnapshot` exclusive
access to individual `VkSnapshotApiCallInfo` objects.

Reviewed-by: Marcin Radomski <dextero@google.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33018>

src/gfxstream/codegen/scripts/cereal/decoder.py

@@ -29,6 +29,8 @@ DELAYED_DECODER_DELETE_DICT_ENTRIES = [
     "vkDestroyShaderModule",
 ]
 
+SNAPSHOT_API_CALL_INFO_VARNAME = "snapshotApiCallInfo"
+
 global_state_prefix = "m_state->on_"
 
 decoder_decl_preamble = """
@@ -77,7 +79,8 @@ public:
              m_boxedHandleUnwrapAndDeleteMapping(m_state),
              m_boxedHandleUnwrapAndDeletePreserveBoxedMapping(m_state),
              m_prevSeqno(std::nullopt),
-             m_queueSubmitWithCommandsEnabled(m_state->getFeatures().VulkanQueueSubmitWithCommands.enabled) {}
+             m_queueSubmitWithCommandsEnabled(m_state->getFeatures().VulkanQueueSubmitWithCommands.enabled),
+             m_snapshotsEnabled(m_state->snapshotsEnabled()) {}
     %s* stream() { return &m_vkStream; }
     VulkanMemReadingStream* readStream() { return &m_vkMemReadingStream; }
 
@@ -103,6 +106,7 @@ private:
     BoxedHandleUnwrapAndDeletePreserveBoxedMapping m_boxedHandleUnwrapAndDeletePreserveBoxedMapping;
     std::optional<uint32_t> m_prevSeqno;
     bool m_queueSubmitWithCommandsEnabled = false;
+    const bool m_snapshotsEnabled = false;
 };
 
 VkDecoder::VkDecoder() :
@@ -358,7 +362,7 @@ def emit_global_state_wrapped_call(api, cgen, context):
         print("Error: Cannot generate a global state wrapped call that is also a delayed delete (yet)");
         raise
 
-    customParams = ["&m_pool"] + list(map(lambda p: p.paramName, api.parameters))
+    customParams = ["&m_pool", SNAPSHOT_API_CALL_INFO_VARNAME] + list(map(lambda p: p.paramName, api.parameters))
     if context:
         customParams += ["context"]
     cgen.vkApiCall(api, customPrefix=global_state_prefix, \
@@ -461,9 +465,10 @@ def emit_seqno_incr(api, cgen):
 
 def emit_snapshot(typeInfo, api, cgen):
     additionalParams = [ \
+        makeVulkanTypeSimple(False, "android::base::BumpPool", 1, "&m_pool"),
+        makeVulkanTypeSimple(True, "VkSnapshotApiCallInfo", 1, SNAPSHOT_API_CALL_INFO_VARNAME),
         makeVulkanTypeSimple(True, "uint8_t", 1, "packet"),
         makeVulkanTypeSimple(False, "size_t", 0, "packetLen"),
-        makeVulkanTypeSimple(False, "android::base::BumpPool", 1, "&m_pool"),
     ]
 
     retTypeName = api.getRetTypeExpr()
@@ -487,7 +492,7 @@ def emit_snapshot(typeInfo, api, cgen):
         api.withCustomReturnType(makeVulkanTypeSimple(False, "void", 0, "void")). \
             withCustomParameters(customParams)
 
-    cgen.beginIf("m_state->snapshotsEnabled()")
+    cgen.beginIf("m_snapshotsEnabled")
     cgen.vkApiCall(apiForSnapshot, customPrefix="m_state->snapshot()->")
     cgen.endIf()
 
@@ -912,6 +917,13 @@ size_t VkDecoder::Impl::decode(void* buf, size_t len, IOStream* ioStream,
         }
         """)
 
+        self.cgen.line("""
+        VkSnapshotApiCallInfo* %s = nullptr;
+        if (m_snapshotsEnabled) {
+            %s = m_state->snapshot()->createApiCallInfo();
+        }
+        """ % (SNAPSHOT_API_CALL_INFO_VARNAME, SNAPSHOT_API_CALL_INFO_VARNAME))
+
         self.cgen.line("""
         gfx_logger.recordCommandExecution();
         """)
@@ -958,6 +970,12 @@ size_t VkDecoder::Impl::decode(void* buf, size_t len, IOStream* ioStream,
 
         self.cgen.endBlock() # switch stmt
 
+        self.cgen.line("""
+        if (m_snapshotsEnabled) {
+            m_state->snapshot()->destroyApiCallInfoIfUnused(%s);
+        }
+        """ % (SNAPSHOT_API_CALL_INFO_VARNAME))
+
         self.cgen.stmt("ptr += packetLen")
         self.cgen.stmt("vkStream->clearPool()")
         self.cgen.endBlock() # while loop

src/gfxstream/codegen/scripts/cereal/decodersnapshot.py

@@ -23,6 +23,9 @@ class Stream;
 } // namespace base {
 } // namespace android {
 
+namespace gfxstream {
+namespace vk {
+
 class VkDecoderSnapshot {
 public:
     VkDecoderSnapshot();
@@ -31,7 +34,9 @@ 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);
+
+    VkSnapshotApiCallInfo* createApiCallInfo();
+    void destroyApiCallInfoIfUnused(VkSnapshotApiCallInfo* info);
 """
 
 decoder_snapshot_decl_postamble = """
@@ -40,14 +45,19 @@ private:
     std::unique_ptr<Impl> mImpl;
 
 };
+
+}  // namespace vk
+}  // namespace gfxstream
 """
 
 decoder_snapshot_impl_preamble ="""
 
-using namespace gfxstream::vk;
 using emugl::GfxApiLogger;
 using emugl::HealthMonitor;
 
+namespace gfxstream {
+namespace vk {
+
 class VkDecoderSnapshot::Impl {
 public:
     Impl() { }
@@ -61,9 +71,14 @@ public:
         mReconstruction.load(stream, gfx_logger, healthMonitor);
     }
 
-    void createExtraHandlesForNextApi(const uint64_t* created, uint32_t count) {
+    VkSnapshotApiCallInfo* createApiCallInfo() {
-        mLock.lock();
+        android::base::AutoLock lock(mLock);
-        mReconstruction.createExtraHandlesForNextApi(created, count);
+        return mReconstruction.createApiCallInfo();
+    }
+
+    void destroyApiCallInfoIfUnused(VkSnapshotApiCallInfo* info) {
+        android::base::AutoLock lock(mLock);
+        return mReconstruction.destroyApiCallInfoIfUnused(info);
     }
 """
 
@@ -85,13 +100,25 @@ 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) {
+VkSnapshotApiCallInfo* VkDecoderSnapshot::createApiCallInfo() {
-    mImpl->createExtraHandlesForNextApi(created, count);
+    return mImpl->createApiCallInfo();
+}
+
+void VkDecoderSnapshot::destroyApiCallInfoIfUnused(VkSnapshotApiCallInfo* info) {
+    mImpl->destroyApiCallInfoIfUnused(info);
 }
 
 VkDecoderSnapshot::~VkDecoderSnapshot() = default;
 """
 
+decoder_snapshot_namespace_postamble = """
+
+}  // namespace vk
+}  // namespace gfxstream
+
+"""
+
+
 AUXILIARY_SNAPSHOT_API_BASE_PARAM_COUNT = 3
 
 AUXILIARY_SNAPSHOT_API_PARAM_NAMES = [
@@ -236,14 +263,13 @@ def api_special_implementation_vkBindImageMemory2(api, cgen):
               (childObj, "1", childObj))
     cgen.endFor()
 
-    cgen.stmt("auto apiHandle = mReconstruction.createApiInfo()")
+    cgen.stmt("auto apiCallHandle = apiCallInfo->handle")
-    cgen.stmt("auto apiInfo = mReconstruction.getApiInfo(apiHandle)")
+    cgen.stmt("mReconstruction.setApiTrace(apiCallInfo, apiCallPacket, apiCallPacketSize)")
-    cgen.stmt("mReconstruction.setApiTrace(apiInfo, snapshotTraceBegin, snapshotTraceBytes)")
     cgen.line("// Note: the implementation does not work with bindInfoCount > 1");
     cgen.beginFor("uint32_t i = 0", "i < bindInfoCount", "++i")
     cgen.stmt("%s boxed_%s = unboxed_to_boxed_non_dispatchable_%s(pBindInfos[i].image)"
               % (childType, childType, childType))
-    cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*)&{childObj}, {1}, apiHandle, VkReconstruction::BOUND_MEMORY)")
+    cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*)&{childObj}, {1}, apiCallHandle, VkReconstruction::BOUND_MEMORY)")
     cgen.endFor()
 
 apiSpecialImplementation = {
@@ -359,14 +385,13 @@ def emit_impl(typeInfo, api, cgen):
             if api.name in specialCaseDependencyExtractors:
                 specialCaseDependencyExtractors[api.name](p, boxed_access, lenExpr, api, cgen)
 
-            cgen.stmt("auto apiHandle = mReconstruction.createApiInfo()")
+            cgen.stmt("auto apiCallHandle = apiCallInfo->handle")
-            cgen.stmt("auto apiInfo = mReconstruction.getApiInfo(apiHandle)")
+            cgen.stmt("mReconstruction.setApiTrace(apiCallInfo, apiCallPacket, apiCallPacketSize)")
-            cgen.stmt("mReconstruction.setApiTrace(apiInfo, snapshotTraceBegin, snapshotTraceBytes)")
             if lenAccessGuard is not None:
                 cgen.beginIf(lenAccessGuard)
-            cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*){boxed_access}, {lenExpr}, apiHandle, {get_target_state(api, p)})")
+            cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*){boxed_access}, {lenExpr}, apiCallHandle, {get_target_state(api, p)})")
             if p.isCreatedBy(api):
-                cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiHandle, (const uint64_t*)%s, %s)" % (boxed_access, lenExpr))
+                cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiCallHandle, (const uint64_t*)%s, %s)" % (boxed_access, lenExpr))
             if lenAccessGuard is not None:
                 cgen.endIf()
             if isCreateExtraHandleApi:
@@ -391,20 +416,18 @@ def emit_impl(typeInfo, api, cgen):
             cgen.endIf();
             cgen.stmt("uint64_t handle = m_state->newGlobalVkGenericHandle()")
             cgen.stmt("mReconstruction.addHandles((const uint64_t*)(&handle), 1)");
-            cgen.stmt("auto apiHandle = mReconstruction.createApiInfo()")
+            cgen.stmt("auto apiCallHandle = apiCallInfo->handle")
-            cgen.stmt("auto apiInfo = mReconstruction.getApiInfo(apiHandle)")
+            cgen.stmt("mReconstruction.setApiTrace(apiCallInfo, apiCallPacket, apiCallPacketSize)")
-            cgen.stmt("mReconstruction.setApiTrace(apiInfo, snapshotTraceBegin, snapshotTraceBytes)")
             if api.name in specialCaseDependencyExtractors:
                 specialCaseDependencyExtractors[api.name](p, None, None, api, cgen)
-            cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*)(&handle), 1, apiHandle, {get_target_state(api, p)})")
+            cgen.stmt(f"mReconstruction.forEachHandleAddApi((const uint64_t*)(&handle), 1, apiCallHandle, {get_target_state(api, p)})")
-            cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiHandle, (const uint64_t*)(&handle), 1)")
+            cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiCallHandle, (const uint64_t*)(&handle), 1)")
 
         elif is_modify_operation(api, p) or is_clear_modifier_operation(api, p):
             cgen.stmt("android::base::AutoLock lock(mLock)")
             cgen.line("// %s modify" % p.paramName)
-            cgen.stmt("auto apiHandle = mReconstruction.createApiInfo()")
+            cgen.stmt("auto apiCallHandle = apiCallInfo->handle")
-            cgen.stmt("auto apiInfo = mReconstruction.getApiInfo(apiHandle)")
+            cgen.stmt("mReconstruction.setApiTrace(apiCallInfo, apiCallPacket, apiCallPacketSize)")
-            cgen.stmt("mReconstruction.setApiTrace(apiInfo, snapshotTraceBegin, snapshotTraceBytes)")
             if lenAccessGuard is not None:
                 cgen.beginIf(lenAccessGuard)
             cgen.beginFor("uint32_t i = 0", "i < %s" % lenExpr, "++i")
@@ -414,7 +437,7 @@ def emit_impl(typeInfo, api, cgen):
                 cgen.line("// %s is already boxed, no need to box again" % p.paramName)
                 cgen.stmt("%s boxed = %s(%s[i])" % (p.typeName, p.typeName, access))
             if is_modify_operation(api, p):
-                cgen.stmt("mReconstruction.forEachHandleAddModifyApi((const uint64_t*)(&boxed), 1, apiHandle)")
+                cgen.stmt("mReconstruction.forEachHandleAddModifyApi((const uint64_t*)(&boxed), 1, apiCallHandle)")
             else: # is clear modifier operation
                 cgen.stmt("mReconstruction.forEachHandleClearModifyApi((const uint64_t*)(&boxed), 1)")
             cgen.endFor()
@@ -453,9 +476,11 @@ class VulkanDecoderSnapshot(VulkanWrapperGenerator):
         api = self.typeInfo.apis[name]
 
         additionalParams = [ \
-            makeVulkanTypeSimple(True, "uint8_t", 1, "snapshotTraceBegin"),
+            makeVulkanTypeSimple(False, "android::base::BumpPool", 1, "pool"),
-            makeVulkanTypeSimple(False, "size_t", 0, "snapshotTraceBytes"),
+            makeVulkanTypeSimple(False, "VkSnapshotApiCallInfo", 1, "apiCallInfo"),
-            makeVulkanTypeSimple(False, "android::base::BumpPool", 1, "pool"),]
+            makeVulkanTypeSimple(True, "uint8_t", 1, "apiCallPacket"),
+            makeVulkanTypeSimple(False, "size_t", 0, "apiCallPacketSize"),
+        ]
 
         if api.retType.typeName != "void":
             additionalParams.append( \
@@ -495,4 +520,4 @@ class VulkanDecoderSnapshot(VulkanWrapperGenerator):
                 self.cgenImpl.line("#endif")
 
         self.module.appendImpl(self.cgenImpl.swapCode())
-
+        self.module.appendImpl(decoder_snapshot_namespace_postamble)

src/gfxstream/codegen/scripts/cereal/subdecode.py

@@ -268,7 +268,7 @@ def emit_dispatch_call(api, cgen):
 
 
 def emit_global_state_wrapped_call(api, cgen, context=False):
-    customParams = ["pool", "(VkCommandBuffer)(boxed_dispatchHandle)"] + \
+    customParams = ["pool", "nullptr", "(VkCommandBuffer)(boxed_dispatchHandle)"] + \
         list(map(lambda p: p.paramName, api.parameters[1:]))
     if context:
         customParams += ["context"];

src/gfxstream/codegen/scripts/cerealgenerator.py

@@ -485,6 +485,7 @@ using DlSymFunc = void* (void*, const char*);
 
         decoderSnapshotHeaderIncludes = f"""
 #include <memory>
+#include "VkSnapshotApiCall.h"
 #include "{self.utilsHeaderDirPrefix}/GfxApiLogger.h"
 #include "{self.baseLibDirPrefix}/HealthMonitor.h"
 #include "goldfish_vk_private_defs.h"