Skip to content
This repository has been archived by the owner on Dec 24, 2024. It is now read-only.

Fixes to work with latest driver and userspace BO sync #36

Draft
wants to merge 7 commits into
base: iree-aie
Choose a base branch
from

Conversation

eddierichter-amd
Copy link
Collaborator

This PR includes:

  • Modifications to work with the latest xdna driver.
  • Remove the get_handle_from_vaddr() sideband and instead only pass addresses from the application.
  • Sync BOs in userspace rather than making ioctls to the xdna driver.

* Instead of using handles in the packet and in the configure hardware context sideband, we are instead passing virtual addresses. This removes some sidebands to get the handle from a virtual address and simplfies the application. The ROCr runtime has access to both so can always do the translation.
* This is modifying the ioctl interface to xdna to work with the latest driver, which includes changing the heap size, passing the size of a buffer into the sync, and some small misc changes.

This commit leaves one TODO which is having the user specify the sizes of operands so we know the size. This is an intermediate step to sync the BOs in userspace to avoid the syscall.
uint64_t base_addr = reinterpret_cast<uint64_t>(base) + offset;
for (int i = 0; i < len / cacheline_size; i++) {
uint64_t cur_addr = base_addr + cacheline_size * i;
_mm_clflush(reinterpret_cast<void *>(cur_addr));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This failed for me w/ gcc13 with _mm_clflush undeclared. Is it in x86intrin.h?

Copy link

@fifield fifield Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This failed for me w/ gcc13 with _mm_clflush undeclared. Is it in x86intrin.h?

It built fine when I added my own declaration.
The correct fix on my system was to move #include <x86intrin.h> outside of the namespace rocr block in runtime/hsa-runtime/core/util/utils.h

__u64 vaddr;
__u64 size;
__u32 type;
Copy link

@fifield fifield Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This struct field ordering change breaks other code:

/work/acdc/ROCR-Runtime/runtime/hsa-runtime/core/driver/xdna/amd_xdna_driver.cpp:300:61: error: designator order for field ‘amdxdna_drm_create_bo::vaddr’ does not match declaration order in ‘amdxdna_drm_create_bo’
  300 |                                        .size = dev_heap_size};

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants