Re: [RFC PATCH v4 5/6] libxl: Allow PCI device passthrough using -device Qemu command line

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Apr 23, 2026 at 04:50:30PM +0200, Thierry Escande wrote:
> 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

s/json array/JSON object/

> created by using libxl__device_pci_get_qapi_json() introduced by the
> previous patch.
> 
> 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.

That's not the reason we do `query-pci`, we run the command to find out
where QEMU assigned the device, to extract the the devfn. We already
know the device is present, either because QEMU started or because the
QMP command didn't returned an error.

We could avoid the `query-pci` command if we tell QEMU where to place
the new device.

So here, we skip `device_add` because that's already been done via the
command line.


> 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.

:-(, so we are duplicating code to handle pci->seize. That's not great.
Could you extract that so we can call it for hotplug and domain startup?

> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 511ec76a65..fb0aeea640 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -1798,6 +1798,39 @@ 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;
> +
> +            for (i = 0; i < guest_config->num_pcidevs; i++) {
> +                pci = &guest_config->pcidevs[i];
> +
> +                if (pci->hotplug)
> +                    continue;

Since I don't think having an existing `pci->hotplug` field is a good
idea, you could run the whole loop based on the DM's machine type, that
is if it is Q35 we add pci-passthrough devices on the command line,
otherwise it's done via QMP.

In libxl__add_pcis(), we have access to `d_config`, so we can check
`device_model_machine` and pass the information to run `device_add` cmd
or not via the parameters of libxl__device_pci_add().

If it turns out to be needed for i440FX machines, we could introduce a
domain config for the whole VM rather than by PCI devices we want to
passthrough.

> +                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)) {

There's the macro `CTX` that can be used instead of `libxl__gc_owner(gc)`.

> +                    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_qapi_json(gc, pci);

It's not `qmp_json` here :-), QMP is the protocol use to communicate
with QEMU via a socket, but there's no QMP involve on the command line.
That function doesn't return any JSON, it returns an object that can
represent some JSON. 

Here, we have an object to be passed to `-device` or `device_add`, maybe
just `pci_obj` would be ok as a name for the variable.

> +                json_str = libxl__json_object_to_json(gc, qmp_json, false);
> +                if (!json_str)
> +                    return ERROR_NOMEM;
> +
> +                flexarray_vappend(dm_args, "-device", json_str, NULL);
> +            }
> +        }
> +
>          if (state->dm_runas) {
>              if (qemu_opts->have_runwith_user) {
>                  flexarray_append_pair(dm_args, "-run-with",
> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> index 5004ca47d9..f5216f6b33 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -1148,7 +1148,10 @@ static void pci_add_qmp_device_add(libxl__egc *egc, 
> pci_add_state *pas)
>      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);

Could you add the mention /* must be last */ on the same line?
Also, the call to pci_add_qmp_device_add() should be immediately followed by
"return", we don't want to be in a position where "pci_add_dm_done()"
could be executed twice.

Could you add a comment? Saying we skip "device_add" because the device
has already be added via command line?

>      if (rc) goto out;

So, this could be moved to the true block of the if, right after
libxl__ev_qmp_send() call.

>      return;
>  
> @@ -1830,6 +1833,14 @@ 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 */

That's not the only thing been done. After trying to ask nicely QEMU to
detach from the PCI device, there's a bunch more cleanup done in dom0.
If you look at the success and error path, we always call
`pci_remove_detached()`.

The only problem is that when Q35 is been use, we just want be able to
hot-unplug the device from QEMU's emulated PCI bus, but that's just a
detail; and it doesn't really matter how the device is been added to the
machine, there isn't much difference between QEMU's command line and QMP.

> +    if (!pci->hotplug) {
> +        pci_remove_done(egc, prs, 0);
> +        return;
> +    }
> +

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech