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

What version of the RISC-V Debug Specification is used? #999

Open
mateoconlechuga opened this issue Jan 17, 2024 · 16 comments
Open

What version of the RISC-V Debug Specification is used? #999

mateoconlechuga opened this issue Jan 17, 2024 · 16 comments

Comments

@mateoconlechuga
Copy link

Hello, I am wondering if there is a way to know either through a tag, command line argument, or documentation which RISC-V debug specification is implemented? Currently it seems like 1.0, but I would like to target hardware which uses 0.13, and I am unsure where/how to get/compile a version of OpenOCD that works with different versions of the specification.

Thanks for the help.

@mateoconlechuga
Copy link
Author

The issue I am trying to deal with is the backwards-incompatible change to allow access to tdata2 while the trigger is disabled, specifically these lines: https://github.com/riscv/riscv-openocd/blob/4fc0d86ff0c5ac45058777c9f0e5c53d3d17d7f9/src/target/riscv/riscv.c#L653-L659

The hardware I am working with does not support this modification.

@TommyMurphyTM1234
Copy link
Collaborator

TommyMurphyTM1234 commented Jan 17, 2024

You don't need to build it for any/each specific debug specification. The one build supports most or all of all relevant debug specs:

@TommyMurphyTM1234
Copy link
Collaborator

TommyMurphyTM1234 commented Jan 17, 2024

The issue I am trying to deal with is the backwards-incompatible change to allow access to tdata2 while the trigger is disabled, specifically these lines:

https://github.com/riscv/riscv-openocd/blob/4fc0d86ff0c5ac45058777c9f0e5c53d3d17d7f9/src/target/riscv/riscv.c#L653-L659

The hardware I am working with does not support this modification.

To add to what I posted above - that does not rule out the possibility of bugs where, for example, something that works in one version of the debug spec may not work in another. I already had one today :-)

If you can explain in more detail what you understand the issue to be here it might help others to figure out if there's a possible explanation or fix.

@TommyMurphyTM1234
Copy link
Collaborator

Also - it might be worth collecting a verbose (openocd -d3) log for the issue that you are experiencing and attaching that to this thread. That may shed more light on the problem.

@TommyMurphyTM1234
Copy link
Collaborator

The issue I am trying to deal with is the backwards-incompatible change to allow access to tdata2 while the trigger is disabled, specifically these lines:

https://github.com/riscv/riscv-openocd/blob/4fc0d86ff0c5ac45058777c9f0e5c53d3d17d7f9/src/target/riscv/riscv.c#L653-L659

The hardware I am working with does not support this modification.

I had a quick skim of the 0.13.2 debug spec and can't see anything that says that a trigger must be enabled for tdata2 to be accessed/written. Where are you seeing this?

@mateoconlechuga
Copy link
Author

mateoconlechuga commented Jan 17, 2024

Thanks for the info! Yes I am seeing that now. Here is the debug log with the write of 0 to tdata1 to disable the trigger, followed by the write to tdata2. Since the trigger is disabled, the hardware does not accept the write, and leads to the failing readback verification of tdata2 later in the function.

Debug: 4974 26611 riscv.c:1105 maybe_add_trigger_t2_t6_for_bp(): [target.cpu] trying to setup equality match trigger
Debug: 4975 26611 riscv.c:847 try_setup_single_match_trigger(): [target.cpu] trying to set up a match trigger
Debug: 4976 26611 riscv.c:747 log_trigger_request_info(): tdata1=280000000000105c, tdata2=fc000000, tdata1_ignore_mask=7e0000000000000
Debug: 4977 26611 riscv.c:615 find_next_free_trigger(): [target.cpu] 1 trigger(s) of type 2 found on index 0, chained == false
Debug: 4978 26611 riscv.c:5186 riscv_set_or_write_register(): [target.cpu] Writing to target: tselect <- 0x0 (cacheable=false, valid=false, dirty=false)
Debug: 4979 26611 riscv-013.c:4870 riscv013_set_register(): [target.cpu] writing 0x0 to register tselect
Debug: 4980 26611 riscv-013.c:1680 register_write_direct(): [target.cpu] Writing 0x0 to tselect
Debug: 4981 26611 riscv-013.c:393 dump_field(): 41b w 00000000 @05 -> + 00000000 @04; 0i
Debug: 4982 26611 riscv-013.c:401 dump_field(): write: 
Debug: 4983 26611 riscv-013.c:393 dump_field(): 41b - 00000000 @05 -> + 00000000 @05; 0i
Debug: 4984 26611 riscv-013.c:393 dump_field(): 41b w 00000000 @04 -> + 00000000 @05; 0i
Debug: 4985 26611 riscv-013.c:401 dump_field(): write: 
Debug: 4986 26611 riscv-013.c:393 dump_field(): 41b - 00000000 @04 -> + 00000000 @04; 0i
Debug: 4987 26611 riscv-013.c:882 execute_abstract_command(): [target.cpu] access register=0x3307a0 {regno=0x7a0 write=register transfer=enabled postexec=disabled aarpostincrement=disabled aarsize=64bit}
Debug: 4988 26611 riscv-013.c:393 dump_field(): 41b w 003307a0 @17 -> + 00000000 @04; 0i
Debug: 4989 26611 riscv-013.c:401 dump_field(): write: command=0x3307a0 {control=0x3307a0}
Debug: 4990 26611 riscv-013.c:393 dump_field(): 41b r 00000000 @16 -> + 00000000 @17; 0i
Debug: 4991 26612 riscv-013.c:393 dump_field(): 41b - 00000000 @16 -> + 08000004 @16; 0i
Debug: 4992 26612 riscv-013.c:407 dump_field(): read: abstractcs=0x8000004 {datacount=4 progbufsize=8}
Debug: 4993 26612 riscv-013.c:1699 register_write_direct(): [target.cpu] tselect <- 0x0
Debug: 4994 26612 riscv.c:5202 riscv_set_or_write_register(): [target.cpu] Wrote 0x0 to tselect (cacheable=false, valid=false, dirty=false)
Debug: 4995 26612 riscv.c:5186 riscv_set_or_write_register(): [target.cpu] Writing to target: tdata1 <- 0x0 (cacheable=false, valid=false, dirty=false)
Debug: 4996 26612 riscv-013.c:4870 riscv013_set_register(): [target.cpu] writing 0x0 to register tdata1
Debug: 4997 26612 riscv-013.c:1680 register_write_direct(): [target.cpu] Writing 0x0 to tdata1
Debug: 4998 26612 riscv-013.c:393 dump_field(): 41b w 00000000 @05 -> + 00000000 @16; 0i
Debug: 4999 26612 riscv-013.c:401 dump_field(): write: 
Debug: 5000 26612 riscv-013.c:393 dump_field(): 41b - 00000000 @05 -> + 00000000 @05; 0i
Debug: 5001 26612 riscv-013.c:393 dump_field(): 41b w 00000000 @04 -> + 00000000 @05; 0i
Debug: 5002 26612 riscv-013.c:401 dump_field(): write: 
Debug: 5003 26612 riscv-013.c:393 dump_field(): 41b - 00000000 @04 -> + 00000000 @04; 0i
Debug: 5004 26612 riscv-013.c:882 execute_abstract_command(): [target.cpu] access register=0x3307a1 {regno=0x7a1 write=register transfer=enabled postexec=disabled aarpostincrement=disabled aarsize=64bit}
Debug: 5005 26612 riscv-013.c:393 dump_field(): 41b w 003307a1 @17 -> + 00000000 @04; 0i
Debug: 5006 26612 riscv-013.c:401 dump_field(): write: command=0x3307a1 {control=0x3307a1}
Debug: 5007 26612 riscv-013.c:393 dump_field(): 41b r 00000000 @16 -> + 00000000 @17; 0i
Debug: 5008 26613 riscv-013.c:393 dump_field(): 41b - 00000000 @16 -> + 08000004 @16; 0i
Debug: 5009 26613 riscv-013.c:407 dump_field(): read: abstractcs=0x8000004 {datacount=4 progbufsize=8}
Debug: 5010 26613 riscv-013.c:1699 register_write_direct(): [target.cpu] tdata1 <- 0x0
Debug: 5011 26613 riscv.c:5202 riscv_set_or_write_register(): [target.cpu] Wrote 0x0 to tdata1 (cacheable=false, valid=false, dirty=false)
Debug: 5012 26613 riscv.c:5186 riscv_set_or_write_register(): [target.cpu] Writing to target: tdata2 <- 0xfc000000 (cacheable=false, valid=false, dirty=false)
Debug: 5013 26613 riscv-013.c:4870 riscv013_set_register(): [target.cpu] writing 0xfc000000 to register tdata2
Debug: 5014 26613 riscv-013.c:1680 register_write_direct(): [target.cpu] Writing 0xfc000000 to tdata2
Debug: 5015 26613 riscv-013.c:393 dump_field(): 41b w 00000000 @05 -> + 00000000 @16; 0i
Debug: 5016 26613 riscv-013.c:401 dump_field(): write: 
Debug: 5017 26613 riscv-013.c:393 dump_field(): 41b - 00000000 @05 -> + 00000000 @05; 0i
Debug: 5018 26613 riscv-013.c:393 dump_field(): 41b w fc000000 @04 -> + 00000000 @05; 0i
Debug: 5019 26613 riscv-013.c:401 dump_field(): write: 
Debug: 5020 26613 riscv-013.c:393 dump_field(): 41b - 00000000 @04 -> + 00000000 @04; 0i
Debug: 5021 26613 riscv-013.c:882 execute_abstract_command(): [target.cpu] access register=0x3307a2 {regno=0x7a2 write=register transfer=enabled postexec=disabled aarpostincrement=disabled aarsize=64bit}
Debug: 5022 26613 riscv-013.c:393 dump_field(): 41b w 003307a2 @17 -> + 00000000 @04; 0i
Debug: 5023 26613 riscv-013.c:401 dump_field(): write: command=0x3307a2 {control=0x3307a2}
Debug: 5024 26613 riscv-013.c:393 dump_field(): 41b r 00000000 @16 -> + 00000000 @17; 0i
Debug: 5025 26614 riscv-013.c:393 dump_field(): 41b - 00000000 @16 -> + 08000004 @16; 0i
Debug: 5026 26614 riscv-013.c:407 dump_field(): read: abstractcs=0x8000004 {datacount=4 progbufsize=8}
Debug: 5027 26614 riscv-013.c:1699 register_write_direct(): [target.cpu] tdata2 <- 0xfc000000
Debug: 5028 26614 riscv.c:5202 riscv_set_or_write_register(): [target.cpu] Wrote 0xfc000000 to tdata2 (cacheable=false, valid=false, dirty=false)
Debug: 5029 26614 riscv.c:5186 riscv_set_or_write_register(): [target.cpu] Writing to target: tdata1 <- 0x280000000000105c (cacheable=false, valid=false, dirty=false)
Debug: 5030 26614 riscv-013.c:4870 riscv013_set_register(): [target.cpu] writing 0x280000000000105c to register tdata1
Debug: 5031 26614 riscv-013.c:1680 register_write_direct(): [target.cpu] Writing 0x280000000000105c to tdata1
Debug: 5032 26614 riscv-013.c:393 dump_field(): 41b w 28000000 @05 -> + 00000000 @16; 0i
Debug: 5033 26614 riscv-013.c:401 dump_field(): write: 
Debug: 5034 26614 riscv-013.c:393 dump_field(): 41b - 00000000 @05 -> + 00000000 @05; 0i
Debug: 5035 26614 riscv-013.c:393 dump_field(): 41b w 0000105c @04 -> + 00000000 @05; 0i
Debug: 5036 26614 riscv-013.c:401 dump_field(): write: 
Debug: 5037 26614 riscv-013.c:393 dump_field(): 41b - 00000000 @04 -> + 00000000 @04; 0i
Debug: 5038 26614 riscv-013.c:882 execute_abstract_command(): [target.cpu] access register=0x3307a1 {regno=0x7a1 write=register transfer=enabled postexec=disabled aarpostincrement=disabled aarsize=64bit}
Debug: 5039 26614 riscv-013.c:393 dump_field(): 41b w 003307a1 @17 -> + 00000000 @04; 0i
Debug: 5040 26614 riscv-013.c:401 dump_field(): write: command=0x3307a1 {control=0x3307a1}
Debug: 5041 26614 riscv-013.c:393 dump_field(): 41b r 00000000 @16 -> + 00000000 @17; 0i
Debug: 5042 26614 riscv-013.c:393 dump_field(): 41b - 00000000 @16 -> + 08000004 @16; 0i
Debug: 5043 26614 riscv-013.c:407 dump_field(): read: abstractcs=0x8000004 {datacount=4 progbufsize=8}
Debug: 5044 26614 riscv-013.c:1699 register_write_direct(): [target.cpu] tdata1 <- 0x280000000000105c
Debug: 5045 26614 riscv.c:5202 riscv_set_or_write_register(): [target.cpu] Wrote 0x280000000000105c to tdata1 (cacheable=false, valid=false, dirty=false)
Debug: 5046 26614 riscv.c:5278 riscv_get_register(): [target.cpu] Reading tdata1 from target
Debug: 5047 26614 riscv-013.c:4854 riscv013_get_register(): [target.cpu] reading register tdata1
Debug: 5048 26614 riscv-013.c:1709 register_read_direct(): [target.cpu] Reading tdata1
Debug: 5049 26614 riscv-013.c:882 execute_abstract_command(): [target.cpu] access register=0x3207a1 {regno=0x7a1 write=arg0 transfer=enabled postexec=disabled aarpostincrement=disabled aarsize=64bit}
Debug: 5050 26615 riscv-013.c:393 dump_field(): 41b w 003207a1 @17 -> + 00000000 @16; 0i
Debug: 5051 26615 riscv-013.c:401 dump_field(): write: command=0x3207a1 {control=0x3207a1}
Debug: 5052 26615 riscv-013.c:393 dump_field(): 41b r 00000000 @16 -> + 00000000 @17; 0i
Debug: 5053 26615 riscv-013.c:393 dump_field(): 41b - 00000000 @16 -> + 08000004 @16; 0i
Debug: 5054 26615 riscv-013.c:407 dump_field(): read: abstractcs=0x8000004 {datacount=4 progbufsize=8}
Debug: 5055 26615 riscv-013.c:393 dump_field(): 41b r 00000000 @05 -> + 00000000 @16; 0i
Debug: 5056 26615 riscv-013.c:393 dump_field(): 41b - 00000000 @05 -> + 2fe00000 @05; 0i
Debug: 5057 26615 riscv-013.c:407 dump_field(): read: 
Debug: 5058 26615 riscv-013.c:393 dump_field(): 41b r 00000000 @04 -> + 00000000 @05; 0i
Debug: 5059 26615 riscv-013.c:393 dump_field(): 41b - 00000000 @04 -> + 0000105c @04; 0i
Debug: 5060 26615 riscv-013.c:407 dump_field(): read: 
Debug: 5061 26615 riscv-013.c:1728 register_read_direct(): [target.cpu] tdata1 = 0x2fe000000000105c
Debug: 5062 26615 riscv.c:5286 riscv_get_register(): [target.cpu] Read tdata1: 0x2fe000000000105c
Debug: 5063 26615 riscv.c:5278 riscv_get_register(): [target.cpu] Reading tdata2 from target
Debug: 5064 26615 riscv-013.c:4854 riscv013_get_register(): [target.cpu] reading register tdata2
Debug: 5065 26615 riscv-013.c:1709 register_read_direct(): [target.cpu] Reading tdata2
Debug: 5066 26615 riscv-013.c:882 execute_abstract_command(): [target.cpu] access register=0x3207a2 {regno=0x7a2 write=arg0 transfer=enabled postexec=disabled aarpostincrement=disabled aarsize=64bit}
Debug: 5067 26616 riscv-013.c:393 dump_field(): 41b w 003207a2 @17 -> + 00000000 @04; 0i
Debug: 5068 26616 riscv-013.c:401 dump_field(): write: command=0x3207a2 {control=0x3207a2}
Debug: 5069 26616 riscv-013.c:393 dump_field(): 41b r 00000000 @16 -> + 00000000 @17; 0i
Debug: 5070 26616 riscv-013.c:393 dump_field(): 41b - 00000000 @16 -> + 08000004 @16; 0i
Debug: 5071 26616 riscv-013.c:407 dump_field(): read: abstractcs=0x8000004 {datacount=4 progbufsize=8}
Debug: 5072 26616 riscv-013.c:393 dump_field(): 41b r 00000000 @05 -> + 00000000 @16; 0i
Debug: 5073 26616 riscv-013.c:393 dump_field(): 41b - 00000000 @05 -> + 00000000 @05; 0i
Debug: 5074 26616 riscv-013.c:407 dump_field(): read: 
Debug: 5075 26616 riscv-013.c:393 dump_field(): 41b r 00000000 @04 -> + 00000000 @05; 0i
Debug: 5076 26616 riscv-013.c:393 dump_field(): 41b - 00000000 @04 -> + 00000000 @04; 0i
Debug: 5077 26616 riscv-013.c:407 dump_field(): read: 
Debug: 5078 26616 riscv-013.c:1728 register_read_direct(): [target.cpu] tdata2 = 0x0
Debug: 5079 26616 riscv.c:5286 riscv_get_register(): [target.cpu] Read tdata2: 0x0
Debug: 5080 26616 riscv.c:673 set_trigger(): [target.cpu] Trigger 0 doesn't support what we need.
Debug: 5081 26616 riscv.c:681 set_trigger(): [target.cpu] wrote 0xfc000000 to tdata2 but read back 0x0

@aap-sc
Copy link
Collaborator

aap-sc commented Jan 17, 2024

@mateoconlechuga could you double check this issue? #870 Does this have relevance to your case?

@mateoconlechuga
Copy link
Author

@aap-sc Yes that issue is relevant - the hardware I am using is 0.13 specification compliant and doesn't accept writes to tdata2 when the trigger is disabled. The fix in that issue implemented the 1.0 specification which assumes that tdata2 is writable even when the trigger is disabled, which is what I am running into.

@TommyMurphyTM1234
Copy link
Collaborator

@aap-sc Yes that issue is relevant - the hardware I am using is 0.13 specification compliant and doesn't accept writes to tdata2 when the trigger is disabled.

Can you clarify where in the 0.13 spec it states that tdata2 cannot be written unless the trigger is disabled? I can't find it.

@aap-sc
Copy link
Collaborator

aap-sc commented Jan 17, 2024

Regarding the supported version.

@mateoconlechuga , @TommyMurphyTM1234 the current OpenOCD codebase does not make a distinction between 1.0 and 0.13. The intention is to support both. There may be indeed be inconsistencies between 1.0 and 0.13 - it's just that no one encountered such case before (at least there was no pending issue to indicate that).

  1. RISCV spec allows for runtime detection of which spec is used. See DM_DMSTATUS_VERSION_1_0 , DM_DMSTATUS_VERSION_0_13 , DM_DMSTATUS_VERSION_0_11 defines. So in theory one could query this information during examine and adjust the codebase. You may wonder why these defines are not used. This is because there another mechanism to identify if we on 0_11 or 0_13+ (see DTM_DTMCS_VERSION_0_11).
  2. It would be nice to double check if there is really a non-backwards compatible change or is it a bug in HW implementation. Note: the reason for the non-conformant implementation may be a non-backward compatible change to the spec itself, but it does not matter much. I'm unable to express my opinion on the matter today, maybe someone else will (like @TommyMurphyTM1234) will analyse, or I'll take a look tomorrow.
  3. If this is a HW bug , depending of the complexity of a workaround OpenOCD maintainers tend to accept such patches. It's just this needs to be properly documented.
  4. But I should warn that there is a major problem. It's testing. In OS OpenOCD is tested on spike. And spike implementation of debug subsystem focuses mostly on 1.0 spec. It may be difficult to publish relevant changes there (though not impossible). Without a proper test case the code will eventually break even if workaround is implemented.

@mateoconlechuga
Copy link
Author

mateoconlechuga commented Jan 17, 2024

@TommyMurphyTM1234

Under section 5.2 Trigger Registers:

If a debugger writes an unsupported configuration, the register will read back a value that is supported (which may simply be a disabled trigger).

When OpenOCD writes 0 to the tdata1, it effectively disables the trigger in hardware, even though it still exists.
This masks the return of the tdata2 register and causes it to return a "valid" value of 0, even though it was written while the register was in a disabled state.

The 1.0 specification clarifies the intended behavior a lot better in the updated specification section 5.7 Trigger Module Registers, and that tdata2 should be writable in the new section. The "guarantee" that is mentioned does not apply to the hardware this issue references, because the hardware was written before the clarification in the specification.

The combination of these rules means that a debugger cannot simply set a trigger by writing
tdata1, then tdata2, etc. The current value of tdata2 might not be legal with the new value of
tdata1. To help with this situation, it is guaranteed that writing 0 to tdata1 disables the trigger,
and leaves it in a state where tdata2 and tdata3 can be written with any value that makes sense
for any trigger type supported by this trigger.

@TommyMurphyTM1234
Copy link
Collaborator

@TommyMurphyTM1234

Under section 5.2 Trigger Registers:

If a debugger writes an unsupported configuration, the register will read back a value that is supported (which may simply be a disabled trigger).

