[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230815183854.GU22185@unreal>
Date: Tue, 15 Aug 2023 21:38:54 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
edumazet@...gle.com, netdev@...r.kernel.org,
Paul M Stillwell Jr <paul.m.stillwell.jr@...el.com>,
jacob.e.keller@...el.com, horms@...nel.org,
Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net-next v3 2/5] ice: configure FW logging
On Tue, Aug 15, 2023 at 09:57:47AM -0700, Tony Nguyen wrote:
> From: Paul M Stillwell Jr <paul.m.stillwell.jr@...el.com>
>
> Users want the ability to debug FW issues by retrieving the
> FW logs from the E8xx devices. Use debugfs to allow the user to
> read/write the FW log configuration by adding a 'fwlog/modules' file.
> Reading the file will show either the currently running configuration or
> the next configuration (if the user has changed the configuration).
> Writing to the file will update the configuration, but NOT enable the
> configuration (that is a separate command).
>
> To see the status of FW logging then read the 'fwlog/modules' file like
> this:
>
> cat /sys/kernel/debug/ice/0000\:18\:00.0/fwlog/modules
>
> To change the configuration of FW logging then write to the 'fwlog/modules'
> file like this:
>
> echo DCB NORMAL > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog/modules
>
> The format to change the configuration is
>
> echo <fwlog_module> <fwlog_level> > /sys/kernel/debug/ice/<pci device
This line is truncated, it is not clear where you are writing.
And more general question, a long time ago, netdev had a policy of
not-allowing writing to debugfs, was it changed?
>
> where
>
> * fwlog_level is a name as described below. Each level includes the
> messages from the previous/lower level
>
> * NONE
> * ERROR
> * WARNING
> * NORMAL
> * VERBOSE
>
> * fwlog_event is a name that represents the module to receive events for.
> The module names are
>
> * GENERAL
> * CTRL
> * LINK
> * LINK_TOPO
> * DNL
> * I2C
> * SDP
> * MDIO
> * ADMINQ
> * HDMA
> * LLDP
> * DCBX
> * DCB
> * XLR
> * NVM
> * AUTH
> * VPD
> * IOSF
> * PARSER
> * SW
> * SCHEDULER
> * TXQ
> * RSVD
> * POST
> * WATCHDOG
> * TASK_DISPATCH
> * MNG
> * SYNCE
> * HEALTH
> * TSDRV
> * PFREG
> * MDLVER
> * ALL
>
> The name ALL is special and specifies setting all of the modules to the
> specified fwlog_level.
>
> If the NVM supports FW logging then the file 'fwlog' will be created
> under the PCI device ID for the ice driver. If the file does not exist
> then either the NVM doesn't support FW logging or debugfs is not enabled
> on the system.
>
> In addition to configuring the modules, the user can also configure the
> number of log messages (resolution) to include in a single Admin Receive
> Queue (ARQ) event.The range is 1-128 (1 means push every log message, 128
> means push only when the max AQ command buffer is full). The suggested
> value is 10.
>
> To see/change the resolution the user can read/write the
> 'fwlog/resolution' file. An example changing the value to 50 is
>
> echo 50 > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog/resolution
>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@...el.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com> (A Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
> drivers/net/ethernet/intel/ice/Makefile | 4 +-
> drivers/net/ethernet/intel/ice/ice.h | 18 +
> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 80 ++++
> drivers/net/ethernet/intel/ice/ice_common.c | 5 +
> drivers/net/ethernet/intel/ice/ice_debugfs.c | 450 ++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_fwlog.c | 231 +++++++++
> drivers/net/ethernet/intel/ice/ice_fwlog.h | 55 +++
> drivers/net/ethernet/intel/ice/ice_main.c | 21 +
> drivers/net/ethernet/intel/ice/ice_type.h | 4 +
> 9 files changed, 867 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/intel/ice/ice_debugfs.c
> create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.c
> create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.h
>
> diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
> index 960277d78e09..d43a59e5f8ee 100644
> --- a/drivers/net/ethernet/intel/ice/Makefile
> +++ b/drivers/net/ethernet/intel/ice/Makefile
> @@ -34,7 +34,8 @@ ice-y := ice_main.o \
> ice_lag.o \
> ice_ethtool.o \
> ice_repr.o \
> - ice_tc_lib.o
> + ice_tc_lib.o \
> + ice_fwlog.o
> ice-$(CONFIG_PCI_IOV) += \
> ice_sriov.o \
> ice_virtchnl.o \
> @@ -49,3 +50,4 @@ ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
> ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
> ice-$(CONFIG_ICE_SWITCHDEV) += ice_eswitch.o ice_eswitch_br.o
> ice-$(CONFIG_GNSS) += ice_gnss.o
> +ice-$(CONFIG_DEBUG_FS) += ice_debugfs.o
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index 5ac0ad12f9f1..e6dd9f6f9eee 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -556,6 +556,8 @@ struct ice_pf {
> struct ice_vsi_stats **vsi_stats;
> struct ice_sw *first_sw; /* first switch created by firmware */
> u16 eswitch_mode; /* current mode of eswitch */
> + struct dentry *ice_debugfs_pf;
> + struct dentry *ice_debugfs_pf_fwlog;
> struct ice_vfs vfs;
> DECLARE_BITMAP(features, ICE_F_MAX);
> DECLARE_BITMAP(state, ICE_STATE_NBITS);
> @@ -861,6 +863,22 @@ static inline bool ice_is_adq_active(struct ice_pf *pf)
> return false;
> }
>
> +#ifdef CONFIG_DEBUG_FS
There is no need in this CONFIG_DEBUG_FS and code should be written
without debugfs stubs.
> +void ice_debugfs_fwlog_init(struct ice_pf *pf);
> +void ice_debugfs_init(void);
> +void ice_debugfs_exit(void);
> +void ice_pf_fwlog_update_module(struct ice_pf *pf, int log_level, int module);
> +#else
> +static inline void ice_debugfs_fwlog_init(struct ice_pf *pf) { }
> +static inline void ice_debugfs_init(void) { }
> +static inline void ice_debugfs_exit(void) { }
> +static void
> +ice_pf_fwlog_update_module(struct ice_pf *pf, int log_level, int module)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_DEBUG_FS */
Thanks
Powered by blists - more mailing lists