linux-kernel - Re: [PATCH] mailbox/bcm2835: Fix mailbox full detection.

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: <CABb+yY2rEG4Fh8Me_ktvzO=uOJcu3dAcxC-VfPgLQ8caDjjq7A@mail.gmail.com>
Date:	Fri, 29 May 2015 12:36:22 +0530
From:	Jassi Brar <jassisinghbrar@...il.com>
To:	Eric Anholt <eric@...olt.net>
Cc:	Stephen Warren <swarren@...dotorg.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-rpi-kernel@...ts.infradead.org
Subject: Re: [PATCH] mailbox/bcm2835: Fix mailbox full detection.

On Fri, May 29, 2015 at 3:16 AM, Eric Anholt <eric@...olt.net> wrote:
> Stephen Warren <swarren@...dotorg.org> writes:
>
>> On 05/13/2015 02:10 PM, Eric Anholt wrote:
>>> With the VC reader blocked and the ARM writing, MAIL0_STA reads empty
>>> permanently while MAIL1_STA goes from empty (0x40000000) to non-empty
>>> (0x00000001-0x00000007) to full (0x80000008).
>>>
>>> This bug ended up having no effect on us, because all of our
>>> transactions in the client driver were synchronous and under a mutex.
>>
>> If you could get someone at the RPi Foundation or Broadcom to update the
>> register descriptions and example code at the following URLs, that would
>> be rather useful. Otherwise, this code will appear incorrect when
>> compared against the documentation:
>>
>> https://github.com/raspberrypi/firmware/wiki/Mailboxes
>> ("Mailbox registers" at the bottom)
>>
>> https://github.com/raspberrypi/firmware/wiki/Accessing-mailboxes
>> ("Sample code")
>
> Since it's a wiki, I went ahead and edited the first one.  Hopefully
> that clarifies how the c++ in the other page is supposed to be used.
>
>>> diff --git a/drivers/mailbox/bcm2835-mailbox.c b/drivers/mailbox/bcm2835-mailbox.c
>>
>>> @@ -117,7 +118,7 @@ static bool bcm2835_last_tx_done(struct mbox_chan *link)
>>>      bool ret;
>>>
>>>      spin_lock(&mbox->lock);
>>> -    ret = !(readl(mbox->regs + MAIL0_STA) & ARM_MS_FULL);
>>> +    ret = !(readl(mbox->regs + MAIL1_STA) & ARM_MS_FULL);
>>
>> What does "tx done" mean semantically?
>>
>> If "tx done" means "remote side received all our messages", then surely
>> this should check MAIL1_STA for emptiness, which is different to the
>> "not full" check implemented here?
>>
Mailbox core queues messages internally. The controller driver is
provided one message at a time.

>> If "tx done" means "there's space to transmit more messages", then
>> consider this:
>
> The mailbox core appears to use this hook as "there's space to transmit
> more messages."  The name does seem really confusing.
>
'tx_done'  => 'is tx done' / 'tx is done'.  'tx' is the last submitted
message to be transferred.
So 'tx_done' marks if the last transmitted message was received by the
remote. That event could be indicated by some register's bit cleared
or even a message(ack) returned by remote (client does
mbox_client_txdone).  And if last tx was done, we should be able to
send next message.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/