[PATCH 1/4] KVM: support VT-d device hotplug

[PATCH 1/4] KVM: support VT-d device hotplug

[Han, Weidong]

[-- Attachment #1: Type: text/plain, Size: 5812 bytes --]

>From bba614bf2acf22f765995fb2364de04cec039226 Mon Sep 17 00:00:00 2001
From: Weidong Han <weidong.han@intel.com>
Date: Fri, 14 Nov 2008 16:53:10 +0800
Subject: [PATCH] support VT-d device hotplug

wrap kvm_assign_device() and kvm_deassign_device() to support assign/deassign a device to a guest

Signed-off-by: Weidong Han <weidong.han@intel.com>
---
 include/linux/kvm.h      |    2 +
 include/linux/kvm_host.h |   18 +++++++++++++++
 virt/kvm/kvm_main.c      |   45 ++++++++++++++++++++++++++++++++++++-
 virt/kvm/vtd.c           |   55 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 119 insertions(+), 1 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 44fd7fa..558bc32 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -425,6 +425,8 @@ struct kvm_trace_rec {
 				   struct kvm_assigned_pci_dev)
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
+#define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
+				     struct kvm_assigned_pci_dev)
 
 /*
  * ioctls for vcpu fds
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3a0fb77..4830372 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -326,6 +326,10 @@ int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
 int kvm_iommu_map_guest(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *assigned_dev);
 int kvm_iommu_unmap_guest(struct kvm *kvm);
+int kvm_assign_device(struct kvm *kvm,
+		      struct kvm_assigned_dev_kernel *assigned_dev);
+int kvm_deassign_device(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev);
 #else /* CONFIG_DMAR */
 static inline int kvm_iommu_map_pages(struct kvm *kvm,
 				      gfn_t base_gfn,
@@ -345,6 +349,20 @@ static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
 {
 	return 0;
 }
+
+static inline int kvm_assign_device(struct kvm *kvm,
+				    struct kvm_assigned_dev_kernel
+				    *assigned_dev)
+{
+	return 0;
+}
+
+static inline int kvm_deassign_device(struct kvm *kvm,
+				      struct kvm_assigned_dev_kernel
+				      *assigned_dev)
+{
+	return 0;
+}
 #endif /* CONFIG_DMAR */
 
 static inline void kvm_guest_enter(void)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4f43abe..689d615 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -293,7 +293,11 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
 	list_add(&match->list, &kvm->arch.assigned_dev_head);
 
 	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
-		r = kvm_iommu_map_guest(kvm, match);
+		if (kvm->arch.intel_iommu_domain)
+			r = kvm_assign_device(kvm, match);
+		else
+			r = kvm_iommu_map_guest(kvm, match);
+
 		if (r)
 			goto out_list_del;
 	}
@@ -313,6 +317,34 @@ out_free:
 	mutex_unlock(&kvm->lock);
 	return r;
 }
+
+static int kvm_vm_ioctl_deassign_device(struct kvm *kvm,
+                                        struct kvm_assigned_pci_dev
+					*assigned_dev)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *match;
+
+	mutex_lock(&kvm->lock);
+
+	match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      assigned_dev->assigned_dev_id);
+	if (!match) {
+		printk(KERN_INFO "%s: device hasn't been assigned before, "
+		  "so cannot be deassigned\n", __func__);
+		r = -EINVAL;
+		goto out;
+	}
+
+	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU)
+		kvm_deassign_device(kvm, match);
+
+	kvm_free_assigned_device(kvm, match);
+
+out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
 #endif
 
 static inline int valid_vcpu(int n)
@@ -1650,6 +1682,17 @@ static long kvm_vm_ioctl(struct file *filp,
 			goto out;
 		break;
 	}
+	case KVM_DEASSIGN_PCI_DEVICE: {
+		struct kvm_assigned_pci_dev assigned_dev;
+
+		r = -EFAULT;
+		if (copy_from_user(&assigned_dev, argp, sizeof assigned_dev))
+			goto out;
+		r = kvm_vm_ioctl_deassign_device(kvm, &assigned_dev);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
diff --git a/virt/kvm/vtd.c b/virt/kvm/vtd.c
index a770874..de55457 100644
--- a/virt/kvm/vtd.c
+++ b/virt/kvm/vtd.c
@@ -86,6 +86,61 @@ static int kvm_iommu_map_memslots(struct kvm *kvm)
 	return r;
 }
 
