drm/i915/gvt: Dma-buf support for GVT-g
v5 cover drm/i915/gvt: Dma-buf support for GVT-g
v4->v5:
1) fix bug while checking whether the gem obj is gvt's dma-buf when user change caching mode or domains. Add a helper function to do it.
2) add definition for the query plane and create dma-buf.
v3->v4:
1) fix bug while checking whether the gem obj is gvt's dma-buf when set caching mode or doamins.
v2->v3:
1) add a field gvt_plane_info in the drm_i915_gem_obj structure to save the decoded plane information to avoid look up while need the plane info.
2) declare a new flag I915_GEM_OBJECT_IS_GVT_DMABUF in drm_i915_gem_object to represent the gem obj for gvt's dma-buf. The tiling mode, caching mode and domains can not be changed for this kind of gem object.
3) change dma-buf related information to be more generic. So other vendor can use the same interface.
v1->v2:
1) create a management fd for dma-buf operations.
2) alloc gem object's backing storage in gem obj's get_pages() callback.
This patch set adds the dma-buf support for intel GVT-g. dma-buf is a uniform mechanism to share DMA buffers across different devices and sub-systems.
dma-buf for intel GVT-g is mainly used to share the vgpu's framebuffer to other users or sub-systems so they can use the dma-buf to show the desktop of a vm which uses intel vgpu.
The main idea is we create a gem object and set vgpu's framebuffer as the backing storage of this gem object. And associate this gem obj to a dma-buf object then export this dma-buf at the meantime generate a file descriptor for this dma-buf. Finally deliver this file descriptor to user space. And user can use this dma-buf fd to do render or other operations.
User need to create a fd(for intel GVT-g dma-buf support it is a:dma-buf management fd) then user can use this fd to query the plane information or create a dma-buf. The life cycle of this fd is managed by GVT-g user do not need to care about that.
We have an example program on how to use the dma-buf. You can download the program to have a try. Good luck :)
git repo: https://github.com/01org/igvtg-qemu branch:kvmgt_dmabuf_example
Xiaoguang Chen (5):
drm/i915/gvt: Extend the GVT-g architecture to support vfio device
region
drm/i915/gvt: OpRegion support for GVT-g
drm/i915/gvt: Frame buffer decoder support for GVT-g drm/i915/gvt: Dmabuf support for GVT-g
drm/i915/gvt: Adding interface so user space can get the dma-buf
drivers/gpu/drm/i915/gvt/Makefile | 3 +- drivers/gpu/drm/i915/gvt/display.c | 2 +- drivers/gpu/drm/i915/gvt/display.h | 2 + drivers/gpu/drm/i915/gvt/dmabuf.c | 277 +++++++++++++++++++ drivers/gpu/drm/i915/gvt/dmabuf.h | 32 +++ drivers/gpu/drm/i915/gvt/fb_decoder.c | 479 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/fb_decoder.h | 168 ++++++++++++ drivers/gpu/drm/i915/gvt/gvt.c | 2 + drivers/gpu/drm/i915/gvt/gvt.h | 6 + drivers/gpu/drm/i915/gvt/kvmgt.c | 225 +++++++++++++++- drivers/gpu/drm/i915/gvt/opregion.c | 8 +- drivers/gpu/drm/i915/i915_gem.c | 8 + drivers/gpu/drm/i915/i915_gem_object.h | 9 + include/uapi/linux/vfio.h | 50 +++- 14 files changed, 1264 insertions(+), 7 deletions(-) create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.h create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h
Signature · 6 lines
2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v5 1/5 drm/i915/gvt: Extend the GVT-g architecture to support vfio device region
--- drivers/gpu/drm/i915/gvt/kvmgt.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/gvt/kvmgt.c +18 -3
@@ -53,11 +53,21 @@ static const struct intel_gvt_ops *intel_gvt_ops;#define VFIO_PCI_INDEX_TO_OFFSET (index) ((u64)(index) << VFIO_PCI_OFFSET_SHIFT)#define VFIO_PCI_OFFSET_MASK (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1) +struct vfio_region ; +struct intel_vgpu_regops { +size_t (*rw)(struct intel_vgpu *vgpu,char *buf, +size_t count,loff_t *ppos,bool iswrite); +void (*release)(struct intel_vgpu *vgpu, +struct vfio_region *region); +}; +struct vfio_region {u32 type;u32 subtype;size_t size;u32 flags; +const struct intel_vgpu_regops *ops; +void *data; };struct kvmgt_pgfn {
@@ -642,7 +652,7 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,int ret = -EINVAL ; -if (index >=VFIO_PCI_NUM_REGIONS ) { +if (index >=VFIO_PCI_NUM_REGIONS + vgpu->vdev.num_regions) {gvt_vgpu_err ("invalid index: %u\n" , index);return -EINVAL ; }
@@ -676,8 +686,11 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,case VFIO_PCI_BAR5_REGION_INDEX :case VFIO_PCI_VGA_REGION_INDEX :case VFIO_PCI_ROM_REGION_INDEX : +break ;default : -gvt_vgpu_err ("unsupported region: %u\n" , index); + index -=VFIO_PCI_NUM_REGIONS ; +return vgpu->vdev.region[index].ops->rw (vgpu, buf, count, + ppos, is_write); }return ret ==0 ? count : ret;
@@ -940,7 +953,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, info.flags =VFIO_DEVICE_FLAGS_PCI ; info.flags |=VFIO_DEVICE_FLAGS_RESET ; - info.num_regions =VFIO_PCI_NUM_REGIONS ; + info.num_regions =VFIO_PCI_NUM_REGIONS + + vgpu->vdev.num_regions; info.num_irqs =VFIO_PCI_NUM_IRQS ; return copy_to_user((void __user *)arg, &info, minsz) ?
@@ -1061,6 +1075,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, }if (caps.size) { + info.flags |=VFIO_REGION_INFO_FLAG_CAPS ;if (info.argsz <sizeof (info) + caps.size) { info.argsz =sizeof (info) + caps.size; info.cap_offset =0 ;
Signature · 6 lines
2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v5 2/5 drm/i915/gvt: OpRegion support for GVT-g
In reply to Xiaoguang ChenOpRegion is needed to support display related operation for intel vgpu.
A vfio device region is added to intel vgpu to deliver the host OpRegion information to user space so user space can construct the OpRegion for vgpu.
--- drivers/gpu/drm/i915/gvt/kvmgt.c | 97 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/opregion.c | 8 ++- 2 files changed, 104 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/gvt/kvmgt.c +97 -0
@@ -53,6 +53,8 @@ static const struct intel_gvt_ops *intel_gvt_ops;#define VFIO_PCI_INDEX_TO_OFFSET (index) ((u64)(index) << VFIO_PCI_OFFSET_SHIFT)#define VFIO_PCI_OFFSET_MASK (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1) +#define OPREGION_SIGNATURE "IntelGraphicsMem" +struct vfio_region ;struct intel_vgpu_regops { size_t (*rw)(struct intel_vgpu *vgpu, char *buf,
@@ -436,6 +438,92 @@ static void kvmgt_protect_table_del(struct kvmgt_guest_info *info, } } +static size_t intel_vgpu_reg_rw_opregion (struct intel_vgpu *vgpu,char *buf, +size_t count,loff_t *ppos,bool iswrite) +{ +unsigned int i =VFIO_PCI_OFFSET_TO_INDEX (*ppos) - +VFIO_PCI_NUM_REGIONS ; +void *base = vgpu->vdev.region[i].data; +loff_t pos = *ppos &VFIO_PCI_OFFSET_MASK ; + +if (pos >= vgpu->vdev.region[i].size || iswrite) { +gvt_vgpu_err ("invalid op or offset for Intel vgpu OpRegion\n" ); +return -EINVAL ; + } + count =min (count, (size_t )(vgpu->vdev.region[i].size - pos)); +memcpy (buf, base + pos, count); + +return count; +} + +static void intel_vgpu_reg_release_opregion (struct intel_vgpu *vgpu, +struct vfio_region *region) +{ +memunmap (region->data); +} + +static const struct intel_vgpu_regops intel_vgpu_regops_opregion = { + .rw = intel_vgpu_reg_rw_opregion, + .release = intel_vgpu_reg_release_opregion, +}; + +static int intel_vgpu_register_reg (struct intel_vgpu *vgpu, +unsigned int type,unsigned int subtype, +const struct intel_vgpu_regops *ops, +size_t size,u32 flags,void *data) +{ +struct vfio_region *region; + + region =krealloc (vgpu->vdev.region, + (vgpu->vdev.num_regions +1 ) *sizeof (*region), +GFP_KERNEL ); +if (!region) +return -ENOMEM ; + + vgpu->vdev.region = region; + vgpu->vdev.region[vgpu->vdev.num_regions].type = type; + vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype; + vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops; + vgpu->vdev.region[vgpu->vdev.num_regions].size = size; + vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags; + vgpu->vdev.region[vgpu->vdev.num_regions].data = data; + vgpu->vdev.num_regions++; + +return 0 ; +} + +static int intel_vgpu_reg_init_opregion (struct intel_vgpu *vgpu) +{ +unsigned int addr; +void *base; +int ret; + + addr = vgpu->gvt->opregion.opregion_pa; +if (!addr || !(~addr)) +return -ENODEV ; + + base =memremap (addr,OPREGION_SIZE ,MEMREMAP_WB ); +if (!base) +return -ENOMEM ; + +if (memcmp (base,OPREGION_SIGNATURE ,16 )) { +memunmap (base); +return -EINVAL ; + } + + ret =intel_vgpu_register_reg (vgpu, +PCI_VENDOR_ID_INTEL |VFIO_REGION_TYPE_PCI_VENDOR_TYPE , +VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION , + &intel_vgpu_regops_opregion,OPREGION_SIZE , +VFIO_REGION_INFO_FLAG_READ , base); +if (ret) { +memunmap (base); +return ret; + } + +return ret; +} +static int intel_vgpu_create (struct kobject *kobj,struct mdev_device *mdev) {struct intel_vgpu *vgpu =NULL ;
@@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev) vgpu->vdev.mdev = mdev;mdev_set_drvdata (mdev, vgpu); + ret =intel_vgpu_reg_init_opregion (vgpu); +if (ret) { +gvt_vgpu_err ("create OpRegion failed\n" ); + goto out; + } + +gvt_dbg_core ("create OpRegion succeeded for mdev:%s\n" , +dev_name (mdev_dev (mdev))); +gvt_dbg_core ("intel_vgpu_create succeeded for mdev: %s\n" ,dev_name (mdev_dev (mdev))); ret =0 ;
drivers/gpu/drm/i915/gvt/opregion.c +7 -1
@@ -114,6 +114,7 @@ void intel_vgpu_clean_opregion(struct intel_vgpu *vgpu)int intel_vgpu_init_opregion (struct intel_vgpu *vgpu,u32 gpa) {int ret; +unsigned long pfn;gvt_dbg_core ("vgpu%d: init vgpu opregion\n" , vgpu->id);
@@ -127,7 +128,12 @@ int intel_vgpu_init_opregion(struct intel_vgpu *vgpu, u32 gpa) ret =map_vgpu_opregion (vgpu, true);if (ret)return ret; - } + }else { + pfn =intel_gvt_hypervisor_gfn_to_mfn (vgpu, gpa >>PAGE_SHIFT ); +vgpu_opregion (vgpu)->va =memremap (pfn <<PAGE_SHIFT , +INTEL_GVT_OPREGION_SIZE , +MEMREMAP_WB ); + }return 0 ; }
Signature · 6 lines
2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v5 3/5 drm/i915/gvt: Frame buffer decoder support for GVT-g
In reply to Xiaoguang Chendecode frambuffer attributes of primary, cursor and sprite plane
--- drivers/gpu/drm/i915/gvt/Makefile | 3 +- drivers/gpu/drm/i915/gvt/display.c | 2 +- drivers/gpu/drm/i915/gvt/display.h | 2 + drivers/gpu/drm/i915/gvt/fb_decoder.c | 479 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/fb_decoder.h | 170 ++++++++++++ drivers/gpu/drm/i915/gvt/gvt.h | 1 + 6 files changed, 655 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h
drivers/gpu/drm/i915/gvt/Makefile +2 -1
@@ -1,7 +1,8 @@ GVT_DIR := gvt GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \ interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ - execlist.o scheduler.o sched_policy.o render.o cmd_parser.o + execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \ + fb_decoder.o ccflags-y += -I$(src) -I$(src)/$(GVT_DIR) -Wall i915-y += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
drivers/gpu/drm/i915/gvt/display.c +1 -1
@@ -67,7 +67,7 @@ static int edp_pipe_is_enabled(struct intel_vgpu *vgpu)return 1 ; } -static int pipe_is_enabled (struct intel_vgpu *vgpu,int pipe) +int pipe_is_enabled (struct intel_vgpu *vgpu,int pipe) {struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
drivers/gpu/drm/i915/gvt/display.h +2 -0
@@ -179,4 +179,6 @@ int intel_vgpu_init_display(struct intel_vgpu *vgpu, u64 resolution);void intel_vgpu_reset_display (struct intel_vgpu *vgpu);void intel_vgpu_clean_display (struct intel_vgpu *vgpu); +int pipe_is_enabled (struct intel_vgpu *vgpu,int pipe); +#endif
drivers/gpu/drm/i915/gvt/fb_decoder.c new file +479 -0
479 lines not on this page · raw message
drivers/gpu/drm/i915/gvt/fb_decoder.h new file +170 -0
170 lines not on this page · raw message
drivers/gpu/drm/i915/gvt/gvt.h +1 -0
@@ -46,6 +46,7 @@#include "sched_policy.h" #include "render.h" #include "cmd_parser.h" +#include "fb_decoder.h" #define GVT_MAX_VGPU 8
Signature · 6 lines
2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v5 4/5 drm/i915/gvt: Dmabuf support for GVT-g
In reply to Xiaoguang Chendmabuf for GVT-g can be exported to users who can use the dmabuf to show the desktop of vm which use intel vgpu.
Currently we provide query and create new dmabuf operations.
Users of dmabuf can cache some created dmabufs and related information such as the framebuffer's address, size, tiling mode, width, height etc. When refresh the screen first query the currnet vgpu's frambuffer and compare with the cached ones(address, size, tiling, width, height etc) if found one then reuse the found dmabuf to gain performance improvment.
If there is no dmabuf created yet or not found in the cached dmabufs then need to create a new dmabuf. To create a dmabuf first a gem object will be created and the backing storage of this gem object is the vgpu's framebuffer(primary/cursor).
Set caching mode, change tiling mode and set domains of this gem object is not supported.
Then associate this gem object to a dmabuf and export this dmabuf. A file descriptor will be generated for this dmabuf and this file descriptor can be sent to user space to do display.
--- drivers/gpu/drm/i915/gvt/Makefile | 2 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 276 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/dmabuf.h | 53 +++++++ drivers/gpu/drm/i915/gvt/gvt.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 8 + drivers/gpu/drm/i915/i915_gem_object.h | 9 ++ 6 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.h
drivers/gpu/drm/i915/gvt/Makefile +1 -1
@@ -2,7 +2,7 @@ GVT_DIR := gvt GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \ interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \ - fb_decoder.o + fb_decoder.o dmabuf.o ccflags-y += -I$(src) -I$(src)/$(GVT_DIR) -Wall i915-y += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
drivers/gpu/drm/i915/gvt/dmabuf.c new file +276 -0
276 lines not on this page · raw message
drivers/gpu/drm/i915/gvt/dmabuf.h new file +53 -0
53 lines not on this page · raw message
drivers/gpu/drm/i915/gvt/gvt.h +1 -0
@@ -47,6 +47,7 @@#include "render.h" #include "cmd_parser.h" #include "fb_decoder.h" +#include "dmabuf.h" #define GVT_MAX_VGPU 8
drivers/gpu/drm/i915/i915_gem.c +8 -0
@@ -1609,6 +1609,10 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,if (!obj)return -ENOENT ; +/* The obj is a gvt dma-buf object and set domain is not supported */ +if (i915_gem_object_is_gvt_dmabuf (obj)) +return -EPERM ; +/* Try to flush the object off the GPU without holding the lock. * We will repeat the flush holding the lock in the normal manner * to catch cases where we are gazumped.
@@ -3717,6 +3721,10 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,if (!obj)return -ENOENT ; +/* the obj is a gvt dma-buf and set caching mode is not supported */ +if (i915_gem_object_is_gvt_dmabuf (obj)) +return -EPERM ; +if (obj->cache_level == level) goto out;
drivers/gpu/drm/i915/i915_gem_object.h +9 -0
@@ -39,6 +39,7 @@ struct drm_i915_gem_object_ops {unsigned int flags;#define I915_GEM_OBJECT_HAS_STRUCT_PAGE 0x1#define I915_GEM_OBJECT_IS_SHRINKABLE 0x2 +#define I915_GEM_OBJECT_IS_GVT_DMABUF 0x4/* Interface between the GEM object and its backing storage. * get_pages() is called once prior to the use of the associated set
@@ -184,6 +185,8 @@ struct drm_i915_gem_object { } userptr;unsigned long scratch; + +void *gvt_plane_info; };/** for phys allocated objects */
@@ -286,6 +289,12 @@ i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) }static inline bool +i915_gem_object_is_gvt_dmabuf (const struct drm_i915_gem_object *obj) +{ +return obj->ops->flags &I915_GEM_OBJECT_IS_GVT_DMABUF ; +} + +static inline bool i915_gem_object_is_active (const struct drm_i915_gem_object *obj) {return obj->active_count;
Signature · 6 lines
2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v5 4/5] drm/i915/gvt: Dmabuf support for GVT-g
Hi,
Quoted 13 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +11
-0+ }else if (plane_id ==INTEL_GVT_PLANE_CURSOR ) { + c = &pipe->cursor; +if (c !=NULL ) { + info->start = c->base; + info->width = c->width; + info->height = c->height; + info->stride = c->width * (c->bpp /8 ); + info->drm_format_mod =0 ; + info->x_pos = c->x_pos; + info->y_pos = c->y_pos; + info->size = (((info->stride * c->height *
c->bpp) / 8)(quoted diff) +1
-0+ + (PAGE_SIZE - 1)) >>
Quoted 1 line
PAGE_SHIFT;
Leaves info->drm_format unset.
drivers/gpu/drm/i915/gvt/dmabuf.h line 40
Quoted 12 lines
drivers/gpu/drm/i915/gvt/dmabuf.h new file +12
-0+struct intel_vgpu_plane_info { +uint32_t plane_id; +uint32_t drm_format; +uint32_t width; +uint32_t height; +uint32_t stride; +uint32_t start; +uint32_t x_pos; +uint32_t y_pos; +uint32_t size; +uint64_t drm_format_mod; +};
drm_format_mod is unaligned. Should be avoided, otherwise the struct layout is different on i386 and x86_64.
Patch 5/5 moves around this struct. Better have a separate patch adding the structs and ioctls, order it before this one, so you don't have to rename the stuff just added ...
drivers/gpu/drm/i915/gvt/dmabuf.h line 48
Quoted 4 lines
drivers/gpu/drm/i915/gvt/dmabuf.h new file +4
-0+struct intel_vgpu_dmabuf { +uint32_t fd; +struct intel_vgpu_plane_info plane_info; +};
Has alignment problems too.
cheers,
Gerd
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v5 4/5] drm/i915/gvt: Dmabuf support for GVT-g
In reply to Xiaoguang ChenQuoted 92 lines · On Tue, 23 May 2017 18:32:00 +0800 Xiaoguang Chen <[email protected]>…
dmabuf for GVT-g can be exported to users who can use the dmabuf to show the desktop of vm which use intel vgpu.
Currently we provide query and create new dmabuf operations.
Users of dmabuf can cache some created dmabufs and related information such as the framebuffer's address, size, tiling mode, width, height etc. When refresh the screen first query the currnet vgpu's frambuffer and compare with the cached
ones(address, size, tiling, width, height etc)if found one then reuse the found dmabuf to gain performance improvment.If there is no dmabuf created yet or not found in the cached dmabufs then need to create a new dmabuf. To create a dmabuf first a gem object will be created and the backing storage of this gem object is the vgpu's framebuffer(primary/cursor).
Set caching mode, change tiling mode and set domains of this gem object is not supported.
Then associate this gem object to a dmabuf and export this dmabuf. A file descriptor will be generated for this dmabuf and this file descriptor can be sent to user space to do display.Signed-off-by Xiaoguang Chen <[email protected]>--- drivers/gpu/drm/i915/gvt/Makefile | 2 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 276 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/dmabuf.h | 53 +++++++ drivers/gpu/drm/i915/gvt/gvt.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 8 + drivers/gpu/drm/i915/i915_gem_object.h | 9 ++ 6 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.hdrivers/gpu/drm/i915/gvt/Makefile +1
-1@@ -2,7 +2,7 @@ GVT_DIR := gvt GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \ interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \ - fb_decoder.o + fb_decoder.o dmabuf.o ccflags-y += -I$(src) -I$(src)/$(GVT_DIR) -Wall i915-y += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))drivers/gpu/drm/i915/gvt/dmabuf.c new file +40
-0@@ -0,0 +1,276 @@ +/* +* Copyright 2017 Intel Corporation. All rights reserved. +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +* DEALINGS IN THE SOFTWARE. +* +* Authors: +* Zhiyuan Lv <[email protected]> +* +* Contributors: +* Xiaoguang Chen <[email protected]> +*/ + +#include <linux/dma-buf.h> +#include <drm/drmP.h> + +#include "i915_drv.h" +#include "gvt.h" + +#define GEN8_DECODE_PTE (pte) \ + ((dma_addr_t)(((((u64)pte) >> 12) & 0x7ffffffULL) << 12)) + +static struct sg_table *intel_vgpu_gem_get_pages ( +struct drm_i915_gem_object *obj)
drivers/gpu/drm/i915/gvt/dmabuf.c line 50
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+{ +struct drm_i915_private *dev_priv =to_i915 (obj->base.dev); +struct sg_table *st; +struct scatterlist *sg; +int i, ret; +gen8_pte_t __iomem *gtt_entries; +unsigned int fb_gma =0 , fb_size =0 ; +struct intel_vgpu_plane_info *plane_info; + + plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info;
I can't find where gvt_plane_info is defined, but it's curious why it's not already this type.
Quoted 100 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +100
-0+if (WARN_ON (!plane_info)) +return ERR_PTR (-EINVAL ); + + fb_gma = plane_info->start; + fb_size = plane_info->size; + + st =kmalloc (sizeof (*st),GFP_KERNEL ); +if (!st) { + ret = -ENOMEM ; +return ERR_PTR (ret); + } + + ret =sg_alloc_table (st, fb_size,GFP_KERNEL ); +if (ret) { +kfree (st); +return ERR_PTR (ret); + } + gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm + + (fb_gma >>PAGE_SHIFT ); +for_each_sg (st->sgl, sg, fb_size, i) { + sg->offset =0 ; + sg->length =PAGE_SIZE ; +sg_dma_address (sg) = +GEN8_DECODE_PTE (readq (>t_entries[i])); +sg_dma_len (sg) =PAGE_SIZE ; + } + +return st; +} + +static void intel_vgpu_gem_put_pages (struct drm_i915_gem_object *obj, +struct sg_table *pages) +{ +struct intel_vgpu_plane_info *plane_info; + + plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; +if (WARN_ON (!plane_info)) +return ; + +sg_free_table (pages); +kfree (pages); +} + +static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = { + .flags =I915_GEM_OBJECT_IS_GVT_DMABUF , + .get_pages = intel_vgpu_gem_get_pages, + .put_pages = intel_vgpu_gem_put_pages, +}; + +static struct drm_i915_gem_object *intel_vgpu_create_gem (struct drm_device *dev, +struct intel_vgpu_plane_info *info) +{ +struct drm_i915_private *pri = dev->dev_private; +struct drm_i915_gem_object *obj; + + obj =i915_gem_object_alloc (pri); +if (obj ==NULL ) +return NULL ; + +drm_gem_private_object_init (dev, &obj->base, + info->size <<PAGE_SHIFT ); +i915_gem_object_init (obj, &intel_vgpu_gem_ops); + + obj->base.read_domains =I915_GEM_DOMAIN_GTT ; + obj->base.write_domain =0 ; + obj->framebuffer_references++; + obj->gvt_plane_info = info; + +if (IS_SKYLAKE (pri)) { +unsigned int tiling_mode =0 ; + +switch (info->drm_format_mod <<10 ) { +case PLANE_CTL_TILED_LINEAR : + tiling_mode =I915_TILING_NONE ; +break ; +case PLANE_CTL_TILED_X : + tiling_mode =I915_TILING_X ; +break ; +case PLANE_CTL_TILED_Y : + tiling_mode =I915_TILING_Y ; +break ; +default : +gvt_dbg_core ("not supported tiling mode\n" ); + } + obj->tiling_and_stride = tiling_mode | info->stride; + }else { + obj->tiling_and_stride = info->drm_format_mod ? +I915_TILING_X :0 ; + } + +return obj; +} + +static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info ( +struct drm_device *dev, +struct intel_vgpu *vgpu,uint32_t plane_id) +{ +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_vgpu_primary_plane_format *p; +struct intel_vgpu_cursor_plane_format *c;
drivers/gpu/drm/i915/gvt/dmabuf.c line 160
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+struct intel_vgpu_plane_info *info; +struct intel_vgpu_pipe_format *pipe; + + info =kmalloc (sizeof (*info),GFP_KERNEL ); +if (!info) +return NULL ; + + pipe =intel_vgpu_decode_plane (dev, vgpu); +if (pipe ==NULL ) +return NULL ;
Leaks info, reverse order?
Quoted 53 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +53
-0+ +if (plane_id ==INTEL_GVT_PLANE_PRIMARY ) { + p = &pipe->primary; +if (p !=NULL ) { + info->start = p->base; + info->width = p->width; + info->height = p->height; + info->stride = p->stride; + info->drm_format = p->drm_format; + info->drm_format_mod = p->tiled; + info->size = (((p->stride * p->height * p->bpp) /8 ) + + (PAGE_SIZE -1 )) >>PAGE_SHIFT ; + }else { +kfree (info); +gvt_vgpu_err ("invalid primary plane\n" ); +return NULL ; + } + }else if (plane_id ==INTEL_GVT_PLANE_CURSOR ) { + c = &pipe->cursor; +if (c !=NULL ) { + info->start = c->base; + info->width = c->width; + info->height = c->height; + info->stride = c->width * (c->bpp /8 ); + info->drm_format_mod =0 ; + info->x_pos = c->x_pos; + info->y_pos = c->y_pos; + info->size = (((info->stride * c->height * c->bpp) /8 ) + + (PAGE_SIZE -1 )) >>PAGE_SHIFT ; + }else { +kfree (info); +gvt_vgpu_err ("invalid cursor plane\n" ); +return NULL ; + } + }else { +kfree (info); +gvt_vgpu_err ("invalid plane id:%d\n" , plane_id); +return NULL ; + } + +if (info->size ==0 ) { +kfree (info); +gvt_vgpu_err ("fb size is zero\n" ); +return NULL ; + } + +if (info->start & (PAGE_SIZE -1 )) { +kfree (info); +gvt_vgpu_err ("Not aligned fb address:0x%x\n" , info->start); +return NULL ; + } +if (((info->start >>PAGE_SHIFT ) + info->size) > +ggtt_total_entries (&dev_priv->ggtt)) {
drivers/gpu/drm/i915/gvt/dmabuf.c line 223
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+kfree (info); +gvt_vgpu_err ("Invalid GTT offset or size\n" ); +return NULL ; + } + +if (!intel_gvt_ggtt_validate_range (vgpu, info->start, info->size)) { +kfree (info); +gvt_vgpu_err ("invalid gma addr\n" ); +return NULL ; + }
Would it be useful to use ERR_PTR() so we could pass a relevant errno back up the stack to the user?
drivers/gpu/drm/i915/gvt/dmabuf.c line 235
Quoted 12 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +12
-0+ +return info; +} + +int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args) +{ +struct drm_device *dev = &vgpu->gvt->dev_priv->drm; +struct intel_vgpu_plane_info *info = args; + + info =intel_vgpu_get_plane_info (dev, vgpu, info->plane_id); +if (info ==NULL ) +return -EINVAL ;
Rather than this?
Quoted 12 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +12
-0+ +return 0 ; +} + +int intel_vgpu_create_dmabuf (struct intel_vgpu *vgpu,void *args) +{ +struct dma_buf *dmabuf; +struct drm_i915_gem_object *obj; +struct drm_device *dev = &vgpu->gvt->dev_priv->drm; +struct intel_vgpu_dmabuf *gvt_dmabuf = args; +struct intel_vgpu_plane_info *info; +int ret;
drivers/gpu/drm/i915/gvt/dmabuf.c line 257
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+ + info =intel_vgpu_get_plane_info (dev, vgpu, + gvt_dmabuf->plane_info.plane_id); +if (info ==NULL ) +return -EINVAL ; + + obj =intel_vgpu_create_gem (dev, info); +if (obj ==NULL ) { +gvt_vgpu_err ("create gvt gem obj failed:%d\n" , vgpu->id); +return -EINVAL ;
@info is leaked?
drivers/gpu/drm/i915/gvt/dmabuf.c line 264
Quoted 7 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +7
-0+ } + + dmabuf =i915_gem_prime_export (dev, &obj->base,DRM_CLOEXEC |DRM_RDWR ); + +if (IS_ERR (dmabuf)) { +gvt_vgpu_err ("export dma-buf failed\n" ); +return -EINVAL ;
@info & @obj leaked?
drivers/gpu/drm/i915/gvt/dmabuf.c line 270
Quoted 6 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +6
-0+ } + + ret =dma_buf_fd (dmabuf,DRM_CLOEXEC |DRM_RDWR ); +if (ret <0 ) { +gvt_vgpu_err ("create dma-buf fd failed ret:%d\n" , ret); +return -EINVAL ;
same
Also, the fd is provided to the user and is a reference to the device, we need to increment the vfio_device reference and decrement on release.
drivers/gpu/drm/i915/gvt/dmabuf.c line 275
Quoted 5 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +5
-0+ } + gvt_dmabuf->fd = ret; + gvt_dmabuf->plane_info = *info; + +return 0 ;
same
Quoted 37 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +1
-0+}drivers/gpu/drm/i915/gvt/dmabuf.h new file +30
-0@@ -0,0 +1,53 @@ + +/* +* Copyright(c) 2017 Intel Corporation. All rights reserved. +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +* SOFTWARE. +* +*/ + +#ifndef _GVT_DMABUF_H_ +#define _GVT_DMABUF_H_ + +struct intel_vgpu_plane_info { + uint32_t plane_id;
drivers/gpu/drm/i915/gvt/dmabuf.h line 40
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.h new file +10
-0+uint32_t drm_format; +uint32_t width; +uint32_t height; +uint32_t stride; +uint32_t start; +uint32_t x_pos; +uint32_t y_pos; +uint32_t size; +uint64_t drm_format_mod; +};
Alignment issue as Gerd mentions.
drivers/gpu/drm/i915/gvt/dmabuf.h line 48
Quoted 8 lines
drivers/gpu/drm/i915/gvt/dmabuf.h new file +8
-0+ +#define INTEL_VGPU_QUERY_PLANE 0 +#define INTEL_VGPU_GENERATE_DMABUF 1 + +struct intel_vgpu_dmabuf { +uint32_t fd; +struct intel_vgpu_plane_info plane_info; +};
And here, also as Gerd mentions.
Quoted 67 lines
drivers/gpu/drm/i915/gvt/dmabuf.h new file +5
-0+ +int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args); +int intel_vgpu_create_dmabuf (struct intel_vgpu *vgpu,void *args); + +#endif drivers/gpu/drm/i915/gvt/gvt.h +1
-0@@ -47,6 +47,7 @@#include "render.h" #include "cmd_parser.h" #include "fb_decoder.h" +#include "dmabuf.h" #define GVT_MAX_VGPU 8drivers/gpu/drm/i915/i915_gem.c +8
-0@@ -1609,6 +1609,10 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,if (!obj)return -ENOENT ; +/* The obj is a gvt dma-buf object and set domain is not supported */ +if (i915_gem_object_is_gvt_dmabuf (obj)) +return -EPERM ; +/* Try to flush the object off the GPU without holding the lock. * We will repeat the flush holding the lock in the normal manner * to catch cases where we are gazumped. @@ -3717,6 +3721,10 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,if (!obj)return -ENOENT ; +/* the obj is a gvt dma-buf and set caching mode is not supported */ +if (i915_gem_object_is_gvt_dmabuf (obj)) +return -EPERM ; +if (obj->cache_level == level) goto out;drivers/gpu/drm/i915/i915_gem_object.h +3
-0@@ -39,6 +39,7 @@ struct drm_i915_gem_object_ops {unsigned int flags;#define I915_GEM_OBJECT_HAS_STRUCT_PAGE 0x1#define I915_GEM_OBJECT_IS_SHRINKABLE 0x2 +#define I915_GEM_OBJECT_IS_GVT_DMABUF 0x4/* Interface between the GEM object and its backing storage. * get_pages() is called once prior to the use of the associated set @@ -184,6 +185,8 @@ struct drm_i915_gem_object { } userptr;unsigned long scratch; + +void *gvt_plane_info; };/** for phys allocated objects */ @@ -286,6 +289,12 @@ i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) }
drivers/gpu/drm/i915/i915_gem_object.h i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) · line 300
Quoted 10 lines
drivers/gpu/drm/i915/i915_gem_object.h +6
-0static inline bool +i915_gem_object_is_gvt_dmabuf (const struct drm_i915_gem_object *obj) +{ +return obj->ops->flags &I915_GEM_OBJECT_IS_GVT_DMABUF ; +} + +static inline bool i915_gem_object_is_active (const struct drm_i915_gem_object *obj) {return obj->active_count;
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/5] drm/i915/gvt: Dmabuf support for GVT-g
In reply to Xiaoguang ChenQuoted 79 lines · Xiaoguang Chen
dmabuf for GVT-g can be exported to users who can use the dmabuf to show the desktop of vm which use intel vgpu.
Currently we provide query and create new dmabuf operations.
Users of dmabuf can cache some created dmabufs and related information such as the framebuffer's address, size, tiling mode, width, height etc. When refresh the screen first query the currnet vgpu's frambuffer and compare with the cached
ones(address, size, tiling, width, height etc)if found one then reuse the found dmabuf to gain performance improvment.If there is no dmabuf created yet or not found in the cached dmabufs then need to create a new dmabuf. To create a dmabuf first a gem object will be created and the backing storage of this gem object is the vgpu's framebuffer(primary/cursor).
Set caching mode, change tiling mode and set domains of this gem object is not supported.
Then associate this gem object to a dmabuf and export this dmabuf. A file descriptor will be generated for this dmabuf and this file descriptor can be sent to user space to do display.Signed-off-by Xiaoguang Chen <[email protected]>--- drivers/gpu/drm/i915/gvt/Makefile | 2 +- drivers/gpu/drm/i915/gvt/dmabuf.c | 276 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/gvt/dmabuf.h | 53 +++++++ drivers/gpu/drm/i915/gvt/gvt.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 8 + drivers/gpu/drm/i915/i915_gem_object.h | 9 ++ 6 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.hdrivers/gpu/drm/i915/gvt/Makefile +1
-1@@ -2,7 +2,7 @@ GVT_DIR := gvt GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \ interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \ execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \ - fb_decoder.o + fb_decoder.o dmabuf.o ccflags-y += -I$(src) -I$(src)/$(GVT_DIR) -Wall i915-y += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))drivers/gpu/drm/i915/gvt/dmabuf.c new file +27
-0@@ -0,0 +1,276 @@ +/* +* Copyright 2017 Intel Corporation. All rights reserved. +* +* Permission is hereby granted, free of charge, to any person obtaining a +* copy of this software and associated documentation files (the "Software"), +* to deal in the Software without restriction, including without limitation +* the rights to use, copy, modify, merge, publish, distribute, sublicense, +* and/or sell copies of the Software, and to permit persons to whom the +* Software is furnished to do so, subject to the following conditions: +* +* The above copyright notice and this permission notice (including the next +* paragraph) shall be included in all copies or substantial portions of the +* Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +* DEALINGS IN THE SOFTWARE. +* +* Authors: +* Zhiyuan Lv <[email protected]> +* +* Contributors: +* Xiaoguang Chen <[email protected]>
drivers/gpu/drm/i915/gvt/dmabuf.c line 37
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+*/ + +#include <linux/dma-buf.h> +#include <drm/drmP.h> + +#include "i915_drv.h" +#include "gvt.h" + +#define GEN8_DECODE_PTE (pte) \ + ((dma_addr_t)(((((u64)pte) >> 12) & 0x7ffffffULL) << 12))
pte & GENMASK_ULL(63, 12)
Quoted 5 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +5
-0+ +static struct sg_table *intel_vgpu_gem_get_pages ( +struct drm_i915_gem_object *obj) +{ +struct drm_i915_private *dev_priv =to_i915 (obj->base.dev);
drivers/gpu/drm/i915/gvt/dmabuf.c line 52
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+struct sg_table *st; +struct scatterlist *sg; +int i, ret; +gen8_pte_t __iomem *gtt_entries; +unsigned int fb_gma =0 , fb_size =0 ; +struct intel_vgpu_plane_info *plane_info; + + plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; +if (WARN_ON (!plane_info)) +return ERR_PTR (-EINVAL );
This is an internal bug, don't EINVAL.
Quoted 12 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +12
-0+ fb_gma = plane_info->start; + fb_size = plane_info->size; + + st =kmalloc (sizeof (*st),GFP_KERNEL ); +if (!st) { + ret = -ENOMEM ; +return ERR_PTR (ret); + } + + ret =sg_alloc_table (st, fb_size,GFP_KERNEL ); +if (ret) { +kfree (st);
drivers/gpu/drm/i915/gvt/dmabuf.c line 75
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+return ERR_PTR (ret); + } + gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm + + (fb_gma >>PAGE_SHIFT ); +for_each_sg (st->sgl, sg, fb_size, i) { + sg->offset =0 ; + sg->length =PAGE_SIZE ; +sg_dma_address (sg) = +GEN8_DECODE_PTE (readq (>t_entries[i])); +sg_dma_len (sg) =PAGE_SIZE ;
This assumes that the entries are PAGE_SIZE. This will not remain true. Also sg coalescing?
drivers/gpu/drm/i915/gvt/dmabuf.c line 88
Quoted 13 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +13
-0+ } + +return st; +} + +static void intel_vgpu_gem_put_pages (struct drm_i915_gem_object *obj, +struct sg_table *pages) +{ +struct intel_vgpu_plane_info *plane_info; + + plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; +if (WARN_ON (!plane_info)) +return ;
Is there any value in this check?
drivers/gpu/drm/i915/gvt/dmabuf.c line 95
Quoted 6 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +6
-0+sg_free_table (pages); +kfree (pages); +} + +static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = { + .flags =I915_GEM_OBJECT_IS_GVT_DMABUF ,
I'm going to suggest to call this bit I915_GEM_OBJECT_IS_PROXY to generalise it beyond this special case. But I'm still debating how much I approve of this change to the uABI.
drivers/gpu/drm/i915/gvt/dmabuf.c line 103
Quoted 8 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +8
-0+ .get_pages = intel_vgpu_gem_get_pages, + .put_pages = intel_vgpu_gem_put_pages, +}; + +static struct drm_i915_gem_object *intel_vgpu_create_gem (struct drm_device *dev, +struct intel_vgpu_plane_info *info) +{ +struct drm_i915_private *pri = dev->dev_private;
to_i915(dev);
Please tell me that pri isn't common shorthand in gvt/! Commonly called dev_priv, but I'm crusading for i915.
Quoted 5 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +5
-0+struct drm_i915_gem_object *obj; + + obj =i915_gem_object_alloc (pri); +if (obj ==NULL ) +return NULL ;
drivers/gpu/drm/i915/gvt/dmabuf.c line 116
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +8
-0+ +drm_gem_private_object_init (dev, &obj->base, + info->size <<PAGE_SHIFT ); +i915_gem_object_init (obj, &intel_vgpu_gem_ops); + + obj->base.read_domains =I915_GEM_DOMAIN_GTT ; + obj->base.write_domain =0 ; + obj->framebuffer_references++;
Missing comment. (And not amused with the magic, if it's only purpose is to prevent set_tiling.)
Quoted 9 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +9
-0+ obj->gvt_plane_info = info; + +if (IS_SKYLAKE (pri)) { +unsigned int tiling_mode =0 ; + +switch (info->drm_format_mod <<10 ) { +case PLANE_CTL_TILED_LINEAR : + tiling_mode =I915_TILING_NONE ; +break ;
drivers/gpu/drm/i915/gvt/dmabuf.c line 135
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+case PLANE_CTL_TILED_X : + tiling_mode =I915_TILING_X ; +break ; +case PLANE_CTL_TILED_Y : + tiling_mode =I915_TILING_Y ; +break ; +default : +gvt_dbg_core ("not supported tiling mode\n" ); + } + obj->tiling_and_stride = tiling_mode | info->stride;
Stride should be zero if I915_TILING_NONE;
drivers/gpu/drm/i915/gvt/dmabuf.c line 148
Quoted 13 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +13
-0+ }else { + obj->tiling_and_stride = info->drm_format_mod ? +I915_TILING_X :0 ; + } + +return obj; +} + +static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info ( +struct drm_device *dev, +struct intel_vgpu *vgpu,uint32_t plane_id) +{ +struct drm_i915_private *dev_priv = dev->dev_private;
dev_priv here! Still use to_i915() though.
drivers/gpu/drm/i915/gvt/dmabuf.c line 160
Quoted 12 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +12
-0+struct intel_vgpu_primary_plane_format *p; +struct intel_vgpu_cursor_plane_format *c; +struct intel_vgpu_plane_info *info; +struct intel_vgpu_pipe_format *pipe; + + info =kmalloc (sizeof (*info),GFP_KERNEL ); +if (!info) +return NULL ; + + pipe =intel_vgpu_decode_plane (dev, vgpu); +if (pipe ==NULL ) +return NULL ;
Onion unwind to kill the leaks as others have mentioned.
Quoted 20 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +20
-0+int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args) +{ +struct drm_device *dev = &vgpu->gvt->dev_priv->drm; +struct intel_vgpu_plane_info *info = args; + + info =intel_vgpu_get_plane_info (dev, vgpu, info->plane_id); +if (info ==NULL ) +return -EINVAL ; + +return 0 ; +} + +int intel_vgpu_create_dmabuf (struct intel_vgpu *vgpu,void *args) +{ +struct dma_buf *dmabuf; +struct drm_i915_gem_object *obj; +struct drm_device *dev = &vgpu->gvt->dev_priv->drm; +struct intel_vgpu_dmabuf *gvt_dmabuf = args; +struct intel_vgpu_plane_info *info; +int ret;
drivers/gpu/drm/i915/gvt/dmabuf.c line 257
Quoted 10 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +10
-0+ + info =intel_vgpu_get_plane_info (dev, vgpu, + gvt_dmabuf->plane_info.plane_id); +if (info ==NULL ) +return -EINVAL ; + + obj =intel_vgpu_create_gem (dev, info); +if (obj ==NULL ) { +gvt_vgpu_err ("create gvt gem obj failed:%d\n" , vgpu->id); +return -EINVAL ;
Of all the errors that can cause this, do you think the user is at fault?
drivers/gpu/drm/i915/gvt/dmabuf.c line 264
Quoted 7 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +7
-0+ } + + dmabuf =i915_gem_prime_export (dev, &obj->base,DRM_CLOEXEC |DRM_RDWR ); + +if (IS_ERR (dmabuf)) { +gvt_vgpu_err ("export dma-buf failed\n" ); +return -EINVAL ;
Again, was this a user error? You even have the right error to hand!
drivers/gpu/drm/i915/gvt/dmabuf.c line 270
Quoted 6 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +6
-0+ } + + ret =dma_buf_fd (dmabuf,DRM_CLOEXEC |DRM_RDWR ); +if (ret <0 ) { +gvt_vgpu_err ("create dma-buf fd failed ret:%d\n" , ret); +return -EINVAL ;
Sitll a user error? Not ret?
Quoted 24 lines
drivers/gpu/drm/i915/gvt/dmabuf.c new file +6
-0+ } + gvt_dmabuf->fd = ret; + gvt_dmabuf->plane_info = *info; + +return 0 ; +}drivers/gpu/drm/i915/gvt/gvt.h +1
-0@@ -47,6 +47,7 @@#include "render.h" #include "cmd_parser.h" #include "fb_decoder.h" +#include "dmabuf.h" #define GVT_MAX_VGPU 8drivers/gpu/drm/i915/i915_gem.c +0
-0
drivers/gpu/drm/i915/i915_gem.c i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, · line 1614
Quoted 6 lines
drivers/gpu/drm/i915/i915_gem.c +3
-0@@ -1609,6 +1609,10 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,if (!obj)return -ENOENT ; +/* The obj is a gvt dma-buf object and set domain is not supported */ +if (i915_gem_object_is_gvt_dmabuf (obj)) +return -EPERM ;
Leaks the obj.
Ok, I'm slightly warming to this.
Newly broken ioctls:
i915_gem_set_caching_ioctl i915_gem_set_domain_ioctl i915_gem_sw_finish_ioctl i915_gem_set_tiling_ioctl i915_gem_madvise_ioctl
These should all be -EPERM.
Existing broken ioctls:
i915_gem_mmap_ioctl
Sadly this used -EINVAL. Might sneak a change in to use -EPERM for constistency.
Misbehaving ioctls:
i915_gem_busy_ioctl i915_gem_throttle_ioctl i915_gem_wait_ioctl
These only work on the proxy bo and do not reflect the real object. -Chris
Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 4/5] drm/i915/gvt: Dmabuf support for GVT-g
Quoted 11 lines · Chris Wilson
drivers/gpu/drm/i915/gvt/dmabuf.c line 75
Quoted 8 lines · Xiaoguang Chen
drivers/gpu/drm/i915/gvt/dmabuf.c new file +8
-0+ gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm + + (fb_gma >>PAGE_SHIFT ); +for_each_sg (st->sgl, sg, fb_size, i) { + sg->offset =0 ; + sg->length =PAGE_SIZE ; +sg_dma_address (sg) = +GEN8_DECODE_PTE (readq (>t_entries[i])); +sg_dma_len (sg) =PAGE_SIZE ;This assumes that the entries are
PAGE_SIZE. This will not remain true.
Ok, we will only be supporting different page sizes for ppgtt. However, it is probably better to use I915_GTT_PAGE_SIZE to match our insertions. -Chris
Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH v5 5/5 drm/i915/gvt: Adding interface so user space can get the dma-buf
In reply to Xiaoguang ChenUser space will try to create a management fd for the dma-buf operation. Using this management fd user can query the plane information and create a dma-buf fd if necessary.
GVT-g will handle the life cycle of the management fd and will align the life cycle of the fd with the vfio device.
User space should handle the life cycle of the created dma-buf fd close the dma-buf fd timely when finishing use.
--- drivers/gpu/drm/i915/gvt/dmabuf.c | 25 ++++---- drivers/gpu/drm/i915/gvt/dmabuf.h | 21 ------- drivers/gpu/drm/i915/gvt/fb_decoder.h | 2 - drivers/gpu/drm/i915/gvt/gvt.c | 2 + drivers/gpu/drm/i915/gvt/gvt.h | 4 ++ drivers/gpu/drm/i915/gvt/kvmgt.c | 107 ++++++++++++++++++++++++++++++++++ include/uapi/linux/vfio.h | 50 +++++++++++++++- 7 files changed, 175 insertions(+), 36 deletions(-)
drivers/gpu/drm/i915/gvt/dmabuf.c +13 -12
@@ -29,6 +29,7 @@#include <linux/dma-buf.h> #include <drm/drmP.h> +#include <linux/vfio.h> #include "i915_drv.h" #include "gvt.h"
@@ -45,9 +46,9 @@ static struct sg_table *intel_vgpu_gem_get_pages(int i, ret;gen8_pte_t __iomem *gtt_entries;unsigned int fb_gma =0 , fb_size =0 ; -struct intel_vgpu_plane_info *plane_info; +struct plane_info *plane_info; - plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; + plane_info = (struct plane_info *)obj->gvt_plane_info;if (WARN_ON (!plane_info))return ERR_PTR (-EINVAL );
@@ -81,9 +82,9 @@ static struct sg_table *intel_vgpu_gem_get_pages(static void intel_vgpu_gem_put_pages (struct drm_i915_gem_object *obj,struct sg_table *pages) { -struct intel_vgpu_plane_info *plane_info; +struct plane_info *plane_info; - plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; + plane_info = (struct plane_info *)obj->gvt_plane_info;if (WARN_ON (!plane_info))return ;
@@ -98,7 +99,7 @@ static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = { };static struct drm_i915_gem_object *intel_vgpu_create_gem (struct drm_device *dev, -struct intel_vgpu_plane_info *info) +struct plane_info *info) {struct drm_i915_private *pri = dev->dev_private;struct drm_i915_gem_object *obj;
@@ -141,14 +142,14 @@ static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,return obj; } -static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info ( +static struct plane_info *intel_vgpu_get_plane_info (struct drm_device *dev,struct intel_vgpu *vgpu,uint32_t plane_id) {struct drm_i915_private *dev_priv = dev->dev_private;struct intel_vgpu_primary_plane_format *p;struct intel_vgpu_cursor_plane_format *c; -struct intel_vgpu_plane_info *info; +struct plane_info *info;struct intel_vgpu_pipe_format *pipe; info =kmalloc (sizeof (*info),GFP_KERNEL );
@@ -159,7 +160,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(if (pipe ==NULL )return NULL ; -if (plane_id ==INTEL_GVT_PLANE_PRIMARY ) { +if (plane_id ==VFIO_PRIMARY_PLANE ) { p = &pipe->primary;if (p !=NULL ) { info->start = p->base;
@@ -175,7 +176,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(gvt_vgpu_err ("invalid primary plane\n" );return NULL ; } - }else if (plane_id ==INTEL_GVT_PLANE_CURSOR ) { + }else if (plane_id ==VFIO_CURSOR_PLANE ) { c = &pipe->cursor;if (c !=NULL ) { info->start = c->base;
@@ -228,7 +229,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args) {struct drm_device *dev = &vgpu->gvt->dev_priv->drm; -struct intel_vgpu_plane_info *info = args; +struct plane_info *info = args; info =intel_vgpu_get_plane_info (dev, vgpu, info->plane_id);if (info ==NULL )
@@ -242,8 +243,8 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args)struct dma_buf *dmabuf;struct drm_i915_gem_object *obj;struct drm_device *dev = &vgpu->gvt->dev_priv->drm; -struct intel_vgpu_dmabuf *gvt_dmabuf = args; -struct intel_vgpu_plane_info *info; +struct dmabuf_info *gvt_dmabuf = args; +struct plane_info *info;int ret; info = intel_vgpu_get_plane_info(dev, vgpu,
drivers/gpu/drm/i915/gvt/dmabuf.h +0 -21
@@ -26,27 +26,6 @@#ifndef _GVT_DMABUF_H_#define _GVT_DMABUF_H_ -struct intel_vgpu_plane_info { -uint32_t plane_id; -uint32_t drm_format; -uint32_t width; -uint32_t height; -uint32_t stride; -uint32_t start; -uint32_t x_pos; -uint32_t y_pos; -uint32_t size; -uint64_t drm_format_mod; -}; - -#define INTEL_VGPU_QUERY_PLANE 0 -#define INTEL_VGPU_GENERATE_DMABUF 1 - -struct intel_vgpu_dmabuf { -uint32_t fd; -struct intel_vgpu_plane_info plane_info; -}; -int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args);int intel_vgpu_create_dmabuf (struct intel_vgpu *vgpu,void *args);
drivers/gpu/drm/i915/gvt/fb_decoder.h +0 -2
@@ -81,8 +81,6 @@#define _SPRITE_OFFSET_START_X_MASK (0x1fff << _SPRITE_OFFSET_START_X_SHIFT)#define _SPRITE_OFFSET_START_Y_MASK (0xfff << _SPRITE_OFFSET_START_Y_SHIFT) -#define INTEL_GVT_PLANE_PRIMARY 1 -#define INTEL_GVT_PLANE_CURSOR 2#define INTEL_GVT_PLANE_SPRITE 3enum GVT_FB_EVENT {
drivers/gpu/drm/i915/gvt/gvt.c +2 -0
@@ -54,6 +54,8 @@ static const struct intel_gvt_ops intel_gvt_ops = { .vgpu_reset = intel_gvt_reset_vgpu, .vgpu_activate = intel_gvt_activate_vgpu, .vgpu_deactivate = intel_gvt_deactivate_vgpu, + .vgpu_query_plane = intel_vgpu_query_plane, + .vgpu_create_dmabuf = intel_vgpu_create_dmabuf, };/**
drivers/gpu/drm/i915/gvt/gvt.h +4 -0
@@ -185,8 +185,10 @@ struct intel_vgpu {struct kvm *kvm;struct work_struct release_work;atomic_t released; +struct vfio_device *vfio_device; } vdev;#endif +int dmabuf_mgr_fd; }; struct intel_gvt_gm {
@@ -467,6 +469,8 @@ struct intel_gvt_ops {void (*vgpu_reset)(struct intel_vgpu *);void (*vgpu_activate)(struct intel_vgpu *);void (*vgpu_deactivate)(struct intel_vgpu *); +int (*vgpu_query_plane)(struct intel_vgpu *vgpu,void *); +int (*vgpu_create_dmabuf)(struct intel_vgpu *vgpu,void *); };
drivers/gpu/drm/i915/gvt/kvmgt.c +107 -0
125 lines not on this page · raw message
include/uapi/linux/vfio.h +49 -1
59 lines not on this page · raw message
Signature · 6 lines
2.7.4 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v5 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
Hi,
Quoted 26 lines
include/uapi/linux/vfio.h +0
-0@@ -502,10 +502,58 @@ struct vfio_pci_hot_reset {
#defineVFIO_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
+ */include/uapi/linux/vfio.h +7
-0+ +#define VFIO_DEVICE_GET_FD _IO(VFIO_TYPE, VFIO_BASE + 14) + +#define VFIO_DEVICE_DMABUF_MGR_FD 0/* Supported fd types */ + +/* +* VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15, struct
Quoted 4 lines
plane_info)
+ * Query plane information for a plane
+ */
+structplane_info{
That is a pretty generic name. vfio_vgpu_plane_info? Or vfio_dmabuf_plane_info?
Quoted 4 lines
include/uapi/linux/vfio.h +4
-0+__u32 plane_id; +__u32 drm_format; +__u32 width; +__u32 height;
include/uapi/linux/vfio.h struct vfio_pci_hot_reset { · line 537
Quoted 10 lines
include/uapi/linux/vfio.h +10
-0+__u32 stride; +__u32 start; +__u32 x_pos; +__u32 y_pos; +__u32 size; +__u64 drm_format_mod; +}; + +#define VFIO_PRIMARY_PLANE 1 +#define VFIO_CURSOR_PLANE 2
I think we should use "enum drm_plane_type" values instead of creating something new.
cheers,
Gerd
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH v5 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
Quoted 27 lines · Gerd Hoffmann
Hi,
Quoted 25 lines
include/uapi/linux/vfio.h +5
-0@@ -502,10 +502,58 @@ 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
+ */include/uapi/linux/vfio.h +6
-0+ +#define VFIO_DEVICE_GET_FD _IO(VFIO_TYPE, VFIO_BASE + 14) + +#define VFIO_DEVICE_DMABUF_MGR_FD 0/* Supported fd types */ + +/*
Quoted 9 lines
Quoted 5 lines
+ *
VFIO_DEVICE_QUERY_PLANE-_IO(VFIO_TYPE, VFIO_BASE + 15, struct plane_info)
+ * Query plane information for a plane
+ */
+structplane_info{That is a pretty generic name.
vfio_vgpu_plane_info? Orvfio_dmabuf_plane_info?
Agree with Gerd, another suggestion vfio_vgpu_surface_info since all solutions might not always use dmabuf.
Another way to provide surface is by adding a VGA region for vGPU device using region capability which would be mmaped by QEMU and then QEMU can directly use that region to get surface. This structure could be made generic such that user can either use it using dmabuf or a separate region.
I also back Gerd's comment on 4/5 patch, change in include/uapi/linux/vfio.h should be a seperate patch.
Thanks,
Kirti
Quoted 21 lines
Quoted 4 lines
include/uapi/linux/vfio.h +4
-0+__u32 plane_id; +__u32 drm_format; +__u32 width; +__u32 height;include/uapi/linux/vfio.h struct vfio_pci_hot_reset { · line 537
Quoted 10 lines
include/uapi/linux/vfio.h +10
-0+__u32 stride; +__u32 start; +__u32 x_pos; +__u32 y_pos; +__u32 size; +__u64 drm_format_mod; +}; + +#define VFIO_PRIMARY_PLANE 1 +#define VFIO_CURSOR_PLANE 2I think we should use "enum
drm_plane_type" values instead of creating something new.cheers,
Gerd
Re: [PATCH v5 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf
In reply to Xiaoguang ChenQuoted 7 lines · On Tue, 23 May 2017 18:32:01 +0800 Xiaoguang Chen <[email protected]>…
User space will try to create a management fd for the dma-buf operation. Using this management fd user can query the plane information and create a dma-buf fd if necessary.
GVT-g will handle the life cycle of the management fd and will align the life cycle of the fd with the vfio device.
User space should handle the life cycle of the created dma-buf fd close the dma-buf fd timely when finishing use.
The user cannot be expected to do this, each dmabuf fd also needs to take a reference to the vfio device. You'll need to find a way to hook into the dmabuf release function.
Quoted 353 lines
Signed-off-by Xiaoguang Chen <[email protected]>--- drivers/gpu/drm/i915/gvt/dmabuf.c | 25 ++++---- drivers/gpu/drm/i915/gvt/dmabuf.h | 21 ------- drivers/gpu/drm/i915/gvt/fb_decoder.h | 2 - drivers/gpu/drm/i915/gvt/gvt.c | 2 + drivers/gpu/drm/i915/gvt/gvt.h | 4 ++ drivers/gpu/drm/i915/gvt/kvmgt.c | 107 ++++++++++++++++++++++++++++++++++ include/uapi/linux/vfio.h | 50 +++++++++++++++- 7 files changed, 175 insertions(+), 36 deletions(-)drivers/gpu/drm/i915/gvt/dmabuf.c +13
-12@@ -29,6 +29,7 @@#include <linux/dma-buf.h> #include <drm/drmP.h> +#include <linux/vfio.h> #include "i915_drv.h" #include "gvt.h" @@ -45,9 +46,9 @@ static struct sg_table *intel_vgpu_gem_get_pages(int i, ret;gen8_pte_t __iomem *gtt_entries;unsigned int fb_gma =0 , fb_size =0 ; -struct intel_vgpu_plane_info *plane_info; +struct plane_info *plane_info; - plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; + plane_info = (struct plane_info *)obj->gvt_plane_info;if (WARN_ON (!plane_info))return ERR_PTR (-EINVAL );@@ -81,9 +82,9 @@ static struct sg_table *intel_vgpu_gem_get_pages(static void intel_vgpu_gem_put_pages (struct drm_i915_gem_object *obj,struct sg_table *pages) { -struct intel_vgpu_plane_info *plane_info; +struct plane_info *plane_info; - plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info; + plane_info = (struct plane_info *)obj->gvt_plane_info;if (WARN_ON (!plane_info))return ;@@ -98,7 +99,7 @@ static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = { };static struct drm_i915_gem_object *intel_vgpu_create_gem (struct drm_device *dev, -struct intel_vgpu_plane_info *info) +struct plane_info *info) {struct drm_i915_private *pri = dev->dev_private;struct drm_i915_gem_object *obj;@@ -141,14 +142,14 @@ static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,return obj; } -static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info ( +static struct plane_info *intel_vgpu_get_plane_info (struct drm_device *dev,struct intel_vgpu *vgpu,uint32_t plane_id) {struct drm_i915_private *dev_priv = dev->dev_private;struct intel_vgpu_primary_plane_format *p;struct intel_vgpu_cursor_plane_format *c; -struct intel_vgpu_plane_info *info; +struct plane_info *info;struct intel_vgpu_pipe_format *pipe; info =kmalloc (sizeof (*info),GFP_KERNEL );@@ -159,7 +160,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(if (pipe ==NULL )return NULL ; -if (plane_id ==INTEL_GVT_PLANE_PRIMARY ) { +if (plane_id ==VFIO_PRIMARY_PLANE ) { p = &pipe->primary;if (p !=NULL ) { info->start = p->base;@@ -175,7 +176,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(gvt_vgpu_err ("invalid primary plane\n" );return NULL ; } - }else if (plane_id ==INTEL_GVT_PLANE_CURSOR ) { + }else if (plane_id ==VFIO_CURSOR_PLANE ) { c = &pipe->cursor;if (c !=NULL ) { info->start = c->base;@@ -228,7 +229,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args) {struct drm_device *dev = &vgpu->gvt->dev_priv->drm; -struct intel_vgpu_plane_info *info = args; +struct plane_info *info = args; info =intel_vgpu_get_plane_info (dev, vgpu, info->plane_id);if (info ==NULL )@@ -242,8 +243,8 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args)struct dma_buf *dmabuf;struct drm_i915_gem_object *obj;struct drm_device *dev = &vgpu->gvt->dev_priv->drm; -struct intel_vgpu_dmabuf *gvt_dmabuf = args; -struct intel_vgpu_plane_info *info; +struct dmabuf_info *gvt_dmabuf = args; +struct plane_info *info;int ret; info = intel_vgpu_get_plane_info(dev, vgpu,drivers/gpu/drm/i915/gvt/dmabuf.h +0
-21@@ -26,27 +26,6 @@#ifndef _GVT_DMABUF_H_#define _GVT_DMABUF_H_ -struct intel_vgpu_plane_info { -uint32_t plane_id; -uint32_t drm_format; -uint32_t width; -uint32_t height; -uint32_t stride; -uint32_t start; -uint32_t x_pos; -uint32_t y_pos; -uint32_t size; -uint64_t drm_format_mod; -}; - -#define INTEL_VGPU_QUERY_PLANE 0 -#define INTEL_VGPU_GENERATE_DMABUF 1 - -struct intel_vgpu_dmabuf { -uint32_t fd; -struct intel_vgpu_plane_info plane_info; -}; -int intel_vgpu_query_plane (struct intel_vgpu *vgpu,void *args);int intel_vgpu_create_dmabuf (struct intel_vgpu *vgpu,void *args);drivers/gpu/drm/i915/gvt/fb_decoder.h +0
-2@@ -81,8 +81,6 @@#define _SPRITE_OFFSET_START_X_MASK (0x1fff << _SPRITE_OFFSET_START_X_SHIFT)#define _SPRITE_OFFSET_START_Y_MASK (0xfff << _SPRITE_OFFSET_START_Y_SHIFT) -#define INTEL_GVT_PLANE_PRIMARY 1 -#define INTEL_GVT_PLANE_CURSOR 2#define INTEL_GVT_PLANE_SPRITE 3enum GVT_FB_EVENT {drivers/gpu/drm/i915/gvt/gvt.c +2
-0@@ -54,6 +54,8 @@ static const struct intel_gvt_ops intel_gvt_ops = { .vgpu_reset = intel_gvt_reset_vgpu, .vgpu_activate = intel_gvt_activate_vgpu, .vgpu_deactivate = intel_gvt_deactivate_vgpu, + .vgpu_query_plane = intel_vgpu_query_plane, + .vgpu_create_dmabuf = intel_vgpu_create_dmabuf, };/** drivers/gpu/drm/i915/gvt/gvt.h +4
-0@@ -185,8 +185,10 @@ struct intel_vgpu {struct kvm *kvm;struct work_struct release_work;atomic_t released; +struct vfio_device *vfio_device; } vdev;#endif +int dmabuf_mgr_fd; }; struct intel_gvt_gm {@@ -467,6 +469,8 @@ struct intel_gvt_ops {void (*vgpu_reset)(struct intel_vgpu *);void (*vgpu_activate)(struct intel_vgpu *);void (*vgpu_deactivate)(struct intel_vgpu *); +int (*vgpu_query_plane)(struct intel_vgpu *vgpu,void *); +int (*vgpu_create_dmabuf)(struct intel_vgpu *vgpu,void *); };drivers/gpu/drm/i915/gvt/kvmgt.c +107
-0@@ -41,6 +41,7 @@#include <linux/kvm_host.h> #include <linux/vfio.h> #include <linux/mdev.h> +#include <linux/anon_inodes.h> #include "i915_drv.h" #include "gvt.h" @@ -524,6 +525,85 @@ static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu)return ret; } +static int intel_vgpu_dmabuf_mgr_fd_mmap (struct file *file, +struct vm_area_struct *vma) +{ +return -EPERM ; +} + +static int intel_vgpu_dmabuf_mgr_fd_release (struct inode *inode, +struct file *filp) +{ +struct intel_vgpu *vgpu = filp->private_data; + +if (WARN_ON (!vgpu->vdev.vfio_device)) +return -EINVAL ; + +vfio_device_put (vgpu->vdev.vfio_device); + +return 0 ; +} + +static long intel_vgpu_dmabuf_mgr_fd_ioctl (struct file *filp, +unsigned int ioctl,unsigned long arg) +{ +struct intel_vgpu *vgpu = filp->private_data; +int minsz; +int ret; +struct fd f; + + f =fdget (vgpu->dmabuf_mgr_fd); +if (!f.file) +return -EBADF ; + +if (ioctl ==VFIO_DEVICE_QUERY_PLANE ) { +struct plane_info info; + + minsz = offsetofend(struct plane_info, drm_format_mod); +if (copy_from_user (&info, (void __user *)arg, minsz)) { +fdput (f); +return -EFAULT ; + } + ret = intel_gvt_ops->vgpu_query_plane (vgpu, &info); +if (ret !=0 ) { +fdput (f); +gvt_vgpu_err ("query plane failed:%d\n" , ret); +return -EINVAL ; + } +fdput (f); +return copy_to_user ((void __user *)arg, &info, minsz) ? + -EFAULT :0 ; + }else if (ioctl ==VFIO_DEVICE_CREATE_DMABUF ) { +struct dmabuf_info dmabuf; + + minsz = offsetofend(struct dmabuf_info, plane_info); +if (copy_from_user (&dmabuf, (void __user *)arg, minsz)) { +fdput (f); +return -EFAULT ; + } + ret = intel_gvt_ops->vgpu_create_dmabuf (vgpu, &dmabuf); +if (ret !=0 ) { +fdput (f); +gvt_vgpu_err ("create dmabuf failed:%d\n" , ret); +return -EINVAL ; + } +fdput (f); +return copy_to_user ((void __user *)arg, &dmabuf, minsz) ? + -EFAULT :0 ; + } + +fdput (f); +gvt_vgpu_err ("unsupported dmabuf operation\n" ); + +return -EINVAL ; +} + +static const struct file_operations intel_vgpu_dmabuf_mgr_fd_ops = { + .release = intel_vgpu_dmabuf_mgr_fd_release, + .unlocked_ioctl = intel_vgpu_dmabuf_mgr_fd_ioctl, + .mmap = intel_vgpu_dmabuf_mgr_fd_mmap, + .llseek = noop_llseek, +};static int intel_vgpu_create (struct kobject *kobj,struct mdev_device *mdev) {struct intel_vgpu *vgpu =NULL ;@@ -1259,6 +1339,33 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, }else if (cmd ==VFIO_DEVICE_RESET ) { intel_gvt_ops->vgpu_reset (vgpu);return 0 ; + }else if (cmd ==VFIO_DEVICE_GET_FD ) { +int fd; +u32 type; +struct vfio_device *device; + +if (copy_from_user (&type, (void __user *)arg,sizeof (type))) +return -EINVAL ; +if (type !=VFIO_DEVICE_DMABUF_MGR_FD ) +return -EINVAL ; + + device =vfio_device_get_from_dev (mdev_dev (mdev)); +if (device ==NULL ) { +gvt_vgpu_err ("kvmgt: vfio device is null\n" ); +return -EINVAL ; + } + vgpu->vdev.vfio_device = device; + + fd =anon_inode_getfd ("intel-vgpu-dmabuf-mgr-fd" , + &intel_vgpu_dmabuf_mgr_fd_ops, + vgpu,O_RDWR |O_CLOEXEC ); +if (fd <0 ) { +gvt_vgpu_err ("create dmabuf mgr fd failed\n" ); +return -EINVAL ; + } + vgpu->dmabuf_mgr_fd = fd; + +return fd; }return 0 ;include/uapi/linux/vfio.h +20
-0@@ -502,10 +502,58 @@ 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) + +#define VFIO_DEVICE_DMABUF_MGR_FD 0/* Supported fd types */ + +/* +* VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15, struct plane_info) +* Query plane information for a plane +*/ +struct plane_info { +__u32 plane_id;
include/uapi/linux/vfio.h struct vfio_pci_hot_reset { · line 534
Quoted 10 lines
include/uapi/linux/vfio.h +10
-0+__u32 drm_format; +__u32 width; +__u32 height; +__u32 stride; +__u32 start; +__u32 x_pos; +__u32 y_pos; +__u32 size; +__u64 drm_format_mod; +};
vfio ioctls should have flags and argsz or else they shouldn't be defined as _IO type.
Quoted 7 lines
include/uapi/linux/vfio.h +7
-0+ +#define VFIO_PRIMARY_PLANE 1 +#define VFIO_CURSOR_PLANE 2 + +#define VFIO_DEVICE_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15) + +/*
include/uapi/linux/vfio.h struct vfio_pci_hot_reset { · line 551
Quoted 10 lines
include/uapi/linux/vfio.h +10
-0+* VFIO_DEVICE_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16, struct dmabuf_info) +* +* Create a dma-buf for a plane +*/ +struct dmabuf_info { +__u32 fd; +struct plane_info plane_info; +}; +#define VFIO_DEVICE_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16) +
Same here. Shouldn't fd also be an __s32?
include/uapi/linux/vfio.h struct vfio_pci_hot_reset { · line 556
Quoted 6 lines
include/uapi/linux/vfio.h +2
-1/* -------- API for Type1 VFIO IOMMU -------- */ /** -* VFIO_IOMMU_GET_INFO - _IOR(VFIO_TYPE, VFIO_BASE + 12, struct vfio_iommu_info) +* VFIO_IOMMU_GET_INFO - _IOR(VFIO_TYPE, VFIO_BASE + 12, +* struct vfio_iommu_info)
Unrelated?
include/uapi/linux/vfio.h struct vfio_pci_hot_reset { · line 559
Quoted 3 lines
include/uapi/linux/vfio.h +0
-0* * Retrieve information about the IOMMU object. Fills in provided * struct vfio_iommu_info. Caller sets argsz.
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Patchwork
CI✓ Fi.CI.BAT: success for drm/i915/gvt: dma-buf support for GVT-g (rev5)
In reply to Xiaoguang Chen== Series Details ==
== Series Details ==
Series: drm/i915/gvt: dma-buf support for GVT-g (rev5)
URL : https://patchwork.freedesktop.org/series/23686/
State : success
== Summary ==
Series 23686v5 drm/i915/gvt: dma-buf support for GVT-g
https://patchwork.freedesktop.org/api/1.0/series/23686/revisions/5/mbox/
Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS (fi-snb-2600) fdo#100125
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
pass -> SKIP (fi-snb-2520m) fdo#101048fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#101048 https://bugs.freedesktop.org/show_bug.cgi?id=101048
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:445s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:432s fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:587s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:515s fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:485s fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:489s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:419s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:487s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:464s fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:468s fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:568s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:457s fi-skl-6700hq total:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:436s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:462s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:499s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:440s fi-snb-2520m total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:532s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:405s
46468fb47d8b68443c409eadd0414e2bd5fb8738 drm-tip: 2017y-05m-23d-10h-55m-00s UTC integration manifest 4c03515 drm/i915/gvt: Adding interface so user space can get the dma-buf
73c94cf drm/i915/gvt: Dmabuf support for GVT-g
c7d7456 drm/i915/gvt: Frame buffer decoder support for GVT-g
fe17891 drm/i915/gvt: OpRegion support for GVT-g
1acfa90 drm/i915/gvt: Extend the GVT-g architecture to support vfio device region
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4785/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx