linux-kernel - Re: [PATCH v5 1/3] dt-bindings: mailbox: add google,gs101-mbox

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: <7dc02926-905e-430d-91f5-e1ad7af7135e@linaro.org>
Date: Mon, 13 Jan 2025 09:34:17 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Jassi Brar <jassisinghbrar@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
 Alim Akhtar <alim.akhtar@...sung.com>, linux-kernel@...r.kernel.org,
 linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, andre.draszik@...aro.org,
 peter.griffin@...aro.org, kernel-team@...roid.com, willmcvicker@...gle.com,
 daniel.lezcano@...aro.org, vincent.guittot@...aro.org,
 ulf.hansson@...aro.org, arnd@...db.de
Subject: Re: [PATCH v5 1/3] dt-bindings: mailbox: add google,gs101-mbox



On 1/12/25 4:59 PM, Jassi Brar wrote:

>>>>> Then I updated the mailbox core to allow clients to request channels by
>>>>> passing some args containing channel identifiers to the controllers,
>>>>> that the controllers xlate() using their own method.
>>>>>
>>>> This is unnecessary.
>>>> If you don't pass the doorbell number from DT, each channel populated
>>>> by the driver is just a s/w construct or a 'virtual' channel. Make use
>>>> of 'void *data'  in send_data() to specify the doorbell.
>>>>
>>> I think this introduces concurrency problems if the channel identifiers
>>> passed by 'void *data' don't match the virtual channel used for sending
>>> the messages. Do we want to allow this?
>>>
>>> Also, if we use 'void *data' to pass channel identifiers, the channel
>>> checks will have to be made at send_data() time. Thus if passing wrong
>>> channel type for example, the mailbox client will eventually get a
>>> -ENOBUFS and a "Try increasing MBOX_TX_QUEUE_LEN" message, which I find
>>> misleading.
>>
>> Shall I still use 'void *data' to pass channel identifiers through
>> send_data()? I'd like to respin everything.
>>
> Yes, please do.
> 

What shall I do in driver's of_xlate method, always return
&mbox->chans[0], as bcm2835 does? All 14 doorbels will be serialized
with mobox->chans[0].lock.

I could use a list of channels in the controller and provide some
get/put channel methods, but the virtual channel ID will not match the
actual channel ID that's used for communication. I'll also need to
introduce channel ops, to put the channel that was acquired via of_xlate
from the list of available channels.

Aren't we better off with the mbox_request_channel_by_args() that I
introduced in v6? Or if you think there's better option I'll be happy to
implement it. I need an agreement on the overall idea.

Thanks,
ta