[Xen-devel] [PATCH v8 0/4] Fix PCI passthrough for HVM with stubdomain

[Xen-devel] [PATCH v8 0/4] Fix PCI passthrough for HVM with stubdomain

[Marek Marczykowski-Górecki]

This version is rebase libxl patches on staging. Changes to "libxl: attach PCI
device to qemu only after setting pciback/pcifront" are significant enough that
I've dropped Wei's ack.
Also, there is a new patch to fix regression introduced during async conversion.

Changes in v2:
 - new "xen/x86: Allow stubdom access to irq created for msi" patch
 - applied review comments from v1
Changes is v3:
 - apply suggestions by Roger
 - add PHYSDEVOP_msi_msix_set_enable
Changes in v4:
 - implement suggestions by Wei, Roger, Jan
 - plug new physdevop into XSM
Changes in v5:
 - rebase on master
 - rename to PHYSDEVOP_msi_control
 - move granting access to IRQ into create_irq
Changes in v6:
 - simplify granting IRQ access, record dm domid for cleanup
 - rename to PHYSDEVOP_interrupt_control
 - include INTx control in the hypercall
Changes in v7:
 - update "xen/x86: Allow stubdom access to irq created for msi"
 - drop "xen/x86: add PHYSDEVOP_interrupt_control"
 - drop "tools/libxc: add wrapper for PHYSDEVOP_interrupt_control"
Chages in v8:
 - drop applied "xen/x86: Allow stubdom access to irq created for msi"
 - new patch "libxl: fix cold plugged PCI device with stubdomain"
 - rebase libxl patches on staging

---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Marek Marczykowski-Górecki (4):
  libxl: fix cold plugged PCI device with stubdomain
  libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use
  libxl: attach PCI device to qemu only after setting pciback/pcifront
  libxl: don't try to manipulate json config for stubdomain

 tools/libxl/libxl_pci.c | 96 +++++++++++++++++++++++++++++++++---------
 1 file changed, 77 insertions(+), 19 deletions(-)

base-commit: 7a4e6711114905b3cbbe48e81c3222361a7f3579
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain

[Marek Marczykowski-Górecki]

When libxl__device_pci_add() is called, stubdomain is already running,
even when still constructing the target domain. Previously, do_pci_add()
was called with 'starting' hardcoded to false, but now do_pci_add() shares
'starting' flag in pci_add_state for both target domain and stubdomain.

Fix this by resetting (local) 'starting' to false in pci_add_dm_done()
(previously part of do_pci_add()) when handling stubdomain, regardless
of pas->starting value.

Fixes: 11db56f9a6 (libxl_pci: Use libxl__ao_device with libxl__device_pci_add)
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libxl/libxl_pci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4725817..2edff64 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1300,6 +1300,10 @@ static void pci_add_dm_done(libxl__egc *egc,
 
     if (rc) goto out;
 
+    /* stubdomain is always running by now, even at create time */
+    if (isstubdom)
+        starting = false;
+
     sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
                            pcidev->bus, pcidev->dev, pcidev->func);
     f = fopen(sysfs_path, "r");
