Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Horia Geanta <horia.geanta@nxp.com>
To:     Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
CC:     Eric Biggers <ebiggers@kernel.org>,
        Iuliana Prodan <iuliana.prodan@nxp.com>,
        "David S. Miller" <davem@davemloft.net>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH] crypto: gcm - fix cacheline sharing
Thread-Topic: [PATCH] crypto: gcm - fix cacheline sharing
Thread-Index: AQHVFkF+dOvdDzZqYUSH856vNqfARw==
Date:   Fri, 31 May 2019 06:05:06 +0000
Message-ID: <VI1PR0402MB3485E750B15281E7227EC49198190@VI1PR0402MB3485.eurprd04.prod.outlook.com>
References: <1559149856-7938-1-git-send-email-iuliana.prodan@nxp.com>
 <20190529202728.GA35103@gmail.com>
 <20190530053421.keesqb54yu5w7hgk@gondor.apana.org.au>
 <VI1PR0402MB3485ADA3C4410D61191582A498180@VI1PR0402MB3485.eurprd04.prod.outlook.com>
 <CAKv+Gu84HndAnkn7DU=ykjCokw_+bAHEcF0Rm12-hnXhVy2u_Q@mail.gmail.com>
 <VI1PR0402MB34859577A96645E890BD8F3198180@VI1PR0402MB3485.eurprd04.prod.outlook.com>
 <CAKv+Gu_ucBhuK0YgGZ+Qhv4zqvU868GVRpWY7KVaTsj0O8k3OQ@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: f9d0da71-9492-4713-d8e8-08d6e58dee24
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 May 2019 06:05:06.6466
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: horia.geanta@nxp.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB2943
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 5/30/2019 4:26 PM, Ard Biesheuvel wrote:=0A=
> On Thu, 30 May 2019 at 15:18, Horia Geanta <horia.geanta@nxp.com> wrote:=
=0A=
>>=0A=
>> On 5/30/2019 11:08 AM, Ard Biesheuvel wrote:=0A=
>>> On Thu, 30 May 2019 at 09:46, Horia Geanta <horia.geanta@nxp.com> wrote=
:=0A=
>>>>=0A=
>>>> On 5/30/2019 8:34 AM, Herbert Xu wrote:=0A=
>>>>> It would appear that Ard's latest suggestion should fix the problem=
=0A=
>>>>> and is the correct approach.=0A=
>>>>>=0A=
>>>> I disagree.=0A=
>>>> We shouldn't force crypto implementations to be aware of such inconsis=
tencies in=0A=
>>>> the I/O data buffers (pointed to by src/dst scatterlists) that are sup=
posed to=0A=
>>>> be safely DMA mapped.=0A=
>>>>=0A=
>>>=0A=
>>> I'm on the fence here. On the one hand, it is slightly dodgy for the=0A=
>>> GCM driver to pass a scatterlist referencing a buffer that shares a=0A=
>>> cacheline with another buffer passed by an ordinary pointer, and for=0A=
>>> which an explicit requirement exists that the callee should update it=
=0A=
>>> before returning.=0A=
>>>=0A=
>>> On the other hand, I think it is reasonable to require drivers not to=
=0A=
>>> perform such updates while the scatterlist is mapped for DMA, since=0A=
>>> fixing it in the callers puts a disproportionate burden on them, given=
=0A=
>>> that non-coherent DMA only represents a small minority of use cases.=0A=
>>>=0A=
>> The problem with this approach is that the buffers in the scatterlist co=
uld=0A=
>> hypothetically share cache lines with *any* other CPU-updated data, not =
just the=0A=
>> IV in the crypto request (as it happens here).=0A=
>> How could a non-coherent DMA implementation cope with this?=0A=
>>=0A=
> =0A=
> I don't think the situation is that bad. We only support allocations=0A=
> in the linear map for DMA/scatterlists, and buffers in the linear map=0A=
> can only share a cacheline if they were allocated using the same=0A=
> kmalloc() invocation (on systems that support non-coherent DMA)=0A=
> =0A=
> That does mean that it is actually more straightforward to deal with=0A=
> this at the level where the allocation occurs, and that would justify=0A=
> dealing with it in the callers.=0A=
> =0A=
> However, from the callee's point of view, it simply means that you=0A=
> should not dereference any request struct pointers for writing while=0A=
> the 'dst' scatterlist is mapped for DMA.=0A=
> =0A=
Is this the only restriction, or I might find out more in the future?=0A=
=0A=
This kind of violation of an abstraction layer must be clearly documented.=
=0A=
=0A=
In particular, if crypto API implementations cannot rely on DMA API guarant=
ees,=0A=
then exceptions must be listed.=0A=
=0A=
Thanks,=0A=
Horia=0A=