+int kvm_assign_device(struct kvm *kvm,
+		      struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct pci_dev *pdev = NULL;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	int r;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	pdev = assigned_dev->dev;
+	if (pdev == NULL)
+		return -ENODEV;
+
+	intel_iommu_detach_dev(domain, pdev->bus->number, pdev->devfn);
+
+	r = intel_iommu_context_mapping(domain,	pdev);
+	if (r) {
+		printk(KERN_ERR "Domain context map for %s failed",
+		       pci_name(pdev));
+		return r;
+        }
+
+	printk(KERN_DEBUG "%s: host bdf = %x:%x:%x\n",
+	       __func__,
+	       assigned_dev->host_busnr,
+	       PCI_SLOT(assigned_dev->host_devfn),
+	       PCI_FUNC(assigned_dev->host_devfn));
+
+	return 0;
+}
+
+int kvm_deassign_device(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	/* detach kvm dmar domain */
+	intel_iommu_detach_dev(domain, assigned_dev->host_busnr,
+			       assigned_dev->host_devfn);
+
+	printk(KERN_DEBUG "%s: host bdf = %x:%x:%x\n",
+		__func__,
+	       assigned_dev->host_busnr,
+	       PCI_SLOT(assigned_dev->host_devfn),
+	       PCI_FUNC(assigned_dev->host_devfn));
+
+	return 0;
+}
+
 int kvm_iommu_map_guest(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *assigned_dev)
 {
-- 
1.5.1

[-- Attachment #2: 0001-support-VT-d-device-hotplug.patch --]
[-- Type: application/octet-stream, Size: 5610 bytes --]

From bba614bf2acf22f765995fb2364de04cec039226 Mon Sep 17 00:00:00 2001
From: Weidong Han <weidong.han@intel.com>
Date: Fri, 14 Nov 2008 16:53:10 +0800
Subject: [PATCH] support VT-d device hotplug

wrap kvm_assign_device() and kvm_deassign_device() to support assign/deassign a device to a guest

Signed-off-by: Weidong Han <weidong.han@intel.com>
---
 include/linux/kvm.h      |    2 +
 include/linux/kvm_host.h |   18 +++++++++++++++
 virt/kvm/kvm_main.c      |   45 ++++++++++++++++++++++++++++++++++++-
 virt/kvm/vtd.c           |   55 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 119 insertions(+), 1 deletions(-)

diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 44fd7fa..558bc32 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -425,6 +425,8 @@ struct kvm_trace_rec {
 				   struct kvm_assigned_pci_dev)
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
+#define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
+				     struct kvm_assigned_pci_dev)
 
 /*
  * ioctls for vcpu fds
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3a0fb77..4830372 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -326,6 +326,10 @@ int kvm_iommu_map_pages(struct kvm *kvm, gfn_t base_gfn,
 int kvm_iommu_map_guest(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *assigned_dev);
 int kvm_iommu_unmap_guest(struct kvm *kvm);
+int kvm_assign_device(struct kvm *kvm,
+		      struct kvm_assigned_dev_kernel *assigned_dev);
+int kvm_deassign_device(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev);
 #else /* CONFIG_DMAR */
 static inline int kvm_iommu_map_pages(struct kvm *kvm,
 				      gfn_t base_gfn,
@@ -345,6 +349,20 @@ static inline int kvm_iommu_unmap_guest(struct kvm *kvm)
 {
 	return 0;
 }
+
+static inline int kvm_assign_device(struct kvm *kvm,
+				    struct kvm_assigned_dev_kernel
+				    *assigned_dev)
+{
+	return 0;
+}
+
+static inline int kvm_deassign_device(struct kvm *kvm,
+				      struct kvm_assigned_dev_kernel
+				      *assigned_dev)
+{
+	return 0;
+}
 #endif /* CONFIG_DMAR */
 
 static inline void kvm_guest_enter(void)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4f43abe..689d615 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -293,7 +293,11 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
 	list_add(&match->list, &kvm->arch.assigned_dev_head);
 
 	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
-		r = kvm_iommu_map_guest(kvm, match);
+		if (kvm->arch.intel_iommu_domain)
+			r = kvm_assign_device(kvm, match);
+		else
+			r = kvm_iommu_map_guest(kvm, match);
+
 		if (r)
 			goto out_list_del;
 	}
@@ -313,6 +317,34 @@ out_free:
 	mutex_unlock(&kvm->lock);
 	return r;
 }
+
+static int kvm_vm_ioctl_deassign_device(struct kvm *kvm,
+                                        struct kvm_assigned_pci_dev
+					*assigned_dev)
+{
+	int r = 0;
+	struct kvm_assigned_dev_kernel *match;
+
+	mutex_lock(&kvm->lock);
+
+	match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
+				      assigned_dev->assigned_dev_id);
+	if (!match) {
+		printk(KERN_INFO "%s: device hasn't been assigned before, "
+		  "so cannot be deassigned\n", __func__);
+		r = -EINVAL;
+		goto out;
+	}
+
+	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU)
+		kvm_deassign_device(kvm, match);
+
+	kvm_free_assigned_device(kvm, match);
+
+out:
+	mutex_unlock(&kvm->lock);
+	return r;
+}
 #endif
 
 static inline int valid_vcpu(int n)
@@ -1650,6 +1682,17 @@ static long kvm_vm_ioctl(struct file *filp,
 			goto out;
 		break;
 	}
+	case KVM_DEASSIGN_PCI_DEVICE: {
+		struct kvm_assigned_pci_dev assigned_dev;
+
+		r = -EFAULT;
+		if (copy_from_user(&assigned_dev, argp, sizeof assigned_dev))
+			goto out;
+		r = kvm_vm_ioctl_deassign_device(kvm, &assigned_dev);
+		if (r)
+			goto out;
+		break;
+	}
 #endif
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
diff --git a/virt/kvm/vtd.c b/virt/kvm/vtd.c
index a770874..de55457 100644
--- a/virt/kvm/vtd.c
+++ b/virt/kvm/vtd.c
@@ -86,6 +86,61 @@ static int kvm_iommu_map_memslots(struct kvm *kvm)
 	return r;
 }
 
+int kvm_assign_device(struct kvm *kvm,
+		      struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct pci_dev *pdev = NULL;
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+	int r;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	pdev = assigned_dev->dev;
+	if (pdev == NULL)
+		return -ENODEV;
+
+	intel_iommu_detach_dev(domain, pdev->bus->number, pdev->devfn);
+
+	r = intel_iommu_context_mapping(domain,	pdev);
+	if (r) {
+		printk(KERN_ERR "Domain context map for %s failed",
+		       pci_name(pdev));
+		return r;
+        }
+
+	printk(KERN_DEBUG "%s: host bdf = %x:%x:%x\n",
+	       __func__,
+	       assigned_dev->host_busnr,
+	       PCI_SLOT(assigned_dev->host_devfn),
+	       PCI_FUNC(assigned_dev->host_devfn));
+
+	return 0;
+}
+
+int kvm_deassign_device(struct kvm *kvm,
+			struct kvm_assigned_dev_kernel *assigned_dev)
+{
+	struct dmar_domain *domain = kvm->arch.intel_iommu_domain;
+
+	/* check if iommu exists and in use */
+	if (!domain)
+		return 0;
+
+	/* detach kvm dmar domain */
+	intel_iommu_detach_dev(domain, assigned_dev->host_busnr,
+			       assigned_dev->host_devfn);
+
+	printk(KERN_DEBUG "%s: host bdf = %x:%x:%x\n",
+		__func__,
+	       assigned_dev->host_busnr,
+	       PCI_SLOT(assigned_dev->host_devfn),
+	       PCI_FUNC(assigned_dev->host_devfn));
+
+	return 0;
+}
+
 int kvm_iommu_map_guest(struct kvm *kvm,
 			struct kvm_assigned_dev_kernel *assigned_dev)
 {
-- 
1.5.1

Re: [PATCH 1/4] KVM: support VT-d device hotplug

[Avi Kivity]

Han, Weidong wrote:
> From bba614bf2acf22f765995fb2364de04cec039226 Mon Sep 17 00:00:00 2001
> From: Weidong Han <weidong.han@intel.com>
> Date: Fri, 14 Nov 2008 16:53:10 +0800
> Subject: [PATCH] support VT-d device hotplug
>
> wrap kvm_assign_device() and kvm_deassign_device() to support assign/deassign a device to a guest
>   

> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -425,6 +425,8 @@ struct kvm_trace_rec {
>  				   struct kvm_assigned_pci_dev)
>  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
>  			    struct kvm_assigned_irq)
> +#define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
> +				     struct kvm_assigned_pci_dev)
>  
>   

