From a44a29c3c52fd7c347811bfe90a07da57f96226f Mon Sep 17 00:00:00 2001 From: Jyotheeswar Ganne Date: Mon, 1 Jun 2026 00:37:49 -0600 Subject: [PATCH] xdp: skip run-lifecycle hooks for XDP-internal runs Runs created by XDP plugins themselves (e.g. xrt::ext::kernel "XDP_KERNEL:{IPUV1CNN}") must not re-enter the XDP run-lifecycle hooks. Enforce this once in the shared XDPPlugin base class so every present and future plugin inherits the skip automatically with no per-plugin code. * Add public non-virtual XDPPlugin::run{Constructor,Start,Wait}Hook wrappers and protected virtual run*Impl methods (default no-ops). The wrappers prefix-match the kernel name against "XDP_KERNEL" and return on match; otherwise delegate to the *Impl. Plugin _cb.cpp glue continues to call the *Hook wrappers; plugins override the *Impl methods. The wrappers are non-virtual so derived plugins cannot accidentally bypass the filter. * Single source of truth: the "XDP_KERNEL" prefix constant and the helper live file-local in vp_base_plugin.cpp. The helper is also exposed as xdp::isXdpInternalKernel via vp_base/utility.h for any non-hook XDP code path that needs the same check. * Convert AieDtracePlugin (the only current consumer): rename runConstructorHook/Start/Wait to runConstructorImpl/StartImpl/ WaitImpl as protected overrides; bodies unchanged. aie_dtrace_cb.cpp is unchanged - aieDtracePluginInstance.runConstructorHook(...) now resolves to the new base-class filtered wrapper and dispatches through the virtual *Impl override. * core/common/xdp/profile.cpp and profile.h are NOT modified. The no-plugin overhead is unchanged - the existing xrt_core::config::get_aie_dtrace() (and friends) config gate in profile.cpp's dispatchers remains the first and only check on the no-plugin path. Signed-off-by: Jyotheeswar Ganne Co-authored-by: Cursor --- .../plugin/aie_dtrace/aie_dtrace_plugin.cpp | 6 +-- profile/plugin/aie_dtrace/aie_dtrace_plugin.h | 19 ++++++-- profile/plugin/vp_base/utility.h | 8 ++++ profile/plugin/vp_base/vp_base_plugin.cpp | 48 +++++++++++++++++++ profile/plugin/vp_base/vp_base_plugin.h | 38 +++++++++++++++ 5 files changed, 111 insertions(+), 8 deletions(-) diff --git a/profile/plugin/aie_dtrace/aie_dtrace_plugin.cpp b/profile/plugin/aie_dtrace/aie_dtrace_plugin.cpp index a2967496..095de1e0 100644 --- a/profile/plugin/aie_dtrace/aie_dtrace_plugin.cpp +++ b/profile/plugin/aie_dtrace/aie_dtrace_plugin.cpp @@ -186,7 +186,7 @@ namespace xdp { handleToAIEDtraceImpl.erase(itr); } - void AieDtracePlugin::runConstructorHook(void* run_impl_ptr, void* hwctx, uint32_t run_uid, + void AieDtracePlugin::runConstructorImpl(void* run_impl_ptr, void* hwctx, uint32_t run_uid, const std::string& kernel_name, void* elf_handle) { if (!xrt_core::config::get_aie_dtrace()) @@ -201,7 +201,7 @@ namespace xdp { itr->second->generateCTForRun(run_impl_ptr, hwctx, run_uid, kernel_name, elf_handle); } - void AieDtracePlugin::runStartHook(void* run_impl_ptr, void* hwctx, uint32_t run_uid, + void AieDtracePlugin::runStartImpl(void* run_impl_ptr, void* hwctx, uint32_t run_uid, const std::string& kernel_name) { if (!xrt_core::config::get_aie_dtrace()) @@ -213,7 +213,7 @@ namespace xdp { (void)kernel_name; } - void AieDtracePlugin::runWaitHook(void* run_impl_ptr, void* hwctx, uint32_t run_uid, + void AieDtracePlugin::runWaitImpl(void* run_impl_ptr, void* hwctx, uint32_t run_uid, const std::string& kernel_name, int ert_cmd_state) { if (!xrt_core::config::get_aie_dtrace()) diff --git a/profile/plugin/aie_dtrace/aie_dtrace_plugin.h b/profile/plugin/aie_dtrace/aie_dtrace_plugin.h index bceec08e..c74c4ecf 100644 --- a/profile/plugin/aie_dtrace/aie_dtrace_plugin.h +++ b/profile/plugin/aie_dtrace/aie_dtrace_plugin.h @@ -17,14 +17,23 @@ namespace xdp { ~AieDtracePlugin(); void updateAIEDtraceDevice(void* handle, bool hw_context_flow); void endPollforDevice(void* handle); - void runConstructorHook(void* run_impl_ptr, void* hwctx, uint32_t run_uid, - const std::string& kernel_name, void* elf_handle); - void runStartHook(void* run_impl_ptr, void* hwctx, uint32_t run_uid, const std::string& kernel_name); - void runWaitHook(void* run_impl_ptr, void* hwctx, uint32_t run_uid, const std::string& kernel_name, - int ert_cmd_state); static bool alive(); void broadcast(VPDatabase::MessageType msg, void* blob); + protected: + // Overrides of the XDPPlugin run-lifecycle hook implementations. + // aie_dtrace_cb.cpp must NOT call these directly; it calls the + // public XDPPlugin::run*Hook wrappers, which filter out runs + // submitted by XDP plugins themselves before delegating here. + void runConstructorImpl(void* run_impl_ptr, void* hwctx, uint32_t run_uid, + const std::string& kernel_name, + void* elf_handle) override; + void runStartImpl(void* run_impl_ptr, void* hwctx, uint32_t run_uid, + const std::string& kernel_name) override; + void runWaitImpl(void* run_impl_ptr, void* hwctx, uint32_t run_uid, + const std::string& kernel_name, + int ert_cmd_state) override; + private: void writeAll(bool openNewFiles) override; uint64_t getDeviceIDFromHandle(void* handle); diff --git a/profile/plugin/vp_base/utility.h b/profile/plugin/vp_base/utility.h index 3adb4794..2b8b3768 100644 --- a/profile/plugin/vp_base/utility.h +++ b/profile/plugin/vp_base/utility.h @@ -25,6 +25,14 @@ namespace xdp { XDP_CORE_EXPORT uint64_t getAlignedTraceBufSize(uint64_t totalBytes, unsigned int numChunks); + // Returns true if the given kernel name identifies a run that an + // XDP plugin submitted itself (as opposed to a user-application + // run). The convention is a "XDP_KERNEL" prefix on the kernel name + // passed to xrt::ext::kernel; see vp_base_plugin.cpp for the + // canonical definition. Use this from any non-hook XDP code path + // (e.g. device-flow hooks) that needs the same filter. + XDP_CORE_EXPORT bool isXdpInternalKernel(const std::string& kernel_name); + enum Flow { SW_EMU = 0, HW_EMU = 1, diff --git a/profile/plugin/vp_base/vp_base_plugin.cpp b/profile/plugin/vp_base/vp_base_plugin.cpp index 16ba284c..c6ae0919 100644 --- a/profile/plugin/vp_base/vp_base_plugin.cpp +++ b/profile/plugin/vp_base/vp_base_plugin.cpp @@ -18,7 +18,9 @@ #define XDP_CORE_SOURCE #include +#include +#include "xdp/profile/plugin/vp_base/utility.h" #include "xdp/profile/plugin/vp_base/vp_base_plugin.h" #include "xdp/profile/writer/vp_base/vp_run_summary.h" #include "xdp/profile/database/database.h" @@ -34,6 +36,23 @@ namespace xdp { + // Single source of truth for the convention that distinguishes a run + // submitted by an XDP plugin itself from a run submitted by user code. + // xrt_core::api kernel_impl::get_name() strips everything after the + // first ':' (see core/common/api/xrt_kernel.cpp), so an XDP plugin + // kernel constructed as e.g. "XDP_KERNEL:{IPUV1CNN}" arrives at the + // run-lifecycle hooks as exactly "XDP_KERNEL". A prefix match is used + // (rather than equality) to leave room for future internal kernels of + // the form "XDP_KERNEL_FOO" without changing this convention. + namespace { + constexpr const char* kXdpInternalKernelPrefix = "XDP_KERNEL"; + } // namespace + + bool isXdpInternalKernel(const std::string& kernel_name) + { + return kernel_name.rfind(kXdpInternalKernelPrefix, 0) == 0; + } + unsigned int XDPPlugin::trace_file_dump_int_s = 5; bool XDPPlugin::trace_int_cached = false; @@ -164,4 +183,33 @@ namespace xdp { } } + void XDPPlugin::runConstructorHook(void* run_impl_ptr, void* hwctx, + uint32_t run_uid, + const std::string& kernel_name, + void* elf_handle) + { + if (isXdpInternalKernel(kernel_name)) + return; + runConstructorImpl(run_impl_ptr, hwctx, run_uid, kernel_name, elf_handle); + } + + void XDPPlugin::runStartHook(void* run_impl_ptr, void* hwctx, + uint32_t run_uid, + const std::string& kernel_name) + { + if (isXdpInternalKernel(kernel_name)) + return; + runStartImpl(run_impl_ptr, hwctx, run_uid, kernel_name); + } + + void XDPPlugin::runWaitHook(void* run_impl_ptr, void* hwctx, + uint32_t run_uid, + const std::string& kernel_name, + int ert_cmd_state) + { + if (isXdpInternalKernel(kernel_name)) + return; + runWaitImpl(run_impl_ptr, hwctx, run_uid, kernel_name, ert_cmd_state); + } + } // end namespace xdp diff --git a/profile/plugin/vp_base/vp_base_plugin.h b/profile/plugin/vp_base/vp_base_plugin.h index 9518df74..8f3ee3f9 100644 --- a/profile/plugin/vp_base/vp_base_plugin.h +++ b/profile/plugin/vp_base/vp_base_plugin.h @@ -18,6 +18,8 @@ #ifndef VP_BASE_PLUGIN_DOT_H #define VP_BASE_PLUGIN_DOT_H +#include +#include #include #include "xdp/profile/database/database.h" @@ -73,6 +75,24 @@ namespace xdp { XDP_CORE_EXPORT void endWrite(); XDP_CORE_EXPORT void trySafeWrite(const std::string& type, bool openNewFiles); + // Run-lifecycle hook implementations. Plugins that want to react to + // xrt::run construction / start / wait override one or more of these. + // Defaults are no-ops so plugins that don't care need no code. + // These are NEVER called directly by plugin _cb.cpp glue; the public + // non-virtual run*Hook wrappers below are the only entry point and + // they filter out XDP-internal runs first. + virtual void runConstructorImpl(void* /*run_impl_ptr*/, void* /*hwctx*/, + uint32_t /*run_uid*/, + const std::string& /*kernel_name*/, + void* /*elf_handle*/) {} + virtual void runStartImpl(void* /*run_impl_ptr*/, void* /*hwctx*/, + uint32_t /*run_uid*/, + const std::string& /*kernel_name*/) {} + virtual void runWaitImpl(void* /*run_impl_ptr*/, void* /*hwctx*/, + uint32_t /*run_uid*/, + const std::string& /*kernel_name*/, + int /*ert_cmd_state*/) {} + public: XDP_CORE_EXPORT XDPPlugin() ; XDP_CORE_EXPORT virtual ~XDPPlugin() ; @@ -90,6 +110,24 @@ namespace xdp { XDP_CORE_EXPORT static unsigned int get_trace_file_dump_int_s (); + + // Run-lifecycle hook entry points. Plugin _cb.cpp glue MUST call + // these (not the protected *Impl methods). Each one first skips + // runs that an XDP plugin itself submitted (kernels whose name has + // the "XDP_KERNEL" prefix) and otherwise delegates to the + // corresponding virtual *Impl. These are intentionally non-virtual + // so derived plugins cannot accidentally bypass the filter. + XDP_CORE_EXPORT void runConstructorHook(void* run_impl_ptr, void* hwctx, + uint32_t run_uid, + const std::string& kernel_name, + void* elf_handle); + XDP_CORE_EXPORT void runStartHook(void* run_impl_ptr, void* hwctx, + uint32_t run_uid, + const std::string& kernel_name); + XDP_CORE_EXPORT void runWaitHook(void* run_impl_ptr, void* hwctx, + uint32_t run_uid, + const std::string& kernel_name, + int ert_cmd_state); } ; }