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

Discussion:

b***@tutanota.com

2018-12-08 17:52:01 UTC

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!

Conrad Meyer

2018-12-08 18:35:31 UTC

Permalink

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

Post by b***@tutanota.com
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().
...
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!
_______________________________________________
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers

Jason Wolfe

2018-12-10 19:38:31 UTC

Permalink

Hey,

https://wiki.freebsd.org/Phabricator is a good guide on how to get your
patch some visibility and hopefully ushered in.

Jason

Thank you, this information helped a lot!
I am not sure where I should submit my patch to ... I'll post it here
if that's okay. If it needs to go somewhere else, please provide
instructions or if you don't mind ... submit it on my behalf ^^;
This patch has been created against -CURRENT, and has been tested
successfully on 12.0-RC3. It allows me to power off my ASRock X399M
(mATX) motherboard, and should also work for ASRock X399 (ATX) users
who have reported the same problem of ACPI hanging during shutdown.
Hopefully it will be useful to other users in the future who
experience ACPI shutdown issues as a fallback option.
I relinquish the entire copyright to the FreeBSD project. It's public
domain, no credit is necessary.
As requested, I moved IPMI to PRI-2 instead of PRI-1. I then placed
EFI shutdown at PRI-1.
In the spirit of DRY, I modified efi_reset_system(void) to
efi_reset_system(int type). I did a grep -rn . search on -CURRENT, and
absolutely nothing ever uses efi_reset_system(void), nor is it exposed
via the /dev/efi ioctl interface. I hope this is okay.
I named the sysctl tunable "hw.efi.poweroff", after the similarly
named "hw.acpi." set of tunables, and the use of "poweroff" over
"shutdown" in various other tunables. Please feel free to rename the
tunable to anything you like.
I think it would be nice if we exposed this tunable to the "sysctl"
tool, and allowed modifying it at run-time. I didn't want to
complicate the patch, but if you're okay with that, please add that as
well. However, if you're worried about people mucking with the value
inadvertently, I'm okay if it remains a "secret" /boot/loader.conf
option only.
Lastly, I added the needed event handler headers, and added the new
EFI_RESET_SHUTDOWN define.
I tried to respect all formatting conventions of the existing code.
But as with everything else, please feel free to make any changes you
like.
If I can be of any further assistance on this, please let me know.
Thank you very much!
diff -ru old/sys/dev/efidev/efirt.c new/sys/dev/efidev/efirt.c
--- old/sys/dev/efidev/efirt.c 2018-12-10 04:32:01.231607000 +0900
+++ new/sys/dev/efidev/efirt.c 2018-12-10 04:45:29.548147000 +0900
@@ -46,6 +46,8 @@
#include <sys/sysctl.h>
#include <sys/systm.h>
#include <sys/vmmeter.h>
+#include <sys/eventhandler.h>
+#include <sys/reboot.h>
#include <machine/fpu.h>
#include <machine/efi.h>
@@ -126,6 +128,24 @@
return (false);
}
+static void
+efi_shutdown_final(void *unused, int howto)
+{
+ int efi_poweroff;
+
+ /*
+ * On some systems, ACPI S5 is missing or does not function properly.
+ * Allow shutdown via EFI Runtime Services instead with a sysctl
tunable.
+ */
+ if((howto & RB_POWEROFF) != 0) {
+ efi_poweroff = 0;
+ TUNABLE_INT_FETCH("hw.efi.poweroff", &efi_poweroff);
+ if (efi_poweroff == 1) {
+ efi_reset_system(EFI_RESET_SHUTDOWN);
+ }
+ }
+}
+
static int
efi_init(void)
{
@@ -214,6 +234,13 @@
}
#endif
+ /*
+ * We use SHUTDOWN_PRI_LAST - 1 to trigger after IPMI, but before
ACPI.
+ * When not enabled via sysctl tunable, this will fall through to
ACPI.
+ */
+ EVENTHANDLER_REGISTER(shutdown_final, efi_shutdown_final, NULL,
+ SHUTDOWN_PRI_LAST - 1);
+
return (0);
}
@@ -411,16 +438,20 @@
}
int
-efi_reset_system(void)
+efi_reset_system(int type)
{
struct efirt_callinfo ec;
+ if (type != EFI_RESET_COLD
+ && type != EFI_RESET_WARM
+ && type != EFI_RESET_SHUTDOWN)
+ return (EINVAL);
if (efi_runtime == NULL)
return (ENXIO);
bzero(&ec, sizeof(ec));
ec.ec_name = "rt_reset";
ec.ec_argcnt = 4;
- ec.ec_arg1 = (uintptr_t)EFI_RESET_WARM;
+ ec.ec_arg1 = (uintptr_t)type;
ec.ec_arg2 = (uintptr_t)0;
ec.ec_arg3 = (uintptr_t)0;
ec.ec_arg4 = (uintptr_t)NULL;
diff -ru old/sys/dev/ipmi/ipmi.c new/sys/dev/ipmi/ipmi.c
--- old/sys/dev/ipmi/ipmi.c 2018-12-10 04:34:07.535522000 +0900
+++ new/sys/dev/ipmi/ipmi.c 2018-12-10 04:36:35.757611000 +0900
@@ -938,14 +938,14 @@
} else if (!on)
(void)ipmi_set_watchdog(sc, 0);
/*
- * Power cycle the system off using IPMI. We use last - 1 since we
don't
+ * Power cycle the system off using IPMI. We use last - 2 since we
don't
* handle all the other kinds of reboots. We'll let others handle them.
* We only try to do this if the BMC supports the Chassis device.
*/
if (sc->ipmi_dev_support & IPMI_ADS_CHASSIS) {
device_printf(dev, "Establishing power cycle handler\n");
sc->ipmi_power_cycle_tag = EVENTHANDLER_REGISTER(shutdown_final,
-     ipmi_power_cycle, sc, SHUTDOWN_PRI_LAST - 1);
+     ipmi_power_cycle, sc, SHUTDOWN_PRI_LAST - 2);
}
}
diff -ru old/sys/sys/efi.h new/sys/sys/efi.h
--- old/sys/sys/efi.h 2018-12-10 04:32:34.850892000 +0900
+++ new/sys/sys/efi.h 2018-12-10 04:35:41.011396000 +0900
@@ -43,7 +43,8 @@
enum efi_reset {
EFI_RESET_COLD,
- EFI_RESET_WARM
+ EFI_RESET_WARM,
+ EFI_RESET_SHUTDOWN
};
typedef uint16_t efi_char;
@@ -184,7 +185,7 @@
int efi_get_table(struct uuid *uuid, void **ptr);
int efi_get_time(struct efi_tm *tm);
int efi_get_time_capabilities(struct efi_tmcap *tmcap);
-int efi_reset_system(void);
+int efi_reset_system(int type);
int efi_set_time(struct efi_tm *tm);
int efi_var_get(uint16_t *name, struct uuid *vendor, uint32_t *attrib,
     size_t *datasize, void *data);

Post by 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

Post by b***@tutanota.com
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
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!
_______________________________________________
mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
<https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>
To unsubscribe, send any mail to ">>

_______________________________________________
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to

b***@tutanota.com

2018-12-10 13:12:06 UTC

Permalink

In case the formatting was damaged by posting it through the mailing list ... here's another copy of the patch:

https://pastebin.com/raw/US5VbZGp <https://pastebin.com/raw/US5VbZGp>

Thank you, this information helped a lot!
I am not sure where I should submit my patch to ... I'll post it here if that's okay. If it needs to go somewhere else, please provide instructions or if you don't mind ... submit it on my behalf ^^;
This patch has been created against -CURRENT, and has been tested successfully on 12.0-RC3. It allows me to power off my ASRock X399M (mATX) motherboard, and should also work for ASRock X399 (ATX) users who have reported the same problem of ACPI hanging during shutdown. Hopefully it will be useful to other users in the future who experience ACPI shutdown issues as a fallback option.
I relinquish the entire copyright to the FreeBSD project. It's public domain, no credit is necessary.
As requested, I moved IPMI to PRI-2 instead of PRI-1. I then placed EFI shutdown at PRI-1.
In the spirit of DRY, I modified efi_reset_system(void) to efi_reset_system(int type). I did a grep -rn . search on -CURRENT, and absolutely nothing ever uses efi_reset_system(void), nor is it exposed via the /dev/efi ioctl interface. I hope this is okay.
I named the sysctl tunable "hw.efi.poweroff", after the similarly named "hw.acpi." set of tunables, and the use of "poweroff" over "shutdown" in various other tunables. Please feel free to rename the tunable to anything you like.
I think it would be nice if we exposed this tunable to the "sysctl" tool, and allowed modifying it at run-time. I didn't want to complicate the patch, but if you're okay with that, please add that as well. However, if you're worried about people mucking with the value inadvertently, I'm okay if it remains a "secret" /boot/loader.conf option only.
Lastly, I added the needed event handler headers, and added the new EFI_RESET_SHUTDOWN define.
I tried to respect all formatting conventions of the existing code. But as with everything else, please feel free to make any changes you like.
If I can be of any further assistance on this, please let me know. Thank you very much!
diff -ru old/sys/dev/efidev/efirt.c new/sys/dev/efidev/efirt.c
--- old/sys/dev/efidev/efirt.c 2018-12-10 04:32:01.231607000 +0900
+++ new/sys/dev/efidev/efirt.c 2018-12-10 04:45:29.548147000 +0900
@@ -46,6 +46,8 @@
#include <sys/sysctl.h>
#include <sys/systm.h>
#include <sys/vmmeter.h>
+#include <sys/eventhandler.h>
+#include <sys/reboot.h>
#include <machine/fpu.h>
#include <machine/efi.h>
@@ -126,6 +128,24 @@
return (false);
}
+static void
+efi_shutdown_final(void *unused, int howto)
+{
+ int efi_poweroff;
+
+ /*
+ * On some systems, ACPI S5 is missing or does not function properly.
+ * Allow shutdown via EFI Runtime Services instead with a sysctl tunable.
+ */
+ if((howto & RB_POWEROFF) != 0) {
+ efi_poweroff = 0;
+ TUNABLE_INT_FETCH("hw.efi.poweroff", &efi_poweroff);
+ if (efi_poweroff == 1) {
+ efi_reset_system(EFI_RESET_SHUTDOWN);
+ }
+ }
+}
+
static int
efi_init(void)
{
@@ -214,6 +234,13 @@
}
#endif
+ /*
+ * We use SHUTDOWN_PRI_LAST - 1 to trigger after IPMI, but before ACPI.
+ * When not enabled via sysctl tunable, this will fall through to ACPI.
+ */
+ EVENTHANDLER_REGISTER(shutdown_final, efi_shutdown_final, NULL,
+ SHUTDOWN_PRI_LAST - 1);
+
return (0);
}
@@ -411,16 +438,20 @@
}
int
-efi_reset_system(void)
+efi_reset_system(int type)
{
struct efirt_callinfo ec;
+ if (type != EFI_RESET_COLD
+ && type != EFI_RESET_WARM
+ && type != EFI_RESET_SHUTDOWN)
+ return (EINVAL);
if (efi_runtime == NULL)
return (ENXIO);
bzero(&ec, sizeof(ec));
ec.ec_name = "rt_reset";
ec.ec_argcnt = 4;
- ec.ec_arg1 = (uintptr_t)EFI_RESET_WARM;
+ ec.ec_arg1 = (uintptr_t)type;
ec.ec_arg2 = (uintptr_t)0;
ec.ec_arg3 = (uintptr_t)0;
ec.ec_arg4 = (uintptr_t)NULL;
diff -ru old/sys/dev/ipmi/ipmi.c new/sys/dev/ipmi/ipmi.c
--- old/sys/dev/ipmi/ipmi.c 2018-12-10 04:34:07.535522000 +0900
+++ new/sys/dev/ipmi/ipmi.c 2018-12-10 04:36:35.757611000 +0900
@@ -938,14 +938,14 @@
} else if (!on)
(void)ipmi_set_watchdog(sc, 0);
/*
- * Power cycle the system off using IPMI. We use last - 1 since we don't
+ * Power cycle the system off using IPMI. We use last - 2 since we don't
* handle all the other kinds of reboots. We'll let others handle them.
* We only try to do this if the BMC supports the Chassis device.
*/
if (sc->ipmi_dev_support & IPMI_ADS_CHASSIS) {
device_printf(dev, "Establishing power cycle handler\n");
sc->ipmi_power_cycle_tag = EVENTHANDLER_REGISTER(shutdown_final,
-     ipmi_power_cycle, sc, SHUTDOWN_PRI_LAST - 1);
+     ipmi_power_cycle, sc, SHUTDOWN_PRI_LAST - 2);
}
}
diff -ru old/sys/sys/efi.h new/sys/sys/efi.h
--- old/sys/sys/efi.h 2018-12-10 04:32:34.850892000 +0900
+++ new/sys/sys/efi.h 2018-12-10 04:35:41.011396000 +0900
@@ -43,7 +43,8 @@
enum efi_reset {
EFI_RESET_COLD,
- EFI_RESET_WARM
+ EFI_RESET_WARM,
+ EFI_RESET_SHUTDOWN
};
typedef uint16_t efi_char;
@@ -184,7 +185,7 @@
int efi_get_table(struct uuid *uuid, void **ptr);
int efi_get_time(struct efi_tm *tm);
int efi_get_time_capabilities(struct efi_tmcap *tmcap);
-int efi_reset_system(void);
+int efi_reset_system(int type);
int efi_set_time(struct efi_tm *tm);
int efi_var_get(uint16_t *name, struct uuid *vendor, uint32_t *attrib,
     size_t *datasize, void *data);

