Skip to content

RFC: gallium: Add util_copy_constant_buffer_as_resource

Alyssa Rosenzweig requested to merge alyssa/mesa:gallium/user-buffer-fix into main

This helper acts like util_copy_constant_buffer, except it always copies a resource. If a user buffer is passed in, it is uploaded before binding. If drivers use this helper, they do not need to worry about constant buffers. This helper means that the simplest thing (copying with this helper) is also the correct thing (uploading user buffers on bind, rather than holding a dangling pointer).

The question here is ... Is it legal to hold on to the user_buffer pointer? Lots of drivers do so, but it's subtly broken. !21685 was an attempt at making this safe, but it's stalled. llvmpipe seems to think mesa/st is correct and the drivers holding onto the pointer are wrong, with the comment:

   /* user_buffer is only valid until the next set_constant_buffer (at most,
    * possibly until shader deletion), so we need to upload it now to make
    * sure it doesn't get updated/freed out from under us.

I see two options.

Option 1, we decide that mesa/st is correct and it is invalid to hold onto the pointer. In this case, a number of drivers are broken (see the commit log of this MR, I did my best to audit callers). Drivers like iris, radeonsi, and zink are correct under this rule, but drivers like v3d and panfrost are broken. freedreno is in a fun middle ground where a6xx is correct but <=a5xx are broken. asahi is fixed in this MR by switching to the new copying helper, but other drivers seem to be nontrivial to fix due to CPU access to UBO#0.

Option 2, we decide that mesa/st is wrong and it is valid to hold onto the pointer. in this case, !21685 needs to be picked back up.

This addresses ASAN failures.

Edited by Alyssa Rosenzweig

Merge request reports