Add Renesas RH850 architecture support

[virtualabs]

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.

[wtdcode]

Hello, thanks for your contribution!

But first of all, may I confirm that the target code under target/rh850 is port-ed from elsewhere or written by hand? I can't find it from upstream qemu here: https://github.com/qemu/qemu/tree/master/target

[virtualabs]

The target code under target/rh850 is based on the partial implementation of Marko Klocic's forked qemu repo (https://github.com/markok314/qemu) which had some bugs and missing instructions. We fixed most of the bugs and added support of the missing instructions, and tested it against some RH850 firmware files we had. This code is part of a research we did and presented at the 2023 Qualcomm Product Security Summit (https://qcbizdev.my.salesforce-sites.com/QCTConference/GenericSitePage?eventname=SecuritySummit&page=Summit+Schedule), but it took us some time to polish the code and do some more testing on our target.

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).

[wtdcode]

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?

[wtdcode]

btw, you will need to fix CI firstly ;)

I just reran all failed jobs.

[virtualabs]

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.

[virtualabs]

It looks like I'm spamming the CI with my modifications :( ... Solved one issue (missing target-config.h file), but there is still some errors with MSVC. I should have created another branch to test my modifications, sorry.

[wtdcode]

It looks like I'm spamming the CI with my modifications :( ... Solved one issue (missing target-config.h file), but there is still some errors with MSVC. I should have created another branch to test my modifications, sorry.

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.

[wtdcode]

btw, I just found your target branch is not correct. You also need to rebase to dev branch afterward.

[virtualabs]

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 dev branch in order to get a clean PR.

[bet4it]

Could you add Rust bindings?

[virtualabs]

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.

[virtualabs]

Okay, so I finally managed to rebase my branch to the dev branch ! Unfortunately my code does not work anymore due to some core changes in Unicorn, I'll need some time to fix my code and get everything working.

[wtdcode]

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:

1. Get a clean branch based on the master branch. I found rh850-arch-fix-ci in your fork repo but I'm not sure if it's intended.
2. Merge it with our current dev branch.
3. If there are conflicts, I can have a look.

Sorry for slow response.

[virtualabs]

Got some help from a colleague to solve this one, hope now the commit history will be clean enough.

[virtualabs]

Added the missing Rust bindings (arch and mode), but still messed up with my branch commits.

[shaqed]

Hey, first of all let me please thank you for this amazing work! Not sure if this is the correct place to put this. But I found a weird behaviour with the SETF instruction.

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

$ python rh850_setf_bug.py 
Testing when R1==R2
R28: 12345678
R28: 61736C30
$
$ python rh850_setf_bug.py 
Testing when R1==R2
R28: 12345678
R28: 5A79AC30
$
$ python rh850_setf_bug.py 
Testing when R1==R2
R28: 12345678
R28: BBB9AC30

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)

$ python rh850_setf_bug.py 
Testing when R1!=R2
R28: 12345678
R28: 00000001
$
$ python rh850_setf_bug.py 
Testing when R1!=R2
R28: 12345678
R28: 00000001
$
$ python rh850_setf_bug.py 
Testing when R1!=R2
R28: 12345678
R28: 00000001

I could not detect anything weird in the translate.c code which handles this instruction so maybe you can help me figure out what's wrong:

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.

[virtualabs]

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 operand_local to zero is not executed for some reason (when placed as the first instruction of the IR block) and you end up with a random value in the destination register when the condition is not met. It could be related with the TCG IR optimizations, perhaps.