linux-kernel - Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver

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: <CABb+yY3CfWFBDh-qoGHDeHyLhVC7mPQZFB=VDS54OaypG-BqPw@mail.gmail.com>
Date:   Mon, 8 May 2017 12:17:02 +0530
From:   Jassi Brar <jassisinghbrar@...il.com>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Jeffrey Hugo <jhugo@...eaurora.org>,
        Andy Gross <andy.gross@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ohad Ben-Cohen <ohad@...ery.com>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        Devicetree List <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-remoteproc@...r.kernel.org
Subject: Re: [PATCH v4 3/5] soc: qcom: Introduce APCS IPC driver

On Mon, May 8, 2017 at 11:24 AM, Bjorn Andersson
<bjorn.andersson@...aro.org> wrote:
> On Fri 05 May 21:48 PDT 2017, Jassi Brar wrote:
>
> The APCS IPC register serves the basis for all inter-processor
> communication in a Qualcomm platform, so it's not only the RPM driver
> discussed earlier that uses this. It's also used for other non-FIFO
> based communication channels, where the signalled information either
> isn't acked at all or acked on a system-level.
>
Something has to indicate consumption of data or "requested action
taken". Otherwise the protocol is design-wise broken.

> But regardless of the protocol implemented ontop, the purpose of the
> APCS IPC bit is _only_ to invoke some remote handler to consume some
> data, somewhere - the event in itself does not carry any information.
>
Yes, every platform that uses shared-memory works like that. However
there is always something that tells if the command has been acted
upon by the remote. In your case that is the read-pointer movement.

>> The client should call mbox_client_txdone() after
>> mbox_send_message().
>
> So every time we call mbox_send_message() from any of the client drivers
> we also needs to call mbox_client_txdone()?
>
Yes.

> This seems like an awkward side effect of using the mailbox framework -
> which has to be spread out in at least 6 different client drivers :(
>
No. Mailbox or whatever you implement - you must (and do) tick the
state machine to keep the messages moving.
  Best designs have some interrupt occurring when the message has been
consumed by the remote. Some designs have a flag set which needs to be
polled to detect completion. Very few (like yours) that support
neither irq nor polling, have to be driven by the upper protocol layer
by some ack packet (or tracking read/write pointers like you do).
These three cases are denoted by TXDONE_BY_IRQ, TXDONE_BY_POLL and
TXDONE_BY_ACK respectively.

  If no client driver will ever submit a message if there is no space
in FIFO, then you can specify TXDONE_BY_POLL and have last_tx_done()
always return true. That way you don't need to call
mbox_client_txdone().