[PATCH net-next 0/4] mlx5: Trust lockdown health syndrome

[PATCH net-next 0/4] mlx5: Trust lockdown health syndrome

[Tariq Toukan]

Hi,

This series introduces a new error type in the health syndrome,
specifically for trust lock-down.  Additionally, it exposes the CRR bit
in the health buffer, which, when set, indicates that the error cannot
be recovered without a process involving a cold reset. We add The CRR
bit value to the health buffer info log and update it to be logged on
any syndrome.

Regards,
Tariq


Moshe Shemesh (2):
  net/mlx5: Avoid report two health errors on same syndrome
  net/mlx5: Log health buffer data on any syndrome

Shahar Shitrit (2):
  net/mlx5: Expose crr in health buffer
  net/mlx5: Add trust lockdown error to health syndrome print function

 drivers/net/ethernet/mellanox/mlx5/core/health.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


base-commit: 91c8d8e4b7a38dc099b26e14b22f814ca4e75089
-- 
2.45.0

[PATCH net-next 1/4] net/mlx5: Avoid report two health errors on same syndrome

[Tariq Toukan]

From: Moshe Shemesh <moshe@nvidia.com>

In case health counter has not increased for few polling intervals, miss
counter will reach max misses threshold and health report will be
triggered for FW health reporter. In case syndrome found on same health
poll another health report will be triggered.

Avoid two health reports on same syndrome by marking this syndrome as
already known.

Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/health.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c
index a6329ca2d9bf..52c8035547be 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/health.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c
@@ -799,6 +799,7 @@ static void poll_health(struct timer_list *t)
 	health->prev = count;
 	if (health->miss_counter == MAX_MISSES) {
 		mlx5_core_err(dev, "device's health compromised - reached miss count\n");
+		health->synd = ioread8(&h->synd);
 		print_health_info(dev);
 		queue_work(health->wq, &health->report_work);
 	}
-- 
2.45.0

Re: [PATCH net-next 1/4] net/mlx5: Avoid report two health errors on same syndrome

[Kalesh Anakkur Purayil]

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

On Wed, Feb 26, 2025 at 6:01 PM Tariq Toukan <tariqt@nvidia.com> wrote:
>
> From: Moshe Shemesh <moshe@nvidia.com>
>
> In case health counter has not increased for few polling intervals, miss
> counter will reach max misses threshold and health report will be
> triggered for FW health reporter. In case syndrome found on same health
> poll another health report will be triggered.
>
> Avoid two health reports on same syndrome by marking this syndrome as
> already known.
>
> Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>


-- 
Regards,
Kalesh AP

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

[PATCH net-next 2/4] net/mlx5: Log health buffer data on any syndrome

[Tariq Toukan]

From: Moshe Shemesh <moshe@nvidia.com>

Currently health buffer data is logged either when FW fatal error
detected or miss counter reached max misses threshold.

Log health buffer whenever new health syndrome is detected.

Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/health.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c
index 52c8035547be..665cbce89175 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/health.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c
@@ -806,8 +806,10 @@ static void poll_health(struct timer_list *t)
 
 	prev_synd = health->synd;
 	health->synd = ioread8(&h->synd);
-	if (health->synd && health->synd != prev_synd)
+	if (health->synd && health->synd != prev_synd) {
+		print_health_info(dev);
 		queue_work(health->wq, &health->report_work);
+	}
 
 out:
 	mod_timer(&health->timer, get_next_poll_jiffies(dev));
-- 
2.45.0

Re: [PATCH net-next 2/4] net/mlx5: Log health buffer data on any syndrome

[Kalesh Anakkur Purayil]

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

On Wed, Feb 26, 2025 at 6:01 PM Tariq Toukan <tariqt@nvidia.com> wrote:
>
> From: Moshe Shemesh <moshe@nvidia.com>
>
> Currently health buffer data is logged either when FW fatal error
> detected or miss counter reached max misses threshold.
>
> Log health buffer whenever new health syndrome is detected.
>
> Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>


-- 
Regards,
Kalesh AP

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

[PATCH net-next 3/4] net/mlx5: Expose crr in health buffer

[Tariq Toukan]

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");
 }
 
 static int
-- 
2.45.0

Re: [PATCH net-next 3/4] net/mlx5: Expose crr in health buffer

[Michal Swiatkowski]

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

Re: [PATCH net-next 3/4] net/mlx5: Expose crr in health buffer

[Tariq Toukan]

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.

[PATCH net-next 4/4] net/mlx5: Add trust lockdown error to health syndrome print function

[Tariq Toukan]

From: Shahar Shitrit <shshitrit@nvidia.com>

Add the new health syndrome value to hsynd_str() function
to indicate that the device got a trust lockdown fault.

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 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c
index c7ff646e0865..91613d5a36cd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/health.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c
@@ -380,6 +380,8 @@ static const char *hsynd_str(u8 synd)
 		return "High temperature";
 	case MLX5_INITIAL_SEG_HEALTH_SYNDROME_ICM_PCI_POISONED_ERR:
 		return "ICM fetch PCI data poisoned error";
+	case MLX5_INITIAL_SEG_HEALTH_SYNDROME_TRUST_LOCKDOWN_ERR:
+		return "Trust lockdown error";
 	default:
 		return "unrecognized error";
 	}
-- 
2.45.0

Re: [PATCH net-next 4/4] net/mlx5: Add trust lockdown error to health syndrome print function

[Kalesh Anakkur Purayil]

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

On Wed, Feb 26, 2025 at 6:02 PM Tariq Toukan <tariqt@nvidia.com> wrote:
>
> From: Shahar Shitrit <shshitrit@nvidia.com>
>
> Add the new health syndrome value to hsynd_str() function
> to indicate that the device got a trust lockdown fault.
>
> Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>


-- 
Regards,
Kalesh AP

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

Re: [PATCH net-next 0/4] mlx5: Trust lockdown health syndrome

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Wed, 26 Feb 2025 14:25:39 +0200 you wrote:
> Hi,
> 
> This series introduces a new error type in the health syndrome,
> specifically for trust lock-down.  Additionally, it exposes the CRR bit
> in the health buffer, which, when set, indicates that the error cannot
> be recovered without a process involving a cold reset. We add The CRR
> bit value to the health buffer info log and update it to be logged on
> any syndrome.
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] net/mlx5: Avoid report two health errors on same syndrome
    https://git.kernel.org/netdev/net-next/c/b5d7b2f04ebc
  - [net-next,2/4] net/mlx5: Log health buffer data on any syndrome
    https://git.kernel.org/netdev/net-next/c/6bdce277a326
  - [net-next,3/4] net/mlx5: Expose crr in health buffer
    https://git.kernel.org/netdev/net-next/c/63f26199721f
  - [net-next,4/4] net/mlx5: Add trust lockdown error to health syndrome print function
    https://git.kernel.org/netdev/net-next/c/680173b6bb6b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html