On Wed, Nov 28, 2018 at 3:43 AM Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
>
> On 12/11/2018 15:18, Thierry Reding wrote:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > The mailbox framework supports blocking transfers via completions for
> > clients that can sleep. In order to support blocking transfers in cases
> > where the transmission is not permitted to sleep, add a new ->flush()
> > callback that controller drivers can implement to busy loop until the
> > transmission has been completed. This will automatically be called when
> > available and interrupts are disabled for clients that request blocking
> > transfers.
> >
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> > drivers/mailbox/mailbox.c | 8 ++++++++
> > include/linux/mailbox_controller.h | 4 ++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > index 674b35f402f5..0eaf21259874 100644
> > --- a/drivers/mailbox/mailbox.c
> > +++ b/drivers/mailbox/mailbox.c
> > @@ -267,6 +267,14 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
> > unsigned long wait;
> > int ret;
> >
> > + if (irqs_disabled() && chan->mbox->ops->flush) {
> > + ret = chan->mbox->ops->flush(chan, chan->cl->tx_tout);
> > + if (ret < 0)
> > + tx_tick(chan, ret);
> > +
> > + return ret;
> > + }
>
> It seems to me that if mbox_send_message() is called from an atomic
> context AND tx_block is true, then if 'flush' is not populated this
> should be an error condition as we do not wish to call
> wait_for_completion from an atomic context.
>
> I understand that there is some debate about adding such flush support,
> but irrespective of the above change, it seems to me that if the
> mbox_send_message() can be called from an atomic context (which it
> appears to), then it should be detecting if someone is trying to do so
> with 'tx_block' set as this should be an error.
>
Layers within kernel space have to trust each other. A badly written
client can break the consumer in so many ways, we can not catch every
possibility.
> Furthermore, if the underlying mailbox driver can support sending a
> message from an atomic context and busy wait until it is done, surely
> the mailbox framework should provide a means to support this?
>
Being able to put the message on bus in atomic context is a feature -
which we do support. But busy-waiting in a loop is not a feature, and
we don't want to encourage that.
Cheers!
