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

ISS mismatch on writing 0 to tdata1 #1517

Closed
silabs-oysteink opened this issue Nov 30, 2022 · 13 comments
Closed

ISS mismatch on writing 0 to tdata1 #1517

silabs-oysteink opened this issue Nov 30, 2022 · 13 comments

Comments

@silabs-oysteink
Copy link
Contributor

silabs-oysteink commented Nov 30, 2022

ISS mismatch on writing 0 to tdata1

Writing 0 to tdata1 shall cause tdata1 to contain a disabled trigger (type 15)
image

When writing 0 to tdata1 I get the following ISS mismatch:
image

Type

  • ISS error

Steps to Reproduce

Use the same steps as issue #1516 to reproduce with the following modifications:

  1. replace line 83 (csrw tdata1, t0) with (csrw tdata1,x0)
  2. uncomment line 633 in tb/uvmt/uvmt_cv32e40x_imperas_dv_wrap.sv to enable checks for tdata1.
@eroom1966
Copy link
Contributor

Hi @silabs-oysteink
(mention @Silabs-ArjanB)
I have not yet tried to reproduce, but looking at the error, isn't this an illegal write violating the WARL behavior ?
https://docs.openhwgroup.org/projects/cv32e40x-user-manual/en/0.6.0/control_status_registers.html#trigger-data-1-tdata1

The specification as my understanding goes, if an illegal value is written to a CSR, the value remains unchanged.
0x0 is not a legal value for tdata1

@silabs-oysteink
Copy link
Contributor Author

Hi @eroom1966 ,
I agree that the current user manual describes the WARL behavior as you say. However, the debug spec is explicit in the case of writing 0 to tdata1, so we will add to the note for tdata1 in the user manual that "writing 0x0 disables the trigger (and therefore changes the value of tdata1 to 0xf800_0000, which is the only supported value for a disabled trigger)"

@eroom1966
Copy link
Contributor

@silabs-oysteink
investigating this a little further - It appears from the debug spec there is an implementation design choice which says when writing the value 0, which non-zero value is assigned. I think this also needs adding to your specification document, that a write of type=0, results in a read value of type=0xf

This will need a revision of the model, which I can investigate now, I have your testcase, so I do have something I can quickly test against.

On a related note, the reason I could not get the tracing to work was an error in the yaml file for the etrigger test, the yaml is still referring to the trigger test.

@Silabs-ArjanB
Copy link
Contributor

Hi @eroom1966 I made an update to our user manual (which will also get merged over to the CV32E40S) that explains this behavior openhwgroup/cv32e40x#712

@eroom1966
Copy link
Contributor

Hi @Silabs-ArjanB
Can I ask the following question, what should happen if tdata1 is written with (for example) 0x0abcdeff, where 0abcdeff != 0000000, but tdata.type is 0x0

Thx
Lee

@Silabs-ArjanB
Copy link
Contributor

Hi @eroom1966 tdata1 would then remain unchanged. The supported types are 0x5, 0x6, 0xF and also a write of tdata1 to 0x0000_0000 is supported (and will result in tdata1 of 0xF800_0000)

@eroom1966
Copy link
Contributor

Thanks for the clarification, this is what we thought, but wanted to be absolutely sure

@Silabs-ArjanB
Copy link
Contributor

Hi @eroom1966 , we looked at above behavior again and unforunately we changed our mind. The change is documented in openhwgroup/cv32e40x#733 and will be released later. Effectively if type is written to an illegal value tdata1 will change to 0xF800_0000. So in the example you gave when writing 0x0abcdeff the resulting value will become 0xF800_0000 as well. The advantage for us for this change is that we can then express the behavior of this register with our regular WARL syntax (i.e. in this case we marked 0XF as the default resolution value for type).

@eroom1966
Copy link
Contributor

eroom1966 commented Dec 20, 2022

@Silabs-ArjanB
For the change documented here - I am assuming this is going to be in version 0.7.0 ?
what does the addition of the asterisk '*' mean, I could not find a description, I am guessing it means default value when an illegal value written, but would ask for clarification

@Silabs-ArjanB
Copy link
Contributor

Silabs-ArjanB commented Dec 21, 2022

Hi @eroom1966 , yes, this change and others will soon be released in a new user manual version (0.7.0).

Your interpretation of the asterisk is correct. It is documented at https://docs.openhwgroup.org/projects/cv32e40x-user-manual/en/0.6.0/control_status_registers.html#csr-descriptions:

image

@eroom1966
Copy link
Contributor

@silabs-oysteink
I believe this issue is now resolved, could you comment on the status as you understand ?

@eroom1966 eroom1966 removed their assignment Feb 21, 2023
@eroom1966
Copy link
Contributor

@MikeOpenHWGroup @silabs-oysteink
is this issue still relevant ?
If it is could I please have a testcase to reproduce

@halfdan-dolva
Copy link
Contributor

This issue has been fixed.

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

4 participants