Raise system error when memory free is denied

If user application tries to free memory that is currently being used by
the underlying HW device, the hsaKmtFreeMemory function call will fail.

This would be caused by an incorrect call by the user application. A
system memory error is raised and the user application is expected to
abort when this happens.

Note: This leaves the allocation_map_ table in an inconsistent state as
this address entry is removed from it while the pointer is not actually
free'd. But re-organising the FreeMemory() function would require the
memory_lock_ to be held for much longer and may affect performance.
Since this is a very unlikely and invalid use case, we prefer to leave
the FreeMemory() function as is.

Change-Id: I24279eb98620c32d34f4c5ad1b7a0a30cb65835d
Signed-off-by: David Yat Sin <David.YatSin@amd.com>

runtime/hsa-runtime/core/inc/amd_memory_region.h

@@ -82,7 +82,7 @@ class MemoryRegion : public core::MemoryRegion {
                                  size_t size);
 
   /// @brief Free agent accessible memory (system / local memory).
-  static void FreeKfdMemory(void* ptr, size_t size);
+  static bool FreeKfdMemory(void* ptr, size_t size);
 
   static bool RegisterMemory(void* ptr, size_t size, const HsaMemFlags& MemFlags);
 
@@ -93,7 +93,7 @@ class MemoryRegion : public core::MemoryRegion {
                                     size_t size, uint64_t* alternate_va, HsaMemMapFlags map_flag);
 
   /// @brief Unpin memory.
-  static void MakeKfdMemoryUnresident(const void* ptr);
+  static bool MakeKfdMemoryUnresident(const void* ptr);
 
   MemoryRegion(bool fine_grain, bool kernarg, bool full_profile, bool extended_scope_fine_grain,
                bool user_visible, core::Agent* owner, const HsaMemoryProperties& mem_props);

runtime/hsa-runtime/core/runtime/amd_memory_region.cpp

@@ -65,13 +65,17 @@ void* MemoryRegion::AllocateKfdMemory(const HsaMemFlags& flag, HSAuint32 node_id
   return (status == HSAKMT_STATUS_SUCCESS) ? ret : NULL;
 }
 
-void MemoryRegion::FreeKfdMemory(void* ptr, size_t size) {
+bool MemoryRegion::FreeKfdMemory(void* ptr, size_t size) {
   if (ptr == NULL || size == 0) {
-    return;
+    debug_print("Invalid free ptr:%p size:%lu\n", ptr, size);
+    return true;
   }
 
-  HSAKMT_STATUS status = hsaKmtFreeMemory(ptr, size);
-  assert(status == HSAKMT_STATUS_SUCCESS);
+  if (hsaKmtFreeMemory(ptr, size) != HSAKMT_STATUS_SUCCESS) {
+    debug_print("Failed to free ptr:%p size:%lu\n", ptr, size);
+    return false;
+  }
+  return true;
 }
 
 bool MemoryRegion::RegisterMemory(void* ptr, size_t size, const HsaMemFlags& MemFlags) {
@@ -97,8 +101,9 @@ bool MemoryRegion::MakeKfdMemoryResident(size_t num_node, const uint32_t* nodes,
   return (status == HSAKMT_STATUS_SUCCESS);
 }
 
-void MemoryRegion::MakeKfdMemoryUnresident(const void* ptr) {
-  hsaKmtUnmapMemoryToGPU(const_cast<void*>(ptr));
+bool MemoryRegion::MakeKfdMemoryUnresident(const void* ptr) {
+  const HSAKMT_STATUS status = hsaKmtUnmapMemoryToGPU(const_cast<void*>(ptr));
+  return (status == HSAKMT_STATUS_SUCCESS);
 }
 
 MemoryRegion::MemoryRegion(bool fine_grain, bool kernarg, bool full_profile,
@@ -311,9 +316,7 @@ hsa_status_t MemoryRegion::FreeImpl(void* address, size_t size) const {
 
   MakeKfdMemoryUnresident(address);
 
-  FreeKfdMemory(address, size);
-
-  return HSA_STATUS_SUCCESS;
+  return FreeKfdMemory(address, size) ? HSA_STATUS_SUCCESS : HSA_STATUS_ERROR;
 }
 
 // TODO:  Look into a better name and/or making this process transparent to exporting.

runtime/hsa-runtime/core/runtime/runtime.cpp

@@ -370,7 +370,53 @@ hsa_status_t Runtime::FreeMemory(void* ptr) {
   if (alloc_flags & core::MemoryRegion::AllocateAsan)
     assert(hsaKmtReturnAsanHeaderPage(ptr) == HSAKMT_STATUS_SUCCESS);
 
-  return region->Free(ptr, size);
+  const hsa_status_t err = region->Free(ptr, size);
+  if (err != HSA_STATUS_SUCCESS) {
+    // hsaKmtFreeMemory failed to free this pointer. Throw a memory error event
+
+    // Note: This should be treated as a fatal exception by the System Event Handler because:
+    //  - This leaves allocation_map_ in an inconsistent state as this pointer entry has already
+    //  been removed.
+    //  - We already called back the notifier, but did not actually free.
+    //  - We removed the ASAN Header but did not actually free.
+    //
+    // But this is a very unlikely use case and calling region->Free(..) before updating
+    // allocation_map_ would require us to hold the memory_lock_ for much longer and we would not be
+    // able to call hsaKmtReturnAsanHeaderPage after calling region->Free(..)
+
+    const core::Agent* agentOwner = region->owner();
+    hsa_status_t custom_handler_status = HSA_STATUS_ERROR;
+    auto system_event_handlers = runtime_singleton_->GetSystemEventHandlers();
+
+    if (!system_event_handlers.empty()) {
+      hsa_amd_event_t memory_error_event;
+      memory_error_event.event_type = HSA_AMD_GPU_MEMORY_ERROR_EVENT;
+      hsa_amd_gpu_memory_error_info_t& error_info = memory_error_event.memory_error;
+
+      error_info.virtual_address = reinterpret_cast<const uint64_t>(ptr);
+      error_info.error_reason_mask = HSA_AMD_MEMORY_ERROR_MEMORY_IN_USE;
+      error_info.agent = Agent::Convert(agentOwner);
+
+      for (auto& callback : system_event_handlers) {
+        hsa_status_t err = callback.first(&memory_error_event, callback.second);
+        if (err == HSA_STATUS_SUCCESS) custom_handler_status = HSA_STATUS_SUCCESS;
+      }
+    }
+    // No custom VM fault handler registered or it failed.
+    if (custom_handler_status != HSA_STATUS_SUCCESS) {
+      fprintf(stderr,
+              "Memory critical error by agent node-%u (Agent handle: %p) on address %p. Reason: "
+              "Memory in use. \n",
+              agentOwner->node_id(), reinterpret_cast<void*>(agentOwner->public_handle().handle),
+              ptr);
+
+      assert(false && "GPU memory error.");
+      std::abort();
+    }
+    return HSA_STATUS_ERROR;
+  }
+
+  return HSA_STATUS_SUCCESS;
 }
 
 hsa_status_t Runtime::RegisterReleaseNotifier(void* ptr, hsa_amd_deallocation_callback_t callback,