7
31
Fork
You've already forked blackmagic
23

WIP: Feature: WCH RISC-V support #1399

Draft
perigoso wants to merge 27 commits from perigoso/feature/wch-support into main
pull from: perigoso/feature/wch-support
merge into: blackmagic-debug:main
blackmagic-debug:main
blackmagic-debug:fix/pre-bmp-v3-cross-file-cleanup
blackmagic-debug:feature/bmda-remote-comms
blackmagic-debug:ALTracer/feature/aarch64-ident
blackmagic-debug:feature/better-meson-optimisation-handling
blackmagic-debug:feature/am335x-support
blackmagic-debug:feature/esp32-c3-support
blackmagic-debug:feature/cortex-ar-software-breakpoints
blackmagic-debug:feature/unit-testing
blackmagic-debug:feature/windows-usb-serial-interface-naming
blackmagic-debug:fix/bmp-external-spi
blackmagic-debug:ALTracer/feature/bluepillplus-platform
blackmagic-debug:ALTracer/feature/at32f43x-unrdp
blackmagic-debug:feature/const-correctness
blackmagic-debug:ALTracer/feature/fault_handlers
blackmagic-debug:ALTracer/feature/hazard3-ice40-support
blackmagic-debug:fix/ci-cleanup
blackmagic-debug:ALTracer/fix/gdb-10-12-thread
blackmagic-debug:ALTracer/feature/blackpill-f4-adc
blackmagic-debug:ALTracer/fix/cortex-desc-allocfail-report
blackmagic-debug:ALTracer/feature/spi-perf
blackmagic-debug:ALTracer/feature/calibrate_swd
blackmagic-debug:ALTracer/feature/blank-check
blackmagic-debug:feature/avr
blackmagic-debug:v2.0
blackmagic-debug:v1.9
blackmagic-debug:v1.10
blackmagic-debug:v1.8
perigoso commented 2023年02月23日 16:46:34 +01:00 (Migrated from github.com)
Copy link

Detailed description

Support support for WCH RISC-V family of MCUs

WCH uses a custom DTM (Debug Transport Module) for their RISC-V MCUs, RVSWD a 2 wire SWD "like" protocol (not SWD), and a SDI 1 wire protocol

RVSWD is currently not publlicly documented, part of this implementation is the reverse engineering of the protocol, resulting documentation will be hosted here

Sigrok protocol decoder for RVSWD here

SDI documentation has been recently published, but implementation of will probably come as a separate PR
The only SDI MCU at the moment is the CH32V003, all other WCH RISC-V MCUs use RVSWD

As part of the process support for the WCH-Link debug probe by WCH is also implemented, it's a naive implementation based on a rough reverse engineer of the protocol, which too is not publicly documented

This PR might be broken down in the future

As this PR is built on top of feature/esp32-c3-support it might be hard to read, try this diff (feature/esp32-c3-support <- feature/wch-support)

Discord thread

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

This PR is targeted at v2.0 milestone

Depends on #292 and feature/esp32-c3-support

Closes #1576

<!-- Filling this template is mandatory --> ## Detailed description Support support for WCH RISC-V family of MCUs WCH uses a custom DTM (Debug Transport Module) for their RISC-V MCUs, RVSWD a 2 wire SWD "like" protocol (**not** SWD), and a SDI 1 wire protocol RVSWD is currently not publlicly documented, part of this implementation is the reverse engineering of the protocol, resulting documentation will be hosted [here](https://perigoso.github.io/rins/) Sigrok protocol decoder for RVSWD [here](https://github.com/perigoso/sigrok-rvswd) SDI documentation has been recently published, but implementation of will probably come as a separate PR The only SDI MCU at the moment is the CH32V003, all other WCH RISC-V MCUs use RVSWD As part of the process support for the WCH-Link debug probe by WCH is also implemented, it's a naive implementation based on a rough reverse engineer of the protocol, which too is not publicly documented This PR might be broken down in the future As this PR is built on top of feature/esp32-c3-support it might be hard to read, try [this diff (feature/esp32-c3-support <- feature/wch-support)](https://github.com/blackmagic-debug/blackmagic/compare/feature/esp32-c3-support...perigoso:feature/wch-support) [Discord thread](https://discord.com/channels/613131135903596547/1078343582689263646) <!-- Explain the **details** for making this change. * Is a new feature implemented? * What existing problem(s) does the pull request solve? * How does the pull request solve these problems? Please provide enough information so that others can review your pull request. Information embedded in the description part of the commits doesn't count. --> ## Your checklist for this pull request * [x] I've read the [Code of Conduct](https://github.com/blackmagic-debug/blackmagic/blob/main/CODE_OF_CONDUCT.md) * [x] I've read the [guidelines for contributing](https://github.com/blackmagic-debug/blackmagic/blob/main/CONTRIBUTING.md) to this repository * [x] It builds for hardware native (`make PROBE_HOST=native`) * [x] It builds as BMDA (`make PROBE_HOST=hosted`) * [x] I've tested it to the best of my ability * [x] My commit messages provide a useful short description of what the commits do ## Closing issues This PR is targeted at v2.0 milestone Depends on #292 and [feature/esp32-c3-support](https://github.com/blackmagic-debug/blackmagic/compare/main...feature/esp32-c3-support) <!-- put "fixes #XXXX" here to auto-close the issue(s) that your PR fixes (if any). --> Closes #1576

