netdev - Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset

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]

Date:   Thu, 5 Jan 2023 11:22:38 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Lorenzo Bianconi <lorenzo@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, lorenzo.bianconi@...hat.com,
        nbd@....name, john@...ozen.org, sean.wang@...iatek.com,
        Mark-MC.Lee@...iatek.com, sujuan.chen@...iatek.com,
        daniel@...rotopia.org
Subject: Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add
 reset/reset_complete callbacks

On Wed, Jan 04, 2023 at 03:59:17PM +0100, Lorenzo Bianconi wrote:
> > On Wed, Jan 04, 2023 at 03:03:14PM +0100, Lorenzo Bianconi wrote:
> > > Introduce reset and reset_complete wlan callback to schedule WLAN driver
> > > reset when ethernet/wed driver is resetting.
> > > 
> > > Tested-by: Daniel Golle <daniel@...rotopia.org>
> > > Co-developed-by: Sujuan Chen <sujuan.chen@...iatek.com>
> > > Signed-off-by: Sujuan Chen <sujuan.chen@...iatek.com>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > > ---
> > >  drivers/net/ethernet/mediatek/mtk_eth_soc.c |  6 ++++
> > >  drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
> > >  drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
> > >  include/linux/soc/mediatek/mtk_wed.h        |  2 ++
> > >  4 files changed, 56 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index bafae4f0312e..2d74e26f45c9 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
> > >  		mtk_w32(eth, val, MTK_MAC_MCR(i));
> > >  	}
> > >  
> > > +	rtnl_unlock();
> > > +	mtk_wed_fe_reset();
> > > +	rtnl_lock();
> > 
> > Is it safe to call rtnl_unlock(), perform some work and lock again?
> 
> Yes, mtk_pending_work sets MTK_RESETTING bit and a new reset work is not
> scheduled until this bit is cleared

I'm more worried about opening a window for user-space access while you
are performing FW reset.

<...>

> > > diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h
> > > index db637a13888d..ddff54fc9717 100644
> > > --- a/include/linux/soc/mediatek/mtk_wed.h
> > > +++ b/include/linux/soc/mediatek/mtk_wed.h
> > > @@ -150,6 +150,8 @@ struct mtk_wed_device {
> > >  		void (*release_rx_buf)(struct mtk_wed_device *wed);
> > >  		void (*update_wo_rx_stats)(struct mtk_wed_device *wed,
> > >  					   struct mtk_wed_wo_rx_stats *stats);
> > > +		int (*reset)(struct mtk_wed_device *wed);
> > > +		int (*reset_complete)(struct mtk_wed_device *wed);
> > 
> > I don't see any driver implementation of these callbacks in this series.
> > Did I miss it?
> 
> These callbacks are implemented in the mt76 driver. I have not added these
> patches to the series since mt76 patches usually go through Felix/Kalle's
> trees (anyway I am fine to add them to the series if they can go into net-next
> directly).

Usually patches that use specific functionality are submitted together
with API changes.

> 
> Regards,
> Lorenzo
> 
> > 
> > Thanks
> > 
> > >  	} wlan;
> > >  #endif
> > >  };
> > > -- 
> > > 2.39.0
> > >