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

[Juergen Gross <jgross@xxxxxxxx>]

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.

Otherwise options like CONFIG_X86_32 (which depends on !X86_64) would make
no sense either.

So the way to go seems to have some default minimal configs (in our case
e.g. shim and no-shim), which can then be expanded via allyesconfig or
allnoconfig.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature