On 3/28/22 06:57, Xiaojuan Yang wrote:
This patch realize the EIOINTC interrupt controller.
Signed-off-by: Xiaojuan Yang <yangxiaoj...@loongson.cn>
Signed-off-by: Song Gao <gaos...@loongson.cn>
---
hw/intc/Kconfig | 3 +
hw/intc/loongarch_extioi.c | 408 +++++++++++++++++++++++++++++
hw/intc/meson.build | 1 +
hw/intc/trace-events | 11 +
hw/loongarch/Kconfig | 1 +
include/hw/intc/loongarch_extioi.h | 77 ++++++
6 files changed, 501 insertions(+)
create mode 100644 hw/intc/loongarch_extioi.c
create mode 100644 include/hw/intc/loongarch_extioi.h
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 71c04c328e..28bd1f185d 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -96,3 +96,6 @@ config LOONGARCH_PCH_MSI
select MSI_NONBROKEN
bool
select UNIMP
+
+config LOONGARCH_EXTIOI
+ bool
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
new file mode 100644
index 0000000000..af28e8d6e9
--- /dev/null
+++ b/hw/intc/loongarch_extioi.c
@@ -0,0 +1,408 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongson 3A5000 ext interrupt controller emulation
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/loongarch.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "hw/intc/loongarch_extioi.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void extioi_update_irq(void *opaque, int irq_num, int level)
+{
+ LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
I think this is not opaque anymore; you've already resolved it in the caller.
I think level should be 'bool'.
+ uint8_t ipnum, cpu;
+ unsigned long found1, found2;
+
+ ipnum = s->sw_ipmap[irq_num];
+ cpu = s->sw_coremap[irq_num];
+ if (level == 1) {
Just if (level).
+ if (test_bit(irq_num, (void *)s->enable) == false) {
This, and every other cast you're using for bitops.h functions, is wrong. You would need
to declare these bitmaps properly as 'unsigned long name[BITS_TO_LONGS(N)];'.
That said, I would definitely use uint64_t, because that matches up with the description
of these registers in the manual.
+ return;
+ }
+ bitmap_set((void *)s->coreisr[cpu], irq_num, 1);
+ found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
find_next_bit with offset=0 is find_first_bit...
+ bitmap_set((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+
+ if (found1 >= EXTIOI_IRQS) {
+ qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+ }
... but what's the bitmap search doing? It appears to be checking that there are *no*
bits set between 0 and EXTIOI_IRQS, and then raising the irq if no bits set. That seems
wrong.
+ } else {
+ bitmap_clear((void *)s->coreisr[cpu], irq_num, 1);
+ found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+ bitmap_clear((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+ found2 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+
+ if ((found1 < EXTIOI_IRQS) && (found2 >= EXTIOI_IRQS)) {
+ qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+ }
+ }
+}
It *seems* like all of this should be
uint64_t sum = 0;
s->isr[ipnum / 64] = deposit64(s->isr[ipnum / 64], ipnum % 64, 1, level);
for (int i = 0; i < ARRAY_SIZE(s->isr); i++) {
sum |= s->isr[i] & s->ena[i];
}
qemu_set_irq(parent, sum != 0);
If that's not the case, you need many more comments.
r~