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

DasharoSystemFeaturesUiLib: Use callbacks to update CPU Throttling Threshold #202

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

miczyg1
Copy link
Contributor

@miczyg1 miczyg1 commented Jan 17, 2025

For some reason the CPU Throttling Threshold dsiplayed negative value when user changed CPU throttling, exited Power Management Options menu, went back to front page and then entered Power Management Options menu again.

Always calculate the threshold based on PCD and current throttling offset value. Add interactive flag to the options and patch the value in the callback.

@krystian-hebel
Copy link
Contributor

Check if UEFI revocation list is up-to-date / check (pull_request) Failing after 7s

Is it because of CVE-2024-7344?

@mkopec
Copy link
Member

mkopec commented Jan 20, 2025

@miczyg1 miczyg1 force-pushed the cpu_throttle_offset_fix branch 2 times, most recently from 0820226 to e149e1e Compare January 30, 2025 12:16
@SergiiDmytruk
Copy link
Member

For some reason the CPU Throttling Threshold dsiplayed negative value when user changed CPU throttling, exited Power Management Options menu, went back to front page and then entered Power Management Options menu again.

This is worrisome, like there is a memory issue that will now affect something else since a field has been removed (e.g., HybridCpuArchitecture which is the next one). Or is this an issue with how forms are implemented (now necessarily in our code)? Were you able to reproduce it on QEMU? It didn't work for me on v0.2 with all the menus.

@miczyg1
Copy link
Contributor Author

miczyg1 commented Jan 30, 2025

For some reason the CPU Throttling Threshold dsiplayed negative value when user changed CPU throttling, exited Power Management Options menu, went back to front page and then entered Power Management Options menu again.

This is worrisome, like there is a memory issue that will now affect something else since a field has been removed (e.g., HybridCpuArchitecture which is the next one). Or is this an issue with how forms are implemented (now necessarily in our code)? Were you able to reproduce it on QEMU? It didn't work for me on v0.2 with all the menus.

I think it has to do with how read directive in VFR/IFR works and when it is invoked, rather than memory issue.

…reshold

For some reason the CPU Throttling Threshold dsiplayed negative value when
user changed CPU throttling, exited Power Management Options menu, went back
to front page and then entered Power Management Options menu again.

Always calculate the threshold based on PCD and current throttling offset
value. Add interactive flag to the options and patch the value in the
callback.

Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
@miczyg1 miczyg1 force-pushed the cpu_throttle_offset_fix branch from e149e1e to 27e4a00 Compare February 4, 2025 12:59
Copy link
Member

@SergiiDmytruk SergiiDmytruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OVMF seems to work the same with and without these changes (variables are updated correctly), so seems like they at least don't break anything.

@SergiiDmytruk SergiiDmytruk merged commit 27e4a00 into dasharo Feb 4, 2025
3 checks passed
@SergiiDmytruk SergiiDmytruk deleted the cpu_throttle_offset_fix branch February 4, 2025 17:38
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

Successfully merging this pull request may close these issues.

4 participants