libhsakmt: Remove redundant checks, small bug fixes
Some g_system and Node ID checks already performed by validate_nodeid().
Fixes bug where hsaKmtGetNodeProperties() would return on
validate_nodeid() error without releasing lock. Switches some
conditionals to return INVALID_NODE_UNIT instead of INVALID_PARAMETER if
NodeId >= NumNodes. Removes some g_system assertions to return error
code rather than trigger a hard fault.
Signed-off-by: Graham Sider <Graham.Sider@amd.com>
Change-Id: Id1604b20c2cef8808b98cdad61bd47aa7ea3d229
[ROCm/ROCR-Runtime commit: 29369753f5]
This commit is contained in:
@@ -2036,21 +2036,9 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeProperties(HSAuint32 NodeId,
|
|||||||
CHECK_KFD_OPEN();
|
CHECK_KFD_OPEN();
|
||||||
pthread_mutex_lock(&hsakmt_mutex);
|
pthread_mutex_lock(&hsakmt_mutex);
|
||||||
|
|
||||||
/* KFD ADD page 18, snapshot protocol violation */
|
|
||||||
if (!g_system) {
|
|
||||||
err = HSAKMT_STATUS_INVALID_NODE_UNIT;
|
|
||||||
assert(g_system);
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (NodeId >= g_system->NumNodes) {
|
|
||||||
err = HSAKMT_STATUS_INVALID_PARAMETER;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
err = validate_nodeid(NodeId, &gpu_id);
|
err = validate_nodeid(NodeId, &gpu_id);
|
||||||
if (err != HSAKMT_STATUS_SUCCESS)
|
if (err != HSAKMT_STATUS_SUCCESS)
|
||||||
return err;
|
goto out;
|
||||||
|
|
||||||
*NodeProperties = g_props[NodeId].node;
|
*NodeProperties = g_props[NodeId].node;
|
||||||
/* For CPU only node don't add any additional GPU memory banks. */
|
/* For CPU only node don't add any additional GPU memory banks. */
|
||||||
@@ -2085,19 +2073,6 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeMemoryProperties(HSAuint32 NodeId,
|
|||||||
CHECK_KFD_OPEN();
|
CHECK_KFD_OPEN();
|
||||||
pthread_mutex_lock(&hsakmt_mutex);
|
pthread_mutex_lock(&hsakmt_mutex);
|
||||||
|
|
||||||
/* KFD ADD page 18, snapshot protocol violation */
|
|
||||||
if (!g_system) {
|
|
||||||
err = HSAKMT_STATUS_INVALID_NODE_UNIT;
|
|
||||||
assert(g_system);
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
/* Check still necessary */
|
|
||||||
if (NodeId >= g_system->NumNodes) {
|
|
||||||
err = HSAKMT_STATUS_INVALID_PARAMETER;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
err = validate_nodeid(NodeId, &gpu_id);
|
err = validate_nodeid(NodeId, &gpu_id);
|
||||||
if (err != HSAKMT_STATUS_SUCCESS)
|
if (err != HSAKMT_STATUS_SUCCESS)
|
||||||
goto out;
|
goto out;
|
||||||
@@ -2183,13 +2158,12 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeCacheProperties(HSAuint32 NodeId,
|
|||||||
pthread_mutex_lock(&hsakmt_mutex);
|
pthread_mutex_lock(&hsakmt_mutex);
|
||||||
|
|
||||||
/* KFD ADD page 18, snapshot protocol violation */
|
/* KFD ADD page 18, snapshot protocol violation */
|
||||||
if (!g_system) {
|
if (!g_system || NodeId >= g_system->NumNodes) {
|
||||||
err = HSAKMT_STATUS_INVALID_NODE_UNIT;
|
err = HSAKMT_STATUS_INVALID_NODE_UNIT;
|
||||||
assert(g_system);
|
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (NodeId >= g_system->NumNodes || NumCaches > g_props[NodeId].node.NumCaches) {
|
if (NumCaches > g_props[NodeId].node.NumCaches) {
|
||||||
err = HSAKMT_STATUS_INVALID_PARAMETER;
|
err = HSAKMT_STATUS_INVALID_PARAMETER;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
@@ -2221,13 +2195,12 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtGetNodeIoLinkProperties(HSAuint32 NodeId,
|
|||||||
pthread_mutex_lock(&hsakmt_mutex);
|
pthread_mutex_lock(&hsakmt_mutex);
|
||||||
|
|
||||||
/* KFD ADD page 18, snapshot protocol violation */
|
/* KFD ADD page 18, snapshot protocol violation */
|
||||||
if (!g_system) {
|
if (!g_system || NodeId >= g_system->NumNodes ) {
|
||||||
err = HSAKMT_STATUS_INVALID_NODE_UNIT;
|
err = HSAKMT_STATUS_INVALID_NODE_UNIT;
|
||||||
assert(g_system);
|
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (NodeId >= g_system->NumNodes || NumIoLinks > g_props[NodeId].node.NumIOLinks) {
|
if (NumIoLinks > g_props[NodeId].node.NumIOLinks) {
|
||||||
err = HSAKMT_STATUS_INVALID_PARAMETER;
|
err = HSAKMT_STATUS_INVALID_PARAMETER;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -90,7 +90,7 @@ TEST_F(KFDTopologyTest, GetNodePropertiesInvalidNodeNum) {
|
|||||||
|
|
||||||
HsaNodeProperties nodeProperties;
|
HsaNodeProperties nodeProperties;
|
||||||
memset(&nodeProperties, 0, sizeof(nodeProperties));
|
memset(&nodeProperties, 0, sizeof(nodeProperties));
|
||||||
EXPECT_EQ(HSAKMT_STATUS_INVALID_PARAMETER, hsaKmtGetNodeProperties(m_SystemProperties.NumNodes, &nodeProperties));
|
EXPECT_EQ(HSAKMT_STATUS_INVALID_NODE_UNIT, hsaKmtGetNodeProperties(m_SystemProperties.NumNodes, &nodeProperties));
|
||||||
|
|
||||||
TEST_END
|
TEST_END
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user