Re: [PATCH 0/6] In-kernel QMI handling

[Dan Williams]

On Tue, 2017-08-08 at 15:42 -0700, Bjorn Andersson wrote:
> On Tue 08 Aug 04:02 PDT 2017, Bj?rn Mork wrote:
> 
> > Bjorn Andersson  writes:
> > 
> > > This series starts by moving the common definitions of the QMUX
> > > protocol to the
> > > uapi header, as they are shared with clients - both in kernel and
> > > userspace.
> > > 
> > > This series then introduces in-kernel helper functions for aiding
> > > the handling
> > > of QMI encoded messages in the kernel. QMI encoding is a wire-
> > > format used in
> > > exchanging messages between the majority of QRTR clients and
> > > services.
> > 
> > Interesting!  I tried to add some QMI handling in the kernel a few
> > years
> > ago, but was thankfully voted down.  See
> > https://www.spinics.net/lists/netdev/msg183101.html and the
> > following
> > discussion. I am convinced that was the right decision, for the
> > client
> > side at least. The protocol is just too extensive and ever-growing
> > to be
> > implemented in the kernel. We would be catching up forever.
> > 
> > Note that I had very limited knowledge of the protocol at the time
> > I
> > wrote that driver.  Still have, in fact :-)
> > 
> 
> Thanks for the pointer, I definitely think there's more work to be
> done
> here to figure out the proper way to interact with these devices.
> 
> But I think that Dan's reply shows a huge source of confusion here;
> the
> acronym "QMI" covers a large amount of different things - and means
> different things for different people.

I would agree, sorry for any confusion caused.  Great discussion so
far.

> In the modem world QMI seems to mean a defined set of logical
> endpoints
> that accepts TLV-encoded messages to do modem-related things. But the
> TLV-encoding is used for non-modem related services and the only
> common
> denominator of everything called QMI is the TLV-encoding.
> 
> 
> Due to my limited exposure to the USB attached "QMI thingies" I
> haven't
> previously looked into the exact differences. The proposed patches
> aimed
> to support implementing a few non-modem-related clients using
> QMI-encoded messages over ipcrouter.
> 
> Looking at your patch above, and oPhono, seems to highlight a few
> important differences that will take some thinking to overcome.
> 
> = Transport
> The transport header in the USB case is your struct qmux, which
> contains
> the type of message (in "flags") and the transaction id. The
> "service"
> in the QMUX header matches the service id being communicated with.
> But
> in order to communicate with a service it seems like one requests a
> client-id from the control service.

Correct.  You cannot talk to a service on the modem without getting an
allocated client ID from the CTL service, which has a well-defined
client ID.

> In the smartphone world (with shared memory communication) the
> transport
> is ipcrouter - with a header very similar to UDP - and there's no
> information about the payload, it provides only the means of
> delivering

Can you explain a bit about the relationship of SMD to [I/R]PC, qrtr,
and QMI?  A couple years ago there was smd_qmi.c (like for the Nexus 4
with APQ8064 and a discrete MDM9215) which from a 10 minute fresh look
appears to just push QMUX+QMI via SMD rather than being backed by the
RPC/IPC stuff.  I could be wrong, there's a lot of indirection there
and it may well end up going over the router.  But that's buried deeper
than a 10m look for me.

Is it perhaps only with on-chip blocks where the QMUX/QMI/qrtr/irpc
stuff you describe here is used?  If so, perhaps that's the distinction
to be made.  I'll let you correct me here since you clearly know more
than I about the internals of these devices.

> messages from one address/port to another address/port. A typical
> smartphone has 3-4 nodes (modem, sensors, audio etc) and ports are
> dynamically allocated. The control messages in the QMUX protocol (not
> the same QMUX protocol as in the USB case!) are used for clients to
> find
> the mapping from service id to a port on the given address.  The
> source
> port is dynamically allocated in this case.
> 
> = QMI-encoded messages
> The list of TLV-entries have a "QMI header" prepended in both cases,
> but
> in the QMUX case the header consists only of "msgid" and length.
> 
> In the ipcrouter case the transport doesn't carry any information
> regarding the payload, so the header prepended the TLV entries
> includes
> "type", transaction id, "msg_id" and length.

I'll assume that in this case, because the client has already found out
how to contact the target service directly, that it has no use for a
"fat" QMUX header that includes the client ID and service stuff.

I don't really have an issue with the kernel doing "thin" QMUX-related
stuff.  That's pretty simple.

> It looks as if once past the differences in the transport and QMI
> message header the messages (TLV-encoded data) are the same. But I'm
> not
> yet sure about how we can hide the transport 

Re: [PATCH 0/6] In-kernel QMI handling

[Bjorn Andersson]

On Tue 08 Aug 04:02 PDT 2017, Bj?rn Mork wrote:

> Bjorn Andersson  writes:
> 
> > This series starts by moving the common definitions of the QMUX protocol to 
> > the
> > uapi header, as they are shared with clients - both in kernel and userspace.
> >
> > This series then introduces in-kernel helper functions for aiding the 
> > handling
> > of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
> > exchanging messages between the majority of QRTR clients and services.
> 
> Interesting!  I tried to add some QMI handling in the kernel a few years
> ago, but was thankfully voted down.  See
> https://www.spinics.net/lists/netdev/msg183101.html and the following
> discussion. I am convinced that was the right decision, for the client
> side at least. The protocol is just too extensive and ever-growing to be
> implemented in the kernel. We would be catching up forever.
> 
> Note that I had very limited knowledge of the protocol at the time I
> wrote that driver.  Still have, in fact :-)
> 

Thanks for the pointer, I definitely think there's more work to be done
here to figure out the proper way to interact with these devices.

But I think that Dan's reply shows a huge source of confusion here; the
acronym "QMI" covers a large amount of different things - and means
different things for different people.

In the modem world QMI seems to mean a defined set of logical endpoints
that accepts TLV-encoded messages to do modem-related things. But the
TLV-encoding is used for non-modem related services and the only common
denominator of everything called QMI is the TLV-encoding.


Due to my limited exposure to the USB attached "QMI thingies" I haven't
previously looked into the exact differences. The proposed patches aimed
to support implementing a few non-modem-related clients using
QMI-encoded messages over ipcrouter.

Looking at your patch above, and oPhono, seems to highlight a few
important differences that will take some thinking to overcome.

= Transport
The transport header in the USB case is your struct qmux, which contains
the type of message (in "flags") and the transaction id. The "service"
in the QMUX header matches the service id being communicated with. But
in order to communicate with a service it seems like one requests a
client-id from the control service.

In the smartphone world (with shared memory communication) the transport
is ipcrouter - with a header very similar to UDP - and there's no
information about the payload, it provides only the means of delivering
messages from one address/port to another address/port. A typical
smartphone has 3-4 nodes (modem, sensors, audio etc) and ports are
dynamically allocated. The control messages in the QMUX protocol (not
the same QMUX protocol as in the USB case!) are used for clients to find
the mapping from service id to a port on the given address.  The source
port is dynamically allocated in this case.

= QMI-encoded messages
The list of TLV-entries have a "QMI header" prepended in both cases, but
in the QMUX case the header consists only of "msgid" and length.

In the ipcrouter case the transport doesn't carry any information
regarding the payload, so the header prepended the TLV entries includes
"type", transaction id, "msg_id" and length.


It looks as if once past the differences in the transport and QMI
message header the messages (TLV-encoded data) are the same. But I'm not
yet sure about how we can hide the transport differences.

Regards,
Bjorn

Re: [PATCH 0/6] In-kernel QMI handling

[Marcel Holtmann]

Hi Bjorn,

>> This series starts by moving the common definitions of the QMUX protocol to 
>> the
>> uapi header, as they are shared with clients - both in kernel and userspace.
>> 
>> This series then introduces in-kernel helper functions for aiding the 
>> handling
>> of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
>> exchanging messages between the majority of QRTR clients and services.
> 
> Interesting!  I tried to add some QMI handling in the kernel a few years
> ago, but was thankfully voted down.  See
> https://www.spinics.net/lists/netdev/msg183101.html and the following
> discussion. I am convinced that was the right decision, for the client
> side at least. The protocol is just too extensive and ever-growing to be
> implemented in the kernel. We would be catching up forever.

I think that even back then I said, that it has to be done as a proper 
subsystem if it has a chance to be in the kernel. So something similar to 
Phonet and CAIF where the service registration is handled by the kernel, but 
applications can be fully in userspace. None of this is actually brand new 
Qualcomm design since Nokia has had its Phonet long before QMI existed.

The real importance is that Qualcomm is behind this and wants to get this done 
a clean way with a proper API. The /dev/qmi thing was a pretty broken 
interface. Any subsystem has to support multiple QMI devices. Even if this is 
unlikely in a phone design, it has to be supported so that attaching two USB 
QMI based dongles does not end up with some pointless errors.

Regards

Marcel

Re: [PATCH 0/6] In-kernel QMI handling

[Bjørn Mork]

Bjorn Andersson  writes:

> This series starts by moving the common definitions of the QMUX protocol to 
> the
> uapi header, as they are shared with clients - both in kernel and userspace.
>
> This series then introduces in-kernel helper functions for aiding the handling
> of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
> exchanging messages between the majority of QRTR clients and services.

Interesting!  I tried to add some QMI handling in the kernel a few years
ago, but was thankfully voted down.  See
https://www.spinics.net/lists/netdev/msg183101.html and the following
discussion. I am convinced that was the right decision, for the client
side at least. The protocol is just too extensive and ever-growing to be
implemented in the kernel. We would be catching up forever.

Note that I had very limited knowledge of the protocol at the time I
wrote that driver.  Still have, in fact :-)


Bjørn

Re: [PATCH 0/6] In-kernel QMI handling

[Marcel Holtmann]

Hi Bjorn,

> This series starts by moving the common definitions of the QMUX
> protocol to the
> uapi header, as they are shared with clients - both in kernel and
> userspace.
> 
> This series then introduces in-kernel helper functions for aiding the
> handling
> of QMI encoded messages in the kernel. QMI encoding is a wire-format
> used in
> exchanging messages between the majority of QRTR clients and
> services.
 
 This raises a few red-flags for me.
>>> 
>>> I'm glad it does. In discussions with the responsible team within
>>> Qualcomm I've highlighted a number of concerns about enabling this
>>> support in the kernel. Together we're continuously looking into what
>>> should be pushed out to user space, and trying to not introduce
>>> unnecessary new users.
>>> 
 So far, we've kept almost everything QMI related in userspace and
 handled all QMI control-channel messages from libraries like libqmi or
 uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
 driver.  The kernel drivers just serve as the transport.
 
>>> 
>>> The path that was taken to support the MSM-style devices was to
>>> implement net/qrtr, which exposes a socket interface to abstract the
>>> physical transports (QMUX or IPCROUTER in Qualcomm terminology).
>>> 
>>> As I share you view on letting the kernel handle the transportation only
>>> the task of keeping track of registered services (service id -> node and
>>> port mapping) was done in a user space process and so far we've only
>>> ever have to deal with QMI encoded messages in various user space tools.
>> 
>> I think that the transport and multiplexing can be in the kernel as
>> long as it is done as proper subsystem. Similar to Phonet or CAIF.
>> Meaning it should have a well defined socket interface that can be
>> easily used from userspace, but also a clean in-kernel interface
>> handling.
>> 
> 
> In a mobile Qualcomm device there's a few different components involved
> here: message routing, QMUX protocol and QMI-encoding.
> 
> The downstream Qualcomm kernel implements the two first in the
> IPCROUTER, upstream this is split between the kernel net/qrtr and a user
> space service-register implementing the QMUX protocol for knowing where
> services are located.

as long as all of QMUX moves into the kernel and userspace doesn’t need to know 
about QMUX anymore, that would be good. The cross termination of QMUX in kernel 
space and userspace is a really bad idea. It is even worse if userspace has to 
do service registration. That is just a recipe for disaster.

One extra thing to keep in mind is that all the USB dongle should register with 
such a new QMI subsystem. And have their network interfaces being proper 
children of the QMI node. And please do not forget QMI passthrough via MBIM. 
Just saying we move some QMUX code into the kernel is not enough. It really 
needs to be a proper subsystem with a proper hierarchy of the child devices.

> The common encoding of messages passed between endpoints of the message
> routing is QMI, which is made an affair totally that of each client.
> 
>> If Qualcomm is supportive of this effort and is willing to actually
>> assist and/or open some of the specs or interface descriptions, then
>> this is a good thing. Service registration and cleanup is really done
>> best in the kernel. Same applies to multiplexing. Trying to do
>> multiplexing in userspace is always cumbersome and leads to overhead
>> that is of no gain. For example within oFono, we had to force
>> everything to go via oFono since it was the only sane way of handling
>> it. Other approaches were error prone and full of race conditions. You
>> need a central entity that can clean up.
>> 
> 
> The current upstream solution depends on a collaboration between
> net/qrtr and the user space service register for figuring out whom to
> send messages to. After that muxing et al is handled by the socket
> interface and service registry does not need to be involved.
> 
> Qualcomm is very supporting of this solution and we're collaborating on
> transitioning "downstream" to use this implementation.

It would be good if someone looks into oFono and makes sure that it works there 
as well. I would prefer at least some initial patches to proof-point the kernel 
APIs. oFono is a full telephony stack. So if you can make that one work, then 
you are most likely on the right track.

>> For the definition of an UAPI to share some code, I am actually not
>> sure that is such a good idea. For example the QMI code in oFono
>> follows a way simpler approach. And I am not convinced that all the
>> macros are actually beneficial. For example, the whole netlink macros
>> are pretty cumbersome. Adding some Documentation/qmi.txt on how the
>> wire format looks like and what is expected seems to be a way better
>> approach.
>> 
> 
> The socket interface provided by the kernel expects some knowledge of
> the QMUX 

Re: [PATCH 0/6] In-kernel QMI handling

[Bjorn Andersson]

On Mon 07 Aug 12:19 PDT 2017, Marcel Holtmann wrote:

> Hi Bjorn,
> 
> >>> This series starts by moving the common definitions of the QMUX
> >>> protocol to the
> >>> uapi header, as they are shared with clients - both in kernel and
> >>> userspace.
> >>> 
> >>> This series then introduces in-kernel helper functions for aiding the
> >>> handling
> >>> of QMI encoded messages in the kernel. QMI encoding is a wire-format
> >>> used in
> >>> exchanging messages between the majority of QRTR clients and
> >>> services.
> >> 
> >> This raises a few red-flags for me.
> > 
> > I'm glad it does. In discussions with the responsible team within
> > Qualcomm I've highlighted a number of concerns about enabling this
> > support in the kernel. Together we're continuously looking into what
> > should be pushed out to user space, and trying to not introduce
> > unnecessary new users.
> > 
> >> So far, we've kept almost everything QMI related in userspace and
> >> handled all QMI control-channel messages from libraries like libqmi or
> >> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
> >> driver.  The kernel drivers just serve as the transport.
> >> 
> > 
> > The path that was taken to support the MSM-style devices was to
> > implement net/qrtr, which exposes a socket interface to abstract the
> > physical transports (QMUX or IPCROUTER in Qualcomm terminology).
> > 
> > As I share you view on letting the kernel handle the transportation only
> > the task of keeping track of registered services (service id -> node and
> > port mapping) was done in a user space process and so far we've only
> > ever have to deal with QMI encoded messages in various user space tools.
> 
> I think that the transport and multiplexing can be in the kernel as
> long as it is done as proper subsystem. Similar to Phonet or CAIF.
> Meaning it should have a well defined socket interface that can be
> easily used from userspace, but also a clean in-kernel interface
> handling.
> 

In a mobile Qualcomm device there's a few different components involved
here: message routing, QMUX protocol and QMI-encoding.

The downstream Qualcomm kernel implements the two first in the
IPCROUTER, upstream this is split between the kernel net/qrtr and a user
space service-register implementing the QMUX protocol for knowing where
services are located.

The common encoding of messages passed between endpoints of the message
routing is QMI, which is made an affair totally that of each client.

> If Qualcomm is supportive of this effort and is willing to actually
> assist and/or open some of the specs or interface descriptions, then
> this is a good thing. Service registration and cleanup is really done
> best in the kernel. Same applies to multiplexing. Trying to do
> multiplexing in userspace is always cumbersome and leads to overhead
> that is of no gain. For example within oFono, we had to force
> everything to go via oFono since it was the only sane way of handling
> it. Other approaches were error prone and full of race conditions. You
> need a central entity that can clean up.
> 

The current upstream solution depends on a collaboration between
net/qrtr and the user space service register for figuring out whom to
send messages to. After that muxing et al is handled by the socket
interface and service registry does not need to be involved.

Qualcomm is very supporting of this solution and we're collaborating on
transitioning "downstream" to use this implementation.

