On 14/08/2023 15:43, John Garry wrote:

On 14/08/2023 15:15, James Clark wrote:

...

On 14/08/2023 14:07, John Garry wrote:

...

On 11/08/2023 15:39, James Clark wrote:

...

Currently variant and revision fields are masked out of the MIDR so it's not possible to compare different versions of the same CPU. In a later commit a workaround will be removed just for N2 r0p3, so enable comparisons on version.

This has the side effect of changing the MIDR stored in the header of the perf.data file to no longer have masked version fields.

Did you consider adding a raw version of _get_cpuid(), which returns the full MIDR just for the purpose of caller strcmp_cpuid_str()?

I did, but I thought that seeing as it would only be used in one place, and that changing the existing one didn't break anything, that it was better to not fragment the CPU ID interface. I thought it might also have repercussions for the other architectures as well. It would also mean that the MIDR that's stored in the header wouldn't have the version information, which if we're starting to do things with that could be bad.

There are already callers of strcmp_cpuid_str() so it's probably best to keep it using the same get_cpuid() string. Unless there is a reason _not_  to do it? There isn't really anything that can't be done with it accepting/returning the full unmasked MIDR. If you want the old behavior, you just set the version fields to 0, which I've also used in a later patch and is already done in mapfile.csv

ok, fine, so we seems that we would be following x86 on this in terms of using strcmp_cpuid_str(). It would be good to mention that there is already a weak version of strcmp_cpuid_str() for !x86 in your commit message.

Yep I can add this.

Let me check your code again...

Thanks, John