* [Qemu-devel] [PATCH] sparc32: fix per cpu counter/timer
@ 2008-01-05 1:08 Robert Reif
2008-01-05 11:30 ` Blue Swirl
0 siblings, 1 reply; 3+ messages in thread
From: Robert Reif @ 2008-01-05 1:08 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]
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.
[-- Attachment #2: maxcpus.diff.txt --]
[-- Type: text/plain, Size: 28739 bytes --]
Index: hw/sbi.c
===================================================================
RCS file: /sources/qemu/qemu/hw/sbi.c,v
retrieving revision 1.2
diff -p -u -r1.2 sbi.c
--- hw/sbi.c 1 Jan 2008 17:06:38 -0000 1.2
+++ hw/sbi.c 5 Jan 2008 00:43:27 -0000
@@ -34,15 +34,13 @@ do { printf("IRQ: " fmt , ##args); } whi
#define DPRINTF(fmt, args...)
#endif
-#define MAX_CPUS 16
-
#define SBI_NREGS 16
typedef struct SBIState {
uint32_t regs[SBI_NREGS];
- uint32_t intreg_pending[MAX_CPUS];
- qemu_irq *cpu_irqs[MAX_CPUS];
- uint32_t pil_out[MAX_CPUS];
+ uint32_t intreg_pending[MAX_SUN4D_CPUS];
+ qemu_irq *cpu_irqs[MAX_SUN4D_CPUS];
+ uint32_t pil_out[MAX_SUN4D_CPUS];
} SBIState;
#define SBI_SIZE (SBI_NREGS * 4)
@@ -107,7 +105,7 @@ static void sbi_save(QEMUFile *f, void *
SBIState *s = opaque;
unsigned int i;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4D_CPUS; i++) {
qemu_put_be32s(f, &s->intreg_pending[i]);
}
}
@@ -120,7 +118,7 @@ static int sbi_load(QEMUFile *f, void *o
if (version_id != 1)
return -EINVAL;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4D_CPUS; i++) {
qemu_get_be32s(f, &s->intreg_pending[i]);
}
sbi_check_interrupts(s);
@@ -133,7 +131,7 @@ static void sbi_reset(void *opaque)
SBIState *s = opaque;
unsigned int i;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4D_CPUS; i++) {
s->intreg_pending[i] = 0;
}
sbi_check_interrupts(s);
@@ -150,7 +148,7 @@ void *sbi_init(target_phys_addr_t addr,
if (!s)
return NULL;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4D_CPUS; i++) {
s->cpu_irqs[i] = parent_irq[i];
}
@@ -160,7 +158,7 @@ void *sbi_init(target_phys_addr_t addr,
register_savevm("sbi", addr, 1, sbi_save, sbi_load, s);
qemu_register_reset(sbi_reset, s);
*irq = qemu_allocate_irqs(sbi_set_irq, s, 32);
- *cpu_irq = qemu_allocate_irqs(sbi_set_timer_irq_cpu, s, MAX_CPUS);
+ *cpu_irq = qemu_allocate_irqs(sbi_set_timer_irq_cpu, s, MAX_SUN4D_CPUS);
sbi_reset(s);
return s;
Index: hw/slavio_intctl.c
===================================================================
RCS file: /sources/qemu/qemu/hw/slavio_intctl.c,v
retrieving revision 1.29
diff -p -u -r1.29 slavio_intctl.c
--- hw/slavio_intctl.c 1 Jan 2008 20:57:25 -0000 1.29
+++ hw/slavio_intctl.c 5 Jan 2008 00:43:27 -0000
@@ -46,21 +46,20 @@ do { printf("IRQ: " fmt , ##args); } whi
*
*/
-#define MAX_CPUS 16
#define MAX_PILS 16
typedef struct SLAVIO_INTCTLState {
- uint32_t intreg_pending[MAX_CPUS];
+ uint32_t intreg_pending[MAX_SUN4M_CPUS];
uint32_t intregm_pending;
uint32_t intregm_disabled;
uint32_t target_cpu;
#ifdef DEBUG_IRQ_COUNT
uint64_t irq_count[32];
#endif
- qemu_irq *cpu_irqs[MAX_CPUS];
+ qemu_irq *cpu_irqs[MAX_SUN4M_CPUS];
const uint32_t *intbit_to_level;
uint32_t cputimer_lbit, cputimer_mbit;
- uint32_t pil_out[MAX_CPUS];
+ uint32_t pil_out[MAX_SUN4M_CPUS];
} SLAVIO_INTCTLState;
#define INTCTL_MAXADDR 0xf
@@ -84,7 +83,7 @@ static uint32_t slavio_intctl_mem_readl(
uint32_t saddr, ret;
int cpu;
- cpu = (addr & (MAX_CPUS - 1) * TARGET_PAGE_SIZE) >> 12;
+ cpu = (addr & (MAX_SUN4M_CPUS - 1) * TARGET_PAGE_SIZE) >> 12;
saddr = (addr & INTCTL_MAXADDR) >> 2;
switch (saddr) {
case 0:
@@ -105,7 +104,7 @@ static void slavio_intctl_mem_writel(voi
uint32_t saddr;
int cpu;
- cpu = (addr & (MAX_CPUS - 1) * TARGET_PAGE_SIZE) >> 12;
+ cpu = (addr & (MAX_SUN4M_CPUS - 1) * TARGET_PAGE_SIZE) >> 12;
saddr = (addr & INTCTL_MAXADDR) >> 2;
DPRINTF("write cpu %d reg 0x" TARGET_FMT_plx " = %x\n", cpu, addr, val);
switch (saddr) {
@@ -190,7 +189,7 @@ static void slavio_intctlm_mem_writel(vo
DPRINTF("Disabled master irq mask %x, curmask %x\n", val, s->intregm_disabled);
break;
case 4:
- s->target_cpu = val & (MAX_CPUS - 1);
+ s->target_cpu = val & (MAX_SUN4M_CPUS - 1);
slavio_check_interrupts(s);
DPRINTF("Set master irq cpu %d\n", s->target_cpu);
break;
@@ -216,7 +215,7 @@ void slavio_pic_info(void *opaque)
SLAVIO_INTCTLState *s = opaque;
int i;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4M_CPUS; i++) {
term_printf("per-cpu %d: pending 0x%08x\n", i, s->intreg_pending[i]);
}
term_printf("master: pending 0x%08x, disabled 0x%08x\n", s->intregm_pending, s->intregm_disabled);
@@ -249,7 +248,7 @@ static void slavio_check_interrupts(void
pending &= ~s->intregm_disabled;
DPRINTF("pending %x disabled %x\n", pending, s->intregm_disabled);
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4M_CPUS; i++) {
pil_pending = 0;
if (pending && !(s->intregm_disabled & MASTER_DISABLE) &&
(i == s->target_cpu)) {
@@ -322,7 +321,7 @@ static void slavio_intctl_save(QEMUFile
SLAVIO_INTCTLState *s = opaque;
int i;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4M_CPUS; i++) {
qemu_put_be32s(f, &s->intreg_pending[i]);
}
qemu_put_be32s(f, &s->intregm_pending);
@@ -335,10 +334,10 @@ static int slavio_intctl_load(QEMUFile *
SLAVIO_INTCTLState *s = opaque;
int i;
- if (version_id != 1)
+ if (version_id != 2)
return -EINVAL;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4M_CPUS; i++) {
qemu_get_be32s(f, &s->intreg_pending[i]);
}
qemu_get_be32s(f, &s->intregm_pending);
@@ -353,7 +352,7 @@ static void slavio_intctl_reset(void *op
SLAVIO_INTCTLState *s = opaque;
int i;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4M_CPUS; i++) {
s->intreg_pending[i] = 0;
}
s->intregm_disabled = ~MASTER_IRQ_MASK;
@@ -375,7 +374,7 @@ void *slavio_intctl_init(target_phys_add
return NULL;
s->intbit_to_level = intbit_to_level;
- for (i = 0; i < MAX_CPUS; i++) {
+ for (i = 0; i < MAX_SUN4M_CPUS; i++) {
slavio_intctl_io_memory = cpu_register_io_memory(0, slavio_intctl_mem_read, slavio_intctl_mem_write, s);
cpu_register_physical_memory(addr + i * TARGET_PAGE_SIZE, INTCTL_SIZE,
slavio_intctl_io_memory);
@@ -385,11 +384,11 @@ void *slavio_intctl_init(target_phys_add
slavio_intctlm_io_memory = cpu_register_io_memory(0, slavio_intctlm_mem_read, slavio_intctlm_mem_write, s);
cpu_register_physical_memory(addrg, INTCTLM_SIZE, slavio_intctlm_io_memory);
- register_savevm("slavio_intctl", addr, 1, slavio_intctl_save, slavio_intctl_load, s);
+ register_savevm("slavio_intctl", addr, 2, slavio_intctl_save, slavio_intctl_load, s);
qemu_register_reset(slavio_intctl_reset, s);
*irq = qemu_allocate_irqs(slavio_set_irq, s, 32);
- *cpu_irq = qemu_allocate_irqs(slavio_set_timer_irq_cpu, s, MAX_CPUS);
+ *cpu_irq = qemu_allocate_irqs(slavio_set_timer_irq_cpu, s, MAX_SUN4M_CPUS);
s->cputimer_mbit = 1 << cputimer;
s->cputimer_lbit = 1 << intbit_to_level[cputimer];
slavio_intctl_reset(s);
Index: hw/slavio_timer.c
===================================================================
RCS file: /sources/qemu/qemu/hw/slavio_timer.c,v
retrieving revision 1.28
diff -p -u -r1.28 slavio_timer.c
--- hw/slavio_timer.c 1 Jan 2008 17:06:38 -0000 1.28
+++ hw/slavio_timer.c 5 Jan 2008 00:43:27 -0000
@@ -47,10 +47,13 @@ do { printf("TIMER: " fmt , ##args); } w
* Per-CPU timers interrupt local CPU, system timer uses normal
* interrupt routing.
*
+ * SMP systems (SS-10, SS-20, SS-600MP) have 4 processor counter/timers.
+ * UP systems (SS-5) have a single processor counter/timer mapped at multiple
+ * addresses.
+ *
+ * Counters are always running. User timers may be stopped and started.
*/
-#define MAX_CPUS 16
-
typedef struct SLAVIO_TIMERState {
qemu_irq irq;
ptimer_state *timer;
@@ -61,8 +64,8 @@ typedef struct SLAVIO_TIMERState {
struct SLAVIO_TIMERState *master;
int slave_index;
// system only
- unsigned int num_slaves;
- struct SLAVIO_TIMERState *slave[MAX_CPUS];
+ int smp;
+ struct SLAVIO_TIMERState *slave[MAX_SUN4M_CPUS];
uint32_t slave_mode;
} SLAVIO_TIMERState;
@@ -93,6 +96,11 @@ static int slavio_timer_is_user(SLAVIO_T
return s->master && (s->master->slave_mode & (1 << s->slave_index));
}
+static int slavio_timer_is_mapped(SLAVIO_TIMERState *s)
+{
+ return s->master && (!s->master->smp && s->slave_index > 1);
+}
+
// Update count, set irq, update expire_time
// Convert from ptimer countdown units
static void slavio_timer_get_out(SLAVIO_TIMERState *s)
@@ -104,10 +112,7 @@ static void slavio_timer_get_out(SLAVIO_
else
limit = s->limit;
- 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));
DPRINTF("get_out: limit %" PRIx64 " count %x%08x\n", s->limit,
s->counthigh, s->count);
@@ -122,10 +127,9 @@ static void slavio_timer_irq(void *opaqu
slavio_timer_get_out(s);
DPRINTF("callback: count %x%08x\n", s->counthigh, s->count);
- if (!slavio_timer_is_user(s)) {
- s->reached = TIMER_REACHED;
+ s->reached = TIMER_REACHED;
+ if (!slavio_timer_is_user(s))
qemu_irq_raise(s->irq);
- }
}
static uint32_t slavio_timer_mem_readl(void *opaque, target_phys_addr_t addr)
@@ -133,6 +137,9 @@ static uint32_t slavio_timer_mem_readl(v
SLAVIO_TIMERState *s = opaque;
uint32_t saddr, ret;
+ if (slavio_timer_is_mapped(s))
+ s = s->master->slave[0];
+
saddr = (addr & TIMER_MAXADDR) >> 2;
switch (saddr) {
case TIMER_LIMIT:
@@ -141,7 +148,7 @@ static uint32_t slavio_timer_mem_readl(v
if (slavio_timer_is_user(s)) {
// read user timer MSW
slavio_timer_get_out(s);
- ret = s->counthigh;
+ ret = s->counthigh | s->reached;
} else {
// read limit
// clear irq
@@ -183,89 +190,112 @@ static void slavio_timer_mem_writel(void
uint32_t val)
{
SLAVIO_TIMERState *s = opaque;
+#ifdef DEBUG_TIMER
+ SLAVIO_TIMERState *original = opaque;
+#endif
uint32_t saddr;
+ if (slavio_timer_is_mapped(s))
+ s = s->master->slave[0];
+
DPRINTF("write " TARGET_FMT_plx " %08x\n", addr, val);
saddr = (addr & TIMER_MAXADDR) >> 2;
switch (saddr) {
case TIMER_LIMIT:
if (slavio_timer_is_user(s)) {
+ uint64_t count;
// set user counter MSW, reset counter
qemu_irq_lower(s->irq);
- 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);
- 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));
} else
DPRINTF("not user timer\n");
break;
case TIMER_COUNTER_NORST:
// set limit without resetting counter
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), 0);
- else
- ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 0);
- }
+ if (s->limit == 0) /* free-run */
+ ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 0);
+ else
+ ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(s->limit), 0);
break;
case TIMER_STATUS:
if (slavio_timer_is_user(s)) {
// start/stop user counter
if ((val & 1) && !s->running) {
- DPRINTF("processor %d user timer started\n", s->slave_index);
- if (s->timer)
- ptimer_run(s->timer, 0);
+ DPRINTF("processor %d user timer started\n", original->slave_index);
+ ptimer_run(s->timer, 0);
s->running = 1;
} else if (!(val & 1) && s->running) {
- DPRINTF("processor %d user timer stopped\n", s->slave_index);
- if (s->timer)
- ptimer_stop(s->timer);
+ DPRINTF("processor %d user timer stopped\n", original->slave_index);
+ ptimer_stop(s->timer);
s->running = 0;
}
}
break;
case TIMER_MODE:
if (s->master == NULL) {
- unsigned int i;
+ unsigned int i, num_slaves = s->smp ? MAX_SUN4M_CPUS : 1;
- 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)
+ 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);
+ }
}
}
- s->slave_mode = val & ((1 << s->num_slaves) - 1);
} else
DPRINTF("not system timer\n");
break;
@@ -296,8 +326,7 @@ static void slavio_timer_save(QEMUFile *
qemu_put_be32s(f, &s->counthigh);
qemu_put_be32s(f, &s->reached);
qemu_put_be32s(f, &s->running);
- if (s->timer)
- qemu_put_ptimer(f, s->timer);
+ qemu_put_ptimer(f, s->timer);
}
static int slavio_timer_load(QEMUFile *f, void *opaque, int version_id)
@@ -312,8 +341,7 @@ static int slavio_timer_load(QEMUFile *f
qemu_get_be32s(f, &s->counthigh);
qemu_get_be32s(f, &s->reached);
qemu_get_be32s(f, &s->running);
- if (s->timer)
- qemu_get_ptimer(f, s->timer);
+ qemu_get_ptimer(f, s->timer);
return 0;
}
@@ -326,10 +354,8 @@ static void slavio_timer_reset(void *opa
s->count = 0;
s->reached = 0;
s->slave_mode = 0;
- if (!s->master || s->slave_index < s->master->num_slaves) {
- ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
- ptimer_run(s->timer, 0);
- }
+ ptimer_set_limit(s->timer, LIMIT_TO_PERIODS(TIMER_MAX_COUNT32), 1);
+ ptimer_run(s->timer, 0);
s->running = 1;
qemu_irq_lower(s->irq);
}
@@ -337,7 +363,7 @@ static void slavio_timer_reset(void *opa
static SLAVIO_TIMERState *slavio_timer_init(target_phys_addr_t addr,
qemu_irq irq,
SLAVIO_TIMERState *master,
- 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);
- master->num_slaves = num_cpus;
+ master->smp = smp;
- 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);
}
}
Index: hw/sun4m.c
===================================================================
RCS file: /sources/qemu/qemu/hw/sun4m.c,v
retrieving revision 1.79
diff -p -u -r1.79 sun4m.c
--- hw/sun4m.c 1 Jan 2008 20:57:25 -0000 1.79
+++ hw/sun4m.c 5 Jan 2008 00:43:28 -0000
@@ -75,7 +75,6 @@
#define PROM_VADDR 0xffd00000
#define PROM_FILENAME "openbios-sparc32"
-#define MAX_CPUS 16
#define MAX_PILS 16
struct hwdef {
@@ -366,10 +365,10 @@ static void sun4m_hw_init(const struct h
const char *initrd_filename, const char *cpu_model)
{
- CPUState *env, *envs[MAX_CPUS];
+ CPUState *env, *envs[MAX_SUN4M_CPUS];
unsigned int i;
void *iommu, *espdma, *ledma, *main_esp, *nvram;
- qemu_irq *cpu_irqs[MAX_CPUS], *slavio_irq, *slavio_cpu_irq,
+ qemu_irq *cpu_irqs[MAX_SUN4M_CPUS], *slavio_irq, *slavio_cpu_irq,
*espdma_irq, *ledma_irq;
qemu_irq *esp_reset, *le_reset;
unsigned long prom_offset, kernel_size;
@@ -378,6 +377,15 @@ static void sun4m_hw_init(const struct h
BlockDriverState *fd[MAX_FD];
int index;
+ /* sun4m SMP systems support up to 4 CPUs */
+ 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);
+ }
+
/* init CPUs */
if (!cpu_model)
cpu_model = hwdef->default_cpu_model;
@@ -385,7 +393,7 @@ static void sun4m_hw_init(const struct h
for(i = 0; i < smp_cpus; i++) {
env = cpu_init(cpu_model);
if (!env) {
- fprintf(stderr, "Unable to find Sparc CPU definition\n");
+ fprintf(stderr, "qemu: Unable to find Sparc CPU definition\n");
exit(1);
}
cpu_sparc_set_id(env, i);
@@ -401,13 +409,14 @@ static void sun4m_hw_init(const struct h
env->prom_addr = hwdef->slavio_base;
}
- for (i = smp_cpus; i < MAX_CPUS; i++)
+ for (i = smp_cpus; i < MAX_SUN4M_CPUS; i++)
cpu_irqs[i] = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, MAX_PILS);
/* allocate RAM */
if ((uint64_t)RAM_size > hwdef->max_mem) {
- fprintf(stderr, "qemu: Too much memory for this machine: %d, maximum %d\n",
+ fprintf(stderr,
+ "qemu: Too much memory for this machine: %d, maximum %d\n",
(unsigned int)RAM_size / (1024 * 1024),
(unsigned int)(hwdef->max_mem / (1024 * 1024)));
exit(1);
@@ -481,7 +490,7 @@ static void sun4m_hw_init(const struct h
hwdef->nvram_size, 8);
slavio_timer_init_all(hwdef->counter_base, slavio_irq[hwdef->clock1_irq],
- slavio_cpu_irq, smp_cpus);
+ slavio_cpu_irq, hwdef->machine_id != 0x80);
slavio_serial_ms_kbd_init(hwdef->ms_kb_base, slavio_irq[hwdef->ms_kb_irq],
nographic);
@@ -548,13 +557,20 @@ static void sun4c_hw_init(const struct h
BlockDriverState *fd[MAX_FD];
int index;
+ /* sun4c is single processor only */
+ if (smp_cpus != 1) {
+ fprintf(stderr, "qemu: Too many CPUs for this machine: %d maiximum 1\n",
+ smp_cpus);
+ exit(1);
+ }
+
/* init CPU */
if (!cpu_model)
cpu_model = hwdef->default_cpu_model;
env = cpu_init(cpu_model);
if (!env) {
- fprintf(stderr, "Unable to find Sparc CPU definition\n");
+ fprintf(stderr, "qemu: Unable to find Sparc CPU definition\n");
exit(1);
}
@@ -567,7 +583,8 @@ static void sun4c_hw_init(const struct h
/* allocate RAM */
if ((uint64_t)RAM_size > hwdef->max_mem) {
- fprintf(stderr, "qemu: Too much memory for this machine: %d, maximum %d\n",
+ fprintf(stderr,
+ "qemu: Too much memory for this machine: %d, maximum %d\n",
(unsigned int)RAM_size / (1024 * 1024),
(unsigned int)hwdef->max_mem / (1024 * 1024));
exit(1);
@@ -1023,10 +1040,10 @@ static void sun4d_hw_init(const struct s
const char *kernel_cmdline,
const char *initrd_filename, const char *cpu_model)
{
- CPUState *env, *envs[MAX_CPUS];
+ CPUState *env, *envs[MAX_SUN4D_CPUS];
unsigned int i;
void *iounits[MAX_IOUNITS], *espdma, *ledma, *main_esp, *nvram, *sbi;
- qemu_irq *cpu_irqs[MAX_CPUS], *sbi_irq, *sbi_cpu_irq,
+ qemu_irq *cpu_irqs[MAX_SUN4D_CPUS], *sbi_irq, *sbi_cpu_irq,
*espdma_irq, *ledma_irq;
qemu_irq *esp_reset, *le_reset;
unsigned long prom_offset, kernel_size;
@@ -1034,6 +1051,13 @@ static void sun4d_hw_init(const struct s
char buf[1024];
int index;
+ if (smp_cpus > MAX_SUN4D_CPUS) {
+ fprintf(stderr,
+ "qemu: Too many CPUs for this machine: %d maiximum %d\n",
+ smp_cpus, MAX_SUN4D_CPUS);
+ exit(1);
+ }
+
/* init CPUs */
if (!cpu_model)
cpu_model = hwdef->default_cpu_model;
@@ -1041,7 +1065,7 @@ static void sun4d_hw_init(const struct s
for (i = 0; i < smp_cpus; i++) {
env = cpu_init(cpu_model);
if (!env) {
- fprintf(stderr, "Unable to find Sparc CPU definition\n");
+ fprintf(stderr, "qemu: Unable to find Sparc CPU definition\n");
exit(1);
}
cpu_sparc_set_id(env, i);
@@ -1057,12 +1081,13 @@ static void sun4d_hw_init(const struct s
env->prom_addr = hwdef->slavio_base;
}
- for (i = smp_cpus; i < MAX_CPUS; i++)
+ for (i = smp_cpus; i < MAX_SUN4D_CPUS; i++)
cpu_irqs[i] = qemu_allocate_irqs(dummy_cpu_set_irq, NULL, MAX_PILS);
/* allocate RAM */
if ((uint64_t)RAM_size > hwdef->max_mem) {
- fprintf(stderr, "qemu: Too much memory for this machine: %d, maximum %d\n",
+ fprintf(stderr,
+ "qemu: Too much memory for this machine: %d, maximum %d\n",
(unsigned int)RAM_size / (1024 * 1024),
(unsigned int)(hwdef->max_mem / (1024 * 1024)));
exit(1);
@@ -1083,8 +1108,7 @@ static void sun4d_hw_init(const struct s
if (ret < 0 || ret > PROM_SIZE_MAX)
ret = load_image(buf, phys_ram_base + prom_offset);
if (ret < 0 || ret > PROM_SIZE_MAX) {
- fprintf(stderr, "qemu: could not load prom '%s'\n",
- buf);
+ fprintf(stderr, "qemu: could not load prom '%s'\n", buf);
exit(1);
}
@@ -1125,7 +1149,7 @@ static void sun4d_hw_init(const struct s
hwdef->nvram_size, 8);
slavio_timer_init_all(hwdef->counter_base, sbi_irq[hwdef->clock1_irq],
- sbi_cpu_irq, smp_cpus);
+ sbi_cpu_irq, 1);
slavio_serial_ms_kbd_init(hwdef->ms_kb_base, sbi_irq[hwdef->ms_kb_irq],
nographic);
Index: hw/sun4m.h
===================================================================
RCS file: /sources/qemu/qemu/hw/sun4m.h,v
retrieving revision 1.8
diff -p -u -r1.8 sun4m.h
--- hw/sun4m.h 1 Jan 2008 17:04:45 -0000 1.8
+++ hw/sun4m.h 5 Jan 2008 00:43:28 -0000
@@ -1,6 +1,9 @@
#ifndef SUN4M_H
#define SUN4M_H
+#define MAX_SUN4M_CPUS 4
+#define MAX_SUN4D_CPUS 20
+
/* Devices used by sparc32 system. */
/* iommu.c */
@@ -44,7 +47,7 @@ void *sun4c_intctl_init(target_phys_addr
/* slavio_timer.c */
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_serial.c */
SerialState *slavio_serial_init(target_phys_addr_t base, qemu_irq irq,
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] sparc32: fix per cpu counter/timer
2008-01-05 1:08 [Qemu-devel] [PATCH] sparc32: fix per cpu counter/timer Robert Reif
@ 2008-01-05 11:30 ` Blue Swirl
2008-01-05 15:07 ` Robert Reif
0 siblings, 1 reply; 3+ messages in thread
From: Blue Swirl @ 2008-01-05 11:30 UTC (permalink / raw)
To: qemu-devel
On 1/5/08, Robert Reif <reif@earthlink.net> wrote:
> 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.
The idea for this part is OK, but I don't like some parts of the
implementation, see below.
> 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.
These changes should be in separate patches.
> 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.
Well, why bother then?
> - 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.
> +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.
> - 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.
> + if (slavio_timer_is_mapped(s))
> + s = s->master->slave[0];
> +
And here.
> - 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);
> - 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.
> - 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)
> + 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.
> static SLAVIO_TIMERState *slavio_timer_init(target_phys_addr_t addr,
> qemu_irq irq,
> SLAVIO_TIMERState *master,
> - 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);
>
> - master->num_slaves = num_cpus;
> + master->smp = smp;
>
> - 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.
> + 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.
> - 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] sparc32: fix per cpu counter/timer
2008-01-05 11:30 ` Blue Swirl
@ 2008-01-05 15:07 ` Robert Reif
0 siblings, 0 replies; 3+ messages in thread
From: Robert Reif @ 2008-01-05 15:07 UTC (permalink / raw)
To: qemu-devel
This patch is trying to make qemu behave like real hardware. This is what
the OSs expect. The ability to create hardware that never existed and
can't
exist due to real hardware limitations is cool but it's not going to work
properly with existing OSs. At best you will have the OS never accessing
the extra hardware because of known hardwired limits in the OS or worse,
you will have the OS accessing off the end of fixed size arrays. Neither
are fixable without changing the OS. I know that the boot prom provides
a layer of abstraction between the hardware and the OS and linux is very
trusting of what the prom provides, even when it makes no sense.
However there are some assumptions that the OS writers knew about and
do ignore what the prom says. You can have the prom tell linux that
there a a million CPUs and it will happily print out that the prom said
there are a million CPUs but it knows that there are 4 and has fixed size
arrays for just those 4 :-)
Blue Swirl wrote:
>>- 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.
>
The number is a constant, 1 for UP and 4 for SMP. That's defined
by the real hardware. The OS writers knew that.
>>+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.
>
>
This doesn't change any logic. It's just a helper to determine if the
counter/timer
being accessed is really the one at that address or another one being
accessed
from a different address.
>>- 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.
>
>
There are never any counter/timers created with s->timer being NULL.
We are
creating alternate address ranges for the mapped counter/timers so qemu
is happy
but we redirect access to those spaces to a single real counter/timer.
The alternate
address spaces are never accessed. Thats why they and not initialized,
reset, loaded
or saved.
>>+ if (slavio_timer_is_mapped(s))
>>+ s = s->master->slave[0];
>>+
>>
>>
>
>And here.
>
>
Here we are redirecting accesses to mapped counter/timers to the single
real one.
>>- 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);
>>- 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.
>
>
>
>>- 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)
>>+ 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.
>
>
The original code was really broken and not salvagable. This code only
makes changes when something actually changes and makes the proper
changes to reflect how the hardware actually works. The code immediately
above this also makes sure that that only real hardware is accessed. I
could
break this up into less stages of brokenness until it's finally fixed
but those
intermediate patches would still be broken, just less so.
>> static SLAVIO_TIMERState *slavio_timer_init(target_phys_addr_t addr,
>> qemu_irq irq,
>> SLAVIO_TIMERState *master,
>>- 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);
>>
>>- master->num_slaves = num_cpus;
>>+ master->smp = smp;
>>
>>- 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.
>
>
The counter/timer is not disabled. It is not there. The memory space
is allocated
for it in qemu but all accesses are redirected (mapped) to the single
real counter/timer.
>>+ 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.
>
>
That's the whole point of this patch. There never were any sun4m SMP
machines
with more than 4 CPUs and there never will be. OS writers know that.
That's
why linux has fixed size arrays for per-cpu resources. It's a a
hardware and
architectural limitation for sun4m machines. You would need to patch linux
to make more than 4 CPU work. What's the point of doing that. qemu doesn't
really support smp anyway. SMP instruction locking isn't implemented in
qemu.
>>- 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.
>
>
>
All but a few have qemu: in the error message. I'm just correcting an
over site but that
can go in a split up patch.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-01-05 15:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-05 1:08 [Qemu-devel] [PATCH] sparc32: fix per cpu counter/timer Robert Reif
2008-01-05 11:30 ` Blue Swirl
2008-01-05 15:07 ` Robert Reif
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).