Patchwork rtl8139 timer interrupt rewrote

login
register
mail settings
Submitter Frediano Ziglio
Date Feb. 7, 2010, 3:22 p.m.
Message ID <aa1b71111002070722s525aa6dfj8f5a7e67eea60e3b@mail.gmail.com>
Download mbox | patch
Permalink /patch/44739/
State New
Headers show

Comments

Frediano Ziglio - Feb. 7, 2010, 3:22 p.m.
rewrote timer implementation for rtl8139. Add a QEMU
timer only when needed (timeout status not set, timeout irq wanted and
timer set).

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
--
 #include "hw.h"
@@ -61,7 +65,7 @@
 #define RTL8139_CALCULATE_RXCRC 1

 /* Uncomment to enable on-board timer interrupts */
-//#define RTL8139_ONBOARD_TIMER 1
+#define RTL8139_ONBOARD_TIMER 1

 #if defined(RTL8139_CALCULATE_RXCRC)
 /* For crc32 */
@@ -489,11 +493,20 @@ typedef struct RTL8139State {
     int        cplus_txbuffer_len;
     int        cplus_txbuffer_offset;

+#ifdef RTL8139_ONBOARD_TIMER
     /* PCI interrupt timer */
     QEMUTimer *timer;
+    int64_t TimerExpire;
+#endif

 } RTL8139State;

+#ifdef RTL8139_ONBOARD_TIMER
+static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
+#else
+#define rtl8139_set_next_tctr_time(s,t) do { ; } while(0)
+#endif
+
 static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
 {
     DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
@@ -2522,7 +2535,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 +2570,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 +2764,45 @@ static void rtl8139_io_writew(void *opaque,
uint8_t addr, uint32_t val)
     }
 }

+#ifdef RTL8139_ONBOARD_TIMER
+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);
+    }
+}
+#endif
+
 static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
 {
     RTL8139State *s = opaque;
@@ -2784,13 +2848,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 +3067,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 +3173,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 +3181,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,57 +3312,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 */

@@ -3357,10 +3401,9 @@ static int pci_rtl8139_init(PCIDevice *dev)
     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)));
+    rtl8139_set_next_tctr_time(s, qemu_get_clock(vm_clock));
 #endif /* RTL8139_ONBOARD_TIMER */
     return 0;
 }
Igor V. Kovalenko - Feb. 8, 2010, 5:33 a.m.
On Sun, Feb 7, 2010 at 6:22 PM, Frediano Ziglio <freddy77@gmail.com> wrote:
> rewrote timer implementation for rtl8139. Add a QEMU
> timer only when needed (timeout status not set, timeout irq wanted and
> timer set).
>
> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
> --
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index f04dd54..0d877b7 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
> + *
> + *  2009-Feb-04  Frediano Ziglio:   Rewrote timer support using QEMU timer only
> + *                                  when strictly needed (required for for
> + *                                  Darwin)
>  */
>
>  #include "hw.h"
> @@ -61,7 +65,7 @@
>  #define RTL8139_CALCULATE_RXCRC 1
>
>  /* Uncomment to enable on-board timer interrupts */
> -//#define RTL8139_ONBOARD_TIMER 1
> +#define RTL8139_ONBOARD_TIMER 1

Please remove this macro.

>
> +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());
> +}
> +

Seems like TCTR is not used after restoring from saved state. Is it intentional?

Can you please check if freebsd works with this change?
Frediano Ziglio - Feb. 8, 2010, 10:33 a.m.
2010/2/8 Igor Kovalenko <igor.v.kovalenko@gmail.com>:
> On Sun, Feb 7, 2010 at 6:22 PM, Frediano Ziglio <freddy77@gmail.com> wrote:
>> rewrote timer implementation for rtl8139. Add a QEMU
>> timer only when needed (timeout status not set, timeout irq wanted and
>> timer set).
>>
>> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
>> --
>> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
>> index f04dd54..0d877b7 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
>> + *
>> + *  2009-Feb-04  Frediano Ziglio:   Rewrote timer support using QEMU timer only
>> + *                                  when strictly needed (required for for
>> + *                                  Darwin)
>>  */
>>
>>  #include "hw.h"
>> @@ -61,7 +65,7 @@
>>  #define RTL8139_CALCULATE_RXCRC 1
>>
>>  /* Uncomment to enable on-board timer interrupts */
>> -//#define RTL8139_ONBOARD_TIMER 1
>> +#define RTL8139_ONBOARD_TIMER 1
>
> Please remove this macro.
>

Removed (see updated patch)

>>
>> +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());
>> +}
>> +
>
> Seems like TCTR is not used after restoring from saved state. Is it intentional?
>

Yes, wanted, mainly TCTR is computed from TCTR_base and tick counts so
is currently quite unused but is computed for compatibility with
previous state.

> Can you please check if freebsd works with this change?
>

Yes, it works correctly. I don't know FreeBSD that much but I
downloaded latest 8.0 live version, got a shell, configured with
network manually with old ifconfig and tried some data exchange
(mainly scp/ssh).

Regards
 Frediano Ziglio

Patch

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index f04dd54..0d877b7 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
+ *
+ *  2009-Feb-04  Frediano Ziglio:   Rewrote timer support using QEMU timer only
+ *                                  when strictly needed (required for for
+ *                                  Darwin)
  */