| Message ID | 20210225182716.1402449-2-thara.gopinath@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for AEAD algorithms in Qualcomm Crypto Engine driver | expand |
On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote: > MAC_FAILED gets set in the status register if authenthication fails > for ccm algorithms(during decryption). Add support to catch and flag > this error. > > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > --- > drivers/crypto/qce/common.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c > index dceb9579d87a..7c3cb483749e 100644 > --- a/drivers/crypto/qce/common.c > +++ b/drivers/crypto/qce/common.c > @@ -403,7 +403,8 @@ int qce_start(struct crypto_async_request *async_req, u32 type) > } > > #define STATUS_ERRORS \ > - (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | BIT(HSD_ERR_SHIFT)) > + (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | \ > + BIT(HSD_ERR_SHIFT) | BIT(MAC_FAILED_SHIFT)) > > int qce_check_status(struct qce_device *qce, u32 *status) > { > @@ -417,8 +418,12 @@ int qce_check_status(struct qce_device *qce, u32 *status) > * use result_status from result dump the result_status needs to be byte > * swapped, since we set the device to little endian. > */ > - if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) > - ret = -ENXIO; > + if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) { > + if (*status & BIT(MAC_FAILED_SHIFT)) Afaict MAC_FAILED indicates a different category of errors from the others. So I would prefer that the conditionals are flattened. Is OPERATION_DONE set when MAC_FAILED? If so: if (errors || !done) return -ENXIO; else if (*status & BIT(MAC_FAILED)) return -EBADMSG; Would be cleaner in my opinion. Regards, Bjorn > + ret = -EBADMSG; > + else > + ret = -ENXIO; > + } > > return ret; > } > -- > 2.25.1 >
Hi Bjorn, Thanks for the reviews. I realized that I had these replies in my draft for a while and forgot to send them! On 4/5/21 1:36 PM, Bjorn Andersson wrote: > On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote: > >> MAC_FAILED gets set in the status register if authenthication fails >> for ccm algorithms(during decryption). Add support to catch and flag >> this error. >> >> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >> --- >> drivers/crypto/qce/common.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c >> index dceb9579d87a..7c3cb483749e 100644 >> --- a/drivers/crypto/qce/common.c >> +++ b/drivers/crypto/qce/common.c >> @@ -403,7 +403,8 @@ int qce_start(struct crypto_async_request *async_req, u32 type) >> } >> >> #define STATUS_ERRORS \ >> - (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | BIT(HSD_ERR_SHIFT)) >> + (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | \ >> + BIT(HSD_ERR_SHIFT) | BIT(MAC_FAILED_SHIFT)) >> >> int qce_check_status(struct qce_device *qce, u32 *status) >> { >> @@ -417,8 +418,12 @@ int qce_check_status(struct qce_device *qce, u32 *status) >> * use result_status from result dump the result_status needs to be byte >> * swapped, since we set the device to little endian. >> */ >> - if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) >> - ret = -ENXIO; >> + if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) { >> + if (*status & BIT(MAC_FAILED_SHIFT)) > > Afaict MAC_FAILED indicates a different category of errors from the > others. So I would prefer that the conditionals are flattened. > > Is OPERATION_DONE set when MAC_FAILED? Yes it is. I will change the check to the pattern you have suggested. It is less confusing.. > > If so: > > if (errors || !done) > return -ENXIO; > else if (*status & BIT(MAC_FAILED)) > return -EBADMSG; > > Would be cleaner in my opinion. > > Regards, > Bjorn > >> + ret = -EBADMSG; >> + else >> + ret = -ENXIO; >> + } >> >> return ret; >> } >> -- >> 2.25.1 >> -- Warm Regards Thara
diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c index dceb9579d87a..7c3cb483749e 100644 --- a/drivers/crypto/qce/common.c +++ b/drivers/crypto/qce/common.c @@ -403,7 +403,8 @@ int qce_start(struct crypto_async_request *async_req, u32 type) } #define STATUS_ERRORS \ - (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | BIT(HSD_ERR_SHIFT)) + (BIT(SW_ERR_SHIFT) | BIT(AXI_ERR_SHIFT) | \ + BIT(HSD_ERR_SHIFT) | BIT(MAC_FAILED_SHIFT)) int qce_check_status(struct qce_device *qce, u32 *status) { @@ -417,8 +418,12 @@ int qce_check_status(struct qce_device *qce, u32 *status) * use result_status from result dump the result_status needs to be byte * swapped, since we set the device to little endian. */ - if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) - ret = -ENXIO; + if (*status & STATUS_ERRORS || !(*status & BIT(OPERATION_DONE_SHIFT))) { + if (*status & BIT(MAC_FAILED_SHIFT)) + ret = -EBADMSG; + else + ret = -ENXIO; + } return ret; }
MAC_FAILED gets set in the status register if authenthication fails for ccm algorithms(during decryption). Add support to catch and flag this error. Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- drivers/crypto/qce/common.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) -- 2.25.1