qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>,
	xen-devel@lists.xenproject.org, qemu-devel@nongnu.org,
	Masami Hiramatsu <masami.hiramatsu@linaro.org>,
	Paul Durrant <paul@xen.org>
Subject: Re: [PATCH] meson.build: fix building of Xen support for aarch64
Date: Thu, 29 Oct 2020 10:01:27 +0000	[thread overview]
Message-ID: <87d011mjuw.fsf@linaro.org> (raw)
In-Reply-To: <alpine.DEB.2.21.2010281406080.12247@sstabellini-ThinkPad-T480s>


Stefano Stabellini <sstabellini@kernel.org> 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@linaro.org>
>> Cc: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Paul Durrant <paul@xen.org>
>> 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


  reply	other threads:[~2020-10-29 10:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28 17:44 [PATCH] meson.build: fix building of Xen support for aarch64 Alex Bennée
2020-10-28 21:17 ` Stefano Stabellini
2020-10-29 10:01   ` Alex Bennée [this message]
2020-10-29 13:37     ` Jason Andryuk
2020-10-29 20:00       ` Stefano Stabellini
2020-10-29 20:06     ` Stefano Stabellini
2020-10-29  2:11 ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d011mjuw.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=anthony.perard@citrix.com \
    --cc=masami.hiramatsu@linaro.org \
    --cc=paul@xen.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).