Replies: 2 comments
-
Just tell users not to do so. |
Beta Was this translation helpful? Give feedback.
0 replies
-
@sbski told about a lock on PCI config space. I'm not aware that such an extreme kind of lock is even possible. He told me that 1usmus does use this kind of lock. I would like to know if this kind of lock could hurt something else before we use it to fix something which only very rarely occur (if at all) |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
How to reproduce it:
Because we don't check if it is ok to clear the response register and if it is ok to write argument registers, this will lead into problems.
Normally, we don't see them because:
I could find on issue after 1000's of calls: https://github.com/FlyGoat/RyzenAdj/blob/master/lib/nb_smu_ops.c#L51
@FlyGoat what do you think about this? I would say this isn't an issue because it is very unlikely to happen. But I am still not sure if it is good to clear the response register without checking anything else. But I don't know how this low-level stuff should be handled correctly. You will know it much better than me :)
I am not sure how you should handle concurrency on register level without knowing how the other tools do it. In best-case anybody could agree to a register which shows anybody else that the component is currently busy.
It doesn't help if we implement locking like the tools from @irusanov because locking does only help if anybody uses the same lock (don't mix tools)
The commit b32ad89 from pull request #137 does already solve 99% of the issues because only transfer table is likely to be executed so often, and it is very effective. I did use this solution to test, that's why I needed to include the 3rd step "some manually executed ryzenAdj CLI calls" to trigger an issue because with our skip PSMU call solution we don't cause register overwrites anymore.
Beta Was this translation helpful? Give feedback.
All reactions