Sheng,
Thanks for the comments, I sent an updated version of the patches.
Please see reply below.
Thanks,
Ben
On Thu, 2008-07-17 at 10:00 +0800, Yang, Sheng wrote:
> Some comments below. :)
>
> On Wednesday 16 July 2008 23:56:50 Ben-Ami Yassour wrote:
> > + __func__);
> > + goto out_put;
>
> pci_disable_device()?
fixed in the new version.
>
> > + }
> > + match->pt_dev.guest.busnr = pci_pt_dev->guest.busnr;
> > + match->pt_dev.guest.devfn = pci_pt_dev->guest.devfn;
> > + match->pt_dev.host.busnr = pci_pt_dev->host.busnr;
> > + match->pt_dev.host.devfn = pci_pt_dev->host.devfn;
> > + match->pt_dev.dev = dev;
> > +
> > + write_lock(&kvm_pci_pt_lock);
> > +
> > + INIT_WORK(&match->pt_dev.int_work.work, kvm_pci_pt_int_work_fn);
> > + INIT_WORK(&match->pt_dev.ack_work.work, kvm_pci_pt_ack_work_fn);
> > +
> > + match->pt_dev.kvm = kvm;
> > + match->pt_dev.int_work.pt_dev = &match->pt_dev;
> > + match->pt_dev.ack_work.pt_dev = &match->pt_dev;
> > +
> > + list_add(&match->list, &kvm->arch.pci_pt_dev_head);
> > +
> > + write_unlock(&kvm_pci_pt_lock);
> > +
> > + if (irqchip_in_kernel(kvm)) {
> > + match->pt_dev.guest.irq = pci_pt_dev->guest.irq;
> > + match->pt_dev.host.irq = dev->irq;
> > + if (kvm->arch.vioapic)
> > + kvm->arch.vioapic->ack_notifier = kvm_pci_pt_ack_irq;
> > + if (kvm->arch.vpic)
> > + kvm->arch.vpic->ack_notifier = kvm_pci_pt_ack_irq;
> > +
> > + /* Even though this is PCI, we don't want to use shared
> > + * interrupts. Sharing host devices with guest-assigned devices
> > + * on the same interrupt line is not a happy situation: there
> > + * are going to be long delays in accepting, acking, etc.
> > + */
> > + if (request_irq(dev->irq, kvm_pci_pt_dev_intr, 0,
> > + "kvm_pt_device", (void *)&match->pt_dev)) {
> > + printk(KERN_INFO "%s: couldn't allocate irq for pv "
> > + "device\n", __func__);
> > + r = -EIO;
> > + goto out_list_del;
> > + }
>
> PCI memory region has not been freed if request_irq() fail.
>
fixed in the new version.
> > + struct list_head *ptr, *ptr2;
> > + struct kvm_pci_pt_dev_list *pci_pt_dev;
> > +
> > + write_lock(&kvm_pci_pt_lock);
> > + list_for_each_safe(ptr, ptr2, &kvm->arch.pci_pt_dev_head) {
> > + pci_pt_dev = list_entry(ptr, struct kvm_pci_pt_dev_list, list);
> > +
> > + if (irqchip_in_kernel(kvm) && pci_pt_dev->pt_dev.host.irq)
> > + free_irq(pci_pt_dev->pt_dev.host.irq,
> > + (void *)&pci_pt_dev->pt_dev);
> > +
> > + if (cancel_work_sync(&pci_pt_dev->pt_dev.int_work.work))
> > + /* We had pending work. That means we will have to take
> > + * care of kvm_put_kvm.
> > + */
> > + kvm_put_kvm(kvm);
>
> Cancel_work_sync() is might_sleep() within spinlock...
Due to other changes the lock was no longer needed here and removed.
--
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
