Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drm: vc4: Block swiotlb bounce buffers being imported as dmabuf #5734

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

6by9
Copy link
Contributor

@6by9 6by9 commented Nov 22, 2023

The dmabuf import already checks that the backing buffer is contiguous and rejects it if it isn't. vc4 also requires that the buffer is in the bottom 1GB of RAM, and this is all correctly defined via dma-ranges.

However the kernel silently uses swiotlb to bounce dma buffers around if they are in the wrong region. This relies on dma sync functions to be called in order to copy the data to/from the bounce buffer.

DRM is based on all memory allocations being coherent with the GPU so that any updates to a framebuffer will be acted on without the need for any additional update. This is fairly fundamentally incompatible with needing to call dma_sync_ to handle the bounce buffer copies, and therefore we have to detect and reject mappings that use bounce buffers.

The dmabuf import already checks that the backing buffer is contiguous
and rejects it if it isn't. vc4 also requires that the buffer is
in the bottom 1GB of RAM, and this is all correctly defined via
dma-ranges.

However the kernel silently uses swiotlb to bounce dma buffers
around if they are in the wrong region. This relies on dma sync
functions to be called in order to copy the data to/from the
bounce buffer.

DRM is based on all memory allocations being coherent with the
GPU so that any updates to a framebuffer will be acted on without
the need for any additional update. This is fairly fundamentally
incompatible with needing to call dma_sync_ to handle the bounce
buffer copies, and therefore we have to detect and reject mappings
that use bounce buffers.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
Copy link
Contributor Author

6by9 commented Nov 22, 2023

Fixes #5727

@pelwell
Copy link
Contributor

pelwell commented Nov 22, 2023

This is going to reject buffers that end up in swiotlb pages. How does this rejection manifest itself? Does it not cause another problem?

A very brief skim of some documentation on dmabufs (https://docs.kernel.org/driver-api/dma-buf.html) made me think that the DMA synchronisation is done at the request of the application, using the DMA_BUF_IOCTL_SYNC ioctl.

@6by9
Copy link
Contributor Author

6by9 commented Nov 23, 2023

This rejection will cause the userspace ìoctl(DRM_IOCTL_PRIME_FD_TO_HANDLE) to fail, which is permitted.
If the buffer isn't contiguous then you'll get the same error - https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/gpu/drm/drm_gem_dma_helper.c#L469-L470

Yes the DMABUF API covers syncing to the main dmabuf allocation, but that doesn't cover syncing to the bounce buffer which only happens when the DMA consumer (ie HVS) says it wants to sync. DRM totally ignores doing that, therefore the bounce buffer never gets copied into.
V3D having an iommu and able to address all of memory will import this buffer happily, and the DMABUF API will ensure that the data that it sees is coherent whenever the CPU has modified it.

This is effectively the same issue that the firmware used to have to handle with the transposer and display_rotate for 90 and 270 deg. The system sees the framebuffer and thinks it is on the screen, but something has to be kicked to generate the rotated (or bounced) buffer every frame. That existed in the firmware, but doesn't in DRM.

@pelwell
Copy link
Contributor

pelwell commented Nov 23, 2023

I'd like to discuss this more - in person is easier - but I'll merge the PR as at least an improvement.

@pelwell pelwell merged commit a984fda into raspberrypi:rpi-6.1.y Nov 23, 2023
12 checks passed
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Nov 23, 2023
…OGUE_GAIN

See: raspberrypi/linux#5711

kernel: DRM fixes
See: raspberrypi/linux#5716

kernel: drm: vc4: Free the dlist alloc immediately if it never hit the hw
See: raspberrypi/linux#5733

kernel: Include I2C bus details in ft5x06 and goodix touch driver names
See: raspberrypi/linux#5703

kernel: drm: vc4: Block swiotlb bounce buffers being imported as dmabuf
See: raspberrypi/linux#5734
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Nov 23, 2023
…OGUE_GAIN

See: raspberrypi/linux#5711

kernel: DRM fixes
See: raspberrypi/linux#5716

kernel: drm: vc4: Free the dlist alloc immediately if it never hit the hw
See: raspberrypi/linux#5733

kernel: Include I2C bus details in ft5x06 and goodix touch driver names
See: raspberrypi/linux#5703

kernel: drm: vc4: Block swiotlb bounce buffers being imported as dmabuf
See: raspberrypi/linux#5734
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants