[PATCH v2 0/2] Bug fixes to mlxbf-pmc

[PATCH v2 0/2] Bug fixes to mlxbf-pmc

[Shravan Kumar Ramani]

This submission contains 2 patches:

Patch 1 fixes an issue with matching an input event_name string with the
list of supported events in the event_list.

Patch 2 adds checks to validate the input for event and enable fields, and
prevents the user from writing invalid values to these fields.

Shravan Kumar Ramani (2):
  platform/mellanox: mlxbf-pmc: Replace strcmp with strncmp
  platform/mellanox: mlxbf-pmc: Validate event/enable input

 drivers/platform/mellanox/mlxbf-pmc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

-- 
2.30.1

[PATCH v2 1/2] platform/mellanox: mlxbf-pmc: Replace strcmp with strncmp

[Shravan Kumar Ramani]

Since the input string passed via the command line appends a newline char,
comparison using strcmp is not correct. Use the string length of the
event_list entries to match the string using strncmp instead.

Fixes: 1a218d312e65 ("platform/mellanox: mlxbf-pmc: Add Mellanox BlueField PMC driver")
Signed-off-by: Shravan Kumar Ramani <shravankr@nvidia.com>
Reviewed-by: David Thompson <davthomspson@nvidia.com>
---
 drivers/platform/mellanox/mlxbf-pmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index 900069eb186e..366c0cba447f 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -1215,7 +1215,7 @@ static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
 		return -EINVAL;
 
 	for (i = 0; i < size; ++i) {
-		if (!strcmp(evt, events[i].evt_name))
+		if (!strncmp(evt, events[i].evt_name, strlen(events[i].evt_name)))
 			return events[i].evt_num;
 	}
 
-- 
2.30.1

Re: [PATCH v2 1/2] platform/mellanox: mlxbf-pmc: Replace strcmp with strncmp

[Ilpo Järvinen]

On Wed, 18 Jun 2025, Shravan Kumar Ramani wrote:

> Since the input string passed via the command line appends a newline char,
> comparison using strcmp is not correct. Use the string length of the
> event_list entries to match the string using strncmp instead.

Please include () after any function name (don't forget those in the 
shortlog).

> Fixes: 1a218d312e65 ("platform/mellanox: mlxbf-pmc: Add Mellanox BlueField PMC driver")
> Signed-off-by: Shravan Kumar Ramani <shravankr@nvidia.com>
> Reviewed-by: David Thompson <davthomspson@nvidia.com>
> ---
>  drivers/platform/mellanox/mlxbf-pmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
> index 900069eb186e..366c0cba447f 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -1215,7 +1215,7 @@ static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
>  		return -EINVAL;
>  
>  	for (i = 0; i < size; ++i) {
> -		if (!strcmp(evt, events[i].evt_name))
> +		if (!strncmp(evt, events[i].evt_name, strlen(events[i].evt_name)))

So if there's extra garbage behind the input, it will also match 
spuriously? So store the len and reduce it if there's a trailing newline 
to make it more robust?

>  			return events[i].evt_num;
>  	}
>  
> 

-- 
 i.

[PATCH v2 2/2] platform/mellanox: mlxbf-pmc: Validate event/enable input

[Shravan Kumar Ramani]

Before programming the event info, validate the event number received as input
by checking if it exists in the event_list. Also fix a typo in the comment for
the mlxbf_pmc_get_event_name routine to correctly mention that it returns the
event name when taking the event number as input, and not the other way round.
For the enable setting, the value should be only 0 or 1. Make this check common
for all scenarios in enable store.

Fixes: 423c3361855c ("platform/mellanox: mlxbf-pmc: Add support for BlueField-3")
Signed-off-by: Shravan Kumar Ramani <shravankr@nvidia.com>
Reviewed-by: David Thompson <davthompson@nvidia.com>
---
 drivers/platform/mellanox/mlxbf-pmc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
index 366c0cba447f..fcc3392ff150 100644
--- a/drivers/platform/mellanox/mlxbf-pmc.c
+++ b/drivers/platform/mellanox/mlxbf-pmc.c
@@ -1222,7 +1222,7 @@ static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
 	return -ENODEV;
 }
 
