Re: [Intel-wired-lan] [PATCH iwl-next v2 08/14] idpf: refactor idpf to use libie controlq and Xn APIs

[Larysa Zaremba <larysa.zaremba@intel.com>]

On Thu, Apr 24, 2025 at 05:32:17PM +0200, Paul Menzel wrote:
> Dear Larysa, dear Pavan,
> 
> 
> Thank you for the patch.
> 
> Am 24.04.25 um 13:32 schrieb Larysa Zaremba:
> > From: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> > 
> > Support to initialize and configure controlq, Xn manager,
> > MMIO and reset APIs was introduced in libie. As part of it,
> > most of the existing controlq structures are renamed and
> > modified. Use those APIs in idpf and make all the necessary changes.
> > 
> > Previously for the send and receive virtchnl messages, there
> > used to be a memcpy involved in controlq code to copy the buffer
> > info passed by the send function into the controlq specific
> > buffers. There was no restriction to use automatic memory
> > in that case. The new implementation in libie removed copying
> > of the send buffer info and introduced DMA mapping of the
> > send buffer itself. To accommodate it, use dynamic memory for
> > the send buffers. In case of receive, idpf receives a page pool
> > buffer allocated by the libie and care should be taken to
> > release it after use in the idpf.
> > 
> > The changes are fairly trivial and localized, with a notable exception
> > being the consolidation of idpf_vc_xn_shutdown and idpf_deinit_dflt_mbx
> > under the latter name. This has some additional consequences that are
> > addressed in the following patches.
> 
> (You could reflow the text above to have consistent line length.)
> 
> Also, how can your patchset be verified?

Just normal regression testing with kernel debug options enabled, a large 
portion of the touched code is covered by just loading-unloading the driver and 
doing a PCI reset, stuff like PTP needs to be checked separately, because it 
heavily uses control queue itself.

> Does the module size change?

idpf size does decrease, but overall size increases. It was 585728B for idpf, 
now it is 557056 + 16384 + 53248 [B], this amounts to +40KB of storage usage on 
systems that will not use ixd.

After
*********
idpf                  557056  0
ixd                    40960  0
libie_pci              16384  2 ixd,idpf
libie_cp               53248  2 ixd,idpf
libeth                 16384  2 idpf,libie_cp

Before
*********
idpf                  585728  0
libeth                 16384  1 idpf

> Is the
> resource usage different for certain test cases?
>

We now pre-allocate small TX buffers, so that does increase the memory usage, 
but reduces the need to allocate. This results in additional 256 * 128B of 
memory permanently used, but our ctlq RX buffers need to be of size 4096B anyway 
(not changed by the patchset), so this is hardly noticable.

The worst-case memory usage should stay almost the same + abovementioned 32KB. 
As for the timings, we are mostly limited by the HW response time, which is far 
from instant.

> > Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> > Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> > Co-developed-by: Larysa Zaremba <larysa.zaremba@intel.com>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> > ---
> >   drivers/net/ethernet/intel/idpf/Kconfig       |    1 +
> >   drivers/net/ethernet/intel/idpf/Makefile      |    2 -
> >   drivers/net/ethernet/intel/idpf/idpf.h        |   42 +-
> >   .../net/ethernet/intel/idpf/idpf_controlq.c   |  624 -------
> >   .../net/ethernet/intel/idpf/idpf_controlq.h   |  130 --
> >   .../ethernet/intel/idpf/idpf_controlq_api.h   |  177 --
> >   .../ethernet/intel/idpf/idpf_controlq_setup.c |  171 --
> >   drivers/net/ethernet/intel/idpf/idpf_dev.c    |   91 +-
> >   drivers/net/ethernet/intel/idpf/idpf_lib.c    |   49 +-
> >   drivers/net/ethernet/intel/idpf/idpf_main.c   |   87 +-
> >   drivers/net/ethernet/intel/idpf/idpf_mem.h    |   20 -
> >   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |    2 +-
> >   drivers/net/ethernet/intel/idpf/idpf_vf_dev.c |   89 +-
> >   .../net/ethernet/intel/idpf/idpf_virtchnl.c   | 1622 ++++++-----------
> >   .../net/ethernet/intel/idpf/idpf_virtchnl.h   |   89 +-
> >   .../ethernet/intel/idpf/idpf_virtchnl_ptp.c   |  303 ++-
> >   16 files changed, 886 insertions(+), 2613 deletions(-)
> >   delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.c
> >   delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq.h
> >   delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_api.h
> >   delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_controlq_setup.c
> >   delete mode 100644 drivers/net/ethernet/intel/idpf/idpf_mem.h
> 
> […]
> 
> 
> Kind regards,
> 
> Paul