[PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[Andrew Cooper]

This makes it behave slightly more like a regular boolean option.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

Slightly RFC, because there is no easy way of making the opposite "normal
boolean" case work for no-vpmu.
---
 xen/arch/x86/cpu/vpmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 4fedc7c57012..501d4f0af94e 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -66,7 +66,9 @@ static int __init cf_check parse_vpmu_params(const char *s)
         if ( !ss )
             ss = strchr(s, '\0');
 
-        if ( (val = parse_bool(s, ss)) >= 0 )
+        if ( s == ss )
+            opt_vpmu_enabled = true;
+        else if ( (val = parse_bool(s, ss)) >= 0 )
         {
             opt_vpmu_enabled = val;
             if ( !val )
-- 
2.11.0

Re: [PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[Jan Beulich]

On 02.03.2022 23:11, Andrew Cooper wrote:
> This makes it behave slightly more like a regular boolean option.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> Slightly RFC, because there is no easy way of making the opposite "normal
> boolean" case work for no-vpmu.

There's nothing to do to make this work afaict: Generic command line
handling converts "no-<option>" to "<option>=no" for custom params.

Jan

Re: [PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[Andrew Cooper]

On 03/03/2022 07:44, Jan Beulich wrote:
> On 02.03.2022 23:11, Andrew Cooper wrote:
>> This makes it behave slightly more like a regular boolean option.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
>> Slightly RFC, because there is no easy way of making the opposite "normal
>> boolean" case work for no-vpmu.
> There's nothing to do to make this work afaict: Generic command line
> handling converts "no-<option>" to "<option>=no" for custom params.

Oh - I'd forgotten that, in which case this patch actually wants to be
simply:

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index adff2d2c77f3..2cea1da781ac 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -162,6 +162,11 @@ static int parse_params(const char *cmdline, const
struct kernel_param *start,
                     safe_strcpy(opt, "no");
                     optval = opt;
                 }
+                else if ( !*optval )
+                {
+                    safe_strcpy(opt, "1");
+                    optval = opt;
+                }
                 rctmp = param->par.func(optval);
                 break;
             case OPT_IGNORE:

to turn "option\0" into "option=1", no?

~Andrew

Re: [PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[Jan Beulich]

On 03.03.2022 11:48, Andrew Cooper wrote:
> On 03/03/2022 07:44, Jan Beulich wrote:
>> On 02.03.2022 23:11, Andrew Cooper wrote:
>>> This makes it behave slightly more like a regular boolean option.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>>> Slightly RFC, because there is no easy way of making the opposite "normal
>>> boolean" case work for no-vpmu.
>> There's nothing to do to make this work afaict: Generic command line
>> handling converts "no-<option>" to "<option>=no" for custom params.
> 
> Oh - I'd forgotten that, in which case this patch actually wants to be
> simply:
> 
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index adff2d2c77f3..2cea1da781ac 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -162,6 +162,11 @@ static int parse_params(const char *cmdline, const
> struct kernel_param *start,
>                      safe_strcpy(opt, "no");
>                      optval = opt;
>                  }
> +                else if ( !*optval )
> +                {
> +                    safe_strcpy(opt, "1");
> +                    optval = opt;
> +                }
>                  rctmp = param->par.func(optval);
>                  break;
>              case OPT_IGNORE:
> 
> to turn "option\0" into "option=1", no?

Iirc extending this to the positive case was deliberately not done, for
the risk of breaking custom handlers not expecting the standard boolean
forms. We could likely go this route, but only after auditing all custom
handlers, I'm afraid.

Jan

Re: [PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[Andrew Cooper]

On 03/03/2022 11:04, Jan Beulich wrote:
> On 03.03.2022 11:48, Andrew Cooper wrote:
>> On 03/03/2022 07:44, Jan Beulich wrote:
>>> On 02.03.2022 23:11, Andrew Cooper wrote:
>>>> This makes it behave slightly more like a regular boolean option.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>>
>>>> Slightly RFC, because there is no easy way of making the opposite "normal
>>>> boolean" case work for no-vpmu.
>>> There's nothing to do to make this work afaict: Generic command line
>>> handling converts "no-<option>" to "<option>=no" for custom params.
>> Oh - I'd forgotten that, in which case this patch actually wants to be
>> simply:
>>
>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
>> index adff2d2c77f3..2cea1da781ac 100644
>> --- a/xen/common/kernel.c
>> +++ b/xen/common/kernel.c
>> @@ -162,6 +162,11 @@ static int parse_params(const char *cmdline, const
>> struct kernel_param *start,
>>                      safe_strcpy(opt, "no");
>>                      optval = opt;
>>                  }
>> +                else if ( !*optval )
>> +                {
>> +                    safe_strcpy(opt, "1");
>> +                    optval = opt;
>> +                }
>>                  rctmp = param->par.func(optval);
>>                  break;
>>              case OPT_IGNORE:
>>
>> to turn "option\0" into "option=1", no?
> Iirc extending this to the positive case was deliberately not done, for
> the risk of breaking custom handlers not expecting the standard boolean
> forms. We could likely go this route, but only after auditing all custom
> handlers, I'm afraid.

Well - I've already audited them all once recently.  What's once more...

I'll have a go in due course; I'd definitely prefer to avoid special
casing the positive boolean form in individual handlers.

~Andrew

Re: [PATCH] x86/cmdline: Interpret 'vpmu' as a positive boolean

[Jan Beulich]

On 03.03.2022 12:15, Andrew Cooper wrote:
> On 03/03/2022 11:04, Jan Beulich wrote:
>> On 03.03.2022 11:48, Andrew Cooper wrote:
>>> On 03/03/2022 07:44, Jan Beulich wrote:
>>>> On 02.03.2022 23:11, Andrew Cooper wrote:
>>>>> This makes it behave slightly more like a regular boolean option.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>>> Slightly RFC, because there is no easy way of making the opposite "normal
>>>>> boolean" case work for no-vpmu.
>>>> There's nothing to do to make this work afaict: Generic command line
>>>> handling converts "no-<option>" to "<option>=no" for custom params.
>>> Oh - I'd forgotten that, in which case this patch actually wants to be
>>> simply:
>>>
>>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
>>> index adff2d2c77f3..2cea1da781ac 100644
>>> --- a/xen/common/kernel.c
>>> +++ b/xen/common/kernel.c
>>> @@ -162,6 +162,11 @@ static int parse_params(const char *cmdline, const
>>> struct kernel_param *start,
>>>                      safe_strcpy(opt, "no");
>>>                      optval = opt;
>>>                  }
>>> +                else if ( !*optval )
>>> +                {
>>> +                    safe_strcpy(opt, "1");
>>> +                    optval = opt;
>>> +                }
>>>                  rctmp = param->par.func(optval);
>>>                  break;
>>>              case OPT_IGNORE:
>>>
>>> to turn "option\0" into "option=1", no?
>> Iirc extending this to the positive case was deliberately not done, for
>> the risk of breaking custom handlers not expecting the standard boolean
>> forms. We could likely go this route, but only after auditing all custom
>> handlers, I'm afraid.
> 
> Well - I've already audited them all once recently.  What's once more...

Of course if you did an audit (for this particular property) recently,
that's definitely enough. Feel free to apply my earlier provided R-b
also to this alternative change then.

Jan

> I'll have a go in due course; I'd definitely prefer to avoid special
> casing the positive boolean form in individual handlers.
> 
> ~Andrew