| Message ID | 20250226122543.147594-4-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mlx5: Trust lockdown health syndrome | expand |
On Wed, Feb 26, 2025 at 02:25:42PM +0200, Tariq Toukan wrote: > From: Shahar Shitrit <shshitrit@nvidia.com> > > Expose crr bit in struct health buffer. When set, it indicates that > the error cannot be recovered without flow involving a cold reset. > Add its value to the health buffer info log. > > Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com> > Reviewed-by: Moshe Shemesh <moshe@nvidia.com> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/health.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c > index 665cbce89175..c7ff646e0865 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c > @@ -96,6 +96,11 @@ static int mlx5_health_get_rfr(u8 rfr_severity) > return rfr_severity >> MLX5_RFR_BIT_OFFSET; > } > > +static int mlx5_health_get_crr(u8 rfr_severity) > +{ > + return (rfr_severity >> MLX5_CRR_BIT_OFFSET) & 0x01; > +} > + > static bool sensor_fw_synd_rfr(struct mlx5_core_dev *dev) > { > struct mlx5_core_health *health = &dev->priv.health; > @@ -442,12 +447,15 @@ static void print_health_info(struct mlx5_core_dev *dev) > mlx5_log(dev, severity, "time %u\n", ioread32be(&h->time)); > mlx5_log(dev, severity, "hw_id 0x%08x\n", ioread32be(&h->hw_id)); > mlx5_log(dev, severity, "rfr %d\n", mlx5_health_get_rfr(rfr_severity)); > + mlx5_log(dev, severity, "crr %d\n", mlx5_health_get_crr(rfr_severity)); > mlx5_log(dev, severity, "severity %d (%s)\n", severity, mlx5_loglevel_str(severity)); > mlx5_log(dev, severity, "irisc_index %d\n", ioread8(&h->irisc_index)); > mlx5_log(dev, severity, "synd 0x%x: %s\n", ioread8(&h->synd), > hsynd_str(ioread8(&h->synd))); > mlx5_log(dev, severity, "ext_synd 0x%04x\n", ioread16be(&h->ext_synd)); > mlx5_log(dev, severity, "raw fw_ver 0x%08x\n", ioread32be(&h->fw_ver)); > + if (mlx5_health_get_crr(rfr_severity)) > + mlx5_core_warn(dev, "Cold reset is required\n"); I wonder if it shouldn't be right after the print about crr value to tell the user that cold reset is required because of that value. Patch looks fine, thanks. Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > } > > static int > -- > 2.45.0
On 27/02/2025 7:56, Michal Swiatkowski wrote: > On Wed, Feb 26, 2025 at 02:25:42PM +0200, Tariq Toukan wrote: >> From: Shahar Shitrit <shshitrit@nvidia.com> >> >> Expose crr bit in struct health buffer. When set, it indicates that >> the error cannot be recovered without flow involving a cold reset. >> Add its value to the health buffer info log. >> >> Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com> >> Reviewed-by: Moshe Shemesh <moshe@nvidia.com> >> Signed-off-by: Tariq Toukan <tariqt@nvidia.com> >> --- >> drivers/net/ethernet/mellanox/mlx5/core/health.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c >> index 665cbce89175..c7ff646e0865 100644 >> --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c >> @@ -96,6 +96,11 @@ static int mlx5_health_get_rfr(u8 rfr_severity) >> return rfr_severity >> MLX5_RFR_BIT_OFFSET; >> } >> >> +static int mlx5_health_get_crr(u8 rfr_severity) >> +{ >> + return (rfr_severity >> MLX5_CRR_BIT_OFFSET) & 0x01; >> +} >> + >> static bool sensor_fw_synd_rfr(struct mlx5_core_dev *dev) >> { >> struct mlx5_core_health *health = &dev->priv.health; >> @@ -442,12 +447,15 @@ static void print_health_info(struct mlx5_core_dev *dev) >> mlx5_log(dev, severity, "time %u\n", ioread32be(&h->time)); >> mlx5_log(dev, severity, "hw_id 0x%08x\n", ioread32be(&h->hw_id)); >> mlx5_log(dev, severity, "rfr %d\n", mlx5_health_get_rfr(rfr_severity)); >> + mlx5_log(dev, severity, "crr %d\n", mlx5_health_get_crr(rfr_severity)); >> mlx5_log(dev, severity, "severity %d (%s)\n", severity, mlx5_loglevel_str(severity)); >> mlx5_log(dev, severity, "irisc_index %d\n", ioread8(&h->irisc_index)); >> mlx5_log(dev, severity, "synd 0x%x: %s\n", ioread8(&h->synd), >> hsynd_str(ioread8(&h->synd))); >> mlx5_log(dev, severity, "ext_synd 0x%04x\n", ioread16be(&h->ext_synd)); >> mlx5_log(dev, severity, "raw fw_ver 0x%08x\n", ioread32be(&h->fw_ver)); >> + if (mlx5_health_get_crr(rfr_severity)) >> + mlx5_core_warn(dev, "Cold reset is required\n"); > I wonder if it shouldn't be right after the print about crr value to > tell the user that cold reset is required because of that value. > I think it's fine here, to not interfere the mlx5_log sequence. Also, in the future we might have multiple cold reset reasons, generating the same single print. I'll keep it as-is. > Patch looks fine, thanks. > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Thanks for your review.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c index 665cbce89175..c7ff646e0865 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c @@ -96,6 +96,11 @@ static int mlx5_health_get_rfr(u8 rfr_severity) return rfr_severity >> MLX5_RFR_BIT_OFFSET; } +static int mlx5_health_get_crr(u8 rfr_severity) +{ + return (rfr_severity >> MLX5_CRR_BIT_OFFSET) & 0x01; +} + static bool sensor_fw_synd_rfr(struct mlx5_core_dev *dev) { struct mlx5_core_health *health = &dev->priv.health; @@ -442,12 +447,15 @@ static void print_health_info(struct mlx5_core_dev *dev) mlx5_log(dev, severity, "time %u\n", ioread32be(&h->time)); mlx5_log(dev, severity, "hw_id 0x%08x\n", ioread32be(&h->hw_id)); mlx5_log(dev, severity, "rfr %d\n", mlx5_health_get_rfr(rfr_severity)); + mlx5_log(dev, severity, "crr %d\n", mlx5_health_get_crr(rfr_severity)); mlx5_log(dev, severity, "severity %d (%s)\n", severity, mlx5_loglevel_str(severity)); mlx5_log(dev, severity, "irisc_index %d\n", ioread8(&h->irisc_index)); mlx5_log(dev, severity, "synd 0x%x: %s\n", ioread8(&h->synd), hsynd_str(ioread8(&h->synd))); mlx5_log(dev, severity, "ext_synd 0x%04x\n", ioread16be(&h->ext_synd)); mlx5_log(dev, severity, "raw fw_ver 0x%08x\n", ioread32be(&h->fw_ver)); + if (mlx5_health_get_crr(rfr_severity)) + mlx5_core_warn(dev, "Cold reset is required\n"); } static int