linux-kernel - Re: [PATCH v2 1/2] mailbox: check mailbox queue is full or not

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] [day] [month] [year] [list]

Message-ID: <c9f65597-52ba-41ef-842a-2569c2074d6f@amd.com>
Date: Tue, 18 Nov 2025 12:51:08 -0600
From: Tanmay Shah <tanmay.shah@....com>
To: Jassi Brar <jassisinghbrar@...il.com>
CC: <andersson@...nel.org>, <mathieu.poirier@...aro.org>,
	<linux-kernel@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] mailbox: check mailbox queue is full or not



On 11/15/25 11:38 AM, Jassi Brar wrote:
> On Wed, Oct 15, 2025 at 10:27 AM Tanmay Shah <tanmay.shah@....com> wrote:
>>>>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>>>>> index 5cd8ae222073..c2e187aa5d22 100644
>>>>> --- a/drivers/mailbox/mailbox.c
>>>>> +++ b/drivers/mailbox/mailbox.c
>>>>> @@ -35,6 +35,7 @@ static int add_to_rbuf(struct mbox_chan *chan, void
>>>>> *mssg)
>>>>>           idx = chan->msg_free;
>>>>>           chan->msg_data[idx] = mssg;
>>>>>           chan->msg_count++;
>>>>> +       chan->msg_slot_ro = (MBOX_TX_QUEUE_LEN - chan->msg_count);
>>>>>
>>>>>           if (idx == MBOX_TX_QUEUE_LEN - 1)
>>>>>                   chan->msg_free = 0;
>>>>> @@ -70,6 +71,7 @@ static void msg_submit(struct mbox_chan *chan)
>>>>>                   if (!err) {
>>>>>                           chan->active_req = data;
>>>>>                           chan->msg_count--;
>>>>> +                       chan->msg_slot_ro = (MBOX_TX_QUEUE_LEN -
>>>>> chan->msg_count);
>>>>>
>>>> No, I had suggested adding this info in client structure.
>>>> There is no point in carrying msg_count and msg_slot_ro in mbox_chan.
>>>> The client needs this info but can/should not access the chan internals.
>>>>
>>>
>>> Hi Jassi,
>>>
>>> If I move msg_slot_ro to mbox_client that means, each channel needs its
>>> own client structure. But it is possible to map single client to
>>> multiple channels.
>>>
>>> How about if I rename msg_slot_ro to msg_slot_tx_ro -> that says this
>>> field should be used only for "tx" channel.
>>>
>>> Or is it must to map unique client data structure to each channel? If
>>> so, can I update mbox_client structure documentation ?
>>>
>>
>> Hi Jassi,
>>
>> I looked into this further. Looks like it's not possible to introduce
>> msg_slot_ro in the client data structure as of today. Because multiple
>> drivers are sharing same client for "tx" and "rx" both channels.
>> [references: 1, 2, 3]
>>
>> so, msg_slot_ro won't be calculated correctly I believe.
>>
> I don't see it. Can you please explain how the calculated value could be wrong?
> 

Hi Jassi,

so on my platform I introduced some extra logs:

[   80.827479] mbox chan Rx send message
[   80.827485] add_to_rbuf: &chan->cl->msg_slot_ro = 00000000ab5fa771, 
msg slot ro = 19
[   80.827494] msg_submit: &chan->cl->msg_slot_ro = 00000000ab5fa771, 
msg slot ro = 20
[   80.833064] mbox chan Tx send message
[   80.833070] add_to_rbuf: &chan->cl->msg_slot_ro = 00000000ab5fa771, 
msg slot ro = 19
[   80.833079] msg_submit: &chan->cl->msg_slot_ro = 00000000ab5fa771, 
msg slot ro = 20
[   80.837486] mbox chan Rx send message
[   80.837492] add_to_rbuf: &chan->cl->msg_slot_ro = 00000000ab5fa771, 
msg slot ro = 19
[   80.837501] msg_submit: &chan->cl->msg_slot_ro = 00000000ab5fa771, 
msg slot ro = 20

Tx and Rx both channels are updating same address of msg_slot_ro. If 
user wants to check msg_slot_ro for Tx channel, then it is possible that 
it was updated by Rx channel instead. Ideally there should be two copies 
of msg_slot_ro, one for Tx and one for Rx channel.

This happens because Tx and Rx both channels shares same client data 
structure.

Same can happen on other platforms as well.

Thanks,
Tanmay

> thnx
> -jassi