riscv: add flashstub framework and helper functions (riscv32 only)

[mean00]

Detailed description

Hi
This is a temptative patch to add some flashstub helper functions for the rv32 targets.
It tries to be very similar to the cortexm version. i.e.
Call riscv32_run_stub( target, loadaddr, param1, param2, param3, param4)
and the flashstub must end up with riscv_stub_exit(errorcode)

The main difference in behavior is the code saves and restore PC & MIE to avoid letting things dangling in ram.
Not completely sure this is needed, just to be on the safe side.

I've tested it with a CH32V307 board using the upcoming board & rvswd protocol support with a ~2x speedup.

As a sidenote and for the record, my original version was providing a temporary stack for the stub to avoid strict compiler rules about not using stack. Dont know if it is a valid idea. Nonetheless, this MR is assuming the stack is not used at all by the stub.

Thank you.

Your checklist for this pull request

• I've read the Code of Conduct
• I've read the guidelines for contributing to this repository
• [Builds till linking then out of flash ] It builds for hardware native (see Building the firmware)
• It builds as BMDA (see Building the BMDA)
• I've tested it to the best of my ability
  Tested with a BMP derivative using CH32V3xx chips. Cant test (yet!) with vanilla bmp
• My commit messages provide a useful short description of what the commits do

[dragonmux]

We'll dive in and review this tomorrow, but as for the stack thing - the stub can use stack, it just has to set one up itself - see the RP2040 stub in #1609 which does this to improve code density and size. It's not really viable for the stub runner to do this itself, and with the lui instruction in RISC-V, it would be trivial to load a 4KiB-aligned stack pointer in the stub.

[mean00]

Hi,
This MR exhibits an issue related to the CSR register management introduced earlier (my bad) because it manipulates some CSR directly.
The CSR registers are offset by RV_CSR_GDB_OFFSET to present them as regular register to gdb
It means that also when used internally, something like
t->reg_write(t, RV_CSR_MIE, &zero, 4);
should actually be
t->reg_write(t, RV_CSR_MIE+RV_CSR_GDB_OFFSET, &zero, 4);

This looks ugly and a bit error prone. Not sure what is the best way to deal with that.

[dragonmux]

Given that going through the target API there is just a convoluted way to access riscv_csr_read/riscv_csr_write - we would suggest that the logic use these functions directly, solving the need to deal with the GDB offset and specify the register width that way (you can use RV_CSR_FORCE_32_BIT with RV_CSR_MIE to ensure the right width access will always happen, or just assume because it's a RISC-V 32 part that it's the right width)

[mean00]

Indeed, thank you for the pointer, that's way better.

[mean00]

Updated accordingly.
Thank you.

[dragonmux]

This is looking good, though there are some items to address and talk about before this can be considered ready for merge. Most of the items are about formatting and code style though.

[mean00]

I think i've taken all or about all of the comments into account.

I've left the two helper functions as inline to have a simpler code. I guess they might be helpful later on.
The generated codesize should be the same.

Similarily i've let the error handling a bit detailed as to allow further more detailed use cases. It should not cause binary bloat.

( On a side note, clang-tidy 17 does not like much the .clang-tidy due to varying indentation and after fixing it complains really a lot )

[dragonmux]

This code is looking significantly better - good work! We do have a few small items found in re-reviewing, but with those fixed we'll be very happy to get this merged.

[dragonmux]

The content of this multi-line comment (and the others below) are over-indented after the start of line *'s - we run a single space between the * and the first word in the comment in this context.

[dragonmux]

tun => run? Seems like a typo.

[dragonmux]

These still need suffixing with U to mark them unsigned.

[dragonmux]

The parens here can be dropped - == binds tighter than =, so there is no ambiguity without them on what this means. the 0 should be suffixed with a U to prevent a signed-unsigned conversion during comparison.