| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 14 Jun 2017 03:18:31 +0000
From: "Chen, Xiaoguang" <xiaoguang.chen@...el.com>
To: Alex Williamson <alex.williamson@...hat.com>
CC: "Tian, Kevin" <kevin.tian@...el.com>,
"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"zhenyuw@...ux.intel.com" <zhenyuw@...ux.intel.com>,
"chris@...is-wilson.co.uk" <chris@...is-wilson.co.uk>,
"Lv, Zhiyuan" <zhiyuan.lv@...el.com>,
"intel-gvt-dev@...ts.freedesktop.org"
<intel-gvt-dev@...ts.freedesktop.org>,
"Wang, Zhi A" <zhi.a.wang@...el.com>,
"kraxel@...hat.com" <kraxel@...hat.com>
Subject: RE: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>-----Original Message-----
>From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@...ts.freedesktop.org] On
>Behalf Of Alex Williamson
>Sent: Wednesday, June 14, 2017 11:06 AM
>To: Chen, Xiaoguang <xiaoguang.chen@...el.com>
>Cc: Tian, Kevin <kevin.tian@...el.com>; intel-gfx@...ts.freedesktop.org; linux-
>kernel@...r.kernel.org; zhenyuw@...ux.intel.com; chris@...is-wilson.co.uk; Lv,
>Zhiyuan <zhiyuan.lv@...el.com>; intel-gvt-dev@...ts.freedesktop.org; Wang, Zhi
>A <zhi.a.wang@...el.com>; kraxel@...hat.com
>Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations
>
>On Wed, 14 Jun 2017 02:53:24 +0000
>"Chen, Xiaoguang" <xiaoguang.chen@...el.com> wrote:
>
>> >-----Original Message-----
>> >From: Alex Williamson [mailto:alex.williamson@...hat.com]
>> >Sent: Wednesday, June 14, 2017 5:25 AM
>> >To: Chen, Xiaoguang <xiaoguang.chen@...el.com>
>> >Cc: kraxel@...hat.com; chris@...is-wilson.co.uk; intel-
>> >gfx@...ts.freedesktop.org; linux-kernel@...r.kernel.org;
>> >zhenyuw@...ux.intel.com; Lv, Zhiyuan <zhiyuan.lv@...el.com>;
>> >intel-gvt- dev@...ts.freedesktop.org; Wang, Zhi A
>> ><zhi.a.wang@...el.com>; Tian, Kevin <kevin.tian@...el.com>
>> >Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf
>> >operations
>> >
>> >On Fri, 9 Jun 2017 14:50:40 +0800
>> >Xiaoguang Chen <xiaoguang.chen@...el.com> wrote:
>> >
>> >> Here we defined a new ioctl to create a fd for a vfio device based
>> >> on the input type. Now only one type is supported that is a dma-buf
>> >> management fd.
>> >> Two ioctls are defined for the dma-buf management fd: query the
>> >> vfio vgpu's plane information and create a dma-buf for a plane.
>> >>
>> >> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@...el.com>
>> >> ---
>> >> include/uapi/linux/vfio.h | 58
>> >> +++++++++++++++++++++++++++++++++++++++++++++++
>> >> 1 file changed, 58 insertions(+)
>> >>
>> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> >> index ae46105..24427b7 100644
>> >> --- a/include/uapi/linux/vfio.h
>> >> +++ b/include/uapi/linux/vfio.h
>> >> @@ -502,6 +502,64 @@ struct vfio_pci_hot_reset {
>> >>
>> >> #define VFIO_DEVICE_PCI_HOT_RESET _IO(VFIO_TYPE, VFIO_BASE + 13)
>> >>
>> >> +/**
>> >> + * VFIO_DEVICE_GET_FD - _IO(VFIO_TYPE, VFIO_BASE + 14, __u32)
>> >> + *
>> >> + * Create a fd for a vfio device based on the input type
>> >> + * Vendor driver should handle this ioctl to create a fd and
>> >> +manage the
>> >> + * life cycle of this fd.
>> >> + *
>> >> + * Return: a fd if vendor support that type, -errno if not
>> >> +supported */
>> >> +
>> >> +#define VFIO_DEVICE_GET_FD _IO(VFIO_TYPE, VFIO_BASE + 14)
>> >> +
>> >> +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;
>> >> +};
>> >> +
>> >> +#define VFIO_DEVICE_DMABUF_MGR_FD 0 /* Supported fd types */
>> >
>> >Move this #define up above vfio_vgpu_plane_info to associate it with
>> >the VFIO_DEVICE_GET_FD ioctl.
>> OK.
>>
>> >
>> >> +
>> >> +/*
>> >> + * VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15,
>> >> + * struct vfio_vgpu_query_plane)
>> >> + * Query plane information
>> >> + */
>> >> +struct vfio_vgpu_query_plane {
>> >> + __u32 argsz;
>> >> + __u32 flags;
>> >> + struct vfio_vgpu_plane_info plane_info;
>> >> + __u32 plane_id;
>> >> + __u32 padding;
>> >
>> >This padding doesn't make sense.
>> This padding is still needed if we do not move the plane_id into
>vfio_vgpu_plane_info. Right?
>
>I don't see why this padding is ever needed, can you explain?
I thought we add the padding to make sure the structure layout is the same in both 32bit and 64bit systems.
Am I right?
>Does the structure
>not being a multiple of 8 bytes affect any of the offsets within the structure?
No. it will not affect any offsets in the structure.
>
>> >> +};
>> >> +
>> >> +#define VFIO_DEVICE_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
>> >> +
>> >> +/*
>> >> + * VFIO_DEVICE_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16,
>> >> + * struct
>> >vfio_vgpu_create_dmabuf)
>> >> + *
>> >> + * Create a dma-buf for a plane
>> >> + */
>> >> +struct vfio_vgpu_create_dmabuf {
>> >> + __u32 argsz;
>> >> + __u32 flags;
>> >> + struct vfio_vgpu_plane_info plane_info;
>> >> + __s32 fd;
>> >> + __u32 plane_id;
>> >> +};
>> >
>> >Both of these have a plane_id, should plane_id simply replace the padding in
>> >plane_info?
>> Precisely speaking plane_id does not belong to the plane info. All the other
>information are decoded from plane except plane id.
>
>Ok, let's keep is separate then. Thanks,
>
>Alex
>
>> >If not, let's at least put them in the same order so that plane_id is
>> >after plane_info for both structs.
>> Ok.
>>
>> >
>> >> +
>> >> +#define VFIO_DEVICE_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16)
>> >
>> >I don't think these should be named just VFIO_DEVICE_foo, that
>> >implies they're ioctls on the vfio device fd, they're not. They need
>> >to be associated both in name and more complete descriptions as
>> >ioctls to the fd returned from a request for a
>> >VFIO_DEVICE_DMABUF_MGR_FD. Perhaps
>VFIO_DMABUF_MGR_QUERY_PLANE and
>> >VFIO_DMABUF_MGR_CREATE_DMABUF. I'm also not sure why we're using
>"vgpu" in the structure names here either, the ioctls aren't named after vgpus.
>> >Aren't these rather generic to graphics dmabufs, not specifically vgpus?
>> Make sense. I will change the names.
>>
>> Thanks,
>> >
>> >Alex
>> >
>> >> +
>> >> /* -------- API for Type1 VFIO IOMMU -------- */
>> >>
>> >> /**
>>
>
>_______________________________________________
>intel-gvt-dev mailing list
>intel-gvt-dev@...ts.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
Powered by blists - more mailing lists