On 29.04.2022 10:10, Roger Pau Monné wrote:
> On Thu, Apr 28, 2022 at 12:13:31PM +0200, Jan Beulich wrote:
>> On 28.04.2022 10:52, Andrew Cooper wrote:
>>> @@ -283,6 +283,8 @@ CET is incompatible with 32bit PV guests. If any CET
>>> sub-options are active,
>>> they will override the `pv=32` boolean to `false`. Backwards compatibility
>>> can be maintained with the pv-shim mechanism.
>>>
>>> +* An unqualified boolean is shorthand for setting all suboptions at once.
>>
>> You're the native speaker, but I wonder whether there an "a" missing
>> before "shorthand".
>>
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -117,7 +117,20 @@ static int __init cf_check parse_cet(const char *s)
>>> if ( !ss )
>>> ss = strchr(s, '\0');
>>>
>>> - if ( (val = parse_boolean("shstk", s, ss)) >= 0 )
>>> + if ( (val = parse_bool(s, ss)) >= 0 )
>>> + {
>>> +#ifdef CONFIG_XEN_SHSTK
>>> + opt_xen_shstk = val;
>>> +#else
>>> + no_config_param("XEN_SHSTK", "cet", s, ss);
>>> +#endif
>>> +#ifdef CONFIG_XEN_IBT
>>> + opt_xen_ibt = val;
>>> +#else
>>> + no_config_param("XEN_IBT", "cet", s, ss);
>>> +#endif
>>
>> There shouldn't be two invocations of no_config_param() here; imo if
>> either CONFIG_* is defined, use of the option shouldn't produce any
>> warning at all.
>
> Hm, I think we would want to warn if someone sets cet=1 but some of
> the options have not been built in? Or else a not very conscious
> administrator might believe that all CET options are enabled when some
> might not be present in the build. This would also assume that all
> options are positive.
But the positive options aren't really the interesting ones here, as
things are enabled by default anyway. I would expect "cet=0" to be
silent unless neither CONFIG_* is enabled in the build - it simply
means "disable whatever CET support there is".
I can agree that "cet=1" may indeed be useful to warn, though, but I
wonder whether the logic here then wouldn't become unduly complicated.
> IMO the current approach doesn't seem bad to me, I think it's always
> better to error on the side of printing too verbose information rather
> than omitting it, specially when it's related to user input on the
> command line and security sensitive options.
While fundamentally I agree, too verbose output can also raise
unnecessary questions or induce unnecessary investigations.
Jan
