Re: [RFC PATCH] xen: EXPERT clean-up

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 3 Nov 2020, Jan Beulich wrote:
> On 02.11.2020 22:41, Stefano Stabellini wrote:
> > On Mon, 2 Nov 2020, Jan Beulich wrote:
> >> On 31.10.2020 01:24, Stefano Stabellini wrote:
> >>> @@ -79,8 +79,8 @@ config SBSA_VUART_CONSOLE
> >>>     SBSA Generic UART implements a subset of ARM PL011 UART.
> >>>  
> >>>  config ARM_SSBD
> >>> - bool "Speculative Store Bypass Disable" if EXPERT
> >>> - depends on HAS_ALTERNATIVE
> >>> + bool "Speculative Store Bypass Disable"
> >>> + depends on HAS_ALTERNATIVE && EXPERT
> >>>   default y
> >>
> >> At the example of this, I'm afraid when the default isn't "n"
> >> (or there's no default directive at all, as ought to be
> >> equivalent to and preferred over "default n"), such a
> >> transformation is not functionally identical: Before your
> >> change, with !EXPERT this option defaults to y. After your
> >> change this option is unavailable (which resolves to it being
> >> off for all consuming purposes).
> >>
> >> IOW there are reasons to have "if ..." attached to the prompts
> >> (for this construct indeed only making the prompt conditional,
> >> not the entire option), but there are also cases where the
> >> cleanup you do is indeed desirable / helpful.
> > 
> > Yeah, thanks for catching it, it is obviously a problem.
> > 
> > My intention was just to "tag" somehow the options to EXPERT so that it
> > would show on the menu. Maybe a better, simpler, way to do it is
> > to add the word "EXPERT" to the one line prompt:
> > 
> >  config ARM_SSBD
> > -   bool "Speculative Store Bypass Disable" if EXPERT
> > +   bool "Speculative Store Bypass Disable (EXPERT)" if EXPERT
> >     depends on HAS_ALTERNATIVE
> >     default y
> >     help
> > 
> > 
> > What do you think?
> 
> While on the surface this may look like an improvement, I don't
> see how it would actually help: If you read the Kconfig file
> itself, the dependency is seen anyway. And on the menu I don't
> see the point of telling someone who has enabled EXPERT that a
> certain option is (or is not) an expert one. If they think
> they're experts, so should they be treated. (It was, after all,
> a deliberate decision to make enabling expert mode easier, and
> hence easier to use for what one might consider not-really-
> experts. I realize saying so may be considered tendentious; I
> mean it in a purely technical sense, and I'd like to apologize
> in advance to anyone not sharing this as a possible perspective
> to take.)
> 
> Plus, of course, the addition of such (EXPERT) markers to
> future options' prompts is liable to get forgotten now and then,
> so sooner or later we'd likely end up with an inconsistent
> mixture anyway.

I tend to agree with you on everything you wrote. The fundamental issue
is that we are (mis)using EXPERT to tag features that are experimental,
as defined by SUPPORT.md.

It is important to be able to distinguish clearly at the kconfig level
options that are (security) supported from options that are
unsupported/experimental. Today the only way to do it is with EXPERT
which is not great because:

- it doesn't convey the idea that it is for unsupported/experimental
  features
- if you want to enable one unsupported feature, it is not clear what
  you have to do

So maybe we should replace EXPERT with UNSUPPORTED (or EXPERIMENTAL) in
the Kconfig menu?

It would make it clearer that by enabling UNSUPPORTED you are going to
get a configuration that is not security supported. And ideally we would
also tag features like ACPI as UNSUPPORTED as I suggested above.