* On Wednesday 30 Jul 2008 17:28:01 Ben-Ami Yassour wrote:
> On Wed, 2008-07-30 at 11:33 +0530, Amit Shah wrote:
> > * On Tuesday 29 July 2008 15:08:27 Ben-Ami Yassour wrote:
> > > 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.
> >
> > Just that we shouldn't accept any new interrupts to ack because we're
> > going to free the irq as the next step. This is more relevant now that we
> > don't have the rwlock for the device assignment structures. If the work
> > gets scheduled after we free the resources, it's going to bomb:
>
> These are two separate things.
> The ack notifier is called when the guest is signaling the virutal
> IOAPIC with EOI, and it has no direct relation with the host interrupt
> handler.
Of course. You're right.
--
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
