From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH v3 1/2] drm/gem: drm_gem_dumb_map_offset(): reject dma-buf Date: Fri, 18 Aug 2017 16:37:35 -0700 Message-ID: <878tigphvk.fsf@eliezer.anholt.net> References: <1502986891-36764-1-git-send-email-noralf@tronnes.org> <1502986891-36764-2-git-send-email-noralf@tronnes.org> <20170818074656.xawukspyerve6wnb@phenom.ffwll.local> <1de97ff3-44d5-aeac-e03d-4976e455ab67@tronnes.org> <87mv6wdb9a.fsf@eliezer.anholt.net> <20170818201730.62slltrw4fwlj5qe@phenom.ffwll.local> <87shgopqca.fsf@eliezer.anholt.net> <20170818210150.mwn5npajcsgbjbsb@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0701269075==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id A96956E033 for ; Fri, 18 Aug 2017 23:43:54 +0000 (UTC) In-Reply-To: <20170818210150.mwn5npajcsgbjbsb@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: narmstrong@baylibre.com, daniel.vetter@ffwll.ch, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, thierry.reding@gmail.com, laurent.pinchart@ideasonboard.com, daniel.vetter@intel.com, marex@denx.de, boris.brezillon@free-electrons.com, abrodkin@synopsys.com, linux@armlinux.org.uk, z.liuxinliang@hisilicon.com, kong.kongxinwei@hisilicon.com, tomi.valkeinen@ti.com, airlied@redhat.com, puck.chen@hisilicon.com, jsarha@ti.com, vincent.abriou@st.com, alison.wang@freescale.com, sw0312.kim@samsung.com, philippe.cornu@st.com, yannick.fertre@st.com, kyungmin.park@samsung.com, zourongrong@gmail.com, maxime.ripard@free-electrons.com, shawnguo@kernel.org, kraxel@redhat.com List-Id: dri-devel@lists.freedesktop.org --===============0701269075== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Daniel Vetter writes: > On Fri, Aug 18, 2017 at 01:34:45PM -0700, Eric Anholt wrote: >> Daniel Vetter writes: >>=20 >> > On Fri, Aug 18, 2017 at 10:41:21AM -0700, Eric Anholt wrote: >> >> Noralf Tr=C3=B8nnes writes: >> >> > Den 18.08.2017 09.46, skrev Daniel Vetter: >> >> >> On Thu, Aug 17, 2017 at 06:21:30PM +0200, Noralf Tr=C3=B8nnes wrot= e: >> >> >>> Reject mapping an imported dma-buf since is's an invalid use-case. >> >> >>> >> >> >>> Cc: Philipp Zabel >> >> >>> Cc: Laurent Pinchart >> >> >>> Cc: Sean Paul >> >> >>> Cc: Daniel Vetter >> >> >>> Signed-off-by: Noralf Tr=C3=B8nnes >> >> >> I think acks from someone using mali would be good too. amdgpu alr= eady has >> >> >> such checks, so I think on the desktop side we're ok. >> >> >> >> >> >> Acked-by: Daniel Vetter >> >> >> >> >> >> But I think this one here definitely needs a few more acks. I coul= d break >> >> >> uabi if we're unlucky, so let's not rush it. >> >> > >> >> > Ok, I've CC'ed the affected parties to increase the odds that they = look >> >> > at this. These are the drivers using drm_gem_dumb_map_offset() >> >> > (hopefully I got the list right): >> >>=20 >> >> If I understand the affected path right, this would break the PL111+V= C4 >> >> combination: PL111 makes (dumb) buffers for scanout, and VC4 imports >> >> them and uses them for rendering. A vc4 glReadPixels of the window >> >> system buffer would map it and fail. >> > >> > It only rejects the map call on dumb buffers, and mmap on imported dma= -buf >> > tends to not really work well, or at least break a few abstractions. A= re >> > you sure this works currently? >>=20 >> OK, that's right -- vc4 would be doing its "native" map call (the same >> code), not dumb map. >>=20 >> Furthermore, I had it backwards (I had written things both ways at >> different points, iirc). We have VC4 making the buffers and PL111 >> dma-buf importing them. I don't see X11 mapping those buffers if glamor >> is enabled, so this should be OK for vc4. >>=20 >> GBM's dumb mapping looks safe to me. X11 does some dumb maps, but I >> don't think any of those would be on imports. > > Yeah the idea is only to lock down the dumb mmap and make sure abi abuse > (which might work on a specific combo of exporters/kms drivers) is caught > for this generic interface. dumb really should only be used for > unaccelarated rendering on exactly that driver. > > So ack from you? I'm hesitant, but that's mostly because I don't see the reason we're trying to lock it down, so it just looks like a chance of breakage from my perspective. However, given that some drivers are banning it already, let's make things consistent until we find we need to relax it globally. Acked-by: Eric Anholt --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlmXekAACgkQtdYpNtH8 nuh2NA/+K7Kbb9UkWo1SC5iirRgzHn3o+J1ue8yk2mGmu179HOTh9Z8iij+As75C J4tVr2zP6TdSPnblQtXPqY2wlg6uwfmnjA/MmWjxwxPYNJDwf2CYsi0smhz0cVec XiyS0svO8zo01QA+ETIWYI9elu9pOtrAteTLgf15H+AwUFzAy1TPfleTC9c8vfAC cIrlsYy9qhqsEZp9bNJQNSQfNFlL099uXSix4lTP773zD3dAM3SQdiF5XTpOnoJG mAf0MmCO1R1oRpxUk06Oxa7Txi+gJ3iKi5m7H9Bqmxi+zwgc8zc1oNL+NlX+8vTs QN33PmIMsO+xLNAJwECz4Ihyge2+5yeKMxhCXgJHSUqCW850GHViAS9Rltw3f/Dk pzgyZcmoOm4188ZyzX8/vAh39lNX2nvj2RI5T1vX7dUdQPuQrNsXjPA6ckg9edQ8 +qevwqsKmKvJF2+zgyxXTCIwxNvBte+Dn22NcNSJWxsMLQS+CWbYoAMuk8RyUliD 1e6EkUTXO5tCuTD3jdkPfvj3EXz5DzejXrzJFWn4ZMAE3Xco5ORcAmLjus4Is94F 1EMooi+gyMibYJFJRC181i8VdmqOHWWSXnkYl8Tt270ocJdchHxQqQeIXjzZsyeU wlYjidQA/oSmQF/nSMlV1bM67G+lBq1Ry3jf2cWDyVQUPMuXAVQ= =w17m -----END PGP SIGNATURE----- --=-=-=-- --===============0701269075== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0701269075==--