acpi: Don't attach a "wake" sysctl node to devices without the ACPI flags IVAR

[jhb]

Summary

Not all bus drivers for ACPI-aware devices implement the ACPI flags
IVAR used by the acpi_wake_set_sysctl handler. In some cases this may
be a feature as some new-bus devices share the same ACPI handle (e.g.
a pcibX device and its child pciY device) which can lead to confusing
results (e.g. setting the sysctl on pciY changes the behavior of the
parent pcibX device, but the "wake" sysctl for pcibX won't reflect the
new behavior, or reflect the device's state).

[ngie]

Oof! I'll give this diff a shot on my MBP!

[ngie]

My MacBookPro13,1 no longer panics when I run sysctl dev.pci..

@pho : would it make sense to make this a regression test?

[ngie]

Oddly enough, this may have also fixed some of the sysctl issues with asmc(4) on my MBP13,1 as asmc(4) is an acpi(4) consumer. It was previously complaining about bogus duplicate OIDs, but I figured that was something that I did on the branch, not an issue with acpi(4).
I'll check on my MBP16,1 as well and report back.
CC: @guest-seuros

[ngie]

In D55562#1271457, @ngie wrote:

Oddly enough, this may have also fixed some of the sysctl issues with asmc(4) on my MBP13,1 as asmc(4) is an acpi(4) consumer. It was previously complaining about bogus duplicate OIDs, but I figured that was something that I did on the branch, not an issue with acpi(4).
I'll check on my MBP16,1 as well and report back.
CC: @guest-seuros

Nevermind. The duplicate temp sensor sysctl(9) OID messages are there still (still poking around trying to fix that on my branch).

[pho]

In D55562#1271443, @ngie wrote:

My MacBookPro13,1 no longer panics when I run sysctl dev.pci..

@pho : would it make sense to make this a regression test?

There is already a "sysctl -a" scenario that triggers the panic:

root@mercat1:~ # cd /usr/src/tools/test/stress2/misc
root@mercat1:/usr/src/tools/test/stress2/misc # ./sysctl2.sh
panic: acpi_get_flags failed for pci8 on bus pcib8, error = 2

This change fixes the panic for me.

[ngie]

In D55562#1271486, @pho wrote:

In D55562#1271443, @ngie wrote:

My MacBookPro13,1 no longer panics when I run sysctl dev.pci..

@pho : would it make sense to make this a regression test?

There is already a "sysctl -a" scenario that triggers the panic:

root@mercat1:~ # cd /usr/src/tools/test/stress2/misc
root@mercat1:/usr/src/tools/test/stress2/misc # ./sysctl2.sh
panic: acpi_get_flags failed for pci8 on bus pcib8, error = 2

This change fixes the panic for me.

Cool -- thanks for verifying!

[ngie]

Hmm... atopcase(4) still panicked with a similar issue (once I figured out that I needed to add ig4 to my KERNCONF). This may have also occurred because my Mac had low battery and the hardware sends out events at the hardware level to OSes to suspend the host (there's always a chance that this is a bug with atopcase(4)).

[obiwac]

In D55562#1271617, @ngie wrote:

Hmm... atopcase(4) still panicked with a similar issue (once I figured out that I needed to add ig4 to my KERNCONF). This may have also occurred because my Mac had low battery and the hardware sends out events at the hardware level to OSes to suspend the host (there's always a chance that this is a bug with atopcase(4)).

This is probably because of the acpi_get_flags() call in acpi_wake_{sleep,run}_prep(). We should be checking if we have ACPI_IVAR_FLAGS in those functions too, or be doing something like D55609.

I think the former is the better solution, because otherwise we're still creating the sysctls even if the driver doesn't support this ivar.

[jhb]

In D55562#1273431, @obiwac wrote:

In D55562#1271617, @ngie wrote:

Hmm... atopcase(4) still panicked with a similar issue (once I figured out that I needed to add ig4 to my KERNCONF). This may have also occurred because my Mac had low battery and the hardware sends out events at the hardware level to OSes to suspend the host (there's always a chance that this is a bug with atopcase(4)).

This is probably because of the acpi_get_flags() call in acpi_wake_{sleep,run}_prep(). We should be checking if we have ACPI_IVAR_FLAGS in those functions too, or be doing something like D55609.

I think the former is the better solution, because otherwise we're still creating the sysctls even if the driver doesn't support this ivar.

See earlier in the review stack. The first commit is supposed to fix the panic by making the IVAR global. The later parts of this stack are refinements to fix an oddity in how this sysctl worked on PCI buses (where we duplicated it).

[obiwac]

ah, right, sorry, I didn't see there were other revisions