On Wed, 2008-07-16 at 18:04 +0300, Avi Kivity wrote:
> Ben-Ami Yassour wrote:
> > +/* Stores information for identifying host PCI devices assigned to the
> > + * guest: this is used in the host kernel and in the userspace.
> > + */
> > +struct kvm_pci_pt_info {
> > + unsigned char busnr;
> > + unsigned int devfn;
> > + __u32 irq;
> > +};
> >
>
> Badly aligned. Please use __u32 for all fields, and add a __u32 padding
> field at the end so we have the same ABI for i386 and x86_64.
>
> Some pci devices support more than one irq. Should we make irq an
> array? Alternatively, decouple irq assignment from pci information
> and
> let userspace handle everything. This lets us assign non-pci devices
> (like serial ports, etc.).
Can we limit the first version to single irq devices, and handle this
after the merge?
> > * ioctls for vcpu fds
> > diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> > index 8ce93c7..0b5bc40 100644
> > --- a/virt/kvm/ioapic.c
> > +++ b/virt/kvm/ioapic.c
> > @@ -288,13 +288,22 @@ void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic,
> > int irq, int level)
> > static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int gsi)
> > {
> > union ioapic_redir_entry *ent;
> > + struct kvm_pci_pt_dev_list *match;
> > + unsigned long flags;
> >
> > ent = &ioapic->redirtbl[gsi];
> > ASSERT(ent->fields.trig_mode == IOAPIC_LEVEL_TRIG);
> >
> > ent->fields.remote_irr = 0;
> > - if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
> > - ioapic_service(ioapic, gsi);
> > +
> > + read_lock_irqsave(&kvm_pci_pt_lock, flags);
> > + match = kvm_find_pci_pt_dev(&ioapic->kvm->arch.pci_pt_dev_head, NULL,
> > + gsi, KVM_PT_SOURCE_IRQ_ACK);
> > + read_unlock_irqrestore(&kvm_pci_pt_lock, flags);
> > + if (!match) {
> > + if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
> > + ioapic_service(ioapic, gsi);
> > + }
> >
>
> What's device assignment code doing in the ioapic? This should be done
> through the notifier (if it needs to return a value, great).
>
This purpose of this code is to avoid the call to ioapic_service for
assigned devices, because it is not required and causes a double
injection of the interrupt.
Can you please explain why as part of eoi we inject a new interrupt?
Thanks,
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
