From owner-svn-src-head@freebsd.org Fri Jun 1 09:44:24 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2AD64F7E00E; Fri, 1 Jun 2018 09:44:24 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id CCF2E6E9EA; Fri, 1 Jun 2018 09:44:23 +0000 (UTC) (envelope-from avg@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 mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id AD69227212; Fri, 1 Jun 2018 09:44:23 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w519iNkZ006376; Fri, 1 Jun 2018 09:44:23 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w519iNkT006375; Fri, 1 Jun 2018 09:44:23 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201806010944.w519iNkT006375@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Fri, 1 Jun 2018 09:44:23 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r334479 - head/sys/dev/acpica X-SVN-Group: head X-SVN-Commit-Author: avg X-SVN-Commit-Paths: head/sys/dev/acpica X-SVN-Commit-Revision: 334479 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Jun 2018 09:44:24 -0000 Author: avg Date: Fri Jun 1 09:44:23 2018 New Revision: 334479 URL: https://svnweb.freebsd.org/changeset/base/334479 Log: call AcpiLeaveSleepStatePrep after re-enabling interrupts I want to do this change because this call (actually, AcpiHwLegacyWakePrep) does a memory allocation and ACPI namespace evaluation. Although it is not very likely to run into any trouble, it is still not safe to make those calls with interrupts disabled. witness(4) and malloc(9) do not currently check for a context with interrupts disabled via intr_disable and we lack a facility for doing that. So, those unsafe operations fly under the radar. But if intr_disable in acpi_EnterSleepState was replaced with spinlock_enter (which it probably should be), then witness and malloc would immediately complain. Also, AcpiLeaveSleepStatePrep is documented as called when interrupts are enabled. It used to require disabled interrupts, but that requirement was changed a long time ago when support for _BFS and _GTS was removed from ACPICA. The ACPI wakeup sequence is very sensitive to changes. I consider this change to be correct, but there can be fallouts from it. What AcpiHwLegacyWakePrep essentially does is writing a value corresponding to S0 into SLP_TYPx bits of PM1 Control Register(s). According to ACPI specifications that write should be a NOP as SLP_EN bit is not set. But I see in some chipset specifications that they allow to ignore SLP_EN altogether and to act on a change of SLP_TYPx alone. Also, there are a couple of accesses to ACPI hardware before the new location of the call to AcpiLeaveSleepStatePrep. One is to clear the power button status and the other is to enable SCI. So, the move may affect the interaction between then OS and ACPI platform. I have not seen any regressions on my test system, but it's a desktop. MFC after: 5 weeks Modified: head/sys/dev/acpica/acpi.c Modified: head/sys/dev/acpica/acpi.c ============================================================================== --- head/sys/dev/acpica/acpi.c Fri Jun 1 09:41:15 2018 (r334478) +++ head/sys/dev/acpica/acpi.c Fri Jun 1 09:44:23 2018 (r334479) @@ -2977,8 +2977,6 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state) if (sleep_result == 1 && state != ACPI_STATE_S4) AcpiWriteBitRegister(ACPI_BITREG_SCI_ENABLE, ACPI_ENABLE_EVENT); - AcpiLeaveSleepStatePrep(state); - if (sleep_result == 1 && state == ACPI_STATE_S3) { /* * Prevent mis-interpretation of the wakeup by power button @@ -3007,6 +3005,8 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state) /* call acpi_wakeup_machdep() again with interrupt enabled */ acpi_wakeup_machdep(sc, state, sleep_result, 1); + AcpiLeaveSleepStatePrep(state); + if (sleep_result == -1) goto backout; @@ -3015,8 +3015,8 @@ acpi_EnterSleepState(struct acpi_softc *sc, int state) AcpiEnable(); } else { status = AcpiEnterSleepState(state); - AcpiLeaveSleepStatePrep(state); intr_restore(intr); + AcpiLeaveSleepStatePrep(state); if (ACPI_FAILURE(status)) { device_printf(sc->acpi_dev, "AcpiEnterSleepState failed - %s\n", AcpiFormatException(status));