lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 10 Apr 2023 16:43:37 +0800
From:   Jia Jie Ho <jiajie.ho@...rfivetech.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
CC:     "David S . Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Palmer Dabbelt <palmer@...belt.com>,
        Emil Renner Berthing <kernel@...il.dk>,
        Conor Dooley <conor.dooley@...rochip.com>,
        <linux-crypto@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-riscv@...ts.infradead.org>
Subject: Re: [PATCH v4 4/4] crypto: starfive - Add hash and HMAC support

On 10/4/2023 3:50 pm, Herbert Xu wrote:
> On Mon, Apr 10, 2023 at 03:37:52PM +0800, Jia Jie Ho wrote:
>>
>> +static void starfive_hash_start(void *param)
>> +{
>> +	struct starfive_cryp_ctx *ctx = param;
>> +	struct starfive_cryp_request_ctx *rctx = ctx->rctx;
>> +	struct starfive_cryp_dev *cryp = ctx->cryp;
>> +	union starfive_alg_cr alg_cr;
>> +	union starfive_hash_csr csr;
>> +
>> +	dma_unmap_sg(cryp->dev, rctx->in_sg, rctx->in_sg_len, DMA_TO_DEVICE);
>> +
>> +	alg_cr.v = 0;
>> +	alg_cr.clear = 1;
>> +
>> +	writel(alg_cr.v, cryp->base + STARFIVE_ALG_CR_OFFSET);
>> +
>> +	csr.v = readl(cryp->base + STARFIVE_HASH_SHACSR);
>> +	csr.firstb = 0;
>> +	csr.final = 1;
>> +
>> +	reinit_completion(&cryp->hash_done);
>> +	writel(~STARFIVE_IE_MASK_HASH_DONE, cryp->base + STARFIVE_IE_MASK_OFFSET);
>> +	writel(csr.v, cryp->base + STARFIVE_HASH_SHACSR);
>> +}
> 
> Why are you still using a completion? The callback function should
> invoke the crypto_engine finalize_request call directly.
> 

Hi Herbert,
The hardware requires user to set a 'final' bit after data transfer completed.
This completion is to wait for the interrupt signal from device that the final digest 
has been populated to the read registers.

I'll do the finalize_request call directly in the next version.

>> +static int starfive_hash_xmit(struct starfive_cryp_ctx *ctx)
>> +{
>> +	struct starfive_cryp_request_ctx *rctx = ctx->rctx;
>> +	struct starfive_cryp_dev *cryp = ctx->cryp;
>> +	int ret;
>> +
>> +	rctx->csr.hash.v = 0;
>> +	rctx->csr.hash.reset = 1;
>> +	writel(rctx->csr.hash.v, cryp->base + STARFIVE_HASH_SHACSR);
>> +
>> +	if (starfive_hash_wait_busy(ctx))
>> +		return dev_err_probe(cryp->dev, -ETIMEDOUT, "Error resetting engine.\n");
>> +
>> +	rctx->csr.hash.v = 0;
>> +	rctx->csr.hash.mode = ctx->hash_mode & STARFIVE_HASH_MODE_MASK;
>> +	rctx->csr.hash.ie = 1;
>> +
>> +	if (ctx->hash_mode & STARFIVE_HASH_HMAC_FLAGS) {
>> +		ret = starfive_hash_hmac_key(ctx);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		rctx->csr.hash.start = 1;
>> +		rctx->csr.hash.firstb = 1;
>> +		writel(rctx->csr.hash.v, cryp->base + STARFIVE_HASH_SHACSR);
>> +	}
>> +
>> +	ret = starfive_hash_xmit_dma(ctx);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (!wait_for_completion_timeout(&cryp->hash_done, msecs_to_jiffies(10000)))
>> +		return dev_err_probe(cryp->dev, -ETIMEDOUT, "Timeout waiting for hash done\n");
> 
> There is no point in waiting for completion.  Just return 0 and
> you're done.
> 

I'll change this in the next version too.

Thanks for taking time reviewing this patch.

Best regards,
Jia Jie

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