-
Notifications
You must be signed in to change notification settings - Fork 566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature request: add RISC-V support #3544
Comments
Hi, we don't currently have plans for RISC-V support. We are a small developer team but welcome and will support anyone or any group who want to add features such as RISC-V. Do you have specific use-cases in mind, i.e. what sort of problems would you hope to address with DynamoRIO if RISC-V was available? |
As an aside, I would suggest posting your question on the users list https://groups.google.com/forum/#!forum/DynamoRIO-User This will be seen by more people than this issues list and we'll get an idea of how many are interested in RISC-V support. |
Hi, Just FYI that the PLCT Lab is planning to port the DynamoRIO to RISC-V (RV64G)[1]. Not started yet, but we are optimistic that we are able to finish the porting project and contributing back to upstream before the end of 2021. |
That's exciting news! I have a few suggestions:
|
Thanks for the suggestions. We will send patch set for early review/comments once we got a basic framework. The process of porting is also open (on github, as a fork of the repo).
Thanks. I'll check it out. I've built some CI infrastructure before, so I am optimistic that automated testing of DynamoRIO might not be hard to me:) |
Hi @lazyparser Can I know your current progress? I am currently preparing to implement RISC-V's DBI tool base on dynamorio, I hope to communicate with you more in the future. I am currently a graduate student at iscas. Thanks, |
Hi @onezibo Glad to hear you are working on this. I had not start to coding yet. The plan is to finish the porting/support before the end of year 2021, and get started after April. Feel free to drop me an email. :-) |
Somewhat echoing the earlier suggestions, I would suggest not having a forked repo being the official port: it will diverge and we'll end up with a fractured state where RISC-V is only available with a stale and outdated rest of the system. A forked repo results in a tendency to commit only to that repo, to build up large differences which are impractical to review and merge, and combined with ongoing evolution of the other repo eventually results in permanent divergence. IMHO it is better to build new features directly in the community's existing repo from the beginning; it is always more work to try to merge later. |
@derekbruening I totally agree with you. The fork under the github/plctlab account is only used for sending pull requests to upstream. My long term goal is to make the RISC-V as a tier-1 platform support in upstream. |
+1, SGTM! |
Hi, I started porting DynamoRIO to RISC-V and it's just starting. At present, I directly use the relevant code of AArch64 to make a general framework, and then I will solve the ISA-related problems first, and make the overall RISC-V framework clearer and easier for more developers to participate. All work will be displayed under the default branch of this repository: https://github.com/bekcpear/dynamorio I will synchronize the follow-up progress here in time, and I also very much hope that you can give me some suggestions on porting or codes. |
Hi everyone. So far I have a codebase that can be fully compiled and linked (both cross and native) which contains mostly non-functional stubs and some trivial bits (some possibly wrong). The biggest missing piece is the instruction encoder/decoder, which is next on my TODO list. The code can be found in At this point I would appreciate any feedback on the porting approach I've taken (which is also heavily based on the Aarch64) port. I think we shuold start discussing on a possible changes to architecture specific code separation as the current Additionally there are two commits where I've tried to separate specific questions that I have:
|
Please see our suggestions and requests above, especially #3544 (comment) and #3544 (comment): we do not recommend separate forks. Building up code in separate repositories makes it much harder to merge later. It happens all too often that it is so much work to merge a huge amount of code that was developed separately that it never happens. I am afraid we will end up with multiple conflicting ports that will all drift from the main repository and never be merged...it is much less work in the long run to work in the main repository from the start and most importantly send small incremental code reviews early and often. |
+1 to Derek's suggestion. Additionally, forked repositories do not automatically get the bug fixes and features that we regularly add to the main repository, which accumulate over time. It is also easier to report/debug issues and get feedback on new code if it is merged with the main repository. Echoing #3544 (comment), new developers can be provided more privileges when they are up to speed, so that they can help perform code reviews for their groups. We also have a Triager rotation in place to provide better support to our community, for code reviews and dynamorio-users list discussions. |
Hi, all. I'm glad to receive your attention. I really want to merge RISC-V support into this repository, not separate development in my forked repo (the task of my fork is to make PR). I'm still familiar with the overall structure, and I will submit an appropriate PR as soon as possible. I am also very much looking forward to collaborating with you ;) @semihalf-kardach-stanislaw. If you get some work done, hopefully it can be updated instantly (new commits on your fork or make a new PR to this repo) to avoid duplication, so do I. |
I think there I've caused some confusion by being brief. The reason I've posted my code is to start the integration process as soon as possible (hence the fork as a pull request source). I haven't posted anything earlier because achieving full compilation would help finding places that need to be stubbed/implemented and at the same time it would provide some semblance of validation (even though not a runtime one) that what I'm doing makes sense. @derekbruening Do you consider such pull request too big and therefore I should split it? Second reason is that I want to keep my development process open and visible, especially in-between pull requests. This is so that I can discuss with You whether my understanding of a given module is proper or not, all without spamming RFC pull requests. In essence I want to shorten the feedback loop and allow a broader community to chime in. So my current plan is the following:
Please let me know if that makes sense to everyone. |
I'd like to contribute here: either Cloud VM (amd64, hosting QEMU/RISC-V) or physical Unmatched are available, provided by the PLCT Lab. Collaborating with RISC-V International, the PLCT Lab, ISCAS has built a large RISC-V Lab for open source communities. Feel free to send a PR to https://github.com/plctlab/riscv-lab-access , appending your github-id to the developers.list (the system will grab your pubkeys from github). |
This does seem reasonable, getting far enough to know the approach is going to work and to find the scope of what needs to be done.
It is large. We normally suggest <1500 lines (https://dynamorio.org/page_code_reviews.html#autotoc_md110). Looks like: master...Semihalf:dynamorio:experimental-riscv If most changes are simple a few thousand might be ok -- but smaller pieces are always easier to find time to review. I would suggest breaking it into at least 2 pieces, even if there's no clear logical split, just to have manageable sizes for people to look at in one sitting. |
Thank you for the offer! For connecting to Github, Jenkins may be a good fit: we're currently using that to run the test suite on an AArch64 machine for pull requests. |
Amazing news! I have started a similar process to add support for RISC-V for DynamoRIO, but it makes a lot more sense to join forces and not duplicate effort! I am excited that more people are looking into this, and look forward to participate in the effort! On my side, I have access to some RISC-V boards, which unfortunately can't make accessible remotely. But I can certainly run and test things. |
When code uses register names or offsets directly it is hard to follow whether a given register has a special purpose or any scratch register can be used. It also results in IF_<arch>_ELSE ladders which become less readable with each new architecture added. This commit proposes potential solution to this: choose semantic register names and shows several use-cases: 1. core/translate.c: Unit tests for IBL used DR_REG_XCX/R2 directly where TBL_TARGET_SLOT and IBL_TARGET_REG are defined. In such scenarios - replace direct register name with the semantic one to avoid confusion. 2. core/ir/decodelib.c and core/arch/arch.h: Two places utilizing same register fields and requiring synchronization. Instead define semantic name and use it in both places. 3. core/arch/arch.h: According to documentation dr_insert_call() register r11 is clobbered. However this works only because X86 and ARM both have a register named r11. New architectures might not, i.e. RISC-V names registers xN. Instead add a CALL_SCRATCH_REG definition inside arch.h and use this instead. Similarly it seems that if documentation would use semantic names, it would be easier to produce a single table appendix with mapping to concrete registers. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
Add definitions and structures describing basic architecture specifics such as registers, scratch registers, SIMD data, TLS, stub sizes, syscall/signal ABI, ELF section types and calling conventions. This will serve as a basis for later commits which will enable DynamoRIO compilation for a RISC-V architecture. NOTE: This code is not validated and may contain stubs as the main point was to achieve compilation and estimate the effort required for the port. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
Add minimal opcode, operand and instruction generation macro definitions to enable the rest of the code to compile. NOTE: This code is not validated and mostly contains stubs as the main point was to achieve compilation and estimate the effort required for the port. Some of the trivial logic has been implemented though. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
Add definitions required for samples to compile for RISC-V. NOTE: This code is not validated and mostly contains stubs as the main point was to achieve compilation and estimate the effort required for the port. Some of the trivial logic has been implemented though. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
Add definitions required for bundled-in clients to compile for RISC-V. NOTE: This code is not validated and mostly contains stubs as the main point was to achieve compilation and estimate the effort required for the port. Some of the trivial logic has been implemented though. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
Add RISC-V implementation of most atomic functions. Several are left unimplemented (the exchange variants), only containing a non-atomic logic as a reminder of what the given function should do. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
Add definitions required for extensions to compile for RISC-V. NOTE: This code is not validated and mostly contains stubs as the main point was to achieve compilation and estimate the effort required for the port. Some of the trivial logic has been implemented though. Issue: DynamoRIO#3544 Signed-off-by: Stanislaw Kardach <[email protected]>
1. Conditional branch instruction of RISC-V 'C' extension may not reach after adding clean call. So like X86, we add support to detect and convert compressed cbr to longer version. 2. For RISCV64, when a cbr instruction uses the stolen reg, we need to replace it with a scratch reg. Now cbr works for RISCV64. Still does not work for ARM32/ARM64 because of some bugs unrelated to cbr sample. Issue: #3544
This is a follow-up patch of adding RISC-V vector (RVV) extension support to the core, part1 in PR #6810 (f1ce1bc). This patch: 1. fixes several issues in the codec introduced in part1, codec unit tests will be submitted separately in follow-up PRs; 2. rename and reuse SVE vector length getter/setter functions to be more concise on APIs for vector extensions; 3. adds RISC-V vector support to drdisas; 4. support code cache and clean call context switch; For now, we support RISC-V vector lengths up to 256 bits, longer vector lengths will exceed the limit of DynamoRIO stack size and 12-bit signed immediate range. Issue: #3544
1. Enable the document generation for drcov2lcov options as RISC-V binaries now work. 2. Enable the build of the drcov test linux.eintr for RISC-V. This test can be built and runs correctly on the physical machine. However, due to qemu issues, it cannot be added to the RUNS_ON_QEMU list for RISC-V now. Issue: #3544
Substitute value of return address register to enable post wrappers. Partially enables drwrap-test for RISC-V: 1. Add drwrap-test-callconv test to pipeline 2. Partially supported drwrap-drreg-test: check for pre/post wrappers passes, but checks for clean_call and restore registers functionality are failing. Issue: #3544
Similar to the way we mangle exclusive loads using the stolen register, spill out a register and load the guest value of tp. One more argument is added to pick_scratch_reg() for cases where two scratch registers have been occupied. Issue: DynamoRIO#3544
Similar to the way we mangle exclusive loads using the stolen register, spill out a register and load the guest value of tp. One more argument is added to pick_scratch_reg() for cases where two scratch registers have been occupied. Issue: DynamoRIO#3544
Similar to the way we mangle exclusive loads using the stolen register, spill out a register and load the guest value of tp. One more argument is added to pick_scratch_reg() for cases where two scratch registers have been occupied. Tests will be included after all corner cases have been covered. Issue: DynamoRIO#3544
Similar to the way we mangle exclusive loads using the stolen register, spill out a register and load the guest value of tp. One more argument is added to pick_scratch_reg() for cases where two scratch registers have been occupied. Tests will be included after all corner cases have been covered. For now, a hand-crafted assembly sequence like ``` _start: lla a0, slot mv tp, a0 lr.w.aqrl s11, (tp) mv a0, s11 li a7, 93 ecall .align 4 slot: .long 55 ``` could be used as a test by checking return value. Manually checking generated code confirms scratch registers are correctly saved. Issue: #3544
PC-relative addressing is done through auipc on RISC-V, which comes with only one register operand, so it's always safe to use a0 here. This handles code sequence like ``` _start: lla tp, slot lw a0, 0(tp) li a7, 93 ecall slot: .long 0x5a ``` Issue: DynamoRIO#3544
Reuse mangle_stolen_reg_and_tp_reg() to simplify register spilling. This handles code sequence like ``` _start: lla tp, slot lw a0, 0(tp) li a7, 93 ecall slot: .long 0x5a ``` Issue: #3544
Disassemble result of instructions like jal and auipc on riscv64 is location-sensitve, i.e. depends on current PC. We used to omit the check since address of the buffer that we encode instructions to is unknown before running the test. This commit splits IR tests of these instructions out and enables regex match to handle the varying addresses. It's important to keep it small and standalone to workaround the inefficient regex implementation of CMake. Issue: DynamoRIO#3544
Disassemble result of instructions like jal and auipc on riscv64 is PC-sensitve, i.e. depends on the current PC value. We used to omit the check since address of the buffer that we encode instructions to is unknown before running the test. This commit splits IR tests of these instructions out and enables regex match to handle the varying addresses. It's important to keep it small and standalone to workaround the inefficient regex implementation of CMake. Issue: DynamoRIO#3544
Disassemble result of instructions like jal and auipc on riscv64 is PC-sensitve, i.e. depends on the current PC value. We used to omit the check since address of the buffer that we encode instructions to is unknown before running the test. This commit splits IR tests of these instructions out and enables regex match to handle the varying addresses. It's important to keep it small and standalone to workaround the inefficient regex implementation of CMake. Issue: #3544
This patch was authored at the Computer Architecture and VLSI Laboratory, Institute of Computer Science, Foundation of Research and Technology, Hellas. It emits the necessary instructions that handle saving and restoring the vl and vtype during the context switches, when using the vector extension of the RISC-V ISA. This is required, in order to use the vector extension correctly. Also corrected the order of append_restore_xflags and append_restore_simd_reg (in emit_utils_shared.c ) in order to have the restoration happen in the reverse order of the saving. This specific code is not currently used for RISC-V, but it might in the future. Issue: #3544
Port assembly in common/getretaddr.c to riscv64 and implements tailcall_with_retaddr(). Issue: DynamoRIO#3544
Port assembly in common/getretaddr.c to riscv64 and implements tailcall_with_retaddr(). Issue: #3544
Hi , as we all know , Risc-V popularity is rising more in time.
Will Any support be avalible for Risc-V ?
There is already new soc released for risc-v
https://www.sifive.com/boards
The text was updated successfully, but these errors were encountered: