[v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

[PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Martin Kletzander 2 months, 2 weeks ago

When resctrl is mounted with the "mba_MBps" option the default (maximum)
bandwidth is the maximum unsigned value for the type.  However when
using the same value that already exists in the schemata file it is then
rounded up to the bandwidth granularity and overflows to a small number
instead, making it difficult to reset memory bandwidth allocation value
back to its default.

Since the granularity and minimum bandwidth are not used when the
software controller is used (resctrl is mounted with the "mba_MBps"),
skip the rounding up as well and return early from bw_validate().

Signed-off-by: Martin Kletzander <nert.pinx@gmail.com>
---
Changes in v2:
 - actually save the value in the output parameter @data

 arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 50fa1fe9a073..702b1a372e9c 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -48,8 +48,13 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 		return false;
 	}
 
-	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
-	    !is_mba_sc(r)) {
+	/* Nothing else to do if software controller is enabled */
+	if (is_mba_sc(r)) {
+		*data = bw;
+		return true;
+	}
+
+	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
 		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
 				    r->membw.min_bw, r->default_ctrl);
 		return false;
-- 
2.46.0

Re: [PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Reinette Chatre 2 months, 1 week ago

Hi Martin,

On 9/16/24 6:07 AM, Martin Kletzander wrote:
> When resctrl is mounted with the "mba_MBps" option the default (maximum)
> bandwidth is the maximum unsigned value for the type.  However when
> using the same value that already exists in the schemata file it is then
> rounded up to the bandwidth granularity and overflows to a small number
> instead, making it difficult to reset memory bandwidth allocation value
> back to its default.
> 
> Since the granularity and minimum bandwidth are not used when the
> software controller is used (resctrl is mounted with the "mba_MBps"),
> skip the rounding up as well and return early from bw_validate().

Thank you very much for finding the issue and proposing a fix.

> 
> Signed-off-by: Martin Kletzander <nert.pinx@gmail.com>
> ---
> Changes in v2:
>   - actually save the value in the output parameter @data
> 
>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 50fa1fe9a073..702b1a372e9c 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -48,8 +48,13 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>   		return false;
>   	}
>   
> -	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
> -	    !is_mba_sc(r)) {
> +	/* Nothing else to do if software controller is enabled */
> +	if (is_mba_sc(r)) {
> +		*data = bw;
> +		return true;

While this would fix the scenario tested I do not believe this fully addresses the
overflow issue. As I understand the test wrote U32_MAX to the schemata file,
which triggered the overflow because of the rounding and is fixed by this patch. Looks like,
after this patch, writing "U32_MAX + 1" will still trigger the overflow.

The overflow appears to result from some inconsistent type use and not using
appropriate parsing API that is able to detect overflow.

How about something like below:

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 50fa1fe9a073..53defc5a6784 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -29,10 +29,10 @@
   * hardware. The allocated bandwidth percentage is rounded to the next
   * control step available on the hardware.
   */
-static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
+static bool bw_validate(char *buf, u32 *data, struct rdt_resource *r)
  {
-	unsigned long bw;
  	int ret;
+	u32 bw;
  
  	/*
  	 * Only linear delay values is supported for current Intel SKUs.
@@ -42,14 +42,19 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
  		return false;
  	}
  
-	ret = kstrtoul(buf, 10, &bw);
+	ret = kstrtou32(buf, 10, &bw);
  	if (ret) {
-		rdt_last_cmd_printf("Non-decimal digit in MB value %s\n", buf);
+		rdt_last_cmd_printf("Invalid MB value %s\n", buf);
  		return false;
  	}
  
-	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
-	    !is_mba_sc(r)) {
+	/* Nothing else to do if software controller is enabled. */
+	if (is_mba_sc(r)) {
+		*data = bw;
+		return true;
+	}
+
+	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
  		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
  				    r->membw.min_bw, r->default_ctrl);
  		return false;
@@ -65,7 +70,7 @@ int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
  	struct resctrl_staged_config *cfg;
  	u32 closid = data->rdtgrp->closid;
  	struct rdt_resource *r = s->res;
-	unsigned long bw_val;
+	u32 bw_val;
  
  	cfg = &d->staged_config[s->conf_type];
  	if (cfg->have_new_ctrl) {



> +	}
> +
> +	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
>   		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
>   				    r->membw.min_bw, r->default_ctrl);
>   		return false;

Reinette

Re: [PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Martin Kletzander 2 months, 1 week ago

On Mon, Sep 16, 2024 at 09:56:01AM -0700, Reinette Chatre wrote:
>Hi Martin,
>
>On 9/16/24 6:07 AM, Martin Kletzander wrote:
>> When resctrl is mounted with the "mba_MBps" option the default (maximum)
>> bandwidth is the maximum unsigned value for the type.  However when
>> using the same value that already exists in the schemata file it is then
>> rounded up to the bandwidth granularity and overflows to a small number
>> instead, making it difficult to reset memory bandwidth allocation value
>> back to its default.
>>
>> Since the granularity and minimum bandwidth are not used when the
>> software controller is used (resctrl is mounted with the "mba_MBps"),
>> skip the rounding up as well and return early from bw_validate().
>
>Thank you very much for finding the issue and proposing a fix.
>
>>
>> Signed-off-by: Martin Kletzander <nert.pinx@gmail.com>
>> ---
>> Changes in v2:
>>   - actually save the value in the output parameter @data
>>
>>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> index 50fa1fe9a073..702b1a372e9c 100644
>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> @@ -48,8 +48,13 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>>   		return false;
>>   	}
>>
>> -	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
>> -	    !is_mba_sc(r)) {
>> +	/* Nothing else to do if software controller is enabled */
>> +	if (is_mba_sc(r)) {
>> +		*data = bw;
>> +		return true;
>
>While this would fix the scenario tested I do not believe this fully addresses the
>overflow issue. As I understand the test wrote U32_MAX to the schemata file,
>which triggered the overflow because of the rounding and is fixed by this patch. Looks like,
>after this patch, writing "U32_MAX + 1" will still trigger the overflow.
>
>The overflow appears to result from some inconsistent type use and not using
>appropriate parsing API that is able to detect overflow.
>
>How about something like below:
>

That makes much more sense, I have not considered changing the data type
as I wanted to keep the changes at minimum, but your solution is even
better.  Should I leave the fix up to you or do you want me to send a v3?

>diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>index 50fa1fe9a073..53defc5a6784 100644
>--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>@@ -29,10 +29,10 @@
>   * hardware. The allocated bandwidth percentage is rounded to the next
>   * control step available on the hardware.
>   */
>-static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>+static bool bw_validate(char *buf, u32 *data, struct rdt_resource *r)
>  {
>-	unsigned long bw;
>  	int ret;
>+	u32 bw;
>
>  	/*
>  	 * Only linear delay values is supported for current Intel SKUs.
>@@ -42,14 +42,19 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>  		return false;
>  	}
>
>-	ret = kstrtoul(buf, 10, &bw);
>+	ret = kstrtou32(buf, 10, &bw);
>  	if (ret) {
>-		rdt_last_cmd_printf("Non-decimal digit in MB value %s\n", buf);
>+		rdt_last_cmd_printf("Invalid MB value %s\n", buf);
>  		return false;
>  	}
>
>-	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
>-	    !is_mba_sc(r)) {
>+	/* Nothing else to do if software controller is enabled. */
>+	if (is_mba_sc(r)) {
>+		*data = bw;
>+		return true;
>+	}
>+
>+	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
>  		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
>  				    r->membw.min_bw, r->default_ctrl);
>  		return false;
>@@ -65,7 +70,7 @@ int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
>  	struct resctrl_staged_config *cfg;
>  	u32 closid = data->rdtgrp->closid;
>  	struct rdt_resource *r = s->res;
>-	unsigned long bw_val;
>+	u32 bw_val;
>
>  	cfg = &d->staged_config[s->conf_type];
>  	if (cfg->have_new_ctrl) {
>
>
>
>> +	}
>> +
>> +	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
>>   		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
>>   				    r->membw.min_bw, r->default_ctrl);
>>   		return false;
>
>Reinette

Re: [PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Reinette Chatre 2 months, 1 week ago

Hi Martin,

On 9/17/24 1:12 AM, Martin Kletzander wrote:
> On Mon, Sep 16, 2024 at 09:56:01AM -0700, Reinette Chatre wrote:
>> On 9/16/24 6:07 AM, Martin Kletzander wrote:
>>> When resctrl is mounted with the "mba_MBps" option the default (maximum)
>>> bandwidth is the maximum unsigned value for the type.  However when
>>> using the same value that already exists in the schemata file it is then
>>> rounded up to the bandwidth granularity and overflows to a small number
>>> instead, making it difficult to reset memory bandwidth allocation value
>>> back to its default.
>>>
>>> Since the granularity and minimum bandwidth are not used when the
>>> software controller is used (resctrl is mounted with the "mba_MBps"),
>>> skip the rounding up as well and return early from bw_validate().
>>
>> Thank you very much for finding the issue and proposing a fix.
>>
>>>
>>> Signed-off-by: Martin Kletzander <nert.pinx@gmail.com>
>>> ---
>>> Changes in v2:
>>>   - actually save the value in the output parameter @data
>>>
>>>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>> index 50fa1fe9a073..702b1a372e9c 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>> @@ -48,8 +48,13 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>>>           return false;
>>>       }
>>>
>>> -    if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
>>> -        !is_mba_sc(r)) {
>>> +    /* Nothing else to do if software controller is enabled */
>>> +    if (is_mba_sc(r)) {
>>> +        *data = bw;
>>> +        return true;
>>
>> While this would fix the scenario tested I do not believe this fully addresses the
>> overflow issue. As I understand the test wrote U32_MAX to the schemata file,
>> which triggered the overflow because of the rounding and is fixed by this patch. Looks like,
>> after this patch, writing "U32_MAX + 1" will still trigger the overflow.
>>
>> The overflow appears to result from some inconsistent type use and not using
>> appropriate parsing API that is able to detect overflow.
>>
>> How about something like below:
>>
> 
> That makes much more sense, I have not considered changing the data type
> as I wanted to keep the changes at minimum, but your solution is even
> better.  Should I leave the fix up to you or do you want me to send a v3?

Could you please try it out to ensure it works for you and then send a v3?

Thank you very much.

Reinette

Re: [PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Martin Kletzander 2 months, 1 week ago

On Tue, Sep 17, 2024 at 08:08:50AM -0700, Reinette Chatre wrote:
>Hi Martin,
>
>On 9/17/24 1:12 AM, Martin Kletzander wrote:
>> On Mon, Sep 16, 2024 at 09:56:01AM -0700, Reinette Chatre wrote:
>>> On 9/16/24 6:07 AM, Martin Kletzander wrote:
>>>> When resctrl is mounted with the "mba_MBps" option the default (maximum)
>>>> bandwidth is the maximum unsigned value for the type.  However when
>>>> using the same value that already exists in the schemata file it is then
>>>> rounded up to the bandwidth granularity and overflows to a small number
>>>> instead, making it difficult to reset memory bandwidth allocation value
>>>> back to its default.
>>>>
>>>> Since the granularity and minimum bandwidth are not used when the
>>>> software controller is used (resctrl is mounted with the "mba_MBps"),
>>>> skip the rounding up as well and return early from bw_validate().
>>>
>>> Thank you very much for finding the issue and proposing a fix.
>>>
>>>>
>>>> Signed-off-by: Martin Kletzander <nert.pinx@gmail.com>
>>>> ---
>>>> Changes in v2:
>>>>   - actually save the value in the output parameter @data
>>>>
>>>>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++++++--
>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>>> index 50fa1fe9a073..702b1a372e9c 100644
>>>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>>> @@ -48,8 +48,13 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>>>>           return false;
>>>>       }
>>>>
>>>> -    if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
>>>> -        !is_mba_sc(r)) {
>>>> +    /* Nothing else to do if software controller is enabled */
>>>> +    if (is_mba_sc(r)) {
>>>> +        *data = bw;
>>>> +        return true;
>>>
>>> While this would fix the scenario tested I do not believe this fully addresses the
>>> overflow issue. As I understand the test wrote U32_MAX to the schemata file,
>>> which triggered the overflow because of the rounding and is fixed by this patch. Looks like,
>>> after this patch, writing "U32_MAX + 1" will still trigger the overflow.
>>>
>>> The overflow appears to result from some inconsistent type use and not using
>>> appropriate parsing API that is able to detect overflow.
>>>
>>> How about something like below:
>>>
>>
>> That makes much more sense, I have not considered changing the data type
>> as I wanted to keep the changes at minimum, but your solution is even
>> better.  Should I leave the fix up to you or do you want me to send a v3?
>
>Could you please try it out to ensure it works for you and then send a v3?
>

I wanted but the diff has some weird line numbering and could not be
applied.  I'll write it manually later, test it out, and send a v3.
Thanks!

>Thank you very much.
>
>Reinette

Re: [PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Reinette Chatre 2 months, 1 week ago

Hi Martin,

On 9/17/24 8:47 AM, Martin Kletzander wrote:
> On Tue, Sep 17, 2024 at 08:08:50AM -0700, Reinette Chatre wrote:
>> On 9/17/24 1:12 AM, Martin Kletzander wrote:
>>> On Mon, Sep 16, 2024 at 09:56:01AM -0700, Reinette Chatre wrote:
>>>> On 9/16/24 6:07 AM, Martin Kletzander wrote:
>>>>> When resctrl is mounted with the "mba_MBps" option the default (maximum)
>>>>> bandwidth is the maximum unsigned value for the type.  However when
>>>>> using the same value that already exists in the schemata file it is then
>>>>> rounded up to the bandwidth granularity and overflows to a small number
>>>>> instead, making it difficult to reset memory bandwidth allocation value
>>>>> back to its default.
>>>>>
>>>>> Since the granularity and minimum bandwidth are not used when the
>>>>> software controller is used (resctrl is mounted with the "mba_MBps"),
>>>>> skip the rounding up as well and return early from bw_validate().
>>>>
>>>> Thank you very much for finding the issue and proposing a fix.
>>>>
>>>>>
>>>>> Signed-off-by: Martin Kletzander <nert.pinx@gmail.com>
>>>>> ---
>>>>> Changes in v2:
>>>>>   - actually save the value in the output parameter @data
>>>>>
>>>>>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 9 +++++++--
>>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>>>> index 50fa1fe9a073..702b1a372e9c 100644
>>>>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>>>>> @@ -48,8 +48,13 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>>>>>           return false;
>>>>>       }
>>>>>
>>>>> -    if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
>>>>> -        !is_mba_sc(r)) {
>>>>> +    /* Nothing else to do if software controller is enabled */
>>>>> +    if (is_mba_sc(r)) {
>>>>> +        *data = bw;
>>>>> +        return true;
>>>>
>>>> While this would fix the scenario tested I do not believe this fully addresses the
>>>> overflow issue. As I understand the test wrote U32_MAX to the schemata file,
>>>> which triggered the overflow because of the rounding and is fixed by this patch. Looks like,
>>>> after this patch, writing "U32_MAX + 1" will still trigger the overflow.
>>>>
>>>> The overflow appears to result from some inconsistent type use and not using
>>>> appropriate parsing API that is able to detect overflow.
>>>>
>>>> How about something like below:
>>>>
>>>
>>> That makes much more sense, I have not considered changing the data type
>>> as I wanted to keep the changes at minimum, but your solution is even
>>> better.  Should I leave the fix up to you or do you want me to send a v3?
>>
>> Could you please try it out to ensure it works for you and then send a v3?
>>
> 
> I wanted but the diff has some weird line numbering and could not be
> applied.  I'll write it manually later, test it out, and send a v3.
> Thanks!

Apologies. Please try:

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 50fa1fe9a073..53defc5a6784 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -29,10 +29,10 @@
  * hardware. The allocated bandwidth percentage is rounded to the next
  * control step available on the hardware.
  */
-static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
+static bool bw_validate(char *buf, u32 *data, struct rdt_resource *r)
 {
-	unsigned long bw;
 	int ret;
+	u32 bw;
 
 	/*
 	 * Only linear delay values is supported for current Intel SKUs.
@@ -42,14 +42,19 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
 		return false;
 	}
 
-	ret = kstrtoul(buf, 10, &bw);
+	ret = kstrtou32(buf, 10, &bw);
 	if (ret) {
-		rdt_last_cmd_printf("Non-decimal digit in MB value %s\n", buf);
+		rdt_last_cmd_printf("Invalid MB value %s\n", buf);
 		return false;
 	}
 
-	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
-	    !is_mba_sc(r)) {
+	/* Nothing else to do if software controller is enabled. */
+	if (is_mba_sc(r)) {
+		*data = bw;
+		return true;
+	}
+
+	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
 		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
 				    r->membw.min_bw, r->default_ctrl);
 		return false;
@@ -65,7 +70,7 @@ int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
 	struct resctrl_staged_config *cfg;
 	u32 closid = data->rdtgrp->closid;
 	struct rdt_resource *r = s->res;
-	unsigned long bw_val;
+	u32 bw_val;
 
 	cfg = &d->staged_config[s->conf_type];
 	if (cfg->have_new_ctrl) {

Re: [PATCH v2] x86/resctrl: Avoid overflow in MB settings in bw_validate()

Posted by Reinette Chatre 2 months ago

Hi Martin,

On 9/17/24 10:19 AM, Reinette Chatre wrote:
> On 9/17/24 8:47 AM, Martin Kletzander wrote:
>> I wanted but the diff has some weird line numbering and could not be
>> applied.  I'll write it manually later, test it out, and send a v3.
>> Thanks!
> 
> Apologies. Please try:
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 50fa1fe9a073..53defc5a6784 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -29,10 +29,10 @@
>   * hardware. The allocated bandwidth percentage is rounded to the next
>   * control step available on the hardware.
>   */
> -static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
> +static bool bw_validate(char *buf, u32 *data, struct rdt_resource *r)
>  {
> -	unsigned long bw;
>  	int ret;
> +	u32 bw;
>  
>  	/*
>  	 * Only linear delay values is supported for current Intel SKUs.
> @@ -42,14 +42,19 @@ static bool bw_validate(char *buf, unsigned long *data, struct rdt_resource *r)
>  		return false;
>  	}
>  
> -	ret = kstrtoul(buf, 10, &bw);
> +	ret = kstrtou32(buf, 10, &bw);
>  	if (ret) {
> -		rdt_last_cmd_printf("Non-decimal digit in MB value %s\n", buf);
> +		rdt_last_cmd_printf("Invalid MB value %s\n", buf);
>  		return false;
>  	}
>  
> -	if ((bw < r->membw.min_bw || bw > r->default_ctrl) &&
> -	    !is_mba_sc(r)) {
> +	/* Nothing else to do if software controller is enabled. */
> +	if (is_mba_sc(r)) {
> +		*data = bw;
> +		return true;
> +	}
> +
> +	if (bw < r->membw.min_bw || bw > r->default_ctrl) {
>  		rdt_last_cmd_printf("MB value %ld out of range [%d,%d]\n", bw,
>  				    r->membw.min_bw, r->default_ctrl);
>  		return false;
> @@ -65,7 +70,7 @@ int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
>  	struct resctrl_staged_config *cfg;
>  	u32 closid = data->rdtgrp->closid;
>  	struct rdt_resource *r = s->res;
> -	unsigned long bw_val;
> +	u32 bw_val;
>  
>  	cfg = &d->staged_config[s->conf_type];
>  	if (cfg->have_new_ctrl) {
> 

If you do find this appropriate you are welcome to add:

Co-developed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette