Hi Am 12.06.20 um 20:54 schrieb Gurchetan Singh: > On Fri, Jun 12, 2020 at 3:17 AM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote: >> >> On Fri, Jun 12, 2020 at 11:47:55AM +0200, Thomas Zimmermann wrote: >>> Hi >>> >>> Am 12.06.20 um 03:36 schrieb Gurchetan Singh: >>>> This is useful for the next patch. Also, should we only unmap the >>>> amount entries that we mapped with the dma-api? >>> >>> It looks like you're moving virtio code into shmem. >> >> Well, not really. >> >> virtio has -- for historical reasons -- the oddity that it may or may >> not need to dma_map resources, depending on device configuration. >> Initially virtio went with "it's just a vm, lets simply operate on >> physical ram addresses". That shortcut turned out to be a bad idea >> later on, especially with the arrival of iommu emulation support in >> qemu. But we couldn't just scratch it for backward compatibility >> reasons. See virtio_has_iommu_quirk(). >> >> This just allows to enable/disable dma_map, I guess to fix some fallout >> from recent shmem helper changes > > Yes, the main goal is to fix the double free. > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c > b/drivers/gpu/drm/virtio/virtgpu_object.c > index 346cef5ce251..2f7b6cd25a4b 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -78,7 +78,6 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo) > shmem->mapped = 0; > } > > - sg_free_table(shmem->pages); > shmem->pages = NULL; > drm_gem_shmem_unpin(&bo->base.base); > } > > Doing only that on it's own causes log spam though > > [ 10.368385] virtio_gpu virtio5: swiotlb buffer is full (sz: 4096 > bytes), total 0 (slots), used 0 (slots) > [ 10.384957] virtio_gpu virtio5: swiotlb buffer is full (sz: 4096 > bytes), total 0 (slots), used 0 (slots) > [ 10.389920] virtio_gpu virtio5: swiotlb buffer is full (sz: 4096 > bytes), total 0 (slots), used 0 (slots) > [ 10.396859] virtio_gpu virtio5: swiotlb buffer is full (sz: 4096 > bytes), total 0 (slots), used 0 (slots) > [ 10.401954] virtio_gpu virtio5: swiotlb buffer is full (sz: 4096 > bytes), total 0 (slots), used 0 (slots) > [ 10.406694] virtio_gpu virtio5: swiotlb buffer is full (sz: 8192 > bytes), total 0 (slots), used 0 (slots) > [ 10.495744] virtio_gpu virtio5: swiotlb buffer is full (sz: 4096 > bytes), total 0 (slots), used 0 (slots) > > Plus, I just realized the virtio dma ops and ops used by drm shmem are > different, so virtio would have to unconditionally have to skip the > shmem path. Perhaps the best policy is to revert d323bb44e4d2? Can you fork the shmem code into virtio and modify it according to your needs? I know that code splitting is unpopular, but at least it's a clean solution. For some GEM object functions, you might still be able to share code with shmem helpers. Best regards Thomas > >> (Gurchetan, that kind of stuff should >> be mentioned in cover letter and commit messages). > > Good tip. > >> >> I'm not sure virtio actually needs that patch though. I *think* doing >> the dma_map+dma_unmap unconditionally, but then ignore the result in >> case we don't need it should work. And it shouldn't be a horrible >> performance hit either, in case we don't have a (virtual) iommu in the >> VM dma mapping is essentially a nop ... >> >> take care, >> Gerd >> -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
