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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

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