[PATCH v5 0/2] Containing AER unrecoverable errors

[PATCH v5 0/2] Containing AER unrecoverable errors

[Venu Busireddy]

This patch set is part of a set of patches that together allow containment
of unrecoverable AER errors from PCIe devices assigned to guests in
passthrough mode. The containment is achieved by forcibly removing the
erring PCIe device from the guest.

The original xen-pciback patch corresponding to this patch set is:
https://lists.xen.org/archives/html/xen-devel/2017-06/msg03274.html.
It will be reposted after this patch set is accepted.

Changes in v5:
  * v4 worked only in the case of guests created using 'xl' command.
    Enhanced the fix to work for guests created using libvirt too.

Changes in v4:
  * Made the following changes suggested by Wei Liu.
    - Combine multiple LIBXL_HAVE_* definitions into one.
    - Use libxl__calloc() instead of malloc().

Changes in v3:
  * Made the following changes suggested by Wei Liu.
    - Added LIBXL_HAVE macros to libxl.h.
    - Don't hard-code dom0's domid to 0. Instead, use libxl__get_domid().
    - Corrected comments.
  * Made the following changes based on comments from Ian Jackson.
    - Got rid of the global variable aer_watch.
    - Added documentation (comments in code) for the new API calls.
    - Removed the unnecessary writes to xenstore.

Changes in v2:
  - Instead of killing the guest and hiding the device, forcibly remove
    the device from the guest.

Venu Busireddy (2):
  libxl: Implement the handler to handle unrecoverable AER errors
  xl: Register the AER event handler that handles AER errors

 tools/libxl/libxl.h          |   7 +++
 tools/libxl/libxl_create.c   |  11 +++-
 tools/libxl/libxl_domain.c   |   1 +
 tools/libxl/libxl_event.h    |   7 +++
 tools/libxl/libxl_internal.h |   8 +++
 tools/libxl/libxl_pci.c      | 123 +++++++++++++++++++++++++++++++++++++++++++
 tools/xl/xl_vmcontrol.c      |  14 ++++-
 7 files changed, 168 insertions(+), 3 deletions(-)


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

[PATCH v5 1/2] libxl: Implement the handler to handle unrecoverable AER errors

[Venu Busireddy]

Implement the callback function to handle unrecoverable AER errors, and
also the public APIs that can be used to register/unregister the handler.
When an AER error occurs, the handler will forcibly remove the erring
PCIe device from the guest.

Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
---
 tools/libxl/libxl.h          |   7 +++
 tools/libxl/libxl_event.h    |   7 +++
 tools/libxl/libxl_internal.h |   8 +++
 tools/libxl/libxl_pci.c      | 123 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index eca0ea2c50..99a3c8ae1f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1120,6 +1120,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_PV_SHIM 1
 
+/* LIBXL_HAVE_AER_EVENTS_HANDLER
+ *
+ * If this is defined, libxl has the library functions called
+ * libxl_reg_aer_events_handler and libxl_unreg_aer_events_handler.
+ */
+#define LIBXL_HAVE_AER_EVENTS_HANDLER 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
index 1ea789e231..63c29ae800 100644
--- a/tools/libxl/libxl_event.h
+++ b/tools/libxl/libxl_event.h
@@ -184,6 +184,13 @@ void libxl_evdisable_domain_death(libxl_ctx *ctx, libxl_evgen_domain_death*);
    * may generate only a DEATH event.
    */
 
+typedef struct libxl__aer_watch libxl_aer_watch;
+int libxl_reg_aer_events_handler(libxl_ctx *, uint32_t);
+  /*
+   * Registers a handler to handle the occurrence of unrecoverable AER errors.
+   */
+void libxl_unreg_aer_events_handler(libxl_ctx *, uint32_t);
+
 typedef struct libxl__evgen_disk_eject libxl_evgen_disk_eject;
 int libxl_evenable_disk_eject(libxl_ctx *ctx, uint32_t domid, const char *vdev,
                         libxl_ev_user, libxl_evgen_disk_eject **evgen_out);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 506687fbe9..7972490050 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -356,6 +356,14 @@ struct libxl__ev_child {
     LIBXL_LIST_ENTRY(struct libxl__ev_child) entry;
 };
 
+/*
+ * Structure used for AER event handling.
+ */
+struct libxl__aer_watch {
+    uint32_t domid;
+    libxl__ev_xswatch watch;
+    struct libxl__aer_watch *next;
+};
 
 /*
  * evgen structures, which are the state we use for generating
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4755a0c93c..c121c9f8cc 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1686,6 +1686,129 @@ static int libxl_device_pci_compare(libxl_device_pci *d1,
     return COMPARE_PCI(d1, d2);
 }
 
+static void aer_backend_watch_callback(libxl__egc *egc,
+                                       libxl__ev_xswatch *watch,
+                                       const char *watch_path,
+                                       const char *event_path)
+{
+    EGC_GC;
+    libxl_aer_watch *aer_ws = CONTAINER_OF(watch, *aer_ws, watch);
+    int rc;
+    uint32_t dom, bus, dev, fn;
+    uint32_t domid = aer_ws->domid;
+    char *p, *path;
+    const char *aerFailedSBDF;
+    libxl_device_pci pcidev;
+
+    /* Extract the backend directory. */
+    path = libxl__strdup(gc, event_path);
+    p = strrchr(path, '/');
+    if ((p == NULL) || (strcmp(p, "/aerFailedSBDF") != 0))
+        return;
+    /* Truncate the string so it points to the backend directory. */
+    *p = '\0';
+
+    /* Fetch the value of the failed PCI device. */
+    rc = libxl__xs_read_checked(gc, XBT_NULL,
+            GCSPRINTF("%s/aerFailedSBDF", path), &aerFailedSBDF);
+    if (rc || !aerFailedSBDF)
+        return;
+    LOGD(ERROR, domid, " aerFailedSBDF = %s", aerFailedSBDF);
+    sscanf(aerFailedSBDF, "%x:%x:%x.%x", &dom, &bus, &dev, &fn);
+
+    libxl_device_pci_init(&pcidev);
+    pcidev_struct_fill(&pcidev, dom, bus, dev, fn, 0);
+    /* Forcibly remove the device from the guest */
+    rc = libxl__device_pci_remove_common(gc, domid, &pcidev, 1);
+    if (rc)
+        LOGD(ERROR, domid, " libxl__device_pci_remove_common() failed, rc=x%x",
+                (unsigned int)rc);
+
+    return;
+}
+
+static libxl_aer_watch *manage_aer_ws_list(libxl_aer_watch *in, uint32_t domid)
+{
+    static libxl_aer_watch *aer_ws = NULL;
+    libxl_aer_watch *iter, *prev = NULL;
+
+    if (in) {
+        if (aer_ws)
+            in->next = aer_ws;
+        iter = aer_ws = in;
+    } else {
+        iter = aer_ws;
+        while (iter) {
+            if (iter->domid == domid) {
+                if (prev)
+                    prev->next = iter->next;
+                else
+                    aer_ws = iter->next;
+                break;
+            }
+            prev = iter;
+            iter = iter->next;
+        }
+    }
+    return iter;
+}
+
+static void store_aer_ws(libxl_aer_watch *aer_ws)
+{
+    manage_aer_ws_list(aer_ws, 0);
+    return;
+}
+
+static libxl_aer_watch *retrieve_aer_ws(uint32_t domid)
+{
+    return manage_aer_ws_list(NULL, domid);
+}
+
+int libxl_reg_aer_events_handler(libxl_ctx *ctx, uint32_t domid)
+{
+    int rc = 0;
+    char *be_path;
+    uint32_t pciback_domid;
+    libxl_aer_watch *aer_ws;
+    GC_INIT(ctx);
+
+    rc = libxl__get_domid(gc, (uint32_t *)(&pciback_domid));
+    if (rc) {
+        LOGD(ERROR, domid, " libxl__get_domid() failed, rc = %d", rc);
+        goto out;
+    }
+
+    aer_ws = libxl__calloc(NOGC, 1, sizeof(libxl_aer_watch));
+    aer_ws->domid = domid;
+    aer_ws->next = NULL;
+    store_aer_ws(aer_ws);
+    be_path = GCSPRINTF("/local/domain/%u/backend/pci/%u/%u/%s",
+            pciback_domid, domid, pciback_domid, "aerFailedSBDF");
+    rc = libxl__ev_xswatch_register(gc, &aer_ws->watch,
+            aer_backend_watch_callback, be_path);
+
+out:
+    GC_FREE;
+    return rc;
+}
+
+void libxl_unreg_aer_events_handler(libxl_ctx *ctx, uint32_t domid)
+{
+    GC_INIT(ctx);
+    libxl_aer_watch *aer_ws;
+
+    aer_ws = retrieve_aer_ws(domid);
+    if (!aer_ws)
+        goto out;
+
+    libxl__ev_xswatch_deregister(gc, &aer_ws->watch);
+    free(aer_ws);
+
+out:
+    GC_FREE;
+    return;
+}
+
 #define libxl__device_pci_update_devid NULL
 
 DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci, PCI);

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

[PATCH v5 2/2] xl: Register the AER event handler that handles AER errors

[Venu Busireddy]

When a guest is created, register the AER event handler to handle the
AER errors. When an AER error occurs, the handler will forcibly remove
the erring PCIe device from the guest.

Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
Signed-off-by: Wim Ten Have <wim.ten.have@oracle.com>
---
 tools/libxl/libxl_create.c | 11 +++++++++--
 tools/libxl/libxl_domain.c |  1 +
 tools/xl/xl_vmcontrol.c    | 14 +++++++++++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index c498135246..2d247da5f0 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1663,7 +1663,7 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
 {
     AO_CREATE(ctx, 0, ao_how);
     libxl__app_domain_create_state *cdcs;
-    int rc;
+    int rc, ao_rc;
 
     GCNEW(cdcs);
     cdcs->dcs.ao = ao;
@@ -1698,7 +1698,14 @@ static int do_domain_create(libxl_ctx *ctx, libxl_domain_config *d_config,
 
     initiate_domain_create(egc, &cdcs->dcs);
 
-    return AO_INPROGRESS;
+    ao_rc = AO_INPROGRESS;
+    rc = libxl_reg_aer_events_handler(ctx, *domid);
+    if (rc) {
+        /* Log the error, and move on... */
+        LOGD(ERROR, *domid,
+                "libxl_reg_aer_events_handler() failed, rc = %d", rc);
+    }
+    return ao_rc;
 
  out_err:
     return AO_CREATE_FAIL(rc);
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 13b1c73d40..b8fb5e0349 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -906,6 +906,7 @@ void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
     STATE_AO_GC(dds->ao);
     uint32_t stubdomid = libxl_get_stubdom_id(CTX, dds->domid);
 
+    libxl_unreg_aer_events_handler(CTX, dds->domid);
     if (stubdomid) {
         dds->stubdom.ao = ao;
         dds->stubdom.domid = stubdomid;
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 89c2b25ded..5bf415fa6e 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -945,8 +945,11 @@ start:
         libxl_domain_unpause(ctx, domid);
 
     ret = domid; /* caller gets success in parent */
-    if (!daemonize && !monitor)
+    if (!daemonize && !monitor) {
+        /* Unregister aer events handler before returning/exiting */
+        libxl_unreg_aer_events_handler(ctx, domid);
         goto out;
+    }
 
     if (dom_info->vnc)
         autoconnect_vncviewer(domid, vncautopass);
@@ -958,9 +961,17 @@ start:
         ret = do_daemonize(name, NULL);
         free(name);
         if (ret) {
+            /* Unregister aer events handler before returning/exiting */
+            libxl_unreg_aer_events_handler(ctx, domid);
             ret = (ret == 1) ? domid : ret;
             goto out;
         }
+        /* Child has new ctx. Re-register the events handler in child's ctx */
+        ret = libxl_reg_aer_events_handler(ctx, domid);
+        if (ret) {
+            /* Log the error, and move on... */
+            LOG("libxl_reg_aer_events_handler() failed, ret = %d", ret);
+        }
         need_daemon = 0;
     }
     LOG("Waiting for domain %s (domid %u) to die [pid %ld]",
@@ -1059,6 +1070,7 @@ start:
 
         case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
             LOG("Domain %u has been destroyed.", domid);
+            libxl_unreg_aer_events_handler(ctx, domid);
             libxl_event_free(ctx, event);
             ret = 0;
             goto out;

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

Re: [PATCH v5 0/2] Containing AER unrecoverable errors

[Konrad Rzeszutek Wilk]

On Tue, Mar 13, 2018 at 01:43:39PM -0500, Venu Busireddy wrote:
> This patch set is part of a set of patches that together allow containment
> of unrecoverable AER errors from PCIe devices assigned to guests in
> passthrough mode. The containment is achieved by forcibly removing the
> erring PCIe device from the guest.
> 
> The original xen-pciback patch corresponding to this patch set is:
> https://lists.xen.org/archives/html/xen-devel/2017-06/msg03274.html.
> It will be reposted after this patch set is accepted.
> 
> Changes in v5:
>   * v4 worked only in the case of guests created using 'xl' command.
>     Enhanced the fix to work for guests created using libvirt too.

Are there any libvirt patches for this?
> 
> Changes in v4:
>   * Made the following changes suggested by Wei Liu.
>     - Combine multiple LIBXL_HAVE_* definitions into one.
>     - Use libxl__calloc() instead of malloc().
> 
> Changes in v3:
>   * Made the following changes suggested by Wei Liu.
>     - Added LIBXL_HAVE macros to libxl.h.
>     - Don't hard-code dom0's domid to 0. Instead, use libxl__get_domid().
>     - Corrected comments.
>   * Made the following changes based on comments from Ian Jackson.
>     - Got rid of the global variable aer_watch.
>     - Added documentation (comments in code) for the new API calls.
>     - Removed the unnecessary writes to xenstore.
> 
> Changes in v2:
>   - Instead of killing the guest and hiding the device, forcibly remove
>     the device from the guest.
> 
> Venu Busireddy (2):
>   libxl: Implement the handler to handle unrecoverable AER errors
>   xl: Register the AER event handler that handles AER errors
> 
>  tools/libxl/libxl.h          |   7 +++
>  tools/libxl/libxl_create.c   |  11 +++-
>  tools/libxl/libxl_domain.c   |   1 +
>  tools/libxl/libxl_event.h    |   7 +++
>  tools/libxl/libxl_internal.h |   8 +++
>  tools/libxl/libxl_pci.c      | 123 +++++++++++++++++++++++++++++++++++++++++++
>  tools/xl/xl_vmcontrol.c      |  14 ++++-
>  7 files changed, 168 insertions(+), 3 deletions(-)
> 

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

Re: [PATCH v5 0/2] Containing AER unrecoverable errors

[Venu Busireddy]

On 2018-03-16 10:32:10 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 13, 2018 at 01:43:39PM -0500, Venu Busireddy wrote:
> > This patch set is part of a set of patches that together allow containment
> > of unrecoverable AER errors from PCIe devices assigned to guests in
> > passthrough mode. The containment is achieved by forcibly removing the
> > erring PCIe device from the guest.
> > 
> > The original xen-pciback patch corresponding to this patch set is:
> > https://lists.xen.org/archives/html/xen-devel/2017-06/msg03274.html.
> > It will be reposted after this patch set is accepted.
> > 
> > Changes in v5:
> >   * v4 worked only in the case of guests created using 'xl' command.
> >     Enhanced the fix to work for guests created using libvirt too.
> 
> Are there any libvirt patches for this?

No. There is no change to libvirt itself. All the changes are contained
within xl and xen-pciback.

Venu

> > 
> > Changes in v4:
> >   * Made the following changes suggested by Wei Liu.
> >     - Combine multiple LIBXL_HAVE_* definitions into one.
> >     - Use libxl__calloc() instead of malloc().
> > 
> > Changes in v3:
> >   * Made the following changes suggested by Wei Liu.
> >     - Added LIBXL_HAVE macros to libxl.h.
> >     - Don't hard-code dom0's domid to 0. Instead, use libxl__get_domid().
> >     - Corrected comments.
> >   * Made the following changes based on comments from Ian Jackson.
> >     - Got rid of the global variable aer_watch.
> >     - Added documentation (comments in code) for the new API calls.
> >     - Removed the unnecessary writes to xenstore.
> > 
> > Changes in v2:
> >   - Instead of killing the guest and hiding the device, forcibly remove
> >     the device from the guest.
> > 
> > Venu Busireddy (2):
> >   libxl: Implement the handler to handle unrecoverable AER errors
> >   xl: Register the AER event handler that handles AER errors
> > 
> >  tools/libxl/libxl.h          |   7 +++
> >  tools/libxl/libxl_create.c   |  11 +++-
> >  tools/libxl/libxl_domain.c   |   1 +
> >  tools/libxl/libxl_event.h    |   7 +++
> >  tools/libxl/libxl_internal.h |   8 +++
> >  tools/libxl/libxl_pci.c      | 123 +++++++++++++++++++++++++++++++++++++++++++
> >  tools/xl/xl_vmcontrol.c      |  14 ++++-
> >  7 files changed, 168 insertions(+), 3 deletions(-)
> > 

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

Re: [PATCH v5 0/2] Containing AER unrecoverable errors

[Wim ten Have]

On Fri, 16 Mar 2018 10:32:10 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Tue, Mar 13, 2018 at 01:43:39PM -0500, Venu Busireddy wrote:
> > This patch set is part of a set of patches that together allow containment
> > of unrecoverable AER errors from PCIe devices assigned to guests in
> > passthrough mode. The containment is achieved by forcibly removing the
> > erring PCIe device from the guest.
> > 
> > The original xen-pciback patch corresponding to this patch set is:
> > https://lists.xen.org/archives/html/xen-devel/2017-06/msg03274.html.
> > It will be reposted after this patch set is accepted.
> > 
> > Changes in v5:
> >   * v4 worked only in the case of guests created using 'xl' command.
> >     Enhanced the fix to work for guests created using libvirt too.  
> 
> Are there any libvirt patches for this?

  No, the change to libxl (xenlight) Xen driver adds the support to
  libvirt when Xen guests are in effect per the libvirt loaded DSO.

> > Changes in v4:
> >   * Made the following changes suggested by Wei Liu.
> >     - Combine multiple LIBXL_HAVE_* definitions into one.
> >     - Use libxl__calloc() instead of malloc().
> > 
> > Changes in v3:
> >   * Made the following changes suggested by Wei Liu.
> >     - Added LIBXL_HAVE macros to libxl.h.
> >     - Don't hard-code dom0's domid to 0. Instead, use libxl__get_domid().
> >     - Corrected comments.
> >   * Made the following changes based on comments from Ian Jackson.
> >     - Got rid of the global variable aer_watch.
> >     - Added documentation (comments in code) for the new API calls.
> >     - Removed the unnecessary writes to xenstore.
> > 
> > Changes in v2:
> >   - Instead of killing the guest and hiding the device, forcibly remove
> >     the device from the guest.
> > 
> > Venu Busireddy (2):
> >   libxl: Implement the handler to handle unrecoverable AER errors
> >   xl: Register the AER event handler that handles AER errors
> > 
> >  tools/libxl/libxl.h          |   7 +++
> >  tools/libxl/libxl_create.c   |  11 +++-
> >  tools/libxl/libxl_domain.c   |   1 +
> >  tools/libxl/libxl_event.h    |   7 +++
> >  tools/libxl/libxl_internal.h |   8 +++
> >  tools/libxl/libxl_pci.c      | 123 +++++++++++++++++++++++++++++++++++++++++++
> >  tools/xl/xl_vmcontrol.c      |  14 ++++-
> >  7 files changed, 168 insertions(+), 3 deletions(-)
> >   


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