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(-)

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 ;;
-- 
2.42.0

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.

-- 
WBR, Volodymyr