@@ -1559,7 +1563,6 @@ void libxl__device_pci_add(libxl__egc *egc, uint32_t domid,
         GCNEW(pcidev_s);
         libxl_device_pci_init(pcidev_s);
         libxl_device_pci_copy(CTX, pcidev_s, pcidev);
-        /* stubdomain is always running by now, even at create time */
         pas->callback = device_pci_add_stubdom_done;
         do_pci_add(egc, stubdomid, pcidev_s, pas); /* must be last */
         return;
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain

[Anthony PERARD]

On Sat, Sep 28, 2019 at 04:20:34PM +0200, Marek Marczykowski-Górecki wrote:
> When libxl__device_pci_add() is called, stubdomain is already running,
> even when still constructing the target domain. Previously, do_pci_add()
> was called with 'starting' hardcoded to false, but now do_pci_add() shares
> 'starting' flag in pci_add_state for both target domain and stubdomain.
> 
> Fix this by resetting (local) 'starting' to false in pci_add_dm_done()
> (previously part of do_pci_add()) when handling stubdomain, regardless
> of pas->starting value.
> 
> Fixes: 11db56f9a6 (libxl_pci: Use libxl__ao_device with libxl__device_pci_add)
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain

[Ian Jackson]

Hi Juergen.  This series
  https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg03072.html
needs your release review.

Here's the first patch.  I can bounce you a digest if you like.

Thanks,
Ian.

Marek Marczykowski-Górecki writes ("[PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain"):
> When libxl__device_pci_add() is called, stubdomain is already running,
> even when still constructing the target domain. Previously, do_pci_add()
> was called with 'starting' hardcoded to false, but now do_pci_add() shares
> 'starting' flag in pci_add_state for both target domain and stubdomain.
> 
> Fix this by resetting (local) 'starting' to false in pci_add_dm_done()
> (previously part of do_pci_add()) when handling stubdomain, regardless
> of pas->starting value.
> 
> Fixes: 11db56f9a6 (libxl_pci: Use libxl__ao_device with libxl__device_pci_add)
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  tools/libxl/libxl_pci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 4725817..2edff64 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1300,6 +1300,10 @@ static void pci_add_dm_done(libxl__egc *egc,
>  
>      if (rc) goto out;
>  
> +    /* stubdomain is always running by now, even at create time */
> +    if (isstubdom)
> +        starting = false;
> +
>      sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
>                             pcidev->bus, pcidev->dev, pcidev->func);
>      f = fopen(sysfs_path, "r");
> @@ -1559,7 +1563,6 @@ void libxl__device_pci_add(libxl__egc *egc, uint32_t domid,
>          GCNEW(pcidev_s);
>          libxl_device_pci_init(pcidev_s);
>          libxl_device_pci_copy(CTX, pcidev_s, pcidev);
> -        /* stubdomain is always running by now, even at create time */
>          pas->callback = device_pci_add_stubdom_done;
>          do_pci_add(egc, stubdomid, pcidev_s, pas); /* must be last */
>          return;
> -- 
> git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain

[Jürgen Groß]

On 02.10.19 17:43, Ian Jackson wrote:
> Hi Juergen.  This series
>    https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg03072.html
> needs your release review.

Fir the series:

Release-acked-by: Juergen Gross <jgross@suse.com>

even if I don't see why it is necessary: the series was posted first way
before last posting date.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain

[Ian Jackson]

Jürgen Groß writes ("Re: [PATCH v8 1/4] libxl: fix cold plugged PCI device with stubdomain"):
> On 02.10.19 17:43, Ian Jackson wrote:
> > Hi Juergen.  This series
> >    https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg03072.html
> > needs your release review.
> 
> Fir the series:
> 
> Release-acked-by: Juergen Gross <jgross@suse.com>

Thanks.  I have pushed this (and added a couple of acks of my own).

> even if I don't see why it is necessary: the series was posted first way
> before last posting date.

I think I may have been confused.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 2/4] libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use

[Marek Marczykowski-Górecki]

HVM domains use IOMMU and device model assistance for communicating with
PCI devices, xen-pcifront/pciback isn't directly needed by HVM domain.
But pciback serve also second function - it reset the device when it is
deassigned from the guest and for this reason pciback needs to be used
with HVM domain too.
When HVM domain has device model in stubdomain, attaching xen-pciback to
the target domain itself may prevent attaching xen-pciback to the
(PV) stubdomain, effectively breaking PCI passthrough.

Fix this by attaching pciback only to one domain: if PV stubdomain is in
use, let it be stubdomain (the commit prevents attaching device to target
HVM in this case); otherwise, attach it to the target domain.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Changes in v2:
 - previously called "libxl: attach xen-pciback only to PV domains"
 - instead of excluding all HVMs, change the condition to what actually
   matters here - check if stubdomain is in use; this way xen-pciback is
   always in use (either for the target domain, or it's stubdomain),
   fixing PCI reset by xen-pciback concerns
Changes in v3:
 - adjust commit message
Changes in v8:
 - rebase on staging
---
 tools/libxl/libxl_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2edff64..ac597a5 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1399,7 +1399,7 @@ out_no_irq:
         }
     }
 
-    if (!starting)
+    if (!starting && !libxl_get_stubdom_id(CTX, domid))
         rc = libxl__device_pci_add_xenstore(gc, domid, pcidev, starting);
     else
         rc = 0;
