acpi: Forbid requesting S4 when S4BIOS is not supported

Pending the OS-supported hibernate functionality, prevent requesting S4
when S4BIOS is not supported.

To this end, make sure that acpi_supported_stypes[] indicates that
POWER_STYPE_FW_HIBERNATE is not supported if 'acpi_s4bios_supported' is
false, even if S4 is supported by the platform (which is only
a power-down-like state, without any support to save the system image by
itself).  This will cause requests to enter S4, which are translated to
POWER_STYPE_FW_HIBERNATE before reaching
acpi_ReqSleepState()/acpi_EnterSleepState(), to fail in this case.

Retire the 'hw.acpi.s4bios' sysctl knob, as having it to 0 by default
(S4BIOS not supported) or setting it to 0 (default is 1 when S4BIOS is
supported) could only lead, on a S4 request, to a power down without any
possibility to restore the system (and, since a recent commit, it has
not been possible anymore to force it to 1 when S4BIOS is not announced
supported in the FACS table, which would cause a failure or a crash).

When OS-supported hibernate is introduced, it will become the default
hibernate method over S4BIOS, as we have not heard of any semi-recent
hardware platform implementing it (and when it is, it usually needs
a sufficiently large dedicated slice/partition, and does not use FreeBSD
swap slices/partitions).  Then, for people still wanting to use S4BIOS
on older platforms that support it, the tentative plans are to modify
acpiconf(8) to accept '4BIOS' as an argument to '-s', or modify zzz(8)
so that one can pass a specific suspension method.

Reviewed by:    obiwac
Event:          Halifax Hackathon 202606
Sponsored by:   The FreeBSD Foundation
Differential Revision:  https://reviews.freebsd.org/D57414

4 files changed, 20 insertions, 38 deletions

diff --git a/sys/amd64/acpica/acpi_wakeup.c b/sys/amd64/acpica/acpi_wakeup.c
index 0dd670300da9..3e64e449dbd9 100644
--- a/sys/amd64/acpica/acpi_wakeup.c
+++ b/sys/amd64/acpica/acpi_wakeup.c
@@ -227,8 +227,12 @@ acpi_sleep_machdep(struct acpi_softc *sc, int state)
 		WAKECODE_FIXUP(wakeup_gdt, uint16_t, pcb->pcb_gdt.rd_limit);
 		WAKECODE_FIXUP(wakeup_gdt + 2, uint64_t, pcb->pcb_gdt.rd_base);
 
-		/* Call ACPICA to enter the desired sleep state */
-		if (state == ACPI_STATE_S4 && acpi_should_do_s4bios(sc))
+		/*
+		 * Call ACPICA to enter the desired sleep state.  Currently,
+		 * reaching this code with 'state' being ACPI_STATE_S4 implies
+		 * a S4BIOS request.
+		 */
+		if (state == ACPI_STATE_S4)
 			status = AcpiEnterSleepStateS4bios();
 		else
 			status = AcpiEnterSleepState(state);
diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index 9a85a0844de6..7103eea16053 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -189,7 +189,6 @@ static void	acpi_system_eventhandler_sleep(void *arg,
 		    enum power_stype stype);
 static void	acpi_system_eventhandler_wakeup(void *arg,
 		    enum power_stype stype);
-static int	acpi_s4bios_sysctl(SYSCTL_HANDLER_ARGS);
 static enum power_stype	acpi_sstate_to_stype(int sstate);
 static int	acpi_sname_to_sstate(const char *sname);
 static const char	*acpi_sstate_to_sname(int sstate);
@@ -626,8 +625,8 @@ acpi_attach(device_t dev)
      * Mark whether S4BIOS is available according to the FACS, and if it is,
      * enable it by default.
      */
