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

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

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

byuu
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!

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

Conrad Meyer-2
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 <[hidden email]> 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!
>
> _______________________________________________
> [hidden email] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "[hidden email]"
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

byuu
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.

Some notes:

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);

Dec 9, 2018, 3:35 AM by [hidden email]:

> 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 <> [hidden email] <mailto:[hidden email]>> > 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!
>>
>> _______________________________________________
>> [hidden email] <mailto:[hidden email]>>>  mailing list
>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers <https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>
>> To unsubscribe, send any mail to ">> [hidden email] <mailto:[hidden email]>>> "
>>

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

byuu
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>

Dec 10, 2018, 10:05 PM by [hidden email]:

> 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.
>
> Some notes:
>
> 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);
>
> Dec 9, 2018, 3:35 AM by > [hidden email] <mailto:[hidden email]>> :
>
>> 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 <>> [hidden email] <mailto:[hidden email]>>> > 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!
>>>
>>> _______________________________________________
>>> [hidden email] <mailto:[hidden email]>>>>  mailing list
>>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers <https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>
>>> To unsubscribe, send any mail to ">>> [hidden email] <mailto:[hidden email]>>>> "
>>>
>
>

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

Jason Wolfe-4
In reply to this post by byuu
Hey,

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

Jason

On 2018-12-10 06:05, [hidden email] wrote:

> 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.
>
> Some notes:
>
> 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);
>
> Dec 9, 2018, 3:35 AM by [hidden email]:
>
>> 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 <> [hidden email]
>> <mailto:[hidden email]>> > 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!
>>>
>>> _______________________________________________
>>> [hidden email] <mailto:[hidden email]>>>  
>>> mailing list
>>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers 
>>> <https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>
>>> To unsubscribe, send any mail to ">>
>>> [hidden email]
>>> <mailto:[hidden email]>>> "
>>>
>
> _______________________________________________
> [hidden email] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to
> "[hidden email]"
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

Conrad Meyer-2
In reply to this post by byuu
On Mon, Dec 10, 2018 at 5:06 AM <[hidden email]> wrote:
>
> 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
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

byuu
In reply to this post by byuu
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 ^-^;

Dec 11, 2018, 6:31 AM by [hidden email]:

> On Mon, Dec 10, 2018 at 5:06 AM <> [hidden email] <mailto:[hidden email]>> > wrote:
>
>>
>> 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
> _______________________________________________
> [hidden email] <mailto:[hidden email]>>  mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers <https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>
> To unsubscribe, send any mail to "> [hidden email] <mailto:[hidden email]>> "
>

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

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

Dec 11, 2018, 8:47 AM by [hidden email]:

> 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 ^-^;
>
> Dec 11, 2018, 6:31 AM by > [hidden email] <mailto:[hidden email]>> :
>
>> On Mon, Dec 10, 2018 at 5:06 AM <>> [hidden email] <mailto:[hidden email]>>> > wrote:
>>
>>>
>>> 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
>> _______________________________________________
>> [hidden email] <mailto:[hidden email]>>>  mailing list
>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers <https://lists.freebsd.org/mailman/listinfo/freebsd-hackers>
>> To unsubscribe, send any mail to ">> [hidden email] <mailto:[hidden email]>>> "
>>
>
>

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

Conrad Meyer-2
In reply to this post by byuu
On Mon, Dec 10, 2018 at 3:56 PM <[hidden email]> wrote:
>
> 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!

> 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.

> 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
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

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

byuu
In reply to this post by byuu


> Hm, it should.  Oh well!
>
The tutorial I read said to use "Code Review" on the left, which wasn't there for me =/

>> 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.

> 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! ^-^;


_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[hidden email]"