Re: [PATCH] meson.build: fix building of Xen support for aarch64

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 28 Oct 2020, Alex Bennée wrote:
> Xen is supported on aarch64 although weirdly using the i386-softmmu
> model. Checking based on the host CPU meant we never enabled Xen
> support. It would be nice to enable CONFIG_XEN for aarch64-softmmu to
> make it not seem weird but that will require further build surgery.
> 
> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Cc: Masami Hiramatsu <masami.hiramatsu@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>
> Cc: Paul Durrant <paul@xxxxxxx>
> Fixes: 8a19980e3f ("configure: move accelerator logic to meson")
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 835424999d..f1fcbfed4c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -81,6 +81,8 @@ if cpu in ['x86', 'x86_64']
>      'CONFIG_HVF': ['x86_64-softmmu'],
>      'CONFIG_WHPX': ['i386-softmmu', 'x86_64-softmmu'],
>    }
> +elif cpu in [ 'arm', 'aarch64' ]
> +  accelerator_targets += { 'CONFIG_XEN': ['i386-softmmu'] }
>  endif

This looks very reasonable -- the patch makes sense.


However I have two questions, mostly for my own understanding. I tried
to repro the aarch64 build problem but it works at my end, even without
this patch. I wonder why. I suspect it works thanks to these lines in
meson.build:

  if not get_option('xen').disabled() and 'CONFIG_XEN_BACKEND' in config_host
    accelerators += 'CONFIG_XEN'
    have_xen_pci_passthrough = not get_option('xen_pci_passthrough').disabled() 
and targetos == 'linux'
  else
    have_xen_pci_passthrough = false
  endif

But I am not entirely sure who is adding CONFIG_XEN_BACKEND to
config_host.


The other question is: does it make sense to print the value of
CONFIG_XEN as part of the summary? Something like:

diff --git a/meson.build b/meson.build
index 47e32e1fcb..c6e7832dc9 100644
--- a/meson.build
+++ b/meson.build
@@ -2070,6 +2070,7 @@ summary_info += {'KVM support':       
config_all.has_key('CONFIG_KVM')}
 summary_info += {'HAX support':       config_all.has_key('CONFIG_HAX')}
 summary_info += {'HVF support':       config_all.has_key('CONFIG_HVF')}
 summary_info += {'WHPX support':      config_all.has_key('CONFIG_WHPX')}
+summary_info += {'XEN support':      config_all.has_key('CONFIG_XEN')}
 summary_info += {'TCG support':       config_all.has_key('CONFIG_TCG')}
 if config_all.has_key('CONFIG_TCG')
   summary_info += {'TCG debug enabled': 
config_host.has_key('CONFIG_DEBUG_TCG')}


But I realize there is already:

summary_info += {'xen support':       config_host.has_key('CONFIG_XEN_BACKEND')}

so it would be a bit of a duplicate