On Tue, 2008-07-29 at 14:49 +0530, Amit Shah wrote:
> * On Monday 28 Jul 2008 21:56:26 Ben-Ami Yassour wrote:
>
> > +static int kvm_vm_ioctl_assign_device(struct kvm *kvm,
> > + struct kvm_assigned_pci_dev *assigned_dev)
> > +{
>
>
> > + if (pci_enable_device(dev)) {
> > + printk(KERN_INFO "%s: Could not enable PCI device\n", __func__);
> > + r = -EBUSY;
> > + goto out_put;
> > + }
> > + r = pci_request_regions(dev, "kvm_assigned_device");
> > + if (r) {
> > + printk(KERN_INFO "%s: Could not get access to device regions\n",
> > + __func__);
> > + goto out_disable;
>
> Shouldn't disable here unconditionally (see my comment earlier to the
> previous
> patch).
Why? the device should not be used by the host at the same time.
What is the condition that you were thinking of?
>
> > +static void kvm_free_assigned_devices(struct kvm *kvm)
> > +{
> > + struct list_head *ptr, *ptr2;
> > + struct kvm_assigned_dev_kernel *assigned_dev;
> > +
> > + list_for_each_safe(ptr, ptr2, &kvm->arch.assigned_dev_head) {
> > + assigned_dev = list_entry(ptr,
> > + struct kvm_assigned_dev_kernel,
> > + list);
> > +
> > + if (irqchip_in_kernel(kvm) && assigned_dev->irq_requested) {
> > + free_irq(assigned_dev->host_irq,
> > + (void *)assigned_dev);
> > +
> > + kvm_unregister_irq_ack_notifier(kvm,
> > + &assigned_dev->
> > + ack_notifier);
> > + }
>
> Unregister the notifier before freeing irq
I don't think that there is any importance to the order in this case,
but just in case, the order should be in reverse to the initialization
order, which is the case here.
Regards,
Ben
--
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
