[Xen-devel] [Xen-unstable] XenbusState signaling with regard to pci-detach on HVM guests

[Sander Eikelenboom <linux@xxxxxxxxxxxxxx>]

Hi Konrad / Ian,

Today i thought of doing some DIY .. and took the problem i encountered:

"other issue with removing pci devices passed through to HVM guests related to 
the signaling via xenstore" described in:
http://lists.xen.org/archives/html/xen-devel/2014-07/msg01875.html

It seems there are quite some (little) issues. I got things working for my 
specific case, however how to get things done in a backwards compatible matter 
(for multiple kernel version and the three variants of pv_guest, qemu_trad and 
qemu_xen is still a question :-).

But one of the main questions is .. 
- Is there some document describing the various states in more detail then in 
  xenbus.h ?
- Are there rules with regard to backend, toolstack(libxl) and frontend as to 
  who does what in changing states ?

For example when domains are setup and running with pci-passthrough devices, if
you list the devices with 'xenstore-ls' these are still in 
XenbusStateInitialised(3) instead of XenbusStateConnected(4), seems incorrect 
to 
me.


But back to the problem i was trying to solve:

Here are both a crude patch for xen/libxl and pciback that change the signaling 
and now allow the removal of an individual pci device from a HVM guest.
I hope i took the right states based on the descriptions and code i found.

What these patches try to do on remove of a pci device (and when everything 
goes well) is:
- after libxl has removed the device from the guest with a qmp command
- let libxl set the pci-device-state to XenbusStateClosing AND 
  set pci-root-state to XenbusStateReconfiguring
- libxl waits for the pci-root-state to change to XenbusStateReconfigured

- pciback has a watch on the pci-root-state and removes the device and resets 
in 
  pci config space, after that it sets the pci-device-state to 
XenbusStateClosed AND
  set pci-root-state to XenbusStateReconfigured

- libxl now continues from the on XenbusStateReconfigured, cleans up the 
xenstore entries for 
  the device and finally sets pci-root-state back to XenbusStateConnected

However the problem now seems to be on shutdown .. where removing multiple 
passed through device mess up the signalling on pci-root causing the 
libxl__wait_for_backend to last forever *sigh*
                                            
But perhaps someone has suggestions ...

(patches are also attached to prevent mail mangling)

--
Sander


LIBXL patch:

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2782d0e..b54531c 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -204,6 +204,23 @@ retry_transaction:
         }
     }
 
+   if (domtype == LIBXL_DOMAIN_TYPE_HVM) {
+        if (libxl__wait_for_backend(gc, be_path, "8") < 0) {
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "pci backend at %s is not 
ready", be_path);
+            return ERROR_FAIL;
+        } else {
+            if (atoi(libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, 
"%s/state-%d", be_path, i))) != 6 ) {
+                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "pci backend at %s didn't 
set device into closing state: %d",
+                           libxl__sprintf(gc, "%s/state-%d", be_path, i), 
+                           atoi(libxl__xs_read(gc, XBT_NULL, 
libxl__sprintf(gc, "%s/state-%d", be_path, i))));
+                return ERROR_FAIL;
+            } else {
+                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "pci backend at %s has set 
device into XenbusStateClosed, "
+                           "continue with removing xenstore entry", 
libxl__sprintf(gc, "%s/state-%d", be_path, i));
+            }
+        }
+    }
+
 retry_transaction2:
     t = xs_transaction_start(ctx->xsh);
     xs_rm(ctx->xsh, t, libxl__sprintf(gc, "%s/state-%d", be_path, i));
@@ -245,6 +262,8 @@ retry_transaction2:
             xs_rm(ctx->xsh, t, tmppath);
         }
     }
+    /* switch guest pci root back from state XenbusStateReconfigured to state 
XenbusStateConnected after */
+    xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/state", be_path), "4", 
strlen("4"));
     if (!xs_transaction_end(ctx->xsh, t, 0))
         if (errno == EAGAIN)
             goto retry_transaction2;





PCIBACK patch:

diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index f09081c..ecb00bf 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -4,6 +4,8 @@
  *   Author: Ryan Wilson <hap9@xxxxxxxxxxxxxx>
  */
 
+#define DEBUG
+
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/module.h>
@@ -485,9 +487,21 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device 
*pdev)
 
                        err = xen_pcibk_remove_device(pdev, domain, bus, slot,
                                                    func);
-                       if (err)
+                       if (err){
+                               xenbus_dev_fatal(pdev->xdev, err, "Error 
removing pci device ");
                                goto out;
-
+                       } else {
+                               /* device removed succesfully,inform toolstack
+                                * by switching state of device to 
XenbusStateClosed 
+                                */
+                               err = xenbus_printf(XBT_NIL, 
pdev->xdev->nodename, state_str,
+                                                   "%d", XenbusStateClosed);
+                               if (err) {
+                                       xenbus_dev_fatal(pdev->xdev, err, 
+                                                        "Error switching state 
of device to XenbusStateClosed");
+                                       goto out;
+                               }
+                       }
                        /* TODO: If at some point we implement support for pci
                         * root hot-remove on pcifront side, we'll need to
                         * remove unnecessary xenstore nodes of pci roots here.
@@ -664,6 +678,10 @@ static void xen_pcibk_be_watch(struct xenbus_watch *watch,
                xen_pcibk_setup_backend(pdev);
                break;
 
+       case XenbusStateReconfiguring:
+               xen_pcibk_reconfigure(pdev);
+               break;
+
        default:
                break;
        }

Attachment: patch-libxl-removal-individual-pci-devices.diff
Description: Binary data

Attachment: patch-pciback-removal-individual-pci-devices.diff
Description: Binary data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel