Skip to contentSkip to conversation

crypto: qat - change size of status variable

PATCH In review Meadhbh Fitzpatrick 3 participants 4 messages Mar 9, 2023 – Mar 10, 2023 Crypto API

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 crypto: qat - change size of status variable

The status variable is of type int but it is used to store the output of an s8 type returned by the functions qat_comp_get_xlt_status() and qat_comp_get_cmp_status().

Declare status as s8 to match the output type of the two functions.

This commit does not implement any functional changes.

---
 drivers/crypto/qat/qat_common/qat_comp_algs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/crypto/qat/qat_common/qat_comp_algs.c +1 -1
index b533984906ec..726b71d2a4a8 100644
@@ -183,7 +183,7 @@ static void qat_comp_generic_callback(struct qat_compression_req *qat_req,
 	int consumed, produced;
 	s8 cmp_err, xlt_err;
 	int res = -EBADMSG;
-	int status;
+	s8 status;
 	u8 cnv;
 
 	status = qat_comp_get_cmp_status(resp);
Signature · 13 lines
2.37.3

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
maintainer
#1 lore raw

drivers/crypto/qat/qat_common/qat_comp_algs.c static void qat_comp_generic_callback(struct qat_compression_req *qat_req, · line 186

Quoted 11 lines · Meadhbh Fitzpatrick
drivers/crypto/qat/qat_common/qat_comp_algs.c +1 -1
index b533984906ec..726b71d2a4a8 100644
@@ -183,7 +183,7 @@ static void qat_comp_generic_callback(struct qat_compression_req *qat_req,
 	int consumed, produced;
 	s8 cmp_err, xlt_err;
 	int res = -EBADMSG;
-	int status;
+	s8 status;

Sorry, but this makes no sense. Why are these s8's to begin with? It doesn't look like you even allow negative values.

Cheers,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#2 lore raw
Quoted 15 lines · Herbert Xu

drivers/crypto/qat/qat_common/qat_comp_algs.c static void qat_comp_generic_callback(struct qat_compression_req *qat_req, · line 186

Quoted 11 lines · Meadhbh Fitzpatrick
drivers/crypto/qat/qat_common/qat_comp_algs.c +1 -1
index b533984906ec..726b71d2a4a8 100644
@@ -183,7 +183,7 @@ static void qat_comp_generic_callback(struct qat_compression_req *qat_req,
 	int consumed, produced;
 	s8 cmp_err, xlt_err;
 	int res = -EBADMSG;
-	int status;
+	s8 status;

Sorry, but this makes no sense. Why are these s8's to begin with? It doesn't look like you even allow negative values.

You are right. Thanks for spotting this.
When reviewing it I mixed up the cmp_err and xlt_err with the status which is a u8.

The functions qat_comp_get_cmp_status() and qat_comp_get_xlt_status() should be changed to return an u8 as the field returned by the firmware is an unsigned byte [1].

The status variable in qat_comp_generic_callback() can be changed to s8 or stay as int. It is just used to check if a value is set.

...

    if (unlikely(status != ICP_QAT_FW_COMN_STATUS_FLAG_OK))
            goto end;

...

where ICP_QAT_FW_COMN_STATUS_FLAG_OK is 0.

[1] https://elixir.bootlin.com/linux/v6.3-rc1/source/drivers/crypto/qat/qat_common/icp_qat_fw.h#L104

Regards,

Signature · 1 line
Giovanni
maintainer
#3 lore raw
Quoted 4 lines · Giovanni Cabiddu

You are right. Thanks for spotting this.
When reviewing it I mixed up the cmp_err and xlt_err with the status which is a u8.

In general s8/u8 should only be used when you are directly reading or writing from hardware. Once the value has entered your driver, you should use int/unsigned int to deal with them.

Thanks,

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt