[RESEND,2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly
diff mbox series

Message ID 20200430142548.23751-3-julien@xen.org
State New
Headers show
Series
  • xen: Allow EXPERT mode to be selected from the menuconfig directly
Related show

Commit Message

Julien Grall April 30, 2020, 2:25 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

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 of one of the make
command (even a clean), then .config will get rewritten. This may lead
to a rather frustrating experience as it is difficult to diagnostic the
issue.

A lot of the options behind EXPERT would benefit to get more tested in
order to be mark as fully supported in the future.

In order to make easier to experiment, the option EXPERT can now be
selected from the menuconfig rather than make command line. This does
not change the fact a kernel with EXPERT mode selected will not be
security supported.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---

This may require some changes in OSSTest as we select the EXPERT mode
when building (This is necessary for booting Xen on Thunder-X box).
---
 xen/Kconfig  | 10 +++++++++-
 xen/Makefile |  1 -
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Jan Beulich April 30, 2020, 2:50 p.m. UTC | #1
On 30.04.2020 16:25, Julien Grall wrote:
> 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 of one of the make
> command (even a clean), then .config will get rewritten. This may lead
> to a rather frustrating experience as it is difficult to diagnostic the
> issue.

Is / will this still be true after Anthony's rework of the build
system? Right now we already have

clean-targets := %clean
no-dot-config-targets := $(clean-targets) \
                         ...

> A lot of the options behind EXPERT would benefit to get more tested in
> order to be mark as fully supported in the future.

Anyone intending to get an EXPERT-only option fully supported will
need to do focused testing; I don't think we can expect to move
items out of this category just because more people happen to test
something every now and then.

> In order to make easier to experiment, the option EXPERT can now be
> selected from the menuconfig rather than make command line. This does
> not change the fact a kernel with EXPERT mode selected will not be
> security supported.

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.

> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -35,7 +35,15 @@ config DEFCONFIG_LIST
>  	default ARCH_DEFCONFIG
>  
>  config EXPERT
> -	def_bool y if "$(XEN_CONFIG_EXPERT)" = "y"
> +	bool "Configure standard Xen features (expert users)"
> +	help
> +	  This option allows certain base Xen options and settings
> +	  to be disabled or tweaked. This is for specialized environments
> +	  which can tolerate a "non-standard" Xen.
> +	  Only use this if you really know what you are doing.
> +	  Xen binaries built with this option enabled are not security
> +	  supported.
> +	default n

I don't think the last line is needed.

Jan
Julien Grall April 30, 2020, 3:35 p.m. UTC | #2
Hi Jan,

On 30/04/2020 15:50, Jan Beulich wrote:
> On 30.04.2020 16:25, Julien Grall wrote:
>> 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 of one of the make
>> command (even a clean), then .config will get rewritten. This may lead
>> to a rather frustrating experience as it is difficult to diagnostic the
>> issue.
> 
> Is / will this still be true after Anthony's rework of the build
> system? Right now we already have
> 
> clean-targets := %clean
> no-dot-config-targets := $(clean-targets) \
>                           ...

I haven't tried Anthony's rework yet. But I guess the problem would be 
the same if you forget to add XEN_CONFIG_EXPERT=y on make.

> 
>> A lot of the options behind EXPERT would benefit to get more tested in
>> order to be mark as fully supported in the future.
> 
> Anyone intending to get an EXPERT-only option fully supported will
> need to do focused testing; I don't think we can expect to move
> items out of this category just because more people happen to test
> something every now and then.

I didn't imply this was the only condition to get a feature security 
suported. I merely pointed out that more testing would help to harden 
the code. If you make difficult to access an option then it will be less 
tested and take longer to get it security supported.

> 
>> In order to make easier to experiment, the option EXPERT can now be
>> selected from the menuconfig rather than make command line. This does
>> not change the fact a kernel with EXPERT mode selected will not be
>> security supported.
> 
> 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.

Some of the options behind EXPERT mode don't make sense. For instance, 
how adding a built-in command line requires to be expert? I understand 
we don't want to support it, but I don't see any reason to make the 
user's life more difficult here.

Cheers,
Jan Beulich May 4, 2020, 9:18 a.m. UTC | #3
On 30.04.2020 17:35, Julien Grall wrote:
> On 30/04/2020 15:50, Jan Beulich wrote:
>> On 30.04.2020 16:25, Julien Grall wrote:
>>> 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 of one of the make
>>> command (even a clean), then .config will get rewritten. This may lead
>>> to a rather frustrating experience as it is difficult to diagnostic the
>>> issue.
>>
>> Is / will this still be true after Anthony's rework of the build
>> system? Right now we already have
>>
>> clean-targets := %clean
>> no-dot-config-targets := $(clean-targets) \
>>                           ...
> 
> I haven't tried Anthony's rework yet. But I guess the problem would
> be the same if you forget to add XEN_CONFIG_EXPERT=y on make.

Why? xen/.config would get re-written only if kconfig got run in
the first place. It is my understanding that no-dot-config-targets
exist to avoid including .config, and as a result make won't find
a need anymore to cause it to re-made if out of date.

Jan
Julien Grall May 4, 2020, 9:30 a.m. UTC | #4
Hi Jan,

On 04/05/2020 10:18, Jan Beulich wrote:
> On 30.04.2020 17:35, Julien Grall wrote:
>> On 30/04/2020 15:50, Jan Beulich wrote:
>>> On 30.04.2020 16:25, Julien Grall wrote:
>>>> 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 of one of the make
>>>> command (even a clean), then .config will get rewritten. This may lead
>>>> to a rather frustrating experience as it is difficult to diagnostic the
>>>> issue.
>>>
>>> Is / will this still be true after Anthony's rework of the build
>>> system? Right now we already have
>>>
>>> clean-targets := %clean
>>> no-dot-config-targets := $(clean-targets) \
>>>                            ...
>>
>> I haven't tried Anthony's rework yet. But I guess the problem would
>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make.
> 
> Why? xen/.config would get re-written only if kconfig got run in
> the first place. It is my understanding that no-dot-config-targets
> exist to avoid including .config, and as a result make won't find
> a need anymore to cause it to re-made if out of date.

kconfig may be executed because you change one of the */Kconfig file. So 
if you happen to forget XEN_CONFIG_EXPERT=y on your build command line, 
then you will have your .config rewritten without expert options.

Cheers,
Jan Beulich May 4, 2020, 9:37 a.m. UTC | #5
On 04.05.2020 11:30, Julien Grall wrote:
> Hi Jan,
> 
> On 04/05/2020 10:18, Jan Beulich wrote:
>> On 30.04.2020 17:35, Julien Grall wrote:
>>> On 30/04/2020 15:50, Jan Beulich wrote:
>>>> On 30.04.2020 16:25, Julien Grall wrote:
>>>>> 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 of one of the make
>>>>> command (even a clean), then .config will get rewritten. This may lead
>>>>> to a rather frustrating experience as it is difficult to diagnostic the
>>>>> issue.
>>>>
>>>> Is / will this still be true after Anthony's rework of the build
>>>> system? Right now we already have
>>>>
>>>> clean-targets := %clean
>>>> no-dot-config-targets := $(clean-targets) \
>>>>                            ...
>>>
>>> I haven't tried Anthony's rework yet. But I guess the problem would
>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make.
>>
>> Why? xen/.config would get re-written only if kconfig got run in
>> the first place. It is my understanding that no-dot-config-targets
>> exist to avoid including .config, and as a result make won't find
>> a need anymore to cause it to re-made if out of date.
> 
> kconfig may be executed because you change one of the */Kconfig file.
> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command
> line, then you will have your .config rewritten without expert options.

That's still a build system issue then (if this is really what happens):
Dependencies of xen/.config shouldn't be evaluated as long as it doesn't
get used.

Jan
Julien Grall May 4, 2020, 9:54 a.m. UTC | #6
Hi Jan,

On 04/05/2020 10:37, Jan Beulich wrote:
> On 04.05.2020 11:30, Julien Grall wrote:
>> Hi Jan,
>>
>> On 04/05/2020 10:18, Jan Beulich wrote:
>>> On 30.04.2020 17:35, Julien Grall wrote:
>>>> On 30/04/2020 15:50, Jan Beulich wrote:
>>>>> On 30.04.2020 16:25, Julien Grall wrote:
>>>>>> 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 of one of the make
>>>>>> command (even a clean), then .config will get rewritten. This may lead
>>>>>> to a rather frustrating experience as it is difficult to diagnostic the
>>>>>> issue.
>>>>>
>>>>> Is / will this still be true after Anthony's rework of the build
>>>>> system? Right now we already have
>>>>>
>>>>> clean-targets := %clean
>>>>> no-dot-config-targets := $(clean-targets) \
>>>>>                             ...
>>>>
>>>> I haven't tried Anthony's rework yet. But I guess the problem would
>>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make.
>>>
>>> Why? xen/.config would get re-written only if kconfig got run in
>>> the first place. It is my understanding that no-dot-config-targets
>>> exist to avoid including .config, and as a result make won't find
>>> a need anymore to cause it to re-made if out of date.
>>
>> kconfig may be executed because you change one of the */Kconfig file.
>> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command
>> line, then you will have your .config rewritten without expert options.
> 
> That's still a build system issue then (if this is really what happens):
> Dependencies of xen/.config shouldn't be evaluated as long as it doesn't
> get used.

I am not sure to understand what you mean by "doesn't get used here". 
When you build Xen, xen/.config is a dependency for the auto-generated 
header. So 'make' will actually check whether there are any modification 
in */Kconfig.

A user would also expect that any modification in a */Kconfig will be 
picked by 'make' when building the hypervisor. This is how it works in 
Linux and I see no reason to diverge in Xen for this.

Cheers,
Jan Beulich May 4, 2020, 10:18 a.m. UTC | #7
On 04.05.2020 11:54, Julien Grall wrote:
> Hi Jan,
> 
> On 04/05/2020 10:37, Jan Beulich wrote:
>> On 04.05.2020 11:30, Julien Grall wrote:
>>> Hi Jan,
>>>
>>> On 04/05/2020 10:18, Jan Beulich wrote:
>>>> On 30.04.2020 17:35, Julien Grall wrote:
>>>>> On 30/04/2020 15:50, Jan Beulich wrote:
>>>>>> On 30.04.2020 16:25, Julien Grall wrote:
>>>>>>> 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 of one of the make
>>>>>>> command (even a clean), then .config will get rewritten. This may lead
>>>>>>> to a rather frustrating experience as it is difficult to diagnostic the
>>>>>>> issue.
>>>>>>
>>>>>> Is / will this still be true after Anthony's rework of the build
>>>>>> system? Right now we already have
>>>>>>
>>>>>> clean-targets := %clean
>>>>>> no-dot-config-targets := $(clean-targets) \
>>>>>>                             ...
>>>>>
>>>>> I haven't tried Anthony's rework yet. But I guess the problem would
>>>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make.
>>>>
>>>> Why? xen/.config would get re-written only if kconfig got run in
>>>> the first place. It is my understanding that no-dot-config-targets
>>>> exist to avoid including .config, and as a result make won't find
>>>> a need anymore to cause it to re-made if out of date.
>>>
>>> kconfig may be executed because you change one of the */Kconfig file.
>>> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command
>>> line, then you will have your .config rewritten without expert options.
>>
>> That's still a build system issue then (if this is really what happens):
>> Dependencies of xen/.config shouldn't be evaluated as long as it doesn't
>> get used.
> 
> I am not sure to understand what you mean by "doesn't get used here". When you build Xen, xen/.config is a dependency for the auto-generated header. So 'make' will actually check whether there are any modification in */Kconfig.

But you were talking about "make clean", weren't you?

Jan
Julien Grall May 4, 2020, 10:23 a.m. UTC | #8
On 04/05/2020 11:18, Jan Beulich wrote:
> On 04.05.2020 11:54, Julien Grall wrote:
>> Hi Jan,
>>
>> On 04/05/2020 10:37, Jan Beulich wrote:
>>> On 04.05.2020 11:30, Julien Grall wrote:
>>>> Hi Jan,
>>>>
>>>> On 04/05/2020 10:18, Jan Beulich wrote:
>>>>> On 30.04.2020 17:35, Julien Grall wrote:
>>>>>> On 30/04/2020 15:50, Jan Beulich wrote:
>>>>>>> On 30.04.2020 16:25, Julien Grall wrote:
>>>>>>>> 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 of one of the make
>>>>>>>> command (even a clean), then .config will get rewritten. This may lead
>>>>>>>> to a rather frustrating experience as it is difficult to diagnostic the
>>>>>>>> issue.
>>>>>>>
>>>>>>> Is / will this still be true after Anthony's rework of the build
>>>>>>> system? Right now we already have
>>>>>>>
>>>>>>> clean-targets := %clean
>>>>>>> no-dot-config-targets := $(clean-targets) \
>>>>>>>                              ...
>>>>>>
>>>>>> I haven't tried Anthony's rework yet. But I guess the problem would
>>>>>> be the same if you forget to add XEN_CONFIG_EXPERT=y on make.
>>>>>
>>>>> Why? xen/.config would get re-written only if kconfig got run in
>>>>> the first place. It is my understanding that no-dot-config-targets
>>>>> exist to avoid including .config, and as a result make won't find
>>>>> a need anymore to cause it to re-made if out of date.
>>>>
>>>> kconfig may be executed because you change one of the */Kconfig file.
>>>> So if you happen to forget XEN_CONFIG_EXPERT=y on your build command
>>>> line, then you will have your .config rewritten without expert options.
>>>
>>> That's still a build system issue then (if this is really what happens):
>>> Dependencies of xen/.config shouldn't be evaluated as long as it doesn't
>>> get used.
>>
>> I am not sure to understand what you mean by "doesn't get used here". When you build Xen, xen/.config is a dependency for the auto-generated header. So 'make' will actually check whether there are any modification in */Kconfig.
> 
> But you were talking about "make clean", weren't you?

In the commit message yes... You asked whether this was true and I 
answered I didn't get a chance to test Anthony's rework. However, I also
pointed out that it wouldn't solve a simple 'make' issue.

I considered that your 'why?' were related to the simple 'make'.

Cheers,
George Dunlap May 4, 2020, 11:10 a.m. UTC | #9
> On Apr 30, 2020, at 3:50 PM, Jan Beulich <JBeulich@suse.com> wrote:
> 
>> In order to make easier to experiment, the option EXPERT can now be
>> selected from the menuconfig rather than make command line. This does
>> not change the fact a kernel with EXPERT mode selected will not be
>> security supported.
> 
> 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.

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.

 -George
Ian Jackson May 4, 2020, 12:34 p.m. UTC | #10
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@suse.com> 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.

Ian.
Julien Grall May 6, 2020, 9:46 a.m. UTC | #11
Hi George,

On 04/05/2020 12:10, George Dunlap wrote:
> 
> 
>> On Apr 30, 2020, at 3:50 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>
>>> In order to make easier to experiment, the option EXPERT can now be
>>> selected from the menuconfig rather than make command line. This does
>>> not change the fact a kernel with EXPERT mode selected will not be
>>> security supported.
>>
>> 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.
> 
> 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.

Assuming I reword the commit message regarding "make clean", could I 
consider this as an acked-by?

Cheers,
Julien Grall May 6, 2020, 9:47 a.m. UTC | #12
Hi Ian,

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@suse.com> 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? :)

