Re: [PATCH 2/3] x86/pv-shim: don't even allow enabling GRANT_TABLE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.12.2022 08:21, Jan Beulich wrote:
> On 06.12.2022 21:26, Andrew Cooper wrote:
>> On 06/12/2022 14:30, Jan Beulich wrote:
>>> Grant table code is unused in shim mode, so there's no point in
>>> building it in the first place for shim-exclusive mode.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> nack.
>>
>> This is bogus, as is every other "depends on !PV_SHIM_EXCLUSIVE".
> 
> But why? Doing things like this in Kconfig is exactly ...
> 
>> The only reason I haven't reverting the others so `make allyesconfig`
>> doesn't disable CONFIG_HVM, is because I haven't had time.  This change
>> further breaks allyesconfig by disabling GRANT_TABLE too.
>>
>> PV_SHIM_EXCLUSIVE is a simple option for a bit of dead code
>> elimination.  It is not valid to be used like this.
> 
> ... for the purpose of dead code elimination. By nack-ing a change like
> this (and by having voiced opposition to earlier ones) you simply call
> for yet more entirely unhelpful #ifdef-ary. See the last paragraph of
> the description of patch 1, half of which this change rectifies. The
> solution on the evtchn side, unfortunately, looks to be #ifdef-ary,
> short of there being a suitable Kconfig option.
> 
> Furthermore Kconfig, in my view, is specifically intended to allow to
> prevent the user from selecting entirely bogus option combinations
> (and even more so suggest entirely bogus configurations by bogus
> default settings). If you disagree, then I'm afraid we have a 2nd
> Kconfig usage topic which we need to settle on in a project-wide
> manner. If only we ever made any progress on such ...
> 
> As to allyesconfig - I can see your point there, but then arrangements
> need to be invented to avoid this kind of unhelpful behavior.

Thinking more about it, if allyesconfig is meant to be useful, then
any "depends on !..." (other than for base architecture identifiers)
would be wrong (see e.g. COVERAGE depending on !LIVEPATCH or
ARM_SMMU_V3 depending on !ACPI). And this would extend to Linux as
well - how do they deal with that?

Jan