* [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
@ 2010-02-16 8:35 Frediano Ziglio
2010-02-19 21:50 ` Anthony Liguori
2010-02-19 23:24 ` Anthony Liguori
0 siblings, 2 replies; 12+ messages in thread
From: Frediano Ziglio @ 2010-02-16 8:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Frediano Ziglio
Add a QEMU timer only when needed (timeout status not set, timeout
irq wanted and timer set).
This patch is required for Darwin. Patch has been tested under
FreeBSD, Darwin and Linux.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
hw/rtl8139.c | 136 ++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 81 insertions(+), 55 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index f04dd54..e43c15c 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -41,6 +41,10 @@
* segmentation offloading
* Removed slirp.h dependency
* Added rx/tx buffer reset when enabling rx/tx operation
+ *
+ * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
+ * when strictly needed (required for for
+ * Darwin)
*/
#include "hw.h"
@@ -60,9 +64,6 @@
/* Calculate CRCs properly on Rx packets */
#define RTL8139_CALCULATE_RXCRC 1
-/* Uncomment to enable on-board timer interrupts */
-//#define RTL8139_ONBOARD_TIMER 1
-
#if defined(RTL8139_CALCULATE_RXCRC)
/* For crc32 */
#include <zlib.h>
@@ -491,9 +492,12 @@ typedef struct RTL8139State {
/* PCI interrupt timer */
QEMUTimer *timer;
+ int64_t TimerExpire;
} RTL8139State;
+static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
+
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
{
DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
@@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
s->IntrMask = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
}
static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
@@ -2555,12 +2561,22 @@ 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 loosing
+ */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
#endif
}
static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
{
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+
uint32_t ret = s->IntrStatus;
DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
@@ -2739,6 +2755,43 @@ 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)
+{
+ DEBUG_PRINT(("RTL8139: 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
+ */
+ s->TimerExpire = 0;
+ if (!s->TimerInt) {
+ return;
+ }
+
+ int64_t pci_time = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
+ get_ticks_per_sec());
+ uint32_t low_pci = pci_time & 0xffffffff;
+ pci_time = pci_time - low_pci + s->TimerInt;
+ if (low_pci >= s->TimerInt) {
+ pci_time += 0x100000000LL;
+ }
+ int64_t 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) {
+ qemu_mod_timer(s->timer, next_time);
+ }
+}
+
static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
{
RTL8139State *s = opaque;
@@ -2784,13 +2837,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
case Timer:
DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
- s->TCTR = 0;
s->TCTR_base = qemu_get_clock(vm_clock);
+ rtl8139_set_next_tctr_time(s, s->TCTR_base);
break;
case FlashReg:
DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
- s->TimerInt = val;
+ if (s->TimerInt != val) {
+ s->TimerInt = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+ }
break;
default:
@@ -3000,7 +3056,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
break;
case Timer:
- ret = s->TCTR;
+ ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
+ PCI_FREQUENCY, get_ticks_per_sec());
DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
break;
@@ -3105,6 +3162,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
static int rtl8139_post_load(void *opaque, int version_id)
{
RTL8139State* s = opaque;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
if (version_id < 4) {
s->cplus_enabled = s->CpCmd != 0;
}
@@ -3112,12 +3170,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
return 0;
}
+static void rtl8139_pre_save(void *opaque)
+{
+ RTL8139State* s = opaque;
+
+ // set IntrStatus correctly
+ int64_t current_time = qemu_get_clock(vm_clock);
+ rtl8139_set_next_tctr_time(s, current_time);
+ s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
+ get_ticks_per_sec());
+}
+
static const VMStateDescription vmstate_rtl8139 = {
.name = "rtl8139",
.version_id = 4,
.minimum_version_id = 3,
.minimum_version_id_old = 3,
.post_load = rtl8139_post_load,
+ .pre_save = rtl8139_pre_save,
.fields = (VMStateField []) {
VMSTATE_PCI_DEVICE(dev, RTL8139State),
VMSTATE_PARTIAL_BUFFER(phys, RTL8139State, 6),
@@ -3231,59 +3301,20 @@ static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
rtl8139_mmio_writel,
};
-static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
-{
- int64_t next_time = current_time +
- muldiv64(1, get_ticks_per_sec(), PCI_FREQUENCY);
- if (next_time <= current_time)
- next_time = current_time + 1;
- return next_time;
-}
-
-#ifdef RTL8139_ONBOARD_TIMER
static void rtl8139_timer(void *opaque)
{
RTL8139State *s = opaque;
- int is_timeout = 0;
-
- int64_t curr_time;
- uint32_t curr_tick;
-
if (!s->clock_enabled)
{
DEBUG_PRINT(("RTL8139: >>> timer: clock is not running\n"));
return;
}
- curr_time = qemu_get_clock(vm_clock);
-
- curr_tick = muldiv64(curr_time - s->TCTR_base, PCI_FREQUENCY,
- get_ticks_per_sec());
-
- if (s->TimerInt && curr_tick >= s->TimerInt)
- {
- if (s->TCTR < s->TimerInt || curr_tick < s->TCTR)
- {
- is_timeout = 1;
- }
- }
-
- s->TCTR = curr_tick;
-
-// DEBUG_PRINT(("RTL8139: >>> timer: tick=%08u\n", s->TCTR));
-
- if (is_timeout)
- {
- DEBUG_PRINT(("RTL8139: >>> timer: timeout tick=%08u\n", s->TCTR));
- s->IntrStatus |= PCSTimeout;
- rtl8139_update_irq(s);
- }
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,curr_time));
+ s->IntrStatus |= PCSTimeout;
+ rtl8139_update_irq(s);
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
}
-#endif /* RTL8139_ONBOARD_TIMER */
static void rtl8139_cleanup(VLANClientState *nc)
{
@@ -3301,10 +3332,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
qemu_free(s->cplus_txbuffer);
s->cplus_txbuffer = NULL;
}
-#ifdef RTL8139_ONBOARD_TIMER
qemu_del_timer(s->timer);
qemu_free_timer(s->timer);
-#endif
qemu_del_vlan_client(&s->nic->nc);
return 0;
}
@@ -3356,12 +3385,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
s->cplus_txbuffer_len = 0;
s->cplus_txbuffer_offset = 0;
-#ifdef RTL8139_ONBOARD_TIMER
+ s->TimerExpire = 0;
s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
-#endif /* RTL8139_ONBOARD_TIMER */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
return 0;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-16 8:35 [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139 Frediano Ziglio
@ 2010-02-19 21:50 ` Anthony Liguori
2010-02-19 22:22 ` Alexander Graf
2010-02-19 23:24 ` Anthony Liguori
1 sibling, 1 reply; 12+ messages in thread
From: Anthony Liguori @ 2010-02-19 21:50 UTC (permalink / raw)
To: Frediano Ziglio; +Cc: qemu-devel, Alexander Graf
On 02/16/2010 02:35 AM, Frediano Ziglio wrote:
> Add a QEMU timer only when needed (timeout status not set, timeout
> irq wanted and timer set).
>
> This patch is required for Darwin. Patch has been tested under
> FreeBSD, Darwin and Linux.
>
> Signed-off-by: Frediano Ziglio<freddy77@gmail.com>
>
Alex, I know you've played with this feature for OS X in the past.
Is this something we should be enabling unconditionally?
Regards,
Anthony Liguori
> ---
> hw/rtl8139.c | 136 ++++++++++++++++++++++++++++++++++-----------------------
> 1 files changed, 81 insertions(+), 55 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index f04dd54..e43c15c 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -41,6 +41,10 @@
> * segmentation offloading
> * Removed slirp.h dependency
> * Added rx/tx buffer reset when enabling rx/tx operation
> + *
> + * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
> + * when strictly needed (required for for
> + * Darwin)
> */
>
> #include "hw.h"
> @@ -60,9 +64,6 @@
> /* Calculate CRCs properly on Rx packets */
> #define RTL8139_CALCULATE_RXCRC 1
>
> -/* Uncomment to enable on-board timer interrupts */
> -//#define RTL8139_ONBOARD_TIMER 1
> -
> #if defined(RTL8139_CALCULATE_RXCRC)
> /* For crc32 */
> #include<zlib.h>
> @@ -491,9 +492,12 @@ typedef struct RTL8139State {
>
> /* PCI interrupt timer */
> QEMUTimer *timer;
> + int64_t TimerExpire;
>
> } RTL8139State;
>
> +static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
> +
> static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
> {
> DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
> @@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
>
> s->IntrMask = val;
>
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> rtl8139_update_irq(s);
> +
> }
>
> static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
> @@ -2555,12 +2561,22 @@ 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 loosing
> + */
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> rtl8139_update_irq(s);
> +
> #endif
> }
>
> static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
> {
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> +
> uint32_t ret = s->IntrStatus;
>
> DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
> @@ -2739,6 +2755,43 @@ 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)
> +{
> + DEBUG_PRINT(("RTL8139: 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
> + */
> + s->TimerExpire = 0;
> + if (!s->TimerInt) {
> + return;
> + }
> +
> + int64_t pci_time = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> + get_ticks_per_sec());
> + uint32_t low_pci = pci_time& 0xffffffff;
> + pci_time = pci_time - low_pci + s->TimerInt;
> + if (low_pci>= s->TimerInt) {
> + pci_time += 0x100000000LL;
> + }
> + int64_t 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) {
> + qemu_mod_timer(s->timer, next_time);
> + }
> +}
> +
> static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
> {
> RTL8139State *s = opaque;
> @@ -2784,13 +2837,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
>
> case Timer:
> DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
> - s->TCTR = 0;
> s->TCTR_base = qemu_get_clock(vm_clock);
> + rtl8139_set_next_tctr_time(s, s->TCTR_base);
> break;
>
> case FlashReg:
> DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
> - s->TimerInt = val;
> + if (s->TimerInt != val) {
> + s->TimerInt = val;
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> + }
> break;
>
> default:
> @@ -3000,7 +3056,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
> break;
>
> case Timer:
> - ret = s->TCTR;
> + ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
> + PCI_FREQUENCY, get_ticks_per_sec());
> DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
> break;
>
> @@ -3105,6 +3162,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
> static int rtl8139_post_load(void *opaque, int version_id)
> {
> RTL8139State* s = opaque;
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> if (version_id< 4) {
> s->cplus_enabled = s->CpCmd != 0;
> }
> @@ -3112,12 +3170,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
> return 0;
> }
>
> +static void rtl8139_pre_save(void *opaque)
> +{
> + RTL8139State* s = opaque;
> +
> + // set IntrStatus correctly
> + int64_t current_time = qemu_get_clock(vm_clock);
> + rtl8139_set_next_tctr_time(s, current_time);
> + s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> + get_ticks_per_sec());
> +}
> +
> static const VMStateDescription vmstate_rtl8139 = {
> .name = "rtl8139",
> .version_id = 4,
> .minimum_version_id = 3,
> .minimum_version_id_old = 3,
> .post_load = rtl8139_post_load,
> + .pre_save = rtl8139_pre_save,
> .fields = (VMStateField []) {
> VMSTATE_PCI_DEVICE(dev, RTL8139State),
> VMSTATE_PARTIAL_BUFFER(phys, RTL8139State, 6),
> @@ -3231,59 +3301,20 @@ static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
> rtl8139_mmio_writel,
> };
>
> -static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
> -{
> - int64_t next_time = current_time +
> - muldiv64(1, get_ticks_per_sec(), PCI_FREQUENCY);
> - if (next_time<= current_time)
> - next_time = current_time + 1;
> - return next_time;
> -}
> -
> -#ifdef RTL8139_ONBOARD_TIMER
> static void rtl8139_timer(void *opaque)
> {
> RTL8139State *s = opaque;
>
> - int is_timeout = 0;
> -
> - int64_t curr_time;
> - uint32_t curr_tick;
> -
> if (!s->clock_enabled)
> {
> DEBUG_PRINT(("RTL8139:>>> timer: clock is not running\n"));
> return;
> }
>
> - curr_time = qemu_get_clock(vm_clock);
> -
> - curr_tick = muldiv64(curr_time - s->TCTR_base, PCI_FREQUENCY,
> - get_ticks_per_sec());
> -
> - if (s->TimerInt&& curr_tick>= s->TimerInt)
> - {
> - if (s->TCTR< s->TimerInt || curr_tick< s->TCTR)
> - {
> - is_timeout = 1;
> - }
> - }
> -
> - s->TCTR = curr_tick;
> -
> -// DEBUG_PRINT(("RTL8139:>>> timer: tick=%08u\n", s->TCTR));
> -
> - if (is_timeout)
> - {
> - DEBUG_PRINT(("RTL8139:>>> timer: timeout tick=%08u\n", s->TCTR));
> - s->IntrStatus |= PCSTimeout;
> - rtl8139_update_irq(s);
> - }
> -
> - qemu_mod_timer(s->timer,
> - rtl8139_get_next_tctr_time(s,curr_time));
> + s->IntrStatus |= PCSTimeout;
> + rtl8139_update_irq(s);
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> }
> -#endif /* RTL8139_ONBOARD_TIMER */
>
> static void rtl8139_cleanup(VLANClientState *nc)
> {
> @@ -3301,10 +3332,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
> qemu_free(s->cplus_txbuffer);
> s->cplus_txbuffer = NULL;
> }
> -#ifdef RTL8139_ONBOARD_TIMER
> qemu_del_timer(s->timer);
> qemu_free_timer(s->timer);
> -#endif
> qemu_del_vlan_client(&s->nic->nc);
> return 0;
> }
> @@ -3356,12 +3385,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
> s->cplus_txbuffer_len = 0;
> s->cplus_txbuffer_offset = 0;
>
> -#ifdef RTL8139_ONBOARD_TIMER
> + s->TimerExpire = 0;
> s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);
> -
> - qemu_mod_timer(s->timer,
> - rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
> -#endif /* RTL8139_ONBOARD_TIMER */
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-19 21:50 ` Anthony Liguori
@ 2010-02-19 22:22 ` Alexander Graf
2010-02-19 23:26 ` Anthony Liguori
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2010-02-19 22:22 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Frediano Ziglio, qemu-devel
On 19.02.2010, at 22:50, Anthony Liguori wrote:
> On 02/16/2010 02:35 AM, Frediano Ziglio wrote:
>> Add a QEMU timer only when needed (timeout status not set, timeout
>> irq wanted and timer set).
>>
>> This patch is required for Darwin. Patch has been tested under
>> FreeBSD, Darwin and Linux.
>>
>> Signed-off-by: Frediano Ziglio<freddy77@gmail.com>
>>
>
> Alex, I know you've played with this feature for OS X in the past.
>
> Is this something we should be enabling unconditionally?
I've played with it and realized that
1) OSX's rtl8139 driver uses this timer instead of tx/rx interrupts to process the queue
2) it was broken
I haven't gotten around to test if this patch makes it work, but I'd suppose so.
So yes, I think it should be enabled by default. It's a feature real hw has, so we should emulate it.
Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-16 8:35 [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139 Frediano Ziglio
2010-02-19 21:50 ` Anthony Liguori
@ 2010-02-19 23:24 ` Anthony Liguori
1 sibling, 0 replies; 12+ messages in thread
From: Anthony Liguori @ 2010-02-19 23:24 UTC (permalink / raw)
To: Frediano Ziglio; +Cc: qemu-devel
On 02/16/2010 02:35 AM, Frediano Ziglio wrote:
> Add a QEMU timer only when needed (timeout status not set, timeout
> irq wanted and timer set).
>
> This patch is required for Darwin. Patch has been tested under
> FreeBSD, Darwin and Linux.
>
> Signed-off-by: Frediano Ziglio<freddy77@gmail.com>
> ---
> hw/rtl8139.c | 136 ++++++++++++++++++++++++++++++++++-----------------------
> 1 files changed, 81 insertions(+), 55 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index f04dd54..e43c15c 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -41,6 +41,10 @@
> * segmentation offloading
> * Removed slirp.h dependency
> * Added rx/tx buffer reset when enabling rx/tx operation
> + *
> + * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
> + * when strictly needed (required for for
> + * Darwin)
> */
>
> #include "hw.h"
> @@ -60,9 +64,6 @@
> /* Calculate CRCs properly on Rx packets */
> #define RTL8139_CALCULATE_RXCRC 1
>
> -/* Uncomment to enable on-board timer interrupts */
> -//#define RTL8139_ONBOARD_TIMER 1
> -
> #if defined(RTL8139_CALCULATE_RXCRC)
> /* For crc32 */
> #include<zlib.h>
> @@ -491,9 +492,12 @@ typedef struct RTL8139State {
>
> /* PCI interrupt timer */
> QEMUTimer *timer;
> + int64_t TimerExpire;
>
> } RTL8139State;
>
> +static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
> +
> static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
> {
> DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
> @@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
>
> s->IntrMask = val;
>
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> rtl8139_update_irq(s);
> +
> }
>
> static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
> @@ -2555,12 +2561,22 @@ 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 loosing
> + */
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> rtl8139_update_irq(s);
> +
> #endif
> }
>
> static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
> {
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> +
> uint32_t ret = s->IntrStatus;
>
> DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
> @@ -2739,6 +2755,43 @@ 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)
> +{
> + DEBUG_PRINT(("RTL8139: 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
> + */
> + s->TimerExpire = 0;
> + if (!s->TimerInt) {
> + return;
> + }
> +
> + int64_t pci_time = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> + get_ticks_per_sec());
> + uint32_t low_pci = pci_time& 0xffffffff;
>
Please don't mix variable declarations in code.
> + pci_time = pci_time - low_pci + s->TimerInt;
> + if (low_pci>= s->TimerInt) {
> + pci_time += 0x100000000LL;
> + }
> + int64_t 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) {
> + qemu_mod_timer(s->timer, next_time);
> + }
> +}
> +
> static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
> {
> RTL8139State *s = opaque;
> @@ -2784,13 +2837,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
>
> case Timer:
> DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
> - s->TCTR = 0;
> s->TCTR_base = qemu_get_clock(vm_clock);
> + rtl8139_set_next_tctr_time(s, s->TCTR_base);
> break;
>
> case FlashReg:
> DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
> - s->TimerInt = val;
> + if (s->TimerInt != val) {
> + s->TimerInt = val;
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> + }
> break;
>
> default:
> @@ -3000,7 +3056,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
> break;
>
> case Timer:
> - ret = s->TCTR;
> + ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
> + PCI_FREQUENCY, get_ticks_per_sec());
> DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
> break;
>
> @@ -3105,6 +3162,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
> static int rtl8139_post_load(void *opaque, int version_id)
> {
> RTL8139State* s = opaque;
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> if (version_id< 4) {
> s->cplus_enabled = s->CpCmd != 0;
> }
> @@ -3112,12 +3170,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
> return 0;
> }
>
> +static void rtl8139_pre_save(void *opaque)
> +{
> + RTL8139State* s = opaque;
> +
> + // set IntrStatus correctly
>
Please avoid C99 comments.
> + int64_t current_time = qemu_get_clock(vm_clock);
> + rtl8139_set_next_tctr_time(s, current_time);
> + s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> + get_ticks_per_sec());
> +}
>
Usually, timers have to be saved as part of save/restore in order for
them to fire correctly when restarted. I suspect this breaks save/restore.
I'd like to see some performance data too. I'm concerned that a change
like this could have a significant impact on performance.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-19 22:22 ` Alexander Graf
@ 2010-02-19 23:26 ` Anthony Liguori
2010-02-20 5:52 ` Igor Kovalenko
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Anthony Liguori @ 2010-02-19 23:26 UTC (permalink / raw)
To: Alexander Graf; +Cc: Frediano Ziglio, qemu-devel
On 02/19/2010 04:22 PM, Alexander Graf wrote:
> On 19.02.2010, at 22:50, Anthony Liguori wrote:
>
>
>> On 02/16/2010 02:35 AM, Frediano Ziglio wrote:
>>
>>> Add a QEMU timer only when needed (timeout status not set, timeout
>>> irq wanted and timer set).
>>>
>>> This patch is required for Darwin. Patch has been tested under
>>> FreeBSD, Darwin and Linux.
>>>
>>> Signed-off-by: Frediano Ziglio<freddy77@gmail.com>
>>>
>>>
>> Alex, I know you've played with this feature for OS X in the past.
>>
>> Is this something we should be enabling unconditionally?
>>
> I've played with it and realized that
>
> 1) OSX's rtl8139 driver uses this timer instead of tx/rx interrupts to process the queue
> 2) it was broken
>
> I haven't gotten around to test if this patch makes it work, but I'd suppose so.
>
> So yes, I think it should be enabled by default. It's a feature real hw has, so we should emulate it.
>
The thing that concerns me is why it was disabled by default. Igor,
what was the reasoning for disabling it if you recall?
Regards,
Anthony Liguori
> Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-19 23:26 ` Anthony Liguori
@ 2010-02-20 5:52 ` Igor Kovalenko
2010-02-20 16:14 ` Frediano Ziglio
2010-02-20 16:06 ` Frediano Ziglio
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Igor Kovalenko @ 2010-02-20 5:52 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Alexander Graf, Frediano Ziglio
On Sat, Feb 20, 2010 at 2:26 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 02/19/2010 04:22 PM, Alexander Graf wrote:
>>
>> On 19.02.2010, at 22:50, Anthony Liguori wrote:
>>
>>
>>>
>>> On 02/16/2010 02:35 AM, Frediano Ziglio wrote:
>>>
>>>>
>>>> Add a QEMU timer only when needed (timeout status not set, timeout
>>>> irq wanted and timer set).
>>>>
>>>> This patch is required for Darwin. Patch has been tested under
>>>> FreeBSD, Darwin and Linux.
>>>>
>>>> Signed-off-by: Frediano Ziglio<freddy77@gmail.com>
>>>>
>>>>
>>>
>>> Alex, I know you've played with this feature for OS X in the past.
>>>
>>> Is this something we should be enabling unconditionally?
>>>
>>
>> I've played with it and realized that
>>
>> 1) OSX's rtl8139 driver uses this timer instead of tx/rx interrupts to
>> process the queue
>> 2) it was broken
>>
>> I haven't gotten around to test if this patch makes it work, but I'd
>> suppose so.
>>
>> So yes, I think it should be enabled by default. It's a feature real hw
>> has, so we should emulate it.
>>
>
> The thing that concerns me is why it was disabled by default. Igor, what
> was the reasoning for disabling it if you recall?
It was very inefficient pci timer implementation which fired callback
frequently.
Once it was found that (freebsd?) was relying on pci timer in realtek driver
to communicate we just added missing piece.
Now this patch fixes that inefficiency issue.
--
Kind regards,
Igor V. Kovalenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-19 23:26 ` Anthony Liguori
2010-02-20 5:52 ` Igor Kovalenko
@ 2010-02-20 16:06 ` Frediano Ziglio
2010-02-20 17:13 ` Anthony Liguori
2010-02-20 17:27 ` Frediano Ziglio
2010-02-20 17:50 ` Frediano Ziglio
3 siblings, 1 reply; 12+ messages in thread
From: Frediano Ziglio @ 2010-02-20 16:06 UTC (permalink / raw)
To: anthony, igor.v.kovalenko; +Cc: qemu-devel, Frediano Ziglio
Add a QEMU timer only when needed (timeout status not set, timeout
irq wanted and timer set).
This patch is required for Darwin. Patch has been tested under
FreeBSD, Darwin and Linux.
---
hw/rtl8139.c | 139 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 84 insertions(+), 55 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index f04dd54..72e2242 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -41,6 +41,10 @@
* segmentation offloading
* Removed slirp.h dependency
* Added rx/tx buffer reset when enabling rx/tx operation
+ *
+ * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
+ * when strictly needed (required for for
+ * Darwin)
*/
#include "hw.h"
@@ -60,9 +64,6 @@
/* Calculate CRCs properly on Rx packets */
#define RTL8139_CALCULATE_RXCRC 1
-/* Uncomment to enable on-board timer interrupts */
-//#define RTL8139_ONBOARD_TIMER 1
-
#if defined(RTL8139_CALCULATE_RXCRC)
/* For crc32 */
#include <zlib.h>
@@ -491,9 +492,12 @@ typedef struct RTL8139State {
/* PCI interrupt timer */
QEMUTimer *timer;
+ int64_t TimerExpire;
} RTL8139State;
+static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
+
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
{
DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
@@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
s->IntrMask = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
}
static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
@@ -2555,12 +2561,22 @@ 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 loosing
+ */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
#endif
}
static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
{
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+
uint32_t ret = s->IntrStatus;
DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
@@ -2739,6 +2755,46 @@ 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)
+{
+ int64_t pci_time, next_time;
+ uint32_t low_pci;
+
+ DEBUG_PRINT(("RTL8139: 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
+ */
+ 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;
+ }
+ 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) {
+ qemu_mod_timer(s->timer, next_time);
+ }
+}
+
static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
{
RTL8139State *s = opaque;
@@ -2784,13 +2840,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
case Timer:
DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
- s->TCTR = 0;
s->TCTR_base = qemu_get_clock(vm_clock);
+ rtl8139_set_next_tctr_time(s, s->TCTR_base);
break;
case FlashReg:
DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
- s->TimerInt = val;
+ if (s->TimerInt != val) {
+ s->TimerInt = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+ }
break;
default:
@@ -3000,7 +3059,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
break;
case Timer:
- ret = s->TCTR;
+ ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
+ PCI_FREQUENCY, get_ticks_per_sec());
DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
break;
@@ -3105,6 +3165,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
static int rtl8139_post_load(void *opaque, int version_id)
{
RTL8139State* s = opaque;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
if (version_id < 4) {
s->cplus_enabled = s->CpCmd != 0;
}
@@ -3112,12 +3173,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
return 0;
}
+static void rtl8139_pre_save(void *opaque)
+{
+ RTL8139State* s = opaque;
+ int64_t current_time = qemu_get_clock(vm_clock);
+
+ /* set IntrStatus correctly */
+ rtl8139_set_next_tctr_time(s, current_time);
+ s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
+ get_ticks_per_sec());
+}
+
static const VMStateDescription vmstate_rtl8139 = {
.name = "rtl8139",
.version_id = 4,
.minimum_version_id = 3,
.minimum_version_id_old = 3,
.post_load = rtl8139_post_load,
+ .pre_save = rtl8139_pre_save,
.fields = (VMStateField []) {
VMSTATE_PCI_DEVICE(dev, RTL8139State),
VMSTATE_PARTIAL_BUFFER(phys, RTL8139State, 6),
@@ -3231,59 +3304,20 @@ static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
rtl8139_mmio_writel,
};
-static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
-{
- int64_t next_time = current_time +
- muldiv64(1, get_ticks_per_sec(), PCI_FREQUENCY);
- if (next_time <= current_time)
- next_time = current_time + 1;
- return next_time;
-}
-
-#ifdef RTL8139_ONBOARD_TIMER
static void rtl8139_timer(void *opaque)
{
RTL8139State *s = opaque;
- int is_timeout = 0;
-
- int64_t curr_time;
- uint32_t curr_tick;
-
if (!s->clock_enabled)
{
DEBUG_PRINT(("RTL8139: >>> timer: clock is not running\n"));
return;
}
- curr_time = qemu_get_clock(vm_clock);
-
- curr_tick = muldiv64(curr_time - s->TCTR_base, PCI_FREQUENCY,
- get_ticks_per_sec());
-
- if (s->TimerInt && curr_tick >= s->TimerInt)
- {
- if (s->TCTR < s->TimerInt || curr_tick < s->TCTR)
- {
- is_timeout = 1;
- }
- }
-
- s->TCTR = curr_tick;
-
-// DEBUG_PRINT(("RTL8139: >>> timer: tick=%08u\n", s->TCTR));
-
- if (is_timeout)
- {
- DEBUG_PRINT(("RTL8139: >>> timer: timeout tick=%08u\n", s->TCTR));
- s->IntrStatus |= PCSTimeout;
- rtl8139_update_irq(s);
- }
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,curr_time));
+ s->IntrStatus |= PCSTimeout;
+ rtl8139_update_irq(s);
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
}
-#endif /* RTL8139_ONBOARD_TIMER */
static void rtl8139_cleanup(VLANClientState *nc)
{
@@ -3301,10 +3335,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
qemu_free(s->cplus_txbuffer);
s->cplus_txbuffer = NULL;
}
-#ifdef RTL8139_ONBOARD_TIMER
qemu_del_timer(s->timer);
qemu_free_timer(s->timer);
-#endif
qemu_del_vlan_client(&s->nic->nc);
return 0;
}
@@ -3356,12 +3388,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
s->cplus_txbuffer_len = 0;
s->cplus_txbuffer_offset = 0;
-#ifdef RTL8139_ONBOARD_TIMER
+ s->TimerExpire = 0;
s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
-#endif /* RTL8139_ONBOARD_TIMER */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
return 0;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-20 5:52 ` Igor Kovalenko
@ 2010-02-20 16:14 ` Frediano Ziglio
0 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2010-02-20 16:14 UTC (permalink / raw)
To: Igor Kovalenko, Anthony Liguori; +Cc: Alexander Graf, qemu-devel
2010/2/20 Igor Kovalenko <igor.v.kovalenko@gmail.com>:
> On Sat, Feb 20, 2010 at 2:26 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>> On 02/19/2010 04:22 PM, Alexander Graf wrote:
>>>
>>> On 19.02.2010, at 22:50, Anthony Liguori wrote:
>>>
>>>
>>>>
>>>> On 02/16/2010 02:35 AM, Frediano Ziglio wrote:
>>>>
>>>>>
>>>>> Add a QEMU timer only when needed (timeout status not set, timeout
>>>>> irq wanted and timer set).
>>>>>
>>>>> This patch is required for Darwin. Patch has been tested under
>>>>> FreeBSD, Darwin and Linux.
>>>>>
>>>>> Signed-off-by: Frediano Ziglio<freddy77@gmail.com>
>>>>>
>>>>>
>>>>
>>>> Alex, I know you've played with this feature for OS X in the past.
>>>>
>>>> Is this something we should be enabling unconditionally?
>>>>
>>>
>>> I've played with it and realized that
>>>
>>> 1) OSX's rtl8139 driver uses this timer instead of tx/rx interrupts to
>>> process the queue
>>> 2) it was broken
>>>
>>> I haven't gotten around to test if this patch makes it work, but I'd
>>> suppose so.
>>>
>>> So yes, I think it should be enabled by default. It's a feature real hw
>>> has, so we should emulate it.
>>>
>>
>> The thing that concerns me is why it was disabled by default. Igor, what
>> was the reasoning for disabling it if you recall?
>
> It was very inefficient pci timer implementation which fired callback
> frequently.
> Once it was found that (freebsd?) was relying on pci timer in realtek driver
> to communicate we just added missing piece.
>
> Now this patch fixes that inefficiency issue.
>
I'm not that use with this ML way... sorry if somebody will receive a
lot of SPAM...
I updated my patch with Anthony suggestions (just code style).
Previous code (current git) issue a timer every pci tick (about 33
millions per second!) in order to compute correctly TCTR. This patch
enable QEMU timers only when strictly needed (for interrupt purposes)
computing TCTR when needed (only on driver read).
I tested with patch under Linux, FreeBSD and Darwin (Mac OS X) and it
works correctly. I also bought a physical card to test the behaviour
with a small test program and test program work with emulated code too
(previously code does not). I sent the test program but I don't know
if is possible to integrate it into QEMU code (is a standalone test).
Frediano Ziglio
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-20 16:06 ` Frediano Ziglio
@ 2010-02-20 17:13 ` Anthony Liguori
0 siblings, 0 replies; 12+ messages in thread
From: Anthony Liguori @ 2010-02-20 17:13 UTC (permalink / raw)
To: Frediano Ziglio; +Cc: qemu-devel
On 02/20/2010 10:06 AM, Frediano Ziglio wrote:
> Add a QEMU timer only when needed (timeout status not set, timeout
> irq wanted and timer set).
>
> This patch is required for Darwin. Patch has been tested under
> FreeBSD, Darwin and Linux.
>
This is missing a Signed-off-by.
Regards,
Anthony Liguori
> ---
> hw/rtl8139.c | 139 +++++++++++++++++++++++++++++++++++-----------------------
> 1 files changed, 84 insertions(+), 55 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index f04dd54..72e2242 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -41,6 +41,10 @@
> * segmentation offloading
> * Removed slirp.h dependency
> * Added rx/tx buffer reset when enabling rx/tx operation
> + *
> + * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
> + * when strictly needed (required for for
> + * Darwin)
> */
>
> #include "hw.h"
> @@ -60,9 +64,6 @@
> /* Calculate CRCs properly on Rx packets */
> #define RTL8139_CALCULATE_RXCRC 1
>
> -/* Uncomment to enable on-board timer interrupts */
> -//#define RTL8139_ONBOARD_TIMER 1
> -
> #if defined(RTL8139_CALCULATE_RXCRC)
> /* For crc32 */
> #include<zlib.h>
> @@ -491,9 +492,12 @@ typedef struct RTL8139State {
>
> /* PCI interrupt timer */
> QEMUTimer *timer;
> + int64_t TimerExpire;
>
> } RTL8139State;
>
> +static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
> +
> static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
> {
> DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
> @@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
>
> s->IntrMask = val;
>
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> rtl8139_update_irq(s);
> +
> }
>
> static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
> @@ -2555,12 +2561,22 @@ 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 loosing
> + */
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> rtl8139_update_irq(s);
> +
> #endif
> }
>
> static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
> {
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> +
> uint32_t ret = s->IntrStatus;
>
> DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
> @@ -2739,6 +2755,46 @@ 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)
> +{
> + int64_t pci_time, next_time;
> + uint32_t low_pci;
> +
> + DEBUG_PRINT(("RTL8139: 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
> + */
> + 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;
> + }
> + 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) {
> + qemu_mod_timer(s->timer, next_time);
> + }
> +}
> +
> static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
> {
> RTL8139State *s = opaque;
> @@ -2784,13 +2840,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
>
> case Timer:
> DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
> - s->TCTR = 0;
> s->TCTR_base = qemu_get_clock(vm_clock);
> + rtl8139_set_next_tctr_time(s, s->TCTR_base);
> break;
>
> case FlashReg:
> DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
> - s->TimerInt = val;
> + if (s->TimerInt != val) {
> + s->TimerInt = val;
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> + }
> break;
>
> default:
> @@ -3000,7 +3059,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
> break;
>
> case Timer:
> - ret = s->TCTR;
> + ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
> + PCI_FREQUENCY, get_ticks_per_sec());
> DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
> break;
>
> @@ -3105,6 +3165,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
> static int rtl8139_post_load(void *opaque, int version_id)
> {
> RTL8139State* s = opaque;
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> if (version_id< 4) {
> s->cplus_enabled = s->CpCmd != 0;
> }
> @@ -3112,12 +3173,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
> return 0;
> }
>
> +static void rtl8139_pre_save(void *opaque)
> +{
> + RTL8139State* s = opaque;
> + int64_t current_time = qemu_get_clock(vm_clock);
> +
> + /* set IntrStatus correctly */
> + rtl8139_set_next_tctr_time(s, current_time);
> + s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
> + get_ticks_per_sec());
> +}
> +
> static const VMStateDescription vmstate_rtl8139 = {
> .name = "rtl8139",
> .version_id = 4,
> .minimum_version_id = 3,
> .minimum_version_id_old = 3,
> .post_load = rtl8139_post_load,
> + .pre_save = rtl8139_pre_save,
> .fields = (VMStateField []) {
> VMSTATE_PCI_DEVICE(dev, RTL8139State),
> VMSTATE_PARTIAL_BUFFER(phys, RTL8139State, 6),
> @@ -3231,59 +3304,20 @@ static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
> rtl8139_mmio_writel,
> };
>
> -static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
> -{
> - int64_t next_time = current_time +
> - muldiv64(1, get_ticks_per_sec(), PCI_FREQUENCY);
> - if (next_time<= current_time)
> - next_time = current_time + 1;
> - return next_time;
> -}
> -
> -#ifdef RTL8139_ONBOARD_TIMER
> static void rtl8139_timer(void *opaque)
> {
> RTL8139State *s = opaque;
>
> - int is_timeout = 0;
> -
> - int64_t curr_time;
> - uint32_t curr_tick;
> -
> if (!s->clock_enabled)
> {
> DEBUG_PRINT(("RTL8139:>>> timer: clock is not running\n"));
> return;
> }
>
> - curr_time = qemu_get_clock(vm_clock);
> -
> - curr_tick = muldiv64(curr_time - s->TCTR_base, PCI_FREQUENCY,
> - get_ticks_per_sec());
> -
> - if (s->TimerInt&& curr_tick>= s->TimerInt)
> - {
> - if (s->TCTR< s->TimerInt || curr_tick< s->TCTR)
> - {
> - is_timeout = 1;
> - }
> - }
> -
> - s->TCTR = curr_tick;
> -
> -// DEBUG_PRINT(("RTL8139:>>> timer: tick=%08u\n", s->TCTR));
> -
> - if (is_timeout)
> - {
> - DEBUG_PRINT(("RTL8139:>>> timer: timeout tick=%08u\n", s->TCTR));
> - s->IntrStatus |= PCSTimeout;
> - rtl8139_update_irq(s);
> - }
> -
> - qemu_mod_timer(s->timer,
> - rtl8139_get_next_tctr_time(s,curr_time));
> + s->IntrStatus |= PCSTimeout;
> + rtl8139_update_irq(s);
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> }
> -#endif /* RTL8139_ONBOARD_TIMER */
>
> static void rtl8139_cleanup(VLANClientState *nc)
> {
> @@ -3301,10 +3335,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
> qemu_free(s->cplus_txbuffer);
> s->cplus_txbuffer = NULL;
> }
> -#ifdef RTL8139_ONBOARD_TIMER
> qemu_del_timer(s->timer);
> qemu_free_timer(s->timer);
> -#endif
> qemu_del_vlan_client(&s->nic->nc);
> return 0;
> }
> @@ -3356,12 +3388,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
> s->cplus_txbuffer_len = 0;
> s->cplus_txbuffer_offset = 0;
>
> -#ifdef RTL8139_ONBOARD_TIMER
> + s->TimerExpire = 0;
> s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);
> -
> - qemu_mod_timer(s->timer,
> - rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
> -#endif /* RTL8139_ONBOARD_TIMER */
> + rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-19 23:26 ` Anthony Liguori
2010-02-20 5:52 ` Igor Kovalenko
2010-02-20 16:06 ` Frediano Ziglio
@ 2010-02-20 17:27 ` Frediano Ziglio
2010-02-25 14:24 ` Paul Brook
2010-02-20 17:50 ` Frediano Ziglio
3 siblings, 1 reply; 12+ messages in thread
From: Frediano Ziglio @ 2010-02-20 17:27 UTC (permalink / raw)
To: anthony, igor.v.kovalenko; +Cc: qemu-devel, Frediano Ziglio
Add a QEMU timer only when needed (timeout status not set, timeout
irq wanted and timer set).
This patch is required for Darwin. Patch has been tested under
FreeBSD, Darwin and Linux.
---
hw/rtl8139.c | 139 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 84 insertions(+), 55 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index f04dd54..72e2242 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -41,6 +41,10 @@
* segmentation offloading
* Removed slirp.h dependency
* Added rx/tx buffer reset when enabling rx/tx operation
+ *
+ * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
+ * when strictly needed (required for for
+ * Darwin)
*/
#include "hw.h"
@@ -60,9 +64,6 @@
/* Calculate CRCs properly on Rx packets */
#define RTL8139_CALCULATE_RXCRC 1
-/* Uncomment to enable on-board timer interrupts */
-//#define RTL8139_ONBOARD_TIMER 1
-
#if defined(RTL8139_CALCULATE_RXCRC)
/* For crc32 */
#include <zlib.h>
@@ -491,9 +492,12 @@ typedef struct RTL8139State {
/* PCI interrupt timer */
QEMUTimer *timer;
+ int64_t TimerExpire;
} RTL8139State;
+static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
+
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
{
DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
@@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
s->IntrMask = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
}
static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
@@ -2555,12 +2561,22 @@ 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 loosing
+ */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
#endif
}
static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
{
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+
uint32_t ret = s->IntrStatus;
DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
@@ -2739,6 +2755,46 @@ 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)
+{
+ int64_t pci_time, next_time;
+ uint32_t low_pci;
+
+ DEBUG_PRINT(("RTL8139: 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
+ */
+ 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;
+ }
+ 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) {
+ qemu_mod_timer(s->timer, next_time);
+ }
+}
+
static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
{
RTL8139State *s = opaque;
@@ -2784,13 +2840,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
case Timer:
DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
- s->TCTR = 0;
s->TCTR_base = qemu_get_clock(vm_clock);
+ rtl8139_set_next_tctr_time(s, s->TCTR_base);
break;
case FlashReg:
DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
- s->TimerInt = val;
+ if (s->TimerInt != val) {
+ s->TimerInt = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+ }
break;
default:
@@ -3000,7 +3059,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
break;
case Timer:
- ret = s->TCTR;
+ ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
+ PCI_FREQUENCY, get_ticks_per_sec());
DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
break;
@@ -3105,6 +3165,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
static int rtl8139_post_load(void *opaque, int version_id)
{
RTL8139State* s = opaque;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
if (version_id < 4) {
s->cplus_enabled = s->CpCmd != 0;
}
@@ -3112,12 +3173,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
return 0;
}
+static void rtl8139_pre_save(void *opaque)
+{
+ RTL8139State* s = opaque;
+ int64_t current_time = qemu_get_clock(vm_clock);
+
+ /* set IntrStatus correctly */
+ rtl8139_set_next_tctr_time(s, current_time);
+ s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
+ get_ticks_per_sec());
+}
+
static const VMStateDescription vmstate_rtl8139 = {
.name = "rtl8139",
.version_id = 4,
.minimum_version_id = 3,
.minimum_version_id_old = 3,
.post_load = rtl8139_post_load,
+ .pre_save = rtl8139_pre_save,
.fields = (VMStateField []) {
VMSTATE_PCI_DEVICE(dev, RTL8139State),
VMSTATE_PARTIAL_BUFFER(phys, RTL8139State, 6),
@@ -3231,59 +3304,20 @@ static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
rtl8139_mmio_writel,
};
-static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
-{
- int64_t next_time = current_time +
- muldiv64(1, get_ticks_per_sec(), PCI_FREQUENCY);
- if (next_time <= current_time)
- next_time = current_time + 1;
- return next_time;
-}
-
-#ifdef RTL8139_ONBOARD_TIMER
static void rtl8139_timer(void *opaque)
{
RTL8139State *s = opaque;
- int is_timeout = 0;
-
- int64_t curr_time;
- uint32_t curr_tick;
-
if (!s->clock_enabled)
{
DEBUG_PRINT(("RTL8139: >>> timer: clock is not running\n"));
return;
}
- curr_time = qemu_get_clock(vm_clock);
-
- curr_tick = muldiv64(curr_time - s->TCTR_base, PCI_FREQUENCY,
- get_ticks_per_sec());
-
- if (s->TimerInt && curr_tick >= s->TimerInt)
- {
- if (s->TCTR < s->TimerInt || curr_tick < s->TCTR)
- {
- is_timeout = 1;
- }
- }
-
- s->TCTR = curr_tick;
-
-// DEBUG_PRINT(("RTL8139: >>> timer: tick=%08u\n", s->TCTR));
-
- if (is_timeout)
- {
- DEBUG_PRINT(("RTL8139: >>> timer: timeout tick=%08u\n", s->TCTR));
- s->IntrStatus |= PCSTimeout;
- rtl8139_update_irq(s);
- }
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,curr_time));
+ s->IntrStatus |= PCSTimeout;
+ rtl8139_update_irq(s);
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
}
-#endif /* RTL8139_ONBOARD_TIMER */
static void rtl8139_cleanup(VLANClientState *nc)
{
@@ -3301,10 +3335,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
qemu_free(s->cplus_txbuffer);
s->cplus_txbuffer = NULL;
}
-#ifdef RTL8139_ONBOARD_TIMER
qemu_del_timer(s->timer);
qemu_free_timer(s->timer);
-#endif
qemu_del_vlan_client(&s->nic->nc);
return 0;
}
@@ -3356,12 +3388,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
s->cplus_txbuffer_len = 0;
s->cplus_txbuffer_offset = 0;
-#ifdef RTL8139_ONBOARD_TIMER
+ s->TimerExpire = 0;
s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
-#endif /* RTL8139_ONBOARD_TIMER */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
return 0;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-19 23:26 ` Anthony Liguori
` (2 preceding siblings ...)
2010-02-20 17:27 ` Frediano Ziglio
@ 2010-02-20 17:50 ` Frediano Ziglio
3 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2010-02-20 17:50 UTC (permalink / raw)
To: anthony, igor.v.kovalenko; +Cc: qemu-devel, Frediano Ziglio
Add a QEMU timer only when needed (timeout status not set, timeout
irq wanted and timer set).
This patch is required for Darwin. Patch has been tested under
FreeBSD, Darwin and Linux.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
hw/rtl8139.c | 139 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 84 insertions(+), 55 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index f04dd54..72e2242 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -41,6 +41,10 @@
* segmentation offloading
* Removed slirp.h dependency
* Added rx/tx buffer reset when enabling rx/tx operation
+ *
+ * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
+ * when strictly needed (required for for
+ * Darwin)
*/
#include "hw.h"
@@ -60,9 +64,6 @@
/* Calculate CRCs properly on Rx packets */
#define RTL8139_CALCULATE_RXCRC 1
-/* Uncomment to enable on-board timer interrupts */
-//#define RTL8139_ONBOARD_TIMER 1
-
#if defined(RTL8139_CALCULATE_RXCRC)
/* For crc32 */
#include <zlib.h>
@@ -491,9 +492,12 @@ typedef struct RTL8139State {
/* PCI interrupt timer */
QEMUTimer *timer;
+ int64_t TimerExpire;
} RTL8139State;
+static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
+
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
{
DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
@@ -2522,7 +2526,9 @@ static void rtl8139_IntrMask_write(RTL8139State *s, uint32_t val)
s->IntrMask = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
}
static uint32_t rtl8139_IntrMask_read(RTL8139State *s)
@@ -2555,12 +2561,22 @@ 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 loosing
+ */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
rtl8139_update_irq(s);
+
#endif
}
static uint32_t rtl8139_IntrStatus_read(RTL8139State *s)
{
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+
uint32_t ret = s->IntrStatus;
DEBUG_PRINT(("RTL8139: IntrStatus read(w) val=0x%04x\n", ret));
@@ -2739,6 +2755,46 @@ 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)
+{
+ int64_t pci_time, next_time;
+ uint32_t low_pci;
+
+ DEBUG_PRINT(("RTL8139: 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
+ */
+ 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;
+ }
+ 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) {
+ qemu_mod_timer(s->timer, next_time);
+ }
+}
+
static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
{
RTL8139State *s = opaque;
@@ -2784,13 +2840,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
case Timer:
DEBUG_PRINT(("RTL8139: TCTR Timer reset on write\n"));
- s->TCTR = 0;
s->TCTR_base = qemu_get_clock(vm_clock);
+ rtl8139_set_next_tctr_time(s, s->TCTR_base);
break;
case FlashReg:
DEBUG_PRINT(("RTL8139: FlashReg TimerInt write val=0x%08x\n", val));
- s->TimerInt = val;
+ if (s->TimerInt != val) {
+ s->TimerInt = val;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
+ }
break;
default:
@@ -3000,7 +3059,8 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
break;
case Timer:
- ret = s->TCTR;
+ ret = muldiv64(qemu_get_clock(vm_clock) - s->TCTR_base,
+ PCI_FREQUENCY, get_ticks_per_sec());
DEBUG_PRINT(("RTL8139: TCTR Timer read val=0x%08x\n", ret));
break;
@@ -3105,6 +3165,7 @@ static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
static int rtl8139_post_load(void *opaque, int version_id)
{
RTL8139State* s = opaque;
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
if (version_id < 4) {
s->cplus_enabled = s->CpCmd != 0;
}
@@ -3112,12 +3173,24 @@ static int rtl8139_post_load(void *opaque, int version_id)
return 0;
}
+static void rtl8139_pre_save(void *opaque)
+{
+ RTL8139State* s = opaque;
+ int64_t current_time = qemu_get_clock(vm_clock);
+
+ /* set IntrStatus correctly */
+ rtl8139_set_next_tctr_time(s, current_time);
+ s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
+ get_ticks_per_sec());
+}
+
static const VMStateDescription vmstate_rtl8139 = {
.name = "rtl8139",
.version_id = 4,
.minimum_version_id = 3,
.minimum_version_id_old = 3,
.post_load = rtl8139_post_load,
+ .pre_save = rtl8139_pre_save,
.fields = (VMStateField []) {
VMSTATE_PCI_DEVICE(dev, RTL8139State),
VMSTATE_PARTIAL_BUFFER(phys, RTL8139State, 6),
@@ -3231,59 +3304,20 @@ static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
rtl8139_mmio_writel,
};
-static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
-{
- int64_t next_time = current_time +
- muldiv64(1, get_ticks_per_sec(), PCI_FREQUENCY);
- if (next_time <= current_time)
- next_time = current_time + 1;
- return next_time;
-}
-
-#ifdef RTL8139_ONBOARD_TIMER
static void rtl8139_timer(void *opaque)
{
RTL8139State *s = opaque;
- int is_timeout = 0;
-
- int64_t curr_time;
- uint32_t curr_tick;
-
if (!s->clock_enabled)
{
DEBUG_PRINT(("RTL8139: >>> timer: clock is not running\n"));
return;
}
- curr_time = qemu_get_clock(vm_clock);
-
- curr_tick = muldiv64(curr_time - s->TCTR_base, PCI_FREQUENCY,
- get_ticks_per_sec());
-
- if (s->TimerInt && curr_tick >= s->TimerInt)
- {
- if (s->TCTR < s->TimerInt || curr_tick < s->TCTR)
- {
- is_timeout = 1;
- }
- }
-
- s->TCTR = curr_tick;
-
-// DEBUG_PRINT(("RTL8139: >>> timer: tick=%08u\n", s->TCTR));
-
- if (is_timeout)
- {
- DEBUG_PRINT(("RTL8139: >>> timer: timeout tick=%08u\n", s->TCTR));
- s->IntrStatus |= PCSTimeout;
- rtl8139_update_irq(s);
- }
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,curr_time));
+ s->IntrStatus |= PCSTimeout;
+ rtl8139_update_irq(s);
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
}
-#endif /* RTL8139_ONBOARD_TIMER */
static void rtl8139_cleanup(VLANClientState *nc)
{
@@ -3301,10 +3335,8 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
qemu_free(s->cplus_txbuffer);
s->cplus_txbuffer = NULL;
}
-#ifdef RTL8139_ONBOARD_TIMER
qemu_del_timer(s->timer);
qemu_free_timer(s->timer);
-#endif
qemu_del_vlan_client(&s->nic->nc);
return 0;
}
@@ -3356,12 +3388,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
s->cplus_txbuffer_len = 0;
s->cplus_txbuffer_offset = 0;
-#ifdef RTL8139_ONBOARD_TIMER
+ s->TimerExpire = 0;
s->timer = qemu_new_timer(vm_clock, rtl8139_timer, s);
-
- qemu_mod_timer(s->timer,
- rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
-#endif /* RTL8139_ONBOARD_TIMER */
+ rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
return 0;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139.
2010-02-20 17:27 ` Frediano Ziglio
@ 2010-02-25 14:24 ` Paul Brook
0 siblings, 0 replies; 12+ messages in thread
From: Paul Brook @ 2010-02-25 14:24 UTC (permalink / raw)
To: qemu-devel
> +++ b/hw/rtl8139.c
> @@ -41,6 +41,10 @@
> * segmentation offloading
> * Removed slirp.h dependency
> * Added rx/tx buffer reset when enabling
> rx/tx operation + *
> + * 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer
> only + * when strictly needed (required
> for for + * Darwin)
Please don't do this. We have proper source control to track changes. Adding
random log entries to the top of the file just adds noise.
Paul
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-02-25 14:24 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-16 8:35 [Qemu-devel] [PATCH] rewrote timer implementation for rtl8139 Frediano Ziglio
2010-02-19 21:50 ` Anthony Liguori
2010-02-19 22:22 ` Alexander Graf
2010-02-19 23:26 ` Anthony Liguori
2010-02-20 5:52 ` Igor Kovalenko
2010-02-20 16:14 ` Frediano Ziglio
2010-02-20 16:06 ` Frediano Ziglio
2010-02-20 17:13 ` Anthony Liguori
2010-02-20 17:27 ` Frediano Ziglio
2010-02-25 14:24 ` Paul Brook
2010-02-20 17:50 ` Frediano Ziglio
2010-02-19 23:24 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).