Re: [PATCH 1/3] x86/pv: Options to disable and/or compile out 32bit PV support

[Jürgen Groß <jgross@xxxxxxxx>]

On 23.04.20 19:35, Andrew Cooper wrote:
> On 21/04/2020 07:02, Jan Beulich wrote:
> > On 20.04.2020 20:05, Andrew Cooper wrote:
> > > On 20/04/2020 15:05, Jan Beulich wrote:
> > > > I'm in particular
> > > > concerned that we may gain a large number of such printk()s over
> > > > time, if we added them in such cases.
> > > The printk() was a bit of an afterthought, but deliberately avoiding the
> > > -EINVAL path was specifically not.
> > >
> > > In the case that the user tries to use `pv=no-32` without CONFIG_PV32,
> > > they should see something other than
> > >
> > > (XEN) parameter "pv=no-32" unknown!
> > Why - to this specific build of Xen the parameter is unknown.
> Because it is unnecessarily problematic and borderline obnoxious to
> users, as well as occasional Xen developers.
>
> "you've not got the correct CONFIG_$X for that to be meaningful" is
> specifically useful to separate from "I've got no idea".
>
> > > I don't think overloading the return value is a clever move, because
> > > then every parse function has to take care of ensuring that -EOPNOTSUPP
> > > (or ENODEV?) never clobbers -EINVAL.
> > I didn't suggest overloading the return value. Instead I
> > specifically want this to go the -EINVAL path.
> >
> > > We could have a generic helper which looks like:
> > >
> > > static inline void ignored_param(const char *cfg, const char *name,
> > > const char *s, const char *ss)
> > > {
> > >      printk(XENLOG_INFO "%s disabled - ignoring '%s=%*.s' setting\n",
> > > cfg, name, s, (int)(ss - s));
> > > }
> > >
> > > which at least would keep all the users consistent.
> > Further bloating the binary with (almost) useless string literals.
> > I'd specifically like to avoid this.
> I don't accept that as a valid argument.
>
> We're talking about literally tens of bytes (which will merge anyway, so
> 0 in practice), and a resulting UI which helps people get out of
> problems rather than penalises them for having gotten into a problem to
> begin with.
>
> I will absolutely prioritise a more helpful UI over a handful of bytes.
What about a kconfig option (defaulting to "yes") enabling this feature?

That way an embedded variant can be made smaller while a server one is
more user friendly.


Juergen