From owner-svn-src-stable-8@freebsd.org Mon Dec 19 10:00:57 2016 Return-Path: Delivered-To: svn-src-stable-8@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6F1D4C87438; Mon, 19 Dec 2016 10:00:57 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 49CD416AD; Mon, 19 Dec 2016 10:00:57 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id uBJA0uKM091593; Mon, 19 Dec 2016 10:00:56 GMT (envelope-from hselasky@FreeBSD.org) Received: (from hselasky@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id uBJA0uvS091592; Mon, 19 Dec 2016 10:00:56 GMT (envelope-from hselasky@FreeBSD.org) Message-Id: <201612191000.uBJA0uvS091592@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: hselasky set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky Date: Mon, 19 Dec 2016 10:00:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r310256 - stable/8/sys/dev/acpica X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Dec 2016 10:00:57 -0000 Author: hselasky Date: Mon Dec 19 10:00:56 2016 New Revision: 310256 URL: https://svnweb.freebsd.org/changeset/base/310256 Log: MFC r309400: Fix for endless recursion in the ACPI GPE handler during boot. When handling a GPE ACPI interrupt object the EcSpaceHandler() function can be called which checks the EC_EVENT_SCI bit and then recurse on the EcGpeQueryHandler() function. If there are multiple GPE events pending the EC_EVENT_SCI bit will be set at the next call to EcSpaceHandler() causing it to recurse again via the EcGpeQueryHandler() function. This leads to a slow never ending recursion during boot which prevents proper system startup, because the EC_EVENT_SCI bit never gets cleared in this scenario. The behaviour is reproducible with the ALASKA AMI in combination with a newer Skylake based mainboard in the following way: Enter BIOS and adjust the clock one hour forward. Save and exit the BIOS. System fails to boot due to the above mentioned bug in EcGpeQueryHandler() which was observed recursing multiple times. This patch adds a simple recursion guard to the EcGpeQueryHandler() function and also also adds logic to detect if new GPE events occurred during the execution of EcGpeQueryHandler() and then loop on this function instead of recursing. Reviewed by: jhb Modified: stable/8/sys/dev/acpica/acpi_ec.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/dev/ (props changed) stable/8/sys/dev/acpica/ (props changed) Modified: stable/8/sys/dev/acpica/acpi_ec.c ============================================================================== --- stable/8/sys/dev/acpica/acpi_ec.c Mon Dec 19 09:54:59 2016 (r310255) +++ stable/8/sys/dev/acpica/acpi_ec.c Mon Dec 19 10:00:56 2016 (r310256) @@ -618,16 +618,14 @@ EcCheckStatus(struct acpi_ec_softc *sc, } static void -EcGpeQueryHandler(void *Context) +EcGpeQueryHandlerSub(struct acpi_ec_softc *sc) { - struct acpi_ec_softc *sc = (struct acpi_ec_softc *)Context; UINT8 Data; ACPI_STATUS Status; int retry; char qxx[5]; ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__); - KASSERT(Context != NULL, ("EcGpeQueryHandler called with NULL")); /* Serialize user access with EcSpaceHandler(). */ Status = EcLock(sc); @@ -654,7 +652,6 @@ EcGpeQueryHandler(void *Context) else break; } - sc->ec_sci_pend = FALSE; if (ACPI_FAILURE(Status)) { EcUnlock(sc); device_printf(sc->ec_dev, "GPE query failed: %s\n", @@ -685,6 +682,29 @@ EcGpeQueryHandler(void *Context) } } +static void +EcGpeQueryHandler(void *Context) +{ + struct acpi_ec_softc *sc = (struct acpi_ec_softc *)Context; + int pending; + + KASSERT(Context != NULL, ("EcGpeQueryHandler called with NULL")); + + do { + /* Read the current pending count */ + pending = atomic_load_acq_int(&sc->ec_sci_pend); + + /* Call GPE handler function */ + EcGpeQueryHandlerSub(sc); + + /* + * Try to reset the pending count to zero. If this fails we + * know another GPE event has occurred while handling the + * current GPE event and need to loop. + */ + } while (!atomic_cmpset_int(&sc->ec_sci_pend, pending, 0)); +} + /* * The GPE handler is called when IBE/OBF or SCI events occur. We are * called from an unknown lock context. @@ -713,13 +733,14 @@ EcGpeHandler(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_fetchadd_int(&sc->ec_sci_pend, 1) == 0) { 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_store_rel_int(&sc->ec_sci_pend, 0); + } } return (0); } @@ -766,7 +787,8 @@ EcSpaceHandler(UINT32 Function, ACPI_PHY * 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_fetchadd_int(&sc->ec_sci_pend, 1) == 0) { CTR0(KTR_ACPI, "ec running gpe handler directly"); EcGpeQueryHandler(sc); }