-
Notifications
You must be signed in to change notification settings - Fork 32
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
Adding optional CPUID field in microarchitecture schema and in json file for Arm architectures #110
Conversation
- Added the new optional field in microarchitectures schema - Add field where appropriate in microarchitecture
Sorry, didn't realise I had not tagged @alalazo in the 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.
Let's work on this one first, to get the detection based on cpuid
/cpupart
working. Then we can add neoverse_n2
and deprecate the neoverse_v2
optional features.
cpu/microarchitectures.json
Outdated
@@ -2906,7 +2907,8 @@ | |||
"flags": "-tp {name}" | |||
} | |||
] | |||
} | |||
}, | |||
"cpuid": "0x41d0c" |
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.
We already have the vendor
field above, which if I understand correctly covers for the first part of the cpuid
information (there's a mapping below in the file to go from hex code to vendor name). The second part seems to be the cpupart
, i.e. 0xd0c
. If that's the case, can we just add that?
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.
You are right. I have replaced cpuid
with cpupart
and removed the cpu implementer
part from the string.
In the archspec repo, I'll apply the changes to deal with the cpupart
field from the json.
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.
Hi @alalazo , I have pushed on this branch, the introduction of cpupart instead of cpuid as per above and kept the field optional.
The `cpuid` is the concatenation of the `cpu implementer` and the `cpu part` coming from /proc/cpuinfo. Instead of having the `cpuid`, we have `cpupart` in the json file, as the `cpu implementer` can be decuded from the conversions table at the end of the microarchitectures.json file.
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.
Thanks
As part of archspec/archspec#168 on the archspec repo, it was decided to add, where it made sense for ARM cores a new field, called
cpuid
rather than adding a new submodule just for that.These changes aim at:
This field will be used in the cpu/detect.py file in the archspec repo, for which a separate PR will be opened.
CC: @dslarm