| Message ID | 3a9be7f66702c6fb18ab09ad9db0fdd50b1a7df6.1625801410.git.alistair.francis@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/5] target/riscv: Expose interrupt pending bits as GPIO lines | expand |
On 7/8/21 8:30 PM, Alistair Francis wrote: > +typedef struct sifive_clint_callback { > + SiFiveCLINTState *s; > + int num; > +} sifive_clint_callback; Perhaps better to put "num", perhaps with a more descriptive name (hartid?), into SiFiveCLINTState itself? It would avoid some amount of double-indirection, and some awkward memory allocation in sifive_clint_create. > } else if ((addr & 0x3) == 0) { > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MSIP, BOOL_TO_MASK(value)); > + if (value) { > + qemu_irq_raise(clint->soft_irqs[hartid]); > + } else { > + qemu_irq_lower(clint->soft_irqs[hartid]); > + } You should use qemu_irq_set here. r~
On Sat, Jul 10, 2021 at 1:36 AM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 7/8/21 8:30 PM, Alistair Francis wrote: > > +typedef struct sifive_clint_callback { > > + SiFiveCLINTState *s; > > + int num; > > +} sifive_clint_callback; > > Perhaps better to put "num", perhaps with a more descriptive name (hartid?), into > SiFiveCLINTState itself? The problem is that there is a single SiFiveCLINTState because there is a single CLINT, but we want to have a timer callback for each CPU so we need something here that is per CPU. > > It would avoid some amount of double-indirection, and some awkward memory allocation in > sifive_clint_create. > > > > } else if ((addr & 0x3) == 0) { > > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MSIP, BOOL_TO_MASK(value)); > > + if (value) { > > + qemu_irq_raise(clint->soft_irqs[hartid]); > > + } else { > > + qemu_irq_lower(clint->soft_irqs[hartid]); > > + } > > You should use qemu_irq_set here. Will do! Alistair > > > r~
On Fri, Jul 9, 2021 at 9:01 AM Alistair Francis <alistair.francis@wdc.com> wrote: > > Instead of using riscv_cpu_update_mip() let's instead use the new RISC-V > CPU GPIO lines to set the timer and soft MIP bits. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > include/hw/intc/sifive_clint.h | 2 + > hw/intc/sifive_clint.c | 72 ++++++++++++++++++++++++---------- > 2 files changed, 54 insertions(+), 20 deletions(-) > > diff --git a/include/hw/intc/sifive_clint.h b/include/hw/intc/sifive_clint.h > index a30be0f3d6..921b1561dd 100644 > --- a/include/hw/intc/sifive_clint.h > +++ b/include/hw/intc/sifive_clint.h > @@ -40,6 +40,8 @@ typedef struct SiFiveCLINTState { > uint32_t time_base; > uint32_t aperture_size; > uint32_t timebase_freq; > + qemu_irq *timer_irqs; > + qemu_irq *soft_irqs; > } SiFiveCLINTState; > > DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, > diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c > index 0f41e5ea1c..c635a47507 100644 > --- a/hw/intc/sifive_clint.c > +++ b/hw/intc/sifive_clint.c > @@ -28,6 +28,12 @@ > #include "hw/qdev-properties.h" > #include "hw/intc/sifive_clint.h" > #include "qemu/timer.h" > +#include "hw/irq.h" > + > +typedef struct sifive_clint_callback { > + SiFiveCLINTState *s; > + int num; > +} sifive_clint_callback; > > static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > { > @@ -39,7 +45,9 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > * Called when timecmp is written to update the QEMU timer or immediately > * trigger timer interrupt if mtimecmp <= current timer value. > */ > -static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > +static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, > + int hartid, > + uint64_t value, > uint32_t timebase_freq) > { > uint64_t next; > @@ -51,12 +59,12 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > if (cpu->env.timecmp <= rtc_r) { > /* if we're setting an MTIMECMP value in the "past", > immediately raise the timer interrupt */ > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); > + qemu_irq_raise(s->timer_irqs[hartid]); This breaks multi-socket support. Please use "hartid - s->hartid_base" as an index. > return; > } > > /* otherwise, set up the future timer interrupt */ > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); > + qemu_irq_lower(s->timer_irqs[hartid]); > diff = cpu->env.timecmp - rtc_r; > /* back to ns (note args switched in muldiv64) */ > next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > @@ -70,8 +78,9 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > */ > static void sifive_clint_timer_cb(void *opaque) > { > - RISCVCPU *cpu = opaque; > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); > + sifive_clint_callback *state = opaque; > + > + qemu_irq_raise(state->s->timer_irqs[state->num]); > } > > /* CPU wants to read rtc or timecmp register */ > @@ -137,7 +146,11 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, > if (!env) { > error_report("clint: invalid timecmp hartid: %zu", hartid); > } else if ((addr & 0x3) == 0) { > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MSIP, BOOL_TO_MASK(value)); > + if (value) { > + qemu_irq_raise(clint->soft_irqs[hartid]); > + } else { > + qemu_irq_lower(clint->soft_irqs[hartid]); > + } Eventthis is broken for multi-socket. Use "hartid - clint->hartid_base" as index. > } else { > error_report("clint: invalid sip write: %08x", (uint32_t)addr); > } > @@ -153,13 +166,13 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, > } else if ((addr & 0x7) == 0) { > /* timecmp_lo */ > uint64_t timecmp_hi = env->timecmp >> 32; > - sifive_clint_write_timecmp(RISCV_CPU(cpu), > + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, > timecmp_hi << 32 | (value & 0xFFFFFFFF), clint->timebase_freq); > return; > } else if ((addr & 0x7) == 4) { > /* timecmp_hi */ > uint64_t timecmp_lo = env->timecmp; > - sifive_clint_write_timecmp(RISCV_CPU(cpu), > + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, > value << 32 | (timecmp_lo & 0xFFFFFFFF), clint->timebase_freq); > } else { > error_report("clint: invalid timecmp write: %08x", (uint32_t)addr); > @@ -205,6 +218,12 @@ static void sifive_clint_realize(DeviceState *dev, Error **errp) > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_clint_ops, s, > TYPE_SIFIVE_CLINT, s->aperture_size); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > + > + s->timer_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); > + qdev_init_gpio_out(dev, s->timer_irqs, s->num_harts); > + > + s->soft_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); > + qdev_init_gpio_out(dev, s->soft_irqs, s->num_harts); > } > > static void sifive_clint_class_init(ObjectClass *klass, void *data) > @@ -228,7 +247,6 @@ static void sifive_clint_register_types(void) > > type_init(sifive_clint_register_types) > > - > /* > * Create CLINT device. > */ > @@ -238,29 +256,43 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, > bool provide_rdtime) > { > int i; > + > + DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); > + qdev_prop_set_uint32(dev, "hartid-base", hartid_base); > + qdev_prop_set_uint32(dev, "num-harts", num_harts); > + qdev_prop_set_uint32(dev, "sip-base", sip_base); > + qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); > + qdev_prop_set_uint32(dev, "time-base", time_base); > + qdev_prop_set_uint32(dev, "aperture-size", size); > + qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > + > for (i = 0; i < num_harts; i++) { > CPUState *cpu = qemu_get_cpu(hartid_base + i); > + RISCVCPU *rvcpu = RISCV_CPU(cpu); > CPURISCVState *env = cpu ? cpu->env_ptr : NULL; > + sifive_clint_callback *cb = g_malloc0(sizeof(sifive_clint_callback)); > + > if (!env) { > + g_free(cb); > continue; > } > if (provide_rdtime) { > riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, timebase_freq); > } > + > + cb->s = SIFIVE_CLINT(dev); > + cb->num = i; > env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > - &sifive_clint_timer_cb, cpu); > + &sifive_clint_timer_cb, cb); > env->timecmp = 0; > + > + qdev_connect_gpio_out_named(dev, NULL, i, > + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_TIMER)); > + qdev_connect_gpio_out_named(dev, NULL, num_harts + i, > + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_SOFT)); > } > > - DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); > - qdev_prop_set_uint32(dev, "hartid-base", hartid_base); > - qdev_prop_set_uint32(dev, "num-harts", num_harts); > - qdev_prop_set_uint32(dev, "sip-base", sip_base); > - qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); > - qdev_prop_set_uint32(dev, "time-base", time_base); > - qdev_prop_set_uint32(dev, "aperture-size", size); > - qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); > - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > return dev; > } > -- > 2.31.1 > > Regards, Anup
On Tue, Jul 13, 2021 at 2:06 PM Anup Patel <anup@brainfault.org> wrote: > > On Fri, Jul 9, 2021 at 9:01 AM Alistair Francis > <alistair.francis@wdc.com> wrote: > > > > Instead of using riscv_cpu_update_mip() let's instead use the new RISC-V > > CPU GPIO lines to set the timer and soft MIP bits. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > include/hw/intc/sifive_clint.h | 2 + > > hw/intc/sifive_clint.c | 72 ++++++++++++++++++++++++---------- > > 2 files changed, 54 insertions(+), 20 deletions(-) > > > > diff --git a/include/hw/intc/sifive_clint.h b/include/hw/intc/sifive_clint.h > > index a30be0f3d6..921b1561dd 100644 > > --- a/include/hw/intc/sifive_clint.h > > +++ b/include/hw/intc/sifive_clint.h > > @@ -40,6 +40,8 @@ typedef struct SiFiveCLINTState { > > uint32_t time_base; > > uint32_t aperture_size; > > uint32_t timebase_freq; > > + qemu_irq *timer_irqs; > > + qemu_irq *soft_irqs; > > } SiFiveCLINTState; > > > > DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, > > diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c > > index 0f41e5ea1c..c635a47507 100644 > > --- a/hw/intc/sifive_clint.c > > +++ b/hw/intc/sifive_clint.c > > @@ -28,6 +28,12 @@ > > #include "hw/qdev-properties.h" > > #include "hw/intc/sifive_clint.h" > > #include "qemu/timer.h" > > +#include "hw/irq.h" > > + > > +typedef struct sifive_clint_callback { > > + SiFiveCLINTState *s; > > + int num; > > +} sifive_clint_callback; > > > > static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > > { > > @@ -39,7 +45,9 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > > * Called when timecmp is written to update the QEMU timer or immediately > > * trigger timer interrupt if mtimecmp <= current timer value. > > */ > > -static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > > +static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, > > + int hartid, > > + uint64_t value, > > uint32_t timebase_freq) > > { > > uint64_t next; > > @@ -51,12 +59,12 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > > if (cpu->env.timecmp <= rtc_r) { > > /* if we're setting an MTIMECMP value in the "past", > > immediately raise the timer interrupt */ > > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); > > + qemu_irq_raise(s->timer_irqs[hartid]); > > This breaks multi-socket support. > > Please use "hartid - s->hartid_base" as an index. > > > return; > > } > > > > /* otherwise, set up the future timer interrupt */ > > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); > > + qemu_irq_lower(s->timer_irqs[hartid]); > > diff = cpu->env.timecmp - rtc_r; > > /* back to ns (note args switched in muldiv64) */ > > next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > > @@ -70,8 +78,9 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > > */ > > static void sifive_clint_timer_cb(void *opaque) > > { > > - RISCVCPU *cpu = opaque; > > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); > > + sifive_clint_callback *state = opaque; > > + > > + qemu_irq_raise(state->s->timer_irqs[state->num]); > > } > > > > /* CPU wants to read rtc or timecmp register */ > > @@ -137,7 +146,11 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, > > if (!env) { > > error_report("clint: invalid timecmp hartid: %zu", hartid); > > } else if ((addr & 0x3) == 0) { > > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MSIP, BOOL_TO_MASK(value)); > > + if (value) { > > + qemu_irq_raise(clint->soft_irqs[hartid]); > > + } else { > > + qemu_irq_lower(clint->soft_irqs[hartid]); > > + } > > Eventthis is broken for multi-socket. > > Use "hartid - clint->hartid_base" as index. Thanks for testing this. I have fixed this and will send a v2. I also added multi-socket tests to my automated tests. Alistair > > > } else { > > error_report("clint: invalid sip write: %08x", (uint32_t)addr); > > } > > @@ -153,13 +166,13 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, > > } else if ((addr & 0x7) == 0) { > > /* timecmp_lo */ > > uint64_t timecmp_hi = env->timecmp >> 32; > > - sifive_clint_write_timecmp(RISCV_CPU(cpu), > > + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, > > timecmp_hi << 32 | (value & 0xFFFFFFFF), clint->timebase_freq); > > return; > > } else if ((addr & 0x7) == 4) { > > /* timecmp_hi */ > > uint64_t timecmp_lo = env->timecmp; > > - sifive_clint_write_timecmp(RISCV_CPU(cpu), > > + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, > > value << 32 | (timecmp_lo & 0xFFFFFFFF), clint->timebase_freq); > > } else { > > error_report("clint: invalid timecmp write: %08x", (uint32_t)addr); > > @@ -205,6 +218,12 @@ static void sifive_clint_realize(DeviceState *dev, Error **errp) > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_clint_ops, s, > > TYPE_SIFIVE_CLINT, s->aperture_size); > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > + > > + s->timer_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); > > + qdev_init_gpio_out(dev, s->timer_irqs, s->num_harts); > > + > > + s->soft_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); > > + qdev_init_gpio_out(dev, s->soft_irqs, s->num_harts); > > } > > > > static void sifive_clint_class_init(ObjectClass *klass, void *data) > > @@ -228,7 +247,6 @@ static void sifive_clint_register_types(void) > > > > type_init(sifive_clint_register_types) > > > > - > > /* > > * Create CLINT device. > > */ > > @@ -238,29 +256,43 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, > > bool provide_rdtime) > > { > > int i; > > + > > + DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); > > + qdev_prop_set_uint32(dev, "hartid-base", hartid_base); > > + qdev_prop_set_uint32(dev, "num-harts", num_harts); > > + qdev_prop_set_uint32(dev, "sip-base", sip_base); > > + qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); > > + qdev_prop_set_uint32(dev, "time-base", time_base); > > + qdev_prop_set_uint32(dev, "aperture-size", size); > > + qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); > > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > > + > > for (i = 0; i < num_harts; i++) { > > CPUState *cpu = qemu_get_cpu(hartid_base + i); > > + RISCVCPU *rvcpu = RISCV_CPU(cpu); > > CPURISCVState *env = cpu ? cpu->env_ptr : NULL; > > + sifive_clint_callback *cb = g_malloc0(sizeof(sifive_clint_callback)); > > + > > if (!env) { > > + g_free(cb); > > continue; > > } > > if (provide_rdtime) { > > riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, timebase_freq); > > } > > + > > + cb->s = SIFIVE_CLINT(dev); > > + cb->num = i; > > env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > > - &sifive_clint_timer_cb, cpu); > > + &sifive_clint_timer_cb, cb); > > env->timecmp = 0; > > + > > + qdev_connect_gpio_out_named(dev, NULL, i, > > + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_TIMER)); > > + qdev_connect_gpio_out_named(dev, NULL, num_harts + i, > > + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_SOFT)); > > } > > > > - DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); > > - qdev_prop_set_uint32(dev, "hartid-base", hartid_base); > > - qdev_prop_set_uint32(dev, "num-harts", num_harts); > > - qdev_prop_set_uint32(dev, "sip-base", sip_base); > > - qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); > > - qdev_prop_set_uint32(dev, "time-base", time_base); > > - qdev_prop_set_uint32(dev, "aperture-size", size); > > - qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); > > - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > > return dev; > > } > > -- > > 2.31.1 > > > > > > Regards, > Anup
On Wed, Jul 14, 2021 at 11:41 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Tue, Jul 13, 2021 at 2:06 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Fri, Jul 9, 2021 at 9:01 AM Alistair Francis > > <alistair.francis@wdc.com> wrote: > > > > > > Instead of using riscv_cpu_update_mip() let's instead use the new RISC-V > > > CPU GPIO lines to set the timer and soft MIP bits. > > > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > > --- > > > include/hw/intc/sifive_clint.h | 2 + > > > hw/intc/sifive_clint.c | 72 ++++++++++++++++++++++++---------- > > > 2 files changed, 54 insertions(+), 20 deletions(-) > > > > > > diff --git a/include/hw/intc/sifive_clint.h b/include/hw/intc/sifive_clint.h > > > index a30be0f3d6..921b1561dd 100644 > > > --- a/include/hw/intc/sifive_clint.h > > > +++ b/include/hw/intc/sifive_clint.h > > > @@ -40,6 +40,8 @@ typedef struct SiFiveCLINTState { > > > uint32_t time_base; > > > uint32_t aperture_size; > > > uint32_t timebase_freq; > > > + qemu_irq *timer_irqs; > > > + qemu_irq *soft_irqs; > > > } SiFiveCLINTState; > > > > > > DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, > > > diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c > > > index 0f41e5ea1c..c635a47507 100644 > > > --- a/hw/intc/sifive_clint.c > > > +++ b/hw/intc/sifive_clint.c > > > @@ -28,6 +28,12 @@ > > > #include "hw/qdev-properties.h" > > > #include "hw/intc/sifive_clint.h" > > > #include "qemu/timer.h" > > > +#include "hw/irq.h" > > > + > > > +typedef struct sifive_clint_callback { > > > + SiFiveCLINTState *s; > > > + int num; > > > +} sifive_clint_callback; > > > > > > static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > > > { > > > @@ -39,7 +45,9 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) > > > * Called when timecmp is written to update the QEMU timer or immediately > > > * trigger timer interrupt if mtimecmp <= current timer value. > > > */ > > > -static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > > > +static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, > > > + int hartid, > > > + uint64_t value, > > > uint32_t timebase_freq) > > > { > > > uint64_t next; > > > @@ -51,12 +59,12 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > > > if (cpu->env.timecmp <= rtc_r) { > > > /* if we're setting an MTIMECMP value in the "past", > > > immediately raise the timer interrupt */ > > > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); > > > + qemu_irq_raise(s->timer_irqs[hartid]); > > > > This breaks multi-socket support. > > > > Please use "hartid - s->hartid_base" as an index. > > > > > return; > > > } > > > > > > /* otherwise, set up the future timer interrupt */ > > > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); > > > + qemu_irq_lower(s->timer_irqs[hartid]); > > > diff = cpu->env.timecmp - rtc_r; > > > /* back to ns (note args switched in muldiv64) */ > > > next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > > > @@ -70,8 +78,9 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, > > > */ > > > static void sifive_clint_timer_cb(void *opaque) > > > { > > > - RISCVCPU *cpu = opaque; > > > - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); > > > + sifive_clint_callback *state = opaque; > > > + > > > + qemu_irq_raise(state->s->timer_irqs[state->num]); > > > } > > > > > > /* CPU wants to read rtc or timecmp register */ > > > @@ -137,7 +146,11 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, > > > if (!env) { > > > error_report("clint: invalid timecmp hartid: %zu", hartid); > > > } else if ((addr & 0x3) == 0) { > > > - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MSIP, BOOL_TO_MASK(value)); > > > + if (value) { > > > + qemu_irq_raise(clint->soft_irqs[hartid]); > > > + } else { > > > + qemu_irq_lower(clint->soft_irqs[hartid]); > > > + } > > > > Eventthis is broken for multi-socket. > > > > Use "hartid - clint->hartid_base" as index. > > Thanks for testing this. I have fixed this and will send a v2. I also > added multi-socket tests to my automated tests. Thanks, I will rebase the ACLINT series on your v2 series. Regards, Anup > > Alistair > > > > > > } else { > > > error_report("clint: invalid sip write: %08x", (uint32_t)addr); > > > } > > > @@ -153,13 +166,13 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, > > > } else if ((addr & 0x7) == 0) { > > > /* timecmp_lo */ > > > uint64_t timecmp_hi = env->timecmp >> 32; > > > - sifive_clint_write_timecmp(RISCV_CPU(cpu), > > > + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, > > > timecmp_hi << 32 | (value & 0xFFFFFFFF), clint->timebase_freq); > > > return; > > > } else if ((addr & 0x7) == 4) { > > > /* timecmp_hi */ > > > uint64_t timecmp_lo = env->timecmp; > > > - sifive_clint_write_timecmp(RISCV_CPU(cpu), > > > + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, > > > value << 32 | (timecmp_lo & 0xFFFFFFFF), clint->timebase_freq); > > > } else { > > > error_report("clint: invalid timecmp write: %08x", (uint32_t)addr); > > > @@ -205,6 +218,12 @@ static void sifive_clint_realize(DeviceState *dev, Error **errp) > > > memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_clint_ops, s, > > > TYPE_SIFIVE_CLINT, s->aperture_size); > > > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); > > > + > > > + s->timer_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); > > > + qdev_init_gpio_out(dev, s->timer_irqs, s->num_harts); > > > + > > > + s->soft_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); > > > + qdev_init_gpio_out(dev, s->soft_irqs, s->num_harts); > > > } > > > > > > static void sifive_clint_class_init(ObjectClass *klass, void *data) > > > @@ -228,7 +247,6 @@ static void sifive_clint_register_types(void) > > > > > > type_init(sifive_clint_register_types) > > > > > > - > > > /* > > > * Create CLINT device. > > > */ > > > @@ -238,29 +256,43 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, > > > bool provide_rdtime) > > > { > > > int i; > > > + > > > + DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); > > > + qdev_prop_set_uint32(dev, "hartid-base", hartid_base); > > > + qdev_prop_set_uint32(dev, "num-harts", num_harts); > > > + qdev_prop_set_uint32(dev, "sip-base", sip_base); > > > + qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); > > > + qdev_prop_set_uint32(dev, "time-base", time_base); > > > + qdev_prop_set_uint32(dev, "aperture-size", size); > > > + qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); > > > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > > > + > > > for (i = 0; i < num_harts; i++) { > > > CPUState *cpu = qemu_get_cpu(hartid_base + i); > > > + RISCVCPU *rvcpu = RISCV_CPU(cpu); > > > CPURISCVState *env = cpu ? cpu->env_ptr : NULL; > > > + sifive_clint_callback *cb = g_malloc0(sizeof(sifive_clint_callback)); > > > + > > > if (!env) { > > > + g_free(cb); > > > continue; > > > } > > > if (provide_rdtime) { > > > riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, timebase_freq); > > > } > > > + > > > + cb->s = SIFIVE_CLINT(dev); > > > + cb->num = i; > > > env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, > > > - &sifive_clint_timer_cb, cpu); > > > + &sifive_clint_timer_cb, cb); > > > env->timecmp = 0; > > > + > > > + qdev_connect_gpio_out_named(dev, NULL, i, > > > + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_TIMER)); > > > + qdev_connect_gpio_out_named(dev, NULL, num_harts + i, > > > + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_SOFT)); > > > } > > > > > > - DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); > > > - qdev_prop_set_uint32(dev, "hartid-base", hartid_base); > > > - qdev_prop_set_uint32(dev, "num-harts", num_harts); > > > - qdev_prop_set_uint32(dev, "sip-base", sip_base); > > > - qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); > > > - qdev_prop_set_uint32(dev, "time-base", time_base); > > > - qdev_prop_set_uint32(dev, "aperture-size", size); > > > - qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); > > > - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > > - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); > > > return dev; > > > } > > > -- > > > 2.31.1 > > > > > > > > > > Regards, > > Anup
diff --git a/include/hw/intc/sifive_clint.h b/include/hw/intc/sifive_clint.h index a30be0f3d6..921b1561dd 100644 --- a/include/hw/intc/sifive_clint.h +++ b/include/hw/intc/sifive_clint.h @@ -40,6 +40,8 @@ typedef struct SiFiveCLINTState { uint32_t time_base; uint32_t aperture_size; uint32_t timebase_freq; + qemu_irq *timer_irqs; + qemu_irq *soft_irqs; } SiFiveCLINTState; DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, diff --git a/hw/intc/sifive_clint.c b/hw/intc/sifive_clint.c index 0f41e5ea1c..c635a47507 100644 --- a/hw/intc/sifive_clint.c +++ b/hw/intc/sifive_clint.c @@ -28,6 +28,12 @@ #include "hw/qdev-properties.h" #include "hw/intc/sifive_clint.h" #include "qemu/timer.h" +#include "hw/irq.h" + +typedef struct sifive_clint_callback { + SiFiveCLINTState *s; + int num; +} sifive_clint_callback; static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) { @@ -39,7 +45,9 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq) * Called when timecmp is written to update the QEMU timer or immediately * trigger timer interrupt if mtimecmp <= current timer value. */ -static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, +static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu, + int hartid, + uint64_t value, uint32_t timebase_freq) { uint64_t next; @@ -51,12 +59,12 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, if (cpu->env.timecmp <= rtc_r) { /* if we're setting an MTIMECMP value in the "past", immediately raise the timer interrupt */ - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); + qemu_irq_raise(s->timer_irqs[hartid]); return; } /* otherwise, set up the future timer interrupt */ - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0)); + qemu_irq_lower(s->timer_irqs[hartid]); diff = cpu->env.timecmp - rtc_r; /* back to ns (note args switched in muldiv64) */ next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + @@ -70,8 +78,9 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value, */ static void sifive_clint_timer_cb(void *opaque) { - RISCVCPU *cpu = opaque; - riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(1)); + sifive_clint_callback *state = opaque; + + qemu_irq_raise(state->s->timer_irqs[state->num]); } /* CPU wants to read rtc or timecmp register */ @@ -137,7 +146,11 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, if (!env) { error_report("clint: invalid timecmp hartid: %zu", hartid); } else if ((addr & 0x3) == 0) { - riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MSIP, BOOL_TO_MASK(value)); + if (value) { + qemu_irq_raise(clint->soft_irqs[hartid]); + } else { + qemu_irq_lower(clint->soft_irqs[hartid]); + } } else { error_report("clint: invalid sip write: %08x", (uint32_t)addr); } @@ -153,13 +166,13 @@ static void sifive_clint_write(void *opaque, hwaddr addr, uint64_t value, } else if ((addr & 0x7) == 0) { /* timecmp_lo */ uint64_t timecmp_hi = env->timecmp >> 32; - sifive_clint_write_timecmp(RISCV_CPU(cpu), + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, timecmp_hi << 32 | (value & 0xFFFFFFFF), clint->timebase_freq); return; } else if ((addr & 0x7) == 4) { /* timecmp_hi */ uint64_t timecmp_lo = env->timecmp; - sifive_clint_write_timecmp(RISCV_CPU(cpu), + sifive_clint_write_timecmp(clint, RISCV_CPU(cpu), hartid, value << 32 | (timecmp_lo & 0xFFFFFFFF), clint->timebase_freq); } else { error_report("clint: invalid timecmp write: %08x", (uint32_t)addr); @@ -205,6 +218,12 @@ static void sifive_clint_realize(DeviceState *dev, Error **errp) memory_region_init_io(&s->mmio, OBJECT(dev), &sifive_clint_ops, s, TYPE_SIFIVE_CLINT, s->aperture_size); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); + + s->timer_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); + qdev_init_gpio_out(dev, s->timer_irqs, s->num_harts); + + s->soft_irqs = g_malloc(sizeof(qemu_irq) * s->num_harts); + qdev_init_gpio_out(dev, s->soft_irqs, s->num_harts); } static void sifive_clint_class_init(ObjectClass *klass, void *data) @@ -228,7 +247,6 @@ static void sifive_clint_register_types(void) type_init(sifive_clint_register_types) - /* * Create CLINT device. */ @@ -238,29 +256,43 @@ DeviceState *sifive_clint_create(hwaddr addr, hwaddr size, bool provide_rdtime) { int i; + + DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); + qdev_prop_set_uint32(dev, "hartid-base", hartid_base); + qdev_prop_set_uint32(dev, "num-harts", num_harts); + qdev_prop_set_uint32(dev, "sip-base", sip_base); + qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); + qdev_prop_set_uint32(dev, "time-base", time_base); + qdev_prop_set_uint32(dev, "aperture-size", size); + qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); + for (i = 0; i < num_harts; i++) { CPUState *cpu = qemu_get_cpu(hartid_base + i); + RISCVCPU *rvcpu = RISCV_CPU(cpu); CPURISCVState *env = cpu ? cpu->env_ptr : NULL; + sifive_clint_callback *cb = g_malloc0(sizeof(sifive_clint_callback)); + if (!env) { + g_free(cb); continue; } if (provide_rdtime) { riscv_cpu_set_rdtime_fn(env, cpu_riscv_read_rtc, timebase_freq); } + + cb->s = SIFIVE_CLINT(dev); + cb->num = i; env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, - &sifive_clint_timer_cb, cpu); + &sifive_clint_timer_cb, cb); env->timecmp = 0; + + qdev_connect_gpio_out_named(dev, NULL, i, + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_TIMER)); + qdev_connect_gpio_out_named(dev, NULL, num_harts + i, + qdev_get_gpio_in(DEVICE(rvcpu), IRQ_M_SOFT)); } - DeviceState *dev = qdev_new(TYPE_SIFIVE_CLINT); - qdev_prop_set_uint32(dev, "hartid-base", hartid_base); - qdev_prop_set_uint32(dev, "num-harts", num_harts); - qdev_prop_set_uint32(dev, "sip-base", sip_base); - qdev_prop_set_uint32(dev, "timecmp-base", timecmp_base); - qdev_prop_set_uint32(dev, "time-base", time_base); - qdev_prop_set_uint32(dev, "aperture-size", size); - qdev_prop_set_uint32(dev, "timebase-freq", timebase_freq); - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); return dev; }
Instead of using riscv_cpu_update_mip() let's instead use the new RISC-V CPU GPIO lines to set the timer and soft MIP bits. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- include/hw/intc/sifive_clint.h | 2 + hw/intc/sifive_clint.c | 72 ++++++++++++++++++++++++---------- 2 files changed, 54 insertions(+), 20 deletions(-)