-/* Get the event number given the name */
+/* Get the event name given the number */
 static char *mlxbf_pmc_get_event_name(const char *blk, u32 evt)
 {
 	const struct mlxbf_pmc_events *events;
@@ -1799,6 +1799,9 @@ static ssize_t mlxbf_pmc_event_store(struct device *dev,
 		err = kstrtouint(buf, 0, &evt_num);
 		if (err < 0)
 			return err;
+
+		if (!mlxbf_pmc_get_event_name(pmc->block_name[blk_num], evt_num))
+			return -EINVAL;
 	}
 
 	if (strstr(pmc->block_name[blk_num], "l3cache"))
@@ -1889,6 +1892,9 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
 	if (err < 0)
 		return err;
 
+	if (en != 0 && en != 1)
+		return -EINVAL;
+
 	if (pmc->block[blk_num].type == MLXBF_PMC_TYPE_CRSPACE) {
 		err = mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
 			MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
@@ -1905,9 +1911,6 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
 			MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
 			MLXBF_PMC_WRITE_REG_32, word);
 	} else {
-		if (en && en != 1)
-			return -EINVAL;
-
 		err = mlxbf_pmc_config_l3_counters(blk_num, false, !!en);
 		if (err)
 			return err;
-- 
2.30.1

Re: [PATCH v2 2/2] platform/mellanox: mlxbf-pmc: Validate event/enable input

[Ilpo Järvinen]

On Wed, 18 Jun 2025, Shravan Kumar Ramani wrote:

> Before programming the event info, validate the event number received as input
> by checking if it exists in the event_list. Also fix a typo in the comment for
> the mlxbf_pmc_get_event_name routine to correctly mention that it returns the
> event name when taking the event number as input, and not the other way round.
> For the enable setting, the value should be only 0 or 1. Make this check common
> for all scenarios in enable store.
> 
> Fixes: 423c3361855c ("platform/mellanox: mlxbf-pmc: Add support for BlueField-3")
> Signed-off-by: Shravan Kumar Ramani <shravankr@nvidia.com>
> Reviewed-by: David Thompson <davthompson@nvidia.com>
> ---
>  drivers/platform/mellanox/mlxbf-pmc.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlxbf-pmc.c b/drivers/platform/mellanox/mlxbf-pmc.c
> index 366c0cba447f..fcc3392ff150 100644
> --- a/drivers/platform/mellanox/mlxbf-pmc.c
> +++ b/drivers/platform/mellanox/mlxbf-pmc.c
> @@ -1222,7 +1222,7 @@ static int mlxbf_pmc_get_event_num(const char *blk, const char *evt)
>  	return -ENODEV;
>  }
>  
> -/* Get the event number given the name */
> +/* Get the event name given the number */
>  static char *mlxbf_pmc_get_event_name(const char *blk, u32 evt)
>  {
>  	const struct mlxbf_pmc_events *events;
> @@ -1799,6 +1799,9 @@ static ssize_t mlxbf_pmc_event_store(struct device *dev,
>  		err = kstrtouint(buf, 0, &evt_num);
>  		if (err < 0)
>  			return err;
> +
> +		if (!mlxbf_pmc_get_event_name(pmc->block_name[blk_num], evt_num))
> +			return -EINVAL;
>  	}
>  
>  	if (strstr(pmc->block_name[blk_num], "l3cache"))
> @@ -1889,6 +1892,9 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
>  	if (err < 0)
>  		return err;
>  
> +	if (en != 0 && en != 1)
> +		return -EINVAL;

This is using kstrtouint() but there's also kstrtobool() which would do 
this check for you.

> +
>  	if (pmc->block[blk_num].type == MLXBF_PMC_TYPE_CRSPACE) {
>  		err = mlxbf_pmc_readl(pmc->block[blk_num].mmio_base +
>  			MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
> @@ -1905,9 +1911,6 @@ static ssize_t mlxbf_pmc_enable_store(struct device *dev,
>  			MLXBF_PMC_CRSPACE_PERFMON_CTL(pmc->block[blk_num].counters),
>  			MLXBF_PMC_WRITE_REG_32, word);
>  	} else {
> -		if (en && en != 1)
> -			return -EINVAL;
> -
>  		err = mlxbf_pmc_config_l3_counters(blk_num, false, !!en);
>  		if (err)
>  			return err;
> 

-- 
 i.