> For the definition of an UAPI to share some code, I am actually not
> sure that is such a good idea. For example the QMI code in oFono
> follows a way simpler approach. And I am not convinced that all the
> macros are actually beneficial. For example, the whole netlink macros
> are pretty cumbersome. Adding some Documentation/qmi.txt on how the
> wire format looks like and what is expected seems to be a way better
> approach.
> 

The socket interface provided by the kernel expects some knowledge of
the QMUX protocol, for service management. The majority of this
knowledge is already public, but I agree that it would be good to gather
this in a document. The common data structure for the control message is
what I've put in the uapi, as this is used by anyone dealing with
control messages.

When it comes to the QMI-encoded messages these are application
specific, just like e.g. protobuf definitions are application specific.

As the core infrastructure is becoming available upstream and boards
like the DB410c and DB820c aim to be supported by open solutions we will
have a natural place to discuss publication of at least some of the
application level protocols.

Regards,
Bjorn

Re: [PATCH 0/6] In-kernel QMI handling

[Marcel Holtmann]

Hi Bjorn,

>>> This series starts by moving the common definitions of the QMUX
>>> protocol to the
>>> uapi header, as they are shared with clients - both in kernel and
>>> userspace.
>>> 
>>> This series then introduces in-kernel helper functions for aiding the
>>> handling
>>> of QMI encoded messages in the kernel. QMI encoding is a wire-format
>>> used in
>>> exchanging messages between the majority of QRTR clients and
>>> services.
>> 
>> This raises a few red-flags for me.
> 
> I'm glad it does. In discussions with the responsible team within
> Qualcomm I've highlighted a number of concerns about enabling this
> support in the kernel. Together we're continuously looking into what
> should be pushed out to user space, and trying to not introduce
> unnecessary new users.
> 
>> So far, we've kept almost everything QMI related in userspace and
>> handled all QMI control-channel messages from libraries like libqmi or
>> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
>> driver.  The kernel drivers just serve as the transport.
>> 
> 
> The path that was taken to support the MSM-style devices was to
> implement net/qrtr, which exposes a socket interface to abstract the
> physical transports (QMUX or IPCROUTER in Qualcomm terminology).
> 
> As I share you view on letting the kernel handle the transportation only
> the task of keeping track of registered services (service id -> node and
> port mapping) was done in a user space process and so far we've only
> ever have to deal with QMI encoded messages in various user space tools.

I think that the transport and multiplexing can be in the kernel as long as it 
is done as proper subsystem. Similar to Phonet or CAIF. Meaning it should have 
a well defined socket interface that can be easily used from userspace, but 
also a clean in-kernel interface handling.

If Qualcomm is supportive of this effort and is willing to actually assist 
and/or open some of the specs or interface descriptions, then this is a good 
thing. Service registration and cleanup is really done best in the kernel. Same 
applies to multiplexing. Trying to do multiplexing in userspace is always 
cumbersome and leads to overhead that is of no gain. For example within oFono, 
we had to force everything to go via oFono since it was the only sane way of 
handling it. Other approaches were error prone and full of race conditions. You 
need a central entity that can clean up.

For the definition of an UAPI to share some code, I am actually not sure that 
is such a good idea. For example the QMI code in oFono follows a way simpler 
approach. And I am not convinced that all the macros are actually beneficial. 
For example, the whole netlink macros are pretty cumbersome. Adding some 
Documentation/qmi.txt on how the wire format looks like and what is expected 
seems to be a way better approach.

Regards

Marcel

Re: [PATCH 0/6] In-kernel QMI handling

[Bjorn Andersson]

On Fri 04 Aug 08:36 PDT 2017, Dan Williams wrote:

> On Fri, 2017-08-04 at 07:59 -0700, Bjorn Andersson wrote:
> > This series starts by moving the common definitions of the QMUX
> > protocol to the
> > uapi header, as they are shared with clients - both in kernel and
> > userspace.
> > 
> > This series then introduces in-kernel helper functions for aiding the
> > handling
> > of QMI encoded messages in the kernel. QMI encoding is a wire-format
> > used in
> > exchanging messages between the majority of QRTR clients and
> > services.
> 
> This raises a few red-flags for me.

I'm glad it does. In discussions with the responsible team within
Qualcomm I've highlighted a number of concerns about enabling this
support in the kernel. Together we're continuously looking into what
should be pushed out to user space, and trying to not introduce
unnecessary new users.

> So far, we've kept almost everything QMI related in userspace and
> handled all QMI control-channel messages from libraries like libqmi or
> uqmi via the cdc-wdm driver and the "rmnet" interface via the qmi_wwan
> driver.  The kernel drivers just serve as the transport.
> 

The path that was taken to support the MSM-style devices was to
implement net/qrtr, which exposes a socket interface to abstract the
physical transports (QMUX or IPCROUTER in Qualcomm terminology).

As I share you view on letting the kernel handle the transportation only
the task of keeping track of registered services (service id -> node and
port mapping) was done in a user space process and so far we've only
ever have to deal with QMI encoded messages in various user space tools.

> Can you describe what kinds of in-kernel drivers need to actually parse
> QMI messages as part of their operation?
> 

= "sysmon"
sysmon is a mechanism in Qualcomm devices to notify remote processors
when other remote processors goes away. This is required for such things
as letting the modem firmware know that the audio path is gone in the
case that the adsp abruptly disappears - i.e. when the adsp hits a
watchdog bite and doesn't have a chance to close the modem<->audio
communication channels.

In previous platforms the sysmon messages was just handled by packed
structs, but this was changed to being done using QMI and IPCROUTER
recently.

These messages needs to be sent before we attempt to restart the remote
processor and doing this from user space causes synchronization issues.

= Qualcomm slimbus implementation
The slimbus implementation in Qualcomm MSM devices expects a few QMI
encoded control messages to be sent for configuration and power
management purposes. So in order to get audio from the audio blocks to
the codec we need to send a few QMI encoded messages in the Qualcomm
slimbus driver.

As this is used to communicate power management states from the kernel
driver I see it infeasible to do this from user space.

= IPQ8074 WiFi driver
In many Qualcomm SoC the WiFi solution is split between an in-SoC core
that does protocol handling and an off-chip RF module. The communication
of control messages with the protocol core is done over shared memory
transports (SMD or GLINK) and has in the past (WCN3620, 3660 and 3680)
been done with packed structs. In the latest incarnation this is
replaced by QMI-encoded messages.

Qualcomm is trying to move forward in upstreaming a driver for this, as
part of their push to get IPQ8074 support upstream. I have not reviewed
the new version of this driver, but the driver for the previous
generation dealt with a mixture of control messages, dealing with DMA
channels and direct hardware control - so this does indeed look to
require in-kernel QMI messaging.

Regards,
Bjorn

> Dan
> 
> > It then adds an abstractions to reduce the duplication of common code
> > in
> > drivers that needs to query the name server and send and receive
> > encoded
> > messages to a remote service.
> > 
> > Finally it introduces a sample implementation for showing QRTR and
> > the QMI
> > helpers in action. The sample device instantiates in response to
> > finding the
> > "test service" and implements the "test protocol".
> > 
> > Bjorn Andersson (6):
> >   net: qrtr: Invoke sk_error_report() after setting sk_err
> >   net: qrtr: Move constants to header file
> >   net: qrtr: Add control packet definition to uapi
> >   soc: qcom: Introduce QMI encoder/decoder
> >   soc: qcom: Introduce QMI helpers
> >   samples: Introduce Qualcomm QRTR sample client
> > 
> >  drivers/soc/qcom/Kconfig  |   8 +
> >  drivers/soc/qcom/Makefile |   3 +
> >  drivers/soc/qcom/qmi_encdec.c | 812
> > ++
> >  drivers/soc/qcom/qmi_interface.c  | 540 +
> >  include/linux/soc/qcom/qmi.h  | 249 
> >  include/uapi/linux/qrtr.h |  35 ++
> >  net/qrtr/qrtr.c   |  16 +-
> >  samples/Kconfig   |   8 +
> >  samples/Makefile  |   2 +-
> >  samples/qrtr/Makefile

Re: [PATCH 0/6] In-kernel QMI handling

[Dan Williams]

On Fri, 2017-08-04 at 07:59 -0700, Bjorn Andersson wrote:
> This series starts by moving the common definitions of the QMUX
> protocol to the
> uapi header, as they are shared with clients - both in kernel and
> userspace.
> 
> This series then introduces in-kernel helper functions for aiding the
> handling
> of QMI encoded messages in the kernel. QMI encoding is a wire-format
> used in
> exchanging messages between the majority of QRTR clients and
> services.

