From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752488AbdEEPLV (ORCPT ); Fri, 5 May 2017 11:11:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50734 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751384AbdEEPLT (ORCPT ); Fri, 5 May 2017 11:11:19 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EB1CD9D40B Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=alex.williamson@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com EB1CD9D40B Date: Fri, 5 May 2017 09:11:15 -0600 From: Alex Williamson To: Gerd Hoffmann Cc: "Chen, Xiaoguang" , "Tian, Kevin" , "intel-gfx@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "zhenyuw@linux.intel.com" , "Lv, Zhiyuan" , "intel-gvt-dev@lists.freedesktop.org" , "Wang, Zhi A" Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf Message-ID: <20170505091115.7a680636@t450s.home> In-Reply-To: <1493967331.371.53.camel@redhat.com> References: <1493372130-27727-1-git-send-email-xiaoguang.chen@intel.com> <1493372130-27727-7-git-send-email-xiaoguang.chen@intel.com> <1493718658.8581.82.camel@redhat.com> <20170504100833.199bc8ba@t450s.home> <1493967331.371.53.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 05 May 2017 15:11:19 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 05 May 2017 08:55:31 +0200 Gerd Hoffmann wrote: > Hi, > > > > >>Hmm, that looks like a rather strange way to return a file descriptor. > > > >> > > > >>What is the reason to not use ioctls on the vfio file handle, like > > > >>older version of these patches did? > > > >If I understood correctly that Alex prefer not to change the ioctls on the vfio file > > > >handle like the old version. > > > >So I used this way the smallest change to general vfio framework only adding a > > > >subregion definition. > > > > I think I was hoping we could avoid a separate file descriptor > > altogether and use a vfio region instead. > > What exactly did you have in mind? Put the framebuffer information > (struct intel_vgpu_dmabuf) into the vfio region, then access it using > read/write/mmap? Yeah, that was my hope. Adding a new file descriptor means we have one more reference floating around complicating the life cycle of the device, group, and container. Furthermore this one is really only visible to the mdev vendor driver, so we can't rely on vfio-core, the vendor driver will need to consider the reference when releasing the device. > > However, it was explained > > previously why this really needs to be a separate fd and I agree that > > using a region to expose an fd is really awkward. > > Now with this patchset we have *two* kinds of separate file handles. > First the anon-fd created by reading from the region. This is then used > to run the intel ioctls on, which in turn create the other kind of file > handle (dma-buf-fd). > > The dma-buf-fd really needs to be a separate fd, because it gets passed > around as handle and because this is the way dma-bufs work (guess this > is the discussion you are referring to). Yep, we're going to need to trust the vendor driver to manage it, we have lots of places where we need to trust the vendor driver for an mdev device, unfortunately. > I can't see a compelling reason for the anon-fd though. I suspect this > was done due to a misunderstanding ... Yeah, vfio-core passes device ioctls to the vendor driver, so the vendor driver should be able to implement a VFIO_DEVICE_GVT_GET_DMABUF_FD ioctl direclty. Ideally maybe this isn't even GVT specific, and we'd s/GVT_//. Thanks, Alex