acpi/amd64: fix attach of wakeup handler

[royger]

Summary

If the initialization of the ACPI device failed (acpi_dev) the call to
bus_generic_attach will still return success, but calling
acpi_install_wakeup_handler will panic the system because the softc
of the acpi_dev has been freed:

Fatal trap 12: page fault while in kernel mode
cpuid = 0; apic id = 00
fault virtual address = 0x60
fault code = supervisor write data, page not present
instruction pointer = 0x20:0xffffffff8109e61c
stack pointer = 0x28:0xffffffff822d79c0
frame pointer = 0x28:0xffffffff822d7a30
code segment = base 0x0, limit 0xfffff, type 0x1b

			= DPL 0, pres 1, long 1, def32 0, gran 1

processor eflags = interrupt enabled, resume, IOPL = 0
current process = 0 (swapper)
[ thread pid 0 tid 100000 ]
Stopped at acpi_install_wakeup_handler+0x1dc: movq %rdi,ll+0x3f(%r14)

Fix this by attaching the wakeup handler in acpi_attach itself for
amd64.

Sponsored by: Citrix Systems R&D

[royger]

I've been thinking about this patch, is there any reason the wakeup memory chunk is not allocated in acpi_attach?

I will update the patch to do that.

[royger]

Move acpi_install_wakeup_handler call.

[royger]

Use "ifdef" instead of "if defined"

[jkim]

In D14400#302522, @royger wrote:

I've been thinking about this patch, is there any reason the wakeup memory chunk is not allocated in acpi_attach?

Yes. We try our best not to add MD-code in sys/dev/acpica/acpi.c.

[jhb]

i386 calls acpi_install_wakeup_handler() in the existing MD attach hook acpi_machdep_init() earlier in acpi_machdep.c. amd64 should probably follow the same approach. Hmm, it seems amd64 intentionally stopped doing that in r197863, but it's not clear to me why that change was made. The difference seems to be whether the memory is allocated before or after all of the devices (not just acpi0) are probed and attached. If r197863 could be reverted I think that is the cleanest approach.

[jkim]

In D14400#302932, @jhb wrote:

i386 calls acpi_install_wakeup_handler() in the existing MD attach hook acpi_machdep_init() earlier in acpi_machdep.c. amd64 should probably follow the same approach. Hmm, it seems amd64 intentionally stopped doing that in r197863, but it's not clear to me why that change was made. The difference seems to be whether the memory is allocated before or after all of the devices (not just acpi0) are probed and attached. If r197863 could be reverted I think that is the cleanest approach.

I am not 100% sure why I did it but I *think* it was not possible at the time, probably because amd64 and i386 had too many differences. I believe it's okay now. Please try this patch.

acpi.diff996 BDownload

[royger]

In D14400#303264, @jkim wrote:

In D14400#302932, @jhb wrote:

i386 calls acpi_install_wakeup_handler() in the existing MD attach hook acpi_machdep_init() earlier in acpi_machdep.c. amd64 should probably follow the same approach. Hmm, it seems amd64 intentionally stopped doing that in r197863, but it's not clear to me why that change was made. The difference seems to be whether the memory is allocated before or after all of the devices (not just acpi0) are probed and attached. If r197863 could be reverted I think that is the cleanest approach.

I am not 100% sure why I did it but I *think* it was not possible at the time, probably because amd64 and i386 had too many differences. I believe it's okay now. Please try this patch.

acpi.diff996 BDownload

That's LGTM, feel free to add my reviewed by and tested by tags. I've also realised that i386 and amd64 acpi_machdep.c are almost identical, any chance of unifying them under the x86 umbrella?

Thanks, Roger.

[jkim]

In D14400#303756, @royger wrote:

That's LGTM, feel free to add my reviewed by and tested by tags.

Committed, thanks!

I've also realised that i386 and amd64 acpi_machdep.c are almost identical, any chance of unifying them under the x86 umbrella?

I cannot test i386 ATM. If you can, please feel free.

[jhb]

This should probably be abandoned now?