Need a KVM_CAP_ to indicate this is available.

>  
>  static inline void kvm_guest_enter(void)
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 4f43abe..689d615 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -293,7 +293,11 @@ static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
>  	list_add(&match->list, &kvm->arch.assigned_dev_head);
>  
>  	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
> -		r = kvm_iommu_map_guest(kvm, match);
> +		if (kvm->arch.intel_iommu_domain)
> +			r = kvm_assign_device(kvm, match);
> +		else
> +			r = kvm_iommu_map_guest(kvm, match);
> +
>   

Shouldn't kvm_assign_device() be called unconditionally?  Perhaps some 
code needs to be removed from kvm_iommu_map_guest().


-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 1/4] KVM: support VT-d device hotplug

[Han, Weidong]

Avi Kivity wrote:
> Han, Weidong wrote:
>> From bba614bf2acf22f765995fb2364de04cec039226 Mon Sep 17 00:00:00
>> 2001 
>> From: Weidong Han <weidong.han@intel.com>
>> Date: Fri, 14 Nov 2008 16:53:10 +0800
>> Subject: [PATCH] support VT-d device hotplug
>> 
>> wrap kvm_assign_device() and kvm_deassign_device() to support
>> assign/deassign a device to a guest 
>> 
> 
>> --- a/include/linux/kvm.h
>> +++ b/include/linux/kvm.h
>> @@ -425,6 +425,8 @@ struct kvm_trace_rec {
>>  				   struct kvm_assigned_pci_dev)
>>  #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
>>  			    struct kvm_assigned_irq)
>> +#define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
>> +				     struct kvm_assigned_pci_dev)
>> 
>> 
> 
> Need a KVM_CAP_ to indicate this is available.
> 

KVM_ASSIGN_PCI_DEVICE and KVM_ASSIGN_IRQ don't need a KVM_CAP_xxx. Why do you need it for KVM_DEASSIGN_PCI_DEVICE?

>> 
>>  static inline void kvm_guest_enter(void)
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 4f43abe..689d615 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -293,7 +293,11 @@ static int kvm_vm_ioctl_assign_device(struct
>>  	kvm *kvm, list_add(&match->list, &kvm->arch.assigned_dev_head);
>> 
>>  	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
>> -		r = kvm_iommu_map_guest(kvm, match);
>> +		if (kvm->arch.intel_iommu_domain)
>> +			r = kvm_assign_device(kvm, match);
>> +		else
>> +			r = kvm_iommu_map_guest(kvm, match);
>> +
>> 
> 
> Shouldn't kvm_assign_device() be called unconditionally?  Perhaps some
> code needs to be removed from kvm_iommu_map_guest().

Yes, kvm_iommu_map_guest() just needs to allocate intel_iommu_domain and map memslots. It needs to be called once for a guest. The code will look like:
	...
	if (assigned_dev->flags & KVM_DEV_ASSIGN_ENABLE_IOMMU) {
		if (!kvm->arch.intel_iommu_domain)
			r = kvm_iommu_map_guest(kvm, match);
		r = kvm_assign_device(kvm, match);
	...	

I will update the patch.

Regards,
Weidong

Re: [PATCH 1/4] KVM: support VT-d device hotplug

[Avi Kivity]

Han, Weidong wrote:
>> Need a KVM_CAP_ to indicate this is available.
>>
>>     
>
> KVM_ASSIGN_PCI_DEVICE and KVM_ASSIGN_IRQ don't need a KVM_CAP_xxx. Why do you need it for KVM_DEASSIGN_PCI_DEVICE?
>
>   

#define KVM_CAP_DEVICE_ASSIGNMENT 17

-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 1/4] KVM: support VT-d device hotplug

[Han, Weidong]

Avi Kivity wrote:
> Han, Weidong wrote:
>>> Need a KVM_CAP_ to indicate this is available.
>>> 
>>> 
>> 
>> KVM_ASSIGN_PCI_DEVICE and KVM_ASSIGN_IRQ don't need a KVM_CAP_xxx.
>> Why do you need it for KVM_DEASSIGN_PCI_DEVICE? 
>> 
>> 
> 
> #define KVM_CAP_DEVICE_ASSIGNMENT 17

Do you mean add it like that:

#ifdef KVM_CAP_DEVICE_ASSIGNMENT
#define KVM_ASSIGN_PCI_DEVICE _IOR(KVMIO, 0x69, \
				                struct kvm_assigned_pci_dev)
