CR-1201062: Fixing issue where enabling both AIE trace and PL trace r…

…esulted in PL trace not being generated (#8241)

src/runtime_src/xdp/profile/database/static_info/device_info.cpp

@@ -1,6 +1,6 @@
 /**
  * Copyright (C) 2021 Xilinx, Inc
- * Copyright (C) 2022-2023 Advanced Micro Devices, Inc - All rights reserved.
+ * Copyright (C) 2022-2024 Advanced Micro Devices, Inc - All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"). You may
  * not use this file except in compliance with the License. A copy of the
@@ -20,7 +20,7 @@
 #include "xdp/profile/database/static_info/device_info.h"
 #include "xdp/profile/database/static_info/pl_constructs.h"
 #include "xdp/profile/database/static_info/xclbin_info.h"
-#include "xdp/profile/device/device_intf.h"
+#include "xdp/profile/device/pl_device_intf.h"
 
 namespace xdp {
 
@@ -56,9 +56,9 @@ namespace xdp {
     // When loading a new xclbin, we need to destroy any existing
     //  device interface.
     if (loadedXclbins.size() > 0) {
-      if (loadedXclbins.back()->deviceIntf != nullptr) {
-        delete loadedXclbins.back()->deviceIntf ;
-        loadedXclbins.back()->deviceIntf = nullptr ;
+      if (loadedXclbins.back()->plDeviceIntf != nullptr) {
+        delete loadedXclbins.back()->plDeviceIntf ;
+        loadedXclbins.back()->plDeviceIntf = nullptr ;
       }
     }
     loadedXclbins.push_back(xclbin) ;

src/runtime_src/xdp/profile/database/static_info/xclbin_info.cpp

@@ -1,6 +1,6 @@
 /**
  * Copyright (C) 2021 Xilinx, Inc
- * Copyright (C) 2022-2023 Advanced Micro Devices, Inc. - All rights reserved
+ * Copyright (C) 2022-2024 Advanced Micro Devices, Inc. - All rights reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License"). You may
  * not use this file except in compliance with the License. A copy of the
@@ -20,7 +20,7 @@
 #include "xdp/profile/database/static_info/aie_constructs.h"
 #include "xdp/profile/database/static_info/pl_constructs.h"
 #include "xdp/profile/database/static_info/xclbin_info.h"
-#include "xdp/profile/device/device_intf.h"
+#include "xdp/profile/device/pl_device_intf.h"
 
 namespace xdp {
 
@@ -103,8 +103,8 @@ namespace xdp {
 
   XclbinInfo::~XclbinInfo()
   {
-    if (deviceIntf)
-      delete deviceIntf ;
+    if (plDeviceIntf)
+      delete plDeviceIntf ;
   }
 
 } // end namespace xdp

src/runtime_src/xdp/profile/database/static_info/xclbin_info.h

@@ -32,7 +32,7 @@
 namespace xdp {
 
   // Forward declarations
-  class DeviceIntf ;
+  class PLDeviceIntf;
   struct Monitor ;
   struct Memory ;
   class ComputeUnitInstance ;
@@ -162,7 +162,7 @@ namespace xdp {
     // The interface with actually communicating with the device.  This
     //  handles the abstractions necessary for communicating in emulation,
     //  actual hardware, and through different mechanisms.
-    DeviceIntf* deviceIntf = nullptr ;
+    PLDeviceIntf* plDeviceIntf = nullptr ;
 
     // The configuration of the PL portion of the design
     PLInfo pl ;

src/runtime_src/xdp/profile/database/static_info_database.cpp

@@ -47,7 +47,8 @@
 #include "xdp/profile/database/static_info/pl_constructs.h"
 #include "xdp/profile/database/static_info/xclbin_info.h"
 #include "xdp/profile/database/static_info_database.h"
-#include "xdp/profile/device/device_intf.h"
+#include "xdp/profile/device/pl_device_intf.h"
+#include "xdp/profile/device/xdp_base_device.h"
 #include "xdp/profile/plugin/vp_base/utility.h"
 #include "xdp/profile/writer/vp_base/vp_run_summary.h"
 
@@ -306,9 +307,9 @@ namespace xdp {
     XclbinInfo* xclbin = deviceInfo[deviceId]->currentXclbin() ;
     if (!xclbin)
       return ;
-    if (xclbin->deviceIntf) {
-      delete xclbin->deviceIntf ;
-      xclbin->deviceIntf = nullptr ;
+    if (xclbin->plDeviceIntf) {
+      delete xclbin->plDeviceIntf ;
+      xclbin->plDeviceIntf = nullptr ;
     }
   }
 
@@ -380,7 +381,7 @@ namespace xdp {
     return deviceInfo[deviceId]->deviceName ;
   }
 
-  DeviceIntf* VPStaticDatabase::getDeviceIntf(uint64_t deviceId)
+  PLDeviceIntf* VPStaticDatabase::getDeviceIntf(uint64_t deviceId)
   {
     std::lock_guard<std::mutex> lock(deviceLock) ;
 
@@ -390,55 +391,39 @@ namespace xdp {
     if (!xclbin)
       return nullptr ;
 
-    return xclbin->deviceIntf ;
+    return xclbin->plDeviceIntf ;
   }
 
-  DeviceIntf* VPStaticDatabase::createDeviceIntf(uint64_t deviceId,
-                                                 xdp::Device* dev)
+  // This function will create a PL Device Interface if an xdp::Device is
+  // passed in, and then associate it with the current xclbin loaded onto
+  // the device corresponding to deviceId.
+  void VPStaticDatabase::createPLDeviceIntf(uint64_t deviceId, xdp::Device* dev)
   {
     std::lock_guard<std::mutex> lock(deviceLock);
 
+    if (dev == nullptr)
+      return;
     if (deviceInfo.find(deviceId) == deviceInfo.end())
-      return nullptr;
+      return;
     XclbinInfo* xclbin = deviceInfo[deviceId]->currentXclbin();
     if (xclbin == nullptr)
-      return nullptr;
-    if (xclbin->deviceIntf != nullptr)
-      return xclbin->deviceIntf;
+      return;
+    if (xclbin->plDeviceIntf != nullptr)
+      delete xclbin->plDeviceIntf; // It shouldn't be...
 
-    xclbin->deviceIntf = new DeviceIntf();
-    xclbin->deviceIntf->setDevice(dev);
+    xclbin->plDeviceIntf = new PLDeviceIntf();
+    xclbin->plDeviceIntf->setDevice(dev);
     try {
-      xclbin->deviceIntf->readDebugIPlayout();
+      xclbin->plDeviceIntf->readDebugIPlayout();
     }
     catch (std::exception& /* e */) {
       // If reading the debug ip layout fails, we shouldn't have
       // any device interface at all
-      delete xclbin->deviceIntf;
-      xclbin->deviceIntf = nullptr;
+      delete xclbin->plDeviceIntf;
+      xclbin->plDeviceIntf = nullptr;
     }
