On 17/01/2025 10:43 pm, Stefano Stabellini wrote: > 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.coop...@citrix.com> >>>>>> Signed-off-by: Jan Beulich <jbeul...@suse.com> >>>>>> >>>>>> --- >>>>>> 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.
Debating the naming is missing the point. The problem here is the wish to have PV_SHIM_EXCLUSIVE behave in a way that Kconfig is not capable of expressing. Specifically, what is broken is having "lower level" options inhibit unrelated "higher level" options when the graph gets rescanned[1]. That's why we're in the laughable position of `make allyesconfig` turning off CONFIG_HVM. Jan, you want "echo PV_SHIM_EXCLUSIVE=y >> .config && make" to mean "reset me back to a PV Shim". Kconfig spells this "make $foo_defconfig" for an appropriately given foo. There should be: 1) an option called PV_SHIM_EXCLUSIVE which does *nothing* other than making the boolean be a compile time constant. 2) a pvshim_defconfig target which expresses what a pvshim ought to look like. Trying to fight against the behaviour of Kconfig is not a good use of anyone's time. ~Andrew [1] default to unrelated symbols is also broken for a related reason. The result you get is sensitive to the order of processing of symbols.
