Drop the use of -coreid in RISC-V targets

[en-sc]

Currently -coreid is used in RISC-V targets as hartid.

riscv-openocd/src/target/riscv/riscv-013.c

Line 1909 in a4020f1

info->index = target->coreid;

This is misleading.

(hartid name is probably not the best. Here it's used just as an example.)

1. By default hartid can be assigned based on target index See Drop the use of -coreid in RISC-V targets #1147 (comment)
2. RISC-V-specific -hartid configure option can be introduced.
3. Configuring -coreid on a RISC-V target should have the same effect as -hartid and a deprecation message should be shown.

[JanMatCodasip]

Hi @en-sc

I agree that the name -coreid is misleading in case of RISC-V.

By default hartid can be assigned based on target index.

I am not sure I understand this fully. Where would the hard index be taken from if -coreid is removed?

RISC-V-specific -hartid configure option can be introduced.

That'd be my preference. Furthermore:

• I'd pick a different name for the parameter to not confuse the user that it has something to do with the value of mhartid CSR register. It could be named for example -hart_index <num> or -hart <num> or similar.
• It'd be good to still support -coreid <num> but show a deprecation warning, allowing the users to adapt their TCL scripts.

[en-sc]

It'd be good to still support -coreid but show a deprecation warning

You are right. Updated the description.

I am not sure I understand this fully. Where would the hard index be taken from if -coreid is removed?

From the order the targets are created in:

target create ... # hartid implicitly 0
target create ... # hartid implicitly 1

IMHO it's a nice default, considering that the targets are usually the same anyway (SMP) (especially when they are connected to the same DM).

Moreover, this will make validation of hartid unnecessary (it should be contiguous and start from zero).

And there is also the case of a platform with multiple DMs (here hartid is not unique).

[JanMatCodasip]

Where would the hard index be taken from if -coreid is removed?

From the order the targets are created in

I'd strongly prefer to not have implicitly incrementing hart numbers for these reasons:

• For clarity ("explicit is better than implicit" - PEP 20)
• To not change the behavior: now omitted hart ID means fixed 0, after the change the omitted hart ID would behave very differently
• For various (less or more obscure) reasons the user may not want to debug all harts belonging to a given debug module. For example the user may only be interested in debugging harts 0 and 3.

Furthermore, it'd be good to check the uniqueness of hart number for a given debug module (that is, for a given pair of JTAG TAP and -dbgbase).

[en-sc]

I see your point, however I'd like to disagree:

To not change the behavior: now omitted hart ID means fixed 0, after the change the omitted hart ID would behave very differently

This is not true. AFAIU, if harts with omitted hart ID are on the same DM -- it's an error that is not currently diagnosed.
If hart ID is specified with a value greater then the detected number of harts on the DM --it's also an error that is not diagnosed.
(it sometimes "works" since hartsel is WARL). What I'm trying to say is, IMHO there is not much "behavior" to preserve here.

For various (less or more obscure) reasons the user may not want to debug all harts belonging to a given debug module. For example the user may only be interested in debugging harts 0 and 3.

AFAIU, this is a non-default scenario which can be covered by non-default config.

For clarity ("explicit is better than implicit" - PEP 20)

Oh why did you have to quote PEP here.. I'd agree with the argument but I hate Python with all my heart :)

[en-sc]

In all seriousness though, my main argument is: assigning hartid based on target index makes it correct by default most of the time. Otherwise extensive verification is required.

E.g.a system with two DMs on one TAP:

• Explicit hart ID:|

target create ... -dbgbase 0 -hartid 0
target create ... -dbgbase 0 -hartid 1
target create ... -dbgbase yyy -hartid 0
target create ... -dbgbase yyy -hartid 1

• Implicit hart ID:

target create ...  # -dbgbase 0, hartid 0
target create ...
target create ... # -dbgbase based on the value of `nextdm`, hartid 0
target create ...

You can see that requiring the user to specify hartid explicitly makes it so the user needs to specify information that can be easily examined (and should be examined to verify that user configuration is correct).

[JanMatCodasip]

me: To not change the behavior: now omitted hart ID means fixed 0, after the change the omitted hart ID would behave very differently

en-sc: This is not true. AFAIU, if harts with omitted hart ID are on the same DM -- it's an error that is not currently diagnosed. (...) IMHO there is not much "behavior" to preserve here.

Fair point, I agree!

I also am in agreement that OpenOCD should auto-detect the base addresses of all DMs and the number of harts on each DM.

me: For various (less or more obscure) reasons the user may not want to debug all harts (...).

en-sc: AFAIU, this is a non-default scenario which can be covered by non-default config.

I would like there to not be two ways of writing the configuration (default vs. non-default, or we could call them automatic vs. explicit).

In my opinion, the only advantage of the "automatic" configuration is shorter configuration - less characters to type.

Other than that, I am afraid that I can only see disadvantages:

• Too much automation happening behind the scenes (dbgbase and coreid automatically assigned).
• Two ways of writing the same thing: automatic vs. explicit.
• How would automatic and explicit configuration work together? What if just some -hartid and -dbgbase items are specified and others left blank. How would they interact? Would it behave like implicitly vs. explicitly defined values in C enums? Or differently? The chosen behavior would also all need to be described clearly in the documentation. The users could have problems understanding the meaining of the configs without reading that documentation.
• Structure of the system will not be clear after reading the configuration file.
• It won't be immediately apparent to the user that -dbgbase and -coreid options actually exist just by looking at the configuration file.
• Increased likelihood of mistakes: Configuration file for one SoC could be by mistake applied onto a different SoC with different structure but same number of harts - the user may not notice.
• Automatic configuration would not follow the principles that OpenOCD already uses in similar places: Take the JTAG chain configuration as an example. OpenOCD is able to auto-probe the JTAG chain but still requires the user to specify the expected chain structure.

For those reasons, I'd prefer to not have the automatic configuration.

To summarize, my preference would be:

• Keep the current defaults only: -dbgbase 0x0, -hartid 0. Do not try to make OpenOCD more clever. The current defaults nicely cover the most basic case of single DM and single hart.
• Ask the user to explicitly specify dbgbase and hartid for any case that is more complex than the above.
• Display error messages on obvious mistakes in the configuration prior to examine() (e.g. duplicate pairs of dbgbase and hartid).
• Detect the number of DMs and harts in the hardware during examine().
• Validate the user configuration against what has been detected.
• If there is a mismatch, provide a hint to the user what configuration commands they should use - in the same manner as JTAG chain auto-probing works.