Allow cpu-v6.h to work on Cortex-A8 with SMP

[andrew]

Summary

Fix the static inline functions in cpu-v6.h to only use the IS variants
when SMP is enabled. This allow these to be used with Cortex-A8

Test Plan

Boot

[skra]

I understand the reason for this change, however, do we really want to run SMP kernel on UP boards? Maybe, __ARM_ARCH >= 7 && defined SMP test is only some obscure way how to eliminate boards which have not implemented SMP variants of cp15 operations like cortex A8?

sys/arm/include/cpu-v6.h

429 ↗

(On Diff #20863)

Could same solution be used like in tlb_flush_range() to not test mp_ncpus repeatedly in a loop?

484 ↗

(On Diff #20863)

Same note like in icache_sync().

[mmel]

I'm sorry but I don't think that this is right way.
From local point of view:

• mp_ncpus is not right variable for testing presence of multiprocessing extension. I have system where first CPU core is used for FreeBSD, second one is dedicated for application computing (without kernel), but SMP coherency I still required. I think that MPIDR[31:30] register is much better for this job.
• I really don’t want to slowdown each single kernel by numerous “if (SMP)”. If you want to go by this way, then please use proper #ifdefs.
• and, as Svata said, inserting immutable ‘if’s into inner loop is not optimal.

From global point of view I think that single generic kernel (UP and SMP) is in ARM world pure nonsense.

• the speed difference between for compiled with -march=armv7a and -mcpu=cortex-a15 is more over 10% for computing intensive user mode program (I have not benchmarks for kernel yet, sorry). Mainly because HW divider is not present in all Cortex cores and because dual issue instruction scheduling.
• there are numerous other places that must be fixed – page table attributes, TTBR…

But what makes sense for me is to have a “generic” kernel for each particular CPU type (A5/A6/A7/A9/A15/A17) or, at least for each “feature compatible” pair (A15+A7, …)

Again, I'm sorry for this type of negativism...

[manu]

Having a GENERIC kernel would be easier for image and re@ work.
Once pkgbase is there nothing is preventing us to provide GENERIC-CORTEXA7, GENERIC-CORTEXA8 etc ...
But having all board booting from the same image (minus the U-Boot) part will be better for everyone.

[skra]

Here, we face the fact that various Cortex-A arm cores have various features. And even if I understand the reason for GENERIC, I don't like penalization of either custom or native kernel configurations just because of it. So, here we would need three functions set - one for UP, one for SMP, and one for GENERIC.

And I would like to remind that GENERIC will always be optimized for the worst arm core supported by it. So, some features would be unavailable if run on better arm cores.

[mmel]

In D8092#167970, @manu wrote:

Having a GENERIC kernel would be easier for image and re@ work.

I understand. But from user point of view we gets:

1. Longer boot times (because size of kernel image)
2. Suboptimal GENERIC kernel performance.
3. Slowdown of each specialized kernel.

I'm not able to understand motivation for 1) and 2) but, surely, I can live with this.
But I'm not able to accept 3), in any case.

Once pkgbase is there nothing is preventing us to provide GENERIC-CORTEXA7, GENERIC-CORTEXA8 etc ...
But having all board booting from the same image (minus the U-Boot) part will be better for everyone.

It's nice idea, but I'm not sure if this is possible at all.

[andrew]

Use the mpide multiprocess extensions bit to check when to use them

[skra]

I suggest to create ARM_GENERIC_KERNEL option which could be used in situation like here when something bizarre is implemented.

It brings two things in this case.
(1) Nobody won't wonder why it's needed to test mp extension feature again and again in SMP case.
(2) The mp extension feature test will apply only if GENERIC kernel is used. And it will be clear.

It makes code clean, self-explanatory, and useless things won't be done in case of not-GENERIC kernels.

sys/arm/arm/cpuinfo.c

135 ↗

(On Diff #20956)

KASSERT(cpuinfo.mp_ext != 0, ...) if SMP is defined but not ARM_GENERIC_KERNEL.

sys/arm/include/cpu-v6.h

347 ↗

(On Diff #20956)

I think that SMP is not supported for __ARM_ARCH == 6 as hardware TLB maintenance broadcasting is needed. If so, CTASSERT(__ARM_ARCH >= 7, ...) if SMP is defined - with explaining comment. Here, test SMP only.

This __ARM_ARCH >= 7 && defined SMP test may be used in more places, so if I'm correct, it may be replaced by defined SMP everywhere then.

352 ↗

(On Diff #20956)

Here and on other places below, test cpuinfo.mp_ext only if ARM_GENERIC_KERNEL is defined.

[andrew]

You're confusing two things: the SMP option, and the MP extensions. The former just tells the kernel it should run on the all CPUs it can (with a few restrictions), the latter is the HW support needed for running on multiple CPUs.

There is no reason to artificially restrict running an SMP kernel on a Cortex-A8 just because it doesn't have the MP extensions, just as we should be able to run a non-SMP kernel on later hardware with the MP extensions.

I have created D8138 to start to split these out from each other, but it still needs us to fix any incorrect checks on SMP.

[andrew]

Use CPU_CORTEXA8 to determine when to check for the MP extensions

[skra]

In D8092#168719, @andrew wrote:

You're confusing two things: the SMP option, and the MP extensions. The former just tells the kernel it should run on the all CPUs it can (with a few restrictions), the latter is the HW support needed for running on multiple CPUs.

There is no reason to artificially restrict running an SMP kernel on a Cortex-A8 just because it doesn't have the MP extensions, just as we should be able to run a non-SMP kernel on later hardware with the MP extensions.

I was just strict about that. However, you are right. It should be rather a warning (not KASSERT) that the kernel is built with SMP option but run on HW without MP extension. Note that I suggested to exclude GENERIC kernel from that check. Nevertheless, it's not related to your change. It was noted just as an extra idea.

I have created D8138 to start to split these out from each other, but it still needs us to fix any incorrect checks on SMP.

I'm glad that you have adopted the idea that not-GENERIC kernels should not be penalized by GENERIC one. Even if you did it your way. Thank you.

However, your change looks a little complicated now, when the knowledge about MP extension is presented (CPU_CORTEXA_MP). May I suggest the following pattern which utilizes the knowledge precisely?

#ifdef  CPU_CORTEXA_MP
do A
#else
do B
#endif

I would change CPU_CORTEXA_MP to something more general, but it may be done later.

[skra]

Well, the pattern should be the following to work with GENERIC kernel.

#ifdef CPU_CORTEXA_MP
#define ARM_HAVE_MP_EXTENSIONS 1
#else
#define ARM_HAVE_MP_EXTENSIONS 0
#endif



#if ARM_HAVE_MP_EXTENSIONS == 0
if (cpuinfo.mp_ext == 0)
    do not-MP-ops
else
#endif
    do MP-ops

Note that ARM_HAVE_MP_EXTENSIONS may be defined in kernel configurations.

sys/arm/include/cpu-v6.h

365 ↗

(On Diff #21024)

How does it work in GENERIC kernel case?

[skra]

Hmm, if ARM_HAVE_MP_EXTENSIONS will be a hint defined in kernel configuration file, then the pattern will be the following:

#ifndef ARM_HAVE_MP_EXTENSIONS
if (cpuinfo.mp_ext == 0)
    do not-MP-ops
else
#endif
    do MP-ops

Of course, some run-time check when ARM_HAVE_MP_EXTENSIONS is defined that the hint is correct would be nice in place where cpuinfo.mp_ext is set.