In amdvgpu_bo_free(), when the reference count drops to 0, vdrm_flush()
is called before removing the bo from the handle_to_vbo hash table.
Since vdrm_flush() is a time-consuming operation and is executed outside
of the handle_to_vbo_mutex lock, another thread calling amdvgpu_bo_import()
can concurrently find this bo in the hash table, increment its refcount,
and attempt to use it. Once vdrm_flush() finishes, amdvgpu_bo_free()
proceeds to remove the bo and call free(), leaving the importing thread
with a dangling pointer, which leads to a use-after-free or double free
crash.
To fix this race condition, we must remove the bo from the hash table
under the lock first. After the bo is safely unlinked and the lock is
released, we can then perform the time-consuming vdrm_flush() and the
actual memory release.
Signed-off-by: zhaqian <zhaqian@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/41146>
Reported by clang tools.
See: https://clangd.llvm.org/guides/include-cleaner
struct ac_cmdbuf had to be moved to ac_cmdbuf_base.h because we can't
include ac_cmdbuf.h->sid.h->amdgfxregs.h in radeon_winsys.h for r300.
Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/41091>
I don't know if the old code would have never used UINT32_MAX as a key,
which is going to become invalid for _mesa_hash_table_create_u32_keys.
Signed-off-by: Rhys Perry <pendingchaos02@gmail.com>
Acked-by: Mike Blumenkrantz <michael.blumenkrantz@gmail.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/40196>
Use fd after dup instead of the one before dup to avoid
drm_syncobj_find failed in guest kernel when dev is found in
dev_list.
When dev is not found in dev_list, it uses device fd which is
duplicated, to init sync provider. And when it's found, the same
device fd should be used. Otherwise, it would caused inconsistency
and failures like in the Android domU CTS test where the guest
kernel attempts to locate a syncobj. This occurs because
vdrm_device_connect and VIRTGPU_EXECBUFFER ioctl use fd after dup
while util_sync_provider_drm uses the one before dup.
The fix has been validated with the CtsSdkSandboxWebkitTestCases in
Android domU, and the previously failing test cases no longer occur.
Signed-off-by: Ruitang.Wang@amd.com
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39520>
Unmap bo in destroy_host_blob when hb->cpu_addr is not NULL.
This avoid memory leak caused by bo refcount is not 0 when
amdvgpu_bo_free is called.
Signed-off-by: Julia Zhang <Julia.Zhang@amd.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38440>
Calls to radv_amdgpu_cs_submit will come with either no waits
or through vk_queue.
vk_queue uses VK_SYNC_WAIT_PENDING which is translated as
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE.
WAIT_AVAILABLE will wait for the fence to materialize, so
it means it's safe to pass the syncobj in a
AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT chunk without the
WAIT_FOR_SUBMIT flag.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Acked-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34470>
The only missing bit was supporting the timeline chunk_ids
during submit.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Acked-by: Rob Clark <robdclark@chromium.org>
Acked-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34470>
util_sync_provider provides a wrapper to manipulate syncobjs.
This allows replacing direct ioctl usages with other functions,
and is going to be used to support vpipe.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Acked-by: Rob Clark <robdclark@chromium.org>
Acked-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34470>
Let's use the shared helpers instead of our own version.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Rob Clark <robdclark@chromium.org>
Acked-by: Marek Olšák <marek.olsak@amd.com>
Acked-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34470>
On the host side virglrenderer creates dmabuf on demand when:
* cpu mapping is requested
* setting up scan out
* sharing buffers between guest processes
On-demand dmabuf creation only works if the ctx that created the
BO still exists and knows about this BO. This assumption works ok for
the first 2 cases, but can break with the last one (and it does cause
issues on Android). eg:
* process A allocates BO and exports it as a guest dmabuf
* process A closes its handle to the BO (-> detach_resource)
* process B imports the guest dmabuf -> this triggers the attach_resource
function in virglrenderer. If the given resource isn't a
VIRGL_RESOURCE_FD_DMABUF it'll try to get one... But for this to work,
process A needs to be used -> this fails because this resource was
detached from it.
The reason we create dmabuf on demand is to avoid hitting the number of
open file descriptor limit. So to cover the 3rd case, we'll use the
VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag, but try to limit to as few possible
buffers as possible.
Acked-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21658>
Native context support is implemented by diverting the libdrm_amdgpu functions
into new functions that use virtio-gpu.
VA allocations are done directly in the guest, using newly exposed libdrm_amdgpu
helpers (retrieved through dlopen/dlsym).
Guest <-> Host roundtrips can be expensive so we try to avoid them as much as
possible. When possible we also don't wait for the host reply in case where
it's not needed to get correct result.
Implicit sync works because virtio-gpu commands are submitted in order to the
host (there a single queue per device, shared by all the guest processes).
virtio-gpu also only supports one context per file description (but multiple
file descriptions per process) while amdgpu only allows one fd per process,
but multiple contexts per fd. This causes synchronization problems, because
virtio-gpu drops all sync primitive if they belong to the same fd/context/ring:
ie the amdgpu_ctx can't be expressed in virtio-gpu terms.
For now the solution is to only allocate a single amdgpu_ctx per application.
Contrary to radeonsi/radv, amdgpu_virtio can use libdrm_amdgpu directly: the
ones that don't rely on ioctl() are safe to use here.
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21658>
