| Message ID | 20210701185638.3437487-2-sean.anderson@seco.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] crypto: mxs-dcp: Check for DMA mapping errors | expand |
----- Ursprüngliche Mail ----- > Von: "Sean Anderson" <sean.anderson@seco.com> > An: "Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>, "Herbert Xu" <herbert@gondor.apana.org.au>, "davem" > <davem@davemloft.net> > CC: "horia geanta" <horia.geanta@nxp.com>, "aymen sghaier" <aymen.sghaier@nxp.com>, "richard" <richard@nod.at>, > "linux-arm-kernel" <linux-arm-kernel@lists.infradead.org>, "Marek Vasut" <marex@denx.de>, "Sean Anderson" > <sean.anderson@seco.com> > Gesendet: Donnerstag, 1. Juli 2021 20:56:37 > Betreff: [PATCH v2 1/2] crypto: mxs-dcp: Check for DMA mapping errors > After calling dma_map_single(), we must also call dma_mapping_error(). > This fixes the following warning when compiling with CONFIG_DMA_API_DEBUG: > > [ 311.241478] WARNING: CPU: 0 PID: 428 at kernel/dma/debug.c:1027 > check_unmap+0x79c/0x96c > [ 311.249547] DMA-API: mxs-dcp 2280000.crypto: device driver failed to check > map error[device address=0x00000000860cb080] [size=32 bytes] [mapped as single] > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > (no changes since v1) > > drivers/crypto/mxs-dcp.c | 45 +++++++++++++++++++++++++++++++--------- > 1 file changed, 35 insertions(+), 10 deletions(-) > > diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c > index d6a7784d2988..f397cc5bf102 100644 > --- a/drivers/crypto/mxs-dcp.c > +++ b/drivers/crypto/mxs-dcp.c > @@ -170,15 +170,19 @@ static struct dcp *global_sdcp; > > static int mxs_dcp_start_dma(struct dcp_async_ctx *actx) > { > + int dma_err; > struct dcp *sdcp = global_sdcp; > const int chan = actx->chan; > uint32_t stat; > unsigned long ret; > struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan]; > - > dma_addr_t desc_phys = dma_map_single(sdcp->dev, desc, sizeof(*desc), > DMA_TO_DEVICE); > > + dma_err = dma_mapping_error(sdcp->dev, desc_phys); > + if (dma_err) > + return dma_err; > + Minor nit, you don't need to propagate the return code from dma_mapping_error(). It just returns 0 for success and -ENOMEM on failure. So treating it as boolean function is fine, IMHO and keeps the code simpler. But it is a matter of taste. :-) > reinit_completion(&sdcp->completion[chan]); > > /* Clear status register. */ > @@ -216,18 +220,29 @@ static int mxs_dcp_start_dma(struct dcp_async_ctx *actx) > static int mxs_dcp_run_aes(struct dcp_async_ctx *actx, > struct skcipher_request *req, int init) > { > + dma_addr_t key_phys, src_phys, dst_phys; > struct dcp *sdcp = global_sdcp; > struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan]; > struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req); > int ret; > > - dma_addr_t key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key, > - 2 * AES_KEYSIZE_128, > - DMA_TO_DEVICE); > - dma_addr_t src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, > - DCP_BUF_SZ, DMA_TO_DEVICE); > - dma_addr_t dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, > - DCP_BUF_SZ, DMA_FROM_DEVICE); > + key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key, > + 2 * AES_KEYSIZE_128, DMA_TO_DEVICE); > + ret = dma_mapping_error(sdcp->dev, key_phys); > + if (ret) > + return ret; > + > + src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, > + DCP_BUF_SZ, DMA_TO_DEVICE); > + ret = dma_mapping_error(sdcp->dev, src_phys); > + if (ret) > + goto err_src; > + > + dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, > + DCP_BUF_SZ, DMA_FROM_DEVICE); > + ret = dma_mapping_error(sdcp->dev, dst_phys); > + if (ret) > + goto err_dst; > > if (actx->fill % AES_BLOCK_SIZE) { > dev_err(sdcp->dev, "Invalid block size!\n"); > @@ -265,10 +280,12 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx, > ret = mxs_dcp_start_dma(actx); > > aes_done_run: > + dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE); > +err_dst: > + dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE); > +err_src: > dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128, > DMA_TO_DEVICE); > - dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE); > - dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE); > > return ret; > } > @@ -557,6 +574,10 @@ static int mxs_dcp_run_sha(struct ahash_request *req) > dma_addr_t buf_phys = dma_map_single(sdcp->dev, sdcp->coh->sha_in_buf, > DCP_BUF_SZ, DMA_TO_DEVICE); > > + ret = dma_mapping_error(sdcp->dev, buf_phys); > + if (ret) > + return ret; > + > /* Fill in the DMA descriptor. */ > desc->control0 = MXS_DCP_CONTROL0_DECR_SEMAPHORE | > MXS_DCP_CONTROL0_INTERRUPT | > @@ -589,6 +610,10 @@ static int mxs_dcp_run_sha(struct ahash_request *req) > if (rctx->fini) { > digest_phys = dma_map_single(sdcp->dev, sdcp->coh->sha_out_buf, > DCP_SHA_PAY_SZ, DMA_FROM_DEVICE); > + ret = dma_mapping_error(sdcp->dev, digest_phys); > + if (ret) > + goto done_run; > + > desc->control0 |= MXS_DCP_CONTROL0_HASH_TERM; > desc->payload = digest_phys; > } Reviewed-by: Richard Weinberger <richard@nod.at> Thanks, //richard
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c index d6a7784d2988..f397cc5bf102 100644 --- a/drivers/crypto/mxs-dcp.c +++ b/drivers/crypto/mxs-dcp.c @@ -170,15 +170,19 @@ static struct dcp *global_sdcp; static int mxs_dcp_start_dma(struct dcp_async_ctx *actx) { + int dma_err; struct dcp *sdcp = global_sdcp; const int chan = actx->chan; uint32_t stat; unsigned long ret; struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan]; - dma_addr_t desc_phys = dma_map_single(sdcp->dev, desc, sizeof(*desc), DMA_TO_DEVICE); + dma_err = dma_mapping_error(sdcp->dev, desc_phys); + if (dma_err) + return dma_err; + reinit_completion(&sdcp->completion[chan]); /* Clear status register. */ @@ -216,18 +220,29 @@ static int mxs_dcp_start_dma(struct dcp_async_ctx *actx) static int mxs_dcp_run_aes(struct dcp_async_ctx *actx, struct skcipher_request *req, int init) { + dma_addr_t key_phys, src_phys, dst_phys; struct dcp *sdcp = global_sdcp; struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan]; struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req); int ret; - dma_addr_t key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key, - 2 * AES_KEYSIZE_128, - DMA_TO_DEVICE); - dma_addr_t src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, - DCP_BUF_SZ, DMA_TO_DEVICE); - dma_addr_t dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, - DCP_BUF_SZ, DMA_FROM_DEVICE); + key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key, + 2 * AES_KEYSIZE_128, DMA_TO_DEVICE); + ret = dma_mapping_error(sdcp->dev, key_phys); + if (ret) + return ret; + + src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, + DCP_BUF_SZ, DMA_TO_DEVICE); + ret = dma_mapping_error(sdcp->dev, src_phys); + if (ret) + goto err_src; + + dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, + DCP_BUF_SZ, DMA_FROM_DEVICE); + ret = dma_mapping_error(sdcp->dev, dst_phys); + if (ret) + goto err_dst; if (actx->fill % AES_BLOCK_SIZE) { dev_err(sdcp->dev, "Invalid block size!\n"); @@ -265,10 +280,12 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx, ret = mxs_dcp_start_dma(actx); aes_done_run: + dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE); +err_dst: + dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE); +err_src: dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128, DMA_TO_DEVICE); - dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE); - dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE); return ret; } @@ -557,6 +574,10 @@ static int mxs_dcp_run_sha(struct ahash_request *req) dma_addr_t buf_phys = dma_map_single(sdcp->dev, sdcp->coh->sha_in_buf, DCP_BUF_SZ, DMA_TO_DEVICE); + ret = dma_mapping_error(sdcp->dev, buf_phys); + if (ret) + return ret; + /* Fill in the DMA descriptor. */ desc->control0 = MXS_DCP_CONTROL0_DECR_SEMAPHORE | MXS_DCP_CONTROL0_INTERRUPT | @@ -589,6 +610,10 @@ static int mxs_dcp_run_sha(struct ahash_request *req) if (rctx->fini) { digest_phys = dma_map_single(sdcp->dev, sdcp->coh->sha_out_buf, DCP_SHA_PAY_SZ, DMA_FROM_DEVICE); + ret = dma_mapping_error(sdcp->dev, digest_phys); + if (ret) + goto done_run; + desc->control0 |= MXS_DCP_CONTROL0_HASH_TERM; desc->payload = digest_phys; }
After calling dma_map_single(), we must also call dma_mapping_error(). This fixes the following warning when compiling with CONFIG_DMA_API_DEBUG: [ 311.241478] WARNING: CPU: 0 PID: 428 at kernel/dma/debug.c:1027 check_unmap+0x79c/0x96c [ 311.249547] DMA-API: mxs-dcp 2280000.crypto: device driver failed to check map error[device address=0x00000000860cb080] [size=32 bytes] [mapped as single] Signed-off-by: Sean Anderson <sean.anderson@seco.com> --- (no changes since v1) drivers/crypto/mxs-dcp.c | 45 +++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 10 deletions(-)