-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add Renesas RH850 architecture support #1918
base: dev
Are you sure you want to change the base?
Conversation
Hello, thanks for your contribution! But first of all, may I confirm that the target code under |
The target code under So it has not been taken from upstream qemu and is basically a rework/improvement of a previous implementation made by Marko Klopcic (https://github.com/markok314). |
I checked your slides and really amazing work! I will give a review hopefully before the end of this week. I need to grab some docs about rh850 firstly. Btw, is there any showcase using this port I can play with? |
btw, you will need to fix CI firstly ;) I just reran all failed jobs. |
Yes I saw all that failed jobs, I thought it failed on my forked repo because of a badly configured CI but it seems I was wrong. I will work on it ASAP and update the PR once all checks pass and look for some piece of code I can share to test the implementation. |
It looks like I'm spamming the CI with my modifications :( ... Solved one issue (missing |
No worry. I will suggest you doing this in your fork repo which is easier. From my personal experience, our CI should work anywhere except release stages which require tokens stored in our repo. |
btw, I just found your target branch is not correct. You also need to rebase to dev branch afterward. |
Okay, I managed to fix all the remaining issues and it builds nicely in my forked repository (all checks passed). I've updated this PR to reflect these changes, but still need to rebase to the |
Could you add Rust bindings? |
To be honest, I'm not a git guru and having our forked unicorn repo based on unicorn's master branch rather than dev is a bit puzzling especially to push new commits while testing everything on my side to check everything is fine. If you have any tip or advice to sync the forked repo with the correct branch while keeping this PR OK i'll take it. @bet4it I thought Rust bindings were automatically generated but in fact they don't seem so. Will do it once my working repository is synced with the dev branch. |
- Fixed some bugs in unicorn building system - Added RH850 unit test file - Modified instruction decoding routine to handle emulation end address
… of R3 instead of R2.
Looks like you rebase your work against the master branch? This is causing chaos in the commit history. Regarding the git problem, my suggestion is:
Sorry for slow response. |
77a09c0
to
087b430
Compare
Got some help from a colleague to solve this one, hope now the commit history will be clean enough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forget to add mode and arch ID in Rust bindings
Added the missing Rust bindings (arch and mode), but still messed up with my branch commits. |
Hey, first of all let me please thank you for this amazing work! According to the manual: If a specified condition is met, 0 should be written to the destination register. Else, write 1 to the destination register. It seems that when the condition is met, 0 is not written. But instead random data is written to the destination register each time I ran it. Here's some code from the Python binding (maybe that's the problem?) from unicorn import *
from unicorn.rh850_const import *
uc = Uc(UC_ARCH_RH850, 0)
CODE_AREA = 0x000F_0000
CODE_AREA_SIZE = 0x1000
uc.mem_map(CODE_AREA, CODE_AREA_SIZE)
# cmp r1,r2
# setf nz, r28
uc.mem_write(CODE_AREA, b"\xe1\x11\xea\xe7\x00\x00")
print("Testing when R1==R2")
uc.reg_write(UC_RH850_REG_PC, CODE_AREA)
uc.reg_write(UC_RH850_REG_R1, 1)
uc.reg_write(UC_RH850_REG_R2, 1)
uc.reg_write(UC_RH850_REG_R28, 0x12345678)
print("R28: {:08X}".format(uc.reg_read(UC_RH850_REG_R28)))
uc.emu_start(CODE_AREA, CODE_AREA+6)
print("R28: {:08X}".format(uc.reg_read(UC_RH850_REG_R28))) You can see each run yields different values in the destination register R28
However, if the R1 does not match R2, 1 is correctly written to R28. print("Testing when R1!=R2")
uc.reg_write(UC_RH850_REG_PC, CODE_AREA)
uc.reg_write(UC_RH850_REG_R1, 1)
uc.reg_write(UC_RH850_REG_R2, 2)
uc.reg_write(UC_RH850_REG_R28, 0x12345678)
I could not detect anything weird in the case OPC_RH850_SETF_cccc_reg2:{
TCGv operand_local = tcg_temp_local_new_i32(tcg_ctx);
int_cond = extract32(ctx->opcode,0,4);
TCGv condResult = condition_satisfied(tcg_ctx, int_cond);
cont = gen_new_label(tcg_ctx);
tcg_gen_movi_i32(tcg_ctx, operand_local, 0x00000000);
tcg_gen_brcondi_i32(tcg_ctx, TCG_COND_NE, condResult, 0x1, cont);
tcg_gen_movi_i32(tcg_ctx, operand_local, 0x00000001);
gen_set_label(tcg_ctx, cont);
gen_set_gpr(tcg_ctx, rs2, operand_local);
tcg_temp_free(tcg_ctx, condResult);
tcg_temp_free(tcg_ctx, operand_local);
} Please forgive me if this comments sections is not the appropriate place to point this out, I couldn't post a new issue in the original Quarkslab repo. |
I made some experiments with the TCG code corresponding to the SETF instruction, I changed the code a bit to force the destination to be set to 0 when the conditional test fails and it produces the expected behavior. I used the following code: TCGv operand_local = tcg_temp_local_new_i32(tcg_ctx);
int_cond = extract32(ctx->opcode,0,4);
TCGv condResult = condition_satisfied(tcg_ctx, int_cond);
cont = gen_new_label(tcg_ctx);
end = gen_new_label(tcg_ctx);
//tcg_gen_movi_i32(tcg_ctx, operand_local, 0x00000000);
tcg_gen_brcondi_i32(tcg_ctx, TCG_COND_NE, condResult, 0x1, cont); /* Jump to cont if not equal */
tcg_gen_movi_i32(tcg_ctx, operand_local, 0x00000001);
tcg_gen_br(tcg_ctx, end); /* jump to end */
/* cont: */
gen_set_label(tcg_ctx, cont);
tcg_gen_movi_i32(tcg_ctx, operand_local, 0x00000000);
/* end: */
gen_set_label(tcg_ctx, end);
gen_set_gpr(tcg_ctx, rs2, operand_local);
tcg_temp_free(tcg_ctx, condResult);
tcg_temp_free(tcg_ctx, operand_local); My guess is that the IR code setting the |
So, is the pull request merged with the dev branch? |
Unfortunately, I'm having limited capacity recently due to difficulties in real life. I will be back asap. |
Thank you for your work on RH850. |
Hello,
This PR adds support for the Renesas RH850 architecture and has been tested against different firmware files designed for the Renesas F1KM-S4 development board.
This implementation supports all instructions for the V850e3v2 CPU used in the Renesas RH850 architecture.