target/riscv: hide_csrs configuration option #787

aap-sc · 2023-01-10T14:59:26Z

This option allows users to mark certain CSRs as hidden so they could be expluded from reg output and target.xml

Change-Id: Iddf8456cd3901f572f8590329ebba5229974d24a

aap-sc · 2023-01-10T15:00:21Z

I describe the motivation for this PR here: #785

timsifive

Looks alright. Can you document this new command in doc/openocd.texi?

timsifive · 2023-01-12T19:29:06Z

src/target/riscv/riscv.h

@@ -248,6 +248,10 @@ typedef struct {
 	 * from range 0xc000 ... 0xffff. */
 	struct list_head expose_custom;

+	/* The list of registers to mark as "hidden". Hidden registers are available
+	 * but do not appear in gdb targets description or reg command output. */
+	struct list_head hidden_csr;


Suggested change

struct list_head hidden_csr;

struct list_head hide_csr;

To keep naming consistent with expose_csr.

timsifive · 2023-01-12T19:33:28Z

src/target/riscv/riscv.c

+		.help = "Configure a list of inclusive ranges for CSRs to mark them as "
+			"hidden. Hidden registers are still available, but are not listed in "


Suggested change

.help = "Configure a list of inclusive ranges for CSRs to mark them as "

"hidden. Hidden registers are still available, but are not listed in "

.help = "Configure a list of inclusive ranges for CSRs to hide from gdb. "

"Hidden registers are still available, but are not listed in "

Thanks for suggestion, addressed

timsifive · 2023-01-12T19:34:03Z

src/target/riscv/riscv.c

+				range_list_t *entry;
+				list_for_each_entry(entry, &info->hidden_csr, list)
+					if ((entry->low <= csr_number) && (csr_number <= entry->high)) {
+						LOG_DEBUG("Hiding CSR %d (name=%s)", csr_number, r->name);


Suggested change

LOG_DEBUG("Hiding CSR %d (name=%s)", csr_number, r->name);

LOG_TARGET_DEBUG(target, "Hiding CSR %d (name=%s)", csr_number, r->name);

TommyMurphyTM1234 · 2023-01-12T20:01:01Z

This option allows users to mark certain CSRs as hidden so they could be expluded from reg output and target.xml

Change-Id: Iddf8456cd3901f572f8590329ebba5229974d24a

If it's going to be added then why not generalise it to allow ANY register to be hidden - integer, floating point, CSR - using any of the canonical name, the assembler mnemonic name, the ABI name(s?), any other relevant names (e.g. CSR alias names, deprecated/demoted CSR names)?

TommyMurphyTM1234 · 2023-01-12T20:03:29Z

src/target/riscv/riscv.c

@@ -3675,6 +3700,16 @@ static const struct command_registration riscv_exec_command_handlers[] = {
 			"expose. custom0 is accessed as abstract register number 0xc000, "


Why does it have to be executed before init?
Might the GDB user want to do monitor ... to change the register list?

Will the commands work via the OpenOCD telnet interface?

See here for my comment about identifying registers by name:

target/riscv: hide_csrs configuration option #787 (comment)

Why does it have to be executed before init?

Because list of available registers is built/initialized during target examination procedure.

Might the GDB user want to do monitor ... to change the register list?
Will the commands work via the OpenOCD telnet interface?

This won't work. There are a couple of reasons for that.

As is - the command can be run during the configuration stage only. That is before gdbserver client can connect.

AFAIK the remote target configuration is requested by gdb only once upon connection. Maybe there are ways to re-request it, but I'm not aware of them currently (+ see the next item).

The way registers are initialized in OpenOCD complicates things. These are initialized during the examine procedure. And making OpenOCD to re-examine target may imply way too many changes.

aap-sc · 2023-01-12T21:01:19Z

Regarding this one (from @TommyMurphyTM1234 ):

If it's going to be added then why not generalise it to allow ANY register to be hidden - integer, floating point, CSR - using any of the canonical name, the assembler mnemonic name, the ABI name(s?), any other relevant names (e.g. CSR alias names, deprecated/demoted CSR names)?

Regarding other classes of registers like GPR, FPR and VPR. I'll investigate this, however GDB may have some special handling for these kinds of registers and this may require some additional testing. I'm fairly certain that the code works for CSR's since I experimented with it quite a lot - not so sure for others.
Regarding aliases and canonical/ABI names... I don't believe we have canonical naming scheme present in the current codebase of OpenOCD and GDB. It was removed a couple of years ago from the respected codebases. As for aliases - I'll double check, but I don't believe we have any.

aap-sc · 2023-01-12T21:03:15Z

@timsifive, regarding this one:

Can you document this new command in doc/openocd.texi?

Sure (I've completely forgotten about it).

timsifive · 2023-01-17T17:28:07Z

If it's going to be added then why not generalise it to allow ANY register to be hidden - integer, floating point, CSR - using any of the canonical name, the assembler mnemonic name, the ABI name(s?), any other relevant names (e.g. CSR alias names, deprecated/demoted CSR names)?

That sounds like a lot of work for registers that nobody might ever want to hide. As it is the CSR work is already done so easy to hook into for this feature. At a minimum the parser would get a lot more complex to support what you mentioned above.

TommyMurphyTM1234 · 2023-01-17T17:37:33Z

That sounds like a lot of work for registers that nobody might ever want to hide. As it is the CSR work is already done so easy to hook into for this feature. At a minimum the parser would get a lot more complex to support what you mentioned above.

OK - fair enough. :-)

src/target/riscv/riscv.c

aap-sc · 2023-02-09T22:59:02Z

@timsifive

Looks alright. Can you document this new command in doc/openocd.texi?

I've added a short section.

@TommyMurphyTM1234 and @timsifive

That sounds like a lot of work for registers that nobody might ever want to hide

yep. I would prefer not to touch that code. It's messy even in GDB.

This option allows users to mark certain CSRs as hidden so they could be expluded from *reg* output and target.xml Change-Id: Iddf8456cd3901f572f8590329ebba5229974d24a

timsifive

Looks good.

JanMatCodasip

This looks fine. I have noticed just two small typos.

doc/openocd.texi

src/target/riscv/riscv.c

Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> Signed-off-by: Anatoly Parshintsev <114445139+aap-sc@users.noreply.github.com>

timsifive

It's even better now. :-)

aap-sc mentioned this pull request Jan 10, 2023

[question/feature] facilities to hide some CSRs from the user #785

Closed

timsifive requested changes Jan 12, 2023

View reviewed changes

TommyMurphyTM1234 reviewed Jan 12, 2023

View reviewed changes

JanMatCodasip requested changes Jan 23, 2023

View reviewed changes

src/target/riscv/riscv.c Outdated Show resolved Hide resolved

src/target/riscv/riscv.c Outdated Show resolved Hide resolved

aap-sc force-pushed the aap-sc/hide_csr_support branch from 4e0bf6f to 6856cd4 Compare February 9, 2023 22:54

aap-sc requested review from timsifive and JanMatCodasip and removed request for timsifive and JanMatCodasip February 9, 2023 22:59

target/riscv: hide_csrs configuration option

57aacec

This option allows users to mark certain CSRs as hidden so they could be expluded from *reg* output and target.xml Change-Id: Iddf8456cd3901f572f8590329ebba5229974d24a

aap-sc force-pushed the aap-sc/hide_csr_support branch from 6856cd4 to 57aacec Compare February 10, 2023 21:30

timsifive previously approved these changes Feb 13, 2023

View reviewed changes

JanMatCodasip previously approved these changes Feb 14, 2023

View reviewed changes

doc/openocd.texi Outdated Show resolved Hide resolved

src/target/riscv/riscv.c Outdated Show resolved Hide resolved

Update doc/openocd.texi

bfaceec

Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> Signed-off-by: Anatoly Parshintsev <114445139+aap-sc@users.noreply.github.com>

aap-sc dismissed stale reviews from JanMatCodasip and timsifive via bfaceec February 14, 2023 18:09

Update src/target/riscv/riscv.c

fe23c54

Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> Signed-off-by: Anatoly Parshintsev <114445139+aap-sc@users.noreply.github.com>

timsifive approved these changes Feb 14, 2023

View reviewed changes

JanMatCodasip approved these changes Feb 15, 2023

View reviewed changes

timsifive merged commit da5d274 into riscv-collab:riscv Feb 15, 2023

TommyMurphyTM1234 mentioned this pull request May 21, 2024

Using the info reg all command within gdb returns an remote failure reply 'E0E' #1070

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

target/riscv: hide_csrs configuration option #787

target/riscv: hide_csrs configuration option #787

aap-sc commented Jan 10, 2023

aap-sc commented Jan 10, 2023 •

edited

Loading

timsifive left a comment

timsifive Jan 12, 2023

aap-sc Feb 9, 2023

timsifive Jan 12, 2023

aap-sc Feb 9, 2023

timsifive Jan 12, 2023

aap-sc Feb 9, 2023

TommyMurphyTM1234 commented Jan 12, 2023

TommyMurphyTM1234 Jan 12, 2023

aap-sc Jan 12, 2023 •

edited

Loading

aap-sc commented Jan 12, 2023 •

edited

Loading

aap-sc commented Jan 12, 2023

timsifive commented Jan 17, 2023

TommyMurphyTM1234 commented Jan 17, 2023

aap-sc commented Feb 9, 2023

timsifive left a comment

JanMatCodasip left a comment

timsifive left a comment

		.help = "Configure a list of inclusive ranges for CSRs to mark them as "
		"hidden. Hidden registers are still available, but are not listed in "

	LOG_DEBUG("Hiding CSR %d (name=%s)", csr_number, r->name);
	LOG_TARGET_DEBUG(target, "Hiding CSR %d (name=%s)", csr_number, r->name);

		@@ -3675,6 +3700,16 @@ static const struct command_registration riscv_exec_command_handlers[] = {
		"expose. custom0 is accessed as abstract register number 0xc000, "

target/riscv: hide_csrs configuration option #787

target/riscv: hide_csrs configuration option #787

Conversation

aap-sc commented Jan 10, 2023

aap-sc commented Jan 10, 2023 • edited Loading

timsifive left a comment

Choose a reason for hiding this comment

timsifive Jan 12, 2023

Choose a reason for hiding this comment

aap-sc Feb 9, 2023

Choose a reason for hiding this comment

timsifive Jan 12, 2023

Choose a reason for hiding this comment

aap-sc Feb 9, 2023

Choose a reason for hiding this comment

timsifive Jan 12, 2023

Choose a reason for hiding this comment

aap-sc Feb 9, 2023

Choose a reason for hiding this comment

TommyMurphyTM1234 commented Jan 12, 2023

TommyMurphyTM1234 Jan 12, 2023

Choose a reason for hiding this comment

aap-sc Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

aap-sc commented Jan 12, 2023 • edited Loading

aap-sc commented Jan 12, 2023

timsifive commented Jan 17, 2023

TommyMurphyTM1234 commented Jan 17, 2023

aap-sc commented Feb 9, 2023

timsifive left a comment

Choose a reason for hiding this comment

JanMatCodasip left a comment

Choose a reason for hiding this comment

timsifive left a comment

Choose a reason for hiding this comment

aap-sc commented Jan 10, 2023 •

edited

Loading

aap-sc Jan 12, 2023 •

edited

Loading

aap-sc commented Jan 12, 2023 •

edited

Loading