Series comparison

This series adds support for a new 'hotplug' option for PCI devices to be
passthrough'd to guests.

The current mechanism for device passthrough uses QMP device_add command
to hot-plug PCI devices to the guest Qemu instance. This is an issue for
guests running on Q35 chipset (preliminary support posted at [1]) since the
Q35 PCI root bus does not support hotplug. Devices could be hotplugged to
a secondary PCI bus but Xen only support 1 PCI bus for now.

The 'hotplug' option allows to control how devices are attached to the
guest, either by using the legacy QMP mechanism (this is the default) or by
passing it directly to the Qemu command line using xen-pci-passthrough
device.

Example usage in cfg file:
 pci = [ "00:03.0,seize=1,hotplug=0" ]

Since Qemu -device option accepts parameters in json format, this
patchset adds a new internal function libxl__device_pci_get_qmp_json() that
generates the device json object used for both QMP hotplug and command line
passing.
Also, the function libxl_pci_assignable() is made available from libxl.h
for access in libxl_dm.c. I don't know if it's ok to make it public or if
it should stay private. Maybe it should be renamed as
libxl_device_pci_assignable() to match the other PCI device APIs.

This has been successfully tested on Xen 4.22-dev and Qemu 10.0 running
Debian VMs in both Bios and UEFI mode with a passthrough'd nvme disk.
Disabling the hotplug mechanism might be made mandatory for Q35 machines
later, once Q35 support is merged upstream.

[1] https://lore.kernel.org/xen-devel/20260313163455.790692-1-thierry.escande@vates.tech/

Thierry Escande (3):
  xl: Add pci device hotplug option
  libxl: Allow PCI device passthrough using -device Qemu command line
  docs: provide description for pci hotplug option

docs/man/xl-pci-configuration.5.pod | 17 ++++++++++++
 docs/man/xl.cfg.5.pod.in            |  6 +++++
 tools/include/libxl.h               |  1 +
 tools/libs/light/libxl_dm.c         | 31 ++++++++++++++++++++++
 tools/libs/light/libxl_internal.h   |  2 ++
 tools/libs/light/libxl_pci.c        | 41 ++++++++++++++++++++++-------
 tools/libs/light/libxl_types.idl    |  1 +
 tools/libs/util/libxlu_pci.c        |  2 ++
 tools/xl/xl_parse.c                 |  5 ++++
 9 files changed, 96 insertions(+), 10 deletions(-)

-- 
2.53.0

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

This option is used to passthrough PCI devices using the Qemu command
line (-device driver,...) instead of the QMP hot-plug mechanism.

This is needed for Q35 support since its PCI root bus doesn't support
hotplugging.

If not specified, the default behavior is to use hotplug via QMP.

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
---
 tools/libs/light/libxl_types.idl | 1 +
 tools/libs/util/libxlu_pci.c     | 2 ++
 tools/xl/xl_parse.c              | 5 +++++
 3 files changed, 8 insertions(+)

diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index XXXXXXX..XXXXXXX 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -XXX,XX +XXX,XX @@ libxl_device_pci = Struct("device_pci", [
     ("seize", bool),
     ("rdm_policy", libxl_rdm_reserve_policy),
     ("name", string),
+    ("hotplug", bool),
     ])
 
 libxl_device_rdm = Struct("device_rdm", [
diff --git a/tools/libs/util/libxlu_pci.c b/tools/libs/util/libxlu_pci.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/libs/util/libxlu_pci.c
+++ b/tools/libs/util/libxlu_pci.c
@@ -XXX,XX +XXX,XX @@ int xlu_pci_parse_spec_string(XLU_Config *cfg, libxl_device_pci *pci,
             name_present = true;
             pci->name = strdup(val);
             if (!pci->name) ret = ERROR_NOMEM;
+        } else if (!strcmp(key, "hotplug")) {
+            pci->hotplug = atoi(val);
         } else {
             XLU__PCI_ERR(cfg, "Unknown PCI_SPEC_STRING option: %s", key);
             ret = ERROR_INVAL;
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -XXX,XX +XXX,XX @@ void parse_config_data(const char *config_source,
     int pci_msitranslate = 0;
     int pci_permissive = 0;
     int pci_seize = 0;
+    int pci_hotplug = 1;
     int i, e;
     int num_llc_colors;
     int num_xs_quota;
@@ -XXX,XX +XXX,XX @@ void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long (config, "pci_seize", &l, 0))
         pci_seize = l;
 
+    if (!xlu_cfg_get_long (config, "pci_hotplug", &l, 0))
+        pci_hotplug = l;
+
     if (!xlu_cfg_get_string(config, "rdm", &buf, 0)) {
         libxl_rdm_reserve rdm;
         if (!xlu_rdm_parse(config, &rdm, buf)) {
@@ -XXX,XX +XXX,XX @@ void parse_config_data(const char *config_source,
             pci->power_mgmt = pci_power_mgmt;
             pci->permissive = pci_permissive;
             pci->seize = pci_seize;
+            pci->hotplug = pci_hotplug;
             /*
              * Like other pci option, the per-device policy always follows
              * the global policy by default.
-- 
2.53.0

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

This change makes use of the new option 'hotplug' for host PCI devices
passthrough'd to the guest. If hotplug=0 is used in the pci device
configuration table, the device will be attached to the guest using the
Qemu command line as '-device xen-pci-passthrough,hostaddr=...'

The host device configuration is passed to the -device option as a json
array, just like it's done for hotplug using QMP. The json array is
created by a new internal function libxl__device_pci_get_qmp_json() that
is also used by pci_add_qmp_device_add().

Then, instead of sending the 'device_add' command, the device_add
callback is called to perform the 'query-pci' check to make sure the
passthrough'd device is present.

In the same way at shutdown, the device is not removed using QMP and
only the pci_remove_done() function is called.

As with QMP, the use of the 'hotplug=0' option honors the 'seize' option
by adding the PCI device to the assignable list if needed.

Example use:
 pci = [ "00:03.0,seize=1,hotplug=0" ]

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
---
 tools/include/libxl.h             |  1 +
 tools/libs/light/libxl_dm.c       | 31 +++++++++++++++++++++++
 tools/libs/light/libxl_internal.h |  2 ++
 tools/libs/light/libxl_pci.c      | 41 +++++++++++++++++++++++--------
 4 files changed, 65 insertions(+), 10 deletions(-)

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

This patch adds description for the 'hotplug' PCI option, allowing
devices to be attached to the guest using the legacy QMP hotplug
mechanism or by passing them to the Qemu command line using '-device'.

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
---
 docs/man/xl-pci-configuration.5.pod | 17 +++++++++++++++++
 docs/man/xl.cfg.5.pod.in            |  6 ++++++
 2 files changed, 23 insertions(+)

diff --git a/docs/man/xl-pci-configuration.5.pod b/docs/man/xl-pci-configuration.5.pod
index XXXXXXX..XXXXXXX 100644
--- a/docs/man/xl-pci-configuration.5.pod
+++ b/docs/man/xl-pci-configuration.5.pod
@@ -XXX,XX +XXX,XX @@ dom0 without confirmation.  Please use with care.
 
 =back
 
+=item B<hotplug>=I<BOOLEAN>
+
+=over 4
+
+=item Description
+
+Tells L<xl(1)> to use QMP hotplug mechanism to attach assignable device to
+the guest, or to pass it via Qemu command line using
+'B<-device xen-pci-passthrough,...>'. The default is to use the legacy QMP
+mechanism.
+
+=item Default Value
+
+1
+
+=back
+
 =item B<power_mgmt>=I<BOOLEAN>
 
 =over 4
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index XXXXXXX..XXXXXXX 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -XXX,XX +XXX,XX @@ B<(HVM only)> Changes the default value of B<power_mgmt> for all PCI
 devices passed through to this VM. See B<power_mgmt>
 above.
 
+=item B<pci_hotplug=BOOLEAN>
+
+B<(HVM only)> Changes the default value of B<hotplug> for all PCI
+devices passed through to this VM. See L<xl-pci-configuration(5)> for
+more details.
+
 =item B<gfx_passthru=BOOLEAN|"STRING">
 
 Enable graphics device PCI passthrough. This option makes an assigned
-- 
2.53.0

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

This series adds support for a new 'hotplug' option for PCI devices to be
passthrough'd to guests.

Example usage in cfg file:
 pci = [ "00:03.0,seize=1,hotplug=0" ]

Also, to handle the assignable PCI device list, the function
libxl_pci_assignable() is renamed as libxl_device_pci_assignable() and is
made available from libxl.h for access in libxl_dm.c. 2 other functions are
also exported internally: libxl__pciback_dev_is_assigned() and
libxl__device_pci_assignable_add().

[1] https://lore.kernel.org/xen-devel/20260313163455.790692-1-thierry.escande@vates.tech/

Changes in v2:
- Add support for YAJL json parser

Changes in v3:
- Move code block of device command line parameters creation
- Better handling of pci device assignation

Thierry Escande (3):
  xl: Add pci device hotplug option
  libxl: Allow PCI device passthrough using -device Qemu command line
  docs: provide description for pci hotplug option

-- 
2.53.0

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

This option is used to passthrough PCI devices using the Qemu command
line (-device driver,...) instead of the QMP hot-plug mechanism.

This is needed for Q35 support since its PCI root bus doesn't support
hotplugging.

If not specified, the default behavior is to use hotplug via QMP.

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
---
v2: no change
v3: no change
---
 tools/libs/light/libxl_types.idl | 1 +
 tools/libs/util/libxlu_pci.c     | 2 ++
 tools/xl/xl_parse.c              | 5 +++++
 3 files changed, 8 insertions(+)

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Then, instead of sending the 'device_add' command, the device_add
callback is called to perform the 'query-pci' check to make sure the
passthrough'd device is present.

In the same way at shutdown, the device is not removed using QMP and
only the pci_remove_done() function is called.

As with QMP, the use of the 'hotplug=0' option honors the 'seize' option
by adding the PCI device to the assignable list if needed. This mimics
what is done in libxl__device_pci_add() with regards to seize option and
the assignable PCI device list. This allows to display a proper error
message if the device is not assignable before Qemu starts.
To do so the function pciback_dev_is_assigned() has been renamed as
libxl__pciback_dev_is_assigned() and made available internally, as well
as libxl__device_pci_assignable_add(). Also, libxl_pci_assignable() is
now exported in libxl.h and renamed as libxl_device_pci_assignable()
since its prototype looks like the other libxl_device_pci_*() APIs.

Example use:
 pci = [ "00:03.0,seize=1,hotplug=0" ]

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
---
v2:
- Add support for YAJL json parser

v3:
- Move code block for device command line parameters creation to a
  correct place.
- Better handling of PCI device assignation check to display the correct
  error message if the device is not assignable.
---
 tools/include/libxl.h             |  1 +
 tools/libs/light/libxl_dm.c       | 85 +++++++++++++++++++++++++++++++
 tools/libs/light/libxl_internal.h |  7 +++
 tools/libs/light/libxl_pci.c      | 57 ++++++++++++++-------
 4 files changed, 132 insertions(+), 18 deletions(-)

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index XXXXXXX..XXXXXXX 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -XXX,XX +XXX,XX @@ int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_device_pci *pci, int r
 int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_device_pci *pci, int rebind);
 libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num);
 void libxl_device_pci_assignable_list_free(libxl_device_pci *list, int num);
+bool libxl_device_pci_assignable(libxl_ctx *ctx, libxl_device_pci *pci);
 
 /* CPUID handling */
 int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str);
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -XXX,XX +XXX,XX @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
             break;
         }
 
+        if (guest_config->num_pcidevs) {
+            libxl_device_pci *pci;
+            libxl__json_object *qmp_json;
+            char *json_str;
+#ifdef HAVE_LIBJSONC
+            json_object *jso;
+            const char *buf;
+#elif defined(HAVE_LIBYAJL)
+            yajl_gen hand;
+            /* memory for 'buf' is owned by 'hand' */
+            const unsigned char *buf;
+            libxl_yajl_length len;
+#else
+#           error Missing JSON library
+#endif
+
+            for (i = 0; i < guest_config->num_pcidevs; i++) {
+                pci = &guest_config->pcidevs[i];
+
+                if (pci->hotplug)
+                    continue;
+
+                if (pci->seize && !libxl__pciback_dev_is_assigned(gc, pci)) {
+                    rc = libxl__device_pci_assignable_add(gc, pci, 1);
+                    if (rc)
+                        return rc;
+                }
+
+                if (!libxl_device_pci_assignable(libxl__gc_owner(gc), pci)) {
+                    LOGD(ERROR, guest_domid, "PCI device %x:%x:%x.%x is not assignable",
+                         pci->domain, pci->bus, pci->dev, pci->func);
+                    return ERROR_FAIL;
+                }
+
+                qmp_json = libxl__device_pci_get_qmp_json(gc, pci);
+
+#ifdef HAVE_LIBJSONC
+                rc = libxl__json_object_to_json_object(gc, &jso, qmp_json);
+                if (rc)
+                    return rc;
+
+                buf = json_object_to_json_string_ext(jso,
+                                                     JSON_C_TO_STRING_PLAIN);
+                if (!buf) {
+                    json_object_put(jso);
+                    return ERROR_NOMEM;
+                }
+#elif defined(HAVE_LIBYAJL)
+                hand = libxl_yajl_gen_alloc(NULL);
+                if (!hand) {
+                    return ERROR_NOMEM;
+                }
+#if HAVE_YAJL_V2
+                /* Disable beautify for data sent to QEMU */
+                yajl_gen_config(hand, yajl_gen_beautify, 0);
+#endif
+
+                rc = libxl__json_object_to_yajl_gen(gc, hand, qmp_json);
+                if (rc) {
+                    yajl_gen_free(hand);
+                    return rc;
+                }
+
+                rc = yajl_gen_get_buf(hand, &buf, &len);
+                if (rc != yajl_gen_status_ok) {
+                    yajl_gen_free(hand);
+                    return rc;
+                }
+#endif
+
+                json_str = libxl__strdup(gc, (const char *)buf);
+                if (json_str)
+                    flexarray_vappend(dm_args, "-device", json_str, NULL);
+
+#ifdef HAVE_LIBJSONC
+                json_object_put(jso);
+#elif defined(HAVE_LIBYAJL)
+                yajl_gen_free(hand);
+#endif
+
+                if (!json_str)
+                    return ERROR_NOMEM;
+            }
+        }
+
         if (state->dm_runas) {
             if (qemu_opts->have_runwith_user) {
                 flexarray_append_pair(dm_args, "-run-with",
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index XXXXXXX..XXXXXXX 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -XXX,XX +XXX,XX @@ _hidden int libxl__device_pci_setdefault(libxl__gc *gc, uint32_t domid,
                                          libxl_device_pci *pci, bool hotplug);
 _hidden bool libxl__is_igd_vga_passthru(libxl__gc *gc,
                                         const libxl_domain_config *d_config);
+_hidden libxl__json_object *libxl__device_pci_get_qmp_json(libxl__gc *gc,
+                                                         libxl_device_pci *pci);
+_hidden int libxl__pciback_dev_is_assigned(libxl__gc *gc,
+                                           libxl_device_pci *pci);
+_hidden int libxl__device_pci_assignable_add(libxl__gc *gc,
+                                             libxl_device_pci *pci,
+                                             int rebind);
 
 /* from libxl_dtdev */
 
diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
index XXXXXXX..XXXXXXX 100644
--- a/tools/libs/light/libxl_pci.c
+++ b/tools/libs/light/libxl_pci.c
@@ -XXX,XX +XXX,XX @@ out:
     return rc;
 }
 
-static int pciback_dev_is_assigned(libxl__gc *gc, libxl_device_pci *pci)
+int libxl__pciback_dev_is_assigned(libxl__gc *gc, libxl_device_pci *pci)
 {
     char * spath;
     int rc;
@@ -XXX,XX +XXX,XX @@ static int pciback_dev_unassign(libxl__gc *gc, libxl_device_pci *pci)
     return 0;
 }
 
-static int libxl__device_pci_assignable_add(libxl__gc *gc,
-                                            libxl_device_pci *pci,
-                                            int rebind)
+int libxl__device_pci_assignable_add(libxl__gc *gc,
+                                     libxl_device_pci *pci,
+                                     int rebind)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     unsigned dom, bus, dev, func;
@@ -XXX,XX +XXX,XX @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
     }
 
     /* Check to see if it's already assigned to pciback */
-    rc = pciback_dev_is_assigned(gc, pci);
+    rc = libxl__pciback_dev_is_assigned(gc, pci);
     if ( rc < 0 ) {
         return ERROR_FAIL;
     }
@@ -XXX,XX +XXX,XX @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
     }
 
     /* Unbind from pciback */
-    if ( (rc = pciback_dev_is_assigned(gc, pci)) < 0 ) {
+    if ( (rc = libxl__pciback_dev_is_assigned(gc, pci)) < 0 ) {
         return ERROR_FAIL;
     } else if ( rc ) {
         pciback_dev_unassign(gc, pci);
@@ -XXX,XX +XXX,XX @@ out:
     pci_add_dm_done(egc, pas, rc); /* must be last */
 }
 
-static void pci_add_qmp_device_add(libxl__egc *egc, pci_add_state *pas)
+libxl__json_object *libxl__device_pci_get_qmp_json(libxl__gc *gc,
+                                                   libxl_device_pci *pci)
 {
-    STATE_AO_GC(pas->aodev->ao);
     libxl__json_object *args = NULL;
-    int rc;
-
-    /* Convenience aliases */
-    libxl_domid domid = pas->domid;
-    libxl_device_pci *pci = &pas->pci;
-    libxl__ev_qmp *const qmp = &pas->qmp;
 
     libxl__qmp_param_add_string(gc, &args, "driver",
                                 "xen-pci-passthrough");
@@ -XXX,XX +XXX,XX @@ static void pci_add_qmp_device_add(libxl__egc *egc, pci_add_state *pas)
     if (pci->permissive)
         libxl__qmp_param_add_bool(gc, &args, "permissive", true);
 
+    return args;
+}
+
+static void pci_add_qmp_device_add(libxl__egc *egc, pci_add_state *pas)
+{
+    STATE_AO_GC(pas->aodev->ao);
+    libxl__json_object *args = NULL;
+    int rc = 0;
+
+    /* Convenience aliases */
+    libxl_domid domid = pas->domid;
+    libxl_device_pci *pci = &pas->pci;
+    libxl__ev_qmp *const qmp = &pas->qmp;
+
+    args = libxl__device_pci_get_qmp_json(gc, pci);
+
     qmp->ao = pas->aodev->ao;
     qmp->domid = domid;
     qmp->payload_fd = -1;
     qmp->callback = pci_add_qmp_device_add_cb;
-    rc = libxl__ev_qmp_send(egc, qmp, "device_add", args);
+    if (pci->hotplug)
+        rc = libxl__ev_qmp_send(egc, qmp, "device_add", args);
+    else
+        pci_add_qmp_device_add_cb(egc, qmp, NULL, 0);
     if (rc) goto out;
     return;
 
@@ -XXX,XX +XXX,XX @@ int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid,
     return AO_INPROGRESS;
 }
 
-static bool libxl_pci_assignable(libxl_ctx *ctx, libxl_device_pci *pci)
+bool libxl_device_pci_assignable(libxl_ctx *ctx, libxl_device_pci *pci)
 {
     libxl_device_pci *pcis;
     int num;
@@ -XXX,XX +XXX,XX @@ void libxl__device_pci_add(libxl__egc *egc, uint32_t domid,
     rc = libxl__device_pci_setdefault(gc, domid, pci, !starting);
     if (rc) goto out;
 
-    if (pci->seize && !pciback_dev_is_assigned(gc, pci)) {
+    if (pci->seize && !libxl__pciback_dev_is_assigned(gc, pci)) {
         rc = libxl__device_pci_assignable_add(gc, pci, 1);
         if ( rc )
             goto out;
     }
 
-    if (!libxl_pci_assignable(ctx, pci)) {
+    if (!libxl_device_pci_assignable(ctx, pci)) {
         LOGD(ERROR, domid, "PCI device %x:%x:%x.%x is not assignable",
              pci->domain, pci->bus, pci->dev, pci->func);
         rc = ERROR_FAIL;
@@ -XXX,XX +XXX,XX @@ static void do_pci_remove(libxl__egc *egc, pci_remove_state *prs)
     libxl_domain_type type = libxl__domain_type(gc, domid);
     libxl_device_pci *pci = &prs->pci;
     int rc, num;
+
+    /* Passthrough'd device has been passed to Qemu command line so there is
+     * no need to remove it via QMP */
+    if (!pci->hotplug) {
+        pci_remove_done(egc, prs, 0);
+        return;
+    }
+
     pcis = libxl_device_pci_list(ctx, domid, &num);
     if (!pcis) {
         rc = ERROR_FAIL;
-- 
2.53.0

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
---
v2: no change
v3: no change
---
 docs/man/xl-pci-configuration.5.pod | 17 +++++++++++++++++
 docs/man/xl.cfg.5.pod.in            |  6 ++++++
 2 files changed, 23 insertions(+)

--
Thierry Escande | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech