On 3/9/25 14:35, Djordje Todorovic wrote:
On 1. 9. 25. 13:05, Philippe Mathieu-Daudé wrote:
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 1/9/25 10:17, Djordje Todorovic wrote:
On 8. 8. 25. 17:52, Philippe Mathieu-Daudé wrote:
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 17/7/25 11:38, 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 <[email protected]>
Signed-off-by: Djordje Todorovic <[email protected]>
---
hw/intc/riscv_aclint.c | 21 +++++++++++++++++++--
hw/intc/riscv_aplic.c | 11 ++++++++---
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
index b0139f03f5..22ac4133d5 100644
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -292,7 +292,13 @@ 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 *cpu_by_hartid = cpu_by_arch_id(s->hartid_base + i);
+ if (cpu_by_hartid == NULL) {
+ qemu_log_mask(LOG_GUEST_ERROR, "aclint-mtimer: invalid
hartid: %u",
+ s->hartid_base + i);
DeviceRealize() handlers are part of machine modelling, not guest uses.
IOW, triggering this is a programming mistake, so we should just
abort() here.
Well, if we do it that way, our Boston board target for P8700 cannot
run.
So the problem is elsewhere :)
I see. Would something like this be acceptable:
--- a/hw/intc/riscv_aclint.c
+++ b/hw/intc/riscv_aclint.c
@@ -279,7 +279,7 @@ static const Property
riscv_aclint_mtimer_properties[] = {
static void riscv_aclint_mtimer_realize(DeviceState *dev, Error **errp)
{
RISCVAclintMTimerState *s = RISCV_ACLINT_MTIMER(dev);
- int i;
+ CPUState *cpu;
memory_region_init_io(&s->mmio, OBJECT(dev), &riscv_aclint_mtimer_ops,
s, TYPE_RISCV_ACLINT_MTIMER, s->aperture_size);
@@ -291,18 +291,15 @@ static void
riscv_aclint_mtimer_realize(DeviceState *dev, Error **errp)
s->timers = g_new0(QEMUTimer *, s->num_harts);
s->timecmp = g_new0(uint64_t, s->num_harts);
/* Claim timer interrupt bits */
- for (i = 0; i < s->num_harts; i++) {
- CPUState *cpu_by_hartid = cpu_by_arch_id(s->hartid_base + i);
- if (cpu_by_hartid == NULL) {
- qemu_log_mask(LOG_GUEST_ERROR, "aclint-mtimer: invalid
hartid: %u",
- s->hartid_base + i);
- continue;
- }
- RISCVCPU *cpu = RISCV_CPU(cpu_by_hartid);
- if (riscv_cpu_claim_interrupts(cpu, MIP_MTIP) < 0) {
- error_report("MTIP already claimed");
- exit(1);
- }
+ CPU_FOREACH(cpu) {
+ if (cpu == NULL)
+ abort();
Why do you end having a NULL vcpu in the global cpus_queue?
(this is the 'elsewhere problem').
+
+ RISCVCPU *cpu_riscv = RISCV_CPU(cpu);
+ if (riscv_cpu_claim_interrupts(cpu_riscv, MIP_MTIP) < 0) {
+ error_report("MTIP already claimed");
+ exit(1);
+ }
}
}
Thanks,
Djordje
