From patchwork Wed Sep 27 09:03:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Zhang, Tina" X-Patchwork-Id: 9973501 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 04F1460365 for ; Wed, 27 Sep 2017 09:04:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EC2EB1FFB2 for ; Wed, 27 Sep 2017 09:04:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E09B728F5E; Wed, 27 Sep 2017 09:04:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id ACEE01FFB2 for ; Wed, 27 Sep 2017 09:04:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B36D76E689; Wed, 27 Sep 2017 09:04:03 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9EBBE6E689; Wed, 27 Sep 2017 09:04:02 +0000 (UTC) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP; 27 Sep 2017 02:04:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,444,1500966000"; d="scan'208";a="316733279" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 27 Sep 2017 02:04:01 -0700 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 27 Sep 2017 02:04:01 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 27 Sep 2017 02:04:01 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.159]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Wed, 27 Sep 2017 17:03:59 +0800 From: "Zhang, Tina" To: Gerd Hoffmann , "zhenyuw@linux.intel.com" , "Wang, Zhi A" , "Tian, Kevin" Thread-Topic: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation Thread-Index: AQHTGAyPZC0Qp0s/BEOXanksUqYfrKLGd0yAgAIXUSA= Date: Wed, 27 Sep 2017 09:03:59 +0000 Message-ID: <237F54289DF84E4997F34151298ABEBC7C5E5ADC@SHSMSX101.ccr.corp.intel.com> References: <1503051696-5158-1-git-send-email-tina.zhang@intel.com> <1503051696-5158-6-git-send-email-tina.zhang@intel.com> <1506409923.32072.9.camel@redhat.com> In-Reply-To: <1506409923.32072.9.camel@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: Daniel Vetter , "intel-gfx@lists.freedesktop.org" , "intel-gvt-dev@lists.freedesktop.org" , "Lv, Zhiyuan" Subject: Re: [Intel-gfx] [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP It's really tricky to handle the cached dmabuf_obj's life-cycle without touching other kernel modules (e.g. i915 or dmabuf).The proposed two ioctls will be helpful. So, there is a problem about the releasing cached dmabuf_obj. We cannot rely on the drm_i915_gem_object_ops.release() to release the cached dmabuf_obj, as this release operation is running in another thread, which leads to a racing condition and tricky to be solved without touching other modules. So, in order to solve that kind of problem, I’d like to add one more ioctl, which is used for user mode to close the dmabuf_obj. Including the proposed two ioctls, the incremental patch is like this: 2. VFIO_DEVICE_GET_GFX_DMABUF: is used for user mode to ask kernel part which interface it likes the buffer to be wrapped. Actually, I think there could be a bunch of types, including DMBUF type. So, maybe we can change the IOCTL's name to some generic name and use flags field to distinguish them. In dmabuf case, a new dmabuf will be created each time with this ioctl invoked, and installed with a new fd. The dmabuf is just a wrapper of kernel dmabuf_obj distinguished by dmabuf_id. So there could be more than one dmabuf as the wrappers of the same dmabuf_obj, if this ioctl was invoked with the same dmabuf_id many times. The dmabuf and its fd is fully controlled by user mode. And the VFIO_DEVICE_CLOSE_BUF can guarantee the referenced dmabuf_obj will be released at last, after all the dmabufs are released. 3. VFIO_DEVICE_CLOSE_BUF: is used for user mode to tell kernel part to release that buffer. In dmabuf case, this ioctl won't release the dmabuf_obj at once. It just decreases the reference count of the dmabuf_obj and make sure this dmabuf_obj won't be reused through VFIO_DEVICE_QUERY_GFX_PLANE again. At last, after all the referencing dmabuf are released by user mode, the dmabuf_obj will be released by kernel. Thanks, BR, Tina > -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Tuesday, September 26, 2017 3:12 PM > To: Zhang, Tina ; zhenyuw@linux.intel.com > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org; Alex > Williamson ; Daniel Vetter > > Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation > > Hi, > > [ bringing a private discussion back to the list ] > > > The dma-buf's life cycle is handled by user mode and tracked by > > kernel. > > The returned fd in struct vfio_device_query_gfx_plane can be a new fd > > or an old fd of a re-exported dma-buf. Host user mode can check the > > value of fd and to see if it needs to create new resource according to > > the new fd or just use the existed resource related to the old fd. > > Ok, this idea has a fundamental flaw: The life cycle of the dma-buf and the file > handle are not identical. The dma-buf can exist longer than the file handle in > case other references to the dma-buf exist. So when trying to use the file handle > as identifier for the dma-buf you'll end up with all sorts of strange effects. > > So, I'd suggest to use a id instead, and add a ioctl to get a dmabuf for a given id > (incremental patch): > > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -538,12 +538,22 @@ struct vfio_device_gfx_plane_info { >         __u32 y_pos;    /* vertical position of cursor plane, upper left corner in > lines*/ >         union { >                 __u32 region_index;     /* region index */ > -               __s32 fd;       /* dma-buf fd */ > +               __u32 dmabuf_id;        /* dma-buf id */ >         }; >  }; > >  #define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14) > > +struct vfio_device_gfx_dmabuf_fd { > +       __u32 argsz; > +       __u32 flags; > +       /* in */ > +       __u32 dmabuf_id; > +       /* out */ > +       __s32 dmabuf_fd; > +}; > + > +#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15) > >  /* -------- API for Type1 VFIO IOMMU -------- */ > > [ no changes for a region-based display ] > > git branch, kernel, with updated dmabuf patch: > https://www.kraxel.org/cgit/linux/log/?h=gvt-dmabuf-v14 > > qemu branch: > https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu > > cheers, > Gerd diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h index bf40f7b..6aa6860 100644 --- a/linux-headers/linux/vfio.h +++ b/linux-headers/linux/vfio.h @@ -538,12 +538,33 @@ struct vfio_device_gfx_plane_info { __u32 y_pos; /* vertical position of cursor plane, upper left corner in lines*/ union { __u32 region_index; /* region index */ - __s32 fd; /* dma-buf fd */ + __s32 dmabuf_id; /* dma-buf fd */ }; }; #define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14) +struct vfio_device_gfx_dmabuf_fd { + __u32 argsz; + __u32 flags; + /* in */ + __u32 dmabuf_id; + /* out */ + __s32 dmabuf_fd; +}; + +#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15) + + +struct vfio_device_gfx_buffer { + __u32 argsz; + __u32 flags; + /* in */ + __u32 id; +}; + +#define VFIO_DEVICE_CLOSE_BUF _IO(VFIO_TYPE, VFIO_BASE + 16) And here are some details: 1. VFIO_DEVICE_QUERY_GFX_PLANE: is used for user mode to ask kernel part to create a buffer (in dmabuf case is dmabuf_obj in kernel) and return the buffer info.