On 2019/7/10 上午11:36, Longpeng (Mike) wrote:
在 2019/7/10 11:25, Jason Wang 写道:
On 2019/7/8 下午5:47, Dr. David Alan Gilbert wrote:
* longpeng (longpe...@huawei.com) wrote:
Hi guys,
We found a qemu core in our testing environment, the assertion
'assert(bus->irq_count[i] == 0)' in pcibus_reset() was triggered and
the bus->irq_count[i] is '-1'.
Through analysis, it was happened after VM migration and we think
it was caused by the following sequence:
*Migration Source*
1. save bus pci.0 state, including irq_count[x] ( =0 , old )
2. save E1000:
e1000_pre_save
e1000_mit_timer
set_interrupt_cause
pci_set_irq --> update pci_dev->irq_state to 1 and
update bus->irq_count[x] to 1 ( new )
the irq_state sent to dest.
*Migration Dest*
1. Receive the irq_count[x] of pci.0 is 0 , but the irq_state of e1000 is 1.
2. If the e1000 need change irqline , it would call to pci_irq_handler(),
the irq_state maybe change to 0 and bus->irq_count[x] will become
-1 in this situation.
3. do VM reboot then the assertion will be triggered.
We also found some guys faced the similar problem:
[1] https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02525.html
[2] https://bugs.launchpad.net/qemu/+bug/1702621
Is there some patches to fix this problem ?
I don't remember any.
Can we save pcibus state after all the pci devs are saved ?
Does this problem only happen with e1000? I think so.
If it's only e1000 I think we should fix it - I think once the VM is
stopped for doing the device migration it shouldn't be raising
interrupts.
I wonder maybe we can simply fix this by no setting ICS on pre_save() but
scheduling mit timer unconditionally in post_load().
I also think this is a bug of e1000 because we find more cores with the same
frame thease days.
I'm not familiar with e1000 so hope someone could fix it, thanks. :)
Draft a path in attachment, please test.
Thanks
Thanks
Dave
Thanks,
Longpeng(Mike)
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
.
>From afe9258486672d76d7bf133ac9032a0d457bcd0b Mon Sep 17 00:00:00 2001
From: Jason Wang <jasow...@redhat.com>
Date: Wed, 10 Jul 2019 11:52:53 +0800
Subject: [PATCH] e1000: don't raise interrupt in pre_save()
We should not raise any interrupt after VM has been stopped but this
is what e1000 currently did when mit timer is active in
pre_save(). Fixing this by scheduling a timer in post_load() which can
make sure the interrupt was raised when VM is running.
Signed-off-by: Jason Wang <jasow...@redhat.com>
---
hw/net/e1000.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 1dc1466332..a023ceb27c 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1381,11 +1381,6 @@ static int e1000_pre_save(void *opaque)
E1000State *s = opaque;
NetClientState *nc = qemu_get_queue(s->nic);
- /* If the mitigation timer is active, emulate a timeout now. */
- if (s->mit_timer_on) {
- e1000_mit_timer(s);
- }
-
/*
* If link is down and auto-negotiation is supported and ongoing,
* complete auto-negotiation immediately. This allows us to look
@@ -1423,7 +1418,8 @@ static int e1000_post_load(void *opaque, int version_id)
s->mit_irq_level = false;
}
s->mit_ide = 0;
- s->mit_timer_on = false;
+ s->mit_timer_on = true;
+ timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
/* nc.link_down can't be migrated, so infer link_down according
* to link status bit in mac_reg[STATUS].
--
2.19.1
