| Message ID | 20240110095627.227454-2-julia.zhang@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement device_attach for virtio gpu | expand |
Am 10.01.24 um 10:56 schrieb Julia Zhang: > drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be > implemented, or else return ENOSYS. Virtio has no get_sg_table > implemented for vram object. To fix this, add a new device_attach to > call drm_gem_map_attach() for shmem object and return 0 for vram object > instead of calling drm_gem_map_attach for both of these two kinds of > object. Well as far as I can see this is nonsense from the DMA-buf side of things. SG tables are always needed as long as you don't re-import the same object into your driver and then you shouldn't end up in this function in the first place. So that drm_gem_map_attach() requires get_sg_table to be implemented is intentional and should never be overridden like this. Regards, Christian. > > Signed-off-by: Julia Zhang <julia.zhang@amd.com> > --- > drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c > index 44425f20d91a..f0b0ff6f3813 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c > @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > drm_gem_unmap_dma_buf(attach, sgt, dir); > } > > +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, > + struct dma_buf_attachment *attach) > +{ > + struct drm_gem_object *obj = attach->dmabuf->priv; > + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); > + > + if (virtio_gpu_is_vram(bo)) > + return 0; > + > + return drm_gem_map_attach(dma_buf, attach); > +} > + > static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > .ops = { > .cache_sgt_mapping = true, > @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > .vmap = drm_gem_dmabuf_vmap, > .vunmap = drm_gem_dmabuf_vunmap, > }, > - .device_attach = drm_gem_map_attach, > + .device_attach = virtgpu_gem_device_attach, > .get_uuid = virtgpu_virtio_get_uuid, > }; >
On Wed, Jan 10, 2024 at 05:56:28PM +0800, Julia Zhang wrote: > drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be > implemented, or else return ENOSYS. Virtio has no get_sg_table > implemented for vram object. To fix this, add a new device_attach to > call drm_gem_map_attach() for shmem object and return 0 for vram object > instead of calling drm_gem_map_attach for both of these two kinds of > object. > > Signed-off-by: Julia Zhang <julia.zhang@amd.com> > --- > drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c > index 44425f20d91a..f0b0ff6f3813 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c > @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > drm_gem_unmap_dma_buf(attach, sgt, dir); > } > > +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, > + struct dma_buf_attachment *attach) > +{ > + struct drm_gem_object *obj = attach->dmabuf->priv; > + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); > + > + if (virtio_gpu_is_vram(bo)) > + return 0; You need to reject attach here because these vram buffer objects cannot be used by any other driver. In that case dma_buf_attach _must_ fail, not silently succeed. Because if it silently succeeds then the subsequent dma_buf_map_attachment will blow up because you don't have the ->get_sg_table hook implemented. Per the documentation the error code for this case must be -EBUSY, see the section for the attach hook here: https://dri.freedesktop.org/docs/drm/driver-api/dma-buf.html#c.dma_buf_ops Since you're looking into this area, please make sure there's not other similar mistake in virtio code. Also can you please make a kerneldoc patch for struct virtio_dma_buf_ops to improve the documentation there? I think it would be good to move those to the inline style and then at least put a kernel-doc hyperlink to struct dma_buf_ops.attach and mention that attach must fail for non-shareable buffers. In general the virtio_dma_buf kerneldoc seems to be on the "too minimal, explains nothing" side of things :-/ Cheers, Sima > + > + return drm_gem_map_attach(dma_buf, attach); > +} > + > static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > .ops = { > .cache_sgt_mapping = true, > @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > .vmap = drm_gem_dmabuf_vmap, > .vunmap = drm_gem_dmabuf_vunmap, > }, > - .device_attach = drm_gem_map_attach, > + .device_attach = virtgpu_gem_device_attach, > .get_uuid = virtgpu_virtio_get_uuid, > }; > > -- > 2.34.1 >
On Wed, Jan 10, 2024 at 11:19:37AM +0100, Christian König wrote: > Am 10.01.24 um 10:56 schrieb Julia Zhang: > > drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be > > implemented, or else return ENOSYS. Virtio has no get_sg_table > > implemented for vram object. To fix this, add a new device_attach to > > call drm_gem_map_attach() for shmem object and return 0 for vram object > > instead of calling drm_gem_map_attach for both of these two kinds of > > object. > > Well as far as I can see this is nonsense from the DMA-buf side of things. > > SG tables are always needed as long as you don't re-import the same object > into your driver and then you shouldn't end up in this function in the first > place. > > So that drm_gem_map_attach() requires get_sg_table to be implemented is > intentional and should never be overridden like this. See my reply, tldr; you're allowed to reject ->attach with -EBUSY to handle exactly this case of non-shareable buffer types. But definitely don't silently fail, that's a "we'll oops on map_attachment" kind of bug :-) -Sima > > Regards, > Christian. > > > > > Signed-off-by: Julia Zhang <julia.zhang@amd.com> > > --- > > drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c > > index 44425f20d91a..f0b0ff6f3813 100644 > > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c > > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c > > @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > > drm_gem_unmap_dma_buf(attach, sgt, dir); > > } > > +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, > > + struct dma_buf_attachment *attach) > > +{ > > + struct drm_gem_object *obj = attach->dmabuf->priv; > > + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); > > + > > + if (virtio_gpu_is_vram(bo)) > > + return 0; > > + > > + return drm_gem_map_attach(dma_buf, attach); > > +} > > + > > static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > > .ops = { > > .cache_sgt_mapping = true, > > @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > > .vmap = drm_gem_dmabuf_vmap, > > .vunmap = drm_gem_dmabuf_vunmap, > > }, > > - .device_attach = drm_gem_map_attach, > > + .device_attach = virtgpu_gem_device_attach, > > .get_uuid = virtgpu_virtio_get_uuid, > > }; >
Am 10.01.24 um 11:22 schrieb Daniel Vetter: > On Wed, Jan 10, 2024 at 11:19:37AM +0100, Christian König wrote: >> Am 10.01.24 um 10:56 schrieb Julia Zhang: >>> drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be >>> implemented, or else return ENOSYS. Virtio has no get_sg_table >>> implemented for vram object. To fix this, add a new device_attach to >>> call drm_gem_map_attach() for shmem object and return 0 for vram object >>> instead of calling drm_gem_map_attach for both of these two kinds of >>> object. >> Well as far as I can see this is nonsense from the DMA-buf side of things. >> >> SG tables are always needed as long as you don't re-import the same object >> into your driver and then you shouldn't end up in this function in the first >> place. >> >> So that drm_gem_map_attach() requires get_sg_table to be implemented is >> intentional and should never be overridden like this. > See my reply, tldr; you're allowed to reject ->attach with -EBUSY to > handle exactly this case of non-shareable buffer types. But definitely > don't silently fail, that's a "we'll oops on map_attachment" kind of bug > :-) Ah, yes that makes much more sense! So basically just the "return 0;" needs to be "return -EBUSY;". Regards, Christian. > -Sima > >> Regards, >> Christian. >> >>> Signed-off-by: Julia Zhang <julia.zhang@amd.com> >>> --- >>> drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c >>> index 44425f20d91a..f0b0ff6f3813 100644 >>> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c >>> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c >>> @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, >>> drm_gem_unmap_dma_buf(attach, sgt, dir); >>> } >>> +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, >>> + struct dma_buf_attachment *attach) >>> +{ >>> + struct drm_gem_object *obj = attach->dmabuf->priv; >>> + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); >>> + >>> + if (virtio_gpu_is_vram(bo)) >>> + return 0; >>> + >>> + return drm_gem_map_attach(dma_buf, attach); >>> +} >>> + >>> static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { >>> .ops = { >>> .cache_sgt_mapping = true, >>> @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { >>> .vmap = drm_gem_dmabuf_vmap, >>> .vunmap = drm_gem_dmabuf_vunmap, >>> }, >>> - .device_attach = drm_gem_map_attach, >>> + .device_attach = virtgpu_gem_device_attach, >>> .get_uuid = virtgpu_virtio_get_uuid, >>> };
On Wed, Jan 10, 2024 at 11:46:35AM +0100, Christian König wrote: > Am 10.01.24 um 11:22 schrieb Daniel Vetter: > > On Wed, Jan 10, 2024 at 11:19:37AM +0100, Christian König wrote: > > > Am 10.01.24 um 10:56 schrieb Julia Zhang: > > > > drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be > > > > implemented, or else return ENOSYS. Virtio has no get_sg_table > > > > implemented for vram object. To fix this, add a new device_attach to > > > > call drm_gem_map_attach() for shmem object and return 0 for vram object > > > > instead of calling drm_gem_map_attach for both of these two kinds of > > > > object. > > > Well as far as I can see this is nonsense from the DMA-buf side of things. > > > > > > SG tables are always needed as long as you don't re-import the same object > > > into your driver and then you shouldn't end up in this function in the first > > > place. > > > > > > So that drm_gem_map_attach() requires get_sg_table to be implemented is > > > intentional and should never be overridden like this. > > See my reply, tldr; you're allowed to reject ->attach with -EBUSY to > > handle exactly this case of non-shareable buffer types. But definitely > > don't silently fail, that's a "we'll oops on map_attachment" kind of bug > > :-) > > Ah, yes that makes much more sense! > > So basically just the "return 0;" needs to be "return -EBUSY;". Well plus 2nd patch to polish the virtio_dma_buf docs a bit, that would be nice :-D -Sima > > Regards, > Christian. > > > -Sima > > > > > Regards, > > > Christian. > > > > > > > Signed-off-by: Julia Zhang <julia.zhang@amd.com> > > > > --- > > > > drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- > > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c > > > > index 44425f20d91a..f0b0ff6f3813 100644 > > > > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c > > > > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c > > > > @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, > > > > drm_gem_unmap_dma_buf(attach, sgt, dir); > > > > } > > > > +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, > > > > + struct dma_buf_attachment *attach) > > > > +{ > > > > + struct drm_gem_object *obj = attach->dmabuf->priv; > > > > + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); > > > > + > > > > + if (virtio_gpu_is_vram(bo)) > > > > + return 0; > > > > + > > > > + return drm_gem_map_attach(dma_buf, attach); > > > > +} > > > > + > > > > static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > > > > .ops = { > > > > .cache_sgt_mapping = true, > > > > @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { > > > > .vmap = drm_gem_dmabuf_vmap, > > > > .vunmap = drm_gem_dmabuf_vunmap, > > > > }, > > > > - .device_attach = drm_gem_map_attach, > > > > + .device_attach = virtgpu_gem_device_attach, > > > > .get_uuid = virtgpu_virtio_get_uuid, > > > > }; >
On 2024/1/10 18:21, Daniel Vetter wrote: > On Wed, Jan 10, 2024 at 05:56:28PM +0800, Julia Zhang wrote: >> drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be >> implemented, or else return ENOSYS. Virtio has no get_sg_table >> implemented for vram object. To fix this, add a new device_attach to >> call drm_gem_map_attach() for shmem object and return 0 for vram object >> instead of calling drm_gem_map_attach for both of these two kinds of >> object. >> >> Signed-off-by: Julia Zhang <julia.zhang@amd.com> >> --- >> drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c >> index 44425f20d91a..f0b0ff6f3813 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c >> @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, >> drm_gem_unmap_dma_buf(attach, sgt, dir); >> } >> >> +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, >> + struct dma_buf_attachment *attach) >> +{ >> + struct drm_gem_object *obj = attach->dmabuf->priv; >> + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); >> + >> + if (virtio_gpu_is_vram(bo)) >> + return 0; > > You need to reject attach here because these vram buffer objects cannot be > used by any other driver. In that case dma_buf_attach _must_ fail, not > silently succeed. > Do you mean these vram buffer objects should not be imported by other drivers? > Because if it silently succeeds then the subsequent dma_buf_map_attachment > will blow up because you don't have the ->get_sg_table hook implemented. > I saw only this call stack would call ->get_sg_table: #0 ->get_sg_table #1 drm_gem_map_dma_buf #2 virtgpu_gem_map_dma_buf #3 __map_dma_buf #4 dma_buf_dynamic_attach #5 amdgpu_gem_prime_import this stack is for shmem object and it requires ->get_sg_table get implemented. But for vram object, __map_dma_buf will call like this: #0 sg_alloc_table #1 virtio_gpu_vram_map_dma_buf #2 virtgpu_gem_map_dma_buf #3 __map_dma_buf #4 dma_buf_dynamic_attach #5 amdgpu_gem_prime_import which will not call ->get_sg_table but alloc a sg table instead and fill it from the vram object. Before __map_dma_buf, the call stack of virtgpu_gem_device_attach is: #0 virtgpu_gem_device_attach #1 virtio_dma_buf_attach #2 dma_buf_dynamic_attach #3 amdgpu_gem_prime_import So my problem is that to realize dGPU prime feature on VM, I actually want dma_buf_attach succeed for vram object so that passthrough dGPU can import these vram objects and blit data to it. If here return -EBUSY, then amdgpu_gem_prime_import will fail and the whole feature will fail. > Per the documentation the error code for this case must be -EBUSY, see the > section for the attach hook here: > > https://dri.freedesktop.org/docs/drm/driver-api/dma-buf.html#c.dma_buf_ops > > Since you're looking into this area, please make sure there's not other > similar mistake in virtio code. > > Also can you please make a kerneldoc patch for struct virtio_dma_buf_ops > to improve the documentation there? I think it would be good to move those > to the inline style and then at least put a kernel-doc hyperlink to struct > dma_buf_ops.attach and mention that attach must fail for non-shareable > buffers. > > In general the virtio_dma_buf kerneldoc seems to be on the "too minimal, > explains nothing" side of things :-/ OK, let me take a look and try to do it. Regards, Julia > > Cheers, Sima > >> + >> + return drm_gem_map_attach(dma_buf, attach); >> +} >> + >> static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { >> .ops = { >> .cache_sgt_mapping = true, >> @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { >> .vmap = drm_gem_dmabuf_vmap, >> .vunmap = drm_gem_dmabuf_vunmap, >> }, >> - .device_attach = drm_gem_map_attach, >> + .device_attach = virtgpu_gem_device_attach, >> .get_uuid = virtgpu_virtio_get_uuid, >> }; >> >> -- >> 2.34.1 >> >
Am 11.01.24 um 09:52 schrieb Zhang, Julia: > > On 2024/1/10 18:21, Daniel Vetter wrote: >> On Wed, Jan 10, 2024 at 05:56:28PM +0800, Julia Zhang wrote: >>> drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be >>> implemented, or else return ENOSYS. Virtio has no get_sg_table >>> implemented for vram object. To fix this, add a new device_attach to >>> call drm_gem_map_attach() for shmem object and return 0 for vram object >>> instead of calling drm_gem_map_attach for both of these two kinds of >>> object. >>> >>> Signed-off-by: Julia Zhang <julia.zhang@amd.com> >>> --- >>> drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- >>> 1 file changed, 13 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c >>> index 44425f20d91a..f0b0ff6f3813 100644 >>> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c >>> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c >>> @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, >>> drm_gem_unmap_dma_buf(attach, sgt, dir); >>> } >>> >>> +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, >>> + struct dma_buf_attachment *attach) >>> +{ >>> + struct drm_gem_object *obj = attach->dmabuf->priv; >>> + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); >>> + >>> + if (virtio_gpu_is_vram(bo)) >>> + return 0; >> You need to reject attach here because these vram buffer objects cannot be >> used by any other driver. In that case dma_buf_attach _must_ fail, not >> silently succeed. >> > Do you mean these vram buffer objects should not be imported by other drivers? > >> Because if it silently succeeds then the subsequent dma_buf_map_attachment >> will blow up because you don't have the ->get_sg_table hook implemented. >> > I saw only this call stack would call ->get_sg_table: > > #0 ->get_sg_table > #1 drm_gem_map_dma_buf > #2 virtgpu_gem_map_dma_buf > #3 __map_dma_buf > #4 dma_buf_dynamic_attach > #5 amdgpu_gem_prime_import > > this stack is for shmem object and it requires ->get_sg_table get implemented. > > > But for vram object, __map_dma_buf will call like this: > > #0 sg_alloc_table > #1 virtio_gpu_vram_map_dma_buf > #2 virtgpu_gem_map_dma_buf > #3 __map_dma_buf > #4 dma_buf_dynamic_attach > #5 amdgpu_gem_prime_import > > which will not call ->get_sg_table but alloc a sg table instead and fill it from the vram object. Yeah and exactly that won't work for this use case. The VRAM of amdgpu is exposed through an MMIO BAR the guest can't access. > Before __map_dma_buf, the call stack of virtgpu_gem_device_attach is: > > #0 virtgpu_gem_device_attach > #1 virtio_dma_buf_attach > #2 dma_buf_dynamic_attach > #3 amdgpu_gem_prime_import > > So my problem is that to realize dGPU prime feature on VM, I actually want dma_buf_attach succeed > for vram object so that passthrough dGPU can import these vram objects and blit data to it. That is completely futile effort, the dGPU physically can't access that stuff or otherwise we have a major security hole in the VM. > If here return -EBUSY, then amdgpu_gem_prime_import will fail and the whole feature will fail. Yeah and that is completely intentional. Let's discuss that use case AMD internally first. Regards, Christian. > >> Per the documentation the error code for this case must be -EBUSY, see the >> section for the attach hook here: >> >> https://dri.freedesktop.org/docs/drm/driver-api/dma-buf.html#c.dma_buf_ops >> >> Since you're looking into this area, please make sure there's not other >> similar mistake in virtio code. >> >> Also can you please make a kerneldoc patch for struct virtio_dma_buf_ops >> to improve the documentation there? I think it would be good to move those >> to the inline style and then at least put a kernel-doc hyperlink to struct >> dma_buf_ops.attach and mention that attach must fail for non-shareable >> buffers. >> >> In general the virtio_dma_buf kerneldoc seems to be on the "too minimal, >> explains nothing" side of things :-/ > OK, let me take a look and try to do it. > > Regards, > Julia > >> Cheers, Sima >> >>> + >>> + return drm_gem_map_attach(dma_buf, attach); >>> +} >>> + >>> static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { >>> .ops = { >>> .cache_sgt_mapping = true, >>> @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { >>> .vmap = drm_gem_dmabuf_vmap, >>> .vunmap = drm_gem_dmabuf_vunmap, >>> }, >>> - .device_attach = drm_gem_map_attach, >>> + .device_attach = virtgpu_gem_device_attach, >>> .get_uuid = virtgpu_virtio_get_uuid, >>> }; >>> >>> -- >>> 2.34.1 >>>
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c index 44425f20d91a..f0b0ff6f3813 100644 --- a/drivers/gpu/drm/virtio/virtgpu_prime.c +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c @@ -71,6 +71,18 @@ static void virtgpu_gem_unmap_dma_buf(struct dma_buf_attachment *attach, drm_gem_unmap_dma_buf(attach, sgt, dir); } +static int virtgpu_gem_device_attach(struct dma_buf *dma_buf, + struct dma_buf_attachment *attach) +{ + struct drm_gem_object *obj = attach->dmabuf->priv; + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj); + + if (virtio_gpu_is_vram(bo)) + return 0; + + return drm_gem_map_attach(dma_buf, attach); +} + static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { .ops = { .cache_sgt_mapping = true, @@ -83,7 +95,7 @@ static const struct virtio_dma_buf_ops virtgpu_dmabuf_ops = { .vmap = drm_gem_dmabuf_vmap, .vunmap = drm_gem_dmabuf_vunmap, }, - .device_attach = drm_gem_map_attach, + .device_attach = virtgpu_gem_device_attach, .get_uuid = virtgpu_virtio_get_uuid, };
drm_gem_map_attach() requires drm_gem_object_funcs.get_sg_table to be implemented, or else return ENOSYS. Virtio has no get_sg_table implemented for vram object. To fix this, add a new device_attach to call drm_gem_map_attach() for shmem object and return 0 for vram object instead of calling drm_gem_map_attach for both of these two kinds of object. Signed-off-by: Julia Zhang <julia.zhang@amd.com> --- drivers/gpu/drm/virtio/virtgpu_prime.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)