Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20220702071426.1915429-1-oferh@marvell.com>
References: <20220702071426.1915429-1-oferh@marvell.com>
Subject: Re: [PATCH] crypto: inside-secure: fix packed bit-field result descriptor
From:   Antoine Tenart <atenart@kernel.org>
To:     davem@davemloft.net, herbert@gondor.apana.org.au,
        linux-crypto@vger.kernel.org, oferh@marvell.com
Message-ID: <165691958960.3764.6967842126015044710@kwain>
Date:   Mon, 04 Jul 2022 09:26:29 +0200
Precedence: bulk

Hi Ofer,

Quoting oferh@marvell.com (2022-07-02 09:14:26)
> From: Ofer Heifetz <oferh@marvell.com>
>=20
> When mixing bit-field and none bit-filed in packed struct the
> none bit-field starts at a distinct memory location, thus adding
> an additional byte to the overall structure which is used in
> memory zero-ing and other configuration calculations.
>=20
> Fix this by removing the none bit-field that has a following
> bit-field.
>=20
> Signed-off-by: Ofer Heifetz <oferh@marvell.com>

Nice catch!

Note: since those fields were not used before and IIRC the below result
struct size is set dynamically (the h/w doesn't expect a fixed size)
this doesn't need to be backported to stable trees. Can't test it on
real h/w though.

Acked-by: Antoine Tenart <atenart@kernel.org>

Thanks!

> ---
>  drivers/crypto/inside-secure/safexcel.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>=20
> diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/ins=
ide-secure/safexcel.h
> index ce1e611a163e..797ff91512e0 100644
> --- a/drivers/crypto/inside-secure/safexcel.h
> +++ b/drivers/crypto/inside-secure/safexcel.h
> @@ -497,15 +497,15 @@ struct result_data_desc {
>         u32 packet_length:17;
>         u32 error_code:15;
> =20
> -       u8 bypass_length:4;
> -       u8 e15:1;
> -       u16 rsvd0;
> -       u8 hash_bytes:1;
> -       u8 hash_length:6;
> -       u8 generic_bytes:1;
> -       u8 checksum:1;
> -       u8 next_header:1;
> -       u8 length:1;
> +       u32 bypass_length:4;
> +       u32 e15:1;
> +       u32 rsvd0:16;
> +       u32 hash_bytes:1;
> +       u32 hash_length:6;
> +       u32 generic_bytes:1;
> +       u32 checksum:1;
> +       u32 next_header:1;
> +       u32 length:1;
> =20
>         u16 application_id;
>         u16 rsvd1;
> --=20
> 2.25.1
>=20