Re: [iwl-next v1 1/7] ice: devlink PF MSI-X max and min parameter

[Jiri Pirko <jiri@resnulli.us>]

Tue, Feb 13, 2024 at 11:01:08AM CET, michal.swiatkowski@linux.intel.com wrote:
>On Tue, Feb 13, 2024 at 10:03:05AM +0100, Jiri Pirko wrote:
>> Tue, Feb 13, 2024 at 08:35:03AM CET, michal.swiatkowski@linux.intel.com wrote:
>> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X
>> >range.
>> >
>> >Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>> >Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
>> >---
>> > drivers/net/ethernet/intel/ice/ice.h         |  8 ++
>> > drivers/net/ethernet/intel/ice/ice_devlink.c | 82 ++++++++++++++++++++
>> > drivers/net/ethernet/intel/ice/ice_irq.c     |  6 ++
>> > 3 files changed, 96 insertions(+)
>> >
>> >diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
>> >index c4127d5f2be3..24085f3c0966 100644
>> >--- a/drivers/net/ethernet/intel/ice/ice.h
>> >+++ b/drivers/net/ethernet/intel/ice/ice.h
>> >@@ -94,6 +94,7 @@
>> > #define ICE_MIN_LAN_TXRX_MSIX	1
>> > #define ICE_MIN_LAN_OICR_MSIX	1
>> > #define ICE_MIN_MSIX		(ICE_MIN_LAN_TXRX_MSIX + ICE_MIN_LAN_OICR_MSIX)
>> >+#define ICE_MAX_MSIX		256
>> > #define ICE_FDIR_MSIX		2
>> > #define ICE_RDMA_NUM_AEQ_MSIX	4
>> > #define ICE_MIN_RDMA_MSIX	2
>> >@@ -535,6 +536,12 @@ struct ice_agg_node {
>> > 	u8 valid;
>> > };
>> > 
>> >+struct ice_pf_msix {
>> >+	u16 cur;
>> >+	u16 min;
>> >+	u16 max;
>> >+};
>> >+
>> > struct ice_pf {
>> > 	struct pci_dev *pdev;
>> > 
>> >@@ -604,6 +611,7 @@ struct ice_pf {
>> > 	struct msi_map ll_ts_irq;	/* LL_TS interrupt MSIX vector */
>> > 	u16 max_pf_txqs;	/* Total Tx queues PF wide */
>> > 	u16 max_pf_rxqs;	/* Total Rx queues PF wide */
>> >+	struct ice_pf_msix msix;
>> > 	u16 num_lan_msix;	/* Total MSIX vectors for base driver */
>> > 	u16 num_lan_tx;		/* num LAN Tx queues setup */
>> > 	u16 num_lan_rx;		/* num LAN Rx queues setup */
>> >diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >index cc717175178b..b82ff9556a4b 100644
>> >--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>> >@@ -1603,6 +1603,78 @@ enum ice_param_id {
>> > 	ICE_DEVLINK_PARAM_ID_LOOPBACK,
>> > };
>> > 
>> >+static int
>> >+ice_devlink_msix_max_pf_get(struct devlink *devlink, u32 id,
>> >+			    struct devlink_param_gset_ctx *ctx)
>> >+{
>> >+	struct ice_pf *pf = devlink_priv(devlink);
>> >+
>> >+	ctx->val.vu16 = pf->msix.max;
>> >+
>> >+	return 0;
>> >+}
>> >+
>> >+static int
>> >+ice_devlink_msix_max_pf_set(struct devlink *devlink, u32 id,
>> >+			    struct devlink_param_gset_ctx *ctx)
>> >+{
>> >+	struct ice_pf *pf = devlink_priv(devlink);
>> >+	u16 max = ctx->val.vu16;
>> >+
>> >+	pf->msix.max = max;
>> 
>> What's permanent about this exactly?
>> 
>
>I want to store the value here after driver reinit. Isn't it enough to
>use this parameter type? Which one should be used for this purpose?

Documentation/networking/devlink/devlink-params.rst say:

.. list-table:: Possible configuration modes
   :widths: 5 90

   * - Name
     - Description
   * - ``runtime``
     - set while the driver is running, and takes effect immediately. No
       reset is required.
   * - ``driverinit``
     - applied while the driver initializes. Requires the user to restart
       the driver using the ``devlink`` reload command.
   * - ``permanent``
     - written to the device's non-volatile memory. A hard reset is required
       for it to take effect.


[...]