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.
Regards,
Ben
> 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