Conrad Meyer

2018-12-10 21:31:45 UTC

Permalink

Thank you, this information helped a lot!

I am glad to hear it.

I am not sure where I should submit my patch to ... I'll post it here if that's okay. If it needs to go somewhere else, please provide instructions or if you don't mind ... submit it on my behalf ^^;

I'll second Jason's request — please go ahead and submit a diff to
Phabricator (reviews.freebsd.org), if you don't mind.

This patch has been created against -CURRENT, and has been tested successfully on 12.0-RC3. It allows me to power off my ASRock X399M (mATX) motherboard, and should also work for ASRock X399 (ATX) users who have reported the same problem of ACPI hanging during shutdown. Hopefully it will be useful to other users in the future who experience ACPI shutdown issues as a fallback option.

Hm, I don't recall my ASRock X399 board having issues shutting down with ACPI.

Thanks,
Conrad

b***@tutanota.com

2018-12-10 23:47:41 UTC

Permalink

I made an account on reviews.freebsd.org, but it won't allow me to submit a patch via the website.

Tutorials online want me to install a bunch of software, install certificates, configure Git, and ... I'm very sorry, but I don't have the patience just for a quick patch. No hard feelings if you can't use it like this.

As far as the bug not occurring on your X399 ... very interesting. Are you using a Threadripper as well? This user was having the same issue on the X399 as I am on my X399M: https://forums.freenas.org/index.php?threads/amd-threadripper-build.70194/#post-485064 <https://forums.freenas.org/index.php?threads/amd-threadripper-build.70194/#post-485064>

