Hi,
I've managed to debug a problem in acpi_ec.c with a recent skylake laptop:
In EcSpaceHandler() there is a call to EcGpeQueryHandler(). This causes
EcSpaceHandler() to recurse during cold boot.
When EcGpeQueryHandler() is recursing it appears that because the
completion of a GPE interrupt also invokes EcSpaceHandler() again, we go
into an infinite loop, and the ACPI just generate more and more commands
that EcGpeQueryHandler() never completes, because it is reading the
command queue first all the way.
Solution: Don't allow EcGpeQueryHandler() to recurse this way. The
attached patch entirely fixes my problem.
Can someone here review the attached patch?
--HPS
diff --git a/sys/dev/acpica/acpi_ec.c b/sys/dev/acpica/acpi_ec.c
index 6482b3f..7721e10 100644
--- a/sys/dev/acpica/acpi_ec.c
+++ b/sys/dev/acpica/acpi_ec.c
@@ -647,12 +647,11 @@ EcGpeQueryHandler(void *Context)
EC_EVENT_INPUT_BUFFER_EMPTY)))
break;
}
- sc->ec_sci_pend = FALSE;
if (ACPI_FAILURE(Status)) {
EcUnlock(sc);
device_printf(sc->ec_dev, "GPE query failed: %s\n",
AcpiFormatException(Status));
- return;
+ goto done;
}
Data = EC_GET_DATA(sc);
@@ -666,7 +665,7 @@ EcGpeQueryHandler(void *Context)
/* Ignore the value for "no outstanding event". (13.3.5) */
CTR2(KTR_ACPI, "ec query ok,%s running _Q%02X", Data ? "" : " not", Data);
if (Data == 0)
- return;
+ goto done;
/* Evaluate _Qxx to respond to the controller. */
snprintf(qxx, sizeof(qxx), "_Q%02X", Data);
@@ -676,6 +675,9 @@ EcGpeQueryHandler(void *Context)
device_printf(sc->ec_dev, "evaluation of query method %s failed: %s\n",
qxx, AcpiFormatException(Status));
}
+done:
+ /* Allow EcGpeQueryHandler() to be called again */
+ atomic_cmpset_int(&sc->ec_sci_pend, 1, 0);
}
/*
@@ -706,13 +708,14 @@ EcGpeHandler(ACPI_HANDLE GpeDevice, UINT32 GpeNumber, void *Context)
* It will run the query and _Qxx method later, under the lock.
*/
EcStatus = EC_GET_CSR(sc);
- if ((EcStatus & EC_EVENT_SCI) && !sc->ec_sci_pend) {
+ if ((EcStatus & EC_EVENT_SCI) &&
+ atomic_cmpset_int(&sc->ec_sci_pend, 0, 1)) {
CTR0(KTR_ACPI, "ec gpe queueing query handler");
Status = AcpiOsExecute(OSL_GPE_HANDLER, EcGpeQueryHandler, Context);
- if (ACPI_SUCCESS(Status))
- sc->ec_sci_pend = TRUE;
- else
+ if (ACPI_FAILURE(Status)) {
printf("EcGpeHandler: queuing GPE query handler failed\n");
+ atomic_cmpset_int(&sc->ec_sci_pend, 1, 0);
+ }
}
return (ACPI_REENABLE_GPE);
}
@@ -759,7 +762,8 @@ EcSpaceHandler(UINT32 Function, ACPI_PHYSICAL_ADDRESS Address, UINT32 Width,
* we call it directly here since our thread taskq is not active yet.
*/
if (cold || rebooting || sc->ec_suspending) {
- if ((EC_GET_CSR(sc) & EC_EVENT_SCI)) {
+ if ((EC_GET_CSR(sc) & EC_EVENT_SCI) &&
+ atomic_cmpset_int(&sc->ec_sci_pend, 0, 1)) {
CTR0(KTR_ACPI, "ec running gpe handler directly");
EcGpeQueryHandler(sc);
}
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "[email protected]"
