[PATCH] shim: don't let build modify shim.config

[PATCH] shim: don't let build modify shim.config

[Juergen Gross]

Currently building the shim will modify shim.config in case some config
option was added or modified in the hypervisor.

Avoid that by copying shim.config to an intermediate file instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
Not sure whether its worth to take that for 4.11.
In case the maintainers think it should be part of 4.11 then LGTM.
A followup patch should reduce the config options specified in
shim.config.
---
 .gitignore                      |  1 +
 tools/firmware/xen-dir/Makefile | 13 +++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/.gitignore b/.gitignore
index 7004349d5a..e7983859ba 100644
--- a/.gitignore
+++ b/.gitignore
@@ -161,6 +161,7 @@ tools/firmware/xen-dir/linkfarm.stamp*
 tools/firmware/xen-dir/xen-root
 tools/firmware/xen-dir/xen-shim
 tools/firmware/xen-dir/xen-shim-syms
+tools/firmware/xen-dir/xen-shim.config
 tools/flask/utils/flask-getenforce
 tools/flask/utils/flask-get-bool
 tools/flask/utils/flask-loadpolicy
diff --git a/tools/firmware/xen-dir/Makefile b/tools/firmware/xen-dir/Makefile
index a7e69ae777..666cb10018 100644
--- a/tools/firmware/xen-dir/Makefile
+++ b/tools/firmware/xen-dir/Makefile
@@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
 $(D): linkfarm.stamp
 	$(MAKE) -C $(D)/xen distclean
 
-.PHONY: shim-%config
-shim-%config: $(D) FORCE
-	$(MAKE) -C $(D)/xen $*config \
+xen-shim.config: shim.config
+	cp $< $@
+	$(MAKE) -C $(D)/xen olddefconfig \
 		XEN_CONFIG_EXPERT=y \
-		KCONFIG_CONFIG=$(CURDIR)/shim.config
+		KCONFIG_CONFIG=$(CURDIR)/xen-shim.config
 
-xen-shim: $(D) shim-olddefconfig
+xen-shim: $(D) xen-shim.config
 	$(MAKE) -C $(D)/xen build \
 		XEN_CONFIG_EXPERT=y \
-		KCONFIG_CONFIG=$(CURDIR)/shim.config
+		KCONFIG_CONFIG=$(CURDIR)/xen-shim.config
 	ln -sf $(D)/xen/xen $@
 	ln -sf $(D)/xen/xen-syms $@-syms
 
 .PHONY: distclean clean
 distclean clean:
 	rm -f xen-shim xen-shim-syms *.old
+	rm -f xen-shim.config
 	rm -rf $(D)
 	rm -f linkfarm.stamp*
-- 
2.13.6


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Jan Beulich]

>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
> --- a/tools/firmware/xen-dir/Makefile
> +++ b/tools/firmware/xen-dir/Makefile
> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
>  $(D): linkfarm.stamp
>  	$(MAKE) -C $(D)/xen distclean
>  
> -.PHONY: shim-%config
> -shim-%config: $(D) FORCE
> -	$(MAKE) -C $(D)/xen $*config \

I think there was a reason for the % and $*.

> +xen-shim.config: shim.config
> +	cp $< $@

Why not copy it to where it actually belong - xen/.config?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Juergen Gross]

On 19/04/18 12:47, Jan Beulich wrote:
>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
>> --- a/tools/firmware/xen-dir/Makefile
>> +++ b/tools/firmware/xen-dir/Makefile
>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
>>  $(D): linkfarm.stamp
>>  	$(MAKE) -C $(D)/xen distclean
>>  
>> -.PHONY: shim-%config
>> -shim-%config: $(D) FORCE
>> -	$(MAKE) -C $(D)/xen $*config \
> 
> I think there was a reason for the % and $*.

The only one I could think of is the possibility to do

make shim-defaultconfig

or similar calls. Question is who will ever need that. In case it is
believed to be valuable I can keep it.

>> +xen-shim.config: shim.config
>> +	cp $< $@
> 
> Why not copy it to where it actually belong - xen/.config?

