Ben-Ami Yassour wrote:
> On Thu, 2008-08-07 at 09:21 +0800, Han, Weidong wrote:
>> 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.
>>
>
> I agree that the detach call is not the ideal solution.
> On the other hand, I don't think that forcing the user to reboot is
> reasonable.
> Another approach is to update the VT-d driver code in such a way that
> will make sure that the context is cleared when the driver is removed,
> e.g. tie it to the device release path.
>
Okay, we can consider this when implement dummy driver to hide/unbind
passthrough devices from host. And also, we need to define a clear and
friendly VT-d usage later.
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
