* [Qemu-devel] [PULL 2/6] rtl8139: simplify timer logic
2015-02-06 14:10 [Qemu-devel] [PULL 0/6] Net patches Stefan Hajnoczi
2015-02-06 14:10 ` [Qemu-devel] [PULL 1/6] MAINTAINERS: add Jason Wang as net subsystem maintainer Stefan Hajnoczi
@ 2015-02-06 14:10 ` Stefan Hajnoczi
2015-02-06 14:10 ` [Qemu-devel] [PULL 3/6] monitor: print hub port name during info network Stefan Hajnoczi
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2015-02-06 14:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Stefan Hajnoczi, Paolo Bonzini
From: Paolo Bonzini <pbonzini@redhat.com>
Pavel Dovgalyuk reports that TimerExpire and the timer are not restored
correctly on the receiving end of migration.
It is not clear to me whether this is really the case, but we can take
the occasion to get rid of the complicated code that computes PCSTimeout
on the fly upon changes to IntrStatus/IntrMask. Just always keep a
timer running, it will fire every ~130 seconds at most if the interrupt
is masked with TimerInt != 0.
This makes rtl8139_set_next_tctr_time idempotent (when the virtual clock
is stopped between two calls, as is the case during migration).
Tested with Frediano's qtest.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1421765099-26190-1-git-send-email-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/rtl8139.c | 77 ++++++++++++++++++++------------------------------------
1 file changed, 27 insertions(+), 50 deletions(-)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 6fa9e0a..b7b87a6 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -508,7 +508,6 @@ typedef struct RTL8139State {
/* PCI interrupt timer */
QEMUTimer *timer;
- int64_t TimerExpire;
MemoryRegion bar_io;
MemoryRegion bar_mem;
@@ -520,7 +519,7 @@ typedef struct RTL8139State {
/* Writes tally counters to memory via DMA */
static void RTL8139TallyCounters_dma_write(RTL8139State *s, dma_addr_t tc_addr);
-static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
+static void rtl8139_set_next_tctr_time(RTL8139State *s);
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
{
@@ -1282,6 +1281,7 @@ static void rtl8139_reset(DeviceState *d)
s->TCTR = 0;
s->TimerInt = 0;
s->TCTR_base = 0;
+ rtl8139_set_next_tctr_time(s);
/* reset tally counters */
RTL8139TallyCounters_clear(&s->tally_counters);
@@ -2652,7 +2652,6 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
s->IntrMask = val;
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
rtl8139_update_irq(s);
}
@@ -2687,13 +2686,7 @@ static void rtl8139_IntrStatus_write(RTL8139State *s, uint32_t val)
rtl8139_update_irq(s);
s->IntrStatus = newStatus;
- /*
- * Computing if we miss an interrupt here is not that correct but
- * considered that we should have had already an interrupt
- * and probably emulated is slower is better to assume this resetting was
- * done before testing on previous rtl8139_update_irq lead to IRQ losing
- */
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ rtl8139_set_next_tctr_time(s);
rtl8139_update_irq(s);
#endif
@@ -2701,8 +2694,6 @@ static void rtl8139_IntrStatus_write(RTL8139State *s, uint32_t val)
static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
{
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
-
uint32_t ret = s->IntrStatus;
DPRINTF("IntrStatus read(w) val=0x%04x\n", ret);
@@ -2885,43 +2876,32 @@ static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
}
}
-static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time)
+static void rtl8139_set_next_tctr_time(RTL8139State *s)
{
- int64_t pci_time, next_time;
- uint32_t low_pci;
+ const uint64_t ns_per_period =
+ muldiv64(0x100000000LL, get_ticks_per_sec(), PCI_FREQUENCY);
DPRINTF("entered rtl8139_set_next_tctr_time\n");
- if (s->TimerExpire && current_time >= s->TimerExpire) {
- s->IntrStatus |= PCSTimeout;
- rtl8139_update_irq(s);
- }
-
- /* Set QEMU timer only if needed that is
- * - TimerInt <> 0 (we have a timer)
- * - mask = 1 (we want an interrupt timer)
- * - irq = 0 (irq is not already active)
- * If any of above change we need to compute timer again
- * Also we must check if timer is passed without QEMU timer
+ /* This function is called at least once per period, so it is a good
+ * place to update the timer base.
+ *
+ * After one iteration of this loop the value in the Timer register does
+ * not change, but the device model is counting up by 2^32 ticks (approx.
+ * 130 seconds).
*/
- s->TimerExpire = 0;
- if (!s->TimerInt) {
- return;
- }
-
- pci_time = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
- get_ticks_per_sec());
- low_pci = pci_time & 0xffffffff;
- pci_time = pci_time - low_pci + s->TimerInt;
- if (low_pci >= s->TimerInt) {
- pci_time += 0x100000000LL;
+ while (s->TCTR_base + ns_per_period <= qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
+ s->TCTR_base += ns_per_period;
}
- next_time = s->TCTR_base + muldiv64(pci_time, get_ticks_per_sec(),
- PCI_FREQUENCY);
- s->TimerExpire = next_time;
- if ((s->IntrMask & PCSTimeout) != 0 && (s->IntrStatus & PCSTimeout) == 0) {
- timer_mod(s->timer, next_time);
+ if (!s->TimerInt) {
+ timer_del(s->timer);
+ } else {
+ uint64_t delta = muldiv64(s->TimerInt, get_ticks_per_sec(), PCI_FREQUENCY);
+ if (s->TCTR_base + delta <= qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
+ delta += ns_per_period;
+ }
+ timer_mod(s->timer, s->TCTR_base + delta);
}
}
@@ -2969,14 +2949,14 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
case Timer:
DPRINTF("TCTR Timer reset on write\n");
s->TCTR_base = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- rtl8139_set_next_tctr_time(s, s->TCTR_base);
+ rtl8139_set_next_tctr_time(s);
break;
case FlashReg:
DPRINTF("FlashReg TimerInt write val=0x%08x\n", val);
if (s->TimerInt != val) {
s->TimerInt = val;
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ rtl8139_set_next_tctr_time(s);
}
break;
@@ -3253,7 +3233,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, hwaddr addr)
static int rtl8139_post_load(void *opaque, int version_id)
{
RTL8139State* s = opaque;
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ rtl8139_set_next_tctr_time(s);
if (version_id < 4) {
s->cplus_enabled = s->CpCmd != 0;
}
@@ -3284,8 +3264,7 @@ static void rtl8139_pre_save(void *opaque)
RTL8139State* s = opaque;
int64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- /* set IntrStatus correctly */
- rtl8139_set_next_tctr_time(s, current_time);
+ /* for migration to older versions */
s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
get_ticks_per_sec());
s->rtl8139_mmio_io_addr_dummy = 0;
@@ -3452,7 +3431,7 @@ static void rtl8139_timer(void *opaque)
s->IntrStatus |= PCSTimeout;
rtl8139_update_irq(s);
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+ rtl8139_set_next_tctr_time(s);
}
static void pci_rtl8139_uninit(PCIDevice *dev)
@@ -3530,9 +3509,7 @@ static int pci_rtl8139_init(PCIDevice *dev)
s->cplus_txbuffer_len = 0;
s->cplus_txbuffer_offset = 0;
- s->TimerExpire = 0;
s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, rtl8139_timer, s);
- rtl8139_set_next_tctr_time(s, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
return 0;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PULL 3/6] monitor: print hub port name during info network
2015-02-06 14:10 [Qemu-devel] [PULL 0/6] Net patches Stefan Hajnoczi
2015-02-06 14:10 ` [Qemu-devel] [PULL 1/6] MAINTAINERS: add Jason Wang as net subsystem maintainer Stefan Hajnoczi
2015-02-06 14:10 ` [Qemu-devel] [PULL 2/6] rtl8139: simplify timer logic Stefan Hajnoczi
@ 2015-02-06 14:10 ` Stefan Hajnoczi
2015-02-06 14:10 ` [Qemu-devel] [PULL 4/6] net: remove the wrong comment in net_init_hubport() Stefan Hajnoczi
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2015-02-06 14:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jason Wang, Stefan Hajnoczi
From: Jason Wang <jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Message-id: 1422860798-17495-1-git-send-email-jasowang@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/hub.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/hub.c b/net/hub.c
index 7e0f2d6..ef09d5f 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -245,9 +245,12 @@ void net_hub_info(Monitor *mon)
QLIST_FOREACH(hub, &hubs, next) {
monitor_printf(mon, "hub %d\n", hub->id);
QLIST_FOREACH(port, &hub->ports, next) {
+ monitor_printf(mon, " \\ %s", port->nc.name);
if (port->nc.peer) {
- monitor_printf(mon, " \\ ");
+ monitor_printf(mon, ": ");
print_net_client(mon, port->nc.peer);
+ } else {
+ monitor_printf(mon, "\n");
}
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PULL 6/6] monitor: more accurate completion for host_net_remove()
2015-02-06 14:10 [Qemu-devel] [PULL 0/6] Net patches Stefan Hajnoczi
` (4 preceding siblings ...)
2015-02-06 14:10 ` [Qemu-devel] [PULL 5/6] net: del hub port when peer is deleted Stefan Hajnoczi
@ 2015-02-06 14:10 ` Stefan Hajnoczi
2015-02-06 16:10 ` [Qemu-devel] [PULL 0/6] Net patches Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2015-02-06 14:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Jason Wang, Stefan Hajnoczi, Luiz Capitulino
From: Jason Wang <jasowang@redhat.com>
Current completion for host_net_remove will show hub ports and clients
that were not peered with hub ports. Fix this.
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Message-id: 1422860798-17495-4-git-send-email-jasowang@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
monitor.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/monitor.c b/monitor.c
index 7e4f605..e6dc50a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4597,8 +4597,13 @@ void host_net_remove_completion(ReadLineState *rs, int nb_args, const char *str)
count = qemu_find_net_clients_except(NULL, ncs,
NET_CLIENT_OPTIONS_KIND_NIC, 255);
for (i = 0; i < count; i++) {
+ int id;
const char *name;
+ if (ncs[i]->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT ||
+ net_hub_id_for_client(ncs[i], &id)) {
+ continue;
+ }
name = ncs[i]->name;
if (!strncmp(str, name, len)) {
readline_add_completion(rs, name);
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread