From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751744AbeBSH6T (ORCPT ); Mon, 19 Feb 2018 02:58:19 -0500 Received: from pegase1.c-s.fr ([93.17.236.30]:53106 "EHLO pegase1.c-s.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751609AbeBSH6R (ORCPT ); Mon, 19 Feb 2018 02:58:17 -0500 Subject: Re: [PATCH 16/18] crypto: talitos - do hw_context DMA mapping outside the requests To: =?UTF-8?Q?Horia_Geant=c4=83?= , Herbert Xu , "David S. Miller" Cc: "linux-crypto@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" References: <2338eb8d-a26a-9722-d667-5a9785b32977@c-s.fr> From: Christophe LEROY Message-ID: <630c313c-5eca-224d-a5ac-fb3f2783573a@c-s.fr> Date: Mon, 19 Feb 2018 08:58:15 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1258; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 18/02/2018 à 18:14, Horia Geantã a écrit : > On 2/17/2018 6:32 PM, Christophe LEROY wrote: >> >> >> Le 07/02/2018 à 15:39, Horia Geantã a écrit : >>> On 10/6/2017 4:06 PM, Christophe Leroy wrote: >>>> At every request, we map and unmap the same hash hw_context. >>>> >>>> This patch moves the dma mapping/unmapping in functions ahash_init() >>>> and ahash_import(). >>>> >>>> Signed-off-by: Christophe Leroy >>>> --- >>>> drivers/crypto/talitos.c | 80 ++++++++++++++++++++++++++++++++++-------------- >>>> 1 file changed, 57 insertions(+), 23 deletions(-) >>>> >>>> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c >>>> index ebfd6d982ed6..d495649d5267 100644 >>>> --- a/drivers/crypto/talitos.c >>>> +++ b/drivers/crypto/talitos.c >>>> @@ -819,6 +819,7 @@ struct talitos_ctx { >>>> unsigned int keylen; >>>> unsigned int enckeylen; >>>> unsigned int authkeylen; >>>> + dma_addr_t dma_hw_context; >>> This doesn't look correct. >>> >>> talitos_ctx structure is the tfm context. >>> dma_hw_context is the IOVA of hw_context, located in talitos_ahash_req_ctx >>> structure (request context). >> >> Yes but I have now found how I can know that the request context is >> being released in order to unmap() dma at that time. >> It is tricky to use the tmf context I agree, but at least I know when >> tmf context get destroyed, ie in talitos_cra_exit_ahash() >> The request context is created by ahash_request_alloc() and released by >> ahash_request_free(). I have not found the way to call dma_unmap() >> before ahash_request_free() gets called. >> >>> >>> If there are multiple requests in flight for the same tfm, dma_hw_context will >>> be overwritten. >> >> Before overwritting dma_hw_context, it is always released, see >> talitos_cra_exit_ahash(), ahash_init(), ahash_import() >> > The problem is not the unmapping. > If there are two requests for the same tfm, then given the following sequence > 1. tfm->ahash_init(req1) > tfm_ctx->dma_hw_context points to req1_ctx->hw_context > 2. tfm->ahash_init(req2) > tfm_ctx->dma_hw_context [unmapped, then] points to req2_ctx->hw_context > i.e. req1 will use the hw_context of req2. > >>> >>> dma_hw_context needs to be moved in request context (talitos_ahash_req_ctx struct). >> >> Any suggestion then on how to handle the issue explained above ? >> > There is no ahash_exit() callback mirroring ahash_init(). > > The clean-up of request ctx should be done in the last states of the hash flows > described here: > https://www.kernel.org/doc/html/latest/crypto/devel-algos.html#cipher-definition-with-struct-shash-alg-and-ahash-alg > for e.g. in the final() callback. Unfortunatly it seems that we can't rely on those finalising functions being called all the time. If you look into test_ahash_jiffies() for instance, in case of error the call of crypto_hash_final() is skipped. So at the time being, I can't see any place to put the unmapping to be 100% sure it will be done before the call of ahash_request_free() Christophe > > Hope this helps, > Horia >