@@ -1697,7 +1697,7 @@ static void add_pcidevs_done(libxl__egc *egc, libxl__multidev *multidev,
     libxl_domid domid = apds->domid;
     libxl__ao_device *aodev = apds->outer_aodev;
 
-    if (d_config->num_pcidevs > 0) {
+    if (d_config->num_pcidevs > 0 && !libxl_get_stubdom_id(CTX, domid)) {
         rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
             d_config->num_pcidevs);
         if (rc < 0) {
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use

[Ian Jackson]

Marek Marczykowski-Górecki writes ("[PATCH v8 2/4] libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use"):
> HVM domains use IOMMU and device model assistance for communicating with
> PCI devices, xen-pcifront/pciback isn't directly needed by HVM domain.
> But pciback serve also second function - it reset the device when it is
> deassigned from the guest and for this reason pciback needs to be used
> with HVM domain too.
> When HVM domain has device model in stubdomain, attaching xen-pciback to
> the target domain itself may prevent attaching xen-pciback to the
> (PV) stubdomain, effectively breaking PCI passthrough.
> 
> Fix this by attaching pciback only to one domain: if PV stubdomain is in
> use, let it be stubdomain (the commit prevents attaching device to target
> HVM in this case); otherwise, attach it to the target domain.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

FTR, this had a trivial conflict with 7988902b
  libxl_pci: Don't ignore PCI PT error at guest creation
which I am fixing up.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 3/4] libxl: attach PCI device to qemu only after setting pciback/pcifront

[Marek Marczykowski-Górecki]

When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes in v2:
- Fixed code style since previous version.
Changes in v8:
- rebase on staging
- rework for async api
---
 tools/libxl/libxl_pci.c | 45 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index ac597a5..ba0287d 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1012,6 +1012,9 @@ typedef struct pci_add_state {
     bool starting;
     void (*callback)(libxl__egc *, struct pci_add_state *, int rc);
 
+    /* private to device_pci_add_stubdom_wait */
+    libxl__ev_devstate pciback_ds;
+
     /* private to do_pci_add */
     libxl__xswait_state xswait;
     libxl__ev_qmp qmp;
@@ -1487,6 +1490,10 @@ static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci *pcidev)
     return i != num;
 }
 
+static void device_pci_add_stubdom_wait(libxl__egc *egc,
+    pci_add_state *pas, int rc);
+static void device_pci_add_stubdom_ready(libxl__egc *egc,
+    libxl__ev_devstate* ds, int rc);
 static void device_pci_add_stubdom_done(libxl__egc *egc,
     pci_add_state *, int rc);
 static void device_pci_add_done(libxl__egc *egc,
@@ -1563,7 +1570,8 @@ void libxl__device_pci_add(libxl__egc *egc, uint32_t domid,
         GCNEW(pcidev_s);
         libxl_device_pci_init(pcidev_s);
         libxl_device_pci_copy(CTX, pcidev_s, pcidev);
-        pas->callback = device_pci_add_stubdom_done;
+        pas->callback = device_pci_add_stubdom_wait;
+
         do_pci_add(egc, stubdomid, pcidev_s, pas); /* must be last */
         return;
     }
@@ -1575,6 +1583,41 @@ out:
     device_pci_add_done(egc, pas, rc); /* must be last */
 }
 
+static void device_pci_add_stubdom_wait(libxl__egc *egc,
+                                        pci_add_state *pas,
+                                        int rc)
+{
+    libxl__ao_device *aodev = pas->aodev;
+    STATE_AO_GC(aodev->ao);
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int stubdomid = libxl_get_stubdom_id(ctx, pas->domid);
+    char *state_path;
+
+    if (rc) goto out;
+
+    /* Wait for the device actually being connected, otherwise device model
+     * running there will fail to find the device. */
+    state_path = libxl__sprintf(gc, "%s/backend/pci/%d/0/state",
+            libxl__xs_get_dompath(gc, 0), stubdomid);
+    rc = libxl__ev_devstate_wait(aodev->ao, &pas->pciback_ds,
+            device_pci_add_stubdom_ready,
+            state_path, XenbusStateConnected,
+            LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000);
+    if (rc) goto out;
+    return;
+out:
+    device_pci_add_done(egc, pas, rc); /* must be last */
+}
+
+static void device_pci_add_stubdom_ready(libxl__egc *egc,
+                                         libxl__ev_devstate* ds,
+                                         int rc)
+{
+    pci_add_state *pas = CONTAINER_OF(ds, *pas, pciback_ds);
+
+    device_pci_add_stubdom_done(egc, pas, rc); /* must be last */
+}
+
 static void device_pci_add_stubdom_done(libxl__egc *egc,
                                         pci_add_state *pas,
                                         int rc)
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 3/4] libxl: attach PCI device to qemu only after setting pciback/pcifront

[Anthony PERARD]

