arm64: add a driver for the Apple Interrupt Controller

[kevans]

Summary

Some limited support for later multi-die SoC is included, but not at all
tested and not expected to be functional yet. kevans needs to finish
getting his serial boards constructed, as the beefiest AS machine that
actually has multiple die to support in his fleet is currently a
dedicated serial console.

Co-authored-by: Andrew Turner <[e-mail]>
Co-authored-by: Mike Karels <[e-mail]>

[andrew]

sys/arm64/apple/apple_aic.c

4

You can remove All rights reserved. from my copyright.

488

Won't this always panic?

505–507

Needed?

528

Needed?

544

Needed?

548

Is this used in the non-SMP case? I think you can expand #ifdef SMP to cover the whole function.

628

It would be useful to add a comment explaining why we need to read non-interrupt controller registers.

699

What is the ordering requirement for this? It looks like the above atomic_set needs to be visible to other CPUs before the later write to the IPI register.

[kevans]

Address review feedback

[kevans]

sys/arm64/apple/apple_aic.c

488

Heh, whoops- yeah, I guess we don't disable interrupts in any of the drivers we have in my local branch.

699

Correct; I've added a comment to both.

[kevans]

sys/arm64/apple/apple_aic.c

4

I missed this one, staged locally

[andrew]

sys/arm64/apple/apple_aic.c

699

As mentioned in IRC a wmb() isn't strong enough as the msr instruction in WRITE_SPECIALREG may be moved before it. A dsb(ish) is strong enough as it ensures later instructions that can alter any state of the system are not executed until the dsb completes.

ish can be used as the store to sc->sc_ipimasks[cpu] will be to inner-shareable memory.

[kevans]

Strip "All rights reserved" at Anderw's request, upgrade dmb -> dsb.

The rmb() on the other side is dropped.

[andrew]

sys/arm64/apple/apple_aic.c

31

Don't need this anymore (was only for __FBSDID)

128–130

Empty struct?

[ehem_freebsd_m5p.com]

sys/arm64/apple/apple_aic.c

775–776

No need for the static here. This is mildly troublesome if it needs to be converted to DEFINE_CLASS() or DEFINE_CLASS_1(). The usual pattern is to omit. (I've got a big cleanup commit which does all of these instances)

[kevans]

sys/arm64/apple/apple_aic.c

775–776

What's your argument for giving these things global linkage? I care much less about the usual pattern because the usual pattern isn't always right than I do about the actual motivation.

[ehem_freebsd_m5p.com]

sys/arm64/apple/apple_aic.c

775–776

I am not suggesting these should have global linkage. I am suggesting having static before DEFINE_CLASS_0() is wrong. In particular static DEFINE_CLASS_0() works since it only creates a single structure. static DEFINE_CLASS_1() fails since DEFINE_CLASS_1() creates two structures and since the first structure is explicitly static the compiler sees static static kobj_class_t ....

As these are variables, not functions, unless there is an explicit DECLARE_CLASS() (or an equivalent using extern) somewhere the structures will have file scope, not global scope. Remember this is C, which is "quirky, flawed, and an enormous success". I understand the desire to have them be explicitly local, but it causes trouble.

Perhaps there is a need for STATIC_DEFINE_CLASS_#() macros which explicitly declare the structure static? (right now there is no way to have DEFINE_CLASS_1() be explicitly local)

[kevans]

sys/arm64/apple/apple_aic.c

775–776

None of this seems at all relevant to this change. static DEFINE_CLASS_0 is perfectly fine and makes sense today in this change because this doesn't need global linkage. I'm not randomly changing it just for your theoretical annoyances that aren't an issue with this usage; if you'd like to propose a change that can make sense for these where we don't need global linkage in addition to scenarios where we do, feel free- but that's an entirely separate review/issue.

I'm not using DEFINE_CLASS_1 and don't need the lecture on why that would be wrong; I also have eyes and consider myself a competent C developer. Thanks.

[ehem_freebsd_m5p.com]

sys/arm64/apple/apple_aic.c

775–776

Ah ha, now I see what has been going on. I guess the standards did move to make file-level variables emulate function behavior (no static -> fully global). Otherwise the FreeBSD compiler options trigger that behavior. I was thinking traditional behavior of no extern -> local. My goof, sorry about the noise.

[ehem_freebsd_m5p.com]

Now there is D48449 which proposes {PRIVATE|PUBLIC}_DEFINE_CLASS() to explicitly declare visibility. This also takes care of the issue there is no way to make private classes which have one or more parents.