Received-SPF: pass (google.com: best guess record for domain of linux-kernel-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:     Marek Szyprowski <m.szyprowski@samsung.com>,
        Robin Murphy <robin.murphy@arm.com>,
        Christoph Hellwig <hch@lst.de>
CC:     Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH] swiotlb: sync buffer when mapping FROM_DEVICE
Thread-Topic: [PATCH] swiotlb: sync buffer when mapping FROM_DEVICE
Thread-Index: AQHVEG7XwyI2zYKLCECD0q7/YpNC2Q==
Date:   Thu, 23 May 2019 16:25:54 +0000
Message-ID: <AM0PR0402MB3476949ECC57B188E83B820B98010@AM0PR0402MB3476.eurprd04.prod.outlook.com>
References: <20190522072018.10660-1-horia.geanta@nxp.com>
 <20190522123243.GA26390@lst.de>
 <6cbe5470-16a6-17e9-337d-6ba18b16b6e8@arm.com>
 <20190522130921.GA26874@lst.de>
 <fdfd7318-7999-1fe6-01b6-ae1fb7ba8c30@arm.com>
 <20190522133400.GA27229@lst.de>
 <CGME20190522135556epcas2p34e0c14f2565abfdccc7035463f60a71b@epcas2p3.samsung.com>
 <ed26de5e-aee4-4e19-095c-cc551012d475@arm.com>
 <0c79721a-11cb-c945-5626-3d43cc299fe6@samsung.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="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 7a0a4b2c-4721-4767-1917-08d6df9b5418
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 May 2019 16:25:54.2364
 (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-Transport-CrossTenantHeadersStamped: AM0PR0402MB3378
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 5/23/2019 8:35 AM, Marek Szyprowski wrote:=0A=
> Hi Robin,=0A=
> =0A=
> On 2019-05-22 15:55, Robin Murphy wrote:=0A=
>> On 22/05/2019 14:34, Christoph Hellwig wrote:=0A=
>>> On Wed, May 22, 2019 at 02:25:38PM +0100, Robin Murphy wrote:=0A=
>>>> Sure, but that should be irrelevant since the effective problem here =
=0A=
>>>> is in=0A=
>>>> the sync_*_for_cpu direction, and it's the unmap which nobbles the =0A=
>>>> buffer.=0A=
>>>> If the driver does this:=0A=
>>>>=0A=
>>>> =A0=A0=A0=A0dma_map_single(whole buffer);=0A=
>>>> =A0=A0=A0=A0<device writes to part of buffer>=0A=
>>>> =A0=A0=A0=A0dma_unmap_single(whole buffer);=0A=
>>>> =A0=A0=A0=A0<contents of rest of buffer now undefined>=0A=
>>>>=0A=
>>>> then it could instead do this and be happy:=0A=
>>>>=0A=
>>>> =A0=A0=A0=A0dma_map_single(whole buffer, SKIP_CPU_SYNC);=0A=
>>>> =A0=A0=A0=A0<device writes to part of buffer>=0A=
>>>> =A0=A0=A0=A0dma_sync_single_for_cpu(updated part of buffer);=0A=
>>>> =A0=A0=A0=A0dma_unmap_single(whole buffer, SKIP_CPU_SYNC);=0A=
>>>> =A0=A0=A0=A0<contents of rest of buffer still valid>=0A=
>>>=0A=
>>> Assuming the driver knows how much was actually DMAed this would=0A=
>>> solve the issue.=A0 Horia, does this work for you?=0A=
In my particular case, input is provided as a scatterlist, out of which fir=
st N=0A=
bytes are problematic (not written to by device and corrupted when swiotlb=
=0A=
bouncing is needed), while remaining bytes (Total - N) are updated by the d=
evice.=0A=
=0A=
>>=0A=
>> Ohhh, and now I've just twigged what you were suggesting - your =0A=
>> DMA_ATTR_PARTIAL flag would mean "treat this as a read-modify-write of =
=0A=
>> the buffer because we *don't* know exactly which parts the device may =
=0A=
>> write to". So indeed if we did go down that route we wouldn't need any =
=0A=
>> of the sync stuff I was worrying about (but I might suggest naming it =
=0A=
>> DMA_ATTR_UPDATE instead). Apologies for being slow :)=0A=
> =0A=
> Don't we have DMA_BIDIRECTIONAL for such case? Maybe we should update =0A=
> documentation a bit to point that DMA_FROM_DEVICE expects the whole =0A=
> buffer to be filled by the device?=0A=
> =0A=
Or, put more bluntly, driver must not rely on previous data in the area map=
ped=0A=
DMA_FROM_DEVICE. This limitation stems from the buffer bouncing mechanism o=
f the=0A=
swiotlb DMA API backend, which other backends might not suffer from (e.g. I=
OMMU).=0A=
=0A=
Btw, the device I am working on (caam crypto engine) is deployed in several=
 SoCs=0A=
configured differently - with or without an IOMMU (and coherent or non-cohe=
rent=0A=
etc.). IOW it's a "power user" of the DMA API and I appreciate all the help=
 in=0A=
solving / clarifying this kind of implicit assumptions.=0A=
=0A=
Thanks,=0A=
Horia=0A=