netdev - RE: [iwl-next 6/6] idpf: implement get LAN MMIO memory regions

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <DM4PR11MB65022C4B91166794FE816CDDD467A@DM4PR11MB6502.namprd11.prod.outlook.com>
Date: Wed, 28 May 2025 21:35:04 +0000
From: "Hay, Joshua A" <joshua.a.hay@...el.com>
To: Simon Horman <horms@...nel.org>, "Nikolova, Tatyana E"
	<tatyana.e.nikolova@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"jgg@...dia.com" <jgg@...dia.com>, "leon@...nel.org" <leon@...nel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "kuba@...nel.org"
	<kuba@...nel.org>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>
Subject: RE: [iwl-next 6/6] idpf: implement get LAN MMIO memory regions

> -----Original Message-----
> From: Simon Horman <horms@...nel.org>
> Sent: Wednesday, May 28, 2025 3:44 AM
> To: Nikolova, Tatyana E <tatyana.e.nikolova@...el.com>
> Cc: intel-wired-lan@...ts.osuosl.org; jgg@...dia.com; leon@...nel.org; linux-
> rdma@...r.kernel.org; netdev@...r.kernel.org; kuba@...nel.org; Nguyen,
> Anthony L <anthony.l.nguyen@...el.com>; Hay, Joshua A
> <joshua.a.hay@...el.com>
> Subject: Re: [iwl-next 6/6] idpf: implement get LAN MMIO memory regions
> 
> On Fri, May 23, 2025 at 12:04:35PM -0500, Tatyana Nikolova wrote:
> > From: Joshua Hay <joshua.a.hay@...el.com>
> >
> > The RDMA driver needs to map its own MMIO regions for the sake of
> > performance, meaning the IDPF needs to avoid mapping portions of the BAR
> > space. However, the IDPF cannot assume where these are and must avoid
> > mapping hard coded regions as much as possible.
> >
> > The IDPF maps the bare minimum to load and communicate with the
> > control plane, i.e., the mailbox registers and the reset state
> > registers. Because of how and when mailbox reigster offsets are
> > initialized, it is easier to adjust the existing defines to be relative
> > to the mailbox region starting address. Use a specific mailbox register
> > write function that uses these relative offsets. The reset state
> > register addresses are calculated the same way as for other registers,
> > described below.
> >
> > The IDPF then calls a new virtchnl op to fetch a list of MMIO regions
> > that it should map. The addresses for the registers in these regions are
> > calculated by determining what region the register resides in, adjusting
> > the offset to be relative to that region, and then adding the
> > register's offset to that region's mapped address.
> >
> > If the new virtchnl op is not supported, the IDPF will fallback to
> > mapping the whole bar. However, it will still map them as separate
> > regions outside the mailbox and reset state registers. This way we can
> > use the same logic in both cases to access the MMIO space.
> >
> > Reviewed-by: Madhu Chittim <madhu.chittim@...el.com>
> > Signed-off-by: Joshua Hay <joshua.a.hay@...el.com>
> > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@...el.com>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_idc.c
> b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> 
> ...
> 
> > @@ -447,12 +469,15 @@ int idpf_idc_init_aux_core_dev(struct
> idpf_adapter *adapter,
> >   */
> >  void idpf_idc_deinit_core_aux_device(struct iidc_rdma_core_dev_info
> *cdev_info)
> >  {
> > +	struct iidc_rdma_priv_dev_info *privd = cdev_info->iidc_priv;
> > +
> 
> Hi Joshua, Tatyana, all,
> 
> On the line below it is assumed that cdev_info may be NULL.
> But on the line above cdev_info is unconditionally dereferenced.
> This doesn't seem consistent.
> 
> Flagged by Smatch.

Right, that is a mistake. Will fix in v2. 

Thanks,
Josh

> 
> >  	if (!cdev_info)
> >  		return;
> >
> >  	idpf_unplug_aux_dev(cdev_info->adev);
> >
> > -	kfree(cdev_info->iidc_priv);
> > +	kfree(privd->mapped_mem_regions);
> > +	kfree(privd);
> >  	kfree(cdev_info);
> >  }
> >
> 
> ...