On Sat, Sep 28, 2019 at 04:20:36PM +0200, Marek Marczykowski-Górecki wrote:
> When qemu is running in stubdomain, handling "pci-ins" command will fail
> if pcifront is not initialized already. Fix this by sending such command
> only after confirming that pciback/front is running.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Changes in v2:
> - Fixed code style since previous version.
> Changes in v8:
> - rebase on staging
> - rework for async api

The patch looks good, I only have coding style comments. With those
addressed:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>


> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index ac597a5..ba0287d 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1487,6 +1490,10 @@ static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci *pcidev)
>      return i != num;
>  }
>  
> +static void device_pci_add_stubdom_wait(libxl__egc *egc,
> +    pci_add_state *pas, int rc);
> +static void device_pci_add_stubdom_ready(libxl__egc *egc,
> +    libxl__ev_devstate* ds, int rc);

s/* ds/ *ds/

>  static void device_pci_add_stubdom_done(libxl__egc *egc,
>      pci_add_state *, int rc);
>  static void device_pci_add_done(libxl__egc *egc,
> @@ -1575,6 +1583,41 @@ out:
>      device_pci_add_done(egc, pas, rc); /* must be last */
>  }
>  
> +static void device_pci_add_stubdom_wait(libxl__egc *egc,
> +                                        pci_add_state *pas,
> +                                        int rc)
> +{
> +    libxl__ao_device *aodev = pas->aodev;
> +    STATE_AO_GC(aodev->ao);
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    int stubdomid = libxl_get_stubdom_id(ctx, pas->domid);

You can use `CTX' instead of declaring a local `ctx'

> +    char *state_path;
> +
> +    if (rc) goto out;
> +
> +    /* Wait for the device actually being connected, otherwise device model
> +     * running there will fail to find the device. */
> +    state_path = libxl__sprintf(gc, "%s/backend/pci/%d/0/state",
> +            libxl__xs_get_dompath(gc, 0), stubdomid);

I think you could use libxl__domain_device_backend_path(gc, 0,
stubdomid, 0, LIBXL__DEVICE_KIND_PCI) here instead.
Or at least "GCSPRINTF(" instead of "libxl__sprintf(gc, "

> +    rc = libxl__ev_devstate_wait(aodev->ao, &pas->pciback_ds,

You can use `ao' instead of `aodev->ao'. `ao' is decleared by
STATE_AO_GC().

> +            device_pci_add_stubdom_ready,
> +            state_path, XenbusStateConnected,
> +            LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000);

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8.1 3/4] libxl: attach PCI device to qemu only after setting pciback/pcifront

[Marek Marczykowski-Górecki]

When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
---
Changes in v2:
- Fixed code style since previous version.
Changes in v8:
- rebase on staging
- rework for async api
Changes in v8.1:
- code style fixes
---
 tools/libxl/libxl_pci.c | 45 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index ac597a58fe..8dc352c961 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1012,6 +1012,9 @@ typedef struct pci_add_state {
     bool starting;
     void (*callback)(libxl__egc *, struct pci_add_state *, int rc);
 
+    /* private to device_pci_add_stubdom_wait */
+    libxl__ev_devstate pciback_ds;
+
     /* private to do_pci_add */
     libxl__xswait_state xswait;
     libxl__ev_qmp qmp;
@@ -1487,6 +1490,10 @@ static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci *pcidev)
     return i != num;
 }
 
+static void device_pci_add_stubdom_wait(libxl__egc *egc,
+    pci_add_state *pas, int rc);
+static void device_pci_add_stubdom_ready(libxl__egc *egc,
+    libxl__ev_devstate *ds, int rc);
 static void device_pci_add_stubdom_done(libxl__egc *egc,
     pci_add_state *, int rc);
 static void device_pci_add_done(libxl__egc *egc,
@@ -1563,7 +1570,8 @@ void libxl__device_pci_add(libxl__egc *egc, uint32_t domid,
         GCNEW(pcidev_s);
         libxl_device_pci_init(pcidev_s);
         libxl_device_pci_copy(CTX, pcidev_s, pcidev);
-        pas->callback = device_pci_add_stubdom_done;
+        pas->callback = device_pci_add_stubdom_wait;
+
         do_pci_add(egc, stubdomid, pcidev_s, pas); /* must be last */
         return;
     }
@@ -1575,6 +1583,41 @@ out:
     device_pci_add_done(egc, pas, rc); /* must be last */
 }
 