#define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
  			             struct kvm_assigned_irq)
#define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
				                     struct kvm_assigned_pci_dev)
#endif

Regards,
Weidong

Re: [PATCH 1/4] KVM: support VT-d device hotplug

[Avi Kivity]

Han, Weidong wrote:
> Avi Kivity wrote:
>   
>> Han, Weidong wrote:
>>     
>>>> Need a KVM_CAP_ to indicate this is available.
>>>>
>>>>
>>>>         
>>> KVM_ASSIGN_PCI_DEVICE and KVM_ASSIGN_IRQ don't need a KVM_CAP_xxx.
>>> Why do you need it for KVM_DEASSIGN_PCI_DEVICE? 
>>>
>>>
>>>       
>> #define KVM_CAP_DEVICE_ASSIGNMENT 17
>>     
>
> Do you mean add it like that:
>
> #ifdef KVM_CAP_DEVICE_ASSIGNMENT
> #define KVM_ASSIGN_PCI_DEVICE _IOR(KVMIO, 0x69, \
> 				                struct kvm_assigned_pci_dev)
> #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
>   			             struct kvm_assigned_irq)
> #define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
> 				                     struct kvm_assigned_pci_dev)
> #endif
>   

No, sorry for being unclear.  KVM_CAP_DEVICE_ASSIGNMENT already exists 
in kvm.h

This is how KVM_CAP_ works:

- kvm.h defines KVM_CAP_DEVICE_ASSIGNMENT
- userspace compiles device assignment code only if it sees 
KVM_CAP_DEVICE_ASSIGNMENT in kvm.h
- kvm.ko returns nonzero to ioctl(KVM_CHECK_EXTENSION, 
KVM_CAP_DEVICE_ASSIGNMENT) if it supports device assignment (recent 
enough module, iommu found)
- userspace runs device assignment code only if above ioctl passes

We need something similar to deassignment, since it didn't arrive at the 
same time as assignment.

-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 1/4] KVM: support VT-d device hotplug

[Han, Weidong]

Avi Kivity wrote:
> Han, Weidong wrote:
>> Avi Kivity wrote:
>> 
>>> Han, Weidong wrote:
>>> 
>>>>> Need a KVM_CAP_ to indicate this is available.
>>>>> 
>>>>> 
>>>>> 
>>>> KVM_ASSIGN_PCI_DEVICE and KVM_ASSIGN_IRQ don't need a KVM_CAP_xxx.
>>>> Why do you need it for KVM_DEASSIGN_PCI_DEVICE?
>>>> 
>>>> 
>>>> 
>>> #define KVM_CAP_DEVICE_ASSIGNMENT 17
>>> 
>> 
>> Do you mean add it like that:
>> 
>> #ifdef KVM_CAP_DEVICE_ASSIGNMENT
>> #define KVM_ASSIGN_PCI_DEVICE _IOR(KVMIO, 0x69, \
>> 				                struct kvm_assigned_pci_dev)
>> #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
>>   			             struct kvm_assigned_irq)
>> #define KVM_DEASSIGN_PCI_DEVICE _IOR(KVMIO, 0x71, \
>> 				                     struct kvm_assigned_pci_dev) #endif
>> 
> 
> No, sorry for being unclear.  KVM_CAP_DEVICE_ASSIGNMENT already exists
> in kvm.h
> 
> This is how KVM_CAP_ works:
> 
> - kvm.h defines KVM_CAP_DEVICE_ASSIGNMENT
> - userspace compiles device assignment code only if it sees
> KVM_CAP_DEVICE_ASSIGNMENT in kvm.h
> - kvm.ko returns nonzero to ioctl(KVM_CHECK_EXTENSION,
> KVM_CAP_DEVICE_ASSIGNMENT) if it supports device assignment (recent
> enough module, iommu found)
> - userspace runs device assignment code only if above ioctl passes
> 
> We need something similar to deassignment, since it didn't arrive at
> the same time as assignment.

In my patches, deassignment already does the similar thing as assignment. Can you point out where it is missed?

Regards,
Weidong

Re: [PATCH 1/4] KVM: support VT-d device hotplug

[Avi Kivity]

Han, Weidong wrote:
>> This is how KVM_CAP_ works:
>>
>> - kvm.h defines KVM_CAP_DEVICE_ASSIGNMENT
>> - userspace compiles device assignment code only if it sees
>> KVM_CAP_DEVICE_ASSIGNMENT in kvm.h
>> - kvm.ko returns nonzero to ioctl(KVM_CHECK_EXTENSION,
>> KVM_CAP_DEVICE_ASSIGNMENT) if it supports device assignment (recent
>> enough module, iommu found)
>> - userspace runs device assignment code only if above ioctl passes
>>
>> We need something similar to deassignment, since it didn't arrive at
>> the same time as assignment.
>>     
>
> In my patches, deassignment already does the similar thing as assignment. Can you point out where it is missed?
>   

It needs a new KVM_CAP_ symbol.  2.6.28 has KVM_CAP_DEVICE_ASSIGNMENT 
defined, but will not have the deassignment code.

-- 
error compiling committee.c: too many arguments to function

RE: [PATCH 1/4] KVM: support VT-d device hotplug

[Han, Weidong]

Avi Kivity wrote:
> Han, Weidong wrote:
>>> This is how KVM_CAP_ works:
>>> 
>>> - kvm.h defines KVM_CAP_DEVICE_ASSIGNMENT
>>> - userspace compiles device assignment code only if it sees
>>> KVM_CAP_DEVICE_ASSIGNMENT in kvm.h
>>> - kvm.ko returns nonzero to ioctl(KVM_CHECK_EXTENSION,
>>> KVM_CAP_DEVICE_ASSIGNMENT) if it supports device assignment (recent
>>> enough module, iommu found) - userspace runs device assignment code
>>> only if above ioctl passes 
>>> 
>>> We need something similar to deassignment, since it didn't arrive
>>> at the same time as assignment. 
>>> 
>> 
>> In my patches, deassignment already does the similar thing as
>> assignment. Can you point out where it is missed? 
>> 
> 
> It needs a new KVM_CAP_ symbol.  2.6.28 has KVM_CAP_DEVICE_ASSIGNMENT
> defined, but will not have the deassignment code.

In fact, without multiple device assignment, VT-d hotplug cannot be supported cleanly. Given hot add a device under a different iommu, it will be mapped in a wrong iommu, thus it won't work. I will start to implement multiple device assignment first. 

Regards,
Weidong

Re: [PATCH 1/4] KVM: support VT-d device hotplug

[Avi Kivity]

Han, Weidong wrote:
> In fact, without multiple device assignment, VT-d hotplug cannot be supported cleanly. Given hot add a device under a different iommu, it will be mapped in a wrong iommu, thus it won't work. I will start to implement multiple device assignment first. 
>   

Yes, the 2.6.28 implementation has some weaknesses.  Please work with 
Joerg Roedel so that it fits within his iommu api.

-- 
error compiling committee.c: too many arguments to function