Cheers,
Ian Jackson May 7, 2020, 5:01 p.m. UTC | #13
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@suse.com> 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 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.

Alternatively, if you can get agreement from others on the wording of
the commit message, you may put my ack on the patch.

HTH,
Ian.
Julien Grall May 11, 2020, 1:30 p.m. UTC | #14
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@suse.com> 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.

A lot of the options behind EXPERT would benefit to be more accessible 
so user can experiment with it and voice any concern before they are 
fully be supported.

So rather than making really difficult to experiment or tweak your Xen 
(for instance by adding a built-in command line), this option can now be 
selected from the menuconfig.

This doesn't change the fact a Xen with EXPERT mode selected will not be 
security supported.
"

Cheers,
Jan Beulich May 11, 2020, 1:40 p.m. UTC | #15
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@suse.com> 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.

Jan
Julien Grall May 11, 2020, 1:57 p.m. UTC | #16
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@suse.com> 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?

But yes, there are ways to make it permanent. But that involves hacking 
Xen 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.

The compiler is another option that would be worth to move in the 
Kconfig. I have stopped counting the number of time I mixed up between 
Arm64, Arm32 and x86 compilers when building Xen...

Cheers,
Jan Beulich May 11, 2020, 2:07 p.m. UTC | #17
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@suse.com> 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

> 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.)

Jan
Julien Grall May 11, 2020, 2:11 p.m. UTC | #18
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@suse.com> 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,
Jan Beulich May 11, 2020, 2:14 p.m. UTC | #19
On 11.05.2020 16:11, Julien Grall wrote:
> 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@suse.com> 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?

I'm trying to make the point that your patch, to me, looks to be
trying to overcome a problem for which we have had a solution all
the time.

Jan
George Dunlap May 11, 2020, 2:27 p.m. UTC | #20
> On May 11, 2020, at 3:11 PM, Julien Grall <julien@xen.org> wrote:
> 
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> Hi,
> 
> On 11/05/2020 15:07, Jan Beulich wrote:
>> On 11.05.2020 15:57, Julien Grall wrote:
>>> 
>>> 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...

Oh, is that not documented?  It’s quite venerable at this point — if it’s not documented that should change.  (Although I guess there’s an argument that everything which would be included there should be added to either KConfig or configure.)

I don’t think I would have thought to add XEN_CONFIG_EXPERT=y to .config to prevent the issue Julien is seeing.  That maybe part of the reason why this doesn’t bother you as much as it does him.

From a UI perspective, I think that’s a poor UI — enabling CONFIG_EXPERT from the menuconfig is more discoverable and more in line with what people expect.

 -George
Julien Grall May 11, 2020, 2:31 p.m. UTC | #21
On 11/05/2020 15:14, Jan Beulich wrote:
> On 11.05.2020 16:11, Julien Grall wrote:
>> 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@suse.com> 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?
> 
> I'm trying to make the point that your patch, to me, looks to be
> trying to overcome a problem for which we have had a solution all
> the time.

For a first, AFAICT, your solution is not documented anywhere at all... 
It would be possible to document it thought.

However, having two .config for managing Xen options is not very great. 
You have to remember where to add the options and that you need to 
provide the two files if you want someone else to reproduce your setup.

So I would still like to push for a single .config.

As an aside, if you look at the placement of ./.config, it is meant to 
be applied for the full tree. XEN_CONFIG_EXPERT is a config specific to 
the hypervisor, so it is a bit counterintuitive to put it in that file.

Cheers,
George Dunlap May 11, 2020, 3:27 p.m. UTC | #22
> On May 11, 2020, at 2:30 PM, Julien Grall <julien@xen.org> wrote:
> 
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> 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@suse.com> 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.
> 
> A lot of the options behind EXPERT would benefit to be more accessible so user can experiment with it and voice any concern before they are fully be supported.
> 
> So rather than making really difficult to experiment or tweak your Xen (for instance by adding a built-in command line), this option can now be selected from the menuconfig.
> 
> This doesn't change the fact a Xen with EXPERT mode selected will not be security supported.
> "

