-
Notifications
You must be signed in to change notification settings - Fork 587
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
aarch64: Save/Restore PAC keys #3569
Conversation
Should fix #3568.
We'll look into getting a buildkite agent set up on an M1, so we can actually test this. |
Seeing the following test failures on an M1 with this PR on Fedora Asahi Remix, so I think it's mostly working:
|
I guess to fix |
From the testing that Ben did |
Friendly ping @Keno to possibly merge as is (existing tests that failed before on aarch64 now pass) and add follow-up PRs if/as necessary. |
I was hoping to get the M1 buildkite up, so we can actually test this. Let me follow up on that. |
@Keno Just wanted to ping about this. Is the testing infrastructure in place for this now? |
Ping; this also fixed my PAC issue. Note that in addition to the keys themselves, we'll also need to save/restore the key enabled state, which is available via |
Yup, fixing things here as well :) |
Is there anything that the community can do to help move this forward? |
There are a few known missing aspects of PAC support that I'm aware of:
But these are fairly minor features: for 99+% of users who record and replay on the same system, rr will work fine. And if replay fails due to missing PAC support, it will not be obvious what the issue is unless you already know about PAC. So I think this should land as is, and the remaining features can be added in followup changes and the CI support can be added asynchronously. Since there's no PAC support in CI, at worst we introduce a bug that breaks PAC and then we're in the same position as if this patch didn't land. |
I merged this with a small tweak to fix the non-ARM builds. Please file issues for the followup work. Thanks! |
Should fix #3568.