Expose EFI wake time API
Details
Details
- Reviewers
kib - Commits
- rG1808b5577da8: Add efiwake tool
rG3f3ad56520ac: Expose EFI wake time API
Use the included efiwake tool.
Get the current wake time:
$ /usr/sbin/efiwake Current EFI time: 2023-04-24T15:56:29 EFI wake time: 2023-04-23T22:00:00; enabled=1, pending=0
Set a valid wake time:
$ /usr/sbin/efiwake -e 2023-10-24T22:00:00 Current EFI time: 2023-04-24T16:00:32 EFI wake time: 2023-04-24T22:00:00; enabled=1, pending=0
Set an invalid wake time (31st of Apr does not exist):
$ /usr/sbin/efiwake -e 2023-04-31T22:00:00 Current EFI time: 2023-04-24T16:01:22 efiwake: cannot enable EFI wake time: Invalid argument
Disable wake time:
$ /usr/sbin/efiwake -d Current EFI time: 2023-04-24T16:02:06 EFI wake time: 2023-04-24T16:02:06; enabled=0, pending=0
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
I was unable to test this patch. efi.h patch fails against CURRENT. D28620 abstracted the EFI public interface functions to support Xen VMs.
Comment Actions
The patch looks good.... for an older system. I like the concepts here, and maybe you could rebase to -current?
Comment Actions
hi @imp & @john.grafton_runbox.com, thanks for looking!
this patch was against stable-13. I don't have spare hardware to test on current...
how useful do you think this patch is? it did not actually work for me... i suspect a dodgy bios in the nas box, as it wakes from s5 but not from s3.
Comment Actions
Yeah, I think supporting EFI wake time could be useful to some folks. I've got a box running current I'll test with if you're able to rebase the patch.
Comment Actions
I tested this patch on a Lenovo T480 laptop and a generic AMD desktop running CURRENT. Both reported that they supported EFI wake time.
Unfortunately, I had the same experience you had in 13. Everything *looked* like it should work but neither system powered on or woke from S3. :(
Comment Actions
Hm... no idea how that is supposed to work then...
I suspect that something something ACPI needs to happen.
For example, for my NAS box I have (sysctl):
dev.acpi_sysresource.3.wake: 0
which is the super-io / power management controller (cf D36424).
But messing around with that wake parameter does not seem to do anything.
Comment Actions
I suspect the lack of actual wakeup is related to PM1.RTC_EN bit not set properly. Look up it in the section 4.8.3.1.2 PM1Enable Registers of ACPI spec v. 6.5.
Comment Actions
thanks for the pointer, kib!
spec looks relevant. but i have no idea how to do anything about those flags in the fbsd source code...
Comment Actions
I suspect it is along the lines of
AcpiWriteBitRegister(ACPI_BITREG_RT_CLOCK_ENABLE, 1);
| sys/dev/efidev/efirt.c | ||
|---|---|---|
| 615 | Why not? Of course, it must be covered by the #ifdef DEV_ACPI guards. Also, you need to disable RT_CLOCK bit if the set_waketime got enable == false. | |
Comment Actions
will update patch shortly.
meanwhile, got this in the console after (first) resume:
ACPI Error: No installed handler for fixed event - RealTimeClock (4), disabling (20201113/evevent-436)
Otherwise seems to be working fine.
Comment Actions
Patch against current. I've checked that it compiles but cannot runtime test it.
The 13-stable version of this patch works fine on my hardware.
| sys/dev/efidev/efirt.c | ||
|---|---|---|
| 575 | ||
| 599 | If efi_call() returned error, the call to ReadBit seems to be useless. | |
| 603 | Don't you want to inform caller when ReadBit failed? | |
| 629 | Same note about efi_call() failing. | |
| usr.sbin/efiwake/efiwake.c | ||
| 49 ↗ | (On Diff #120872) | No need to put tab between type and var declaration. |
| 55 ↗ | (On Diff #120872) | Why not use getopt(3)? |
| 83 ↗ | (On Diff #120872) | Look at the err(3) |
| 141 ↗ | (On Diff #120872) | |
Comment Actions
Thanks for reviewing!
Would be good to get folks to test this out on their hardware. I'm not sure what to make of the BIOS in my mainboard... it allows setting the wake timer, e.g. 2025-10-31T20:50:59, but ignores the month component! So you get a timer date of 2025-04-31T..., a date that does not exist. Or maybe I made a mistake somewhere...
Comment Actions
Could you try to add printf()s in the kernel code right before SET_WAKETIME and after GET_WAKETIME efi_calls, to see what is the month value passed to/from UEFI RT methods. It might be not bios but some bug in userspace/kernel.
| usr.sbin/efiwake/efiwake.c | ||
|---|---|---|
| 68 ↗ | (On Diff #120897) | I suggest to add exit(EX_USAGE) to usage() and remove that return's. |
| 77 ↗ | (On Diff #120897) | check that only one of enable/disable flags is set? Also why not use a bool type for them? |
Comment Actions
/usr/sbin/efiwake -e 2023-10-23T22:00:00 tail /var/log/messages Apr 24 16:56:29 fred kernel: DEBUG before efi_set_waketime: 2023-10-23T22:0:0 Apr 24 16:56:29 fred kernel: DEBUG after efi_get_waketime: 2023-4-23T22:0:0
The time makes it through to the kernel alright. Something inside EFI mangles the month.
The machine does actually wake on the mangled date-time, tried it last night.
Comment Actions
I tested this patch against CURRENT and the update wakes my generic AMD desktop at the specified time using efiwake.
Comment Actions
thanks for testing!
Could you try and give efiwake a non-April date and see if it gets set correctly. (I suspect it's only a problem on my hardware, but worth checking)
| sys/dev/efidev/efirt.c | ||
|---|---|---|
| 603 | You need to either translate ACPI error code to errno namespace, or just assign some error value there, in case of !ACPI_SUCCESS | |
| 632 | Same. | |
| usr.sbin/efiwake/efiwake.c | ||
| 60 ↗ | (On Diff #120946) | case should be indented on the same level as switch. Basically un-indent the switch body by one tab. |
| sys/dev/efidev/efirt.c | ||
|---|---|---|
| 603 | I would rather not try to translate error codes... how about fixing it to EOPNOTSUPP? There's a bit of a can of worms: if the EFI call succeeds but ACPI fails, do we need to undo the EFI call? | |
| usr.sbin/efiwake/efiwake.c | ||
| 60 ↗ | (On Diff #120946) | IDE is forever fighting me on every key stroke... :/ |
| sys/dev/efidev/efirt.c | ||
|---|---|---|
| 603 | As I suggested above, single error code is fine too, e.g. EIO. You might printf something which would include raw value of the ACPI error. Also I do not think that we need to undo anything if ACPI call failed. If it is failed (i.e. the BIOS and hw are broken) so be it. We informed user about the situation. | |
Comment Actions
Please send me two git patches for this review, one for all kernel bits, and another for userspace utility addition. Please ensure that the patch metadata is correct, esp. the author' name and email.