Ben-Ami Yassour wrote:
> On Wed, 2008-08-06 at 17:12 +0800, Han, Weidong wrote:
>> Ben-Ami Yassour wrote:
>>> On Wed, 2008-08-06 at 14:18 +0800, Han, Weidong wrote:
>>>> Ben-Ami Yassour wrote:
>>>>> On Tue, 2008-08-05 at 22:46 +0800, Han, Weidong wrote:
>>>>>> Ben-Ami Yassour wrote:
>>>>>>> [Ben: fixed memory pinning]
>>>>>>>
>>>>>>> Signed-off-by: Kay, Allen M <[EMAIL PROTECTED]>
>>>>>>> Signed-off-by: Weidong Han <[EMAIL PROTECTED]>
>>>>>>> Signed-off-by: Ben-Ami Yassour <[EMAIL PROTECTED]>
>>>>>>> ---
>>>>>>> +
>>>>>>> +int kvm_iommu_map_guest(struct kvm *kvm,
>>>>>>> + struct kvm_assigned_dev_kernel
*assigned_dev)
>>>>>>> +{
>>>>>>> + struct pci_dev *pdev = NULL;
>>>>>>> + int rc;
>>>>>>> +
>>>>>>> + printk(KERN_DEBUG "VT-d direct map: host bdf =
%x:%x:%x\n",
>>>>>>> + assigned_dev->host_busnr,
>>>>>>> + PCI_SLOT(assigned_dev->host_devfn),
>>>>>>> + PCI_FUNC(assigned_dev->host_devfn));
>>>>>>> +
>>>>>>> + pdev = assigned_dev->dev;
>>>>>>> +
>>>>>>> + if (pdev == NULL) {
>>>>>>> + if (kvm->arch.intel_iommu_domain) {
>>>>>>> +
>>>>>> intel_iommu_domain_exit(kvm->arch.intel_iommu_domain);
>>>>>>> + kvm->arch.intel_iommu_domain = NULL;
>>>>>>> + }
>>>>>>> + return -ENODEV;
>>>>>>> + }
>>>>>>> +
>>>>>>> + kvm->arch.intel_iommu_domain =
>>>>>>> intel_iommu_domain_alloc(pdev); + + rc =
>>>>>>> kvm_iommu_map_memslots(kvm); + if (rc) {
>>>>>>> + kvm_iommu_unmap_memslots(kvm);
>>>>>>> + return rc;
>>>>>>> + }
>>>>>>> +
>>>>>>> + intel_iommu_detach_dev(kvm->arch.intel_iommu_domain,
>>>>>>> + pdev->bus->number, pdev->devfn);
>>>>>>
>>>>>> I think we should remove intel_iommu_detach_dev(), which detaches
>>>>>> device from iommu. If the device has been assigned to other
>>>>>> domain already, it should not be assigned to this domain. Maybe
>>>>>> we need to add a check whether the device has been assigned
>>>>>> already.
>>>>>
>>>>> The problem is that in the generic VT-d code, the IOMMU is not
>>>>> updated when a device is detached, for example when the driver
>>>>> releases the device. Consider the following scenario:
>>>>> 1. load device driver on host
>>>>> 2. use device on host
>>>>> 3. unload device driver on host
>>>>> 4. Start KVM and assign the device to the guest.
>>>>>
>>>>> In this case there is no clearing of the IOMMU on step 3, and we
>>>>> get Vt-d failures (if we remove the above call).
>>>>>
>>>>
>>>> I don't prefer this usage process. As we discussed before, there
>>>> should be a dummy driver to hide/unbind passthrough devices from
>>>> host, only these devices can be assigned. There should be a check
>>>> whether device is assignable or not. If the device is not hidden,
>>>> or has been assigned to other guest already, it cannot be
>>>> assigned. At this point, I perfer to remove the device driver
>>>> before restart host to avoid loading driver during booting.
>>>
>>> I agree, but this is an orthogonal issue. Checking if the device is
>>> assigned has to be part of the device assignment solution in
>>> general, it is not specific to VT-d.
>>
>> Yes, the check is general. When the check fails, should quit guest
>> creation, and prompt the reason to user.
>>
>>> As for VT-d detach, the point is that the generic VT-d code is not
>>> detaching the domain, so we have to do that in KVM.
>>
>> In intel_iommu_unmap_guest(), assigned devices will be detached from
>> the guest. I think it's enough.
>>
>>> Again, removing the device driver is not enough (with the current
>>> VT-d code).
>>
>> Why? It works for me. e.g. I removed e1000.ko before reboot host, it
>> works well. BTW, for USB assignment, I add "nousb" in host grub to
>> avoid loading driver for USB controllers, it also works.
> What about the scenario described above:
> 1. load device driver on host
> 2. use device on host
> 3. unload device driver on host
> 4. Start KVM and assign the device to the guest.
>
> Does it work for you as well?
>
I didn't try the scenario you described, it should not work if removing
the detach call. I means removing device driver before reboot host. We
should implement a mechanism to ensure assignable devices won't be set
in context, instead of adding the detach call unconditionally in
kvm_iommu_map_guest(). At this moment, I perfer to remove corresponding
device driver before reboot host to assign the device.
Randy (Weidong)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