I've exhausted all latest version BIOS settings, toggled lots of sysctl tunables, etc to no avail. Not a big deal really, just had nothing better to work on while waiting for 12-RELEASE ^-^;

Post by Conrad Meyer

Thank you, this information helped a lot!

I am glad to hear it.

I am not sure where I should submit my patch to ... I'll post it here if that's okay. If it needs to go somewhere else, please provide instructions or if you don't mind ... submit it on my behalf ^^;

I'll second Jason's request — please go ahead and submit a diff to
Phabricator (reviews.freebsd.org), if you don't mind.

Hm, I don't recall my ASRock X399 board having issues shutting down with ACPI.
Thanks,
Conrad
_______________________________________________
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers <https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>

b***@tutanota.com

2018-12-10 23:49:49 UTC

Permalink

Sorry, I meant to say, "Are you using a Threadripper 2 as well?", of course it's a TR4 socket board ... sigh.

Post by b***@tutanota.com
I made an account on reviews.freebsd.org, but it won't allow me to submit a patch via the website.
Tutorials online want me to install a bunch of software, install certificates, configure Git, and ... I'm very sorry, but I don't have the patience just for a quick patch. No hard feelings if you can't use it like this.
As far as the bug not occurring on your X399 ... very interesting. Are you using a Threadripper as well? This user was having the same issue on the X399 as I am on my X399M: > https://forums.freenas.org/index.php?threads/amd-threadripper-build.70194/#post-485064 <https://forums.freenas.org/index.php?threads/amd-threadripper-build.70194/#post-485064>
I've exhausted all latest version BIOS settings, toggled lots of sysctl tunables, etc to no avail. Not a big deal really, just had nothing better to work on while waiting for 12-RELEASE ^-^;

