proposal: dap-server: allow read register value by default

[eZioPan]

see

• dap-server default register restricted_read value block viewing register value on STM32H5 #2265

for detail

[noppej]

@burrbull Would you be open to advising on this? I originally took the conservative approach (to compensate for my limited experience in this area), because I was concerned about unintended consequences when reading modifies some registers.

What do you recommend?

[burrbull]

I recommend to use expand_properties on Device after parsing.
After this access == None always is read-write.

[eZioPan]

@burrbull thanks for reply!

I have try your suggestion, but expand_properties just not work in this situation. Since the Device has been expanded before:

probe-rs/probe-rs/src/bin/probe-rs/cmd/dap_server/peripherals/svd_variables.rs

Lines 38 to 40 in b6f1f1e

	let svd_cache = match svd::parse_with_config(
	svd_xml,
	&Config::default().expand(true).ignore_enums(true),

And after the expantion at parsing, the .access value of .default_register_properties just be None. Thus rerun expand_properties still get None.

I guess the reason this happend is from svd-rs

https://github.com/rust-embedded/svd/blob/819473991adee9b205cd77d1ed7ae28d89dbe318/svd-rs/src/registerproperties.rs#L33-L38

the default value of access is None, and none of the crates set it to Some(ReadWrite)

[burrbull]

I have try your suggestion, but expand_properties just not work in this situation. Since the Device has been expanded before:

.expand(true) is about arrays/clusters and derivedFrom.
There is also .expand_properties(true) option.
It does not convert None to Some(ReadWrite) (should it?). It just update missing register properties (size, access, resetValue) from peripheral's or device's default_register_properties. Nothing more smart.
I recommend to enable it anyway.

P.S. Saying about .expand(true). If you want to decrease memory usage you should disable it. Create Index and then "derive" peripherals/registers, expand arrays, etc. manually on request. Although this is not easy task.

[eZioPan]

Ah, so you mean "use expand_properties to add an extra guarantee, that None can be safely assume to read-write", not "use expand_properties to make None to Some(ReadWrite)"?

[burrbull]

Ah, so you mean "use expand_properties to add an extra guarantee, that None can be safely assume to read-write"?

Yes. SVD can keep part of information upper on tree.

[burrbull]

To check if reading register is danger you also should look at each field in register. It also has readAction.
Although I never did this as svd2rust can do nothing with this information except better docs.

[eZioPan]

P.S. Saying about .expand(true). If you want to decrease memory usage you should disable it. Create Index and then "derive" peripherals/registers, expand arrays, etc. manually on request. Although this is not easy task.

Thank for the hints! I might just keep track on the issue, this suggestion might be a little over my ability. XD

[eZioPan]

you also should look at each field in register. It also has readAction.

Yeah, and it has already been checked in original code:

probe-rs/probe-rs/src/bin/probe-rs/cmd/dap_server/peripherals/svd_variables.rs

Lines 150 to 155 in b6f1f1e

	let field_has_restricted_read = register_has_restricted_read
	|| field.read_action.is_some()
	|| field
	.access
	.map(|a| !a.can_read())
	.unwrap_or(register_has_restricted_read);

[noppej]

P.S. Saying about .expand(true). If you want to decrease memory usage you should disable it. Create Index and then "derive" peripherals/registers, expand arrays, etc. manually on request. Although this is not easy task.

Thank for the hints! I might just keep track on the issue, this suggestion might be a little over my ability. XD

@eZioPan I am a bit slammed this week, but will look into this early next week and if you are keen, I can help you with it.

[eZioPan]

and if you are keen, I can help you with it.

@noppej Thank you for your kindness!

I have no experience on using svd library before, and it might be a long time for me to learn it, and potentially let you wait a lot. I would ask leaving the task for someone more familiar than me. And thanks again!