Hi,
I just got an oops (with 2.6.28-rc6) when running "qemu-kvm -S
-pcidevice ..." and immediately quitting rather than starting the guest.
The issue is that at this point ASSIGN_PCI_DEVICE has been called, but
not ASSIGN_IRQ, so kvm_unregister_irq_ack_notifier() oops when we try
and remove a notifier which hasn't already been added.
The fix is simple - use hlist_del_init() rather than hlist_del() - but I
also came across this patch in Avi's tree ...
On Mon, 2008-10-20 at 16:07 +0800, Sheng Yang wrote:
...
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index 55ad76e..9fbbdea 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -58,12 +58,16 @@ void kvm_notify_acked_irq(struct kvm *kvm, unsigned gsi)
> void kvm_register_irq_ack_notifier(struct kvm *kvm,
> struct kvm_irq_ack_notifier *kian)
> {
> + /* Must be called with in-kernel IRQ chip, otherwise it's nonsense */
> + ASSERT(irqchip_in_kernel(kvm));
This is a seriously ugly assertion - there is no reason for the IRQ ACK
notifier abstraction to know anything about when it is called, and it's
easy to verify that kvm_register_irq_ack_notifier() is only called with
the in-kernel irqchip ... it's only called in one place:
if (irqchip_in_kernel(kvm)) {
/* Register ack nofitier */
match->ack_notifier.gsi = -1;
match->ack_notifier.irq_acked =
kvm_assigned_dev_ack_irq;
kvm_register_irq_ack_notifier(kvm,
&match->ack_notifier);
> + ASSERT(kian);
This is bogus; the ack notifier structure is embedded in assigned device
structure, so we can never pass NULL here - it's not like it's a
dynamically allocated structure.
> hlist_add_head(&kian->link, &kvm->arch.irq_ack_notifier_list);
> }
>
> -void kvm_unregister_irq_ack_notifier(struct kvm *kvm,
> - struct kvm_irq_ack_notifier *kian)
> +void kvm_unregister_irq_ack_notifier(struct kvm_irq_ack_notifier *kian)
> {
> + if (!kian)
> + return;
> hlist_del(&kian->link);
This is where I think you were trying to fix the issue I saw ... but
again, it's bogus. We will never pass a NULL ack notifier struct, but we
may well pass one which hasn't been previously registered.
I'm going to follow up with a number of patches to clean some of this
up.
Cheers,
Mark.
--
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
