Re: [PATCH v2 1/4] x86: provide an inverted Kconfig control for shim-exclusive mode

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 17 Jan 2025, Jan Beulich wrote:
> On 17.01.2025 13:24, Alejandro Vallejo wrote:
> > On Fri Jan 17, 2025 at 10:31 AM GMT, Roger Pau Monné wrote:
> >> On Thu, Jan 16, 2025 at 04:31:46PM -0800, Stefano Stabellini wrote:
> >>> On Wed, 1 Mar 2023, Jan Beulich wrote:
> >>>> While we want certain things turned off in shim-exclusive mode, doing
> >>>> so via "depends on !PV_SHIM_EXCLUSIVE" badly affects allyesconfig: Since
> >>>> that will turn on PV_SHIM_EXCLUSIVE, other options will be turned off as
> >>>> a result. Yet allyesconfig wants to enable as much of the functionality
> >>>> as possible.
> >>>>
> >>>> Retain PV_SHIM_EXCLUSIVE as a prompt-less option such that first of all
> >>>> C code using it can remain as is. This isn't just for less code churn,
> >>>> but also because I think that symbol is more logical to use in many
> >>>> (all?) places.
> >>>>
> >>>> Requested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>>>
> >>>> ---
> >>>> The new Kconfig control's name is up for improvement suggestions, but I
> >>>> think it's already better than the originally thought of
> >>>> FULL_HYPERVISOR.
> >>>
> >>> I think the approach in general is OK, maybe we can improve the naming
> >>> further. What about one of the following?
> >>>
> >>> NO_PV_SHIM_EXCLUSIVE
> >>> PV_SHIM_NOT_EXCLUSIVE
> >>
> >> IMO negated options are confusing, and if possible I think we should
> >> avoid using them unless strictly necessary.
> > 
> > The problem is that the option is negative in nature. It's asking for
> > hypervisor without _something_. I do agree with Stefano that shim would be
> > better in the name. Otherwise readers are forced to play divination tricks.
> > 
> > How about something like:
> > 
> >     SHIMLESS_HYPERVISOR
> > 
> > That's arguably not negated, preserves "shim" in the name and has the 
> > correct
> > polarity for allyesconfig to yield the right thing.
> 
> Except that a hypervisor with this option enabled isn't shim-less, but permits
> working in shim as well as in non-shim mode.

First, let's recognize that we have two opposing requirements. One
requirement is to make it as easy as possible to configure for the user.
Ideally without using negative CONFIG options, such as
NO_PV_SHIM_EXCLUSIVE. From the user point of view, honestly,
PV_SHIM_EXCLUSIVE is a pretty good name. Better than all of the others,
I think.

On the other hand, we have the requirement that we don't want
allyesconfig to end up disabling a bunch of CONFIG options. Now this
requirement can be satisfied easily by adding something like
NO_PV_SHIM_EXCLUSIVE. However, it would go somewhat against the previous
requirement.

So we need a compromise, something that doesn't end up disabling other
CONFIG options, to make allyesconfig happy, but also not too confusing
for the user (which is a matter of personal opinion).

In short, expect that people will have different opinions on this and
will find different compromises better or worse. For one, I prefer to
compromise on "no negative CONFIG options" and use
PV_SHIM_NOT_EXCLUSIVE. Because it serves the allyesconfig goal, and
while it is not as clear as PV_SHIM_EXCLUSIVE, is still better than a
completely generic FULL_HYPERVISOR option, which means nothing to me.

I cannot see a way to have a good and clear non-negated CONFIG option,
and also satisfy the allyesconfig requirement. So I prefer to compromise
on the "non-negated" part.