| Message ID | 20231124232400.943580-4-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] hw/xen: Set XenBackendInstance in the XenDevice before realizing it | expand |
On Fri, 2023-11-24 at 23:24 +0000, Volodymyr Babchuk wrote: > 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. > > To be able to disable legacy backends we need to alter couple of > source files that unintentionally depend on them. For example > xen-all.c used xen_pv_printf to report errors, while not providing any > additional like xendev pointer. Also, we need to move xenstore > structure from xen-legacy-backend.c, because it is apparently used in > xen-all.c. > > 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 the build system, thus this > patch is tagged as RFC. Hm, I was imagining a new CONFIG_LEGACY_XEN_BACKENDS option which would look a lot like CONFIG_XEN_BUS (which would now be only for the new XenBus code). This looks weird to me: > --- 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')) I'd prefer to see just: -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c')) +system_ss.add(when: 'CONFIG_XEN_LEGACY_BACKENDS', if_true: files('xenfb.c')) Probably also better to split out the bits in accel/xen/xen-all.c and hw/xen/xen-legacy-backend.c to a separate preparatory commit. Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
Hi David, "Woodhouse, David" <dwmw@amazon.co.uk> writes: > [[S/MIME Signed Part:Undecided]] > On Fri, 2023-11-24 at 23:24 +0000, Volodymyr Babchuk wrote: >> 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. >> >> To be able to disable legacy backends we need to alter couple of >> source files that unintentionally depend on them. For example >> xen-all.c used xen_pv_printf to report errors, while not providing any >> additional like xendev pointer. Also, we need to move xenstore >> structure from xen-legacy-backend.c, because it is apparently used in >> xen-all.c. >> >> 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 the build system, thus this >> patch is tagged as RFC. > > Hm, I was imagining a new CONFIG_LEGACY_XEN_BACKENDS option which would > look a lot like CONFIG_XEN_BUS (which would now be only for the new > XenBus code). > It was my original intention too. But it appears that it is not possible to add Kconfig value and then make it configurable via ./config script. As I understood it can be set only via defconfig file. > This looks weird to me: > >> --- 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')) > > I'd prefer to see just: > > -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c')) > +system_ss.add(when: 'CONFIG_XEN_LEGACY_BACKENDS', if_true: files('xenfb.c')) I tried, but it does not work this way. I need to create Kconfig variable to do this, but then other problems appear. > > Probably also better to split out the bits in accel/xen/xen-all.c and > hw/xen/xen-legacy-backend.c to a separate preparatory commit. Okay, will do.
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c index 5ff0cb8bd9..188b29597f 100644 --- a/accel/xen/xen-all.c +++ b/accel/xen/xen-all.c @@ -29,6 +29,7 @@ bool xen_allowed; xc_interface *xen_xc; xenforeignmemory_handle *xen_fmem; xendevicemodel_handle *xen_dmod; +struct qemu_xs_handle *xenstore; static void xenstore_record_dm_state(const char *state) { @@ -78,20 +79,26 @@ static int xen_init(MachineState *ms) { MachineClass *mc = MACHINE_GET_CLASS(ms); + xenstore = qemu_xen_xs_open(); + if (!xenstore) { + error_report("can't connect to xenstored\n"); + exit(1); + } + xen_xc = xc_interface_open(0, 0, 0); if (xen_xc == NULL) { - xen_pv_printf(NULL, 0, "can't open xen interface\n"); + error_report("can't open xen interface\n"); return -1; } xen_fmem = xenforeignmemory_open(0, 0); if (xen_fmem == NULL) { - xen_pv_printf(NULL, 0, "can't open xen fmem interface\n"); + error_report("can't open xen fmem interface\n"); xc_interface_close(xen_xc); return -1; } xen_dmod = xendevicemodel_open(0, 0); if (xen_dmod == NULL) { - xen_pv_printf(NULL, 0, "can't open xen devicemodel interface\n"); + error_report("can't open xen devicemodel interface\n"); xenforeignmemory_close(xen_fmem); xc_interface_close(xen_xc); return -1; 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/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c index 124dd5f3d6..717d5efc06 100644 --- a/hw/xen/xen-legacy-backend.c +++ b/hw/xen/xen-legacy-backend.c @@ -39,7 +39,6 @@ BusState *xen_sysbus; /* ------------------------------------------------------------- */ /* public */ -struct qemu_xs_handle *xenstore; const char *xen_protocol; /* private */ @@ -605,12 +604,6 @@ static void xen_set_dynamic_sysbus(void) void xen_be_init(void) { - xenstore = qemu_xen_xs_open(); - if (!xenstore) { - xen_pv_printf(NULL, 0, "can't connect to xenstored\n"); - exit(1); - } - if (xen_evtchn_ops == NULL || xen_gnttab_ops == NULL) { xen_pv_printf(NULL, 0, "Xen operations not set up\n"); exit(1); 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. To be able to disable legacy backends we need to alter couple of source files that unintentionally depend on them. For example xen-all.c used xen_pv_printf to report errors, while not providing any additional like xendev pointer. Also, we need to move xenstore structure from xen-legacy-backend.c, because it is apparently used in xen-all.c. 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 the build system, thus this patch is tagged as RFC. Changes in v3: - New patch in v3 --- accel/xen/xen-all.c | 13 ++++++++++--- 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/xen/xen-legacy-backend.c | 7 ------- hw/xenpv/xen_machine_pv.c | 2 ++ meson.build | 5 +++++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 4 ++++ 12 files changed, 45 insertions(+), 16 deletions(-)