linux-kernel - Re: [PATCH v2] platform/chrome: cros_ec: Send host event for prepare/complete

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: <CACeCKafvVATzEqOnkT3-QPyfaTV8baHNKbLy8069-K+P+RPeMg@mail.gmail.com>
Date:   Thu, 14 Jul 2022 16:33:19 -0700
From:   Prashant Malani <pmalani@...omium.org>
To:     Tim Van Patten <timvp@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, rrangel@...omium.org,
        robbarnes@...gle.com, Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        chrome-platform@...ts.linux.dev
Subject: Re: [PATCH v2] platform/chrome: cros_ec: Send host event for prepare/complete

On Thu, Jul 14, 2022 at 3:17 PM Tim Van Patten <timvp@...gle.com> wrote:
>
> Hi,
>
> > Calling such a theoretical new EC host command from the userspace power manager
> > would probably accomplish the same thing.
>
> We investigated something like that internally initially:
> https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3646238)
>
> We decided to take this simpler approach instead, since it
> accomplishes the same goal without requiring new host commands.

- That link discusses updating the existing host command and the EC
implementations;
have you investigated adding a command for logging needs and calling it from
power_manager/powerd directly?
- That link addresses several busses (UART, SPI etc.) whereas this patch only
touches LPC. Is this intentional?

>
> > From the kernel documentation[2], "The prepare phase is meant to
> > prevent races by preventing new devices from being registered; the PM core
> > would never know that all the children of a device had been suspended if new
> > children could be registered at will." and "The method may also prepare the
> > device or driver in some way for the upcoming system power transition, but
> > it should not put the device into a low-power state."
> >
> > So it seems like an incorrect usage of this callback.
>
> Why is this usage incorrect?
>
> Sending the message to the EC is the preparation for the AP to enter
> the system power transition. For example, it allows the EC to begin
> watchdogging the AP once the suspend begins and stop once the resume
> completes. This allows the EC to watchdog the entire process, without
> any gaps - a beneficial side effect of this change.

OK. Is that all it does? Does this not allow the EC itself to enter a
deep sleep state,
or put connected peripherals and/or state machines in a low power state??

If you're still set on this approach, please update the function names;
setting the .prepare()/.complete() function pointers to
cros_ec_lpc_suspend/resume()
is inconsistent, if those functions aren't being used elsewhere in the file.

BR,

-Prashant