Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver (fwd)

2021-04-04 Thread Julia Lawall 
There is a clear use after free on line 213.

julia

-- Forwarded message --
Date: Sat, 3 Apr 2021 04:42:45 +0800
From: kernel test robot 
To: kbu...@lists.01.org
Cc: l...@intel.com, Julia Lawall 
Subject: Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

CC: kbuild-...@lists.01.org
In-Reply-To: <1617372397-13988-2-git-send-email-loic.poul...@linaro.org>
References: <1617372397-13988-2-git-send-email-loic.poul...@linaro.org>
TO: Loic Poulain 
TO: gre...@linuxfoundation.org
TO: k...@kernel.org
TO: da...@davemloft.net
CC: linux-arm-...@vger.kernel.org
CC: aleksan...@aleksander.es
CC: linux-kernel@vger.kernel.org
CC: net...@vger.kernel.org
CC: bjorn.anders...@linaro.org
CC: manivannan.sadhasi...@linaro.org
CC: Loic Poulain 

Hi Loic,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Loic-Poulain/net-Add-a-WWAN-subsystem/20210402-220002
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 
bd78980be1a68d14524c51c4b4170782fada622b
:: branch date: 7 hours ago
:: commit date: 7 hours ago
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 
Reported-by: Julia Lawall 


cocci warnings: (new ones prefixed by >>)
>> drivers/net/wwan/mhi_wwan_ctrl.c:213:17-24: ERROR: reference preceded by 
>> free on line 212

vim +213 drivers/net/wwan/mhi_wwan_ctrl.c

16d753f4f524ce Loic Poulain 2021-04-02  184
16d753f4f524ce Loic Poulain 2021-04-02  185  static int 
mhi_wwan_ctrl_probe(struct mhi_device *mhi_dev,
16d753f4f524ce Loic Poulain 2021-04-02  186const 
struct mhi_device_id *id)
16d753f4f524ce Loic Poulain 2021-04-02  187  {
16d753f4f524ce Loic Poulain 2021-04-02  188 struct mhi_controller *cntrl = 
mhi_dev->mhi_cntrl;
16d753f4f524ce Loic Poulain 2021-04-02  189 struct mhi_wwan_dev *mhiwwan;
16d753f4f524ce Loic Poulain 2021-04-02  190
16d753f4f524ce Loic Poulain 2021-04-02  191 mhiwwan = 
kzalloc(sizeof(*mhiwwan), GFP_KERNEL);
16d753f4f524ce Loic Poulain 2021-04-02  192 if (!mhiwwan)
16d753f4f524ce Loic Poulain 2021-04-02  193 return -ENOMEM;
16d753f4f524ce Loic Poulain 2021-04-02  194
16d753f4f524ce Loic Poulain 2021-04-02  195 mhiwwan->mhi_dev = mhi_dev;
16d753f4f524ce Loic Poulain 2021-04-02  196 mhiwwan->mtu = MHI_WWAN_MAX_MTU;
16d753f4f524ce Loic Poulain 2021-04-02  197 INIT_WORK(>rx_refill, 
mhi_wwan_ctrl_refill_work);
16d753f4f524ce Loic Poulain 2021-04-02  198 
spin_lock_init(>tx_lock);
16d753f4f524ce Loic Poulain 2021-04-02  199
16d753f4f524ce Loic Poulain 2021-04-02  200 if (mhi_dev->dl_chan)
16d753f4f524ce Loic Poulain 2021-04-02  201 
set_bit(MHI_WWAN_DL_CAP, >flags);
16d753f4f524ce Loic Poulain 2021-04-02  202 if (mhi_dev->ul_chan)
16d753f4f524ce Loic Poulain 2021-04-02  203 
set_bit(MHI_WWAN_UL_CAP, >flags);
16d753f4f524ce Loic Poulain 2021-04-02  204
16d753f4f524ce Loic Poulain 2021-04-02  205 dev_set_drvdata(_dev->dev, 
mhiwwan);
16d753f4f524ce Loic Poulain 2021-04-02  206
16d753f4f524ce Loic Poulain 2021-04-02  207 /* Register as a wwan port, 
id->driver_data contains wwan port type */
16d753f4f524ce Loic Poulain 2021-04-02  208 mhiwwan->wwan_port = 
wwan_create_port(>mhi_dev->dev,
16d753f4f524ce Loic Poulain 2021-04-02  209 
  id->driver_data,
16d753f4f524ce Loic Poulain 2021-04-02  210 
  _pops, mhiwwan);
16d753f4f524ce Loic Poulain 2021-04-02  211 if (IS_ERR(mhiwwan->wwan_port)) 
{
16d753f4f524ce Loic Poulain 2021-04-02 @212 kfree(mhiwwan);
16d753f4f524ce Loic Poulain 2021-04-02 @213 return 
PTR_ERR(mhiwwan->wwan_port);
16d753f4f524ce Loic Poulain 2021-04-02  214 }
16d753f4f524ce Loic Poulain 2021-04-02  215
16d753f4f524ce Loic Poulain 2021-04-02  216 return 0;
16d753f4f524ce Loic Poulain 2021-04-02  217  };
16d753f4f524ce Loic Poulain 2021-04-02  218

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

.config.gz
Description: application/gzip

Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

2021-04-02 Thread Loic Poulain 
On Fri, 2 Apr 2021 at 17:43, Greg KH  wrote:
>
> On Fri, Apr 02, 2021 at 05:41:01PM +0200, Loic Poulain wrote:
> > On Fri, 2 Apr 2021 at 16:05, Greg KH  wrote:
> > >
> > > On Fri, Apr 02, 2021 at 04:06:37PM +0200, Loic Poulain wrote:
> > > > The MHI WWWAN control driver allows MHI QCOM-based modems to expose
> > > > different modem control protocols/ports via the WWAN framework, so that
> > > > userspace modem tools or daemon (e.g. ModemManager) can control WWAN
> > > > config and state (APN config, SMS, provider selection...). A QCOM-based
> > > > modem can expose one or several of the following protocols:
> > > > - AT: Well known AT commands interactive protocol (microcom, minicom...)
> > > > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> > > > - QMI: QCOM MSM/Modem Interface (libqmi, qmicli)
> > > > - QCDM: QCOM Modem diagnostic interface (libqcdm)
> > > > - FIREHOSE: XML-based protocol for Modem firmware management
> > > > (qmi-firmware-update)
> > > >
> > > > Note that this patch is mostly a rework of the earlier MHI UCI
> > > > tentative that was a generic interface for accessing MHI bus from
> > > > userspace. As suggested, this new version is WWAN specific and is
> > > > dedicated to only expose channels used for controlling a modem, and
> > > > for which related opensource userpace support exist.
> > > >
> > > > Signed-off-by: Loic Poulain 
> > > > ---
> > > >  v2: update copyright (2021)
> > > >  v3: Move driver to dedicated drivers/net/wwan directory
> > > >  v4: Rework to use wwan framework instead of self cdev management
> > > >  v5: Fix errors/typos in Kconfig
> > > >  v6: - Move to new wwan interface, No need dedicated call to 
> > > > wwan_dev_create
> > > >  - Cleanup code (remove legacy from mhi_uci, unused defines/vars...)
> > > >  - Remove useless write_lock mutex
> > > >  - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq 
> > > > helpers
> > > >  - Rework locking
> > > >  - Add MHI_WWAN_TX_FULL flag
> > > >  - Add support for NONBLOCK read/write
> > > >  v7: Fix change log (mixed up 1/2 and 2/2)
> > > >  v8: - Implement wwan_port_ops (instead of fops)
> > > >  - Remove all mhi wwan data obsolete members (kref, lock, 
> > > > waitqueues)
> > > >  - Add tracking of RX buffer budget
> > > >  - Use WWAN TX flow control function to stop TX when MHI queue is 
> > > > full
> > > >
> > > >  drivers/net/wwan/Kconfig |  14 +++
> > > >  drivers/net/wwan/Makefile|   2 +
> > > >  drivers/net/wwan/mhi_wwan_ctrl.c | 253 
> > > > +++
> > > >  3 files changed, 269 insertions(+)
> > > >  create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> > > >
> > > > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > > > index 545fe54..ce0bbfb 100644
> > > > --- a/drivers/net/wwan/Kconfig
> > > > +++ b/drivers/net/wwan/Kconfig
> > > > @@ -19,4 +19,18 @@ config WWAN_CORE
> > > > To compile this driver as a module, choose M here: the module 
> > > > will be
> > > > called wwan.
> > > >
> > > > +config MHI_WWAN_CTRL
> > > > + tristate "MHI WWAN control driver for QCOM-based PCIe modems"
> > > > + select WWAN_CORE
> > > > + depends on MHI_BUS
> > > > + help
> > > > +   MHI WWAN CTRL allows QCOM-based PCIe modems to expose different 
> > > > modem
> > > > +   control protocols/ports to userspace, including AT, MBIM, QMI, 
> > > > DIAG
> > > > +   and FIREHOSE. These protocols can be accessed directly from 
> > > > userspace
> > > > +   (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> > > > +   libqcdm...).
> > > > +
> > > > +   To compile this driver as a module, choose M here: the module 
> > > > will be
> > > > +   called mhi_wwan_ctrl
> > > > +
> > > >  endif # WWAN
> > > > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > > > index 934590b..556cd90 100644
> > > > --- a/drivers/net/wwan/Makefile
> > > > +++ b/drivers/net/wwan/Makefile
> > > > @@ -5,3 +5,5 @@
> > > >
> > > >  obj-$(CONFIG_WWAN_CORE) += wwan.o
> > > >  wwan-objs += wwan_core.o
> > > > +
> > > > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> > > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c 
> > > > b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > > new file mode 100644
> > > > index 000..f2fab23
> > > > --- /dev/null
> > > > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > > @@ -0,0 +1,253 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/* Copyright (c) 2021, Linaro Ltd  */
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +#include 
> > > > +
> > > > +/* MHI wwan flags */
> > > > +#define MHI_WWAN_DL_CAP  BIT(0)
> > > > +#define MHI_WWAN_UL_CAP  BIT(1)
> > > > +#define MHI_WWAN_STARTED BIT(2)
> > > > +
> > > > +#define MHI_WWAN_MAX_MTU 0x8000
> > > > +
> > > > +struct mhi_wwan_dev {
> > > > + /* Lower level is a mhi dev, upper level

Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

2021-04-02 Thread kernel test robot 
Hi Loic,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Loic-Poulain/net-Add-a-WWAN-subsystem/20210402-220002
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 
bd78980be1a68d14524c51c4b4170782fada622b
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/16d753f4f524ce1aa95f45057244b1d28b50adc9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Loic-Poulain/net-Add-a-WWAN-subsystem/20210402-220002
git checkout 16d753f4f524ce1aa95f45057244b1d28b50adc9
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

>> drivers/net/wwan/mhi_wwan_ctrl.c:46:6: warning: no previous prototype for 
>> '__mhi_skb_destructor' [-Wmissing-prototypes]
  46 | void __mhi_skb_destructor(struct sk_buff *skb)
 |  ^~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for SND_ATMEL_SOC_PDC
   Depends on SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC && HAS_DMA
   Selected by
   - SND_ATMEL_SOC_SSC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC
   - SND_ATMEL_SOC_SSC_PDC && SOUND && !UML && SND && SND_SOC && SND_ATMEL_SOC 
&& ATMEL_SSC


vim +/__mhi_skb_destructor +46 drivers/net/wwan/mhi_wwan_ctrl.c

45  
  > 46  void __mhi_skb_destructor(struct sk_buff *skb)
47  {
48  struct mhi_wwan_dev *mhiwwan = skb_shinfo(skb)->destructor_arg;
49  
50  /* RX buffer has been consumed, increase the allowed budget */
51  atomic_inc(>rx_budget);
52  
53  if (mhi_wwan_ctrl_refill_needed(mhiwwan))
54  schedule_work(>rx_refill);
55  }
56  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip

Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

