Re: [PATCH v2 3/5] hw/intc: ibex_plic: Convert the PLIC to use RISC-V CPU GPIO lines

[Bin Meng]

On Wed, Jul 14, 2021 at 3:25 PM 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 external MIP bits.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/hw/intc/ibex_plic.h |  2 ++
>  hw/intc/ibex_plic.c         | 17 ++++++-----------
>  hw/riscv/opentitan.c        |  8 ++++++++
>  3 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/intc/ibex_plic.h b/include/hw/intc/ibex_plic.h
> index 7fc495db99..d596436e06 100644
> --- a/include/hw/intc/ibex_plic.h
> +++ b/include/hw/intc/ibex_plic.h
> @@ -60,6 +60,8 @@ struct IbexPlicState {
>      uint32_t threshold_base;
>
>      uint32_t claim_base;
> +
> +    qemu_irq *external_irqs;
>  };
>
>  #endif /* HW_IBEX_PLIC_H */
> diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c
> index edf76e4f61..ff430356f8 100644
> --- a/hw/intc/ibex_plic.c
> +++ b/hw/intc/ibex_plic.c
> @@ -27,6 +27,7 @@
>  #include "target/riscv/cpu_bits.h"
>  #include "target/riscv/cpu.h"
>  #include "hw/intc/ibex_plic.h"
> +#include "hw/irq.h"
>
>  static bool addr_between(uint32_t addr, uint32_t base, uint32_t num)
>  {
> @@ -92,19 +93,10 @@ static bool ibex_plic_irqs_pending(IbexPlicState *s, 
> uint32_t context)
>
>  static void ibex_plic_update(IbexPlicState *s)
>  {
> -    CPUState *cpu;
> -    int level, i;
> +    int i;
>
>      for (i = 0; i < s->num_cpus; i++) {
> -        cpu = qemu_get_cpu(i);
> -
> -        if (!cpu) {
> -            continue;
> -        }
> -
> -        level = ibex_plic_irqs_pending(s, 0);
> -
> -        riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, BOOL_TO_MASK(level));
> +        qemu_set_irq(s->external_irqs[i], ibex_plic_irqs_pending(s, 0));
>      }
>  }
>
> @@ -268,6 +260,9 @@ static void ibex_plic_realize(DeviceState *dev, Error 
> **errp)
>
>      qdev_init_gpio_in(dev, ibex_plic_irq_request, s->num_sources);
>
> +    s->external_irqs = g_malloc(sizeof(qemu_irq) * s->num_cpus);
> +    qdev_init_gpio_out(dev, s->external_irqs, s->num_cpus);
> +
>      /*
>       * We can't allow the supervisor to control SEIP as this would allow the
>       * supervisor to clear a pending external interrupt which will result in
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index c5a7e3bacb..88a0200972 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -116,6 +116,7 @@ static void lowrisc_ibex_soc_realize(DeviceState 
> *dev_soc, Error **errp)
>      MachineState *ms = MACHINE(qdev_get_machine());
>      LowRISCIbexSoCState *s = RISCV_IBEX_SOC(dev_soc);
>      MemoryRegion *sys_mem = get_system_memory();
> +    int i;
>
>      object_property_set_str(OBJECT(&s->cpus), "cpu-type", ms->cpu_type,
>                              &error_abort);
> @@ -142,6 +143,13 @@ static void lowrisc_ibex_soc_realize(DeviceState 
> *dev_soc, Error **errp)
>      }
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->plic), 0, memmap[IBEX_DEV_PLIC].base);
>
> +    for (i = 0; i < ms->smp.cpus; i++) {
> +        CPUState *cpu = qemu_get_cpu(i);
> +
> +        qdev_connect_gpio_out_named(DEVICE(&s->plic), NULL, 0,

I think this should be: qdev_connect_gpio(DEVICE(&s->plic), NULL, i,

> +                                    qdev_get_gpio_in(DEVICE(cpu), 
> IRQ_M_EXT));
> +    }
> +
>      /* UART */
>      qdev_prop_set_chr(DEVICE(&(s->uart)), "chardev", serial_hd(0));
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->uart), errp)) {

Regards,
Bin