From d2497bb2d3b46ceadb17a5374aca11ec9b1515c9 Mon Sep 17 00:00:00 2001
From: Charis Poag <Charis.Poag@amd.com>
Date: Thu, 2 Mar 2023 13:24:38 -0600
Subject: [PATCH] [SWDEV-335697 + SWDEV-342812] Fix NPS & Compute tests

Updates:
    * Fixed rsmi_dev_compute_partition_get
      & rsmi_dev_nps_mode_get to properly check
      for invalid arguments
    * Updated compute partition & NPS mode tests
      - Now properly confirms the invalid
        argument is seen
      - Spacing for multiple devices is added
        to better see distinction between
        separate device's tests (for verbose output)
      - Changed expect to assert calls, so errors
        are observed faster for test failures
      - Fixed multiple device testing where a
        variable should have been unset, but
        having multiple devices caused it to
        set
      - Updated multiple device testing to iterate
        accross all devices (previously returned,
        instead of continuing checking support
        after RSMI_STATUS_NOT_SUPPORTED detected)
      - Fixed a few spelling errors & verbose output

Change-Id: Ieba9e5b46763c6cd880fbf27fcdf58be8ecbc683
Signed-off-by: Charis Poag <Charis.Poag@amd.com>


[ROCm/amdsmi commit: c252ecccd1a0bc76a23675e9330c86ce24ff90f6]
---
 .../amdsmi/rocm_smi/docs/ROCm_SMI_Manual.pdf  | Bin 527272 -> 527272 bytes
 projects/amdsmi/src/rocm_smi.cc               |   8 +--
 .../functional/computepartition_read_write.cc |  51 +++++++-----------
 .../functional/npsmode_read_write.cc          |  46 ++++++++--------
 4 files changed, 49 insertions(+), 56 deletions(-)

