Re: [PATCH 3/3] x86/pv: Compile out compat_gdt in !CONFIG_PV builds

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.04.2020 17:50, Andrew Cooper wrote:
> There is no need for the Compat GDT if there are no 32bit PV guests.  This
> saves 4k per online CPU
> 
> Bloat-o-meter reports the following savings in Xen itself:
> 
>   add/remove: 0/3 grow/shrink: 1/4 up/down: 7/-4612 (-4605)
>   Function                                     old     new   delta
>   cpu_smpboot_free                            1249    1256      +7
>   per_cpu__compat_gdt_l1e                        8       -      -8
>   per_cpu__compat_gdt                            8       -      -8
>   init_idt_traps                               442     420     -22
>   load_system_tables                           414     364     -50
>   trap_init                                    444     280    -164
>   cpu_smpboot_callback                        1255     991    -264
>   boot_compat_gdt                             4096       -   -4096
>   Total: Before=3062726, After=3058121, chg -0.15%
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> The increase in cpu_smpboot_free() appears to be a consequence of a totally
> different layout of basic blocks.
> ---
>  xen/arch/x86/cpu/common.c |  5 +++--
>  xen/arch/x86/desc.c       |  2 ++
>  xen/arch/x86/smpboot.c    |  5 ++++-
>  xen/arch/x86/traps.c      | 10 +++++++---
>  4 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index 1b33f1ed71..7b093cb421 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -752,8 +752,9 @@ void load_system_tables(void)
>  
>       _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss,
>                        sizeof(*tss) - 1, SYS_DESC_tss_avail);
> -     _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss,
> -                      sizeof(*tss) - 1, SYS_DESC_tss_busy);
> +     if ( IS_ENABLED(CONFIG_PV32) )
> +             _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss,
> +                              sizeof(*tss) - 1, SYS_DESC_tss_busy);

Wouldn't this better be "if ( opt_pv32 )"? Also elsewhere then.

Jan