From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Disallow pin ioctl completely for kms drivers Date: Mon, 23 Feb 2015 15:52:00 -0800 Message-ID: <54EBBD20.2090205@virtuousgeek.org> References: <1416825025-11287-1-git-send-email-daniel.vetter@ffwll.ch> <20141124103529.GF28099@nuc-i3427.alporthouse.com> <20141124141005.GR25711@phenom.ffwll.local> <20141124141304.GA17410@nuc-i3427.alporthouse.com> <54EB9BD5.3070308@virtuousgeek.org> <20150223234039.GZ24485@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150223234039.GZ24485@phenom.ffwll.local> Sender: stable-owner@vger.kernel.org To: Daniel Vetter Cc: Chris Wilson , Intel Graphics Development , stable@vger.kernel.org, Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On 02/23/2015 03:40 PM, Daniel Vetter wrote: > On Mon, Feb 23, 2015 at 01:29:57PM -0800, Jesse Barnes wrote: >> On 11/24/2014 06:13 AM, Chris Wilson wrote: >>> On Mon, Nov 24, 2014 at 03:10:05PM +0100, Daniel Vetter wrote: >>>> On Mon, Nov 24, 2014 at 10:35:29AM +0000, Chris Wilson wrote: >>>>> Pinning is a useful tool and it would also be useful to have again on >>>>> gen6+. >>>> >>>> I think softpin or similar is doable with ppgtt. But with shared ggtt I'm >>>> not really enthusiastic about providing this kind of rope to userspace. >>>> And softpin is a different type of pinning, so we don't really lose >>>> anything by ripping out the userspace hard pinning ioctls. >>> >>> I am not talking about softpin, but being able to pin memory and a GGTT >>> binding itself is useful. >> >> I see you merged this over Chris's objections and then shot down his >> revert. I'm not clear on why you're so opposed to the pin ioctl? It's >> a privileged op and definitely has its uses as Chris has repeatedly >> pointed out. >> >> Why so insistent on dropping this particular ioctl? Do you see it >> causing actual problems? Or do you just like preventing userspace from >> doing things you don't agree with? >> >> (Sorry, catching up on ancient backlog from intel-gfx, so maybe there's >> a thread I missed when re-looking at this. If so, please point me at it.) > > People are way too happy to abuse it instead of using dma-buf. And at > least some of the uses sna has also cause a bunch of problems with being a > bit too clever around reloc handling (so we essentially _have_ to take the > toys away since giving it back would cause regressions). Some interfaces are more dangerous than others. But that doesn't mean they're necessarily bad. > If there's a real users then we can look at this again imo, but I think > most things are better solved with proper kernel interfaces since in the > end the kernel does mm for the gpu, and if userspace interferes we can't > do that. > > So overall my answer is: > - re-enable will cause regressions Which regressions? In SNA? It sounded like Chris was the one requesting this here. And really, dropping pin altogether was a big regression in the ABI to begin with and probably shouldn't have been allowed (the one back in 2013; I think both Chris and Ben objected back then too). > - I don't see a justified user What about SNA? What about debug? Yes there's an alternative in the SNA case, but Chris mentioned it had a huge perf hit. And fwiw the Beignet team is using this too, so at the very least it needs to work on aliasing PPGTT on gen7/7.5. > - we should never have allowed this with kms to begin with, it was an > oversight. Not sure about that; as Chris mentioned, mlock() has uses too. It needs to be limited, obviously, and can cause trouble if you're not careful. But that's not a reason to disallow it or remove it altogether. Anyway, the patches have no r-bs or acks, only nacks going back to gen6, and you're still merging these. That's what's not sitting well with me. Jesse