Skip to content

Commit

Permalink
[SWDEV-335697 + SWDEV-342812] Fix NPS & Compute tests
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@charis-poag-amd
charis-poag-amd committed Mar 2, 2023
1 parent fcb6afa commit c252ecc
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 55 deletions.
Binary file modified rocm_smi/docs/ROCm_SMI_Manual.pdf
View file
Binary file not shown.
8 changes: 4 additions & 4 deletions src/rocm_smi.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
49 changes: 19 additions & 30 deletions tests/rocm_smi_test/functional/computepartition_read_write.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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_) {
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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;
Expand All @@ -181,18 +185,7 @@ 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);
IF_VERB(STANDARD) {
if (err == RSMI_STATUS_NOT_SUPPORTED) {
std::cout << "\t**"
<< "Confirmed RSMI_STATUS_NOT_SUPPORTED 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));
ASSERT_EQ(err, RSMI_STATUS_INVALID_ARGS);
IF_VERB(STANDARD) {
if (err == RSMI_STATUS_INVALID_ARGS) {
std::cout << "\t**"
Expand All @@ -202,26 +195,22 @@ void TestComputePartitionReadWrite::Run(void) {
}

// 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) ||
err = rsmi_dev_compute_partition_get(dv_ind, orig_char_computePartition, 0);
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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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);
}
}
46 changes: 25 additions & 21 deletions tests/rocm_smi_test/functional/npsmode_read_write.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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;
Expand All @@ -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**"
Expand All @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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) {
Expand All @@ -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**"
Expand All @@ -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 */
Expand Down Expand Up @@ -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);
}
}

0 comments on commit c252ecc

Please sign in to comment.