diff mbox

[v3,1/2] libxl: Implement the handler to handle unrecoverable AER errors

Message ID 20170807235457.3943-2-venu.busireddy@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Venu Busireddy Aug. 7, 2017, 11:54 p.m. UTC
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          | 14 +++++++
 tools/libxl/libxl_event.h    | 13 +++++++
 tools/libxl/libxl_internal.h |  7 ++++
 tools/libxl/libxl_pci.c      | 90 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+)

Comments

Wei Liu Aug. 8, 2017, 2:33 p.m. UTC | #1
On Mon, Aug 07, 2017 at 06:54:56PM -0500, Venu Busireddy wrote:
> 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          | 14 +++++++
>  tools/libxl/libxl_event.h    | 13 +++++++
>  tools/libxl/libxl_internal.h |  7 ++++
>  tools/libxl/libxl_pci.c      | 90 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 124 insertions(+)
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 7cf0f31..c5af0aa 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -1044,6 +1044,20 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
>   */
>  #define LIBXL_HAVE_QED 1
>  
> +/* LIBXL_HAVE_REG_AER_EVENTS_HANDLER
> + *
> + * If it is defined, libxl has a library function called
> + * libxl_reg_aer_events_handler.
> + */
> +#define LIBXL_HAVE_REG_AER_EVENTS_HANDLER 1
> +
> +/* LIBXL_HAVE_UNREG_AER_EVENTS_HANDLER
> + *
> + * If it is defined, libxl has a library function called
> + * libxl_unreg_aer_events_handler.
> + */
> +#define LIBXL_HAVE_UNREG_AER_EVENTS_HANDLER 1
> +

You can consolidate both into

LIBLX_HAVE_AER_EVENTS_HANDLER

>  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 1ea789e..1aea906 100644
> --- a/tools/libxl/libxl_event.h
> +++ b/tools/libxl/libxl_event.h
> @@ -184,6 +184,19 @@ 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, libxl_aer_watch **)
> +                        LIBXL_EXTERNAL_CALLERS_ONLY;
> +  /*
> +   * Registers a handler to handle the occurrence of unrecoverable AER errors.
> +   * This function depends on the calling application running the libxl's
> +   * internal event loop. Toolstacks that do not use libxl's internal
> +   * event loop must arrange to have their own event loop created and enter
> +   * libxl (say, call libxl_event_wait()), to enable the event to be processed.
> +   */
> +void libxl_unreg_aer_events_handler(libxl_ctx *, uint32_t, libxl_aer_watch *)
> +                        LIBXL_EXTERNAL_CALLERS_ONLY;
> +
>  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 afe6652..2b74286 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -352,6 +352,13 @@ 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;
> +};
>  
>  /*
>   * 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 65ad5e5..feedf27 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1678,6 +1678,96 @@ 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;
> +    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;
> +}
> +
> +int libxl_reg_aer_events_handler(libxl_ctx *ctx,
> +                                 uint32_t domid,
> +                                 libxl_aer_watch **aer_ws_out)

Afaict libxl_aer_watch is an opaque type to external caller, so this
won't work, right?

> +{
> +    int rc = 0;
> +    int dom0_domid;

uint32_t pciback_domid;

> +    char *be_path;
> +    libxl_aer_watch *aer_ws = NULL;
> +    GC_INIT(ctx);
> +
> +    *aer_ws_out = NULL;
> +
> +    rc = libxl__get_domid(gc, (uint32_t *)(&dom0_domid));
> +    if (rc) {
> +        LOGD(ERROR, domid, " libxl__get_domid() failed, rc = %d", rc);
> +        goto out;
> +    }
> +
> +    aer_ws = malloc(sizeof(libxl_aer_watch));

libxl__calloc(NOGC, ...);

And then you can skip the check and memset.

> +    if (!aer_ws) {
> +        rc = ERROR_NOMEM;
> +        goto out;
> +    }
> +    memset(aer_ws, 0, sizeof(libxl_aer_watch));
> +
> +    aer_ws->domid = domid;
> +    be_path = GCSPRINTF("/local/domain/%d/backend/pci/%u/%d/%s",
> +            dom0_domid, domid, dom0_domid, "aerFailedSBDF");

Use %u here.

> +    rc = libxl__ev_xswatch_register(gc, &aer_ws->watch,
> +            aer_backend_watch_callback, be_path);
> +    *aer_ws_out = aer_ws;
> +
> +out:
> +    GC_FREE;
> +    return rc;
> +}
> +
> +void libxl_unreg_aer_events_handler(libxl_ctx *ctx,
> +                                    uint32_t domid,
> +                                    libxl_aer_watch *aer_ws)
> +{
> +    GC_INIT(ctx);
> +
> +    libxl__ev_xswatch_deregister(gc, &aer_ws->watch);
> +
> +    free(aer_ws);
> +    GC_FREE;
> +    return;
> +}

I think a bigger question is whether you agree with Ian's comments
regarding API design and whether you have more questions?
Wei Liu Aug. 8, 2017, 2:40 p.m. UTC | #2
On Tue, Aug 08, 2017 at 03:33:01PM +0100, Wei Liu wrote:
> > +
> > +int libxl_reg_aer_events_handler(libxl_ctx *ctx,
> > +                                 uint32_t domid,
> > +                                 libxl_aer_watch **aer_ws_out)
> 
> Afaict libxl_aer_watch is an opaque type to external caller, so this
> won't work, right?
> 

Oh, it is a pointer to a pointer, so the storage size is known.
Venu Busireddy Aug. 8, 2017, 2:51 p.m. UTC | #3
On 2017-08-08 15:33:01 +0100, Wei Liu wrote:
> On Mon, Aug 07, 2017 at 06:54:56PM -0500, Venu Busireddy wrote:
> > 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          | 14 +++++++
> >  tools/libxl/libxl_event.h    | 13 +++++++
> >  tools/libxl/libxl_internal.h |  7 ++++
> >  tools/libxl/libxl_pci.c      | 90 ++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 124 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 7cf0f31..c5af0aa 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -1044,6 +1044,20 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
> >   */
> >  #define LIBXL_HAVE_QED 1
> >  
> > +/* LIBXL_HAVE_REG_AER_EVENTS_HANDLER
> > + *
> > + * If it is defined, libxl has a library function called
> > + * libxl_reg_aer_events_handler.
> > + */
> > +#define LIBXL_HAVE_REG_AER_EVENTS_HANDLER 1
> > +
> > +/* LIBXL_HAVE_UNREG_AER_EVENTS_HANDLER
> > + *
> > + * If it is defined, libxl has a library function called
> > + * libxl_unreg_aer_events_handler.
> > + */
> > +#define LIBXL_HAVE_UNREG_AER_EVENTS_HANDLER 1
> > +
> 
> You can consolidate both into
> 
> LIBLX_HAVE_AER_EVENTS_HANDLER
> 
> >  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 1ea789e..1aea906 100644
> > --- a/tools/libxl/libxl_event.h
> > +++ b/tools/libxl/libxl_event.h
> > @@ -184,6 +184,19 @@ 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, libxl_aer_watch **)
> > +                        LIBXL_EXTERNAL_CALLERS_ONLY;
> > +  /*
> > +   * Registers a handler to handle the occurrence of unrecoverable AER errors.
> > +   * This function depends on the calling application running the libxl's
> > +   * internal event loop. Toolstacks that do not use libxl's internal
> > +   * event loop must arrange to have their own event loop created and enter
> > +   * libxl (say, call libxl_event_wait()), to enable the event to be processed.
> > +   */
> > +void libxl_unreg_aer_events_handler(libxl_ctx *, uint32_t, libxl_aer_watch *)
> > +                        LIBXL_EXTERNAL_CALLERS_ONLY;
> > +
> >  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 afe6652..2b74286 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -352,6 +352,13 @@ 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;
> > +};
> >  
> >  /*
> >   * 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 65ad5e5..feedf27 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -1678,6 +1678,96 @@ 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;
> > +    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;
> > +}
> > +
> > +int libxl_reg_aer_events_handler(libxl_ctx *ctx,
> > +                                 uint32_t domid,
> > +                                 libxl_aer_watch **aer_ws_out)
> 
> Afaict libxl_aer_watch is an opaque type to external caller, so this
> won't work, right?
> 
> > +{
> > +    int rc = 0;
> > +    int dom0_domid;
> 
> uint32_t pciback_domid;
> 
> > +    char *be_path;
> > +    libxl_aer_watch *aer_ws = NULL;
> > +    GC_INIT(ctx);
> > +
> > +    *aer_ws_out = NULL;
> > +
> > +    rc = libxl__get_domid(gc, (uint32_t *)(&dom0_domid));
> > +    if (rc) {
> > +        LOGD(ERROR, domid, " libxl__get_domid() failed, rc = %d", rc);
> > +        goto out;
> > +    }
> > +
> > +    aer_ws = malloc(sizeof(libxl_aer_watch));
> 
> libxl__calloc(NOGC, ...);
> 
> And then you can skip the check and memset.
> 
> > +    if (!aer_ws) {
> > +        rc = ERROR_NOMEM;
> > +        goto out;
> > +    }
> > +    memset(aer_ws, 0, sizeof(libxl_aer_watch));
> > +
> > +    aer_ws->domid = domid;
> > +    be_path = GCSPRINTF("/local/domain/%d/backend/pci/%u/%d/%s",
> > +            dom0_domid, domid, dom0_domid, "aerFailedSBDF");
> 
> Use %u here.
> 
> > +    rc = libxl__ev_xswatch_register(gc, &aer_ws->watch,
> > +            aer_backend_watch_callback, be_path);
> > +    *aer_ws_out = aer_ws;
> > +
> > +out:
> > +    GC_FREE;
> > +    return rc;
> > +}
> > +
> > +void libxl_unreg_aer_events_handler(libxl_ctx *ctx,
> > +                                    uint32_t domid,
> > +                                    libxl_aer_watch *aer_ws)
> > +{
> > +    GC_INIT(ctx);
> > +
> > +    libxl__ev_xswatch_deregister(gc, &aer_ws->watch);
> > +
> > +    free(aer_ws);
> > +    GC_FREE;
> > +    return;
> > +}

I will implement all your above suggestions in v4.

> I think a bigger question is whether you agree with Ian's comments
> regarding API design and whether you have more questions?

Ian suggested that I document the use of the API (about the event loop),
and I believe I addressed it. I don't have any more questions. Just
waiting for Ian's "Ack", or more comments.

Venu
Wei Liu Aug. 8, 2017, 3:12 p.m. UTC | #4
On Tue, Aug 08, 2017 at 09:51:36AM -0500, Venu Busireddy wrote:
> I will implement all your above suggestions in v4.
> 
> > I think a bigger question is whether you agree with Ian's comments
> > regarding API design and whether you have more questions?
> 
> Ian suggested that I document the use of the API (about the event loop),
> and I believe I addressed it. I don't have any more questions. Just
> waiting for Ian's "Ack", or more comments.
> 

Ian is currently away and won't be back till August 28 iirc. I suggest
you wait for him to comment. I am not quite sure what he asked for.
Ian Jackson Sept. 21, 2017, 5:12 p.m. UTC | #5
Venu Busireddy writes ("Re: [PATCH v3 1/2] libxl: Implement the handler to handle unrecoverable AER errors"):
> On 2017-08-08 15:33:01 +0100, Wei Liu wrote:
> > I think a bigger question is whether you agree with Ian's comments
> > regarding API design and whether you have more questions?
> 
> Ian suggested that I document the use of the API (about the event loop),
> and I believe I addressed it. I don't have any more questions. Just
> waiting for Ian's "Ack", or more comments.

I'm afraid that I still have reservations about the design questions.
Evidently I didn't make my questions clear enough.

The most important question that seems unanswered to me is this:

  Why is this only sometimes the right thing to do ?  On what basis
  might a user choose ?

To which you answered:

  This is not an "only sometimes" thing. User doesn't choose it. We always
  want to watch for AER errors.

But this leads to more fundamental questions.

If this behaviour is always required, why do we have an API call to
request it ?  It sounds like not calling this new function of yours is
always a mistake.  Ie this function (which has an obscure name) is
like "IAC DONT RANDOMY-LOSE" (see RFC748, from 1st April 1978)
except that you are making DO RANDOMLY-LOSE the default (in violation
of the RFC, should anyone talk to the server over telnet...)

If you are inventing a new kind of monitoring process that must be run
for all domains, that is a thing that libxl does not have right now.
At least, it doesn't have it in this form.  (xl has the reboot
monitor, and this is done differently in libvirt.)

It was indeed a design principle of libxl that it should (at least,
wherever possible) be possible to run a domain _without_ a monitoring
process imposed by libxl.

So: why is what this API call requests, not done automatically by
pciback or by Xen ?

And: if you are inventing a new monitoring process that must be run
for every domain, you should call this out much more explicitly as a
fundamental design change.

We will then have to think about more questions: should this process
be run automatically by libxl, without special application request
(like the way that libxl runs qemu) ?

If not, how do we ensure that exactly one of these processes is
running for each guest ?

If your new design involves new behaviour in callers of libxl, do you
intend to send patches for libvirt to enable it ?


Looking at the code:

You handle errors by logging and continuing.  Why is that correct ?

If we are to keep the current API for the client, it needs to have
better doc comments.

Is the xenstore watch implementation vulnerable to unexpected paths
appearing in watch events ?

Why is the API not a never-completing ao ?  Or, why is it not an
evreg ?

But the fundamental design questions need answering first.

Ian.
diff mbox

Patch

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 7cf0f31..c5af0aa 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1044,6 +1044,20 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_QED 1
 
+/* LIBXL_HAVE_REG_AER_EVENTS_HANDLER
+ *
+ * If it is defined, libxl has a library function called
+ * libxl_reg_aer_events_handler.
+ */
+#define LIBXL_HAVE_REG_AER_EVENTS_HANDLER 1
+
+/* LIBXL_HAVE_UNREG_AER_EVENTS_HANDLER
+ *
+ * If it is defined, libxl has a library function called
+ * libxl_unreg_aer_events_handler.
+ */
+#define LIBXL_HAVE_UNREG_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 1ea789e..1aea906 100644
--- a/tools/libxl/libxl_event.h
+++ b/tools/libxl/libxl_event.h
@@ -184,6 +184,19 @@  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, libxl_aer_watch **)
+                        LIBXL_EXTERNAL_CALLERS_ONLY;
+  /*
+   * Registers a handler to handle the occurrence of unrecoverable AER errors.
+   * This function depends on the calling application running the libxl's
+   * internal event loop. Toolstacks that do not use libxl's internal
+   * event loop must arrange to have their own event loop created and enter
+   * libxl (say, call libxl_event_wait()), to enable the event to be processed.
+   */
+void libxl_unreg_aer_events_handler(libxl_ctx *, uint32_t, libxl_aer_watch *)
+                        LIBXL_EXTERNAL_CALLERS_ONLY;
+
 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 afe6652..2b74286 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -352,6 +352,13 @@  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;
+};
 
 /*
  * 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 65ad5e5..feedf27 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1678,6 +1678,96 @@  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;
+    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;
+}
+
+int libxl_reg_aer_events_handler(libxl_ctx *ctx,
+                                 uint32_t domid,
+                                 libxl_aer_watch **aer_ws_out)
+{
+    int rc = 0;
+    int dom0_domid;
+    char *be_path;
+    libxl_aer_watch *aer_ws = NULL;
+    GC_INIT(ctx);
+
+    *aer_ws_out = NULL;
+
+    rc = libxl__get_domid(gc, (uint32_t *)(&dom0_domid));
+    if (rc) {
+        LOGD(ERROR, domid, " libxl__get_domid() failed, rc = %d", rc);
+        goto out;
+    }
+
+    aer_ws = malloc(sizeof(libxl_aer_watch));
+    if (!aer_ws) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+    memset(aer_ws, 0, sizeof(libxl_aer_watch));
+
+    aer_ws->domid = domid;
+    be_path = GCSPRINTF("/local/domain/%d/backend/pci/%u/%d/%s",
+            dom0_domid, domid, dom0_domid, "aerFailedSBDF");
+    rc = libxl__ev_xswatch_register(gc, &aer_ws->watch,
+            aer_backend_watch_callback, be_path);
+    *aer_ws_out = aer_ws;
+
+out:
+    GC_FREE;
+    return rc;
+}
+
+void libxl_unreg_aer_events_handler(libxl_ctx *ctx,
+                                    uint32_t domid,
+                                    libxl_aer_watch *aer_ws)
+{
+    GC_INIT(ctx);
+
+    libxl__ev_xswatch_deregister(gc, &aer_ws->watch);
+
+    free(aer_ws);
+    GC_FREE;
+    return;
+}
+
 DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci);
 
 /*