Post by Conrad Meyer

Thank you, this information helped a lot!

I am glad to hear it.

I am not sure where I should submit my patch to ... I'll post it here if that's okay. If it needs to go somewhere else, please provide instructions or if you don't mind ... submit it on my behalf ^^;

I'll second Jason's request — please go ahead and submit a diff to
Phabricator (reviews.freebsd.org), if you don't mind.

Conrad Meyer

2018-12-11 00:44:19 UTC

Permalink

Post by b***@tutanota.com
I made an account on reviews.freebsd.org, but it won't allow me to submit a patch via the website.

Hm, it should. Oh well!

Post by b***@tutanota.com
Tutorials online want me to install a bunch of software, install certificates, configure Git, and ... I'm very sorry, but I don't have the patience just for a quick patch. No hard feelings if you can't use it like this.

No, it's fine. We'll make it work. Thanks.

Post by b***@tutanota.com
As far as the bug not occurring on your X399 ... very interesting. Are you using a Threadripper [2] as well? This user was having the same issue on the X399 as I am on my X399M: https://forums.freenas.org/index.php?threads/amd-threadripper-build.70194/#post-485064
I've exhausted all latest version BIOS settings, toggled lots of sysctl tunables, etc to no avail. Not a big deal really, just had nothing better to work on while waiting for 12-RELEASE ^-^;

First-gen Threadripper (1950x), and newest BIOS version (3.30). I
might be misremembering, though — I rarely shut down this machine.
Usually either reboot or panic -> reboot :-).

Best,
Conrad

b***@tutanota.com

2018-12-11 01:19:37 UTC

Permalink

Post by Conrad Meyer
Hm, it should. Oh well!

The tutorial I read said to use "Code Review" on the left, which wasn't there for me =/

Post by Conrad Meyer

Post by Conrad Meyer
No, it's fine. We'll make it work. Thanks.

That's very generous, thank you! If you're ever in Tokyo, I owe you a beer ;)

I'll learn Phabricator for next time. Apologies again.

Post by Conrad Meyer
First-gen Threadripper (1950x), and newest BIOS version (3.30). I
might be misremembering, though — I rarely shut down this machine.
Usually either reboot or panic -> reboot :-).

Hmm, then it reduces the value of the patch somewhat ... perhaps it's a TR2-only issue? Tried 3.20 and 3.30 here.

Well, I already spent two days on this and have wasted enough of your time, to save myself five seconds on shutting down ... thank you again! ^-^;