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

Shravan Kumar Ramani posted 2 patches 3 months, 3 weeks ago
[PATCH v2 2/2] platform/mellanox: mlxbf-pmc: Validate event/enable input
Posted by Shravan Kumar Ramani 3 months, 3 weeks ago
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
Posted by Ilpo Järvinen 3 months, 2 weeks ago
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.