efirt: When present, attempt to use EFI runtime services to shutdown
ClosedPublic
Actions

Authored by cem on Dec 11 2018, 2:23 AM.

Details

Reviewers

imp
byuu_tutanota.com

Group Reviewers

manpages

Commits

rS342108: efirt: When present, attempt to use EFI runtime services to shutdown

Summary

Submitted by: byuu <byuu AT tutanota.com> (previous version)

Test Plan

Unlike byuu's original submission, I left efi_reset_system(void) alone for two
reasons:

Potential out of tree consumers (maybe Netflix?)
Laziness: Changing the signature requires updating efirt.9

I would be happy to switch efi_reset_system() to the enum-taking variety if
that seems better.

I made the efi_poweroff knob the default setting on EFIRT-enabled systems, but
do not feel strongly about it. We could make it off by default if you prefer.

We could similarly handle RB_REBOOT / whatever shutdown_final events with
"rt_reset" calls. Again, if you feel strongly about it, I can add that
support. I am considering adding the same functionality to the IPMI
shutdown_final event handler anyway -- we use a different IPMI reset mode than
the one in FreeBSD base tree at ISLN.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

cem created this revision.Dec 11 2018, 2:23 AM

Harbormaster completed remote builds in B21486: Diff 51846.Dec 11 2018, 2:23 AM

cem added reviewers: imp, byuu_tutanota.com.Dec 11 2018, 2:25 AM

Netflix doesn't use efi_system_reset() in our version of FreeBSD. Changing the signature is likely fine.

sys/dev/efidev/efirt.c
115 ↗	(On Diff #51846)	Small quibble. s/shutdown/power off/ I think that we should use the most specific term here.
458 ↗	(On Diff #51846)	We (NFLX) don't do anything out of tree with this.
sys/sys/efi.h
45 ↗	(On Diff #51846)	as there's more than 2, maybe having explicit values would be good. Maybe they should have been explicit all the time.

Netflix doesn't use efi_system_reset() in our version of FreeBSD. Changing the signature is likely fine.

Ok, works for me.

sys/dev/efidev/efirt.c
115 ↗	(On Diff #51846)	Will fix
sys/sys/efi.h
45 ↗	(On Diff #51846)	I'm happy to make them explicit if you'd prefer that. They are just 0, 1, 2.

Use explicit terminology in sysctl text.
Change type of efi_reset_system(9), and document in efirt.9 (which needs cleanup, but that will be left for future work).
Explicitly define enum efi_reset values.

Harbormaster completed remote builds in B21507: Diff 51885.Dec 12 2018, 2:10 AM