Re: [PATCH 1/1] hw/intc/riscv_aclint:Change the way to get CPUState from hard-base to pu_index

[Dongxue Zhang]

Reviewed-by: Dongxue Zhang <zhangdongxue@canaan-creative.com>


On Thu, Nov 9, 2023 at 10:22 AM Leo Hou <LeoHou@canaan-creative.com> wrote:
>
> From: Leo Hou <houyingle@canaan-creative.com>
>
> cpu_by_arch_id() uses hartid-base as the index to obtain the corresponding 
> CPUState structure variable.
> qemu_get_cpu() uses cpu_index as the index to obtain the corresponding 
> CPUState structure variable.
>
> In heterogeneous CPU or multi-socket scenarios, multiple aclint needs to be 
> instantiated,
> and the hartid-base of each cpu bound by aclint can start from 0. If 
> cpu_by_arch_id() is still used
> in this case, all aclint will bind to the earliest initialized hart with 
> hartid-base 0 and cause conflicts.
>
> So with cpu_index as the index, use qemu_get_cpu() to get the CPUState struct 
> variable,
> and connect the aclint interrupt line to the hart of the CPU indexed with 
> cpu_index
> (the corresponding hartid-base can start at 0). It's more reasonable.
>
> Signed-off-by: Leo Hou <houyingle@canaan-creative.com>
> ---
>  hw/intc/riscv_aclint.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> index ab1a0b4b3a..be8f539fcb 100644
> --- a/hw/intc/riscv_aclint.c
> +++ b/hw/intc/riscv_aclint.c
> @@ -130,7 +130,7 @@ static uint64_t riscv_aclint_mtimer_read(void *opaque, 
> hwaddr addr,
>          addr < (mtimer->timecmp_base + (mtimer->num_harts << 3))) {
>          size_t hartid = mtimer->hartid_base +
>                          ((addr - mtimer->timecmp_base) >> 3);
> -        CPUState *cpu = cpu_by_arch_id(hartid);
> +        CPUState *cpu = qemu_get_cpu(hartid);
>          CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>          if (!env) {
>              qemu_log_mask(LOG_GUEST_ERROR,
> @@ -173,7 +173,7 @@ static void riscv_aclint_mtimer_write(void *opaque, 
> hwaddr addr,
>          addr < (mtimer->timecmp_base + (mtimer->num_harts << 3))) {
>          size_t hartid = mtimer->hartid_base +
>                          ((addr - mtimer->timecmp_base) >> 3);
> -        CPUState *cpu = cpu_by_arch_id(hartid);
> +        CPUState *cpu = qemu_get_cpu(hartid);
>          CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>          if (!env) {
>              qemu_log_mask(LOG_GUEST_ERROR,
> @@ -232,7 +232,7 @@ static void riscv_aclint_mtimer_write(void *opaque, 
> hwaddr addr,
>
>          /* Check if timer interrupt is triggered for each hart. */
>          for (i = 0; i < mtimer->num_harts; i++) {
> -            CPUState *cpu = cpu_by_arch_id(mtimer->hartid_base + i);
> +            CPUState *cpu = qemu_get_cpu(mtimer->hartid_base + i);
>              CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>              if (!env) {
>                  continue;
> @@ -293,7 +293,7 @@ static void riscv_aclint_mtimer_realize(DeviceState *dev, 
> Error **errp)
>      s->timecmp = g_new0(uint64_t, s->num_harts);
>      /* Claim timer interrupt bits */
>      for (i = 0; i < s->num_harts; i++) {
> -        RISCVCPU *cpu = RISCV_CPU(cpu_by_arch_id(s->hartid_base + i));
> +        RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(s->hartid_base + i));
>          if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
>              error_report("MTIP already claimed");
>              exit(1);
> @@ -373,7 +373,7 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, 
> hwaddr size,
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
>      for (i = 0; i < num_harts; i++) {
> -        CPUState *cpu = cpu_by_arch_id(hartid_base + i);
> +        CPUState *cpu = qemu_get_cpu(hartid_base + i);
>          RISCVCPU *rvcpu = RISCV_CPU(cpu);
>          CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>          riscv_aclint_mtimer_callback *cb =
> @@ -408,7 +408,7 @@ static uint64_t riscv_aclint_swi_read(void *opaque, 
> hwaddr addr,
>
>      if (addr < (swi->num_harts << 2)) {
>          size_t hartid = swi->hartid_base + (addr >> 2);
> -        CPUState *cpu = cpu_by_arch_id(hartid);
> +        CPUState *cpu = qemu_get_cpu(hartid);
>          CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>          if (!env) {
>              qemu_log_mask(LOG_GUEST_ERROR,
> @@ -431,7 +431,7 @@ static void riscv_aclint_swi_write(void *opaque, hwaddr 
> addr, uint64_t value,
>
>      if (addr < (swi->num_harts << 2)) {
>          size_t hartid = swi->hartid_base + (addr >> 2);
> -        CPUState *cpu = cpu_by_arch_id(hartid);
> +        CPUState *cpu = qemu_get_cpu(hartid);
>          CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>          if (!env) {
>              qemu_log_mask(LOG_GUEST_ERROR,
> @@ -546,7 +546,7 @@ DeviceState *riscv_aclint_swi_create(hwaddr addr, 
> uint32_t hartid_base,
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>
>      for (i = 0; i < num_harts; i++) {
> -        CPUState *cpu = cpu_by_arch_id(hartid_base + i);
> +        CPUState *cpu = qemu_get_cpu(hartid_base + i);
>          RISCVCPU *rvcpu = RISCV_CPU(cpu);
>
>          qdev_connect_gpio_out(dev, i,
> --
> 2.34.1
>