Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly

[Julien Grall <julien@xxxxxxx>]

Hi,

On 11/05/2020 15:07, Jan Beulich wrote:
> On 11.05.2020 15:57, Julien Grall wrote:
> > Hi,
> >
> > On 11/05/2020 14:40, Jan Beulich wrote:
> > > On 11.05.2020 15:30, Julien Grall wrote:
> > > > Hi Ian,
> > > >
> > > > Thank you for the clarification.
> > > >
> > > > On 07/05/2020 18:01, Ian Jackson wrote:
> > > > > Julien Grall writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected 
> > > > > from the menuconfig directly"):
> > > > > > On 04/05/2020 13:34, Ian Jackson wrote:
> > > > > > > George Dunlap writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected 
> > > > > > > from the menuconfig directly"):
> > > > > > > > On Apr 30, 2020, at 3:50 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> > > > > > > > > Well, if I'm not mis-remembering it was on purpose to make it more
> > > > > > > > > difficult for people to declare themselves "experts". FAOD I'm not
> > > > > > > > > meaning to imply I don't see and accept the frustration aspect you
> > > > > > > > > mention further up. The two need to be carefully weighed against
> > > > > > > > > one another.
> > > > > > > Yes, it was on purpose.  However, I had my doubts at the time and
> > > > > > > I think experience has shown that this was a mistake.
> > > > > > >
> > > > > > > > I don’t think we need to make it difficult for people to declare
> > > > > > > > themselves experts, particularly as “all” it means at the moment is,
> > > > > > > > “Can build something which is not security supported”.  People who
> > > > > > > > are building their own hypervisors are already pretty well advanced;
> > > > > > > > I think we can let them shoot themselves in the foot if they want
> > > > > > > > to.
> > > > > > > Precisely.
> > > > > > Can I consider this as an Acked-by? :)
> > > > > I am happy with the principle of the change.  I haven't reviewed the
> > > > > details of the commit message etc.
> > > > >
> > > > > I reviewed the thread and there were two concernes raised:
> > > > >
> > > > >     * The question of principle.  I disagree with this concern
> > > > >       because I approve of principle of the patch.
> > > > >
> > > > >     * Some detail about the precise justificaton as written in
> > > > >       the commit message, regarding `clean' targets.  Apparently the
> > > > >       assertion may not be completely true.  I haven't seen a proposed
> > > > >       alternative wording.
> > > > I have checked the latest staging, the `clean` target doesn't trash
> > > > .config anymore.
> > > >
> > > > > I don't feel I should ack a controversial patch with an unresolved
> > > > > wording issue.  Can you tell me what your proposed wording is ?
> > > > > To avoid blocking this change I would be happy to review your wording
> > > > > and see if it meets my reading of the stated objection.
> > > > Here a suggested rewording:
> > > >
> > > > "EXPERT mode is currently used to gate any options that are in technical
> > > > preview or not security supported At the moment, the only way to select
> > > > it is to use XEN_CONFIG_EXPERT=y on the make command line.
> > > >
> > > > However, if the user forget to add the option when (re)building or when
> > > > using menuconfig, then .config will get rewritten. This may lead to a
> > > > rather frustrating experience as it is difficult to diagnostic the
> > > > issue.
> > > To me this looks very similar to e.g. not suitably overriding the
> > > default toolchain binaries, if one has a need to build with newer
> > > ones than what a distro provides. According to some of my routinely
> > > built configs both can be done by putting suitable entries into
> > > ./.config (not xen/.config), removing the need to remember adding
> > > either to the make command line.
> > I have never heard of ./.config before. So what are you referring to?
> I'm referring to this line in ./Config.mk:
>
> -include $(XEN_ROOT)/.config
Great another undocumented way to do things...

> > But yes, there are ways to make it permanent. But that involves hacking
> > Xen source.
> Why would there be any need for a source modification? Just like
> xen/.config, ./.config is not considered part of the source.
>
> > This is not a very great approach because if you need to
> > bisect, then you have to remember to apply the change everytime. It also
> > doesn't work if you have to build for multiple different target from the
> > same source.
> Why wouldn't it? I'm doing exactly this, far beyond just x86 and
> Arm builds, and it all works fine. (It would work even better
> with out-of-tree builds, but it looks like Anthony is getting us
> there.)
... let me ask it differently. Are you concerned of my wording or by the 
fact we make easier to a user to EXPERT mode?
Cheers,

--
Julien Grall