diff --git a/projects/amdsmi/rocm_smi/docs/ROCm_SMI_Manual.pdf b/projects/amdsmi/rocm_smi/docs/ROCm_SMI_Manual.pdf
index 3a3ec7aa6f9428734840c6ed2459fafa04032f93..e3d19724581c408237ba25668d396c2279d7db5d 100644
GIT binary patch
delta 128
zcmZ26U17y^g@zW!7N!>F7M2#)7Pc+ymrrsU8yFcH8XKD$Prq}LT?)b7{{JNV5;kW?
v10xq_Qxgkw3rA;XHzOBgClgawXE$?G3u9MT0~a$p1sg(2wm-YTe#i;{v>YWN

delta 128
zcmZ26U17y^g@zW!7N!>F7M2#)7Pc+ymrrsU85tOw8W<TGOuuuIT?)b7{{JNV5;kWS
v17k}gQ!`6rBS&LLLsvs5LuW?|V@C^917k-sBPS<21sg(2wm-YTe#i;{kNYH7

diff --git a/projects/amdsmi/src/rocm_smi.cc b/projects/amdsmi/src/rocm_smi.cc
index 19e4724fe5..96073462dd 100755
--- a/projects/amdsmi/src/rocm_smi.cc
+++ b/projects/amdsmi/src/rocm_smi.cc
@@ -3791,12 +3791,12 @@ get_compute_partition(uint32_t dv_ind, std::string &compute_partition) {
 rsmi_status_t
 rsmi_dev_compute_partition_get(uint32_t dv_ind, char *compute_partition,
                                uint32_t len) {
-  CHK_SUPPORT_NAME_ONLY(compute_partition)
+  TRY
   if ((len == 0) || (compute_partition == nullptr)) {
     return RSMI_STATUS_INVALID_ARGS;
   }
+  CHK_SUPPORT_NAME_ONLY(compute_partition)
 
-  TRY
   std::string returning_compute_partition;
   rsmi_status_t ret = get_compute_partition(dv_ind,
                                returning_compute_partition);
@@ -3996,12 +3996,12 @@ rsmi_dev_nps_mode_set(uint32_t dv_ind, rsmi_nps_mode_type_t nps_mode) {
 rsmi_status_t
 rsmi_dev_nps_mode_get(uint32_t dv_ind, char *nps_mode,
                                uint32_t len) {
-  CHK_SUPPORT_NAME_ONLY(nps_mode)
+  TRY
   if ((len == 0) || (nps_mode == nullptr)) {
     return RSMI_STATUS_INVALID_ARGS;
   }
+  CHK_SUPPORT_NAME_ONLY(nps_mode)
 
-  TRY
   std::string returning_nps_mode;
   rsmi_status_t ret = get_nps_mode(dv_ind,
                                returning_nps_mode);
diff --git a/projects/amdsmi/tests/rocm_smi_test/functional/computepartition_read_write.cc b/projects/amdsmi/tests/rocm_smi_test/functional/computepartition_read_write.cc
index 1263669cb8..691a18aae6 100755
--- a/projects/amdsmi/tests/rocm_smi_test/functional/computepartition_read_write.cc
+++ b/projects/amdsmi/tests/rocm_smi_test/functional/computepartition_read_write.cc
@@ -55,8 +55,8 @@
 
 TestComputePartitionReadWrite::TestComputePartitionReadWrite() : TestBase() {
   set_title("RSMI Compute Partition Read/Write Test");
-  set_description("The Compute Parition tests verifies that the compute "
-                  "parition can be read and updated properly.");
+  set_description("The Compute Partition tests verifies that the compute "
+                  "partition can be read and updated properly.");
 }
 
 TestComputePartitionReadWrite::~TestComputePartitionReadWrite(void) {
@@ -127,7 +127,6 @@ void TestComputePartitionReadWrite::Run(void) {
   rsmi_status_t ret, err;
   char orig_char_computePartition[255];
   char current_char_computePartition[255];
-  rsmi_compute_partition_type new_computePartition;
 
   TestBase::Run();
   if (setup_failed_) {
@@ -136,6 +135,11 @@ void TestComputePartitionReadWrite::Run(void) {
   }
 
   for (uint32_t dv_ind = 0; dv_ind < num_monitor_devs(); ++dv_ind) {
+    if (dv_ind != 0) {
+      IF_VERB(STANDARD) {
+        std::cout << std::endl;
+      }
+    }
     PrintDeviceHeader(dv_ind);
 
     //Standard checks to see if API is supported, before running full tests
@@ -144,9 +148,9 @@ void TestComputePartitionReadWrite::Run(void) {
     if (ret == RSMI_STATUS_NOT_SUPPORTED) {
        IF_VERB(STANDARD) {
           std::cout << "\t**" <<  ": "
-                    << "Not supported on this machine" << std::endl;
+                    << "Not supported on this device" << std::endl;
         }
-        return;
+        continue;
     } else {
         CHK_ERR_ASRT(ret)
     }
@@ -160,9 +164,9 @@ void TestComputePartitionReadWrite::Run(void) {
        (orig_char_computePartition[0] == '\0')) {
       std::cout << "***System compute partition value is not defined. "
                   "Skip compute partition test." << std::endl;
-      return;
+      continue;
     }
-    EXPECT_EQ(RSMI_STATUS_SUCCESS, ret);
+    ASSERT_EQ(RSMI_STATUS_SUCCESS, ret);
 
     // Verify api support checking functionality is working
     uint32_t length = 2;
@@ -181,47 +185,32 @@ void TestComputePartitionReadWrite::Run(void) {
 
     // Verify api support checking functionality is working
     err = rsmi_dev_compute_partition_get(dv_ind, nullptr, 255);
-    ASSERT_EQ(err, RSMI_STATUS_NOT_SUPPORTED);
+    ASSERT_EQ(err, RSMI_STATUS_INVALID_ARGS);
     IF_VERB(STANDARD) {
-      if (err == RSMI_STATUS_NOT_SUPPORTED) {
+      if (err == RSMI_STATUS_INVALID_ARGS) {
         std::cout << "\t**"
-                  << "Confirmed RSMI_STATUS_NOT_SUPPORTED was returned."
+                  << "Confirmed RSMI_STATUS_INVALID_ARGS was returned."
                   << std::endl;
       }
     }
 
     // Verify api support checking functionality is working
     err = rsmi_dev_compute_partition_get(dv_ind, orig_char_computePartition, 0);
-    ASSERT_EQ(err, (RSMI_STATUS_INVALID_ARGS || RSMI_STATUS_NOT_SUPPORTED));
-    IF_VERB(STANDARD) {
-      if (err == RSMI_STATUS_INVALID_ARGS) {
-        std::cout << "\t**"
-                  << "Confirmed RSMI_STATUS_INVALID_ARGS was returned."
-                  << std::endl;
-      }
-    }
-
-    // Verify api support checking functionality is working
-    err = rsmi_dev_compute_partition_set(dv_ind, new_computePartition);
-    // Note: new_computePartition is not set
-    EXPECT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
+    ASSERT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
                 (err == RSMI_STATUS_NOT_SUPPORTED));
     IF_VERB(STANDARD) {
       if (err == RSMI_STATUS_INVALID_ARGS) {
         std::cout << "\t**"
                   << "Confirmed RSMI_STATUS_INVALID_ARGS was returned."
                   << std::endl;
-      } else {
-        DISPLAY_RSMI_ERR(err)
       }
     }
-    ASSERT_FALSE(err == RSMI_STATUS_PERMISSION);
 
     // Verify api support checking functionality is working
-    new_computePartition
+    rsmi_compute_partition_type new_computePartition
       = rsmi_compute_partition_type::RSMI_COMPUTE_PARTITION_INVALID;
     err = rsmi_dev_compute_partition_set(dv_ind, new_computePartition);
-    EXPECT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
+    ASSERT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
                 (err == RSMI_STATUS_NOT_SUPPORTED) ||
                 (err == RSMI_STATUS_PERMISSION));
     IF_VERB(STANDARD) {
@@ -241,7 +230,7 @@ void TestComputePartitionReadWrite::Run(void) {
     // Re-run original get, so we can reset to later
     ret = rsmi_dev_compute_partition_get(dv_ind, orig_char_computePartition,
                                           255);
-    EXPECT_EQ(RSMI_STATUS_SUCCESS, ret);
+    ASSERT_EQ(RSMI_STATUS_SUCCESS, ret);
 
     /**
      * RSMI_COMPUTE_PARTITION_INVALID = 0,
@@ -384,8 +373,8 @@ void TestComputePartitionReadWrite::Run(void) {
                 << current_char_computePartition
                 << std::endl;
     }
-    EXPECT_EQ(RSMI_STATUS_SUCCESS, ret);
-    EXPECT_STREQ(computePartitionString(new_computePartition).c_str(),
+    ASSERT_EQ(RSMI_STATUS_SUCCESS, ret);
+    ASSERT_STREQ(computePartitionString(new_computePartition).c_str(),
                  current_char_computePartition);
   }
 }
diff --git a/projects/amdsmi/tests/rocm_smi_test/functional/npsmode_read_write.cc b/projects/amdsmi/tests/rocm_smi_test/functional/npsmode_read_write.cc
index f5bcabf048..67296ffbba 100755
--- a/projects/amdsmi/tests/rocm_smi_test/functional/npsmode_read_write.cc
+++ b/projects/amdsmi/tests/rocm_smi_test/functional/npsmode_read_write.cc
@@ -113,7 +113,6 @@ void TestNPSModeReadWrite::Run(void) {
   char current_nps_mode[255];
   orig_nps_mode[0] = '\0';
   current_nps_mode[0] = '\0';
-  rsmi_nps_mode_type new_nps_mode;
 
   TestBase::Run();
   if (setup_failed_) {
@@ -122,6 +121,11 @@ void TestNPSModeReadWrite::Run(void) {
   }
 
   for (uint32_t dv_ind = 0; dv_ind < num_monitor_devs(); ++dv_ind) {
+    if (dv_ind != 0) {
+      IF_VERB(STANDARD) {
+        std::cout << std::endl;
+      }
+    }
     PrintDeviceHeader(dv_ind);
 
     //Standard checks to see if API is supported, before running full tests
@@ -131,7 +135,7 @@ void TestNPSModeReadWrite::Run(void) {
           std::cout << "\t**" <<  ": "
                     << "Not supported on this machine" << std::endl;
         }
-        return;
+        continue;
     } else {
         CHK_ERR_ASRT(ret)
     }
@@ -145,9 +149,9 @@ void TestNPSModeReadWrite::Run(void) {
        (orig_nps_mode[0] == '\0')) {
       std::cout << "***System nps mode value is not defined or received unexpected data. "
                   "Skip nps mode test." << std::endl;
-      return;
+      continue;
     }
-    EXPECT_TRUE(ret == RSMI_STATUS_SUCCESS);
+    ASSERT_TRUE(ret == RSMI_STATUS_SUCCESS);
 
     // Verify api support checking functionality is working
     uint32_t length = 2;
@@ -166,19 +170,20 @@ void TestNPSModeReadWrite::Run(void) {
 
     // Verify api support checking functionality is working
     err = rsmi_dev_nps_mode_get(dv_ind, nullptr, 255);
-    ASSERT_EQ(err, RSMI_STATUS_NOT_SUPPORTED);
+    ASSERT_EQ(err, RSMI_STATUS_INVALID_ARGS);
 
-    if (err == RSMI_STATUS_NOT_SUPPORTED) {
+    if (err == RSMI_STATUS_INVALID_ARGS) {
       IF_VERB(STANDARD) {
         std::cout << "\t**"
-                  << "Confirmed RSMI_STATUS_NOT_SUPPORTED was returned."
+                  << "Confirmed RSMI_STATUS_INVALID_ARGS was returned."
                   << std::endl;
       }
     }
 
     // Verify api support checking functionality is working
     err = rsmi_dev_nps_mode_get(dv_ind, orig_nps_mode, 0);
-    ASSERT_EQ(err, (RSMI_STATUS_INVALID_ARGS || RSMI_STATUS_NOT_SUPPORTED));
+    ASSERT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
+                (err == RSMI_STATUS_NOT_SUPPORTED));
     if (err == RSMI_STATUS_INVALID_ARGS) {
       IF_VERB(STANDARD) {
         std::cout << "\t**"
@@ -191,9 +196,10 @@ void TestNPSModeReadWrite::Run(void) {
     /* rsmi_dev_nps_mode_set(...) */
     /******************************/
     // Verify api support checking functionality is working
+    rsmi_nps_mode_type new_nps_mode;
     err = rsmi_dev_nps_mode_set(dv_ind, new_nps_mode);
     // Note: new_nps_mode is not set
-    EXPECT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
+    ASSERT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
                 (err == RSMI_STATUS_NOT_SUPPORTED));
     if (err == RSMI_STATUS_INVALID_ARGS) {
       IF_VERB(STANDARD) {
@@ -204,12 +210,12 @@ void TestNPSModeReadWrite::Run(void) {
     } else if (err == RSMI_STATUS_NOT_SUPPORTED) {
          IF_VERB(STANDARD) {
           std::cout << "\t**" <<  ": "
-                    << "rsmi_dev_nps_mode_set not supported on this machine"
-                    << "\n\t    (if rsmi_dev_nps_mode_get work, then likely "
+                    << "rsmi_dev_nps_mode_set not supported on this device"
+                    << "\n\t    (if rsmi_dev_nps_mode_get works, then likely "
                     << "need to set in bios)"
                     << std::endl;
         }
-        return;
+        continue;
     } else {
         DISPLAY_RSMI_ERR(err)
     }
@@ -218,7 +224,7 @@ void TestNPSModeReadWrite::Run(void) {
     // Verify api support checking functionality is working
     new_nps_mode = rsmi_nps_mode_type::RSMI_MEMORY_PARTITION_UNKNOWN;
     err = rsmi_dev_nps_mode_set(dv_ind, new_nps_mode);
-    EXPECT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
+    ASSERT_TRUE((err == RSMI_STATUS_INVALID_ARGS) ||
                 (err == RSMI_STATUS_NOT_SUPPORTED) ||
                 (err == RSMI_STATUS_PERMISSION));
     if (err == RSMI_STATUS_INVALID_ARGS) {
@@ -237,13 +243,12 @@ void TestNPSModeReadWrite::Run(void) {
 
     // Re-run original get, so we can reset to later
     ret = rsmi_dev_nps_mode_get(dv_ind, orig_nps_mode, 255);
-    EXPECT_EQ(RSMI_STATUS_SUCCESS, ret);
+    ASSERT_EQ(RSMI_STATUS_SUCCESS, ret);
 
     for (int partition = RSMI_MEMORY_PARTITION_NPS1;
          partition <= RSMI_MEMORY_PARTITION_NPS8;
          partition++) {
-      new_nps_mode
-        = static_cast<rsmi_nps_mode_type>(partition);
+      new_nps_mode = static_cast<rsmi_nps_mode_type>(partition);
       IF_VERB(STANDARD) {
         std::cout << std::endl;
         std::cout << "\t**"
@@ -265,8 +270,8 @@ void TestNPSModeReadWrite::Run(void) {
         std::cout << "\t**"
                   << "Current nps mode: " << current_nps_mode << std::endl;
       }
-      EXPECT_EQ(RSMI_STATUS_SUCCESS, ret);
-      EXPECT_STREQ(npsModeString(new_nps_mode).c_str(), current_nps_mode);
+      ASSERT_EQ(RSMI_STATUS_SUCCESS, ret);
+      ASSERT_STREQ(npsModeString(new_nps_mode).c_str(), current_nps_mode);
     }
 
     /* TEST RETURN TO BOOT NPS MODE SETTING */
@@ -331,8 +336,7 @@ void TestNPSModeReadWrite::Run(void) {
                 << "\t**" << "Current nps mode: " << current_nps_mode
                 << std::endl;
     }
-    EXPECT_EQ(RSMI_STATUS_SUCCESS, ret);
-    EXPECT_STREQ(npsModeString(new_nps_mode).c_str(), current_nps_mode);
-
+    ASSERT_EQ(RSMI_STATUS_SUCCESS, ret);
+    ASSERT_STREQ(npsModeString(new_nps_mode).c_str(), current_nps_mode);
   }
 }