| Message ID | 20231202014108.2017803-5-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it | expand |
On Sat, Dec 02, 2023 at 01:41:22AM +0000, Volodymyr Babchuk wrote: > diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c > index 9f9f137f99..03a55f345c 100644 > --- a/hw/xenpv/xen_machine_pv.c > +++ b/hw/xenpv/xen_machine_pv.c > @@ -37,7 +37,9 @@ static void xen_init_pv(MachineState *machine) > setup_xen_backend_ops(); > > /* Initialize backend core & drivers */ > +#ifdef CONFIG_XEN_LEGACY_BACKENDS > xen_be_init(); > +#endif There's more code that depends on legacy backend support in this function: Call to xen_be_register() and xen_config_dev_nic() and symbol xen_config_cleanup, and the code commented with "configure framebuffer". I've tried to build this on x86. > > switch (xen_mode) { > case XEN_ATTACH: > diff --git a/meson.build b/meson.build > index ec01f8b138..c8a43dd97d 100644 > --- a/meson.build > +++ b/meson.build > @@ -2219,6 +2222,7 @@ config_host_data.set('CONFIG_DBUS_DISPLAY', dbus_display) > config_host_data.set('CONFIG_CFI', get_option('cfi')) > config_host_data.set('CONFIG_SELINUX', selinux.found()) > config_host_data.set('CONFIG_XEN_BACKEND', xen.found()) > +config_host_data.set('CONFIG_XEN_LEGACY_BACKENDS', have_xen_legacy_backends) I don't know if "config_host_data" is the right place to have "#define CONFIG_XEN_LEGACY_BACKENDS", but the alternative is probably to define a Kconfig value, but I don't know if that would be correct as well. I guess this is fine here, for now. > config_host_data.set('CONFIG_LIBDW', libdw.found()) > if xen.found() > # protect from xen.version() having less than three components > @@ -3049,6 +3053,7 @@ config_all += config_targetos > config_all += config_all_disas > config_all += { > 'CONFIG_XEN': xen.found(), > + 'CONFIG_XEN_LEGACY_BACKENDS': have_xen_legacy_backends, I don't think this is useful here, or even wanted. I think things added to config_all are used only in "meson.build" files, for things like "system_ss.add(when: ['CONFIG_XEN_LEGACY_BACKENDS'] ..." But you use "if have_xen_legacy_backends" instead, which is probably ok (because objects also depends on CONFIG_XEN_BUS). > 'CONFIG_SYSTEM_ONLY': have_system, > 'CONFIG_USER_ONLY': have_user, > 'CONFIG_ALL': true, > diff --git a/meson_options.txt b/meson_options.txt > index c9baeda639..91dd677257 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -77,6 +77,8 @@ option('nvmm', type: 'feature', value: 'auto', > description: 'NVMM acceleration support') > option('xen', type: 'feature', value: 'auto', > description: 'Xen backend support') > +option('xen-legacy-backends', type: 'feature', value: 'auto', Every other meson options are using '_', I haven't found any single '-'. Shouldn't this new option follow the same trend and be named "xen_legacy_backends" ? > + description: 'Xen legacy backends (9pfs, fb, qusb) support') This description feels a bit wrong somehow. "Legacy backend" is internal to QEMU's code, and meant that the backends are implemented using legacy support that we want to retire. But the backends them self, as seen by a guest aren't going to change, and are not legacy. Also, a few month ago, "qnic" would have been part of the list. Maybe a description like "Xen backends based on legacy support" might be more appropriate. I'm not sure listing the different backend in the description is a good idea, as we will have to remember to change it whenever one of those backend is been upgraded. Cheers,
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build index 2944ea63c3..e8306ba8d2 100644 --- a/hw/9pfs/meson.build +++ b/hw/9pfs/meson.build @@ -15,7 +15,9 @@ fs_ss.add(files( )) fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c')) fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c')) -fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c')) +if have_xen_legacy_backends + fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c')) +endif system_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss) specific_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-device.c')) diff --git a/hw/display/meson.build b/hw/display/meson.build index 344dfe3d8c..18d657f6b3 100644 --- a/hw/display/meson.build +++ b/hw/display/meson.build @@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true: files('pl110.c')) system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c')) system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c')) system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c')) -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c')) +if have_xen_legacy_backends + system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c')) +endif system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c')) system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c')) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 29b9964733..91857af428 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1263,7 +1263,9 @@ void pc_basic_device_init(struct PCMachineState *pcms, pci_create_simple(pcms->bus, -1, "xen-platform"); } pcms->xenbus = xen_bus_init(); +#ifdef CONFIG_XEN_LEGACY_BACKENDS xen_be_init(); +#endif } #endif diff --git a/hw/usb/meson.build b/hw/usb/meson.build index e94149ebde..8d395745b2 100644 --- a/hw/usb/meson.build +++ b/hw/usb/meson.build @@ -84,6 +84,9 @@ if libusb.found() hw_usb_modules += {'host': usbhost_ss} endif -system_ss.add(when: ['CONFIG_USB', 'CONFIG_XEN_BUS', libusb], if_true: files('xen-usb.c')) +if have_xen_legacy_backends + system_ss.add(when: ['CONFIG_USB', 'CONFIG_XEN_BUS', libusb], + if_true: files('xen-usb.c')) +endif modules += { 'hw-usb': hw_usb_modules } diff --git a/hw/xen/meson.build b/hw/xen/meson.build index d887fa9ba4..964c3364f2 100644 --- a/hw/xen/meson.build +++ b/hw/xen/meson.build @@ -2,11 +2,16 @@ system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files( 'xen-backend.c', 'xen-bus-helper.c', 'xen-bus.c', - 'xen-legacy-backend.c', - 'xen_devconfig.c', - 'xen_pvdev.c', )) +if have_xen_legacy_backends + system_ss.add(when: ['CONFIG_XEN_BUS'], if_true: files( + 'xen_pvdev.c', + 'xen-legacy-backend.c', + 'xen_devconfig.c', + )) +endif + system_ss.add(when: ['CONFIG_XEN', xen], if_true: files( 'xen-operations.c', )) diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index 565dc39c8f..2e7897dbd2 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -869,7 +869,9 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, xen_bus_init(); +#ifdef CONFIG_XEN_LEGACY_BACKENDS xen_be_init(); +#endif return; diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c index 9f9f137f99..03a55f345c 100644 --- a/hw/xenpv/xen_machine_pv.c +++ b/hw/xenpv/xen_machine_pv.c @@ -37,7 +37,9 @@ static void xen_init_pv(MachineState *machine) setup_xen_backend_ops(); /* Initialize backend core & drivers */ +#ifdef CONFIG_XEN_LEGACY_BACKENDS xen_be_init(); +#endif switch (xen_mode) { case XEN_ATTACH: diff --git a/meson.build b/meson.build index ec01f8b138..c8a43dd97d 100644 --- a/meson.build +++ b/meson.build @@ -1749,6 +1749,9 @@ have_xen_pci_passthrough = get_option('xen_pci_passthrough') \ error_message: 'Xen PCI passthrough not available on this platform') \ .allowed() +have_xen_legacy_backends = get_option('xen-legacy-backends').require(xen.found(), + error_message: 'Xen legacy backends requested but Xen not enabled').allowed() + cacard = not_found if not get_option('smartcard').auto() or have_system @@ -2219,6 +2222,7 @@ config_host_data.set('CONFIG_DBUS_DISPLAY', dbus_display) config_host_data.set('CONFIG_CFI', get_option('cfi')) config_host_data.set('CONFIG_SELINUX', selinux.found()) config_host_data.set('CONFIG_XEN_BACKEND', xen.found()) +config_host_data.set('CONFIG_XEN_LEGACY_BACKENDS', have_xen_legacy_backends) config_host_data.set('CONFIG_LIBDW', libdw.found()) if xen.found() # protect from xen.version() having less than three components @@ -3049,6 +3053,7 @@ config_all += config_targetos config_all += config_all_disas config_all += { 'CONFIG_XEN': xen.found(), + 'CONFIG_XEN_LEGACY_BACKENDS': have_xen_legacy_backends, 'CONFIG_SYSTEM_ONLY': have_system, 'CONFIG_USER_ONLY': have_user, 'CONFIG_ALL': true, diff --git a/meson_options.txt b/meson_options.txt index c9baeda639..91dd677257 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -77,6 +77,8 @@ option('nvmm', type: 'feature', value: 'auto', description: 'NVMM acceleration support') option('xen', type: 'feature', value: 'auto', description: 'Xen backend support') +option('xen-legacy-backends', type: 'feature', value: 'auto', + description: 'Xen legacy backends (9pfs, fb, qusb) support') option('xen_pci_passthrough', type: 'feature', value: 'auto', description: 'Xen PCI passthrough support') option('tcg', type: 'feature', value: 'enabled', diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 680fa3f581..b5acef008f 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -218,6 +218,8 @@ meson_options_help() { printf "%s\n" ' werror Treat warnings as errors' printf "%s\n" ' whpx WHPX acceleration support' printf "%s\n" ' xen Xen backend support' + printf "%s\n" ' xen-legacy-backends' + printf "%s\n" ' Xen legacy backends (9pfs, fb, qusb) support' printf "%s\n" ' xen-pci-passthrough' printf "%s\n" ' Xen PCI passthrough support' printf "%s\n" ' xkbcommon xkbcommon support' @@ -556,6 +558,8 @@ _meson_option_parse() { --disable-whpx) printf "%s" -Dwhpx=disabled ;; --enable-xen) printf "%s" -Dxen=enabled ;; --disable-xen) printf "%s" -Dxen=disabled ;; + --enable-xen-legacy-backends) printf "%s" -Dxen-legacy-backends=enabled ;; + --disable-xen-legacy-backends) printf "%s" -Dxen-legacy-backends=disabled ;; --enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;; --disable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=disabled ;; --enable-xkbcommon) printf "%s" -Dxkbcommon=enabled ;;
This patch makes legacy backends optional. As was discussed at [1] this is a solution to a problem when we can't run QEMU as a device model in a non-privileged domain. This is because legacy backends assume that they are always running in domain with ID = 0. Actually, this may prevent running QEMU in a privileged domain with ID not equal to zero. With this patch it is possible to provide "--disable-xen-legacy-backends" configure option to get QEMU binary that can run in a driver domain. With price of not be able to use legacy backends of course. [1] https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- I am not sure if I made correct changes to build system, so this patch is tagged as RFC. Changes in v3: - New patch in v3 --- hw/9pfs/meson.build | 4 +++- hw/display/meson.build | 4 +++- hw/i386/pc.c | 2 ++ hw/usb/meson.build | 5 ++++- hw/xen/meson.build | 11 ++++++++--- hw/xen/xen-hvm-common.c | 2 ++ hw/xenpv/xen_machine_pv.c | 2 ++ meson.build | 5 +++++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 4 ++++ 10 files changed, 35 insertions(+), 6 deletions(-)