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

[Juergen Gross <jgross@xxxxxxxx>]

On 07.12.22 10:35, Jan Beulich wrote:
> On 07.12.2022 10:11, Juergen Gross wrote:
> > On 07.12.22 09:55, Jan Beulich wrote:
> > > 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?
> > Isn't allyesconfig for new options only? At least in Linux it is
> > documented this way.
> Is it? I only find
>
>       "make allyesconfig"
>                          Create a ./.config file by setting symbol
>                          values to 'y' as much as possible.
Hmm, strange. I seem to have either looked at something else, or I haven't
read it correctly.

Sorry for the noise.

And probably time for more coffee.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature