diff mbox series

[2/4] crypto: sun8i-ce - wrap accesses to descriptor address fields

Message ID 20240616220719.26641-3-andre.przywara@arm.com
State Superseded
Headers show
Series crypto: sun8i-ce: add Allwinner H616 support | expand

Commit Message

Andre Przywara June 16, 2024, 10:07 p.m. UTC
The Allwinner H616 (and later) SoCs support more than 32 bits worth of
physical addresses. To accommodate the larger address space, the CE task
descriptor fields holding addresses are now encoded as "word addresses",
so take the actual address divided by four.
This is true for the fields within the descriptor, but also for the
descriptor base address, in the CE_TDA register.

Wrap all accesses to those fields in a function, which will do the
required division if needed. For now this in unused, so there should be
no change in behaviour.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c |  8 ++++----
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c   |  3 ++-
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c   |  6 +++---
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c   |  6 +++---
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c   |  2 +-
 drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h        | 10 ++++++++++
 6 files changed, 23 insertions(+), 12 deletions(-)

Comments

kernel test robot June 18, 2024, 7:39 a.m. UTC | #1
Hi Andre,

kernel test robot noticed the following build warnings:

[auto build test WARNING on sunxi/sunxi/for-next]
[also build test WARNING on herbert-cryptodev-2.6/master herbert-crypto-2.6/master linus/master v6.10-rc4 next-20240617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andre-Przywara/dt-bindings-crypto-sun8i-ce-Add-compatible-for-H616/20240617-061144
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
patch link:    https://lore.kernel.org/r/20240616220719.26641-3-andre.przywara%40arm.com
patch subject: [PATCH 2/4] crypto: sun8i-ce - wrap accesses to descriptor address fields
config: loongarch-randconfig-r111-20240618 (https://download.01.org/0day-ci/archive/20240618/202406181436.RZPPffYb-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240618/202406181436.RZPPffYb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406181436.RZPPffYb-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:175:34: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] value @@     got restricted __le32 @@
   drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:175:34: sparse:     expected unsigned int [usertype] value
   drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:175:34: sparse:     got restricted __le32

vim +175 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c

   167	
   168		mutex_lock(&ce->mlock);
   169	
   170		v = readl(ce->base + CE_ICR);
   171		v |= 1 << flow;
   172		writel(v, ce->base + CE_ICR);
   173	
   174		reinit_completion(&ce->chanlist[flow].complete);
 > 175		writel(sun8i_ce_desc_addr(ce, ce->chanlist[flow].t_phy),
   176		       ce->base + CE_TDQ);
   177	
   178		ce->chanlist[flow].status = 0;
   179		/* Be sure all data is written before enabling the task */
   180		wmb();
   181	
   182		/* Only H6 needs to write a part of t_common_ctl along with "1", but since it is ignored
   183		 * on older SoCs, we have no reason to complicate things.
   184		 */
   185		v = 1 | ((le32_to_cpu(ce->chanlist[flow].tl->t_common_ctl) & 0x7F) << 8);
   186		writel(v, ce->base + CE_TLR);
   187		mutex_unlock(&ce->mlock);
   188	
   189		wait_for_completion_interruptible_timeout(&ce->chanlist[flow].complete,
   190				msecs_to_jiffies(ce->chanlist[flow].timeout));
   191	
   192		if (ce->chanlist[flow].status == 0) {
   193			dev_err(ce->dev, "DMA timeout for %s (tm=%d) on flow %d\n", name,
   194				ce->chanlist[flow].timeout, flow);
   195			err = -EFAULT;
   196		}
   197		/* No need to lock for this read, the channel is locked so
   198		 * nothing could modify the error value for this channel
   199		 */
   200		v = readl(ce->base + CE_ESR);
   201		switch (ce->variant->esr) {
   202		case ESR_H3:
   203			/* Sadly, the error bit is not per flow */
   204			if (v) {
   205				dev_err(ce->dev, "CE ERROR: %x for flow %x\n", v, flow);
   206				err = -EFAULT;
   207				print_hex_dump(KERN_INFO, "TASK: ", DUMP_PREFIX_NONE, 16, 4,
   208					       cet, sizeof(struct ce_task), false);
   209			}
   210			if (v & CE_ERR_ALGO_NOTSUP)
   211				dev_err(ce->dev, "CE ERROR: algorithm not supported\n");
   212			if (v & CE_ERR_DATALEN)
   213				dev_err(ce->dev, "CE ERROR: data length error\n");
   214			if (v & CE_ERR_KEYSRAM)
   215				dev_err(ce->dev, "CE ERROR: keysram access error for AES\n");
   216			break;
   217		case ESR_A64:
   218		case ESR_D1:
   219		case ESR_H5:
   220		case ESR_R40:
   221			v >>= (flow * 4);
   222			v &= 0xF;
   223			if (v) {
   224				dev_err(ce->dev, "CE ERROR: %x for flow %x\n", v, flow);
   225				err = -EFAULT;
   226				print_hex_dump(KERN_INFO, "TASK: ", DUMP_PREFIX_NONE, 16, 4,
   227					       cet, sizeof(struct ce_task), false);
   228			}
   229			if (v & CE_ERR_ALGO_NOTSUP)
   230				dev_err(ce->dev, "CE ERROR: algorithm not supported\n");
   231			if (v & CE_ERR_DATALEN)
   232				dev_err(ce->dev, "CE ERROR: data length error\n");
   233			if (v & CE_ERR_KEYSRAM)
   234				dev_err(ce->dev, "CE ERROR: keysram access error for AES\n");
   235			break;
   236		case ESR_H6:
   237			v >>= (flow * 8);
   238			v &= 0xFF;
   239			if (v) {
   240				dev_err(ce->dev, "CE ERROR: %x for flow %x\n", v, flow);
   241				err = -EFAULT;
   242				print_hex_dump(KERN_INFO, "TASK: ", DUMP_PREFIX_NONE, 16, 4,
   243					       cet, sizeof(struct ce_task), false);
   244			}
   245			if (v & CE_ERR_ALGO_NOTSUP)
   246				dev_err(ce->dev, "CE ERROR: algorithm not supported\n");
   247			if (v & CE_ERR_DATALEN)
   248				dev_err(ce->dev, "CE ERROR: data length error\n");
   249			if (v & CE_ERR_KEYSRAM)
   250				dev_err(ce->dev, "CE ERROR: keysram access error for AES\n");
   251			if (v & CE_ERR_ADDR_INVALID)
   252				dev_err(ce->dev, "CE ERROR: address invalid\n");
   253			if (v & CE_ERR_KEYLADDER)
   254				dev_err(ce->dev, "CE ERROR: key ladder configuration error\n");
   255			break;
   256		}
   257	
   258		return err;
   259	}
   260
kernel test robot June 18, 2024, 1:38 p.m. UTC | #2
Hi Andre,

kernel test robot noticed the following build warnings:

[auto build test WARNING on sunxi/sunxi/for-next]
[also build test WARNING on herbert-cryptodev-2.6/master herbert-crypto-2.6/master linus/master v6.10-rc4 next-20240617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andre-Przywara/dt-bindings-crypto-sun8i-ce-Add-compatible-for-H616/20240617-061144
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
patch link:    https://lore.kernel.org/r/20240616220719.26641-3-andre.przywara%40arm.com
patch subject: [PATCH 2/4] crypto: sun8i-ce - wrap accesses to descriptor address fields
config: mips-randconfig-r121-20240618 (https://download.01.org/0day-ci/archive/20240618/202406182149.eAIruF9N-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 78ee473784e5ef6f0b19ce4cb111fb6e4d23c6b2)
reproduce: (https://download.01.org/0day-ci/archive/20240618/202406182149.eAIruF9N-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406182149.eAIruF9N-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:175:34: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] val @@     got restricted __le32 @@
   drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:175:34: sparse:     expected unsigned int [usertype] val
   drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:175:34: sparse:     got restricted __le32

vim +175 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c

   167	
   168		mutex_lock(&ce->mlock);
   169	
   170		v = readl(ce->base + CE_ICR);
   171		v |= 1 << flow;
   172		writel(v, ce->base + CE_ICR);
   173	
   174		reinit_completion(&ce->chanlist[flow].complete);
 > 175		writel(sun8i_ce_desc_addr(ce, ce->chanlist[flow].t_phy),
   176		       ce->base + CE_TDQ);
   177	
   178		ce->chanlist[flow].status = 0;
   179		/* Be sure all data is written before enabling the task */
   180		wmb();
   181	
   182		/* Only H6 needs to write a part of t_common_ctl along with "1", but since it is ignored
   183		 * on older SoCs, we have no reason to complicate things.
   184		 */
   185		v = 1 | ((le32_to_cpu(ce->chanlist[flow].tl->t_common_ctl) & 0x7F) << 8);
   186		writel(v, ce->base + CE_TLR);
   187		mutex_unlock(&ce->mlock);
   188	
   189		wait_for_completion_interruptible_timeout(&ce->chanlist[flow].complete,
   190				msecs_to_jiffies(ce->chanlist[flow].timeout));
   191	
   192		if (ce->chanlist[flow].status == 0) {
   193			dev_err(ce->dev, "DMA timeout for %s (tm=%d) on flow %d\n", name,
   194				ce->chanlist[flow].timeout, flow);
   195			err = -EFAULT;
   196		}
   197		/* No need to lock for this read, the channel is locked so
   198		 * nothing could modify the error value for this channel
   199		 */
   200		v = readl(ce->base + CE_ESR);
   201		switch (ce->variant->esr) {
   202		case ESR_H3:
   203			/* Sadly, the error bit is not per flow */
   204			if (v) {
   205				dev_err(ce->dev, "CE ERROR: %x for flow %x\n", v, flow);
   206				err = -EFAULT;
   207				print_hex_dump(KERN_INFO, "TASK: ", DUMP_PREFIX_NONE, 16, 4,
   208					       cet, sizeof(struct ce_task), false);
   209			}
   210			if (v & CE_ERR_ALGO_NOTSUP)
   211				dev_err(ce->dev, "CE ERROR: algorithm not supported\n");
   212			if (v & CE_ERR_DATALEN)
   213				dev_err(ce->dev, "CE ERROR: data length error\n");
   214			if (v & CE_ERR_KEYSRAM)
   215				dev_err(ce->dev, "CE ERROR: keysram access error for AES\n");
   216			break;
   217		case ESR_A64:
   218		case ESR_D1:
   219		case ESR_H5:
   220		case ESR_R40:
   221			v >>= (flow * 4);
   222			v &= 0xF;
   223			if (v) {
   224				dev_err(ce->dev, "CE ERROR: %x for flow %x\n", v, flow);
   225				err = -EFAULT;
   226				print_hex_dump(KERN_INFO, "TASK: ", DUMP_PREFIX_NONE, 16, 4,
   227					       cet, sizeof(struct ce_task), false);
   228			}
   229			if (v & CE_ERR_ALGO_NOTSUP)
   230				dev_err(ce->dev, "CE ERROR: algorithm not supported\n");
   231			if (v & CE_ERR_DATALEN)
   232				dev_err(ce->dev, "CE ERROR: data length error\n");
   233			if (v & CE_ERR_KEYSRAM)
   234				dev_err(ce->dev, "CE ERROR: keysram access error for AES\n");
   235			break;
   236		case ESR_H6:
   237			v >>= (flow * 8);
   238			v &= 0xFF;
   239			if (v) {
   240				dev_err(ce->dev, "CE ERROR: %x for flow %x\n", v, flow);
   241				err = -EFAULT;
   242				print_hex_dump(KERN_INFO, "TASK: ", DUMP_PREFIX_NONE, 16, 4,
   243					       cet, sizeof(struct ce_task), false);
   244			}
   245			if (v & CE_ERR_ALGO_NOTSUP)
   246				dev_err(ce->dev, "CE ERROR: algorithm not supported\n");
   247			if (v & CE_ERR_DATALEN)
   248				dev_err(ce->dev, "CE ERROR: data length error\n");
   249			if (v & CE_ERR_KEYSRAM)
   250				dev_err(ce->dev, "CE ERROR: keysram access error for AES\n");
   251			if (v & CE_ERR_ADDR_INVALID)
   252				dev_err(ce->dev, "CE ERROR: address invalid\n");
   253			if (v & CE_ERR_KEYLADDER)
   254				dev_err(ce->dev, "CE ERROR: key ladder configuration error\n");
   255			break;
   256		}
   257	
   258		return err;
   259	}
   260
Chen-Yu Tsai June 21, 2024, 2:53 p.m. UTC | #3
On Mon, Jun 17, 2024 at 6:08 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> The Allwinner H616 (and later) SoCs support more than 32 bits worth of
> physical addresses. To accommodate the larger address space, the CE task
> descriptor fields holding addresses are now encoded as "word addresses",
> so take the actual address divided by four.
> This is true for the fields within the descriptor, but also for the
> descriptor base address, in the CE_TDA register.
>
> Wrap all accesses to those fields in a function, which will do the
> required division if needed. For now this in unused, so there should be
> no change in behaviour.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Chen-Yu Tsai <wens@csie.org>

though you need to fix up the reported sparse warning in sun8i_ce_run_task().

> ---
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c |  8 ++++----
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c   |  3 ++-
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c   |  6 +++---
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c   |  6 +++---
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c   |  2 +-
>  drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h        | 10 ++++++++++
>  6 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> index de50c00ba218f..3a5674b1bd3c0 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> @@ -190,7 +190,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
>                 err = -EFAULT;
>                 goto theend;
>         }
> -       cet->t_key = cpu_to_le32(rctx->addr_key);
> +       cet->t_key = sun8i_ce_desc_addr(ce, rctx->addr_key);
>
>         ivsize = crypto_skcipher_ivsize(tfm);
>         if (areq->iv && crypto_skcipher_ivsize(tfm) > 0) {
> @@ -208,7 +208,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
>                         err = -ENOMEM;
>                         goto theend_iv;
>                 }
> -               cet->t_iv = cpu_to_le32(rctx->addr_iv);
> +               cet->t_iv = sun8i_ce_desc_addr(ce, rctx->addr_iv);
>         }
>
>         if (areq->src == areq->dst) {
> @@ -236,7 +236,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
>
>         len = areq->cryptlen;
>         for_each_sg(areq->src, sg, nr_sgs, i) {
> -               cet->t_src[i].addr = cpu_to_le32(sg_dma_address(sg));
> +               cet->t_src[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
>                 todo = min(len, sg_dma_len(sg));
>                 cet->t_src[i].len = cpu_to_le32(todo / 4);
>                 dev_dbg(ce->dev, "%s total=%u SG(%d %u off=%d) todo=%u\n", __func__,
> @@ -251,7 +251,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
>
>         len = areq->cryptlen;
>         for_each_sg(areq->dst, sg, nr_sgd, i) {
> -               cet->t_dst[i].addr = cpu_to_le32(sg_dma_address(sg));
> +               cet->t_dst[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
>                 todo = min(len, sg_dma_len(sg));
>                 cet->t_dst[i].len = cpu_to_le32(todo / 4);
>                 dev_dbg(ce->dev, "%s total=%u SG(%d %u off=%d) todo=%u\n", __func__,
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> index 0408b2d5d533b..89ab3e08f0697 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> @@ -172,7 +172,8 @@ int sun8i_ce_run_task(struct sun8i_ce_dev *ce, int flow, const char *name)
>         writel(v, ce->base + CE_ICR);
>
>         reinit_completion(&ce->chanlist[flow].complete);
> -       writel(ce->chanlist[flow].t_phy, ce->base + CE_TDQ);
> +       writel(sun8i_ce_desc_addr(ce, ce->chanlist[flow].t_phy),
> +              ce->base + CE_TDQ);
>
>         ce->chanlist[flow].status = 0;
>         /* Be sure all data is written before enabling the task */
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
> index ee2a28c906ede..a710ec9aa96f1 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
> @@ -403,7 +403,7 @@ int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
>
>         len = areq->nbytes;
>         for_each_sg(areq->src, sg, nr_sgs, i) {
> -               cet->t_src[i].addr = cpu_to_le32(sg_dma_address(sg));
> +               cet->t_src[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
>                 todo = min(len, sg_dma_len(sg));
>                 cet->t_src[i].len = cpu_to_le32(todo / 4);
>                 len -= todo;
> @@ -414,7 +414,7 @@ int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
>                 goto theend;
>         }
>         addr_res = dma_map_single(ce->dev, result, digestsize, DMA_FROM_DEVICE);
> -       cet->t_dst[0].addr = cpu_to_le32(addr_res);
> +       cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, addr_res);
>         cet->t_dst[0].len = cpu_to_le32(digestsize / 4);
>         if (dma_mapping_error(ce->dev, addr_res)) {
>                 dev_err(ce->dev, "DMA map dest\n");
> @@ -445,7 +445,7 @@ int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
>         }
>
>         addr_pad = dma_map_single(ce->dev, buf, j * 4, DMA_TO_DEVICE);
> -       cet->t_src[i].addr = cpu_to_le32(addr_pad);
> +       cet->t_src[i].addr = sun8i_ce_desc_addr(ce, addr_pad);
>         cet->t_src[i].len = cpu_to_le32(j);
>         if (dma_mapping_error(ce->dev, addr_pad)) {
>                 dev_err(ce->dev, "DMA error on padding SG\n");
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
> index 80815379f6fc5..f030167f95945 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
> @@ -132,10 +132,10 @@ int sun8i_ce_prng_generate(struct crypto_rng *tfm, const u8 *src,
>         cet->t_sym_ctl = cpu_to_le32(sym);
>         cet->t_asym_ctl = 0;
>
> -       cet->t_key = cpu_to_le32(dma_iv);
> -       cet->t_iv = cpu_to_le32(dma_iv);
> +       cet->t_key = sun8i_ce_desc_addr(ce, dma_iv);
> +       cet->t_iv = sun8i_ce_desc_addr(ce, dma_iv);
>
> -       cet->t_dst[0].addr = cpu_to_le32(dma_dst);
> +       cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, dma_dst);
>         cet->t_dst[0].len = cpu_to_le32(todo / 4);
>         ce->chanlist[flow].timeout = 2000;
>
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> index 9c35f2a83eda8..465c1c512eb85 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> @@ -77,7 +77,7 @@ static int sun8i_ce_trng_read(struct hwrng *rng, void *data, size_t max, bool wa
>         cet->t_sym_ctl = 0;
>         cet->t_asym_ctl = 0;
>
> -       cet->t_dst[0].addr = cpu_to_le32(dma_dst);
> +       cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, dma_dst);
>         cet->t_dst[0].len = cpu_to_le32(todo / 4);
>         ce->chanlist[flow].timeout = todo;
>
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> index 93d4985def87a..8fa58f3bb7f86 100644
> --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> @@ -149,6 +149,7 @@ struct ce_variant {
>         bool hash_t_dlen_in_bits;
>         bool prng_t_dlen_in_bytes;
>         bool trng_t_dlen_in_bytes;
> +       bool needs_word_addresses;
>         struct ce_clock ce_clks[CE_MAX_CLOCKS];
>         int esr;
>         unsigned char prng;
> @@ -241,6 +242,15 @@ struct sun8i_ce_dev {
>  #endif
>  };
>
> +static inline __le32 sun8i_ce_desc_addr(struct sun8i_ce_dev *dev,
> +                                       dma_addr_t addr)
> +{
> +       if (dev->variant->needs_word_addresses)
> +               return cpu_to_le32(addr / 4);
> +
> +       return cpu_to_le32(addr);
> +}
> +
>  /*
>   * struct sun8i_cipher_req_ctx - context for a skcipher request
>   * @op_dir:            direction (encrypt vs decrypt) for this request
> --
> 2.39.4
>
Andre Przywara June 24, 2024, 4:32 p.m. UTC | #4
On Fri, 21 Jun 2024 22:53:47 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

Hi,

> On Mon, Jun 17, 2024 at 6:08 AM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > The Allwinner H616 (and later) SoCs support more than 32 bits worth of
> > physical addresses. To accommodate the larger address space, the CE task
> > descriptor fields holding addresses are now encoded as "word addresses",
> > so take the actual address divided by four.
> > This is true for the fields within the descriptor, but also for the
> > descriptor base address, in the CE_TDA register.
> >
> > Wrap all accesses to those fields in a function, which will do the
> > required division if needed. For now this in unused, so there should be
> > no change in behaviour.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>  
> 
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Thanks for that!

> though you need to fix up the reported sparse warning in sun8i_ce_run_task().

Yeah, that turned out to be a nasty one, and uncovered an actual big
endian bug.
I dropped your R-b from v2 (in your inbox after testing), since I split up
the function and used a different variant for this one writel() caller. So
if you are happy with the changes in sun8i-ce.h and sun8i-ce-core.c: the
other files just use the renamed function name and didn't change otherwise.

Cheers,
Andre

> 
> > ---
> >  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c |  8 ++++----
> >  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c   |  3 ++-
> >  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c   |  6 +++---
> >  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c   |  6 +++---
> >  drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c   |  2 +-
> >  drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h        | 10 ++++++++++
> >  6 files changed, 23 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> > index de50c00ba218f..3a5674b1bd3c0 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> > @@ -190,7 +190,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
> >                 err = -EFAULT;
> >                 goto theend;
> >         }
> > -       cet->t_key = cpu_to_le32(rctx->addr_key);
> > +       cet->t_key = sun8i_ce_desc_addr(ce, rctx->addr_key);
> >
> >         ivsize = crypto_skcipher_ivsize(tfm);
> >         if (areq->iv && crypto_skcipher_ivsize(tfm) > 0) {
> > @@ -208,7 +208,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
> >                         err = -ENOMEM;
> >                         goto theend_iv;
> >                 }
> > -               cet->t_iv = cpu_to_le32(rctx->addr_iv);
> > +               cet->t_iv = sun8i_ce_desc_addr(ce, rctx->addr_iv);
> >         }
> >
> >         if (areq->src == areq->dst) {
> > @@ -236,7 +236,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
> >
> >         len = areq->cryptlen;
> >         for_each_sg(areq->src, sg, nr_sgs, i) {
> > -               cet->t_src[i].addr = cpu_to_le32(sg_dma_address(sg));
> > +               cet->t_src[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
> >                 todo = min(len, sg_dma_len(sg));
> >                 cet->t_src[i].len = cpu_to_le32(todo / 4);
> >                 dev_dbg(ce->dev, "%s total=%u SG(%d %u off=%d) todo=%u\n", __func__,
> > @@ -251,7 +251,7 @@ static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
> >
> >         len = areq->cryptlen;
> >         for_each_sg(areq->dst, sg, nr_sgd, i) {
> > -               cet->t_dst[i].addr = cpu_to_le32(sg_dma_address(sg));
> > +               cet->t_dst[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
> >                 todo = min(len, sg_dma_len(sg));
> >                 cet->t_dst[i].len = cpu_to_le32(todo / 4);
> >                 dev_dbg(ce->dev, "%s total=%u SG(%d %u off=%d) todo=%u\n", __func__,
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> > index 0408b2d5d533b..89ab3e08f0697 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> > @@ -172,7 +172,8 @@ int sun8i_ce_run_task(struct sun8i_ce_dev *ce, int flow, const char *name)
> >         writel(v, ce->base + CE_ICR);
> >
> >         reinit_completion(&ce->chanlist[flow].complete);
> > -       writel(ce->chanlist[flow].t_phy, ce->base + CE_TDQ);
> > +       writel(sun8i_ce_desc_addr(ce, ce->chanlist[flow].t_phy),
> > +              ce->base + CE_TDQ);
> >
> >         ce->chanlist[flow].status = 0;
> >         /* Be sure all data is written before enabling the task */
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
> > index ee2a28c906ede..a710ec9aa96f1 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
> > @@ -403,7 +403,7 @@ int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
> >
> >         len = areq->nbytes;
> >         for_each_sg(areq->src, sg, nr_sgs, i) {
> > -               cet->t_src[i].addr = cpu_to_le32(sg_dma_address(sg));
> > +               cet->t_src[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
> >                 todo = min(len, sg_dma_len(sg));
> >                 cet->t_src[i].len = cpu_to_le32(todo / 4);
> >                 len -= todo;
> > @@ -414,7 +414,7 @@ int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
> >                 goto theend;
> >         }
> >         addr_res = dma_map_single(ce->dev, result, digestsize, DMA_FROM_DEVICE);
> > -       cet->t_dst[0].addr = cpu_to_le32(addr_res);
> > +       cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, addr_res);
> >         cet->t_dst[0].len = cpu_to_le32(digestsize / 4);
> >         if (dma_mapping_error(ce->dev, addr_res)) {
> >                 dev_err(ce->dev, "DMA map dest\n");
> > @@ -445,7 +445,7 @@ int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
> >         }
> >
> >         addr_pad = dma_map_single(ce->dev, buf, j * 4, DMA_TO_DEVICE);
> > -       cet->t_src[i].addr = cpu_to_le32(addr_pad);
> > +       cet->t_src[i].addr = sun8i_ce_desc_addr(ce, addr_pad);
> >         cet->t_src[i].len = cpu_to_le32(j);
> >         if (dma_mapping_error(ce->dev, addr_pad)) {
> >                 dev_err(ce->dev, "DMA error on padding SG\n");
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
> > index 80815379f6fc5..f030167f95945 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
> > @@ -132,10 +132,10 @@ int sun8i_ce_prng_generate(struct crypto_rng *tfm, const u8 *src,
> >         cet->t_sym_ctl = cpu_to_le32(sym);
> >         cet->t_asym_ctl = 0;
> >
> > -       cet->t_key = cpu_to_le32(dma_iv);
> > -       cet->t_iv = cpu_to_le32(dma_iv);
> > +       cet->t_key = sun8i_ce_desc_addr(ce, dma_iv);
> > +       cet->t_iv = sun8i_ce_desc_addr(ce, dma_iv);
> >
> > -       cet->t_dst[0].addr = cpu_to_le32(dma_dst);
> > +       cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, dma_dst);
> >         cet->t_dst[0].len = cpu_to_le32(todo / 4);
> >         ce->chanlist[flow].timeout = 2000;
> >
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> > index 9c35f2a83eda8..465c1c512eb85 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> > @@ -77,7 +77,7 @@ static int sun8i_ce_trng_read(struct hwrng *rng, void *data, size_t max, bool wa
> >         cet->t_sym_ctl = 0;
> >         cet->t_asym_ctl = 0;
> >
> > -       cet->t_dst[0].addr = cpu_to_le32(dma_dst);
> > +       cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, dma_dst);
> >         cet->t_dst[0].len = cpu_to_le32(todo / 4);
> >         ce->chanlist[flow].timeout = todo;
> >
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> > index 93d4985def87a..8fa58f3bb7f86 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
> > @@ -149,6 +149,7 @@ struct ce_variant {
> >         bool hash_t_dlen_in_bits;
> >         bool prng_t_dlen_in_bytes;
> >         bool trng_t_dlen_in_bytes;
> > +       bool needs_word_addresses;
> >         struct ce_clock ce_clks[CE_MAX_CLOCKS];
> >         int esr;
> >         unsigned char prng;
> > @@ -241,6 +242,15 @@ struct sun8i_ce_dev {
> >  #endif
> >  };
> >
> > +static inline __le32 sun8i_ce_desc_addr(struct sun8i_ce_dev *dev,
> > +                                       dma_addr_t addr)
> > +{
> > +       if (dev->variant->needs_word_addresses)
> > +               return cpu_to_le32(addr / 4);
> > +
> > +       return cpu_to_le32(addr);
> > +}
> > +
> >  /*
> >   * struct sun8i_cipher_req_ctx - context for a skcipher request
> >   * @op_dir:            direction (encrypt vs decrypt) for this request
> > --
> > 2.39.4
> >
diff mbox series

Patch

diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
index de50c00ba218f..3a5674b1bd3c0 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
@@ -190,7 +190,7 @@  static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
 		err = -EFAULT;
 		goto theend;
 	}
-	cet->t_key = cpu_to_le32(rctx->addr_key);
+	cet->t_key = sun8i_ce_desc_addr(ce, rctx->addr_key);
 
 	ivsize = crypto_skcipher_ivsize(tfm);
 	if (areq->iv && crypto_skcipher_ivsize(tfm) > 0) {
@@ -208,7 +208,7 @@  static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
 			err = -ENOMEM;
 			goto theend_iv;
 		}
-		cet->t_iv = cpu_to_le32(rctx->addr_iv);
+		cet->t_iv = sun8i_ce_desc_addr(ce, rctx->addr_iv);
 	}
 
 	if (areq->src == areq->dst) {
@@ -236,7 +236,7 @@  static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
 
 	len = areq->cryptlen;
 	for_each_sg(areq->src, sg, nr_sgs, i) {
-		cet->t_src[i].addr = cpu_to_le32(sg_dma_address(sg));
+		cet->t_src[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
 		todo = min(len, sg_dma_len(sg));
 		cet->t_src[i].len = cpu_to_le32(todo / 4);
 		dev_dbg(ce->dev, "%s total=%u SG(%d %u off=%d) todo=%u\n", __func__,
@@ -251,7 +251,7 @@  static int sun8i_ce_cipher_prepare(struct crypto_engine *engine, void *async_req
 
 	len = areq->cryptlen;
 	for_each_sg(areq->dst, sg, nr_sgd, i) {
-		cet->t_dst[i].addr = cpu_to_le32(sg_dma_address(sg));
+		cet->t_dst[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
 		todo = min(len, sg_dma_len(sg));
 		cet->t_dst[i].len = cpu_to_le32(todo / 4);
 		dev_dbg(ce->dev, "%s total=%u SG(%d %u off=%d) todo=%u\n", __func__,
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
index 0408b2d5d533b..89ab3e08f0697 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
@@ -172,7 +172,8 @@  int sun8i_ce_run_task(struct sun8i_ce_dev *ce, int flow, const char *name)
 	writel(v, ce->base + CE_ICR);
 
 	reinit_completion(&ce->chanlist[flow].complete);
-	writel(ce->chanlist[flow].t_phy, ce->base + CE_TDQ);
+	writel(sun8i_ce_desc_addr(ce, ce->chanlist[flow].t_phy),
+	       ce->base + CE_TDQ);
 
 	ce->chanlist[flow].status = 0;
 	/* Be sure all data is written before enabling the task */
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
index ee2a28c906ede..a710ec9aa96f1 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-hash.c
@@ -403,7 +403,7 @@  int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
 
 	len = areq->nbytes;
 	for_each_sg(areq->src, sg, nr_sgs, i) {
-		cet->t_src[i].addr = cpu_to_le32(sg_dma_address(sg));
+		cet->t_src[i].addr = sun8i_ce_desc_addr(ce, sg_dma_address(sg));
 		todo = min(len, sg_dma_len(sg));
 		cet->t_src[i].len = cpu_to_le32(todo / 4);
 		len -= todo;
@@ -414,7 +414,7 @@  int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
 		goto theend;
 	}
 	addr_res = dma_map_single(ce->dev, result, digestsize, DMA_FROM_DEVICE);
-	cet->t_dst[0].addr = cpu_to_le32(addr_res);
+	cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, addr_res);
 	cet->t_dst[0].len = cpu_to_le32(digestsize / 4);
 	if (dma_mapping_error(ce->dev, addr_res)) {
 		dev_err(ce->dev, "DMA map dest\n");
@@ -445,7 +445,7 @@  int sun8i_ce_hash_run(struct crypto_engine *engine, void *breq)
 	}
 
 	addr_pad = dma_map_single(ce->dev, buf, j * 4, DMA_TO_DEVICE);
-	cet->t_src[i].addr = cpu_to_le32(addr_pad);
+	cet->t_src[i].addr = sun8i_ce_desc_addr(ce, addr_pad);
 	cet->t_src[i].len = cpu_to_le32(j);
 	if (dma_mapping_error(ce->dev, addr_pad)) {
 		dev_err(ce->dev, "DMA error on padding SG\n");
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
index 80815379f6fc5..f030167f95945 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-prng.c
@@ -132,10 +132,10 @@  int sun8i_ce_prng_generate(struct crypto_rng *tfm, const u8 *src,
 	cet->t_sym_ctl = cpu_to_le32(sym);
 	cet->t_asym_ctl = 0;
 
-	cet->t_key = cpu_to_le32(dma_iv);
-	cet->t_iv = cpu_to_le32(dma_iv);
+	cet->t_key = sun8i_ce_desc_addr(ce, dma_iv);
+	cet->t_iv = sun8i_ce_desc_addr(ce, dma_iv);
 
-	cet->t_dst[0].addr = cpu_to_le32(dma_dst);
+	cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, dma_dst);
 	cet->t_dst[0].len = cpu_to_le32(todo / 4);
 	ce->chanlist[flow].timeout = 2000;
 
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
index 9c35f2a83eda8..465c1c512eb85 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
@@ -77,7 +77,7 @@  static int sun8i_ce_trng_read(struct hwrng *rng, void *data, size_t max, bool wa
 	cet->t_sym_ctl = 0;
 	cet->t_asym_ctl = 0;
 
-	cet->t_dst[0].addr = cpu_to_le32(dma_dst);
+	cet->t_dst[0].addr = sun8i_ce_desc_addr(ce, dma_dst);
 	cet->t_dst[0].len = cpu_to_le32(todo / 4);
 	ce->chanlist[flow].timeout = todo;
 
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
index 93d4985def87a..8fa58f3bb7f86 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h
@@ -149,6 +149,7 @@  struct ce_variant {
 	bool hash_t_dlen_in_bits;
 	bool prng_t_dlen_in_bytes;
 	bool trng_t_dlen_in_bytes;
+	bool needs_word_addresses;
 	struct ce_clock ce_clks[CE_MAX_CLOCKS];
 	int esr;
 	unsigned char prng;
@@ -241,6 +242,15 @@  struct sun8i_ce_dev {
 #endif
 };
 
+static inline __le32 sun8i_ce_desc_addr(struct sun8i_ce_dev *dev,
+					dma_addr_t addr)
+{
+	if (dev->variant->needs_word_addresses)
+		return cpu_to_le32(addr / 4);
+
+	return cpu_to_le32(addr);
+}
+
 /*
  * struct sun8i_cipher_req_ctx - context for a skcipher request
  * @op_dir:		direction (encrypt vs decrypt) for this request