How about this, clarifying that top-level .config is an option, but that it’s still better to put it in menuconfig?  (Also note a number of grammar tweaks.)

—

EXPERT mode is currently used to gate any options that are in technical
preview or not security supported. At the moment, this is selected by adding XEN_CONFIG_EXPERT=y on the make command line, or to the (currently undocumented) top-level .config file.

This makes the option very unintuitive to use: If the user forgets to add the option when (re)building or when using menuconfig, then xen/.config will be silently rewritten, leading to behavior which is very difficult to diagnose.  Adding XEN_CONFIG_EXPERT=y to the top-level .config is not obvious behavior, particularly as the file is undocumented.

A lot of the options behind EXPERT would benefit from being more accessible so users can experiment with them and voice any concerns before they are fully supported.

To make this option more discoverable and consistent to use, make it possible to select it from the menuconfig.

This doesn't change the fact a Xen with EXPERT mode selected will not be security supported.

—

 -George
Julien Grall May 11, 2020, 3:29 p.m. UTC | #23
Hi George,

On 11/05/2020 16:27, George Dunlap wrote:
> 
> 
>> On May 11, 2020, at 2:30 PM, Julien Grall <julien@xen.org> wrote:
>>
>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>
>> 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@suse.com> 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.
>>
>> A lot of the options behind EXPERT would benefit to be more accessible so user can experiment with it and voice any concern before they are fully be supported.
>>
>> So rather than making really difficult to experiment or tweak your Xen (for instance by adding a built-in command line), this option can now be selected from the menuconfig.
>>
>> This doesn't change the fact a Xen with EXPERT mode selected will not be security supported.
>> "
> 
> How about this, clarifying that top-level .config is an option, but that it’s still better to put it in menuconfig?  (Also note a number of grammar tweaks.)
> 
> —
> 
> EXPERT mode is currently used to gate any options that are in technical
> preview or not security supported. At the moment, this is selected by adding XEN_CONFIG_EXPERT=y on the make command line, or to the (currently undocumented) top-level .config file.
> 
> This makes the option very unintuitive to use: If the user forgets to add the option when (re)building or when using menuconfig, then xen/.config will be silently rewritten, leading to behavior which is very difficult to diagnose.  Adding XEN_CONFIG_EXPERT=y to the top-level .config is not obvious behavior, particularly as the file is undocumented.
> 
> A lot of the options behind EXPERT would benefit from being more accessible so users can experiment with them and voice any concerns before they are fully supported.
> 
> To make this option more discoverable and consistent to use, make it possible to select it from the menuconfig.
> 
> This doesn't change the fact a Xen with EXPERT mode selected will not be security supported.

I am happy this wording.

Cheers,
Ian Jackson May 11, 2020, 5:12 p.m. UTC | #24
Julien Grall writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
> I am happy this wording.

Thanks for engaging with the commit message issue.

This patch is:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

with any of the suggested commit message wordings.  I trust your
judgement to pick a suitable wording.

Ian.
Ian Jackson May 11, 2020, 5:14 p.m. UTC | #25
Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
> I'm trying to make the point that your patch, to me, looks to be
> trying to overcome a problem for which we have had a solution all
> the time.

Thanks for this clear statement of your objection.  I'm afraid I don't
agree.  Even though .config exists (and is even used by osstest, so I
know about it) I don't think it is as good as having it in
menuconfig.

I agree with George's comments:

| From a UI perspective, I think that’s a poor UI — enabling
| CONFIG_EXPERT from the menuconfig is more discoverable and more in
| line with what people expect.

and I haven't seen good reasons (in my opinion) for diverging from
that discoverability and expectation.

So I have given my ack in the other mail.

Regards,
Ian.
Jan Beulich May 12, 2020, 7:18 a.m. UTC | #26
On 11.05.2020 19:14, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>> I'm trying to make the point that your patch, to me, looks to be
>> trying to overcome a problem for which we have had a solution all
>> the time.
> 
> Thanks for this clear statement of your objection.  I'm afraid I don't
> agree.  Even though .config exists (and is even used by osstest, so I
> know about it) I don't think it is as good as having it in
> menuconfig.

But you realize that my objection is (was) more towards the reasoning
behind the change, than towards the change itself. If, as a community,
we decide to undo what we might now call a mistake, and if we're ready
to deal with the consequences, so be it.

Jan
Julien Grall May 12, 2020, 10:08 a.m. UTC | #27
Hi,

