Date:   Tue, 08 Jun 2021 13:30:30 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Stephan Gerhold <stephan@...hold.net>
Cc:     Loic Poulain <loic.poulain@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Aleksander Morgado <aleksander@...ksander.es>,
        Network Development <netdev@...r.kernel.org>,
        linux-remoteproc@...r.kernel.org,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Ohad Ben-Cohen <ohad@...ery.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>
Subject: Re: [RFC] Integrate RPMSG/SMD into WWAN subsystem

Stephan Gerhold <stephan@...hold.net> writes:

> On Mon, Jun 07, 2021 at 01:23:18PM +0200, Toke Høiland-Jørgensen wrote:
>> Stephan Gerhold <stephan@...hold.net> writes:
>> > On Mon, Jun 07, 2021 at 11:27:07AM +0200, Loic Poulain wrote:
>> >> On Sat, 5 Jun 2021 at 11:25, Stephan Gerhold <stephan@...hold.net> wrote:
>> >> > On Fri, Jun 04, 2021 at 11:11:45PM +0200, Loic Poulain wrote:
>> >> > > On Wed, 2 Jun 2021 at 20:20, Stephan Gerhold <stephan@...hold.net> wrote:
>> >> > > > I've been thinking about creating some sort of "RPMSG" driver for the
>> >> > > > new WWAN subsystem; this would be used as a QMI/AT channel to the
>> >> > > > integrated modem on some older Qualcomm SoCs such as MSM8916 and MSM8974.
>> >> > > >
>> >> > > > It's easy to confuse all the different approaches that Qualcomm has to
>> >> > > > talk to their modems, so I will first try to briefly give an overview
>> >> > > > about those that I'm familiar with:
>> >> > > >
>> >> > > > ---
>> >> > > > There is USB and MHI that are mainly used to talk to "external" modems.
>> >> > > >
>> >> > > > For the integrated modems in many Qualcomm SoCs there is typically
>> >> > > > a separate control and data path. They are not really related to each
>> >> > > > other (e.g. currently no common parent device in sysfs).
>> >> > > >
>> >> > > > For the data path (network interface) there is "IPA" (drivers/net/ipa)
>> >> > > > on newer SoCs or "BAM-DMUX" on some older SoCs (e.g. MSM8916/MSM8974).
>> >> > > > I have a driver for BAM-DMUX that I hope to finish up and submit soon.
>> >> > > >
>> >> > > > The connection is set up via QMI. The messages are either sent via
>> >> > > > a shared RPMSG channel (net/qrtr sockets in Linux) or via standalone
>> >> > > > SMD/RPMSG channels (e.g. "DATA5_CNTL" for QMI and "DATA1" for AT).
>> >> > > >
>> >> > > > This gives a lot of possible combinations like BAM-DMUX+RPMSG
>> >> > > > (MSM8916, MSM8974), or IPA+QRTR (SDM845) but also other funny
>> >> > > > combinations like IPA+RPMSG (MSM8994) or BAM-DMUX+QRTR (MSM8937).
>> >> > > >
>> >> > > > Simply put, supporting all these in userspace like ModemManager
>> >> > > > is a mess (Aleksander can probably confirm).
>> >> > > > It would be nice if this could be simplified through the WWAN subsystem.
>> >> > > >
>> >> > > > It's not clear to me if or how well QRTR sockets can be mapped to a char
>> >> > > > device for the WWAN subsystem, so for now I'm trying to focus on the
>> >> > > > standalone RPMSG approach (for MSM8916, MSM8974, ...).
>> >> > > > ---
>> >> > > >
>> >> > > > Currently ModemManager uses the RPMSG channels via the rpmsg-chardev
>> >> > > > (drivers/rpmsg/rpmsg_char.c). It wasn't my idea to use it like this,
>> >> > > > I just took that over from someone else. Realistically speaking, the
>> >> > > > current approach isn't too different from the UCI "backdoor interface"
>> >> > > > approach that was rejected for MHI...
>> >> > > >
>> >> > > > I kind of expected that I can just trivially copy some code from
>> >> > > > rpmsg_char.c into a WWAN driver since they both end up as a simple char
>> >> > > > device. But it looks like the abstractions in wwan_core are kind of
>> >> > > > getting in the way here... As far as I can tell, they don't really fit
>> >> > > > together with the RPMSG interface.
>> >> > > >
>> >> > > > For example there is rpmsg_send(...) (blocking) and rpmsg_trysend(...)
>> >> > > > (non-blocking) and even a rpmsg_poll(...) [1] but I don't see a way to
>> >> > > > get notified when the TX queue is full or no longer full so I can call
>> >> > > > wwan_port_txon/off().
>> >> > > >
>> >> > > > Any suggestions or other thoughts?
>> >> > >
>> >> > > It would be indeed nice to get this in the WWAN framework.
>> >> > > I don't know much about rpmsg but I think it is straightforward for
>> >> > > the RX path, the ept_cb can simply forward the buffers to
>> >> > > wwan_port_rx.
>> >> >
>> >> > Right, that part should be straightforward.
>> >> >
>> >> > > For tx, simply call rpmsg_trysend() in the wwan tx
>> >> > > callback and don't use the txon/off helpers. In short, keep it simple
>> >> > > and check if you observe any issues.
>> >> > >
>> >> >
>> >> > I'm not sure that's a good idea. This sounds like exactly the kind of
>> >> > thing that might explode later just because I don't manage to get the
>> >> > TX queue full in my tests. In that case, writing to the WWAN char dev
>> >> > would not block, even if O_NONBLOCK is not set.
>> >> 
>> >> Right, if you think it could be a problem, you can always implement a
>> >> more complex solution like calling rpmsg_send from a
>> >> workqueue/kthread, and only re-enable tx once rpmsg_send returns.
>> >> 
>> >
>> > I did run into trouble when I tried to stream lots of data into the WWAN
>> > char device (e.g. using dd). However, in practice (with ModemManager) 
>> > I did not manage to cause such issues yet. Personally, I think it's
>> > something we should get right, just to avoid trouble later
>> > (like "modem suddenly stops working").
>> >
>> > Right now I extended the WWAN port ops a bit so I tells me if the write
>> > should be non-blocking or blocking and so I can call rpmsg_poll(...).
>> >
>> > But having some sort of workqueue also sounds like it could work quite
>> > well, thanks for the suggestion! Will think about it some more, or
>> > I might post what I have right now so you can take a look.
>> 
>> How big are those hardware TXQs? Just pushing packets to the hardware
>> until it overflows sounds like a recipe for absolutely terrible
>> bufferbloat... That would be bad!
>> 
>
> For reference, we're not really talking about "hardware" TXQs here.
> As far as I understand, the RPMSG channels on Qualcomm devices are
> mostly just a firmware convention for communicating between different
> CPUs/DSPs via shared memory.
>
> The packets are copied into some kind of shared FIFO/ring buffer
> per channel, with varying sizes. On my test device, the firmware
> allcates 1024 bytes for the QMI channel and 8192 bytes
> for the AT channel.
>
> I'm not sure how this would cause any kind of overflow/bufferbloat.
> The remote side (e.g. modem DSP) is notified separately for every packet
> that is sent. If we're really writing more quickly than the remote side
> will read, rpmsg_send() will block and therefore the client will
> block as well (since tx was disabled before calling rpmsg_send()).

Hmm, okay, if this is just control channel traffic and the buffers are
no bigger than that maybe this is not such a huge issue. As long as the
client (which I guess is whichever application is trying to control the
modem?) can block and back off, so it won't just keep queuing up
commands faster than the modem can process them...

-Toke

Powered by blists - more mailing lists