|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH v3 2/3] libxl: Allow PCI device passthrough using -device Qemu command line
Le 10/04/2026 à 17:06, Thierry Escande a écrit :
> 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().
If (in this specific case) we're not using QMP anymore, I think
libxl__device_pci_get_qmp_json should lose the "qmp" qualification.
(unless QEMU also considers JSON-formatted command-line paramters as QMP)
>
> 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@xxxxxxxxxx>
> ---
> 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 7c098edab6..efd2664a90 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -2666,6 +2666,7 @@ 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 511ec76a65..28e4adbd4f 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -1798,6 +1798,91 @@ 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 b65e0064b9..cab2ab4526 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -1729,6 +1729,13 @@ _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 49d272d0de..07d005e71d 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -686,7 +686,7 @@ 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;
> @@ -755,9 +755,9 @@ 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;
> @@ -798,7 +798,7 @@ 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;
> }
> @@ -913,7 +913,7 @@ 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);
> @@ -1098,16 +1098,10 @@ 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");
> @@ -1134,11 +1128,30 @@ 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;
Is it expected to set this callback when !pci->hotplug (given it's the
same than the one we call afterward) ?
(I don't really know well how works libxl though)
> - 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;
>
> @@ -1509,7 +1522,7 @@ 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;
> @@ -1572,13 +1585,13 @@ 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;
> @@ -1820,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 */
> + if (!pci->hotplug) {
> + pci_remove_done(egc, prs, 0);
> + return;
> + }
> +
I think you can move the renaming/refactoring part into a separate patch
and make the hotplug part separate.
> pcis = libxl_device_pci_list(ctx, domid, &num);
> if (!pcis) {
> rc = ERROR_FAIL;
--
Teddy Astie | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |