Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751999AbdFAQi7 (ORCPT ); Thu, 1 Jun 2017 12:38:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57842 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751198AbdFAQi5 (ORCPT ); Thu, 1 Jun 2017 12:38:57 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0657F8553C Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=alex.williamson@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 0657F8553C Date: Thu, 1 Jun 2017 10:38:53 -0600 From: Alex Williamson To: "Chen, Xiaoguang" Cc: Kirti Wankhede , Gerd Hoffmann , "chris@chris-wilson.co.uk" , "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" , "Tian, Kevin" Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations Message-ID: <20170601103853.5ead4d39@w520.home> In-Reply-To: References: <1495874332-2851-1-git-send-email-xiaoguang.chen@intel.com> <1495874332-2851-5-git-send-email-xiaoguang.chen@intel.com> <1496042420.21582.3.camel@redhat.com> <93758f79-8076-9644-a8a7-6e2ebfd91fee@nvidia.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.28]); Thu, 01 Jun 2017 16:38:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3095 Lines: 89 On Thu, 1 Jun 2017 03:01:28 +0000 "Chen, Xiaoguang" wrote: > Hi Kirti, > > >-----Original Message----- > >From: Kirti Wankhede [mailto:kwankhede@nvidia.com] > >Sent: Thursday, June 01, 2017 1:23 AM > >To: Chen, Xiaoguang ; Gerd Hoffmann > >; alex.williamson@redhat.com; chris@chris-wilson.co.uk; > >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 ; Tian, Kevin > > > >Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations > > > > > > > >On 5/31/2017 11:48 AM, Chen, Xiaoguang wrote: > >> Hi, > >> > >>> -----Original Message----- > >>> From: Gerd Hoffmann [mailto:kraxel@redhat.com] > >>> Sent: Monday, May 29, 2017 3:20 PM > >>> To: Chen, Xiaoguang ; > >>> alex.williamson@redhat.com; chris@chris-wilson.co.uk; 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 > >>> ; Tian, Kevin > >>> Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf > >>> operations > >>> > >>>> +struct vfio_vgpu_dmabuf_info { > >>>> + __u32 argsz; > >>>> + __u32 flags; > >>>> + struct vfio_vgpu_plane_info plane_info; > >>>> + __s32 fd; > >>>> + __u32 pad; > >>>> +}; > >>> > >>> Hmm, now you have argsz and flags twice in vfio_vgpu_dmabuf_info ... > >>> > >>> I think we should have something like this: > >>> > >>> struct vfio_vgpu_plane_info { > >>> __u64 start; > >>> __u64 drm_format_mod; > >>> __u32 drm_format; > >>> __u32 width; > >>> __u32 height; > >>> __u32 stride; > >>> __u32 size; > >>> __u32 x_pos; > >>> __u32 y_pos; > >>> __u32 padding; > >>> }; > >>> > >>> struct vfio_vgpu_query_plane { > >>> __u32 argsz; > >>> __u32 flags; > >>> struct vfio_vgpu_plane_info plane_info; > >>> __u32 plane_id; > >>> __u32 padding; > >>> }; > >>> > >>> struct vfio_vgpu_create_dmabuf { > >>> __u32 argsz; > >>> __u32 flags; > >>> struct vfio_vgpu_plane_info plane_info; > >>> __u32 plane_id; > >>> __s32 fd; > >>> }; > >> Good suggestion will apply in the next version. > >> Thanks for review :) > >> > > > >Can you define what are the expected values of 'flags' would be? > Flags is not used in this case. It is defined to follow the rules of vfio ioctls. An important note about flags, the vendor driver must validate it. If they don't and the user passes an arbitrary value there, then we have a backwards compatibility issue with ever attempting to use the flags field. The user passing in a flag unknown to the vendor driver should return an -EINVAL response. In this case, we haven't defined any flags, so the vendor driver needs to force the user to pass zero. Thanks, Alex