linux-kernel - Re: [PATCH] drivers/acpi: Clear status of an event before enabling it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0h7dYEFHSOHengJ3OFyXBW+XZVbjCAegpzqg1VxeeaXog@mail.gmail.com>
Date:   Thu, 21 Mar 2019 00:11:28 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Furquan Shaikh <furquan@...gle.com>
Cc:     Robert Moore <robert.moore@...el.com>,
        "Schmauss, Erik" <erik.schmauss@...el.com>,
        Rafael Wysocki <rafael.j.wysocki@...el.com>,
        Len Brown <lenb@...nel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        devel@...ica.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Rajat Jain <rajatja@...gle.com>, evgreen@...gle.com,
        dlaurie@...gle.com
Subject: Re: [PATCH] drivers/acpi: Clear status of an event before enabling it

On Wed, Mar 20, 2019 at 11:34 PM Furquan Shaikh <furquan@...gle.com> wrote:
>
> Commit 18996f2db918 ("ACPICA: Events: Stop unconditionally
> clearing ACPI IRQs during suspend/resume") was added to stop clearing
> of event status bits unconditionally on suspend and resume paths. This
> was done because of an issue
> reported (https://bugzilla.kernel.org/show_bug.cgi?id=196249) where
> lid status stays closed even on resume (which happens because event
> status bits are cleared unconditionally on resume). Though this change
> fixed the issue on suspend path, it introduced regressions on several
> resume paths.
>
> First regression was reported and fixed on S5 path by the following
> change: commit fa85015c0d95 ("ACPICA: Clear status of all events when
> entering S5"). Next regression was reported and fixed on all legacy
> sleep paths by the commit f317c7dc12b7 ("ACPICA: Clear status of all
> events when entering sleep states"). However, regression still exists
> on S0ix sleep path since it does not follow the legacy sleep path.

What exactly is failing?

> In case of S0ix, events are enabled as part of device suspend path. If
> status bits for the events are set when they are enabled, it could
> result in premature wake from S0ix. This change ensures that status is
> cleared for any event that is being enabled so that any stale events
> are cleared out.
>
> Signed-off-by: Furquan Shaikh <furquan@...gle.com>
> ---
>  drivers/acpi/acpica/evgpe.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpica/evgpe.c b/drivers/acpi/acpica/evgpe.c
> index 62d3aa74277b4..61455ab42fc87 100644
> --- a/drivers/acpi/acpica/evgpe.c
> +++ b/drivers/acpi/acpica/evgpe.c
> @@ -81,8 +81,12 @@ acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
>
>         ACPI_FUNCTION_TRACE(ev_enable_gpe);
>
> -       /* Enable the requested GPE */
> +       /* Clear the GPE (of stale events) */
> +       status = acpi_hw_clear_gpe(gpe_event_info);
> +       if (ACPI_FAILURE(status))
> +               return_ACPI_STATUS(status);

Well, this may cause events to be missed.

>
> +       /* Enable the requested GPE */
>         status = acpi_hw_low_set_gpe(gpe_event_info, ACPI_GPE_ENABLE);
>         return_ACPI_STATUS(status);
>  }
> --
> 2.21.0.225.g810b269d1ac-goog
>