Hmm, true. In case nobody objects I'll do that.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Wei Liu]

On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
> On 19/04/18 12:47, Jan Beulich wrote:
> >>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
> >> --- a/tools/firmware/xen-dir/Makefile
> >> +++ b/tools/firmware/xen-dir/Makefile
> >> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
> >>  $(D): linkfarm.stamp
> >>  	$(MAKE) -C $(D)/xen distclean
> >>  
> >> -.PHONY: shim-%config
> >> -shim-%config: $(D) FORCE
> >> -	$(MAKE) -C $(D)/xen $*config \
> > 
> > I think there was a reason for the % and $*.
> 
> The only one I could think of is the possibility to do
> 
> make shim-defaultconfig
> 
> or similar calls. Question is who will ever need that. In case it is
> believed to be valuable I can keep it.

Not likely I think.

> 
> >> +xen-shim.config: shim.config
> >> +	cp $< $@
> > 
> > Why not copy it to where it actually belong - xen/.config?
> 
> Hmm, true. In case nobody objects I'll do that.
> 
> 

No objection.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Andrew Cooper]

On 19/04/18 15:54, Wei Liu wrote:
> On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
>> On 19/04/18 12:47, Jan Beulich wrote:
>>>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
>>>> --- a/tools/firmware/xen-dir/Makefile
>>>> +++ b/tools/firmware/xen-dir/Makefile
>>>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
>>>>  $(D): linkfarm.stamp
>>>>  	$(MAKE) -C $(D)/xen distclean
>>>>  
>>>> -.PHONY: shim-%config
>>>> -shim-%config: $(D) FORCE
>>>> -	$(MAKE) -C $(D)/xen $*config \
>>> I think there was a reason for the % and $*.
>> The only one I could think of is the possibility to do
>>
>> make shim-defaultconfig
>>
>> or similar calls. Question is who will ever need that. In case it is
>> believed to be valuable I can keep it.
> Not likely I think.

Using `make shim-menuconfig` was a specific usecase for my original
version, but with this difference in representation, the usecase has
mostly gone away.

With this change in place, what is the expected workflow for making an
update to the configuration?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Wei Liu]

On Thu, Apr 19, 2018 at 05:14:38PM +0100, Andrew Cooper wrote:
> On 19/04/18 15:54, Wei Liu wrote:
> > On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
> >> On 19/04/18 12:47, Jan Beulich wrote:
> >>>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
> >>>> --- a/tools/firmware/xen-dir/Makefile
> >>>> +++ b/tools/firmware/xen-dir/Makefile
> >>>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
> >>>>  $(D): linkfarm.stamp
> >>>>  	$(MAKE) -C $(D)/xen distclean
> >>>>  
> >>>> -.PHONY: shim-%config
> >>>> -shim-%config: $(D) FORCE
> >>>> -	$(MAKE) -C $(D)/xen $*config \
> >>> I think there was a reason for the % and $*.
> >> The only one I could think of is the possibility to do
> >>
> >> make shim-defaultconfig
> >>
> >> or similar calls. Question is who will ever need that. In case it is
> >> believed to be valuable I can keep it.
> > Not likely I think.
> 
> Using `make shim-menuconfig` was a specific usecase for my original
> version, but with this difference in representation, the usecase has
> mostly gone away.
> 
> With this change in place, what is the expected workflow for making an
> update to the configuration?
> 

Just commit the must-have's to the tracked file and let Xen's build
system handle the rest?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Juergen Gross]

On 19/04/18 18:14, Andrew Cooper wrote:
> On 19/04/18 15:54, Wei Liu wrote:
>> On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
>>> On 19/04/18 12:47, Jan Beulich wrote:
>>>>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
>>>>> --- a/tools/firmware/xen-dir/Makefile
>>>>> +++ b/tools/firmware/xen-dir/Makefile
>>>>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
>>>>>  $(D): linkfarm.stamp
>>>>>  	$(MAKE) -C $(D)/xen distclean
>>>>>  
>>>>> -.PHONY: shim-%config
>>>>> -shim-%config: $(D) FORCE
>>>>> -	$(MAKE) -C $(D)/xen $*config \
>>>> I think there was a reason for the % and $*.
>>> The only one I could think of is the possibility to do
>>>
>>> make shim-defaultconfig
>>>
>>> or similar calls. Question is who will ever need that. In case it is
>>> believed to be valuable I can keep it.
>> Not likely I think.
> 
> Using `make shim-menuconfig` was a specific usecase for my original
> version, but with this difference in representation, the usecase has
> mostly gone away.
> 
> With this change in place, what is the expected workflow for making an
> update to the configuration?

You can still cd to tools/firmware/xen-dir/xen-root/xen and do the
"make menuconfig" there. Afterwards copy the resulting .config to
../../shim.config and you have the same result. Question is whether
you want to do that in case we are stripping down shim.config later.

Above assumes V2 of my patch will be taken, of course.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Roger Pau Monné]

On Thu, Apr 19, 2018 at 05:22:28PM +0100, Wei Liu wrote:
> On Thu, Apr 19, 2018 at 05:14:38PM +0100, Andrew Cooper wrote:
> > On 19/04/18 15:54, Wei Liu wrote:
> > > On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
> > >> On 19/04/18 12:47, Jan Beulich wrote:
> > >>>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
> > >>>> --- a/tools/firmware/xen-dir/Makefile
> > >>>> +++ b/tools/firmware/xen-dir/Makefile
> > >>>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
> > >>>>  $(D): linkfarm.stamp
> > >>>>  	$(MAKE) -C $(D)/xen distclean
> > >>>>  
> > >>>> -.PHONY: shim-%config
> > >>>> -shim-%config: $(D) FORCE
> > >>>> -	$(MAKE) -C $(D)/xen $*config \
> > >>> I think there was a reason for the % and $*.
> > >> The only one I could think of is the possibility to do
> > >>
> > >> make shim-defaultconfig
> > >>
> > >> or similar calls. Question is who will ever need that. In case it is
> > >> believed to be valuable I can keep it.
> > > Not likely I think.
> > 
> > Using `make shim-menuconfig` was a specific usecase for my original
> > version, but with this difference in representation, the usecase has
> > mostly gone away.
> > 
> > With this change in place, what is the expected workflow for making an
> > update to the configuration?
> > 
> 
> Just commit the must-have's to the tracked file and let Xen's build
> system handle the rest?

+1. As a bare minimum:

CONFIG_XEN_GUEST=y
CONFIG_PVH_GUEST=y
CONFIG_PV_SHIM=y
CONFIG_PV_SHIM_EXCLUSIVE=y

But I expect some more might be needed.

Having a full config will always be a PITA because it will get out of
synch.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Andrew Cooper]

On 20/04/18 10:28, Roger Pau Monné wrote:
> On Thu, Apr 19, 2018 at 05:22:28PM +0100, Wei Liu wrote:
>> On Thu, Apr 19, 2018 at 05:14:38PM +0100, Andrew Cooper wrote:
>>> On 19/04/18 15:54, Wei Liu wrote:
>>>> On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
>>>>> On 19/04/18 12:47, Jan Beulich wrote:
>>>>>>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
>>>>>>> --- a/tools/firmware/xen-dir/Makefile
>>>>>>> +++ b/tools/firmware/xen-dir/Makefile
>>>>>>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
>>>>>>>  $(D): linkfarm.stamp
>>>>>>>  	$(MAKE) -C $(D)/xen distclean
>>>>>>>  
>>>>>>> -.PHONY: shim-%config
>>>>>>> -shim-%config: $(D) FORCE
>>>>>>> -	$(MAKE) -C $(D)/xen $*config \
>>>>>> I think there was a reason for the % and $*.
>>>>> The only one I could think of is the possibility to do
>>>>>
>>>>> make shim-defaultconfig
>>>>>
>>>>> or similar calls. Question is who will ever need that. In case it is
>>>>> believed to be valuable I can keep it.
>>>> Not likely I think.
>>> Using `make shim-menuconfig` was a specific usecase for my original
>>> version, but with this difference in representation, the usecase has
>>> mostly gone away.
>>>
>>> With this change in place, what is the expected workflow for making an
>>> update to the configuration?
>>>
>> Just commit the must-have's to the tracked file and let Xen's build
>> system handle the rest?
> +1. As a bare minimum:
>
> CONFIG_XEN_GUEST=y
> CONFIG_PVH_GUEST=y
> CONFIG_PV_SHIM=y
> CONFIG_PV_SHIM_EXCLUSIVE=y
>
> But I expect some more might be needed.

