I'd like to submit a kernel patch for a new sysctl to use EFI instead of ACPI for poweroff + reboot

[Conrad Meyer]

Hi,

I think the way you would do this properly is by adding a
'EVENTHANDLER_REGISTER(shutdown_final, efi_shutdown_final, ...,
SHUTDOWN_PRI_LAST - 1);' in efidev/efirt (or '- 2', see below).
Because the priority number is *lower* than acpi's
(SHUTDOWN_PRI_LAST), it will be invoked *before* acpi.  If an EFI
shutdown is inappropriate or the particular howto mode is unsupported
on that system, it can just do return doing nothing; the ACPI
acpi_shutdown_final handler will be called next.

You can see numerous examples of such handlers in various ARM devices
which don't have ACPI (grep for
'EVENTHANDLER_REGISTER(shutdown_final').

One other relevant example on x86 is ipmi shutdown, which like I just
suggested for efi shutdown, registers itself as SHUTDOWN_PRI_LAST - 1
to precede ACPI shutdown.  I guess if IPMI is configured, maybe it
should supersede both EFI and ACPI.  So maybe your patch should bump
the IPMI number to SHUTDOWN_PRI_LAST - 2 and add EFI as - 1.

As hinted above, your efirt_shutdown_final() would take the place of
acpi_shutdown_final() in your callstack above, called from
kern_reboot() -> shutdown_final event.

I hope that helps,
Conrad
On Sat, Dec 8, 2018 at 9:53 AM <byuu at tutanota.com> wrote:
>
> Hello, first time poster here, please go easy on me ^-^;
>
> I would like to submit a patch for inclusion to the FreeBSD kernel, but want to first see if there is a chance it will be accepted before writing it or not.
>
> Currently, /usr/src/sys/dev/efidev/efirt.c contains the efi_reset_system() function, which calls EFI_RESET_WARM. It's exposed in /usr/src/sys/sys/efi.h
>
> I would like to add efi_poweroff_system() to efirt.c, which is identical to efi_reset_system(), but with EFI_RESET_SHUTDOWN as arg1, which we have to add as an enum item to efi.h as well (its value is 2, so it would go directly after EFI_RESET_WARM.)
>
> (These two functions are wrappers to invoke the EFI Runtime Services function ResetSystem with EfiResetWarm and EfiResetShutdown. FreeBSD's loader.eli uses them to implement its reboot and poweroff commands, for example.)
>
> Next, I would like to add a patch to /usr/src/sys/dev/acpica/acpi.c to acpi_shutdown_final to check a new kernel sysctl, not picky about the name, but something like "hw.acpi.efi_shutdown=0", which can be optionally changed to 1 by the users. It could be changed at run-time with no ill effect.
>
> When this option is set to 1, and efi_systbl_phys is not NULL (eg we are running on an EFI system), I would like to invoke efi_poweroff_system() instead of AcpiEnterSleepStatePrep(ACPI_STATE_S5)
>
> This is well after all services have been halted, all disks have been detached, and all USB devices have been detached. acpi_shutdown_final calls AcpiEnterSleepStatePrep (_PTS), followed by AcpiEnterSleepState (_S5), and that's it. The idea is to reuse all of the shutdown handling we can.
>
> If it would be desired, I could do the same for reset events to invoke efi_reset_system().
>
> ...
>
> The reason I am requesting this, is because I am the owner of a Threadripper 2950X with an ASRock Taichi X399M motherboard, and when I attempt to shutdown my system from FreeBSD 11.2-RELEASE or 12.0-RC3, the system hangs forever and fails to power down. The call frame for this is as follows:
>
> kern_psignal(initproc, SIGUSR2);
>   kern_reboot
>     shutdown_final
>       acpi_shutdown_final
>         AcpiEnterSleepStatePre
>           AcpiEvaluateObject
>             AcpiNsEvaluate
>               AcpiPsExecuteMethod
>                 AcpiPsParseAml
>                   AcpiPsParseLoop  //from this point on, it likely varies per DSDT
>                     WalkState->AscendingCallback
>                       AcpiDsExecEndOp
>                         AcpiGbl_OpTypeDispatch[OpType]
>                           AcpiExOpcode_1A_0T_0R
>                             AcpiExSystemDoSleep
>                               AcpiOsSleep
>                                 pause("acpislp", 300) (eg tsleep)
>
> I do not know why, but the call to pause never returns. I have tried over a dozen things to find a fix for this: analyzing my DSDT ASL code via acpidump, disabling SMT + all cores, wrapping pause in a critical section, trying SLEEP instead of pause, trying a spinloop instead of pause, trying to skip the AcpiEnterSleepStatePrep call, building a kernel with options ACPI_DEBUG and trying to selectively disable every device driver and ACPI subsystem possible (including USB) while still allowing me to get to a login prompt, tweaking every sysctl I could find around ACPI, USB, debugging, etc. Nothing helped.
>
> Looking into Linux' ACPI support, they have acpi_no_s5, which they claim is used for certain motherboards which lack an _S5 DSDT entry.
>
> I believe my proposed patch would be useful both for the case of a missing _S5 entry, as well as for when there are bugs in FreeBSD, or the motherboard DSDT, or in Intel's ASL parser, or in the hardware itself. Obviously, it's most desirable to fix ACPI on the Threadripper + Taichi X399 platform, but a sysctl as I'm proposing would be beneficial for this and future platforms while users wait for a fix.
>
> If someone with kernel commit access would be interested, I can write all of the code against -CURRENT, and submit a diff patch for review. It would be only a small amount of code, maybe 30 lines or so, with appropriate error checking and fallbacks in place. Again, I'm very new to this, so please bear with me.
>
> Personally, for now, I just monkey patched it into acpica.c and confirmed that the concept works.
>
> If there's no interest, then I will drop the matter.
>
> Thank you everyone for your time!
>
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"