From:   Lionel DEBIEVE <lionel.debieve@st.com>
To:     Eric Biggers <ebiggers@kernel.org>
CC:     Herbert Xu <herbert@gondor.apana.org.au>,
        "David S . Miller" <davem@davemloft.net>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        "Alexandre TORGUE" <alexandre.torgue@st.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Benjamin GAIGNARD <benjamin.gaignard@st.com>,
        Fabien DESSENNE <fabien.dessenne@st.com>,
        Ludovic BARRE <ludovic.barre@st.com>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH 1/1] crypto: testmgr - call shash_init in crc32c algo
Thread-Topic: [PATCH 1/1] crypto: testmgr - call shash_init in crc32c algo
Thread-Index: AQHU6TRB39TFooeiy0SNdS6nTm4gO6Y3/68A
Date:   Fri, 12 Apr 2019 06:34:42 +0000
Message-ID: <58095a24-8d96-fe7e-0c99-461b015184e6@st.com>
References: <1554123264-24243-1-git-send-email-lionel.debieve@st.com>
 <20190401173056.GD131675@gmail.com>
 <e1f694bd-e0f7-a3f4-426a-894a45c88a30@st.com>
In-Reply-To: <e1f694bd-e0f7-a3f4-426a-894a45c88a30@st.com>
Accept-Language: fr-FR, en-US
Content-Language: en-US
user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.75.127.46]
Content-Type: text/plain; charset="Windows-1252"
Content-ID: <7FB9AE74DCDC324F891EC099F642EC4F@st.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-04-12_04:,,
 signatures=0
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

Hi Eric,

On 4/1/19 7:30 PM, Eric Biggers wrote:

> Hi Lionel,
>
> On Mon, Apr 01, 2019 at 02:54:24PM +0200, Lionel Debieve wrote:
>> In case of device call required in low level driver,
>> the context must be initialized before calling the final
>> function.
>>
>> Signed-off-by: Lionel Debieve<lionel.debieve@st.com>
>> ---
>>   crypto/testmgr.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
>> index 8386038..4a00d7c 100644
>> --- a/crypto/testmgr.c
>> +++ b/crypto/testmgr.c
>> @@ -2181,6 +2181,13 @@ static int alg_test_crc32c(const struct alg_test_=
desc *desc,
>>   		shash->tfm =3D tfm;
>>   		shash->flags =3D 0;
>>  =20
>> +		err =3D crypto_shash_init(shash);
>> +		if (err) {
>> +			printk(KERN_ERR "alg: crc32c: init failed for "
>> +			       "%s: %d\n", driver, err);
>> +			break;
>> +		}
>> +
>>   		*ctx =3D 420553207;
>>   		err =3D crypto_shash_final(shash, (u8 *)&val);
>>   		if (err) {
>> --=20
>> 2.7.4
>>
> This defeats the point of the test, which is that crc32c implementations =
are
> expected to use the same shash_desc context format and be usable by calli=
ng
> crypto_shash_update() directly after initializing the context manually, w=
ithout
> a prior crypto_shash_init().  See for example ext4_chksum() in fs/ext4/ex=
t4.h:
>
> 	static inline u32 ext4_chksum(struct ext4_sb_info *sbi, u32 crc,
> 				      const void *address, unsigned int length)
> 	{
> 		struct {
> 			struct shash_desc shash;
> 			char ctx[4];
> 		} desc;
>
> 		BUG_ON(crypto_shash_descsize(sbi->s_chksum_driver)!=3Dsizeof(desc.ctx))=
;
>
> 		desc.shash.tfm =3D sbi->s_chksum_driver;
> 		desc.shash.flags =3D 0;
> 		*(u32 *)desc.ctx =3D crc;
>
> 		BUG_ON(crypto_shash_update(&desc.shash, address, length));
>
> 		return *(u32 *)desc.ctx;
> 	}
>
> I think you need to fix the stm32 crc32 driver to not store anything extr=
a in
> the shash_desc context, and only use hardware during ->update().
>
> - Eric
>
OK, catch your point but refering to the devel-algos.rst documentation, it =
seems that there is no way
to bypass the init part. I'm based on a hardware that need a clear initiali=
zation to be up and running.
Is the doc not updated? I'm working to optimized data transfer into my hard=
ware by pushing only 32 bits
datas and I'm saving the others to be pushed later.

If we assume that hardware must be only used in the update function, I've t=
o rework the driver as today,
HW init is made in the init function too.

BR
- Lionel