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

[Alex Bennée <alex.bennee@xxxxxxxxxx>]

Stefano Stabellini <sstabellini@xxxxxxxxxx> writes:

> 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.

Building on a x86 host or with cross compiler?

> 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 is part of the top level configure check - which basically checks
for --enable-xen or autodetects the presence of the userspace libraries.
I'm not sure if we have a slight over proliferation of symbols for Xen
support (although I'm about to add more).

> 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

Hmm so what we have is:

  CONFIG_XEN_BACKEND
    - ensures that appropriate compiler flags are added
    - pegs RAM_ADDR_MAX at UINT64_MAX (instead of UINTPTR_MAX)
  CONFIG_XEN
    - which controls a bunch of build objects, some of which may be i386 only?
    ./accel/meson.build:15:specific_ss.add_all(when: ['CONFIG_XEN'], if_true: 
dummy_ss)
    ./accel/stubs/meson.build:2:specific_ss.add(when: 'CONFIG_XEN', if_false: 
files('xen-stub.c'))
    ./accel/xen/meson.build:1:specific_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen-all.c'))
    ./hw/9pfs/meson.build:17:fs_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen-9p-backend.c'))
    ./hw/block/dataplane/meson.build:2:specific_ss.add(when: 'CONFIG_XEN', 
if_true: files('xen-block.c'))
    ./hw/block/meson.build:14:softmmu_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen-block.c'))
    ./hw/char/meson.build:23:softmmu_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen_console.c'))
    ./hw/display/meson.build:18:softmmu_ss.add(when: 'CONFIG_XEN', if_true: 
files('xenfb.c'))
    ./hw/i386/xen/meson.build:1:i386_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen-hvm.c',
                                                                               
'xen-mapcache.c',
                                                                               
'xen_apic.c',
                                                                               
'xen_platform.c',
                                                                               
'xen_pvdevice.c')
    ./hw/net/meson.build:2:softmmu_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen_nic.c'))
    ./hw/usb/meson.build:76:softmmu_ss.add(when: ['CONFIG_USB', 'CONFIG_XEN', 
libusb], if_true: files('xen-usb.c'))
    ./hw/xen/meson.build:1:softmmu_ss.add(when: ['CONFIG_XEN', xen], if_true: 
files(
    ./hw/xen/meson.build:20:specific_ss.add_all(when: ['CONFIG_XEN', xen], 
if_true: xen_specific_ss)
    ./hw/xenpv/meson.build:3:xenpv_ss.add(when: 'CONFIG_XEN', if_true: 
files('xen_machine_pv.c'))
    - there are also some stubbed inline functions controlled by it
  CONFIG_XEN_IGD_PASSTHROUGH
    - specific x86 PC only feature via Kconfig rule
  CONFIG_XEN_PCI_PASSTHROUGH
    - control Linux specific specific feature (via meson rule)


First obvious question - is everything in hw/i386/xen actually i386
only? APIC seems pretty PC orientated but what about xen_platform and
pvdevice? There seem to be some dependancies on xen-mapcache across the
code.

-- 
Alex Bennée