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

Improvement to CPU Vendor tests #284

Merged
merged 3 commits into from
Aug 9, 2024
Merged

Improvement to CPU Vendor tests #284

merged 3 commits into from
Aug 9, 2024

Conversation

fritz-fritz
Copy link
Contributor

In response to #274, this should prevent Intel uCode from being attempted on AMD. It has not been tested on Intel CPUs however and should be before merging.

The existing check from the AMD.pm was implemented for the Intel.pm in the same style. Both were updated to use a case-insensitive regex.

Adds a check similar to the existing AMD uCode code to decide if we should continue.
Changed the test here only to keep consistent with the test introduced to the Intel uCode.
@stbuehler
Copy link
Contributor

As I said in #274 I think GenuineIntel would probably the more appropriate check for intel, and I don't see why you'd want to change AuthenticAMD to /amd/i unless you can show the previous check is a problem.

(I assume that for both intel and amd we always get those strings for "official" cpus. No idea whether there are CPUs out there that claim to be somewhat compatible that would match /intel|amd/i, but if there are, I'd be very surprised if they work with the microcode handling here.)

Apart from that the changes look good to me - works on both intel and amd for me.

Instead of using a loose regex, continue using hardcoded known values.
@fritz-fritz
Copy link
Contributor Author

Sorry for letting this sit, I initially used a loose regex on Intel as I wasn't sure if I could rely on GenuineIntel to be standard and have no Intel to test on. So then I adapted the AMD check to match just to keep the code consistent.

I went ahead and pushed a new commit changing both to hardcoded checks on AuthenticAMD and GenuineIntel. I am also willing to rebase to squash the commit history if you'd like. Otherwise I think this is good to merge 🚀

@fritz-fritz fritz-fritz marked this pull request as ready for review August 2, 2024 08:06
@liske liske merged commit f862b0f into liske:master Aug 9, 2024
@liske
Copy link
Owner

liske commented Aug 9, 2024

Thanks!

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.

3 participants