CONFIG_SHADOW_PAGING=n
CONFIG_NR_CPUS=32
CONFIG_VIDEO=n
CONFIG_HVM_FEP=n
CONFIG_TBOOT=n
CONFIG_KEXEC=n
CONFIG_TMEM=n
CONFIG_XENOPROF=n

Remember that shim also wants to be as much of a slimline build as possible

> Having a full config will always be a PITA because it will get out of
> synch.

However we store it, it will get out of sync.  I don't see a solution to
this problem.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] shim: don't let build modify shim.config

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On 20/04/18 11:33, Andrew Cooper wrote:
> On 20/04/18 10:28, Roger Pau Monné wrote:
>> On Thu, Apr 19, 2018 at 05:22:28PM +0100, Wei Liu wrote:
>>> On Thu, Apr 19, 2018 at 05:14:38PM +0100, Andrew Cooper wrote:
>>>> On 19/04/18 15:54, Wei Liu wrote:
>>>>> On Thu, Apr 19, 2018 at 01:01:53PM +0200, Juergen Gross wrote:
>>>>>> On 19/04/18 12:47, Jan Beulich wrote:
>>>>>>>>>> On 19.04.18 at 12:30, <jgross@suse.com> wrote:
>>>>>>>> --- a/tools/firmware/xen-dir/Makefile
>>>>>>>> +++ b/tools/firmware/xen-dir/Makefile
>>>>>>>> @@ -41,21 +41,22 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
>>>>>>>>  $(D): linkfarm.stamp
>>>>>>>>  	$(MAKE) -C $(D)/xen distclean
>>>>>>>>  
>>>>>>>> -.PHONY: shim-%config
>>>>>>>> -shim-%config: $(D) FORCE
>>>>>>>> -	$(MAKE) -C $(D)/xen $*config \
>>>>>>> I think there was a reason for the % and $*.
>>>>>> The only one I could think of is the possibility to do
>>>>>>
>>>>>> make shim-defaultconfig
>>>>>>
>>>>>> or similar calls. Question is who will ever need that. In case it is
>>>>>> believed to be valuable I can keep it.
>>>>> Not likely I think.
>>>> Using `make shim-menuconfig` was a specific usecase for my original
>>>> version, but with this difference in representation, the usecase has
>>>> mostly gone away.
>>>>
>>>> With this change in place, what is the expected workflow for making an
>>>> update to the configuration?
>>>>
>>> Just commit the must-have's to the tracked file and let Xen's build
>>> system handle the rest?
>> +1. As a bare minimum:
>>
>> CONFIG_XEN_GUEST=y
>> CONFIG_PVH_GUEST=y
>> CONFIG_PV_SHIM=y
>> CONFIG_PV_SHIM_EXCLUSIVE=y
>>
>> But I expect some more might be needed.
> 
> CONFIG_SHADOW_PAGING=n
> CONFIG_NR_CPUS=32
> CONFIG_VIDEO=n
> CONFIG_HVM_FEP=n
> CONFIG_TBOOT=n
> CONFIG_KEXEC=n
> CONFIG_TMEM=n
> CONFIG_XENOPROF=n
> 
> Remember that shim also wants to be as much of a slimline build as possible

Attached is the minimal shim.config to be fed into "make olddefconfig"
resulting in the current config used for the shim.


Juergen

[-- Attachment #2: shim.config --]
[-- Type: application/x-config, Size: 482 bytes --]

[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel