[PATCH net 0/3] mlxsw: Fixes

[PATCH net 0/3] mlxsw: Fixes

[Petr Machata]

This patchset fixes the following issues:

- During driver de-initialization the driver unregisters the EMAD
  response trap by setting its action to DISCARD. However the manual
  only permits TRAP and FORWARD, and future firmware versions will
  enforce this.

  In patch #1, suppress the error message by aligning the driver to the
  manual and use a FORWARD (NOP) action when unregistering the trap.

- The driver queries the Management Capabilities Mask (MCAM) register
  during initialization to understand if certain features are supported.

  However, not all firmware versions support this register, leading to
  the driver failing to load.

  Patches #2 and #3 fix this issue by treating an error in the register
  query as an indication that the feature is not supported.

Ido Schimmel (3):
  mlxsw: core: Unregister EMAD trap using FORWARD action
  mlxsw: core_env: Fix driver initialization with old firmware
  mlxsw: pci: Fix driver initialization with old firmware

 drivers/net/ethernet/mellanox/mlxsw/core.c     |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/core_env.c | 10 ++++------
 drivers/net/ethernet/mellanox/mlxsw/pci.c      | 10 ++++------
 3 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.43.0

[PATCH net 1/3] mlxsw: core: Unregister EMAD trap using FORWARD action

[Petr Machata]

From: Ido Schimmel <idosch@nvidia.com>

The device's manual (PRM - Programmer's Reference Manual) classifies the
trap that is used to deliver EMAD responses as an "event trap". Among
other things, it means that the only actions that can be associated with
the trap are TRAP and FORWARD (NOP).

Currently, during driver de-initialization the driver unregisters the
trap by setting its action to DISCARD, which violates the above
guideline. Future firmware versions will prevent such misuses by
returning an error. This does not prevent the driver from working, but
an error will be printed to the kernel log during module removal /
devlink reload:

mlxsw_spectrum 0000:03:00.0: Reg cmd access status failed (status=7(bad parameter))
mlxsw_spectrum 0000:03:00.0: Reg cmd access failed (reg_id=7003(hpkt),type=write)

Suppress the error message by aligning the driver to the manual and use
a FORWARD (NOP) action when unregistering the trap.

Fixes: 4ec14b7634b2 ("mlxsw: Add interface to access registers and process events")
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Amit Cohen <amcohen@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index e4d7739bd7c8..4a79c0d7e7ad 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -849,7 +849,7 @@ static void mlxsw_emad_rx_listener_func(struct sk_buff *skb, u16 local_port,
 
 static const struct mlxsw_listener mlxsw_emad_rx_listener =
 	MLXSW_RXL(mlxsw_emad_rx_listener_func, ETHEMAD, TRAP_TO_CPU, false,
-		  EMAD, DISCARD);
+		  EMAD, FORWARD);
 
 static int mlxsw_emad_tlv_enable(struct mlxsw_core *mlxsw_core)
 {
-- 
2.43.0

Re: [PATCH net 1/3] mlxsw: core: Unregister EMAD trap using FORWARD action

[Simon Horman]

On Tue, Apr 16, 2024 at 12:24:14PM +0200, Petr Machata wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> The device's manual (PRM - Programmer's Reference Manual) classifies the
> trap that is used to deliver EMAD responses as an "event trap". Among
> other things, it means that the only actions that can be associated with
> the trap are TRAP and FORWARD (NOP).
> 
> Currently, during driver de-initialization the driver unregisters the
> trap by setting its action to DISCARD, which violates the above
> guideline. Future firmware versions will prevent such misuses by
> returning an error. This does not prevent the driver from working, but
> an error will be printed to the kernel log during module removal /
> devlink reload:
> 
> mlxsw_spectrum 0000:03:00.0: Reg cmd access status failed (status=7(bad parameter))
> mlxsw_spectrum 0000:03:00.0: Reg cmd access failed (reg_id=7003(hpkt),type=write)
> 
> Suppress the error message by aligning the driver to the manual and use
> a FORWARD (NOP) action when unregistering the trap.
> 
> Fixes: 4ec14b7634b2 ("mlxsw: Add interface to access registers and process events")
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: Amit Cohen <amcohen@nvidia.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net 2/3] mlxsw: core_env: Fix driver initialization with old firmware

[Petr Machata]

From: Ido Schimmel <idosch@nvidia.com>

The driver queries the Management Capabilities Mask (MCAM) register
during initialization to understand if it can read up to 128 bytes from
transceiver modules.

