qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anup Patel <anup@brainfault.org>
To: Alistair Francis <alistair.francis@wdc.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	Bin Meng <bmeng.cn@gmail.com>,
	"open list:RISC-V" <qemu-riscv@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Alistair Francis <alistair23@gmail.com>
Subject: Re: [PATCH v1 2/5] hw/intc: sifive_clint: Use RISC-V CPU GPIO lines
Date: Tue, 13 Jul 2021 09:35:51 +0530	[thread overview]
Message-ID: <CAAhSdy3w7NbJLjUd3V2i98a4e3V9TKzQHic8inz1TERXMmW4+A@mail.gmail.com> (raw)
In-Reply-To: <3a9be7f66702c6fb18ab09ad9db0fdd50b1a7df6.1625801410.git.alistair.francis@wdc.com>

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


  parent reply	other threads:[~2021-07-13  4:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09  3:30 [PATCH v1 1/5] target/riscv: Expose interrupt pending bits as GPIO lines Alistair Francis
2021-07-09  3:30 ` [PATCH v1 2/5] hw/intc: sifive_clint: Use RISC-V CPU " Alistair Francis
2021-07-09 15:36   ` Richard Henderson
2021-07-12  4:49     ` Alistair Francis
2021-07-13  4:05   ` Anup Patel [this message]
2021-07-14  6:11     ` Alistair Francis
2021-07-22 12:15       ` Anup Patel
2021-07-09  3:31 ` [PATCH v1 3/5] hw/intc: ibex_plic: Convert the PLIC to use " Alistair Francis
2021-07-09  7:26   ` Philippe Mathieu-Daudé
2021-07-09 15:39   ` Richard Henderson
2021-07-09  3:31 ` [PATCH v1 4/5] hw/intc: sifive_plic: " Alistair Francis
2021-07-09  7:26   ` Philippe Mathieu-Daudé
2021-07-09 15:41   ` Richard Henderson
2021-07-13  4:31   ` Anup Patel
2021-07-13  5:08   ` Anup Patel
2021-07-09  3:31 ` [PATCH v1 5/5] hw/intc: ibex_timer: Convert the timer " Alistair Francis
2021-07-09  7:26   ` Philippe Mathieu-Daudé
2021-07-09 15:43   ` Richard Henderson
2021-07-09  7:25 ` [PATCH v1 1/5] target/riscv: Expose interrupt pending bits as " Philippe Mathieu-Daudé
2021-07-09 15:20 ` Richard Henderson
2021-07-10 14:40 ` Bin Meng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAAhSdy3w7NbJLjUd3V2i98a4e3V9TKzQHic8inz1TERXMmW4+A@mail.gmail.com \
    --to=anup@brainfault.org \
    --cc=alistair.francis@wdc.com \
    --cc=alistair23@gmail.com \
    --cc=bmeng.cn@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).