[RFC PATCH v4 4/6] xen: add option to disable legacy backends

Volodymyr Babchuk posted 6 patches 12 months ago
[RFC PATCH v4 4/6] xen: add option to disable legacy backends
Posted by Volodymyr Babchuk 12 months ago
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(-)

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 ;;
-- 
2.42.0
Re: [RFC PATCH v4 4/6] xen: add option to disable legacy backends
Posted by Anthony PERARD 11 months, 2 weeks ago
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,

-- 
Anthony PERARD