Re: [PATCH v1 4/5] hw/intc: sifive_plic: Convert the PLIC to use RISC-V CPU GPIO lines

2021-07-12 Thread Anup Patel
On Fri, Jul 9, 2021 at 9:06 AM Alistair Francis
 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 
> ---
>  include/hw/intc/sifive_plic.h |  4 
>  hw/intc/sifive_plic.c | 38 ---
>  hw/riscv/microchip_pfsoc.c|  2 +-
>  hw/riscv/shakti_c.c   |  3 ++-
>  hw/riscv/sifive_e.c   |  2 +-
>  hw/riscv/sifive_u.c   |  2 +-
>  hw/riscv/virt.c   |  3 ++-
>  7 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/intc/sifive_plic.h b/include/hw/intc/sifive_plic.h
> index 1e451a270c..da1dc64c6d 100644
> --- a/include/hw/intc/sifive_plic.h
> +++ b/include/hw/intc/sifive_plic.h
> @@ -72,9 +72,13 @@ struct SiFivePLICState {
>  uint32_t context_base;
>  uint32_t context_stride;
>  uint32_t aperture_size;
> +
> +qemu_irq *s_external_irqs;
> +qemu_irq *m_external_irqs;
>  };
>
>  DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> +uint32_t num_harts,
>  uint32_t hartid_base, uint32_t num_sources,
>  uint32_t num_priorities, uint32_t priority_base,
>  uint32_t pending_base, uint32_t enable_base,
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index 78903beb06..dc17b55408 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -29,6 +29,7 @@
>  #include "hw/intc/sifive_plic.h"
>  #include "target/riscv/cpu.h"
>  #include "migration/vmstate.h"
> +#include "hw/irq.h"
>
>  #define RISCV_DEBUG_PLIC 0
>
> @@ -139,18 +140,22 @@ static void sifive_plic_update(SiFivePLICState *plic)
>  for (addrid = 0; addrid < plic->num_addrs; addrid++) {
>  uint32_t hartid = plic->addr_config[addrid].hartid;
>  PLICMode mode = plic->addr_config[addrid].mode;
> -CPUState *cpu = qemu_get_cpu(hartid);
> -CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
> -if (!env) {
> -continue;
> -}
>  int level = sifive_plic_irqs_pending(plic, addrid);
> +
>  switch (mode) {
>  case PLICMode_M:
> -riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, 
> BOOL_TO_MASK(level));
> +if (level) {
> +qemu_irq_raise(plic->m_external_irqs[hartid]);
> +} else {
> +qemu_irq_lower(plic->m_external_irqs[hartid]);
> +}
>  break;
>  case PLICMode_S:
> -riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
> BOOL_TO_MASK(level));
> +if (level) {
> +qemu_irq_raise(plic->s_external_irqs[hartid]);
> +} else {
> +qemu_irq_lower(plic->s_external_irqs[hartid]);
> +}
>  break;
>  default:
>  break;
> @@ -456,6 +461,12 @@ static void sifive_plic_realize(DeviceState *dev, Error 
> **errp)
>  sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mmio);
>  qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
>
> +plic->s_external_irqs = g_malloc(sizeof(qemu_irq) * plic->num_harts);
> +qdev_init_gpio_out(dev, plic->s_external_irqs, plic->num_harts);
> +
> +plic->m_external_irqs = g_malloc(sizeof(qemu_irq) * plic->num_harts);
> +qdev_init_gpio_out(dev, plic->m_external_irqs, plic->num_harts);
> +
>  /* 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
>   * lost a interrupt in the case a PLIC is attached. The SEIP bit must be
> @@ -520,6 +531,7 @@ type_init(sifive_plic_register_types)
>   * Create PLIC device.
>   */
>  DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> +uint32_t num_harts,
>  uint32_t hartid_base, uint32_t num_sources,
>  uint32_t num_priorities, uint32_t priority_base,
>  uint32_t pending_base, uint32_t enable_base,
> @@ -527,6 +539,8 @@ DeviceState *sifive_plic_create(hwaddr addr, char 
> *hart_config,
>  uint32_t context_stride, uint32_t aperture_size)
>  {
>  DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> +int i;
> +
>  assert(enable_stride == (enable_stride & -enable_stride));
>  assert(context_stride == (context_stride & -context_stride));
>  qdev_prop_set_string(dev, "hart-config", hart_config);
> @@ -542,5 +556,15 @@ DeviceState *sifive_plic_create(hwaddr addr, char 
> *hart_config,
>  qdev_prop_set_uint32(dev, "aperture-size", aperture_size);
>  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _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);
> +
> +qdev_connect_gpio_out_named(dev, NULL, i,
> +qdev_get_gpio_in(DEVICE(cpu), 
> IRQ_S_EXT));
> +qdev_connect_gpio_out_named(dev, NULL, num_harts + i,
> +   

Re: [PATCH v1 4/5] hw/intc: sifive_plic: Convert the PLIC to use RISC-V CPU GPIO lines

2021-07-12 Thread Anup Patel
On Fri, Jul 9, 2021 at 9:06 AM Alistair Francis
 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 
> ---
>  include/hw/intc/sifive_plic.h |  4 
>  hw/intc/sifive_plic.c | 38 ---
>  hw/riscv/microchip_pfsoc.c|  2 +-
>  hw/riscv/shakti_c.c   |  3 ++-
>  hw/riscv/sifive_e.c   |  2 +-
>  hw/riscv/sifive_u.c   |  2 +-
>  hw/riscv/virt.c   |  3 ++-
>  7 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/intc/sifive_plic.h b/include/hw/intc/sifive_plic.h
> index 1e451a270c..da1dc64c6d 100644
> --- a/include/hw/intc/sifive_plic.h
> +++ b/include/hw/intc/sifive_plic.h
> @@ -72,9 +72,13 @@ struct SiFivePLICState {
>  uint32_t context_base;
>  uint32_t context_stride;
>  uint32_t aperture_size;
> +
> +qemu_irq *s_external_irqs;
> +qemu_irq *m_external_irqs;
>  };
>
>  DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> +uint32_t num_harts,
>  uint32_t hartid_base, uint32_t num_sources,
>  uint32_t num_priorities, uint32_t priority_base,
>  uint32_t pending_base, uint32_t enable_base,
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index 78903beb06..dc17b55408 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -29,6 +29,7 @@
>  #include "hw/intc/sifive_plic.h"
>  #include "target/riscv/cpu.h"
>  #include "migration/vmstate.h"
> +#include "hw/irq.h"
>
>  #define RISCV_DEBUG_PLIC 0
>
> @@ -139,18 +140,22 @@ static void sifive_plic_update(SiFivePLICState *plic)
>  for (addrid = 0; addrid < plic->num_addrs; addrid++) {
>  uint32_t hartid = plic->addr_config[addrid].hartid;
>  PLICMode mode = plic->addr_config[addrid].mode;
> -CPUState *cpu = qemu_get_cpu(hartid);
> -CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
> -if (!env) {
> -continue;
> -}
>  int level = sifive_plic_irqs_pending(plic, addrid);
> +
>  switch (mode) {
>  case PLICMode_M:
> -riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, 
> BOOL_TO_MASK(level));
> +if (level) {
> +qemu_irq_raise(plic->m_external_irqs[hartid]);
> +} else {
> +qemu_irq_lower(plic->m_external_irqs[hartid]);
> +}
>  break;
>  case PLICMode_S:
> -riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
> BOOL_TO_MASK(level));
> +if (level) {
> +qemu_irq_raise(plic->s_external_irqs[hartid]);
> +} else {
> +qemu_irq_lower(plic->s_external_irqs[hartid]);
> +}

All qemu_irq_xyz() calls are broken for multi-socket, just like CLINT.

Please use "hartid - plic->hartid_base" as index.

Regards,
Anup

>  break;
>  default:
>  break;
> @@ -456,6 +461,12 @@ static void sifive_plic_realize(DeviceState *dev, Error 
> **errp)
>  sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mmio);
>  qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
>
> +plic->s_external_irqs = g_malloc(sizeof(qemu_irq) * plic->num_harts);
> +qdev_init_gpio_out(dev, plic->s_external_irqs, plic->num_harts);
> +
> +plic->m_external_irqs = g_malloc(sizeof(qemu_irq) * plic->num_harts);
> +qdev_init_gpio_out(dev, plic->m_external_irqs, plic->num_harts);
> +
>  /* 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
>   * lost a interrupt in the case a PLIC is attached. The SEIP bit must be
> @@ -520,6 +531,7 @@ type_init(sifive_plic_register_types)
>   * Create PLIC device.
>   */
>  DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
> +uint32_t num_harts,
>  uint32_t hartid_base, uint32_t num_sources,
>  uint32_t num_priorities, uint32_t priority_base,
>  uint32_t pending_base, uint32_t enable_base,
> @@ -527,6 +539,8 @@ DeviceState *sifive_plic_create(hwaddr addr, char 
> *hart_config,
>  uint32_t context_stride, uint32_t aperture_size)
>  {
>  DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
> +int i;
> +
>  assert(enable_stride == (enable_stride & -enable_stride));
>  assert(context_stride == (context_stride & -context_stride));
>  qdev_prop_set_string(dev, "hart-config", hart_config);
> @@ -542,5 +556,15 @@ DeviceState *sifive_plic_create(hwaddr addr, char 
> *hart_config,
>  qdev_prop_set_uint32(dev, "aperture-size", aperture_size);
>  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _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);
> +
> +qdev_connect_gpio_out_named(dev, NULL, i,
> +

Re: [PATCH v1 4/5] hw/intc: sifive_plic: Convert the PLIC to use RISC-V CPU GPIO lines

2021-07-09 Thread Richard Henderson

On 7/8/21 8:31 PM, Alistair Francis wrote:

  switch (mode) {
  case PLICMode_M:
-riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, 
BOOL_TO_MASK(level));
+if (level) {
+qemu_irq_raise(plic->m_external_irqs[hartid]);
+} else {
+qemu_irq_lower(plic->m_external_irqs[hartid]);
+}
  break;
  case PLICMode_S:
-riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
BOOL_TO_MASK(level));
+if (level) {
+qemu_irq_raise(plic->s_external_irqs[hartid]);
+} else {
+qemu_irq_lower(plic->s_external_irqs[hartid]);
+}
  break;


qemu_irq_set.  Otherwise,

Reviewed-by: Richard Henderson 

r~



Re: [PATCH v1 4/5] hw/intc: sifive_plic: Convert the PLIC to use RISC-V CPU GPIO lines

2021-07-09 Thread Philippe Mathieu-Daudé
On 7/9/21 5:31 AM, Alistair Francis 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 
> ---
>  include/hw/intc/sifive_plic.h |  4 
>  hw/intc/sifive_plic.c | 38 ---
>  hw/riscv/microchip_pfsoc.c|  2 +-
>  hw/riscv/shakti_c.c   |  3 ++-
>  hw/riscv/sifive_e.c   |  2 +-
>  hw/riscv/sifive_u.c   |  2 +-
>  hw/riscv/virt.c   |  3 ++-
>  7 files changed, 42 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 



[PATCH v1 4/5] hw/intc: sifive_plic: Convert the PLIC to use RISC-V CPU GPIO lines

2021-07-08 Thread Alistair Francis
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 
---
 include/hw/intc/sifive_plic.h |  4 
 hw/intc/sifive_plic.c | 38 ---
 hw/riscv/microchip_pfsoc.c|  2 +-
 hw/riscv/shakti_c.c   |  3 ++-
 hw/riscv/sifive_e.c   |  2 +-
 hw/riscv/sifive_u.c   |  2 +-
 hw/riscv/virt.c   |  3 ++-
 7 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/include/hw/intc/sifive_plic.h b/include/hw/intc/sifive_plic.h
index 1e451a270c..da1dc64c6d 100644
--- a/include/hw/intc/sifive_plic.h
+++ b/include/hw/intc/sifive_plic.h
@@ -72,9 +72,13 @@ struct SiFivePLICState {
 uint32_t context_base;
 uint32_t context_stride;
 uint32_t aperture_size;
+
+qemu_irq *s_external_irqs;
+qemu_irq *m_external_irqs;
 };
 
 DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
+uint32_t num_harts,
 uint32_t hartid_base, uint32_t num_sources,
 uint32_t num_priorities, uint32_t priority_base,
 uint32_t pending_base, uint32_t enable_base,
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index 78903beb06..dc17b55408 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -29,6 +29,7 @@
 #include "hw/intc/sifive_plic.h"
 #include "target/riscv/cpu.h"
 #include "migration/vmstate.h"
+#include "hw/irq.h"
 
 #define RISCV_DEBUG_PLIC 0
 
@@ -139,18 +140,22 @@ static void sifive_plic_update(SiFivePLICState *plic)
 for (addrid = 0; addrid < plic->num_addrs; addrid++) {
 uint32_t hartid = plic->addr_config[addrid].hartid;
 PLICMode mode = plic->addr_config[addrid].mode;
-CPUState *cpu = qemu_get_cpu(hartid);
-CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
-if (!env) {
-continue;
-}
 int level = sifive_plic_irqs_pending(plic, addrid);
+
 switch (mode) {
 case PLICMode_M:
-riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, 
BOOL_TO_MASK(level));
+if (level) {
+qemu_irq_raise(plic->m_external_irqs[hartid]);
+} else {
+qemu_irq_lower(plic->m_external_irqs[hartid]);
+}
 break;
 case PLICMode_S:
-riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
BOOL_TO_MASK(level));
+if (level) {
+qemu_irq_raise(plic->s_external_irqs[hartid]);
+} else {
+qemu_irq_lower(plic->s_external_irqs[hartid]);
+}
 break;
 default:
 break;
@@ -456,6 +461,12 @@ static void sifive_plic_realize(DeviceState *dev, Error 
**errp)
 sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mmio);
 qdev_init_gpio_in(dev, sifive_plic_irq_request, plic->num_sources);
 
+plic->s_external_irqs = g_malloc(sizeof(qemu_irq) * plic->num_harts);
+qdev_init_gpio_out(dev, plic->s_external_irqs, plic->num_harts);
+
+plic->m_external_irqs = g_malloc(sizeof(qemu_irq) * plic->num_harts);
+qdev_init_gpio_out(dev, plic->m_external_irqs, plic->num_harts);
+
 /* 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
  * lost a interrupt in the case a PLIC is attached. The SEIP bit must be
@@ -520,6 +531,7 @@ type_init(sifive_plic_register_types)
  * Create PLIC device.
  */
 DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
+uint32_t num_harts,
 uint32_t hartid_base, uint32_t num_sources,
 uint32_t num_priorities, uint32_t priority_base,
 uint32_t pending_base, uint32_t enable_base,
@@ -527,6 +539,8 @@ DeviceState *sifive_plic_create(hwaddr addr, char 
*hart_config,
 uint32_t context_stride, uint32_t aperture_size)
 {
 DeviceState *dev = qdev_new(TYPE_SIFIVE_PLIC);
+int i;
+
 assert(enable_stride == (enable_stride & -enable_stride));
 assert(context_stride == (context_stride & -context_stride));
 qdev_prop_set_string(dev, "hart-config", hart_config);
@@ -542,5 +556,15 @@ DeviceState *sifive_plic_create(hwaddr addr, char 
*hart_config,
 qdev_prop_set_uint32(dev, "aperture-size", aperture_size);
 sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _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);
+
+qdev_connect_gpio_out_named(dev, NULL, i,
+qdev_get_gpio_in(DEVICE(cpu), IRQ_S_EXT));
+qdev_connect_gpio_out_named(dev, NULL, num_harts + i,
+qdev_get_gpio_in(DEVICE(cpu), IRQ_M_EXT));
+}
+
 return dev;
 }
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index eb8e79e0a1..eef55f69fd 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -274,7 +274,7 @@ static void