-
Notifications
You must be signed in to change notification settings - Fork 228
Tests and Coverpoints for the LR/SC instructions of Atomic extension #600
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
base: dev
Are you sure you want to change the base?
Conversation
Here is some feedback from a review of the testplan shared by Muhammad Maarij (10xEngineers) at I have not reviewed whether these tests implement the testplan or whether any of the concerns about the testplan have been resolved by the tests. Aligned LR/SC Success: “matching reservation sets” is unclear. Do you mean exactly matching addresses? The size of the reservation set is implementation-dependent. The comment about testing at the extreme of granularity probably belongs here, and the one just outside granularity belongs in SC Fails. Misaligned Exceptions: these require Zicsr. Are you separating out tests that require privileged support from tests that can run without Zicsr? LR and SC can also generate access faults, but I don’t see entries. SC Fails - Another SC: This is interesting to test both with SC2 the same address as well as a different one in the same set and a different one in a different set. LR/SC normal order: I don’t know how you check “LR/SC sequences can occur before or after surrounding memory ops” Acquire/Release semantics/Sequenntially Consistent: I don’t know how you check no ops observed before/after. Empty LR/SC Sequence: what does “no instructions” mean? No LR and SC? Or nothing between them? How do you check for observable ordering constraints? Constrained LR/SC Loop success: this would need flushing out to define what goes beween the LR and SC. In particular, I think having a load and a store to the reservation set and to a different address is an interesting one to check. Constrained Loop Failure - Unsuccessful SC: if SC continues to fail, how do you exit the loop? Constrained Loop - Max Instructions: I’m not sure what this tests? That a particular loop with 16 instructions always succeeds? Be specific about what is in the loop, besides 16 instructions. Constrained loop > Max Instructions: How does a test determine whether it may succeed on some attempts? I don’t think there is anything we can learn from this test - if it may or may not succeed, all behaviors are legal, and it can't catch any bugs in the design, so isn’t worth performing. Unconstrained LR/SC: Again, what bug could this catch? SC Fail - store from another hart: This is only testable in a multicore system, and probably belongs in a different testplan than single-core LR/SC. The spec technically calls for “no write from a device other than the hart” so it is not just normal stores, but also sc or cboz or floating or compressed stores, as well as peripheral accesses that should be tested. I think there are other easy architectural things that should be tested, such as lr and sc should be able to use every register for source/destinations (except rs1=x0), lr can read zero and non-zero values from memory and sc and write the same. lr.w on RV64 should sign-extend properly. Both .w and .d flavors need testing. |
Other interesting cases:
- address aliasing, where 2 VAs map to the PA, and the LR uses one, and
the SC uses the other.
There is an (unnamed) architectural option to allow or disallow matches
in that case.
- tests for SC completely inside the LR range (e.g. a byte SC at
offset 1 of a word LR range),
and completely outside the LR range (e.g. a byte LR at
offset 1 of a word SC range),
(they should succeed)
…On Tuesday, February 18, 2025, David Harris ***@***.***> wrote:
Here is some feedback from a review of the testplan shared by Muhammad
Maarij (10xEngineers) at
https://docs.google.com/spreadsheets/d/1kYHKfZmSDmKRlSjKyRu-vJVfNVUDYSDgLT42q_Obbxw/edit?gid=0#gid=0
I have not reviewed whether these tests implement the testplan or whether
any of the concerns about the testplan have been resolved by the tests.
Aligned LR/SC Success: “matching reservation sets” is unclear. Do you mean
exactly matching addresses? The size of the reservation set is
implementation-dependent. The comment about testing at the extreme of
granularity probably belongs here, and the one just outside granularity
belongs in SC Fails.
Misaligned Exceptions: these require Zicsr. Are you separating out tests
that require privileged support from tests that can run without Zicsr?
LR and SC can also generate access faults, but I don’t see entries.
SC Fails - Another SC: This is interesting to test both with SC2 the same
address as well as a different one in the same set and a different one in a
different set.
LR/SC normal order: I don’t know how you check “LR/SC sequences can occur
before or after surrounding memory ops”
Acquire/Release semantics/Sequenntially Consistent: I don’t know how you
check no ops observed before/after.
Empty LR/SC Sequence: what does “no instructions” mean? No LR and SC? Or
nothing between them? How do you check for observable ordering constraints?
Constrained LR/SC Loop success: this would need flushing out to define
what goes beween the LR and SC. In particular, I think having a load and a
store to the reservation set and to a different address is an interesting
one to check.
Constrained Loop Failure - Unsuccessful SC: if SC continues to fail, how
do you exit the loop?
Constrained Loop - Max Instructions: I’m not sure what this tests? That a
particular loop with 16 instructions always succeeds? Be specific about
what is in the loop, besides 16 instructions.
Constrained loop > Max Instructions: How does a test determine whether it
may succeed on some attempts? I don’t think there is anything we can learn
from this test - if it may or may not succeed, all behaviors are legal, and
it can't catch any bugs in the design, so isn’t worth performing.
Unconstrained LR/SC: Again, what bug could this catch?
SC Fail - store from another hart: This is only testable in a multicore
system, and probably belongs in a different testplan than single-core
LR/SC. The spec technically calls for “no write from a device other than
the hart” so it is not just normal stores, but also sc or cboz or floating
or compressed stores, as well as peripheral accesses that should be tested.
I think there are other easy architectural things that should be tested,
such as lr and sc should be able to use every register for
source/destinations (except rs1=x0), lr can read zero and non-zero values
from memory and sc and write the same. lr.w on RV64 should sign-extend
properly. Both .w and .d flavors need testing.
—
Reply to this email directly, view it on GitHub
<#600 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJTXMVS4PBRM66NGMSD2QMJ6NAVCNFSM6AAAAABVPTL2WGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRVGM3DSMJSGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
[image: davidharrishmc]*davidharrishmc* left a comment
(riscv-non-isa/riscv-arch-test#600)
<#600 (comment)>
Here is some feedback from a review of the testplan shared by Muhammad
Maarij (10xEngineers) at
https://docs.google.com/spreadsheets/d/1kYHKfZmSDmKRlSjKyRu-vJVfNVUDYSDgLT42q_Obbxw/edit?gid=0#gid=0
I have not reviewed whether these tests implement the testplan or whether
any of the concerns about the testplan have been resolved by the tests.
Aligned LR/SC Success: “matching reservation sets” is unclear. Do you mean
exactly matching addresses? The size of the reservation set is
implementation-dependent. The comment about testing at the extreme of
granularity probably belongs here, and the one just outside granularity
belongs in SC Fails.
Misaligned Exceptions: these require Zicsr. Are you separating out tests
that require privileged support from tests that can run without Zicsr?
LR and SC can also generate access faults, but I don’t see entries.
SC Fails - Another SC: This is interesting to test both with SC2 the same
address as well as a different one in the same set and a different one in a
different set.
LR/SC normal order: I don’t know how you check “LR/SC sequences can occur
before or after surrounding memory ops”
Acquire/Release semantics/Sequenntially Consistent: I don’t know how you
check no ops observed before/after.
Empty LR/SC Sequence: what does “no instructions” mean? No LR and SC? Or
nothing between them? How do you check for observable ordering constraints?
Constrained LR/SC Loop success: this would need flushing out to define
what goes beween the LR and SC. In particular, I think having a load and a
store to the reservation set and to a different address is an interesting
one to check.
Constrained Loop Failure - Unsuccessful SC: if SC continues to fail, how
do you exit the loop?
Constrained Loop - Max Instructions: I’m not sure what this tests? That a
particular loop with 16 instructions always succeeds? Be specific about
what is in the loop, besides 16 instructions.
Constrained loop > Max Instructions: How does a test determine whether it
may succeed on some attempts? I don’t think there is anything we can learn
from this test - if it may or may not succeed, all behaviors are legal, and
it can't catch any bugs in the design, so isn’t worth performing.
Unconstrained LR/SC: Again, what bug could this catch?
SC Fail - store from another hart: This is only testable in a multicore
system, and probably belongs in a different testplan than single-core
LR/SC. The spec technically calls for “no write from a device other than
the hart” so it is not just normal stores, but also sc or cboz or floating
or compressed stores, as well as peripheral accesses that should be tested.
I think there are other easy architectural things that should be tested,
such as lr and sc should be able to use every register for
source/destinations (except rs1=x0), lr can read zero and non-zero values
from memory and sc and write the same. lr.w on RV64 should sign-extend
properly. Both .w and .d flavors need testing.
—
Reply to this email directly, view it on GitHub
<#600 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJTXMVS4PBRM66NGMSD2QMJ6NAVCNFSM6AAAAABVPTL2WGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRVGM3DSMJSGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
There doesn't seem to be a test sequence with two (or more) LRs followed by an SC. The LRs may or may not be to overlapping reservations. The spec requires implies there is only one (or at most one) reservation set active at a time. It would be good to have a test for that. |
I thought I had seen tests like that, but I'll look again. There certainly
should be, in any case.
…On Wed, Mar 5, 2025 at 7:30 AM Prashanth Mundkur ***@***.***> wrote:
There doesn't seem to be a test sequence with two (or more) LRs followed
by an SC. The LRs may or may not be to overlapping reservations. The spec
requires implies there is only one (or at most one) reservation set active
at a time. It would be good to have a test for that.
—
Reply to this email directly, view it on GitHub
<#600 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJSQDIBK6OXIWRDJAJT2S4KBFAVCNFSM6AAAAABVPTL2WGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBRGI4DINRWHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
[image: pmundkur]*pmundkur* left a comment
(riscv-non-isa/riscv-arch-test#600)
<#600 (comment)>
There doesn't seem to be a test sequence with two (or more) LRs followed
by an SC. The LRs may or may not be to overlapping reservations. The spec
requires implies there is only one (or at most one) reservation set active
at a time. It would be good to have a test for that.
—
Reply to this email directly, view it on GitHub
<#600 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJSQDIBK6OXIWRDJAJT2S4KBFAVCNFSM6AAAAABVPTL2WGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMBRGI4DINRWHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Description
Adds tests and coverpoints for LR/SC instructions in Atomic (A) Extension. Updated the arch_test.h to include the macros to run the LR/SC tests.
Related Issues
N/A
Ratified/Unratified Extensions
List Extensions
Atomic Extension (A)
Reference Model Used
Mandatory Checklist:
Optional Checklist: