Page MenuHomeFreeBSD

Add detection of CPU class for ARMv6/v7
ClosedPublic

Authored by mw_semihalf.com on May 25 2017, 3:29 PM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

andrew added a reviewer: bz.May 26 2017, 1:15 PM
bz edited edge metadata.May 26 2017, 1:29 PM

I have had the same diff in my local tree for ages. I am sure I mentioned it somewhere (bug report, follow-up to the commit, or just IRC).

While it does fix it when hwpmc is built into the kernel for one specific CPU, the module will not work at all, and I have not found a way to run-time detect which CPU we are running on to initialise the correct one (as for modules we need to built-in all). So in some ways this is all very backwards compared to real needs and the real problem is from months ago when the ability to run-time detect was removed from armv6/7.

andrew requested changes to this revision.May 26 2017, 1:36 PM

You should add cpu_class to sys/arm/identcpu-v6.c and set it in identify_arm_cpu.

This revision now requires changes to proceed.May 26 2017, 1:36 PM
zbb edited edge metadata.May 26 2017, 1:42 PM
In D10909#226365, @bz wrote:

I have had the same diff in my local tree for ages. I am sure I mentioned it somewhere (bug report, follow-up to the commit, or just IRC).

Good for you :P

mw_semihalf.com edited edge metadata.
mw_semihalf.com retitled this revision from Remove cpu_class as it is not defined for armv7 to Add detection of CPU class for ARMv6/v7.
mw_semihalf.com edited the summary of this revision. (Show Details)

Instead of removing cpu_class check in hwpmc, enable its usage amoung ARMv6/v7 SoCs.

bz added a comment.Jun 8 2017, 1:16 PM

Hmm I wonder if the entire struct can be exposed rather than just the class as not all "CPU_CLASS_CORTEXA" are the same and hwpmc will want to choose depending on .. hmm never mind; it's reading that directly from a register, so this will be fine. Thanks for doing it!

andrew accepted this revision.Jun 8 2017, 1:35 PM
This revision is now accepted and ready to land.Jun 8 2017, 1:35 PM
This revision was automatically updated to reflect the committed changes.
mmel added a subscriber: mmel.Jun 14 2017, 4:31 PM

Too late but still...
What exactly is "cpu_class" and why we need at all? I understand why it's useful for ARMv4, but it's near useless for ARMv7 (there is too much variants)

On ARMv6+, Performance Monitors Extension is optional unit, and its presence and version is determined by ID_DFR0[27:24]. Guessing context of ID_DFR0 using "cpu_class" doesn't look like optimal way.

Indeed, but what you propose (remove cpu_class check for ARMv7), was done in the first version of this patch - briefly rejected.