linux-kernel - Re: [PATCH v2 1/2] mailbox: add async request mechanism to empower controllers w/ hw queues

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+yY0H4cATB9Gz2EitnR6R179aKDzR1N87fz7Hq9Hm-_8Rmw@mail.gmail.com>
Date: Mon, 21 Oct 2024 11:32:40 -0500
From: Jassi Brar <jassisinghbrar@...il.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>
Cc: krzk@...nel.org, alim.akhtar@...sung.com, mst@...hat.com, 
	javierm@...hat.com, tzimmermann@...e.de, bartosz.golaszewski@...aro.org, 
	luzmaximilian@...il.com, sudeep.holla@....com, conor.dooley@...rochip.com, 
	bjorn@...osinc.com, ulf.hansson@...aro.org, linux-samsung-soc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	marcan@...can.st, neal@...pa.dev, alyssa@...enzweig.io, broonie@...nel.org, 
	andre.draszik@...aro.org, willmcvicker@...gle.com, peter.griffin@...aro.org, 
	kernel-team@...roid.com, vincent.guittot@...aro.org, 
	daniel.lezcano@...aro.org
Subject: Re: [PATCH v2 1/2] mailbox: add async request mechanism to empower
 controllers w/ hw queues

> On 10/18/24 8:49 AM, Tudor Ambarus wrote:

> The active request is considered completed when TX completes. But it seems
> that TX is not in direct relation with RX,

TX and RX are assumed independent operations (which they are).
TX is sending a message/request to the remote successfully. 'Success'
can be indicated by any of the three methods  TXDONE_BY_{POLLING, IRQ,
ACK}.
You seem to assume it should always be an ACK where we receive an
acknowledgment/response packet, which is not the case.

...

>> Correct, and it is not meant to be.
>> You are assuming there is always an RX in response to a TX, which is

> Not really. If there's no response expected, clients can set req->rx to NULL.

You are assuming the default behaviour is that there is a reply
associated with a TX, otherwise "clients can set req->rx to NULL".
This api can be _used_ only for protocols that expect a response for
each TX. For other protocols,  it is simply a passthrough wrapper (by
doing things like req->rx = NULL). For handling this special case of
Tx->Rx, maybe a higher level common helper function would be better.

...

>> reasons, it is left to the user to tie an incoming RX to some previous
>> TX, or not.

> Is there a specific driver that I can look at in order to understand the
> case where RX is not tied to TX?

 Many...
 * The remote end owns sensors and can asynchronously send, say
thermal, notifications to Linux.
 * On some platform the RX may be asynchronous, that is sent later
with some identifying tag of the past TX.
 * Just reverse the roles of local and remote - remote sends us a
request (RX for us) and we are supposed to send a response (TX).

> Also, if you think there's a better way to enable controllers to manage
> their hardware queues, please say.
>
Tying RX to TX has nothing to do with hardware queues. There can be a
hardware queue and the protocol can still be
"local-to-remote-broadcast".

While I don't think we need the "Rx is in relation to some past Tx"
api, I am open to ideas to better utilize h/w queues.

The h/w TX queue/fifo may hold, say, 8 messages while the controller
transmits them to the remote. Currently it is implemented by
.last_tx_done() returning true as long as fifo is not full.
The other option, to have more than one TX in-flight, only complicates
the mailbox api without fetching any real benefits because the
protocol would still need to handle cases like Tx was successful but
the remote rejected the request or the Rx failed on the h/w fifo
(there could be rx-fifo too, right). Is where I am at right now.

Regards,
Jassi