x86/shim: Fix defconfig selection and trim the build further

[PATCH for-4.14] x86/shim: Fix defconfig selection and trim the build further

Posted by Andrew Cooper 3 years, 11 months ago

Several options (TBOOT, XENOPROF, Scheduler) depend on EXPERT to be able to
deselect/configure.

Enabling EXPERT now causes the request of the Credit1 scheduler to be honoured
(rather than giving us Credit2), but take this opportunity to switch to Null,
as the previously problematic issues are now believed to be fixed.

Enabling EXPERT also allows XEN_SHSTK to be selected, and we don't want this
being built for shim.  We also don't want TRACEBUFFER or GDBSX either.

Take this oppotunity to swap the disable of HVM_FEP for a general disable of
HVM (likely to have wider impliciations in the future), and disable ARGO (will
necesserily need plumbing work to function in shim).

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Paul Durrant <paul@xen.org>
CC: Dario Faggioli <dfaggioli@suse.com>
---
 xen/arch/x86/configs/pvshim_defconfig | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/configs/pvshim_defconfig b/xen/arch/x86/configs/pvshim_defconfig
index 830660e022..3af48d6c06 100644
--- a/xen/arch/x86/configs/pvshim_defconfig
+++ b/xen/arch/x86/configs/pvshim_defconfig
@@ -5,19 +5,25 @@ CONFIG_PVH_GUEST=y
 CONFIG_PV_SHIM=y
 CONFIG_PV_SHIM_EXCLUSIVE=y
 CONFIG_NR_CPUS=32
+CONFIG_EXPERT=y
+CONFIG_SCHED_NULL=y
 # Disable features not used by the PV shim
+# CONFIG_HVM is not set
+# CONFIG_XEN_SHSTK is not set
 # CONFIG_HYPFS is not set
 # CONFIG_SHADOW_PAGING is not set
 # CONFIG_BIGMEM is not set
-# CONFIG_HVM_FEP is not set
 # CONFIG_TBOOT is not set
 # CONFIG_KEXEC is not set
 # CONFIG_XENOPROF is not set
 # CONFIG_XSM is not set
+# CONFIG_ARGO is not set
+# CONFIG_SCHED_CREDIT is not set
 # CONFIG_SCHED_CREDIT2 is not set
 # CONFIG_SCHED_RTDS is not set
 # CONFIG_SCHED_ARINC653 is not set
-# CONFIG_SCHED_NULL is not set
 # CONFIG_LIVEPATCH is not set
 # CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
+# CONFIG_TRACEBUFFER is not set
 # CONFIG_DEBUG is not set
+# CONFIG_GDBSX is not set
-- 
2.11.0

Re: [PATCH for-4.14] x86/shim: Fix defconfig selection and trim the build further

Posted by Jan Beulich 3 years, 11 months ago

On 03.06.2020 19:09, Andrew Cooper wrote:
> Several options (TBOOT, XENOPROF, Scheduler) depend on EXPERT to be able to
> deselect/configure.
> 
> Enabling EXPERT now causes the request of the Credit1 scheduler to be honoured
> (rather than giving us Credit2), but take this opportunity to switch to Null,
> as the previously problematic issues are now believed to be fixed.
> 
> Enabling EXPERT also allows XEN_SHSTK to be selected, and we don't want this
> being built for shim.  We also don't want TRACEBUFFER or GDBSX either.
> 
> Take this oppotunity to swap the disable of HVM_FEP for a general disable of
> HVM (likely to have wider impliciations in the future), and disable ARGO (will
> necesserily need plumbing work to function in shim).

Odd. I was quite sure this is the case already; in particular my
own build test of a shim config has this already.

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

I have a question though (without implying the patch here needs
adjusting, but rather with a look towards after 4.14):

> --- a/xen/arch/x86/configs/pvshim_defconfig
> +++ b/xen/arch/x86/configs/pvshim_defconfig
> @@ -5,19 +5,25 @@ CONFIG_PVH_GUEST=y
>  CONFIG_PV_SHIM=y
>  CONFIG_PV_SHIM_EXCLUSIVE=y
>  CONFIG_NR_CPUS=32
> +CONFIG_EXPERT=y
> +CONFIG_SCHED_NULL=y
>  # Disable features not used by the PV shim
> +# CONFIG_HVM is not set
> +# CONFIG_XEN_SHSTK is not set
>  # CONFIG_HYPFS is not set
>  # CONFIG_SHADOW_PAGING is not set
>  # CONFIG_BIGMEM is not set
> -# CONFIG_HVM_FEP is not set
>  # CONFIG_TBOOT is not set
>  # CONFIG_KEXEC is not set
>  # CONFIG_XENOPROF is not set
>  # CONFIG_XSM is not set
> +# CONFIG_ARGO is not set
> +# CONFIG_SCHED_CREDIT is not set
>  # CONFIG_SCHED_CREDIT2 is not set
>  # CONFIG_SCHED_RTDS is not set
>  # CONFIG_SCHED_ARINC653 is not set
> -# CONFIG_SCHED_NULL is not set
>  # CONFIG_LIVEPATCH is not set
>  # CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
> +# CONFIG_TRACEBUFFER is not set
>  # CONFIG_DEBUG is not set
> +# CONFIG_GDBSX is not set

I assume both the "enable" and "disable" sections here are ordered
like they would be in a resulting full .config. But this being two
separate sections, doing so doesn't help e.g. diff-ing. How about
we sort both sections alphabetically (short of other good sorting
criteria, yet better than entirely unsorted)?

Jan

RE: [PATCH for-4.14] x86/shim: Fix defconfig selection and trim the build further

Posted by Paul Durrant 3 years, 11 months ago

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 04 June 2020 07:43
> To: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Xen-devel <xen-devel@lists.xenproject.org>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; Juergen Gross <jgross@suse.com>; Paul Durrant <paul@xen.org>; Dario Faggioli
> <dfaggioli@suse.com>
> Subject: Re: [PATCH for-4.14] x86/shim: Fix defconfig selection and trim the build further
> 
> On 03.06.2020 19:09, Andrew Cooper wrote:
> > Several options (TBOOT, XENOPROF, Scheduler) depend on EXPERT to be able to
> > deselect/configure.
> >
> > Enabling EXPERT now causes the request of the Credit1 scheduler to be honoured
> > (rather than giving us Credit2), but take this opportunity to switch to Null,
> > as the previously problematic issues are now believed to be fixed.
> >
> > Enabling EXPERT also allows XEN_SHSTK to be selected, and we don't want this
> > being built for shim.  We also don't want TRACEBUFFER or GDBSX either.
> >
> > Take this oppotunity to swap the disable of HVM_FEP for a general disable of
> > HVM (likely to have wider impliciations in the future), and disable ARGO (will
> > necesserily need plumbing work to function in shim).
> 
> Odd. I was quite sure this is the case already; in particular my
> own build test of a shim config has this already.
> 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Release-acked-by: Paul Durrant <paul@xen.org>

> 
> I have a question though (without implying the patch here needs
> adjusting, but rather with a look towards after 4.14):
> 
> > --- a/xen/arch/x86/configs/pvshim_defconfig
> > +++ b/xen/arch/x86/configs/pvshim_defconfig
> > @@ -5,19 +5,25 @@ CONFIG_PVH_GUEST=y
> >  CONFIG_PV_SHIM=y
> >  CONFIG_PV_SHIM_EXCLUSIVE=y
> >  CONFIG_NR_CPUS=32
> > +CONFIG_EXPERT=y
> > +CONFIG_SCHED_NULL=y
> >  # Disable features not used by the PV shim
> > +# CONFIG_HVM is not set
> > +# CONFIG_XEN_SHSTK is not set
> >  # CONFIG_HYPFS is not set
> >  # CONFIG_SHADOW_PAGING is not set
> >  # CONFIG_BIGMEM is not set
> > -# CONFIG_HVM_FEP is not set
> >  # CONFIG_TBOOT is not set
> >  # CONFIG_KEXEC is not set
> >  # CONFIG_XENOPROF is not set
> >  # CONFIG_XSM is not set
> > +# CONFIG_ARGO is not set
> > +# CONFIG_SCHED_CREDIT is not set
> >  # CONFIG_SCHED_CREDIT2 is not set
> >  # CONFIG_SCHED_RTDS is not set
> >  # CONFIG_SCHED_ARINC653 is not set
> > -# CONFIG_SCHED_NULL is not set
> >  # CONFIG_LIVEPATCH is not set
> >  # CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
> > +# CONFIG_TRACEBUFFER is not set
> >  # CONFIG_DEBUG is not set
> > +# CONFIG_GDBSX is not set
> 
> I assume both the "enable" and "disable" sections here are ordered
> like they would be in a resulting full .config. But this being two
> separate sections, doing so doesn't help e.g. diff-ing. How about
> we sort both sections alphabetically (short of other good sorting
> criteria, yet better than entirely unsorted)?
> 
> Jan

