XRT
XRT copied to clipboard
Xilinx
64-bit BO handle in ishim on Linux
Signed-off-by: Max Zhen [email protected]
Problem solved by the commit
Today, a BO in Linux XRT is represented by a handle of xclBufferHandle type, which is essentially a 32-bit unsigned integer. This works fine since, so far, all BOs' book keeping data structure is maintained by xocl/zocl through DRM in Linux kernel. And this 32-bit handle is exactly the DRM BO handle passed up from xocl/zocl through entire XRT user space software stack. It is even exposed to end user via XRT's HAL APIs.
As support for new platforms are added to XRT, new drivers and SHIM implementation are introduced where a BO's book keeping data structure may start having user space part alongside the kernel part maintained by new driver through DRM. However, a 32-bit handle can only help to identify the kernel part, not the user part of the data structure. Hence, it is desired to change the handle to be a 64-bit integer or void *, so that it can hold:
- a 32-bit DRM handle
- an opaque pointer to user space data structure containing the DRM handle alongside other members for user space code
Or, in user space, we can maintain a map from a 32-bit integer to a pointer and keep using the 32-bit handle as-is. But this solution is not ideal because:
- For each BO processing, say exec_buf(), we need to search two mapping tables one in user space and one in kernel via DRM. This is really inefficient.
- It is good to have this flexibility to be able to hide the implementation detail in SHIM from the rest of software stack. Thus, we can keep the upper stack as OS and platform/driver independent.
In Windows XRT, xclBufferHandle has already been defined as void *. However, we can't just simply do the same in Linux since the xclBufferHandle is exposed to end user through HAL APIs, which we can't change.
Considering that HAL APIs are obsolete, we do not have to support them in the SHIM for new platforms. So, as long as we have a unified BO handle representation at ishim level, we are good. The new SHIM implementation will only extend ishim, not adding support for HAL APIs. And ishim is internal to XRT which can be changed at any time.
In this PR, a new BO handle type (xrt_buffer_handle) is introduced replacing xclBufferHandle in ishim API interface. The code using ishim will start using xrt_buffer_handle as BO handle. When the ishim implementation calls into existing SHIM on legacy platforms, xrt_buffer_handle will be explicitly converted to xclBufferHandle. There should be no change to existing SHIM/HAL APIs. When a BO is created via HAL APIs on existing platforms by xocl, its handle will be explicitly converted to xrt_buffer_handle right before it gets passed over to ishim users. So, ishim users should not see xclBufferHandle any more.
New SHIM and driver implementation will extend ishim and can convert xrt_buffer_handle to its own data type as required.
As mentioned above, xrt_buffer_handle can be defined as uint64_t or void *. But, the latter seems better because:
- It is compatible with Windows' implementation
- Compiler cannot perform implicit conversion between xrt_buffer_handle and xclBufferHandle, so I get to examine every place the conversion is needed to make sure I don't miss any.
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
N/A
How problem was solved, alternative solutions (if any) and why they were rejected
See PR description for alternative solutions.
Risks (if any) associated the changes in the commit
Some code calls both ishim and HAL APIs, hopefully the conversion is done properly for them, but there may still be bugs. Some of these code are in current SHIM implementation, so does not apply to new platform anyway. Some of them are in utility code which part also does not apply to new platform. All platform independent code should start using new type, if change is missed, it's a bug.
What has been tested and how, request additional testing if necessary
Built and installed XRT package and ran xbutil examine/validate on U55n. Did full edge build as well.
Documentation impact (if any)
N/A
