[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250528104407.GB365796@horms.kernel.org>
Date: Wed, 28 May 2025 11:44:07 +0100
From: Simon Horman <horms@...nel.org>
To: Tatyana Nikolova <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,
anthony.l.nguyen@...el.com, Joshua Hay <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.
> 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);
> }
>
...
Powered by blists - more mailing lists