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 for Android: free password hash cracker in your pocket

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

Message-ID: <CABXOdTcb2iMfokJL-rHJ5FkUYTyvjbaqooVXud2pfoE-_fEg9Q@mail.gmail.com>
Date:   Thu, 14 Jul 2022 07:15:46 -0700
From:   Guenter Roeck <groeck@...gle.com>
To:     Tzung-Bi Shih <tzungbi@...nel.org>
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 8:41 PM Tzung-Bi Shih <tzungbi@...nel.org> wrote:
>
> 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?

If -EAGAIN is used elsewhere, let's stick with it.

Thanks,
Guenter

>
> [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