However, not all firmware versions support this register, leading to the
driver failing to load.

Fix by treating an error in the register query as an indication that the
feature is not supported.

Fixes: 1f4aea1f72da ("mlxsw: core_env: Read transceiver module EEPROM in 128 bytes chunks")
Reported-by: Tim 'mithro' Ansell <me@mith.ro>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core_env.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_env.c b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
index 53b150b7ae4e..5d02b6aef4d2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_env.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
@@ -1360,17 +1360,15 @@ static struct mlxsw_linecards_event_ops mlxsw_env_event_ops = {
 static int mlxsw_env_max_module_eeprom_len_query(struct mlxsw_env *mlxsw_env)
 {
 	char mcam_pl[MLXSW_REG_MCAM_LEN];
-	bool mcia_128b_supported;
+	bool mcia_128b_supported = false;
 	int err;
 
 	mlxsw_reg_mcam_pack(mcam_pl,
 			    MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES);
 	err = mlxsw_reg_query(mlxsw_env->core, MLXSW_REG(mcam), mcam_pl);
-	if (err)
-		return err;
-
-	mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_MCIA_128B,
-			      &mcia_128b_supported);
+	if (!err)
+		mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_MCIA_128B,
+				      &mcia_128b_supported);
 
 	mlxsw_env->max_eeprom_len = mcia_128b_supported ? 128 : 48;
 
-- 
2.43.0

Re: [PATCH net 2/3] mlxsw: core_env: Fix driver initialization with old firmware

[Simon Horman]

On Tue, Apr 16, 2024 at 12:24:15PM +0200, Petr Machata wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> The driver queries the Management Capabilities Mask (MCAM) register
> during initialization to understand if it can read up to 128 bytes from
> transceiver modules.
> 
> However, not all firmware versions support this register, leading to the
> driver failing to load.
> 
> Fix by treating an error in the register query as an indication that the
> feature is not supported.
> 
> Fixes: 1f4aea1f72da ("mlxsw: core_env: Read transceiver module EEPROM in 128 bytes chunks")
> Reported-by: Tim 'mithro' Ansell <me@mith.ro>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/core_env.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_env.c b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
> index 53b150b7ae4e..5d02b6aef4d2 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_env.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
> @@ -1360,17 +1360,15 @@ static struct mlxsw_linecards_event_ops mlxsw_env_event_ops = {
>  static int mlxsw_env_max_module_eeprom_len_query(struct mlxsw_env *mlxsw_env)
>  {
>  	char mcam_pl[MLXSW_REG_MCAM_LEN];
> -	bool mcia_128b_supported;
> +	bool mcia_128b_supported = false;
>  	int err;
>  
>  	mlxsw_reg_mcam_pack(mcam_pl,
>  			    MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES);
>  	err = mlxsw_reg_query(mlxsw_env->core, MLXSW_REG(mcam), mcam_pl);
> -	if (err)
> -		return err;
> -
> -	mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_MCIA_128B,
> -			      &mcia_128b_supported);
> +	if (!err)
> +		mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_MCIA_128B,
> +				      &mcia_128b_supported);
>  
>  	mlxsw_env->max_eeprom_len = mcia_128b_supported ? 128 : 48;

Hi Petr and Ido,

This function now always returns 0.
Perhaps the return type can be updated to void?

Re: [PATCH net 2/3] mlxsw: core_env: Fix driver initialization with old firmware

[Ido Schimmel]

On Tue, Apr 16, 2024 at 03:36:26PM +0100, Simon Horman wrote:
> This function now always returns 0.
> Perhaps the return type can be updated to void?

OK, will change to void.

Thanks

[PATCH net 3/3] mlxsw: pci: Fix driver initialization with old firmware

[Petr Machata]

From: Ido Schimmel <idosch@nvidia.com>

The driver queries the Management Capabilities Mask (MCAM) register
during initialization to understand if a new and deeper reset flow is
supported.

However, not all firmware versions support this register, leading to the
driver failing to load.

Fix by treating an error in the register query as an indication that the
feature is not supported.

Fixes: f257c73e5356 ("mlxsw: pci: Add support for new reset flow")
Reported-by: Tim 'mithro' Ansell <me@mith.ro>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 4d617057af25..13fd067c39ed 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1545,7 +1545,7 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
 {
 	struct pci_dev *pdev = mlxsw_pci->pdev;
 	char mcam_pl[MLXSW_REG_MCAM_LEN];
-	bool pci_reset_supported;
+	bool pci_reset_supported = false;
 	u32 sys_status;
 	int err;
 
@@ -1563,11 +1563,9 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
 	mlxsw_reg_mcam_pack(mcam_pl,
 			    MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES);
 	err = mlxsw_reg_query(mlxsw_pci->core, MLXSW_REG(mcam), mcam_pl);
-	if (err)
-		return err;
-
-	mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_PCI_RESET,
-			      &pci_reset_supported);
+	if (!err)
+		mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_PCI_RESET,
+				      &pci_reset_supported);
 
 	if (pci_reset_supported) {
 		pci_dbg(pdev, "Starting PCI reset flow\n");
-- 
2.43.0

Re: [PATCH net 3/3] mlxsw: pci: Fix driver initialization with old firmware

[Simon Horman]

On Tue, Apr 16, 2024 at 12:24:16PM +0200, Petr Machata wrote:
> From: Ido Schimmel <idosch@nvidia.com>
> 
> The driver queries the Management Capabilities Mask (MCAM) register
> during initialization to understand if a new and deeper reset flow is
> supported.
> 
> However, not all firmware versions support this register, leading to the
> driver failing to load.
> 
> Fix by treating an error in the register query as an indication that the
> feature is not supported.
> 
> Fixes: f257c73e5356 ("mlxsw: pci: Add support for new reset flow")
> Reported-by: Tim 'mithro' Ansell <me@mith.ro>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 3/3] mlxsw: pci: Fix driver initialization with old firmware

[Kalesh Anakkur Purayil]

[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]

On Tue, Apr 16, 2024 at 3:57 PM Petr Machata <petrm@nvidia.com> wrote:
>
> From: Ido Schimmel <idosch@nvidia.com>
>
> The driver queries the Management Capabilities Mask (MCAM) register
> during initialization to understand if a new and deeper reset flow is
> supported.
>
> However, not all firmware versions support this register, leading to the
> driver failing to load.
>
> Fix by treating an error in the register query as an indication that the
> feature is not supported.
>
> Fixes: f257c73e5356 ("mlxsw: pci: Add support for new reset flow")
> Reported-by: Tim 'mithro' Ansell <me@mith.ro>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Petr Machata <petrm@nvidia.com>

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
> index 4d617057af25..13fd067c39ed 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
> @@ -1545,7 +1545,7 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
>  {
>         struct pci_dev *pdev = mlxsw_pci->pdev;
>         char mcam_pl[MLXSW_REG_MCAM_LEN];
> -       bool pci_reset_supported;
> +       bool pci_reset_supported = false;
>         u32 sys_status;
>         int err;
>
> @@ -1563,11 +1563,9 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
>         mlxsw_reg_mcam_pack(mcam_pl,
>                             MLXSW_REG_MCAM_FEATURE_GROUP_ENHANCED_FEATURES);
>         err = mlxsw_reg_query(mlxsw_pci->core, MLXSW_REG(mcam), mcam_pl);
> -       if (err)
> -               return err;
> -
> -       mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_PCI_RESET,
> -                             &pci_reset_supported);
> +       if (!err)
> +               mlxsw_reg_mcam_unpack(mcam_pl, MLXSW_REG_MCAM_PCI_RESET,
> +                                     &pci_reset_supported);
>
>         if (pci_reset_supported) {
>                 pci_dbg(pdev, "Starting PCI reset flow\n");
> --
> 2.43.0
>
>


-- 
Regards,
Kalesh A P

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4239 bytes --]

[PATCH net 0/3] mlxsw: Fixes

[Petr Machata]

This patchset fixes two issues with mlxsw driver initialization, and a
memory corruption issue in shared buffer occupancy handling.

Ido Schimmel (3):
  mlxsw: pci: Fix driver initialization with Spectrum-4
  mlxsw: core_thermal: Fix driver initialization failure
  mlxsw: spectrum_buffers: Fix memory corruptions on Spectrum-4 systems

 .../ethernet/mellanox/mlxsw/core_thermal.c    | 50 ++++++++++---------
 drivers/net/ethernet/mellanox/mlxsw/pci.c     | 18 +++++--
 drivers/net/ethernet/mellanox/mlxsw/reg.h     |  2 +
 .../mellanox/mlxsw/spectrum_buffers.c         | 20 +++++---
 4 files changed, 57 insertions(+), 33 deletions(-)

-- 
2.45.0