Skip to contentSkip to conversation

crypto: qat - fix some timeout tests

PATCH In review Dan Carpenter 2 participants 3 messages Dec 15, 2015

Subsystems (2)
Kernel janitors Crypto API
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 crypto: qat - fix some timeout tests

We do an "if (!times)" test later to see if we ran the loop too many times, but because it is a postop negate that means times is -1 when we exit that way. I have fixed this by changing it from a post op to a preop.

Fixes b3416fb8a2f5 ('crypto: qat - Intel(R) QAT accelengine part of fw loader')
Signed-off-by Dan Carpenter <[email protected]>

---
This means we only loop 9999 times instead of 10000 like before but I figure it probably doesn't matter.

drivers/crypto/qat/qat_common/qat_hal.c +1 -1
index 45c1739..2b078a6 100644
@@ -171,7 +171,7 @@ static int qat_hal_wait_cycles(struct icp_qat_fw_loader_handle *handle,
 
 	qat_hal_rd_ae_csr(handle, ae, PROFILE_COUNT, &base_cnt);
 	base_cnt &= 0xffff;
-	while ((int)cycles > elapsed_cycles && times--) {
+	while ((int)cycles > elapsed_cycles && --times) {
 		if (chk_inactive)
 			qat_hal_rd_ae_csr(handle, ae, ACTIVE_CTX_STATUS, &csr);
#1 lore raw
Quoted 17 lines · Dan Carpenter

We do an "if (!times)" test later to see if we ran the loop too many times, but because it is a postop negate that means times is -1 when we exit that way. I have fixed this by changing it from a post op to a preop.

Fixes b3416fb8a2f5 ('crypto: qat - Intel(R) QAT accelengine part of fw loader')
Signed-off-by Dan Carpenter <[email protected]>

---
This means we only loop 9999 times instead of 10000 like before but I figure it probably doesn't matter.

drivers/crypto/qat/qat_common/qat_hal.c +0 -0
index 45c1739..2b078a6 100644

drivers/crypto/qat/qat_common/qat_hal.c static int qat_hal_wait_cycles(struct icp_qat_fw_loader_handle *handle, · line 177

Quoted 8 lines
drivers/crypto/qat/qat_common/qat_hal.c +1 -1
@@ -171,7 +171,7 @@ static int qat_hal_wait_cycles(struct icp_qat_fw_loader_handle *handle,
 
 	qat_hal_rd_ae_csr(handle, ae, PROFILE_COUNT, &base_cnt);
 	base_cnt &= 0xffff;
-	while ((int)cycles > elapsed_cycles && times--) {
+	while ((int)cycles > elapsed_cycles && --times) {
 		if (chk_inactive)
 			qat_hal_rd_ae_csr(handle, ae, ACTIVE_CTX_STATUS, &csr);

Since the times var is an signed int I think we should rather change the condition:

drivers/crypto/qat/qat_common/qat_hal.c +1 -1
index 45c1739..864a5ea 100644
@@ -186,7 +186,7 @@ static int qat_hal_wait_cycles(struct icp_qat_fw_loader_handle *handle,
 		if (elapsed_cycles >= 8 && !(csr & (1 << ACS_ABO_BITPOS)))
 			return 0;
 	}
-	if (!times) {
+	if (!(0 < times)) {
 		pr_err("QAT: wait_num_cycles time out\n");
 		return -EFAULT;
 	}

Pingchao could you test and send a patch please.
Thanks

Signature · 1 line
TS
author
#2 lore raw
Quoted 1 line · Tadeusz Struk

Since the times var is an signed int I think we should rather change the condition:

I don't see what signed int has to do with anything.

drivers/crypto/qat/qat_common/qat_hal.c static int qat_hal_wait_cycles(struct icp_qat_fw_loader_handle *handle, · line 189

Quoted 11 lines
drivers/crypto/qat/qat_common/qat_hal.c +1 -1
index 45c1739..864a5ea 100644
@@ -186,7 +186,7 @@ static int qat_hal_wait_cycles(struct icp_qat_fw_loader_handle *handle,
 		if (elapsed_cycles >= 8 && !(csr & (1 << ACS_ABO_BITPOS)))
 			return 0;
 	}
-	if (!times) {
+	if (!(0 < times)) {

Oh, wow that's nasty! Why would you write it in such an obfuscated way? Plus it's buggy and wrong... What on earth.

regards,
dan carpenter