Skip to content
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

target/riscv: restrict BSCAN-related commands to before-init #1115

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

en-sc
Copy link
Collaborator

@en-sc en-sc commented Aug 14, 2024

Logically, BSCAN tunneling is used to establish a connection, therefore it should be set up before the communication starts (i.e. before init).

Moreover, current implementation does not support changing bscan_tunnel_ir_width after init. This is evident by RISC-V handler of the init itself.
Link:

if (bscan_tunnel_ir_width != 0) {
uint32_t ir_user4_raw = bscan_tunnel_ir_id;
/* Provide a default value which target some Xilinx FPGA USER4 IR */
if (ir_user4_raw == 0) {
assert(target->tap->ir_length >= 6);
ir_user4_raw = 0x23 << (target->tap->ir_length - 6);
}
h_u32_to_le(ir_user4, ir_user4_raw);
select_user4.num_bits = target->tap->ir_length;
bscan_tunneled_ir_width[0] = bscan_tunnel_ir_width;
if (bscan_tunnel_type == BSCAN_TUNNEL_DATA_REGISTER)
bscan_tunnel_data_register_select_dmi[1].num_bits = bscan_tunnel_ir_width;
else /* BSCAN_TUNNEL_NESTED_TAP */
bscan_tunnel_nested_tap_select_dmi[2].num_bits = bscan_tunnel_ir_width;
}

Change-Id: I817c6a996f7f7171b2286e181daf1092bd358f69

Logically, BSCAN tunneling is used to establish a connection, therefore
it should be set up before the communication starts (i.e. before
`init`).

Moreover, current implementation does not support changing
`bscan_tunnel_ir_width` after `init`. This is evident by RISC-V handler
of the `init` itself.
Link: https://github.com/riscv-collab/riscv-openocd/blob/9a23c9e67978f77d9166102cefc7b537b714b561/src/target/riscv/riscv.c#L467-L481

Change-Id: I817c6a996f7f7171b2286e181daf1092bd358f69
Signed-off-by: Evgeniy Naydanov <[email protected]>
@en-sc en-sc self-assigned this Aug 14, 2024
@en-sc
Copy link
Collaborator Author

en-sc commented Aug 14, 2024

@Dolu1990, @GregSavin, @command-paul could you please take a look if time permits?

@GregSavin
Copy link
Collaborator

The change from COMMAND_ANY to COMMAND_CONFIG looks good and correct to me.

@JanMatCodasip
Copy link
Collaborator

JanMatCodasip commented Aug 15, 2024

@en-sc The code change looks all right to me, thank you.

Broader question to all participants in this thread (@Dolu1990, @GregSavin, @command-paul) -

I am concerned about maintenance of this the proprietary BSCAN tunnel functionality in OpenOCD as we do not have a way to test it. If you have any suggestions in this area, that would be most welcome.

@GregSavin
Copy link
Collaborator

Regarding testing, that is a fair point, and actually there are now 2 slightly different proprietary variants covered by that tunnel feature (nested TAP and tunneled DR). Presumably testable on their own respective FPGA-based environments but not on Spike. Modifications to Spike might be possible, as build-time and run-time options, but I don't know if these tunneling schemes are currently codified in RISC-V standards or drafts, so that might be a barrier for getting that approach accepted into the main Spike branch. (I used to work for one of the companies that implemented one of the BSCAN tunnel schemes on some of their FPGA images for RISC-V based systems, but now I work for a different company).

@Dolu1990
Copy link
Contributor

Hi,

So i would say, we don't need any hardware to test it. Simulation can be good enough.
In my case, i use some Scala + SpinalHDL+Verilator based simulation testbench which let me connect real openocd to the simulated system through a jtag over TCP connection.
It isn't "fast", but fast enough to connect in 3 seconds and load a binary at a rate of about 1KB/s.
The installation requirements are : Java + Verilator + SBT. Could even remove the SBT requirements by releasing a JAR file.

Let me know if interrested, i can setup something.
(based on https://github.com/SpinalHDL/VexiiRiscv/blob/dev/src/main/tcl/openocd/vexiiriscv_sim_tunneled.tcl)

@JanMatCodasip
Copy link
Collaborator

Hello @GregSavin, @Dolu1990,

Thank you for replying (and sorry for my late reaction due to being OoO).

If the preference would be to go with Spike (as @GregSavin suggests), maybe a thin wrapper over Spike could be written to simulate both the BSCAN tunnel modes. Perhaps something along these lines. This way Spike could remain unmodified.

The solution from @Dolu1990 to have a simulated HW with the BSCAN tunnel also sounds fine to me.

@Dolu1990
Copy link
Contributor

Dolu1990 commented Sep 3, 2024

I think the best is probably to try the spike solution first, as it would be more compliant / much faster to simulate.

@en-sc en-sc merged commit 909bbb8 into riscv-collab:riscv Sep 4, 2024
4 checks passed
@en-sc en-sc deleted the en-sc/fixup-bscan branch September 4, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants