-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add other atomic instructions #558
Conversation
e1c78be
to
8f10aef
Compare
86c3148
to
7748b4c
Compare
85a9702
to
b9eb437
Compare
Fixes vbpf#483 Signed-off-by: Dave Thaler <[email protected]>
I'm probably missing something, but aren't the instructions atomic because they change memory regions that are already volatile? |
Apparently atomic operations can also work on stack memory. Not that you need atomic operations for them, but if you do them they shouldn't fail verification per se (and don't on Linux). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general. See my question about the choice of fields though.
Signed-off-by: Dave Thaler <[email protected]>
Signed-off-by: Dave Thaler <[email protected]>
Signed-off-by: Dave Thaler <[email protected]>
Signed-off-by: Dave Thaler <[email protected]>
Signed-off-by: Dave Thaler <[email protected]>
I think I've now made changes according to all your comments. Please re-review. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just a minor comment and a question.
Signed-off-by: Dave Thaler <[email protected]>
(INST_MEM << 5)
etc. with the more readable and less bug-proneINST_MODE_MEM
.Fixes #483