Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 30 Jun 2021 19:44:21 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Marek Szyprowski <m.szyprowski@samsung.com>,
        Leon Romanovsky <leon@kernel.org>
Cc:     Doug Ledford <dledford@redhat.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Maor Gottlieb <maorg@nvidia.com>,
        Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>,
        linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
        Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>,
        Yishai Hadas <yishaih@nvidia.com>,
        Zhu Yanjun <zyjzyj2000@gmail.com>,
        Christoph Hellwig <hch@lst.de>,
        Robin Murphy <robin.murphy@arm.com>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH rdma-next v1 1/2] lib/scatterlist: Fix wrong update of
 orig_nents
Message-ID: <YNytdbEG9OSHOT1z@orome.fritz.box>
References: <cover.1624955710.git.leonro@nvidia.com>
 <dadb01a81e7498f6415233cf19cfc2a0d9b312f2.1624955710.git.leonro@nvidia.com>
 <CGME20210630111227eucas1p2212b63f5d9da6788e57319c35ce9eaf4@eucas1p2.samsung.com>
 <a9462d67-2279-93f1-e042-d46033c208df@samsung.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="k6cvBpDdjY2dq+5c"
Content-Disposition: inline
In-Reply-To: <a9462d67-2279-93f1-e042-d46033c208df@samsung.com>
User-Agent: Mutt/2.1 (4b100969) (2021-06-12)
Precedence: bulk


--k6cvBpDdjY2dq+5c
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 30, 2021 at 01:12:26PM +0200, Marek Szyprowski wrote:
> Hi Leon,
>=20
> On 29.06.2021 10:40, Leon Romanovsky wrote:
> > From: Maor Gottlieb <maorg@nvidia.com>
> >
> > orig_nents should represent the number of entries with pages,
> > but __sg_alloc_table_from_pages sets orig_nents as the number of
> > total entries in the table. This is wrong when the API is used for
> > dynamic allocation where not all the table entries are mapped with
> > pages. It wasn't observed until now, since RDMA umem who uses this
> > API in the dynamic form doesn't use orig_nents implicit or explicit
> > by the scatterlist APIs.
> >
> > Fix it by:
> > 1. Set orig_nents as number of entries with pages also in
> >     __sg_alloc_table_from_pages.
> > 2. Add a new field total_nents to reflect the total number of entries
> >     in the table. This is required for the release flow (sg_free_table).
> >     This filed should be used internally only by scatterlist.
> >
> > Fixes: 07da1223ec93 ("lib/scatterlist: Add support in dynamic allocatio=
n of SG table from pages")
> > Signed-off-by: Maor Gottlieb <maorg@nvidia.com>
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
>=20
> This patch landed in linux-next 20210630 as commit a52724456928=20
> ("lib/scatterlist: Fix wrong update of orig_nents"). It causes serious=20
> regression in DMA-IOMMU integration, which can be observed for example=20
> on ARM Juno board during boot:
>=20
> Unable to handle kernel paging request at virtual address 00376f42a6e40454
> Mem abort info:
>  =C2=A0 ESR =3D 0x96000004
>  =C2=A0 EC =3D 0x25: DABT (current EL), IL =3D 32 bits
>  =C2=A0 SET =3D 0, FnV =3D 0
>  =C2=A0 EA =3D 0, S1PTW =3D 0
>  =C2=A0 FSC =3D 0x04: level 0 translation fault
> Data abort info:
>  =C2=A0 ISV =3D 0, ISS =3D 0x00000004
>  =C2=A0 CM =3D 0, WnR =3D 0
> [00376f42a6e40454] address between user and kernel address ranges
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.13.0-next-20210630+ #3585
> Hardware name: ARM Juno development board (r1) (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=3D--)
> pc : __sg_free_table+0x60/0xa0
> lr : __sg_free_table+0x7c/0xa0
> ..
> Call trace:
>  =C2=A0__sg_free_table+0x60/0xa0
>  =C2=A0sg_free_table+0x1c/0x28
>  =C2=A0iommu_dma_alloc+0xc8/0x388
>  =C2=A0dma_alloc_attrs+0xcc/0xf0
>  =C2=A0dmam_alloc_attrs+0x68/0xb8
>  =C2=A0sil24_port_start+0x60/0xe0
>  =C2=A0ata_host_start.part.32+0xbc/0x208
>  =C2=A0ata_host_activate+0x64/0x150
>  =C2=A0sil24_init_one+0x1e8/0x268
>  =C2=A0local_pci_probe+0x3c/0xa0
>  =C2=A0pci_device_probe+0x128/0x1c8
>  =C2=A0really_probe+0x138/0x2d0
>  =C2=A0__driver_probe_device+0x78/0xd8
>  =C2=A0driver_probe_device+0x40/0x110
>  =C2=A0__driver_attach+0xcc/0x118
>  =C2=A0bus_for_each_dev+0x68/0xc8
>  =C2=A0driver_attach+0x20/0x28
>  =C2=A0bus_add_driver+0x168/0x1f8
>  =C2=A0driver_register+0x60/0x110
>  =C2=A0__pci_register_driver+0x5c/0x68
>  =C2=A0sil24_pci_driver_init+0x20/0x28
>  =C2=A0do_one_initcall+0x84/0x450
>  =C2=A0kernel_init_freeable+0x31c/0x38c
>  =C2=A0kernel_init+0x20/0x120
>  =C2=A0ret_from_fork+0x10/0x18
> Code: d37be885 6b01007f 52800004 540000a2 (f8656813)
> ---[ end trace 4ba4f0c9c48711a1 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=3D0x0000000b
>=20
> It looks that some changes to the scatterlist structures are missing=20
> outside of the lib/scatterlist.c.
>=20
> For now I would suggest to revert this change.

I see a very similar crash on Tegra during the HDA driver's probe.

Leon, let me know if you need a tester for a replacement patch.

Thanks,
Thierry

--k6cvBpDdjY2dq+5c
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmDcrXIACgkQ3SOs138+
s6FhdxAAj3EkX+HHRXHebKLx4fKYxCviBAZT2HFs5aBw0/JsZnsONBmup94Hu6oT
eDWmuaKbQHiah1JQBuO/pcP5SiYzo46khUmCjcrTLH0/R/F4D+MiozEl1b+u6p+I
c/7L2bRE+qvPp34srQeN5sXmr9fYwyi5NsS/h2dy1TdsfmVMmiI14uIkv5bbB1bL
mMixQGYV1GeJEbgI9/93ybpaA7ksE8DEq/Yhn6vrL/qKKwMN2gI1b6xZlzrawzZE
Vtt+KSfxXfCuLGiitdwLzl75p6RGVmfxxd9HzRfLZU4V5Qzo4ZKiGP9QJQCWYyeW
6JB2bRx9q0S64srNf4ePmnpbC3COnwDuGH3YuAkZeh5QAE/OqQ8NrlKKUqe+XGXj
WcfzkPPMoHoJBXAz3am1ul+AMcvLxzEJq/YHJhMPaQ+EGz3oqBk7ijzSdtsFpM6o
FQo+TDtMA45sTGNGv7TCqUzJy7mkQbkgExF1J6mXECCAwH8kZAfRfIBIHz0/3BDV
xG5EXtTREeARBhsDkLoIlamjy7W0a5qn9P71euuV3w22y3dD7ir4W9FuyoX92HoZ
4dagb5SaCDGC171Z5nD/qLRH88nUytOzs7GAo0K8bG+KzYK9ql0f3/5+E/ZM28Bv
OEHUZe5iHid5LDkwyrMgBrlICgvh5f7PTqAuFODFnCyVCZtezP8=
=3wNO
-----END PGP SIGNATURE-----

--k6cvBpDdjY2dq+5c--