This raises a few red-flags for me.  So far, we've kept almost
everything QMI related in userspace and handled all QMI control-channel 
messages from libraries like libqmi or uqmi via the cdc-wdm driver and
the "rmnet" interface via the qmi_wwan driver.  The kernel drivers just
serve as the transport.

Can you describe what kinds of in-kernel drivers need to actually parse
QMI messages as part of their operation?

Dan

> It then adds an abstractions to reduce the duplication of common code
> in
> drivers that needs to query the name server and send and receive
> encoded
> messages to a remote service.
> 
> Finally it introduces a sample implementation for showing QRTR and
> the QMI
> helpers in action. The sample device instantiates in response to
> finding the
> "test service" and implements the "test protocol".
> 
> Bjorn Andersson (6):
>   net: qrtr: Invoke sk_error_report() after setting sk_err
>   net: qrtr: Move constants to header file
>   net: qrtr: Add control packet definition to uapi
>   soc: qcom: Introduce QMI encoder/decoder
>   soc: qcom: Introduce QMI helpers
>   samples: Introduce Qualcomm QRTR sample client
> 
>  drivers/soc/qcom/Kconfig  |   8 +
>  drivers/soc/qcom/Makefile |   3 +
>  drivers/soc/qcom/qmi_encdec.c | 812
> ++
>  drivers/soc/qcom/qmi_interface.c  | 540 +
>  include/linux/soc/qcom/qmi.h  | 249 
>  include/uapi/linux/qrtr.h |  35 ++
>  net/qrtr/qrtr.c   |  16 +-
>  samples/Kconfig   |   8 +
>  samples/Makefile  |   2 +-
>  samples/qrtr/Makefile |   1 +
>  samples/qrtr/qrtr_sample_client.c | 603 
>  11 files changed, 2261 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/soc/qcom/qmi_encdec.c
>  create mode 100644 drivers/soc/qcom/qmi_interface.c
>  create mode 100644 include/linux/soc/qcom/qmi.h
>  create mode 100644 samples/qrtr/Makefile
>  create mode 100644 samples/qrtr/qrtr_sample_client.c
> 

[PATCH 0/6] In-kernel QMI handling

[Bjorn Andersson]

This series starts by moving the common definitions of the QMUX protocol to the
uapi header, as they are shared with clients - both in kernel and userspace.

This series then introduces in-kernel helper functions for aiding the handling
of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
exchanging messages between the majority of QRTR clients and services.

It then adds an abstractions to reduce the duplication of common code in
drivers that needs to query the name server and send and receive encoded
messages to a remote service.

Finally it introduces a sample implementation for showing QRTR and the QMI
helpers in action. The sample device instantiates in response to finding the
"test service" and implements the "test protocol".

Bjorn Andersson (6):
  net: qrtr: Invoke sk_error_report() after setting sk_err
  net: qrtr: Move constants to header file
  net: qrtr: Add control packet definition to uapi
  soc: qcom: Introduce QMI encoder/decoder
  soc: qcom: Introduce QMI helpers
  samples: Introduce Qualcomm QRTR sample client

 drivers/soc/qcom/Kconfig  |   8 +
 drivers/soc/qcom/Makefile |   3 +
 drivers/soc/qcom/qmi_encdec.c | 812 ++
 drivers/soc/qcom/qmi_interface.c  | 540 +
 include/linux/soc/qcom/qmi.h  | 249 
 include/uapi/linux/qrtr.h |  35 ++
 net/qrtr/qrtr.c   |  16 +-
 samples/Kconfig   |   8 +
 samples/Makefile  |   2 +-
 samples/qrtr/Makefile |   1 +
 samples/qrtr/qrtr_sample_client.c | 603 
 11 files changed, 2261 insertions(+), 16 deletions(-)
 create mode 100644 drivers/soc/qcom/qmi_encdec.c
 create mode 100644 drivers/soc/qcom/qmi_interface.c
 create mode 100644 include/linux/soc/qcom/qmi.h
 create mode 100644 samples/qrtr/Makefile
 create mode 100644 samples/qrtr/qrtr_sample_client.c

-- 
2.12.0