-    return xclbin->deviceIntf;
   }
 
-  DeviceIntf* VPStaticDatabase::createDeviceIntfClient(uint64_t deviceId,
-                                                 xdp::Device* dev)
-  {
-    std::lock_guard<std::mutex> lock(deviceLock);
-
-    if (deviceInfo.find(deviceId) == deviceInfo.end())
-      return nullptr;
-    XclbinInfo* xclbin = deviceInfo[deviceId]->currentXclbin();
-    if (xclbin == nullptr)
-      return nullptr;
-    if (xclbin->deviceIntf != nullptr)
-      return xclbin->deviceIntf;
-
-    xclbin->deviceIntf = new DeviceIntf();
-    xclbin->deviceIntf->setDevice(dev);
-    return xclbin->deviceIntf;
-  }
-
-
   uint64_t VPStaticDatabase::getKDMACount(uint64_t deviceId)
   {
     std::lock_guard<std::mutex> lock(deviceLock) ;
@@ -650,7 +635,7 @@ namespace xdp {
       return ;
 
     // User space AM in sorted order of their slotIds.  Matches with
-    //  sorted list of AM in xdp::DeviceIntf
+    //  sorted list of AM in xdp::PLDeviceIntf
     size_t count = 0 ;
     for (auto mon : xclbin->pl.ams) {
       if (count >= size)
@@ -1323,9 +1308,10 @@ namespace xdp {
   // This function is called whenever a device is loaded with an
   //  xclbin.  It has to clear out any previous device information and
   //  reload our information.
-  void VPStaticDatabase::updateDevice(uint64_t deviceId, void* devHandle)
+  void VPStaticDatabase::updateDevice(uint64_t deviceId, xdp::Device* xdpDevice, void* devHandle)
   {
-    std::shared_ptr<xrt_core::device> device = xrt_core::get_userpf_device(devHandle);
+    std::shared_ptr<xrt_core::device> device =
+      xrt_core::get_userpf_device(devHandle);
     if (nullptr == device)
       return;
 
@@ -1336,15 +1322,16 @@ namespace xdp {
       return;
 
     xrt::xclbin xrtXclbin = device->get_xclbin(device->get_xclbin_uuid());
-    DeviceInfo* devInfo   = updateDevice(deviceId, xrtXclbin, false);
+    DeviceInfo* devInfo   = updateDevice(deviceId, xrtXclbin, xdpDevice, false);
     if (device->is_nodma())
       devInfo->isNoDMADevice = true;
   }
 
   void VPStaticDatabase::updateDeviceClient(uint64_t deviceId, std::shared_ptr<xrt_core::device> device)
   {
     xrt::xclbin xrtXclbin = device->get_xclbin(device->get_xclbin_uuid());
-    updateDevice(deviceId, xrtXclbin, true);
+    // Client should have no PL interface
+    updateDevice(deviceId, xrtXclbin, nullptr, true);
   }
 
   // Return true if we should reset the device information.
@@ -2027,7 +2014,7 @@ namespace xdp {
     NoCNode* noc = new NoCNode(index, debugIpData->m_name, readTrafficClass,
                                writeTrafficClass) ;
     xclbin->aie.nocList.push_back(noc) ;
-    // nocList in xdp::DeviceIntf is sorted; Is that required here?
+    // nocList in xdp::PLDeviceIntf is sorted; Is that required here?
   }
 
   void VPStaticDatabase::initializeTS2MM(DeviceInfo* devInfo,
@@ -2066,19 +2053,26 @@ namespace xdp {
   {
     xrt::xclbin xrtXclbin = xrt::xclbin(xclbinFile);
 
-    updateDevice(deviceId, xrtXclbin, false);
+    // The PL post-processor does not need a connection to the actual hardware
+    updateDevice(deviceId, xrtXclbin, nullptr, false);
   }
 
   // Methods using xrt::xclbin to retrive static information
 
-  DeviceInfo* VPStaticDatabase::updateDevice(uint64_t deviceId, xrt::xclbin xrtXclbin, bool clientBuild)
+  DeviceInfo* VPStaticDatabase::updateDevice(uint64_t deviceId, xrt::xclbin xrtXclbin, xdp::Device* xdpDevice, bool clientBuild)
   {
     // We need to update the device, but if we had an xclbin previously loaded
-    //  then we need to mark it
+    //  then we need to mark it and remove the PL interface.  We'll
+    //  create a new PL interface if necessary
     if (deviceInfo.find(deviceId) != deviceInfo.end()) {
       XclbinInfo* xclbin = deviceInfo[deviceId]->currentXclbin() ;
-      if (xclbin)
+      if (xclbin) {
         db->getDynamicInfo().markXclbinEnd(deviceId) ;
+        if (xclbin->plDeviceIntf != nullptr) {
+          delete xclbin->plDeviceIntf;
+          xclbin->plDeviceIntf = nullptr;
+        }
+      }
     }
 
     DeviceInfo* devInfo = nullptr ;
@@ -2111,6 +2105,8 @@ namespace xdp {
 
     if (!initializeStructure(currentXclbin, xrtXclbin)) {
       delete currentXclbin;
+      if (xdpDevice != nullptr)
+        delete xdpDevice;
       return devInfo;
     }
 
@@ -2121,8 +2117,10 @@ namespace xdp {
     // Add aie clock to xclbin
     setAIEClockRateMHz(deviceId, xrtXclbin);
 
-    return devInfo;
+    if (xdpDevice != nullptr)
+      createPLDeviceIntf(deviceId, xdpDevice);
 
+    return devInfo;
   }
 
   void VPStaticDatabase::setDeviceNameFromXclbin(uint64_t deviceId, xrt::xclbin xrtXclbin)

src/runtime_src/xdp/profile/database/static_info_database.h

@@ -38,7 +38,8 @@ namespace xdp {
   // Forward declarations of general XDP constructs
   class VPDatabase;
   class VPWriter;
-  class DeviceIntf ;
+  class PLDeviceIntf;
+  class Device;
 
   // Forward declarations of PL contents
   struct Monitor ;
@@ -144,9 +145,12 @@ namespace xdp {
     bool initializeStructure(XclbinInfo*, xrt::xclbin);
     bool initializeProfileMonitors(DeviceInfo*, xrt::xclbin);
     double findClockRate(xrt::xclbin);
-    DeviceInfo* updateDevice(uint64_t deviceId, xrt::xclbin xrtXclbin, bool clientBuild) ;
-
 
+    // This common private updateDevice functionality takes an xdp::Device
+    // pointer to handle any connection to the PL side as necessary.
+    // Some plugins do not require any PL control and will pass in nullptr
+    DeviceInfo* updateDevice(uint64_t deviceId, xrt::xclbin xrtXclbin,
+                             xdp::Device* xdpDevice, bool clientBuild) ;
 
   public:
     VPStaticDatabase(VPDatabase* d) ;
@@ -255,9 +259,8 @@ namespace xdp {
     XDP_CORE_EXPORT double getPLMaxClockRateMHz(uint64_t deviceId);
     XDP_CORE_EXPORT void setDeviceName(uint64_t deviceId, const std::string& name) ;
     XDP_CORE_EXPORT std::string getDeviceName(uint64_t deviceId) ;
-    XDP_CORE_EXPORT DeviceIntf* getDeviceIntf(uint64_t deviceId) ;
-    XDP_CORE_EXPORT DeviceIntf* createDeviceIntf(uint64_t deviceId, xdp::Device* dev);
-    XDP_CORE_EXPORT DeviceIntf* createDeviceIntfClient(uint64_t deviceId, xdp::Device* dev);
+    XDP_CORE_EXPORT PLDeviceIntf* getDeviceIntf(uint64_t deviceId) ;
+    XDP_CORE_EXPORT void createPLDeviceIntf(uint64_t deviceId, xdp::Device* xdpDevice);
     XDP_CORE_EXPORT uint64_t getKDMACount(uint64_t deviceId) ;
     XDP_CORE_EXPORT void setHostMaxReadBW(uint64_t deviceId, double bw) ;
     XDP_CORE_EXPORT double getHostMaxReadBW(uint64_t deviceId) ;
@@ -272,7 +275,7 @@ namespace xdp {
     XDP_CORE_EXPORT ComputeUnitInstance* getCU(uint64_t deviceId, int32_t cuId) ;
     XDP_CORE_EXPORT Memory* getMemory(uint64_t deviceId, int32_t memId) ;
     // Reseting device information whenever a new xclbin is added
-    XDP_CORE_EXPORT void updateDevice(uint64_t deviceId, void* devHandle) ;
+    XDP_CORE_EXPORT void updateDevice(uint64_t deviceId, xdp::Device* xdpDevice, void* devHandle) ;
     XDP_CORE_EXPORT void updateDeviceClient(uint64_t deviceId, std::shared_ptr<xrt_core::device> device);
 
     // *********************************************************

src/runtime_src/xdp/profile/device/aie_trace/aie_trace_offload.cpp

@@ -1,6 +1,6 @@
 /**
  * Copyright (C) 2019-2022 Xilinx, Inc
- * Copyright (C) 2022-2023 Advanced Micro Devices, Inc. - All rights reserved
+ * Copyright (C) 2022-2024 Advanced Micro Devices, Inc. - All rights reserved
  *
  * Licensed under the Apache License, Version 2.0 (the "License"). You may
  * not use this file except in compliance with the License. A copy of the
@@ -25,7 +25,7 @@
 #include "xdp/profile/database/static_info/aie_constructs.h"
 #include "xdp/profile/device/aie_trace/aie_trace_logger.h"
 #include "xdp/profile/device/aie_trace/aie_trace_offload.h"
-#include "xdp/profile/device/device_intf.h"
+#include "xdp/profile/device/pl_device_intf.h"
 #include "xdp/profile/plugin/aie_trace/x86/aie_trace_kernel_config.h"
 
 /*
@@ -43,7 +43,7 @@ namespace xdp {
 
 AIETraceOffload::AIETraceOffload
   ( void* handle, uint64_t id
-  , DeviceIntf* dInt
+  , PLDeviceIntf* dInt
   , AIETraceLogger* logger
   , bool isPlio
   , uint64_t totalSize

src/runtime_src/xdp/profile/device/aie_trace/aie_trace_offload.h

@@ -40,7 +40,7 @@
 
 namespace xdp {
 
-class DeviceIntf;
+class PLDeviceIntf;
 class AIETraceLogger;
 
 #define debug_stream \
@@ -90,7 +90,7 @@ class AIETraceOffload
 {
   public:
     AIETraceOffload(void* handle, uint64_t id,
-                    DeviceIntf*, AIETraceLogger*,
+                    PLDeviceIntf*, AIETraceLogger*,
                     bool     isPlio,
                     uint64_t totalSize,
                     uint64_t numStrm
@@ -121,7 +121,7 @@ class AIETraceOffload
 
     void*           deviceHandle;
     uint64_t        deviceId;
-    DeviceIntf*     deviceIntf;
+    PLDeviceIntf*     deviceIntf;
     AIETraceLogger* traceLogger;
 
     bool isPLIO;