+static void device_pci_add_stubdom_wait(libxl__egc *egc,
+                                        pci_add_state *pas,
+                                        int rc)
+{
+    libxl__ao_device *aodev = pas->aodev;
+    STATE_AO_GC(aodev->ao);
+    int stubdomid = libxl_get_stubdom_id(CTX, pas->domid);
+    char *state_path;
+
+    if (rc) goto out;
+
+    /* Wait for the device actually being connected, otherwise device model
+     * running there will fail to find the device. */
+    state_path = GCSPRINTF("%s/state",
+            libxl__domain_device_backend_path(gc, 0, stubdomid, 0,
+                                              LIBXL__DEVICE_KIND_PCI));
+    rc = libxl__ev_devstate_wait(ao, &pas->pciback_ds,
+            device_pci_add_stubdom_ready,
+            state_path, XenbusStateConnected,
+            LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000);
+    if (rc) goto out;
+    return;
+out:
+    device_pci_add_done(egc, pas, rc); /* must be last */
+}
+
+static void device_pci_add_stubdom_ready(libxl__egc *egc,
+                                         libxl__ev_devstate *ds,
+                                         int rc)
+{
+    pci_add_state *pas = CONTAINER_OF(ds, *pas, pciback_ds);
+
+    device_pci_add_stubdom_done(egc, pas, rc); /* must be last */
+}
+
 static void device_pci_add_stubdom_done(libxl__egc *egc,
                                         pci_add_state *pas,
                                         int rc)
-- 
2.21.0

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 4/4] libxl: don't try to manipulate json config for stubdomain

[Marek Marczykowski-Górecki]

Stubdomain do not have it's own config file - its configuration is
derived from target domains. Do not try to manipulate it when attaching
PCI device.

This bug prevented starting HVM with stubdomain and PCI passthrough
device attached.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Changes in v3:
 - skip libxl__dm_check_start too, as stubdomain is guaranteed to be
   running at this stage already
 - do not init d_config at all, as it is used only for json manipulation
Changes in v4:
 - adjust comment style
Changes in v8:
 - rebase on staging
---
 tools/libxl/libxl_pci.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index ba0287d..dc96163 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -126,8 +126,11 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
     int rc;
     libxl_domain_config d_config;
     libxl__domain_userdata_lock *lock = NULL;
+    bool is_stubdomain = libxl_is_stubdom(CTX, domid, NULL);
 
-    libxl_domain_config_init(&d_config);
+    /* Stubdomain doesn't have own config. */
+    if (!is_stubdomain)
+        libxl_domain_config_init(&d_config);
 
     be_path = libxl__domain_device_backend_path(gc, 0, domid, 0,
                                                 LIBXL__DEVICE_KIND_PCI);
@@ -153,27 +156,35 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
     if (!starting)
         flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateReconfiguring));
 
-    lock = libxl__lock_domain_userdata(gc, domid);
-    if (!lock) {
-        rc = ERROR_LOCK_FAIL;
-        goto out;
-    }
+    /*
+     * Stubdomin config is derived from its target domain, it doesn't have
+     * its own file.
+     */
+    if (!is_stubdomain) {
+        lock = libxl__lock_domain_userdata(gc, domid);
+        if (!lock) {
+            rc = ERROR_LOCK_FAIL;
+            goto out;
+        }
 
-    rc = libxl__get_domain_configuration(gc, domid, &d_config);
-    if (rc) goto out;
+        rc = libxl__get_domain_configuration(gc, domid, &d_config);
+        if (rc) goto out;
 
-    device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
-                             pcidev);
+        device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
+                                 pcidev);
 
-    rc = libxl__dm_check_start(gc, &d_config, domid);
-    if (rc) goto out;
+        rc = libxl__dm_check_start(gc, &d_config, domid);
+        if (rc) goto out;
+    }
 
     for (;;) {
         rc = libxl__xs_transaction_start(gc, &t);
         if (rc) goto out;
 
-        rc = libxl__set_domain_configuration(gc, domid, &d_config);
-        if (rc) goto out;
+        if (lock) {
+            rc = libxl__set_domain_configuration(gc, domid, &d_config);
+            if (rc) goto out;
+        }
 
         libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc, back));
 
@@ -185,7 +196,8 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
 out:
     libxl__xs_transaction_abort(gc, &t);
     if (lock) libxl__unlock_domain_userdata(lock);
-    libxl_domain_config_dispose(&d_config);
+    if (!is_stubdomain)
+        libxl_domain_config_dispose(&d_config);
     return rc;
 }
 
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel