[PATCH v2] drm/i915/gvt: Pin vgpu dma address before using
Zhenyu Wang
zhenyuw at linux.intel.com
Fri Dec 13 03:18:29 UTC 2019
On 2019.12.12 16:33:29 +0800, Zhenyu Wang wrote:
> On 2019.12.12 22:13:42 +0800, Tina Zhang wrote:
> > Dma-buf display uses the vgpu dma address saved in the guest part GGTT
> > table which is updated by vCPU thread. In host side, when the dma
> > address is used by qemu ui thread, gvt-g must make sure the dma address
> > is validated before letting it go to the HW. Invalid guest dma address
> > will easily cause DMA fault and make GPU hang.
> >
> > v2: Rebase
>
> Potentially we would be possible to pin undesired page longer even
> that's not used by gpu, but still a mitigation for current fault
> anyway. Hopefully this can work with display buffer tracking
> gracefully with possible user event helper. So,
>
> Acked-by: Zhenyu Wang <zhenyuw at linux.intel.com>
>
I still get below warning when applying this.
Applying: drm/i915/gvt: Pin vgpu dma address before using
[gvt-fixes d54f5d6105a4] drm/i915/gvt: Pin vgpu dma address before using
Author: Tina Zhang <tina.zhang at intel.com>
Date: Thu Dec 12 22:13:42 2019 +0800
4 files changed, 97 insertions(+), 4 deletions(-)
d54f5d6105a4 (HEAD -> gvt-fixes) drm/i915/gvt: Pin vgpu dma address before using
-:27: CHECK: Alignment should match open parenthesis
#27: FILE: drivers/gpu/drm/i915/gvt/dmabuf.c:40:
+static int vgpu_pin_dma_address(struct intel_vgpu *vgpu, unsigned long size,
+ dma_addr_t dma_addr)
-:38: CHECK: Alignment should match open parenthesis
#38: FILE: drivers/gpu/drm/i915/gvt/dmabuf.c:51:
+static void vgpu_unpin_dma_address(struct intel_vgpu *vgpu,
+ dma_addr_t dma_addr)
-:193: CHECK: Lines should not end with a '('
#193: FILE: drivers/gpu/drm/i915/gvt/mpt.h:265:
+static inline int intel_gvt_hypervisor_dma_pin_guest_page(
total: 0 errors, 0 warnings, 3 checks, 161 lines checked
We've got several times warning of style issue on old code...
Please pay attention for that next time.
> >
> > Signed-off-by: Tina Zhang <tina.zhang at intel.com>
> > ---
> > drivers/gpu/drm/i915/gvt/dmabuf.c | 62 ++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/gvt/hypercall.h | 2 +
> > drivers/gpu/drm/i915/gvt/kvmgt.c | 23 +++++++++++
> > drivers/gpu/drm/i915/gvt/mpt.h | 14 +++++++
> > 4 files changed, 97 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > index e451298d11c3..9836ce6119a8 100644
> > --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> > +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> > @@ -36,13 +36,31 @@
> >
> > #define GEN8_DECODE_PTE(pte) (pte & GENMASK_ULL(63, 12))
> >
> > +static int vgpu_pin_dma_address(struct intel_vgpu *vgpu, unsigned long size,
> > + dma_addr_t dma_addr)
> > +{
> > + int ret = 0;
> > +
> > + if (intel_gvt_hypervisor_dma_pin_guest_page(vgpu, dma_addr))
> > + ret = -EINVAL;
> > +
> > + return ret;
> > +}
> > +
> > +static void vgpu_unpin_dma_address(struct intel_vgpu *vgpu,
> > + dma_addr_t dma_addr)
> > +{
> > + intel_gvt_hypervisor_dma_unmap_guest_page(vgpu, dma_addr);
> > +}
> > +
> > static int vgpu_gem_get_pages(
> > struct drm_i915_gem_object *obj)
> > {
> > struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
> > + struct intel_vgpu *vgpu;
> > struct sg_table *st;
> > struct scatterlist *sg;
> > - int i, ret;
> > + int i, j, ret;
> > gen8_pte_t __iomem *gtt_entries;
> > struct intel_vgpu_fb_info *fb_info;
> > u32 page_num;
> > @@ -51,6 +69,10 @@ static int vgpu_gem_get_pages(
> > if (WARN_ON(!fb_info))
> > return -ENODEV;
> >
> > + vgpu = fb_info->obj->vgpu;
> > + if (WARN_ON(!vgpu))
> > + return -ENODEV;
> > +
> > st = kmalloc(sizeof(*st), GFP_KERNEL);
> > if (unlikely(!st))
> > return -ENOMEM;
> > @@ -64,21 +86,53 @@ static int vgpu_gem_get_pages(
> > gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
> > (fb_info->start >> PAGE_SHIFT);
> > for_each_sg(st->sgl, sg, page_num, i) {
> > + dma_addr_t dma_addr =
> > + GEN8_DECODE_PTE(readq(>t_entries[i]));
> > + if (vgpu_pin_dma_address(vgpu, PAGE_SIZE, dma_addr)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > sg->offset = 0;
> > sg->length = PAGE_SIZE;
> > - sg_dma_address(sg) =
> > - GEN8_DECODE_PTE(readq(>t_entries[i]));
> > sg_dma_len(sg) = PAGE_SIZE;
> > + sg_dma_address(sg) = dma_addr;
> > }
> >
> > __i915_gem_object_set_pages(obj, st, PAGE_SIZE);
> > +out:
> > + if (ret) {
> > + dma_addr_t dma_addr;
> > +
> > + for_each_sg(st->sgl, sg, i, j) {
> > + dma_addr = sg_dma_address(sg);
> > + if (dma_addr)
> > + vgpu_unpin_dma_address(vgpu, dma_addr);
> > + }
> > + sg_free_table(st);
> > + kfree(st);
> > + }
> > +
> > + return ret;
> >
> > - return 0;
> > }
> >
> > static void vgpu_gem_put_pages(struct drm_i915_gem_object *obj,
> > struct sg_table *pages)
> > {
> > + struct scatterlist *sg;
> > +
> > + if (obj->base.dma_buf) {
> > + struct intel_vgpu_fb_info *fb_info = obj->gvt_info;
> > + struct intel_vgpu_dmabuf_obj *obj = fb_info->obj;
> > + struct intel_vgpu *vgpu = obj->vgpu;
> > + int i;
> > +
> > + for_each_sg(pages->sgl, sg, fb_info->size, i)
> > + vgpu_unpin_dma_address(vgpu,
> > + sg_dma_address(sg));
> > + }
> > +
> > sg_free_table(pages);
> > kfree(pages);
> > }
> > diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> > index 9599c0a762b2..b17c4a1599cd 100644
> > --- a/drivers/gpu/drm/i915/gvt/hypercall.h
> > +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> > @@ -66,6 +66,8 @@ struct intel_gvt_mpt {
> > unsigned long size, dma_addr_t *dma_addr);
> > void (*dma_unmap_guest_page)(unsigned long handle, dma_addr_t dma_addr);
> >
> > + int (*dma_pin_guest_page)(unsigned long handle, dma_addr_t dma_addr);
> > +
> > int (*map_gfn_to_mfn)(unsigned long handle, unsigned long gfn,
> > unsigned long mfn, unsigned int nr, bool map);
> > int (*set_trap_area)(unsigned long handle, u64 start, u64 end,
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index a699ecade3fc..bd79a9718cc7 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -1914,6 +1914,28 @@ static int kvmgt_dma_map_guest_page(unsigned long handle, unsigned long gfn,
> > return ret;
> > }
> >
> > +static int kvmgt_dma_pin_guest_page(unsigned long handle, dma_addr_t dma_addr)
> > +{
> > + struct kvmgt_guest_info *info;
> > + struct gvt_dma *entry;
> > + int ret = 0;
> > +
> > + if (!handle_valid(handle))
> > + return -ENODEV;
> > +
> > + info = (struct kvmgt_guest_info *)handle;
> > +
> > + mutex_lock(&info->vgpu->vdev.cache_lock);
> > + entry = __gvt_cache_find_dma_addr(info->vgpu, dma_addr);
> > + if (entry)
> > + kref_get(&entry->ref);
> > + else
> > + ret = -ENOMEM;
> > + mutex_unlock(&info->vgpu->vdev.cache_lock);
> > +
> > + return ret;
> > +}
> > +
> > static void __gvt_dma_release(struct kref *ref)
> > {
> > struct gvt_dma *entry = container_of(ref, typeof(*entry), ref);
> > @@ -2025,6 +2047,7 @@ static struct intel_gvt_mpt kvmgt_mpt = {
> > .gfn_to_mfn = kvmgt_gfn_to_pfn,
> > .dma_map_guest_page = kvmgt_dma_map_guest_page,
> > .dma_unmap_guest_page = kvmgt_dma_unmap_guest_page,
> > + .dma_pin_guest_page = kvmgt_dma_pin_guest_page,
> > .set_opregion = kvmgt_set_opregion,
> > .set_edid = kvmgt_set_edid,
> > .get_vfio_device = kvmgt_get_vfio_device,
> > diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> > index 0f9440128123..3bc40b0ba4cf 100644
> > --- a/drivers/gpu/drm/i915/gvt/mpt.h
> > +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> > @@ -254,6 +254,20 @@ static inline void intel_gvt_hypervisor_dma_unmap_guest_page(
> > intel_gvt_host.mpt->dma_unmap_guest_page(vgpu->handle, dma_addr);
> > }
> >
> > +/**
> > + * intel_gvt_hypervisor_dma_pin_guest_page - pin guest dma buf
> > + * @vgpu: a vGPU
> > + * @dma_addr: guest dma addr
> > + *
> > + * Returns:
> > + * 0 on success, negative error code if failed.
> > + */
> > +static inline int intel_gvt_hypervisor_dma_pin_guest_page(
> > + struct intel_vgpu *vgpu, dma_addr_t dma_addr)
> > +{
> > + return intel_gvt_host.mpt->dma_pin_guest_page(vgpu->handle, dma_addr);
> > +}
> > +
> > /**
> > * intel_gvt_hypervisor_map_gfn_to_mfn - map a GFN region to MFN
> > * @vgpu: a vGPU
> > --
> > 2.17.1
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
>
> --
> Open Source Technology Center, Intel ltd.
>
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20191213/35c99ab4/attachment.sig>
More information about the intel-gvt-dev
mailing list