sparc32: fix per cpu counter/timer

Sun4m SMP machines support a maximum of 4 CPUs. Linux knows this and uses fixed size arrays for per-cpu counter/timers and interrupt controllers. Sun4m uni-processor machines use the slaveio chip which has a single per-cpu counter/timer and interrupt controller. However it does not fully decode the address so the same counter/timer or interrupt controller can be accesses from multiple addresses.

This patch changes the per-cpu counter/timer to work the way the real hardware works: 4 per-cpu counter/timers for SMP and 1 counter/timer for UP mapped at multiple addresses.

This patch also fixes a number of per-cpu user timer bugs:
limit bit set when limit reached, count saved and used when written, limit bit reset on count write and system timer configuration register updated properly for per-cpu user timer mode.

Sun4d currently uses the sun4m counter/timer code. They are simular but not the same. This patch will break the broken sun4d implementation further. The real fix is to create a proper sun4d counter/timer implementation. Since the sun4d implementation doesn't currently work anyway, this shouldn't be an issue.

slavio_timer.c +12 -10

-            s->limit = TIMER_MAX_COUNT64;
-            DPRINTF("processor %d user timer reset\n", s->slave_index);
-            if (s->timer)
-                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
+            s->reached = 0;
+            s->counthigh = val & (TIMER_MAX_COUNT64 >> 32);
+            count = s->count | (uint64_t)s->counthigh << 32;
+            DPRINTF("processor %d user timer set to %016llx\n",
+                    original->slave_index, count);
+            ptimer_set_count(s->timer, LIMIT_TO_PERIODS(s->limit - count));
         } else {
             // set limit, reset counter
             qemu_irq_lower(s->irq);
             s->limit = val & TIMER_MAX_COUNT32;
-            if (s->timer) {
-                if (s->limit == 0) /* free-run */
-                    ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
-                else
-                    ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
-            }
+            if (s->limit == 0) /* free-run */
+                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32),
+                                 1);
+            else
+                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
         }
         break;
     case TIMER_COUNTER:
         if (slavio_timer_is_user(s)) {
+            uint64_t count;
             // set user counter LSW, reset counter
             qemu_irq_lower(s->irq);

slavio_timer.c +21 -14

-            for (i = 0; i < s->num_slaves; i++) {
-                if (val & (1 << i)) {
-                    qemu_irq_lower(s->slave[i]->irq);
-                    s->slave[i]->limit = -1ULL;
-                } else {
-                    ptimer_stop(s->slave[i]->timer);
-                }
-                if ((val & (1 << i)) != (s->slave_mode & (1 << i))) {
-                    ptimer_stop(s->slave[i]->timer);
-                    ptimer_set_limit(s->slave[i]->timer,
-                                     LIMIT_TO_PERIODS(s->slave[i]->limit), 1);
-                    DPRINTF("processor %d timer changed\n",
-                            s->slave[i]->slave_index);
-                    ptimer_run(s->slave[i]->timer, 0);
+            for (i = 0; i < num_slaves; i++) {
+                unsigned int processor = 1 << i;
+                // check for a change in timer mode for this processor
+                if ((val & processor) != (s->slave_mode & processor)) {
+                    if (val & processor) { // counter -> user timer
+                        qemu_irq_lower(s->slave[i]->irq);
+                        // counters are always running
+                        ptimer_stop(s->slave[i]->timer);
+                        s->slave[i]->running = 0;
+                        // user timer limit is always the same
+                        s->slave[i]->limit = TIMER_MAX_COUNT64;
+                        ptimer_set_limit(s->slave[i]->timer,
+                                         LIMIT_TO_PERIODS(s->slave[i]->limit), 1);
+                        // set this processors user timer bit in config
+                        // register
+                        s->slave_mode |= processor;
+                        DPRINTF("processor %d changed from counter to user "
+                                "timer\n", s->slave[i]->slave_index);
+                    } else { // user timer -> counter
+                        // stop the user timer if it is running
+                        if (s->slave[i]->running)

static SLAVIO_TIMERState *slavio_timer_init(target_phys_addr_t addr,
qemu_irq irq, SLAVIO_TIMERState *master,

slavio_timer.c +10 -8

-                                            int slave_index)
+                                            int slave_index, int mapped)
 {
     int slavio_timer_io_memory;
     SLAVIO_TIMERState *s;

@@ -349,7 +375,7 @@ static SLAVIO_TIMERState *slavio_timer_i
     s->irq = irq;
     s->master = master;
     s->slave_index = slave_index;
-    if (!master || slave_index < master->num_slaves) {
+    if (!mapped) { /* don't create a qemu timer for mapped devices */
         bh = qemu_bh_new(slavio_timer_irq, s);
         s->timer = ptimer_init(bh);
         ptimer_set_period(s->timer, TIMER_PERIOD);

@@ -363,27 +389,30 @@ static SLAVIO_TIMERState *slavio_timer_i
     else
         cpu_register_physical_memory(addr, SYS_TIMER_SIZE,
                                      slavio_timer_io_memory);
-    register_savevm("slavio_timer", addr, 3, slavio_timer_save,
-                    slavio_timer_load, s);
-    qemu_register_reset(slavio_timer_reset, s);
-    slavio_timer_reset(s);
+    if (!mapped) { /* don't register mapped devices */
+        register_savevm("slavio_timer", addr, 3, slavio_timer_save,
+                        slavio_timer_load, s);
+        qemu_register_reset(slavio_timer_reset, s);
+        slavio_timer_reset(s);
+    }
 
     return s;
 }
 
 void slavio_timer_init_all(target_phys_addr_t base, qemu_irq master_irq,
-                           qemu_irq *cpu_irqs, unsigned int num_cpus)
+                           qemu_irq *cpu_irqs, int smp)
 {
     SLAVIO_TIMERState *master;
     unsigned int i;
 
-    master = slavio_timer_init(base + SYS_TIMER_OFFSET, master_irq, NULL, 0);
+    master = slavio_timer_init(base + SYS_TIMER_OFFSET, master_irq, NULL, 0, 0);
 

slavio_timer.c typedef struct SLAVIO_TIMERState { · line 68

Quoted 6 lines

slavio_timer.c +2 -2

-    unsigned int num_slaves;
-    struct SLAVIO_TIMERState *slave[MAX_CPUS];
+    int smp;
+    struct SLAVIO_TIMERState *slave[MAX_SUN4M_CPUS];

I don't think the change from num_slaves to smp is needed.

slavio_timer.c static int slavio_timer_is_user(SLAVIO_T · line 102

Quoted 6 lines

slavio_timer.c +4 -0

+static int slavio_timer_is_mapped(SLAVIO_TIMERState *s)
+{
+    return s->master && (!s->master->smp && s->slave_index > 1);
+}

These kind of helpers should be introduced so that the logic is not changed, now it's hard to see what changes and what doesn't.

slavio_timer.c static void slavio_timer_get_out(SLAVIO_ · line 115

Quoted 7 lines

slavio_timer.c +1 -4

-    if (s->timer)
-        count = limit - PERIODS_TO_LIMIT(ptimer_get_count(s->timer));
-    else
-        count = 0;
+    count = limit - PERIODS_TO_LIMIT(ptimer_get_count(s->timer));

Same here.

slavio_timer.c static uint32_t slavio_timer_mem_readl(v · line 142

Quoted 5 lines

slavio_timer.c +3 -0

+    if (slavio_timer_is_mapped(s))
+        s = s->master->slave[0];
+

And here.

Quoted 34 lines

slavio_timer.c +12 -10

-            s->limit = TIMER_MAX_COUNT64;
-            DPRINTF("processor %d user timer reset\n", s->slave_index);
-            if (s->timer)
-                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
+            s->reached = 0;
+            s->counthigh = val & (TIMER_MAX_COUNT64 >> 32);
+            count = s->count | (uint64_t)s->counthigh << 32;
+            DPRINTF("processor %d user timer set to %016llx\n",
+                    original->slave_index, count);
+            ptimer_set_count(s->timer, LIMIT_TO_PERIODS(s->limit - count));
        } else {
            // set limit, reset counter
            qemu_irq_lower(s->irq);
            s->limit = val & TIMER_MAX_COUNT32;
-            if (s->timer) {
-                if (s->limit == 0) /* free-run */
-                    ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
-                else
-                    ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
-            }
+            if (s->limit == 0) /* free-run */
+                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32),
+                                 1);
+            else
+                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
        }
        break;
    case TIMER_COUNTER:
        if (slavio_timer_is_user(s)) {
+            uint64_t count;
            // set user counter LSW, reset counter
            qemu_irq_lower(s->irq);

slavio_timer.c static void slavio_timer_mem_writel(void · line 236

Quoted 10 lines

slavio_timer.c +6 -4

-            s->limit = TIMER_MAX_COUNT64;
-            DPRINTF("processor %d user timer reset\n", s->slave_index);
-            if (s->timer)
-                ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 1);
+            s->reached = 0;
+            s->count = val & TIMER_MAX_COUNT64;
+            count = s->count | (uint64_t)s->counthigh << 32;
+            DPRINTF("processor %d user timer set to %016llx\n",
+                    original->slave_index, count);
+            ptimer_set_count(s->timer, LIMIT_TO_PERIODS(s->limit - count));

These are separate.

Quoted 37 lines

slavio_timer.c +21 -14

-            for (i = 0; i < s->num_slaves; i++) {
-                if (val & (1 << i)) {
-                    qemu_irq_lower(s->slave[i]->irq);
-                    s->slave[i]->limit = -1ULL;
-                } else {
-                    ptimer_stop(s->slave[i]->timer);
-                }
-                if ((val & (1 << i)) != (s->slave_mode & (1 << i))) {
-                    ptimer_stop(s->slave[i]->timer);
-                    ptimer_set_limit(s->slave[i]->timer,
-                                     LIMIT_TO_PERIODS(s->slave[i]->limit), 1);
-                    DPRINTF("processor %d timer changed\n",
-                            s->slave[i]->slave_index);
-                    ptimer_run(s->slave[i]->timer, 0);
+            for (i = 0; i < num_slaves; i++) {
+                unsigned int processor = 1 << i;
+                // check for a change in timer mode for this processor
+                if ((val & processor) != (s->slave_mode & processor)) {
+                    if (val & processor) { // counter -> user timer
+                        qemu_irq_lower(s->slave[i]->irq);
+                        // counters are always running
+                        ptimer_stop(s->slave[i]->timer);
+                        s->slave[i]->running = 0;
+                        // user timer limit is always the same
+                        s->slave[i]->limit = TIMER_MAX_COUNT64;
+                        ptimer_set_limit(s->slave[i]->timer,
+                                         LIMIT_TO_PERIODS(s->slave[i]->limit), 1);
+                        // set this processors user timer bit in config
+                        // register
+                        s->slave_mode |= processor;
+                        DPRINTF("processor %d changed from counter to user "
+                                "timer\n", s->slave[i]->slave_index);
+                    } else { // user timer -> counter
+                        // stop the user timer if it is running
+                        if (s->slave[i]->running)

slavio_timer.c static void slavio_timer_mem_writel(void · line 296

Quoted 10 lines

slavio_timer.c +10 -0

+                            ptimer_stop(s->slave[i]->timer);
+                        // start the counter
+                        ptimer_run(s->slave[i]->timer, 0);
+                        s->slave[i]->running = 1;
+                        // clear this processors user timer bit in config
+                        // register
+                        s->slave_mode &= ~processor;
+                        DPRINTF("processor %d changed from user timer to "
+                                "counter\n", s->slave[i]->slave_index);
+                    }

Too many changes at once.

Quoted 48 lines

static SLAVIO_TIMERState *slavio_timer_init(target_phys_addr_t addr,
qemu_irq irq, SLAVIO_TIMERState *master,

slavio_timer.c +1 -1

-                                            int slave_index)
+                                            int slave_index, int mapped)

{
    int slavio_timer_io_memory;
    SLAVIO_TIMERState *s;

slavio_timer.c +10 -8

@@ -349,7 +375,7 @@ static SLAVIO_TIMERState *slavio_timer_i
    s->irq = irq;
    s->master = master;
    s->slave_index = slave_index;
-    if (!master || slave_index < master->num_slaves) {
+    if (!mapped) { /* don't create a qemu timer for mapped devices */
        bh = qemu_bh_new(slavio_timer_irq, s);
        s->timer = ptimer_init(bh);
        ptimer_set_period(s->timer, TIMER_PERIOD);

@@ -363,27 +389,30 @@ static SLAVIO_TIMERState *slavio_timer_i
    else
        cpu_register_physical_memory(addr, SYS_TIMER_SIZE,
                                     slavio_timer_io_memory);
-    register_savevm("slavio_timer", addr, 3, slavio_timer_save,
-                    slavio_timer_load, s);
-    qemu_register_reset(slavio_timer_reset, s);
-    slavio_timer_reset(s);
+    if (!mapped) { /* don't register mapped devices */
+        register_savevm("slavio_timer", addr, 3, slavio_timer_save,
+                        slavio_timer_load, s);
+        qemu_register_reset(slavio_timer_reset, s);
+        slavio_timer_reset(s);
+    }
 
    return s;
 }
 
 void slavio_timer_init_all(target_phys_addr_t base, qemu_irq master_irq,
-                           qemu_irq *cpu_irqs, unsigned int num_cpus)
+                           qemu_irq *cpu_irqs, int smp)
 {
    SLAVIO_TIMERState *master;
    unsigned int i;
 
-    master = slavio_timer_init(base + SYS_TIMER_OFFSET, master_irq, NULL, 0);
+    master = slavio_timer_init(base + SYS_TIMER_OFFSET, master_irq, NULL, 0, 0);
 
-    master->num_slaves = num_cpus;
+    master->smp = smp;
 

slavio_timer.c static SLAVIO_TIMERState *slavio_timer_i

Quoted 9 lines

slavio_timer.c +3 -2

-    for (i = 0; i < MAX_CPUS; i++) {
+    for (i = 0; i < MAX_SUN4M_CPUS; i++) {
        master->slave[i] = slavio_timer_init(base + (target_phys_addr_t)
                                             CPU_TIMER_OFFSET(i),
-                                             cpu_irqs[i], master, i);
+                                             cpu_irqs[i], master, i,
+                                             !smp && i != 0);
 
 

This part is not OK. "mapped" should be "disabled" or something more descriptive. Currently a functioning device is created for units that have a corresponding CPU, and a non-functioning device for the other slots. I think that a non-functioning device is still needed for other slots for SMP kernels to work.

sun4m.c static void sun4m_hw_init(const struct h · line 386

Quoted 8 lines

sun4m.c +6 -0

+    if ((hwdef->machine_id == 0x80 && smp_cpus > 1) ||
+        (hwdef->machine_id != 0x80 && smp_cpus > MAX_SUN4M_CPUS)) {
+        fprintf(stderr,
+                "qemu: Too many CPUs for this machine: %d maiximum %d\n",
+                smp_cpus, hwdef->machine_id == 0x80 ? 1 : MAX_SUN4M_CPUS);
+        exit(1);

I don't want to limit the CPUs, so a warning is enough. A cleaner implementation is to put the CPU limit and extra timer parameters to hwdef.

Review

Quoted 4 lines

(quoted diff) +1 -1

-            fprintf(stderr, "Unable to find Sparc CPU definition\n");
+            fprintf(stderr, "qemu: Unable to find Sparc CPU definition\n");

Unrelated, like the next few wrapping changes.