Hi Dmitry,
> Subject: Re: [PATCH v4 2/5] drm/virtio: Add a helper to map and note the
> dma addrs and lengths
>
> > +int virtgpu_dma_buf_import_sgt(struct virtio_gpu_mem_entry **ents,
> > + unsigned int *nents,
> > + struct virtio_gpu_object *bo,
> > + struct dma_buf_attachment *attach)
> > +{
> > + struct scatterlist *sl;
> > + struct sg_table *sgt;
> > + long i, ret;
> > +
> > + dma_resv_assert_held(attach->dmabuf->resv);
> > +
> > + ret = dma_resv_wait_timeout(attach->dmabuf->resv,
> > + DMA_RESV_USAGE_KERNEL,
> > + false, MAX_SCHEDULE_TIMEOUT);
> > + if (ret <= 0)
> > + return ret < 0 ? ret : -ETIMEDOUT;
> > +
> > + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> > + if (IS_ERR(sgt))
> > + return PTR_ERR(sgt);
> > +
> > + *ents = kvmalloc_array(sgt->nents,
> > + sizeof(struct virtio_gpu_mem_entry),
> > + GFP_KERNEL);
> > + if (!(*ents)) {
> > + dma_buf_unmap_attachment(attach, sgt,
> DMA_BIDIRECTIONAL);
> > + return -ENOMEM;
> > + }
> > +
> > + *nents = sgt->nents;
> > + for_each_sgtable_dma_sg(sgt, sl, i) {
> > + (*ents)[i].addr = cpu_to_le64(sg_dma_address(sl));
> > + (*ents)[i].length = cpu_to_le32(sg_dma_len(sl));
> > + (*ents)[i].padding = 0;
> > + }
> > +
> > + bo->sgt = sgt;
> > + bo->detached = false;
>
> This bo->detached=false was missing in v3 and it fixes the flickering
> problem, as expected. \o/
>
> The DG2+virtio-gpu still don't work for me in guest using both i915 and
> Xe drivers like before:
>
> Invalid read at addr 0x7200005014, size 1, region '(null)', reason: rejected
> Invalid write at addr 0x7200005014, size 1, region '(null)', reason:
> rejected
> Invalid write at addr 0x7200005000, size 4, region '(null)', reason:
> rejected
> Invalid read at addr 0x7200005004, size 4, region '(null)', reason: rejected
> Invalid write at addr 0x7200005000, size 4, region '(null)', reason:
> rejected
> Invalid read at addr 0x7200005004, size 4, region '(null)', reason: rejected
> [ 0.876189] virtio_gpu virtio1: virtio: device uses modern interface
> but does not have VIRTIO_F_VERSION_1
> Invalid read at addr 0x7200005014, size 1, region '(null)', reason: rejected
> Invalid write at addr 0x7200005014, size 1, region '(null)', reason:
> rejected
> [ 0.876597] virtio_gpu virtio1: probe with driver virtio_gpu failed
> with error -22
>
> Wondering if it could be a problem with my guest kernel config. I
> attached my config to the email, please try to boot guest with my config
> if you'll have time.
Sure, let me try to test with your config. Could you also please share your
Qemu launch parameters?
>
> I'm almost ready to apply the patches after a small code improvement.
> Let's move bo->detached=false into virtio_gpu_object_attach() and
> replace bo->detached with bo->attached. I can apply this change while
> merging patches, otherwise please make v5 if you agree with the suggestion:
I definitely agree with your suggestion; attached is much more intuitive than
detached. I'll include the following changes (along with your other suggestions) in v5.
Thanks,
Vivek
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index ad6960aa464e..8328107edf78 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -93,7 +93,7 @@ struct virtio_gpu_object {
> uint32_t hw_res_handle;
> bool dumb;
> bool created;
> - bool detached;
> + bool attached;
> bool host3d_blob, guest_blob;
> uint32_t blob_mem, blob_flags;
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
> b/drivers/gpu/drm/virtio/virtgpu_object.c
> index 2b19bb8c6ec3..5517cff8715c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -105,7 +105,7 @@ int virtio_gpu_detach_object_fenced(struct
> virtio_gpu_object *bo)
> struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
> struct virtio_gpu_fence *fence;
>
> - if (bo->detached)
> + if (!bo->attached)
> return 0;
>
> fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0);
> @@ -118,8 +118,6 @@ int virtio_gpu_detach_object_fenced(struct
> virtio_gpu_object *bo)
> dma_fence_wait(&fence->f, false);
> dma_fence_put(&fence->f);
>
> - bo->detached = true;
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c
> b/drivers/gpu/drm/virtio/virtgpu_prime.c
> index 5031b800c521..3c4ea592e23c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> @@ -181,7 +181,6 @@ int virtgpu_dma_buf_import_sgt(struct
> virtio_gpu_mem_entry **ents,
> }
>
> bo->sgt = sgt;
> - bo->detached = false;
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
> b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index b8b296b912a1..ad91624df42d 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -1120,16 +1120,26 @@ void virtio_gpu_object_attach(struct
> virtio_gpu_device *vgdev,
> struct virtio_gpu_mem_entry *ents,
> unsigned int nents)
> {
> + if (obj->attached)
> + return;
> +
> virtio_gpu_cmd_resource_attach_backing(vgdev, obj-
> >hw_res_handle,
> ents, nents, NULL);
> +
> + obj->attached = true;
> }
>
> void virtio_gpu_object_detach(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_object *obj,
> struct virtio_gpu_fence *fence)
> {
> + if (!obj->attached)
> + return;
> +
> virtio_gpu_cmd_resource_detach_backing(vgdev, obj-
> >hw_res_handle,
> fence);
> +
> + obj->attached = false;
> }
>
> void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev,
>
>
>
> --
> Best regards,
> Dmitry
