hsa_amd_agent_memory_pool_get_info gives wrong results for gfx803. Root cause: missing break point when querying the num hop attribute. Other change: max the reported num hop to 1 since the runtime does not have enough information about each hop, also clarified the comment about HSA_AMD_AGENT_MEMORY_POOL_INFO_NUM_LINK_HOPS attribute in the header file

Change-Id: I5d868eb457666e1377d5308f6145e76176bbfaf7

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

@@ -374,35 +374,41 @@ hsa_status_t MemoryRegion::GetAgentPoolInfo(
   const core::Runtime::LinkInfo link_info =
       core::Runtime::runtime_singleton_->GetLinkInfo(node_id_from, node_id_to);
 
+  /**
+   *  ---------------------------------------------------
+   *  |              |CPU        |GPU (owner)|GPU (peer) |
+   *  ---------------------------------------------------
+   *  |system memory |allowed    |disallowed |disallowed |
+   *  ---------------------------------------------------
+   *  |fb private    |never      |allowed    |never      |
+   *  ---------------------------------------------------
+   *  |fb public     |disallowed |allowed    |disallowed |
+   *  ---------------------------------------------------
+   *  |others        |never      |allowed    |never      |
+   *  ---------------------------------------------------
+   */
+  const hsa_amd_memory_pool_access_t access_type =
+      ((IsSystem() && (agent.device_type() == core::Agent::kAmdCpuDevice)) ||
+       (agent.node_id() == owner()->node_id()))
+          ? HSA_AMD_MEMORY_POOL_ACCESS_ALLOWED_BY_DEFAULT
+          : (IsSystem() || (IsPublic() && link_info.num_hop > 0))
+                ? HSA_AMD_MEMORY_POOL_ACCESS_DISALLOWED_BY_DEFAULT
+                : HSA_AMD_MEMORY_POOL_ACCESS_NEVER_ALLOWED;
+
   switch (attribute) {
     case HSA_AMD_AGENT_MEMORY_POOL_INFO_ACCESS:
-      /**
-      *  ---------------------------------------------------
-      *  |              |CPU        |GPU (owner)|GPU (peer) |
-      *  ---------------------------------------------------
-      *  |system memory |allowed    |disallowed |disallowed |
-      *  ---------------------------------------------------
-      *  |fb private    |never      |allowed    |never      |
-      *  ---------------------------------------------------
-      *  |fb public     |disallowed |allowed    |disallowed |
-      *  ---------------------------------------------------
-      *  |others        |never      |allowed    |never      |
-      *  ---------------------------------------------------
-      */
-      *((hsa_amd_memory_pool_access_t*)value) =
-          (((IsSystem()) &&
-            (agent.device_type() == core::Agent::kAmdCpuDevice)) ||
-           (agent.node_id() == owner()->node_id()))
-              ? HSA_AMD_MEMORY_POOL_ACCESS_ALLOWED_BY_DEFAULT
-              : (IsSystem() || (IsPublic() && link_info.num_hop > 0))
-                    ? HSA_AMD_MEMORY_POOL_ACCESS_DISALLOWED_BY_DEFAULT
-                    : HSA_AMD_MEMORY_POOL_ACCESS_NEVER_ALLOWED;
+      *((hsa_amd_memory_pool_access_t*)value) = access_type;
       break;
     case HSA_AMD_AGENT_MEMORY_POOL_INFO_NUM_LINK_HOPS:
-      *((uint32_t*)value) = link_info.num_hop;
+      *((uint32_t*)value) =
+          (access_type != HSA_AMD_MEMORY_POOL_ACCESS_NEVER_ALLOWED)
+              ? link_info.num_hop
+              : 0;
+      break;
     case HSA_AMD_AGENT_MEMORY_POOL_INFO_LINK_INFO:
       memset(value, 0, sizeof(hsa_amd_memory_pool_link_info_t));
-      if (link_info.num_hop > 0) {
+      if ((access_type != HSA_AMD_MEMORY_POOL_ACCESS_NEVER_ALLOWED) &&
+          (link_info.num_hop > 0)) {
         memcpy(value, &link_info.info, sizeof(hsa_amd_memory_pool_link_info_t));
       }
       break;

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

@@ -261,6 +261,10 @@ void Runtime::RegisterLinkInfo(uint32_t node_id_from, uint32_t node_id_to,
   const uint32_t idx = GetIndexLinkInfo(node_id_from, node_id_to);
   link_matrix_[idx].num_hop = num_hop;
   link_matrix_[idx].info = link_info;
+
+  // Limit the number of hop to 1 since the runtime does not have enough
+  // information to share to the user about each hop.
+  link_matrix_[idx].num_hop = std::min(link_matrix_[idx].num_hop , 1U);
 }
 
 const Runtime::LinkInfo Runtime::GetLinkInfo(uint32_t node_id_from,

runtime/hsa-runtime/inc/hsa_ext_amd.h

@@ -263,7 +263,7 @@ hsa_status_t HSA_API hsa_amd_profiling_get_dispatch_time(
 
 /**
  * @brief Computes the frequency ratio and offset between the agent clock and
- * HSA system clock and converts the agent’s tick to HSA system domain tick.
+ * HSA system clock and converts the agent's tick to HSA system domain tick.
  *
  * @param[in] agent The agent used to retrieve the agent_tick. It is user's
  * responsibility to make sure the tick number is from this agent, otherwise,
@@ -392,7 +392,7 @@ hsa_status_t HSA_API
  *
  * @details Allows waiting for any of several signal and conditions pairs to be
  * satisfied. The function returns the index into the list of signals of the
- * first satisfying signal-condition pair. The value of the satisfying signal’s
+ * first satisfying signal-condition pair. The value of the satisfying signal's
  * value is returned in satisfying_value unless satisfying_value is NULL. This
  * function provides only relaxed memory semantics.
  */
@@ -857,7 +857,10 @@ typedef enum {
 
   /**
   * Number of links to hop when accessing the memory pool from the specified
-  * agent. The type of this attribute is uint32_t.
+  * agent. The value of this attribute is zero if the memory pool is associated
+  * with the agent, or if the access type is
+  * HSA_AMD_MEMORY_POOL_ACCESS_NEVER_ALLOWED. The type of this attribute is
+  * uint32_t.
   */
   HSA_AMD_AGENT_MEMORY_POOL_INFO_NUM_LINK_HOPS = 1,