From: dv <dv@openbsd.org>
Date: Thu, 29 Jun 2023 20:58:08 +0000 (+0000)
Subject: acpi(4)/acpibtn(4): use opt-in approach for wakeup GPEs
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d8f86d5931f8efff41165fa21cdcd083288e9ce9;p=openbsd

acpi(4)/acpibtn(4): use opt-in approach for wakeup GPEs

Previously, any discovered GPE that could be enabled for wake was
being enabled prior to entering ACPI-based S3 or S4. On some newer
machines, this caused S3-based suspend to break as some devices we
don't fully control (e.g. lack of driver) might be capable of waking
the system and will do so almost instantly if the GPE is enabled for
wake up (i.e. the so called "instant wake" issue).

This changes to a model of having device drivers explicitly opt
into using their GPE for wake up.

The first driver to explicitly toggle a GPE for wake is acpibtn(4).

Variations of the above have been in snapshots for 2 weeks, so
tested by many.

ok deraadt@
---

diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c
index 3bedd6967d2..42cd7bfdb7e 100644
--- a/sys/dev/acpi/acpi.c
+++ b/sys/dev/acpi/acpi.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: acpi.c,v 1.420 2023/03/15 13:01:40 kettenis Exp $ */
+/* $OpenBSD: acpi.c,v 1.421 2023/06/29 20:58:08 dv Exp $ */
 /*
  * Copyright (c) 2005 Thorsten Lockert <tholo@sigmasoft.com>
  * Copyright (c) 2005 Jordan Hargrave <jordan@openbsd.org>
@@ -2264,7 +2264,7 @@ acpi_enable_wakegpes(struct acpi_softc *sc, int state)
 		dnprintf(10, "%.4s(S%d) gpe %.2x\n", wentry->q_node->name,
 		    wentry->q_state,
 		    wentry->q_gpe);
-		if (state <= wentry->q_state)
+		if (wentry->q_enabled && state <= wentry->q_state)
 			acpi_enable_onegpe(sc, wentry->q_gpe);
 	}
 }
@@ -2354,11 +2354,32 @@ acpi_foundprw(struct aml_node *node, void *arg)
 			wq->q_gpe = wq->q_wakepkg->v_package[0]->v_integer;
 		if (wq->q_wakepkg->v_package[1]->type == AML_OBJTYPE_INTEGER)
 			wq->q_state = wq->q_wakepkg->v_package[1]->v_integer;
+		wq->q_enabled = 0;
 	}
 	SIMPLEQ_INSERT_TAIL(&sc->sc_wakedevs, wq, q_next);
 	return 0;
 }
 
+int
+acpi_toggle_wakedev(struct acpi_softc *sc, struct aml_node *node, int enable)
+{
+	struct acpi_wakeq *wentry;
+	int ret = -1;
+
+	SIMPLEQ_FOREACH(wentry, &sc->sc_wakedevs, q_next) {
+		if (wentry->q_node == node) {
+			wentry->q_enabled = enable ? 1 : 0;
+			dnprintf(10, "%.4s(S%d) gpe %.2x %sabled\n",
+			    wentry->q_node->name, wentry->q_state,
+			    wentry->q_gpe, enable ? "en" : "dis");
+			ret = 0;
+			break;
+		}
+	}
+
+	return ret;
+}
+
 struct gpe_block *
 acpi_find_gpe(struct acpi_softc *sc, int gpe)
 {
diff --git a/sys/dev/acpi/acpibtn.c b/sys/dev/acpi/acpibtn.c
index a16cb32ab0a..373f05e1377 100644
--- a/sys/dev/acpi/acpibtn.c
+++ b/sys/dev/acpi/acpibtn.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: acpibtn.c,v 1.50 2023/04/03 13:38:18 millert Exp $ */
+/* $OpenBSD: acpibtn.c,v 1.51 2023/06/29 20:58:08 dv Exp $ */
 /*
  * Copyright (c) 2005 Marco Peereboom <marco@openbsd.org>
  *
@@ -168,7 +168,7 @@ acpibtn_attach(struct device *parent, struct device *self, void *aux)
 	sc->sc_acpi = (struct acpi_softc *)parent;
 	sc->sc_devnode = aa->aaa_node;
 
-	printf(": %s\n", sc->sc_devnode->name);
+	printf(": %s", sc->sc_devnode->name);
 
 	if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "_STA", 0, NULL, &st))
 		st = STA_PRESENT | STA_ENABLED | STA_DEV_OK;
@@ -203,6 +203,15 @@ acpibtn_attach(struct device *parent, struct device *self, void *aux)
 		sc->sc_sens.value = lid_open;
 	}
 
+	/* Enable any related GPEs for wake. */
+	if (acpi_toggle_wakedev(sc->sc_acpi, sc->sc_devnode, 1) == 0) {
+#ifdef SUSPEND
+		device_register_wakeup(self);
+		printf("(wakeup)");
+#endif /* SUSPEND */
+	}
+
+	printf("\n");
 	aml_register_notify(sc->sc_devnode, aa->aaa_dev, acpibtn_notify,
 	    sc, ACPIDEV_NOPOLL);
 }
diff --git a/sys/dev/acpi/acpivar.h b/sys/dev/acpi/acpivar.h
index a9b4a2ae98a..d783dd26db3 100644
--- a/sys/dev/acpi/acpivar.h
+++ b/sys/dev/acpi/acpivar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: acpivar.h,v 1.122 2022/09/13 17:14:54 kettenis Exp $	*/
+/*	$OpenBSD: acpivar.h,v 1.123 2023/06/29 20:58:08 dv Exp $	*/
 /*
  * Copyright (c) 2005 Thorsten Lockert <tholo@sigmasoft.com>
  *
@@ -105,6 +105,7 @@ struct acpi_wakeq {
 	struct aml_value		*q_wakepkg;
 	int				 q_gpe;
 	int				 q_state;
+	int				 q_enabled;
 };
 
 #if NACPIPWRRES > 0
@@ -415,6 +416,8 @@ struct acpi_q *acpi_maptable(struct acpi_softc *sc, paddr_t,
 
 bus_dma_tag_t acpi_iommu_device_map(struct aml_node *, bus_dma_tag_t);
 
+int	acpi_toggle_wakedev(struct acpi_softc *, struct aml_node *, int);
+
 #endif
 
 #endif /* !_ACPI_WAKECODE */