When OpenOCD writes 0 to the tdata1, it effectively disables the trigger in hardware, even though it still exists. This masks the return of the tdata2 register and causes it to return a "valid" value of 0, even though it was written while the register was in a disabled state.

Yes - but I still don't see how that mandates that accesses/writes to tdata2 cannot be made when the trigger is enabled even if it might make more sense to only do this when the trigger is disabled?

@mateoconlechuga
Copy link
Author

I don't think I read your comment here correctly the first time:

Can you clarify where in the 0.13 spec it states that tdata2 cannot be written unless the trigger is disabled? I can't find it.

The behavior I am experiencing is the other way around - tdata2 cannot be written when the trigger is disabled.


Section 5.7 of the 1.0 specification clarifies this a lot more. This clarification about writing a 0 to disable the register and allow writes to tdata2 is nonexistent in the 0.13 specification, which the hardware was built against.

Almost all trigger functionality is optional. All tdata registers follow write-any-read-legal semantics. If a debugger writes an unsupported configuration, the register will read back a value that is supported (which may simply be a disabled trigger). This means that a debugger must always read back values it writes to tdata registers, unless it already knows already what is supported. Writes to one tdata register must not modify the contents of other tdata registers, nor the configuration of any trigger besides the one that is currently selected.

The combination of these rules means that a debugger cannot simply set a trigger by writing tdata1, then tdata2, etc. The current value of tdata2 might not be legal with the new value of tdata1. To help with this situation, it is guaranteed that writing 0 to tdata1 disables the trigger, and leaves it in a state where tdata2 and tdata3 can be written with any value that makes sense for any trigger type supported by this trigger.

@aap-sc
Copy link
Collaborator

aap-sc commented Jan 17, 2024

@mateoconlechuga a note regarding the spec interpretation. Such questions are better to be raised here (if there is no agreement): https://github.com/riscv/riscv-debug-spec . I believe RISCV debug workgroup still provides support for both versions of the spec.

@mateoconlechuga
Copy link
Author

@aap-sc Thanks! I don't believe that this is a specification interpretation issue though.

This is the backwards-incompatible specification change that is called out in the 1.0.0 specification that OpenOCD currently does not support:

1.2.1.2 Incompatible Changes from 0.13 to 1.0

  1. When a selected trigger is disabled, tdata2 and tdata3 can be written with any value supported by any of the types this trigger supports.

@aap-sc
Copy link
Collaborator

aap-sc commented Jan 18, 2024

@mateoconlechuga

I don't believe that this is a specification interpretation issue though.

Yes, the note you've mentioned looks convincing enough to me. So if you are going to submit a patch - please, consider creating a patch for spike too, so we could test this case.

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

No branches or pull requests

3 participants