linux-kernel - Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support

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+yY3uJMxcPgV4agGOd447ETe5UF-EqKWadvgfzy_QzzeGCQ@mail.gmail.com>
Date:   Wed, 31 Aug 2016 14:15:29 +0530
From:   Jassi Brar <jassisinghbrar@...il.com>
To:     Horng-Shyang Liao <hs.liao@...iatek.com>
Cc:     Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Daniel Kurtz <djkurtz@...omium.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Devicetree List <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        linux-mediatek@...ts.infradead.org, srv_heupstream@...iatek.com,
        Sascha Hauer <kernel@...gutronix.de>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Nicolas Boichat <drinkcat@...omium.org>,
        CK HU <ck.hu@...iatek.com>,
        cawa cheng <cawa.cheng@...iatek.com>,
        Bibby Hsieh <bibby.hsieh@...iatek.com>,
        YT Shen <yt.shen@...iatek.com>,
        Daoyuan Huang <daoyuan.huang@...iatek.com>,
        Damon Chu <damon.chu@...iatek.com>,
        Josh-YC Liu <josh-yc.liu@...iatek.com>,
        Glory Hung <glory.hung@...iatek.com>,
        Jiaguang Zhang <jiaguang.zhang@...iatek.com>,
        Dennis-YC Hsieh <dennis-yc.hsieh@...iatek.com>,
        Monica Wang <monica.wang@...iatek.com>
Subject: Re: [PATCH v13 0/4] Mediatek MT8173 CMDQ support

On Wed, Aug 31, 2016 at 1:43 PM, Horng-Shyang Liao <hs.liao@...iatek.com> wrote:

>> >> The driver uses the mailbox framework, so it should live in the
>> >> drivers/mailbox folder.
>> >
>> > As you know, the maximum number of gce threads is 16.
>> > However, we plan to support more clients in the future,
>> > and they may need to use more than 16 gce threads.
>> >
>> > For this issue, our plan is to let multiple clients share the same gce
>> > thread; i.e. we will acquire gce thread for client dynamically by
>> > internal policy in cmdq driver.
>> > Unfortunately. mailbox channel has exclusive feature.
>> > Quote from comment of mbox_request_channel().
>> > "The channel is exclusively allocated and can't be used by another
>> > client before the owner calls mbox_free_channel."
>> > Therefore, we plan to remove mailbox framework from cmdq driver in the
>> > future.
>> >
>> Platforms that need shared access to a channel, implement a 'server'
>> driver that serialise (which is needed still) the access to common
>> channel. If you think you don't need mutual exclusion and don't care
>> about replies, simply share the mailbox handle among different
>> clients.
>
> Thank you for your kindly reply.
> We would like to discuss further with you on this topic.
>
> Our requirement is
> (1) cmdq task cannot be split, and
> (2) cmdq thread can have multiple cmdq tasks from different clients.
>
> According to your comment "implement a 'server' driver that serialise
> the access to common channel", do you mean we should implement cmdq
> client (mailbox client) as a server and other clients call the functions
> of cmdq client?
>
> clients --> cmdq client (mailbox client) --> cmdq (mailbox controller)
>
> If so, could you please tell us the benefit of using mailbox framework?
>
You don't have to reinvent 80% of the wheel and reuse the mailbox.c
core that supports many features and is tested on many platforms. Your
implementation is going to be quite similar, only you clump all the
code in one file and you use different terminology.

You said "we will acquire gce thread for client dynamically by
internal policy in cmdq driver"
On mailbox api, this maps to simply sharing the channel/thread handle,
protected by a lock, among clients on some basis (like FCFS or
whatever you internal policy is). So your server driver could be very
thin. And all your clients could follow the mailbox api (which is good
from the point of reusability/portability).

> Our original plan is to let cmdq driver manage cmdq thread internally.
> Cmdq driver can choose a suitable cmdq thread to execute a flushed cmdq
> task dynamically, and client doesn't need to know the existence of cmdq
> thread.
>
>
> Could you also please tell us the purpose of putting all mailbox
> driver into mailbox folder?
> We know that some other drivers also follow this rule, and just want
> to know more details.
>
Any driver that implements the Mailbox API should live in
drivers/mailbox/.  And why you should implement mailbox api, is
explained above.