Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 8 Dec 2018 10:35:31 -0800
From:      Conrad Meyer <cem@freebsd.org>
To:        byuu@tutanota.com
Cc:        "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>
Subject:   Re: I'd like to submit a kernel patch for a new sysctl to use EFI instead of ACPI for poweroff + reboot
Message-ID:  <CAG6CVpU5dczi16TpbvBrPKn8xSa-z2kGWL-RJ-5=nF1r7BfBsg@mail.gmail.com>
In-Reply-To: <LTDwcvo--3-1@tutanota.com>
References:  <LTDwcvo--3-1@tutanota.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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@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 w=
ant 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/ef=
i.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 t=
o add as an enum item to efi.h as well (its value is 2, so it would go dire=
ctly after EFI_RESET_WARM.)
>
> (These two functions are wrappers to invoke the EFI Runtime Services func=
tion ResetSystem with EfiResetWarm and EfiResetShutdown. FreeBSD's loader.e=
li 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 ac=
pi_shutdown_final to check a new kernel sysctl, not picky about the name, b=
ut something like "hw.acpi.efi_shutdown=3D0", which can be optionally chang=
ed 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() ins=
tead of AcpiEnterSleepStatePrep(ACPI_STATE_S5)
>
> This is well after all services have been halted, all disks have been det=
ached, and all USB devices have been detached. acpi_shutdown_final calls Ac=
piEnterSleepStatePrep (_PTS), followed by AcpiEnterSleepState (_S5), and th=
at'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 ef=
i_reset_system().
>
> ...
>
> The reason I am requesting this, is because I am the owner of a Threadrip=
per 2950X with an ASRock Taichi X399M motherboard, and when I attempt to sh=
utdown my system from FreeBSD 11.2-RELEASE or 12.0-RC3, the system hangs fo=
rever 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 acpi=
dump, disabling SMT + all cores, wrapping pause in a critical section, tryi=
ng SLEEP instead of pause, trying a spinloop instead of pause, trying to sk=
ip the AcpiEnterSleepStatePrep call, building a kernel with options ACPI_DE=
BUG and trying to selectively disable every device driver and ACPI subsyste=
m possible (including USB) while still allowing me to get to a login prompt=
, tweaking every sysctl I could find around ACPI, USB, debugging, etc. Noth=
ing 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 missin=
g _S5 entry, as well as for when there are bugs in FreeBSD, or the motherbo=
ard DSDT, or in Intel's ASL parser, or in the hardware itself. Obviously, i=
t'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 platf=
orms 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 err=
or 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@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org=
"



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpU5dczi16TpbvBrPKn8xSa-z2kGWL-RJ-5=nF1r7BfBsg>