Add ACPI support for arm64

[andrew]

Summary

This adds basic support for ACPI. It splits out the nexus driver to
two new drivers, one for fdt, one for acpi. It then uses this to decide
if it will use fdt or acpi.

When using acpi a small driver is added to handle the MADT and GTDT. The
former is used to attach the gicv2 driver, the latter the generic timer.
This is needed because both of these need devices to attach to.

A few x86 specific places have been marked as such, and pci support is
missing.

[andrew]

I should also mention that we still need fdt for now to get the kernel console. The uart driver will need a bit of work to have it decide if it should look at acpi or fdt.

[emaste]

sys/arm64/acpica/acpi_arm64.c

1 ↗

(On Diff #5231)

Add a (C) and license block?

sys/arm64/arm64/gic.h

35–36 ↗

(On Diff #5231)

should we make this _MACHINE_GIC_H_?

sys/arm64/arm64/machdep.c

357–361 ↗

(On Diff #5231)

Unrelated (but reasonable) change, commit separately perhaps?

397 ↗

(On Diff #5231)

Do we have a constant for this somewhere or is it usually spelled this way in ACPI code?

sys/arm64/arm64/nexus.c

71–72 ↗

(On Diff #5231)

Do these belong in #ifdef DEV_ACPI guards?
I suspect it doesn't hurt either way, but would expect it to be consistent with #ifdef FDT above

[jkim]

acpi_cpu.c and acpi_throttle.c have "#if defined(i386) || defined(arm64)" and these look very suspicious to me.

[andrew]

Update based on feedback

[andrew]

Missed machdep.c

sys/arm64/arm64/gic.h

35–36 ↗

(On Diff #5231)

I named it _ARM64_GIC_H_ because of its location. I didn't put it into sys/arm64/include as I'd like to move to a common gic driver under sys/dev.

sys/arm64/arm64/machdep.c

357–361 ↗

(On Diff #5231)

It from an earlier change that didn't get fully reverted.

397 ↗

(On Diff #5231)

This is the same on i386 and amd64, we could use UINT_MAX, or something based on it.

sys/arm64/arm64/nexus.c

71–72 ↗

(On Diff #5231)

Yes

[emaste]

sys/arm64/arm64/gic.h

35–36 ↗

(On Diff #5252)

ok

sys/arm64/arm64/machdep.c

397 ↗

(On Diff #5252)

Ok, it's not a magic number in the way that register offsets etc. would be and doesn't bother me too much if i386/amd64 are the same.

[emaste]

This seems reasonable to me with minor comments on two copyright blocks; ideally someone with more direct ACPI knowledge can give it a go-ahead.

sys/arm64/acpica/OsdEnvironment.c

2–3 ↗

(On Diff #5252)

this seems to be new/trivial content?

sys/arm64/arm64/gic.h

6–9 ↗

(On Diff #5252)

does this apply?

[andrew]

sys/arm64/acpica/OsdEnvironment.c

2–3 ↗

(On Diff #5252)

No, it's mostly copied from sys/x86/acpica/OsdEnvironment.c with the removal of acpi_get_root_from_memory, but git is claiming otherwise.

sys/arm64/arm64/gic.h

6–9 ↗

(On Diff #5252)

This was copied from sys/arm/arm/gic.c which has this.

[andrew]

Update to the latest head, and remove the broken git copy logic

[jhb]

sys/arm64/acpica/acpi_arm64.c

80 ↗

(On Diff #5324)

Having the structures and methods be "acpi_arm64" and the device name "arm64_acpi" is a bit confusing, perhaps just pick one?

95 ↗

(On Diff #5324)

Looking at this driver, I feel like this should be ordered a bit differently. That is, I would move the logic to probe the tables into identify routines for the "gic" and timer drivers and let these drivers hang directly off of acpi0. This would remove the arm64_acpi device entirely. The "gic" identify routine would walk the MADT (an identify routine can create multiple devices), and the timer identify routine would walk the GTDT table. (Basically, acpi_arm64_add_gtdt would become the timer's identify routine, and acpi_arm64_add_madt would become "gic"'s identify routine.)

sys/arm64/arm64/nexus.c

328 ↗

(On Diff #5324)

You only need this method if you are using one of the bus_generic_rl_* methods in your driver. They are the only consumer of this. If you aren't using those, you can remove this.

[andrew]

Move the table parsing code to the identify functions

[andrew]

Use an uploaded diff, git is having issues where it thinks files have been copied.

[andrew]

sys/arm64/arm64/nexus.c

328 ↗

(On Diff #5324)

Without this I get:

acpi0: <ARMLTD ARM-JUNO>
acpi0: Power Button (fixed)
acpi0: Sleep Button (fixed)
panic: vm_fault failed: ffffff8000039ae8
Uptime: 1s
Automatic reboot in 15 seconds - press a key on the console to abort

This is because acpi_sysres_alloc calls BUS_GET_RESOURCE_LIST on the nexus device.

[jkim]

Please see r283261:

https://reviews.freebsd.org/rS283261

[jkim]

sys/conf/Makefile.arm64

37 ↗

(On Diff #5527)

This should be moved to acpica_machdep.h.

[emaste]

@andrew is a new version of this coming soon?

[jhb]

sys/arm/arm/generic_timer.c

325 ↗

(On Diff #5527)

One trick you can use here is BUS_PROBE_NOWILDCARD. This will the only attach to devices that are explicitly added with your driver name. I think that means you can remove your other two checks and just have:

device_set_desc(...);
return (BUS_PROBE_NOWILDCARD);

sys/arm64/arm64/gic_acpi.c

149 ↗

(On Diff #5527)

Same BUS_PROBE_NO_WILDCARD suggestion here.

sys/arm64/arm64/nexus.c

328 ↗

(On Diff #5527)

Oh, ugh. Ok then. I don't have a good idea on how to do this better currently, but that's an ACPI issue, not an arm64 one.

[andrew]

Rebase and update based on comments

[jkim]

sys/dev/acpica/acpi_cpu.c

1254 ↗

(On Diff #5916)

It is a gratuitous white space change. Actually, this macro is used without a leading blank line in couple of places in the same file. ;-)

[emaste]

sys/dev/acpica/acpi_cpu.c

1254 ↗

(On Diff #5916)

Per style(9) it's supposed to be there, but in this case I think it's just a result of adding and then removing a change here. Agreed that we should drop this change in the committed version.

[jhb]

This looks fine as far as I can tell.