This is needed for riscv based CPUs by MIPS since those may have
sparse hart-ID layouts. ACLINT and APLIC still assume a dense
range, and if a hart is missing, this causes NULL derefs.
Signed-off-by: Chao-ying Fu <cfu@mips.com>
Signed-off-by: Djordje Todorovic <djordje.todorovic@htecgroup.com>
---
hw/intc/riscv_aclint.c | 27 +++++++++++++++++++++++++--
hw/intc/riscv_aplic.c | 10 +++++++---
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index b0139f03f5..ef1fc57610 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -233,6 +233,9 @@ 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);
+ if (cpu == NULL) {
+ continue;
+ }
CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
if (!env) {
continue;
@@ -292,7 +295,11 @@ 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));
+ CPUState *temp = cpu_by_arch_id(s->hartid_base + i);
+ if (temp == NULL) {
+ continue;
+ }
+ RISCVCPU *cpu = RISCV_CPU(temp);
if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
error_report("MTIP already claimed");
exit(1);
@@ -373,6 +380,9 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, hwaddr size,
for (i = 0; i < num_harts; i++) {
CPUState *cpu = cpu_by_arch_id(hartid_base + i);
+ if (cpu == NULL) {
+ continue;
+ }
RISCVCPU *rvcpu = RISCV_CPU(cpu);
CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
riscv_aclint_mtimer_callback *cb =
@@ -408,6 +418,9 @@ 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);
+ if (cpu == NULL) {
+ return 0;
+ }
CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
if (!env) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -431,6 +444,9 @@ 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);
+ if (cpu == NULL) {
+ return;
+ }
CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
if (!env) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -481,7 +497,11 @@ static void riscv_aclint_swi_realize(DeviceState *dev, Error **errp)
/* Claim software interrupt bits */
for (i = 0; i < swi->num_harts; i++) {
- RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(swi->hartid_base + i));
+ CPUState *temp = cpu_by_arch_id(swi->hartid_base + i);
+ if (temp == NULL) {
+ continue;
+ }
+ RISCVCPU *cpu = RISCV_CPU(temp);
/* We don't claim mip.SSIP because it is writable by software */
if (riscv_cpu_claim_interrupts(cpu, swi->sswi ? 0 : MIP_MSIP) < 0) {
error_report("MSIP already claimed");
@@ -545,6 +565,9 @@ DeviceState *riscv_aclint_swi_create(hwaddr addr, uint32_t hartid_base,
for (i = 0; i < num_harts; i++) {
CPUState *cpu = cpu_by_arch_id(hartid_base + i);
+ if (cpu == NULL) {
+ continue;
+ }
RISCVCPU *rvcpu = RISCV_CPU(cpu);
qdev_connect_gpio_out(dev, i,
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 8bcd9f4697..360a3dc117 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -899,9 +899,11 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
if (!aplic->msimode) {
/* Claim the CPU interrupt to be triggered by this APLIC */
for (i = 0; i < aplic->num_harts; i++) {
- RISCVCPU *cpu;
-
- cpu = RISCV_CPU(cpu_by_arch_id(aplic->hartid_base + i));
+ CPUState *temp = cpu_by_arch_id(aplic->hartid_base + i);
+ if (temp == NULL) {
+ continue;
+ }
+ RISCVCPU *cpu = RISCV_CPU(temp);
if (riscv_cpu_claim_interrupts(cpu,
(aplic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
error_report("%s already claimed",
@@ -1076,6 +1078,8 @@ DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
if (!msimode) {
for (i = 0; i < num_harts; i++) {
CPUState *cpu = cpu_by_arch_id(hartid_base + i);
+ if (cpu == NULL)
+ continue;
qdev_connect_gpio_out_named(dev, NULL, i,
qdev_get_gpio_in(DEVICE(cpu),
--
2.34.1
On 6/18/25 9:27 AM, Djordje Todorovic wrote:
> This is needed for riscv based CPUs by MIPS since those may have
> sparse hart-ID layouts. ACLINT and APLIC still assume a dense
> range, and if a hart is missing, this causes NULL derefs.
>
> Signed-off-by: Chao-ying Fu <cfu@mips.com>
> Signed-off-by: Djordje Todorovic <djordje.todorovic@htecgroup.com>
> ---
> hw/intc/riscv_aclint.c | 27 +++++++++++++++++++++++++--
> hw/intc/riscv_aplic.c | 10 +++++++---
> 2 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> index b0139f03f5..ef1fc57610 100644
> --- a/hw/intc/riscv_aclint.c
> +++ b/hw/intc/riscv_aclint.c
> @@ -233,6 +233,9 @@ 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);
> + if (cpu == NULL) {
> + continue;
> + }
> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
> if (!env) {
> continue;
> @@ -292,7 +295,11 @@ 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));
> + CPUState *temp = cpu_by_arch_id(s->hartid_base + i);
> + if (temp == NULL) {
> + continue;
> + }
> + RISCVCPU *cpu = RISCV_CPU(temp);
> if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
> error_report("MTIP already claimed");
> exit(1);
> @@ -373,6 +380,9 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr addr, hwaddr size,
>
> for (i = 0; i < num_harts; i++) {
> CPUState *cpu = cpu_by_arch_id(hartid_base + i);
> + if (cpu == NULL) {
> + continue;
> + }
> RISCVCPU *rvcpu = RISCV_CPU(cpu);
> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
> riscv_aclint_mtimer_callback *cb =
> @@ -408,6 +418,9 @@ 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);
> + if (cpu == NULL) {
> + return 0;
> + }
I don't think we need this change, and the one below in riscv_aclint_swi_write(). The
existing code is handling the case where cpu == NULL:
size_t hartid = swi->hartid_base + (addr >> 2);
CPUState *cpu = cpu_by_arch_id(hartid);
CPURISCVState *env = cpu ? cpu_env(cpu) : NULL; <-------------
if (!env) { <-----------------
qemu_log_mask(LOG_GUEST_ERROR,
"aclint-swi: invalid hartid: %zu", hartid);
} else if {...}
In fact what we have is better: we do a qemu_log() informing about the invalid hart-id
instead of silently returning.
Thanks,
Daniel
> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
> if (!env) {
> qemu_log_mask(LOG_GUEST_ERROR,
> @@ -431,6 +444,9 @@ 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);
> + if (cpu == NULL) {
> + return;
> + }
> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
> if (!env) {
> qemu_log_mask(LOG_GUEST_ERROR,
> @@ -481,7 +497,11 @@ static void riscv_aclint_swi_realize(DeviceState *dev, Error **errp)
>
> /* Claim software interrupt bits */
> for (i = 0; i < swi->num_harts; i++) {
> - RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(swi->hartid_base + i));
> + CPUState *temp = cpu_by_arch_id(swi->hartid_base + i);
> + if (temp == NULL) {
> + continue;
> + }
> + RISCVCPU *cpu = RISCV_CPU(temp);
> /* We don't claim mip.SSIP because it is writable by software */
> if (riscv_cpu_claim_interrupts(cpu, swi->sswi ? 0 : MIP_MSIP) < 0) {
> error_report("MSIP already claimed");
> @@ -545,6 +565,9 @@ DeviceState *riscv_aclint_swi_create(hwaddr addr, uint32_t hartid_base,
>
> for (i = 0; i < num_harts; i++) {
> CPUState *cpu = cpu_by_arch_id(hartid_base + i);
> + if (cpu == NULL) {
> + continue;
> + }
> RISCVCPU *rvcpu = RISCV_CPU(cpu);
>
> qdev_connect_gpio_out(dev, i,
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index 8bcd9f4697..360a3dc117 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -899,9 +899,11 @@ static void riscv_aplic_realize(DeviceState *dev, Error **errp)
> if (!aplic->msimode) {
> /* Claim the CPU interrupt to be triggered by this APLIC */
> for (i = 0; i < aplic->num_harts; i++) {
> - RISCVCPU *cpu;
> -
> - cpu = RISCV_CPU(cpu_by_arch_id(aplic->hartid_base + i));
> + CPUState *temp = cpu_by_arch_id(aplic->hartid_base + i);
> + if (temp == NULL) {
> + continue;
> + }
> + RISCVCPU *cpu = RISCV_CPU(temp);
> if (riscv_cpu_claim_interrupts(cpu,
> (aplic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
> error_report("%s already claimed",
> @@ -1076,6 +1078,8 @@ DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
> if (!msimode) {
> for (i = 0; i < num_harts; i++) {
> CPUState *cpu = cpu_by_arch_id(hartid_base + i);
> + if (cpu == NULL)
> + continue;
>
> qdev_connect_gpio_out_named(dev, NULL, i,
> qdev_get_gpio_in(DEVICE(cpu),
On 18. 6. 25. 19:48, Daniel Henrique Barboza wrote:
> [You don't often get email from dbarboza@ventanamicro.com. Learn why
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you recognize the sender
> and know the content is safe.
>
>
> On 6/18/25 9:27 AM, Djordje Todorovic wrote:
>> This is needed for riscv based CPUs by MIPS since those may have
>> sparse hart-ID layouts. ACLINT and APLIC still assume a dense
>> range, and if a hart is missing, this causes NULL derefs.
>>
>> Signed-off-by: Chao-ying Fu <cfu@mips.com>
>> Signed-off-by: Djordje Todorovic <djordje.todorovic@htecgroup.com>
>> ---
>> hw/intc/riscv_aclint.c | 27 +++++++++++++++++++++++++--
>> hw/intc/riscv_aplic.c | 10 +++++++---
>> 2 files changed, 32 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
>> index b0139f03f5..ef1fc57610 100644
>> --- a/hw/intc/riscv_aclint.c
>> +++ b/hw/intc/riscv_aclint.c
>> @@ -233,6 +233,9 @@ 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);
>> + if (cpu == NULL) {
>> + continue;
>> + }
>> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>> if (!env) {
>> continue;
>> @@ -292,7 +295,11 @@ 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));
>> + CPUState *temp = cpu_by_arch_id(s->hartid_base + i);
>> + if (temp == NULL) {
>> + continue;
>> + }
>> + RISCVCPU *cpu = RISCV_CPU(temp);
>> if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
>> error_report("MTIP already claimed");
>> exit(1);
>> @@ -373,6 +380,9 @@ DeviceState *riscv_aclint_mtimer_create(hwaddr
>> addr, hwaddr size,
>>
>> for (i = 0; i < num_harts; i++) {
>> CPUState *cpu = cpu_by_arch_id(hartid_base + i);
>> + if (cpu == NULL) {
>> + continue;
>> + }
>> RISCVCPU *rvcpu = RISCV_CPU(cpu);
>> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>> riscv_aclint_mtimer_callback *cb =
>> @@ -408,6 +418,9 @@ 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);
>> + if (cpu == NULL) {
>> + return 0;
>> + }
>
> I don't think we need this change, and the one below in
> riscv_aclint_swi_write(). The
> existing code is handling the case where cpu == NULL:
>
> size_t hartid = swi->hartid_base + (addr >> 2);
> CPUState *cpu = cpu_by_arch_id(hartid);
> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL; <-------------
> if (!env) { <-----------------
> qemu_log_mask(LOG_GUEST_ERROR,
> "aclint-swi: invalid hartid: %zu", hartid);
> } else if {...}
>
>
> In fact what we have is better: we do a qemu_log() informing about the
> invalid hart-id
> instead of silently returning.
>
Yes. We do not need all of those. But I will add logging for the cases
we do.
Thanks,
Djordje
>
> Thanks,
>
> Daniel
>
>
>> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>> if (!env) {
>> qemu_log_mask(LOG_GUEST_ERROR,
>> @@ -431,6 +444,9 @@ 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);
>> + if (cpu == NULL) {
>> + return;
>> + }
>> CPURISCVState *env = cpu ? cpu_env(cpu) : NULL;
>> if (!env) {
>> qemu_log_mask(LOG_GUEST_ERROR,
>> @@ -481,7 +497,11 @@ static void riscv_aclint_swi_realize(DeviceState
>> *dev, Error **errp)
>>
>> /* Claim software interrupt bits */
>> for (i = 0; i < swi->num_harts; i++) {
>> - RISCVCPU *cpu = RISCV_CPU(qemu_get_cpu(swi->hartid_base + i));
>> + CPUState *temp = cpu_by_arch_id(swi->hartid_base + i);
>> + if (temp == NULL) {
>> + continue;
>> + }
>> + RISCVCPU *cpu = RISCV_CPU(temp);
>> /* We don't claim mip.SSIP because it is writable by
>> software */
>> if (riscv_cpu_claim_interrupts(cpu, swi->sswi ? 0 :
>> MIP_MSIP) < 0) {
>> error_report("MSIP already claimed");
>> @@ -545,6 +565,9 @@ DeviceState *riscv_aclint_swi_create(hwaddr addr,
>> uint32_t hartid_base,
>>
>> for (i = 0; i < num_harts; i++) {
>> CPUState *cpu = cpu_by_arch_id(hartid_base + i);
>> + if (cpu == NULL) {
>> + continue;
>> + }
>> RISCVCPU *rvcpu = RISCV_CPU(cpu);
>>
>> qdev_connect_gpio_out(dev, i,
>> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
>> index 8bcd9f4697..360a3dc117 100644
>> --- a/hw/intc/riscv_aplic.c
>> +++ b/hw/intc/riscv_aplic.c
>> @@ -899,9 +899,11 @@ static void riscv_aplic_realize(DeviceState
>> *dev, Error **errp)
>> if (!aplic->msimode) {
>> /* Claim the CPU interrupt to be triggered by this
>> APLIC */
>> for (i = 0; i < aplic->num_harts; i++) {
>> - RISCVCPU *cpu;
>> -
>> - cpu = RISCV_CPU(cpu_by_arch_id(aplic->hartid_base +
>> i));
>> + CPUState *temp = cpu_by_arch_id(aplic->hartid_base +
>> i);
>> + if (temp == NULL) {
>> + continue;
>> + }
>> + RISCVCPU *cpu = RISCV_CPU(temp);
>> if (riscv_cpu_claim_interrupts(cpu,
>> (aplic->mmode) ? MIP_MEIP : MIP_SEIP) < 0) {
>> error_report("%s already claimed",
>> @@ -1076,6 +1078,8 @@ DeviceState *riscv_aplic_create(hwaddr addr,
>> hwaddr size,
>> if (!msimode) {
>> for (i = 0; i < num_harts; i++) {
>> CPUState *cpu = cpu_by_arch_id(hartid_base + i);
>> + if (cpu == NULL)
>> + continue;
>>
>> qdev_connect_gpio_out_named(dev, NULL, i,
>> qdev_get_gpio_in(DEVICE(cpu),
>
© 2016 - 2025 Red Hat, Inc.