> -----Original Message----- > From: Simon Horman <[email protected]> > Sent: Wednesday, May 28, 2025 3:44 AM > To: Nikolova, Tatyana E <[email protected]> > Cc: [email protected]; [email protected]; [email protected]; linux- > [email protected]; [email protected]; [email protected]; Nguyen, > Anthony L <[email protected]>; Hay, Joshua A > <[email protected]> > 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 <[email protected]> > > > > 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 <[email protected]> > > Signed-off-by: Joshua Hay <[email protected]> > > Signed-off-by: Tatyana Nikolova <[email protected]> > > ... > > > 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); > > } > > > > ...