Please rebase this on feature/esp32-c3-support when you get a chance.

Please rebase this on feature/esp32-c3-support when you get a chance.
koendv commented 2023年12月13日 06:57:58 +01:00 (Migrated from github.com)
Copy link

This is a PR I would not mind testing, But it needs rebasing.

This is a PR I would not mind testing, But it needs rebasing.
perigoso commented 2023年12月13日 15:41:57 +01:00 (Migrated from github.com)
Copy link

This is a PR I would not mind testing, But it needs rebasing.

You can test as is, it should compile and work (and would be welcome!), that said it does need rebasing, but I'm tight on free time for a bit. Maybe at the begining of the new year.

> This is a PR I would not mind testing, But it needs rebasing. You can test as is, it should compile and work (and would be welcome!), that said it does need rebasing, but I'm tight on free time for a bit. Maybe at the begining of the new year.
perigoso commented 2023年12月15日 19:22:19 +01:00 (Migrated from github.com)
Copy link

@koendv Got some time and rebased this on main. Unfortunately the last commit is extremely experimental and not in a state for a clean rebase, so I just reverted the files to the state on the original PR, meaning it reverts some probably important changes on the current main, beware! It should work for scan of CH32 Risc-V chips with the WCH-LinkE, I just tested.

@koendv Got some time and rebased this on main. Unfortunately the last commit is extremely experimental and not in a state for a clean rebase, so I just reverted the files to the state on the original PR, meaning it reverts some probably important changes on the current main, beware! It should work for scan of CH32 Risc-V chips with the WCH-LinkE, I just tested.
koendv commented 2023年12月15日 20:54:16 +01:00 (Migrated from github.com)
Copy link

On 2023年12月15日 10:22:30 -0800
Rafael Silva @.***> wrote:

@koendv Got some time and rebased this on main. Unfortunately the
last commit is extremely experimental and not in a state for a clean
rebase, so I just reverted the files to the state on the original PR,
meaning it reverts some probably important changes on the current
main, beware! It should work for scan of CH32 Risc-V chips with the
WCH-LinkE, I just tested.

OK. Appreciated.

koen

On 2023年12月15日 10:22:30 -0800 Rafael Silva ***@***.***> wrote: > @koendv Got some time and rebased this on main. Unfortunately the > last commit is extremely experimental and not in a state for a clean > rebase, so I just reverted the files to the state on the original PR, > meaning it reverts some probably important changes on the current > main, beware! It should work for scan of CH32 Risc-V chips with the > WCH-LinkE, I just tested. > OK. Appreciated. koen
dragonmux left a comment
Copy link

Apparently we started a review and then failed to ever actually post it, so here we are - it's incomplete, but a start.

Apparently we started a review and then failed to ever actually post it, so here we are - it's incomplete, but a start.

This should probably be defined contingent on ENABLE_RISCV and we'd suggest that, at least for now, this be limited to BMDA. Iff we can figure out how to implement the protocol in the firmware we can then de-restrict it by removing the suggested check for PC_HOSTED.

This should probably be defined contingent on `ENABLE_RISCV` and we'd suggest that, at least for now, this be limited to BMDA. Iff we can figure out how to implement the protocol in the firmware we can then de-restrict it by removing the suggested check for `PC_HOSTED`.
@ -0,0 +1,387 @@
/*

Given this could feasibly be extended if we can figure out any more of the error codes, perhaps use an if-based structure such as this?:

	if (command == WCH_CMD_CONTROL && subcommand == WCH_CONTROL_SUBCMD_ATTACH && error == WCH_ERR_ATTACH)
		return "Failed to attach to target";
	return "Unknown";
Given this could feasibly be extended if we can figure out any more of the error codes, perhaps use an `if`-based structure such as this?: ```suggestion if (command == WCH_CMD_CONTROL && subcommand == WCH_CONTROL_SUBCMD_ATTACH && error == WCH_ERR_ATTACH) return "Failed to attach to target"; return "Unknown"; ```

"Copy in the" on the front?

"Copy in the" on the front?

Might be worth noting the restrictions the protocol has on the address? eg:

	/* Construct the payload. The DMI register address must be a 7 or 8-bit address */
