Re: [libvirt] [PATCHv4 3/3] qemu: Prefer VFIO for PCI device passthrough
On 10/09/13 16:01, Laine Stump wrote:
> On 10/08/2013 06:46 PM, Peter Krempa wrote:
>> Prefer using VFIO (if available) to the legacy KVM device passthrough.
>>
>> With this patch a PCI passthrough device without the driver configured
>> will be started with VFIO if it's available on the host. If not legacy
>> KVM passthrough is checked and error is reported if it's not available.
>> ---
>>
>> Notes:
>> Version 4:
>> - Adapted to call tree change
>>
>> docs/formatdomain.html.in | 9 -
>> src/conf/domain_conf.h| 2 +-
>> src/qemu/qemu_command.c | 2 +-
>> src/qemu/qemu_hostdev.c | 30 +-
>> src/qemu/qemu_hostdev.h | 4 +++-
>> src/qemu/qemu_hotplug.c | 2 +-
>> src/qemu/qemu_process.c | 15 ---
>> tests/qemuxml2argvtest.c | 11 +++
>> 8 files changed, 54 insertions(+), 21 deletions(-)
>>
>
> ACk, with a less misleading error message.
>
I've changed the error message to "_("invalid PCI passthrough type
'%s'")" and pushed. Thanks for the review.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 3/3] qemu: Prefer VFIO for PCI device passthrough
On 10/08/2013 06:46 PM, Peter Krempa wrote:
> Prefer using VFIO (if available) to the legacy KVM device passthrough.
>
> With this patch a PCI passthrough device without the driver configured
> will be started with VFIO if it's available on the host. If not legacy
> KVM passthrough is checked and error is reported if it's not available.
> ---
>
> Notes:
> Version 4:
> - Adapted to call tree change
>
> docs/formatdomain.html.in | 9 -
> src/conf/domain_conf.h| 2 +-
> src/qemu/qemu_command.c | 2 +-
> src/qemu/qemu_hostdev.c | 30 +-
> src/qemu/qemu_hostdev.h | 4 +++-
> src/qemu/qemu_hotplug.c | 2 +-
> src/qemu/qemu_process.c | 15 ---
> tests/qemuxml2argvtest.c | 11 +++
> 8 files changed, 54 insertions(+), 21 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 3689399..6f3f7cf 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2755,11 +2755,10 @@
> backend, which is compatible with UEFI SecureBoot) or "kvm"
> (for the legacy device assignment handled directly by the KVM
> kernel module)Since 1.0.5 (QEMU and KVM
> -only, requires kernel 3.6 or newer). Currently, "kvm"
> -is the default used by libvirt when not explicitly provided,
> -but since the two are functionally equivalent, this default
> -could be changed in the future with no impact to domains that
> -don't specify anything.
> +only, requires kernel 3.6 or newer). The default, when
> +the driver name is not explicitly specified, is to check wether
> +VFIO is available and use it if it's the case. If VFIO is not
> +available, the legacy "kvm" assignment is attempted.
>
>readonly
>Indicates that the device is readonly, only supported by SCSI host
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index f20a916..6b825d8 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -399,7 +399,7 @@ enum virDomainHostdevSubsysType {
>
> /* the backend driver used for PCI hostdev devices */
> typedef enum {
> -VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* currently kvm, could change */
> +VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* detect automaticaly, prefer
> VFIO */
> VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,/* force legacy kvm style */
> VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO, /* force vfio */
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 52dc295..da53c51 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5487,7 +5487,6 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
>
> switch ((virDomainHostdevSubsysPciBackendType)
> dev->source.subsys.u.pci.backend) {
> -case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
> case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
> virBufferAddLit(&buf, "pci-assign");
> if (configfd && *configfd)
> @@ -5498,6 +5497,7 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
> virBufferAddLit(&buf, "vfio-pci");
> break;
>
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
> case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("PCI passhthrough type needs to be specified"));
I think this error message would be misleading to a user. What they
would think is "Oh, I need to specify the 'PCI passthrough type'; I
wonder what *that* is..." (since that's not what it's called in the
XML). But what it really means is "There is an error in the code -
u.pci.backend should have been set to either KVM or VFIO by the
functions that called here; please report this as a bug!".
As the error is right now, I think it would do as much damage as good -
maybe you could change it to "Unable to determine whether to use VFIO or
legacy KVM driver for PCI device assignment". (or something else that
avoids saying "needs to be specified").
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index 7f3170d..81e0e88 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -564,7 +564,8 @@ qemuHostdevHostSupportsPassthroughLegacy(void)
>
> static bool
> qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr *hostdevs,
> - size_t nhostdevs)
> + size_t nhostdevs,
> + virQEMUCapsPtr qemuCaps)
> {
> bool supportsPassthroughKVM = qemuHostdevHostSupportsPassthroughLegacy();
> bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO();
> @@ -581,6 +582,23 @@ qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr
> *hostdevs,
> continue;
>
> switch ((virDomainHostdevSubsysPciBackendType) *backend) {
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
> +if (supportsPassth
[libvirt] [PATCHv4 3/3] qemu: Prefer VFIO for PCI device passthrough
Prefer using VFIO (if available) to the legacy KVM device passthrough.
With this patch a PCI passthrough device without the driver configured
will be started with VFIO if it's available on the host. If not legacy
KVM passthrough is checked and error is reported if it's not available.
---
Notes:
Version 4:
- Adapted to call tree change
docs/formatdomain.html.in | 9 -
src/conf/domain_conf.h| 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_hostdev.c | 30 +-
src/qemu/qemu_hostdev.h | 4 +++-
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_process.c | 15 ---
tests/qemuxml2argvtest.c | 11 +++
8 files changed, 54 insertions(+), 21 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3689399..6f3f7cf 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2755,11 +2755,10 @@
backend, which is compatible with UEFI SecureBoot) or "kvm"
(for the legacy device assignment handled directly by the KVM
kernel module)Since 1.0.5 (QEMU and KVM
-only, requires kernel 3.6 or newer). Currently, "kvm"
-is the default used by libvirt when not explicitly provided,
-but since the two are functionally equivalent, this default
-could be changed in the future with no impact to domains that
-don't specify anything.
+only, requires kernel 3.6 or newer). The default, when
+the driver name is not explicitly specified, is to check wether
+VFIO is available and use it if it's the case. If VFIO is not
+available, the legacy "kvm" assignment is attempted.
readonly
Indicates that the device is readonly, only supported by SCSI host
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f20a916..6b825d8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -399,7 +399,7 @@ enum virDomainHostdevSubsysType {
/* the backend driver used for PCI hostdev devices */
typedef enum {
-VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* currently kvm, could change */
+VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* detect automaticaly, prefer
VFIO */
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,/* force legacy kvm style */
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO, /* force vfio */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 52dc295..da53c51 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5487,7 +5487,6 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
switch ((virDomainHostdevSubsysPciBackendType)
dev->source.subsys.u.pci.backend) {
-case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
virBufferAddLit(&buf, "pci-assign");
if (configfd && *configfd)
@@ -5498,6 +5497,7 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
virBufferAddLit(&buf, "vfio-pci");
break;
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("PCI passhthrough type needs to be specified"));
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 7f3170d..81e0e88 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -564,7 +564,8 @@ qemuHostdevHostSupportsPassthroughLegacy(void)
static bool
qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr *hostdevs,
- size_t nhostdevs)
+ size_t nhostdevs,
+ virQEMUCapsPtr qemuCaps)
{
bool supportsPassthroughKVM = qemuHostdevHostSupportsPassthroughLegacy();
bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO();
@@ -581,6 +582,23 @@ qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr
*hostdevs,
continue;
switch ((virDomainHostdevSubsysPciBackendType) *backend) {
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
+if (supportsPassthroughVFIO &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
+*backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
+} else if (supportsPassthroughKVM &&
+ (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIDEVICE) ||
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) {
+*backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM;
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("host doesn't support passthrough of "
+ "host PCI devices"));
+return false;
+}
+
+break;
+
case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
if (!supportsPassthroughVFIO) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -589,7 +607,6 @@
