* [PATCH 1/2] libxl: Fail domain creation if pci assignment fails @ 2014-03-04 13:38 George Dunlap 2014-03-04 13:38 ` [PATCH 2/2] xl: Add "seize" option to PCI devices George Dunlap 2014-03-10 12:04 ` [PATCH 1/2] libxl: Fail domain creation if pci assignment fails Ian Jackson 0 siblings, 2 replies; 6+ messages in thread From: George Dunlap @ 2014-03-04 13:38 UTC (permalink / raw) To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell Actually pay attention to the return value of libxl__device_pci_add. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> --- CC: Ian Jackson <ian.jackson@citrix.com> CC: Ian Campbell <ian.campbell@citrix.com> --- tools/libxl/libxl_create.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index a604cd8..53e7cb6 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -1262,8 +1262,14 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev, goto error_out; } - for (i = 0; i < d_config->num_pcidevs; i++) - libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1); + for (i = 0; i < d_config->num_pcidevs; i++) { + ret = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1); + if (ret < 0) { + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, + "libxl_device_pci_add failed: %d", ret); + goto error_out; + } + } if (d_config->num_pcidevs > 0) { ret = libxl__create_pci_backend(gc, domid, d_config->pcidevs, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] xl: Add "seize" option to PCI devices 2014-03-04 13:38 [PATCH 1/2] libxl: Fail domain creation if pci assignment fails George Dunlap @ 2014-03-04 13:38 ` George Dunlap 2014-03-04 14:06 ` George Dunlap 2014-03-10 12:05 ` Ian Jackson 2014-03-10 12:04 ` [PATCH 1/2] libxl: Fail domain creation if pci assignment fails Ian Jackson 1 sibling, 2 replies; 6+ messages in thread From: George Dunlap @ 2014-03-04 13:38 UTC (permalink / raw) To: xen-devel; +Cc: George Dunlap, Ian Jackson, Ian Campbell The "seize" option tells the toolstack to attempt to automatically unbind devices and re-bind them to the pciback driver. This should make creating VMs that habitually use pass-through (such as driver domain VMs and gaming VMs) easier to use and manage. Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> --- CC: Ian Jackson <ian.jackson@citrix.com> CC: Ian Campbell <ian.campbell@citrix.com> --- docs/man/xl.cfg.pod.5 | 14 ++++++++++++++ tools/libxl/libxl_pci.c | 6 ++++++ tools/libxl/libxl_types.idl | 1 + tools/libxl/libxlu_pci.c | 2 ++ tools/libxl/xl_cmdimpl.c | 5 +++++ 5 files changed, 28 insertions(+) diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index e15a49f..946b438 100644 --- a/docs/man/xl.cfg.pod.5 +++ b/docs/man/xl.cfg.pod.5 @@ -509,6 +509,15 @@ the PCI device regardless whether the guest uses INTx or MSI. Some device drivers, such as NVIDIA's, detect an inconsistency and do not function when this option is enabled. Therefore the default is false (0). +=item B<seize=BOOLEAN> + +Tells xl to automatically attempt to re-assign a device to +pciback if it is not already assigned. + +WARNING: If you set this option, xl will gladly re-assign a critical +system device, such as a network or a disk controller being used by +dom0 without confirmation. Please use with care. + =item B<power_mgmt=BOOLEAN> (HVM only) Specifies that the VM should be able to program the @@ -530,6 +539,11 @@ above. Changes the default value of 'msitranslate' for all PCI devices passed through to this VM. See L<msitranslate|/"msitranslate_boolean"> above. +=item B<pci_seize=BOOLEAN> + +Changes the default value of 'seize' for all PCI devices passed +through to this VM. See L<seize|/"seize_boolean"> above. + =item B<pci_power_mgmt=BOOLEAN> (HVM only) Changes the default value of 'power_mgmt' for all PCI diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 2e52470..44d0453 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -1050,6 +1050,12 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide rc = libxl__device_pci_setdefault(gc, pcidev); if (rc) goto out; + if (pcidev->seize && !pciback_dev_is_assigned(gc, pcidev)) { + rc = libxl__device_pci_assignable_add(gc, pcidev, 1); + if ( rc ) + goto out; + } + if (!libxl_pcidev_assignable(ctx, pcidev)) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not assignable", pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 649ce50..7d3a62b 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -444,6 +444,7 @@ libxl_device_pci = Struct("device_pci", [ ("msitranslate", bool), ("power_mgmt", bool), ("permissive", bool), + ("seize", bool), ]) libxl_device_vtpm = Struct("device_vtpm", [ diff --git a/tools/libxl/libxlu_pci.c b/tools/libxl/libxlu_pci.c index f5dee93..26fb143 100644 --- a/tools/libxl/libxlu_pci.c +++ b/tools/libxl/libxlu_pci.c @@ -141,6 +141,8 @@ int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str pcidev->power_mgmt = atoi(tok); }else if ( !strcmp(optkey, "permissive") ) { pcidev->permissive = atoi(tok); + }else if ( !strcmp(optkey, "seize") ) { + pcidev->seize = atoi(tok); }else{ XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s", optkey); } diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 4fc46eb..5f59bbc 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -737,6 +737,7 @@ static void parse_config_data(const char *config_source, int pci_power_mgmt = 0; int pci_msitranslate = 0; int pci_permissive = 0; + int pci_seize = 0; int i, e; libxl_domain_create_info *c_info = &d_config->c_info; @@ -1462,6 +1463,9 @@ skip_vfb: if (!xlu_cfg_get_long (config, "pci_permissive", &l, 0)) pci_permissive = l; + if (!xlu_cfg_get_long (config, "pci_seize", &l, 0)) + pci_seize = l; + /* To be reworked (automatically enabled) once the auto ballooning * after guest starts is done (with PCI devices passed in). */ if (c_info->type == LIBXL_DOMAIN_TYPE_PV) { @@ -1481,6 +1485,7 @@ skip_vfb: pcidev->msitranslate = pci_msitranslate; pcidev->power_mgmt = pci_power_mgmt; pcidev->permissive = pci_permissive; + pcidev->seize = pci_seize; if (!xlu_pci_parse_bdf(config, pcidev, buf)) d_config->num_pcidevs++; } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] xl: Add "seize" option to PCI devices 2014-03-04 13:38 ` [PATCH 2/2] xl: Add "seize" option to PCI devices George Dunlap @ 2014-03-04 14:06 ` George Dunlap 2014-03-04 14:09 ` Ian Jackson 2014-03-10 12:05 ` Ian Jackson 1 sibling, 1 reply; 6+ messages in thread From: George Dunlap @ 2014-03-04 14:06 UTC (permalink / raw) To: xen-devel; +Cc: Ian Jackson, Ian Campbell On 03/04/2014 01:38 PM, George Dunlap wrote: > The "seize" option tells the toolstack to attempt to automatically > unbind devices and re-bind them to the pciback driver. This should > make creating VMs that habitually use pass-through (such as driver domain > VMs and gaming VMs) easier to use and manage. > > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com> Oops -- sorry, just realized that I had forgotten to add "LIBXL_HAS_PCI_SEIZE"... While I'm here, might as well ask: I was going back and forth on this one, whether "seize" should actually be something passed in via the pci struct, as it's not really a property of the pci device itself. Other options would be: 1. Keep it out of libxl entirely: have the toolstack (in this case xl) do it 2. Have it done in libxl's domain creation 3. Make it an extra argument to libxl_pci_add() When having my initial "second thoughts", #1 seemed most reasonable. But I'm not aware of any other such options handled in xl at the moment; this would be the only example of such an option. Also, the current form allows someone to specify *which* devices to seize, which may be useful in some circumstances. From a practical perspective, adding it to the struct makes it easy to add in the option without requiring an entirely new function for backwards compatibility. So at the moment I'm inclined to leave it as it is; but I'm open to other ideas. -George > --- > CC: Ian Jackson <ian.jackson@citrix.com> > CC: Ian Campbell <ian.campbell@citrix.com> > --- > docs/man/xl.cfg.pod.5 | 14 ++++++++++++++ > tools/libxl/libxl_pci.c | 6 ++++++ > tools/libxl/libxl_types.idl | 1 + > tools/libxl/libxlu_pci.c | 2 ++ > tools/libxl/xl_cmdimpl.c | 5 +++++ > 5 files changed, 28 insertions(+) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index e15a49f..946b438 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -509,6 +509,15 @@ the PCI device regardless whether the guest uses INTx or MSI. Some > device drivers, such as NVIDIA's, detect an inconsistency and do not > function when this option is enabled. Therefore the default is false (0). > > +=item B<seize=BOOLEAN> > + > +Tells xl to automatically attempt to re-assign a device to > +pciback if it is not already assigned. > + > +WARNING: If you set this option, xl will gladly re-assign a critical > +system device, such as a network or a disk controller being used by > +dom0 without confirmation. Please use with care. > + > =item B<power_mgmt=BOOLEAN> > > (HVM only) Specifies that the VM should be able to program the > @@ -530,6 +539,11 @@ above. > Changes the default value of 'msitranslate' for all PCI devices passed > through to this VM. See L<msitranslate|/"msitranslate_boolean"> above. > > +=item B<pci_seize=BOOLEAN> > + > +Changes the default value of 'seize' for all PCI devices passed > +through to this VM. See L<seize|/"seize_boolean"> above. > + > =item B<pci_power_mgmt=BOOLEAN> > > (HVM only) Changes the default value of 'power_mgmt' for all PCI > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c > index 2e52470..44d0453 100644 > --- a/tools/libxl/libxl_pci.c > +++ b/tools/libxl/libxl_pci.c > @@ -1050,6 +1050,12 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide > rc = libxl__device_pci_setdefault(gc, pcidev); > if (rc) goto out; > > + if (pcidev->seize && !pciback_dev_is_assigned(gc, pcidev)) { > + rc = libxl__device_pci_assignable_add(gc, pcidev, 1); > + if ( rc ) > + goto out; > + } > + > if (!libxl_pcidev_assignable(ctx, pcidev)) { > LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not assignable", > pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 649ce50..7d3a62b 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -444,6 +444,7 @@ libxl_device_pci = Struct("device_pci", [ > ("msitranslate", bool), > ("power_mgmt", bool), > ("permissive", bool), > + ("seize", bool), > ]) > > libxl_device_vtpm = Struct("device_vtpm", [ > diff --git a/tools/libxl/libxlu_pci.c b/tools/libxl/libxlu_pci.c > index f5dee93..26fb143 100644 > --- a/tools/libxl/libxlu_pci.c > +++ b/tools/libxl/libxlu_pci.c > @@ -141,6 +141,8 @@ int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str > pcidev->power_mgmt = atoi(tok); > }else if ( !strcmp(optkey, "permissive") ) { > pcidev->permissive = atoi(tok); > + }else if ( !strcmp(optkey, "seize") ) { > + pcidev->seize = atoi(tok); > }else{ > XLU__PCI_ERR(cfg, "Unknown PCI BDF option: %s", optkey); > } > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 4fc46eb..5f59bbc 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -737,6 +737,7 @@ static void parse_config_data(const char *config_source, > int pci_power_mgmt = 0; > int pci_msitranslate = 0; > int pci_permissive = 0; > + int pci_seize = 0; > int i, e; > > libxl_domain_create_info *c_info = &d_config->c_info; > @@ -1462,6 +1463,9 @@ skip_vfb: > if (!xlu_cfg_get_long (config, "pci_permissive", &l, 0)) > pci_permissive = l; > > + if (!xlu_cfg_get_long (config, "pci_seize", &l, 0)) > + pci_seize = l; > + > /* To be reworked (automatically enabled) once the auto ballooning > * after guest starts is done (with PCI devices passed in). */ > if (c_info->type == LIBXL_DOMAIN_TYPE_PV) { > @@ -1481,6 +1485,7 @@ skip_vfb: > pcidev->msitranslate = pci_msitranslate; > pcidev->power_mgmt = pci_power_mgmt; > pcidev->permissive = pci_permissive; > + pcidev->seize = pci_seize; > if (!xlu_pci_parse_bdf(config, pcidev, buf)) > d_config->num_pcidevs++; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] xl: Add "seize" option to PCI devices 2014-03-04 14:06 ` George Dunlap @ 2014-03-04 14:09 ` Ian Jackson 0 siblings, 0 replies; 6+ messages in thread From: Ian Jackson @ 2014-03-04 14:09 UTC (permalink / raw) To: George Dunlap; +Cc: Ian Campbell, xen-devel George Dunlap writes ("Re: [PATCH 2/2] xl: Add "seize" option to PCI devices"): > While I'm here, might as well ask: I was going back and forth on this > one, whether "seize" should actually be something passed in via the pci > struct, as it's not really a property of the pci device itself. Other > options would be: > 1. Keep it out of libxl entirely: have the toolstack (in this case xl) do it > 2. Have it done in libxl's domain creation > 3. Make it an extra argument to libxl_pci_add() > > When having my initial "second thoughts", #1 seemed most reasonable. > But I'm not aware of any other such options handled in xl at the moment; > this would be the only example of such an option. It should definitely be in the pci struct. These libxl structs all specify not just how the device should appear in the guest, but also the host resources which are to be used. To my mind that very much includes the manner in which the host resource allocation is to be done (such as whether or not to automatically seize the device). Ian. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] xl: Add "seize" option to PCI devices 2014-03-04 13:38 ` [PATCH 2/2] xl: Add "seize" option to PCI devices George Dunlap 2014-03-04 14:06 ` George Dunlap @ 2014-03-10 12:05 ` Ian Jackson 1 sibling, 0 replies; 6+ messages in thread From: Ian Jackson @ 2014-03-10 12:05 UTC (permalink / raw) To: George Dunlap; +Cc: Ian Campbell, xen-devel George Dunlap writes ("[PATCH 2/2] xl: Add "seize" option to PCI devices"): > The "seize" option tells the toolstack to attempt to automatically > unbind devices and re-bind them to the pciback driver. This should > make creating VMs that habitually use pass-through (such as driver domain > VMs and gaming VMs) easier to use and manage. I have applied this. (Fixing some trailing whitespace in the xl.cfg.pod.5 patch while I was there.) I wonder if it would be useful to have a global option to make seize the default for every domain. Ian. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] libxl: Fail domain creation if pci assignment fails 2014-03-04 13:38 [PATCH 1/2] libxl: Fail domain creation if pci assignment fails George Dunlap 2014-03-04 13:38 ` [PATCH 2/2] xl: Add "seize" option to PCI devices George Dunlap @ 2014-03-10 12:04 ` Ian Jackson 1 sibling, 0 replies; 6+ messages in thread From: Ian Jackson @ 2014-03-10 12:04 UTC (permalink / raw) To: George Dunlap; +Cc: Ian Campbell, xen-devel George Dunlap writes ("[PATCH 1/2] libxl: Fail domain creation if pci assignment fails"): > Actually pay attention to the return value of libxl__device_pci_add. Applied, thanks. Ian. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-10 12:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-04 13:38 [PATCH 1/2] libxl: Fail domain creation if pci assignment fails George Dunlap 2014-03-04 13:38 ` [PATCH 2/2] xl: Add "seize" option to PCI devices George Dunlap 2014-03-04 14:06 ` George Dunlap 2014-03-04 14:09 ` Ian Jackson 2014-03-10 12:05 ` Ian Jackson 2014-03-10 12:04 ` [PATCH 1/2] libxl: Fail domain creation if pci assignment fails Ian Jackson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.