Re: [PATCH for-4.14] x86/shim: Fix defconfig selection and trim the build further

Posted by Andrew Cooper 3 years, 11 months ago

On 04/06/2020 07:43, Jan Beulich wrote:
> On 03.06.2020 19:09, Andrew Cooper wrote:
>> Several options (TBOOT, XENOPROF, Scheduler) depend on EXPERT to be able to
>> deselect/configure.
>>
>> Enabling EXPERT now causes the request of the Credit1 scheduler to be honoured
>> (rather than giving us Credit2), but take this opportunity to switch to Null,
>> as the previously problematic issues are now believed to be fixed.
>>
>> Enabling EXPERT also allows XEN_SHSTK to be selected, and we don't want this
>> being built for shim.  We also don't want TRACEBUFFER or GDBSX either.
>>
>> Take this oppotunity to swap the disable of HVM_FEP for a general disable of
>> HVM (likely to have wider impliciations in the future), and disable ARGO (will
>> necesserily need plumbing work to function in shim).
> Odd. I was quite sure this is the case already; in particular my
> own build test of a shim config has this already.

It is currently off because of its default in Xen, but even if that were
to change, its not usable in shim without further development work.

>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

>
> I have a question though (without implying the patch here needs
> adjusting, but rather with a look towards after 4.14):
>
>> --- a/xen/arch/x86/configs/pvshim_defconfig
>> +++ b/xen/arch/x86/configs/pvshim_defconfig
>> @@ -5,19 +5,25 @@ CONFIG_PVH_GUEST=y
>>  CONFIG_PV_SHIM=y
>>  CONFIG_PV_SHIM_EXCLUSIVE=y
>>  CONFIG_NR_CPUS=32
>> +CONFIG_EXPERT=y
>> +CONFIG_SCHED_NULL=y
>>  # Disable features not used by the PV shim
>> +# CONFIG_HVM is not set
>> +# CONFIG_XEN_SHSTK is not set
>>  # CONFIG_HYPFS is not set
>>  # CONFIG_SHADOW_PAGING is not set
>>  # CONFIG_BIGMEM is not set
>> -# CONFIG_HVM_FEP is not set
>>  # CONFIG_TBOOT is not set
>>  # CONFIG_KEXEC is not set
>>  # CONFIG_XENOPROF is not set
>>  # CONFIG_XSM is not set
>> +# CONFIG_ARGO is not set
>> +# CONFIG_SCHED_CREDIT is not set
>>  # CONFIG_SCHED_CREDIT2 is not set
>>  # CONFIG_SCHED_RTDS is not set
>>  # CONFIG_SCHED_ARINC653 is not set
>> -# CONFIG_SCHED_NULL is not set
>>  # CONFIG_LIVEPATCH is not set
>>  # CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set
>> +# CONFIG_TRACEBUFFER is not set
>>  # CONFIG_DEBUG is not set
>> +# CONFIG_GDBSX is not set
> I assume both the "enable" and "disable" sections here are ordered
> like they would be in a resulting full .config.

They are, yes.

> But this being two
> separate sections, doing so doesn't help e.g. diff-ing.

Having them in order helps massively with scanning through the two files
together.

I'm not entirely certain why the two sections are separate to begin
with.  Merging them would probably make things even easier, but I think
the file does want to stay in .config order.

~Andrew