revise API to patch address directly #8272
Conversation
Signed-off-by: Max Zhen <[email protected]>
Signed-off-by: Max Zhen <[email protected]>
| @@ -1285,9 +1268,35 @@ patch(const xrt::module& module, const std::string& argnm, size_t index, const x | |||
| } | |||
|
|
|||
| void | |||
| patch(const xrt::module& module, const std::string& argnm, size_t index, uint64_t address) | |||
| patch(const xrt::module& module, uint8_t *buf, size_t *sz, const std::vector< std::pair<std::string, uint64_t> > *args) | |||
There was a problem hiding this comment.
| patch(const xrt::module& module, uint8_t *buf, size_t *sz, const std::vector< std::pair<std::string, uint64_t> > *args) | |
| patch(const xrt::module& module, uint8_t* buf, size_t* sz, const std::vector<std::pair<std::string, uint64_t>>& args) |
Please document this function. It's not obvious that with buf==nullptr, the function returns the size the caller should allocate for buf. What is args representing?
Commenting should be in header declaration of course.
There was a problem hiding this comment.
The args here representing a list of arguments that needs to be patched to the control code buffer. I have added comments to make it clear that how caller can discover the size for buffer allocation. I still need to keep args as a pointer (v.s. a reference) since when caller tries to discover the real size, it does not pass in a real arg list (nullptr would do).
| module.get_handle()->patch(argnm, index, address); | ||
| auto hdl = module.get_handle(); | ||
| size_t orig_sz = *sz; | ||
| const struct buf *inst = nullptr; |
There was a problem hiding this comment.
| const struct buf *inst = nullptr; | |
| const buf* inst = nullptr; |
| throw std::runtime_error{"Control code buffer passed in is too small"}; // Need a bigger buffer. | ||
| std::memcpy(buf, inst->data(), *sz); | ||
|
|
||
| for (size_t index = 0; index < args->size(); index++) { |
There was a problem hiding this comment.
Use structured binding. Please pick descriptive names of what ever an element in args represents.
| for (size_t index = 0; index < args->size(); index++) { | |
| for (auto& [str, size] : args) { | |
| ... | |
| } |
There was a problem hiding this comment.
I added a loop counter and used more descriptive names in the binding.
Signed-off-by: Max Zhen <[email protected]>
Problem solved by the commit
Revise API introduced in PR#8226 for patching address directly. The original API requires module_sram, which, in turn, requires xrt::hwctx, which is not available for SHIM level test cases. Revising the API to only require module_elf, which is more friendly to SHIM test cases.
What has been tested and how, request additional testing if necessary
Run through a SHIM test case which uses this API and make sure the control buffer is patched properly.