Might be worth noting the restrictions the protocol has on the address? eg: ```suggestion /* Construct the payload. The DMI register address must be a 7 or 8-bit address */ ```

Is this not missing a break; after this assignment?

Is this not missing a `break;` after this assignment?

Would be good to = NULL this to prevent any possible read-before-init issues

Would be good to `= NULL` this to prevent any possible read-before-init issues
perigoso (Migrated from github.com) reviewed 2024年01月17日 16:08:57 +01:00
perigoso (Migrated from github.com) commented 2024年01月17日 16:08:57 +01:00
Copy link

That makes sense, I will consider how to gate this when it's more complete, there will still be a native implementation

That makes sense, I will consider how to gate this when it's more complete, there will still be a native implementation
perigoso (Migrated from github.com) reviewed 2024年01月17日 16:11:13 +01:00
@ -0,0 +1,387 @@
/*
perigoso (Migrated from github.com) commented 2024年01月17日 16:11:13 +01:00
Copy link

I believe this was copied from JLink, which also looks like it's missing a break, I refactored it here, but it's pending on testing still.

I believe this was copied from JLink, which also looks like it's missing a break, I refactored it here, but it's pending on testing still.
mean00 commented 2024年08月09日 07:23:13 +02:00 (Migrated from github.com)
Copy link

In case it is helpful to make things move forward , please find attached
a partially reverse engineered RVSWD/DM protocol document (jpeg image). it is incomplete but enough to make it work.

( as an example, there is a driver written in basic c++ here : https://github.com/mean00/swindle/blob/main/swindle/src/bmp_rvTap.cpp , it could be used as a starting point to write a native BMP driver. It is full of #define macros due to popular demand. It works with CH32V2 and CH32V3)

rvswd

Thank you.

In case it is helpful to make things move forward , please find attached a partially reverse engineered RVSWD/DM protocol document (jpeg image). it is incomplete but enough to make it work. ( as an example, there is a driver written in basic c++ here : https://github.com/mean00/swindle/blob/main/swindle/src/bmp_rvTap.cpp , it could be used as a starting point to write a native BMP driver. It is full of #define macros due to popular demand. It works with CH32V2 and CH32V3) ![rvswd](https://github.com/user-attachments/assets/a724b9b2-bd7b-412b-bfdc-c55ae3fee729) Thank you.
perigoso commented 2024年11月07日 16:37:49 +01:00 (Migrated from github.com)
Copy link

Thank you very much @mean00, I'm planing on picking this up again soon, but it might take some time to get back up to speed

Thank you very much @mean00, I'm planing on picking this up again soon, but it might take some time to get back up to speed
This pull request has changes conflicting with the target branch.
  • src/command.c
  • src/gdb_packet.c
  • src/include/target.h
  • src/platforms/hosted/platform.c
  • src/platforms/hosted/platform.h
  • src/platforms/hosted/wchlink.c
  • src/platforms/hosted/wchlink.h
  • src/platforms/hosted/wchlink_protocol.h
  • src/platforms/hosted/wchlink_riscv_dtm.c
  • src/platforms/hosted/wchlink_rvswd.c
  • src/target/ch32vx.c
  • src/target/meson.build
  • src/target/riscv32.c
  • src/target/riscv_debug.c
  • src/target/riscv_debug.h
  • src/target/riscv_jtag_dtm.c
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin perigoso/feature/wch-support:perigoso/feature/wch-support
git switch perigoso/feature/wch-support
Sign in to join this conversation.
No reviewers
Labels
Clear labels
BMD App
Black Magic Debug App (aka. PC hosted) (not firmware)
BMP Firmware
Black Magic Probe Firmware (not PC hosted software)
Bug
Confirmed bug
Build system
Build system
Can't reproduce
Maintainers can't reproduce this problem
CI
Continuous Integration System
Contribution wanted
User contributions welcome
Documentation
Project documentation
Draft
Work in progress draft
Duplicate
This issue or pull request already exists
Enhancement
General project improvement
Feedback wanted
Requires additional submitter feedback
Foreign Host Board
Non Native hardware to runing Black Magic firmware on
GDB
Issue/PR related to GDB
Good first issue
Good for newcommers
HwIssue Mitigation
Solving or mitigating a Hardware issue in Software
Information Needed
Maintainers need more information
NativeHardware
Official Black Magic Debug Hardware
New Host Board
New hardware to run Black Magic firmware on
New Target
New debug target
Off Topic
Something that does not involve the project in any way
Potential Bug
A potential, unconfirmed or very special circumstance bug
Regression
Bug caused by a regression
User Interest Needed
More user interest required before consideration
User Testing Needed
Looking for user testing reports
Won't fix
Outside of the project scope or works as intended
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
blackmagic-debug/blackmagic!1399
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "perigoso/feature/wch-support"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?