-    if (AcpiGbl_FACS != NULL && AcpiGbl_FACS->Flags & ACPI_FACS_S4_BIOS_PRESENT)
-	sc->acpi_s4bios = sc->acpi_s4bios_supported = true;
+    sc->acpi_s4bios_supported = AcpiGbl_FACS != NULL &&
+	(AcpiGbl_FACS->Flags & ACPI_FACS_S4_BIOS_PRESENT) != 0;
 
     /*
      * Probe all supported ACPI sleep states.  Awake (S0) is always supported,
@@ -646,6 +645,12 @@ acpi_attach(device_t dev)
 	    sc->acpi_supported_stypes[acpi_sstate_to_stype(state)] = true;
 	}
     }
+    /*
+     * Prevent users from requesting firmware-supported image saving if firmware
+     * does not indicate it as supported.
+     */
+    if (!sc->acpi_s4bios_supported)
+	sc->acpi_supported_stypes[POWER_STYPE_FW_HIBERNATE] = false;
 
     /*
      * Dispatch the default sleep type to devices.  The lid switch is set
@@ -758,10 +763,6 @@ acpi_attach(device_t dev)
     SYSCTL_ADD_INT(&sc->acpi_sysctl_ctx, SYSCTL_CHILDREN(sc->acpi_sysctl_tree),
 	OID_AUTO, "sleep_delay", CTLFLAG_RW, &sc->acpi_sleep_delay, 0,
 	"sleep delay in seconds");
-    SYSCTL_ADD_PROC(&sc->acpi_sysctl_ctx, SYSCTL_CHILDREN(sc->acpi_sysctl_tree),
-	OID_AUTO, "s4bios", CTLTYPE_U8 | CTLFLAG_RW | CTLFLAG_MPSAFE,
-	sc, 0, acpi_s4bios_sysctl, "CU",
-	"On hibernate, have the firmware save/restore the machine state (S4BIOS).");
     SYSCTL_ADD_BOOL(&sc->acpi_sysctl_ctx, SYSCTL_CHILDREN(sc->acpi_sysctl_tree),
 	OID_AUTO, "s4bios_supported", CTLFLAG_RD, &sc->acpi_s4bios_supported, 0,
 	"Whether firmware supports saving/restoring the machine state (S4BIOS).");
@@ -4489,25 +4490,6 @@ acpiioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, struct thread *t
 }
 
 static int
-acpi_s4bios_sysctl(SYSCTL_HANDLER_ARGS)
-{
-    struct acpi_softc *const sc = arg1;
-    bool val;
-    int error;
-
-    val = sc->acpi_s4bios;
-    error = sysctl_handle_bool(oidp, &val, 0, req);
-    if (error != 0 || req->newptr == NULL)
-	return (error);
-
-    if (val && !sc->acpi_s4bios_supported)
-	return (EOPNOTSUPP);
-    sc->acpi_s4bios = val;
-
-    return (0);
-}
-
-static int
 acpi_sname_to_sstate(const char *sname)
 {
     int sstate;
diff --git a/sys/dev/acpica/acpivar.h b/sys/dev/acpica/acpivar.h
index 476170f5743b..01e7ddcd74cd 100644
--- a/sys/dev/acpica/acpivar.h
+++ b/sys/dev/acpica/acpivar.h
@@ -68,7 +68,6 @@ struct acpi_softc {
     enum power_stype	acpi_lid_switch_stype;
 
     int			acpi_standby_sx;
-    bool		acpi_s4bios;
     bool		acpi_s4bios_supported;
 
     int			acpi_sleep_delay;
@@ -517,13 +516,6 @@ acpi_d_state_to_str(int state)
     return (strs[state]);
 }
 
-static __inline bool
-acpi_should_do_s4bios(struct acpi_softc *sc)
-{
-    MPASS(!sc->acpi_s4bios || sc->acpi_s4bios_supported);
-    return (sc->acpi_s4bios);
-}
-
 char		*acpi_name(ACPI_HANDLE handle);
 int		acpi_avoid(ACPI_HANDLE handle);
 int		acpi_disabled(char *subsys);
diff --git a/sys/i386/acpica/acpi_wakeup.c b/sys/i386/acpica/acpi_wakeup.c
index b5390f3cd659..f49464e6efb8 100644
--- a/sys/i386/acpica/acpi_wakeup.c
+++ b/sys/i386/acpica/acpi_wakeup.c
@@ -239,8 +239,12 @@ acpi_sleep_machdep(struct acpi_softc *sc, int state)
 		 */
 		pmap_remap_lowptdi(true);
 
-		/* Call ACPICA to enter the desired sleep state */
-		if (state == ACPI_STATE_S4 && acpi_should_do_s4bios(sc))
+		/*
+		 * Call ACPICA to enter the desired sleep state.  Currently,
+		 * reaching this code with 'state' being ACPI_STATE_S4 implies
+		 * a S4BIOS request.
+		 */
+		if (state == ACPI_STATE_S4)
 			status = AcpiEnterSleepStateS4bios();
 		else
 			status = AcpiEnterSleepState(state);