On 12/05/2020 08:18, Jan Beulich wrote:
> On 11.05.2020 19:14, Ian Jackson wrote:
>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>>> I'm trying to make the point that your patch, to me, looks to be
>>> trying to overcome a problem for which we have had a solution all
>>> the time.
>>
>> Thanks for this clear statement of your objection.  I'm afraid I don't
>> agree.  Even though .config exists (and is even used by osstest, so I
>> know about it) I don't think it is as good as having it in
>> menuconfig.
> 
> But you realize that my objection is (was) more towards the reasoning
> behind the change, than towards the change itself. If, as a community,
> we decide to undo what we might now call a mistake, and if we're ready
> to deal with the consequences, so be it.

Would you mind to explain the fall out you expect from this patch? Are 
you worry more people may contact security@xen.org for non-security issue?

Cheers,
Jan Beulich May 12, 2020, 10:15 a.m. UTC | #28
On 12.05.2020 12:08, Julien Grall wrote:
> On 12/05/2020 08:18, Jan Beulich wrote:
>> On 11.05.2020 19:14, Ian Jackson wrote:
>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>>>> I'm trying to make the point that your patch, to me, looks to be
>>>> trying to overcome a problem for which we have had a solution all
>>>> the time.
>>>
>>> Thanks for this clear statement of your objection.  I'm afraid I don't
>>> agree.  Even though .config exists (and is even used by osstest, so I
>>> know about it) I don't think it is as good as having it in
>>> menuconfig.
>>
>> But you realize that my objection is (was) more towards the reasoning
>> behind the change, than towards the change itself. If, as a community,
>> we decide to undo what we might now call a mistake, and if we're ready
>> to deal with the consequences, so be it.
> 
> Would you mind to explain the fall out you expect from this patch? Are 
> you worry more people may contact security@xen.org for non-security issue?

That's one possible thing that might happen. But even more generally
the likelihood will increase that people report issues without paying
attention that they depend on their choice of configuration. We'll
have to both take this into consideration and ask back for the
specific .config they've used.

Jan
Julien Grall May 12, 2020, 11 a.m. UTC | #29
Hi Jan,

On 12/05/2020 11:15, Jan Beulich wrote:
> On 12.05.2020 12:08, Julien Grall wrote:
>> On 12/05/2020 08:18, Jan Beulich wrote:
>>> On 11.05.2020 19:14, Ian Jackson wrote:
>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>>>>> I'm trying to make the point that your patch, to me, looks to be
>>>>> trying to overcome a problem for which we have had a solution all
>>>>> the time.
>>>>
>>>> Thanks for this clear statement of your objection.  I'm afraid I don't
>>>> agree.  Even though .config exists (and is even used by osstest, so I
>>>> know about it) I don't think it is as good as having it in
>>>> menuconfig.
>>>
>>> But you realize that my objection is (was) more towards the reasoning
>>> behind the change, than towards the change itself. If, as a community,
>>> we decide to undo what we might now call a mistake, and if we're ready
>>> to deal with the consequences, so be it.
>>
>> Would you mind to explain the fall out you expect from this patch? Are
>> you worry more people may contact security@xen.org for non-security issue?
> 
> That's one possible thing that might happen. But even more generally
> the likelihood will increase that people report issues without paying
> attention that they depend on their choice of configuration.
I agree that you are going to get more report because there are more 
users to try new things. So inevitently, you will get more incomplete 
report. This is always the downside of allowing more flexibility.

But we also need to look at the upside. I can see 2 advantages:
     1) It will be easier to try upcoming features (e.g Argo). The more 
testing and input, the more chance a feature will be a success.
     2) It will be easier to tailor Xen (such as built-in command line).

In both cases, you make Xen more compelling because you allow to 
experiment and make it more flexible. IHMO, this is one of the best way 
to attract users and possible new contributors/reviewers to Xen community.

>  We'll
> have to both take this into consideration and ask back for the
> specific .config they've used.
Correct me if I am wrong, but this is not very specific to EXPERT mode. 
You can already select different options that will affect the behavior 
of the hypervisor. For instance, on x86, you can disable PV guest 
support. How do you figure that out today without asking the .config?

Cheers,
Jan Beulich May 12, 2020, 11:03 a.m. UTC | #30
On 12.05.2020 13:00, Julien Grall wrote:
> Hi Jan,
> 
> On 12/05/2020 11:15, Jan Beulich wrote:
>> On 12.05.2020 12:08, Julien Grall wrote:
>>> On 12/05/2020 08:18, Jan Beulich wrote:
>>>> On 11.05.2020 19:14, Ian Jackson wrote:
>>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>>>>>> I'm trying to make the point that your patch, to me, looks to be
>>>>>> trying to overcome a problem for which we have had a solution all
>>>>>> the time.
>>>>>
>>>>> Thanks for this clear statement of your objection.  I'm afraid I don't
>>>>> agree.  Even though .config exists (and is even used by osstest, so I
>>>>> know about it) I don't think it is as good as having it in
>>>>> menuconfig.
>>>>
>>>> But you realize that my objection is (was) more towards the reasoning
>>>> behind the change, than towards the change itself. If, as a community,
>>>> we decide to undo what we might now call a mistake, and if we're ready
>>>> to deal with the consequences, so be it.
>>>
>>> Would you mind to explain the fall out you expect from this patch? Are
>>> you worry more people may contact security@xen.org for non-security issue?
>>
>> That's one possible thing that might happen. But even more generally
>> the likelihood will increase that people report issues without paying
>> attention that they depend on their choice of configuration.
> I agree that you are going to get more report because there are more 
> users to try new things. So inevitently, you will get more incomplete 
> report. This is always the downside of allowing more flexibility.
> 
> But we also need to look at the upside. I can see 2 advantages:
>      1) It will be easier to try upcoming features (e.g Argo). The more 
> testing and input, the more chance a feature will be a success.
>      2) It will be easier to tailor Xen (such as built-in command line).
> 
> In both cases, you make Xen more compelling because you allow to 
> experiment and make it more flexible. IHMO, this is one of the best way 
> to attract users and possible new contributors/reviewers to Xen community.

I'm fully aware of the upsides.

>>  We'll
>> have to both take this into consideration and ask back for the
>> specific .config they've used.
> Correct me if I am wrong, but this is not very specific to EXPERT mode. 
> You can already select different options that will affect the behavior 
> of the hypervisor. For instance, on x86, you can disable PV guest 
> support. How do you figure that out today without asking the .config?

I didn't say this is a new problem; I indicated this is going to
become more likely to be one.

Jan
George Dunlap May 12, 2020, 11:05 a.m. UTC | #31
> On May 12, 2020, at 12:03 PM, Jan Beulich <jbeulich@suse.com> wrote:
> 
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 12.05.2020 13:00, Julien Grall wrote:
>> Hi Jan,
>> 
>> On 12/05/2020 11:15, Jan Beulich wrote:
>>> On 12.05.2020 12:08, Julien Grall wrote:
>>>> On 12/05/2020 08:18, Jan Beulich wrote:
>>>>> On 11.05.2020 19:14, Ian Jackson wrote:
>>>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>>>>>>> I'm trying to make the point that your patch, to me, looks to be
>>>>>>> trying to overcome a problem for which we have had a solution all
>>>>>>> the time.
>>>>>> 
>>>>>> Thanks for this clear statement of your objection.  I'm afraid I don't
>>>>>> agree.  Even though .config exists (and is even used by osstest, so I
>>>>>> know about it) I don't think it is as good as having it in
>>>>>> menuconfig.
>>>>> 
>>>>> But you realize that my objection is (was) more towards the reasoning
>>>>> behind the change, than towards the change itself. If, as a community,
>>>>> we decide to undo what we might now call a mistake, and if we're ready
>>>>> to deal with the consequences, so be it.
>>>> 
>>>> Would you mind to explain the fall out you expect from this patch? Are
>>>> you worry more people may contact security@xen.org for non-security issue?
>>> 
>>> That's one possible thing that might happen. But even more generally
>>> the likelihood will increase that people report issues without paying
>>> attention that they depend on their choice of configuration.
>> I agree that you are going to get more report because there are more 
>> users to try new things. So inevitently, you will get more incomplete 
>> report. This is always the downside of allowing more flexibility.
>> 
>> But we also need to look at the upside. I can see 2 advantages:
>>     1) It will be easier to try upcoming features (e.g Argo). The more 
>> testing and input, the more chance a feature will be a success.
>>     2) It will be easier to tailor Xen (such as built-in command line).
>> 
>> In both cases, you make Xen more compelling because you allow to 
>> experiment and make it more flexible. IHMO, this is one of the best way 
>> to attract users and possible new contributors/reviewers to Xen community.
> 
> I'm fully aware of the upsides.
> 
>>> We'll
>>> have to both take this into consideration and ask back for the
>>> specific .config they've used.
>> Correct me if I am wrong, but this is not very specific to EXPERT mode. 
>> You can already select different options that will affect the behavior 
>> of the hypervisor. For instance, on x86, you can disable PV guest 
>> support. How do you figure that out today without asking the .config?
> 
> I didn't say this is a new problem; I indicated this is going to
> become more likely to be one.

I feel like there’s a misunderstanding here — Jan, are you simply explaining yourself and/or making sure that we all understand the implications of our choice?  Or are you arguing against acceptance in an implicitly Nack-ing manner?

I understood Jan to be doing the former; and that as such with Ian’s ack, this patch (with the modified commit message) can go in.

 -George
Jan Beulich May 12, 2020, 11:18 a.m. UTC | #32
On 12.05.2020 13:05, George Dunlap wrote:
> 
> 
>> On May 12, 2020, at 12:03 PM, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>
>> On 12.05.2020 13:00, Julien Grall wrote:
>>> Hi Jan,
>>>
>>> On 12/05/2020 11:15, Jan Beulich wrote:
>>>> On 12.05.2020 12:08, Julien Grall wrote:
>>>>> On 12/05/2020 08:18, Jan Beulich wrote:
>>>>>> On 11.05.2020 19:14, Ian Jackson wrote:
>>>>>>> Jan Beulich writes ("Re: [PATCH RESEND 2/2] xen: Allow EXPERT mode to be selected from the menuconfig directly"):
>>>>>>>> I'm trying to make the point that your patch, to me, looks to be
>>>>>>>> trying to overcome a problem for which we have had a solution all
>>>>>>>> the time.
>>>>>>>
>>>>>>> Thanks for this clear statement of your objection.  I'm afraid I don't
>>>>>>> agree.  Even though .config exists (and is even used by osstest, so I
>>>>>>> know about it) I don't think it is as good as having it in
>>>>>>> menuconfig.
>>>>>>
>>>>>> But you realize that my objection is (was) more towards the reasoning
>>>>>> behind the change, than towards the change itself. If, as a community,
>>>>>> we decide to undo what we might now call a mistake, and if we're ready
>>>>>> to deal with the consequences, so be it.
>>>>>
>>>>> Would you mind to explain the fall out you expect from this patch? Are
>>>>> you worry more people may contact security@xen.org for non-security issue?
>>>>
>>>> That's one possible thing that might happen. But even more generally
>>>> the likelihood will increase that people report issues without paying
>>>> attention that they depend on their choice of configuration.
>>> I agree that you are going to get more report because there are more 
>>> users to try new things. So inevitently, you will get more incomplete 
>>> report. This is always the downside of allowing more flexibility.
>>>
>>> But we also need to look at the upside. I can see 2 advantages:
>>>     1) It will be easier to try upcoming features (e.g Argo). The more 
>>> testing and input, the more chance a feature will be a success.
>>>     2) It will be easier to tailor Xen (such as built-in command line).
>>>
>>> In both cases, you make Xen more compelling because you allow to 
>>> experiment and make it more flexible. IHMO, this is one of the best way 
>>> to attract users and possible new contributors/reviewers to Xen community.
>>
>> I'm fully aware of the upsides.
>>
>>>> We'll
>>>> have to both take this into consideration and ask back for the
>>>> specific .config they've used.
>>> Correct me if I am wrong, but this is not very specific to EXPERT mode. 
>>> You can already select different options that will affect the behavior 
>>> of the hypervisor. For instance, on x86, you can disable PV guest 
>>> support. How do you figure that out today without asking the .config?
>>
>> I didn't say this is a new problem; I indicated this is going to
>> become more likely to be one.
> 
> I feel like there’s a misunderstanding here — Jan, are you simply
> explaining yourself and/or making sure that we all understand the
> implications of our choice?  Or are you arguing against acceptance
> in an implicitly Nack-ing manner?

The former - it would have seemed impolite if I hadn't replied to
Julien's question.

> I understood Jan to be doing the former; and that as such with
> Ian’s ack, this patch (with the modified commit message) can go in.

Indeed. Looks like I'm the only one anyway to be concerned of the
extra effort.

Jan

Patch
diff mbox series

diff --git a/xen/Kconfig b/xen/Kconfig
index 120b5f412993..34c318bfa2c7 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -35,7 +35,15 @@  config DEFCONFIG_LIST
 	default ARCH_DEFCONFIG
 
 config EXPERT
-	def_bool y if "$(XEN_CONFIG_EXPERT)" = "y"
+	bool "Configure standard Xen features (expert users)"
+	help
+	  This option allows certain base Xen options and settings
+	  to be disabled or tweaked. This is for specialized environments
+	  which can tolerate a "non-standard" Xen.
+	  Only use this if you really know what you are doing.
+	  Xen binaries built with this option enabled are not security
+	  supported.
+	default n
 
 config LTO
 	bool "Link Time Optimisation"
diff --git a/xen/Makefile b/xen/Makefile
index 2b1dacb49754..286f374b549f 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -11,7 +11,6 @@  export XEN_DOMAIN	?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) |
 export XEN_BUILD_DATE	?= $(shell LC_ALL=C date)
 export XEN_BUILD_TIME	?= $(shell LC_ALL=C date +%T)
 export XEN_BUILD_HOST	?= $(shell hostname)
-export XEN_CONFIG_EXPERT ?= n
 
 # Best effort attempt to find a python interpreter, defaulting to Python 3 if
 # available.  Fall back to just `python` if `which` is nowhere to be found.