diff mbox

[2/6] xl: Add commands for hiding and unhiding pcie passthrough devices

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

Commit Message

Venu Busireddy June 27, 2017, 5:14 p.m. UTC
xl: Add commands for hiding and unhiding pcie passthrough devices

Introduce three subcommands: 'xl pci-assignable-hide <s:b:d.f>' to hide a
device, 'xl pci-assignable-unhide <s:b:d.f>' to unhide a previously hidden
device, and 'xl pci-assignable-list-hidden' to list the hidden devices.

Changed create_domain() to register a handler for unrecoverable AER
errors.

Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
---
 tools/xl/xl.h           |   3 ++
 tools/xl/xl_cmdtable.c  |  17 +++++++
 tools/xl/xl_pci.c       | 125 +++++++++++++++++++++++++++++++++++++++++++++++-
 tools/xl/xl_vmcontrol.c |  11 +++++
 4 files changed, 154 insertions(+), 2 deletions(-)

Comments

Wei Liu June 30, 2017, 10:18 a.m. UTC | #1
I haven't reviewed the code in detail, but I have some questions
regarding the design. See the end of this email.

On Tue, Jun 27, 2017 at 12:14:54PM -0500, Venu Busireddy wrote:
>  
> +static void pciassignable_list_hidden(void)
> +{
> +    libxl_device_pci *pcidevs;
> +    int num, i;
> +
> +    pcidevs = libxl_device_pci_assignable_list(ctx, &num);
> +
> +    if ( pcidevs == NULL )

Coding style.

> +        return;
> +    for (i = 0; i < num; i++) {
> +        if (libxl_device_pci_assignable_is_hidden(ctx, &pcidevs[i]))
> +            printf("%04x:%02x:%02x.%01x\n",
> +                   pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
> +        libxl_device_pci_dispose(&pcidevs[i]);
> +    }
> +    free(pcidevs);
> +}
> +
> +int main_pciassignable_list_hidden(int argc, char **argv)
> +{
> +    int opt;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "pci-assignable-list-hidden", 0) {
> +        /* No options */
> +    }
> +
> +    pciassignable_list_hidden();
> +    return 0;
> +}
> +
> +static int pciassignable_hide(const char *bdf)
> +{
> +    libxl_device_pci pcidev;
> +    XLU_Config *config;
> +    int r = EXIT_SUCCESS;
> +
> +    libxl_device_pci_init(&pcidev);
> +
> +    config = xlu_cfg_init(stderr, "command line");
> +    if (!config) {
> +        perror("xlu_cfg_init");
> +        exit(-1);

If you don't want EXIT_FAILURE, please document these exit values
somewhere -- manpage would be a good place.

> +    }
> +
> +    if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
> +        fprintf(stderr, "pci-assignable-hide: malformed BDF specification \"%s\"\n", bdf);
> +        exit(2);
> +    }
> +
> +    if (libxl_device_pci_assignable_hide(ctx, &pcidev))
> +        r = EXIT_FAILURE;
> +
> +    libxl_device_pci_dispose(&pcidev);
> +    xlu_cfg_destroy(config);
> +
> +    return r;
> +}
> +
> +int main_pciassignable_hide(int argc, char **argv)
> +{
> +    int opt;
> +    const char *bdf = NULL;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "main_pciassignable_hide", 1) {
> +        /* No options */
> +    }
> +
> +    bdf = argv[optind];
> +
> +    if (pciassignable_hide(bdf))
> +        return EXIT_FAILURE;
> +
> +    return EXIT_SUCCESS;
> +}
[...]
> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 89c2b25..10a48a9 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -966,6 +966,15 @@ start:
>      LOG("Waiting for domain %s (domid %u) to die [pid %ld]",
>          d_config.c_info.name, domid, (long)getpid());
>  
> +    ret = libxl_reg_aer_events_handler(ctx, domid);
> +    if (ret) {
> +        /*
> +         * This error may not be severe enough to fail the creation of the VM.
> +         * Log the error, and continue with the creation.
> +         */
> +        LOG("libxl_reg_aer_events_handler() failed, ret = 0x%08x", ret);
> +    }
> +

First thing this suggests the ordering of this patch series is wrong --
you need to put the patch that implements the new function before this.

The other thing you need to be aware is that if the user chooses to not
use a daemonised xl, he / she doesn't get a chance to handle these
events.

This is potentially problematic for driver domains. You probably want to
also modify xl devd command. Also on the subject, what's your thought on
driver domain? I'm not sure if a driver domain has the permission to
kill the guest.

>      ret = libxl_evenable_domain_death(ctx, domid, 0, &deathw);
>      if (ret) goto out;
>  
> @@ -993,6 +1002,7 @@ start:
>              LOG("Domain %u has shut down, reason code %d 0x%x", domid,
>                  event->u.domain_shutdown.shutdown_reason,
>                  event->u.domain_shutdown.shutdown_reason);
> +            libxl_unreg_aer_events_handler(ctx, domid);
>              switch (handle_domain_death(&domid, event, &d_config)) {
>              case DOMAIN_RESTART_SOFT_RESET:
>                  domid_soft_reset = domid;
> @@ -1059,6 +1069,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;
Venu Busireddy July 5, 2017, 7:52 p.m. UTC | #2
On 2017-06-30 11:18:10 +0100, Wei Liu wrote:
> I haven't reviewed the code in detail, but I have some questions
> regarding the design. See the end of this email.
> 
> On Tue, Jun 27, 2017 at 12:14:54PM -0500, Venu Busireddy wrote:
> >  
> > +static void pciassignable_list_hidden(void)
> > +{
> > +    libxl_device_pci *pcidevs;
> > +    int num, i;
> > +
> > +    pcidevs = libxl_device_pci_assignable_list(ctx, &num);
> > +
> > +    if ( pcidevs == NULL )
> 
> Coding style.

Will fix.

> > +        return;
> > +    for (i = 0; i < num; i++) {
> > +        if (libxl_device_pci_assignable_is_hidden(ctx, &pcidevs[i]))
> > +            printf("%04x:%02x:%02x.%01x\n",
> > +                   pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
> > +        libxl_device_pci_dispose(&pcidevs[i]);
> > +    }
> > +    free(pcidevs);
> > +}
> > +
> > +int main_pciassignable_list_hidden(int argc, char **argv)
> > +{
> > +    int opt;
> > +
> > +    SWITCH_FOREACH_OPT(opt, "", NULL, "pci-assignable-list-hidden", 0) {
> > +        /* No options */
> > +    }
> > +
> > +    pciassignable_list_hidden();
> > +    return 0;
> > +}
> > +
> > +static int pciassignable_hide(const char *bdf)
> > +{
> > +    libxl_device_pci pcidev;
> > +    XLU_Config *config;
> > +    int r = EXIT_SUCCESS;
> > +
> > +    libxl_device_pci_init(&pcidev);
> > +
> > +    config = xlu_cfg_init(stderr, "command line");
> > +    if (!config) {
> > +        perror("xlu_cfg_init");
> > +        exit(-1);
> 
> If you don't want EXIT_FAILURE, please document these exit values
> somewhere -- manpage would be a good place.

I was following the semantics that other similar functions in that file
(such as pciassignable_add(), etc.) were following, and hence the exit
value of '-1'. I will change this to exit with EXIT_FAILURE.

> > +    }
> > +
> > +    if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
> > +        fprintf(stderr, "pci-assignable-hide: malformed BDF specification \"%s\"\n", bdf);
> > +        exit(2);
> > +    }
> > +
> > +    if (libxl_device_pci_assignable_hide(ctx, &pcidev))
> > +        r = EXIT_FAILURE;
> > +
> > +    libxl_device_pci_dispose(&pcidev);
> > +    xlu_cfg_destroy(config);
> > +
> > +    return r;
> > +}
> > +
> > +int main_pciassignable_hide(int argc, char **argv)
> > +{
> > +    int opt;
> > +    const char *bdf = NULL;
> > +
> > +    SWITCH_FOREACH_OPT(opt, "", NULL, "main_pciassignable_hide", 1) {
> > +        /* No options */
> > +    }
> > +
> > +    bdf = argv[optind];
> > +
> > +    if (pciassignable_hide(bdf))
> > +        return EXIT_FAILURE;
> > +
> > +    return EXIT_SUCCESS;
> > +}
> [...]
> > diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> > index 89c2b25..10a48a9 100644
> > --- a/tools/xl/xl_vmcontrol.c
> > +++ b/tools/xl/xl_vmcontrol.c
> > @@ -966,6 +966,15 @@ start:
> >      LOG("Waiting for domain %s (domid %u) to die [pid %ld]",
> >          d_config.c_info.name, domid, (long)getpid());
> >  
> > +    ret = libxl_reg_aer_events_handler(ctx, domid);
> > +    if (ret) {
> > +        /*
> > +         * This error may not be severe enough to fail the creation of the VM.
> > +         * Log the error, and continue with the creation.
> > +         */
> > +        LOG("libxl_reg_aer_events_handler() failed, ret = 0x%08x", ret);
> > +    }
> > +
> 
> First thing this suggests the ordering of this patch series is wrong --
> you need to put the patch that implements the new function before this.

I will change the order in the next revision.

> The other thing you need to be aware is that if the user chooses to not
> use a daemonised xl, he / she doesn't get a chance to handle these
> events.
> 
> This is potentially problematic for driver domains. You probably want to
> also modify xl devd command. Also on the subject, what's your thought on
> driver domain? I'm not sure if a driver domain has the permission to
> kill the guest.

I don't know if I understood your question correctly, but it is not the
driver domain that is killing another guest. It is Dom0 that is killing
the guest to which the device is assigned in passthrough mode. That guest
should still be killable by Dom0, even if it is a driver domain. Right?

However, I have been asked by Jan Beulich (and many others) on the
need to kill the guest, and why the device can't be unassigned from
that guest! My initial thinking (for the first revision) was that the
guest and the device together are party to evil things, and hence the
guest should be killed. But I agree that unassigning the device should
be sufficient. Once the device is removed, the guest can't do much that
any other guest can't. Therefore, I plan to change this patchset to
simply unassign the device from the guest. This aspect is also covered
in the thread:

https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00552.html

May I request you review that thread and post your thoughts?

And if we go with that approach, some of the questions related to
hide/unhide operations will be obviated!

Thanks,

Venu
Wei Liu July 7, 2017, 10:56 a.m. UTC | #3
On Wed, Jul 05, 2017 at 02:52:41PM -0500, Venu Busireddy wrote:
> > [...]
> > > diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> > > index 89c2b25..10a48a9 100644
> > > --- a/tools/xl/xl_vmcontrol.c
> > > +++ b/tools/xl/xl_vmcontrol.c
> > > @@ -966,6 +966,15 @@ start:
> > >      LOG("Waiting for domain %s (domid %u) to die [pid %ld]",
> > >          d_config.c_info.name, domid, (long)getpid());
> > >  
> > > +    ret = libxl_reg_aer_events_handler(ctx, domid);
> > > +    if (ret) {
> > > +        /*
> > > +         * This error may not be severe enough to fail the creation of the VM.
> > > +         * Log the error, and continue with the creation.
> > > +         */
> > > +        LOG("libxl_reg_aer_events_handler() failed, ret = 0x%08x", ret);
> > > +    }
> > > +
> > 
> > First thing this suggests the ordering of this patch series is wrong --
> > you need to put the patch that implements the new function before this.
> 
> I will change the order in the next revision.
> 
> > The other thing you need to be aware is that if the user chooses to not
> > use a daemonised xl, he / she doesn't get a chance to handle these
> > events.
> > 
> > This is potentially problematic for driver domains. You probably want to
> > also modify xl devd command. Also on the subject, what's your thought on
> > driver domain? I'm not sure if a driver domain has the permission to
> > kill the guest.
> 
> I don't know if I understood your question correctly, but it is not the
> driver domain that is killing another guest. It is Dom0 that is killing
> the guest to which the device is assigned in passthrough mode. That guest
> should still be killable by Dom0, even if it is a driver domain. Right?

OK. I'm not sure my understanding of how PCI passthrough works is
correct, so please correct me if I'm wrong.

First, let's split the two concepts: toolstack domain and driver domain.
They are mostly the same one (Dom0), but they don't have to.

A driver domain drives the underlying hardware and provides virtualised
devices to a DomU.

AIUI (again, I could be very wrong about this):

1. PV PCI passthrough is done via pciback, which means the physical
   device is assigned to the driver domain. All events to / from the
   guest / device are handled by the driver domain -- which includes
   the AER error you're trying to handle.

2. HVM PCI passthrough is done via QEMU, but you also need to pre-assign
   the device to the driver domain in which QEMU runs. All events are only
   visible to the driver domain.

Yes, a guest is going to be always killable by Dom0 (the toolstack
domain), even if some devices of the guest are handled by a driver
domain.

But Dom0 now can't see the AER event so it won't be able to issue the
"kill" or whatever action you want it to do. Is this not the case? Do
you expect the event to be always delivered to Dom0?

> 
> However, I have been asked by Jan Beulich (and many others) on the
> need to kill the guest, and why the device can't be unassigned from
> that guest! My initial thinking (for the first revision) was that the
> guest and the device together are party to evil things, and hence the
> guest should be killed. But I agree that unassigning the device should
> be sufficient. Once the device is removed, the guest can't do much that
> any other guest can't. Therefore, I plan to change this patchset to
> simply unassign the device from the guest. This aspect is also covered
> in the thread:
> 
> https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00552.html
> 
> May I request you review that thread and post your thoughts?
> 

Sure. But that's orthogonal to the problem we have here. I will reply to
that thread.
Konrad Rzeszutek Wilk July 7, 2017, 2 p.m. UTC | #4
On Fri, Jul 07, 2017 at 11:56:43AM +0100, Wei Liu wrote:
> On Wed, Jul 05, 2017 at 02:52:41PM -0500, Venu Busireddy wrote:
> > > [...]
> > > > diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> > > > index 89c2b25..10a48a9 100644
> > > > --- a/tools/xl/xl_vmcontrol.c
> > > > +++ b/tools/xl/xl_vmcontrol.c
> > > > @@ -966,6 +966,15 @@ start:
> > > >      LOG("Waiting for domain %s (domid %u) to die [pid %ld]",
> > > >          d_config.c_info.name, domid, (long)getpid());
> > > >  
> > > > +    ret = libxl_reg_aer_events_handler(ctx, domid);
> > > > +    if (ret) {
> > > > +        /*
> > > > +         * This error may not be severe enough to fail the creation of the VM.
> > > > +         * Log the error, and continue with the creation.
> > > > +         */
> > > > +        LOG("libxl_reg_aer_events_handler() failed, ret = 0x%08x", ret);
> > > > +    }
> > > > +
> > > 
> > > First thing this suggests the ordering of this patch series is wrong --
> > > you need to put the patch that implements the new function before this.
> > 
> > I will change the order in the next revision.
> > 
> > > The other thing you need to be aware is that if the user chooses to not
> > > use a daemonised xl, he / she doesn't get a chance to handle these
> > > events.
> > > 
> > > This is potentially problematic for driver domains. You probably want to
> > > also modify xl devd command. Also on the subject, what's your thought on
> > > driver domain? I'm not sure if a driver domain has the permission to
> > > kill the guest.
> > 
> > I don't know if I understood your question correctly, but it is not the
> > driver domain that is killing another guest. It is Dom0 that is killing
> > the guest to which the device is assigned in passthrough mode. That guest
> > should still be killable by Dom0, even if it is a driver domain. Right?
> 
> OK. I'm not sure my understanding of how PCI passthrough works is
> correct, so please correct me if I'm wrong.
> 
> First, let's split the two concepts: toolstack domain and driver domain.
> They are mostly the same one (Dom0), but they don't have to.
> 
> A driver domain drives the underlying hardware and provides virtualised
> devices to a DomU.
> 
> AIUI (again, I could be very wrong about this):
> 
> 1. PV PCI passthrough is done via pciback, which means the physical
>    device is assigned to the driver domain. All events to / from the
>    guest / device are handled by the driver domain -- which includes
>    the AER error you're trying to handle.
> 
> 2. HVM PCI passthrough is done via QEMU, but you also need to pre-assign
>    the device to the driver domain in which QEMU runs. All events are only
>    visible to the driver domain.
> 
> Yes, a guest is going to be always killable by Dom0 (the toolstack
> domain), even if some devices of the guest are handled by a driver
> domain.
> 
> But Dom0 now can't see the AER event so it won't be able to issue the
> "kill" or whatever action you want it to do. Is this not the case? Do

It can. That is how it works right now - the AER errors are sent to the
PCIe bridge which is a device driver in domain0. Then the kernel
sends it to pciback (which owns the device) to deal with.

> you expect the event to be always delivered to Dom0?

Yes.
> 
> > 
> > However, I have been asked by Jan Beulich (and many others) on the
> > need to kill the guest, and why the device can't be unassigned from
> > that guest! My initial thinking (for the first revision) was that the
> > guest and the device together are party to evil things, and hence the
> > guest should be killed. But I agree that unassigning the device should
> > be sufficient. Once the device is removed, the guest can't do much that
> > any other guest can't. Therefore, I plan to change this patchset to
> > simply unassign the device from the guest. This aspect is also covered
> > in the thread:
> > 
> > https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00552.html
> > 
> > May I request you review that thread and post your thoughts?
> > 
> 
> Sure. But that's orthogonal to the problem we have here. I will reply to
> that thread.
Wei Liu July 18, 2017, 1:38 p.m. UTC | #5
On Fri, Jul 07, 2017 at 10:00:01AM -0400, Konrad Rzeszutek Wilk wrote:
> > 
> > 1. PV PCI passthrough is done via pciback, which means the physical
> >    device is assigned to the driver domain. All events to / from the
> >    guest / device are handled by the driver domain -- which includes
> >    the AER error you're trying to handle.
> > 
> > 2. HVM PCI passthrough is done via QEMU, but you also need to pre-assign
> >    the device to the driver domain in which QEMU runs. All events are only
> >    visible to the driver domain.
> > 
> > Yes, a guest is going to be always killable by Dom0 (the toolstack
> > domain), even if some devices of the guest are handled by a driver
> > domain.
> > 
> > But Dom0 now can't see the AER event so it won't be able to issue the
> > "kill" or whatever action you want it to do. Is this not the case? Do
> 
> It can. That is how it works right now - the AER errors are sent to the
> PCIe bridge which is a device driver in domain0. Then the kernel
> sends it to pciback (which owns the device) to deal with.
> 

So pciback will have to run in Dom0?
Konrad Rzeszutek Wilk July 18, 2017, 3:38 p.m. UTC | #6
On Tue, Jul 18, 2017 at 02:38:14PM +0100, Wei Liu wrote:
> On Fri, Jul 07, 2017 at 10:00:01AM -0400, Konrad Rzeszutek Wilk wrote:
> > > 
> > > 1. PV PCI passthrough is done via pciback, which means the physical
> > >    device is assigned to the driver domain. All events to / from the
> > >    guest / device are handled by the driver domain -- which includes
> > >    the AER error you're trying to handle.
> > > 
> > > 2. HVM PCI passthrough is done via QEMU, but you also need to pre-assign
> > >    the device to the driver domain in which QEMU runs. All events are only
> > >    visible to the driver domain.
> > > 
> > > Yes, a guest is going to be always killable by Dom0 (the toolstack
> > > domain), even if some devices of the guest are handled by a driver
> > > domain.
> > > 
> > > But Dom0 now can't see the AER event so it won't be able to issue the
> > > "kill" or whatever action you want it to do. Is this not the case? Do
> > 
> > It can. That is how it works right now - the AER errors are sent to the
> > PCIe bridge which is a device driver in domain0. Then the kernel
> > sends it to pciback (which owns the device) to deal with.
> > 
> 
> So pciback will have to run in Dom0?

You need a kernel piece to deal with AER (as it is an normal interrupt to
the PCIe bridge and the PCIe bridge picks this up and does its thing).

The AER mechanism (in Linux) then walks through all the devices underneath
the PCIe bridge and gives it options to do things - and in case of pciback
we can do the right thing.
diff mbox

Patch

diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index aa95b77..915fe86 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -121,9 +121,12 @@  int main_vncviewer(int argc, char **argv);
 int main_pcilist(int argc, char **argv);
 int main_pcidetach(int argc, char **argv);
 int main_pciattach(int argc, char **argv);
+int main_pciassignable_hide(int argc, char **argv);
+int main_pciassignable_unhide(int argc, char **argv);
 int main_pciassignable_add(int argc, char **argv);
 int main_pciassignable_remove(int argc, char **argv);
 int main_pciassignable_list(int argc, char **argv);
+int main_pciassignable_list_hidden(int argc, char **argv);
 #ifndef LIBXL_HAVE_NO_SUSPEND_RESUME
 int main_restore(int argc, char **argv);
 int main_migrate_receive(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 30eb93c..e23bd15 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -119,6 +119,23 @@  struct cmd_spec cmd_table[] = {
       "List all the assignable pci devices",
       "",
     },
+    { "pci-assignable-list-hidden",
+      &main_pciassignable_list_hidden, 0, 0,
+      "List all the pci devices hidden due to AER errors",
+      "",
+    },
+    { "pci-assignable-hide",
+      &main_pciassignable_hide, 0, 1,
+      "Hide a PCI device",
+      "<BDF>",
+      "-h                      Print this help.\n"
+    },
+    { "pci-assignable-unhide",
+      &main_pciassignable_unhide, 0, 1,
+      "Unhide a PCI device",
+      "<BDF>",
+      "-h                      Print this help.\n"
+    },
     { "pause",
       &main_pause, 0, 1,
       "Pause execution of a domain",
diff --git a/tools/xl/xl_pci.c b/tools/xl/xl_pci.c
index 58345bd..f48c469 100644
--- a/tools/xl/xl_pci.c
+++ b/tools/xl/xl_pci.c
@@ -163,8 +163,9 @@  static void pciassignable_list(void)
     if ( pcidevs == NULL )
         return;
     for (i = 0; i < num; i++) {
-        printf("%04x:%02x:%02x.%01x\n",
-               pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+        if (!libxl_device_pci_assignable_is_hidden(ctx, &pcidevs[i]))
+            printf("%04x:%02x:%02x.%01x\n",
+                   pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
         libxl_device_pci_dispose(&pcidevs[i]);
     }
     free(pcidevs);
@@ -182,6 +183,126 @@  int main_pciassignable_list(int argc, char **argv)
     return 0;
 }
 
+static void pciassignable_list_hidden(void)
+{
+    libxl_device_pci *pcidevs;
+    int num, i;
+
+    pcidevs = libxl_device_pci_assignable_list(ctx, &num);
+
+    if ( pcidevs == NULL )
+        return;
+    for (i = 0; i < num; i++) {
+        if (libxl_device_pci_assignable_is_hidden(ctx, &pcidevs[i]))
+            printf("%04x:%02x:%02x.%01x\n",
+                   pcidevs[i].domain, pcidevs[i].bus, pcidevs[i].dev, pcidevs[i].func);
+        libxl_device_pci_dispose(&pcidevs[i]);
+    }
+    free(pcidevs);
+}
+
+int main_pciassignable_list_hidden(int argc, char **argv)
+{
+    int opt;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "pci-assignable-list-hidden", 0) {
+        /* No options */
+    }
+
+    pciassignable_list_hidden();
+    return 0;
+}
+
+static int pciassignable_hide(const char *bdf)
+{
+    libxl_device_pci pcidev;
+    XLU_Config *config;
+    int r = EXIT_SUCCESS;
+
+    libxl_device_pci_init(&pcidev);
+
+    config = xlu_cfg_init(stderr, "command line");
+    if (!config) {
+        perror("xlu_cfg_init");
+        exit(-1);
+    }
+
+    if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
+        fprintf(stderr, "pci-assignable-hide: malformed BDF specification \"%s\"\n", bdf);
+        exit(2);
+    }
+
+    if (libxl_device_pci_assignable_hide(ctx, &pcidev))
+        r = EXIT_FAILURE;
+
+    libxl_device_pci_dispose(&pcidev);
+    xlu_cfg_destroy(config);
+
+    return r;
+}
+
+int main_pciassignable_hide(int argc, char **argv)
+{
+    int opt;
+    const char *bdf = NULL;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "main_pciassignable_hide", 1) {
+        /* No options */
+    }
+
+    bdf = argv[optind];
+
+    if (pciassignable_hide(bdf))
+        return EXIT_FAILURE;
+
+    return EXIT_SUCCESS;
+}
+
+static int pciassignable_unhide(const char *bdf)
+{
+    libxl_device_pci pcidev;
+    XLU_Config *config;
+    int r = EXIT_SUCCESS;
+
+    libxl_device_pci_init(&pcidev);
+
+    config = xlu_cfg_init(stderr, "command line");
+    if (!config) {
+        perror("xlu_cfg_init");
+        exit(-1);
+    }
+
+    if (xlu_pci_parse_bdf(config, &pcidev, bdf)) {
+        fprintf(stderr, "pci-assignable-unhide: malformed BDF specification \"%s\"\n", bdf);
+        exit(2);
+    }
+
+    if (libxl_device_pci_assignable_unhide(ctx, &pcidev))
+        r = EXIT_FAILURE;
+
+    libxl_device_pci_dispose(&pcidev);
+    xlu_cfg_destroy(config);
+
+    return r;
+}
+
+int main_pciassignable_unhide(int argc, char **argv)
+{
+    int opt;
+    const char *bdf = NULL;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "main_pciassignable_unhide", 1) {
+        /* No options */
+    }
+
+    bdf = argv[optind];
+
+    if (pciassignable_unhide(bdf))
+        return EXIT_FAILURE;
+
+    return EXIT_SUCCESS;
+}
+
 static int pciassignable_add(const char *bdf, int rebind)
 {
     libxl_device_pci pcidev;
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 89c2b25..10a48a9 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -966,6 +966,15 @@  start:
     LOG("Waiting for domain %s (domid %u) to die [pid %ld]",
         d_config.c_info.name, domid, (long)getpid());
 
+    ret = libxl_reg_aer_events_handler(ctx, domid);
+    if (ret) {
+        /*
+         * This error may not be severe enough to fail the creation of the VM.
+         * Log the error, and continue with the creation.
+         */
+        LOG("libxl_reg_aer_events_handler() failed, ret = 0x%08x", ret);
+    }
+
     ret = libxl_evenable_domain_death(ctx, domid, 0, &deathw);
     if (ret) goto out;
 
@@ -993,6 +1002,7 @@  start:
             LOG("Domain %u has shut down, reason code %d 0x%x", domid,
                 event->u.domain_shutdown.shutdown_reason,
                 event->u.domain_shutdown.shutdown_reason);
+            libxl_unreg_aer_events_handler(ctx, domid);
             switch (handle_domain_death(&domid, event, &d_config)) {
             case DOMAIN_RESTART_SOFT_RESET:
                 domid_soft_reset = domid;
@@ -1059,6 +1069,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;