linux-kernel - Re: [RESEND PATCH 07/11] platform/chrome: cros_ec_proto: return -EAGAIN when retries timed out

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: <Ys+QZyOdIT8d1rvW@google.com>
Date:   Thu, 14 Jul 2022 03:41:27 +0000
From:   Tzung-Bi Shih <tzungbi@...nel.org>
To:     Guenter Roeck <groeck@...gle.com>
Cc:     Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        "open list:CHROME HARDWARE PLATFORM SUPPORT" 
        <chrome-platform@...ts.linux.dev>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH 07/11] platform/chrome: cros_ec_proto: return
 -EAGAIN when retries timed out

On Wed, Jul 13, 2022 at 11:18:32AM -0700, Guenter Roeck wrote:
> On Mon, Jun 27, 2022 at 7:49 PM Tzung-Bi Shih <tzungbi@...nel.org> wrote:
> >
> > While EC_COMMS_STATUS_PROCESSING flag is still on after it tries
> > EC_COMMAND_RETRIES times for sending EC_CMD_GET_COMMS_STATUS,
> > cros_ec_wait_until_complete() doesn't return an error code.
> >
> > Return -EAGAIN in the case instead.
> 
> Does this make sense, or should it be -ETIMEDOUT ? What does the EC do
> if it is still busy (stuck ?) with executing a command and it gets
> another one ?

AFAIK, most existing ECs use single task for host command[1][2].  As a
result, EC won't reply if it was busying on executing a host command.
Not sure if it would change after leveraging Zephyr (if enabling multi-core
support).

EC_CMD_GET_COMMS_STATUS is the only exception.  EC executes the command in
interrupt context[3].  That's why AP can use EC_CMD_GET_COMMS_STATUS to query
the status while EC was busying on another host command.

I have no strong preference for the return code but tried to align to another
timeout case (when cros_ec_xfer_command() returned -EAGAIN for
EC_COMMAND_RETRIES times).  Do we want to separate the cases: one for -EAGAIN
and another one for -ETIMEDOUT?

[1]: https://crrev.com/4c0ae8814a68f2c2655ebb0b3b80ec4529d07cb3/common/host_command.c#428
[2]: https://crrev.com/4c0ae8814a68f2c2655ebb0b3b80ec4529d07cb3/board/volteer/ec.tasklist#20
[3]: https://crrev.com/4c0ae8814a68f2c2655ebb0b3b80ec4529d07cb3/common/host_command.c#176