2021-04-02 Thread Greg KH 
On Fri, Apr 02, 2021 at 05:41:01PM +0200, Loic Poulain wrote:
> On Fri, 2 Apr 2021 at 16:05, Greg KH  wrote:
> >
> > On Fri, Apr 02, 2021 at 04:06:37PM +0200, Loic Poulain wrote:
> > > The MHI WWWAN control driver allows MHI QCOM-based modems to expose
> > > different modem control protocols/ports via the WWAN framework, so that
> > > userspace modem tools or daemon (e.g. ModemManager) can control WWAN
> > > config and state (APN config, SMS, provider selection...). A QCOM-based
> > > modem can expose one or several of the following protocols:
> > > - AT: Well known AT commands interactive protocol (microcom, minicom...)
> > > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> > > - QMI: QCOM MSM/Modem Interface (libqmi, qmicli)
> > > - QCDM: QCOM Modem diagnostic interface (libqcdm)
> > > - FIREHOSE: XML-based protocol for Modem firmware management
> > > (qmi-firmware-update)
> > >
> > > Note that this patch is mostly a rework of the earlier MHI UCI
> > > tentative that was a generic interface for accessing MHI bus from
> > > userspace. As suggested, this new version is WWAN specific and is
> > > dedicated to only expose channels used for controlling a modem, and
> > > for which related opensource userpace support exist.
> > >
> > > Signed-off-by: Loic Poulain 
> > > ---
> > >  v2: update copyright (2021)
> > >  v3: Move driver to dedicated drivers/net/wwan directory
> > >  v4: Rework to use wwan framework instead of self cdev management
> > >  v5: Fix errors/typos in Kconfig
> > >  v6: - Move to new wwan interface, No need dedicated call to 
> > > wwan_dev_create
> > >  - Cleanup code (remove legacy from mhi_uci, unused defines/vars...)
> > >  - Remove useless write_lock mutex
> > >  - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq 
> > > helpers
> > >  - Rework locking
> > >  - Add MHI_WWAN_TX_FULL flag
> > >  - Add support for NONBLOCK read/write
> > >  v7: Fix change log (mixed up 1/2 and 2/2)
> > >  v8: - Implement wwan_port_ops (instead of fops)
> > >  - Remove all mhi wwan data obsolete members (kref, lock, waitqueues)
> > >  - Add tracking of RX buffer budget
> > >  - Use WWAN TX flow control function to stop TX when MHI queue is full
> > >
> > >  drivers/net/wwan/Kconfig |  14 +++
> > >  drivers/net/wwan/Makefile|   2 +
> > >  drivers/net/wwan/mhi_wwan_ctrl.c | 253 
> > > +++
> > >  3 files changed, 269 insertions(+)
> > >  create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> > >
> > > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > > index 545fe54..ce0bbfb 100644
> > > --- a/drivers/net/wwan/Kconfig
> > > +++ b/drivers/net/wwan/Kconfig
> > > @@ -19,4 +19,18 @@ config WWAN_CORE
> > > To compile this driver as a module, choose M here: the module 
> > > will be
> > > called wwan.
> > >
> > > +config MHI_WWAN_CTRL
> > > + tristate "MHI WWAN control driver for QCOM-based PCIe modems"
> > > + select WWAN_CORE
> > > + depends on MHI_BUS
> > > + help
> > > +   MHI WWAN CTRL allows QCOM-based PCIe modems to expose different 
> > > modem
> > > +   control protocols/ports to userspace, including AT, MBIM, QMI, 
> > > DIAG
> > > +   and FIREHOSE. These protocols can be accessed directly from 
> > > userspace
> > > +   (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> > > +   libqcdm...).
> > > +
> > > +   To compile this driver as a module, choose M here: the module 
> > > will be
> > > +   called mhi_wwan_ctrl
> > > +
> > >  endif # WWAN
> > > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > > index 934590b..556cd90 100644
> > > --- a/drivers/net/wwan/Makefile
> > > +++ b/drivers/net/wwan/Makefile
> > > @@ -5,3 +5,5 @@
> > >
> > >  obj-$(CONFIG_WWAN_CORE) += wwan.o
> > >  wwan-objs += wwan_core.o
> > > +
> > > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> > > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c 
> > > b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > new file mode 100644
> > > index 000..f2fab23
> > > --- /dev/null
> > > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> > > @@ -0,0 +1,253 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/* Copyright (c) 2021, Linaro Ltd  */
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +/* MHI wwan flags */
> > > +#define MHI_WWAN_DL_CAP  BIT(0)
> > > +#define MHI_WWAN_UL_CAP  BIT(1)
> > > +#define MHI_WWAN_STARTED BIT(2)
> > > +
> > > +#define MHI_WWAN_MAX_MTU 0x8000
> > > +
> > > +struct mhi_wwan_dev {
> > > + /* Lower level is a mhi dev, upper level is a wwan port */
> > > + struct mhi_device *mhi_dev;
> > > + struct wwan_port *wwan_port;
> > > +
> > > + /* State and capabilities */
> > > + unsigned long flags;
> > > + size_t mtu;
> > > +
> > > + /* Protect against concurrent TX and TX-completion (bh) */
> > >

Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

2021-04-02 Thread Loic Poulain 
On Fri, 2 Apr 2021 at 16:05, Greg KH  wrote:
>
> On Fri, Apr 02, 2021 at 04:06:37PM +0200, Loic Poulain wrote:
> > The MHI WWWAN control driver allows MHI QCOM-based modems to expose
> > different modem control protocols/ports via the WWAN framework, so that
> > userspace modem tools or daemon (e.g. ModemManager) can control WWAN
> > config and state (APN config, SMS, provider selection...). A QCOM-based
> > modem can expose one or several of the following protocols:
> > - AT: Well known AT commands interactive protocol (microcom, minicom...)
> > - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> > - QMI: QCOM MSM/Modem Interface (libqmi, qmicli)
> > - QCDM: QCOM Modem diagnostic interface (libqcdm)
> > - FIREHOSE: XML-based protocol for Modem firmware management
> > (qmi-firmware-update)
> >
> > Note that this patch is mostly a rework of the earlier MHI UCI
> > tentative that was a generic interface for accessing MHI bus from
> > userspace. As suggested, this new version is WWAN specific and is
> > dedicated to only expose channels used for controlling a modem, and
> > for which related opensource userpace support exist.
> >
> > Signed-off-by: Loic Poulain 
> > ---
> >  v2: update copyright (2021)
> >  v3: Move driver to dedicated drivers/net/wwan directory
> >  v4: Rework to use wwan framework instead of self cdev management
> >  v5: Fix errors/typos in Kconfig
> >  v6: - Move to new wwan interface, No need dedicated call to wwan_dev_create
> >  - Cleanup code (remove legacy from mhi_uci, unused defines/vars...)
> >  - Remove useless write_lock mutex
> >  - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq helpers
> >  - Rework locking
> >  - Add MHI_WWAN_TX_FULL flag
> >  - Add support for NONBLOCK read/write
> >  v7: Fix change log (mixed up 1/2 and 2/2)
> >  v8: - Implement wwan_port_ops (instead of fops)
> >  - Remove all mhi wwan data obsolete members (kref, lock, waitqueues)
> >  - Add tracking of RX buffer budget
> >  - Use WWAN TX flow control function to stop TX when MHI queue is full
> >
> >  drivers/net/wwan/Kconfig |  14 +++
> >  drivers/net/wwan/Makefile|   2 +
> >  drivers/net/wwan/mhi_wwan_ctrl.c | 253 
> > +++
> >  3 files changed, 269 insertions(+)
> >  create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> >
> > diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> > index 545fe54..ce0bbfb 100644
> > --- a/drivers/net/wwan/Kconfig
> > +++ b/drivers/net/wwan/Kconfig
> > @@ -19,4 +19,18 @@ config WWAN_CORE
> > To compile this driver as a module, choose M here: the module will 
> > be
> > called wwan.
> >
> > +config MHI_WWAN_CTRL
> > + tristate "MHI WWAN control driver for QCOM-based PCIe modems"
> > + select WWAN_CORE
> > + depends on MHI_BUS
> > + help
> > +   MHI WWAN CTRL allows QCOM-based PCIe modems to expose different 
> > modem
> > +   control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> > +   and FIREHOSE. These protocols can be accessed directly from 
> > userspace
> > +   (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> > +   libqcdm...).
> > +
> > +   To compile this driver as a module, choose M here: the module will 
> > be
> > +   called mhi_wwan_ctrl
> > +
> >  endif # WWAN
> > diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> > index 934590b..556cd90 100644
> > --- a/drivers/net/wwan/Makefile
> > +++ b/drivers/net/wwan/Makefile
> > @@ -5,3 +5,5 @@
> >
> >  obj-$(CONFIG_WWAN_CORE) += wwan.o
> >  wwan-objs += wwan_core.o
> > +
> > +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> > diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c 
> > b/drivers/net/wwan/mhi_wwan_ctrl.c
> > new file mode 100644
> > index 000..f2fab23
> > --- /dev/null
> > +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright (c) 2021, Linaro Ltd  */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* MHI wwan flags */
> > +#define MHI_WWAN_DL_CAP  BIT(0)
> > +#define MHI_WWAN_UL_CAP  BIT(1)
> > +#define MHI_WWAN_STARTED BIT(2)
> > +
> > +#define MHI_WWAN_MAX_MTU 0x8000
> > +
> > +struct mhi_wwan_dev {
> > + /* Lower level is a mhi dev, upper level is a wwan port */
> > + struct mhi_device *mhi_dev;
> > + struct wwan_port *wwan_port;
> > +
> > + /* State and capabilities */
> > + unsigned long flags;
> > + size_t mtu;
> > +
> > + /* Protect against concurrent TX and TX-completion (bh) */
> > + spinlock_t tx_lock;
> > +
> > + struct work_struct rx_refill;
> > + atomic_t rx_budget;
>
> Why is this atomic if you have a real lock already?

Access to rx_budget value is not under any locking protection and can
be modified (dec/inc) from different and possibly concurrent places.

>
>
> > +};
> > +

Re: [PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

2021-04-02 Thread Greg KH 
On Fri, Apr 02, 2021 at 04:06:37PM +0200, Loic Poulain wrote:
> The MHI WWWAN control driver allows MHI QCOM-based modems to expose
> different modem control protocols/ports via the WWAN framework, so that
> userspace modem tools or daemon (e.g. ModemManager) can control WWAN
> config and state (APN config, SMS, provider selection...). A QCOM-based
> modem can expose one or several of the following protocols:
> - AT: Well known AT commands interactive protocol (microcom, minicom...)
> - MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
> - QMI: QCOM MSM/Modem Interface (libqmi, qmicli)
> - QCDM: QCOM Modem diagnostic interface (libqcdm)
> - FIREHOSE: XML-based protocol for Modem firmware management
> (qmi-firmware-update)
> 
> Note that this patch is mostly a rework of the earlier MHI UCI
> tentative that was a generic interface for accessing MHI bus from
> userspace. As suggested, this new version is WWAN specific and is
> dedicated to only expose channels used for controlling a modem, and
> for which related opensource userpace support exist.
> 
> Signed-off-by: Loic Poulain 
> ---
>  v2: update copyright (2021)
>  v3: Move driver to dedicated drivers/net/wwan directory
>  v4: Rework to use wwan framework instead of self cdev management
>  v5: Fix errors/typos in Kconfig
>  v6: - Move to new wwan interface, No need dedicated call to wwan_dev_create
>  - Cleanup code (remove legacy from mhi_uci, unused defines/vars...)
>  - Remove useless write_lock mutex
>  - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq helpers
>  - Rework locking
>  - Add MHI_WWAN_TX_FULL flag
>  - Add support for NONBLOCK read/write
>  v7: Fix change log (mixed up 1/2 and 2/2)
>  v8: - Implement wwan_port_ops (instead of fops)
>  - Remove all mhi wwan data obsolete members (kref, lock, waitqueues)
>  - Add tracking of RX buffer budget
>  - Use WWAN TX flow control function to stop TX when MHI queue is full
> 
>  drivers/net/wwan/Kconfig |  14 +++
>  drivers/net/wwan/Makefile|   2 +
>  drivers/net/wwan/mhi_wwan_ctrl.c | 253 
> +++
>  3 files changed, 269 insertions(+)
>  create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c
> 
> diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
> index 545fe54..ce0bbfb 100644
> --- a/drivers/net/wwan/Kconfig
> +++ b/drivers/net/wwan/Kconfig
> @@ -19,4 +19,18 @@ config WWAN_CORE
> To compile this driver as a module, choose M here: the module will be
> called wwan.
>  
> +config MHI_WWAN_CTRL
> + tristate "MHI WWAN control driver for QCOM-based PCIe modems"
> + select WWAN_CORE
> + depends on MHI_BUS
> + help
> +   MHI WWAN CTRL allows QCOM-based PCIe modems to expose different modem
> +   control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
> +   and FIREHOSE. These protocols can be accessed directly from userspace
> +   (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
> +   libqcdm...).
> +
> +   To compile this driver as a module, choose M here: the module will be
> +   called mhi_wwan_ctrl
> +
>  endif # WWAN
> diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
> index 934590b..556cd90 100644
> --- a/drivers/net/wwan/Makefile
> +++ b/drivers/net/wwan/Makefile
> @@ -5,3 +5,5 @@
>  
>  obj-$(CONFIG_WWAN_CORE) += wwan.o
>  wwan-objs += wwan_core.o
> +
> +obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c 
> b/drivers/net/wwan/mhi_wwan_ctrl.c
> new file mode 100644
> index 000..f2fab23
> --- /dev/null
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2021, Linaro Ltd  */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* MHI wwan flags */
> +#define MHI_WWAN_DL_CAP  BIT(0)
> +#define MHI_WWAN_UL_CAP  BIT(1)
> +#define MHI_WWAN_STARTED BIT(2)
> +
> +#define MHI_WWAN_MAX_MTU 0x8000
> +
> +struct mhi_wwan_dev {
> + /* Lower level is a mhi dev, upper level is a wwan port */
> + struct mhi_device *mhi_dev;
> + struct wwan_port *wwan_port;
> +
> + /* State and capabilities */
> + unsigned long flags;
> + size_t mtu;
> +
> + /* Protect against concurrent TX and TX-completion (bh) */
> + spinlock_t tx_lock;
> +
> + struct work_struct rx_refill;
> + atomic_t rx_budget;

Why is this atomic if you have a real lock already?


> +};
> +
> +static bool mhi_wwan_ctrl_refill_needed(struct mhi_wwan_dev *mhiwwan)
> +{
> + if (!test_bit(MHI_WWAN_STARTED, >flags))
> + return false;
> +
> + if (!test_bit(MHI_WWAN_DL_CAP, >flags))
> + return false;

What prevents these bits from being changed right after reading them?

> +
> + if (!atomic_read(>rx_budget))
> + return false;

Why is this atomic?  What happens if it changes right

[PATCH net-next v8 2/2] net: Add Qcom WWAN control driver

2021-04-02 Thread Loic Poulain 
The MHI WWWAN control driver allows MHI QCOM-based modems to expose
different modem control protocols/ports via the WWAN framework, so that
userspace modem tools or daemon (e.g. ModemManager) can control WWAN
config and state (APN config, SMS, provider selection...). A QCOM-based
modem can expose one or several of the following protocols:
- AT: Well known AT commands interactive protocol (microcom, minicom...)
- MBIM: Mobile Broadband Interface Model (libmbim, mbimcli)
- QMI: QCOM MSM/Modem Interface (libqmi, qmicli)
- QCDM: QCOM Modem diagnostic interface (libqcdm)
- FIREHOSE: XML-based protocol for Modem firmware management
(qmi-firmware-update)

Note that this patch is mostly a rework of the earlier MHI UCI
tentative that was a generic interface for accessing MHI bus from
userspace. As suggested, this new version is WWAN specific and is
dedicated to only expose channels used for controlling a modem, and
for which related opensource userpace support exist.

Signed-off-by: Loic Poulain 
---
 v2: update copyright (2021)
 v3: Move driver to dedicated drivers/net/wwan directory
 v4: Rework to use wwan framework instead of self cdev management
 v5: Fix errors/typos in Kconfig
 v6: - Move to new wwan interface, No need dedicated call to wwan_dev_create
 - Cleanup code (remove legacy from mhi_uci, unused defines/vars...)
 - Remove useless write_lock mutex
 - Add mhi_wwan_wait_writable and mhi_wwan_wait_dlqueue_lock_irq helpers
 - Rework locking
 - Add MHI_WWAN_TX_FULL flag
 - Add support for NONBLOCK read/write
 v7: Fix change log (mixed up 1/2 and 2/2)
 v8: - Implement wwan_port_ops (instead of fops)
 - Remove all mhi wwan data obsolete members (kref, lock, waitqueues)
 - Add tracking of RX buffer budget
 - Use WWAN TX flow control function to stop TX when MHI queue is full

 drivers/net/wwan/Kconfig |  14 +++
 drivers/net/wwan/Makefile|   2 +
 drivers/net/wwan/mhi_wwan_ctrl.c | 253 +++
 3 files changed, 269 insertions(+)
 create mode 100644 drivers/net/wwan/mhi_wwan_ctrl.c

diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
index 545fe54..ce0bbfb 100644
--- a/drivers/net/wwan/Kconfig
+++ b/drivers/net/wwan/Kconfig
@@ -19,4 +19,18 @@ config WWAN_CORE
  To compile this driver as a module, choose M here: the module will be
  called wwan.
 
+config MHI_WWAN_CTRL
+   tristate "MHI WWAN control driver for QCOM-based PCIe modems"
+   select WWAN_CORE
+   depends on MHI_BUS
+   help
+ MHI WWAN CTRL allows QCOM-based PCIe modems to expose different modem
+ control protocols/ports to userspace, including AT, MBIM, QMI, DIAG
+ and FIREHOSE. These protocols can be accessed directly from userspace
+ (e.g. AT commands) or via libraries/tools (e.g. libmbim, libqmi,
+ libqcdm...).
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_wwan_ctrl
+
 endif # WWAN
diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
index 934590b..556cd90 100644
--- a/drivers/net/wwan/Makefile
+++ b/drivers/net/wwan/Makefile
@@ -5,3 +5,5 @@
 
 obj-$(CONFIG_WWAN_CORE) += wwan.o
 wwan-objs += wwan_core.o
+
+obj-$(CONFIG_MHI_WWAN_CTRL) += mhi_wwan_ctrl.o
diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
new file mode 100644
index 000..f2fab23
--- /dev/null
+++ b/drivers/net/wwan/mhi_wwan_ctrl.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2021, Linaro Ltd  */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* MHI wwan flags */
+#define MHI_WWAN_DL_CAPBIT(0)
+#define MHI_WWAN_UL_CAPBIT(1)
+#define MHI_WWAN_STARTED   BIT(2)
+
+#define MHI_WWAN_MAX_MTU   0x8000
+
+struct mhi_wwan_dev {
+   /* Lower level is a mhi dev, upper level is a wwan port */
+   struct mhi_device *mhi_dev;
+   struct wwan_port *wwan_port;
+
+   /* State and capabilities */
+   unsigned long flags;
+   size_t mtu;
+
+   /* Protect against concurrent TX and TX-completion (bh) */
+   spinlock_t tx_lock;
+
+   struct work_struct rx_refill;
+   atomic_t rx_budget;
+};
+
+static bool mhi_wwan_ctrl_refill_needed(struct mhi_wwan_dev *mhiwwan)
+{
+   if (!test_bit(MHI_WWAN_STARTED, >flags))
+   return false;
+
+   if (!test_bit(MHI_WWAN_DL_CAP, >flags))
+   return false;
+
+   if (!atomic_read(>rx_budget))
+   return false;
+
+   return true;
+}
+
+void __mhi_skb_destructor(struct sk_buff *skb)
+{
+   struct mhi_wwan_dev *mhiwwan = skb_shinfo(skb)->destructor_arg;
+
+   /* RX buffer has been consumed, increase the allowed budget */
+   atomic_inc(>rx_budget);
+
+   if (mhi_wwan_ctrl_refill_needed(mhiwwan))
+   schedule_work(>rx_refill);
+}
+
+static void mhi_wwan_ctrl_refill_work(struct work_struct *work)
+{
+